linux-kernel - Re: [PATCH v2,1/5] media: mediatek: vcodec: fix incorrect MT2T format information

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] [thread-next>] [day] [month] [year] [list]

Date: Thu, 18 Apr 2024 16:17:46 -0400
From: Nicolas Dufresne <nicolas.dufresne@...labora.com>
To: Yunfei Dong <yunfei.dong@...iatek.com>, Nícolas "F .
 R . A . Prado" <nfraprado@...labora.com>, Hans Verkuil
 <hverkuil-cisco@...all.nl>, AngeloGioacchino Del Regno
 <angelogioacchino.delregno@...labora.com>, Benjamin Gaignard
 <benjamin.gaignard@...labora.com>, Nathan Hebert <nhebert@...omium.org>, 
 Sebastian Fricke <sebastian.fricke@...labora.com>
Cc: Hsin-Yi Wang <hsinyi@...omium.org>, Fritz Koenig
 <frkoenig@...omium.org>,  Daniel Vetter <daniel@...ll.ch>, Steve Cho
 <stevecho@...omium.org>, linux-media@...r.kernel.org, 
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, 
 linux-arm-kernel@...ts.infradead.org, linux-mediatek@...ts.infradead.org, 
 Project_Global_Chrome_Upstream_Group@...iatek.com
Subject: Re: [PATCH v2,1/5] media: mediatek: vcodec: fix incorrect MT2T
 format information

Le mardi 09 avril 2024 à 14:44 +0800, Yunfei Dong a écrit :
> Changing the bpp and hdiv values to make sure the bytesperline
> of plane[0] and plane[1] are the same. The width and height are
> 64 align.
> 
> Fixes: 6afcc2b0aebf ("media: mediatek: vcodec: Add capture format to support 10bit tile mode")
> Signed-off-by: Yunfei Dong <yunfei.dong@...iatek.com>
> ---
>  drivers/media/v4l2-core/v4l2-common.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> index d34d210908d9..8587cd14741c 100644
> --- a/drivers/media/v4l2-core/v4l2-common.c
> +++ b/drivers/media/v4l2-core/v4l2-common.c
> @@ -265,8 +265,8 @@ const struct v4l2_format_info *v4l2_format_info(u32 format)
>  		{ .format = V4L2_PIX_FMT_VYUY,    .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 1 },
>  		{ .format = V4L2_PIX_FMT_Y212,    .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 1, .bpp = { 4, 0, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 1 },
>  		{ .format = V4L2_PIX_FMT_YUV48_12, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 1, .bpp = { 6, 0, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 1, .vdiv = 1 },
> -		{ .format = V4L2_PIX_FMT_MT2110T, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 2, .comp_planes = 2, .bpp = { 5, 10, 0, 0 }, .bpp_div = { 4, 4, 1, 1 }, .hdiv = 2, .vdiv = 2,
> -		  .block_w = { 16, 8, 0, 0 }, .block_h = { 32, 16, 0, 0 }},
> +		{ .format = V4L2_PIX_FMT_MT2110T, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 2, .comp_planes = 2, .bpp = { 5, 5, 0, 0 }, .bpp_div = { 4, 4, 1, 1 }, .hdiv = 1, .vdiv = 2,
> +		  .block_w = { 64, 64, 0, 0 }, .block_h = { 64, 64, 0, 0 }},

The code that calculate the info you are fixing:

plane->bytesperline =
	info->bpp[i] * DIV_ROUND_UP(aligned_width, hdiv) / info->bpp_div[i];
plane->sizeimage = plane->bytesperline * DIV_ROUND_UP(aligned_height, vdiv);

hdiv is 1 for the first plane, and 2 for second plane. Let say we have 1920x1080
video, bytesperline for that first planes would be 5 * (1920 / 1) / 4 = 2400.
But for the second plane with your change we have 5 * (1920 / 2) / 4 = 1200. For
me, you failed to achieve your goal, or there is a bug elsewhere.

The second change should have its own commit, but I think it should be dropped.
The driver is hacked to be compatible with the Mali GPUs (64x64 textures), but
does not need this alignment. And in any case, the block should represent the
pixel format required alignment, so I'd suggest to leave it like this, and
simply align width/height before calling the function to calculate
stride/sizeimage. The 64x64 alignment can be applied using
v4l2_apply_frmsize_constraints() during try fmt (or v4l_bound_align_image(), but
first is preferred).

Same comments apply to the following patch, which should be squashed.

>  		{ .format = V4L2_PIX_FMT_MT2110R, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 2, .comp_planes = 2, .bpp = { 5, 10, 0, 0 }, .bpp_div = { 4, 4, 1, 1 }, .hdiv = 2, .vdiv = 2,
>  		  .block_w = { 16, 8, 0, 0 }, .block_h = { 32, 16, 0, 0 }},
>