lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Tue, 14 Mar 2023 09:20:00 +0000
From:   Yunfei Dong (董云飞) 
        <Yunfei.Dong@...iatek.com>
To:     "nfraprado@...labora.com" <nfraprado@...labora.com>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "nicolas@...fresne.ca" <nicolas@...fresne.ca>,
        "frkoenig@...omium.org" <frkoenig@...omium.org>,
        Tiffany Lin (林慧珊) 
        <tiffany.lin@...iatek.com>,
        "wenst@...omium.org" <wenst@...omium.org>,
        "linux-media@...r.kernel.org" <linux-media@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "stevecho@...omium.org" <stevecho@...omium.org>,
        "linux-mediatek@...ts.infradead.org" 
        <linux-mediatek@...ts.infradead.org>,
        "mchehab@...nel.org" <mchehab@...nel.org>,
        "daniel@...ll.ch" <daniel@...ll.ch>,
        Project_Global_Chrome_Upstream_Group 
        <Project_Global_Chrome_Upstream_Group@...iatek.com>,
        "benjamin.gaignard@...labora.com" <benjamin.gaignard@...labora.com>,
        "hverkuil-cisco@...all.nl" <hverkuil-cisco@...all.nl>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "hsinyi@...omium.org" <hsinyi@...omium.org>,
        "matthias.bgg@...il.com" <matthias.bgg@...il.com>,
        "angelogioacchino.delregno@...labora.com" 
        <angelogioacchino.delregno@...labora.com>
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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