Re: [PATCH 2/3] media: verisilicon: program the H1 JPEG stream limit in 64-bit words

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

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

Le lundi 09 novembre 2026 à 16:43 +0300, Sergei Khomenkov via B4 Relay a écrit :
> From: Sergei Khomenkov <shomenkow@gmail.com>
> 
> The H1 JPEG encoder takes the stream buffer limit in 64-bit words. The
> same register reads back the stream size in bits when the frame is
> done, which hantro_h1_jpeg_enc_done() already divides by 8. The driver
> writes the room left in the CAPTURE plane in bytes, so the hardware may
> write up to eight times the buffer size, and a stream that does not fit
> runs past the end of the plane into whatever memory follows it.
> 
> Userspace can get there. hantro_try_fmt() keeps any non-zero CAPTURE
> sizeimage, and even the default of header_size + width * height * 2 is
> not a bound: at quality 100 a high-entropy frame takes up to about 2.75
> bytes per pixel. On a 640x480 frame the encoder wrote 230 KB past the
> end of a default-sized buffer, on a 1920x1088 frame 1.6 MB. Without an
> IOMMU in front of the encoder that is memory owned by someone else.
> 
> Write the limit in 64-bit words, rounded down to whole 64-byte bursts:
> on the RK3026 the encoder writes the stream in 64-byte bursts and
> finishes a burst it has started, so a limit that is not a multiple of 8
> words lets it write up to 56 bytes past it. Rockchip's MPP library
> programs this register the same way for H.264, in 64-bit words rounded
> down to a multiple of 8 (mpp/hal/vpu/h264e/hal_h264e_vepu1_v2.c). The
> stream starts header_size (624) bytes into the plane, so its address
> stays 8-byte aligned. A stream that does not fit now stops at the limit
> with a buffer-full interrupt, and the frame comes back with the ERROR
> flag. A plane with less than one burst after the header is left to the
> next patch.
> 
> The STM32MP25 VENC uses the same code, so it gets this change too. I
> cannot test it there, and its interrupt handler does not get the stop
> of the previous patch. rockchip_vpu2_jpeg_enc_set_buffers() writes
> VEPU_REG_STR_BUF_LIMIT in bytes the same way, and
> rockchip_vpu2_jpeg_enc_done() divides the read-back by 8 as well; I
> cannot test the VEPU2 either, so this patch leaves it alone.
> 
> 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, with CAPTURE buffers imported
> through DMABUF from a larger buffer filled with a pattern: without this
> change the encoder wrote the rest of the stream past the end of the
> plane in each of 16 overflowing frames (640x480 to 1920x1088, quality
> 100, 12 of them in default-sized planes). With this and the previous
> patch, 14 overflowing frames (640x480 and 1920x1088, four of them queued
> back to back) stopped exactly at the limit with BUFFER_FULL in the
> status, nothing past the plane changed, and frames that fit are byte for
> byte the same as before. With the limit in words but not rounded down to
> a burst, the encoder wrote up to 56 bytes past it. Not tested on other
> SoCs.

Trim this down, simple patch, simple explanation please.

Nicolas

> 
> 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_h1_jpeg_enc.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/verisilicon/hantro_h1_jpeg_enc.c b/drivers/media/platform/verisilicon/hantro_h1_jpeg_enc.c
> index 86cc1a07026f..1e548e91612b 100644
> --- a/drivers/media/platform/verisilicon/hantro_h1_jpeg_enc.c
> +++ b/drivers/media/platform/verisilicon/hantro_h1_jpeg_enc.c
> @@ -55,7 +55,15 @@ static void hantro_h1_jpeg_enc_set_buffers(struct hantro_dev *vpu,
>  	vepu_write_relaxed(vpu, vb2_dma_contig_plane_dma_addr(dst_buf, 0) +
>  				ctx->vpu_dst_fmt->header_size,
>  			   H1_REG_ADDR_OUTPUT_STREAM);
> -	vepu_write_relaxed(vpu, size_left, H1_REG_STR_BUF_LIMIT);
> +	/*
> +	 * The stream buffer limit is counted in 64-bit words, not in bytes
> +	 * (the register reads back the stream size in bits once the frame is
> +	 * done). On the RK3026 the encoder writes the stream in 64-byte
> +	 * bursts and finishes a burst it has started, so give it whole
> +	 * bursts only.
> +	 */
> +	vepu_write_relaxed(vpu, round_down(size_left, 64) / 8,
> +			   H1_REG_STR_BUF_LIMIT);
>  
>  	if (pix_fmt->num_planes == 1) {
>  		src[0] = vb2_dma_contig_plane_dma_addr(src_buf, 0);

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