[PATCH v4,1/2] media: mediatek: vcodec: Force capture queue format to MM21

[PATCH v4,1/2] media: mediatek: vcodec: Force capture queue format to MM21

[Yunfei Dong]

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.

Fixes: 7501edef6b1f ("media: mediatek: vcodec: Different codec using different capture format")
Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
changed with v3:
- re-write commit message.
- add one new patch to fix v4l2-compliance fail.
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(-)

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

[PATCH v4,2/2] media: mediatek: vcodec: Using MM21 as the default capture format

[Yunfei Dong]

For the capture queue only support MM21 format with LibYuv, need to set MM21 as the
default format.

Fixes: 7501edef6b1f ("media: mediatek: vcodec: Different codec using different capture format")
Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
---
 .../platform/mediatek/vcodec/mtk_vcodec_dec_stateless.c   | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_stateless.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_stateless.c
index 04beb3f08eea..3000db975e5f 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_stateless.c
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_stateless.c
@@ -392,14 +392,14 @@ static void mtk_vcodec_get_supported_formats(struct mtk_vcodec_ctx *ctx)
 	if (num_formats)
 		return;
 
-	if (ctx->dev->dec_capability & MTK_VDEC_FORMAT_MM21) {
-		mtk_vcodec_add_formats(V4L2_PIX_FMT_MM21, ctx);
-		cap_format_count++;
-	}
 	if (ctx->dev->dec_capability & MTK_VDEC_FORMAT_MT21C) {
 		mtk_vcodec_add_formats(V4L2_PIX_FMT_MT21C, 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++;
+	}
 	if (ctx->dev->dec_capability & MTK_VDEC_FORMAT_H264_SLICE) {
 		mtk_vcodec_add_formats(V4L2_PIX_FMT_H264_SLICE, ctx);
 		out_format_count++;
-- 
2.25.1

Re: [PATCH v4,2/2] media: mediatek: vcodec: Using MM21 as the default capture format

[Nícolas F. R. A. Prado]

Hi Yunfei,

thanks for the patch.

The commit title should be in imperative, so I suggest:

	media: mediatek: vcodec: Make MM21 the default capture format

On Fri, Mar 17, 2023 at 11:08:33AM +0800, Yunfei Dong wrote:
> For the capture queue only support MM21 format with LibYuv, need to set MM21 as the
> default format.

Again, I think this commit message could be improved a bit. Here's a suggestion:

	Given that only the MM21 capture format is supported by userspace tools (like
	gstreamer and libyuv), make it the default capture format.

	This allows us to force the MM21 format even when a MM21 and MT21C capable
	firmware is available (which is needed while dynamic format switching isn't
	implemented in the driver), without causing the following regressions on
	v4l2-compliance:

				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

Also, I think it would be slightly better if this was the first patch in the
series, so that v4l2-compliance doesn't fail in between the patches.

> 
> Fixes: 7501edef6b1f ("media: mediatek: vcodec: Different codec using different capture format")
> Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>

With this change I've confirmed that all v4l2-compliance tests are passing again:

Total for mtk-vcodec-dec device /dev/video2: 46, Succeeded: 46, Failed: 0, Warnings: 0

So, after the above comments are addressed,

Reviewed-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
Tested-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>

Thanks,
Nícolas

Re: [PATCH v4,2/2] media: mediatek: vcodec: Using MM21 as the default capture format

[Yunfei Dong (董云飞)]

Hi Nicolas,

Thanks for your suggestion and test result.
On Fri, 2023-03-17 at 12:16 -0400, Nícolas F. R. A. Prado wrote:
> Hi Yunfei,
> 
> thanks for the patch.
> 
> The commit title should be in imperative, so I suggest:
> 
> 	media: mediatek: vcodec: Make MM21 the default capture format
> 
Accepted in next patch v4.
> On Fri, Mar 17, 2023 at 11:08:33AM +0800, Yunfei Dong wrote:
> > For the capture queue only support MM21 format with LibYuv, need to
> > set MM21 as the
> > default format.
> 
> Again, I think this commit message could be improved a bit. Here's a
> suggestion:
> 
> 	Given that only the MM21 capture format is supported by
> userspace tools (like
> 	gstreamer and libyuv), make it the default capture format.
> 
> 	This allows us to force the MM21 format even when a MM21 and
> MT21C capable
> 	firmware is available (which is needed while dynamic format
> switching isn't
> 	implemented in the driver), without causing the following
> regressions on
> 	v4l2-compliance:
> 
> 				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
> 
> Also, I think it would be slightly better if this was the first patch
> in the
> series, so that v4l2-compliance doesn't fail in between the patches.
> 
Accepted in next patch v4.

Best Regards,
Yunfei Dong
> > 
> > Fixes: 7501edef6b1f ("media: mediatek: vcodec: Different codec
> > using different capture format")
> > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
> 
> With this change I've confirmed that all v4l2-compliance tests are
> passing again:
> 
> Total for mtk-vcodec-dec device /dev/video2: 46, Succeeded: 46,
> Failed: 0, Warnings: 0
> 
> So, after the above comments are addressed,
> 
> Reviewed-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> Tested-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> 
> Thanks,
> Nícolas

Re: [PATCH v4,1/2] media: mediatek: vcodec: Force capture queue format to MM21

[Nícolas F. R. A. Prado]

On Fri, Mar 17, 2023 at 11:08:32AM +0800, Yunfei Dong wrote:
> 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.
> 
> Fixes: 7501edef6b1f ("media: mediatek: vcodec: Different codec using different capture format")
> Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Reviewed-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
Tested-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>


With this patch and the new firmware [1], I was able to run fluster using the
VP8, VP9 and H.264 codecs on both MT8192 and MT8195:

MT8192:
	        VP8: 59/61
	        VP9: 250/303
		     0/6 (HIGH)
	        H.264: 92/135
		       27/69 (JVT-FR-EXT)

MT8195:
	        VP8: 59/61
	        VP9: 276/303
		     0/6 (HIGH)
	        H.264: 95/135
		       27/69 (JVT-FR-EXT)

[1] https://lore.kernel.org/linux-firmware/a43524a089a783f70adbe89b83eeb01fbd405d04.camel@mediatek.com/T/#mb0591267d7921bbfada7c06ee2bda128db554648

Thanks,
Nícolas