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

[PATCH v3 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.

Changes in v3:
- Move the encoder STOP state transition before runtime resume in
  stop_streaming(), so a resume failure cannot leave the instance in
  PIC_RUN.
- Factor decoder host-resource cleanup into a helper and return -EIO
  directly after resume failure, avoiding an unheld mutex unlock and an
  unmatched runtime PM put.
- Add Jackson Lee's Reviewed-by tag to the encoder patch.
- Keep the m2m buffer completion helper patch unchanged.

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.

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

 .../chips-media/wave5/wave5-vpu-dec.c         | 49 ++++++++++++++++---
 .../chips-media/wave5/wave5-vpu-enc.c         | 35 +++++++++++--
 .../platform/chips-media/wave5/wave5-vpuapi.c | 47 +++++++++++-------
 3 files changed, 103 insertions(+), 28 deletions(-)


base-commit: d7e7bca7169873b46e857c025c10f3336c1258d8
-- 
2.34.1

[PATCH v3 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

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

[Brandon Brnich]

Hi Jiale

Thanks for the patch.

On 10/21/26 10:15, Jiale Yao wrote:
> 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;
> +	}
> +

Wouldn't this lead to an infinite loop in the m2m scheduler? looking 
around other drivers (rockchip and hantro) they handle this case with 
failing the buffers and then marking the job as finished. Similar to 
what you did in the encoder path. Maybe you can make a helper so both 
device runs can use?

Best,
Brandon

>   	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);
>   }
>   

[PATCH v3 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

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

[Brandon Brnich]

Hi Jiale,

Thanks for the patch.

On 10/21/26 10:15, Jiale Yao wrote:
> 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.
> 

You can add fixes here. 158b39a0f7d198 is the commit-sha

Tested-by: Brandon Brnich <b-brnich@ti.com>
> 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;
>   	}
>   

[PATCH v3 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. Also stop the instance in stop_streaming() before attempting the
runtime resume, since the state transition only touches software state.
Propagate the error from start_streaming() and preserve buffer cleanup
without accessing the hardware or issuing an unmatched put.

Fixes: 2092b3833487 ("media: chips-media: wave5: Support runtime suspend/resume")
Cc: stable@vger.kernel.org
Reviewed-by: Jackson Lee <jackson.lee@chipsnmedia.com>
Signed-off-by: Jiale Yao <yaojiale02@163.com>
---
 .../chips-media/wave5/wave5-vpu-enc.c         | 35 ++++++++++++++++---
 1 file changed, 31 insertions(+), 4 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..3463b7d7fb 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,11 +1464,18 @@ 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);
-
 	if (wave5_vpu_both_queues_are_streaming(inst))
 		switch_state(inst, VPU_INST_STATE_STOP);
 
