[PATCH 0/5] HDCP1.4 fixes

[PATCH 0/5] HDCP1.4 fixes

[Ramalingam C]

This series addresses the requriement of below HDCP compliance tests
	DP: 1A-06 and 1B-05
	HDMI: 1A-04 and 1A-07a

One of the patch uses the I915 power infra-structure for checking
the power state of PW#1. Which enables the init path for all legacy
platforms.

And encoder specific msg availability detection is moved into hdcp_shim.
This will help to sync with DP hdcp data availability in the best
possible way by fielding the CP_IRQ.


Ramalingam C (5):
  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: Poll hdcp register on sudden NACK
  drm/i915: Move hdcp msg detection into shim

 drivers/gpu/drm/i915/intel_dp.c   |  70 ++++++++++++++--
 drivers/gpu/drm/i915/intel_drv.h  |   6 +-
 drivers/gpu/drm/i915/intel_hdcp.c | 166 +++++++++++++++++++++++++++-----------
 drivers/gpu/drm/i915/intel_hdmi.c |  28 ++++++-
 4 files changed, 214 insertions(+), 56 deletions(-)

-- 
2.7.4

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

[PATCH 1/5] drm/i915: Read HDCP R0 thrice in case of mismatch

[Ramalingam C]

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.

DP CTS Test: 1A-06.

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

diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
index 14ca5d3057a7..730681d2dbfb 100644
--- a/drivers/gpu/drm/i915/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/intel_hdcp.c
@@ -496,25 +496,35 @@ 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
 	 * the stupid thing instead of polling on one and not the other.
 	 */
-	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++) {
+		wait_remaining_ms_from_jiffies(r0_prime_gen_start,
+					       100 * (i + 1));
 
-	/* Wait for Ri prime match */
-	if (wait_for(I915_READ(PORT_HDCP_STATUS(port)) &
-		     (HDCP_STATUS_RI_MATCH | HDCP_STATUS_ENC), 1)) {
+		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))
+			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

Re: [PATCH 1/5] drm/i915: Read HDCP R0 thrice in case of mismatch

[Sean Paul]

On Mon, Feb 26, 2018 at 10:42:35PM +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.

"R0' must be available for the HDCP Transmitter to read within 100
milliseconds from the time that the HDCP Transmitter finishes writing Aksv to
the video receiver."

100ms is a minimum, waiting for 300ms is quite generous.

The retry is to account for link errors, so there's no need to include the wait
in the retry loop.

Sean

> 
> DP CTS Test: 1A-06.
> 
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_hdcp.c | 34 ++++++++++++++++++++++------------
>  1 file changed, 22 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
> index 14ca5d3057a7..730681d2dbfb 100644
> --- a/drivers/gpu/drm/i915/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/intel_hdcp.c
> @@ -496,25 +496,35 @@ 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
>  	 * the stupid thing instead of polling on one and not the other.
>  	 */
> -	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++) {
> +		wait_remaining_ms_from_jiffies(r0_prime_gen_start,
> +					       100 * (i + 1));
>  
> -	/* Wait for Ri prime match */
> -	if (wait_for(I915_READ(PORT_HDCP_STATUS(port)) &
> -		     (HDCP_STATUS_RI_MATCH | HDCP_STATUS_ENC), 1)) {
> +		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))
> +			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

Re: [PATCH 1/5] drm/i915: Read HDCP R0 thrice in case of mismatch

[Ramalingam C]

On Monday 26 February 2018 11:07 PM, Sean Paul wrote:
> On Mon, Feb 26, 2018 at 10:42:35PM +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.
> "R0' must be available for the HDCP Transmitter to read within 100
> milliseconds from the time that the HDCP Transmitter finishes writing Aksv to
> the video receiver."
>
> 100ms is a minimum, waiting for 300ms is quite generous.

Yes Even with this patch, total wait time for R0 will be 300 mSec only. Splitted into 3 100mSec wait one in each loop.
First loop waits for 100mSec from AKSV write, second loop waits for 200mSec from AKSV write and
third is 300mSec from AKSV write.

This way if a monitor prepares the R0 in 100 mSec, we wont simply wait for 300mSec.
And also we fulfill DP compliance expectation of three attempts to read valid R0.

--Ram

>
> The retry is to account for link errors, so there's no need to include the wait
> in the retry loop.
>
> Sean
>
>> DP CTS Test: 1A-06.
>>
>> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_hdcp.c | 34 ++++++++++++++++++++++------------
>>   1 file changed, 22 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
>> index 14ca5d3057a7..730681d2dbfb 100644
>> --- a/drivers/gpu/drm/i915/intel_hdcp.c
>> +++ b/drivers/gpu/drm/i915/intel_hdcp.c
>> @@ -496,25 +496,35 @@ 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
>>   	 * the stupid thing instead of polling on one and not the other.
>>   	 */
>> -	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++) {
>> +		wait_remaining_ms_from_jiffies(r0_prime_gen_start,
>> +					       100 * (i + 1));
>>   
>> -	/* Wait for Ri prime match */
>> -	if (wait_for(I915_READ(PORT_HDCP_STATUS(port)) &
>> -		     (HDCP_STATUS_RI_MATCH | HDCP_STATUS_ENC), 1)) {
>> +		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))
>> +			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

[PATCH 2/5] drm/i915: read Vprime thrice incase of mismatch

[Ramalingam C]

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

DP HDCP CTS Test: 1B-05

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

diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
index 730681d2dbfb..7ea55fa46f41 100644
--- a/drivers/gpu/drm/i915/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/intel_hdcp.c
@@ -150,7 +150,7 @@ int intel_hdcp_auth_downstream(struct intel_digital_port *intel_dig_port,
 	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;
+	int ret, i, j, sha_idx, tries = 3;
 
 	dev_priv = intel_dig_port->base.base.dev->dev_private;
 
@@ -189,6 +189,7 @@ int intel_hdcp_auth_downstream(struct intel_digital_port *intel_dig_port,
 	if (ret)
 		return ret;
 
+read_v_prime:
 	/* 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);
@@ -385,6 +386,13 @@ int intel_hdcp_auth_downstream(struct intel_digital_port *intel_dig_port,
 		return -ETIMEDOUT;
 	}
 	if (!(I915_READ(HDCP_REP_CTL) & HDCP_SHA1_V_MATCH)) {
+
+		/*
+		 * When V prime mismatches, DP Spec mandates re-read of
+		 * V prime atleast twice.
+		 */
+		if (--tries)
+			goto read_v_prime;
 		DRM_ERROR("SHA-1 mismatch, HDCP failed\n");
 		return -ENXIO;
 	}
-- 
2.7.4

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

Re: [PATCH 2/5] drm/i915: read Vprime thrice incase of mismatch

[Sean Paul]

On Mon, Feb 26, 2018 at 10:42:36PM +0530, Ramalingam C wrote:
> In case of V prime mismatch, DP HDCP spec mandates the re-read of
> Vprime atleast twice.
> 
> DP HDCP CTS Test: 1B-05
> 
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_hdcp.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
> index 730681d2dbfb..7ea55fa46f41 100644
> --- a/drivers/gpu/drm/i915/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/intel_hdcp.c
> @@ -150,7 +150,7 @@ int intel_hdcp_auth_downstream(struct intel_digital_port *intel_dig_port,
>  	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;
> +	int ret, i, j, sha_idx, tries = 3;
>  
>  	dev_priv = intel_dig_port->base.base.dev->dev_private;
>  
> @@ -189,6 +189,7 @@ int intel_hdcp_auth_downstream(struct intel_digital_port *intel_dig_port,
>  	if (ret)
>  		return ret;
>  
> +read_v_prime:

Instead of using the label for retry, please break it out into a function that's
enclosed in a retry loop.

Sean

>  	/* 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);
> @@ -385,6 +386,13 @@ int intel_hdcp_auth_downstream(struct intel_digital_port *intel_dig_port,
>  		return -ETIMEDOUT;
>  	}
>  	if (!(I915_READ(HDCP_REP_CTL) & HDCP_SHA1_V_MATCH)) {
> +
> +		/*
> +		 * When V prime mismatches, DP Spec mandates re-read of
> +		 * V prime atleast twice.
> +		 */
> +		if (--tries)
> +			goto read_v_prime;
>  		DRM_ERROR("SHA-1 mismatch, HDCP failed\n");
>  		return -ENXIO;
>  	}
> -- 
> 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

[PATCH v2] drm/i915: read Vprime thrice incase of mismatch

[Ramalingam C]

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

Fixes DP HDCP 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 730681d2dbfb..3a02463d2b7a 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

[PATCH 3/5] drm/i915: Check hdcp key loadability

[Ramalingam C]

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.

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

diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
index 7ea55fa46f41..95081aaa832a 100644
--- a/drivers/gpu/drm/i915/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/intel_hdcp.c
@@ -37,6 +37,33 @@ 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;
+	bool enabled = false;
+
+	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 == SKL_DISP_PW_1) {
+			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);
@@ -598,8 +625,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

Re: [PATCH 3/5] drm/i915: Check hdcp key loadability

[Sean Paul]

On Mon, Feb 26, 2018 at 10:42:37PM +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.
> 
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>

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

> ---
>  drivers/gpu/drm/i915/intel_hdcp.c | 31 +++++++++++++++++++++++++++++--
>  1 file changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
> index 7ea55fa46f41..95081aaa832a 100644
> --- a/drivers/gpu/drm/i915/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/intel_hdcp.c
> @@ -37,6 +37,33 @@ 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;
> +	bool enabled = false;
> +
> +	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 == SKL_DISP_PW_1) {
> +			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);
> @@ -598,8 +625,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
> 

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

[PATCH 4/5] drm/i915: Poll hdcp register on sudden NACK

[Ramalingam C]

In a connected state, If a HDMI HDCP sink is responded with NACK for
HDCP I2C register access, then HDMI HDCP spec mandates the polling
of any HDCP space registers for accessibility, minimum once in 2Secs
atleast for 4Secs.

Just to make it simple, this is generically implemented for both HDMI
and DP. But we dont expect that this scanario will occur for DP.

HDMI HDCP CTS Tests: 1A-04 and 1A-07A.

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

diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
index 95081aaa832a..14be14a45e5a 100644
--- a/drivers/gpu/drm/i915/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/intel_hdcp.c
@@ -16,6 +16,47 @@
 
 #define KEY_LOAD_TRIES	5
 
+static
+struct intel_digital_port *conn_to_dig_port(struct intel_connector *connector)
+{
+	return enc_to_dig_port(&intel_attached_encoder(&connector->base)->base);
+}
+
+static inline bool hdcp_port_accessible(struct intel_connector *connector)
+{
+	struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector);
+	int ret = -ENXIO;
+	u8 bksv[DRM_HDCP_KSV_LEN];
+
+	ret = connector->hdcp_shim->read_bksv(intel_dig_port, bksv);
+	if (!ret)
+		return true;
+	return false;
+}
+
+static bool wait_for_hdcp_port(struct intel_connector *connector)
+{
+	int i, tries = 10;
+
+	for (i = 0; i < tries; i++) {
+		if (connector->base.status != connector_status_connected ||
+		    connector->base.state->content_protection ==
+		    DRM_MODE_CONTENT_PROTECTION_UNDESIRED ||
+		    connector->hdcp_value ==
+		    DRM_MODE_CONTENT_PROTECTION_UNDESIRED)
+			return false;
+
+		if (hdcp_port_accessible(connector))
+			break;
+
+		msleep_interruptible(500);
+	}
+
+	if (i == tries)
+		return false;
+	return true;
+}
+
 static int intel_hdcp_poll_ksv_fifo(struct intel_digital_port *intel_dig_port,
 				    const struct intel_hdcp_shim *shim)
 {
@@ -584,12 +625,6 @@ static int intel_hdcp_auth(struct intel_digital_port *intel_dig_port,
 	return 0;
 }
 
-static
-struct intel_digital_port *conn_to_dig_port(struct intel_connector *connector)
-{
-	return enc_to_dig_port(&intel_attached_encoder(&connector->base)->base);
-}
-
 static int _intel_hdcp_disable(struct intel_connector *connector)
 {
 	struct drm_i915_private *dev_priv = connector->base.dev->dev_private;
@@ -719,14 +754,26 @@ int intel_hdcp_init(struct intel_connector *connector,
 
 int intel_hdcp_enable(struct intel_connector *connector)
 {
-	int ret;
+	int ret, tries = 2;
 
 	if (!connector->hdcp_shim)
 		return -ENOENT;
 
 	mutex_lock(&connector->hdcp_mutex);
 
+enable_hdcp:
 	ret = _intel_hdcp_enable(connector);
+
+	/*
+	 * Suddenly if sink is NACK-ed for the access of HDCP
+	 * registers, but display is still connected, poll for hdcp
+	 * port accessibility. One of the HDCP spec requirement.
+	 */
+	if ((ret == -EIO || ret == -ENXIO) &&
+	    connector->base.status == connector_status_connected &&
+	    !hdcp_port_accessible(connector))
+		if (wait_for_hdcp_port(connector) && --tries)
+			goto enable_hdcp;
 	if (ret)
 		goto out;
 
@@ -838,6 +885,19 @@ int intel_hdcp_check_link(struct intel_connector *connector)
 		goto out;
 	}
 
+	/*
+	 * Suddenly if sink is NACK-ed for the access of HDCP
+	 * registers, but display is still connected, poll for hdcp
+	 * port accessibility. One of the HDCP spec requirement.
+	 */
+	if (connector->base.status == connector_status_connected &&
+	    !hdcp_port_accessible(connector)) {
+		mutex_unlock(&connector->hdcp_mutex);
+		if (!wait_for_hdcp_port(connector))
+			return ret;
+		mutex_lock(&connector->hdcp_mutex);
+	}
+
 	ret = _intel_hdcp_enable(connector);
 	if (ret) {
 		DRM_ERROR("Failed to enable hdcp (%d)\n", ret);
-- 
2.7.4

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

Re: [PATCH 4/5] drm/i915: Poll hdcp register on sudden NACK

[Sean Paul]

On Mon, Feb 26, 2018 at 10:42:38PM +0530, Ramalingam C wrote:
> In a connected state, If a HDMI HDCP sink is responded with NACK for
> HDCP I2C register access, then HDMI HDCP spec mandates the polling
> of any HDCP space registers for accessibility, minimum once in 2Secs
> atleast for 4Secs.
> 

I'm not convinced this is the right place to do this. 

The reason is that this is more complex than how you have it below. You can't
access state outside of check/commit, so polling state->content_protection is
not permissable from check_link. If the check fails, the driver will change the
current value of content_protection, so userspace will be able to retry.

In the case of enable, since it's synchronous, the error will be propagated to
userspace and it can retry if that's the right thing to do.

Sean

> Just to make it simple, this is generically implemented for both HDMI
> and DP. But we dont expect that this scanario will occur for DP.
> 
> HDMI HDCP CTS Tests: 1A-04 and 1A-07A.
> 
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_hdcp.c | 74 +++++++++++++++++++++++++++++++++++----
>  1 file changed, 67 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
> index 95081aaa832a..14be14a45e5a 100644
> --- a/drivers/gpu/drm/i915/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/intel_hdcp.c
> @@ -16,6 +16,47 @@
>  
>  #define KEY_LOAD_TRIES	5
>  
> +static
> +struct intel_digital_port *conn_to_dig_port(struct intel_connector *connector)
> +{
> +	return enc_to_dig_port(&intel_attached_encoder(&connector->base)->base);
> +}
> +
> +static inline bool hdcp_port_accessible(struct intel_connector *connector)
> +{
> +	struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector);
> +	int ret = -ENXIO;
> +	u8 bksv[DRM_HDCP_KSV_LEN];
> +
> +	ret = connector->hdcp_shim->read_bksv(intel_dig_port, bksv);
> +	if (!ret)
> +		return true;
> +	return false;
> +}
> +
> +static bool wait_for_hdcp_port(struct intel_connector *connector)
> +{
> +	int i, tries = 10;
> +
> +	for (i = 0; i < tries; i++) {
> +		if (connector->base.status != connector_status_connected ||
> +		    connector->base.state->content_protection ==
> +		    DRM_MODE_CONTENT_PROTECTION_UNDESIRED ||
> +		    connector->hdcp_value ==
> +		    DRM_MODE_CONTENT_PROTECTION_UNDESIRED)
> +			return false;
> +
> +		if (hdcp_port_accessible(connector))
> +			break;
> +
> +		msleep_interruptible(500);
> +	}
> +
> +	if (i == tries)
> +		return false;
> +	return true;
> +}
> +
>  static int intel_hdcp_poll_ksv_fifo(struct intel_digital_port *intel_dig_port,
>  				    const struct intel_hdcp_shim *shim)
>  {
> @@ -584,12 +625,6 @@ static int intel_hdcp_auth(struct intel_digital_port *intel_dig_port,
>  	return 0;
>  }
>  
> -static
> -struct intel_digital_port *conn_to_dig_port(struct intel_connector *connector)
> -{
> -	return enc_to_dig_port(&intel_attached_encoder(&connector->base)->base);
> -}
> -
>  static int _intel_hdcp_disable(struct intel_connector *connector)
>  {
>  	struct drm_i915_private *dev_priv = connector->base.dev->dev_private;
> @@ -719,14 +754,26 @@ int intel_hdcp_init(struct intel_connector *connector,
>  
>  int intel_hdcp_enable(struct intel_connector *connector)
>  {
> -	int ret;
> +	int ret, tries = 2;
>  
>  	if (!connector->hdcp_shim)
>  		return -ENOENT;
>  
>  	mutex_lock(&connector->hdcp_mutex);
>  
> +enable_hdcp:
>  	ret = _intel_hdcp_enable(connector);
> +
> +	/*
> +	 * Suddenly if sink is NACK-ed for the access of HDCP
> +	 * registers, but display is still connected, poll for hdcp
> +	 * port accessibility. One of the HDCP spec requirement.
> +	 */
> +	if ((ret == -EIO || ret == -ENXIO) &&
> +	    connector->base.status == connector_status_connected &&
> +	    !hdcp_port_accessible(connector))
> +		if (wait_for_hdcp_port(connector) && --tries)
> +			goto enable_hdcp;
>  	if (ret)
>  		goto out;
>  
> @@ -838,6 +885,19 @@ int intel_hdcp_check_link(struct intel_connector *connector)
>  		goto out;
>  	}
>  
> +	/*
> +	 * Suddenly if sink is NACK-ed for the access of HDCP
> +	 * registers, but display is still connected, poll for hdcp
> +	 * port accessibility. One of the HDCP spec requirement.
> +	 */
> +	if (connector->base.status == connector_status_connected &&
> +	    !hdcp_port_accessible(connector)) {
> +		mutex_unlock(&connector->hdcp_mutex);
> +		if (!wait_for_hdcp_port(connector))
> +			return ret;
> +		mutex_lock(&connector->hdcp_mutex);
> +	}
> +
>  	ret = _intel_hdcp_enable(connector);
>  	if (ret) {
>  		DRM_ERROR("Failed to enable hdcp (%d)\n", ret);
> -- 
> 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

Re: [PATCH 4/5] drm/i915: Poll hdcp register on sudden NACK

[Ramalingam C]

On Monday 26 February 2018 11:22 PM, Sean Paul wrote:
> On Mon, Feb 26, 2018 at 10:42:38PM +0530, Ramalingam C wrote:
>> In a connected state, If a HDMI HDCP sink is responded with NACK for
>> HDCP I2C register access, then HDMI HDCP spec mandates the polling
>> of any HDCP space registers for accessibility, minimum once in 2Secs
>> atleast for 4Secs.
>>
> I'm not convinced this is the right place to do this.
>
> The reason is that this is more complex than how you have it below. You can't
> access state outside of check/commit, so polling state->content_protection is
> not permissable from check_link. If the check fails, the driver will change the
> current value of content_protection, so userspace will be able to retry.
>
> In the case of enable, since it's synchronous, the error will be propagated to
> userspace and it can retry if that's the right thing to do.

Sean,

I am not grasping the concern clearly. Please help me on that.

Do you want to say why do you want to poll the hdcp space register's availability?
Because specification is asking for it. Functionally from our usecase perspective no
difference with and without this NACK handling.

Retry from userspace will not help to address the failures in the CTS tests mentioned below.

This could be done without referring connector state.

--Ram

>
> Sean
>
>> Just to make it simple, this is generically implemented for both HDMI
>> and DP. But we dont expect that this scanario will occur for DP.
>>
>> HDMI HDCP CTS Tests: 1A-04 and 1A-07A.
>>
>> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_hdcp.c | 74 +++++++++++++++++++++++++++++++++++----
>>   1 file changed, 67 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
>> index 95081aaa832a..14be14a45e5a 100644
>> --- a/drivers/gpu/drm/i915/intel_hdcp.c
>> +++ b/drivers/gpu/drm/i915/intel_hdcp.c
>> @@ -16,6 +16,47 @@
>>   
>>   #define KEY_LOAD_TRIES	5
>>   
>> +static
>> +struct intel_digital_port *conn_to_dig_port(struct intel_connector *connector)
>> +{
>> +	return enc_to_dig_port(&intel_attached_encoder(&connector->base)->base);
>> +}
>> +
>> +static inline bool hdcp_port_accessible(struct intel_connector *connector)
>> +{
>> +	struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector);
>> +	int ret = -ENXIO;
>> +	u8 bksv[DRM_HDCP_KSV_LEN];
>> +
>> +	ret = connector->hdcp_shim->read_bksv(intel_dig_port, bksv);
>> +	if (!ret)
>> +		return true;
>> +	return false;
>> +}
>> +
>> +static bool wait_for_hdcp_port(struct intel_connector *connector)
>> +{
>> +	int i, tries = 10;
>> +
>> +	for (i = 0; i < tries; i++) {
>> +		if (connector->base.status != connector_status_connected ||
>> +		    connector->base.state->content_protection ==
>> +		    DRM_MODE_CONTENT_PROTECTION_UNDESIRED ||
>> +		    connector->hdcp_value ==
>> +		    DRM_MODE_CONTENT_PROTECTION_UNDESIRED)
>> +			return false;
>> +
>> +		if (hdcp_port_accessible(connector))
>> +			break;
>> +
>> +		msleep_interruptible(500);
>> +	}
>> +
>> +	if (i == tries)
>> +		return false;
>> +	return true;
>> +}
>> +
>>   static int intel_hdcp_poll_ksv_fifo(struct intel_digital_port *intel_dig_port,
>>   				    const struct intel_hdcp_shim *shim)
>>   {
>> @@ -584,12 +625,6 @@ static int intel_hdcp_auth(struct intel_digital_port *intel_dig_port,
>>   	return 0;
>>   }
>>   
>> -static
>> -struct intel_digital_port *conn_to_dig_port(struct intel_connector *connector)
>> -{
>> -	return enc_to_dig_port(&intel_attached_encoder(&connector->base)->base);
>> -}
>> -
>>   static int _intel_hdcp_disable(struct intel_connector *connector)
>>   {
>>   	struct drm_i915_private *dev_priv = connector->base.dev->dev_private;
>> @@ -719,14 +754,26 @@ int intel_hdcp_init(struct intel_connector *connector,
>>   
>>   int intel_hdcp_enable(struct intel_connector *connector)
>>   {
>> -	int ret;
>> +	int ret, tries = 2;
>>   
>>   	if (!connector->hdcp_shim)
>>   		return -ENOENT;
>>   
>>   	mutex_lock(&connector->hdcp_mutex);
>>   
>> +enable_hdcp:
>>   	ret = _intel_hdcp_enable(connector);
>> +
>> +	/*
>> +	 * Suddenly if sink is NACK-ed for the access of HDCP
>> +	 * registers, but display is still connected, poll for hdcp
>> +	 * port accessibility. One of the HDCP spec requirement.
>> +	 */
>> +	if ((ret == -EIO || ret == -ENXIO) &&
>> +	    connector->base.status == connector_status_connected &&
>> +	    !hdcp_port_accessible(connector))
>> +		if (wait_for_hdcp_port(connector) && --tries)
>> +			goto enable_hdcp;
>>   	if (ret)
>>   		goto out;
>>   
>> @@ -838,6 +885,19 @@ int intel_hdcp_check_link(struct intel_connector *connector)
>>   		goto out;
>>   	}
>>   
>> +	/*
>> +	 * Suddenly if sink is NACK-ed for the access of HDCP
>> +	 * registers, but display is still connected, poll for hdcp
>> +	 * port accessibility. One of the HDCP spec requirement.
>> +	 */
>> +	if (connector->base.status == connector_status_connected &&
>> +	    !hdcp_port_accessible(connector)) {
>> +		mutex_unlock(&connector->hdcp_mutex);
>> +		if (!wait_for_hdcp_port(connector))
>> +			return ret;
>> +		mutex_lock(&connector->hdcp_mutex);
>> +	}
>> +
>>   	ret = _intel_hdcp_enable(connector);
>>   	if (ret) {
>>   		DRM_ERROR("Failed to enable hdcp (%d)\n", ret);
>> -- 
>> 2.7.4
>>

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

[PATCH 5/5] drm/i915: Move hdcp msg detection into shim

[Ramalingam C]

DP and HDMI HDCP specifications are varying with respect to
detecting the R0 and ksv_fifo availability.

DP will produce CP_IRQ and set a bit for indicating the R0 and
FIFO_READY status.

Whereas HDMI will set a bit for FIFO_READY but there is no bit
indication for R0 ready. And also polling of READY bit is needed for
HDMI as ther is no CP_IRQ.

So Fielding the CP_IRQ for DP and the polling the READY bit for a
period and blindly waiting for a fixed time for R0 incase of HDMI are
moved into corresponding hdcp_shim.

Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c   | 70 +++++++++++++++++++++++++++++++++++----
 drivers/gpu/drm/i915/intel_drv.h  |  6 ++--
 drivers/gpu/drm/i915/intel_hdcp.c | 39 ++--------------------
 drivers/gpu/drm/i915/intel_hdmi.c | 28 +++++++++++++++-
 4 files changed, 98 insertions(+), 45 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 2c3eb90b9499..afe310a91d3c 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4436,6 +4436,7 @@ static bool
 intel_dp_short_pulse(struct intel_dp *intel_dp)
 {
 	struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
+	struct intel_connector *connector = intel_dp->attached_connector;
 	u8 sink_irq_vector = 0;
 	u8 old_sink_count = intel_dp->sink_count;
 	bool ret;
@@ -4470,8 +4471,11 @@ intel_dp_short_pulse(struct intel_dp *intel_dp)
 
 		if (sink_irq_vector & DP_AUTOMATED_TEST_REQUEST)
 			intel_dp_handle_test_request(intel_dp);
-		if (sink_irq_vector & (DP_CP_IRQ | DP_SINK_SPECIFIC_IRQ))
-			DRM_DEBUG_DRIVER("CP or sink specific irq unhandled\n");
+		if (sink_irq_vector & DP_CP_IRQ)
+			if (connector->hdcp_shim)
+				complete_all(&connector->cp_irq_recved);
+		if (sink_irq_vector & DP_SINK_SPECIFIC_IRQ)
+			DRM_DEBUG_DRIVER("Sink specific irq unhandled\n");
 	}
 
 	intel_dp_check_link_status(intel_dp);
@@ -5083,6 +5087,25 @@ void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder)
 	pps_unlock(intel_dp);
 }
 
+static int intel_dp_hdcp_wait_for_cp_irq(struct completion *cp_irq_recved,
+					 int timeout)
+{
+	long ret;
+
+	if (completion_done(cp_irq_recved))
+		reinit_completion(cp_irq_recved);
+
+	ret = wait_for_completion_interruptible_timeout(cp_irq_recved,
+							msecs_to_jiffies(
+							timeout));
+	reinit_completion(cp_irq_recved);
+	if (ret < 0)
+		return (int)ret;
+	else if (!ret)
+		return -ETIMEDOUT;
+	return 0;
+}
+
 static
 int intel_dp_hdcp_write_an_aksv(struct intel_digital_port *intel_dig_port,
 				u8 *an)
@@ -5188,11 +5211,38 @@ int intel_dp_hdcp_repeater_present(struct intel_digital_port *intel_dig_port,
 	return 0;
 }
 
+static int intel_dp_hdcp_wait_for_r0(struct intel_digital_port *intel_dig_port)
+{
+	struct intel_dp *dp = &intel_dig_port->dp;
+	struct intel_connector *connector = dp->attached_connector;
+	int ret;
+	u8 bstatus;
+
+	intel_dp_hdcp_wait_for_cp_irq(&connector->cp_irq_recved, 100);
+
+	ret = drm_dp_dpcd_read(&intel_dig_port->dp.aux, DP_AUX_HDCP_BSTATUS,
+			       &bstatus, 1);
+	if (ret != 1) {
+		DRM_ERROR("Read bstatus from DP/AUX failed (%d)\n", ret);
+		return ret >= 0 ? -EIO : ret;
+	}
+
+	if (!(bstatus & DP_BSTATUS_R0_PRIME_READY))
+		return -ETIMEDOUT;
+
+	return 0;
+}
+
 static
 int intel_dp_hdcp_read_ri_prime(struct intel_digital_port *intel_dig_port,
 				u8 *ri_prime)
 {
 	ssize_t ret;
+
+	ret = intel_dp_hdcp_wait_for_r0(intel_dig_port);
+	if (!ret)
+		return ret;
+
 	ret = drm_dp_dpcd_read(&intel_dig_port->dp.aux, DP_AUX_HDCP_RI_PRIME,
 			       ri_prime, DRM_HDCP_RI_LEN);
 	if (ret != DRM_HDCP_RI_LEN) {
@@ -5203,18 +5253,26 @@ int intel_dp_hdcp_read_ri_prime(struct intel_digital_port *intel_dig_port,
 }
 
 static
-int intel_dp_hdcp_read_ksv_ready(struct intel_digital_port *intel_dig_port,
-				 bool *ksv_ready)
+int intel_dp_hdcp_ksv_ready(struct intel_digital_port *intel_dig_port)
 {
+	struct intel_dp *dp = &intel_dig_port->dp;
+	struct intel_connector *connector = dp->attached_connector;
 	ssize_t ret;
 	u8 bstatus;
+
+	intel_dp_hdcp_wait_for_cp_irq(&connector->cp_irq_recved,
+				      5 * 1000 * 1000);
+
 	ret = drm_dp_dpcd_read(&intel_dig_port->dp.aux, DP_AUX_HDCP_BSTATUS,
 			       &bstatus, 1);
 	if (ret != 1) {
 		DRM_ERROR("Read bstatus from DP/AUX failed (%zd)\n", ret);
 		return ret >= 0 ? -EIO : ret;
 	}
-	*ksv_ready = bstatus & DP_BSTATUS_READY;
+
+	if (!(bstatus & DP_BSTATUS_READY))
+		return -ETIMEDOUT;
+
 	return 0;
 }
 
@@ -5305,7 +5363,7 @@ static const struct intel_hdcp_shim intel_dp_hdcp_shim = {
 	.read_bstatus = intel_dp_hdcp_read_bstatus,
 	.repeater_present = intel_dp_hdcp_repeater_present,
 	.read_ri_prime = intel_dp_hdcp_read_ri_prime,
-	.read_ksv_ready = intel_dp_hdcp_read_ksv_ready,
+	.wait_for_ksv_ready = intel_dp_hdcp_ksv_ready,
 	.read_ksv_fifo = intel_dp_hdcp_read_ksv_fifo,
 	.read_v_prime_part = intel_dp_hdcp_read_v_prime_part,
 	.toggle_signalling = intel_dp_hdcp_toggle_signalling,
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 8f38e584d375..ab975107c978 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -353,8 +353,7 @@ struct intel_hdcp_shim {
 	int (*read_ri_prime)(struct intel_digital_port *intel_dig_port, u8 *ri);
 
 	/* Determines if the receiver's KSV FIFO is ready for consumption */
-	int (*read_ksv_ready)(struct intel_digital_port *intel_dig_port,
-			      bool *ksv_ready);
+	int (*wait_for_ksv_ready)(struct intel_digital_port *intel_dig_port);
 
 	/* Reads the ksv fifo for num_downstream devices */
 	int (*read_ksv_fifo)(struct intel_digital_port *intel_dig_port,
@@ -413,6 +412,9 @@ struct intel_connector {
 	uint64_t hdcp_value; /* protected by hdcp_mutex */
 	struct delayed_work hdcp_check_work;
 	struct work_struct hdcp_prop_work;
+
+	/* To indicate the assertion of CP_IRQ */
+	struct completion cp_irq_recved;
 };
 
 struct intel_digital_connector_state {
diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
index 14be14a45e5a..a077e1333886 100644
--- a/drivers/gpu/drm/i915/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/intel_hdcp.c
@@ -57,27 +57,6 @@ static bool wait_for_hdcp_port(struct intel_connector *connector)
 	return true;
 }
 
-static int intel_hdcp_poll_ksv_fifo(struct intel_digital_port *intel_dig_port,
-				    const struct intel_hdcp_shim *shim)
-{
-	int ret, read_ret;
-	bool ksv_ready;
-
-	/* Poll for ksv list ready (spec says max time allowed is 5s) */
-	ret = __wait_for(read_ret = shim->read_ksv_ready(intel_dig_port,
-							 &ksv_ready),
-			 read_ret || ksv_ready, 5 * 1000 * 1000, 1000,
-			 100 * 1000);
-	if (ret)
-		return ret;
-	if (read_ret)
-		return read_ret;
-	if (!ksv_ready)
-		return -ETIMEDOUT;
-
-	return 0;
-}
-
 static bool hdcp_key_loadable(struct drm_i915_private *dev_priv)
 {
 	struct i915_power_domains *power_domains = &dev_priv->power_domains;
@@ -222,7 +201,7 @@ int intel_hdcp_auth_downstream(struct intel_digital_port *intel_dig_port,
 
 	dev_priv = intel_dig_port->base.base.dev->dev_private;
 
-	ret = intel_hdcp_poll_ksv_fifo(intel_dig_port, shim);
+	ret = shim->wait_for_ksv_ready(intel_dig_port);
 	if (ret) {
 		DRM_ERROR("KSV list failed to become ready (%d)\n", ret);
 		return ret;
@@ -476,7 +455,6 @@ static int intel_hdcp_auth(struct intel_digital_port *intel_dig_port,
 {
 	struct drm_i915_private *dev_priv;
 	enum port port;
-	unsigned long r0_prime_gen_start;
 	int ret, i, tries = 2;
 	union {
 		u32 reg[2];
@@ -531,8 +509,6 @@ static int intel_hdcp_auth(struct intel_digital_port *intel_dig_port,
 	if (ret)
 		return ret;
 
-	r0_prime_gen_start = jiffies;
-
 	memset(&bksv, 0, sizeof(bksv));
 
 	/* HDCP spec states that we must retry the bksv if it is invalid */
@@ -572,22 +548,11 @@ static int intel_hdcp_auth(struct intel_digital_port *intel_dig_port,
 	}
 
 	/*
-	 * 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
-	 * the stupid thing instead of polling on one and not the other.
 	 */
-
 	tries = 3;
 	for (i = 0; i < tries; i++) {
-		wait_remaining_ms_from_jiffies(r0_prime_gen_start,
-					       100 * (i + 1));
-
 		ri.reg = 0;
 		ret = shim->read_ri_prime(intel_dig_port, ri.shim);
 		if (ret)
@@ -749,6 +714,8 @@ int intel_hdcp_init(struct intel_connector *connector,
 	mutex_init(&connector->hdcp_mutex);
 	INIT_DELAYED_WORK(&connector->hdcp_check_work, intel_hdcp_check_work);
 	INIT_WORK(&connector->hdcp_prop_work, intel_hdcp_prop_work);
+
+	init_completion(&connector->cp_irq_recved);
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index f5d7bfb43006..532d13f91602 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1007,6 +1007,9 @@ int intel_hdmi_hdcp_read_ri_prime(struct intel_digital_port *intel_dig_port,
 				  u8 *ri_prime)
 {
 	int ret;
+
+	wait_remaining_ms_from_jiffies(jiffies, 100);
+
 	ret = intel_hdmi_hdcp_read(intel_dig_port, DRM_HDCP_DDC_RI_PRIME,
 				   ri_prime, DRM_HDCP_RI_LEN);
 	if (ret)
@@ -1027,6 +1030,29 @@ int intel_hdmi_hdcp_read_ksv_ready(struct intel_digital_port *intel_dig_port,
 		return ret;
 	}
 	*ksv_ready = val & DRM_HDCP_DDC_BCAPS_KSV_FIFO_READY;
+
+	return 0;
+}
+
+static
+int intel_hdmi_hdcp_ksv_ready(struct intel_digital_port *intel_dig_port)
+{
+	int ret, read_ret;
+	bool ksv_ready;
+
+	/* Poll for ksv list ready (spec says max time allowed is 5s) */
+	ret = __wait_for(read_ret =
+			 intel_hdmi_hdcp_read_ksv_ready(intel_dig_port,
+							&ksv_ready),
+			 read_ret || ksv_ready, 5 * 1000 * 1000, 1000,
+			 100 * 1000);
+	if (ret)
+		return ret;
+	if (read_ret)
+		return read_ret;
+	if (!ksv_ready)
+		return -ETIMEDOUT;
+
 	return 0;
 }
 
@@ -1112,7 +1138,7 @@ static const struct intel_hdcp_shim intel_hdmi_hdcp_shim = {
 	.read_bstatus = intel_hdmi_hdcp_read_bstatus,
 	.repeater_present = intel_hdmi_hdcp_repeater_present,
 	.read_ri_prime = intel_hdmi_hdcp_read_ri_prime,
-	.read_ksv_ready = intel_hdmi_hdcp_read_ksv_ready,
+	.wait_for_ksv_ready = intel_hdmi_hdcp_ksv_ready,
 	.read_ksv_fifo = intel_hdmi_hdcp_read_ksv_fifo,
 	.read_v_prime_part = intel_hdmi_hdcp_read_v_prime_part,
 	.toggle_signalling = intel_hdmi_hdcp_toggle_signalling,
-- 
2.7.4

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

Re: [PATCH 5/5] drm/i915: Move hdcp msg detection into shim

[Sean Paul]

On Mon, Feb 26, 2018 at 10:42:39PM +0530, Ramalingam C wrote:
> DP and HDMI HDCP specifications are varying with respect to
> detecting the R0 and ksv_fifo availability.
> 
> DP will produce CP_IRQ and set a bit for indicating the R0 and
> FIFO_READY status.

I'm not sure what the benefit is? Keeping things common was a deliberate
decision on my part to keep complexity down and increase the amount of common
code.

Sean

> 
> Whereas HDMI will set a bit for FIFO_READY but there is no bit
> indication for R0 ready. And also polling of READY bit is needed for
> HDMI as ther is no CP_IRQ.
> 
> So Fielding the CP_IRQ for DP and the polling the READY bit for a
> period and blindly waiting for a fixed time for R0 incase of HDMI are
> moved into corresponding hdcp_shim.
> 
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c   | 70 +++++++++++++++++++++++++++++++++++----
>  drivers/gpu/drm/i915/intel_drv.h  |  6 ++--
>  drivers/gpu/drm/i915/intel_hdcp.c | 39 ++--------------------
>  drivers/gpu/drm/i915/intel_hdmi.c | 28 +++++++++++++++-
>  4 files changed, 98 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 2c3eb90b9499..afe310a91d3c 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4436,6 +4436,7 @@ static bool
>  intel_dp_short_pulse(struct intel_dp *intel_dp)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
> +	struct intel_connector *connector = intel_dp->attached_connector;
>  	u8 sink_irq_vector = 0;
>  	u8 old_sink_count = intel_dp->sink_count;
>  	bool ret;
> @@ -4470,8 +4471,11 @@ intel_dp_short_pulse(struct intel_dp *intel_dp)
>  
>  		if (sink_irq_vector & DP_AUTOMATED_TEST_REQUEST)
>  			intel_dp_handle_test_request(intel_dp);
> -		if (sink_irq_vector & (DP_CP_IRQ | DP_SINK_SPECIFIC_IRQ))
> -			DRM_DEBUG_DRIVER("CP or sink specific irq unhandled\n");
> +		if (sink_irq_vector & DP_CP_IRQ)
> +			if (connector->hdcp_shim)
> +				complete_all(&connector->cp_irq_recved);
> +		if (sink_irq_vector & DP_SINK_SPECIFIC_IRQ)
> +			DRM_DEBUG_DRIVER("Sink specific irq unhandled\n");
>  	}
>  
>  	intel_dp_check_link_status(intel_dp);
> @@ -5083,6 +5087,25 @@ void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder)
>  	pps_unlock(intel_dp);
>  }
>  
> +static int intel_dp_hdcp_wait_for_cp_irq(struct completion *cp_irq_recved,
> +					 int timeout)
> +{
> +	long ret;
> +
> +	if (completion_done(cp_irq_recved))
> +		reinit_completion(cp_irq_recved);
> +
> +	ret = wait_for_completion_interruptible_timeout(cp_irq_recved,
> +							msecs_to_jiffies(
> +							timeout));
> +	reinit_completion(cp_irq_recved);
> +	if (ret < 0)
> +		return (int)ret;
> +	else if (!ret)
> +		return -ETIMEDOUT;
> +	return 0;
> +}
> +
>  static
>  int intel_dp_hdcp_write_an_aksv(struct intel_digital_port *intel_dig_port,
>  				u8 *an)
> @@ -5188,11 +5211,38 @@ int intel_dp_hdcp_repeater_present(struct intel_digital_port *intel_dig_port,
>  	return 0;
>  }
>  
> +static int intel_dp_hdcp_wait_for_r0(struct intel_digital_port *intel_dig_port)
> +{
> +	struct intel_dp *dp = &intel_dig_port->dp;
> +	struct intel_connector *connector = dp->attached_connector;
> +	int ret;
> +	u8 bstatus;
> +
> +	intel_dp_hdcp_wait_for_cp_irq(&connector->cp_irq_recved, 100);
> +
> +	ret = drm_dp_dpcd_read(&intel_dig_port->dp.aux, DP_AUX_HDCP_BSTATUS,
> +			       &bstatus, 1);
> +	if (ret != 1) {
> +		DRM_ERROR("Read bstatus from DP/AUX failed (%d)\n", ret);
> +		return ret >= 0 ? -EIO : ret;
> +	}
> +
> +	if (!(bstatus & DP_BSTATUS_R0_PRIME_READY))
> +		return -ETIMEDOUT;
> +
> +	return 0;
> +}
> +
>  static
>  int intel_dp_hdcp_read_ri_prime(struct intel_digital_port *intel_dig_port,
>  				u8 *ri_prime)
>  {
>  	ssize_t ret;
> +
> +	ret = intel_dp_hdcp_wait_for_r0(intel_dig_port);
> +	if (!ret)
> +		return ret;
> +
>  	ret = drm_dp_dpcd_read(&intel_dig_port->dp.aux, DP_AUX_HDCP_RI_PRIME,
>  			       ri_prime, DRM_HDCP_RI_LEN);
>  	if (ret != DRM_HDCP_RI_LEN) {
> @@ -5203,18 +5253,26 @@ int intel_dp_hdcp_read_ri_prime(struct intel_digital_port *intel_dig_port,
>  }
>  
>  static
> -int intel_dp_hdcp_read_ksv_ready(struct intel_digital_port *intel_dig_port,
> -				 bool *ksv_ready)
> +int intel_dp_hdcp_ksv_ready(struct intel_digital_port *intel_dig_port)
>  {
> +	struct intel_dp *dp = &intel_dig_port->dp;
> +	struct intel_connector *connector = dp->attached_connector;
>  	ssize_t ret;
>  	u8 bstatus;
> +
> +	intel_dp_hdcp_wait_for_cp_irq(&connector->cp_irq_recved,
> +				      5 * 1000 * 1000);
> +
>  	ret = drm_dp_dpcd_read(&intel_dig_port->dp.aux, DP_AUX_HDCP_BSTATUS,
>  			       &bstatus, 1);
>  	if (ret != 1) {
>  		DRM_ERROR("Read bstatus from DP/AUX failed (%zd)\n", ret);
>  		return ret >= 0 ? -EIO : ret;
>  	}
> -	*ksv_ready = bstatus & DP_BSTATUS_READY;
> +
> +	if (!(bstatus & DP_BSTATUS_READY))
> +		return -ETIMEDOUT;
> +
>  	return 0;
>  }
>  
> @@ -5305,7 +5363,7 @@ static const struct intel_hdcp_shim intel_dp_hdcp_shim = {
>  	.read_bstatus = intel_dp_hdcp_read_bstatus,
>  	.repeater_present = intel_dp_hdcp_repeater_present,
>  	.read_ri_prime = intel_dp_hdcp_read_ri_prime,
> -	.read_ksv_ready = intel_dp_hdcp_read_ksv_ready,
> +	.wait_for_ksv_ready = intel_dp_hdcp_ksv_ready,
>  	.read_ksv_fifo = intel_dp_hdcp_read_ksv_fifo,
>  	.read_v_prime_part = intel_dp_hdcp_read_v_prime_part,
>  	.toggle_signalling = intel_dp_hdcp_toggle_signalling,
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 8f38e584d375..ab975107c978 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -353,8 +353,7 @@ struct intel_hdcp_shim {
>  	int (*read_ri_prime)(struct intel_digital_port *intel_dig_port, u8 *ri);
>  
>  	/* Determines if the receiver's KSV FIFO is ready for consumption */
> -	int (*read_ksv_ready)(struct intel_digital_port *intel_dig_port,
> -			      bool *ksv_ready);
> +	int (*wait_for_ksv_ready)(struct intel_digital_port *intel_dig_port);
>  
>  	/* Reads the ksv fifo for num_downstream devices */
>  	int (*read_ksv_fifo)(struct intel_digital_port *intel_dig_port,
> @@ -413,6 +412,9 @@ struct intel_connector {
>  	uint64_t hdcp_value; /* protected by hdcp_mutex */
>  	struct delayed_work hdcp_check_work;
>  	struct work_struct hdcp_prop_work;
> +
> +	/* To indicate the assertion of CP_IRQ */
> +	struct completion cp_irq_recved;
>  };
>  
>  struct intel_digital_connector_state {
> diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
> index 14be14a45e5a..a077e1333886 100644
> --- a/drivers/gpu/drm/i915/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/intel_hdcp.c
> @@ -57,27 +57,6 @@ static bool wait_for_hdcp_port(struct intel_connector *connector)
>  	return true;
>  }
>  
> -static int intel_hdcp_poll_ksv_fifo(struct intel_digital_port *intel_dig_port,
> -				    const struct intel_hdcp_shim *shim)
> -{
> -	int ret, read_ret;
> -	bool ksv_ready;
> -
> -	/* Poll for ksv list ready (spec says max time allowed is 5s) */
> -	ret = __wait_for(read_ret = shim->read_ksv_ready(intel_dig_port,
> -							 &ksv_ready),
> -			 read_ret || ksv_ready, 5 * 1000 * 1000, 1000,
> -			 100 * 1000);
> -	if (ret)
> -		return ret;
> -	if (read_ret)
> -		return read_ret;
> -	if (!ksv_ready)
> -		return -ETIMEDOUT;
> -
> -	return 0;
> -}
> -
>  static bool hdcp_key_loadable(struct drm_i915_private *dev_priv)
>  {
>  	struct i915_power_domains *power_domains = &dev_priv->power_domains;
> @@ -222,7 +201,7 @@ int intel_hdcp_auth_downstream(struct intel_digital_port *intel_dig_port,
>  
>  	dev_priv = intel_dig_port->base.base.dev->dev_private;
>  
> -	ret = intel_hdcp_poll_ksv_fifo(intel_dig_port, shim);
> +	ret = shim->wait_for_ksv_ready(intel_dig_port);
>  	if (ret) {
>  		DRM_ERROR("KSV list failed to become ready (%d)\n", ret);
>  		return ret;
> @@ -476,7 +455,6 @@ static int intel_hdcp_auth(struct intel_digital_port *intel_dig_port,
>  {
>  	struct drm_i915_private *dev_priv;
>  	enum port port;
> -	unsigned long r0_prime_gen_start;
>  	int ret, i, tries = 2;
>  	union {
>  		u32 reg[2];
> @@ -531,8 +509,6 @@ static int intel_hdcp_auth(struct intel_digital_port *intel_dig_port,
>  	if (ret)
>  		return ret;
>  
> -	r0_prime_gen_start = jiffies;
> -
>  	memset(&bksv, 0, sizeof(bksv));
>  
>  	/* HDCP spec states that we must retry the bksv if it is invalid */
> @@ -572,22 +548,11 @@ static int intel_hdcp_auth(struct intel_digital_port *intel_dig_port,
>  	}
>  
>  	/*
> -	 * 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
> -	 * the stupid thing instead of polling on one and not the other.
>  	 */
> -
>  	tries = 3;
>  	for (i = 0; i < tries; i++) {
> -		wait_remaining_ms_from_jiffies(r0_prime_gen_start,
> -					       100 * (i + 1));
> -
>  		ri.reg = 0;
>  		ret = shim->read_ri_prime(intel_dig_port, ri.shim);
>  		if (ret)
> @@ -749,6 +714,8 @@ int intel_hdcp_init(struct intel_connector *connector,
>  	mutex_init(&connector->hdcp_mutex);
>  	INIT_DELAYED_WORK(&connector->hdcp_check_work, intel_hdcp_check_work);
>  	INIT_WORK(&connector->hdcp_prop_work, intel_hdcp_prop_work);
> +
> +	init_completion(&connector->cp_irq_recved);
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index f5d7bfb43006..532d13f91602 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1007,6 +1007,9 @@ int intel_hdmi_hdcp_read_ri_prime(struct intel_digital_port *intel_dig_port,
>  				  u8 *ri_prime)
>  {
>  	int ret;
> +
> +	wait_remaining_ms_from_jiffies(jiffies, 100);
> +
>  	ret = intel_hdmi_hdcp_read(intel_dig_port, DRM_HDCP_DDC_RI_PRIME,
>  				   ri_prime, DRM_HDCP_RI_LEN);
>  	if (ret)
> @@ -1027,6 +1030,29 @@ int intel_hdmi_hdcp_read_ksv_ready(struct intel_digital_port *intel_dig_port,
>  		return ret;
>  	}
>  	*ksv_ready = val & DRM_HDCP_DDC_BCAPS_KSV_FIFO_READY;
> +
> +	return 0;
> +}
> +
> +static
> +int intel_hdmi_hdcp_ksv_ready(struct intel_digital_port *intel_dig_port)
> +{
> +	int ret, read_ret;
> +	bool ksv_ready;
> +
> +	/* Poll for ksv list ready (spec says max time allowed is 5s) */
> +	ret = __wait_for(read_ret =
> +			 intel_hdmi_hdcp_read_ksv_ready(intel_dig_port,
> +							&ksv_ready),
> +			 read_ret || ksv_ready, 5 * 1000 * 1000, 1000,
> +			 100 * 1000);
> +	if (ret)
> +		return ret;
> +	if (read_ret)
> +		return read_ret;
> +	if (!ksv_ready)
> +		return -ETIMEDOUT;
> +
>  	return 0;
>  }
>  
> @@ -1112,7 +1138,7 @@ static const struct intel_hdcp_shim intel_hdmi_hdcp_shim = {
>  	.read_bstatus = intel_hdmi_hdcp_read_bstatus,
>  	.repeater_present = intel_hdmi_hdcp_repeater_present,
>  	.read_ri_prime = intel_hdmi_hdcp_read_ri_prime,
> -	.read_ksv_ready = intel_hdmi_hdcp_read_ksv_ready,
> +	.wait_for_ksv_ready = intel_hdmi_hdcp_ksv_ready,
>  	.read_ksv_fifo = intel_hdmi_hdcp_read_ksv_fifo,
>  	.read_v_prime_part = intel_hdmi_hdcp_read_v_prime_part,
>  	.toggle_signalling = intel_hdmi_hdcp_toggle_signalling,
> -- 
> 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

Re: [PATCH 5/5] drm/i915: Move hdcp msg detection into shim

[Ramalingam C]

On Monday 26 February 2018 11:31 PM, Sean Paul wrote:
> On Mon, Feb 26, 2018 at 10:42:39PM +0530, Ramalingam C wrote:
>> DP and HDMI HDCP specifications are varying with respect to
>> detecting the R0 and ksv_fifo availability.
>>
>> DP will produce CP_IRQ and set a bit for indicating the R0 and
>> FIFO_READY status.
> I'm not sure what the benefit is? Keeping things common was a deliberate
> decision on my part to keep complexity down and increase the amount of common
> code.
I am proposing this expecting that this will address the DP compliance 
failure
incase of regular repeater tests(attempt to read wrong FIFO size, due to 
wrong device count). Possibly a corner case!?.

And since anyway we have the shim for handling
the DP and HDMI specific code, i feel this will do only better but harm.

--Ram
>
> Sean
>
>> Whereas HDMI will set a bit for FIFO_READY but there is no bit
>> indication for R0 ready. And also polling of READY bit is needed for
>> HDMI as ther is no CP_IRQ.
>>
>> So Fielding the CP_IRQ for DP and the polling the READY bit for a
>> period and blindly waiting for a fixed time for R0 incase of HDMI are
>> moved into corresponding hdcp_shim.
>>
>> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_dp.c   | 70 +++++++++++++++++++++++++++++++++++----
>>   drivers/gpu/drm/i915/intel_drv.h  |  6 ++--
>>   drivers/gpu/drm/i915/intel_hdcp.c | 39 ++--------------------
>>   drivers/gpu/drm/i915/intel_hdmi.c | 28 +++++++++++++++-
>>   4 files changed, 98 insertions(+), 45 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index 2c3eb90b9499..afe310a91d3c 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -4436,6 +4436,7 @@ static bool
>>   intel_dp_short_pulse(struct intel_dp *intel_dp)
>>   {
>>   	struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
>> +	struct intel_connector *connector = intel_dp->attached_connector;
>>   	u8 sink_irq_vector = 0;
>>   	u8 old_sink_count = intel_dp->sink_count;
>>   	bool ret;
>> @@ -4470,8 +4471,11 @@ intel_dp_short_pulse(struct intel_dp *intel_dp)
>>   
>>   		if (sink_irq_vector & DP_AUTOMATED_TEST_REQUEST)
>>   			intel_dp_handle_test_request(intel_dp);
>> -		if (sink_irq_vector & (DP_CP_IRQ | DP_SINK_SPECIFIC_IRQ))
>> -			DRM_DEBUG_DRIVER("CP or sink specific irq unhandled\n");
>> +		if (sink_irq_vector & DP_CP_IRQ)
>> +			if (connector->hdcp_shim)
>> +				complete_all(&connector->cp_irq_recved);
>> +		if (sink_irq_vector & DP_SINK_SPECIFIC_IRQ)
>> +			DRM_DEBUG_DRIVER("Sink specific irq unhandled\n");
>>   	}
>>   
>>   	intel_dp_check_link_status(intel_dp);
>> @@ -5083,6 +5087,25 @@ void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder)
>>   	pps_unlock(intel_dp);
>>   }
>>   
>> +static int intel_dp_hdcp_wait_for_cp_irq(struct completion *cp_irq_recved,
>> +					 int timeout)
>> +{
>> +	long ret;
>> +
>> +	if (completion_done(cp_irq_recved))
>> +		reinit_completion(cp_irq_recved);
>> +
>> +	ret = wait_for_completion_interruptible_timeout(cp_irq_recved,
>> +							msecs_to_jiffies(
>> +							timeout));
>> +	reinit_completion(cp_irq_recved);
>> +	if (ret < 0)
>> +		return (int)ret;
>> +	else if (!ret)
>> +		return -ETIMEDOUT;
>> +	return 0;
>> +}
>> +
>>   static
>>   int intel_dp_hdcp_write_an_aksv(struct intel_digital_port *intel_dig_port,
>>   				u8 *an)
>> @@ -5188,11 +5211,38 @@ int intel_dp_hdcp_repeater_present(struct intel_digital_port *intel_dig_port,
>>   	return 0;
>>   }
>>   
>> +static int intel_dp_hdcp_wait_for_r0(struct intel_digital_port *intel_dig_port)
>> +{
>> +	struct intel_dp *dp = &intel_dig_port->dp;
>> +	struct intel_connector *connector = dp->attached_connector;
>> +	int ret;
>> +	u8 bstatus;
>> +
>> +	intel_dp_hdcp_wait_for_cp_irq(&connector->cp_irq_recved, 100);
>> +
>> +	ret = drm_dp_dpcd_read(&intel_dig_port->dp.aux, DP_AUX_HDCP_BSTATUS,
>> +			       &bstatus, 1);
>> +	if (ret != 1) {
>> +		DRM_ERROR("Read bstatus from DP/AUX failed (%d)\n", ret);
>> +		return ret >= 0 ? -EIO : ret;
>> +	}
>> +
>> +	if (!(bstatus & DP_BSTATUS_R0_PRIME_READY))
>> +		return -ETIMEDOUT;
>> +
>> +	return 0;
>> +}
>> +
>>   static
>>   int intel_dp_hdcp_read_ri_prime(struct intel_digital_port *intel_dig_port,
>>   				u8 *ri_prime)
>>   {
>>   	ssize_t ret;
>> +
>> +	ret = intel_dp_hdcp_wait_for_r0(intel_dig_port);
>> +	if (!ret)
>> +		return ret;
>> +
>>   	ret = drm_dp_dpcd_read(&intel_dig_port->dp.aux, DP_AUX_HDCP_RI_PRIME,
>>   			       ri_prime, DRM_HDCP_RI_LEN);
>>   	if (ret != DRM_HDCP_RI_LEN) {
>> @@ -5203,18 +5253,26 @@ int intel_dp_hdcp_read_ri_prime(struct intel_digital_port *intel_dig_port,
>>   }
>>   
>>   static
>> -int intel_dp_hdcp_read_ksv_ready(struct intel_digital_port *intel_dig_port,
>> -				 bool *ksv_ready)
>> +int intel_dp_hdcp_ksv_ready(struct intel_digital_port *intel_dig_port)
>>   {
>> +	struct intel_dp *dp = &intel_dig_port->dp;
>> +	struct intel_connector *connector = dp->attached_connector;
>>   	ssize_t ret;
>>   	u8 bstatus;
>> +
>> +	intel_dp_hdcp_wait_for_cp_irq(&connector->cp_irq_recved,
>> +				      5 * 1000 * 1000);
>> +
>>   	ret = drm_dp_dpcd_read(&intel_dig_port->dp.aux, DP_AUX_HDCP_BSTATUS,
>>   			       &bstatus, 1);
>>   	if (ret != 1) {
>>   		DRM_ERROR("Read bstatus from DP/AUX failed (%zd)\n", ret);
>>   		return ret >= 0 ? -EIO : ret;
>>   	}
>> -	*ksv_ready = bstatus & DP_BSTATUS_READY;
>> +
>> +	if (!(bstatus & DP_BSTATUS_READY))
>> +		return -ETIMEDOUT;
>> +
>>   	return 0;
>>   }
>>   
>> @@ -5305,7 +5363,7 @@ static const struct intel_hdcp_shim intel_dp_hdcp_shim = {
>>   	.read_bstatus = intel_dp_hdcp_read_bstatus,
>>   	.repeater_present = intel_dp_hdcp_repeater_present,
>>   	.read_ri_prime = intel_dp_hdcp_read_ri_prime,
>> -	.read_ksv_ready = intel_dp_hdcp_read_ksv_ready,
>> +	.wait_for_ksv_ready = intel_dp_hdcp_ksv_ready,
>>   	.read_ksv_fifo = intel_dp_hdcp_read_ksv_fifo,
>>   	.read_v_prime_part = intel_dp_hdcp_read_v_prime_part,
>>   	.toggle_signalling = intel_dp_hdcp_toggle_signalling,
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index 8f38e584d375..ab975107c978 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -353,8 +353,7 @@ struct intel_hdcp_shim {
>>   	int (*read_ri_prime)(struct intel_digital_port *intel_dig_port, u8 *ri);
>>   
>>   	/* Determines if the receiver's KSV FIFO is ready for consumption */
>> -	int (*read_ksv_ready)(struct intel_digital_port *intel_dig_port,
>> -			      bool *ksv_ready);
>> +	int (*wait_for_ksv_ready)(struct intel_digital_port *intel_dig_port);
>>   
>>   	/* Reads the ksv fifo for num_downstream devices */
>>   	int (*read_ksv_fifo)(struct intel_digital_port *intel_dig_port,
>> @@ -413,6 +412,9 @@ struct intel_connector {
>>   	uint64_t hdcp_value; /* protected by hdcp_mutex */
>>   	struct delayed_work hdcp_check_work;
>>   	struct work_struct hdcp_prop_work;
>> +
>> +	/* To indicate the assertion of CP_IRQ */
>> +	struct completion cp_irq_recved;
>>   };
>>   
>>   struct intel_digital_connector_state {
>> diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
>> index 14be14a45e5a..a077e1333886 100644
>> --- a/drivers/gpu/drm/i915/intel_hdcp.c
>> +++ b/drivers/gpu/drm/i915/intel_hdcp.c
>> @@ -57,27 +57,6 @@ static bool wait_for_hdcp_port(struct intel_connector *connector)
>>   	return true;
>>   }
>>   
>> -static int intel_hdcp_poll_ksv_fifo(struct intel_digital_port *intel_dig_port,
>> -				    const struct intel_hdcp_shim *shim)
>> -{
>> -	int ret, read_ret;
>> -	bool ksv_ready;
>> -
>> -	/* Poll for ksv list ready (spec says max time allowed is 5s) */
>> -	ret = __wait_for(read_ret = shim->read_ksv_ready(intel_dig_port,
>> -							 &ksv_ready),
>> -			 read_ret || ksv_ready, 5 * 1000 * 1000, 1000,
>> -			 100 * 1000);
>> -	if (ret)
>> -		return ret;
>> -	if (read_ret)
>> -		return read_ret;
>> -	if (!ksv_ready)
>> -		return -ETIMEDOUT;
>> -
>> -	return 0;
>> -}
>> -
>>   static bool hdcp_key_loadable(struct drm_i915_private *dev_priv)
>>   {
>>   	struct i915_power_domains *power_domains = &dev_priv->power_domains;
>> @@ -222,7 +201,7 @@ int intel_hdcp_auth_downstream(struct intel_digital_port *intel_dig_port,
>>   
>>   	dev_priv = intel_dig_port->base.base.dev->dev_private;
>>   
>> -	ret = intel_hdcp_poll_ksv_fifo(intel_dig_port, shim);
>> +	ret = shim->wait_for_ksv_ready(intel_dig_port);
>>   	if (ret) {
>>   		DRM_ERROR("KSV list failed to become ready (%d)\n", ret);
>>   		return ret;
>> @@ -476,7 +455,6 @@ static int intel_hdcp_auth(struct intel_digital_port *intel_dig_port,
>>   {
>>   	struct drm_i915_private *dev_priv;
>>   	enum port port;
>> -	unsigned long r0_prime_gen_start;
>>   	int ret, i, tries = 2;
>>   	union {
>>   		u32 reg[2];
>> @@ -531,8 +509,6 @@ static int intel_hdcp_auth(struct intel_digital_port *intel_dig_port,
>>   	if (ret)
>>   		return ret;
>>   
>> -	r0_prime_gen_start = jiffies;
>> -
>>   	memset(&bksv, 0, sizeof(bksv));
>>   
>>   	/* HDCP spec states that we must retry the bksv if it is invalid */
>> @@ -572,22 +548,11 @@ static int intel_hdcp_auth(struct intel_digital_port *intel_dig_port,
>>   	}
>>   
>>   	/*
>> -	 * 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
>> -	 * the stupid thing instead of polling on one and not the other.
>>   	 */
>> -
>>   	tries = 3;
>>   	for (i = 0; i < tries; i++) {
>> -		wait_remaining_ms_from_jiffies(r0_prime_gen_start,
>> -					       100 * (i + 1));
>> -
>>   		ri.reg = 0;
>>   		ret = shim->read_ri_prime(intel_dig_port, ri.shim);
>>   		if (ret)
>> @@ -749,6 +714,8 @@ int intel_hdcp_init(struct intel_connector *connector,
>>   	mutex_init(&connector->hdcp_mutex);
>>   	INIT_DELAYED_WORK(&connector->hdcp_check_work, intel_hdcp_check_work);
>>   	INIT_WORK(&connector->hdcp_prop_work, intel_hdcp_prop_work);
>> +
>> +	init_completion(&connector->cp_irq_recved);
>>   	return 0;
>>   }
>>   
>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
>> index f5d7bfb43006..532d13f91602 100644
>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>> @@ -1007,6 +1007,9 @@ int intel_hdmi_hdcp_read_ri_prime(struct intel_digital_port *intel_dig_port,
>>   				  u8 *ri_prime)
>>   {
>>   	int ret;
>> +
>> +	wait_remaining_ms_from_jiffies(jiffies, 100);
>> +
>>   	ret = intel_hdmi_hdcp_read(intel_dig_port, DRM_HDCP_DDC_RI_PRIME,
>>   				   ri_prime, DRM_HDCP_RI_LEN);
>>   	if (ret)
>> @@ -1027,6 +1030,29 @@ int intel_hdmi_hdcp_read_ksv_ready(struct intel_digital_port *intel_dig_port,
>>   		return ret;
>>   	}
>>   	*ksv_ready = val & DRM_HDCP_DDC_BCAPS_KSV_FIFO_READY;
>> +
>> +	return 0;
>> +}
>> +
>> +static
>> +int intel_hdmi_hdcp_ksv_ready(struct intel_digital_port *intel_dig_port)
>> +{
>> +	int ret, read_ret;
>> +	bool ksv_ready;
>> +
>> +	/* Poll for ksv list ready (spec says max time allowed is 5s) */
>> +	ret = __wait_for(read_ret =
>> +			 intel_hdmi_hdcp_read_ksv_ready(intel_dig_port,
>> +							&ksv_ready),
>> +			 read_ret || ksv_ready, 5 * 1000 * 1000, 1000,
>> +			 100 * 1000);
>> +	if (ret)
>> +		return ret;
>> +	if (read_ret)
>> +		return read_ret;
>> +	if (!ksv_ready)
>> +		return -ETIMEDOUT;
>> +
>>   	return 0;
>>   }
>>   
>> @@ -1112,7 +1138,7 @@ static const struct intel_hdcp_shim intel_hdmi_hdcp_shim = {
>>   	.read_bstatus = intel_hdmi_hdcp_read_bstatus,
>>   	.repeater_present = intel_hdmi_hdcp_repeater_present,
>>   	.read_ri_prime = intel_hdmi_hdcp_read_ri_prime,
>> -	.read_ksv_ready = intel_hdmi_hdcp_read_ksv_ready,
>> +	.wait_for_ksv_ready = intel_hdmi_hdcp_ksv_ready,
>>   	.read_ksv_fifo = intel_hdmi_hdcp_read_ksv_fifo,
>>   	.read_v_prime_part = intel_hdmi_hdcp_read_v_prime_part,
>>   	.toggle_signalling = intel_hdmi_hdcp_toggle_signalling,
>> -- 
>> 2.7.4
>>

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

Re: [Intel-gfx] [PATCH 5/5] drm/i915: Move hdcp msg detection into shim

[Chris Wilson]

Quoting Ramalingam C (2018-02-26 17:12:39)
> DP and HDMI HDCP specifications are varying with respect to
> detecting the R0 and ksv_fifo availability.
> 
> DP will produce CP_IRQ and set a bit for indicating the R0 and
> FIFO_READY status.
> 
> Whereas HDMI will set a bit for FIFO_READY but there is no bit
> indication for R0 ready. And also polling of READY bit is needed for
> HDMI as ther is no CP_IRQ.
> 
> So Fielding the CP_IRQ for DP and the polling the READY bit for a
> period and blindly waiting for a fixed time for R0 incase of HDMI are
> moved into corresponding hdcp_shim.
> 
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> ---
> +static int intel_dp_hdcp_wait_for_cp_irq(struct completion *cp_irq_recved,
> +                                        int timeout)
> +{
> +       long ret;
> +
> +       if (completion_done(cp_irq_recved))
> +               reinit_completion(cp_irq_recved);
> +
> +       ret = wait_for_completion_interruptible_timeout(cp_irq_recved,
> +                                                       msecs_to_jiffies(
> +                                                       timeout));
> +       reinit_completion(cp_irq_recved);

This is not thread-safe.

The trick is to use a waitqueue to interrupt the sleeps inside the wait
loops to complete the polling quicker. (As stated, you can't escape the
polling, and you always need to check the IRQ was for the event you
expected anyway.)
-Chris
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 5/5] drm/i915: Move hdcp msg detection into shim

[Ramalingam C]

On Tuesday 27 February 2018 04:20 AM, Chris Wilson wrote:
> Quoting Ramalingam C (2018-02-26 17:12:39)
>> DP and HDMI HDCP specifications are varying with respect to
>> detecting the R0 and ksv_fifo availability.
>>
>> DP will produce CP_IRQ and set a bit for indicating the R0 and
>> FIFO_READY status.
>>
>> Whereas HDMI will set a bit for FIFO_READY but there is no bit
>> indication for R0 ready. And also polling of READY bit is needed for
>> HDMI as ther is no CP_IRQ.
>>
>> So Fielding the CP_IRQ for DP and the polling the READY bit for a
>> period and blindly waiting for a fixed time for R0 incase of HDMI are
>> moved into corresponding hdcp_shim.
>>
>> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
>> ---
>> +static int intel_dp_hdcp_wait_for_cp_irq(struct completion *cp_irq_recved,
>> +                                        int timeout)
>> +{
>> +       long ret;
>> +
>> +       if (completion_done(cp_irq_recved))
>> +               reinit_completion(cp_irq_recved);
>> +
>> +       ret = wait_for_completion_interruptible_timeout(cp_irq_recved,
>> +                                                       msecs_to_jiffies(
>> +                                                       timeout));
>> +       reinit_completion(cp_irq_recved);
> This is not thread-safe.
>
> The trick is to use a waitqueue to interrupt the sleeps inside the wait
> loops to complete the polling quicker. (As stated, you can't escape the
> polling, and you always need to check the IRQ was for the event you
> expected anyway.)
> -Chris

Chris,

I think I am lost here. Could you please help me understand what are we missing here?
Completion also uses the wait_queue and each thread that want to wait for a event waits on it.
And on IRQ arrival through complete_all we are marking the completion->done and waking up all the thread sleeping at completion wait_queue.

Are you suggesting wait_event_timeout as we have in intel_dp_aux_wait_done?

Thanks,
--Ram


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

✓ Fi.CI.BAT: success for HDCP1.4 fixes

[Patchwork]

== Series Details ==

Series: HDCP1.4 fixes
URL   : https://patchwork.freedesktop.org/series/38978/
State : success

== Summary ==

Series 38978v1 HDCP1.4 fixes
https://patchwork.freedesktop.org/api/1.0/series/38978/revisions/1/mbox/

---- Known issues:

Test debugfs_test:
        Subgroup read_all_entries:
                incomplete -> PASS       (fi-snb-2520m) fdo#103713

fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:414s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:421s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:372s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:485s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:286s
fi-bxt-dsi       total:119  pass:107  dwarn:0   dfail:0   fail:0   skip:12 
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:476s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:470s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:451s
fi-cfl-8700k     total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:388s
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:559s
fi-cnl-y3        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:580s
fi-elk-e7500     total:288  pass:229  dwarn:0   dfail:0   fail:0   skip:59  time:410s
fi-gdg-551       total:288  pass:179  dwarn:0   dfail:0   fail:1   skip:108 time:285s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:502s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:388s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:408s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:453s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:413s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:450s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:488s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:445s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:491s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:594s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:427s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:502s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:518s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:481s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:479s
fi-skl-guc       total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:416s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:426s
fi-snb-2520m     total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:524s
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:393s

3a86cab5785058d50346e5a26f51dacbaab29c2e drm-tip: 2018y-02m-26d-15h-41m-02s UTC integration manifest
b398f39b1428 drm/i915: Move hdcp msg detection into shim
e107fd3bb4fb drm/i915: Poll hdcp register on sudden NACK
39fc6362ea03 drm/i915: Check hdcp key loadability
58a427113d28 drm/i915: read Vprime thrice incase of mismatch
17d5b4718d50 drm/i915: Read HDCP R0 thrice in case of mismatch

== Logs ==

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

✓ Fi.CI.IGT: success for HDCP1.4 fixes

[Patchwork]

== Series Details ==

Series: HDCP1.4 fixes
URL   : https://patchwork.freedesktop.org/series/38978/
State : success

== Summary ==

---- Known issues:

Test kms_cursor_legacy:
        Subgroup short-flip-after-cursor-atomic-transitions-varying-size:
                pass       -> SKIP       (shard-snb) fdo#102670
Test kms_flip:
        Subgroup dpms-vs-vblank-race:
                pass       -> FAIL       (shard-snb) fdo#103060 +2
        Subgroup plain-flip-fb-recreate-interruptible:
                fail       -> PASS       (shard-hsw) fdo#100368
Test kms_flip_tiling:
        Subgroup flip-yf-tiled:
                fail       -> PASS       (shard-apl) fdo#103822
Test kms_frontbuffer_tracking:
        Subgroup fbc-rgb565-draw-blt:
                pass       -> SKIP       (shard-snb) fdo#101623
Test kms_rotation_crc:
        Subgroup primary-rotation-180:
                fail       -> PASS       (shard-hsw) fdo#103925

fdo#102670 https://bugs.freedesktop.org/show_bug.cgi?id=102670
fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#103822 https://bugs.freedesktop.org/show_bug.cgi?id=103822
fdo#101623 https://bugs.freedesktop.org/show_bug.cgi?id=101623
fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925

shard-apl        total:3558 pass:1869 dwarn:1   dfail:0   fail:7   skip:1680 time:12471s
shard-hsw        total:3558 pass:1821 dwarn:1   dfail:0   fail:2   skip:1733 time:12225s
shard-snb        total:3460 pass:1356 dwarn:1   dfail:0   fail:2   skip:2101 time:6624s
Blacklisted hosts:
shard-kbl        total:3379 pass:1885 dwarn:1   dfail:1   fail:8   skip:1482 time:8946s

== Logs ==

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

Re: [PATCH 0/5] HDCP1.4 fixes

[Rodrigo Vivi]

On Mon, Feb 26, 2018 at 10:42:34PM +0530, Ramalingam C wrote:
> This series addresses the requriement of below HDCP compliance tests
> 	DP: 1A-06 and 1B-05
> 	HDMI: 1A-04 and 1A-07a
> 
> One of the patch uses the I915 power infra-structure for checking
> the power state of PW#1. Which enables the init path for all legacy
> platforms.
> 
> And encoder specific msg availability detection is moved into hdcp_shim.
> This will help to sync with DP hdcp data availability in the best
> possible way by fielding the CP_IRQ.

Do we want any of these on 4.16 or backported?
Please consider the use of "Fixes:" on whatever makes sense.

Thanks,
Rodrigo.

> 
> 
> Ramalingam C (5):
>   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: Poll hdcp register on sudden NACK
>   drm/i915: Move hdcp msg detection into shim
> 
>  drivers/gpu/drm/i915/intel_dp.c   |  70 ++++++++++++++--
>  drivers/gpu/drm/i915/intel_drv.h  |   6 +-
>  drivers/gpu/drm/i915/intel_hdcp.c | 166 +++++++++++++++++++++++++++-----------
>  drivers/gpu/drm/i915/intel_hdmi.c |  28 ++++++-
>  4 files changed, 214 insertions(+), 56 deletions(-)
> 
> -- 
> 2.7.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/5] HDCP1.4 fixes

[Ramalingam C]

On Tuesday 27 February 2018 04:54 AM, Rodrigo Vivi wrote:
> On Mon, Feb 26, 2018 at 10:42:34PM +0530, Ramalingam C wrote:
>> This series addresses the requriement of below HDCP compliance tests
>> 	DP: 1A-06 and 1B-05
>> 	HDMI: 1A-04 and 1A-07a
>>
>> One of the patch uses the I915 power infra-structure for checking
>> the power state of PW#1. Which enables the init path for all legacy
>> platforms.
>>
>> And encoder specific msg availability detection is moved into hdcp_shim.
>> This will help to sync with DP hdcp data availability in the best
>> possible way by fielding the CP_IRQ.
> Do we want any of these on 4.16 or backported?

Rodrigo,

The changes are for addressing the compliance gaps.
So IMHO at present there is no need for back porting them.

> Please consider the use of "Fixes:" on whatever makes sense.

Sure thanks.

--Ram.

>
> Thanks,
> Rodrigo.
>
>>
>> Ramalingam C (5):
>>    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: Poll hdcp register on sudden NACK
>>    drm/i915: Move hdcp msg detection into shim
>>
>>   drivers/gpu/drm/i915/intel_dp.c   |  70 ++++++++++++++--
>>   drivers/gpu/drm/i915/intel_drv.h  |   6 +-
>>   drivers/gpu/drm/i915/intel_hdcp.c | 166 +++++++++++++++++++++++++++-----------
>>   drivers/gpu/drm/i915/intel_hdmi.c |  28 ++++++-
>>   4 files changed, 214 insertions(+), 56 deletions(-)
>>
>> -- 
>> 2.7.4
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

✗ Fi.CI.BAT: failure for HDCP1.4 fixes (rev2)

[Patchwork]

== Series Details ==

Series: HDCP1.4 fixes (rev2)
URL   : https://patchwork.freedesktop.org/series/38978/
State : failure

== Summary ==

Applying: drm/i915: Read HDCP R0 thrice in case of mismatch
Applying: drm/i915: read Vprime thrice incase of mismatch
Applying: drm/i915: Check hdcp key loadability
Applying: drm/i915: Poll hdcp register on sudden NACK
Applying: drm/i915: Move hdcp msg detection into shim
error: Failed to merge in the changes.
Using index info to reconstruct a base tree...
M	drivers/gpu/drm/i915/intel_dp.c
M	drivers/gpu/drm/i915/intel_hdcp.c
Falling back to patching base and 3-way merge...
Auto-merging drivers/gpu/drm/i915/intel_hdcp.c
CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/intel_hdcp.c
Auto-merging drivers/gpu/drm/i915/intel_dp.c
Patch failed at 0005 drm/i915: Move hdcp msg detection into shim
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

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