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

[Pekka Paalanen]

On Tue, 28 Apr 2020 16:51:57 +0200
Daniel Vetter <daniel at ffwll.ch> wrote:

> On Fri, Apr 24, 2020 at 11:32:16AM +0300, Pekka Paalanen wrote:
> > On Thu, 23 Apr 2020 17:01:49 +0200
> > Daniel Vetter <daniel at ffwll.ch> wrote:
> >   
> > > On Tue, Apr 21, 2020 at 4:33 PM Pekka Paalanen <ppaalanen at gmail.com> wrote:  
> > > >
> > > > On Tue, 21 Apr 2020 14:15:52 +0200
> > > > Daniel Vetter <daniel at ffwll.ch> wrote:
> > > >    
> > 
> > ...

Hi,

please skip to the TL;DR at the bottom, the rest is just how I ended up
there.

> >   
> > > > > Note that the kernel isn't entire consistent on this. I've looked a bit
> > > > > more closely at stuff. Ignoring content protection I've found following
> > > > > approaches things:
> > > > >
> > > > > - self refresh helpers, which are entirely transparent. Therefore we do a
> > > > >   hack to set allow_modeset when the self-refresh helpers need to do a
> > > > >   modeset, to avoid total surprise for userspace. I think this is only ok
> > > > >   for these kind of behind-the-scenes helpers like self-refresh.
> > > > >
> > > > > - link-status is always reset to "good" when you include any connector,
> > > > >   which might force a modeset. Even when allow_modeset isn't set by
> > > > >   userspace. Maybe we should fix that, but we've discussed forever how to
> > > > >   make sure a bad link isn't ever stuck at "bad" for old userspace, so
> > > > >   we've gone with this. But maybe limiting to only allow_modeset cases
> > > > >   would also work.    
> > > >
> > > > Wait, what do you mean "include any connector"?
> > > >
> > > > What exactly could cause a modeset instead of failure when
> > > > ALLOW_MODESET is not set?    
> > > 
> > > If you do an atomic commit with the connector included that has the
> > > bad link status, then it'll reset it to Good to try to get a full
> > > modeset to restore stuff. If you don't also have ALLOW_MODESET set,
> > > it'll fail and userspace might be sad and not understand what's going
> > > on. We can easily fix this by only forcing the link training to be
> > > fixed if userspace has set ALLOW_MODESET.  
> > 
> > Hi,
> > 
> > oh, that's ok, the fail case is there like it should.
> > 
> > It sounded like even if userspace does not set ALLOW_MODESET, the
> > kernel would still do a modeset in come cases. I'm happy to have
> > misunderstood.  
> 
> Well currently that can go wrong, if you include a connector and a
> link-status is bad. But could be fixed fairly easily.

What do you mean by "include a connector"? Which property on which
object?

Weston always submits more or less full KMS state (known properties on
intended-active outputs) on all updates, on simple pageflips too, so I
am worried that the connector is "included", leading to automatic
papering over link-status failures, and Weston doesn't handle
link-status yet.

Weston does this, because it is the easy thing to do and debug. You
don't have to track back thousands of frames to figure out what the
mode or the connectors are, when something doesn't work like it should.
Or to figure out why some state wasn't emitted when it was supposed to.


> > > > I'd probably not go there, a blind save does not guarantee a good
> > > > state. The fix to "Content Protection" is not necessary (as long as
> > > > userspace does not do a blind save/restore) if we can get the default
> > > > state from the kernel. If we get the default state from the kernel,
> > > > then userspace would be doing a blind restore but not save, meaning
> > > > that the state actually is sane and writable.
> > > >
> > > > I'd love to volunteer for writing the Weston code to make use of "get me
> > > > sane default state" UAPI, but I'm afraid I'm not in that much control
> > > > of my time.    
> > > 
> > > The problem is, what is your default state? Driver defaults (generally
> > > fairly random and mostly everything is off)? After fbcon has done
> > > it's, which might never happen when you've disabling fbdev/fbcon?
> > > Boot-up state from the firmware for drivers like i915 that support
> > > fastboot (and what if that's garbage)? These can all be different too.  
> > 
> > I believe what I want is more or less the driver defaults, or DRM core
> > defaults for common KMS properties. It does not matter if they are
> > arbitrary, as long as they remain the same across kernel versions. They
> > need to be constants, so that I can rely on always getting to the same
> > state on the same piece of hardware when I use the "default state"
> > regardless of what might be happening software wise.
> > 
> > But of course the defaults must be legal values and they should make
> > some sense. I'd consider "neutral", "identity", and "off" to be
> > sensible values, but what that means for each property depends on the
> > exact property. So there may be work to be done to unify the default
> > values across drivers for driver-specific properties.
> > 
> > Having stuff off by default is ok. If that makes things not work, then
> > we'll fix userspace to understand the new properties that are
> > necessary. At no point it can be said to be a kernel regression - at
> > least I very much hope and intend so.
> > 
> > There is always the problem of someone using new userspace first on old
> > kernel that does not expose "default state" so userspace doesn't use
> > "default state". Then upgrading to new kernel that has "default state"
> > that is not good for making things work, but userspace starts using it
> > because it becomes available and fails. Is there any way we can stop
> > that from being seen as a kernel regression?  
> 
> If you upgrade the kernel and existing userspace breaks, it's a
> regression.
> 
> There's no wiggle room in this, the only wiggle room is that sometimes
> people don't report issues, and eventually all affected hw is dead.

Then the only possibility is that the transition from not using default
state to using default state does not change anything. Which means that
the default state must be the state the KMS program would normally
inherit, so, fbcon state?

> > Fbcon has exactly the problem that fbcon might not exist, so we cannot
> > rely on that. Is fbcon even reliable enough to offer constant defaults
> > that could be relied upon to stay the same across kernel versions and
> > software configurations?  
> 
> It's getting hacked on quite a bit, in a fairly ad-hoc fashion. So
> "defaults as fbcon sets them" is something that's under fair bit of
> change.

Yet, it seems to be the only "sane state" defined so far. Maybe we have
to live with it changing from boot to boot.

We also cannot ever have anything better, because of the above issue
"new userspace, old kernel, then upgrade kernel", can we? It doesn't
"break" userspace but it could be a regression for the end user,
because upgrading the kernel changes something that is visible and no
longer the same as before, e.g. breaking monitor calibration or profile
because suddenly some color related property is set differently.

When new KMS properties are introduced, how do people pick their
default values in the kernel when no userspace sets them yet? Are those
defaults immutable for eternity afterwards?

> > State from firmware I would not trust at all, who knows when a firmware
> > upgrade changes things.
> > 
> > What I'm saying is, the "default state" would very much be UABI. Not
> > just how to access it, but the exact values of it for all existing
> > properties. The important invariant is: for the same piece of hardware,
> > I always get the same defaults (if any), regardless of any software or
> > firmware versions.  
> 
> Yup, that's why default state exposed by the kernel is such a pain: It's
> uapi. If we change it, we can break something somewhere. And the problem
> is complex enough that there's going to be lots of opinions about what
> the default should be.

I think the current situation is just *hoping* that kernel doesn't
change its defaults when they are not even defined in any one place.
The defaults userspace currently gets in most cases is whatever it
inherits from fbcon. And you said fbcon changes ad-hoc.

> fbcon is a lot easier here, since there there's at least some agreement
> that a working console should be visible and preferrably right side up and
> all that.

Yeah, but what about gamma, color and other details that the console
usually doesn't care about as long as e.g. red is somewhat reddish?

I remember someone telling me, that fbcon does not reset gamma tables,
which means any gamma using KMS app can mess up the console and all
other KMS apps that don't use gamma.

> > This invariant should guarantee that e.g. measured monitor color
> > profiles stay valid across kernel upgrades. I know, people who actually
> > care about color do not trust even reboots, but if we can let them get
> > away with just verifying the profile instead of wasting hours in
> > re-measuring the profile, that would be good.
> > 
> > When a driver starts supporting a new property, it must set the default
> > value of the property to the value it has been using implicitly before.
> > But if that implicit value was not hardcoded and depends on variable
> > factors, then we probably cannot avoid a change in behaviour when
> > introducing the property or "default values".  
> 
> The problem is in figuring out that implicit value. In some cases I think
> it's even "whatever the hw booted with", but I've tried to handle these
> kinds of issues with atomic at least.
> 
> > > The hard part isn't the uapi, it's the implementation and defining
> > > what you mean. Generally everyone wants their own version of default
> > > value semantics, so this a) encodes policy, which we try to avoid in
> > > the kernel and b) will be epic amounts of endless bikeshedding and
> > > fighting between use-cases.  
> > 
> > It's a policy, yes. But we need a single source of arbitrary default
> > values, so that we have a strategy for achieving the same hardware
> > state even when userspace does not understand all KMS properties:
> > 
> > - after losing and re-gaining DRM master status (e.g. temporary
> >   VT-switch to another KMS program that modifies state I don't
> >   understand), and
> > 
> > - across changes in the software stack.
> > 
> > The former is the most important point, the latter would be good to
> > have.  
> 
> But you can fix the above use-case also with save&restore. Which we're
> trying to support (but even that has bugs).

