[PATCH v9 0/6] HDCP2.2 Phase II

[PATCH v9 0/6] HDCP2.2 Phase II

[Ramalingam C]

Adding the uAPI support for the HDCP content type is the main focus
here. Along with that uevent is implemented for the
"Content Protection" state change that got triggered by kernel.

v9:
  KDoc improvements [pekka]

Ramalingam C (6):
  drm: Add Content protection type property
  drm/i915: Attach content type property
  drm: uevent for connector status change
  drm/hdcp: update content protection property with uevent
  drm/i915: update the hdcp state with uevent
  drm/hdcp: reference for srm file format

 drivers/gpu/drm/drm_atomic_uapi.c         |  4 ++
 drivers/gpu/drm/drm_connector.c           | 56 +++++++++++++++--
 drivers/gpu/drm/drm_hdcp.c                | 77 ++++++++++++++++++++++-
 drivers/gpu/drm/drm_sysfs.c               | 35 +++++++++++
 drivers/gpu/drm/i915/display/intel_ddi.c  | 39 ++++++++++--
 drivers/gpu/drm/i915/display/intel_hdcp.c | 53 ++++++++++------
 drivers/gpu/drm/i915/display/intel_hdcp.h |  2 +-
 include/drm/drm_connector.h               |  7 +++
 include/drm/drm_hdcp.h                    |  4 +-
 include/drm/drm_mode_config.h             |  6 ++
 include/drm/drm_sysfs.h                   |  5 +-
 include/uapi/drm/drm_mode.h               |  4 ++
 12 files changed, 258 insertions(+), 34 deletions(-)

-- 
2.19.1

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

[PATCH v9 1/6] drm: Add Content protection type property

[Ramalingam C]

This patch adds a DRM ENUM property to the selected connectors.
This property is used for mentioning the protected content's type
from userspace to kernel HDCP authentication.

Type of the stream is decided by the protected content providers.
Type 0 content can be rendered on any HDCP protected display wires.
But Type 1 content can be rendered only on HDCP2.2 protected paths.

So when a userspace sets this property to Type 1 and starts the HDCP
enable, kernel will honour it only if HDCP2.2 authentication is through
for type 1. Else HDCP enable will be failed.

Need ACK for this new conenctor property from userspace consumer.

v2:
  cp_content_type is replaced with content_protection_type [daniel]
  check at atomic_set_property is removed [Maarten]
v3:
  %s/content_protection_type/hdcp_content_type [Pekka]
v4:
  property is created for the first requested connector and then reused.
	[Danvet]
v5:
  kernel doc nits addressed [Daniel]
  Rebased as part of patch reordering.
v6:
  Kernel docs are modified [pekka]
v7:
  More details in Kernel docs. [pekka]

Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_atomic_uapi.c         |  4 +++
 drivers/gpu/drm/drm_connector.c           | 39 +++++++++++++++++++++++
 drivers/gpu/drm/drm_hdcp.c                | 36 ++++++++++++++++++++-
 drivers/gpu/drm/i915/display/intel_hdcp.c |  4 ++-
 include/drm/drm_connector.h               |  7 ++++
 include/drm/drm_hdcp.h                    |  2 +-
 include/drm/drm_mode_config.h             |  6 ++++
 include/uapi/drm/drm_mode.h               |  4 +++
 8 files changed, 99 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index abe38bdf85ae..19ae119f1a5d 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -747,6 +747,8 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
 			return -EINVAL;
 		}
 		state->content_protection = val;
