Re: [Intel-gfx] [PATCH] drm/i915: Implement a bit of bw_state readout

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Aug 25, 2022 at 10:54:48AM +0300, Lisovskiy, Stanislav wrote:
> On Fri, Jun 17, 2022 at 07:07:30PM +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > We currently fail to reconstruct the bw related cdclk limits
> > during readout, which triggers a  cdclk reclaculation during
> > fastboot, which is then likely forces a full modeset anyway.
> > Reconstruct some of the missing state so that we can skip
> > the cdclk recomputation and thus have a higher chance for
> > flicker free boot.
> 
> Problem is that intel_bw_min_cdclk is using intel_bw_dbuf_min_cdclk,
> which in turns relies that bw_state->dbuf_bw was already calculated,
> however this is calculated in intel_bw_calc_min_cdclk, which is called
> from intel_cdclk_atomic_check.
> 
> So wondering will that work?

Hmm. I guess we're more or less just missing a call to
skl_crtc_calc_dbuf_bw(), but I don't have all that code paged
in atm so might be missing something.

So as is this is probably not a complete solution, but it did
avoid the cdclk modeset on some machine I tested. I guess that
one was a tgl and thus the maximum pipe read bandwidth stuff
kicked in and populated things suffiiciently to avoid the full
cdclk recalculation. I'll have to try this on some older machine
to check how it behaves...

> 
> Stan
> 
> > 
> > Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_bw.c      | 9 ++++++---
> >  drivers/gpu/drm/i915/display/intel_display.c | 7 +++++--
> >  2 files changed, 11 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
> > index 79269d2c476b..30ffec63f9a3 100644
> > --- a/drivers/gpu/drm/i915/display/intel_bw.c
> > +++ b/drivers/gpu/drm/i915/display/intel_bw.c
> > @@ -627,11 +627,14 @@ void intel_bw_crtc_update(struct intel_bw_state *bw_state,
> >  		intel_bw_crtc_data_rate(crtc_state);
> >  	bw_state->num_active_planes[crtc->pipe] =
> >  		intel_bw_crtc_num_active_planes(crtc_state);
> > +	bw_state->min_cdclk[crtc->pipe] =
> > +		intel_bw_crtc_min_cdclk(crtc_state);
> >  
> > -	drm_dbg_kms(&i915->drm, "pipe %c data rate %u num active planes %u\n",
> > -		    pipe_name(crtc->pipe),
> > +	drm_dbg_kms(&i915->drm, "[CRTC:%d:%s] data rate %u num active planes %u min cdclk %d kHz\n",
> > +		    crtc->base.base.id, crtc->base.name,
> >  		    bw_state->data_rate[crtc->pipe],
> > -		    bw_state->num_active_planes[crtc->pipe]);
> > +		    bw_state->num_active_planes[crtc->pipe],
> > +		    bw_state->min_cdclk[crtc->pipe]);
> >  }
> >  
> >  static unsigned int intel_bw_num_active_planes(struct drm_i915_private *dev_priv,
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index 90bd26431e31..b17b9493c68f 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -2498,6 +2498,7 @@ static void intel_crtc_disable_noatomic(struct intel_crtc *crtc,
> >  
> >  	bw_state->data_rate[pipe] = 0;
> >  	bw_state->num_active_planes[pipe] = 0;
> > +	bw_state->min_cdclk[pipe] = 0;
> >  }
> >  
> >  /*
> > @@ -9310,6 +9311,8 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
> >  		to_intel_cdclk_state(dev_priv->cdclk.obj.state);
> >  	struct intel_dbuf_state *dbuf_state =
> >  		to_intel_dbuf_state(dev_priv->dbuf.obj.state);
> > +	struct intel_bw_state *bw_state =
> > +		to_intel_bw_state(dev_priv->bw_obj.state);
> >  	enum pipe pipe;
> >  	struct intel_crtc *crtc;
> >  	struct intel_encoder *encoder;
> > @@ -9425,8 +9428,6 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
> >  	drm_connector_list_iter_end(&conn_iter);
> >  
> >  	for_each_intel_crtc(dev, crtc) {
> > -		struct intel_bw_state *bw_state =
> > -			to_intel_bw_state(dev_priv->bw_obj.state);
> >  		struct intel_crtc_state *crtc_state =
> >  			to_intel_crtc_state(crtc->base.state);
> >  		struct intel_plane *plane;
> > @@ -9490,6 +9491,8 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
> >  
> >  		intel_bw_crtc_update(bw_state, crtc_state);
> >  	}
> > +
> > +	cdclk_state->bw_min_cdclk = intel_bw_min_cdclk(dev_priv, bw_state);
> >  }
> >  
> >  static void
> > -- 
> > 2.35.1
> > 

-- 
Ville Syrjälä
Intel