Re: [PATCH v3 14/15] media: vsp1: Complete buffers as queued in start_streaming error path

[Laurent Pinchart]

Hi Paul,

Thank you for the patch.

On Thu, Oct 15, 2026 at 04:51:54PM +0900, Paul Elder wrote:
> In the error path of vsp1_video_start_streaming, the queued buffers are
> released via vsp1_video_release_buffers with the state
> VB2_BUF_STATE_ERROR. This causes subsequent start_streaming calls to hit
> the following warning in vb2_start_streaming:
> 
> /*
>  * If done_list is not empty, then start_streaming() didn't call
>  * vb2_buffer_done(vb, VB2_BUF_STATE_QUEUED) but STATE_ERROR or
>  * STATE_DONE.
> */

Missing space. I recommend copying indentation from the original source
to make it clearer:

	/*
	 * If done_list is not empty, then start_streaming() didn't call
	 * vb2_buffer_done(vb, VB2_BUF_STATE_QUEUED) but STATE_ERROR or
	 * STATE_DONE.
	 */
	WARN_ON(!list_empty(&q->done_list));

> WARN_ON(!list_empty(&q->done_list));
> 
> Fix this by completing the queued buffers with the state set to
> VB2_BUF_STATE_QUEUED in the error path of vsp1_video_start_streaming.
> 
> Signed-off-by: Paul Elder <paul.elder+renesas@xxxxxxxxxxxxxxxx>

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx>

> ---
>  drivers/media/platform/renesas/vsp1/vsp1_video.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/platform/renesas/vsp1/vsp1_video.c b/drivers/media/platform/renesas/vsp1/vsp1_video.c
> index b85351986fae50aa678032b05b0352dff2b5a26a..46e0d6f8a66bddb4064e52e1521f3c9da7192868 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_video.c
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_video.c
> @@ -765,7 +765,8 @@ static int vsp1_video_setup_pipeline(struct vsp1_pipeline *pipe)
>  	return 0;
>  }
>  
> -static void vsp1_video_release_buffers(struct vsp1_video *video)
> +static void vsp1_video_release_buffers(struct vsp1_video *video,
> +				       enum vb2_buffer_state state)
>  {
>  	struct vsp1_vb2_buffer *buffer;
>  
> @@ -773,7 +774,7 @@ static void vsp1_video_release_buffers(struct vsp1_video *video)
>  	guard(spinlock_irqsave)(&video->irqlock);
>  
>  	list_for_each_entry(buffer, &video->irqqueue, queue)
> -		vb2_buffer_done(&buffer->buf.vb2_buf, VB2_BUF_STATE_ERROR);
> +		vb2_buffer_done(&buffer->buf.vb2_buf, state);
>  	INIT_LIST_HEAD(&video->irqqueue);
>  }
>  
> @@ -802,7 +803,7 @@ static int vsp1_video_start_streaming(struct vb2_queue *vq, unsigned int count)
>  		if (pipe->stream_count == pipe->num_inputs) {
>  			ret = vsp1_video_setup_pipeline(pipe);
>  			if (ret < 0) {
> -				vsp1_video_release_buffers(video);
> +				vsp1_video_release_buffers(video, VB2_BUF_STATE_QUEUED);
>  				vsp1_video_cleanup_pipeline(pipe);
>  				return ret;
>  			}
> @@ -858,7 +859,7 @@ static void vsp1_video_stop_streaming(struct vb2_queue *vq)
>  	}
>  
>  	video_device_pipeline_stop(&video->video);
> -	vsp1_video_release_buffers(video);
> +	vsp1_video_release_buffers(video, VB2_BUF_STATE_ERROR);
>  	vsp1_video_pipeline_put(pipe);
>  }
>  

-- 
Regards,

Laurent Pinchart