Re: [PATCH v1 7/7] media: chips-media: wave5: Resume device before setting EOS flag

[Nicolas Dufresne]

Hi,

Le vendredi 26 juin 2026 à 10:22 +0900, Jackson.lee a écrit :
> From: Jackson Lee <jackson.lee@xxxxxxxxxxxxxxx>
> 
> Setting the EOS flag talks to the firmware via send_firmware_command(),
> which accesses VPU registers. Both the STREAMOFF path
> (wave5_vpu_dec_job_abort()) and the V4L2_DEC_CMD_STOP path
> (wave5_vpu_dec_stop()) can run while the device is runtime suspended, so
> those register accesses hit powered-down hardware and the SoC raises an
> asynchronous SError, panicking the kernel:
> 
>   SError Interrupt on CPU3, code 0x00000000bf000000 -- SError
>    send_firmware_command+0x2c/0x160 [wave5]
>    wave5_vpu_dec_set_bitstream_flag+0x6c/0x80 [wave5]
>    wave5_vpu_dec_update_bitstream_buffer+0x80/0xec [wave5]
>    wave5_vpu_dec_job_abort+0x44/0xa0 [wave5]
>    v4l2_m2m_cancel_job+0x110/0x19c [v4l2_mem2mem]
>    v4l2_m2m_streamoff+0x24/0x140 [v4l2_mem2mem]
> 
> Resume the device with pm_runtime_resume_and_get() around the EOS
> firmware command and release it with pm_runtime_put_autosuspend(),
> matching the runtime PM handling already done in
> wave5_vpu_dec_device_run().
> 
> Fixes: 9707a6254a8a ("media: chips-media: wave5: Add the v4l2 layer")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Jackson Lee <jackson.lee@xxxxxxxxxxxxxxx>
> Signed-off-by: Nas Chung <nas.chung@xxxxxxxxxxxxxxx>
> ---
>  .../chips-media/wave5/wave5-vpu-dec.c         | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> index 1817b83c5884..6564cf3ec739 100644
> --- a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> @@ -823,7 +823,15 @@ static int wave5_vpu_dec_stop(struct vpu_instance *inst)
>  		 * calls do not block on a mutex while inside this spinlock.
>  		 */
>  		spin_unlock_irqrestore(&inst->state_spinlock, flags);
> +		/*
> +		 * V4L2_DEC_CMD_STOP can arrive while the device is runtime
> +		 * suspended (e.g. on pipeline teardown). Setting the EOS
> flag
> +		 * accesses VPU registers via send_firmware_command(), so the
> +		 * device must be resumed first to avoid an asynchronous
> SError.
> +		 */
> +		pm_runtime_resume_and_get(inst->dev->dev);

I'll take this one, as it improves things a lot, but please follow-up with
fixing all the missing return value heck for pm_runtime_resume_and_get().

Reviewed-by: Nicolas Dufresne <nicolas.dufresne@xxxxxxxxxxxxx>

>  		ret = wave5_vpu_dec_set_eos_on_firmware(inst);
> +		pm_runtime_put_autosuspend(inst->dev->dev);
>  		if (ret)
>  			return ret;
>  
> @@ -1797,11 +1805,22 @@ static void wave5_vpu_dec_job_abort(void *priv)
>  	if (ret)
>  		return;
>  
> +	/*
> +	 * job_abort() runs from the STREAMOFF path and may be called while
> the
> +	 * device is runtime suspended. Setting the EOS flag talks to the
> +	 * firmware (send_firmware_command() accesses VPU registers), so the
> +	 * device must be resumed first; otherwise the register access faults
> +	 * with an asynchronous SError.
> +	 */
> +	pm_runtime_resume_and_get(inst->dev->dev);
> +
>  	ret = wave5_vpu_dec_set_eos_on_firmware(inst);
>  	if (ret)
>  		dev_warn(inst->dev->dev,
>  			 "Setting EOS for the bitstream, fail: %d\n", ret);
>  
> +	pm_runtime_put_autosuspend(inst->dev->dev);
> +
>  	v4l2_m2m_job_finish(inst->v4l2_m2m_dev, m2m_ctx);
>  }
>

Attachment: signature.asc
Description: This is a digitally signed message part