Re: [PATCH 1/4] media: rcar-vin: Free buffers with error if hardware stop fails

From: Hans Verkuil <hverkuil-cisco@xs4all.nl>
To: "Niklas Söderlund" <niklas.soderlund+renesas@ragnatech.se>,
	linux-media@vger.kernel.org
Cc: linux-renesas-soc@vger.kernel.org
Subject: Re: [PATCH 1/4] media: rcar-vin: Free buffers with error if hardware stop fails
Date: Mon, 15 Nov 2021 15:36:39 +0100	[thread overview]
Message-ID: <f6a00224-82b1-c506-2c1b-4ebb75657268@xs4all.nl> (raw)
In-Reply-To: <20211108160220.767586-2-niklas.soderlund+renesas@ragnatech.se>

On 08/11/2021 17:02, Niklas Söderlund wrote:
> The driver already have logic to detect if it fails to stop properly and
> report this error to the user. The driver however did not report the
> unused buffers or buffers given to the hardware (if any) with an error,
> the buffers where instead returned to user-space in the active state.
> 
> Build on the existing detection of the error condition and correctly
> return the buffers with an error if it triggers.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
>  drivers/media/platform/rcar-vin/rcar-dma.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
> index 25ead9333d0046e7..79bb9081853f8781 100644
> --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> @@ -1371,6 +1371,16 @@ void rvin_stop_streaming(struct rvin_dev *vin)
>  
>  	spin_unlock_irqrestore(&vin->qlock, flags);
>  
> +	/* If something went wrong, free buffers with an error. */
> +	if (!buffersFreed) {
> +		return_unused_buffers(vin, VB2_BUF_STATE_ERROR);
> +		for (i = 0; i < HW_BUFFER_NUM; i++) {
> +			if (vin->buf_hw[i].buffer)
> +				vb2_buffer_done(&vin->buf_hw[i].buffer->vb2_buf,
> +						VB2_BUF_STATE_ERROR);
> +		}
> +	}
> +
>  	rvin_set_stream(vin, 0);
>  
>  	/* disable interrupts */
> 

I'll take this patch, but mark the other three as 'Changes Requested'.

Regards,

	Hans