Date: Wed, 10 Jan 2024 13:26:46 +0000
From: Daniel Stone <daniel@...ishbar.org>
To: Andri Yngvason <andri@...vason.is>, Harry Wentland <harry.wentland@....com>, 
	Leo Li <sunpeng.li@....com>, Rodrigo Siqueira <Rodrigo.Siqueira@....com>, 
	Alex Deucher <alexander.deucher@....com>, Christian König <christian.koenig@....com>, 
	"Pan, Xinhui" <Xinhui.Pan@....com>, David Airlie <airlied@...il.com>, 
	Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>, 
	Thomas Zimmermann <tzimmermann@...e.de>, Jani Nikula <jani.nikula@...ux.intel.com>, 
	Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>, Rodrigo Vivi <rodrigo.vivi@...el.com>, 
	Tvrtko Ursulin <tvrtko.ursulin@...ux.intel.com>, amd-gfx@...ts.freedesktop.org, 
	dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org, 
	intel-gfx@...ts.freedesktop.org, Simon Ser <contact@...rsion.fr>, 
	Werner Sembach <wse@...edocomputers.com>
Cc: Daniel Vetter <daniel@...ll.ch>
Subject: Re: [PATCH 2/7] drm/uAPI: Add "active color format" drm property as
 feedback for userspace

Hi,

On Wed, 10 Jan 2024 at 10:44, Daniel Vetter <daniel@...ll.ch> wrote:
> On Tue, Jan 09, 2024 at 11:12:11PM +0000, Andri Yngvason wrote:
> > ţri., 9. jan. 2024 kl. 22:32 skrifađi Daniel Stone <daniel@...ishbar.org>:
> > > How does userspace determine what's happened without polling? Will it
> > > only change after an `ALLOW_MODESET` commit, and be guaranteed to be
> > > updated after the commit has completed and the event being sent?
> > > Should it send a HOTPLUG event? Other?
> > >
> >
> > Userspace does not determine what's happened without polling. The purpose
> > of this property is not for programmatic verification that the preferred
> > property was applied. It is my understanding that it's mostly intended for
> > debugging purposes. It should only change as a consequence of modesetting,
> > although I didn't actually look into what happens if you set the "preferred
> > color format" outside of a modeset.
>
> This feels a bit irky to me, since we don't have any synchronization and
> it kinda breaks how userspace gets to know about stuff.
>
> For context the current immutable properties are all stuff that's derived
> from the sink (like edid, or things like that). Userspace is guaranteed to
> get a hotplug event (minus driver bugs as usual) if any of these change,
> and we've added infrastructure so that the hotplug event even contains the
> specific property so that userspace can avoid re-read (which can cause
> some costly re-probing) them all.

Right.

> [...]
>
> This thing here works entirely differently, and I think we need somewhat
> new semantics for this:
>
> - I agree it should be read-only for userspace, so immutable sounds right.
>
> - But I also agree with Daniel Stone that this should be tied more
>   directly to the modeset state.
>
> So I think the better approach would be to put the output type into
> drm_connector_state, require that drivers compute it in their
> ->atomic_check code (which in the future would allow us to report it out
> for TEST_ONLY commits too), and so guarantee that the value is updated
> right after the kms ioctl returns (and not somewhen later for non-blocking
> commits).

That's exactly the point. Whether userspace gets an explicit
notification or it has to 'know' when to read isn't much of an issue -
just as long as it's well defined. I think the suggestion of 'do it in
atomic_check and then it's guaranteed to be readable when the commit
completes' is a good one.

I do still have some reservations - for instance, why do we have the
fallback to auto when userspace has explicitly requested a certain
type? - but they may have been covered previously.

Cheers,
Daniel

Powered by blists - more mailing lists