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

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

[Ramalingam C]

Rebased the remaining 5 patches from the original series.
https://patchwork.freedesktop.org/series/57232/

And as per pekka's review comments few additions are done in the kernel
documentation.

Test-with: <20190703095446.14092-2-ramalingam.c@intel.com>

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           | 22 +++++++
 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, 228 insertions(+), 30 deletions(-)

-- 
2.19.1

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

[PATCH v8 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]

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           | 22 ++++++++++++++
 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, 82 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..17aef88c03a6 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -951,6 +951,28 @@ static const struct drm_prop_enum_list hdmi_colorspaces[] = {
  *	  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 property is used by the userspace to configure the kernel with
+ *	to be displayed stream's content type. 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 Type0 streams can be transmitted on a link which is
+ *		encrypted with HDCP 1.4 or higher versions of HDCP(i.e HDCP2.2
+ *		and more).
+ *	  - "HDCP Type1": DRM_MODE_HDCP_CONTENT_TYPE1 = 1
+ *		HDCP Type1 streams can be transmitted on a link which is
+ *		encrypted only with HDCP 2.2. In future higher versions also
+ *		might support Type1 based on their spec.
+ *
+ *	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
  *
  * HDR_OUTPUT_METADATA:
  *	Connector property to enable userspace to send HDR Metadata to
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

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

[Pekka Paalanen]

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

On Fri,  5 Jul 2019 06:16:37 +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]
> 
> 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           | 22 ++++++++++++++
>  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, 82 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..17aef88c03a6 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -951,6 +951,28 @@ static const struct drm_prop_enum_list hdmi_colorspaces[] = {
>   *	  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 property is used by the userspace to configure the kernel with
> + *	to be displayed stream's content type. 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 Type0 streams can be transmitted on a link which is
> + *		encrypted with HDCP 1.4 or higher versions of HDCP(i.e HDCP2.2
> + *		and more).
> + *	  - "HDCP Type1": DRM_MODE_HDCP_CONTENT_TYPE1 = 1
> + *		HDCP Type1 streams can be transmitted on a link which is
> + *		encrypted only with HDCP 2.2. In future higher versions also
> + *		might support Type1 based on their spec.
> + *
> + *	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

Hi,

I think this doc covers all my previous comments on this patch now. One
more thing, the wording here reads as:
- userspace configures the content type
- the kernel transmits the content if the link satisfies the type
  requirement
- if the link does not satisfy the type requirement, the kernel will
  not transmit the content

This is of course false, the kernel transmits anyway, but that is how
the text reads from the "stream's content type" and "can be transmitted
on". The problem is, that a userspace developer will think the stream
is what he pushes into KMS, not what goes on the wire. The text also
magnifies that misconception because it sounds like the stream is
something different from the link. Actually, I don't understand what
"the stream" is supposed to be here.

Instead, I think you should explain how the content type property
guides the kernel driver's attempts in negotiating the link encryption
and how that then reflects in the content protection property (DESIRED
vs. ENABLED). It might be best to not say anything about any "stream".


Thanks,
pq

>   *
>   * HDR_OUTPUT_METADATA:
>   *	Connector property to enable userspace to send HDR Metadata to
> 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 --]

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

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

[Pekka Paalanen]

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

On Mon, 8 Jul 2019 12:52:17 +0300
Pekka Paalanen <ppaalanen@gmail.com> wrote:

> On Fri,  5 Jul 2019 06:16:37 +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]
> > 
> > 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           | 22 ++++++++++++++
> >  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, 82 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..17aef88c03a6 100644
> > --- a/drivers/gpu/drm/drm_connector.c
> > +++ b/drivers/gpu/drm/drm_connector.c
> > @@ -951,6 +951,28 @@ static const struct drm_prop_enum_list hdmi_colorspaces[] = {
> >   *	  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 property is used by the userspace to configure the kernel with
> > + *	to be displayed stream's content type. 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 Type0 streams can be transmitted on a link which is
> > + *		encrypted with HDCP 1.4 or higher versions of HDCP(i.e HDCP2.2
> > + *		and more).
> > + *	  - "HDCP Type1": DRM_MODE_HDCP_CONTENT_TYPE1 = 1
> > + *		HDCP Type1 streams can be transmitted on a link which is
> > + *		encrypted only with HDCP 2.2. In future higher versions also
> > + *		might support Type1 based on their spec.
> > + *
> > + *	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  
> 
> Hi,
> 
> I think this doc covers all my previous comments on this patch now. One
> more thing, the wording here reads as:
> - userspace configures the content type
> - the kernel transmits the content if the link satisfies the type
>   requirement
> - if the link does not satisfy the type requirement, the kernel will
>   not transmit the content
> 
> This is of course false, the kernel transmits anyway, but that is how
> the text reads from the "stream's content type" and "can be transmitted
> on". The problem is, that a userspace developer will think the stream
> is what he pushes into KMS, not what goes on the wire. The text also
> magnifies that misconception because it sounds like the stream is
> something different from the link. Actually, I don't understand what
> "the stream" is supposed to be here.
> 
> Instead, I think you should explain how the content type property
> guides the kernel driver's attempts in negotiating the link encryption
> and how that then reflects in the content protection property (DESIRED
> vs. ENABLED). It might be best to not say anything about any "stream".

Btw. this UAPI has the following fundamental flaws:

- userspace cannot know which encryption is used on the link
- userspace cannot force Type0 if the driver succeeds enabling Type1

To me this seems like poor UAPI design. Why was this designed like this?


Thanks,
pq

> >   *
> >   * HDR_OUTPUT_METADATA:
> >   *	Connector property to enable userspace to send HDR Metadata to
> > 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 v8 1/6] drm: Add Content protection type property

[Ramalingam C]

On 2019-07-08 at 12:59:59 +0300, Pekka Paalanen wrote:
> On Mon, 8 Jul 2019 12:52:17 +0300
> Pekka Paalanen <ppaalanen@gmail.com> wrote:
> 
> > On Fri,  5 Jul 2019 06:16:37 +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]
> > > 
> > > 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           | 22 ++++++++++++++
> > >  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, 82 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..17aef88c03a6 100644
> > > --- a/drivers/gpu/drm/drm_connector.c
> > > +++ b/drivers/gpu/drm/drm_connector.c
> > > @@ -951,6 +951,28 @@ static const struct drm_prop_enum_list hdmi_colorspaces[] = {
> > >   *	  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 property is used by the userspace to configure the kernel with
> > > + *	to be displayed stream's content type. 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 Type0 streams can be transmitted on a link which is
> > > + *		encrypted with HDCP 1.4 or higher versions of HDCP(i.e HDCP2.2
> > > + *		and more).
> > > + *	  - "HDCP Type1": DRM_MODE_HDCP_CONTENT_TYPE1 = 1
> > > + *		HDCP Type1 streams can be transmitted on a link which is
> > > + *		encrypted only with HDCP 2.2. In future higher versions also
> > > + *		might support Type1 based on their spec.
> > > + *
> > > + *	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  
> > 
> > Hi,
> > 
> > I think this doc covers all my previous comments on this patch now. One
> > more thing, the wording here reads as:
> > - userspace configures the content type
> > - the kernel transmits the content if the link satisfies the type
> >   requirement
> > - if the link does not satisfy the type requirement, the kernel will
> >   not transmit the content
> > 
> > This is of course false, the kernel transmits anyway, but that is how
> > the text reads from the "stream's content type" and "can be transmitted
> > on". The problem is, that a userspace developer will think the stream
> > is what he pushes into KMS, not what goes on the wire. The text also
> > magnifies that misconception because it sounds like the stream is
> > something different from the link. Actually, I don't understand what
> > "the stream" is supposed to be here.
Stream is nothing but the any display content that needs the hdcp
protection.
> > 
> > Instead, I think you should explain how the content type property
> > guides the kernel driver's attempts in negotiating the link encryption
> > and how that then reflects in the content protection property (DESIRED
> > vs. ENABLED). It might be best to not say anything about any "stream".

I will explain in different and more words, so that this questions wont
raise.
> 
> Btw. this UAPI has the following fundamental flaws:
> 
> - userspace cannot know which encryption is used on the link
This claim is not true.

"HDCP content type" and "content protection" are used
together in
	requesting the HDCP state
	confirming that required state is achieved
For ex:
The state "HDCP content type" = "HDCP Type1" and "content protection" =
"DESIRED" stands for userspace has requested for HDCP Type 1 protection
from kernel.

When kernel changes the "content protection" to "ENABLED" when "HDCP
content type" is "HDCP Type1", kernel indicates the userspace that HDCP
authentication for Type 1 is successful.

I am not sure why do you think that userspace is not knowing link's
authentication state w.r.t type.

> - userspace cannot force Type0 if the driver succeeds enabling Type1
When you set Type 0, kernel will authenticate the link for Type 0 only.
Not for Type 1. I guess you are trying to say HDCP1.4 is not possible
over HDCP2.2.
I dont see any reason why anyone want this except for testing.

Type 0 can be achieved through HDCP2.2 and HDCP1.4. And HDCP2.2 is attempted
first and HDCP1.4 will be attempted only if HDCP2.2 is failed for some
reasons. This is done because HDCP2.2 is preferred over 1.4 due to its
better crypto algo. 

Thanks for pointing it out. There is a scope to add all these
information in the documentation. Which I will do it in the next version
today.

Thanks,
-Ram.

> 
> To me this seems like poor UAPI design. Why was this designed like this?
> 
> 
> Thanks,
> pq
> 
> > >   *
> > >   * HDR_OUTPUT_METADATA:
> > >   *	Connector property to enable userspace to send HDR Metadata to
> > > 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 v8 1/6] drm: Add Content protection type property

[Pekka Paalanen]

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

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

> On 2019-07-08 at 12:59:59 +0300, Pekka Paalanen wrote:
> > On Mon, 8 Jul 2019 12:52:17 +0300
> > Pekka Paalanen <ppaalanen@gmail.com> wrote:
> >   
> > > On Fri,  5 Jul 2019 06:16:37 +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]
> > > > 
> > > > 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           | 22 ++++++++++++++
> > > >  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, 82 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..17aef88c03a6 100644
> > > > --- a/drivers/gpu/drm/drm_connector.c
> > > > +++ b/drivers/gpu/drm/drm_connector.c
> > > > @@ -951,6 +951,28 @@ static const struct drm_prop_enum_list hdmi_colorspaces[] = {
> > > >   *	  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 property is used by the userspace to configure the kernel with
> > > > + *	to be displayed stream's content type. 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 Type0 streams can be transmitted on a link which is
> > > > + *		encrypted with HDCP 1.4 or higher versions of HDCP(i.e HDCP2.2
> > > > + *		and more).
> > > > + *	  - "HDCP Type1": DRM_MODE_HDCP_CONTENT_TYPE1 = 1
> > > > + *		HDCP Type1 streams can be transmitted on a link which is
> > > > + *		encrypted only with HDCP 2.2. In future higher versions also
> > > > + *		might support Type1 based on their spec.
> > > > + *
> > > > + *	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    
> > > 
> > > Hi,
> > > 
> > > I think this doc covers all my previous comments on this patch now. One
> > > more thing, the wording here reads as:
> > > - userspace configures the content type
> > > - the kernel transmits the content if the link satisfies the type
> > >   requirement
> > > - if the link does not satisfy the type requirement, the kernel will
> > >   not transmit the content
> > > 
> > > This is of course false, the kernel transmits anyway, but that is how
> > > the text reads from the "stream's content type" and "can be transmitted
> > > on". The problem is, that a userspace developer will think the stream
> > > is what he pushes into KMS, not what goes on the wire. The text also
> > > magnifies that misconception because it sounds like the stream is
> > > something different from the link. Actually, I don't understand what
> > > "the stream" is supposed to be here.  
> Stream is nothing but the any display content that needs the hdcp
> protection.
> > > 
> > > Instead, I think you should explain how the content type property
> > > guides the kernel driver's attempts in negotiating the link encryption
> > > and how that then reflects in the content protection property (DESIRED
> > > vs. ENABLED). It might be best to not say anything about any "stream".  
> 
> I will explain in different and more words, so that this questions wont
> raise.
> > 
> > Btw. this UAPI has the following fundamental flaws:
> > 
> > - userspace cannot know which encryption is used on the link  
> This claim is not true.
> 
> "HDCP content type" and "content protection" are used
> together in
> 	requesting the HDCP state
> 	confirming that required state is achieved
> For ex:
> The state "HDCP content type" = "HDCP Type1" and "content protection" =
> "DESIRED" stands for userspace has requested for HDCP Type 1 protection
> from kernel.
> 
> When kernel changes the "content protection" to "ENABLED" when "HDCP
> content type" is "HDCP Type1", kernel indicates the userspace that HDCP
> authentication for Type 1 is successful.
> 
> I am not sure why do you think that userspace is not knowing link's
> authentication state w.r.t type.

Hi,

if userspace asks for Type0, the kernel is free to use Type1 instead
and switch "Content Protection" to "ENABLED". Userspace has no way of
knowing that the link actually runs with Type1.

You can argue that it does not matter, because Type1 protection is
stronger than Type0, so everybody should be happy, but that does not
change the fact that userspace thinks wrong of what the protection
really is.

> > - userspace cannot force Type0 if the driver succeeds enabling Type1  
> When you set Type 0, kernel will authenticate the link for Type 0 only.

Really? Sorry, I have completely missed this. Please make it clear in
UAPI and kAPI docs. This invalidates my comment above as well.

But you are also conflicting with your own doc, which said:

+ *	  - "HDCP Type0": DRM_MODE_HDCP_CONTENT_TYPE0 = 0
+ *		HDCP Type0 streams can be transmitted on a link which is
+ *		encrypted with HDCP 1.4 or higher versions of HDCP(i.e HDCP2.2
+ *		and more).

To me that reads as: if userspace asks for Type0, the kernel is free to
use Type0 or Type1 or anything stronger to say it succeeded.

Hence my confusion.

> Not for Type 1. I guess you are trying to say HDCP1.4 is not possible
> over HDCP2.2.

I wouldn't know what that means.

> I dont see any reason why anyone want this except for testing.
> 
> Type 0 can be achieved through HDCP2.2 and HDCP1.4. And HDCP2.2 is attempted
> first and HDCP1.4 will be attempted only if HDCP2.2 is failed for some
> reasons. This is done because HDCP2.2 is preferred over 1.4 due to its
> better crypto algo. 

I'm running with the assumption that:
- Type0 == HDCP 1.4
- Type1 == HDCP 2.2
and I have no idea why you are sometimes talking about Type and
sometimes about HDCP version.

Is there any reason to actually talk about HDCP versions at all in the
UAPI doc? I'm starting to suspect that my assumptions are not right,
and the use of dual-terminology in the UAPI doc confuses me.

> Thanks for pointing it out. There is a scope to add all these
> information in the documentation. Which I will do it in the next version
> today.

Awesome, let's see.


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 v8 1/6] drm: Add Content protection type property

[Ramalingam C]

On 2019-07-09 at 16:26:31 +0300, Pekka Paalanen wrote:
> On Mon, 8 Jul 2019 14:42:29 +0530
> Ramalingam C <ramalingam.c@intel.com> wrote:
> 
> > On 2019-07-08 at 12:59:59 +0300, Pekka Paalanen wrote:
> > > On Mon, 8 Jul 2019 12:52:17 +0300
> > > Pekka Paalanen <ppaalanen@gmail.com> wrote:
> > >   
> > > > On Fri,  5 Jul 2019 06:16:37 +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]
> > > > > 
> > > > > 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           | 22 ++++++++++++++
> > > > >  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, 82 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..17aef88c03a6 100644
> > > > > --- a/drivers/gpu/drm/drm_connector.c
> > > > > +++ b/drivers/gpu/drm/drm_connector.c
> > > > > @@ -951,6 +951,28 @@ static const struct drm_prop_enum_list hdmi_colorspaces[] = {
> > > > >   *	  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 property is used by the userspace to configure the kernel with
> > > > > + *	to be displayed stream's content type. 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 Type0 streams can be transmitted on a link which is
> > > > > + *		encrypted with HDCP 1.4 or higher versions of HDCP(i.e HDCP2.2
> > > > > + *		and more).
> > > > > + *	  - "HDCP Type1": DRM_MODE_HDCP_CONTENT_TYPE1 = 1
> > > > > + *		HDCP Type1 streams can be transmitted on a link which is
> > > > > + *		encrypted only with HDCP 2.2. In future higher versions also
> > > > > + *		might support Type1 based on their spec.
> > > > > + *
> > > > > + *	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    
> > > > 
> > > > Hi,
> > > > 
> > > > I think this doc covers all my previous comments on this patch now. One
> > > > more thing, the wording here reads as:
> > > > - userspace configures the content type
> > > > - the kernel transmits the content if the link satisfies the type
> > > >   requirement
> > > > - if the link does not satisfy the type requirement, the kernel will
> > > >   not transmit the content
> > > > 
> > > > This is of course false, the kernel transmits anyway, but that is how
> > > > the text reads from the "stream's content type" and "can be transmitted
> > > > on". The problem is, that a userspace developer will think the stream
> > > > is what he pushes into KMS, not what goes on the wire. The text also
> > > > magnifies that misconception because it sounds like the stream is
> > > > something different from the link. Actually, I don't understand what
> > > > "the stream" is supposed to be here.  
> > Stream is nothing but the any display content that needs the hdcp
> > protection.
> > > > 
> > > > Instead, I think you should explain how the content type property
> > > > guides the kernel driver's attempts in negotiating the link encryption
> > > > and how that then reflects in the content protection property (DESIRED
> > > > vs. ENABLED). It might be best to not say anything about any "stream".  
> > 
> > I will explain in different and more words, so that this questions wont
> > raise.
> > > 
> > > Btw. this UAPI has the following fundamental flaws:
> > > 
> > > - userspace cannot know which encryption is used on the link  
> > This claim is not true.
> > 
> > "HDCP content type" and "content protection" are used
> > together in
> > 	requesting the HDCP state
> > 	confirming that required state is achieved
> > For ex:
> > The state "HDCP content type" = "HDCP Type1" and "content protection" =
> > "DESIRED" stands for userspace has requested for HDCP Type 1 protection
> > from kernel.
> > 
> > When kernel changes the "content protection" to "ENABLED" when "HDCP
> > content type" is "HDCP Type1", kernel indicates the userspace that HDCP
> > authentication for Type 1 is successful.
> > 
> > I am not sure why do you think that userspace is not knowing link's
> > authentication state w.r.t type.
> 
> Hi,
> 
> if userspace asks for Type0, the kernel is free to use Type1 instead
> and switch "Content Protection" to "ENABLED". Userspace has no way of
> knowing that the link actually runs with Type1.
> 
> You can argue that it does not matter, because Type1 protection is
> stronger than Type0, so everybody should be happy, but that does not
> change the fact that userspace thinks wrong of what the protection
> really is.
> 
> > > - userspace cannot force Type0 if the driver succeeds enabling Type1  
> > When you set Type 0, kernel will authenticate the link for Type 0 only.
> 
> Really? Sorry, I have completely missed this. Please make it clear in
> UAPI and kAPI docs. This invalidates my comment above as well.
> 
> But you are also conflicting with your own doc, which said:
> 
> + *	  - "HDCP Type0": DRM_MODE_HDCP_CONTENT_TYPE0 = 0
> + *		HDCP Type0 streams can be transmitted on a link which is
> + *		encrypted with HDCP 1.4 or higher versions of HDCP(i.e HDCP2.2
> + *		and more).
> 
> To me that reads as: if userspace asks for Type0, the kernel is free to
> use Type0 or Type1 or anything stronger to say it succeeded.
> 
> Hence my confusion.
> 
> > Not for Type 1. I guess you are trying to say HDCP1.4 is not possible
> > over HDCP2.2.
> 
> I wouldn't know what that means.
> 
> > I dont see any reason why anyone want this except for testing.
> > 
> > Type 0 can be achieved through HDCP2.2 and HDCP1.4. And HDCP2.2 is attempted
> > first and HDCP1.4 will be attempted only if HDCP2.2 is failed for some
> > reasons. This is done because HDCP2.2 is preferred over 1.4 due to its
> > better crypto algo. 
> 
> I'm running with the assumption that:
> - Type0 == HDCP 1.4
> - Type1 == HDCP 2.2
> and I have no idea why you are sometimes talking about Type and
> sometimes about HDCP version.
This is a wrong assumption. Type 0 is not attached to HDCP1.4. Even
HDCP2.2 support Type 0 along with Type 1. Thats why when Type 0 is
requested Type 0  through HDCP2.2 will be attempted first. When that
fails, we go for HDCP1.4(Type 0 only)
> 
> Is there any reason to actually talk about HDCP versions at all in the
> UAPI doc? I'm starting to suspect that my assumptions are not right,
> and the use of dual-terminology in the UAPI doc confuses me.
In practical sense we dont need to mention the HDCP version in uAPI doc.
But I feel it is better to provide the understanding of the association
of the content type with HDCP version. May be we should try to avoid the
confusion :)

