Intel-XE Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
To: intel-gfx@lists.freedesktop.org
Cc: intel-xe@lists.freedesktop.org, uma.shankar@intel.com,
	ankit.k.nautiyal@intel.com, chaitanya.kumar.borah@intel.com
Subject: [PATCH v7 06/10] drm/i915/vrr: Compute CMRR fractional timings generically
Date: Tue, 28 Jul 2026 20:29:38 +0530	[thread overview]
Message-ID: <20260728145943.3848704-7-mitulkumar.ajitkumar.golani@intel.com> (raw)
In-Reply-To: <20260728145943.3848704-1-mitulkumar.ajitkumar.golani@intel.com>

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 requirements. Note
that the CMRR enable path is wired up separately; this patch only lays
down the generic computation. Remove computation of mode_flags,
I915_MODE_FLAG_VRR, as CMRR is being moved to the fixed refresh rate
path. Also return early from the CMRR computation if the existing
computation is already sufficient.

--v2:
- Derive video_mode locally instead of caching it in the persistent
  struct intel_crtc state (Jani, Chaitanya)
- Fix numerator unit in comment: milli-Hz, not kHz (Chaitanya)
- Fix "requirement" typo and clarify that CMRR is not yet enabled in
  the commit message (Chaitanya)
- Fix precision issue while computing the M/N ratio (Chaitanya)
- Rename multiplier_m and multiplier_n to improve readability
  (Chaitanya)
- Compute vtotal as it is required for dithering as per the algorithm
  implementation (Chaitanya)
- Replace the misleading adjusted_pixel_rate with dividend, which is
  more descriptive

--v3:
- Mention the rationale for not computing I915_MODE_FLAG_VRR
  (Chaitanya)
- Remove the redundant return statement at the end of the CMRR
  computation (Chaitanya)
- Correct cmrr_n calculation (Chaitanya)
- Add an early return if the current computation is already sufficient
  to drive the mode without CMRR (Chaitanya)
- Round up requested_refresh_rate (Validation)
- Fix integer overflow

--v4:
- Update cmrr_n calculation to avoid aggressive dithering
- Use div64_u64 to truncate to the floor value instead of
  DIV_ROUND_UP_ULL, which rounds up to the ceiling value
- Add a TODO comment to the cmrr_n calculation
- Add a condition for custom refresh rate requests
- Update the CMRR computation comment block in compute_config

--v5:
- Simplify computation
- Correct typo in commit message (Chaitanya)
- Compute early return condition (Chaitanya)

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

diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
index 6d3333fd5281..7746b2ead7d1 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,86 @@ 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;
+	u32 mode_rate_mhz;
+	u32 requested_rate_mhz = crtc->force_cmrr.numerator;
+	int rr_multiplier = 1, rr_divider = 1;
 
-	/* 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 (!requested_rate_mhz)
+		return;
 
-	if (video_mode_required) {
-		multiplier_m = 1001;
-		multiplier_n = 1000;
+	/* Requested rate must match the mode's nominal (integer) refresh rate */
+	if (DIV_ROUND_CLOSEST(requested_rate_mhz, 1000) !=
+			drm_mode_vrefresh(adjusted_mode)) {
+		drm_dbg_kms(display->drm,
+			    "[CRTC:%d:%s] CMRR requested %u.%03u Hz doesn't match mode %d Hz\n",
+			    crtc->base.base.id, crtc->base.name,
+			    requested_rate_mhz / 1000, requested_rate_mhz % 1000,
+			    drm_mode_vrefresh(adjusted_mode));
+		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;
-}
+	/* Actual rate produced by the current timings, in milli-Hz */
+	mode_rate_mhz =
+		DIV_ROUND_CLOSEST_ULL((u64)adjusted_mode->crtc_clock * 1000 * 1000,
+				      adjusted_mode->crtc_vtotal *
+				      adjusted_mode->crtc_htotal);
 
-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;
+	if (crtc->force_cmrr.denominator == 1000) {
+		rr_multiplier = 1;
+		rr_divider = 1;
+
+		/* Mode already hits the exact rate -> CMRR not needed */
+		if (DIV_ROUND_CLOSEST(mode_rate_mhz, 10) ==
+		    DIV_ROUND_CLOSEST(requested_rate_mhz, 10)) {
+			drm_dbg_kms(display->drm,
+				    "[CRTC:%d:%s] %u.%03u Hz can be driven without CMRR\n",
+				    crtc->base.base.id, crtc->base.name,
+				    requested_rate_mhz / 1000, requested_rate_mhz % 1000);
+			return;
+		}
+	} else {
+		rr_multiplier = 1000;
+		rr_divider = 1001;
+	}
 
-	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 = (requested_rate_mhz x htotal x rr_multiplier) / 1000
+	 * cmrr_m = (pixel_clock_hz x rr_divider) % cmrr_n
+	 *
+	 * where requested_rate_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)requested_rate_mhz *
+			  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 +450,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 +483,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 +1160,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;
 
-- 
2.48.1


  parent reply	other threads:[~2026-07-28 15:11 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-28 14:59 [PATCH v7 00/10] Enable CMRR in fixed-RR VRR path Mitul Golani
2026-07-28 14:59 ` [PATCH v7 01/10] drm/i915/vrr: Return from PSR2 compute config in case of CMRR enabled Mitul Golani
2026-07-28 15:23   ` [PATCH v8 " Mitul Golani
2026-07-30  8:43     ` Borah, Chaitanya Kumar
2026-07-28 14:59 ` [PATCH v7 02/10] drm/i915/vrr: Restrict CMRR enable condition to VRR-TG-default platforms Mitul Golani
2026-07-28 14:59 ` [PATCH v7 03/10] drm/i915/vrr: Add per-CRTC vrr/cmrr debugfs control Mitul Golani
2026-07-28 14:59 ` [PATCH v7 04/10] drm/i915/vrr: Update AS_SDP target_rr_divider based on CMRR config request Mitul Golani
2026-07-28 14:59 ` [PATCH v7 05/10] drm/i915/display: Move CMRR crtc_state members under VRR Mitul Golani
2026-07-30  8:44   ` Borah, Chaitanya Kumar
2026-07-28 14:59 ` Mitul Golani [this message]
2026-07-30  8:45   ` [PATCH v7 06/10] drm/i915/vrr: Compute CMRR fractional timings generically Borah, Chaitanya Kumar
2026-07-28 14:59 ` [PATCH v7 07/10] drm/i915/vrr: Latch CMRR ratio via fastset on debugfs write Mitul Golani
2026-07-30  8:48   ` Borah, Chaitanya Kumar
2026-07-28 14:59 ` [PATCH v7 08/10] drm/i915/vrr: Program CMRR enable/disable from transcoder timings Mitul Golani
2026-07-30  8:55   ` Borah, Chaitanya Kumar
2026-07-28 14:59 ` [PATCH v7 09/10] drm/i915/vrr: Dump CMRR state in the crtc state dump Mitul Golani
2026-07-28 14:59 ` [PATCH v7 10/10] drm/i915/vrr: Enable cmrr Mitul Golani
2026-07-30  8:55   ` Borah, Chaitanya Kumar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260728145943.3848704-7-mitulkumar.ajitkumar.golani@intel.com \
    --to=mitulkumar.ajitkumar.golani@intel.com \
    --cc=ankit.k.nautiyal@intel.com \
    --cc=chaitanya.kumar.borah@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=uma.shankar@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox