On Tue, Apr 21, 2020 at 7:46 AM Pekka Paalanen <ppaalanen@xxxxxxxxx> wrote: > > On Tue, 21 Apr 2020 14:37:41 +0200 > Hans de Goede <hdegoede@xxxxxxxxxx> wrote: > > > TL;DR: Yes there will be races, because of both userspace + > > the firmware having; and potentially using r/w access to > > the privacy-screen state. But in practice I expect these > > to not really be an issue. Important here is that userspace > > only commits the property in a transaction to commit if > > it actually intends to change the property so as to not > > needlessly create a situation where we might hit the race. > > > > As for 1 vs 2 properties for this I guess that in preparation > > for potential devices where the state is locked, having a > > r/w sw-state + a ro hw-state property makes sense. > > > > So I suggest that we replace the current "privacy-screen" property > > from Rajat's patch-set with 2 props named: > > > > "privacy-screen-sw-state" (r/w) > > "privacy-screen-hw-state" (ro) > > > > Where for current gen hardware the privacy-screen-hw-state is > > just a mirror of the sw-state. Just to make sure I understand the semantics correctly: - The "privacy-screen-hw-state" shall be read-only, and can be modified by: - Hardware (e.g. HW kill switch). - Firmware. - (Potentially) needs a notification/irq to the kernel when this changes (or may be kernel can read it only when userspace queries for it). - The "privacy-screen-sw-state" shall be read-write, and can only be modified by user space. - If user space toggles it, the kernel will attempt to "request" the change to hardware. - Whether the request to hardware was successful or not, the "privacy-screen-sw-state" will always reflect the latest value userspace wrote. - If the request to hardware was successful, the "privacy-screen-hw-state" will also change (probably via a separate notification/irq from HW). - We expect the user space to write to "privacy-screen-sw-state" only if it really wants to toggle the value. What is not clear to me is if any change to"privacy-screen-hw-state" shall be propagated to "privacy-screen-sw-state"? - If yes, then I think we are not solving any problems of single property. - If no, then why do we require userspace to write to sw state only if something has changed? Also, it seems to me that in my current patchset, the property I have already behaves like "privacy-screen-sw-state". Do I just need to rename it? Thanks, Rajat > > Hi, > > this sounds like a good plan to me, assuming the kernel writes only to > the ro property and never to the r/w property. > > I understand that as long as firmware hotkeys will toggle actual state, > there is no design that could work reliably if userspace will always > commit all KMS state even when it is not necessary. But not committing > KMS state unless it is actually necessary is really For implementing the "privacy-screen-sw-state".a new requirement on > userspace, so that needs to be documented before it's too late. > > It's not enough to document "don't set it unless you want to > overwrite/change it" for privacy screen properties. It needs to be > documented as a general rule that userspace must follow with *unknown* > properties as well. "Do not restore unrecognized properties unless the > kernel KMS state might be incorrect compared to what you used to have." > > This means that with a display server that does not understand privacy > screen properties, the end user will lose the privacy screen state on > every VT-switch back to the display server. > > However, if we had a way to query the kernel for the default state to > reset unknown properties to, the kernel implementation could return the > current value of the privacy screen property instead of "off" to not > lose the firmware state. Assuming firmware hotkeys exist, but if they > don't then return just "off". The point is that the kernel who knows > all the properties makes the decision what a sane reset value is. > Userspace can always override the reset value for the properties > it recognizes. > > > Thanks,For implementing the "privacy-screen-sw-state". > pq _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel