Re: [PATCH v2 2/2] drm: lcdif: Wait for vblank before disabling DMA

[Lucas Stach]

Am Donnerstag, dem 02.04.2026 um 20:33 +0200 schrieb Paul Kocialkowski:
> It is necessary to wait for the full frame to finish streaming
> through the DMA engine before we can safely disable it by removing
> the DISP_PARA_DISP_ON bit. Disabling it in-flight can leave the
> hardware confused and unable to resume streaming for the next frame.
> 
> This causes the FIFO underrun and empty status bits to be set and
> a single solid color to be shown on the display, coming from one of
> the pixels of the previous frame. The issue occurs sporadically when
> a new mode is set, which triggers the crtc disable and enable paths.
> 
> Setting the shadow load bit and waiting for it to be cleared by the
> DMA engine allows waiting for completion.
> 
> The NXP BSP driver addresses this issue with a hardcoded 25 ms sleep.
> 
> Fixes: 9db35bb349a0 ("drm: lcdif: Add support for i.MX8MP LCDIF variant")
> Signed-off-by: Paul Kocialkowski <paulk@xxxxxxxxxxx>
> Co-developed-by: Lucas Stach <l.stach@xxxxxxxxxxxxxx>

Looks good to me. Since the co-developed tag requires a sign-off from
me, I'll add that instead of a review tag. And you might want to move
your sign-off down, as you are the one submitting the patch.

Signed-off-by: Lucas Stach <l.stach@xxxxxxxxxxxxxx>

> ---
>  drivers/gpu/drm/mxsfb/lcdif_kms.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mxsfb/lcdif_kms.c b/drivers/gpu/drm/mxsfb/lcdif_kms.c
> index a00c4f6d63f4..0d04a0028671 100644
> --- a/drivers/gpu/drm/mxsfb/lcdif_kms.c
> +++ b/drivers/gpu/drm/mxsfb/lcdif_kms.c
> @@ -375,14 +375,23 @@ static void lcdif_disable_controller(struct lcdif_drm_private *lcdif)
>  	int ret;
>  
>  	reg = readl(lcdif->base + LCDC_V8_CTRLDESCL0_5);
> +	/* Disable the layer for DMA. */
>  	reg &= ~CTRLDESCL0_5_EN;
> +	/*
> +	 * It is necessary to wait for the full frame to finish streaming
> +	 * through the DMA engine before we can safely disable it by removing
> +	 * the DISP_PARA_DISP_ON bit. Disabling it in-flight can leave the
> +	 * hardware confused and unable to resume streaming for the next frame.
> +	 */
> +	reg |= CTRLDESCL0_5_SHADOW_LOAD_EN;
>  	writel(reg, lcdif->base + LCDC_V8_CTRLDESCL0_5);
>  
> +	/* Wait for the frame to finish or timeout after 50 ms. */
>  	ret = readl_poll_timeout(lcdif->base + LCDC_V8_CTRLDESCL0_5,
> -				 reg, !(reg & CTRLDESCL0_5_EN),
> -				 0, 36000);	/* Wait ~2 frame times max */
> +				 reg, !(reg & CTRLDESCL0_5_SHADOW_LOAD_EN),
> +				 200, 50000);
>  	if (ret)
> -		drm_err(lcdif->drm, "Failed to disable controller!\n");
> +		drm_err(lcdif->drm, "Timed out waiting for final vblank!\n");
>  
>  	reg = readl(lcdif->base + LCDC_V8_DISP_PARA);
>  	reg &= ~DISP_PARA_DISP_ON;