devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/6] Add support for MT8196 video encoder
@ 2025-08-14  8:56 Kyrie Wu
  2025-08-14  8:56 ` [PATCH v3 1/6] media: mediatek: encoder: Add a new encoder driver interface Kyrie Wu
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: Kyrie Wu @ 2025-08-14  8:56 UTC (permalink / raw)
  To: Tiffany Lin, Andrew-CT Chen, Yunfei Dong, Mauro Carvalho Chehab,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
	AngeloGioacchino Del Regno, Kyrie Wu, Hans Verkuil,
	Nicolas Dufresne, Christophe JAILLET, Sebastian Fricke,
	Nathan Hebert, Arnd Bergmann, Irui Wang, George Sun, linux-media,
	devicetree, linux-kernel, linux-arm-kernel, linux-mediatek
  Cc: Neil Armstrong, Andrzej Pietrasiewicz

This patch series add support for MT8196 video encoder.
patch 1~3: Add a new encoder driver interface for new VCP firmware.
patch 4~6: Add MT8196 encoder driver platform data.

About adding new driver support, the v4l2-compliance report shows:
"Total for mtk-vcodec-enc device /dev/video3: 47, Succeeded: 46, Failed: 1, Warnings: 0"
The 1 Failed case is not caused by current patch set.

This patch set depends on "media: mediatek: vcodec: support video decoder in mt8196"[1]

[1] https://patchwork.linuxtv.org/project/linux-media/list/?series=15232

Change in v3:
 - Add venc rc buffer alloc failure error handling.
 - Add mediatek,vcp property definition in dt-bindning.

Change in v2:
 - Add support for VCP encode process.
 - Add MT8196 encoder driver platform data.
 - Rebase encoder patch onto decoder's patch set.
 - Fix some review comments in v1.

Irui Wang (6):
  media: mediatek: encoder: Add a new encoder driver interface
  media: mediatek: encoder: Add support for common firmware interface
  media: mediatek: encoder: Add support for VCP encode process
  media: mediatek: encoder: Add a new platform data member
  dt-bindings: media: Add MT8196 mediatek,vcodec-encoder
  media: mediatek: encoder: Add MT8196 encoder compatible data

 .../media/mediatek,vcodec-encoder.yaml        |  17 +
 .../mediatek/vcodec/common/mtk_vcodec_fw.c    |  19 +
 .../mediatek/vcodec/common/mtk_vcodec_fw.h    |   2 +
 .../vcodec/common/mtk_vcodec_fw_priv.h        |   1 +
 .../vcodec/common/mtk_vcodec_fw_vcp.c         |   6 +
 .../platform/mediatek/vcodec/encoder/Makefile |   1 +
 .../mediatek/vcodec/encoder/mtk_vcodec_enc.c  |  37 +-
 .../vcodec/encoder/mtk_vcodec_enc_drv.c       |  28 +
 .../vcodec/encoder/mtk_vcodec_enc_drv.h       |  15 +-
 .../vcodec/encoder/venc/venc_common_if.c      | 683 ++++++++++++++++++
 .../vcodec/encoder/venc/venc_h264_if.c        |   8 +-
 .../mediatek/vcodec/encoder/venc_drv_if.c     |   3 +-
 .../mediatek/vcodec/encoder/venc_drv_if.h     |  11 +-
 .../mediatek/vcodec/encoder/venc_ipi_msg.h    |  26 +
 .../mediatek/vcodec/encoder/venc_vpu_if.c     |  51 +-
 15 files changed, 853 insertions(+), 55 deletions(-)
 create mode 100644 drivers/media/platform/mediatek/vcodec/encoder/venc/venc_common_if.c

-- 
2.46.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v3 1/6] media: mediatek: encoder: Add a new encoder driver interface
  2025-08-14  8:56 [PATCH v3 0/6] Add support for MT8196 video encoder Kyrie Wu
@ 2025-08-14  8:56 ` Kyrie Wu
  2025-08-14  8:56 ` [PATCH v3 2/6] media: mediatek: encoder: Add support for common firmware interface Kyrie Wu
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Kyrie Wu @ 2025-08-14  8:56 UTC (permalink / raw)
  To: Tiffany Lin, Andrew-CT Chen, Yunfei Dong, Mauro Carvalho Chehab,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
	AngeloGioacchino Del Regno, Kyrie Wu, Hans Verkuil,
	Nicolas Dufresne, Christophe JAILLET, Sebastian Fricke,
	Nathan Hebert, Arnd Bergmann, Irui Wang, George Sun, linux-media,
	devicetree, linux-kernel, linux-arm-kernel, linux-mediatek
  Cc: Neil Armstrong, Andrzej Pietrasiewicz

From: Irui Wang <irui.wang@mediatek.com>

Introduce a new encoder kernel driver interface to ensure compatibility
with the updated encoder software driver running in firmware.
The new driver interface is expected to support more encoder formats,
share more encode parameters between kernel and firmware.

Signed-off-by: Irui Wang <irui.wang@mediatek.com>
---
 .../mediatek/vcodec/common/mtk_vcodec_fw.c    |  13 +
 .../mediatek/vcodec/common/mtk_vcodec_fw.h    |   1 +
 .../platform/mediatek/vcodec/encoder/Makefile |   1 +
 .../mediatek/vcodec/encoder/mtk_vcodec_enc.c  |  14 +-
 .../vcodec/encoder/mtk_vcodec_enc_drv.h       |   8 +-
 .../vcodec/encoder/venc/venc_common_if.c      | 672 ++++++++++++++++++
 .../vcodec/encoder/venc/venc_h264_if.c        |   8 +-
 .../mediatek/vcodec/encoder/venc_drv_if.h     |  11 +-
 8 files changed, 710 insertions(+), 18 deletions(-)
 create mode 100644 drivers/media/platform/mediatek/vcodec/encoder/venc/venc_common_if.c

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 4ed7639dfa30..0381acceda25 100644
--- a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.c
+++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.c
@@ -18,6 +18,19 @@ int mtk_vcodec_fw_get_ipi(enum mtk_vcodec_fw_type type, int hw_id)
 }
 EXPORT_SYMBOL_GPL(mtk_vcodec_fw_get_ipi);
 
+int mtk_vcodec_fw_get_venc_ipi(enum mtk_vcodec_fw_type type)
+{
+	switch (type) {
+	case SCP:
+		return SCP_IPI_VENC_H264;
+	case VCP:
+		return VCP_IPI_ENCODER;
+	default:
+		return -EINVAL;
+	}
+}
+EXPORT_SYMBOL_GPL(mtk_vcodec_fw_get_venc_ipi);
+
 struct mtk_vcodec_fw *mtk_vcodec_fw_select(void *priv, enum mtk_vcodec_fw_type type,
 					   enum mtk_vcodec_fw_use fw_use)
 {
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 142e2e87905c..e7304a7dd3e0 100644
--- a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.h
+++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.h
@@ -42,5 +42,6 @@ int mtk_vcodec_fw_ipi_send(struct mtk_vcodec_fw *fw, int id,
 			   void *buf, unsigned int len, unsigned int wait);
 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);
 
 #endif /* _MTK_VCODEC_FW_H_ */
diff --git a/drivers/media/platform/mediatek/vcodec/encoder/Makefile b/drivers/media/platform/mediatek/vcodec/encoder/Makefile
index e621b5b7e5e6..9d3229d56e39 100644
--- a/drivers/media/platform/mediatek/vcodec/encoder/Makefile
+++ b/drivers/media/platform/mediatek/vcodec/encoder/Makefile
@@ -4,6 +4,7 @@ obj-$(CONFIG_VIDEO_MEDIATEK_VCODEC) += mtk-vcodec-enc.o
 
 mtk-vcodec-enc-y := venc/venc_vp8_if.o \
 		venc/venc_h264_if.o \
+		venc/venc_common_if.o \
 		mtk_vcodec_enc.o \
 		mtk_vcodec_enc_drv.o \
 		mtk_vcodec_enc_pm.o \
diff --git a/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc.c b/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc.c
index dcafb1303c29..03512bf7a072 100644
--- a/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc.c
+++ b/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc.c
@@ -81,11 +81,11 @@ static int vidioc_venc_s_ctrl(struct v4l2_ctrl *ctrl)
 		break;
 	case V4L2_CID_MPEG_VIDEO_H264_PROFILE:
 		mtk_v4l2_venc_dbg(2, ctx, "V4L2_CID_MPEG_VIDEO_H264_PROFILE val = %d", ctrl->val);
-		p->h264_profile = ctrl->val;
+		p->profile = ctrl->val;
 		break;
 	case V4L2_CID_MPEG_VIDEO_H264_LEVEL:
 		mtk_v4l2_venc_dbg(2, ctx, "V4L2_CID_MPEG_VIDEO_H264_LEVEL val = %d", ctrl->val);
-		p->h264_level = ctrl->val;
+		p->level = ctrl->val;
 		break;
 	case V4L2_CID_MPEG_VIDEO_H264_I_PERIOD:
 		mtk_v4l2_venc_dbg(2, ctx, "V4L2_CID_MPEG_VIDEO_H264_I_PERIOD val = %d", ctrl->val);
@@ -386,8 +386,8 @@ static void mtk_venc_set_param(struct mtk_vcodec_enc_ctx *ctx,
 		mtk_v4l2_venc_err(ctx, "Unsupported fourcc =%d", q_data_src->fmt->fourcc);
 		break;
 	}
-	param->h264_profile = enc_params->h264_profile;
-	param->h264_level = enc_params->h264_level;
+	param->profile = enc_params->profile;
+	param->level = enc_params->level;
 
 	/* Config visible resolution */
 	param->width = q_data_src->visible_width;
@@ -403,8 +403,8 @@ static void mtk_venc_set_param(struct mtk_vcodec_enc_ctx *ctx,
 
 	mtk_v4l2_venc_dbg(0, ctx,
 			  "fmt 0x%x, P/L %d/%d w/h %d/%d buf %d/%d fps/bps %d/%d gop %d i_per %d",
-			  param->input_yuv_fmt, param->h264_profile,
-			  param->h264_level, param->width, param->height,
+			  param->input_yuv_fmt, param->profile,
+			  param->level, param->width, param->height,
 			  param->buf_width, param->buf_height,
 			  param->frm_rate, param->bitrate,
 			  param->gop_size, param->intra_period);
@@ -1174,6 +1174,8 @@ static void mtk_venc_worker(struct work_struct *work)
 		frm_buf.fb_addr[i].size =
 				(size_t)src_buf->vb2_buf.planes[i].length;
 	}
+	frm_buf.num_planes = src_buf->vb2_buf.num_planes;
+
 	bs_buf.va = vb2_plane_vaddr(&dst_buf->vb2_buf, 0);
 	bs_buf.dma_addr = vb2_dma_contig_plane_dma_addr(&dst_buf->vb2_buf, 0);
 	bs_buf.size = (size_t)dst_buf->vb2_buf.planes[0].length;
diff --git a/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.h b/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.h
index 0bd85d0fb379..b1277bcfcf53 100644
--- a/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.h
+++ b/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.h
@@ -69,8 +69,8 @@ enum mtk_encode_param {
  * @framerate_denom: frame rate denominator. ex: framerate_num=30 and
  *		     framerate_denom=1 means FPS is 30
  * @h264_max_qp: Max value for H.264 quantization parameter
- * @h264_profile: V4L2 defined H.264 profile
- * @h264_level: V4L2 defined H.264 level
+ * @profile: V4L2 defined profile
+ * @level: V4L2 defined level
  * @force_intra: force/insert intra frame
  */
 struct mtk_enc_params {
@@ -84,8 +84,8 @@ struct mtk_enc_params {
 	unsigned int	framerate_num;
 	unsigned int	framerate_denom;
 	unsigned int	h264_max_qp;
-	unsigned int	h264_profile;
-	unsigned int	h264_level;
+	unsigned int	profile;
+	unsigned int	level;
 	unsigned int	force_intra;
 };
 
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
new file mode 100644
index 000000000000..da7cf90bd54b
--- /dev/null
+++ b/drivers/media/platform/mediatek/vcodec/encoder/venc/venc_common_if.c
@@ -0,0 +1,672 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2025 MediaTek Inc.
+ */
+
+#include "../mtk_vcodec_enc.h"
+#include "../mtk_vcodec_enc_drv.h"
+#include "../venc_drv_base.h"
+#include "../venc_drv_if.h"
+#include "../venc_vpu_if.h"
+#include "../../common/mtk_vcodec_intr.h"
+#include "../../common/mtk_vcodec_util.h"
+
+#define SEQ_HEADER_SIZE 1024
+#define PPS_SIZE 128
+#define MAX_DPB_SIZE 16
+#define MAX_VENC_CORE 3
+#define VENC_CONFIG_LENGTH 115
+#define VENC_CONFIG_DATA 128
+#define VENC_PIC_BITSTREAM_BYTE_CNT 0x0098
+
+/**
+ * enum venc_bs_mode - encode bitstream mode
+ * @VENC_BS_MODE_SPS: encode sps
+ * @VENC_BS_MODE_PPS: encode pps
+ * @VENC_BS_MODE_VPS: encode vps
+ * @VENC_BS_MODE_SEQ_HDR: encode sequence header
+ * @VENC_BS_MODE_FRAME: encode frame
+ * @VENC_BS_MODE_FRAME_FINAL: encode final frame
+ * @VENC_BS_MODE_MAX: max value
+ */
+enum venc_bs_mode {
+	VENC_BS_MODE_SPS = 0,
+	VENC_BS_MODE_PPS,
+	VENC_BS_MODE_VPS,
+	VENC_BS_MODE_SEQ_HDR,
+	VENC_BS_MODE_FRAME,
+	VENC_BS_MODE_FRAME_FINAL,
+	VENC_BS_MODE_MAX
+};
+
+/**
+ * struct venc_config - Structure for encoder configuration
+ *                      AP-W/R : AP is writer/reader on this item
+ *                      MCU-W/R: MCU is write/reader on this item
+ * @input_fourcc: input format fourcc
+ * @bitrate: target bitrate (in bps)
+ * @pic_w: visible width of resolution
+ * @pic_h: visible height of resolution
+ * @buf_w: buffer alignment width of resolution
+ * @buf_h: buffer alignment height of resolution
+ * @gop_size: group of picture size (IDR frame period)
+ * @intra_period: I frame period
+ * @framerate: frame rate in fps
+ * @profile: profile_idc in SPS
+ * @level: level_idc in SPS
+ * @core_num: encoder core num
+ * @dpb_size: encode dpb size
+ * @reserved: reserved fields config
+ */
+struct venc_config {
+	__u32 input_fourcc;
+	__u32 bitrate;
+	__u32 pic_w;
+	__u32 pic_h;
+	__u32 buf_w;
+	__u32 buf_h;
+	__u32 gop_size;
+	__u32 intra_period;
+	__u32 framerate;
+	__u32 profile;
+	__u32 level;
+	__u32 core_num;
+	__u32 dpb_size;
+	__u32 reserved[VENC_CONFIG_LENGTH];
+};
+
+/**
+ * struct venc_config_data - Structure for configuration data
+ * @config_data: extended configuration data besides the basic configuration
+ */
+struct venc_config_data {
+	unsigned int config_data[VENC_CONFIG_DATA];
+};
+
+/**
+ * struct venc_work_buf - Structure for working buffer information
+ *                               AP-W/R : AP is writer/reader on this item
+ *                               MCU-W/R: MCU is write/reader on this item
+ * @iova: IO virtual address
+ * @pa: physical address
+ * @pa_64: for 64bit pa padding
+ * @va: virtual address
+ * @va_padding: for 64bit va padding
+ * @size: buffer size
+ * @size_padding: for 64bit size padding
+ */
+struct venc_work_buf {
+	unsigned long long iova;
+	union {
+		unsigned int pa;
+		unsigned long long pa_64;
+	};
+	union {
+		void *va;
+		unsigned long long va_padding;
+	};
+	union {
+		unsigned int size;
+		unsigned long long size_padding;
+	};
+};
+
+/**
+ * struct venc_work_buf_list - Structure for encode working buffer list
+ * @rc_code: RC code buffer
+ * @rc_info: RC info buffer
+ * @luma: luma buffer
+ * @chroma: chroma buffer
+ * @sub_luma: sub luma buffer
+ * @sub_write: sub write buffer
+ * @col_mv: col_mv buffer
+ * @wpp: wpp buffer
+ * @wpp_nbm: wpp nbm buffer
+ * @skip_frame: skip frame buffer
+ */
+struct venc_work_buf_list {
+	struct venc_work_buf rc_code;
+	struct venc_work_buf rc_info[MAX_VENC_CORE];
+	struct venc_work_buf luma[MAX_DPB_SIZE];
+	struct venc_work_buf chroma[MAX_DPB_SIZE];
+	struct venc_work_buf sub_luma[MAX_DPB_SIZE];
+	struct venc_work_buf sub_write[MAX_DPB_SIZE];
+	struct venc_work_buf col_mv[MAX_DPB_SIZE];
+	struct venc_work_buf wpp[MAX_VENC_CORE];
+	struct venc_work_buf wpp_nbm[MAX_VENC_CORE];
+	struct venc_work_buf skip_frame;
+};
+
+/**
+ * struct venc_info -  Structure for encode frame and bs information
+ * @fb_addr: frame buffer address array
+ * @fb_size: frame buffer size array
+ * @bs_addr: bitstream buffer address
+ * @bs_size: bitstream buffer size
+ */
+struct venc_info {
+	unsigned long long fb_addr[VIDEO_MAX_PLANES];
+	unsigned int fb_size[VIDEO_MAX_PLANES];
+	unsigned long long bs_addr;
+	unsigned long long bs_size;
+};
+
+/**
+ * struct venc_vsi - Structure for VCP driver control and info share
+ *                   AP-W/R : AP is writer/reader on this item
+ *                   VCP-W/R: VCP is write/reader on this item
+ * @config: encoder configuration
+ * @data: encoder configuration data
+ * @bufs: encoder working buffers
+ * @venc: encoder information
+ */
+struct venc_vsi {
+	struct venc_config config;
+	struct venc_config_data data;
+	struct venc_work_buf_list bufs;
+	struct venc_info venc;
+};
+
+/**
+ * struct venc_inst - Structure for encoder instance
+ * @hw_base: hardware io address
+ * @pps_buf: PPS buffer
+ * @seq_buf: sequence header buffer
+ * @work_buf_allocated: work buffer allocated or not
+ * @frm_cnt: encoded frame count
+ * @skip_frm_cnt: encoded skip frame count
+ * @prepend_hdr: prepend header flag
+ * @vpu_inst: vpu instance
+ * @vsi: encode vsi
+ * @ctx: encoder context
+ */
+struct venc_inst {
+	void __iomem *hw_base;
+	struct mtk_vcodec_mem pps_buf;
+	struct mtk_vcodec_mem seq_buf;
+	bool work_buf_allocated;
+	unsigned int frm_cnt;
+	unsigned int skip_frm_cnt;
+	unsigned int prepend_hdr;
+	struct venc_vpu_inst vpu_inst;
+	struct venc_vsi *vsi;
+	struct mtk_vcodec_enc_ctx *ctx;
+};
+
+static int venc_init(struct mtk_vcodec_enc_ctx *ctx)
+{
+	int ret = 0;
+	struct venc_inst *inst;
+
+	inst = kzalloc(sizeof(*inst), GFP_KERNEL);
+	if (!inst)
+		return -ENOMEM;
+
+	inst->ctx = ctx;
+	inst->vpu_inst.ctx = ctx;
+	inst->vpu_inst.id = mtk_vcodec_fw_get_venc_ipi(ctx->dev->fw_handler->type);
+	inst->hw_base = mtk_vcodec_get_reg_addr(inst->ctx->dev->reg_base, VENC_SYS);
+
+	ret = vpu_enc_init(&inst->vpu_inst);
+	inst->vsi = (struct venc_vsi *)inst->vpu_inst.vsi;
+
+	if (ret)
+		kfree(inst);
+	else
+		ctx->drv_handle = inst;
+
+	return ret;
+}
+
+static inline u32 venc_read_reg(struct venc_inst *inst, u32 addr)
+{
+	return readl(inst->hw_base + addr);
+}
+
+static unsigned int venc_wait_encode_done(struct venc_inst *inst)
+{
+	unsigned int irq_status = 0;
+	struct mtk_vcodec_enc_ctx *ctx = (struct mtk_vcodec_enc_ctx *)inst->ctx;
+
+	if (!mtk_vcodec_wait_for_done_ctx(ctx, MTK_INST_IRQ_RECEIVED,
+					  WAIT_INTR_TIMEOUT_MS, 0)) {
+		irq_status = ctx->irq_status;
+		mtk_venc_debug(ctx, "irq_status %x <-", irq_status);
+	}
+	return irq_status;
+}
+
+static void venc_set_bufs(struct venc_inst *inst,
+			  struct venc_frm_buf *frm_buf,
+			  struct mtk_vcodec_mem *bs_buf)
+{
+	unsigned int i;
+
+	if (frm_buf) {
+		for (i = 0; i < frm_buf->num_planes; i++) {
+			inst->vsi->venc.fb_addr[i] = frm_buf->fb_addr[i].dma_addr;
+			inst->vsi->venc.fb_size[i] = frm_buf->fb_addr[i].size;
+			mtk_venc_debug(inst->ctx, "%s: fb_buf[%d]: %llx(%d)\n",
+				       __func__, i,
+				       inst->vsi->venc.fb_addr[i],
+				       inst->vsi->venc.fb_size[i]);
+		}
+	}
+
+	if (bs_buf) {
+		inst->vsi->venc.bs_addr = bs_buf->dma_addr;
+		inst->vsi->venc.bs_size = bs_buf->size;
+		mtk_venc_debug(inst->ctx, "%s: bs_buf: %llx(%d)\n",
+			       __func__,
+			       inst->vsi->venc.bs_addr,
+			       (unsigned int)inst->vsi->venc.bs_size);
+	}
+}
+
+static int venc_encode_sps(struct venc_inst *inst,
+			   struct mtk_vcodec_mem *bs_buf,
+			   unsigned int *bs_size)
+{
+	int ret = 0;
+	unsigned int irq_status;
+
+	venc_set_bufs(inst, NULL, bs_buf);
+	ret = vpu_enc_encode(&inst->vpu_inst, VENC_BS_MODE_SPS, NULL, bs_buf, NULL);
+	if (ret)
+		return ret;
+
+	irq_status = venc_wait_encode_done(inst);
+	if (irq_status != MTK_VENC_IRQ_STATUS_SPS) {
+		mtk_venc_err(inst->ctx, "expect irq status %d", MTK_VENC_IRQ_STATUS_SPS);
+		return -EINVAL;
+	}
+
+	*bs_size = venc_read_reg(inst, VENC_PIC_BITSTREAM_BYTE_CNT);
+	mtk_venc_debug(inst->ctx, "sps bs size %d <-", *bs_size);
+
+	return ret;
+}
+
+static int venc_encode_pps(struct venc_inst *inst,
+			   struct mtk_vcodec_mem *bs_buf,
+			   unsigned int *bs_size)
+{
+	int ret = 0;
+	unsigned int irq_status;
+
+	venc_set_bufs(inst, NULL, bs_buf);
+	ret = vpu_enc_encode(&inst->vpu_inst, VENC_BS_MODE_PPS, NULL, bs_buf, NULL);
+	if (ret)
+		return ret;
+
+	irq_status = venc_wait_encode_done(inst);
+	if (irq_status != MTK_VENC_IRQ_STATUS_PPS) {
+		mtk_venc_err(inst->ctx, "expect irq status %d", MTK_VENC_IRQ_STATUS_PPS);
+		return -EINVAL;
+	}
+
+	*bs_size = venc_read_reg(inst, VENC_PIC_BITSTREAM_BYTE_CNT);
+	mtk_venc_debug(inst->ctx, "pps bs size %d <-", *bs_size);
+
+	return ret;
+}
+
+static int venc_encode_header(struct venc_inst *inst,
+			      struct mtk_vcodec_mem *bs_buf,
+			      unsigned int *bs_size)
+{
+	int ret = 0;
+	unsigned int bs_size_sps;
+	unsigned int bs_size_pps;
+
+	ret = venc_encode_sps(inst, bs_buf, &bs_size_sps);
+	if (ret)
+		return ret;
+
+	ret = venc_encode_pps(inst, &inst->pps_buf, &bs_size_pps);
+	if (ret)
+		return ret;
+
+	memcpy(bs_buf->va + bs_size_sps, inst->pps_buf.va, bs_size_pps);
+	*bs_size = bs_size_sps + bs_size_pps;
+
+	return ret;
+}
+
+static int venc_encode_frame(struct venc_inst *inst,
+			     struct venc_frm_buf *frm_buf,
+			     struct mtk_vcodec_mem *bs_buf,
+			     unsigned int *bs_size)
+{
+	int ret = 0;
+	unsigned int irq_status;
+
+	venc_set_bufs(inst, frm_buf, bs_buf);
+	ret = vpu_enc_encode(&inst->vpu_inst, VENC_BS_MODE_FRAME, frm_buf, bs_buf, NULL);
+	if (ret)
+		return ret;
+
+	irq_status = venc_wait_encode_done(inst);
+	if (irq_status != MTK_VENC_IRQ_STATUS_FRM) {
+		mtk_venc_err(inst->ctx, "expect irq status %d", MTK_VENC_IRQ_STATUS_FRM);
+		return -EINVAL;
+	}
+
+	*bs_size = venc_read_reg(inst, VENC_PIC_BITSTREAM_BYTE_CNT);
+
+	++inst->frm_cnt;
+
+	return ret;
+}
+
+static int venc_encode(void *handle,
+		       enum venc_start_opt opt,
+		       struct venc_frm_buf *frm_buf,
+		       struct mtk_vcodec_mem *bs_buf,
+		       struct venc_done_result *result)
+{
+	int ret = 0;
+	struct venc_inst *inst = (struct venc_inst *)handle;
+	struct mtk_vcodec_enc_ctx *ctx;
+	unsigned int bs_size_hdr;
+
+	if (WARN_ON(!inst || !inst->vsi))
+		return -EINVAL;
+
+	ctx = inst->ctx;
+
+	mtk_venc_debug(ctx, "%s: opt: %d\n", __func__, opt);
+
+	enable_irq(ctx->dev->enc_irq);
+	switch (opt) {
+	case VENC_START_OPT_ENCODE_SEQUENCE_HEADER: {
+		ret = venc_encode_header(inst, bs_buf, &bs_size_hdr);
+		if (ret)
+			goto encode_err;
+
+		result->bs_size = bs_size_hdr;
+		result->is_key_frm = false;
+		break;
+	}
+
+	case VENC_START_OPT_ENCODE_FRAME: {
+		if (!inst->prepend_hdr) {
+			ret = venc_encode_frame(inst, frm_buf, bs_buf, &result->bs_size);
+			if (ret)
+				goto encode_err;
+
+			result->is_key_frm = inst->vpu_inst.is_key_frm;
+			break;
+		}
+
+		ret = venc_encode_header(inst, &inst->seq_buf, &bs_size_hdr);
+		if (ret)
+			goto encode_err;
+
+		ret = venc_encode_frame(inst, frm_buf, bs_buf, &result->bs_size);
+		if (ret)
+			goto encode_err;
+
+		memmove(bs_buf->va + bs_size_hdr, bs_buf->va, result->bs_size);
+		memcpy(bs_buf->va, inst->seq_buf.va, bs_size_hdr);
+		result->bs_size += bs_size_hdr;
+
+		inst->prepend_hdr = 0;
+		result->is_key_frm = inst->vpu_inst.is_key_frm;
+		break;
+	}
+
+	default:
+		mtk_venc_err(inst->ctx, "venc_opt %d not supported", opt);
+		ret = -EINVAL;
+		break;
+	}
+
+encode_err:
+	disable_irq(ctx->dev->enc_irq);
+	mtk_venc_debug(ctx, "opt %d, return %d", opt, ret);
+
+	return ret;
+}
+
+static int mtk_venc_mem_alloc(struct venc_inst *inst,
+			      struct device *dev,
+			      struct venc_work_buf *buf)
+{
+	dma_addr_t dma_addr;
+
+	if (WARN_ON(!dev || !buf))
+		return -EINVAL;
+
+	if (buf->size == 0)
+		return 0;
+
+	buf->va = dma_alloc_coherent(dev, buf->size, &dma_addr, GFP_KERNEL);
+	if (!buf->va)
+		return -ENOMEM;
+
+	buf->iova = (unsigned long long)dma_addr;
+
+	mtk_venc_debug(inst->ctx, "allocate buffer, size: %d, va: %p, iova: 0x%llx",
+		       buf->size, buf->va, buf->iova);
+
+	return 0;
+}
+
+static void mtk_venc_mem_free(struct venc_inst *inst,
+			      struct device *dev,
+			      struct venc_work_buf *buf)
+{
+	if (WARN_ON(!dev || !buf))
+		return;
+
+	if (!buf->va)
+		return;
+
+	mtk_venc_debug(inst->ctx, "free buffer, size: %d, va: %p, iova: 0x%llx",
+		       buf->size, buf->va, buf->iova);
+
+	dma_free_coherent(dev, buf->size, buf->va, buf->iova);
+	buf->va = NULL;
+	buf->iova = 0;
+	buf->size = 0;
+}
+
+static void venc_free_rc_buf(struct venc_inst *inst,
+			     struct venc_work_buf_list *bufs,
+			     unsigned int core_num)
+{
+	int i;
+	struct device *dev;
+
+	dev = &inst->ctx->dev->plat_dev->dev;
+	mtk_venc_mem_free(inst, dev, &bufs->rc_code);
+
+	for (i = 0; i < core_num; i++)
+		mtk_venc_mem_free(inst, dev, &bufs->rc_info[i]);
+}
+
+static void venc_free_work_buf(struct venc_inst *inst)
+{
+	int i;
+	struct venc_work_buf_list *bufs = &inst->vsi->bufs;
+	unsigned int core_num = inst->vsi->config.core_num;
+	unsigned int dpb_size = inst->vsi->config.dpb_size;
+	struct device *dev;
+
+	if (bufs->rc_code.va)
+		venc_free_rc_buf(inst, bufs, core_num);
+
+	dev = &inst->ctx->dev->plat_dev->dev;
+
+	for (i = 0; i < core_num; i++) {
+		mtk_venc_mem_free(inst, dev, &bufs->wpp[i]);
+		mtk_venc_mem_free(inst, dev, &bufs->wpp_nbm[i]);
+	}
+
+	for (i = 0; i < dpb_size; i++) {
+		mtk_venc_mem_free(inst, dev, &bufs->luma[i]);
+		mtk_venc_mem_free(inst, dev, &bufs->chroma[i]);
+		mtk_venc_mem_free(inst, dev, &bufs->sub_luma[i]);
+		mtk_venc_mem_free(inst, dev, &bufs->sub_write[i]);
+		mtk_venc_mem_free(inst, dev, &bufs->col_mv[i]);
+	}
+
+	if (inst->pps_buf.va)
+		mtk_vcodec_mem_free(inst->ctx, &inst->pps_buf);
+
+	if (inst->seq_buf.va)
+		mtk_vcodec_mem_free(inst->ctx, &inst->seq_buf);
+}
+
+static int venc_alloc_rc_buf(struct venc_inst *inst,
+			     struct venc_work_buf_list *bufs,
+			     unsigned int core_num)
+{
+	int i;
+	struct mtk_vcodec_fw *fw = inst->ctx->dev->fw_handler;
+	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;
+
+	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]))
+			goto err_rc_buf;
+	}
+
+	return 0;
+
+err_rc_buf:
+	venc_free_rc_buf(inst, bufs, core_num);
+
+	return -ENOMEM;
+}
+
+static int venc_alloc_work_buf(struct venc_inst *inst)
+{
+	int i, ret;
+	struct venc_work_buf_list *bufs = &inst->vsi->bufs;
+	unsigned int core_num = inst->vsi->config.core_num;
+	unsigned int dpb_size = inst->vsi->config.dpb_size;
+	struct device *dev;
+
+	if (bufs->rc_code.size != 0) {
+		ret = venc_alloc_rc_buf(inst, bufs, core_num);
+		if (ret) {
+			mtk_venc_err(inst->ctx, "cannot allocate rc buf");
+			return -ENOMEM;
+		}
+	}
+
+	dev = &inst->ctx->dev->plat_dev->dev;
+
+	for (i = 0; i < core_num; i++) {
+		if (mtk_venc_mem_alloc(inst, dev, &bufs->wpp[i]) ||
+		    mtk_venc_mem_alloc(inst, dev, &bufs->wpp_nbm[i]))
+			goto err_alloc;
+	}
+
+	for (i = 0; i < dpb_size; i++) {
+		if (mtk_venc_mem_alloc(inst, dev, &bufs->luma[i]) ||
+		    mtk_venc_mem_alloc(inst, dev, &bufs->chroma[i]) ||
+		    mtk_venc_mem_alloc(inst, dev, &bufs->sub_luma[i]) ||
+		    mtk_venc_mem_alloc(inst, dev, &bufs->sub_write[i]) ||
+		    mtk_venc_mem_alloc(inst, dev, &bufs->col_mv[i]))
+			goto err_alloc;
+	}
+
+	/* the pps_buf and seq_buf are used by AP side only */
+	inst->pps_buf.size = PPS_SIZE;
+	ret = mtk_vcodec_mem_alloc(inst->ctx, &inst->pps_buf);
+	if (ret) {
+		mtk_venc_err(inst->ctx, "cannot allocate pps_buf");
+		goto err_alloc;
+	}
+
+	inst->seq_buf.size = SEQ_HEADER_SIZE;
+	ret = mtk_vcodec_mem_alloc(inst->ctx, &inst->seq_buf);
+	if (ret) {
+		mtk_venc_err(inst->ctx, "cannot allocate seq_buf");
+		goto err_alloc;
+	}
+	return 0;
+
+err_alloc:
+	venc_free_work_buf(inst);
+	return -ENOMEM;
+}
+
+static int venc_set_param(void *handle,
+			  enum venc_set_param_type type,
+			  struct venc_enc_param *enc_prm)
+{
+	int ret = 0;
+	struct venc_inst *inst = (struct venc_inst *)handle;
+
+	switch (type) {
+	case VENC_SET_PARAM_ENC:
+		if (WARN_ON(!inst->vsi))
+			return -EINVAL;
+		inst->vsi->config.input_fourcc = enc_prm->input_yuv_fmt;
+		inst->vsi->config.bitrate = enc_prm->bitrate;
+		inst->vsi->config.pic_w = enc_prm->width;
+		inst->vsi->config.pic_h = enc_prm->height;
+		inst->vsi->config.buf_w = enc_prm->buf_width;
+		inst->vsi->config.buf_h = enc_prm->buf_height;
+		inst->vsi->config.gop_size = enc_prm->gop_size;
+		inst->vsi->config.framerate = enc_prm->frm_rate;
+		inst->vsi->config.intra_period = enc_prm->intra_period;
+		inst->vsi->config.profile = enc_prm->profile;
+		inst->vsi->config.level = enc_prm->level;
+
+		ret = vpu_enc_set_param(&inst->vpu_inst, type, enc_prm);
+		if (ret)
+			break;
+
+		if (inst->work_buf_allocated) {
+			venc_free_work_buf(inst);
+			inst->work_buf_allocated = false;
+		}
+		ret = venc_alloc_work_buf(inst);
+		if (ret)
+			break;
+		inst->work_buf_allocated = true;
+		break;
+	case VENC_SET_PARAM_PREPEND_HEADER:
+		inst->prepend_hdr = 1;
+		break;
+	default:
+		ret = vpu_enc_set_param(&inst->vpu_inst, type, enc_prm);
+		break;
+	}
+
+	return ret;
+}
+
+static int venc_deinit(void *handle)
+{
+	int ret = 0;
+	struct venc_inst *inst = (struct venc_inst *)handle;
+
+	ret = vpu_enc_deinit(&inst->vpu_inst);
+
+	if (inst->work_buf_allocated)
+		venc_free_work_buf(inst);
+
+	kfree(inst);
+
+	return ret;
+}
+
+const struct venc_common_if venc_if = {
+	.init = venc_init,
+	.encode = venc_encode,
+	.set_param = venc_set_param,
+	.deinit = venc_deinit,
+};
diff --git a/drivers/media/platform/mediatek/vcodec/encoder/venc/venc_h264_if.c b/drivers/media/platform/mediatek/vcodec/encoder/venc/venc_h264_if.c
index 0f63657d8bad..4160dc995806 100644
--- a/drivers/media/platform/mediatek/vcodec/encoder/venc/venc_h264_if.c
+++ b/drivers/media/platform/mediatek/vcodec/encoder/venc/venc_h264_if.c
@@ -723,9 +723,9 @@ static void h264_enc_set_vsi_configs(struct venc_h264_inst *inst,
 	inst->vsi->config.framerate = enc_prm->frm_rate;
 	inst->vsi->config.intra_period = enc_prm->intra_period;
 	inst->vsi->config.profile =
-		h264_get_profile(inst, enc_prm->h264_profile);
+		h264_get_profile(inst, enc_prm->profile);
 	inst->vsi->config.level =
-		h264_get_level(inst, enc_prm->h264_level);
+		h264_get_level(inst, enc_prm->level);
 	inst->vsi->config.wfd = 0;
 }
 
@@ -742,9 +742,9 @@ static void h264_enc_set_vsi_34_configs(struct venc_h264_inst *inst,
 	inst->vsi_34->config.framerate = enc_prm->frm_rate;
 	inst->vsi_34->config.intra_period = enc_prm->intra_period;
 	inst->vsi_34->config.profile =
-		h264_get_profile(inst, enc_prm->h264_profile);
+		h264_get_profile(inst, enc_prm->profile);
 	inst->vsi_34->config.level =
-		h264_get_level(inst, enc_prm->h264_level);
+		h264_get_level(inst, enc_prm->level);
 	inst->vsi_34->config.wfd = 0;
 }
 
diff --git a/drivers/media/platform/mediatek/vcodec/encoder/venc_drv_if.h b/drivers/media/platform/mediatek/vcodec/encoder/venc_drv_if.h
index 889440a436b6..3c2a1b5e9312 100644
--- a/drivers/media/platform/mediatek/vcodec/encoder/venc_drv_if.h
+++ b/drivers/media/platform/mediatek/vcodec/encoder/venc_drv_if.h
@@ -66,8 +66,8 @@ enum venc_set_param_type {
  * struct venc_enc_prm - encoder settings for VENC_SET_PARAM_ENC used in
  *					  venc_if_set_param()
  * @input_fourcc: input yuv format
- * @h264_profile: V4L2 defined H.264 profile
- * @h264_level: V4L2 defined H.264 level
+ * @profile: V4L2 defined profile
+ * @level: V4L2 defined level
  * @width: image width
  * @height: image height
  * @buf_width: buffer width
@@ -79,8 +79,8 @@ enum venc_set_param_type {
  */
 struct venc_enc_param {
 	enum venc_yuv_fmt input_yuv_fmt;
-	unsigned int h264_profile;
-	unsigned int h264_level;
+	unsigned int profile;
+	unsigned int level;
 	unsigned int width;
 	unsigned int height;
 	unsigned int buf_width;
@@ -107,9 +107,11 @@ struct venc_frame_info {
 /*
  * struct venc_frm_buf - frame buffer information used in venc_if_encode()
  * @fb_addr: plane frame buffer addresses
+ * @num_planes: number of planes
  */
 struct venc_frm_buf {
 	struct mtk_vcodec_fb fb_addr[MTK_VCODEC_MAX_PLANES];
+	unsigned int num_planes;
 };
 
 /*
@@ -124,6 +126,7 @@ struct venc_done_result {
 
 extern const struct venc_common_if venc_h264_if;
 extern const struct venc_common_if venc_vp8_if;
+extern const struct venc_common_if venc_if;
 
 /*
  * venc_if_init - Create the driver handle
-- 
2.46.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 2/6] media: mediatek: encoder: Add support for common firmware interface
  2025-08-14  8:56 [PATCH v3 0/6] Add support for MT8196 video encoder Kyrie Wu
  2025-08-14  8:56 ` [PATCH v3 1/6] media: mediatek: encoder: Add a new encoder driver interface Kyrie Wu
