[PATCH v2 0/4] media: wave5: fix runtime PM error handling

[PATCH v2 0/4] media: wave5: fix runtime PM error handling

[Jiale Yao]

The Wave5 decoder, encoder, and close paths assume that
pm_runtime_resume_and_get() always succeeds. Runtime resume can fail while
enabling the VPU clocks, after which firmware commands can access unavailable
registers and cleanup paths can drop references that were not acquired.

Check runtime resume results before hardware access. On errors from callbacks
that cannot return an error, return queued buffers and complete the active
job. When closing an instance, still release host-side resources and return
-EIO so the common release path completes instance cleanup.

media-next already checks the decoder destination queue resume result, but
uses vb2_buffer_done() instead of the mem2mem completion helper. This series
keeps the remaining decoder resume fixes separate from that interface fix.

Jiale Yao (4):
  media: wave5: handle remaining decoder runtime resume failures
  media: wave5: use m2m buffer completion helper
  media: wave5: handle encoder runtime resume failures
  media: wave5: handle runtime resume failure when closing

---
Changes in v2:
  - Rebase on media-next.
  - Split the decoder changes: fix the remaining unchecked resume results
    separately from the existing destination-buffer completion path.
  - Use v4l2_m2m_buf_done() for the destination-buffer completion path.
  - Stop the encoder and fail both queues on a runtime resume failure to
    prevent the mem2mem scheduler from repeatedly running a failed job.
  - Rebase the close-path cleanup on the already accepted runtime PM
    reference leak fix.
  - Drop v1 patch 4 because its runtime PM reference leak fix is already
    accepted upstream.

 .../chips-media/wave5/wave5-vpu-dec.c         | 49 ++++++++++++++++---
 .../chips-media/wave5/wave5-vpu-enc.c         | 33 +++++++++++--
 .../platform/chips-media/wave5/wave5-vpuapi.c | 18 ++++++-
 3 files changed, 89 insertions(+), 11 deletions(-)

-- 
2.34.1

[PATCH v2 1/4] media: wave5: handle remaining decoder runtime resume failures

[Jiale Yao]

Several decoder callbacks continue into firmware commands after
pm_runtime_resume_and_get() fails. The runtime resume callback can fail
while enabling the VPU clocks, so those commands may access registers
while the device is unavailable. The matching runtime PM puts can also
be issued without a reference.

Check the remaining resume results. Propagate errors from callbacks that
can return them, complete buffers or jobs from void callbacks, and avoid
firmware access and unmatched puts on failure.

Fixes: 2092b3833487 ("media: chips-media: wave5: Support runtime suspend/resume")
Fixes: cbb9c0d50e47 ("media: chips-media: wave5: Fix SError of kernel panic when closed")
Fixes: a52e6f7923c1 ("media: chips-media: wave5: Resume device before setting EOS flag")
Signed-off-by: Jiale Yao <yaojiale02@163.com>
---
 .../chips-media/wave5/wave5-vpu-dec.c         | 47 +++++++++++++++++--
 1 file changed, 42 insertions(+), 5 deletions(-)

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 86bc36ee17..320b40581b 100644
--- a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
+++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
@@ -829,7 +829,10 @@ static int wave5_vpu_dec_stop(struct vpu_instance *inst)
 		 * 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);
+		ret = pm_runtime_resume_and_get(inst->dev->dev);
+		if (ret < 0)
+			return ret;
+
 		ret = wave5_vpu_dec_set_eos_on_firmware(inst);
 		pm_runtime_put_autosuspend(inst->dev->dev);
 		if (ret)
