[PATCH 2/4] drm/i915/vrr: Use static guardband to support seamless LRR switching
Nautiyal, Ankit K
ankit.k.nautiyal at intel.com
Mon Aug 4 13:53:22 UTC 2025
On 7/16/2025 2:52 PM, Golani, Mitulkumar Ajitkumar wrote:
>
>> -----Original Message-----
>> From: Intel-gfx <intel-gfx-bounces at lists.freedesktop.org> On Behalf Of Ankit
>> Nautiyal
>> Sent: 07 July 2025 11:33
>> 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 2/4] 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.
>>
>> 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_psr.c | 41 ++++++++++
>> drivers/gpu/drm/i915/display/intel_psr.h | 2 +
>> drivers/gpu/drm/i915/display/intel_vrr.c | 79 +++++++++++++++++++-
>> drivers/gpu/drm/i915/display/intel_vrr.h | 3 +-
>> 5 files changed, 123 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
>> b/drivers/gpu/drm/i915/display/intel_display.c
>> index 456fc4b04cda..c09f0a7f1fc1 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>> @@ -4754,7 +4754,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) {
>> @@ -4766,6 +4765,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_psr.c
>> b/drivers/gpu/drm/i915/display/intel_psr.c
>> index ae9053919211..e44c95093dc2 100644
>> --- a/drivers/gpu/drm/i915/display/intel_psr.c
>> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
>> @@ -33,6 +33,7 @@
>> #include "intel_atomic.h"
>> #include "intel_crtc.h"
>> #include "intel_cursor_regs.h"
>> +#include "intel_cx0_phy.h"
>> #include "intel_ddi.h"
>> #include "intel_de.h"
>> #include "intel_display_irq.h"
>> @@ -4270,3 +4271,43 @@ bool intel_psr_needs_alpm_aux_less(struct
>> intel_dp *intel_dp, {
>> return intel_dp_is_edp(intel_dp) && crtc_state->has_panel_replay; }
>> +
>> +int intel_psr_compute_max_link_wake_latency(struct intel_dp *intel_dp,
>> + struct intel_crtc_state *crtc_state) {
>> +#define PHY_ESTABLISHMENT_PERIOD_MS 50000
>> +#define TFW_EXIT_LATENCY_MS 20000
>> +#define FAST_WAKE_LATENCY_MS 12000 /* Preamble: 8us; PHY
>> wake: 4us */
>> +#define LFPS_PERIOD_MS 800
>> +#define SILENCE_MAX_MS 180
>> + struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
>> + int linkrate_mhz = crtc_state->port_clock / 1000;
>> + int io_buffer_wake_ms;
>> + int clock_data_switch_ms;
>> + int aux_wake_latency_us;
>> + int auxless_latency_us;
>> + int time_ml_phy_lock_ms;
>> + int num_ml_phy_lock;
>> +
>> + io_buffer_wake_ms = intel_encoder_is_c10phy(encoder) ? 9790 :
>> 14790;
>> +
>> + aux_wake_latency_us =
>> + DIV_ROUND_UP(io_buffer_wake_ms +
>> TFW_EXIT_LATENCY_MS +
>> +FAST_WAKE_LATENCY_MS, 1000);
>> +
>> + /*
>> + * TPS4 length = 252
>> + * tML_PHY_LOCK = TPS4 Length * ( 10 / (Link Rate in MHz) )
>> + * Number ML_PHY_LOCK = ( 7 + CEILING( 6.5us / tML_PHY_LOCK ) + 1)
>> + * t2 = Number ML_PHY_LOCK * tML_PHY_LOCK
>> + * tCDS term = 2 * t2
>> + * =>tCDS_term = 2 * (7 * (252 * (10 /linkrate))+6.5)
>> + */
>> + time_ml_phy_lock_ms = (1000 * 252 * 10) / linkrate_mhz;
>> + num_ml_phy_lock = 7 + DIV_ROUND_UP(6500 * 1000,
>> time_ml_phy_lock_ms) / 1000 + 1;
>> + clock_data_switch_ms = 2 * time_ml_phy_lock_ms *
>> num_ml_phy_lock;
>> +
>> + auxless_latency_us = (LFPS_PERIOD_MS + SILENCE_MAX_MS +
>> PHY_ESTABLISHMENT_PERIOD_MS +
>> + clock_data_switch_ms) / 1000;
>> +
>> + return max(aux_wake_latency_us, auxless_latency_us); }
>> diff --git a/drivers/gpu/drm/i915/display/intel_psr.h
>> b/drivers/gpu/drm/i915/display/intel_psr.h
>> index 9b061a22361f..42277842af01 100644
>> --- a/drivers/gpu/drm/i915/display/intel_psr.h
>> +++ b/drivers/gpu/drm/i915/display/intel_psr.h
>> @@ -81,5 +81,7 @@ void intel_psr_debugfs_register(struct intel_display
>> *display); bool intel_psr_needs_alpm(struct intel_dp *intel_dp, const struct
>> intel_crtc_state *crtc_state); bool intel_psr_needs_alpm_aux_less(struct
>> intel_dp *intel_dp,
>> const struct intel_crtc_state *crtc_state);
>> +int intel_psr_compute_max_link_wake_latency(struct intel_dp *intel_dp,
>> + struct intel_crtc_state *crtc_state);
>>
>> #endif /* __INTEL_PSR_H__ */
>> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c
>> b/drivers/gpu/drm/i915/display/intel_vrr.c
>> index 46a85720411f..b73d99877ce3 100644
>> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
>> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
>> @@ -10,6 +10,8 @@
>> #include "intel_display_regs.h"
>> #include "intel_display_types.h"
>> #include "intel_dp.h"
>> +#include "intel_panel.h"
>> +#include "intel_psr.h"
>> #include "intel_vrr.h"
>> #include "intel_vrr_regs.h"
>>
>> @@ -413,15 +415,88 @@ intel_vrr_compute_config(struct intel_crtc_state
>> *crtc_state,
>> }
>> }
>>
>> -void intel_vrr_compute_config_late(struct intel_crtc_state *crtc_state)
>> +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);
>> + struct intel_dp *intel_dp;
>> + int dsc_prefill_time = 0;
>> + int psr2_pr_latency = 0;
>> + int scaler_prefill_time;
>> + int wm0_prefill_time;
>> + int sdp_latency = 0;
>> + int guardband_us;
>> + int linetime_us;
>> + int guardband;
>> + int vblank_us;
>> + int pm_delay;
>> +
>> + linetime_us = DIV_ROUND_UP(adjusted_mode->crtc_htotal * 1000,
>> + adjusted_mode->crtc_clock);
>> +
>> + /* Assuming max wm0 lines = 4 */
>> + wm0_prefill_time = 4 * linetime_us + 20;
>> +
>> + /*
>> + * 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)
>> + */
>> + scaler_prefill_time = 4 * 2 * 2 * linetime_us;
>> +
>> + if (crtc_state->dsc.compression_enable)
>> + dsc_prefill_time = (3 * 2 * 2 * 2 * linetime_us) / 2;
> Can we devide these all calculations into small separate static functions ?
>
> And also, we are already having part of these implementations like dsc prefill time and scaler prefill time calculated, can we utilize them so that duplications can be avoided ?
>
> Or if we want to use this, then we also need to revert them from skl_is_vblank_too_short ? As now we are already taking care of those latencies to here.
Thanks Mitul for the comments. I agree with the suggestions. I have made
separate functions for these in the new version.
You are right, we need to check the guardband. Earlier we were checking
for vblank since guardband was matched to vblank length.
With optimized guardband we need to remove these checks.
I have removed the changes from skl_is_vblank_too_short and added a new
check for guardband in the next version.
Thanks & Regards,
Ankit
>
>> +
>> + pm_delay = crtc_state->framestart_delay +
>> + display->sagv.block_time_us +
>> + wm0_prefill_time +
>> + scaler_prefill_time +
>> + dsc_prefill_time;
>> +
>> + switch (connector->base.connector_type) {
>> + case DRM_MODE_CONNECTOR_eDP:
>> + case DRM_MODE_CONNECTOR_DisplayPort:
>> + intel_dp = intel_attached_dp(connector);
>> + psr2_pr_latency =
>> intel_psr_compute_max_link_wake_latency(intel_dp, crtc_state);
>> + /* Assuming VSC_EXT is required */
>> + sdp_latency = 10 * linetime_us;
>> + break;
>> + default:
>> + break;
>> + }
>> +
>> + guardband_us = max(sdp_latency, psr2_pr_latency);
>> + guardband_us = max(guardband_us, pm_delay);
>> + vblank_us = (adjusted_mode->crtc_vtotal -
>> +adjusted_mode->crtc_vblank_start) * linetime_us;
>> +
>> + if (vblank_us > 0 && guardband_us > vblank_us) {
>> + drm_dbg_kms(display->drm, "guardband_us %dus > vblank_us
>> %dus\n", guardband_us, vblank_us);
>> + guardband_us = vblank_us;
>> + }
>> +
>> + guardband = DIV_ROUND_UP(guardband_us, linetime_us);
>> + 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.flipline = crtc_state->vrr.guardband +
>> + adjusted_mode-
>>> crtc_vblank_start;
>> + } 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);
>> 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-gfx
mailing list