[PATCH 1/7] media: mediatek: vcodec: Getting the chip name of each platform

[PATCH 1/7] media: mediatek: vcodec: Getting the chip name of each platform

[Yunfei Dong]

Getting the chip name of each platform according to the device
compatible to set different parameter.

Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
---
 .../mediatek/vcodec/decoder/mtk_vcodec_dec.c  | 24 +----------------
 .../vcodec/decoder/mtk_vcodec_dec_drv.c       | 26 +++++++++++++++++++
 .../vcodec/decoder/mtk_vcodec_dec_drv.h       | 17 ++++++++++++
 3 files changed, 44 insertions(+), 23 deletions(-)

diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
index 91ed576d6821..ba742f0e391d 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
@@ -208,36 +208,14 @@ static int vidioc_vdec_dqbuf(struct file *file, void *priv,
 	return v4l2_m2m_dqbuf(file, ctx->m2m_ctx, buf);
 }
 
-static int mtk_vcodec_dec_get_chip_name(void *priv)
-{
-	struct mtk_vcodec_dec_ctx *ctx = fh_to_dec_ctx(priv);
-	struct device *dev = &ctx->dev->plat_dev->dev;
-
-	if (of_device_is_compatible(dev->of_node, "mediatek,mt8173-vcodec-dec"))
-		return 8173;
-	else if (of_device_is_compatible(dev->of_node, "mediatek,mt8183-vcodec-dec"))
-		return 8183;
-	else if (of_device_is_compatible(dev->of_node, "mediatek,mt8192-vcodec-dec"))
-		return 8192;
-	else if (of_device_is_compatible(dev->of_node, "mediatek,mt8195-vcodec-dec"))
-		return 8195;
-	else if (of_device_is_compatible(dev->of_node, "mediatek,mt8186-vcodec-dec"))
-		return 8186;
-	else if (of_device_is_compatible(dev->of_node, "mediatek,mt8188-vcodec-dec"))
-		return 8188;
-	else
-		return 8173;
-}
-
 static int vidioc_vdec_querycap(struct file *file, void *priv,
 				struct v4l2_capability *cap)
 {
 	struct mtk_vcodec_dec_ctx *ctx = fh_to_dec_ctx(priv);
 	struct device *dev = &ctx->dev->plat_dev->dev;
-	int platform_name = mtk_vcodec_dec_get_chip_name(priv);
 
 	strscpy(cap->driver, dev->driver->name, sizeof(cap->driver));
-	snprintf(cap->card, sizeof(cap->card), "MT%d video decoder", platform_name);
+	snprintf(cap->card, sizeof(cap->card), "MT%d video decoder", ctx->dev->chip_name);
 
 	return 0;
 }
diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c
index 0a89ce452ac3..f47c98faf068 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c
@@ -326,6 +326,26 @@ static const struct v4l2_file_operations mtk_vcodec_fops = {
 	.mmap		= v4l2_m2m_fop_mmap,
 };
 
+static void mtk_vcodec_dec_get_chip_name(struct mtk_vcodec_dec_dev *vdec_dev)
+{
+	struct device *dev = &vdec_dev->plat_dev->dev;
+
+	if (of_device_is_compatible(dev->of_node, "mediatek,mt8173-vcodec-dec"))
+		vdec_dev->chip_name = MTK_VDEC_MT8173;
+	else if (of_device_is_compatible(dev->of_node, "mediatek,mt8183-vcodec-dec"))
+		vdec_dev->chip_name = MTK_VDEC_MT8183;
+	else if (of_device_is_compatible(dev->of_node, "mediatek,mt8192-vcodec-dec"))
+		vdec_dev->chip_name = MTK_VDEC_MT8192;
+	else if (of_device_is_compatible(dev->of_node, "mediatek,mt8195-vcodec-dec"))
+		vdec_dev->chip_name = MTK_VDEC_MT8195;
+	else if (of_device_is_compatible(dev->of_node, "mediatek,mt8186-vcodec-dec"))
+		vdec_dev->chip_name = MTK_VDEC_MT8186;
+	else if (of_device_is_compatible(dev->of_node, "mediatek,mt8188-vcodec-dec"))
+		vdec_dev->chip_name = MTK_VDEC_MT8188;
+	else
+		vdec_dev->chip_name = MTK_VDEC_INVAL;
+}
+
 static int mtk_vcodec_probe(struct platform_device *pdev)
 {
 	struct mtk_vcodec_dec_dev *dev;
@@ -341,6 +361,12 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
 	INIT_LIST_HEAD(&dev->ctx_list);
 	dev->plat_dev = pdev;
 
+	mtk_vcodec_dec_get_chip_name(dev);
+	if (dev->chip_name == MTK_VDEC_INVAL) {
+		dev_err(&pdev->dev, "Failed to get decoder chip name");
+		return -EINVAL;
+	}
+
 	dev->vdec_pdata = of_device_get_match_data(&pdev->dev);
 	if (!of_property_read_u32(pdev->dev.of_node, "mediatek,vpu",
 				  &rproc_phandle)) {
diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h
index 7e36b2c69b7d..8f228ba9aa47 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h
+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h
@@ -18,6 +18,19 @@
 #define IS_VDEC_LAT_ARCH(hw_arch) ((hw_arch) >= MTK_VDEC_LAT_SINGLE_CORE)
 #define IS_VDEC_INNER_RACING(capability) ((capability) & MTK_VCODEC_INNER_RACING)
 
+/*
+ * enum mtk_vcodec_dec_chip_name - Structure used to separate different platform
+ */
+enum mtk_vcodec_dec_chip_name {
+	MTK_VDEC_INVAL = 0,
+	MTK_VDEC_MT8173 = 8173,
+	MTK_VDEC_MT8183 = 8183,
+	MTK_VDEC_MT8186 = 8186,
+	MTK_VDEC_MT8188 = 8188,
+	MTK_VDEC_MT8192 = 8192,
+	MTK_VDEC_MT8195 = 8195,
+};
+
 /*
  * enum mtk_vdec_format_types - Structure used to get supported
  *		  format types according to decoder capability
@@ -249,6 +262,8 @@ struct mtk_vcodec_dec_ctx {
  * @vdec_racing_info: record register value
  * @dec_racing_info_mutex: mutex lock used for inner racing mode
  * @dbgfs: debug log related information
+ *
+ * @chip_name: the chip name used to separate different platform
  */
 struct mtk_vcodec_dec_dev {
 	struct v4l2_device v4l2_dev;
@@ -289,6 +304,8 @@ struct mtk_vcodec_dec_dev {
 	/* Protects access to vdec_racing_info data */
 	struct mutex dec_racing_info_mutex;
 	struct mtk_vcodec_dbgfs dbgfs;
+
+	enum mtk_vcodec_dec_chip_name chip_name;
 };
 
 static inline struct mtk_vcodec_dec_ctx *fh_to_dec_ctx(struct v4l2_fh *fh)
-- 
2.18.0

[PATCH 2/7] media: mediatek: vcodec: Setting the supported h264 level for each platform

[Yunfei Dong]

The supported resolution and fps of different platforms are not the
same. Need to set the supported level according to the chip name.

Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
---
 .../vcodec/decoder/mtk_vcodec_dec_stateless.c | 45 +++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
index e29c9c58f3da..f4af81bddc58 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
@@ -56,6 +56,15 @@ static const struct mtk_stateless_control mtk_stateless_controls[] = {
 		},
 		.codec_type = V4L2_PIX_FMT_H264_SLICE,
 	},
+	{
+		.cfg = {
+			.id = V4L2_CID_MPEG_VIDEO_H264_LEVEL,
+			.min = V4L2_MPEG_VIDEO_H264_LEVEL_1_0,
+			.def = V4L2_MPEG_VIDEO_H264_LEVEL_4_1,
+			.max = V4L2_MPEG_VIDEO_H264_LEVEL_4_2,
+		},
+		.codec_type = V4L2_PIX_FMT_H264_SLICE,
+	},
 	{
 		.cfg = {
 			.id = V4L2_CID_STATELESS_H264_DECODE_MODE,
@@ -519,6 +528,40 @@ static const struct v4l2_ctrl_ops mtk_vcodec_dec_ctrl_ops = {
 	.s_ctrl = mtk_vdec_s_ctrl,
 };
 
+static void mtk_vcodec_dec_fill_h264_level(struct v4l2_ctrl_config *cfg,
+					   struct mtk_vcodec_dec_ctx *ctx)
+{
+	switch (ctx->dev->chip_name) {
+	case MTK_VDEC_MT8192:
+		cfg->max = V4L2_MPEG_VIDEO_H264_LEVEL_5_1;
+		break;
+	case MTK_VDEC_MT8188:
+	case MTK_VDEC_MT8195:
+		cfg->max = V4L2_MPEG_VIDEO_H264_LEVEL_5_2;
+		break;
+	case MTK_VDEC_MT8183:
+	case MTK_VDEC_MT8186:
+		cfg->max = V4L2_MPEG_VIDEO_H264_LEVEL_4_2;
+		break;
+	default:
+		cfg->max = V4L2_MPEG_VIDEO_H264_LEVEL_4_1;
+		break;
+	};
+}
+
+static void mtk_vcodec_dec_reset_controls(struct v4l2_ctrl_config *cfg,
+					  struct mtk_vcodec_dec_ctx *ctx)
+{
+	switch (cfg->id) {
+	case V4L2_CID_MPEG_VIDEO_H264_LEVEL:
+		mtk_vcodec_dec_fill_h264_level(cfg, ctx);
+		mtk_v4l2_vdec_dbg(3, ctx, "h264 supported level: %lld %lld", cfg->max, cfg->def);
+		break;
+	default:
+		break;
+	};
+}
+
 static int mtk_vcodec_dec_ctrls_setup(struct mtk_vcodec_dec_ctx *ctx)
 {
 	unsigned int i;
@@ -532,6 +575,8 @@ static int mtk_vcodec_dec_ctrls_setup(struct mtk_vcodec_dec_ctx *ctx)
 	for (i = 0; i < NUM_CTRLS; i++) {
 		struct v4l2_ctrl_config cfg = mtk_stateless_controls[i].cfg;
 		cfg.ops = &mtk_vcodec_dec_ctrl_ops;
+
+		mtk_vcodec_dec_reset_controls(&cfg, ctx);
 		v4l2_ctrl_new_custom(&ctx->ctrl_hdl, &cfg, NULL);
 		if (ctx->ctrl_hdl.error) {
 			mtk_v4l2_vdec_err(ctx, "Adding control %d failed %d", i,
-- 
2.18.0

[PATCH 3/7] media: mediatek: vcodec: Setting the supported h265 level for each platform

[Yunfei Dong]

The supported resolution and fps of different platforms are not the
same. Need to set the supported level according to the chip name.

Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
---
 .../vcodec/decoder/mtk_vcodec_dec_stateless.c | 28 +++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
index f4af81bddc58..1fdb21dbacb8 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
@@ -147,6 +147,16 @@ static const struct mtk_stateless_control mtk_stateless_controls[] = {
 		},
 		.codec_type = V4L2_PIX_FMT_HEVC_SLICE,
 	},
+	{
+		.cfg = {
+			.id = V4L2_CID_MPEG_VIDEO_HEVC_LEVEL,
+			.min = V4L2_MPEG_VIDEO_HEVC_LEVEL_1,
+			.def = V4L2_MPEG_VIDEO_HEVC_LEVEL_4,
+			.max = V4L2_MPEG_VIDEO_HEVC_LEVEL_4_1,
+		},
+		.codec_type = V4L2_PIX_FMT_HEVC_SLICE,
+	},
+
 	{
 		.cfg = {
 			.id = V4L2_CID_STATELESS_HEVC_DECODE_MODE,
@@ -549,6 +559,20 @@ static void mtk_vcodec_dec_fill_h264_level(struct v4l2_ctrl_config *cfg,
 	};
 }
 
+static void mtk_vcodec_dec_fill_h265_level(struct v4l2_ctrl_config *cfg,
+					   struct mtk_vcodec_dec_ctx *ctx)
+{
+	switch (ctx->dev->chip_name) {
+	case MTK_VDEC_MT8188:
+	case MTK_VDEC_MT8195:
+		cfg->max = V4L2_MPEG_VIDEO_HEVC_LEVEL_5_1;
+		break;
+	default:
+		cfg->max = V4L2_MPEG_VIDEO_HEVC_LEVEL_4;
+		break;
+	};
+}
+
 static void mtk_vcodec_dec_reset_controls(struct v4l2_ctrl_config *cfg,
 					  struct mtk_vcodec_dec_ctx *ctx)
 {
@@ -557,6 +581,10 @@ static void mtk_vcodec_dec_reset_controls(struct v4l2_ctrl_config *cfg,
 		mtk_vcodec_dec_fill_h264_level(cfg, ctx);
 		mtk_v4l2_vdec_dbg(3, ctx, "h264 supported level: %lld %lld", cfg->max, cfg->def);
 		break;
+	case V4L2_CID_MPEG_VIDEO_HEVC_LEVEL:
+		mtk_vcodec_dec_fill_h265_level(cfg, ctx);
+		mtk_v4l2_vdec_dbg(3, ctx, "h265 supported level: %lld %lld", cfg->max, cfg->def);
+		break;
 	default:
 		break;
 	};
-- 
2.18.0

[PATCH 4/7] media: mediatek: vcodec: Setting the supported h264 profile for each platform

[Yunfei Dong]

The supported format type of different platforms are not the
same. Need to set the supported profile according to the chip name.

Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
---
 .../vcodec/decoder/mtk_vcodec_dec_stateless.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
index 1fdb21dbacb8..84c0bed577ed 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
@@ -559,6 +559,20 @@ static void mtk_vcodec_dec_fill_h264_level(struct v4l2_ctrl_config *cfg,
 	};
 }
 
+static void mtk_vcodec_dec_fill_h264_profile(struct v4l2_ctrl_config *cfg,
+					     struct mtk_vcodec_dec_ctx *ctx)
+{
+	switch (ctx->dev->chip_name) {
+	case MTK_VDEC_MT8188:
+	case MTK_VDEC_MT8195:
+		cfg->max = V4L2_MPEG_VIDEO_H264_PROFILE_HIGH_10;
+		break;
+	default:
+		cfg->max = V4L2_MPEG_VIDEO_H264_PROFILE_HIGH;
+		break;
+	};
+}
+
 static void mtk_vcodec_dec_fill_h265_level(struct v4l2_ctrl_config *cfg,
 					   struct mtk_vcodec_dec_ctx *ctx)
 {
@@ -585,6 +599,11 @@ static void mtk_vcodec_dec_reset_controls(struct v4l2_ctrl_config *cfg,
 		mtk_vcodec_dec_fill_h265_level(cfg, ctx);
 		mtk_v4l2_vdec_dbg(3, ctx, "h265 supported level: %lld %lld", cfg->max, cfg->def);
 		break;
+	case V4L2_CID_MPEG_VIDEO_H264_PROFILE:
+		mtk_vcodec_dec_fill_h264_profile(cfg, ctx);
+		mtk_v4l2_vdec_dbg(3, ctx, "h264 supported profile: %lld %lld", cfg->max,
+				  cfg->menu_skip_mask);
+		break;
 	default:
 		break;
 	};
-- 
2.18.0

[PATCH 5/7] media: mediatek: vcodec: Setting the supported h265 profile for each platform

[Yunfei Dong]

The supported format type of different platforms are not the
same. Need to set the supported profile according to the chip name.

Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
---
 .../vcodec/decoder/mtk_vcodec_dec_stateless.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
index 84c0bed577ed..b15ed773374f 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
@@ -587,6 +587,20 @@ static void mtk_vcodec_dec_fill_h265_level(struct v4l2_ctrl_config *cfg,
 	};
 }
 
+static void mtk_vcodec_dec_fill_h265_profile(struct v4l2_ctrl_config *cfg,
+					     struct mtk_vcodec_dec_ctx *ctx)
+{
+	switch (ctx->dev->chip_name) {
+	case MTK_VDEC_MT8188:
+	case MTK_VDEC_MT8195:
+		cfg->max = V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN_10;
+		break;
+	default:
+		cfg->max = V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN_STILL_PICTURE;
+		break;
+	};
+}
+
 static void mtk_vcodec_dec_reset_controls(struct v4l2_ctrl_config *cfg,
 					  struct mtk_vcodec_dec_ctx *ctx)
 {
@@ -604,6 +618,11 @@ static void mtk_vcodec_dec_reset_controls(struct v4l2_ctrl_config *cfg,
 		mtk_v4l2_vdec_dbg(3, ctx, "h264 supported profile: %lld %lld", cfg->max,
 				  cfg->menu_skip_mask);
 		break;
+	case V4L2_CID_MPEG_VIDEO_HEVC_PROFILE:
+		mtk_vcodec_dec_fill_h265_profile(cfg, ctx);
+		mtk_v4l2_vdec_dbg(3, ctx, "h265 supported profile: %lld %lld", cfg->max,
+				  cfg->menu_skip_mask);
+		break;
 	default:
 		break;
 	};
-- 
2.18.0

[PATCH 6/7] media: mediatek: vcodec: Setting the supported vp9 level for each platform

[Yunfei Dong]

The supported resolution and fps of different platforms are not the
same. Need to set the supported level according to the chip name.

Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
---
 .../vcodec/decoder/mtk_vcodec_dec_stateless.c | 33 +++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
index b15ed773374f..02985184fa0f 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
@@ -113,6 +113,15 @@ static const struct mtk_stateless_control mtk_stateless_controls[] = {
 		},
 		.codec_type = V4L2_PIX_FMT_VP9_FRAME,
 	},
+	{
+		.cfg = {
+			.id = V4L2_CID_MPEG_VIDEO_VP9_LEVEL,
+			.min = V4L2_MPEG_VIDEO_VP9_LEVEL_1_0,
+			.def = V4L2_MPEG_VIDEO_VP9_LEVEL_4_0,
+			.max = V4L2_MPEG_VIDEO_VP9_LEVEL_4_1,
+		},
+		.codec_type = V4L2_PIX_FMT_VP9_FRAME,
+	},
 	{
 		.cfg = {
 			.id = V4L2_CID_STATELESS_HEVC_SPS,
@@ -601,6 +610,26 @@ static void mtk_vcodec_dec_fill_h265_profile(struct v4l2_ctrl_config *cfg,
 	};
 }
 
+static void mtk_vcodec_dec_fill_vp9_level(struct v4l2_ctrl_config *cfg,
+					  struct mtk_vcodec_dec_ctx *ctx)
+{
+	switch (ctx->dev->chip_name) {
+	case MTK_VDEC_MT8188:
+	case MTK_VDEC_MT8195:
+		cfg->max = V4L2_MPEG_VIDEO_VP9_LEVEL_5_1;
+		break;
+	case MTK_VDEC_MT8192:
+		cfg->max = V4L2_MPEG_VIDEO_VP9_LEVEL_5_0;
+		break;
+	case MTK_VDEC_MT8186:
+		cfg->max = V4L2_MPEG_VIDEO_VP9_LEVEL_4_1;
+		break;
+	default:
+		cfg->max = V4L2_MPEG_VIDEO_VP9_LEVEL_4_0;
+		break;
+	};
+}
+
 static void mtk_vcodec_dec_reset_controls(struct v4l2_ctrl_config *cfg,
 					  struct mtk_vcodec_dec_ctx *ctx)
 {
@@ -613,6 +642,10 @@ static void mtk_vcodec_dec_reset_controls(struct v4l2_ctrl_config *cfg,
 		mtk_vcodec_dec_fill_h265_level(cfg, ctx);
 		mtk_v4l2_vdec_dbg(3, ctx, "h265 supported level: %lld %lld", cfg->max, cfg->def);
 		break;
+	case V4L2_CID_MPEG_VIDEO_VP9_LEVEL:
+		mtk_vcodec_dec_fill_vp9_level(cfg, ctx);
+		mtk_v4l2_vdec_dbg(3, ctx, "vp9 supported level: %lld %lld", cfg->max, cfg->def);
+		break;
 	case V4L2_CID_MPEG_VIDEO_H264_PROFILE:
 		mtk_vcodec_dec_fill_h264_profile(cfg, ctx);
 		mtk_v4l2_vdec_dbg(3, ctx, "h264 supported profile: %lld %lld", cfg->max,
-- 
2.18.0

[PATCH 7/7] media: mediatek: vcodec: Setting the supported vp9 profile for each platform

[Yunfei Dong]

The supported format type of different platforms are not the
same. Need to set the supported profile according to the chip name.

Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
---
 .../vcodec/decoder/mtk_vcodec_dec_stateless.c | 22 ++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
index 02985184fa0f..ae181498c3c3 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
@@ -109,7 +109,8 @@ static const struct mtk_stateless_control mtk_stateless_controls[] = {
 			.id = V4L2_CID_MPEG_VIDEO_VP9_PROFILE,
 			.min = V4L2_MPEG_VIDEO_VP9_PROFILE_0,
 			.def = V4L2_MPEG_VIDEO_VP9_PROFILE_0,
-			.max = V4L2_MPEG_VIDEO_VP9_PROFILE_3,
+			.max = V4L2_MPEG_VIDEO_VP9_PROFILE_2,
+			.menu_skip_mask = BIT(V4L2_MPEG_VIDEO_VP9_PROFILE_1),
 		},
 		.codec_type = V4L2_PIX_FMT_VP9_FRAME,
 	},
@@ -630,6 +631,20 @@ static void mtk_vcodec_dec_fill_vp9_level(struct v4l2_ctrl_config *cfg,
 	};
 }
 
+static void mtk_vcodec_dec_fill_vp9_profile(struct v4l2_ctrl_config *cfg,
+					    struct mtk_vcodec_dec_ctx *ctx)
+{
+	switch (ctx->dev->chip_name) {
+	case MTK_VDEC_MT8188:
+	case MTK_VDEC_MT8195:
+		cfg->max = V4L2_MPEG_VIDEO_VP9_PROFILE_2;
+		break;
+	default:
+		cfg->max = V4L2_MPEG_VIDEO_VP9_PROFILE_1;
+		break;
+	};
+}
+
 static void mtk_vcodec_dec_reset_controls(struct v4l2_ctrl_config *cfg,
 					  struct mtk_vcodec_dec_ctx *ctx)
 {
@@ -656,6 +671,11 @@ static void mtk_vcodec_dec_reset_controls(struct v4l2_ctrl_config *cfg,
 		mtk_v4l2_vdec_dbg(3, ctx, "h265 supported profile: %lld %lld", cfg->max,
 				  cfg->menu_skip_mask);
 		break;
+	case V4L2_CID_MPEG_VIDEO_VP9_PROFILE:
+		mtk_vcodec_dec_fill_vp9_profile(cfg, ctx);
+		mtk_v4l2_vdec_dbg(3, ctx, "vp9 supported profile: %lld %lld", cfg->max,
+				  cfg->menu_skip_mask);
+		break;
 	default:
 		break;
 	};
-- 
2.18.0

Re: [PATCH 1/7] media: mediatek: vcodec: Getting the chip name of each platform

[AngeloGioacchino Del Regno]

Il 16/10/23 08:43, Yunfei Dong ha scritto:
> Getting the chip name of each platform according to the device
> compatible to set different parameter.
> 
> Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Re: [PATCH 1/7] media: mediatek: vcodec: Getting the chip name of each platform

[Sebastian Fricke]

Hey Yunfei,

Thanks for your patches!

Could you provide a cover-letter for the next version please?
This will help to get a good context of why we need these changes and to
store the changelog in a helpful manner.
Thanks.

On 16.10.2023 14:43, Yunfei Dong wrote:
>Getting the chip name of each platform according to the device
>compatible to set different parameter.

I would reword this commit description slightly, basically what you
change is that you store the chip name in context permanently and that
you utilize a enum to be more descriptive.

So how about:

"""
Store the name of the chip in the context of the driver in order to be
able to choose the correct configuration values for the different codecs.
Use a enum value instead of an integer to store a more descriptive name.
"""

A few more comments below.

>
>Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
>---
> .../mediatek/vcodec/decoder/mtk_vcodec_dec.c  | 24 +----------------
> .../vcodec/decoder/mtk_vcodec_dec_drv.c       | 26 +++++++++++++++++++
> .../vcodec/decoder/mtk_vcodec_dec_drv.h       | 17 ++++++++++++
> 3 files changed, 44 insertions(+), 23 deletions(-)
>
>diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
>index 91ed576d6821..ba742f0e391d 100644
>--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
>+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
>@@ -208,36 +208,14 @@ static int vidioc_vdec_dqbuf(struct file *file, void *priv,
> 	return v4l2_m2m_dqbuf(file, ctx->m2m_ctx, buf);
> }
>
>-static int mtk_vcodec_dec_get_chip_name(void *priv)
>-{
>-	struct mtk_vcodec_dec_ctx *ctx = fh_to_dec_ctx(priv);
>-	struct device *dev = &ctx->dev->plat_dev->dev;
>-
>-	if (of_device_is_compatible(dev->of_node, "mediatek,mt8173-vcodec-dec"))
>-		return 8173;
>-	else if (of_device_is_compatible(dev->of_node, "mediatek,mt8183-vcodec-dec"))
>-		return 8183;
>-	else if (of_device_is_compatible(dev->of_node, "mediatek,mt8192-vcodec-dec"))
>-		return 8192;
>-	else if (of_device_is_compatible(dev->of_node, "mediatek,mt8195-vcodec-dec"))
>-		return 8195;
>-	else if (of_device_is_compatible(dev->of_node, "mediatek,mt8186-vcodec-dec"))
>-		return 8186;
>-	else if (of_device_is_compatible(dev->of_node, "mediatek,mt8188-vcodec-dec"))
>-		return 8188;
>-	else
>-		return 8173;
>-}
>-
> static int vidioc_vdec_querycap(struct file *file, void *priv,
> 				struct v4l2_capability *cap)
> {
> 	struct mtk_vcodec_dec_ctx *ctx = fh_to_dec_ctx(priv);
> 	struct device *dev = &ctx->dev->plat_dev->dev;
>-	int platform_name = mtk_vcodec_dec_get_chip_name(priv);
>
> 	strscpy(cap->driver, dev->driver->name, sizeof(cap->driver));
>-	snprintf(cap->card, sizeof(cap->card), "MT%d video decoder", platform_name);
>+	snprintf(cap->card, sizeof(cap->card), "MT%d video decoder", ctx->dev->chip_name);
>
> 	return 0;
> }
>diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c
>index 0a89ce452ac3..f47c98faf068 100644
>--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c
>+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c
>@@ -326,6 +326,26 @@ static const struct v4l2_file_operations mtk_vcodec_fops = {
> 	.mmap		= v4l2_m2m_fop_mmap,
> };
>
>+static void mtk_vcodec_dec_get_chip_name(struct mtk_vcodec_dec_dev *vdec_dev)
>+{
>+	struct device *dev = &vdec_dev->plat_dev->dev;
>+
>+	if (of_device_is_compatible(dev->of_node, "mediatek,mt8173-vcodec-dec"))
>+		vdec_dev->chip_name = MTK_VDEC_MT8173;
>+	else if (of_device_is_compatible(dev->of_node, "mediatek,mt8183-vcodec-dec"))
>+		vdec_dev->chip_name = MTK_VDEC_MT8183;
>+	else if (of_device_is_compatible(dev->of_node, "mediatek,mt8192-vcodec-dec"))
>+		vdec_dev->chip_name = MTK_VDEC_MT8192;
>+	else if (of_device_is_compatible(dev->of_node, "mediatek,mt8195-vcodec-dec"))
>+		vdec_dev->chip_name = MTK_VDEC_MT8195;
>+	else if (of_device_is_compatible(dev->of_node, "mediatek,mt8186-vcodec-dec"))
>+		vdec_dev->chip_name = MTK_VDEC_MT8186;
>+	else if (of_device_is_compatible(dev->of_node, "mediatek,mt8188-vcodec-dec"))
>+		vdec_dev->chip_name = MTK_VDEC_MT8188;
>+	else
>+		vdec_dev->chip_name = MTK_VDEC_INVAL;
>+}
>+
> static int mtk_vcodec_probe(struct platform_device *pdev)
> {
> 	struct mtk_vcodec_dec_dev *dev;
>@@ -341,6 +361,12 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
> 	INIT_LIST_HEAD(&dev->ctx_list);
> 	dev->plat_dev = pdev;
>
>+	mtk_vcodec_dec_get_chip_name(dev);
>+	if (dev->chip_name == MTK_VDEC_INVAL) {
>+		dev_err(&pdev->dev, "Failed to get decoder chip name");
>+		return -EINVAL;
>+	}
>+
> 	dev->vdec_pdata = of_device_get_match_data(&pdev->dev);
> 	if (!of_property_read_u32(pdev->dev.of_node, "mediatek,vpu",
> 				  &rproc_phandle)) {
>diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h
>index 7e36b2c69b7d..8f228ba9aa47 100644
>--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h
>+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h
>@@ -18,6 +18,19 @@
> #define IS_VDEC_LAT_ARCH(hw_arch) ((hw_arch) >= MTK_VDEC_LAT_SINGLE_CORE)
> #define IS_VDEC_INNER_RACING(capability) ((capability) & MTK_VCODEC_INNER_RACING)
>
>+/*
>+ * enum mtk_vcodec_dec_chip_name - Structure used to separate different platform
>+ */

I don't feel like this comment is terribly helpful because it is pretty
clear what the enum is about, I would just drop it.

>+enum mtk_vcodec_dec_chip_name {
>+	MTK_VDEC_INVAL = 0,
>+	MTK_VDEC_MT8173 = 8173,
>+	MTK_VDEC_MT8183 = 8183,
>+	MTK_VDEC_MT8186 = 8186,
>+	MTK_VDEC_MT8188 = 8188,
>+	MTK_VDEC_MT8192 = 8192,
>+	MTK_VDEC_MT8195 = 8195,
>+};
>+
> /*
>  * enum mtk_vdec_format_types - Structure used to get supported
>  *		  format types according to decoder capability
>@@ -249,6 +262,8 @@ struct mtk_vcodec_dec_ctx {
>  * @vdec_racing_info: record register value
>  * @dec_racing_info_mutex: mutex lock used for inner racing mode
>  * @dbgfs: debug log related information
>+ *
>+ * @chip_name: the chip name used to separate different platform

I wouldn't repeat chip name in the description and specify more
concretely why we need to separate the platforms.

My suggestion:

  * @chip_name: used to distinguish platforms and select the correct codec configuration values.

>  */
> struct mtk_vcodec_dec_dev {
> 	struct v4l2_device v4l2_dev;
>@@ -289,6 +304,8 @@ struct mtk_vcodec_dec_dev {
> 	/* Protects access to vdec_racing_info data */
> 	struct mutex dec_racing_info_mutex;
> 	struct mtk_vcodec_dbgfs dbgfs;
>+
>+	enum mtk_vcodec_dec_chip_name chip_name;
> };
>
> static inline struct mtk_vcodec_dec_ctx *fh_to_dec_ctx(struct v4l2_fh *fh)

Besides those small wording choices, the patch looks good.

So with these issues resolved:

Reviewed-by: Sebastian Fricke <sebastian.fricke@collabora.com>

Regards,
Sebastian
>-- 
>2.18.0
>

Re: [PATCH 2/7] media: mediatek: vcodec: Setting the supported h264 level for each platform

[Sebastian Fricke]

Hey Yunfei,

On 16.10.2023 14:43, Yunfei Dong wrote:
>The supported resolution and fps of different platforms are not the
>same. Need to set the supported level according to the chip name.

I would suggest the following rewording:

Set the maximum H264 codec level for each platform.
The various mediatek platforms support different levels for decoding,
the level of the codec limits among others the maximum resolution, bit
rate and frame rate for the decoder.

With that you can add:
Reviewed-by: Sebastian Fricke <sebastian.fricke@collabora.com>

Regards,
Sebastian

>
>Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
>---
> .../vcodec/decoder/mtk_vcodec_dec_stateless.c | 45 +++++++++++++++++++
> 1 file changed, 45 insertions(+)
>
>diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
>index e29c9c58f3da..f4af81bddc58 100644
>--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
>+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
>@@ -56,6 +56,15 @@ static const struct mtk_stateless_control mtk_stateless_controls[] = {
> 		},
> 		.codec_type = V4L2_PIX_FMT_H264_SLICE,
> 	},
>+	{
>+		.cfg = {
>+			.id = V4L2_CID_MPEG_VIDEO_H264_LEVEL,
>+			.min = V4L2_MPEG_VIDEO_H264_LEVEL_1_0,
>+			.def = V4L2_MPEG_VIDEO_H264_LEVEL_4_1,
>+			.max = V4L2_MPEG_VIDEO_H264_LEVEL_4_2,
>+		},
>+		.codec_type = V4L2_PIX_FMT_H264_SLICE,
>+	},
> 	{
> 		.cfg = {
> 			.id = V4L2_CID_STATELESS_H264_DECODE_MODE,
>@@ -519,6 +528,40 @@ static const struct v4l2_ctrl_ops mtk_vcodec_dec_ctrl_ops = {
> 	.s_ctrl = mtk_vdec_s_ctrl,
> };
>
>+static void mtk_vcodec_dec_fill_h264_level(struct v4l2_ctrl_config *cfg,
>+					   struct mtk_vcodec_dec_ctx *ctx)
>+{
>+	switch (ctx->dev->chip_name) {
>+	case MTK_VDEC_MT8192:
>+		cfg->max = V4L2_MPEG_VIDEO_H264_LEVEL_5_1;
>+		break;
>+	case MTK_VDEC_MT8188:
>+	case MTK_VDEC_MT8195:
>+		cfg->max = V4L2_MPEG_VIDEO_H264_LEVEL_5_2;
>+		break;
>+	case MTK_VDEC_MT8183:
>+	case MTK_VDEC_MT8186:
>+		cfg->max = V4L2_MPEG_VIDEO_H264_LEVEL_4_2;
>+		break;
>+	default:
>+		cfg->max = V4L2_MPEG_VIDEO_H264_LEVEL_4_1;
>+		break;
>+	};
>+}
>+
>+static void mtk_vcodec_dec_reset_controls(struct v4l2_ctrl_config *cfg,
>+					  struct mtk_vcodec_dec_ctx *ctx)
>+{
>+	switch (cfg->id) {
>+	case V4L2_CID_MPEG_VIDEO_H264_LEVEL:
>+		mtk_vcodec_dec_fill_h264_level(cfg, ctx);
>+		mtk_v4l2_vdec_dbg(3, ctx, "h264 supported level: %lld %lld", cfg->max, cfg->def);
>+		break;
>+	default:
>+		break;
>+	};
>+}
>+
> static int mtk_vcodec_dec_ctrls_setup(struct mtk_vcodec_dec_ctx *ctx)
> {
> 	unsigned int i;
>@@ -532,6 +575,8 @@ static int mtk_vcodec_dec_ctrls_setup(struct mtk_vcodec_dec_ctx *ctx)
> 	for (i = 0; i < NUM_CTRLS; i++) {
> 		struct v4l2_ctrl_config cfg = mtk_stateless_controls[i].cfg;
> 		cfg.ops = &mtk_vcodec_dec_ctrl_ops;
>+
>+		mtk_vcodec_dec_reset_controls(&cfg, ctx);
> 		v4l2_ctrl_new_custom(&ctx->ctrl_hdl, &cfg, NULL);
> 		if (ctx->ctrl_hdl.error) {
> 			mtk_v4l2_vdec_err(ctx, "Adding control %d failed %d", i,
>-- 
>2.18.0
>

Re: [PATCH 3/7] media: mediatek: vcodec: Setting the supported h265 level for each platform

[Sebastian Fricke]

Hey Yunfei,

On 16.10.2023 14:43, Yunfei Dong wrote:
>The supported resolution and fps of different platforms are not the
>same. Need to set the supported level according to the chip name.

I would suggest the following rewording:

Set the maximum H265 codec level for each platform.
The various mediatek platforms support different levels for decoding,
the level of the codec limits among others the maximum resolution, bit
rate and frame rate for the decoder.

With that you can add:
Reviewed-by: Sebastian Fricke <sebastian.fricke@collabora.com>

Regards,
Sebastian

>Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
>---
> .../vcodec/decoder/mtk_vcodec_dec_stateless.c | 28 +++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
>diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
>index f4af81bddc58..1fdb21dbacb8 100644
>--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
>+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
>@@ -147,6 +147,16 @@ static const struct mtk_stateless_control mtk_stateless_controls[] = {
> 		},
> 		.codec_type = V4L2_PIX_FMT_HEVC_SLICE,
> 	},
>+	{
>+		.cfg = {
>+			.id = V4L2_CID_MPEG_VIDEO_HEVC_LEVEL,
>+			.min = V4L2_MPEG_VIDEO_HEVC_LEVEL_1,
>+			.def = V4L2_MPEG_VIDEO_HEVC_LEVEL_4,
>+			.max = V4L2_MPEG_VIDEO_HEVC_LEVEL_4_1,
>+		},
>+		.codec_type = V4L2_PIX_FMT_HEVC_SLICE,
>+	},
>+
> 	{
> 		.cfg = {
> 			.id = V4L2_CID_STATELESS_HEVC_DECODE_MODE,
>@@ -549,6 +559,20 @@ static void mtk_vcodec_dec_fill_h264_level(struct v4l2_ctrl_config *cfg,
> 	};
> }
>
>+static void mtk_vcodec_dec_fill_h265_level(struct v4l2_ctrl_config *cfg,
>+					   struct mtk_vcodec_dec_ctx *ctx)
>+{
>+	switch (ctx->dev->chip_name) {
>+	case MTK_VDEC_MT8188:
>+	case MTK_VDEC_MT8195:
>+		cfg->max = V4L2_MPEG_VIDEO_HEVC_LEVEL_5_1;
>+		break;
>+	default:
>+		cfg->max = V4L2_MPEG_VIDEO_HEVC_LEVEL_4;
>+		break;
>+	};
>+}
>+
> static void mtk_vcodec_dec_reset_controls(struct v4l2_ctrl_config *cfg,
> 					  struct mtk_vcodec_dec_ctx *ctx)
> {
>@@ -557,6 +581,10 @@ static void mtk_vcodec_dec_reset_controls(struct v4l2_ctrl_config *cfg,
> 		mtk_vcodec_dec_fill_h264_level(cfg, ctx);
> 		mtk_v4l2_vdec_dbg(3, ctx, "h264 supported level: %lld %lld", cfg->max, cfg->def);
> 		break;
>+	case V4L2_CID_MPEG_VIDEO_HEVC_LEVEL:
>+		mtk_vcodec_dec_fill_h265_level(cfg, ctx);
>+		mtk_v4l2_vdec_dbg(3, ctx, "h265 supported level: %lld %lld", cfg->max, cfg->def);
>+		break;
> 	default:
> 		break;
> 	};
>-- 
>2.18.0
>

Re: [PATCH 4/7] media: mediatek: vcodec: Setting the supported h264 profile for each platform

[Sebastian Fricke]

Hey Yunfei,

On 16.10.2023 14:43, Yunfei Dong wrote:
>The supported format type of different platforms are not the
>same. Need to set the supported profile according to the chip name.

I would suggest the following rewording:

Set the maximum H264 codec profile for each platform.
The various mediatek platforms support different profiles for decoding,
the profile of the codec limits the capabilities for decoding.

With that you can add:
Reviewed-by: Sebastian Fricke <sebastian.fricke@collabora.com>

Regards,
Sebastian

>
>Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
>---
> .../vcodec/decoder/mtk_vcodec_dec_stateless.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
>diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
>index 1fdb21dbacb8..84c0bed577ed 100644
>--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
>+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
>@@ -559,6 +559,20 @@ static void mtk_vcodec_dec_fill_h264_level(struct v4l2_ctrl_config *cfg,
> 	};
> }
>
>+static void mtk_vcodec_dec_fill_h264_profile(struct v4l2_ctrl_config *cfg,
>+					     struct mtk_vcodec_dec_ctx *ctx)
>+{
>+	switch (ctx->dev->chip_name) {
>+	case MTK_VDEC_MT8188:
>+	case MTK_VDEC_MT8195:
>+		cfg->max = V4L2_MPEG_VIDEO_H264_PROFILE_HIGH_10;
>+		break;
>+	default:
>+		cfg->max = V4L2_MPEG_VIDEO_H264_PROFILE_HIGH;
>+		break;
>+	};
>+}
>+
> static void mtk_vcodec_dec_fill_h265_level(struct v4l2_ctrl_config *cfg,
> 					   struct mtk_vcodec_dec_ctx *ctx)
> {
>@@ -585,6 +599,11 @@ static void mtk_vcodec_dec_reset_controls(struct v4l2_ctrl_config *cfg,
> 		mtk_vcodec_dec_fill_h265_level(cfg, ctx);
> 		mtk_v4l2_vdec_dbg(3, ctx, "h265 supported level: %lld %lld", cfg->max, cfg->def);
> 		break;
>+	case V4L2_CID_MPEG_VIDEO_H264_PROFILE:
>+		mtk_vcodec_dec_fill_h264_profile(cfg, ctx);
>+		mtk_v4l2_vdec_dbg(3, ctx, "h264 supported profile: %lld %lld", cfg->max,
>+				  cfg->menu_skip_mask);
>+		break;
> 	default:
> 		break;
> 	};
>-- 
>2.18.0
>

Re: [PATCH 5/7] media: mediatek: vcodec: Setting the supported h265 profile for each platform

[Sebastian Fricke]

Hey Yunfei,

Please replace Setting with Set in the title.

On 16.10.2023 14:43, Yunfei Dong wrote:
>The supported format type of different platforms are not the
>same. Need to set the supported profile according to the chip name.

I would suggest the following rewording:

Set the maximum H265 codec profile for each platform.
The various mediatek platforms support different profiles for decoding,
the profile of the codec limits the capabilities for decoding.

With that you can add:
Reviewed-by: Sebastian Fricke <sebastian.fricke@collabora.com>

Regards,
Sebastian

>
>Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
>---
> .../vcodec/decoder/mtk_vcodec_dec_stateless.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
>diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
>index 84c0bed577ed..b15ed773374f 100644
>--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
>+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
>@@ -587,6 +587,20 @@ static void mtk_vcodec_dec_fill_h265_level(struct v4l2_ctrl_config *cfg,
> 	};
> }
>
>+static void mtk_vcodec_dec_fill_h265_profile(struct v4l2_ctrl_config *cfg,
>+					     struct mtk_vcodec_dec_ctx *ctx)
>+{
>+	switch (ctx->dev->chip_name) {
>+	case MTK_VDEC_MT8188:
>+	case MTK_VDEC_MT8195:
>+		cfg->max = V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN_10;
>+		break;
>+	default:
>+		cfg->max = V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN_STILL_PICTURE;
>+		break;
>+	};
>+}
>+
> static void mtk_vcodec_dec_reset_controls(struct v4l2_ctrl_config *cfg,
> 					  struct mtk_vcodec_dec_ctx *ctx)
> {
>@@ -604,6 +618,11 @@ static void mtk_vcodec_dec_reset_controls(struct v4l2_ctrl_config *cfg,
> 		mtk_v4l2_vdec_dbg(3, ctx, "h264 supported profile: %lld %lld", cfg->max,
> 				  cfg->menu_skip_mask);
> 		break;
>+	case V4L2_CID_MPEG_VIDEO_HEVC_PROFILE:
>+		mtk_vcodec_dec_fill_h265_profile(cfg, ctx);
>+		mtk_v4l2_vdec_dbg(3, ctx, "h265 supported profile: %lld %lld", cfg->max,
>+				  cfg->menu_skip_mask);
>+		break;
> 	default:
> 		break;
> 	};
>-- 
>2.18.0
>

Re: [PATCH 6/7] media: mediatek: vcodec: Setting the supported vp9 level for each platform

[Sebastian Fricke]

Hey Yunfei,

please replace Setting with Set in the title.

On 16.10.2023 14:43, Yunfei Dong wrote:
>The supported resolution and fps of different platforms are not the
>same. Need to set the supported level according to the chip name.

I would suggest the following rewording:

Set the maximum VP9 codec level for each platform.
The various mediatek platforms support different levels for decoding,
the level of the codec limits among others the maximum resolution, bit
rate, frame rate and compression rate for the decoder.

With that you can add:
Reviewed-by: Sebastian Fricke <sebastian.fricke@collabora.com>

Regards,
Sebastian

>
>Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
>---
> .../vcodec/decoder/mtk_vcodec_dec_stateless.c | 33 +++++++++++++++++++
> 1 file changed, 33 insertions(+)
>
>diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
>index b15ed773374f..02985184fa0f 100644
>--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
>+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
>@@ -113,6 +113,15 @@ static const struct mtk_stateless_control mtk_stateless_controls[] = {
> 		},
> 		.codec_type = V4L2_PIX_FMT_VP9_FRAME,
> 	},
>+	{
>+		.cfg = {
>+			.id = V4L2_CID_MPEG_VIDEO_VP9_LEVEL,
>+			.min = V4L2_MPEG_VIDEO_VP9_LEVEL_1_0,
>+			.def = V4L2_MPEG_VIDEO_VP9_LEVEL_4_0,
>+			.max = V4L2_MPEG_VIDEO_VP9_LEVEL_4_1,
>+		},
>+		.codec_type = V4L2_PIX_FMT_VP9_FRAME,
>+	},
> 	{
> 		.cfg = {
> 			.id = V4L2_CID_STATELESS_HEVC_SPS,
>@@ -601,6 +610,26 @@ static void mtk_vcodec_dec_fill_h265_profile(struct v4l2_ctrl_config *cfg,
> 	};
> }
>
>+static void mtk_vcodec_dec_fill_vp9_level(struct v4l2_ctrl_config *cfg,
>+					  struct mtk_vcodec_dec_ctx *ctx)
>+{
>+	switch (ctx->dev->chip_name) {
>+	case MTK_VDEC_MT8188:
>+	case MTK_VDEC_MT8195:
>+		cfg->max = V4L2_MPEG_VIDEO_VP9_LEVEL_5_1;
>+		break;
>+	case MTK_VDEC_MT8192:
>+		cfg->max = V4L2_MPEG_VIDEO_VP9_LEVEL_5_0;
>+		break;
>+	case MTK_VDEC_MT8186:
>+		cfg->max = V4L2_MPEG_VIDEO_VP9_LEVEL_4_1;
>+		break;
>+	default:
>+		cfg->max = V4L2_MPEG_VIDEO_VP9_LEVEL_4_0;
>+		break;
>+	};
>+}
>+
> static void mtk_vcodec_dec_reset_controls(struct v4l2_ctrl_config *cfg,
> 					  struct mtk_vcodec_dec_ctx *ctx)
> {
>@@ -613,6 +642,10 @@ static void mtk_vcodec_dec_reset_controls(struct v4l2_ctrl_config *cfg,
> 		mtk_vcodec_dec_fill_h265_level(cfg, ctx);
> 		mtk_v4l2_vdec_dbg(3, ctx, "h265 supported level: %lld %lld", cfg->max, cfg->def);
> 		break;
>+	case V4L2_CID_MPEG_VIDEO_VP9_LEVEL:
>+		mtk_vcodec_dec_fill_vp9_level(cfg, ctx);
>+		mtk_v4l2_vdec_dbg(3, ctx, "vp9 supported level: %lld %lld", cfg->max, cfg->def);
>+		break;
> 	case V4L2_CID_MPEG_VIDEO_H264_PROFILE:
> 		mtk_vcodec_dec_fill_h264_profile(cfg, ctx);
> 		mtk_v4l2_vdec_dbg(3, ctx, "h264 supported profile: %lld %lld", cfg->max,
>-- 
>2.18.0
>

Re: [PATCH 7/7] media: mediatek: vcodec: Setting the supported vp9 profile for each platform

[Sebastian Fricke]

Hey Yunfei,

Please replace Setting with Set in the title.

On 16.10.2023 14:43, Yunfei Dong wrote:
>The supported format type of different platforms are not the
>same. Need to set the supported profile according to the chip name.

I would suggest the following rewording:

Set the maximum VP9 codec profile for each platform.
The various mediatek platforms support different profiles for decoding,
the profile of the codec limits the capabilities for decoding.

With that you can add:
Reviewed-by: Sebastian Fricke <sebastian.fricke@collabora.com>

Regards,
Sebastian

>
>Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
>---
> .../vcodec/decoder/mtk_vcodec_dec_stateless.c | 22 ++++++++++++++++++-
> 1 file changed, 21 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
>index 02985184fa0f..ae181498c3c3 100644
>--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
>+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
>@@ -109,7 +109,8 @@ static const struct mtk_stateless_control mtk_stateless_controls[] = {
> 			.id = V4L2_CID_MPEG_VIDEO_VP9_PROFILE,
> 			.min = V4L2_MPEG_VIDEO_VP9_PROFILE_0,
> 			.def = V4L2_MPEG_VIDEO_VP9_PROFILE_0,
>-			.max = V4L2_MPEG_VIDEO_VP9_PROFILE_3,
>+			.max = V4L2_MPEG_VIDEO_VP9_PROFILE_2,
>+			.menu_skip_mask = BIT(V4L2_MPEG_VIDEO_VP9_PROFILE_1),
> 		},
> 		.codec_type = V4L2_PIX_FMT_VP9_FRAME,
> 	},
>@@ -630,6 +631,20 @@ static void mtk_vcodec_dec_fill_vp9_level(struct v4l2_ctrl_config *cfg,
> 	};
> }
>
>+static void mtk_vcodec_dec_fill_vp9_profile(struct v4l2_ctrl_config *cfg,
>+					    struct mtk_vcodec_dec_ctx *ctx)
>+{
>+	switch (ctx->dev->chip_name) {
>+	case MTK_VDEC_MT8188:
>+	case MTK_VDEC_MT8195:
>+		cfg->max = V4L2_MPEG_VIDEO_VP9_PROFILE_2;
>+		break;
>+	default:
>+		cfg->max = V4L2_MPEG_VIDEO_VP9_PROFILE_1;
>+		break;
>+	};
>+}
>+
> static void mtk_vcodec_dec_reset_controls(struct v4l2_ctrl_config *cfg,
> 					  struct mtk_vcodec_dec_ctx *ctx)
> {
>@@ -656,6 +671,11 @@ static void mtk_vcodec_dec_reset_controls(struct v4l2_ctrl_config *cfg,
> 		mtk_v4l2_vdec_dbg(3, ctx, "h265 supported profile: %lld %lld", cfg->max,
> 				  cfg->menu_skip_mask);
> 		break;
>+	case V4L2_CID_MPEG_VIDEO_VP9_PROFILE:
>+		mtk_vcodec_dec_fill_vp9_profile(cfg, ctx);
>+		mtk_v4l2_vdec_dbg(3, ctx, "vp9 supported profile: %lld %lld", cfg->max,
>+				  cfg->menu_skip_mask);
>+		break;
> 	default:
> 		break;
> 	};
>-- 
>2.18.0
>

Re: [PATCH 1/7] media: mediatek: vcodec: Getting the chip name of each platform

[Sebastian Fricke]

Hey Yunfei,

please replace "Getting" with "Get" in the title and replace "of each"
with "for each".

On 21.10.2023 10:30, Sebastian Fricke wrote:
>Hey Yunfei,
>
>Thanks for your patches!
>
>Could you provide a cover-letter for the next version please?
>This will help to get a good context of why we need these changes and to
>store the changelog in a helpful manner.
>Thanks.
>
>On 16.10.2023 14:43, Yunfei Dong wrote:
>>Getting the chip name of each platform according to the device
>>compatible to set different parameter.
>
>I would reword this commit description slightly, basically what you
>change is that you store the chip name in context permanently and that
>you utilize a enum to be more descriptive.
>
>So how about:
>
>"""
>Store the name of the chip in the context of the driver in order to be
>able to choose the correct configuration values for the different codecs.
>Use a enum value instead of an integer to store a more descriptive name.
>"""
>
>A few more comments below.
>
>>
>>Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
>>---
>>.../mediatek/vcodec/decoder/mtk_vcodec_dec.c  | 24 +----------------
>>.../vcodec/decoder/mtk_vcodec_dec_drv.c       | 26 +++++++++++++++++++
>>.../vcodec/decoder/mtk_vcodec_dec_drv.h       | 17 ++++++++++++
>>3 files changed, 44 insertions(+), 23 deletions(-)
>>
>>diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
>>index 91ed576d6821..ba742f0e391d 100644
>>--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
>>+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
>>@@ -208,36 +208,14 @@ static int vidioc_vdec_dqbuf(struct file *file, void *priv,
>>	return v4l2_m2m_dqbuf(file, ctx->m2m_ctx, buf);
>>}
>>
>>-static int mtk_vcodec_dec_get_chip_name(void *priv)
>>-{
>>-	struct mtk_vcodec_dec_ctx *ctx = fh_to_dec_ctx(priv);
>>-	struct device *dev = &ctx->dev->plat_dev->dev;
>>-
>>-	if (of_device_is_compatible(dev->of_node, "mediatek,mt8173-vcodec-dec"))
>>-		return 8173;
>>-	else if (of_device_is_compatible(dev->of_node, "mediatek,mt8183-vcodec-dec"))
>>-		return 8183;
>>-	else if (of_device_is_compatible(dev->of_node, "mediatek,mt8192-vcodec-dec"))
>>-		return 8192;
>>-	else if (of_device_is_compatible(dev->of_node, "mediatek,mt8195-vcodec-dec"))
>>-		return 8195;
>>-	else if (of_device_is_compatible(dev->of_node, "mediatek,mt8186-vcodec-dec"))
>>-		return 8186;
>>-	else if (of_device_is_compatible(dev->of_node, "mediatek,mt8188-vcodec-dec"))
>>-		return 8188;
>>-	else
>>-		return 8173;
>>-}
>>-
>>static int vidioc_vdec_querycap(struct file *file, void *priv,
>>				struct v4l2_capability *cap)
>>{
>>	struct mtk_vcodec_dec_ctx *ctx = fh_to_dec_ctx(priv);
>>	struct device *dev = &ctx->dev->plat_dev->dev;
>>-	int platform_name = mtk_vcodec_dec_get_chip_name(priv);
>>
>>	strscpy(cap->driver, dev->driver->name, sizeof(cap->driver));
>>-	snprintf(cap->card, sizeof(cap->card), "MT%d video decoder", platform_name);
>>+	snprintf(cap->card, sizeof(cap->card), "MT%d video decoder", ctx->dev->chip_name);
>>
>>	return 0;
>>}
>>diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c
>>index 0a89ce452ac3..f47c98faf068 100644
>>--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c
>>+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c
>>@@ -326,6 +326,26 @@ static const struct v4l2_file_operations mtk_vcodec_fops = {
>>	.mmap		= v4l2_m2m_fop_mmap,
>>};
>>
>>+static void mtk_vcodec_dec_get_chip_name(struct mtk_vcodec_dec_dev *vdec_dev)
>>+{
>>+	struct device *dev = &vdec_dev->plat_dev->dev;
>>+
>>+	if (of_device_is_compatible(dev->of_node, "mediatek,mt8173-vcodec-dec"))
>>+		vdec_dev->chip_name = MTK_VDEC_MT8173;
>>+	else if (of_device_is_compatible(dev->of_node, "mediatek,mt8183-vcodec-dec"))
>>+		vdec_dev->chip_name = MTK_VDEC_MT8183;
>>+	else if (of_device_is_compatible(dev->of_node, "mediatek,mt8192-vcodec-dec"))
>>+		vdec_dev->chip_name = MTK_VDEC_MT8192;
>>+	else if (of_device_is_compatible(dev->of_node, "mediatek,mt8195-vcodec-dec"))
>>+		vdec_dev->chip_name = MTK_VDEC_MT8195;
>>+	else if (of_device_is_compatible(dev->of_node, "mediatek,mt8186-vcodec-dec"))
>>+		vdec_dev->chip_name = MTK_VDEC_MT8186;
>>+	else if (of_device_is_compatible(dev->of_node, "mediatek,mt8188-vcodec-dec"))
>>+		vdec_dev->chip_name = MTK_VDEC_MT8188;
>>+	else
>>+		vdec_dev->chip_name = MTK_VDEC_INVAL;
>>+}
>>+
>>static int mtk_vcodec_probe(struct platform_device *pdev)
>>{
>>	struct mtk_vcodec_dec_dev *dev;
>>@@ -341,6 +361,12 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
>>	INIT_LIST_HEAD(&dev->ctx_list);
>>	dev->plat_dev = pdev;
>>
>>+	mtk_vcodec_dec_get_chip_name(dev);
>>+	if (dev->chip_name == MTK_VDEC_INVAL) {
>>+		dev_err(&pdev->dev, "Failed to get decoder chip name");
>>+		return -EINVAL;
>>+	}
>>+
>>	dev->vdec_pdata = of_device_get_match_data(&pdev->dev);
>>	if (!of_property_read_u32(pdev->dev.of_node, "mediatek,vpu",
>>				  &rproc_phandle)) {
>>diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h
>>index 7e36b2c69b7d..8f228ba9aa47 100644
>>--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h
>>+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h
>>@@ -18,6 +18,19 @@
>>#define IS_VDEC_LAT_ARCH(hw_arch) ((hw_arch) >= MTK_VDEC_LAT_SINGLE_CORE)
>>#define IS_VDEC_INNER_RACING(capability) ((capability) & MTK_VCODEC_INNER_RACING)
>>
>>+/*
>>+ * enum mtk_vcodec_dec_chip_name - Structure used to separate different platform
>>+ */
>
>I don't feel like this comment is terribly helpful because it is pretty
>clear what the enum is about, I would just drop it.
>
>>+enum mtk_vcodec_dec_chip_name {
>>+	MTK_VDEC_INVAL = 0,
>>+	MTK_VDEC_MT8173 = 8173,
>>+	MTK_VDEC_MT8183 = 8183,
>>+	MTK_VDEC_MT8186 = 8186,
>>+	MTK_VDEC_MT8188 = 8188,
>>+	MTK_VDEC_MT8192 = 8192,
>>+	MTK_VDEC_MT8195 = 8195,
>>+};
>>+
>>/*
>> * enum mtk_vdec_format_types - Structure used to get supported
>> *		  format types according to decoder capability
>>@@ -249,6 +262,8 @@ struct mtk_vcodec_dec_ctx {
>> * @vdec_racing_info: record register value
>> * @dec_racing_info_mutex: mutex lock used for inner racing mode
>> * @dbgfs: debug log related information
>>+ *
>>+ * @chip_name: the chip name used to separate different platform
>
>I wouldn't repeat chip name in the description and specify more
>concretely why we need to separate the platforms.
>
>My suggestion:
>
> * @chip_name: used to distinguish platforms and select the correct codec configuration values.
>
>> */
>>struct mtk_vcodec_dec_dev {
>>	struct v4l2_device v4l2_dev;
>>@@ -289,6 +304,8 @@ struct mtk_vcodec_dec_dev {
>>	/* Protects access to vdec_racing_info data */
>>	struct mutex dec_racing_info_mutex;
>>	struct mtk_vcodec_dbgfs dbgfs;
>>+
>>+	enum mtk_vcodec_dec_chip_name chip_name;
>>};
>>
>>static inline struct mtk_vcodec_dec_ctx *fh_to_dec_ctx(struct v4l2_fh *fh)
>
>Besides those small wording choices, the patch looks good.
>
>So with these issues resolved:
>
>Reviewed-by: Sebastian Fricke <sebastian.fricke@collabora.com>
>
>Regards,
>Sebastian
>>-- 
>>2.18.0
>>

Re: [PATCH 2/7] media: mediatek: vcodec: Setting the supported h264 level for each platform

[Sebastian Fricke]

Hey Yunfei,

please replace Setting with Set in the title.

On 21.10.2023 11:23, Sebastian Fricke wrote:
>Hey Yunfei,
>
>On 16.10.2023 14:43, Yunfei Dong wrote:
>>The supported resolution and fps of different platforms are not the
>>same. Need to set the supported level according to the chip name.
>
>I would suggest the following rewording:
>
>Set the maximum H264 codec level for each platform.
>The various mediatek platforms support different levels for decoding,
>the level of the codec limits among others the maximum resolution, bit
>rate and frame rate for the decoder.
>
>With that you can add:
>Reviewed-by: Sebastian Fricke <sebastian.fricke@collabora.com>
>
>Regards,
>Sebastian
>
>>
>>Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
>>---
>>.../vcodec/decoder/mtk_vcodec_dec_stateless.c | 45 +++++++++++++++++++
>>1 file changed, 45 insertions(+)
>>
>>diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
>>index e29c9c58f3da..f4af81bddc58 100644
>>--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
>>+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
>>@@ -56,6 +56,15 @@ static const struct mtk_stateless_control mtk_stateless_controls[] = {
>>		},
>>		.codec_type = V4L2_PIX_FMT_H264_SLICE,
>>	},
>>+	{
>>+		.cfg = {
>>+			.id = V4L2_CID_MPEG_VIDEO_H264_LEVEL,
>>+			.min = V4L2_MPEG_VIDEO_H264_LEVEL_1_0,
>>+			.def = V4L2_MPEG_VIDEO_H264_LEVEL_4_1,
>>+			.max = V4L2_MPEG_VIDEO_H264_LEVEL_4_2,
>>+		},
>>+		.codec_type = V4L2_PIX_FMT_H264_SLICE,
>>+	},
>>	{
>>		.cfg = {
>>			.id = V4L2_CID_STATELESS_H264_DECODE_MODE,
>>@@ -519,6 +528,40 @@ static const struct v4l2_ctrl_ops mtk_vcodec_dec_ctrl_ops = {
>>	.s_ctrl = mtk_vdec_s_ctrl,
>>};
>>
>>+static void mtk_vcodec_dec_fill_h264_level(struct v4l2_ctrl_config *cfg,
>>+					   struct mtk_vcodec_dec_ctx *ctx)
>>+{
>>+	switch (ctx->dev->chip_name) {
>>+	case MTK_VDEC_MT8192:
>>+		cfg->max = V4L2_MPEG_VIDEO_H264_LEVEL_5_1;
>>+		break;
>>+	case MTK_VDEC_MT8188:
>>+	case MTK_VDEC_MT8195:
>>+		cfg->max = V4L2_MPEG_VIDEO_H264_LEVEL_5_2;
>>+		break;
>>+	case MTK_VDEC_MT8183:
>>+	case MTK_VDEC_MT8186:
>>+		cfg->max = V4L2_MPEG_VIDEO_H264_LEVEL_4_2;
>>+		break;
>>+	default:
>>+		cfg->max = V4L2_MPEG_VIDEO_H264_LEVEL_4_1;
>>+		break;
>>+	};
>>+}
>>+
>>+static void mtk_vcodec_dec_reset_controls(struct v4l2_ctrl_config *cfg,
>>+					  struct mtk_vcodec_dec_ctx *ctx)
>>+{
>>+	switch (cfg->id) {
>>+	case V4L2_CID_MPEG_VIDEO_H264_LEVEL:
>>+		mtk_vcodec_dec_fill_h264_level(cfg, ctx);
>>+		mtk_v4l2_vdec_dbg(3, ctx, "h264 supported level: %lld %lld", cfg->max, cfg->def);
>>+		break;
>>+	default:
>>+		break;
>>+	};
>>+}
>>+
>>static int mtk_vcodec_dec_ctrls_setup(struct mtk_vcodec_dec_ctx *ctx)
>>{
>>	unsigned int i;
>>@@ -532,6 +575,8 @@ static int mtk_vcodec_dec_ctrls_setup(struct mtk_vcodec_dec_ctx *ctx)
>>	for (i = 0; i < NUM_CTRLS; i++) {
>>		struct v4l2_ctrl_config cfg = mtk_stateless_controls[i].cfg;
>>		cfg.ops = &mtk_vcodec_dec_ctrl_ops;
>>+
>>+		mtk_vcodec_dec_reset_controls(&cfg, ctx);
>>		v4l2_ctrl_new_custom(&ctx->ctrl_hdl, &cfg, NULL);
>>		if (ctx->ctrl_hdl.error) {
>>			mtk_v4l2_vdec_err(ctx, "Adding control %d failed %d", i,
>>-- 
>>2.18.0
>>

Re: [PATCH 3/7] media: mediatek: vcodec: Setting the supported h265 level for each platform

[Sebastian Fricke]

Hey Yunfei,

please replace Setting with Set in the title.

On 21.10.2023 11:25, Sebastian Fricke wrote:
>Hey Yunfei,
>
>On 16.10.2023 14:43, Yunfei Dong wrote:
>>The supported resolution and fps of different platforms are not the
>>same. Need to set the supported level according to the chip name.
>
>I would suggest the following rewording:
>
>Set the maximum H265 codec level for each platform.
>The various mediatek platforms support different levels for decoding,
>the level of the codec limits among others the maximum resolution, bit
>rate and frame rate for the decoder.
>
>With that you can add:
>Reviewed-by: Sebastian Fricke <sebastian.fricke@collabora.com>
>
>Regards,
>Sebastian
>
>>Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
>>---
>>.../vcodec/decoder/mtk_vcodec_dec_stateless.c | 28 +++++++++++++++++++
>>1 file changed, 28 insertions(+)
>>
>>diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
>>index f4af81bddc58..1fdb21dbacb8 100644
>>--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
>>+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
>>@@ -147,6 +147,16 @@ static const struct mtk_stateless_control mtk_stateless_controls[] = {
>>		},
>>		.codec_type = V4L2_PIX_FMT_HEVC_SLICE,
>>	},
>>+	{
>>+		.cfg = {
>>+			.id = V4L2_CID_MPEG_VIDEO_HEVC_LEVEL,
>>+			.min = V4L2_MPEG_VIDEO_HEVC_LEVEL_1,
>>+			.def = V4L2_MPEG_VIDEO_HEVC_LEVEL_4,
>>+			.max = V4L2_MPEG_VIDEO_HEVC_LEVEL_4_1,
>>+		},
>>+		.codec_type = V4L2_PIX_FMT_HEVC_SLICE,
>>+	},
>>+
>>	{
>>		.cfg = {
>>			.id = V4L2_CID_STATELESS_HEVC_DECODE_MODE,
>>@@ -549,6 +559,20 @@ static void mtk_vcodec_dec_fill_h264_level(struct v4l2_ctrl_config *cfg,
>>	};
>>}
>>
>>+static void mtk_vcodec_dec_fill_h265_level(struct v4l2_ctrl_config *cfg,
>>+					   struct mtk_vcodec_dec_ctx *ctx)
>>+{
>>+	switch (ctx->dev->chip_name) {
>>+	case MTK_VDEC_MT8188:
>>+	case MTK_VDEC_MT8195:
>>+		cfg->max = V4L2_MPEG_VIDEO_HEVC_LEVEL_5_1;
>>+		break;
>>+	default:
>>+		cfg->max = V4L2_MPEG_VIDEO_HEVC_LEVEL_4;
>>+		break;
>>+	};
>>+}
>>+
>>static void mtk_vcodec_dec_reset_controls(struct v4l2_ctrl_config *cfg,
>>					  struct mtk_vcodec_dec_ctx *ctx)
>>{
>>@@ -557,6 +581,10 @@ static void mtk_vcodec_dec_reset_controls(struct v4l2_ctrl_config *cfg,
>>		mtk_vcodec_dec_fill_h264_level(cfg, ctx);
>>		mtk_v4l2_vdec_dbg(3, ctx, "h264 supported level: %lld %lld", cfg->max, cfg->def);
>>		break;
>>+	case V4L2_CID_MPEG_VIDEO_HEVC_LEVEL:
>>+		mtk_vcodec_dec_fill_h265_level(cfg, ctx);
>>+		mtk_v4l2_vdec_dbg(3, ctx, "h265 supported level: %lld %lld", cfg->max, cfg->def);
>>+		break;
>>	default:
>>		break;
>>	};
>>-- 
>>2.18.0
>>

Re: [PATCH 4/7] media: mediatek: vcodec: Setting the supported h264 profile for each platform

[Sebastian Fricke]

Hey Yunfei,

please replace Setting with Set in the title.

On 21.10.2023 11:29, Sebastian Fricke wrote:
>Hey Yunfei,
>
>On 16.10.2023 14:43, Yunfei Dong wrote:
>>The supported format type of different platforms are not the
>>same. Need to set the supported profile according to the chip name.
>
>I would suggest the following rewording:
>
>Set the maximum H264 codec profile for each platform.
>The various mediatek platforms support different profiles for decoding,
>the profile of the codec limits the capabilities for decoding.
>
>With that you can add:
>Reviewed-by: Sebastian Fricke <sebastian.fricke@collabora.com>
>
>Regards,
>Sebastian
>
>>
>>Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
>>---
>>.../vcodec/decoder/mtk_vcodec_dec_stateless.c | 19 +++++++++++++++++++
>>1 file changed, 19 insertions(+)
>>
>>diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
>>index 1fdb21dbacb8..84c0bed577ed 100644
>>--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
>>+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
>>@@ -559,6 +559,20 @@ static void mtk_vcodec_dec_fill_h264_level(struct v4l2_ctrl_config *cfg,
>>	};
>>}
>>
>>+static void mtk_vcodec_dec_fill_h264_profile(struct v4l2_ctrl_config *cfg,
>>+					     struct mtk_vcodec_dec_ctx *ctx)
>>+{
>>+	switch (ctx->dev->chip_name) {
>>+	case MTK_VDEC_MT8188:
>>+	case MTK_VDEC_MT8195:
>>+		cfg->max = V4L2_MPEG_VIDEO_H264_PROFILE_HIGH_10;
>>+		break;
>>+	default:
>>+		cfg->max = V4L2_MPEG_VIDEO_H264_PROFILE_HIGH;
>>+		break;
>>+	};
>>+}
>>+
>>static void mtk_vcodec_dec_fill_h265_level(struct v4l2_ctrl_config *cfg,
>>					   struct mtk_vcodec_dec_ctx *ctx)
>>{
>>@@ -585,6 +599,11 @@ static void mtk_vcodec_dec_reset_controls(struct v4l2_ctrl_config *cfg,
>>		mtk_vcodec_dec_fill_h265_level(cfg, ctx);
>>		mtk_v4l2_vdec_dbg(3, ctx, "h265 supported level: %lld %lld", cfg->max, cfg->def);
>>		break;
>>+	case V4L2_CID_MPEG_VIDEO_H264_PROFILE:
>>+		mtk_vcodec_dec_fill_h264_profile(cfg, ctx);
>>+		mtk_v4l2_vdec_dbg(3, ctx, "h264 supported profile: %lld %lld", cfg->max,
>>+				  cfg->menu_skip_mask);
>>+		break;
>>	default:
>>		break;
>>	};
>>-- 
>>2.18.0
>>

Re: [PATCH 1/7] media: mediatek: vcodec: Getting the chip name of each platform

[Yunfei Dong (董云飞)]

Hi Sebastian,

Thanks for your advice.

Send the patch v2 to fix the below items:

Fix the commit message for some patches.
Fix the level of mt8195 and mt8192.

Best Regards,
Yunfei Dong
On Sat, 2023-10-21 at 10:30 +0200, Sebastian Fricke wrote:
> Hey Yunfei,
> 
> Thanks for your patches!
> 
> Could you provide a cover-letter for the next version please?
> This will help to get a good context of why we need these changes and
> to
> store the changelog in a helpful manner.
> Thanks.
> 
> On 16.10.2023 14:43, Yunfei Dong wrote:
> > Getting the chip name of each platform according to the device
> > compatible to set different parameter.
> 
> I would reword this commit description slightly, basically what you
> change is that you store the chip name in context permanently and
> that
> you utilize a enum to be more descriptive.
> 
> So how about:
> 
> """
> Store the name of the chip in the context of the driver in order to
> be
> able to choose the correct configuration values for the different
> codecs.
> Use a enum value instead of an integer to store a more descriptive
> name.
> """
> 
> A few more comments below.
> 
> > 
> > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
> > ---
> > .../mediatek/vcodec/decoder/mtk_vcodec_dec.c  | 24 +---------------
> > -
> > .../vcodec/decoder/mtk_vcodec_dec_drv.c       | 26
> > +++++++++++++++++++
> > .../vcodec/decoder/mtk_vcodec_dec_drv.h       | 17 ++++++++++++
> > 3 files changed, 44 insertions(+), 23 deletions(-)
> > 
> > diff --git
> > a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
> > b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
> > index 91ed576d6821..ba742f0e391d 100644
> > ---
> > a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
> > +++
> > b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
> > @@ -208,36 +208,14 @@ static int vidioc_vdec_dqbuf(struct file
> > *file, void *priv,
> > 	return v4l2_m2m_dqbuf(file, ctx->m2m_ctx, buf);
> > }
> > 
> > -static int mtk_vcodec_dec_get_chip_name(void *priv)
> > -{
> > -	struct mtk_vcodec_dec_ctx *ctx = fh_to_dec_ctx(priv);
> > -	struct device *dev = &ctx->dev->plat_dev->dev;
> > -
> > -	if (of_device_is_compatible(dev->of_node, "mediatek,mt8173-
> > vcodec-dec"))
> > -		return 8173;
> > -	else if (of_device_is_compatible(dev->of_node,
> > "mediatek,mt8183-vcodec-dec"))
> > -		return 8183;
> > -	else if (of_device_is_compatible(dev->of_node,
> > "mediatek,mt8192-vcodec-dec"))
> > -		return 8192;
> > -	else if (of_device_is_compatible(dev->of_node,
> > "mediatek,mt8195-vcodec-dec"))
> > -		return 8195;
> > -	else if (of_device_is_compatible(dev->of_node,
> > "mediatek,mt8186-vcodec-dec"))
> > -		return 8186;
> > -	else if (of_device_is_compatible(dev->of_node,
> > "mediatek,mt8188-vcodec-dec"))
> > -		return 8188;
> > -	else
> > -		return 8173;
> > -}
> > -
> > static int vidioc_vdec_querycap(struct file *file, void *priv,
> > 				struct v4l2_capability *cap)
> > {
> > 	struct mtk_vcodec_dec_ctx *ctx = fh_to_dec_ctx(priv);
> > 	struct device *dev = &ctx->dev->plat_dev->dev;
> > -	int platform_name = mtk_vcodec_dec_get_chip_name(priv);
> > 
> > 	strscpy(cap->driver, dev->driver->name, sizeof(cap->driver));
> > -	snprintf(cap->card, sizeof(cap->card), "MT%d video decoder",
> > platform_name);
> > +	snprintf(cap->card, sizeof(cap->card), "MT%d video decoder",
> > ctx->dev->chip_name);
> > 
> > 	return 0;
> > }
> > diff --git
> > a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv
> > .c
> > b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv
> > .c
> > index 0a89ce452ac3..f47c98faf068 100644
> > ---
> > a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv
> > .c
> > +++
> > b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv
> > .c
> > @@ -326,6 +326,26 @@ static const struct v4l2_file_operations
> > mtk_vcodec_fops = {
> > 	.mmap		= v4l2_m2m_fop_mmap,
> > };
> > 
> > +static void mtk_vcodec_dec_get_chip_name(struct mtk_vcodec_dec_dev
> > *vdec_dev)
> > +{
> > +	struct device *dev = &vdec_dev->plat_dev->dev;
> > +
> > +	if (of_device_is_compatible(dev->of_node, "mediatek,mt8173-
> > vcodec-dec"))
> > +		vdec_dev->chip_name = MTK_VDEC_MT8173;
> > +	else if (of_device_is_compatible(dev->of_node,
> > "mediatek,mt8183-vcodec-dec"))
> > +		vdec_dev->chip_name = MTK_VDEC_MT8183;
> > +	else if (of_device_is_compatible(dev->of_node,
> > "mediatek,mt8192-vcodec-dec"))
> > +		vdec_dev->chip_name = MTK_VDEC_MT8192;
> > +	else if (of_device_is_compatible(dev->of_node,
> > "mediatek,mt8195-vcodec-dec"))
> > +		vdec_dev->chip_name = MTK_VDEC_MT8195;
> > +	else if (of_device_is_compatible(dev->of_node,
> > "mediatek,mt8186-vcodec-dec"))
> > +		vdec_dev->chip_name = MTK_VDEC_MT8186;
> > +	else if (of_device_is_compatible(dev->of_node,
> > "mediatek,mt8188-vcodec-dec"))
> > +		vdec_dev->chip_name = MTK_VDEC_MT8188;
> > +	else
> > +		vdec_dev->chip_name = MTK_VDEC_INVAL;
> > +}
> > +
> > static int mtk_vcodec_probe(struct platform_device *pdev)
> > {
> > 	struct mtk_vcodec_dec_dev *dev;
> > @@ -341,6 +361,12 @@ static int mtk_vcodec_probe(struct
> > platform_device *pdev)
> > 	INIT_LIST_HEAD(&dev->ctx_list);
> > 	dev->plat_dev = pdev;
> > 
> > +	mtk_vcodec_dec_get_chip_name(dev);
> > +	if (dev->chip_name == MTK_VDEC_INVAL) {
> > +		dev_err(&pdev->dev, "Failed to get decoder chip name");
> > +		return -EINVAL;
> > +	}
> > +
> > 	dev->vdec_pdata = of_device_get_match_data(&pdev->dev);
> > 	if (!of_property_read_u32(pdev->dev.of_node, "mediatek,vpu",
> > 				  &rproc_phandle)) {
> > diff --git
> > a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv
> > .h
> > b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv
> > .h
> > index 7e36b2c69b7d..8f228ba9aa47 100644
> > ---
> > a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv
> > .h
> > +++
> > b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv
> > .h
> > @@ -18,6 +18,19 @@
> > #define IS_VDEC_LAT_ARCH(hw_arch) ((hw_arch) >=
> > MTK_VDEC_LAT_SINGLE_CORE)
> > #define IS_VDEC_INNER_RACING(capability) ((capability) &
> > MTK_VCODEC_INNER_RACING)
> > 
> > +/*
> > + * enum mtk_vcodec_dec_chip_name - Structure used to separate
> > different platform
> > + */
> 
> I don't feel like this comment is terribly helpful because it is
> pretty
> clear what the enum is about, I would just drop it.
> 
> > +enum mtk_vcodec_dec_chip_name {
> > +	MTK_VDEC_INVAL = 0,
> > +	MTK_VDEC_MT8173 = 8173,
> > +	MTK_VDEC_MT8183 = 8183,
> > +	MTK_VDEC_MT8186 = 8186,
> > +	MTK_VDEC_MT8188 = 8188,
> > +	MTK_VDEC_MT8192 = 8192,
> > +	MTK_VDEC_MT8195 = 8195,
> > +};
> > +
> > /*
> >  * enum mtk_vdec_format_types - Structure used to get supported
> >  *		  format types according to decoder capability
> > @@ -249,6 +262,8 @@ struct mtk_vcodec_dec_ctx {
> >  * @vdec_racing_info: record register value
> >  * @dec_racing_info_mutex: mutex lock used for inner racing mode
> >  * @dbgfs: debug log related information
> > + *
> > + * @chip_name: the chip name used to separate different platform
> 
> I wouldn't repeat chip name in the description and specify more
> concretely why we need to separate the platforms.
> 
> My suggestion:
> 
>   * @chip_name: used to distinguish platforms and select the correct
> codec configuration values.
> 
> >  */
> > struct mtk_vcodec_dec_dev {
> > 	struct v4l2_device v4l2_dev;
> > @@ -289,6 +304,8 @@ struct mtk_vcodec_dec_dev {
> > 	/* Protects access to vdec_racing_info data */
> > 	struct mutex dec_racing_info_mutex;
> > 	struct mtk_vcodec_dbgfs dbgfs;
> > +
> > +	enum mtk_vcodec_dec_chip_name chip_name;
> > };
> > 
> > static inline struct mtk_vcodec_dec_ctx *fh_to_dec_ctx(struct
> > v4l2_fh *fh)
> 
> Besides those small wording choices, the patch looks good.
> 
> So with these issues resolved:
> 
> Reviewed-by: Sebastian Fricke <sebastian.fricke@collabora.com>
> 
> Regards,
> Sebastian
> > -- 
> > 2.18.0
> >