Re: [PATCH v2 05/11] drm/i915/vrr: Enable/Disable CMRR based on enable/disable preconditions

[Borah, Chaitanya Kumar]

On 6/16/2026 8:12 PM, Mitul Golani wrote:

Enable or disable CMRR based on it is being computed and set of
preconditions. Should be enabled when dueing adaptive mode as well
as conditions which sets cmrr.enable gets set similarly to disable case.
Also separate out register writes for both enable and disable case.

The intention of this patch is unclear to me.

Remove VRR_CTL_CMRR_ENABLE write in the current path, add the same
when actually CMRR is enabled.

Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.gol...@intel.com>
---
  drivers/gpu/drm/i915/display/intel_vrr.c | 81 ++++++++++++++++++++++--
  1 file changed, 76 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c 
b/drivers/gpu/drm/i915/display/intel_vrr.c
index 5678c3a86796..c979950d32cb 100644
--- a/drivers/gpu/drm/i915/display/intel_vrr.c
+++ b/drivers/gpu/drm/i915/display/intel_vrr.c
@@ -34,6 +34,43 @@
  #define DCB_CORRECTION_AGGRESSIVENESS 1000 /* ms × 100; 10 ms */
  #define DCB_BLANK_TARGET              50

+#define is_enabling(feature, old_crtc_state, new_crtc_state) \

+       ((!(old_crtc_state)->feature || intel_crtc_needs_modeset(new_crtc_state)) 
&& \
+        (new_crtc_state)->feature)
+#define is_disabling(feature, old_crtc_state, new_crtc_state) \
+       ((old_crtc_state)->feature && \
+        (!(new_crtc_state)->feature || 
intel_crtc_needs_modeset(new_crtc_state)))
+
+static bool intel_crtc_cmrr_enabling(struct intel_atomic_state *state,
+                                    struct intel_crtc *crtc)
+{
+       const struct intel_crtc_state *old_crtc_state =
+               intel_atomic_get_old_crtc_state(state, crtc);
+       const struct intel_crtc_state *new_crtc_state =
+               intel_atomic_get_new_crtc_state(state, crtc);
+
+       if (!new_crtc_state->hw.active)
+               return false;
+
+       return is_enabling(cmrr.enable, old_crtc_state, new_crtc_state) ||
+               (new_crtc_state->cmrr.enable);
+}
+
+static bool intel_crtc_cmrr_disabling(struct intel_atomic_state *state,
+                                     struct intel_crtc *crtc)
+{
+       const struct intel_crtc_state *old_crtc_state =
+               intel_atomic_get_old_crtc_state(state, crtc);
+       const struct intel_crtc_state *new_crtc_state =
+               intel_atomic_get_new_crtc_state(state, crtc);
+
+       if (!new_crtc_state->hw.active)
+               return false;
+
+       return is_disabling(cmrr.enable, old_crtc_state, new_crtc_state) ||
+                       (old_crtc_state->cmrr.enable);
+}
+
  bool intel_vrr_is_capable(struct intel_connector *connector)
  {
        struct intel_display *display = to_intel_display(connector);
@@ -831,6 +868,34 @@ static void intel_vrr_set_vrr_timings(const struct 
intel_crtc_state *crtc_state)
                       intel_vrr_hw_flipline(crtc_state) - 1);
  }

+static void

+intel_vrr_enable_cmrr(const struct intel_crtc_state *crtc_state)
+{
+       struct intel_display *display = to_intel_display(crtc_state);
+       enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
+
+       intel_de_write(display, TRANS_CMRR_M_HI(display, cpu_transcoder),
+                      upper_32_bits(crtc_state->cmrr.cmrr_m));
+       intel_de_write(display, TRANS_CMRR_M_LO(display, cpu_transcoder),
+                      lower_32_bits(crtc_state->cmrr.cmrr_m));
+       intel_de_write(display, TRANS_CMRR_N_HI(display, cpu_transcoder),
+                      upper_32_bits(crtc_state->cmrr.cmrr_n));
+       intel_de_write(display, TRANS_CMRR_N_LO(display, cpu_transcoder),
+                      lower_32_bits(crtc_state->cmrr.cmrr_n));
+}
+
+static void
+intel_vrr_disable_cmrr(const struct intel_crtc_state *crtc_state)
+{
+       struct intel_display *display = to_intel_display(crtc_state);
+       enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
+
+       intel_de_write(display, TRANS_CMRR_M_HI(display, cpu_transcoder), 0);
+       intel_de_write(display, TRANS_CMRR_M_LO(display, cpu_transcoder), 0);
+       intel_de_write(display, TRANS_CMRR_N_HI(display, cpu_transcoder), 0);
+       intel_de_write(display, TRANS_CMRR_N_LO(display, cpu_transcoder), 0);
+}
+
  static void
  intel_vrr_enable_dc_balancing(const struct intel_crtc_state *crtc_state)
  {
@@ -934,8 +999,6 @@ static void intel_vrr_tg_enable(const struct 
intel_crtc_state *crtc_state,
         * even VRR_CTL_CMRR_ENABLE is armed by TRANS_CMRR_N_HI
         * when enabling CMRR (but not when disabling CMRR?).
         */
-       if (cmrr_enable)
-               vrr_ctl |= VRR_CTL_CMRR_ENABLE;

intel_de_write(display, TRANS_VRR_CTL(display, cpu_transcoder), vrr_ctl); @@ -962,10 +1025,15 @@ static void intel_vrr_tg_disable(const struct intel_crtc_state *old_crtc_state)

  void intel_vrr_enable(const struct intel_crtc_state *crtc_state)
  {
        struct intel_display *display = to_intel_display(crtc_state);
+       struct intel_atomic_state *state = 
to_intel_atomic_state(crtc_state->uapi.state);
+       struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);

if (!crtc_state->vrr.enable)

                return;

+ if (intel_crtc_cmrr_disabling(state, crtc))

+               intel_vrr_disable_cmrr(crtc_state);
+
        intel_vrr_set_vrr_timings(crtc_state);
        intel_vrr_enable_dc_balancing(crtc_state);

@@ -976,6 +1044,8 @@ void intel_vrr_enable(const struct intel_crtc_state *crtc_state)

  void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state)
  {
        struct intel_display *display = to_intel_display(old_crtc_state);
+       struct intel_atomic_state *state = 
to_intel_atomic_state(old_crtc_state->uapi.state);
+       struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->uapi.crtc);

if (!old_crtc_state->vrr.enable)

                return;
@@ -984,6 +1054,10 @@ void intel_vrr_disable(const struct intel_crtc_state 
*old_crtc_state)
                intel_vrr_tg_disable(old_crtc_state);

intel_vrr_disable_dc_balancing(old_crtc_state);

+
+       if (intel_crtc_cmrr_enabling(state, crtc))
+               intel_vrr_enable_cmrr(old_crtc_state);
+
        intel_vrr_set_fixed_rr_timings(old_crtc_state, 
old_crtc_state->cpu_transcoder);
  }

@@ -1066,9 +1140,6 @@ void intel_vrr_get_config(struct intel_crtc_state *crtc_state)

        trans_vrr_ctl = intel_de_read(display,
                                      TRANS_VRR_CTL(display, cpu_transcoder));

- if (HAS_CMRR(display))

-               crtc_state->cmrr.enable = (trans_vrr_ctl & VRR_CTL_CMRR_ENABLE);
-
        if (crtc_state->cmrr.enable) {
                crtc_state->cmrr.cmrr_n =
                        intel_de_read64_2x32(display, TRANS_CMRR_N_LO(display, 
cpu_transcoder));