Re: [PATCH v3] media: mediatek: vcodec: Force capture queue format to MM21

[Yunfei Dong (董云飞)]

Hi Nicolas,

Thanks for your suggestion.
On Thu, 2023-03-16 at 16:51 -0400, Nícolas F. R. A. Prado wrote:
> Hi Yunfei,
> 
> thanks for the patch.
> 
> On Tue, Mar 14, 2023 at 03:25:57PM +0800, Yunfei Dong wrote:
> > Libyuv is one software library used to covert format. Only covert
> > mediatek uncompressed mode MM21 to standard yuv420 for MT21 is
> > compressed mode. Need to set capture queue format to MM21 in order
> > to use Libyuv when scp firmware support MM21 and MT21.
> 
> This commit message is a bit confusing still. Here's a suggestion:
> 
> 	While the decoder can produce frames in both MM21 and MT21C
> formats, only MM21
> 	is currently supported by userspace tools (like gstreamer and
> libyuv). In order
> 	to ensure userspace keeps working after the SCP firmware is
> updated to support
> 	both MM21 and MT21C formats, force the MM21 format for the
> capture queue.
> 
> 	This is meant as a stopgap solution while dynamic format
> switching between 
> 	MM21 and MT21C isn't implemented in the driver.
> 
Will change it like this.
> > 
> > Fixes: 7501edef6b1f ("media: mediatek: vcodec: Different codec
> > using different capture format")
> > Signed-off-by: Yunfei Dong <yunfei.dong@xxxxxxxxxxxx>
> > ---
> > changed with v2:
> > - re-write commit message.
> > - change the driver flow.
> > changed with v1:
> > - add Fixes tag.
> > ---
> >  .../platform/mediatek/vcodec/mtk_vcodec_dec.c | 24 +++----------
> > ------
> >  1 file changed, 3 insertions(+), 21 deletions(-)
> > 
> 
> While this change works (that is, I'm able to run fluster tests on
> both MT8192
> and MT8195 using the new MM21+MT21C firmware), it causes a regression
> on
> v4l2-compliance:
> 
> 	[..]
> 	Format ioctls:
> 	[..]
> 			fail: v4l2-test-formats.cpp(478): pixelformat
> 3132544d (MT21) for buftype 9 not reported by ENUM_FMT
> 		test VIDIOC_G_FMT: FAIL
> 			fail: v4l2-test-formats.cpp(478): pixelformat
> 3132544d (MT21) for buftype 9 not reported by ENUM_FMT
> 		test VIDIOC_TRY_FMT: FAIL
> 			fail: v4l2-test-formats.cpp(478): pixelformat
> 3132544d (MT21) for buftype 9 not reported by ENUM_FMT
> 		test VIDIOC_S_FMT: FAIL
> 	[..]
> 	Total for mtk-vcodec-dec device /dev/video2: 46, Succeeded: 43,
> Failed: 3, Warnings: 0
> 
Need to add one patch to change MM21 as the default capture queue
format.

Best Regards,
Yunfei Dong
> The patch makes only MM21 show in VIDIOC_ENUM_FMT, but VIDIOC_S_FMT,
> VIDIOC_G_FMT and VIDIOC_TRY_FMT still show MT21. So I think you need
> to do the
> same forcing of MM21 for those ioctls.
> 
> Thanks,
> Nícolas
> 
> > diff --git
> > a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
> > b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
> > index 641f533c417f..c99705681a03 100644
> > --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
> > +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
> > @@ -39,10 +39,9 @@ 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;
> >  
> > +	fmt = &dec_pdata->vdec_formats[format_index];
> >  	for (i = 0; i < *dec_pdata->num_formats; i++) {
> >  		if (dec_pdata->vdec_formats[i].type != MTK_FMT_FRAME)
> >  			continue;
> > @@ -50,27 +49,10 @@ static bool mtk_vdec_get_cap_fmt(struct
> > mtk_vcodec_ctx *ctx, int format_index)
> >  		num_frame_count++;
> >  	}
> >  
> > -	if (num_frame_count == 1)
> > +	if (num_frame_count == 1 || fmt->fourcc == V4L2_PIX_FMT_MM21)
> >  		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;
> > +	return false;
> >  }
> >  
> >  static struct mtk_q_data *mtk_vdec_get_q_data(struct
> > mtk_vcodec_ctx *ctx,
> > -- 
> > 2.25.1
> > 
> 
>