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

Fri Apr 24 08:32:16 UTC 2020

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:
> >  

...

> > > 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.

> > Does that mean that I'll never need to implement link-status handling
> > in Weston, because the kernel will recover the link anyway? If the
> > kernel does that, then what's the point of having a link-status
> > property to begin with?  
> 
> Well generally all your compositor does all day long is flip buffers.
> So you'll never get the kernel into restoring the link. Hence the
> uevent, so that the compositor can a) update the mode list, which
> might have changed b) do the modeset to restore stuff. The
> auto-fallback is so that stuff like users manually
> disabling/re-enabling an output actually helps with fixing stuff.

Ok, that's fine.

If link goes bad while Weston does not implement link-status, I'm
totally happy to degree any fallout from that to be a Weston bug. It
has never worked (right?), so it cannot be a kernel regression. It is
quite important to me to be able to say to that a failure is a Weston
bug, not a kernel regression, to not piss on the kernel devs.

> > > - I guess we could do stuff that only fires off when allow_modeset is set,
> > >   but I haven't found some examples. I thought we've had some outside of
> > >   self-refresh helpers already. The closest I've found is again
> > >   link-status, which never allows userspace to set BAD, and silently
> > >   upgrades to GOOD. So that userspace doing a blind safe/restore can't
> > >   wreak stuff permanently.  
> >
> > Sounds like link-status was designed with a blind save/restore in mind.  
> 
> Yeah that part we didn't screw up.
> 
> > > It's all a bit nasty :-/
> > >
> > > I think we should at least allow userspace to do a blind restore with
> > > allow_modeset and not expect bad side-effects. That would mean fixing at
> > > least the content protection stuff.
> > >
> > > Plus documenting this in the kernel somewhere. As the official thing to
> > > do. But maybe we want some actual userspace doing this before we enshrine
> > > it as official policy. The content protection fix is a one-liner and can
> > > be cc'ed stable.  
> >
> > 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?

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?

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.

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 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.

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.

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/20200424/42994b73/attachment.sig>