[Intel-gfx] [PATCH 2/2] drm/i915: Stop adding planes to the commit needlessly

[Lisovskiy, Stanislav]

On Thu, Mar 25, 2021 at 02:44:15AM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> The dbuf bandwidth calculations don't need the planes to be
> added to the state. Each plane's data rate has already been
> precalculated and stored in the crtc state, and that with
> the dbuf slice usage for each plane is all the dbuf bandwidth
> code needs to figure out what the minimum cdclk is.
> 
> What we're trying to do here is make sure each plane recalculates
> its minimum cdclk (ie. plane->min_cdclk()) on those platforms where
> the number of active planes affects the result of said calculation.
> Nothing to do with any dbuf cdclk requirements.

So does it mean that if we lets say had active plane mask as
011(planes 0, 1 were active) and new active planes are 101(planes 0, 2
are active) - we should not add plane 2 to the state?
Because hamming weight will be obviously same, however I think it would
be wrong not have plane 2 in the state at all then..

Or will it be added somewhere else?


Stan

> 
> Not sure if we had stuff in slightly different order or what,
> but at least in the current scheme this is not necessary.
> 
> Cc: Stanislav Lisovskiy <stanislav.lisovskiy at intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 17490d29dc13..2300d58ba47f 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -9811,7 +9811,7 @@ static bool active_planes_affects_min_cdclk(struct drm_i915_private *dev_priv)
>  	/* See {hsw,vlv,ivb}_plane_ratio() */
>  	return IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv) ||
>  		IS_CHERRYVIEW(dev_priv) || IS_VALLEYVIEW(dev_priv) ||
> -		IS_IVYBRIDGE(dev_priv) || (DISPLAY_VER(dev_priv) >= 11);
> +		IS_IVYBRIDGE(dev_priv);
>  }
>  
>  static int intel_crtc_add_bigjoiner_planes(struct intel_atomic_state *state,
> @@ -9898,13 +9898,7 @@ static int intel_atomic_check_planes(struct intel_atomic_state *state)
>  		old_active_planes = old_crtc_state->active_planes & ~BIT(PLANE_CURSOR);
>  		new_active_planes = new_crtc_state->active_planes & ~BIT(PLANE_CURSOR);
>  
> -		/*
> -		 * Not only the number of planes, but if the plane configuration had
> -		 * changed might already mean we need to recompute min CDCLK,
> -		 * because different planes might consume different amount of Dbuf bandwidth
> -		 * according to formula: Bw per plane = Pixel rate * bpp * pipe/plane scale factor
> -		 */
> -		if (old_active_planes == new_active_planes)
> +		if (hweight8(old_active_planes) == hweight8(new_active_planes))
>  			continue;
>  
>  		ret = intel_crtc_add_planes_to_state(state, crtc, new_active_planes);
> -- 
> 2.26.2
>