Re: [PATCH 6/8] drm/i915/display: Check if final vblank is sufficient for PSR features

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

On Tue, 2025-10-14 at 16:57 +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.
> 
> Introduce intel_dp_compute_config_late() to handle late-stage
> configuration checks for DP/eDP features. For now, it validates
> 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.
> 
> Check if vblank is sufficient for PSR features, and disable them 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)
> 
> 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_ddi.c |  3 +
>  drivers/gpu/drm/i915/display/intel_dp.c  |  9 +++
>  drivers/gpu/drm/i915/display/intel_dp.h  |  3 +
>  drivers/gpu/drm/i915/display/intel_psr.c | 84 +++++++++++++++++++++-
> --
>  drivers/gpu/drm/i915/display/intel_psr.h |  2 +
>  5 files changed, 93 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> b/drivers/gpu/drm/i915/display/intel_ddi.c
> index c09aa759f4d4..94c593bbedf4 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -4560,6 +4560,9 @@ static int intel_ddi_compute_config_late(struct
> intel_encoder *encoder,
>  	struct drm_connector *connector = conn_state->connector;
>  	u8 port_sync_transcoders = 0;
>  
> +	if (intel_crtc_has_dp_encoder(crtc_state))
> +		intel_dp_compute_config_late(encoder, crtc_state,
> conn_state);
> +
>  	drm_dbg_kms(display->drm, "[ENCODER:%d:%s] [CRTC:%d:%s]\n",
>  		    encoder->base.base.id, encoder->base.name,
>  		    crtc_state->uapi.crtc->base.id, crtc_state-
> >uapi.crtc->name);
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index a723e846321f..e481ff4c4959 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -6979,3 +6979,12 @@ void intel_dp_mst_resume(struct intel_display
> *display)
>  		}
>  	}
>  }
> +
> +void intel_dp_compute_config_late(struct intel_encoder *encoder,
> +				  struct intel_crtc_state
> *crtc_state,
> +				  struct drm_connector_state
> *conn_state)
> +{
> +	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> +
> +	intel_psr_compute_config_late(intel_dp, crtc_state);
> +}
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h
> b/drivers/gpu/drm/i915/display/intel_dp.h
> index b379443e0211..0d9573ca44cb 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> @@ -218,5 +218,8 @@ int intel_dp_compute_min_hblank(struct
> intel_crtc_state *crtc_state,
>  int intel_dp_dsc_bpp_step_x16(const struct intel_connector
> *connector);
>  void intel_dp_dpcd_set_probe(struct intel_dp *intel_dp, bool
> force_on_external);
>  bool intel_dp_in_hdr_mode(const struct drm_connector_state
> *conn_state);
> +void intel_dp_compute_config_late(struct intel_encoder *encoder,
> +				  struct intel_crtc_state
> *crtc_state,
> +				  struct drm_connector_state
> *conn_state);
>  
>  #endif /* __INTEL_DP_H__ */
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index 28efa4410c2a..bb76fb52ac97 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,
> @@ -4346,3 +4362,55 @@ bool intel_psr_needs_alpm_aux_less(struct
> intel_dp *intel_dp,
>  {
>  	return intel_dp_is_edp(intel_dp) && crtc_state-
> >has_panel_replay;
>  }
> +
> +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))
> +		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;

Sorry for late notice :

You still need to check at least intel_psr_needs_wa_18037818876.
Instead of just adding it into here you I recommend moving everything
after "/* Wa_18037818876 */" (and including it) from
intel_psr_compute_config into psr_compute_config_late. If you are doing
this I think you should split this patch as two separate patches. First
one adding psr_compute_config_late and moving the code from
psr_compute_config. Second one adding this vblank check into
psr_compute_config_late.

BR,

Jouni Högander

> +	}
> +
> +	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;
> +	}
> +}
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.h
> b/drivers/gpu/drm/i915/display/intel_psr.h
> index 9147996d6c9e..b17ce312dc37 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.h
> +++ b/drivers/gpu/drm/i915/display/intel_psr.h
> @@ -83,5 +83,7 @@ void intel_psr_debugfs_register(struct
> intel_display *display);
>  bool intel_psr_needs_alpm(struct intel_dp *intel_dp, const struct
> intel_crtc_state *crtc_state);
>  bool intel_psr_needs_alpm_aux_less(struct intel_dp *intel_dp,
>  				   const struct intel_crtc_state
> *crtc_state);
> +void intel_psr_compute_config_late(struct intel_dp *intel_dp,
> +				   struct intel_crtc_state
> *crtc_state);
>  
>  #endif /* __INTEL_PSR_H__ */