[External] Re: RFC: Drm-connector properties managed by another driver / privacy screen support

[Mark Pearson]

Hi,

> -----Original Message-----
> From: Hans de Goede <hdegoede at redhat.com>
> Sent: Wednesday, April 15, 2020 5:21 PM
> 
> Hi,
> 
> On 4/15/20 11:10 PM, Jani Nikula wrote:
> > On Wed, 15 Apr 2020, Rajat Jain <rajatja at google.com> wrote:
> >> Hello,
> >>
> >> On Wed, Apr 15, 2020 at 8:40 AM Hans de Goede
> <hdegoede at redhat.com> wrote:
> >>>
> >>> Hi,
> >>>
> >>> On 4/15/20 5:28 PM, Jani Nikula wrote:
> >>>> On Wed, 15 Apr 2020, Hans de Goede <hdegoede at redhat.com>
> wrote:
> >>>>> ii. Currently the "privacy-screen" property added by Rajat's
> >>>>> patch-set is an enum with 2 possible values:
> >>>>> "Enabled"
> >>>>> "Disabled"
> >>>>>
> >>>>> We could add a third value "Not Available", which would be the
> >>>>> default and then for internal panels always add the property
> >>>>> so that we avoid the problem that detecting if the laptop has
> >>>>> an internal privacy screen needs to be done before the connector
> >>>>> is registered. Then we can add some hooks which allow an
> >>>>> lcdshadow-driver to register itself against a connector later
> >>>>> (which is non trivial wrt probe order, but lets ignore that for now).
> >>>>
> >>>> I regret dropping the ball on Rajat's series (sorry!).
> >>>>
> >>>> I do think having the connector property for this is the way to go.
> >>>
> >>> I 100% agree.
> >>>
> >>>> Even
> >>>> if we couldn't necessarily figure out all the details on the kernel
> >>>> internal connections, can we settle on the property though, so we could
> >>>> move forward with Rajat's series?
> >>
> >> Thanks, it would be great!.
> >>
> >>>
> >>> Yes please, this will also allow us to move forward with userspace
> >>> support even if for testing that we do some hacks for the kernel's
> >>> internal connections for now.
> >>>
> >>>> Moreover, do we actually need two properties, one which could
> indicate
> >>>> userspace's desire for the property, and another that tells the hardware
> >>>> state?
> >>>
> >>> No I do not think so. I would expect there to just be one property,
> >>> I guess that if the state is (partly) firmware controlled then there
> >>> might be a race, but we will need a notification mechanism (*) for
> >>> firmware triggered state changes anyways, so shortly after loosing
> >>> the race userspace will process the notification and it will know
> >>> about it.
> >>
> >> I agree with Hans here that I think it would be better if we could do
> >> it with one property.
> >>
> >>   * I can imagine demand for laptops that have a "hardware kill switch"
> >> for privacy screen (just like there are for camera etc today). So I
> >> think in future we may have to deal with this case anyway. In such
> >> devices it's the hardware (as opposite to firmware) that will change
> >> the state. The HW will likely provide an interrupt to the software to
> >> notify of the change. This is all imaginative at this point though.
> >>
> >> * I think having 2 properties might be a confusing UAPI. Also, we have
> >> existing properties like link-status that can be changed by both the
> >> user and the hardware.
> >
> > I think the consensus is that all properties that get changed by both
> > userspace and the kernel are mistakes, and the way to handle it is to
> > have two properties.
> 
> But the actual privacy screen has only 1 state, having two properties
> for this will only be confusing. As I mentioned before we have a similar
> case with e.g. keyboard backlighting and there the userspace API
> also has a single sysfs attribute for the brightness, with change
> notifications to userspace if the firmware changes the brightness
> on its own and this works well.
> 
> What would the semantics of these 2 different properties be? And
> what sort of extra functionality would these semantics offer which
> the single property versions does not offer?
> 
I'm worried I'm missing the point but logically this doesn't make sense to me 
either. We're just providing two mechanisms for controlling ePrivacy - neither 
is wrong. I'm not sure where the consensus was reached and why - I'm afraid I 
don't have that history. 
Not sure it's the best example but you can control the system volume using 
buttons or the SW slider. It's still just the volume. Isn't this similar (ish). Both 
have their place and benefits.

What if users want the hotkey functionality? It is after all quite helpful: 
doing Fn+d is a lot quicker than finding the control box and toggling something 
if someone is looking over your shoulder and you don't want them to see. I'm 
not sure why a user would want it disabled - as long as it is tied in properly with 
the user space reporting.

<Lenovo hat on>
On that note - (I started replying in an earlier thread but got side tracked) there is 
little chance the BIOS team will make significant changes to existing BIOS
for adding extra disable functionality. There are just too many repercussions 
on testing as it impacts Windows. I know it sucks but it's just the way it is and I 
don't think there is a strong enough story to really push for it. Let me know if I'm 
out to lunch or being unreasonable 😊I'm happy to have that conversation with them
but I already know the results...maybe for future platforms but there are issues there
too (when shipping Linux systems it's just another setting to tweak and we're trying to
reduce them not increase them...but that's another story)

Mark