On 2/26/2025 8:30 PM, Ville Syrjälä wrote:
On Mon, Feb 24, 2025 at 11:47:13AM +0530, Ankit Nautiyal wrote:
To have Guardband/Pipeline_full reconfigured seamlessly, move the
guardband and pipeline_full checks out from the pure !fastset block in
intel_pipe_config_compare().
Update the intel_set_transcoder_timings_lrr() function to use
fixed refresh rate timings for platforms which always use
VRR timing generator.

Signed-off-by: Ankit Nautiyal <ankit.k.nauti...@intel.com>
---
  drivers/gpu/drm/i915/display/intel_display.c | 43 ++++++++++++++++----
  drivers/gpu/drm/i915/display/intel_vrr.c     | 13 +++++-
  drivers/gpu/drm/i915/display/intel_vrr.h     |  2 +
  3 files changed, 48 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
b/drivers/gpu/drm/i915/display/intel_display.c
index 752ed44d8892..d3c12bc3645c 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -2854,6 +2854,9 @@ static void intel_set_transcoder_timings_lrr(const struct 
intel_crtc_state *crtc
        intel_de_write(dev_priv, TRANS_VTOTAL(dev_priv, cpu_transcoder),
                       VACTIVE(crtc_vdisplay - 1) |
                       VTOTAL(crtc_vtotal - 1));
+
+       intel_vrr_set_fixed_rr_timings(crtc_state);
+       intel_vrr_update_guardband(crtc_state, false);
  }
static void intel_set_pipe_src_size(const struct intel_crtc_state *crtc_state)
@@ -5127,17 +5130,36 @@ pipe_config_cx0pll_mismatch(struct drm_printer *p, bool 
fastset,
        intel_cx0pll_dump_hw_state(display, b);
  }
-static bool allow_vblank_delay_fastset(const struct intel_crtc_state *old_crtc_state)
+static
+bool allow_vrr_guardband_fastset(const struct intel_crtc_state *old_crtc_state,
+                                const struct intel_crtc_state *new_crtc_state)
+{
+       struct intel_display *display = to_intel_display(old_crtc_state);
+
+       if (!HAS_VRR(display))
+               return false;
+
+       if (DISPLAY_VER(display) >= 13)
+               return new_crtc_state->vrr.guardband != 
old_crtc_state->vrr.guardband;
+
+       return new_crtc_state->vrr.pipeline_full != 
old_crtc_state->vrr.pipeline_full;
+}
None of this stuff should be necessary. allow_vblank_delay_fastset()
only says whether we're allowed the fastset or not. It isn't supposed
to check if it's necessary or not.

+
+static bool allow_vblank_delay_fastset(const struct intel_crtc_state 
*old_crtc_state,
+                                      const struct intel_crtc_state 
*new_crtc_state)
  {
        struct intel_display *display = to_intel_display(old_crtc_state);
/*
-        * Allow fastboot to fix up vblank delay (handled via LRR
-        * codepaths), a bit dodgy as the registers aren't
+        * Allow fastboot to fix up vblank delay, vrr guardband/pipeline_full
+        * (handled via LRR codepaths), a bit dodgy as the registers aren't
         * double buffered but seems to be working more or less...
         */
-       return HAS_LRR(display) && old_crtc_state->inherited &&
-               !intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_DSI);
+       if (!HAS_LRR(display) || intel_crtc_has_type(old_crtc_state, 
INTEL_OUTPUT_DSI))
+               return false;
+
+       return old_crtc_state->inherited ||
+              allow_vrr_guardband_fastset(old_crtc_state, new_crtc_state);
  }
