[Intel-gfx] [PATCH] drm/i915/adl_p: Increase CDCLK by 15% if PSR2 is used

[Lisovskiy, Stanislav]

On Mon, Mar 21, 2022 at 07:01:27PM +0200, Souza, Jose wrote:
> On Mon, 2022-03-21 at 12:49 +0200, Stanislav Lisovskiy wrote:
> > We are currently getting FIFO underruns, in particular
> > when PSR2 is enabled. There seem to be no existing workaround
> > or patches, which can fix that issue(were expecting some recent
> > selective fetch update and DBuf bw/SAGV fixes to help,
> > which unfortunately didn't).
> > Current idea is that it looks like for some reason the
> > DBuf prefill time isn't enough once we exit PSR2, despite its
> > theoretically correct.
> > So bump it up a bit by 15%(minimum experimental amount required
> > to get it working), if PSR2 is enabled.
> > For PSR1 there is no need in this hack, so we limit it only
> > to PSR2 and Alderlake.
> > 
> > v2: - Added comment(Jose Souza)
> >     - Fixed 15% calculation(Jose Souza)
> > 
> > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy at intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_cdclk.c | 26 ++++++++++++++++++++++
> >  1 file changed, 26 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > index 8888fda8b701..92d57869983a 100644
> > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > @@ -2325,6 +2325,32 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
> >  					dev_priv->max_cdclk_freq));
> >  	}
> >  
> > +
> > +	/*
> > +	 * HACK.  We are getting FIFO underruns, in particular
> > +	 * when PSR2 is enabled. There seem to be no existing workaround
> > +	 * or patches as of now.
> > +	 * Current idea is that it looks like for some reason the
> > +	 * DBuf prefill time isn't enough once we exit PSR2, despite its
> > +	 * theoretically correct.
> > +	 * So bump it up a bit by 15%(minimum experimental amount required
> > +	 * to get it working), if PSR2 is enabled.
> > +	 * For PSR1 there is no need in this hack, so we limit it only
> > +	 * to PSR2 and Alderlake.
> > +	 */
> > +	if (IS_ALDERLAKE_P(dev_priv)) {
> 
> 
> And please check if we can only apply this when two or more pipes are enabled.
> Otherwise this will impact power numbers in the case that is matters most.

That one I can check. Probably need someone at office to disconnect all the pipes, except eDP to see
if its still reproducible, however I think I've seen it already happening.

Stan

> 
> > +		struct intel_encoder *encoder;
> > +
> > +		for_each_intel_encoder_with_psr(&dev_priv->drm, encoder) {
> > +			struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> > +
> > +			if (intel_dp->psr.psr2_enabled) {
> > +				min_cdclk = DIV_ROUND_UP(min_cdclk * 115, 100);
> > +				break;
> > +			}
> > +		}
> > +	}
> > +
> >  	if (min_cdclk > dev_priv->max_cdclk_freq) {
> >  		drm_dbg_kms(&dev_priv->drm,
> >  			    "required cdclk (%d kHz) exceeds max (%d kHz)\n",
>