Re: [PATCH v3 3/6] media: mediatek: encoder: Add support for VCP encode process

From: Nicolas Dufresne

Date: Fri Nov 14 2025 - 16:39:12 EST

Hi,

Le jeudi 14 août 2025 à 16:56 +0800, Kyrie Wu a écrit :
> From: Irui Wang <irui.wang@xxxxxxxxxxxx>
>
> When encoding by VCP interface, encoder driver need change to VCP path:

Please use something like:

Adapt the encoder driver to support VCP firmwares interface.

> Firstly, set encoder driver fw type to 'VCP'. Then, allocate RC buffers

Drop "Firstly, ", go straight to the verb.

Set the encoder driver fw type ...

> by the VCP device. Finally, send the shared memory address into VCP and
> map the encoder vsi address by the VCP shared memory address.

This one is a little too broken for me, please rework this comment. When we say
"map address", we don't expect that to be mapped by another address. "map" is
the action, so I suppose you wanted to specify where it is mapped to ? CPU
address space or device address space ?

Please rework this message.

>
> Signed-off-by: Irui Wang <irui.wang@xxxxxxxxxxxx>
> ---
> .../mediatek/vcodec/common/mtk_vcodec_fw.c    | 6 +++++
> .../mediatek/vcodec/common/mtk_vcodec_fw.h    | 1 +
> .../vcodec/common/mtk_vcodec_fw_priv.h        | 1 +
> .../vcodec/common/mtk_vcodec_fw_vcp.c         | 6 +++++
> .../vcodec/encoder/mtk_vcodec_enc_drv.c       | 3 +++
> .../vcodec/encoder/venc/venc_common_if.c      | 23 ++++++++++++++-----
> .../mediatek/vcodec/encoder/venc_vpu_if.c     | 14 ++++++++++-
> 7 files changed, 47 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.c b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.c
> index 0381acceda25..7a504f093bd8 100644
> --- a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.c
> +++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.c
> @@ -105,3 +105,9 @@ int mtk_vcodec_fw_get_type(struct mtk_vcodec_fw *fw)
> return fw->type;
> }
> EXPORT_SYMBOL_GPL(mtk_vcodec_fw_get_type);
> +
> +struct device *mtk_vcodec_fw_get_dev(struct mtk_vcodec_fw *fw)
> +{
> + return fw->ops->get_fw_dev(fw);
> +}
> +EXPORT_SYMBOL_GPL(mtk_vcodec_fw_get_dev);
> diff --git a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.h b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.h
> index e7304a7dd3e0..56c26b91651e 100644
> --- a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.h
> +++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.h
> @@ -43,5 +43,6 @@ int mtk_vcodec_fw_ipi_send(struct mtk_vcodec_fw *fw, int id,
> int mtk_vcodec_fw_get_type(struct mtk_vcodec_fw *fw);
> int mtk_vcodec_fw_get_ipi(enum mtk_vcodec_fw_type type, int hw_id);
> int mtk_vcodec_fw_get_venc_ipi(enum mtk_vcodec_fw_type type);
> +struct device *mtk_vcodec_fw_get_dev(struct mtk_vcodec_fw *fw);
>
> #endif /* _MTK_VCODEC_FW_H_ */
> diff --git a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_priv.h b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_priv.h
> index 0a2a9b010244..710c83c871f4 100644
> --- a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_priv.h
> +++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_priv.h
> @@ -29,6 +29,7 @@ struct mtk_vcodec_fw_ops {
> int (*ipi_send)(struct mtk_vcodec_fw *fw, int id, void *buf,
> unsigned int len, unsigned int wait);
> void (*release)(struct mtk_vcodec_fw *fw);
> + struct device *(*get_fw_dev)(struct mtk_vcodec_fw *fw);
> };
>
> #if IS_ENABLED(CONFIG_VIDEO_MEDIATEK_VCODEC_VPU)
> diff --git a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.c b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.c
> index f6b93e1bcbf3..646e3944dd4f 100644
> --- a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.c
> +++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.c
> @@ -432,6 +432,11 @@ static void mtk_vcodec_vcp_release(struct mtk_vcodec_fw *fw)
> {
> }
>
> +static struct device *mtk_vcodec_vcp_get_fw_dev(struct mtk_vcodec_fw *fw)
> +{
> + return fw->vcp->vcp_device->dev;
> +}
> +
> static const struct mtk_vcodec_fw_ops mtk_vcodec_vcp_msg = {
> .load_firmware = mtk_vcodec_vcp_load_firmware,
> .get_vdec_capa = mtk_vcodec_vcp_get_vdec_capa,
> @@ -440,6 +445,7 @@ static const struct mtk_vcodec_fw_ops mtk_vcodec_vcp_msg = {
> .ipi_register = mtk_vcodec_vcp_set_ipi_register,
> .ipi_send = mtk_vcodec_vcp_ipi_send,
> .release = mtk_vcodec_vcp_release,
> + .get_fw_dev = mtk_vcodec_vcp_get_fw_dev,
> };
>
> struct mtk_vcodec_fw *mtk_vcodec_fw_vcp_init(void *priv, enum mtk_vcodec_fw_use fw_use)
> diff --git a/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.c b/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.c
> index a1e4483abcdb..bb913dfe5f04 100644
> --- a/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.c
> +++ b/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.c
> @@ -252,6 +252,9 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
> } else if (!of_property_read_u32(pdev->dev.of_node, "mediatek,scp",
> &rproc_phandle)) {
> fw_type = SCP;
> + } else if (!of_property_read_u32(pdev->dev.of_node, "mediatek,vcp",
> + &rproc_phandle)) {
> + fw_type = VCP;
> } else {
> dev_err(&pdev->dev, "[MTK VCODEC] Could not get venc IPI device");
> return -ENODEV;
> diff --git a/drivers/media/platform/mediatek/vcodec/encoder/venc/venc_common_if.c b/drivers/media/platform/mediatek/vcodec/encoder/venc/venc_common_if.c
> index da7cf90bd54b..b28d559285ea 100644
> --- a/drivers/media/platform/mediatek/vcodec/encoder/venc/venc_common_if.c
> +++ b/drivers/media/platform/mediatek/vcodec/encoder/venc/venc_common_if.c
> @@ -478,8 +478,13 @@ static void venc_free_rc_buf(struct venc_inst *inst,
> {
> int i;
> struct device *dev;
> + struct mtk_vcodec_fw *fw = inst->ctx->dev->fw_handler;
> +
> + if (mtk_vcodec_fw_get_type(fw) == VCP)
> + dev = mtk_vcodec_fw_get_dev(fw);
> + else
> + dev = &inst->ctx->dev->plat_dev->dev;
>
> - dev = &inst->ctx->dev->plat_dev->dev;
> mtk_venc_mem_free(inst, dev, &bufs->rc_code);
>
> for (i = 0; i < core_num; i++)
> @@ -528,12 +533,18 @@ static int venc_alloc_rc_buf(struct venc_inst *inst,
> struct device *dev;
> void *tmp_va;
>
> - dev = &inst->ctx->dev->plat_dev->dev;
> - if (mtk_venc_mem_alloc(inst, dev, &bufs->rc_code))
> - return -ENOMEM;
> + if (mtk_vcodec_fw_get_type(fw) == VCP) {
> + dev = mtk_vcodec_fw_get_dev(fw);
> + if (mtk_venc_mem_alloc(inst, dev, &bufs->rc_code))
> + return -ENOMEM;
> + } else {
> + dev = &inst->ctx->dev->plat_dev->dev;
> + if (mtk_venc_mem_alloc(inst, dev, &bufs->rc_code))
> + return -ENOMEM;
>
> - tmp_va = mtk_vcodec_fw_map_dm_addr(fw, bufs->rc_code.pa);
> - memcpy(bufs->rc_code.va, tmp_va, bufs->rc_code.size);
> + tmp_va = mtk_vcodec_fw_map_dm_addr(fw, bufs->rc_code.pa);
> + memcpy(bufs->rc_code.va, tmp_va, bufs->rc_code.size);
> + }
>
> for (i = 0; i < core_num; i++) {
> if (mtk_venc_mem_alloc(inst, dev, &bufs->rc_info[i]))
> diff --git a/drivers/media/platform/mediatek/vcodec/encoder/venc_vpu_if.c b/drivers/media/platform/mediatek/vcodec/encoder/venc_vpu_if.c
> index 537b9955932e..9a90c2271297 100644
> --- a/drivers/media/platform/mediatek/vcodec/encoder/venc_vpu_if.c
> +++ b/drivers/media/platform/mediatek/vcodec/encoder/venc_vpu_if.c
> @@ -8,13 +8,23 @@
> #include "venc_ipi_msg.h"
> #include "venc_vpu_if.h"
>
> +#define VSI_OFFSET_MASK 0x0FFFFFFF
> +
> static void handle_enc_init_msg(struct venc_vpu_inst *vpu, const void *data)
> {
> const struct venc_vpu_ipi_msg_init_comm *msg = data;
> struct mtk_vcodec_fw *fw = vpu->ctx->dev->fw_handler;
> + u64 pa_start, vsi_offset;
>
> vpu->inst_addr = msg->init_ack.vpu_inst_addr;
> - vpu->vsi = mtk_vcodec_fw_map_dm_addr(fw, vpu->inst_addr);
> +
> + if (mtk_vcodec_fw_get_type(fw) == VCP) {
> + pa_start = (u64)fw->vcp->iova_addr;
> + vsi_offset = (msg->vpu_vsi_addr & VSI_OFFSET_MASK) - (pa_start & VSI_OFFSET_MASK);
> + vpu->vsi = mtk_vcodec_fw_map_dm_addr(fw, ENCODER_MEM) + vsi_offset;
> + } else {
> + vpu->vsi = mtk_vcodec_fw_map_dm_addr(fw, msg->vpu_vsi_addr);
> + }
>
> /* Firmware version field value is unspecified on MT8173. */
> if (mtk_vcodec_fw_get_type(fw) == VPU)
> @@ -155,6 +165,8 @@ int vpu_enc_init(struct venc_vpu_inst *vpu)
> out.base.venc_inst = (unsigned long)vpu;
> if (MTK_ENC_DRV_IS_COMM(vpu->ctx)) {
> out.codec_type = vpu->ctx->q_data[MTK_Q_DATA_DST].fmt->fourcc;
> + if (mtk_vcodec_fw_get_type(vpu->ctx->dev->fw_handler) == VCP)
> + out.shared_iova = vpu->ctx->dev->fw_handler->vcp->iova_addr;
> msg_size = sizeof(struct venc_ap_ipi_msg_init_comm);
> } else {
> msg_size = sizeof(struct venc_ap_ipi_msg_init);

Code looks fine to me, so just resend with a comprehensible message please.

cheers,
Nicolas

Attachment: signature.asc
Description: This is a digitally signed message part