[PATCH v3 00/10] HDCP2.2 Phase II

[PATCH v3 00/10] HDCP2.2 Phase II

[Ramalingam C]

HDCP2.2 phase-II introduces below features:
	Addition of three connector properties
		HDCP Content Type
		HDCP Topology
	Addition of binary sysfs "hdcp_srm"
	parsing for HDCP1.4 and 2.2 SRM table
	Once HDCP1.4/2.2 authentication is completed gathering the all
		downstream topology for userspace 
	Extending debugfs entry to provide the HDCP2.2 capability too.
	uevent for HDCP state change.

HDCP Content Type:
	This property is used to indicate the content type
classification of a stream. Which indicate the HDCP version required
for the rendering of that streams. This conten type is one of the
parameter in the HDCP2.2 authentication flow, as even downstream
repeaters will mandate the HDCP version requirement.

Two values possible for content type of a stream:
	Type 0: Stream can be rendered only on HDCP encrypted link no
		restriction on HDCP versions.
	Type 1: Stream can be rendered only on HDCP2.2 encrypted link.

There is a parallel effort in #wayland community to add the support for
HDCP2.2 along with content type support. Patches are under review in
#wayland community.

HDCP Topology:
This blob property is used by the kernel to pass the downstream topology
of the HDCP encrypted port to the userspace.

This is used by the userspace to implement the HDCP repeater, which KMD
implementing the HDCP transmitters(downstream ports) and userspace
implementing the upstream port(HDCP receiver).

Discussion is on going to add the downstream_info support in the
weston HDCP stack.

hdcp_srm: write only binary sysfs used by the userspace to pass the SRM
table of HDCP1.4 and 2.2. These are nothing but revocated list of
receiver IDs of the HDCP sinks. KMD will use this list to identify the
revocated devices in the HDCP authentication and deny the hdcp encryption to it.

Daniel has suggested about moving the SRM node implementation into DRM core.
Still dome more clarification is required. Once that is done another
respin on SRM patches are expected.


v2:
  srm is passed through binary sysfs [Daniel]
  CP abbreviation is expanded except for downstream_info [Daniel]
  restrictions at atomic_set_property is removed [Maarten]
  upon content type change durin encryption, HDCP is restarted within
	kernel [Maarten]
v3:
  property names are reworked [Pekka and Daniel]
  uevent is generated for HDCP state change. [Pekka and Daniel]
 
Series can be cloned from github
https://github.com/ramalingampc2008/drm-tip.git hdcp2_2_p2_v3

Test-with: <20190321174444.10099-1-ramalingam.c@intel.com>

Ramalingam C (10):
  drm/i915: debugfs: HDCP2.2 capability read
  drm: Add Content protection type property
  drm/i915: Attach content type property
  drm/i915: HDCP SRM parsing and revocation check
  drm/i915/sysfs: Node for hdcp srm
  drm: Add CP downstream_info property
  drm/i915: Populate downstream info for HDCP1.4
  drm/i915: Populate downstream info for HDCP2.2
  drm: uevent for connector status change
  drm/i915: uevent for HDCP status change

 drivers/gpu/drm/drm_atomic_uapi.c   |   8 +
 drivers/gpu/drm/drm_connector.c     | 149 ++++++++++
 drivers/gpu/drm/drm_sysfs.c         |  28 ++
 drivers/gpu/drm/i915/i915_debugfs.c |  13 +-
 drivers/gpu/drm/i915/i915_drv.c     |   1 +
 drivers/gpu/drm/i915/i915_drv.h     |   6 +
 drivers/gpu/drm/i915/i915_sysfs.c   |  32 +++
 drivers/gpu/drm/i915/intel_ddi.c    |  21 +-
 drivers/gpu/drm/i915/intel_drv.h    |   7 +-
 drivers/gpu/drm/i915/intel_hdcp.c   | 407 ++++++++++++++++++++++++++--
 include/drm/drm_connector.h         |  27 ++
 include/drm/drm_hdcp.h              |  33 +++
 include/drm/drm_sysfs.h             |   5 +-
 include/uapi/drm/drm_mode.h         |  39 +++
 14 files changed, 743 insertions(+), 33 deletions(-)

-- 
2.19.1

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

[PATCH v3 01/10] drm/i915: debugfs: HDCP2.2 capability read

[Ramalingam C]

Adding the HDCP2.2 capability of HDCP src and sink info into debugfs
entry "i915_hdcp_sink_capability"

This helps the userspace tests to skip the HDCP2.2 test on non HDCP2.2
sinks.

Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 13 +++++++++++--
 drivers/gpu/drm/i915/intel_drv.h    |  1 +
 drivers/gpu/drm/i915/intel_hdcp.c   |  2 +-
 3 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 08683dca7775..912f30e8e01a 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -4751,6 +4751,7 @@ static int i915_hdcp_sink_capability_show(struct seq_file *m, void *data)
 {
 	struct drm_connector *connector = m->private;
 	struct intel_connector *intel_connector = to_intel_connector(connector);
+	bool hdcp_cap, hdcp2_cap;
 
 	if (connector->status != connector_status_connected)
 		return -ENODEV;
@@ -4761,8 +4762,16 @@ static int i915_hdcp_sink_capability_show(struct seq_file *m, void *data)
 
 	seq_printf(m, "%s:%d HDCP version: ", connector->name,
 		   connector->base.id);
-	seq_printf(m, "%s ", !intel_hdcp_capable(intel_connector) ?
-		   "None" : "HDCP1.4");
+	hdcp_cap = intel_hdcp_capable(intel_connector);
+	hdcp2_cap = intel_hdcp2_capable(intel_connector);
+
+	if (hdcp_cap)
+		seq_puts(m, "HDCP1.4 ");
+	if (hdcp2_cap)
+		seq_puts(m, "HDCP2.2 ");
+
+	if (!hdcp_cap && !hdcp2_cap)
+		seq_puts(m, "None");
 	seq_puts(m, "\n");
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 4d7ae579fc92..a37a4477994c 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -2182,6 +2182,7 @@ int intel_hdcp_enable(struct intel_connector *connector);
 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);
+bool intel_hdcp2_capable(struct intel_connector *connector);
 void intel_hdcp_component_init(struct drm_i915_private *dev_priv);
 void intel_hdcp_component_fini(struct drm_i915_private *dev_priv);
 void intel_hdcp_cleanup(struct intel_connector *connector);
diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
index 9ce09f67776d..ff9497e5c591 100644
--- a/drivers/gpu/drm/i915/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/intel_hdcp.c
@@ -76,7 +76,7 @@ bool intel_hdcp_capable(struct intel_connector *connector)
 }
 
 /* Is HDCP2.2 capable on Platform and Sink */
-static bool intel_hdcp2_capable(struct intel_connector *connector)
+bool intel_hdcp2_capable(struct intel_connector *connector)
 {
 	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
 	struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector);
-- 
2.19.1

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

Re: [PATCH v3 01/10] drm/i915: debugfs: HDCP2.2 capability read

[Daniel Vetter]

On Fri, Mar 22, 2019 at 06:14:39AM +0530, Ramalingam C wrote:
> Adding the HDCP2.2 capability of HDCP src and sink info into debugfs
> entry "i915_hdcp_sink_capability"
> 
> This helps the userspace tests to skip the HDCP2.2 test on non HDCP2.2
> sinks.
> 
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>

Still not a fan of exposing stuff to debugfs, but can't really do better.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 13 +++++++++++--
>  drivers/gpu/drm/i915/intel_drv.h    |  1 +
>  drivers/gpu/drm/i915/intel_hdcp.c   |  2 +-
>  3 files changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 08683dca7775..912f30e8e01a 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -4751,6 +4751,7 @@ static int i915_hdcp_sink_capability_show(struct seq_file *m, void *data)
>  {
>  	struct drm_connector *connector = m->private;
>  	struct intel_connector *intel_connector = to_intel_connector(connector);
> +	bool hdcp_cap, hdcp2_cap;
>  
>  	if (connector->status != connector_status_connected)
>  		return -ENODEV;
> @@ -4761,8 +4762,16 @@ static int i915_hdcp_sink_capability_show(struct seq_file *m, void *data)
>  
>  	seq_printf(m, "%s:%d HDCP version: ", connector->name,
>  		   connector->base.id);
> -	seq_printf(m, "%s ", !intel_hdcp_capable(intel_connector) ?
> -		   "None" : "HDCP1.4");
> +	hdcp_cap = intel_hdcp_capable(intel_connector);
> +	hdcp2_cap = intel_hdcp2_capable(intel_connector);
> +
> +	if (hdcp_cap)
> +		seq_puts(m, "HDCP1.4 ");
> +	if (hdcp2_cap)
> +		seq_puts(m, "HDCP2.2 ");
> +
> +	if (!hdcp_cap && !hdcp2_cap)
> +		seq_puts(m, "None");
>  	seq_puts(m, "\n");
>  
>  	return 0;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 4d7ae579fc92..a37a4477994c 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -2182,6 +2182,7 @@ int intel_hdcp_enable(struct intel_connector *connector);
>  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);
> +bool intel_hdcp2_capable(struct intel_connector *connector);
>  void intel_hdcp_component_init(struct drm_i915_private *dev_priv);
>  void intel_hdcp_component_fini(struct drm_i915_private *dev_priv);
>  void intel_hdcp_cleanup(struct intel_connector *connector);
> diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
> index 9ce09f67776d..ff9497e5c591 100644
> --- a/drivers/gpu/drm/i915/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/intel_hdcp.c
> @@ -76,7 +76,7 @@ bool intel_hdcp_capable(struct intel_connector *connector)
>  }
>  
>  /* Is HDCP2.2 capable on Platform and Sink */
> -static bool intel_hdcp2_capable(struct intel_connector *connector)
> +bool intel_hdcp2_capable(struct intel_connector *connector)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>  	struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector);
> -- 
> 2.19.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v3 02/10] 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.

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]

Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
---
 drivers/gpu/drm/drm_atomic_uapi.c |  4 ++
 drivers/gpu/drm/drm_connector.c   | 63 +++++++++++++++++++++++++++++++
 include/drm/drm_connector.h       | 15 ++++++++
 include/uapi/drm/drm_mode.h       |  4 ++
 4 files changed, 86 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index 4eb81f10bc54..857ca6fa6fd0 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -746,6 +746,8 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
 			return -EINVAL;
 		}
 		state->content_protection = val;
+	} else if (property == connector->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) {
@@ -822,6 +824,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
 		*val = state->scaling_mode;
 	} else if (property == connector->content_protection_property) {
 		*val = state->content_protection;
+	} else if (property == connector->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 2355124849db..ff61c3208307 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -857,6 +857,13 @@ static const struct drm_prop_enum_list hdmi_colorspaces[] = {
 	{ DRM_MODE_COLORIMETRY_DCI_P3_RGB_THEATER, "DCI-P3_RGB_Theater" },
 };
 
+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)
+
 /**
  * DOC: standard connector properties
  *
@@ -962,6 +969,23 @@ 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
+ *	upcoming 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:
+ *	  - DRM_MODE_HDCP_CONTENT_TYPE0 = 0
+ *		HDCP Type0 streams can be transmitted on a link which is
+ *		encrypted with HDCP 1.4 or HDCP 2.2.
+ *	  - DRM_MODE_HDCP_CONTENT_TYPE1 = 1
+ *		HDCP Type1 streams can be transmitted on a link which is
+ *		encrypted only with HDCP 2.2.
+ *
+ *	Please note this content type is introduced at HDCP 2.2 and used in its
+ *	authentication process. 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
  *
  * max bpc:
  *	This range property is used by userspace to limit the bit depth. When
@@ -1551,6 +1575,45 @@ int drm_connector_attach_content_protection_property(
 }
 EXPORT_SYMBOL(drm_connector_attach_content_protection_property);
 
+/**
+ * drm_connector_attach_hdcp_content_type_property - attach HDCP
+ * content type property
+ *
+ * @connector: connector to attach HDCP content type property on.
+ *
+ * This is used to add support 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.
+ *
+ * This information will be used during the HDCP 2.2 authentication.
+ *
+ * Content type will be set to &drm_connector_state.hdcp_content_type.
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+int
+drm_connector_attach_hdcp_content_type_property(struct drm_connector *
+						      connector)
+{
+	struct drm_device *dev = connector->dev;
+	struct drm_property *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);
+	connector->hdcp_content_type_property = prop;
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_connector_attach_hdcp_content_type_property);
+
 /**
  * drm_mode_create_aspect_ratio_property - create aspect ratio property
  * @dev: DRM device
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index c8061992d6cb..f0830985367f 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -518,6 +518,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.
@@ -1035,6 +1041,12 @@ struct drm_connector {
 	 */
 	struct drm_property *colorspace_property;
 
+	/**
+	 * @hdcp_content_type_property: DRM ENUM property for type of
+	 * Protected Content.
+	 */
+	struct drm_property *hdcp_content_type_property;
+
 	/**
 	 * @path_blob_ptr:
 	 *
@@ -1294,6 +1306,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);
@@ -1309,6 +1322,8 @@ int drm_connector_attach_vrr_capable_property(
 		struct drm_connector *connector);
 int drm_connector_attach_content_protection_property(
 		struct drm_connector *connector);
+int drm_connector_attach_hdcp_content_type_property(
+		struct drm_connector *connector);
 int drm_mode_create_aspect_ratio_property(struct drm_device *dev);
 int drm_mode_create_colorspace_property(struct drm_connector *connector);
 int drm_mode_create_content_type_property(struct drm_device *dev);
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index a439c2e67896..44412e8b77cd 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -210,6 +210,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 v3 02/10] drm: Add Content protection type property

[Daniel Vetter]

On Fri, Mar 22, 2019 at 06:14:40AM +0530, Ramalingam C wrote:
> This patch adds a DRM ENUM property to the selected connectors.
> This property is used for mentioning the protected content's type
> from userspace to kernel HDCP authentication.
> 
> Type of the stream is decided by the protected content providers.
> Type 0 content can be rendered on any HDCP protected display wires.
> But Type 1 content can be rendered only on HDCP2.2 protected paths.
> 
> So when a userspace sets this property to Type 1 and starts the HDCP
> enable, kernel will honour it only if HDCP2.2 authentication is through
> for type 1. Else HDCP enable will be failed.
> 
> 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]
> 
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> ---
>  drivers/gpu/drm/drm_atomic_uapi.c |  4 ++
>  drivers/gpu/drm/drm_connector.c   | 63 +++++++++++++++++++++++++++++++
>  include/drm/drm_connector.h       | 15 ++++++++
>  include/uapi/drm/drm_mode.h       |  4 ++
>  4 files changed, 86 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index 4eb81f10bc54..857ca6fa6fd0 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -746,6 +746,8 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
>  			return -EINVAL;
>  		}
>  		state->content_protection = val;
> +	} else if (property == connector->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) {
> @@ -822,6 +824,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
>  		*val = state->scaling_mode;
>  	} else if (property == connector->content_protection_property) {
>  		*val = state->content_protection;
> +	} else if (property == connector->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 2355124849db..ff61c3208307 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -857,6 +857,13 @@ static const struct drm_prop_enum_list hdmi_colorspaces[] = {
>  	{ DRM_MODE_COLORIMETRY_DCI_P3_RGB_THEATER, "DCI-P3_RGB_Theater" },
>  };
>  
> +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)
> +
>  /**
>   * DOC: standard connector properties
>   *
> @@ -962,6 +969,23 @@ 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
> + *	upcoming 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:
> + *	  - DRM_MODE_HDCP_CONTENT_TYPE0 = 0
> + *		HDCP Type0 streams can be transmitted on a link which is
> + *		encrypted with HDCP 1.4 or HDCP 2.2.
> + *	  - DRM_MODE_HDCP_CONTENT_TYPE1 = 1
> + *		HDCP Type1 streams can be transmitted on a link which is
> + *		encrypted only with HDCP 2.2.
> + *
> + *	Please note this content type is introduced at HDCP 2.2 and used in its
> + *	authentication process. 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
>   *
>   * max bpc:
>   *	This range property is used by userspace to limit the bit depth. When
> @@ -1551,6 +1575,45 @@ int drm_connector_attach_content_protection_property(
>  }
>  EXPORT_SYMBOL(drm_connector_attach_content_protection_property);
>  
> +/**
> + * drm_connector_attach_hdcp_content_type_property - attach HDCP
> + * content type property
> + *
> + * @connector: connector to attach HDCP content type property on.
> + *
> + * This is used to add support 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.
> + *
> + * This information will be used during the HDCP 2.2 authentication.
> + *
> + * Content type will be set to &drm_connector_state.hdcp_content_type.
> + *
> + * Returns:
> + * Zero on success, negative errno on failure.
> + */
> +int
> +drm_connector_attach_hdcp_content_type_property(struct drm_connector *
> +						      connector)
> +{
> +	struct drm_device *dev = connector->dev;
> +	struct drm_property *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 we always create the same property, then the usual pattern is to have a
global one stored in dev->mode_config, created at init time, and only
attach it. You can attach the same property to multiple objects, and all
objects will have distinct values for that property.

drm_property = metadata describing the name/value range/..., _not_ the value itself

I guess would be good to address that with content protection property
too.

Also, not sure we need 2 functions for this, I'd just add a bool
enable_content_type to drm_connector_attach_content_protection_property(),
for a simpler interface in drivers.


> +	if (!prop)
> +		return -ENOMEM;
> +
> +	drm_object_attach_property(&connector->base, prop,
> +				   DRM_MODE_HDCP_CONTENT_TYPE0);
> +	connector->hdcp_content_type_property = prop;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_connector_attach_hdcp_content_type_property);
> +
>  /**
>   * drm_mode_create_aspect_ratio_property - create aspect ratio property
>   * @dev: DRM device
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index c8061992d6cb..f0830985367f 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -518,6 +518,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.
> @@ -1035,6 +1041,12 @@ struct drm_connector {
>  	 */
>  	struct drm_property *colorspace_property;
>  
> +	/**
> +	 * @hdcp_content_type_property: DRM ENUM property for type of
> +	 * Protected Content.
> +	 */
> +	struct drm_property *hdcp_content_type_property;

Please move these two next to the existing content protection stuff.

> +
>  	/**
>  	 * @path_blob_ptr:
>  	 *
> @@ -1294,6 +1306,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);
> @@ -1309,6 +1322,8 @@ int drm_connector_attach_vrr_capable_property(
>  		struct drm_connector *connector);
>  int drm_connector_attach_content_protection_property(
>  		struct drm_connector *connector);
> +int drm_connector_attach_hdcp_content_type_property(
> +		struct drm_connector *connector);
>  int drm_mode_create_aspect_ratio_property(struct drm_device *dev);
>  int drm_mode_create_colorspace_property(struct drm_connector *connector);
>  int drm_mode_create_content_type_property(struct drm_device *dev);
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index a439c2e67896..44412e8b77cd 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -210,6 +210,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;

Aside from the nits looks all good to me. Ofc we need an r-b/ack from
userspace people on the interface itself.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 02/10] drm: Add Content protection type property

[Ramalingam C]

On 2019-03-27 at 10:56:32 +0100, Daniel Vetter wrote:
> On Fri, Mar 22, 2019 at 06:14:40AM +0530, Ramalingam C wrote:
> > This patch adds a DRM ENUM property to the selected connectors.
> > This property is used for mentioning the protected content's type
> > from userspace to kernel HDCP authentication.
> > 
> > Type of the stream is decided by the protected content providers.
> > Type 0 content can be rendered on any HDCP protected display wires.
> > But Type 1 content can be rendered only on HDCP2.2 protected paths.
> > 
> > So when a userspace sets this property to Type 1 and starts the HDCP
> > enable, kernel will honour it only if HDCP2.2 authentication is through
> > for type 1. Else HDCP enable will be failed.
> > 
> > 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]
> > 
> > Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> > ---
> >  drivers/gpu/drm/drm_atomic_uapi.c |  4 ++
> >  drivers/gpu/drm/drm_connector.c   | 63 +++++++++++++++++++++++++++++++
> >  include/drm/drm_connector.h       | 15 ++++++++
> >  include/uapi/drm/drm_mode.h       |  4 ++
> >  4 files changed, 86 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> > index 4eb81f10bc54..857ca6fa6fd0 100644
> > --- a/drivers/gpu/drm/drm_atomic_uapi.c
> > +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> > @@ -746,6 +746,8 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
> >  			return -EINVAL;
> >  		}
> >  		state->content_protection = val;
> > +	} else if (property == connector->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) {
> > @@ -822,6 +824,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
> >  		*val = state->scaling_mode;
> >  	} else if (property == connector->content_protection_property) {
> >  		*val = state->content_protection;
> > +	} else if (property == connector->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 2355124849db..ff61c3208307 100644
> > --- a/drivers/gpu/drm/drm_connector.c
> > +++ b/drivers/gpu/drm/drm_connector.c
> > @@ -857,6 +857,13 @@ static const struct drm_prop_enum_list hdmi_colorspaces[] = {
> >  	{ DRM_MODE_COLORIMETRY_DCI_P3_RGB_THEATER, "DCI-P3_RGB_Theater" },
> >  };
> >  
> > +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)
> > +
> >  /**
> >   * DOC: standard connector properties
> >   *
> > @@ -962,6 +969,23 @@ 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
> > + *	upcoming 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:
> > + *	  - DRM_MODE_HDCP_CONTENT_TYPE0 = 0
> > + *		HDCP Type0 streams can be transmitted on a link which is
> > + *		encrypted with HDCP 1.4 or HDCP 2.2.
> > + *	  - DRM_MODE_HDCP_CONTENT_TYPE1 = 1
> > + *		HDCP Type1 streams can be transmitted on a link which is
> > + *		encrypted only with HDCP 2.2.
> > + *
> > + *	Please note this content type is introduced at HDCP 2.2 and used in its
> > + *	authentication process. 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
> >   *
> >   * max bpc:
> >   *	This range property is used by userspace to limit the bit depth. When
> > @@ -1551,6 +1575,45 @@ int drm_connector_attach_content_protection_property(
> >  }
> >  EXPORT_SYMBOL(drm_connector_attach_content_protection_property);
> >  
> > +/**
> > + * drm_connector_attach_hdcp_content_type_property - attach HDCP
> > + * content type property
> > + *
> > + * @connector: connector to attach HDCP content type property on.
> > + *
> > + * This is used to add support 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.
> > + *
> > + * This information will be used during the HDCP 2.2 authentication.
> > + *
> > + * Content type will be set to &drm_connector_state.hdcp_content_type.
> > + *
> > + * Returns:
> > + * Zero on success, negative errno on failure.
> > + */
> > +int
> > +drm_connector_attach_hdcp_content_type_property(struct drm_connector *
> > +						      connector)
> > +{
> > +	struct drm_device *dev = connector->dev;
> > +	struct drm_property *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 we always create the same property, then the usual pattern is to have a
> global one stored in dev->mode_config, created at init time, and only
> attach it. You can attach the same property to multiple objects, and all
> objects will have distinct values for that property.
> 
> drm_property = metadata describing the name/value range/..., _not_ the value itself
> 
> I guess would be good to address that with content protection property
> too.
Just aligned with existing content protection proeprty. I will address
the content protection property first and then I will add the property
for the HDCP content type and HDCP topology in the same fucntion.
> 
> Also, not sure we need 2 functions for this, I'd just add a bool
> enable_content_type to drm_connector_attach_content_protection_property(),
> for a simpler interface in drivers.
> 
> 
> > +	if (!prop)
> > +		return -ENOMEM;
> > +
> > +	drm_object_attach_property(&connector->base, prop,
> > +				   DRM_MODE_HDCP_CONTENT_TYPE0);
> > +	connector->hdcp_content_type_property = prop;
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL(drm_connector_attach_hdcp_content_type_property);
> > +
> >  /**
> >   * drm_mode_create_aspect_ratio_property - create aspect ratio property
> >   * @dev: DRM device
> > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> > index c8061992d6cb..f0830985367f 100644
> > --- a/include/drm/drm_connector.h
> > +++ b/include/drm/drm_connector.h
> > @@ -518,6 +518,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.
> > @@ -1035,6 +1041,12 @@ struct drm_connector {
> >  	 */
> >  	struct drm_property *colorspace_property;
> >  
> > +	/**
> > +	 * @hdcp_content_type_property: DRM ENUM property for type of
> > +	 * Protected Content.
> > +	 */
> > +	struct drm_property *hdcp_content_type_property;
> 
> Please move these two next to the existing content protection stuff.

some rebasing mistakes. Will address it.
> 
> > +
> >  	/**
> >  	 * @path_blob_ptr:
> >  	 *
> > @@ -1294,6 +1306,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);
> > @@ -1309,6 +1322,8 @@ int drm_connector_attach_vrr_capable_property(
> >  		struct drm_connector *connector);
> >  int drm_connector_attach_content_protection_property(
> >  		struct drm_connector *connector);
> > +int drm_connector_attach_hdcp_content_type_property(
> > +		struct drm_connector *connector);
> >  int drm_mode_create_aspect_ratio_property(struct drm_device *dev);
> >  int drm_mode_create_colorspace_property(struct drm_connector *connector);
> >  int drm_mode_create_content_type_property(struct drm_device *dev);
> > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> > index a439c2e67896..44412e8b77cd 100644
> > --- a/include/uapi/drm/drm_mode.h
> > +++ b/include/uapi/drm/drm_mode.h
> > @@ -210,6 +210,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;
> 
> Aside from the nits looks all good to me. Ofc we need an r-b/ack from
> userspace people on the interface itself.
Sure. Thanks a lot. Hopefully we will get it soon in weston.

-Ram
> -Daniel
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v3 03/10] 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.

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]

Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c  | 21 +++++++++++++++------
 drivers/gpu/drm/i915/intel_drv.h  |  2 +-
 drivers/gpu/drm/i915/intel_hdcp.c | 30 +++++++++++++++++++++++++-----
 3 files changed, 41 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 933df3a57a8a..43859f126b82 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -3495,7 +3495,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,
@@ -3558,21 +3559,29 @@ static void intel_ddi_update_pipe_dp(struct intel_encoder *encoder,
 	intel_panel_update_backlight(encoder, crtc_state, conn_state);
 }
 
-static void intel_ddi_update_pipe(struct intel_encoder *encoder,
+static void intel_ddi_update_hdcp(struct intel_encoder *encoder,
 				  const struct intel_crtc_state *crtc_state,
 				  const struct drm_connector_state *conn_state)
 {
-	if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
-		intel_ddi_update_pipe_dp(encoder, crtc_state, conn_state);
-
 	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);
 	else if (conn_state->content_protection ==
 		 DRM_MODE_CONTENT_PROTECTION_UNDESIRED)
 		intel_hdcp_disable(to_intel_connector(conn_state->connector));
 }
 
+static void intel_ddi_update_pipe(struct intel_encoder *encoder,
+				  const struct intel_crtc_state *crtc_state,
+				  const struct drm_connector_state *conn_state)
+{
+	if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
+		intel_ddi_update_pipe_dp(encoder, crtc_state, conn_state);
+
+	intel_ddi_update_hdcp(encoder, crtc_state, conn_state);
+}
+
 static void intel_ddi_set_fia_lane_count(struct intel_encoder *encoder,
 					 const struct intel_crtc_state *pipe_config,
 					 enum port port)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a37a4477994c..e387e842f414 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -2178,7 +2178,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);
diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
index ff9497e5c591..9b4904a62744 100644
--- a/drivers/gpu/drm/i915/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/intel_hdcp.c
@@ -1782,6 +1782,13 @@ static void intel_hdcp2_init(struct intel_connector *connector)
 		return;
 	}
 
+	ret = drm_connector_attach_hdcp_content_type_property(
+						&connector->base);
+	if (ret) {
+		kfree(hdcp->port_data.streams);
+		return;
+	}
+
 	hdcp->hdcp2_supported = true;
 }
 
@@ -1811,7 +1818,7 @@ int intel_hdcp_init(struct intel_connector *connector,
 	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;
@@ -1822,6 +1829,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
@@ -1833,8 +1841,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);
 	}
 
@@ -1901,6 +1913,8 @@ void intel_hdcp_atomic_check(struct drm_connector *connector,
 {
 	u64 old_cp = old_state->content_protection;
 	u64 new_cp = new_state->content_protection;
+	u64 old_type = old_state->hdcp_content_type;
+	u64 new_type = new_state->hdcp_content_type;
 	struct drm_crtc_state *crtc_state;
 
 	if (!new_state->crtc) {
@@ -1920,8 +1934,14 @@ void intel_hdcp_atomic_check(struct drm_connector *connector,
 	 */
 	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_type == new_type)
+			return;
+
+		new_state->content_protection =
+			DRM_MODE_CONTENT_PROTECTION_DESIRED;
+		intel_hdcp_disable(to_intel_connector(connector));
+	}
 
 	crtc_state = drm_atomic_get_new_crtc_state(new_state->state,
 						   new_state->crtc);
-- 
2.19.1

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

Re: [PATCH v3 03/10] drm/i915: Attach content type property

[Daniel Vetter]

On Fri, Mar 22, 2019 at 06:14:41AM +0530, Ramalingam C wrote:
> 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.
> 
> 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]
> 
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c  | 21 +++++++++++++++------
>  drivers/gpu/drm/i915/intel_drv.h  |  2 +-
>  drivers/gpu/drm/i915/intel_hdcp.c | 30 +++++++++++++++++++++++++-----
>  3 files changed, 41 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 933df3a57a8a..43859f126b82 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -3495,7 +3495,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,
> @@ -3558,21 +3559,29 @@ static void intel_ddi_update_pipe_dp(struct intel_encoder *encoder,
>  	intel_panel_update_backlight(encoder, crtc_state, conn_state);
>  }
>  
> -static void intel_ddi_update_pipe(struct intel_encoder *encoder,
> +static void intel_ddi_update_hdcp(struct intel_encoder *encoder,
>  				  const struct intel_crtc_state *crtc_state,
>  				  const struct drm_connector_state *conn_state)

Not sure why you split this out, doesn't feel necessary ...

>  {
> -	if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
> -		intel_ddi_update_pipe_dp(encoder, crtc_state, conn_state);
> -
>  	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);
>  	else if (conn_state->content_protection ==
>  		 DRM_MODE_CONTENT_PROTECTION_UNDESIRED)
>  		intel_hdcp_disable(to_intel_connector(conn_state->connector));
>  }
>  
> +static void intel_ddi_update_pipe(struct intel_encoder *encoder,
> +				  const struct intel_crtc_state *crtc_state,
> +				  const struct drm_connector_state *conn_state)
> +{
> +	if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
> +		intel_ddi_update_pipe_dp(encoder, crtc_state, conn_state);
> +
> +	intel_ddi_update_hdcp(encoder, crtc_state, conn_state);
> +}
> +
>  static void intel_ddi_set_fia_lane_count(struct intel_encoder *encoder,
>  					 const struct intel_crtc_state *pipe_config,
>  					 enum port port)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index a37a4477994c..e387e842f414 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -2178,7 +2178,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);
> diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
> index ff9497e5c591..9b4904a62744 100644
> --- a/drivers/gpu/drm/i915/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/intel_hdcp.c
> @@ -1782,6 +1782,13 @@ static void intel_hdcp2_init(struct intel_connector *connector)
>  		return;
>  	}
>  
> +	ret = drm_connector_attach_hdcp_content_type_property(
> +						&connector->base);
> +	if (ret) {
> +		kfree(hdcp->port_data.streams);
> +		return;
> +	}
> +
>  	hdcp->hdcp2_supported = true;
>  }
>  
> @@ -1811,7 +1818,7 @@ int intel_hdcp_init(struct intel_connector *connector,
>  	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;
> @@ -1822,6 +1829,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
> @@ -1833,8 +1841,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);
>  	}
>  
> @@ -1901,6 +1913,8 @@ void intel_hdcp_atomic_check(struct drm_connector *connector,
>  {
>  	u64 old_cp = old_state->content_protection;
>  	u64 new_cp = new_state->content_protection;
> +	u64 old_type = old_state->hdcp_content_type;
> +	u64 new_type = new_state->hdcp_content_type;
>  	struct drm_crtc_state *crtc_state;
>  
>  	if (!new_state->crtc) {
> @@ -1920,8 +1934,14 @@ void intel_hdcp_atomic_check(struct drm_connector *connector,
>  	 */
>  	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_type == new_type)
> +			return;
> +
> +		new_state->content_protection =
> +			DRM_MODE_CONTENT_PROTECTION_DESIRED;
> +		intel_hdcp_disable(to_intel_connector(connector));

No touching hw (or any persistent sw state outside of the free-standing
state structures) from atomic_check, ever. ATOMIC_TEST_ONLY is implemented
by running atomic_check code, and then simply releasing the computed
state. If you touch hw/sw state, then that's not undone.

We need to integrated this into the pipe_update function I think.
-Daniel

> +	}
>  
>  	crtc_state = drm_atomic_get_new_crtc_state(new_state->state,
>  						   new_state->crtc);
> -- 
> 2.19.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 03/10] drm/i915: Attach content type property

[Ramalingam C]

On 2019-03-27 at 11:00:17 +0100, Daniel Vetter wrote:
> On Fri, Mar 22, 2019 at 06:14:41AM +0530, Ramalingam C wrote:
> > 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.
> > 
> > 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]
> > 
> > Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_ddi.c  | 21 +++++++++++++++------
> >  drivers/gpu/drm/i915/intel_drv.h  |  2 +-
> >  drivers/gpu/drm/i915/intel_hdcp.c | 30 +++++++++++++++++++++++++-----
> >  3 files changed, 41 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index 933df3a57a8a..43859f126b82 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -3495,7 +3495,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,
> > @@ -3558,21 +3559,29 @@ static void intel_ddi_update_pipe_dp(struct intel_encoder *encoder,
> >  	intel_panel_update_backlight(encoder, crtc_state, conn_state);
> >  }
> >  
> > -static void intel_ddi_update_pipe(struct intel_encoder *encoder,
> > +static void intel_ddi_update_hdcp(struct intel_encoder *encoder,
> >  				  const struct intel_crtc_state *crtc_state,
> >  				  const struct drm_connector_state *conn_state)
> 
> Not sure why you split this out, doesn't feel necessary ...
this was growing considering, with some activity based on the type state
along with content protection state. Not in this version. I will merge
in the ddi_update_pipe only.
> 
> >  {
> > -	if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
> > -		intel_ddi_update_pipe_dp(encoder, crtc_state, conn_state);
> > -
> >  	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);
> >  	else if (conn_state->content_protection ==
> >  		 DRM_MODE_CONTENT_PROTECTION_UNDESIRED)
> >  		intel_hdcp_disable(to_intel_connector(conn_state->connector));
> >  }
> >  
> > +static void intel_ddi_update_pipe(struct intel_encoder *encoder,
> > +				  const struct intel_crtc_state *crtc_state,
> > +				  const struct drm_connector_state *conn_state)
> > +{
> > +	if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
> > +		intel_ddi_update_pipe_dp(encoder, crtc_state, conn_state);
> > +
> > +	intel_ddi_update_hdcp(encoder, crtc_state, conn_state);
> > +}
> > +
> >  static void intel_ddi_set_fia_lane_count(struct intel_encoder *encoder,
> >  					 const struct intel_crtc_state *pipe_config,
> >  					 enum port port)
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index a37a4477994c..e387e842f414 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -2178,7 +2178,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);
> > diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
> > index ff9497e5c591..9b4904a62744 100644
> > --- a/drivers/gpu/drm/i915/intel_hdcp.c
> > +++ b/drivers/gpu/drm/i915/intel_hdcp.c
> > @@ -1782,6 +1782,13 @@ static void intel_hdcp2_init(struct intel_connector *connector)
> >  		return;
> >  	}
> >  
> > +	ret = drm_connector_attach_hdcp_content_type_property(
> > +						&connector->base);
> > +	if (ret) {
> > +		kfree(hdcp->port_data.streams);
> > +		return;
> > +	}
> > +
> >  	hdcp->hdcp2_supported = true;
> >  }
> >  
> > @@ -1811,7 +1818,7 @@ int intel_hdcp_init(struct intel_connector *connector,
> >  	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;
> > @@ -1822,6 +1829,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
> > @@ -1833,8 +1841,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);
> >  	}
> >  
> > @@ -1901,6 +1913,8 @@ void intel_hdcp_atomic_check(struct drm_connector *connector,
> >  {
> >  	u64 old_cp = old_state->content_protection;
> >  	u64 new_cp = new_state->content_protection;
> > +	u64 old_type = old_state->hdcp_content_type;
> > +	u64 new_type = new_state->hdcp_content_type;
> >  	struct drm_crtc_state *crtc_state;
> >  
> >  	if (!new_state->crtc) {
> > @@ -1920,8 +1934,14 @@ void intel_hdcp_atomic_check(struct drm_connector *connector,
> >  	 */
> >  	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_type == new_type)
> > +			return;
> > +
> > +		new_state->content_protection =
> > +			DRM_MODE_CONTENT_PROTECTION_DESIRED;
> > +		intel_hdcp_disable(to_intel_connector(connector));
> 
> No touching hw (or any persistent sw state outside of the free-standing
> state structures) from atomic_check, ever. ATOMIC_TEST_ONLY is implemented
> by running atomic_check code, and then simply releasing the computed
> state. If you touch hw/sw state, then that's not undone.
> 
> We need to integrated this into the pipe_update function I think.
I think we cant retrieve the old conn state at atomic commit. If not we can
compare the type in conn_state with conn->hdcp->content_type and take
action of reauthentication.

-Ram
> -Daniel
> 
> > +	}
> >  
> >  	crtc_state = drm_atomic_get_new_crtc_state(new_state->state,
> >  						   new_state->crtc);
> > -- 
> > 2.19.1
> > 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v3 04/10] drm/i915: HDCP SRM parsing and revocation check

[Ramalingam C]

Implements the SRM table parsing for HDCP 1.4 and 2.2.
And also revocation check is added at authentication of HDCP1.4
and 2.2

Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c   |   1 +
 drivers/gpu/drm/i915/i915_drv.h   |   6 +
 drivers/gpu/drm/i915/intel_drv.h  |   2 +
 drivers/gpu/drm/i915/intel_hdcp.c | 308 ++++++++++++++++++++++++++++--
 include/drm/drm_hdcp.h            |  32 ++++
 5 files changed, 334 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index a3b00ecc58c9..60a894fab4fc 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -873,6 +873,7 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv)
 	mutex_init(&dev_priv->wm.wm_mutex);
 	mutex_init(&dev_priv->pps_mutex);
 	mutex_init(&dev_priv->hdcp_comp_mutex);
+	mutex_init(&dev_priv->srm_mutex);
 
 	i915_memcpy_init_early(dev_priv);
 	intel_runtime_pm_init_early(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c65c2e6649df..9b9daf6c9490 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2061,6 +2061,12 @@ struct drm_i915_private {
 	/* Mutex to protect the above hdcp component related values. */
 	struct mutex hdcp_comp_mutex;
 
+	unsigned int revocated_ksv_cnt;
+	u8 *revocated_ksv_list;
+
+	/* Mutex to protect the data about revocated ksvs */
+	struct mutex srm_mutex;
+
 	/*
 	 * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
 	 * will be rejected. Instead look for a better place.
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index e387e842f414..4db15ee81042 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -2187,6 +2187,8 @@ void intel_hdcp_component_init(struct drm_i915_private *dev_priv);
 void intel_hdcp_component_fini(struct drm_i915_private *dev_priv);
 void intel_hdcp_cleanup(struct intel_connector *connector);
 void intel_hdcp_handle_cp_irq(struct intel_connector *connector);
+ssize_t intel_hdcp_srm_update(struct drm_i915_private *dev_priv, char *buf,
+			      size_t count);
 
 /* intel_psr.c */
 #define CAN_PSR(dev_priv) (HAS_PSR(dev_priv) && dev_priv->psr.sink_support)
diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
index 9b4904a62744..921f07c64350 100644
--- a/drivers/gpu/drm/i915/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/intel_hdcp.c
@@ -273,6 +273,62 @@ u32 intel_hdcp_get_repeater_ctl(struct intel_digital_port *intel_dig_port)
 	return -EINVAL;
 }
 
+static inline void intel_hdcp_print_ksv(u8 *ksv)
+{
+	DRM_DEBUG_KMS("\t%#04x, %#04x, %#04x, %#04x, %#04x\n", *ksv,
+		      *(ksv + 1), *(ksv + 2), *(ksv + 3), *(ksv + 4));
+}
+
+static inline
+struct intel_connector *intel_hdcp_to_connector(struct intel_hdcp *hdcp)
+{
+	return container_of(hdcp, struct intel_connector, hdcp);
+}
+
+/* Check if any of the KSV is revocated by DCP LLC through SRM table */
+static inline
+bool intel_hdcp_ksvs_revocated(struct intel_hdcp *hdcp, u8 *ksvs, u32 ksv_count)
+{
+	struct intel_connector *connector = intel_hdcp_to_connector(hdcp);
+	struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector);
+	struct drm_i915_private *dev_priv =
+				intel_dig_port->base.base.dev->dev_private;
+	u32 rev_ksv_cnt, cnt, i, j;
+	u8 *rev_ksv_list;
+
+	mutex_lock(&dev_priv->srm_mutex);
+	rev_ksv_cnt = dev_priv->revocated_ksv_cnt;
+	rev_ksv_list = dev_priv->revocated_ksv_list;
+
+	/* If the Revocated ksv list is empty */
+	if (!rev_ksv_cnt || !rev_ksv_list) {
+		mutex_unlock(&dev_priv->srm_mutex);
+		return false;
+	}
+
+	for  (cnt = 0; cnt < ksv_count; cnt++) {
+		rev_ksv_list = dev_priv->revocated_ksv_list;
+		for (i = 0; i < rev_ksv_cnt; i++) {
+			for (j = 0; j < DRM_HDCP_KSV_LEN; j++)
+				if (*(ksvs + j) != *(rev_ksv_list + j)) {
+					break;
+				} else if (j == (DRM_HDCP_KSV_LEN - 1)) {
+					DRM_DEBUG_KMS("Revocated KSV is ");
+					intel_hdcp_print_ksv(ksvs);
+					mutex_unlock(&dev_priv->srm_mutex);
+					return true;
+				}
+			/* Move the offset to next KSV in the revocated list */
+			rev_ksv_list += DRM_HDCP_KSV_LEN;
+		}
+
+		/* Iterate to next ksv_offset */
+		ksvs += DRM_HDCP_KSV_LEN;
+	}
+	mutex_unlock(&dev_priv->srm_mutex);
+	return false;
+}
+
 static
 int intel_hdcp_validate_v_prime(struct intel_digital_port *intel_dig_port,
 				const struct intel_hdcp_shim *shim,
@@ -490,9 +546,10 @@ int intel_hdcp_validate_v_prime(struct intel_digital_port *intel_dig_port,
 
 /* Implements Part 2 of the HDCP authorization procedure */
 static
-int intel_hdcp_auth_downstream(struct intel_digital_port *intel_dig_port,
-			       const struct intel_hdcp_shim *shim)
+int intel_hdcp_auth_downstream(struct intel_hdcp *hdcp,
+			       struct intel_digital_port *intel_dig_port)
 {
+	const struct intel_hdcp_shim *shim = hdcp->shim;
 	u8 bstatus[2], num_downstream, *ksv_fifo;
 	int ret, i, tries = 3;
 
@@ -531,6 +588,11 @@ int intel_hdcp_auth_downstream(struct intel_digital_port *intel_dig_port,
 	if (ret)
 		goto err;
 
+	if (intel_hdcp_ksvs_revocated(hdcp, ksv_fifo, num_downstream)) {
+		DRM_ERROR("Revocated Ksv(s) in ksv_fifo\n");
+		return -EPERM;
+	}
+
 	/*
 	 * When V prime mismatches, DP Spec mandates re-read of
 	 * V prime atleast twice.
@@ -557,9 +619,11 @@ int intel_hdcp_auth_downstream(struct intel_digital_port *intel_dig_port,
 }
 
 /* Implements Part 1 of the HDCP authorization procedure */
-static int intel_hdcp_auth(struct intel_digital_port *intel_dig_port,
-			   const struct intel_hdcp_shim *shim)
+static int intel_hdcp_auth(struct intel_connector *connector)
 {
+	struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector);
+	struct intel_hdcp *hdcp = &connector->hdcp;
+	const struct intel_hdcp_shim *shim = hdcp->shim;
 	struct drm_i915_private *dev_priv;
 	enum port port;
 	unsigned long r0_prime_gen_start;
@@ -625,6 +689,11 @@ static int intel_hdcp_auth(struct intel_digital_port *intel_dig_port,
 	if (ret < 0)
 		return ret;
 
+	if (intel_hdcp_ksvs_revocated(hdcp, bksv.shim, 1)) {
+		DRM_ERROR("BKSV is revocated\n");
+		return -EPERM;
+	}
+
 	I915_WRITE(PORT_HDCP_BKSVLO(port), bksv.reg[0]);
 	I915_WRITE(PORT_HDCP_BKSVHI(port), bksv.reg[1]);
 
@@ -698,7 +767,7 @@ static int intel_hdcp_auth(struct intel_digital_port *intel_dig_port,
 	 */
 
 	if (repeater_present)
-		return intel_hdcp_auth_downstream(intel_dig_port, shim);
+		return intel_hdcp_auth_downstream(hdcp, intel_dig_port);
 
 	DRM_DEBUG_KMS("HDCP is enabled (no repeater present)\n");
 	return 0;
@@ -735,7 +804,6 @@ static int _intel_hdcp_disable(struct intel_connector *connector)
 
 static int _intel_hdcp_enable(struct intel_connector *connector)
 {
-	struct intel_hdcp *hdcp = &connector->hdcp;
 	struct drm_i915_private *dev_priv = connector->base.dev->dev_private;
 	int i, ret, tries = 3;
 
@@ -760,9 +828,9 @@ static int _intel_hdcp_enable(struct intel_connector *connector)
 
 	/* Incase of authentication failures, HDCP spec expects reauth. */
 	for (i = 0; i < tries; i++) {
-		ret = intel_hdcp_auth(conn_to_dig_port(connector), hdcp->shim);
+		ret = intel_hdcp_auth(connector);
 		if (!ret) {
-			hdcp->hdcp_encrypted = true;
+			connector->hdcp.hdcp_encrypted = true;
 			return 0;
 		}
 
@@ -776,12 +844,6 @@ static int _intel_hdcp_enable(struct intel_connector *connector)
 	return ret;
 }
 
-static inline
-struct intel_connector *intel_hdcp_to_connector(struct intel_hdcp *hdcp)
-{
-	return container_of(hdcp, struct intel_connector, hdcp);
-}
-
 /* Implements Part 3 of the HDCP authorization procedure */
 static int intel_hdcp_check_link(struct intel_connector *connector)
 {
@@ -1193,6 +1255,12 @@ static int hdcp2_authentication_key_exchange(struct intel_connector *connector)
 
 	hdcp->is_repeater = HDCP_2_2_RX_REPEATER(msgs.send_cert.rx_caps[2]);
 
+	if (intel_hdcp_ksvs_revocated(hdcp,
+				      msgs.send_cert.cert_rx.receiver_id, 1)) {
+		DRM_ERROR("Receiver ID is revocated\n");
+		return -EPERM;
+	}
+
 	/*
 	 * Here msgs.no_stored_km will hold msgs corresponding to the km
 	 * stored also.
@@ -1351,7 +1419,7 @@ int hdcp2_authenticate_repeater_topology(struct intel_connector *connector)
 	} msgs;
 	const struct intel_hdcp_shim *shim = hdcp->shim;
 	u8 *rx_info;
-	u32 seq_num_v;
+	u32 seq_num_v, device_cnt;
 	int ret;
 
 	ret = shim->read_2_2_msg(intel_dig_port, HDCP_2_2_REP_SEND_RECVID_LIST,
@@ -1376,6 +1444,14 @@ int hdcp2_authenticate_repeater_topology(struct intel_connector *connector)
 		return -EINVAL;
 	}
 
+	device_cnt = HDCP_2_2_DEV_COUNT_HI(rx_info[0]) << 4 ||
+			HDCP_2_2_DEV_COUNT_LO(rx_info[1]);
+	if (intel_hdcp_ksvs_revocated(hdcp, msgs.recvid_list.receiver_ids,
+				      device_cnt)) {
+		DRM_ERROR("Revoked receiver ID(s) is in list\n");
+		return -EPERM;
+	}
+
 	ret = hdcp2_verify_rep_topology_prepare_ack(connector,
 						    &msgs.recvid_list,
 						    &msgs.rep_ack);
@@ -1818,6 +1894,208 @@ int intel_hdcp_init(struct intel_connector *connector,
 	return 0;
 }
 
+static u32 intel_hdcp_get_revocated_ksv_count(u8 *buf, u32 vrls_length)
+{
+	u32 parsed_bytes = 0, ksv_count = 0, vrl_ksv_cnt, vrl_sz;
+
+	do {
+		vrl_ksv_cnt = *buf;
+		ksv_count += vrl_ksv_cnt;
+
+		vrl_sz = (vrl_ksv_cnt * DRM_HDCP_KSV_LEN) + 1;
+		buf += vrl_sz;
+		parsed_bytes += vrl_sz;
+	} while (parsed_bytes < vrls_length);
+
+	return ksv_count;
+}
+
+static u32 intel_hdcp_get_revocated_ksvs(u8 *ksv_list, const u8 *buf,
+					 u32 vrls_length)
+{
+	u32 parsed_bytes = 0, ksv_count = 0;
+	u32 vrl_ksv_cnt, vrl_ksv_sz, vrl_idx = 0;
+
+	do {
+		vrl_ksv_cnt = *buf;
+		vrl_ksv_sz = vrl_ksv_cnt * DRM_HDCP_KSV_LEN;
+
+		buf++;
+
+		DRM_DEBUG_KMS("vrl: %d, Revoked KSVs: %d\n", vrl_idx++,
+			      vrl_ksv_cnt);
+		memcpy(ksv_list, buf, vrl_ksv_sz);
+
+		ksv_count += vrl_ksv_cnt;
+		ksv_list += vrl_ksv_sz;
+		buf += vrl_ksv_sz;
+
+		parsed_bytes += (vrl_ksv_sz + 1);
+	} while (parsed_bytes < vrls_length);
+
+	return ksv_count;
+}
+
+static int intel_hdcp_parse_srm(struct drm_i915_private *dev_priv, char *buf,
+				size_t count)
+{
+	struct hdcp_srm_header *header;
+	u32 vrl_length, ksv_count;
+
+	if (count < (sizeof(struct hdcp_srm_header) +
+	    DRM_HDCP_1_4_VRL_LENGTH_SIZE + DRM_HDCP_1_4_DCP_SIG_SIZE)) {
+		DRM_ERROR("Invalid blob length\n");
+		return -EINVAL;
+	}
+
+	header = (struct hdcp_srm_header *)buf;
+	mutex_lock(&dev_priv->srm_mutex);
+	DRM_DEBUG_KMS("SRM ID: 0x%x, SRM Ver: 0x%x, SRM Gen No: 0x%x\n",
+		      header->spec_indicator.srm_id,
+		      __swab16(header->srm_version), header->srm_gen_no);
+
+	WARN_ON(header->spec_indicator.reserved_hi ||
+		header->spec_indicator.reserved_lo);
+
+	if (header->spec_indicator.srm_id != DRM_HDCP_1_4_SRM_ID) {
+		DRM_ERROR("Invalid srm_id\n");
+		mutex_unlock(&dev_priv->srm_mutex);
+		return -EINVAL;
+	}
+
+	buf = buf + sizeof(*header);
+	vrl_length = (*buf << 16 | *(buf + 1) << 8 | *(buf + 2));
+	if (count < (sizeof(struct hdcp_srm_header) + vrl_length) ||
+	    vrl_length < (DRM_HDCP_1_4_VRL_LENGTH_SIZE +
+			  DRM_HDCP_1_4_DCP_SIG_SIZE)) {
+		DRM_ERROR("Invalid blob length or vrl length\n");
+		mutex_unlock(&dev_priv->srm_mutex);
+		return -EINVAL;
+	}
+
+	/* Length of the all vrls combined */
+	vrl_length -= (DRM_HDCP_1_4_VRL_LENGTH_SIZE +
+		       DRM_HDCP_1_4_DCP_SIG_SIZE);
+
+	if (!vrl_length) {
+		DRM_DEBUG_KMS("No vrl found\n");
+		mutex_unlock(&dev_priv->srm_mutex);
+		return -EINVAL;
+	}
+
+	buf += DRM_HDCP_1_4_VRL_LENGTH_SIZE;
+	ksv_count = intel_hdcp_get_revocated_ksv_count(buf, vrl_length);
+	if (!ksv_count) {
+		DRM_DEBUG_KMS("Revocated KSV count is 0\n");
+		mutex_unlock(&dev_priv->srm_mutex);
+		return count;
+	}
+
+	kfree(dev_priv->revocated_ksv_list);
+	dev_priv->revocated_ksv_list = kzalloc(ksv_count * DRM_HDCP_KSV_LEN,
+					       GFP_KERNEL);
+	if (!dev_priv->revocated_ksv_list) {
+		DRM_ERROR("Out of Memory\n");
+		mutex_unlock(&dev_priv->srm_mutex);
+		return -ENOMEM;
+	}
+
+	if (intel_hdcp_get_revocated_ksvs(dev_priv->revocated_ksv_list,
+					  buf, vrl_length) != ksv_count) {
+		dev_priv->revocated_ksv_cnt = 0;
+		kfree(dev_priv->revocated_ksv_list);
+		mutex_unlock(&dev_priv->srm_mutex);
+		return -EINVAL;
+	}
+
+	dev_priv->revocated_ksv_cnt = ksv_count;
+	mutex_unlock(&dev_priv->srm_mutex);
+	return count;
+}
+
+static int intel_hdcp2_parse_srm(struct drm_i915_private *dev_priv, char *buf,
+				 size_t count)
+{
+	struct hdcp2_srm_header *header;
+	u32 vrl_length, ksv_count, ksv_sz;
+
+	mutex_lock(&dev_priv->srm_mutex);
+	if (count < (sizeof(struct hdcp2_srm_header) +
+	    DRM_HDCP_2_VRL_LENGTH_SIZE + DRM_HDCP_2_DCP_SIG_SIZE)) {
+		DRM_ERROR("Invalid blob length\n");
+		mutex_unlock(&dev_priv->srm_mutex);
+		return -EINVAL;
+	}
+
+	header = (struct hdcp2_srm_header *)buf;
+	DRM_DEBUG_KMS("SRM ID: 0x%x, SRM Ver: 0x%x, SRM Gen No: 0x%x\n",
+		      header->spec_indicator.srm_id,
+		      __swab16(header->srm_version), header->srm_gen_no);
+
+	WARN_ON(header->spec_indicator.reserved);
+	buf = buf + sizeof(*header);
+	vrl_length = (*buf << 16 | *(buf + 1) << 8 | *(buf + 2));
+
+	if (count < (sizeof(struct hdcp2_srm_header) + vrl_length) ||
+	    vrl_length < (DRM_HDCP_2_VRL_LENGTH_SIZE +
+	    DRM_HDCP_2_DCP_SIG_SIZE)) {
+		DRM_ERROR("Invalid blob length or vrl length\n");
+		mutex_unlock(&dev_priv->srm_mutex);
+		return -EINVAL;
+	}
+
+	/* Length of the all vrls combined */
+	vrl_length -= (DRM_HDCP_2_VRL_LENGTH_SIZE +
+		       DRM_HDCP_2_DCP_SIG_SIZE);
+
+	if (!vrl_length) {
+		DRM_DEBUG_KMS("No vrl found\n");
+		mutex_unlock(&dev_priv->srm_mutex);
+		return -EINVAL;
+	}
+
+	buf += DRM_HDCP_2_VRL_LENGTH_SIZE;
+	ksv_count = (*buf << 2) | DRM_HDCP_2_KSV_COUNT_2_LSBITS(*(buf + 1));
+	if (!ksv_count) {
+		DRM_DEBUG_KMS("Revocated KSV count is 0\n");
+		mutex_unlock(&dev_priv->srm_mutex);
+		return count;
+	}
+
+	kfree(dev_priv->revocated_ksv_list);
+	dev_priv->revocated_ksv_list = kzalloc(ksv_count * DRM_HDCP_KSV_LEN,
+					       GFP_KERNEL);
+	if (!dev_priv->revocated_ksv_list) {
+		DRM_ERROR("Out of Memory\n");
+		mutex_unlock(&dev_priv->srm_mutex);
+		return -ENOMEM;
+	}
+
+	ksv_sz = ksv_count * DRM_HDCP_KSV_LEN;
+	buf += DRM_HDCP_2_NO_OF_DEV_PLUS_RESERVED_SZ;
+
+	DRM_DEBUG_KMS("Revoked KSVs: %d\n", ksv_count);
+	memcpy(dev_priv->revocated_ksv_list, buf, ksv_sz);
+
+	dev_priv->revocated_ksv_cnt = ksv_count;
+	mutex_unlock(&dev_priv->srm_mutex);
+	return count;
+}
+
+ssize_t intel_hdcp_srm_update(struct drm_i915_private *dev_priv, char *buf,
+			      size_t count)
+{
+	u8 srm_id;
+
+	srm_id = *((u8 *)buf);
+	if (srm_id == 0x80)
+		return (ssize_t)intel_hdcp_parse_srm(dev_priv, buf, count);
+	else if (srm_id == 0x91)
+		return (ssize_t)intel_hdcp2_parse_srm(dev_priv, buf, count);
+
+	return (ssize_t)-EINVAL;
+}
+
 int intel_hdcp_enable(struct intel_connector *connector, u8 content_type)
 {
 	struct intel_hdcp *hdcp = &connector->hdcp;
diff --git a/include/drm/drm_hdcp.h b/include/drm/drm_hdcp.h
index f243408ecf26..652aaf5d658e 100644
--- a/include/drm/drm_hdcp.h
+++ b/include/drm/drm_hdcp.h
@@ -265,4 +265,36 @@ void drm_hdcp2_u32_to_seq_num(u8 seq_num[HDCP_2_2_SEQ_NUM_LEN], u32 val)
 	seq_num[2] = val;
 }
 
+#define DRM_HDCP_1_4_SRM_ID			0x8
+#define DRM_HDCP_1_4_VRL_LENGTH_SIZE		3
+#define DRM_HDCP_1_4_DCP_SIG_SIZE		40
+
+struct hdcp_srm_header {
+	struct {
+		u8 reserved_hi:4;
+		u8 srm_id:4;
+		u8 reserved_lo;
+	} spec_indicator;
+	u16 srm_version;
+	u8 srm_gen_no;
+} __packed;
+
+#define DRM_HDCP_2_SRM_ID			0x9
+#define DRM_HDCP_2_INDICATOR			0x1
+#define DRM_HDCP_2_VRL_LENGTH_SIZE		3
+#define DRM_HDCP_2_DCP_SIG_SIZE			384
+#define DRM_HDCP_2_NO_OF_DEV_PLUS_RESERVED_SZ	4
+
+#define DRM_HDCP_2_KSV_COUNT_2_LSBITS(byte)	(((byte) & 0xC) >> 6)
+
+struct hdcp2_srm_header {
+	struct {
+		u8 hdcp2_indicator:4;
+		u8 srm_id:4;
+		u8 reserved;
+	} spec_indicator;
+	u16 srm_version;
+	u8 srm_gen_no;
+} __packed;
+
 #endif
-- 
2.19.1

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

Re: [PATCH v3 04/10] drm/i915: HDCP SRM parsing and revocation check

[Daniel Vetter]

On Fri, Mar 22, 2019 at 06:14:42AM +0530, Ramalingam C wrote:
> Implements the SRM table parsing for HDCP 1.4 and 2.2.
> And also revocation check is added at authentication of HDCP1.4
> and 2.2
> 
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c   |   1 +
>  drivers/gpu/drm/i915/i915_drv.h   |   6 +
>  drivers/gpu/drm/i915/intel_drv.h  |   2 +
>  drivers/gpu/drm/i915/intel_hdcp.c | 308 ++++++++++++++++++++++++++++--
>  include/drm/drm_hdcp.h            |  32 ++++

Needs to be split out as a drm core patch.

I also wonder whether some of the SRM parsing should be helper functions
in a new drm_hdcp.c file ... Would fit really well with the overall single
drm sysfs file for SRM design, where all the parsing is done once in the
core. And then drivers just have functions to either get the entire SRM
(for upload to ME), or check a ksv against the revocation list (hdcp1.x).

Since the parsing code might be slightly different if we integrate it more
tightly with a sysfs binary file I'll wait with reviewing the details
until we've settled on the big picture. But looks all reasonable.
-Daniel


>  5 files changed, 334 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index a3b00ecc58c9..60a894fab4fc 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -873,6 +873,7 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv)
>  	mutex_init(&dev_priv->wm.wm_mutex);
>  	mutex_init(&dev_priv->pps_mutex);
>  	mutex_init(&dev_priv->hdcp_comp_mutex);
> +	mutex_init(&dev_priv->srm_mutex);
>  
>  	i915_memcpy_init_early(dev_priv);
>  	intel_runtime_pm_init_early(dev_priv);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index c65c2e6649df..9b9daf6c9490 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2061,6 +2061,12 @@ struct drm_i915_private {
>  	/* Mutex to protect the above hdcp component related values. */
>  	struct mutex hdcp_comp_mutex;
>  
> +	unsigned int revocated_ksv_cnt;
> +	u8 *revocated_ksv_list;
> +
> +	/* Mutex to protect the data about revocated ksvs */
> +	struct mutex srm_mutex;
> +
>  	/*
>  	 * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
>  	 * will be rejected. Instead look for a better place.
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index e387e842f414..4db15ee81042 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -2187,6 +2187,8 @@ void intel_hdcp_component_init(struct drm_i915_private *dev_priv);
>  void intel_hdcp_component_fini(struct drm_i915_private *dev_priv);
>  void intel_hdcp_cleanup(struct intel_connector *connector);
>  void intel_hdcp_handle_cp_irq(struct intel_connector *connector);
> +ssize_t intel_hdcp_srm_update(struct drm_i915_private *dev_priv, char *buf,
> +			      size_t count);
>  
>  /* intel_psr.c */
>  #define CAN_PSR(dev_priv) (HAS_PSR(dev_priv) && dev_priv->psr.sink_support)
> diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
> index 9b4904a62744..921f07c64350 100644
> --- a/drivers/gpu/drm/i915/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/intel_hdcp.c
> @@ -273,6 +273,62 @@ u32 intel_hdcp_get_repeater_ctl(struct intel_digital_port *intel_dig_port)
>  	return -EINVAL;
>  }
>  
> +static inline void intel_hdcp_print_ksv(u8 *ksv)
> +{
> +	DRM_DEBUG_KMS("\t%#04x, %#04x, %#04x, %#04x, %#04x\n", *ksv,
> +		      *(ksv + 1), *(ksv + 2), *(ksv + 3), *(ksv + 4));
> +}
> +
> +static inline
> +struct intel_connector *intel_hdcp_to_connector(struct intel_hdcp *hdcp)
> +{
> +	return container_of(hdcp, struct intel_connector, hdcp);
> +}
> +
> +/* Check if any of the KSV is revocated by DCP LLC through SRM table */
> +static inline
> +bool intel_hdcp_ksvs_revocated(struct intel_hdcp *hdcp, u8 *ksvs, u32 ksv_count)
> +{
> +	struct intel_connector *connector = intel_hdcp_to_connector(hdcp);
> +	struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector);
> +	struct drm_i915_private *dev_priv =
> +				intel_dig_port->base.base.dev->dev_private;
> +	u32 rev_ksv_cnt, cnt, i, j;
> +	u8 *rev_ksv_list;
> +
> +	mutex_lock(&dev_priv->srm_mutex);
> +	rev_ksv_cnt = dev_priv->revocated_ksv_cnt;
> +	rev_ksv_list = dev_priv->revocated_ksv_list;
> +
> +	/* If the Revocated ksv list is empty */
> +	if (!rev_ksv_cnt || !rev_ksv_list) {
> +		mutex_unlock(&dev_priv->srm_mutex);
> +		return false;
> +	}
> +
> +	for  (cnt = 0; cnt < ksv_count; cnt++) {
> +		rev_ksv_list = dev_priv->revocated_ksv_list;
> +		for (i = 0; i < rev_ksv_cnt; i++) {
> +			for (j = 0; j < DRM_HDCP_KSV_LEN; j++)
> +				if (*(ksvs + j) != *(rev_ksv_list + j)) {
> +					break;
> +				} else if (j == (DRM_HDCP_KSV_LEN - 1)) {
> +					DRM_DEBUG_KMS("Revocated KSV is ");
> +					intel_hdcp_print_ksv(ksvs);
> +					mutex_unlock(&dev_priv->srm_mutex);
> +					return true;
> +				}
> +			/* Move the offset to next KSV in the revocated list */
> +			rev_ksv_list += DRM_HDCP_KSV_LEN;
> +		}
> +
> +		/* Iterate to next ksv_offset */
> +		ksvs += DRM_HDCP_KSV_LEN;
> +	}
> +	mutex_unlock(&dev_priv->srm_mutex);
> +	return false;
> +}
> +
>  static
>  int intel_hdcp_validate_v_prime(struct intel_digital_port *intel_dig_port,
>  				const struct intel_hdcp_shim *shim,
> @@ -490,9 +546,10 @@ int intel_hdcp_validate_v_prime(struct intel_digital_port *intel_dig_port,
>  
>  /* Implements Part 2 of the HDCP authorization procedure */
>  static
> -int intel_hdcp_auth_downstream(struct intel_digital_port *intel_dig_port,
> -			       const struct intel_hdcp_shim *shim)
> +int intel_hdcp_auth_downstream(struct intel_hdcp *hdcp,
> +			       struct intel_digital_port *intel_dig_port)
>  {
> +	const struct intel_hdcp_shim *shim = hdcp->shim;
>  	u8 bstatus[2], num_downstream, *ksv_fifo;
>  	int ret, i, tries = 3;
>  
> @@ -531,6 +588,11 @@ int intel_hdcp_auth_downstream(struct intel_digital_port *intel_dig_port,
>  	if (ret)
>  		goto err;
>  
> +	if (intel_hdcp_ksvs_revocated(hdcp, ksv_fifo, num_downstream)) {
> +		DRM_ERROR("Revocated Ksv(s) in ksv_fifo\n");
> +		return -EPERM;
> +	}
> +
>  	/*
>  	 * When V prime mismatches, DP Spec mandates re-read of
>  	 * V prime atleast twice.
> @@ -557,9 +619,11 @@ int intel_hdcp_auth_downstream(struct intel_digital_port *intel_dig_port,
>  }
>  
>  /* Implements Part 1 of the HDCP authorization procedure */
> -static int intel_hdcp_auth(struct intel_digital_port *intel_dig_port,
> -			   const struct intel_hdcp_shim *shim)
> +static int intel_hdcp_auth(struct intel_connector *connector)
>  {
> +	struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector);
> +	struct intel_hdcp *hdcp = &connector->hdcp;
> +	const struct intel_hdcp_shim *shim = hdcp->shim;
>  	struct drm_i915_private *dev_priv;
>  	enum port port;
>  	unsigned long r0_prime_gen_start;
> @@ -625,6 +689,11 @@ static int intel_hdcp_auth(struct intel_digital_port *intel_dig_port,
>  	if (ret < 0)
>  		return ret;
>  
> +	if (intel_hdcp_ksvs_revocated(hdcp, bksv.shim, 1)) {
> +		DRM_ERROR("BKSV is revocated\n");
> +		return -EPERM;
> +	}
> +
>  	I915_WRITE(PORT_HDCP_BKSVLO(port), bksv.reg[0]);
>  	I915_WRITE(PORT_HDCP_BKSVHI(port), bksv.reg[1]);
>  
> @@ -698,7 +767,7 @@ static int intel_hdcp_auth(struct intel_digital_port *intel_dig_port,
>  	 */
>  
>  	if (repeater_present)
> -		return intel_hdcp_auth_downstream(intel_dig_port, shim);
> +		return intel_hdcp_auth_downstream(hdcp, intel_dig_port);
>  
>  	DRM_DEBUG_KMS("HDCP is enabled (no repeater present)\n");
>  	return 0;
> @@ -735,7 +804,6 @@ static int _intel_hdcp_disable(struct intel_connector *connector)
>  
>  static int _intel_hdcp_enable(struct intel_connector *connector)
>  {
> -	struct intel_hdcp *hdcp = &connector->hdcp;
>  	struct drm_i915_private *dev_priv = connector->base.dev->dev_private;
>  	int i, ret, tries = 3;
>  
> @@ -760,9 +828,9 @@ static int _intel_hdcp_enable(struct intel_connector *connector)
>  
>  	/* Incase of authentication failures, HDCP spec expects reauth. */
>  	for (i = 0; i < tries; i++) {
> -		ret = intel_hdcp_auth(conn_to_dig_port(connector), hdcp->shim);
> +		ret = intel_hdcp_auth(connector);
>  		if (!ret) {
> -			hdcp->hdcp_encrypted = true;
> +			connector->hdcp.hdcp_encrypted = true;
>  			return 0;
>  		}
>  
> @@ -776,12 +844,6 @@ static int _intel_hdcp_enable(struct intel_connector *connector)
>  	return ret;
>  }
>  
> -static inline
> -struct intel_connector *intel_hdcp_to_connector(struct intel_hdcp *hdcp)
> -{
> -	return container_of(hdcp, struct intel_connector, hdcp);
> -}
> -
>  /* Implements Part 3 of the HDCP authorization procedure */
>  static int intel_hdcp_check_link(struct intel_connector *connector)
>  {
> @@ -1193,6 +1255,12 @@ static int hdcp2_authentication_key_exchange(struct intel_connector *connector)
>  
>  	hdcp->is_repeater = HDCP_2_2_RX_REPEATER(msgs.send_cert.rx_caps[2]);
>  
> +	if (intel_hdcp_ksvs_revocated(hdcp,
> +				      msgs.send_cert.cert_rx.receiver_id, 1)) {
> +		DRM_ERROR("Receiver ID is revocated\n");
> +		return -EPERM;
> +	}
> +
>  	/*
>  	 * Here msgs.no_stored_km will hold msgs corresponding to the km
>  	 * stored also.
> @@ -1351,7 +1419,7 @@ int hdcp2_authenticate_repeater_topology(struct intel_connector *connector)
>  	} msgs;
>  	const struct intel_hdcp_shim *shim = hdcp->shim;
>  	u8 *rx_info;
> -	u32 seq_num_v;
> +	u32 seq_num_v, device_cnt;
>  	int ret;
>  
>  	ret = shim->read_2_2_msg(intel_dig_port, HDCP_2_2_REP_SEND_RECVID_LIST,
> @@ -1376,6 +1444,14 @@ int hdcp2_authenticate_repeater_topology(struct intel_connector *connector)
>  		return -EINVAL;
>  	}
>  
> +	device_cnt = HDCP_2_2_DEV_COUNT_HI(rx_info[0]) << 4 ||
> +			HDCP_2_2_DEV_COUNT_LO(rx_info[1]);
> +	if (intel_hdcp_ksvs_revocated(hdcp, msgs.recvid_list.receiver_ids,
> +				      device_cnt)) {
> +		DRM_ERROR("Revoked receiver ID(s) is in list\n");
> +		return -EPERM;
> +	}
> +
>  	ret = hdcp2_verify_rep_topology_prepare_ack(connector,
>  						    &msgs.recvid_list,
>  						    &msgs.rep_ack);
> @@ -1818,6 +1894,208 @@ int intel_hdcp_init(struct intel_connector *connector,
>  	return 0;
>  }
>  
> +static u32 intel_hdcp_get_revocated_ksv_count(u8 *buf, u32 vrls_length)
> +{
> +	u32 parsed_bytes = 0, ksv_count = 0, vrl_ksv_cnt, vrl_sz;
> +
> +	do {
> +		vrl_ksv_cnt = *buf;
> +		ksv_count += vrl_ksv_cnt;
> +
> +		vrl_sz = (vrl_ksv_cnt * DRM_HDCP_KSV_LEN) + 1;
> +		buf += vrl_sz;
> +		parsed_bytes += vrl_sz;
> +	} while (parsed_bytes < vrls_length);
> +
> +	return ksv_count;
> +}
> +
> +static u32 intel_hdcp_get_revocated_ksvs(u8 *ksv_list, const u8 *buf,
> +					 u32 vrls_length)
> +{
> +	u32 parsed_bytes = 0, ksv_count = 0;
> +	u32 vrl_ksv_cnt, vrl_ksv_sz, vrl_idx = 0;
> +
> +	do {
> +		vrl_ksv_cnt = *buf;
> +		vrl_ksv_sz = vrl_ksv_cnt * DRM_HDCP_KSV_LEN;
> +
> +		buf++;
> +
> +		DRM_DEBUG_KMS("vrl: %d, Revoked KSVs: %d\n", vrl_idx++,
> +			      vrl_ksv_cnt);
> +		memcpy(ksv_list, buf, vrl_ksv_sz);
> +
> +		ksv_count += vrl_ksv_cnt;
> +		ksv_list += vrl_ksv_sz;
> +		buf += vrl_ksv_sz;
> +
> +		parsed_bytes += (vrl_ksv_sz + 1);
> +	} while (parsed_bytes < vrls_length);
> +
> +	return ksv_count;
> +}
> +
> +static int intel_hdcp_parse_srm(struct drm_i915_private *dev_priv, char *buf,
> +				size_t count)
> +{
> +	struct hdcp_srm_header *header;
> +	u32 vrl_length, ksv_count;
> +
> +	if (count < (sizeof(struct hdcp_srm_header) +
> +	    DRM_HDCP_1_4_VRL_LENGTH_SIZE + DRM_HDCP_1_4_DCP_SIG_SIZE)) {
> +		DRM_ERROR("Invalid blob length\n");
> +		return -EINVAL;
> +	}
> +
> +	header = (struct hdcp_srm_header *)buf;
> +	mutex_lock(&dev_priv->srm_mutex);
> +	DRM_DEBUG_KMS("SRM ID: 0x%x, SRM Ver: 0x%x, SRM Gen No: 0x%x\n",
> +		      header->spec_indicator.srm_id,
> +		      __swab16(header->srm_version), header->srm_gen_no);
> +
> +	WARN_ON(header->spec_indicator.reserved_hi ||
> +		header->spec_indicator.reserved_lo);
> +
> +	if (header->spec_indicator.srm_id != DRM_HDCP_1_4_SRM_ID) {
> +		DRM_ERROR("Invalid srm_id\n");
> +		mutex_unlock(&dev_priv->srm_mutex);
> +		return -EINVAL;
> +	}
> +
> +	buf = buf + sizeof(*header);
> +	vrl_length = (*buf << 16 | *(buf + 1) << 8 | *(buf + 2));
> +	if (count < (sizeof(struct hdcp_srm_header) + vrl_length) ||
> +	    vrl_length < (DRM_HDCP_1_4_VRL_LENGTH_SIZE +
> +			  DRM_HDCP_1_4_DCP_SIG_SIZE)) {
> +		DRM_ERROR("Invalid blob length or vrl length\n");
> +		mutex_unlock(&dev_priv->srm_mutex);
> +		return -EINVAL;
> +	}
> +
> +	/* Length of the all vrls combined */
> +	vrl_length -= (DRM_HDCP_1_4_VRL_LENGTH_SIZE +
> +		       DRM_HDCP_1_4_DCP_SIG_SIZE);
> +
> +	if (!vrl_length) {
> +		DRM_DEBUG_KMS("No vrl found\n");
> +		mutex_unlock(&dev_priv->srm_mutex);
> +		return -EINVAL;
> +	}
> +
> +	buf += DRM_HDCP_1_4_VRL_LENGTH_SIZE;
> +	ksv_count = intel_hdcp_get_revocated_ksv_count(buf, vrl_length);
> +	if (!ksv_count) {
> +		DRM_DEBUG_KMS("Revocated KSV count is 0\n");
> +		mutex_unlock(&dev_priv->srm_mutex);
> +		return count;
> +	}
> +
> +	kfree(dev_priv->revocated_ksv_list);
> +	dev_priv->revocated_ksv_list = kzalloc(ksv_count * DRM_HDCP_KSV_LEN,
> +					       GFP_KERNEL);
> +	if (!dev_priv->revocated_ksv_list) {
> +		DRM_ERROR("Out of Memory\n");
> +		mutex_unlock(&dev_priv->srm_mutex);
> +		return -ENOMEM;
> +	}
> +
> +	if (intel_hdcp_get_revocated_ksvs(dev_priv->revocated_ksv_list,
> +					  buf, vrl_length) != ksv_count) {
> +		dev_priv->revocated_ksv_cnt = 0;
> +		kfree(dev_priv->revocated_ksv_list);
> +		mutex_unlock(&dev_priv->srm_mutex);
> +		return -EINVAL;
> +	}
> +
> +	dev_priv->revocated_ksv_cnt = ksv_count;
> +	mutex_unlock(&dev_priv->srm_mutex);
> +	return count;
> +}
> +
> +static int intel_hdcp2_parse_srm(struct drm_i915_private *dev_priv, char *buf,
> +				 size_t count)
> +{
> +	struct hdcp2_srm_header *header;
> +	u32 vrl_length, ksv_count, ksv_sz;
> +
> +	mutex_lock(&dev_priv->srm_mutex);
> +	if (count < (sizeof(struct hdcp2_srm_header) +
> +	    DRM_HDCP_2_VRL_LENGTH_SIZE + DRM_HDCP_2_DCP_SIG_SIZE)) {
> +		DRM_ERROR("Invalid blob length\n");
> +		mutex_unlock(&dev_priv->srm_mutex);
> +		return -EINVAL;
> +	}
> +
> +	header = (struct hdcp2_srm_header *)buf;
> +	DRM_DEBUG_KMS("SRM ID: 0x%x, SRM Ver: 0x%x, SRM Gen No: 0x%x\n",
> +		      header->spec_indicator.srm_id,
> +		      __swab16(header->srm_version), header->srm_gen_no);
> +
> +	WARN_ON(header->spec_indicator.reserved);
> +	buf = buf + sizeof(*header);
> +	vrl_length = (*buf << 16 | *(buf + 1) << 8 | *(buf + 2));
> +
> +	if (count < (sizeof(struct hdcp2_srm_header) + vrl_length) ||
> +	    vrl_length < (DRM_HDCP_2_VRL_LENGTH_SIZE +
> +	    DRM_HDCP_2_DCP_SIG_SIZE)) {
> +		DRM_ERROR("Invalid blob length or vrl length\n");
> +		mutex_unlock(&dev_priv->srm_mutex);
> +		return -EINVAL;
> +	}
> +
> +	/* Length of the all vrls combined */
> +	vrl_length -= (DRM_HDCP_2_VRL_LENGTH_SIZE +
> +		       DRM_HDCP_2_DCP_SIG_SIZE);
> +
> +	if (!vrl_length) {
> +		DRM_DEBUG_KMS("No vrl found\n");
> +		mutex_unlock(&dev_priv->srm_mutex);
> +		return -EINVAL;
> +	}
> +
> +	buf += DRM_HDCP_2_VRL_LENGTH_SIZE;
> +	ksv_count = (*buf << 2) | DRM_HDCP_2_KSV_COUNT_2_LSBITS(*(buf + 1));
> +	if (!ksv_count) {
> +		DRM_DEBUG_KMS("Revocated KSV count is 0\n");
> +		mutex_unlock(&dev_priv->srm_mutex);
> +		return count;
> +	}
> +
> +	kfree(dev_priv->revocated_ksv_list);
> +	dev_priv->revocated_ksv_list = kzalloc(ksv_count * DRM_HDCP_KSV_LEN,
> +					       GFP_KERNEL);
> +	if (!dev_priv->revocated_ksv_list) {
> +		DRM_ERROR("Out of Memory\n");
> +		mutex_unlock(&dev_priv->srm_mutex);
> +		return -ENOMEM;
> +	}
> +
> +	ksv_sz = ksv_count * DRM_HDCP_KSV_LEN;
> +	buf += DRM_HDCP_2_NO_OF_DEV_PLUS_RESERVED_SZ;
> +
> +	DRM_DEBUG_KMS("Revoked KSVs: %d\n", ksv_count);
> +	memcpy(dev_priv->revocated_ksv_list, buf, ksv_sz);
> +
> +	dev_priv->revocated_ksv_cnt = ksv_count;
> +	mutex_unlock(&dev_priv->srm_mutex);
> +	return count;
> +}
> +
> +ssize_t intel_hdcp_srm_update(struct drm_i915_private *dev_priv, char *buf,
> +			      size_t count)
> +{
> +	u8 srm_id;
> +
> +	srm_id = *((u8 *)buf);
> +	if (srm_id == 0x80)
> +		return (ssize_t)intel_hdcp_parse_srm(dev_priv, buf, count);
> +	else if (srm_id == 0x91)
> +		return (ssize_t)intel_hdcp2_parse_srm(dev_priv, buf, count);
> +
> +	return (ssize_t)-EINVAL;
> +}
> +
>  int intel_hdcp_enable(struct intel_connector *connector, u8 content_type)
>  {
>  	struct intel_hdcp *hdcp = &connector->hdcp;
> diff --git a/include/drm/drm_hdcp.h b/include/drm/drm_hdcp.h
> index f243408ecf26..652aaf5d658e 100644
> --- a/include/drm/drm_hdcp.h
> +++ b/include/drm/drm_hdcp.h
> @@ -265,4 +265,36 @@ void drm_hdcp2_u32_to_seq_num(u8 seq_num[HDCP_2_2_SEQ_NUM_LEN], u32 val)
>  	seq_num[2] = val;
>  }
>  
> +#define DRM_HDCP_1_4_SRM_ID			0x8
> +#define DRM_HDCP_1_4_VRL_LENGTH_SIZE		3
> +#define DRM_HDCP_1_4_DCP_SIG_SIZE		40
> +
> +struct hdcp_srm_header {
> +	struct {
> +		u8 reserved_hi:4;
> +		u8 srm_id:4;
> +		u8 reserved_lo;
> +	} spec_indicator;
> +	u16 srm_version;
> +	u8 srm_gen_no;
> +} __packed;
> +
> +#define DRM_HDCP_2_SRM_ID			0x9
> +#define DRM_HDCP_2_INDICATOR			0x1
> +#define DRM_HDCP_2_VRL_LENGTH_SIZE		3
> +#define DRM_HDCP_2_DCP_SIG_SIZE			384
> +#define DRM_HDCP_2_NO_OF_DEV_PLUS_RESERVED_SZ	4
> +
> +#define DRM_HDCP_2_KSV_COUNT_2_LSBITS(byte)	(((byte) & 0xC) >> 6)
> +
> +struct hdcp2_srm_header {
> +	struct {
> +		u8 hdcp2_indicator:4;
> +		u8 srm_id:4;
> +		u8 reserved;
> +	} spec_indicator;
> +	u16 srm_version;
> +	u8 srm_gen_no;
> +} __packed;
> +
>  #endif
> -- 
> 2.19.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 04/10] drm/i915: HDCP SRM parsing and revocation check

[Ramalingam C]

On 2019-03-27 at 11:16:40 +0100, Daniel Vetter wrote:
> On Fri, Mar 22, 2019 at 06:14:42AM +0530, Ramalingam C wrote:
> > Implements the SRM table parsing for HDCP 1.4 and 2.2.
> > And also revocation check is added at authentication of HDCP1.4
> > and 2.2
> > 
> > Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.c   |   1 +
> >  drivers/gpu/drm/i915/i915_drv.h   |   6 +
> >  drivers/gpu/drm/i915/intel_drv.h  |   2 +
> >  drivers/gpu/drm/i915/intel_hdcp.c | 308 ++++++++++++++++++++++++++++--
> >  include/drm/drm_hdcp.h            |  32 ++++
> 
> Needs to be split out as a drm core patch.
> 
> I also wonder whether some of the SRM parsing should be helper functions
> in a new drm_hdcp.c file ... Would fit really well with the overall single
> drm sysfs file for SRM design, where all the parsing is done once in the
> core. And then drivers just have functions to either get the entire SRM
> (for upload to ME), or check a ksv against the revocation list (hdcp1.x).
> 
> Since the parsing code might be slightly different if we integrate it more
> tightly with a sysfs binary file I'll wait with reviewing the details
> until we've settled on the big picture. But looks all reasonable.

I will cache the whole SRM blob in the global buffer in drm after the
sanity check.

Then I will add some APIs like drm_hdcp_srm_ksvs_revoked(<list of ksvs>,
<count of ksvs>)

As of now even for HDCP2.2 I am not passing the SRM into ME FW. As that
is not compulsary, using the HDCP1.4 way only.

but to create the class binary sysfs, we might need to create new API, something
like class_create_bin_file similar existing class_create_file.
I will send the patch soon to discuss upon.

-Ram
> -Daniel
> 
> 
> >  5 files changed, 334 insertions(+), 15 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > index a3b00ecc58c9..60a894fab4fc 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -873,6 +873,7 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv)
> >  	mutex_init(&dev_priv->wm.wm_mutex);
> >  	mutex_init(&dev_priv->pps_mutex);
> >  	mutex_init(&dev_priv->hdcp_comp_mutex);
> > +	mutex_init(&dev_priv->srm_mutex);
> >  
> >  	i915_memcpy_init_early(dev_priv);
> >  	intel_runtime_pm_init_early(dev_priv);
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index c65c2e6649df..9b9daf6c9490 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -2061,6 +2061,12 @@ struct drm_i915_private {
> >  	/* Mutex to protect the above hdcp component related values. */
> >  	struct mutex hdcp_comp_mutex;
> >  
> > +	unsigned int revocated_ksv_cnt;
> > +	u8 *revocated_ksv_list;
> > +
> > +	/* Mutex to protect the data about revocated ksvs */
> > +	struct mutex srm_mutex;
> > +
> >  	/*
> >  	 * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
> >  	 * will be rejected. Instead look for a better place.
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index e387e842f414..4db15ee81042 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -2187,6 +2187,8 @@ void intel_hdcp_component_init(struct drm_i915_private *dev_priv);
> >  void intel_hdcp_component_fini(struct drm_i915_private *dev_priv);
> >  void intel_hdcp_cleanup(struct intel_connector *connector);
> >  void intel_hdcp_handle_cp_irq(struct intel_connector *connector);
> > +ssize_t intel_hdcp_srm_update(struct drm_i915_private *dev_priv, char *buf,
> > +			      size_t count);
> >  
> >  /* intel_psr.c */
> >  #define CAN_PSR(dev_priv) (HAS_PSR(dev_priv) && dev_priv->psr.sink_support)
> > diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
> > index 9b4904a62744..921f07c64350 100644
> > --- a/drivers/gpu/drm/i915/intel_hdcp.c
> > +++ b/drivers/gpu/drm/i915/intel_hdcp.c
> > @@ -273,6 +273,62 @@ u32 intel_hdcp_get_repeater_ctl(struct intel_digital_port *intel_dig_port)
> >  	return -EINVAL;
> >  }
> >  
> > +static inline void intel_hdcp_print_ksv(u8 *ksv)
> > +{
> > +	DRM_DEBUG_KMS("\t%#04x, %#04x, %#04x, %#04x, %#04x\n", *ksv,
> > +		      *(ksv + 1), *(ksv + 2), *(ksv + 3), *(ksv + 4));
> > +}
> > +
> > +static inline
> > +struct intel_connector *intel_hdcp_to_connector(struct intel_hdcp *hdcp)
> > +{
> > +	return container_of(hdcp, struct intel_connector, hdcp);
> > +}
> > +
> > +/* Check if any of the KSV is revocated by DCP LLC through SRM table */
> > +static inline
> > +bool intel_hdcp_ksvs_revocated(struct intel_hdcp *hdcp, u8 *ksvs, u32 ksv_count)
> > +{
> > +	struct intel_connector *connector = intel_hdcp_to_connector(hdcp);
> > +	struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector);
> > +	struct drm_i915_private *dev_priv =
> > +				intel_dig_port->base.base.dev->dev_private;
> > +	u32 rev_ksv_cnt, cnt, i, j;
> > +	u8 *rev_ksv_list;
> > +
> > +	mutex_lock(&dev_priv->srm_mutex);
> > +	rev_ksv_cnt = dev_priv->revocated_ksv_cnt;
> > +	rev_ksv_list = dev_priv->revocated_ksv_list;
> > +
> > +	/* If the Revocated ksv list is empty */
> > +	if (!rev_ksv_cnt || !rev_ksv_list) {
> > +		mutex_unlock(&dev_priv->srm_mutex);
> > +		return false;
> > +	}
> > +
> > +	for  (cnt = 0; cnt < ksv_count; cnt++) {
> > +		rev_ksv_list = dev_priv->revocated_ksv_list;
> > +		for (i = 0; i < rev_ksv_cnt; i++) {
> > +			for (j = 0; j < DRM_HDCP_KSV_LEN; j++)
> > +				if (*(ksvs + j) != *(rev_ksv_list + j)) {
> > +					break;
> > +				} else if (j == (DRM_HDCP_KSV_LEN - 1)) {
> > +					DRM_DEBUG_KMS("Revocated KSV is ");
> > +					intel_hdcp_print_ksv(ksvs);
> > +					mutex_unlock(&dev_priv->srm_mutex);
> > +					return true;
> > +				}
> > +			/* Move the offset to next KSV in the revocated list */
> > +			rev_ksv_list += DRM_HDCP_KSV_LEN;
> > +		}
> > +
> > +		/* Iterate to next ksv_offset */
> > +		ksvs += DRM_HDCP_KSV_LEN;
> > +	}
> > +	mutex_unlock(&dev_priv->srm_mutex);
> > +	return false;
> > +}
> > +
> >  static
> >  int intel_hdcp_validate_v_prime(struct intel_digital_port *intel_dig_port,
> >  				const struct intel_hdcp_shim *shim,
> > @@ -490,9 +546,10 @@ int intel_hdcp_validate_v_prime(struct intel_digital_port *intel_dig_port,
> >  
> >  /* Implements Part 2 of the HDCP authorization procedure */
> >  static
> > -int intel_hdcp_auth_downstream(struct intel_digital_port *intel_dig_port,
> > -			       const struct intel_hdcp_shim *shim)
> > +int intel_hdcp_auth_downstream(struct intel_hdcp *hdcp,
> > +			       struct intel_digital_port *intel_dig_port)
> >  {
> > +	const struct intel_hdcp_shim *shim = hdcp->shim;
> >  	u8 bstatus[2], num_downstream, *ksv_fifo;
> >  	int ret, i, tries = 3;
> >  
> > @@ -531,6 +588,11 @@ int intel_hdcp_auth_downstream(struct intel_digital_port *intel_dig_port,
> >  	if (ret)
> >  		goto err;
> >  
> > +	if (intel_hdcp_ksvs_revocated(hdcp, ksv_fifo, num_downstream)) {
> > +		DRM_ERROR("Revocated Ksv(s) in ksv_fifo\n");
> > +		return -EPERM;
> > +	}
> > +
> >  	/*
> >  	 * When V prime mismatches, DP Spec mandates re-read of
> >  	 * V prime atleast twice.
> > @@ -557,9 +619,11 @@ int intel_hdcp_auth_downstream(struct intel_digital_port *intel_dig_port,
> >  }
> >  
> >  /* Implements Part 1 of the HDCP authorization procedure */
> > -static int intel_hdcp_auth(struct intel_digital_port *intel_dig_port,
> > -			   const struct intel_hdcp_shim *shim)
> > +static int intel_hdcp_auth(struct intel_connector *connector)
> >  {
> > +	struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector);
> > +	struct intel_hdcp *hdcp = &connector->hdcp;
> > +	const struct intel_hdcp_shim *shim = hdcp->shim;
> >  	struct drm_i915_private *dev_priv;
> >  	enum port port;
> >  	unsigned long r0_prime_gen_start;
> > @@ -625,6 +689,11 @@ static int intel_hdcp_auth(struct intel_digital_port *intel_dig_port,
> >  	if (ret < 0)
> >  		return ret;
> >  
> > +	if (intel_hdcp_ksvs_revocated(hdcp, bksv.shim, 1)) {
> > +		DRM_ERROR("BKSV is revocated\n");
> > +		return -EPERM;
> > +	}
> > +
> >  	I915_WRITE(PORT_HDCP_BKSVLO(port), bksv.reg[0]);
> >  	I915_WRITE(PORT_HDCP_BKSVHI(port), bksv.reg[1]);
> >  
> > @@ -698,7 +767,7 @@ static int intel_hdcp_auth(struct intel_digital_port *intel_dig_port,
> >  	 */
> >  
> >  	if (repeater_present)
> > -		return intel_hdcp_auth_downstream(intel_dig_port, shim);
> > +		return intel_hdcp_auth_downstream(hdcp, intel_dig_port);
> >  
> >  	DRM_DEBUG_KMS("HDCP is enabled (no repeater present)\n");
> >  	return 0;
> > @@ -735,7 +804,6 @@ static int _intel_hdcp_disable(struct intel_connector *connector)
> >  
> >  static int _intel_hdcp_enable(struct intel_connector *connector)
> >  {
> > -	struct intel_hdcp *hdcp = &connector->hdcp;
> >  	struct drm_i915_private *dev_priv = connector->base.dev->dev_private;
> >  	int i, ret, tries = 3;
> >  
> > @@ -760,9 +828,9 @@ static int _intel_hdcp_enable(struct intel_connector *connector)
> >  
> >  	/* Incase of authentication failures, HDCP spec expects reauth. */
> >  	for (i = 0; i < tries; i++) {
> > -		ret = intel_hdcp_auth(conn_to_dig_port(connector), hdcp->shim);
> > +		ret = intel_hdcp_auth(connector);
> >  		if (!ret) {
> > -			hdcp->hdcp_encrypted = true;
> > +			connector->hdcp.hdcp_encrypted = true;
> >  			return 0;
> >  		}
> >  
> > @@ -776,12 +844,6 @@ static int _intel_hdcp_enable(struct intel_connector *connector)
> >  	return ret;
> >  }
> >  
> > -static inline
> > -struct intel_connector *intel_hdcp_to_connector(struct intel_hdcp *hdcp)
> > -{
> > -	return container_of(hdcp, struct intel_connector, hdcp);
> > -}
> > -
> >  /* Implements Part 3 of the HDCP authorization procedure */
> >  static int intel_hdcp_check_link(struct intel_connector *connector)
> >  {
> > @@ -1193,6 +1255,12 @@ static int hdcp2_authentication_key_exchange(struct intel_connector *connector)
> >  
> >  	hdcp->is_repeater = HDCP_2_2_RX_REPEATER(msgs.send_cert.rx_caps[2]);
> >  
> > +	if (intel_hdcp_ksvs_revocated(hdcp,
> > +				      msgs.send_cert.cert_rx.receiver_id, 1)) {
> > +		DRM_ERROR("Receiver ID is revocated\n");
> > +		return -EPERM;
> > +	}
> > +
> >  	/*
> >  	 * Here msgs.no_stored_km will hold msgs corresponding to the km
> >  	 * stored also.
> > @@ -1351,7 +1419,7 @@ int hdcp2_authenticate_repeater_topology(struct intel_connector *connector)
> >  	} msgs;
> >  	const struct intel_hdcp_shim *shim = hdcp->shim;
> >  	u8 *rx_info;
> > -	u32 seq_num_v;
> > +	u32 seq_num_v, device_cnt;
> >  	int ret;
> >  
> >  	ret = shim->read_2_2_msg(intel_dig_port, HDCP_2_2_REP_SEND_RECVID_LIST,
> > @@ -1376,6 +1444,14 @@ int hdcp2_authenticate_repeater_topology(struct intel_connector *connector)
> >  		return -EINVAL;
> >  	}
> >  
> > +	device_cnt = HDCP_2_2_DEV_COUNT_HI(rx_info[0]) << 4 ||
> > +			HDCP_2_2_DEV_COUNT_LO(rx_info[1]);
> > +	if (intel_hdcp_ksvs_revocated(hdcp, msgs.recvid_list.receiver_ids,
> > +				      device_cnt)) {
> > +		DRM_ERROR("Revoked receiver ID(s) is in list\n");
> > +		return -EPERM;
> > +	}
> > +
> >  	ret = hdcp2_verify_rep_topology_prepare_ack(connector,
> >  						    &msgs.recvid_list,
> >  						    &msgs.rep_ack);
> > @@ -1818,6 +1894,208 @@ int intel_hdcp_init(struct intel_connector *connector,
> >  	return 0;
> >  }
> >  
> > +static u32 intel_hdcp_get_revocated_ksv_count(u8 *buf, u32 vrls_length)
> > +{
> > +	u32 parsed_bytes = 0, ksv_count = 0, vrl_ksv_cnt, vrl_sz;
> > +
> > +	do {
> > +		vrl_ksv_cnt = *buf;
> > +		ksv_count += vrl_ksv_cnt;
> > +
> > +		vrl_sz = (vrl_ksv_cnt * DRM_HDCP_KSV_LEN) + 1;
> > +		buf += vrl_sz;
> > +		parsed_bytes += vrl_sz;
> > +	} while (parsed_bytes < vrls_length);
> > +
> > +	return ksv_count;
> > +}
> > +
> > +static u32 intel_hdcp_get_revocated_ksvs(u8 *ksv_list, const u8 *buf,
> > +					 u32 vrls_length)
> > +{
> > +	u32 parsed_bytes = 0, ksv_count = 0;
> > +	u32 vrl_ksv_cnt, vrl_ksv_sz, vrl_idx = 0;
> > +
> > +	do {
> > +		vrl_ksv_cnt = *buf;
> > +		vrl_ksv_sz = vrl_ksv_cnt * DRM_HDCP_KSV_LEN;
> > +
> > +		buf++;
> > +
> > +		DRM_DEBUG_KMS("vrl: %d, Revoked KSVs: %d\n", vrl_idx++,
> > +			      vrl_ksv_cnt);
> > +		memcpy(ksv_list, buf, vrl_ksv_sz);
> > +
> > +		ksv_count += vrl_ksv_cnt;
> > +		ksv_list += vrl_ksv_sz;
> > +		buf += vrl_ksv_sz;
> > +
> > +		parsed_bytes += (vrl_ksv_sz + 1);
> > +	} while (parsed_bytes < vrls_length);
> > +
> > +	return ksv_count;
> > +}
> > +
> > +static int intel_hdcp_parse_srm(struct drm_i915_private *dev_priv, char *buf,
> > +				size_t count)
> > +{
> > +	struct hdcp_srm_header *header;
> > +	u32 vrl_length, ksv_count;
> > +
> > +	if (count < (sizeof(struct hdcp_srm_header) +
> > +	    DRM_HDCP_1_4_VRL_LENGTH_SIZE + DRM_HDCP_1_4_DCP_SIG_SIZE)) {
> > +		DRM_ERROR("Invalid blob length\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	header = (struct hdcp_srm_header *)buf;
> > +	mutex_lock(&dev_priv->srm_mutex);
> > +	DRM_DEBUG_KMS("SRM ID: 0x%x, SRM Ver: 0x%x, SRM Gen No: 0x%x\n",
> > +		      header->spec_indicator.srm_id,
> > +		      __swab16(header->srm_version), header->srm_gen_no);
> > +
> > +	WARN_ON(header->spec_indicator.reserved_hi ||
> > +		header->spec_indicator.reserved_lo);
> > +
> > +	if (header->spec_indicator.srm_id != DRM_HDCP_1_4_SRM_ID) {
> > +		DRM_ERROR("Invalid srm_id\n");
> > +		mutex_unlock(&dev_priv->srm_mutex);
> > +		return -EINVAL;
> > +	}
> > +
> > +	buf = buf + sizeof(*header);
> > +	vrl_length = (*buf << 16 | *(buf + 1) << 8 | *(buf + 2));
> > +	if (count < (sizeof(struct hdcp_srm_header) + vrl_length) ||
> > +	    vrl_length < (DRM_HDCP_1_4_VRL_LENGTH_SIZE +
> > +			  DRM_HDCP_1_4_DCP_SIG_SIZE)) {
> > +		DRM_ERROR("Invalid blob length or vrl length\n");
> > +		mutex_unlock(&dev_priv->srm_mutex);
> > +		return -EINVAL;
> > +	}
> > +
> > +	/* Length of the all vrls combined */
> > +	vrl_length -= (DRM_HDCP_1_4_VRL_LENGTH_SIZE +
> > +		       DRM_HDCP_1_4_DCP_SIG_SIZE);
> > +
> > +	if (!vrl_length) {
> > +		DRM_DEBUG_KMS("No vrl found\n");
> > +		mutex_unlock(&dev_priv->srm_mutex);
> > +		return -EINVAL;
> > +	}
> > +
> > +	buf += DRM_HDCP_1_4_VRL_LENGTH_SIZE;
> > +	ksv_count = intel_hdcp_get_revocated_ksv_count(buf, vrl_length);
> > +	if (!ksv_count) {
> > +		DRM_DEBUG_KMS("Revocated KSV count is 0\n");
> > +		mutex_unlock(&dev_priv->srm_mutex);
> > +		return count;
> > +	}
> > +
> > +	kfree(dev_priv->revocated_ksv_list);
> > +	dev_priv->revocated_ksv_list = kzalloc(ksv_count * DRM_HDCP_KSV_LEN,
> > +					       GFP_KERNEL);
> > +	if (!dev_priv->revocated_ksv_list) {
> > +		DRM_ERROR("Out of Memory\n");
> > +		mutex_unlock(&dev_priv->srm_mutex);
> > +		return -ENOMEM;
> > +	}
> > +
> > +	if (intel_hdcp_get_revocated_ksvs(dev_priv->revocated_ksv_list,
> > +					  buf, vrl_length) != ksv_count) {
> > +		dev_priv->revocated_ksv_cnt = 0;
> > +		kfree(dev_priv->revocated_ksv_list);
> > +		mutex_unlock(&dev_priv->srm_mutex);
> > +		return -EINVAL;
> > +	}
> > +
> > +	dev_priv->revocated_ksv_cnt = ksv_count;
> > +	mutex_unlock(&dev_priv->srm_mutex);
> > +	return count;
> > +}
> > +
> > +static int intel_hdcp2_parse_srm(struct drm_i915_private *dev_priv, char *buf,
> > +				 size_t count)
> > +{
> > +	struct hdcp2_srm_header *header;
> > +	u32 vrl_length, ksv_count, ksv_sz;
> > +
> > +	mutex_lock(&dev_priv->srm_mutex);
> > +	if (count < (sizeof(struct hdcp2_srm_header) +
> > +	    DRM_HDCP_2_VRL_LENGTH_SIZE + DRM_HDCP_2_DCP_SIG_SIZE)) {
> > +		DRM_ERROR("Invalid blob length\n");
> > +		mutex_unlock(&dev_priv->srm_mutex);
> > +		return -EINVAL;
> > +	}
> > +
> > +	header = (struct hdcp2_srm_header *)buf;
> > +	DRM_DEBUG_KMS("SRM ID: 0x%x, SRM Ver: 0x%x, SRM Gen No: 0x%x\n",
> > +		      header->spec_indicator.srm_id,
> > +		      __swab16(header->srm_version), header->srm_gen_no);
> > +
> > +	WARN_ON(header->spec_indicator.reserved);
> > +	buf = buf + sizeof(*header);
> > +	vrl_length = (*buf << 16 | *(buf + 1) << 8 | *(buf + 2));
> > +
> > +	if (count < (sizeof(struct hdcp2_srm_header) + vrl_length) ||
> > +	    vrl_length < (DRM_HDCP_2_VRL_LENGTH_SIZE +
> > +	    DRM_HDCP_2_DCP_SIG_SIZE)) {
> > +		DRM_ERROR("Invalid blob length or vrl length\n");
> > +		mutex_unlock(&dev_priv->srm_mutex);
> > +		return -EINVAL;
> > +	}
> > +
> > +	/* Length of the all vrls combined */
> > +	vrl_length -= (DRM_HDCP_2_VRL_LENGTH_SIZE +
> > +		       DRM_HDCP_2_DCP_SIG_SIZE);
> > +
> > +	if (!vrl_length) {
> > +		DRM_DEBUG_KMS("No vrl found\n");
> > +		mutex_unlock(&dev_priv->srm_mutex);
> > +		return -EINVAL;
> > +	}
> > +
> > +	buf += DRM_HDCP_2_VRL_LENGTH_SIZE;
> > +	ksv_count = (*buf << 2) | DRM_HDCP_2_KSV_COUNT_2_LSBITS(*(buf + 1));
> > +	if (!ksv_count) {
> > +		DRM_DEBUG_KMS("Revocated KSV count is 0\n");
> > +		mutex_unlock(&dev_priv->srm_mutex);
> > +		return count;
> > +	}
> > +
> > +	kfree(dev_priv->revocated_ksv_list);
> > +	dev_priv->revocated_ksv_list = kzalloc(ksv_count * DRM_HDCP_KSV_LEN,
> > +					       GFP_KERNEL);
> > +	if (!dev_priv->revocated_ksv_list) {
> > +		DRM_ERROR("Out of Memory\n");
> > +		mutex_unlock(&dev_priv->srm_mutex);
> > +		return -ENOMEM;
> > +	}
> > +
> > +	ksv_sz = ksv_count * DRM_HDCP_KSV_LEN;
> > +	buf += DRM_HDCP_2_NO_OF_DEV_PLUS_RESERVED_SZ;
> > +
> > +	DRM_DEBUG_KMS("Revoked KSVs: %d\n", ksv_count);
> > +	memcpy(dev_priv->revocated_ksv_list, buf, ksv_sz);
> > +
> > +	dev_priv->revocated_ksv_cnt = ksv_count;
> > +	mutex_unlock(&dev_priv->srm_mutex);
> > +	return count;
> > +}
> > +
> > +ssize_t intel_hdcp_srm_update(struct drm_i915_private *dev_priv, char *buf,
> > +			      size_t count)
> > +{
> > +	u8 srm_id;
> > +
> > +	srm_id = *((u8 *)buf);
> > +	if (srm_id == 0x80)
> > +		return (ssize_t)intel_hdcp_parse_srm(dev_priv, buf, count);
> > +	else if (srm_id == 0x91)
> > +		return (ssize_t)intel_hdcp2_parse_srm(dev_priv, buf, count);
> > +
> > +	return (ssize_t)-EINVAL;
> > +}
> > +
> >  int intel_hdcp_enable(struct intel_connector *connector, u8 content_type)
> >  {
> >  	struct intel_hdcp *hdcp = &connector->hdcp;
> > diff --git a/include/drm/drm_hdcp.h b/include/drm/drm_hdcp.h
> > index f243408ecf26..652aaf5d658e 100644
> > --- a/include/drm/drm_hdcp.h
> > +++ b/include/drm/drm_hdcp.h
> > @@ -265,4 +265,36 @@ void drm_hdcp2_u32_to_seq_num(u8 seq_num[HDCP_2_2_SEQ_NUM_LEN], u32 val)
> >  	seq_num[2] = val;
> >  }
> >  
> > +#define DRM_HDCP_1_4_SRM_ID			0x8
> > +#define DRM_HDCP_1_4_VRL_LENGTH_SIZE		3
> > +#define DRM_HDCP_1_4_DCP_SIG_SIZE		40
> > +
> > +struct hdcp_srm_header {
> > +	struct {
> > +		u8 reserved_hi:4;
> > +		u8 srm_id:4;
> > +		u8 reserved_lo;
> > +	} spec_indicator;
> > +	u16 srm_version;
> > +	u8 srm_gen_no;
> > +} __packed;
> > +
> > +#define DRM_HDCP_2_SRM_ID			0x9
> > +#define DRM_HDCP_2_INDICATOR			0x1
> > +#define DRM_HDCP_2_VRL_LENGTH_SIZE		3
> > +#define DRM_HDCP_2_DCP_SIG_SIZE			384
> > +#define DRM_HDCP_2_NO_OF_DEV_PLUS_RESERVED_SZ	4
> > +
> > +#define DRM_HDCP_2_KSV_COUNT_2_LSBITS(byte)	(((byte) & 0xC) >> 6)
> > +
> > +struct hdcp2_srm_header {
> > +	struct {
> > +		u8 hdcp2_indicator:4;
> > +		u8 srm_id:4;
> > +		u8 reserved;
> > +	} spec_indicator;
> > +	u16 srm_version;
> > +	u8 srm_gen_no;
> > +} __packed;
> > +
> >  #endif
> > -- 
> > 2.19.1
> > 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v3 05/10] drm/i915/sysfs: Node for hdcp srm

[Ramalingam C]

Binary Sysfs entry is created to pass the HDCP SRM table into
kerel for the HDCP authentication purpose.

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

diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
index 41313005af42..4621e944a24e 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -576,6 +576,36 @@ static void i915_setup_error_capture(struct device *kdev) {}
 static void i915_teardown_error_capture(struct device *kdev) {}
 #endif
 
+static ssize_t
+i915_srm_write(struct file *filp, struct kobject *kobj,
+	       struct bin_attribute *attr, char *buf,
+	       loff_t offset, size_t count)
+{
+	struct device *kdev = kobj_to_dev(kobj);
+	struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev);
+
+	return intel_hdcp_srm_update(dev_priv, buf, count);
+}
+
+static const struct bin_attribute srm_attrs = {
+	.attr = {.name = "hdcp_srm", .mode = S_IWUSR},
+	.read = NULL,
+	.write = i915_srm_write,
+	.mmap = NULL,
+	.private = (void *)0
+};
+
+static void i915_setup_hdcp_srm(struct device *kdev)
+{
+	if (sysfs_create_bin_file(&kdev->kobj, &srm_attrs))
+		DRM_ERROR("error_state sysfs setup failed\n");
+}
+
+static void i915_teardown_hdcp_srm(struct device *kdev)
+{
+	sysfs_remove_bin_file(&kdev->kobj, &srm_attrs);
+}
+
 void i915_setup_sysfs(struct drm_i915_private *dev_priv)
 {
 	struct device *kdev = dev_priv->drm.primary->kdev;
@@ -623,12 +653,14 @@ void i915_setup_sysfs(struct drm_i915_private *dev_priv)
 		DRM_ERROR("RPS sysfs setup failed\n");
 
 	i915_setup_error_capture(kdev);
+	i915_setup_hdcp_srm(kdev);
 }
 
 void i915_teardown_sysfs(struct drm_i915_private *dev_priv)
 {
 	struct device *kdev = dev_priv->drm.primary->kdev;
 
+	i915_teardown_hdcp_srm(kdev);
 	i915_teardown_error_capture(kdev);
 
 	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
-- 
2.19.1

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

[PATCH v3 06/10] drm: Add CP downstream_info property

[Ramalingam C]

This patch adds a optional CP downstream info blob property to the
connectors. This enables the Userspace to read the information of HDCP
authenticated downstream topology.

Driver will updated this blob with all downstream information at the
end of the authentication.

In case userspace configures this platform as repeater, then this
information is needed for the authentication with upstream HDCP
transmitter.

v2:
  s/cp_downstream/content_protection_downstream [daniel]
v3:
  s/content_protection_downstream/hdcp_topology [daniel]

Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
---
 drivers/gpu/drm/drm_atomic_uapi.c |  4 ++
 drivers/gpu/drm/drm_connector.c   | 86 +++++++++++++++++++++++++++++++
 include/drm/drm_connector.h       | 12 +++++
 include/uapi/drm/drm_mode.h       | 27 ++++++++++
 4 files changed, 129 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index 857ca6fa6fd0..4246e8988c29 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -826,6 +826,10 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
 		*val = state->content_protection;
 	} else if (property == connector->hdcp_content_type_property) {
 		*val = state->hdcp_content_type;
+	} else if (property ==
+		   connector->hdcp_topology_property) {
+		*val = connector->hdcp_topology_blob_ptr ?
+			connector->hdcp_topology_blob_ptr->base.id : 0;
 	} 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 ff61c3208307..0de8b441a449 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -246,6 +246,7 @@ int drm_connector_init(struct drm_device *dev,
 	mutex_init(&connector->mutex);
 	connector->edid_blob_ptr = NULL;
 	connector->tile_blob_ptr = NULL;
+	connector->hdcp_topology_blob_ptr = NULL;
 	connector->status = connector_status_unknown;
 	connector->display_info.panel_orientation =
 		DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
@@ -986,6 +987,25 @@ DRM_ENUM_NAME_FN(drm_get_hdcp_content_type_name,
  *	authentication process. 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
+ * HDCP Topology:
+ *	This blob property is used to pass the HDCP downstream topology details
+ *	of a HDCP encrypted connector, from kernel to userspace.
+ *	This provides all required information to userspace, so that userspace
+ *	can implement the HDCP repeater using the kernel as downstream ports of
+ *	the repeater. as illustrated below:
+ *
+ *                          HDCP Repeaters
+ * +--------------------------------------------------------------+
+ * |                                                              |
+ * |                               |                              |
+ * |   Userspace HDCP Receiver  +----->    KMD HDCP transmitters  |
+ * |      (Upstream Port)      <------+     (Downstream Ports)    |
+ * |                               |                              |
+ * |                                                              |
+ * +--------------------------------------------------------------+
+ *
+ *	Kernel will populate this blob only when the HDCP authentication is
+ *	successful.
  *
  * max bpc:
  *	This range property is used by userspace to limit the bit depth. When
@@ -1614,6 +1634,72 @@ drm_connector_attach_hdcp_content_type_property(struct drm_connector *
 }
 EXPORT_SYMBOL(drm_connector_attach_hdcp_content_type_property);
 
+/**
+ * drm_connector_attach_hdcp_topology_property - attach hdcp topology property
+ *
+ * @connector: connector to attach hdcp topology property with.
+ *
+ * This is used to add support for hdcp topology support on select connectors.
+ * When Intel platform is configured as repeater, this downstream info is used
+ * by userspace, to complete the repeater authentication of HDCP specification
+ * with upstream HDCP transmitter.
+ *
+ * The blob_id of the hdcp topology info will be set to
+ * &drm_connector_state.hdcp_topology
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+int drm_connector_attach_hdcp_topology_property(struct drm_connector *connector)
+{
+	struct drm_device *dev = connector->dev;
+	struct drm_property *prop;
+
+	prop = drm_property_create(dev, DRM_MODE_PROP_BLOB |
+				   DRM_MODE_PROP_IMMUTABLE,
+				   "HDCP Topology", 0);
+	if (!prop)
+		return -ENOMEM;
+
+	drm_object_attach_property(&connector->base, prop, 0);
+
+	connector->hdcp_topology_property = prop;
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_connector_attach_hdcp_topology_property);
+
+/**
+ * drm_connector_update_hdcp_topology_property - update the hdcp topology
+ * property of a connector
+ * @connector: drm connector, the topology is associated to
+ * @hdcp_topology_info: new content for the blob of hdcp_topology property
+ *
+ * This function creates a new blob modeset object and assigns its id to the
+ * connector's hdcp_topology property.
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+int
+drm_connector_update_hdcp_topology_property(struct drm_connector *connector,
+					const struct hdcp_topology_info *info)
+{
+	struct drm_device *dev = connector->dev;
+	int ret;
+
+	if (!info)
+		return -EINVAL;
+
+	ret = drm_property_replace_global_blob(dev,
+			&connector->hdcp_topology_blob_ptr,
+			sizeof(struct hdcp_topology_info),
+			info, &connector->base,
+			connector->hdcp_topology_property);
+	return ret;
+}
+EXPORT_SYMBOL(drm_connector_update_hdcp_topology_property);
+
 /**
  * drm_mode_create_aspect_ratio_property - create aspect ratio property
  * @dev: DRM device
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index f0830985367f..c016a0bcedac 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1047,6 +1047,13 @@ struct drm_connector {
 	 */
 	struct drm_property *hdcp_content_type_property;
 
+	/**
+	 * @hdcp_topology_property: DRM BLOB property for hdcp downstream
+	 * topology information.
+	 */
+	struct drm_property *hdcp_topology_property;
+	struct drm_property_blob *hdcp_topology_blob_ptr;
+
 	/**
 	 * @path_blob_ptr:
 	 *
@@ -1324,6 +1331,11 @@ int drm_connector_attach_content_protection_property(
 		struct drm_connector *connector);
 int drm_connector_attach_hdcp_content_type_property(
 		struct drm_connector *connector);
+int drm_connector_attach_hdcp_topology_property(
+		struct drm_connector *connector);
+int drm_connector_update_hdcp_topology_property(
+		struct drm_connector *connector,
+		const struct hdcp_topology_info *info);
 int drm_mode_create_aspect_ratio_property(struct drm_device *dev);
 int drm_mode_create_colorspace_property(struct drm_connector *connector);
 int drm_mode_create_content_type_property(struct drm_device *dev);
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 44412e8b77cd..03d3aa2b1a49 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -214,6 +214,33 @@ extern "C" {
 #define DRM_MODE_HDCP_CONTENT_TYPE0		0
 #define DRM_MODE_HDCP_CONTENT_TYPE1		1
 
+#define DRM_MODE_HDCP_KSV_LEN			5
+#define DRM_MODE_HDCP_MAX_DEVICE_CNT		127
+
+struct hdcp_topology_info {
+	/* KSV of immediate HDCP Sink. In Little-Endian Format. */
+	char bksv[DRM_MODE_HDCP_KSV_LEN];
+
+	/* Whether Immediate HDCP sink is a repeater? */
+	bool is_repeater;
+
+	/* Depth received from immediate downstream repeater */
+	__u8 depth;
+
+	/* Device count received from immediate downstream repeater */
+	__u32 device_count;
+
+	/*
+	 * Max buffer required to hold ksv list received from immediate
+	 * repeater. In this array first device_count * DRM_MODE_HDCP_KSV_LEN
+	 * will hold the valid ksv bytes.
+	 * If authentication specification is
+	 *	HDCP1.4 - each KSV's Bytes will be in Little-Endian format.
+	 *	HDCP2.2 - each KSV's Bytes will be in Big-Endian format.
+	 */
+	char ksv_list[DRM_MODE_HDCP_KSV_LEN * DRM_MODE_HDCP_MAX_DEVICE_CNT];
+};
+
 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 v3 06/10] drm: Add CP downstream_info property

[Daniel Vetter]

On Fri, Mar 22, 2019 at 06:14:44AM +0530, Ramalingam C wrote:
> This patch adds a optional CP downstream info blob property to the
> connectors. This enables the Userspace to read the information of HDCP
> authenticated downstream topology.
> 
> Driver will updated this blob with all downstream information at the
> end of the authentication.
> 
> In case userspace configures this platform as repeater, then this
> information is needed for the authentication with upstream HDCP
> transmitter.
> 
> v2:
>   s/cp_downstream/content_protection_downstream [daniel]
> v3:
>   s/content_protection_downstream/hdcp_topology [daniel]
> 
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> ---
>  drivers/gpu/drm/drm_atomic_uapi.c |  4 ++
>  drivers/gpu/drm/drm_connector.c   | 86 +++++++++++++++++++++++++++++++
>  include/drm/drm_connector.h       | 12 +++++
>  include/uapi/drm/drm_mode.h       | 27 ++++++++++
>  4 files changed, 129 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index 857ca6fa6fd0..4246e8988c29 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -826,6 +826,10 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
>  		*val = state->content_protection;
>  	} else if (property == connector->hdcp_content_type_property) {
>  		*val = state->hdcp_content_type;
> +	} else if (property ==
> +		   connector->hdcp_topology_property) {
> +		*val = connector->hdcp_topology_blob_ptr ?
> +			connector->hdcp_topology_blob_ptr->base.id : 0;
>  	} 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 ff61c3208307..0de8b441a449 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -246,6 +246,7 @@ int drm_connector_init(struct drm_device *dev,
>  	mutex_init(&connector->mutex);
>  	connector->edid_blob_ptr = NULL;
>  	connector->tile_blob_ptr = NULL;
> +	connector->hdcp_topology_blob_ptr = NULL;
>  	connector->status = connector_status_unknown;
>  	connector->display_info.panel_orientation =
>  		DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
> @@ -986,6 +987,25 @@ DRM_ENUM_NAME_FN(drm_get_hdcp_content_type_name,
>   *	authentication process. 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
> + * HDCP Topology:
> + *	This blob property is used to pass the HDCP downstream topology details
> + *	of a HDCP encrypted connector, from kernel to userspace.
> + *	This provides all required information to userspace, so that userspace
> + *	can implement the HDCP repeater using the kernel as downstream ports of
> + *	the repeater. as illustrated below:
> + *
> + *                          HDCP Repeaters
> + * +--------------------------------------------------------------+
> + * |                                                              |
> + * |                               |                              |
> + * |   Userspace HDCP Receiver  +----->    KMD HDCP transmitters  |
> + * |      (Upstream Port)      <------+     (Downstream Ports)    |
> + * |                               |                              |
> + * |                                                              |
> + * +--------------------------------------------------------------+

I didn't check, but I think this doesn't format correctly in the html
output. I think you need to indent, and start with :: to denote a fixed
width font example.

> + *
> + *	Kernel will populate this blob only when the HDCP authentication is
> + *	successful.
>   *
>   * max bpc:
>   *	This range property is used by userspace to limit the bit depth. When
> @@ -1614,6 +1634,72 @@ drm_connector_attach_hdcp_content_type_property(struct drm_connector *
>  }
>  EXPORT_SYMBOL(drm_connector_attach_hdcp_content_type_property);
>  
> +/**
> + * drm_connector_attach_hdcp_topology_property - attach hdcp topology property
> + *
> + * @connector: connector to attach hdcp topology property with.
> + *
> + * This is used to add support for hdcp topology support on select connectors.
> + * When Intel platform is configured as repeater, this downstream info is used
> + * by userspace, to complete the repeater authentication of HDCP specification
> + * with upstream HDCP transmitter.
> + *
> + * The blob_id of the hdcp topology info will be set to
> + * &drm_connector_state.hdcp_topology
> + *
> + * Returns:
> + * Zero on success, negative errno on failure.
> + */
> +int drm_connector_attach_hdcp_topology_property(struct drm_connector *connector)
> +{
> +	struct drm_device *dev = connector->dev;
> +	struct drm_property *prop;
> +
> +	prop = drm_property_create(dev, DRM_MODE_PROP_BLOB |
> +				   DRM_MODE_PROP_IMMUTABLE,
> +				   "HDCP Topology", 0);

Again global prop in dev->mode_config, and I think just add a flag to the
overall "attach content protection stuff to connnector" function.

> +	if (!prop)
> +		return -ENOMEM;
> +
> +	drm_object_attach_property(&connector->base, prop, 0);
> +
> +	connector->hdcp_topology_property = prop;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_connector_attach_hdcp_topology_property);
> +
> +/**
> + * drm_connector_update_hdcp_topology_property - update the hdcp topology
> + * property of a connector
> + * @connector: drm connector, the topology is associated to
> + * @hdcp_topology_info: new content for the blob of hdcp_topology property
> + *
> + * This function creates a new blob modeset object and assigns its id to the
> + * connector's hdcp_topology property.
> + *
> + * Returns:
> + * Zero on success, negative errno on failure.
> + */
> +int
> +drm_connector_update_hdcp_topology_property(struct drm_connector *connector,
> +					const struct hdcp_topology_info *info)
> +{
> +	struct drm_device *dev = connector->dev;
> +	int ret;
> +
> +	if (!info)
> +		return -EINVAL;
> +
> +	ret = drm_property_replace_global_blob(dev,
> +			&connector->hdcp_topology_blob_ptr,
> +			sizeof(struct hdcp_topology_info),
> +			info, &connector->base,
> +			connector->hdcp_topology_property);
> +	return ret;
> +}
> +EXPORT_SYMBOL(drm_connector_update_hdcp_topology_property);
> +
>  /**
>   * drm_mode_create_aspect_ratio_property - create aspect ratio property
>   * @dev: DRM device
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index f0830985367f..c016a0bcedac 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -1047,6 +1047,13 @@ struct drm_connector {
>  	 */
>  	struct drm_property *hdcp_content_type_property;
>  
> +	/**
> +	 * @hdcp_topology_property: DRM BLOB property for hdcp downstream
> +	 * topology information.
> +	 */
> +	struct drm_property *hdcp_topology_property;
> +	struct drm_property_blob *hdcp_topology_blob_ptr;

Need kerneldoc for both or kerneldoc gets a bit unhappy.

> +
>  	/**
>  	 * @path_blob_ptr:
>  	 *
> @@ -1324,6 +1331,11 @@ int drm_connector_attach_content_protection_property(
>  		struct drm_connector *connector);
>  int drm_connector_attach_hdcp_content_type_property(
>  		struct drm_connector *connector);
> +int drm_connector_attach_hdcp_topology_property(
> +		struct drm_connector *connector);
> +int drm_connector_update_hdcp_topology_property(
> +		struct drm_connector *connector,
> +		const struct hdcp_topology_info *info);
>  int drm_mode_create_aspect_ratio_property(struct drm_device *dev);
>  int drm_mode_create_colorspace_property(struct drm_connector *connector);
>  int drm_mode_create_content_type_property(struct drm_device *dev);
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 44412e8b77cd..03d3aa2b1a49 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -214,6 +214,33 @@ extern "C" {
>  #define DRM_MODE_HDCP_CONTENT_TYPE0		0
>  #define DRM_MODE_HDCP_CONTENT_TYPE1		1
>  
> +#define DRM_MODE_HDCP_KSV_LEN			5
> +#define DRM_MODE_HDCP_MAX_DEVICE_CNT		127
> +
> +struct hdcp_topology_info {
> +	/* KSV of immediate HDCP Sink. In Little-Endian Format. */
> +	char bksv[DRM_MODE_HDCP_KSV_LEN];

This isn't aligned to __u32. Just make the length 128 bytes.

> +
> +	/* Whether Immediate HDCP sink is a repeater? */
> +	bool is_repeater;

no bool in uapi structs. Just go with __u8.

> +
> +	/* Depth received from immediate downstream repeater */
> +	__u8 depth;

Needs to be aligned with explicit padding.

> +
> +	/* Device count received from immediate downstream repeater */
> +	__u32 device_count;
> +
> +	/*
> +	 * Max buffer required to hold ksv list received from immediate
> +	 * repeater. In this array first device_count * DRM_MODE_HDCP_KSV_LEN
> +	 * will hold the valid ksv bytes.
> +	 * If authentication specification is
> +	 *	HDCP1.4 - each KSV's Bytes will be in Little-Endian format.
> +	 *	HDCP2.2 - each KSV's Bytes will be in Big-Endian format.

Why is this ksv list be for hdcp2.2, but bksv is le for both case? I'm
confused.

> +	 */
> +	char ksv_list[DRM_MODE_HDCP_KSV_LEN * DRM_MODE_HDCP_MAX_DEVICE_CNT];

Again better to align this. Also maybe make it a nested array (it's the
same underlying layout, but easier to use for userspace.)

For uapi struct recommendations, see https://gitlab.com/TeeFirefly/linux-kernel/blob/7408b38cfdf9b0c6c3bda97402c75bd27ef69a85/Documentation/ioctl/botching-up-ioctls.txt

Cheers, Daniel
> +};
> +
>  struct drm_mode_modeinfo {
>  	__u32 clock;
>  	__u16 hdisplay;
> -- 
> 2.19.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 06/10] drm: Add CP downstream_info property

[Ramalingam C]

On 2019-03-27 at 11:25:04 +0100, Daniel Vetter wrote:
> On Fri, Mar 22, 2019 at 06:14:44AM +0530, Ramalingam C wrote:
> > This patch adds a optional CP downstream info blob property to the
> > connectors. This enables the Userspace to read the information of HDCP
> > authenticated downstream topology.
> > 
> > Driver will updated this blob with all downstream information at the
> > end of the authentication.
> > 
> > In case userspace configures this platform as repeater, then this
> > information is needed for the authentication with upstream HDCP
> > transmitter.
> > 
> > v2:
> >   s/cp_downstream/content_protection_downstream [daniel]
> > v3:
> >   s/content_protection_downstream/hdcp_topology [daniel]
> > 
> > Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> > ---
> >  drivers/gpu/drm/drm_atomic_uapi.c |  4 ++
> >  drivers/gpu/drm/drm_connector.c   | 86 +++++++++++++++++++++++++++++++
> >  include/drm/drm_connector.h       | 12 +++++
> >  include/uapi/drm/drm_mode.h       | 27 ++++++++++
> >  4 files changed, 129 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> > index 857ca6fa6fd0..4246e8988c29 100644
> > --- a/drivers/gpu/drm/drm_atomic_uapi.c
> > +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> > @@ -826,6 +826,10 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
> >  		*val = state->content_protection;
> >  	} else if (property == connector->hdcp_content_type_property) {
> >  		*val = state->hdcp_content_type;
> > +	} else if (property ==
> > +		   connector->hdcp_topology_property) {
> > +		*val = connector->hdcp_topology_blob_ptr ?
> > +			connector->hdcp_topology_blob_ptr->base.id : 0;
> >  	} 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 ff61c3208307..0de8b441a449 100644
> > --- a/drivers/gpu/drm/drm_connector.c
> > +++ b/drivers/gpu/drm/drm_connector.c
> > @@ -246,6 +246,7 @@ int drm_connector_init(struct drm_device *dev,
> >  	mutex_init(&connector->mutex);
> >  	connector->edid_blob_ptr = NULL;
> >  	connector->tile_blob_ptr = NULL;
> > +	connector->hdcp_topology_blob_ptr = NULL;
> >  	connector->status = connector_status_unknown;
> >  	connector->display_info.panel_orientation =
> >  		DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
> > @@ -986,6 +987,25 @@ DRM_ENUM_NAME_FN(drm_get_hdcp_content_type_name,
> >   *	authentication process. 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
> > + * HDCP Topology:
> > + *	This blob property is used to pass the HDCP downstream topology details
> > + *	of a HDCP encrypted connector, from kernel to userspace.
> > + *	This provides all required information to userspace, so that userspace
> > + *	can implement the HDCP repeater using the kernel as downstream ports of
> > + *	the repeater. as illustrated below:
> > + *
> > + *                          HDCP Repeaters
> > + * +--------------------------------------------------------------+
> > + * |                                                              |
> > + * |                               |                              |
> > + * |   Userspace HDCP Receiver  +----->    KMD HDCP transmitters  |
> > + * |      (Upstream Port)      <------+     (Downstream Ports)    |
> > + * |                               |                              |
> > + * |                                                              |
> > + * +--------------------------------------------------------------+
> 
> I didn't check, but I think this doesn't format correctly in the html
> output. I think you need to indent, and start with :: to denote a fixed
> width font example.
Looks good at mutt and at
https://patchwork.freedesktop.org/patch/293490/?series=57232&rev=4
Should i still do something on it?
> 
> > + *
> > + *	Kernel will populate this blob only when the HDCP authentication is
> > + *	successful.
> >   *
> >   * max bpc:
> >   *	This range property is used by userspace to limit the bit depth. When
> > @@ -1614,6 +1634,72 @@ drm_connector_attach_hdcp_content_type_property(struct drm_connector *
> >  }
> >  EXPORT_SYMBOL(drm_connector_attach_hdcp_content_type_property);
> >  
> > +/**
> > + * drm_connector_attach_hdcp_topology_property - attach hdcp topology property
> > + *
> > + * @connector: connector to attach hdcp topology property with.
> > + *
> > + * This is used to add support for hdcp topology support on select connectors.
> > + * When Intel platform is configured as repeater, this downstream info is used
> > + * by userspace, to complete the repeater authentication of HDCP specification
> > + * with upstream HDCP transmitter.
> > + *
> > + * The blob_id of the hdcp topology info will be set to
> > + * &drm_connector_state.hdcp_topology
> > + *
> > + * Returns:
> > + * Zero on success, negative errno on failure.
> > + */
> > +int drm_connector_attach_hdcp_topology_property(struct drm_connector *connector)
> > +{
> > +	struct drm_device *dev = connector->dev;
> > +	struct drm_property *prop;
> > +
> > +	prop = drm_property_create(dev, DRM_MODE_PROP_BLOB |
> > +				   DRM_MODE_PROP_IMMUTABLE,
> > +				   "HDCP Topology", 0);
> 
> Again global prop in dev->mode_config, and I think just add a flag to the
> overall "attach content protection stuff to connnector" function.
Will be done in the next version :)

-Ram
> 
> > +	if (!prop)
> > +		return -ENOMEM;
> > +
> > +	drm_object_attach_property(&connector->base, prop, 0);
> > +
> > +	connector->hdcp_topology_property = prop;
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL(drm_connector_attach_hdcp_topology_property);
> > +
> > +/**
> > + * drm_connector_update_hdcp_topology_property - update the hdcp topology
> > + * property of a connector
> > + * @connector: drm connector, the topology is associated to
> > + * @hdcp_topology_info: new content for the blob of hdcp_topology property
> > + *
> > + * This function creates a new blob modeset object and assigns its id to the
> > + * connector's hdcp_topology property.
> > + *
> > + * Returns:
> > + * Zero on success, negative errno on failure.
> > + */
> > +int
> > +drm_connector_update_hdcp_topology_property(struct drm_connector *connector,
> > +					const struct hdcp_topology_info *info)
> > +{
> > +	struct drm_device *dev = connector->dev;
> > +	int ret;
> > +
> > +	if (!info)
> > +		return -EINVAL;
> > +
> > +	ret = drm_property_replace_global_blob(dev,
> > +			&connector->hdcp_topology_blob_ptr,
> > +			sizeof(struct hdcp_topology_info),
> > +			info, &connector->base,
> > +			connector->hdcp_topology_property);
> > +	return ret;
> > +}
> > +EXPORT_SYMBOL(drm_connector_update_hdcp_topology_property);
> > +
> >  /**
> >   * drm_mode_create_aspect_ratio_property - create aspect ratio property
> >   * @dev: DRM device
> > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> > index f0830985367f..c016a0bcedac 100644
> > --- a/include/drm/drm_connector.h
> > +++ b/include/drm/drm_connector.h
> > @@ -1047,6 +1047,13 @@ struct drm_connector {
> >  	 */
> >  	struct drm_property *hdcp_content_type_property;
> >  
> > +	/**
> > +	 * @hdcp_topology_property: DRM BLOB property for hdcp downstream
> > +	 * topology information.
> > +	 */
> > +	struct drm_property *hdcp_topology_property;
> > +	struct drm_property_blob *hdcp_topology_blob_ptr;
> 
> Need kerneldoc for both or kerneldoc gets a bit unhappy.
Sure.
> 
> > +
> >  	/**
> >  	 * @path_blob_ptr:
> >  	 *
> > @@ -1324,6 +1331,11 @@ int drm_connector_attach_content_protection_property(
> >  		struct drm_connector *connector);
> >  int drm_connector_attach_hdcp_content_type_property(
> >  		struct drm_connector *connector);
> > +int drm_connector_attach_hdcp_topology_property(
> > +		struct drm_connector *connector);
> > +int drm_connector_update_hdcp_topology_property(
> > +		struct drm_connector *connector,
> > +		const struct hdcp_topology_info *info);
> >  int drm_mode_create_aspect_ratio_property(struct drm_device *dev);
> >  int drm_mode_create_colorspace_property(struct drm_connector *connector);
> >  int drm_mode_create_content_type_property(struct drm_device *dev);
> > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> > index 44412e8b77cd..03d3aa2b1a49 100644
> > --- a/include/uapi/drm/drm_mode.h
> > +++ b/include/uapi/drm/drm_mode.h
> > @@ -214,6 +214,33 @@ extern "C" {
> >  #define DRM_MODE_HDCP_CONTENT_TYPE0		0
> >  #define DRM_MODE_HDCP_CONTENT_TYPE1		1
> >  
> > +#define DRM_MODE_HDCP_KSV_LEN			5
> > +#define DRM_MODE_HDCP_MAX_DEVICE_CNT		127
> > +
> > +struct hdcp_topology_info {
> > +	/* KSV of immediate HDCP Sink. In Little-Endian Format. */
> > +	char bksv[DRM_MODE_HDCP_KSV_LEN];
> 
> This isn't aligned to __u32. Just make the length 128 bytes.
> 
> > +
> > +	/* Whether Immediate HDCP sink is a repeater? */
> > +	bool is_repeater;
> 
> no bool in uapi structs. Just go with __u8.
> 
> > +
> > +	/* Depth received from immediate downstream repeater */
> > +	__u8 depth;
> 
> Needs to be aligned with explicit padding.
not sure why do we need this explicit padding. currently this struct
works between IGT and kernel. Might be co-incident too!!


Will check on this.
> 
> > +
> > +	/* Device count received from immediate downstream repeater */
> > +	__u32 device_count;
> > +
> > +	/*
> > +	 * Max buffer required to hold ksv list received from immediate
> > +	 * repeater. In this array first device_count * DRM_MODE_HDCP_KSV_LEN
> > +	 * will hold the valid ksv bytes.
> > +	 * If authentication specification is
> > +	 *	HDCP1.4 - each KSV's Bytes will be in Little-Endian format.
> > +	 *	HDCP2.2 - each KSV's Bytes will be in Big-Endian format.
> 
> Why is this ksv list be for hdcp2.2, but bksv is le for both case? I'm
> confused.
I used the KSV and receiver ID interchangeably. receiver ID/KSV lists
from repeater changes with endianness. I guess we need not bother about
it at present.
> 
> > +	 */
> > +	char ksv_list[DRM_MODE_HDCP_KSV_LEN * DRM_MODE_HDCP_MAX_DEVICE_CNT];
> 
> Again better to align this. Also maybe make it a nested array (it's the
> same underlying layout, but easier to use for userspace.)
> 
> For uapi struct recommendations, see https://gitlab.com/TeeFirefly/linux-kernel/blob/7408b38cfdf9b0c6c3bda97402c75bd27ef69a85/Documentation/ioctl/botching-up-ioctls.txt
I will go through and align to it. Thanks for sharing.

-Ram
> 
> Cheers, Daniel
> > +};
> > +
> >  struct drm_mode_modeinfo {
> >  	__u32 clock;
> >  	__u16 hdisplay;
> > -- 
> > 2.19.1
> > 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 06/10] drm: Add CP downstream_info property

[Daniel Vetter]

On Wed, Mar 27, 2019 at 2:58 PM Ramalingam C <ramalingam.c@intel.com> wrote:
>
> On 2019-03-27 at 11:25:04 +0100, Daniel Vetter wrote:
> > On Fri, Mar 22, 2019 at 06:14:44AM +0530, Ramalingam C wrote:
> > > This patch adds a optional CP downstream info blob property to the
> > > connectors. This enables the Userspace to read the information of HDCP
> > > authenticated downstream topology.
> > >
> > > Driver will updated this blob with all downstream information at the
> > > end of the authentication.
> > >
> > > In case userspace configures this platform as repeater, then this
> > > information is needed for the authentication with upstream HDCP
> > > transmitter.
> > >
> > > v2:
> > >   s/cp_downstream/content_protection_downstream [daniel]
> > > v3:
> > >   s/content_protection_downstream/hdcp_topology [daniel]
> > >
> > > Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> > > ---
> > >  drivers/gpu/drm/drm_atomic_uapi.c |  4 ++
> > >  drivers/gpu/drm/drm_connector.c   | 86 +++++++++++++++++++++++++++++++
> > >  include/drm/drm_connector.h       | 12 +++++
> > >  include/uapi/drm/drm_mode.h       | 27 ++++++++++
> > >  4 files changed, 129 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> > > index 857ca6fa6fd0..4246e8988c29 100644
> > > --- a/drivers/gpu/drm/drm_atomic_uapi.c
> > > +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> > > @@ -826,6 +826,10 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
> > >             *val = state->content_protection;
> > >     } else if (property == connector->hdcp_content_type_property) {
> > >             *val = state->hdcp_content_type;
> > > +   } else if (property ==
> > > +              connector->hdcp_topology_property) {
> > > +           *val = connector->hdcp_topology_blob_ptr ?
> > > +                   connector->hdcp_topology_blob_ptr->base.id : 0;
> > >     } 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 ff61c3208307..0de8b441a449 100644
> > > --- a/drivers/gpu/drm/drm_connector.c
> > > +++ b/drivers/gpu/drm/drm_connector.c
> > > @@ -246,6 +246,7 @@ int drm_connector_init(struct drm_device *dev,
> > >     mutex_init(&connector->mutex);
> > >     connector->edid_blob_ptr = NULL;
> > >     connector->tile_blob_ptr = NULL;
> > > +   connector->hdcp_topology_blob_ptr = NULL;
> > >     connector->status = connector_status_unknown;
> > >     connector->display_info.panel_orientation =
> > >             DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
> > > @@ -986,6 +987,25 @@ DRM_ENUM_NAME_FN(drm_get_hdcp_content_type_name,
> > >   * authentication process. 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
> > > + * HDCP Topology:
> > > + * This blob property is used to pass the HDCP downstream topology details
> > > + * of a HDCP encrypted connector, from kernel to userspace.
> > > + * This provides all required information to userspace, so that userspace
> > > + * can implement the HDCP repeater using the kernel as downstream ports of
> > > + * the repeater. as illustrated below:
> > > + *
> > > + *                          HDCP Repeaters
> > > + * +--------------------------------------------------------------+
> > > + * |                                                              |
> > > + * |                               |                              |
> > > + * |   Userspace HDCP Receiver  +----->    KMD HDCP transmitters  |
> > > + * |      (Upstream Port)      <------+     (Downstream Ports)    |
> > > + * |                               |                              |
> > > + * |                                                              |
> > > + * +--------------------------------------------------------------+
> >
> > I didn't check, but I think this doesn't format correctly in the html
> > output. I think you need to indent, and start with :: to denote a fixed
> > width font example.
> Looks good at mutt and at
> https://patchwork.freedesktop.org/patch/293490/?series=57232&rev=4
> Should i still do something on it?


$ make htmldocs

Look at the generated output. These kerneldoc comments are parsed as
rest/sphinx formatted text, and I'm pretty sure the above won't parse.
The plaintext as plaintext looks fine.

> >
> > > + *
> > > + * Kernel will populate this blob only when the HDCP authentication is
> > > + * successful.
> > >   *
> > >   * max bpc:
> > >   * This range property is used by userspace to limit the bit depth. When
> > > @@ -1614,6 +1634,72 @@ drm_connector_attach_hdcp_content_type_property(struct drm_connector *
> > >  }
> > >  EXPORT_SYMBOL(drm_connector_attach_hdcp_content_type_property);
> > >
> > > +/**
> > > + * drm_connector_attach_hdcp_topology_property - attach hdcp topology property
> > > + *
> > > + * @connector: connector to attach hdcp topology property with.
> > > + *
> > > + * This is used to add support for hdcp topology support on select connectors.
> > > + * When Intel platform is configured as repeater, this downstream info is used
> > > + * by userspace, to complete the repeater authentication of HDCP specification
> > > + * with upstream HDCP transmitter.
> > > + *
> > > + * The blob_id of the hdcp topology info will be set to
> > > + * &drm_connector_state.hdcp_topology
> > > + *
> > > + * Returns:
> > > + * Zero on success, negative errno on failure.
> > > + */
> > > +int drm_connector_attach_hdcp_topology_property(struct drm_connector *connector)
> > > +{
> > > +   struct drm_device *dev = connector->dev;
> > > +   struct drm_property *prop;
> > > +
> > > +   prop = drm_property_create(dev, DRM_MODE_PROP_BLOB |
> > > +                              DRM_MODE_PROP_IMMUTABLE,
> > > +                              "HDCP Topology", 0);
> >
> > Again global prop in dev->mode_config, and I think just add a flag to the
> > overall "attach content protection stuff to connnector" function.
> Will be done in the next version :)
>
> -Ram
> >
> > > +   if (!prop)
> > > +           return -ENOMEM;
> > > +
> > > +   drm_object_attach_property(&connector->base, prop, 0);
> > > +
> > > +   connector->hdcp_topology_property = prop;
> > > +
> > > +   return 0;
> > > +}
> > > +EXPORT_SYMBOL(drm_connector_attach_hdcp_topology_property);
> > > +
> > > +/**
> > > + * drm_connector_update_hdcp_topology_property - update the hdcp topology
> > > + * property of a connector
> > > + * @connector: drm connector, the topology is associated to
> > > + * @hdcp_topology_info: new content for the blob of hdcp_topology property
> > > + *
> > > + * This function creates a new blob modeset object and assigns its id to the
> > > + * connector's hdcp_topology property.
> > > + *
> > > + * Returns:
> > > + * Zero on success, negative errno on failure.
> > > + */
> > > +int
> > > +drm_connector_update_hdcp_topology_property(struct drm_connector *connector,
> > > +                                   const struct hdcp_topology_info *info)
> > > +{
> > > +   struct drm_device *dev = connector->dev;
> > > +   int ret;
> > > +
> > > +   if (!info)
> > > +           return -EINVAL;
> > > +
> > > +   ret = drm_property_replace_global_blob(dev,
> > > +                   &connector->hdcp_topology_blob_ptr,
> > > +                   sizeof(struct hdcp_topology_info),
> > > +                   info, &connector->base,
> > > +                   connector->hdcp_topology_property);
> > > +   return ret;
> > > +}
> > > +EXPORT_SYMBOL(drm_connector_update_hdcp_topology_property);
> > > +
> > >  /**
> > >   * drm_mode_create_aspect_ratio_property - create aspect ratio property
> > >   * @dev: DRM device
> > > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> > > index f0830985367f..c016a0bcedac 100644
> > > --- a/include/drm/drm_connector.h
> > > +++ b/include/drm/drm_connector.h
> > > @@ -1047,6 +1047,13 @@ struct drm_connector {
> > >      */
> > >     struct drm_property *hdcp_content_type_property;
> > >
> > > +   /**
> > > +    * @hdcp_topology_property: DRM BLOB property for hdcp downstream
> > > +    * topology information.
> > > +    */
> > > +   struct drm_property *hdcp_topology_property;
> > > +   struct drm_property_blob *hdcp_topology_blob_ptr;
> >
> > Need kerneldoc for both or kerneldoc gets a bit unhappy.
> Sure.
> >
> > > +
> > >     /**
> > >      * @path_blob_ptr:
> > >      *
> > > @@ -1324,6 +1331,11 @@ int drm_connector_attach_content_protection_property(
> > >             struct drm_connector *connector);
> > >  int drm_connector_attach_hdcp_content_type_property(
> > >             struct drm_connector *connector);
> > > +int drm_connector_attach_hdcp_topology_property(
> > > +           struct drm_connector *connector);
> > > +int drm_connector_update_hdcp_topology_property(
> > > +           struct drm_connector *connector,
> > > +           const struct hdcp_topology_info *info);
> > >  int drm_mode_create_aspect_ratio_property(struct drm_device *dev);
> > >  int drm_mode_create_colorspace_property(struct drm_connector *connector);
> > >  int drm_mode_create_content_type_property(struct drm_device *dev);
> > > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> > > index 44412e8b77cd..03d3aa2b1a49 100644
> > > --- a/include/uapi/drm/drm_mode.h
> > > +++ b/include/uapi/drm/drm_mode.h
> > > @@ -214,6 +214,33 @@ extern "C" {
> > >  #define DRM_MODE_HDCP_CONTENT_TYPE0                0
> > >  #define DRM_MODE_HDCP_CONTENT_TYPE1                1
> > >
> > > +#define DRM_MODE_HDCP_KSV_LEN                      5
> > > +#define DRM_MODE_HDCP_MAX_DEVICE_CNT               127
> > > +
> > > +struct hdcp_topology_info {
> > > +   /* KSV of immediate HDCP Sink. In Little-Endian Format. */
> > > +   char bksv[DRM_MODE_HDCP_KSV_LEN];
> >
> > This isn't aligned to __u32. Just make the length 128 bytes.
> >
> > > +
> > > +   /* Whether Immediate HDCP sink is a repeater? */
> > > +   bool is_repeater;
> >
> > no bool in uapi structs. Just go with __u8.
> >
> > > +
> > > +   /* Depth received from immediate downstream repeater */
> > > +   __u8 depth;
> >
> > Needs to be aligned with explicit padding.
> not sure why do we need this explicit padding. currently this struct
> works between IGT and kernel. Might be co-incident too!!

32bit vs 64bit, depending upon architecture. Since this is core code,
it needs to work everywhere. igt is 64bit like the kernel (at least in
our CI, and probably on your machine too), that case always works.
It's mixed mode where userspace and the kernel might disagree on
alignment, and where therefore must align manually.

The other bit is that for uapi structs you must _never_ leak kernel
data out. It's pretty hard to avoid that if you have padding bytes you
can't even access because there's no member for them :-)
-Daniel

>
>
> Will check on this.
> >
> > > +
> > > +   /* Device count received from immediate downstream repeater */
> > > +   __u32 device_count;
> > > +
> > > +   /*
> > > +    * Max buffer required to hold ksv list received from immediate
> > > +    * repeater. In this array first device_count * DRM_MODE_HDCP_KSV_LEN
> > > +    * will hold the valid ksv bytes.
> > > +    * If authentication specification is
> > > +    *      HDCP1.4 - each KSV's Bytes will be in Little-Endian format.
> > > +    *      HDCP2.2 - each KSV's Bytes will be in Big-Endian format.
> >
> > Why is this ksv list be for hdcp2.2, but bksv is le for both case? I'm
> > confused.
> I used the KSV and receiver ID interchangeably. receiver ID/KSV lists
> from repeater changes with endianness. I guess we need not bother about
> it at present.
> >
> > > +    */
> > > +   char ksv_list[DRM_MODE_HDCP_KSV_LEN * DRM_MODE_HDCP_MAX_DEVICE_CNT];
> >
> > Again better to align this. Also maybe make it a nested array (it's the
> > same underlying layout, but easier to use for userspace.)
> >
> > For uapi struct recommendations, see https://gitlab.com/TeeFirefly/linux-kernel/blob/7408b38cfdf9b0c6c3bda97402c75bd27ef69a85/Documentation/ioctl/botching-up-ioctls.txt
> I will go through and align to it. Thanks for sharing.
>
> -Ram
> >
> > Cheers, Daniel
> > > +};
> > > +
> > >  struct drm_mode_modeinfo {
> > >     __u32 clock;
> > >     __u16 hdisplay;
> > > --
> > > 2.19.1
> > >
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v3 07/10] drm/i915: Populate downstream info for HDCP1.4

[Ramalingam C]

Implements drm blob property content_protection_downstream_info
property on HDCP capable connectors.

Downstream topology info is gathered across authentication stages
and stored in intel_hdcp. When HDCP authentication is complete,
new blob with latest downstream topology information is updated to
content_protection_downstream_info property.

v2:
  %s/cp_downstream/content_protection_downstream [daniel]
v3:
  %s/content_protection_downstream/hdcp_topology [daniel]

Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h  |  2 ++
 drivers/gpu/drm/i915/intel_hdcp.c | 30 +++++++++++++++++++++++++++++-
 include/drm/drm_hdcp.h            |  1 +
 include/uapi/drm/drm_mode.h       |  5 +++++
 4 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 4db15ee81042..46325fb33507 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -482,6 +482,8 @@ struct intel_hdcp {
 	wait_queue_head_t cp_irq_queue;
 	atomic_t cp_irq_count;
 	int cp_irq_count_cached;
+
+	struct hdcp_topology_info *topology_info;
 };
 
 struct intel_connector {
diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
index 921f07c64350..0e3d7afecd5a 100644
--- a/drivers/gpu/drm/i915/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/intel_hdcp.c
@@ -580,6 +580,9 @@ int intel_hdcp_auth_downstream(struct intel_hdcp *hdcp,
 	if (num_downstream == 0)
 		return -EINVAL;
 
+	hdcp->topology_info->device_count = num_downstream;
+	hdcp->topology_info->depth = DRM_HDCP_DEPTH(bstatus[1]);
+
 	ksv_fifo = kcalloc(DRM_HDCP_KSV_LEN, num_downstream, GFP_KERNEL);
 	if (!ksv_fifo)
 		return -ENOMEM;
@@ -593,6 +596,8 @@ int intel_hdcp_auth_downstream(struct intel_hdcp *hdcp,
 		return -EPERM;
 	}
 
+	memcpy(hdcp->topology_info->ksv_list, ksv_fifo,
+	       num_downstream * DRM_HDCP_KSV_LEN);
 	/*
 	 * When V prime mismatches, DP Spec mandates re-read of
 	 * V prime atleast twice.
@@ -694,15 +699,20 @@ static int intel_hdcp_auth(struct intel_connector *connector)
 		return -EPERM;
 	}
 
+	hdcp->topology_info->ver_in_force = DRM_MODE_HDCP14_IN_FORCE;
+	memcpy(hdcp->topology_info->bksv, bksv.shim, DRM_MODE_HDCP_KSV_LEN);
+
 	I915_WRITE(PORT_HDCP_BKSVLO(port), bksv.reg[0]);
 	I915_WRITE(PORT_HDCP_BKSVHI(port), bksv.reg[1]);
 
 	ret = shim->repeater_present(intel_dig_port, &repeater_present);
 	if (ret)
 		return ret;
-	if (repeater_present)
+	if (repeater_present) {
 		I915_WRITE(HDCP_REP_CTL,
 			   intel_hdcp_get_repeater_ctl(intel_dig_port));
+		hdcp->topology_info->is_repeater = true;
+	}
 
 	ret = shim->toggle_signalling(intel_dig_port, true);
 	if (ret)
@@ -798,6 +808,12 @@ static int _intel_hdcp_disable(struct intel_connector *connector)
 		return ret;
 	}
 
+	memset(hdcp->topology_info, 0, sizeof(struct hdcp_topology_info));
+
+	if (drm_connector_update_hdcp_topology_property(&connector->base,
+						connector->hdcp.topology_info))
+		DRM_ERROR("Downstream_info update failed.\n");
+
 	DRM_DEBUG_KMS("HDCP is disabled\n");
 	return 0;
 }
@@ -831,6 +847,10 @@ static int _intel_hdcp_enable(struct intel_connector *connector)
 		ret = intel_hdcp_auth(connector);
 		if (!ret) {
 			connector->hdcp.hdcp_encrypted = true;
+			if (drm_connector_update_hdcp_topology_property(
+					&connector->base,
+					connector->hdcp.topology_info))
+				DRM_ERROR("Downstream_info update failed.\n");
 			return 0;
 		}
 
@@ -1882,6 +1902,14 @@ int intel_hdcp_init(struct intel_connector *connector,
 	if (ret)
 		return ret;
 
+	ret = drm_connector_attach_hdcp_topology_property(&connector->base);
+	if (ret)
+		return ret;
+
+	hdcp->topology_info = kzalloc(sizeof(*hdcp->topology_info), GFP_KERNEL);
+	if (!hdcp->topology_info)
+		return -ENOMEM;
+
 	hdcp->shim = shim;
 	mutex_init(&hdcp->mutex);
 	INIT_DELAYED_WORK(&hdcp->check_work, intel_hdcp_check_work);
diff --git a/include/drm/drm_hdcp.h b/include/drm/drm_hdcp.h
index 652aaf5d658e..654a170f03eb 100644
--- a/include/drm/drm_hdcp.h
+++ b/include/drm/drm_hdcp.h
@@ -23,6 +23,7 @@
 #define DRM_HDCP_V_PRIME_PART_LEN		4
 #define DRM_HDCP_V_PRIME_NUM_PARTS		5
 #define DRM_HDCP_NUM_DOWNSTREAM(x)		(x & 0x7f)
+#define DRM_HDCP_DEPTH(x)			((x) & 0x7)
 #define DRM_HDCP_MAX_CASCADE_EXCEEDED(x)	(x & BIT(3))
 #define DRM_HDCP_MAX_DEVICE_EXCEEDED(x)		(x & BIT(7))
 
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 03d3aa2b1a49..733db7283e57 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -216,8 +216,13 @@ extern "C" {
 
 #define DRM_MODE_HDCP_KSV_LEN			5
 #define DRM_MODE_HDCP_MAX_DEVICE_CNT		127
+#define DRM_MODE_HDCP14_IN_FORCE		(1 << 0)
+#define DRM_MODE_HDCP22_IN_FORCE		(1 << 1)
 
 struct hdcp_topology_info {
+	/* Version of HDCP authenticated (1.4/2.2) */
+	__u32 ver_in_force;
+
 	/* KSV of immediate HDCP Sink. In Little-Endian Format. */
 	char bksv[DRM_MODE_HDCP_KSV_LEN];
 
-- 
2.19.1

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

Re: [PATCH v3 07/10] drm/i915: Populate downstream info for HDCP1.4

[Daniel Vetter]

On Fri, Mar 22, 2019 at 06:14:45AM +0530, Ramalingam C wrote:
> Implements drm blob property content_protection_downstream_info
> property on HDCP capable connectors.
> 
> Downstream topology info is gathered across authentication stages
> and stored in intel_hdcp. When HDCP authentication is complete,
> new blob with latest downstream topology information is updated to
> content_protection_downstream_info property.
> 
> v2:
>   %s/cp_downstream/content_protection_downstream [daniel]
> v3:
>   %s/content_protection_downstream/hdcp_topology [daniel]
> 
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_drv.h  |  2 ++
>  drivers/gpu/drm/i915/intel_hdcp.c | 30 +++++++++++++++++++++++++++++-
>  include/drm/drm_hdcp.h            |  1 +
>  include/uapi/drm/drm_mode.h       |  5 +++++
>  4 files changed, 37 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 4db15ee81042..46325fb33507 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -482,6 +482,8 @@ struct intel_hdcp {
>  	wait_queue_head_t cp_irq_queue;
>  	atomic_t cp_irq_count;
>  	int cp_irq_count_cached;
> +
> +	struct hdcp_topology_info *topology_info;
>  };
>  
>  struct intel_connector {
> diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
> index 921f07c64350..0e3d7afecd5a 100644
> --- a/drivers/gpu/drm/i915/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/intel_hdcp.c
> @@ -580,6 +580,9 @@ int intel_hdcp_auth_downstream(struct intel_hdcp *hdcp,
>  	if (num_downstream == 0)
>  		return -EINVAL;
>  
> +	hdcp->topology_info->device_count = num_downstream;
> +	hdcp->topology_info->depth = DRM_HDCP_DEPTH(bstatus[1]);
> +
>  	ksv_fifo = kcalloc(DRM_HDCP_KSV_LEN, num_downstream, GFP_KERNEL);
>  	if (!ksv_fifo)
>  		return -ENOMEM;
> @@ -593,6 +596,8 @@ int intel_hdcp_auth_downstream(struct intel_hdcp *hdcp,
>  		return -EPERM;
>  	}
>  
> +	memcpy(hdcp->topology_info->ksv_list, ksv_fifo,
> +	       num_downstream * DRM_HDCP_KSV_LEN);
>  	/*
>  	 * When V prime mismatches, DP Spec mandates re-read of
>  	 * V prime atleast twice.
> @@ -694,15 +699,20 @@ static int intel_hdcp_auth(struct intel_connector *connector)
>  		return -EPERM;
>  	}
>  
> +	hdcp->topology_info->ver_in_force = DRM_MODE_HDCP14_IN_FORCE;
> +	memcpy(hdcp->topology_info->bksv, bksv.shim, DRM_MODE_HDCP_KSV_LEN);
> +
>  	I915_WRITE(PORT_HDCP_BKSVLO(port), bksv.reg[0]);
>  	I915_WRITE(PORT_HDCP_BKSVHI(port), bksv.reg[1]);
>  
>  	ret = shim->repeater_present(intel_dig_port, &repeater_present);
>  	if (ret)
>  		return ret;
> -	if (repeater_present)
> +	if (repeater_present) {
>  		I915_WRITE(HDCP_REP_CTL,
>  			   intel_hdcp_get_repeater_ctl(intel_dig_port));
> +		hdcp->topology_info->is_repeater = true;
> +	}
>  
>  	ret = shim->toggle_signalling(intel_dig_port, true);
>  	if (ret)
> @@ -798,6 +808,12 @@ static int _intel_hdcp_disable(struct intel_connector *connector)
>  		return ret;
>  	}
>  
> +	memset(hdcp->topology_info, 0, sizeof(struct hdcp_topology_info));
> +
> +	if (drm_connector_update_hdcp_topology_property(&connector->base,
> +						connector->hdcp.topology_info))
> +		DRM_ERROR("Downstream_info update failed.\n");
> +
>  	DRM_DEBUG_KMS("HDCP is disabled\n");
>  	return 0;
>  }
> @@ -831,6 +847,10 @@ static int _intel_hdcp_enable(struct intel_connector *connector)
>  		ret = intel_hdcp_auth(connector);
>  		if (!ret) {
>  			connector->hdcp.hdcp_encrypted = true;
> +			if (drm_connector_update_hdcp_topology_property(
> +					&connector->base,
> +					connector->hdcp.topology_info))
> +				DRM_ERROR("Downstream_info update failed.\n");
>  			return 0;
>  		}
>  
> @@ -1882,6 +1902,14 @@ int intel_hdcp_init(struct intel_connector *connector,
>  	if (ret)
>  		return ret;
>  
> +	ret = drm_connector_attach_hdcp_topology_property(&connector->base);
> +	if (ret)
> +		return ret;
> +
> +	hdcp->topology_info = kzalloc(sizeof(*hdcp->topology_info), GFP_KERNEL);
> +	if (!hdcp->topology_info)
> +		return -ENOMEM;

Since you need to assemble the info struct manually from information from
all over: What if we make that the interface of the update function (i.e.
you pass it the list of ksv, bksv, and a bunch of flags), and the core
function bakes that into a structure and correct property? Would be a
slightly cleaner interface I think, and no risk that someone accidentally
forgets to set one of the values and leaves something stale behind.

> +
>  	hdcp->shim = shim;
>  	mutex_init(&hdcp->mutex);
>  	INIT_DELAYED_WORK(&hdcp->check_work, intel_hdcp_check_work);
> diff --git a/include/drm/drm_hdcp.h b/include/drm/drm_hdcp.h
> index 652aaf5d658e..654a170f03eb 100644
> --- a/include/drm/drm_hdcp.h
> +++ b/include/drm/drm_hdcp.h
> @@ -23,6 +23,7 @@
>  #define DRM_HDCP_V_PRIME_PART_LEN		4
>  #define DRM_HDCP_V_PRIME_NUM_PARTS		5
>  #define DRM_HDCP_NUM_DOWNSTREAM(x)		(x & 0x7f)
> +#define DRM_HDCP_DEPTH(x)			((x) & 0x7)
>  #define DRM_HDCP_MAX_CASCADE_EXCEEDED(x)	(x & BIT(3))
>  #define DRM_HDCP_MAX_DEVICE_EXCEEDED(x)		(x & BIT(7))
>  
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 03d3aa2b1a49..733db7283e57 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -216,8 +216,13 @@ extern "C" {
>  
>  #define DRM_MODE_HDCP_KSV_LEN			5
>  #define DRM_MODE_HDCP_MAX_DEVICE_CNT		127
> +#define DRM_MODE_HDCP14_IN_FORCE		(1 << 0)
> +#define DRM_MODE_HDCP22_IN_FORCE		(1 << 1)
>  
>  struct hdcp_topology_info {
> +	/* Version of HDCP authenticated (1.4/2.2) */
> +	__u32 ver_in_force;

Looks like misplaced hunk. Maybe use one of the __u8 padding fields you
need anyway.
-Daniel

> +
>  	/* KSV of immediate HDCP Sink. In Little-Endian Format. */
>  	char bksv[DRM_MODE_HDCP_KSV_LEN];
>  
> -- 
> 2.19.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v3 08/10] drm/i915: Populate downstream info for HDCP2.2

[Ramalingam C]

Populates the downstream info for HDCP2.2 encryption also. On success
of encryption Blob is updated.

Additional two variable are added to downstream info blob. Such as
ver_in_force and content type.

v2:
  s/cp_downstream/content_protection_downstream [daniel]
v3:
  s/content_protection_downstream/hdcp_topology [daniel]

Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
---
 drivers/gpu/drm/i915/intel_hdcp.c | 26 +++++++++++++++++++++++++-
 include/uapi/drm/drm_mode.h       |  3 +++
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
index 0e3d7afecd5a..1eea553cdf81 100644
--- a/drivers/gpu/drm/i915/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/intel_hdcp.c
@@ -1281,6 +1281,12 @@ static int hdcp2_authentication_key_exchange(struct intel_connector *connector)
 		return -EPERM;
 	}
 
+	hdcp->topology_info->ver_in_force = DRM_MODE_HDCP22_IN_FORCE;
+	hdcp->topology_info->content_type = hdcp->content_type;
+	memcpy(hdcp->topology_info->bksv, msgs.send_cert.cert_rx.receiver_id,
+	       HDCP_2_2_RECEIVER_ID_LEN);
+	hdcp->topology_info->is_repeater = hdcp->is_repeater;
+
 	/*
 	 * Here msgs.no_stored_km will hold msgs corresponding to the km
 	 * stored also.
@@ -1472,6 +1478,11 @@ int hdcp2_authenticate_repeater_topology(struct intel_connector *connector)
 		return -EPERM;
 	}
 
+	hdcp->topology_info->device_count = device_cnt;
+	hdcp->topology_info->depth = HDCP_2_2_DEPTH(rx_info[0]);
+	memcpy(hdcp->topology_info->ksv_list, msgs.recvid_list.receiver_ids,
+	       device_cnt * HDCP_2_2_RECEIVER_ID_LEN);
+
 	ret = hdcp2_verify_rep_topology_prepare_ack(connector,
 						    &msgs.recvid_list,
 						    &msgs.rep_ack);
@@ -1658,6 +1669,12 @@ static int _intel_hdcp2_enable(struct intel_connector *connector)
 	if (ret) {
 		DRM_DEBUG_KMS("HDCP2 Type%d  Enabling Failed. (%d)\n",
 			      hdcp->content_type, ret);
+
+		memset(hdcp->topology_info, 0,
+		       sizeof(struct hdcp_topology_info));
+		drm_connector_update_hdcp_topology_property(&connector->base,
+							  hdcp->topology_info);
+
 		return ret;
 	}
 
@@ -1665,12 +1682,16 @@ static int _intel_hdcp2_enable(struct intel_connector *connector)
 		      connector->base.name, connector->base.base.id,
 		      hdcp->content_type);
 
+	drm_connector_update_hdcp_topology_property(&connector->base,
+						    hdcp->topology_info);
 	hdcp->hdcp2_encrypted = true;
+
 	return 0;
 }
 
 static int _intel_hdcp2_disable(struct intel_connector *connector)
 {
+	struct intel_hdcp *hdcp = &connector->hdcp;
 	int ret;
 
 	DRM_DEBUG_KMS("[%s:%d] HDCP2.2 is being Disabled\n",
@@ -1681,8 +1702,11 @@ static int _intel_hdcp2_disable(struct intel_connector *connector)
 	if (hdcp2_deauthenticate_port(connector) < 0)
 		DRM_DEBUG_KMS("Port deauth failed.\n");
 
-	connector->hdcp.hdcp2_encrypted = false;
+	hdcp->hdcp2_encrypted = false;
 
+	memset(hdcp->topology_info, 0, sizeof(struct hdcp_topology_info));
+	drm_connector_update_hdcp_topology_property(&connector->base,
+						    hdcp->topology_info);
 	return ret;
 }
 
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 733db7283e57..22f5940a47cc 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -223,6 +223,9 @@ struct hdcp_topology_info {
 	/* Version of HDCP authenticated (1.4/2.2) */
 	__u32 ver_in_force;
 
+	/* Applicable only for HDCP2.2 */
+	__u8 content_type;
+
 	/* KSV of immediate HDCP Sink. In Little-Endian Format. */
 	char bksv[DRM_MODE_HDCP_KSV_LEN];
 
-- 
2.19.1

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

Re: [PATCH v3 08/10] drm/i915: Populate downstream info for HDCP2.2

[Daniel Vetter]

On Fri, Mar 22, 2019 at 06:14:46AM +0530, Ramalingam C wrote:
> Populates the downstream info for HDCP2.2 encryption also. On success
> of encryption Blob is updated.
> 
> Additional two variable are added to downstream info blob. Such as
> ver_in_force and content type.
> 
> v2:
>   s/cp_downstream/content_protection_downstream [daniel]
> v3:
>   s/content_protection_downstream/hdcp_topology [daniel]
> 
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_hdcp.c | 26 +++++++++++++++++++++++++-
>  include/uapi/drm/drm_mode.h       |  3 +++
>  2 files changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
> index 0e3d7afecd5a..1eea553cdf81 100644
> --- a/drivers/gpu/drm/i915/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/intel_hdcp.c
> @@ -1281,6 +1281,12 @@ static int hdcp2_authentication_key_exchange(struct intel_connector *connector)
>  		return -EPERM;
>  	}
>  
> +	hdcp->topology_info->ver_in_force = DRM_MODE_HDCP22_IN_FORCE;
> +	hdcp->topology_info->content_type = hdcp->content_type;
> +	memcpy(hdcp->topology_info->bksv, msgs.send_cert.cert_rx.receiver_id,
> +	       HDCP_2_2_RECEIVER_ID_LEN);
> +	hdcp->topology_info->is_repeater = hdcp->is_repeater;
> +
>  	/*
>  	 * Here msgs.no_stored_km will hold msgs corresponding to the km
>  	 * stored also.
> @@ -1472,6 +1478,11 @@ int hdcp2_authenticate_repeater_topology(struct intel_connector *connector)
>  		return -EPERM;
>  	}
>  
> +	hdcp->topology_info->device_count = device_cnt;
> +	hdcp->topology_info->depth = HDCP_2_2_DEPTH(rx_info[0]);
> +	memcpy(hdcp->topology_info->ksv_list, msgs.recvid_list.receiver_ids,
> +	       device_cnt * HDCP_2_2_RECEIVER_ID_LEN);
> +
>  	ret = hdcp2_verify_rep_topology_prepare_ack(connector,
>  						    &msgs.recvid_list,
>  						    &msgs.rep_ack);
> @@ -1658,6 +1669,12 @@ static int _intel_hdcp2_enable(struct intel_connector *connector)
>  	if (ret) {
>  		DRM_DEBUG_KMS("HDCP2 Type%d  Enabling Failed. (%d)\n",
>  			      hdcp->content_type, ret);
> +
> +		memset(hdcp->topology_info, 0,
> +		       sizeof(struct hdcp_topology_info));
> +		drm_connector_update_hdcp_topology_property(&connector->base,
> +							  hdcp->topology_info);
> +
>  		return ret;
>  	}
>  
> @@ -1665,12 +1682,16 @@ static int _intel_hdcp2_enable(struct intel_connector *connector)
>  		      connector->base.name, connector->base.base.id,
>  		      hdcp->content_type);
>  
> +	drm_connector_update_hdcp_topology_property(&connector->base,
> +						    hdcp->topology_info);
>  	hdcp->hdcp2_encrypted = true;
> +
>  	return 0;
>  }
>  
>  static int _intel_hdcp2_disable(struct intel_connector *connector)
>  {
> +	struct intel_hdcp *hdcp = &connector->hdcp;
>  	int ret;
>  
>  	DRM_DEBUG_KMS("[%s:%d] HDCP2.2 is being Disabled\n",
> @@ -1681,8 +1702,11 @@ static int _intel_hdcp2_disable(struct intel_connector *connector)
>  	if (hdcp2_deauthenticate_port(connector) < 0)
>  		DRM_DEBUG_KMS("Port deauth failed.\n");
>  
> -	connector->hdcp.hdcp2_encrypted = false;
> +	hdcp->hdcp2_encrypted = false;
>  
> +	memset(hdcp->topology_info, 0, sizeof(struct hdcp_topology_info));
> +	drm_connector_update_hdcp_topology_property(&connector->base,
> +						    hdcp->topology_info);
>  	return ret;
>  }
>  
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 733db7283e57..22f5940a47cc 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -223,6 +223,9 @@ struct hdcp_topology_info {
>  	/* Version of HDCP authenticated (1.4/2.2) */
>  	__u32 ver_in_force;
>  
> +	/* Applicable only for HDCP2.2 */
> +	__u8 content_type;

Even more misplaced hunks. You can't change an uapi struct once it's baked
in.

Same thought as on the hdcp1 patch, perhaps we even want 2 functions: One
for hdcp1, and one for hdcp2, to avoid the possiblity of screw-ups. After
all this interface is fairly fragile, and supposed to work across drivers.

Aside: Where's the userspace for this? Kinda wondering how an open source
hdcp repeater implementation would even work ...
-Daniel

> +
>  	/* KSV of immediate HDCP Sink. In Little-Endian Format. */
>  	char bksv[DRM_MODE_HDCP_KSV_LEN];
>  
> -- 
> 2.19.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 08/10] drm/i915: Populate downstream info for HDCP2.2

[Ramalingam C]

On 2019-03-27 at 11:31:12 +0100, Daniel Vetter wrote:
> On Fri, Mar 22, 2019 at 06:14:46AM +0530, Ramalingam C wrote:
> > Populates the downstream info for HDCP2.2 encryption also. On success
> > of encryption Blob is updated.
> > 
> > Additional two variable are added to downstream info blob. Such as
> > ver_in_force and content type.
> > 
> > v2:
> >   s/cp_downstream/content_protection_downstream [daniel]
> > v3:
> >   s/content_protection_downstream/hdcp_topology [daniel]
> > 
> > Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_hdcp.c | 26 +++++++++++++++++++++++++-
> >  include/uapi/drm/drm_mode.h       |  3 +++
> >  2 files changed, 28 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
> > index 0e3d7afecd5a..1eea553cdf81 100644
> > --- a/drivers/gpu/drm/i915/intel_hdcp.c
> > +++ b/drivers/gpu/drm/i915/intel_hdcp.c
> > @@ -1281,6 +1281,12 @@ static int hdcp2_authentication_key_exchange(struct intel_connector *connector)
> >  		return -EPERM;
> >  	}
> >  
> > +	hdcp->topology_info->ver_in_force = DRM_MODE_HDCP22_IN_FORCE;
> > +	hdcp->topology_info->content_type = hdcp->content_type;
> > +	memcpy(hdcp->topology_info->bksv, msgs.send_cert.cert_rx.receiver_id,
> > +	       HDCP_2_2_RECEIVER_ID_LEN);
> > +	hdcp->topology_info->is_repeater = hdcp->is_repeater;
> > +
> >  	/*
> >  	 * Here msgs.no_stored_km will hold msgs corresponding to the km
> >  	 * stored also.
> > @@ -1472,6 +1478,11 @@ int hdcp2_authenticate_repeater_topology(struct intel_connector *connector)
> >  		return -EPERM;
> >  	}
> >  
> > +	hdcp->topology_info->device_count = device_cnt;
> > +	hdcp->topology_info->depth = HDCP_2_2_DEPTH(rx_info[0]);
> > +	memcpy(hdcp->topology_info->ksv_list, msgs.recvid_list.receiver_ids,
> > +	       device_cnt * HDCP_2_2_RECEIVER_ID_LEN);
> > +
> >  	ret = hdcp2_verify_rep_topology_prepare_ack(connector,
> >  						    &msgs.recvid_list,
> >  						    &msgs.rep_ack);
> > @@ -1658,6 +1669,12 @@ static int _intel_hdcp2_enable(struct intel_connector *connector)
> >  	if (ret) {
> >  		DRM_DEBUG_KMS("HDCP2 Type%d  Enabling Failed. (%d)\n",
> >  			      hdcp->content_type, ret);
> > +
> > +		memset(hdcp->topology_info, 0,
> > +		       sizeof(struct hdcp_topology_info));
> > +		drm_connector_update_hdcp_topology_property(&connector->base,
> > +							  hdcp->topology_info);
> > +
> >  		return ret;
> >  	}
> >  
> > @@ -1665,12 +1682,16 @@ static int _intel_hdcp2_enable(struct intel_connector *connector)
> >  		      connector->base.name, connector->base.base.id,
> >  		      hdcp->content_type);
> >  
> > +	drm_connector_update_hdcp_topology_property(&connector->base,
> > +						    hdcp->topology_info);
> >  	hdcp->hdcp2_encrypted = true;
> > +
> >  	return 0;
> >  }
> >  
> >  static int _intel_hdcp2_disable(struct intel_connector *connector)
> >  {
> > +	struct intel_hdcp *hdcp = &connector->hdcp;
> >  	int ret;
> >  
> >  	DRM_DEBUG_KMS("[%s:%d] HDCP2.2 is being Disabled\n",
> > @@ -1681,8 +1702,11 @@ static int _intel_hdcp2_disable(struct intel_connector *connector)
> >  	if (hdcp2_deauthenticate_port(connector) < 0)
> >  		DRM_DEBUG_KMS("Port deauth failed.\n");
> >  
> > -	connector->hdcp.hdcp2_encrypted = false;
> > +	hdcp->hdcp2_encrypted = false;
> >  
> > +	memset(hdcp->topology_info, 0, sizeof(struct hdcp_topology_info));
> > +	drm_connector_update_hdcp_topology_property(&connector->base,
> > +						    hdcp->topology_info);
> >  	return ret;
> >  }
> >  
> > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> > index 733db7283e57..22f5940a47cc 100644
> > --- a/include/uapi/drm/drm_mode.h
> > +++ b/include/uapi/drm/drm_mode.h
> > @@ -223,6 +223,9 @@ struct hdcp_topology_info {
> >  	/* Version of HDCP authenticated (1.4/2.2) */
> >  	__u32 ver_in_force;
> >  
> > +	/* Applicable only for HDCP2.2 */
> > +	__u8 content_type;
> 
> Even more misplaced hunks. You can't change an uapi struct once it's baked
> in.
I did this under the assumption that the whole series completes the
uAPI.
> 
> Same thought as on the hdcp1 patch, perhaps we even want 2 functions: One
> for hdcp1, and one for hdcp2, to avoid the possiblity of screw-ups. After
> all this interface is fairly fragile, and supposed to work across drivers.
> 
> Aside: Where's the userspace for this? Kinda wondering how an open source
> hdcp repeater implementation would even work ...
I am afraid that we wont be able to preare the userspace for the
topology property. So we wont be able upstream this new uAPI. :(

-Ram
> -Daniel
> 
> > +
> >  	/* KSV of immediate HDCP Sink. In Little-Endian Format. */
> >  	char bksv[DRM_MODE_HDCP_KSV_LEN];
> >  
> > -- 
> > 2.19.1
> > 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 08/10] drm/i915: Populate downstream info for HDCP2.2

[Daniel Vetter]

On Wed, Mar 27, 2019 at 08:19:55PM +0530, Ramalingam C wrote:
> On 2019-03-27 at 11:31:12 +0100, Daniel Vetter wrote:
> > On Fri, Mar 22, 2019 at 06:14:46AM +0530, Ramalingam C wrote:
> > > Populates the downstream info for HDCP2.2 encryption also. On success
> > > of encryption Blob is updated.
> > > 
> > > Additional two variable are added to downstream info blob. Such as
> > > ver_in_force and content type.
> > > 
> > > v2:
> > >   s/cp_downstream/content_protection_downstream [daniel]
> > > v3:
> > >   s/content_protection_downstream/hdcp_topology [daniel]
> > > 
> > > Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_hdcp.c | 26 +++++++++++++++++++++++++-
> > >  include/uapi/drm/drm_mode.h       |  3 +++
> > >  2 files changed, 28 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
> > > index 0e3d7afecd5a..1eea553cdf81 100644
> > > --- a/drivers/gpu/drm/i915/intel_hdcp.c
> > > +++ b/drivers/gpu/drm/i915/intel_hdcp.c
> > > @@ -1281,6 +1281,12 @@ static int hdcp2_authentication_key_exchange(struct intel_connector *connector)
> > >  		return -EPERM;
> > >  	}
> > >  
> > > +	hdcp->topology_info->ver_in_force = DRM_MODE_HDCP22_IN_FORCE;
> > > +	hdcp->topology_info->content_type = hdcp->content_type;
> > > +	memcpy(hdcp->topology_info->bksv, msgs.send_cert.cert_rx.receiver_id,
> > > +	       HDCP_2_2_RECEIVER_ID_LEN);
> > > +	hdcp->topology_info->is_repeater = hdcp->is_repeater;
> > > +
> > >  	/*
> > >  	 * Here msgs.no_stored_km will hold msgs corresponding to the km
> > >  	 * stored also.
> > > @@ -1472,6 +1478,11 @@ int hdcp2_authenticate_repeater_topology(struct intel_connector *connector)
> > >  		return -EPERM;
> > >  	}
> > >  
> > > +	hdcp->topology_info->device_count = device_cnt;
> > > +	hdcp->topology_info->depth = HDCP_2_2_DEPTH(rx_info[0]);
> > > +	memcpy(hdcp->topology_info->ksv_list, msgs.recvid_list.receiver_ids,
> > > +	       device_cnt * HDCP_2_2_RECEIVER_ID_LEN);
> > > +
> > >  	ret = hdcp2_verify_rep_topology_prepare_ack(connector,
> > >  						    &msgs.recvid_list,
> > >  						    &msgs.rep_ack);
> > > @@ -1658,6 +1669,12 @@ static int _intel_hdcp2_enable(struct intel_connector *connector)
> > >  	if (ret) {
> > >  		DRM_DEBUG_KMS("HDCP2 Type%d  Enabling Failed. (%d)\n",
> > >  			      hdcp->content_type, ret);
> > > +
> > > +		memset(hdcp->topology_info, 0,
> > > +		       sizeof(struct hdcp_topology_info));
> > > +		drm_connector_update_hdcp_topology_property(&connector->base,
> > > +							  hdcp->topology_info);
> > > +
> > >  		return ret;
> > >  	}
> > >  
> > > @@ -1665,12 +1682,16 @@ static int _intel_hdcp2_enable(struct intel_connector *connector)
> > >  		      connector->base.name, connector->base.base.id,
> > >  		      hdcp->content_type);
> > >  
> > > +	drm_connector_update_hdcp_topology_property(&connector->base,
> > > +						    hdcp->topology_info);
> > >  	hdcp->hdcp2_encrypted = true;
> > > +
> > >  	return 0;
> > >  }
> > >  
> > >  static int _intel_hdcp2_disable(struct intel_connector *connector)
> > >  {
> > > +	struct intel_hdcp *hdcp = &connector->hdcp;
> > >  	int ret;
> > >  
> > >  	DRM_DEBUG_KMS("[%s:%d] HDCP2.2 is being Disabled\n",
> > > @@ -1681,8 +1702,11 @@ static int _intel_hdcp2_disable(struct intel_connector *connector)
> > >  	if (hdcp2_deauthenticate_port(connector) < 0)
> > >  		DRM_DEBUG_KMS("Port deauth failed.\n");
> > >  
> > > -	connector->hdcp.hdcp2_encrypted = false;
> > > +	hdcp->hdcp2_encrypted = false;
> > >  
> > > +	memset(hdcp->topology_info, 0, sizeof(struct hdcp_topology_info));
> > > +	drm_connector_update_hdcp_topology_property(&connector->base,
> > > +						    hdcp->topology_info);
> > >  	return ret;
> > >  }
> > >  
> > > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> > > index 733db7283e57..22f5940a47cc 100644
> > > --- a/include/uapi/drm/drm_mode.h
> > > +++ b/include/uapi/drm/drm_mode.h
> > > @@ -223,6 +223,9 @@ struct hdcp_topology_info {
> > >  	/* Version of HDCP authenticated (1.4/2.2) */
> > >  	__u32 ver_in_force;
> > >  
> > > +	/* Applicable only for HDCP2.2 */
> > > +	__u8 content_type;
> > 
> > Even more misplaced hunks. You can't change an uapi struct once it's baked
> > in.
> I did this under the assumption that the whole series completes the
> uAPI.

Unusual approach, since if someone e.g. cherry-picks only hdcp1.x support,
then they end up with different uapi. Also makes it much harder to review
structure alignment and stuff like that.

We can generally extend uapi structs later on, but only at the end. And
unfortunately not for properties.

> > Same thought as on the hdcp1 patch, perhaps we even want 2 functions: One
> > for hdcp1, and one for hdcp2, to avoid the possiblity of screw-ups. After
> > all this interface is fairly fragile, and supposed to work across drivers.
> > 
> > Aside: Where's the userspace for this? Kinda wondering how an open source
> > hdcp repeater implementation would even work ...
> I am afraid that we wont be able to preare the userspace for the
> topology property. So we wont be able upstream this new uAPI. :(

Is there some other useful thing we can do with this? Being able to
support the repeater usecase is kinda neat ... But yeah I guess that needs
access to private hdcp key and all that.
-Daniel

> 
> -Ram
> > -Daniel
> > 
> > > +
> > >  	/* KSV of immediate HDCP Sink. In Little-Endian Format. */
> > >  	char bksv[DRM_MODE_HDCP_KSV_LEN];
> > >  
> > > -- 
> > > 2.19.1
> > > 
> > 
> > -- 
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v3 09/10] drm: uevent for connector status change

[Ramalingam C]

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

Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
---
 drivers/gpu/drm/drm_sysfs.c | 28 ++++++++++++++++++++++++++++
 include/drm/drm_sysfs.h     |  5 ++++-
 2 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index ecb7b33002bb..c76c65ca4122 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -330,6 +330,34 @@ 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 it property id related to status
+ * changed.
+ */
+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 };
+
+	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

Re: [PATCH v3 09/10] drm: uevent for connector status change

[Daniel Vetter]

On Fri, Mar 22, 2019 at 06:14:47AM +0530, Ramalingam C wrote:
> DRM API for generating uevent for a status changes of connector's
> property.
> 
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> ---
>  drivers/gpu/drm/drm_sysfs.c | 28 ++++++++++++++++++++++++++++
>  include/drm/drm_sysfs.h     |  5 ++++-
>  2 files changed, 32 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> index ecb7b33002bb..c76c65ca4122 100644
> --- a/drivers/gpu/drm/drm_sysfs.c
> +++ b/drivers/gpu/drm/drm_sysfs.c
> @@ -330,6 +330,34 @@ 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 it property id related to status
> + * changed.

... and the connector id aling with the property id related to the status
change.

With that: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> + */
> +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 };
> +
> +	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
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 09/10] drm: uevent for connector status change

[Daniel Vetter]

On Fri, Mar 22, 2019 at 06:14:47AM +0530, Ramalingam C wrote:
> DRM API for generating uevent for a status changes of connector's
> property.
> 
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> ---
>  drivers/gpu/drm/drm_sysfs.c | 28 ++++++++++++++++++++++++++++
>  include/drm/drm_sysfs.h     |  5 ++++-
>  2 files changed, 32 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> index ecb7b33002bb..c76c65ca4122 100644
> --- a/drivers/gpu/drm/drm_sysfs.c
> +++ b/drivers/gpu/drm/drm_sysfs.c
> @@ -330,6 +330,34 @@ void drm_sysfs_hotplug_event(struct drm_device *dev)
>  }
>  EXPORT_SYMBOL(drm_sysfs_hotplug_event);

Maybe add a comment to the kerneldoc of drm_sysfs_hotplug_event() that new
uapi should use drm_sysfs_connector_status_event().
-Daniel

>  
> +/**
> + * 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 it property id related to status
> + * changed.
> + */
> +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 };
> +
> +	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
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 09/10] drm: uevent for connector status change

[Ramalingam C]

On 2019-03-27 at 12:01:37 +0100, Daniel Vetter wrote:
> On Fri, Mar 22, 2019 at 06:14:47AM +0530, Ramalingam C wrote:
> > DRM API for generating uevent for a status changes of connector's
> > property.
> > 
> > Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> > ---
> >  drivers/gpu/drm/drm_sysfs.c | 28 ++++++++++++++++++++++++++++
> >  include/drm/drm_sysfs.h     |  5 ++++-
> >  2 files changed, 32 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> > index ecb7b33002bb..c76c65ca4122 100644
> > --- a/drivers/gpu/drm/drm_sysfs.c
> > +++ b/drivers/gpu/drm/drm_sysfs.c
> > @@ -330,6 +330,34 @@ void drm_sysfs_hotplug_event(struct drm_device *dev)
> >  }
> >  EXPORT_SYMBOL(drm_sysfs_hotplug_event);
> 
> Maybe add a comment to the kerneldoc of drm_sysfs_hotplug_event() that new
> uapi should use drm_sysfs_connector_status_event().
Sure.

-Ram
> -Daniel
> 
> >  
> > +/**
> > + * 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 it property id related to status
> > + * changed.
> > + */
> > +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 };
> > +
> > +	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
> > 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v3 10/10] drm/i915: uevent for HDCP status change

[Ramalingam C]

Invoking the uevent generator for the content protection property state
change of a connector. This helps the userspace to detect the new state
change without polling the property val.

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

diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
index 1eea553cdf81..1c38026f4de7 100644
--- a/drivers/gpu/drm/i915/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/intel_hdcp.c
@@ -8,6 +8,7 @@
 
 #include <drm/drm_hdcp.h>
 #include <drm/i915_component.h>
+#include <drm/drm_sysfs.h>
 #include <linux/i2c.h>
 #include <linux/random.h>
 #include <linux/component.h>
@@ -19,6 +20,14 @@
 #define ENCRYPT_STATUS_CHANGE_TIMEOUT_MS	50
 #define HDCP2_LC_RETRY_CNT			3
 
+static inline
+void intel_hdcp_state_change_event(struct drm_connector *connector)
+{
+	struct drm_property *property = connector->content_protection_property;
+
+	drm_sysfs_connector_status_event(connector, property);
+}
+
 static
 bool intel_hdcp_is_ksv_valid(u8 *ksv)
 {
@@ -943,6 +952,7 @@ static void intel_hdcp_prop_work(struct work_struct *work)
 	if (hdcp->value != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
 		state = connector->base.state;
 		state->content_protection = hdcp->value;
+		intel_hdcp_state_change_event(&connector->base);
 	}
 
 	mutex_unlock(&hdcp->mutex);
@@ -2206,6 +2216,7 @@ int intel_hdcp_disable(struct intel_connector *connector)
 			ret = _intel_hdcp2_disable(connector);
 		else if (hdcp->hdcp_encrypted)
 			ret = _intel_hdcp_disable(connector);
+		intel_hdcp_state_change_event(&connector->base);
 	}
 
 	mutex_unlock(&hdcp->mutex);
-- 
2.19.1

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

Re: [PATCH v3 10/10] drm/i915: uevent for HDCP status change

[Daniel Vetter]

On Fri, Mar 22, 2019 at 06:14:48AM +0530, Ramalingam C wrote:
> Invoking the uevent generator for the content protection property state
> change of a connector. This helps the userspace to detect the new state
> change without polling the property val.
> 
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_hdcp.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
> index 1eea553cdf81..1c38026f4de7 100644
> --- a/drivers/gpu/drm/i915/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/intel_hdcp.c
> @@ -8,6 +8,7 @@
>  
>  #include <drm/drm_hdcp.h>
>  #include <drm/i915_component.h>
> +#include <drm/drm_sysfs.h>
>  #include <linux/i2c.h>
>  #include <linux/random.h>
>  #include <linux/component.h>
> @@ -19,6 +20,14 @@
>  #define ENCRYPT_STATUS_CHANGE_TIMEOUT_MS	50
>  #define HDCP2_LC_RETRY_CNT			3
>  
> +static inline
> +void intel_hdcp_state_change_event(struct drm_connector *connector)
> +{
> +	struct drm_property *property = connector->content_protection_property;
> +
> +	drm_sysfs_connector_status_event(connector, property);
> +}
> +
>  static
>  bool intel_hdcp_is_ksv_valid(u8 *ksv)
>  {
> @@ -943,6 +952,7 @@ static void intel_hdcp_prop_work(struct work_struct *work)
>  	if (hdcp->value != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
>  		state = connector->base.state;
>  		state->content_protection = hdcp->value;
> +		intel_hdcp_state_change_event(&connector->base);

I think it'd be good to have a helper to update both
state->content_protection and send out the event. Locking is a bit
complicated, so we also need a lockdep assert to make sure
dev->mode_config.connection_mutex is held.

That way I hope that any update in the property will actually result in
the event being sent out, and not accidentally forgotten.

>  	}
>  
>  	mutex_unlock(&hdcp->mutex);
> @@ -2206,6 +2216,7 @@ int intel_hdcp_disable(struct intel_connector *connector)
>  			ret = _intel_hdcp2_disable(connector);
>  		else if (hdcp->hdcp_encrypted)
>  			ret = _intel_hdcp_disable(connector);
> +		intel_hdcp_state_change_event(&connector->base);

Why do we need this here? We don't update the property here ...
-Daniel

>  	}
>  
>  	mutex_unlock(&hdcp->mutex);
> -- 
> 2.19.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 10/10] drm/i915: uevent for HDCP status change

[Ramalingam C]

On 2019-03-27 at 12:06:39 +0100, Daniel Vetter wrote:
> On Fri, Mar 22, 2019 at 06:14:48AM +0530, Ramalingam C wrote:
> > Invoking the uevent generator for the content protection property state
> > change of a connector. This helps the userspace to detect the new state
> > change without polling the property val.
> > 
> > Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_hdcp.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
> > index 1eea553cdf81..1c38026f4de7 100644
> > --- a/drivers/gpu/drm/i915/intel_hdcp.c
> > +++ b/drivers/gpu/drm/i915/intel_hdcp.c
> > @@ -8,6 +8,7 @@
> >  
> >  #include <drm/drm_hdcp.h>
> >  #include <drm/i915_component.h>
> > +#include <drm/drm_sysfs.h>
> >  #include <linux/i2c.h>
> >  #include <linux/random.h>
> >  #include <linux/component.h>
> > @@ -19,6 +20,14 @@
> >  #define ENCRYPT_STATUS_CHANGE_TIMEOUT_MS	50
> >  #define HDCP2_LC_RETRY_CNT			3
> >  
> > +static inline
> > +void intel_hdcp_state_change_event(struct drm_connector *connector)
> > +{
> > +	struct drm_property *property = connector->content_protection_property;
> > +
> > +	drm_sysfs_connector_status_event(connector, property);
> > +}
> > +
> >  static
> >  bool intel_hdcp_is_ksv_valid(u8 *ksv)
> >  {
> > @@ -943,6 +952,7 @@ static void intel_hdcp_prop_work(struct work_struct *work)
> >  	if (hdcp->value != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
> >  		state = connector->base.state;
> >  		state->content_protection = hdcp->value;
> > +		intel_hdcp_state_change_event(&connector->base);
> 
> I think it'd be good to have a helper to update both
> state->content_protection and send out the event.
but adding this in intel_hdcp_prop_work also does the same. Do you
suggest we need to move the intel_hdcp_state_change out of prop_work and
put both of them in a wrapper?

> Locking is a bit
> complicated, so we also need a lockdep assert to make sure
> dev->mode_config.connection_mutex is held.
in intel_hdcp_state_change ?
> 
> That way I hope that any update in the property will actually result in
> the event being sent out, and not accidentally forgotten.
> 
> >  	}
> >  
> >  	mutex_unlock(&hdcp->mutex);
> > @@ -2206,6 +2216,7 @@ int intel_hdcp_disable(struct intel_connector *connector)
> >  			ret = _intel_hdcp2_disable(connector);
> >  		else if (hdcp->hdcp_encrypted)
> >  			ret = _intel_hdcp_disable(connector);
> > +		intel_hdcp_state_change_event(&connector->base);
> 
> Why do we need this here? We don't update the property here ...
Change is requested from the userspace. So we dont update the property
state in conn_state. But in rethinking over it, only when kernel
triggers the change of property state we need the uevent. So we dont
need it here as it is triggered from the userspace.

-Ram
> -Daniel
> 
> >  	}
> >  
> >  	mutex_unlock(&hdcp->mutex);
> > -- 
> > 2.19.1
> > 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 10/10] drm/i915: uevent for HDCP status change

