Re: [Intel-gfx] [PATCH resend 0/9] drm: Add privacy-screen class and connector properties

2021-04-14 Thread Rajat Jain
+Jesse Barnes

Hello,

On Wed, Apr 14, 2021 at 8:11 AM Hans de Goede  wrote:
>
> Hi All,
>
> Here is the privacy-screen related code which I last posted in August
> of last year. To the best of my knowledge there is consensus about /
> everyone is in agreement with the new userspace API (2 connector properties)
> this patch-set add (patch 1 of the series).
>
> The blocker the last time was that there were no userspace users of
> the new properties and as a rule we don't add new drm userspace API
> without users.
>
> There now is GNOME userspace code using the new properties:
> https://hackmd.io/@3v1n0/rkyIy3BOw
>
> The new API works as designed for this userspace user and the branches
> mentioned at the above link add the following features to GNOME:
>
> 1. Showing an OSD notification when the privacy-screen is toggled on/off
>through hotkeys handled by the embedded-controller
> 2. Allowing control of the privacy-screen from the GNOME control-panel,
>including the on/off slider shown there updating to match the hw-setting
>when the setting is changed with the control-panel open.
> 3. Restoring the last user-setting at login
>
> This series consists of a number of different parts:
>
> 1. A new version of Rajat's privacy-screen connector properties patch,
> this adds new userspace API in the form of new properties

Thanks a lot Hans!

>
> 2. Since on most devices the privacy screen is actually controlled by
> some vendor specific ACPI/WMI interface which has a driver under
> drivers/platform/x86, we need some "glue" code to make this functionality
> available to KMS drivers. Patches 2-4 add a new privacy-screen class for
> this, which allows non KMS drivers (and possibly KMS drivers too) to
> register a privacy-screen device and also adds an interface for KMS drivers
> to get access to the privacy-screen associated with a specific connector.
> This is modelled similar to how we deal with e.g. PWMs and GPIOs in the
> kernel, including separate includes for consumers and providers(drivers).
>
> 3. Some drm_connector helper functions to keep the actual changes needed
> for this in individual KMS drivers as small as possible (patch 5).
>
> 4. Make the thinkpad_acpi code register a privacy-screen device on
> ThinkPads with a privacy-screen (patches 6-8)
>
> 5. Make the i915 driver export the privacy-screen functionality through
> the connector properties on the eDP connector.
>
> I believe that it would be best to merge the entire series, including
> the thinkpad_acpi changes through drm-misc in one go. As the pdx86
> subsys maintainer I hereby give my ack for merging the thinkpad_acpi
> changes through drm-misc.

Huge +1 to this. This feature has been waiting for acceptance for
almost 1.5 years now, and we (Chromeos) have been carrying it out of
the tree since then. We have real products today that use this
feature. If a version of it can please be accepted and applied, we
look forward to cherry-pick / backport it to our kernels (and adapt chrome
code to the new API).

Thanks,
Rajat



>
> There is one small caveat with this series, which it is good to be
> aware of. The i915 driver will now return -EPROBE_DEFER on Thinkpads
> with an eprivacy screen, until the thinkpad_acpi driver is loaded.
> This means that initrd generation tools will need to be updated to
> include thinkpad_acpi when the i915 driver is added to the initrd.
> Without this the loading of the i915 driver will be delayed to after
> the switch to real rootfs.
>
> Regards,
>
> Hans
>
>
> Hans de Goede (8):
>   drm: Add privacy-screen class
>   drm/privacy-screen: Add X86 specific arch init code
>   drm/privacy-screen: Add notifier support
>   drm/connector: Add a drm_connector privacy-screen helper functions
>   platform/x86: thinkpad_acpi: Add hotkey_notify_extended_hotkey()
> helper
>   platform/x86: thinkpad_acpi: Get privacy-screen / lcdshadow ACPI
> handles only once
>   platform/x86: thinkpad_acpi: Register a privacy-screen device
>   drm/i915: Add privacy-screen support
>
> Rajat Jain (1):
>   drm/connector: Add support for privacy-screen properties (v4)
>
>  Documentation/gpu/drm-kms-helpers.rst|  15 +
>  Documentation/gpu/drm-kms.rst|   2 +
>  MAINTAINERS  |   8 +
>  drivers/gpu/drm/Kconfig  |   5 +
>  drivers/gpu/drm/Makefile |   4 +
>  drivers/gpu/drm/drm_atomic_uapi.c|   4 +
>  drivers/gpu/drm/drm_connector.c  | 214 
>  drivers/gpu/drm/drm_privacy_screen.c | 493 +++
>  drivers/gpu/drm/drm_privacy_screen_x86.c |  82 +++
>  drivers/gpu/drm/i915/display/intel_dis

[Intel-gfx] [PATCH v9 4/5] drm/i915: Add helper code for ACPI privacy screen

2020-03-12 Thread Rajat Jain
Add helper functions that can allow i915 to detect and control
an integrated privacy screen via ACPI methods. These shall be used
in the next patch.

Signed-off-by: Rajat Jain 
---
v9: same as v8
v8: Initial version. formed by refactoring the previous patch 4.
print the connector name in the debug messages.

 drivers/gpu/drm/i915/Makefile |   3 +-
 .../drm/i915/display/intel_privacy_screen.c   | 184 ++
 .../drm/i915/display/intel_privacy_screen.h   |  27 +++
 3 files changed, 213 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.c
 create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 9f887a86e555d..da42389107f9c 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -209,7 +209,8 @@ i915-y += \
display/intel_vga.o
 i915-$(CONFIG_ACPI) += \
display/intel_acpi.o \
-   display/intel_opregion.o
+   display/intel_opregion.o \
+   display/intel_privacy_screen.o
 i915-$(CONFIG_DRM_FBDEV_EMULATION) += \
display/intel_fbdev.o
 
diff --git a/drivers/gpu/drm/i915/display/intel_privacy_screen.c 
b/drivers/gpu/drm/i915/display/intel_privacy_screen.c
new file mode 100644
index 0..66039103c821b
--- /dev/null
+++ b/drivers/gpu/drm/i915/display/intel_privacy_screen.c
@@ -0,0 +1,184 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT
+/*
+ * Intel ACPI privacy screen code
+ *
+ * Copyright © 2020 Google Inc.
+ *
+ * This code can help detect and control an integrated EPS (electronic
+ * privacy screen) via ACPI functions. It expects an ACPI node for the
+ * drm connector device with the following elements:
+ *
+ * UUID should be "c7033113-8720-4ceb-9090-9d52b3e52d73"
+ *
+ * _ADR = ACPI address per Spec (also see intel_acpi_device_id_update())
+ * https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf
+ * Pages 1119 - 1123.
+ *
+ * _DSM method that will perform the following functions according to
+ * Local1 argument passed to it:
+ *  - Local1 = 0 (EPS capabilities): Report EPS presence and capabilities.
+ *  - Local1 = 1 (EPS State)  :  _DSM returns 1 if EPS is enabled, 0 otherwise.
+ *  - Local1 = 2 (EPS Enable) :  _DSM enables EPS
+ *  - Local1 = 3 (EPS Disable):  _DSM disables EPS
+ *
+ * Here is a sample ACPI node:
+ *
+ *  Scope (\_SB.PCI0.GFX0) // Intel graphics device (PCI device)
+ *  {
+ *  Method (_DOD, 0, NotSerialized)  // _DOD: Display Output Devices
+ *  {
+ *  Return (Package (0x01)
+ *  {
+ *  0x80010400
+ *  })
+ *  }
+ *
+ *  Device (LCD)
+ *  {
+ *  Name (_ADR, 0x80010400)  // _ADR: Address
+ *  Name (_STA, 0x0F)  // _STA: Status
+ *
+ *  Method (EPSP, 0, NotSerialized) // EPS Present
+ *  {
+ *  Return (0x01)
+ *  }
+ *
+ *  Method (EPSS, 0, NotSerialized) // EPS State
+ *  {
+ *  Local0 = \_SB.PCI0.GRXS (0xCD)
+ *  Return (Local0)
+ *  }
+ *
+ *  Method (EPSE, 0, NotSerialized) // EPS Enable
+ *  {
+ *  \_SB.PCI0.STXS (0xCD)
+ *  }
+ *
+ *  Method (EPSD, 0, NotSerialized) // EPS Disable
+ *  {
+ *  \_SB.PCI0.CTXS (0xCD)
+ *  }
+ *
+ *  Method (_DSM, 4, Serialized)  // _DSM: Device-Specific Method
+ *  {
+ *  ToBuffer (Arg0, Local0)
+ *  If ((Local0 == ToUUID 
("c7033113-8720-4ceb-9090-9d52b3e52d73")))
+ *  {
+ *  ToInteger (Arg2, Local1)
+ *  If ((Local1 == Zero))
+ *  {
+ *  Local2 = EPSP ()
+ *  If ((Local2 == One))
+ *  {
+ *  Return (Buffer (One)
+ *  {
+ *   0x0F
+ *  })
+ *  }
+ *  }
+ *
+ *  If ((Local1 == One))
+ *  {
+ *  Return (EPSS ())
+ *  }
+ *
+ *  If ((Local1 == 0x02))
+ *  {
+ *  EPSE ()
+ *  }
+ *
+ *  If ((Local1 == 0x03))
+ *  {
+ *  EPSD ()
+ *  }
+ *
+ *  Return (Buffer (One)
+ *  {
+ *   0x00
+ *  })
+ *  }
+ *
+ *  Return (Buffer (One)
+ *  {
+ *   0x00
+ *  })
+ *  }
+ *  }
+ *  }
+ */
+
+#include 
+
+#include "intel_privacy_screen.h"
+
+#define CONN_NAME(conn)\
+   (conn->base.kdev ? dev_name(conn->base.kdev) : "NONAME")
+
+#define CONNECTOR_DSM_REVI

Re: [Intel-gfx] [PATCH v7 4/4] drm/i915: Add support for integrated privacy screen

2020-03-11 Thread Rajat Jain
Hi Jani,

On Mon, Mar 9, 2020 at 5:18 PM Rajat Jain  wrote:
>
> Hi Jani,
>
> I have 1 question / need 1 help about this patch:

Kind ignore, I found the answer, and posted my new patchset here:
https://patchwork.freedesktop.org/series/74607/

I got a "failed to apply" email from the patchwork. Can you please let
me know on which branch is it trying to apply it? I have currently
rebased my patchset to drm-intel-next-queued.

Thanks & Best Regards,

Rajat

>
> On Mon, Mar 9, 2020 at 5:06 PM Rajat Jain  wrote:
> >
> > Add support for an ACPI based integrated privacy screen that is
> > available on some systems.
> >
> > Signed-off-by: Rajat Jain 
> > ---
> > v7: * Move the privacy-screen property back into drm core.
> > * Do the actual HW EPS toggling at commit time.
> > * Provide a sample ACPI node for reference in comments.
> > v6: Always initialize prop in intel_attach_privacy_screen_property()
> > v5: fix a cosmetic checkpatch warning
> > v4: Fix a typo in intel_privacy_screen.h
> > v3: * Change license to GPL-2.0 OR MIT
> > * Move privacy screen enum from UAPI to intel_display_types.h
> > * Rename parameter name and some other minor changes.
> > v2: Formed by splitting the original patch into multiple patches.
> > - All code has been moved into i915 now.
> > - Privacy screen is a i915 property
> > - Have a local state variable to store the prvacy screen. Don't read
> >   it from hardware.
> >
> >  drivers/gpu/drm/i915/Makefile |   3 +-
> >  drivers/gpu/drm/i915/display/intel_atomic.c   |   1 +
> >  drivers/gpu/drm/i915/display/intel_dp.c   |  30 ++-
> >  .../drm/i915/display/intel_privacy_screen.c   | 175 ++
> >  .../drm/i915/display/intel_privacy_screen.h   |  27 +++
> >  5 files changed, 234 insertions(+), 2 deletions(-)
> >  create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.c
> >  create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.h
> >
> > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> > index 9f887a86e555d..da42389107f9c 100644
> > --- a/drivers/gpu/drm/i915/Makefile
> > +++ b/drivers/gpu/drm/i915/Makefile
> > @@ -209,7 +209,8 @@ i915-y += \
> > display/intel_vga.o
> >  i915-$(CONFIG_ACPI) += \
> > display/intel_acpi.o \
> > -   display/intel_opregion.o
> > +   display/intel_opregion.o \
> > +   display/intel_privacy_screen.o
> >  i915-$(CONFIG_DRM_FBDEV_EMULATION) += \
> > display/intel_fbdev.o
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c 
> > b/drivers/gpu/drm/i915/display/intel_atomic.c
> > index d043057d2fa03..fc6264b4a7f73 100644
> > --- a/drivers/gpu/drm/i915/display/intel_atomic.c
> > +++ b/drivers/gpu/drm/i915/display/intel_atomic.c
> > @@ -150,6 +150,7 @@ int intel_digital_connector_atomic_check(struct 
> > drm_connector *conn,
> > new_conn_state->base.picture_aspect_ratio != 
> > old_conn_state->base.picture_aspect_ratio ||
> > new_conn_state->base.content_type != 
> > old_conn_state->base.content_type ||
> > new_conn_state->base.scaling_mode != 
> > old_conn_state->base.scaling_mode ||
> > +   new_conn_state->base.privacy_screen_status != 
> > old_conn_state->base.privacy_screen_status ||
> > !blob_equal(new_conn_state->base.hdr_output_metadata,
> > old_conn_state->base.hdr_output_metadata))
> > crtc_state->mode_changed = true;
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
> > b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 41c623b029464..a39b0c420b50a 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -62,6 +62,7 @@
> >  #include "intel_lspcon.h"
> >  #include "intel_lvds.h"
> >  #include "intel_panel.h"
> > +#include "intel_privacy_screen.h"
> >  #include "intel_psr.h"
> >  #include "intel_sideband.h"
> >  #include "intel_tc.h"
> > @@ -5886,6 +5887,12 @@ intel_dp_connector_register(struct drm_connector 
> > *connector)
> > dev_priv->acpi_scan_done = true;
> > }
> >
> > +   /* Check for integrated Privacy screen support */
> > +   if (intel_privacy_screen_present(to_intel_connector(connector)))
> > +   drm_connector_attach_pr

[Intel-gfx] [PATCH v8 4/5] drm/i915: Add helper code for ACPI privacy screen

2020-03-11 Thread Rajat Jain
Add helper functions that can allow i915 to detect and control
an integrated privacy screen via ACPI methods. These shall be used
in the next patch.

Signed-off-by: Rajat Jain 
---
v8: Initial version. formed by refactoring the previous patch 4.
print the connector name in the debug messages.

 drivers/gpu/drm/i915/Makefile |   3 +-
 .../drm/i915/display/intel_privacy_screen.c   | 184 ++
 .../drm/i915/display/intel_privacy_screen.h   |  27 +++
 3 files changed, 213 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.c
 create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 9f887a86e555d..da42389107f9c 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -209,7 +209,8 @@ i915-y += \
display/intel_vga.o
 i915-$(CONFIG_ACPI) += \
display/intel_acpi.o \
-   display/intel_opregion.o
+   display/intel_opregion.o \
+   display/intel_privacy_screen.o
 i915-$(CONFIG_DRM_FBDEV_EMULATION) += \
display/intel_fbdev.o
 