next version is closure in that direction i guess.

-Ram
> 
> > Thanks for pointing it out. There is a scope to add all these
> > information in the documentation. Which I will do it in the next version
> > today.
> 
> Awesome, let's see.
> 
> 
> Thanks,
> pq


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

[PATCH v8 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 v8 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 v8 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]

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

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

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

[Pekka Paalanen]

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

On Fri,  5 Jul 2019 06:16:40 +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]
> 
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/drm_hdcp.c | 34 ++++++++++++++++++++++++++++++++++
>  include/drm/drm_hdcp.h     |  2 ++
>  2 files changed, 36 insertions(+)
> 
> 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.
> + */

Hi,

this patch is not yet adding the documentation I asked for.

The uevent behaviour needs to be also documented with the Content
Protection property description. No userspace developer will know to
come to dig up these kernel-internal functions.

> +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));

Still missing WARN_ON(val == UNDESIRED) to prevent other drivers from
making that mistake in the future.


Thanks,
pq

> +	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 --]

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

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

[Ramalingam C]

On 2019-07-08 at 13:09:14 +0300, Pekka Paalanen wrote:
> On Fri,  5 Jul 2019 06:16:40 +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]
> > 
> > Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > ---
> >  drivers/gpu/drm/drm_hdcp.c | 34 ++++++++++++++++++++++++++++++++++
> >  include/drm/drm_hdcp.h     |  2 ++
> >  2 files changed, 36 insertions(+)
> > 
> > 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.

This para was rewritten to explain in which all are events uevents will be sent
and when events wont be sent.

> > + */
> 
> Hi,
> 
> this patch is not yet adding the documentation I asked for.
> 
> The uevent behaviour needs to be also documented with the Content
> Protection property description. No userspace developer will know to
> come to dig up these kernel-internal functions.

Sure this will help. I will do it. Thanks.

> 
> > +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));
> 
> Still missing WARN_ON(val == UNDESIRED) to prevent other drivers from
> making that mistake in the future.

IMHO This is not required. as kernel is not changing the "val" which is the
state requested by userspace tracked by "state->content_protection". So
this function will return as state->content_protection == val is true.

Thanks,
Ram.
> 
> 
> Thanks,
> pq
> 
> > +	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
> 


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

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

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

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

[Pekka Paalanen]

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

On Fri,  5 Jul 2019 06:16:42 +0530
Ramalingam C <ramalingam.c@intel.com> wrote:

> 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.
>   */

Hi,

this look good, publicly accessible spec links even. I'm happy with
this, but I repeat that the Weston work[1] does not directly prove this
UAPI (perhaps not necessary either?).


[1] https://gitlab.freedesktop.org/wayland/weston/merge_requests/48

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 v8 6/6] drm/hdcp: reference for srm file format

[Ramalingam C]

On 2019-07-08 at 13:16:23 +0300, Pekka Paalanen wrote:
> On Fri,  5 Jul 2019 06:16:42 +0530
> Ramalingam C <ramalingam.c@intel.com> wrote:
> 
> > 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.
> >   */
> 
> Hi,
> 
> this look good, publicly accessible spec links even. I'm happy with
> this, but I repeat that the Weston work[1] does not directly prove this
> UAPI (perhaps not necessary either?).
Thanks for helping to improve this part.

-Ram
> 
> 
> [1] https://gitlab.freedesktop.org/wayland/weston/merge_requests/48
> 
> Thanks,
> pq


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

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

[Patchwork]

== Series Details ==

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

== Summary ==

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

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

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

total: 0 errors, 0 warnings, 3 checks, 165 lines checked
acf44a9d064f drm/i915: Attach content type property
dbaa63e3c359 drm: uevent for connector status change
02c9c78d915a drm/hdcp: update content protection property with uevent
-:68: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#68: 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, 49 lines checked
ae26d53cd3e2 drm/i915: update the hdcp state with uevent
c96deafcd854 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: failure for HDCP2.2 Phase II (rev10)

[Patchwork]

== Series Details ==

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

== Summary ==

CI Bug Log - changes from CI_DRM_6422 -> Patchwork_13541
====================================================

