All of lore.kernel.org
 help / color / mirror / Atom feed
* [Intel-gfx] [RFC] drm/i915/hdcp: Gen12 HDCP 1.4 support over DP MST
@ 2020-09-01 12:10 Anshuman Gupta
  2020-09-01 12:40 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for " Patchwork
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Anshuman Gupta @ 2020-09-01 12:10 UTC (permalink / raw)
  To: intel-gfx; +Cc: seanpaul

Gen12 has measure changes with respect to HDCP display
engine instaces lies in Trascoder insead of DDI as in Gen11.

This requires hdcp driver to use mst_master_transcoder for link
authentication and stream trascoder for stream encryption
separately.

It also requires to validate the stream encryption status
in HDCP_STATUS_{TRANSCODER,PORT} driving that link register.

There is also some changes over existing HDCP 1.4  DP MST Gen11
implementation, related to Multistream HDCP Select bit in
TRANS_DDI_FUNC_CTL need to be required with respect to B.Spec
Documentation.

Cc: Ramalingam C <ramalingam.c@intel.com>
Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
 drivers/gpu/drm/i915/display/intel_ddi.c      | 12 +--
 drivers/gpu/drm/i915/display/intel_ddi.h      |  6 +-
 .../drm/i915/display/intel_display_types.h    |  9 +++
 drivers/gpu/drm/i915/display/intel_dp_hdcp.c  | 73 ++++++++++++++++---
 drivers/gpu/drm/i915/display/intel_dp_mst.c   |  4 +-
 drivers/gpu/drm/i915/display/intel_hdcp.c     | 35 ++++++---
 drivers/gpu/drm/i915/display/intel_hdcp.h     |  4 +-
 drivers/gpu/drm/i915/display/intel_hdmi.c     | 16 ++--
 drivers/gpu/drm/i915/i915_reg.h               |  1 +
 9 files changed, 121 insertions(+), 39 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index a2b7dcf84430..5d6e4fd7bccd 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -1849,9 +1849,9 @@ void intel_ddi_disable_transcoder_func(const struct intel_crtc_state *crtc_state
 	}
 }
 
-int intel_ddi_toggle_hdcp_signalling(struct intel_encoder *intel_encoder,
-				     enum transcoder cpu_transcoder,
-				     bool enable)
+int intel_ddi_toggle_hdcp_bits(struct intel_encoder *intel_encoder,
+			       enum transcoder cpu_transcoder,
+			       bool enable, u32 hdcp_mask)
 {
 	struct drm_device *dev = intel_encoder->base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
@@ -1866,9 +1866,9 @@ int intel_ddi_toggle_hdcp_signalling(struct intel_encoder *intel_encoder,
 
 	tmp = intel_de_read(dev_priv, TRANS_DDI_FUNC_CTL(cpu_transcoder));
 	if (enable)
-		tmp |= TRANS_DDI_HDCP_SIGNALLING;
+		tmp |= hdcp_mask;
 	else
-		tmp &= ~TRANS_DDI_HDCP_SIGNALLING;
+		tmp &= ~hdcp_mask;
 	intel_de_write(dev_priv, TRANS_DDI_FUNC_CTL(cpu_transcoder), tmp);
 	intel_display_power_put(dev_priv, intel_encoder->power_domain, wakeref);
 	return ret;
@@ -3967,7 +3967,7 @@ static void intel_enable_ddi(struct intel_atomic_state *state,
 	if (conn_state->content_protection ==
 	    DRM_MODE_CONTENT_PROTECTION_DESIRED)
 		intel_hdcp_enable(to_intel_connector(conn_state->connector),
-				  crtc_state->cpu_transcoder,
+				  crtc_state,
 				  (u8)conn_state->hdcp_content_type);
 }
 
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.h b/drivers/gpu/drm/i915/display/intel_ddi.h
index f5fb62fc9400..69d9e495992c 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.h
+++ b/drivers/gpu/drm/i915/display/intel_ddi.h
@@ -43,9 +43,9 @@ void intel_ddi_compute_min_voltage_level(struct drm_i915_private *dev_priv,
 					 struct intel_crtc_state *crtc_state);
 u32 bxt_signal_levels(struct intel_dp *intel_dp);
 u32 ddi_signal_levels(struct intel_dp *intel_dp);
-int intel_ddi_toggle_hdcp_signalling(struct intel_encoder *intel_encoder,
-				     enum transcoder cpu_transcoder,
-				     bool enable);
+int intel_ddi_toggle_hdcp_bits(struct intel_encoder *intel_encoder,
+			       enum transcoder cpu_transcoder,
+			       bool enable, u32 hdcp_mask);
 void icl_sanitize_encoder_pll_mapping(struct intel_encoder *encoder);
 
 #endif /* __INTEL_DDI_H__ */
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 413b60337a0b..dc71ee4d314a 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -317,6 +317,13 @@ struct intel_hdcp_shim {
 				 enum transcoder cpu_transcoder,
 				 bool enable);
 
+	/* Select/Deselect HDCP stream on the port DP MST Transport Link */
+	int (*toggle_select_hdcp)(struct intel_digital_port *intel_dig_port,
+				  bool enable);
+
+	/* Enable HDCP stream encyption on DP MST Transport Link */
+	int (*stream_encryption)(struct intel_digital_port *intel_dig_port);
+
 	/* Ensures the link is still protected */
 	bool (*check_link)(struct intel_digital_port *dig_port,
 			   struct intel_connector *connector);
@@ -410,6 +417,8 @@ struct intel_hdcp {
 	 * Hence caching the transcoder here.
 	 */
 	enum transcoder cpu_transcoder;
+	/* Only used for DP MST stream encryption */
+	enum transcoder stream_transcoder;
 };
 
 struct intel_connector {
diff --git a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
index 03424d20e9f7..8a6427f3690b 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
@@ -16,6 +16,30 @@
 #include "intel_dp.h"
 #include "intel_hdcp.h"
 
+static unsigned int trasncoder_to_stream_enc_status(enum transcoder cpu_transcoder)
+{
+	u32 stream_enc_mask;
+
+	switch (cpu_transcoder) {
+	case TRANSCODER_A:
+		stream_enc_mask = HDCP_STATUS_STREAM_A_ENC;
+		break;
+	case TRANSCODER_B:
+		stream_enc_mask = HDCP_STATUS_STREAM_B_ENC;
+		break;
+	case TRANSCODER_C:
+		stream_enc_mask = HDCP_STATUS_STREAM_C_ENC;
+		break;
+	case TRANSCODER_D:
+		stream_enc_mask = HDCP_STATUS_STREAM_D_ENC;
+		break;
+	default:
+		stream_enc_mask = 0;
+	}
+
+	return stream_enc_mask;
+}
+
 static void intel_dp_hdcp_wait_for_cp_irq(struct intel_hdcp *hdcp, int timeout)
 {
 	long ret;
@@ -622,24 +646,51 @@ static const struct intel_hdcp_shim intel_dp_hdcp_shim = {
 };
 
 static int
-intel_dp_mst_hdcp_toggle_signalling(struct intel_digital_port *dig_port,
-				    enum transcoder cpu_transcoder,
-				    bool enable)
+intel_dp_mst_toggle_select_hdcp_strem(struct intel_digital_port *dig_port,
+				      bool enable)
 {
 	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
+	struct intel_dp *dp = &dig_port->dp;
+	struct intel_hdcp *hdcp = &dp->attached_connector->hdcp;
 	int ret;
 
-	if (!enable)
-		usleep_range(6, 60); /* Bspec says >= 6us */
+	ret = intel_ddi_toggle_hdcp_bits(&dig_port->base,
+					 hdcp->stream_transcoder, enable,
+					 TRANS_DDI_HDCP_SELECT);
 
-	ret = intel_ddi_toggle_hdcp_signalling(&dig_port->base,
-					       cpu_transcoder, enable);
 	if (ret)
-		drm_dbg_kms(&i915->drm, "%s HDCP signalling failed (%d)\n",
-			      enable ? "Enable" : "Disable", ret);
+		drm_dbg_kms(&i915->drm, "%s HDCP select failed (%d)\n",
+			    enable ? "Enable" : "Disable", ret);
 	return ret;
 }
 
+static int
+intel_dp_mst_hdcp_strem_encryption(struct intel_digital_port *dig_port)
+{
+	struct drm_i915_private *i915 = to_i915(idig_port->base.base.dev);
+	struct intel_dp *dp = &dig_port->dp;
+	struct intel_hdcp *hdcp = &dp->attached_connector->hdcp;
+	enum port port = dig_port->base.port;
+	enum transcoder cpu_transcoder = hdcp->cpu_transcoder;
+	u32 stream_enc_status;
+
+	stream_enc_status =  trasncoder_to_stream_enc_status(hdcp->stream_transcoder);
+
+	if (!stream_enc_status)
+		return -EINVAL;
+
+	/* Wait for encryption confirmation */
+	if (intel_de_wait_for_set(i915,
+				  HDCP_STATUS(i915, cpu_transcoder, port),
+				  stream_enc_status,
+				  ENCRYPT_STATUS_CHANGE_TIMEOUT_MS)) {
+		drm_err(&i915->drm, "Timed out waiting for stream encryption enabled\n");
+		return -ETIMEDOUT;
+	}
+
+	return 0;
+}
+
 static
 bool intel_dp_mst_hdcp_check_link(struct intel_digital_port *dig_port,
 				  struct intel_connector *connector)
@@ -673,7 +724,9 @@ static const struct intel_hdcp_shim intel_dp_mst_hdcp_shim = {
 	.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,
+	.toggle_signalling = intel_dp_hdcp_toggle_signalling,
+	.toggle_select_hdcp = intel_dp_mst_toggle_select_hdcp_strem,
+	.stream_encryption = intel_dp_mst_hdcp_strem_encryption,
 	.check_link = intel_dp_mst_hdcp_check_link,
 	.hdcp_capable = intel_dp_hdcp_capable,
 
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index b6424bf5d544..8d06931e0805 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -564,7 +564,7 @@ static void intel_mst_enable_dp(struct intel_atomic_state *state,
 	if (conn_state->content_protection ==
 	    DRM_MODE_CONTENT_PROTECTION_DESIRED)
 		intel_hdcp_enable(to_intel_connector(conn_state->connector),
-				  pipe_config->cpu_transcoder,
+				  pipe_config,
 				  (u8)conn_state->hdcp_content_type);
 }
 
@@ -811,7 +811,7 @@ static struct drm_connector *intel_dp_add_mst_connector(struct drm_dp_mst_topolo
 
 
 	/* TODO: Figure out how to make HDCP work on GEN12+ */
-	if (INTEL_GEN(dev_priv) < 12) {
+	if (INTEL_GEN(dev_priv) <= 12) {
 		ret = intel_dp_init_hdcp(dig_port, intel_connector);
 		if (ret)
 			DRM_DEBUG_KMS("HDCP init failed, skipping.\n");
diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
index 5492076d1ae0..1436fb2910d4 100644
--- a/drivers/gpu/drm/i915/display/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
@@ -23,7 +23,6 @@
 #include "intel_connector.h"
 
 #define KEY_LOAD_TRIES	5
-#define ENCRYPT_STATUS_CHANGE_TIMEOUT_MS	50
 #define HDCP2_LC_RETRY_CNT			3
 
 static
@@ -700,6 +699,7 @@ static int intel_hdcp_auth(struct intel_connector *connector)
 	ret = shim->repeater_present(dig_port, &repeater_present);
 	if (ret)
 		return ret;
+
 	if (repeater_present)
 		intel_de_write(dev_priv, HDCP_REP_CTL,
 			       intel_hdcp_get_repeater_ctl(dev_priv, cpu_transcoder, port));
@@ -771,6 +771,11 @@ static int intel_hdcp_auth(struct intel_connector *connector)
 	 * XXX: If we have MST-connected devices, we need to enable encryption
 	 * on those as well.
 	 */
+	if (shim->toggle_select_hdcp)
+		ret = shim->toggle_select_hdcp(dig_port, true);
+
+	if (shim->stream_encryption)
+		ret = shim->stream_encryption(dig_port);
 
 	if (repeater_present)
 		return intel_hdcp_auth_downstream(connector);
@@ -797,12 +802,13 @@ static int _intel_hdcp_disable(struct intel_connector *connector)
 	 * it. Instead, toggle the HDCP signalling off on that particular
 	 * connector/pipe and exit.
 	 */
-	if (dig_port->num_hdcp_streams > 0) {
-		ret = hdcp->shim->toggle_signalling(dig_port,
-						    cpu_transcoder, false);
-		if (ret)
-			DRM_ERROR("Failed to disable HDCP signalling\n");
-		return ret;
+	if (intel_dig_port->num_hdcp_streams > 0) {
+		if (hdcp->shim->toggle_select_hdcp) {
+			ret = hdcp->shim->toggle_select_hdcp(dig_port, false);
+			if (ret)
+				DRM_ERROR("Failed to disable HDCP signalling\n");
+			return ret;
+		}
 	}
 
 	hdcp->hdcp_encrypted = false;
@@ -2072,7 +2078,7 @@ int intel_hdcp_init(struct intel_connector *connector,
 }
 
 int intel_hdcp_enable(struct intel_connector *connector,
-		      enum transcoder cpu_transcoder, u8 content_type)
+		      const struct intel_crtc_state *pipe_config, u8 content_type)
 {
 	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
 	struct intel_digital_port *dig_port = intel_attached_dig_port(connector);
@@ -2088,10 +2094,17 @@ int intel_hdcp_enable(struct intel_connector *connector,
 	drm_WARN_ON(&dev_priv->drm,
 		    hdcp->value == DRM_MODE_CONTENT_PROTECTION_ENABLED);
 	hdcp->content_type = content_type;
-	hdcp->cpu_transcoder = cpu_transcoder;
+
+	if (intel_crtc_has_type(pipe_config, INTEL_OUTPUT_DP_MST)) {
+		hdcp->cpu_transcoder = pipe_config->mst_master_transcoder;
+		hdcp->stream_transcoder = pipe_config->cpu_transcoder;
+	} else {
+		hdcp->cpu_transcoder = pipe_config->cpu_transcoder;
+		hdcp->stream_transcoder = INVALID_TRANSCODER;
+	}
 
 	if (INTEL_GEN(dev_priv) >= 12)
-		hdcp->port_data.fw_tc = intel_get_mei_fw_tc(cpu_transcoder);
+		hdcp->port_data.fw_tc = intel_get_mei_fw_tc(hdcp->cpu_transcoder);
 
 	/*
 	 * Considering that HDCP2.2 is more secure than HDCP1.4, If the setup
@@ -2202,7 +2215,7 @@ void intel_hdcp_update_pipe(struct intel_atomic_state *state,
 
 	if (desired_and_not_enabled || content_protection_type_changed)
 		intel_hdcp_enable(connector,
-				  crtc_state->cpu_transcoder,
+				  crtc_state,
 				  (u8)conn_state->hdcp_content_type);
 }
 
diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.h b/drivers/gpu/drm/i915/display/intel_hdcp.h
index 1bbf5b67ed0a..36a1b81aca16 100644
--- a/drivers/gpu/drm/i915/display/intel_hdcp.h
+++ b/drivers/gpu/drm/i915/display/intel_hdcp.h
@@ -19,13 +19,15 @@ struct intel_hdcp_shim;
 enum port;
 enum transcoder;
 
+#define ENCRYPT_STATUS_CHANGE_TIMEOUT_MS	50
+
 void intel_hdcp_atomic_check(struct drm_connector *connector,
 			     struct drm_connector_state *old_state,
 			     struct drm_connector_state *new_state);
 int intel_hdcp_init(struct intel_connector *connector, enum port port,
 		    const struct intel_hdcp_shim *hdcp_shim);
 int intel_hdcp_enable(struct intel_connector *connector,
-		      enum transcoder cpu_transcoder, u8 content_type);
+		      const struct intel_crtc_state *pipe_config, u8 content_type);
 int intel_hdcp_disable(struct intel_connector *connector);
 void intel_hdcp_update_pipe(struct intel_atomic_state *state,
 			    struct intel_encoder *encoder,
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 0978b0d8f4c6..955d2250b86f 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -1495,15 +1495,18 @@ static int kbl_repositioning_enc_en_signal(struct intel_connector *connector,
 		usleep_range(25, 50);
 	}
 
-	ret = intel_ddi_toggle_hdcp_signalling(&dig_port->base, cpu_transcoder,
-					       false);
+	ret = intel_ddi_toggle_hdcp_bits(&dig_port->base, cpu_transcoder,
+					 false, TRANS_DDI_HDCP_SIGNALLING);
+
 	if (ret) {
 		drm_err(&dev_priv->drm,
 			"Disable HDCP signalling failed (%d)\n", ret);
 		return ret;
 	}
-	ret = intel_ddi_toggle_hdcp_signalling(&dig_port->base, cpu_transcoder,
-					       true);
+
+	ret = intel_ddi_toggle_hdcp_bits(&dig_port->base, cpu_transcoder,
+					 true, TRANS_DDI_HDCP_SIGNALLING);
+
 	if (ret) {
 		drm_err(&dev_priv->drm,
 			"Enable HDCP signalling failed (%d)\n", ret);
@@ -1526,8 +1529,9 @@ int intel_hdmi_hdcp_toggle_signalling(struct intel_digital_port *dig_port,
 	if (!enable)
 		usleep_range(6, 60); /* Bspec says >= 6us */
 
-	ret = intel_ddi_toggle_hdcp_signalling(&dig_port->base, cpu_transcoder,
-					       enable);
+	ret = intel_ddi_toggle_hdcp_bits(&dig_port->base,
+					 cpu_transcoder, enable,
+					 TRANS_DDI_HDCP_SIGNALLING);
 	if (ret) {
 		drm_err(&dev_priv->drm, "%s HDCP signalling failed (%d)\n",
 			enable ? "Enable" : "Disable", ret);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index ab4b1abd4364..f6e40a458f7b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -9945,6 +9945,7 @@ enum skl_power_gate {
 #define  TRANS_DDI_DP_VC_PAYLOAD_ALLOC	(1 << 8)
 #define  TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE (1 << 7)
 #define  TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ (1 << 6)
+#define  TRANS_DDI_HDCP_SELECT	(1 << 5)
 #define  TRANS_DDI_BFI_ENABLE		(1 << 4)
 #define  TRANS_DDI_HIGH_TMDS_CHAR_RATE	(1 << 4)
 #define  TRANS_DDI_HDMI_SCRAMBLING	(1 << 0)
-- 
2.26.2

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

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

* [Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/i915/hdcp: Gen12 HDCP 1.4 support over DP MST
  2020-09-01 12:10 [Intel-gfx] [RFC] drm/i915/hdcp: Gen12 HDCP 1.4 support over DP MST Anshuman Gupta
@ 2020-09-01 12:40 ` Patchwork
  2020-09-01 13:57 ` [Intel-gfx] [RFC] " Sean Paul
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2020-09-01 12:40 UTC (permalink / raw)
  To: Anshuman Gupta; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/hdcp: Gen12 HDCP 1.4 support over DP MST
URL   : https://patchwork.freedesktop.org/series/81222/
State : failure

== Summary ==

CALL    scripts/checksyscalls.sh
  CALL    scripts/atomic/check-atomics.sh
  DESCEND  objtool
  CHK     include/generated/compile.h
  CC [M]  drivers/gpu/drm/i915/display/intel_hdcp.o
drivers/gpu/drm/i915/display/intel_hdcp.c: In function ‘_intel_hdcp_disable’:
drivers/gpu/drm/i915/display/intel_hdcp.c:805:6: error: ‘intel_dig_port’ undeclared (first use in this function); did you mean ‘intel_digital_port’?
  if (intel_dig_port->num_hdcp_streams > 0) {
      ^~~~~~~~~~~~~~
      intel_digital_port
drivers/gpu/drm/i915/display/intel_hdcp.c:805:6: note: each undeclared identifier is reported only once for each function it appears in
scripts/Makefile.build:283: recipe for target 'drivers/gpu/drm/i915/display/intel_hdcp.o' failed
make[4]: *** [drivers/gpu/drm/i915/display/intel_hdcp.o] Error 1
scripts/Makefile.build:500: recipe for target 'drivers/gpu/drm/i915' failed
make[3]: *** [drivers/gpu/drm/i915] Error 2
scripts/Makefile.build:500: recipe for target 'drivers/gpu/drm' failed
make[2]: *** [drivers/gpu/drm] Error 2
scripts/Makefile.build:500: recipe for target 'drivers/gpu' failed
make[1]: *** [drivers/gpu] Error 2
Makefile:1788: recipe for target 'drivers' failed
make: *** [drivers] Error 2


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

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

* Re: [Intel-gfx] [RFC] drm/i915/hdcp: Gen12 HDCP 1.4 support over DP MST
  2020-09-01 12:10 [Intel-gfx] [RFC] drm/i915/hdcp: Gen12 HDCP 1.4 support over DP MST Anshuman Gupta
  2020-09-01 12:40 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for " Patchwork
@ 2020-09-01 13:57 ` Sean Paul
  2020-09-02  7:45   ` Anshuman Gupta
  2020-09-01 15:21 ` kernel test robot
  2020-09-01 17:45 ` kernel test robot
  3 siblings, 1 reply; 6+ messages in thread
From: Sean Paul @ 2020-09-01 13:57 UTC (permalink / raw)
  To: Anshuman Gupta; +Cc: Intel Graphics Development, Sean Paul

On Tue, Sep 1, 2020 at 8:22 AM Anshuman Gupta <anshuman.gupta@intel.com> wrote:
>

Hi Anshuman,
Thank you for sending this along! I have a few comments below.

> Gen12 has measure changes with respect to HDCP display
> engine instaces lies in Trascoder insead of DDI as in Gen11.

*instances
*transcoder
*instead

>
> This requires hdcp driver to use mst_master_transcoder for link
> authentication and stream trascoder for stream encryption

*transcoder

> separately.
>
> It also requires to validate the stream encryption status
> in HDCP_STATUS_{TRANSCODER,PORT} driving that link register.
>
> There is also some changes over existing HDCP 1.4  DP MST Gen11
> implementation, related to Multistream HDCP Select bit in
> TRANS_DDI_FUNC_CTL need to be required with respect to B.Spec
> Documentation.
>
> Cc: Ramalingam C <ramalingam.c@intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c      | 12 +--
>  drivers/gpu/drm/i915/display/intel_ddi.h      |  6 +-
>  .../drm/i915/display/intel_display_types.h    |  9 +++
>  drivers/gpu/drm/i915/display/intel_dp_hdcp.c  | 73 ++++++++++++++++---
>  drivers/gpu/drm/i915/display/intel_dp_mst.c   |  4 +-
>  drivers/gpu/drm/i915/display/intel_hdcp.c     | 35 ++++++---
>  drivers/gpu/drm/i915/display/intel_hdcp.h     |  4 +-
>  drivers/gpu/drm/i915/display/intel_hdmi.c     | 16 ++--
>  drivers/gpu/drm/i915/i915_reg.h               |  1 +
>  9 files changed, 121 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index a2b7dcf84430..5d6e4fd7bccd 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -1849,9 +1849,9 @@ void intel_ddi_disable_transcoder_func(const struct intel_crtc_state *crtc_state
>         }
>  }
>
> -int intel_ddi_toggle_hdcp_signalling(struct intel_encoder *intel_encoder,
> -                                    enum transcoder cpu_transcoder,
> -                                    bool enable)
> +int intel_ddi_toggle_hdcp_bits(struct intel_encoder *intel_encoder,
> +                              enum transcoder cpu_transcoder,
> +                              bool enable, u32 hdcp_mask)
>  {
>         struct drm_device *dev = intel_encoder->base.dev;
>         struct drm_i915_private *dev_priv = to_i915(dev);
> @@ -1866,9 +1866,9 @@ int intel_ddi_toggle_hdcp_signalling(struct intel_encoder *intel_encoder,
>
>         tmp = intel_de_read(dev_priv, TRANS_DDI_FUNC_CTL(cpu_transcoder));
>         if (enable)
> -               tmp |= TRANS_DDI_HDCP_SIGNALLING;
> +               tmp |= hdcp_mask;
>         else
> -               tmp &= ~TRANS_DDI_HDCP_SIGNALLING;
> +               tmp &= ~hdcp_mask;
>         intel_de_write(dev_priv, TRANS_DDI_FUNC_CTL(cpu_transcoder), tmp);
>         intel_display_power_put(dev_priv, intel_encoder->power_domain, wakeref);
>         return ret;
> @@ -3967,7 +3967,7 @@ static void intel_enable_ddi(struct intel_atomic_state *state,
>         if (conn_state->content_protection ==
>             DRM_MODE_CONTENT_PROTECTION_DESIRED)
>                 intel_hdcp_enable(to_intel_connector(conn_state->connector),
> -                                 crtc_state->cpu_transcoder,
> +                                 crtc_state,
>                                   (u8)conn_state->hdcp_content_type);
>  }
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.h b/drivers/gpu/drm/i915/display/intel_ddi.h
> index f5fb62fc9400..69d9e495992c 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.h
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.h
> @@ -43,9 +43,9 @@ void intel_ddi_compute_min_voltage_level(struct drm_i915_private *dev_priv,
>                                          struct intel_crtc_state *crtc_state);
>  u32 bxt_signal_levels(struct intel_dp *intel_dp);
>  u32 ddi_signal_levels(struct intel_dp *intel_dp);
> -int intel_ddi_toggle_hdcp_signalling(struct intel_encoder *intel_encoder,
> -                                    enum transcoder cpu_transcoder,
> -                                    bool enable);
> +int intel_ddi_toggle_hdcp_bits(struct intel_encoder *intel_encoder,
> +                              enum transcoder cpu_transcoder,
> +                              bool enable, u32 hdcp_mask);
>  void icl_sanitize_encoder_pll_mapping(struct intel_encoder *encoder);
>
>  #endif /* __INTEL_DDI_H__ */
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 413b60337a0b..dc71ee4d314a 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -317,6 +317,13 @@ struct intel_hdcp_shim {
>                                  enum transcoder cpu_transcoder,
>                                  bool enable);
>
> +       /* Select/Deselect HDCP stream on the port DP MST Transport Link */
> +       int (*toggle_select_hdcp)(struct intel_digital_port *intel_dig_port,
> +                                 bool enable);
> +
> +       /* Enable HDCP stream encyption on DP MST Transport Link */

*encryption

> +       int (*stream_encryption)(struct intel_digital_port *intel_dig_port);
> +
>         /* Ensures the link is still protected */
>         bool (*check_link)(struct intel_digital_port *dig_port,
>                            struct intel_connector *connector);
> @@ -410,6 +417,8 @@ struct intel_hdcp {
>          * Hence caching the transcoder here.
>          */
>         enum transcoder cpu_transcoder;
> +       /* Only used for DP MST stream encryption */
> +       enum transcoder stream_transcoder;
>  };
>
>  struct intel_connector {
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
> index 03424d20e9f7..8a6427f3690b 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
> @@ -16,6 +16,30 @@
>  #include "intel_dp.h"
>  #include "intel_hdcp.h"
>
> +static unsigned int trasncoder_to_stream_enc_status(enum transcoder cpu_transcoder)

*transcoder

> +{
> +       u32 stream_enc_mask;
> +
> +       switch (cpu_transcoder) {
> +       case TRANSCODER_A:
> +               stream_enc_mask = HDCP_STATUS_STREAM_A_ENC;
> +               break;
> +       case TRANSCODER_B:
> +               stream_enc_mask = HDCP_STATUS_STREAM_B_ENC;
> +               break;
> +       case TRANSCODER_C:
> +               stream_enc_mask = HDCP_STATUS_STREAM_C_ENC;
> +               break;
> +       case TRANSCODER_D:
> +               stream_enc_mask = HDCP_STATUS_STREAM_D_ENC;
> +               break;
> +       default:
> +               stream_enc_mask = 0;
> +       }
> +
> +       return stream_enc_mask;
> +}
> +
>  static void intel_dp_hdcp_wait_for_cp_irq(struct intel_hdcp *hdcp, int timeout)
>  {
>         long ret;
> @@ -622,24 +646,51 @@ static const struct intel_hdcp_shim intel_dp_hdcp_shim = {
>  };
>
>  static int
> -intel_dp_mst_hdcp_toggle_signalling(struct intel_digital_port *dig_port,
> -                                   enum transcoder cpu_transcoder,
> -                                   bool enable)
> +intel_dp_mst_toggle_select_hdcp_strem(struct intel_digital_port *dig_port,
> +                                     bool enable)

*stream

>  {
>         struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
> +       struct intel_dp *dp = &dig_port->dp;
> +       struct intel_hdcp *hdcp = &dp->attached_connector->hdcp;
>         int ret;
>
> -       if (!enable)
> -               usleep_range(6, 60); /* Bspec says >= 6us */

Is this no longer needed on older generations?

> +       ret = intel_ddi_toggle_hdcp_bits(&dig_port->base,
> +                                        hdcp->stream_transcoder, enable,
> +                                        TRANS_DDI_HDCP_SELECT);
>

Remove blank line

> -       ret = intel_ddi_toggle_hdcp_signalling(&dig_port->base,
> -                                              cpu_transcoder, enable);
>         if (ret)
> -               drm_dbg_kms(&i915->drm, "%s HDCP signalling failed (%d)\n",
> -                             enable ? "Enable" : "Disable", ret);
> +               drm_dbg_kms(&i915->drm, "%s HDCP select failed (%d)\n",
> +                           enable ? "Enable" : "Disable", ret);
>         return ret;
>  }
>
> +static int
> +intel_dp_mst_hdcp_strem_encryption(struct intel_digital_port *dig_port)
> +{
> +       struct drm_i915_private *i915 = to_i915(idig_port->base.base.dev);
> +       struct intel_dp *dp = &dig_port->dp;
> +       struct intel_hdcp *hdcp = &dp->attached_connector->hdcp;
> +       enum port port = dig_port->base.port;
> +       enum transcoder cpu_transcoder = hdcp->cpu_transcoder;
> +       u32 stream_enc_status;
> +
> +       stream_enc_status =  trasncoder_to_stream_enc_status(hdcp->stream_transcoder);
> +

Remove blank line

> +       if (!stream_enc_status)
> +               return -EINVAL;
> +
> +       /* Wait for encryption confirmation */
> +       if (intel_de_wait_for_set(i915,
> +                                 HDCP_STATUS(i915, cpu_transcoder, port),
> +                                 stream_enc_status,
> +                                 ENCRYPT_STATUS_CHANGE_TIMEOUT_MS)) {
> +               drm_err(&i915->drm, "Timed out waiting for stream encryption enabled\n");
> +               return -ETIMEDOUT;
> +       }
> +
> +       return 0;
> +}
> +
>  static
>  bool intel_dp_mst_hdcp_check_link(struct intel_digital_port *dig_port,
>                                   struct intel_connector *connector)
> @@ -673,7 +724,9 @@ static const struct intel_hdcp_shim intel_dp_mst_hdcp_shim = {
>         .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,
> +       .toggle_signalling = intel_dp_hdcp_toggle_signalling,
> +       .toggle_select_hdcp = intel_dp_mst_toggle_select_hdcp_strem,
> +       .stream_encryption = intel_dp_mst_hdcp_strem_encryption,
>         .check_link = intel_dp_mst_hdcp_check_link,
>         .hdcp_capable = intel_dp_hdcp_capable,
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index b6424bf5d544..8d06931e0805 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -564,7 +564,7 @@ static void intel_mst_enable_dp(struct intel_atomic_state *state,
>         if (conn_state->content_protection ==
>             DRM_MODE_CONTENT_PROTECTION_DESIRED)
>                 intel_hdcp_enable(to_intel_connector(conn_state->connector),
> -                                 pipe_config->cpu_transcoder,
> +                                 pipe_config,
>                                   (u8)conn_state->hdcp_content_type);
>  }
>
> @@ -811,7 +811,7 @@ static struct drm_connector *intel_dp_add_mst_connector(struct drm_dp_mst_topolo
>
>
>         /* TODO: Figure out how to make HDCP work on GEN12+ */

I think this comment is no longer valid

> -       if (INTEL_GEN(dev_priv) < 12) {
> +       if (INTEL_GEN(dev_priv) <= 12) {

Is there any benefit to limiting this any longer? Perhaps just delete this now.

>                 ret = intel_dp_init_hdcp(dig_port, intel_connector);
>                 if (ret)
>                         DRM_DEBUG_KMS("HDCP init failed, skipping.\n");
> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
> index 5492076d1ae0..1436fb2910d4 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> @@ -23,7 +23,6 @@
>  #include "intel_connector.h"
>
>  #define KEY_LOAD_TRIES 5
> -#define ENCRYPT_STATUS_CHANGE_TIMEOUT_MS       50
>  #define HDCP2_LC_RETRY_CNT                     3
>
>  static
> @@ -700,6 +699,7 @@ static int intel_hdcp_auth(struct intel_connector *connector)
>         ret = shim->repeater_present(dig_port, &repeater_present);
>         if (ret)
>                 return ret;
> +
>         if (repeater_present)
>                 intel_de_write(dev_priv, HDCP_REP_CTL,
>                                intel_hdcp_get_repeater_ctl(dev_priv, cpu_transcoder, port));
> @@ -771,6 +771,11 @@ static int intel_hdcp_auth(struct intel_connector *connector)
>          * XXX: If we have MST-connected devices, we need to enable encryption
>          * on those as well.
>          */

This comment is also obsolete now.

> +       if (shim->toggle_select_hdcp)
> +               ret = shim->toggle_select_hdcp(dig_port, true);
> +
> +       if (shim->stream_encryption)
> +               ret = shim->stream_encryption(dig_port);

Instead of adding 2 new hooks, couldn't you just combine these into 1?

>
>         if (repeater_present)
>                 return intel_hdcp_auth_downstream(connector);
> @@ -797,12 +802,13 @@ static int _intel_hdcp_disable(struct intel_connector *connector)
>          * it. Instead, toggle the HDCP signalling off on that particular
>          * connector/pipe and exit.
>          */
> -       if (dig_port->num_hdcp_streams > 0) {
> -               ret = hdcp->shim->toggle_signalling(dig_port,
> -                                                   cpu_transcoder, false);
> -               if (ret)
> -                       DRM_ERROR("Failed to disable HDCP signalling\n");
> -               return ret;
> +       if (intel_dig_port->num_hdcp_streams > 0) {
> +               if (hdcp->shim->toggle_select_hdcp) {

Combining these with &&?

> +                       ret = hdcp->shim->toggle_select_hdcp(dig_port, false);
> +                       if (ret)
> +                               DRM_ERROR("Failed to disable HDCP signalling\n");
> +                       return ret;
> +               }
>         }
>
>         hdcp->hdcp_encrypted = false;
> @@ -2072,7 +2078,7 @@ int intel_hdcp_init(struct intel_connector *connector,
>  }
>
>  int intel_hdcp_enable(struct intel_connector *connector,
> -                     enum transcoder cpu_transcoder, u8 content_type)
> +                     const struct intel_crtc_state *pipe_config, u8 content_type)
>  {
>         struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>         struct intel_digital_port *dig_port = intel_attached_dig_port(connector);
> @@ -2088,10 +2094,17 @@ int intel_hdcp_enable(struct intel_connector *connector,
>         drm_WARN_ON(&dev_priv->drm,
>                     hdcp->value == DRM_MODE_CONTENT_PROTECTION_ENABLED);
>         hdcp->content_type = content_type;
> -       hdcp->cpu_transcoder = cpu_transcoder;
> +
> +       if (intel_crtc_has_type(pipe_config, INTEL_OUTPUT_DP_MST)) {
> +               hdcp->cpu_transcoder = pipe_config->mst_master_transcoder;
> +               hdcp->stream_transcoder = pipe_config->cpu_transcoder;
> +       } else {
> +               hdcp->cpu_transcoder = pipe_config->cpu_transcoder;
> +               hdcp->stream_transcoder = INVALID_TRANSCODER;
> +       }
>
>         if (INTEL_GEN(dev_priv) >= 12)
> -               hdcp->port_data.fw_tc = intel_get_mei_fw_tc(cpu_transcoder);
> +               hdcp->port_data.fw_tc = intel_get_mei_fw_tc(hdcp->cpu_transcoder);
>
>         /*
>          * Considering that HDCP2.2 is more secure than HDCP1.4, If the setup
> @@ -2202,7 +2215,7 @@ void intel_hdcp_update_pipe(struct intel_atomic_state *state,
>
>         if (desired_and_not_enabled || content_protection_type_changed)
>                 intel_hdcp_enable(connector,
> -                                 crtc_state->cpu_transcoder,
> +                                 crtc_state,
>                                   (u8)conn_state->hdcp_content_type);
>  }
>
> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.h b/drivers/gpu/drm/i915/display/intel_hdcp.h
> index 1bbf5b67ed0a..36a1b81aca16 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdcp.h
> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.h
> @@ -19,13 +19,15 @@ struct intel_hdcp_shim;
>  enum port;
>  enum transcoder;
>
> +#define ENCRYPT_STATUS_CHANGE_TIMEOUT_MS       50

Now that this is exposed in a header, best to make it more descriptive:

#define HDCP_ENCRYPT_STATUS_CHANGE_TIMEOUT_MS

> +
>  void intel_hdcp_atomic_check(struct drm_connector *connector,
>                              struct drm_connector_state *old_state,
>                              struct drm_connector_state *new_state);
>  int intel_hdcp_init(struct intel_connector *connector, enum port port,
>                     const struct intel_hdcp_shim *hdcp_shim);
>  int intel_hdcp_enable(struct intel_connector *connector,
> -                     enum transcoder cpu_transcoder, u8 content_type);
> +                     const struct intel_crtc_state *pipe_config, u8 content_type);
>  int intel_hdcp_disable(struct intel_connector *connector);
>  void intel_hdcp_update_pipe(struct intel_atomic_state *state,
>                             struct intel_encoder *encoder,
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index 0978b0d8f4c6..955d2250b86f 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -1495,15 +1495,18 @@ static int kbl_repositioning_enc_en_signal(struct intel_connector *connector,
>                 usleep_range(25, 50);
>         }
>
> -       ret = intel_ddi_toggle_hdcp_signalling(&dig_port->base, cpu_transcoder,
> -                                              false);
> +       ret = intel_ddi_toggle_hdcp_bits(&dig_port->base, cpu_transcoder,
> +                                        false, TRANS_DDI_HDCP_SIGNALLING);
> +

Remove blank line

>         if (ret) {
>                 drm_err(&dev_priv->drm,
>                         "Disable HDCP signalling failed (%d)\n", ret);
>                 return ret;
>         }
> -       ret = intel_ddi_toggle_hdcp_signalling(&dig_port->base, cpu_transcoder,
> -                                              true);
> +
> +       ret = intel_ddi_toggle_hdcp_bits(&dig_port->base, cpu_transcoder,
> +                                        true, TRANS_DDI_HDCP_SIGNALLING);
> +

Remove blank line

>         if (ret) {
>                 drm_err(&dev_priv->drm,
>                         "Enable HDCP signalling failed (%d)\n", ret);
> @@ -1526,8 +1529,9 @@ int intel_hdmi_hdcp_toggle_signalling(struct intel_digital_port *dig_port,
>         if (!enable)
>                 usleep_range(6, 60); /* Bspec says >= 6us */
>
> -       ret = intel_ddi_toggle_hdcp_signalling(&dig_port->base, cpu_transcoder,
> -                                              enable);
> +       ret = intel_ddi_toggle_hdcp_bits(&dig_port->base,
> +                                        cpu_transcoder, enable,
> +                                        TRANS_DDI_HDCP_SIGNALLING);
>         if (ret) {
>                 drm_err(&dev_priv->drm, "%s HDCP signalling failed (%d)\n",
>                         enable ? "Enable" : "Disable", ret);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index ab4b1abd4364..f6e40a458f7b 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -9945,6 +9945,7 @@ enum skl_power_gate {
>  #define  TRANS_DDI_DP_VC_PAYLOAD_ALLOC (1 << 8)
>  #define  TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE (1 << 7)
>  #define  TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ (1 << 6)
> +#define  TRANS_DDI_HDCP_SELECT (1 << 5)
>  #define  TRANS_DDI_BFI_ENABLE          (1 << 4)
>  #define  TRANS_DDI_HIGH_TMDS_CHAR_RATE (1 << 4)
>  #define  TRANS_DDI_HDMI_SCRAMBLING     (1 << 0)
> --
> 2.26.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [RFC] drm/i915/hdcp: Gen12 HDCP 1.4 support over DP MST
  2020-09-01 12:10 [Intel-gfx] [RFC] drm/i915/hdcp: Gen12 HDCP 1.4 support over DP MST Anshuman Gupta
  2020-09-01 12:40 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for " Patchwork
  2020-09-01 13:57 ` [Intel-gfx] [RFC] " Sean Paul
@ 2020-09-01 15:21 ` kernel test robot
  2020-09-01 17:45 ` kernel test robot
  3 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2020-09-01 15:21 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 4679 bytes --]

Hi Anshuman,

[FYI, it's a private test report for your RFC patch.]
[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on drm-tip/drm-tip]
[cannot apply to v5.9-rc3 next-20200828]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Anshuman-Gupta/drm-i915-hdcp-Gen12-HDCP-1-4-support-over-DP-MST/20200901-202424
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-randconfig-a006-20200901 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce (this is a W=1 build):
        # save the attached .config to linux build tree
        make W=1 ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/gpu/drm/i915/display/intel_hdcp.c: In function '_intel_hdcp_disable':
>> drivers/gpu/drm/i915/display/intel_hdcp.c:805:6: error: 'intel_dig_port' undeclared (first use in this function); did you mean 'intel_digital_port'?
     805 |  if (intel_dig_port->num_hdcp_streams > 0) {
         |      ^~~~~~~~~~~~~~
         |      intel_digital_port
   drivers/gpu/drm/i915/display/intel_hdcp.c:805:6: note: each undeclared identifier is reported only once for each function it appears in
--
   drivers/gpu/drm/i915/display/intel_dp_hdcp.c: In function 'intel_dp_mst_hdcp_strem_encryption':
>> drivers/gpu/drm/i915/display/intel_dp_hdcp.c:670:42: error: 'idig_port' undeclared (first use in this function); did you mean 'dig_port'?
     670 |  struct drm_i915_private *i915 = to_i915(idig_port->base.base.dev);
         |                                          ^~~~~~~~~
         |                                          dig_port
   drivers/gpu/drm/i915/display/intel_dp_hdcp.c:670:42: note: each undeclared identifier is reported only once for each function it appears in

# https://github.com/0day-ci/linux/commit/d6c89b9a28b4d968e8b014579048586fc79214dc
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Anshuman-Gupta/drm-i915-hdcp-Gen12-HDCP-1-4-support-over-DP-MST/20200901-202424
git checkout d6c89b9a28b4d968e8b014579048586fc79214dc
vim +805 drivers/gpu/drm/i915/display/intel_hdcp.c

   786	
   787	static int _intel_hdcp_disable(struct intel_connector *connector)
   788	{
   789		struct intel_digital_port *dig_port = intel_attached_dig_port(connector);
   790		struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
   791		struct intel_hdcp *hdcp = &connector->hdcp;
   792		enum port port = dig_port->base.port;
   793		enum transcoder cpu_transcoder = hdcp->cpu_transcoder;
   794		u32 repeater_ctl;
   795		int ret;
   796	
   797		drm_dbg_kms(&dev_priv->drm, "[%s:%d] HDCP is being disabled...\n",
   798			    connector->base.name, connector->base.base.id);
   799	
   800		/*
   801		 * If there are other connectors on this port using HDCP, don't disable
   802		 * it. Instead, toggle the HDCP signalling off on that particular
   803		 * connector/pipe and exit.
   804		 */
 > 805		if (intel_dig_port->num_hdcp_streams > 0) {
   806			if (hdcp->shim->toggle_select_hdcp) {
   807				ret = hdcp->shim->toggle_select_hdcp(dig_port, false);
   808				if (ret)
   809					DRM_ERROR("Failed to disable HDCP signalling\n");
   810				return ret;
   811			}
   812		}
   813	
   814		hdcp->hdcp_encrypted = false;
   815		intel_de_write(dev_priv, HDCP_CONF(dev_priv, cpu_transcoder, port), 0);
   816		if (intel_de_wait_for_clear(dev_priv,
   817					    HDCP_STATUS(dev_priv, cpu_transcoder, port),
   818					    ~0, ENCRYPT_STATUS_CHANGE_TIMEOUT_MS)) {
   819			drm_err(&dev_priv->drm,
   820				"Failed to disable HDCP, timeout clearing status\n");
   821			return -ETIMEDOUT;
   822		}
   823	
   824		repeater_ctl = intel_hdcp_get_repeater_ctl(dev_priv, cpu_transcoder,
   825							   port);
   826		intel_de_write(dev_priv, HDCP_REP_CTL,
   827			       intel_de_read(dev_priv, HDCP_REP_CTL) & ~repeater_ctl);
   828	
   829		ret = hdcp->shim->toggle_signalling(dig_port, cpu_transcoder, false);
   830		if (ret) {
   831			drm_err(&dev_priv->drm, "Failed to disable HDCP signalling\n");
   832			return ret;
   833		}
   834	
   835		drm_dbg_kms(&dev_priv->drm, "HDCP is disabled\n");
   836		return 0;
   837	}
   838	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 31247 bytes --]

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

* Re: [Intel-gfx] [RFC] drm/i915/hdcp: Gen12 HDCP 1.4 support over DP MST
  2020-09-01 12:10 [Intel-gfx] [RFC] drm/i915/hdcp: Gen12 HDCP 1.4 support over DP MST Anshuman Gupta
                   ` (2 preceding siblings ...)
  2020-09-01 15:21 ` kernel test robot
@ 2020-09-01 17:45 ` kernel test robot
  3 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2020-09-01 17:45 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 3880 bytes --]

Hi Anshuman,

[FYI, it's a private test report for your RFC patch.]
[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on drm-tip/drm-tip]
[cannot apply to v5.9-rc3 next-20200828]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Anshuman-Gupta/drm-i915-hdcp-Gen12-HDCP-1-4-support-over-DP-MST/20200901-202424
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-randconfig-a012-20200901 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project c10e63677f5d20f18010f8f68c631ddc97546f7d)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/gpu/drm/i915/display/intel_dp_hdcp.c:670:42: error: use of undeclared identifier 'idig_port'; did you mean 'dig_port'?
           struct drm_i915_private *i915 = to_i915(idig_port->base.base.dev);
                                                   ^~~~~~~~~
                                                   dig_port
   drivers/gpu/drm/i915/display/intel_dp_hdcp.c:668:63: note: 'dig_port' declared here
   intel_dp_mst_hdcp_strem_encryption(struct intel_digital_port *dig_port)
                                                                 ^
   1 error generated.
--
>> drivers/gpu/drm/i915/display/intel_hdcp.c:805:6: error: use of undeclared identifier 'intel_dig_port'
           if (intel_dig_port->num_hdcp_streams > 0) {
               ^
>> drivers/gpu/drm/i915/display/intel_hdcp.c:805:6: error: use of undeclared identifier 'intel_dig_port'
>> drivers/gpu/drm/i915/display/intel_hdcp.c:805:6: error: use of undeclared identifier 'intel_dig_port'
   3 errors generated.

# https://github.com/0day-ci/linux/commit/d6c89b9a28b4d968e8b014579048586fc79214dc
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Anshuman-Gupta/drm-i915-hdcp-Gen12-HDCP-1-4-support-over-DP-MST/20200901-202424
git checkout d6c89b9a28b4d968e8b014579048586fc79214dc
vim +670 drivers/gpu/drm/i915/display/intel_dp_hdcp.c

   666	
   667	static int
   668	intel_dp_mst_hdcp_strem_encryption(struct intel_digital_port *dig_port)
   669	{
 > 670		struct drm_i915_private *i915 = to_i915(idig_port->base.base.dev);
   671		struct intel_dp *dp = &dig_port->dp;
   672		struct intel_hdcp *hdcp = &dp->attached_connector->hdcp;
   673		enum port port = dig_port->base.port;
   674		enum transcoder cpu_transcoder = hdcp->cpu_transcoder;
   675		u32 stream_enc_status;
   676	
   677		stream_enc_status =  trasncoder_to_stream_enc_status(hdcp->stream_transcoder);
   678	
   679		if (!stream_enc_status)
   680			return -EINVAL;
   681	
   682		/* Wait for encryption confirmation */
   683		if (intel_de_wait_for_set(i915,
   684					  HDCP_STATUS(i915, cpu_transcoder, port),
   685					  stream_enc_status,
   686					  ENCRYPT_STATUS_CHANGE_TIMEOUT_MS)) {
   687			drm_err(&i915->drm, "Timed out waiting for stream encryption enabled\n");
   688			return -ETIMEDOUT;
   689		}
   690	
   691		return 0;
   692	}
   693	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 37394 bytes --]

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

* Re: [Intel-gfx] [RFC] drm/i915/hdcp: Gen12 HDCP 1.4 support over DP MST
  2020-09-01 13:57 ` [Intel-gfx] [RFC] " Sean Paul
@ 2020-09-02  7:45   ` Anshuman Gupta
  0 siblings, 0 replies; 6+ messages in thread
From: Anshuman Gupta @ 2020-09-02  7:45 UTC (permalink / raw)
  To: Sean Paul, ramalingam.c; +Cc: Intel Graphics Development, Sean Paul

On 2020-09-01 at 09:57:35 -0400, Sean Paul wrote:
> On Tue, Sep 1, 2020 at 8:22 AM Anshuman Gupta <anshuman.gupta@intel.com> wrote:
> >
> 
> Hi Anshuman,
> Thank you for sending this along! I have a few comments below.
Thanks sean for your comment.
I have some doubts over ENCRYPT_STATUS_CHANGE_TIMEOUT_MS may be ram can
provide some insight over it.
Thanks,
Anshuman Gupta.
> 
> > Gen12 has measure changes with respect to HDCP display
> > engine instaces lies in Trascoder insead of DDI as in Gen11.
> 
> *instances
> *transcoder
> *instead
> 
> >
> > This requires hdcp driver to use mst_master_transcoder for link
> > authentication and stream trascoder for stream encryption
> 
> *transcoder
> 
> > separately.
> >
> > It also requires to validate the stream encryption status
> > in HDCP_STATUS_{TRANSCODER,PORT} driving that link register.
> >
> > There is also some changes over existing HDCP 1.4  DP MST Gen11
> > implementation, related to Multistream HDCP Select bit in
> > TRANS_DDI_FUNC_CTL need to be required with respect to B.Spec
> > Documentation.
> >
> > Cc: Ramalingam C <ramalingam.c@intel.com>
> > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_ddi.c      | 12 +--
> >  drivers/gpu/drm/i915/display/intel_ddi.h      |  6 +-
> >  .../drm/i915/display/intel_display_types.h    |  9 +++
> >  drivers/gpu/drm/i915/display/intel_dp_hdcp.c  | 73 ++++++++++++++++---
> >  drivers/gpu/drm/i915/display/intel_dp_mst.c   |  4 +-
> >  drivers/gpu/drm/i915/display/intel_hdcp.c     | 35 ++++++---
> >  drivers/gpu/drm/i915/display/intel_hdcp.h     |  4 +-
> >  drivers/gpu/drm/i915/display/intel_hdmi.c     | 16 ++--
> >  drivers/gpu/drm/i915/i915_reg.h               |  1 +
> >  9 files changed, 121 insertions(+), 39 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index a2b7dcf84430..5d6e4fd7bccd 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -1849,9 +1849,9 @@ void intel_ddi_disable_transcoder_func(const struct intel_crtc_state *crtc_state
> >         }
> >  }
> >
> > -int intel_ddi_toggle_hdcp_signalling(struct intel_encoder *intel_encoder,
> > -                                    enum transcoder cpu_transcoder,
> > -                                    bool enable)
> > +int intel_ddi_toggle_hdcp_bits(struct intel_encoder *intel_encoder,
> > +                              enum transcoder cpu_transcoder,
> > +                              bool enable, u32 hdcp_mask)
> >  {
> >         struct drm_device *dev = intel_encoder->base.dev;
> >         struct drm_i915_private *dev_priv = to_i915(dev);
> > @@ -1866,9 +1866,9 @@ int intel_ddi_toggle_hdcp_signalling(struct intel_encoder *intel_encoder,
> >
> >         tmp = intel_de_read(dev_priv, TRANS_DDI_FUNC_CTL(cpu_transcoder));
> >         if (enable)
> > -               tmp |= TRANS_DDI_HDCP_SIGNALLING;
> > +               tmp |= hdcp_mask;
> >         else
> > -               tmp &= ~TRANS_DDI_HDCP_SIGNALLING;
> > +               tmp &= ~hdcp_mask;
> >         intel_de_write(dev_priv, TRANS_DDI_FUNC_CTL(cpu_transcoder), tmp);
> >         intel_display_power_put(dev_priv, intel_encoder->power_domain, wakeref);
> >         return ret;
> > @@ -3967,7 +3967,7 @@ static void intel_enable_ddi(struct intel_atomic_state *state,
> >         if (conn_state->content_protection ==
> >             DRM_MODE_CONTENT_PROTECTION_DESIRED)
> >                 intel_hdcp_enable(to_intel_connector(conn_state->connector),
> > -                                 crtc_state->cpu_transcoder,
> > +                                 crtc_state,
> >                                   (u8)conn_state->hdcp_content_type);
> >  }
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.h b/drivers/gpu/drm/i915/display/intel_ddi.h
> > index f5fb62fc9400..69d9e495992c 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.h
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.h
> > @@ -43,9 +43,9 @@ void intel_ddi_compute_min_voltage_level(struct drm_i915_private *dev_priv,
> >                                          struct intel_crtc_state *crtc_state);
> >  u32 bxt_signal_levels(struct intel_dp *intel_dp);
> >  u32 ddi_signal_levels(struct intel_dp *intel_dp);
> > -int intel_ddi_toggle_hdcp_signalling(struct intel_encoder *intel_encoder,
> > -                                    enum transcoder cpu_transcoder,
> > -                                    bool enable);
> > +int intel_ddi_toggle_hdcp_bits(struct intel_encoder *intel_encoder,
> > +                              enum transcoder cpu_transcoder,
> > +                              bool enable, u32 hdcp_mask);
> >  void icl_sanitize_encoder_pll_mapping(struct intel_encoder *encoder);
> >
> >  #endif /* __INTEL_DDI_H__ */
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index 413b60337a0b..dc71ee4d314a 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -317,6 +317,13 @@ struct intel_hdcp_shim {
> >                                  enum transcoder cpu_transcoder,
> >                                  bool enable);
> >
> > +       /* Select/Deselect HDCP stream on the port DP MST Transport Link */
> > +       int (*toggle_select_hdcp)(struct intel_digital_port *intel_dig_port,
> > +                                 bool enable);
> > +
> > +       /* Enable HDCP stream encyption on DP MST Transport Link */
> 
> *encryption
> 
> > +       int (*stream_encryption)(struct intel_digital_port *intel_dig_port);
> > +
> >         /* Ensures the link is still protected */
> >         bool (*check_link)(struct intel_digital_port *dig_port,
> >                            struct intel_connector *connector);
> > @@ -410,6 +417,8 @@ struct intel_hdcp {
> >          * Hence caching the transcoder here.
> >          */
> >         enum transcoder cpu_transcoder;
> > +       /* Only used for DP MST stream encryption */
> > +       enum transcoder stream_transcoder;
> >  };
> >
> >  struct intel_connector {
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
> > index 03424d20e9f7..8a6427f3690b 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
> > @@ -16,6 +16,30 @@
> >  #include "intel_dp.h"
> >  #include "intel_hdcp.h"
> >
> > +static unsigned int trasncoder_to_stream_enc_status(enum transcoder cpu_transcoder)
> 
> *transcoder
> 
> > +{
> > +       u32 stream_enc_mask;
> > +
> > +       switch (cpu_transcoder) {
> > +       case TRANSCODER_A:
> > +               stream_enc_mask = HDCP_STATUS_STREAM_A_ENC;
> > +               break;
> > +       case TRANSCODER_B:
> > +               stream_enc_mask = HDCP_STATUS_STREAM_B_ENC;
> > +               break;
> > +       case TRANSCODER_C:
> > +               stream_enc_mask = HDCP_STATUS_STREAM_C_ENC;
> > +               break;
> > +       case TRANSCODER_D:
> > +               stream_enc_mask = HDCP_STATUS_STREAM_D_ENC;
> > +               break;
> > +       default:
> > +               stream_enc_mask = 0;
> > +       }
> > +
> > +       return stream_enc_mask;
> > +}
> > +
> >  static void intel_dp_hdcp_wait_for_cp_irq(struct intel_hdcp *hdcp, int timeout)
> >  {
> >         long ret;
> > @@ -622,24 +646,51 @@ static const struct intel_hdcp_shim intel_dp_hdcp_shim = {
> >  };
> >
> >  static int
> > -intel_dp_mst_hdcp_toggle_signalling(struct intel_digital_port *dig_port,
> > -                                   enum transcoder cpu_transcoder,
> > -                                   bool enable)
> > +intel_dp_mst_toggle_select_hdcp_strem(struct intel_digital_port *dig_port,
> > +                                     bool enable)
> 
> *stream
> 
> >  {
> >         struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
> > +       struct intel_dp *dp = &dig_port->dp;
> > +       struct intel_hdcp *hdcp = &dp->attached_connector->hdcp;
> >         int ret;
> >
> > -       if (!enable)
> > -               usleep_range(6, 60); /* Bspec says >= 6us */
> 
> Is this no longer needed on older generations?
6us wait is required only for  HDMI/DVI panels AFAI can see in B.Spec
> 
> > +       ret = intel_ddi_toggle_hdcp_bits(&dig_port->base,
> > +                                        hdcp->stream_transcoder, enable,
> > +                                        TRANS_DDI_HDCP_SELECT);
> >
> 
> Remove blank line
> 
> > -       ret = intel_ddi_toggle_hdcp_signalling(&dig_port->base,
> > -                                              cpu_transcoder, enable);
> >         if (ret)
> > -               drm_dbg_kms(&i915->drm, "%s HDCP signalling failed (%d)\n",
> > -                             enable ? "Enable" : "Disable", ret);
> > +               drm_dbg_kms(&i915->drm, "%s HDCP select failed (%d)\n",
> > +                           enable ? "Enable" : "Disable", ret);
> >         return ret;
> >  }
> >
> > +static int
> > +intel_dp_mst_hdcp_strem_encryption(struct intel_digital_port *dig_port)
> > +{
> > +       struct drm_i915_private *i915 = to_i915(idig_port->base.base.dev);
> > +       struct intel_dp *dp = &dig_port->dp;
> > +       struct intel_hdcp *hdcp = &dp->attached_connector->hdcp;
> > +       enum port port = dig_port->base.port;
> > +       enum transcoder cpu_transcoder = hdcp->cpu_transcoder;
> > +       u32 stream_enc_status;
> > +
> > +       stream_enc_status =  trasncoder_to_stream_enc_status(hdcp->stream_transcoder);
> > +
> 
> Remove blank line
Sure i will incorporate all typo fixes and consmetic chamges in next RFC version. 
> 
> > +       if (!stream_enc_status)
> > +               return -EINVAL;
> > +
> > +       /* Wait for encryption confirmation */
> > +       if (intel_de_wait_for_set(i915,
> > +                                 HDCP_STATUS(i915, cpu_transcoder, port),
> > +                                 stream_enc_status,
> > +                                 ENCRYPT_STATUS_CHANGE_TIMEOUT_MS)) {
> > +               drm_err(&i915->drm, "Timed out waiting for stream encryption enabled\n");
> > +               return -ETIMEDOUT;
> > +       }
> > +
> > +       return 0;
> > +}
> > +
> >  static
> >  bool intel_dp_mst_hdcp_check_link(struct intel_digital_port *dig_port,
> >                                   struct intel_connector *connector)
> > @@ -673,7 +724,9 @@ static const struct intel_hdcp_shim intel_dp_mst_hdcp_shim = {
> >         .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,
> > +       .toggle_signalling = intel_dp_hdcp_toggle_signalling,
> > +       .toggle_select_hdcp = intel_dp_mst_toggle_select_hdcp_strem,
> > +       .stream_encryption = intel_dp_mst_hdcp_strem_encryption,
> >         .check_link = intel_dp_mst_hdcp_check_link,
> >         .hdcp_capable = intel_dp_hdcp_capable,
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > index b6424bf5d544..8d06931e0805 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > @@ -564,7 +564,7 @@ static void intel_mst_enable_dp(struct intel_atomic_state *state,
> >         if (conn_state->content_protection ==
> >             DRM_MODE_CONTENT_PROTECTION_DESIRED)
> >                 intel_hdcp_enable(to_intel_connector(conn_state->connector),
> > -                                 pipe_config->cpu_transcoder,
> > +                                 pipe_config,
> >                                   (u8)conn_state->hdcp_content_type);
> >  }
> >
> > @@ -811,7 +811,7 @@ static struct drm_connector *intel_dp_add_mst_connector(struct drm_dp_mst_topolo
> >
> >
> >         /* TODO: Figure out how to make HDCP work on GEN12+ */
> 
> I think this comment is no longer valid
> 
> > -       if (INTEL_GEN(dev_priv) < 12) {
> > +       if (INTEL_GEN(dev_priv) <= 12) {
> 
> Is there any benefit to limiting this any longer? Perhaps just delete this now.
sure.
> 
> >                 ret = intel_dp_init_hdcp(dig_port, intel_connector);
> >                 if (ret)
> >                         DRM_DEBUG_KMS("HDCP init failed, skipping.\n");
> > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
> > index 5492076d1ae0..1436fb2910d4 100644
> > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> > @@ -23,7 +23,6 @@
> >  #include "intel_connector.h"
> >
> >  #define KEY_LOAD_TRIES 5
> > -#define ENCRYPT_STATUS_CHANGE_TIMEOUT_MS       50
> >  #define HDCP2_LC_RETRY_CNT                     3
> >
> >  static
> > @@ -700,6 +699,7 @@ static int intel_hdcp_auth(struct intel_connector *connector)
> >         ret = shim->repeater_present(dig_port, &repeater_present);
> >         if (ret)
> >                 return ret;
> > +
> >         if (repeater_present)
> >                 intel_de_write(dev_priv, HDCP_REP_CTL,
> >                                intel_hdcp_get_repeater_ctl(dev_priv, cpu_transcoder, port));
> > @@ -771,6 +771,11 @@ static int intel_hdcp_auth(struct intel_connector *connector)
> >          * XXX: If we have MST-connected devices, we need to enable encryption
> >          * on those as well.
> >          */
> 
> This comment is also obsolete now.
> 
> > +       if (shim->toggle_select_hdcp)
> > +               ret = shim->toggle_select_hdcp(dig_port, true);
> > +
> > +       if (shim->stream_encryption)
> > +               ret = shim->stream_encryption(dig_port);
> 
> Instead of adding 2 new hooks, couldn't you just combine these into 1?
yes it can be done.
> 
> >
> >         if (repeater_present)
> >                 return intel_hdcp_auth_downstream(connector);
> > @@ -797,12 +802,13 @@ static int _intel_hdcp_disable(struct intel_connector *connector)
> >          * it. Instead, toggle the HDCP signalling off on that particular
> >          * connector/pipe and exit.
> >          */
> > -       if (dig_port->num_hdcp_streams > 0) {
> > -               ret = hdcp->shim->toggle_signalling(dig_port,
> > -                                                   cpu_transcoder, false);
> > -               if (ret)
> > -                       DRM_ERROR("Failed to disable HDCP signalling\n");
> > -               return ret;
> > +       if (intel_dig_port->num_hdcp_streams > 0) {
> > +               if (hdcp->shim->toggle_select_hdcp) {
> 
> Combining these with &&?
yes i will do.
> 
> > +                       ret = hdcp->shim->toggle_select_hdcp(dig_port, false);
> > +                       if (ret)
> > +                               DRM_ERROR("Failed to disable HDCP signalling\n");
> > +                       return ret;
> > +               }
> >         }
> >
> >         hdcp->hdcp_encrypted = false;
> > @@ -2072,7 +2078,7 @@ int intel_hdcp_init(struct intel_connector *connector,
> >  }
> >
> >  int intel_hdcp_enable(struct intel_connector *connector,
> > -                     enum transcoder cpu_transcoder, u8 content_type)
> > +                     const struct intel_crtc_state *pipe_config, u8 content_type)
> >  {
> >         struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> >         struct intel_digital_port *dig_port = intel_attached_dig_port(connector);
> > @@ -2088,10 +2094,17 @@ int intel_hdcp_enable(struct intel_connector *connector,
> >         drm_WARN_ON(&dev_priv->drm,
> >                     hdcp->value == DRM_MODE_CONTENT_PROTECTION_ENABLED);
> >         hdcp->content_type = content_type;
> > -       hdcp->cpu_transcoder = cpu_transcoder;
> > +
> > +       if (intel_crtc_has_type(pipe_config, INTEL_OUTPUT_DP_MST)) {
> > +               hdcp->cpu_transcoder = pipe_config->mst_master_transcoder;
> > +               hdcp->stream_transcoder = pipe_config->cpu_transcoder;
> > +       } else {
> > +               hdcp->cpu_transcoder = pipe_config->cpu_transcoder;
> > +               hdcp->stream_transcoder = INVALID_TRANSCODER;
> > +       }
> >
> >         if (INTEL_GEN(dev_priv) >= 12)
> > -               hdcp->port_data.fw_tc = intel_get_mei_fw_tc(cpu_transcoder);
> > +               hdcp->port_data.fw_tc = intel_get_mei_fw_tc(hdcp->cpu_transcoder);
> >
> >         /*
> >          * Considering that HDCP2.2 is more secure than HDCP1.4, If the setup
> > @@ -2202,7 +2215,7 @@ void intel_hdcp_update_pipe(struct intel_atomic_state *state,
> >
> >         if (desired_and_not_enabled || content_protection_type_changed)
> >                 intel_hdcp_enable(connector,
> > -                                 crtc_state->cpu_transcoder,
> > +                                 crtc_state,
> >                                   (u8)conn_state->hdcp_content_type);
> >  }
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.h b/drivers/gpu/drm/i915/display/intel_hdcp.h
> > index 1bbf5b67ed0a..36a1b81aca16 100644
> > --- a/drivers/gpu/drm/i915/display/intel_hdcp.h
> > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.h
> > @@ -19,13 +19,15 @@ struct intel_hdcp_shim;
> >  enum port;
> >  enum transcoder;
> >
> > +#define ENCRYPT_STATUS_CHANGE_TIMEOUT_MS       50
> 
> Now that this is exposed in a header, best to make it more descriptive:
> 
> #define HDCP_ENCRYPT_STATUS_CHANGE_TIMEOUT_MS
Hi Ram ,
This suppoese to be a frame time as per B.Spec with respect to HDCP2 encryption status
, stream encryption status and a vblank time for HDCP 1 encryption status.
Any reasoning to choose 50ms of timeout for encrystion status in HDCP 1 and HDCP 2.
 
> 
> > +
> >  void intel_hdcp_atomic_check(struct drm_connector *connector,
> >                              struct drm_connector_state *old_state,
> >                              struct drm_connector_state *new_state);
> >  int intel_hdcp_init(struct intel_connector *connector, enum port port,
> >                     const struct intel_hdcp_shim *hdcp_shim);
> >  int intel_hdcp_enable(struct intel_connector *connector,
> > -                     enum transcoder cpu_transcoder, u8 content_type);
> > +                     const struct intel_crtc_state *pipe_config, u8 content_type);
> >  int intel_hdcp_disable(struct intel_connector *connector);
> >  void intel_hdcp_update_pipe(struct intel_atomic_state *state,
> >                             struct intel_encoder *encoder,
> > diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> > index 0978b0d8f4c6..955d2250b86f 100644
> > --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> > @@ -1495,15 +1495,18 @@ static int kbl_repositioning_enc_en_signal(struct intel_connector *connector,
> >                 usleep_range(25, 50);
> >         }
> >
> > -       ret = intel_ddi_toggle_hdcp_signalling(&dig_port->base, cpu_transcoder,
> > -                                              false);
> > +       ret = intel_ddi_toggle_hdcp_bits(&dig_port->base, cpu_transcoder,
> > +                                        false, TRANS_DDI_HDCP_SIGNALLING);
> > +
> 
> Remove blank line
> 
> >         if (ret) {
> >                 drm_err(&dev_priv->drm,
> >                         "Disable HDCP signalling failed (%d)\n", ret);
> >                 return ret;
> >         }
> > -       ret = intel_ddi_toggle_hdcp_signalling(&dig_port->base, cpu_transcoder,
> > -                                              true);
> > +
> > +       ret = intel_ddi_toggle_hdcp_bits(&dig_port->base, cpu_transcoder,
> > +                                        true, TRANS_DDI_HDCP_SIGNALLING);
> > +
> 
> Remove blank line
> 
> >         if (ret) {
> >                 drm_err(&dev_priv->drm,
> >                         "Enable HDCP signalling failed (%d)\n", ret);
> > @@ -1526,8 +1529,9 @@ int intel_hdmi_hdcp_toggle_signalling(struct intel_digital_port *dig_port,
> >         if (!enable)
> >                 usleep_range(6, 60); /* Bspec says >= 6us */
> >
> > -       ret = intel_ddi_toggle_hdcp_signalling(&dig_port->base, cpu_transcoder,
> > -                                              enable);
> > +       ret = intel_ddi_toggle_hdcp_bits(&dig_port->base,
> > +                                        cpu_transcoder, enable,
> > +                                        TRANS_DDI_HDCP_SIGNALLING);
> >         if (ret) {
> >                 drm_err(&dev_priv->drm, "%s HDCP signalling failed (%d)\n",
> >                         enable ? "Enable" : "Disable", ret);
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index ab4b1abd4364..f6e40a458f7b 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -9945,6 +9945,7 @@ enum skl_power_gate {
> >  #define  TRANS_DDI_DP_VC_PAYLOAD_ALLOC (1 << 8)
> >  #define  TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE (1 << 7)
> >  #define  TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ (1 << 6)
> > +#define  TRANS_DDI_HDCP_SELECT (1 << 5)
> >  #define  TRANS_DDI_BFI_ENABLE          (1 << 4)
> >  #define  TRANS_DDI_HIGH_TMDS_CHAR_RATE (1 << 4)
> >  #define  TRANS_DDI_HDMI_SCRAMBLING     (1 << 0)
> > --
> > 2.26.2
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

end of thread, other threads:[~2020-09-02  7:56 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-01 12:10 [Intel-gfx] [RFC] drm/i915/hdcp: Gen12 HDCP 1.4 support over DP MST Anshuman Gupta
2020-09-01 12:40 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for " Patchwork
2020-09-01 13:57 ` [Intel-gfx] [RFC] " Sean Paul
2020-09-02  7:45   ` Anshuman Gupta
2020-09-01 15:21 ` kernel test robot
2020-09-01 17:45 ` kernel test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.