diff --git a/drivers/gpu/drm/i915/display/intel_privacy_screen.c 
b/drivers/gpu/drm/i915/display/intel_privacy_screen.c
new file mode 100644
index 0..66039103c821b
--- /dev/null
+++ b/drivers/gpu/drm/i915/display/intel_privacy_screen.c
@@ -0,0 +1,184 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT
+/*
+ * Intel ACPI privacy screen code
+ *
+ * Copyright © 2020 Google Inc.
+ *
+ * This code can help detect and control an integrated EPS (electronic
+ * privacy screen) via ACPI functions. It expects an ACPI node for the
+ * drm connector device with the following elements:
+ *
+ * UUID should be "c7033113-8720-4ceb-9090-9d52b3e52d73"
+ *
+ * _ADR = ACPI address per Spec (also see intel_acpi_device_id_update())
+ * https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf
+ * Pages 1119 - 1123.
+ *
+ * _DSM method that will perform the following functions according to
+ * Local1 argument passed to it:
+ *  - Local1 = 0 (EPS capabilities): Report EPS presence and capabilities.
+ *  - Local1 = 1 (EPS State)  :  _DSM returns 1 if EPS is enabled, 0 otherwise.
+ *  - Local1 = 2 (EPS Enable) :  _DSM enables EPS
+ *  - Local1 = 3 (EPS Disable):  _DSM disables EPS
+ *
+ * Here is a sample ACPI node:
+ *
+ *  Scope (\_SB.PCI0.GFX0) // Intel graphics device (PCI device)
+ *  {
+ *  Method (_DOD, 0, NotSerialized)  // _DOD: Display Output Devices
+ *  {
+ *  Return (Package (0x01)
+ *  {
+ *  0x80010400
+ *  })
+ *  }
+ *
+ *  Device (LCD)
+ *  {
+ *  Name (_ADR, 0x80010400)  // _ADR: Address
+ *  Name (_STA, 0x0F)  // _STA: Status
+ *
+ *  Method (EPSP, 0, NotSerialized) // EPS Present
+ *  {
+ *  Return (0x01)
+ *  }
+ *
+ *  Method (EPSS, 0, NotSerialized) // EPS State
+ *  {
+ *  Local0 = \_SB.PCI0.GRXS (0xCD)
+ *  Return (Local0)
+ *  }
+ *
+ *  Method (EPSE, 0, NotSerialized) // EPS Enable
+ *  {
+ *  \_SB.PCI0.STXS (0xCD)
+ *  }
+ *
+ *  Method (EPSD, 0, NotSerialized) // EPS Disable
+ *  {
+ *  \_SB.PCI0.CTXS (0xCD)
+ *  }
+ *
+ *  Method (_DSM, 4, Serialized)  // _DSM: Device-Specific Method
+ *  {
+ *  ToBuffer (Arg0, Local0)
+ *  If ((Local0 == ToUUID 
("c7033113-8720-4ceb-9090-9d52b3e52d73")))
+ *  {
+ *  ToInteger (Arg2, Local1)
+ *  If ((Local1 == Zero))
+ *  {
+ *  Local2 = EPSP ()
+ *  If ((Local2 == One))
+ *  {
+ *  Return (Buffer (One)
+ *  {
+ *   0x0F
+ *  })
+ *  }
+ *  }
+ *
+ *  If ((Local1 == One))
+ *  {
+ *  Return (EPSS ())
+ *  }
+ *
+ *  If ((Local1 == 0x02))
+ *  {
+ *  EPSE ()
+ *  }
+ *
+ *  If ((Local1 == 0x03))
+ *  {
+ *  EPSD ()
+ *  }
+ *
+ *  Return (Buffer (One)
+ *  {
+ *   0x00
+ *  })
+ *  }
+ *
+ *  Return (Buffer (One)
+ *  {
+ *   0x00
+ *  })
+ *  }
+ *  }
+ *  }
+ */
+
+#include 
+
+#include "intel_privacy_screen.h"
+
+#define CONN_NAME(conn)\
+   (conn->base.kdev ? dev_name(conn->base.kdev) : "NONAME")
+
+#define CONNECTOR_DSM_REVID 1
+
+#define CO

Re: [Intel-gfx] [PATCH v7 4/4] drm/i915: Add support for integrated privacy screen

2020-03-09 Thread Rajat Jain
Hi Jani,

I have 1 question / need 1 help about this patch:

On Mon, Mar 9, 2020 at 5:06 PM Rajat Jain  wrote:
>
> Add support for an ACPI based integrated privacy screen that is
> available on some systems.
>
> Signed-off-by: Rajat Jain 
> ---
> v7: * Move the privacy-screen property back into drm core.
> * Do the actual HW EPS toggling at commit time.
> * Provide a sample ACPI node for reference in comments.
> v6: Always initialize prop in intel_attach_privacy_screen_property()
> v5: fix a cosmetic checkpatch warning
> v4: Fix a typo in intel_privacy_screen.h
> v3: * Change license to GPL-2.0 OR MIT
> * Move privacy screen enum from UAPI to intel_display_types.h
> * Rename parameter name and some other minor changes.
> v2: Formed by splitting the original patch into multiple patches.
> - All code has been moved into i915 now.
> - Privacy screen is a i915 property
> - Have a local state variable to store the prvacy screen. Don't read
>   it from hardware.
>
>  drivers/gpu/drm/i915/Makefile |   3 +-
>  drivers/gpu/drm/i915/display/intel_atomic.c   |   1 +
>  drivers/gpu/drm/i915/display/intel_dp.c   |  30 ++-
>  .../drm/i915/display/intel_privacy_screen.c   | 175 ++
>  .../drm/i915/display/intel_privacy_screen.h   |  27 +++
>  5 files changed, 234 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.c
>  create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.h
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 9f887a86e555d..da42389107f9c 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -209,7 +209,8 @@ i915-y += \
> display/intel_vga.o
>  i915-$(CONFIG_ACPI) += \
> display/intel_acpi.o \
> -   display/intel_opregion.o
> +   display/intel_opregion.o \
> +   display/intel_privacy_screen.o
>  i915-$(CONFIG_DRM_FBDEV_EMULATION) += \
> display/intel_fbdev.o
>
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c 
> b/drivers/gpu/drm/i915/display/intel_atomic.c
> index d043057d2fa03..fc6264b4a7f73 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic.c
> @@ -150,6 +150,7 @@ int intel_digital_connector_atomic_check(struct 
> drm_connector *conn,
> new_conn_state->base.picture_aspect_ratio != 
> old_conn_state->base.picture_aspect_ratio ||
> new_conn_state->base.content_type != 
> old_conn_state->base.content_type ||
> new_conn_state->base.scaling_mode != 
> old_conn_state->base.scaling_mode ||
> +   new_conn_state->base.privacy_screen_status != 
> old_conn_state->base.privacy_screen_status ||
> !blob_equal(new_conn_state->base.hdr_output_metadata,
> old_conn_state->base.hdr_output_metadata))
> crtc_state->mode_changed = true;
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index 41c623b029464..a39b0c420b50a 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -62,6 +62,7 @@
>  #include "intel_lspcon.h"
>  #include "intel_lvds.h"
>  #include "intel_panel.h"
> +#include "intel_privacy_screen.h"
>  #include "intel_psr.h"
>  #include "intel_sideband.h"
>  #include "intel_tc.h"
> @@ -5886,6 +5887,12 @@ intel_dp_connector_register(struct drm_connector 
> *connector)
> dev_priv->acpi_scan_done = true;
> }
>
> +   /* Check for integrated Privacy screen support */
> +   if (intel_privacy_screen_present(to_intel_connector(connector)))
> +   drm_connector_attach_privacy_screen_property(connector);
> +   else
> +   drm_connector_destroy_privacy_screen_property(connector);
> +
> DRM_DEBUG_KMS("registering %s bus for %s\n",
>   intel_dp->aux.name, connector->kdev->kobj.name);
>
> @@ -6881,9 +6888,30 @@ intel_dp_add_properties(struct intel_dp *intel_dp, 
> struct drm_connector *connect
> drm_connector_attach_scaling_mode_property(connector, 
> allowed_scalers);
>
> connector->state->scaling_mode = DRM_MODE_SCALE_ASPECT;
> +
> +   drm_connector_create_privacy_screen_property(connector);
> }
>  }
>
> +static void intel_dp_update_privacy_screen(struct intel_encoder *encoder,
> +   const struct intel_crtc_state *crtc_state,
&g

[Intel-gfx] [PATCH v7 4/4] drm/i915: Add support for integrated privacy screen

2020-03-09 Thread Rajat Jain
Add support for an ACPI based integrated privacy screen that is
available on some systems.

Signed-off-by: Rajat Jain 
---
v7: * Move the privacy-screen property back into drm core.
* Do the actual HW EPS toggling at commit time.
* Provide a sample ACPI node for reference in comments. 
v6: Always initialize prop in intel_attach_privacy_screen_property()
v5: fix a cosmetic checkpatch warning
v4: Fix a typo in intel_privacy_screen.h
v3: * Change license to GPL-2.0 OR MIT
* Move privacy screen enum from UAPI to intel_display_types.h
* Rename parameter name and some other minor changes.
v2: Formed by splitting the original patch into multiple patches.
- All code has been moved into i915 now.
- Privacy screen is a i915 property
- Have a local state variable to store the prvacy screen. Don't read
  it from hardware.

 drivers/gpu/drm/i915/Makefile |   3 +-
 drivers/gpu/drm/i915/display/intel_atomic.c   |   1 +
 drivers/gpu/drm/i915/display/intel_dp.c   |  30 ++-
 .../drm/i915/display/intel_privacy_screen.c   | 175 ++
 .../drm/i915/display/intel_privacy_screen.h   |  27 +++
 5 files changed, 234 insertions(+), 2 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.c
 create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 9f887a86e555d..da42389107f9c 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -209,7 +209,8 @@ i915-y += \
display/intel_vga.o
 i915-$(CONFIG_ACPI) += \
display/intel_acpi.o \
-   display/intel_opregion.o
+   display/intel_opregion.o \
+   display/intel_privacy_screen.o
 i915-$(CONFIG_DRM_FBDEV_EMULATION) += \
display/intel_fbdev.o
 
diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c 
b/drivers/gpu/drm/i915/display/intel_atomic.c
index d043057d2fa03..fc6264b4a7f73 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic.c
@@ -150,6 +150,7 @@ int intel_digital_connector_atomic_check(struct 
drm_connector *conn,
new_conn_state->base.picture_aspect_ratio != 
old_conn_state->base.picture_aspect_ratio ||
new_conn_state->base.content_type != 
old_conn_state->base.content_type ||
new_conn_state->base.scaling_mode != 
old_conn_state->base.scaling_mode ||
+   new_conn_state->base.privacy_screen_status != 
old_conn_state->base.privacy_screen_status ||
!blob_equal(new_conn_state->base.hdr_output_metadata,
old_conn_state->base.hdr_output_metadata))
crtc_state->mode_changed = true;
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
b/drivers/gpu/drm/i915/display/intel_dp.c
index 41c623b029464..a39b0c420b50a 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -62,6 +62,7 @@
 #include "intel_lspcon.h"
 #include "intel_lvds.h"
 #include "intel_panel.h"
+#include "intel_privacy_screen.h"
 #include "intel_psr.h"
 #include "intel_sideband.h"
 #include "intel_tc.h"
@@ -5886,6 +5887,12 @@ intel_dp_connector_register(struct drm_connector 
*connector)
dev_priv->acpi_scan_done = true;
}
 
+   /* Check for integrated Privacy screen support */
+   if (intel_privacy_screen_present(to_intel_connector(connector)))
+   drm_connector_attach_privacy_screen_property(connector);
+   else
+   drm_connector_destroy_privacy_screen_property(connector);
+
DRM_DEBUG_KMS("registering %s bus for %s\n",
  intel_dp->aux.name, connector->kdev->kobj.name);
 
@@ -6881,9 +6888,30 @@ intel_dp_add_properties(struct intel_dp *intel_dp, 
struct drm_connector *connect
drm_connector_attach_scaling_mode_property(connector, 
allowed_scalers);
 
connector->state->scaling_mode = DRM_MODE_SCALE_ASPECT;
+
+   drm_connector_create_privacy_screen_property(connector);
}
 }
 
+static void intel_dp_update_privacy_screen(struct intel_encoder *encoder,
+   const struct intel_crtc_state *crtc_state,
+   const struct drm_connector_state *conn_state)
+{
+   struct drm_connector *connector = conn_state->connector;
+
+   if (connector->privacy_screen_property)
+   intel_privacy_screen_set_val(to_intel_connector(connector),
+conn_state->privacy_screen_status);
+}
+
+static void intel_dp_update_pipe(struct intel_encoder *encoder,
+const struct intel_crtc_state *crtc_state,
+const struct drm_connector_state *conn_state)
+{
+   intel_dp

Re: [Intel-gfx] [PATCH v6 3/3] drm/i915: Add support for integrated privacy screens

2020-03-05 Thread Rajat Jain
Hi Jani,

Thank you for the comments. Please see my responses inline.

On Thu, Mar 5, 2020 at 2:02 AM Jani Nikula  wrote:
>
> On Wed, 04 Mar 2020, 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 intel_connector for the panel, that
> > the userspace can then use to control and check the status.
> >
> > 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?).
>
> I think you should add the property at the drm core level in
> drm_connector.c, not in i915, to ensure we have the same property across
> drivers. Even if, for now, you leave the acpi implementation part in
> i915.

OK, will do. In order to do that I may need to introduce driver level
hooks that i915 driver can populate and drm core can call (or may be
some functions to add privacy screen property that drm core exports
and i915 driver will call).

>
> More comments inline.
>
> >
> > Signed-off-by: Rajat Jain 
> > ---
> > v6: Always initialize prop in intel_attach_privacy_screen_property()
> > v5: fix a cosmetic checkpatch warning
> > v4: Fix a typo in intel_privacy_screen.h
> > v3: * Change license to GPL-2.0 OR MIT
> > * Move privacy screen enum from UAPI to intel_display_types.h
> > * Rename parameter name and some other minor changes.
> > v2: Formed by splitting the original patch into multiple patches.
> > - All code has been moved into i915 now.
> > - Privacy screen is a i915 property
> > - Have a local state variable to store the prvacy screen. Don't read
> >   it from hardware.
> >
> >  drivers/gpu/drm/i915/Makefile |  3 +-
> >  drivers/gpu/drm/i915/display/intel_atomic.c   | 13 +++-
> >  .../gpu/drm/i915/display/intel_connector.c| 35 +
> >  .../gpu/drm/i915/display/intel_connector.h|  1 +
> >  .../drm/i915/display/intel_display_types.h| 18 +
> >  drivers/gpu/drm/i915/display/intel_dp.c   |  6 ++
> >  .../drm/i915/display/intel_privacy_screen.c   | 72 +++
> >  .../drm/i915/display/intel_privacy_screen.h   | 27 +++
> >  8 files changed, 171 insertions(+), 4 deletions(-)
> >  create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.c
> >  create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.h
> >
> > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> > index 991a8c537d123..825951b4cd006 100644
> > --- a/drivers/gpu/drm/i915/Makefile
> > +++ b/drivers/gpu/drm/i915/Makefile
> > @@ -208,7 +208,8 @@ i915-y += \
> >   display/intel_vga.o
> >  i915-$(CONFIG_ACPI) += \
> >   display/intel_acpi.o \
> > - display/intel_opregion.o
> > + display/intel_opregion.o \
> > + display/intel_privacy_screen.o
> >  i915-$(CONFIG_DRM_FBDEV_EMULATION) += \
> >   display/intel_fbdev.o
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c 
> > b/drivers/gpu/drm/i915/display/intel_atomic.c
> > index d043057d2fa03..4ed537c87 100644
> > --- a/drivers/gpu/drm/i915/display/intel_atomic.c
> > +++ b/drivers/gpu/drm/i915/display/intel_atomic.c
> > @@ -40,6 +40,7 @@
> >  #include "intel_global_state.h"
> >  #include "intel_hdcp.h"
> >  #include "intel_psr.h"
> > +#include "intel_privacy_screen.h"
> >  #include "intel_sprite.h"
> >
> >  /**
> > @@ -60,11 +61,14 @@ int intel_digital_connector_atomic_get_property(struct 
> > drm_connector *connector,
> >   struct drm_i915_private *dev_priv = to_i915(dev);
> >   struct intel_digital_connector_state *intel_conn_state =
> >   to_intel_digital_connector_state(state);
> > + struct intel_connector *intel_connector = 
> > to_intel_connector(connector);
> >
> >   if (property == dev_priv->force_audio_property)
> >   *val = intel_conn_state->force_audio;
> >   else if (property == dev_priv->broadcast_rgb_property)
> >   *val = intel_conn_state->broadcast_rgb;
> > + else if (property == intel_connector->privacy_screen_property)
> > + *val = intel_conn_state->privacy_screen_status;
&

[Intel-gfx] [PATCH v6 3/3] drm/i915: Add support for integrated privacy screens

2020-03-04 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 intel_connector for the panel, that
the userspace can then use to control and check the status.

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?).

Signed-off-by: Rajat Jain 
---
v6: Always initialize prop in intel_attach_privacy_screen_property()
v5: fix a cosmetic checkpatch warning
v4: Fix a typo in intel_privacy_screen.h
v3: * Change license to GPL-2.0 OR MIT
* Move privacy screen enum from UAPI to intel_display_types.h
* Rename parameter name and some other minor changes.
v2: Formed by splitting the original patch into multiple patches.
- All code has been moved into i915 now.
- Privacy screen is a i915 property
- Have a local state variable to store the prvacy screen. Don't read
  it from hardware.

 drivers/gpu/drm/i915/Makefile |  3 +-
 drivers/gpu/drm/i915/display/intel_atomic.c   | 13 +++-
 .../gpu/drm/i915/display/intel_connector.c| 35 +
 .../gpu/drm/i915/display/intel_connector.h|  1 +
 .../drm/i915/display/intel_display_types.h| 18 +
 drivers/gpu/drm/i915/display/intel_dp.c   |  6 ++
 .../drm/i915/display/intel_privacy_screen.c   | 72 +++
 .../drm/i915/display/intel_privacy_screen.h   | 27 +++
 8 files changed, 171 insertions(+), 4 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.c
 create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 991a8c537d123..825951b4cd006 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -208,7 +208,8 @@ i915-y += \
display/intel_vga.o
 i915-$(CONFIG_ACPI) += \
display/intel_acpi.o \
-   display/intel_opregion.o
+   display/intel_opregion.o \
+   display/intel_privacy_screen.o
 i915-$(CONFIG_DRM_FBDEV_EMULATION) += \
display/intel_fbdev.o
 
diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c 
b/drivers/gpu/drm/i915/display/intel_atomic.c
index d043057d2fa03..4ed537c87 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic.c
@@ -40,6 +40,7 @@
 #include "intel_global_state.h"
 #include "intel_hdcp.h"
 #include "intel_psr.h"
+#include "intel_privacy_screen.h"
 #include "intel_sprite.h"
 
 /**
@@ -60,11 +61,14 @@ int intel_digital_connector_atomic_get_property(struct 
drm_connector *connector,
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_digital_connector_state *intel_conn_state =
to_intel_digital_connector_state(state);
+   struct intel_connector *intel_connector = to_intel_connector(connector);
 
if (property == dev_priv->force_audio_property)
*val = intel_conn_state->force_audio;
else if (property == dev_priv->broadcast_rgb_property)
*val = intel_conn_state->broadcast_rgb;
+   else if (property == intel_connector->privacy_screen_property)
+   *val = intel_conn_state->privacy_screen_status;
else {
drm_dbg_atomic(&dev_priv->drm,
   "Unknown property [PROP:%d:%s]\n",
@@ -93,15 +97,18 @@ int intel_digital_connector_atomic_set_property(struct 
drm_connector *connector,
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_digital_connector_state *intel_conn_state =
to_intel_digital_connector_state(state);
+   struct intel_connector *intel_connector = to_intel_connector(connector);
 
if (property == dev_priv->force_audio_property) {
intel_conn_state->force_audio = val;
return 0;
-   }
-
-   if (property == dev_priv->broadcast_rgb_property) {
+   } else if (property == dev_priv->broadcast_rgb_property) {
intel_conn_state->broadcast_rgb = val;
return 0;
+   } else if (property == intel_connector->privacy_screen_property) {
+   intel_privacy_screen_set_val(intel_connector, val);
+   intel_conn_state->privacy_screen_status = val;
+   return 0;
}
 
drm_dbg_atomic(&dev_priv->drm, "Unknown property [PROP:%d:%s]\n",
diff --git a/drivers/gpu/drm/i915/display/intel_connector.c 
b/drivers/gpu/drm/i915/display/intel_connector.c
index 903e49659f561..55f80219cb269 100644
--- a/drivers/gpu/drm/i915/display/intel_connector.c
+++ b/drive

[Intel-gfx] [PATCH v6 3/3] drm/i915: Add support for integrated privacy screens

2020-03-04 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 intel_connector for the panel, that
the userspace can then use to control and check the status.

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?).

Signed-off-by: Rajat Jain 
---
v6: Always initialize prop in intel_attach_privacy_screen_property()
v5: fix a cosmetic checkpatch warning
v4: Fix a typo in intel_privacy_screen.h
v3: * Change license to GPL-2.0 OR MIT
* Move privacy screen enum from UAPI to intel_display_types.h
* Rename parameter name and some other minor changes.
v2: Formed by splitting the original patch into multiple patches.
- All code has been moved into i915 now.
- Privacy screen is a i915 property
- Have a local state variable to store the prvacy screen. Don't read
  it from hardware.

 drivers/gpu/drm/i915/Makefile |  3 +-
 drivers/gpu/drm/i915/display/intel_atomic.c   | 13 +++-
 .../gpu/drm/i915/display/intel_connector.c| 35 +
 .../gpu/drm/i915/display/intel_connector.h|  1 +
 .../drm/i915/display/intel_display_types.h| 18 +
 drivers/gpu/drm/i915/display/intel_dp.c   |  6 ++
 .../drm/i915/display/intel_privacy_screen.c   | 72 +++
 .../drm/i915/display/intel_privacy_screen.h   | 27 +++
 8 files changed, 171 insertions(+), 4 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.c
 create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 991a8c537d123..825951b4cd006 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -208,7 +208,8 @@ i915-y += \
display/intel_vga.o
 i915-$(CONFIG_ACPI) += \
display/intel_acpi.o \
-   display/intel_opregion.o
+   display/intel_opregion.o \
+   display/intel_privacy_screen.o
 i915-$(CONFIG_DRM_FBDEV_EMULATION) += \
display/intel_fbdev.o
 
diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c 
b/drivers/gpu/drm/i915/display/intel_atomic.c
index d043057d2fa03..4ed537c87 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic.c
@@ -40,6 +40,7 @@
 #include "intel_global_state.h"
 #include "intel_hdcp.h"
 #include "intel_psr.h"
+#include "intel_privacy_screen.h"
 #include "intel_sprite.h"
 
 /**
@@ -60,11 +61,14 @@ int intel_digital_connector_atomic_get_property(struct 
drm_connector *connector,
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_digital_connector_state *intel_conn_state =
to_intel_digital_connector_state(state);
+   struct intel_connector *intel_connector = to_intel_connector(connector);
 
if (property == dev_priv->force_audio_property)
*val = intel_conn_state->force_audio;
else if (property == dev_priv->broadcast_rgb_property)
*val = intel_conn_state->broadcast_rgb;
+   else if (property == intel_connector->privacy_screen_property)
+   *val = intel_conn_state->privacy_screen_status;
else {
drm_dbg_atomic(&dev_priv->drm,
   "Unknown property [PROP:%d:%s]\n",
@@ -93,15 +97,18 @@ int intel_digital_connector_atomic_set_property(struct 
drm_connector *connector,
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_digital_connector_state *intel_conn_state =
to_intel_digital_connector_state(state);
+   struct intel_connector *intel_connector = to_intel_connector(connector);
 
if (property == dev_priv->force_audio_property) {
intel_conn_state->force_audio = val;
return 0;
-   }
-
-   if (property == dev_priv->broadcast_rgb_property) {
+   } else if (property == dev_priv->broadcast_rgb_property) {
intel_conn_state->broadcast_rgb = val;
return 0;
+   } else if (property == intel_connector->privacy_screen_property) {
+   intel_privacy_screen_set_val(intel_connector, val);
+   intel_conn_state->privacy_screen_status = val;
+   return 0;
}
 
drm_dbg_atomic(&dev_priv->drm, "Unknown property [PROP:%d:%s]\n",
diff --git a/drivers/gpu/drm/i915/display/intel_connector.c 
b/drivers/gpu/drm/i915/display/intel_connector.c
index 903e49659f561..55f80219cb269 100644
--- a/drivers/gpu/drm/i915/display/intel_connector.c
+++ b/drive

Re: [Intel-gfx] [v5, 3/3] drm/i915: Add support for integrated privacy screens

2020-01-27 Thread Rajat Jain
On Fri, Jan 24, 2020 at 4:11 PM Guenter Roeck  wrote:
>
> On Fri, Dec 20, 2019 at 12:03:53PM -0800, 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 intel_connector for the panel, that
> > the userspace can then use to control and check the status.
> >
> > 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?).
> >
> > Signed-off-by: Rajat Jain 
> > ---
> > v5: fix a cosmetic checkpatch warning
> > v4: Fix a typo in intel_privacy_screen.h
> > v3: * Change license to GPL-2.0 OR MIT
> > * Move privacy screen enum from UAPI to intel_display_types.h
> > * Rename parameter name and some other minor changes.
> > v2: Formed by splitting the original patch into multiple patches.
> > - All code has been moved into i915 now.
> > - Privacy screen is a i915 property
> > - Have a local state variable to store the prvacy screen. Don't read
> >   it from hardware.
> >
> >  drivers/gpu/drm/i915/Makefile |  3 +-
> >  drivers/gpu/drm/i915/display/intel_atomic.c   | 13 +++-
> >  .../gpu/drm/i915/display/intel_connector.c| 35 +
> >  .../gpu/drm/i915/display/intel_connector.h|  1 +
> >  .../drm/i915/display/intel_display_types.h| 18 +
> >  drivers/gpu/drm/i915/display/intel_dp.c   |  6 ++
> >  .../drm/i915/display/intel_privacy_screen.c   | 72 +++
> >  .../drm/i915/display/intel_privacy_screen.h   | 27 +++
> >  8 files changed, 171 insertions(+), 4 deletions(-)
> >  create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.c
> >  create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.h
> >
> > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> > index 90dcf09f52cc..f7067c8f0407 100644
> > --- a/drivers/gpu/drm/i915/Makefile
> > +++ b/drivers/gpu/drm/i915/Makefile
> > @@ -197,7 +197,8 @@ i915-y += \
> >   display/intel_vga.o
> >  i915-$(CONFIG_ACPI) += \
> >   display/intel_acpi.o \
> > - display/intel_opregion.o
> > + display/intel_opregion.o \
> > + display/intel_privacy_screen.o
> >  i915-$(CONFIG_DRM_FBDEV_EMULATION) += \
> >   display/intel_fbdev.o
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c 
> > b/drivers/gpu/drm/i915/display/intel_atomic.c
> > index c2875b10adf9..c73b81c4c3f6 100644
> > --- a/drivers/gpu/drm/i915/display/intel_atomic.c
> > +++ b/drivers/gpu/drm/i915/display/intel_atomic.c
> > @@ -37,6 +37,7 @@
> >  #include "intel_atomic.h"
> >  #include "intel_display_types.h"
> >  #include "intel_hdcp.h"
> > +#include "intel_privacy_screen.h"
> >  #include "intel_sprite.h"
> >
> >  /**
> > @@ -57,11 +58,14 @@ int intel_digital_connector_atomic_get_property(struct 
> > drm_connector *connector,
> >   struct drm_i915_private *dev_priv = to_i915(dev);
> >   struct intel_digital_connector_state *intel_conn_state =
> >   to_intel_digital_connector_state(state);
> > + struct intel_connector *intel_connector = 
> > to_intel_connector(connector);
> >
> >   if (property == dev_priv->force_audio_property)
> >   *val = intel_conn_state->force_audio;
> >   else if (property == dev_priv->broadcast_rgb_property)
> >   *val = intel_conn_state->broadcast_rgb;
> > + else if (property == intel_connector->privacy_screen_property)
> > + *val = intel_conn_state->privacy_screen_status;
> >   else {
> >   DRM_DEBUG_ATOMIC("Unknown property [PROP:%d:%s]\n",
> >property->base.id, property->name);
> > @@ -89,15 +93,18 @@ int intel_digital_connector_atomic_set_property(struct 
> > drm_connector *connector,
> >   struct drm_i915_private *dev_priv = to_i915(dev);
> >   struct intel_digital_connector_state *intel_conn_state =
> >   to_intel_digital_connector_state(state);
> > + struct intel_connector *intel_connector = 
> > to_intel_connector(connector);
> >
> >   if (property == dev_pri

Re: [Intel-gfx] [PATCH v5 3/3] drm/i915: Add support for integrated privacy screens

2020-01-20 Thread Rajat Jain
Hello Jani,

On Fri, Dec 20, 2019 at 12:04 PM 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 intel_connector for the panel, that
> the userspace can then use to control and check the status.
>
> 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?).
>
> Signed-off-by: Rajat Jain 

I was wondering if you got a chance to look at this patchset. I'd
appreciate it if you could please take a look and provide your
comments.

Thanks & Best Regards,

Rajat Jain


> ---
> v5: fix a cosmetic checkpatch warning
> v4: Fix a typo in intel_privacy_screen.h
> v3: * Change license to GPL-2.0 OR MIT
> * Move privacy screen enum from UAPI to intel_display_types.h
> * Rename parameter name and some other minor changes.
> v2: Formed by splitting the original patch into multiple patches.
> - All code has been moved into i915 now.
> - Privacy screen is a i915 property
> - Have a local state variable to store the prvacy screen. Don't read
>   it from hardware.
>
>  drivers/gpu/drm/i915/Makefile |  3 +-
>  drivers/gpu/drm/i915/display/intel_atomic.c   | 13 +++-
>  .../gpu/drm/i915/display/intel_connector.c| 35 +
>  .../gpu/drm/i915/display/intel_connector.h|  1 +
>  .../drm/i915/display/intel_display_types.h| 18 +
>  drivers/gpu/drm/i915/display/intel_dp.c   |  6 ++
>  .../drm/i915/display/intel_privacy_screen.c   | 72 +++
>  .../drm/i915/display/intel_privacy_screen.h   | 27 +++
>  8 files changed, 171 insertions(+), 4 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.c
>  create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.h
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 90dcf09f52cc..f7067c8f0407 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -197,7 +197,8 @@ i915-y += \
> display/intel_vga.o
>  i915-$(CONFIG_ACPI) += \
> display/intel_acpi.o \
> -   display/intel_opregion.o
> +   display/intel_opregion.o \
> +   display/intel_privacy_screen.o
>  i915-$(CONFIG_DRM_FBDEV_EMULATION) += \
> display/intel_fbdev.o
>
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c 
> b/drivers/gpu/drm/i915/display/intel_atomic.c
> index c2875b10adf9..c73b81c4c3f6 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic.c
> @@ -37,6 +37,7 @@
>  #include "intel_atomic.h"
>  #include "intel_display_types.h"
>  #include "intel_hdcp.h"
> +#include "intel_privacy_screen.h"
>  #include "intel_sprite.h"
>
>  /**
> @@ -57,11 +58,14 @@ int intel_digital_connector_atomic_get_property(struct 
> drm_connector *connector,
> struct drm_i915_private *dev_priv = to_i915(dev);
> struct intel_digital_connector_state *intel_conn_state =
> to_intel_digital_connector_state(state);
> +   struct intel_connector *intel_connector = 
> to_intel_connector(connector);
>
> if (property == dev_priv->force_audio_property)
> *val = intel_conn_state->force_audio;
> else if (property == dev_priv->broadcast_rgb_property)
> *val = intel_conn_state->broadcast_rgb;
> +   else if (property == intel_connector->privacy_screen_property)
> +   *val = intel_conn_state->privacy_screen_status;
> else {
> DRM_DEBUG_ATOMIC("Unknown property [PROP:%d:%s]\n",
>  property->base.id, property->name);
> @@ -89,15 +93,18 @@ int intel_digital_connector_atomic_set_property(struct 
> drm_connector *connector,
> struct drm_i915_private *dev_priv = to_i915(dev);
> struct intel_digital_connector_state *intel_conn_state =
> to_intel_digital_connector_state(state);
> +   struct intel_connector *intel_connector = 
> to_intel_connector(connector);
>
> if (property == dev_priv->force_audio_property) {
> intel_conn_state->force_audio = val;
> return 0;
> -   }
> -
> -   if (property == dev_priv->broadcast_rgb_property) {
> +   } else if (property == dev_priv->broadcas

Re: [Intel-gfx] [PATCH v2 3/3] drm/i915: Add support for integrated privacy screens

2019-12-20 Thread Rajat Jain
HI Jani,


On Thu, Dec 5, 2019 at 1:34 AM Rajat Jain  wrote:
>
> On Wed, Nov 20, 2019 at 7:04 AM Jani Nikula  
> wrote:
> >
> > On Mon, 04 Nov 2019, 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 intel_connector for the panel, that
> > > the userspace can then use to control and check the status.
> > >
> > > 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?).
> > >
> > > Signed-off-by: Rajat Jain 
> > > Change-Id: Ic9ff07fc4a50797d2d0dfb919f11aa0821a4b548
> > > ---
> > > v2: Formed by splitting the original patch into multiple patches.
> > > - All code has been moved into i915 now.
> > > - Privacy screen is a i915 property
> > > - Have a local state variable to store the prvacy screen. Don't read
> > >   it from hardware.
> > >
> > >  drivers/gpu/drm/i915/Makefile |  3 +-
> > >  drivers/gpu/drm/i915/display/intel_atomic.c   | 13 +++-
> > >  .../gpu/drm/i915/display/intel_connector.c| 35 ++
> > >  .../gpu/drm/i915/display/intel_connector.h|  1 +
> > >  .../drm/i915/display/intel_display_types.h|  4 ++
> > >  drivers/gpu/drm/i915/display/intel_dp.c   |  5 ++
> > >  .../drm/i915/display/intel_privacy_screen.c   | 70 +++
> > >  .../drm/i915/display/intel_privacy_screen.h   | 25 +++
> > >  include/uapi/drm/i915_drm.h   | 14 
> > >  9 files changed, 166 insertions(+), 4 deletions(-)
> > >  create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.c
> > >  create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.h
> > >
> > > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> > > index 2587ea834f06..3589ebcf27bc 100644
> > > --- a/drivers/gpu/drm/i915/Makefile
> > > +++ b/drivers/gpu/drm/i915/Makefile
> > > @@ -185,7 +185,8 @@ i915-y += \
> > >   display/intel_tc.o
> > >  i915-$(CONFIG_ACPI) += \
> > >   display/intel_acpi.o \
> > > - display/intel_opregion.o
> > > + display/intel_opregion.o \
> > > + display/intel_privacy_screen.o
> >
> > Mmmh, wonder if there'll be non-ACPI based privacy screens. I guess we
> > can sort this out then. *shrug*
> >
> > >  i915-$(CONFIG_DRM_FBDEV_EMULATION) += \
> > >   display/intel_fbdev.o
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c 
> > > b/drivers/gpu/drm/i915/display/intel_atomic.c
> > > index d3fb75bb9eb1..378772d3449c 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_atomic.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_atomic.c
> > > @@ -37,6 +37,7 @@
> > >  #include "intel_atomic.h"
> > >  #include "intel_display_types.h"
> > >  #include "intel_hdcp.h"
> > > +#include "intel_privacy_screen.h"
> > >  #include "intel_sprite.h"
> > >
> > >  /**
> > > @@ -57,11 +58,14 @@ int 
> > > intel_digital_connector_atomic_get_property(struct drm_connector 
> > > *connector,
> > >   struct drm_i915_private *dev_priv = to_i915(dev);
> > >   struct intel_digital_connector_state *intel_conn_state =
> > >   to_intel_digital_connector_state(state);
> > > + struct intel_connector *intel_connector = 
> > > to_intel_connector(connector);
> > >
> > >   if (property == dev_priv->force_audio_property)
> > >   *val = intel_conn_state->force_audio;
> > >   else if (property == dev_priv->broadcast_rgb_property)
> > >   *val = intel_conn_state->broadcast_rgb;
> > > + else if (property == intel_connector->privacy_screen_property)
> > > + *val = intel_conn_state->privacy_screen_status;
> > >   else {
> > >   DRM_DEBUG_ATOMIC("Unknown property [PROP:%d:%s]\n",
> > >property->base.id, property->name);
> &

[Intel-gfx] [PATCH v5 3/3] drm/i915: Add support for integrated privacy screens

2019-12-20 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 intel_connector for the panel, that
the userspace can then use to control and check the status.

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?).

Signed-off-by: Rajat Jain 
---
v5: fix a cosmetic checkpatch warning
v4: Fix a typo in intel_privacy_screen.h
v3: * Change license to GPL-2.0 OR MIT
* Move privacy screen enum from UAPI to intel_display_types.h
* Rename parameter name and some other minor changes.
v2: Formed by splitting the original patch into multiple patches.
- All code has been moved into i915 now.
- Privacy screen is a i915 property
- Have a local state variable to store the prvacy screen. Don't read
  it from hardware.

 drivers/gpu/drm/i915/Makefile |  3 +-
 drivers/gpu/drm/i915/display/intel_atomic.c   | 13 +++-
 .../gpu/drm/i915/display/intel_connector.c| 35 +
 .../gpu/drm/i915/display/intel_connector.h|  1 +
 .../drm/i915/display/intel_display_types.h| 18 +
 drivers/gpu/drm/i915/display/intel_dp.c   |  6 ++
 .../drm/i915/display/intel_privacy_screen.c   | 72 +++
 .../drm/i915/display/intel_privacy_screen.h   | 27 +++
 8 files changed, 171 insertions(+), 4 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.c
 create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 90dcf09f52cc..f7067c8f0407 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -197,7 +197,8 @@ i915-y += \
display/intel_vga.o
 i915-$(CONFIG_ACPI) += \
display/intel_acpi.o \
-   display/intel_opregion.o
+   display/intel_opregion.o \
+   display/intel_privacy_screen.o
 i915-$(CONFIG_DRM_FBDEV_EMULATION) += \
display/intel_fbdev.o
 
diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c 
b/drivers/gpu/drm/i915/display/intel_atomic.c
index c2875b10adf9..c73b81c4c3f6 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic.c
@@ -37,6 +37,7 @@
 #include "intel_atomic.h"
 #include "intel_display_types.h"
 #include "intel_hdcp.h"
+#include "intel_privacy_screen.h"
 #include "intel_sprite.h"
 
 /**
@@ -57,11 +58,14 @@ int intel_digital_connector_atomic_get_property(struct 
drm_connector *connector,
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_digital_connector_state *intel_conn_state =
to_intel_digital_connector_state(state);
+   struct intel_connector *intel_connector = to_intel_connector(connector);
 
if (property == dev_priv->force_audio_property)
*val = intel_conn_state->force_audio;
else if (property == dev_priv->broadcast_rgb_property)
*val = intel_conn_state->broadcast_rgb;
+   else if (property == intel_connector->privacy_screen_property)
+   *val = intel_conn_state->privacy_screen_status;
else {
DRM_DEBUG_ATOMIC("Unknown property [PROP:%d:%s]\n",
 property->base.id, property->name);
@@ -89,15 +93,18 @@ int intel_digital_connector_atomic_set_property(struct 
drm_connector *connector,
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_digital_connector_state *intel_conn_state =
to_intel_digital_connector_state(state);
+   struct intel_connector *intel_connector = to_intel_connector(connector);
 
if (property == dev_priv->force_audio_property) {
intel_conn_state->force_audio = val;
return 0;
-   }
-
-   if (property == dev_priv->broadcast_rgb_property) {
+   } else if (property == dev_priv->broadcast_rgb_property) {
intel_conn_state->broadcast_rgb = val;
return 0;
+   } else if (property == intel_connector->privacy_screen_property) {
+   intel_privacy_screen_set_val(intel_connector, val);
+   intel_conn_state->privacy_screen_status = val;
+   return 0;
}
 
DRM_DEBUG_ATOMIC("Unknown property [PROP:%d:%s]\n",
diff --git a/drivers/gpu/drm/i915/display/intel_connector.c 
b/drivers/gpu/drm/i915/display/intel_connector.c
index 1133c4e97bb4..f3e041c737de 100644
--- a/drivers/gpu/drm/i915/display/intel_connector.c
+++ b/drivers/gpu/drm/i915/display/intel_connector.c
@@ -296,3 +296,38 @

[Intel-gfx] [PATCH v4 1/3] drm/i915: Move the code to populate ACPI device ID into intel_acpi

2019-12-18 Thread Rajat Jain
Move the code that populates the ACPI device ID for devices, into
more appripriate intel_acpi.c. This is done in preparation for more
users of this code (in next patch).

Signed-off-by: Rajat Jain 
---
v4: Same as v3
v3: * Renamed the function to intel_acpi_*
* Used forward declaration for structure instead of header file inclusion.
* Fix a typo
v2: v1 doesn't exist. Found existing code in i915 driver to assign the ACPI ID
which is what I plan to re-use.

 drivers/gpu/drm/i915/display/intel_acpi.c | 89 +++
 drivers/gpu/drm/i915/display/intel_acpi.h |  5 ++
 drivers/gpu/drm/i915/display/intel_opregion.c | 80 +
 3 files changed, 98 insertions(+), 76 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_acpi.c 
b/drivers/gpu/drm/i915/display/intel_acpi.c
index 3456d33feb46..e21fb14d5e07 100644
--- a/drivers/gpu/drm/i915/display/intel_acpi.c
+++ b/drivers/gpu/drm/i915/display/intel_acpi.c
@@ -10,6 +10,7 @@
 
 #include "i915_drv.h"
 #include "intel_acpi.h"
+#include "intel_display_types.h"
 
 #define INTEL_DSM_REVISION_ID 1 /* For Calpella anyway... */
 #define INTEL_DSM_FN_PLATFORM_MUX_INFO 1 /* No args */
