On Tue, 16 Jun 2026, Mitul Golani <[email protected]> wrote:
> Replace the disabled, eDP-only, 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 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;
compute config must not change permanent state, only crtc state.
Moreover, the video_mode here is just a funcion of denominator, so why
do we cache it when it can be computed on the fly?
> } 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
> + * 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;
> + 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;
> + }
>
> -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;
> + 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;
> + 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 -
--
Jani Nikula, Intel
