[v3,1/5] drm/i915: Decouple cdclk calculation from modeset checks
diff mbox series

Message ID 20200330122354.24752-2-stanislav.lisovskiy@intel.com
State New
Headers show
Series
  • Consider DBuf bandwidth when calculating CDCLK
Related show

Commit Message

Lisovskiy, Stanislav March 30, 2020, 12:23 p.m. UTC
We need to calculate cdclk after watermarks/ddb has been calculated
as with recent hw CDCLK needs to be adjusted accordingly to DBuf
requirements, which is not possible with current code organization.

Setting CDCLK according to DBuf BW requirements and not just rejecting
if it doesn't satisfy BW requirements, will allow us to save power when
it is possible and gain additional bandwidth when it's needed - i.e
boosting both our power management and perfomance capabilities.

This patch is preparation for that, first we now extract modeset
calculation from modeset checks, in order to call it after wm/ddb
has been calculated.

v2: - Extract only intel_modeset_calc_cdclk from intel_modeset_checks
      (Ville Syrjälä)

Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

Comments

Ville Syrjälä March 30, 2020, 3:44 p.m. UTC | #1
On Mon, Mar 30, 2020 at 03:23:50PM +0300, Stanislav Lisovskiy wrote:
> We need to calculate cdclk after watermarks/ddb has been calculated
> as with recent hw CDCLK needs to be adjusted accordingly to DBuf
> requirements, which is not possible with current code organization.
> 
> Setting CDCLK according to DBuf BW requirements and not just rejecting
> if it doesn't satisfy BW requirements, will allow us to save power when
> it is possible and gain additional bandwidth when it's needed - i.e
> boosting both our power management and perfomance capabilities.
> 
> This patch is preparation for that, first we now extract modeset
> calculation from modeset checks, in order to call it after wm/ddb
> has been calculated.
> 
> v2: - Extract only intel_modeset_calc_cdclk from intel_modeset_checks
>       (Ville Syrjälä)
> 
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 7c45d676c9b7..17d83f37f49f 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -14545,10 +14545,6 @@ static int intel_modeset_checks(struct intel_atomic_state *state)
>  			return ret;
>  	}
>  
> -	ret = intel_modeset_calc_cdclk(state);
> -	if (ret)
> -		return ret;
> -
>  	intel_modeset_clear_plls(state);
>  
>  	if (IS_HASWELL(dev_priv))
> @@ -14882,10 +14878,6 @@ static int intel_atomic_check(struct drm_device *dev,
>  			goto fail;
>  	}
>  
> -	ret = intel_atomic_check_crtcs(state);
> -	if (ret)
> -		goto fail;
> -
>  	intel_fbc_choose_crtc(dev_priv, state);
>  	ret = calc_watermark_data(state);
>  	if (ret)
> @@ -14895,6 +14887,16 @@ static int intel_atomic_check(struct drm_device *dev,
>  	if (ret)
>  		goto fail;
>  
> +	if (any_ms) {
> +		ret = intel_modeset_calc_cdclk(state);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ret = intel_atomic_check_crtcs(state);
> +	if (ret)
> +		goto fail;

I was thinking we'd do this as two patches. One with just the
extraction, and another one with the bigger reordering. But I think I
convinced myself that it should be safe, so maybe a single patch is
fine.

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

> +
>  	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
>  					    new_crtc_state, i) {
>  		if (!needs_modeset(new_crtc_state) &&
> -- 
> 2.24.1.485.gad05a3d8e5

Patch
diff mbox series

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 7c45d676c9b7..17d83f37f49f 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -14545,10 +14545,6 @@  static int intel_modeset_checks(struct intel_atomic_state *state)
 			return ret;
 	}
 
-	ret = intel_modeset_calc_cdclk(state);
-	if (ret)
-		return ret;
-
 	intel_modeset_clear_plls(state);
 
 	if (IS_HASWELL(dev_priv))
@@ -14882,10 +14878,6 @@  static int intel_atomic_check(struct drm_device *dev,
 			goto fail;
 	}
 
-	ret = intel_atomic_check_crtcs(state);
-	if (ret)
-		goto fail;
-
 	intel_fbc_choose_crtc(dev_priv, state);
 	ret = calc_watermark_data(state);
 	if (ret)
@@ -14895,6 +14887,16 @@  static int intel_atomic_check(struct drm_device *dev,
 	if (ret)
 		goto fail;
 
+	if (any_ms) {
+		ret = intel_modeset_calc_cdclk(state);
+		if (ret)
+			return ret;
+	}
+
+	ret = intel_atomic_check_crtcs(state);
+	if (ret)
+		goto fail;
+
 	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
 					    new_crtc_state, i) {
 		if (!needs_modeset(new_crtc_state) &&