* [PATCH v7 1/4] intel_acpi: Rename drm_dev local variable to dev
2020-03-10 0:06 [PATCH v7 0/4] drm: Add support for integrated privacy screen Rajat Jain
@ 2020-03-10 0:06 ` Rajat Jain
2020-03-10 0:06 ` [PATCH v7 2/4] drm/connector: Add support for privacy-screen property Rajat Jain
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Rajat Jain @ 2020-03-10 0:06 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Sean Paul, David Airlie,
Daniel Vetter, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
Ville Syrjälä, Chris Wilson, Imre Deak,
José Roberto de Souza, linux-kernel, dri-devel, intel-gfx,
gregkh, mathewk, Daniel Thompson, Jonathan Corbet, Pavel Machek,
seanpaul, Duncan Laurie, jsbarnes, Thierry Reding, mpearson,
Nitin Joshi1, Sugumaran Lacshiminarayanan, Tomoki Maruichi
Cc: Rajat Jain, rajatxjain, Jani Nikula
Change the struct drm_device * local variable from drm_dev to dev
per the feedback received here:
https://lkml.org/lkml/2020/1/24/1143
Signed-off-by: Rajat Jain <rajatja@google.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
---
v7: same as v6
v6: Initial patch (v6 to match other patches in the set)
drivers/gpu/drm/i915/display/intel_acpi.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_acpi.c b/drivers/gpu/drm/i915/display/intel_acpi.c
index e21fb14d5e07b..3e6831cca4ac1 100644
--- a/drivers/gpu/drm/i915/display/intel_acpi.c
+++ b/drivers/gpu/drm/i915/display/intel_acpi.c
@@ -224,13 +224,13 @@ static u32 acpi_display_type(struct intel_connector *connector)
void intel_acpi_device_id_update(struct drm_i915_private *dev_priv)
{
- struct drm_device *drm_dev = &dev_priv->drm;
+ struct drm_device *dev = &dev_priv->drm;
struct intel_connector *connector;
struct drm_connector_list_iter conn_iter;
u8 display_index[16] = {};
/* Populate the ACPI IDs for all connectors for a given drm_device */
- drm_connector_list_iter_begin(drm_dev, &conn_iter);
+ drm_connector_list_iter_begin(dev, &conn_iter);
for_each_intel_connector_iter(connector, &conn_iter) {
u32 device_id, type;
--
2.25.1.481.gfbce0eb801-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v7 2/4] drm/connector: Add support for privacy-screen property
2020-03-10 0:06 [PATCH v7 0/4] drm: Add support for integrated privacy screen Rajat Jain
2020-03-10 0:06 ` [PATCH v7 1/4] intel_acpi: Rename drm_dev local variable to dev Rajat Jain
@ 2020-03-10 0:06 ` Rajat Jain
2020-03-10 0:06 ` [PATCH v7 3/4] drm/i915: Lookup and attach ACPI device node for connectors Rajat Jain
2020-03-10 0:06 ` [PATCH v7 4/4] drm/i915: Add support for integrated privacy screen Rajat Jain
3 siblings, 0 replies; 9+ messages in thread
From: Rajat Jain @ 2020-03-10 0:06 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Sean Paul, David Airlie,
Daniel Vetter, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
Ville Syrjälä, Chris Wilson, Imre Deak,
José Roberto de Souza, linux-kernel, dri-devel, intel-gfx,
gregkh, mathewk, Daniel Thompson, Jonathan Corbet, Pavel Machek,
seanpaul, Duncan Laurie, jsbarnes, Thierry Reding, mpearson,
Nitin Joshi1, Sugumaran Lacshiminarayanan, Tomoki Maruichi
Cc: Rajat Jain, rajatxjain
Add support for generic electronic privacy screen property, that
can be added by systems that have an integrated EPS.
Signed-off-by: Rajat Jain <rajatja@google.com>
---
v7: * Initial version, formed by moving the privacy-screen property into
drm core.
* Break the init_property() into create_property() and attach_property()
so that property can be created while registering connector, but
attached in late_register() (after ACPI node detection).
drivers/gpu/drm/drm_atomic_uapi.c | 4 +++
drivers/gpu/drm/drm_connector.c | 56 +++++++++++++++++++++++++++++++
include/drm/drm_connector.h | 25 ++++++++++++++
3 files changed, 85 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index a1e5e262bae2d..843a8cdacd149 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -766,6 +766,8 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
fence_ptr);
} else if (property == connector->max_bpc_property) {
state->max_requested_bpc = val;
+ } else if (property == connector->privacy_screen_property) {
+ state->privacy_screen_status = val;
} else if (connector->funcs->atomic_set_property) {
return connector->funcs->atomic_set_property(connector,
state, property, val);
@@ -842,6 +844,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
*val = 0;
} else if (property == connector->max_bpc_property) {
*val = state->max_requested_bpc;
+ } else if (property == connector->privacy_screen_property) {
+ *val = state->privacy_screen_status;
} else if (connector->funcs->atomic_get_property) {
return connector->funcs->atomic_get_property(connector,
state, property, val);
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index f632ca05960e7..39303e15063a9 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1171,6 +1171,10 @@ static const struct drm_prop_enum_list dp_colorspaces[] = {
* can also expose this property to external outputs, in which case they
* must support "None", which should be the default (since external screens
* have a built-in scaler).
+ *
+ * privacy-screen:
+ * This optional property can be used to enable / disable an integrated
+ * electronic privacy screen that is available on some displays.
*/
int drm_connector_create_standard_properties(struct drm_device *dev)
@@ -2137,6 +2141,58 @@ int drm_connector_set_panel_orientation_with_quirk(
}
EXPORT_SYMBOL(drm_connector_set_panel_orientation_with_quirk);
+static const struct drm_prop_enum_list privacy_screen_enum[] = {
+ { PRIVACY_SCREEN_DISABLED, "Disabled" },
+ { PRIVACY_SCREEN_ENABLED, "Enabled" },
+};
+
+/**
+ * drm_connector_init_privacy_screen_property -
+ * create and attach the drm 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
+drm_connector_create_privacy_screen_property(struct drm_connector *connector)
+{
+ if (connector->privacy_screen_property)
+ return;
+
+ connector->privacy_screen_property =
+ drm_property_create_enum(connector->dev, DRM_MODE_PROP_ENUM,
+ "privacy-screen", privacy_screen_enum,
+ ARRAY_SIZE(privacy_screen_enum));
+}
+EXPORT_SYMBOL(drm_connector_create_privacy_screen_property);
+
+void
+drm_connector_attach_privacy_screen_property(struct drm_connector *connector)
+{
+ struct drm_property *prop = connector->privacy_screen_property;
+
+ if (!prop)
+ return;
+
+ drm_object_attach_property(&connector->base, prop,
+ PRIVACY_SCREEN_DISABLED);
+}
+EXPORT_SYMBOL(drm_connector_attach_privacy_screen_property);
+
+void
+drm_connector_destroy_privacy_screen_property(struct drm_connector *connector)
+{
+ struct drm_property *prop = connector->privacy_screen_property;
+
+ if (!prop)
+ return;
+
+ drm_property_destroy(connector->dev, prop);
+ connector->privacy_screen_property = NULL;
+}
+EXPORT_SYMBOL(drm_connector_destroy_privacy_screen_property);
+
int drm_connector_set_obj_prop(struct drm_mode_object *obj,
struct drm_property *property,
uint64_t value)
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 2113500b4075d..257f398ce8720 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -254,6 +254,20 @@ enum drm_panel_orientation {
DRM_MODE_PANEL_ORIENTATION_RIGHT_UP,
};
+/**
+ * enum drm_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 drm_privacy_screen_status {
+ PRIVACY_SCREEN_DISABLED = 0,
+ PRIVACY_SCREEN_ENABLED = 1,
+};
+
/*
* This is a consolidated colorimetry list supported by HDMI and
* DP protocol standard. The respective connectors will register
@@ -656,6 +670,8 @@ struct drm_connector_state {
*/
u8 max_bpc;
+ enum drm_privacy_screen_status privacy_screen_status;
+
/**
* @hdr_output_metadata:
* DRM blob property for HDR output metadata
@@ -1255,6 +1271,12 @@ struct drm_connector {
*/
struct drm_property *max_bpc_property;
+ /**
+ * @privacy_screen_property: Optional property for the connector to
+ * control the integrated privacy screen, if available.
+ */
+ struct drm_property *privacy_screen_property;
+
#define DRM_CONNECTOR_POLL_HPD (1 << 0)
#define DRM_CONNECTOR_POLL_CONNECT (1 << 1)
#define DRM_CONNECTOR_POLL_DISCONNECT (1 << 2)
@@ -1561,6 +1583,9 @@ int drm_connector_set_panel_orientation_with_quirk(
int width, int height);
int drm_connector_attach_max_bpc_property(struct drm_connector *connector,
int min, int max);
+void drm_connector_create_privacy_screen_property(struct drm_connector *conn);
+void drm_connector_attach_privacy_screen_property(struct drm_connector *conn);
+void drm_connector_destroy_privacy_screen_property(struct drm_connector *conn);
/**
* struct drm_tile_group - Tile group metadata
--
2.25.1.481.gfbce0eb801-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v7 3/4] drm/i915: Lookup and attach ACPI device node for connectors
2020-03-10 0:06 [PATCH v7 0/4] drm: Add support for integrated privacy screen Rajat Jain
2020-03-10 0:06 ` [PATCH v7 1/4] intel_acpi: Rename drm_dev local variable to dev Rajat Jain
2020-03-10 0:06 ` [PATCH v7 2/4] drm/connector: Add support for privacy-screen property Rajat Jain
@ 2020-03-10 0:06 ` Rajat Jain
2020-03-10 0:06 ` [PATCH v7 4/4] drm/i915: Add support for integrated privacy screen Rajat Jain
3 siblings, 0 replies; 9+ messages in thread
From: Rajat Jain @ 2020-03-10 0:06 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Sean Paul, David Airlie,
Daniel Vetter, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
Ville Syrjälä, Chris Wilson, Imre Deak,
José Roberto de Souza, linux-kernel, dri-devel, intel-gfx,
gregkh, mathewk, Daniel Thompson, Jonathan Corbet, Pavel Machek,
seanpaul, Duncan Laurie, jsbarnes, Thierry Reding, mpearson,
Nitin Joshi1, Sugumaran Lacshiminarayanan, Tomoki Maruichi
Cc: Rajat Jain, rajatxjain
Lookup and attach ACPI nodes for intel connectors. The lookup is done
in compliance with ACPI Spec 6.3
https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf
(Ref: Pages 1119 - 1123).
This can be useful for any connector specific platform properties. (This
will be used for privacy screen in next patch).
Signed-off-by: Rajat Jain <rajatja@google.com>
---
v7: Look for ACPI node in ->late_register() hook.
Do the scan only once per drm_device (instead of 1 per drm_connector)
v6: Addressed minor comments from Jani at
https://lkml.org/lkml/2020/1/24/1143
- local variable renamed.
- used drm_dbg_kms()
- used acpi_device_handle()
- Used opaque type acpi_handle instead of void*
v5: same as v4
v4: Same as v3
v3: fold the code into existing acpi_device_id_update() function
v2: formed by splitting the original patch into ACPI lookup, and privacy
screen property. Also move it into i915 now that I found existing code
in i915 that can be re-used.
drivers/gpu/drm/i915/display/intel_acpi.c | 24 +++++++++++++++++++
.../drm/i915/display/intel_display_types.h | 5 ++++
drivers/gpu/drm/i915/display/intel_dp.c | 15 +++++++++++-
drivers/gpu/drm/i915/i915_drv.h | 2 ++
4 files changed, 45 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_acpi.c b/drivers/gpu/drm/i915/display/intel_acpi.c
index 3e6831cca4ac1..5679f6882922e 100644
--- a/drivers/gpu/drm/i915/display/intel_acpi.c
+++ b/drivers/gpu/drm/i915/display/intel_acpi.c
@@ -222,13 +222,26 @@ static u32 acpi_display_type(struct intel_connector *connector)
return display_type;
}
+/*
+ * Ref: ACPI Spec 6.3
+ * https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf
+ * Pages 1119 - 1123 describe, what I believe, a standard way of
+ * identifying / addressing "display panels" in the ACPI. It provides
+ * a way for the ACPI to define devices for the display panels attached
+ * to the system. It thus provides a way for the BIOS to export any panel
+ * specific properties to the system via ACPI (like device trees).
+ */
void intel_acpi_device_id_update(struct drm_i915_private *dev_priv)
{
struct drm_device *dev = &dev_priv->drm;
struct intel_connector *connector;
struct drm_connector_list_iter conn_iter;
+ struct acpi_device *conn_dev, *parent;
+ u64 conn_addr;
u8 display_index[16] = {};
+ parent = ACPI_COMPANION(&dev->pdev->dev);
+
/* Populate the ACPI IDs for all connectors for a given drm_device */
drm_connector_list_iter_begin(dev, &conn_iter);
for_each_intel_connector_iter(connector, &conn_iter) {
@@ -242,6 +255,17 @@ void intel_acpi_device_id_update(struct drm_i915_private *dev_priv)
device_id |= display_index[type]++ << ACPI_DISPLAY_INDEX_SHIFT;
connector->acpi_device_id = device_id;
+
+ /* Build the _ADR to look for */
+ conn_addr = device_id | ACPI_DEVICE_ID_SCHEME |
+ ACPI_BIOS_CAN_DETECT;
+
+ drm_dbg_kms(dev, "Checking connector ACPI node at _ADR=%llX\n",
+ conn_addr);
+
+ /* Look up the connector device, under the PCI device */
+ conn_dev = acpi_find_child_device(parent, conn_addr, false);
+ connector->acpi_handle = acpi_device_handle(conn_dev);
}
drm_connector_list_iter_end(&conn_iter);
}
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 5e00e611f077f..d70612cc1ba2a 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -411,9 +411,14 @@ struct intel_connector {
*/
struct intel_encoder *encoder;
+#ifdef CONFIG_ACPI
/* ACPI device id for ACPI and driver cooperation */
u32 acpi_device_id;
+ /* ACPI handle corresponding to this connector display, if found */
+ acpi_handle acpi_handle;
+#endif
+
/* Reads out the current hw, returning true if the connector is enabled
* and active (i.e. dpms ON state). */
bool (*get_hw_state)(struct intel_connector *);
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 0a417cd2af2bc..41c623b029464 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -44,6 +44,7 @@
#include "i915_debugfs.h"
#include "i915_drv.h"
#include "i915_trace.h"
+#include "intel_acpi.h"
#include "intel_atomic.h"
#include "intel_audio.h"
#include "intel_connector.h"
@@ -5863,6 +5864,7 @@ static int intel_dp_get_modes(struct drm_connector *connector)
static int
intel_dp_connector_register(struct drm_connector *connector)
{
+ struct drm_i915_private *dev_priv = to_i915(connector->dev);
struct intel_dp *intel_dp = intel_attached_dp(to_intel_connector(connector));
int ret;
@@ -5872,6 +5874,18 @@ intel_dp_connector_register(struct drm_connector *connector)
intel_connector_debugfs_add(connector);
+ /*
+ * Lookup the ACPI node corresponding to the connector. This needs
+ * to be done in ->late_register() hook since it needs to iterate
+ * over all the connectors after they are registered. Calling it
+ * once for the device is enough since a single call() will update
+ * for all connectors.
+ */
+ if (!dev_priv->acpi_scan_done) {
+ intel_acpi_device_id_update(dev_priv);
+ dev_priv->acpi_scan_done = true;
+ }
+
DRM_DEBUG_KMS("registering %s bus for %s\n",
intel_dp->aux.name, connector->kdev->kobj.name);
@@ -6867,7 +6881,6 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
drm_connector_attach_scaling_mode_property(connector, allowed_scalers);
connector->state->scaling_mode = DRM_MODE_SCALE_ASPECT;
-
}
}
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 19195bde4921b..4e23d7a4a2129 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1189,6 +1189,8 @@ struct drm_i915_private {
bool ipc_enabled;
+ bool acpi_scan_done;
+
/* Used to save the pipe-to-encoder mapping for audio */
struct intel_encoder *av_enc_map[I915_MAX_PIPES];
--
2.25.1.481.gfbce0eb801-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v7 4/4] drm/i915: Add support for integrated privacy screen
2020-03-10 0:06 [PATCH v7 0/4] drm: Add support for integrated privacy screen Rajat Jain
` (2 preceding siblings ...)
2020-03-10 0:06 ` [PATCH v7 3/4] drm/i915: Lookup and attach ACPI device node for connectors Rajat Jain
@ 2020-03-10 0:06 ` Rajat Jain
2020-03-10 0:18 ` Rajat Jain
3 siblings, 1 reply; 9+ messages in thread
From: Rajat Jain @ 2020-03-10 0:06 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Sean Paul, David Airlie,
Daniel Vetter, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
Ville Syrjälä, Chris Wilson, Imre Deak,
José Roberto de Souza, linux-kernel, dri-devel, intel-gfx,
gregkh, mathewk, Daniel Thompson, Jonathan Corbet, Pavel Machek,
seanpaul, Duncan Laurie, jsbarnes, Thierry Reding, mpearson,
Nitin Joshi1, Sugumaran Lacshiminarayanan, Tomoki Maruichi
Cc: Rajat Jain, rajatxjain
Add support for an ACPI based integrated privacy screen that is
available on some systems.
Signed-off-by: Rajat Jain <rajatja@google.com>
---
v7: * Move the privacy-screen property back into drm core.
* Do the actual HW EPS toggling at commit time.
* Provide a sample ACPI node for reference in comments.
v6: Always initialize prop in intel_attach_privacy_screen_property()
v5: fix a cosmetic checkpatch warning
v4: Fix a typo in intel_privacy_screen.h
v3: * Change license to GPL-2.0 OR MIT
* Move privacy screen enum from UAPI to intel_display_types.h
* Rename parameter name and some other minor changes.
v2: Formed by splitting the original patch into multiple patches.
- All code has been moved into i915 now.
- Privacy screen is a i915 property
- Have a local state variable to store the prvacy screen. Don't read
it from hardware.
drivers/gpu/drm/i915/Makefile | 3 +-
drivers/gpu/drm/i915/display/intel_atomic.c | 1 +
drivers/gpu/drm/i915/display/intel_dp.c | 30 ++-
.../drm/i915/display/intel_privacy_screen.c | 175 ++++++++++++++++++
.../drm/i915/display/intel_privacy_screen.h | 27 +++
5 files changed, 234 insertions(+), 2 deletions(-)
create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.c
create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.h
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 9f887a86e555d..da42389107f9c 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -209,7 +209,8 @@ i915-y += \
display/intel_vga.o
i915-$(CONFIG_ACPI) += \
display/intel_acpi.o \
- display/intel_opregion.o
+ display/intel_opregion.o \
+ display/intel_privacy_screen.o
i915-$(CONFIG_DRM_FBDEV_EMULATION) += \
display/intel_fbdev.o
diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c b/drivers/gpu/drm/i915/display/intel_atomic.c
index d043057d2fa03..fc6264b4a7f73 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic.c
@@ -150,6 +150,7 @@ int intel_digital_connector_atomic_check(struct drm_connector *conn,
new_conn_state->base.picture_aspect_ratio != old_conn_state->base.picture_aspect_ratio ||
new_conn_state->base.content_type != old_conn_state->base.content_type ||
new_conn_state->base.scaling_mode != old_conn_state->base.scaling_mode ||
+ new_conn_state->base.privacy_screen_status != old_conn_state->base.privacy_screen_status ||
!blob_equal(new_conn_state->base.hdr_output_metadata,
old_conn_state->base.hdr_output_metadata))
crtc_state->mode_changed = true;
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 41c623b029464..a39b0c420b50a 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -62,6 +62,7 @@
#include "intel_lspcon.h"
#include "intel_lvds.h"
#include "intel_panel.h"
+#include "intel_privacy_screen.h"
#include "intel_psr.h"
#include "intel_sideband.h"
#include "intel_tc.h"
@@ -5886,6 +5887,12 @@ intel_dp_connector_register(struct drm_connector *connector)
dev_priv->acpi_scan_done = true;
}
+ /* Check for integrated Privacy screen support */
+ if (intel_privacy_screen_present(to_intel_connector(connector)))
+ drm_connector_attach_privacy_screen_property(connector);
+ else
+ drm_connector_destroy_privacy_screen_property(connector);
+
DRM_DEBUG_KMS("registering %s bus for %s\n",
intel_dp->aux.name, connector->kdev->kobj.name);
@@ -6881,9 +6888,30 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
drm_connector_attach_scaling_mode_property(connector, allowed_scalers);
connector->state->scaling_mode = DRM_MODE_SCALE_ASPECT;
+
+ drm_connector_create_privacy_screen_property(connector);
}
}
+static void intel_dp_update_privacy_screen(struct intel_encoder *encoder,
+ const struct intel_crtc_state *crtc_state,
+ const struct drm_connector_state *conn_state)
+{
+ struct drm_connector *connector = conn_state->connector;
+
+ if (connector->privacy_screen_property)
+ intel_privacy_screen_set_val(to_intel_connector(connector),
+ conn_state->privacy_screen_status);
+}
+
+static void intel_dp_update_pipe(struct intel_encoder *encoder,
+ const struct intel_crtc_state *crtc_state,
+ const struct drm_connector_state *conn_state)
+{
+ intel_dp_update_privacy_screen(encoder, crtc_state, conn_state);
+ intel_panel_update_backlight(encoder, crtc_state, conn_state);
+}
+
static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp)
{
intel_dp->panel_power_off_time = ktime_get_boottime();
@@ -7825,7 +7853,7 @@ bool intel_dp_init(struct drm_i915_private *dev_priv,
intel_encoder->compute_config = intel_dp_compute_config;
intel_encoder->get_hw_state = intel_dp_get_hw_state;
intel_encoder->get_config = intel_dp_get_config;
- intel_encoder->update_pipe = intel_panel_update_backlight;
+ intel_encoder->update_pipe = intel_dp_update_pipe;
intel_encoder->suspend = intel_dp_encoder_suspend;
if (IS_CHERRYVIEW(dev_priv)) {
intel_encoder->pre_pll_enable = chv_dp_pre_pll_enable;
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 0000000000000..6ff61ddf4c0a4
--- /dev/null
+++ b/drivers/gpu/drm/i915/display/intel_privacy_screen.c
@@ -0,0 +1,175 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT
+/*
+ * Intel ACPI privacy screen code
+ *
+ * Copyright © 2020 Google Inc.
+ *
+ * This code can help detect and control an integrated EPS (electronic
+ * privacy screen) via ACPI functions. It expects an ACPI node for the
+ * drm connector device with the following elements:
+ *
+ * UUID should be "c7033113-8720-4ceb-9090-9d52b3e52d73"
+ *
+ * _ADR = ACPI address per Spec (also see intel_acpi_device_id_update())
+ * https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf
+ * Pages 1119 - 1123.
+ *
+ * _DSM method that will perform the following functions according to
+ * Local1 argument passed to it:
+ * - Local1 = 0 (EPS capabilities): Report EPS presence and capabilities.
+ * - Local1 = 1 (EPS State) : _DSM returns 1 if EPS is enabled, 0 otherwise.
+ * - Local1 = 2 (EPS Enable) : _DSM enables EPS
+ * - Local1 = 3 (EPS Disable): _DSM disables EPS
+ *
+ * Here is a sample ACPI node:
+ *
+ * Scope (\_SB.PCI0.GFX0) // Intel graphics device (PCI device)
+ * {
+ * Method (_DOD, 0, NotSerialized) // _DOD: Display Output Devices
+ * {
+ * Return (Package (0x01)
+ * {
+ * 0x80010400
+ * })
+ * }
+ *
+ * Device (LCD)
+ * {
+ * Name (_ADR, 0x80010400) // _ADR: Address
+ * Name (_STA, 0x0F) // _STA: Status
+ *
+ * Method (EPSP, 0, NotSerialized) // EPS Present
+ * {
+ * Return (0x01)
+ * }
+ *
+ * Method (EPSS, 0, NotSerialized) // EPS State
+ * {
+ * Local0 = \_SB.PCI0.GRXS (0xCD)
+ * Return (Local0)
+ * }
+ *
+ * Method (EPSE, 0, NotSerialized) // EPS Enable
+ * {
+ * \_SB.PCI0.STXS (0xCD)
+ * }
+ *
+ * Method (EPSD, 0, NotSerialized) // EPS Disable
+ * {
+ * \_SB.PCI0.CTXS (0xCD)
+ * }
+ *
+ * Method (_DSM, 4, Serialized) // _DSM: Device-Specific Method
+ * {
+ * ToBuffer (Arg0, Local0)
+ * If ((Local0 == ToUUID ("c7033113-8720-4ceb-9090-9d52b3e52d73")))
+ * {
+ * ToInteger (Arg2, Local1)
+ * If ((Local1 == Zero))
+ * {
+ * Local2 = EPSP ()
+ * If ((Local2 == One))
+ * {
+ * Return (Buffer (One)
+ * {
+ * 0x0F
+ * })
+ * }
+ * }
+ *
+ * If ((Local1 == One))
+ * {
+ * Return (EPSS ())
+ * }
+ *
+ * If ((Local1 == 0x02))
+ * {
+ * EPSE ()
+ * }
+ *
+ * If ((Local1 == 0x03))
+ * {
+ * EPSD ()
+ * }
+ *
+ * Return (Buffer (One)
+ * {
+ * 0x00
+ * })
+ * }
+ *
+ * Return (Buffer (One)
+ * {
+ * 0x00
+ * })
+ * }
+ * }
+ * }
+ */
+
+#include <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(struct intel_connector *connector,
+ u64 func)
+{
+ union acpi_object *obj;
+ acpi_handle acpi_handle = connector->acpi_handle;
+
+ if (!acpi_handle)
+ return;
+
+ obj = acpi_evaluate_dsm(acpi_handle, &drm_conn_dsm_guid,
+ CONNECTOR_DSM_REVID, func, NULL);
+ if (!obj) {
+ drm_err(connector->base.dev,
+ "failed to evaluate _DSM for fn %llx\n", func);
+ return;
+ }
+
+ ACPI_FREE(obj);
+}
+
+void intel_privacy_screen_set_val(struct intel_connector *connector,
+ enum drm_privacy_screen_status val)
+{
+ if (val == PRIVACY_SCREEN_DISABLED)
+ acpi_privacy_screen_call_dsm(connector,
+ CONNECTOR_DSM_FN_PRIVACY_DISABLE);
+ else if (val == PRIVACY_SCREEN_ENABLED)
+ acpi_privacy_screen_call_dsm(connector,
+ CONNECTOR_DSM_FN_PRIVACY_ENABLE);
+ else
+ drm_err(connector->base.dev,
+ "Cannot set privacy screen to invalid val %u\n", val);
+}
+
+bool intel_privacy_screen_present(struct intel_connector *connector)
+{
+ acpi_handle handle = 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_dbg_kms(connector->base.dev,
+ "ACPI node but no privacy scrn\n");
+ return false;
+ }
+ drm_info(connector->base.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 0000000000000..f8d2e246ea0bd
--- /dev/null
+++ b/drivers/gpu/drm/i915/display/intel_privacy_screen.h
@@ -0,0 +1,27 @@
+/* SPDX-License-Identifier: GPL-2.0 OR MIT */
+/*
+ * 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 *connector);
+void intel_privacy_screen_set_val(struct intel_connector *connector,
+ enum drm_privacy_screen_status val);
+#else
+static bool intel_privacy_screen_present(struct intel_connector *connector)
+{
+ return false;
+}
+
+static void
+intel_privacy_screen_set_val(struct intel_connector *connector,
+ enum drm_privacy_screen_status val)
+{ }
+#endif /* CONFIG_ACPI */
+
+#endif /* __DRM_PRIVACY_SCREEN_H__ */
--
2.25.1.481.gfbce0eb801-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v7 4/4] drm/i915: Add support for integrated privacy screen
2020-03-10 0:06 ` [PATCH v7 4/4] drm/i915: Add support for integrated privacy screen Rajat Jain
@ 2020-03-10 0:18 ` Rajat Jain
2020-03-12 5:55 ` Rajat Jain
0 siblings, 1 reply; 9+ messages in thread
From: Rajat Jain @ 2020-03-10 0:18 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Sean Paul, David Airlie,
Daniel Vetter, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
Ville Syrjälä, Chris Wilson, Imre Deak,
José Roberto de Souza, Linux Kernel Mailing List, dri-devel,
intel-gfx, Greg Kroah-Hartman, Mat King, Daniel Thompson,
Jonathan Corbet, Pavel Machek, Sean Paul, Duncan Laurie,
Jesse Barnes, Thierry Reding, Mark Pearson, Nitin Joshi1,
Sugumaran Lacshiminarayanan, Tomoki Maruichi
Cc: Rajat Jain
Hi Jani,
I have 1 question / need 1 help about this patch:
On Mon, Mar 9, 2020 at 5:06 PM Rajat Jain <rajatja@google.com> wrote:
>
> Add support for an ACPI based integrated privacy screen that is
> available on some systems.
>
> Signed-off-by: Rajat Jain <rajatja@google.com>
> ---
> v7: * Move the privacy-screen property back into drm core.
> * Do the actual HW EPS toggling at commit time.
> * Provide a sample ACPI node for reference in comments.
> v6: Always initialize prop in intel_attach_privacy_screen_property()
> v5: fix a cosmetic checkpatch warning
> v4: Fix a typo in intel_privacy_screen.h
> v3: * Change license to GPL-2.0 OR MIT
> * Move privacy screen enum from UAPI to intel_display_types.h
> * Rename parameter name and some other minor changes.
> v2: Formed by splitting the original patch into multiple patches.
> - All code has been moved into i915 now.
> - Privacy screen is a i915 property
> - Have a local state variable to store the prvacy screen. Don't read
> it from hardware.
>
> drivers/gpu/drm/i915/Makefile | 3 +-
> drivers/gpu/drm/i915/display/intel_atomic.c | 1 +
> drivers/gpu/drm/i915/display/intel_dp.c | 30 ++-
> .../drm/i915/display/intel_privacy_screen.c | 175 ++++++++++++++++++
> .../drm/i915/display/intel_privacy_screen.h | 27 +++
> 5 files changed, 234 insertions(+), 2 deletions(-)
> create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.c
> create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.h
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 9f887a86e555d..da42389107f9c 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -209,7 +209,8 @@ i915-y += \
> display/intel_vga.o
> i915-$(CONFIG_ACPI) += \
> display/intel_acpi.o \
> - display/intel_opregion.o
> + display/intel_opregion.o \
> + display/intel_privacy_screen.o
> i915-$(CONFIG_DRM_FBDEV_EMULATION) += \
> display/intel_fbdev.o
>
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c b/drivers/gpu/drm/i915/display/intel_atomic.c
> index d043057d2fa03..fc6264b4a7f73 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic.c
> @@ -150,6 +150,7 @@ int intel_digital_connector_atomic_check(struct drm_connector *conn,
> new_conn_state->base.picture_aspect_ratio != old_conn_state->base.picture_aspect_ratio ||
> new_conn_state->base.content_type != old_conn_state->base.content_type ||
> new_conn_state->base.scaling_mode != old_conn_state->base.scaling_mode ||
> + new_conn_state->base.privacy_screen_status != old_conn_state->base.privacy_screen_status ||
> !blob_equal(new_conn_state->base.hdr_output_metadata,
> old_conn_state->base.hdr_output_metadata))
> crtc_state->mode_changed = true;
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 41c623b029464..a39b0c420b50a 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -62,6 +62,7 @@
> #include "intel_lspcon.h"
> #include "intel_lvds.h"
> #include "intel_panel.h"
> +#include "intel_privacy_screen.h"
> #include "intel_psr.h"
> #include "intel_sideband.h"
> #include "intel_tc.h"
> @@ -5886,6 +5887,12 @@ intel_dp_connector_register(struct drm_connector *connector)
> dev_priv->acpi_scan_done = true;
> }
>
> + /* Check for integrated Privacy screen support */
> + if (intel_privacy_screen_present(to_intel_connector(connector)))
> + drm_connector_attach_privacy_screen_property(connector);
> + else
> + drm_connector_destroy_privacy_screen_property(connector);
> +
> DRM_DEBUG_KMS("registering %s bus for %s\n",
> intel_dp->aux.name, connector->kdev->kobj.name);
>
> @@ -6881,9 +6888,30 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
> drm_connector_attach_scaling_mode_property(connector, allowed_scalers);
>
> connector->state->scaling_mode = DRM_MODE_SCALE_ASPECT;
> +
> + drm_connector_create_privacy_screen_property(connector);
> }
> }
>
> +static void intel_dp_update_privacy_screen(struct intel_encoder *encoder,
> + const struct intel_crtc_state *crtc_state,
> + const struct drm_connector_state *conn_state)
> +{
> + struct drm_connector *connector = conn_state->connector;
> +
> + if (connector->privacy_screen_property)
> + intel_privacy_screen_set_val(to_intel_connector(connector),
> + conn_state->privacy_screen_status);
> +}
> +
> +static void intel_dp_update_pipe(struct intel_encoder *encoder,
> + const struct intel_crtc_state *crtc_state,
> + const struct drm_connector_state *conn_state)
> +{
> + intel_dp_update_privacy_screen(encoder, crtc_state, conn_state);
> + intel_panel_update_backlight(encoder, crtc_state, conn_state);
> +}
> +
> static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp)
> {
> intel_dp->panel_power_off_time = ktime_get_boottime();
> @@ -7825,7 +7853,7 @@ bool intel_dp_init(struct drm_i915_private *dev_priv,
> intel_encoder->compute_config = intel_dp_compute_config;
> intel_encoder->get_hw_state = intel_dp_get_hw_state;
> intel_encoder->get_config = intel_dp_get_config;
> - intel_encoder->update_pipe = intel_panel_update_backlight;
> + intel_encoder->update_pipe = intel_dp_update_pipe;
For my testing purposes, I'm testing this using the proptest userspace utility
in our distribution (I think from
https://github.com/CPFL/drm/blob/master/tests/proptest/proptest.c). I
notice that when I change the value of the property from userspace,
even though the drm_connector_state->privacy_screen_status gets
updated and reflects the change, the encoder->update_pipe() is not
getting called. Just wanted to ask if this is expected since you seem
to have implied that this update_pipe() might *not* get called if there *is* a
full modeset? (What is the hook that gets called for a full modeset
where i915 driver should commit this property change to the hardware?)
Thanks a lot for all your help,
Best Regards,
Rajat
> intel_encoder->suspend = intel_dp_encoder_suspend;
> if (IS_CHERRYVIEW(dev_priv)) {
> intel_encoder->pre_pll_enable = chv_dp_pre_pll_enable;
> 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 0000000000000..6ff61ddf4c0a4
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/display/intel_privacy_screen.c
> @@ -0,0 +1,175 @@
> +// SPDX-License-Identifier: GPL-2.0 OR MIT
> +/*
> + * Intel ACPI privacy screen code
> + *
> + * Copyright © 2020 Google Inc.
> + *
> + * This code can help detect and control an integrated EPS (electronic
> + * privacy screen) via ACPI functions. It expects an ACPI node for the
> + * drm connector device with the following elements:
> + *
> + * UUID should be "c7033113-8720-4ceb-9090-9d52b3e52d73"
> + *
> + * _ADR = ACPI address per Spec (also see intel_acpi_device_id_update())
> + * https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf
> + * Pages 1119 - 1123.
> + *
> + * _DSM method that will perform the following functions according to
> + * Local1 argument passed to it:
> + * - Local1 = 0 (EPS capabilities): Report EPS presence and capabilities.
> + * - Local1 = 1 (EPS State) : _DSM returns 1 if EPS is enabled, 0 otherwise.
> + * - Local1 = 2 (EPS Enable) : _DSM enables EPS
> + * - Local1 = 3 (EPS Disable): _DSM disables EPS
> + *
> + * Here is a sample ACPI node:
> + *
> + * Scope (\_SB.PCI0.GFX0) // Intel graphics device (PCI device)
> + * {
> + * Method (_DOD, 0, NotSerialized) // _DOD: Display Output Devices
> + * {
> + * Return (Package (0x01)
> + * {
> + * 0x80010400
> + * })
> + * }
> + *
> + * Device (LCD)
> + * {
> + * Name (_ADR, 0x80010400) // _ADR: Address
> + * Name (_STA, 0x0F) // _STA: Status
> + *
> + * Method (EPSP, 0, NotSerialized) // EPS Present
> + * {
> + * Return (0x01)
> + * }
> + *
> + * Method (EPSS, 0, NotSerialized) // EPS State
> + * {
> + * Local0 = \_SB.PCI0.GRXS (0xCD)
> + * Return (Local0)
> + * }
> + *
> + * Method (EPSE, 0, NotSerialized) // EPS Enable
> + * {
> + * \_SB.PCI0.STXS (0xCD)
> + * }
> + *
> + * Method (EPSD, 0, NotSerialized) // EPS Disable
> + * {
> + * \_SB.PCI0.CTXS (0xCD)
> + * }
> + *
> + * Method (_DSM, 4, Serialized) // _DSM: Device-Specific Method
> + * {
> + * ToBuffer (Arg0, Local0)
> + * If ((Local0 == ToUUID ("c7033113-8720-4ceb-9090-9d52b3e52d73")))
> + * {
> + * ToInteger (Arg2, Local1)
> + * If ((Local1 == Zero))
> + * {
> + * Local2 = EPSP ()
> + * If ((Local2 == One))
> + * {
> + * Return (Buffer (One)
> + * {
> + * 0x0F
> + * })
> + * }
> + * }
> + *
> + * If ((Local1 == One))
> + * {
> + * Return (EPSS ())
> + * }
> + *
> + * If ((Local1 == 0x02))
> + * {
> + * EPSE ()
> + * }
> + *
> + * If ((Local1 == 0x03))
> + * {
> + * EPSD ()
> + * }
> + *
> + * Return (Buffer (One)
> + * {
> + * 0x00
> + * })
> + * }
> + *
> + * Return (Buffer (One)
> + * {
> + * 0x00
> + * })
> + * }
> + * }
> + * }
> + */
> +
> +#include <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(struct intel_connector *connector,
> + u64 func)
> +{
> + union acpi_object *obj;
> + acpi_handle acpi_handle = connector->acpi_handle;
> +
> + if (!acpi_handle)
> + return;
> +
> + obj = acpi_evaluate_dsm(acpi_handle, &drm_conn_dsm_guid,
> + CONNECTOR_DSM_REVID, func, NULL);
> + if (!obj) {
> + drm_err(connector->base.dev,
> + "failed to evaluate _DSM for fn %llx\n", func);
> + return;
> + }
> +
> + ACPI_FREE(obj);
> +}
> +
> +void intel_privacy_screen_set_val(struct intel_connector *connector,
> + enum drm_privacy_screen_status val)
> +{
> + if (val == PRIVACY_SCREEN_DISABLED)
> + acpi_privacy_screen_call_dsm(connector,
> + CONNECTOR_DSM_FN_PRIVACY_DISABLE);
> + else if (val == PRIVACY_SCREEN_ENABLED)
> + acpi_privacy_screen_call_dsm(connector,
> + CONNECTOR_DSM_FN_PRIVACY_ENABLE);
> + else
> + drm_err(connector->base.dev,
> + "Cannot set privacy screen to invalid val %u\n", val);
> +}
> +
> +bool intel_privacy_screen_present(struct intel_connector *connector)
> +{
> + acpi_handle handle = 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_dbg_kms(connector->base.dev,
> + "ACPI node but no privacy scrn\n");
> + return false;
> + }
> + drm_info(connector->base.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 0000000000000..f8d2e246ea0bd
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/display/intel_privacy_screen.h
> @@ -0,0 +1,27 @@
> +/* SPDX-License-Identifier: GPL-2.0 OR MIT */
> +/*
> + * 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 *connector);
> +void intel_privacy_screen_set_val(struct intel_connector *connector,
> + enum drm_privacy_screen_status val);
> +#else
> +static bool intel_privacy_screen_present(struct intel_connector *connector)
> +{
> + return false;
> +}
> +
> +static void
> +intel_privacy_screen_set_val(struct intel_connector *connector,
> + enum drm_privacy_screen_status val)
> +{ }
> +#endif /* CONFIG_ACPI */
> +
> +#endif /* __DRM_PRIVACY_SCREEN_H__ */
> --
> 2.25.1.481.gfbce0eb801-goog
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v7 4/4] drm/i915: Add support for integrated privacy screen
2020-03-10 0:18 ` Rajat Jain
@ 2020-03-12 5:55 ` Rajat Jain
2020-03-12 11:24 ` Jani Nikula
0 siblings, 1 reply; 9+ messages in thread
From: Rajat Jain @ 2020-03-12 5:55 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Sean Paul, David Airlie,
Daniel Vetter, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
Ville Syrjälä, Chris Wilson, Imre Deak,
José Roberto de Souza, Linux Kernel Mailing List, dri-devel,
intel-gfx, Greg Kroah-Hartman, Mat King, Daniel Thompson,
Jonathan Corbet, Pavel Machek, Sean Paul, Duncan Laurie,
Jesse Barnes, Thierry Reding, Mark Pearson, Nitin Joshi1,
Sugumaran Lacshiminarayanan, Tomoki Maruichi
Cc: Rajat Jain
Hi Jani,
On Mon, Mar 9, 2020 at 5:18 PM Rajat Jain <rajatja@google.com> wrote:
>
> Hi Jani,
>
> I have 1 question / need 1 help about this patch:
Kind ignore, I found the answer, and posted my new patchset here:
https://patchwork.freedesktop.org/series/74607/
I got a "failed to apply" email from the patchwork. Can you please let
me know on which branch is it trying to apply it? I have currently
rebased my patchset to drm-intel-next-queued.
Thanks & Best Regards,
Rajat
>
> On Mon, Mar 9, 2020 at 5:06 PM Rajat Jain <rajatja@google.com> wrote:
> >
> > Add support for an ACPI based integrated privacy screen that is
> > available on some systems.
> >
> > Signed-off-by: Rajat Jain <rajatja@google.com>
> > ---
> > v7: * Move the privacy-screen property back into drm core.
> > * Do the actual HW EPS toggling at commit time.
> > * Provide a sample ACPI node for reference in comments.
> > v6: Always initialize prop in intel_attach_privacy_screen_property()
> > v5: fix a cosmetic checkpatch warning
> > v4: Fix a typo in intel_privacy_screen.h
> > v3: * Change license to GPL-2.0 OR MIT
> > * Move privacy screen enum from UAPI to intel_display_types.h
> > * Rename parameter name and some other minor changes.
> > v2: Formed by splitting the original patch into multiple patches.
> > - All code has been moved into i915 now.
> > - Privacy screen is a i915 property
> > - Have a local state variable to store the prvacy screen. Don't read
> > it from hardware.
> >
> > drivers/gpu/drm/i915/Makefile | 3 +-
> > drivers/gpu/drm/i915/display/intel_atomic.c | 1 +
> > drivers/gpu/drm/i915/display/intel_dp.c | 30 ++-
> > .../drm/i915/display/intel_privacy_screen.c | 175 ++++++++++++++++++
> > .../drm/i915/display/intel_privacy_screen.h | 27 +++
> > 5 files changed, 234 insertions(+), 2 deletions(-)
> > create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.c
> > create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.h
> >
> > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> > index 9f887a86e555d..da42389107f9c 100644
> > --- a/drivers/gpu/drm/i915/Makefile
> > +++ b/drivers/gpu/drm/i915/Makefile
> > @@ -209,7 +209,8 @@ i915-y += \
> > display/intel_vga.o
> > i915-$(CONFIG_ACPI) += \
> > display/intel_acpi.o \
> > - display/intel_opregion.o
> > + display/intel_opregion.o \
> > + display/intel_privacy_screen.o
> > i915-$(CONFIG_DRM_FBDEV_EMULATION) += \
> > display/intel_fbdev.o
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c b/drivers/gpu/drm/i915/display/intel_atomic.c
> > index d043057d2fa03..fc6264b4a7f73 100644
> > --- a/drivers/gpu/drm/i915/display/intel_atomic.c
> > +++ b/drivers/gpu/drm/i915/display/intel_atomic.c
> > @@ -150,6 +150,7 @@ int intel_digital_connector_atomic_check(struct drm_connector *conn,
> > new_conn_state->base.picture_aspect_ratio != old_conn_state->base.picture_aspect_ratio ||
> > new_conn_state->base.content_type != old_conn_state->base.content_type ||
> > new_conn_state->base.scaling_mode != old_conn_state->base.scaling_mode ||
> > + new_conn_state->base.privacy_screen_status != old_conn_state->base.privacy_screen_status ||
> > !blob_equal(new_conn_state->base.hdr_output_metadata,
> > old_conn_state->base.hdr_output_metadata))
> > crtc_state->mode_changed = true;
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 41c623b029464..a39b0c420b50a 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -62,6 +62,7 @@
> > #include "intel_lspcon.h"
> > #include "intel_lvds.h"
> > #include "intel_panel.h"
> > +#include "intel_privacy_screen.h"
> > #include "intel_psr.h"
> > #include "intel_sideband.h"
> > #include "intel_tc.h"
> > @@ -5886,6 +5887,12 @@ intel_dp_connector_register(struct drm_connector *connector)
> > dev_priv->acpi_scan_done = true;
> > }
> >
> > + /* Check for integrated Privacy screen support */
> > + if (intel_privacy_screen_present(to_intel_connector(connector)))
> > + drm_connector_attach_privacy_screen_property(connector);
> > + else
> > + drm_connector_destroy_privacy_screen_property(connector);
> > +
> > DRM_DEBUG_KMS("registering %s bus for %s\n",
> > intel_dp->aux.name, connector->kdev->kobj.name);
> >
> > @@ -6881,9 +6888,30 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
> > drm_connector_attach_scaling_mode_property(connector, allowed_scalers);
> >
> > connector->state->scaling_mode = DRM_MODE_SCALE_ASPECT;
> > +
> > + drm_connector_create_privacy_screen_property(connector);
> > }
> > }
> >
> > +static void intel_dp_update_privacy_screen(struct intel_encoder *encoder,
> > + const struct intel_crtc_state *crtc_state,
> > + const struct drm_connector_state *conn_state)
> > +{
> > + struct drm_connector *connector = conn_state->connector;
> > +
> > + if (connector->privacy_screen_property)
> > + intel_privacy_screen_set_val(to_intel_connector(connector),
> > + conn_state->privacy_screen_status);
> > +}
> > +
> > +static void intel_dp_update_pipe(struct intel_encoder *encoder,
> > + const struct intel_crtc_state *crtc_state,
> > + const struct drm_connector_state *conn_state)
> > +{
> > + intel_dp_update_privacy_screen(encoder, crtc_state, conn_state);
> > + intel_panel_update_backlight(encoder, crtc_state, conn_state);
> > +}
> > +
> > static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp)
> > {
> > intel_dp->panel_power_off_time = ktime_get_boottime();
> > @@ -7825,7 +7853,7 @@ bool intel_dp_init(struct drm_i915_private *dev_priv,
> > intel_encoder->compute_config = intel_dp_compute_config;
> > intel_encoder->get_hw_state = intel_dp_get_hw_state;
> > intel_encoder->get_config = intel_dp_get_config;
> > - intel_encoder->update_pipe = intel_panel_update_backlight;
> > + intel_encoder->update_pipe = intel_dp_update_pipe;
>
> For my testing purposes, I'm testing this using the proptest userspace utility
> in our distribution (I think from
> https://github.com/CPFL/drm/blob/master/tests/proptest/proptest.c). I
> notice that when I change the value of the property from userspace,
> even though the drm_connector_state->privacy_screen_status gets
> updated and reflects the change, the encoder->update_pipe() is not
> getting called. Just wanted to ask if this is expected since you seem
> to have implied that this update_pipe() might *not* get called if there *is* a
> full modeset? (What is the hook that gets called for a full modeset
> where i915 driver should commit this property change to the hardware?)
>
> Thanks a lot for all your help,
>
> Best Regards,
>
> Rajat
>
> > intel_encoder->suspend = intel_dp_encoder_suspend;
> > if (IS_CHERRYVIEW(dev_priv)) {
> > intel_encoder->pre_pll_enable = chv_dp_pre_pll_enable;
> > 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 0000000000000..6ff61ddf4c0a4
> > --- /dev/null
> > +++ b/drivers/gpu/drm/i915/display/intel_privacy_screen.c
> > @@ -0,0 +1,175 @@
> > +// SPDX-License-Identifier: GPL-2.0 OR MIT
> > +/*
> > + * Intel ACPI privacy screen code
> > + *
> > + * Copyright © 2020 Google Inc.
> > + *
> > + * This code can help detect and control an integrated EPS (electronic
> > + * privacy screen) via ACPI functions. It expects an ACPI node for the
> > + * drm connector device with the following elements:
> > + *
> > + * UUID should be "c7033113-8720-4ceb-9090-9d52b3e52d73"
> > + *
> > + * _ADR = ACPI address per Spec (also see intel_acpi_device_id_update())
> > + * https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf
> > + * Pages 1119 - 1123.
> > + *
> > + * _DSM method that will perform the following functions according to
> > + * Local1 argument passed to it:
> > + * - Local1 = 0 (EPS capabilities): Report EPS presence and capabilities.
> > + * - Local1 = 1 (EPS State) : _DSM returns 1 if EPS is enabled, 0 otherwise.
> > + * - Local1 = 2 (EPS Enable) : _DSM enables EPS
> > + * - Local1 = 3 (EPS Disable): _DSM disables EPS
> > + *
> > + * Here is a sample ACPI node:
> > + *
> > + * Scope (\_SB.PCI0.GFX0) // Intel graphics device (PCI device)
> > + * {
> > + * Method (_DOD, 0, NotSerialized) // _DOD: Display Output Devices
> > + * {
> > + * Return (Package (0x01)
> > + * {
> > + * 0x80010400
> > + * })
> > + * }
> > + *
> > + * Device (LCD)
> > + * {
> > + * Name (_ADR, 0x80010400) // _ADR: Address
> > + * Name (_STA, 0x0F) // _STA: Status
> > + *
> > + * Method (EPSP, 0, NotSerialized) // EPS Present
> > + * {
> > + * Return (0x01)
> > + * }
> > + *
> > + * Method (EPSS, 0, NotSerialized) // EPS State
> > + * {
> > + * Local0 = \_SB.PCI0.GRXS (0xCD)
> > + * Return (Local0)
> > + * }
> > + *
> > + * Method (EPSE, 0, NotSerialized) // EPS Enable
> > + * {
> > + * \_SB.PCI0.STXS (0xCD)
> > + * }
> > + *
> > + * Method (EPSD, 0, NotSerialized) // EPS Disable
> > + * {
> > + * \_SB.PCI0.CTXS (0xCD)
> > + * }
> > + *
> > + * Method (_DSM, 4, Serialized) // _DSM: Device-Specific Method
> > + * {
> > + * ToBuffer (Arg0, Local0)
> > + * If ((Local0 == ToUUID ("c7033113-8720-4ceb-9090-9d52b3e52d73")))
> > + * {
> > + * ToInteger (Arg2, Local1)
> > + * If ((Local1 == Zero))
> > + * {
> > + * Local2 = EPSP ()
> > + * If ((Local2 == One))
> > + * {
> > + * Return (Buffer (One)
> > + * {
> > + * 0x0F
> > + * })
> > + * }
> > + * }
> > + *
> > + * If ((Local1 == One))
> > + * {
> > + * Return (EPSS ())
> > + * }
> > + *
> > + * If ((Local1 == 0x02))
> > + * {
> > + * EPSE ()
> > + * }
> > + *
> > + * If ((Local1 == 0x03))
> > + * {
> > + * EPSD ()
> > + * }
> > + *
> > + * Return (Buffer (One)
> > + * {
> > + * 0x00
> > + * })
> > + * }
> > + *
> > + * Return (Buffer (One)
> > + * {
> > + * 0x00
> > + * })
> > + * }
> > + * }
> > + * }
> > + */
> > +
> > +#include <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(struct intel_connector *connector,
> > + u64 func)
> > +{
> > + union acpi_object *obj;
> > + acpi_handle acpi_handle = connector->acpi_handle;
> > +
> > + if (!acpi_handle)
> > + return;
> > +
> > + obj = acpi_evaluate_dsm(acpi_handle, &drm_conn_dsm_guid,
> > + CONNECTOR_DSM_REVID, func, NULL);
> > + if (!obj) {
> > + drm_err(connector->base.dev,
> > + "failed to evaluate _DSM for fn %llx\n", func);
> > + return;
> > + }
> > +
> > + ACPI_FREE(obj);
> > +}
> > +
> > +void intel_privacy_screen_set_val(struct intel_connector *connector,
> > + enum drm_privacy_screen_status val)
> > +{
> > + if (val == PRIVACY_SCREEN_DISABLED)
> > + acpi_privacy_screen_call_dsm(connector,
> > + CONNECTOR_DSM_FN_PRIVACY_DISABLE);
> > + else if (val == PRIVACY_SCREEN_ENABLED)
> > + acpi_privacy_screen_call_dsm(connector,
> > + CONNECTOR_DSM_FN_PRIVACY_ENABLE);
> > + else
> > + drm_err(connector->base.dev,
> > + "Cannot set privacy screen to invalid val %u\n", val);
> > +}
> > +
> > +bool intel_privacy_screen_present(struct intel_connector *connector)
> > +{
> > + acpi_handle handle = 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_dbg_kms(connector->base.dev,
> > + "ACPI node but no privacy scrn\n");
> > + return false;
> > + }
> > + drm_info(connector->base.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 0000000000000..f8d2e246ea0bd
> > --- /dev/null
> > +++ b/drivers/gpu/drm/i915/display/intel_privacy_screen.h
> > @@ -0,0 +1,27 @@
> > +/* SPDX-License-Identifier: GPL-2.0 OR MIT */
> > +/*
> > + * 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 *connector);
> > +void intel_privacy_screen_set_val(struct intel_connector *connector,
> > + enum drm_privacy_screen_status val);
> > +#else
> > +static bool intel_privacy_screen_present(struct intel_connector *connector)
> > +{
> > + return false;
> > +}
> > +
> > +static void
> > +intel_privacy_screen_set_val(struct intel_connector *connector,
> > + enum drm_privacy_screen_status val)
> > +{ }
> > +#endif /* CONFIG_ACPI */
> > +
> > +#endif /* __DRM_PRIVACY_SCREEN_H__ */
> > --
> > 2.25.1.481.gfbce0eb801-goog
> >
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v7 4/4] drm/i915: Add support for integrated privacy screen
2020-03-12 5:55 ` Rajat Jain
@ 2020-03-12 11:24 ` Jani Nikula
2020-04-06 22:07 ` Rajat Jain
0 siblings, 1 reply; 9+ messages in thread
From: Jani Nikula @ 2020-03-12 11:24 UTC (permalink / raw)
To: Rajat Jain, Maarten Lankhorst, Maxime Ripard, Sean Paul,
David Airlie, Daniel Vetter, Joonas Lahtinen, Rodrigo Vivi,
Ville Syrjälä, Chris Wilson, Imre Deak,
José Roberto de Souza, Linux Kernel Mailing List, dri-devel,
intel-gfx, Greg Kroah-Hartman, Mat King, Daniel Thompson,
Jonathan Corbet, Pavel Machek, Sean Paul, Duncan Laurie,
Jesse Barnes, Thierry Reding, Mark Pearson, Nitin Joshi1,
Sugumaran Lacshiminarayanan, Tomoki Maruichi
Cc: Rajat Jain
On Wed, 11 Mar 2020, Rajat Jain <rajatja@google.com> wrote:
> I got a "failed to apply" email from the patchwork. Can you please let
> me know on which branch is it trying to apply it? I have currently
> rebased my patchset to drm-intel-next-queued.
drm-tip branch of https://cgit.freedesktop.org/drm/drm-tip
It's kind of like linux-next of drm.
BR,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v7 4/4] drm/i915: Add support for integrated privacy screen
2020-03-12 11:24 ` Jani Nikula
@ 2020-04-06 22:07 ` Rajat Jain
0 siblings, 0 replies; 9+ messages in thread
From: Rajat Jain @ 2020-04-06 22:07 UTC (permalink / raw)
To: Jani Nikula
Cc: Maarten Lankhorst, Maxime Ripard, Sean Paul, David Airlie,
Daniel Vetter, Joonas Lahtinen, Rodrigo Vivi,
Ville Syrjälä, Chris Wilson, Imre Deak,
José Roberto de Souza, Linux Kernel Mailing List, dri-devel,
intel-gfx, Greg Kroah-Hartman, Mat King, Daniel Thompson,
Jonathan Corbet, Pavel Machek, Sean Paul, Duncan Laurie,
Jesse Barnes, Thierry Reding, Mark Pearson, Nitin Joshi1,
Sugumaran Lacshiminarayanan, Tomoki Maruichi, Rajat Jain
On Thu, Mar 12, 2020 at 4:24 AM Jani Nikula <jani.nikula@linux.intel.com> wrote:
>
> On Wed, 11 Mar 2020, Rajat Jain <rajatja@google.com> wrote:
> > I got a "failed to apply" email from the patchwork. Can you please let
> > me know on which branch is it trying to apply it? I have currently
> > rebased my patchset to drm-intel-next-queued.
>
> drm-tip branch of https://cgit.freedesktop.org/drm/drm-tip
>
> It's kind of like linux-next of drm.
>
Hi Jani,
Haven't heard any comments in a while, so just checking to see if you
got a chance to look at my last patchset. It is here:
https://patchwork.freedesktop.org/patch/357452/
Thanks & Best Regards,
Rajat Jain
> BR,
> Jani.
>
>
> --
> Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 9+ messages in thread