[v8,32/35] drm/i915: Commit CP without modeset

Submitted by Ramalingam C on Nov. 27, 2018, 10:43 a.m.

Details

Message ID 1543315413-24302-33-git-send-email-ramalingam.c@intel.com
State New
Headers show
Series "drm/i915: Implement HDCP2.2" ( rev: 10 ) in Intel GFX

Not browsing as part of any series.

Commit Message

Ramalingam C Nov. 27, 2018, 10:43 a.m.
Commits the content protection change of a connector,
without crtc modeset. This improves the user experience.

Originally proposed by Sean Paul at v3 of HDCP1.4 framework
https://patchwork.freedesktop.org/patch/191759/. For some
reason this was dropped, but needed for the proper functionality
of HDCP encryption.

Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
Suggested-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/i915/intel_ddi.c     |  7 -------
 drivers/gpu/drm/i915/intel_display.c | 10 ++++++++++
 drivers/gpu/drm/i915/intel_drv.h     |  5 +++++
 drivers/gpu/drm/i915/intel_hdcp.c    | 32 ++++++++++++++++++++++++++++----
 4 files changed, 43 insertions(+), 11 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index ad11540ac436..128cc558d75f 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -3469,11 +3469,6 @@  static void intel_enable_ddi(struct intel_encoder *encoder,
 		intel_enable_ddi_hdmi(encoder, crtc_state, conn_state);
 	else
 		intel_enable_ddi_dp(encoder, crtc_state, conn_state);
-
-	/* Enable hdcp if it's desired */
-	if (conn_state->content_protection ==
-	    DRM_MODE_CONTENT_PROTECTION_DESIRED)
-		intel_hdcp_enable(to_intel_connector(conn_state->connector));
 }
 
 static void intel_disable_ddi_dp(struct intel_encoder *encoder,
@@ -3513,8 +3508,6 @@  static void intel_disable_ddi(struct intel_encoder *encoder,
 			      const struct intel_crtc_state *old_crtc_state,
 			      const struct drm_connector_state *old_conn_state)
 {
-	intel_hdcp_disable(to_intel_connector(old_conn_state->connector));
-
 	if (intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_HDMI))
 		intel_disable_ddi_hdmi(encoder, old_crtc_state, old_conn_state);
 	else
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index fc63babce165..98aaf536146f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12854,6 +12854,8 @@  static void intel_atomic_commit_tail(struct drm_atomic_state *state)
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
 	struct intel_crtc_state *new_intel_crtc_state, *old_intel_crtc_state;
+	struct drm_connector_state *old_conn_state, *new_conn_state;
+	struct drm_connector *connector;
 	struct drm_crtc *crtc;
 	struct intel_crtc *intel_crtc;
 	u64 put_domains[I915_MAX_PIPES] = {};
@@ -12947,9 +12949,17 @@  static void intel_atomic_commit_tail(struct drm_atomic_state *state)
 		}
 	}
 
+	for_each_oldnew_connector_in_state(state, connector, old_conn_state,
+					   new_conn_state, i)
+		intel_hdcp_atomic_pre_commit(connector, old_conn_state,
+					     new_conn_state);
+
 	/* Now enable the clocks, plane, pipe, and connectors that we set up. */
 	dev_priv->display.update_crtcs(state);
 
+	for_each_new_connector_in_state(state, connector, new_conn_state, i)
+		intel_hdcp_atomic_commit(connector, new_conn_state);
+
 	/* FIXME: We should call drm_atomic_helper_commit_hw_done() here
 	 * already, but still need the state for the delayed optimization. To
 	 * fix this:
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 2b7181b76475..68eae1e569e4 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -2084,6 +2084,11 @@  static inline void intel_backlight_device_unregister(struct intel_connector *con
 void intel_hdcp_atomic_check(struct drm_connector *connector,
 			     struct drm_connector_state *old_state,
 			     struct drm_connector_state *new_state);
+void intel_hdcp_atomic_pre_commit(struct drm_connector *connector,
+				  struct drm_connector_state *old_state,
+				  struct drm_connector_state *new_state);
+void intel_hdcp_atomic_commit(struct drm_connector *connector,
+			      struct drm_connector_state *new_state);
 int intel_hdcp_init(struct intel_connector *connector,
 		    const struct intel_hdcp_shim *hdcp_shim,
 		    bool hdcp2_supported);
diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
index be475914c3cd..9f4056e156ec 100644
--- a/drivers/gpu/drm/i915/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/intel_hdcp.c
@@ -2005,7 +2005,6 @@  void intel_hdcp_atomic_check(struct drm_connector *connector,
 {
 	uint64_t old_cp = old_state->content_protection;
 	uint64_t new_cp = new_state->content_protection;
-	struct drm_crtc_state *crtc_state;
 
 	if (!new_state->crtc) {
 		/*
@@ -2026,10 +2025,35 @@  void intel_hdcp_atomic_check(struct drm_connector *connector,
 	    (old_cp == DRM_MODE_CONTENT_PROTECTION_DESIRED &&
 	     new_cp == DRM_MODE_CONTENT_PROTECTION_ENABLED))
 		return;
+}
+
+void intel_hdcp_atomic_pre_commit(struct drm_connector *connector,
+				  struct drm_connector_state *old_state,
+				  struct drm_connector_state *new_state)
+{
+	u64 old_cp = old_state->content_protection;
+	u64 new_cp = new_state->content_protection;
+
+	/*
+	 * Disable HDCP if the connector is becoming disabled, or if requested
+	 * via the property.
+	 */
+	if ((!new_state->crtc &&
+	     old_cp != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) ||
+	    (new_state->crtc &&
+	     old_cp != DRM_MODE_CONTENT_PROTECTION_UNDESIRED &&
+	     new_cp == DRM_MODE_CONTENT_PROTECTION_UNDESIRED))
+		intel_hdcp_disable(to_intel_connector(connector));
+}
+
+void intel_hdcp_atomic_commit(struct drm_connector *connector,
+			      struct drm_connector_state *new_state)
+{
+	u64 new_cp = new_state->content_protection;
 
-	crtc_state = drm_atomic_get_new_crtc_state(new_state->state,
-						   new_state->crtc);
-	crtc_state->mode_changed = true;
+	/* Enable hdcp if it's desired */
+	if (new_state->crtc && new_cp == DRM_MODE_CONTENT_PROTECTION_DESIRED)
+		intel_hdcp_enable(to_intel_connector(connector));
 }
 
 /* Handles the CP_IRQ raised from the DP HDCP sink */

Comments

On Tue, Nov 27, 2018 at 04:13:30PM +0530, Ramalingam C wrote:
> Commits the content protection change of a connector,
> without crtc modeset. This improves the user experience.
> 
> Originally proposed by Sean Paul at v3 of HDCP1.4 framework
> https://patchwork.freedesktop.org/patch/191759/. For some
> reason this was dropped, but needed for the proper functionality
> of HDCP encryption.
> 
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> Suggested-by: Sean Paul <seanpaul@chromium.org>

I think best to split this out from this series, since it's a separate
concern really.

Wrt implementation, this problem here is pretty similar to other sink
setup stuff like infoframes, where atm we also require a full modeset. The
idea that iirc Ville&me toyed around with is a kind of
per-encoder/connector fixup function, where all this stuff could be set if
we did _not_ do a full modeset. For a full modeset I think it's best to
still keep all this in connectors.

Plus for anything where we do this kind of fast-modeset trick we need to
lock down the state tracking using the state readout code. Otherwise this
is very hard to validate properly.
-Daniel
> ---
>  drivers/gpu/drm/i915/intel_ddi.c     |  7 -------
>  drivers/gpu/drm/i915/intel_display.c | 10 ++++++++++
>  drivers/gpu/drm/i915/intel_drv.h     |  5 +++++
>  drivers/gpu/drm/i915/intel_hdcp.c    | 32 ++++++++++++++++++++++++++++----
>  4 files changed, 43 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index ad11540ac436..128cc558d75f 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -3469,11 +3469,6 @@ static void intel_enable_ddi(struct intel_encoder *encoder,
>  		intel_enable_ddi_hdmi(encoder, crtc_state, conn_state);
>  	else
>  		intel_enable_ddi_dp(encoder, crtc_state, conn_state);
> -
> -	/* Enable hdcp if it's desired */
> -	if (conn_state->content_protection ==
> -	    DRM_MODE_CONTENT_PROTECTION_DESIRED)
> -		intel_hdcp_enable(to_intel_connector(conn_state->connector));
>  }
>  
>  static void intel_disable_ddi_dp(struct intel_encoder *encoder,
> @@ -3513,8 +3508,6 @@ static void intel_disable_ddi(struct intel_encoder *encoder,
>  			      const struct intel_crtc_state *old_crtc_state,
>  			      const struct drm_connector_state *old_conn_state)
>  {
> -	intel_hdcp_disable(to_intel_connector(old_conn_state->connector));
> -
>  	if (intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_HDMI))
>  		intel_disable_ddi_hdmi(encoder, old_crtc_state, old_conn_state);
>  	else
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index fc63babce165..98aaf536146f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12854,6 +12854,8 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
>  	struct intel_crtc_state *new_intel_crtc_state, *old_intel_crtc_state;
> +	struct drm_connector_state *old_conn_state, *new_conn_state;
> +	struct drm_connector *connector;
>  	struct drm_crtc *crtc;
>  	struct intel_crtc *intel_crtc;
>  	u64 put_domains[I915_MAX_PIPES] = {};
> @@ -12947,9 +12949,17 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
>  		}
>  	}
>  
> +	for_each_oldnew_connector_in_state(state, connector, old_conn_state,
> +					   new_conn_state, i)
> +		intel_hdcp_atomic_pre_commit(connector, old_conn_state,
> +					     new_conn_state);
> +
>  	/* Now enable the clocks, plane, pipe, and connectors that we set up. */
>  	dev_priv->display.update_crtcs(state);
>  
> +	for_each_new_connector_in_state(state, connector, new_conn_state, i)
> +		intel_hdcp_atomic_commit(connector, new_conn_state);
> +
>  	/* FIXME: We should call drm_atomic_helper_commit_hw_done() here
>  	 * already, but still need the state for the delayed optimization. To
>  	 * fix this:
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 2b7181b76475..68eae1e569e4 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -2084,6 +2084,11 @@ static inline void intel_backlight_device_unregister(struct intel_connector *con
>  void intel_hdcp_atomic_check(struct drm_connector *connector,
>  			     struct drm_connector_state *old_state,
>  			     struct drm_connector_state *new_state);
> +void intel_hdcp_atomic_pre_commit(struct drm_connector *connector,
> +				  struct drm_connector_state *old_state,
> +				  struct drm_connector_state *new_state);
> +void intel_hdcp_atomic_commit(struct drm_connector *connector,
> +			      struct drm_connector_state *new_state);
>  int intel_hdcp_init(struct intel_connector *connector,
>  		    const struct intel_hdcp_shim *hdcp_shim,
>  		    bool hdcp2_supported);
> diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
> index be475914c3cd..9f4056e156ec 100644
> --- a/drivers/gpu/drm/i915/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/intel_hdcp.c
> @@ -2005,7 +2005,6 @@ void intel_hdcp_atomic_check(struct drm_connector *connector,
>  {
>  	uint64_t old_cp = old_state->content_protection;
>  	uint64_t new_cp = new_state->content_protection;
> -	struct drm_crtc_state *crtc_state;
>  
>  	if (!new_state->crtc) {
>  		/*
> @@ -2026,10 +2025,35 @@ void intel_hdcp_atomic_check(struct drm_connector *connector,
>  	    (old_cp == DRM_MODE_CONTENT_PROTECTION_DESIRED &&
>  	     new_cp == DRM_MODE_CONTENT_PROTECTION_ENABLED))
>  		return;
> +}
> +
> +void intel_hdcp_atomic_pre_commit(struct drm_connector *connector,
> +				  struct drm_connector_state *old_state,
> +				  struct drm_connector_state *new_state)
> +{
> +	u64 old_cp = old_state->content_protection;
> +	u64 new_cp = new_state->content_protection;
> +
> +	/*
> +	 * Disable HDCP if the connector is becoming disabled, or if requested
> +	 * via the property.
> +	 */
> +	if ((!new_state->crtc &&
> +	     old_cp != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) ||
> +	    (new_state->crtc &&
> +	     old_cp != DRM_MODE_CONTENT_PROTECTION_UNDESIRED &&
> +	     new_cp == DRM_MODE_CONTENT_PROTECTION_UNDESIRED))
> +		intel_hdcp_disable(to_intel_connector(connector));
> +}
> +
> +void intel_hdcp_atomic_commit(struct drm_connector *connector,
> +			      struct drm_connector_state *new_state)
> +{
> +	u64 new_cp = new_state->content_protection;
>  
> -	crtc_state = drm_atomic_get_new_crtc_state(new_state->state,
> -						   new_state->crtc);
> -	crtc_state->mode_changed = true;
> +	/* Enable hdcp if it's desired */
> +	if (new_state->crtc && new_cp == DRM_MODE_CONTENT_PROTECTION_DESIRED)
> +		intel_hdcp_enable(to_intel_connector(connector));
>  }
>  
>  /* Handles the CP_IRQ raised from the DP HDCP sink */
> -- 
> 2.7.4
>