[Intel-gfx] [PATCH 0/3] drm/i915/hdcp: Additional conditions to enable hdcp

[Intel-gfx] [PATCH 0/3] drm/i915/hdcp: Additional conditions to enable hdcp

[Suraj Kandpal]

We are seeing a issue when we close the lid of a laptop or dock a
monitor hdcp content is not being reenabled automatically this is
because when we dock a monitor we end up with a enable and
disable connector cycle but if hdcp content is running we get the
userspace in enabled state and driver maintaining a undesired
state which causes the content to stop playing and we only enabe
hdcp if the userspace state in desired.
This first and second patch refactors the code while the third one adds the
new conditions to enable hdcp.

Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>

Suraj Kandpal (3):
  drm/i915/hdcp: Rename HCDP 1.4 enablement function
  drm/i915/hdcp: Convert intel_hdcp_enable to a blanket function
  drm/i915/hdcp: Add more conditions to enable hdcp

 drivers/gpu/drm/i915/display/intel_ddi.c    |  5 +--
 drivers/gpu/drm/i915/display/intel_dp_mst.c |  5 +--
 drivers/gpu/drm/i915/display/intel_hdcp.c   | 37 ++++++++++++++++-----
 drivers/gpu/drm/i915/display/intel_hdcp.h   |  8 ++---
 4 files changed, 35 insertions(+), 20 deletions(-)

-- 
2.25.1

[Intel-gfx] [PATCH 1/3] drm/i915/hdcp: Rename HCDP 1.4 enablement function

[Suraj Kandpal]

Rename hdcp 1.4 enablement function from _intel_hdcp_enable to
intel_hdcp1_enable to better represent what version of hdcp is
being enabled

Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_hdcp.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
index c89da3568ebd..7c0cfcb48521 100644
--- a/drivers/gpu/drm/i915/display/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
@@ -923,7 +923,7 @@ static int _intel_hdcp_disable(struct intel_connector *connector)
 	return 0;
 }
 
-static int _intel_hdcp_enable(struct intel_connector *connector)
+static int intel_hdcp1_enable(struct intel_connector *connector)
 {
 	struct drm_i915_private *i915 = to_i915(connector->base.dev);
 	struct intel_hdcp *hdcp = &connector->hdcp;
@@ -1058,7 +1058,7 @@ static int intel_hdcp_check_link(struct intel_connector *connector)
 		goto out;
 	}
 
-	ret = _intel_hdcp_enable(connector);
+	ret = intel_hdcp1_enable(connector);
 	if (ret) {
 		drm_err(&i915->drm, "Failed to enable hdcp (%d)\n", ret);
 		intel_hdcp_update_value(connector,
@@ -2388,7 +2388,7 @@ int intel_hdcp_enable(struct intel_atomic_state *state,
 	 */
 	if (ret && intel_hdcp_capable(connector) &&
 	    hdcp->content_type != DRM_MODE_HDCP_CONTENT_TYPE1) {
-		ret = _intel_hdcp_enable(connector);
+		ret = intel_hdcp1_enable(connector);
 	}
 
 	if (!ret) {
-- 
2.25.1

[Intel-gfx] [PATCH 2/3] drm/i915/hdcp: Convert intel_hdcp_enable to a blanket function

[Suraj Kandpal]

Let's convert intel_hdcp_enable to a blanket function
which just has some conditions which needs to be checked
before connectors enable hdcp.
This cleans up code and avoids code duplication.

--v3
-Keep function name as intel_hdcp_enable() [Jani]

Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_ddi.c    |  5 +----
 drivers/gpu/drm/i915/display/intel_dp_mst.c |  5 +----
 drivers/gpu/drm/i915/display/intel_hdcp.c   | 21 ++++++++++++++++-----
 drivers/gpu/drm/i915/display/intel_hdcp.h   |  8 ++++----
 4 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 9151d5add960..b644cf981846 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -3259,10 +3259,7 @@ static void intel_enable_ddi(struct intel_atomic_state *state,
 	else
 		intel_enable_ddi_dp(state, encoder, crtc_state, conn_state);
 
-	/* Enable hdcp if it's desired */
-	if (conn_state->content_protection ==
-	    DRM_MODE_CONTENT_PROTECTION_DESIRED)
-		intel_hdcp_enable(state, encoder, crtc_state, conn_state);
+	intel_hdcp_enable(state, encoder, crtc_state, conn_state);
 }
 
 static void intel_disable_ddi_dp(struct intel_atomic_state *state,
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 7b4628f4f124..4366da79fe81 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -836,10 +836,7 @@ static void intel_mst_enable_dp(struct intel_atomic_state *state,
 
 	intel_audio_codec_enable(encoder, pipe_config, conn_state);
 
-	/* Enable hdcp if it's desired */
-	if (conn_state->content_protection ==
-	    DRM_MODE_CONTENT_PROTECTION_DESIRED)
-		intel_hdcp_enable(state, encoder, pipe_config, conn_state);
+	intel_hdcp_enable(state, encoder, pipe_config, conn_state);
 }
 
 static bool intel_dp_mst_enc_get_hw_state(struct intel_encoder *encoder,
diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
index 7c0cfcb48521..44c0a93f3af8 100644
--- a/drivers/gpu/drm/i915/display/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
@@ -2324,10 +2324,10 @@ intel_hdcp_set_streams(struct intel_digital_port *dig_port,
 	return 0;
 }
 
-int intel_hdcp_enable(struct intel_atomic_state *state,
-		      struct intel_encoder *encoder,
-		      const struct intel_crtc_state *pipe_config,
-		      const struct drm_connector_state *conn_state)
+static int _intel_hdcp_enable(struct intel_atomic_state *state,
+			      struct intel_encoder *encoder,
+			      const struct intel_crtc_state *pipe_config,
+			      const struct drm_connector_state *conn_state)
 {
 	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
 	struct intel_connector *connector =
@@ -2404,6 +2404,17 @@ int intel_hdcp_enable(struct intel_atomic_state *state,
 	return ret;
 }
 
+void intel_hdcp_enable(struct intel_atomic_state *state,
+		       struct intel_encoder *encoder,
+		       const struct intel_crtc_state *crtc_state,
+		       const struct drm_connector_state *conn_state)
+{
+	/* Enable hdcp if it's desired */
+	if (conn_state->content_protection ==
+	    DRM_MODE_CONTENT_PROTECTION_DESIRED)
+		_intel_hdcp_enable(state, encoder, crtc_state, conn_state);
+}
+
 int intel_hdcp_disable(struct intel_connector *connector)
 {
 	struct intel_digital_port *dig_port = intel_attached_dig_port(connector);
@@ -2491,7 +2502,7 @@ void intel_hdcp_update_pipe(struct intel_atomic_state *state,
 	}
 
 	if (desired_and_not_enabled || content_protection_type_changed)
-		intel_hdcp_enable(state, encoder, crtc_state, conn_state);
+		_intel_hdcp_enable(state, encoder, crtc_state, conn_state);
 }
 
 void intel_hdcp_component_fini(struct drm_i915_private *i915)
diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.h b/drivers/gpu/drm/i915/display/intel_hdcp.h
index 5997c52a0958..a9c784fd9ba5 100644
--- a/drivers/gpu/drm/i915/display/intel_hdcp.h
+++ b/drivers/gpu/drm/i915/display/intel_hdcp.h
@@ -28,10 +28,10 @@ void intel_hdcp_atomic_check(struct drm_connector *connector,
 int intel_hdcp_init(struct intel_connector *connector,
 		    struct intel_digital_port *dig_port,
 		    const struct intel_hdcp_shim *hdcp_shim);
-int intel_hdcp_enable(struct intel_atomic_state *state,
-		      struct intel_encoder *encoder,
-		      const struct intel_crtc_state *pipe_config,
-		      const struct drm_connector_state *conn_state);
+void intel_hdcp_enable(struct intel_atomic_state *state,
+		       struct intel_encoder *encoder,
+		       const struct intel_crtc_state *pipe_config,
+		       const struct drm_connector_state *conn_state);
 int intel_hdcp_disable(struct intel_connector *connector);
 void intel_hdcp_update_pipe(struct intel_atomic_state *state,
 			    struct intel_encoder *encoder,
-- 
2.25.1

[Intel-gfx] [PATCH 2/3] drm/i915/hdcp: Create a blanket hdcp enable function

[Suraj Kandpal]

Let's create a blanket function which just has some conditions
which need to be checked before connectors enable hdcp.
This cleans up code and avoids code duplication.

--v3
-Keep function name as intel_hdcp_enable() [Jani]

Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_ddi.c    |  5 +----
 drivers/gpu/drm/i915/display/intel_dp_mst.c |  5 +----
 drivers/gpu/drm/i915/display/intel_hdcp.c   | 21 ++++++++++++++++-----
 drivers/gpu/drm/i915/display/intel_hdcp.h   |  8 ++++----
 4 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 9151d5add960..b644cf981846 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -3259,10 +3259,7 @@ static void intel_enable_ddi(struct intel_atomic_state *state,
 	else
 		intel_enable_ddi_dp(state, encoder, crtc_state, conn_state);
 
-	/* Enable hdcp if it's desired */
-	if (conn_state->content_protection ==
-	    DRM_MODE_CONTENT_PROTECTION_DESIRED)
-		intel_hdcp_enable(state, encoder, crtc_state, conn_state);
+	intel_hdcp_enable(state, encoder, crtc_state, conn_state);
 }
 
 static void intel_disable_ddi_dp(struct intel_atomic_state *state,
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 7b4628f4f124..4366da79fe81 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -836,10 +836,7 @@ static void intel_mst_enable_dp(struct intel_atomic_state *state,
 
 	intel_audio_codec_enable(encoder, pipe_config, conn_state);
 
-	/* Enable hdcp if it's desired */
-	if (conn_state->content_protection ==
-	    DRM_MODE_CONTENT_PROTECTION_DESIRED)
-		intel_hdcp_enable(state, encoder, pipe_config, conn_state);
+	intel_hdcp_enable(state, encoder, pipe_config, conn_state);
 }
 
 static bool intel_dp_mst_enc_get_hw_state(struct intel_encoder *encoder,
diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
index 7c0cfcb48521..44c0a93f3af8 100644
--- a/drivers/gpu/drm/i915/display/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
@@ -2324,10 +2324,10 @@ intel_hdcp_set_streams(struct intel_digital_port *dig_port,
 	return 0;
 }
 
-int intel_hdcp_enable(struct intel_atomic_state *state,
-		      struct intel_encoder *encoder,
-		      const struct intel_crtc_state *pipe_config,
-		      const struct drm_connector_state *conn_state)
+static int _intel_hdcp_enable(struct intel_atomic_state *state,
+			      struct intel_encoder *encoder,
+			      const struct intel_crtc_state *pipe_config,
+			      const struct drm_connector_state *conn_state)
 {
 	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
 	struct intel_connector *connector =
@@ -2404,6 +2404,17 @@ int intel_hdcp_enable(struct intel_atomic_state *state,
 	return ret;
 }
 
+void intel_hdcp_enable(struct intel_atomic_state *state,
+		       struct intel_encoder *encoder,
+		       const struct intel_crtc_state *crtc_state,
+		       const struct drm_connector_state *conn_state)
+{
+	/* Enable hdcp if it's desired */
+	if (conn_state->content_protection ==
+	    DRM_MODE_CONTENT_PROTECTION_DESIRED)
+		_intel_hdcp_enable(state, encoder, crtc_state, conn_state);
+}
+
 int intel_hdcp_disable(struct intel_connector *connector)
 {
 	struct intel_digital_port *dig_port = intel_attached_dig_port(connector);
@@ -2491,7 +2502,7 @@ void intel_hdcp_update_pipe(struct intel_atomic_state *state,
 	}
 
 	if (desired_and_not_enabled || content_protection_type_changed)
-		intel_hdcp_enable(state, encoder, crtc_state, conn_state);
+		_intel_hdcp_enable(state, encoder, crtc_state, conn_state);
 }
 
 void intel_hdcp_component_fini(struct drm_i915_private *i915)
diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.h b/drivers/gpu/drm/i915/display/intel_hdcp.h
index 5997c52a0958..a9c784fd9ba5 100644
--- a/drivers/gpu/drm/i915/display/intel_hdcp.h
+++ b/drivers/gpu/drm/i915/display/intel_hdcp.h
@@ -28,10 +28,10 @@ void intel_hdcp_atomic_check(struct drm_connector *connector,
 int intel_hdcp_init(struct intel_connector *connector,
 		    struct intel_digital_port *dig_port,
 		    const struct intel_hdcp_shim *hdcp_shim);
-int intel_hdcp_enable(struct intel_atomic_state *state,
-		      struct intel_encoder *encoder,
-		      const struct intel_crtc_state *pipe_config,
-		      const struct drm_connector_state *conn_state);
+void intel_hdcp_enable(struct intel_atomic_state *state,
+		       struct intel_encoder *encoder,
+		       const struct intel_crtc_state *pipe_config,
+		       const struct drm_connector_state *conn_state);
 int intel_hdcp_disable(struct intel_connector *connector);
 void intel_hdcp_update_pipe(struct intel_atomic_state *state,
 			    struct intel_encoder *encoder,
-- 
2.25.1

[Intel-gfx] [PATCH 3/3] drm/i915/hdcp: Add more conditions to enable hdcp

[Suraj Kandpal]

When we dock a monitor we end up with a enable and disable connector
cycle but if hdcp content is running we get the userspace in
enabled state and driver maintaining a undesired state which causes
the content to stop playing and we only enabe hdcp if the userspace
state in desired. This patch fixes that.

--v2
-Move code to intel_hdcp [Jani]

Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_hdcp.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
index 44c0a93f3af8..39b3f7c0c77c 100644
--- a/drivers/gpu/drm/i915/display/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
@@ -2409,9 +2409,19 @@ void intel_hdcp_enable(struct intel_atomic_state *state,
 		       const struct intel_crtc_state *crtc_state,
 		       const struct drm_connector_state *conn_state)
 {
-	/* Enable hdcp if it's desired */
+	struct intel_connector *connector =
+		to_intel_connector(conn_state->connector);
+	struct intel_hdcp *hdcp = &connector->hdcp;
+
+	/*
+	 * Enable hdcp if it's desired or if userspace is enabled and
+	 * driver set its state to undesired
+	 */
 	if (conn_state->content_protection ==
-	    DRM_MODE_CONTENT_PROTECTION_DESIRED)
+	    DRM_MODE_CONTENT_PROTECTION_DESIRED ||
+	    (conn_state->content_protection ==
+	    DRM_MODE_CONTENT_PROTECTION_ENABLED && hdcp->value ==
+	    DRM_MODE_CONTENT_PROTECTION_UNDESIRED))
 		_intel_hdcp_enable(state, encoder, crtc_state, conn_state);
 }
 
-- 
2.25.1

Re: [Intel-gfx] [PATCH 2/3] drm/i915/hdcp: Create a blanket hdcp enable function

[Jani Nikula]

On Thu, 26 Oct 2023, Suraj Kandpal <suraj.kandpal@intel.com> wrote:
> Let's create a blanket function which just has some conditions
> which need to be checked before connectors enable hdcp.
> This cleans up code and avoids code duplication.

This series has two 2/3 patches... confused me, probably going to
confuse CI too...

BR,
Jani.


>
> --v3
> -Keep function name as intel_hdcp_enable() [Jani]
>
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c    |  5 +----
>  drivers/gpu/drm/i915/display/intel_dp_mst.c |  5 +----
>  drivers/gpu/drm/i915/display/intel_hdcp.c   | 21 ++++++++++++++++-----
>  drivers/gpu/drm/i915/display/intel_hdcp.h   |  8 ++++----
>  4 files changed, 22 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 9151d5add960..b644cf981846 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -3259,10 +3259,7 @@ static void intel_enable_ddi(struct intel_atomic_state *state,
>  	else
>  		intel_enable_ddi_dp(state, encoder, crtc_state, conn_state);
>  
> -	/* Enable hdcp if it's desired */
> -	if (conn_state->content_protection ==
> -	    DRM_MODE_CONTENT_PROTECTION_DESIRED)
> -		intel_hdcp_enable(state, encoder, crtc_state, conn_state);
> +	intel_hdcp_enable(state, encoder, crtc_state, conn_state);
>  }
>  
>  static void intel_disable_ddi_dp(struct intel_atomic_state *state,
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 7b4628f4f124..4366da79fe81 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -836,10 +836,7 @@ static void intel_mst_enable_dp(struct intel_atomic_state *state,
>  
>  	intel_audio_codec_enable(encoder, pipe_config, conn_state);
>  
> -	/* Enable hdcp if it's desired */
> -	if (conn_state->content_protection ==
> -	    DRM_MODE_CONTENT_PROTECTION_DESIRED)
> -		intel_hdcp_enable(state, encoder, pipe_config, conn_state);
> +	intel_hdcp_enable(state, encoder, pipe_config, conn_state);
>  }
>  
>  static bool intel_dp_mst_enc_get_hw_state(struct intel_encoder *encoder,
> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
> index 7c0cfcb48521..44c0a93f3af8 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> @@ -2324,10 +2324,10 @@ intel_hdcp_set_streams(struct intel_digital_port *dig_port,
>  	return 0;
>  }
>  
> -int intel_hdcp_enable(struct intel_atomic_state *state,
> -		      struct intel_encoder *encoder,
> -		      const struct intel_crtc_state *pipe_config,
> -		      const struct drm_connector_state *conn_state)
> +static int _intel_hdcp_enable(struct intel_atomic_state *state,
> +			      struct intel_encoder *encoder,
> +			      const struct intel_crtc_state *pipe_config,
> +			      const struct drm_connector_state *conn_state)
>  {
>  	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
>  	struct intel_connector *connector =
> @@ -2404,6 +2404,17 @@ int intel_hdcp_enable(struct intel_atomic_state *state,
>  	return ret;
>  }
>  
> +void intel_hdcp_enable(struct intel_atomic_state *state,
> +		       struct intel_encoder *encoder,
> +		       const struct intel_crtc_state *crtc_state,
> +		       const struct drm_connector_state *conn_state)
> +{
> +	/* Enable hdcp if it's desired */
> +	if (conn_state->content_protection ==
> +	    DRM_MODE_CONTENT_PROTECTION_DESIRED)
> +		_intel_hdcp_enable(state, encoder, crtc_state, conn_state);
> +}
> +
>  int intel_hdcp_disable(struct intel_connector *connector)
>  {
>  	struct intel_digital_port *dig_port = intel_attached_dig_port(connector);
> @@ -2491,7 +2502,7 @@ void intel_hdcp_update_pipe(struct intel_atomic_state *state,
>  	}
>  
>  	if (desired_and_not_enabled || content_protection_type_changed)
> -		intel_hdcp_enable(state, encoder, crtc_state, conn_state);
> +		_intel_hdcp_enable(state, encoder, crtc_state, conn_state);
>  }
>  
>  void intel_hdcp_component_fini(struct drm_i915_private *i915)
> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.h b/drivers/gpu/drm/i915/display/intel_hdcp.h
> index 5997c52a0958..a9c784fd9ba5 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdcp.h
> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.h
> @@ -28,10 +28,10 @@ void intel_hdcp_atomic_check(struct drm_connector *connector,
>  int intel_hdcp_init(struct intel_connector *connector,
>  		    struct intel_digital_port *dig_port,
>  		    const struct intel_hdcp_shim *hdcp_shim);
> -int intel_hdcp_enable(struct intel_atomic_state *state,
> -		      struct intel_encoder *encoder,
> -		      const struct intel_crtc_state *pipe_config,
> -		      const struct drm_connector_state *conn_state);
> +void intel_hdcp_enable(struct intel_atomic_state *state,
> +		       struct intel_encoder *encoder,
> +		       const struct intel_crtc_state *pipe_config,
> +		       const struct drm_connector_state *conn_state);
>  int intel_hdcp_disable(struct intel_connector *connector);
>  void intel_hdcp_update_pipe(struct intel_atomic_state *state,
>  			    struct intel_encoder *encoder,

-- 
Jani Nikula, Intel

Re: [Intel-gfx] [PATCH 2/3] drm/i915/hdcp: Create a blanket hdcp enable function

[Kandpal, Suraj]

> -----Original Message-----
> From: Jani Nikula <jani.nikula@linux.intel.com>
> Sent: Thursday, October 26, 2023 3:34 PM
> To: Kandpal, Suraj <suraj.kandpal@intel.com>; intel-gfx@lists.freedesktop.org
> Cc: Shankar, Uma <uma.shankar@intel.com>; Nautiyal, Ankit K
> <ankit.k.nautiyal@intel.com>; Kandpal, Suraj <suraj.kandpal@intel.com>
> Subject: Re: [PATCH 2/3] drm/i915/hdcp: Create a blanket hdcp enable function
> 
> On Thu, 26 Oct 2023, Suraj Kandpal <suraj.kandpal@intel.com> wrote:
> > Let's create a blanket function which just has some conditions which
> > need to be checked before connectors enable hdcp.
> > This cleans up code and avoids code duplication.
> 
> This series has two 2/3 patches... confused me, probably going to confuse CI
> too...
> 
Weird will send out a new patch series just in case
Though the patchwork seems to catch it properly
https://patchwork.freedesktop.org/series/125550/

Regards,
Suraj Kandpal
> BR,
> Jani.
> 
> 
> >
> > --v3
> > -Keep function name as intel_hdcp_enable() [Jani]
> >
> > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_ddi.c    |  5 +----
> >  drivers/gpu/drm/i915/display/intel_dp_mst.c |  5 +----
> >  drivers/gpu/drm/i915/display/intel_hdcp.c   | 21 ++++++++++++++++-----
> >  drivers/gpu/drm/i915/display/intel_hdcp.h   |  8 ++++----
> >  4 files changed, 22 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index 9151d5add960..b644cf981846 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -3259,10 +3259,7 @@ static void intel_enable_ddi(struct
> intel_atomic_state *state,
> >  	else
> >  		intel_enable_ddi_dp(state, encoder, crtc_state, conn_state);
> >
> > -	/* Enable hdcp if it's desired */
> > -	if (conn_state->content_protection ==
> > -	    DRM_MODE_CONTENT_PROTECTION_DESIRED)
> > -		intel_hdcp_enable(state, encoder, crtc_state, conn_state);
> > +	intel_hdcp_enable(state, encoder, crtc_state, conn_state);
> >  }
> >
> >  static void intel_disable_ddi_dp(struct intel_atomic_state *state,
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > index 7b4628f4f124..4366da79fe81 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > @@ -836,10 +836,7 @@ static void intel_mst_enable_dp(struct
> > intel_atomic_state *state,
> >
> >  	intel_audio_codec_enable(encoder, pipe_config, conn_state);
> >
> > -	/* Enable hdcp if it's desired */
> > -	if (conn_state->content_protection ==
> > -	    DRM_MODE_CONTENT_PROTECTION_DESIRED)
> > -		intel_hdcp_enable(state, encoder, pipe_config, conn_state);
> > +	intel_hdcp_enable(state, encoder, pipe_config, conn_state);
> >  }
> >
> >  static bool intel_dp_mst_enc_get_hw_state(struct intel_encoder
> > *encoder, diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c
> > b/drivers/gpu/drm/i915/display/intel_hdcp.c
> > index 7c0cfcb48521..44c0a93f3af8 100644
> > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> > @@ -2324,10 +2324,10 @@ intel_hdcp_set_streams(struct intel_digital_port
> *dig_port,
> >  	return 0;
> >  }
> >
> > -int intel_hdcp_enable(struct intel_atomic_state *state,
> > -		      struct intel_encoder *encoder,
> > -		      const struct intel_crtc_state *pipe_config,
> > -		      const struct drm_connector_state *conn_state)
> > +static int _intel_hdcp_enable(struct intel_atomic_state *state,
> > +			      struct intel_encoder *encoder,
> > +			      const struct intel_crtc_state *pipe_config,
> > +			      const struct drm_connector_state *conn_state)
> >  {
> >  	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> >  	struct intel_connector *connector =
> > @@ -2404,6 +2404,17 @@ int intel_hdcp_enable(struct intel_atomic_state
> *state,
> >  	return ret;
> >  }
> >
> > +void intel_hdcp_enable(struct intel_atomic_state *state,
> > +		       struct intel_encoder *encoder,
> > +		       const struct intel_crtc_state *crtc_state,
> > +		       const struct drm_connector_state *conn_state) {
> > +	/* Enable hdcp if it's desired */
> > +	if (conn_state->content_protection ==
> > +	    DRM_MODE_CONTENT_PROTECTION_DESIRED)
> > +		_intel_hdcp_enable(state, encoder, crtc_state, conn_state); }
> > +
> >  int intel_hdcp_disable(struct intel_connector *connector)  {
> >  	struct intel_digital_port *dig_port =
> > intel_attached_dig_port(connector);
> > @@ -2491,7 +2502,7 @@ void intel_hdcp_update_pipe(struct
> intel_atomic_state *state,
> >  	}
> >
> >  	if (desired_and_not_enabled || content_protection_type_changed)
> > -		intel_hdcp_enable(state, encoder, crtc_state, conn_state);
> > +		_intel_hdcp_enable(state, encoder, crtc_state, conn_state);
> >  }
> >
> >  void intel_hdcp_component_fini(struct drm_i915_private *i915) diff
> > --git a/drivers/gpu/drm/i915/display/intel_hdcp.h
> > b/drivers/gpu/drm/i915/display/intel_hdcp.h
> > index 5997c52a0958..a9c784fd9ba5 100644
> > --- a/drivers/gpu/drm/i915/display/intel_hdcp.h
> > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.h
> > @@ -28,10 +28,10 @@ void intel_hdcp_atomic_check(struct drm_connector
> > *connector,  int intel_hdcp_init(struct intel_connector *connector,
> >  		    struct intel_digital_port *dig_port,
> >  		    const struct intel_hdcp_shim *hdcp_shim); -int
> > intel_hdcp_enable(struct intel_atomic_state *state,
> > -		      struct intel_encoder *encoder,
> > -		      const struct intel_crtc_state *pipe_config,
> > -		      const struct drm_connector_state *conn_state);
> > +void intel_hdcp_enable(struct intel_atomic_state *state,
> > +		       struct intel_encoder *encoder,
> > +		       const struct intel_crtc_state *pipe_config,
> > +		       const struct drm_connector_state *conn_state);
> >  int intel_hdcp_disable(struct intel_connector *connector);  void
> > intel_hdcp_update_pipe(struct intel_atomic_state *state,
> >  			    struct intel_encoder *encoder,
> 
> --
> Jani Nikula, Intel