On Fri, 2025-10-17 at 13:15 +0300, Hogander, Jouni wrote: > On Fri, 2025-10-17 at 12:58 +0300, Hogander, Jouni wrote: > > On Fri, 2025-10-17 at 15:07 +0530, Nautiyal, Ankit K wrote: > > > > > > On 10/17/2025 2:37 PM, Hogander, Jouni wrote: > > > > On Fri, 2025-10-17 at 10:31 +0530, Ankit Nautiyal wrote: > > > > > Introduce a helper to compute the max link wake latency when > > > > > using > > > > > Auxless/Aux wake mechanism for PSR/Panel Replay/LOBF > > > > > features. > > > > > > > > > > This will be used to compute the minimum guardband so that > > > > > the > > > > > link > > > > > wake > > > > > latencies are accounted and these features work smoothly for > > > > > higher > > > > > refresh rate panels. > > > > > > > > > > Bspec: 70151, 71477 > > > > > Signed-off-by: Ankit Nautiyal <[email protected]> > > > > > --- > > > > > drivers/gpu/drm/i915/display/intel_psr.c | 12 ++++++++++++ > > > > > drivers/gpu/drm/i915/display/intel_psr.h | 1 + > > > > > 2 files changed, 13 insertions(+) > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c > > > > > b/drivers/gpu/drm/i915/display/intel_psr.c > > > > > index 703e5f6af04c..a8303b669853 100644 > > > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c > > > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > > > > > @@ -4416,3 +4416,15 @@ void > > > > > intel_psr_compute_config_late(struct > > > > > intel_dp *intel_dp, > > > > > > > > > > intel_psr_set_non_psr_pipes(intel_dp, crtc_state); > > > > > } > > > > > + > > > > > +int intel_psr_min_guardband(struct intel_crtc_state > > > > > *crtc_state) > > > > > +{ > > > > > + struct intel_display *display = > > > > > to_intel_display(crtc_state); > > > > > + int auxless_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; > > > > > + > > > > > + return max(auxless_wake_lines, wake_lines); > > > > hmm, now if you add: > > > > > > > > if (crtc_state->req_psr2_sdp_prior_scanline) > > > > psr_min_guardband++; > > > > > > I did not get this part. Do we need to account for 1 more > > > wakelines > > > if > > > this flag is set? > > > > If you look at how that flag affects vblank check in > > intel_psr_compute_config_late: > > > > ... > > 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; > > ... > > > > So to take that into account when calculating minimum guardband > > needed > > by PSR you need to increase by one. Same goes with SCL: > > > > ... > > int scl = _intel_psr_min_set_context_latency(crtc_state, > > > > needs_panel_replay, > > > > needs_sel_update); > > vblank -= scl; > > ... > > > > You are already partially taking into account PSR needs when > > calculating optimized guardband except these two lines which are > > needed > > conditionally. > > > > Also intel_psr_compute_config is run at this point -> you know > > which > > one to use: auxless wake time or aux wake time. no need to use > > max() > > with them. Just choose the one which is relevant. > > > > With these changes you could drop intel_psr_compute_config_late as > > vblank would be long enough for PSR mode computed by > > intel_psr_compute_config, no? > > Ok, noticed now this in the last patch: > > ... > crtc_state->vrr.guardband = min(guardband, > intel_vrr_max_guardband(crtc_state)); > ... > > So if we need to fall back to intel_vrr_max_guardband we need to have > that intel_psr_compute_config_late. > > Anyways I think you need to take into account that > req_psr2_sdp_prior_scanline and _intel_psr_min_set_context_latency in > intel_psr_min_guardband.
Also you can use auxless wake time for Panel Replay with ALPM and aux wake time for PSR2 because only following changes are possible in intel_psr_compute_config_late: PSR2 (aux wake time) -> PSR1 -> guardband length doesn't matter Panel Replay (auxless wake time) -> disabled -> guardband length doesn't matter BR, Jouni Högander > > BR, > > Jouni Högander > > > > BR, > > > > Jouni Högander > > > > > > > > > > What we want to do is to check for min guardband for > > > panel_replay/sel_update based on the required wakelines. > > > > > > Whether we can use the auxless_wake_lines and wake_lines as > > > computed > > > above to estimate the max wakelines or instead we should use > > > functions > > > from alpm.c : > > > > > > io_buffer_wake_time() and _lnl_compute_aux_less_wake_time() to > > > get > > > the > > > worst case wakelines. > > > > > > > > > > > > > > Whatever is the PSR mode it can be enabled what comes to vblank > > > > restrictions and you can drop psr_compute_config_late? > > > > > > > > > I think we cannot drop the psr_compute_config_late as it checks > > > whether > > > the actual guardband is enough for PSR features. > > > > > > intel_psr_min_guardband() is used along with other features to > > > have > > > an estimate on the guardband that works for all cases, without a > > > need > > > to change the guardband. > > > It is bounded by the vblank length available > > > > > > Regards, > > > > > > Ankit > > > > > > > > > > > BR, > > > > > > > > Jouni Högander > > > > > > > > > +} > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.h > > > > > b/drivers/gpu/drm/i915/display/intel_psr.h > > > > > index b17ce312dc37..620b35928832 100644 > > > > > --- a/drivers/gpu/drm/i915/display/intel_psr.h > > > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.h > > > > > @@ -85,5 +85,6 @@ 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); > > > > > +int intel_psr_min_guardband(struct intel_crtc_state > > > > > *crtc_state); > > > > > > > > > > #endif /* __INTEL_PSR_H__ */ > > >
