lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<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

Powered by Openwall GNU/*/Linux Powered by OpenVZ