On 7/22/2026 10:09 AM, Mitul Golani wrote:
Rework the fractional-CMRR computation into a generic,
transcoder-agnostic helper driven by an explicit per-CRTC debugfs
target, replacing the previous disabled, eDP-only code path. Compute
CMRR_M and CMRR_N timings based on the video mode requirement. Note the
CMRR enable path is wired up separately; this patch only lays down the
generic computation. Remove computing mode_flags, I915_MODE_FLAG_VRR
as we are moving CMRR to fixed refresh rate path. Also return early
from CMRR computation if existing computation is well sufficient.

--v2:
- Derive video_mode locally instead of caching it in persistent
   struct intel_crtc state (Jani, Chaitanya)
- Fix numerator unit in comment: milli-Hz, not kHz (Chaitanya)
- Fix "reqirement" typo and clarify CMRR is not yet enabled in the
   commit message (Chaitanya)
- Fix precision issue while computing M/N ration (Chaitanya)

typo: ratio

- Multiplier_m and n naming update to increase readability. (Chaitanya)
- Compute vtotal as it is required to deither as per algo

typo: dither

implementation. (Chaitanya)
- Replace misleading adjusted_pixel_rate to dividend which is somewhat
relatable.

--v3:
- Mention rationale for not to compute I915_MODE_FLAG_VRR. (Chaitanya)
- Remove redundant return statement from last line of cmrr compute.
(Chaitanya)
- Correct cmrr_n calculation. (Chaitanya)
- Add early return condition is current computaion is well sufficient

typo: computation


to drive without CMRR. (Chaitanya)
- Take round up for requested_refresh_rate. (Validation)
- Correct integer overflow

--v4:
- Update cmrr_n calculation to aviod aggressive dithering

typo: avoid

- Use div64_u64 to truncate value to floor value, as DIV_ROUND_UP_ULL
rounding to ceil value.
- Add TODO comment at cmrr_n calculation
- Add condition for custom refresh rate request
- Update CMRR computational comment block in copute config

typo: compute


Signed-off-by: Mitul Golani <[email protected]>
---
  drivers/gpu/drm/i915/display/intel_vrr.c | 138 ++++++++++++-----------
  1 file changed, 72 insertions(+), 66 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c 
b/drivers/gpu/drm/i915/display/intel_vrr.c
index 84b1ec7f1616..e8c6516fd36a 100644
--- a/drivers/gpu/drm/i915/display/intel_vrr.c
+++ b/drivers/gpu/drm/i915/display/intel_vrr.c
@@ -27,9 +27,6 @@
  #include "skl_prefill.h"
  #include "skl_watermark.h"
-#define FIXED_POINT_PRECISION 100
-#define CMRR_PRECISION_TOLERANCE       10
-
  /*
   * Tunable parameters for DC Balance correction.
   * These are captured based on experimentations.
@@ -198,68 +195,79 @@ static bool intel_vrr_cmrr_possible(const struct 
intel_crtc_state *crtc_state)
        return (HAS_CMRR(display) && intel_vrr_always_use_vrr_tg(display));
  }
-static bool
-is_cmrr_frac_required(struct intel_crtc_state *crtc_state)
+static void
+intel_vrr_cmrr_compute_config(struct intel_crtc_state *crtc_state)
  {
-       int calculated_refresh_k, actual_refresh_k, pixel_clock_per_line;
+       struct intel_display *display = to_intel_display(crtc_state);
+       struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
        struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
+       u64 dividend;
+       int requested_refresh_rate, current_refresh_rate;
+       int rr_multiplier = 1, rr_divider = 1;
+       bool video_mode;
- /* Avoid CMRR for now till we have VRR with fixed timings working */
-       if (!intel_vrr_cmrr_possible(crtc_state) || true)
-               return false;
-
-       actual_refresh_k =
-               drm_mode_vrefresh(adjusted_mode) * FIXED_POINT_PRECISION;
-       pixel_clock_per_line =
-               adjusted_mode->crtc_clock * 1000 / adjusted_mode->crtc_htotal;
-       calculated_refresh_k =
-               pixel_clock_per_line * FIXED_POINT_PRECISION / 
adjusted_mode->crtc_vtotal;
-
-       if ((actual_refresh_k - calculated_refresh_k) < 
CMRR_PRECISION_TOLERANCE)
-               return false;
-
-       return true;
-}
-
-static unsigned int
-cmrr_get_vtotal(struct intel_crtc_state *crtc_state, bool video_mode_required)
-{
-       int multiplier_m = 1, multiplier_n = 1, vtotal, desired_refresh_rate;
-       u64 adjusted_pixel_rate;
-       struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
+       if (!intel_vrr_cmrr_possible(crtc_state))
+               return;
- desired_refresh_rate = drm_mode_vrefresh(adjusted_mode);
+       /* No CMRR ratio configured through debugfs */
+       if (!crtc->force_cmrr.numerator)
+               return;
- if (video_mode_required) {
-               multiplier_m = 1001;
-               multiplier_n = 1000;
+       /*
+        * The numerator encodes the requested refresh rate in milli-Hz, so the
+        * requested refresh rate in Hz is numerator / 1000. It must match the
+        * refresh rate of the current mode.
+        */
+       requested_refresh_rate = DIV_ROUND_UP_ULL(crtc->force_cmrr.numerator, 
1000);
+       current_refresh_rate = drm_mode_vrefresh(adjusted_mode);
+
+       if (requested_refresh_rate != current_refresh_rate) {
+               drm_dbg_kms(display->drm,
+                           "[CRTC:%d:%s] CMRR requested refresh rate %d Hz does not 
match current mode refresh rate %d Hz\n",
+                               crtc->base.base.id, crtc->base.name,
+                               requested_refresh_rate, current_refresh_rate);
+               return;
        }
- crtc_state->vrr.cmrr.cmrr_n = mul_u32_u32(desired_refresh_rate * adjusted_mode->crtc_htotal,
-                                                 multiplier_n);
-       vtotal = DIV_ROUND_UP_ULL(mul_u32_u32(adjusted_mode->crtc_clock * 1000, 
multiplier_n),
-                                 crtc_state->vrr.cmrr.cmrr_n);
-       adjusted_pixel_rate = mul_u32_u32(adjusted_mode->crtc_clock * 1000, 
multiplier_m);
-       crtc_state->vrr.cmrr.cmrr_m = do_div(adjusted_pixel_rate, 
crtc_state->vrr.cmrr.cmrr_n);
-
-       return vtotal;
-}
-
-static
-void intel_vrr_compute_cmrr_timings(struct intel_crtc_state *crtc_state)
-{
        /*
-        * TODO: Compute precise target refresh rate to determine
-        * if video_mode_required should be true. Currently set to
-        * false due to uncertainty about the precise target
-        * refresh Rate.
+        * A 1:1 ratio (denominator == 1000) means no video timing is required
+        * Any other ratio (e.g. 1000/1001) requires the video timing.
         */
-       crtc_state->vrr.vmax = cmrr_get_vtotal(crtc_state, false);
-       crtc_state->vrr.vmin = crtc_state->vrr.vmax;
-       crtc_state->vrr.flipline = crtc_state->vrr.vmin;
+       video_mode = crtc->force_cmrr.denominator != 1000;
+       if (video_mode) {
+               rr_multiplier = 1000;
+               rr_divider = 1001;
+       } else if (crtc->force_cmrr.numerator == current_refresh_rate * 1000) {

As discussed this should be a comparision between num/den and actual refresh rate derived from the mode line. Moreover, this check should be done for both video and non video mode branches.

+               drm_dbg_kms(display->drm,
+                           "[CRTC:%d:%s] Requested mode refresh rate %d Hz can be 
driven without CMRR\n",
+                               crtc->base.base.id, crtc->base.name,
+                               requested_refresh_rate);
+               return;
+       }
- crtc_state->vrr.cmrr.enable = true;
-       crtc_state->mode_flags |= I915_MODE_FLAG_VRR;
+       /*
+        * Let pixel_clock_hz = adjusted_mode->crtc_clock * 1000.
+        *
+        * cmrr_n = (numerator_mHz x htotal x rr_multiplier) / 1000
+        * cmrr_m = (pixel_clock_hz x rr_divider) % cmrr_n
+        *
+        * where numerator_mHz is the requested refresh rate in milli-Hz
+        * and rr_multiplier/rr_divider = 1000/1001 when the video timing
+        * is required, else 1/1. The integer vtotal term is tracked in SW
+        * (it is the programmed mode vtotal) while the fractional part
+        * represented by cmrr_m/cmrr_n is tracked in HW.
+        *
+        * TODO: Using the actual desired rate for cmrr_n in video
+        * mode produces more aggressive vtotal dithering than
+        * expected; revisit once the Bspec algorithm is clarified.
+        */
+       crtc_state->vrr.cmrr.cmrr_n =
+               div64_u64((u64)crtc->force_cmrr.numerator *
+                          adjusted_mode->crtc_htotal * rr_multiplier, 1000);
+       dividend = (u64)adjusted_mode->crtc_clock * rr_divider * 1000;
+       adjusted_mode->crtc_vtotal = div64_u64_rem(dividend,
+                                                  crtc_state->vrr.cmrr.cmrr_n,
+                                                  
&crtc_state->vrr.cmrr.cmrr_m);
  }
static
@@ -435,8 +443,6 @@ intel_vrr_compute_config(struct intel_crtc_state 
*crtc_state,
        struct intel_display *display = to_intel_display(crtc_state);
        struct intel_connector *connector =
                to_intel_connector(conn_state->connector);
-       struct intel_dp *intel_dp = intel_attached_dp(connector);
-       bool is_edp = intel_dp_is_edp(intel_dp);
        struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
        int vmin, vmax;
@@ -470,12 +476,17 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
                vmax = vmin;
        }
- if (crtc_state->uapi.vrr_enabled && vmin < vmax)
+       if (crtc_state->uapi.vrr_enabled && vmin < vmax) {
                intel_vrr_compute_vrr_timings(crtc_state, vmin, vmax);
-       else if (is_cmrr_frac_required(crtc_state) && is_edp)
-               intel_vrr_compute_cmrr_timings(crtc_state);
-       else
+       } else {
+               /*
+                * CMRR is a fixed average Vtotal mode and is only computed on
+                * the fixed refresh rate path. It is generic across transcoders
+                * and gated on platform support and a valid debugfs ratio.
+                */
+               intel_vrr_cmrr_compute_config(crtc_state);
                intel_vrr_compute_fixed_rr_timings(crtc_state);
+       }
if (HAS_AS_SDP(display)) {
                crtc_state->vrr.vsync_start =
@@ -1142,11 +1153,6 @@ void intel_vrr_get_config(struct intel_crtc_state 
*crtc_state)
intel_vrr_get_dc_balance_config(crtc_state); - /*
-        * #TODO: For Both VRR and CMRR the flag I915_MODE_FLAG_VRR is set for 
mode_flags.
-        * Since CMRR is currently disabled, set this flag for VRR for now.
-        * Need to keep this in mind while re-enabling CMRR.
-        */
        if (crtc_state->vrr.enable)
                crtc_state->mode_flags |= I915_MODE_FLAG_VRR;

Reply via email to