[v2,01/12] drm/i915: Fix sha_text population code
diff mbox series

Message ID 20191212190230.188505-2-sean@poorly.run
State New
Headers show
Series
  • drm/i915: Add support for HDCP 1.4 over MST connectors
Related show

Commit Message

Sean Paul Dec. 12, 2019, 7:02 p.m. UTC
From: Sean Paul <seanpaul@chromium.org>

This patch fixes a few bugs:

1- We weren't taking into account sha_leftovers when adding multiple
   ksvs to sha_text. As such, we were or'ing the end of ksv[j - 1] with
   the beginning of ksv[j]

2- In the sha_leftovers == 2 and sha_leftovers == 3 case, bstatus was
   being placed on the wrong half of sha_text, overlapping the leftover
   ksv value

3- In the sha_leftovers == 2 case, we need to manually terminate the
   byte stream with 0x80 since the hardware doesn't have enough room to
   add it after writing M0

The upside is that all of the "HDCP supported" HDMI repeaters I could
find on Amazon just strip HDCP anyways, so it turns out to be _really_
hard to hit any of these cases without an MST hub, which is not (yet)
supported. Oh, and the sha_leftovers == 1 case works perfectly!

Fixes: ee5e5e7a5e0f ("drm/i915: Add HDCP framework + base implementation")
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ramalingam C <ramalingam.c@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Sean Paul <seanpaul@chromium.org>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Cc: <stable@vger.kernel.org> # v4.17+
Signed-off-by: Sean Paul <seanpaul@chromium.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20191203173638.94919-2-sean@poorly.run #v1

Changes in v2:
-None
---
 drivers/gpu/drm/i915/display/intel_hdcp.c | 25 +++++++++++++++++------
 include/drm/drm_hdcp.h                    |  3 +++
 2 files changed, 22 insertions(+), 6 deletions(-)

Comments

Ramalingam C Dec. 13, 2019, 10:20 a.m. UTC | #1
On 2019-12-12 at 14:02:19 -0500, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> This patch fixes a few bugs:
> 
> 1- We weren't taking into account sha_leftovers when adding multiple
>    ksvs to sha_text. As such, we were or'ing the end of ksv[j - 1] with
>    the beginning of ksv[j]
> 
> 2- In the sha_leftovers == 2 and sha_leftovers == 3 case, bstatus was
>    being placed on the wrong half of sha_text, overlapping the leftover
>    ksv value
> 
> 3- In the sha_leftovers == 2 case, we need to manually terminate the
>    byte stream with 0x80 since the hardware doesn't have enough room to
>    add it after writing M0
> 
> The upside is that all of the "HDCP supported" HDMI repeaters I could
> find on Amazon just strip HDCP anyways, so it turns out to be _really_
> hard to hit any of these cases without an MST hub, which is not (yet)
> supported. Oh, and the sha_leftovers == 1 case works perfectly!

Yes. The repeaters tested at our side too with sha_leftovers = 1 with
mostly ksv count of 1. non availability of the repeaters is killer here.
> 
> Fixes: ee5e5e7a5e0f ("drm/i915: Add HDCP framework + base implementation")
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Ramalingam C <ramalingam.c@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Sean Paul <seanpaul@chromium.org>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: intel-gfx@lists.freedesktop.org
> Cc: <stable@vger.kernel.org> # v4.17+
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> Link: https://patchwork.freedesktop.org/patch/msgid/20191203173638.94919-2-sean@poorly.run #v1
> 
> Changes in v2:
> -None
> ---
>  drivers/gpu/drm/i915/display/intel_hdcp.c | 25 +++++++++++++++++------
>  include/drm/drm_hdcp.h                    |  3 +++
>  2 files changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
> index 0fdbd39f6641..eaab9008feef 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> @@ -335,8 +335,10 @@ int intel_hdcp_validate_v_prime(struct intel_connector *connector,
>  
>  		/* Fill up the empty slots in sha_text and write it out */
>  		sha_empty = sizeof(sha_text) - sha_leftovers;
> -		for (j = 0; j < sha_empty; j++)
> -			sha_text |= ksv[j] << ((sizeof(sha_text) - j - 1) * 8);
> +		for (j = 0; j < sha_empty; j++) {
> +			u8 off = ((sizeof(sha_text) - j - 1 - sha_leftovers) * 8);
Didn't hit this as ksv count was 1 mostly with sha_leftovers = 1.
Thanks for fixing it.
> +			sha_text |= ksv[j] << off;
> +		}
>  
>  		ret = intel_write_sha_text(dev_priv, sha_text);
>  		if (ret < 0)
> @@ -426,7 +428,7 @@ int intel_hdcp_validate_v_prime(struct intel_connector *connector,
>  	} else if (sha_leftovers == 2) {
>  		/* Write 32 bits of text */
>  		I915_WRITE(HDCP_REP_CTL, rep_ctl | HDCP_SHA1_TEXT_32);
> -		sha_text |= bstatus[0] << 24 | bstatus[1] << 16;
> +		sha_text |= bstatus[0] << 8 | bstatus[1];
>  		ret = intel_write_sha_text(dev_priv, sha_text);
>  		if (ret < 0)
>  			return ret;
> @@ -440,16 +442,27 @@ int intel_hdcp_validate_v_prime(struct intel_connector *connector,
>  				return ret;
>  			sha_idx += sizeof(sha_text);
>  		}
> +
> +		/*
> +		 * Terminate the SHA-1 stream by hand. For the other leftover
> +		 * cases this is appended by the hardware.
> +		 */
> +		I915_WRITE(HDCP_REP_CTL, rep_ctl | HDCP_SHA1_TEXT_32);
> +		sha_text = DRM_HDCP_SHA1_TERMINATOR << 24;
> +		ret = intel_write_sha_text(dev_priv, sha_text);
> +		if (ret < 0)
> +			return ret;
> +		sha_idx += sizeof(sha_text);
>  	} else if (sha_leftovers == 3) {
> -		/* Write 32 bits of text */
> +		/* Write 32 bits of text (filled from LSB) */
>  		I915_WRITE(HDCP_REP_CTL, rep_ctl | HDCP_SHA1_TEXT_32);
> -		sha_text |= bstatus[0] << 24;
> +		sha_text |= bstatus[0];

Looks good to me though this path never been exercised. Hopefully MST
will do it.

Reviewed-by: Ramalingam C <ramalingam.c@intel.com>

>  		ret = intel_write_sha_text(dev_priv, sha_text);
>  		if (ret < 0)
>  			return ret;
>  		sha_idx += sizeof(sha_text);
>  
> -		/* Write 8 bits of text, 24 bits of M0 */
> +		/* Write 8 bits of text (filled from LSB), 24 bits of M0 */
>  		I915_WRITE(HDCP_REP_CTL, rep_ctl | HDCP_SHA1_TEXT_8);
>  		ret = intel_write_sha_text(dev_priv, bstatus[1]);
>  		if (ret < 0)
> diff --git a/include/drm/drm_hdcp.h b/include/drm/drm_hdcp.h
> index 06a11202a097..20498c822204 100644
> --- a/include/drm/drm_hdcp.h
> +++ b/include/drm/drm_hdcp.h
> @@ -29,6 +29,9 @@
>  /* Slave address for the HDCP registers in the receiver */
>  #define DRM_HDCP_DDC_ADDR			0x3A
>  
> +/* Value to use at the end of the SHA-1 bytestream used for repeaters */
> +#define DRM_HDCP_SHA1_TERMINATOR		0x80
> +
>  /* HDCP register offsets for HDMI/DVI devices */
>  #define DRM_HDCP_DDC_BKSV			0x00
>  #define DRM_HDCP_DDC_RI_PRIME			0x08
> -- 
> Sean Paul, Software Engineer, Google / Chromium OS
>

Patch
diff mbox series

diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
index 0fdbd39f6641..eaab9008feef 100644
--- a/drivers/gpu/drm/i915/display/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
@@ -335,8 +335,10 @@  int intel_hdcp_validate_v_prime(struct intel_connector *connector,
 
 		/* Fill up the empty slots in sha_text and write it out */
 		sha_empty = sizeof(sha_text) - sha_leftovers;
-		for (j = 0; j < sha_empty; j++)
-			sha_text |= ksv[j] << ((sizeof(sha_text) - j - 1) * 8);
+		for (j = 0; j < sha_empty; j++) {
+			u8 off = ((sizeof(sha_text) - j - 1 - sha_leftovers) * 8);
+			sha_text |= ksv[j] << off;
+		}
 
 		ret = intel_write_sha_text(dev_priv, sha_text);
 		if (ret < 0)
@@ -426,7 +428,7 @@  int intel_hdcp_validate_v_prime(struct intel_connector *connector,
 	} else if (sha_leftovers == 2) {
 		/* Write 32 bits of text */
 		I915_WRITE(HDCP_REP_CTL, rep_ctl | HDCP_SHA1_TEXT_32);
-		sha_text |= bstatus[0] << 24 | bstatus[1] << 16;
+		sha_text |= bstatus[0] << 8 | bstatus[1];
 		ret = intel_write_sha_text(dev_priv, sha_text);
 		if (ret < 0)
 			return ret;
@@ -440,16 +442,27 @@  int intel_hdcp_validate_v_prime(struct intel_connector *connector,
 				return ret;
 			sha_idx += sizeof(sha_text);
 		}
+
+		/*
+		 * Terminate the SHA-1 stream by hand. For the other leftover
+		 * cases this is appended by the hardware.
+		 */
+		I915_WRITE(HDCP_REP_CTL, rep_ctl | HDCP_SHA1_TEXT_32);
+		sha_text = DRM_HDCP_SHA1_TERMINATOR << 24;
+		ret = intel_write_sha_text(dev_priv, sha_text);
+		if (ret < 0)
+			return ret;
+		sha_idx += sizeof(sha_text);
 	} else if (sha_leftovers == 3) {
-		/* Write 32 bits of text */
+		/* Write 32 bits of text (filled from LSB) */
 		I915_WRITE(HDCP_REP_CTL, rep_ctl | HDCP_SHA1_TEXT_32);
-		sha_text |= bstatus[0] << 24;
+		sha_text |= bstatus[0];
 		ret = intel_write_sha_text(dev_priv, sha_text);
 		if (ret < 0)
 			return ret;
 		sha_idx += sizeof(sha_text);
 
-		/* Write 8 bits of text, 24 bits of M0 */
+		/* Write 8 bits of text (filled from LSB), 24 bits of M0 */
 		I915_WRITE(HDCP_REP_CTL, rep_ctl | HDCP_SHA1_TEXT_8);
 		ret = intel_write_sha_text(dev_priv, bstatus[1]);
 		if (ret < 0)
diff --git a/include/drm/drm_hdcp.h b/include/drm/drm_hdcp.h
index 06a11202a097..20498c822204 100644
--- a/include/drm/drm_hdcp.h
+++ b/include/drm/drm_hdcp.h
@@ -29,6 +29,9 @@ 
 /* Slave address for the HDCP registers in the receiver */
 #define DRM_HDCP_DDC_ADDR			0x3A
 
+/* Value to use at the end of the SHA-1 bytestream used for repeaters */
+#define DRM_HDCP_SHA1_TERMINATOR		0x80
+
 /* HDCP register offsets for HDMI/DVI devices */
 #define DRM_HDCP_DDC_BKSV			0x00
 #define DRM_HDCP_DDC_RI_PRIME			0x08