@@ -156,3 +157,91 @@ void intel_register_dsm_handler(void)
 void intel_unregister_dsm_handler(void)
 {
 }
+
+/*
+ * ACPI Specification, Revision 5.0, Appendix B.3.2 _DOD (Enumerate All Devices
+ * Attached to the Display Adapter).
+ */
+#define ACPI_DISPLAY_INDEX_SHIFT   0
+#define ACPI_DISPLAY_INDEX_MASK(0xf << 0)
+#define ACPI_DISPLAY_PORT_ATTACHMENT_SHIFT 4
+#define ACPI_DISPLAY_PORT_ATTACHMENT_MASK  (0xf << 4)
+#define ACPI_DISPLAY_TYPE_SHIFT8
+#define ACPI_DISPLAY_TYPE_MASK (0xf << 8)
+#define ACPI_DISPLAY_TYPE_OTHER(0 << 8)
+#define ACPI_DISPLAY_TYPE_VGA  (1 << 8)
+#define ACPI_DISPLAY_TYPE_TV   (2 << 8)
+#define ACPI_DISPLAY_TYPE_EXTERNAL_DIGITAL (3 << 8)
+#define ACPI_DISPLAY_TYPE_INTERNAL_DIGITAL (4 << 8)
+#define ACPI_VENDOR_SPECIFIC_SHIFT 12
+#define ACPI_VENDOR_SPECIFIC_MASK  (0xf << 12)
+#define ACPI_BIOS_CAN_DETECT   (1 << 16)
+#define ACPI_DEPENDS_ON_VGA(1 << 17)
+#define ACPI_PIPE_ID_SHIFT 18
+#define ACPI_PIPE_ID_MASK  (7 << 18)
+#define ACPI_DEVICE_ID_SCHEME  (1ULL << 31)
+
+static u32 acpi_display_type(struct intel_connector *connector)
+{
+   u32 display_type;
+
+   switch (connector->base.connector_type) {
+   case DRM_MODE_CONNECTOR_VGA:
+   case DRM_MODE_CONNECTOR_DVIA:
+   display_type = ACPI_DISPLAY_TYPE_VGA;
+   break;
+   case DRM_MODE_CONNECTOR_Composite:
+   case DRM_MODE_CONNECTOR_SVIDEO:
+   case DRM_MODE_CONNECTOR_Component:
+   case DRM_MODE_CONNECTOR_9PinDIN:
+   case DRM_MODE_CONNECTOR_TV:
+   display_type = ACPI_DISPLAY_TYPE_TV;
+   break;
+   case DRM_MODE_CONNECTOR_DVII:
+   case DRM_MODE_CONNECTOR_DVID:
+   case DRM_MODE_CONNECTOR_DisplayPort:
+   case DRM_MODE_CONNECTOR_HDMIA:
+   case DRM_MODE_CONNECTOR_HDMIB:
+   display_type = ACPI_DISPLAY_TYPE_EXTERNAL_DIGITAL;
+   break;
+   case DRM_MODE_CONNECTOR_LVDS:
+   case DRM_MODE_CONNECTOR_eDP:
+   case DRM_MODE_CONNECTOR_DSI:
+   display_type = ACPI_DISPLAY_TYPE_INTERNAL_DIGITAL;
+   break;
+   case DRM_MODE_CONNECTOR_Unknown:
+   case DRM_MODE_CONNECTOR_VIRTUAL:
+   display_type = ACPI_DISPLAY_TYPE_OTHER;
+   break;
+   default:
+   MISSING_CASE(connector->base.connector_type);
+   display_type = ACPI_DISPLAY_TYPE_OTHER;
+   break;
+   }
+
+   return display_type;
+}
+
+void intel_acpi_device_id_update(struct drm_i915_private *dev_priv)
+{
+   struct drm_device *drm_dev = &dev_priv->drm;
+   struct intel_connector *connector;
+   struct drm_connector_list_iter conn_iter;
+   u8 display_index[16] = {};
+
+   /* Populate the ACPI IDs for all connectors for a given drm_device */
+   drm_connector_list_iter_begin(drm_dev, &conn_iter);
+   for_each_intel_connector_iter(connector, &conn_iter) {
+   u32 device_id, type;
+
+   device_id = acpi_display_type(connector);
+
+   /* Use display type specific display index. */
+   type = (device_id & ACPI_DISPLAY_TYPE_MASK)
+   >> ACPI_DISPLAY_TYPE_SHIFT;
+   device_id |= display_index[type]++ << ACPI_DISPLAY_INDEX_SHIFT;
+
+   connector->acpi_device_id = device_id;
+   }
+   drm_connector_list_iter_end(&conn_iter);
+}
diff --git a/driv

[Intel-gfx] [PATCH v4 3/3] drm/i915: Add support for integrated privacy screens

2019-12-18 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 intel_connector for the panel, that
the userspace can then use to control and check the status.

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?).

Signed-off-by: Rajat Jain 
---
v4: Fix a typo in intel_privacy_screen.h
v3: * Change license to GPL-2.0 OR MIT
* Move privacy screen enum from UAPI to intel_display_types.h
* Rename parameter name and some other minor changes.
v2: Formed by splitting the original patch into multiple patches.
- All code has been moved into i915 now.
- Privacy screen is a i915 property
- Have a local state variable to store the prvacy screen. Don't read
  it from hardware.

 drivers/gpu/drm/i915/Makefile |  3 +-
 drivers/gpu/drm/i915/display/intel_atomic.c   | 13 +++-
 .../gpu/drm/i915/display/intel_connector.c| 35 +
 .../gpu/drm/i915/display/intel_connector.h|  1 +
 .../drm/i915/display/intel_display_types.h| 18 +
 drivers/gpu/drm/i915/display/intel_dp.c   |  6 ++
 .../drm/i915/display/intel_privacy_screen.c   | 72 +++
 .../drm/i915/display/intel_privacy_screen.h   | 26 +++
 8 files changed, 170 insertions(+), 4 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.c
 create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 90dcf09f52cc..f7067c8f0407 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -197,7 +197,8 @@ i915-y += \
display/intel_vga.o
 i915-$(CONFIG_ACPI) += \
display/intel_acpi.o \
-   display/intel_opregion.o
+   display/intel_opregion.o \
+   display/intel_privacy_screen.o
 i915-$(CONFIG_DRM_FBDEV_EMULATION) += \
display/intel_fbdev.o
 
diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c 
b/drivers/gpu/drm/i915/display/intel_atomic.c
index c2875b10adf9..c73b81c4c3f6 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic.c
@@ -37,6 +37,7 @@
 #include "intel_atomic.h"
 #include "intel_display_types.h"
 #include "intel_hdcp.h"
+#include "intel_privacy_screen.h"
 #include "intel_sprite.h"
 
 /**
@@ -57,11 +58,14 @@ int intel_digital_connector_atomic_get_property(struct 
drm_connector *connector,
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_digital_connector_state *intel_conn_state =
to_intel_digital_connector_state(state);
+   struct intel_connector *intel_connector = to_intel_connector(connector);
 
if (property == dev_priv->force_audio_property)
*val = intel_conn_state->force_audio;
else if (property == dev_priv->broadcast_rgb_property)
*val = intel_conn_state->broadcast_rgb;
+   else if (property == intel_connector->privacy_screen_property)
+   *val = intel_conn_state->privacy_screen_status;
else {
DRM_DEBUG_ATOMIC("Unknown property [PROP:%d:%s]\n",
 property->base.id, property->name);
@@ -89,15 +93,18 @@ int intel_digital_connector_atomic_set_property(struct 
drm_connector *connector,
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_digital_connector_state *intel_conn_state =
to_intel_digital_connector_state(state);
+   struct intel_connector *intel_connector = to_intel_connector(connector);
 
if (property == dev_priv->force_audio_property) {
intel_conn_state->force_audio = val;
return 0;
-   }
-
-   if (property == dev_priv->broadcast_rgb_property) {
+   } else if (property == dev_priv->broadcast_rgb_property) {
intel_conn_state->broadcast_rgb = val;
return 0;
+   } else if (property == intel_connector->privacy_screen_property) {
+   intel_privacy_screen_set_val(intel_connector, val);
+   intel_conn_state->privacy_screen_status = val;
+   return 0;
}
 
DRM_DEBUG_ATOMIC("Unknown property [PROP:%d:%s]\n",
diff --git a/drivers/gpu/drm/i915/display/intel_connector.c 
b/drivers/gpu/drm/i915/display/intel_connector.c
index 1133c4e97bb4..f3e041c737de 100644
--- a/drivers/gpu/drm/i915/display/intel_connector.c
+++ b/drivers/gpu/drm/i915/display/intel_connector.c
@@ -296,3 +296,38 @@ intel_attach_colorspace_property(struct drm_connector 
*connector)
 

[Intel-gfx] [PATCH RESEND v4 3/3] drm/i915: Add support for integrated privacy screens

2019-12-18 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 intel_connector for the panel, that
the userspace can then use to control and check the status.

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?).

Signed-off-by: Rajat Jain 
---
v4: Fix a typo in intel_privacy_screen.h
v3: * Change license to GPL-2.0 OR MIT
* Move privacy screen enum from UAPI to intel_display_types.h
* Rename parameter name and some other minor changes.
v2: Formed by splitting the original patch into multiple patches.
- All code has been moved into i915 now.
- Privacy screen is a i915 property
- Have a local state variable to store the prvacy screen. Don't read
  it from hardware.

 drivers/gpu/drm/i915/Makefile |  3 +-
 drivers/gpu/drm/i915/display/intel_atomic.c   | 13 +++-
 .../gpu/drm/i915/display/intel_connector.c| 35 +
 .../gpu/drm/i915/display/intel_connector.h|  1 +
 .../drm/i915/display/intel_display_types.h| 18 +
 drivers/gpu/drm/i915/display/intel_dp.c   |  6 ++
 .../drm/i915/display/intel_privacy_screen.c   | 72 +++
 .../drm/i915/display/intel_privacy_screen.h   | 26 +++
 8 files changed, 170 insertions(+), 4 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.c
 create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 90dcf09f52cc..f7067c8f0407 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -197,7 +197,8 @@ i915-y += \
display/intel_vga.o
 i915-$(CONFIG_ACPI) += \
display/intel_acpi.o \
-   display/intel_opregion.o
+   display/intel_opregion.o \
+   display/intel_privacy_screen.o
 i915-$(CONFIG_DRM_FBDEV_EMULATION) += \
display/intel_fbdev.o
 
diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c 
b/drivers/gpu/drm/i915/display/intel_atomic.c
index c2875b10adf9..c73b81c4c3f6 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic.c
@@ -37,6 +37,7 @@
 #include "intel_atomic.h"
 #include "intel_display_types.h"
 #include "intel_hdcp.h"
+#include "intel_privacy_screen.h"
 #include "intel_sprite.h"
 
 /**
@@ -57,11 +58,14 @@ int intel_digital_connector_atomic_get_property(struct 
drm_connector *connector,
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_digital_connector_state *intel_conn_state =
to_intel_digital_connector_state(state);
+   struct intel_connector *intel_connector = to_intel_connector(connector);
 
if (property == dev_priv->force_audio_property)
*val = intel_conn_state->force_audio;
else if (property == dev_priv->broadcast_rgb_property)
*val = intel_conn_state->broadcast_rgb;
+   else if (property == intel_connector->privacy_screen_property)
+   *val = intel_conn_state->privacy_screen_status;
else {
DRM_DEBUG_ATOMIC("Unknown property [PROP:%d:%s]\n",
 property->base.id, property->name);
@@ -89,15 +93,18 @@ int intel_digital_connector_atomic_set_property(struct 
drm_connector *connector,
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_digital_connector_state *intel_conn_state =
to_intel_digital_connector_state(state);
+   struct intel_connector *intel_connector = to_intel_connector(connector);
 
if (property == dev_priv->force_audio_property) {
intel_conn_state->force_audio = val;
return 0;
-   }
-
-   if (property == dev_priv->broadcast_rgb_property) {
+   } else if (property == dev_priv->broadcast_rgb_property) {
intel_conn_state->broadcast_rgb = val;
return 0;
+   } else if (property == intel_connector->privacy_screen_property) {
+   intel_privacy_screen_set_val(intel_connector, val);
+   intel_conn_state->privacy_screen_status = val;
+   return 0;
}
 
DRM_DEBUG_ATOMIC("Unknown property [PROP:%d:%s]\n",
diff --git a/drivers/gpu/drm/i915/display/intel_connector.c 
b/drivers/gpu/drm/i915/display/intel_connector.c
index 1133c4e97bb4..f3e041c737de 100644
--- a/drivers/gpu/drm/i915/display/intel_connector.c
+++ b/drivers/gpu/drm/i915/display/intel_connector.c
@@ -296,3 +296,38 @@ intel_attach_colorspace_property(struct drm_connector 
*connector)
 

Re: [Intel-gfx] [PATCH v3 3/3] drm/i915: Add support for integrated privacy screens

2019-12-16 Thread Rajat Jain
On Thu, Dec 5, 2019 at 1:34 AM 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 intel_connector for the panel, that
> the userspace can then use to control and check the status.
>
> 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?).
>
> Signed-off-by: Rajat Jain 

Hi Jani,

 Just checking to see if you got a chance to look at this...

Thanks,

Rajat

> ---
> v3: * Change license to GPL-2.0 OR MIT
> * Move privacy screen enum from UAPI to intel_display_types.h
> * Rename parameter name and some other minor changes.
> v2: Formed by splitting the original patch into multiple patches.
> - All code has been moved into i915 now.
> - Privacy screen is a i915 property
> - Have a local state variable to store the prvacy screen. Don't read
>   it from hardware.
>
>  drivers/gpu/drm/i915/Makefile |  3 +-
>  drivers/gpu/drm/i915/display/intel_atomic.c   | 13 +++-
>  .../gpu/drm/i915/display/intel_connector.c| 35 +
>  .../gpu/drm/i915/display/intel_connector.h|  1 +
>  .../drm/i915/display/intel_display_types.h| 18 +
>  drivers/gpu/drm/i915/display/intel_dp.c   |  6 ++
>  .../drm/i915/display/intel_privacy_screen.c   | 72 +++
>  .../drm/i915/display/intel_privacy_screen.h   | 25 +++
>  8 files changed, 169 insertions(+), 4 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.c
>  create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.h
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 90dcf09f52cc..f7067c8f0407 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -197,7 +197,8 @@ i915-y += \
> display/intel_vga.o
>  i915-$(CONFIG_ACPI) += \
> display/intel_acpi.o \
> -   display/intel_opregion.o
> +   display/intel_opregion.o \
> +   display/intel_privacy_screen.o
>  i915-$(CONFIG_DRM_FBDEV_EMULATION) += \
> display/intel_fbdev.o
>
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c 
> b/drivers/gpu/drm/i915/display/intel_atomic.c
> index c2875b10adf9..c73b81c4c3f6 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic.c
> @@ -37,6 +37,7 @@
>  #include "intel_atomic.h"
>  #include "intel_display_types.h"
>  #include "intel_hdcp.h"
> +#include "intel_privacy_screen.h"
>  #include "intel_sprite.h"
>
>  /**
> @@ -57,11 +58,14 @@ int intel_digital_connector_atomic_get_property(struct 
> drm_connector *connector,
> struct drm_i915_private *dev_priv = to_i915(dev);
> struct intel_digital_connector_state *intel_conn_state =
> to_intel_digital_connector_state(state);
> +   struct intel_connector *intel_connector = 
> to_intel_connector(connector);
>
> if (property == dev_priv->force_audio_property)
> *val = intel_conn_state->force_audio;
> else if (property == dev_priv->broadcast_rgb_property)
> *val = intel_conn_state->broadcast_rgb;
> +   else if (property == intel_connector->privacy_screen_property)
> +   *val = intel_conn_state->privacy_screen_status;
> else {
> DRM_DEBUG_ATOMIC("Unknown property [PROP:%d:%s]\n",
>  property->base.id, property->name);
> @@ -89,15 +93,18 @@ int intel_digital_connector_atomic_set_property(struct 
> drm_connector *connector,
> struct drm_i915_private *dev_priv = to_i915(dev);
> struct intel_digital_connector_state *intel_conn_state =
> to_intel_digital_connector_state(state);
> +   struct intel_connector *intel_connector = 
> to_intel_connector(connector);
>
> if (property == dev_priv->force_audio_property) {
> intel_conn_state->force_audio = val;
> return 0;
> -   }
> -
> -   if (property == dev_priv->broadcast_rgb_property) {
> +   } else if (property == dev_priv->broadcast_rgb_property) {
> intel_conn_state->broadcast_rgb = val;
> return 0;
> +   } else if (property == intel_connector->privacy_screen_property) {
> +   

[Intel-gfx] [PATCH v3 1/3] drm/i915: Move the code to populate ACPI device ID into intel_acpi

2019-12-05 Thread Rajat Jain
Move the code that populates the ACPI device ID for devices, into
more appripriate intel_acpi.c. This is done in preparation for more
users of this code (in next patch).

Signed-off-by: Rajat Jain 
---
v3: * Renamed the function to intel_acpi_*
* Used forward declaration for structure instead of header file inclusion.
* Fix a typo
v2: v1 doesn't exist. Found existing code in i915 driver to assign the ACPI ID
which is what I plan to re-use.

 drivers/gpu/drm/i915/display/intel_acpi.c | 89 +++
 drivers/gpu/drm/i915/display/intel_acpi.h |  5 ++
 drivers/gpu/drm/i915/display/intel_opregion.c | 80 +
 3 files changed, 98 insertions(+), 76 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_acpi.c 
b/drivers/gpu/drm/i915/display/intel_acpi.c
index 3456d33feb46..e21fb14d5e07 100644
--- a/drivers/gpu/drm/i915/display/intel_acpi.c
+++ b/drivers/gpu/drm/i915/display/intel_acpi.c
@@ -10,6 +10,7 @@
 
 #include "i915_drv.h"
 #include "intel_acpi.h"
+#include "intel_display_types.h"
 
 #define INTEL_DSM_REVISION_ID 1 /* For Calpella anyway... */
 #define INTEL_DSM_FN_PLATFORM_MUX_INFO 1 /* No args */
@@ -156,3 +157,91 @@ void intel_register_dsm_handler(void)
 void intel_unregister_dsm_handler(void)
 {
 }
+
+/*
+ * ACPI Specification, Revision 5.0, Appendix B.3.2 _DOD (Enumerate All Devices
+ * Attached to the Display Adapter).
+ */
+#define ACPI_DISPLAY_INDEX_SHIFT   0
+#define ACPI_DISPLAY_INDEX_MASK(0xf << 0)
+#define ACPI_DISPLAY_PORT_ATTACHMENT_SHIFT 4
+#define ACPI_DISPLAY_PORT_ATTACHMENT_MASK  (0xf << 4)
+#define ACPI_DISPLAY_TYPE_SHIFT8
+#define ACPI_DISPLAY_TYPE_MASK (0xf << 8)
+#define ACPI_DISPLAY_TYPE_OTHER(0 << 8)
+#define ACPI_DISPLAY_TYPE_VGA  (1 << 8)
+#define ACPI_DISPLAY_TYPE_TV   (2 << 8)
+#define ACPI_DISPLAY_TYPE_EXTERNAL_DIGITAL (3 << 8)
+#define ACPI_DISPLAY_TYPE_INTERNAL_DIGITAL (4 << 8)
+#define ACPI_VENDOR_SPECIFIC_SHIFT 12
+#define ACPI_VENDOR_SPECIFIC_MASK  (0xf << 12)
+#define ACPI_BIOS_CAN_DETECT   (1 << 16)
+#define ACPI_DEPENDS_ON_VGA(1 << 17)
+#define ACPI_PIPE_ID_SHIFT 18
+#define ACPI_PIPE_ID_MASK  (7 << 18)
+#define ACPI_DEVICE_ID_SCHEME  (1ULL << 31)
+
+static u32 acpi_display_type(struct intel_connector *connector)
+{
+   u32 display_type;
+
+   switch (connector->base.connector_type) {
+   case DRM_MODE_CONNECTOR_VGA:
+   case DRM_MODE_CONNECTOR_DVIA:
+   display_type = ACPI_DISPLAY_TYPE_VGA;
+   break;
+   case DRM_MODE_CONNECTOR_Composite:
+   case DRM_MODE_CONNECTOR_SVIDEO:
+   case DRM_MODE_CONNECTOR_Component:
+   case DRM_MODE_CONNECTOR_9PinDIN:
+   case DRM_MODE_CONNECTOR_TV:
+   display_type = ACPI_DISPLAY_TYPE_TV;
+   break;
+   case DRM_MODE_CONNECTOR_DVII:
+   case DRM_MODE_CONNECTOR_DVID:
+   case DRM_MODE_CONNECTOR_DisplayPort:
+   case DRM_MODE_CONNECTOR_HDMIA:
+   case DRM_MODE_CONNECTOR_HDMIB:
+   display_type = ACPI_DISPLAY_TYPE_EXTERNAL_DIGITAL;
+   break;
+   case DRM_MODE_CONNECTOR_LVDS:
+   case DRM_MODE_CONNECTOR_eDP:
+   case DRM_MODE_CONNECTOR_DSI:
+   display_type = ACPI_DISPLAY_TYPE_INTERNAL_DIGITAL;
+   break;
+   case DRM_MODE_CONNECTOR_Unknown:
+   case DRM_MODE_CONNECTOR_VIRTUAL:
+   display_type = ACPI_DISPLAY_TYPE_OTHER;
+   break;
+   default:
+   MISSING_CASE(connector->base.connector_type);
+   display_type = ACPI_DISPLAY_TYPE_OTHER;
+   break;
+   }
+
+   return display_type;
+}
+
+void intel_acpi_device_id_update(struct drm_i915_private *dev_priv)
+{
+   struct drm_device *drm_dev = &dev_priv->drm;
+   struct intel_connector *connector;
+   struct drm_connector_list_iter conn_iter;
+   u8 display_index[16] = {};
+
+   /* Populate the ACPI IDs for all connectors for a given drm_device */
+   drm_connector_list_iter_begin(drm_dev, &conn_iter);
+   for_each_intel_connector_iter(connector, &conn_iter) {
+   u32 device_id, type;
+
+   device_id = acpi_display_type(connector);
+
+   /* Use display type specific display index. */
+   type = (device_id & ACPI_DISPLAY_TYPE_MASK)
+   >> ACPI_DISPLAY_TYPE_SHIFT;
+   device_id |= display_index[type]++ << ACPI_DISPLAY_INDEX_SHIFT;
+
+   connector->acpi_device_id = device_id;
+   }
+   drm_connector_list_iter_end(&conn_iter);
+}
diff --git a/drivers/gpu/drm/i915/di

[Intel-gfx] [PATCH v3 3/3] drm/i915: Add support for integrated privacy screens

2019-12-05 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 intel_connector for the panel, that
the userspace can then use to control and check the status.

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?).

Signed-off-by: Rajat Jain 
---
v3: * Change license to GPL-2.0 OR MIT
* Move privacy screen enum from UAPI to intel_display_types.h
* Rename parameter name and some other minor changes.
v2: Formed by splitting the original patch into multiple patches.
- All code has been moved into i915 now.
- Privacy screen is a i915 property
- Have a local state variable to store the prvacy screen. Don't read
  it from hardware.

 drivers/gpu/drm/i915/Makefile |  3 +-
 drivers/gpu/drm/i915/display/intel_atomic.c   | 13 +++-
 .../gpu/drm/i915/display/intel_connector.c| 35 +
 .../gpu/drm/i915/display/intel_connector.h|  1 +
 .../drm/i915/display/intel_display_types.h| 18 +
 drivers/gpu/drm/i915/display/intel_dp.c   |  6 ++
 .../drm/i915/display/intel_privacy_screen.c   | 72 +++
 .../drm/i915/display/intel_privacy_screen.h   | 25 +++
 8 files changed, 169 insertions(+), 4 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.c
 create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 90dcf09f52cc..f7067c8f0407 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -197,7 +197,8 @@ i915-y += \
display/intel_vga.o
 i915-$(CONFIG_ACPI) += \
display/intel_acpi.o \
-   display/intel_opregion.o
+   display/intel_opregion.o \
+   display/intel_privacy_screen.o
 i915-$(CONFIG_DRM_FBDEV_EMULATION) += \
display/intel_fbdev.o
 
diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c 
b/drivers/gpu/drm/i915/display/intel_atomic.c
index c2875b10adf9..c73b81c4c3f6 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic.c
@@ -37,6 +37,7 @@
 #include "intel_atomic.h"
 #include "intel_display_types.h"
 #include "intel_hdcp.h"
+#include "intel_privacy_screen.h"
 #include "intel_sprite.h"
 
 /**
@@ -57,11 +58,14 @@ int intel_digital_connector_atomic_get_property(struct 
drm_connector *connector,
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_digital_connector_state *intel_conn_state =
to_intel_digital_connector_state(state);
+   struct intel_connector *intel_connector = to_intel_connector(connector);
 
if (property == dev_priv->force_audio_property)
*val = intel_conn_state->force_audio;
else if (property == dev_priv->broadcast_rgb_property)
*val = intel_conn_state->broadcast_rgb;
+   else if (property == intel_connector->privacy_screen_property)
+   *val = intel_conn_state->privacy_screen_status;
else {
DRM_DEBUG_ATOMIC("Unknown property [PROP:%d:%s]\n",
 property->base.id, property->name);
@@ -89,15 +93,18 @@ int intel_digital_connector_atomic_set_property(struct 
drm_connector *connector,
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_digital_connector_state *intel_conn_state =
to_intel_digital_connector_state(state);
+   struct intel_connector *intel_connector = to_intel_connector(connector);
 
if (property == dev_priv->force_audio_property) {
intel_conn_state->force_audio = val;
return 0;
-   }
-
-   if (property == dev_priv->broadcast_rgb_property) {
+   } else if (property == dev_priv->broadcast_rgb_property) {
intel_conn_state->broadcast_rgb = val;
return 0;
+   } else if (property == intel_connector->privacy_screen_property) {
+   intel_privacy_screen_set_val(intel_connector, val);
+   intel_conn_state->privacy_screen_status = val;
+   return 0;
}
 
DRM_DEBUG_ATOMIC("Unknown property [PROP:%d:%s]\n",
diff --git a/drivers/gpu/drm/i915/display/intel_connector.c 
b/drivers/gpu/drm/i915/display/intel_connector.c
index 1133c4e97bb4..f3e041c737de 100644
--- a/drivers/gpu/drm/i915/display/intel_connector.c
+++ b/drivers/gpu/drm/i915/display/intel_connector.c
@@ -296,3 +296,38 @@ intel_attach_colorspace_property(struct drm_connector 
*connector)
drm_object_attach_pro

Re: [Intel-gfx] [PATCH v2 1/3] drm/i915: Move the code to populate ACPI device ID into intel_acpi

2019-12-05 Thread Rajat Jain
Hi Jani,

Thanks for the review.

On Wed, Nov 20, 2019 at 6:41 AM Jani Nikula  wrote:
>
> On Mon, 04 Nov 2019, Rajat Jain  wrote:
> > Move the code that populates the ACPI device ID for devices, into
> > more appripriate intel_acpi.c. This is done in preparation for more
> > users of this code (in next patch).
>
> I don't think your use of the code makes sense (I'll explain in reply to
> the other patches)

OK, I'll discuss this there.

> but I could be persuaded to move the code to
> intel_acpi.c.
>
> > Signed-off-by: Rajat Jain 
> > Change-Id: Ifb3bd458734985c2a78ba682e6f0a2e63e0626ca
>
> Please drop Change-Ids.

Done.

>
> > ---
> > v2: v1 doesn't exist. Found existing code in i915 driver to assign the ACPI 
> > ID
> > which is what I plan to re-use.
> >
> >
> >  drivers/gpu/drm/i915/display/intel_acpi.c | 87 +++
> >  drivers/gpu/drm/i915/display/intel_acpi.h |  6 ++
> >  drivers/gpu/drm/i915/display/intel_opregion.c | 80 +
> >  3 files changed, 97 insertions(+), 76 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_acpi.c 
> > b/drivers/gpu/drm/i915/display/intel_acpi.c
> > index 3456d33feb46..748d9b3125dd 100644
> > --- a/drivers/gpu/drm/i915/display/intel_acpi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_acpi.c
> > @@ -156,3 +156,90 @@ void intel_register_dsm_handler(void)
> >  void intel_unregister_dsm_handler(void)
> >  {
> >  }
> > +
> > +/*
> > + * ACPI Specification, Revision 5.0, Appendix B.3.2 _DOD (Enumerate All 
> > Devices
> > + * Attached to the Display Adapter).
> > + */
> > +#define ACPI_DISPLAY_INDEX_SHIFT 0
> > +#define ACPI_DISPLAY_INDEX_MASK  (0xf << 0)
> > +#define ACPI_DISPLAY_PORT_ATTACHMENT_SHIFT   4
> > +#define ACPI_DISPLAY_PORT_ATTACHMENT_MASK(0xf << 4)
> > +#define ACPI_DISPLAY_TYPE_SHIFT  8
> > +#define ACPI_DISPLAY_TYPE_MASK   (0xf << 8)
> > +#define ACPI_DISPLAY_TYPE_OTHER  (0 << 8)
> > +#define ACPI_DISPLAY_TYPE_VGA(1 << 8)
> > +#define ACPI_DISPLAY_TYPE_TV (2 << 8)
> > +#define ACPI_DISPLAY_TYPE_EXTERNAL_DIGITAL   (3 << 8)
> > +#define ACPI_DISPLAY_TYPE_INTERNAL_DIGITAL   (4 << 8)
> > +#define ACPI_VENDOR_SPECIFIC_SHIFT   12
> > +#define ACPI_VENDOR_SPECIFIC_MASK(0xf << 12)
> > +#define ACPI_BIOS_CAN_DETECT (1 << 16)
> > +#define ACPI_DEPENDS_ON_VGA  (1 << 17)
> > +#define ACPI_PIPE_ID_SHIFT   18
> > +#define ACPI_PIPE_ID_MASK(7 << 18)
> > +#define ACPI_DEVICE_ID_SCHEME(1ULL << 31)
> > +
> > +static u32 acpi_display_type(struct intel_connector *connector)
> > +{
> > + u32 display_type;
> > +
> > + switch (connector->base.connector_type) {
> > + case DRM_MODE_CONNECTOR_VGA:
> > + case DRM_MODE_CONNECTOR_DVIA:
> > + display_type = ACPI_DISPLAY_TYPE_VGA;
> > + break;
> > + case DRM_MODE_CONNECTOR_Composite:
> > + case DRM_MODE_CONNECTOR_SVIDEO:
> > + case DRM_MODE_CONNECTOR_Component:
> > + case DRM_MODE_CONNECTOR_9PinDIN:
> > + case DRM_MODE_CONNECTOR_TV:
> > + display_type = ACPI_DISPLAY_TYPE_TV;
> > + break;
> > + case DRM_MODE_CONNECTOR_DVII:
> > + case DRM_MODE_CONNECTOR_DVID:
> > + case DRM_MODE_CONNECTOR_DisplayPort:
> > + case DRM_MODE_CONNECTOR_HDMIA:
> > + case DRM_MODE_CONNECTOR_HDMIB:
> > + display_type = ACPI_DISPLAY_TYPE_EXTERNAL_DIGITAL;
> > + break;
> > + case DRM_MODE_CONNECTOR_LVDS:
> > + case DRM_MODE_CONNECTOR_eDP:
> > + case DRM_MODE_CONNECTOR_DSI:
> > + display_type = ACPI_DISPLAY_TYPE_INTERNAL_DIGITAL;
> > + break;
> > + case DRM_MODE_CONNECTOR_Unknown:
> > + case DRM_MODE_CONNECTOR_VIRTUAL:
> > + display_type = ACPI_DISPLAY_TYPE_OTHER;
> > + break;
> > + default:
> > + MISSING_CASE(connector->base.connector_type);
> > + display_type = ACPI_DISPLAY_TYPE_OTHER;
> > + break;
> > + }
> > +
> > + return display_type;
> > +}
> > +
> > +void intel_populate_acpi_ids_for_all_connectors(struct drm_device *drm_dev)
>
> Plase use inte

Re: [Intel-gfx] [PATCH v2 2/3] drm/i915: Lookup and attach ACPI device node for connectors

2019-12-05 Thread Rajat Jain
On Wed, Nov 20, 2019 at 6:51 AM Jani Nikula  wrote:
>
> On Mon, 04 Nov 2019, Rajat Jain  wrote:
> > Lookup and attach ACPI nodes for intel connectors. The lookup is done
> > in compliance with ACPI Spec 6.3
> > https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf
> > (Ref: Pages 1119 - 1123).
> >
> > This can be useful for any connector specific platform properties. (This
> > will be used for privacy screen in next patch).
> >
> > Signed-off-by: Rajat Jain 
> > Change-Id: I798e70714a4402554c8cd2a8e58268353f75814f
> > ---
> > v2: formed by splitting the original patch into ACPI lookup, and privacy
> > screen property. Also move it into i915 now that I found existing code
> > in i915 that can be re-used.
> >
> >  drivers/gpu/drm/i915/display/intel_acpi.c | 50 +++
> >  drivers/gpu/drm/i915/display/intel_acpi.h |  4 +-
> >  .../drm/i915/display/intel_display_types.h|  3 ++
> >  drivers/gpu/drm/i915/display/intel_dp.c   |  4 ++
> >  4 files changed, 60 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_acpi.c 
> > b/drivers/gpu/drm/i915/display/intel_acpi.c
> > index 748d9b3125dd..0c10516430b1 100644
> > --- a/drivers/gpu/drm/i915/display/intel_acpi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_acpi.c
> > @@ -243,3 +243,53 @@ void intel_populate_acpi_ids_for_all_connectors(struct 
> > drm_device *drm_dev)
> >   }
> >   drm_connector_list_iter_end(&conn_iter);
> >  }
> > +
> > +/*
> > + * Ref: ACPI Spec 6.3
> > + * https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf
> > + * Pages 1119 - 1123 describe, what I believe, a standard way of
> > + * identifying / addressing "display panels" in the ACPI. It provides
> > + * a way for the ACPI to define devices for the display panels attached
> > + * to the system. It thus provides a way for the BIOS to export any panel
> > + * specific properties to the system via ACPI (like device trees).
> > + *
> > + * The following functions looks up the ACPI node for a connector and 
> > returns
> > + * it. Technically it is independent from the i915 code, and
> > + * ideally may be called for all connectors. It is generally a good idea to
> > + * be able to attach an ACPI node to describe anything if needed. (This can
> > + * help in future for other panel specific features maybe). However, it
> > + * needs an acpi device ID which is build using an index within a 
> > particular
> > + * type of port (Ref to the pages of spec mentioned above, and to code in
> > + * intel_populate_acpi_ids_for_all_connectors()). This device index
> > + * unfortunately is not available in DRM code, so currently its call is
> > + * originated from i915 driver. If in future this is useful for other 
> > drivers
> > + * and we can find a generic way of getting a device index, we should move 
> > this
> > + * function to drm code, maybe.
> > + */
> > +void intel_connector_lookup_acpi_node(struct intel_connector 
> > *intel_connector)
>
> Nitpick, I'd expect a "lookup" function to return whatever it is looking
> up, not modify its argument.

I folded this function into the other function as you suggested below.

