[PATCH 10/12] drm/i915/vrr: Use static guardband to support seamless LRR switching
Golani, Mitulkumar Ajitkumar
mitulkumar.ajitkumar.golani at intel.com
Tue Aug 19 09:23:52 UTC 2025
> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces at lists.freedesktop.org> On Behalf Of Ankit
> Nautiyal
> Sent: 18 August 2025 13:01
> To: intel-gfx at lists.freedesktop.org; intel-xe at lists.freedesktop.org
> Cc: ville.syrjala at linux.intel.com; Nautiyal, Ankit K <ankit.k.nautiyal at intel.com>
> Subject: [PATCH 10/12] drm/i915/vrr: Use static guardband to support seamless
> LRR switching
>
> 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.
>
> v2:
> -Use helpers for dsc/scaler prefill latencies. (Mitul) -Account for pkgc latency and
> take max of pkgc and sagv latencies.
> v3: Use new helper for PSR2/Panel Replay latency.
>
> Bspec: 70151
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal at intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 2 +-
> drivers/gpu/drm/i915/display/intel_vrr.c | 132 ++++++++++++++++++-
> drivers/gpu/drm/i915/display/intel_vrr.h | 3 +-
> 3 files changed, 133 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 9138cd1d6284..17e674c06b18 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -4903,7 +4903,6 @@ intel_modeset_pipe_config_late(struct
> intel_atomic_state *state,
> struct drm_connector *connector;
> int i;
>
> - intel_vrr_compute_config_late(crtc_state);
>
> for_each_new_connector_in_state(&state->base, connector,
> conn_state, i) {
> @@ -4915,6 +4914,7 @@ intel_modeset_pipe_config_late(struct
> intel_atomic_state *state,
> !encoder->compute_config_late)
> continue;
>
> + intel_vrr_compute_config_late(crtc_state, conn_state);
> ret = encoder->compute_config_late(encoder, crtc_state,
> conn_state);
> if (ret)
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c
> b/drivers/gpu/drm/i915/display/intel_vrr.c
> index 46a85720411f..170f7bcdb8a8 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -6,12 +6,15 @@
>
> #include <drm/drm_print.h>
>
> +#include "intel_alpm.h"
> #include "intel_de.h"
> #include "intel_display_regs.h"
> #include "intel_display_types.h"
> #include "intel_dp.h"
> #include "intel_vrr.h"
> #include "intel_vrr_regs.h"
> +#include "skl_scaler.h"
> +#include "skl_watermark.h"
>
> #define FIXED_POINT_PRECISION 100
> #define CMRR_PRECISION_TOLERANCE 10
> @@ -413,15 +416,140 @@ intel_vrr_compute_config(struct intel_crtc_state
> *crtc_state,
> }
> }
>
> -void intel_vrr_compute_config_late(struct intel_crtc_state *crtc_state)
> +static
> +int scaler_prefill_latency(struct intel_crtc_state *crtc_state, int
> +linetime_us) {
Can we differentiate in terms of naming, as we are calculating scaler prefill latency for 2 different purpose, same name can confuse later.
> + int chroma_downscaling_factor =
> skl_scaler_chroma_downscale_factor(crtc_state);
> + u64 hscale_k, vscale_k;
> + int cdclk_adjustment;
> + int num_scaler_users;
> +
> + /*
> + * Assuming:
> + * Both scaler enabled.
> + * scaler 1 downscaling factor as 2 x 2 (Horiz x Vert)
> + * scaler 2 downscaling factor as 2 x 1 (Horiz x Vert)
> + * Cdclk Adjustment : 1
> + */
> + num_scaler_users = 2;
> + hscale_k = 2 * 1000;
> + vscale_k = 2 * 1000;
> + cdclk_adjustment = 1;
> +
> + return intel_display_scaler_prefill_latency(num_scaler_users, hscale_k,
> vscale_k,
> + chroma_downscaling_factor,
> + cdclk_adjustment,
> + linetime_us);
> +}
> +
> +static
> +int dsc_prefill_latency(struct intel_crtc_state *crtc_state, int
> +linetime_us) { #define MAX_SCALERS 2
> + int chroma_downscaling_factor =
> skl_scaler_chroma_downscale_factor(crtc_state);
> + u64 hscale_k[MAX_SCALERS], vscale_k[MAX_SCALERS];
> + int cdclk_adjustment;
> + int num_scaler_users;
> +
> + /*
> + * Assuming:
> + * Both scaler enabled.
> + * scaler 1 downscaling factor as 2 x 2 (Horiz x Vert)
> + * scaler 2 downscaling factor as 2 x 1 (Horiz x Vert)
> + * Cdclk Adjustment : 1
> + */
> + num_scaler_users = MAX_SCALERS;
> + hscale_k[0] = 2 * 1000;
> + vscale_k[0] = 2 * 1000;
> + hscale_k[1] = 2 * 1000;
> + vscale_k[1] = 1 * 1000;
> +
> + cdclk_adjustment = 1;
> +
> + return intel_display_dsc_prefill_latency(num_scaler_users, hscale_k,
> vscale_k,
> + chroma_downscaling_factor,
> + cdclk_adjustment,
> + linetime_us);
> +}
> +
> +static
> +int intel_vrr_compute_guardband(struct intel_crtc_state *crtc_state,
> + struct intel_connector *connector)
> +{
> + const struct drm_display_mode *adjusted_mode = &crtc_state-
> >hw.adjusted_mode;
> + struct intel_display *display = to_intel_display(crtc_state);
> + int dsc_prefill_time = 0;
> + int psr2_pr_latency = 0;
> + int scaler_prefill_time;
> + int wm0_prefill_time;
> + int pkgc_max_latency;
> + int sagv_latency;
> + int sdp_latency = 0;
> + int guardband_us;
> + int linetime_us;
> + int guardband;
> + int pm_delay;
> +
> + linetime_us = DIV_ROUND_UP(adjusted_mode->crtc_htotal * 1000,
> + adjusted_mode->crtc_clock);
> +
> + pkgc_max_latency = skl_watermark_max_latency(display, 1);
> + sagv_latency = display->sagv.block_time_us;
> +
> + /* Assuming max wm0 lines = 4 */
> + wm0_prefill_time = 4 * linetime_us + 20;
> +
> + scaler_prefill_time = scaler_prefill_latency(crtc_state, linetime_us);
> +
> + if (crtc_state->dsc.compression_enable)
> + dsc_prefill_time = dsc_prefill_latency(crtc_state, linetime_us);
> +
> + pm_delay = crtc_state->framestart_delay +
> + max(sagv_latency, pkgc_max_latency) +
> + wm0_prefill_time +
> + scaler_prefill_time +
> + dsc_prefill_time;
> +
> + switch (connector->base.connector_type) {
> + case DRM_MODE_CONNECTOR_eDP:
> + case DRM_MODE_CONNECTOR_DisplayPort:
> + psr2_pr_latency =
> intel_alpm_compute_max_link_wake_latency(crtc_state, true);
> + sdp_latency = intel_dp_compute_sdp_latency(crtc_state, true);
> + break;
> + default:
> + break;
> + }
> +
> + guardband_us = max(sdp_latency, psr2_pr_latency);
> + guardband_us = max(guardband_us, pm_delay);
> +
> + guardband = DIV_ROUND_UP(guardband_us, linetime_us);
> +
> + /* guardband cannot be more than the Vmax vblank */
> + guardband = min(guardband, crtc_state->vrr.vmax -
> +adjusted_mode->crtc_vblank_start);
> +
> + return guardband;
> +}
> +
> +void intel_vrr_compute_config_late(struct intel_crtc_state *crtc_state,
> + struct drm_connector_state *conn_state)
> {
> struct intel_display *display = to_intel_display(crtc_state);
> const struct drm_display_mode *adjusted_mode = &crtc_state-
> >hw.adjusted_mode;
> + struct intel_connector *connector =
> + to_intel_connector(conn_state->connector);
>
> if (!intel_vrr_possible(crtc_state))
> return;
>
> - if (DISPLAY_VER(display) >= 13) {
> + if (intel_vrr_always_use_vrr_tg(display)) {
> + crtc_state->vrr.guardband =
> intel_vrr_compute_guardband(crtc_state, connector);
> + if (crtc_state->uapi.vrr_enabled) {
> + crtc_state->vrr.vmin = crtc_state->vrr.guardband +
> + adjusted_mode->crtc_vblank_start;
> + crtc_state->vrr.flipline = crtc_state->vrr.vmin;
> + }
> + } else if (DISPLAY_VER(display) >= 13) {
> crtc_state->vrr.guardband =
> crtc_state->vrr.vmin - adjusted_mode-
> >crtc_vblank_start;
> } else {
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h
> b/drivers/gpu/drm/i915/display/intel_vrr.h
> index 38bf9996b883..4b15c2838492 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.h
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.h
> @@ -21,7 +21,8 @@ bool intel_vrr_possible(const struct intel_crtc_state
> *crtc_state); void intel_vrr_check_modeset(struct intel_atomic_state *state);
> void intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
> struct drm_connector_state *conn_state); -void
> intel_vrr_compute_config_late(struct intel_crtc_state *crtc_state);
> +void intel_vrr_compute_config_late(struct intel_crtc_state *crtc_state,
> + struct drm_connector_state *conn_state);
As we construct guardband with worst case latency, and compare with vblank length.
changes look good. Only naming can be checked before merge for respective purpose.
Reviewed-by: Mitul Golani <mitulkumar.ajitkumar.golani at intel.com>
> void intel_vrr_set_transcoder_timings(const struct intel_crtc_state *crtc_state);
> void intel_vrr_enable(const struct intel_crtc_state *crtc_state); void
> intel_vrr_send_push(struct intel_dsb *dsb,
> --
> 2.45.2
More information about the Intel-xe
mailing list