diff mbox series

[v7,5/7] media: mediatek: vcodec: Different codec using different capture format

Message ID 20220518123004.18286-6-yunfei.dong@mediatek.com (mailing list archive)
State New, archived
Headers show
Series support mt8195 decoder | expand

Commit Message

Yunfei Dong May 18, 2022, 12:30 p.m. UTC
Vp8 need to use MM21, but vp9 and h264 need to use HyFbc mode
for mt8195. Vp8/vp9/h264 use the same MM21 format for mt8192.

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

Comments

Nícolas F. R. A. Prado Jan. 12, 2023, 8:58 p.m. UTC | #1
On Wed, May 18, 2022 at 08:30:02PM +0800, Yunfei Dong wrote:
> Vp8 need to use MM21, but vp9 and h264 need to use HyFbc mode
> for mt8195. Vp8/vp9/h264 use the same MM21 format for mt8192.

Hi Yunfei,

why do VP9 and H264 need to use HyFbc (is this the same as MT21C?) mode on
MT8195? The SCP firmware on linux-firmware for MT8195 [1] only has MM21
available and based on my testing it works just fine. And contrary to what the
commit message states this logic is also being applied to MT8192, preventing it
to use MM21 when there are more than one format available.

Thanks,
Nícolas

[1] https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/tree/mediatek/mt8195/scp.img?id=375d4500d315ff20c59911d12d86b477d4979b1d