>
> > +{
> > + struct drm_device *drm_dev = intel_connector->base.dev;
> > + struct device *dev = &drm_dev->pdev->dev;
> > + struct acpi_device *conn_dev;
> > + u64 conn_addr;
> > +
> > + /*
> > +  * Repopulate ACPI IDs for all connectors is needed because the 
> > display
> > +  * index may have changed as a result of hotplugging and unplugging
> > +  * connectors
> > +  */
>
> I think that can only be true for DP MST. For everything else, I don't
> think so. Anyway, why are we doing it here then, depending on whether
> someone calls this function or not? If it matters, we should be doing
> this whenever there's a chance they've changed, right?
>

Actually I removed that comment now. To be really honest, my
understanding about the need to do this on every resume was only based
on the observation that this was being done on every call to
intel_opregion_resume() in addition to intel_opregion_register(). I'm
not sure if my understanding is correct, so unless the original author
of said code intel_opregion_* chimes in, I'm hesitant to change that
code. For privacy screen purposes, this works fine.

> > + intel_populate_acpi_ids_for_all_connectors(drm_dev);
> > +
> > + /* Build the _ADR to 

[Intel-gfx] [PATCH v3 2/3] drm/i915: Lookup and attach ACPI device node for connectors

2019-12-05 Thread Rajat Jain
Lookup and attach ACPI nodes for intel connectors. The lookup is done
in compliance with ACPI Spec 6.3
https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf
(Ref: Pages 1119 - 1123).

This can be useful for any connector specific platform properties. (This
will be used for privacy screen in next patch).

Signed-off-by: Rajat Jain 
---
v3: fold the code into existing acpi_device_id_update() function
v2: formed by splitting the original patch into ACPI lookup, and privacy
screen property. Also move it into i915 now that I found existing code
in i915 that can be re-used.

 drivers/gpu/drm/i915/display/intel_acpi.c | 24 +++
 .../drm/i915/display/intel_display_types.h|  3 +++
 drivers/gpu/drm/i915/display/intel_dp.c   |  3 +++
 3 files changed, 30 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_acpi.c 
b/drivers/gpu/drm/i915/display/intel_acpi.c
index e21fb14d5e07..101a56c08996 100644
--- a/drivers/gpu/drm/i915/display/intel_acpi.c
+++ b/drivers/gpu/drm/i915/display/intel_acpi.c
@@ -222,11 +222,23 @@ static u32 acpi_display_type(struct intel_connector 
*connector)
return display_type;
 }
 
+/*
+ * Ref: ACPI Spec 6.3
+ * https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf
+ * Pages 1119 - 1123 describe, what I believe, a standard way of
+ * identifying / addressing "display panels" in the ACPI. It provides
+ * a way for the ACPI to define devices for the display panels attached
+ * to the system. It thus provides a way for the BIOS to export any panel
+ * specific properties to the system via ACPI (like device trees).
+ */
 void intel_acpi_device_id_update(struct drm_i915_private *dev_priv)
 {
struct drm_device *drm_dev = &dev_priv->drm;
struct intel_connector *connector;
struct drm_connector_list_iter conn_iter;
+   struct device *dev = &drm_dev->pdev->dev;
+   struct acpi_device *conn_dev;
+   u64 conn_addr;
u8 display_index[16] = {};
 
/* Populate the ACPI IDs for all connectors for a given drm_device */
@@ -242,6 +254,18 @@ void intel_acpi_device_id_update(struct drm_i915_private 
*dev_priv)
device_id |= display_index[type]++ << ACPI_DISPLAY_INDEX_SHIFT;
 
connector->acpi_device_id = device_id;
+
+   /* Build the _ADR to look for */
+   conn_addr = device_id | ACPI_DEVICE_ID_SCHEME |
+   ACPI_BIOS_CAN_DETECT;
+
+   DRM_DEV_INFO(dev, "Checking connector ACPI node at _ADR=%llX\n",
+conn_addr);
+
+   /* Look up the connector device, under the PCI device */
+   conn_dev = acpi_find_child_device(ACPI_COMPANION(dev),
+ conn_addr, false);
+   connector->acpi_handle = conn_dev ? conn_dev->handle : NULL;
}
drm_connector_list_iter_end(&conn_iter);
 }
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h 
b/drivers/gpu/drm/i915/display/intel_display_types.h
index 1a7334dbe802..0a4a04116091 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -407,6 +407,9 @@ struct intel_connector {
/* ACPI device id for ACPI and driver cooperation */
u32 acpi_device_id;
 
+   /* ACPI handle corresponding to this connector display, if found */
+   void *acpi_handle;
+
/* Reads out the current hw, returning true if the connector is enabled
 * and active (i.e. dpms ON state). */
bool (*get_hw_state)(struct intel_connector *);
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
b/drivers/gpu/drm/i915/display/intel_dp.c
index c61ac0c3acb5..6b209193cbbb 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -45,6 +45,7 @@
 #include "i915_debugfs.h"
 #include "i915_drv.h"
 #include "i915_trace.h"
+#include "intel_acpi.h"
 #include "intel_atomic.h"
 #include "intel_audio.h"
 #include "intel_connector.h"
@@ -6628,6 +6629,8 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct 
drm_connector *connect
 
connector->state->scaling_mode = DRM_MODE_SCALE_ASPECT;
 
+   /* Lookup the ACPI node corresponding to the connector */
+   intel_acpi_device_id_update(dev_priv);
}
 }
 
-- 
2.24.0.393.g34dc348eaf-goog

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

Re: [Intel-gfx] [PATCH v2 3/3] drm/i915: Add support for integrated privacy screens

2019-12-05 Thread Rajat Jain
On Wed, Nov 20, 2019 at 7:04 AM Jani Nikula  wrote:
>
> On Mon, 04 Nov 2019, 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 intel_connector for the panel, that
> > the userspace can then use to control and check the status.
> >
> > 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?).
> >
> > Signed-off-by: Rajat Jain 
> > Change-Id: Ic9ff07fc4a50797d2d0dfb919f11aa0821a4b548
> > ---
> > v2: Formed by splitting the original patch into multiple patches.
> > - All code has been moved into i915 now.
> > - Privacy screen is a i915 property
> > - Have a local state variable to store the prvacy screen. Don't read
> >   it from hardware.
> >
> >  drivers/gpu/drm/i915/Makefile |  3 +-
> >  drivers/gpu/drm/i915/display/intel_atomic.c   | 13 +++-
> >  .../gpu/drm/i915/display/intel_connector.c| 35 ++
> >  .../gpu/drm/i915/display/intel_connector.h|  1 +
> >  .../drm/i915/display/intel_display_types.h|  4 ++
> >  drivers/gpu/drm/i915/display/intel_dp.c   |  5 ++
> >  .../drm/i915/display/intel_privacy_screen.c   | 70 +++
> >  .../drm/i915/display/intel_privacy_screen.h   | 25 +++
> >  include/uapi/drm/i915_drm.h   | 14 
> >  9 files changed, 166 insertions(+), 4 deletions(-)
> >  create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.c
> >  create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.h
> >
> > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> > index 2587ea834f06..3589ebcf27bc 100644
> > --- a/drivers/gpu/drm/i915/Makefile
> > +++ b/drivers/gpu/drm/i915/Makefile
> > @@ -185,7 +185,8 @@ i915-y += \
> >   display/intel_tc.o
> >  i915-$(CONFIG_ACPI) += \
> >   display/intel_acpi.o \
> > - display/intel_opregion.o
> > + display/intel_opregion.o \
> > + display/intel_privacy_screen.o
>
> Mmmh, wonder if there'll be non-ACPI based privacy screens. I guess we
> can sort this out then. *shrug*
>
> >  i915-$(CONFIG_DRM_FBDEV_EMULATION) += \
> >   display/intel_fbdev.o
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c 
> > b/drivers/gpu/drm/i915/display/intel_atomic.c
> > index d3fb75bb9eb1..378772d3449c 100644
> > --- a/drivers/gpu/drm/i915/display/intel_atomic.c
> > +++ b/drivers/gpu/drm/i915/display/intel_atomic.c
> > @@ -37,6 +37,7 @@
> >  #include "intel_atomic.h"
> >  #include "intel_display_types.h"
> >  #include "intel_hdcp.h"
> > +#include "intel_privacy_screen.h"
> >  #include "intel_sprite.h"
> >
> >  /**
> > @@ -57,11 +58,14 @@ int intel_digital_connector_atomic_get_property(struct 
> > drm_connector *connector,
> >   struct drm_i915_private *dev_priv = to_i915(dev);
> >   struct intel_digital_connector_state *intel_conn_state =
> >   to_intel_digital_connector_state(state);
> > + struct intel_connector *intel_connector = 
> > to_intel_connector(connector);
> >
> >   if (property == dev_priv->force_audio_property)
> >   *val = intel_conn_state->force_audio;
> >   else if (property == dev_priv->broadcast_rgb_property)
> >   *val = intel_conn_state->broadcast_rgb;
> > + else if (property == intel_connector->privacy_screen_property)
> > + *val = intel_conn_state->privacy_screen_status;
> >   else {
> >   DRM_DEBUG_ATOMIC("Unknown property [PROP:%d:%s]\n",
> >property->base.id, property->name);
> > @@ -89,15 +93,18 @@ int intel_digital_connector_atomic_set_property(struct 
> > drm_connector *connector,
> >   struct drm_i915_private *dev_priv = to_i915(dev);
> >   struct intel_digital_connector_state *intel_conn_state =
> >   to_intel_digital_connector_state(state);
> > + struct intel_connector *intel_connector = 
> > to_intel_connector(connector);
> >
> >   if (property == dev_priv->force_audio_property) {
> >   intel_

Re: [Intel-gfx] [PATCH v2 3/3] drm/i915: Add support for integrated privacy screens

2019-11-20 Thread Rajat Jain
Hi Jani,

On Wed, Nov 20, 2019 at 7:11 AM Jani Nikula  wrote:
>
> On Tue, 12 Nov 2019, Rajat Jain  wrote:
> > On Mon, Nov 4, 2019 at 11:41 AM 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 intel_connector for the panel, that
> >> the userspace can then use to control and check the status.
> >>
> >> 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?).
> >>
> >> Signed-off-by: Rajat Jain 
> >> Change-Id: Ic9ff07fc4a50797d2d0dfb919f11aa0821a4b548
> >
> >
> > Hi Folks,
> >
> > I posted a v2 taking care of the comments I received (also split it
> > into 3 patches now, and resused some ACPI code I found in i915
> > driver). . Wondering if any one got a chance to look at this?
>
> For future reference, please post the updated series standalone, *not*
> in reply to long, old threads. Besides myself, it'll also help our CI
> find your patches and crunch a bunch of tests on them.

Will do.

>
> Also, do you have an open userspace for this? See [1]. I think this
> looks like good stuff to me, but then I'm not responsible for any
> userspace component that would actually use this.

Not sure what you meant but the user for this on Chromebooks (what I
work on) would be the Chrome browser most likely.

Thanks & Best Regards,

Rajat

>
> BR,
> Jani.
>
>
> [1] 
> https://www.kernel.org/doc/html/latest/gpu/drm-uapi.html#open-source-userspace-requirements
>
>
>
> >
> > Thanks,
> >
> > Rajat
> >
> >> ---
> >> v2: Formed by splitting the original patch into multiple patches.
> >> - All code has been moved into i915 now.
> >> - Privacy screen is a i915 property
> >> - Have a local state variable to store the prvacy screen. Don't read
> >>   it from hardware.
> >>
> >>  drivers/gpu/drm/i915/Makefile |  3 +-
> >>  drivers/gpu/drm/i915/display/intel_atomic.c   | 13 +++-
> >>  .../gpu/drm/i915/display/intel_connector.c| 35 ++
> >>  .../gpu/drm/i915/display/intel_connector.h|  1 +
> >>  .../drm/i915/display/intel_display_types.h|  4 ++
> >>  drivers/gpu/drm/i915/display/intel_dp.c   |  5 ++
> >>  .../drm/i915/display/intel_privacy_screen.c   | 70 +++
> >>  .../drm/i915/display/intel_privacy_screen.h   | 25 +++
> >>  include/uapi/drm/i915_drm.h   | 14 
> >>  9 files changed, 166 insertions(+), 4 deletions(-)
> >>  create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.c
> >>  create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.h
> >>
> >> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> >> index 2587ea834f06..3589ebcf27bc 100644
> >> --- a/drivers/gpu/drm/i915/Makefile
> >> +++ b/drivers/gpu/drm/i915/Makefile
> >> @@ -185,7 +185,8 @@ i915-y += \
> >> display/intel_tc.o
> >>  i915-$(CONFIG_ACPI) += \
> >> display/intel_acpi.o \
> >> -   display/intel_opregion.o
> >> +   display/intel_opregion.o \
> >> +   display/intel_privacy_screen.o
> >>  i915-$(CONFIG_DRM_FBDEV_EMULATION) += \
> >> display/intel_fbdev.o
> >>
> >> diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c 
> >> b/drivers/gpu/drm/i915/display/intel_atomic.c
> >> index d3fb75bb9eb1..378772d3449c 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_atomic.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_atomic.c
> >> @@ -37,6 +37,7 @@
> >>  #include "intel_atomic.h"
> >>  #include "intel_display_types.h"
> >>  #include "intel_hdcp.h"
> >> +#include "intel_privacy_screen.h"
> >>  #include "intel_sprite.h"
> >>
> >>  /**
> >> @@ -57,11 +58,14 @@ int intel_digital_connector_atomic_get_property(struct 
> >> drm_connector *connector,
> >> struct drm_i915_private *dev_priv = to_i915(dev);
> >> struct intel_digital_connector_state *intel_conn_state 

Re: [Intel-gfx] [PATCH v2 3/3] drm/i915: Add support for integrated privacy screens

2019-11-12 Thread Rajat Jain
On Mon, Nov 4, 2019 at 11:41 AM 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 intel_connector for the panel, that
> the userspace can then use to control and check the status.
>
> 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?).
>
> Signed-off-by: Rajat Jain 
> Change-Id: Ic9ff07fc4a50797d2d0dfb919f11aa0821a4b548


Hi Folks,

I posted a v2 taking care of the comments I received (also split it
into 3 patches now, and resused some ACPI code I found in i915
driver). . Wondering if any one got a chance to look at this?

Thanks,

Rajat

> ---
> v2: Formed by splitting the original patch into multiple patches.
> - All code has been moved into i915 now.
> - Privacy screen is a i915 property
> - Have a local state variable to store the prvacy screen. Don't read
>   it from hardware.
>
>  drivers/gpu/drm/i915/Makefile |  3 +-
>  drivers/gpu/drm/i915/display/intel_atomic.c   | 13 +++-
>  .../gpu/drm/i915/display/intel_connector.c| 35 ++
>  .../gpu/drm/i915/display/intel_connector.h|  1 +
>  .../drm/i915/display/intel_display_types.h|  4 ++
>  drivers/gpu/drm/i915/display/intel_dp.c   |  5 ++
>  .../drm/i915/display/intel_privacy_screen.c   | 70 +++
>  .../drm/i915/display/intel_privacy_screen.h   | 25 +++
>  include/uapi/drm/i915_drm.h   | 14 
>  9 files changed, 166 insertions(+), 4 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.c
>  create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.h
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 2587ea834f06..3589ebcf27bc 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -185,7 +185,8 @@ i915-y += \
> display/intel_tc.o
>  i915-$(CONFIG_ACPI) += \
> display/intel_acpi.o \
> -   display/intel_opregion.o
> +   display/intel_opregion.o \
> +   display/intel_privacy_screen.o
>  i915-$(CONFIG_DRM_FBDEV_EMULATION) += \
> display/intel_fbdev.o
>
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c 
> b/drivers/gpu/drm/i915/display/intel_atomic.c
> index d3fb75bb9eb1..378772d3449c 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic.c
> @@ -37,6 +37,7 @@
>  #include "intel_atomic.h"
>  #include "intel_display_types.h"
>  #include "intel_hdcp.h"
> +#include "intel_privacy_screen.h"
>  #include "intel_sprite.h"
>
>  /**
> @@ -57,11 +58,14 @@ int intel_digital_connector_atomic_get_property(struct 
> drm_connector *connector,
> struct drm_i915_private *dev_priv = to_i915(dev);
> struct intel_digital_connector_state *intel_conn_state =
> to_intel_digital_connector_state(state);
> +   struct intel_connector *intel_connector = 
> to_intel_connector(connector);
>
> if (property == dev_priv->force_audio_property)
> *val = intel_conn_state->force_audio;
> else if (property == dev_priv->broadcast_rgb_property)
> *val = intel_conn_state->broadcast_rgb;
> +   else if (property == intel_connector->privacy_screen_property)
> +   *val = intel_conn_state->privacy_screen_status;
> else {
> DRM_DEBUG_ATOMIC("Unknown property [PROP:%d:%s]\n",
>  property->base.id, property->name);
> @@ -89,15 +93,18 @@ int intel_digital_connector_atomic_set_property(struct 
> drm_connector *connector,
> struct drm_i915_private *dev_priv = to_i915(dev);
> struct intel_digital_connector_state *intel_conn_state =
> to_intel_digital_connector_state(state);
> +   struct intel_connector *intel_connector = 
> to_intel_connector(connector);
>
> if (property == dev_priv->force_audio_property) {
> intel_conn_state->force_audio = val;
> return 0;
> -   }
> -
> -   if (property == dev_priv->broadcast_rgb_property) {
> +   } else if (property == dev_priv->broadcast_rgb_property) {
> intel_conn_state->broadcast_rgb = val;
> return 0;
> +   } else

[Intel-gfx] [PATCH v2 2/3] drm/i915: Lookup and attach ACPI device node for connectors

2019-11-05 Thread Rajat Jain
Lookup and attach ACPI nodes for intel connectors. The lookup is done
in compliance with ACPI Spec 6.3
https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf
(Ref: Pages 1119 - 1123).

This can be useful for any connector specific platform properties. (This
will be used for privacy screen in next patch).

Signed-off-by: Rajat Jain 
Change-Id: I798e70714a4402554c8cd2a8e58268353f75814f
---
v2: formed by splitting the original patch into ACPI lookup, and privacy
screen property. Also move it into i915 now that I found existing code
in i915 that can be re-used.

 drivers/gpu/drm/i915/display/intel_acpi.c | 50 +++
 drivers/gpu/drm/i915/display/intel_acpi.h |  4 +-
 .../drm/i915/display/intel_display_types.h|  3 ++
 drivers/gpu/drm/i915/display/intel_dp.c   |  4 ++
 4 files changed, 60 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_acpi.c 
b/drivers/gpu/drm/i915/display/intel_acpi.c
index 748d9b3125dd..0c10516430b1 100644
--- a/drivers/gpu/drm/i915/display/intel_acpi.c
+++ b/drivers/gpu/drm/i915/display/intel_acpi.c
@@ -243,3 +243,53 @@ void intel_populate_acpi_ids_for_all_connectors(struct 
drm_device *drm_dev)
}
drm_connector_list_iter_end(&conn_iter);
 }
+
+/*
+ * Ref: ACPI Spec 6.3
+ * https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf
+ * Pages 1119 - 1123 describe, what I believe, a standard way of
+ * identifying / addressing "display panels" in the ACPI. It provides
+ * a way for the ACPI to define devices for the display panels attached
+ * to the system. It thus provides a way for the BIOS to export any panel
+ * specific properties to the system via ACPI (like device trees).
+ *
+ * The following functions looks up the ACPI node for a connector and returns
+ * it. Technically it is independent from the i915 code, and
+ * ideally may be called for all connectors. It is generally a good idea to
+ * be able to attach an ACPI node to describe anything if needed. (This can
+ * help in future for other panel specific features maybe). However, it
+ * needs an acpi device ID which is build using an index within a particular
+ * type of port (Ref to the pages of spec mentioned above, and to code in
+ * intel_populate_acpi_ids_for_all_connectors()). This device index
+ * unfortunately is not available in DRM code, so currently its call is
+ * originated from i915 driver. If in future this is useful for other drivers
+ * and we can find a generic way of getting a device index, we should move this
+ * function to drm code, maybe.
+ */
+void intel_connector_lookup_acpi_node(struct intel_connector *intel_connector)
+{
+   struct drm_device *drm_dev = intel_connector->base.dev;
+   struct device *dev = &drm_dev->pdev->dev;
+   struct acpi_device *conn_dev;
+   u64 conn_addr;
+
+   /*
+* Repopulate ACPI IDs for all connectors is needed because the display
+* index may have changed as a result of hotplugging and unplugging
+* connectors
+*/
+   intel_populate_acpi_ids_for_all_connectors(drm_dev);
+
+   /* Build the _ADR to look for */
+   conn_addr = intel_connector->acpi_device_id;
+   conn_addr |= ACPI_DEVICE_ID_SCHEME;
+   conn_addr |= ACPI_BIOS_CAN_DETECT;
+
+   DRM_DEV_INFO(dev, "Looking for connector ACPI node at _ADR=%llX\n",
+conn_addr);
+
+   /* Look up the connector device, under the PCI device */
+   conn_dev = acpi_find_child_device(ACPI_COMPANION(dev), conn_addr,
+ false);
+   intel_connector->acpi_handle = conn_dev ? conn_dev->handle : NULL;
+}
diff --git a/drivers/gpu/drm/i915/display/intel_acpi.h 
b/drivers/gpu/drm/i915/display/intel_acpi.h
index 8f6d850df6fa..61a4392fac4a 100644
--- a/drivers/gpu/drm/i915/display/intel_acpi.h
+++ b/drivers/gpu/drm/i915/display/intel_acpi.h
@@ -9,14 +9,16 @@
 #include "intel_display_types.h"
 
 #ifdef CONFIG_ACPI