Ok, so we're back to blind save & restore with undefined default state,
that depends not only on the hardware, the firmware, the kernel and
drivers, but also what was the previous KMS client doing before you
took over.

The only way to avoid undefined state is for all KMS programs to learn
to know all existing properties, and maybe warn the user if any
unrecognised properties appear. If KMS programs refuse to work when
unrecognised properties appear, that means you will never be able to
add anything to the properties again. Then you probably add caps for
new properties and we are back to square one except the UAPI is a lot
more complicated and userspace can longer warn the end user about
potentially bad KMS state.

Since the kernel refuses to provide a constant default state, maybe we
need a userspace library for that - one that *must* be updated for
every new property added to the kernel. Then we could circumvent the
kernel stable UAPI rules to provide predictable behaviour for userspace
that wants it.

Except, that too, would hit the "new userspace, old kernel, then
upgrade kernel" scenario where a kernel upgrade breaks userspace
because it allows userspace to start using new properties with legal
and working but different values from what it had before.

Are we really in a dead end with this?

> > We already have a policy in the kernel: fbcon KMS state. If everyone
> > can agree to that state, it's fine by me, but it also needs to be
> > available when fbcon support is disabled. Maybe this could be a path
> > forward? Expose what state fbcon would use, regardless of whether fbcon
> > exists. It would probably miss the latter point, though, but that could
> > be acceptable.
> > 
> > If we could trust that the KMS state is "sane" when any KMS program
> > starts, then this problem doesn't exist: just read out the KMS state on
> > start-up and use that. But since we don't know where the existing KMS
> > state comes from, it could be anything. And for smooth hand-off between
> > display servers, we can't have per DRM file description KMS state
> > either.  
> 
> You can assume that boot-up state is about as "sane" as it gets. So a
> system compositor (in logind or whatever, it already deals with this
> stuff) can reset the screen.

System compositors largely do not exist, the whole concept has been
dismissed. If logind would reset the screen, we would no longer have
seamless hand-off between any display servers and anything. Logind
could hand off the state for others to use as default state though.

How would one get the boot-up state?

Why do we need a userspace daemon to record KMS boot-up state instead
of the kernel giving it to userspace? How would that daemon itself get
the boot-up state if it has already been modified by fbcon?

> For everything else I'm not sure kernel defaults actually fixes anything.
> It just gives everyont the illusion that now the problem is fixed, except
> the fix is now kernel uapi, which means we can never change it.

If we want constant defaults, then by definition they cannot ever be
changed. It doesn't matter where it comes from.

I suppose the conclusion is that we cannot have constant defaults at
all. The only defaults we can have are undefined. If someone does not
like it, he needs to implement unrecognised properties in whatever
userspace he cares about.

> The upshot of doing the exact same as a logind request along the lines of
> "pls reset display to sane state, thx" is that logind can be changed much
> easier than kernel uapi in case we get this wrong. Which we will.

Sorry, I don't see the difference if the goal is to have fixed
defaults. The only difference with logind is that there is no angry
Linus.


TL;DR:

If blind save & restore (but restricted only to cases where KMS state
may have been lost!) works, it will ensure that a running userspace
program has what it was started with, but it does not guarantee that
the state is the same between restarts of the program.

It seems that the default state is always undefined, and cannot be
defined because whatever one defines might not always result in a
picture on screen. Bugs in defined default state cannot be fixed, by
definition as the default state is immutable once defined.

Ensuring consistent state between restarts is the responsibility of
userspace and each KMS program individually. They might attempt to
blindly save KMS state in a file to be re-used on the next start.


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/20200429/04d52830/attachment-0001.sig>