Re: [PATCH] media: imx: imx7-media-csi: Fix buffer return upon stream start failure

["Rui Miguel Silva" <rmfrfs@gmail.com>]

Hi Laurent,
On Wed May 19, 2021 at 1:58 AM WEST, Laurent Pinchart wrote:

> When the stream fails to start, the first two buffers in the queue have
> been moved to the active_vb2_buf array and are returned to vb2 by
> imx7_csi_dma_unsetup_vb2_buf(). The function is called with the buffer
> state set to VB2_BUF_STATE_ERROR unconditionally, which is correct when
> stopping the stream, but not when the start operation fails. In that
> case, the state should be set to VB2_BUF_STATE_QUEUED. Fix it.

Did not know this one. Thanks for the fix.

Reviewed-by: Rui Miguel Silva <rmfrfs@gmail.com>

------
Cheers,
     Rui

>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/staging/media/imx/imx7-media-csi.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c
> index f644a640a831..da768ea21d03 100644
> --- a/drivers/staging/media/imx/imx7-media-csi.c
> +++ b/drivers/staging/media/imx/imx7-media-csi.c
> @@ -361,6 +361,7 @@ static void imx7_csi_dma_unsetup_vb2_buf(struct imx7_csi *csi,
>  
>  			vb->timestamp = ktime_get_ns();
>  			vb2_buffer_done(vb, return_status);
> +			csi->active_vb2_buf[i] = NULL;
>  		}
>  	}
>  }
> @@ -386,9 +387,10 @@ static int imx7_csi_dma_setup(struct imx7_csi *csi)
>  	return 0;
>  }
>  
> -static void imx7_csi_dma_cleanup(struct imx7_csi *csi)
> +static void imx7_csi_dma_cleanup(struct imx7_csi *csi,
> +				 enum vb2_buffer_state return_status)
>  {
> -	imx7_csi_dma_unsetup_vb2_buf(csi, VB2_BUF_STATE_ERROR);
> +	imx7_csi_dma_unsetup_vb2_buf(csi, return_status);
>  	imx_media_free_dma_buf(csi->dev, &csi->underrun_buf);
>  }
>  
> @@ -526,9 +528,10 @@ static int imx7_csi_init(struct imx7_csi *csi)
>  	return 0;
>  }
>  
> -static void imx7_csi_deinit(struct imx7_csi *csi)
> +static void imx7_csi_deinit(struct imx7_csi *csi,
> +			    enum vb2_buffer_state return_status)
>  {
> -	imx7_csi_dma_cleanup(csi);
> +	imx7_csi_dma_cleanup(csi, return_status);
>  	imx7_csi_init_default(csi);
>  	imx7_csi_dmareq_rff_disable(csi);
>  	clk_disable_unprepare(csi->mclk);
> @@ -691,7 +694,7 @@ static int imx7_csi_s_stream(struct v4l2_subdev *sd, int enable)
>  
>  		ret = v4l2_subdev_call(csi->src_sd, video, s_stream, 1);
>  		if (ret < 0) {
> -			imx7_csi_deinit(csi);
> +			imx7_csi_deinit(csi, VB2_BUF_STATE_QUEUED);
>  			goto out_unlock;
>  		}
>  
> @@ -701,7 +704,7 @@ static int imx7_csi_s_stream(struct v4l2_subdev *sd, int enable)
>  
>  		v4l2_subdev_call(csi->src_sd, video, s_stream, 0);
>  
> -		imx7_csi_deinit(csi);
> +		imx7_csi_deinit(csi, VB2_BUF_STATE_ERROR);
>  	}
>  
>  	csi->is_streaming = !!enable;
> -- 
> Regards,
>
> Laurent Pinchart