Re: [Intel-gfx] [PATCH 4/4] drm/i915: Don't allocate extra ddb during async flip for DG2

["Lisovskiy, Stanislav" <stanislav.lisovskiy@intel.com>]

On Mon, Jan 24, 2022 at 08:55:57PM +0200, Ville Syrjälä wrote:
> On Mon, Jan 24, 2022 at 03:52:34PM +0200, Stanislav Lisovskiy wrote:
> > In terms of async flip optimization we don't to allocate
> > extra ddb space, so lets skip it.
> > 
> > v2: - Extracted min ddb async flip check to separate function
> >       (Ville Syrjälä)
> >     - Used this function to prevent false positive WARN
> >       to be triggered(Ville Syrjälä)
> > 
> > v3: - Renamed dg2_need_min_ddb to need_min_ddb thus making
> >       it more universal.
> >     - Also used DISPLAY_VER instead of IS_DG2(Ville Syrjälä)
> >     - Use rate = 0 instead of just setting extra = 0, thus
> >       letting other planes to use extra ddb and avoiding WARN
> >       (Ville Syrjälä)
> > 
> > v4: - Renamed needs_min_ddb as s/needs/use/ to match
> >       the wm0 counterpart(Ville Syrjälä)
> >     - Added plane->async_flip check to use_min_ddb(now
> >       passing plane as a parameter to do that)(Ville Syrjälä)
> >     - Account for use_min_ddb also when calculating total data rate
> >       (Ville Syrjälä)
> > 
> > v5:
> >     - Use for_each_intel_plane_on_crtc instead of for_each_intel_plane_id
> >       to get plane->async_flip check and account for all planes(Ville Syrjälä)
> >     - Fix line wrapping(Ville Syrjälä)
> >     - Set plane data rate conditionally, avoiding on redundant assignment
> >       (Ville Syrjälä)
> >     - Removed redundant whitespace(Ville Syrjälä)
> >     - Handle use_min_ddb case in skl_plane_relative_data_rate instead of
> >       icl_get_total_relative_data_rate(Ville Syrjälä)
> > 
> > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c | 18 ++++++++++++++++++
> >  1 file changed, 18 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 0bb4c941f950..bb147e5a77b6 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -4906,6 +4906,16 @@ static u8 skl_compute_dbuf_slices(struct intel_crtc *crtc, u8 active_pipes)
> >  	return active_pipes & BIT(pipe) ? BIT(DBUF_S1) : 0;
> >  }
> >  
> > +static bool use_min_ddb(const struct intel_crtc_state *crtc_state,
> > +			struct intel_plane *plane)
> > +{
> > +	struct drm_i915_private *i915 = to_i915(plane->base.dev);
> > +
> > +	return DISPLAY_VER(i915) >= 13 &&
> > +	       crtc_state->uapi.async_flip &&
> > +	       plane->async_flip;
> > +}
> > +
> >  static bool use_minimal_wm0_only(const struct intel_crtc_state *crtc_state,
> >  				 struct intel_plane *plane)
> >  {
> > @@ -4934,6 +4944,14 @@ skl_plane_relative_data_rate(const struct intel_crtc_state *crtc_state,
> >  	if (plane->id == PLANE_CURSOR)
> >  		return 0;
> >  
> > +	/*
> > +	 * We calculate extra ddb based on ratio plane rate/total data rate
> > +	 * in case, in some cases we should not allocate extra ddb for the plane,
> > +	 * so do not count its data rate, if this is the case.
> > +	 */
> > +	if (use_min_ddb(crtc_state, plane))
> > +		return 0;
> > +
> >  	if (color_plane == 1 &&
> >  	    !intel_format_info_is_yuv_semiplanar(fb->format, fb->modifier))
> >  		return 0;
> 
> Yeah this looks nice and simple. Only minor nit is that I'd probably
> have put it after this ccs vs. planar related thing which is a more
> static decision than the async flip optimization.
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

I guess the first patch in this series is also ok?

Just wondering, if I can push it now.

Stan

> 
> > -- 
> > 2.24.1.485.gad05a3d8e5
> 
> -- 
> Ville Syrjälä
> Intel