Re: Operating KMS UAPI (Re: RFC: Drm-connector properties managed by another driver / privacy screen support)
On Tue, 5 May 2020 10:48:52 +0200 Daniel Vetter wrote: > Refocusing on where I think we still have a bit a disconnnect. > > On Mon, May 04, 2020 at 03:22:28PM +0300, Pekka Paalanen wrote: > > On Mon, 4 May 2020 13:00:02 +0200 > > Daniel Vetter wrote: > > > On Mon, May 4, 2020 at 11:49 AM Pekka Paalanen > > > wrote: > > > > On Thu, 30 Apr 2020 15:53:23 +0200 > > > > Daniel Vetter wrote: > > > I guess my point is, this is a lot bigger problem than just the > > > default state. This = vt switching that doesn't look horrible and > > > doesn't result in artifacts and issues on the new compositor. > > > > I am interested in getting reliably to the same hardware state as you > > used to have before, either after reboots or after vt-switches. The > > transition does not have to be guaranteed to be smooth, but at the same > > time the restore policy should not exclude the possibility of a > > smooth transition. > > So my point is kinda that if you want both, then the only way to get there > to have a very clear set of what's allowed to be left behind be the > outgoing compositor. For example: > - only primary plane > - only untiled > - no color transform magic > - ... Hi, agreed, and to this I was implicitly adding "for everything else the old compositor must leave behind the so called default state". That means the new compositor restoring defaults to the properties it does not understand is a no-op, unless the old compositor violated the smooth transition rules, in which case it might not be a smooth transition. If on some system, the default state does not result in a "usable" picture on screen, then a compositor that does not understand the property will always be broken and has always been broken. If such compositor ever worked ok on such system, it was because it inherited a property value that made things work. So if using defaults breaks things, it means the defaults were not actually the defaults. This definition probably means that defaults cannot be universal constants, but I think it would be enough if they were constants given the hardware and firmware at hand. IOW, not changed by simply rebooting or upgrading the kernel. > > Imo this is a related problem to the save/restore topic, since if one > compositor does something the new one doesn't understand (e.g. tiled > buffers with modifiers, and new compositor doesn't use getfb2), then it's > going to break. > > Similar, if the old compositor sets a new color transform property that > the new compositor doesn't understand, then you get a mess. > > Blind restore handles the permanent mess issue, but it doesn't handle the > smooth transition issue. But both problems are of the "old compositor did > something the new compositor doesn't understand", hence why I chuck them > into the same bin. And the issue with a blind save/restore, or kernel > defaults, or any of the solutions proposed here is that they pretty much > guarantee non-smooth transitions, they only solve the permanent damange > part of the problem. Right, except I disagree on the "guarantee non-smooth transition", as I explained above. > I think to solve both, we need some kind of proper compositor switching > protocol, e.g. using logind: > - old compositor transitions to the minimal config as specified somewhere > - logind forces all other properties to "defaults", whether that's > restoring boot-up defaults or defaults obtained from the kernel or > something else is tbd In my mind, the new compositor would do this step. If the old compositor did its job, it's a no-op. > - logind maybe even does a transition if there's multiple version of the > protocol, e.g. v2 allows modifiers, v1 only untiled, so it'd need to do > a blit to untiled if the new compositor only supports v1 switching > protocol That would be fancy, but I'm not sure it's necessary. Maybe it becomes useful one day, and then that day we can look into involving actual protocol or logind. I just don't see a problem implementing the de facto protocol v1 that already exists without logind or such. > - new compositor takes over, and can continue the smooth transition since > it's a well-defined starting state with limited feature usage, _and_ > everything else is reset to "defaults" > > I fear that if we only solve the "resets to defaults" issue then we can > draw ourselves into a corner for the smooth transition stuff, if e.g. the > wrong entity in the above dance forces the reset to defaults. I guess we fundamentally agree. It just doesn't matter who does the reset to defaults if the old compositor does not, the transition cannot be smooth. To me it seems the key here is that the old compositor *must* use "a simple KMS setup" and reset everything else to defaults. Actually, that's what we require in practise already. If the old compositor leaves something funny in KMS state, the new compositor will either glitch or be permanently broken when taking over. The only thing that needs
Re: Operating KMS UAPI (Re: RFC: Drm-connector properties managed by another driver / privacy screen support)
Refocusing on where I think we still have a bit a disconnnect. On Mon, May 04, 2020 at 03:22:28PM +0300, Pekka Paalanen wrote: > On Mon, 4 May 2020 13:00:02 +0200 > Daniel Vetter wrote: > > On Mon, May 4, 2020 at 11:49 AM Pekka Paalanen wrote: > > > On Thu, 30 Apr 2020 15:53:23 +0200 > > > Daniel Vetter wrote: > > I guess my point is, this is a lot bigger problem than just the > > default state. This = vt switching that doesn't look horrible and > > doesn't result in artifacts and issues on the new compositor. > > I am interested in getting reliably to the same hardware state as you > used to have before, either after reboots or after vt-switches. The > transition does not have to be guaranteed to be smooth, but at the same > time the restore policy should not exclude the possibility of a > smooth transition. So my point is kinda that if you want both, then the only way to get there to have a very clear set of what's allowed to be left behind be the outgoing compositor. For example: - only primary plane - only untiled - no color transform magic - ... Imo this is a related problem to the save/restore topic, since if one compositor does something the new one doesn't understand (e.g. tiled buffers with modifiers, and new compositor doesn't use getfb2), then it's going to break. Similar, if the old compositor sets a new color transform property that the new compositor doesn't understand, then you get a mess. Blind restore handles the permanent mess issue, but it doesn't handle the smooth transition issue. But both problems are of the "old compositor did something the new compositor doesn't understand", hence why I chuck them into the same bin. And the issue with a blind save/restore, or kernel defaults, or any of the solutions proposed here is that they pretty much guarantee non-smooth transitions, they only solve the permanent damange part of the problem. I think to solve both, we need some kind of proper compositor switching protocol, e.g. using logind: - old compositor transitions to the minimal config as specified somewhere - logind forces all other properties to "defaults", whether that's restoring boot-up defaults or defaults obtained from the kernel or something else is tbd - logind maybe even does a transition if there's multiple version of the protocol, e.g. v2 allows modifiers, v1 only untiled, so it'd need to do a blit to untiled if the new compositor only supports v1 switching protocol - new compositor takes over, and can continue the smooth transition since it's a well-defined starting state with limited feature usage, _and_ everything else is reset to "defaults" I fear that if we only solve the "resets to defaults" issue then we can draw ourselves into a corner for the smooth transition stuff, if e.g. the wrong entity in the above dance forces the reset to defaults. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Operating KMS UAPI (Re: RFC: Drm-connector properties managed by another driver / privacy screen support)
On Mon, 4 May 2020 13:00:02 +0200 Daniel Vetter wrote: > On Mon, May 4, 2020 at 11:49 AM Pekka Paalanen wrote: > > > > On Thu, 30 Apr 2020 15:53:23 +0200 > > Daniel Vetter wrote: > > > > > On Wed, Apr 29, 2020 at 01:07:54PM +0300, Pekka Paalanen wrote: > > > > On Tue, 28 Apr 2020 16:51:57 +0200 > > > > Daniel Vetter 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 wrote: > > > > > > > > > > > > > On Tue, Apr 21, 2020 at 4:33 PM Pekka Paalanen > > > > > > > wrote: > > > > > > > > > > > > > > > > On Tue, 21 Apr 2020 14:15:52 +0200 > > > > > > > > Daniel Vetter wrote: > > > > > > > > > > > > > > > > > Include a connector = you have a property for a connector included in your > > > atomic update. Sounds like you do that, so if you have a real world > > > link-status failure, then things go a bit boom already. > > > > Are you talking about setting a connector's property, like "CRTC_ID", > > "Content Protection", "HDCP Content Type"? Weston sets all of those on > > every update if they exist. Or is it about any property on any > > connector, not just a specific property on the specific connector, or > > any property on the specific connector? > > Any property on the specific connector where link-status has gone "bad". Ok, now I got it clearly. Thanks. > > Also CRTC properties "MODE_ID" and "ACTIVE" are set on every update, > > modeset or not. Weston just trusts that no-op changes in state do not > > require a modeset > > Yup that's generally valid assumption, but also that's enough to hit > the link-status == "bad" forced modeset I think. > > > Is it not the opposite? If a link fails, then Weston will produce a > > glitch when the kernel automatically re-trains the link, and then > > everything continues happily? > > That's the soft link failure, where the kernel can recover on its own. > There's the harder one where the new link status is degraded, and the > old mode doesn't fit. Or the kernel just threw a fit and can't make > stuff work anymore, and waits for the next userspace commit. Iirc for > MST links we can't easily do this because locking, so gets pushed to a > full modeset commit. Or something like that. > > > > I guess we need a kernel patch to make sure link-status only gets fixed > > > when userspace is ok with a modeset. > > > > That would be nice, but would it not also mean the above Weston case > > ends up with a permanent black screen instead of a temporary glitch? > > > > Or is there a hotplug uevent at play here too? > > Yeah you get a hotplug uevent, so as long as you eventually process > that and do a full modeset should be ok. I suppose Weston falls short there. Getting a hotplug event does not force a modeset on all active CRTCs. Weston does re-read resources and properties, but it does not trigger anything like re-picking the video mode if the connection status and EDID remains the same. I hope we can say Weston never worked with link failures to begin with, and fix things properly through the link status property in Weston. > > > > 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. > > > > > > > > ... > > > > > One idea I proposed on irc is that logind would capture the boot-up state, > > > and you'd do a loginctl reset-screen cmd to reset it if something broke > > > somewhere. logind already has the code for forced vt switching, that feels > > > like a reasonable extension of that. > > > > Needing to run a command manually to restore the screen instead of > > "simply" switching to the graphical login manager to re-gain user > > control seems a bit difficult. > > But that requires mind reading skills. Computer can't tell the "pls > don't wreak stuff, I want smooth switching" from "pls wreak stuff, > because it's glitching" wish the user has when vt-switching. No, it doesn't. The graphical login manager already saved its default state and it will always reinstate its default state. The session compositor hopefully inherits the same or at least compatible default state from the login manager, so the session compositor has it too. So smooth hand-over is always possible unless the session compositor goes out of its way to set otherwise. If the session compositor screws something up like a gamma table, the login manager's default state will just overwrite the bad data anyway. Whether that becomes a smooth transition or not depends on whether the reset needs a modeset and if the old FB is understandable. The session will be permanently broken, as it will re-break when switching back to it, but at least the login manager remains always accessible. > > > Then you could pick betw
Re: Operating KMS UAPI (Re: RFC: Drm-connector properties managed by another driver / privacy screen support)
On Mon, May 4, 2020 at 11:49 AM Pekka Paalanen wrote: > > On Thu, 30 Apr 2020 15:53:23 +0200 > Daniel Vetter wrote: > > > On Wed, Apr 29, 2020 at 01:07:54PM +0300, Pekka Paalanen wrote: > > > On Tue, 28 Apr 2020 16:51:57 +0200 > > > Daniel Vetter 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 wrote: > > > > > > > > > > > On Tue, Apr 21, 2020 at 4:33 PM Pekka Paalanen > > > > > > wrote: > > > > > > > > > > > > > > On Tue, 21 Apr 2020 14:15:52 +0200 > > > > > > > Daniel Vetter 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. > > > > Include a connector = you have a property for a connector included in your > > atomic update. Sounds like you do that, so if you have a real world > > link-status failure, then things go a bit boom already. > > Are you talking about setting a connector's property, like "CRTC_ID", > "Content Protection", "HDCP Content Type"? Weston sets all of those on > every update if they exist. Or is it about any property on any > connector, not just a specific property on the specific connector, or > any property on the specific connector? Any property on the specific connector where link-status has gone "bad". > Also CRTC properties "MODE_ID" and "ACTIVE" are set on every update, > modeset or not. Weston just trusts that no-op changes in state do not > require a modeset Yup that's generally valid assumption, but also that's enough to hit the link-status == "bad" forced modeset I think. > Is it not the opposite? If a link fails, then Weston will produce a > glitch when the kernel automatically re-trains the link, and then > everything continues happily? That's the soft link failure, where the kernel can recover on its own. There's the harder one where the new link status is degraded, and the old mode doesn't fit. Or the kernel just threw a fit and can't make stuff work anymore, and waits for the next userspace commit. Iirc for MST links we can't easily do this because locking, so gets pushed to a full modeset commit. Or something like that. > > I g
Re: Operating KMS UAPI (Re: RFC: Drm-connector properties managed by another driver / privacy screen support)
On Thu, 30 Apr 2020 15:53:23 +0200 Daniel Vetter wrote: > On Wed, Apr 29, 2020 at 01:07:54PM +0300, Pekka Paalanen wrote: > > On Tue, 28 Apr 2020 16:51:57 +0200 > > Daniel Vetter 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 wrote: > > > > > > > > > On Tue, Apr 21, 2020 at 4:33 PM Pekka Paalanen > > > > > wrote: > > > > > > > > > > > > On Tue, 21 Apr 2020 14:15:52 +0200 > > > > > > Daniel Vetter 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. > > Include a connector = you have a property for a connector included in your > atomic update. Sounds like you do that, so if you have a real world > link-status failure, then things go a bit boom already. Are you talking about setting a connector's property, like "CRTC_ID", "Content Protection", "HDCP Content Type"? Weston sets all of those on every update if they exist. Or is it about any property on any connector, not just a specific property on the specific connector, or any property on the specific connector? Also CRTC properties "MODE_ID" and "ACTIVE" are set on every update, modeset or not. Weston just trusts that no-op changes in state do not require a modeset. Is it not the opposite? If a link fails, then Weston will produce a glitch when the kernel automatically re-trains the link, and then everything continues happily? > I guess we need a kernel patch to make sure link-status only gets fixed > when userspace is ok with a modeset. That would be nice, but would it not also mean the above Weston case ends up with a permanent black screen instead of a temporary glitch? Or is there a hotplug uevent at play here too? > > 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. > > ... > One idea I proposed on irc is that logind would capture the boot-up state, > and you'd do a loginctl reset-screen cmd to reset it if something broke > somewhere. logind already has the c
Re: Operating KMS UAPI (Re: RFC: Drm-connector properties managed by another driver / privacy screen support)
On Wed, Apr 29, 2020 at 01:07:54PM +0300, Pekka Paalanen wrote: > On Tue, 28 Apr 2020 16:51:57 +0200 > Daniel Vetter 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 wrote: > > > > > > > On Tue, Apr 21, 2020 at 4:33 PM Pekka Paalanen > > > > wrote: > > > > > > > > > > On Tue, 21 Apr 2020 14:15:52 +0200 > > > > > Daniel Vetter 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. Include a connector = you have a property for a connector included in your atomic update. Sounds like you do that, so if you have a real world link-status failure, then things go a bit boom already. I guess we need a kernel patch to make sure link-status only gets fixed when userspace is ok with a modeset. > 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 b
Re: Operating KMS UAPI (Re: RFC: Drm-connector properties managed by another driver / privacy screen support)
On Tue, 28 Apr 2020 16:51:57 +0200 Daniel Vetter 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 wrote: > > > > > On Tue, Apr 21, 2020 at 4:33 PM Pekka Paalanen > > > wrote: > > > > > > > > On Tue, 21 Apr 2020 14:15:52 +0200 > > > > Daniel Vetter 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'
Re: Operating KMS UAPI (Re: RFC: Drm-connector properties managed by another driver / privacy screen support)
On Fri, Apr 24, 2020 at 11:32:16AM +0300, Pekka Paalanen wrote: > On Thu, 23 Apr 2020 17:01:49 +0200 > Daniel Vetter wrote: > > > On Tue, Apr 21, 2020 at 4:33 PM Pekka Paalanen wrote: > > > > > > On Tue, 21 Apr 2020 14:15:52 +0200 > > > Daniel Vetter 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. Well currently that can go wrong, if you include a connector and a link-status is bad. But could be fixed fairly easily. > > > 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
Re: Operating KMS UAPI (Re: RFC: Drm-connector properties managed by another driver / privacy screen support)
On Thu, 23 Apr 2020 17:01:49 +0200 Daniel Vetter wrote: > On Tue, Apr 21, 2020 at 4:33 PM Pekka Paalanen wrote: > > > > On Tue, 21 Apr 2020 14:15:52 +0200 > > Daniel Vetter 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
Re: Operating KMS UAPI (Re: RFC: Drm-connector properties managed by another driver / privacy screen support)
On Tue, Apr 21, 2020 at 4:33 PM Pekka Paalanen wrote: > > On Tue, 21 Apr 2020 14:15:52 +0200 > Daniel Vetter wrote: > > > On Mon, Apr 20, 2020 at 01:04:20PM +0300, Pekka Paalanen wrote: > > > On Mon, 20 Apr 2020 11:27:04 +0300 > > > Pekka Paalanen wrote: > > > > > > > On Fri, 17 Apr 2020 16:17:18 +0200 > > > > Daniel Vetter wrote: > > > > > > > > > On Fri, Apr 17, 2020 at 11:02 AM Pekka Paalanen > > > > > 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 ? > > > > Sadly, nothing changed since then. > > > > > Thinking more, would the below work? > > > > Yup, this would somewhat work. Except not always, I've found one case > > where even this goes wrong: > > > > - Content-protection property, if enabled, has the kernel automatically > > switch to enabled if hdcp is actually on and authenticated and all that. > > Writing back that enabled value will fail. But there's good chances that > > at boot-up content protection isn't enabled, so should work out nicely. > > > > - We could fix this by silently downcasting enabled to requested, but > > might still lead to surprises since it makes hdcp rather more sticky > > than some users might like. > > The fix doesn't make HDCP any more or less sticky, it just makes it > possible to not fail a resetting atomic commit. Without a resetting > commit, it will remain DESIRED/ENABLED. > > If "Content Protection" reads back as DESIRED, userspace that relies on > read-back for reset will reset it to DESIRED. Could as well be ENABLED > that the kernel just takes as DESIRED
Re: Operating KMS UAPI (Re: RFC: Drm-connector properties managed by another driver / privacy screen support)
On Tuesday, April 21, 2020 4:33 PM, Pekka Paalanen wrote: > 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. I'm interested in this problem too. If someone writes the kernel side, I'll gladly write user-space for it. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Operating KMS UAPI (Re: RFC: Drm-connector properties managed by another driver / privacy screen support)
On Tue, 21 Apr 2020 14:15:52 +0200 Daniel Vetter wrote: > On Mon, Apr 20, 2020 at 01:04:20PM +0300, Pekka Paalanen wrote: > > On Mon, 20 Apr 2020 11:27:04 +0300 > > Pekka Paalanen wrote: > > > > > On Fri, 17 Apr 2020 16:17:18 +0200 > > > Daniel Vetter wrote: > > > > > > > On Fri, Apr 17, 2020 at 11:02 AM Pekka Paalanen > > > > 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 ? > > Sadly, nothing changed since then. > > > Thinking more, would the below work? > > Yup, this would somewhat work. Except not always, I've found one case > where even this goes wrong: > > - Content-protection property, if enabled, has the kernel automatically > switch to enabled if hdcp is actually on and authenticated and all that. > Writing back that enabled value will fail. But there's good chances that > at boot-up content protection isn't enabled, so should work out nicely. > > - We could fix this by silently downcasting enabled to requested, but > might still lead to surprises since it makes hdcp rather more sticky > than some users might like. The fix doesn't make HDCP any more or less sticky, it just makes it possible to not fail a resetting atomic commit. Without a resetting commit, it will remain DESIRED/ENABLED. If "Content Protection" reads back as DESIRED, userspace that relies on read-back for reset will reset it to DESIRED. Could as well be ENABLED that the kernel just takes as DESIRED when written. > > Actor: a KMS userspace program, e.g. a display server > > > > - On start-up, read all KMS properties and their values. The properties > > that are not recognised are saved in a set called "reset unknowns" > > with
Re: Operating KMS UAPI (Re: RFC: Drm-connector properties managed by another driver / privacy screen support)
On Mon, Apr 20, 2020 at 01:04:20PM +0300, Pekka Paalanen wrote: > On Mon, 20 Apr 2020 11:27:04 +0300 > Pekka Paalanen wrote: > > > On Fri, 17 Apr 2020 16:17:18 +0200 > > Daniel Vetter wrote: > > > > > On Fri, Apr 17, 2020 at 11:02 AM Pekka Paalanen > > > 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 ? Sadly, nothing changed since then. > Thinking more, would the below work? Yup, this would somewhat work. Except not always, I've found one case where even this goes wrong: - Content-protection property, if enabled, has the kernel automatically switch to enabled if hdcp is actually on and authenticated and all that. Writing back that enabled value will fail. But there's good chances that at boot-up content protection isn't enabled, so should work out nicely. - We could fix this by silently downcasting enabled to requested, but might still lead to surprises since it makes hdcp rather more sticky than some users might like. > Actor: a KMS userspace program, e.g. a display server > > - On start-up, read all KMS properties and their values. The properties > that are not recognised are saved in a set called "reset unknowns" > with their current values. > > Optional: The program commits the "reset unknown" state to KMS with > ALLOW_MODESET to ensure it all is writable as is; if that fails, > there is no guarantee that the program could recover later, so it's > best to abort in that case. This could be part of the initial > modeset, too. > > - When the program has lost and regained DRM master status, meaning > that (unrecognised) KMS state is potentially incorrect, prepare an > atomic commit with "reset unknowns" set and add all the recognised > state the program knows of on top. This resets everything to like it > was, with ALLOW_MODESET. > > - At
Re: Operating KMS UAPI (Re: RFC: Drm-connector properties managed by another driver / privacy screen support)
On Monday, April 20, 2020 2:22 PM, Pekka Paalanen wrote: > On Mon, 20 Apr 2020 10:15:39 + > Simon Ser cont...@emersion.fr wrote: > > > On Monday, April 20, 2020 10:27 AM, Pekka Paalanen ppaala...@gmail.com > > wrote: > > > > > 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? > > I meant fbcon, not fbdev above. > > > Note, this is not the case when using e.g. a display manager. In the > > past there have been cases of a display manager setting a hw cursor > > and launching a compositor not supporting hw cursors. This results in > > a stuck hw cursor. > > Indeed. So the display manager might get sensible defaults, but the > session compositor might not. Or maybe boot splash uses KMS already, so > even display manager doesn't get all-defaults state. > > It seems we really do need "sane defaults" from the kernel explicitly. > Writing a userspace tool to save it at boot time before any KMS program > runs would be awkward. Agreed. > > > 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. > > > > User-space needs to reset the value to GOOD when recovering from a BAD > > value. > > What if userspace writes BAD? > > BAD cannot be default state, so getting default state from somewhere > would solve this property's restoring as well. Reading back the true > current value could accidentally return BAD. Interestingly, setting it to BAD is a no-op (BAD is silently discarded): /* Never downgrade from GOOD to BAD on userspace's request here, * only hw issues can do that. * * For an atomic property the userspace doesn't need to be able * to understand all the properties, but needs to be able to * restore the state it wants on VT switch. So if the userspace * tries to change the link_status from GOOD to BAD, driver * silently rejects it and returns a 0. This prevents userspace * from accidently breaking the display when it restores the * state. */ if (state->link_status != DRM_LINK_STATUS_GOOD) state->link_status = val; So restoring the "sane default" would be work, even if the link happens to be BAD when saving said "sane defaults". > Just to reiterate for everyone, the important thing here is to figure > out how userspace is supposed to reset unknown properties to sensible > defaults. Once we know how that should work, we can review whether new > properties support or break that. Yes, that's a good description of the problem. I see two main solutions here: either the kernel provides the default values in its property descriptions (e.g. drmModeGetProperty), either user-space can ask the kernel to reset properties to their default values. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Operating KMS UAPI (Re: RFC: Drm-connector properties managed by another driver / privacy screen support)
On Mon, 20 Apr 2020 10:15:39 + Simon Ser wrote: > On Monday, April 20, 2020 10:27 AM, Pekka Paalanen > wrote: > > > 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? I meant fbcon, not fbdev above. > Note, this is not the case when using e.g. a display manager. In the > past there have been cases of a display manager setting a hw cursor > and launching a compositor not supporting hw cursors. This results in > a stuck hw cursor. Indeed. So the display manager might get sensible defaults, but the session compositor might not. Or maybe boot splash uses KMS already, so even display manager doesn't get all-defaults state. It seems we really do need "sane defaults" from the kernel explicitly. Writing a userspace tool to save it at boot time before any KMS program runs would be awkward. > > 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. > > User-space needs to reset the value to GOOD when recovering from a BAD > value. What if userspace writes BAD? BAD cannot be default state, so getting default state from somewhere would solve this property's restoring as well. Reading back the true current value could accidentally return BAD. Just to reiterate for everyone, the important thing here is to figure out how userspace is supposed to reset unknown properties to sensible defaults. Once we know how that should work, we can review whether new properties support or break that. Thanks, pq pgpPkqO5kpdHt.pgp Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Operating KMS UAPI (Re: RFC: Drm-connector properties managed by another driver / privacy screen support)
I'd really like a drmModeAtomicAddDefaultProperty that resets a prop to its default value… ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Operating KMS UAPI (Re: RFC: Drm-connector properties managed by another driver / privacy screen support)
On Monday, April 20, 2020 10:27 AM, Pekka Paalanen wrote: > 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? Note, this is not the case when using e.g. a display manager. In the past there have been cases of a display manager setting a hw cursor and launching a compositor not supporting hw cursors. This results in a stuck hw cursor. > 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. User-space needs to reset the value to GOOD when recovering from a BAD value. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Operating KMS UAPI (Re: RFC: Drm-connector properties managed by another driver / privacy screen support)
On Mon, 20 Apr 2020 11:27:04 +0300 Pekka Paalanen wrote: > On Fri, 17 Apr 2020 16:17:18 +0200 > Daniel Vetter wrote: > > > On Fri, Apr 17, 2020 at 11:02 AM Pekka Paalanen > > 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 ? Thinking more, would the below work? Actor: a KMS userspace program, e.g. a display server - On start-up, read all KMS properties and their values. The properties that are not recognised are saved in a set called "reset unknowns" with their current values. Optional: The program commits the "reset unknown" state to KMS with ALLOW_MODESET to ensure it all is writable as is; if that fails, there is no guarantee that the program could recover later, so it's best to abort in that case. This could be part of the initial modeset, too. - When the program has lost and regained DRM master status, meaning that (unrecognised) KMS state is potentially incorrect, prepare an atomic commit with "reset unknowns" set and add all the recognised state the program knows of on top. This resets everything to like it was, with ALLOW_MODESET. - At any other time, do not use the "reset unknowns" set. The final point is the important one. I have assumed it would be safe to use always, but apparently not? Good thing I haven't yet written code to do that. You have to recognise the property to know if it is safe to set needlessly (for convenience in both code simplicity and ease of debugging)? Also, when using "reset unknowns" set, it actually has to be partitioned by KMS objects (CRTC, connector, plane...) so if e.g. a connector no longer exist, you don't attempt to set it. However, this still leaves writable properties whose value read is not legal to write as broken. Let's pray that fbcon or a system compositor will never succeed in enabling HDCP... Thanks, pq pgppwaGEs71uE.pgp Description: OpenPGP digital signature ___ dri-deve
Operating KMS UAPI (Re: RFC: Drm-connector properties managed by another driver / privacy screen support)
On Fri, 17 Apr 2020 16:17:18 +0200 Daniel Vetter wrote: > On Fri, Apr 17, 2020 at 11:02 AM Pekka Paalanen 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