+	} else if (property == config->hdcp_content_type_property) {
+		state->hdcp_content_type = val;
 	} else if (property == connector->colorspace_property) {
 		state->colorspace = val;
 	} else if (property == config->writeback_fb_id_property) {
@@ -831,6 +833,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
 			state->hdr_output_metadata->base.id : 0;
 	} else if (property == config->content_protection_property) {
 		*val = state->content_protection;
+	} else if (property == config->hdcp_content_type_property) {
+		*val = state->hdcp_content_type;
 	} else if (property == config->writeback_fb_id_property) {
 		/* Writeback framebuffer is one-shot, write and forget */
 		*val = 0;
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 068d4b05f1be..732f6645643d 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -952,6 +952,45 @@ static const struct drm_prop_enum_list hdmi_colorspaces[] = {
  *	  is no longer protected and userspace should take appropriate action
  *	  (whatever that might be).
  *
+ * HDCP Content Type:
+ *	This Enum property is used by the userspace to declare the content type
+ *	of the display stream, to kernel. Here display stream stands for any
+ *	display content that userspace intended to render with HDCP encryption.
+ *
+ *	Content Type of a stream is decided by the owner of the stream, as
+ *	"HDCP Type0" or "HDCP Type1".
+ *
+ *	The value of the property can be one the below:
+ *	  - "HDCP Type0": DRM_MODE_HDCP_CONTENT_TYPE0 = 0
+ *	  - "HDCP Type1": DRM_MODE_HDCP_CONTENT_TYPE1 = 1
+ *
+ *	When kernel starts the HDCP authentication upon the "DESIRED" state of
+ *	the "Content Protection", it refers the "HDCP Content Type" property
+ *	state. And perform the HDCP authentication with the display sink for
+ *	the content type mentioned by "HDCP Content Type".
+ *
+ *	Stream classified as HDCP Type0 can be transmitted on a link which is
+ *	encrypted with HDCP 1.4 or higher versions of HDCP(i.e HDCP2.2
+ *	and more).
+ *
+ *	Streams classified as HDCP Type1 can be transmitted on a link which is
+ *	encrypted only with HDCP 2.2. In future, HDCP versions >2.2 also might
+ *	support Type1 based on their spec.
+ *
+ *	HDCP2.2 authentication protocol itself takes the "Content Type" as a
+ *	parameter, which is a input for the DP HDCP2.2 encryption algo.
+ *
+ *	Note that the HDCP Content Type property is introduced at HDCP 2.2, and
+ *	defaults to type 0. It is only exposed by drivers supporting HDCP 2.2.
+ *	Based on how next versions of HDCP specs are defined content Type could
+ *	be used for higher versions too.
+ *
+ *	If content type is changed when "Content Protection" is not UNDESIRED,
+ *	then kernel will disable the HDCP and re-enable with new type in the
+ *	same atomic commit. And when "Content Protection" is ENABLED, it means
+ *	that link is HDCP authenticated and encrypted, for the transmission of
+ *	the Type of stream mentioned at "HDCP Content Type".
+ *
  * HDR_OUTPUT_METADATA:
  *	Connector property to enable userspace to send HDR Metadata to
  *	driver. This metadata is based on the composition and blending
diff --git a/drivers/gpu/drm/drm_hdcp.c b/drivers/gpu/drm/drm_hdcp.c
index cd837bd409f7..ce235fd1c844 100644
--- a/drivers/gpu/drm/drm_hdcp.c
+++ b/drivers/gpu/drm/drm_hdcp.c
@@ -344,23 +344,41 @@ static struct drm_prop_enum_list drm_cp_enum_list[] = {
 };
 DRM_ENUM_NAME_FN(drm_get_content_protection_name, drm_cp_enum_list)
 
+static struct drm_prop_enum_list drm_hdcp_content_type_enum_list[] = {
+	{ DRM_MODE_HDCP_CONTENT_TYPE0, "HDCP Type0" },
+	{ DRM_MODE_HDCP_CONTENT_TYPE1, "HDCP Type1" },
+};
+DRM_ENUM_NAME_FN(drm_get_hdcp_content_type_name,
+		 drm_hdcp_content_type_enum_list)
+
 /**
  * drm_connector_attach_content_protection_property - attach content protection
  * property
  *
  * @connector: connector to attach CP property on.
+ * @hdcp_content_type: is HDCP Content Type property needed for connector
  *
  * This is used to add support for content protection on select connectors.
  * Content Protection is intentionally vague to allow for different underlying
  * technologies, however it is most implemented by HDCP.
  *
+ * When hdcp_content_type is true enum property called HDCP Content Type is
+ * created (if it is not already) and attached to the connector.
+ *
+ * This property is used for sending the protected content's stream type
+ * from userspace to kernel on selected connectors. Protected content provider
+ * will decide their type of their content and declare the same to kernel.
+ *
+ * Content type will be used during the HDCP 2.2 authentication.
+ * Content type will be set to &drm_connector_state.hdcp_content_type.
+ *
  * The content protection will be set to &drm_connector_state.content_protection
  *
  * Returns:
  * Zero on success, negative errno on failure.
  */
 int drm_connector_attach_content_protection_property(
-		struct drm_connector *connector)
+		struct drm_connector *connector, bool hdcp_content_type)
 {
 	struct drm_device *dev = connector->dev;
 	struct drm_property *prop =
@@ -377,6 +395,22 @@ int drm_connector_attach_content_protection_property(
 				   DRM_MODE_CONTENT_PROTECTION_UNDESIRED);
 	dev->mode_config.content_protection_property = prop;
 
+	if (!hdcp_content_type)
+		return 0;
+
+	prop = dev->mode_config.hdcp_content_type_property;
+	if (!prop)
+		prop = drm_property_create_enum(dev, 0, "HDCP Content Type",
+					drm_hdcp_content_type_enum_list,
+					ARRAY_SIZE(
+					drm_hdcp_content_type_enum_list));
+	if (!prop)
+		return -ENOMEM;
+
+	drm_object_attach_property(&connector->base, prop,
+				   DRM_MODE_HDCP_CONTENT_TYPE0);
+	dev->mode_config.hdcp_content_type_property = prop;
+
 	return 0;
 }
 EXPORT_SYMBOL(drm_connector_attach_content_protection_property);
diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
index bc3a94d491c4..2a4d10952b74 100644
--- a/drivers/gpu/drm/i915/display/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
@@ -1829,7 +1829,9 @@ int intel_hdcp_init(struct intel_connector *connector,
 	if (!shim)
 		return -EINVAL;
 
-	ret = drm_connector_attach_content_protection_property(&connector->base);
+	ret =
+	drm_connector_attach_content_protection_property(&connector->base,
+							 false);
 	if (ret)
 		return ret;
 
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 4c30d751487a..d6432967a2a9 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -601,6 +601,12 @@ struct drm_connector_state {
 	 */
 	unsigned int content_type;
 
+	/**
+	 * @hdcp_content_type: Connector property to pass the type of
+	 * protected content. This is most commonly used for HDCP.
+	 */
+	unsigned int hdcp_content_type;
+
 	/**
 	 * @scaling_mode: Connector property to control the
 	 * upscaling, mostly used for built-in panels.
@@ -1484,6 +1490,7 @@ const char *drm_get_dvi_i_select_name(int val);
 const char *drm_get_tv_subconnector_name(int val);
 const char *drm_get_tv_select_name(int val);
 const char *drm_get_content_protection_name(int val);
+const char *drm_get_hdcp_content_type_name(int val);
 
 int drm_mode_create_dvi_i_properties(struct drm_device *dev);
 int drm_mode_create_tv_margin_properties(struct drm_device *dev);
diff --git a/include/drm/drm_hdcp.h b/include/drm/drm_hdcp.h
index 13771a496e2b..2970abdfaf12 100644
--- a/include/drm/drm_hdcp.h
+++ b/include/drm/drm_hdcp.h
@@ -291,5 +291,5 @@ struct drm_connector;
 bool drm_hdcp_check_ksvs_revoked(struct drm_device *dev,
 				 u8 *ksvs, u32 ksv_count);
 int drm_connector_attach_content_protection_property(
-		struct drm_connector *connector);
+		struct drm_connector *connector, bool hdcp_content_type);
 #endif
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 759d462d028b..6c4b5bc85951 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -849,6 +849,12 @@ struct drm_mode_config {
 	 */
 	struct drm_property *content_protection_property;
 
+	/**
+	 * @hdcp_content_type_property: DRM ENUM property for type of
+	 * Protected Content.
+	 */
+	struct drm_property *hdcp_content_type_property;
+
 	/* dumb ioctl parameters */
 	uint32_t preferred_depth, prefer_shadow;
 
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 5ab331e5dc23..5c954394093f 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -218,6 +218,10 @@ extern "C" {
 #define DRM_MODE_CONTENT_PROTECTION_DESIRED     1
 #define DRM_MODE_CONTENT_PROTECTION_ENABLED     2
 
+/* Content Type classification for HDCP2.2 vs others */
+#define DRM_MODE_HDCP_CONTENT_TYPE0		0
+#define DRM_MODE_HDCP_CONTENT_TYPE1		1
+
 struct drm_mode_modeinfo {
 	__u32 clock;
 	__u16 hdisplay;
-- 
2.19.1

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

[PATCH v9 2/6] drm/i915: Attach content type property

[Ramalingam C]

Attaches the content type property for HDCP2.2 capable connectors.

Implements the update of content type from property and apply the
restriction on HDCP version selection.

Need ACK for content type property from userspace consumer.

v2:
  s/cp_content_type/content_protection_type [daniel]
  disable at hdcp_atomic_check to avoid check at atomic_set_property
	[Maarten]
v3:
  s/content_protection_type/hdcp_content_type [Pekka]
v4:
  hdcp disable incase of type change is moved into commit [daniel].
v5:
  Simplified the Type change procedure. [Daniel]
v6:
  Type change with UNDESIRED state is ignored.

Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/display/intel_ddi.c  | 39 ++++++++++++++++----
 drivers/gpu/drm/i915/display/intel_hdcp.c | 43 +++++++++++++++--------
 drivers/gpu/drm/i915/display/intel_hdcp.h |  2 +-
 3 files changed, 62 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index a4172595c8d8..862907393a6d 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -3514,7 +3514,8 @@ static void intel_enable_ddi(struct intel_encoder *encoder,
 	/* Enable hdcp if it's desired */
 	if (conn_state->content_protection ==
 	    DRM_MODE_CONTENT_PROTECTION_DESIRED)
-		intel_hdcp_enable(to_intel_connector(conn_state->connector));
+		intel_hdcp_enable(to_intel_connector(conn_state->connector),
+				  (u8)conn_state->hdcp_content_type);
 }
 
 static void intel_disable_ddi_dp(struct intel_encoder *encoder,
@@ -3583,15 +3584,41 @@ static void intel_ddi_update_pipe(struct intel_encoder *encoder,
 				  const struct intel_crtc_state *crtc_state,
 				  const struct drm_connector_state *conn_state)
 {
+	struct intel_connector *connector =
+				to_intel_connector(conn_state->connector);
+	struct intel_hdcp *hdcp = &connector->hdcp;
+	bool content_protection_type_changed =
+			(conn_state->hdcp_content_type != hdcp->content_type &&
+			 conn_state->content_protection !=
+			 DRM_MODE_CONTENT_PROTECTION_UNDESIRED);
+
 	if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
 		intel_ddi_update_pipe_dp(encoder, crtc_state, conn_state);
 
+	/*
+	 * During the HDCP encryption session if Type change is requested,
+	 * disable the HDCP and reenable it with new TYPE value.
+	 */
 	if (conn_state->content_protection ==
-	    DRM_MODE_CONTENT_PROTECTION_DESIRED)
-		intel_hdcp_enable(to_intel_connector(conn_state->connector));
-	else if (conn_state->content_protection ==
-		 DRM_MODE_CONTENT_PROTECTION_UNDESIRED)
-		intel_hdcp_disable(to_intel_connector(conn_state->connector));
+	    DRM_MODE_CONTENT_PROTECTION_UNDESIRED ||
+	    content_protection_type_changed)
+		intel_hdcp_disable(connector);
+
+	/*
+	 * Mark the hdcp state as DESIRED after the hdcp disable of type
+	 * change procedure.
+	 */
+	if (content_protection_type_changed) {
+		mutex_lock(&hdcp->mutex);
+		hdcp->value = DRM_MODE_CONTENT_PROTECTION_DESIRED;
+		schedule_work(&hdcp->prop_work);
+		mutex_unlock(&hdcp->mutex);
+	}
+
+	if (conn_state->content_protection ==
+	    DRM_MODE_CONTENT_PROTECTION_DESIRED ||
+	    content_protection_type_changed)
+		intel_hdcp_enable(connector, (u8)conn_state->hdcp_content_type);
 }
 
 static void intel_ddi_set_fia_lane_count(struct intel_encoder *encoder,
diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
index 2a4d10952b74..4580af57bddb 100644
--- a/drivers/gpu/drm/i915/display/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
@@ -1748,14 +1748,15 @@ static const struct component_ops i915_hdcp_component_ops = {
 	.unbind = i915_hdcp_component_unbind,
 };
 
-static inline int initialize_hdcp_port_data(struct intel_connector *connector)
+static inline int initialize_hdcp_port_data(struct intel_connector *connector,
+					    const struct intel_hdcp_shim *shim)
 {
 	struct intel_hdcp *hdcp = &connector->hdcp;
 	struct hdcp_port_data *data = &hdcp->port_data;
 
 	data->port = connector->encoder->port;
 	data->port_type = (u8)HDCP_PORT_TYPE_INTEGRATED;
-	data->protocol = (u8)hdcp->shim->protocol;
+	data->protocol = (u8)shim->protocol;
 
 	data->k = 1;
 	if (!data->streams)
@@ -1805,12 +1806,13 @@ void intel_hdcp_component_init(struct drm_i915_private *dev_priv)
 	}
 }
 
-static void intel_hdcp2_init(struct intel_connector *connector)
+static void intel_hdcp2_init(struct intel_connector *connector,
+			     const struct intel_hdcp_shim *shim)
 {
 	struct intel_hdcp *hdcp = &connector->hdcp;
 	int ret;
 
-	ret = initialize_hdcp_port_data(connector);
+	ret = initialize_hdcp_port_data(connector, shim);
 	if (ret) {
 		DRM_DEBUG_KMS("Mei hdcp data init failed\n");
 		return;
@@ -1829,25 +1831,28 @@ int intel_hdcp_init(struct intel_connector *connector,
 	if (!shim)
 		return -EINVAL;
 
+	if (is_hdcp2_supported(dev_priv))
+		intel_hdcp2_init(connector, shim);
+
 	ret =
 	drm_connector_attach_content_protection_property(&connector->base,
-							 false);
-	if (ret)
+							 hdcp->hdcp2_supported);
+	if (ret) {
+		hdcp->hdcp2_supported = false;
+		kfree(hdcp->port_data.streams);
 		return ret;
+	}
 
 	hdcp->shim = shim;
 	mutex_init(&hdcp->mutex);
 	INIT_DELAYED_WORK(&hdcp->check_work, intel_hdcp_check_work);
 	INIT_WORK(&hdcp->prop_work, intel_hdcp_prop_work);
-
-	if (is_hdcp2_supported(dev_priv))
-		intel_hdcp2_init(connector);
 	init_waitqueue_head(&hdcp->cp_irq_queue);
 
 	return 0;
 }
 
-int intel_hdcp_enable(struct intel_connector *connector)
+int intel_hdcp_enable(struct intel_connector *connector, u8 content_type)
 {
 	struct intel_hdcp *hdcp = &connector->hdcp;
 	unsigned long check_link_interval = DRM_HDCP_CHECK_PERIOD_MS;
@@ -1858,6 +1863,7 @@ int intel_hdcp_enable(struct intel_connector *connector)
 
 	mutex_lock(&hdcp->mutex);
 	WARN_ON(hdcp->value == DRM_MODE_CONTENT_PROTECTION_ENABLED);
+	hdcp->content_type = content_type;
 
 	/*
 	 * Considering that HDCP2.2 is more secure than HDCP1.4, If the setup
@@ -1869,8 +1875,12 @@ int intel_hdcp_enable(struct intel_connector *connector)
 			check_link_interval = DRM_HDCP2_CHECK_PERIOD_MS;
 	}
 
-	/* When HDCP2.2 fails, HDCP1.4 will be attempted */
-	if (ret && intel_hdcp_capable(connector)) {
+	/*
+	 * When HDCP2.2 fails and Content Type is not Type1, HDCP1.4 will
+	 * be attempted.
+	 */
+	if (ret && intel_hdcp_capable(connector) &&
+	    hdcp->content_type != DRM_MODE_HDCP_CONTENT_TYPE1) {
 		ret = _intel_hdcp_enable(connector);
 	}
 
@@ -1952,12 +1962,15 @@ void intel_hdcp_atomic_check(struct drm_connector *connector,
 
 	/*
 	 * Nothing to do if the state didn't change, or HDCP was activated since
-	 * the last commit
+	 * the last commit. And also no change in hdcp content type.
 	 */
 	if (old_cp == new_cp ||
 	    (old_cp == DRM_MODE_CONTENT_PROTECTION_DESIRED &&
-	     new_cp == DRM_MODE_CONTENT_PROTECTION_ENABLED))
-		return;
+	     new_cp == DRM_MODE_CONTENT_PROTECTION_ENABLED)) {
+		if (old_state->hdcp_content_type ==
+				new_state->hdcp_content_type)
+			return;
+	}
 
 	crtc_state = drm_atomic_get_new_crtc_state(new_state->state,
 						   new_state->crtc);
diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.h b/drivers/gpu/drm/i915/display/intel_hdcp.h
index be8da85c866a..13555b054930 100644
--- a/drivers/gpu/drm/i915/display/intel_hdcp.h
+++ b/drivers/gpu/drm/i915/display/intel_hdcp.h
@@ -21,7 +21,7 @@ void intel_hdcp_atomic_check(struct drm_connector *connector,
 			     struct drm_connector_state *new_state);
 int intel_hdcp_init(struct intel_connector *connector,
 		    const struct intel_hdcp_shim *hdcp_shim);
-int intel_hdcp_enable(struct intel_connector *connector);
+int intel_hdcp_enable(struct intel_connector *connector, u8 content_type);
 int intel_hdcp_disable(struct intel_connector *connector);
 bool is_hdcp_supported(struct drm_i915_private *dev_priv, enum port port);
 bool intel_hdcp_capable(struct intel_connector *connector);
-- 
2.19.1

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

[PATCH v9 3/6] drm: uevent for connector status change

[Ramalingam C]

DRM API for generating uevent for a status changes of connector's
property.

This uevent will have following details related to the status change:

  HOTPLUG=1, CONNECTOR=<connector_id> and PROPERTY=<property_id>

Need ACK from this uevent from userspace consumer.

v2:
  Minor fixes at KDoc comments [Daniel]
v3:
  Check the property is really attached with connector [Daniel]

Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_sysfs.c | 35 +++++++++++++++++++++++++++++++++++
 include/drm/drm_sysfs.h     |  5 ++++-
 2 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index ad10810bc972..d13a77057045 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -26,6 +26,7 @@
 #include <drm/drm_sysfs.h>
 
 #include "drm_internal.h"
+#include "drm_crtc_internal.h"
 
 #define to_drm_minor(d) dev_get_drvdata(d)
 #define to_drm_connector(d) dev_get_drvdata(d)
@@ -325,6 +326,9 @@ void drm_sysfs_lease_event(struct drm_device *dev)
  * Send a uevent for the DRM device specified by @dev.  Currently we only
  * set HOTPLUG=1 in the uevent environment, but this could be expanded to
  * deal with other types of events.
+ *
+ * Any new uapi should be using the drm_sysfs_connector_status_event()
+ * for uevents on connector status change.
  */
 void drm_sysfs_hotplug_event(struct drm_device *dev)
 {
@@ -337,6 +341,37 @@ void drm_sysfs_hotplug_event(struct drm_device *dev)
 }
 EXPORT_SYMBOL(drm_sysfs_hotplug_event);
 
+/**
+ * drm_sysfs_connector_status_event - generate a DRM uevent for connector
+ * property status change
+ * @connector: connector on which property status changed
+ * @property: connector property whoes status changed.
+ *
+ * Send a uevent for the DRM device specified by @dev.  Currently we
+ * set HOTPLUG=1 and connector id along with the attached property id
+ * related to the status change.
+ */
+void drm_sysfs_connector_status_event(struct drm_connector *connector,
+				      struct drm_property *property)
+{
+	struct drm_device *dev = connector->dev;
+	char hotplug_str[] = "HOTPLUG=1", conn_id[30], prop_id[30];
+	char *envp[4] = { hotplug_str, conn_id, prop_id, NULL };
+
+	WARN_ON(!drm_mode_obj_find_prop_id(&connector->base,
+					   property->base.id));
+
+	snprintf(conn_id, ARRAY_SIZE(conn_id),
+		 "CONNECTOR=%u", connector->base.id);
+	snprintf(prop_id, ARRAY_SIZE(prop_id),
+		 "PROPERTY=%u", property->base.id);
+
+	DRM_DEBUG("generating connector status event\n");
+
+	kobject_uevent_env(&dev->primary->kdev->kobj, KOBJ_CHANGE, envp);
+}
+EXPORT_SYMBOL(drm_sysfs_connector_status_event);
+
 static void drm_sysfs_release(struct device *dev)
 {
 	kfree(dev);
diff --git a/include/drm/drm_sysfs.h b/include/drm/drm_sysfs.h
index 4f311e836cdc..d454ef617b2c 100644
--- a/include/drm/drm_sysfs.h
+++ b/include/drm/drm_sysfs.h
@@ -4,10 +4,13 @@
 
 struct drm_device;
 struct device;
+struct drm_connector;
+struct drm_property;
 
 int drm_class_device_register(struct device *dev);
 void drm_class_device_unregister(struct device *dev);
 
 void drm_sysfs_hotplug_event(struct drm_device *dev);
-
+void drm_sysfs_connector_status_event(struct drm_connector *connector,
+				      struct drm_property *property);
 #endif
-- 
2.19.1

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

[PATCH v9 4/6] drm/hdcp: update content protection property with uevent

[Ramalingam C]

drm function is defined and exported to update a connector's
content protection property state and to generate a uevent along
with it.

Need ACK for the uevent from userspace consumer.

v2:
  Update only when state is different from old one.
v3:
  KDoc is added [Daniel]
v4:
  KDoc is extended bit more [pekka]
v5:
  Uevent usage is documented at kdoc of "Content Protection" also
  [pekka]

Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_connector.c | 17 +++++++++++++----
 drivers/gpu/drm/drm_hdcp.c      | 34 +++++++++++++++++++++++++++++++++
 include/drm/drm_hdcp.h          |  2 ++
 3 files changed, 49 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 732f6645643d..6de906ef10b3 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -947,10 +947,19 @@ static const struct drm_prop_enum_list hdmi_colorspaces[] = {
  *	- If the state is DESIRED, kernel should attempt to re-authenticate the
  *	  link whenever possible. This includes across disable/enable, dpms,
  *	  hotplug, downstream device changes, link status failures, etc..
- *	- Userspace is responsible for polling the property to determine when
- *	  the value transitions from ENABLED to DESIRED. This signifies the link
- *	  is no longer protected and userspace should take appropriate action
- *	  (whatever that might be).
+ *	- Kernel sends uevent with the connector id and property id through
+ *	  @drm_hdcp_update_content_protection, upon below kernel triggered
+ *	  scenarios:
+ *		DESIRED -> ENABLED	(authentication success)
+ *		ENABLED -> DESIRED	(termination of authentication)
+ *	- Please note no uevents for userspace triggered property state changes,
+ *	  which can't fail such as
+ *		DESIRED/ENABLED -> UNDESIRED
+ *		UNDESIRED -> DESIRED
+ *	- Userspace is responsible for polling the property or listen to uevents
+ *	  to determine when the value transitions from ENABLED to DESIRED.
+ *	  This signifies the link is no longer protected and userspace should
+ *	  take appropriate action (whatever that might be).
  *
  * HDCP Content Type:
  *	This Enum property is used by the userspace to declare the content type
diff --git a/drivers/gpu/drm/drm_hdcp.c b/drivers/gpu/drm/drm_hdcp.c
index ce235fd1c844..77433ee3d652 100644
--- a/drivers/gpu/drm/drm_hdcp.c
+++ b/drivers/gpu/drm/drm_hdcp.c
@@ -374,6 +374,10 @@ DRM_ENUM_NAME_FN(drm_get_hdcp_content_type_name,
  *
  * The content protection will be set to &drm_connector_state.content_protection
  *
+ * When kernel triggered content protection state change like DESIRED->ENABLED
+ * and ENABLED->DESIRED, will use drm_hdcp_update_content_protection() to update
+ * the content protection state of a connector.
+ *
  * Returns:
  * Zero on success, negative errno on failure.
  */
@@ -414,3 +418,33 @@ int drm_connector_attach_content_protection_property(
 	return 0;
 }
 EXPORT_SYMBOL(drm_connector_attach_content_protection_property);
+
+/**
+ * drm_hdcp_update_content_protection - Updates the content protection state
+ * of a connector
+ *
+ * @connector: drm_connector on which content protection state needs an update
+ * @val: New state of the content protection property
+ *
+ * This function can be used by display drivers, to update the kernel triggered
+ * content protection state changes of a drm_connector such as DESIRED->ENABLED
+ * and ENABLED->DESIRED. No uevent for DESIRED->UNDESIRED or ENABLED->UNDESIRED,
+ * as userspace is triggering such state change and kernel performs it without
+ * fail.This function update the new state of the property into the connector's
+ * state and generate an uevent to notify the userspace.
+ */
+void drm_hdcp_update_content_protection(struct drm_connector *connector,
+					u64 val)
+{
+	struct drm_device *dev = connector->dev;
+	struct drm_connector_state *state = connector->state;
+
+	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
+	if (state->content_protection == val)
+		return;
+
+	state->content_protection = val;
+	drm_sysfs_connector_status_event(connector,
+				 dev->mode_config.content_protection_property);
+}
+EXPORT_SYMBOL(drm_hdcp_update_content_protection);
diff --git a/include/drm/drm_hdcp.h b/include/drm/drm_hdcp.h
index 2970abdfaf12..dd864ac9ce85 100644
--- a/include/drm/drm_hdcp.h
+++ b/include/drm/drm_hdcp.h
@@ -292,4 +292,6 @@ bool drm_hdcp_check_ksvs_revoked(struct drm_device *dev,
 				 u8 *ksvs, u32 ksv_count);
 int drm_connector_attach_content_protection_property(
 		struct drm_connector *connector, bool hdcp_content_type);
+void drm_hdcp_update_content_protection(struct drm_connector *connector,
+					u64 val);
 #endif
-- 
2.19.1

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

[PATCH v9 5/6] drm/i915: update the hdcp state with uevent

[Ramalingam C]

drm function to update the content protection property state and to
generate a uevent is invoked from the intel hdcp property work.

Hence whenever kernel changes the property state, userspace will be
updated with a uevent.

Need a ACK for uevent generating uAPI from userspace.

v2:
  state update is moved into drm function [daniel]

Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/display/intel_hdcp.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
index 4580af57bddb..e56969ebdd25 100644
--- a/drivers/gpu/drm/i915/display/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
@@ -865,7 +865,6 @@ static void intel_hdcp_prop_work(struct work_struct *work)
 					       prop_work);
 	struct intel_connector *connector = intel_hdcp_to_connector(hdcp);
 	struct drm_device *dev = connector->base.dev;
-	struct drm_connector_state *state;
 
 	drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
 	mutex_lock(&hdcp->mutex);
@@ -875,10 +874,9 @@ static void intel_hdcp_prop_work(struct work_struct *work)
 	 * those to UNDESIRED is handled by core. If value == UNDESIRED,
 	 * we're running just after hdcp has been disabled, so just exit
 	 */
-	if (hdcp->value != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
-		state = connector->base.state;
-		state->content_protection = hdcp->value;
-	}
+	if (hdcp->value != DRM_MODE_CONTENT_PROTECTION_UNDESIRED)
+		drm_hdcp_update_content_protection(&connector->base,
+						   hdcp->value);
 
 	mutex_unlock(&hdcp->mutex);
 	drm_modeset_unlock(&dev->mode_config.connection_mutex);
-- 
2.19.1

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

[PATCH v9 6/6] drm/hdcp: reference for srm file format

[Ramalingam C]

In the kernel documentation, HDCP specifications links are shared as a
reference for SRM table format.

Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
---
 drivers/gpu/drm/drm_hdcp.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/drm_hdcp.c b/drivers/gpu/drm/drm_hdcp.c
index 77433ee3d652..803bf8283b83 100644
--- a/drivers/gpu/drm/drm_hdcp.c
+++ b/drivers/gpu/drm/drm_hdcp.c
@@ -271,6 +271,13 @@ static void drm_hdcp_request_srm(struct drm_device *drm_dev)
  *
  * SRM should be presented in the name of "display_hdcp_srm.bin".
  *
+ * Format of the SRM table that userspace needs to write into the binary file
+ * is defined at
+ * 1. Renewability chapter on 55th page of HDCP 1.4 specification
+ * https://www.digital-cp.com/sites/default/files/specifications/HDCP%20Specification%20Rev1_4_Secure.pdf
+ * 2. Renewability chapter on 63rd page of HDCP 2.2 specification
+ * https://www.digital-cp.com/sites/default/files/specifications/HDCP%20on%20HDMI%20Specification%20Rev2_2_Final1.pdf
+ *
  * Returns:
  * TRUE on any of the KSV is revoked, else FALSE.
  */
-- 
2.19.1

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

✗ Fi.CI.CHECKPATCH: warning for HDCP2.2 Phase II (rev11)

[Patchwork]

== Series Details ==

Series: HDCP2.2 Phase II (rev11)
URL   : https://patchwork.freedesktop.org/series/57232/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
4850c38d571b drm: Add Content protection type property
-:123: CHECK:LINE_SPACING: Please use a blank line after function/struct/union/enum declarations
#123: FILE: drivers/gpu/drm/drm_hdcp.c:351:
+};
+DRM_ENUM_NAME_FN(drm_get_hdcp_content_type_name,

-:168: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#168: FILE: drivers/gpu/drm/drm_hdcp.c:404:
+		prop = drm_property_create_enum(dev, 0, "HDCP Content Type",
+					drm_hdcp_content_type_enum_list,

-:169: CHECK:OPEN_ENDED_LINE: Lines should not end with a '('
#169: FILE: drivers/gpu/drm/drm_hdcp.c:405:
+					ARRAY_SIZE(

total: 0 errors, 0 warnings, 3 checks, 182 lines checked
3bc2f2d69357 drm/i915: Attach content type property
80479a8101ac drm: uevent for connector status change
d37bd325f08f drm/hdcp: update content protection property with uevent
-:99: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#99: FILE: drivers/gpu/drm/drm_hdcp.c:448:
+	drm_sysfs_connector_status_event(connector,
+				 dev->mode_config.content_protection_property);

total: 0 errors, 0 warnings, 1 checks, 72 lines checked
ee2bd3463203 drm/i915: update the hdcp state with uevent
584ff79d12ea drm/hdcp: reference for srm file format

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

✓ Fi.CI.BAT: success for HDCP2.2 Phase II (rev11)

[Patchwork]

== Series Details ==

Series: HDCP2.2 Phase II (rev11)
URL   : https://patchwork.freedesktop.org/series/57232/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6431 -> Patchwork_13570
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13570/

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_create@basic-files:
    - fi-icl-u3:          [PASS][1] -> [INCOMPLETE][2] ([fdo#107713] / [fdo#109100])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6431/fi-icl-u3/igt@gem_ctx_create@basic-files.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13570/fi-icl-u3/igt@gem_ctx_create@basic-files.html
    - fi-icl-dsi:         [PASS][3] -> [INCOMPLETE][4] ([fdo#107713] / [fdo#109100])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6431/fi-icl-dsi/igt@gem_ctx_create@basic-files.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13570/fi-icl-dsi/igt@gem_ctx_create@basic-files.html

  * igt@i915_module_load@reload:
    - fi-blb-e6850:       [PASS][5] -> [INCOMPLETE][6] ([fdo#107718])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6431/fi-blb-e6850/igt@i915_module_load@reload.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13570/fi-blb-e6850/igt@i915_module_load@reload.html

  * igt@i915_selftest@live_hangcheck:
    - fi-icl-u2:          [PASS][7] -> [INCOMPLETE][8] ([fdo#107713] / [fdo#108569])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6431/fi-icl-u2/igt@i915_selftest@live_hangcheck.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13570/fi-icl-u2/igt@i915_selftest@live_hangcheck.html

  * igt@kms_frontbuffer_tracking@basic:
    - fi-hsw-peppy:       [PASS][9] -> [DMESG-WARN][10] ([fdo#102614])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6431/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13570/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html

  
#### Possible fixes ####

  * igt@gem_close_race@basic-process:
    - fi-icl-u3:          [DMESG-WARN][11] ([fdo#107724]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6431/fi-icl-u3/igt@gem_close_race@basic-process.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13570/fi-icl-u3/igt@gem_close_race@basic-process.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       [FAIL][13] ([fdo#109485]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6431/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13570/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html

  * igt@kms_frontbuffer_tracking@basic:
    - fi-icl-u2:          [FAIL][15] ([fdo#103167]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6431/fi-icl-u2/igt@kms_frontbuffer_tracking@basic.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13570/fi-icl-u2/igt@kms_frontbuffer_tracking@basic.html

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

  [fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#109100]: https://bugs.freedesktop.org/show_bug.cgi?id=109100
  [fdo#109485]: https://bugs.freedesktop.org/show_bug.cgi?id=109485


Participating hosts (53 -> 46)
------------------------------

  Additional (1): fi-pnv-d510 
  Missing    (8): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-bwr-2160 fi-icl-y fi-byt-clapper fi-bdw-samus 


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

  * Linux: CI_DRM_6431 -> Patchwork_13570

  CI_DRM_6431: 9a40fb28e45261f2fc44a9b271c19faf1f071138 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5090: c6c75e11175baeb6b984e0cc13c6fbe2863a0794 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13570: 584ff79d12eacd58cfd07e6b06351e21464c3fd5 @ git://anongit.freedesktop.org/gfx-ci/linux


== Kernel 32bit build ==

Warning: Kernel 32bit buildtest failed:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13570/build_32bit.log

  CALL    scripts/checksyscalls.sh
  CALL    scripts/atomic/check-atomics.sh
  CHK     include/generated/compile.h
Kernel: arch/x86/boot/bzImage is ready  (#1)
  Building modules, stage 2.
  MODPOST 112 modules
ERROR: "__udivdi3" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
ERROR: "__divdi3" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
scripts/Makefile.modpost:91: recipe for target '__modpost' failed
make[1]: *** [__modpost] Error 1
Makefile:1287: recipe for target 'modules' failed
make: *** [modules] Error 2


== Linux commits ==

584ff79d12ea drm/hdcp: reference for srm file format
ee2bd3463203 drm/i915: update the hdcp state with uevent
d37bd325f08f drm/hdcp: update content protection property with uevent
80479a8101ac drm: uevent for connector status change
3bc2f2d69357 drm/i915: Attach content type property
4850c38d571b drm: Add Content protection type property

== Logs ==

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

✓ Fi.CI.IGT: success for HDCP2.2 Phase II (rev11)

[Patchwork]

== Series Details ==

Series: HDCP2.2 Phase II (rev11)
URL   : https://patchwork.freedesktop.org/series/57232/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6431_full -> Patchwork_13570_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

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

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_pm_rc6_residency@rc6-accuracy:
    - shard-kbl:          [PASS][1] -> [SKIP][2] ([fdo#109271])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6431/shard-kbl1/igt@i915_pm_rc6_residency@rc6-accuracy.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13570/shard-kbl2/igt@i915_pm_rc6_residency@rc6-accuracy.html

  * igt@i915_selftest@live_hangcheck:
    - shard-iclb:         [PASS][3] -> [INCOMPLETE][4] ([fdo#107713] / [fdo#108569])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6431/shard-iclb8/igt@i915_selftest@live_hangcheck.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13570/shard-iclb5/igt@i915_selftest@live_hangcheck.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size:
    - shard-snb:          [PASS][5] -> [SKIP][6] ([fdo#109271])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6431/shard-snb6/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13570/shard-snb6/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html

  * igt@kms_draw_crc@draw-method-rgb565-render-xtiled:
    - shard-skl:          [PASS][7] -> [FAIL][8] ([fdo#103184] / [fdo#103232])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6431/shard-skl2/igt@kms_draw_crc@draw-method-rgb565-render-xtiled.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13570/shard-skl7/igt@kms_draw_crc@draw-method-rgb565-render-xtiled.html

  * igt@kms_flip@dpms-vs-vblank-race-interruptible:
    - shard-glk:          [PASS][9] -> [FAIL][10] ([fdo#103060])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6431/shard-glk9/igt@kms_flip@dpms-vs-vblank-race-interruptible.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13570/shard-glk5/igt@kms_flip@dpms-vs-vblank-race-interruptible.html

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-skl:          [PASS][11] -> [FAIL][12] ([fdo#105363])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6431/shard-skl1/igt@kms_flip@flip-vs-expired-vblank.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13570/shard-skl4/igt@kms_flip@flip-vs-expired-vblank.html

  * igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite:
    - shard-iclb:         [PASS][13] -> [FAIL][14] ([fdo#103167]) +2 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6431/shard-iclb3/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13570/shard-iclb7/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite.html

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b-frame-sequence:
    - shard-skl:          [PASS][15] -> [FAIL][16] ([fdo#103191])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6431/shard-skl5/igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b-frame-sequence.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13570/shard-skl10/igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b-frame-sequence.html

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
    - shard-apl:          [PASS][17] -> [DMESG-WARN][18] ([fdo#108566])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6431/shard-apl3/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13570/shard-apl3/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes:
    - shard-kbl:          [PASS][19] -> [DMESG-WARN][20] ([fdo#108566]) +4 similar issues
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6431/shard-kbl3/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13570/shard-kbl6/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
    - shard-iclb:         [PASS][21] -> [INCOMPLETE][22] ([fdo#107713] / [fdo#110042])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6431/shard-iclb5/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13570/shard-iclb3/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min:
    - shard-skl:          [PASS][23] -> [FAIL][24] ([fdo#108145])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6431/shard-skl2/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13570/shard-skl7/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html

  * igt@kms_psr@psr2_sprite_render:
    - shard-iclb:         [PASS][25] -> [SKIP][26] ([fdo#109441]) +1 similar issue
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6431/shard-iclb2/igt@kms_psr@psr2_sprite_render.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13570/shard-iclb6/igt@kms_psr@psr2_sprite_render.html

  
#### Possible fixes ####

  * igt@gem_eio@unwedge-stress:
    - shard-snb:          [FAIL][27] ([fdo#109661]) -> [PASS][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6431/shard-snb5/igt@gem_eio@unwedge-stress.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13570/shard-snb2/igt@gem_eio@unwedge-stress.html

  * igt@i915_pm_rpm@i2c:
    - shard-hsw:          [FAIL][29] ([fdo#104097]) -> [PASS][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6431/shard-hsw2/igt@i915_pm_rpm@i2c.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13570/shard-hsw8/igt@i915_pm_rpm@i2c.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-skl:          [FAIL][31] ([fdo#105363]) -> [PASS][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6431/shard-skl10/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13570/shard-skl2/igt@kms_flip@flip-vs-expired-vblank-interruptible.html

  * igt@kms_flip@plain-flip-fb-recreate-interruptible:
    - shard-skl:          [FAIL][33] ([fdo#100368]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6431/shard-skl3/igt@kms_flip@plain-flip-fb-recreate-interruptible.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13570/shard-skl6/igt@kms_flip@plain-flip-fb-recreate-interruptible.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render:
    - shard-iclb:         [FAIL][35] ([fdo#103167]) -> [PASS][36] +5 similar issues
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6431/shard-iclb2/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13570/shard-iclb2/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render.html

  * igt@kms_plane@pixel-format-pipe-a-planes-source-clamping:
    - shard-iclb:         [INCOMPLETE][37] ([fdo#107713] / [fdo#110036 ]) -> [PASS][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6431/shard-iclb8/igt@kms_plane@pixel-format-pipe-a-planes-source-clamping.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13570/shard-iclb1/igt@kms_plane@pixel-format-pipe-a-planes-source-clamping.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes:
    - shard-apl:          [DMESG-WARN][39] ([fdo#108566]) -> [PASS][40] +1 similar issue
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6431/shard-apl7/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13570/shard-apl1/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes.html

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          [FAIL][41] ([fdo#108145] / [fdo#110403]) -> [PASS][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6431/shard-skl4/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13570/shard-skl5/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html

  * igt@kms_psr@psr2_cursor_blt:
    - shard-iclb:         [SKIP][43] ([fdo#109441]) -> [PASS][44] +3 similar issues
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6431/shard-iclb5/igt@kms_psr@psr2_cursor_blt.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13570/shard-iclb2/igt@kms_psr@psr2_cursor_blt.html

  * igt@kms_rotation_crc@multiplane-rotation-cropping-bottom:
    - shard-iclb:         [INCOMPLETE][45] ([fdo#107713] / [fdo#110026]) -> [PASS][46]
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6431/shard-iclb7/igt@kms_rotation_crc@multiplane-rotation-cropping-bottom.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13570/shard-iclb7/igt@kms_rotation_crc@multiplane-rotation-cropping-bottom.html

  
#### Warnings ####

  * igt@kms_content_protection@atomic:
    - shard-kbl:          [DMESG-FAIL][47] -> [FAIL][48] ([fdo#110321] / [fdo#110336])
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6431/shard-kbl2/igt@kms_content_protection@atomic.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13570/shard-kbl3/igt@kms_content_protection@atomic.html

  
  [fdo#100368]: https://bugs.freedesktop.org/show_bug.cgi?id=100368
  [fdo#103060]: https://bugs.freedesktop.org/show_bug.cgi?id=103060
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103184]: https://bugs.freedesktop.org/show_bug.cgi?id=103184
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#104097]: https://bugs.freedesktop.org/show_bug.cgi?id=104097
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109661]: https://bugs.freedesktop.org/show_bug.cgi?id=109661
  [fdo#110026]: https://bugs.freedesktop.org/show_bug.cgi?id=110026
  [fdo#110036 ]: https://bugs.freedesktop.org/show_bug.cgi?id=110036 
  [fdo#110042]: https://bugs.freedesktop.org/show_bug.cgi?id=110042
  [fdo#110321]: https://bugs.freedesktop.org/show_bug.cgi?id=110321
  [fdo#110336]: https://bugs.freedesktop.org/show_bug.cgi?id=110336
  [fdo#110403]: https://bugs.freedesktop.org/show_bug.cgi?id=110403


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

  No changes in participating hosts


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

  * Linux: CI_DRM_6431 -> Patchwork_13570

  CI_DRM_6431: 9a40fb28e45261f2fc44a9b271c19faf1f071138 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5090: c6c75e11175baeb6b984e0cc13c6fbe2863a0794 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13570: 584ff79d12eacd58cfd07e6b06351e21464c3fd5 @ 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_13570/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v9 1/6] drm: Add Content protection type property

[Ramalingam C]

On 2019-07-09 at 17:31:10 +0300, Pekka Paalanen wrote:
> On Mon,  8 Jul 2019 16:51:11 +0530
> Ramalingam C <ramalingam.c@intel.com> wrote:
> 
> > This patch adds a DRM ENUM property to the selected connectors.
> > This property is used for mentioning the protected content's type
> > from userspace to kernel HDCP authentication.
> > 
> > Type of the stream is decided by the protected content providers.
> > Type 0 content can be rendered on any HDCP protected display wires.
> > But Type 1 content can be rendered only on HDCP2.2 protected paths.
> > 
> > So when a userspace sets this property to Type 1 and starts the HDCP
> > enable, kernel will honour it only if HDCP2.2 authentication is through
> > for type 1. Else HDCP enable will be failed.
> > 
> > Need ACK for this new conenctor property from userspace consumer.
> > 
> > v2:
> >   cp_content_type is replaced with content_protection_type [daniel]
> >   check at atomic_set_property is removed [Maarten]
> > v3:
> >   %s/content_protection_type/hdcp_content_type [Pekka]
> > v4:
> >   property is created for the first requested connector and then reused.
> > 	[Danvet]
> > v5:
> >   kernel doc nits addressed [Daniel]
> >   Rebased as part of patch reordering.
> > v6:
> >   Kernel docs are modified [pekka]
> > v7:
> >   More details in Kernel docs. [pekka]
> > 
> > Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > ---
> >  drivers/gpu/drm/drm_atomic_uapi.c         |  4 +++
> >  drivers/gpu/drm/drm_connector.c           | 39 +++++++++++++++++++++++
> >  drivers/gpu/drm/drm_hdcp.c                | 36 ++++++++++++++++++++-
> >  drivers/gpu/drm/i915/display/intel_hdcp.c |  4 ++-
> >  include/drm/drm_connector.h               |  7 ++++
> >  include/drm/drm_hdcp.h                    |  2 +-
> >  include/drm/drm_mode_config.h             |  6 ++++
> >  include/uapi/drm/drm_mode.h               |  4 +++
> >  8 files changed, 99 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> > index abe38bdf85ae..19ae119f1a5d 100644
> > --- a/drivers/gpu/drm/drm_atomic_uapi.c
> > +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> > @@ -747,6 +747,8 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
> >  			return -EINVAL;
> >  		}
> >  		state->content_protection = val;
> > +	} else if (property == config->hdcp_content_type_property) {
> > +		state->hdcp_content_type = val;
> >  	} else if (property == connector->colorspace_property) {
> >  		state->colorspace = val;
> >  	} else if (property == config->writeback_fb_id_property) {
> > @@ -831,6 +833,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
> >  			state->hdr_output_metadata->base.id : 0;
> >  	} else if (property == config->content_protection_property) {
> >  		*val = state->content_protection;
> > +	} else if (property == config->hdcp_content_type_property) {
> > +		*val = state->hdcp_content_type;
> >  	} else if (property == config->writeback_fb_id_property) {
> >  		/* Writeback framebuffer is one-shot, write and forget */
> >  		*val = 0;
> > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> > index 068d4b05f1be..732f6645643d 100644
> > --- a/drivers/gpu/drm/drm_connector.c
> > +++ b/drivers/gpu/drm/drm_connector.c
> > @@ -952,6 +952,45 @@ static const struct drm_prop_enum_list hdmi_colorspaces[] = {
> >   *	  is no longer protected and userspace should take appropriate action
> >   *	  (whatever that might be).
> >   *
> > + * HDCP Content Type:
> > + *	This Enum property is used by the userspace to declare the content type
> > + *	of the display stream, to kernel. Here display stream stands for any
> > + *	display content that userspace intended to render with HDCP encryption.
> 
> Hi,
> 
> I'd suggest s/render with/display through/.
> 
> As a gfx dev, rendering is something quite different to me.
Ok.
> 
> > + *
> > + *	Content Type of a stream is decided by the owner of the stream, as
> > + *	"HDCP Type0" or "HDCP Type1".
> > + *
> > + *	The value of the property can be one the below:
> 
> *one of the below
Sure.
> 
> > + *	  - "HDCP Type0": DRM_MODE_HDCP_CONTENT_TYPE0 = 0
> > + *	  - "HDCP Type1": DRM_MODE_HDCP_CONTENT_TYPE1 = 1
> > + *
> > + *	When kernel starts the HDCP authentication upon the "DESIRED" state of
> > + *	the "Content Protection", it refers the "HDCP Content Type" property
> > + *	state. And perform the HDCP authentication with the display sink for
> > + *	the content type mentioned by "HDCP Content Type".
> 
> How about:
> 
> 	When kernel starts the HDCP authentication (see "Content Protection"
> 	for details), it uses the content type in "HDCP Content Type"
> 	for performing the HDCP authentication with the display sink.
less confusing :) Thanks.
> 
> > + *
> > + *	Stream classified as HDCP Type0 can be transmitted on a link which is
> > + *	encrypted with HDCP 1.4 or higher versions of HDCP(i.e HDCP2.2
> > + *	and more).
> 
> This is where I get confused, see my earlier email from today on the
> previous revision of this patch series. Is it necessary to talk about
> HDCP versions here? The only thing that matters for UAPI is the content
> type, right?
> 
> Previously you said that the kernel will not use Type1 if userspace
> only asked for Type0, but to me this text reads as quite the opposite.
Simple. HDCP2.2 itself support both Type 0 and Type 1. where as HDCP1.4
by default supports the Type 0 and doesn't support the Type 1.

I guess you are getting confused by assigning the type to the versions.
> 
> > + *
> > + *	Streams classified as HDCP Type1 can be transmitted on a link which is
> > + *	encrypted only with HDCP 2.2. In future, HDCP versions >2.2 also might
> > + *	support Type1 based on their spec.
> > + *
> > + *	HDCP2.2 authentication protocol itself takes the "Content Type" as a
> > + *	parameter, which is a input for the DP HDCP2.2 encryption algo.
> > + *
> > + *	Note that the HDCP Content Type property is introduced at HDCP 2.2, and
> > + *	defaults to type 0. It is only exposed by drivers supporting HDCP 2.2.
> > + *	Based on how next versions of HDCP specs are defined content Type could
> > + *	be used for higher versions too.
> 
> Ok, userspace does not have to cope with a "HDCP Content Type" property
> that is missing the enum value Type1. I think the Weston patch would
> attempt something silly or misbehave if Type1 value was ever missing.
> Not having the whole property is fine, of course.
 If the Type1 is not supported then there is no need for the "HDCP
 Content Type" property itself. Thats why when a driver has the support
 for Type1( as of now HDCP2.2) only then "HDCP Content Type" will be
 exposed. So weston is fine at its current state.

 -Ram
> 
> > + *
> > + *	If content type is changed when "Content Protection" is not UNDESIRED,
> > + *	then kernel will disable the HDCP and re-enable with new type in the
> > + *	same atomic commit. And when "Content Protection" is ENABLED, it means
> > + *	that link is HDCP authenticated and encrypted, for the transmission of
> > + *	the Type of stream mentioned at "HDCP Content Type".
> > + *
> 
> Very good! This wording is much better already.
> 
> 
> Thanks,
> pq
> 
> 
> >   * HDR_OUTPUT_METADATA:
> >   *	Connector property to enable userspace to send HDR Metadata to
> >   *	driver. This metadata is based on the composition and blending
> > diff --git a/drivers/gpu/drm/drm_hdcp.c b/drivers/gpu/drm/drm_hdcp.c
> > index cd837bd409f7..ce235fd1c844 100644
> > --- a/drivers/gpu/drm/drm_hdcp.c
> > +++ b/drivers/gpu/drm/drm_hdcp.c
> > @@ -344,23 +344,41 @@ static struct drm_prop_enum_list drm_cp_enum_list[] = {
> >  };
> >  DRM_ENUM_NAME_FN(drm_get_content_protection_name, drm_cp_enum_list)
> >  
> > +static struct drm_prop_enum_list drm_hdcp_content_type_enum_list[] = {
> > +	{ DRM_MODE_HDCP_CONTENT_TYPE0, "HDCP Type0" },
> > +	{ DRM_MODE_HDCP_CONTENT_TYPE1, "HDCP Type1" },
> > +};
> > +DRM_ENUM_NAME_FN(drm_get_hdcp_content_type_name,
> > +		 drm_hdcp_content_type_enum_list)
> > +
> >  /**
> >   * drm_connector_attach_content_protection_property - attach content protection
> >   * property
> >   *
> >   * @connector: connector to attach CP property on.
> > + * @hdcp_content_type: is HDCP Content Type property needed for connector
> >   *
> >   * This is used to add support for content protection on select connectors.
> >   * Content Protection is intentionally vague to allow for different underlying
> >   * technologies, however it is most implemented by HDCP.
> >   *
> > + * When hdcp_content_type is true enum property called HDCP Content Type is
> > + * created (if it is not already) and attached to the connector.
> > + *
> > + * This property is used for sending the protected content's stream type
> > + * from userspace to kernel on selected connectors. Protected content provider
> > + * will decide their type of their content and declare the same to kernel.
> > + *
> > + * Content type will be used during the HDCP 2.2 authentication.
> > + * Content type will be set to &drm_connector_state.hdcp_content_type.
> > + *
> >   * The content protection will be set to &drm_connector_state.content_protection
> >   *
> >   * Returns:
> >   * Zero on success, negative errno on failure.
> >   */
> >  int drm_connector_attach_content_protection_property(
> > -		struct drm_connector *connector)
> > +		struct drm_connector *connector, bool hdcp_content_type)
> >  {
> >  	struct drm_device *dev = connector->dev;
> >  	struct drm_property *prop =
> > @@ -377,6 +395,22 @@ int drm_connector_attach_content_protection_property(
> >  				   DRM_MODE_CONTENT_PROTECTION_UNDESIRED);
> >  	dev->mode_config.content_protection_property = prop;
> >  
> > +	if (!hdcp_content_type)
> > +		return 0;
> > +
> > +	prop = dev->mode_config.hdcp_content_type_property;
> > +	if (!prop)
> > +		prop = drm_property_create_enum(dev, 0, "HDCP Content Type",
> > +					drm_hdcp_content_type_enum_list,
> > +					ARRAY_SIZE(
> > +					drm_hdcp_content_type_enum_list));
> > +	if (!prop)
> > +		return -ENOMEM;
> > +
> > +	drm_object_attach_property(&connector->base, prop,
> > +				   DRM_MODE_HDCP_CONTENT_TYPE0);
> > +	dev->mode_config.hdcp_content_type_property = prop;
> > +
> >  	return 0;
> >  }
> >  EXPORT_SYMBOL(drm_connector_attach_content_protection_property);
> > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
> > index bc3a94d491c4..2a4d10952b74 100644
> > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> > @@ -1829,7 +1829,9 @@ int intel_hdcp_init(struct intel_connector *connector,
> >  	if (!shim)
> >  		return -EINVAL;
> >  
> > -	ret = drm_connector_attach_content_protection_property(&connector->base);
> > +	ret =
> > +	drm_connector_attach_content_protection_property(&connector->base,
> > +							 false);
> >  	if (ret)
> >  		return ret;
> >  
> > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> > index 4c30d751487a..d6432967a2a9 100644
> > --- a/include/drm/drm_connector.h
> > +++ b/include/drm/drm_connector.h
> > @@ -601,6 +601,12 @@ struct drm_connector_state {
> >  	 */
> >  	unsigned int content_type;
> >  
> > +	/**
> > +	 * @hdcp_content_type: Connector property to pass the type of
> > +	 * protected content. This is most commonly used for HDCP.
> > +	 */
> > +	unsigned int hdcp_content_type;
> > +
> >  	/**
> >  	 * @scaling_mode: Connector property to control the
> >  	 * upscaling, mostly used for built-in panels.
> > @@ -1484,6 +1490,7 @@ const char *drm_get_dvi_i_select_name(int val);
> >  const char *drm_get_tv_subconnector_name(int val);
> >  const char *drm_get_tv_select_name(int val);
> >  const char *drm_get_content_protection_name(int val);
> > +const char *drm_get_hdcp_content_type_name(int val);
> >  
> >  int drm_mode_create_dvi_i_properties(struct drm_device *dev);
> >  int drm_mode_create_tv_margin_properties(struct drm_device *dev);
> > diff --git a/include/drm/drm_hdcp.h b/include/drm/drm_hdcp.h
> > index 13771a496e2b..2970abdfaf12 100644
> > --- a/include/drm/drm_hdcp.h
> > +++ b/include/drm/drm_hdcp.h
> > @@ -291,5 +291,5 @@ struct drm_connector;
> >  bool drm_hdcp_check_ksvs_revoked(struct drm_device *dev,
> >  				 u8 *ksvs, u32 ksv_count);
> >  int drm_connector_attach_content_protection_property(
> > -		struct drm_connector *connector);
> > +		struct drm_connector *connector, bool hdcp_content_type);
> >  #endif
> > diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> > index 759d462d028b..6c4b5bc85951 100644
> > --- a/include/drm/drm_mode_config.h
> > +++ b/include/drm/drm_mode_config.h
> > @@ -849,6 +849,12 @@ struct drm_mode_config {
> >  	 */
> >  	struct drm_property *content_protection_property;
> >  
> > +	/**
> > +	 * @hdcp_content_type_property: DRM ENUM property for type of
> > +	 * Protected Content.
> > +	 */
> > +	struct drm_property *hdcp_content_type_property;
> > +
> >  	/* dumb ioctl parameters */
> >  	uint32_t preferred_depth, prefer_shadow;
> >  
> > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> > index 5ab331e5dc23..5c954394093f 100644
> > --- a/include/uapi/drm/drm_mode.h
> > +++ b/include/uapi/drm/drm_mode.h
> > @@ -218,6 +218,10 @@ extern "C" {
> >  #define DRM_MODE_CONTENT_PROTECTION_DESIRED     1
> >  #define DRM_MODE_CONTENT_PROTECTION_ENABLED     2
> >  
> > +/* Content Type classification for HDCP2.2 vs others */
> > +#define DRM_MODE_HDCP_CONTENT_TYPE0		0
> > +#define DRM_MODE_HDCP_CONTENT_TYPE1		1
> > +
> >  struct drm_mode_modeinfo {
> >  	__u32 clock;
> >  	__u16 hdisplay;
> 


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

Re: [PATCH v9 1/6] drm: Add Content protection type property

[Pekka Paalanen]

[-- Attachment #1.1: Type: text/plain, Size: 12849 bytes --]

On Mon,  8 Jul 2019 16:51:11 +0530
Ramalingam C <ramalingam.c@intel.com> wrote:

> This patch adds a DRM ENUM property to the selected connectors.
> This property is used for mentioning the protected content's type
> from userspace to kernel HDCP authentication.
> 
> Type of the stream is decided by the protected content providers.
> Type 0 content can be rendered on any HDCP protected display wires.
> But Type 1 content can be rendered only on HDCP2.2 protected paths.
> 
> So when a userspace sets this property to Type 1 and starts the HDCP
> enable, kernel will honour it only if HDCP2.2 authentication is through
> for type 1. Else HDCP enable will be failed.
> 
> Need ACK for this new conenctor property from userspace consumer.
> 
> v2:
>   cp_content_type is replaced with content_protection_type [daniel]
>   check at atomic_set_property is removed [Maarten]
> v3:
>   %s/content_protection_type/hdcp_content_type [Pekka]
> v4:
>   property is created for the first requested connector and then reused.
> 	[Danvet]
> v5:
>   kernel doc nits addressed [Daniel]
>   Rebased as part of patch reordering.
> v6:
>   Kernel docs are modified [pekka]
> v7:
>   More details in Kernel docs. [pekka]
> 
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/drm_atomic_uapi.c         |  4 +++
>  drivers/gpu/drm/drm_connector.c           | 39 +++++++++++++++++++++++
>  drivers/gpu/drm/drm_hdcp.c                | 36 ++++++++++++++++++++-
>  drivers/gpu/drm/i915/display/intel_hdcp.c |  4 ++-
>  include/drm/drm_connector.h               |  7 ++++
>  include/drm/drm_hdcp.h                    |  2 +-
>  include/drm/drm_mode_config.h             |  6 ++++
>  include/uapi/drm/drm_mode.h               |  4 +++
>  8 files changed, 99 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index abe38bdf85ae..19ae119f1a5d 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -747,6 +747,8 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
>  			return -EINVAL;
>  		}
>  		state->content_protection = val;
> +	} else if (property == config->hdcp_content_type_property) {
> +		state->hdcp_content_type = val;
>  	} else if (property == connector->colorspace_property) {
>  		state->colorspace = val;
>  	} else if (property == config->writeback_fb_id_property) {
> @@ -831,6 +833,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
>  			state->hdr_output_metadata->base.id : 0;
>  	} else if (property == config->content_protection_property) {
>  		*val = state->content_protection;
> +	} else if (property == config->hdcp_content_type_property) {
> +		*val = state->hdcp_content_type;
>  	} else if (property == config->writeback_fb_id_property) {
>  		/* Writeback framebuffer is one-shot, write and forget */
>  		*val = 0;
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 068d4b05f1be..732f6645643d 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -952,6 +952,45 @@ static const struct drm_prop_enum_list hdmi_colorspaces[] = {
>   *	  is no longer protected and userspace should take appropriate action
>   *	  (whatever that might be).
>   *
> + * HDCP Content Type:
> + *	This Enum property is used by the userspace to declare the content type
> + *	of the display stream, to kernel. Here display stream stands for any
> + *	display content that userspace intended to render with HDCP encryption.

Hi,

I'd suggest s/render with/display through/.

As a gfx dev, rendering is something quite different to me.

> + *
> + *	Content Type of a stream is decided by the owner of the stream, as
> + *	"HDCP Type0" or "HDCP Type1".
> + *
> + *	The value of the property can be one the below:

*one of the below

> + *	  - "HDCP Type0": DRM_MODE_HDCP_CONTENT_TYPE0 = 0
> + *	  - "HDCP Type1": DRM_MODE_HDCP_CONTENT_TYPE1 = 1
> + *
> + *	When kernel starts the HDCP authentication upon the "DESIRED" state of
> + *	the "Content Protection", it refers the "HDCP Content Type" property
> + *	state. And perform the HDCP authentication with the display sink for
> + *	the content type mentioned by "HDCP Content Type".

How about:

	When kernel starts the HDCP authentication (see "Content Protection"
	for details), it uses the content type in "HDCP Content Type"
	for performing the HDCP authentication with the display sink.

> + *
> + *	Stream classified as HDCP Type0 can be transmitted on a link which is
> + *	encrypted with HDCP 1.4 or higher versions of HDCP(i.e HDCP2.2
> + *	and more).

This is where I get confused, see my earlier email from today on the
previous revision of this patch series. Is it necessary to talk about
HDCP versions here? The only thing that matters for UAPI is the content
type, right?

Previously you said that the kernel will not use Type1 if userspace
only asked for Type0, but to me this text reads as quite the opposite.

> + *
> + *	Streams classified as HDCP Type1 can be transmitted on a link which is
> + *	encrypted only with HDCP 2.2. In future, HDCP versions >2.2 also might
> + *	support Type1 based on their spec.
> + *
> + *	HDCP2.2 authentication protocol itself takes the "Content Type" as a
> + *	parameter, which is a input for the DP HDCP2.2 encryption algo.
> + *
> + *	Note that the HDCP Content Type property is introduced at HDCP 2.2, and
> + *	defaults to type 0. It is only exposed by drivers supporting HDCP 2.2.
> + *	Based on how next versions of HDCP specs are defined content Type could
> + *	be used for higher versions too.

Ok, userspace does not have to cope with a "HDCP Content Type" property
that is missing the enum value Type1. I think the Weston patch would
attempt something silly or misbehave if Type1 value was ever missing.
Not having the whole property is fine, of course.

> + *
> + *	If content type is changed when "Content Protection" is not UNDESIRED,
> + *	then kernel will disable the HDCP and re-enable with new type in the
> + *	same atomic commit. And when "Content Protection" is ENABLED, it means
> + *	that link is HDCP authenticated and encrypted, for the transmission of
> + *	the Type of stream mentioned at "HDCP Content Type".
> + *

Very good! This wording is much better already.


Thanks,
pq


>   * HDR_OUTPUT_METADATA:
>   *	Connector property to enable userspace to send HDR Metadata to
>   *	driver. This metadata is based on the composition and blending
> diff --git a/drivers/gpu/drm/drm_hdcp.c b/drivers/gpu/drm/drm_hdcp.c
> index cd837bd409f7..ce235fd1c844 100644
> --- a/drivers/gpu/drm/drm_hdcp.c
> +++ b/drivers/gpu/drm/drm_hdcp.c
> @@ -344,23 +344,41 @@ static struct drm_prop_enum_list drm_cp_enum_list[] = {
>  };
>  DRM_ENUM_NAME_FN(drm_get_content_protection_name, drm_cp_enum_list)
>  
> +static struct drm_prop_enum_list drm_hdcp_content_type_enum_list[] = {
> +	{ DRM_MODE_HDCP_CONTENT_TYPE0, "HDCP Type0" },
> +	{ DRM_MODE_HDCP_CONTENT_TYPE1, "HDCP Type1" },
> +};
> +DRM_ENUM_NAME_FN(drm_get_hdcp_content_type_name,
> +		 drm_hdcp_content_type_enum_list)
> +
>  /**
>   * drm_connector_attach_content_protection_property - attach content protection
>   * property
>   *
>   * @connector: connector to attach CP property on.
> + * @hdcp_content_type: is HDCP Content Type property needed for connector
>   *
>   * This is used to add support for content protection on select connectors.
>   * Content Protection is intentionally vague to allow for different underlying
>   * technologies, however it is most implemented by HDCP.
>   *
> + * When hdcp_content_type is true enum property called HDCP Content Type is
> + * created (if it is not already) and attached to the connector.
> + *
> + * This property is used for sending the protected content's stream type
> + * from userspace to kernel on selected connectors. Protected content provider
> + * will decide their type of their content and declare the same to kernel.
> + *
> + * Content type will be used during the HDCP 2.2 authentication.
> + * Content type will be set to &drm_connector_state.hdcp_content_type.
> + *
>   * The content protection will be set to &drm_connector_state.content_protection
>   *
>   * Returns:
>   * Zero on success, negative errno on failure.
>   */
>  int drm_connector_attach_content_protection_property(
> -		struct drm_connector *connector)
> +		struct drm_connector *connector, bool hdcp_content_type)
>  {
>  	struct drm_device *dev = connector->dev;
>  	struct drm_property *prop =
> @@ -377,6 +395,22 @@ int drm_connector_attach_content_protection_property(
>  				   DRM_MODE_CONTENT_PROTECTION_UNDESIRED);
>  	dev->mode_config.content_protection_property = prop;
>  
> +	if (!hdcp_content_type)
> +		return 0;
> +
> +	prop = dev->mode_config.hdcp_content_type_property;
> +	if (!prop)
> +		prop = drm_property_create_enum(dev, 0, "HDCP Content Type",
> +					drm_hdcp_content_type_enum_list,
> +					ARRAY_SIZE(
> +					drm_hdcp_content_type_enum_list));
> +	if (!prop)
> +		return -ENOMEM;
> +
> +	drm_object_attach_property(&connector->base, prop,
> +				   DRM_MODE_HDCP_CONTENT_TYPE0);
> +	dev->mode_config.hdcp_content_type_property = prop;
> +
>  	return 0;
>  }
>  EXPORT_SYMBOL(drm_connector_attach_content_protection_property);
> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
> index bc3a94d491c4..2a4d10952b74 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> @@ -1829,7 +1829,9 @@ int intel_hdcp_init(struct intel_connector *connector,
>  	if (!shim)
>  		return -EINVAL;
>  
> -	ret = drm_connector_attach_content_protection_property(&connector->base);
> +	ret =
> +	drm_connector_attach_content_protection_property(&connector->base,
> +							 false);
>  	if (ret)
>  		return ret;
>  
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 4c30d751487a..d6432967a2a9 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -601,6 +601,12 @@ struct drm_connector_state {
>  	 */
>  	unsigned int content_type;
>  
> +	/**
> +	 * @hdcp_content_type: Connector property to pass the type of
> +	 * protected content. This is most commonly used for HDCP.
> +	 */
> +	unsigned int hdcp_content_type;
> +
>  	/**
>  	 * @scaling_mode: Connector property to control the
>  	 * upscaling, mostly used for built-in panels.
> @@ -1484,6 +1490,7 @@ const char *drm_get_dvi_i_select_name(int val);
>  const char *drm_get_tv_subconnector_name(int val);
>  const char *drm_get_tv_select_name(int val);
>  const char *drm_get_content_protection_name(int val);
> +const char *drm_get_hdcp_content_type_name(int val);
>  
>  int drm_mode_create_dvi_i_properties(struct drm_device *dev);
>  int drm_mode_create_tv_margin_properties(struct drm_device *dev);
> diff --git a/include/drm/drm_hdcp.h b/include/drm/drm_hdcp.h
> index 13771a496e2b..2970abdfaf12 100644
> --- a/include/drm/drm_hdcp.h
> +++ b/include/drm/drm_hdcp.h
> @@ -291,5 +291,5 @@ struct drm_connector;
>  bool drm_hdcp_check_ksvs_revoked(struct drm_device *dev,
>  				 u8 *ksvs, u32 ksv_count);
>  int drm_connector_attach_content_protection_property(
> -		struct drm_connector *connector);
> +		struct drm_connector *connector, bool hdcp_content_type);
>  #endif
> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index 759d462d028b..6c4b5bc85951 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -849,6 +849,12 @@ struct drm_mode_config {
>  	 */
>  	struct drm_property *content_protection_property;
>  
> +	/**
> +	 * @hdcp_content_type_property: DRM ENUM property for type of
> +	 * Protected Content.
> +	 */
> +	struct drm_property *hdcp_content_type_property;
> +
>  	/* dumb ioctl parameters */
>  	uint32_t preferred_depth, prefer_shadow;
>  
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 5ab331e5dc23..5c954394093f 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -218,6 +218,10 @@ extern "C" {
>  #define DRM_MODE_CONTENT_PROTECTION_DESIRED     1
>  #define DRM_MODE_CONTENT_PROTECTION_ENABLED     2
>  
> +/* Content Type classification for HDCP2.2 vs others */
> +#define DRM_MODE_HDCP_CONTENT_TYPE0		0
> +#define DRM_MODE_HDCP_CONTENT_TYPE1		1
> +
>  struct drm_mode_modeinfo {
>  	__u32 clock;
>  	__u16 hdisplay;


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH v9 4/6] drm/hdcp: update content protection property with uevent

[Pekka Paalanen]

[-- Attachment #1.1: Type: text/plain, Size: 5473 bytes --]

On Mon,  8 Jul 2019 16:51:14 +0530
Ramalingam C <ramalingam.c@intel.com> wrote:

> drm function is defined and exported to update a connector's
> content protection property state and to generate a uevent along
> with it.
> 
> Need ACK for the uevent from userspace consumer.
> 
> v2:
>   Update only when state is different from old one.
> v3:
>   KDoc is added [Daniel]
> v4:
>   KDoc is extended bit more [pekka]
> v5:
>   Uevent usage is documented at kdoc of "Content Protection" also
>   [pekka]
> 
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/drm_connector.c | 17 +++++++++++++----
>  drivers/gpu/drm/drm_hdcp.c      | 34 +++++++++++++++++++++++++++++++++
>  include/drm/drm_hdcp.h          |  2 ++
>  3 files changed, 49 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 732f6645643d..6de906ef10b3 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -947,10 +947,19 @@ static const struct drm_prop_enum_list hdmi_colorspaces[] = {
>   *	- If the state is DESIRED, kernel should attempt to re-authenticate the
>   *	  link whenever possible. This includes across disable/enable, dpms,
>   *	  hotplug, downstream device changes, link status failures, etc..
> - *	- Userspace is responsible for polling the property to determine when
> - *	  the value transitions from ENABLED to DESIRED. This signifies the link
> - *	  is no longer protected and userspace should take appropriate action
> - *	  (whatever that might be).
> + *	- Kernel sends uevent with the connector id and property id through
> + *	  @drm_hdcp_update_content_protection, upon below kernel triggered
> + *	  scenarios:
> + *		DESIRED -> ENABLED	(authentication success)
> + *		ENABLED -> DESIRED	(termination of authentication)
> + *	- Please note no uevents for userspace triggered property state changes,
> + *	  which can't fail such as
> + *		DESIRED/ENABLED -> UNDESIRED
> + *		UNDESIRED -> DESIRED
> + *	- Userspace is responsible for polling the property or listen to uevents
> + *	  to determine when the value transitions from ENABLED to DESIRED.
> + *	  This signifies the link is no longer protected and userspace should
> + *	  take appropriate action (whatever that might be).

Yes!

This doc is exactly what I hoped to see. Good job.

This is also exactly how
https://gitlab.freedesktop.org/wayland/weston/merge_requests/48 deals
with this in userspace. That MR still has some open issues, but I think
nothing related to the uevent.


Thanks,
pq

>   *
>   * HDCP Content Type:
>   *	This Enum property is used by the userspace to declare the content type
> diff --git a/drivers/gpu/drm/drm_hdcp.c b/drivers/gpu/drm/drm_hdcp.c
> index ce235fd1c844..77433ee3d652 100644
> --- a/drivers/gpu/drm/drm_hdcp.c
> +++ b/drivers/gpu/drm/drm_hdcp.c
> @@ -374,6 +374,10 @@ DRM_ENUM_NAME_FN(drm_get_hdcp_content_type_name,
>   *
>   * The content protection will be set to &drm_connector_state.content_protection
>   *
> + * When kernel triggered content protection state change like DESIRED->ENABLED
> + * and ENABLED->DESIRED, will use drm_hdcp_update_content_protection() to update
> + * the content protection state of a connector.
> + *
>   * Returns:
>   * Zero on success, negative errno on failure.
>   */
> @@ -414,3 +418,33 @@ int drm_connector_attach_content_protection_property(
>  	return 0;
>  }
>  EXPORT_SYMBOL(drm_connector_attach_content_protection_property);
> +
> +/**
> + * drm_hdcp_update_content_protection - Updates the content protection state
> + * of a connector
> + *
> + * @connector: drm_connector on which content protection state needs an update
> + * @val: New state of the content protection property
> + *
> + * This function can be used by display drivers, to update the kernel triggered
> + * content protection state changes of a drm_connector such as DESIRED->ENABLED
> + * and ENABLED->DESIRED. No uevent for DESIRED->UNDESIRED or ENABLED->UNDESIRED,
> + * as userspace is triggering such state change and kernel performs it without
> + * fail.This function update the new state of the property into the connector's
> + * state and generate an uevent to notify the userspace.
> + */
> +void drm_hdcp_update_content_protection(struct drm_connector *connector,
> +					u64 val)
> +{
> +	struct drm_device *dev = connector->dev;
> +	struct drm_connector_state *state = connector->state;
> +
> +	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
> +	if (state->content_protection == val)
> +		return;
> +
> +	state->content_protection = val;
> +	drm_sysfs_connector_status_event(connector,
> +				 dev->mode_config.content_protection_property);
> +}
> +EXPORT_SYMBOL(drm_hdcp_update_content_protection);
> diff --git a/include/drm/drm_hdcp.h b/include/drm/drm_hdcp.h
> index 2970abdfaf12..dd864ac9ce85 100644
> --- a/include/drm/drm_hdcp.h
> +++ b/include/drm/drm_hdcp.h
> @@ -292,4 +292,6 @@ bool drm_hdcp_check_ksvs_revoked(struct drm_device *dev,
>  				 u8 *ksvs, u32 ksv_count);
>  int drm_connector_attach_content_protection_property(
>  		struct drm_connector *connector, bool hdcp_content_type);
> +void drm_hdcp_update_content_protection(struct drm_connector *connector,
> +					u64 val);
>  #endif


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH v9 1/6] drm: Add Content protection type property

[Pekka Paalanen]

[-- Attachment #1.1: Type: text/plain, Size: 8621 bytes --]

On Tue, 9 Jul 2019 18:17:59 +0530
Ramalingam C <ramalingam.c@intel.com> wrote:

> On 2019-07-09 at 17:31:10 +0300, Pekka Paalanen wrote:
> > On Mon,  8 Jul 2019 16:51:11 +0530
> > Ramalingam C <ramalingam.c@intel.com> wrote:
> >   
> > > This patch adds a DRM ENUM property to the selected connectors.
> > > This property is used for mentioning the protected content's type
> > > from userspace to kernel HDCP authentication.
> > > 
> > > Type of the stream is decided by the protected content providers.
> > > Type 0 content can be rendered on any HDCP protected display wires.
> > > But Type 1 content can be rendered only on HDCP2.2 protected paths.
> > > 
> > > So when a userspace sets this property to Type 1 and starts the HDCP
> > > enable, kernel will honour it only if HDCP2.2 authentication is through
> > > for type 1. Else HDCP enable will be failed.
> > > 
> > > Need ACK for this new conenctor property from userspace consumer.
> > > 
> > > v2:
> > >   cp_content_type is replaced with content_protection_type [daniel]
> > >   check at atomic_set_property is removed [Maarten]
> > > v3:
> > >   %s/content_protection_type/hdcp_content_type [Pekka]
> > > v4:
> > >   property is created for the first requested connector and then reused.
> > > 	[Danvet]
> > > v5:
> > >   kernel doc nits addressed [Daniel]
> > >   Rebased as part of patch reordering.
> > > v6:
> > >   Kernel docs are modified [pekka]
> > > v7:
> > >   More details in Kernel docs. [pekka]
> > > 
> > > Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> > > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > ---
> > >  drivers/gpu/drm/drm_atomic_uapi.c         |  4 +++
> > >  drivers/gpu/drm/drm_connector.c           | 39 +++++++++++++++++++++++
> > >  drivers/gpu/drm/drm_hdcp.c                | 36 ++++++++++++++++++++-
> > >  drivers/gpu/drm/i915/display/intel_hdcp.c |  4 ++-
> > >  include/drm/drm_connector.h               |  7 ++++
> > >  include/drm/drm_hdcp.h                    |  2 +-
> > >  include/drm/drm_mode_config.h             |  6 ++++
> > >  include/uapi/drm/drm_mode.h               |  4 +++
> > >  8 files changed, 99 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> > > index abe38bdf85ae..19ae119f1a5d 100644
> > > --- a/drivers/gpu/drm/drm_atomic_uapi.c
> > > +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> > > @@ -747,6 +747,8 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
> > >  			return -EINVAL;
> > >  		}
> > >  		state->content_protection = val;
> > > +	} else if (property == config->hdcp_content_type_property) {
> > > +		state->hdcp_content_type = val;
> > >  	} else if (property == connector->colorspace_property) {
> > >  		state->colorspace = val;
> > >  	} else if (property == config->writeback_fb_id_property) {
> > > @@ -831,6 +833,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
> > >  			state->hdr_output_metadata->base.id : 0;
> > >  	} else if (property == config->content_protection_property) {
> > >  		*val = state->content_protection;
> > > +	} else if (property == config->hdcp_content_type_property) {
> > > +		*val = state->hdcp_content_type;
> > >  	} else if (property == config->writeback_fb_id_property) {
> > >  		/* Writeback framebuffer is one-shot, write and forget */
> > >  		*val = 0;
> > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> > > index 068d4b05f1be..732f6645643d 100644
> > > --- a/drivers/gpu/drm/drm_connector.c
> > > +++ b/drivers/gpu/drm/drm_connector.c
> > > @@ -952,6 +952,45 @@ static const struct drm_prop_enum_list hdmi_colorspaces[] = {
> > >   *	  is no longer protected and userspace should take appropriate action
> > >   *	  (whatever that might be).
> > >   *
> > > + * HDCP Content Type:
> > > + *	This Enum property is used by the userspace to declare the content type
> > > + *	of the display stream, to kernel. Here display stream stands for any
> > > + *	display content that userspace intended to render with HDCP encryption.  
> > 
> > Hi,
> > 
> > I'd suggest s/render with/display through/.
> > 
> > As a gfx dev, rendering is something quite different to me.  
> Ok.
> >   
> > > + *
> > > + *	Content Type of a stream is decided by the owner of the stream, as
> > > + *	"HDCP Type0" or "HDCP Type1".
> > > + *
> > > + *	The value of the property can be one the below:  
> > 
> > *one of the below  
> Sure.
> >   
> > > + *	  - "HDCP Type0": DRM_MODE_HDCP_CONTENT_TYPE0 = 0
> > > + *	  - "HDCP Type1": DRM_MODE_HDCP_CONTENT_TYPE1 = 1
> > > + *
> > > + *	When kernel starts the HDCP authentication upon the "DESIRED" state of
> > > + *	the "Content Protection", it refers the "HDCP Content Type" property
> > > + *	state. And perform the HDCP authentication with the display sink for
> > > + *	the content type mentioned by "HDCP Content Type".  
> > 
> > How about:
> > 
> > 	When kernel starts the HDCP authentication (see "Content Protection"
> > 	for details), it uses the content type in "HDCP Content Type"
> > 	for performing the HDCP authentication with the display sink.  
> less confusing :) Thanks.
> >   
> > > + *
> > > + *	Stream classified as HDCP Type0 can be transmitted on a link which is
> > > + *	encrypted with HDCP 1.4 or higher versions of HDCP(i.e HDCP2.2
> > > + *	and more).  
> > 
> > This is where I get confused, see my earlier email from today on the
> > previous revision of this patch series. Is it necessary to talk about
> > HDCP versions here? The only thing that matters for UAPI is the content
> > type, right?
> > 
> > Previously you said that the kernel will not use Type1 if userspace
> > only asked for Type0, but to me this text reads as quite the opposite.  
> Simple. HDCP2.2 itself support both Type 0 and Type 1. where as HDCP1.4
> by default supports the Type 0 and doesn't support the Type 1.
> 
> I guess you are getting confused by assigning the type to the versions.

Hi,

yes, I am indeed.

Is the HDCP version ever exposed to userspace in any way?

If it is, then explaining how the types relate to the versions may well
be useful. But if userspace cannot even know what HDCP version is being
used or available, explaining it in the UAPI doc seems to just confuse
the reader.

If the reader is interested in HDCP versions, I suppose it is not too
hard to figure out how the types relate to versions on their own,
right? Just search for the definitions of the types in any spec that
defines them.

> >   
> > > + *
> > > + *	Streams classified as HDCP Type1 can be transmitted on a link which is
> > > + *	encrypted only with HDCP 2.2. In future, HDCP versions >2.2 also might
> > > + *	support Type1 based on their spec.
> > > + *
> > > + *	HDCP2.2 authentication protocol itself takes the "Content Type" as a
> > > + *	parameter, which is a input for the DP HDCP2.2 encryption algo.
> > > + *
> > > + *	Note that the HDCP Content Type property is introduced at HDCP 2.2, and
> > > + *	defaults to type 0. It is only exposed by drivers supporting HDCP 2.2.
> > > + *	Based on how next versions of HDCP specs are defined content Type could
> > > + *	be used for higher versions too.  
> > 
> > Ok, userspace does not have to cope with a "HDCP Content Type" property
> > that is missing the enum value Type1. I think the Weston patch would
> > attempt something silly or misbehave if Type1 value was ever missing.
> > Not having the whole property is fine, of course.  
>  If the Type1 is not supported then there is no need for the "HDCP
>  Content Type" property itself. Thats why when a driver has the support
>  for Type1( as of now HDCP2.2) only then "HDCP Content Type" will be
>  exposed. So weston is fine at its current state.

Yes. I'm just saying that if the kernel ever exposes "HDCP Content
Type" property without the Type1 value (because it may have e.g. Type3
value from the future but does not support Type1), then it will break
userspace.

Adding new enum values is not a problem for the Weston code I have been
reviewing. It simply won't use values it doesn't know, resetting
unknown values to some value it does know.

Setting the rules on both whether enum values can be removed and added
is important for reviewing the userspace, because those need to be
explicitly taken care of in userspace code.

(It's not actually that different from reviewing Wayland protocol
extensions.)


Thanks,
pq

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH v9 1/6] drm: Add Content protection type property

[Ramalingam C]

On 2019-07-10 at 11:16:24 +0300, Pekka Paalanen wrote:
> On Tue, 9 Jul 2019 18:17:59 +0530
> Ramalingam C <ramalingam.c@intel.com> wrote:
> 
> > On 2019-07-09 at 17:31:10 +0300, Pekka Paalanen wrote:
> > > On Mon,  8 Jul 2019 16:51:11 +0530
> > > Ramalingam C <ramalingam.c@intel.com> wrote:
> > >   
> > > > This patch adds a DRM ENUM property to the selected connectors.
> > > > This property is used for mentioning the protected content's type
> > > > from userspace to kernel HDCP authentication.
> > > > 
> > > > Type of the stream is decided by the protected content providers.
> > > > Type 0 content can be rendered on any HDCP protected display wires.
> > > > But Type 1 content can be rendered only on HDCP2.2 protected paths.
> > > > 
> > > > So when a userspace sets this property to Type 1 and starts the HDCP
> > > > enable, kernel will honour it only if HDCP2.2 authentication is through
> > > > for type 1. Else HDCP enable will be failed.
> > > > 
> > > > Need ACK for this new conenctor property from userspace consumer.
> > > > 
> > > > v2:
> > > >   cp_content_type is replaced with content_protection_type [daniel]
> > > >   check at atomic_set_property is removed [Maarten]
> > > > v3:
> > > >   %s/content_protection_type/hdcp_content_type [Pekka]
> > > > v4:
> > > >   property is created for the first requested connector and then reused.
> > > > 	[Danvet]
> > > > v5:
> > > >   kernel doc nits addressed [Daniel]
> > > >   Rebased as part of patch reordering.
> > > > v6:
> > > >   Kernel docs are modified [pekka]
> > > > v7:
> > > >   More details in Kernel docs. [pekka]
> > > > 
> > > > Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> > > > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > > ---
> > > >  drivers/gpu/drm/drm_atomic_uapi.c         |  4 +++
> > > >  drivers/gpu/drm/drm_connector.c           | 39 +++++++++++++++++++++++
> > > >  drivers/gpu/drm/drm_hdcp.c                | 36 ++++++++++++++++++++-
> > > >  drivers/gpu/drm/i915/display/intel_hdcp.c |  4 ++-
> > > >  include/drm/drm_connector.h               |  7 ++++
> > > >  include/drm/drm_hdcp.h                    |  2 +-
> > > >  include/drm/drm_mode_config.h             |  6 ++++
> > > >  include/uapi/drm/drm_mode.h               |  4 +++
> > > >  8 files changed, 99 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> > > > index abe38bdf85ae..19ae119f1a5d 100644
> > > > --- a/drivers/gpu/drm/drm_atomic_uapi.c
> > > > +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> > > > @@ -747,6 +747,8 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
> > > >  			return -EINVAL;
> > > >  		}
> > > >  		state->content_protection = val;
> > > > +	} else if (property == config->hdcp_content_type_property) {
> > > > +		state->hdcp_content_type = val;
> > > >  	} else if (property == connector->colorspace_property) {
> > > >  		state->colorspace = val;
> > > >  	} else if (property == config->writeback_fb_id_property) {
> > > > @@ -831,6 +833,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
> > > >  			state->hdr_output_metadata->base.id : 0;
> > > >  	} else if (property == config->content_protection_property) {
> > > >  		*val = state->content_protection;
> > > > +	} else if (property == config->hdcp_content_type_property) {
> > > > +		*val = state->hdcp_content_type;
> > > >  	} else if (property == config->writeback_fb_id_property) {
> > > >  		/* Writeback framebuffer is one-shot, write and forget */
> > > >  		*val = 0;
> > > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> > > > index 068d4b05f1be..732f6645643d 100644
> > > > --- a/drivers/gpu/drm/drm_connector.c
> > > > +++ b/drivers/gpu/drm/drm_connector.c
> > > > @@ -952,6 +952,45 @@ static const struct drm_prop_enum_list hdmi_colorspaces[] = {
> > > >   *	  is no longer protected and userspace should take appropriate action
> > > >   *	  (whatever that might be).
> > > >   *
> > > > + * HDCP Content Type:
> > > > + *	This Enum property is used by the userspace to declare the content type
> > > > + *	of the display stream, to kernel. Here display stream stands for any
> > > > + *	display content that userspace intended to render with HDCP encryption.  
> > > 
> > > Hi,
> > > 
> > > I'd suggest s/render with/display through/.
> > > 
> > > As a gfx dev, rendering is something quite different to me.  
> > Ok.
> > >   
> > > > + *
> > > > + *	Content Type of a stream is decided by the owner of the stream, as
> > > > + *	"HDCP Type0" or "HDCP Type1".
> > > > + *
> > > > + *	The value of the property can be one the below:  
> > > 
> > > *one of the below  
> > Sure.
> > >   
> > > > + *	  - "HDCP Type0": DRM_MODE_HDCP_CONTENT_TYPE0 = 0
> > > > + *	  - "HDCP Type1": DRM_MODE_HDCP_CONTENT_TYPE1 = 1
> > > > + *
> > > > + *	When kernel starts the HDCP authentication upon the "DESIRED" state of
> > > > + *	the "Content Protection", it refers the "HDCP Content Type" property
> > > > + *	state. And perform the HDCP authentication with the display sink for
> > > > + *	the content type mentioned by "HDCP Content Type".  
> > > 
> > > How about:
> > > 
> > > 	When kernel starts the HDCP authentication (see "Content Protection"
> > > 	for details), it uses the content type in "HDCP Content Type"
> > > 	for performing the HDCP authentication with the display sink.  
> > less confusing :) Thanks.
> > >   
> > > > + *
> > > > + *	Stream classified as HDCP Type0 can be transmitted on a link which is
> > > > + *	encrypted with HDCP 1.4 or higher versions of HDCP(i.e HDCP2.2
> > > > + *	and more).  
> > > 
> > > This is where I get confused, see my earlier email from today on the
> > > previous revision of this patch series. Is it necessary to talk about
> > > HDCP versions here? The only thing that matters for UAPI is the content
> > > type, right?
> > > 
> > > Previously you said that the kernel will not use Type1 if userspace
> > > only asked for Type0, but to me this text reads as quite the opposite.  
> > Simple. HDCP2.2 itself support both Type 0 and Type 1. where as HDCP1.4
> > by default supports the Type 0 and doesn't support the Type 1.
> > 
> > I guess you are getting confused by assigning the type to the versions.
> 
> Hi,
> 
> yes, I am indeed.
> 
> Is the HDCP version ever exposed to userspace in any way?
> 
> If it is, then explaining how the types relate to the versions may well
> be useful. But if userspace cannot even know what HDCP version is being
> used or available,
Hi,

Implicitly content type capability is exposing the HDCP version capability.
Say if HDCP Type 1 is supported we infer that underlying kernel support
HDCP2.2.

And content type is introduced to restrict few catagories of the content
with few HDCP versions' protection, IMHO it is better to capture the
relationship between Type and the underlying HDCP versions.

See if the below para will do this task with out any ambiguous.

 * HDCP Content Type:
 *      This Enum property is used by the userspace to declare the content type
 *      of the display stream, to kernel. Here display stream stands for any
 *      display content that userspace intended to display through HDCP
 *      encryption.
 *
 *      Content Type of a stream is decided by the owner of the stream, as
 *      "HDCP Type0" or "HDCP Type1".
 *
 *      The value of the property can be one of the below:
 *        - "HDCP Type0": DRM_MODE_HDCP_CONTENT_TYPE0 = 0
 *        - "HDCP Type1": DRM_MODE_HDCP_CONTENT_TYPE1 = 1
 *
 *      When kernel starts the HDCP authentication (see "Content Protection"
 *      for details), it uses the content type in "HDCP Content Type"
 *      for performing the HDCP authentication with the display sink.
 *
 *      Please note in HDCP spec versions, a link can be authenticated with
 *      HDCP 2.2 for Content Type 0/Content Type 1. Where as a link can be
 *      authenticated with HDCP1.4 only for Content Type 0(though it is implicit
 *      in nature. As there is no reference for Content Type in HDCP1.4).
 *
  *     HDCP2.2 authentication protocol itself takes the "Content Type" as a
 *      parameter, which is a input for the DP HDCP2.2 encryption algo.
 *
 *      In case of Type 0 content protection request, kernel driver can choose
 *      either of HDCP spec versions 1.4 and 2.2. When HDCP2.2 is used for
 *      "HDCP Type 0", a HDCP 2.2 capable repeater in the downstream can send
 *      that content to a HDCP 1.4 authenticated HDCP sink (Type0 link).
 *      But if the content is classified as "HDCP Type 1", above mentioned
 *      HDCP 2.2 repeater wont send the content to the HDCP sink as it can't
 *      authenticate the HDCP1.4 capable sink for "HDCP Type 1".
 *
 *      Please note userspace can be ignorant of the HDCP versions used by the
 *      kernel driver to achieve the "HDCP Content Type".
 *
 *      At current scenario, classifying a content as Type 1 ensures that the
 *      content will be displayed only through the HDCP2.2 encrypted link.
 *
 *      Note that the HDCP Content Type property is introduced at HDCP 2.2, and
 *      defaults to type 0. It is only exposed by drivers supporting HDCP 2.2
 *      (hence supporting Type 0 and Type 1). Based on how next versions of
 *      HDCP specs are defined content Type could be used for higher versions too.
 *
 *      If content type is changed when "Content Protection" is not UNDESIRED,
 *      then kernel will disable the HDCP and re-enable with new type in the
 *      same atomic commit. And when "Content Protection" is ENABLED, it means
 *      that link is HDCP authenticated and encrypted, for the transmission of
 *      the Type of stream mentioned at "HDCP Content Type".

Please share your view on this.

-Ram

> explaining it in the UAPI doc seems to just confuse
> the reader.
> 
> If the reader is interested in HDCP versions, I suppose it is not too
> hard to figure out how the types relate to versions on their own,
> right? Just search for the definitions of the types in any spec that
> defines them.
> 
> > >   
> > > > + *
> > > > + *	Streams classified as HDCP Type1 can be transmitted on a link which is
> > > > + *	encrypted only with HDCP 2.2. In future, HDCP versions >2.2 also might
> > > > + *	support Type1 based on their spec.
> > > > + *
> > > > + *	HDCP2.2 authentication protocol itself takes the "Content Type" as a
> > > > + *	parameter, which is a input for the DP HDCP2.2 encryption algo.
> > > > + *
> > > > + *	Note that the HDCP Content Type property is introduced at HDCP 2.2, and
> > > > + *	defaults to type 0. It is only exposed by drivers supporting HDCP 2.2.
> > > > + *	Based on how next versions of HDCP specs are defined content Type could
> > > > + *	be used for higher versions too.  
> > > 
> > > Ok, userspace does not have to cope with a "HDCP Content Type" property
> > > that is missing the enum value Type1. I think the Weston patch would
> > > attempt something silly or misbehave if Type1 value was ever missing.
> > > Not having the whole property is fine, of course.  
> >  If the Type1 is not supported then there is no need for the "HDCP
> >  Content Type" property itself. Thats why when a driver has the support
> >  for Type1( as of now HDCP2.2) only then "HDCP Content Type" will be
> >  exposed. So weston is fine at its current state.
> 
> Yes. I'm just saying that if the kernel ever exposes "HDCP Content
> Type" property without the Type1 value (because it may have e.g. Type3
> value from the future but does not support Type1), then it will break
> userspace.
> 
> Adding new enum values is not a problem for the Weston code I have been
> reviewing. It simply won't use values it doesn't know, resetting
> unknown values to some value it does know.
> 
> Setting the rules on both whether enum values can be removed and added
> is important for reviewing the userspace, because those need to be
> explicitly taken care of in userspace code.
> 
> (It's not actually that different from reviewing Wayland protocol
> extensions.)
> 
> 
> Thanks,
> pq


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

Re: [Intel-gfx] [PATCH v9 1/6] drm: Add Content protection type property

[Sean Paul]

On Mon, Jul 08, 2019 at 04:51:11PM +0530, Ramalingam C wrote:
> This patch adds a DRM ENUM property to the selected connectors.
> This property is used for mentioning the protected content's type
> from userspace to kernel HDCP authentication.
> 
> Type of the stream is decided by the protected content providers.
> Type 0 content can be rendered on any HDCP protected display wires.
> But Type 1 content can be rendered only on HDCP2.2 protected paths.
> 
> So when a userspace sets this property to Type 1 and starts the HDCP
> enable, kernel will honour it only if HDCP2.2 authentication is through
> for type 1. Else HDCP enable will be failed.
> 
> Need ACK for this new conenctor property from userspace consumer.
> 
> v2:
>   cp_content_type is replaced with content_protection_type [daniel]
>   check at atomic_set_property is removed [Maarten]
> v3:
>   %s/content_protection_type/hdcp_content_type [Pekka]
> v4:
>   property is created for the first requested connector and then reused.
> 	[Danvet]
> v5:
>   kernel doc nits addressed [Daniel]
>   Rebased as part of patch reordering.
> v6:
>   Kernel docs are modified [pekka]
> v7:
>   More details in Kernel docs. [pekka]
> 
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/drm_atomic_uapi.c         |  4 +++
>  drivers/gpu/drm/drm_connector.c           | 39 +++++++++++++++++++++++
>  drivers/gpu/drm/drm_hdcp.c                | 36 ++++++++++++++++++++-
>  drivers/gpu/drm/i915/display/intel_hdcp.c |  4 ++-
>  include/drm/drm_connector.h               |  7 ++++
>  include/drm/drm_hdcp.h                    |  2 +-
>  include/drm/drm_mode_config.h             |  6 ++++
>  include/uapi/drm/drm_mode.h               |  4 +++
>  8 files changed, 99 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index abe38bdf85ae..19ae119f1a5d 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -747,6 +747,8 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
>  			return -EINVAL;
>  		}
>  		state->content_protection = val;
> +	} else if (property == config->hdcp_content_type_property) {
> +		state->hdcp_content_type = val;
>  	} else if (property == connector->colorspace_property) {
>  		state->colorspace = val;
>  	} else if (property == config->writeback_fb_id_property) {
> @@ -831,6 +833,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
>  			state->hdr_output_metadata->base.id : 0;
>  	} else if (property == config->content_protection_property) {
>  		*val = state->content_protection;
> +	} else if (property == config->hdcp_content_type_property) {
> +		*val = state->hdcp_content_type;
>  	} else if (property == config->writeback_fb_id_property) {
>  		/* Writeback framebuffer is one-shot, write and forget */
>  		*val = 0;
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 068d4b05f1be..732f6645643d 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -952,6 +952,45 @@ static const struct drm_prop_enum_list hdmi_colorspaces[] = {
>   *	  is no longer protected and userspace should take appropriate action
>   *	  (whatever that might be).
>   *
> + * HDCP Content Type:
> + *	This Enum property is used by the userspace to declare the content type
> + *	of the display stream, to kernel. Here display stream stands for any
> + *	display content that userspace intended to render with HDCP encryption.
> + *
> + *	Content Type of a stream is decided by the owner of the stream, as
> + *	"HDCP Type0" or "HDCP Type1".
> + *
> + *	The value of the property can be one the below:
> + *	  - "HDCP Type0": DRM_MODE_HDCP_CONTENT_TYPE0 = 0
> + *	  - "HDCP Type1": DRM_MODE_HDCP_CONTENT_TYPE1 = 1
> + *
> + *	When kernel starts the HDCP authentication upon the "DESIRED" state of
> + *	the "Content Protection", it refers the "HDCP Content Type" property
> + *	state. And perform the HDCP authentication with the display sink for
> + *	the content type mentioned by "HDCP Content Type".
> + *
> + *	Stream classified as HDCP Type0 can be transmitted on a link which is
> + *	encrypted with HDCP 1.4 or higher versions of HDCP(i.e HDCP2.2
> + *	and more).
> + *
> + *	Streams classified as HDCP Type1 can be transmitted on a link which is
> + *	encrypted only with HDCP 2.2. In future, HDCP versions >2.2 also might
> + *	support Type1 based on their spec.
> + *
> + *	HDCP2.2 authentication protocol itself takes the "Content Type" as a
> + *	parameter, which is a input for the DP HDCP2.2 encryption algo.
> + *
> + *	Note that the HDCP Content Type property is introduced at HDCP 2.2, and
> + *	defaults to type 0. It is only exposed by drivers supporting HDCP 2.2.
> + *	Based on how next versions of HDCP specs are defined content Type could
> + *	be used for higher versions too.
> + *
> + *	If content type is changed when "Content Protection" is not UNDESIRED,
> + *	then kernel will disable the HDCP and re-enable with new type in the
> + *	same atomic commit. And when "Content Protection" is ENABLED, it means
> + *	that link is HDCP authenticated and encrypted, for the transmission of
> + *	the Type of stream mentioned at "HDCP Content Type".
> + *
>   * HDR_OUTPUT_METADATA:
>   *	Connector property to enable userspace to send HDR Metadata to
>   *	driver. This metadata is based on the composition and blending

Do we actually need an entirely new property? Can't we just add a new
entry to the existing Content Protection property which is "Desired Type1" or
similar? The kernel will then either attempt 2.2 auth or it will ignore it the
request if it's not supported.

Sean

> diff --git a/drivers/gpu/drm/drm_hdcp.c b/drivers/gpu/drm/drm_hdcp.c
> index cd837bd409f7..ce235fd1c844 100644
> --- a/drivers/gpu/drm/drm_hdcp.c
> +++ b/drivers/gpu/drm/drm_hdcp.c
> @@ -344,23 +344,41 @@ static struct drm_prop_enum_list drm_cp_enum_list[] = {
>  };
>  DRM_ENUM_NAME_FN(drm_get_content_protection_name, drm_cp_enum_list)
>  
> +static struct drm_prop_enum_list drm_hdcp_content_type_enum_list[] = {
> +	{ DRM_MODE_HDCP_CONTENT_TYPE0, "HDCP Type0" },
> +	{ DRM_MODE_HDCP_CONTENT_TYPE1, "HDCP Type1" },
> +};
> +DRM_ENUM_NAME_FN(drm_get_hdcp_content_type_name,
> +		 drm_hdcp_content_type_enum_list)
> +
>  /**
>   * drm_connector_attach_content_protection_property - attach content protection
>   * property
>   *
>   * @connector: connector to attach CP property on.
> + * @hdcp_content_type: is HDCP Content Type property needed for connector
>   *
>   * This is used to add support for content protection on select connectors.
>   * Content Protection is intentionally vague to allow for different underlying
>   * technologies, however it is most implemented by HDCP.
>   *
> + * When hdcp_content_type is true enum property called HDCP Content Type is
> + * created (if it is not already) and attached to the connector.
> + *
> + * This property is used for sending the protected content's stream type
> + * from userspace to kernel on selected connectors. Protected content provider
> + * will decide their type of their content and declare the same to kernel.
> + *
> + * Content type will be used during the HDCP 2.2 authentication.
> + * Content type will be set to &drm_connector_state.hdcp_content_type.
> + *
>   * The content protection will be set to &drm_connector_state.content_protection
>   *
>   * Returns:
>   * Zero on success, negative errno on failure.
>   */
>  int drm_connector_attach_content_protection_property(
> -		struct drm_connector *connector)
> +		struct drm_connector *connector, bool hdcp_content_type)
>  {
>  	struct drm_device *dev = connector->dev;
>  	struct drm_property *prop =
> @@ -377,6 +395,22 @@ int drm_connector_attach_content_protection_property(
>  				   DRM_MODE_CONTENT_PROTECTION_UNDESIRED);
>  	dev->mode_config.content_protection_property = prop;
>  
> +	if (!hdcp_content_type)
> +		return 0;
> +
> +	prop = dev->mode_config.hdcp_content_type_property;
> +	if (!prop)
> +		prop = drm_property_create_enum(dev, 0, "HDCP Content Type",
> +					drm_hdcp_content_type_enum_list,
> +					ARRAY_SIZE(
> +					drm_hdcp_content_type_enum_list));
> +	if (!prop)
> +		return -ENOMEM;
> +
> +	drm_object_attach_property(&connector->base, prop,
> +				   DRM_MODE_HDCP_CONTENT_TYPE0);
> +	dev->mode_config.hdcp_content_type_property = prop;
> +
>  	return 0;
>  }
>  EXPORT_SYMBOL(drm_connector_attach_content_protection_property);
> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
> index bc3a94d491c4..2a4d10952b74 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> @@ -1829,7 +1829,9 @@ int intel_hdcp_init(struct intel_connector *connector,
>  	if (!shim)
>  		return -EINVAL;
>  
> -	ret = drm_connector_attach_content_protection_property(&connector->base);
> +	ret =
> +	drm_connector_attach_content_protection_property(&connector->base,
> +							 false);
>  	if (ret)
>  		return ret;
>  
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 4c30d751487a..d6432967a2a9 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -601,6 +601,12 @@ struct drm_connector_state {
>  	 */
>  	unsigned int content_type;
>  
> +	/**
> +	 * @hdcp_content_type: Connector property to pass the type of
> +	 * protected content. This is most commonly used for HDCP.
> +	 */
> +	unsigned int hdcp_content_type;
> +
>  	/**
>  	 * @scaling_mode: Connector property to control the
>  	 * upscaling, mostly used for built-in panels.
> @@ -1484,6 +1490,7 @@ const char *drm_get_dvi_i_select_name(int val);
>  const char *drm_get_tv_subconnector_name(int val);
>  const char *drm_get_tv_select_name(int val);
>  const char *drm_get_content_protection_name(int val);
> +const char *drm_get_hdcp_content_type_name(int val);
>  
>  int drm_mode_create_dvi_i_properties(struct drm_device *dev);
>  int drm_mode_create_tv_margin_properties(struct drm_device *dev);
> diff --git a/include/drm/drm_hdcp.h b/include/drm/drm_hdcp.h
> index 13771a496e2b..2970abdfaf12 100644
> --- a/include/drm/drm_hdcp.h
> +++ b/include/drm/drm_hdcp.h
> @@ -291,5 +291,5 @@ struct drm_connector;
>  bool drm_hdcp_check_ksvs_revoked(struct drm_device *dev,
>  				 u8 *ksvs, u32 ksv_count);
>  int drm_connector_attach_content_protection_property(
> -		struct drm_connector *connector);
> +		struct drm_connector *connector, bool hdcp_content_type);
>  #endif
> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index 759d462d028b..6c4b5bc85951 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -849,6 +849,12 @@ struct drm_mode_config {
>  	 */
>  	struct drm_property *content_protection_property;
>  
> +	/**
> +	 * @hdcp_content_type_property: DRM ENUM property for type of
> +	 * Protected Content.
> +	 */
> +	struct drm_property *hdcp_content_type_property;
> +
>  	/* dumb ioctl parameters */
>  	uint32_t preferred_depth, prefer_shadow;
>  
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 5ab331e5dc23..5c954394093f 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -218,6 +218,10 @@ extern "C" {
>  #define DRM_MODE_CONTENT_PROTECTION_DESIRED     1
>  #define DRM_MODE_CONTENT_PROTECTION_ENABLED     2
>  
> +/* Content Type classification for HDCP2.2 vs others */
> +#define DRM_MODE_HDCP_CONTENT_TYPE0		0
> +#define DRM_MODE_HDCP_CONTENT_TYPE1		1
> +
>  struct drm_mode_modeinfo {
>  	__u32 clock;
>  	__u16 hdisplay;
> -- 
> 2.19.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH v9 3/6] drm: uevent for connector status change

[Sean Paul]

On Mon, Jul 08, 2019 at 04:51:13PM +0530, Ramalingam C wrote:
> DRM API for generating uevent for a status changes of connector's
> property.
> 
> This uevent will have following details related to the status change:
> 
>   HOTPLUG=1, CONNECTOR=<connector_id> and PROPERTY=<property_id>
> 
> Need ACK from this uevent from userspace consumer.
> 
> v2:
>   Minor fixes at KDoc comments [Daniel]
> v3:
>   Check the property is really attached with connector [Daniel]
> 
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/drm_sysfs.c | 35 +++++++++++++++++++++++++++++++++++
>  include/drm/drm_sysfs.h     |  5 ++++-
>  2 files changed, 39 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> index ad10810bc972..d13a77057045 100644
> --- a/drivers/gpu/drm/drm_sysfs.c
> +++ b/drivers/gpu/drm/drm_sysfs.c
> @@ -26,6 +26,7 @@
>  #include <drm/drm_sysfs.h>
>  
>  #include "drm_internal.h"
> +#include "drm_crtc_internal.h"
>  
>  #define to_drm_minor(d) dev_get_drvdata(d)
>  #define to_drm_connector(d) dev_get_drvdata(d)
> @@ -325,6 +326,9 @@ void drm_sysfs_lease_event(struct drm_device *dev)
>   * Send a uevent for the DRM device specified by @dev.  Currently we only
>   * set HOTPLUG=1 in the uevent environment, but this could be expanded to
>   * deal with other types of events.
> + *
> + * Any new uapi should be using the drm_sysfs_connector_status_event()
> + * for uevents on connector status change.
>   */
>  void drm_sysfs_hotplug_event(struct drm_device *dev)
>  {
> @@ -337,6 +341,37 @@ void drm_sysfs_hotplug_event(struct drm_device *dev)
>  }
>  EXPORT_SYMBOL(drm_sysfs_hotplug_event);
>  
> +/**
> + * drm_sysfs_connector_status_event - generate a DRM uevent for connector
> + * property status change
> + * @connector: connector on which property status changed
> + * @property: connector property whoes status changed.

s/whoes/whose/

> + *
> + * Send a uevent for the DRM device specified by @dev.  Currently we
> + * set HOTPLUG=1 and connector id along with the attached property id
> + * related to the status change.
> + */
> +void drm_sysfs_connector_status_event(struct drm_connector *connector,
> +				      struct drm_property *property)
> +{
> +	struct drm_device *dev = connector->dev;
> +	char hotplug_str[] = "HOTPLUG=1", conn_id[30], prop_id[30];

30 chars is a bit aggressive. UINT_MAX will only take up 10 chars, so with
the NULL char, you should only need 21 for conn_id and 20 for prop_id.

> +	char *envp[4] = { hotplug_str, conn_id, prop_id, NULL };
> +
> +	WARN_ON(!drm_mode_obj_find_prop_id(&connector->base,
> +					   property->base.id));
> +
> +	snprintf(conn_id, ARRAY_SIZE(conn_id),
> +		 "CONNECTOR=%u", connector->base.id);
> +	snprintf(prop_id, ARRAY_SIZE(prop_id),
> +		 "PROPERTY=%u", property->base.id);
> +
> +	DRM_DEBUG("generating connector status event\n");
> +
> +	kobject_uevent_env(&dev->primary->kdev->kobj, KOBJ_CHANGE, envp);
> +}
> +EXPORT_SYMBOL(drm_sysfs_connector_status_event);

With nits addressed,

Reviewed-by: Sean Paul <sean@poorly.run>


> +
>  static void drm_sysfs_release(struct device *dev)
>  {
>  	kfree(dev);
> diff --git a/include/drm/drm_sysfs.h b/include/drm/drm_sysfs.h
> index 4f311e836cdc..d454ef617b2c 100644
> --- a/include/drm/drm_sysfs.h
> +++ b/include/drm/drm_sysfs.h
> @@ -4,10 +4,13 @@
>  
>  struct drm_device;
>  struct device;
> +struct drm_connector;
> +struct drm_property;
>  
>  int drm_class_device_register(struct device *dev);
>  void drm_class_device_unregister(struct device *dev);
>  
>  void drm_sysfs_hotplug_event(struct drm_device *dev);
> -
> +void drm_sysfs_connector_status_event(struct drm_connector *connector,
> +				      struct drm_property *property);
>  #endif
> -- 
> 2.19.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v9 1/6] drm: Add Content protection type property

[Ramalingam C]

On 2019-07-11 at 10:18:22 -0400, Sean Paul wrote:
> On Mon, Jul 08, 2019 at 04:51:11PM +0530, Ramalingam C wrote:
> > This patch adds a DRM ENUM property to the selected connectors.
> > This property is used for mentioning the protected content's type
> > from userspace to kernel HDCP authentication.
> > 
> > Type of the stream is decided by the protected content providers.
> > Type 0 content can be rendered on any HDCP protected display wires.
> > But Type 1 content can be rendered only on HDCP2.2 protected paths.
> > 
> > So when a userspace sets this property to Type 1 and starts the HDCP
> > enable, kernel will honour it only if HDCP2.2 authentication is through
> > for type 1. Else HDCP enable will be failed.
> > 
> > Need ACK for this new conenctor property from userspace consumer.
> > 
> > v2:
> >   cp_content_type is replaced with content_protection_type [daniel]
> >   check at atomic_set_property is removed [Maarten]
> > v3:
> >   %s/content_protection_type/hdcp_content_type [Pekka]
> > v4:
> >   property is created for the first requested connector and then reused.
> > 	[Danvet]
> > v5:
> >   kernel doc nits addressed [Daniel]
> >   Rebased as part of patch reordering.
> > v6:
> >   Kernel docs are modified [pekka]
> > v7:
> >   More details in Kernel docs. [pekka]
> > 
> > Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > ---
> >  drivers/gpu/drm/drm_atomic_uapi.c         |  4 +++
> >  drivers/gpu/drm/drm_connector.c           | 39 +++++++++++++++++++++++
> >  drivers/gpu/drm/drm_hdcp.c                | 36 ++++++++++++++++++++-
> >  drivers/gpu/drm/i915/display/intel_hdcp.c |  4 ++-
> >  include/drm/drm_connector.h               |  7 ++++
> >  include/drm/drm_hdcp.h                    |  2 +-
> >  include/drm/drm_mode_config.h             |  6 ++++
> >  include/uapi/drm/drm_mode.h               |  4 +++
> >  8 files changed, 99 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> > index abe38bdf85ae..19ae119f1a5d 100644
> > --- a/drivers/gpu/drm/drm_atomic_uapi.c
> > +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> > @@ -747,6 +747,8 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
> >  			return -EINVAL;
> >  		}
> >  		state->content_protection = val;
> > +	} else if (property == config->hdcp_content_type_property) {
> > +		state->hdcp_content_type = val;
> >  	} else if (property == connector->colorspace_property) {
> >  		state->colorspace = val;
> >  	} else if (property == config->writeback_fb_id_property) {
> > @@ -831,6 +833,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
> >  			state->hdr_output_metadata->base.id : 0;
> >  	} else if (property == config->content_protection_property) {
> >  		*val = state->content_protection;
> > +	} else if (property == config->hdcp_content_type_property) {
> > +		*val = state->hdcp_content_type;
> >  	} else if (property == config->writeback_fb_id_property) {
> >  		/* Writeback framebuffer is one-shot, write and forget */
> >  		*val = 0;
> > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> > index 068d4b05f1be..732f6645643d 100644
> > --- a/drivers/gpu/drm/drm_connector.c
> > +++ b/drivers/gpu/drm/drm_connector.c
> > @@ -952,6 +952,45 @@ static const struct drm_prop_enum_list hdmi_colorspaces[] = {
> >   *	  is no longer protected and userspace should take appropriate action
> >   *	  (whatever that might be).
> >   *
> > + * HDCP Content Type:
> > + *	This Enum property is used by the userspace to declare the content type
> > + *	of the display stream, to kernel. Here display stream stands for any
> > + *	display content that userspace intended to render with HDCP encryption.
> > + *
> > + *	Content Type of a stream is decided by the owner of the stream, as
> > + *	"HDCP Type0" or "HDCP Type1".
> > + *
> > + *	The value of the property can be one the below:
> > + *	  - "HDCP Type0": DRM_MODE_HDCP_CONTENT_TYPE0 = 0
> > + *	  - "HDCP Type1": DRM_MODE_HDCP_CONTENT_TYPE1 = 1
> > + *
> > + *	When kernel starts the HDCP authentication upon the "DESIRED" state of
> > + *	the "Content Protection", it refers the "HDCP Content Type" property
> > + *	state. And perform the HDCP authentication with the display sink for
> > + *	the content type mentioned by "HDCP Content Type".
> > + *
> > + *	Stream classified as HDCP Type0 can be transmitted on a link which is
> > + *	encrypted with HDCP 1.4 or higher versions of HDCP(i.e HDCP2.2
> > + *	and more).
> > + *
> > + *	Streams classified as HDCP Type1 can be transmitted on a link which is
> > + *	encrypted only with HDCP 2.2. In future, HDCP versions >2.2 also might
> > + *	support Type1 based on their spec.
> > + *
> > + *	HDCP2.2 authentication protocol itself takes the "Content Type" as a
> > + *	parameter, which is a input for the DP HDCP2.2 encryption algo.
> > + *
> > + *	Note that the HDCP Content Type property is introduced at HDCP 2.2, and
> > + *	defaults to type 0. It is only exposed by drivers supporting HDCP 2.2.
> > + *	Based on how next versions of HDCP specs are defined content Type could
> > + *	be used for higher versions too.
> > + *
> > + *	If content type is changed when "Content Protection" is not UNDESIRED,
> > + *	then kernel will disable the HDCP and re-enable with new type in the
> > + *	same atomic commit. And when "Content Protection" is ENABLED, it means
> > + *	that link is HDCP authenticated and encrypted, for the transmission of
> > + *	the Type of stream mentioned at "HDCP Content Type".
> > + *
> >   * HDR_OUTPUT_METADATA:
> >   *	Connector property to enable userspace to send HDR Metadata to
> >   *	driver. This metadata is based on the composition and blending
> 
> Do we actually need an entirely new property? Can't we just add a new
> entry to the existing Content Protection property which is "Desired Type1" or
> similar? The kernel will then either attempt 2.2 auth or it will ignore it the
> request if it's not supported.
Thanks Sean for the review.

As you have mentioned earlier, "Content Protection" property is supposed
to be generic for any encryption, not just HDCP.
And content type is specific to the HDCP implementation.

And also the way content type is defined and used at HDCP2.2 spec, we
could expect when you version of spec is added to fix/improve  2.2 new
content type will be added as specific to that version.

All these points make the separate HDCP related property as more preferred.
so that only HDCP implementing driver will attach the property to the
drm connector.

Thanks,
Ram.
> 
> Sean
> 
> > diff --git a/drivers/gpu/drm/drm_hdcp.c b/drivers/gpu/drm/drm_hdcp.c
> > index cd837bd409f7..ce235fd1c844 100644
> > --- a/drivers/gpu/drm/drm_hdcp.c
> > +++ b/drivers/gpu/drm/drm_hdcp.c
> > @@ -344,23 +344,41 @@ static struct drm_prop_enum_list drm_cp_enum_list[] = {
> >  };
> >  DRM_ENUM_NAME_FN(drm_get_content_protection_name, drm_cp_enum_list)
> >  
> > +static struct drm_prop_enum_list drm_hdcp_content_type_enum_list[] = {
> > +	{ DRM_MODE_HDCP_CONTENT_TYPE0, "HDCP Type0" },
> > +	{ DRM_MODE_HDCP_CONTENT_TYPE1, "HDCP Type1" },
> > +};
> > +DRM_ENUM_NAME_FN(drm_get_hdcp_content_type_name,
> > +		 drm_hdcp_content_type_enum_list)
> > +
> >  /**
> >   * drm_connector_attach_content_protection_property - attach content protection
> >   * property
> >   *
> >   * @connector: connector to attach CP property on.
> > + * @hdcp_content_type: is HDCP Content Type property needed for connector
> >   *
> >   * This is used to add support for content protection on select connectors.
> >   * Content Protection is intentionally vague to allow for different underlying
> >   * technologies, however it is most implemented by HDCP.
> >   *
> > + * When hdcp_content_type is true enum property called HDCP Content Type is
> > + * created (if it is not already) and attached to the connector.
> > + *
> > + * This property is used for sending the protected content's stream type
> > + * from userspace to kernel on selected connectors. Protected content provider
> > + * will decide their type of their content and declare the same to kernel.
> > + *
> > + * Content type will be used during the HDCP 2.2 authentication.
> > + * Content type will be set to &drm_connector_state.hdcp_content_type.
> > + *
> >   * The content protection will be set to &drm_connector_state.content_protection
> >   *
> >   * Returns:
> >   * Zero on success, negative errno on failure.
> >   */
> >  int drm_connector_attach_content_protection_property(
> > -		struct drm_connector *connector)
> > +		struct drm_connector *connector, bool hdcp_content_type)
> >  {
> >  	struct drm_device *dev = connector->dev;
> >  	struct drm_property *prop =
> > @@ -377,6 +395,22 @@ int drm_connector_attach_content_protection_property(
> >  				   DRM_MODE_CONTENT_PROTECTION_UNDESIRED);
> >  	dev->mode_config.content_protection_property = prop;
> >  
> > +	if (!hdcp_content_type)
> > +		return 0;
> > +
> > +	prop = dev->mode_config.hdcp_content_type_property;
> > +	if (!prop)
> > +		prop = drm_property_create_enum(dev, 0, "HDCP Content Type",
> > +					drm_hdcp_content_type_enum_list,
> > +					ARRAY_SIZE(
> > +					drm_hdcp_content_type_enum_list));
> > +	if (!prop)
> > +		return -ENOMEM;
> > +
> > +	drm_object_attach_property(&connector->base, prop,
> > +				   DRM_MODE_HDCP_CONTENT_TYPE0);
> > +	dev->mode_config.hdcp_content_type_property = prop;
> > +
> >  	return 0;
> >  }
> >  EXPORT_SYMBOL(drm_connector_attach_content_protection_property);
> > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
> > index bc3a94d491c4..2a4d10952b74 100644
> > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> > @@ -1829,7 +1829,9 @@ int intel_hdcp_init(struct intel_connector *connector,
> >  	if (!shim)
> >  		return -EINVAL;
> >  
> > -	ret = drm_connector_attach_content_protection_property(&connector->base);
> > +	ret =
> > +	drm_connector_attach_content_protection_property(&connector->base,
> > +							 false);
> >  	if (ret)
> >  		return ret;
> >  
> > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> > index 4c30d751487a..d6432967a2a9 100644
> > --- a/include/drm/drm_connector.h
> > +++ b/include/drm/drm_connector.h
> > @@ -601,6 +601,12 @@ struct drm_connector_state {
> >  	 */
> >  	unsigned int content_type;
> >  
> > +	/**
> > +	 * @hdcp_content_type: Connector property to pass the type of
> > +	 * protected content. This is most commonly used for HDCP.
> > +	 */
> > +	unsigned int hdcp_content_type;
> > +
> >  	/**
> >  	 * @scaling_mode: Connector property to control the
> >  	 * upscaling, mostly used for built-in panels.
> > @@ -1484,6 +1490,7 @@ const char *drm_get_dvi_i_select_name(int val);
> >  const char *drm_get_tv_subconnector_name(int val);
> >  const char *drm_get_tv_select_name(int val);
> >  const char *drm_get_content_protection_name(int val);
> > +const char *drm_get_hdcp_content_type_name(int val);
> >  
> >  int drm_mode_create_dvi_i_properties(struct drm_device *dev);
> >  int drm_mode_create_tv_margin_properties(struct drm_device *dev);
> > diff --git a/include/drm/drm_hdcp.h b/include/drm/drm_hdcp.h
> > index 13771a496e2b..2970abdfaf12 100644
> > --- a/include/drm/drm_hdcp.h
> > +++ b/include/drm/drm_hdcp.h
> > @@ -291,5 +291,5 @@ struct drm_connector;
> >  bool drm_hdcp_check_ksvs_revoked(struct drm_device *dev,
> >  				 u8 *ksvs, u32 ksv_count);
> >  int drm_connector_attach_content_protection_property(
> > -		struct drm_connector *connector);
> > +		struct drm_connector *connector, bool hdcp_content_type);
> >  #endif
> > diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> > index 759d462d028b..6c4b5bc85951 100644
> > --- a/include/drm/drm_mode_config.h
> > +++ b/include/drm/drm_mode_config.h
> > @@ -849,6 +849,12 @@ struct drm_mode_config {
> >  	 */
> >  	struct drm_property *content_protection_property;
> >  
> > +	/**
> > +	 * @hdcp_content_type_property: DRM ENUM property for type of
> > +	 * Protected Content.
> > +	 */
> > +	struct drm_property *hdcp_content_type_property;
> > +
> >  	/* dumb ioctl parameters */
> >  	uint32_t preferred_depth, prefer_shadow;
> >  
> > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> > index 5ab331e5dc23..5c954394093f 100644
> > --- a/include/uapi/drm/drm_mode.h
> > +++ b/include/uapi/drm/drm_mode.h
> > @@ -218,6 +218,10 @@ extern "C" {
> >  #define DRM_MODE_CONTENT_PROTECTION_DESIRED     1
> >  #define DRM_MODE_CONTENT_PROTECTION_ENABLED     2
> >  
> > +/* Content Type classification for HDCP2.2 vs others */
> > +#define DRM_MODE_HDCP_CONTENT_TYPE0		0
> > +#define DRM_MODE_HDCP_CONTENT_TYPE1		1
> > +
> >  struct drm_mode_modeinfo {
> >  	__u32 clock;
> >  	__u16 hdisplay;
> > -- 
> > 2.19.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v9 1/6] drm: Add Content protection type property

[Ramalingam C]

On 2019-07-12 at 14:11:05 +0300, Pekka Paalanen wrote:
> On Thu, 11 Jul 2019 11:20:49 +0530
> Ramalingam C <ramalingam.c@intel.com> wrote:
> 
> > On 2019-07-10 at 11:16:24 +0300, Pekka Paalanen wrote:
> > > On Tue, 9 Jul 2019 18:17:59 +0530
> > > Ramalingam C <ramalingam.c@intel.com> wrote:
> > >   
> > > > On 2019-07-09 at 17:31:10 +0300, Pekka Paalanen wrote:  
> > > > > On Mon,  8 Jul 2019 16:51:11 +0530
> > > > > Ramalingam C <ramalingam.c@intel.com> wrote:
> > > > >     
> > > > > > This patch adds a DRM ENUM property to the selected connectors.
> > > > > > This property is used for mentioning the protected content's type
> > > > > > from userspace to kernel HDCP authentication.
> > > > > > 
> > > > > > Type of the stream is decided by the protected content providers.
> > > > > > Type 0 content can be rendered on any HDCP protected display wires.
> > > > > > But Type 1 content can be rendered only on HDCP2.2 protected paths.
> > > > > > 
> > > > > > So when a userspace sets this property to Type 1 and starts the HDCP
> > > > > > enable, kernel will honour it only if HDCP2.2 authentication is through
> > > > > > for type 1. Else HDCP enable will be failed.
> > > > > > 
> > > > > > Need ACK for this new conenctor property from userspace consumer.
> > > > > > 
> > > > > > v2:
> > > > > >   cp_content_type is replaced with content_protection_type [daniel]
> > > > > >   check at atomic_set_property is removed [Maarten]
> > > > > > v3:
> > > > > >   %s/content_protection_type/hdcp_content_type [Pekka]
> > > > > > v4:
> > > > > >   property is created for the first requested connector and then reused.
> > > > > > 	[Danvet]
> > > > > > v5:
> > > > > >   kernel doc nits addressed [Daniel]
> > > > > >   Rebased as part of patch reordering.
> > > > > > v6:
> > > > > >   Kernel docs are modified [pekka]
> > > > > > v7:
> > > > > >   More details in Kernel docs. [pekka]
> > > > > > 
> > > > > > Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> > > > > > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > > > > ---
> > > > > >  drivers/gpu/drm/drm_atomic_uapi.c         |  4 +++
> > > > > >  drivers/gpu/drm/drm_connector.c           | 39 +++++++++++++++++++++++
> > > > > >  drivers/gpu/drm/drm_hdcp.c                | 36 ++++++++++++++++++++-
> > > > > >  drivers/gpu/drm/i915/display/intel_hdcp.c |  4 ++-
> > > > > >  include/drm/drm_connector.h               |  7 ++++
> > > > > >  include/drm/drm_hdcp.h                    |  2 +-
> > > > > >  include/drm/drm_mode_config.h             |  6 ++++
> > > > > >  include/uapi/drm/drm_mode.h               |  4 +++
> > > > > >  8 files changed, 99 insertions(+), 3 deletions(-)
> > > > > > 
> > > > > > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> > > > > > index abe38bdf85ae..19ae119f1a5d 100644
> > > > > > --- a/drivers/gpu/drm/drm_atomic_uapi.c
> > > > > > +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> > > > > > @@ -747,6 +747,8 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
> > > > > >  			return -EINVAL;
> > > > > >  		}
> > > > > >  		state->content_protection = val;
> > > > > > +	} else if (property == config->hdcp_content_type_property) {
> > > > > > +		state->hdcp_content_type = val;
> > > > > >  	} else if (property == connector->colorspace_property) {
> > > > > >  		state->colorspace = val;
> > > > > >  	} else if (property == config->writeback_fb_id_property) {
> > > > > > @@ -831,6 +833,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
> > > > > >  			state->hdr_output_metadata->base.id : 0;
> > > > > >  	} else if (property == config->content_protection_property) {
> > > > > >  		*val = state->content_protection;
> > > > > > +	} else if (property == config->hdcp_content_type_property) {
> > > > > > +		*val = state->hdcp_content_type;
> > > > > >  	} else if (property == config->writeback_fb_id_property) {
> > > > > >  		/* Writeback framebuffer is one-shot, write and forget */
> > > > > >  		*val = 0;
> > > > > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> > > > > > index 068d4b05f1be..732f6645643d 100644
> > > > > > --- a/drivers/gpu/drm/drm_connector.c
> > > > > > +++ b/drivers/gpu/drm/drm_connector.c
> > > > > > @@ -952,6 +952,45 @@ static const struct drm_prop_enum_list hdmi_colorspaces[] = {
> > > > > >   *	  is no longer protected and userspace should take appropriate action
> > > > > >   *	  (whatever that might be).
> > > > > >   *
> > > > > > + * HDCP Content Type:
> > > > > > + *	This Enum property is used by the userspace to declare the content type
> > > > > > + *	of the display stream, to kernel. Here display stream stands for any
> > > > > > + *	display content that userspace intended to render with HDCP encryption.    
> > > > > 
> > > > > Hi,
> > > > > 
> > > > > I'd suggest s/render with/display through/.
> > > > > 
> > > > > As a gfx dev, rendering is something quite different to me.    
> > > > Ok.  
> > > > >     
> > > > > > + *
> > > > > > + *	Content Type of a stream is decided by the owner of the stream, as
> > > > > > + *	"HDCP Type0" or "HDCP Type1".
> > > > > > + *
> > > > > > + *	The value of the property can be one the below:    
> > > > > 
> > > > > *one of the below    
> > > > Sure.  
> > > > >     
> > > > > > + *	  - "HDCP Type0": DRM_MODE_HDCP_CONTENT_TYPE0 = 0
> > > > > > + *	  - "HDCP Type1": DRM_MODE_HDCP_CONTENT_TYPE1 = 1
> > > > > > + *
> > > > > > + *	When kernel starts the HDCP authentication upon the "DESIRED" state of
> > > > > > + *	the "Content Protection", it refers the "HDCP Content Type" property
> > > > > > + *	state. And perform the HDCP authentication with the display sink for
> > > > > > + *	the content type mentioned by "HDCP Content Type".    
> > > > > 
> > > > > How about:
> > > > > 
> > > > > 	When kernel starts the HDCP authentication (see "Content Protection"
> > > > > 	for details), it uses the content type in "HDCP Content Type"
> > > > > 	for performing the HDCP authentication with the display sink.    
> > > > less confusing :) Thanks.  
> > > > >     
> > > > > > + *
> > > > > > + *	Stream classified as HDCP Type0 can be transmitted on a link which is
> > > > > > + *	encrypted with HDCP 1.4 or higher versions of HDCP(i.e HDCP2.2
> > > > > > + *	and more).    
> > > > > 
> > > > > This is where I get confused, see my earlier email from today on the
> > > > > previous revision of this patch series. Is it necessary to talk about
> > > > > HDCP versions here? The only thing that matters for UAPI is the content
> > > > > type, right?
> > > > > 
> > > > > Previously you said that the kernel will not use Type1 if userspace
> > > > > only asked for Type0, but to me this text reads as quite the opposite.    
> > > > Simple. HDCP2.2 itself support both Type 0 and Type 1. where as HDCP1.4
> > > > by default supports the Type 0 and doesn't support the Type 1.
> > > > 
> > > > I guess you are getting confused by assigning the type to the versions.  
> > > 
> > > Hi,
> > > 
> > > yes, I am indeed.
> > > 
> > > Is the HDCP version ever exposed to userspace in any way?
> > > 
> > > If it is, then explaining how the types relate to the versions may well
> > > be useful. But if userspace cannot even know what HDCP version is being
> > > used or available,  
> > Hi,
> > 
> > Implicitly content type capability is exposing the HDCP version capability.
> > Say if HDCP Type 1 is supported we infer that underlying kernel support
> > HDCP2.2.
> 
> Hi,
> 
> this is kind of a circular argument. ;-)
> 
> Do you think that userspace would actually use the HDCP version
> information for anything?
> 
> E.g. telling the end user that HDCP 2.2 is available if the display
> server finds "HDCP Content Type" property? Is this expected use of the
> property?
> 
> I also understand that this would only mean HDCP 2.2 support in the
> display driver and hardware, but it says nothing about the sinks, so
> userspace cannot know if HDCP 2.2 is actually being used unless it
> configures Type1 encryption and succeeds.
> 
> So if the HDCP version is useful to userspace, then this property is
> not enough to communicate it. But if it is not useful to userspace,
> then why mention it at all?
> 
> > And content type is introduced to restrict few catagories of the content
> > with few HDCP versions' protection, IMHO it is better to capture the
> > relationship between Type and the underlying HDCP versions.
> > 
> > See if the below para will do this task with out any ambiguous.
> > 
> >  * HDCP Content Type:
> >  *      This Enum property is used by the userspace to declare the content type
> >  *      of the display stream, to kernel. Here display stream stands for any
> >  *      display content that userspace intended to display through HDCP
> >  *      encryption.
> >  *
> >  *      Content Type of a stream is decided by the owner of the stream, as
> >  *      "HDCP Type0" or "HDCP Type1".
> >  *
> >  *      The value of the property can be one of the below:
> >  *        - "HDCP Type0": DRM_MODE_HDCP_CONTENT_TYPE0 = 0
> >  *        - "HDCP Type1": DRM_MODE_HDCP_CONTENT_TYPE1 = 1
> >  *
> >  *      When kernel starts the HDCP authentication (see "Content Protection"
> >  *      for details), it uses the content type in "HDCP Content Type"
> >  *      for performing the HDCP authentication with the display sink.
> >  *
> >  *      Please note in HDCP spec versions, a link can be authenticated with
> >  *      HDCP 2.2 for Content Type 0/Content Type 1. Where as a link can be
> >  *      authenticated with HDCP1.4 only for Content Type 0(though it is implicit
> >  *      in nature. As there is no reference for Content Type in HDCP1.4).
> 
> Ok, this makes it perfectly clear to me.
> 
> >  *
> >  *     HDCP2.2 authentication protocol itself takes the "Content Type" as a
> >  *      parameter, which is a input for the DP HDCP2.2 encryption algo.
> >  *
> >  *      In case of Type 0 content protection request, kernel driver can choose
> >  *      either of HDCP spec versions 1.4 and 2.2. When HDCP2.2 is used for
> >  *      "HDCP Type 0", a HDCP 2.2 capable repeater in the downstream can send
> >  *      that content to a HDCP 1.4 authenticated HDCP sink (Type0 link).
> >  *      But if the content is classified as "HDCP Type 1", above mentioned
> >  *      HDCP 2.2 repeater wont send the content to the HDCP sink as it can't
> >  *      authenticate the HDCP1.4 capable sink for "HDCP Type 1".
> >  *
> 
> Right, but are these details meaningful to the userspace using this
> API? I mean, are these the property of this API rather than just
> mandated by HDCP spec 2.2 in general?
> 
> >  *      Please note userspace can be ignorant of the HDCP versions used by the
> >  *      kernel driver to achieve the "HDCP Content Type".
> >  *
> >  *      At current scenario, classifying a content as Type 1 ensures that the
> >  *      content will be displayed only through the HDCP2.2 encrypted link.
> >  *
> >  *      Note that the HDCP Content Type property is introduced at HDCP 2.2, and
> >  *      defaults to type 0. It is only exposed by drivers supporting HDCP 2.2
> >  *      (hence supporting Type 0 and Type 1). Based on how next versions of
> >  *      HDCP specs are defined content Type could be used for higher versions too.
> >  *
> >  *      If content type is changed when "Content Protection" is not UNDESIRED,
> >  *      then kernel will disable the HDCP and re-enable with new type in the
> >  *      same atomic commit. And when "Content Protection" is ENABLED, it means
> >  *      that link is HDCP authenticated and encrypted, for the transmission of
> >  *      the Type of stream mentioned at "HDCP Content Type".
> > 
> > Please share your view on this.
> 
> That is a big piece of well-written doc. If you believe all these
> details can be relevant to userspace using this property, then I am
> happy see this in the UAPI doc. It is detailed and clear. My only
> objection is whether it is all relevant, and I'm ok giving that up.

These information might be needed for a userspace who want to classify
their content, as implication of the each classification is called out
here. Agreed not everyone will seek for this piece of info, but bit more
is ok in doc I guess.

Thanks for approving this. I will respin this patch.

-Ram.
> 
> 
> Thanks,
> pq


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

Re: [PATCH v9 1/6] drm: Add Content protection type property

[Pekka Paalanen]

[-- Attachment #1.1: Type: text/plain, Size: 11772 bytes --]

On Thu, 11 Jul 2019 11:20:49 +0530
Ramalingam C <ramalingam.c@intel.com> wrote:

> On 2019-07-10 at 11:16:24 +0300, Pekka Paalanen wrote:
> > On Tue, 9 Jul 2019 18:17:59 +0530
> > Ramalingam C <ramalingam.c@intel.com> wrote:
> >   
> > > On 2019-07-09 at 17:31:10 +0300, Pekka Paalanen wrote:  
> > > > On Mon,  8 Jul 2019 16:51:11 +0530
> > > > Ramalingam C <ramalingam.c@intel.com> wrote:
> > > >     
> > > > > This patch adds a DRM ENUM property to the selected connectors.
> > > > > This property is used for mentioning the protected content's type
> > > > > from userspace to kernel HDCP authentication.
> > > > > 
> > > > > Type of the stream is decided by the protected content providers.
> > > > > Type 0 content can be rendered on any HDCP protected display wires.
> > > > > But Type 1 content can be rendered only on HDCP2.2 protected paths.
> > > > > 
> > > > > So when a userspace sets this property to Type 1 and starts the HDCP
> > > > > enable, kernel will honour it only if HDCP2.2 authentication is through
> > > > > for type 1. Else HDCP enable will be failed.
> > > > > 
> > > > > Need ACK for this new conenctor property from userspace consumer.
> > > > > 
> > > > > v2:
> > > > >   cp_content_type is replaced with content_protection_type [daniel]
> > > > >   check at atomic_set_property is removed [Maarten]
> > > > > v3:
> > > > >   %s/content_protection_type/hdcp_content_type [Pekka]
> > > > > v4:
> > > > >   property is created for the first requested connector and then reused.
> > > > > 	[Danvet]
> > > > > v5:
> > > > >   kernel doc nits addressed [Daniel]
> > > > >   Rebased as part of patch reordering.
> > > > > v6:
> > > > >   Kernel docs are modified [pekka]
> > > > > v7:
> > > > >   More details in Kernel docs. [pekka]
> > > > > 
> > > > > Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> > > > > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > > > ---
> > > > >  drivers/gpu/drm/drm_atomic_uapi.c         |  4 +++
> > > > >  drivers/gpu/drm/drm_connector.c           | 39 +++++++++++++++++++++++
> > > > >  drivers/gpu/drm/drm_hdcp.c                | 36 ++++++++++++++++++++-
> > > > >  drivers/gpu/drm/i915/display/intel_hdcp.c |  4 ++-
> > > > >  include/drm/drm_connector.h               |  7 ++++
> > > > >  include/drm/drm_hdcp.h                    |  2 +-
> > > > >  include/drm/drm_mode_config.h             |  6 ++++
> > > > >  include/uapi/drm/drm_mode.h               |  4 +++
> > > > >  8 files changed, 99 insertions(+), 3 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> > > > > index abe38bdf85ae..19ae119f1a5d 100644
> > > > > --- a/drivers/gpu/drm/drm_atomic_uapi.c
> > > > > +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> > > > > @@ -747,6 +747,8 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
> > > > >  			return -EINVAL;
> > > > >  		}
> > > > >  		state->content_protection = val;
> > > > > +	} else if (property == config->hdcp_content_type_property) {
> > > > > +		state->hdcp_content_type = val;
> > > > >  	} else if (property == connector->colorspace_property) {
> > > > >  		state->colorspace = val;
> > > > >  	} else if (property == config->writeback_fb_id_property) {
> > > > > @@ -831,6 +833,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
> > > > >  			state->hdr_output_metadata->base.id : 0;
> > > > >  	} else if (property == config->content_protection_property) {
> > > > >  		*val = state->content_protection;
> > > > > +	} else if (property == config->hdcp_content_type_property) {
> > > > > +		*val = state->hdcp_content_type;
> > > > >  	} else if (property == config->writeback_fb_id_property) {
> > > > >  		/* Writeback framebuffer is one-shot, write and forget */
> > > > >  		*val = 0;
> > > > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> > > > > index 068d4b05f1be..732f6645643d 100644
> > > > > --- a/drivers/gpu/drm/drm_connector.c
> > > > > +++ b/drivers/gpu/drm/drm_connector.c
> > > > > @@ -952,6 +952,45 @@ static const struct drm_prop_enum_list hdmi_colorspaces[] = {
> > > > >   *	  is no longer protected and userspace should take appropriate action
> > > > >   *	  (whatever that might be).
> > > > >   *
> > > > > + * HDCP Content Type:
> > > > > + *	This Enum property is used by the userspace to declare the content type
> > > > > + *	of the display stream, to kernel. Here display stream stands for any
> > > > > + *	display content that userspace intended to render with HDCP encryption.    
> > > > 
> > > > Hi,
> > > > 
> > > > I'd suggest s/render with/display through/.
> > > > 
> > > > As a gfx dev, rendering is something quite different to me.    
> > > Ok.  
> > > >     
> > > > > + *
> > > > > + *	Content Type of a stream is decided by the owner of the stream, as
> > > > > + *	"HDCP Type0" or "HDCP Type1".
> > > > > + *
> > > > > + *	The value of the property can be one the below:    
> > > > 
> > > > *one of the below    
> > > Sure.  
> > > >     
> > > > > + *	  - "HDCP Type0": DRM_MODE_HDCP_CONTENT_TYPE0 = 0
> > > > > + *	  - "HDCP Type1": DRM_MODE_HDCP_CONTENT_TYPE1 = 1
> > > > > + *
> > > > > + *	When kernel starts the HDCP authentication upon the "DESIRED" state of
> > > > > + *	the "Content Protection", it refers the "HDCP Content Type" property
> > > > > + *	state. And perform the HDCP authentication with the display sink for
> > > > > + *	the content type mentioned by "HDCP Content Type".    
> > > > 
> > > > How about:
> > > > 
> > > > 	When kernel starts the HDCP authentication (see "Content Protection"
> > > > 	for details), it uses the content type in "HDCP Content Type"
> > > > 	for performing the HDCP authentication with the display sink.    
> > > less confusing :) Thanks.  
> > > >     
> > > > > + *
> > > > > + *	Stream classified as HDCP Type0 can be transmitted on a link which is
> > > > > + *	encrypted with HDCP 1.4 or higher versions of HDCP(i.e HDCP2.2
> > > > > + *	and more).    
> > > > 
> > > > This is where I get confused, see my earlier email from today on the
> > > > previous revision of this patch series. Is it necessary to talk about
> > > > HDCP versions here? The only thing that matters for UAPI is the content
> > > > type, right?
> > > > 
> > > > Previously you said that the kernel will not use Type1 if userspace
> > > > only asked for Type0, but to me this text reads as quite the opposite.    
> > > Simple. HDCP2.2 itself support both Type 0 and Type 1. where as HDCP1.4
> > > by default supports the Type 0 and doesn't support the Type 1.
> > > 
> > > I guess you are getting confused by assigning the type to the versions.  
> > 
> > Hi,
> > 
> > yes, I am indeed.
> > 
> > Is the HDCP version ever exposed to userspace in any way?
> > 
> > If it is, then explaining how the types relate to the versions may well
> > be useful. But if userspace cannot even know what HDCP version is being
> > used or available,  
> Hi,
> 
> Implicitly content type capability is exposing the HDCP version capability.
> Say if HDCP Type 1 is supported we infer that underlying kernel support
> HDCP2.2.

Hi,

this is kind of a circular argument. ;-)

Do you think that userspace would actually use the HDCP version
information for anything?

E.g. telling the end user that HDCP 2.2 is available if the display
server finds "HDCP Content Type" property? Is this expected use of the
property?

I also understand that this would only mean HDCP 2.2 support in the
display driver and hardware, but it says nothing about the sinks, so
userspace cannot know if HDCP 2.2 is actually being used unless it
configures Type1 encryption and succeeds.

So if the HDCP version is useful to userspace, then this property is
not enough to communicate it. But if it is not useful to userspace,
then why mention it at all?

> And content type is introduced to restrict few catagories of the content
> with few HDCP versions' protection, IMHO it is better to capture the
> relationship between Type and the underlying HDCP versions.
> 
> See if the below para will do this task with out any ambiguous.
> 
>  * HDCP Content Type:
>  *      This Enum property is used by the userspace to declare the content type
>  *      of the display stream, to kernel. Here display stream stands for any
>  *      display content that userspace intended to display through HDCP
>  *      encryption.
>  *
>  *      Content Type of a stream is decided by the owner of the stream, as
>  *      "HDCP Type0" or "HDCP Type1".
>  *
>  *      The value of the property can be one of the below:
>  *        - "HDCP Type0": DRM_MODE_HDCP_CONTENT_TYPE0 = 0
>  *        - "HDCP Type1": DRM_MODE_HDCP_CONTENT_TYPE1 = 1
>  *
>  *      When kernel starts the HDCP authentication (see "Content Protection"
>  *      for details), it uses the content type in "HDCP Content Type"
>  *      for performing the HDCP authentication with the display sink.
>  *
>  *      Please note in HDCP spec versions, a link can be authenticated with
>  *      HDCP 2.2 for Content Type 0/Content Type 1. Where as a link can be
>  *      authenticated with HDCP1.4 only for Content Type 0(though it is implicit
>  *      in nature. As there is no reference for Content Type in HDCP1.4).

Ok, this makes it perfectly clear to me.

>  *
>  *     HDCP2.2 authentication protocol itself takes the "Content Type" as a
>  *      parameter, which is a input for the DP HDCP2.2 encryption algo.
>  *
>  *      In case of Type 0 content protection request, kernel driver can choose
>  *      either of HDCP spec versions 1.4 and 2.2. When HDCP2.2 is used for
>  *      "HDCP Type 0", a HDCP 2.2 capable repeater in the downstream can send
>  *      that content to a HDCP 1.4 authenticated HDCP sink (Type0 link).
>  *      But if the content is classified as "HDCP Type 1", above mentioned
>  *      HDCP 2.2 repeater wont send the content to the HDCP sink as it can't
>  *      authenticate the HDCP1.4 capable sink for "HDCP Type 1".
>  *

Right, but are these details meaningful to the userspace using this
API? I mean, are these the property of this API rather than just
mandated by HDCP spec 2.2 in general?

>  *      Please note userspace can be ignorant of the HDCP versions used by the
>  *      kernel driver to achieve the "HDCP Content Type".
>  *
>  *      At current scenario, classifying a content as Type 1 ensures that the
>  *      content will be displayed only through the HDCP2.2 encrypted link.
>  *
>  *      Note that the HDCP Content Type property is introduced at HDCP 2.2, and
>  *      defaults to type 0. It is only exposed by drivers supporting HDCP 2.2
>  *      (hence supporting Type 0 and Type 1). Based on how next versions of
>  *      HDCP specs are defined content Type could be used for higher versions too.
>  *
>  *      If content type is changed when "Content Protection" is not UNDESIRED,
>  *      then kernel will disable the HDCP and re-enable with new type in the
>  *      same atomic commit. And when "Content Protection" is ENABLED, it means
>  *      that link is HDCP authenticated and encrypted, for the transmission of
>  *      the Type of stream mentioned at "HDCP Content Type".
> 
> Please share your view on this.

That is a big piece of well-written doc. If you believe all these
details can be relevant to userspace using this property, then I am
happy see this in the UAPI doc. It is detailed and clear. My only
objection is whether it is all relevant, and I'm ok giving that up.


Thanks,
pq

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH v9 1/6] drm: Add Content protection type property

[Pekka Paalanen]

[-- Attachment #1.1: Type: text/plain, Size: 4694 bytes --]

On Thu, 11 Jul 2019 10:18:22 -0400
Sean Paul <sean@poorly.run> wrote:

> On Mon, Jul 08, 2019 at 04:51:11PM +0530, Ramalingam C wrote:
> > This patch adds a DRM ENUM property to the selected connectors.
> > This property is used for mentioning the protected content's type
> > from userspace to kernel HDCP authentication.
> > 
> > Type of the stream is decided by the protected content providers.
> > Type 0 content can be rendered on any HDCP protected display wires.
> > But Type 1 content can be rendered only on HDCP2.2 protected paths.
> > 
> > So when a userspace sets this property to Type 1 and starts the HDCP
> > enable, kernel will honour it only if HDCP2.2 authentication is through
> > for type 1. Else HDCP enable will be failed.
> > 
> > Need ACK for this new conenctor property from userspace consumer.

...

> > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> > index 068d4b05f1be..732f6645643d 100644
> > --- a/drivers/gpu/drm/drm_connector.c
> > +++ b/drivers/gpu/drm/drm_connector.c
> > @@ -952,6 +952,45 @@ static const struct drm_prop_enum_list hdmi_colorspaces[] = {
> >   *	  is no longer protected and userspace should take appropriate action
> >   *	  (whatever that might be).
> >   *
> > + * HDCP Content Type:
> > + *	This Enum property is used by the userspace to declare the content type
> > + *	of the display stream, to kernel. Here display stream stands for any
> > + *	display content that userspace intended to render with HDCP encryption.
> > + *
> > + *	Content Type of a stream is decided by the owner of the stream, as
> > + *	"HDCP Type0" or "HDCP Type1".
> > + *
> > + *	The value of the property can be one the below:
> > + *	  - "HDCP Type0": DRM_MODE_HDCP_CONTENT_TYPE0 = 0
> > + *	  - "HDCP Type1": DRM_MODE_HDCP_CONTENT_TYPE1 = 1
> > + *
> > + *	When kernel starts the HDCP authentication upon the "DESIRED" state of
> > + *	the "Content Protection", it refers the "HDCP Content Type" property
> > + *	state. And perform the HDCP authentication with the display sink for
> > + *	the content type mentioned by "HDCP Content Type".
> > + *
> > + *	Stream classified as HDCP Type0 can be transmitted on a link which is
> > + *	encrypted with HDCP 1.4 or higher versions of HDCP(i.e HDCP2.2
> > + *	and more).
> > + *
> > + *	Streams classified as HDCP Type1 can be transmitted on a link which is
> > + *	encrypted only with HDCP 2.2. In future, HDCP versions >2.2 also might
> > + *	support Type1 based on their spec.
> > + *
> > + *	HDCP2.2 authentication protocol itself takes the "Content Type" as a
> > + *	parameter, which is a input for the DP HDCP2.2 encryption algo.
> > + *
> > + *	Note that the HDCP Content Type property is introduced at HDCP 2.2, and
> > + *	defaults to type 0. It is only exposed by drivers supporting HDCP 2.2.
> > + *	Based on how next versions of HDCP specs are defined content Type could
> > + *	be used for higher versions too.
> > + *
> > + *	If content type is changed when "Content Protection" is not UNDESIRED,
> > + *	then kernel will disable the HDCP and re-enable with new type in the
> > + *	same atomic commit. And when "Content Protection" is ENABLED, it means
> > + *	that link is HDCP authenticated and encrypted, for the transmission of
> > + *	the Type of stream mentioned at "HDCP Content Type".
> > + *
> >   * HDR_OUTPUT_METADATA:
> >   *	Connector property to enable userspace to send HDR Metadata to
> >   *	driver. This metadata is based on the composition and blending  
> 
> Do we actually need an entirely new property? Can't we just add a new
> entry to the existing Content Protection property which is "Desired Type1" or
> similar? The kernel will then either attempt 2.2 auth or it will ignore it the
> request if it's not supported.

Hi,

IMHO the existing "Content Protection" property is already complicated
enough that one should not add anything new to it.

If you added "desired-type-1", the readback of it would become
ambiguous if it was "ENABLED", userspace would not know if the value
written was "DESIRED" or "desired-type-1". Sure, it's not a problem
when a display server knows what it just wrote into it, but shouldn't
we try to keep KMS state readable as well, if for nothing but debugging?

I think using the same property for communicating in both directions
between the kernel and userspace (value can be set by both userspace and
kernel at times) was a mistake to begin with. It has already caused
long discussions on what the readback actually should reflect and
whether there are races for a given userspace implementation.


Thanks,
pq

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH v9 1/6] drm: Add Content protection type property

[Sean Paul]

On Fri, Jul 12, 2019 at 02:39:05PM +0300, Pekka Paalanen wrote:
> On Thu, 11 Jul 2019 10:18:22 -0400
> Sean Paul <sean@poorly.run> wrote:
> 
> > On Mon, Jul 08, 2019 at 04:51:11PM +0530, Ramalingam C wrote:
> > > This patch adds a DRM ENUM property to the selected connectors.
> > > This property is used for mentioning the protected content's type
> > > from userspace to kernel HDCP authentication.
> > > 
> > > Type of the stream is decided by the protected content providers.
> > > Type 0 content can be rendered on any HDCP protected display wires.
> > > But Type 1 content can be rendered only on HDCP2.2 protected paths.
> > > 
> > > So when a userspace sets this property to Type 1 and starts the HDCP
> > > enable, kernel will honour it only if HDCP2.2 authentication is through
> > > for type 1. Else HDCP enable will be failed.
> > > 
> > > Need ACK for this new conenctor property from userspace consumer.
> 
> ...
> 
> > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> > > index 068d4b05f1be..732f6645643d 100644
> > > --- a/drivers/gpu/drm/drm_connector.c
> > > +++ b/drivers/gpu/drm/drm_connector.c
> > > @@ -952,6 +952,45 @@ static const struct drm_prop_enum_list hdmi_colorspaces[] = {
> > >   *	  is no longer protected and userspace should take appropriate action
> > >   *	  (whatever that might be).
> > >   *
> > > + * HDCP Content Type:
> > > + *	This Enum property is used by the userspace to declare the content type
> > > + *	of the display stream, to kernel. Here display stream stands for any
> > > + *	display content that userspace intended to render with HDCP encryption.
> > > + *
> > > + *	Content Type of a stream is decided by the owner of the stream, as
> > > + *	"HDCP Type0" or "HDCP Type1".
> > > + *
> > > + *	The value of the property can be one the below:
> > > + *	  - "HDCP Type0": DRM_MODE_HDCP_CONTENT_TYPE0 = 0
> > > + *	  - "HDCP Type1": DRM_MODE_HDCP_CONTENT_TYPE1 = 1
> > > + *
> > > + *	When kernel starts the HDCP authentication upon the "DESIRED" state of
> > > + *	the "Content Protection", it refers the "HDCP Content Type" property
> > > + *	state. And perform the HDCP authentication with the display sink for
> > > + *	the content type mentioned by "HDCP Content Type".
> > > + *
> > > + *	Stream classified as HDCP Type0 can be transmitted on a link which is
> > > + *	encrypted with HDCP 1.4 or higher versions of HDCP(i.e HDCP2.2
> > > + *	and more).
> > > + *
> > > + *	Streams classified as HDCP Type1 can be transmitted on a link which is
> > > + *	encrypted only with HDCP 2.2. In future, HDCP versions >2.2 also might
> > > + *	support Type1 based on their spec.
> > > + *
> > > + *	HDCP2.2 authentication protocol itself takes the "Content Type" as a
> > > + *	parameter, which is a input for the DP HDCP2.2 encryption algo.
> > > + *
> > > + *	Note that the HDCP Content Type property is introduced at HDCP 2.2, and
> > > + *	defaults to type 0. It is only exposed by drivers supporting HDCP 2.2.
> > > + *	Based on how next versions of HDCP specs are defined content Type could
> > > + *	be used for higher versions too.
> > > + *
> > > + *	If content type is changed when "Content Protection" is not UNDESIRED,
> > > + *	then kernel will disable the HDCP and re-enable with new type in the
> > > + *	same atomic commit. And when "Content Protection" is ENABLED, it means
> > > + *	that link is HDCP authenticated and encrypted, for the transmission of
> > > + *	the Type of stream mentioned at "HDCP Content Type".
> > > + *
> > >   * HDR_OUTPUT_METADATA:
> > >   *	Connector property to enable userspace to send HDR Metadata to
> > >   *	driver. This metadata is based on the composition and blending  
> > 
> > Do we actually need an entirely new property? Can't we just add a new
> > entry to the existing Content Protection property which is "Desired Type1" or
> > similar? The kernel will then either attempt 2.2 auth or it will ignore it the
> > request if it's not supported.
> 
> Hi,
> 
> IMHO the existing "Content Protection" property is already complicated
> enough that one should not add anything new to it.
> 
> If you added "desired-type-1", the readback of it would become
> ambiguous if it was "ENABLED", userspace would not know if the value
> written was "DESIRED" or "desired-type-1". Sure, it's not a problem
> when a display server knows what it just wrote into it, but shouldn't
> we try to keep KMS state readable as well, if for nothing but debugging?

Yeah, that's a fair point, it would also create a problem if the HDCP version
was somehow downgraded between u/s polling the property.

> 
> I think using the same property for communicating in both directions
> between the kernel and userspace (value can be set by both userspace and
> kernel at times) was a mistake to begin with. 

We can agree on that. I figured that since we already had the subpar UAPI, we're
stuck with it anyways.

In light of the above though, I agree a new property makes sense to me.

Sean


> It has already caused
> long discussions on what the readback actually should reflect and
> whether there are races for a given userspace implementation.
> 
> 
> Thanks,
> pq



-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx