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

[Pekka Paalanen]

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 ?

> > Assuming the property is userspace-writable: if kernel goes and
> > changes the property value on its own, it will very likely be just
> > overwritten by userspace right after if userspace does not manage to
> > process the uevent first. If that happens and userspace later
> > processes the uevent, userspace queries the kernel for the current
> > proprerty state which is now what userspace wrote, not what firmware
> > set.
> >
> > Therefore you end up with the firmware hotkey working only randomly.
> >
> > It would be much better to have the hotkey events delivered to
> > userspace so that userspace can control the privacy screen and
> > everything will be reliable, both the hotkeys and any GUI for it. The
> > other reliable option is that userspace must never be able to change
> > privacy screen state, only the hardware hotkeys can.  
> 
> We have fancy new uevents which give you both the connector and the
> property, so you know what's going on.

Yeah, userspace can know what changed, but not the new value with the
uevent.

> Also, a property which userspace and the kernel can race like you
> describe above is broken. We don't have these, and we won't merge
> them.

That's what I would expect, yes, but I'm not that optimistic that the
knowledgeable maintainers can always catch them all, which is why I try
to comment on UAPI additions that look fishy to me.

> The ones we do have the state transitions are a lot clearer, and
> userspace overwriting what the kernel has done is not actually going
> to cause a big pain. At least in the sense of the transition will be
> lost, since for e.g. both link_status and hdcp the value the kernel
> sets is not a value userspace can set. But it can result in problems
> if you just blindly write them again causing modesets you'd not
> expect.

Yeah, HDCP "Content Protection" is very carefully engineered to cope
with the awkwardness that both userspace and kernel will write it. It's
cumbersome.

Btw. I searched for all occurrences of link_status in
https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html and it seems it
only has two possible values, good and bad, and no mention whether it
is writable. Looks like it's writable. There does not seem to be a) an
explanation how exactly it needs to the handled (writing it does
something? what can you write?) or b) any way discern between kernel
and userspace set values like HDCP "Content Protection" has.

Weston does not support link_status yet, FWIW.

Unless you have some other idea on how to reset unknown KMS state to
sensible defaults that are always the same, I think any KMS property
where the query can return a value that cannot be written back to it
with ALLOW_MODESET is broken.


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/3257177a/attachment.sig>