[PATCH 3/3] media: mediatek: vcodec: Add driver to support 10bit

Yunfei Dong posted 3 patches 1 year, 2 months ago
There is a newer version of this series
[PATCH 3/3] media: mediatek: vcodec: Add driver to support 10bit
Posted by Yunfei Dong 1 year, 2 months ago
From: Mingjia Zhang <mingjia.zhang@mediatek.com>

Adding to support capture formats V4L2_PIX_FMT_MT2110T and
V4L2_PIX_FMT_MT2110R for 10bit playback. Need to get the size
of each plane again when user space setting syntax to get 10bit
information.

V4L2_PIX_FMT_MT2110T for AV1/VP9/HEVC.
V4L2_PIX_FMT_MT2110R for H264.

Signed-off-by: Mingjia Zhang <mingjia.zhang@mediatek.com>
Co-developed-by: Yunfei Dong <yunfei.dong@mediatek.com>
Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
---
 .../mediatek/vcodec/decoder/mtk_vcodec_dec.c  |  22 ++-
 .../vcodec/decoder/mtk_vcodec_dec_drv.h       |   5 +
 .../vcodec/decoder/mtk_vcodec_dec_stateless.c | 140 +++++++++++++++++-
 3 files changed, 163 insertions(+), 4 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 5acb7dff18f2..91ed576d6821 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
@@ -37,7 +37,9 @@ static bool mtk_vdec_get_cap_fmt(struct mtk_vcodec_dec_ctx *ctx, int format_inde
 {
 	const struct mtk_vcodec_dec_pdata *dec_pdata = ctx->dev->vdec_pdata;
 	const struct mtk_video_fmt *fmt;
+	struct mtk_q_data *q_data;
 	int num_frame_count = 0, i;
+	bool ret = false;
 
 	fmt = &dec_pdata->vdec_formats[format_index];
 	for (i = 0; i < *dec_pdata->num_formats; i++) {
@@ -47,10 +49,26 @@ static bool mtk_vdec_get_cap_fmt(struct mtk_vcodec_dec_ctx *ctx, int format_inde
 		num_frame_count++;
 	}
 
-	if (num_frame_count == 1 || fmt->fourcc == V4L2_PIX_FMT_MM21)
+	if (num_frame_count == 1 || (!ctx->is_10bit_bitstream && fmt->fourcc == V4L2_PIX_FMT_MM21))
 		return true;
 
-	return false;
+	q_data = &ctx->q_data[MTK_Q_DATA_SRC];
+	switch (q_data->fmt->fourcc) {
+	case V4L2_PIX_FMT_H264_SLICE:
+		if (ctx->is_10bit_bitstream && fmt->fourcc == V4L2_PIX_FMT_MT2110R)
+			ret = true;
+		break;
+	case V4L2_PIX_FMT_VP9_FRAME:
+	case V4L2_PIX_FMT_AV1_FRAME:
+	case V4L2_PIX_FMT_HEVC_SLICE:
+		if (ctx->is_10bit_bitstream && fmt->fourcc == V4L2_PIX_FMT_MT2110T)
+			ret = true;
+		break;
+	default:
+		break;
+	}
+
+	return ret;
 }
 
 static struct mtk_q_data *mtk_vdec_get_q_data(struct mtk_vcodec_dec_ctx *ctx,
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 c8b4374c5e6c..cd607e90fe9c 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
@@ -31,6 +31,7 @@ enum mtk_vdec_format_types {
 	MTK_VDEC_FORMAT_AV1_FRAME = 0x800,
 	MTK_VDEC_FORMAT_HEVC_FRAME = 0x1000,
 	MTK_VCODEC_INNER_RACING = 0x20000,
+	MTK_VDEC_IS_SUPPORT_10BIT = 0x40000,
 };
 
 /*
@@ -160,6 +161,8 @@ struct mtk_vcodec_dec_pdata {
  * @hw_id: hardware index used to identify different hardware.
  *
  * @msg_queue: msg queue used to store lat buffer information.
+ *
+ * @is_10bit_bitstream: set to true if it's 10bit bitstream
  */
 struct mtk_vcodec_dec_ctx {
 	enum mtk_instance_type type;
@@ -202,6 +205,8 @@ struct mtk_vcodec_dec_ctx {
 	int hw_id;
 
 	struct vdec_msg_queue msg_queue;
+
+	bool is_10bit_bitstream;
 };
 
 /**
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 99a84c7e1901..cef937fdf462 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
@@ -200,7 +200,7 @@ static const struct mtk_stateless_control mtk_stateless_controls[] = {
 
 #define NUM_CTRLS ARRAY_SIZE(mtk_stateless_controls)
 
-static struct mtk_video_fmt mtk_video_formats[7];
+static struct mtk_video_fmt mtk_video_formats[9];
 
 static struct mtk_video_fmt default_out_format;
 static struct mtk_video_fmt default_cap_format;
@@ -387,6 +387,134 @@ static int mtk_vdec_flush_decoder(struct mtk_vcodec_dec_ctx *ctx)
 	return vdec_if_decode(ctx, NULL, NULL, &res_chg);
 }
 
+static int mtk_vcodec_get_pic_info(struct mtk_vcodec_dec_ctx *ctx)
+{
+	struct mtk_q_data *q_data;
+	int ret = 0;
+
+	q_data = &ctx->q_data[MTK_Q_DATA_DST];
+	if (q_data->fmt->num_planes == 1) {
+		mtk_v4l2_vdec_err(ctx, "[%d]Error!! 10bit mode not support one plane", ctx->id);
+		return -EINVAL;
+	}
+
+	ctx->capture_fourcc = q_data->fmt->fourcc;
+	ret = vdec_if_get_param(ctx, GET_PARAM_PIC_INFO, &ctx->picinfo);
+	if (ret) {
+		mtk_v4l2_vdec_err(ctx, "[%d]Error!! Get GET_PARAM_PICTURE_INFO Fail", ctx->id);
+		return ret;
+	}
+
+	ctx->last_decoded_picinfo = ctx->picinfo;
+
+	q_data->sizeimage[0] = ctx->picinfo.fb_sz[0];
+	q_data->bytesperline[0] = ctx->picinfo.buf_w * 5 / 4;
+
+	q_data->sizeimage[1] = ctx->picinfo.fb_sz[1];
+	q_data->bytesperline[1] = ctx->picinfo.buf_w * 5 / 4;
+
+	q_data->coded_width = ctx->picinfo.buf_w;
+	q_data->coded_height = ctx->picinfo.buf_h;
+	mtk_v4l2_vdec_dbg(1, ctx, "[%d] wxh=%dx%d pic wxh=%dx%d sz[0]=0x%x sz[1]=0x%x",
+			  ctx->id, ctx->picinfo.buf_w, ctx->picinfo.buf_h,
+			  ctx->picinfo.pic_w, ctx->picinfo.pic_h,
+			  q_data->sizeimage[0], q_data->sizeimage[1]);
+
+	return ret;
+}
+
+static int mtk_vdec_s_ctrl(struct v4l2_ctrl *ctrl)
+{
+	struct mtk_vcodec_dec_ctx *ctx = ctrl_to_dec_ctx(ctrl);
+	struct v4l2_ctrl_h264_sps *h264;
+	struct v4l2_ctrl_hevc_sps *h265;
+	struct v4l2_ctrl_vp9_frame *frame;
+	struct v4l2_ctrl_av1_sequence *seq;
+	struct v4l2_ctrl *hdr_ctrl;
+	const struct mtk_vcodec_dec_pdata *dec_pdata = ctx->dev->vdec_pdata;
+	const struct mtk_video_fmt *fmt;
+	int i = 0, ret = 0;
+
+	hdr_ctrl = ctrl;
+	if (!hdr_ctrl || !hdr_ctrl->p_cur.p)
+		return -EINVAL;
+
+	switch (hdr_ctrl->id) {
+	case V4L2_CID_STATELESS_H264_SPS:
+		h264 = (struct v4l2_ctrl_h264_sps *)hdr_ctrl->p_new.p;
+		if (h264->bit_depth_chroma_minus8 == 2 && h264->bit_depth_luma_minus8 == 2) {
+			ctx->is_10bit_bitstream = true;
+		} else if (h264->bit_depth_chroma_minus8 != 0 &&
+			   h264->bit_depth_luma_minus8 != 0) {
+			mtk_v4l2_vdec_err(ctx, "H264: chroma_minus8:%d, luma_minus8:%d",
+					  h264->bit_depth_chroma_minus8,
+					  h264->bit_depth_luma_minus8);
+			return -EINVAL;
+		}
+		break;
+	case V4L2_CID_STATELESS_HEVC_SPS:
+		h265 = (struct v4l2_ctrl_hevc_sps *)hdr_ctrl->p_new.p;
+		if (h265->bit_depth_chroma_minus8 == 2 && h265->bit_depth_luma_minus8 == 2) {
+			ctx->is_10bit_bitstream = true;
+		} else if (h265->bit_depth_chroma_minus8 != 0 &&
+			   h265->bit_depth_luma_minus8 != 0) {
+			mtk_v4l2_vdec_err(ctx, "HEVC: chroma_minus8:%d, luma_minus8:%d",
+					  h265->bit_depth_chroma_minus8,
+					  h265->bit_depth_luma_minus8);
+			return -EINVAL;
+		}
+		break;
+	case V4L2_CID_STATELESS_VP9_FRAME:
+		frame = (struct v4l2_ctrl_vp9_frame *)hdr_ctrl->p_new.p;
+		if (frame->bit_depth == 10) {
+			ctx->is_10bit_bitstream = true;
+		} else if (frame->bit_depth != 8) {
+			mtk_v4l2_vdec_err(ctx, "VP9: bit_depth:%d", frame->bit_depth);
+			return -EINVAL;
+		}
+		break;
+	case V4L2_CID_STATELESS_AV1_SEQUENCE:
+		seq = (struct v4l2_ctrl_av1_sequence *)hdr_ctrl->p_new.p;
+		if (seq->bit_depth == 10) {
+			ctx->is_10bit_bitstream = true;
+		} else if (seq->bit_depth != 8) {
+			mtk_v4l2_vdec_err(ctx, "AV1: bit_depth:%d", seq->bit_depth);
+			return -EINVAL;
+		}
+		break;
+	default:
+		mtk_v4l2_vdec_err(ctx, "Not supported ctrl id: 0x%x\n", hdr_ctrl->id);
+		return -EINVAL;
+	}
+
+	if (!ctx->is_10bit_bitstream)
+		return ret;
+
+	for (i = 0; i < *dec_pdata->num_formats; i++) {
+		fmt = &dec_pdata->vdec_formats[i];
+		if (fmt->fourcc == V4L2_PIX_FMT_MT2110R &&
+		    hdr_ctrl->id == V4L2_CID_STATELESS_H264_SPS) {
+			ctx->q_data[MTK_Q_DATA_DST].fmt = fmt;
+			break;
+		}
+
+		if (fmt->fourcc == V4L2_PIX_FMT_MT2110T &&
+		    (hdr_ctrl->id == V4L2_CID_STATELESS_HEVC_SPS ||
+		    hdr_ctrl->id == V4L2_CID_STATELESS_VP9_FRAME ||
+		    hdr_ctrl->id == V4L2_CID_STATELESS_AV1_SEQUENCE)) {
+			ctx->q_data[MTK_Q_DATA_DST].fmt = fmt;
+			break;
+		}
+	}
+	ret = mtk_vcodec_get_pic_info(ctx);
+
+	return ret;
+}
+
+static const struct v4l2_ctrl_ops mtk_vcodec_dec_ctrl_ops = {
+	.s_ctrl = mtk_vdec_s_ctrl,
+};
+
 static int mtk_vcodec_dec_ctrls_setup(struct mtk_vcodec_dec_ctx *ctx)
 {
 	unsigned int i;
@@ -399,7 +527,7 @@ 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;
 		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,
@@ -466,6 +594,8 @@ static void mtk_vcodec_add_formats(unsigned int fourcc,
 		break;
 	case V4L2_PIX_FMT_MM21:
 	case V4L2_PIX_FMT_MT21C:
+	case V4L2_PIX_FMT_MT2110T:
+	case V4L2_PIX_FMT_MT2110R:
 		mtk_video_formats[count_formats].fourcc = fourcc;
 		mtk_video_formats[count_formats].type = MTK_FMT_FRAME;
 		mtk_video_formats[count_formats].num_planes = 2;
@@ -491,6 +621,12 @@ static void mtk_vcodec_get_supported_formats(struct mtk_vcodec_dec_ctx *ctx)
 		mtk_vcodec_add_formats(V4L2_PIX_FMT_MT21C, ctx);
 		cap_format_count++;
 	}
+	if (ctx->dev->dec_capability & MTK_VDEC_IS_SUPPORT_10BIT) {
+		mtk_vcodec_add_formats(V4L2_PIX_FMT_MT2110T, ctx);
+		cap_format_count++;
+		mtk_vcodec_add_formats(V4L2_PIX_FMT_MT2110R, ctx);
+		cap_format_count++;
+	}
 	if (ctx->dev->dec_capability & MTK_VDEC_FORMAT_MM21) {
 		mtk_vcodec_add_formats(V4L2_PIX_FMT_MM21, ctx);
 		cap_format_count++;
-- 
2.18.0
Re: [PATCH 3/3] media: mediatek: vcodec: Add driver to support 10bit
Posted by Nicolas Dufresne 1 year, 2 months ago
Hi Yunfei,

this is a partial drive by review, I'll do more testing and more review soon.

Le mardi 11 juillet 2023 à 20:57 +0800, Yunfei Dong a écrit :
> From: Mingjia Zhang <mingjia.zhang@mediatek.com>
> 
> Adding to support capture formats V4L2_PIX_FMT_MT2110T and
> V4L2_PIX_FMT_MT2110R for 10bit playback. Need to get the size
> of each plane again when user space setting syntax to get 10bit
> information.
> 
> V4L2_PIX_FMT_MT2110T for AV1/VP9/HEVC.
> V4L2_PIX_FMT_MT2110R for H264.
> 
> Signed-off-by: Mingjia Zhang <mingjia.zhang@mediatek.com>
> Co-developed-by: Yunfei Dong <yunfei.dong@mediatek.com>
> Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
> ---
>  .../mediatek/vcodec/decoder/mtk_vcodec_dec.c  |  22 ++-
>  .../vcodec/decoder/mtk_vcodec_dec_drv.h       |   5 +
>  .../vcodec/decoder/mtk_vcodec_dec_stateless.c | 140 +++++++++++++++++-
>  3 files changed, 163 insertions(+), 4 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 5acb7dff18f2..91ed576d6821 100644
> --- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
> +++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
> @@ -37,7 +37,9 @@ static bool mtk_vdec_get_cap_fmt(struct mtk_vcodec_dec_ctx *ctx, int format_inde
>  {
>  	const struct mtk_vcodec_dec_pdata *dec_pdata = ctx->dev->vdec_pdata;
>  	const struct mtk_video_fmt *fmt;
> +	struct mtk_q_data *q_data;
>  	int num_frame_count = 0, i;
> +	bool ret = false;
>  
>  	fmt = &dec_pdata->vdec_formats[format_index];
>  	for (i = 0; i < *dec_pdata->num_formats; i++) {
> @@ -47,10 +49,26 @@ static bool mtk_vdec_get_cap_fmt(struct mtk_vcodec_dec_ctx *ctx, int format_inde
>  		num_frame_count++;
>  	}
>  
> -	if (num_frame_count == 1 || fmt->fourcc == V4L2_PIX_FMT_MM21)
> +	if (num_frame_count == 1 || (!ctx->is_10bit_bitstream && fmt->fourcc == V4L2_PIX_FMT_MM21))
>  		return true;
>  
> -	return false;
> +	q_data = &ctx->q_data[MTK_Q_DATA_SRC];
> +	switch (q_data->fmt->fourcc) {
> +	case V4L2_PIX_FMT_H264_SLICE:
> +		if (ctx->is_10bit_bitstream && fmt->fourcc == V4L2_PIX_FMT_MT2110R)
> +			ret = true;
> +		break;
> +	case V4L2_PIX_FMT_VP9_FRAME:
> +	case V4L2_PIX_FMT_AV1_FRAME:
> +	case V4L2_PIX_FMT_HEVC_SLICE:
> +		if (ctx->is_10bit_bitstream && fmt->fourcc == V4L2_PIX_FMT_MT2110T)
> +			ret = true;
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	return ret;
>  }
>  
>  static struct mtk_q_data *mtk_vdec_get_q_data(struct mtk_vcodec_dec_ctx *ctx,
> 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 c8b4374c5e6c..cd607e90fe9c 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
> @@ -31,6 +31,7 @@ enum mtk_vdec_format_types {
>  	MTK_VDEC_FORMAT_AV1_FRAME = 0x800,
>  	MTK_VDEC_FORMAT_HEVC_FRAME = 0x1000,
>  	MTK_VCODEC_INNER_RACING = 0x20000,
> +	MTK_VDEC_IS_SUPPORT_10BIT = 0x40000,
>  };
>  
>  /*
> @@ -160,6 +161,8 @@ struct mtk_vcodec_dec_pdata {
>   * @hw_id: hardware index used to identify different hardware.
>   *
>   * @msg_queue: msg queue used to store lat buffer information.
> + *
> + * @is_10bit_bitstream: set to true if it's 10bit bitstream
>   */
>  struct mtk_vcodec_dec_ctx {
>  	enum mtk_instance_type type;
> @@ -202,6 +205,8 @@ struct mtk_vcodec_dec_ctx {
>  	int hw_id;
>  
>  	struct vdec_msg_queue msg_queue;
> +
> +	bool is_10bit_bitstream;
>  };
>  
>  /**
> 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 99a84c7e1901..cef937fdf462 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
> @@ -200,7 +200,7 @@ static const struct mtk_stateless_control mtk_stateless_controls[] = {
>  
>  #define NUM_CTRLS ARRAY_SIZE(mtk_stateless_controls)
>  
> -static struct mtk_video_fmt mtk_video_formats[7];
> +static struct mtk_video_fmt mtk_video_formats[9];
>  
>  static struct mtk_video_fmt default_out_format;
>  static struct mtk_video_fmt default_cap_format;
> @@ -387,6 +387,134 @@ static int mtk_vdec_flush_decoder(struct mtk_vcodec_dec_ctx *ctx)
>  	return vdec_if_decode(ctx, NULL, NULL, &res_chg);
>  }
>  
> +static int mtk_vcodec_get_pic_info(struct mtk_vcodec_dec_ctx *ctx)
> +{
> +	struct mtk_q_data *q_data;
> +	int ret = 0;
> +
> +	q_data = &ctx->q_data[MTK_Q_DATA_DST];
> +	if (q_data->fmt->num_planes == 1) {
> +		mtk_v4l2_vdec_err(ctx, "[%d]Error!! 10bit mode not support one plane", ctx->id);
> +		return -EINVAL;
> +	}
> +
> +	ctx->capture_fourcc = q_data->fmt->fourcc;
> +	ret = vdec_if_get_param(ctx, GET_PARAM_PIC_INFO, &ctx->picinfo);
> +	if (ret) {
> +		mtk_v4l2_vdec_err(ctx, "[%d]Error!! Get GET_PARAM_PICTURE_INFO Fail", ctx->id);
> +		return ret;
> +	}
> +
> +	ctx->last_decoded_picinfo = ctx->picinfo;
> +
> +	q_data->sizeimage[0] = ctx->picinfo.fb_sz[0];
> +	q_data->bytesperline[0] = ctx->picinfo.buf_w * 5 / 4;
> +
> +	q_data->sizeimage[1] = ctx->picinfo.fb_sz[1];
> +	q_data->bytesperline[1] = ctx->picinfo.buf_w * 5 / 4;
> +
> +	q_data->coded_width = ctx->picinfo.buf_w;
> +	q_data->coded_height = ctx->picinfo.buf_h;
> +	mtk_v4l2_vdec_dbg(1, ctx, "[%d] wxh=%dx%d pic wxh=%dx%d sz[0]=0x%x sz[1]=0x%x",
> +			  ctx->id, ctx->picinfo.buf_w, ctx->picinfo.buf_h,
> +			  ctx->picinfo.pic_w, ctx->picinfo.pic_h,
> +			  q_data->sizeimage[0], q_data->sizeimage[1]);
> +
> +	return ret;
> +}
> +
> +static int mtk_vdec_s_ctrl(struct v4l2_ctrl *ctrl)
> +{
> +	struct mtk_vcodec_dec_ctx *ctx = ctrl_to_dec_ctx(ctrl);
> +	struct v4l2_ctrl_h264_sps *h264;
> +	struct v4l2_ctrl_hevc_sps *h265;
> +	struct v4l2_ctrl_vp9_frame *frame;
> +	struct v4l2_ctrl_av1_sequence *seq;
> +	struct v4l2_ctrl *hdr_ctrl;
> +	const struct mtk_vcodec_dec_pdata *dec_pdata = ctx->dev->vdec_pdata;
> +	const struct mtk_video_fmt *fmt;
> +	int i = 0, ret = 0;
> +
> +	hdr_ctrl = ctrl;
> +	if (!hdr_ctrl || !hdr_ctrl->p_cur.p)
> +		return -EINVAL;

There is a null check for hdr_ctrl->p_cur.p ...

> +
> +	switch (hdr_ctrl->id) {
> +	case V4L2_CID_STATELESS_H264_SPS:
> +		h264 = (struct v4l2_ctrl_h264_sps *)hdr_ctrl->p_new.p;

But you are using hdr_ctrl->p_new.p. I don't know if the checks are absolutly
required, I'll have a look.

> +		if (h264->bit_depth_chroma_minus8 == 2 && h264->bit_depth_luma_minus8 == 2) {

In the conformance streams, there is a 9bit luma file, which on all decoders
I've tried decodes fine inside a 10bit image.

> +			ctx->is_10bit_bitstream = true;
> +		} else if (h264->bit_depth_chroma_minus8 != 0 &&
> +			   h264->bit_depth_luma_minus8 != 0) {
> +			mtk_v4l2_vdec_err(ctx, "H264: chroma_minus8:%d, luma_minus8:%d",
> +					  h264->bit_depth_chroma_minus8,
> +					  h264->bit_depth_luma_minus8);
> +			return -EINVAL;
> +		}
> +		break;
> +	case V4L2_CID_STATELESS_HEVC_SPS:
> +		h265 = (struct v4l2_ctrl_hevc_sps *)hdr_ctrl->p_new.p;
> +		if (h265->bit_depth_chroma_minus8 == 2 && h265->bit_depth_luma_minus8 == 2) {
> +			ctx->is_10bit_bitstream = true;
> +		} else if (h265->bit_depth_chroma_minus8 != 0 &&
> +			   h265->bit_depth_luma_minus8 != 0) {
> +			mtk_v4l2_vdec_err(ctx, "HEVC: chroma_minus8:%d, luma_minus8:%d",
> +					  h265->bit_depth_chroma_minus8,
> +					  h265->bit_depth_luma_minus8);
> +			return -EINVAL;
> +		}
> +		break;
> +	case V4L2_CID_STATELESS_VP9_FRAME:
> +		frame = (struct v4l2_ctrl_vp9_frame *)hdr_ctrl->p_new.p;
> +		if (frame->bit_depth == 10) {
> +			ctx->is_10bit_bitstream = true;
> +		} else if (frame->bit_depth != 8) {
> +			mtk_v4l2_vdec_err(ctx, "VP9: bit_depth:%d", frame->bit_depth);
> +			return -EINVAL;
> +		}
> +		break;
> +	case V4L2_CID_STATELESS_AV1_SEQUENCE:
> +		seq = (struct v4l2_ctrl_av1_sequence *)hdr_ctrl->p_new.p;
> +		if (seq->bit_depth == 10) {
> +			ctx->is_10bit_bitstream = true;
> +		} else if (seq->bit_depth != 8) {
> +			mtk_v4l2_vdec_err(ctx, "AV1: bit_depth:%d", seq->bit_depth);
> +			return -EINVAL;
> +		}
> +		break;
> +	default:
> +		mtk_v4l2_vdec_err(ctx, "Not supported ctrl id: 0x%x\n", hdr_ctrl->id);
> +		return -EINVAL;

Haven't tested, but it feels like this will prevent setting the PPS among many
other controls. This should likely not be an error, and should return 0.

> +	}
> +
> +	if (!ctx->is_10bit_bitstream)
> +		return ret;
> +
> +	for (i = 0; i < *dec_pdata->num_formats; i++) {
> +		fmt = &dec_pdata->vdec_formats[i];
> +		if (fmt->fourcc == V4L2_PIX_FMT_MT2110R &&
> +		    hdr_ctrl->id == V4L2_CID_STATELESS_H264_SPS) {
> +			ctx->q_data[MTK_Q_DATA_DST].fmt = fmt;
> +			break;
> +		}
> +
> +		if (fmt->fourcc == V4L2_PIX_FMT_MT2110T &&
> +		    (hdr_ctrl->id == V4L2_CID_STATELESS_HEVC_SPS ||
> +		    hdr_ctrl->id == V4L2_CID_STATELESS_VP9_FRAME ||
> +		    hdr_ctrl->id == V4L2_CID_STATELESS_AV1_SEQUENCE)) {
> +			ctx->q_data[MTK_Q_DATA_DST].fmt = fmt;
> +			break;
> +		}
> +	}
> +	ret = mtk_vcodec_get_pic_info(ctx);
> +
> +	return ret;
> +}
> +
> +static const struct v4l2_ctrl_ops mtk_vcodec_dec_ctrl_ops = {
> +	.s_ctrl = mtk_vdec_s_ctrl,
> +};
> +
>  static int mtk_vcodec_dec_ctrls_setup(struct mtk_vcodec_dec_ctx *ctx)
>  {
>  	unsigned int i;
> @@ -399,7 +527,7 @@ 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;
>  		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,
> @@ -466,6 +594,8 @@ static void mtk_vcodec_add_formats(unsigned int fourcc,
>  		break;
>  	case V4L2_PIX_FMT_MM21:
>  	case V4L2_PIX_FMT_MT21C:
> +	case V4L2_PIX_FMT_MT2110T:
> +	case V4L2_PIX_FMT_MT2110R:
>  		mtk_video_formats[count_formats].fourcc = fourcc;
>  		mtk_video_formats[count_formats].type = MTK_FMT_FRAME;
>  		mtk_video_formats[count_formats].num_planes = 2;
> @@ -491,6 +621,12 @@ static void mtk_vcodec_get_supported_formats(struct mtk_vcodec_dec_ctx *ctx)
>  		mtk_vcodec_add_formats(V4L2_PIX_FMT_MT21C, ctx);
>  		cap_format_count++;
>  	}
> +	if (ctx->dev->dec_capability & MTK_VDEC_IS_SUPPORT_10BIT) {
> +		mtk_vcodec_add_formats(V4L2_PIX_FMT_MT2110T, ctx);
> +		cap_format_count++;
> +		mtk_vcodec_add_formats(V4L2_PIX_FMT_MT2110R, ctx);
> +		cap_format_count++;
> +	}
>  	if (ctx->dev->dec_capability & MTK_VDEC_FORMAT_MM21) {
>  		mtk_vcodec_add_formats(V4L2_PIX_FMT_MM21, ctx);
>  		cap_format_count++;
Re: [PATCH 3/3] media: mediatek: vcodec: Add driver to support 10bit
Posted by Yunfei Dong (董云飞) 1 year, 2 months ago
Hi Nicolas,

Thanks for your suggestion.

On Tue, 2023-07-11 at 12:53 -0400, Nicolas Dufresne wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  Hi Yunfei,
> 
> this is a partial drive by review, I'll do more testing and more
> review soon.
> 
> Le mardi 11 juillet 2023 à 20:57 +0800, Yunfei Dong a écrit :
> > From: Mingjia Zhang <mingjia.zhang@mediatek.com>
> > 
> > Adding to support capture formats V4L2_PIX_FMT_MT2110T and
> > V4L2_PIX_FMT_MT2110R for 10bit playback. Need to get the size
> > of each plane again when user space setting syntax to get 10bit
> > information.
> > 
> > V4L2_PIX_FMT_MT2110T for AV1/VP9/HEVC.
> > V4L2_PIX_FMT_MT2110R for H264.
> > 
> > Signed-off-by: Mingjia Zhang <mingjia.zhang@mediatek.com>
> > Co-developed-by: Yunfei Dong <yunfei.dong@mediatek.com>
> > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
> > ---
> >  .../mediatek/vcodec/decoder/mtk_vcodec_dec.c  |  22 ++-
> >  .../vcodec/decoder/mtk_vcodec_dec_drv.h       |   5 +
> >  .../vcodec/decoder/mtk_vcodec_dec_stateless.c | 140
> +++++++++++++++++-
> >  3 files changed, 163 insertions(+), 4 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 5acb7dff18f2..91ed576d6821 100644
> > ---
> a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
> > +++
> b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
> > @@ -37,7 +37,9 @@ static bool mtk_vdec_get_cap_fmt(struct
> mtk_vcodec_dec_ctx *ctx, int format_inde
> >  {
> >  const struct mtk_vcodec_dec_pdata *dec_pdata = ctx->dev-
> >vdec_pdata;
> >  const struct mtk_video_fmt *fmt;
> > +struct mtk_q_data *q_data;
> >  int num_frame_count = 0, i;
> > +bool ret = false;
> >  
> >  fmt = &dec_pdata->vdec_formats[format_index];
> >  for (i = 0; i < *dec_pdata->num_formats; i++) {
> > @@ -47,10 +49,26 @@ static bool mtk_vdec_get_cap_fmt(struct
> mtk_vcodec_dec_ctx *ctx, int format_inde
> >  num_frame_count++;
> >  }
> >  
> > -if (num_frame_count == 1 || fmt->fourcc == V4L2_PIX_FMT_MM21)
> > +if (num_frame_count == 1 || (!ctx->is_10bit_bitstream && fmt-
> >fourcc == V4L2_PIX_FMT_MM21))
> >  return true;
> >  
> > -return false;
> > +q_data = &ctx->q_data[MTK_Q_DATA_SRC];
> > +switch (q_data->fmt->fourcc) {
> > +case V4L2_PIX_FMT_H264_SLICE:
> > +if (ctx->is_10bit_bitstream && fmt->fourcc ==
> V4L2_PIX_FMT_MT2110R)
> > +ret = true;
> > +break;
> > +case V4L2_PIX_FMT_VP9_FRAME:
> > +case V4L2_PIX_FMT_AV1_FRAME:
> > +case V4L2_PIX_FMT_HEVC_SLICE:
> > +if (ctx->is_10bit_bitstream && fmt->fourcc ==
> V4L2_PIX_FMT_MT2110T)
> > +ret = true;
> > +break;
> > +default:
> > +break;
> > +}
> > +
> > +return ret;
> >  }
> >  
> >  static struct mtk_q_data *mtk_vdec_get_q_data(struct
> mtk_vcodec_dec_ctx *ctx,
> > 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 c8b4374c5e6c..cd607e90fe9c 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
> > @@ -31,6 +31,7 @@ enum mtk_vdec_format_types {
> >  MTK_VDEC_FORMAT_AV1_FRAME = 0x800,
> >  MTK_VDEC_FORMAT_HEVC_FRAME = 0x1000,
> >  MTK_VCODEC_INNER_RACING = 0x20000,
> > +MTK_VDEC_IS_SUPPORT_10BIT = 0x40000,
> >  };
> >  
> >  /*
> > @@ -160,6 +161,8 @@ struct mtk_vcodec_dec_pdata {
> >   * @hw_id: hardware index used to identify different hardware.
> >   *
> >   * @msg_queue: msg queue used to store lat buffer information.
> > + *
> > + * @is_10bit_bitstream: set to true if it's 10bit bitstream
> >   */
> >  struct mtk_vcodec_dec_ctx {
> >  enum mtk_instance_type type;
> > @@ -202,6 +205,8 @@ struct mtk_vcodec_dec_ctx {
> >  int hw_id;
> >  
> >  struct vdec_msg_queue msg_queue;
> > +
> > +bool is_10bit_bitstream;
> >  };
> >  
> >  /**
> > diff --git
> a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_state
> less.c
> b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_state
> less.c
> > index 99a84c7e1901..cef937fdf462 100644
> > ---
> a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_state
> less.c
> > +++
> b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_state
> less.c
> > @@ -200,7 +200,7 @@ static const struct mtk_stateless_control
> mtk_stateless_controls[] = {
> >  
> >  #define NUM_CTRLS ARRAY_SIZE(mtk_stateless_controls)
> >  
> > -static struct mtk_video_fmt mtk_video_formats[7];
> > +static struct mtk_video_fmt mtk_video_formats[9];
> >  
> >  static struct mtk_video_fmt default_out_format;
> >  static struct mtk_video_fmt default_cap_format;
> > @@ -387,6 +387,134 @@ static int mtk_vdec_flush_decoder(struct
> mtk_vcodec_dec_ctx *ctx)
> >  return vdec_if_decode(ctx, NULL, NULL, &res_chg);
> >  }
> >  
> > +static int mtk_vcodec_get_pic_info(struct mtk_vcodec_dec_ctx *ctx)
> > +{
> > +struct mtk_q_data *q_data;
> > +int ret = 0;
> > +
> > +q_data = &ctx->q_data[MTK_Q_DATA_DST];
> > +if (q_data->fmt->num_planes == 1) {
> > +mtk_v4l2_vdec_err(ctx, "[%d]Error!! 10bit mode not support one
> plane", ctx->id);
> > +return -EINVAL;
> > +}
> > +
> > +ctx->capture_fourcc = q_data->fmt->fourcc;
> > +ret = vdec_if_get_param(ctx, GET_PARAM_PIC_INFO, &ctx->picinfo);
> > +if (ret) {
> > +mtk_v4l2_vdec_err(ctx, "[%d]Error!! Get GET_PARAM_PICTURE_INFO
> Fail", ctx->id);
> > +return ret;
> > +}
> > +
> > +ctx->last_decoded_picinfo = ctx->picinfo;
> > +
> > +q_data->sizeimage[0] = ctx->picinfo.fb_sz[0];
> > +q_data->bytesperline[0] = ctx->picinfo.buf_w * 5 / 4;
> > +
> > +q_data->sizeimage[1] = ctx->picinfo.fb_sz[1];
> > +q_data->bytesperline[1] = ctx->picinfo.buf_w * 5 / 4;
> > +
> > +q_data->coded_width = ctx->picinfo.buf_w;
> > +q_data->coded_height = ctx->picinfo.buf_h;
> > +mtk_v4l2_vdec_dbg(1, ctx, "[%d] wxh=%dx%d pic wxh=%dx%d sz[0]=0x%x
> sz[1]=0x%x",
> > +  ctx->id, ctx->picinfo.buf_w, ctx->picinfo.buf_h,
> > +  ctx->picinfo.pic_w, ctx->picinfo.pic_h,
> > +  q_data->sizeimage[0], q_data->sizeimage[1]);
> > +
> > +return ret;
> > +}
> > +
> > +static int mtk_vdec_s_ctrl(struct v4l2_ctrl *ctrl)
> > +{
> > +struct mtk_vcodec_dec_ctx *ctx = ctrl_to_dec_ctx(ctrl);
> > +struct v4l2_ctrl_h264_sps *h264;
> > +struct v4l2_ctrl_hevc_sps *h265;
> > +struct v4l2_ctrl_vp9_frame *frame;
> > +struct v4l2_ctrl_av1_sequence *seq;
> > +struct v4l2_ctrl *hdr_ctrl;
> > +const struct mtk_vcodec_dec_pdata *dec_pdata = ctx->dev-
> >vdec_pdata;
> > +const struct mtk_video_fmt *fmt;
> > +int i = 0, ret = 0;
> > +
> > +hdr_ctrl = ctrl;
> > +if (!hdr_ctrl || !hdr_ctrl->p_cur.p)
> > +return -EINVAL;
> 
> There is a null check for hdr_ctrl->p_cur.p ...
> 
Should be hdr_ctrl->p_new.p.
> > +
> > +switch (hdr_ctrl->id) {
> > +case V4L2_CID_STATELESS_H264_SPS:
> > +h264 = (struct v4l2_ctrl_h264_sps *)hdr_ctrl->p_new.p;
> 
> But you are using hdr_ctrl->p_new.p. I don't know if the checks are
> absolutly
> required, I'll have a look.
> 
> > +if (h264->bit_depth_chroma_minus8 == 2 && h264-
> >bit_depth_luma_minus8 == 2) {
> 
> In the conformance streams, there is a 9bit luma file, which on all
> decoders
> I've tried decodes fine inside a 10bit image.
> 
Confirmed with our designer, the hardware not support 9bit bitstream. 
M
aybe test pass randomly.

> > +ctx->is_10bit_bitstream = true;
> > +} else if (h264->bit_depth_chroma_minus8 != 0 &&
> > +   h264->bit_depth_luma_minus8 != 0) {
> > +mtk_v4l2_vdec_err(ctx, "H264: chroma_minus8:%d, luma_minus8:%d",
> > +  h264->bit_depth_chroma_minus8,
> > +  h264->bit_depth_luma_minus8);
> > +return -EINVAL;
> > +}
> > +break;
> > +case V4L2_CID_STATELESS_HEVC_SPS:
> > +h265 = (struct v4l2_ctrl_hevc_sps *)hdr_ctrl->p_new.p;
> > +if (h265->bit_depth_chroma_minus8 == 2 && h265-
> >bit_depth_luma_minus8 == 2) {
> > +ctx->is_10bit_bitstream = true;
> > +} else if (h265->bit_depth_chroma_minus8 != 0 &&
> > +   h265->bit_depth_luma_minus8 != 0) {
> > +mtk_v4l2_vdec_err(ctx, "HEVC: chroma_minus8:%d, luma_minus8:%d",
> > +  h265->bit_depth_chroma_minus8,
> > +  h265->bit_depth_luma_minus8);
> > +return -EINVAL;
> > +}
> > +break;
> > +case V4L2_CID_STATELESS_VP9_FRAME:
> > +frame = (struct v4l2_ctrl_vp9_frame *)hdr_ctrl->p_new.p;
> > +if (frame->bit_depth == 10) {
> > +ctx->is_10bit_bitstream = true;
> > +} else if (frame->bit_depth != 8) {
> > +mtk_v4l2_vdec_err(ctx, "VP9: bit_depth:%d", frame->bit_depth);
> > +return -EINVAL;
> > +}
> > +break;
> > +case V4L2_CID_STATELESS_AV1_SEQUENCE:
> > +seq = (struct v4l2_ctrl_av1_sequence *)hdr_ctrl->p_new.p;
> > +if (seq->bit_depth == 10) {
> > +ctx->is_10bit_bitstream = true;
> > +} else if (seq->bit_depth != 8) {
> > +mtk_v4l2_vdec_err(ctx, "AV1: bit_depth:%d", seq->bit_depth);
> > +return -EINVAL;
> > +}
> > +break;
> > +default:
> > +mtk_v4l2_vdec_err(ctx, "Not supported ctrl id: 0x%x\n", hdr_ctrl-
> >id);
> > +return -EINVAL;
> 
> Haven't tested, but it feels like this will prevent setting the PPS
> among many
> other controls. This should likely not be an error, and should return
> 0.
> 
You are right, need to remove 'return -EINVAL' with 'return ret';

Best Regards,
Yunfei Dong
> > +}
> > +
> > +if (!ctx->is_10bit_bitstream)
> > +return ret;
> > +
> > +for (i = 0; i < *dec_pdata->num_formats; i++) {
> > +fmt = &dec_pdata->vdec_formats[i];
> > +if (fmt->fourcc == V4L2_PIX_FMT_MT2110R &&
> > +    hdr_ctrl->id == V4L2_CID_STATELESS_H264_SPS) {
> > +ctx->q_data[MTK_Q_DATA_DST].fmt = fmt;
> > +break;
> > +}
> > +
> > +if (fmt->fourcc == V4L2_PIX_FMT_MT2110T &&
> > +    (hdr_ctrl->id == V4L2_CID_STATELESS_HEVC_SPS ||
> > +    hdr_ctrl->id == V4L2_CID_STATELESS_VP9_FRAME ||
> > +    hdr_ctrl->id == V4L2_CID_STATELESS_AV1_SEQUENCE)) {
> > +ctx->q_data[MTK_Q_DATA_DST].fmt = fmt;
> > +break;
> > +}
> > +}
> > +ret = mtk_vcodec_get_pic_info(ctx);
> > +
> > +return ret;
> > +}
> > +
> > +static const struct v4l2_ctrl_ops mtk_vcodec_dec_ctrl_ops = {
> > +.s_ctrl = mtk_vdec_s_ctrl,
> > +};
> > +
> >  static int mtk_vcodec_dec_ctrls_setup(struct mtk_vcodec_dec_ctx
> *ctx)
> >  {
> >  unsigned int i;
> > @@ -399,7 +527,7 @@ 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;
> >  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,
> > @@ -466,6 +594,8 @@ static void mtk_vcodec_add_formats(unsigned int
> fourcc,
> >  break;
> >  case V4L2_PIX_FMT_MM21:
> >  case V4L2_PIX_FMT_MT21C:
> > +case V4L2_PIX_FMT_MT2110T:
> > +case V4L2_PIX_FMT_MT2110R:
> >  mtk_video_formats[count_formats].fourcc = fourcc;
> >  mtk_video_formats[count_formats].type = MTK_FMT_FRAME;
> >  mtk_video_formats[count_formats].num_planes = 2;
> > @@ -491,6 +621,12 @@ static void
> mtk_vcodec_get_supported_formats(struct mtk_vcodec_dec_ctx *ctx)
> >  mtk_vcodec_add_formats(V4L2_PIX_FMT_MT21C, ctx);
> >  cap_format_count++;
> >  }
> > +if (ctx->dev->dec_capability & MTK_VDEC_IS_SUPPORT_10BIT) {
> > +mtk_vcodec_add_formats(V4L2_PIX_FMT_MT2110T, ctx);
> > +cap_format_count++;
> > +mtk_vcodec_add_formats(V4L2_PIX_FMT_MT2110R, ctx);
> > +cap_format_count++;
> > +}
> >  if (ctx->dev->dec_capability & MTK_VDEC_FORMAT_MM21) {
> >  mtk_vcodec_add_formats(V4L2_PIX_FMT_MM21, ctx);
> >  cap_format_count++;
> 
Re: [PATCH 3/3] media: mediatek: vcodec: Add driver to support 10bit
Posted by Nicolas Dufresne 1 year, 2 months ago
Le mardi 11 juillet 2023 à 20:57 +0800, Yunfei Dong a écrit :
> From: Mingjia Zhang <mingjia.zhang@mediatek.com>
> 
> Adding to support capture formats V4L2_PIX_FMT_MT2110T and
> V4L2_PIX_FMT_MT2110R for 10bit playback. Need to get the size
> of each plane again when user space setting syntax to get 10bit
> information.
> 
> V4L2_PIX_FMT_MT2110T for AV1/VP9/HEVC.
> V4L2_PIX_FMT_MT2110R for H264.
> 
> Signed-off-by: Mingjia Zhang <mingjia.zhang@mediatek.com>
> Co-developed-by: Yunfei Dong <yunfei.dong@mediatek.com>
> Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
> ---
>  .../mediatek/vcodec/decoder/mtk_vcodec_dec.c  |  22 ++-
>  .../vcodec/decoder/mtk_vcodec_dec_drv.h       |   5 +
>  .../vcodec/decoder/mtk_vcodec_dec_stateless.c | 140 +++++++++++++++++-
>  3 files changed, 163 insertions(+), 4 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 5acb7dff18f2..91ed576d6821 100644
> --- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
> +++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
> @@ -37,7 +37,9 @@ static bool mtk_vdec_get_cap_fmt(struct mtk_vcodec_dec_ctx *ctx, int format_inde
>  {
>  	const struct mtk_vcodec_dec_pdata *dec_pdata = ctx->dev->vdec_pdata;
>  	const struct mtk_video_fmt *fmt;
> +	struct mtk_q_data *q_data;
>  	int num_frame_count = 0, i;
> +	bool ret = false;
>  
>  	fmt = &dec_pdata->vdec_formats[format_index];
>  	for (i = 0; i < *dec_pdata->num_formats; i++) {
> @@ -47,10 +49,26 @@ static bool mtk_vdec_get_cap_fmt(struct mtk_vcodec_dec_ctx *ctx, int format_inde
>  		num_frame_count++;
>  	}
>  
> -	if (num_frame_count == 1 || fmt->fourcc == V4L2_PIX_FMT_MM21)
> +	if (num_frame_count == 1 || (!ctx->is_10bit_bitstream && fmt->fourcc == V4L2_PIX_FMT_MM21))
>  		return true;
>  
> -	return false;
> +	q_data = &ctx->q_data[MTK_Q_DATA_SRC];
> +	switch (q_data->fmt->fourcc) {
> +	case V4L2_PIX_FMT_H264_SLICE:
> +		if (ctx->is_10bit_bitstream && fmt->fourcc == V4L2_PIX_FMT_MT2110R)
> +			ret = true;
> +		break;
> +	case V4L2_PIX_FMT_VP9_FRAME:
> +	case V4L2_PIX_FMT_AV1_FRAME:
> +	case V4L2_PIX_FMT_HEVC_SLICE:
> +		if (ctx->is_10bit_bitstream && fmt->fourcc == V4L2_PIX_FMT_MT2110T)
> +			ret = true;
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	return ret;
>  }
>  
>  static struct mtk_q_data *mtk_vdec_get_q_data(struct mtk_vcodec_dec_ctx *ctx,
> 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 c8b4374c5e6c..cd607e90fe9c 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
> @@ -31,6 +31,7 @@ enum mtk_vdec_format_types {
>  	MTK_VDEC_FORMAT_AV1_FRAME = 0x800,
>  	MTK_VDEC_FORMAT_HEVC_FRAME = 0x1000,
>  	MTK_VCODEC_INNER_RACING = 0x20000,
> +	MTK_VDEC_IS_SUPPORT_10BIT = 0x40000,
>  };
>  
>  /*
> @@ -160,6 +161,8 @@ struct mtk_vcodec_dec_pdata {
>   * @hw_id: hardware index used to identify different hardware.
>   *
>   * @msg_queue: msg queue used to store lat buffer information.
> + *
> + * @is_10bit_bitstream: set to true if it's 10bit bitstream
>   */
>  struct mtk_vcodec_dec_ctx {
>  	enum mtk_instance_type type;
> @@ -202,6 +205,8 @@ struct mtk_vcodec_dec_ctx {
>  	int hw_id;
>  
>  	struct vdec_msg_queue msg_queue;
> +
> +	bool is_10bit_bitstream;
>  };
>  
>  /**
> 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 99a84c7e1901..cef937fdf462 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
> @@ -200,7 +200,7 @@ static const struct mtk_stateless_control mtk_stateless_controls[] = {
>  
>  #define NUM_CTRLS ARRAY_SIZE(mtk_stateless_controls)
>  
> -static struct mtk_video_fmt mtk_video_formats[7];
> +static struct mtk_video_fmt mtk_video_formats[9];
>  
>  static struct mtk_video_fmt default_out_format;
>  static struct mtk_video_fmt default_cap_format;
> @@ -387,6 +387,134 @@ static int mtk_vdec_flush_decoder(struct mtk_vcodec_dec_ctx *ctx)
>  	return vdec_if_decode(ctx, NULL, NULL, &res_chg);
>  }
>  
> +static int mtk_vcodec_get_pic_info(struct mtk_vcodec_dec_ctx *ctx)
> +{
> +	struct mtk_q_data *q_data;
> +	int ret = 0;
> +
> +	q_data = &ctx->q_data[MTK_Q_DATA_DST];
> +	if (q_data->fmt->num_planes == 1) {
> +		mtk_v4l2_vdec_err(ctx, "[%d]Error!! 10bit mode not support one plane", ctx->id);
> +		return -EINVAL;
> +	}
> +
> +	ctx->capture_fourcc = q_data->fmt->fourcc;
> +	ret = vdec_if_get_param(ctx, GET_PARAM_PIC_INFO, &ctx->picinfo);
> +	if (ret) {
> +		mtk_v4l2_vdec_err(ctx, "[%d]Error!! Get GET_PARAM_PICTURE_INFO Fail", ctx->id);
> +		return ret;
> +	}
> +
> +	ctx->last_decoded_picinfo = ctx->picinfo;
> +
> +	q_data->sizeimage[0] = ctx->picinfo.fb_sz[0];
> +	q_data->bytesperline[0] = ctx->picinfo.buf_w * 5 / 4;
> +
> +	q_data->sizeimage[1] = ctx->picinfo.fb_sz[1];
> +	q_data->bytesperline[1] = ctx->picinfo.buf_w * 5 / 4;
> +
> +	q_data->coded_width = ctx->picinfo.buf_w;
> +	q_data->coded_height = ctx->picinfo.buf_h;
> +	mtk_v4l2_vdec_dbg(1, ctx, "[%d] wxh=%dx%d pic wxh=%dx%d sz[0]=0x%x sz[1]=0x%x",
> +			  ctx->id, ctx->picinfo.buf_w, ctx->picinfo.buf_h,
> +			  ctx->picinfo.pic_w, ctx->picinfo.pic_h,
> +			  q_data->sizeimage[0], q_data->sizeimage[1]);
> +
> +	return ret;
> +}
> +
> +static int mtk_vdec_s_ctrl(struct v4l2_ctrl *ctrl)
> +{
> +	struct mtk_vcodec_dec_ctx *ctx = ctrl_to_dec_ctx(ctrl);
> +	struct v4l2_ctrl_h264_sps *h264;
> +	struct v4l2_ctrl_hevc_sps *h265;
> +	struct v4l2_ctrl_vp9_frame *frame;
> +	struct v4l2_ctrl_av1_sequence *seq;
> +	struct v4l2_ctrl *hdr_ctrl;
> +	const struct mtk_vcodec_dec_pdata *dec_pdata = ctx->dev->vdec_pdata;
> +	const struct mtk_video_fmt *fmt;
> +	int i = 0, ret = 0;
> +
> +	hdr_ctrl = ctrl;
> +	if (!hdr_ctrl || !hdr_ctrl->p_cur.p)
> +		return -EINVAL;
> +
> +	switch (hdr_ctrl->id) {
> +	case V4L2_CID_STATELESS_H264_SPS:
> +		h264 = (struct v4l2_ctrl_h264_sps *)hdr_ctrl->p_new.p;
> +		if (h264->bit_depth_chroma_minus8 == 2 && h264->bit_depth_luma_minus8 == 2) {
> +			ctx->is_10bit_bitstream = true;
> +		} else if (h264->bit_depth_chroma_minus8 != 0 &&
> +			   h264->bit_depth_luma_minus8 != 0) {
> +			mtk_v4l2_vdec_err(ctx, "H264: chroma_minus8:%d, luma_minus8:%d",
> +					  h264->bit_depth_chroma_minus8,
> +					  h264->bit_depth_luma_minus8);
> +			return -EINVAL;
> +		}
> +		break;
> +	case V4L2_CID_STATELESS_HEVC_SPS:
> +		h265 = (struct v4l2_ctrl_hevc_sps *)hdr_ctrl->p_new.p;
> +		if (h265->bit_depth_chroma_minus8 == 2 && h265->bit_depth_luma_minus8 == 2) {
> +			ctx->is_10bit_bitstream = true;
> +		} else if (h265->bit_depth_chroma_minus8 != 0 &&
> +			   h265->bit_depth_luma_minus8 != 0) {
> +			mtk_v4l2_vdec_err(ctx, "HEVC: chroma_minus8:%d, luma_minus8:%d",
> +					  h265->bit_depth_chroma_minus8,
> +					  h265->bit_depth_luma_minus8);
> +			return -EINVAL;
> +		}
> +		break;
> +	case V4L2_CID_STATELESS_VP9_FRAME:
> +		frame = (struct v4l2_ctrl_vp9_frame *)hdr_ctrl->p_new.p;
> +		if (frame->bit_depth == 10) {
> +			ctx->is_10bit_bitstream = true;
> +		} else if (frame->bit_depth != 8) {
> +			mtk_v4l2_vdec_err(ctx, "VP9: bit_depth:%d", frame->bit_depth);
> +			return -EINVAL;
> +		}
> +		break;
> +	case V4L2_CID_STATELESS_AV1_SEQUENCE:
> +		seq = (struct v4l2_ctrl_av1_sequence *)hdr_ctrl->p_new.p;
> +		if (seq->bit_depth == 10) {
> +			ctx->is_10bit_bitstream = true;
> +		} else if (seq->bit_depth != 8) {
> +			mtk_v4l2_vdec_err(ctx, "AV1: bit_depth:%d", seq->bit_depth);
> +			return -EINVAL;
> +		}
> +		break;
> +	default:
> +		mtk_v4l2_vdec_err(ctx, "Not supported ctrl id: 0x%x\n", hdr_ctrl->id);
> +		return -EINVAL;

I can confirm we hit this for every single codec and decoding fails. Written
this way, this code should never have worked, even for 10bit decoding.

> +	}
> +
> +	if (!ctx->is_10bit_bitstream)
> +		return ret;
> +
> +	for (i = 0; i < *dec_pdata->num_formats; i++) {
> +		fmt = &dec_pdata->vdec_formats[i];
> +		if (fmt->fourcc == V4L2_PIX_FMT_MT2110R &&
> +		    hdr_ctrl->id == V4L2_CID_STATELESS_H264_SPS) {
> +			ctx->q_data[MTK_Q_DATA_DST].fmt = fmt;
> +			break;
> +		}
> +
> +		if (fmt->fourcc == V4L2_PIX_FMT_MT2110T &&
> +		    (hdr_ctrl->id == V4L2_CID_STATELESS_HEVC_SPS ||
> +		    hdr_ctrl->id == V4L2_CID_STATELESS_VP9_FRAME ||
> +		    hdr_ctrl->id == V4L2_CID_STATELESS_AV1_SEQUENCE)) {
> +			ctx->q_data[MTK_Q_DATA_DST].fmt = fmt;
> +			break;
> +		}
> +	}
> +	ret = mtk_vcodec_get_pic_info(ctx);
> +
> +	return ret;
> +}
> +
> +static const struct v4l2_ctrl_ops mtk_vcodec_dec_ctrl_ops = {
> +	.s_ctrl = mtk_vdec_s_ctrl,
> +};
> +
>  static int mtk_vcodec_dec_ctrls_setup(struct mtk_vcodec_dec_ctx *ctx)
>  {
>  	unsigned int i;
> @@ -399,7 +527,7 @@ 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;
>  		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,
> @@ -466,6 +594,8 @@ static void mtk_vcodec_add_formats(unsigned int fourcc,
>  		break;
>  	case V4L2_PIX_FMT_MM21:
>  	case V4L2_PIX_FMT_MT21C:
> +	case V4L2_PIX_FMT_MT2110T:
> +	case V4L2_PIX_FMT_MT2110R:
>  		mtk_video_formats[count_formats].fourcc = fourcc;
>  		mtk_video_formats[count_formats].type = MTK_FMT_FRAME;
>  		mtk_video_formats[count_formats].num_planes = 2;
> @@ -491,6 +621,12 @@ static void mtk_vcodec_get_supported_formats(struct mtk_vcodec_dec_ctx *ctx)
>  		mtk_vcodec_add_formats(V4L2_PIX_FMT_MT21C, ctx);
>  		cap_format_count++;
>  	}
> +	if (ctx->dev->dec_capability & MTK_VDEC_IS_SUPPORT_10BIT) {
> +		mtk_vcodec_add_formats(V4L2_PIX_FMT_MT2110T, ctx);
> +		cap_format_count++;
> +		mtk_vcodec_add_formats(V4L2_PIX_FMT_MT2110R, ctx);
> +		cap_format_count++;
> +	}
>  	if (ctx->dev->dec_capability & MTK_VDEC_FORMAT_MM21) {
>  		mtk_vcodec_add_formats(V4L2_PIX_FMT_MM21, ctx);
>  		cap_format_count++;
Re: [PATCH 3/3] media: mediatek: vcodec: Add driver to support 10bit
Posted by Yunfei Dong (董云飞) 1 year, 2 months ago
Hi Nicolas,

Thanks for your suggestion.

On Tue, 2023-07-11 at 16:12 -0400, Nicolas Dufresne wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  Le mardi 11 juillet 2023 à 20:57 +0800, Yunfei Dong a écrit :
> > From: Mingjia Zhang <mingjia.zhang@mediatek.com>
> > 
> > Adding to support capture formats V4L2_PIX_FMT_MT2110T and
> > V4L2_PIX_FMT_MT2110R for 10bit playback. Need to get the size
> > of each plane again when user space setting syntax to get 10bit
> > information.
> > 
> > V4L2_PIX_FMT_MT2110T for AV1/VP9/HEVC.
> > V4L2_PIX_FMT_MT2110R for H264.
> > 
> > Signed-off-by: Mingjia Zhang <mingjia.zhang@mediatek.com>
> > Co-developed-by: Yunfei Dong <yunfei.dong@mediatek.com>
> > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
> > ---
> >  .../mediatek/vcodec/decoder/mtk_vcodec_dec.c  |  22 ++-
> >  .../vcodec/decoder/mtk_vcodec_dec_drv.h       |   5 +
> >  .../vcodec/decoder/mtk_vcodec_dec_stateless.c | 140
> +++++++++++++++++-
> >  3 files changed, 163 insertions(+), 4 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 5acb7dff18f2..91ed576d6821 100644
> > ---
> a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
> > +++
> b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
> > @@ -37,7 +37,9 @@ static bool mtk_vdec_get_cap_fmt(struct
> mtk_vcodec_dec_ctx *ctx, int format_inde
> >  {
> >  const struct mtk_vcodec_dec_pdata *dec_pdata = ctx->dev-
> >vdec_pdata;
> >  const struct mtk_video_fmt *fmt;
> > +struct mtk_q_data *q_data;
> >  int num_frame_count = 0, i;
> > +bool ret = false;
> >  
> >  fmt = &dec_pdata->vdec_formats[format_index];
> >  for (i = 0; i < *dec_pdata->num_formats; i++) {
> > @@ -47,10 +49,26 @@ static bool mtk_vdec_get_cap_fmt(struct
> mtk_vcodec_dec_ctx *ctx, int format_inde
> >  num_frame_count++;
> >  }
> >  
> > -if (num_frame_count == 1 || fmt->fourcc == V4L2_PIX_FMT_MM21)
> > +if (num_frame_count == 1 || (!ctx->is_10bit_bitstream && fmt-
> >fourcc == V4L2_PIX_FMT_MM21))
> >  return true;
> >  
> > -return false;
> > +q_data = &ctx->q_data[MTK_Q_DATA_SRC];
> > +switch (q_data->fmt->fourcc) {
> > +case V4L2_PIX_FMT_H264_SLICE:
> > +if (ctx->is_10bit_bitstream && fmt->fourcc ==
> V4L2_PIX_FMT_MT2110R)
> > +ret = true;
> > +break;
> > +case V4L2_PIX_FMT_VP9_FRAME:
> > +case V4L2_PIX_FMT_AV1_FRAME:
> > +case V4L2_PIX_FMT_HEVC_SLICE:
> > +if (ctx->is_10bit_bitstream && fmt->fourcc ==
> V4L2_PIX_FMT_MT2110T)
> > +ret = true;
> > +break;
> > +default:
> > +break;
> > +}
> > +
> > +return ret;
> >  }
> >  
> >  static struct mtk_q_data *mtk_vdec_get_q_data(struct
> mtk_vcodec_dec_ctx *ctx,
> > 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 c8b4374c5e6c..cd607e90fe9c 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
> > @@ -31,6 +31,7 @@ enum mtk_vdec_format_types {
> >  MTK_VDEC_FORMAT_AV1_FRAME = 0x800,
> >  MTK_VDEC_FORMAT_HEVC_FRAME = 0x1000,
> >  MTK_VCODEC_INNER_RACING = 0x20000,
> > +MTK_VDEC_IS_SUPPORT_10BIT = 0x40000,
> >  };
> >  
> >  /*
> > @@ -160,6 +161,8 @@ struct mtk_vcodec_dec_pdata {
> >   * @hw_id: hardware index used to identify different hardware.
> >   *
> >   * @msg_queue: msg queue used to store lat buffer information.
> > + *
> > + * @is_10bit_bitstream: set to true if it's 10bit bitstream
> >   */
> >  struct mtk_vcodec_dec_ctx {
> >  enum mtk_instance_type type;
> > @@ -202,6 +205,8 @@ struct mtk_vcodec_dec_ctx {
> >  int hw_id;
> >  
> >  struct vdec_msg_queue msg_queue;
> > +
> > +bool is_10bit_bitstream;
> >  };
> >  
> >  /**
> > diff --git
> a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_state
> less.c
> b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_state
> less.c
> > index 99a84c7e1901..cef937fdf462 100644
> > ---
> a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_state
> less.c
> > +++
> b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_state
> less.c
> > @@ -200,7 +200,7 @@ static const struct mtk_stateless_control
> mtk_stateless_controls[] = {
> >  
> >  #define NUM_CTRLS ARRAY_SIZE(mtk_stateless_controls)
> >  
> > -static struct mtk_video_fmt mtk_video_formats[7];
> > +static struct mtk_video_fmt mtk_video_formats[9];
> >  
> >  static struct mtk_video_fmt default_out_format;
> >  static struct mtk_video_fmt default_cap_format;
> > @@ -387,6 +387,134 @@ static int mtk_vdec_flush_decoder(struct
> mtk_vcodec_dec_ctx *ctx)
> >  return vdec_if_decode(ctx, NULL, NULL, &res_chg);
> >  }
> >  
> > +static int mtk_vcodec_get_pic_info(struct mtk_vcodec_dec_ctx *ctx)
> > +{
> > +struct mtk_q_data *q_data;
> > +int ret = 0;
> > +
> > +q_data = &ctx->q_data[MTK_Q_DATA_DST];
> > +if (q_data->fmt->num_planes == 1) {
> > +mtk_v4l2_vdec_err(ctx, "[%d]Error!! 10bit mode not support one
> plane", ctx->id);
> > +return -EINVAL;
> > +}
> > +
> > +ctx->capture_fourcc = q_data->fmt->fourcc;
> > +ret = vdec_if_get_param(ctx, GET_PARAM_PIC_INFO, &ctx->picinfo);
> > +if (ret) {
> > +mtk_v4l2_vdec_err(ctx, "[%d]Error!! Get GET_PARAM_PICTURE_INFO
> Fail", ctx->id);
> > +return ret;
> > +}
> > +
> > +ctx->last_decoded_picinfo = ctx->picinfo;
> > +
> > +q_data->sizeimage[0] = ctx->picinfo.fb_sz[0];
> > +q_data->bytesperline[0] = ctx->picinfo.buf_w * 5 / 4;
> > +
> > +q_data->sizeimage[1] = ctx->picinfo.fb_sz[1];
> > +q_data->bytesperline[1] = ctx->picinfo.buf_w * 5 / 4;
> > +
> > +q_data->coded_width = ctx->picinfo.buf_w;
> > +q_data->coded_height = ctx->picinfo.buf_h;
> > +mtk_v4l2_vdec_dbg(1, ctx, "[%d] wxh=%dx%d pic wxh=%dx%d sz[0]=0x%x
> sz[1]=0x%x",
> > +  ctx->id, ctx->picinfo.buf_w, ctx->picinfo.buf_h,
> > +  ctx->picinfo.pic_w, ctx->picinfo.pic_h,
> > +  q_data->sizeimage[0], q_data->sizeimage[1]);
> > +
> > +return ret;
> > +}
> > +
> > +static int mtk_vdec_s_ctrl(struct v4l2_ctrl *ctrl)
> > +{
> > +struct mtk_vcodec_dec_ctx *ctx = ctrl_to_dec_ctx(ctrl);
> > +struct v4l2_ctrl_h264_sps *h264;
> > +struct v4l2_ctrl_hevc_sps *h265;
> > +struct v4l2_ctrl_vp9_frame *frame;
> > +struct v4l2_ctrl_av1_sequence *seq;
> > +struct v4l2_ctrl *hdr_ctrl;
> > +const struct mtk_vcodec_dec_pdata *dec_pdata = ctx->dev-
> >vdec_pdata;
> > +const struct mtk_video_fmt *fmt;
> > +int i = 0, ret = 0;
> > +
> > +hdr_ctrl = ctrl;
> > +if (!hdr_ctrl || !hdr_ctrl->p_cur.p)
> > +return -EINVAL;
> > +
> > +switch (hdr_ctrl->id) {
> > +case V4L2_CID_STATELESS_H264_SPS:
> > +h264 = (struct v4l2_ctrl_h264_sps *)hdr_ctrl->p_new.p;
> > +if (h264->bit_depth_chroma_minus8 == 2 && h264-
> >bit_depth_luma_minus8 == 2) {
> > +ctx->is_10bit_bitstream = true;
> > +} else if (h264->bit_depth_chroma_minus8 != 0 &&
> > +   h264->bit_depth_luma_minus8 != 0) {
> > +mtk_v4l2_vdec_err(ctx, "H264: chroma_minus8:%d, luma_minus8:%d",
> > +  h264->bit_depth_chroma_minus8,
> > +  h264->bit_depth_luma_minus8);
> > +return -EINVAL;
> > +}
> > +break;
> > +case V4L2_CID_STATELESS_HEVC_SPS:
> > +h265 = (struct v4l2_ctrl_hevc_sps *)hdr_ctrl->p_new.p;
> > +if (h265->bit_depth_chroma_minus8 == 2 && h265-
> >bit_depth_luma_minus8 == 2) {
> > +ctx->is_10bit_bitstream = true;
> > +} else if (h265->bit_depth_chroma_minus8 != 0 &&
> > +   h265->bit_depth_luma_minus8 != 0) {
> > +mtk_v4l2_vdec_err(ctx, "HEVC: chroma_minus8:%d, luma_minus8:%d",
> > +  h265->bit_depth_chroma_minus8,
> > +  h265->bit_depth_luma_minus8);
> > +return -EINVAL;
> > +}
> > +break;
> > +case V4L2_CID_STATELESS_VP9_FRAME:
> > +frame = (struct v4l2_ctrl_vp9_frame *)hdr_ctrl->p_new.p;
> > +if (frame->bit_depth == 10) {
> > +ctx->is_10bit_bitstream = true;
> > +} else if (frame->bit_depth != 8) {
> > +mtk_v4l2_vdec_err(ctx, "VP9: bit_depth:%d", frame->bit_depth);
> > +return -EINVAL;
> > +}
> > +break;
> > +case V4L2_CID_STATELESS_AV1_SEQUENCE:
> > +seq = (struct v4l2_ctrl_av1_sequence *)hdr_ctrl->p_new.p;
> > +if (seq->bit_depth == 10) {
> > +ctx->is_10bit_bitstream = true;
> > +} else if (seq->bit_depth != 8) {
> > +mtk_v4l2_vdec_err(ctx, "AV1: bit_depth:%d", seq->bit_depth);
> > +return -EINVAL;
> > +}
> > +break;
> > +default:
> > +mtk_v4l2_vdec_err(ctx, "Not supported ctrl id: 0x%x\n", hdr_ctrl-
> >id);
> > +return -EINVAL;
> 
> I can confirm we hit this for every single codec and decoding fails.
> Written
> this way, this code should never have worked, even for 10bit
> decoding.
> 
Removed this line in local test, forgot PPS and other syntax. Will fix
in next patch.

Will return 0 when in default case, and replace mtk_v4l2_vdec_err with
mtk_v4l2_vdec_dbg.

Best Regards,
Yunfei Dong
> > +}
> > +
> > +if (!ctx->is_10bit_bitstream)
> > +return ret;
> > +
> > +for (i = 0; i < *dec_pdata->num_formats; i++) {
> > +fmt = &dec_pdata->vdec_formats[i];
> > +if (fmt->fourcc == V4L2_PIX_FMT_MT2110R &&
> > +    hdr_ctrl->id == V4L2_CID_STATELESS_H264_SPS) {
> > +ctx->q_data[MTK_Q_DATA_DST].fmt = fmt;
> > +break;
> > +}
> > +
> > +if (fmt->fourcc == V4L2_PIX_FMT_MT2110T &&
> > +    (hdr_ctrl->id == V4L2_CID_STATELESS_HEVC_SPS ||
> > +    hdr_ctrl->id == V4L2_CID_STATELESS_VP9_FRAME ||
> > +    hdr_ctrl->id == V4L2_CID_STATELESS_AV1_SEQUENCE)) {
> > +ctx->q_data[MTK_Q_DATA_DST].fmt = fmt;
> > +break;
> > +}
> > +}
> > +ret = mtk_vcodec_get_pic_info(ctx);
> > +
> > +return ret;
> > +}
> > +
> > +static const struct v4l2_ctrl_ops mtk_vcodec_dec_ctrl_ops = {
> > +.s_ctrl = mtk_vdec_s_ctrl,
> > +};
> > +
> >  static int mtk_vcodec_dec_ctrls_setup(struct mtk_vcodec_dec_ctx
> *ctx)
> >  {
> >  unsigned int i;
> > @@ -399,7 +527,7 @@ 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;
> >  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,
> > @@ -466,6 +594,8 @@ static void mtk_vcodec_add_formats(unsigned int
> fourcc,
> >  break;
> >  case V4L2_PIX_FMT_MM21:
> >  case V4L2_PIX_FMT_MT21C:
> > +case V4L2_PIX_FMT_MT2110T:
> > +case V4L2_PIX_FMT_MT2110R:
> >  mtk_video_formats[count_formats].fourcc = fourcc;
> >  mtk_video_formats[count_formats].type = MTK_FMT_FRAME;
> >  mtk_video_formats[count_formats].num_planes = 2;
> > @@ -491,6 +621,12 @@ static void
> mtk_vcodec_get_supported_formats(struct mtk_vcodec_dec_ctx *ctx)
> >  mtk_vcodec_add_formats(V4L2_PIX_FMT_MT21C, ctx);
> >  cap_format_count++;
> >  }
> > +if (ctx->dev->dec_capability & MTK_VDEC_IS_SUPPORT_10BIT) {
> > +mtk_vcodec_add_formats(V4L2_PIX_FMT_MT2110T, ctx);
> > +cap_format_count++;
> > +mtk_vcodec_add_formats(V4L2_PIX_FMT_MT2110R, ctx);
> > +cap_format_count++;
> > +}
> >  if (ctx->dev->dec_capability & MTK_VDEC_FORMAT_MM21) {
> >  mtk_vcodec_add_formats(V4L2_PIX_FMT_MM21, ctx);
> >  cap_format_count++;
>