@ 2025-08-14  8:56 ` Kyrie Wu
  2025-08-14  8:56 ` [PATCH v3 3/6] media: mediatek: encoder: Add support for VCP encode process Kyrie Wu
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Kyrie Wu @ 2025-08-14  8:56 UTC (permalink / raw)
  To: Tiffany Lin, Andrew-CT Chen, Yunfei Dong, Mauro Carvalho Chehab,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
	AngeloGioacchino Del Regno, Kyrie Wu, Hans Verkuil,
	Nicolas Dufresne, Christophe JAILLET, Sebastian Fricke,
	Nathan Hebert, Arnd Bergmann, Irui Wang, George Sun, linux-media,
	devicetree, linux-kernel, linux-arm-kernel, linux-mediatek
  Cc: Neil Armstrong, Andrzej Pietrasiewicz

From: Irui Wang <irui.wang@mediatek.com>

The existing encoder firmware interface implied just one type of codec:
H.264. Future encoders may support additional codecs; however adding
entire sets of interfaces for them is not scalable.

Instead, a new "common" firmware interface is defined for non codec
specific messages. The new messages encapsulate the old ones for
backward compatibility.

This patch adds support for these new messages.

Signed-off-by: Irui Wang <irui.wang@mediatek.com>
---
 .../vcodec/encoder/mtk_vcodec_enc_drv.h       |  3 ++
 .../mediatek/vcodec/encoder/venc_drv_if.c     |  3 +-
 .../mediatek/vcodec/encoder/venc_ipi_msg.h    | 26 +++++++++++++
 .../mediatek/vcodec/encoder/venc_vpu_if.c     | 37 ++++++++++++-------
 4 files changed, 54 insertions(+), 15 deletions(-)

diff --git a/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.h b/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.h
index b1277bcfcf53..426b1398f815 100644
--- a/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.h
+++ b/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.h
@@ -16,6 +16,7 @@
 
 #define MTK_ENC_CTX_IS_EXT(ctx) ((ctx)->dev->venc_pdata->uses_ext)
 #define MTK_ENC_IOVA_IS_34BIT(ctx) ((ctx)->dev->venc_pdata->uses_34bit)
+#define MTK_ENC_DRV_IS_COMM(ctx) (((ctx)->dev->venc_pdata->uses_common_fw_iface))
 
 /**
  * struct mtk_vcodec_enc_pdata - compatible data for each IC
@@ -29,6 +30,7 @@
  * @num_output_formats: number of entries in output_formats
  * @core_id: stand for h264 or vp8 encode index
  * @uses_34bit: whether the encoder uses 34-bit iova
+ * @uses_common_fw_iface: whether the encoder uses common driver interface
  */
 struct mtk_vcodec_enc_pdata {
 	bool uses_ext;
@@ -40,6 +42,7 @@ struct mtk_vcodec_enc_pdata {
 	size_t num_output_formats;
 	u8 core_id;
 	bool uses_34bit;
+	bool uses_common_fw_iface;
 };
 
 /*
diff --git a/drivers/media/platform/mediatek/vcodec/encoder/venc_drv_if.c b/drivers/media/platform/mediatek/vcodec/encoder/venc_drv_if.c
index e83747b8d69a..f8c9349c18c0 100644
--- a/drivers/media/platform/mediatek/vcodec/encoder/venc_drv_if.c
+++ b/drivers/media/platform/mediatek/vcodec/encoder/venc_drv_if.c
@@ -19,13 +19,14 @@
 int venc_if_init(struct mtk_vcodec_enc_ctx *ctx, unsigned int fourcc)
 {
 	int ret = 0;
+	const bool uses_common_fw_iface = MTK_ENC_DRV_IS_COMM(ctx);
 
 	switch (fourcc) {
 	case V4L2_PIX_FMT_VP8:
 		ctx->enc_if = &venc_vp8_if;
 		break;
 	case V4L2_PIX_FMT_H264:
-		ctx->enc_if = &venc_h264_if;
+		ctx->enc_if = uses_common_fw_iface ? &venc_if : &venc_h264_if;
 		break;
 	default:
 		return -EINVAL;
diff --git a/drivers/media/platform/mediatek/vcodec/encoder/venc_ipi_msg.h b/drivers/media/platform/mediatek/vcodec/encoder/venc_ipi_msg.h
index bb16d96a7f57..ce3c2c8059fb 100644
--- a/drivers/media/platform/mediatek/vcodec/encoder/venc_ipi_msg.h
+++ b/drivers/media/platform/mediatek/vcodec/encoder/venc_ipi_msg.h
@@ -45,6 +45,20 @@ struct venc_ap_ipi_msg_init {
 	uint64_t venc_inst;
 };
 
+/**
+ * struct venc_ap_ipi_msg_init_comm - AP to VPU init cmd structure
+ * @base: AP to VPU init cmd structure
+ * @codec_type: encoder type
+ * @reserved: reserved field
+ * @shared_iova: shared iova
+ */
+struct venc_ap_ipi_msg_init_comm {
+	struct venc_ap_ipi_msg_init base;
+	u32 codec_type;
+	u32 reserved;
+	u64 shared_iova;
+};
+
 /**
  * struct venc_ap_ipi_msg_set_param - AP to VPU set_param cmd structure
  * @msg_id:	message id (AP_IPIMSG_XXX_ENC_SET_PARAM)
@@ -175,6 +189,18 @@ struct venc_vpu_ipi_msg_init {
 	uint32_t venc_abi_version;
 };
 
+/**
+ * struct venc_vpu_ipi_msg_init_comm - VPU ack AP init cmd structure
+ * @init_ack: AP init cmd structure
+ * @vpu_vsi_addr: VSI address from VPU
+ * @reserved: reserved field
+ */
+struct venc_vpu_ipi_msg_init_comm {
+	struct venc_vpu_ipi_msg_init init_ack;
+	u32 vpu_vsi_addr;
+	u32 reserved;
+};
+
 /**
  * struct venc_vpu_ipi_msg_set_param - VPU ack AP set_param cmd structure
  * @msg_id:	message id (VPU_IPIMSG_XXX_ENC_SET_PARAM_DONE)
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 51bb7ee141b9..537b9955932e 100644
--- a/drivers/media/platform/mediatek/vcodec/encoder/venc_vpu_if.c
+++ b/drivers/media/platform/mediatek/vcodec/encoder/venc_vpu_if.c
@@ -10,24 +10,25 @@
 
 static void handle_enc_init_msg(struct venc_vpu_inst *vpu, const void *data)
 {
-	const struct venc_vpu_ipi_msg_init *msg = data;
+	const struct venc_vpu_ipi_msg_init_comm *msg = data;
+	struct mtk_vcodec_fw *fw = vpu->ctx->dev->fw_handler;
 
-	vpu->inst_addr = msg->vpu_inst_addr;
-	vpu->vsi = mtk_vcodec_fw_map_dm_addr(vpu->ctx->dev->fw_handler,
-					     msg->vpu_inst_addr);
+	vpu->inst_addr = msg->init_ack.vpu_inst_addr;
+	vpu->vsi = mtk_vcodec_fw_map_dm_addr(fw, vpu->inst_addr);
 
 	/* Firmware version field value is unspecified on MT8173. */
-	if (mtk_vcodec_fw_get_type(vpu->ctx->dev->fw_handler) == VPU)
+	if (mtk_vcodec_fw_get_type(fw) == VPU)
 		return;
 
 	/* Check firmware version. */
-	mtk_venc_debug(vpu->ctx, "firmware version: 0x%x\n", msg->venc_abi_version);
-	switch (msg->venc_abi_version) {
+	mtk_venc_debug(vpu->ctx, "firmware version: 0x%x\n",
+		       msg->init_ack.venc_abi_version);
+	switch (msg->init_ack.venc_abi_version) {
 	case 1:
 		break;
 	default:
 		mtk_venc_err(vpu->ctx, "unhandled firmware version 0x%x\n",
-			     msg->venc_abi_version);
+			     msg->init_ack.venc_abi_version);
 		vpu->failure = 1;
 		break;
 	}
@@ -132,7 +133,8 @@ static int vpu_enc_send_msg(struct venc_vpu_inst *vpu, void *msg,
 int vpu_enc_init(struct venc_vpu_inst *vpu)
 {
 	int status;
-	struct venc_ap_ipi_msg_init out;
+	size_t msg_size;
+	struct venc_ap_ipi_msg_init_comm out;
 
 	init_waitqueue_head(&vpu->wq_hd);
 	vpu->signaled = 0;
@@ -149,9 +151,16 @@ int vpu_enc_init(struct venc_vpu_inst *vpu)
 	}
 
 	memset(&out, 0, sizeof(out));
-	out.msg_id = AP_IPIMSG_ENC_INIT;
-	out.venc_inst = (unsigned long)vpu;
-	if (vpu_enc_send_msg(vpu, &out, sizeof(out))) {
+	out.base.msg_id = AP_IPIMSG_ENC_INIT;
+	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;
+		msg_size = sizeof(struct venc_ap_ipi_msg_init_comm);
+	} else {
+		msg_size = sizeof(struct venc_ap_ipi_msg_init);
+	}
+
+	if (vpu_enc_send_msg(vpu, &out, msg_size)) {
 		mtk_venc_err(vpu->ctx, "AP_IPIMSG_ENC_INIT fail");
 		return -EINVAL;
 	}
@@ -260,7 +269,7 @@ static int vpu_enc_encode_32bits(struct venc_vpu_inst *vpu,
 		sizeof(struct venc_ap_ipi_msg_enc);
 	struct venc_ap_ipi_msg_enc_ext out;
 
-	mtk_venc_debug(vpu->ctx, "bs_mode %d ->", bs_mode);
+	mtk_venc_debug(vpu->ctx, "%s, bs_mode %d ->", __func__, bs_mode);
 
 	memset(&out, 0, sizeof(out));
 	out.base.msg_id = AP_IPIMSG_ENC_ENCODE;
@@ -305,7 +314,7 @@ static int vpu_enc_encode_34bits(struct venc_vpu_inst *vpu,
 	struct venc_ap_ipi_msg_enc_ext_34 out;
 	size_t msg_size = sizeof(struct venc_ap_ipi_msg_enc_ext_34);
 
-	mtk_venc_debug(vpu->ctx, "bs_mode %d ->", bs_mode);
+	mtk_venc_debug(vpu->ctx, "%s, bs_mode %d ->", __func__, bs_mode);
 
 	memset(&out, 0, sizeof(out));
 	out.msg_id = AP_IPIMSG_ENC_ENCODE;
-- 
2.46.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 3/6] media: mediatek: encoder: Add support for VCP encode process
  2025-08-14  8:56 [PATCH v3 0/6] Add support for MT8196 video encoder Kyrie Wu
  2025-08-14  8:56 ` [PATCH v3 1/6] media: mediatek: encoder: Add a new encoder driver interface Kyrie Wu
  2025-08-14  8:56 ` [PATCH v3 2/6] media: mediatek: encoder: Add support for common firmware interface Kyrie Wu
@ 2025-08-14  8:56 ` Kyrie Wu
  2025-08-14  8:56 ` [PATCH v3 4/6] media: mediatek: encoder: Add a new platform data member Kyrie Wu
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Kyrie Wu @ 2025-08-14  8:56 UTC (permalink / raw)
  To: Tiffany Lin, Andrew-CT Chen, Yunfei Dong, Mauro Carvalho Chehab,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
	AngeloGioacchino Del Regno, Kyrie Wu, Hans Verkuil,
	Nicolas Dufresne, Christophe JAILLET, Sebastian Fricke,
	Nathan Hebert, Arnd Bergmann, Irui Wang, George Sun, linux-media,
	devicetree, linux-kernel, linux-arm-kernel, linux-mediatek
  Cc: Neil Armstrong, Andrzej Pietrasiewicz

From: Irui Wang <irui.wang@mediatek.com>

When encoding by VCP interface, encoder driver need change to VCP path:
Firstly, set encoder driver fw type to 'VCP'. Then, allocate RC buffers
by the VCP device. Finally, send the shared memory address into VCP and
map the encoder vsi address by 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 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);
-- 
2.46.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 4/6] media: mediatek: encoder: Add a new platform data member
  2025-08-14  8:56 [PATCH v3 0/6] Add support for MT8196 video encoder Kyrie Wu
                   ` (2 preceding siblings ...)
  2025-08-14  8:56 ` [PATCH v3 3/6] media: mediatek: encoder: Add support for VCP encode process Kyrie Wu
@ 2025-08-14  8:56 ` Kyrie Wu
  2025-08-14  8:56 ` [PATCH v3 5/6] dt-bindings: media: Add MT8196 mediatek,vcodec-encoder Kyrie Wu
  2025-08-14  8:56 ` [PATCH v3 6/6] media: mediatek: encoder: Add MT8196 encoder compatible data Kyrie Wu
  5 siblings, 0 replies; 11+ messages in thread
From: Kyrie Wu @ 2025-08-14  8:56 UTC (permalink / raw)
  To: Tiffany Lin, Andrew-CT Chen, Yunfei Dong, Mauro Carvalho Chehab,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
	AngeloGioacchino Del Regno, Kyrie Wu, Hans Verkuil,
	Nicolas Dufresne, Christophe JAILLET, Sebastian Fricke,
	Nathan Hebert, Arnd Bergmann, Irui Wang, George Sun, linux-media,
	devicetree, linux-kernel, linux-arm-kernel, linux-mediatek
  Cc: Neil Armstrong, Andrzej Pietrasiewicz

From: Irui Wang <irui.wang@mediatek.com>

Add a new platform data member to indicate each encoder IC, so that the
get chip name function by 'of_device_is_compatible' can be removed.

Signed-off-by: Irui Wang <irui.wang@mediatek.com>
---
 .../mediatek/vcodec/encoder/mtk_vcodec_enc.c  | 23 ++-----------------
 .../vcodec/encoder/mtk_vcodec_enc_drv.c       |  6 +++++
 .../vcodec/encoder/mtk_vcodec_enc_drv.h       |  2 ++
 3 files changed, 10 insertions(+), 21 deletions(-)

diff --git a/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc.c b/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc.c
index 03512bf7a072..1a7be3e57eef 100644
--- a/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc.c
+++ b/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc.c
@@ -198,34 +198,15 @@ static int vidioc_enum_fmt_vid_out(struct file *file, void *priv,
 			       pdata->num_output_formats);
 }
 
-static int mtk_vcodec_enc_get_chip_name(void *priv)
-{
-	struct mtk_vcodec_enc_ctx *ctx = fh_to_enc_ctx(priv);
-	struct device *dev = &ctx->dev->plat_dev->dev;
-
-	if (of_device_is_compatible(dev->of_node, "mediatek,mt8173-vcodec-enc"))
-		return 8173;
-	else if (of_device_is_compatible(dev->of_node, "mediatek,mt8183-vcodec-enc"))
-		return 8183;
-	else if (of_device_is_compatible(dev->of_node, "mediatek,mt8192-vcodec-enc"))
-		return 8192;
-	else if (of_device_is_compatible(dev->of_node, "mediatek,mt8195-vcodec-enc"))
-		return 8195;
-	else if (of_device_is_compatible(dev->of_node, "mediatek,mt8188-vcodec-enc"))
-		return 8188;
-	else
-		return 8173;
-}
-
 static int vidioc_venc_querycap(struct file *file, void *priv,
 				struct v4l2_capability *cap)
 {
 	struct mtk_vcodec_enc_ctx *ctx = fh_to_enc_ctx(priv);
+	const struct mtk_vcodec_enc_pdata *pdata = ctx->dev->venc_pdata;
 	struct device *dev = &ctx->dev->plat_dev->dev;
-	int platform_name = mtk_vcodec_enc_get_chip_name(priv);
 
 	strscpy(cap->driver, dev->driver->name, sizeof(cap->driver));
-	snprintf(cap->card, sizeof(cap->card), "MT%d video encoder", platform_name);
+	snprintf(cap->card, sizeof(cap->card), "MT%d video encoder", pdata->venc_model_num);
 
 	return 0;
 }
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 bb913dfe5f04..50936949d527 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
@@ -380,6 +380,7 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
 }
 
 static const struct mtk_vcodec_enc_pdata mt8173_avc_pdata = {
+	.venc_model_num = 8173,
 	.capture_formats = mtk_video_formats_capture_h264,
 	.num_capture_formats = ARRAY_SIZE(mtk_video_formats_capture_h264),
 	.output_formats = mtk_video_formats_output,
@@ -390,6 +391,7 @@ static const struct mtk_vcodec_enc_pdata mt8173_avc_pdata = {
 };
 
 static const struct mtk_vcodec_enc_pdata mt8173_vp8_pdata = {
+	.venc_model_num = 8173,
 	.capture_formats = mtk_video_formats_capture_vp8,
 	.num_capture_formats = ARRAY_SIZE(mtk_video_formats_capture_vp8),
 	.output_formats = mtk_video_formats_output,
@@ -400,6 +402,7 @@ static const struct mtk_vcodec_enc_pdata mt8173_vp8_pdata = {
 };
 
 static const struct mtk_vcodec_enc_pdata mt8183_pdata = {
+	.venc_model_num = 8183,
 	.uses_ext = true,
 	.capture_formats = mtk_video_formats_capture_h264,
 	.num_capture_formats = ARRAY_SIZE(mtk_video_formats_capture_h264),
@@ -411,6 +414,7 @@ static const struct mtk_vcodec_enc_pdata mt8183_pdata = {
 };
 
 static const struct mtk_vcodec_enc_pdata mt8188_pdata = {
+	.venc_model_num = 8188,
 	.uses_ext = true,
 	.capture_formats = mtk_video_formats_capture_h264,
 	.num_capture_formats = ARRAY_SIZE(mtk_video_formats_capture_h264),
@@ -423,6 +427,7 @@ static const struct mtk_vcodec_enc_pdata mt8188_pdata = {
 };
 
 static const struct mtk_vcodec_enc_pdata mt8192_pdata = {
+	.venc_model_num = 8192,
 	.uses_ext = true,
 	.capture_formats = mtk_video_formats_capture_h264,
 	.num_capture_formats = ARRAY_SIZE(mtk_video_formats_capture_h264),
@@ -434,6 +439,7 @@ static const struct mtk_vcodec_enc_pdata mt8192_pdata = {
 };
 
 static const struct mtk_vcodec_enc_pdata mt8195_pdata = {
+	.venc_model_num = 8195,
 	.uses_ext = true,
 	.capture_formats = mtk_video_formats_capture_h264,
 	.num_capture_formats = ARRAY_SIZE(mtk_video_formats_capture_h264),
diff --git a/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.h b/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.h
index 426b1398f815..b3206a7b592d 100644
--- a/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.h
+++ b/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.h
@@ -21,6 +21,7 @@
 /**
  * struct mtk_vcodec_enc_pdata - compatible data for each IC
  *
+ * @venc_model_num: encoder model number
  * @uses_ext: whether the encoder uses the extended firmware messaging format
  * @min_bitrate: minimum supported encoding bitrate
  * @max_bitrate: maximum supported encoding bitrate
@@ -33,6 +34,7 @@
  * @uses_common_fw_iface: whether the encoder uses common driver interface
  */
 struct mtk_vcodec_enc_pdata {
+	u16 venc_model_num;
 	bool uses_ext;
 	u64 min_bitrate;
 	u64 max_bitrate;
-- 
2.46.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 5/6] dt-bindings: media: Add MT8196 mediatek,vcodec-encoder
  2025-08-14  8:56 [PATCH v3 0/6] Add support for MT8196 video encoder Kyrie Wu
                   ` (3 preceding siblings ...)
  2025-08-14  8:56 ` [PATCH v3 4/6] media: mediatek: encoder: Add a new platform data member Kyrie Wu
@ 2025-08-14  8:56 ` Kyrie Wu
  2025-08-15  8:54   ` Krzysztof Kozlowski
  2025-08-14  8:56 ` [PATCH v3 6/6] media: mediatek: encoder: Add MT8196 encoder compatible data Kyrie Wu
  5 siblings, 1 reply; 11+ messages in thread
From: Kyrie Wu @ 2025-08-14  8:56 UTC (permalink / raw)
  To: Tiffany Lin, Andrew-CT Chen, Yunfei Dong, Mauro Carvalho Chehab,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
	AngeloGioacchino Del Regno, Kyrie Wu, Hans Verkuil,
	Nicolas Dufresne, Christophe JAILLET, Sebastian Fricke,
	Nathan Hebert, Arnd Bergmann, Irui Wang, George Sun, linux-media,
	devicetree, linux-kernel, linux-arm-kernel, linux-mediatek
  Cc: Neil Armstrong, Andrzej Pietrasiewicz

From: Irui Wang <irui.wang@mediatek.com>

Add MT8196 encoder compatible string, which will reference VCP device.

Signed-off-by: Irui Wang <irui.wang@mediatek.com>
---
 .../bindings/media/mediatek,vcodec-encoder.yaml | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/mediatek,vcodec-encoder.yaml b/Documentation/devicetree/bindings/media/mediatek,vcodec-encoder.yaml
index ebc615584f92..bb4dbf23ccc5 100644
--- a/Documentation/devicetree/bindings/media/mediatek,vcodec-encoder.yaml
+++ b/Documentation/devicetree/bindings/media/mediatek,vcodec-encoder.yaml
@@ -24,6 +24,7 @@ properties:
               - mediatek,mt8188-vcodec-enc
               - mediatek,mt8192-vcodec-enc
               - mediatek,mt8195-vcodec-enc
+              - mediatek,mt8196-vcodec-enc
       - items:
           - const: mediatek,mt8186-vcodec-enc
           - const: mediatek,mt8183-vcodec-enc
@@ -58,6 +59,11 @@ properties:
     description:
       Describes point to scp.
 
+  mediatek,vcp:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      Describes point to vcp.
+
   power-domains:
     maxItems: 1
 
@@ -76,6 +82,17 @@ required:
   - iommus
 
 allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - mediatek,mt8196-vcodec-enc
+
+    then:
+      required:
+        - mediatek,vcp
+
   - if:
       properties:
         compatible:
-- 
2.46.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 6/6] media: mediatek: encoder: Add MT8196 encoder compatible data
  2025-08-14  8:56 [PATCH v3 0/6] Add support for MT8196 video encoder Kyrie Wu
                   ` (4 preceding siblings ...)
  2025-08-14  8:56 ` [PATCH v3 5/6] dt-bindings: media: Add MT8196 mediatek,vcodec-encoder Kyrie Wu
@ 2025-08-14  8:56 ` Kyrie Wu
  5 siblings, 0 replies; 11+ messages in thread
From: Kyrie Wu @ 2025-08-14  8:56 UTC (permalink / raw)
  To: Tiffany Lin, Andrew-CT Chen, Yunfei Dong, Mauro Carvalho Chehab,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
	AngeloGioacchino Del Regno, Kyrie Wu, Hans Verkuil,
	Nicolas Dufresne, Christophe JAILLET, Sebastian Fricke,
	Nathan Hebert, Arnd Bergmann, Irui Wang, George Sun, linux-media,
	devicetree, linux-kernel, linux-arm-kernel, linux-mediatek
  Cc: Neil Armstrong, Andrzej Pietrasiewicz

From: Irui Wang <irui.wang@mediatek.com>

MT8196 encoder use common firmware interface, add compatible data to
support MT8196 encoding, and need set dma mask to support 34bit.

Signed-off-by: Irui Wang <irui.wang@mediatek.com>
---
 .../vcodec/encoder/mtk_vcodec_enc_drv.c       | 19 +++++++++++++++++++
 .../vcodec/encoder/mtk_vcodec_enc_drv.h       |  2 ++
 2 files changed, 21 insertions(+)

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 50936949d527..c869c4245ebc 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
@@ -20,6 +20,8 @@
 #include "mtk_vcodec_enc_pm.h"
 #include "../common/mtk_vcodec_intr.h"
 
+#define VENC_DMA_BIT_MASK 34
+
 static const struct mtk_video_fmt mtk_video_formats_output[] = {
 	{
 		.fourcc = V4L2_PIX_FMT_NV12M,
@@ -299,6 +301,9 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
 		goto err_res;
 	}
 
+	if (dev->venc_pdata->set_dma_bit_mask)
+		dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(VENC_DMA_BIT_MASK));
+
 	mutex_init(&dev->enc_mutex);
 	mutex_init(&dev->dev_mutex);
 	mutex_init(&dev->dev_ctx_lock);
@@ -450,6 +455,19 @@ static const struct mtk_vcodec_enc_pdata mt8195_pdata = {
 	.core_id = VENC_SYS,
 };
 
+static const struct mtk_vcodec_enc_pdata mt8196_pdata = {
+	.venc_model_num = 8196,
+	.capture_formats = mtk_video_formats_capture_h264,
+	.num_capture_formats = ARRAY_SIZE(mtk_video_formats_capture_h264),
+	.output_formats = mtk_video_formats_output,
+	.num_output_formats = ARRAY_SIZE(mtk_video_formats_output),
+	.min_bitrate = 64,
+	.max_bitrate = 100000000,
+	.core_id = VENC_SYS,
+	.uses_common_fw_iface = true,
+	.set_dma_bit_mask = true,
+};
+
 static const struct of_device_id mtk_vcodec_enc_match[] = {
 	{.compatible = "mediatek,mt8173-vcodec-enc",
 			.data = &mt8173_avc_pdata},
@@ -459,6 +477,7 @@ static const struct of_device_id mtk_vcodec_enc_match[] = {
 	{.compatible = "mediatek,mt8188-vcodec-enc", .data = &mt8188_pdata},
 	{.compatible = "mediatek,mt8192-vcodec-enc", .data = &mt8192_pdata},
 	{.compatible = "mediatek,mt8195-vcodec-enc", .data = &mt8195_pdata},
+	{.compatible = "mediatek,mt8196-vcodec-enc", .data = &mt8196_pdata},
 	{},
 };
 MODULE_DEVICE_TABLE(of, mtk_vcodec_enc_match);
diff --git a/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.h b/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.h
index b3206a7b592d..ded794f1b37a 100644
--- a/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.h
+++ b/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.h
@@ -32,6 +32,7 @@
  * @core_id: stand for h264 or vp8 encode index
  * @uses_34bit: whether the encoder uses 34-bit iova
  * @uses_common_fw_iface: whether the encoder uses common driver interface
+ * @set_dma_bit_mask: whether the encoder need set extra DMA bit mask
  */
 struct mtk_vcodec_enc_pdata {
 	u16 venc_model_num;
@@ -45,6 +46,7 @@ struct mtk_vcodec_enc_pdata {
 	u8 core_id;
 	bool uses_34bit;
 	bool uses_common_fw_iface;
+	bool set_dma_bit_mask;
 };
 
 /*
-- 
2.46.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 5/6] dt-bindings: media: Add MT8196 mediatek,vcodec-encoder
  2025-08-14  8:56 ` [PATCH v3 5/6] dt-bindings: media: Add MT8196 mediatek,vcodec-encoder Kyrie Wu
@ 2025-08-15  8:54   ` Krzysztof Kozlowski
  2025-08-20  2:55     ` Irui Wang (王瑞)
  0 siblings, 1 reply; 11+ messages in thread
From: Krzysztof Kozlowski @ 2025-08-15  8:54 UTC (permalink / raw)
  To: Kyrie Wu
  Cc: Tiffany Lin, Andrew-CT Chen, Yunfei Dong, Mauro Carvalho Chehab,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
	AngeloGioacchino Del Regno, Hans Verkuil, Nicolas Dufresne,
	Christophe JAILLET, Sebastian Fricke, Nathan Hebert,
	Arnd Bergmann, Irui Wang, George Sun, linux-media, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek, Neil Armstrong,
	Andrzej Pietrasiewicz

On Thu, Aug 14, 2025 at 04:56:41PM +0800, Kyrie Wu wrote:
> From: Irui Wang <irui.wang@mediatek.com>
> 
> Add MT8196 encoder compatible string, which will reference VCP device.

You ignored comments from v2.

> 
> Signed-off-by: Irui Wang <irui.wang@mediatek.com>

Incorrect SoB chain.

> ---
>  .../bindings/media/mediatek,vcodec-encoder.yaml | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/mediatek,vcodec-encoder.yaml b/Documentation/devicetree/bindings/media/mediatek,vcodec-encoder.yaml
> index ebc615584f92..bb4dbf23ccc5 100644
> --- a/Documentation/devicetree/bindings/media/mediatek,vcodec-encoder.yaml
> +++ b/Documentation/devicetree/bindings/media/mediatek,vcodec-encoder.yaml
> @@ -24,6 +24,7 @@ properties:
>                - mediatek,mt8188-vcodec-enc
>                - mediatek,mt8192-vcodec-enc
>                - mediatek,mt8195-vcodec-enc
> +              - mediatek,mt8196-vcodec-enc
>        - items:
>            - const: mediatek,mt8186-vcodec-enc
>            - const: mediatek,mt8183-vcodec-enc
> @@ -58,6 +59,11 @@ properties:
>      description:
>        Describes point to scp.
>  
> +  mediatek,vcp:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description:
> +      Describes point to vcp.

For what? You just repeated the property name. You must say here
something useful instead: why this is needed, what is its purpose.


> +
>    power-domains:
>      maxItems: 1
>  
> @@ -76,6 +82,17 @@ required:
>    - iommus
>  
>  allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - mediatek,mt8196-vcodec-enc
> +
> +    then:
> +      required:
> +        - mediatek,vcp

else

mediatek,vcp: false


Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 5/6] dt-bindings: media: Add MT8196 mediatek,vcodec-encoder
  2025-08-15  8:54   ` Krzysztof Kozlowski
@ 2025-08-20  2:55     ` Irui Wang (王瑞)
  2025-08-20  5:55       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 11+ messages in thread
From: Irui Wang (王瑞) @ 2025-08-20  2:55 UTC (permalink / raw)
  To: Kyrie Wu (吴晗), krzk@kernel.org
  Cc: linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
	George Sun (孙林),
	Tiffany Lin (林慧珊), andrzejtp2010@gmail.com,
	nhebert@chromium.org, linux-media@vger.kernel.org,
	devicetree@vger.kernel.org, mchehab@kernel.org,
	hverkuil@xs4all.nl, nicolas.dufresne@collabora.com,
	Yunfei Dong (董云飞), conor+dt@kernel.org,
	robh@kernel.org, sebastian.fricke@collabora.com,
	linux-arm-kernel@lists.infradead.org, matthias.bgg@gmail.com,
	christophe.jaillet@wanadoo.fr, krzk+dt@kernel.org, arnd@arndb.de,
	Andrew-CT Chen (陳智迪),
	AngeloGioacchino Del Regno, neil.armstrong@linaro.org

Dear Krzysztof,

Thanks for your reviewing.

On Fri, 2025-08-15 at 10:54 +0200, Krzysztof Kozlowski wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> On Thu, Aug 14, 2025 at 04:56:41PM +0800, Kyrie Wu wrote:
> > From: Irui Wang <irui.wang@mediatek.com>
> > 
> > Add MT8196 encoder compatible string, which will reference VCP
> > device.
> 
> You ignored comments from v2.

I think I misunderstood the v2 comments, I rewrote the title because it
said dt-bindings and encoder twice, this is not enough, right? we need
to describe more information in Body text?

> 
> > 
> > Signed-off-by: Irui Wang <irui.wang@mediatek.com>
> 
> Incorrect SoB chain.
> 
> > ---
> >  .../bindings/media/mediatek,vcodec-encoder.yaml | 17
> > +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/media/mediatek,vcodec-
> > encoder.yaml
> > b/Documentation/devicetree/bindings/media/mediatek,vcodec-
> > encoder.yaml
> > index ebc615584f92..bb4dbf23ccc5 100644
> > --- a/Documentation/devicetree/bindings/media/mediatek,vcodec-
> > encoder.yaml
> > +++ b/Documentation/devicetree/bindings/media/mediatek,vcodec-
> > encoder.yaml
> > @@ -24,6 +24,7 @@ properties:
> >                - mediatek,mt8188-vcodec-enc
> >                - mediatek,mt8192-vcodec-enc
> >                - mediatek,mt8195-vcodec-enc
> > +              - mediatek,mt8196-vcodec-enc
> >        - items:
> >            - const: mediatek,mt8186-vcodec-enc
> >            - const: mediatek,mt8183-vcodec-enc
> > @@ -58,6 +59,11 @@ properties:
> >      description:
> >        Describes point to scp.
> > 
> > +  mediatek,vcp:
> > +    $ref: /schemas/types.yaml#/definitions/phandle
> > +    description:
> > +      Describes point to vcp.
> 
> For what? You just repeated the property name. You must say here
> something useful instead: why this is needed, what is its purpose.

I would like to say:
"define this 'mediatek,vcp' property here, this is a phandle point to
vcp device, for platforms using vcp firmware."

Is this OK for you?
> 
> 

> > +
> >    power-domains:
> >      maxItems: 1
> > 
> > @@ -76,6 +82,17 @@ required:
> >    - iommus
> > 
> >  allOf:
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - mediatek,mt8196-vcodec-enc
> > +
> > +    then:
> > +      required:
> > +        - mediatek,vcp
> 
> else
> 
> mediatek,vcp: false

I think the else statement is no need here. Different platforms are
using different firmware phandle, vpu, scp, and vcp. so we use if-then
to describe the required property for these platforms. 

> 
> 
> Best regards,
> Krzysztof
> 

Thanks again for your kindly reviewing.

Best Regards

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 5/6] dt-bindings: media: Add MT8196 mediatek,vcodec-encoder
  2025-08-20  2:55     ` Irui Wang (王瑞)
@ 2025-08-20  5:55       ` Krzysztof Kozlowski
  2025-08-20  8:24         ` Irui Wang (王瑞)
  0 siblings, 1 reply; 11+ messages in thread
From: Krzysztof Kozlowski @ 2025-08-20  5:55 UTC (permalink / raw)
  To: Irui Wang (王瑞), Kyrie Wu (吴晗)
  Cc: linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
	George Sun (孙林),
	Tiffany Lin (林慧珊), andrzejtp2010@gmail.com,
	nhebert@chromium.org, linux-media@vger.kernel.org,
	devicetree@vger.kernel.org, mchehab@kernel.org,
	hverkuil@xs4all.nl, nicolas.dufresne@collabora.com,
	Yunfei Dong (董云飞), conor+dt@kernel.org,
	robh@kernel.org, sebastian.fricke@collabora.com,
	linux-arm-kernel@lists.infradead.org, matthias.bgg@gmail.com,
	christophe.jaillet@wanadoo.fr, krzk+dt@kernel.org, arnd@arndb.de,
	Andrew-CT Chen (陳智迪),
	AngeloGioacchino Del Regno, neil.armstrong@linaro.org

On 20/08/2025 04:55, Irui Wang (王瑞) wrote:
> Dear Krzysztof,
> 
> Thanks for your reviewing.
> 
> On Fri, 2025-08-15 at 10:54 +0200, Krzysztof Kozlowski wrote:
>> External email : Please do not click links or open attachments until
>> you have verified the sender or the content.
>>
>>
>> On Thu, Aug 14, 2025 at 04:56:41PM +0800, Kyrie Wu wrote:
>>> From: Irui Wang <irui.wang@mediatek.com>
>>>
>>> Add MT8196 encoder compatible string, which will reference VCP
>>> device.
>>
>> You ignored comments from v2.
> 
> I think I misunderstood the v2 comments, I rewrote the title because it
> said dt-bindings and encoder twice, this is not enough, right? we need
> to describe more information in Body text?

where are lore links in the changelog? Why aren't you using b4 to submit
patches?

> 
>>
>>>
>>> Signed-off-by: Irui Wang <irui.wang@mediatek.com>
>>
>> Incorrect SoB chain.
>>
>>> ---
>>>  .../bindings/media/mediatek,vcodec-encoder.yaml | 17
>>> +++++++++++++++++
>>>  1 file changed, 17 insertions(+)
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/media/mediatek,vcodec-
>>> encoder.yaml
>>> b/Documentation/devicetree/bindings/media/mediatek,vcodec-
>>> encoder.yaml
>>> index ebc615584f92..bb4dbf23ccc5 100644
>>> --- a/Documentation/devicetree/bindings/media/mediatek,vcodec-
>>> encoder.yaml
>>> +++ b/Documentation/devicetree/bindings/media/mediatek,vcodec-
>>> encoder.yaml
>>> @@ -24,6 +24,7 @@ properties:
>>>                - mediatek,mt8188-vcodec-enc
>>>                - mediatek,mt8192-vcodec-enc
>>>                - mediatek,mt8195-vcodec-enc
>>> +              - mediatek,mt8196-vcodec-enc
>>>        - items:
>>>            - const: mediatek,mt8186-vcodec-enc
>>>            - const: mediatek,mt8183-vcodec-enc
>>> @@ -58,6 +59,11 @@ properties:
>>>      description:
>>>        Describes point to scp.
>>>
>>> +  mediatek,vcp:
>>> +    $ref: /schemas/types.yaml#/definitions/phandle
>>> +    description:
>>> +      Describes point to vcp.
>>
>> For what? You just repeated the property name. You must say here
>> something useful instead: why this is needed, what is its purpose.
> 
> I would like to say:
> "define this 'mediatek,vcp' property here, this is a phandle point to

Don't explain to us what DT is. Above is 100% redundant.

> vcp device, for platforms using vcp firmware."

Explain what is the purpose of this in hardware, how hardware uses it,
what problem for hardware this addresses.


> 
> Is this OK for you?
>>
>>
> 
>>> +
>>>    power-domains:
>>>      maxItems: 1
>>>
>>> @@ -76,6 +82,17 @@ required:
>>>    - iommus
>>>
>>>  allOf:
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            enum:
>>> +              - mediatek,mt8196-vcodec-enc
>>> +
>>> +    then:
>>> +      required:
>>> +        - mediatek,vcp
>>
>> else
>>
>> mediatek,vcp: false
> 
> I think the else statement is no need here. Different platforms are
> using different firmware phandle, vpu, scp, and vcp. so we use if-then
> to describe the required property for these platforms. 

Hm? I just told you it is needed. Otherwise, explain why each
variant/device has now VCP.

You have entire commit msg to explain all this.


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 5/6] dt-bindings: media: Add MT8196 mediatek,vcodec-encoder
  2025-08-20  5:55       ` Krzysztof Kozlowski
@ 2025-08-20  8:24         ` Irui Wang (王瑞)
  0 siblings, 0 replies; 11+ messages in thread
From: Irui Wang (王瑞) @ 2025-08-20  8:24 UTC (permalink / raw)
  To: Kyrie Wu (吴晗), krzk@kernel.org
  Cc: linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
	George Sun (孙林),
	Tiffany Lin (林慧珊), andrzejtp2010@gmail.com,
	linux-media@vger.kernel.org, nhebert@chromium.org,
	devicetree@vger.kernel.org, mchehab@kernel.org,
	hverkuil@xs4all.nl, nicolas.dufresne@collabora.com,
	Yunfei Dong (董云飞), conor+dt@kernel.org,
	robh@kernel.org, sebastian.fricke@collabora.com,
	linux-arm-kernel@lists.infradead.org, matthias.bgg@gmail.com,
	christophe.jaillet@wanadoo.fr, krzk+dt@kernel.org, arnd@arndb.de,
	Andrew-CT Chen (陳智迪),
	AngeloGioacchino Del Regno, neil.armstrong@linaro.org

Dear Krzysztof,

Thanks for your reviewing.

On Wed, 2025-08-20 at 07:55 +0200, Krzysztof Kozlowski wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> On 20/08/2025 04:55, Irui Wang (王瑞) wrote:
> > Dear Krzysztof,
> > 
> > Thanks for your reviewing.
> > 
> > On Fri, 2025-08-15 at 10:54 +0200, Krzysztof Kozlowski wrote:
> > > External email : Please do not click links or open attachments
> > > until
> > > you have verified the sender or the content.
> > > 
> > > 
> > > On Thu, Aug 14, 2025 at 04:56:41PM +0800, Kyrie Wu wrote:
> > > > From: Irui Wang <irui.wang@mediatek.com>
> > > > 
> > > > Add MT8196 encoder compatible string, which will reference VCP
> > > > device.
> > > 
> > > You ignored comments from v2.
> > 
> > I think I misunderstood the v2 comments, I rewrote the title
> > because it
> > said dt-bindings and encoder twice, this is not enough, right? we
> > need
> > to describe more information in Body text?
> 
> where are lore links in the changelog?

We need put the lore links in the change log, I mean put the previous
version(v2) patchset lore link? We should to indicate the changes in
each version in the change log and we did the same. If each version
lore links or URL is needed, we will start add them in next patch. 

>  Why aren't you using b4 to submit
> patches?

We have been using 'git send-email' to submit patches to community, b4
is not deployed ready on my server yet, we can study it first.

> > 
> > > 
> > > > 
> > > > Signed-off-by: Irui Wang <irui.wang@mediatek.com>
> > > 
> > > Incorrect SoB chain.
> > > 
> > > > ---
> > > >  .../bindings/media/mediatek,vcodec-encoder.yaml | 17
> > > > +++++++++++++++++
> > > >  1 file changed, 17 insertions(+)
> > > > 
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/media/mediatek,vcodec-
> > > > encoder.yaml
> > > > b/Documentation/devicetree/bindings/media/mediatek,vcodec-
> > > > encoder.yaml
> > > > index ebc615584f92..bb4dbf23ccc5 100644
> > > > --- a/Documentation/devicetree/bindings/media/mediatek,vcodec-
> > > > encoder.yaml
> > > > +++ b/Documentation/devicetree/bindings/media/mediatek,vcodec-
> > > > encoder.yaml
> > > > @@ -24,6 +24,7 @@ properties:
> > > >                - mediatek,mt8188-vcodec-enc
> > > >                - mediatek,mt8192-vcodec-enc
> > > >                - mediatek,mt8195-vcodec-enc
> > > > +              - mediatek,mt8196-vcodec-enc
> > > >        - items:
> > > >            - const: mediatek,mt8186-vcodec-enc
> > > >            - const: mediatek,mt8183-vcodec-enc
> > > > @@ -58,6 +59,11 @@ properties:
> > > >      description:
> > > >        Describes point to scp.
> > > > 
> > > > +  mediatek,vcp:
> > > > +    $ref: /schemas/types.yaml#/definitions/phandle
> > > > +    description:
> > > > +      Describes point to vcp.
> > > 
> > > For what? You just repeated the property name. You must say here
> > > something useful instead: why this is needed, what is its
> > > purpose.
> > 
> > I would like to say:
> > "define this 'mediatek,vcp' property here, this is a phandle point
> > to
> 
> Don't explain to us what DT is. Above is 100% redundant.
> 
> > vcp device, for platforms using vcp firmware."
> 
> Explain what is the purpose of this in hardware, how hardware uses
> it,
> what problem for hardware this addresses.

Maybe I would like to say:

phandle to the VCP node, the video encoder kernel driver uses this
phandle to coordinate with the encoder software driver which running in
VCP firmware for command submission and interrupt handling during
encoding process.

> 
> 
> > 
> > Is this OK for you?
> > > 
> > > 
> > > > +
> > > >    power-domains:
> > > >      maxItems: 1
> > > > 
> > > > @@ -76,6 +82,17 @@ required:
> > > >    - iommus
> > > > 
> > > >  allOf:
> > > > +  - if:
> > > > +      properties:
> > > > +        compatible:
> > > > +          contains:
> > > > +            enum:
> > > > +              - mediatek,mt8196-vcodec-enc
> > > > +
> > > > +    then:
> > > > +      required:
> > > > +        - mediatek,vcp
> > > 
> > > else
> > > 
> > > mediatek,vcp: false
> > 
> > I think the else statement is no need here. Different platforms are
> > using different firmware phandle, vpu, scp, and vcp. so we use if-
> > then
> > to describe the required property for these platforms.
> 
> Hm? I just told you it is needed. Otherwise, explain why each
> variant/device has now VCP.

Currently, just mt8196 use VCP devices, so 'mediatek,vcp' property is
required on mt8196 platform, sorry I didn't get your opinion why 'each variant/device has now VCP.' 

> 
> You have entire commit msg to explain all this.
> 
> 
> Best regards,
> Krzysztof

Thanks again for your kindly reviewing.

Best regards

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2025-08-20  8:24 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-14  8:56 [PATCH v3 0/6] Add support for MT8196 video encoder Kyrie Wu
2025-08-14  8:56 ` [PATCH v3 1/6] media: mediatek: encoder: Add a new encoder driver interface Kyrie Wu
2025-08-14  8:56 ` [PATCH v3 2/6] media: mediatek: encoder: Add support for common firmware interface Kyrie Wu
2025-08-14  8:56 ` [PATCH v3 3/6] media: mediatek: encoder: Add support for VCP encode process Kyrie Wu
2025-08-14  8:56 ` [PATCH v3 4/6] media: mediatek: encoder: Add a new platform data member Kyrie Wu
2025-08-14  8:56 ` [PATCH v3 5/6] dt-bindings: media: Add MT8196 mediatek,vcodec-encoder Kyrie Wu
2025-08-15  8:54   ` Krzysztof Kozlowski
2025-08-20  2:55     ` Irui Wang (王瑞)
2025-08-20  5:55       ` Krzysztof Kozlowski
2025-08-20  8:24         ` Irui Wang (王瑞)
2025-08-14  8:56 ` [PATCH v3 6/6] media: mediatek: encoder: Add MT8196 encoder compatible data Kyrie Wu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).