On Fri, 17 Apr 2020 16:17:18 +0200 Daniel Vetter <daniel@xxxxxxxx> wrote: > On Fri, Apr 17, 2020 at 11:02 AM Pekka Paalanen <ppaalanen@xxxxxxxxx> 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
Attachment:
pgpR7euUUaaiM.pgp
Description: OpenPGP digital signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
