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

[Nicolas Dufresne <nicolas.dufresne@collabora.com>]

[-- Attachment #1: Type: text/plain, Size: 9094 bytes --]

Hi,

Le jeudi 14 août 2025 à 16:56 +0800, Kyrie Wu a écrit :
> From: Irui Wang <irui.wang@mediatek.com>
> 
> 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@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 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 #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]