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. 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__ */ >
