[Intel-gfx] [PATCH v9 0/5] drm/i915 Support for integrated privacy screen

[Intel-gfx] [PATCH v9 0/5] drm/i915 Support for integrated privacy screen

[Rajat Jain]

This patchset adds support for integrated privacy screen on some laptops
using the ACPI methods to detect and control the feature.

Rajat Jain (5):
  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 helper code for ACPI privacy screen
  drm/i915: Enable support for integrated privacy screen

 drivers/gpu/drm/drm_atomic_uapi.c             |   4 +
 drivers/gpu/drm/drm_connector.c               |  51 +++++
 drivers/gpu/drm/i915/Makefile                 |   3 +-
 drivers/gpu/drm/i915/display/intel_acpi.c     |  30 ++-
 drivers/gpu/drm/i915/display/intel_atomic.c   |   2 +
 drivers/gpu/drm/i915/display/intel_ddi.c      |   1 +
 .../drm/i915/display/intel_display_types.h    |   5 +
 drivers/gpu/drm/i915/display/intel_dp.c       |  48 ++++-
 drivers/gpu/drm/i915/display/intel_dp.h       |   5 +
 .../drm/i915/display/intel_privacy_screen.c   | 184 ++++++++++++++++++
 .../drm/i915/display/intel_privacy_screen.h   |  27 +++
 drivers/gpu/drm/i915/i915_drv.h               |   2 +
 include/drm/drm_connector.h                   |  24 +++
 13 files changed, 382 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

[Intel-gfx] [PATCH v9 1/5] 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>
---
v9: Same as v8
v8: same as v7
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

[Intel-gfx] [PATCH v9 2/5] 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>
---
v9: rebased on top of https://cgit.freedesktop.org/drm/drm-tip:drm-tip
v8: Remove the ...destroy_privacy_screen() method and let the property
    be destroyed along with others at the time of device destruction.
    (because drm core doesn't also like properties destroyed in
     late_register()).
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   | 51 +++++++++++++++++++++++++++++++
 include/drm/drm_connector.h       | 24 +++++++++++++++
 3 files changed, 79 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 644f0ad106717..182aa557962b2 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1186,6 +1186,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)
@@ -2152,6 +2156,53 @@ 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_create_privacy_screen_property -
+ *     create the drm connecter's privacy-screen property.
+ * @connector: connector for which to create the privacy-screen property
+ *
+ * This function creates the "privacy-screen" property for the
+ * connector. It is not attached.
+ */
+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);
+
+/**
+ * drm_connector_attach_privacy_screen_property -
+ *     attach the drm connecter's privacy-screen property.
+ * @connector: connector on which to attach the privacy-screen property
+ *
+ * This function attaches the "privacy-screen" property to the
+ * connector. Initial state of privacy-screen is set to disabled.
+ */
+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);
+
 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 19ae6bb5c85be..f9ce89cc13542 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -271,6 +271,20 @@ struct drm_monitor_range_info {
 	u8 max_vfreq;
 };
 
+/**
+ * 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
@@ -686,6 +700,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
@@ -1285,6 +1301,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)
@@ -1598,6 +1620,8 @@ 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);
 
 /**
  * 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

[Intel-gfx] [PATCH v9 3/5] 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>
---
v9: same as v8
v8: Also print the connector name in debug message.
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     | 26 +++++++++++++++++++
 .../drm/i915/display/intel_display_types.h    |  5 ++++
 drivers/gpu/drm/i915/display/intel_dp.c       | 14 ++++++++++
 drivers/gpu/drm/i915/i915_drv.h               |  2 ++
 4 files changed, 47 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_acpi.c b/drivers/gpu/drm/i915/display/intel_acpi.c
index 3e6831cca4ac1..c203fdd7fda8d 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,19 @@ 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, "%s: Looking for ACPI node at _ADR=%llX\n",
+			    connector->base.kdev ?
+				dev_name(connector->base.kdev) : "NONAME",
+			    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..3ddc424b028c1 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"
@@ -5864,6 +5865,7 @@ static int
 intel_dp_connector_register(struct drm_connector *connector)
 {
 	struct intel_dp *intel_dp = intel_attached_dp(to_intel_connector(connector));
+	struct drm_i915_private *dev_priv = to_i915(connector->dev);
 	int ret;
 
 	ret = intel_connector_register(connector);
@@ -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);
 
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

[Intel-gfx] [PATCH v9 4/5] drm/i915: Add helper code for ACPI privacy screen

[Rajat Jain]

Add helper functions that can allow i915 to detect and control
an integrated privacy screen via ACPI methods. These shall be used
in the next patch.

Signed-off-by: Rajat Jain <rajatja@google.com>
---
v9: same as v8
v8: Initial version. formed by refactoring the previous patch 4.
    print the connector name in the debug messages.

 drivers/gpu/drm/i915/Makefile                 |   3 +-
 .../drm/i915/display/intel_privacy_screen.c   | 184 ++++++++++++++++++
 .../drm/i915/display/intel_privacy_screen.h   |  27 +++
 3 files changed, 213 insertions(+), 1 deletion(-)
 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_privacy_screen.c b/drivers/gpu/drm/i915/display/intel_privacy_screen.c
new file mode 100644
index 0000000000000..66039103c821b
--- /dev/null
+++ b/drivers/gpu/drm/i915/display/intel_privacy_screen.c
@@ -0,0 +1,184 @@
+// 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 CONN_NAME(conn)						\
+	(conn->base.kdev ? dev_name(conn->base.kdev) : "NONAME")
+
+#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;
+	struct drm_device *drm = connector->base.dev;
+
+	if (!acpi_handle)
+		return;
+
+	obj = acpi_evaluate_dsm(acpi_handle, &drm_conn_dsm_guid,
+				CONNECTOR_DSM_REVID, func, NULL);
+	if (!obj) {
+		drm_err(drm, "%s: failed to evaluate _DSM for fn %llx\n",
+			CONN_NAME(connector), func);
+		return;
+	}
+
+	ACPI_FREE(obj);
+}
+
+void intel_privacy_screen_set_val(struct intel_connector *connector,
+				  enum drm_privacy_screen_status val)
+{
+	struct drm_device *drm = connector->base.dev;
+
+	if (val == PRIVACY_SCREEN_DISABLED) {
+		drm_dbg_kms(drm, "%s: disabling privacy-screen\n",
+			    CONN_NAME(connector));
+		acpi_privacy_screen_call_dsm(connector,
+					     CONNECTOR_DSM_FN_PRIVACY_DISABLE);
+	} else {
+		drm_dbg_kms(drm, "%s: enabling privacy-screen\n",
+			    CONN_NAME(connector));
+		acpi_privacy_screen_call_dsm(connector,
+					     CONNECTOR_DSM_FN_PRIVACY_ENABLE);
+	}
+}
+
+bool intel_privacy_screen_present(struct intel_connector *connector)
+{
+	acpi_handle handle = connector->acpi_handle;
+	struct drm_device *drm = connector->base.dev;
+
+	if (handle &&
+	    acpi_check_dsm(handle, &drm_conn_dsm_guid,
+			   CONNECTOR_DSM_REVID,
+			   1 << CONNECTOR_DSM_FN_PRIVACY_ENABLE |
+			   1 << CONNECTOR_DSM_FN_PRIVACY_DISABLE)) {
+		drm_info(drm, "%s: supports ACPI privacy-screen\n",
+			 CONN_NAME(connector));
+		return true;
+	}
+
+	drm_dbg_kms(drm, "%s: doesn't support ACPI privacy-screen\n",
+		    CONN_NAME(connector));
+	return false;
+}
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..8655745ff9085
--- /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 © 2020 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] [PATCH v9 5/5] drm/i915: Enable 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>
---
v9: same as v8
v8: - separate the APCI privacy screen into a separate patch.
    - Don't destroy the property if there is no privacy screen (because
      drm core doesn't like destroying property in late_register()).
    - The setting change needs to be committed in ->update_pipe() for 
      ddi.c as well as dp.c and both of them call intel_dp_add_properties()
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_atomic.c |  2 ++
 drivers/gpu/drm/i915/display/intel_ddi.c    |  1 +
 drivers/gpu/drm/i915/display/intel_dp.c     | 34 ++++++++++++++++++++-
 drivers/gpu/drm/i915/display/intel_dp.h     |  5 +++
 4 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c b/drivers/gpu/drm/i915/display/intel_atomic.c
index d043057d2fa03..9898d8980e7ce 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic.c
@@ -150,6 +150,8 @@ 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_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 73d0f4648c06a..69a5423216dc5 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -3708,6 +3708,7 @@ static void intel_ddi_update_pipe(struct intel_encoder *encoder,
 	if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
 		intel_ddi_update_pipe_dp(encoder, crtc_state, conn_state);
 
+	intel_dp_update_privacy_screen(encoder, crtc_state, conn_state);
 	intel_hdcp_update_pipe(encoder, crtc_state, conn_state);
 }
 
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 3ddc424b028c1..5f33ebb466135 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,10 @@ 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);
+
 	DRM_DEBUG_KMS("registering %s bus for %s\n",
 		      intel_dp->aux.name, connector->kdev->kobj.name);
 
@@ -6883,6 +6888,33 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
 		connector->state->scaling_mode = DRM_MODE_SCALE_ASPECT;
 
 	}
+
+	/*
+	 * Created here, but depending on result of probing for privacy-screen
+	 * in intel_dp_connector_register(), gets attached in that function.
+	 * Need to create here because the drm core doesn't like creating
+	 * properties during ->late_register().
+	 */
+	drm_connector_create_privacy_screen_property(connector);
+}
+
+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;
+
+	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)
@@ -7826,7 +7858,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_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
index 0c7be8ed1423a..e4594e27ce5a8 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.h
+++ b/drivers/gpu/drm/i915/display/intel_dp.h
@@ -123,4 +123,9 @@ static inline unsigned int intel_dp_unused_lane_mask(int lane_count)
 
 u32 intel_dp_mode_to_fec_clock(u32 mode_clock);
 
