Re: [PATCH v2 2/2] media: chips-media: wave5: Check decoder runtime resume errors

[Nicolas Dufresne <nicolas.dufresne@collabora.com>]

[-- Attachment #1: Type: text/plain, Size: 3852 bytes --]

Le lundi 28 septembre 2026 à 17:13 -0400, Nicolas Dufresne a écrit :
> Le samedi 18 juillet 2026 à 21:00 +0800, Guangshuo Li a écrit :
> > The decoder start_streaming, stop_streaming and device_run callbacks
> > resume the VPU before accessing hardware, but do not check the return
> > value from pm_runtime_resume_and_get().
> > 
> > If runtime resume fails, continuing can access the VPU while it remains
> > suspended. Since pm_runtime_resume_and_get() does not retain a runtime PM
> > usage reference on failure, the later unconditional
> > pm_runtime_put_autosuspend() can also drop an unmatched reference.
> > 
> > Check the return value in all three callbacks. Return queued buffers as
> > required by vb2 when start_streaming fails, clean up buffers without
> > touching hardware when stop_streaming fails, and finish the current
> > mem2mem job when device_run cannot resume.
> > 
> > Only call pm_runtime_put_autosuspend() after a successful runtime
> > resume.
> > 
> > Fixes: 2092b3833487 ("media: chips-media: wave5: Support runtime suspend/resume")
> > Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> 
> Reviewed-by: Nicolas Dufresne ņicolas.dufresne@collabora.com>
> 
> Picked, ty

Spoken too fast, I picked the first, please rebase and resubmit this one, it has
conflicts that are non trivial to fix.

Nicolas

> 
> > ---
> >  .../chips-media/wave5/wave5-vpu-dec.c         | 36 +++++++++++++++++--
> >  1 file changed, 33 insertions(+), 3 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 03d108b808ba..bb59bc962603 100644
> > --- a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> > +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> > @@ -1380,7 +1380,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);
> >  
> > @@ -1544,9 +1548,29 @@ static void wave5_vpu_dec_stop_streaming(struct vb2_queue *q)
> >  	struct v4l2_m2m_ctx *m2m_ctx = inst->v4l2_fh.m2m_ctx;
> >  
> >  	bool check_cmd = TRUE;
> > +	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) {
> > +		struct vpu_src_buffer *vpu_buf;
> > +
> > +		v4l2_m2m_update_stop_streaming_state(m2m_ctx, q);
> > +
> > +		if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
> > +			inst->retry = false;
> > +			inst->queuing_num = 0;
> > +			while ((vpu_buf = inst_src_buf_remove(inst)) != NULL)
> > +				;
> > +			inst->eos = false;
> > +		}
> > +
> > +		wave5_return_bufs(q, VB2_BUF_STATE_ERROR);
> > +		inst->empty_queue = false;
> > +		inst->sent_eos = false;
> > +		return;
> > +	}
> > +
> >  	inst->empty_queue = true;
> >  	while (check_cmd) {
> >  		struct queue_status_info q_status;
> > @@ -1659,7 +1683,13 @@ static void wave5_vpu_dec_device_run(void *priv)
> >  	int ret = 0;
> >  
> >  	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) {
> > +		v4l2_m2m_job_finish(inst->v4l2_m2m_dev, m2m_ctx);
> > +		return;
> > +	}
> > +
> >  	if (!inst->retry) {
> >  		ret = fill_ringbuffer(inst);
> >  		if (ret < 0) {

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]