[2/8] drm/i915: Nuke 'realloc_pipes'
diff mbox series

Message ID 20191011200949.7839-3-ville.syrjala@linux.intel.com
State New
Headers show
Series
  • drm/i915: Some cleanup near the SKL wm/ddb area
Related show

Commit Message

Ville Syrjälä Oct. 11, 2019, 8:09 p.m. UTC
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The 'realloc_pipes' bitmask is pointless. It is either:
a) the set of pipes which are already part of the state,
   in which case adding them again is entirely redundant
b) the set of all pipes which we then add to the state

Also the fact that 'realloc_pipes' uses the crtc indexes is
going to bite is at some point so best get rid of it quick.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 50 ++++++++++++++-------------------
 1 file changed, 21 insertions(+), 29 deletions(-)

Comments

Stanislav Lisovskiy Oct. 16, 2019, 2:27 p.m. UTC | #1
On Fri, 2019-10-11 at 23:09 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The 'realloc_pipes' bitmask is pointless. It is either:
> a) the set of pipes which are already part of the state,
>    in which case adding them again is entirely redundant
> b) the set of all pipes which we then add to the state
> 
> Also the fact that 'realloc_pipes' uses the crtc indexes is
> going to bite is at some point so best get rid of it quick.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 50 ++++++++++++++-----------------
> --
>  1 file changed, 21 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c
> index 49568270a89d..3536c2e975e7 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5235,19 +5235,6 @@ bool skl_ddb_allocation_overlaps(const struct
> skl_ddb_entry *ddb,
>  	return false;
>  }
>  
> -static u32
> -pipes_modified(struct intel_atomic_state *state)
> -{
> -	struct intel_crtc *crtc;
> -	struct intel_crtc_state *crtc_state;
> -	u32 i, ret = 0;
> -
> -	for_each_new_intel_crtc_in_state(state, crtc, crtc_state, i)
> -		ret |= drm_crtc_mask(&crtc->base);
> -
> -	return ret;
> -}
> -
>  static int
>  skl_ddb_add_affected_planes(const struct intel_crtc_state
> *old_crtc_state,
>  			    struct intel_crtc_state *new_crtc_state)
> @@ -5423,14 +5410,26 @@ skl_print_wm_changes(struct
> intel_atomic_state *state)
>  	}
>  }
>  
> +static int intel_add_all_pipes(struct intel_atomic_state *state)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> +	struct intel_crtc *crtc;
> +
> +	for_each_intel_crtc(&dev_priv->drm, crtc) {
> +		struct intel_crtc_state *crtc_state;
> +
> +		crtc_state = intel_atomic_get_crtc_state(&state->base,
> crtc);
> +		if (IS_ERR(crtc_state))
> +			return PTR_ERR(crtc_state);
> +	}
> +
> +	return 0;
> +}
> +
>  static int
>  skl_ddb_add_affected_pipes(struct intel_atomic_state *state)
>  {
> -	struct drm_device *dev = state->base.dev;
> -	const struct drm_i915_private *dev_priv = to_i915(dev);
> -	struct intel_crtc *crtc;
> -	struct intel_crtc_state *crtc_state;
> -	u32 realloc_pipes = pipes_modified(state);
> +	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
>  	int ret;
>  
>  	/*
> @@ -5440,7 +5439,7 @@ skl_ddb_add_affected_pipes(struct
> intel_atomic_state *state)
>  	 * ensure a full DDB recompute.
>  	 */
>  	if (dev_priv->wm.distrust_bios_wm) {
> -		ret = drm_modeset_lock(&dev-
> >mode_config.connection_mutex,
> +		ret = drm_modeset_lock(&dev_priv-
> >drm.mode_config.connection_mutex,
>  				       state->base.acquire_ctx);
>  		if (ret)
>  			return ret;
> @@ -5471,18 +5470,11 @@ skl_ddb_add_affected_pipes(struct
> intel_atomic_state *state)
>  	 * to grab the lock on *all* CRTC's.
>  	 */
>  	if (state->active_pipe_changes || state->modeset) {
> -		realloc_pipes = ~0;
>  		state->wm_results.dirty_pipes = ~0;
> -	}
>  
> -	/*
> -	 * We're not recomputing for the pipes not included in the
> commit, so
> -	 * make sure we start with the current state.
> -	 */
> -	for_each_intel_crtc_mask(dev, crtc, realloc_pipes) {
> -		crtc_state = intel_atomic_get_crtc_state(&state->base,
> crtc);
> -		if (IS_ERR(crtc_state))
> -			return PTR_ERR(crtc_state);
> +		ret = intel_add_all_pipes(state);
> +		if (ret)
> +			return ret;
>  	}
>  
>  	return 0;

Patch
diff mbox series

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 49568270a89d..3536c2e975e7 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5235,19 +5235,6 @@  bool skl_ddb_allocation_overlaps(const struct skl_ddb_entry *ddb,
 	return false;
 }
 
-static u32
-pipes_modified(struct intel_atomic_state *state)
-{
-	struct intel_crtc *crtc;
-	struct intel_crtc_state *crtc_state;
-	u32 i, ret = 0;
-
-	for_each_new_intel_crtc_in_state(state, crtc, crtc_state, i)
-		ret |= drm_crtc_mask(&crtc->base);
-
-	return ret;
-}
-
 static int
 skl_ddb_add_affected_planes(const struct intel_crtc_state *old_crtc_state,
 			    struct intel_crtc_state *new_crtc_state)
@@ -5423,14 +5410,26 @@  skl_print_wm_changes(struct intel_atomic_state *state)
 	}
 }
 
+static int intel_add_all_pipes(struct intel_atomic_state *state)
+{
+	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
+	struct intel_crtc *crtc;
+
+	for_each_intel_crtc(&dev_priv->drm, crtc) {
+		struct intel_crtc_state *crtc_state;
+
+		crtc_state = intel_atomic_get_crtc_state(&state->base, crtc);
+		if (IS_ERR(crtc_state))
+			return PTR_ERR(crtc_state);
+	}
+
+	return 0;
+}
+
 static int
 skl_ddb_add_affected_pipes(struct intel_atomic_state *state)
 {
-	struct drm_device *dev = state->base.dev;
-	const struct drm_i915_private *dev_priv = to_i915(dev);
-	struct intel_crtc *crtc;
-	struct intel_crtc_state *crtc_state;
-	u32 realloc_pipes = pipes_modified(state);
+	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
 	int ret;
 
 	/*
@@ -5440,7 +5439,7 @@  skl_ddb_add_affected_pipes(struct intel_atomic_state *state)
 	 * ensure a full DDB recompute.
 	 */
 	if (dev_priv->wm.distrust_bios_wm) {
-		ret = drm_modeset_lock(&dev->mode_config.connection_mutex,
+		ret = drm_modeset_lock(&dev_priv->drm.mode_config.connection_mutex,
 				       state->base.acquire_ctx);
 		if (ret)
 			return ret;
@@ -5471,18 +5470,11 @@  skl_ddb_add_affected_pipes(struct intel_atomic_state *state)
 	 * to grab the lock on *all* CRTC's.
 	 */
 	if (state->active_pipe_changes || state->modeset) {
-		realloc_pipes = ~0;
 		state->wm_results.dirty_pipes = ~0;
-	}
 
-	/*
-	 * We're not recomputing for the pipes not included in the commit, so
-	 * make sure we start with the current state.
-	 */
-	for_each_intel_crtc_mask(dev, crtc, realloc_pipes) {
-		crtc_state = intel_atomic_get_crtc_state(&state->base, crtc);
-		if (IS_ERR(crtc_state))
-			return PTR_ERR(crtc_state);
+		ret = intel_add_all_pipes(state);
+		if (ret)
+			return ret;
 	}
 
 	return 0;