Re: [PATCH 08/10] drm/i915/psr: Check if final vblank is sufficient for PSR features

["Hogander, Jouni" <jouni.hogander@intel.com>]

On Wed, 2025-10-15 at 11:23 +0300, Hogander, Jouni wrote:
> On Wed, 2025-10-15 at 12:52 +0530, Ankit Nautiyal wrote:
> > Currently, wake line latency checks rely on the vblank length,
> > which does not account for either the extra vblank delay for
> > icl/tgl
> > or for
> > the optimized guardband which will come into picture later at some
> > point.
> > 
> > Validate whether the final vblank (with extra vblank delay) or
> > guardband
> > is sufficient to support wake line latencies required by Panel
> > Replay
> > and
> > PSR2 selective update. Disable the PSR features if their wake
> > requirements
> > cannot be accomodated.
> > 
> > v2: Add comments clarifying wake line checks and rationale for not
> >     resetting SCL. (Jouni)
> > v3: Reset other psr flags based on features that are dropped.
> > (Jouni)
> > v4: Update commit message.
> > 
> > Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> > Cc: Animesh Manna <animesh.manna@intel.com>
> > Cc: Jouni Högander <jouni.hogander@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_psr.c | 78
> > +++++++++++++++++++++-
> > --
> >  1 file changed, 70 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > b/drivers/gpu/drm/i915/display/intel_psr.c
> > index 383e6dc1ed63..816ad67e33cd 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -1405,6 +1405,20 @@ int _intel_psr_min_set_context_latency(const
> > struct intel_crtc_state *crtc_state
> >  		return 1;
> >  }
> >  
> > +static bool _wake_lines_fit_into_vblank(const struct
> > intel_crtc_state *crtc_state,
> > +					int vblank,
> > +					int wake_lines)
> > +{
> > +	if (crtc_state->req_psr2_sdp_prior_scanline)
> > +		vblank -= 1;
> > +
> > +	/* Vblank >= PSR2_CTL Block Count Number maximum line
> > count
> > */
> > +	if (vblank < wake_lines)
> > +		return false;
> > +
> > +	return true;
> > +}
> > +
> >  static bool wake_lines_fit_into_vblank(struct intel_dp *intel_dp,
> >  				       const struct
> > intel_crtc_state
> > *crtc_state,
> >  				       bool aux_less,
> > @@ -1428,14 +1442,16 @@ static bool
> > wake_lines_fit_into_vblank(struct
> > intel_dp *intel_dp,
> >  					       crtc_state-
> > > alpm_state.fast_wake_lines) :
> >  			crtc_state->alpm_state.io_wake_lines;
> >  
> > -	if (crtc_state->req_psr2_sdp_prior_scanline)
> > -		vblank -= 1;
> > -
> > -	/* Vblank >= PSR2_CTL Block Count Number maximum line
> > count
> > */
> > -	if (vblank < wake_lines)
> > -		return false;
> > -
> > -	return true;
> > +	/*
> > +	 * Guardband has not been computed yet, so we
> > conservatively
> > check if the
> > +	 * full vblank duration is sufficient to accommodate wake
> > line requirements
> > +	 * for PSR features like Panel Replay and Selective
> > Update.
> > +	 *
> > +	 * Once the actual guardband is available, a more accurate
> > validation is
> > +	 * performed in intel_psr_compute_config_late(), and PSR
> > features are
> > +	 * disabled if wake lines exceed the available guardband.
> > +	 */
> > +	return _wake_lines_fit_into_vblank(crtc_state, vblank,
> > wake_lines);
> >  }
> >  
> >  static bool alpm_config_valid(struct intel_dp *intel_dp,
> > @@ -4351,6 +4367,52 @@ void intel_psr_compute_config_late(struct
> > intel_dp *intel_dp,
> >  				   struct intel_crtc_state
> > *crtc_state)
> >  {
> >  	struct intel_display *display =
> > to_intel_display(intel_dp);
> > +	int vblank = intel_crtc_vblank_length(crtc_state);
> > +	int aux_less_wake_lines;
> > +	int wake_lines;
> > +
> > +	if (!intel_psr_needs_alpm(intel_dp, crtc_state))
> 
> Now as psr_compute_config_late is not only about vblank you can't
> have
> this early return. You could have (or do it as you see is best):
> 
> 	if (intel_psr_needs_alpm_aux_less(intel_dp, crtc_state))
> 		wake_lines = DISPLAY_VER(display) < 20 ?
> 			psr2_block_count_lines(crtc_state-
> >alpm_state.io_wake_lines,
> 					       crtc_state-
> >alpm_state.fast_wake_lines) :
> 			crtc_state->alpm_state.io_wake_lines;
> 	else if (intel_psr_needs_alpm_aux_less(intel_dp,
> crtc_state))
> 		wake_lines = crtc_state-
> >alpm_state.aux_less_wake_lines;
> 	else
> 		wake_lines = 0;
> 
> 	if (!wake_lines || !_wake_lines_fit_into_vblank(crtc_state,
> vblank, wake_lines)) {

This needs to be:

if (wake_lines && !_wake_lines_fit_into_vblank(crtc_state, vblank, wake_lines)) {

BR,

Jouni Högander

> 		drm_dbg_kms(display->drm,
> 			    "Adjusting PSR/PR mode: vblank too short
> for wake lines = %d\n",
> 			    wake_lines);
> 
> 		if (crtc_state->has_panel_replay) {
> 			crtc_state->has_panel_replay = false;
> 			/*
> 			 * #TODO : Add fall back to PSR/PSR2
> 			 * Since panel replay cannot be supported,
> we can fall back to PSR/PSR2.
> 			 * This will require calling compute_config
> for psr and psr2 with check for
> 			 * actual guardband instead of
> vblank_length.
> 			 */
> 			crtc_state->has_psr = false;
> 		}
> 		crtc_state->has_sel_update = false;
> 		crtc_state->enable_psr2_su_region_et = false;
> 		crtc_state->enable_psr2_sel_fetch = false;
> 	}
> 
> BR,
> 
> Jouni Högander
> 
> > +		return;
> > +
> > +	aux_less_wake_lines = crtc_state-
> > > alpm_state.aux_less_wake_lines;
> > +	wake_lines = DISPLAY_VER(display) < 20 ?
> > +		     psr2_block_count_lines(crtc_state-
> > > alpm_state.io_wake_lines,
> > +					    crtc_state-
> > > alpm_state.fast_wake_lines) :
> > +		     crtc_state->alpm_state.io_wake_lines;
> > +
> > +	/*
> > +	 * Disable the PSR features if wake lines exceed the
> > available vblank.
> > +	 * Though SCL is computed based on these PSR features, it
> > is
> > not reset
> > +	 * even if the PSR features are disabled to avoid changing
> > vblank start
> > +	 * at this stage.
> > +	 */
> > +	if (!crtc_state->has_panel_replay && crtc_state-
> > > has_sel_update &&
> > +	    !_wake_lines_fit_into_vblank(crtc_state, vblank,
> > wake_lines)) {
> > +		drm_dbg_kms(display->drm,
> > +			    "Disabling Selective Update: vblank
> > too
> > short for wake lines = %d\n",
> > +			    wake_lines);
> > +		crtc_state->has_sel_update = false;
> > +		crtc_state->enable_psr2_su_region_et = false;
> > +		crtc_state->enable_psr2_sel_fetch = false;
> > +	}
> > +
> > +	if (intel_psr_needs_alpm_aux_less(intel_dp, crtc_state) &&
> > +	    !_wake_lines_fit_into_vblank(crtc_state, vblank,
> > aux_less_wake_lines)) {
> > +		drm_dbg_kms(display->drm,
> > +			    "Disabling Panel replay: vblank too
> > short for wake lines = %d\n",
> > +			    aux_less_wake_lines);
> > +		crtc_state->has_panel_replay = false;
> > +		/*
> > +		 * #TODO : Add fall back to PSR/PSR2
> > +		 * Since panel replay cannot be supported, we can
> > fall back to PSR/PSR2.
> > +		 * This will require calling compute_config for
> > psr
> > and psr2 with check for
> > +		 * actual guardband instead of vblank_length.
> > +		 */
> > +		crtc_state->has_psr = false;
> > +		crtc_state->has_sel_update = false;
> > +		crtc_state->enable_psr2_su_region_et = false;
> > +		crtc_state->enable_psr2_sel_fetch = false;
> > +	}
> >  
> >  	/* Wa_18037818876 */
> >  	if (intel_psr_needs_wa_18037818876(intel_dp, crtc_state))
> > {
>