| Message ID | 20191104194147.185642-3-rajatja@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: Add support for integrated privacy screens | expand |
On Mon, Nov 4, 2019 at 11:41 AM Rajat Jain <rajatja@google.com> 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 <rajatja@google.com> > 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. > 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 > > 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, > connector->colorspace_property, > 0); > } > + > +static const struct drm_prop_enum_list privacy_screen_enum[] = { > + { PRIVACY_SCREEN_DISABLED, "Disabled" }, > + { PRIVACY_SCREEN_ENABLED, "Enabled" }, > +}; > + > +/** > + * intel_attach_privacy_screen_property - > + * create and attach the connecter's privacy-screen property. * > + * @connector: connector for which to init the privacy-screen property > + * > + * This function creates and attaches the "privacy-screen" property to the > + * connector. Initial state of privacy-screen is set to disabled. > + */ > +void > +intel_attach_privacy_screen_property(struct drm_connector *connector) > +{ > + struct intel_connector *intel_connector = > to_intel_connector(connector); > + struct drm_property *prop; > + > + if (!intel_connector->privacy_screen_property) { > + prop = drm_property_create_enum(connector->dev, > + DRM_MODE_PROP_ENUM, > + "privacy-screen", > + privacy_screen_enum, > + > ARRAY_SIZE(privacy_screen_enum)); > + if (!prop) > + return; > + > + intel_connector->privacy_screen_property = prop; > + } > + > + drm_object_attach_property(&connector->base, prop, > + PRIVACY_SCREEN_DISABLED); > +} > diff --git a/drivers/gpu/drm/i915/display/intel_connector.h > b/drivers/gpu/drm/i915/display/intel_connector.h > index 93a7375c8196..61005f37a338 100644 > --- a/drivers/gpu/drm/i915/display/intel_connector.h > +++ b/drivers/gpu/drm/i915/display/intel_connector.h > @@ -31,5 +31,6 @@ void intel_attach_force_audio_property(struct > drm_connector *connector); > void intel_attach_broadcast_rgb_property(struct drm_connector *connector); > void intel_attach_aspect_ratio_property(struct drm_connector *connector); > void intel_attach_colorspace_property(struct drm_connector *connector); > +void intel_attach_privacy_screen_property(struct drm_connector > *connector); > > #endif /* __INTEL_CONNECTOR_H__ */ > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h > b/drivers/gpu/drm/i915/display/intel_display_types.h > index c2706afc069b..83b8c98049a7 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > @@ -426,6 +426,9 @@ struct intel_connector { > struct work_struct modeset_retry_work; > > struct intel_hdcp hdcp; > + > + /* Optional "privacy-screen" property for the connector panel */ > + struct drm_property *privacy_screen_property; > }; > > struct intel_digital_connector_state { > @@ -433,6 +436,7 @@ struct intel_digital_connector_state { > > enum hdmi_force_audio force_audio; > int broadcast_rgb; > + enum intel_privacy_screen_status privacy_screen_status; > }; > > #define to_intel_digital_connector_state(x) container_of(x, struct > intel_digital_connector_state, base) > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > b/drivers/gpu/drm/i915/display/intel_dp.c > index 4fac408a4299..1963e92404ba 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" > @@ -6358,6 +6359,10 @@ intel_dp_add_properties(struct intel_dp *intel_dp, > struct drm_connector *connect > > /* Lookup the ACPI node corresponding to the connector */ > intel_connector_lookup_acpi_node(intel_connector); > + > + /* Check for integrated Privacy screen support */ > + if (intel_privacy_screen_present(intel_connector)) > + intel_attach_privacy_screen_property(connector); > } > } > > 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 000000000000..4c422e38c51a > --- /dev/null > +++ b/drivers/gpu/drm/i915/display/intel_privacy_screen.c > @@ -0,0 +1,70 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Intel ACPI privacy screen code > + * > + * Copyright © 2019 Google Inc. > + */ > + > +#include <linux/acpi.h> > + > +#include "intel_privacy_screen.h" > + > +#define CONNECTOR_DSM_REVID 1 > + > +#define CONNECTOR_DSM_FN_PRIVACY_ENABLE 2 > +#define CONNECTOR_DSM_FN_PRIVACY_DISABLE 3 > + > +static const guid_t drm_conn_dsm_guid = > + GUID_INIT(0xC7033113, 0x8720, 0x4CEB, > + 0x90, 0x90, 0x9D, 0x52, 0xB3, 0xE5, 0x2D, 0x73); > + > +/* Makes _DSM call to set privacy screen status */ > +static void acpi_privacy_screen_call_dsm(acpi_handle conn_handle, u64 > func) > +{ > + union acpi_object *obj; > + > + obj = acpi_evaluate_dsm(conn_handle, &drm_conn_dsm_guid, > + CONNECTOR_DSM_REVID, func, NULL); > + if (!obj) { > + DRM_DEBUG_DRIVER("failed to evaluate _DSM for fn %llx\n", > func); > + return; > + } > + > + ACPI_FREE(obj); > +} > + > +void intel_privacy_screen_set_val(struct intel_connector *intel_connector, > + enum intel_privacy_screen_status val) > +{ > + acpi_handle acpi_handle = intel_connector->acpi_handle; > + > + if (!acpi_handle) > + return; > + > + if (val == PRIVACY_SCREEN_DISABLED) > + acpi_privacy_screen_call_dsm(acpi_handle, > + > CONNECTOR_DSM_FN_PRIVACY_DISABLE); > + else if (val == PRIVACY_SCREEN_ENABLED) > + acpi_privacy_screen_call_dsm(acpi_handle, > + > CONNECTOR_DSM_FN_PRIVACY_ENABLE); > +} > + > +bool intel_privacy_screen_present(struct intel_connector *intel_connector) > +{ > + acpi_handle handle = intel_connector->acpi_handle; > + > + if (!handle) > + return false; > + > + if (!acpi_check_dsm(handle, &drm_conn_dsm_guid, > + CONNECTOR_DSM_REVID, > + 1 << CONNECTOR_DSM_FN_PRIVACY_ENABLE | > + 1 << CONNECTOR_DSM_FN_PRIVACY_DISABLE)) { > + DRM_WARN("%s: Odd, connector ACPI node but no privacy > scrn?\n", > + dev_name(intel_connector->base.dev->dev)); > + return false; > + } > + DRM_DEV_INFO(intel_connector->base.dev->dev, > + "supports privacy screen\n"); > + return true; > +} > diff --git a/drivers/gpu/drm/i915/display/intel_privacy_screen.h > b/drivers/gpu/drm/i915/display/intel_privacy_screen.h > new file mode 100644 > index 000000000000..212f73349a00 > --- /dev/null > +++ b/drivers/gpu/drm/i915/display/intel_privacy_screen.h > @@ -0,0 +1,25 @@ > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > +/* > + * Copyright © 2019 Google Inc. > + */ > + > +#ifndef __DRM_PRIVACY_SCREEN_H__ > +#define __DRM_PRIVACY_SCREEN_H__ > + > +#include "intel_display_types.h" > + > +#ifdef CONFIG_ACPI > +bool intel_privacy_screen_present(struct intel_connector > *intel_connector); > +void intel_privacy_screen_set_val(struct intel_connector *intel_connector, > + enum intel_privacy_screen_status val); > +#else > +bool intel_privacy_screen_present(struct intel_connector > *intel_connector); > +{ > + return false; > +} > +void intel_privacy_screen_set_val(struct intel_connector *intel_connector, > + enum intel_privacy_screen_status val) > +{ } > +#endif /* CONFIG_ACPI */ > + > +#endif /* __DRM_PRIVACY_SCREEN_H__ */ > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h > index 469dc512cca3..cf08d5636363 100644 > --- a/include/uapi/drm/i915_drm.h > +++ b/include/uapi/drm/i915_drm.h > @@ -2123,6 +2123,20 @@ struct drm_i915_query_engine_info { > struct drm_i915_engine_info engines[]; > }; > > +/** > + * enum intel_privacy_screen_status - privacy_screen status > + * > + * This enum is used to track and control the state of the integrated > privacy > + * screen present on some display panels, via the "privacy-screen" > property. > + * > + * @PRIVACY_SCREEN_DISABLED: The privacy-screen on the panel is disabled > + * @PRIVACY_SCREEN_ENABLED: The privacy-screen on the panel is enabled > + **/ > +enum intel_privacy_screen_status { > + PRIVACY_SCREEN_DISABLED = 0, > + PRIVACY_SCREEN_ENABLED = 1, > +}; > + > #if defined(__cplusplus) > } > #endif > -- > 2.24.0.rc1.363.gb1bccd3e3d-goog > >
On Mon, Nov 4, 2019 at 11:41 AM Rajat Jain <rajatja@google.com> 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 <rajatja@google.com> > 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 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, > connector->colorspace_property, 0); > } > + > +static const struct drm_prop_enum_list privacy_screen_enum[] = { > + { PRIVACY_SCREEN_DISABLED, "Disabled" }, > + { PRIVACY_SCREEN_ENABLED, "Enabled" }, > +}; > + > +/** > + * intel_attach_privacy_screen_property - > + * create and attach the connecter's privacy-screen property. * > + * @connector: connector for which to init the privacy-screen property > + * > + * This function creates and attaches the "privacy-screen" property to the > + * connector. Initial state of privacy-screen is set to disabled. > + */ > +void > +intel_attach_privacy_screen_property(struct drm_connector *connector) > +{ > + struct intel_connector *intel_connector = to_intel_connector(connector); > + struct drm_property *prop; > + > + if (!intel_connector->privacy_screen_property) { > + prop = drm_property_create_enum(connector->dev, > + DRM_MODE_PROP_ENUM, > + "privacy-screen", > + privacy_screen_enum, > + ARRAY_SIZE(privacy_screen_enum)); > + if (!prop) > + return; > + > + intel_connector->privacy_screen_property = prop; > + } > + > + drm_object_attach_property(&connector->base, prop, > + PRIVACY_SCREEN_DISABLED); > +} > diff --git a/drivers/gpu/drm/i915/display/intel_connector.h b/drivers/gpu/drm/i915/display/intel_connector.h > index 93a7375c8196..61005f37a338 100644 > --- a/drivers/gpu/drm/i915/display/intel_connector.h > +++ b/drivers/gpu/drm/i915/display/intel_connector.h > @@ -31,5 +31,6 @@ void intel_attach_force_audio_property(struct drm_connector *connector); > void intel_attach_broadcast_rgb_property(struct drm_connector *connector); > void intel_attach_aspect_ratio_property(struct drm_connector *connector); > void intel_attach_colorspace_property(struct drm_connector *connector); > +void intel_attach_privacy_screen_property(struct drm_connector *connector); > > #endif /* __INTEL_CONNECTOR_H__ */ > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h > index c2706afc069b..83b8c98049a7 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > @@ -426,6 +426,9 @@ struct intel_connector { > struct work_struct modeset_retry_work; > > struct intel_hdcp hdcp; > + > + /* Optional "privacy-screen" property for the connector panel */ > + struct drm_property *privacy_screen_property; > }; > > struct intel_digital_connector_state { > @@ -433,6 +436,7 @@ struct intel_digital_connector_state { > > enum hdmi_force_audio force_audio; > int broadcast_rgb; > + enum intel_privacy_screen_status privacy_screen_status; > }; > > #define to_intel_digital_connector_state(x) container_of(x, struct intel_digital_connector_state, base) > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > index 4fac408a4299..1963e92404ba 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" > @@ -6358,6 +6359,10 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect > > /* Lookup the ACPI node corresponding to the connector */ > intel_connector_lookup_acpi_node(intel_connector); > + > + /* Check for integrated Privacy screen support */ > + if (intel_privacy_screen_present(intel_connector)) > + intel_attach_privacy_screen_property(connector); > } > } > > 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 000000000000..4c422e38c51a > --- /dev/null > +++ b/drivers/gpu/drm/i915/display/intel_privacy_screen.c > @@ -0,0 +1,70 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Intel ACPI privacy screen code > + * > + * Copyright © 2019 Google Inc. > + */ > + > +#include <linux/acpi.h> > + > +#include "intel_privacy_screen.h" > + > +#define CONNECTOR_DSM_REVID 1 > + > +#define CONNECTOR_DSM_FN_PRIVACY_ENABLE 2 > +#define CONNECTOR_DSM_FN_PRIVACY_DISABLE 3 > + > +static const guid_t drm_conn_dsm_guid = > + GUID_INIT(0xC7033113, 0x8720, 0x4CEB, > + 0x90, 0x90, 0x9D, 0x52, 0xB3, 0xE5, 0x2D, 0x73); > + > +/* Makes _DSM call to set privacy screen status */ > +static void acpi_privacy_screen_call_dsm(acpi_handle conn_handle, u64 func) > +{ > + union acpi_object *obj; > + > + obj = acpi_evaluate_dsm(conn_handle, &drm_conn_dsm_guid, > + CONNECTOR_DSM_REVID, func, NULL); > + if (!obj) { > + DRM_DEBUG_DRIVER("failed to evaluate _DSM for fn %llx\n", func); > + return; > + } > + > + ACPI_FREE(obj); > +} > + > +void intel_privacy_screen_set_val(struct intel_connector *intel_connector, > + enum intel_privacy_screen_status val) > +{ > + acpi_handle acpi_handle = intel_connector->acpi_handle; > + > + if (!acpi_handle) > + return; > + > + if (val == PRIVACY_SCREEN_DISABLED) > + acpi_privacy_screen_call_dsm(acpi_handle, > + CONNECTOR_DSM_FN_PRIVACY_DISABLE); > + else if (val == PRIVACY_SCREEN_ENABLED) > + acpi_privacy_screen_call_dsm(acpi_handle, > + CONNECTOR_DSM_FN_PRIVACY_ENABLE); > +} > + > +bool intel_privacy_screen_present(struct intel_connector *intel_connector) > +{ > + acpi_handle handle = intel_connector->acpi_handle; > + > + if (!handle) > + return false; > + > + if (!acpi_check_dsm(handle, &drm_conn_dsm_guid, > + CONNECTOR_DSM_REVID, > + 1 << CONNECTOR_DSM_FN_PRIVACY_ENABLE | > + 1 << CONNECTOR_DSM_FN_PRIVACY_DISABLE)) { > + DRM_WARN("%s: Odd, connector ACPI node but no privacy scrn?\n", > + dev_name(intel_connector->base.dev->dev)); > + return false; > + } > + DRM_DEV_INFO(intel_connector->base.dev->dev, > + "supports privacy screen\n"); > + return true; > +} > diff --git a/drivers/gpu/drm/i915/display/intel_privacy_screen.h b/drivers/gpu/drm/i915/display/intel_privacy_screen.h > new file mode 100644 > index 000000000000..212f73349a00 > --- /dev/null > +++ b/drivers/gpu/drm/i915/display/intel_privacy_screen.h > @@ -0,0 +1,25 @@ > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > +/* > + * Copyright © 2019 Google Inc. > + */ > + > +#ifndef __DRM_PRIVACY_SCREEN_H__ > +#define __DRM_PRIVACY_SCREEN_H__ > + > +#include "intel_display_types.h" > + > +#ifdef CONFIG_ACPI > +bool intel_privacy_screen_present(struct intel_connector *intel_connector); > +void intel_privacy_screen_set_val(struct intel_connector *intel_connector, > + enum intel_privacy_screen_status val); > +#else > +bool intel_privacy_screen_present(struct intel_connector *intel_connector); > +{ > + return false; > +} > +void intel_privacy_screen_set_val(struct intel_connector *intel_connector, > + enum intel_privacy_screen_status val) > +{ } > +#endif /* CONFIG_ACPI */ > + > +#endif /* __DRM_PRIVACY_SCREEN_H__ */ > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h > index 469dc512cca3..cf08d5636363 100644 > --- a/include/uapi/drm/i915_drm.h > +++ b/include/uapi/drm/i915_drm.h > @@ -2123,6 +2123,20 @@ struct drm_i915_query_engine_info { > struct drm_i915_engine_info engines[]; > }; > > +/** > + * enum intel_privacy_screen_status - privacy_screen status > + * > + * This enum is used to track and control the state of the integrated privacy > + * screen present on some display panels, via the "privacy-screen" property. > + * > + * @PRIVACY_SCREEN_DISABLED: The privacy-screen on the panel is disabled > + * @PRIVACY_SCREEN_ENABLED: The privacy-screen on the panel is enabled > + **/ > +enum intel_privacy_screen_status { > + PRIVACY_SCREEN_DISABLED = 0, > + PRIVACY_SCREEN_ENABLED = 1, > +}; > + > #if defined(__cplusplus) > } > #endif > -- > 2.24.0.rc1.363.gb1bccd3e3d-goog >
On Mon, 04 Nov 2019, Rajat Jain <rajatja@google.com> 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 <rajatja@google.com> > 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_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, > connector->colorspace_property, 0); > } > + > +static const struct drm_prop_enum_list privacy_screen_enum[] = { > + { PRIVACY_SCREEN_DISABLED, "Disabled" }, > + { PRIVACY_SCREEN_ENABLED, "Enabled" }, > +}; > + > +/** > + * intel_attach_privacy_screen_property - > + * create and attach the connecter's privacy-screen property. * > + * @connector: connector for which to init the privacy-screen property > + * > + * This function creates and attaches the "privacy-screen" property to the > + * connector. Initial state of privacy-screen is set to disabled. > + */ > +void > +intel_attach_privacy_screen_property(struct drm_connector *connector) > +{ > + struct intel_connector *intel_connector = to_intel_connector(connector); > + struct drm_property *prop; > + > + if (!intel_connector->privacy_screen_property) { > + prop = drm_property_create_enum(connector->dev, > + DRM_MODE_PROP_ENUM, > + "privacy-screen", > + privacy_screen_enum, > + ARRAY_SIZE(privacy_screen_enum)); > + if (!prop) > + return; > + > + intel_connector->privacy_screen_property = prop; > + } > + > + drm_object_attach_property(&connector->base, prop, > + PRIVACY_SCREEN_DISABLED); > +} I think this should be a drm core level property in drm_connector.[ch] so that *all* drivers would use the same thing for privacy screens. Not i915 specific. I think this is the biggest issue in the patch series. > diff --git a/drivers/gpu/drm/i915/display/intel_connector.h b/drivers/gpu/drm/i915/display/intel_connector.h > index 93a7375c8196..61005f37a338 100644 > --- a/drivers/gpu/drm/i915/display/intel_connector.h > +++ b/drivers/gpu/drm/i915/display/intel_connector.h > @@ -31,5 +31,6 @@ void intel_attach_force_audio_property(struct drm_connector *connector); > void intel_attach_broadcast_rgb_property(struct drm_connector *connector); > void intel_attach_aspect_ratio_property(struct drm_connector *connector); > void intel_attach_colorspace_property(struct drm_connector *connector); > +void intel_attach_privacy_screen_property(struct drm_connector *connector); > > #endif /* __INTEL_CONNECTOR_H__ */ > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h > index c2706afc069b..83b8c98049a7 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > @@ -426,6 +426,9 @@ struct intel_connector { > struct work_struct modeset_retry_work; > > struct intel_hdcp hdcp; > + > + /* Optional "privacy-screen" property for the connector panel */ > + struct drm_property *privacy_screen_property; > }; > > struct intel_digital_connector_state { > @@ -433,6 +436,7 @@ struct intel_digital_connector_state { > > enum hdmi_force_audio force_audio; > int broadcast_rgb; > + enum intel_privacy_screen_status privacy_screen_status; > }; > > #define to_intel_digital_connector_state(x) container_of(x, struct intel_digital_connector_state, base) > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > index 4fac408a4299..1963e92404ba 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" > @@ -6358,6 +6359,10 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect > > /* Lookup the ACPI node corresponding to the connector */ > intel_connector_lookup_acpi_node(intel_connector); > + > + /* Check for integrated Privacy screen support */ > + if (intel_privacy_screen_present(intel_connector)) > + intel_attach_privacy_screen_property(connector); > } > } > > 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 000000000000..4c422e38c51a > --- /dev/null > +++ b/drivers/gpu/drm/i915/display/intel_privacy_screen.c > @@ -0,0 +1,70 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later Please read http://mid.mail-archive.com/CAKMK7uH-8+tbKsAoiChsxELEc_77RVVxP2wapHWhqB+0Viifog@mail.gmail.com > +/* > + * Intel ACPI privacy screen code > + * > + * Copyright © 2019 Google Inc. > + */ > + > +#include <linux/acpi.h> > + > +#include "intel_privacy_screen.h" > + > +#define CONNECTOR_DSM_REVID 1 > + > +#define CONNECTOR_DSM_FN_PRIVACY_ENABLE 2 > +#define CONNECTOR_DSM_FN_PRIVACY_DISABLE 3 > + > +static const guid_t drm_conn_dsm_guid = > + GUID_INIT(0xC7033113, 0x8720, 0x4CEB, > + 0x90, 0x90, 0x9D, 0x52, 0xB3, 0xE5, 0x2D, 0x73); > + > +/* Makes _DSM call to set privacy screen status */ > +static void acpi_privacy_screen_call_dsm(acpi_handle conn_handle, u64 func) > +{ > + union acpi_object *obj; > + > + obj = acpi_evaluate_dsm(conn_handle, &drm_conn_dsm_guid, > + CONNECTOR_DSM_REVID, func, NULL); > + if (!obj) { > + DRM_DEBUG_DRIVER("failed to evaluate _DSM for fn %llx\n", func); > + return; > + } > + > + ACPI_FREE(obj); > +} > + > +void intel_privacy_screen_set_val(struct intel_connector *intel_connector, > + enum intel_privacy_screen_status val) Just name the parameter connector, not intel_connector. This throughout. > +{ > + acpi_handle acpi_handle = intel_connector->acpi_handle; > + > + if (!acpi_handle) > + return; > + > + if (val == PRIVACY_SCREEN_DISABLED) > + acpi_privacy_screen_call_dsm(acpi_handle, > + CONNECTOR_DSM_FN_PRIVACY_DISABLE); > + else if (val == PRIVACY_SCREEN_ENABLED) > + acpi_privacy_screen_call_dsm(acpi_handle, > + CONNECTOR_DSM_FN_PRIVACY_ENABLE); else complain? > +} > + > +bool intel_privacy_screen_present(struct intel_connector *intel_connector) > +{ > + acpi_handle handle = intel_connector->acpi_handle; > + > + if (!handle) > + return false; > + > + if (!acpi_check_dsm(handle, &drm_conn_dsm_guid, > + CONNECTOR_DSM_REVID, > + 1 << CONNECTOR_DSM_FN_PRIVACY_ENABLE | > + 1 << CONNECTOR_DSM_FN_PRIVACY_DISABLE)) { > + DRM_WARN("%s: Odd, connector ACPI node but no privacy scrn?\n", > + dev_name(intel_connector->base.dev->dev)); > + return false; > + } > + DRM_DEV_INFO(intel_connector->base.dev->dev, > + "supports privacy screen\n"); > + return true; > +} > diff --git a/drivers/gpu/drm/i915/display/intel_privacy_screen.h b/drivers/gpu/drm/i915/display/intel_privacy_screen.h > new file mode 100644 > index 000000000000..212f73349a00 > --- /dev/null > +++ b/drivers/gpu/drm/i915/display/intel_privacy_screen.h > @@ -0,0 +1,25 @@ > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > +/* > + * Copyright © 2019 Google Inc. > + */ > + > +#ifndef __DRM_PRIVACY_SCREEN_H__ > +#define __DRM_PRIVACY_SCREEN_H__ > + > +#include "intel_display_types.h" > + > +#ifdef CONFIG_ACPI > +bool intel_privacy_screen_present(struct intel_connector *intel_connector); > +void intel_privacy_screen_set_val(struct intel_connector *intel_connector, > + enum intel_privacy_screen_status val); > +#else > +bool intel_privacy_screen_present(struct intel_connector *intel_connector); > +{ > + return false; > +} > +void intel_privacy_screen_set_val(struct intel_connector *intel_connector, > + enum intel_privacy_screen_status val) > +{ } > +#endif /* CONFIG_ACPI */ > + > +#endif /* __DRM_PRIVACY_SCREEN_H__ */ > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h > index 469dc512cca3..cf08d5636363 100644 > --- a/include/uapi/drm/i915_drm.h > +++ b/include/uapi/drm/i915_drm.h > @@ -2123,6 +2123,20 @@ struct drm_i915_query_engine_info { > struct drm_i915_engine_info engines[]; > }; > > +/** > + * enum intel_privacy_screen_status - privacy_screen status > + * > + * This enum is used to track and control the state of the integrated privacy > + * screen present on some display panels, via the "privacy-screen" property. > + * > + * @PRIVACY_SCREEN_DISABLED: The privacy-screen on the panel is disabled > + * @PRIVACY_SCREEN_ENABLED: The privacy-screen on the panel is enabled > + **/ > +enum intel_privacy_screen_status { > + PRIVACY_SCREEN_DISABLED = 0, > + PRIVACY_SCREEN_ENABLED = 1, > +}; > + The drm_property interface UAPI is based on the strings, *not* on the values. Please move the enum out of uapi into the drm code. BR, jani. > #if defined(__cplusplus) > } > #endif
On Tue, 12 Nov 2019, Rajat Jain <rajatja@google.com> wrote: > On Mon, Nov 4, 2019 at 11:41 AM Rajat Jain <rajatja@google.com> 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 <rajatja@google.com> >> 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. 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. 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 = >> 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, >> connector->colorspace_property, 0); >> } >> + >> +static const struct drm_prop_enum_list privacy_screen_enum[] = { >> + { PRIVACY_SCREEN_DISABLED, "Disabled" }, >> + { PRIVACY_SCREEN_ENABLED, "Enabled" }, >> +}; >> + >> +/** >> + * intel_attach_privacy_screen_property - >> + * create and attach the connecter's privacy-screen property. * >> + * @connector: connector for which to init the privacy-screen property >> + * >> + * This function creates and attaches the "privacy-screen" property to the >> + * connector. Initial state of privacy-screen is set to disabled. >> + */ >> +void >> +intel_attach_privacy_screen_property(struct drm_connector *connector) >> +{ >> + struct intel_connector *intel_connector = to_intel_connector(connector); >> + struct drm_property *prop; >> + >> + if (!intel_connector->privacy_screen_property) { >> + prop = drm_property_create_enum(connector->dev, >> + DRM_MODE_PROP_ENUM, >> + "privacy-screen", >> + privacy_screen_enum, >> + ARRAY_SIZE(privacy_screen_enum)); >> + if (!prop) >> + return; >> + >> + intel_connector->privacy_screen_property = prop; >> + } >> + >> + drm_object_attach_property(&connector->base, prop, >> + PRIVACY_SCREEN_DISABLED); >> +} >> diff --git a/drivers/gpu/drm/i915/display/intel_connector.h b/drivers/gpu/drm/i915/display/intel_connector.h >> index 93a7375c8196..61005f37a338 100644 >> --- a/drivers/gpu/drm/i915/display/intel_connector.h >> +++ b/drivers/gpu/drm/i915/display/intel_connector.h >> @@ -31,5 +31,6 @@ void intel_attach_force_audio_property(struct drm_connector *connector); >> void intel_attach_broadcast_rgb_property(struct drm_connector *connector); >> void intel_attach_aspect_ratio_property(struct drm_connector *connector); >> void intel_attach_colorspace_property(struct drm_connector *connector); >> +void intel_attach_privacy_screen_property(struct drm_connector *connector); >> >> #endif /* __INTEL_CONNECTOR_H__ */ >> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h >> index c2706afc069b..83b8c98049a7 100644 >> --- a/drivers/gpu/drm/i915/display/intel_display_types.h >> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h >> @@ -426,6 +426,9 @@ struct intel_connector { >> struct work_struct modeset_retry_work; >> >> struct intel_hdcp hdcp; >> + >> + /* Optional "privacy-screen" property for the connector panel */ >> + struct drm_property *privacy_screen_property; >> }; >> >> struct intel_digital_connector_state { >> @@ -433,6 +436,7 @@ struct intel_digital_connector_state { >> >> enum hdmi_force_audio force_audio; >> int broadcast_rgb; >> + enum intel_privacy_screen_status privacy_screen_status; >> }; >> >> #define to_intel_digital_connector_state(x) container_of(x, struct intel_digital_connector_state, base) >> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c >> index 4fac408a4299..1963e92404ba 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" >> @@ -6358,6 +6359,10 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect >> >> /* Lookup the ACPI node corresponding to the connector */ >> intel_connector_lookup_acpi_node(intel_connector); >> + >> + /* Check for integrated Privacy screen support */ >> + if (intel_privacy_screen_present(intel_connector)) >> + intel_attach_privacy_screen_property(connector); >> } >> } >> >> 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 000000000000..4c422e38c51a >> --- /dev/null >> +++ b/drivers/gpu/drm/i915/display/intel_privacy_screen.c >> @@ -0,0 +1,70 @@ >> +// SPDX-License-Identifier: GPL-2.0-or-later >> +/* >> + * Intel ACPI privacy screen code >> + * >> + * Copyright © 2019 Google Inc. >> + */ >> + >> +#include <linux/acpi.h> >> + >> +#include "intel_privacy_screen.h" >> + >> +#define CONNECTOR_DSM_REVID 1 >> + >> +#define CONNECTOR_DSM_FN_PRIVACY_ENABLE 2 >> +#define CONNECTOR_DSM_FN_PRIVACY_DISABLE 3 >> + >> +static const guid_t drm_conn_dsm_guid = >> + GUID_INIT(0xC7033113, 0x8720, 0x4CEB, >> + 0x90, 0x90, 0x9D, 0x52, 0xB3, 0xE5, 0x2D, 0x73); >> + >> +/* Makes _DSM call to set privacy screen status */ >> +static void acpi_privacy_screen_call_dsm(acpi_handle conn_handle, u64 func) >> +{ >> + union acpi_object *obj; >> + >> + obj = acpi_evaluate_dsm(conn_handle, &drm_conn_dsm_guid, >> + CONNECTOR_DSM_REVID, func, NULL); >> + if (!obj) { >> + DRM_DEBUG_DRIVER("failed to evaluate _DSM for fn %llx\n", func); >> + return; >> + } >> + >> + ACPI_FREE(obj); >> +} >> + >> +void intel_privacy_screen_set_val(struct intel_connector *intel_connector, >> + enum intel_privacy_screen_status val) >> +{ >> + acpi_handle acpi_handle = intel_connector->acpi_handle; >> + >> + if (!acpi_handle) >> + return; >> + >> + if (val == PRIVACY_SCREEN_DISABLED) >> + acpi_privacy_screen_call_dsm(acpi_handle, >> + CONNECTOR_DSM_FN_PRIVACY_DISABLE); >> + else if (val == PRIVACY_SCREEN_ENABLED) >> + acpi_privacy_screen_call_dsm(acpi_handle, >> + CONNECTOR_DSM_FN_PRIVACY_ENABLE); >> +} >> + >> +bool intel_privacy_screen_present(struct intel_connector *intel_connector) >> +{ >> + acpi_handle handle = intel_connector->acpi_handle; >> + >> + if (!handle) >> + return false; >> + >> + if (!acpi_check_dsm(handle, &drm_conn_dsm_guid, >> + CONNECTOR_DSM_REVID, >> + 1 << CONNECTOR_DSM_FN_PRIVACY_ENABLE | >> + 1 << CONNECTOR_DSM_FN_PRIVACY_DISABLE)) { >> + DRM_WARN("%s: Odd, connector ACPI node but no privacy scrn?\n", >> + dev_name(intel_connector->base.dev->dev)); >> + return false; >> + } >> + DRM_DEV_INFO(intel_connector->base.dev->dev, >> + "supports privacy screen\n"); >> + return true; >> +} >> diff --git a/drivers/gpu/drm/i915/display/intel_privacy_screen.h b/drivers/gpu/drm/i915/display/intel_privacy_screen.h >> new file mode 100644 >> index 000000000000..212f73349a00 >> --- /dev/null >> +++ b/drivers/gpu/drm/i915/display/intel_privacy_screen.h >> @@ -0,0 +1,25 @@ >> +/* SPDX-License-Identifier: GPL-2.0-or-later */ >> +/* >> + * Copyright © 2019 Google Inc. >> + */ >> + >> +#ifndef __DRM_PRIVACY_SCREEN_H__ >> +#define __DRM_PRIVACY_SCREEN_H__ >> + >> +#include "intel_display_types.h" >> + >> +#ifdef CONFIG_ACPI >> +bool intel_privacy_screen_present(struct intel_connector *intel_connector); >> +void intel_privacy_screen_set_val(struct intel_connector *intel_connector, >> + enum intel_privacy_screen_status val); >> +#else >> +bool intel_privacy_screen_present(struct intel_connector *intel_connector); >> +{ >> + return false; >> +} >> +void intel_privacy_screen_set_val(struct intel_connector *intel_connector, >> + enum intel_privacy_screen_status val) >> +{ } >> +#endif /* CONFIG_ACPI */ >> + >> +#endif /* __DRM_PRIVACY_SCREEN_H__ */ >> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h >> index 469dc512cca3..cf08d5636363 100644 >> --- a/include/uapi/drm/i915_drm.h >> +++ b/include/uapi/drm/i915_drm.h >> @@ -2123,6 +2123,20 @@ struct drm_i915_query_engine_info { >> struct drm_i915_engine_info engines[]; >> }; >> >> +/** >> + * enum intel_privacy_screen_status - privacy_screen status >> + * >> + * This enum is used to track and control the state of the integrated privacy >> + * screen present on some display panels, via the "privacy-screen" property. >> + * >> + * @PRIVACY_SCREEN_DISABLED: The privacy-screen on the panel is disabled >> + * @PRIVACY_SCREEN_ENABLED: The privacy-screen on the panel is enabled >> + **/ >> +enum intel_privacy_screen_status { >> + PRIVACY_SCREEN_DISABLED = 0, >> + PRIVACY_SCREEN_ENABLED = 1, >> +}; >> + >> #if defined(__cplusplus) >> } >> #endif >> -- >> 2.24.0.rc1.363.gb1bccd3e3d-goog >>
Hi Jani, On Wed, Nov 20, 2019 at 7:11 AM Jani Nikula <jani.nikula@linux.intel.com> wrote: > > On Tue, 12 Nov 2019, Rajat Jain <rajatja@google.com> wrote: > > On Mon, Nov 4, 2019 at 11:41 AM Rajat Jain <rajatja@google.com> 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 <rajatja@google.com> > >> 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 = > >> 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, > >> connector->colorspace_property, 0); > >> } > >> + > >> +static const struct drm_prop_enum_list privacy_screen_enum[] = { > >> + { PRIVACY_SCREEN_DISABLED, "Disabled" }, > >> + { PRIVACY_SCREEN_ENABLED, "Enabled" }, > >> +}; > >> + > >> +/** > >> + * intel_attach_privacy_screen_property - > >> + * create and attach the connecter's privacy-screen property. * > >> + * @connector: connector for which to init the privacy-screen property > >> + * > >> + * This function creates and attaches the "privacy-screen" property to the > >> + * connector. Initial state of privacy-screen is set to disabled. > >> + */ > >> +void > >> +intel_attach_privacy_screen_property(struct drm_connector *connector) > >> +{ > >> + struct intel_connector *intel_connector = to_intel_connector(connector); > >> + struct drm_property *prop; > >> + > >> + if (!intel_connector->privacy_screen_property) { > >> + prop = drm_property_create_enum(connector->dev, > >> + DRM_MODE_PROP_ENUM, > >> + "privacy-screen", > >> + privacy_screen_enum, > >> + ARRAY_SIZE(privacy_screen_enum)); > >> + if (!prop) > >> + return; > >> + > >> + intel_connector->privacy_screen_property = prop; > >> + } > >> + > >> + drm_object_attach_property(&connector->base, prop, > >> + PRIVACY_SCREEN_DISABLED); > >> +} > >> diff --git a/drivers/gpu/drm/i915/display/intel_connector.h b/drivers/gpu/drm/i915/display/intel_connector.h > >> index 93a7375c8196..61005f37a338 100644 > >> --- a/drivers/gpu/drm/i915/display/intel_connector.h > >> +++ b/drivers/gpu/drm/i915/display/intel_connector.h > >> @@ -31,5 +31,6 @@ void intel_attach_force_audio_property(struct drm_connector *connector); > >> void intel_attach_broadcast_rgb_property(struct drm_connector *connector); > >> void intel_attach_aspect_ratio_property(struct drm_connector *connector); > >> void intel_attach_colorspace_property(struct drm_connector *connector); > >> +void intel_attach_privacy_screen_property(struct drm_connector *connector); > >> > >> #endif /* __INTEL_CONNECTOR_H__ */ > >> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h > >> index c2706afc069b..83b8c98049a7 100644 > >> --- a/drivers/gpu/drm/i915/display/intel_display_types.h > >> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > >> @@ -426,6 +426,9 @@ struct intel_connector { > >> struct work_struct modeset_retry_work; > >> > >> struct intel_hdcp hdcp; > >> + > >> + /* Optional "privacy-screen" property for the connector panel */ > >> + struct drm_property *privacy_screen_property; > >> }; > >> > >> struct intel_digital_connector_state { > >> @@ -433,6 +436,7 @@ struct intel_digital_connector_state { > >> > >> enum hdmi_force_audio force_audio; > >> int broadcast_rgb; > >> + enum intel_privacy_screen_status privacy_screen_status; > >> }; > >> > >> #define to_intel_digital_connector_state(x) container_of(x, struct intel_digital_connector_state, base) > >> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > >> index 4fac408a4299..1963e92404ba 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" > >> @@ -6358,6 +6359,10 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect > >> > >> /* Lookup the ACPI node corresponding to the connector */ > >> intel_connector_lookup_acpi_node(intel_connector); > >> + > >> + /* Check for integrated Privacy screen support */ > >> + if (intel_privacy_screen_present(intel_connector)) > >> + intel_attach_privacy_screen_property(connector); > >> } > >> } > >> > >> 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 000000000000..4c422e38c51a > >> --- /dev/null > >> +++ b/drivers/gpu/drm/i915/display/intel_privacy_screen.c > >> @@ -0,0 +1,70 @@ > >> +// SPDX-License-Identifier: GPL-2.0-or-later > >> +/* > >> + * Intel ACPI privacy screen code > >> + * > >> + * Copyright © 2019 Google Inc. > >> + */ > >> + > >> +#include <linux/acpi.h> > >> + > >> +#include "intel_privacy_screen.h" > >> + > >> +#define CONNECTOR_DSM_REVID 1 > >> + > >> +#define CONNECTOR_DSM_FN_PRIVACY_ENABLE 2 > >> +#define CONNECTOR_DSM_FN_PRIVACY_DISABLE 3 > >> + > >> +static const guid_t drm_conn_dsm_guid = > >> + GUID_INIT(0xC7033113, 0x8720, 0x4CEB, > >> + 0x90, 0x90, 0x9D, 0x52, 0xB3, 0xE5, 0x2D, 0x73); > >> + > >> +/* Makes _DSM call to set privacy screen status */ > >> +static void acpi_privacy_screen_call_dsm(acpi_handle conn_handle, u64 func) > >> +{ > >> + union acpi_object *obj; > >> + > >> + obj = acpi_evaluate_dsm(conn_handle, &drm_conn_dsm_guid, > >> + CONNECTOR_DSM_REVID, func, NULL); > >> + if (!obj) { > >> + DRM_DEBUG_DRIVER("failed to evaluate _DSM for fn %llx\n", func); > >> + return; > >> + } > >> + > >> + ACPI_FREE(obj); > >> +} > >> + > >> +void intel_privacy_screen_set_val(struct intel_connector *intel_connector, > >> + enum intel_privacy_screen_status val) > >> +{ > >> + acpi_handle acpi_handle = intel_connector->acpi_handle; > >> + > >> + if (!acpi_handle) > >> + return; > >> + > >> + if (val == PRIVACY_SCREEN_DISABLED) > >> + acpi_privacy_screen_call_dsm(acpi_handle, > >> + CONNECTOR_DSM_FN_PRIVACY_DISABLE); > >> + else if (val == PRIVACY_SCREEN_ENABLED) > >> + acpi_privacy_screen_call_dsm(acpi_handle, > >> + CONNECTOR_DSM_FN_PRIVACY_ENABLE); > >> +} > >> + > >> +bool intel_privacy_screen_present(struct intel_connector *intel_connector) > >> +{ > >> + acpi_handle handle = intel_connector->acpi_handle; > >> + > >> + if (!handle) > >> + return false; > >> + > >> + if (!acpi_check_dsm(handle, &drm_conn_dsm_guid, > >> + CONNECTOR_DSM_REVID, > >> + 1 << CONNECTOR_DSM_FN_PRIVACY_ENABLE | > >> + 1 << CONNECTOR_DSM_FN_PRIVACY_DISABLE)) { > >> + DRM_WARN("%s: Odd, connector ACPI node but no privacy scrn?\n", > >> + dev_name(intel_connector->base.dev->dev)); > >> + return false; > >> + } > >> + DRM_DEV_INFO(intel_connector->base.dev->dev, > >> + "supports privacy screen\n"); > >> + return true; > >> +} > >> diff --git a/drivers/gpu/drm/i915/display/intel_privacy_screen.h b/drivers/gpu/drm/i915/display/intel_privacy_screen.h > >> new file mode 100644 > >> index 000000000000..212f73349a00 > >> --- /dev/null > >> +++ b/drivers/gpu/drm/i915/display/intel_privacy_screen.h > >> @@ -0,0 +1,25 @@ > >> +/* SPDX-License-Identifier: GPL-2.0-or-later */ > >> +/* > >> + * Copyright © 2019 Google Inc. > >> + */ > >> + > >> +#ifndef __DRM_PRIVACY_SCREEN_H__ > >> +#define __DRM_PRIVACY_SCREEN_H__ > >> + > >> +#include "intel_display_types.h" > >> + > >> +#ifdef CONFIG_ACPI > >> +bool intel_privacy_screen_present(struct intel_connector *intel_connector); > >> +void intel_privacy_screen_set_val(struct intel_connector *intel_connector, > >> + enum intel_privacy_screen_status val); > >> +#else > >> +bool intel_privacy_screen_present(struct intel_connector *intel_connector); > >> +{ > >> + return false; > >> +} > >> +void intel_privacy_screen_set_val(struct intel_connector *intel_connector, > >> + enum intel_privacy_screen_status val) > >> +{ } > >> +#endif /* CONFIG_ACPI */ > >> + > >> +#endif /* __DRM_PRIVACY_SCREEN_H__ */ > >> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h > >> index 469dc512cca3..cf08d5636363 100644 > >> --- a/include/uapi/drm/i915_drm.h > >> +++ b/include/uapi/drm/i915_drm.h > >> @@ -2123,6 +2123,20 @@ struct drm_i915_query_engine_info { > >> struct drm_i915_engine_info engines[]; > >> }; > >> > >> +/** > >> + * enum intel_privacy_screen_status - privacy_screen status > >> + * > >> + * This enum is used to track and control the state of the integrated privacy > >> + * screen present on some display panels, via the "privacy-screen" property. > >> + * > >> + * @PRIVACY_SCREEN_DISABLED: The privacy-screen on the panel is disabled > >> + * @PRIVACY_SCREEN_ENABLED: The privacy-screen on the panel is enabled > >> + **/ > >> +enum intel_privacy_screen_status { > >> + PRIVACY_SCREEN_DISABLED = 0, > >> + PRIVACY_SCREEN_ENABLED = 1, > >> +}; > >> + > >> #if defined(__cplusplus) > >> } > >> #endif > >> -- > >> 2.24.0.rc1.363.gb1bccd3e3d-goog > >> > > -- > Jani Nikula, Intel Open Source Graphics Center
On Wed, Nov 20, 2019 at 7:04 AM Jani Nikula <jani.nikula@linux.intel.com> wrote: > > On Mon, 04 Nov 2019, Rajat Jain <rajatja@google.com> 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 <rajatja@google.com> > > 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_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, > > connector->colorspace_property, 0); > > } > > + > > +static const struct drm_prop_enum_list privacy_screen_enum[] = { > > + { PRIVACY_SCREEN_DISABLED, "Disabled" }, > > + { PRIVACY_SCREEN_ENABLED, "Enabled" }, > > +}; > > + > > +/** > > + * intel_attach_privacy_screen_property - > > + * create and attach the connecter's privacy-screen property. * > > + * @connector: connector for which to init the privacy-screen property > > + * > > + * This function creates and attaches the "privacy-screen" property to the > > + * connector. Initial state of privacy-screen is set to disabled. > > + */ > > +void > > +intel_attach_privacy_screen_property(struct drm_connector *connector) > > +{ > > + struct intel_connector *intel_connector = to_intel_connector(connector); > > + struct drm_property *prop; > > + > > + if (!intel_connector->privacy_screen_property) { > > + prop = drm_property_create_enum(connector->dev, > > + DRM_MODE_PROP_ENUM, > > + "privacy-screen", > > + privacy_screen_enum, > > + ARRAY_SIZE(privacy_screen_enum)); > > + if (!prop) > > + return; > > + > > + intel_connector->privacy_screen_property = prop; > > + } > > + > > + drm_object_attach_property(&connector->base, prop, > > + PRIVACY_SCREEN_DISABLED); > > +} > > I think this should be a drm core level property in drm_connector.[ch] > so that *all* drivers would use the same thing for privacy screens. Not > i915 specific. > > I think this is the biggest issue in the patch series. I actually would be happy to make it a drm_connector property, like I had in my original patch series. I changed it to i915 specific because it seemed to me based on the comments that the general sentiment is that anything to do with acpi should be in i915. Note that the privacy screen property essentially needs an ACPI handle to work on, so if I were to move the property into drm_connector, I'd likely rename the intel_privacy_screen* code to dem_privacy_screen* code, and it'll still operate on an ACPI handle (stored in drm_connector structure). The i915's job will then be to lookup that ACPI handle (because lookup requires display index) and populate it in drm_connector. Does this sound OK? > > > diff --git a/drivers/gpu/drm/i915/display/intel_connector.h b/drivers/gpu/drm/i915/display/intel_connector.h > > index 93a7375c8196..61005f37a338 100644 > > --- a/drivers/gpu/drm/i915/display/intel_connector.h > > +++ b/drivers/gpu/drm/i915/display/intel_connector.h > > @@ -31,5 +31,6 @@ void intel_attach_force_audio_property(struct drm_connector *connector); > > void intel_attach_broadcast_rgb_property(struct drm_connector *connector); > > void intel_attach_aspect_ratio_property(struct drm_connector *connector); > > void intel_attach_colorspace_property(struct drm_connector *connector); > > +void intel_attach_privacy_screen_property(struct drm_connector *connector); > > > > #endif /* __INTEL_CONNECTOR_H__ */ > > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h > > index c2706afc069b..83b8c98049a7 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > > @@ -426,6 +426,9 @@ struct intel_connector { > > struct work_struct modeset_retry_work; > > > > struct intel_hdcp hdcp; > > + > > + /* Optional "privacy-screen" property for the connector panel */ > > + struct drm_property *privacy_screen_property; > > }; > > > > struct intel_digital_connector_state { > > @@ -433,6 +436,7 @@ struct intel_digital_connector_state { > > > > enum hdmi_force_audio force_audio; > > int broadcast_rgb; > > + enum intel_privacy_screen_status privacy_screen_status; > > }; > > > > #define to_intel_digital_connector_state(x) container_of(x, struct intel_digital_connector_state, base) > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > > index 4fac408a4299..1963e92404ba 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" > > @@ -6358,6 +6359,10 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect > > > > /* Lookup the ACPI node corresponding to the connector */ > > intel_connector_lookup_acpi_node(intel_connector); > > + > > + /* Check for integrated Privacy screen support */ > > + if (intel_privacy_screen_present(intel_connector)) > > + intel_attach_privacy_screen_property(connector); > > } > > } > > > > 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 000000000000..4c422e38c51a > > --- /dev/null > > +++ b/drivers/gpu/drm/i915/display/intel_privacy_screen.c > > @@ -0,0 +1,70 @@ > > +// SPDX-License-Identifier: GPL-2.0-or-later > > Please read http://mid.mail-archive.com/CAKMK7uH-8+tbKsAoiChsxELEc_77RVVxP2wapHWhqB+0Viifog@mail.gmail.com OK, I changed it to SPDX-License-Identifier: GPL-2.0 OR MIT > > > +/* > > + * Intel ACPI privacy screen code > > + * > > + * Copyright © 2019 Google Inc. > > + */ > > + > > +#include <linux/acpi.h> > > + > > +#include "intel_privacy_screen.h" > > + > > +#define CONNECTOR_DSM_REVID 1 > > + > > +#define CONNECTOR_DSM_FN_PRIVACY_ENABLE 2 > > +#define CONNECTOR_DSM_FN_PRIVACY_DISABLE 3 > > + > > +static const guid_t drm_conn_dsm_guid = > > + GUID_INIT(0xC7033113, 0x8720, 0x4CEB, > > + 0x90, 0x90, 0x9D, 0x52, 0xB3, 0xE5, 0x2D, 0x73); > > + > > +/* Makes _DSM call to set privacy screen status */ > > +static void acpi_privacy_screen_call_dsm(acpi_handle conn_handle, u64 func) > > +{ > > + union acpi_object *obj; > > + > > + obj = acpi_evaluate_dsm(conn_handle, &drm_conn_dsm_guid, > > + CONNECTOR_DSM_REVID, func, NULL); > > + if (!obj) { > > + DRM_DEBUG_DRIVER("failed to evaluate _DSM for fn %llx\n", func); > > + return; > > + } > > + > > + ACPI_FREE(obj); > > +} > > + > > +void intel_privacy_screen_set_val(struct intel_connector *intel_connector, > > + enum intel_privacy_screen_status val) > > Just name the parameter connector, not intel_connector. This throughout. Done. > > > +{ > > + acpi_handle acpi_handle = intel_connector->acpi_handle; > > + > > + if (!acpi_handle) > > + return; > > + > > + if (val == PRIVACY_SCREEN_DISABLED) > > + acpi_privacy_screen_call_dsm(acpi_handle, > > + CONNECTOR_DSM_FN_PRIVACY_DISABLE); > > + else if (val == PRIVACY_SCREEN_ENABLED) > > + acpi_privacy_screen_call_dsm(acpi_handle, > > + CONNECTOR_DSM_FN_PRIVACY_ENABLE); > > else complain? > Done. > > +} > > + > > +bool intel_privacy_screen_present(struct intel_connector *intel_connector) > > +{ > > + acpi_handle handle = intel_connector->acpi_handle; > > + > > + if (!handle) > > + return false; > > + > > + if (!acpi_check_dsm(handle, &drm_conn_dsm_guid, > > + CONNECTOR_DSM_REVID, > > + 1 << CONNECTOR_DSM_FN_PRIVACY_ENABLE | > > + 1 << CONNECTOR_DSM_FN_PRIVACY_DISABLE)) { > > + DRM_WARN("%s: Odd, connector ACPI node but no privacy scrn?\n", > > + dev_name(intel_connector->base.dev->dev)); > > + return false; > > + } > > + DRM_DEV_INFO(intel_connector->base.dev->dev, > > + "supports privacy screen\n"); > > + return true; > > +} > > diff --git a/drivers/gpu/drm/i915/display/intel_privacy_screen.h b/drivers/gpu/drm/i915/display/intel_privacy_screen.h > > new file mode 100644 > > index 000000000000..212f73349a00 > > --- /dev/null > > +++ b/drivers/gpu/drm/i915/display/intel_privacy_screen.h > > @@ -0,0 +1,25 @@ > > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > > +/* > > + * Copyright © 2019 Google Inc. > > + */ > > + > > +#ifndef __DRM_PRIVACY_SCREEN_H__ > > +#define __DRM_PRIVACY_SCREEN_H__ > > + > > +#include "intel_display_types.h" > > + > > +#ifdef CONFIG_ACPI > > +bool intel_privacy_screen_present(struct intel_connector *intel_connector); > > +void intel_privacy_screen_set_val(struct intel_connector *intel_connector, > > + enum intel_privacy_screen_status val); > > +#else > > +bool intel_privacy_screen_present(struct intel_connector *intel_connector); > > +{ > > + return false; > > +} > > +void intel_privacy_screen_set_val(struct intel_connector *intel_connector, > > + enum intel_privacy_screen_status val) > > +{ } > > +#endif /* CONFIG_ACPI */ > > + > > +#endif /* __DRM_PRIVACY_SCREEN_H__ */ > > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h > > index 469dc512cca3..cf08d5636363 100644 > > --- a/include/uapi/drm/i915_drm.h > > +++ b/include/uapi/drm/i915_drm.h > > @@ -2123,6 +2123,20 @@ struct drm_i915_query_engine_info { > > struct drm_i915_engine_info engines[]; > > }; > > > > +/** > > + * enum intel_privacy_screen_status - privacy_screen status > > + * > > + * This enum is used to track and control the state of the integrated privacy > > + * screen present on some display panels, via the "privacy-screen" property. > > + * > > + * @PRIVACY_SCREEN_DISABLED: The privacy-screen on the panel is disabled > > + * @PRIVACY_SCREEN_ENABLED: The privacy-screen on the panel is enabled > > + **/ > > +enum intel_privacy_screen_status { > > + PRIVACY_SCREEN_DISABLED = 0, > > + PRIVACY_SCREEN_ENABLED = 1, > > +}; > > + > > The drm_property interface UAPI is based on the strings, *not* on the > values. Please move the enum out of uapi into the drm code. Oh, so we don't have to expose this to userspace? Understand, so I moved it to intel_display_types.h Thanks, Rajat. > > BR, > jani. > > > #if defined(__cplusplus) > > } > > #endif > > -- > Jani Nikula, Intel Open Source Graphics Center
HI Jani, On Thu, Dec 5, 2019 at 1:34 AM Rajat Jain <rajatja@google.com> wrote: > > On Wed, Nov 20, 2019 at 7:04 AM Jani Nikula <jani.nikula@linux.intel.com> wrote: > > > > On Mon, 04 Nov 2019, Rajat Jain <rajatja@google.com> 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 <rajatja@google.com> > > > 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_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, > > > connector->colorspace_property, 0); > > > } > > > + > > > +static const struct drm_prop_enum_list privacy_screen_enum[] = { > > > + { PRIVACY_SCREEN_DISABLED, "Disabled" }, > > > + { PRIVACY_SCREEN_ENABLED, "Enabled" }, > > > +}; > > > + > > > +/** > > > + * intel_attach_privacy_screen_property - > > > + * create and attach the connecter's privacy-screen property. * > > > + * @connector: connector for which to init the privacy-screen property > > > + * > > > + * This function creates and attaches the "privacy-screen" property to the > > > + * connector. Initial state of privacy-screen is set to disabled. > > > + */ > > > +void > > > +intel_attach_privacy_screen_property(struct drm_connector *connector) > > > +{ > > > + struct intel_connector *intel_connector = to_intel_connector(connector); > > > + struct drm_property *prop; > > > + > > > + if (!intel_connector->privacy_screen_property) { > > > + prop = drm_property_create_enum(connector->dev, > > > + DRM_MODE_PROP_ENUM, > > > + "privacy-screen", > > > + privacy_screen_enum, > > > + ARRAY_SIZE(privacy_screen_enum)); > > > + if (!prop) > > > + return; > > > + > > > + intel_connector->privacy_screen_property = prop; > > > + } > > > + > > > + drm_object_attach_property(&connector->base, prop, > > > + PRIVACY_SCREEN_DISABLED); > > > +} > > > > I think this should be a drm core level property in drm_connector.[ch] > > so that *all* drivers would use the same thing for privacy screens. Not > > i915 specific. > > > > I think this is the biggest issue in the patch series. > > I actually would be happy to make it a drm_connector property, like I > had in my original patch series. I changed it to i915 specific because > it seemed to me based on the comments that the general sentiment is > that anything to do with acpi should be in i915. > > Note that the privacy screen property essentially needs an ACPI handle > to work on, so if I were to move the property into drm_connector, I'd > likely rename the intel_privacy_screen* code to drm_privacy_screen* > code, and it'll still operate on an ACPI handle (stored in > drm_connector structure). The i915's job will then be to lookup that > ACPI handle (because lookup requires display index) and populate it in > drm_connector. Does this sound OK? Wondering if you ever got my email. I found out this week that I needed to be a member of intel-gfx mailing list otherwise my patches won't show up on patchwork.freedesktop.org. I'm now a member, and I have posted my latest patchset at: https://patchwork.freedesktop.org/patch/346275/ I'd appreciate if you could please review and comment and provide your valuable suggestions. Thanks & Best Regards, Rajat > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_connector.h b/drivers/gpu/drm/i915/display/intel_connector.h > > > index 93a7375c8196..61005f37a338 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_connector.h > > > +++ b/drivers/gpu/drm/i915/display/intel_connector.h > > > @@ -31,5 +31,6 @@ void intel_attach_force_audio_property(struct drm_connector *connector); > > > void intel_attach_broadcast_rgb_property(struct drm_connector *connector); > > > void intel_attach_aspect_ratio_property(struct drm_connector *connector); > > > void intel_attach_colorspace_property(struct drm_connector *connector); > > > +void intel_attach_privacy_screen_property(struct drm_connector *connector); > > > > > > #endif /* __INTEL_CONNECTOR_H__ */ > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h > > > index c2706afc069b..83b8c98049a7 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > > > @@ -426,6 +426,9 @@ struct intel_connector { > > > struct work_struct modeset_retry_work; > > > > > > struct intel_hdcp hdcp; > > > + > > > + /* Optional "privacy-screen" property for the connector panel */ > > > + struct drm_property *privacy_screen_property; > > > }; > > > > > > struct intel_digital_connector_state { > > > @@ -433,6 +436,7 @@ struct intel_digital_connector_state { > > > > > > enum hdmi_force_audio force_audio; > > > int broadcast_rgb; > > > + enum intel_privacy_screen_status privacy_screen_status; > > > }; > > > > > > #define to_intel_digital_connector_state(x) container_of(x, struct intel_digital_connector_state, base) > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > > > index 4fac408a4299..1963e92404ba 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" > > > @@ -6358,6 +6359,10 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect > > > > > > /* Lookup the ACPI node corresponding to the connector */ > > > intel_connector_lookup_acpi_node(intel_connector); > > > + > > > + /* Check for integrated Privacy screen support */ > > > + if (intel_privacy_screen_present(intel_connector)) > > > + intel_attach_privacy_screen_property(connector); > > > } > > > } > > > > > > 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 000000000000..4c422e38c51a > > > --- /dev/null > > > +++ b/drivers/gpu/drm/i915/display/intel_privacy_screen.c > > > @@ -0,0 +1,70 @@ > > > +// SPDX-License-Identifier: GPL-2.0-or-later > > > > Please read http://mid.mail-archive.com/CAKMK7uH-8+tbKsAoiChsxELEc_77RVVxP2wapHWhqB+0Viifog@mail.gmail.com > > OK, I changed it to SPDX-License-Identifier: GPL-2.0 OR MIT > > > > > > > +/* > > > + * Intel ACPI privacy screen code > > > + * > > > + * Copyright © 2019 Google Inc. > > > + */ > > > + > > > +#include <linux/acpi.h> > > > + > > > +#include "intel_privacy_screen.h" > > > + > > > +#define CONNECTOR_DSM_REVID 1 > > > + > > > +#define CONNECTOR_DSM_FN_PRIVACY_ENABLE 2 > > > +#define CONNECTOR_DSM_FN_PRIVACY_DISABLE 3 > > > + > > > +static const guid_t drm_conn_dsm_guid = > > > + GUID_INIT(0xC7033113, 0x8720, 0x4CEB, > > > + 0x90, 0x90, 0x9D, 0x52, 0xB3, 0xE5, 0x2D, 0x73); > > > + > > > +/* Makes _DSM call to set privacy screen status */ > > > +static void acpi_privacy_screen_call_dsm(acpi_handle conn_handle, u64 func) > > > +{ > > > + union acpi_object *obj; > > > + > > > + obj = acpi_evaluate_dsm(conn_handle, &drm_conn_dsm_guid, > > > + CONNECTOR_DSM_REVID, func, NULL); > > > + if (!obj) { > > > + DRM_DEBUG_DRIVER("failed to evaluate _DSM for fn %llx\n", func); > > > + return; > > > + } > > > + > > > + ACPI_FREE(obj); > > > +} > > > + > > > +void intel_privacy_screen_set_val(struct intel_connector *intel_connector, > > > + enum intel_privacy_screen_status val) > > > > Just name the parameter connector, not intel_connector. This throughout. > > Done. > > > > > > +{ > > > + acpi_handle acpi_handle = intel_connector->acpi_handle; > > > + > > > + if (!acpi_handle) > > > + return; > > > + > > > + if (val == PRIVACY_SCREEN_DISABLED) > > > + acpi_privacy_screen_call_dsm(acpi_handle, > > > + CONNECTOR_DSM_FN_PRIVACY_DISABLE); > > > + else if (val == PRIVACY_SCREEN_ENABLED) > > > + acpi_privacy_screen_call_dsm(acpi_handle, > > > + CONNECTOR_DSM_FN_PRIVACY_ENABLE); > > > > else complain? > > > > Done. > > > > +} > > > + > > > +bool intel_privacy_screen_present(struct intel_connector *intel_connector) > > > +{ > > > + acpi_handle handle = intel_connector->acpi_handle; > > > + > > > + if (!handle) > > > + return false; > > > + > > > + if (!acpi_check_dsm(handle, &drm_conn_dsm_guid, > > > + CONNECTOR_DSM_REVID, > > > + 1 << CONNECTOR_DSM_FN_PRIVACY_ENABLE | > > > + 1 << CONNECTOR_DSM_FN_PRIVACY_DISABLE)) { > > > + DRM_WARN("%s: Odd, connector ACPI node but no privacy scrn?\n", > > > + dev_name(intel_connector->base.dev->dev)); > > > + return false; > > > + } > > > + DRM_DEV_INFO(intel_connector->base.dev->dev, > > > + "supports privacy screen\n"); > > > + return true; > > > +} > > > diff --git a/drivers/gpu/drm/i915/display/intel_privacy_screen.h b/drivers/gpu/drm/i915/display/intel_privacy_screen.h > > > new file mode 100644 > > > index 000000000000..212f73349a00 > > > --- /dev/null > > > +++ b/drivers/gpu/drm/i915/display/intel_privacy_screen.h > > > @@ -0,0 +1,25 @@ > > > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > > > +/* > > > + * Copyright © 2019 Google Inc. > > > + */ > > > + > > > +#ifndef __DRM_PRIVACY_SCREEN_H__ > > > +#define __DRM_PRIVACY_SCREEN_H__ > > > + > > > +#include "intel_display_types.h" > > > + > > > +#ifdef CONFIG_ACPI > > > +bool intel_privacy_screen_present(struct intel_connector *intel_connector); > > > +void intel_privacy_screen_set_val(struct intel_connector *intel_connector, > > > + enum intel_privacy_screen_status val); > > > +#else > > > +bool intel_privacy_screen_present(struct intel_connector *intel_connector); > > > +{ > > > + return false; > > > +} > > > +void intel_privacy_screen_set_val(struct intel_connector *intel_connector, > > > + enum intel_privacy_screen_status val) > > > +{ } > > > +#endif /* CONFIG_ACPI */ > > > + > > > +#endif /* __DRM_PRIVACY_SCREEN_H__ */ > > > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h > > > index 469dc512cca3..cf08d5636363 100644 > > > --- a/include/uapi/drm/i915_drm.h > > > +++ b/include/uapi/drm/i915_drm.h > > > @@ -2123,6 +2123,20 @@ struct drm_i915_query_engine_info { > > > struct drm_i915_engine_info engines[]; > > > }; > > > > > > +/** > > > + * enum intel_privacy_screen_status - privacy_screen status > > > + * > > > + * This enum is used to track and control the state of the integrated privacy > > > + * screen present on some display panels, via the "privacy-screen" property. > > > + * > > > + * @PRIVACY_SCREEN_DISABLED: The privacy-screen on the panel is disabled > > > + * @PRIVACY_SCREEN_ENABLED: The privacy-screen on the panel is enabled > > > + **/ > > > +enum intel_privacy_screen_status { > > > + PRIVACY_SCREEN_DISABLED = 0, > > > + PRIVACY_SCREEN_ENABLED = 1, > > > +}; > > > + > > > > The drm_property interface UAPI is based on the strings, *not* on the > > values. Please move the enum out of uapi into the drm code. > > Oh, so we don't have to expose this to userspace? Understand, so I > moved it to intel_display_types.h > > Thanks, > > Rajat. > > > > > > BR, > > jani. > > > > > #if defined(__cplusplus) > > > } > > > #endif > > > > -- > > Jani Nikula, Intel Open Source Graphics Center
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, connector->colorspace_property, 0); } + +static const struct drm_prop_enum_list privacy_screen_enum[] = { + { PRIVACY_SCREEN_DISABLED, "Disabled" }, + { PRIVACY_SCREEN_ENABLED, "Enabled" }, +}; + +/** + * intel_attach_privacy_screen_property - + * create and attach the connecter's privacy-screen property. * + * @connector: connector for which to init the privacy-screen property + * + * This function creates and attaches the "privacy-screen" property to the + * connector. Initial state of privacy-screen is set to disabled. + */ +void +intel_attach_privacy_screen_property(struct drm_connector *connector) +{ + struct intel_connector *intel_connector = to_intel_connector(connector); + struct drm_property *prop; + + if (!intel_connector->privacy_screen_property) { + prop = drm_property_create_enum(connector->dev, + DRM_MODE_PROP_ENUM, + "privacy-screen", + privacy_screen_enum, + ARRAY_SIZE(privacy_screen_enum)); + if (!prop) + return; + + intel_connector->privacy_screen_property = prop; + } + + drm_object_attach_property(&connector->base, prop, + PRIVACY_SCREEN_DISABLED); +} diff --git a/drivers/gpu/drm/i915/display/intel_connector.h b/drivers/gpu/drm/i915/display/intel_connector.h index 93a7375c8196..61005f37a338 100644 --- a/drivers/gpu/drm/i915/display/intel_connector.h +++ b/drivers/gpu/drm/i915/display/intel_connector.h @@ -31,5 +31,6 @@ void intel_attach_force_audio_property(struct drm_connector *connector); void intel_attach_broadcast_rgb_property(struct drm_connector *connector); void intel_attach_aspect_ratio_property(struct drm_connector *connector); void intel_attach_colorspace_property(struct drm_connector *connector); +void intel_attach_privacy_screen_property(struct drm_connector *connector); #endif /* __INTEL_CONNECTOR_H__ */ diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index c2706afc069b..83b8c98049a7 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -426,6 +426,9 @@ struct intel_connector { struct work_struct modeset_retry_work; struct intel_hdcp hdcp; + + /* Optional "privacy-screen" property for the connector panel */ + struct drm_property *privacy_screen_property; }; struct intel_digital_connector_state { @@ -433,6 +436,7 @@ struct intel_digital_connector_state { enum hdmi_force_audio force_audio; int broadcast_rgb; + enum intel_privacy_screen_status privacy_screen_status; }; #define to_intel_digital_connector_state(x) container_of(x, struct intel_digital_connector_state, base) diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 4fac408a4299..1963e92404ba 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" @@ -6358,6 +6359,10 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect /* Lookup the ACPI node corresponding to the connector */ intel_connector_lookup_acpi_node(intel_connector); + + /* Check for integrated Privacy screen support */ + if (intel_privacy_screen_present(intel_connector)) + intel_attach_privacy_screen_property(connector); } } 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 000000000000..4c422e38c51a --- /dev/null +++ b/drivers/gpu/drm/i915/display/intel_privacy_screen.c @@ -0,0 +1,70 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Intel ACPI privacy screen code + * + * Copyright © 2019 Google Inc. + */ + +#include <linux/acpi.h> + +#include "intel_privacy_screen.h" + +#define CONNECTOR_DSM_REVID 1 + +#define CONNECTOR_DSM_FN_PRIVACY_ENABLE 2 +#define CONNECTOR_DSM_FN_PRIVACY_DISABLE 3 + +static const guid_t drm_conn_dsm_guid = + GUID_INIT(0xC7033113, 0x8720, 0x4CEB, + 0x90, 0x90, 0x9D, 0x52, 0xB3, 0xE5, 0x2D, 0x73); + +/* Makes _DSM call to set privacy screen status */ +static void acpi_privacy_screen_call_dsm(acpi_handle conn_handle, u64 func) +{ + union acpi_object *obj; + + obj = acpi_evaluate_dsm(conn_handle, &drm_conn_dsm_guid, + CONNECTOR_DSM_REVID, func, NULL); + if (!obj) { + DRM_DEBUG_DRIVER("failed to evaluate _DSM for fn %llx\n", func); + return; + } + + ACPI_FREE(obj); +} + +void intel_privacy_screen_set_val(struct intel_connector *intel_connector, + enum intel_privacy_screen_status val) +{ + acpi_handle acpi_handle = intel_connector->acpi_handle; + + if (!acpi_handle) + return; + + if (val == PRIVACY_SCREEN_DISABLED) + acpi_privacy_screen_call_dsm(acpi_handle, + CONNECTOR_DSM_FN_PRIVACY_DISABLE); + else if (val == PRIVACY_SCREEN_ENABLED) + acpi_privacy_screen_call_dsm(acpi_handle, + CONNECTOR_DSM_FN_PRIVACY_ENABLE); +} + +bool intel_privacy_screen_present(struct intel_connector *intel_connector) +{ + acpi_handle handle = intel_connector->acpi_handle; + + if (!handle) + return false; + + if (!acpi_check_dsm(handle, &drm_conn_dsm_guid, + CONNECTOR_DSM_REVID, + 1 << CONNECTOR_DSM_FN_PRIVACY_ENABLE | + 1 << CONNECTOR_DSM_FN_PRIVACY_DISABLE)) { + DRM_WARN("%s: Odd, connector ACPI node but no privacy scrn?\n", + dev_name(intel_connector->base.dev->dev)); + return false; + } + DRM_DEV_INFO(intel_connector->base.dev->dev, + "supports privacy screen\n"); + return true; +} diff --git a/drivers/gpu/drm/i915/display/intel_privacy_screen.h b/drivers/gpu/drm/i915/display/intel_privacy_screen.h new file mode 100644 index 000000000000..212f73349a00 --- /dev/null +++ b/drivers/gpu/drm/i915/display/intel_privacy_screen.h @@ -0,0 +1,25 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Copyright © 2019 Google Inc. + */ + +#ifndef __DRM_PRIVACY_SCREEN_H__ +#define __DRM_PRIVACY_SCREEN_H__ + +#include "intel_display_types.h" + +#ifdef CONFIG_ACPI +bool intel_privacy_screen_present(struct intel_connector *intel_connector); +void intel_privacy_screen_set_val(struct intel_connector *intel_connector, + enum intel_privacy_screen_status val); +#else +bool intel_privacy_screen_present(struct intel_connector *intel_connector); +{ + return false; +} +void intel_privacy_screen_set_val(struct intel_connector *intel_connector, + enum intel_privacy_screen_status val) +{ } +#endif /* CONFIG_ACPI */ + +#endif /* __DRM_PRIVACY_SCREEN_H__ */ diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index 469dc512cca3..cf08d5636363 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -2123,6 +2123,20 @@ struct drm_i915_query_engine_info { struct drm_i915_engine_info engines[]; }; +/** + * enum intel_privacy_screen_status - privacy_screen status + * + * This enum is used to track and control the state of the integrated privacy + * screen present on some display panels, via the "privacy-screen" property. + * + * @PRIVACY_SCREEN_DISABLED: The privacy-screen on the panel is disabled + * @PRIVACY_SCREEN_ENABLED: The privacy-screen on the panel is enabled + **/ +enum intel_privacy_screen_status { + PRIVACY_SCREEN_DISABLED = 0, + PRIVACY_SCREEN_ENABLED = 1, +}; + #if defined(__cplusplus) } #endif
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 <rajatja@google.com> 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