Re: [PATCH 6/6] video: imx-lcdif: drain write-combine buffer before LCDIF DMA reads pixels

[Lucas Stach <l.stach@pengutronix.de>]

Am Dienstag, dem 02.06.2026 um 04:09 +0000 schrieb Johannes Schneider:
> From: Thomas Haemmerle <thomas.haemmerle@leica-geosystems.com>
> 
> The LCDIF framebuffer is allocated as Normal Non-Cacheable (write-combine)
> memory.  After the splash command calls gu_screen_blit(), which copies the
> decoded PNG via memcpy() from a cached shadow buffer into the hardware
> framebuffer, writes may still reside in the CPU write buffer and not yet
> be visible to the LCDIF DMA engine.  On hardware this manifests as
> partial or corrupted rendering — the bottom portion of the splash image
> renders black or shows garbage.
> 
> Implement fb_damage() to execute a DSB SY barrier after each blit,
> ensuring all pending write-combine writes are committed to DRAM before
> the LCDIF reads the pixel data.
> 
> Assisted-by: Claude:claude-sonnet-4-6
> Signed-of-by: Thomas Haemmerle <thomas.haemmerle@leica-geosystems.com>
> ---
>  drivers/video/imx-lcdif.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/video/imx-lcdif.c b/drivers/video/imx-lcdif.c
> index ae5976c771..8d9b40be0f 100644
> --- a/drivers/video/imx-lcdif.c
> +++ b/drivers/video/imx-lcdif.c
> @@ -20,6 +20,7 @@
>  #include <of_graph.h>
>  #include <video/media-bus-format.h>
>  #include <video/vpl.h>
> +#include <asm/system.h>
>  
>  /* LCDIF V8 register map */
>  #define LCDC_V8_CTRL			0x00
> @@ -262,9 +263,21 @@ static void lcdif_fb_disable(struct fb_info *info)
>  	clk_disable_unprepare(priv->clk_axi);
>  }
>  
> +static void lcdif_fb_damage(struct fb_info *info, struct fb_rect *rect)
> +{
> +	/*
> +	 * The hardware framebuffer is Normal Non-Cacheable (write-combine).
> +	 * Writes from the shadow-buffer memcpy may sit in the CPU write buffer
> +	 * and not yet be visible to the LCDIF DMA engine. Execute DSB SY to
> +	 * drain the write buffer to DRAM before the LCDIF reads the pixels.
> +	 */
> +	dsb();

This will probably work in practice, but it doesn't exactly do what the
comment above states. A dsb does not drain the write combine buffers,
it just makes sure that they are drained before the next instruction is
executed.
So if the fb_damage is the last thing that gets executed, this will not
ensure that the writes will actually become visible to external agents.

A more robust way to drain the write combine buffers is to execute a
readback of a memory location within the WC region.

Regards,
Lucas
> +}
> +
>  static struct fb_ops lcdif_fb_ops = {
>  	.fb_enable  = lcdif_fb_enable,
>  	.fb_disable = lcdif_fb_disable,
> +	.fb_damage  = lcdif_fb_damage,
>  };
>  
>  static int lcdif_probe(struct device *dev)