On 2019-12-12 at 14:02:19 -0500, Sean Paul wrote:
> From: Sean Paul <seanpaul@xxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxxxxx>
> Cc: Ramalingam C <ramalingam.c@xxxxxxxxx>
> Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
> Cc: Sean Paul <seanpaul@xxxxxxxxxxxx>
> Cc: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx>
> Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
> Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: <stable@xxxxxxxxxxxxxxx> # v4.17+
> Signed-off-by: Sean Paul <seanpaul@xxxxxxxxxxxx>
> Link: https://patchwork.freedesktop.org/patch/msgid/20191203173638.94919-2-sean@xxxxxxxxxx #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@xxxxxxxxx>
> 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
>
