dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/4] HDCP1.4 fixes
@ 2018-03-29 14:09 Ramalingam C
  2018-03-29 14:09 ` [PATCH v2 1/4] drm/i915: Read HDCP R0 thrice in case of mismatch Ramalingam C
                   ` (4 more replies)
  0 siblings, 5 replies; 17+ messages in thread
From: Ramalingam C @ 2018-03-29 14:09 UTC (permalink / raw)
  To: intel-gfx, dri-devel, seanpaul; +Cc: rodrigo.vivi

First two patches needed for below DP HDCP compliance tests
	1A-06 and 1B-05

Third patch fixes the HDCP1.4 Key loadability check. where as fourth
one fixes the downstream device count read.

Fix for HDMI HDCP1.4 CTS tests: 1A-04 and 1A-07a are functional.
But the change from v2, as thinking to put through more regressive
testing before upstreaming.

Ramalingam C (4):
  drm/i915: Read HDCP R0 thrice in case of mismatch
  drm/i915: Read Vprime thrice incase of mismatch
  drm/i915: Check hdcp key loadability
  drm/i915: Fix downstream dev count read

 drivers/gpu/drm/i915/intel_hdcp.c | 184 ++++++++++++++++++++++++++------------
 include/drm/drm_hdcp.h            |   2 +-
 2 files changed, 129 insertions(+), 57 deletions(-)

-- 
2.7.4

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v2 1/4] drm/i915: Read HDCP R0 thrice in case of mismatch
  2018-03-29 14:09 [PATCH v2 0/4] HDCP1.4 fixes Ramalingam C
@ 2018-03-29 14:09 ` Ramalingam C
  2018-03-29 14:35   ` Sean Paul
  2018-03-29 14:09 ` [PATCH v2 2/4] drm/i915: Read Vprime thrice incase " Ramalingam C
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 17+ messages in thread
From: Ramalingam C @ 2018-03-29 14:09 UTC (permalink / raw)
  To: intel-gfx, dri-devel, seanpaul; +Cc: rodrigo.vivi

As per DP spec when R0 mismatch is detected, HDCP source supported
re-read the R0 atleast twice.

And For HDMI and DP minimum wait required for the R0 availability is
100mSec. So this patch changes the wait time to 100mSec but retries
twice with the time interval of 100mSec for each attempt.

This patch is needed for DP HDCP1.4 CTS Test: 1A-06.

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

diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
index 14ca5d3057a7..96b9025dc759 100644
--- a/drivers/gpu/drm/i915/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/intel_hdcp.c
@@ -496,9 +496,11 @@ static int intel_hdcp_auth(struct intel_digital_port *intel_dig_port,
 	}
 
 	/*
-	 * Wait for R0' to become available. The spec says 100ms from Aksv, but
-	 * some monitors can take longer than this. We'll set the timeout at
-	 * 300ms just to be sure.
+	 * Wait for R0' to become available. The spec says minimum 100ms from
+	 * Aksv, but some monitors can take longer than this. So we are
+	 * combinely waiting for 300mSec just to be sure in case of HDMI.
+	 * DP HDCP Spec mandates the two more reattempt to read R0, incase
+	 * of R0 mismatch.
 	 *
 	 * On DP, there's an R0_READY bit available but no such bit
 	 * exists on HDMI. Since the upper-bound is the same, we'll just do
@@ -506,15 +508,21 @@ static int intel_hdcp_auth(struct intel_digital_port *intel_dig_port,
 	 */
 	wait_remaining_ms_from_jiffies(r0_prime_gen_start, 300);
 
-	ri.reg = 0;
-	ret = shim->read_ri_prime(intel_dig_port, ri.shim);
-	if (ret)
-		return ret;
-	I915_WRITE(PORT_HDCP_RPRIME(port), ri.reg);
+	tries = 3;
+	for (i = 0; i < tries; i++) {
+		ri.reg = 0;
+		ret = shim->read_ri_prime(intel_dig_port, ri.shim);
+		if (ret)
+			return ret;
+		I915_WRITE(PORT_HDCP_RPRIME(port), ri.reg);
 
-	/* Wait for Ri prime match */
-	if (wait_for(I915_READ(PORT_HDCP_STATUS(port)) &
-		     (HDCP_STATUS_RI_MATCH | HDCP_STATUS_ENC), 1)) {
+		/* Wait for Ri prime match */
+		if (!wait_for(I915_READ(PORT_HDCP_STATUS(port)) &
+		    (HDCP_STATUS_RI_MATCH | HDCP_STATUS_ENC), 1))
+			break;
+	}
+
+	if (i == tries) {
 		DRM_ERROR("Timed out waiting for Ri prime match (%x)\n",
 			  I915_READ(PORT_HDCP_STATUS(port)));
 		return -ETIMEDOUT;
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v2 2/4] drm/i915: Read Vprime thrice incase of mismatch
  2018-03-29 14:09 [PATCH v2 0/4] HDCP1.4 fixes Ramalingam C
  2018-03-29 14:09 ` [PATCH v2 1/4] drm/i915: Read HDCP R0 thrice in case of mismatch Ramalingam C
@ 2018-03-29 14:09 ` Ramalingam C
  2018-03-29 14:38   ` Sean Paul
  2018-03-29 14:09 ` [PATCH v2 3/4] drm/i915: Check hdcp key loadability Ramalingam C
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 17+ messages in thread
From: Ramalingam C @ 2018-03-29 14:09 UTC (permalink / raw)
  To: intel-gfx, dri-devel, seanpaul; +Cc: rodrigo.vivi

In case of V prime mismatch, DP HDCP spec mandates the re-read of
Vprime atleast twice.

This patch needed for DP HDCP1.4 CTS Test: 1B-05.

v2:
  Moved the V' validation into a function for retry. [Sean Paul]

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

diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
index 96b9025dc759..f77d956b2b18 100644
--- a/drivers/gpu/drm/i915/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/intel_hdcp.c
@@ -142,53 +142,17 @@ bool intel_hdcp_is_ksv_valid(u8 *ksv)
 	return true;
 }
 
-/* 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)
+static inline
+int intel_hdcp_validate_v_prime(struct intel_digital_port *intel_dig_port,
+				const struct intel_hdcp_shim *shim,
+				u8 *ksv_fifo, u8 num_downstream, u8 *bstatus)
 {
 	struct drm_i915_private *dev_priv;
 	u32 vprime, sha_text, sha_leftovers, rep_ctl;
-	u8 bstatus[2], num_downstream, *ksv_fifo;
 	int ret, i, j, sha_idx;
 
 	dev_priv = intel_dig_port->base.base.dev->dev_private;
 
-	ret = intel_hdcp_poll_ksv_fifo(intel_dig_port, shim);
-	if (ret) {
-		DRM_ERROR("KSV list failed to become ready (%d)\n", ret);
-		return ret;
-	}
-
-	ret = shim->read_bstatus(intel_dig_port, bstatus);
-	if (ret)
-		return ret;
-
-	if (DRM_HDCP_MAX_DEVICE_EXCEEDED(bstatus[0]) ||
-	    DRM_HDCP_MAX_CASCADE_EXCEEDED(bstatus[1])) {
-		DRM_ERROR("Max Topology Limit Exceeded\n");
-		return -EPERM;
-	}
-
-	/*
-	 * When repeater reports 0 device count, HDCP1.4 spec allows disabling
-	 * the HDCP encryption. That implies that repeater can't have its own
-	 * display. As there is no consumption of encrypted content in the
-	 * repeater with 0 downstream devices, we are failing the
-	 * authentication.
-	 */
-	num_downstream = DRM_HDCP_NUM_DOWNSTREAM(bstatus[0]);
-	if (num_downstream == 0)
-		return -EINVAL;
-
-	ksv_fifo = kzalloc(num_downstream * DRM_HDCP_KSV_LEN, GFP_KERNEL);
-	if (!ksv_fifo)
-		return -ENOMEM;
-
-	ret = shim->read_ksv_fifo(intel_dig_port, num_downstream, ksv_fifo);
-	if (ret)
-		return ret;
-
 	/* Process V' values from the receiver */
 	for (i = 0; i < DRM_HDCP_V_PRIME_NUM_PARTS; i++) {
 		ret = shim->read_v_prime_part(intel_dig_port, i, &vprime);
@@ -353,7 +317,7 @@ int intel_hdcp_auth_downstream(struct intel_digital_port *intel_dig_port,
 			return ret;
 		sha_idx += sizeof(sha_text);
 	} else {
-		DRM_ERROR("Invalid number of leftovers %d\n", sha_leftovers);
+		DRM_DEBUG("Invalid number of leftovers %d\n", sha_leftovers);
 		return -EINVAL;
 	}
 
@@ -381,14 +345,77 @@ int intel_hdcp_auth_downstream(struct intel_digital_port *intel_dig_port,
 	if (intel_wait_for_register(dev_priv, HDCP_REP_CTL,
 				    HDCP_SHA1_COMPLETE,
 				    HDCP_SHA1_COMPLETE, 1)) {
-		DRM_ERROR("Timed out waiting for SHA1 complete\n");
+		DRM_DEBUG("Timed out waiting for SHA1 complete\n");
 		return -ETIMEDOUT;
 	}
 	if (!(I915_READ(HDCP_REP_CTL) & HDCP_SHA1_V_MATCH)) {
-		DRM_ERROR("SHA-1 mismatch, HDCP failed\n");
+		DRM_DEBUG("SHA-1 mismatch, HDCP failed\n");
 		return -ENXIO;
 	}
 
+	return 0;
+}
+
+/* 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)
+{
+	u8 bstatus[2], num_downstream, *ksv_fifo;
+	int ret, i, tries = 3;
+
+	ret = intel_hdcp_poll_ksv_fifo(intel_dig_port, shim);
+	if (ret) {
+		DRM_ERROR("KSV list failed to become ready (%d)\n", ret);
+		return ret;
+	}
+
+	ret = shim->read_bstatus(intel_dig_port, bstatus);
+	if (ret)
+		return ret;
+
+	if (DRM_HDCP_MAX_DEVICE_EXCEEDED(bstatus[0]) ||
+	    DRM_HDCP_MAX_CASCADE_EXCEEDED(bstatus[1])) {
+		DRM_ERROR("Max Topology Limit Exceeded\n");
+		return -EPERM;
+	}
+
+	/*
+	 * When repeater reports 0 device count, HDCP1.4 spec allows disabling
+	 * the HDCP encryption. That implies that repeater can't have its own
+	 * display. As there is no consumption of encrypted content in the
+	 * repeater with 0 downstream devices, we are failing the
+	 * authentication.
+	 */
+	num_downstream = DRM_HDCP_NUM_DOWNSTREAM(bstatus[0]);
+	if (num_downstream == 0)
+		return -EINVAL;
+
+	ksv_fifo = kzalloc(num_downstream * DRM_HDCP_KSV_LEN, GFP_KERNEL);
+	if (!ksv_fifo)
+		return -ENOMEM;
+
+	ret = shim->read_ksv_fifo(intel_dig_port, num_downstream, ksv_fifo);
+	if (ret)
+		return ret;
+
+	/*
+	 * When V prime mismatches, DP Spec mandates re-read of
+	 * V prime atleast twice.
+	 */
+	for (i = 0; i < tries; i++) {
+		ret = intel_hdcp_validate_v_prime(intel_dig_port, shim,
+						  ksv_fifo, num_downstream,
+						  bstatus);
+		if (!ret)
+			break;
+	}
+
+	if (i == tries) {
+		DRM_ERROR("V Prime validation failed.(%d)\n", ret);
+		return ret;
+	}
+
 	DRM_DEBUG_KMS("HDCP is enabled (%d downstream devices)\n",
 		      num_downstream);
 	return 0;
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v2 3/4] drm/i915: Check hdcp key loadability
  2018-03-29 14:09 [PATCH v2 0/4] HDCP1.4 fixes Ramalingam C
  2018-03-29 14:09 ` [PATCH v2 1/4] drm/i915: Read HDCP R0 thrice in case of mismatch Ramalingam C
  2018-03-29 14:09 ` [PATCH v2 2/4] drm/i915: Read Vprime thrice incase " Ramalingam C
@ 2018-03-29 14:09 ` Ramalingam C
  2018-03-29 14:24   ` Ville Syrjälä
  2018-03-29 14:09 ` [PATCH v2 4/4] drm/i915: Fix downstream dev count read Ramalingam C
  2018-03-29 14:12 ` [PATCH v2 0/4] HDCP1.4 fixes Ramalingam C
  4 siblings, 1 reply; 17+ messages in thread
From: Ramalingam C @ 2018-03-29 14:09 UTC (permalink / raw)
  To: intel-gfx, dri-devel, seanpaul; +Cc: rodrigo.vivi

HDCP1.4 key can be loaded, only when Power well #1 is enabled and cdclk
is enabled. Using the I915 power well infrastruture, above requirement
is verified.

This patch enables the hdcp initialization for HSW, BDW, and BXT.

v2:
  Choose the PW# based on the platform.

Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
Reviewed-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/i915/intel_hdcp.c | 41 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 39 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
index f77d956b2b18..d8af09b46a44 100644
--- a/drivers/gpu/drm/i915/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/intel_hdcp.c
@@ -37,6 +37,43 @@ static int intel_hdcp_poll_ksv_fifo(struct intel_digital_port *intel_dig_port,
 	return 0;
 }
 
+static bool hdcp_key_loadable(struct drm_i915_private *dev_priv)
+{
+	struct i915_power_domains *power_domains = &dev_priv->power_domains;
+	struct i915_power_well *power_well;
+	enum i915_power_well_id id;
+	bool enabled = false;
+
+	/*
+	 * On HSW and BDW, Display HW loads the Key as soon as Display resumes.
+	 * On all BXT+, SW can load the keys only when the PW#1 is turned on.
+	 */
+	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
+		id = HSW_DISP_PW_GLOBAL;
+	else
+		id = SKL_DISP_PW_1;
+
+	mutex_lock(&power_domains->lock);
+
+	/* PG1 (power well #1) needs to be enabled */
+	for_each_power_well(dev_priv, power_well) {
+		if (power_well->id == id) {
+			enabled = power_well->ops->is_enabled(dev_priv,
+							      power_well);
+			break;
+		}
+	}
+	mutex_unlock(&power_domains->lock);
+
+	/*
+	 * Another req for hdcp key loadability is enabled state of pll for
+	 * cdclk. Without active crtc we wont land here. So we are assuming that
+	 * cdclk is already on.
+	 */
+
+	return enabled;
+}
+
 static void intel_hdcp_clear_keys(struct drm_i915_private *dev_priv)
 {
 	I915_WRITE(HDCP_KEY_CONF, HDCP_CLEAR_KEYS_TRIGGER);
@@ -615,8 +652,8 @@ static int _intel_hdcp_enable(struct intel_connector *connector)
 	DRM_DEBUG_KMS("[%s:%d] HDCP is being enabled...\n",
 		      connector->base.name, connector->base.base.id);
 
-	if (!(I915_READ(SKL_FUSE_STATUS) & SKL_FUSE_PG_DIST_STATUS(1))) {
-		DRM_ERROR("PG1 is disabled, cannot load keys\n");
+	if (!hdcp_key_loadable(dev_priv)) {
+		DRM_ERROR("HDCP key Load is not possible\n");
 		return -ENXIO;
 	}
 
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v2 4/4] drm/i915: Fix downstream dev count read
  2018-03-29 14:09 [PATCH v2 0/4] HDCP1.4 fixes Ramalingam C
                   ` (2 preceding siblings ...)
  2018-03-29 14:09 ` [PATCH v2 3/4] drm/i915: Check hdcp key loadability Ramalingam C
@ 2018-03-29 14:09 ` Ramalingam C
  2018-03-29 14:39   ` Sean Paul
  2018-03-29 14:12 ` [PATCH v2 0/4] HDCP1.4 fixes Ramalingam C
  4 siblings, 1 reply; 17+ messages in thread
From: Ramalingam C @ 2018-03-29 14:09 UTC (permalink / raw)
  To: intel-gfx, dri-devel, seanpaul; +Cc: rodrigo.vivi

In both HDMI and DP, device count is represented by 6:0 bits of a
register(BInfo/Bstatus)

So macro for bitmasking the device_count is fixed(0x3F->0x7F).

Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
cc: Sean Paul <seanpaul@chromium.org>
---
 include/drm/drm_hdcp.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/drm/drm_hdcp.h b/include/drm/drm_hdcp.h
index 562fa7df2637..98e63d870139 100644
--- a/include/drm/drm_hdcp.h
+++ b/include/drm/drm_hdcp.h
@@ -19,7 +19,7 @@
 #define DRM_HDCP_RI_LEN				2
 #define DRM_HDCP_V_PRIME_PART_LEN		4
 #define DRM_HDCP_V_PRIME_NUM_PARTS		5
-#define DRM_HDCP_NUM_DOWNSTREAM(x)		(x & 0x3f)
+#define DRM_HDCP_NUM_DOWNSTREAM(x)		(x & 0x7f)
 #define DRM_HDCP_MAX_CASCADE_EXCEEDED(x)	(x & BIT(3))
 #define DRM_HDCP_MAX_DEVICE_EXCEEDED(x)		(x & BIT(7))
 
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 0/4] HDCP1.4 fixes
  2018-03-29 14:09 [PATCH v2 0/4] HDCP1.4 fixes Ramalingam C
                   ` (3 preceding siblings ...)
  2018-03-29 14:09 ` [PATCH v2 4/4] drm/i915: Fix downstream dev count read Ramalingam C
@ 2018-03-29 14:12 ` Ramalingam C
  4 siblings, 0 replies; 17+ messages in thread
From: Ramalingam C @ 2018-03-29 14:12 UTC (permalink / raw)
  To: intel-gfx, dri-devel, seanpaul; +Cc: rodrigo.vivi


On Thursday 29 March 2018 07:39 PM, Ramalingam C wrote:
> First two patches needed for below DP HDCP compliance tests
> 	1A-06 and 1B-05
>
> Third patch fixes the HDCP1.4 Key loadability check. where as fourth
> one fixes the downstream device count read.
>
> Fix for HDMI HDCP1.4 CTS tests: 1A-04 and 1A-07a are functional.
> But the change from v2, as thinking to put through more regressive

The change is removed from v2*

All other changes are well tested on KBL.

--Ram

> testing before upstreaming.
>
> Ramalingam C (4):
>    drm/i915: Read HDCP R0 thrice in case of mismatch
>    drm/i915: Read Vprime thrice incase of mismatch
>    drm/i915: Check hdcp key loadability
>    drm/i915: Fix downstream dev count read
>
>   drivers/gpu/drm/i915/intel_hdcp.c | 184 ++++++++++++++++++++++++++------------
>   include/drm/drm_hdcp.h            |   2 +-
>   2 files changed, 129 insertions(+), 57 deletions(-)
>

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 3/4] drm/i915: Check hdcp key loadability
  2018-03-29 14:09 ` [PATCH v2 3/4] drm/i915: Check hdcp key loadability Ramalingam C
@ 2018-03-29 14:24   ` Ville Syrjälä
  2018-04-02  9:05     ` Ramalingam C
  0 siblings, 1 reply; 17+ messages in thread
From: Ville Syrjälä @ 2018-03-29 14:24 UTC (permalink / raw)
  To: Ramalingam C; +Cc: intel-gfx, dri-devel, rodrigo.vivi

On Thu, Mar 29, 2018 at 07:39:07PM +0530, Ramalingam C wrote:
> HDCP1.4 key can be loaded, only when Power well #1 is enabled and cdclk
> is enabled. Using the I915 power well infrastruture, above requirement
> is verified.
> 
> This patch enables the hdcp initialization for HSW, BDW, and BXT.
> 
> v2:
>   Choose the PW# based on the platform.
> 
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> Reviewed-by: Sean Paul <seanpaul@chromium.org>
> ---
>  drivers/gpu/drm/i915/intel_hdcp.c | 41 +++++++++++++++++++++++++++++++++++++--
>  1 file changed, 39 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
> index f77d956b2b18..d8af09b46a44 100644
> --- a/drivers/gpu/drm/i915/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/intel_hdcp.c
> @@ -37,6 +37,43 @@ static int intel_hdcp_poll_ksv_fifo(struct intel_digital_port *intel_dig_port,
>  	return 0;
>  }
>  
> +static bool hdcp_key_loadable(struct drm_i915_private *dev_priv)
> +{
> +	struct i915_power_domains *power_domains = &dev_priv->power_domains;
> +	struct i915_power_well *power_well;
> +	enum i915_power_well_id id;
> +	bool enabled = false;
> +
> +	/*
> +	 * On HSW and BDW, Display HW loads the Key as soon as Display resumes.
> +	 * On all BXT+, SW can load the keys only when the PW#1 is turned on.
> +	 */
> +	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
> +		id = HSW_DISP_PW_GLOBAL;
> +	else
> +		id = SKL_DISP_PW_1;
> +
> +	mutex_lock(&power_domains->lock);
> +
> +	/* PG1 (power well #1) needs to be enabled */
> +	for_each_power_well(dev_priv, power_well) {
> +		if (power_well->id == id) {
> +			enabled = power_well->ops->is_enabled(dev_priv,
> +							      power_well);
> +			break;
> +		}
> +	}
> +	mutex_unlock(&power_domains->lock);
> +
> +	/*
> +	 * Another req for hdcp key loadability is enabled state of pll for
> +	 * cdclk. Without active crtc we wont land here. So we are assuming that
> +	 * cdclk is already on.
> +	 */
> +
> +	return enabled;
> +}
> +
>  static void intel_hdcp_clear_keys(struct drm_i915_private *dev_priv)
>  {
>  	I915_WRITE(HDCP_KEY_CONF, HDCP_CLEAR_KEYS_TRIGGER);
> @@ -615,8 +652,8 @@ static int _intel_hdcp_enable(struct intel_connector *connector)
>  	DRM_DEBUG_KMS("[%s:%d] HDCP is being enabled...\n",
>  		      connector->base.name, connector->base.base.id);
>  
> -	if (!(I915_READ(SKL_FUSE_STATUS) & SKL_FUSE_PG_DIST_STATUS(1))) {
> -		DRM_ERROR("PG1 is disabled, cannot load keys\n");
> +	if (!hdcp_key_loadable(dev_priv)) {
> +		DRM_ERROR("HDCP key Load is not possible\n");
>  		return -ENXIO;
>  	}

If you need the power well then why aren't you grabbing the correct
power domain reference somewhere?

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 1/4] drm/i915: Read HDCP R0 thrice in case of mismatch
  2018-03-29 14:09 ` [PATCH v2 1/4] drm/i915: Read HDCP R0 thrice in case of mismatch Ramalingam C
@ 2018-03-29 14:35   ` Sean Paul
  2018-04-02  9:08     ` Ramalingam C
  0 siblings, 1 reply; 17+ messages in thread
From: Sean Paul @ 2018-03-29 14:35 UTC (permalink / raw)
  To: Ramalingam C; +Cc: intel-gfx, dri-devel, rodrigo.vivi

On Thu, Mar 29, 2018 at 07:39:05PM +0530, Ramalingam C wrote:
> As per DP spec when R0 mismatch is detected, HDCP source supported
> re-read the R0 atleast twice.
> 
> And For HDMI and DP minimum wait required for the R0 availability is
> 100mSec. So this patch changes the wait time to 100mSec but retries
> twice with the time interval of 100mSec for each attempt.
> 
> This patch is needed for DP HDCP1.4 CTS Test: 1A-06.
> 
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_hdcp.c | 30 +++++++++++++++++++-----------
>  1 file changed, 19 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
> index 14ca5d3057a7..96b9025dc759 100644
> --- a/drivers/gpu/drm/i915/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/intel_hdcp.c
> @@ -496,9 +496,11 @@ static int intel_hdcp_auth(struct intel_digital_port *intel_dig_port,
>  	}
>  
>  	/*
> -	 * Wait for R0' to become available. The spec says 100ms from Aksv, but
> -	 * some monitors can take longer than this. We'll set the timeout at
> -	 * 300ms just to be sure.
> +	 * Wait for R0' to become available. The spec says minimum 100ms from
> +	 * Aksv, but some monitors can take longer than this. So we are
> +	 * combinely waiting for 300mSec just to be sure in case of HDMI.
> +	 * DP HDCP Spec mandates the two more reattempt to read R0, incase
> +	 * of R0 mismatch.

I am sorry to nitpick comments, but this doesn't belong here. Leave this comment
alone and add the part about the DP spec requiring retries directly above the
loop where we're actually doing the retries.

Sean

>  	 *
>  	 * On DP, there's an R0_READY bit available but no such bit
>  	 * exists on HDMI. Since the upper-bound is the same, we'll just do
> @@ -506,15 +508,21 @@ static int intel_hdcp_auth(struct intel_digital_port *intel_dig_port,
>  	 */
>  	wait_remaining_ms_from_jiffies(r0_prime_gen_start, 300);
>  
> -	ri.reg = 0;
> -	ret = shim->read_ri_prime(intel_dig_port, ri.shim);
> -	if (ret)
> -		return ret;
> -	I915_WRITE(PORT_HDCP_RPRIME(port), ri.reg);
> +	tries = 3;
> +	for (i = 0; i < tries; i++) {
> +		ri.reg = 0;
> +		ret = shim->read_ri_prime(intel_dig_port, ri.shim);
> +		if (ret)
> +			return ret;
> +		I915_WRITE(PORT_HDCP_RPRIME(port), ri.reg);
>  
> -	/* Wait for Ri prime match */
> -	if (wait_for(I915_READ(PORT_HDCP_STATUS(port)) &
> -		     (HDCP_STATUS_RI_MATCH | HDCP_STATUS_ENC), 1)) {
> +		/* Wait for Ri prime match */
> +		if (!wait_for(I915_READ(PORT_HDCP_STATUS(port)) &
> +		    (HDCP_STATUS_RI_MATCH | HDCP_STATUS_ENC), 1))
> +			break;
> +	}
> +
> +	if (i == tries) {
>  		DRM_ERROR("Timed out waiting for Ri prime match (%x)\n",
>  			  I915_READ(PORT_HDCP_STATUS(port)));
>  		return -ETIMEDOUT;
> -- 
> 2.7.4
> 

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 2/4] drm/i915: Read Vprime thrice incase of mismatch
  2018-03-29 14:09 ` [PATCH v2 2/4] drm/i915: Read Vprime thrice incase " Ramalingam C
@ 2018-03-29 14:38   ` Sean Paul
  2018-04-02  9:16     ` Ramalingam C
  0 siblings, 1 reply; 17+ messages in thread
From: Sean Paul @ 2018-03-29 14:38 UTC (permalink / raw)
  To: Ramalingam C; +Cc: intel-gfx, dri-devel, rodrigo.vivi

On Thu, Mar 29, 2018 at 07:39:06PM +0530, Ramalingam C wrote:
> In case of V prime mismatch, DP HDCP spec mandates the re-read of
> Vprime atleast twice.
> 
> This patch needed for DP HDCP1.4 CTS Test: 1B-05.
> 
> v2:
>   Moved the V' validation into a function for retry. [Sean Paul]
> 
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_hdcp.c | 113 +++++++++++++++++++++++---------------
>  1 file changed, 70 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
> index 96b9025dc759..f77d956b2b18 100644
> --- a/drivers/gpu/drm/i915/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/intel_hdcp.c
> @@ -142,53 +142,17 @@ bool intel_hdcp_is_ksv_valid(u8 *ksv)
>  	return true;
>  }
>  
> -/* 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)
> +static inline

Why inline?

> +int intel_hdcp_validate_v_prime(struct intel_digital_port *intel_dig_port,
> +				const struct intel_hdcp_shim *shim,
> +				u8 *ksv_fifo, u8 num_downstream, u8 *bstatus)
>  {
>  	struct drm_i915_private *dev_priv;
>  	u32 vprime, sha_text, sha_leftovers, rep_ctl;
> -	u8 bstatus[2], num_downstream, *ksv_fifo;
>  	int ret, i, j, sha_idx;
>  
>  	dev_priv = intel_dig_port->base.base.dev->dev_private;
>  
> -	ret = intel_hdcp_poll_ksv_fifo(intel_dig_port, shim);
> -	if (ret) {
> -		DRM_ERROR("KSV list failed to become ready (%d)\n", ret);
> -		return ret;
> -	}
> -
> -	ret = shim->read_bstatus(intel_dig_port, bstatus);
> -	if (ret)
> -		return ret;
> -
> -	if (DRM_HDCP_MAX_DEVICE_EXCEEDED(bstatus[0]) ||
> -	    DRM_HDCP_MAX_CASCADE_EXCEEDED(bstatus[1])) {
> -		DRM_ERROR("Max Topology Limit Exceeded\n");
> -		return -EPERM;
> -	}
> -
> -	/*
> -	 * When repeater reports 0 device count, HDCP1.4 spec allows disabling
> -	 * the HDCP encryption. That implies that repeater can't have its own
> -	 * display. As there is no consumption of encrypted content in the
> -	 * repeater with 0 downstream devices, we are failing the
> -	 * authentication.
> -	 */
> -	num_downstream = DRM_HDCP_NUM_DOWNSTREAM(bstatus[0]);
> -	if (num_downstream == 0)
> -		return -EINVAL;
> -
> -	ksv_fifo = kzalloc(num_downstream * DRM_HDCP_KSV_LEN, GFP_KERNEL);
> -	if (!ksv_fifo)
> -		return -ENOMEM;
> -
> -	ret = shim->read_ksv_fifo(intel_dig_port, num_downstream, ksv_fifo);
> -	if (ret)
> -		return ret;
> -
>  	/* Process V' values from the receiver */
>  	for (i = 0; i < DRM_HDCP_V_PRIME_NUM_PARTS; i++) {
>  		ret = shim->read_v_prime_part(intel_dig_port, i, &vprime);
> @@ -353,7 +317,7 @@ int intel_hdcp_auth_downstream(struct intel_digital_port *intel_dig_port,
>  			return ret;
>  		sha_idx += sizeof(sha_text);
>  	} else {
> -		DRM_ERROR("Invalid number of leftovers %d\n", sha_leftovers);
> +		DRM_DEBUG("Invalid number of leftovers %d\n", sha_leftovers);
>  		return -EINVAL;
>  	}
>  
> @@ -381,14 +345,77 @@ int intel_hdcp_auth_downstream(struct intel_digital_port *intel_dig_port,
>  	if (intel_wait_for_register(dev_priv, HDCP_REP_CTL,
>  				    HDCP_SHA1_COMPLETE,
>  				    HDCP_SHA1_COMPLETE, 1)) {
> -		DRM_ERROR("Timed out waiting for SHA1 complete\n");
> +		DRM_DEBUG("Timed out waiting for SHA1 complete\n");
>  		return -ETIMEDOUT;
>  	}
>  	if (!(I915_READ(HDCP_REP_CTL) & HDCP_SHA1_V_MATCH)) {
> -		DRM_ERROR("SHA-1 mismatch, HDCP failed\n");
> +		DRM_DEBUG("SHA-1 mismatch, HDCP failed\n");

I think the DEBUG should be DEBUG_KMS, consistent with the rest of the file?

>  		return -ENXIO;
>  	}
>  
> +	return 0;
> +}
> +
> +/* 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)
> +{
> +	u8 bstatus[2], num_downstream, *ksv_fifo;
> +	int ret, i, tries = 3;
> +
> +	ret = intel_hdcp_poll_ksv_fifo(intel_dig_port, shim);
> +	if (ret) {
> +		DRM_ERROR("KSV list failed to become ready (%d)\n", ret);
> +		return ret;
> +	}
> +
> +	ret = shim->read_bstatus(intel_dig_port, bstatus);
> +	if (ret)
> +		return ret;
> +
> +	if (DRM_HDCP_MAX_DEVICE_EXCEEDED(bstatus[0]) ||
> +	    DRM_HDCP_MAX_CASCADE_EXCEEDED(bstatus[1])) {
> +		DRM_ERROR("Max Topology Limit Exceeded\n");
> +		return -EPERM;
> +	}
> +
> +	/*
> +	 * When repeater reports 0 device count, HDCP1.4 spec allows disabling
> +	 * the HDCP encryption. That implies that repeater can't have its own
> +	 * display. As there is no consumption of encrypted content in the
> +	 * repeater with 0 downstream devices, we are failing the
> +	 * authentication.
> +	 */
> +	num_downstream = DRM_HDCP_NUM_DOWNSTREAM(bstatus[0]);
> +	if (num_downstream == 0)
> +		return -EINVAL;
> +
> +	ksv_fifo = kzalloc(num_downstream * DRM_HDCP_KSV_LEN, GFP_KERNEL);
> +	if (!ksv_fifo)
> +		return -ENOMEM;
> +
> +	ret = shim->read_ksv_fifo(intel_dig_port, num_downstream, ksv_fifo);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * When V prime mismatches, DP Spec mandates re-read of
> +	 * V prime atleast twice.
> +	 */
> +	for (i = 0; i < tries; i++) {
> +		ret = intel_hdcp_validate_v_prime(intel_dig_port, shim,
> +						  ksv_fifo, num_downstream,
> +						  bstatus);
> +		if (!ret)
> +			break;
> +	}
> +
> +	if (i == tries) {
> +		DRM_ERROR("V Prime validation failed.(%d)\n", ret);
> +		return ret;
> +	}
> +
>  	DRM_DEBUG_KMS("HDCP is enabled (%d downstream devices)\n",
>  		      num_downstream);
>  	return 0;
> -- 
> 2.7.4
> 

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 4/4] drm/i915: Fix downstream dev count read
  2018-03-29 14:09 ` [PATCH v2 4/4] drm/i915: Fix downstream dev count read Ramalingam C
@ 2018-03-29 14:39   ` Sean Paul
  0 siblings, 0 replies; 17+ messages in thread
From: Sean Paul @ 2018-03-29 14:39 UTC (permalink / raw)
  To: Ramalingam C; +Cc: intel-gfx, dri-devel, rodrigo.vivi

On Thu, Mar 29, 2018 at 07:39:08PM +0530, Ramalingam C wrote:
> In both HDMI and DP, device count is represented by 6:0 bits of a
> register(BInfo/Bstatus)
> 
> So macro for bitmasking the device_count is fixed(0x3F->0x7F).
> 


Reviewed-by: Sean Paul <seanpaul@chromium.org>

> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> cc: Sean Paul <seanpaul@chromium.org>
> ---
>  include/drm/drm_hdcp.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/drm/drm_hdcp.h b/include/drm/drm_hdcp.h
> index 562fa7df2637..98e63d870139 100644
> --- a/include/drm/drm_hdcp.h
> +++ b/include/drm/drm_hdcp.h
> @@ -19,7 +19,7 @@
>  #define DRM_HDCP_RI_LEN				2
>  #define DRM_HDCP_V_PRIME_PART_LEN		4
>  #define DRM_HDCP_V_PRIME_NUM_PARTS		5
> -#define DRM_HDCP_NUM_DOWNSTREAM(x)		(x & 0x3f)
> +#define DRM_HDCP_NUM_DOWNSTREAM(x)		(x & 0x7f)
>  #define DRM_HDCP_MAX_CASCADE_EXCEEDED(x)	(x & BIT(3))
>  #define DRM_HDCP_MAX_DEVICE_EXCEEDED(x)		(x & BIT(7))
>  
> -- 
> 2.7.4
> 

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 3/4] drm/i915: Check hdcp key loadability
  2018-03-29 14:24   ` Ville Syrjälä
@ 2018-04-02  9:05     ` Ramalingam C
  2018-04-06 16:02       ` Ville Syrjälä
  0 siblings, 1 reply; 17+ messages in thread
From: Ramalingam C @ 2018-04-02  9:05 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx, dri-devel, rodrigo.vivi



On Thursday 29 March 2018 07:54 PM, Ville Syrjälä wrote:
> On Thu, Mar 29, 2018 at 07:39:07PM +0530, Ramalingam C wrote:
>> HDCP1.4 key can be loaded, only when Power well #1 is enabled and cdclk
>> is enabled. Using the I915 power well infrastruture, above requirement
>> is verified.
>>
>> This patch enables the hdcp initialization for HSW, BDW, and BXT.
>>
>> v2:
>>    Choose the PW# based on the platform.
>>
>> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
>> Reviewed-by: Sean Paul <seanpaul@chromium.org>
>> ---
>>   drivers/gpu/drm/i915/intel_hdcp.c | 41 +++++++++++++++++++++++++++++++++++++--
>>   1 file changed, 39 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
>> index f77d956b2b18..d8af09b46a44 100644
>> --- a/drivers/gpu/drm/i915/intel_hdcp.c
>> +++ b/drivers/gpu/drm/i915/intel_hdcp.c
>> @@ -37,6 +37,43 @@ static int intel_hdcp_poll_ksv_fifo(struct intel_digital_port *intel_dig_port,
>>   	return 0;
>>   }
>>   
>> +static bool hdcp_key_loadable(struct drm_i915_private *dev_priv)
>> +{
>> +	struct i915_power_domains *power_domains = &dev_priv->power_domains;
>> +	struct i915_power_well *power_well;
>> +	enum i915_power_well_id id;
>> +	bool enabled = false;
>> +
>> +	/*
>> +	 * On HSW and BDW, Display HW loads the Key as soon as Display resumes.
>> +	 * On all BXT+, SW can load the keys only when the PW#1 is turned on.
>> +	 */
>> +	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
>> +		id = HSW_DISP_PW_GLOBAL;
>> +	else
>> +		id = SKL_DISP_PW_1;
>> +
>> +	mutex_lock(&power_domains->lock);
>> +
>> +	/* PG1 (power well #1) needs to be enabled */
>> +	for_each_power_well(dev_priv, power_well) {
>> +		if (power_well->id == id) {
>> +			enabled = power_well->ops->is_enabled(dev_priv,
>> +							      power_well);
>> +			break;
>> +		}
>> +	}
>> +	mutex_unlock(&power_domains->lock);
>> +
>> +	/*
>> +	 * Another req for hdcp key loadability is enabled state of pll for
>> +	 * cdclk. Without active crtc we wont land here. So we are assuming that
>> +	 * cdclk is already on.
>> +	 */
>> +
>> +	return enabled;
>> +}
>> +
>>   static void intel_hdcp_clear_keys(struct drm_i915_private *dev_priv)
>>   {
>>   	I915_WRITE(HDCP_KEY_CONF, HDCP_CLEAR_KEYS_TRIGGER);
>> @@ -615,8 +652,8 @@ static int _intel_hdcp_enable(struct intel_connector *connector)
>>   	DRM_DEBUG_KMS("[%s:%d] HDCP is being enabled...\n",
>>   		      connector->base.name, connector->base.base.id);
>>   
>> -	if (!(I915_READ(SKL_FUSE_STATUS) & SKL_FUSE_PG_DIST_STATUS(1))) {
>> -		DRM_ERROR("PG1 is disabled, cannot load keys\n");
>> +	if (!hdcp_key_loadable(dev_priv)) {
>> +		DRM_ERROR("HDCP key Load is not possible\n");
>>   		return -ENXIO;
>>   	}
> If you need the power well then why aren't you grabbing the correct
> power domain reference somewhere?

Ville,

As HDCP is supposed to be enabled after the basic display is up, power well #1 is supposed to be enabled.
If not that is mostly an error w.r.t HDCP.

So at this point we just want to verify whether required PW is up and HDCP key can be loaded from the HW. Else fail the HDCP request.

--Ram

>

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 1/4] drm/i915: Read HDCP R0 thrice in case of mismatch
  2018-03-29 14:35   ` Sean Paul
@ 2018-04-02  9:08     ` Ramalingam C
  0 siblings, 0 replies; 17+ messages in thread
From: Ramalingam C @ 2018-04-02  9:08 UTC (permalink / raw)
  To: Sean Paul; +Cc: intel-gfx, dri-devel, rodrigo.vivi



On Thursday 29 March 2018 08:05 PM, Sean Paul wrote:
> On Thu, Mar 29, 2018 at 07:39:05PM +0530, Ramalingam C wrote:
>> As per DP spec when R0 mismatch is detected, HDCP source supported
>> re-read the R0 atleast twice.
>>
>> And For HDMI and DP minimum wait required for the R0 availability is
>> 100mSec. So this patch changes the wait time to 100mSec but retries
>> twice with the time interval of 100mSec for each attempt.
>>
>> This patch is needed for DP HDCP1.4 CTS Test: 1A-06.
>>
>> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_hdcp.c | 30 +++++++++++++++++++-----------
>>   1 file changed, 19 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
>> index 14ca5d3057a7..96b9025dc759 100644
>> --- a/drivers/gpu/drm/i915/intel_hdcp.c
>> +++ b/drivers/gpu/drm/i915/intel_hdcp.c
>> @@ -496,9 +496,11 @@ static int intel_hdcp_auth(struct intel_digital_port *intel_dig_port,
>>   	}
>>   
>>   	/*
>> -	 * Wait for R0' to become available. The spec says 100ms from Aksv, but
>> -	 * some monitors can take longer than this. We'll set the timeout at
>> -	 * 300ms just to be sure.
>> +	 * Wait for R0' to become available. The spec says minimum 100ms from
>> +	 * Aksv, but some monitors can take longer than this. So we are
>> +	 * combinely waiting for 300mSec just to be sure in case of HDMI.
>> +	 * DP HDCP Spec mandates the two more reattempt to read R0, incase
>> +	 * of R0 mismatch.
> I am sorry to nitpick comments, but this doesn't belong here. Leave this comment
> alone and add the part about the DP spec requiring retries directly above the
> loop where we're actually doing the retries.
Ok. I will take care in the next ver.

--Ram
>
> Sean
>
>>   	 *
>>   	 * On DP, there's an R0_READY bit available but no such bit
>>   	 * exists on HDMI. Since the upper-bound is the same, we'll just do
>> @@ -506,15 +508,21 @@ static int intel_hdcp_auth(struct intel_digital_port *intel_dig_port,
>>   	 */
>>   	wait_remaining_ms_from_jiffies(r0_prime_gen_start, 300);
>>   
>> -	ri.reg = 0;
>> -	ret = shim->read_ri_prime(intel_dig_port, ri.shim);
>> -	if (ret)
>> -		return ret;
>> -	I915_WRITE(PORT_HDCP_RPRIME(port), ri.reg);
>> +	tries = 3;
>> +	for (i = 0; i < tries; i++) {
>> +		ri.reg = 0;
>> +		ret = shim->read_ri_prime(intel_dig_port, ri.shim);
>> +		if (ret)
>> +			return ret;
>> +		I915_WRITE(PORT_HDCP_RPRIME(port), ri.reg);
>>   
>> -	/* Wait for Ri prime match */
>> -	if (wait_for(I915_READ(PORT_HDCP_STATUS(port)) &
>> -		     (HDCP_STATUS_RI_MATCH | HDCP_STATUS_ENC), 1)) {
>> +		/* Wait for Ri prime match */
>> +		if (!wait_for(I915_READ(PORT_HDCP_STATUS(port)) &
>> +		    (HDCP_STATUS_RI_MATCH | HDCP_STATUS_ENC), 1))
>> +			break;
>> +	}
>> +
>> +	if (i == tries) {
>>   		DRM_ERROR("Timed out waiting for Ri prime match (%x)\n",
>>   			  I915_READ(PORT_HDCP_STATUS(port)));
>>   		return -ETIMEDOUT;
>> -- 
>> 2.7.4
>>

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 2/4] drm/i915: Read Vprime thrice incase of mismatch
  2018-03-29 14:38   ` Sean Paul
@ 2018-04-02  9:16     ` Ramalingam C
  0 siblings, 0 replies; 17+ messages in thread
From: Ramalingam C @ 2018-04-02  9:16 UTC (permalink / raw)
  To: Sean Paul; +Cc: intel-gfx, dri-devel, rodrigo.vivi



On Thursday 29 March 2018 08:08 PM, Sean Paul wrote:
> On Thu, Mar 29, 2018 at 07:39:06PM +0530, Ramalingam C wrote:
>> In case of V prime mismatch, DP HDCP spec mandates the re-read of
>> Vprime atleast twice.
>>
>> This patch needed for DP HDCP1.4 CTS Test: 1B-05.
>>
>> v2:
>>    Moved the V' validation into a function for retry. [Sean Paul]
>>
>> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_hdcp.c | 113 +++++++++++++++++++++++---------------
>>   1 file changed, 70 insertions(+), 43 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
>> index 96b9025dc759..f77d956b2b18 100644
>> --- a/drivers/gpu/drm/i915/intel_hdcp.c
>> +++ b/drivers/gpu/drm/i915/intel_hdcp.c
>> @@ -142,53 +142,17 @@ bool intel_hdcp_is_ksv_valid(u8 *ksv)
>>   	return true;
>>   }
>>   
>> -/* 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)
>> +static inline
> Why inline?
Its a mistake. Will correct it. thanks
>
>> +int intel_hdcp_validate_v_prime(struct intel_digital_port *intel_dig_port,
>> +				const struct intel_hdcp_shim *shim,
>> +				u8 *ksv_fifo, u8 num_downstream, u8 *bstatus)
>>   {
>>   	struct drm_i915_private *dev_priv;
>>   	u32 vprime, sha_text, sha_leftovers, rep_ctl;
>> -	u8 bstatus[2], num_downstream, *ksv_fifo;
>>   	int ret, i, j, sha_idx;
>>   
>>   	dev_priv = intel_dig_port->base.base.dev->dev_private;
>>   
>> -	ret = intel_hdcp_poll_ksv_fifo(intel_dig_port, shim);
>> -	if (ret) {
>> -		DRM_ERROR("KSV list failed to become ready (%d)\n", ret);
>> -		return ret;
>> -	}
>> -
>> -	ret = shim->read_bstatus(intel_dig_port, bstatus);
>> -	if (ret)
>> -		return ret;
>> -
>> -	if (DRM_HDCP_MAX_DEVICE_EXCEEDED(bstatus[0]) ||
>> -	    DRM_HDCP_MAX_CASCADE_EXCEEDED(bstatus[1])) {
>> -		DRM_ERROR("Max Topology Limit Exceeded\n");
>> -		return -EPERM;
>> -	}
>> -
>> -	/*
>> -	 * When repeater reports 0 device count, HDCP1.4 spec allows disabling
>> -	 * the HDCP encryption. That implies that repeater can't have its own
>> -	 * display. As there is no consumption of encrypted content in the
>> -	 * repeater with 0 downstream devices, we are failing the
>> -	 * authentication.
>> -	 */
>> -	num_downstream = DRM_HDCP_NUM_DOWNSTREAM(bstatus[0]);
>> -	if (num_downstream == 0)
>> -		return -EINVAL;
>> -
>> -	ksv_fifo = kzalloc(num_downstream * DRM_HDCP_KSV_LEN, GFP_KERNEL);
>> -	if (!ksv_fifo)
>> -		return -ENOMEM;
>> -
>> -	ret = shim->read_ksv_fifo(intel_dig_port, num_downstream, ksv_fifo);
>> -	if (ret)
>> -		return ret;
>> -
>>   	/* Process V' values from the receiver */
>>   	for (i = 0; i < DRM_HDCP_V_PRIME_NUM_PARTS; i++) {
>>   		ret = shim->read_v_prime_part(intel_dig_port, i, &vprime);
>> @@ -353,7 +317,7 @@ int intel_hdcp_auth_downstream(struct intel_digital_port *intel_dig_port,
>>   			return ret;
>>   		sha_idx += sizeof(sha_text);
>>   	} else {
>> -		DRM_ERROR("Invalid number of leftovers %d\n", sha_leftovers);
>> +		DRM_DEBUG("Invalid number of leftovers %d\n", sha_leftovers);
>>   		return -EINVAL;
>>   	}
>>   
>> @@ -381,14 +345,77 @@ int intel_hdcp_auth_downstream(struct intel_digital_port *intel_dig_port,
>>   	if (intel_wait_for_register(dev_priv, HDCP_REP_CTL,
>>   				    HDCP_SHA1_COMPLETE,
>>   				    HDCP_SHA1_COMPLETE, 1)) {
>> -		DRM_ERROR("Timed out waiting for SHA1 complete\n");
>> +		DRM_DEBUG("Timed out waiting for SHA1 complete\n");
>>   		return -ETIMEDOUT;
>>   	}
>>   	if (!(I915_READ(HDCP_REP_CTL) & HDCP_SHA1_V_MATCH)) {
>> -		DRM_ERROR("SHA-1 mismatch, HDCP failed\n");
>> +		DRM_DEBUG("SHA-1 mismatch, HDCP failed\n");
> I think the DEBUG should be DEBUG_KMS, consistent with the rest of the file?
Will make it uniform

--Ram
>
>>   		return -ENXIO;
>>   	}
>>   
>> +	return 0;
>> +}
>> +
>> +/* 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)
>> +{
>> +	u8 bstatus[2], num_downstream, *ksv_fifo;
>> +	int ret, i, tries = 3;
>> +
>> +	ret = intel_hdcp_poll_ksv_fifo(intel_dig_port, shim);
>> +	if (ret) {
>> +		DRM_ERROR("KSV list failed to become ready (%d)\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	ret = shim->read_bstatus(intel_dig_port, bstatus);
>> +	if (ret)
>> +		return ret;
>> +
>> +	if (DRM_HDCP_MAX_DEVICE_EXCEEDED(bstatus[0]) ||
>> +	    DRM_HDCP_MAX_CASCADE_EXCEEDED(bstatus[1])) {
>> +		DRM_ERROR("Max Topology Limit Exceeded\n");
>> +		return -EPERM;
>> +	}
>> +
>> +	/*
>> +	 * When repeater reports 0 device count, HDCP1.4 spec allows disabling
>> +	 * the HDCP encryption. That implies that repeater can't have its own
>> +	 * display. As there is no consumption of encrypted content in the
>> +	 * repeater with 0 downstream devices, we are failing the
>> +	 * authentication.
>> +	 */
>> +	num_downstream = DRM_HDCP_NUM_DOWNSTREAM(bstatus[0]);
>> +	if (num_downstream == 0)
>> +		return -EINVAL;
>> +
>> +	ksv_fifo = kzalloc(num_downstream * DRM_HDCP_KSV_LEN, GFP_KERNEL);
>> +	if (!ksv_fifo)
>> +		return -ENOMEM;
>> +
>> +	ret = shim->read_ksv_fifo(intel_dig_port, num_downstream, ksv_fifo);
>> +	if (ret)
>> +		return ret;
>> +
>> +	/*
>> +	 * When V prime mismatches, DP Spec mandates re-read of
>> +	 * V prime atleast twice.
>> +	 */
>> +	for (i = 0; i < tries; i++) {
>> +		ret = intel_hdcp_validate_v_prime(intel_dig_port, shim,
>> +						  ksv_fifo, num_downstream,
>> +						  bstatus);
>> +		if (!ret)
>> +			break;
>> +	}
>> +
>> +	if (i == tries) {
>> +		DRM_ERROR("V Prime validation failed.(%d)\n", ret);
>> +		return ret;
>> +	}
>> +
>>   	DRM_DEBUG_KMS("HDCP is enabled (%d downstream devices)\n",
>>   		      num_downstream);
>>   	return 0;
>> -- 
>> 2.7.4
>>

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 3/4] drm/i915: Check hdcp key loadability
  2018-04-02  9:05     ` Ramalingam C
@ 2018-04-06 16:02       ` Ville Syrjälä
  2018-04-09  8:28         ` Daniel Vetter
  0 siblings, 1 reply; 17+ messages in thread
From: Ville Syrjälä @ 2018-04-06 16:02 UTC (permalink / raw)
  To: Ramalingam C; +Cc: intel-gfx, dri-devel, rodrigo.vivi

On Mon, Apr 02, 2018 at 02:35:42PM +0530, Ramalingam C wrote:
> 
> 
> On Thursday 29 March 2018 07:54 PM, Ville Syrjälä wrote:
> > On Thu, Mar 29, 2018 at 07:39:07PM +0530, Ramalingam C wrote:
> >> HDCP1.4 key can be loaded, only when Power well #1 is enabled and cdclk
> >> is enabled. Using the I915 power well infrastruture, above requirement
> >> is verified.
> >>
> >> This patch enables the hdcp initialization for HSW, BDW, and BXT.
> >>
> >> v2:
> >>    Choose the PW# based on the platform.
> >>
> >> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> >> Reviewed-by: Sean Paul <seanpaul@chromium.org>
> >> ---
> >>   drivers/gpu/drm/i915/intel_hdcp.c | 41 +++++++++++++++++++++++++++++++++++++--
> >>   1 file changed, 39 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
> >> index f77d956b2b18..d8af09b46a44 100644
> >> --- a/drivers/gpu/drm/i915/intel_hdcp.c
> >> +++ b/drivers/gpu/drm/i915/intel_hdcp.c
> >> @@ -37,6 +37,43 @@ static int intel_hdcp_poll_ksv_fifo(struct intel_digital_port *intel_dig_port,
> >>   	return 0;
> >>   }
> >>   
> >> +static bool hdcp_key_loadable(struct drm_i915_private *dev_priv)
> >> +{
> >> +	struct i915_power_domains *power_domains = &dev_priv->power_domains;
> >> +	struct i915_power_well *power_well;
> >> +	enum i915_power_well_id id;
> >> +	bool enabled = false;
> >> +
> >> +	/*
> >> +	 * On HSW and BDW, Display HW loads the Key as soon as Display resumes.
> >> +	 * On all BXT+, SW can load the keys only when the PW#1 is turned on.
> >> +	 */
> >> +	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
> >> +		id = HSW_DISP_PW_GLOBAL;
> >> +	else
> >> +		id = SKL_DISP_PW_1;
> >> +
> >> +	mutex_lock(&power_domains->lock);
> >> +
> >> +	/* PG1 (power well #1) needs to be enabled */
> >> +	for_each_power_well(dev_priv, power_well) {
> >> +		if (power_well->id == id) {
> >> +			enabled = power_well->ops->is_enabled(dev_priv,
> >> +							      power_well);
> >> +			break;
> >> +		}
> >> +	}
> >> +	mutex_unlock(&power_domains->lock);
> >> +
> >> +	/*
> >> +	 * Another req for hdcp key loadability is enabled state of pll for
> >> +	 * cdclk. Without active crtc we wont land here. So we are assuming that
> >> +	 * cdclk is already on.
> >> +	 */
> >> +
> >> +	return enabled;
> >> +}
> >> +
> >>   static void intel_hdcp_clear_keys(struct drm_i915_private *dev_priv)
> >>   {
> >>   	I915_WRITE(HDCP_KEY_CONF, HDCP_CLEAR_KEYS_TRIGGER);
> >> @@ -615,8 +652,8 @@ static int _intel_hdcp_enable(struct intel_connector *connector)
> >>   	DRM_DEBUG_KMS("[%s:%d] HDCP is being enabled...\n",
> >>   		      connector->base.name, connector->base.base.id);
> >>   
> >> -	if (!(I915_READ(SKL_FUSE_STATUS) & SKL_FUSE_PG_DIST_STATUS(1))) {
> >> -		DRM_ERROR("PG1 is disabled, cannot load keys\n");
> >> +	if (!hdcp_key_loadable(dev_priv)) {
> >> +		DRM_ERROR("HDCP key Load is not possible\n");
> >>   		return -ENXIO;
> >>   	}
> > If you need the power well then why aren't you grabbing the correct
> > power domain reference somewhere?
> 
> Ville,
> 
> As HDCP is supposed to be enabled after the basic display is up, power well #1 is supposed to be enabled.
> If not that is mostly an error w.r.t HDCP.
> 
> So at this point we just want to verify whether required PW is up and HDCP key can be loaded from the HW. Else fail the HDCP request.

So this is in practice dead code to deal with a hypothetical bug
elsewhere in the driver?

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 3/4] drm/i915: Check hdcp key loadability
  2018-04-06 16:02       ` Ville Syrjälä
@ 2018-04-09  8:28         ` Daniel Vetter
  2018-04-11  8:27           ` Ramalingam C
  0 siblings, 1 reply; 17+ messages in thread
From: Daniel Vetter @ 2018-04-09  8:28 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx, dri-devel, rodrigo.vivi

On Fri, Apr 06, 2018 at 07:02:02PM +0300, Ville Syrjälä wrote:
> On Mon, Apr 02, 2018 at 02:35:42PM +0530, Ramalingam C wrote:
> > 
> > 
> > On Thursday 29 March 2018 07:54 PM, Ville Syrjälä wrote:
> > > On Thu, Mar 29, 2018 at 07:39:07PM +0530, Ramalingam C wrote:
> > >> HDCP1.4 key can be loaded, only when Power well #1 is enabled and cdclk
> > >> is enabled. Using the I915 power well infrastruture, above requirement
> > >> is verified.
> > >>
> > >> This patch enables the hdcp initialization for HSW, BDW, and BXT.
> > >>
> > >> v2:
> > >>    Choose the PW# based on the platform.
> > >>
> > >> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> > >> Reviewed-by: Sean Paul <seanpaul@chromium.org>
> > >> ---
> > >>   drivers/gpu/drm/i915/intel_hdcp.c | 41 +++++++++++++++++++++++++++++++++++++--
> > >>   1 file changed, 39 insertions(+), 2 deletions(-)
> > >>
> > >> diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
> > >> index f77d956b2b18..d8af09b46a44 100644
> > >> --- a/drivers/gpu/drm/i915/intel_hdcp.c
> > >> +++ b/drivers/gpu/drm/i915/intel_hdcp.c
> > >> @@ -37,6 +37,43 @@ static int intel_hdcp_poll_ksv_fifo(struct intel_digital_port *intel_dig_port,
> > >>   	return 0;
> > >>   }
> > >>   
> > >> +static bool hdcp_key_loadable(struct drm_i915_private *dev_priv)
> > >> +{
> > >> +	struct i915_power_domains *power_domains = &dev_priv->power_domains;
> > >> +	struct i915_power_well *power_well;
> > >> +	enum i915_power_well_id id;
> > >> +	bool enabled = false;
> > >> +
> > >> +	/*
> > >> +	 * On HSW and BDW, Display HW loads the Key as soon as Display resumes.
> > >> +	 * On all BXT+, SW can load the keys only when the PW#1 is turned on.
> > >> +	 */
> > >> +	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
> > >> +		id = HSW_DISP_PW_GLOBAL;
> > >> +	else
> > >> +		id = SKL_DISP_PW_1;
> > >> +
> > >> +	mutex_lock(&power_domains->lock);
> > >> +
> > >> +	/* PG1 (power well #1) needs to be enabled */
> > >> +	for_each_power_well(dev_priv, power_well) {
> > >> +		if (power_well->id == id) {
> > >> +			enabled = power_well->ops->is_enabled(dev_priv,
> > >> +							      power_well);
> > >> +			break;
> > >> +		}
> > >> +	}
> > >> +	mutex_unlock(&power_domains->lock);
> > >> +
> > >> +	/*
> > >> +	 * Another req for hdcp key loadability is enabled state of pll for
> > >> +	 * cdclk. Without active crtc we wont land here. So we are assuming that
> > >> +	 * cdclk is already on.
> > >> +	 */
> > >> +
> > >> +	return enabled;
> > >> +}
> > >> +
> > >>   static void intel_hdcp_clear_keys(struct drm_i915_private *dev_priv)
> > >>   {
> > >>   	I915_WRITE(HDCP_KEY_CONF, HDCP_CLEAR_KEYS_TRIGGER);
> > >> @@ -615,8 +652,8 @@ static int _intel_hdcp_enable(struct intel_connector *connector)
> > >>   	DRM_DEBUG_KMS("[%s:%d] HDCP is being enabled...\n",
> > >>   		      connector->base.name, connector->base.base.id);
> > >>   
> > >> -	if (!(I915_READ(SKL_FUSE_STATUS) & SKL_FUSE_PG_DIST_STATUS(1))) {
> > >> -		DRM_ERROR("PG1 is disabled, cannot load keys\n");
> > >> +	if (!hdcp_key_loadable(dev_priv)) {
> > >> +		DRM_ERROR("HDCP key Load is not possible\n");
> > >>   		return -ENXIO;
> > >>   	}
> > > If you need the power well then why aren't you grabbing the correct
> > > power domain reference somewhere?
> > 
> > Ville,
> > 
> > As HDCP is supposed to be enabled after the basic display is up, power well #1 is supposed to be enabled.
> > If not that is mostly an error w.r.t HDCP.
> > 
> > So at this point we just want to verify whether required PW is up and HDCP key can be loaded from the HW. Else fail the HDCP request.
> 
> So this is in practice dead code to deal with a hypothetical bug
> elsewhere in the driver?

Yeah looks like it should be wrapped in a WARN_ON, or maybe outright
thrown out. The unclaimed mmio debug stuff will catch when this happens
(or well, should).
-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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 3/4] drm/i915: Check hdcp key loadability
  2018-04-09  8:28         ` Daniel Vetter
@ 2018-04-11  8:27           ` Ramalingam C
  2018-04-13 15:49             ` Daniel Vetter
  0 siblings, 1 reply; 17+ messages in thread
From: Ramalingam C @ 2018-04-11  8:27 UTC (permalink / raw)
  To: Daniel Vetter, Ville Syrjälä; +Cc: intel-gfx, dri-devel, rodrigo.vivi



On Monday 09 April 2018 01:58 PM, Daniel Vetter wrote:
> On Fri, Apr 06, 2018 at 07:02:02PM +0300, Ville Syrjälä wrote:
>> On Mon, Apr 02, 2018 at 02:35:42PM +0530, Ramalingam C wrote:
>>>
>>> On Thursday 29 March 2018 07:54 PM, Ville Syrjälä wrote:
>>>> On Thu, Mar 29, 2018 at 07:39:07PM +0530, Ramalingam C wrote:
>>>>> HDCP1.4 key can be loaded, only when Power well #1 is enabled and cdclk
>>>>> is enabled. Using the I915 power well infrastruture, above requirement
>>>>> is verified.
>>>>>
>>>>> This patch enables the hdcp initialization for HSW, BDW, and BXT.
>>>>>
>>>>> v2:
>>>>>     Choose the PW# based on the platform.
>>>>>
>>>>> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
>>>>> Reviewed-by: Sean Paul <seanpaul@chromium.org>
>>>>> ---
>>>>>    drivers/gpu/drm/i915/intel_hdcp.c | 41 +++++++++++++++++++++++++++++++++++++--
>>>>>    1 file changed, 39 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
>>>>> index f77d956b2b18..d8af09b46a44 100644
>>>>> --- a/drivers/gpu/drm/i915/intel_hdcp.c
>>>>> +++ b/drivers/gpu/drm/i915/intel_hdcp.c
>>>>> @@ -37,6 +37,43 @@ static int intel_hdcp_poll_ksv_fifo(struct intel_digital_port *intel_dig_port,
>>>>>    	return 0;
>>>>>    }
>>>>>    
>>>>> +static bool hdcp_key_loadable(struct drm_i915_private *dev_priv)
>>>>> +{
>>>>> +	struct i915_power_domains *power_domains = &dev_priv->power_domains;
>>>>> +	struct i915_power_well *power_well;
>>>>> +	enum i915_power_well_id id;
>>>>> +	bool enabled = false;
>>>>> +
>>>>> +	/*
>>>>> +	 * On HSW and BDW, Display HW loads the Key as soon as Display resumes.
>>>>> +	 * On all BXT+, SW can load the keys only when the PW#1 is turned on.
>>>>> +	 */
>>>>> +	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
>>>>> +		id = HSW_DISP_PW_GLOBAL;
>>>>> +	else
>>>>> +		id = SKL_DISP_PW_1;
>>>>> +
>>>>> +	mutex_lock(&power_domains->lock);
>>>>> +
>>>>> +	/* PG1 (power well #1) needs to be enabled */
>>>>> +	for_each_power_well(dev_priv, power_well) {
>>>>> +		if (power_well->id == id) {
>>>>> +			enabled = power_well->ops->is_enabled(dev_priv,
>>>>> +							      power_well);
>>>>> +			break;
>>>>> +		}
>>>>> +	}
>>>>> +	mutex_unlock(&power_domains->lock);
>>>>> +
>>>>> +	/*
>>>>> +	 * Another req for hdcp key loadability is enabled state of pll for
>>>>> +	 * cdclk. Without active crtc we wont land here. So we are assuming that
>>>>> +	 * cdclk is already on.
>>>>> +	 */
>>>>> +
>>>>> +	return enabled;
>>>>> +}
>>>>> +
>>>>>    static void intel_hdcp_clear_keys(struct drm_i915_private *dev_priv)
>>>>>    {
>>>>>    	I915_WRITE(HDCP_KEY_CONF, HDCP_CLEAR_KEYS_TRIGGER);
>>>>> @@ -615,8 +652,8 @@ static int _intel_hdcp_enable(struct intel_connector *connector)
>>>>>    	DRM_DEBUG_KMS("[%s:%d] HDCP is being enabled...\n",
>>>>>    		      connector->base.name, connector->base.base.id);
>>>>>    
>>>>> -	if (!(I915_READ(SKL_FUSE_STATUS) & SKL_FUSE_PG_DIST_STATUS(1))) {
>>>>> -		DRM_ERROR("PG1 is disabled, cannot load keys\n");
>>>>> +	if (!hdcp_key_loadable(dev_priv)) {
>>>>> +		DRM_ERROR("HDCP key Load is not possible\n");
>>>>>    		return -ENXIO;
>>>>>    	}
>>>> If you need the power well then why aren't you grabbing the correct
>>>> power domain reference somewhere?
>>> Ville,
>>>
>>> As HDCP is supposed to be enabled after the basic display is up, power well #1 is supposed to be enabled.
>>> If not that is mostly an error w.r.t HDCP.
>>>
>>> So at this point we just want to verify whether required PW is up and HDCP key can be loaded from the HW. Else fail the HDCP request.
>> So this is in practice dead code to deal with a hypothetical bug
>> elsewhere in the driver?
> Yeah looks like it should be wrapped in a WARN_ON, or maybe outright
> thrown out. The unclaimed mmio debug stuff will catch when this happens
> (or well, should).
> -Daniel
Ok. Intention of this patch was generalizing the existing SKL specific 
PW check for all hdcp capable intel platforms.
Please suggest if this needs to be wrapped into WARN_ON or really want 
to discard it.

--Ram

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 3/4] drm/i915: Check hdcp key loadability
  2018-04-11  8:27           ` Ramalingam C
@ 2018-04-13 15:49             ` Daniel Vetter
  0 siblings, 0 replies; 17+ messages in thread
From: Daniel Vetter @ 2018-04-13 15:49 UTC (permalink / raw)
  To: Ramalingam C; +Cc: intel-gfx, dri-devel, rodrigo.vivi

On Wed, Apr 11, 2018 at 01:57:15PM +0530, Ramalingam C wrote:
> 
> 
> On Monday 09 April 2018 01:58 PM, Daniel Vetter wrote:
> > On Fri, Apr 06, 2018 at 07:02:02PM +0300, Ville Syrjälä wrote:
> > > On Mon, Apr 02, 2018 at 02:35:42PM +0530, Ramalingam C wrote:
> > > > 
> > > > On Thursday 29 March 2018 07:54 PM, Ville Syrjälä wrote:
> > > > > On Thu, Mar 29, 2018 at 07:39:07PM +0530, Ramalingam C wrote:
> > > > > > HDCP1.4 key can be loaded, only when Power well #1 is enabled and cdclk
> > > > > > is enabled. Using the I915 power well infrastruture, above requirement
> > > > > > is verified.
> > > > > > 
> > > > > > This patch enables the hdcp initialization for HSW, BDW, and BXT.
> > > > > > 
> > > > > > v2:
> > > > > >     Choose the PW# based on the platform.
> > > > > > 
> > > > > > Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> > > > > > Reviewed-by: Sean Paul <seanpaul@chromium.org>
> > > > > > ---
> > > > > >    drivers/gpu/drm/i915/intel_hdcp.c | 41 +++++++++++++++++++++++++++++++++++++--
> > > > > >    1 file changed, 39 insertions(+), 2 deletions(-)
> > > > > > 
> > > > > > diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
> > > > > > index f77d956b2b18..d8af09b46a44 100644
> > > > > > --- a/drivers/gpu/drm/i915/intel_hdcp.c
> > > > > > +++ b/drivers/gpu/drm/i915/intel_hdcp.c
> > > > > > @@ -37,6 +37,43 @@ static int intel_hdcp_poll_ksv_fifo(struct intel_digital_port *intel_dig_port,
> > > > > >    	return 0;
> > > > > >    }
> > > > > > +static bool hdcp_key_loadable(struct drm_i915_private *dev_priv)
> > > > > > +{
> > > > > > +	struct i915_power_domains *power_domains = &dev_priv->power_domains;
> > > > > > +	struct i915_power_well *power_well;
> > > > > > +	enum i915_power_well_id id;
> > > > > > +	bool enabled = false;
> > > > > > +
> > > > > > +	/*
> > > > > > +	 * On HSW and BDW, Display HW loads the Key as soon as Display resumes.
> > > > > > +	 * On all BXT+, SW can load the keys only when the PW#1 is turned on.
> > > > > > +	 */
> > > > > > +	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
> > > > > > +		id = HSW_DISP_PW_GLOBAL;
> > > > > > +	else
> > > > > > +		id = SKL_DISP_PW_1;
> > > > > > +
> > > > > > +	mutex_lock(&power_domains->lock);
> > > > > > +
> > > > > > +	/* PG1 (power well #1) needs to be enabled */
> > > > > > +	for_each_power_well(dev_priv, power_well) {
> > > > > > +		if (power_well->id == id) {
> > > > > > +			enabled = power_well->ops->is_enabled(dev_priv,
> > > > > > +							      power_well);
> > > > > > +			break;
> > > > > > +		}
> > > > > > +	}
> > > > > > +	mutex_unlock(&power_domains->lock);
> > > > > > +
> > > > > > +	/*
> > > > > > +	 * Another req for hdcp key loadability is enabled state of pll for
> > > > > > +	 * cdclk. Without active crtc we wont land here. So we are assuming that
> > > > > > +	 * cdclk is already on.
> > > > > > +	 */
> > > > > > +
> > > > > > +	return enabled;
> > > > > > +}
> > > > > > +
> > > > > >    static void intel_hdcp_clear_keys(struct drm_i915_private *dev_priv)
> > > > > >    {
> > > > > >    	I915_WRITE(HDCP_KEY_CONF, HDCP_CLEAR_KEYS_TRIGGER);
> > > > > > @@ -615,8 +652,8 @@ static int _intel_hdcp_enable(struct intel_connector *connector)
> > > > > >    	DRM_DEBUG_KMS("[%s:%d] HDCP is being enabled...\n",
> > > > > >    		      connector->base.name, connector->base.base.id);
> > > > > > -	if (!(I915_READ(SKL_FUSE_STATUS) & SKL_FUSE_PG_DIST_STATUS(1))) {
> > > > > > -		DRM_ERROR("PG1 is disabled, cannot load keys\n");
> > > > > > +	if (!hdcp_key_loadable(dev_priv)) {
> > > > > > +		DRM_ERROR("HDCP key Load is not possible\n");
> > > > > >    		return -ENXIO;
> > > > > >    	}
> > > > > If you need the power well then why aren't you grabbing the correct
> > > > > power domain reference somewhere?
> > > > Ville,
> > > > 
> > > > As HDCP is supposed to be enabled after the basic display is up, power well #1 is supposed to be enabled.
> > > > If not that is mostly an error w.r.t HDCP.
> > > > 
> > > > So at this point we just want to verify whether required PW is up and HDCP key can be loaded from the HW. Else fail the HDCP request.
> > > So this is in practice dead code to deal with a hypothetical bug
> > > elsewhere in the driver?
> > Yeah looks like it should be wrapped in a WARN_ON, or maybe outright
> > thrown out. The unclaimed mmio debug stuff will catch when this happens
> > (or well, should).
> > -Daniel
> Ok. Intention of this patch was generalizing the existing SKL specific PW
> check for all hdcp capable intel platforms.
> Please suggest if this needs to be wrapped into WARN_ON or really want to
> discard it.

That's not what the commit message says, and since you replace a check for
a fuse register with driver-internal checks for power well state, also not
what the diff looks like.

In general, we don't do runtime checks for power wells (except for
boot-time display state readout, but that's the only case). Instead you
need to make sure you're holding the right power well references while
using something. Not just while the key is loaded, but the entire time
it's in use.

So not really sure what you should be doing, except most likely not this
patch here.
-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

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2018-04-13 15:49 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-29 14:09 [PATCH v2 0/4] HDCP1.4 fixes Ramalingam C
2018-03-29 14:09 ` [PATCH v2 1/4] drm/i915: Read HDCP R0 thrice in case of mismatch Ramalingam C
2018-03-29 14:35   ` Sean Paul
2018-04-02  9:08     ` Ramalingam C
2018-03-29 14:09 ` [PATCH v2 2/4] drm/i915: Read Vprime thrice incase " Ramalingam C
2018-03-29 14:38   ` Sean Paul
2018-04-02  9:16     ` Ramalingam C
2018-03-29 14:09 ` [PATCH v2 3/4] drm/i915: Check hdcp key loadability Ramalingam C
2018-03-29 14:24   ` Ville Syrjälä
2018-04-02  9:05     ` Ramalingam C
2018-04-06 16:02       ` Ville Syrjälä
2018-04-09  8:28         ` Daniel Vetter
2018-04-11  8:27           ` Ramalingam C
2018-04-13 15:49             ` Daniel Vetter
2018-03-29 14:09 ` [PATCH v2 4/4] drm/i915: Fix downstream dev count read Ramalingam C
2018-03-29 14:39   ` Sean Paul
2018-03-29 14:12 ` [PATCH v2 0/4] HDCP1.4 fixes Ramalingam C

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).