Re: [PATCH v5 13/14] media: mediatek: decoder: fill av1 buffer size with picinfo

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The C plane buffer size (`vref->c.size`) is completely missing initialization for reference frames, leaving it as zero in the shared hardware structure.
--

commit a2c9dc4836cf2314c45a5c3f24634d8cb307be25
Author: Yunfei Dong <yunfei.dong@mediatek.com>

media: mediatek: decoder: fill av1 buffer size with picinfo

The buffer size of y and c plane has been calculated in vcp/scp,
and this commit fills each frame buffer size directly using picinfo.

> diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c
> --- a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c
> +++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c
> @@ -1811,18 +1811,19 @@ static int vdec_av1_slice_setup_core_buffer(struct vdec_av1_slice_instance *inst
>  {
>  	struct vb2_buffer *vb;
>  	struct vb2_queue *vq;
> -	int w, h, plane, size;
> +	int plane;
>  	int i;
>  
>  	plane = instance->ctx->q_data[MTK_Q_DATA_DST].fmt->num_planes;
> -	w = vsi->frame.uh.upscaled_width;
> -	h = vsi->frame.uh.frame_height;
> -	size = ALIGN(w, VCODEC_DEC_ALIGNED_64) * ALIGN(h, VCODEC_DEC_ALIGNED_64);
>  
>  	/* frame buffer */
>  	vsi->fb.y.dma_addr = fb->base_y.dma_addr;
> +
> +	vsi->fb.y.size = instance->ctx->picinfo.fb_sz[0];
> +	vsi->fb.c.size = instance->ctx->picinfo.fb_sz[1];
> +
>  	if (plane == 1)
> -		vsi->fb.c.dma_addr = fb->base_y.dma_addr + size;
> +		vsi->fb.c.dma_addr = fb->base_y.dma_addr + vsi->fb.y.size;
>  	else
>  		vsi->fb.c.dma_addr = fb->base_c.dma_addr;

[ ... ]

> @@ -1845,8 +1846,9 @@ static int vdec_av1_slice_setup_core_buffer(struct vdec_av1_slice_instance *inst
>  		}
>  
>  		vref->y.dma_addr = vb2_dma_contig_plane_dma_addr(vb, 0);
> +		vref->y.size = vsi->fb.y.size;
>  		if (plane == 1)
> -			vref->c.dma_addr = vref->y.dma_addr + size;
> +			vref->c.dma_addr = vref->y.dma_addr + vsi->fb.y.size;
>  		else
>  			vref->c.dma_addr = vb2_dma_contig_plane_dma_addr(vb, 1);
>  	}

[Severity: High]
Are we missing an assignment for vref->c.size here?

Earlier in vdec_av1_slice_setup_core_buffer(), vsi->fb.c.size is explicitly
initialized, but here for the reference frames only vref->y.size is assigned.

Because the underlying vsi structure is backed by zero-initialized memory,
the C plane size for all reference frames will be left as zero.

Could this missing initialization cause visual corruption or trigger a
coprocessor fault when the firmware uses these reference buffer sizes for DMA
configuration and bounds checking?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260603084045.17488-1-kyrie.wu@mediatek.com?part=13