Re: i.MX8MP: Fix HDMI LCDIF FIFO underruns

[Paul Kocialkowski <paulk@sys-base.io>]

[-- Attachment #1: Type: text/plain, Size: 5322 bytes --]

Hi Lucas,

On Mon 30 Mar 26, 18:44, Lucas Stach wrote:
> Am Montag, dem 30.03.2026 um 18:09 +0200 schrieb Paul Kocialkowski:
> > After weeks of investigation I finally narrowed it down to a part of the nxp
> > original code that is missing in the upstream driver:
> > https://github.com/phytec/linux-phytec-imx/blob/v6.6.23-2.0.0-phy/drivers/gpu/imx/lcdifv3/lcdifv3-common.c#L492
> > 
> Thanks for tracking this down! It was also on my list of things to look
> at.

Glad that it helps :)

> > A big old sleep that waits for the DMA engine to finish handling the current
> > frame before it is disabled. When this delay is not respected, there seems to
> > be "unlucky" times when disabling the DMA engine too early will confuse the
> > unit and make it unable to resume proper operation later.
> > 
> I think we can be a bit more clever than a indiscriminate sleep. If we
> set the shadow load enable bit in the disable sequence, we should be
> able to wait for this bit to be cleared by the hardware. All the DMA
> parameters (presumably including the enable state) should be applied by
> the hardware when this bit is cleared.

I just tried it out and it works fine, thanks! It polls for a variable delay
from < 1 ms to < 16 ms, which makes perfect sense for a 60 Hz mode.

This is much better than the hardcoded 25 ms. I'll add your Co-developed-by tag
to the final commit.

> I guess the read_poll_timeout in lcdif_disable_controller() was
> supposed to do exactly that wait, but as it was actually copied from
> the mxsfb driver, it's not working as intended. It's my understanding
> that the old lcdif HW only cleared the enable bit once it is actually
> done, but on the lcdifv3 on i.MX8MP those states are double buffered,
> so the bit in the register would be cleared instantaneously, rendering
> the wait in the current code a no-op. The only reliable indicator to
> see if the HW has updated it's internal state is to set the shadow load
> enable bit and wait for it to be cleared.

Sounds right to me. Thanks for the details! Are there resources about the
different generations/families used by nxp/freescale? They seem to be in-house
designs but with lots of different variations.

> > It is a bit surprising that the issue is not actually related to configuring
> > the display engine but to disabling it. This is why the first mode set always
> > works but subsequentil ones might fail. The crtc is essentially disabled and
> > re-enabled each time a new mode is applied.
> > 
> > Adding the sleep solves the issue on my side and all mode sets now work
> > reliably. It does add a delay before returning to userspace when configuring
> > a new mode, but it seems legitimate to me if the underlying hardware is still
> > in-flight. I'm also unsure if it would apply to an async atomic commit.
> > 
> This hardware doesn't support async commits. All DMA state changes are
> applied during vblank when the shadow load enable bit is set.

Ah sorry I meant non-blocking atomic commit, not async. Since this is in the
CRTC disable path, it probably makes the userspace process sleep even in the
case of a non-blocking commit (unless this happens in a kernel thread, I'm not
sure exactly). But I guess it is legitimate to block until the previous frame
is finished if it is mandatory.

All the best,

Paul

> 
> Regards,
> Lucas
> 
> > I will send a patch adding the delay and the undocumented fifo clear
> > bit that was discovered in this thread too.
> > 
> > All the best,
> > 
> > Paul
> > 
> > Perhaps there is a more elegant way to handle this 
> > 
> > 
> > > Interestingly, Weston usually starts fine in subsequent launches.
> > > 
> > > How is this first (actually second - after the first VSYNC) frame
> > > different from all the others? Maybe we're programming UPDATE_SHADOW
> > > after the actual start of blanking period, but before DE, so it has no
> > > chance to reload registers, yet the DMA is somehow not ready?
> > > 
> > > The manual (LCDIF display control Register (CTRL)) suggests that the DMA
> > > (with present settings) starts to fetch FB data at the end of the
> > > previous active video pixel time (DE and pixel data going inactive):
> > > fetch_start_option (bits 9-8): Indicates when to start fetching for new
> > > frame. This signals also decide the shadow load, fifo clear time
> > > 00b - fetch start as soon as FPV begins(as the end of the data_enable).
> > > 
> > > This should leave a lot of time to fill the FIFO (before the next DE),
> > > shouldn't it?
> > > 
> > > 297 MHz pixclk, 3840x2160, 30 Hz, H back porch 296 pixclks, Vfp 176,
> > > V back porch 72 lines, Hfp 8 lines, VSYNC 10 lines, HSYNC 88 pixclks.
> > > HTotal = 4400 pixels which makes Vactive time = 4400 * 2160/297e6 =
> > > 32 ms, with the remaining 1.3333 ms for all the V sync stuff.
> > > -- 
> > > Krzysztof "Chris" Hałasa
> > > 
> > > Sieć Badawcza Łukasiewicz
> > > Przemysłowy Instytut Automatyki i Pomiarów PIAP
> > > Al. Jerozolimskie 202, 02-486 Warszawa
> > 
> 

-- 
Paul Kocialkowski,

Independent contractor - sys-base - https://www.sys-base.io/
Free software developer - https://www.paulk.fr/

Expert in multimedia, graphics and embedded hardware support with Linux.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]