[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20251008-nondescript-snobbish-rattlesnake-d486a7@houat>
Date: Wed, 8 Oct 2025 15:57:52 +0200
From: Maxime Ripard <mripard@...nel.org>
To: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
Cc: Devarsh Thakkar <devarsht@...com>, dri-devel@...ts.freedesktop.org,
linux-kernel@...r.kernel.org, Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>,
Simona Vetter <simona@...ll.ch>, Andrzej Hajda <andrzej.hajda@...el.com>,
Neil Armstrong <neil.armstrong@...aro.org>, Robert Foss <rfoss@...nel.org>,
Laurent Pinchart <Laurent.pinchart@...asonboard.com>, Jonas Karlman <jonas@...boo.se>,
Jernej Skrabec <jernej.skrabec@...il.com>, Jyri Sarha <jyri.sarha@....fi>
Subject: Re: [PATCH 00/29] drm: Implement state readout support
Hi Tomi,
Thanks for having a look.
On Wed, Oct 08, 2025 at 04:07:57PM +0300, Tomi Valkeinen wrote:
> On 02/09/2025 11:32, Maxime Ripard wrote:
> > Hi,
> >
> > Here's a series that implement what i915 calls "fastboot", ie,
> > initializing the initial KMS state from the hardware state at boot, to
> > skip the first modeset if the firmware already set up the display.
> >
> > This series creates the infrastructure in KMS to create that state by
> > relying on driver specific hooks. It also implements some infrastructure
> > to check during non-blocking commits that the readout helpers work
> > properly by reading out the state that was just committed and comparing
> > it to what was supposed to be commited.
> >
> > This relies on another set of driver hooks to compare the entities
> > states, with helpers providing the default implementation.
> >
> > It then implements the readout support in the TIDSS driver, and was
> > tested with the SK-AM62 board. This board in particular is pretty
> > interesting, since it relies on an DPI to HDMI bridge, and uses the
> > drm_bridge_connector infrastructure.
> >
> > So the readout works with the current state of the art on embedded-ish
> > platforms.
> >
> > The whole thing feels a bit clunky at the moment:
> >
> > - The initial state buildup ties everything together in a state in the
> > old state pointer. It's useful for the initial readout because
> > accessors can then use the usual state accessors to look into the
> > state of other entities. But one of the argument for it was also
> > that for state comparison, it allows to compare the new state
> > (committed) to the old state (readout). It doesn't really work in
> > practice, since in such a case the old state contains the previous
> > hardware state to be freed, and thus we would end up with a memory
> > leak
> >
> > - The framebuffer refcounting is broken.
> >
> > - The tidss atomic_flush waits for the go bit on the initial
> > modesetting, except that if the state is readout we didn't commit
> > anything and the driver will wait forever, eventually resulting in
> > commit timeout
>
> Isn't atomic flush part of the modeset? Why is it called if there's no
> modeset.
No, atomic_flush is ran when we update the planes, so it will trigger
here on the first page flip.
> > - The tidss_crtc_state fields are not read properly at the moment
> > either.
>
> Just because no implemented, or was there something funny with them? I
> guess there's some reverse-mapping that needs to be done.
The bus_format field isn't read properly, I wasn't quite sure what was
going on there.
And also, for bridges, I've yet to figure out a way to read / find the
input/output formats.
> > The main thing works though: the state is picked up properly, doesn't
> > trigger a modeset if what was programmed is the one the first modeset
> > tries to pick as well, will switch properly if it isn't, etc.
>
> This is pretty interesting work. I haven't tested, and I'm sure it still
> breaks in a million ways if used with anything else but the HW you're
> using =).
Thanks :D
> This is related to the boot-splash screen work I've been working on for
> quite a while, although at a different stage.
As far as I'm concerned, once this lands, your work isn't needed at all.
> In my patches I have been trying to avoid hw reset, so that if the DSS
> has been set up by the bootloader, we'll just let it run until we get
> a modeset.
We really only need to power up the hardware around
drm_mode_config_reset.
Once we're done, either the hardware will be active or inactive, but
we'll know for sure.
> And now your series would potentially remove that modeset too, so, in
> theory, we could get up to X/Weston from bootloader with just a single
> modeset in the bootloader.
>
> Of course, fbdev/simpledrm will mess things up there. I had some hacks
> for fbdev too, to retain the bootsplash image, but it was just hacking.
And it works also with fbdev, since fbdev or whatever will trigger a
new commit we can compare to.
Maxime
Download attachment "signature.asc" of type "application/pgp-signature" (274 bytes)
Powered by blists - more mailing lists