+void
+intel_dp_update_privacy_screen(struct intel_encoder *encoder,
+			       const struct intel_crtc_state *crtc_state,
+			       const struct drm_connector_state *conn_state);
+
 #endif /* __INTEL_DP_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: [Intel-gfx] [PATCH v9 5/5] drm/i915: Enable support for integrated privacy screen

[Hans de Goede]

Hi,

On 3/12/20 7:56 PM, Rajat Jain wrote:
> Add support for an ACPI based integrated privacy screen that is
> available on some systems.
> 
> Signed-off-by: Rajat Jain <rajatja@google.com>

So as discussed a while ago I'm working on adding support for the
privacy-screen on Lenovo Thinkpads, introducing a small new
subsystem / helper-class as intermediary for when the privacy-screen
is controlled by e.g. some random drivers/platform/x86 driver rather
then directly by the GPU driver.

I'm almost ready to send out v1. I was working on hooking things
up in the i915 code and I was wondering what you were doing when
the property is actually changed and we need to commit the new
privacy-screen state to the hardware.

This made me look at this patch, some comments inline:

> ---
> v9: same as v8
> v8: - separate the APCI privacy screen into a separate patch.
>      - Don't destroy the property if there is no privacy screen (because
>        drm core doesn't like destroying property in late_register()).
>      - The setting change needs to be committed in ->update_pipe() for
>        ddi.c as well as dp.c and both of them call intel_dp_add_properties()
> 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_atomic.c |  2 ++
>   drivers/gpu/drm/i915/display/intel_ddi.c    |  1 +
>   drivers/gpu/drm/i915/display/intel_dp.c     | 34 ++++++++++++++++++++-
>   drivers/gpu/drm/i915/display/intel_dp.h     |  5 +++
>   4 files changed, 41 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c b/drivers/gpu/drm/i915/display/intel_atomic.c
> index d043057d2fa03..9898d8980e7ce 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic.c
> @@ -150,6 +150,8 @@ 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;

Right I was planning on doing this to.

> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 73d0f4648c06a..69a5423216dc5 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -3708,6 +3708,7 @@ static void intel_ddi_update_pipe(struct intel_encoder *encoder,
>   	if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
>   		intel_ddi_update_pipe_dp(encoder, crtc_state, conn_state);
>   
> +	intel_dp_update_privacy_screen(encoder, crtc_state, conn_state);
>   	intel_hdcp_update_pipe(encoder, crtc_state, conn_state);
>   }
>   

And this too.

> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 3ddc424b028c1..5f33ebb466135 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,10 @@ 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);
> +
>   	DRM_DEBUG_KMS("registering %s bus for %s\n",
>   		      intel_dp->aux.name, connector->kdev->kobj.name);
>   
> @@ -6883,6 +6888,33 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
>   		connector->state->scaling_mode = DRM_MODE_SCALE_ASPECT;
>   
>   	}
> +
> +	/*
> +	 * Created here, but depending on result of probing for privacy-screen
> +	 * in intel_dp_connector_register(), gets attached in that function.
> +	 * Need to create here because the drm core doesn't like creating
> +	 * properties during ->late_register().
> +	 */
> +	drm_connector_create_privacy_screen_property(connector);
> +}
> +
> +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;
> +
> +	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)
> @@ -7826,7 +7858,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;

And this too.

One problem here is that AFAICT the update_pipe callback is only called on
fast modesets. So if the privacy_screen state is changed as part of a
full modeset, then the change will be ignored.

Even if we ignore that for now, this means that we end up calling
intel_privacy_screen_set_val(), or my equivalent of that for
each fast modeset.

In patch 4/5 intel_privacy_screen_set_val() is defined like this:

+void intel_privacy_screen_set_val(struct intel_connector *connector,
+				  enum drm_privacy_screen_status val)
+{
+	struct drm_device *drm = connector->base.dev;
+
+	if (val == PRIVACY_SCREEN_DISABLED) {
+		drm_dbg_kms(drm, "%s: disabling privacy-screen\n",
+			    CONN_NAME(connector));
+		acpi_privacy_screen_call_dsm(connector,
+					     CONNECTOR_DSM_FN_PRIVACY_DISABLE);
+	} else {
+		drm_dbg_kms(drm, "%s: enabling privacy-screen\n",
+			    CONN_NAME(connector));
+		acpi_privacy_screen_call_dsm(connector,
+					     CONNECTOR_DSM_FN_PRIVACY_ENABLE);
+	}
+}
+

There are 2 problems with this:

1. It makes the call even if there is no privacy-screen, and then
acpi_privacy_screen_call_dsm() will log an error (if the connector has an
associated handle but not the DSM).

2. It makes this call on any modeset, even if the property did non change
(and even if there is no privacy-screen) and AFAIK these ACPI calls are somewhat
expensive to make.

1. Should be easy to fix, fixing 2. is trickier. We really need access
to the new and old connector_state here to only make the ACPI calls when
necessary. But ATM all callbacks only ever get passed the new-state and
these callbacks are all called after drm_atomic_helper_swap_state() at
which point there is no way to get the old_state from the new_state.

I've chosen to instead do this to update the privacy-screen change:

--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -15501,6 +15503,9 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
  			intel_color_load_luts(new_crtc_state);
  	}

+	for_each_new_connector_in_state(&state->base, connector, new_connector_state, i)
+		drm_connector_update_privacy_screen(connector, &state->base);
+
  	/*
  	 * Now that the vblank has passed, we can go ahead and program the
  	 * optimal watermarks on platforms that need two-step watermark

With drm_connector_update_privacy_screen() looking like this:

+void drm_connector_update_privacy_screen(struct drm_connector *connector,
+					 struct drm_atomic_state *state)
+{
+	struct drm_connector_state *new_connector_state, *old_connector_state;
+	int ret;
+
+	if (!connector->privacy_screen)
+		return;
+
+	new_connector_state = drm_atomic_get_new_connector_state(state, connector);
+	old_connector_state = drm_atomic_get_old_connector_state(state, connector);
+
+	if (new_connector_state->privacy_screen_sw_state ==
+	    old_connector_state->privacy_screen_sw_state)
+		return;
+
+	ret = drm_privacy_screen_set_sw_state(connector->privacy_screen,
+				new_connector_state->privacy_screen_sw_state);
+	if (ret)
+		drm_err(connector->dev, "Error updating privacy-screen sw_state\n");
+}

Which avoids all the problems described above.

REgards,

Hans




> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
> index 0c7be8ed1423a..e4594e27ce5a8 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> @@ -123,4 +123,9 @@ static inline unsigned int intel_dp_unused_lane_mask(int lane_count)
>   
>   u32 intel_dp_mode_to_fec_clock(u32 mode_clock);
>   
> +void
> +intel_dp_update_privacy_screen(struct intel_encoder *encoder,
> +			       const struct intel_crtc_state *crtc_state,
> +			       const struct drm_connector_state *conn_state);
> +
>   #endif /* __INTEL_DP_H__ */
> 

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

Re: [Intel-gfx] [PATCH v9 5/5] drm/i915: Enable support for integrated privacy screen

[Rajat Jain]

Hello Hans,

On Mon, Jul 6, 2020 at 5:51 AM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
>
> On 3/12/20 7:56 PM, Rajat Jain wrote:
> > Add support for an ACPI based integrated privacy screen that is
> > available on some systems.
> >
> > Signed-off-by: Rajat Jain <rajatja@google.com>
>
> So as discussed a while ago I'm working on adding support for the
> privacy-screen on Lenovo Thinkpads, introducing a small new
> subsystem / helper-class as intermediary for when the privacy-screen
> is controlled by e.g. some random drivers/platform/x86 driver rather
> then directly by the GPU driver.
>
> I'm almost ready to send out v1. I was working on hooking things
> up in the i915 code and I was wondering what you were doing when
> the property is actually changed and we need to commit the new
> privacy-screen state to the hardware.
>
> This made me look at this patch, some comments inline:
>
> > ---
> > v9: same as v8
> > v8: - separate the APCI privacy screen into a separate patch.
> >      - Don't destroy the property if there is no privacy screen (because
> >        drm core doesn't like destroying property in late_register()).
> >      - The setting change needs to be committed in ->update_pipe() for
> >        ddi.c as well as dp.c and both of them call intel_dp_add_properties()
> > 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_atomic.c |  2 ++
> >   drivers/gpu/drm/i915/display/intel_ddi.c    |  1 +
> >   drivers/gpu/drm/i915/display/intel_dp.c     | 34 ++++++++++++++++++++-
> >   drivers/gpu/drm/i915/display/intel_dp.h     |  5 +++
> >   4 files changed, 41 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c b/drivers/gpu/drm/i915/display/intel_atomic.c
> > index d043057d2fa03..9898d8980e7ce 100644
> > --- a/drivers/gpu/drm/i915/display/intel_atomic.c
> > +++ b/drivers/gpu/drm/i915/display/intel_atomic.c
> > @@ -150,6 +150,8 @@ 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;
>
> Right I was planning on doing this to.
>
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index 73d0f4648c06a..69a5423216dc5 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -3708,6 +3708,7 @@ static void intel_ddi_update_pipe(struct intel_encoder *encoder,
> >       if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
> >               intel_ddi_update_pipe_dp(encoder, crtc_state, conn_state);
> >
> > +     intel_dp_update_privacy_screen(encoder, crtc_state, conn_state);
> >       intel_hdcp_update_pipe(encoder, crtc_state, conn_state);
> >   }
> >
>
> And this too.
>
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 3ddc424b028c1..5f33ebb466135 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,10 @@ 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);
> > +
> >       DRM_DEBUG_KMS("registering %s bus for %s\n",
> >                     intel_dp->aux.name, connector->kdev->kobj.name);
> >
> > @@ -6883,6 +6888,33 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
> >               connector->state->scaling_mode = DRM_MODE_SCALE_ASPECT;
> >
> >       }
> > +
> > +     /*
> > +      * Created here, but depending on result of probing for privacy-screen
> > +      * in intel_dp_connector_register(), gets attached in that function.
> > +      * Need to create here because the drm core doesn't like creating
> > +      * properties during ->late_register().
> > +      */
> > +     drm_connector_create_privacy_screen_property(connector);
> > +}
> > +
> > +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;
> > +
> > +     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)
> > @@ -7826,7 +7858,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;
>
> And this too.
>
> One problem here is that AFAICT the update_pipe callback is only called on
> fast modesets. So if the privacy_screen state is changed as part of a
> full modeset, then the change will be ignored.

I'm actually new to the drm / i915, so I did what I thought was right
at the time and was working on my setup. But, yeah, that might be a
possible issue it seems.

>
> Even if we ignore that for now, this means that we end up calling
> intel_privacy_screen_set_val(), or my equivalent of that for
> each fast modeset.
>
> In patch 4/5 intel_privacy_screen_set_val() is defined like this:
>
> +void intel_privacy_screen_set_val(struct intel_connector *connector,
> +                                 enum drm_privacy_screen_status val)
> +{
> +       struct drm_device *drm = connector->base.dev;
> +
> +       if (val == PRIVACY_SCREEN_DISABLED) {
> +               drm_dbg_kms(drm, "%s: disabling privacy-screen\n",
> +                           CONN_NAME(connector));
> +               acpi_privacy_screen_call_dsm(connector,
> +                                            CONNECTOR_DSM_FN_PRIVACY_DISABLE);
> +       } else {
> +               drm_dbg_kms(drm, "%s: enabling privacy-screen\n",
> +                           CONN_NAME(connector));
> +               acpi_privacy_screen_call_dsm(connector,
> +                                            CONNECTOR_DSM_FN_PRIVACY_ENABLE);
> +       }
> +}
> +
>
> There are 2 problems with this:
>
> 1. It makes the call even if there is no privacy-screen, and then
> acpi_privacy_screen_call_dsm() will log an error (if the connector has an
> associated handle but not the DSM).
>
> 2. It makes this call on any modeset, even if the property did non change
> (and even if there is no privacy-screen) and AFAIK these ACPI calls are somewhat
> expensive to make.

Ack to both these problems.

>
> 1. Should be easy to fix, fixing 2. is trickier. We really need access
> to the new and old connector_state here to only make the ACPI calls when
> necessary. But ATM all callbacks only ever get passed the new-state and
> these callbacks are all called after drm_atomic_helper_swap_state() at
> which point there is no way to get the old_state from the new_state.
>
> I've chosen to instead do this to update the privacy-screen change:
>
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -15501,6 +15503,9 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
>                         intel_color_load_luts(new_crtc_state);
>         }
>
> +       for_each_new_connector_in_state(&state->base, connector, new_connector_state, i)
> +               drm_connector_update_privacy_screen(connector, &state->base);
> +
>         /*
>          * Now that the vblank has passed, we can go ahead and program the
>          * optimal watermarks on platforms that need two-step watermark
>
> With drm_connector_update_privacy_screen() looking like this:
>
> +void drm_connector_update_privacy_screen(struct drm_connector *connector,
> +                                        struct drm_atomic_state *state)
> +{
> +       struct drm_connector_state *new_connector_state, *old_connector_state;
> +       int ret;
> +
> +       if (!connector->privacy_screen)
> +               return;
> +
> +       new_connector_state = drm_atomic_get_new_connector_state(state, connector);
> +       old_connector_state = drm_atomic_get_old_connector_state(state, connector);
> +
> +       if (new_connector_state->privacy_screen_sw_state ==
> +           old_connector_state->privacy_screen_sw_state)
> +               return;
> +
> +       ret = drm_privacy_screen_set_sw_state(connector->privacy_screen,
> +                               new_connector_state->privacy_screen_sw_state);
> +       if (ret)
> +               drm_err(connector->dev, "Error updating privacy-screen sw_state\n");
> +}
>
> Which avoids all the problems described above.

Ack. This looks like a better way since it takes care of these
problems. Please feel free to use my patches as you see fit (I didn't
see much activity on them since last many months so I have moved on to
something else now).

Thanks & Best Regards,

Rajat

>
> REgards,
>
> Hans
>
>
>
>
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
> > index 0c7be8ed1423a..e4594e27ce5a8 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.h
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> > @@ -123,4 +123,9 @@ static inline unsigned int intel_dp_unused_lane_mask(int lane_count)
> >
> >   u32 intel_dp_mode_to_fec_clock(u32 mode_clock);
> >
> > +void
> > +intel_dp_update_privacy_screen(struct intel_encoder *encoder,
> > +                            const struct intel_crtc_state *crtc_state,
> > +                            const struct drm_connector_state *conn_state);
> > +
> >   #endif /* __INTEL_DP_H__ */
> >
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v9 5/5] drm/i915: Enable support for integrated privacy screen

[Rajat Jain]

Hello Hans,

On Mon, Jul 6, 2020 at 5:50 PM Rajat Jain <rajatja@google.com> wrote:
>
> Hello Hans,
>
> On Mon, Jul 6, 2020 at 5:51 AM Hans de Goede <hdegoede@redhat.com> wrote:
> >
> > Hi,
> >
> > On 3/12/20 7:56 PM, Rajat Jain wrote:
> > > Add support for an ACPI based integrated privacy screen that is
> > > available on some systems.
> > >
> > > Signed-off-by: Rajat Jain <rajatja@google.com>
> >
> > So as discussed a while ago I'm working on adding support for the
> > privacy-screen on Lenovo Thinkpads, introducing a small new
> > subsystem / helper-class as intermediary for when the privacy-screen
> > is controlled by e.g. some random drivers/platform/x86 driver rather
> > then directly by the GPU driver.
> >
> > I'm almost ready to send out v1. I was working on hooking things
> > up in the i915 code and I was wondering what you were doing when
> > the property is actually changed and we need to commit the new
> > privacy-screen state to the hardware.
> >
> > This made me look at this patch, some comments inline:
> >
> > > ---
> > > v9: same as v8
> > > v8: - separate the APCI privacy screen into a separate patch.
> > >      - Don't destroy the property if there is no privacy screen (because
> > >        drm core doesn't like destroying property in late_register()).
> > >      - The setting change needs to be committed in ->update_pipe() for
> > >        ddi.c as well as dp.c and both of them call intel_dp_add_properties()
> > > 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_atomic.c |  2 ++
> > >   drivers/gpu/drm/i915/display/intel_ddi.c    |  1 +
> > >   drivers/gpu/drm/i915/display/intel_dp.c     | 34 ++++++++++++++++++++-
> > >   drivers/gpu/drm/i915/display/intel_dp.h     |  5 +++
> > >   4 files changed, 41 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c b/drivers/gpu/drm/i915/display/intel_atomic.c
> > > index d043057d2fa03..9898d8980e7ce 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_atomic.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_atomic.c
> > > @@ -150,6 +150,8 @@ 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;
> >
> > Right I was planning on doing this to.
> >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > index 73d0f4648c06a..69a5423216dc5 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > @@ -3708,6 +3708,7 @@ static void intel_ddi_update_pipe(struct intel_encoder *encoder,
> > >       if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
> > >               intel_ddi_update_pipe_dp(encoder, crtc_state, conn_state);
> > >
> > > +     intel_dp_update_privacy_screen(encoder, crtc_state, conn_state);
> > >       intel_hdcp_update_pipe(encoder, crtc_state, conn_state);
> > >   }
> > >
> >
> > And this too.
> >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > > index 3ddc424b028c1..5f33ebb466135 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,10 @@ 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);
> > > +
> > >       DRM_DEBUG_KMS("registering %s bus for %s\n",
> > >                     intel_dp->aux.name, connector->kdev->kobj.name);
> > >
> > > @@ -6883,6 +6888,33 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
> > >               connector->state->scaling_mode = DRM_MODE_SCALE_ASPECT;
> > >
> > >       }
> > > +
> > > +     /*
> > > +      * Created here, but depending on result of probing for privacy-screen
> > > +      * in intel_dp_connector_register(), gets attached in that function.
> > > +      * Need to create here because the drm core doesn't like creating
> > > +      * properties during ->late_register().
> > > +      */
> > > +     drm_connector_create_privacy_screen_property(connector);
> > > +}
> > > +
> > > +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;
> > > +
> > > +     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)
> > > @@ -7826,7 +7858,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;
> >
> > And this too.
> >
> > One problem here is that AFAICT the update_pipe callback is only called on
> > fast modesets. So if the privacy_screen state is changed as part of a
> > full modeset, then the change will be ignored.
>
> I'm actually new to the drm / i915, so I did what I thought was right
> at the time and was working on my setup. But, yeah, that might be a
> possible issue it seems.
>
> >
> > Even if we ignore that for now, this means that we end up calling
> > intel_privacy_screen_set_val(), or my equivalent of that for
> > each fast modeset.
> >
> > In patch 4/5 intel_privacy_screen_set_val() is defined like this:
> >
> > +void intel_privacy_screen_set_val(struct intel_connector *connector,
> > +                                 enum drm_privacy_screen_status val)
> > +{
> > +       struct drm_device *drm = connector->base.dev;
> > +
> > +       if (val == PRIVACY_SCREEN_DISABLED) {
> > +               drm_dbg_kms(drm, "%s: disabling privacy-screen\n",
> > +                           CONN_NAME(connector));
> > +               acpi_privacy_screen_call_dsm(connector,
> > +                                            CONNECTOR_DSM_FN_PRIVACY_DISABLE);
> > +       } else {
> > +               drm_dbg_kms(drm, "%s: enabling privacy-screen\n",
> > +                           CONN_NAME(connector));
> > +               acpi_privacy_screen_call_dsm(connector,
> > +                                            CONNECTOR_DSM_FN_PRIVACY_ENABLE);
> > +       }
> > +}
> > +
> >
> > There are 2 problems with this:
> >
> > 1. It makes the call even if there is no privacy-screen, and then
> > acpi_privacy_screen_call_dsm() will log an error (if the connector has an
> > associated handle but not the DSM).
> >
> > 2. It makes this call on any modeset, even if the property did non change
> > (and even if there is no privacy-screen) and AFAIK these ACPI calls are somewhat
> > expensive to make.
>
> Ack to both these problems.
>
> >
> > 1. Should be easy to fix, fixing 2. is trickier. We really need access
> > to the new and old connector_state here to only make the ACPI calls when
> > necessary. But ATM all callbacks only ever get passed the new-state and
> > these callbacks are all called after drm_atomic_helper_swap_state() at
> > which point there is no way to get the old_state from the new_state.
> >
> > I've chosen to instead do this to update the privacy-screen change:
> >
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -15501,6 +15503,9 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
> >                         intel_color_load_luts(new_crtc_state);
> >         }
> >
> > +       for_each_new_connector_in_state(&state->base, connector, new_connector_state, i)
> > +               drm_connector_update_privacy_screen(connector, &state->base);
> > +
> >         /*
> >          * Now that the vblank has passed, we can go ahead and program the
> >          * optimal watermarks on platforms that need two-step watermark
> >
> > With drm_connector_update_privacy_screen() looking like this:
> >
> > +void drm_connector_update_privacy_screen(struct drm_connector *connector,
> > +                                        struct drm_atomic_state *state)
> > +{
> > +       struct drm_connector_state *new_connector_state, *old_connector_state;
> > +       int ret;
> > +
> > +       if (!connector->privacy_screen)
> > +               return;
> > +
> > +       new_connector_state = drm_atomic_get_new_connector_state(state, connector);
> > +       old_connector_state = drm_atomic_get_old_connector_state(state, connector);
> > +
> > +       if (new_connector_state->privacy_screen_sw_state ==
> > +           old_connector_state->privacy_screen_sw_state)
> > +               return;
> > +
> > +       ret = drm_privacy_screen_set_sw_state(connector->privacy_screen,
> > +                               new_connector_state->privacy_screen_sw_state);
> > +       if (ret)
> > +               drm_err(connector->dev, "Error updating privacy-screen sw_state\n");
> > +}
> >
> > Which avoids all the problems described above.
>
> Ack. This looks like a better way since it takes care of these
> problems. Please feel free to use my patches as you see fit (I didn't
> see much activity on them since last many months so I have moved on to
> something else now).

I'm curious to know what was the fate of these patches. I know you
were working on a version of it. Did the privacy-screen feature
actually find some traction upstream and was accepted in some form?

Thanks,

Rajat


>
> Thanks & Best Regards,
>
> Rajat
>
> >
> > REgards,
> >
> > Hans
> >
> >
> >
> >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
> > > index 0c7be8ed1423a..e4594e27ce5a8 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dp.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> > > @@ -123,4 +123,9 @@ static inline unsigned int intel_dp_unused_lane_mask(int lane_count)
> > >
> > >   u32 intel_dp_mode_to_fec_clock(u32 mode_clock);
> > >
> > > +void
> > > +intel_dp_update_privacy_screen(struct intel_encoder *encoder,
> > > +                            const struct intel_crtc_state *crtc_state,
> > > +                            const struct drm_connector_state *conn_state);
> > > +
> > >   #endif /* __INTEL_DP_H__ */
> > >
> >
_______________________________________________
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/i915 Support for integrated privacy screen

[Patchwork]

== Series Details ==

Series: drm/i915 Support for integrated privacy screen
URL   : https://patchwork.freedesktop.org/series/74650/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
a0d97f65392d intel_acpi: Rename drm_dev local variable to dev
500f89acca9c drm/connector: Add support for privacy-screen property
50fff7891e7a drm/i915: Lookup and attach ACPI device node for connectors
d25c3af2f2e2 drm/i915: Add helper code for ACPI privacy screen
-:27: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#27: 
new file mode 100644

-:145: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'conn' - possible side-effects?
#145: FILE: drivers/gpu/drm/i915/display/intel_privacy_screen.c:114:
+#define CONN_NAME(conn)						\
+	(conn->base.kdev ? dev_name(conn->base.kdev) : "NONAME")

-:145: CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'conn' may be better as '(conn)' to avoid precedence issues
#145: FILE: drivers/gpu/drm/i915/display/intel_privacy_screen.c:114:
+#define CONN_NAME(conn)						\
+	(conn->base.kdev ? dev_name(conn->base.kdev) : "NONAME")

total: 0 errors, 1 warnings, 2 checks, 220 lines checked
6d59d9dc39bf drm/i915: Enable support for integrated privacy screen

_______________________________________________
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/i915 Support for integrated privacy screen

[Patchwork]

== Series Details ==

