On Thu, 23 Apr 2020 17:01:49 +0200 Daniel Vetter <daniel@xxxxxxxx> wrote: > On Tue, Apr 21, 2020 at 4:33 PM Pekka Paalanen <ppaalanen@xxxxxxxxx> wrote: > > > > On Tue, 21 Apr 2020 14:15:52 +0200 > > Daniel Vetter <daniel@xxxxxxxx> 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
Attachment:
pgp3271DuhVCf.pgp
Description: OpenPGP digital signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
