On Fri, Oct 17, 2025 at 10:32:01AM +0530, Ankit Nautiyal wrote: > In the current VRR implementation, vrr.vmin and vrr.guardband are set such > that they do not need to change when switching from fixed refresh rate to > variable refresh rate. Specifically, vrr.guardband is always set to match > the vblank length. This approach works for most cases, but not for LRR, > where the guardband would need to change while the VRR timing generator is > still active. > > With the VRR TG always active, live updates to guardband are unsafe and not > recommended. To ensure hardware safety, guardband was moved out of the > !fastset block, meaning any change now requires a full modeset. > This breaks seamless LRR switching, which was previously supported. > > Since the problem arises from guardband being matched to the vblank length, > solution is to use a minimal, sufficient static value, instead. So we use a > static guardband defined during mode-set that fits within the smallest > expected vblank and remains unchanged in case of features like LRR where > vtotal changes. To compute this minimum guardband we take into account > latencies/delays due to different features as mentioned in the Bspec. > > Introduce a helper to compute the minimal sufficient guardband. > On platforms where the VRR timing generator is always ON, we optimize the > guardband regardless of whether the display is operating in fixed or > variable refresh rate mode. > > v2: > - Use max of sagv latency and skl_wm_latency(1) for PM delay > computation. (Ville) > - Avoid guardband optimization for HDMI for now. (Ville) > - Allow guardband optimization only for platforms with > intel_vrr_always_use_vrr_tg = true. (Ville) > - Add comments for PM delay and a #TODO note for HDMI. > > Bspec: 70151 > Signed-off-by: Ankit Nautiyal <[email protected]> > --- > drivers/gpu/drm/i915/display/intel_vrr.c | 62 +++++++++++++++++++++++- > 1 file changed, 60 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c > b/drivers/gpu/drm/i915/display/intel_vrr.c > index 597008a6c744..cd7bed358984 100644 > --- a/drivers/gpu/drm/i915/display/intel_vrr.c > +++ b/drivers/gpu/drm/i915/display/intel_vrr.c > @@ -6,12 +6,16 @@ > > #include <drm/drm_print.h> > > +#include "intel_crtc.h" > #include "intel_de.h" > #include "intel_display_regs.h" > #include "intel_display_types.h" > #include "intel_dp.h" > +#include "intel_psr.h" > #include "intel_vrr.h" > #include "intel_vrr_regs.h" > +#include "skl_prefill.h" > +#include "skl_watermark.h" > > #define FIXED_POINT_PRECISION 100 > #define CMRR_PRECISION_TOLERANCE 10 > @@ -433,17 +437,71 @@ intel_vrr_max_guardband(struct intel_crtc_state > *crtc_state) > intel_vrr_max_vblank_guardband(crtc_state)); > } > > +static > +int intel_vrr_compute_optimized_guardband(struct intel_crtc_state > *crtc_state) > +{ > + struct intel_display *display = to_intel_display(crtc_state); > + struct skl_prefill_ctx prefill_ctx; > + int prefill_min_guardband; > + int prefill_latency_us; > + int guardband = 0; > + > + skl_prefill_init_worst(&prefill_ctx, crtc_state); > + > + /* > + * The SoC power controller runs SAGV mutually exclusive with package C > states, > + * so the max of package C and SAGV latencies is used to compute the > min prefill guardband. > + * PM delay = max(sagv_latency, pkgc_max_latency (highest enabled wm > level 1 and up)) > + */ > + prefill_latency_us = max(display->sagv.block_time_us, > + skl_watermark_max_latency(display, 1)); > + prefill_min_guardband = > + skl_prefill_min_guardband(&prefill_ctx, > + crtc_state, > + prefill_latency_us);
This could be just guardband = skl_prefill_min_guardband(...) and you can then get rid of the prefill_min_guardband variable as well. But in general lgtm Reviewed-by: Ville Syrjälä <[email protected]> > + > + if (intel_crtc_has_dp_encoder(crtc_state)) { > + guardband = max(guardband, intel_psr_min_guardband(crtc_state)); > + guardband = max(guardband, > intel_dp_sdp_min_guardband(crtc_state, true)); > + } > + > + guardband = max(guardband, prefill_min_guardband); > + > + return guardband; > +} > + > +static bool intel_vrr_use_optimized_guardband(const struct intel_crtc_state > *crtc_state) > +{ > + struct intel_display *display = to_intel_display(crtc_state); > + > + /* > + * #TODO: Enable optimized guardband for HDMI > + * For HDMI lot of infoframes are transmitted a line or two after vsync. > + * Since with optimized guardband the double bufferring point is at > delayed vblank, > + * we need to ensure that vsync happens after delayed vblank for the > HDMI case. > + */ > + if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI)) > + return false; > + > + return intel_vrr_always_use_vrr_tg(display); > +} > + > void intel_vrr_compute_guardband(struct intel_crtc_state *crtc_state) > { > struct intel_display *display = to_intel_display(crtc_state); > struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode; > struct drm_display_mode *pipe_mode = &crtc_state->hw.pipe_mode; > + int guardband; > > if (!intel_vrr_possible(crtc_state)) > return; > > - crtc_state->vrr.guardband = min(crtc_state->vrr.vmin - > adjusted_mode->crtc_vdisplay, > - intel_vrr_max_guardband(crtc_state)); > + if (intel_vrr_use_optimized_guardband(crtc_state)) > + guardband = intel_vrr_compute_optimized_guardband(crtc_state); > + else > + guardband = crtc_state->vrr.vmin - adjusted_mode->crtc_vdisplay; > + > + crtc_state->vrr.guardband = min(guardband, > intel_vrr_max_guardband(crtc_state)); > > if (intel_vrr_always_use_vrr_tg(display)) { > adjusted_mode->crtc_vblank_start = > -- > 2.45.2 -- Ville Syrjälä Intel