Series: drm/i915 Support for integrated privacy screen
URL   : https://patchwork.freedesktop.org/series/74650/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_8127 -> Patchwork_16957
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-tgl-y:           [PASS][1] -> [FAIL][2] ([CI#94])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8127/fi-tgl-y/igt@gem_exec_suspend@basic-s4-devices.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16957/fi-tgl-y/igt@gem_exec_suspend@basic-s4-devices.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@gem_contexts:
    - fi-cml-s:           [DMESG-FAIL][3] ([i915#877]) -> [PASS][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8127/fi-cml-s/igt@i915_selftest@live@gem_contexts.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16957/fi-cml-s/igt@i915_selftest@live@gem_contexts.html

  * igt@i915_selftest@live@hangcheck:
    - fi-ivb-3770:        [INCOMPLETE][5] ([i915#1405]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8127/fi-ivb-3770/igt@i915_selftest@live@hangcheck.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16957/fi-ivb-3770/igt@i915_selftest@live@hangcheck.html

  * igt@i915_selftest@live@mman:
    - fi-ivb-3770:        [DMESG-WARN][7] ([i915#1405]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8127/fi-ivb-3770/igt@i915_selftest@live@mman.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16957/fi-ivb-3770/igt@i915_selftest@live@mman.html

  
  [CI#94]: https://gitlab.freedesktop.org/gfx-ci/i915-infra/issues/94
  [i915#1405]: https://gitlab.freedesktop.org/drm/intel/issues/1405
  [i915#877]: https://gitlab.freedesktop.org/drm/intel/issues/877


Participating hosts (42 -> 37)
------------------------------

  Additional (7): fi-bsw-n3050 fi-cfl-guc fi-snb-2520m fi-kbl-x1275 fi-bsw-kefka fi-kbl-7560u fi-kbl-r 
  Missing    (12): fi-ilk-m540 fi-bdw-samus fi-tgl-dsi fi-hsw-4200u fi-skl-6770hq fi-bdw-gvtdvm fi-bsw-cyan fi-skl-lmem fi-blb-e6850 fi-byt-clapper fi-skl-6600u fi-snb-2600 


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_8127 -> Patchwork_16957

  CI-20190529: 20190529
  CI_DRM_8127: 6b843f994832ac95eafa8d380399c3aef2cab3e5 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5506: 59fd8a0d01dac58dc6c7d86ef391ed4393ab5aae @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_16957: 6d59d9dc39bf79232460b974d3909abcf8682925 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

6d59d9dc39bf drm/i915: Enable support for integrated privacy screen
d25c3af2f2e2 drm/i915: Add helper code for ACPI privacy screen
50fff7891e7a drm/i915: Lookup and attach ACPI device node for connectors
500f89acca9c drm/connector: Add support for privacy-screen property
a0d97f65392d intel_acpi: Rename drm_dev local variable to dev

== Logs ==

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

[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915 Support for integrated privacy screen

[Patchwork]

== Series Details ==

Series: drm/i915 Support for integrated privacy screen
URL   : https://patchwork.freedesktop.org/series/74650/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_8127_full -> Patchwork_16957_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_persistence@close-replace-race:
    - shard-tglb:         [PASS][1] -> [INCOMPLETE][2] ([i915#1402])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8127/shard-tglb6/igt@gem_ctx_persistence@close-replace-race.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16957/shard-tglb1/igt@gem_ctx_persistence@close-replace-race.html

  * igt@gem_ctx_persistence@engines-mixed-process@vecs0:
    - shard-apl:          [PASS][3] -> [FAIL][4] ([i915#679])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8127/shard-apl2/igt@gem_ctx_persistence@engines-mixed-process@vecs0.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16957/shard-apl2/igt@gem_ctx_persistence@engines-mixed-process@vecs0.html

  * igt@gem_exec_balancer@smoke:
    - shard-iclb:         [PASS][5] -> [SKIP][6] ([fdo#110854])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8127/shard-iclb1/igt@gem_exec_balancer@smoke.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16957/shard-iclb8/igt@gem_exec_balancer@smoke.html

  * igt@gem_exec_schedule@implicit-read-write-bsd1:
    - shard-iclb:         [PASS][7] -> [SKIP][8] ([fdo#109276] / [i915#677])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8127/shard-iclb2/igt@gem_exec_schedule@implicit-read-write-bsd1.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16957/shard-iclb7/igt@gem_exec_schedule@implicit-read-write-bsd1.html

  * igt@gem_exec_schedule@pi-shared-iova-bsd:
    - shard-iclb:         [PASS][9] -> [SKIP][10] ([i915#677])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8127/shard-iclb7/igt@gem_exec_schedule@pi-shared-iova-bsd.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16957/shard-iclb4/igt@gem_exec_schedule@pi-shared-iova-bsd.html

  * igt@gem_exec_schedule@wide-bsd:
    - shard-iclb:         [PASS][11] -> [SKIP][12] ([fdo#112146]) +4 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8127/shard-iclb7/igt@gem_exec_schedule@wide-bsd.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16957/shard-iclb4/igt@gem_exec_schedule@wide-bsd.html

  * igt@gem_ppgtt@flink-and-close-vma-leak:
    - shard-glk:          [PASS][13] -> [FAIL][14] ([i915#644])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8127/shard-glk6/igt@gem_ppgtt@flink-and-close-vma-leak.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16957/shard-glk5/igt@gem_ppgtt@flink-and-close-vma-leak.html

  * igt@i915_hangman@error-state-capture-vcs1:
    - shard-iclb:         [PASS][15] -> [SKIP][16] ([fdo#112080]) +8 similar issues
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8127/shard-iclb4/igt@i915_hangman@error-state-capture-vcs1.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16957/shard-iclb7/igt@i915_hangman@error-state-capture-vcs1.html

  * igt@kms_cursor_crc@pipe-a-cursor-64x21-random:
    - shard-kbl:          [PASS][17] -> [FAIL][18] ([i915#54])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8127/shard-kbl6/igt@kms_cursor_crc@pipe-a-cursor-64x21-random.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16957/shard-kbl2/igt@kms_cursor_crc@pipe-a-cursor-64x21-random.html
    - shard-skl:          [PASS][19] -> [FAIL][20] ([i915#54])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8127/shard-skl6/igt@kms_cursor_crc@pipe-a-cursor-64x21-random.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16957/shard-skl4/igt@kms_cursor_crc@pipe-a-cursor-64x21-random.html
    - shard-apl:          [PASS][21] -> [FAIL][22] ([i915#54])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8127/shard-apl4/igt@kms_cursor_crc@pipe-a-cursor-64x21-random.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16957/shard-apl8/igt@kms_cursor_crc@pipe-a-cursor-64x21-random.html

  * igt@kms_cursor_legacy@basic-flip-after-cursor-legacy:
    - shard-hsw:          [PASS][23] -> [INCOMPLETE][24] ([i915#61])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8127/shard-hsw1/igt@kms_cursor_legacy@basic-flip-after-cursor-legacy.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16957/shard-hsw5/igt@kms_cursor_legacy@basic-flip-after-cursor-legacy.html

  * igt@kms_draw_crc@fill-fb:
    - shard-glk:          [PASS][25] -> [FAIL][26] ([i915#52])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8127/shard-glk3/igt@kms_draw_crc@fill-fb.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16957/shard-glk6/igt@kms_draw_crc@fill-fb.html

  * igt@kms_flip@flip-vs-suspend-interruptible:
    - shard-skl:          [PASS][27] -> [INCOMPLETE][28] ([i915#221])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8127/shard-skl2/igt@kms_flip@flip-vs-suspend-interruptible.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16957/shard-skl4/igt@kms_flip@flip-vs-suspend-interruptible.html

  * igt@kms_frontbuffer_tracking@fbc-suspend:
    - shard-apl:          [PASS][29] -> [DMESG-WARN][30] ([i915#180]) +3 similar issues
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8127/shard-apl1/igt@kms_frontbuffer_tracking@fbc-suspend.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16957/shard-apl4/igt@kms_frontbuffer_tracking@fbc-suspend.html

  * igt@kms_hdr@bpc-switch-suspend:
    - shard-skl:          [PASS][31] -> [FAIL][32] ([i915#1188])
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8127/shard-skl8/igt@kms_hdr@bpc-switch-suspend.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16957/shard-skl1/igt@kms_hdr@bpc-switch-suspend.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes:
    - shard-skl:          [PASS][33] -> [INCOMPLETE][34] ([i915#648] / [i915#69])
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8127/shard-skl3/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16957/shard-skl10/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html

  * igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min:
    - shard-skl:          [PASS][35] -> [FAIL][36] ([fdo#108145])
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8127/shard-skl1/igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16957/shard-skl8/igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min.html

  * igt@kms_plane_lowres@pipe-a-tiling-x:
    - shard-glk:          [PASS][37] -> [FAIL][38] ([i915#899])
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8127/shard-glk3/igt@kms_plane_lowres@pipe-a-tiling-x.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16957/shard-glk6/igt@kms_plane_lowres@pipe-a-tiling-x.html

  * igt@kms_psr@psr2_primary_mmap_cpu:
    - shard-iclb:         [PASS][39] -> [SKIP][40] ([fdo#109441]) +2 similar issues
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8127/shard-iclb2/igt@kms_psr@psr2_primary_mmap_cpu.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16957/shard-iclb7/igt@kms_psr@psr2_primary_mmap_cpu.html

  * igt@kms_setmode@basic:
    - shard-apl:          [PASS][41] -> [FAIL][42] ([i915#31])
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8127/shard-apl1/igt@kms_setmode@basic.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16957/shard-apl4/igt@kms_setmode@basic.html

  * igt@kms_vblank@pipe-a-ts-continuation-suspend:
    - shard-kbl:          [PASS][43] -> [DMESG-WARN][44] ([i915#180]) +5 similar issues
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8127/shard-kbl6/igt@kms_vblank@pipe-a-ts-continuation-suspend.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16957/shard-kbl4/igt@kms_vblank@pipe-a-ts-continuation-suspend.html

  * igt@prime_busy@hang-bsd2:
    - shard-iclb:         [PASS][45] -> [SKIP][46] ([fdo#109276]) +27 similar issues
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8127/shard-iclb1/igt@prime_busy@hang-bsd2.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16957/shard-iclb8/igt@prime_busy@hang-bsd2.html

  
#### Possible fixes ####

  * igt@gem_exec_schedule@fifo-bsd1:
    - shard-iclb:         [SKIP][47] ([fdo#109276]) -> [PASS][48] +14 similar issues
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8127/shard-iclb7/igt@gem_exec_schedule@fifo-bsd1.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16957/shard-iclb4/igt@gem_exec_schedule@fifo-bsd1.html

  * igt@gem_exec_schedule@implicit-both-bsd1:
    - shard-iclb:         [SKIP][49] ([fdo#109276] / [i915#677]) -> [PASS][50] +1 similar issue
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8127/shard-iclb6/igt@gem_exec_schedule@implicit-both-bsd1.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16957/shard-iclb1/igt@gem_exec_schedule@implicit-both-bsd1.html

  * igt@gem_exec_schedule@in-order-bsd:
    - shard-iclb:         [SKIP][51] ([fdo#112146]) -> [PASS][52] +2 similar issues
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8127/shard-iclb4/igt@gem_exec_schedule@in-order-bsd.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16957/shard-iclb5/igt@gem_exec_schedule@in-order-bsd.html

  * igt@gem_exec_schedule@pi-userfault-bsd:
    - shard-iclb:         [SKIP][53] ([i915#677]) -> [PASS][54] +1 similar issue
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8127/shard-iclb4/igt@gem_exec_schedule@pi-userfault-bsd.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16957/shard-iclb5/igt@gem_exec_schedule@pi-userfault-bsd.html

  * igt@gem_exec_whisper@basic-fds-forked:
    - shard-tglb:         [TIMEOUT][55] ([i915#1408]) -> [PASS][56]
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8127/shard-tglb6/igt@gem_exec_whisper@basic-fds-forked.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16957/shard-tglb2/igt@gem_exec_whisper@basic-fds-forked.html

  * igt@gem_workarounds@suspend-resume:
    - shard-skl:          [INCOMPLETE][57] ([i915#69]) -> [PASS][58]
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8127/shard-skl6/igt@gem_workarounds@suspend-resume.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16957/shard-skl4/igt@gem_workarounds@suspend-resume.html

  * igt@gem_workarounds@suspend-resume-fd:
    - shard-kbl:          [DMESG-WARN][59] ([i915#180]) -> [PASS][60] +5 similar issues
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8127/shard-kbl4/igt@gem_workarounds@suspend-resume-fd.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16957/shard-kbl1/igt@gem_workarounds@suspend-resume-fd.html

  * igt@i915_pm_dc@dc6-psr:
    - shard-iclb:         [FAIL][61] ([i915#454]) -> [PASS][62]
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8127/shard-iclb6/igt@i915_pm_dc@dc6-psr.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16957/shard-iclb2/igt@i915_pm_dc@dc6-psr.html

  * igt@i915_pm_rps@reset:
    - shard-tglb:         [FAIL][63] ([i915#413]) -> [PASS][64]
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8127/shard-tglb6/igt@i915_pm_rps@reset.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16957/shard-tglb6/igt@i915_pm_rps@reset.html

  * igt@i915_pm_rps@waitboost:
    - shard-iclb:         [FAIL][65] ([i915#413]) -> [PASS][66]
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8127/shard-iclb5/igt@i915_pm_rps@waitboost.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16957/shard-iclb5/igt@i915_pm_rps@waitboost.html

  * igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy:
    - shard-glk:          [FAIL][67] ([i915#72]) -> [PASS][68]
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8127/shard-glk7/igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16957/shard-glk9/igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size:
    - shard-skl:          [FAIL][69] ([IGT#5] / [i915#697]) -> [PASS][70]
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8127/shard-skl6/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16957/shard-skl10/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html

  * igt@kms_draw_crc@draw-method-xrgb8888-mmap-cpu-xtiled:
    - shard-skl:          [FAIL][71] ([i915#52] / [i915#54]) -> [PASS][72]
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8127/shard-skl3/igt@kms_draw_crc@draw-method-xrgb8888-mmap-cpu-xtiled.html
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16957/shard-skl10/igt@kms_draw_crc@draw-method-xrgb8888-mmap-cpu-xtiled.html

  * igt@kms_frontbuffer_tracking@psr-suspend:
    - shard-skl:          [INCOMPLETE][73] ([i915#123] / [i915#69]) -> [PASS][74]
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8127/shard-skl10/igt@kms_frontbuffer_tracking@psr-suspend.html
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16957/shard-skl3/igt@kms_frontbuffer_tracking@psr-suspend.html

  * igt@kms_hdr@bpc-switch-dpms:
    - shard-skl:          [FAIL][75] ([i915#1188]) -> [PASS][76]
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8127/shard-skl9/igt@kms_hdr@bpc-switch-dpms.html
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16957/shard-skl8/igt@kms_hdr@bpc-switch-dpms.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
    - shard-apl:          [DMESG-WARN][77] ([i915#180]) -> [PASS][78] +5 similar issues
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8127/shard-apl1/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16957/shard-apl3/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min:
    - shard-skl:          [FAIL][79] ([fdo#108145]) -> [PASS][80] +1 similar issue
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8127/shard-skl3/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16957/shard-skl10/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          [FAIL][81] ([fdo#108145] / [i915#265]) -> [PASS][82]
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8127/shard-skl4/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16957/shard-skl7/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html

  * igt@kms_psr@psr2_sprite_plane_move:
    - shard-iclb:         [SKIP][83] ([fdo#109441]) -> [PASS][84] +2 similar issues
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8127/shard-iclb3/igt@kms_psr@psr2_sprite_plane_move.html
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16957/shard-iclb2/igt@kms_psr@psr2_sprite_plane_move.html

  * igt@perf_pmu@init-busy-vcs1:
    - shard-iclb:         [SKIP][85] ([fdo#112080]) -> [PASS][86] +8 similar issues
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8127/shard-iclb3/igt@perf_pmu@init-busy-vcs1.html
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16957/shard-iclb2/igt@perf_pmu@init-busy-vcs1.html

  
  [IGT#5]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/5
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#110854]: https://bugs.freedesktop.org/show_bug.cgi?id=110854
  [fdo#112080]: https://bugs.freedesktop.org/show_bug.cgi?id=112080
  [fdo#112146]: https://bugs.freedesktop.org/show_bug.cgi?id=112146
  [i915#1188]: https://gitlab.freedesktop.org/drm/intel/issues/1188
  [i915#123]: https://gitlab.freedesktop.org/drm/intel/issues/123
  [i915#1402]: https://gitlab.freedesktop.org/drm/intel/issues/1402
  [i915#1408]: https://gitlab.freedesktop.org/drm/intel/issues/1408
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#221]: https://gitlab.freedesktop.org/drm/intel/issues/221
  [i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265
  [i915#31]: https://gitlab.freedesktop.org/drm/intel/issues/31
  [i915#413]: https://gitlab.freedesktop.org/drm/intel/issues/413
  [i915#454]: https://gitlab.freedesktop.org/drm/intel/issues/454
  [i915#52]: https://gitlab.freedesktop.org/drm/intel/issues/52
  [i915#54]: https://gitlab.freedesktop.org/drm/intel/issues/54
  [i915#61]: https://gitlab.freedesktop.org/drm/intel/issues/61
  [i915#644]: https://gitlab.freedesktop.org/drm/intel/issues/644
  [i915#648]: https://gitlab.freedesktop.org/drm/intel/issues/648
  [i915#677]: https://gitlab.freedesktop.org/drm/intel/issues/677
  [i915#679]: https://gitlab.freedesktop.org/drm/intel/issues/679
  [i915#69]: https://gitlab.freedesktop.org/drm/intel/issues/69
  [i915#697]: https://gitlab.freedesktop.org/drm/intel/issues/697
  [i915#72]: https://gitlab.freedesktop.org/drm/intel/issues/72
  [i915#899]: https://gitlab.freedesktop.org/drm/intel/issues/899


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

  No changes in participating hosts


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_8127 -> Patchwork_16957

  CI-20190529: 20190529
  CI_DRM_8127: 6b843f994832ac95eafa8d380399c3aef2cab3e5 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5506: 59fd8a0d01dac58dc6c7d86ef391ed4393ab5aae @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_16957: 6d59d9dc39bf79232460b974d3909abcf8682925 @ 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_16957/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v9 0/5] drm/i915 Support for integrated privacy screen

[Rajat Jain]

Hi Jani,

On Thu, Mar 12, 2020 at 11:56 AM Rajat Jain <rajatja@google.com> wrote:
>
> This patchset adds support for integrated privacy screen on some laptops
> using the ACPI methods to detect and control the feature.
>
> Rajat Jain (5):
>   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 helper code for ACPI privacy screen
>   drm/i915: Enable support for integrated privacy screen

Just checking to see if you got a chance to look at this latest
patchset. This takes care of all your review comments.

Thanks & Best Regards,

Rajat

>
>  drivers/gpu/drm/drm_atomic_uapi.c             |   4 +
>  drivers/gpu/drm/drm_connector.c               |  51 +++++
>  drivers/gpu/drm/i915/Makefile                 |   3 +-
>  drivers/gpu/drm/i915/display/intel_acpi.c     |  30 ++-
>  drivers/gpu/drm/i915/display/intel_atomic.c   |   2 +
>  drivers/gpu/drm/i915/display/intel_ddi.c      |   1 +
>  .../drm/i915/display/intel_display_types.h    |   5 +
>  drivers/gpu/drm/i915/display/intel_dp.c       |  48 ++++-
>  drivers/gpu/drm/i915/display/intel_dp.h       |   5 +
>  .../drm/i915/display/intel_privacy_screen.c   | 184 ++++++++++++++++++
>  .../drm/i915/display/intel_privacy_screen.h   |  27 +++
>  drivers/gpu/drm/i915/i915_drv.h               |   2 +
>  include/drm/drm_connector.h                   |  24 +++
>  13 files changed, 382 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

[Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/i915 Support for integrated privacy screen (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915 Support for integrated privacy screen (rev2)
URL   : https://patchwork.freedesktop.org/series/74650/
State : failure

== Summary ==

Applying: intel_acpi: Rename drm_dev local variable to dev
Applying: drm/connector: Add support for privacy-screen property
Applying: drm/i915: Lookup and attach ACPI device node for connectors
Using index info to reconstruct a base tree...
M	drivers/gpu/drm/i915/display/intel_display_types.h
M	drivers/gpu/drm/i915/display/intel_dp.c
M	drivers/gpu/drm/i915/i915_drv.h
Falling back to patching base and 3-way merge...
Auto-merging drivers/gpu/drm/i915/i915_drv.h
Auto-merging drivers/gpu/drm/i915/display/intel_dp.c
CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/display/intel_dp.c
Auto-merging drivers/gpu/drm/i915/display/intel_display_types.h
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0003 drm/i915: Lookup and attach ACPI device node for connectors
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

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