Re: [PATCH v5 14/14] media: ti: j721e-csi2rx: Wait for the last drain completion

[Yemike Abhilash Chandra <y-abhilashchandra@ti.com>]

On 25/08/25 19:55, Rishikesh Donadkar wrote:
> dmaengine_terminate_sync() causes all activity for the DMA channel to be
> stopped, and may discard data in the DMA FIFO which hasn't been fully
> transferred. No callback functions will be called for any
> incomplete transfers[1].
> 
> In multistream use case, calling dmaengine_terminate_sync() immediately
> after issuing the last drain transaction will result in no callback
> for the last drain cycle.
> 
> Implement complete callback for the last drain cycle to make sure that
> the last drain has completed properly, this will ensure that stale data
> is not left out in the HW FIFO.
> 
> [1] : https://docs.kernel.org/driver-api/dmaengine/client.html
> 
> Signed-off-by: Rishikesh Donadkar <r-donadkar@ti.com>
> ---

Reviewed-by: Yemike Abhilash Chandra <y-abhilashchandra@ti.com>

>   drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c | 11 +++++++++++
>   1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c b/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c
> index 4ac6a76b9409..520ee05eb5b4 100644
> --- a/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c
> +++ b/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c
> @@ -62,6 +62,7 @@
>   #define TI_CSI2RX_MAX_PADS		(1 + TI_CSI2RX_MAX_SOURCE_PADS)
>   
>   #define DRAIN_BUFFER_SIZE		SZ_32K
> +#define DRAIN_TIMEOUT_MS		50
>   
>   #define CSI2RX_BRIDGE_SOURCE_PAD	1
>   
> @@ -137,6 +138,7 @@ struct ti_csi2rx_dev {
>   		size_t			len;
>   	} drain;
>   	bool				vc_cached;
> +	struct completion drain_complete;
>   };
>   
>   static inline struct ti_csi2rx_dev *to_csi2rx_dev(struct v4l2_subdev *sd)
> @@ -624,12 +626,14 @@ static void ti_csi2rx_setup_shim(struct ti_csi2rx_ctx *ctx)
>   static void ti_csi2rx_drain_callback(void *param)
>   {
>   	struct ti_csi2rx_ctx *ctx = param;
> +	struct ti_csi2rx_dev *csi = ctx->csi;
>   	struct ti_csi2rx_dma *dma = &ctx->dma;
>   	unsigned long flags;
>   
>   	spin_lock_irqsave(&dma->lock, flags);
>   
>   	if (dma->state == TI_CSI2RX_DMA_STOPPED) {
> +		complete(&csi->drain_complete);
>   		spin_unlock_irqrestore(&dma->lock, flags);
>   		return;
>   	}
> @@ -774,6 +778,7 @@ static int ti_csi2rx_start_dma(struct ti_csi2rx_ctx *ctx,
>   static void ti_csi2rx_stop_dma(struct ti_csi2rx_ctx *ctx)
>   {
>   	struct ti_csi2rx_dma *dma = &ctx->dma;
> +	struct ti_csi2rx_dev *csi = ctx->csi;
>   	enum ti_csi2rx_dma_state state;
>   	unsigned long flags;
>   	int ret;
> @@ -783,6 +788,8 @@ static void ti_csi2rx_stop_dma(struct ti_csi2rx_ctx *ctx)
>   	dma->state = TI_CSI2RX_DMA_STOPPED;
>   	spin_unlock_irqrestore(&dma->lock, flags);
>   
> +	init_completion(&csi->drain_complete);
> +
>   	if (state != TI_CSI2RX_DMA_STOPPED) {
>   		/*
>   		 * Normal DMA termination does not clean up pending data on
> @@ -796,6 +803,10 @@ static void ti_csi2rx_stop_dma(struct ti_csi2rx_ctx *ctx)
>   				 "Failed to drain DMA. Next frame might be bogus\n");
>   	}
>   
> +	if (!wait_for_completion_timeout(&csi->drain_complete,
> +					 msecs_to_jiffies(DRAIN_TIMEOUT_MS)))
> +		dev_dbg(csi->dev, "DMA transfer timed out for drain buffer\n");
> +
>   	ret = dmaengine_terminate_sync(ctx->dma.chan);
>   	if (ret)
>   		dev_err(ctx->csi->dev, "Failed to stop DMA: %d\n", ret);