bool
@@ -5273,7 +5295,7 @@ intel_pipe_config_compare(const struct intel_crtc_state 
*current_config,
        PIPE_CONF_CHECK_I(name.crtc_hsync_start); \
        PIPE_CONF_CHECK_I(name.crtc_hsync_end); \
        PIPE_CONF_CHECK_I(name.crtc_vdisplay); \
-       if (!fastset || !allow_vblank_delay_fastset(current_config)) \
+       if (!fastset || !allow_vblank_delay_fastset(current_config, 
pipe_config)) \
                PIPE_CONF_CHECK_I(name.crtc_vblank_start); \
        PIPE_CONF_CHECK_I(name.crtc_vsync_start); \
        PIPE_CONF_CHECK_I(name.crtc_vsync_end); \
@@ -5569,8 +5591,6 @@ intel_pipe_config_compare(const struct intel_crtc_state 
*current_config,
                PIPE_CONF_CHECK_I(vrr.vmin);
                PIPE_CONF_CHECK_I(vrr.vmax);
                PIPE_CONF_CHECK_I(vrr.flipline);
-               PIPE_CONF_CHECK_I(vrr.pipeline_full);
-               PIPE_CONF_CHECK_I(vrr.guardband);
                PIPE_CONF_CHECK_I(vrr.vsync_start);
                PIPE_CONF_CHECK_I(vrr.vsync_end);
                PIPE_CONF_CHECK_LLI(cmrr.cmrr_m);
@@ -5578,6 +5598,11 @@ intel_pipe_config_compare(const struct intel_crtc_state 
*current_config,
                PIPE_CONF_CHECK_BOOL(cmrr.enable);
        }
+ if (!fastset || !allow_vblank_delay_fastset(current_config, pipe_config)) {
+               PIPE_CONF_CHECK_I(vrr.guardband);
+               PIPE_CONF_CHECK_I(vrr.pipeline_full);
+       }
I'd like to see one patch that just moves these completely out
of the !fastset block, and a second patch that adds the fastboot
exception. That way if we do see any problems from the live
guardband reprogramming we can just revert it.
I have one doubt.

If we remove guardband/pipeline_full from the !fastset block, the VRR-seamless-switch case will start failing.

This is because the test tries a virtual LRR mode, resulting in a change in the VRR guardband.

Since its not in fastset block, we will need a full modeset, but the test expects a seamless switch.

Regards,

Ankit



Hmm, except you haven't changed the vrr_compute_config() yet at
this point in the series, so changing this would break both fastset
and state checker, I think. So we might have to move this patch to be
later in the series.

+
  #undef PIPE_CONF_CHECK_X
  #undef PIPE_CONF_CHECK_I
  #undef PIPE_CONF_CHECK_LLI
@@ -5875,7 +5900,7 @@ static void intel_crtc_check_fastset(const struct 
intel_crtc_state *old_crtc_sta
                drm_dbg_kms(&i915->drm, "[CRTC:%d:%s] fastset requirement not met, 
forcing full modeset\n",
                            crtc->base.base.id, crtc->base.name);
        } else {
-               if (allow_vblank_delay_fastset(old_crtc_state))
+               if (allow_vblank_delay_fastset(old_crtc_state, new_crtc_state))
                        new_crtc_state->update_lrr = true;
                new_crtc_state->uapi.mode_changed = false;
        }
diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c 
b/drivers/gpu/drm/i915/display/intel_vrr.c
index 11b7edbdb116..e77f5b483b09 100644
--- a/drivers/gpu/drm/i915/display/intel_vrr.c
+++ b/drivers/gpu/drm/i915/display/intel_vrr.c
@@ -284,7 +284,6 @@ int intel_vrr_fixed_rr_flipline(const struct 
intel_crtc_state *crtc_state)
        return intel_vrr_fixed_rr_vtotal(crtc_state);
  }
-static
  void intel_vrr_set_fixed_rr_timings(const struct intel_crtc_state *crtc_state)
  {
        struct intel_display *display = to_intel_display(crtc_state);
@@ -677,3 +676,15 @@ void intel_vrr_get_config(struct intel_crtc_state 
*crtc_state)
        if (crtc_state->vrr.enable)
                crtc_state->mode_flags |= I915_MODE_FLAG_VRR;
  }
+
+void intel_vrr_update_guardband(const struct intel_crtc_state *crtc_state, 
bool needs_modeset)
+{
+       struct intel_display *display = to_intel_display(crtc_state);
+       enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
+       u32 vrr_ctl = trans_vrr_ctl(crtc_state);
+
+       if (intel_vrr_always_use_vrr_tg(display) && !needs_modeset)
+               vrr_ctl |= VRR_CTL_VRR_ENABLE;
+
+       intel_de_write(display, TRANS_VRR_CTL(display, cpu_transcoder), 
vrr_ctl);
+}
diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h 
b/drivers/gpu/drm/i915/display/intel_vrr.h
index 514822577e8a..e4e9cadcdf9e 100644
--- a/drivers/gpu/drm/i915/display/intel_vrr.h
+++ b/drivers/gpu/drm/i915/display/intel_vrr.h
@@ -35,5 +35,7 @@ int intel_vrr_vmin_vtotal(const struct intel_crtc_state 
*crtc_state);
  int intel_vrr_vmax_vblank_start(const struct intel_crtc_state *crtc_state);
  int intel_vrr_vmin_vblank_start(const struct intel_crtc_state *crtc_state);
  int intel_vrr_vblank_delay(const struct intel_crtc_state *crtc_state);
+void intel_vrr_update_guardband(const struct intel_crtc_state *crtc_state, 
bool needs_modeset);
+void intel_vrr_set_fixed_rr_timings(const struct intel_crtc_state *crtc_state);
#endif /* __INTEL_VRR_H__ */
--
2.45.2

Reply via email to