| Message ID | 20230317030833.16836-2-yunfei.dong@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v4,1/2] media: mediatek: vcodec: Force capture queue format to MM21 | expand |
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
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
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++;
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(-)