> 
> Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
> ---
>  .../platform/mediatek/vcodec/mtk_vcodec_dec.c | 41 +++++++++++++++++++
>  1 file changed, 41 insertions(+)
> 
> diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
> index 52e5d36aa912..254649240b34 100644
> --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
> +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
> @@ -35,6 +35,44 @@ mtk_vdec_find_format(struct v4l2_format *f,
>  	return NULL;
>  }
>  
> +static bool mtk_vdec_get_cap_fmt(struct mtk_vcodec_ctx *ctx, int format_index)
> +{
> +	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 = true;
> +
> +	for (i = 0; i < *dec_pdata->num_formats; i++) {
> +		if (dec_pdata->vdec_formats[i].type != MTK_FMT_FRAME)
> +			continue;
> +
> +		num_frame_count++;
> +	}
> +
> +	if (num_frame_count == 1)
> +		return true;
> +
> +	fmt = &dec_pdata->vdec_formats[format_index];
> +	q_data = &ctx->q_data[MTK_Q_DATA_SRC];
> +	switch (q_data->fmt->fourcc) {
> +	case V4L2_PIX_FMT_VP8_FRAME:
> +		if (fmt->fourcc == V4L2_PIX_FMT_MM21)
> +			ret = true;
> +		break;
> +	case V4L2_PIX_FMT_H264_SLICE:
> +	case V4L2_PIX_FMT_VP9_FRAME:
> +		if (fmt->fourcc == V4L2_PIX_FMT_MM21)
> +			ret = false;
> +		break;
> +	default:
> +		ret = true;
> +		break;
> +	};
> +
> +	return ret;
> +}
> +
>  static struct mtk_q_data *mtk_vdec_get_q_data(struct mtk_vcodec_ctx *ctx,
>  					      enum v4l2_buf_type type)
>  {
> @@ -566,6 +604,9 @@ static int vidioc_enum_fmt(struct v4l2_fmtdesc *f, void *priv,
>  		    dec_pdata->vdec_formats[i].type != MTK_FMT_FRAME)
>  			continue;
>  
> +		if (!output_queue && !mtk_vdec_get_cap_fmt(ctx, i))
> +			continue;
> +
>  		if (j == f->index)
>  			break;
>  		++j;
> -- 
> 2.18.0
> 
>
Yunfei Dong Jan. 13, 2023, 3:25 a.m. UTC | #2
Hi Nicolas,

Thanks for your information.
On Thu, 2023-01-12 at 15:58 -0500, Nícolas F. R. A. Prado wrote:
> On Wed, May 18, 2022 at 08:30:02PM +0800, Yunfei Dong wrote:
> > Vp8 need to use MM21, but vp9 and h264 need to use HyFbc mode
> > for mt8195. Vp8/vp9/h264 use the same MM21 format for mt8192.
> 
> Hi Yunfei,
> 
> why do VP9 and H264 need to use HyFbc (is this the same as MT21C?)
> mode on
> MT8195? The SCP firmware on linux-firmware for MT8195 [1] only has
> MM21
> available and based on my testing it works just fine. And contrary to
> what the
> commit message states this logic is also being applied to MT8192,
> preventing it
> to use MM21 when there are more than one format available.
> 
The kernel driver not support dynamic switch mt21c and mm21. The driver
flow as below:
1: If there are mt21c and mm21: h264/vp9/av1 will use mt21c(vp8 use
mm21 always);
2: If there are only mt21c or mm21 will use this format only.

But the scp.img is changed to support mt21c and mm21, the kernel driver
will use mt21c. Or force the kernel driver only support mm21(small
effort).

But the best solution is changed the driver to dynamic switch mt21c and
mm21, need to discuss the enum and set format flow(middle effort).

Best Regards,
Yunfei Dong
> Thanks,
> Nícolas
> 
> [1] 
> https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/tree/mediatek/mt8195/scp.img?id=375d4500d315ff20c59911d12d86b477d4979b1d__;!!CTRNKA9wMg0ARbw!gwMxFusfgog4Rd0Zg9nrN7zae3Aweimkw2ml2K_ZxWPcNTenk9ik7TD7sxJXkZv2IahSaZztTCElg982Eeg2zmFh$ 
>  
> 
> > 
> > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
> > ---
> >  .../platform/mediatek/vcodec/mtk_vcodec_dec.c | 41
> > +++++++++++++++++++
> >  1 file changed, 41 insertions(+)
> > 
> > diff --git
> > a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
> > b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
> > index 52e5d36aa912..254649240b34 100644
> > --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
> > +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
> > @@ -35,6 +35,44 @@ mtk_vdec_find_format(struct v4l2_format *f,
> >  	return NULL;
> >  }
> >  
> > +static bool mtk_vdec_get_cap_fmt(struct mtk_vcodec_ctx *ctx, int
> > format_index)
> > +{
> > +	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 = true;
> > +
> > +	for (i = 0; i < *dec_pdata->num_formats; i++) {
> > +		if (dec_pdata->vdec_formats[i].type != MTK_FMT_FRAME)
> > +			continue;
> > +
> > +		num_frame_count++;
> > +	}
> > +
> > +	if (num_frame_count == 1)
> > +		return true;
> > +
> > +	fmt = &dec_pdata->vdec_formats[format_index];
> > +	q_data = &ctx->q_data[MTK_Q_DATA_SRC];
> > +	switch (q_data->fmt->fourcc) {
> > +	case V4L2_PIX_FMT_VP8_FRAME:
> > +		if (fmt->fourcc == V4L2_PIX_FMT_MM21)
> > +			ret = true;
> > +		break;
> > +	case V4L2_PIX_FMT_H264_SLICE:
> > +	case V4L2_PIX_FMT_VP9_FRAME:
> > +		if (fmt->fourcc == V4L2_PIX_FMT_MM21)
> > +			ret = false;
> > +		break;
> > +	default:
> > +		ret = true;
> > +		break;
> > +	};
> > +
> > +	return ret;
> > +}
> > +
> >  static struct mtk_q_data *mtk_vdec_get_q_data(struct
> > mtk_vcodec_ctx *ctx,
> >  					      enum v4l2_buf_type type)
> >  {
> > @@ -566,6 +604,9 @@ static int vidioc_enum_fmt(struct v4l2_fmtdesc
> > *f, void *priv,
> >  		    dec_pdata->vdec_formats[i].type != MTK_FMT_FRAME)
> >  			continue;
> >  
> > +		if (!output_queue && !mtk_vdec_get_cap_fmt(ctx, i))
> > +			continue;
> > +
> >  		if (j == f->index)
> >  			break;
> >  		++j;
> > -- 
> > 2.18.0
> > 
> > 
> 
>
Nícolas F. R. A. Prado Jan. 13, 2023, 3:02 p.m. UTC | #3
On Fri, Jan 13, 2023 at 03:25:46AM +0000, Yunfei Dong (董云飞) wrote:
> Hi Nicolas,
> 
> Thanks for your information.
> On Thu, 2023-01-12 at 15:58 -0500, Nícolas F. R. A. Prado wrote:
> > On Wed, May 18, 2022 at 08:30:02PM +0800, Yunfei Dong wrote:
> > > Vp8 need to use MM21, but vp9 and h264 need to use HyFbc mode
> > > for mt8195. Vp8/vp9/h264 use the same MM21 format for mt8192.
> > 
> > Hi Yunfei,
> > 
> > why do VP9 and H264 need to use HyFbc (is this the same as MT21C?)
> > mode on
> > MT8195? The SCP firmware on linux-firmware for MT8195 [1] only has
> > MM21
> > available and based on my testing it works just fine. And contrary to
> > what the
> > commit message states this logic is also being applied to MT8192,
> > preventing it
> > to use MM21 when there are more than one format available.
> > 
> The kernel driver not support dynamic switch mt21c and mm21. The driver
> flow as below:
> 1: If there are mt21c and mm21: h264/vp9/av1 will use mt21c(vp8 use
> mm21 always);
> 2: If there are only mt21c or mm21 will use this format only.
> 
> But the scp.img is changed to support mt21c and mm21, the kernel driver
> will use mt21c. Or force the kernel driver only support mm21(small
> effort).
> 
> But the best solution is changed the driver to dynamic switch mt21c and
> mm21, need to discuss the enum and set format flow(middle effort).

Agreed. The aim should be to get this dynamic format switching implemented in
the driver so that the firmware can expose both formats and userspace can
select any of them to use.

However, as you said that solution will take more effort, so in the meantime,
given that currently only mm21 is supported by gstreamer, I think forcing the
kernel driver to always select mm21 makes the most sense right now. That way it
doesn't matter whether the firmware has mm21 or mm21+mt21c, decoding will keep
working with gstreamer using mm21.

Thanks,
Nícolas
diff mbox series

Patch

diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
index 52e5d36aa912..254649240b34 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
@@ -35,6 +35,44 @@  mtk_vdec_find_format(struct v4l2_format *f,
 	return NULL;
 }
 
+static bool mtk_vdec_get_cap_fmt(struct mtk_vcodec_ctx *ctx, int format_index)
+{
+	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 = true;
+
+	for (i = 0; i < *dec_pdata->num_formats; i++) {
+		if (dec_pdata->vdec_formats[i].type != MTK_FMT_FRAME)
+			continue;
+
+		num_frame_count++;
+	}
+
+	if (num_frame_count == 1)
+		return true;
+
+	fmt = &dec_pdata->vdec_formats[format_index];
+	q_data = &ctx->q_data[MTK_Q_DATA_SRC];
+	switch (q_data->fmt->fourcc) {
+	case V4L2_PIX_FMT_VP8_FRAME:
+		if (fmt->fourcc == V4L2_PIX_FMT_MM21)
+			ret = true;
+		break;
+	case V4L2_PIX_FMT_H264_SLICE:
+	case V4L2_PIX_FMT_VP9_FRAME:
+		if (fmt->fourcc == V4L2_PIX_FMT_MM21)
+			ret = false;
+		break;
+	default:
+		ret = true;
+		break;
+	};
+
+	return ret;
+}
+
 static struct mtk_q_data *mtk_vdec_get_q_data(struct mtk_vcodec_ctx *ctx,
 					      enum v4l2_buf_type type)
 {
@@ -566,6 +604,9 @@  static int vidioc_enum_fmt(struct v4l2_fmtdesc *f, void *priv,
 		    dec_pdata->vdec_formats[i].type != MTK_FMT_FRAME)
 			continue;
 
+		if (!output_queue && !mtk_vdec_get_cap_fmt(ctx, i))
+			continue;
+
 		if (j == f->index)
 			break;
 		++j;