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

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

On Thu, 2025-10-09 at 14:30 +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.

Now as we are adding this: Can't we just drop checks made earlier and
rely on psr_compute_config_late checking the vblank?

BR,

Jouni Högander

> 
> 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 | 51 ++++++++++++++++++++--
> --
>  drivers/gpu/drm/i915/display/intel_psr.h |  2 +
>  5 files changed, 60 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 212bd244beed..dcab4127b399 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,7 @@ 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;
> +	return _wake_lines_fit_into_vblank(crtc_state, vblank,
> wake_lines);
>  }
>  
>  static bool alpm_config_valid(struct intel_dp *intel_dp,
> @@ -4346,3 +4353,31 @@ 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 = crtc_state-
> >alpm_state.aux_less_wake_lines;
> +	int 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;
> +
> +	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
> insufficient for wakelines = %d\n",
> +			    aux_less_wake_lines);
> +		crtc_state->has_panel_replay = false;
> +	}
> +
> +	if (crtc_state->has_sel_update &&
> +	    !_wake_lines_fit_into_vblank(crtc_state, vblank,
> wake_lines)) {
> +		drm_dbg_kms(display->drm,
> +			    "Disabling Selective Update: vblank
> insufficient for wakelines = %d\n",
> +			    wake_lines);
> +		crtc_state->has_sel_update = 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__ */