diff mbox series

[v8,2/7] drm/i915: Extract cdclk requirements checking to separate function

Message ID 20200514152145.2449-3-stanislav.lisovskiy@intel.com (mailing list archive)
State New, archived
Headers show
Series Consider DBuf bandwidth when calculating CDCLK | expand

Commit Message

Stanislav Lisovskiy May 14, 2020, 3:21 p.m. UTC
In Gen11+ whenever we might exceed DBuf bandwidth we might need to
recalculate CDCLK which DBuf bandwidth is scaled with.
Total Dbuf bw used might change based on particular plane needs.

Thus to calculate if cdclk needs to be changed it is not enough
anymore to check plane configuration and plane min cdclk, per DBuf
bw can be calculated only after wm/ddb calculation is done and
all required planes are added into the state. In order to keep
all min_cdclk related checks in one place let's extract it into
separate function, checking and modifying any_ms.

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

Comments

Navare, Manasi May 14, 2020, 7:36 p.m. UTC | #1
On Thu, May 14, 2020 at 06:21:40PM +0300, Stanislav Lisovskiy wrote:
> In Gen11+ whenever we might exceed DBuf bandwidth we might need to
> recalculate CDCLK which DBuf bandwidth is scaled with.
> Total Dbuf bw used might change based on particular plane needs.
> 
> Thus to calculate if cdclk needs to be changed it is not enough
> anymore to check plane configuration and plane min cdclk, per DBuf
> bw can be calculated only after wm/ddb calculation is done and
> all required planes are added into the state. In order to keep
> all min_cdclk related checks in one place let's extract it into
> separate function, checking and modifying any_ms.
> 
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>

Separating the need_cdclk_calc setting into a new function intel_atomic_check_cdclk()
makes sense.

Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>

Manasi

> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 30 ++++++++++++++------
>  1 file changed, 22 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 005e324d0582..e93a553a344d 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -14572,8 +14572,7 @@ static bool active_planes_affects_min_cdclk(struct drm_i915_private *dev_priv)
>  		IS_IVYBRIDGE(dev_priv);
>  }
>  
> -static int intel_atomic_check_planes(struct intel_atomic_state *state,
> -				     bool *need_cdclk_calc)
> +static int intel_atomic_check_planes(struct intel_atomic_state *state)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
>  	struct intel_crtc_state *old_crtc_state, *new_crtc_state;
> @@ -14623,6 +14622,22 @@ static int intel_atomic_check_planes(struct intel_atomic_state *state,
>  			return ret;
>  	}
>  
> +	return 0;
> +}
> +
> +static int intel_atomic_check_cdclk(struct intel_atomic_state *state,
> +				    bool *need_cdclk_calc)
> +{
> +	struct intel_cdclk_state *new_cdclk_state;
> +	int i;
> +	struct intel_plane_state *plane_state;
> +	struct intel_plane *plane;
> +	int ret;
> +
> +	new_cdclk_state = intel_atomic_get_new_cdclk_state(state);
> +	if (new_cdclk_state && new_cdclk_state->force_min_cdclk_changed)
> +		*need_cdclk_calc = true;
> +
>  	/*
>  	 * active_planes bitmask has been updated, and potentially
>  	 * affected planes are part of the state. We can now
> @@ -14685,7 +14700,6 @@ static int intel_atomic_check(struct drm_device *dev,
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct intel_atomic_state *state = to_intel_atomic_state(_state);
>  	struct intel_crtc_state *old_crtc_state, *new_crtc_state;
> -	struct intel_cdclk_state *new_cdclk_state;
>  	struct intel_crtc *crtc;
>  	int ret, i;
>  	bool any_ms = false;
> @@ -14796,14 +14810,10 @@ static int intel_atomic_check(struct drm_device *dev,
>  	if (ret)
>  		goto fail;
>  
> -	ret = intel_atomic_check_planes(state, &any_ms);
> +	ret = intel_atomic_check_planes(state);
>  	if (ret)
>  		goto fail;
>  
> -	new_cdclk_state = intel_atomic_get_new_cdclk_state(state);
> -	if (new_cdclk_state && new_cdclk_state->force_min_cdclk_changed)
> -		any_ms = true;
> -
>  	/*
>  	 * distrust_bios_wm will force a full dbuf recomputation
>  	 * but the hardware state will only get updated accordingly
> @@ -14833,6 +14843,10 @@ static int intel_atomic_check(struct drm_device *dev,
>  	if (ret)
>  		goto fail;
>  
> +	ret = intel_atomic_check_cdclk(state, &any_ms);
> +	if (ret)
> +		goto fail;
> +
>  	if (any_ms) {
>  		ret = intel_modeset_calc_cdclk(state);
>  		if (ret)
> -- 
> 2.24.1.485.gad05a3d8e5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 005e324d0582..e93a553a344d 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -14572,8 +14572,7 @@  static bool active_planes_affects_min_cdclk(struct drm_i915_private *dev_priv)
 		IS_IVYBRIDGE(dev_priv);
 }
 
-static int intel_atomic_check_planes(struct intel_atomic_state *state,
-				     bool *need_cdclk_calc)
+static int intel_atomic_check_planes(struct intel_atomic_state *state)
 {
 	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
 	struct intel_crtc_state *old_crtc_state, *new_crtc_state;
@@ -14623,6 +14622,22 @@  static int intel_atomic_check_planes(struct intel_atomic_state *state,
 			return ret;
 	}
 
+	return 0;
+}
+
+static int intel_atomic_check_cdclk(struct intel_atomic_state *state,
+				    bool *need_cdclk_calc)
+{
+	struct intel_cdclk_state *new_cdclk_state;
+	int i;
+	struct intel_plane_state *plane_state;
+	struct intel_plane *plane;
+	int ret;
+
+	new_cdclk_state = intel_atomic_get_new_cdclk_state(state);
+	if (new_cdclk_state && new_cdclk_state->force_min_cdclk_changed)
+		*need_cdclk_calc = true;
+
 	/*
 	 * active_planes bitmask has been updated, and potentially
 	 * affected planes are part of the state. We can now
@@ -14685,7 +14700,6 @@  static int intel_atomic_check(struct drm_device *dev,
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_atomic_state *state = to_intel_atomic_state(_state);
 	struct intel_crtc_state *old_crtc_state, *new_crtc_state;
-	struct intel_cdclk_state *new_cdclk_state;
 	struct intel_crtc *crtc;
 	int ret, i;
 	bool any_ms = false;
@@ -14796,14 +14810,10 @@  static int intel_atomic_check(struct drm_device *dev,
 	if (ret)
 		goto fail;
 
-	ret = intel_atomic_check_planes(state, &any_ms);
+	ret = intel_atomic_check_planes(state);
 	if (ret)
 		goto fail;
 
-	new_cdclk_state = intel_atomic_get_new_cdclk_state(state);
-	if (new_cdclk_state && new_cdclk_state->force_min_cdclk_changed)
-		any_ms = true;
-
 	/*
 	 * distrust_bios_wm will force a full dbuf recomputation
 	 * but the hardware state will only get updated accordingly
@@ -14833,6 +14843,10 @@  static int intel_atomic_check(struct drm_device *dev,
 	if (ret)
 		goto fail;
 
+	ret = intel_atomic_check_cdclk(state, &any_ms);
+	if (ret)
+		goto fail;
+
 	if (any_ms) {
 		ret = intel_modeset_calc_cdclk(state);
 		if (ret)