Re: [RFC v2] drm/connector: Add support for privacy-screen properties (v2)

[Pekka Paalanen <ppaalanen@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 4463 bytes --]

On Tue, 12 May 2020 10:02:12 +0200
Hans de Goede <hdegoede@redhat.com> wrote:

> Hi,
> 
> On 5/12/20 9:49 AM, Pekka Paalanen wrote:
> > On Mon, 11 May 2020 19:47:24 +0200
> > Hans de Goede <hdegoede@redhat.com> wrote:
> >   
> >> From: Rajat Jain <rajatja@google.com>
> >>
> >> Add support for generic electronic privacy screen properties, that
> >> can be added by systems that have an integrated EPS.
> >>
> >> Changes in v2 (Hans de Goede)
> >> - Create 2 properties, "privacy-screen sw-state" and
> >>    "privacy-screen hw-state", to deal with devices where the OS might be
> >>    locked out of making state changes
> >> - Write kerneldoc explaining how the 2 properties work together, what
> >>    happens when changes to the state are made outside of the DRM code's
> >>    control, etc.
> >>
> >> Signed-off-by: Rajat Jain <rajatja@google.com>
> >> Co-authored-by: Hans de Goede <hdegoede@redhat.com>
> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> >> ---
> >>   Documentation/gpu/drm-kms.rst     |   2 +
> >>   drivers/gpu/drm/drm_atomic_uapi.c |   6 ++
> >>   drivers/gpu/drm/drm_connector.c   | 100 ++++++++++++++++++++++++++++++
> >>   include/drm/drm_connector.h       |  50 +++++++++++++++
> >>   4 files changed, 158 insertions(+)  
> > 
> > ...
> >   
> >> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> >> index 644f0ad10671..01360edc2376 100644
> >> --- a/drivers/gpu/drm/drm_connector.c
> >> +++ b/drivers/gpu/drm/drm_connector.c
> >> @@ -1186,6 +1186,45 @@ static const struct drm_prop_enum_list dp_colorspaces[] = {
> >>    *	can also expose this property to external outputs, in which case they
> >>    *	must support "None", which should be the default (since external screens
> >>    *	have a built-in scaler).
> >> + *
> >> + * privacy-screen sw-state, privacy-screen hw-state:
> >> + *	These 2 optional properties can be used to query the state of the
> >> + *	electronic privacy screen that is available on some displays; and in
> >> + *	some cases also control the state. If a driver implements these
> >> + *	properties then both properties must be present.
> >> + *
> >> + *	"privacy-screen hw-state" is read-only and reflects the actual state
> >> + *	of the privacy-screen, possible values: "Enabled", "Disabled,
> >> + *	"Enabled, locked", "Disabled, locked". The locked states indicate
> >> + *	that the state cannot be changed through the DRM API. E.g. there
> >> + *	might be devices where the firmware-setup options, or a hardware
> >> + *	slider-switch, offer always on / off modes.
> >> + *
> >> + *	"privacy-screen sw-state" can be set to change the privacy-screen state
> >> + *	when not locked. In this case the driver must update the hw-state
> >> + *	property to reflect the new state on completion of the commit of the
> >> + *	sw-state property. Setting the sw-state property when the hw-state is
> >> + *	locked must be interpreted by the driver as a request to change the
> >> + *	state to the set state when the hw-state becomes unlocked. E.g. if
> >> + *	"privacy-screen hw-state" is "Enabled, locked" and the sw-state
> >> + *	gets set to "Disabled" followed by the user unlocking the state by
> >> + *	changing the slider-switch position, then the driver must set the
> >> + *	state to "Disabled" upon receiving the unlock event.
> >> + *
> >> + *	In some cases the privacy-screen state might change outside of control
> >> + *	of the DRM code. E.g. there might be a firmware handled hotkey which
> >> + *	toggles the state, or the state might be changed through another  
> > 
> > Hi,
> > 
> > in the above three lines, I'd use the term "hardware state" instead of
> > just "state" to be explicit. Or should it be "physical state" since
> > "hardware state" might be confused with "hw-state" property state?  
> 
> Maybe "actual state"? That is what is used a few lines higher:
> 
> '"privacy-screen hw-state" is read-only and reflects the actual state'
> 
> And you use it yourself to describe what we want below:
> 
> > I don't mind as long as it's unambiguous and distinguishes explicitly
> > between actual and the two property states.  
> 
> So since you and I both naturally described it as "actual state" without
> thinking too much what we where writing at the time (I guess that applies
> to your use too), "actual state" seems a good fit ?

Sure!


Thanks,
pq

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel