[PATCH 00/11] drm/i915: Add support for HDCP 1.4 over MST connectors

[PATCH 00/11] drm/i915: Add support for HDCP 1.4 over MST connectors

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

Hey all,
As the subject says, this set adds support for HDCP 1.4 over MST. Most
of the set is plumbing and refactor to allow the MST support to slot in
organically.

I stubbed out HDCP 2.2 support since I don't have a means of testing it.
If no one picks up the slack, I can come back to it at a later date when
I have the right gear.

Please take a look,

Sean

Sean Paul (11):
  drm/i915: Fix sha_text population code
  drm/i915: Intercept Aksv writes in the aux hooks
  drm/i915: Disable HDCP signalling on transcoder disable
  drm/i915: Don't WARN on HDCP toggle if get_hw_state returns false
  drm/i915: Change toggle_signalling() argument to connector
  drm/i915: Factor out hdcp->value assignments
  drm/i915: Don't fully disable HDCP on a port if multiple pipes are
    using it
  drm/i915: Support DP MST in enc_to_dig_port() function
  drm/i915: Use ddi_update_pipe in intel_dp_mst
  drm/i915: Expose HDCP shim functions from dp for use by dp_mst
  drm/i915: Add HDCP 1.4 support for MST connectors

 drivers/gpu/drm/i915/display/intel_ddi.c      |  27 ++--
 .../drm/i915/display/intel_display_types.h    |  48 +++++-
 drivers/gpu/drm/i915/display/intel_dp.c       |  82 +++++-----
 drivers/gpu/drm/i915/display/intel_dp.h       |   6 +
 drivers/gpu/drm/i915/display/intel_dp_mst.c   |  84 ++++++++++
 drivers/gpu/drm/i915/display/intel_hdcp.c     | 145 +++++++++++++-----
 drivers/gpu/drm/i915/display/intel_hdmi.c     |  10 +-
 include/drm/drm_hdcp.h                        |   3 +
 8 files changed, 298 insertions(+), 107 deletions(-)

-- 
Sean Paul, Software Engineer, Google / Chromium OS

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

[Intel-gfx] [PATCH 00/11] drm/i915: Add support for HDCP 1.4 over MST connectors

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

Hey all,
As the subject says, this set adds support for HDCP 1.4 over MST. Most
of the set is plumbing and refactor to allow the MST support to slot in
organically.

I stubbed out HDCP 2.2 support since I don't have a means of testing it.
If no one picks up the slack, I can come back to it at a later date when
I have the right gear.

Please take a look,

Sean

Sean Paul (11):
  drm/i915: Fix sha_text population code
  drm/i915: Intercept Aksv writes in the aux hooks
  drm/i915: Disable HDCP signalling on transcoder disable
  drm/i915: Don't WARN on HDCP toggle if get_hw_state returns false
  drm/i915: Change toggle_signalling() argument to connector
  drm/i915: Factor out hdcp->value assignments
  drm/i915: Don't fully disable HDCP on a port if multiple pipes are
    using it
  drm/i915: Support DP MST in enc_to_dig_port() function
  drm/i915: Use ddi_update_pipe in intel_dp_mst
  drm/i915: Expose HDCP shim functions from dp for use by dp_mst
  drm/i915: Add HDCP 1.4 support for MST connectors

 drivers/gpu/drm/i915/display/intel_ddi.c      |  27 ++--
 .../drm/i915/display/intel_display_types.h    |  48 +++++-
 drivers/gpu/drm/i915/display/intel_dp.c       |  82 +++++-----
 drivers/gpu/drm/i915/display/intel_dp.h       |   6 +
 drivers/gpu/drm/i915/display/intel_dp_mst.c   |  84 ++++++++++
 drivers/gpu/drm/i915/display/intel_hdcp.c     | 145 +++++++++++++-----
 drivers/gpu/drm/i915/display/intel_hdmi.c     |  10 +-
 include/drm/drm_hdcp.h                        |   3 +
 8 files changed, 298 insertions(+), 107 deletions(-)

-- 
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 01/11] drm/i915: Fix sha_text population code

[Sean Paul]

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 <ramalingm.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>
---
 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 f1f41ca8402b..8325bf9501e4 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
-- 
Sean Paul, Software Engineer, Google / Chromium OS

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

[PATCH 01/11] drm/i915: Fix sha_text population code

[Sean Paul]

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 <ramalingm.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>
---
 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 f1f41ca8402b..8325bf9501e4 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
-- 
Sean Paul, Software Engineer, Google / Chromium OS

[Intel-gfx] [PATCH 01/11] drm/i915: Fix sha_text population code

[Sean Paul]

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 <ramalingm.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>
---
 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 f1f41ca8402b..8325bf9501e4 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
-- 
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 01/11] drm/i915: Fix sha_text population code

[Sasha Levin]

Hi,

[This is an automated email]

This commit has been processed because it contains a "Fixes:" tag,
fixing commit: ee5e5e7a5e0f ("drm/i915: Add HDCP framework + base implementation").

The bot has tested the following trees: v5.4.1, v5.3.14, v4.19.87.

v5.4.1: Build OK!
v5.3.14: Build OK!
v4.19.87: Failed to apply! Possible dependencies:
    Unable to calculate


NOTE: The patch will not be queued to stable trees until it is upstream.

How should we proceed with this patch?

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

Re: [Intel-gfx] [PATCH 01/11] drm/i915: Fix sha_text population code

[Sasha Levin]

Hi,

[This is an automated email]

This commit has been processed because it contains a "Fixes:" tag,
fixing commit: ee5e5e7a5e0f ("drm/i915: Add HDCP framework + base implementation").

The bot has tested the following trees: v5.4.1, v5.3.14, v4.19.87.

v5.4.1: Build OK!
v5.3.14: Build OK!
v4.19.87: Failed to apply! Possible dependencies:
    Unable to calculate


NOTE: The patch will not be queued to stable trees until it is upstream.

How should we proceed with this patch?

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

[PATCH 02/11] drm/i915: Intercept Aksv writes in the aux hooks

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

Instead of hand rolling the transfer ourselves in the hdcp hook, inspect
aux messages and add the aksv flag in the aux transfer hook.

IIRC, this was the original implementation and folks wanted this hack to
be isolated to the hdcp code, which makes sense.

However in testing an LG monitor on my desk, I noticed it was passing
back a DEFER reply. This wasn't handled in our hand-rolled code and HDCP
auth was failing as a result. Instead of copy/pasting all of the retry
logic and delays from drm dp helpers, let's just use the helpers and hide
the aksv select as best as we can.

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 64 ++++++++++++-------------
 1 file changed, 31 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index d958e789ab96..7a407c651fb2 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1515,12 +1515,29 @@ intel_dp_aux_header(u8 txbuf[HEADER_SIZE],
 	txbuf[3] = msg->size - 1;
 }
 
+static u32 intel_dp_aux_generate_xfer_flags(struct drm_dp_aux_msg *msg)
+{
+	if ((msg->request & ~DP_AUX_I2C_MOT) != DP_AUX_NATIVE_WRITE)
+		return 0;
+
+	/*
+	 * If we're trying to send the HDCP Aksv, we need to set a the Aksv
+	 * select bit to inform the hardware to send the Aksv after our header
+	 * since we can't access that data from software.
+	 */
+	if (msg->address == DP_AUX_HDCP_AKSV)
+		return DP_AUX_CH_CTL_AUX_AKSV_SELECT;
+
+	return 0;
+}
+
 static ssize_t
 intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
 {
 	struct intel_dp *intel_dp = container_of(aux, struct intel_dp, aux);
 	u8 txbuf[20], rxbuf[20];
 	size_t txsize, rxsize;
+	u32 flags = intel_dp_aux_generate_xfer_flags(msg);
 	int ret;
 
 	intel_dp_aux_header(txbuf, msg);
@@ -1541,7 +1558,7 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
 			memcpy(txbuf + HEADER_SIZE, msg->buffer, msg->size);
 
 		ret = intel_dp_aux_xfer(intel_dp, txbuf, txsize,
-					rxbuf, rxsize, 0);
+					rxbuf, rxsize, flags);
 		if (ret > 0) {
 			msg->reply = rxbuf[0] >> 4;
 
@@ -1564,7 +1581,7 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
 			return -E2BIG;
 
 		ret = intel_dp_aux_xfer(intel_dp, txbuf, txsize,
-					rxbuf, rxsize, 0);
+					rxbuf, rxsize, flags);
 		if (ret > 0) {
 			msg->reply = rxbuf[0] >> 4;
 			/*
@@ -5858,17 +5875,9 @@ static
 int intel_dp_hdcp_write_an_aksv(struct intel_digital_port *intel_dig_port,
 				u8 *an)
 {
-	struct intel_dp *intel_dp = enc_to_intel_dp(&intel_dig_port->base.base);
-	static const struct drm_dp_aux_msg msg = {
-		.request = DP_AUX_NATIVE_WRITE,
-		.address = DP_AUX_HDCP_AKSV,
-		.size = DRM_HDCP_KSV_LEN,
-	};
-	u8 txbuf[HEADER_SIZE + DRM_HDCP_KSV_LEN] = {}, rxbuf[2], reply = 0;
+	u8 txbuf[DRM_HDCP_KSV_LEN] = {};
 	ssize_t dpcd_ret;
-	int ret;
 
-	/* Output An first, that's easy */
 	dpcd_ret = drm_dp_dpcd_write(&intel_dig_port->dp.aux, DP_AUX_HDCP_AN,
 				     an, DRM_HDCP_AN_LEN);
 	if (dpcd_ret != DRM_HDCP_AN_LEN) {
@@ -5878,29 +5887,18 @@ int intel_dp_hdcp_write_an_aksv(struct intel_digital_port *intel_dig_port,
 	}
 
 	/*
-	 * Since Aksv is Oh-So-Secret, we can't access it in software. So in
-	 * order to get it on the wire, we need to create the AUX header as if
-	 * we were writing the data, and then tickle the hardware to output the
-	 * data once the header is sent out.
+	 * Since Aksv is Oh-So-Secret, we can't access it in software. So we
+	 * send an empty buffer of the correct length through the DP helpers. On
+	 * the other side, in the transfer hook, we'll generate a flag based on
+	 * the destination address which will tickle the hardware to output the
+	 * Aksv on our behalf after the header is sent.
 	 */
-	intel_dp_aux_header(txbuf, &msg);
-
-	ret = intel_dp_aux_xfer(intel_dp, txbuf, HEADER_SIZE + msg.size,
-				rxbuf, sizeof(rxbuf),
-				DP_AUX_CH_CTL_AUX_AKSV_SELECT);
-	if (ret < 0) {
-		DRM_DEBUG_KMS("Write Aksv over DP/AUX failed (%d)\n", ret);
-		return ret;
-	} else if (ret == 0) {
-		DRM_DEBUG_KMS("Aksv write over DP/AUX was empty\n");
-		return -EIO;
-	}
-
-	reply = (rxbuf[0] >> 4) & DP_AUX_NATIVE_REPLY_MASK;
-	if (reply != DP_AUX_NATIVE_REPLY_ACK) {
-		DRM_DEBUG_KMS("Aksv write: no DP_AUX_NATIVE_REPLY_ACK %x\n",
-			      reply);
-		return -EIO;
+	dpcd_ret = drm_dp_dpcd_write(&intel_dig_port->dp.aux, DP_AUX_HDCP_AKSV,
+				     txbuf, DRM_HDCP_KSV_LEN);
+	if (dpcd_ret != DRM_HDCP_KSV_LEN) {
+		DRM_DEBUG_KMS("Failed to write Aksv over DP/AUX (%zd)\n",
+			      dpcd_ret);
+		return dpcd_ret >= 0 ? -EIO : dpcd_ret;
 	}
 	return 0;
 }
-- 
Sean Paul, Software Engineer, Google / Chromium OS

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

[Intel-gfx] [PATCH 02/11] drm/i915: Intercept Aksv writes in the aux hooks

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

Instead of hand rolling the transfer ourselves in the hdcp hook, inspect
aux messages and add the aksv flag in the aux transfer hook.

IIRC, this was the original implementation and folks wanted this hack to
be isolated to the hdcp code, which makes sense.

However in testing an LG monitor on my desk, I noticed it was passing
back a DEFER reply. This wasn't handled in our hand-rolled code and HDCP
auth was failing as a result. Instead of copy/pasting all of the retry
logic and delays from drm dp helpers, let's just use the helpers and hide
the aksv select as best as we can.

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 64 ++++++++++++-------------
 1 file changed, 31 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index d958e789ab96..7a407c651fb2 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1515,12 +1515,29 @@ intel_dp_aux_header(u8 txbuf[HEADER_SIZE],
 	txbuf[3] = msg->size - 1;
 }
 
+static u32 intel_dp_aux_generate_xfer_flags(struct drm_dp_aux_msg *msg)
+{
+	if ((msg->request & ~DP_AUX_I2C_MOT) != DP_AUX_NATIVE_WRITE)
+		return 0;
+
+	/*
+	 * If we're trying to send the HDCP Aksv, we need to set a the Aksv
+	 * select bit to inform the hardware to send the Aksv after our header
+	 * since we can't access that data from software.
+	 */
+	if (msg->address == DP_AUX_HDCP_AKSV)
+		return DP_AUX_CH_CTL_AUX_AKSV_SELECT;
+
+	return 0;
+}
+
 static ssize_t
 intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
 {
 	struct intel_dp *intel_dp = container_of(aux, struct intel_dp, aux);
 	u8 txbuf[20], rxbuf[20];
 	size_t txsize, rxsize;
+	u32 flags = intel_dp_aux_generate_xfer_flags(msg);
 	int ret;
 
 	intel_dp_aux_header(txbuf, msg);
@@ -1541,7 +1558,7 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
 			memcpy(txbuf + HEADER_SIZE, msg->buffer, msg->size);
 
 		ret = intel_dp_aux_xfer(intel_dp, txbuf, txsize,
-					rxbuf, rxsize, 0);
+					rxbuf, rxsize, flags);
 		if (ret > 0) {
 			msg->reply = rxbuf[0] >> 4;
 
@@ -1564,7 +1581,7 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
 			return -E2BIG;
 
 		ret = intel_dp_aux_xfer(intel_dp, txbuf, txsize,
-					rxbuf, rxsize, 0);
+					rxbuf, rxsize, flags);
 		if (ret > 0) {
 			msg->reply = rxbuf[0] >> 4;
 			/*
@@ -5858,17 +5875,9 @@ static
 int intel_dp_hdcp_write_an_aksv(struct intel_digital_port *intel_dig_port,
 				u8 *an)
 {
-	struct intel_dp *intel_dp = enc_to_intel_dp(&intel_dig_port->base.base);
-	static const struct drm_dp_aux_msg msg = {
-		.request = DP_AUX_NATIVE_WRITE,
-		.address = DP_AUX_HDCP_AKSV,
-		.size = DRM_HDCP_KSV_LEN,
-	};
-	u8 txbuf[HEADER_SIZE + DRM_HDCP_KSV_LEN] = {}, rxbuf[2], reply = 0;
+	u8 txbuf[DRM_HDCP_KSV_LEN] = {};
 	ssize_t dpcd_ret;
-	int ret;
 
-	/* Output An first, that's easy */
 	dpcd_ret = drm_dp_dpcd_write(&intel_dig_port->dp.aux, DP_AUX_HDCP_AN,
 				     an, DRM_HDCP_AN_LEN);
 	if (dpcd_ret != DRM_HDCP_AN_LEN) {
@@ -5878,29 +5887,18 @@ int intel_dp_hdcp_write_an_aksv(struct intel_digital_port *intel_dig_port,
 	}
 
 	/*
-	 * Since Aksv is Oh-So-Secret, we can't access it in software. So in
-	 * order to get it on the wire, we need to create the AUX header as if
-	 * we were writing the data, and then tickle the hardware to output the
-	 * data once the header is sent out.
+	 * Since Aksv is Oh-So-Secret, we can't access it in software. So we
+	 * send an empty buffer of the correct length through the DP helpers. On
+	 * the other side, in the transfer hook, we'll generate a flag based on
+	 * the destination address which will tickle the hardware to output the
+	 * Aksv on our behalf after the header is sent.
 	 */
-	intel_dp_aux_header(txbuf, &msg);
-
-	ret = intel_dp_aux_xfer(intel_dp, txbuf, HEADER_SIZE + msg.size,
-				rxbuf, sizeof(rxbuf),
-				DP_AUX_CH_CTL_AUX_AKSV_SELECT);
-	if (ret < 0) {
-		DRM_DEBUG_KMS("Write Aksv over DP/AUX failed (%d)\n", ret);
-		return ret;
-	} else if (ret == 0) {
-		DRM_DEBUG_KMS("Aksv write over DP/AUX was empty\n");
-		return -EIO;
-	}
-
-	reply = (rxbuf[0] >> 4) & DP_AUX_NATIVE_REPLY_MASK;
-	if (reply != DP_AUX_NATIVE_REPLY_ACK) {
-		DRM_DEBUG_KMS("Aksv write: no DP_AUX_NATIVE_REPLY_ACK %x\n",
-			      reply);
-		return -EIO;
+	dpcd_ret = drm_dp_dpcd_write(&intel_dig_port->dp.aux, DP_AUX_HDCP_AKSV,
+				     txbuf, DRM_HDCP_KSV_LEN);
+	if (dpcd_ret != DRM_HDCP_KSV_LEN) {
+		DRM_DEBUG_KMS("Failed to write Aksv over DP/AUX (%zd)\n",
+			      dpcd_ret);
+		return dpcd_ret >= 0 ? -EIO : dpcd_ret;
 	}
 	return 0;
 }
-- 
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: [Intel-gfx] [PATCH 02/11] drm/i915: Intercept Aksv writes in the aux hooks

[Ville Syrjälä]

On Tue, Dec 03, 2019 at 12:36:25PM -0500, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> Instead of hand rolling the transfer ourselves in the hdcp hook, inspect
> aux messages and add the aksv flag in the aux transfer hook.
> 
> IIRC, this was the original implementation and folks wanted this hack to
> be isolated to the hdcp code, which makes sense.
> 
> However in testing an LG monitor on my desk, I noticed it was passing
> back a DEFER reply. This wasn't handled in our hand-rolled code and HDCP
> auth was failing as a result. Instead of copy/pasting all of the retry
> logic and delays from drm dp helpers, let's just use the helpers and hide
> the aksv select as best as we can.
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 64 ++++++++++++-------------
>  1 file changed, 31 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index d958e789ab96..7a407c651fb2 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -1515,12 +1515,29 @@ intel_dp_aux_header(u8 txbuf[HEADER_SIZE],
>  	txbuf[3] = msg->size - 1;
>  }
>  
> +static u32 intel_dp_aux_generate_xfer_flags(struct drm_dp_aux_msg *msg)
                                               ^
					       const

"_generate_" seems a bit redundant.

> +{
> +	if ((msg->request & ~DP_AUX_I2C_MOT) != DP_AUX_NATIVE_WRITE)
> +		return 0;
> +
> +	/*
> +	 * If we're trying to send the HDCP Aksv, we need to set a the Aksv
> +	 * select bit to inform the hardware to send the Aksv after our header
> +	 * since we can't access that data from software.
> +	 */
> +	if (msg->address == DP_AUX_HDCP_AKSV)
> +		return DP_AUX_CH_CTL_AUX_AKSV_SELECT;

if (DP_AUX_NATIVE_WRITE &&
    DP_AUX_HDCP_AKSV)
    return ...;

would be a little more clear to me since the two things are related here.

> +
> +	return 0;
> +}
> +
>  static ssize_t
>  intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
>  {
>  	struct intel_dp *intel_dp = container_of(aux, struct intel_dp, aux);
>  	u8 txbuf[20], rxbuf[20];
>  	size_t txsize, rxsize;
> +	u32 flags = intel_dp_aux_generate_xfer_flags(msg);
>  	int ret;
>  
>  	intel_dp_aux_header(txbuf, msg);
> @@ -1541,7 +1558,7 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
>  			memcpy(txbuf + HEADER_SIZE, msg->buffer, msg->size);
>  
>  		ret = intel_dp_aux_xfer(intel_dp, txbuf, txsize,
> -					rxbuf, rxsize, 0);
> +					rxbuf, rxsize, flags);
>  		if (ret > 0) {
>  			msg->reply = rxbuf[0] >> 4;
>  
> @@ -1564,7 +1581,7 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
>  			return -E2BIG;
>  
>  		ret = intel_dp_aux_xfer(intel_dp, txbuf, txsize,
> -					rxbuf, rxsize, 0);
> +					rxbuf, rxsize, flags);
>  		if (ret > 0) {
>  			msg->reply = rxbuf[0] >> 4;
>  			/*
> @@ -5858,17 +5875,9 @@ static
>  int intel_dp_hdcp_write_an_aksv(struct intel_digital_port *intel_dig_port,
>  				u8 *an)
>  {
> -	struct intel_dp *intel_dp = enc_to_intel_dp(&intel_dig_port->base.base);
> -	static const struct drm_dp_aux_msg msg = {
> -		.request = DP_AUX_NATIVE_WRITE,
> -		.address = DP_AUX_HDCP_AKSV,
> -		.size = DRM_HDCP_KSV_LEN,
> -	};
> -	u8 txbuf[HEADER_SIZE + DRM_HDCP_KSV_LEN] = {}, rxbuf[2], reply = 0;
> +	u8 txbuf[DRM_HDCP_KSV_LEN] = {};

s/txbuf/ksv/ or something maybe?

Looks reasonable to me
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  	ssize_t dpcd_ret;
> -	int ret;
>  
> -	/* Output An first, that's easy */
>  	dpcd_ret = drm_dp_dpcd_write(&intel_dig_port->dp.aux, DP_AUX_HDCP_AN,
>  				     an, DRM_HDCP_AN_LEN);
>  	if (dpcd_ret != DRM_HDCP_AN_LEN) {
> @@ -5878,29 +5887,18 @@ int intel_dp_hdcp_write_an_aksv(struct intel_digital_port *intel_dig_port,
>  	}
>  
>  	/*
> -	 * Since Aksv is Oh-So-Secret, we can't access it in software. So in
> -	 * order to get it on the wire, we need to create the AUX header as if
> -	 * we were writing the data, and then tickle the hardware to output the
> -	 * data once the header is sent out.
> +	 * Since Aksv is Oh-So-Secret, we can't access it in software. So we
> +	 * send an empty buffer of the correct length through the DP helpers. On
> +	 * the other side, in the transfer hook, we'll generate a flag based on
> +	 * the destination address which will tickle the hardware to output the
> +	 * Aksv on our behalf after the header is sent.
>  	 */
> -	intel_dp_aux_header(txbuf, &msg);
> -
> -	ret = intel_dp_aux_xfer(intel_dp, txbuf, HEADER_SIZE + msg.size,
> -				rxbuf, sizeof(rxbuf),
> -				DP_AUX_CH_CTL_AUX_AKSV_SELECT);
> -	if (ret < 0) {
> -		DRM_DEBUG_KMS("Write Aksv over DP/AUX failed (%d)\n", ret);
> -		return ret;
> -	} else if (ret == 0) {
> -		DRM_DEBUG_KMS("Aksv write over DP/AUX was empty\n");
> -		return -EIO;
> -	}
> -
> -	reply = (rxbuf[0] >> 4) & DP_AUX_NATIVE_REPLY_MASK;
> -	if (reply != DP_AUX_NATIVE_REPLY_ACK) {
> -		DRM_DEBUG_KMS("Aksv write: no DP_AUX_NATIVE_REPLY_ACK %x\n",
> -			      reply);
> -		return -EIO;
> +	dpcd_ret = drm_dp_dpcd_write(&intel_dig_port->dp.aux, DP_AUX_HDCP_AKSV,
> +				     txbuf, DRM_HDCP_KSV_LEN);
> +	if (dpcd_ret != DRM_HDCP_KSV_LEN) {
> +		DRM_DEBUG_KMS("Failed to write Aksv over DP/AUX (%zd)\n",
> +			      dpcd_ret);
> +		return dpcd_ret >= 0 ? -EIO : dpcd_ret;
>  	}
>  	return 0;
>  }
> -- 
> Sean Paul, Software Engineer, Google / Chromium OS
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [Intel-gfx] [PATCH 02/11] drm/i915: Intercept Aksv writes in the aux hooks

[Ville Syrjälä]

On Tue, Dec 03, 2019 at 12:36:25PM -0500, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> Instead of hand rolling the transfer ourselves in the hdcp hook, inspect
> aux messages and add the aksv flag in the aux transfer hook.
> 
> IIRC, this was the original implementation and folks wanted this hack to
> be isolated to the hdcp code, which makes sense.
> 
> However in testing an LG monitor on my desk, I noticed it was passing
> back a DEFER reply. This wasn't handled in our hand-rolled code and HDCP
> auth was failing as a result. Instead of copy/pasting all of the retry
> logic and delays from drm dp helpers, let's just use the helpers and hide
> the aksv select as best as we can.
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 64 ++++++++++++-------------
>  1 file changed, 31 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index d958e789ab96..7a407c651fb2 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -1515,12 +1515,29 @@ intel_dp_aux_header(u8 txbuf[HEADER_SIZE],
>  	txbuf[3] = msg->size - 1;
>  }
>  
> +static u32 intel_dp_aux_generate_xfer_flags(struct drm_dp_aux_msg *msg)
                                               ^
					       const

"_generate_" seems a bit redundant.

> +{
> +	if ((msg->request & ~DP_AUX_I2C_MOT) != DP_AUX_NATIVE_WRITE)
> +		return 0;
> +
> +	/*
> +	 * If we're trying to send the HDCP Aksv, we need to set a the Aksv
> +	 * select bit to inform the hardware to send the Aksv after our header
> +	 * since we can't access that data from software.
> +	 */
> +	if (msg->address == DP_AUX_HDCP_AKSV)
> +		return DP_AUX_CH_CTL_AUX_AKSV_SELECT;

if (DP_AUX_NATIVE_WRITE &&
    DP_AUX_HDCP_AKSV)
    return ...;

would be a little more clear to me since the two things are related here.

> +
> +	return 0;
> +}
> +
>  static ssize_t
>  intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
>  {
>  	struct intel_dp *intel_dp = container_of(aux, struct intel_dp, aux);
>  	u8 txbuf[20], rxbuf[20];
>  	size_t txsize, rxsize;
> +	u32 flags = intel_dp_aux_generate_xfer_flags(msg);
>  	int ret;
>  
>  	intel_dp_aux_header(txbuf, msg);
> @@ -1541,7 +1558,7 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
>  			memcpy(txbuf + HEADER_SIZE, msg->buffer, msg->size);
>  
>  		ret = intel_dp_aux_xfer(intel_dp, txbuf, txsize,
> -					rxbuf, rxsize, 0);
> +					rxbuf, rxsize, flags);
>  		if (ret > 0) {
>  			msg->reply = rxbuf[0] >> 4;
>  
> @@ -1564,7 +1581,7 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
>  			return -E2BIG;
>  
>  		ret = intel_dp_aux_xfer(intel_dp, txbuf, txsize,
> -					rxbuf, rxsize, 0);
> +					rxbuf, rxsize, flags);
>  		if (ret > 0) {
>  			msg->reply = rxbuf[0] >> 4;
>  			/*
> @@ -5858,17 +5875,9 @@ static
>  int intel_dp_hdcp_write_an_aksv(struct intel_digital_port *intel_dig_port,
>  				u8 *an)
>  {
> -	struct intel_dp *intel_dp = enc_to_intel_dp(&intel_dig_port->base.base);
> -	static const struct drm_dp_aux_msg msg = {
> -		.request = DP_AUX_NATIVE_WRITE,
> -		.address = DP_AUX_HDCP_AKSV,
> -		.size = DRM_HDCP_KSV_LEN,
> -	};
> -	u8 txbuf[HEADER_SIZE + DRM_HDCP_KSV_LEN] = {}, rxbuf[2], reply = 0;
> +	u8 txbuf[DRM_HDCP_KSV_LEN] = {};

s/txbuf/ksv/ or something maybe?

Looks reasonable to me
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  	ssize_t dpcd_ret;
> -	int ret;
>  
> -	/* Output An first, that's easy */
>  	dpcd_ret = drm_dp_dpcd_write(&intel_dig_port->dp.aux, DP_AUX_HDCP_AN,
>  				     an, DRM_HDCP_AN_LEN);
>  	if (dpcd_ret != DRM_HDCP_AN_LEN) {
> @@ -5878,29 +5887,18 @@ int intel_dp_hdcp_write_an_aksv(struct intel_digital_port *intel_dig_port,
>  	}
>  
>  	/*
> -	 * Since Aksv is Oh-So-Secret, we can't access it in software. So in
> -	 * order to get it on the wire, we need to create the AUX header as if
> -	 * we were writing the data, and then tickle the hardware to output the
> -	 * data once the header is sent out.
> +	 * Since Aksv is Oh-So-Secret, we can't access it in software. So we
> +	 * send an empty buffer of the correct length through the DP helpers. On
> +	 * the other side, in the transfer hook, we'll generate a flag based on
> +	 * the destination address which will tickle the hardware to output the
> +	 * Aksv on our behalf after the header is sent.
>  	 */
> -	intel_dp_aux_header(txbuf, &msg);
> -
> -	ret = intel_dp_aux_xfer(intel_dp, txbuf, HEADER_SIZE + msg.size,
> -				rxbuf, sizeof(rxbuf),
> -				DP_AUX_CH_CTL_AUX_AKSV_SELECT);
> -	if (ret < 0) {
> -		DRM_DEBUG_KMS("Write Aksv over DP/AUX failed (%d)\n", ret);
> -		return ret;
> -	} else if (ret == 0) {
> -		DRM_DEBUG_KMS("Aksv write over DP/AUX was empty\n");
> -		return -EIO;
> -	}
> -
> -	reply = (rxbuf[0] >> 4) & DP_AUX_NATIVE_REPLY_MASK;
> -	if (reply != DP_AUX_NATIVE_REPLY_ACK) {
> -		DRM_DEBUG_KMS("Aksv write: no DP_AUX_NATIVE_REPLY_ACK %x\n",
> -			      reply);
> -		return -EIO;
> +	dpcd_ret = drm_dp_dpcd_write(&intel_dig_port->dp.aux, DP_AUX_HDCP_AKSV,
> +				     txbuf, DRM_HDCP_KSV_LEN);
> +	if (dpcd_ret != DRM_HDCP_KSV_LEN) {
> +		DRM_DEBUG_KMS("Failed to write Aksv over DP/AUX (%zd)\n",
> +			      dpcd_ret);
> +		return dpcd_ret >= 0 ? -EIO : dpcd_ret;
>  	}
>  	return 0;
>  }
> -- 
> Sean Paul, Software Engineer, Google / Chromium OS
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[PATCH 03/11] drm/i915: Disable HDCP signalling on transcoder disable

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

Currently we rely on intel_hdcp_disable() to disable HDCP signalling in
the DDI Function Control register. This patch adds a safety net by also
clearing the bit when we disable the transcoder.

Once we have HDCP over MST and disappearing connectors, we want to make
sure that the signalling is truly disabled even if HDCP teardown doesn't
go as planned.

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/i915/display/intel_ddi.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index b51f244ad7a5..e8ac98a8ee7f 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -1952,13 +1952,12 @@ void intel_ddi_disable_transcoder_func(const struct intel_crtc_state *crtc_state
 	i915_reg_t reg = TRANS_DDI_FUNC_CTL(cpu_transcoder);
 	u32 val = I915_READ(reg);
 
-	if (INTEL_GEN(dev_priv) >= 12) {
-		val &= ~(TRANS_DDI_FUNC_ENABLE | TGL_TRANS_DDI_PORT_MASK |
-			 TRANS_DDI_DP_VC_PAYLOAD_ALLOC);
-	} else {
-		val &= ~(TRANS_DDI_FUNC_ENABLE | TRANS_DDI_PORT_MASK |
-			 TRANS_DDI_DP_VC_PAYLOAD_ALLOC);
-	}
+	val &= ~(TRANS_DDI_FUNC_ENABLE | TRANS_DDI_DP_VC_PAYLOAD_ALLOC |
+		 TRANS_DDI_HDCP_SIGNALLING);
+	if (INTEL_GEN(dev_priv) >= 12)
+		val &= ~TGL_TRANS_DDI_PORT_MASK;
+	else
+		val &= ~TRANS_DDI_PORT_MASK;
 	I915_WRITE(reg, val);
 
 	if (dev_priv->quirks & QUIRK_INCREASE_DDI_DISABLED_TIME &&
-- 
Sean Paul, Software Engineer, Google / Chromium OS

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

[Intel-gfx] [PATCH 03/11] drm/i915: Disable HDCP signalling on transcoder disable

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

Currently we rely on intel_hdcp_disable() to disable HDCP signalling in
the DDI Function Control register. This patch adds a safety net by also
clearing the bit when we disable the transcoder.

Once we have HDCP over MST and disappearing connectors, we want to make
sure that the signalling is truly disabled even if HDCP teardown doesn't
go as planned.

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/i915/display/intel_ddi.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index b51f244ad7a5..e8ac98a8ee7f 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -1952,13 +1952,12 @@ void intel_ddi_disable_transcoder_func(const struct intel_crtc_state *crtc_state
 	i915_reg_t reg = TRANS_DDI_FUNC_CTL(cpu_transcoder);
 	u32 val = I915_READ(reg);
 
-	if (INTEL_GEN(dev_priv) >= 12) {
-		val &= ~(TRANS_DDI_FUNC_ENABLE | TGL_TRANS_DDI_PORT_MASK |
-			 TRANS_DDI_DP_VC_PAYLOAD_ALLOC);
-	} else {
-		val &= ~(TRANS_DDI_FUNC_ENABLE | TRANS_DDI_PORT_MASK |
-			 TRANS_DDI_DP_VC_PAYLOAD_ALLOC);
-	}
+	val &= ~(TRANS_DDI_FUNC_ENABLE | TRANS_DDI_DP_VC_PAYLOAD_ALLOC |
+		 TRANS_DDI_HDCP_SIGNALLING);
+	if (INTEL_GEN(dev_priv) >= 12)
+		val &= ~TGL_TRANS_DDI_PORT_MASK;
+	else
+		val &= ~TRANS_DDI_PORT_MASK;
 	I915_WRITE(reg, val);
 
 	if (dev_priv->quirks & QUIRK_INCREASE_DDI_DISABLED_TIME &&
-- 
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: [Intel-gfx] [PATCH 03/11] drm/i915: Disable HDCP signalling on transcoder disable

[Ville Syrjälä]

On Tue, Dec 03, 2019 at 12:36:26PM -0500, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> Currently we rely on intel_hdcp_disable() to disable HDCP signalling in
> the DDI Function Control register. This patch adds a safety net by also
> clearing the bit when we disable the transcoder.
> 
> Once we have HDCP over MST and disappearing connectors, we want to make
> sure that the signalling is truly disabled even if HDCP teardown doesn't
> go as planned.

Why wouldn't it go as planned?

> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index b51f244ad7a5..e8ac98a8ee7f 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -1952,13 +1952,12 @@ void intel_ddi_disable_transcoder_func(const struct intel_crtc_state *crtc_state
>  	i915_reg_t reg = TRANS_DDI_FUNC_CTL(cpu_transcoder);
>  	u32 val = I915_READ(reg);
>  
> -	if (INTEL_GEN(dev_priv) >= 12) {
> -		val &= ~(TRANS_DDI_FUNC_ENABLE | TGL_TRANS_DDI_PORT_MASK |
> -			 TRANS_DDI_DP_VC_PAYLOAD_ALLOC);
> -	} else {
> -		val &= ~(TRANS_DDI_FUNC_ENABLE | TRANS_DDI_PORT_MASK |
> -			 TRANS_DDI_DP_VC_PAYLOAD_ALLOC);
> -	}
> +	val &= ~(TRANS_DDI_FUNC_ENABLE | TRANS_DDI_DP_VC_PAYLOAD_ALLOC |
> +		 TRANS_DDI_HDCP_SIGNALLING);
> +	if (INTEL_GEN(dev_priv) >= 12)
> +		val &= ~TGL_TRANS_DDI_PORT_MASK;
> +	else
> +		val &= ~TRANS_DDI_PORT_MASK;
>  	I915_WRITE(reg, val);
>  
>  	if (dev_priv->quirks & QUIRK_INCREASE_DDI_DISABLED_TIME &&
> -- 
> Sean Paul, Software Engineer, Google / Chromium OS
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [Intel-gfx] [PATCH 03/11] drm/i915: Disable HDCP signalling on transcoder disable

[Ville Syrjälä]

On Tue, Dec 03, 2019 at 12:36:26PM -0500, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> Currently we rely on intel_hdcp_disable() to disable HDCP signalling in
> the DDI Function Control register. This patch adds a safety net by also
> clearing the bit when we disable the transcoder.
> 
> Once we have HDCP over MST and disappearing connectors, we want to make
> sure that the signalling is truly disabled even if HDCP teardown doesn't
> go as planned.

Why wouldn't it go as planned?

> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index b51f244ad7a5..e8ac98a8ee7f 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -1952,13 +1952,12 @@ void intel_ddi_disable_transcoder_func(const struct intel_crtc_state *crtc_state
>  	i915_reg_t reg = TRANS_DDI_FUNC_CTL(cpu_transcoder);
>  	u32 val = I915_READ(reg);
>  
> -	if (INTEL_GEN(dev_priv) >= 12) {
> -		val &= ~(TRANS_DDI_FUNC_ENABLE | TGL_TRANS_DDI_PORT_MASK |
> -			 TRANS_DDI_DP_VC_PAYLOAD_ALLOC);
> -	} else {
> -		val &= ~(TRANS_DDI_FUNC_ENABLE | TRANS_DDI_PORT_MASK |
> -			 TRANS_DDI_DP_VC_PAYLOAD_ALLOC);
> -	}
> +	val &= ~(TRANS_DDI_FUNC_ENABLE | TRANS_DDI_DP_VC_PAYLOAD_ALLOC |
> +		 TRANS_DDI_HDCP_SIGNALLING);
> +	if (INTEL_GEN(dev_priv) >= 12)
> +		val &= ~TGL_TRANS_DDI_PORT_MASK;
> +	else
> +		val &= ~TRANS_DDI_PORT_MASK;
>  	I915_WRITE(reg, val);
>  
>  	if (dev_priv->quirks & QUIRK_INCREASE_DDI_DISABLED_TIME &&
> -- 
> Sean Paul, Software Engineer, Google / Chromium OS
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [Intel-gfx] [PATCH 03/11] drm/i915: Disable HDCP signalling on transcoder disable

[Sean Paul]

On Thu, Dec 05, 2019 at 09:33:19PM +0200, Ville Syrjälä wrote:
> On Tue, Dec 03, 2019 at 12:36:26PM -0500, Sean Paul wrote:
> > From: Sean Paul <seanpaul@chromium.org>
> > 
> > Currently we rely on intel_hdcp_disable() to disable HDCP signalling in
> > the DDI Function Control register. This patch adds a safety net by also
> > clearing the bit when we disable the transcoder.
> > 
> > Once we have HDCP over MST and disappearing connectors, we want to make
> > sure that the signalling is truly disabled even if HDCP teardown doesn't
> > go as planned.
> 
> Why wouldn't it go as planned?
> 

Because things can fail in weird and wonderful ways on unplug :-)

It's a safety net. I saw this function and figured HDCP signalling should be
explicitly cleared here as well.

Sean

> > 
> > Signed-off-by: Sean Paul <seanpaul@chromium.org>
> > ---
> >  drivers/gpu/drm/i915/display/intel_ddi.c | 13 ++++++-------
> >  1 file changed, 6 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index b51f244ad7a5..e8ac98a8ee7f 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -1952,13 +1952,12 @@ void intel_ddi_disable_transcoder_func(const struct intel_crtc_state *crtc_state
> >  	i915_reg_t reg = TRANS_DDI_FUNC_CTL(cpu_transcoder);
> >  	u32 val = I915_READ(reg);
> >  
> > -	if (INTEL_GEN(dev_priv) >= 12) {
> > -		val &= ~(TRANS_DDI_FUNC_ENABLE | TGL_TRANS_DDI_PORT_MASK |
> > -			 TRANS_DDI_DP_VC_PAYLOAD_ALLOC);
> > -	} else {
> > -		val &= ~(TRANS_DDI_FUNC_ENABLE | TRANS_DDI_PORT_MASK |
> > -			 TRANS_DDI_DP_VC_PAYLOAD_ALLOC);
> > -	}
> > +	val &= ~(TRANS_DDI_FUNC_ENABLE | TRANS_DDI_DP_VC_PAYLOAD_ALLOC |
> > +		 TRANS_DDI_HDCP_SIGNALLING);
> > +	if (INTEL_GEN(dev_priv) >= 12)
> > +		val &= ~TGL_TRANS_DDI_PORT_MASK;
> > +	else
> > +		val &= ~TRANS_DDI_PORT_MASK;
> >  	I915_WRITE(reg, val);
> >  
> >  	if (dev_priv->quirks & QUIRK_INCREASE_DDI_DISABLED_TIME &&
> > -- 
> > Sean Paul, Software Engineer, Google / Chromium OS
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel

-- 
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: [Intel-gfx] [PATCH 03/11] drm/i915: Disable HDCP signalling on transcoder disable

[Sean Paul]

On Thu, Dec 05, 2019 at 09:33:19PM +0200, Ville Syrjälä wrote:
> On Tue, Dec 03, 2019 at 12:36:26PM -0500, Sean Paul wrote:
> > From: Sean Paul <seanpaul@chromium.org>
> > 
> > Currently we rely on intel_hdcp_disable() to disable HDCP signalling in
> > the DDI Function Control register. This patch adds a safety net by also
> > clearing the bit when we disable the transcoder.
> > 
> > Once we have HDCP over MST and disappearing connectors, we want to make
> > sure that the signalling is truly disabled even if HDCP teardown doesn't
> > go as planned.
> 
> Why wouldn't it go as planned?
> 

Because things can fail in weird and wonderful ways on unplug :-)

It's a safety net. I saw this function and figured HDCP signalling should be
explicitly cleared here as well.

Sean

> > 
> > Signed-off-by: Sean Paul <seanpaul@chromium.org>
> > ---
> >  drivers/gpu/drm/i915/display/intel_ddi.c | 13 ++++++-------
> >  1 file changed, 6 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index b51f244ad7a5..e8ac98a8ee7f 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -1952,13 +1952,12 @@ void intel_ddi_disable_transcoder_func(const struct intel_crtc_state *crtc_state
> >  	i915_reg_t reg = TRANS_DDI_FUNC_CTL(cpu_transcoder);
> >  	u32 val = I915_READ(reg);
> >  
> > -	if (INTEL_GEN(dev_priv) >= 12) {
> > -		val &= ~(TRANS_DDI_FUNC_ENABLE | TGL_TRANS_DDI_PORT_MASK |
> > -			 TRANS_DDI_DP_VC_PAYLOAD_ALLOC);
> > -	} else {
> > -		val &= ~(TRANS_DDI_FUNC_ENABLE | TRANS_DDI_PORT_MASK |
> > -			 TRANS_DDI_DP_VC_PAYLOAD_ALLOC);
> > -	}
> > +	val &= ~(TRANS_DDI_FUNC_ENABLE | TRANS_DDI_DP_VC_PAYLOAD_ALLOC |
> > +		 TRANS_DDI_HDCP_SIGNALLING);
> > +	if (INTEL_GEN(dev_priv) >= 12)
> > +		val &= ~TGL_TRANS_DDI_PORT_MASK;
> > +	else
> > +		val &= ~TRANS_DDI_PORT_MASK;
> >  	I915_WRITE(reg, val);
> >  
> >  	if (dev_priv->quirks & QUIRK_INCREASE_DDI_DISABLED_TIME &&
> > -- 
> > Sean Paul, Software Engineer, Google / Chromium OS
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel

-- 
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: [Intel-gfx] [PATCH 03/11] drm/i915: Disable HDCP signalling on transcoder disable

[Ville Syrjälä]

On Fri, Dec 06, 2019 at 08:55:09AM -0500, Sean Paul wrote:
> On Thu, Dec 05, 2019 at 09:33:19PM +0200, Ville Syrjälä wrote:
> > On Tue, Dec 03, 2019 at 12:36:26PM -0500, Sean Paul wrote:
> > > From: Sean Paul <seanpaul@chromium.org>
> > > 
> > > Currently we rely on intel_hdcp_disable() to disable HDCP signalling in
> > > the DDI Function Control register. This patch adds a safety net by also
> > > clearing the bit when we disable the transcoder.
> > > 
> > > Once we have HDCP over MST and disappearing connectors, we want to make
> > > sure that the signalling is truly disabled even if HDCP teardown doesn't
> > > go as planned.
> > 
> > Why wouldn't it go as planned?
> > 
> 
> Because things can fail in weird and wonderful ways on unplug :-)

Not really.

> 
> It's a safety net. I saw this function and figured HDCP signalling should be
> explicitly cleared here as well.

I call it dead and confusing code. If we get here with HDCP still
enabled we have a more serious bug somewhere else.

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

Re: [Intel-gfx] [PATCH 03/11] drm/i915: Disable HDCP signalling on transcoder disable

[Ville Syrjälä]

On Fri, Dec 06, 2019 at 08:55:09AM -0500, Sean Paul wrote:
> On Thu, Dec 05, 2019 at 09:33:19PM +0200, Ville Syrjälä wrote:
> > On Tue, Dec 03, 2019 at 12:36:26PM -0500, Sean Paul wrote:
> > > From: Sean Paul <seanpaul@chromium.org>
> > > 
> > > Currently we rely on intel_hdcp_disable() to disable HDCP signalling in
> > > the DDI Function Control register. This patch adds a safety net by also
> > > clearing the bit when we disable the transcoder.
> > > 
> > > Once we have HDCP over MST and disappearing connectors, we want to make
> > > sure that the signalling is truly disabled even if HDCP teardown doesn't
> > > go as planned.
> > 
> > Why wouldn't it go as planned?
> > 
> 
> Because things can fail in weird and wonderful ways on unplug :-)

Not really.

> 
> It's a safety net. I saw this function and figured HDCP signalling should be
> explicitly cleared here as well.

I call it dead and confusing code. If we get here with HDCP still
enabled we have a more serious bug somewhere else.

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

Re: [Intel-gfx] [PATCH 03/11] drm/i915: Disable HDCP signalling on transcoder disable

[Sean Paul]

On Mon, Dec 9, 2019 at 10:18 AM Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
>
> On Fri, Dec 06, 2019 at 08:55:09AM -0500, Sean Paul wrote:
> > On Thu, Dec 05, 2019 at 09:33:19PM +0200, Ville Syrjälä wrote:
> > > On Tue, Dec 03, 2019 at 12:36:26PM -0500, Sean Paul wrote:
> > > > From: Sean Paul <seanpaul@chromium.org>
> > > >
> > > > Currently we rely on intel_hdcp_disable() to disable HDCP signalling in
> > > > the DDI Function Control register. This patch adds a safety net by also
> > > > clearing the bit when we disable the transcoder.
> > > >
> > > > Once we have HDCP over MST and disappearing connectors, we want to make
> > > > sure that the signalling is truly disabled even if HDCP teardown doesn't
> > > > go as planned.
> > >
> > > Why wouldn't it go as planned?
> > >
> >
> > Because things can fail in weird and wonderful ways on unplug :-)
>
> Not really.
>

That is a bold position to take, bugs happen, hardware flakes, etc.

> >
> > It's a safety net. I saw this function and figured HDCP signalling should be
> > explicitly cleared here as well.
>
> I call it dead and confusing code.

...adding a bit to an existing register clear is confusing? That might
be a touch hyperbolic.

> If we get here with HDCP still
> enabled we have a more serious bug somewhere else.
>

Ok, I suppose it's your call as to whether you take this patch, feel
free to drop.

Sean

> --
> Ville Syrjälä
> Intel
> _______________________________________________
> 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: [Intel-gfx] [PATCH 03/11] drm/i915: Disable HDCP signalling on transcoder disable

[Sean Paul]

On Mon, Dec 9, 2019 at 10:18 AM Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
>
> On Fri, Dec 06, 2019 at 08:55:09AM -0500, Sean Paul wrote:
> > On Thu, Dec 05, 2019 at 09:33:19PM +0200, Ville Syrjälä wrote:
> > > On Tue, Dec 03, 2019 at 12:36:26PM -0500, Sean Paul wrote:
> > > > From: Sean Paul <seanpaul@chromium.org>
> > > >
> > > > Currently we rely on intel_hdcp_disable() to disable HDCP signalling in
> > > > the DDI Function Control register. This patch adds a safety net by also
> > > > clearing the bit when we disable the transcoder.
> > > >
> > > > Once we have HDCP over MST and disappearing connectors, we want to make
> > > > sure that the signalling is truly disabled even if HDCP teardown doesn't
> > > > go as planned.
> > >
> > > Why wouldn't it go as planned?
> > >
> >
> > Because things can fail in weird and wonderful ways on unplug :-)
>
> Not really.
>

That is a bold position to take, bugs happen, hardware flakes, etc.

> >
> > It's a safety net. I saw this function and figured HDCP signalling should be
> > explicitly cleared here as well.
>
> I call it dead and confusing code.

...adding a bit to an existing register clear is confusing? That might
be a touch hyperbolic.

> If we get here with HDCP still
> enabled we have a more serious bug somewhere else.
>

Ok, I suppose it's your call as to whether you take this patch, feel
free to drop.

Sean

> --
> Ville Syrjälä
> Intel
> _______________________________________________
> 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

Re: [Intel-gfx] [PATCH 03/11] drm/i915: Disable HDCP signalling on transcoder disable

[Daniel Vetter]

On Mon, Dec 09, 2019 at 11:13:36AM -0500, Sean Paul wrote:
> On Mon, Dec 9, 2019 at 10:18 AM Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
> >
> > On Fri, Dec 06, 2019 at 08:55:09AM -0500, Sean Paul wrote:
> > > On Thu, Dec 05, 2019 at 09:33:19PM +0200, Ville Syrjälä wrote:
> > > > On Tue, Dec 03, 2019 at 12:36:26PM -0500, Sean Paul wrote:
> > > > > From: Sean Paul <seanpaul@chromium.org>
> > > > >
> > > > > Currently we rely on intel_hdcp_disable() to disable HDCP signalling in
> > > > > the DDI Function Control register. This patch adds a safety net by also
> > > > > clearing the bit when we disable the transcoder.
> > > > >
> > > > > Once we have HDCP over MST and disappearing connectors, we want to make
> > > > > sure that the signalling is truly disabled even if HDCP teardown doesn't
> > > > > go as planned.
> > > >
> > > > Why wouldn't it go as planned?
> > > >
> > >
> > > Because things can fail in weird and wonderful ways on unplug :-)
> >
> > Not really.
> >
> 
> That is a bold position to take, bugs happen, hardware flakes, etc.
> 
> > >
> > > It's a safety net. I saw this function and figured HDCP signalling should be
> > > explicitly cleared here as well.
> >
> > I call it dead and confusing code.
> 
> ...adding a bit to an existing register clear is confusing? That might
> be a touch hyperbolic.
> 
> > If we get here with HDCP still
> > enabled we have a more serious bug somewhere else.
> >
> 
> Ok, I suppose it's your call as to whether you take this patch, feel
> free to drop.

Maybe some expansion on this discussion here. We've had super-defensive
modeset code back 10 years ago when i915 started.

It was a mess, since all that "for safety" thing papered over real bugs,
except thanks to the safety net you mostly couldn't observe machines dying
for real. That's why we've gone pretty radical towards "you better know
what state your hw is in".

If you do want safatey, add a WARN_ON or similar that reads back hw state
and double checks it is what we think it should be. That's much better for
validating your driver than papering over all kinds of busg preemptively
and making the real ones very hard to track down.

tldr; WARN_ON hw/sw consistency safety checks good, "let me reclear/set
this" safety code bad. At least if it doesn't come with a huge WARN_ON
that things have gone terribly wrong so we can actually catch bugs in CI
and testing.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH 03/11] drm/i915: Disable HDCP signalling on transcoder disable

[Daniel Vetter]

On Mon, Dec 09, 2019 at 11:13:36AM -0500, Sean Paul wrote:
> On Mon, Dec 9, 2019 at 10:18 AM Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
> >
> > On Fri, Dec 06, 2019 at 08:55:09AM -0500, Sean Paul wrote:
> > > On Thu, Dec 05, 2019 at 09:33:19PM +0200, Ville Syrjälä wrote:
> > > > On Tue, Dec 03, 2019 at 12:36:26PM -0500, Sean Paul wrote:
> > > > > From: Sean Paul <seanpaul@chromium.org>
> > > > >
> > > > > Currently we rely on intel_hdcp_disable() to disable HDCP signalling in
> > > > > the DDI Function Control register. This patch adds a safety net by also
> > > > > clearing the bit when we disable the transcoder.
> > > > >
> > > > > Once we have HDCP over MST and disappearing connectors, we want to make
> > > > > sure that the signalling is truly disabled even if HDCP teardown doesn't
> > > > > go as planned.
> > > >
> > > > Why wouldn't it go as planned?
> > > >
> > >
> > > Because things can fail in weird and wonderful ways on unplug :-)
> >
> > Not really.
> >
> 
> That is a bold position to take, bugs happen, hardware flakes, etc.
> 
> > >
> > > It's a safety net. I saw this function and figured HDCP signalling should be
> > > explicitly cleared here as well.
> >
> > I call it dead and confusing code.
> 
> ...adding a bit to an existing register clear is confusing? That might
> be a touch hyperbolic.
> 
> > If we get here with HDCP still
> > enabled we have a more serious bug somewhere else.
> >
> 
> Ok, I suppose it's your call as to whether you take this patch, feel
> free to drop.

Maybe some expansion on this discussion here. We've had super-defensive
modeset code back 10 years ago when i915 started.

It was a mess, since all that "for safety" thing papered over real bugs,
except thanks to the safety net you mostly couldn't observe machines dying
for real. That's why we've gone pretty radical towards "you better know
what state your hw is in".

If you do want safatey, add a WARN_ON or similar that reads back hw state
and double checks it is what we think it should be. That's much better for
validating your driver than papering over all kinds of busg preemptively
and making the real ones very hard to track down.

tldr; WARN_ON hw/sw consistency safety checks good, "let me reclear/set
this" safety code bad. At least if it doesn't come with a huge WARN_ON
that things have gone terribly wrong so we can actually catch bugs in CI
and testing.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 04/11] drm/i915: Don't WARN on HDCP toggle if get_hw_state returns false

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

Now that we can rely on transcoder disable to toggle signalling off,
it's less of a catastrophe if get_hw_state() returns false.

Once we enable MST, this will be a valid exit path and we want to make
sure we're not spamming the logs needlessly.

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/i915/display/intel_ddi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index e8ac98a8ee7f..ca28913a4c9f 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -1983,7 +1983,7 @@ int intel_ddi_toggle_hdcp_signalling(struct intel_encoder *intel_encoder,
 	if (WARN_ON(!wakeref))
 		return -ENXIO;
 
-	if (WARN_ON(!intel_encoder->get_hw_state(intel_encoder, &pipe))) {
+	if (!intel_encoder->get_hw_state(intel_encoder, &pipe)) {
 		ret = -EIO;
 		goto out;
 	}
-- 
Sean Paul, Software Engineer, Google / Chromium OS

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

[Intel-gfx] [PATCH 04/11] drm/i915: Don't WARN on HDCP toggle if get_hw_state returns false

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

Now that we can rely on transcoder disable to toggle signalling off,
it's less of a catastrophe if get_hw_state() returns false.

Once we enable MST, this will be a valid exit path and we want to make
sure we're not spamming the logs needlessly.

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/i915/display/intel_ddi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index e8ac98a8ee7f..ca28913a4c9f 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -1983,7 +1983,7 @@ int intel_ddi_toggle_hdcp_signalling(struct intel_encoder *intel_encoder,
 	if (WARN_ON(!wakeref))
 		return -ENXIO;
 
-	if (WARN_ON(!intel_encoder->get_hw_state(intel_encoder, &pipe))) {
+	if (!intel_encoder->get_hw_state(intel_encoder, &pipe)) {
 		ret = -EIO;
 		goto out;
 	}
-- 
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 04/11] drm/i915: Don't WARN on HDCP toggle if get_hw_state returns false

[Ville Syrjälä]

On Tue, Dec 03, 2019 at 12:36:27PM -0500, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> Now that we can rely on transcoder disable to toggle signalling off,
> it's less of a catastrophe if get_hw_state() returns false.
> 
> Once we enable MST, this will be a valid exit path and we want to make
> sure we're not spamming the logs needlessly.
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index e8ac98a8ee7f..ca28913a4c9f 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -1983,7 +1983,7 @@ int intel_ddi_toggle_hdcp_signalling(struct intel_encoder *intel_encoder,
>  	if (WARN_ON(!wakeref))
>  		return -ENXIO;
>  
> -	if (WARN_ON(!intel_encoder->get_hw_state(intel_encoder, &pipe))) {
> +	if (!intel_encoder->get_hw_state(intel_encoder, &pipe)) {

How can this get called when the encoder is not enabled?
Feels like this whole thing is trying to paper over some
bigger bug in the hdcp code.

>  		ret = -EIO;
>  		goto out;
>  	}
> -- 
> Sean Paul, Software Engineer, Google / Chromium OS
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

Re: [Intel-gfx] [PATCH 04/11] drm/i915: Don't WARN on HDCP toggle if get_hw_state returns false

[Ville Syrjälä]

On Tue, Dec 03, 2019 at 12:36:27PM -0500, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> Now that we can rely on transcoder disable to toggle signalling off,
> it's less of a catastrophe if get_hw_state() returns false.
> 
> Once we enable MST, this will be a valid exit path and we want to make
> sure we're not spamming the logs needlessly.
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index e8ac98a8ee7f..ca28913a4c9f 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -1983,7 +1983,7 @@ int intel_ddi_toggle_hdcp_signalling(struct intel_encoder *intel_encoder,
>  	if (WARN_ON(!wakeref))
>  		return -ENXIO;
>  
> -	if (WARN_ON(!intel_encoder->get_hw_state(intel_encoder, &pipe))) {
> +	if (!intel_encoder->get_hw_state(intel_encoder, &pipe)) {

How can this get called when the encoder is not enabled?
Feels like this whole thing is trying to paper over some
bigger bug in the hdcp code.

>  		ret = -EIO;
>  		goto out;
>  	}
> -- 
> Sean Paul, Software Engineer, Google / Chromium OS
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

Re: [PATCH 04/11] drm/i915: Don't WARN on HDCP toggle if get_hw_state returns false

[Sean Paul]

On Thu, Dec 05, 2019 at 09:39:35PM +0200, Ville Syrjälä wrote:
> On Tue, Dec 03, 2019 at 12:36:27PM -0500, Sean Paul wrote:
> > From: Sean Paul <seanpaul@chromium.org>
> > 
> > Now that we can rely on transcoder disable to toggle signalling off,
> > it's less of a catastrophe if get_hw_state() returns false.
> > 
> > Once we enable MST, this will be a valid exit path and we want to make
> > sure we're not spamming the logs needlessly.
> > 
> > Signed-off-by: Sean Paul <seanpaul@chromium.org>
> > ---
> >  drivers/gpu/drm/i915/display/intel_ddi.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index e8ac98a8ee7f..ca28913a4c9f 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -1983,7 +1983,7 @@ int intel_ddi_toggle_hdcp_signalling(struct intel_encoder *intel_encoder,
> >  	if (WARN_ON(!wakeref))
> >  		return -ENXIO;
> >  
> > -	if (WARN_ON(!intel_encoder->get_hw_state(intel_encoder, &pipe))) {
> > +	if (!intel_encoder->get_hw_state(intel_encoder, &pipe)) {
> 
> How can this get called when the encoder is not enabled?
> Feels like this whole thing is trying to paper over some
> bigger bug in the hdcp code.

In the MST patch, I've added a call to intel_hdcp_disable() in the connector
destroy path. Usually toggling will be disabled as part of the check_link call
that is initiated on unplug, so in the destroy path it's non-essential to do
this again.

Sean

> 
> >  		ret = -EIO;
> >  		goto out;
> >  	}
> > -- 
> > Sean Paul, Software Engineer, Google / Chromium OS
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Ville Syrjälä
> Intel

-- 
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: [Intel-gfx] [PATCH 04/11] drm/i915: Don't WARN on HDCP toggle if get_hw_state returns false

[Sean Paul]

On Thu, Dec 05, 2019 at 09:39:35PM +0200, Ville Syrjälä wrote:
> On Tue, Dec 03, 2019 at 12:36:27PM -0500, Sean Paul wrote:
> > From: Sean Paul <seanpaul@chromium.org>
> > 
> > Now that we can rely on transcoder disable to toggle signalling off,
> > it's less of a catastrophe if get_hw_state() returns false.
> > 
> > Once we enable MST, this will be a valid exit path and we want to make
> > sure we're not spamming the logs needlessly.
> > 
> > Signed-off-by: Sean Paul <seanpaul@chromium.org>
> > ---
> >  drivers/gpu/drm/i915/display/intel_ddi.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index e8ac98a8ee7f..ca28913a4c9f 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -1983,7 +1983,7 @@ int intel_ddi_toggle_hdcp_signalling(struct intel_encoder *intel_encoder,
> >  	if (WARN_ON(!wakeref))
> >  		return -ENXIO;
> >  
> > -	if (WARN_ON(!intel_encoder->get_hw_state(intel_encoder, &pipe))) {
> > +	if (!intel_encoder->get_hw_state(intel_encoder, &pipe)) {
> 
> How can this get called when the encoder is not enabled?
> Feels like this whole thing is trying to paper over some
> bigger bug in the hdcp code.

In the MST patch, I've added a call to intel_hdcp_disable() in the connector
destroy path. Usually toggling will be disabled as part of the check_link call
that is initiated on unplug, so in the destroy path it's non-essential to do
this again.

Sean

> 
> >  		ret = -EIO;
> >  		goto out;
> >  	}
> > -- 
> > Sean Paul, Software Engineer, Google / Chromium OS
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Ville Syrjälä
> Intel

-- 
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 04/11] drm/i915: Don't WARN on HDCP toggle if get_hw_state returns false

[Ville Syrjälä]

On Fri, Dec 06, 2019 at 08:52:33AM -0500, Sean Paul wrote:
> On Thu, Dec 05, 2019 at 09:39:35PM +0200, Ville Syrjälä wrote:
> > On Tue, Dec 03, 2019 at 12:36:27PM -0500, Sean Paul wrote:
> > > From: Sean Paul <seanpaul@chromium.org>
> > > 
> > > Now that we can rely on transcoder disable to toggle signalling off,
> > > it's less of a catastrophe if get_hw_state() returns false.
> > > 
> > > Once we enable MST, this will be a valid exit path and we want to make
> > > sure we're not spamming the logs needlessly.
> > > 
> > > Signed-off-by: Sean Paul <seanpaul@chromium.org>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_ddi.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > index e8ac98a8ee7f..ca28913a4c9f 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > @@ -1983,7 +1983,7 @@ int intel_ddi_toggle_hdcp_signalling(struct intel_encoder *intel_encoder,
> > >  	if (WARN_ON(!wakeref))
> > >  		return -ENXIO;
> > >  
> > > -	if (WARN_ON(!intel_encoder->get_hw_state(intel_encoder, &pipe))) {
> > > +	if (!intel_encoder->get_hw_state(intel_encoder, &pipe)) {
> > 
> > How can this get called when the encoder is not enabled?
> > Feels like this whole thing is trying to paper over some
> > bigger bug in the hdcp code.
> 
> In the MST patch, I've added a call to intel_hdcp_disable() in the connector
> destroy path. Usually toggling will be disabled as part of the check_link call
> that is initiated on unplug, so in the destroy path it's non-essential to do
> this again.

Can't we just leave things be until userspace disables the thing?
If not, then we should know whether hdcp is still enabled. And if
hdcp is enabled so is the encoder, thus we don't need such silly
checks.

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

Re: [Intel-gfx] [PATCH 04/11] drm/i915: Don't WARN on HDCP toggle if get_hw_state returns false

[Ville Syrjälä]

On Fri, Dec 06, 2019 at 08:52:33AM -0500, Sean Paul wrote:
> On Thu, Dec 05, 2019 at 09:39:35PM +0200, Ville Syrjälä wrote:
> > On Tue, Dec 03, 2019 at 12:36:27PM -0500, Sean Paul wrote:
> > > From: Sean Paul <seanpaul@chromium.org>
> > > 
> > > Now that we can rely on transcoder disable to toggle signalling off,
> > > it's less of a catastrophe if get_hw_state() returns false.
> > > 
> > > Once we enable MST, this will be a valid exit path and we want to make
> > > sure we're not spamming the logs needlessly.
> > > 
> > > Signed-off-by: Sean Paul <seanpaul@chromium.org>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_ddi.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > index e8ac98a8ee7f..ca28913a4c9f 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > @@ -1983,7 +1983,7 @@ int intel_ddi_toggle_hdcp_signalling(struct intel_encoder *intel_encoder,
> > >  	if (WARN_ON(!wakeref))
> > >  		return -ENXIO;
> > >  
> > > -	if (WARN_ON(!intel_encoder->get_hw_state(intel_encoder, &pipe))) {
> > > +	if (!intel_encoder->get_hw_state(intel_encoder, &pipe)) {
> > 
> > How can this get called when the encoder is not enabled?
> > Feels like this whole thing is trying to paper over some
> > bigger bug in the hdcp code.
> 
> In the MST patch, I've added a call to intel_hdcp_disable() in the connector
> destroy path. Usually toggling will be disabled as part of the check_link call
> that is initiated on unplug, so in the destroy path it's non-essential to do
> this again.

Can't we just leave things be until userspace disables the thing?
If not, then we should know whether hdcp is still enabled. And if
hdcp is enabled so is the encoder, thus we don't need such silly
checks.

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

Re: [PATCH 04/11] drm/i915: Don't WARN on HDCP toggle if get_hw_state returns false

[Sean Paul]

On Mon, Dec 9, 2019 at 10:21 AM Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
>
> On Fri, Dec 06, 2019 at 08:52:33AM -0500, Sean Paul wrote:
> > On Thu, Dec 05, 2019 at 09:39:35PM +0200, Ville Syrjälä wrote:
> > > On Tue, Dec 03, 2019 at 12:36:27PM -0500, Sean Paul wrote:
> > > > From: Sean Paul <seanpaul@chromium.org>
> > > >
> > > > Now that we can rely on transcoder disable to toggle signalling off,
> > > > it's less of a catastrophe if get_hw_state() returns false.
> > > >
> > > > Once we enable MST, this will be a valid exit path and we want to make
> > > > sure we're not spamming the logs needlessly.
> > > >
> > > > Signed-off-by: Sean Paul <seanpaul@chromium.org>
> > > > ---
> > > >  drivers/gpu/drm/i915/display/intel_ddi.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > > index e8ac98a8ee7f..ca28913a4c9f 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > > @@ -1983,7 +1983,7 @@ int intel_ddi_toggle_hdcp_signalling(struct intel_encoder *intel_encoder,
> > > >   if (WARN_ON(!wakeref))
> > > >           return -ENXIO;
> > > >
> > > > - if (WARN_ON(!intel_encoder->get_hw_state(intel_encoder, &pipe))) {
> > > > + if (!intel_encoder->get_hw_state(intel_encoder, &pipe)) {
> > >
> > > How can this get called when the encoder is not enabled?
> > > Feels like this whole thing is trying to paper over some
> > > bigger bug in the hdcp code.
> >
> > In the MST patch, I've added a call to intel_hdcp_disable() in the connector
> > destroy path. Usually toggling will be disabled as part of the check_link call
> > that is initiated on unplug, so in the destroy path it's non-essential to do
> > this again.
>
> Can't we just leave things be until userspace disables the thing?

The connector disappears, so userspace won't be able to disable it.

> If not, then we should know whether hdcp is still enabled. And if
> hdcp is enabled so is the encoder, thus we don't need such silly
> checks.

Alright, I'll look at taking this angle.

Sean

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

Re: [Intel-gfx] [PATCH 04/11] drm/i915: Don't WARN on HDCP toggle if get_hw_state returns false

[Sean Paul]

On Mon, Dec 9, 2019 at 10:21 AM Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
>
> On Fri, Dec 06, 2019 at 08:52:33AM -0500, Sean Paul wrote:
> > On Thu, Dec 05, 2019 at 09:39:35PM +0200, Ville Syrjälä wrote:
> > > On Tue, Dec 03, 2019 at 12:36:27PM -0500, Sean Paul wrote:
> > > > From: Sean Paul <seanpaul@chromium.org>
> > > >
> > > > Now that we can rely on transcoder disable to toggle signalling off,
> > > > it's less of a catastrophe if get_hw_state() returns false.
> > > >
> > > > Once we enable MST, this will be a valid exit path and we want to make
> > > > sure we're not spamming the logs needlessly.
> > > >
> > > > Signed-off-by: Sean Paul <seanpaul@chromium.org>
> > > > ---
> > > >  drivers/gpu/drm/i915/display/intel_ddi.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > > index e8ac98a8ee7f..ca28913a4c9f 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > > @@ -1983,7 +1983,7 @@ int intel_ddi_toggle_hdcp_signalling(struct intel_encoder *intel_encoder,
> > > >   if (WARN_ON(!wakeref))
> > > >           return -ENXIO;
> > > >
> > > > - if (WARN_ON(!intel_encoder->get_hw_state(intel_encoder, &pipe))) {
> > > > + if (!intel_encoder->get_hw_state(intel_encoder, &pipe)) {
> > >
> > > How can this get called when the encoder is not enabled?
> > > Feels like this whole thing is trying to paper over some
> > > bigger bug in the hdcp code.
> >
> > In the MST patch, I've added a call to intel_hdcp_disable() in the connector
> > destroy path. Usually toggling will be disabled as part of the check_link call
> > that is initiated on unplug, so in the destroy path it's non-essential to do
> > this again.
>
> Can't we just leave things be until userspace disables the thing?

The connector disappears, so userspace won't be able to disable it.

> If not, then we should know whether hdcp is still enabled. And if
> hdcp is enabled so is the encoder, thus we don't need such silly
> checks.

Alright, I'll look at taking this angle.

Sean

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

Re: [PATCH 04/11] drm/i915: Don't WARN on HDCP toggle if get_hw_state returns false

[Ville Syrjälä]

On Mon, Dec 09, 2019 at 11:16:27AM -0500, Sean Paul wrote:
> On Mon, Dec 9, 2019 at 10:21 AM Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
> >
> > On Fri, Dec 06, 2019 at 08:52:33AM -0500, Sean Paul wrote:
> > > On Thu, Dec 05, 2019 at 09:39:35PM +0200, Ville Syrjälä wrote:
> > > > On Tue, Dec 03, 2019 at 12:36:27PM -0500, Sean Paul wrote:
> > > > > From: Sean Paul <seanpaul@chromium.org>
> > > > >
> > > > > Now that we can rely on transcoder disable to toggle signalling off,
> > > > > it's less of a catastrophe if get_hw_state() returns false.
> > > > >
> > > > > Once we enable MST, this will be a valid exit path and we want to make
> > > > > sure we're not spamming the logs needlessly.
> > > > >
> > > > > Signed-off-by: Sean Paul <seanpaul@chromium.org>
> > > > > ---
> > > > >  drivers/gpu/drm/i915/display/intel_ddi.c | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > > > index e8ac98a8ee7f..ca28913a4c9f 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > > > @@ -1983,7 +1983,7 @@ int intel_ddi_toggle_hdcp_signalling(struct intel_encoder *intel_encoder,
> > > > >   if (WARN_ON(!wakeref))
> > > > >           return -ENXIO;
> > > > >
> > > > > - if (WARN_ON(!intel_encoder->get_hw_state(intel_encoder, &pipe))) {
> > > > > + if (!intel_encoder->get_hw_state(intel_encoder, &pipe)) {
> > > >
> > > > How can this get called when the encoder is not enabled?
> > > > Feels like this whole thing is trying to paper over some
> > > > bigger bug in the hdcp code.
> > >
> > > In the MST patch, I've added a call to intel_hdcp_disable() in the connector
> > > destroy path. Usually toggling will be disabled as part of the check_link call
> > > that is initiated on unplug, so in the destroy path it's non-essential to do
> > > this again.
> >
> > Can't we just leave things be until userspace disables the thing?
> 
> The connector disappears, so userspace won't be able to disable it.

That would make everything broken. The connector should hang around
as a zombie until the last user disappears.

> 
> > If not, then we should know whether hdcp is still enabled. And if
> > hdcp is enabled so is the encoder, thus we don't need such silly
> > checks.
> 
> Alright, I'll look at taking this angle.
> 
> Sean
> 
> >
> > --
> > Ville Syrjälä
> > Intel

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

Re: [Intel-gfx] [PATCH 04/11] drm/i915: Don't WARN on HDCP toggle if get_hw_state returns false

[Ville Syrjälä]

On Mon, Dec 09, 2019 at 11:16:27AM -0500, Sean Paul wrote:
> On Mon, Dec 9, 2019 at 10:21 AM Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
> >
> > On Fri, Dec 06, 2019 at 08:52:33AM -0500, Sean Paul wrote:
> > > On Thu, Dec 05, 2019 at 09:39:35PM +0200, Ville Syrjälä wrote:
> > > > On Tue, Dec 03, 2019 at 12:36:27PM -0500, Sean Paul wrote:
> > > > > From: Sean Paul <seanpaul@chromium.org>
> > > > >
> > > > > Now that we can rely on transcoder disable to toggle signalling off,
> > > > > it's less of a catastrophe if get_hw_state() returns false.
> > > > >
> > > > > Once we enable MST, this will be a valid exit path and we want to make
> > > > > sure we're not spamming the logs needlessly.
> > > > >
> > > > > Signed-off-by: Sean Paul <seanpaul@chromium.org>
> > > > > ---
> > > > >  drivers/gpu/drm/i915/display/intel_ddi.c | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > > > index e8ac98a8ee7f..ca28913a4c9f 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > > > @@ -1983,7 +1983,7 @@ int intel_ddi_toggle_hdcp_signalling(struct intel_encoder *intel_encoder,
> > > > >   if (WARN_ON(!wakeref))
> > > > >           return -ENXIO;
> > > > >
> > > > > - if (WARN_ON(!intel_encoder->get_hw_state(intel_encoder, &pipe))) {
> > > > > + if (!intel_encoder->get_hw_state(intel_encoder, &pipe)) {
> > > >
> > > > How can this get called when the encoder is not enabled?
> > > > Feels like this whole thing is trying to paper over some
> > > > bigger bug in the hdcp code.
> > >
> > > In the MST patch, I've added a call to intel_hdcp_disable() in the connector
> > > destroy path. Usually toggling will be disabled as part of the check_link call
> > > that is initiated on unplug, so in the destroy path it's non-essential to do
> > > this again.
> >
> > Can't we just leave things be until userspace disables the thing?
> 
> The connector disappears, so userspace won't be able to disable it.

That would make everything broken. The connector should hang around
as a zombie until the last user disappears.

> 
> > If not, then we should know whether hdcp is still enabled. And if
> > hdcp is enabled so is the encoder, thus we don't need such silly
> > checks.
> 
> Alright, I'll look at taking this angle.
> 
> Sean
> 
> >
> > --
> > Ville Syrjälä
> > Intel

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

[PATCH 05/11] drm/i915: Change toggle_signalling() argument to connector

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

HDCP over MST requires us to toggle ddi signalling. Since we'll want to
toggle signalling on the pipe associated with the fake encoder as
opposed to the digital port's base, we need to get it from connector.

This patch converts all existing callers and implementations to use
connector instead of digital port.

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/i915/display/intel_display_types.h |  2 +-
 drivers/gpu/drm/i915/display/intel_dp.c            |  2 +-
 drivers/gpu/drm/i915/display/intel_hdcp.c          | 10 ++++------
 drivers/gpu/drm/i915/display/intel_hdmi.c          |  8 ++++----
 4 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 4341bd66a418..bbd44772b9b0 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -300,7 +300,7 @@ struct intel_hdcp_shim {
 				 int i, u32 *part);
 
 	/* Enables HDCP signalling on the port */
-	int (*toggle_signalling)(struct intel_digital_port *intel_dig_port,
+	int (*toggle_signalling)(struct intel_connector *connector,
 				 bool enable);
 
 	/* Ensures the link is still protected */
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 7a407c651fb2..e26fb26b1909 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -6038,7 +6038,7 @@ int intel_dp_hdcp_read_v_prime_part(struct intel_digital_port *intel_dig_port,
 }
 
 static
-int intel_dp_hdcp_toggle_signalling(struct intel_digital_port *intel_dig_port,
+int intel_dp_hdcp_toggle_signalling(struct intel_connector *connector,
 				    bool enable)
 {
 	/* Not used for single stream DisplayPort setups */
diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
index 8325bf9501e4..0966a8ec47d2 100644
--- a/drivers/gpu/drm/i915/display/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
@@ -693,7 +693,7 @@ static int intel_hdcp_auth(struct intel_connector *connector)
 			   intel_hdcp_get_repeater_ctl(dev_priv, cpu_transcoder,
 						       port));
 
-	ret = shim->toggle_signalling(intel_dig_port, true);
+	ret = shim->toggle_signalling(connector, true);
 	if (ret)
 		return ret;
 
@@ -787,7 +787,7 @@ static int _intel_hdcp_disable(struct intel_connector *connector)
 		return -ETIMEDOUT;
 	}
 
-	ret = hdcp->shim->toggle_signalling(intel_dig_port, false);
+	ret = hdcp->shim->toggle_signalling(connector, false);
 	if (ret) {
 		DRM_ERROR("Failed to disable HDCP signalling\n");
 		return ret;
@@ -1537,7 +1537,6 @@ static int hdcp2_authenticate_sink(struct intel_connector *connector)
 
 static int hdcp2_enable_encryption(struct intel_connector *connector)
 {
-	struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector);
 	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
 	struct intel_hdcp *hdcp = &connector->hdcp;
 	enum port port = connector->encoder->port;
@@ -1547,7 +1546,7 @@ static int hdcp2_enable_encryption(struct intel_connector *connector)
 	WARN_ON(I915_READ(HDCP2_STATUS(dev_priv, cpu_transcoder, port)) &
 		LINK_ENCRYPTION_STATUS);
 	if (hdcp->shim->toggle_signalling) {
-		ret = hdcp->shim->toggle_signalling(intel_dig_port, true);
+		ret = hdcp->shim->toggle_signalling(connector, true);
 		if (ret) {
 			DRM_ERROR("Failed to enable HDCP signalling. %d\n",
 				  ret);
@@ -1575,7 +1574,6 @@ static int hdcp2_enable_encryption(struct intel_connector *connector)
 
 static int hdcp2_disable_encryption(struct intel_connector *connector)
 {
-	struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector);
 	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
 	struct intel_hdcp *hdcp = &connector->hdcp;
 	enum port port = connector->encoder->port;
@@ -1598,7 +1596,7 @@ static int hdcp2_disable_encryption(struct intel_connector *connector)
 		DRM_DEBUG_KMS("Disable Encryption Timedout");
 
 	if (hdcp->shim->toggle_signalling) {
-		ret = hdcp->shim->toggle_signalling(intel_dig_port, false);
+		ret = hdcp->shim->toggle_signalling(connector, false);
 		if (ret) {
 			DRM_ERROR("Failed to disable HDCP signalling. %d\n",
 				  ret);
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index f6f5312205c4..6a1e711c4f7a 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -1475,18 +1475,18 @@ static int kbl_repositioning_enc_en_signal(struct intel_connector *connector)
 }
 
 static
-int intel_hdmi_hdcp_toggle_signalling(struct intel_digital_port *intel_dig_port,
+int intel_hdmi_hdcp_toggle_signalling(struct intel_connector *connector,
 				      bool enable)
 {
-	struct intel_hdmi *hdmi = &intel_dig_port->hdmi;
-	struct intel_connector *connector = hdmi->attached_connector;
+	struct intel_encoder *encoder =
+				intel_attached_encoder(&connector->base);
 	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
 	int ret;
 
 	if (!enable)
 		usleep_range(6, 60); /* Bspec says >= 6us */
 
-	ret = intel_ddi_toggle_hdcp_signalling(&intel_dig_port->base, enable);
+	ret = intel_ddi_toggle_hdcp_signalling(encoder, enable);
 	if (ret) {
 		DRM_ERROR("%s HDCP signalling failed (%d)\n",
 			  enable ? "Enable" : "Disable", ret);
-- 
Sean Paul, Software Engineer, Google / Chromium OS

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

[Intel-gfx] [PATCH 05/11] drm/i915: Change toggle_signalling() argument to connector

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

HDCP over MST requires us to toggle ddi signalling. Since we'll want to
toggle signalling on the pipe associated with the fake encoder as
opposed to the digital port's base, we need to get it from connector.

This patch converts all existing callers and implementations to use
connector instead of digital port.

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/i915/display/intel_display_types.h |  2 +-
 drivers/gpu/drm/i915/display/intel_dp.c            |  2 +-
 drivers/gpu/drm/i915/display/intel_hdcp.c          | 10 ++++------
 drivers/gpu/drm/i915/display/intel_hdmi.c          |  8 ++++----
 4 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 4341bd66a418..bbd44772b9b0 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -300,7 +300,7 @@ struct intel_hdcp_shim {
 				 int i, u32 *part);
 
 	/* Enables HDCP signalling on the port */
-	int (*toggle_signalling)(struct intel_digital_port *intel_dig_port,
+	int (*toggle_signalling)(struct intel_connector *connector,
 				 bool enable);
 
 	/* Ensures the link is still protected */
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 7a407c651fb2..e26fb26b1909 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -6038,7 +6038,7 @@ int intel_dp_hdcp_read_v_prime_part(struct intel_digital_port *intel_dig_port,
 }
 
 static
-int intel_dp_hdcp_toggle_signalling(struct intel_digital_port *intel_dig_port,
+int intel_dp_hdcp_toggle_signalling(struct intel_connector *connector,
 				    bool enable)
 {
 	/* Not used for single stream DisplayPort setups */
diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
index 8325bf9501e4..0966a8ec47d2 100644
--- a/drivers/gpu/drm/i915/display/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
@@ -693,7 +693,7 @@ static int intel_hdcp_auth(struct intel_connector *connector)
 			   intel_hdcp_get_repeater_ctl(dev_priv, cpu_transcoder,
 						       port));
 
-	ret = shim->toggle_signalling(intel_dig_port, true);
+	ret = shim->toggle_signalling(connector, true);
 	if (ret)
 		return ret;
 
@@ -787,7 +787,7 @@ static int _intel_hdcp_disable(struct intel_connector *connector)
 		return -ETIMEDOUT;
 	}
 
-	ret = hdcp->shim->toggle_signalling(intel_dig_port, false);
+	ret = hdcp->shim->toggle_signalling(connector, false);
 	if (ret) {
 		DRM_ERROR("Failed to disable HDCP signalling\n");
 		return ret;
@@ -1537,7 +1537,6 @@ static int hdcp2_authenticate_sink(struct intel_connector *connector)
 
 static int hdcp2_enable_encryption(struct intel_connector *connector)
 {
-	struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector);
 	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
 	struct intel_hdcp *hdcp = &connector->hdcp;
 	enum port port = connector->encoder->port;
@@ -1547,7 +1546,7 @@ static int hdcp2_enable_encryption(struct intel_connector *connector)
 	WARN_ON(I915_READ(HDCP2_STATUS(dev_priv, cpu_transcoder, port)) &
 		LINK_ENCRYPTION_STATUS);
 	if (hdcp->shim->toggle_signalling) {
-		ret = hdcp->shim->toggle_signalling(intel_dig_port, true);
+		ret = hdcp->shim->toggle_signalling(connector, true);
 		if (ret) {
 			DRM_ERROR("Failed to enable HDCP signalling. %d\n",
 				  ret);
@@ -1575,7 +1574,6 @@ static int hdcp2_enable_encryption(struct intel_connector *connector)
 
 static int hdcp2_disable_encryption(struct intel_connector *connector)
 {
-	struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector);
 	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
 	struct intel_hdcp *hdcp = &connector->hdcp;
 	enum port port = connector->encoder->port;
@@ -1598,7 +1596,7 @@ static int hdcp2_disable_encryption(struct intel_connector *connector)
 		DRM_DEBUG_KMS("Disable Encryption Timedout");
 
 	if (hdcp->shim->toggle_signalling) {
-		ret = hdcp->shim->toggle_signalling(intel_dig_port, false);
+		ret = hdcp->shim->toggle_signalling(connector, false);
 		if (ret) {
 			DRM_ERROR("Failed to disable HDCP signalling. %d\n",
 				  ret);
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index f6f5312205c4..6a1e711c4f7a 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -1475,18 +1475,18 @@ static int kbl_repositioning_enc_en_signal(struct intel_connector *connector)
 }
 
 static
-int intel_hdmi_hdcp_toggle_signalling(struct intel_digital_port *intel_dig_port,
+int intel_hdmi_hdcp_toggle_signalling(struct intel_connector *connector,
 				      bool enable)
 {
-	struct intel_hdmi *hdmi = &intel_dig_port->hdmi;
-	struct intel_connector *connector = hdmi->attached_connector;
+	struct intel_encoder *encoder =
+				intel_attached_encoder(&connector->base);
 	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
 	int ret;
 
 	if (!enable)
 		usleep_range(6, 60); /* Bspec says >= 6us */
 
-	ret = intel_ddi_toggle_hdcp_signalling(&intel_dig_port->base, enable);
+	ret = intel_ddi_toggle_hdcp_signalling(encoder, enable);
 	if (ret) {
 		DRM_ERROR("%s HDCP signalling failed (%d)\n",
 			  enable ? "Enable" : "Disable", ret);
-- 
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 05/11] drm/i915: Change toggle_signalling() argument to connector

[Ville Syrjälä]

On Tue, Dec 03, 2019 at 12:36:28PM -0500, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> HDCP over MST requires us to toggle ddi signalling. Since we'll want to
> toggle signalling on the pipe associated with the fake encoder as
> opposed to the digital port's base, we need to get it from connector.

I think what you want is the cpu_transcoder, which we've already stuffed
into the hdcp thing when we enabled hdcp.

> 
> This patch converts all existing callers and implementations to use
> connector instead of digital port.
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
>  drivers/gpu/drm/i915/display/intel_display_types.h |  2 +-
>  drivers/gpu/drm/i915/display/intel_dp.c            |  2 +-
>  drivers/gpu/drm/i915/display/intel_hdcp.c          | 10 ++++------
>  drivers/gpu/drm/i915/display/intel_hdmi.c          |  8 ++++----
>  4 files changed, 10 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 4341bd66a418..bbd44772b9b0 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -300,7 +300,7 @@ struct intel_hdcp_shim {
>  				 int i, u32 *part);
>  
>  	/* Enables HDCP signalling on the port */
> -	int (*toggle_signalling)(struct intel_digital_port *intel_dig_port,
> +	int (*toggle_signalling)(struct intel_connector *connector,
>  				 bool enable);
>  
>  	/* Ensures the link is still protected */
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 7a407c651fb2..e26fb26b1909 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -6038,7 +6038,7 @@ int intel_dp_hdcp_read_v_prime_part(struct intel_digital_port *intel_dig_port,
>  }
>  
>  static
> -int intel_dp_hdcp_toggle_signalling(struct intel_digital_port *intel_dig_port,
> +int intel_dp_hdcp_toggle_signalling(struct intel_connector *connector,
>  				    bool enable)
>  {
>  	/* Not used for single stream DisplayPort setups */
> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
> index 8325bf9501e4..0966a8ec47d2 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> @@ -693,7 +693,7 @@ static int intel_hdcp_auth(struct intel_connector *connector)
>  			   intel_hdcp_get_repeater_ctl(dev_priv, cpu_transcoder,
>  						       port));
>  
> -	ret = shim->toggle_signalling(intel_dig_port, true);
> +	ret = shim->toggle_signalling(connector, true);
>  	if (ret)
>  		return ret;
>  
> @@ -787,7 +787,7 @@ static int _intel_hdcp_disable(struct intel_connector *connector)
>  		return -ETIMEDOUT;
>  	}
>  
> -	ret = hdcp->shim->toggle_signalling(intel_dig_port, false);
> +	ret = hdcp->shim->toggle_signalling(connector, false);
>  	if (ret) {
>  		DRM_ERROR("Failed to disable HDCP signalling\n");
>  		return ret;
> @@ -1537,7 +1537,6 @@ static int hdcp2_authenticate_sink(struct intel_connector *connector)
>  
>  static int hdcp2_enable_encryption(struct intel_connector *connector)
>  {
> -	struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector);
>  	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>  	struct intel_hdcp *hdcp = &connector->hdcp;
>  	enum port port = connector->encoder->port;
> @@ -1547,7 +1546,7 @@ static int hdcp2_enable_encryption(struct intel_connector *connector)
>  	WARN_ON(I915_READ(HDCP2_STATUS(dev_priv, cpu_transcoder, port)) &
>  		LINK_ENCRYPTION_STATUS);
>  	if (hdcp->shim->toggle_signalling) {
> -		ret = hdcp->shim->toggle_signalling(intel_dig_port, true);
> +		ret = hdcp->shim->toggle_signalling(connector, true);
>  		if (ret) {
>  			DRM_ERROR("Failed to enable HDCP signalling. %d\n",
>  				  ret);
> @@ -1575,7 +1574,6 @@ static int hdcp2_enable_encryption(struct intel_connector *connector)
>  
>  static int hdcp2_disable_encryption(struct intel_connector *connector)
>  {
> -	struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector);
>  	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>  	struct intel_hdcp *hdcp = &connector->hdcp;
>  	enum port port = connector->encoder->port;
> @@ -1598,7 +1596,7 @@ static int hdcp2_disable_encryption(struct intel_connector *connector)
>  		DRM_DEBUG_KMS("Disable Encryption Timedout");
>  
>  	if (hdcp->shim->toggle_signalling) {
> -		ret = hdcp->shim->toggle_signalling(intel_dig_port, false);
> +		ret = hdcp->shim->toggle_signalling(connector, false);
>  		if (ret) {
>  			DRM_ERROR("Failed to disable HDCP signalling. %d\n",
>  				  ret);
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index f6f5312205c4..6a1e711c4f7a 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -1475,18 +1475,18 @@ static int kbl_repositioning_enc_en_signal(struct intel_connector *connector)
>  }
>  
>  static
> -int intel_hdmi_hdcp_toggle_signalling(struct intel_digital_port *intel_dig_port,
> +int intel_hdmi_hdcp_toggle_signalling(struct intel_connector *connector,
>  				      bool enable)
>  {
> -	struct intel_hdmi *hdmi = &intel_dig_port->hdmi;
> -	struct intel_connector *connector = hdmi->attached_connector;
> +	struct intel_encoder *encoder =
> +				intel_attached_encoder(&connector->base);
>  	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>  	int ret;
>  
>  	if (!enable)
>  		usleep_range(6, 60); /* Bspec says >= 6us */
>  
> -	ret = intel_ddi_toggle_hdcp_signalling(&intel_dig_port->base, enable);
> +	ret = intel_ddi_toggle_hdcp_signalling(encoder, enable);
>  	if (ret) {
>  		DRM_ERROR("%s HDCP signalling failed (%d)\n",
>  			  enable ? "Enable" : "Disable", ret);
> -- 
> Sean Paul, Software Engineer, Google / Chromium OS
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

Re: [Intel-gfx] [PATCH 05/11] drm/i915: Change toggle_signalling() argument to connector

[Ville Syrjälä]

On Tue, Dec 03, 2019 at 12:36:28PM -0500, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> HDCP over MST requires us to toggle ddi signalling. Since we'll want to
> toggle signalling on the pipe associated with the fake encoder as
> opposed to the digital port's base, we need to get it from connector.

I think what you want is the cpu_transcoder, which we've already stuffed
into the hdcp thing when we enabled hdcp.

> 
> This patch converts all existing callers and implementations to use
> connector instead of digital port.
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
>  drivers/gpu/drm/i915/display/intel_display_types.h |  2 +-
>  drivers/gpu/drm/i915/display/intel_dp.c            |  2 +-
>  drivers/gpu/drm/i915/display/intel_hdcp.c          | 10 ++++------
>  drivers/gpu/drm/i915/display/intel_hdmi.c          |  8 ++++----
>  4 files changed, 10 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 4341bd66a418..bbd44772b9b0 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -300,7 +300,7 @@ struct intel_hdcp_shim {
>  				 int i, u32 *part);
>  
>  	/* Enables HDCP signalling on the port */
> -	int (*toggle_signalling)(struct intel_digital_port *intel_dig_port,
> +	int (*toggle_signalling)(struct intel_connector *connector,
>  				 bool enable);
>  
>  	/* Ensures the link is still protected */
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 7a407c651fb2..e26fb26b1909 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -6038,7 +6038,7 @@ int intel_dp_hdcp_read_v_prime_part(struct intel_digital_port *intel_dig_port,
>  }
>  
>  static
> -int intel_dp_hdcp_toggle_signalling(struct intel_digital_port *intel_dig_port,
> +int intel_dp_hdcp_toggle_signalling(struct intel_connector *connector,
>  				    bool enable)
>  {
>  	/* Not used for single stream DisplayPort setups */
> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
> index 8325bf9501e4..0966a8ec47d2 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> @@ -693,7 +693,7 @@ static int intel_hdcp_auth(struct intel_connector *connector)
>  			   intel_hdcp_get_repeater_ctl(dev_priv, cpu_transcoder,
>  						       port));
>  
> -	ret = shim->toggle_signalling(intel_dig_port, true);
> +	ret = shim->toggle_signalling(connector, true);
>  	if (ret)
>  		return ret;
>  
> @@ -787,7 +787,7 @@ static int _intel_hdcp_disable(struct intel_connector *connector)
>  		return -ETIMEDOUT;
>  	}
>  
> -	ret = hdcp->shim->toggle_signalling(intel_dig_port, false);
> +	ret = hdcp->shim->toggle_signalling(connector, false);
>  	if (ret) {
>  		DRM_ERROR("Failed to disable HDCP signalling\n");
>  		return ret;
> @@ -1537,7 +1537,6 @@ static int hdcp2_authenticate_sink(struct intel_connector *connector)
>  
>  static int hdcp2_enable_encryption(struct intel_connector *connector)
>  {
> -	struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector);
>  	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>  	struct intel_hdcp *hdcp = &connector->hdcp;
>  	enum port port = connector->encoder->port;
> @@ -1547,7 +1546,7 @@ static int hdcp2_enable_encryption(struct intel_connector *connector)
>  	WARN_ON(I915_READ(HDCP2_STATUS(dev_priv, cpu_transcoder, port)) &
>  		LINK_ENCRYPTION_STATUS);
>  	if (hdcp->shim->toggle_signalling) {
> -		ret = hdcp->shim->toggle_signalling(intel_dig_port, true);
> +		ret = hdcp->shim->toggle_signalling(connector, true);
>  		if (ret) {
>  			DRM_ERROR("Failed to enable HDCP signalling. %d\n",
>  				  ret);
> @@ -1575,7 +1574,6 @@ static int hdcp2_enable_encryption(struct intel_connector *connector)
>  
>  static int hdcp2_disable_encryption(struct intel_connector *connector)
>  {
> -	struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector);
>  	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>  	struct intel_hdcp *hdcp = &connector->hdcp;
>  	enum port port = connector->encoder->port;
> @@ -1598,7 +1596,7 @@ static int hdcp2_disable_encryption(struct intel_connector *connector)
>  		DRM_DEBUG_KMS("Disable Encryption Timedout");
>  
>  	if (hdcp->shim->toggle_signalling) {
> -		ret = hdcp->shim->toggle_signalling(intel_dig_port, false);
> +		ret = hdcp->shim->toggle_signalling(connector, false);
>  		if (ret) {
>  			DRM_ERROR("Failed to disable HDCP signalling. %d\n",
>  				  ret);
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index f6f5312205c4..6a1e711c4f7a 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -1475,18 +1475,18 @@ static int kbl_repositioning_enc_en_signal(struct intel_connector *connector)
>  }
>  
>  static
> -int intel_hdmi_hdcp_toggle_signalling(struct intel_digital_port *intel_dig_port,
> +int intel_hdmi_hdcp_toggle_signalling(struct intel_connector *connector,
>  				      bool enable)
>  {
> -	struct intel_hdmi *hdmi = &intel_dig_port->hdmi;
> -	struct intel_connector *connector = hdmi->attached_connector;
> +	struct intel_encoder *encoder =
> +				intel_attached_encoder(&connector->base);
>  	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>  	int ret;
>  
>  	if (!enable)
>  		usleep_range(6, 60); /* Bspec says >= 6us */
>  
> -	ret = intel_ddi_toggle_hdcp_signalling(&intel_dig_port->base, enable);
> +	ret = intel_ddi_toggle_hdcp_signalling(encoder, enable);
>  	if (ret) {
>  		DRM_ERROR("%s HDCP signalling failed (%d)\n",
>  			  enable ? "Enable" : "Disable", ret);
> -- 
> Sean Paul, Software Engineer, Google / Chromium OS
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[PATCH 06/11] drm/i915: Factor out hdcp->value assignments

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

This is a bit of housecleaning for a future patch. Instead of sprinkling
hdcp->value assignments and prop_work scheduling everywhere, introduce a
function to do it for us.

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/i915/display/intel_hdcp.c | 67 ++++++++++++++++-------
 1 file changed, 46 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
index 0966a8ec47d2..f34763fa5b42 100644
--- a/drivers/gpu/drm/i915/display/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
@@ -846,6 +846,21 @@ struct intel_connector *intel_hdcp_to_connector(struct intel_hdcp *hdcp)
 	return container_of(hdcp, struct intel_connector, hdcp);
 }
 
+static void intel_hdcp_update_value(struct intel_connector *connector,
+				    u64 value, bool update_property)
+{
+	struct intel_hdcp *hdcp = &connector->hdcp;
+
+	WARN_ON(!mutex_is_locked(&hdcp->mutex));
+
+	if (hdcp->value == value)
+		return;
+
+	hdcp->value = value;
+	if (update_property)
+		schedule_work(&hdcp->prop_work);
+}
+
 /* Implements Part 3 of the HDCP authorization procedure */
 static int intel_hdcp_check_link(struct intel_connector *connector)
 {
@@ -872,15 +887,16 @@ static int intel_hdcp_check_link(struct intel_connector *connector)
 			  I915_READ(HDCP_STATUS(dev_priv, cpu_transcoder,
 						port)));
 		ret = -ENXIO;
-		hdcp->value = DRM_MODE_CONTENT_PROTECTION_DESIRED;
-		schedule_work(&hdcp->prop_work);
+		intel_hdcp_update_value(connector,
+					DRM_MODE_CONTENT_PROTECTION_DESIRED,
+					true);
 		goto out;
 	}
 
 	if (hdcp->shim->check_link(intel_dig_port)) {
 		if (hdcp->value != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
-			hdcp->value = DRM_MODE_CONTENT_PROTECTION_ENABLED;
-			schedule_work(&hdcp->prop_work);
+			intel_hdcp_update_value(connector,
+				DRM_MODE_CONTENT_PROTECTION_ENABLED, true);
 		}
 		goto out;
 	}
@@ -891,16 +907,18 @@ static int intel_hdcp_check_link(struct intel_connector *connector)
 	ret = _intel_hdcp_disable(connector);
 	if (ret) {
 		DRM_ERROR("Failed to disable hdcp (%d)\n", ret);
-		hdcp->value = DRM_MODE_CONTENT_PROTECTION_DESIRED;
-		schedule_work(&hdcp->prop_work);
+		intel_hdcp_update_value(connector,
+					DRM_MODE_CONTENT_PROTECTION_DESIRED,
+					true);
 		goto out;
 	}
 
 	ret = _intel_hdcp_enable(connector);
 	if (ret) {
 		DRM_ERROR("Failed to enable hdcp (%d)\n", ret);
-		hdcp->value = DRM_MODE_CONTENT_PROTECTION_DESIRED;
-		schedule_work(&hdcp->prop_work);
+		intel_hdcp_update_value(connector,
+					DRM_MODE_CONTENT_PROTECTION_DESIRED,
+					true);
 		goto out;
 	}
 
@@ -1706,16 +1724,18 @@ static int intel_hdcp2_check_link(struct intel_connector *connector)
 			  I915_READ(HDCP2_STATUS(dev_priv, cpu_transcoder,
 						 port)));
 		ret = -ENXIO;
-		hdcp->value = DRM_MODE_CONTENT_PROTECTION_DESIRED;
-		schedule_work(&hdcp->prop_work);
+		intel_hdcp_update_value(connector,
+					DRM_MODE_CONTENT_PROTECTION_DESIRED,
+					true);
 		goto out;
 	}
 
 	ret = hdcp->shim->check_2_2_link(intel_dig_port);
 	if (ret == HDCP_LINK_PROTECTED) {
 		if (hdcp->value != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
-			hdcp->value = DRM_MODE_CONTENT_PROTECTION_ENABLED;
-			schedule_work(&hdcp->prop_work);
+			intel_hdcp_update_value(connector,
+					DRM_MODE_CONTENT_PROTECTION_ENABLED,
+					true);
 		}
 		goto out;
 	}
@@ -1727,8 +1747,9 @@ static int intel_hdcp2_check_link(struct intel_connector *connector)
 		DRM_DEBUG_KMS("HDCP2.2 Downstream topology change\n");
 		ret = hdcp2_authenticate_repeater_topology(connector);
 		if (!ret) {
-			hdcp->value = DRM_MODE_CONTENT_PROTECTION_ENABLED;
-			schedule_work(&hdcp->prop_work);
+			intel_hdcp_update_value(connector,
+					DRM_MODE_CONTENT_PROTECTION_ENABLED,
+					true);
 			goto out;
 		}
 		DRM_DEBUG_KMS("[%s:%d] Repeater topology auth failed.(%d)\n",
@@ -1743,8 +1764,8 @@ static int intel_hdcp2_check_link(struct intel_connector *connector)
 	if (ret) {
 		DRM_ERROR("[%s:%d] Failed to disable hdcp2.2 (%d)\n",
 			  connector->base.name, connector->base.base.id, ret);
-		hdcp->value = DRM_MODE_CONTENT_PROTECTION_DESIRED;
-		schedule_work(&hdcp->prop_work);
+		intel_hdcp_update_value(connector,
+				DRM_MODE_CONTENT_PROTECTION_DESIRED, true);
 		goto out;
 	}
 
@@ -1753,8 +1774,9 @@ static int intel_hdcp2_check_link(struct intel_connector *connector)
 		DRM_DEBUG_KMS("[%s:%d] Failed to enable hdcp2.2 (%d)\n",
 			      connector->base.name, connector->base.base.id,
 			      ret);
-		hdcp->value = DRM_MODE_CONTENT_PROTECTION_DESIRED;
-		schedule_work(&hdcp->prop_work);
+		intel_hdcp_update_value(connector,
+					DRM_MODE_CONTENT_PROTECTION_DESIRED,
+					true);
 		goto out;
 	}
 
@@ -2004,8 +2026,9 @@ int intel_hdcp_enable(struct intel_connector *connector, u8 content_type)
 
 	if (!ret) {
 		schedule_delayed_work(&hdcp->check_work, check_link_interval);
-		hdcp->value = DRM_MODE_CONTENT_PROTECTION_ENABLED;
-		schedule_work(&hdcp->prop_work);
+		intel_hdcp_update_value(connector,
+					DRM_MODE_CONTENT_PROTECTION_ENABLED,
+					true);
 	}
 
 	mutex_unlock(&hdcp->mutex);
@@ -2023,7 +2046,9 @@ int intel_hdcp_disable(struct intel_connector *connector)
 	mutex_lock(&hdcp->mutex);
 
 	if (hdcp->value != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
-		hdcp->value = DRM_MODE_CONTENT_PROTECTION_UNDESIRED;
+		intel_hdcp_update_value(connector,
+					DRM_MODE_CONTENT_PROTECTION_UNDESIRED,
+					false);
 		if (hdcp->hdcp2_encrypted)
 			ret = _intel_hdcp2_disable(connector);
 		else if (hdcp->hdcp_encrypted)
-- 
Sean Paul, Software Engineer, Google / Chromium OS

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

[Intel-gfx] [PATCH 06/11] drm/i915: Factor out hdcp->value assignments

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

This is a bit of housecleaning for a future patch. Instead of sprinkling
hdcp->value assignments and prop_work scheduling everywhere, introduce a
function to do it for us.

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/i915/display/intel_hdcp.c | 67 ++++++++++++++++-------
 1 file changed, 46 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
index 0966a8ec47d2..f34763fa5b42 100644
--- a/drivers/gpu/drm/i915/display/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
@@ -846,6 +846,21 @@ struct intel_connector *intel_hdcp_to_connector(struct intel_hdcp *hdcp)
 	return container_of(hdcp, struct intel_connector, hdcp);
 }
 
+static void intel_hdcp_update_value(struct intel_connector *connector,
+				    u64 value, bool update_property)
+{
+	struct intel_hdcp *hdcp = &connector->hdcp;
+
+	WARN_ON(!mutex_is_locked(&hdcp->mutex));
+
+	if (hdcp->value == value)
+		return;
+
+	hdcp->value = value;
+	if (update_property)
+		schedule_work(&hdcp->prop_work);
+}
+
 /* Implements Part 3 of the HDCP authorization procedure */
 static int intel_hdcp_check_link(struct intel_connector *connector)
 {
@@ -872,15 +887,16 @@ static int intel_hdcp_check_link(struct intel_connector *connector)
 			  I915_READ(HDCP_STATUS(dev_priv, cpu_transcoder,
 						port)));
 		ret = -ENXIO;
-		hdcp->value = DRM_MODE_CONTENT_PROTECTION_DESIRED;
-		schedule_work(&hdcp->prop_work);
+		intel_hdcp_update_value(connector,
+					DRM_MODE_CONTENT_PROTECTION_DESIRED,
+					true);
 		goto out;
 	}
 
 	if (hdcp->shim->check_link(intel_dig_port)) {
 		if (hdcp->value != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
-			hdcp->value = DRM_MODE_CONTENT_PROTECTION_ENABLED;
-			schedule_work(&hdcp->prop_work);
+			intel_hdcp_update_value(connector,
+				DRM_MODE_CONTENT_PROTECTION_ENABLED, true);
 		}
 		goto out;
 	}
@@ -891,16 +907,18 @@ static int intel_hdcp_check_link(struct intel_connector *connector)
 	ret = _intel_hdcp_disable(connector);
 	if (ret) {
 		DRM_ERROR("Failed to disable hdcp (%d)\n", ret);
-		hdcp->value = DRM_MODE_CONTENT_PROTECTION_DESIRED;
-		schedule_work(&hdcp->prop_work);
+		intel_hdcp_update_value(connector,
+					DRM_MODE_CONTENT_PROTECTION_DESIRED,
+					true);
 		goto out;
 	}
 
 	ret = _intel_hdcp_enable(connector);
 	if (ret) {
 		DRM_ERROR("Failed to enable hdcp (%d)\n", ret);
-		hdcp->value = DRM_MODE_CONTENT_PROTECTION_DESIRED;
-		schedule_work(&hdcp->prop_work);
+		intel_hdcp_update_value(connector,
+					DRM_MODE_CONTENT_PROTECTION_DESIRED,
+					true);
 		goto out;
 	}
 
@@ -1706,16 +1724,18 @@ static int intel_hdcp2_check_link(struct intel_connector *connector)
 			  I915_READ(HDCP2_STATUS(dev_priv, cpu_transcoder,
 						 port)));
 		ret = -ENXIO;
-		hdcp->value = DRM_MODE_CONTENT_PROTECTION_DESIRED;
-		schedule_work(&hdcp->prop_work);
+		intel_hdcp_update_value(connector,
+					DRM_MODE_CONTENT_PROTECTION_DESIRED,
+					true);
 		goto out;
 	}
 
 	ret = hdcp->shim->check_2_2_link(intel_dig_port);
 	if (ret == HDCP_LINK_PROTECTED) {
 		if (hdcp->value != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
-			hdcp->value = DRM_MODE_CONTENT_PROTECTION_ENABLED;
-			schedule_work(&hdcp->prop_work);
+			intel_hdcp_update_value(connector,
+					DRM_MODE_CONTENT_PROTECTION_ENABLED,
+					true);
 		}
 		goto out;
 	}
@@ -1727,8 +1747,9 @@ static int intel_hdcp2_check_link(struct intel_connector *connector)
 		DRM_DEBUG_KMS("HDCP2.2 Downstream topology change\n");
 		ret = hdcp2_authenticate_repeater_topology(connector);
 		if (!ret) {
-			hdcp->value = DRM_MODE_CONTENT_PROTECTION_ENABLED;
-			schedule_work(&hdcp->prop_work);
+			intel_hdcp_update_value(connector,
+					DRM_MODE_CONTENT_PROTECTION_ENABLED,
+					true);
 			goto out;
 		}
 		DRM_DEBUG_KMS("[%s:%d] Repeater topology auth failed.(%d)\n",
@@ -1743,8 +1764,8 @@ static int intel_hdcp2_check_link(struct intel_connector *connector)
 	if (ret) {
 		DRM_ERROR("[%s:%d] Failed to disable hdcp2.2 (%d)\n",
 			  connector->base.name, connector->base.base.id, ret);
-		hdcp->value = DRM_MODE_CONTENT_PROTECTION_DESIRED;
-		schedule_work(&hdcp->prop_work);
+		intel_hdcp_update_value(connector,
+				DRM_MODE_CONTENT_PROTECTION_DESIRED, true);
 		goto out;
 	}
 
@@ -1753,8 +1774,9 @@ static int intel_hdcp2_check_link(struct intel_connector *connector)
 		DRM_DEBUG_KMS("[%s:%d] Failed to enable hdcp2.2 (%d)\n",
 			      connector->base.name, connector->base.base.id,
 			      ret);
-		hdcp->value = DRM_MODE_CONTENT_PROTECTION_DESIRED;
-		schedule_work(&hdcp->prop_work);
+		intel_hdcp_update_value(connector,
+					DRM_MODE_CONTENT_PROTECTION_DESIRED,
+					true);
 		goto out;
 	}
 
@@ -2004,8 +2026,9 @@ int intel_hdcp_enable(struct intel_connector *connector, u8 content_type)
 
 	if (!ret) {
 		schedule_delayed_work(&hdcp->check_work, check_link_interval);
-		hdcp->value = DRM_MODE_CONTENT_PROTECTION_ENABLED;
-		schedule_work(&hdcp->prop_work);
+		intel_hdcp_update_value(connector,
+					DRM_MODE_CONTENT_PROTECTION_ENABLED,
+					true);
 	}
 
 	mutex_unlock(&hdcp->mutex);
@@ -2023,7 +2046,9 @@ int intel_hdcp_disable(struct intel_connector *connector)
 	mutex_lock(&hdcp->mutex);
 
 	if (hdcp->value != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
-		hdcp->value = DRM_MODE_CONTENT_PROTECTION_UNDESIRED;
+		intel_hdcp_update_value(connector,
+					DRM_MODE_CONTENT_PROTECTION_UNDESIRED,
+					false);
 		if (hdcp->hdcp2_encrypted)
 			ret = _intel_hdcp2_disable(connector);
 		else if (hdcp->hdcp_encrypted)
-- 
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 07/11] drm/i915: Don't fully disable HDCP on a port if multiple pipes are using it

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

This patch is required for HDCP over MST. If a port is being used for
multiple HDCP streams, we don't want to fully disable HDCP on a port if
one of them is disabled. Instead, we just disable the HDCP signalling on
that particular pipe and exit early. The last pipe to disable HDCP will
also bring down HDCP on the port.

In order to achieve this, we need to keep a refcount in intel_digital_port
and protect it using a new hdcp_mutex.

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/i915/display/intel_ddi.c      |  3 ++
 .../drm/i915/display/intel_display_types.h    |  5 ++
 drivers/gpu/drm/i915/display/intel_dp.c       |  2 +
 drivers/gpu/drm/i915/display/intel_hdcp.c     | 49 ++++++++++++++++---
 drivers/gpu/drm/i915/display/intel_hdmi.c     |  2 +
 5 files changed, 53 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index ca28913a4c9f..a55bc7109045 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -4717,6 +4717,9 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
 	intel_encoder = &intel_dig_port->base;
 	encoder = &intel_encoder->base;
 
+	mutex_init(&intel_dig_port->hdcp_mutex);
+	intel_dig_port->num_hdcp_streams = 0;
+
 	drm_encoder_init(&dev_priv->drm, encoder, &intel_ddi_funcs,
 			 DRM_MODE_ENCODER_TMDS, "DDI %c", port_name(port));
 
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index bbd44772b9b0..489e1d00928b 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1309,6 +1309,11 @@ struct intel_digital_port {
 	enum phy_fia tc_phy_fia;
 	u8 tc_phy_fia_idx;
 
+	/* protects num_hdcp_streams reference count */
+	struct mutex hdcp_mutex;
+	/* the number of pipes using HDCP signalling out of this port */
+	unsigned int num_hdcp_streams;
+
 	void (*write_infoframe)(struct intel_encoder *encoder,
 				const struct intel_crtc_state *crtc_state,
 				unsigned int type,
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index e26fb26b1909..fc5a7dc6ab9b 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -7535,6 +7535,8 @@ bool intel_dp_init(struct drm_i915_private *dev_priv,
 	intel_encoder = &intel_dig_port->base;
 	encoder = &intel_encoder->base;
 
+	mutex_init(&intel_dig_port->hdcp_mutex);
+
 	if (drm_encoder_init(&dev_priv->drm, &intel_encoder->base,
 			     &intel_dp_enc_funcs, DRM_MODE_ENCODER_TMDS,
 			     "DP %c", port_name(port)))
diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
index f34763fa5b42..cd246f501738 100644
--- a/drivers/gpu/drm/i915/display/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
@@ -849,6 +849,7 @@ struct intel_connector *intel_hdcp_to_connector(struct intel_hdcp *hdcp)
 static void intel_hdcp_update_value(struct intel_connector *connector,
 				    u64 value, bool update_property)
 {
+	struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector);
 	struct intel_hdcp *hdcp = &connector->hdcp;
 
 	WARN_ON(!mutex_is_locked(&hdcp->mutex));
@@ -856,6 +857,15 @@ static void intel_hdcp_update_value(struct intel_connector *connector,
 	if (hdcp->value == value)
 		return;
 
+	WARN_ON(!mutex_is_locked(&intel_dig_port->hdcp_mutex));
+
+	if (hdcp->value == DRM_MODE_CONTENT_PROTECTION_ENABLED) {
+		if (!WARN_ON(intel_dig_port->num_hdcp_streams == 0))
+			intel_dig_port->num_hdcp_streams--;
+	} else if (value == DRM_MODE_CONTENT_PROTECTION_ENABLED) {
+		intel_dig_port->num_hdcp_streams++;
+	}
+
 	hdcp->value = value;
 	if (update_property)
 		schedule_work(&hdcp->prop_work);
@@ -872,6 +882,8 @@ static int intel_hdcp_check_link(struct intel_connector *connector)
 	int ret = 0;
 
 	mutex_lock(&hdcp->mutex);
+	mutex_lock(&intel_dig_port->hdcp_mutex);
+
 	cpu_transcoder = hdcp->cpu_transcoder;
 
 	/* Check_link valid only when HDCP1.4 is enabled */
@@ -923,6 +935,7 @@ static int intel_hdcp_check_link(struct intel_connector *connector)
 	}
 
 out:
+	mutex_unlock(&intel_dig_port->hdcp_mutex);
 	mutex_unlock(&hdcp->mutex);
 	return ret;
 }
@@ -1994,6 +2007,7 @@ int intel_hdcp_init(struct intel_connector *connector,
 
 int intel_hdcp_enable(struct intel_connector *connector, u8 content_type)
 {
+	struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector);
 	struct intel_hdcp *hdcp = &connector->hdcp;
 	unsigned long check_link_interval = DRM_HDCP_CHECK_PERIOD_MS;
 	int ret = -EINVAL;
@@ -2002,6 +2016,7 @@ int intel_hdcp_enable(struct intel_connector *connector, u8 content_type)
 		return -ENOENT;
 
 	mutex_lock(&hdcp->mutex);
+	mutex_lock(&intel_dig_port->hdcp_mutex);
 	WARN_ON(hdcp->value == DRM_MODE_CONTENT_PROTECTION_ENABLED);
 	hdcp->content_type = content_type;
 
@@ -2031,12 +2046,14 @@ int intel_hdcp_enable(struct intel_connector *connector, u8 content_type)
 					true);
 	}
 
+	mutex_unlock(&intel_dig_port->hdcp_mutex);
 	mutex_unlock(&hdcp->mutex);
 	return ret;
 }
 
 int intel_hdcp_disable(struct intel_connector *connector)
 {
+	struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector);
 	struct intel_hdcp *hdcp = &connector->hdcp;
 	int ret = 0;
 
@@ -2044,17 +2061,33 @@ int intel_hdcp_disable(struct intel_connector *connector)
 		return -ENOENT;
 
 	mutex_lock(&hdcp->mutex);
+	mutex_lock(&intel_dig_port->hdcp_mutex);
 
-	if (hdcp->value != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
-		intel_hdcp_update_value(connector,
-					DRM_MODE_CONTENT_PROTECTION_UNDESIRED,
-					false);
-		if (hdcp->hdcp2_encrypted)
-			ret = _intel_hdcp2_disable(connector);
-		else if (hdcp->hdcp_encrypted)
-			ret = _intel_hdcp_disable(connector);
+	if (hdcp->value == DRM_MODE_CONTENT_PROTECTION_UNDESIRED)
+		goto out;
+
+	intel_hdcp_update_value(connector,
+				DRM_MODE_CONTENT_PROTECTION_UNDESIRED, false);
+
+	/*
+	 * If there are other connectors on this port using HDCP, don't disable
+	 * it. Instead, toggle the HDCP signalling off on that particular
+	 * connector/pipe and exit.
+	 */
+	if (intel_dig_port->num_hdcp_streams > 0) {
+		ret = hdcp->shim->toggle_signalling(connector, false);
+		if (ret)
+			DRM_DEBUG_KMS("Failed to disable HDCP signalling\n");
+		goto out;
 	}
 
+	if (hdcp->hdcp2_encrypted)
+		ret = _intel_hdcp2_disable(connector);
+	else if (hdcp->hdcp_encrypted)
+		ret = _intel_hdcp_disable(connector);
+
+out:
+	mutex_unlock(&intel_dig_port->hdcp_mutex);
 	mutex_unlock(&hdcp->mutex);
 	cancel_delayed_work_sync(&hdcp->check_work);
 	return ret;
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 6a1e711c4f7a..c230d37c4276 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -3243,6 +3243,8 @@ void intel_hdmi_init(struct drm_i915_private *dev_priv,
 
 	intel_encoder = &intel_dig_port->base;
 
+	mutex_init(&intel_dig_port->hdcp_mutex);
+
 	drm_encoder_init(&dev_priv->drm, &intel_encoder->base,
 			 &intel_hdmi_enc_funcs, DRM_MODE_ENCODER_TMDS,
 			 "HDMI %c", port_name(port));
-- 
Sean Paul, Software Engineer, Google / Chromium OS

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

[Intel-gfx] [PATCH 07/11] drm/i915: Don't fully disable HDCP on a port if multiple pipes are using it

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

This patch is required for HDCP over MST. If a port is being used for
multiple HDCP streams, we don't want to fully disable HDCP on a port if
one of them is disabled. Instead, we just disable the HDCP signalling on
that particular pipe and exit early. The last pipe to disable HDCP will
also bring down HDCP on the port.

In order to achieve this, we need to keep a refcount in intel_digital_port
and protect it using a new hdcp_mutex.

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/i915/display/intel_ddi.c      |  3 ++
 .../drm/i915/display/intel_display_types.h    |  5 ++
 drivers/gpu/drm/i915/display/intel_dp.c       |  2 +
 drivers/gpu/drm/i915/display/intel_hdcp.c     | 49 ++++++++++++++++---
 drivers/gpu/drm/i915/display/intel_hdmi.c     |  2 +
 5 files changed, 53 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index ca28913a4c9f..a55bc7109045 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -4717,6 +4717,9 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
 	intel_encoder = &intel_dig_port->base;
 	encoder = &intel_encoder->base;
 
+	mutex_init(&intel_dig_port->hdcp_mutex);
+	intel_dig_port->num_hdcp_streams = 0;
+
 	drm_encoder_init(&dev_priv->drm, encoder, &intel_ddi_funcs,
 			 DRM_MODE_ENCODER_TMDS, "DDI %c", port_name(port));
 
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index bbd44772b9b0..489e1d00928b 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1309,6 +1309,11 @@ struct intel_digital_port {
 	enum phy_fia tc_phy_fia;
 	u8 tc_phy_fia_idx;
 
+	/* protects num_hdcp_streams reference count */
+	struct mutex hdcp_mutex;
+	/* the number of pipes using HDCP signalling out of this port */
+	unsigned int num_hdcp_streams;
+
 	void (*write_infoframe)(struct intel_encoder *encoder,
 				const struct intel_crtc_state *crtc_state,
 				unsigned int type,
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index e26fb26b1909..fc5a7dc6ab9b 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -7535,6 +7535,8 @@ bool intel_dp_init(struct drm_i915_private *dev_priv,
 	intel_encoder = &intel_dig_port->base;
 	encoder = &intel_encoder->base;
 
+	mutex_init(&intel_dig_port->hdcp_mutex);
+
 	if (drm_encoder_init(&dev_priv->drm, &intel_encoder->base,
 			     &intel_dp_enc_funcs, DRM_MODE_ENCODER_TMDS,
 			     "DP %c", port_name(port)))
diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
index f34763fa5b42..cd246f501738 100644
--- a/drivers/gpu/drm/i915/display/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
@@ -849,6 +849,7 @@ struct intel_connector *intel_hdcp_to_connector(struct intel_hdcp *hdcp)
 static void intel_hdcp_update_value(struct intel_connector *connector,
 				    u64 value, bool update_property)
 {
+	struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector);
 	struct intel_hdcp *hdcp = &connector->hdcp;
 
 	WARN_ON(!mutex_is_locked(&hdcp->mutex));
@@ -856,6 +857,15 @@ static void intel_hdcp_update_value(struct intel_connector *connector,
 	if (hdcp->value == value)
 		return;
 
+	WARN_ON(!mutex_is_locked(&intel_dig_port->hdcp_mutex));
+
+	if (hdcp->value == DRM_MODE_CONTENT_PROTECTION_ENABLED) {
+		if (!WARN_ON(intel_dig_port->num_hdcp_streams == 0))
+			intel_dig_port->num_hdcp_streams--;
+	} else if (value == DRM_MODE_CONTENT_PROTECTION_ENABLED) {
+		intel_dig_port->num_hdcp_streams++;
+	}
+
 	hdcp->value = value;
 	if (update_property)
 		schedule_work(&hdcp->prop_work);
@@ -872,6 +882,8 @@ static int intel_hdcp_check_link(struct intel_connector *connector)
 	int ret = 0;
 
 	mutex_lock(&hdcp->mutex);
+	mutex_lock(&intel_dig_port->hdcp_mutex);
+
 	cpu_transcoder = hdcp->cpu_transcoder;
 
 	/* Check_link valid only when HDCP1.4 is enabled */
@@ -923,6 +935,7 @@ static int intel_hdcp_check_link(struct intel_connector *connector)
 	}
 
 out:
+	mutex_unlock(&intel_dig_port->hdcp_mutex);
 	mutex_unlock(&hdcp->mutex);
 	return ret;
 }
@@ -1994,6 +2007,7 @@ int intel_hdcp_init(struct intel_connector *connector,
 
 int intel_hdcp_enable(struct intel_connector *connector, u8 content_type)
 {
+	struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector);
 	struct intel_hdcp *hdcp = &connector->hdcp;
 	unsigned long check_link_interval = DRM_HDCP_CHECK_PERIOD_MS;
 	int ret = -EINVAL;
@@ -2002,6 +2016,7 @@ int intel_hdcp_enable(struct intel_connector *connector, u8 content_type)
 		return -ENOENT;
 
 	mutex_lock(&hdcp->mutex);
+	mutex_lock(&intel_dig_port->hdcp_mutex);
 	WARN_ON(hdcp->value == DRM_MODE_CONTENT_PROTECTION_ENABLED);
 	hdcp->content_type = content_type;
 
@@ -2031,12 +2046,14 @@ int intel_hdcp_enable(struct intel_connector *connector, u8 content_type)
 					true);
 	}
 
+	mutex_unlock(&intel_dig_port->hdcp_mutex);
 	mutex_unlock(&hdcp->mutex);
 	return ret;
 }
 
 int intel_hdcp_disable(struct intel_connector *connector)
 {
+	struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector);
 	struct intel_hdcp *hdcp = &connector->hdcp;
 	int ret = 0;
 
@@ -2044,17 +2061,33 @@ int intel_hdcp_disable(struct intel_connector *connector)
 		return -ENOENT;
 
 	mutex_lock(&hdcp->mutex);
+	mutex_lock(&intel_dig_port->hdcp_mutex);
 
-	if (hdcp->value != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
-		intel_hdcp_update_value(connector,
-					DRM_MODE_CONTENT_PROTECTION_UNDESIRED,
-					false);
-		if (hdcp->hdcp2_encrypted)
-			ret = _intel_hdcp2_disable(connector);
-		else if (hdcp->hdcp_encrypted)
-			ret = _intel_hdcp_disable(connector);
+	if (hdcp->value == DRM_MODE_CONTENT_PROTECTION_UNDESIRED)
+		goto out;
+
+	intel_hdcp_update_value(connector,
+				DRM_MODE_CONTENT_PROTECTION_UNDESIRED, false);
+
+	/*
+	 * If there are other connectors on this port using HDCP, don't disable
+	 * it. Instead, toggle the HDCP signalling off on that particular
+	 * connector/pipe and exit.
+	 */
+	if (intel_dig_port->num_hdcp_streams > 0) {
+		ret = hdcp->shim->toggle_signalling(connector, false);
+		if (ret)
+			DRM_DEBUG_KMS("Failed to disable HDCP signalling\n");
+		goto out;
 	}
 
+	if (hdcp->hdcp2_encrypted)
+		ret = _intel_hdcp2_disable(connector);
+	else if (hdcp->hdcp_encrypted)
+		ret = _intel_hdcp_disable(connector);
+
+out:
+	mutex_unlock(&intel_dig_port->hdcp_mutex);
 	mutex_unlock(&hdcp->mutex);
 	cancel_delayed_work_sync(&hdcp->check_work);
 	return ret;
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 6a1e711c4f7a..c230d37c4276 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -3243,6 +3243,8 @@ void intel_hdmi_init(struct drm_i915_private *dev_priv,
 
 	intel_encoder = &intel_dig_port->base;
 
+	mutex_init(&intel_dig_port->hdcp_mutex);
+
 	drm_encoder_init(&dev_priv->drm, &intel_encoder->base,
 			 &intel_hdmi_enc_funcs, DRM_MODE_ENCODER_TMDS,
 			 "HDMI %c", port_name(port));
-- 
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 08/11] drm/i915: Support DP MST in enc_to_dig_port() function

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

Although DP_MST fake encoders are not subclassed from digital ports,
they are associated with them. Support these encoders.

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 .../drm/i915/display/intel_display_types.h    | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 489e1d00928b..4924784f3f4c 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1414,6 +1414,17 @@ static inline bool intel_encoder_is_dig_port(struct intel_encoder *encoder)
 	}
 }
 
+static inline bool intel_encoder_is_mst(struct intel_encoder *encoder)
+{
+	return encoder->type == INTEL_OUTPUT_DP_MST;
+}
+
+static inline struct intel_dp_mst_encoder *
+enc_to_mst(struct drm_encoder *encoder)
+{
+	return container_of(encoder, struct intel_dp_mst_encoder, base.base);
+}
+
 static inline struct intel_digital_port *
 enc_to_dig_port(struct drm_encoder *encoder)
 {
@@ -1422,6 +1433,8 @@ enc_to_dig_port(struct drm_encoder *encoder)
 	if (intel_encoder_is_dig_port(intel_encoder))
 		return container_of(encoder, struct intel_digital_port,
 				    base.base);
+	else if (intel_encoder_is_mst(intel_encoder))
+		return enc_to_mst(encoder)->primary;
 	else
 		return NULL;
 }
@@ -1432,12 +1445,6 @@ conn_to_dig_port(struct intel_connector *connector)
 	return enc_to_dig_port(&intel_attached_encoder(&connector->base)->base);
 }
 
-static inline struct intel_dp_mst_encoder *
-enc_to_mst(struct drm_encoder *encoder)
-{
-	return container_of(encoder, struct intel_dp_mst_encoder, base.base);
-}
-
 static inline struct intel_dp *enc_to_intel_dp(struct drm_encoder *encoder)
 {
 	return &enc_to_dig_port(encoder)->dp;
-- 
Sean Paul, Software Engineer, Google / Chromium OS

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

[Intel-gfx] [PATCH 08/11] drm/i915: Support DP MST in enc_to_dig_port() function

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

Although DP_MST fake encoders are not subclassed from digital ports,
they are associated with them. Support these encoders.

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 .../drm/i915/display/intel_display_types.h    | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 489e1d00928b..4924784f3f4c 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1414,6 +1414,17 @@ static inline bool intel_encoder_is_dig_port(struct intel_encoder *encoder)
 	}
 }
 
+static inline bool intel_encoder_is_mst(struct intel_encoder *encoder)
+{
+	return encoder->type == INTEL_OUTPUT_DP_MST;
+}
+
+static inline struct intel_dp_mst_encoder *
+enc_to_mst(struct drm_encoder *encoder)
+{
+	return container_of(encoder, struct intel_dp_mst_encoder, base.base);
+}
+
 static inline struct intel_digital_port *
 enc_to_dig_port(struct drm_encoder *encoder)
 {
@@ -1422,6 +1433,8 @@ enc_to_dig_port(struct drm_encoder *encoder)
 	if (intel_encoder_is_dig_port(intel_encoder))
 		return container_of(encoder, struct intel_digital_port,
 				    base.base);
+	else if (intel_encoder_is_mst(intel_encoder))
+		return enc_to_mst(encoder)->primary;
 	else
 		return NULL;
 }
@@ -1432,12 +1445,6 @@ conn_to_dig_port(struct intel_connector *connector)
 	return enc_to_dig_port(&intel_attached_encoder(&connector->base)->base);
 }
 
-static inline struct intel_dp_mst_encoder *
-enc_to_mst(struct drm_encoder *encoder)
-{
-	return container_of(encoder, struct intel_dp_mst_encoder, base.base);
-}
-
 static inline struct intel_dp *enc_to_intel_dp(struct drm_encoder *encoder)
 {
 	return &enc_to_dig_port(encoder)->dp;
-- 
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 09/11] drm/i915: Use ddi_update_pipe in intel_dp_mst

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

In order to act upon content_protection property changes, we'll need to
implement the .update_pipe() hook. We can re-use intel_ddi_update_pipe
for this

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/i915/display/intel_ddi.c    | 9 +++++----
 drivers/gpu/drm/i915/display/intel_dp.h     | 6 ++++++
 drivers/gpu/drm/i915/display/intel_dp_mst.c | 1 +
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index a55bc7109045..005afefbcbfa 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -4029,9 +4029,9 @@ static void intel_ddi_update_pipe_dp(struct intel_encoder *encoder,
 	intel_panel_update_backlight(encoder, crtc_state, conn_state);
 }
 
-static void intel_ddi_update_pipe(struct intel_encoder *encoder,
-				  const struct intel_crtc_state *crtc_state,
-				  const struct drm_connector_state *conn_state)
+void intel_ddi_update_pipe(struct intel_encoder *encoder,
+			   const struct intel_crtc_state *crtc_state,
+			   const struct drm_connector_state *conn_state)
 {
 	struct intel_connector *connector =
 				to_intel_connector(conn_state->connector);
@@ -4041,7 +4041,8 @@ static void intel_ddi_update_pipe(struct intel_encoder *encoder,
 			 conn_state->content_protection !=
 			 DRM_MODE_CONTENT_PROTECTION_UNDESIRED);
 
-	if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
+	if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI) &&
+	    !intel_encoder_is_mst(encoder))
 		intel_ddi_update_pipe_dp(encoder, crtc_state, conn_state);
 
 	/*
diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
index 3da166054788..db732b432809 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.h
+++ b/drivers/gpu/drm/i915/display/intel_dp.h
@@ -123,6 +123,12 @@ static inline unsigned int intel_dp_unused_lane_mask(int lane_count)
 	return ~((1 << lane_count) - 1) & 0xf;
 }
 
+
 u32 intel_dp_mode_to_fec_clock(u32 mode_clock);
 
+/* Shared between intel_dp and intel_dp_mst */
+void intel_ddi_update_pipe(struct intel_encoder *encoder,
+			   const struct intel_crtc_state *crtc_state,
+			   const struct drm_connector_state *conn_state);
+
 #endif /* __INTEL_DP_H__ */
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 715b7109c388..8ebf545c6fe5 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -628,6 +628,7 @@ intel_dp_create_fake_mst_encoder(struct intel_digital_port *intel_dig_port, enum
 	intel_encoder->compute_config = intel_dp_mst_compute_config;
 	intel_encoder->disable = intel_mst_disable_dp;
 	intel_encoder->post_disable = intel_mst_post_disable_dp;
+	intel_encoder->update_pipe = intel_ddi_update_pipe;
 	intel_encoder->pre_pll_enable = intel_mst_pre_pll_enable_dp;
 	intel_encoder->post_pll_disable = intel_mst_post_pll_disable_dp;
 	intel_encoder->pre_enable = intel_mst_pre_enable_dp;
-- 
Sean Paul, Software Engineer, Google / Chromium OS

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

[Intel-gfx] [PATCH 09/11] drm/i915: Use ddi_update_pipe in intel_dp_mst

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

In order to act upon content_protection property changes, we'll need to
implement the .update_pipe() hook. We can re-use intel_ddi_update_pipe
for this

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/i915/display/intel_ddi.c    | 9 +++++----
 drivers/gpu/drm/i915/display/intel_dp.h     | 6 ++++++
 drivers/gpu/drm/i915/display/intel_dp_mst.c | 1 +
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index a55bc7109045..005afefbcbfa 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -4029,9 +4029,9 @@ static void intel_ddi_update_pipe_dp(struct intel_encoder *encoder,
 	intel_panel_update_backlight(encoder, crtc_state, conn_state);
 }
 
-static void intel_ddi_update_pipe(struct intel_encoder *encoder,
-				  const struct intel_crtc_state *crtc_state,
-				  const struct drm_connector_state *conn_state)
+void intel_ddi_update_pipe(struct intel_encoder *encoder,
+			   const struct intel_crtc_state *crtc_state,
+			   const struct drm_connector_state *conn_state)
 {
 	struct intel_connector *connector =
 				to_intel_connector(conn_state->connector);
@@ -4041,7 +4041,8 @@ static void intel_ddi_update_pipe(struct intel_encoder *encoder,
 			 conn_state->content_protection !=
 			 DRM_MODE_CONTENT_PROTECTION_UNDESIRED);
 
-	if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
+	if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI) &&
+	    !intel_encoder_is_mst(encoder))
 		intel_ddi_update_pipe_dp(encoder, crtc_state, conn_state);
 
 	/*
diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
index 3da166054788..db732b432809 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.h
+++ b/drivers/gpu/drm/i915/display/intel_dp.h
@@ -123,6 +123,12 @@ static inline unsigned int intel_dp_unused_lane_mask(int lane_count)
 	return ~((1 << lane_count) - 1) & 0xf;
 }
 
+
 u32 intel_dp_mode_to_fec_clock(u32 mode_clock);
 
+/* Shared between intel_dp and intel_dp_mst */
+void intel_ddi_update_pipe(struct intel_encoder *encoder,
+			   const struct intel_crtc_state *crtc_state,
+			   const struct drm_connector_state *conn_state);
+
 #endif /* __INTEL_DP_H__ */
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 715b7109c388..8ebf545c6fe5 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -628,6 +628,7 @@ intel_dp_create_fake_mst_encoder(struct intel_digital_port *intel_dig_port, enum
 	intel_encoder->compute_config = intel_dp_mst_compute_config;
 	intel_encoder->disable = intel_mst_disable_dp;
 	intel_encoder->post_disable = intel_mst_post_disable_dp;
+	intel_encoder->update_pipe = intel_ddi_update_pipe;
 	intel_encoder->pre_pll_enable = intel_mst_pre_pll_enable_dp;
 	intel_encoder->post_pll_disable = intel_mst_post_pll_disable_dp;
 	intel_encoder->pre_enable = intel_mst_pre_enable_dp;
-- 
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 10/11] drm/i915: Expose HDCP shim functions from dp for use by dp_mst

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

These functions are all the same for dp and dp_mst, so expose them for
use by the dp_mst hdcp implementation.

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 .../drm/i915/display/intel_display_types.h    | 22 +++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_dp.c       | 14 ++----------
 2 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 4924784f3f4c..6fafa92dcf76 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1573,4 +1573,26 @@ static inline u32 intel_plane_ggtt_offset(const struct intel_plane_state *state)
 	return i915_ggtt_offset(state->vma);
 }
 
+int intel_dp_hdcp_write_an_aksv(struct intel_digital_port *intel_dig_port,
+				u8 *an);
+int intel_dp_hdcp_read_bksv(struct intel_digital_port *intel_dig_port,
+			    u8 *bksv);
+int intel_dp_hdcp_read_bstatus(struct intel_digital_port *intel_dig_port,
+				      u8 *bstatus);
+int intel_dp_hdcp_read_bcaps(struct intel_digital_port *intel_dig_port,
+			     u8 *bcaps);
+int intel_dp_hdcp_repeater_present(struct intel_digital_port *intel_dig_port,
+				   bool *repeater_present);
+int intel_dp_hdcp_read_ri_prime(struct intel_digital_port *intel_dig_port,
+				u8 *ri_prime);
+int intel_dp_hdcp_read_ksv_ready(struct intel_digital_port *intel_dig_port,
+				 bool *ksv_ready);
+int intel_dp_hdcp_read_ksv_fifo(struct intel_digital_port *intel_dig_port,
+				int num_downstream, u8 *ksv_fifo);
+int intel_dp_hdcp_read_v_prime_part(struct intel_digital_port *intel_dig_port,
+				    int i, u32 *part);
+bool intel_dp_hdcp_check_link(struct intel_digital_port *intel_dig_port);
+int intel_dp_hdcp_capable(struct intel_digital_port *intel_dig_port,
+			  bool *hdcp_capable);
+
 #endif /*  __INTEL_DISPLAY_TYPES_H__ */
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index fc5a7dc6ab9b..600de7606596 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -5871,7 +5871,6 @@ static void intel_dp_hdcp_wait_for_cp_irq(struct intel_hdcp *hdcp, int timeout)
 		DRM_DEBUG_KMS("Timedout at waiting for CP_IRQ\n");
 }
 
-static
 int intel_dp_hdcp_write_an_aksv(struct intel_digital_port *intel_dig_port,
 				u8 *an)
 {
@@ -5903,8 +5902,7 @@ int intel_dp_hdcp_write_an_aksv(struct intel_digital_port *intel_dig_port,
 	return 0;
 }
 
-static int intel_dp_hdcp_read_bksv(struct intel_digital_port *intel_dig_port,
-				   u8 *bksv)
+int intel_dp_hdcp_read_bksv(struct intel_digital_port *intel_dig_port, u8 *bksv)
 {
 	ssize_t ret;
 	ret = drm_dp_dpcd_read(&intel_dig_port->dp.aux, DP_AUX_HDCP_BKSV, bksv,
@@ -5916,7 +5914,7 @@ static int intel_dp_hdcp_read_bksv(struct intel_digital_port *intel_dig_port,
 	return 0;
 }
 
-static int intel_dp_hdcp_read_bstatus(struct intel_digital_port *intel_dig_port,
+int intel_dp_hdcp_read_bstatus(struct intel_digital_port *intel_dig_port,
 				      u8 *bstatus)
 {
 	ssize_t ret;
@@ -5934,7 +5932,6 @@ static int intel_dp_hdcp_read_bstatus(struct intel_digital_port *intel_dig_port,
 	return 0;
 }
 
-static
 int intel_dp_hdcp_read_bcaps(struct intel_digital_port *intel_dig_port,
 			     u8 *bcaps)
 {
@@ -5950,7 +5947,6 @@ int intel_dp_hdcp_read_bcaps(struct intel_digital_port *intel_dig_port,
 	return 0;
 }
 
-static
 int intel_dp_hdcp_repeater_present(struct intel_digital_port *intel_dig_port,
 				   bool *repeater_present)
 {
@@ -5965,7 +5961,6 @@ int intel_dp_hdcp_repeater_present(struct intel_digital_port *intel_dig_port,
 	return 0;
 }
 
-static
 int intel_dp_hdcp_read_ri_prime(struct intel_digital_port *intel_dig_port,
 				u8 *ri_prime)
 {
@@ -5979,7 +5974,6 @@ int intel_dp_hdcp_read_ri_prime(struct intel_digital_port *intel_dig_port,
 	return 0;
 }
 
-static
 int intel_dp_hdcp_read_ksv_ready(struct intel_digital_port *intel_dig_port,
 				 bool *ksv_ready)
 {
@@ -5995,7 +5989,6 @@ int intel_dp_hdcp_read_ksv_ready(struct intel_digital_port *intel_dig_port,
 	return 0;
 }
 
-static
 int intel_dp_hdcp_read_ksv_fifo(struct intel_digital_port *intel_dig_port,
 				int num_downstream, u8 *ksv_fifo)
 {
@@ -6018,7 +6011,6 @@ int intel_dp_hdcp_read_ksv_fifo(struct intel_digital_port *intel_dig_port,
 	return 0;
 }
 
-static
 int intel_dp_hdcp_read_v_prime_part(struct intel_digital_port *intel_dig_port,
 				    int i, u32 *part)
 {
@@ -6045,7 +6037,6 @@ int intel_dp_hdcp_toggle_signalling(struct intel_connector *connector,
 	return 0;
 }
 
-static
 bool intel_dp_hdcp_check_link(struct intel_digital_port *intel_dig_port)
 {
 	ssize_t ret;
@@ -6061,7 +6052,6 @@ bool intel_dp_hdcp_check_link(struct intel_digital_port *intel_dig_port)
 	return !(bstatus & (DP_BSTATUS_LINK_FAILURE | DP_BSTATUS_REAUTH_REQ));
 }
 
-static
 int intel_dp_hdcp_capable(struct intel_digital_port *intel_dig_port,
 			  bool *hdcp_capable)
 {
-- 
Sean Paul, Software Engineer, Google / Chromium OS

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

[Intel-gfx] [PATCH 10/11] drm/i915: Expose HDCP shim functions from dp for use by dp_mst

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

These functions are all the same for dp and dp_mst, so expose them for
use by the dp_mst hdcp implementation.

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 .../drm/i915/display/intel_display_types.h    | 22 +++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_dp.c       | 14 ++----------
 2 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 4924784f3f4c..6fafa92dcf76 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1573,4 +1573,26 @@ static inline u32 intel_plane_ggtt_offset(const struct intel_plane_state *state)
 	return i915_ggtt_offset(state->vma);
 }
 
+int intel_dp_hdcp_write_an_aksv(struct intel_digital_port *intel_dig_port,
+				u8 *an);
+int intel_dp_hdcp_read_bksv(struct intel_digital_port *intel_dig_port,
+			    u8 *bksv);
+int intel_dp_hdcp_read_bstatus(struct intel_digital_port *intel_dig_port,
+				      u8 *bstatus);
+int intel_dp_hdcp_read_bcaps(struct intel_digital_port *intel_dig_port,
+			     u8 *bcaps);
+int intel_dp_hdcp_repeater_present(struct intel_digital_port *intel_dig_port,
+				   bool *repeater_present);
+int intel_dp_hdcp_read_ri_prime(struct intel_digital_port *intel_dig_port,
+				u8 *ri_prime);
+int intel_dp_hdcp_read_ksv_ready(struct intel_digital_port *intel_dig_port,
+				 bool *ksv_ready);
+int intel_dp_hdcp_read_ksv_fifo(struct intel_digital_port *intel_dig_port,
+				int num_downstream, u8 *ksv_fifo);
+int intel_dp_hdcp_read_v_prime_part(struct intel_digital_port *intel_dig_port,
+				    int i, u32 *part);
+bool intel_dp_hdcp_check_link(struct intel_digital_port *intel_dig_port);
+int intel_dp_hdcp_capable(struct intel_digital_port *intel_dig_port,
+			  bool *hdcp_capable);
+
 #endif /*  __INTEL_DISPLAY_TYPES_H__ */
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index fc5a7dc6ab9b..600de7606596 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -5871,7 +5871,6 @@ static void intel_dp_hdcp_wait_for_cp_irq(struct intel_hdcp *hdcp, int timeout)
 		DRM_DEBUG_KMS("Timedout at waiting for CP_IRQ\n");
 }
 
-static
 int intel_dp_hdcp_write_an_aksv(struct intel_digital_port *intel_dig_port,
 				u8 *an)
 {
@@ -5903,8 +5902,7 @@ int intel_dp_hdcp_write_an_aksv(struct intel_digital_port *intel_dig_port,
 	return 0;
 }
 
-static int intel_dp_hdcp_read_bksv(struct intel_digital_port *intel_dig_port,
-				   u8 *bksv)
+int intel_dp_hdcp_read_bksv(struct intel_digital_port *intel_dig_port, u8 *bksv)
 {
 	ssize_t ret;
 	ret = drm_dp_dpcd_read(&intel_dig_port->dp.aux, DP_AUX_HDCP_BKSV, bksv,
@@ -5916,7 +5914,7 @@ static int intel_dp_hdcp_read_bksv(struct intel_digital_port *intel_dig_port,
 	return 0;
 }
 
-static int intel_dp_hdcp_read_bstatus(struct intel_digital_port *intel_dig_port,
+int intel_dp_hdcp_read_bstatus(struct intel_digital_port *intel_dig_port,
 				      u8 *bstatus)
 {
 	ssize_t ret;
@@ -5934,7 +5932,6 @@ static int intel_dp_hdcp_read_bstatus(struct intel_digital_port *intel_dig_port,
 	return 0;
 }
 
-static
 int intel_dp_hdcp_read_bcaps(struct intel_digital_port *intel_dig_port,
 			     u8 *bcaps)
 {
@@ -5950,7 +5947,6 @@ int intel_dp_hdcp_read_bcaps(struct intel_digital_port *intel_dig_port,
 	return 0;
 }
 
-static
 int intel_dp_hdcp_repeater_present(struct intel_digital_port *intel_dig_port,
 				   bool *repeater_present)
 {
@@ -5965,7 +5961,6 @@ int intel_dp_hdcp_repeater_present(struct intel_digital_port *intel_dig_port,
 	return 0;
 }
 
-static
 int intel_dp_hdcp_read_ri_prime(struct intel_digital_port *intel_dig_port,
 				u8 *ri_prime)
 {
@@ -5979,7 +5974,6 @@ int intel_dp_hdcp_read_ri_prime(struct intel_digital_port *intel_dig_port,
 	return 0;
 }
 
-static
 int intel_dp_hdcp_read_ksv_ready(struct intel_digital_port *intel_dig_port,
 				 bool *ksv_ready)
 {
@@ -5995,7 +5989,6 @@ int intel_dp_hdcp_read_ksv_ready(struct intel_digital_port *intel_dig_port,
 	return 0;
 }
 
-static
 int intel_dp_hdcp_read_ksv_fifo(struct intel_digital_port *intel_dig_port,
 				int num_downstream, u8 *ksv_fifo)
 {
@@ -6018,7 +6011,6 @@ int intel_dp_hdcp_read_ksv_fifo(struct intel_digital_port *intel_dig_port,
 	return 0;
 }
 
-static
 int intel_dp_hdcp_read_v_prime_part(struct intel_digital_port *intel_dig_port,
 				    int i, u32 *part)
 {
@@ -6045,7 +6037,6 @@ int intel_dp_hdcp_toggle_signalling(struct intel_connector *connector,
 	return 0;
 }
 
-static
 bool intel_dp_hdcp_check_link(struct intel_digital_port *intel_dig_port)
 {
 	ssize_t ret;
@@ -6061,7 +6052,6 @@ bool intel_dp_hdcp_check_link(struct intel_digital_port *intel_dig_port)
 	return !(bstatus & (DP_BSTATUS_LINK_FAILURE | DP_BSTATUS_REAUTH_REQ));
 }
 
-static
 int intel_dp_hdcp_capable(struct intel_digital_port *intel_dig_port,
 			  bool *hdcp_capable)
 {
-- 
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 11/11] drm/i915: Add HDCP 1.4 support for MST connectors

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

Now that all the groundwork has been laid, we can turn on HDCP 1.4 over
MST. Everything except for toggling the HDCP signalling and HDCP 2.2
support is the same as the DP case, so we'll re-use those callbacks

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/i915/display/intel_dp_mst.c | 83 +++++++++++++++++++++
 1 file changed, 83 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 8ebf545c6fe5..d26f12cb432c 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -36,6 +36,7 @@
 #include "intel_dp.h"
 #include "intel_dp_mst.h"
 #include "intel_dpio_phy.h"
+#include "intel_hdcp.h"
 
 static int intel_dp_mst_compute_link_config(struct intel_encoder *encoder,
 					    struct intel_crtc_state *crtc_state,
@@ -495,11 +496,83 @@ static bool intel_dp_mst_get_hw_state(struct intel_connector *connector)
 	return false;
 }
 
+static int
+intel_dp_mst_hdcp_toggle_signalling(struct intel_connector *connector,
+				    bool enable)
+{
+	int ret;
+	struct intel_encoder *enc = intel_attached_encoder(&connector->base);
+
+	ret = intel_ddi_toggle_hdcp_signalling(enc, enable);
+	if (ret)
+		DRM_DEBUG_KMS("%s HDCP signalling failed (%d)\n",
+			      enable ? "Enable" : "Disable", ret);
+	return ret;
+}
+
+static
+int intel_dp_mst_hdcp2_write_msg(struct intel_digital_port *intel_dig_port,
+				 void *buf, size_t size)
+{
+	return -EOPNOTSUPP;
+}
+
+static
+int intel_dp_mst_hdcp2_read_msg(struct intel_digital_port *intel_dig_port,
+				u8 msg_id, void *buf, size_t size)
+{
+	return -EOPNOTSUPP;
+}
+
+static int
+intel_dp_mst_hdcp2_config_stream_type(struct intel_digital_port *intel_dig_port,
+				      bool is_repeater, u8 content_type)
+{
+	return -EOPNOTSUPP;
+}
+
+static
+int intel_dp_mst_hdcp2_check_link(struct intel_digital_port *intel_dig_port)
+{
+	return -EOPNOTSUPP;
+}
+
+static
+int intel_dp_mst_hdcp2_capable(struct intel_digital_port *intel_dig_port,
+			       bool *capable)
+{
+	*capable = false;
+	return 0;
+}
+
+static const struct intel_hdcp_shim intel_dp_hdcp_shim = {
+	.write_an_aksv = intel_dp_hdcp_write_an_aksv,
+	.read_bksv = intel_dp_hdcp_read_bksv,
+	.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,
+	.read_ksv_fifo = intel_dp_hdcp_read_ksv_fifo,
+	.read_v_prime_part = intel_dp_hdcp_read_v_prime_part,
+	.toggle_signalling = intel_dp_mst_hdcp_toggle_signalling,
+	.check_link = intel_dp_hdcp_check_link,
+	.hdcp_capable = intel_dp_hdcp_capable,
+
+	.write_2_2_msg = intel_dp_mst_hdcp2_write_msg,
+	.read_2_2_msg = intel_dp_mst_hdcp2_read_msg,
+	.config_stream_type = intel_dp_mst_hdcp2_config_stream_type,
+	.check_2_2_link = intel_dp_mst_hdcp2_check_link,
+	.hdcp_2_2_capable = intel_dp_mst_hdcp2_capable,
+
+	.protocol = HDCP_PROTOCOL_DP,
+};
+
 static struct drm_connector *intel_dp_add_mst_connector(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port, const char *pathprop)
 {
 	struct intel_dp *intel_dp = container_of(mgr, struct intel_dp, mst_mgr);
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 	struct drm_device *dev = intel_dig_port->base.base.dev;
+	struct intel_encoder *intel_encoder = &intel_dig_port->base;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_connector *intel_connector;
 	struct drm_connector *connector;
@@ -544,6 +617,12 @@ static struct drm_connector *intel_dp_add_mst_connector(struct drm_dp_mst_topolo
 	intel_attach_force_audio_property(connector);
 	intel_attach_broadcast_rgb_property(connector);
 
+	if (is_hdcp_supported(dev_priv, intel_encoder->port)) {
+		int ret = intel_hdcp_init(intel_connector, &intel_dp_hdcp_shim);
+		if (ret)
+			DRM_DEBUG_KMS("HDCP init failed, skipping.\n");
+	}
+
 	/*
 	 * Reuse the prop from the SST connector because we're
 	 * not allowed to create new props after device registration.
@@ -575,8 +654,12 @@ static void intel_dp_destroy_mst_connector(struct drm_dp_mst_topology_mgr *mgr,
 					   struct drm_connector *connector)
 {
 	struct drm_i915_private *dev_priv = to_i915(connector->dev);
+	struct intel_connector *intel_connector =
+		to_intel_connector(connector);
 
 	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", connector->base.id, connector->name);
+
+	intel_hdcp_disable(intel_connector);
 	drm_connector_unregister(connector);
 
 	if (dev_priv->fbdev)
-- 
Sean Paul, Software Engineer, Google / Chromium OS

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

[Intel-gfx] [PATCH 11/11] drm/i915: Add HDCP 1.4 support for MST connectors

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

Now that all the groundwork has been laid, we can turn on HDCP 1.4 over
MST. Everything except for toggling the HDCP signalling and HDCP 2.2
support is the same as the DP case, so we'll re-use those callbacks

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/i915/display/intel_dp_mst.c | 83 +++++++++++++++++++++
 1 file changed, 83 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 8ebf545c6fe5..d26f12cb432c 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -36,6 +36,7 @@
 #include "intel_dp.h"
 #include "intel_dp_mst.h"
 #include "intel_dpio_phy.h"
+#include "intel_hdcp.h"
 
 static int intel_dp_mst_compute_link_config(struct intel_encoder *encoder,
 					    struct intel_crtc_state *crtc_state,
@@ -495,11 +496,83 @@ static bool intel_dp_mst_get_hw_state(struct intel_connector *connector)
 	return false;
 }
 
+static int
+intel_dp_mst_hdcp_toggle_signalling(struct intel_connector *connector,
+				    bool enable)
+{
+	int ret;
+	struct intel_encoder *enc = intel_attached_encoder(&connector->base);
+
+	ret = intel_ddi_toggle_hdcp_signalling(enc, enable);
+	if (ret)
+		DRM_DEBUG_KMS("%s HDCP signalling failed (%d)\n",
+			      enable ? "Enable" : "Disable", ret);
+	return ret;
+}
+
+static
+int intel_dp_mst_hdcp2_write_msg(struct intel_digital_port *intel_dig_port,
+				 void *buf, size_t size)
+{
+	return -EOPNOTSUPP;
+}
+
+static
+int intel_dp_mst_hdcp2_read_msg(struct intel_digital_port *intel_dig_port,
+				u8 msg_id, void *buf, size_t size)
+{
+	return -EOPNOTSUPP;
+}
+
+static int
+intel_dp_mst_hdcp2_config_stream_type(struct intel_digital_port *intel_dig_port,
+				      bool is_repeater, u8 content_type)
+{
+	return -EOPNOTSUPP;
+}
+
+static
+int intel_dp_mst_hdcp2_check_link(struct intel_digital_port *intel_dig_port)
+{
+	return -EOPNOTSUPP;
+}
+
+static
+int intel_dp_mst_hdcp2_capable(struct intel_digital_port *intel_dig_port,
+			       bool *capable)
+{
+	*capable = false;
+	return 0;
+}
+
+static const struct intel_hdcp_shim intel_dp_hdcp_shim = {
+	.write_an_aksv = intel_dp_hdcp_write_an_aksv,
+	.read_bksv = intel_dp_hdcp_read_bksv,
+	.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,
+	.read_ksv_fifo = intel_dp_hdcp_read_ksv_fifo,
+	.read_v_prime_part = intel_dp_hdcp_read_v_prime_part,
+	.toggle_signalling = intel_dp_mst_hdcp_toggle_signalling,
+	.check_link = intel_dp_hdcp_check_link,
+	.hdcp_capable = intel_dp_hdcp_capable,
+
+	.write_2_2_msg = intel_dp_mst_hdcp2_write_msg,
+	.read_2_2_msg = intel_dp_mst_hdcp2_read_msg,
+	.config_stream_type = intel_dp_mst_hdcp2_config_stream_type,
+	.check_2_2_link = intel_dp_mst_hdcp2_check_link,
+	.hdcp_2_2_capable = intel_dp_mst_hdcp2_capable,
+
+	.protocol = HDCP_PROTOCOL_DP,
+};
+
 static struct drm_connector *intel_dp_add_mst_connector(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port, const char *pathprop)
 {
 	struct intel_dp *intel_dp = container_of(mgr, struct intel_dp, mst_mgr);
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 	struct drm_device *dev = intel_dig_port->base.base.dev;
+	struct intel_encoder *intel_encoder = &intel_dig_port->base;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_connector *intel_connector;
 	struct drm_connector *connector;
@@ -544,6 +617,12 @@ static struct drm_connector *intel_dp_add_mst_connector(struct drm_dp_mst_topolo
 	intel_attach_force_audio_property(connector);
 	intel_attach_broadcast_rgb_property(connector);
 
+	if (is_hdcp_supported(dev_priv, intel_encoder->port)) {
+		int ret = intel_hdcp_init(intel_connector, &intel_dp_hdcp_shim);
+		if (ret)
+			DRM_DEBUG_KMS("HDCP init failed, skipping.\n");
+	}
+
 	/*
 	 * Reuse the prop from the SST connector because we're
 	 * not allowed to create new props after device registration.
@@ -575,8 +654,12 @@ static void intel_dp_destroy_mst_connector(struct drm_dp_mst_topology_mgr *mgr,
 					   struct drm_connector *connector)
 {
 	struct drm_i915_private *dev_priv = to_i915(connector->dev);
+	struct intel_connector *intel_connector =
+		to_intel_connector(connector);
 
 	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", connector->base.id, connector->name);
+
+	intel_hdcp_disable(intel_connector);
 	drm_connector_unregister(connector);
 
 	if (dev_priv->fbdev)
-- 
Sean Paul, Software Engineer, Google / Chromium OS

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

[Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/i915: Add support for HDCP 1.4 over MST connectors

[Patchwork]

== Series Details ==

Series: drm/i915: Add support for HDCP 1.4 over MST connectors
URL   : https://patchwork.freedesktop.org/series/70393/
State : failure

== Summary ==

Applying: drm/i915: Fix sha_text population code
Applying: drm/i915: Intercept Aksv writes in the aux hooks
Applying: drm/i915: Disable HDCP signalling on transcoder disable
Applying: drm/i915: Don't WARN on HDCP toggle if get_hw_state returns false
Applying: drm/i915: Change toggle_signalling() argument to connector
Applying: drm/i915: Factor out hdcp->value assignments
Applying: drm/i915: Don't fully disable HDCP on a port if multiple pipes are using it
error: sha1 information is lacking or useless (drivers/gpu/drm/i915/display/intel_ddi.c).
error: could not build fake ancestor
hint: Use 'git am --show-current-patch' to see the failed patch
Patch failed at 0007 drm/i915: Don't fully disable HDCP on a port if multiple pipes are using it
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