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 11:37 +0200 schrieb Ahmad Fatoum:
> 
> On 6/2/26 10:12 AM, Lucas Stach wrote:
> > 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.
> 
> Would that on its own suffice though? If we have just the single buffer,
> flushing the write buffer only reduces the race window, but doesn't
> remove it as the framebuffer write may still happen in parallel to the
> scanout.

Updating the (front-)framebuffer will always be racy against the
scanout. The flushes just ensure that the writes eventually reach
memory visible to the scanout DMA master. Without any flushes there is
nothing in the memory model which would prohibit writes from being
stuck in the write buffers indefinitely. 

Regards,
Lucas

> 
> Cheers,
> Ahmad
> 
> > 
> > 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)
> > 
> >