On 6/16/2026 8:12 PM, Mitul Golani wrote:
Replace the disabled,
I guess it is still disabled at this point, so the commit message is a
bit misleading.
eDP-only,
Should be a separate patch.
fractional-CMRR code
with a generic, transcoder-agnostic computation driven by an
explicit per-CRTC target. Compute CMRR_M and CMRR_N timings
based on video mode reqirement
typo
if CMRR is required to be enabled.
Signed-off-by: Mitul Golani <[email protected]>
---
.../drm/i915/display/intel_display_types.h | 2 +
drivers/gpu/drm/i915/display/intel_vrr.c | 121 +++++++++---------
2 files changed, 63 insertions(+), 60 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
b/drivers/gpu/drm/i915/display/intel_display_types.h
index 897a1ffd7b79..39e11362630c 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1548,6 +1548,8 @@ struct intel_crtc {
struct {
u32 numerator;
u32 denominator;
+ /* Derived during atomic check: 1000/1001 video timing required
*/
+ bool video_mode;
I am not sure why this is needed at all, since you can just use the
numerator and denominator to derive if it is a video mode within
compute_config.
} cmrr;
int scanline_offset;
diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c
b/drivers/gpu/drm/i915/display/intel_vrr.c
index 41118883b845..e36c0cab096a 100644
--- a/drivers/gpu/drm/i915/display/intel_vrr.c
+++ b/drivers/gpu/drm/i915/display/intel_vrr.c
@@ -26,9 +26,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.
@@ -186,69 +183,69 @@ int intel_vrr_vmax_vblank_start(const struct
intel_crtc_state *crtc_state)
return intel_vrr_vmax_vtotal(crtc_state) - crtc_state->vrr.guardband;
}
-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)
{
struct intel_display *display = to_intel_display(crtc_state);
- int calculated_refresh_k, actual_refresh_k, pixel_clock_per_line;
+ struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
-
- /* Avoid CMRR for now till we have VRR with fixed timings working */
- if (!HAS_CMRR(display) || 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;
+ int requested_refresh_rate, current_refresh_rate;
+ int multiplier_m = 1, multiplier_n = 1;
- desired_refresh_rate = drm_mode_vrefresh(adjusted_mode);
+ if (!HAS_CMRR(display))
+ return;
- if (video_mode_required) {
- multiplier_m = 1001;
- multiplier_n = 1000;
- }
+ /* No CMRR ratio configured through debugfs */
+ if (!crtc->cmrr.numerator)
+ return;
- crtc_state->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->cmrr.cmrr_n);
- adjusted_pixel_rate = mul_u32_u32(adjusted_mode->crtc_clock * 1000,
multiplier_m);
- crtc_state->cmrr.cmrr_m = do_div(adjusted_pixel_rate,
crtc_state->cmrr.cmrr_n);
+ /*
+ * The numerator encodes the requested refresh rate in kHz, so the
Not accurate, because numerator/divider together provides the refresh
rate. If anything the numerator is in mHz as commented in patch 1.
+ * requested refresh rate in Hz is numerator / 1000. It must match the
+ * refresh rate of the current mode.
+ */
+ requested_refresh_rate = crtc->cmrr.numerator / 1000;
Integer division loses precision. It can be used in the comparison below
but incorrect to use it in actual M and N value calculations done later.
+ 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;
+ }
- return vtotal;
-}
+ /*
+ * A 1:1 ratio (denominator == 1000) means no video timing is required
+ * Any other ratio (e.g. 1000/1001) requires the video timing.
+ */
+ crtc->cmrr.video_mode = crtc->cmrr.denominator != 1000;
+ if (crtc->cmrr.video_mode) {
+ multiplier_m = 1000;
+ multiplier_n = 1001;
+ }
more intuitive to use names like rr_multiplier, rr_divider. Also the
names are reversed now.
-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.
+ * Let pixel_clock_hz = adjusted_mode->crtc_clock * 1000.
+ *
+ * cmrr_n = requested_refresh_rate x htotal x multiplier_m
+ * cmrr_m = (pixel_clock_hz x scale_m) % cmrr_n
+ *
+ * where multiplier_m/multiplier_n = 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.
*/
- 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;
- crtc_state->cmrr.enable = true;
- crtc_state->mode_flags |= I915_MODE_FLAG_VRR;
where do we set this flag now?
+ crtc_state->cmrr.cmrr_n =
+ mul_u32_u32(requested_refresh_rate * adjusted_mode->crtc_htotal,
+ multiplier_m);
+ adjusted_pixel_rate = mul_u32_u32(adjusted_mode->crtc_clock, 1000) *
multiplier_n;
Please align the calculation as per BSpec-68925, calculate the cmrr
votal here and retain the cmrr_get_vtotal semantics.
+ crtc_state->cmrr.cmrr_m = do_div(adjusted_pixel_rate,
crtc_state->cmrr.cmrr_n);
+
+ return;
}
static
@@ -424,8 +421,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;
@@ -459,13 +454,19 @@ 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 {
intel_vrr_compute_fixed_rr_timings(crtc_state);
+ /*
+ * 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);
+ }
+
if (HAS_AS_SDP(display)) {
crtc_state->vrr.vsync_start =
(crtc_state->hw.adjusted_mode.crtc_vtotal -
