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 <[email protected]>
> > Cc: Animesh Manna <[email protected]>
> > Cc: Jouni Högander <[email protected]>
> > ---
> > 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))
> > {
>
