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

["jackson.lee" <jackson.lee@chipsnmedia.com>]

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