On Fri, 2025-10-17 at 16:41 +0530, Nautiyal, Ankit K wrote: > > On 10/17/2025 3:45 PM, 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. > > > Hmm I think you are right, we need to account for > req_psr2_sdp_prior_scanline here. > > But for SCL I still think we do not need to account in wakelines as > we > are already accounting in intel_vrr_max_guardband() which calls : > > intel_vrr_max_vblank_guardband(const struct intel_crtc_state > *crtc_state) { struct intel_display *display = > to_intel_display(crtc_state); const struct drm_display_mode > *adjusted_mode = &crtc_state->hw.adjusted_mode; return > crtc_state->vrr.vmin - adjusted_mode->crtc_vdisplay - > crtc_state->set_context_latency - > intel_vrr_extra_vblank_delay(display); }
intel_vrr_max_guardband is used only if it's smaller than what is computed by intel_vrr_compute_optimized_guardband. I.e. case where intel_psr_min_guard rules used guardband it is not taken into account unless you add it into intel_psr_min_guard. BR, Jouni Högander > > Regards, > > Ankit > > > > > 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__ */
