Re: [PATCH 1/3] media: verisilicon: stop the VEPU1 JPEG encoder after an unfinished frame

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

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

Le lundi 09 novembre 2026 à 16:43 +0300, Sergei Khomenkov via B4 Relay a écrit :
> From: Sergei Khomenkov <shomenkow@gmail.com>
> 
> When a JPEG stream does not fit the CAPTURE buffer, the H1 raises an
> interrupt with BUFFER_FULL set and FRAME_RDY clear, and pauses with the
> enable bit of H1_REG_ENC_CTRL still set, waiting for more output space.
> rockchip_vpu1_vepu_irq() returns the buffer with the ERROR flag but
> clears only the interrupt and AXI registers, so the next job is
> programmed into the paused encoder: its ENC_CTRL write aborts the paused
> frame, which raises a reset interrupt while the new job is being set up,
> and that job times out. The watchdog then calls
> rockchip_vpu1_enc_reset(), which sets H1_REG_INTERRUPT_DIS_BIT, and the
> JPEG encoder path never clears that bit again, so every later job times
> out as well. That second problem is not addressed here.
> 
> Write 0 to the enable register when an interrupt does not report a
> finished frame. The write comes before the interrupt and AXI registers
> are cleared, and before the next job is scheduled. On the RK3026 it
> raises no interrupt of its own: in the test below the encoder raised one
> interrupt per frame and no job timed out.
> 
> The encoder rarely gets there today, because the stream limit it is
> given is eight times too large; the next patch fixes that, and then
> every frame that does not fit ends this way.
> 
> rockchip_vpu2_vepu_irq() and stm32mp25_venc_irq() do not stop the
> encoder either. I cannot test the VEPU2 or the STM32MP25, so this patch
> leaves them 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 the stream limit fixed: without
> this change the job after a buffer-full interrupt gets a reset interrupt
> and every later job times out; with it the frame that does not fit comes
> back with the ERROR flag and the following jobs, overflowing or not,
> complete as before (10 alternating jobs, and four overflowing frames
> queued back to back followed by fitting ones). Counted in
> /proc/interrupts over 24 runs of the series (48 frames, 16 of them
> ending in a buffer full), every frame raised exactly one interrupt, so
> the stop write raises none. Only buffer full was exercised, not bus
> errors or the hardware timeout. Not tested on other SoCs.

Please trim this down to what is really needed.

> 
> 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/rockchip_vpu_hw.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/media/platform/verisilicon/rockchip_vpu_hw.c b/drivers/media/platform/verisilicon/rockchip_vpu_hw.c
> index 02673be9878e..144d320302a1 100644
> --- a/drivers/media/platform/verisilicon/rockchip_vpu_hw.c
> +++ b/drivers/media/platform/verisilicon/rockchip_vpu_hw.c
> @@ -376,6 +376,15 @@ static irqreturn_t rockchip_vpu1_vepu_irq(int irq, void *dev_id)
>  	state = (status & H1_REG_INTERRUPT_FRAME_RDY) ?
>  		VB2_BUF_STATE_DONE : VB2_BUF_STATE_ERROR;
>  
> +	/*
> +	 * An interrupt without FRAME_RDY does not end the frame: after a
> +	 * buffer full the encoder pauses with its enable bit still set,
> +	 * waiting for more output space. Stop it, so that the next job is
> +	 * not programmed into a paused encoder.
> +	 */
> +	if (!(status & H1_REG_INTERRUPT_FRAME_RDY))
> +		vepu_write(vpu, 0, H1_REG_ENC_CTRL);

Can you follow other similar patches (G2), and add some rate limited debug so we
are ware when a fault occured, and what fault it is. Its also best practice to
only reset the bit you care about, Hantro IPs have all sort of side effect per
bits.

Nicolas

> +
>  	vepu_write(vpu, 0, H1_REG_INTERRUPT);
>  	vepu_write(vpu, 0, H1_REG_AXI_CTRL);
>  

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