From: Nicolas Dufresne <nicolas.dufresne@collabora.com>
To: Irui Wang <irui.wang@mediatek.com>,
Hans Verkuil <hverkuil-cisco@xs4all.nl>,
Mauro Carvalho Chehab <mchehab@kernel.org>,
Rob Herring <robh+dt@kernel.org>,
Matthias Brugger <matthias.bgg@gmail.com>,
Krzysztof Kozlowski <krzk+dt@kernel.org>,
angelogioacchino.delregno@collabora.com,
Tiffany Lin <tiffany.lin@mediatek.com>,
kyrie wu <kyrie.wu@mediatek.com>
Cc: Yunfei Dong <yunfei.dong@mediatek.com>,
Maoguang Meng <maoguang.meng@mediatek.com>,
Longfei Wang <longfei.wang@mediatek.com>,
Project_Global_Chrome_Upstream_Group@mediatek.com,
linux-media@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-mediatek@lists.infradead.org
Subject: Re: [PATCH v6 4/6] media: mediatek: encoder: Add support for VCP encode process
Date: Wed, 29 Apr 2026 10:56:33 -0400 [thread overview]
Message-ID: <62415d732ccaed02bf12a8e68e10ac8d58b87fa9.camel@collabora.com> (raw)
In-Reply-To: <20260423073345.27402-5-irui.wang@mediatek.com>
[-- Attachment #1: Type: text/plain, Size: 9102 bytes --]
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@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
> } 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 050b827f0fd0..d981155aeb8c 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
> @@ -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);
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
next prev parent reply other threads:[~2026-04-29 14:56 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-23 7:33 [PATCH v6 0/6] Add support for MT8196 video encoder Irui Wang
2026-04-23 7:33 ` [PATCH v6 1/6] media: dt-bindings: mediatek,vcodec-encoder: Add MT8196 Irui Wang
2026-04-23 7:33 ` [PATCH v6 2/6] media: mediatek: encoder: Add a new encoder driver interface Irui Wang
2026-04-29 14:49 ` Nicolas Dufresne
2026-04-23 7:33 ` [PATCH v6 3/6] media: mediatek: encoder: Add support for common firmware interface Irui Wang
2026-04-23 7:33 ` [PATCH v6 4/6] media: mediatek: encoder: Add support for VCP encode process Irui Wang
2026-04-29 14:56 ` Nicolas Dufresne [this message]
2026-04-23 7:33 ` [PATCH v6 5/6] media: mediatek: encoder: Add a new platform data member Irui Wang
2026-04-23 7:33 ` [PATCH v6 6/6] media: mediatek: encoder: Add MT8196 encoder compatible data Irui Wang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=62415d732ccaed02bf12a8e68e10ac8d58b87fa9.camel@collabora.com \
--to=nicolas.dufresne@collabora.com \
--cc=Project_Global_Chrome_Upstream_Group@mediatek.com \
--cc=angelogioacchino.delregno@collabora.com \
--cc=devicetree@vger.kernel.org \
--cc=hverkuil-cisco@xs4all.nl \
--cc=irui.wang@mediatek.com \
--cc=krzk+dt@kernel.org \
--cc=kyrie.wu@mediatek.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=longfei.wang@mediatek.com \
--cc=maoguang.meng@mediatek.com \
--cc=matthias.bgg@gmail.com \
--cc=mchehab@kernel.org \
--cc=robh+dt@kernel.org \
--cc=tiffany.lin@mediatek.com \
--cc=yunfei.dong@mediatek.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox