Hoi all,
> On Tue, Jun 02, 2026 at 02:12:43PM +0200, Lucas Stach wrote: > > 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 <[email protected]> > > > > > > > > > > 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 <[email protected]> > > > > > --- > > > > > 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. Thanks for all the input, since Ahmad also pointed out that there is already a driver, i'll restructure this patchstack around the upstream on and weave in all the input so far -> expect a v2. > > I am curious if this is actually all necessary. > i'll admit that i don't have a very deep understanding of the whole pixel pipeline, and am just putting/copying (from the kernel mostly) things together and "massaging" them until the display showed something (without any weird distortions or other artifacts) > In the driver Johannes send, there is the comment > > + /* 32bpp XRGB8888 → ARGB8888 pixel format. > + * SHADOW_LOAD_EN causes the descriptor registers (address, format, > pitch) > + * to be latched into the active registers on the next VSYNC, which is > + * required for the initial descriptor load in barebox's single-enable > + * sequence (no DRM page-flip mechanism). EN is ORed in later by > + * lcdif_enable() after DISP_ON. */ > > in lcdif_set_plane of the imx-lcdif driver. > > This exactly flushes the buffer once. I also did this but also > implemented lcdif_crtc_atomic_flush in drivers/video/lcdif_kms.c as the > fb_flush callback which will do exactly this LOAD_EN call every time a > new frame was written. With this change I had no issues with distorted > images. > > > > > > +} > > > > > + > > > > > 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) > > > > > > > > > gruß Johannes
