linux-kernel - Re: [PATCH v4 4/8] media: mediatek: vcodec: Add core-only VP9 decoding support for MT8189

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]

Message-ID: <f5956178a0e5d91dabc12e89f666eac2140f141e.camel@collabora.com>
Date: Thu, 16 Oct 2025 11:19:18 -0400
From: Nicolas Dufresne <nicolas.dufresne@...labora.com>
To: Kyrie Wu <kyrie.wu@...iatek.com>, Tiffany Lin
 <tiffany.lin@...iatek.com>,  Andrew-CT Chen <andrew-ct.chen@...iatek.com>,
 Yunfei Dong <yunfei.dong@...iatek.com>, Mauro Carvalho Chehab	
 <mchehab@...nel.org>, Rob Herring <robh@...nel.org>, Krzysztof Kozlowski	
 <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, Matthias Brugger	
 <matthias.bgg@...il.com>, AngeloGioacchino Del Regno	
 <angelogioacchino.delregno@...labora.com>, Hans Verkuil
 <hverkuil@...all.nl>,  Christophe JAILLET <christophe.jaillet@...adoo.fr>,
 Sebastian Fricke <sebastian.fricke@...labora.com>, Nathan Hebert	
 <nhebert@...omium.org>, Arnd Bergmann <arnd@...db.de>, Irui Wang	
 <irui.wang@...iatek.com>, George Sun <george.sun@...iatek.com>, 
	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
Cc: Neil Armstrong <neil.armstrong@...aro.org>, Andrzej Pietrasiewicz
	 <andrzejtp2010@...il.com>
Subject: Re: [PATCH v4 4/8] media: mediatek: vcodec: Add core-only VP9
 decoding support for MT8189

Hi,

Le jeudi 16 octobre 2025 à 14:07 +0800, Kyrie Wu a écrit :
> Implemented core-only VP9 decoding functions for MT8189.

What does "core-only" means ? Did you mean single core ?

> 
> Signed-off-by: Kyrie Wu <kyrie.wu@...iatek.com>
> ---
>  .../vcodec/decoder/vdec/vdec_vp9_req_lat_if.c | 27 +++++++++++--------
>  1 file changed, 16 insertions(+), 11 deletions(-)
> 
> diff --git
> a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.c
> b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.c
> index fa0f406f7726..04197164fb82 100644
> ---
> a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.c
> +++
> b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.c
> @@ -23,6 +23,7 @@
>  
>  #define VP9_TILE_BUF_SIZE 4096
>  #define VP9_PROB_BUF_SIZE 2560
> +#define VP9_PROB_BUF_4K_SIZE 3840
>  #define VP9_COUNTS_BUF_SIZE 16384
>  
>  #define HDR_FLAG(x) (!!((hdr)->flags & V4L2_VP9_FRAME_FLAG_##x))
> @@ -616,7 +617,10 @@ static int vdec_vp9_slice_alloc_working_buffer(struct
> vdec_vp9_slice_instance *i
>  	}
>  
>  	if (!instance->prob.va) {
> -		instance->prob.size = VP9_PROB_BUF_SIZE;
> +		instance->prob.size = ((ctx->dev->chip_name ==
> MTK_VDEC_MT8196) ||
> +				       (ctx->dev->chip_name ==
> MTK_VDEC_MT8189)) ?
> +					VP9_PROB_BUF_4K_SIZE :
> VP9_PROB_BUF_SIZE;

I feel like this will keep growing, then you'll move to 8K and it will continue.
You already match every SoC in the driver, you should come up with SoC
configuration data structure so you don't have to add doc check conditions all
over the place. This change is also not reflected in the commit message.

> +
>  		if (mtk_vcodec_mem_alloc(ctx, &instance->prob))
>  			goto err;
>  	}
> @@ -696,21 +700,22 @@ static int vdec_vp9_slice_tile_offset(int idx, int
> mi_num, int tile_log2)
>  	return min(offset, mi_num);
>  }
>  
> -static
> -int vdec_vp9_slice_setup_single_from_src_to_dst(struct
> vdec_vp9_slice_instance *instance)
> +static int vdec_vp9_slice_setup_single_from_src_to_dst(struct
> vdec_vp9_slice_instance *instance,
> +						       struct mtk_vcodec_mem
> *bs,
> +						       struct vdec_fb *fb)
>  {
> -	struct vb2_v4l2_buffer *src;
> -	struct vb2_v4l2_buffer *dst;
> +	struct mtk_video_dec_buf *src_buf_info;
> +	struct mtk_video_dec_buf *dst_buf_info;
>  
> -	src = v4l2_m2m_next_src_buf(instance->ctx->m2m_ctx);
> -	if (!src)
> +	src_buf_info = container_of(bs, struct mtk_video_dec_buf, bs_buffer);
> +	if (!src_buf_info)
>  		return -EINVAL;
>  
> -	dst = v4l2_m2m_next_dst_buf(instance->ctx->m2m_ctx);
> -	if (!dst)
> +	dst_buf_info = container_of(fb, struct mtk_video_dec_buf,
> frame_buffer);
> +	if (!dst_buf_info)
>  		return -EINVAL;
>  
> -	v4l2_m2m_buf_copy_metadata(src, dst, true);
> +	v4l2_m2m_buf_copy_metadata(&src_buf_info->m2m_buf.vb, &dst_buf_info-
> >m2m_buf.vb, true);
>  
>  	return 0;
>  }
> @@ -1800,7 +1805,7 @@ static int vdec_vp9_slice_setup_single(struct
> vdec_vp9_slice_instance *instance,
>  	struct vdec_vp9_slice_vsi *vsi = &pfc->vsi;
>  	int ret;
>  
> -	ret = vdec_vp9_slice_setup_single_from_src_to_dst(instance);
> +	ret = vdec_vp9_slice_setup_single_from_src_to_dst(instance, bs, fb);

This entire change is not explained in the commit message at all. Explain why
this is needed, what difference it makes. There is no clear indication we are in
an MT8189 code path, so this change could have a incidence on all single core
SoC (if any).

Nicolas

>  	if (ret)
>  		goto err;
>  

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)