[PATCH v6 4/6] media: mediatek: encoder: Add support for VCP encode process
Irui Wang (王瑞)
Irui.Wang at mediatek.com
Wed Apr 29 21:08:51 PDT 2026
Dear Nicolas,
Thank you greatly for taking the time to help review the patch.
On Wed, 2026-04-29 at 10:56 -0400, Nicolas Dufresne wrote:
> Le jeudi 23 avril 2026 à 15:33 +0800, Irui Wang a écrit :
> > Adapt the encoder driver to support VCP firmware interface.
> >
> > Set the encoder driver firmware type to 'VCP'.
> > Allocate RC buffers using the VCP device.
> > Send the shared memory address to VCP and map the encoder VSI
> > address
> > to the CPU address space using the VCP shared memory address.
> >
> > Signed-off-by: Irui Wang <irui.wang at mediatek.com>
> > ---
> > .../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 6b69ce44d4bb..2859fe78f67d 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
> > @@ -500,6 +500,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,
> > @@ -508,6 +513,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 4e4541b2fc8e..2f6ee0cd15e3 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
> > @@ -262,6 +262,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;
>
> Similar comment on the patchset you depend on, its third firmware
> type, and time
> to cleanup. The of_device_get_match_data() should be sufficient for
> this simple
> case, there is not reason for custom DT parsing here. PLace the
> fw_type in the
> pdata, and move of_device_get_match_data() call earlier.
>
> Please apply the same logic for the entire patchset. Every bit of
> information
> that is purely static to your device is better placed in the pdata
> then through
> addition of if/else conditions all over the place. The pdata can also
> have ops
> in it, letting easily firmware specific code without having to do
> if/else
> everywhere.
>
> Nicolas
I will clean up this if/else in next patch, and with the logic, I will
review and modify the entire patch set if needed.
Thanks
Best Regards
>
> > } 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_i
> > f.c
> > b/drivers/media/platform/mediatek/vcodec/encoder/venc/venc_common_i
> > f.c
> > index 050b827f0fd0..d981155aeb8c 100644
> > ---
> > a/drivers/media/platform/mediatek/vcodec/encoder/venc/venc_common_i
> > f.c
> > +++
> > b/drivers/media/platform/mediatek/vcodec/encoder/venc/venc_common_i
> > f.c
> > @@ -480,8 +480,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++)
> > @@ -530,12 +535,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 7772b8442ebc..0f4693e04a9f 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);
More information about the Linux-mediatek
mailing list