LinuxLists.cc - [PATCH v2] media: mediatek: vcodec: Force capture queue format to MM21

2023-02-10 05:55:41

Subject: [PATCH v2] media: mediatek: vcodec: Force capture queue format to MM21

In order to conver the format of capture queue from mediatek MM21 to
standard yuv420 with Libyuv, need to force capture queue format to
MM21 for Libyuv can't covert mediatek MT21 format at current period.

Fixes: 7501edef6b1f ("media: mediatek: vcodec: Different codec using different capture format")
Signed-off-by: Yunfei Dong <[email protected]>
---
changed with v1:
- add Fixes tag.
---
drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c | 4 ++--
1 file changed, 2 insertions(+), 2 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..4f5e9c20214f 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
@@ -41,7 +41,7 @@ static bool mtk_vdec_get_cap_fmt(struct mtk_vcodec_ctx *ctx, int format_index)
const struct mtk_video_fmt *fmt;
struct mtk_q_data *q_data;
int num_frame_count = 0, i;
- bool ret = true;
+ bool ret = false;

for (i = 0; i < *dec_pdata->num_formats; i++) {
if (dec_pdata->vdec_formats[i].type != MTK_FMT_FRAME)
@@ -63,7 +63,7 @@ static bool mtk_vdec_get_cap_fmt(struct mtk_vcodec_ctx *ctx, int format_index)
case V4L2_PIX_FMT_H264_SLICE:
case V4L2_PIX_FMT_VP9_FRAME:
if (fmt->fourcc == V4L2_PIX_FMT_MM21)
- ret = false;
+ ret = true;
break;
default:
ret = true;
--
2.25.1

2023-02-10 15:36:40

by Nicolas Dufresne

[permalink] [raw]

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

Le vendredi 10 février 2023 à 13:55 +0800, Yunfei Dong a écrit :
> In order to conver the format of capture queue from mediatek MM21 to
> standard yuv420 with Libyuv, need to force capture queue format to
> MM21 for Libyuv can't covert mediatek MT21 format at current period.

Please rework this text, it is hard to understand.

>
> Fixes: 7501edef6b1f ("media: mediatek: vcodec: Different codec using different capture format")
> Signed-off-by: Yunfei Dong <[email protected]>
> ---
> changed with v1:
> - add Fixes tag.
> ---
> drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 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..4f5e9c20214f 100644
> --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
> +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
> @@ -41,7 +41,7 @@ static bool mtk_vdec_get_cap_fmt(struct mtk_vcodec_ctx *ctx, int format_index)
> const struct mtk_video_fmt *fmt;
> struct mtk_q_data *q_data;
> int num_frame_count = 0, i;
> - bool ret = true;
> + bool ret = false;
>
> for (i = 0; i < *dec_pdata->num_formats; i++) {
> if (dec_pdata->vdec_formats[i].type != MTK_FMT_FRAME)
> @@ -63,7 +63,7 @@ static bool mtk_vdec_get_cap_fmt(struct mtk_vcodec_ctx *ctx, int format_index)
> case V4L2_PIX_FMT_H264_SLICE:
> case V4L2_PIX_FMT_VP9_FRAME:
> if (fmt->fourcc == V4L2_PIX_FMT_MM21)
> - ret = false;
> + ret = true;

This makes the VP8 and the other cases identical, leaving anything that touches
MT21 as dead code. I'm not sure, cause I cannot test it, but it should in theory
render MT8192 unusable, unless a new firmware has been submitted to linux-
firmware with MM21 support ?

> break;
> default:
> ret = true;

2023-02-14 02:28:23

by Yunfei Dong (董云飞)

[permalink] [raw]

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

Hi Nicolas,

Thanks for your suggestion.
On Fri, 2023-02-10 at 10:36 -0500, Nicolas Dufresne wrote:
> Le vendredi 10 février 2023 à 13:55 +0800, Yunfei Dong a écrit :
> > In order to conver the format of capture queue from mediatek MM21
> > to
> > standard yuv420 with Libyuv, need to force capture queue format to
> > MM21 for Libyuv can't covert mediatek MT21 format at current
> > period.
>
> Please rework this text, it is hard to understand.
>

Will re-write the commit message in next version as below, could you
please help to review whether it's well to be used ?

Libyuv is one software library used to covert format. Only covert
mediatek MM21 to standard yuv420 for MT21 is compressed mode and MM21
is uncompressed mode at current period. Need to set capture queue
format to MM21 in order to use Libyuv.

> >
> > Fixes: 7501edef6b1f ("media: mediatek: vcodec: Different codec
> > using different capture format")
> > Signed-off-by: Yunfei Dong <[email protected]>
> > ---
> > changed with v1:
> > - add Fixes tag.
> > ---
> > drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 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..4f5e9c20214f 100644
> > --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
> > +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
> > @@ -41,7 +41,7 @@ static bool mtk_vdec_get_cap_fmt(struct
> > mtk_vcodec_ctx *ctx, int format_index)
> > const struct mtk_video_fmt *fmt;
> > struct mtk_q_data *q_data;
> > int num_frame_count = 0, i;
> > - bool ret = true;
> > + bool ret = false;
> >
> > for (i = 0; i < *dec_pdata->num_formats; i++) {
> > if (dec_pdata->vdec_formats[i].type != MTK_FMT_FRAME)
> > @@ -63,7 +63,7 @@ static bool mtk_vdec_get_cap_fmt(struct
> > mtk_vcodec_ctx *ctx, int format_index)
> > case V4L2_PIX_FMT_H264_SLICE:
> > case V4L2_PIX_FMT_VP9_FRAME:
> > if (fmt->fourcc == V4L2_PIX_FMT_MM21)
> > - ret = false;
> > + ret = true;
>
> This makes the VP8 and the other cases identical, leaving anything
> that touches
> MT21 as dead code. I'm not sure, cause I cannot test it, but it
> should in theory
> render MT8192 unusable, unless a new firmware has been submitted to
> linux-
> firmware with MM21 support ?
>

If the firmware only support MT21 => won't exist for vp8 need to use
MM21.

If the firmware only support MM21, will using MM21.
If the firmware support MT21 & MM21, will force VP8 to use MM21,
H264/VP9/AV1 will use MT21 before this change.

Will force all driver to use MM21 after adding this change.

Best Regards,
Yunfei Dong
> > break;
> > default:
> > ret = true;
>
>

2023-02-14 02:29:35

by Yunfei Dong (董云飞)

[permalink] [raw]

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

2023-02-22 21:12:07

by Nícolas F. R. A. Prado

[permalink] [raw]

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

Hi,

On Tue, Feb 14, 2023 at 02:28:04AM +0000, Yunfei Dong (董云飞) wrote:
> Hi Nicolas,
>
> Thanks for your suggestion.
> On Fri, 2023-02-10 at 10:36 -0500, Nicolas Dufresne wrote:
> > Le vendredi 10 février 2023 à 13:55 +0800, Yunfei Dong a écrit :
> > > In order to conver the format of capture queue from mediatek MM21
> > > to
> > > standard yuv420 with Libyuv, need to force capture queue format to
> > > MM21 for Libyuv can't covert mediatek MT21 format at current
> > > period.
> >
> > Please rework this text, it is hard to understand.
> >
>
> Will re-write the commit message in next version as below, could you
> please help to review whether it's well to be used ?
>
> Libyuv is one software library used to covert format. Only covert
> mediatek MM21 to standard yuv420 for MT21 is compressed mode and MM21
> is uncompressed mode at current period. Need to set capture queue
> format to MM21 in order to use Libyuv.
>
> > >
> > > Fixes: 7501edef6b1f ("media: mediatek: vcodec: Different codec
> > > using different capture format")
> > > Signed-off-by: Yunfei Dong <[email protected]>
> > > ---
> > > changed with v1:
> > > - add Fixes tag.
> > > ---
> > > drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 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..4f5e9c20214f 100644
> > > --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
> > > +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
> > > @@ -41,7 +41,7 @@ static bool mtk_vdec_get_cap_fmt(struct
> > > mtk_vcodec_ctx *ctx, int format_index)
> > > const struct mtk_video_fmt *fmt;
> > > struct mtk_q_data *q_data;
> > > int num_frame_count = 0, i;
> > > - bool ret = true;
> > > + bool ret = false;

This change doesn't do anything, so I'd drop it.

> > >
> > > for (i = 0; i < *dec_pdata->num_formats; i++) {
> > > if (dec_pdata->vdec_formats[i].type != MTK_FMT_FRAME)
> > > @@ -63,7 +63,7 @@ static bool mtk_vdec_get_cap_fmt(struct
> > > mtk_vcodec_ctx *ctx, int format_index)
> > > case V4L2_PIX_FMT_H264_SLICE:
> > > case V4L2_PIX_FMT_VP9_FRAME:
> > > if (fmt->fourcc == V4L2_PIX_FMT_MM21)
> > > - ret = false;
> > > + ret = true;
> >
> > This makes the VP8 and the other cases identical, leaving anything
> > that touches
> > MT21 as dead code. I'm not sure, cause I cannot test it, but it
> > should in theory
> > render MT8192 unusable, unless a new firmware has been submitted to
> > linux-
> > firmware with MM21 support ?
> >
>
> If the firmware only support MT21 => won't exist for vp8 need to use
> MM21.

And that's the issue, the scp.img for MT8192 on linux-firmware only supports
MT21 [1]. Can you please update it to support both MM21 and MT21?

For MT8195, only MM21 is supported in scp.img [2], but since the hardware
supports both MM21 and MT21, the firmware should also support both. So please
also update it on linux-firmware.

[1] https://lore.kernel.org/all/20230112204626.ciaff4amseoidybw@notapiano/
[2] https://lore.kernel.org/all/20230112205825.wb5qcqhh5kwvyi3y@notapiano/

Thanks,
Nícolas

>
> If the firmware only support MM21, will using MM21.
> If the firmware support MT21 & MM21, will force VP8 to use MM21,
> H264/VP9/AV1 will use MT21 before this change.
>
> Will force all driver to use MM21 after adding this change.
>
> Best Regards,
> Yunfei Dong
> > > break;
> > > default:
> > > ret = true;
> >
> >

2023-02-27 02:53:39

by Yunfei Dong (董云飞)

[permalink] [raw]

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

Hi Nicolas,

Thanks for your suggestion.
On Wed, 2023-02-22 at 16:11 -0500, Nícolas F. R. A. Prado wrote:
> Hi,
>
> On Tue, Feb 14, 2023 at 02:28:04AM +0000, Yunfei Dong (董云飞) wrote:
> > Hi Nicolas,
> >
> > Thanks for your suggestion.
> > On Fri, 2023-02-10 at 10:36 -0500, Nicolas Dufresne wrote:
> > > Le vendredi 10 février 2023 à 13:55 +0800, Yunfei Dong a écrit :
> > > > In order to conver the format of capture queue from mediatek
> > > > MM21
> > > > to
> > > > standard yuv420 with Libyuv, need to force capture queue format
> > > > to
> > > > MM21 for Libyuv can't covert mediatek MT21 format at current
> > > > period.
> > >
> > > Please rework this text, it is hard to understand.
> > >
> >
> > Will re-write the commit message in next version as below, could
> > you
> > please help to review whether it's well to be used ?
> >
> > Libyuv is one software library used to covert format. Only covert
> > mediatek MM21 to standard yuv420 for MT21 is compressed mode and
> > MM21
> > is uncompressed mode at current period. Need to set capture queue
> > format to MM21 in order to use Libyuv.
> >
> > > >
> > > > Fixes: 7501edef6b1f ("media: mediatek: vcodec: Different codec
> > > > using different capture format")
> > > > Signed-off-by: Yunfei Dong <[email protected]>
> > > > ---
> > > > changed with v1:
> > > > - add Fixes tag.
> > > > ---
> > > > drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c | 4
> > > > ++--
> > > > 1 file changed, 2 insertions(+), 2 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..4f5e9c20214f 100644
> > > > --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
> > > > +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
> > > > @@ -41,7 +41,7 @@ static bool mtk_vdec_get_cap_fmt(struct
> > > > mtk_vcodec_ctx *ctx, int format_index)
> > > > const struct mtk_video_fmt *fmt;
> > > > struct mtk_q_data *q_data;
> > > > int num_frame_count = 0, i;
> > > > - bool ret = true;
> > > > + bool ret = false;
>
> This change doesn't do anything, so I'd drop it.
>
This change is useful when capture fourcc is MT21 will return false,
not support even if scp support mm21 and mt21.
> > > >
> > > > for (i = 0; i < *dec_pdata->num_formats; i++) {
> > > > if (dec_pdata->vdec_formats[i].type !=
> > > > MTK_FMT_FRAME)
> > > > @@ -63,7 +63,7 @@ static bool mtk_vdec_get_cap_fmt(struct
> > > > mtk_vcodec_ctx *ctx, int format_index)
> > > > case V4L2_PIX_FMT_H264_SLICE:
> > > > case V4L2_PIX_FMT_VP9_FRAME:
> > > > if (fmt->fourcc == V4L2_PIX_FMT_MM21)
> > > > - ret = false;
> > > > + ret = true;
> > >
> > > This makes the VP8 and the other cases identical, leaving
> > > anything
> > > that touches
> > > MT21 as dead code. I'm not sure, cause I cannot test it, but it
> > > should in theory
> > > render MT8192 unusable, unless a new firmware has been submitted
> > > to
> > > linux-
> > > firmware with MM21 support ?
> > >
> >
> > If the firmware only support MT21 => won't exist for vp8 need to
> > use
> > MM21.
>
> And that's the issue, the scp.img for MT8192 on linux-firmware only
> supports
> MT21 [1]. Can you please update it to support both MM21 and MT21?
>
> For MT8195, only MM21 is supported in scp.img [2], but since the
> hardware
> supports both MM21 and MT21, the firmware should also support both.
> So please
> also update it on linux-firmware.
>
> [1]
> https://lore.kernel.org/all/20230112204626.ciaff4amseoidybw@notapiano/
> [2]
> https://lore.kernel.org/all/20230112205825.wb5qcqhh5kwvyi3y@notapiano/
>
> Thanks,
> Nícolas
>
MT8192 always use MM21 from the beginning, MT21 have not been enabled.

Best Regards,
Yunfei Dong
> >
> > If the firmware only support MM21, will using MM21.
> > If the firmware support MT21 & MM21, will force VP8 to use MM21,
> > H264/VP9/AV1 will use MT21 before this change.
> >
> > Will force all driver to use MM21 after adding this change.
> >
> > Best Regards,
> > Yunfei Dong
> > > > break;
> > > > default:
> > > > ret = true;
> > >
> > >

2023-02-28 22:03:52

by Nícolas F. R. A. Prado

[permalink] [raw]

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

On Mon, Feb 27, 2023 at 02:17:08AM +0000, Yunfei Dong (董云飞) wrote:
> Hi Nicolas,
>
> Thanks for your suggestion.
> On Wed, 2023-02-22 at 16:11 -0500, Nícolas F. R. A. Prado wrote:
> > Hi,
> >
> > On Tue, Feb 14, 2023 at 02:28:04AM +0000, Yunfei Dong (董云飞) wrote:
> > > Hi Nicolas,
> > >
> > > Thanks for your suggestion.
> > > On Fri, 2023-02-10 at 10:36 -0500, Nicolas Dufresne wrote:
> > > > Le vendredi 10 février 2023 à 13:55 +0800, Yunfei Dong a écrit :
[..]
> > > > > diff --git
> > > > > a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
> > > > > b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
> > > > > index 641f533c417f..4f5e9c20214f 100644
> > > > > --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
> > > > > +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
> > > > > @@ -41,7 +41,7 @@ static bool mtk_vdec_get_cap_fmt(struct
> > > > > mtk_vcodec_ctx *ctx, int format_index)
> > > > > const struct mtk_video_fmt *fmt;
> > > > > struct mtk_q_data *q_data;
> > > > > int num_frame_count = 0, i;
> > > > > - bool ret = true;
> > > > > + bool ret = false;
> >
> > This change doesn't do anything, so I'd drop it.
> >
> This change is useful when capture fourcc is MT21 will return false,
> not support even if scp support mm21 and mt21.

But you have

default:
ret = true;
break;

down below, so you're handling all cases in the switch, and the original value
doesn't matter. It could even be left uninitialized.

> > > > >
> > > > > for (i = 0; i < *dec_pdata->num_formats; i++) {
> > > > > if (dec_pdata->vdec_formats[i].type !=
> > > > > MTK_FMT_FRAME)
> > > > > @@ -63,7 +63,7 @@ static bool mtk_vdec_get_cap_fmt(struct
> > > > > mtk_vcodec_ctx *ctx, int format_index)
> > > > > case V4L2_PIX_FMT_H264_SLICE:
> > > > > case V4L2_PIX_FMT_VP9_FRAME:
> > > > > if (fmt->fourcc == V4L2_PIX_FMT_MM21)
> > > > > - ret = false;
> > > > > + ret = true;
> > > >
> > > > This makes the VP8 and the other cases identical, leaving
> > > > anything
> > > > that touches
> > > > MT21 as dead code. I'm not sure, cause I cannot test it, but it
> > > > should in theory
> > > > render MT8192 unusable, unless a new firmware has been submitted
> > > > to
> > > > linux-
> > > > firmware with MM21 support ?
> > > >
> > >
> > > If the firmware only support MT21 => won't exist for vp8 need to
> > > use
> > > MM21.
> >
> > And that's the issue, the scp.img for MT8192 on linux-firmware only
> > supports
> > MT21 [1]. Can you please update it to support both MM21 and MT21?
> >
> > For MT8195, only MM21 is supported in scp.img [2], but since the
> > hardware
> > supports both MM21 and MT21, the firmware should also support both.
> > So please
> > also update it on linux-firmware.
> >
> > [1]
> > https://lore.kernel.org/all/20230112204626.ciaff4amseoidybw@notapiano/
> > [2]
> > https://lore.kernel.org/all/20230112205825.wb5qcqhh5kwvyi3y@notapiano/
> >
> > Thanks,
> > Nícolas
> >
> MT8192 always use MM21 from the beginning, MT21 have not been enabled.

That's not true, and you can verify through the following:

$ curl https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/plain/mediatek/mt8192/scp.img > /lib/firmware/mediatek/mt8192/scp.img

$ sha256sum /lib/firmware/mediatek/mt8192/scp.img
fb9b4727f5e7cd82b2c63a8a2604d02e8c0f390eae2e9fd87aa1ae0797fb01b3 /lib/firmware/mediatek/mt8192/scp.img

$ v4l2-ctl -d1 --list-formats -D
Driver Info:
Driver name : mtk-vcodec-dec
Card type : MT8192 video decoder
Bus info : platform:16000000.video-codec
Driver version : 6.2.0
Capabilities : 0x84204000
Video Memory-to-Memory Multiplanar
Streaming
Extended Pix Format
Device Capabilities
Device Caps : 0x04204000
Video Memory-to-Memory Multiplanar
Streaming
Extended Pix Format
Media Driver Info:
Driver name : mtk-vcodec-dec
Model : mtk-vcodec-dec
Serial :
Bus info : platform:16000000.video-codec
Media version : 6.2.0
Hardware revision: 0x00000000 (0)
Driver version : 6.2.0
Interface Info:
ID : 0x0300000c
Type : V4L Video
Entity Info:
ID : 0x00000001 (1)
Name : mtk-vcodec-dec-source
Function : V4L2 I/O
Pad 0x01000002 : 0: Source
Link 0x02000008: to remote pad 0x1000004 of entity 'mtk-vcodec-dec-proc' (Video Decoder): Data, Enabled, Immutable
ioctl: VIDIOC_ENUM_FMT
Type: Video Capture Multiplanar

[0]: 'MT21' (Mediatek Compressed Format, compressed)

So the decoder video node is clearly only exposing MT21 in the capture pad.

Furthermore, with

$ echo module mtk_vcodec_dec +pmf > /sys/kernel/debug/dynamic_debug/control

and re-running the v4l2-ctl command, you'll see

[ 3164.998781] mtk_vcodec_dec:fops_vcodec_open: fops_vcodec_open(),212: decoder capability 740

and if you check drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h:

enum mtk_vdec_format_types {
MTK_VDEC_FORMAT_MM21 = 0x20,
MTK_VDEC_FORMAT_MT21C = 0x40,
MTK_VDEC_FORMAT_H264_SLICE = 0x100,
MTK_VDEC_FORMAT_VP8_FRAME = 0x200,
MTK_VDEC_FORMAT_VP9_FRAME = 0x400,
MTK_VCODEC_INNER_RACING = 0x20000,
};

meaning the scp.img firmware is exposing the MT21C but not the MM21 format.

So, again, the mt8192 scp.img firmware needs to be fixed.

Thanks,
Nícolas

2023-03-14 02:05:19

by Yunfei Dong (董云飞)

[permalink] [raw]

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

Hi Nicolas,

Thanks for your suggestion.
On Fri, 2023-02-10 at 10:36 -0500, Nicolas Dufresne wrote:
> Le vendredi 10 février 2023 à 13:55 +0800, Yunfei Dong a écrit :
> > In order to conver the format of capture queue from mediatek MM21
> > to
> > standard yuv420 with Libyuv, need to force capture queue format to
> > MM21 for Libyuv can't covert mediatek MT21 format at current
> > period.
>
> Please rework this text, it is hard to understand.
>
Will re-write the commit message in next version as below, could you
please help to review whether it's well to be used ?

Libyuv is one software library used to covert format. Only covert
mediatek MM21 to standard yuv420 for MT21 is compressed mode and MM21
is uncompressed mode at current period. Need to set capture queue
format to MM21 in order to use Libyuv.

> >
> > Fixes: 7501edef6b1f ("media: mediatek: vcodec: Different codec
> > using different capture format")
> > Signed-off-by: Yunfei Dong <[email protected]>
> > ---
> > changed with v1:
> > - add Fixes tag.
> > ---
> > drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 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..4f5e9c20214f 100644
> > --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
> > +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
> > @@ -41,7 +41,7 @@ static bool mtk_vdec_get_cap_fmt(struct
> > mtk_vcodec_ctx *ctx, int format_index)
> > const struct mtk_video_fmt *fmt;
> > struct mtk_q_data *q_data;
> > int num_frame_count = 0, i;
> > - bool ret = true;
> > + bool ret = false;
> >
> > for (i = 0; i < *dec_pdata->num_formats; i++) {
> > if (dec_pdata->vdec_formats[i].type != MTK_FMT_FRAME)
> > @@ -63,7 +63,7 @@ static bool mtk_vdec_get_cap_fmt(struct
> > mtk_vcodec_ctx *ctx, int format_index)
> > case V4L2_PIX_FMT_H264_SLICE:
> > case V4L2_PIX_FMT_VP9_FRAME:
> > if (fmt->fourcc == V4L2_PIX_FMT_MM21)
> > - ret = false;
> > + ret = true;
>
> This makes the VP8 and the other cases identical, leaving anything
> that touches
> MT21 as dead code. I'm not sure, cause I cannot test it, but it
> should in theory
> render MT8192 unusable, unless a new firmware has been submitted to
> linux-
> firmware with MM21 support ?
>
If the firmware only support MT21 => won't exist for vp8 need to use
MM21.

If the firmware only support MM21, will using MM21.
If the firmware support MT21 & MM21, will force VP8 to use MM21,
H264/VP9/AV1 will use MT21 before this change.

Will force all driver to use MM21 after adding this change.

Best Regards,
Yunfei Dong
> > break;
> > default:
> > ret = true;
>
>

2023-03-14 09:20:21

by Yunfei Dong (董云飞)

[permalink] [raw]

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

Hi Nicolas,

Thanks for your suggestion.
On Tue, 2023-02-28 at 17:03 -0500, Nícolas F. R. A. Prado wrote:
> On Mon, Feb 27, 2023 at 02:17:08AM +0000, Yunfei Dong (董云飞) wrote:
> > Hi Nicolas,
> >
> > Thanks for your suggestion.
> > On Wed, 2023-02-22 at 16:11 -0500, Nícolas F. R. A. Prado wrote:
> > > Hi,
> > >
> > > On Tue, Feb 14, 2023 at 02:28:04AM +0000, Yunfei Dong (董云飞)
> > > wrote:
> > > > Hi Nicolas,
> > > >
> > > > Thanks for your suggestion.
> > > > On Fri, 2023-02-10 at 10:36 -0500, Nicolas Dufresne wrote:
> > > > > Le vendredi 10 février 2023 à 13:55 +0800, Yunfei Dong a
> > > > > écrit :
>
> [..]
> > > > > > diff --git
> > > > > > a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
> > > > > > b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
> > > > > > index 641f533c417f..4f5e9c20214f 100644
> > > > > > ---
> > > > > > a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
> > > > > > +++
> > > > > > b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
> > > > > > @@ -41,7 +41,7 @@ static bool mtk_vdec_get_cap_fmt(struct
> > > > > > mtk_vcodec_ctx *ctx, int format_index)
> > > > > > const struct mtk_video_fmt *fmt;
> > > > > > struct mtk_q_data *q_data;
> > > > > > int num_frame_count = 0, i;
> > > > > > - bool ret = true;
> > > > > > + bool ret = false;
> > >
> > > This change doesn't do anything, so I'd drop it.
> > >
> >
> > This change is useful when capture fourcc is MT21 will return
> > false,
> > not support even if scp support mm21 and mt21.
>
> But you have
>
> default:
> ret = true;
> break;
>
> down below, so you're handling all cases in the switch, and the
> original value
> doesn't matter. It could even be left uninitialized.
>
> > > > > >
> > > > > > for (i = 0; i < *dec_pdata->num_formats; i++) {
> > > > > > if (dec_pdata->vdec_formats[i].type !=
> > > > > > MTK_FMT_FRAME)
> > > > > > @@ -63,7 +63,7 @@ static bool mtk_vdec_get_cap_fmt(struct
> > > > > > mtk_vcodec_ctx *ctx, int format_index)
> > > > > > case V4L2_PIX_FMT_H264_SLICE:
> > > > > > case V4L2_PIX_FMT_VP9_FRAME:
> > > > > > if (fmt->fourcc == V4L2_PIX_FMT_MM21)
> > > > > > - ret = false;
> > > > > > + ret = true;
> > > > >
> > > > > This makes the VP8 and the other cases identical, leaving
> > > > > anything
> > > > > that touches
> > > > > MT21 as dead code. I'm not sure, cause I cannot test it, but
> > > > > it
> > > > > should in theory
> > > > > render MT8192 unusable, unless a new firmware has been
> > > > > submitted
> > > > > to
> > > > > linux-
> > > > > firmware with MM21 support ?
> > > > >
> > > >
> > > > If the firmware only support MT21 => won't exist for vp8 need
> > > > to
> > > > use
> > > > MM21.
> > >
> > > And that's the issue, the scp.img for MT8192 on linux-firmware
> > > only
> > > supports
> > > MT21 [1]. Can you please update it to support both MM21 and MT21?
> > >
> > > For MT8195, only MM21 is supported in scp.img [2], but since the
> > > hardware
> > > supports both MM21 and MT21, the firmware should also support
> > > both.
> > > So please
> > > also update it on linux-firmware.
> > >
> > > [1]
> > >
https://lore.kernel.org/all/20230112204626.ciaff4amseoidybw@notapiano/
> > > [2]
> > >
https://lore.kernel.org/all/20230112205825.wb5qcqhh5kwvyi3y@notapiano/
> > >
> > > Thanks,
> > > Nícolas
> > >
> >
> > MT8192 always use MM21 from the beginning, MT21 have not been
> > enabled.
>
> That's not true, and you can verify through the following:
>
> $ curl
> https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/plain/mediatek/mt8192/scp.img__;!!CTRNKA9wMg0ARbw!i1Sq64yV1SsKzkxtFsZh4pfu8YwGA7ThO2zWtGyDRC4QDlzpmKxYJRvw3toH3oVDlHDxOpT73DZVWfe9OWeyj6if$
> $ > /lib/firmware/mediatek/mt8192/scp.img
>
I have send the patch v3 to be reviewed. And Tinghan will help to
upstream mt8192 and mt8195's firmware scp.img which support MM21 and
MT21 at the same time.

Best Regards,
Yunfei Dong
> $ sha256sum /lib/firmware/mediatek/mt8192/scp.img
> fb9b4727f5e7cd82b2c63a8a2604d02e8c0f390eae2e9fd87aa1ae0797fb01b
> 3 /lib/firmware/mediatek/mt8192/scp.img
>
> $ v4l2-ctl -d1 --list-formats -D
> Driver Info:
> Driver name : mtk-vcodec-dec
> Card type : MT8192 video decoder
> Bus info : platform:16000000.video-codec
> Driver version : 6.2.0
> Capabilities : 0x84204000
> Video Memory-to-Memory Multiplanar
> Streaming
> Extended Pix Format
> Device Capabilities
> Device Caps : 0x04204000
> Video Memory-to-Memory Multiplanar
> Streaming
> Extended Pix Format
> Media Driver Info:
> Driver name : mtk-vcodec-dec
> Model : mtk-vcodec-dec
> Serial :
> Bus info : platform:16000000.video-codec
> Media version : 6.2.0
> Hardware revision: 0x00000000 (0)
> Driver version : 6.2.0
> Interface Info:
> ID : 0x0300000c
> Type : V4L Video
> Entity Info:
> ID : 0x00000001 (1)
> Name : mtk-vcodec-dec-source
> Function : V4L2 I/O
> Pad 0x01000002 : 0: Source
> Link 0x02000008: to remote pad 0x1000004 of entity
> 'mtk-vcodec-dec-proc' (Video Decoder): Data, Enabled, Immutable
> ioctl: VIDIOC_ENUM_FMT
> Type: Video Capture Multiplanar
>
> [0]: 'MT21' (Mediatek Compressed Format, compressed)
>
>
> So the decoder video node is clearly only exposing MT21 in the
> capture pad.
>
> Furthermore, with
>
> $ echo module mtk_vcodec_dec +pmf >
> /sys/kernel/debug/dynamic_debug/control
>
> and re-running the v4l2-ctl command, you'll see
>
> [ 3164.998781] mtk_vcodec_dec:fops_vcodec_open:
> fops_vcodec_open(),212: decoder capability 740
>
> and if you check
> drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h:
>
> enum mtk_vdec_format_types {
> MTK_VDEC_FORMAT_MM21 = 0x20,
> MTK_VDEC_FORMAT_MT21C = 0x40,
> MTK_VDEC_FORMAT_H264_SLICE = 0x100,
> MTK_VDEC_FORMAT_VP8_FRAME = 0x200,
> MTK_VDEC_FORMAT_VP9_FRAME = 0x400,
> MTK_VCODEC_INNER_RACING = 0x20000,
> };
>
> meaning the scp.img firmware is exposing the MT21C but not the MM21
> format.
>
> So, again, the mt8192 scp.img firmware needs to be fixed.
>
> Thanks,
> Nícolas