On Tue, 12 Nov 2019, Rajat Jain <rajatja@xxxxxxxxxx> wrote: > On Mon, Nov 4, 2019 at 11:41 AM Rajat Jain <rajatja@xxxxxxxxxx> 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@xxxxxxxxxx> >> 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 >> -- Jani Nikula, Intel Open Source Graphics Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