+void intel_connector_lookup_acpi_node(struct intel_connector *connector);
 void intel_register_dsm_handler(void);
 void intel_unregister_dsm_handler(void);
 void intel_populate_acpi_ids_for_all_connectors(struct drm_device *drm_dev);
 #else
+static inline void
+intel_connector_lookup_acpi_node(struct intel_connector *connector) { return; }
 static inline void intel_register_dsm_handler(void) { return; }
 static inline void intel_unregister_dsm_handler(void) { return; }
 static inline void
-static inline void
 intel_populate_acpi_ids_for_all_connectors(struct drm_device *drm_dev) { }
 #endif /* CONFIG_ACPI */
 
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h 
b/drivers/gpu/drm/i915/display/intel_display_types.h
index 449abaea619f..c2706afc069b 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -400,6 +400,9 @@ struct intel_connector {
 

[Intel-gfx] [PATCH v2 1/3] drm/i915: Move the code to populate ACPI device ID into intel_acpi

2019-11-05 Thread Rajat Jain
Move the code that populates the ACPI device ID for devices, into
more appripriate intel_acpi.c. This is done in preparation for more
users of this code (in next patch).

Signed-off-by: Rajat Jain 
Change-Id: Ifb3bd458734985c2a78ba682e6f0a2e63e0626ca
---
v2: v1 doesn't exist. Found existing code in i915 driver to assign the ACPI ID
which is what I plan to re-use.


 drivers/gpu/drm/i915/display/intel_acpi.c | 87 +++
 drivers/gpu/drm/i915/display/intel_acpi.h |  6 ++
 drivers/gpu/drm/i915/display/intel_opregion.c | 80 +
 3 files changed, 97 insertions(+), 76 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_acpi.c 
b/drivers/gpu/drm/i915/display/intel_acpi.c
index 3456d33feb46..748d9b3125dd 100644
--- a/drivers/gpu/drm/i915/display/intel_acpi.c
+++ b/drivers/gpu/drm/i915/display/intel_acpi.c
@@ -156,3 +156,90 @@ void intel_register_dsm_handler(void)
 void intel_unregister_dsm_handler(void)
 {
 }
+
+/*
+ * ACPI Specification, Revision 5.0, Appendix B.3.2 _DOD (Enumerate All Devices
+ * Attached to the Display Adapter).
+ */
+#define ACPI_DISPLAY_INDEX_SHIFT   0
+#define ACPI_DISPLAY_INDEX_MASK(0xf << 0)
+#define ACPI_DISPLAY_PORT_ATTACHMENT_SHIFT 4
+#define ACPI_DISPLAY_PORT_ATTACHMENT_MASK  (0xf << 4)
+#define ACPI_DISPLAY_TYPE_SHIFT8
+#define ACPI_DISPLAY_TYPE_MASK (0xf << 8)
+#define ACPI_DISPLAY_TYPE_OTHER(0 << 8)
+#define ACPI_DISPLAY_TYPE_VGA  (1 << 8)
+#define ACPI_DISPLAY_TYPE_TV   (2 << 8)
+#define ACPI_DISPLAY_TYPE_EXTERNAL_DIGITAL (3 << 8)
+#define ACPI_DISPLAY_TYPE_INTERNAL_DIGITAL (4 << 8)
+#define ACPI_VENDOR_SPECIFIC_SHIFT 12
+#define ACPI_VENDOR_SPECIFIC_MASK  (0xf << 12)
+#define ACPI_BIOS_CAN_DETECT   (1 << 16)
+#define ACPI_DEPENDS_ON_VGA(1 << 17)
+#define ACPI_PIPE_ID_SHIFT 18
+#define ACPI_PIPE_ID_MASK  (7 << 18)
+#define ACPI_DEVICE_ID_SCHEME  (1ULL << 31)
+
+static u32 acpi_display_type(struct intel_connector *connector)
+{
+   u32 display_type;
+
+   switch (connector->base.connector_type) {
+   case DRM_MODE_CONNECTOR_VGA:
+   case DRM_MODE_CONNECTOR_DVIA:
+   display_type = ACPI_DISPLAY_TYPE_VGA;
+   break;
+   case DRM_MODE_CONNECTOR_Composite:
+   case DRM_MODE_CONNECTOR_SVIDEO:
+   case DRM_MODE_CONNECTOR_Component:
+   case DRM_MODE_CONNECTOR_9PinDIN:
+   case DRM_MODE_CONNECTOR_TV:
+   display_type = ACPI_DISPLAY_TYPE_TV;
+   break;
+   case DRM_MODE_CONNECTOR_DVII:
+   case DRM_MODE_CONNECTOR_DVID:
+   case DRM_MODE_CONNECTOR_DisplayPort:
+   case DRM_MODE_CONNECTOR_HDMIA:
+   case DRM_MODE_CONNECTOR_HDMIB:
+   display_type = ACPI_DISPLAY_TYPE_EXTERNAL_DIGITAL;
+   break;
+   case DRM_MODE_CONNECTOR_LVDS:
+   case DRM_MODE_CONNECTOR_eDP:
+   case DRM_MODE_CONNECTOR_DSI:
+   display_type = ACPI_DISPLAY_TYPE_INTERNAL_DIGITAL;
+   break;
+   case DRM_MODE_CONNECTOR_Unknown:
+   case DRM_MODE_CONNECTOR_VIRTUAL:
+   display_type = ACPI_DISPLAY_TYPE_OTHER;
+   break;
+   default:
+   MISSING_CASE(connector->base.connector_type);
+   display_type = ACPI_DISPLAY_TYPE_OTHER;
+   break;
+   }
+
+   return display_type;
+}
+
+void intel_populate_acpi_ids_for_all_connectors(struct drm_device *drm_dev)
+{
+   struct intel_connector *connector;
+   struct drm_connector_list_iter conn_iter;
+   u8 display_index[16] = {};
+   u32 device_id, type;
+
+   /* Populate the ACPI IDs for all connectors for a given drm_device */
+   drm_connector_list_iter_begin(drm_dev, &conn_iter);
+   for_each_intel_connector_iter(connector, &conn_iter) {
+
+   device_id = acpi_display_type(connector);
+
+   /* Use display type specific display index. */
+   type = (device_id & ACPI_DISPLAY_TYPE_MASK)
+   >> ACPI_DISPLAY_TYPE_SHIFT;
+   device_id |= display_index[type]++ << ACPI_DISPLAY_INDEX_SHIFT;
+
+   connector->acpi_device_id = device_id;
+   }
+   drm_connector_list_iter_end(&conn_iter);
+}
diff --git a/drivers/gpu/drm/i915/display/intel_acpi.h 
b/drivers/gpu/drm/i915/display/intel_acpi.h
index 1c576b3fb712..8f6d850df6fa 100644
--- a/drivers/gpu/drm/i915/display/intel_acpi.h
+++ b/drivers/gpu/drm/i915/display/intel_acpi.h
@@ -6,12 +6,18 @@
 #ifndef __INTEL_ACPI_H__
 #define __INTEL_ACPI_H__
 
+#include "intel_display_types.h"
+
 #ifdef CONFIG_ACPI
 void intel_register_dsm_handler(void);
 void intel_unre

[Intel-gfx] [PATCH v2 3/3] drm/i915: Add support for integrated privacy screens

2019-11-05 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 intel_connector for the panel, that
the userspace can then use to control and check the status.

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?).

Signed-off-by: Rajat Jain 
Change-Id: Ic9ff07fc4a50797d2d0dfb919f11aa0821a4b548
---
v2: Formed by splitting the original patch into multiple patches.
- All code has been moved into i915 now.
- Privacy screen is a i915 property
- Have a local state variable to store the prvacy screen. Don't read
  it from hardware.

 drivers/gpu/drm/i915/Makefile |  3 +-
 drivers/gpu/drm/i915/display/intel_atomic.c   | 13 +++-
 .../gpu/drm/i915/display/intel_connector.c| 35 ++
 .../gpu/drm/i915/display/intel_connector.h|  1 +
 .../drm/i915/display/intel_display_types.h|  4 ++
 drivers/gpu/drm/i915/display/intel_dp.c   |  5 ++
 .../drm/i915/display/intel_privacy_screen.c   | 70 +++
 .../drm/i915/display/intel_privacy_screen.h   | 25 +++
 include/uapi/drm/i915_drm.h   | 14 
 9 files changed, 166 insertions(+), 4 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.c
 create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 2587ea834f06..3589ebcf27bc 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -185,7 +185,8 @@ i915-y += \
display/intel_tc.o
 i915-$(CONFIG_ACPI) += \
display/intel_acpi.o \
-   display/intel_opregion.o
+   display/intel_opregion.o \
+   display/intel_privacy_screen.o
 i915-$(CONFIG_DRM_FBDEV_EMULATION) += \
display/intel_fbdev.o
 
diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c 
b/drivers/gpu/drm/i915/display/intel_atomic.c
index d3fb75bb9eb1..378772d3449c 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic.c
@@ -37,6 +37,7 @@
 #include "intel_atomic.h"
 #include "intel_display_types.h"
 #include "intel_hdcp.h"
+#include "intel_privacy_screen.h"
 #include "intel_sprite.h"
 
 /**
@@ -57,11 +58,14 @@ int intel_digital_connector_atomic_get_property(struct 
drm_connector *connector,
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_digital_connector_state *intel_conn_state =
to_intel_digital_connector_state(state);
+   struct intel_connector *intel_connector = to_intel_connector(connector);
 
if (property == dev_priv->force_audio_property)
*val = intel_conn_state->force_audio;
else if (property == dev_priv->broadcast_rgb_property)
*val = intel_conn_state->broadcast_rgb;
+   else if (property == intel_connector->privacy_screen_property)
+   *val = intel_conn_state->privacy_screen_status;
else {
DRM_DEBUG_ATOMIC("Unknown property [PROP:%d:%s]\n",
 property->base.id, property->name);
@@ -89,15 +93,18 @@ int intel_digital_connector_atomic_set_property(struct 
drm_connector *connector,
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_digital_connector_state *intel_conn_state =
to_intel_digital_connector_state(state);
+   struct intel_connector *intel_connector = to_intel_connector(connector);
 
if (property == dev_priv->force_audio_property) {
intel_conn_state->force_audio = val;
return 0;
-   }
-
-   if (property == dev_priv->broadcast_rgb_property) {
+   } else if (property == dev_priv->broadcast_rgb_property) {
intel_conn_state->broadcast_rgb = val;
return 0;
+   } else if (property == intel_connector->privacy_screen_property) {
+   intel_privacy_screen_set_val(intel_connector, val);
+   intel_conn_state->privacy_screen_status = val;
+   return 0;
}
 
DRM_DEBUG_ATOMIC("Unknown property [PROP:%d:%s]\n",
diff --git a/drivers/gpu/drm/i915/display/intel_connector.c 
b/drivers/gpu/drm/i915/display/intel_connector.c
index 308ec63207ee..3ccbf52aedf9 100644
--- a/drivers/gpu/drm/i915/display/intel_connector.c
+++ b/drivers/gpu/drm/i915/display/intel_connector.c
@@ -281,3 +281,38 @@ intel_attach_colorspace_property(struct drm_connector 
*connector)
drm_object_attach_property(&connector->base,

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 
> &g

Re: [Intel-gfx] ✗ Fi.CI.BUILD: failure for drm: Add support for integrated privacy screens

2019-10-24 Thread Rajat Jain
On Thu, Oct 24, 2019 at 12:01 AM Jani Nikula
 wrote:
>
> On Thu, 24 Oct 2019, Patchwork  wrote:
> > == Series Details ==
> >
> > Series: drm: Add support for integrated privacy screens
> > URL   : https://patchwork.freedesktop.org/series/68472/
> > State : failure
> >
> > == Summary ==
> >
> > CALLscripts/checksyscalls.sh
> >   CALLscripts/atomic/check-atomics.sh
> >   DESCEND  objtool
> >   CHK include/generated/compile.h
> > Kernel: arch/x86/boot/bzImage is ready  (#1)
> >   Building modules, stage 2.
> >   MODPOST 122 modules
> > ERROR: "drm_privacy_screen_present" [drivers/gpu/drm/i915/i915.ko] 
> > undefined!
>
> This means you need to EXPORT_SYMBOL(drm_privacy_screen_present).

Sorry, will do.

>
> BR,
> Jani.
>
> > scripts/Makefile.modpost:93: recipe for target '__modpost' failed
> > make[1]: *** [__modpost] Error 1
> > Makefile:1282: recipe for target 'modules' failed
> > make: *** [modules] Error 2
> >
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Jani Nikula, Intel Open Source Graphics Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[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_priva