Summary
-------

  **FAILURE**

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

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

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@gem_workarounds@basic-read:
    - fi-icl-dsi:         NOTRUN -> [SKIP][1] +1 similar issue
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-icl-dsi/igt@gem_workarounds@basic-read.html
    - fi-cml-u2:          [PASS][2] -> [SKIP][3]
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6422/fi-cml-u2/igt@gem_workarounds@basic-read.html
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-cml-u2/igt@gem_workarounds@basic-read.html
    - fi-icl-u3:          NOTRUN -> [SKIP][4]
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-icl-u3/igt@gem_workarounds@basic-read.html
    - fi-icl-u2:          [PASS][5] -> [SKIP][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6422/fi-icl-u2/igt@gem_workarounds@basic-read.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-icl-u2/igt@gem_workarounds@basic-read.html

  * {igt@kms_content_protection@srm} (NEW):
    - fi-cfl-8109u:       NOTRUN -> [FAIL][7]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-cfl-8109u/igt@kms_content_protection@srm.html
    - {fi-icl-u4}:        NOTRUN -> [SKIP][8]
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-icl-u4/igt@kms_content_protection@srm.html
    - fi-skl-lmem:        NOTRUN -> [FAIL][9]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-skl-lmem/igt@kms_content_protection@srm.html
    - fi-apl-guc:         NOTRUN -> [FAIL][10]
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-apl-guc/igt@kms_content_protection@srm.html
    - fi-skl-gvtdvm:      NOTRUN -> [FAIL][11]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-skl-gvtdvm/igt@kms_content_protection@srm.html
    - fi-cml-u2:          NOTRUN -> [SKIP][12]
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-cml-u2/igt@kms_content_protection@srm.html

  
#### Warnings ####

  * igt@gem_workarounds@basic-read:
    - fi-hsw-4770r:       [SKIP][13] ([fdo#109271]) -> [FAIL][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6422/fi-hsw-4770r/igt@gem_workarounds@basic-read.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-hsw-4770r/igt@gem_workarounds@basic-read.html
    - fi-snb-2600:        [SKIP][15] ([fdo#109271]) -> [FAIL][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6422/fi-snb-2600/igt@gem_workarounds@basic-read.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-snb-2600/igt@gem_workarounds@basic-read.html
    - fi-ivb-3770:        [SKIP][17] ([fdo#109271]) -> [FAIL][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6422/fi-ivb-3770/igt@gem_workarounds@basic-read.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-ivb-3770/igt@gem_workarounds@basic-read.html
    - fi-byt-n2820:       [SKIP][19] ([fdo#109271]) -> [FAIL][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6422/fi-byt-n2820/igt@gem_workarounds@basic-read.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-byt-n2820/igt@gem_workarounds@basic-read.html
    - fi-hsw-4770:        [SKIP][21] ([fdo#109271]) -> [FAIL][22]
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6422/fi-hsw-4770/igt@gem_workarounds@basic-read.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-hsw-4770/igt@gem_workarounds@basic-read.html
    - fi-byt-j1900:       [SKIP][23] ([fdo#109271]) -> [FAIL][24]
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6422/fi-byt-j1900/igt@gem_workarounds@basic-read.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-byt-j1900/igt@gem_workarounds@basic-read.html
    - fi-gdg-551:         [SKIP][25] ([fdo#109271]) -> [FAIL][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6422/fi-gdg-551/igt@gem_workarounds@basic-read.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-gdg-551/igt@gem_workarounds@basic-read.html
    - fi-blb-e6850:       [SKIP][27] ([fdo#109271]) -> [FAIL][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6422/fi-blb-e6850/igt@gem_workarounds@basic-read.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-blb-e6850/igt@gem_workarounds@basic-read.html
    - fi-elk-e7500:       [SKIP][29] ([fdo#109271]) -> [FAIL][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6422/fi-elk-e7500/igt@gem_workarounds@basic-read.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-elk-e7500/igt@gem_workarounds@basic-read.html
    - fi-ilk-650:         [SKIP][31] ([fdo#109271]) -> [FAIL][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6422/fi-ilk-650/igt@gem_workarounds@basic-read.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-ilk-650/igt@gem_workarounds@basic-read.html
    - fi-pnv-d510:        [SKIP][33] ([fdo#109271]) -> [FAIL][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6422/fi-pnv-d510/igt@gem_workarounds@basic-read.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-pnv-d510/igt@gem_workarounds@basic-read.html
    - fi-snb-2520m:       [SKIP][35] ([fdo#109271]) -> [FAIL][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6422/fi-snb-2520m/igt@gem_workarounds@basic-read.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-snb-2520m/igt@gem_workarounds@basic-read.html
    - fi-bwr-2160:        [SKIP][37] ([fdo#109271]) -> [FAIL][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6422/fi-bwr-2160/igt@gem_workarounds@basic-read.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-bwr-2160/igt@gem_workarounds@basic-read.html
    - fi-hsw-peppy:       [SKIP][39] ([fdo#109271]) -> [FAIL][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6422/fi-hsw-peppy/igt@gem_workarounds@basic-read.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-hsw-peppy/igt@gem_workarounds@basic-read.html

  
#### Suppressed ####

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

  * igt@gem_workarounds@basic-read:
    - {fi-icl-u4}:        [PASS][41] -> [SKIP][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6422/fi-icl-u4/igt@gem_workarounds@basic-read.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-icl-u4/igt@gem_workarounds@basic-read.html

  
New tests
---------

  New tests have been introduced between CI_DRM_6422 and Patchwork_13541:

### New IGT tests (1) ###

  * igt@kms_content_protection@srm:
    - Statuses : 4 fail(s) 7 pass(s) 33 skip(s)
    - Exec time: [0.0, 127.83] s

  

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_workarounds@basic-read:
    - fi-skl-6260u:       [PASS][43] -> [SKIP][44] ([fdo#109271])
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6422/fi-skl-6260u/igt@gem_workarounds@basic-read.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-skl-6260u/igt@gem_workarounds@basic-read.html
    - fi-skl-lmem:        [PASS][45] -> [SKIP][46] ([fdo#109271])
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6422/fi-skl-lmem/igt@gem_workarounds@basic-read.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-skl-lmem/igt@gem_workarounds@basic-read.html
    - fi-skl-6700k2:      [PASS][47] -> [SKIP][48] ([fdo#109271])
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6422/fi-skl-6700k2/igt@gem_workarounds@basic-read.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-skl-6700k2/igt@gem_workarounds@basic-read.html
    - fi-bsw-n3050:       [PASS][49] -> [SKIP][50] ([fdo#109271])
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6422/fi-bsw-n3050/igt@gem_workarounds@basic-read.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-bsw-n3050/igt@gem_workarounds@basic-read.html
    - fi-skl-6600u:       [PASS][51] -> [SKIP][52] ([fdo#109271])
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6422/fi-skl-6600u/igt@gem_workarounds@basic-read.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-skl-6600u/igt@gem_workarounds@basic-read.html
    - fi-apl-guc:         [PASS][53] -> [SKIP][54] ([fdo#109271])
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6422/fi-apl-guc/igt@gem_workarounds@basic-read.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-apl-guc/igt@gem_workarounds@basic-read.html
    - fi-bxt-dsi:         [PASS][55] -> [SKIP][56] ([fdo#109271])
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6422/fi-bxt-dsi/igt@gem_workarounds@basic-read.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-bxt-dsi/igt@gem_workarounds@basic-read.html
    - fi-kbl-8809g:       [PASS][57] -> [SKIP][58] ([fdo#109271])
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6422/fi-kbl-8809g/igt@gem_workarounds@basic-read.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-kbl-8809g/igt@gem_workarounds@basic-read.html
    - fi-kbl-guc:         [PASS][59] -> [SKIP][60] ([fdo#109271])
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6422/fi-kbl-guc/igt@gem_workarounds@basic-read.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-kbl-guc/igt@gem_workarounds@basic-read.html
    - fi-kbl-7500u:       [PASS][61] -> [SKIP][62] ([fdo#109271])
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6422/fi-kbl-7500u/igt@gem_workarounds@basic-read.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-kbl-7500u/igt@gem_workarounds@basic-read.html
    - fi-bxt-j4205:       [PASS][63] -> [SKIP][64] ([fdo#109271])
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6422/fi-bxt-j4205/igt@gem_workarounds@basic-read.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-bxt-j4205/igt@gem_workarounds@basic-read.html
    - fi-kbl-x1275:       [PASS][65] -> [SKIP][66] ([fdo#109271])
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6422/fi-kbl-x1275/igt@gem_workarounds@basic-read.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-kbl-x1275/igt@gem_workarounds@basic-read.html
    - fi-skl-gvtdvm:      [PASS][67] -> [SKIP][68] ([fdo#109271])
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6422/fi-skl-gvtdvm/igt@gem_workarounds@basic-read.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-skl-gvtdvm/igt@gem_workarounds@basic-read.html
    - fi-glk-dsi:         [PASS][69] -> [SKIP][70] ([fdo#109271])
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6422/fi-glk-dsi/igt@gem_workarounds@basic-read.html
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-glk-dsi/igt@gem_workarounds@basic-read.html
    - fi-skl-iommu:       [PASS][71] -> [SKIP][72] ([fdo#109271])
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6422/fi-skl-iommu/igt@gem_workarounds@basic-read.html
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-skl-iommu/igt@gem_workarounds@basic-read.html
    - fi-cfl-guc:         [PASS][73] -> [SKIP][74] ([fdo#109271])
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6422/fi-cfl-guc/igt@gem_workarounds@basic-read.html
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-cfl-guc/igt@gem_workarounds@basic-read.html
    - fi-whl-u:           [PASS][75] -> [SKIP][76] ([fdo#109271])
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6422/fi-whl-u/igt@gem_workarounds@basic-read.html
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-whl-u/igt@gem_workarounds@basic-read.html
    - fi-cfl-8700k:       [PASS][77] -> [SKIP][78] ([fdo#109271])
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6422/fi-cfl-8700k/igt@gem_workarounds@basic-read.html
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-cfl-8700k/igt@gem_workarounds@basic-read.html
    - fi-cfl-8109u:       [PASS][79] -> [SKIP][80] ([fdo#109271])
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6422/fi-cfl-8109u/igt@gem_workarounds@basic-read.html
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-cfl-8109u/igt@gem_workarounds@basic-read.html
    - fi-skl-guc:         [PASS][81] -> [SKIP][82] ([fdo#109271])
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6422/fi-skl-guc/igt@gem_workarounds@basic-read.html
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-skl-guc/igt@gem_workarounds@basic-read.html
    - fi-kbl-7567u:       [PASS][83] -> [SKIP][84] ([fdo#109271])
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6422/fi-kbl-7567u/igt@gem_workarounds@basic-read.html
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-kbl-7567u/igt@gem_workarounds@basic-read.html
    - fi-kbl-r:           [PASS][85] -> [SKIP][86] ([fdo#109271])
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6422/fi-kbl-r/igt@gem_workarounds@basic-read.html
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-kbl-r/igt@gem_workarounds@basic-read.html
    - fi-bdw-5557u:       [PASS][87] -> [SKIP][88] ([fdo#109271])
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6422/fi-bdw-5557u/igt@gem_workarounds@basic-read.html
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-bdw-5557u/igt@gem_workarounds@basic-read.html
    - fi-bdw-gvtdvm:      [PASS][89] -> [SKIP][90] ([fdo#109271])
   [89]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6422/fi-bdw-gvtdvm/igt@gem_workarounds@basic-read.html
   [90]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-bdw-gvtdvm/igt@gem_workarounds@basic-read.html

  * igt@i915_module_load@reload-with-fault-injection:
    - fi-skl-6770hq:      [PASS][91] -> [DMESG-WARN][92] ([fdo#108529]) +2 similar issues
   [91]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6422/fi-skl-6770hq/igt@i915_module_load@reload-with-fault-injection.html
   [92]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-skl-6770hq/igt@i915_module_load@reload-with-fault-injection.html

  * igt@i915_selftest@live_contexts:
    - fi-skl-iommu:       [PASS][93] -> [INCOMPLETE][94] ([fdo#111050])
   [93]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6422/fi-skl-iommu/igt@i915_selftest@live_contexts.html
   [94]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-skl-iommu/igt@i915_selftest@live_contexts.html

  * igt@i915_selftest@live_hangcheck:
    - fi-icl-u2:          [PASS][95] -> [INCOMPLETE][96] ([fdo#107713] / [fdo#108569])
   [95]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6422/fi-icl-u2/igt@i915_selftest@live_hangcheck.html
   [96]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-icl-u2/igt@i915_selftest@live_hangcheck.html
    - fi-kbl-7500u:       [PASS][97] -> [DMESG-WARN][98] ([fdo#111074])
   [97]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6422/fi-kbl-7500u/igt@i915_selftest@live_hangcheck.html
   [98]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-kbl-7500u/igt@i915_selftest@live_hangcheck.html

  * igt@kms_flip@basic-flip-vs-dpms:
    - fi-skl-6770hq:      [PASS][99] -> [SKIP][100] ([fdo#109271]) +24 similar issues
   [99]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6422/fi-skl-6770hq/igt@kms_flip@basic-flip-vs-dpms.html
   [100]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-skl-6770hq/igt@kms_flip@basic-flip-vs-dpms.html

  
#### Possible fixes ####

  * igt@gem_basic@create-close:
    - fi-icl-u3:          [DMESG-WARN][101] ([fdo#107724]) -> [PASS][102]
   [101]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6422/fi-icl-u3/igt@gem_basic@create-close.html
   [102]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-icl-u3/igt@gem_basic@create-close.html

  * igt@kms_frontbuffer_tracking@basic:
    - fi-hsw-peppy:       [DMESG-WARN][103] ([fdo#102614]) -> [PASS][104]
   [103]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6422/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html
   [104]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html
    - fi-icl-u2:          [FAIL][105] ([fdo#103167]) -> [PASS][106]
   [105]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6422/fi-icl-u2/igt@kms_frontbuffer_tracking@basic.html
   [106]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13541/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#102505]: https://bugs.freedesktop.org/show_bug.cgi?id=102505
  [fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#108529]: https://bugs.freedesktop.org/show_bug.cgi?id=108529
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#110566]: https://bugs.freedesktop.org/show_bug.cgi?id=110566
  [fdo#111045]: https://bugs.freedesktop.org/show_bug.cgi?id=111045
  [fdo#111049]: https://bugs.freedesktop.org/show_bug.cgi?id=111049
  [fdo#111050]: https://bugs.freedesktop.org/show_bug.cgi?id=111050
  [fdo#111074]: https://bugs.freedesktop.org/show_bug.cgi?id=111074


Participating hosts (52 -> 46)
------------------------------

  Additional (1): fi-icl-dsi 
  Missing    (7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-y fi-byt-clapper fi-bdw-samus 


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

  * IGT: IGT_5084 -> IGTPW_3237
  * Linux: CI_DRM_6422 -> Patchwork_13541

  CI_DRM_6422: 830a8c536c3a1c62c30447c646735cfebb19c266 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_3237: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3237/
  IGT_5084: 9f45069f9b5136d07e053d8086e8df51e14332eb @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13541: c96deafcd8545fabb7eacb5b220acb45ddc42ff0 @ 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_13541/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 ==

c96deafcd854 drm/hdcp: reference for srm file format
ae26d53cd3e2 drm/i915: update the hdcp state with uevent
02c9c78d915a drm/hdcp: update content pr

== Logs ==

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