RE: [PATCH 3/4] media: wave5: handle runtime resume failure when closing

[jackson.lee]

Hi Jiale

> -----Original Message-----
> From: Jiale Yao <yaojiale02@xxxxxxx>
> Sent: Saturday, October 3, 2026 5:09 PM
> To: Nas Chung <nas.chung@xxxxxxxxxxxxxxx>; jackson.lee
> <jackson.lee@xxxxxxxxxxxxxxx>; Mauro Carvalho Chehab <mchehab@xxxxxxxxxx>;
> Nicolas Dufresne <nicolas.dufresne@xxxxxxxxxxxxx>; Sebastian Fricke
> <sebastian.fricke@xxxxxxxxxxxxx>; Hans Verkuil <hverkuil@xxxxxxxxxx>;
> linux-media@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Cc: Jiale Yao <yaojiale02@xxxxxxx>
> Subject: [PATCH 3/4] media: wave5: handle runtime resume failure when
> closing
> 
> The decoder and encoder close paths ignore runtime resume failures and
> proceed to firmware commands. If enabling the VPU clocks failed, those
> commands access unavailable registers, and the paths later drop a runtime
> PM reference that was not acquired.
> 
> Return the resume error before taking the hardware lock or issuing
> firmware commands.
> 
> Fixes: 2092b3833487 ("media: chips-media: wave5: Support runtime
> suspend/resume")
> Signed-off-by: Jiale Yao <yaojiale02@xxxxxxx>
> ---
>  drivers/media/platform/chips-media/wave5/wave5-vpuapi.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
> b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
> index f77abd5e122a..42bc737c6980 100644
> --- a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
> +++ b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
> @@ -220,7 +220,9 @@ int wave5_vpu_dec_close(struct vpu_instance *inst, u32
> *fail_res)
>  	if (!inst->codec_info)
>  		return -EINVAL;
> 
> -	pm_runtime_resume_and_get(inst->dev->dev);
> +	ret = pm_runtime_resume_and_get(inst->dev->dev);
> +	if (ret < 0)
> +		return ret;


Skipping finish_seq when the device cannot be resumed is right, but
returning here leaks the instance. wave5_vpu_release_device() returns
early for any error other than -EIO, without calling
wave5_cleanup_instance(). By then the m2m context has already been
released and the instance removed from the device list, and the VFS
ignores the return value of ->release(). So inst, codec_info, the ida
id, the kfifo, the control handler, the fh and the bitstream buffer
all leak, together with vb_work, vb_task and the auxiliary buffers
that the close functions free.

When the runtime resume callback fails, rpm_resume() stores the error
in dev->power.runtime_error, and every later pm_runtime_resume_and_get()
returns -EINVAL until the PM status is reset. So after the first
failure, every later close leaks the same way.

On a resume failure, could the close functions skip only the firmware
part (the finish_seq loop and the hw_lock), still free the host-side
memory they own, and return -EIO so that release_device() goes on to
wave5_cleanup_instance()? The same applies to wave5_vpu_enc_close().

Thanks
Jackson

> 
>  	ret_mutex = mutex_lock_interruptible(&vpu_dev->hw_lock);
>  	if (ret_mutex) {
> @@ -750,7 +752,9 @@ int wave5_vpu_enc_close(struct vpu_instance *inst, u32
> *fail_res)
>  	if (!inst->codec_info)
>  		return -EINVAL;
> 
> -	pm_runtime_resume_and_get(inst->dev->dev);
> +	ret = pm_runtime_resume_and_get(inst->dev->dev);
> +	if (ret < 0)
> +		return ret;


> 
>  	ret = mutex_lock_interruptible(&vpu_dev->hw_lock);
>  	if (ret) {
> --
> 2.34.1