On 9/8/2025 10:13 PM, Ville Syrjälä wrote:
On Sun, Sep 07, 2025 at 01:02:39PM +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.

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.
v4: Avoid re-setting the Vmin/Flipline for optimized guardband.

Bspec: 70151
Signed-off-by: Ankit Nautiyal <ankit.k.nauti...@intel.com>
Reviewed-by: Mitul Golani <mitulkumar.ajitkumar.gol...@intel.com> (#v3)
---
  drivers/gpu/drm/i915/display/intel_display.c |   2 +-
  drivers/gpu/drm/i915/display/intel_vrr.c     | 127 ++++++++++++++++++-
  drivers/gpu/drm/i915/display/intel_vrr.h     |   3 +-
  3 files changed, 128 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
b/drivers/gpu/drm/i915/display/intel_display.c
index fb072275b1c7..3fa94675d5e1 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -4902,7 +4902,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) {
@@ -4914,6 +4913,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 855974174afd..fff684eb2514 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,135 @@ 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)
+{
+       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_vrr_guardband_scaler_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_vrr_guardband_dsc_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);
This all looks to be in the wrong place. It needs to be done as part of
intel_crtc_compute_vblank_delay() which then updates crtc_vblank_start
to reflect reality instead of leaving some incorrect junk in there.
Presumably that is why you also had to do that
intel_dsb_wait_for_delayed_vblank() hack.

Hi Ville,

Thanks for for the comments.

Looks like intel_crtc_compute_vblank_delay() indeed is the place to add the vblank_delay and get the crtc_vblank_start point to the delayed vblank.

But I think, there are couple of things need to take care with this change:
1) The SCL currently is derived from crtc_vblank_start - vdisplay, which is ~1 with guradband = vblank length. With modified crtc_vblank_start, this will become huge so perhaps we need to set it to 1?

2) The intel_dsb_wait_vblank_delay() (which inturn uses intel_vrr_real_vblank_delay() for vrr case) also computes a fixed delay based on (crtc_vblank_start - vdisplay) lines Instead of this should we not use intel_dsb_wait_scanline_in() to wait only till scanline reaches in range  [flipline decision boundary, vmax decision boundary]?

Let me know what you think, I can send a follow-up patch with the suggested changes.

Regards,
Ankit



+       } 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 950041647e47..362638fd0d66 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

Reply via email to