Operating KMS UAPI (Re: RFC: Drm-connector properties managed by another driver / privacy screen support)

[Pekka Paalanen]

On Mon, 20 Apr 2020 11:27:04 +0300
Pekka Paalanen <ppaalanen at gmail.com> wrote:

> On Fri, 17 Apr 2020 16:17:18 +0200
> Daniel Vetter <daniel at ffwll.ch> wrote:
> 
> > On Fri, Apr 17, 2020 at 11:02 AM Pekka Paalanen <ppaalanen at gmail.com> wrote:  
> > >
> > > Hi,
> > >
> > > let's think about how userspace uses atomic KMS UAPI. The simplest way
> > > to use atomic correctly is that userspace will for every update send the
> > > full, complete set of all properties that exist, both known and unknown
> > > to userspace (to recover from temporarily VT-switching to another KMS
> > > program that changes unknown properties). Attempting to track which
> > > properties already have their correct values in the kernel is extra
> > > work for just extra bugs.    
> > 
> > Uh if you do that you'll get random surprising failures if you don't
> > also set ALLOW_MODESET, because that way you'll automatically repair
> > link failures and stuff like that. I'm assuming your userspace only
> > supplies all the properties for crtc and planes, and leaves connectors
> > as-is? Otherwise you already have some fun bugs.
> > 
> > In general I'd say userspace shouldn't write stuff it doesn't
> > understand. If you limit yourself to just the properties you do want
> > to (re)set, that's safe. But if you just blindly write everything all
> > the time, random modesets, and hence random failures if you don't set
> > ALLOW_MODESET.  
> 
> Hi,
> 
> how should userspace KMS program A recover from the situation when
> switching the VT back from KMS program B who changed properties that
> program A does not recognise? (I believe Weston does not recover at
> the moment.) This is very important for getting e.g. reliable color
> reproduction, since not all KMS programs are always up-to-date with
> everything the kernel exposes and people may switch between them. Not
> resetting everything may even encourage people to write hacks where you
> temporarily VT-switch away, run a KMS program to set one property, and
> then switch back assuming the property remains set. I have already seen
> someone mention they can enable VRR behind the display server's back
> like this.
> 
> I don't think Weston records and re-sets unknown properties yet, but I
> assumed it is what it needs to do to be able to reliably recover from
> VT-switches. In that case ALLOW_MODESET is of course set since all
> state is unknown and assumed bad.
> 
> I do believe Weston re-submits *everything* it knows about every
> update, except for CRTCs and connectors it has already disabled and
> knows are in disabled state (this could change though).
> 
> However, during steady-state operation when ALLOW_MODESET should not be
> necessary, is it still harmful to re-program *all* properties on every
> update?
> 
> After all, the kernel will just no-op all property setting where the
> value is already the right one, does it not?
> 
> The only "random" KMS state is the properties the userspace KMS
> program does not know that are set on start-up. I have been assuming
> that as long as you had fbdev active before the KMS program started,
> the unknown properties have "harmless" default values. And maybe even at
> driver device init if fbdev does not exist?
> 
> Is there something more up-to-date than
> https://blog.ffwll.ch/2016/01/vt-switching-with-atomic-modeset.html ?

Thinking more, would the below work?

Actor: a KMS userspace program, e.g. a display server

- On start-up, read all KMS properties and their values. The properties
  that are not recognised are saved in a set called "reset unknowns"
  with their current values.

  Optional: The program commits the "reset unknown" state to KMS with
  ALLOW_MODESET to ensure it all is writable as is; if that fails,
  there is no guarantee that the program could recover later, so it's
  best to abort in that case. This could be part of the initial
  modeset, too.

- When the program has lost and regained DRM master status, meaning
  that (unrecognised) KMS state is potentially incorrect, prepare an
  atomic commit with "reset unknowns" set and add all the recognised
  state the program knows of on top. This resets everything to like it
  was, with ALLOW_MODESET.

- At any other time, do not use the "reset unknowns" set.

The final point is the important one. I have assumed it would be safe
to use always, but apparently not? Good thing I haven't yet written
code to do that.

You have to recognise the property to know if it is safe to set
needlessly (for convenience in both code simplicity and ease of
debugging)?

Also, when using "reset unknowns" set, it actually has to be
partitioned by KMS objects (CRTC, connector, plane...) so if e.g. a
connector no longer exist, you don't attempt to set it.

However, this still leaves writable properties whose value read is not
legal to write as broken. Let's pray that fbcon or a system compositor
will never succeed in enabling HDCP...


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20200420/dbdecadd/attachment.sig>