+	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;
+	}
+
 	while (check_cmd) {
 		struct queue_status_info q_status;
 		struct enc_output_info enc_output_info;
@@ -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

[PATCH v3 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. Use a decoder cleanup helper
so auxiliary-buffer cleanup cannot overwrite the resume failure status.
This lets release_device() continue with its common instance cleanup
without unlocking an unheld mutex or issuing an unmatched runtime PM put.

Fixes: 2092b3833487 ("media: chips-media: wave5: Support runtime suspend/resume")
Cc: stable@vger.kernel.org
Signed-off-by: Jiale Yao <yaojiale02@163.com>
---
 .../platform/chips-media/wave5/wave5-vpuapi.c | 47 ++++++++++++-------
 1 file changed, 29 insertions(+), 18 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..31be2581ae 100644
--- a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
+++ b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
@@ -206,13 +206,26 @@ static int reset_auxiliary_buffers(struct vpu_instance *inst, unsigned int index
 	return 0;
 }
 
-int wave5_vpu_dec_close(struct vpu_instance *inst, u32 *fail_res)
+static void wave5_vpu_dec_free_resources(struct vpu_instance *inst)
 {
 	struct dec_info *p_dec_info = &inst->codec_info->dec_info;
+	int i;
+
+	wave5_vdi_free_dma_memory(inst->dev, &p_dec_info->vb_work);
+
+	for (i = 0; i < MAX_REG_FRAME; i++)
+		if (reset_auxiliary_buffers(inst, i))
+			break;
+
+	wave5_vdi_free_dma_memory(inst->dev, &p_dec_info->vb_task);
+	mutex_destroy(&inst->feed_lock);
+}
+
+int wave5_vpu_dec_close(struct vpu_instance *inst, u32 *fail_res)
+{
 	int ret;
 	int retry = 0;
 	struct vpu_device *vpu_dev = inst->dev;
-	int i;
 	struct dec_output_info dec_info;
 	int ret_mutex;
 
@@ -220,7 +233,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) {
+		wave5_vpu_dec_free_resources(inst);
+		return -EIO;
+	}
 
 	ret_mutex = mutex_lock_interruptible(&vpu_dev->hw_lock);
 	if (ret_mutex) {
@@ -258,20 +275,7 @@ int wave5_vpu_dec_close(struct vpu_instance *inst, u32 *fail_res)
 	} while (ret != 0);
 
 	dev_dbg(inst->dev->dev, "%s: dec_finish_seq complete\n", __func__);
-
-	wave5_vdi_free_dma_memory(vpu_dev, &p_dec_info->vb_work);
-
-	for (i = 0 ; i < MAX_REG_FRAME; i++) {
-		ret = reset_auxiliary_buffers(inst, i);
-		if (ret) {
-			ret = 0;
-			break;
-		}
-	}
-
-	wave5_vdi_free_dma_memory(vpu_dev, &p_dec_info->vb_task);
-
-	mutex_destroy(&inst->feed_lock);
+	wave5_vpu_dec_free_resources(inst);
 
 unlock_and_return:
 	mutex_unlock(&vpu_dev->hw_lock);
@@ -750,7 +754,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 +782,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 +793,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 v3 4/4] media: wave5: handle runtime resume failure when closing

[jackson.lee]

Hi Jiale,

Thanks, this version looks good to me. The decoder problem is fixed.

One small idea: you could also use a helper function for the encoder, like you did for the decoder. Then both close functions would work the same way.
This is optional. The encoder code works fine as it is.

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

Thanks,
Jackson

> -----Original Message-----
> From: Jiale Yao <yaojiale02@163.com>
> Sent: Thursday, October 22, 2026 12:16 AM
> 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: stable@vger.kernel.org
> Subject: [PATCH v3 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. Use a decoder cleanup helper so
> auxiliary-buffer cleanup cannot overwrite the resume failure status.
> This lets release_device() continue with its common instance cleanup
> without unlocking an unheld mutex or issuing an unmatched runtime PM put.
> 
> Fixes: 2092b3833487 ("media: chips-media: wave5: Support runtime
> suspend/resume")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jiale Yao <yaojiale02@163.com>
> ---
>  .../platform/chips-media/wave5/wave5-vpuapi.c | 47 ++++++++++++-------
>  1 file changed, 29 insertions(+), 18 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..31be2581ae 100644
> --- a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
> +++ b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
> @@ -206,13 +206,26 @@ static int reset_auxiliary_buffers(struct
> vpu_instance *inst, unsigned int index
>  	return 0;
>  }
> 
> -int wave5_vpu_dec_close(struct vpu_instance *inst, u32 *fail_res)
> +static void wave5_vpu_dec_free_resources(struct vpu_instance *inst)
>  {
>  	struct dec_info *p_dec_info = &inst->codec_info->dec_info;
> +	int i;
> +
> +	wave5_vdi_free_dma_memory(inst->dev, &p_dec_info->vb_work);
> +
> +	for (i = 0; i < MAX_REG_FRAME; i++)
> +		if (reset_auxiliary_buffers(inst, i))
> +			break;
> +
> +	wave5_vdi_free_dma_memory(inst->dev, &p_dec_info->vb_task);
> +	mutex_destroy(&inst->feed_lock);
> +}
> +
> +int wave5_vpu_dec_close(struct vpu_instance *inst, u32 *fail_res) {
>  	int ret;
>  	int retry = 0;
>  	struct vpu_device *vpu_dev = inst->dev;
> -	int i;
>  	struct dec_output_info dec_info;
>  	int ret_mutex;
> 
> @@ -220,7 +233,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) {
> +		wave5_vpu_dec_free_resources(inst);
> +		return -EIO;
> +	}
> 
>  	ret_mutex = mutex_lock_interruptible(&vpu_dev->hw_lock);
>  	if (ret_mutex) {
> @@ -258,20 +275,7 @@ int wave5_vpu_dec_close(struct vpu_instance *inst,
> u32 *fail_res)
>  	} while (ret != 0);
> 
>  	dev_dbg(inst->dev->dev, "%s: dec_finish_seq complete\n", __func__);
> -
> -	wave5_vdi_free_dma_memory(vpu_dev, &p_dec_info->vb_work);
> -
> -	for (i = 0 ; i < MAX_REG_FRAME; i++) {
> -		ret = reset_auxiliary_buffers(inst, i);
> -		if (ret) {
> -			ret = 0;
> -			break;
> -		}
> -	}
> -
> -	wave5_vdi_free_dma_memory(vpu_dev, &p_dec_info->vb_task);
> -
> -	mutex_destroy(&inst->feed_lock);
> +	wave5_vpu_dec_free_resources(inst);
> 
>  unlock_and_return:
>  	mutex_unlock(&vpu_dev->hw_lock);
> @@ -750,7 +754,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 +782,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
> +793,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