Re: [PATCH v5 3/3] drm/i915: Add support for integrated privacy screens
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->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
[PATCH v5 3/3] drm/i915: Add support for integrated privacy screens
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_attach_colorspace_property(struct drm_connector
*connector)
drm_object_attach_property(>base,
