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;