Re: [PATCH 3/3] media: verisilicon: leave room for the JPEG header and one stream burst

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

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

Le lundi 09 novembre 2026 à 16:43 +0300, Sergei Khomenkov via B4 Relay a écrit :
> From: Sergei Khomenkov <shomenkow@gmail.com>
> 
> hantro_try_fmt() keeps any non-zero sizeimage on the JPEG encoder's
> CAPTURE queue, however small, and hantro_buf_prepare() then accepts a
> plane of that size. hantro_jpeg_header_assemble() writes the 624-byte
> header into the plane with the CPU before the hardware starts, and a
> plane smaller than the header also makes the WARN_ON() in
> hantro_h1_jpeg_enc_set_buffers() and
> rockchip_vpu2_jpeg_enc_set_buffers() fire on every job, which userspace
> can trigger at will. After the header the encoder writes its stream in
> bursts, one of them even when the stream limit is 0; on the RK3026
> VEPU1 a burst is 64 bytes. A plane smaller than the header plus 64 bytes
> is therefore written past its end.
> 
> Raise a smaller request to header_size + 64, so that every plane the
> driver accepts holds the header and at least one such burst.
> 
> Found with an LLM coding assistant while porting mainline Linux to a
> Rockchip RK3026 board (VEPU1); the assistant also drafted the code and
> this message. Tested on that board: a 640x480 CAPTURE format set with
> sizeimage 1, 600, 656 and 689 reads back 688, 688, 688 and 689, a
> 656-byte DMABUF plane is then refused with EINVAL, and frames that do
> not fit a 688-byte plane stop exactly at its end and come back with the
> ERROR flag. A request of 600, now raised to 688, encodes with no
> warning; before this change it was kept, and the encode hit the
> WARN_ON() at hantro_h1_jpeg_enc.c:50 (hantro_h1_jpeg_enc_set_buffers(),
> reported in hantro_h1_jpeg_enc_run()). Also before it, with 32 and 8
> bytes after the header (a stream limit of 0), the encoder wrote one
> burst, 32 and 56 bytes past the plane. The burst size is measured there
> only; not tested on other SoCs.

Trim this.

> 
> Fixes: 775fec69008d ("media: add Rockchip VPU JPEG encoder driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Sergei Khomenkov <shomenkow@gmail.com>
> ---
>  drivers/media/platform/verisilicon/hantro_v4l2.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/media/platform/verisilicon/hantro_v4l2.c b/drivers/media/platform/verisilicon/hantro_v4l2.c
> index 83af9fa1ce94..cc35beac8338 100644
> --- a/drivers/media/platform/verisilicon/hantro_v4l2.c
> +++ b/drivers/media/platform/verisilicon/hantro_v4l2.c
> @@ -394,6 +394,16 @@ static int hantro_try_fmt(const struct hantro_ctx *ctx,
>  		 */
>  		pix_mp->plane_fmt[0].sizeimage = fmt->header_size +
>  			pix_mp->width * pix_mp->height * fmt->max_depth;
> +	} else if (fmt->codec_mode == HANTRO_MODE_JPEG_ENC) {
> +		/*
> +		 * The JPEG encoder writes the header with the CPU, then the
> +		 * stream in bursts, one of them even when the stream limit
> +		 * is 0. Leave room for both; a burst is 64 bytes on the
> +		 * RK3026 VEPU1.
> +		 */
> +		pix_mp->plane_fmt[0].sizeimage =
> +			max_t(u32, pix_mp->plane_fmt[0].sizeimage,
> +			      fmt->header_size + 64);

Did you mean to align to 64 instead ?

>  	}
>  
>  	return 0;

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