@@ -1392,7 +1395,11 @@ static int wave5_vpu_dec_start_streaming(struct vb2_queue *q, unsigned int count
 	int ret = 0;
 
 	dev_dbg(inst->dev->dev, "%s: type: %u\n", __func__, q->type);
-	pm_runtime_resume_and_get(inst->dev->dev);
+	ret = pm_runtime_resume_and_get(inst->dev->dev);
+	if (ret < 0) {
+		wave5_return_bufs(q, VB2_BUF_STATE_QUEUED);
+		return ret;
+	}
 
 	v4l2_m2m_update_start_streaming_state(m2m_ctx, q);
 
@@ -1556,9 +1563,28 @@ static void wave5_vpu_dec_stop_streaming(struct vb2_queue *q)
 	struct vpu_instance *inst = vb2_get_drv_priv(q);
 	struct v4l2_m2m_ctx *m2m_ctx = inst->v4l2_fh.m2m_ctx;
 	unsigned long timeout;
+	int ret;
 
 	dev_dbg(inst->dev->dev, "%s: type: %u\n", __func__, q->type);
-	pm_runtime_resume_and_get(inst->dev->dev);
+	ret = pm_runtime_resume_and_get(inst->dev->dev);
+	if (ret < 0) {
+		if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
+			struct vpu_src_buffer *vpu_buf;
+
+			inst->retry = false;
+			inst->queuing_num = 0;
+			while ((vpu_buf = inst_src_buf_remove(inst)) != NULL)
+				;
+			inst->eos = false;
+		}
+
+		v4l2_m2m_update_stop_streaming_state(m2m_ctx, q);
+		wave5_return_bufs(q, VB2_BUF_STATE_ERROR);
+		inst->empty_queue = false;
+		inst->sent_eos = false;
+		return;
+	}
+
 	inst->empty_queue = true;
 
 	timeout = jiffies + msecs_to_jiffies(VPU_DEC_STOP_TIMEOUT);
@@ -1675,7 +1701,13 @@ static void wave5_vpu_dec_device_run(void *priv)
 	bool cmd_issued = false;
 
 	dev_dbg(inst->dev->dev, "%s: Fill the ring buffer with new bitstream data", __func__);
-	pm_runtime_resume_and_get(inst->dev->dev);
+	ret = pm_runtime_resume_and_get(inst->dev->dev);
+	if (ret < 0) {
+		dev_err(inst->dev->dev, "Failed to resume VPU: %d\n", ret);
+		v4l2_m2m_job_finish(inst->v4l2_m2m_dev, m2m_ctx);
+		return;
+	}
+
 	if (!inst->retry) {
 		ret = fill_ringbuffer(inst);
 		if (ret < 0) {
@@ -1818,7 +1850,11 @@ static void wave5_vpu_dec_job_abort(void *priv)
 	 * device must be resumed first; otherwise the register access faults
 	 * with an asynchronous SError.
 	 */
-	pm_runtime_resume_and_get(inst->dev->dev);
+	ret = pm_runtime_resume_and_get(inst->dev->dev);
+	if (ret < 0) {
+		dev_warn(inst->dev->dev, "Failed to resume VPU: %d\n", ret);
+		goto finish_job;
+	}
 
 	ret = wave5_vpu_dec_set_eos_on_firmware(inst);
 	if (ret)
@@ -1827,6 +1863,7 @@ static void wave5_vpu_dec_job_abort(void *priv)
 
 	pm_runtime_put_autosuspend(inst->dev->dev);
 
+finish_job:
 	v4l2_m2m_job_finish(inst->v4l2_m2m_dev, m2m_ctx);
 }
 
-- 
2.34.1

[PATCH v2 2/4] media: wave5: use m2m buffer completion helper

[Jiale Yao]

The decoder destination queue handles a runtime resume failure by
completing the queued buffer. The driver is a mem2mem driver, so use
v4l2_m2m_buf_done() to preserve mem2mem queue bookkeeping instead of
calling vb2_buffer_done() directly.

Signed-off-by: Jiale Yao <yaojiale02@163.com>
---
 drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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 320b40581b..467c68931e 100644
--- a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
+++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
@@ -1309,7 +1309,7 @@ static void wave5_vpu_dec_buf_queue_dst(struct vb2_buffer *vb)
 
 	ret = pm_runtime_resume_and_get(inst->dev->dev);
 	if (ret < 0) {
-		vb2_buffer_done(vb, VB2_BUF_STATE_ERROR);
+		v4l2_m2m_buf_done(vbuf, VB2_BUF_STATE_ERROR);
 		return;
 	}
 
-- 
2.34.1

[PATCH v2 3/4] media: wave5: handle encoder runtime resume failures

[Jiale Yao]

The encoder callbacks ignore failures from pm_runtime_resume_and_get()
and continue issuing firmware commands. Runtime resume can fail while
enabling the VPU clocks, leaving the registers inaccessible. The
callbacks can then drop a runtime PM reference that was not acquired.

Runtime PM retains a resume error, so simply finishing the current job
leaves PIC_RUN runnable and causes the mem2mem scheduler to retry the
same failing job. On a resume failure, stop the instance, mark both
queues in error, return their queued buffers as errors, and finish the
job. Propagate the error from start_streaming() and preserve buffer
cleanup in the other callbacks without accessing the hardware or
issuing an unmatched put.

Fixes: 2092b3833487 ("media: chips-media: wave5: Support runtime suspend/resume")
Signed-off-by: Jiale Yao <yaojiale02@163.com>
---
 .../chips-media/wave5/wave5-vpu-enc.c         | 33 +++++++++++++++++--
 1 file changed, 30 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c b/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
index f9fcdf4c22..7359f86f70 100644
--- a/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
+++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
@@ -1351,7 +1351,12 @@ static int wave5_vpu_enc_start_streaming(struct vb2_queue *q, unsigned int count
 	struct v4l2_m2m_ctx *m2m_ctx = inst->v4l2_fh.m2m_ctx;
 	int ret = 0;
 
-	pm_runtime_resume_and_get(inst->dev->dev);
+	ret = pm_runtime_resume_and_get(inst->dev->dev);
+	if (ret < 0) {
+		wave5_return_bufs(q, VB2_BUF_STATE_QUEUED);
+		return ret;
+	}
+
 	v4l2_m2m_update_start_streaming_state(m2m_ctx, q);
 
 	if (inst->state == VPU_INST_STATE_NONE && q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
@@ -1451,6 +1456,7 @@ static void wave5_vpu_enc_stop_streaming(struct vb2_queue *q)
 {
 	struct vpu_instance *inst = vb2_get_drv_priv(q);
 	bool check_cmd = true;
+	int ret;
 
 	/*
 	 * Note that we don't need m2m_ctx->next_buf_last for this driver, so we
@@ -1458,7 +1464,14 @@ static void wave5_vpu_enc_stop_streaming(struct vb2_queue *q)
 	 */
 
 	dev_dbg(inst->dev->dev, "%s: type: %u\n", __func__, q->type);
-	pm_runtime_resume_and_get(inst->dev->dev);
+	ret = pm_runtime_resume_and_get(inst->dev->dev);
+	if (ret < 0) {
+		if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
+			streamoff_output(inst, q);
+		else
+			streamoff_capture(inst, q);
+		return;
+	}
 
 	if (wave5_vpu_both_queues_are_streaming(inst))
 		switch_state(inst, VPU_INST_STATE_STOP);
@@ -1526,7 +1539,21 @@ static void wave5_vpu_enc_device_run(void *priv)
 	u32 fail_res = 0;
 	int ret = 0;
 
-	pm_runtime_resume_and_get(inst->dev->dev);
+	ret = pm_runtime_resume_and_get(inst->dev->dev);
+	if (ret < 0) {
+		struct vb2_queue *src_vq = v4l2_m2m_get_src_vq(m2m_ctx);
+		struct vb2_queue *dst_vq = v4l2_m2m_get_dst_vq(m2m_ctx);
+
+		dev_err(inst->dev->dev, "Failed to resume VPU: %d\n", ret);
+		switch_state(inst, VPU_INST_STATE_STOP);
+		vb2_queue_error(src_vq);
+		vb2_queue_error(dst_vq);
+		wave5_return_bufs(src_vq, VB2_BUF_STATE_ERROR);
+		wave5_return_bufs(dst_vq, VB2_BUF_STATE_ERROR);
+		v4l2_m2m_job_finish(inst->v4l2_m2m_dev, m2m_ctx);
+		return;
+	}
+
 	switch (inst->state) {
 	case VPU_INST_STATE_PIC_RUN:
 		ret = start_encode(inst, &fail_res);
-- 
2.34.1

RE: [PATCH v2 3/4] media: wave5: handle encoder runtime resume failures

[jackson.lee]

Hi Jiale

Thanks for the update. The device_run() error path looks good to me now.

> -----Original Message-----
> From: Jiale Yao <yaojiale02@163.com>
> Sent: Monday, October 19, 2026 5:20 PM
> To: Nas Chung <nas.chung@chipsnmedia.com>; jackson.lee
> <jackson.lee@chipsnmedia.com>; Mauro Carvalho Chehab <mchehab@kernel.org>;
> Nicolas Dufresne <nicolas.dufresne@collabora.com>; Sebastian Fricke
> <sebastian.fricke@collabora.com>; Hans Verkuil <hverkuil@kernel.org>;
> linux-media@vger.kernel.org; linux-kernel@vger.kernel.org
> Cc: Jiale Yao <yaojiale02@163.com>
> Subject: [PATCH v2 3/4] media: wave5: handle encoder runtime resume
> failures
> 
> The encoder callbacks ignore failures from pm_runtime_resume_and_get() and
> continue issuing firmware commands. Runtime resume can fail while enabling
> the VPU clocks, leaving the registers inaccessible. The callbacks can then
> drop a runtime PM reference that was not acquired.
> 
> Runtime PM retains a resume error, so simply finishing the current job
> leaves PIC_RUN runnable and causes the mem2mem scheduler to retry the same
> failing job. On a resume failure, stop the instance, mark both queues in
> error, return their queued buffers as errors, and finish the job.
> Propagate the error from start_streaming() and preserve buffer cleanup in
> the other callbacks without accessing the hardware or issuing an unmatched
> put.
> 
> Fixes: 2092b3833487 ("media: chips-media: wave5: Support runtime
> suspend/resume")
> Signed-off-by: Jiale Yao <yaojiale02@163.com>
> ---
>  .../chips-media/wave5/wave5-vpu-enc.c         | 33 +++++++++++++++++--
>  1 file changed, 30 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
> b/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
> index f9fcdf4c22..7359f86f70 100644
> --- a/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
> +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
> @@ -1351,7 +1351,12 @@ static int wave5_vpu_enc_start_streaming(struct
> vb2_queue *q, unsigned int count
>  	struct v4l2_m2m_ctx *m2m_ctx = inst->v4l2_fh.m2m_ctx;
>  	int ret = 0;
> 
> -	pm_runtime_resume_and_get(inst->dev->dev);
> +	ret = pm_runtime_resume_and_get(inst->dev->dev);
> +	if (ret < 0) {
> +		wave5_return_bufs(q, VB2_BUF_STATE_QUEUED);
> +		return ret;
> +	}
> +
>  	v4l2_m2m_update_start_streaming_state(m2m_ctx, q);
> 
>  	if (inst->state == VPU_INST_STATE_NONE && q->type ==
> V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) { @@ -1451,6 +1456,7 @@ static void
> wave5_vpu_enc_stop_streaming(struct vb2_queue *q)  {
>  	struct vpu_instance *inst = vb2_get_drv_priv(q);
>  	bool check_cmd = true;
> +	int ret;
> 
>  	/*
>  	 * Note that we don't need m2m_ctx->next_buf_last for this driver,
> so we @@ -1458,7 +1464,14 @@ static void
> wave5_vpu_enc_stop_streaming(struct vb2_queue *q)
>  	 */
> 
>  	dev_dbg(inst->dev->dev, "%s: type: %u\n", __func__, q->type);
> -	pm_runtime_resume_and_get(inst->dev->dev);
> +	ret = pm_runtime_resume_and_get(inst->dev->dev);
> +	if (ret < 0) {
> +		if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
> +			streamoff_output(inst, q);
> +		else
> +			streamoff_capture(inst, q);
> +		return;
> +	}
> 
>  	if (wave5_vpu_both_queues_are_streaming(inst))
>  		switch_state(inst, VPU_INST_STATE_STOP); @@ -1526,7 +1539,21


One small suggestion for stop_streaming(): 
switch_state(inst, VPU_INST_STATE_STOP) only touches software state, 
so it could move before pm_runtime_resume_and_get() and run on both paths. Otherwise the instance stays in PIC_RUN if only the stop_streaming() resume fails.

Reviewed-by: Jackson Lee jackson.lee@chipsnmedia.com

Thanks,
Jackson



> @@ static void wave5_vpu_enc_device_run(void *priv)
>  	u32 fail_res = 0;
>  	int ret = 0;
> 
> -	pm_runtime_resume_and_get(inst->dev->dev);
> +	ret = pm_runtime_resume_and_get(inst->dev->dev);
> +	if (ret < 0) {
> +		struct vb2_queue *src_vq = v4l2_m2m_get_src_vq(m2m_ctx);
> +		struct vb2_queue *dst_vq = v4l2_m2m_get_dst_vq(m2m_ctx);
> +
> +		dev_err(inst->dev->dev, "Failed to resume VPU: %d\n", ret);
> +		switch_state(inst, VPU_INST_STATE_STOP);
> +		vb2_queue_error(src_vq);
> +		vb2_queue_error(dst_vq);
> +		wave5_return_bufs(src_vq, VB2_BUF_STATE_ERROR);
> +		wave5_return_bufs(dst_vq, VB2_BUF_STATE_ERROR);
> +		v4l2_m2m_job_finish(inst->v4l2_m2m_dev, m2m_ctx);
> +		return;
> +	}
> +
>  	switch (inst->state) {
>  	case VPU_INST_STATE_PIC_RUN:
>  		ret = start_encode(inst, &fail_res);
> --
> 2.34.1

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

[Jiale Yao]

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.

Returning the resume error directly also prevents release_device() from
calling wave5_cleanup_instance(). The instance has already been removed
from the device list and its mem2mem context released, so its host-side
resources leak.

On a resume failure, skip the firmware command and hardware lock, free
the close-path resources, and return -EIO. This lets release_device()
continue with its common instance cleanup without issuing an unmatched
runtime PM put.

Fixes: 2092b3833487 ("media: chips-media: wave5: Support runtime suspend/resume")
Signed-off-by: Jiale Yao <yaojiale02@163.com>
---
 .../platform/chips-media/wave5/wave5-vpuapi.c  | 18 ++++++++++++++++--
 1 file changed, 16 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 4c1842f02c..806901e62d 100644
--- a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
+++ b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
@@ -220,7 +220,11 @@ 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) {
+		ret = -EIO;
+		goto free_resources;
+	}
 
 	ret_mutex = mutex_lock_interruptible(&vpu_dev->hw_lock);
 	if (ret_mutex) {
@@ -259,6 +263,7 @@ int wave5_vpu_dec_close(struct vpu_instance *inst, u32 *fail_res)
 
 	dev_dbg(inst->dev->dev, "%s: dec_finish_seq complete\n", __func__);
 
+free_resources:
 	wave5_vdi_free_dma_memory(vpu_dev, &p_dec_info->vb_work);
 
 	for (i = 0 ; i < MAX_REG_FRAME; i++) {
@@ -272,6 +277,8 @@ int wave5_vpu_dec_close(struct vpu_instance *inst, u32 *fail_res)
 	wave5_vdi_free_dma_memory(vpu_dev, &p_dec_info->vb_task);
 
 	mutex_destroy(&inst->feed_lock);
+	if (ret == -EIO)
+		return ret;
 
 unlock_and_return:
 	mutex_unlock(&vpu_dev->hw_lock);
@@ -750,7 +757,11 @@ 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) {
+		ret = -EIO;
+		goto free_resources;
+	}
 
 	ret = mutex_lock_interruptible(&vpu_dev->hw_lock);
 	if (ret) {
@@ -774,6 +785,7 @@ int wave5_vpu_enc_close(struct vpu_instance *inst, u32 *fail_res)
 
 	dev_dbg(inst->dev->dev, "%s: enc_finish_seq complete\n", __func__);
 
+free_resources:
 	wave5_vdi_free_dma_memory(vpu_dev, &p_enc_info->vb_work);
 
 	if (inst->std == W_HEVC_ENC || inst->std == W_AVC_ENC) {
@@ -784,6 +796,8 @@ int wave5_vpu_enc_close(struct vpu_instance *inst, u32 *fail_res)
 	}
 
 	wave5_vdi_free_dma_memory(vpu_dev, &p_enc_info->vb_task);
+	if (ret == -EIO)
+		return ret;
 
 unlock_and_return:
 	mutex_unlock(&vpu_dev->hw_lock);
-- 
2.34.1

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

[jackson.lee]

Hi Jiale

Thanks for the update. 
The direction looks right, and the encoder side looks good to me.

> -----Original Message-----
> From: Jiale Yao <yaojiale02@163.com>
> Sent: Monday, October 19, 2026 5:20 PM
> To: Nas Chung <nas.chung@chipsnmedia.com>; jackson.lee
> <jackson.lee@chipsnmedia.com>; Mauro Carvalho Chehab <mchehab@kernel.org>;
> Nicolas Dufresne <nicolas.dufresne@collabora.com>; Sebastian Fricke
> <sebastian.fricke@collabora.com>; Hans Verkuil <hverkuil@kernel.org>;
> linux-media@vger.kernel.org; linux-kernel@vger.kernel.org
> Cc: Jiale Yao <yaojiale02@163.com>
> Subject: [PATCH v2 4/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.
> 
> Returning the resume error directly also prevents release_device() from
> calling wave5_cleanup_instance(). The instance has already been removed
> from the device list and its mem2mem context released, so its host-side
> resources leak.
> 
> On a resume failure, skip the firmware command and hardware lock, free the
> close-path resources, and return -EIO. This lets release_device() continue
> with its common instance cleanup without issuing an unmatched runtime PM
> put.
> 
> Fixes: 2092b3833487 ("media: chips-media: wave5: Support runtime
> suspend/resume")
> Signed-off-by: Jiale Yao <yaojiale02@163.com>
> ---
>  .../platform/chips-media/wave5/wave5-vpuapi.c  | 18 ++++++++++++++++--
>  1 file changed, 16 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 4c1842f02c..806901e62d 100644
> --- a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
> +++ b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
> @@ -220,7 +220,11 @@ 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) {
> +		ret = -EIO;
> +		goto free_resources;
> +	}
> 
>  	ret_mutex = mutex_lock_interruptible(&vpu_dev->hw_lock);
>  	if (ret_mutex) {
> @@ -259,6 +263,7 @@ int wave5_vpu_dec_close(struct vpu_instance *inst, u32
> *fail_res)
> 
>  	dev_dbg(inst->dev->dev, "%s: dec_finish_seq complete\n", __func__);
> 
> +free_resources:
>  	wave5_vdi_free_dma_memory(vpu_dev, &p_dec_info->vb_work);
> 
>  	for (i = 0 ; i < MAX_REG_FRAME; i++) { 

On the decoder side, there is a problem with the ret == -EIO check.
After the free_resources label, the auxiliary buffer loop overwrites ret:

for (i = 0 ; i < MAX_REG_FRAME; i++) {
	ret = reset_auxiliary_buffers(inst, i);
	if (ret) {
		ret = 0;
		break;
	}
}

So ret is always 0 after the loop, the "if (ret == -EIO)" check never matches, and the resume failure path falls through to unlock_and_return. 
It then unlocks hw_lock that was never taken, calls pm_runtime_put_sync() without a reference, and returns 0.
Could you move the host-side freeing into a small helper and call it from both paths, and return -EIO directly on the resume failure? 
That would avoid relying on the value of ret across the cleanup code.

Thanks,
Jackson


@@ -272,6 +277,8 @@ int
> wave5_vpu_dec_close(struct vpu_instance *inst, u32 *fail_res)
>  	wave5_vdi_free_dma_memory(vpu_dev, &p_dec_info->vb_task);
> 
>  	mutex_destroy(&inst->feed_lock);
> +	if (ret == -EIO)
> +		return ret;
> 
>  unlock_and_return:
>  	mutex_unlock(&vpu_dev->hw_lock);
> @@ -750,7 +757,11 @@ 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) {
> +		ret = -EIO;
> +		goto free_resources;
> +	}
> 
>  	ret = mutex_lock_interruptible(&vpu_dev->hw_lock);
>  	if (ret) {
> @@ -774,6 +785,7 @@ int wave5_vpu_enc_close(struct vpu_instance *inst, u32
> *fail_res)
> 
>  	dev_dbg(inst->dev->dev, "%s: enc_finish_seq complete\n", __func__);
> 
> +free_resources:
>  	wave5_vdi_free_dma_memory(vpu_dev, &p_enc_info->vb_work);
> 
>  	if (inst->std == W_HEVC_ENC || inst->std == W_AVC_ENC) { @@ -784,6
> +796,8 @@ int wave5_vpu_enc_close(struct vpu_instance *inst, u32 *fail_res)
>  	}
> 
>  	wave5_vdi_free_dma_memory(vpu_dev, &p_enc_info->vb_task);
> +	if (ret == -EIO)
> +		return ret;
> 
>  unlock_and_return:
>  	mutex_unlock(&vpu_dev->hw_lock);
> --
> 2.34.1

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

[jiale yao]

Hi jackson,

At 2026-10-20 09:26:19, "jackson.lee" <jackson.lee@chipsnmedia.com> wrote:
>Hi Jiale
>
>Thanks for the update. 
>The direction looks right, and the encoder side looks good to me.
>
>> -----Original Message-----
>> From: Jiale Yao <yaojiale02@163.com>
>> Sent: Monday, October 19, 2026 5:20 PM
>> To: Nas Chung <nas.chung@chipsnmedia.com>; jackson.lee
>> <jackson.lee@chipsnmedia.com>; Mauro Carvalho Chehab <mchehab@kernel.org>;
>> Nicolas Dufresne <nicolas.dufresne@collabora.com>; Sebastian Fricke
>> <sebastian.fricke@collabora.com>; Hans Verkuil <hverkuil@kernel.org>;
>> linux-media@vger.kernel.org; linux-kernel@vger.kernel.org
>> Cc: Jiale Yao <yaojiale02@163.com>
>> Subject: [PATCH v2 4/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.
>> 
>> Returning the resume error directly also prevents release_device() from
>> calling wave5_cleanup_instance(). The instance has already been removed
>> from the device list and its mem2mem context released, so its host-side
>> resources leak.
>> 
>> On a resume failure, skip the firmware command and hardware lock, free the
>> close-path resources, and return -EIO. This lets release_device() continue
>> with its common instance cleanup without issuing an unmatched runtime PM
>> put.
>> 
>> Fixes: 2092b3833487 ("media: chips-media: wave5: Support runtime
>> suspend/resume")
>> Signed-off-by: Jiale Yao <yaojiale02@163.com>
>> ---
>>  .../platform/chips-media/wave5/wave5-vpuapi.c  | 18 ++++++++++++++++--
>>  1 file changed, 16 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 4c1842f02c..806901e62d 100644
>> --- a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
>> +++ b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
>> @@ -220,7 +220,11 @@ 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) {
>> +		ret = -EIO;
>> +		goto free_resources;
>> +	}
>> 
>>  	ret_mutex = mutex_lock_interruptible(&vpu_dev->hw_lock);
>>  	if (ret_mutex) {
>> @@ -259,6 +263,7 @@ int wave5_vpu_dec_close(struct vpu_instance *inst, u32
>> *fail_res)
>> 
>>  	dev_dbg(inst->dev->dev, "%s: dec_finish_seq complete\n", __func__);
>> 
>> +free_resources:
>>  	wave5_vdi_free_dma_memory(vpu_dev, &p_dec_info->vb_work);
>> 
>>  	for (i = 0 ; i < MAX_REG_FRAME; i++) { 
>
>On the decoder side, there is a problem with the ret == -EIO check.
>After the free_resources label, the auxiliary buffer loop overwrites ret:
>
>for (i = 0 ; i < MAX_REG_FRAME; i++) {
>	ret = reset_auxiliary_buffers(inst, i);
>	if (ret) {
>		ret = 0;
>		break;
>	}
>}
>
>So ret is always 0 after the loop, the "if (ret == -EIO)" check never matches, and the resume failure path falls through to unlock_and_return. 
>It then unlocks hw_lock that was never taken, calls pm_runtime_put_sync() without a reference, and returns 0.
>Could you move the host-side freeing into a small helper and call it from both paths, and return -EIO directly on the resume failure? 
>That would avoid relying on the value of ret across the cleanup code.
>
>Thanks,
>Jackson

How about this one, https://lore.kernel.org/all/20261021151552.428590-1-yaojiale02@163.com/

>
>
>@@ -272,6 +277,8 @@ int
>> wave5_vpu_dec_close(struct vpu_instance *inst, u32 *fail_res)
>>  	wave5_vdi_free_dma_memory(vpu_dev, &p_dec_info->vb_task);
>> 
>>  	mutex_destroy(&inst->feed_lock);
>> +	if (ret == -EIO)
>> +		return ret;
>> 
>>  unlock_and_return:
>>  	mutex_unlock(&vpu_dev->hw_lock);
>> @@ -750,7 +757,11 @@ 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) {
>> +		ret = -EIO;
>> +		goto free_resources;
>> +	}
>> 
>>  	ret = mutex_lock_interruptible(&vpu_dev->hw_lock);
>>  	if (ret) {
>> @@ -774,6 +785,7 @@ int wave5_vpu_enc_close(struct vpu_instance *inst, u32
>> *fail_res)
>> 
>>  	dev_dbg(inst->dev->dev, "%s: enc_finish_seq complete\n", __func__);
>> 
>> +free_resources:
>>  	wave5_vdi_free_dma_memory(vpu_dev, &p_enc_info->vb_work);
>> 
>>  	if (inst->std == W_HEVC_ENC || inst->std == W_AVC_ENC) { @@ -784,6
>> +796,8 @@ int wave5_vpu_enc_close(struct vpu_instance *inst, u32 *fail_res)
>>  	}
>> 
>>  	wave5_vdi_free_dma_memory(vpu_dev, &p_enc_info->vb_task);
>> +	if (ret == -EIO)
>> +		return ret;
>> 
>>  unlock_and_return:
>>  	mutex_unlock(&vpu_dev->hw_lock);
>> --
>> 2.34.1