Re: [Intel-gfx] [PATCH] drm: Add support for integrated privacy screens

2019-10-28 Thread Rajat Jain
On Fri, Oct 25, 2019 at 4:36 AM Thierry Reding  wrote:
>
> On Thu, Oct 24, 2019 at 01:45:16PM -0700, Rajat Jain wrote:
> > Hi,
> >
> > Thanks for your review and comments. Please see inline below.
> >
> > On Thu, Oct 24, 2019 at 4:20 AM Thierry Reding  
> > wrote:
> > >
> > > On Tue, Oct 22, 2019 at 05:12:06PM -0700, Rajat Jain wrote:
> > > > Certain laptops now come with panels that have integrated privacy
> > > > screens on them. This patch adds support for such panels by adding
> > > > a privacy-screen property to the drm_connector for the panel, that
> > > > the userspace can then use to control and check the status. The idea
> > > > was discussed here:
> > > >
> > > > https://lkml.org/lkml/2019/10/1/786
> > > >
> > > > ACPI methods are used to identify, query and control privacy screen:
> > > >
> > > > * Identifying an ACPI object corresponding to the panel: The patch
> > > > follows ACPI Spec 6.3 (available at
> > > > https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf).
> > > > Pages 1119 - 1123 describe what I believe, is a standard way of
> > > > identifying / addressing "display panels" in the ACPI tables, thus
> > > > allowing kernel to attach ACPI nodes to the panel. IMHO, this ability
> > > > to identify and attach ACPI nodes to drm connectors may be useful for
> > > > reasons other privacy-screens, in future.
> > > >
> > > > * Identifying the presence of privacy screen, and controlling it, is 
> > > > done
> > > > via ACPI _DSM methods.
> > > >
> > > > Currently, this is done only for the Intel display ports. But in future,
> > > > this can be done for any other ports if the hardware becomes available
> > > > (e.g. external monitors supporting integrated privacy screens?).
> > > >
> > > > Also, this code can be extended in future to support non-ACPI methods
> > > > (e.g. using a kernel GPIO driver to toggle a gpio that controls the
> > > > privacy-screen).
> > > >
> > > > Signed-off-by: Rajat Jain 
> > > > ---
> > > >  drivers/gpu/drm/Makefile|   1 +
> > > >  drivers/gpu/drm/drm_atomic_uapi.c   |   5 +
> > > >  drivers/gpu/drm/drm_connector.c |  38 +
> > > >  drivers/gpu/drm/drm_privacy_screen.c| 176 
> > > >  drivers/gpu/drm/i915/display/intel_dp.c |   3 +
> > > >  include/drm/drm_connector.h |  18 +++
> > > >  include/drm/drm_mode_config.h   |   7 +
> > > >  include/drm/drm_privacy_screen.h|  33 +
> > > >  8 files changed, 281 insertions(+)
> > > >  create mode 100644 drivers/gpu/drm/drm_privacy_screen.c
> > > >  create mode 100644 include/drm/drm_privacy_screen.h
> > >
> > > I like this much better than the prior proposal to use sysfs. However
> > > the support currently looks a bit tangled. I realize that we only have a
> > > single implementation for this in hardware right now, so there's no use
> > > in over-engineering things, but I think we can do a better job from the
> > > start without getting into too many abstractions. See below.
> > >
> > > > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> > > > index 82ff826b33cc..e1fc33d69bb7 100644
> > > > --- a/drivers/gpu/drm/Makefile
> > > > +++ b/drivers/gpu/drm/Makefile
> > > > @@ -19,6 +19,7 @@ drm-y   :=  drm_auth.o drm_cache.o \
> > > >   drm_syncobj.o drm_lease.o drm_writeback.o drm_client.o \
> > > >   drm_client_modeset.o drm_atomic_uapi.o drm_hdcp.o
> > > >
> > > > +drm-$(CONFIG_ACPI) += drm_privacy_screen.o
> > > >  drm-$(CONFIG_DRM_LEGACY) += drm_legacy_misc.o drm_bufs.o drm_context.o 
> > > > drm_dma.o drm_scatter.o drm_lock.o
> > > >  drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o
> > > >  drm-$(CONFIG_DRM_VM) += drm_vm.o
> > > > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
> > > > b/drivers/gpu/drm/drm_atomic_uapi.c
> > > > index 7a26bfb5329c..44131165e4ea 100644
> > > > --- a/drivers/gpu/drm/drm_atomic_uapi.c
> > > > +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> > > > @@ -30,6 +30,7 @@
> > > >  #include 
> > > >  #include 
> > > >  #include 
> > > > +#include 
> > > >  #include 
> > > >  #include 
> > > >
> > > > @@ -766,6 +767,8 @@ static int drm_atomic_connector_set_property(struct 
> > > > drm_connector *connector,
> > > >  fence_ptr);
> > > >   } else if (property == connector->max_bpc_property) {
> > > >   state->max_requested_bpc = val;
> > > > + } else if (property == config->privacy_screen_property) {
> > > > + drm_privacy_screen_set_val(connector, val);
> > >
> > > This doesn't look right. Shouldn't you store the value in the connector
> > > state and then leave it up to the connector driver to set it
> > > appropriately? I think that also has the advantage of untangling this
> > > support a little.
> >
> > Hopefully this gets answered in my explanations below.
> >
> > >
> > > >   } else if (connector->funcs->atomic_set_property) {
> > > >   return connector->f

Re: [Intel-gfx] [PATCH] drm: Add support for integrated privacy screens

2019-10-28 Thread Pekka Paalanen
On Fri, 25 Oct 2019 21:03:12 +0200
Daniel Vetter  wrote:

> On Fri, Oct 25, 2019 at 1:36 PM Thierry Reding  
> wrote:
> >
> > On Thu, Oct 24, 2019 at 01:45:16PM -0700, Rajat Jain wrote:  
> > > Hi,
> > >
> > > Thanks for your review and comments. Please see inline below.
> > >
> > > On Thu, Oct 24, 2019 at 4:20 AM Thierry Reding  
> > > wrote:  
> > > >
> > > > On Tue, Oct 22, 2019 at 05:12:06PM -0700, Rajat Jain wrote:  
> > > > > Certain laptops now come with panels that have integrated privacy
> > > > > screens on them. This patch adds support for such panels by adding
> > > > > a privacy-screen property to the drm_connector for the panel, that
> > > > > the userspace can then use to control and check the status. The idea
> > > > > was discussed here:
> > > > >
> > > > > https://lkml.org/lkml/2019/10/1/786
> > > > >
> > > > > ACPI methods are used to identify, query and control privacy screen:
> > > > >
> > > > > * Identifying an ACPI object corresponding to the panel: The patch
> > > > > follows ACPI Spec 6.3 (available at
> > > > > https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf).
> > > > > Pages 1119 - 1123 describe what I believe, is a standard way of
> > > > > identifying / addressing "display panels" in the ACPI tables, thus
> > > > > allowing kernel to attach ACPI nodes to the panel. IMHO, this ability
> > > > > to identify and attach ACPI nodes to drm connectors may be useful for
> > > > > reasons other privacy-screens, in future.
> > > > >
> > > > > * Identifying the presence of privacy screen, and controlling it, is 
> > > > > done
> > > > > via ACPI _DSM methods.
> > > > >
> > > > > Currently, this is done only for the Intel display ports. But in 
> > > > > future,
> > > > > this can be done for any other ports if the hardware becomes available
> > > > > (e.g. external monitors supporting integrated privacy screens?).
> > > > >
> > > > > Also, this code can be extended in future to support non-ACPI methods
> > > > > (e.g. using a kernel GPIO driver to toggle a gpio that controls the
> > > > > privacy-screen).
> > > > >
> > > > > Signed-off-by: Rajat Jain 
> > > > > ---
> > > > >  drivers/gpu/drm/Makefile|   1 +
> > > > >  drivers/gpu/drm/drm_atomic_uapi.c   |   5 +
> > > > >  drivers/gpu/drm/drm_connector.c |  38 +
> > > > >  drivers/gpu/drm/drm_privacy_screen.c| 176 
> > > > > 
> > > > >  drivers/gpu/drm/i915/display/intel_dp.c |   3 +
> > > > >  include/drm/drm_connector.h |  18 +++
> > > > >  include/drm/drm_mode_config.h   |   7 +
> > > > >  include/drm/drm_privacy_screen.h|  33 +
> > > > >  8 files changed, 281 insertions(+)
> > > > >  create mode 100644 drivers/gpu/drm/drm_privacy_screen.c
> > > > >  create mode 100644 include/drm/drm_privacy_screen.h  
> > > >
> > > > I like this much better than the prior proposal to use sysfs. However
> > > > the support currently looks a bit tangled. I realize that we only have a
> > > > single implementation for this in hardware right now, so there's no use
> > > > in over-engineering things, but I think we can do a better job from the
> > > > start without getting into too many abstractions. See below.
> > > >  
> > > > > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> > > > > index 82ff826b33cc..e1fc33d69bb7 100644
> > > > > --- a/drivers/gpu/drm/Makefile
> > > > > +++ b/drivers/gpu/drm/Makefile
> > > > > @@ -19,6 +19,7 @@ drm-y   :=  drm_auth.o drm_cache.o \
> > > > >   drm_syncobj.o drm_lease.o drm_writeback.o drm_client.o \
> > > > >   drm_client_modeset.o drm_atomic_uapi.o drm_hdcp.o
> > > > >
> > > > > +drm-$(CONFIG_ACPI) += drm_privacy_screen.o
> > > > >  drm-$(CONFIG_DRM_LEGACY) += drm_legacy_misc.o drm_bufs.o 
> > > > > drm_context.o drm_dma.o drm_scatter.o drm_lock.o
> > > > >  drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o
> > > > >  drm-$(CONFIG_DRM_VM) += drm_vm.o
> > > > > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
> > > > > b/drivers/gpu/drm/drm_atomic_uapi.c
> > > > > index 7a26bfb5329c..44131165e4ea 100644
> > > > > --- a/drivers/gpu/drm/drm_atomic_uapi.c
> > > > > +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> > > > > @@ -30,6 +30,7 @@
> > > > >  #include 
> > > > >  #include 
> > > > >  #include 
> > > > > +#include 
> > > > >  #include 
> > > > >  #include 
> > > > >
> > > > > @@ -766,6 +767,8 @@ static int 
> > > > > drm_atomic_connector_set_property(struct drm_connector *connector,
> > > > >  fence_ptr);
> > > > >   } else if (property == connector->max_bpc_property) {
> > > > >   state->max_requested_bpc = val;
> > > > > + } else if (property == config->privacy_screen_property) {
> > > > > + drm_privacy_screen_set_val(connector, val);  
> > > >
> > > > This doesn't look right. Shouldn't you store the value in the connector
> > > > state and then leave it up to the connector driver

Re: [Intel-gfx] [PATCH] drm: Add support for integrated privacy screens

2019-10-26 Thread Daniel Vetter
On Sat, Oct 26, 2019 at 1:07 PM Daniel Stone  wrote:
>
> Hi Thierry,
>
> On Fri, 25 Oct 2019 at 12:36, Thierry Reding  wrote:
> > On Thu, Oct 24, 2019 at 01:45:16PM -0700, Rajat Jain wrote:
> > > I did think about having a state variable in software to get and set
> > > this. However, I think it is not very far fetched that some platforms
> > > may have "hardware kill" switches that allow hardware to switch
> > > privacy-screen on and off directly, in addition to the software
> > > control that we are implementing. Privacy is a touchy subject in
> > > enterprise, and anything that reduces the possibility of having any
> > > inconsistency between software state and hardware state is desirable.
> > > So in this case, I chose to not have a state in software about this -
> > > we just report the hardware state everytime we are asked for it.
> >
> > So this doesn't really work with atomic KMS, then. The main idea behind
> > atomic KMS is that you apply a configuration either completely or not at
> > all. So at least for setting this property you'd have to go through the
> > state object.
> >
> > Now, for reading out the property you might be able to get away with the
> > above. I'm not sure if that's enough to keep the state up-to-date,
> > though. Is there some way for a kill switch to trigger an interrupt or
> > other event of some sort so that the state could be kept up-to-date?
> >
> > Daniel (or anyone else), do you know of any precedent for state that
> > might get modified behind the atomic helpers' back? Seems to me like we
> > need to find some point where we can actually read back the current
> > "hardware value" of this privacy screen property and store that back
> > into the state.
>
> Well, apart from connector state, though that isn't really a property
> as such, there's the link_state property, which is explicitly designed
> to do just that. That has been quite carefully designed for the
> back-and-forth though.

connector state is an immutable property, which is a hilarious way to
say that "only the driver can update it, userspace only reads it". So
not a good template here. But yeah link_state is a good example of
what we need here.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm: Add support for integrated privacy screens

2019-10-26 Thread Daniel Stone
Hi Thierry,

On Fri, 25 Oct 2019 at 12:36, Thierry Reding  wrote:
> On Thu, Oct 24, 2019 at 01:45:16PM -0700, Rajat Jain wrote:
> > I did think about having a state variable in software to get and set
> > this. However, I think it is not very far fetched that some platforms
> > may have "hardware kill" switches that allow hardware to switch
> > privacy-screen on and off directly, in addition to the software
> > control that we are implementing. Privacy is a touchy subject in
> > enterprise, and anything that reduces the possibility of having any
> > inconsistency between software state and hardware state is desirable.
> > So in this case, I chose to not have a state in software about this -
> > we just report the hardware state everytime we are asked for it.
>
> So this doesn't really work with atomic KMS, then. The main idea behind
> atomic KMS is that you apply a configuration either completely or not at
> all. So at least for setting this property you'd have to go through the
> state object.
>
> Now, for reading out the property you might be able to get away with the
> above. I'm not sure if that's enough to keep the state up-to-date,
> though. Is there some way for a kill switch to trigger an interrupt or
> other event of some sort so that the state could be kept up-to-date?
>
> Daniel (or anyone else), do you know of any precedent for state that
> might get modified behind the atomic helpers' back? Seems to me like we
> need to find some point where we can actually read back the current
> "hardware value" of this privacy screen property and store that back
> into the state.

Well, apart from connector state, though that isn't really a property
as such, there's the link_state property, which is explicitly designed
to do just that. That has been quite carefully designed for the
back-and-forth though.

Cheers,
Daniel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm: Add support for integrated privacy screens

2019-10-25 Thread Daniel Vetter
On Fri, Oct 25, 2019 at 1:36 PM Thierry Reding  wrote:
>
> On Thu, Oct 24, 2019 at 01:45:16PM -0700, Rajat Jain wrote:
> > Hi,
> >
> > Thanks for your review and comments. Please see inline below.
> >
> > On Thu, Oct 24, 2019 at 4:20 AM Thierry Reding  
> > wrote:
> > >
> > > On Tue, Oct 22, 2019 at 05:12:06PM -0700, Rajat Jain wrote:
> > > > Certain laptops now come with panels that have integrated privacy
> > > > screens on them. This patch adds support for such panels by adding
> > > > a privacy-screen property to the drm_connector for the panel, that
> > > > the userspace can then use to control and check the status. The idea
> > > > was discussed here:
> > > >
> > > > https://lkml.org/lkml/2019/10/1/786
> > > >
> > > > ACPI methods are used to identify, query and control privacy screen:
> > > >
> > > > * Identifying an ACPI object corresponding to the panel: The patch
> > > > follows ACPI Spec 6.3 (available at
> > > > https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf).
> > > > Pages 1119 - 1123 describe what I believe, is a standard way of
> > > > identifying / addressing "display panels" in the ACPI tables, thus
> > > > allowing kernel to attach ACPI nodes to the panel. IMHO, this ability
> > > > to identify and attach ACPI nodes to drm connectors may be useful for
> > > > reasons other privacy-screens, in future.
> > > >
> > > > * Identifying the presence of privacy screen, and controlling it, is 
> > > > done
> > > > via ACPI _DSM methods.
> > > >
> > > > Currently, this is done only for the Intel display ports. But in future,
> > > > this can be done for any other ports if the hardware becomes available
> > > > (e.g. external monitors supporting integrated privacy screens?).
> > > >
> > > > Also, this code can be extended in future to support non-ACPI methods
> > > > (e.g. using a kernel GPIO driver to toggle a gpio that controls the
> > > > privacy-screen).
> > > >
> > > > Signed-off-by: Rajat Jain 
> > > > ---
> > > >  drivers/gpu/drm/Makefile|   1 +
> > > >  drivers/gpu/drm/drm_atomic_uapi.c   |   5 +
> > > >  drivers/gpu/drm/drm_connector.c |  38 +
> > > >  drivers/gpu/drm/drm_privacy_screen.c| 176 
> > > >  drivers/gpu/drm/i915/display/intel_dp.c |   3 +
> > > >  include/drm/drm_connector.h |  18 +++
> > > >  include/drm/drm_mode_config.h   |   7 +
> > > >  include/drm/drm_privacy_screen.h|  33 +
> > > >  8 files changed, 281 insertions(+)
> > > >  create mode 100644 drivers/gpu/drm/drm_privacy_screen.c
> > > >  create mode 100644 include/drm/drm_privacy_screen.h
> > >
> > > I like this much better than the prior proposal to use sysfs. However
> > > the support currently looks a bit tangled. I realize that we only have a
> > > single implementation for this in hardware right now, so there's no use
> > > in over-engineering things, but I think we can do a better job from the
> > > start without getting into too many abstractions. See below.
> > >
> > > > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> > > > index 82ff826b33cc..e1fc33d69bb7 100644
> > > > --- a/drivers/gpu/drm/Makefile
> > > > +++ b/drivers/gpu/drm/Makefile
> > > > @@ -19,6 +19,7 @@ drm-y   :=  drm_auth.o drm_cache.o \
> > > >   drm_syncobj.o drm_lease.o drm_writeback.o drm_client.o \
> > > >   drm_client_modeset.o drm_atomic_uapi.o drm_hdcp.o
> > > >
> > > > +drm-$(CONFIG_ACPI) += drm_privacy_screen.o
> > > >  drm-$(CONFIG_DRM_LEGACY) += drm_legacy_misc.o drm_bufs.o drm_context.o 
> > > > drm_dma.o drm_scatter.o drm_lock.o
> > > >  drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o
> > > >  drm-$(CONFIG_DRM_VM) += drm_vm.o
> > > > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
> > > > b/drivers/gpu/drm/drm_atomic_uapi.c
> > > > index 7a26bfb5329c..44131165e4ea 100644
> > > > --- a/drivers/gpu/drm/drm_atomic_uapi.c
> > > > +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> > > > @@ -30,6 +30,7 @@
> > > >  #include 
> > > >  #include 
> > > >  #include 
> > > > +#include 
> > > >  #include 
> > > >  #include 
> > > >
> > > > @@ -766,6 +767,8 @@ static int drm_atomic_connector_set_property(struct 
> > > > drm_connector *connector,
> > > >  fence_ptr);
> > > >   } else if (property == connector->max_bpc_property) {
> > > >   state->max_requested_bpc = val;
> > > > + } else if (property == config->privacy_screen_property) {
> > > > + drm_privacy_screen_set_val(connector, val);
> > >
> > > This doesn't look right. Shouldn't you store the value in the connector
> > > state and then leave it up to the connector driver to set it
> > > appropriately? I think that also has the advantage of untangling this
> > > support a little.
> >
> > Hopefully this gets answered in my explanations below.
> >
> > >
> > > >   } else if (connector->funcs->atomic_set_property) {
> > > >   return connector->f

Re: [Intel-gfx] [PATCH] drm: Add support for integrated privacy screens

2019-10-25 Thread Thierry Reding
On Thu, Oct 24, 2019 at 01:45:16PM -0700, Rajat Jain wrote:
> Hi,
> 
> Thanks for your review and comments. Please see inline below.
> 
> On Thu, Oct 24, 2019 at 4:20 AM Thierry Reding  
> wrote:
> >
> > On Tue, Oct 22, 2019 at 05:12:06PM -0700, Rajat Jain wrote:
> > > Certain laptops now come with panels that have integrated privacy
> > > screens on them. This patch adds support for such panels by adding
> > > a privacy-screen property to the drm_connector for the panel, that
> > > the userspace can then use to control and check the status. The idea
> > > was discussed here:
> > >
> > > https://lkml.org/lkml/2019/10/1/786
> > >
> > > ACPI methods are used to identify, query and control privacy screen:
> > >
> > > * Identifying an ACPI object corresponding to the panel: The patch
> > > follows ACPI Spec 6.3 (available at
> > > https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf).
> > > Pages 1119 - 1123 describe what I believe, is a standard way of
> > > identifying / addressing "display panels" in the ACPI tables, thus
> > > allowing kernel to attach ACPI nodes to the panel. IMHO, this ability
> > > to identify and attach ACPI nodes to drm connectors may be useful for
> > > reasons other privacy-screens, in future.
> > >
> > > * Identifying the presence of privacy screen, and controlling it, is done
> > > via ACPI _DSM methods.
> > >
> > > Currently, this is done only for the Intel display ports. But in future,
> > > this can be done for any other ports if the hardware becomes available
> > > (e.g. external monitors supporting integrated privacy screens?).
> > >
> > > Also, this code can be extended in future to support non-ACPI methods
> > > (e.g. using a kernel GPIO driver to toggle a gpio that controls the
> > > privacy-screen).
> > >
> > > Signed-off-by: Rajat Jain 
> > > ---
> > >  drivers/gpu/drm/Makefile|   1 +
> > >  drivers/gpu/drm/drm_atomic_uapi.c   |   5 +
> > >  drivers/gpu/drm/drm_connector.c |  38 +
> > >  drivers/gpu/drm/drm_privacy_screen.c| 176 
> > >  drivers/gpu/drm/i915/display/intel_dp.c |   3 +
> > >  include/drm/drm_connector.h |  18 +++
> > >  include/drm/drm_mode_config.h   |   7 +
> > >  include/drm/drm_privacy_screen.h|  33 +
> > >  8 files changed, 281 insertions(+)
> > >  create mode 100644 drivers/gpu/drm/drm_privacy_screen.c
> > >  create mode 100644 include/drm/drm_privacy_screen.h
> >
> > I like this much better than the prior proposal to use sysfs. However
> > the support currently looks a bit tangled. I realize that we only have a
> > single implementation for this in hardware right now, so there's no use
> > in over-engineering things, but I think we can do a better job from the
> > start without getting into too many abstractions. See below.
> >
> > > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> > > index 82ff826b33cc..e1fc33d69bb7 100644
> > > --- a/drivers/gpu/drm/Makefile
> > > +++ b/drivers/gpu/drm/Makefile
> > > @@ -19,6 +19,7 @@ drm-y   :=  drm_auth.o drm_cache.o \
> > >   drm_syncobj.o drm_lease.o drm_writeback.o drm_client.o \
> > >   drm_client_modeset.o drm_atomic_uapi.o drm_hdcp.o
> > >
> > > +drm-$(CONFIG_ACPI) += drm_privacy_screen.o
> > >  drm-$(CONFIG_DRM_LEGACY) += drm_legacy_misc.o drm_bufs.o drm_context.o 
> > > drm_dma.o drm_scatter.o drm_lock.o
> > >  drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o
> > >  drm-$(CONFIG_DRM_VM) += drm_vm.o
> > > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
> > > b/drivers/gpu/drm/drm_atomic_uapi.c
> > > index 7a26bfb5329c..44131165e4ea 100644
> > > --- a/drivers/gpu/drm/drm_atomic_uapi.c
> > > +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> > > @@ -30,6 +30,7 @@
> > >  #include 
> > >  #include 
> > >  #include 
> > > +#include 
> > >  #include 
> > >  #include 
> > >
> > > @@ -766,6 +767,8 @@ static int drm_atomic_connector_set_property(struct 
> > > drm_connector *connector,
> > >  fence_ptr);
> > >   } else if (property == connector->max_bpc_property) {
> > >   state->max_requested_bpc = val;
> > > + } else if (property == config->privacy_screen_property) {
> > > + drm_privacy_screen_set_val(connector, val);
> >
> > This doesn't look right. Shouldn't you store the value in the connector
> > state and then leave it up to the connector driver to set it
> > appropriately? I think that also has the advantage of untangling this
> > support a little.
> 
> Hopefully this gets answered in my explanations below.
> 
> >
> > >   } else if (connector->funcs->atomic_set_property) {
> > >   return connector->funcs->atomic_set_property(connector,
> > >   state, property, val);
> > > @@ -842,6 +845,8 @@ drm_atomic_connector_get_property(struct 
> > > drm_connector *connector,
> > >   *val = 0;
> > >   } else if (property == connect

Re: [Intel-gfx] [PATCH] drm: Add support for integrated privacy screens

2019-10-24 Thread kbuild test robot
Hi Rajat,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[cannot apply to v5.4-rc4 next-20191024]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:
https://github.com/0day-ci/linux/commits/Rajat-Jain/drm-Add-support-for-integrated-privacy-screens/20191025-020550
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
f116b96685a046a89c25d4a6ba2da489145c
reproduce: make htmldocs

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot 

All warnings (new ones prefixed by >>):

   include/linux/input/sparse-keymap.h:43: warning: Function parameter or 
member 'sw' not described in 'key_entry'
   include/linux/skbuff.h:888: warning: Function parameter or member 
'dev_scratch' not described in 'sk_buff'
   include/linux/skbuff.h:888: warning: Function parameter or member 'list' not 
described in 'sk_buff'
   include/linux/skbuff.h:888: warning: Function parameter or member 
'ip_defrag_offset' not described in 'sk_buff'
   include/linux/skbuff.h:888: warning: Function parameter or member 
'skb_mstamp_ns' not described in 'sk_buff'
   include/linux/skbuff.h:888: warning: Function parameter or member 
'__cloned_offset' not described in 'sk_buff'
   include/linux/skbuff.h:888: warning: Function parameter or member 
'head_frag' not described in 'sk_buff'
   include/linux/skbuff.h:888: warning: Function parameter or member 
'__pkt_type_offset' not described in 'sk_buff'
   include/linux/skbuff.h:888: warning: Function parameter or member 
'encapsulation' not described in 'sk_buff'
   include/linux/skbuff.h:888: warning: Function parameter or member 
'encap_hdr_csum' not described in 'sk_buff'
   include/linux/skbuff.h:888: warning: Function parameter or member 
'csum_valid' not described in 'sk_buff'
   include/linux/skbuff.h:888: warning: Function parameter or member 
'__pkt_vlan_present_offset' not described in 'sk_buff'
   include/linux/skbuff.h:888: warning: Function parameter or member 
'vlan_present' not described in 'sk_buff'
   include/linux/skbuff.h:888: warning: Function parameter or member 
'csum_complete_sw' not described in 'sk_buff'
   include/linux/skbuff.h:888: warning: Function parameter or member 
'csum_level' not described in 'sk_buff'
   include/linux/skbuff.h:888: warning: Function parameter or member 
'inner_protocol_type' not described in 'sk_buff'
   include/linux/skbuff.h:888: warning: Function parameter or member 
'remcsum_offload' not described in 'sk_buff'
   include/linux/skbuff.h:888: warning: Function parameter or member 
'sender_cpu' not described in 'sk_buff'
   include/linux/skbuff.h:888: warning: Function parameter or member 
'reserved_tailroom' not described in 'sk_buff'
   include/linux/skbuff.h:888: warning: Function parameter or member 
'inner_ipproto' not described in 'sk_buff'
   include/net/sock.h:233: warning: Function parameter or member 'skc_addrpair' 
not described in 'sock_common'
   include/net/sock.h:233: warning: Function parameter or member 'skc_portpair' 
not described in 'sock_common'
   include/net/sock.h:233: warning: Function parameter or member 'skc_ipv6only' 
not described in 'sock_common'
   include/net/sock.h:233: warning: Function parameter or member 
'skc_net_refcnt' not described in 'sock_common'
   include/net/sock.h:233: warning: Function parameter or member 'skc_v6_daddr' 
not described in 'sock_common'
   include/net/sock.h:233: warning: Function parameter or member 
'skc_v6_rcv_saddr' not described in 'sock_common'
   include/net/sock.h:233: warning: Function parameter or member 'skc_cookie' 
not described in 'sock_common'
   include/net/sock.h:233: warning: Function parameter or member 'skc_listener' 
not described in 'sock_common'
   include/net/sock.h:233: warning: Function parameter or member 'skc_tw_dr' 
not described in 'sock_common'
   include/net/sock.h:233: warning: Function parameter or member 'skc_rcv_wnd' 
not described in 'sock_common'
   include/net/sock.h:233: warning: Function parameter or member 
'skc_tw_rcv_nxt' not described in 'sock_common'
   include/net/sock.h:515: warning: Function parameter or member 
'sk_rx_skb_cache' not described in 'sock'
   include/net/sock.h:515: warning: Function parameter or member 'sk_wq_raw' 
not described in 'sock'
   include/net/sock.h:515: warning: Function parameter or member 
'tcp_rtx_queue' not described in 'sock'
   include/net/sock.h:515: warning: Function parameter or member 
'sk_tx_skb_cache' not described in 'sock'
   include/net/sock.h:515: warning: Function parameter or member 
'sk_route_forced_caps' not described in 'sock'
   include/net/sock.h:515: warning: Function parameter or member 
'sk_txtime_report_errors' not described in 'sock'
   include/net/sock.h:515: warning: Function parameter or me

Re: [Intel-gfx] [PATCH] drm: Add support for integrated privacy screens

2019-10-24 Thread kbuild test robot
Hi Rajat,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[cannot apply to v5.4-rc4 next-20191024]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:
https://github.com/0day-ci/linux/commits/Rajat-Jain/drm-Add-support-for-integrated-privacy-screens/20191025-020550
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
f116b96685a046a89c25d4a6ba2da489145c
config: i386-defconfig (attached as .config)
compiler: gcc-7 (Debian 7.4.0-14) 7.4.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot 

All warnings (new ones prefixed by >>):

   In file included from include/linux/printk.h:7:0,
from include/linux/kernel.h:15,
from include/linux/list.h:9,
from include/linux/kobject.h:19,
from include/linux/of.h:17,
from include/linux/irqdomain.h:35,
from include/linux/acpi.h:13,
from drivers/gpu//drm/drm_privacy_screen.c:8:
   drivers/gpu//drm/drm_privacy_screen.c: In function 
'drm_privacy_screen_present':
>> include/linux/kern_levels.h:5:18: warning: format '%s' expects argument of 
>> type 'char *', but argument 2 has type 'struct device *' [-Wformat=]
#define KERN_SOH "\001"  /* ASCII Start Of Header */
 ^
   include/linux/kern_levels.h:12:22: note: in expansion of macro 'KERN_SOH'
#define KERN_WARNING KERN_SOH "4" /* warning conditions */
 ^~~~
>> include/drm/drm_print.h:290:15: note: in expansion of macro 'KERN_WARNING'
 printk##once(KERN_##level "[" DRM_NAME "] " fmt, ##__VA_ARGS__)
  ^
   include/drm/drm_print.h:297:2: note: in expansion of macro '_DRM_PRINTK'
 _DRM_PRINTK(, WARNING, fmt, ##__VA_ARGS__)
 ^~~
>> drivers/gpu//drm/drm_privacy_screen.c:170:3: note: in expansion of macro 
>> 'DRM_WARN'
  DRM_WARN("%s: Odd, connector ACPI node but no privacy scrn?\n",
  ^~~~
   drivers/gpu//drm/drm_privacy_screen.c:170:14: note: format string is defined 
here
  DRM_WARN("%s: Odd, connector ACPI node but no privacy scrn?\n",
~^
--
   In file included from include/linux/printk.h:7:0,
from include/linux/kernel.h:15,
from include/linux/list.h:9,
from include/linux/kobject.h:19,
from include/linux/of.h:17,
from include/linux/irqdomain.h:35,
from include/linux/acpi.h:13,
from drivers/gpu/drm/drm_privacy_screen.c:8:
   drivers/gpu/drm/drm_privacy_screen.c: In function 
'drm_privacy_screen_present':
>> include/linux/kern_levels.h:5:18: warning: format '%s' expects argument of 
>> type 'char *', but argument 2 has type 'struct device *' [-Wformat=]
#define KERN_SOH "\001"  /* ASCII Start Of Header */
 ^
   include/linux/kern_levels.h:12:22: note: in expansion of macro 'KERN_SOH'
#define KERN_WARNING KERN_SOH "4" /* warning conditions */
 ^~~~
>> include/drm/drm_print.h:290:15: note: in expansion of macro 'KERN_WARNING'
 printk##once(KERN_##level "[" DRM_NAME "] " fmt, ##__VA_ARGS__)
  ^
   include/drm/drm_print.h:297:2: note: in expansion of macro '_DRM_PRINTK'
 _DRM_PRINTK(, WARNING, fmt, ##__VA_ARGS__)
 ^~~
   drivers/gpu/drm/drm_privacy_screen.c:170:3: note: in expansion of macro 
'DRM_WARN'
  DRM_WARN("%s: Odd, connector ACPI node but no privacy scrn?\n",
  ^~~~
   drivers/gpu/drm/drm_privacy_screen.c:170:14: note: format string is defined 
here
  DRM_WARN("%s: Odd, connector ACPI node but no privacy scrn?\n",
~^

vim +/KERN_WARNING +290 include/drm/drm_print.h

02c9656b2f0d69 Haneen Mohammed 2017-10-17  288  
02c9656b2f0d69 Haneen Mohammed 2017-10-17  289  #define _DRM_PRINTK(once, 
level, fmt, ...)  \
db87086492581c Joe Perches 2018-03-16 @290  
printk##once(KERN_##level "[" DRM_NAME "] " fmt, ##__VA_ARGS__)
02c9656b2f0d69 Haneen Mohammed 2017-10-17  291  

:: The code at line 290 was first introduced by commit
:: db87086492581c87f768b7d17d01308153ecffc1 drm: Reduce object size of 
DRM_DEV_ uses

:: TO: Joe Perches 
:: CC: Daniel Vetter 

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.fre

Re: [Intel-gfx] [PATCH] drm: Add support for integrated privacy screens

2019-10-24 Thread Pavel Machek
On Tue 2019-10-22 17:12:06, Rajat Jain wrote:
> Certain laptops now come with panels that have integrated privacy
> screens on them. This patch adds support for such panels by adding
> a privacy-screen property to the drm_connector for the panel, that
> the userspace can then use to control and check the status. The idea
> was discussed here:

Much better than separate /sys interface, thanks!
Pavel

-- 
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


signature.asc
Description: Digital signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm: Add support for integrated privacy screens

2019-10-24 Thread Thierry Reding
On Tue, Oct 22, 2019 at 05:12:06PM -0700, Rajat Jain wrote:
> Certain laptops now come with panels that have integrated privacy
> screens on them. This patch adds support for such panels by adding
> a privacy-screen property to the drm_connector for the panel, that
> the userspace can then use to control and check the status. The idea
> was discussed here:
> 
> https://lkml.org/lkml/2019/10/1/786
> 
> ACPI methods are used to identify, query and control privacy screen:
> 
> * Identifying an ACPI object corresponding to the panel: The patch
> follows ACPI Spec 6.3 (available at
> https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf).
> Pages 1119 - 1123 describe what I believe, is a standard way of
> identifying / addressing "display panels" in the ACPI tables, thus
> allowing kernel to attach ACPI nodes to the panel. IMHO, this ability
> to identify and attach ACPI nodes to drm connectors may be useful for
> reasons other privacy-screens, in future.
> 
> * Identifying the presence of privacy screen, and controlling it, is done
> via ACPI _DSM methods.
> 
> Currently, this is done only for the Intel display ports. But in future,
> this can be done for any other ports if the hardware becomes available
> (e.g. external monitors supporting integrated privacy screens?).
> 
> Also, this code can be extended in future to support non-ACPI methods
> (e.g. using a kernel GPIO driver to toggle a gpio that controls the
> privacy-screen).
> 
> Signed-off-by: Rajat Jain 
> ---
>  drivers/gpu/drm/Makefile|   1 +
>  drivers/gpu/drm/drm_atomic_uapi.c   |   5 +
>  drivers/gpu/drm/drm_connector.c |  38 +
>  drivers/gpu/drm/drm_privacy_screen.c| 176 
>  drivers/gpu/drm/i915/display/intel_dp.c |   3 +
>  include/drm/drm_connector.h |  18 +++
>  include/drm/drm_mode_config.h   |   7 +
>  include/drm/drm_privacy_screen.h|  33 +
>  8 files changed, 281 insertions(+)
>  create mode 100644 drivers/gpu/drm/drm_privacy_screen.c
>  create mode 100644 include/drm/drm_privacy_screen.h

I like this much better than the prior proposal to use sysfs. However
the support currently looks a bit tangled. I realize that we only have a
single implementation for this in hardware right now, so there's no use
in over-engineering things, but I think we can do a better job from the
start without getting into too many abstractions. See below.

> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 82ff826b33cc..e1fc33d69bb7 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -19,6 +19,7 @@ drm-y   :=  drm_auth.o drm_cache.o \
>   drm_syncobj.o drm_lease.o drm_writeback.o drm_client.o \
>   drm_client_modeset.o drm_atomic_uapi.o drm_hdcp.o
>  
> +drm-$(CONFIG_ACPI) += drm_privacy_screen.o
>  drm-$(CONFIG_DRM_LEGACY) += drm_legacy_misc.o drm_bufs.o drm_context.o 
> drm_dma.o drm_scatter.o drm_lock.o
>  drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o
>  drm-$(CONFIG_DRM_VM) += drm_vm.o
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
> b/drivers/gpu/drm/drm_atomic_uapi.c
> index 7a26bfb5329c..44131165e4ea 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -30,6 +30,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  
> @@ -766,6 +767,8 @@ static int drm_atomic_connector_set_property(struct 
> drm_connector *connector,
>  fence_ptr);
>   } else if (property == connector->max_bpc_property) {
>   state->max_requested_bpc = val;
> + } else if (property == config->privacy_screen_property) {
> + drm_privacy_screen_set_val(connector, val);

This doesn't look right. Shouldn't you store the value in the connector
state and then leave it up to the connector driver to set it
appropriately? I think that also has the advantage of untangling this
support a little.

>   } else if (connector->funcs->atomic_set_property) {
>   return connector->funcs->atomic_set_property(connector,
>   state, property, val);
> @@ -842,6 +845,8 @@ drm_atomic_connector_get_property(struct drm_connector 
> *connector,
>   *val = 0;
>   } else if (property == connector->max_bpc_property) {
>   *val = state->max_requested_bpc;
> + } else if (property == config->privacy_screen_property) {
> + *val = drm_privacy_screen_get_val(connector);

Similarly, I think this can just return the atomic state's value for
this.

>   } else if (connector->funcs->atomic_get_property) {
>   return connector->funcs->atomic_get_property(connector,
>   state, property, val);
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 4c766624b20d..a31e0382132b 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/

[Intel-gfx] [PATCH] drm: Add support for integrated privacy screens

2019-10-23 Thread Rajat Jain
Certain laptops now come with panels that have integrated privacy
screens on them. This patch adds support for such panels by adding
a privacy-screen property to the drm_connector for the panel, that
the userspace can then use to control and check the status. The idea
was discussed here:

https://lkml.org/lkml/2019/10/1/786

ACPI methods are used to identify, query and control privacy screen:

* Identifying an ACPI object corresponding to the panel: The patch
follows ACPI Spec 6.3 (available at
https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf).
Pages 1119 - 1123 describe what I believe, is a standard way of
identifying / addressing "display panels" in the ACPI tables, thus
allowing kernel to attach ACPI nodes to the panel. IMHO, this ability
to identify and attach ACPI nodes to drm connectors may be useful for
reasons other privacy-screens, in future.

* Identifying the presence of privacy screen, and controlling it, is done
via ACPI _DSM methods.

Currently, this is done only for the Intel display ports. But in future,
this can be done for any other ports if the hardware becomes available
(e.g. external monitors supporting integrated privacy screens?).

Also, this code can be extended in future to support non-ACPI methods
(e.g. using a kernel GPIO driver to toggle a gpio that controls the
privacy-screen).

Signed-off-by: Rajat Jain 
---
 drivers/gpu/drm/Makefile|   1 +
 drivers/gpu/drm/drm_atomic_uapi.c   |   5 +
 drivers/gpu/drm/drm_connector.c |  38 +
 drivers/gpu/drm/drm_privacy_screen.c| 176 
 drivers/gpu/drm/i915/display/intel_dp.c |   3 +
 include/drm/drm_connector.h |  18 +++
 include/drm/drm_mode_config.h   |   7 +
 include/drm/drm_privacy_screen.h|  33 +
 8 files changed, 281 insertions(+)
 create mode 100644 drivers/gpu/drm/drm_privacy_screen.c
 create mode 100644 include/drm/drm_privacy_screen.h

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 82ff826b33cc..e1fc33d69bb7 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -19,6 +19,7 @@ drm-y   :=drm_auth.o drm_cache.o \
drm_syncobj.o drm_lease.o drm_writeback.o drm_client.o \
drm_client_modeset.o drm_atomic_uapi.o drm_hdcp.o
 
+drm-$(CONFIG_ACPI) += drm_privacy_screen.o
 drm-$(CONFIG_DRM_LEGACY) += drm_legacy_misc.o drm_bufs.o drm_context.o 
drm_dma.o drm_scatter.o drm_lock.o
 drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o
 drm-$(CONFIG_DRM_VM) += drm_vm.o
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c
index 7a26bfb5329c..44131165e4ea 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -30,6 +30,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -766,6 +767,8 @@ static int drm_atomic_connector_set_property(struct 
drm_connector *connector,
   fence_ptr);
} else if (property == connector->max_bpc_property) {
state->max_requested_bpc = val;
+   } else if (property == config->privacy_screen_property) {
+   drm_privacy_screen_set_val(connector, val);
} else if (connector->funcs->atomic_set_property) {
return connector->funcs->atomic_set_property(connector,
state, property, val);
@@ -842,6 +845,8 @@ drm_atomic_connector_get_property(struct drm_connector 
*connector,
*val = 0;
} else if (property == connector->max_bpc_property) {
*val = state->max_requested_bpc;
+   } else if (property == config->privacy_screen_property) {
+   *val = drm_privacy_screen_get_val(connector);
} else if (connector->funcs->atomic_get_property) {
return connector->funcs->atomic_get_property(connector,
state, property, val);
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 4c766624b20d..a31e0382132b 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -821,6 +821,11 @@ static const struct drm_prop_enum_list 
drm_panel_orientation_enum_list[] = {
{ DRM_MODE_PANEL_ORIENTATION_RIGHT_UP,  "Right Side Up" },
 };
 
+static const struct drm_prop_enum_list drm_privacy_screen_enum_list[] = {
+   { DRM_PRIVACY_SCREEN_DISABLED, "Disabled" },
+   { DRM_PRIVACY_SCREEN_ENABLED, "Enabled" },
+};
+
 static const struct drm_prop_enum_list drm_dvi_i_select_enum_list[] = {
{ DRM_MODE_SUBCONNECTOR_Automatic, "Automatic" }, /* DVI-I and TV-out */
{ DRM_MODE_SUBCONNECTOR_DVID,  "DVI-D" }, /* DVI-I  */
@@ -2253,6 +2258,39 @@ static void drm_tile_group_free(struct kref *kref)
kfree(tg);
 }
 
+/**
+ * drm_connector_init_privacy_screen_property -
+ * create and attach the connecter's privacy-screen property.
+ * @connector: connecto

Re: [Intel-gfx] [PATCH] drm: Add support for integrated privacy screens

2019-10-23 Thread Daniel Vetter
On Tue, Oct 22, 2019 at 05:12:06PM -0700, Rajat Jain wrote:
> Certain laptops now come with panels that have integrated privacy
> screens on them. This patch adds support for such panels by adding
> a privacy-screen property to the drm_connector for the panel, that
> the userspace can then use to control and check the status. The idea
> was discussed here:
> 
> https://lkml.org/lkml/2019/10/1/786
> 
> ACPI methods are used to identify, query and control privacy screen:
> 
> * Identifying an ACPI object corresponding to the panel: The patch
> follows ACPI Spec 6.3 (available at
> https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf).
> Pages 1119 - 1123 describe what I believe, is a standard way of
> identifying / addressing "display panels" in the ACPI tables, thus
> allowing kernel to attach ACPI nodes to the panel. IMHO, this ability
> to identify and attach ACPI nodes to drm connectors may be useful for
> reasons other privacy-screens, in future.
> 
> * Identifying the presence of privacy screen, and controlling it, is done
> via ACPI _DSM methods.
> 
> Currently, this is done only for the Intel display ports. But in future,
> this can be done for any other ports if the hardware becomes available
> (e.g. external monitors supporting integrated privacy screens?).
> 
> Also, this code can be extended in future to support non-ACPI methods
> (e.g. using a kernel GPIO driver to toggle a gpio that controls the
> privacy-screen).
> 
> Signed-off-by: Rajat Jain 

New properties need property docs in the relevant section:

https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#kms-properties

$ make htmldocs

for building them locally.

Cheers, Daniel

> ---
>  drivers/gpu/drm/Makefile|   1 +
>  drivers/gpu/drm/drm_atomic_uapi.c   |   5 +
>  drivers/gpu/drm/drm_connector.c |  38 +
>  drivers/gpu/drm/drm_privacy_screen.c| 176 
>  drivers/gpu/drm/i915/display/intel_dp.c |   3 +
>  include/drm/drm_connector.h |  18 +++
>  include/drm/drm_mode_config.h   |   7 +
>  include/drm/drm_privacy_screen.h|  33 +
>  8 files changed, 281 insertions(+)
>  create mode 100644 drivers/gpu/drm/drm_privacy_screen.c
>  create mode 100644 include/drm/drm_privacy_screen.h
> 
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 82ff826b33cc..e1fc33d69bb7 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -19,6 +19,7 @@ drm-y   :=  drm_auth.o drm_cache.o \
>   drm_syncobj.o drm_lease.o drm_writeback.o drm_client.o \
>   drm_client_modeset.o drm_atomic_uapi.o drm_hdcp.o
>  
> +drm-$(CONFIG_ACPI) += drm_privacy_screen.o
>  drm-$(CONFIG_DRM_LEGACY) += drm_legacy_misc.o drm_bufs.o drm_context.o 
> drm_dma.o drm_scatter.o drm_lock.o
>  drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o
>  drm-$(CONFIG_DRM_VM) += drm_vm.o
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
> b/drivers/gpu/drm/drm_atomic_uapi.c
> index 7a26bfb5329c..44131165e4ea 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -30,6 +30,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  
> @@ -766,6 +767,8 @@ static int drm_atomic_connector_set_property(struct 
> drm_connector *connector,
>  fence_ptr);
>   } else if (property == connector->max_bpc_property) {
>   state->max_requested_bpc = val;
> + } else if (property == config->privacy_screen_property) {
> + drm_privacy_screen_set_val(connector, val);
>   } else if (connector->funcs->atomic_set_property) {
>   return connector->funcs->atomic_set_property(connector,
>   state, property, val);
> @@ -842,6 +845,8 @@ drm_atomic_connector_get_property(struct drm_connector 
> *connector,
>   *val = 0;
>   } else if (property == connector->max_bpc_property) {
>   *val = state->max_requested_bpc;
> + } else if (property == config->privacy_screen_property) {
> + *val = drm_privacy_screen_get_val(connector);
>   } else if (connector->funcs->atomic_get_property) {
>   return connector->funcs->atomic_get_property(connector,
>   state, property, val);
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 4c766624b20d..a31e0382132b 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -821,6 +821,11 @@ static const struct drm_prop_enum_list 
> drm_panel_orientation_enum_list[] = {
>   { DRM_MODE_PANEL_ORIENTATION_RIGHT_UP,  "Right Side Up" },
>  };
>  
> +static const struct drm_prop_enum_list drm_privacy_screen_enum_list[] = {
> + { DRM_PRIVACY_SCREEN_DISABLED, "Disabled" },
> + { DRM_PRIVACY_SCREEN_ENABLED, "Enabled" },
> +};
> +
>  static const struct drm_prop_enum_list drm_dvi_i_