On Thu, Jul 23, 2020 at 6:40 AM Ezequiel Garcia
[off-list ref] wrote:

On Mon, 13 Jul 2020 at 03:09, Alexandre Courbot [off-list ref] wrote:

quoted

From: Yunfei Dong <yunfei.dong@mediatek.com>

Add support for communicating with the SCP firmware, which will be used
by MT8183.

Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
[acourbot: refactor, cleanup and split]
Co-developed-by: Alexandre Courbot <redacted>
Signed-off-by: Alexandre Courbot <redacted>
Acked-by: Tiffany Lin <tiffany.lin@mediatek.com>
---
 drivers/media/platform/Kconfig                |  1 +
 .../platform/mtk-vcodec/mtk_vcodec_dec_drv.c  |  3 +
 .../platform/mtk-vcodec/mtk_vcodec_enc_drv.c  |  3 +
 .../media/platform/mtk-vcodec/mtk_vcodec_fw.c | 56 +++++++++++++++++++
 .../media/platform/mtk-vcodec/mtk_vcodec_fw.h |  2 +
 5 files changed, 65 insertions(+)

diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index c57ee78fa99d..f0dbe048efea 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig

@@ -256,6 +256,7 @@ config VIDEO_MEDIATEK_VCODEC
        select VIDEOBUF2_DMA_CONTIG
        select V4L2_MEM2MEM_DEV
        select VIDEO_MEDIATEK_VPU
+       select MTK_SCP
        help
            Mediatek video codec driver provides HW capability to
            encode and decode in a range of video formats

diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_drv.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_drv.c
index 4f07a5fcce7f..5b5765b98e57 100644
--- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_drv.c
+++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_drv.c

@@ -225,6 +225,9 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
        if (!of_property_read_u32(pdev->dev.of_node, "mediatek,vpu",
                                  &rproc_phandle)) {
                fw_type = VPU;
+       } else if (!of_property_read_u32(pdev->dev.of_node, "mediatek,scp",
+                                        &rproc_phandle)) {
+               fw_type = SCP;
        } else {
                mtk_v4l2_err("Could not get vdec IPI device");
                return -ENODEV;

diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_drv.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_drv.c
index 4340ea10afd0..42530cd01a30 100644
--- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_drv.c
+++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_drv.c

@@ -233,6 +233,9 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
        if (!of_property_read_u32(pdev->dev.of_node, "mediatek,vpu",
                                  &rproc_phandle)) {
                fw_type = VPU;
+       } else if (!of_property_read_u32(pdev->dev.of_node, "mediatek,scp",
+                                        &rproc_phandle)) {
+               fw_type = SCP;
        } else {
                mtk_v4l2_err("Could not get venc IPI device");
                return -ENODEV;

diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_fw.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_fw.c
index 967bb100a990..f2a62ea62fc6 100644
--- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_fw.c
+++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_fw.c

@@ -19,6 +19,7 @@ struct mtk_vcodec_fw {
        enum mtk_vcodec_fw_type type;
        const struct mtk_vcodec_fw_ops *ops;
        struct platform_device *pdev;
+       struct mtk_scp *scp;
 };

 static int mtk_vcodec_vpu_load_firmware(struct mtk_vcodec_fw *fw)

@@ -71,6 +72,48 @@ static const struct mtk_vcodec_fw_ops mtk_vcodec_vpu_msg = {
        .ipi_send = mtk_vcodec_vpu_ipi_send,
 };

+static int mtk_vcodec_scp_load_firmware(struct mtk_vcodec_fw *fw)
+{
+       return rproc_boot(scp_get_rproc(fw->scp));
+}
+
+static unsigned int mtk_vcodec_scp_get_vdec_capa(struct mtk_vcodec_fw *fw)
+{
+       return scp_get_vdec_hw_capa(fw->scp);
+}
+
+static unsigned int mtk_vcodec_scp_get_venc_capa(struct mtk_vcodec_fw *fw)
+{
+       return scp_get_venc_hw_capa(fw->scp);
+}
+
+static void *mtk_vcodec_vpu_scp_dm_addr(struct mtk_vcodec_fw *fw,
+                                       u32 dtcm_dmem_addr)
+{
+       return scp_mapping_dm_addr(fw->scp, dtcm_dmem_addr);
+}
+
+static int mtk_vcodec_scp_set_ipi_register(struct mtk_vcodec_fw *fw, int id,
+               mtk_vcodec_ipi_handler handler, const char *name, void *priv)
+{
+       return scp_ipi_register(fw->scp, id, handler, priv);
+}
+
+static int mtk_vcodec_scp_ipi_send(struct mtk_vcodec_fw *fw, int id, void *buf,
+               unsigned int len, unsigned int wait)
+{
+       return scp_ipi_send(fw->scp, id, buf, len, wait);
+}
+
+static const struct mtk_vcodec_fw_ops mtk_vcodec_rproc_msg = {
+       .load_firmware = mtk_vcodec_scp_load_firmware,
+       .get_vdec_capa = mtk_vcodec_scp_get_vdec_capa,
+       .get_venc_capa = mtk_vcodec_scp_get_venc_capa,
+       .map_dm_addr = mtk_vcodec_vpu_scp_dm_addr,
+       .ipi_register = mtk_vcodec_scp_set_ipi_register,
+       .ipi_send = mtk_vcodec_scp_ipi_send,
+};
+
 static void mtk_vcodec_reset_handler(void *priv)
 {
        struct mtk_vcodec_dev *dev = priv;

@@ -94,6 +137,7 @@ struct mtk_vcodec_fw *mtk_vcodec_fw_select(struct mtk_vcodec_dev *dev,
        const struct mtk_vcodec_fw_ops *ops;
        struct mtk_vcodec_fw *fw;
        struct platform_device *fw_pdev = NULL;
+       struct mtk_scp *scp = NULL;

        switch (type) {
        case VPU:

@@ -106,6 +150,14 @@ struct mtk_vcodec_fw *mtk_vcodec_fw_select(struct mtk_vcodec_dev *dev,
                vpu_wdt_reg_handler(fw_pdev, mtk_vcodec_reset_handler,
                                    dev, rst_id);
                break;
+       case SCP:
+               ops = &mtk_vcodec_rproc_msg;
+               scp = scp_get(dev->plat_dev);
+               if (!scp) {
+                       mtk_v4l2_err("could not get vdec scp handle");
+                       return ERR_PTR(-EPROBE_DEFER);

I suspect the EPROBE_DEFER should be returned by scp_get
itself instead.

scp_get() is a function of of mtk_scp remoteproc driver, so even if we
decide this is desirable (which I am not convinced, as the current
code leaves the freedom to decide how the absence of SCP should be
interpreted to the driver) this is beyond the scope of this series.

quoted

+               }
+               break;
        default:
                mtk_v4l2_err("invalid vcodec fw type");
                return ERR_PTR(-EINVAL);

@@ -118,6 +170,7 @@ struct mtk_vcodec_fw *mtk_vcodec_fw_select(struct mtk_vcodec_dev *dev,
        fw->type = type;
        fw->ops = ops;
        fw->pdev = fw_pdev;
+       fw->scp = scp;

        return fw;
 }

@@ -129,6 +182,9 @@ void mtk_vcodec_fw_release(struct mtk_vcodec_fw *fw)
        case VPU:
                put_device(&fw->pdev->dev);
                break;
+       case SCP:
+               scp_put(fw->scp);

Interestingly scp_put is a wrapper around put_device :-)
Perhaps not a reason to violate the layering.

I don't see what is wrong with the current code? If SCP is in use, we
use SCP functions to manage it. If in the future SCP involves in such
a way that we need to do more than a put_device(), we are covered. Or
am I missing something?