[PATCH v7 0/4] drm: Add support for integrated privacy screen

[PATCH v7 0/4] drm: Add support for integrated privacy screen

[Rajat Jain]

This patchset adds:
 - (Generic) integrated privacy screen support in drm
 - Ability in i915 to attach to ACPI nodes
 - an implmentation for ACPI privacy screen in i915

Rajat Jain (4):
  intel_acpi: Rename drm_dev local variable to dev
  drm/connector: Add support for privacy-screen property
  drm/i915: Lookup and attach ACPI device node for connectors
  drm/i915: Add support for integrated privacy screen

 drivers/gpu/drm/drm_atomic_uapi.c             |   4 +
 drivers/gpu/drm/drm_connector.c               |  56 ++++++
 drivers/gpu/drm/i915/Makefile                 |   3 +-
 drivers/gpu/drm/i915/display/intel_acpi.c     |  28 ++-
 drivers/gpu/drm/i915/display/intel_atomic.c   |   1 +
 .../drm/i915/display/intel_display_types.h    |   5 +
 drivers/gpu/drm/i915/display/intel_dp.c       |  43 ++++-
 .../drm/i915/display/intel_privacy_screen.c   | 175 ++++++++++++++++++
 .../drm/i915/display/intel_privacy_screen.h   |  27 +++
 drivers/gpu/drm/i915/i915_drv.h               |   2 +
 include/drm/drm_connector.h                   |  25 +++
 11 files changed, 365 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

-- 
2.25.1.481.gfbce0eb801-goog

[PATCH v7 0/4] drm: Add support for integrated privacy screen

[Rajat Jain]

This patchset adds:
 - (Generic) integrated privacy screen support in drm
 - Ability in i915 to attach to ACPI nodes
 - an implmentation for ACPI privacy screen in i915

Rajat Jain (4):
  intel_acpi: Rename drm_dev local variable to dev
  drm/connector: Add support for privacy-screen property
  drm/i915: Lookup and attach ACPI device node for connectors
  drm/i915: Add support for integrated privacy screen

 drivers/gpu/drm/drm_atomic_uapi.c             |   4 +
 drivers/gpu/drm/drm_connector.c               |  56 ++++++
 drivers/gpu/drm/i915/Makefile                 |   3 +-
 drivers/gpu/drm/i915/display/intel_acpi.c     |  28 ++-
 drivers/gpu/drm/i915/display/intel_atomic.c   |   1 +
 .../drm/i915/display/intel_display_types.h    |   5 +
 drivers/gpu/drm/i915/display/intel_dp.c       |  43 ++++-
 .../drm/i915/display/intel_privacy_screen.c   | 175 ++++++++++++++++++
 .../drm/i915/display/intel_privacy_screen.h   |  27 +++
 drivers/gpu/drm/i915/i915_drv.h               |   2 +
 include/drm/drm_connector.h                   |  25 +++
 11 files changed, 365 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

-- 
2.25.1.481.gfbce0eb801-goog

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Rajat Jain]

This patchset adds:
 - (Generic) integrated privacy screen support in drm
 - Ability in i915 to attach to ACPI nodes
 - an implmentation for ACPI privacy screen in i915

Rajat Jain (4):
  intel_acpi: Rename drm_dev local variable to dev
  drm/connector: Add support for privacy-screen property
  drm/i915: Lookup and attach ACPI device node for connectors
  drm/i915: Add support for integrated privacy screen

 drivers/gpu/drm/drm_atomic_uapi.c             |   4 +
 drivers/gpu/drm/drm_connector.c               |  56 ++++++
 drivers/gpu/drm/i915/Makefile                 |   3 +-
 drivers/gpu/drm/i915/display/intel_acpi.c     |  28 ++-
 drivers/gpu/drm/i915/display/intel_atomic.c   |   1 +
 .../drm/i915/display/intel_display_types.h    |   5 +
 drivers/gpu/drm/i915/display/intel_dp.c       |  43 ++++-
 .../drm/i915/display/intel_privacy_screen.c   | 175 ++++++++++++++++++
 .../drm/i915/display/intel_privacy_screen.h   |  27 +++
 drivers/gpu/drm/i915/i915_drv.h               |   2 +
 include/drm/drm_connector.h                   |  25 +++
 11 files changed, 365 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

-- 
2.25.1.481.gfbce0eb801-goog

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

[PATCH v7 1/4] intel_acpi: Rename drm_dev local variable to dev

[Rajat Jain]

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

[PATCH v7 1/4] intel_acpi: Rename drm_dev local variable to dev

[Rajat Jain]

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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Intel-gfx] [PATCH v7 1/4] intel_acpi: Rename drm_dev local variable to dev

[Rajat Jain]

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

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

[PATCH v7 2/4] drm/connector: Add support for privacy-screen property

[Rajat Jain]

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

[PATCH v7 2/4] drm/connector: Add support for privacy-screen property

[Rajat Jain]

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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Intel-gfx] [PATCH v7 2/4] drm/connector: Add support for privacy-screen property

[Rajat Jain]

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

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

[PATCH v7 3/4] drm/i915: Lookup and attach ACPI device node for connectors

[Rajat Jain]

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

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

Signed-off-by: Rajat Jain <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

[PATCH v7 3/4] drm/i915: Lookup and attach ACPI device node for connectors

[Rajat Jain]

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

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

Signed-off-by: Rajat Jain <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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Rajat Jain]

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

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

Signed-off-by: Rajat Jain <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

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

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

[Rajat Jain]

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

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

[Rajat Jain]

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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Rajat Jain]

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

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

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

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

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

[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
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[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
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm: Add support for integrated privacy screen

[Patchwork]

== Series Details ==

Series: drm: Add support for integrated privacy screen
URL   : https://patchwork.freedesktop.org/series/74473/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
4d85b0afcb69 intel_acpi: Rename drm_dev local variable to dev
8f19bae8d1da drm/connector: Add support for privacy-screen property
ad4ee254fd45 drm/i915: Lookup and attach ACPI device node for connectors
4d56cad4f624 drm/i915: Add support for integrated privacy screen
-:33: WARNING:LONG_LINE: line over 100 characters
#33: FILE: drivers/gpu/drm/i915/display/intel_atomic.c:153:
+	    new_conn_state->base.privacy_screen_status != old_conn_state->base.privacy_screen_status ||

-:72: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#72: FILE: drivers/gpu/drm/i915/display/intel_dp.c:6897:
+static void intel_dp_update_privacy_screen(struct intel_encoder *encoder,
+				const struct intel_crtc_state *crtc_state,

-:103: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#103: 
new file mode 100644

total: 0 errors, 2 warnings, 1 checks, 275 lines checked

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

[Intel-gfx] ✓ Fi.CI.BAT: success for drm: Add support for integrated privacy screen

[Patchwork]

== Series Details ==

Series: drm: Add support for integrated privacy screen
URL   : https://patchwork.freedesktop.org/series/74473/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_8106 -> Patchwork_16899
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16899/index.html

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_16899:

### IGT changes ###

#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@runner@aborted:
    - {fi-ehl-1}:         NOTRUN -> [FAIL][1]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16899/fi-ehl-1/igt@runner@aborted.html
    - {fi-tgl-u}:         NOTRUN -> [FAIL][2]
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16899/fi-tgl-u/igt@runner@aborted.html

  
Known issues
------------

  Here are the changes found in Patchwork_16899 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_parallel@fds:
    - fi-cfl-guc:         [PASS][3] -> [INCOMPLETE][4] ([i915#1147])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8106/fi-cfl-guc/igt@gem_exec_parallel@fds.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16899/fi-cfl-guc/igt@gem_exec_parallel@fds.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [i915#1147]: https://gitlab.freedesktop.org/drm/intel/issues/1147


Participating hosts (44 -> 39)
------------------------------

  Additional (4): fi-bdw-5557u fi-cfl-8109u fi-skl-6600u fi-snb-2600 
  Missing    (9): fi-hsw-4200u fi-bsw-cyan fi-bwr-2160 fi-ilk-650 fi-ctg-p8600 fi-ivb-3770 fi-byt-clapper fi-bdw-samus fi-kbl-r 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_8106 -> Patchwork_16899

  CI-20190529: 20190529
  CI_DRM_8106: 5b0076e8066ea8218e7857ee1aa28b0670acde94 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5504: d6788bf0404f76b66170e18eb26c85004b5ccb25 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_16899: fb6ae6565e49137d4108902af80b311fb5f2cdeb @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

fb6ae6565e49 drm/i915: Add support for integrated privacy screen
33c026e8a5c6 drm/i915: Lookup and attach ACPI device node for connectors
066f3db05803 drm/connector: Add support for privacy-screen property
929c4147347f intel_acpi: Rename drm_dev local variable to dev

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16899/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ Fi.CI.IGT: failure for drm: Add support for integrated privacy screen

[Patchwork]

== Series Details ==

Series: drm: Add support for integrated privacy screen
URL   : https://patchwork.freedesktop.org/series/74473/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_8106_full -> Patchwork_16899_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_16899_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_16899_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_16899_full:

### IGT changes ###

#### Possible regressions ####

  * igt@runner@aborted:
    - shard-tglb:         NOTRUN -> ([FAIL][1], [FAIL][2], [FAIL][3], [FAIL][4], [FAIL][5], [FAIL][6], [FAIL][7], [FAIL][8], [FAIL][9], [FAIL][10], [FAIL][11], [FAIL][12], [FAIL][13], [FAIL][14], [FAIL][15], [FAIL][16], [FAIL][17], [FAIL][18], [FAIL][19], [FAIL][20], [FAIL][21], [FAIL][22], [FAIL][23], [FAIL][24], [FAIL][25])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16899/shard-tglb3/igt@runner@aborted.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16899/shard-tglb5/igt@runner@aborted.html
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16899/shard-tglb8/igt@runner@aborted.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16899/shard-tglb6/igt@runner@aborted.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16899/shard-tglb3/igt@runner@aborted.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16899/shard-tglb5/igt@runner@aborted.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16899/shard-tglb5/igt@runner@aborted.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16899/shard-tglb6/igt@runner@aborted.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16899/shard-tglb5/igt@runner@aborted.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16899/shard-tglb6/igt@runner@aborted.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16899/shard-tglb5/igt@runner@aborted.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16899/shard-tglb5/igt@runner@aborted.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16899/shard-tglb3/igt@runner@aborted.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16899/shard-tglb7/igt@runner@aborted.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16899/shard-tglb3/igt@runner@aborted.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16899/shard-tglb7/igt@runner@aborted.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16899/shard-tglb3/igt@runner@aborted.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16899/shard-tglb8/igt@runner@aborted.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16899/shard-tglb1/igt@runner@aborted.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16899/shard-tglb2/igt@runner@aborted.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16899/shard-tglb1/igt@runner@aborted.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16899/shard-tglb1/igt@runner@aborted.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16899/shard-tglb1/igt@runner@aborted.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16899/shard-tglb2/igt@runner@aborted.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16899/shard-tglb2/igt@runner@aborted.html

  
Known issues
------------

  Here are the changes found in Patchwork_16899_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_whisper@basic-fds:
    - shard-glk:          [PASS][26] -> [DMESG-WARN][27] ([i915#118] / [i915#95]) +2 similar issues
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8106/shard-glk6/igt@gem_exec_whisper@basic-fds.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16899/shard-glk8/igt@gem_exec_whisper@basic-fds.html

  * igt@gen9_exec_parse@allowed-all:
    - shard-glk:          [PASS][28] -> [INCOMPLETE][29] ([i915#58] / [k.org#198133])
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8106/shard-glk5/igt@gen9_exec_parse@allowed-all.html
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16899/shard-glk4/igt@gen9_exec_parse@allowed-all.html

  * igt@kms_fbcon_fbt@fbc-suspend:
    - shard-apl:          [PASS][30] -> [DMESG-WARN][31] ([i915#180]) +2 similar issues
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8106/shard-apl1/igt@kms_fbcon_fbt@fbc-suspend.html
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16899/shard-apl6/igt@kms_fbcon_fbt@fbc-suspend.html

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-pwrite:
    - shard-glk:          [PASS][32] -> [FAIL][33] ([i915#49])
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8106/shard-glk8/igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-pwrite.html
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16899/shard-glk5/igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-pwrite.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes:
    - shard-kbl:          [PASS][34] -> [DMESG-WARN][35] ([i915#180]) +6 similar issues
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8106/shard-kbl1/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes.html
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16899/shard-kbl2/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes.html

  * igt@kms_setmode@basic:
    - shard-apl:          [PASS][36] -> [FAIL][37] ([i915#31])
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8106/shard-apl1/igt@kms_setmode@basic.html
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16899/shard-apl8/igt@kms_setmode@basic.html

  
#### Possible fixes ####

  * igt@gem_ctx_isolation@rcs0-s3:
    - shard-kbl:          [DMESG-WARN][38] ([i915#180]) -> [PASS][39] +1 similar issue
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8106/shard-kbl1/igt@gem_ctx_isolation@rcs0-s3.html
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16899/shard-kbl7/igt@gem_ctx_isolation@rcs0-s3.html

  * igt@gem_ctx_persistence@close-replace-race:
    - shard-apl:          [INCOMPLETE][40] ([fdo#103927] / [i915#1402]) -> [PASS][41]
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8106/shard-apl1/igt@gem_ctx_persistence@close-replace-race.html
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16899/shard-apl6/igt@gem_ctx_persistence@close-replace-race.html

  * igt@gem_ctx_persistence@processes:
    - shard-kbl:          [FAIL][42] ([i915#570] / [i915#679]) -> [PASS][43]
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8106/shard-kbl2/igt@gem_ctx_persistence@processes.html
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16899/shard-kbl6/igt@gem_ctx_persistence@processes.html

  * igt@gem_exec_whisper@basic-queues-forked:
    - shard-glk:          [DMESG-WARN][44] ([i915#118] / [i915#95]) -> [PASS][45]
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8106/shard-glk2/igt@gem_exec_whisper@basic-queues-forked.html
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16899/shard-glk6/igt@gem_exec_whisper@basic-queues-forked.html

  * igt@gem_linear_blits@normal:
    - shard-apl:          [TIMEOUT][46] ([i915#1322]) -> [PASS][47]
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8106/shard-apl4/igt@gem_linear_blits@normal.html
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16899/shard-apl6/igt@gem_linear_blits@normal.html

  * igt@i915_suspend@sysfs-reader:
    - shard-snb:          [DMESG-WARN][48] ([i915#42]) -> [PASS][49]
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8106/shard-snb4/igt@i915_suspend@sysfs-reader.html
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16899/shard-snb4/igt@i915_suspend@sysfs-reader.html

  * igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
    - shard-glk:          [FAIL][50] ([i915#79]) -> [PASS][51]
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8106/shard-glk9/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible.html
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16899/shard-glk5/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible.html

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - shard-kbl:          [INCOMPLETE][52] -> [PASS][53]
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8106/shard-kbl2/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16899/shard-kbl6/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html

  * igt@kms_vblank@pipe-a-ts-continuation-suspend:
    - shard-apl:          [DMESG-WARN][54] ([i915#180]) -> [PASS][55] +1 similar issue
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8106/shard-apl1/igt@kms_vblank@pipe-a-ts-continuation-suspend.html
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16899/shard-apl7/igt@kms_vblank@pipe-a-ts-continuation-suspend.html

  
#### Warnings ####

  * igt@i915_pm_rpm@fences:
    - shard-snb:          [INCOMPLETE][56] ([i915#82]) -> [SKIP][57] ([fdo#109271])
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8106/shard-snb4/igt@i915_pm_rpm@fences.html
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16899/shard-snb6/igt@i915_pm_rpm@fences.html

  * igt@kms_flip@flip-vs-suspend-interruptible:
    - shard-kbl:          [DMESG-WARN][58] ([i915#180] / [i915#56]) -> [DMESG-WARN][59] ([i915#180])
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8106/shard-kbl1/igt@kms_flip@flip-vs-suspend-interruptible.html
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16899/shard-kbl2/igt@kms_flip@flip-vs-suspend-interruptible.html

  * igt@runner@aborted:
    - shard-apl:          ([FAIL][60], [FAIL][61]) ([fdo#103927] / [i915#1402]) -> [FAIL][62] ([fdo#103927])
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8106/shard-apl1/igt@runner@aborted.html
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8106/shard-apl2/igt@runner@aborted.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16899/shard-apl4/igt@runner@aborted.html

  
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#118]: https://gitlab.freedesktop.org/drm/intel/issues/118
  [i915#1322]: https://gitlab.freedesktop.org/drm/intel/issues/1322
  [i915#1402]: https://gitlab.freedesktop.org/drm/intel/issues/1402
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#31]: https://gitlab.freedesktop.org/drm/intel/issues/31
  [i915#42]: https://gitlab.freedesktop.org/drm/intel/issues/42
  [i915#49]: https://gitlab.freedesktop.org/drm/intel/issues/49
  [i915#56]: https://gitlab.freedesktop.org/drm/intel/issues/56
  [i915#570]: https://gitlab.freedesktop.org/drm/intel/issues/570
  [i915#58]: https://gitlab.freedesktop.org/drm/intel/issues/58
  [i915#679]: https://gitlab.freedesktop.org/drm/intel/issues/679
  [i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
  [i915#82]: https://gitlab.freedesktop.org/drm/intel/issues/82
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133


Participating hosts (10 -> 10)
------------------------------

  No changes in participating hosts


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_8106 -> Patchwork_16899

  CI-20190529: 20190529
  CI_DRM_8106: 5b0076e8066ea8218e7857ee1aa28b0670acde94 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5504: d6788bf0404f76b66170e18eb26c85004b5ccb25 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_16899: fb6ae6565e49137d4108902af80b311fb5f2cdeb @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16899/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

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

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

[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
> >
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[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
> >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Jani Nikula]

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

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

[Jani Nikula]

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Jani Nikula]

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[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

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

[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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx