Re: [PATCH v5 5/8] media: mediatek: vcodec: fix vp9 4096x2176 fail for profile2

[Nicolas Dufresne <nicolas.dufresne@collabora.com>]

[-- Attachment #1: Type: text/plain, Size: 2222 bytes --]

Hi,

Le jeudi 06 novembre 2025 à 14:13 +0800, Kyrie Wu a écrit :
> The dram addr of vp9 decoder tile number, which use dram mode
> to set tile information, may reach to 36bits for 4096x2176.
> It needs to get the highest 4bit of tile buffer address to
> set tile buffer address.

Please rework that commit message. Perhaps:

	The DRAM address of the VP9 decoder mode info (MI) buffers may require
	as much as 36bits for 4096x2176 resolution. Fold the 4 most significant
	bits into the lower (padding) four bits of address.

> 
> Fixes: 5d418351ca8f1 ("media: mediatek: vcodec: support stateless VP9 decoding")
> 
> Signed-off-by: Kyrie Wu <kyrie.wu@mediatek.com>
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---
>  .../mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.c       | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> 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 d966914db4b9..91c563c049bd 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
> @@ -1156,7 +1156,10 @@ static int vdec_vp9_slice_setup_tile_buffer:q(struct vdec_vp9_slice_instance *inst
>  			tiles->size[i][j] = size;
>  			if (tiles->mi_rows[i]) {
>  				*tb++ = (size << 3) + ((offset << 3) & 0x7f);
> -				*tb++ = pa & ~0xf;
> +				*tb = pa & ~0xf;
> +				if (IS_ENABLED(CONFIG_ARCH_DMA_ADDR_T_64BIT))
> +					*tb |= (pa >> 32) & 0xf;
> +				tb++;

You are following the style here, so no complaints, though the macros
GENMASK(3, 0) instead of 0xf and GENMASK(31, 4) instead of ~0xf is another way
to write this code.

Since this is not always obvious when the alignment padding of an address is
being reused to extend the number of bits of an address, it would be nice if you
add a comment explaining in words what his is doing.

cheers,
Nicolas

>  				*tb++ = (pa << 3) & 0x7f;
>  				mi_row = (tiles->mi_rows[i] - 1) & 0x1ff;
>  				mi_col = (tiles->mi_cols[j] - 1) & 0x3f;

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]