[Daniel Vetter]

On Wed, Mar 27, 2019 at 08:07:39PM +0530, Ramalingam C wrote:
> On 2019-03-27 at 12:06:39 +0100, Daniel Vetter wrote:
> > On Fri, Mar 22, 2019 at 06:14:48AM +0530, Ramalingam C wrote:
> > > Invoking the uevent generator for the content protection property state
> > > change of a connector. This helps the userspace to detect the new state
> > > change without polling the property val.
> > > 
> > > Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_hdcp.c | 11 +++++++++++
> > >  1 file changed, 11 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
> > > index 1eea553cdf81..1c38026f4de7 100644
> > > --- a/drivers/gpu/drm/i915/intel_hdcp.c
> > > +++ b/drivers/gpu/drm/i915/intel_hdcp.c
> > > @@ -8,6 +8,7 @@
> > >  
> > >  #include <drm/drm_hdcp.h>
> > >  #include <drm/i915_component.h>
> > > +#include <drm/drm_sysfs.h>
> > >  #include <linux/i2c.h>
> > >  #include <linux/random.h>
> > >  #include <linux/component.h>
> > > @@ -19,6 +20,14 @@
> > >  #define ENCRYPT_STATUS_CHANGE_TIMEOUT_MS	50
> > >  #define HDCP2_LC_RETRY_CNT			3
> > >  
> > > +static inline
> > > +void intel_hdcp_state_change_event(struct drm_connector *connector)
> > > +{
> > > +	struct drm_property *property = connector->content_protection_property;
> > > +
> > > +	drm_sysfs_connector_status_event(connector, property);
> > > +}
> > > +
> > >  static
> > >  bool intel_hdcp_is_ksv_valid(u8 *ksv)
> > >  {
> > > @@ -943,6 +952,7 @@ static void intel_hdcp_prop_work(struct work_struct *work)
> > >  	if (hdcp->value != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
> > >  		state = connector->base.state;
> > >  		state->content_protection = hdcp->value;
> > > +		intel_hdcp_state_change_event(&connector->base);
> > 
> > I think it'd be good to have a helper to update both
> > state->content_protection and send out the event.
> but adding this in intel_hdcp_prop_work also does the same. Do you
> suggest we need to move the intel_hdcp_state_change out of prop_work and
> put both of them in a wrapper?

Yeah something like:

drm_hdcp_update_content_protecion()
{
	assert_lock_held(dev->mode_config.connnection_mutex);
	state->content_protection = hdcp->value;
	drm_sysfs_connector_status_event(&connector->base,
					dev->mode_config.content_protection_prop);
}

And then use that instead of both the assignement and the call to send out
the event in intel_hdcp_prop_work().

> 
> > Locking is a bit
> > complicated, so we also need a lockdep assert to make sure
> > dev->mode_config.connection_mutex is held.
> in intel_hdcp_state_change ?
> > 
> > That way I hope that any update in the property will actually result in
> > the event being sent out, and not accidentally forgotten.
> > 
> > >  	}
> > >  
> > >  	mutex_unlock(&hdcp->mutex);
> > > @@ -2206,6 +2216,7 @@ int intel_hdcp_disable(struct intel_connector *connector)
> > >  			ret = _intel_hdcp2_disable(connector);
> > >  		else if (hdcp->hdcp_encrypted)
> > >  			ret = _intel_hdcp_disable(connector);
> > > +		intel_hdcp_state_change_event(&connector->base);
> > 
> > Why do we need this here? We don't update the property here ...
> Change is requested from the userspace. So we dont update the property
> state in conn_state. But in rethinking over it, only when kernel
> triggers the change of property state we need the uevent. So we dont
> need it here as it is triggered from the userspace.

Yeah, I think that makes sense. Otherwise we might end up in some funny
loop:
1. userspace updates property
2. kernel sends out uevent
3. userspace handles uevent, again resets property, goto 2

Or something silly like that :-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx