Igt-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
To: igt-dev@lists.freedesktop.org
Cc: uma.shankar@intel.com, ramanaidu.naladala@intel.com,
	mitulkumar.ajitkumar.golani@intel.com,
	ankit.k.nautiyal@intel.com
Subject: [PATCH v2 1/2] tests/kms_vrr: Bucketize refresh rate tolerance
Date: Fri, 28 Mar 2025 10:39:57 +0530	[thread overview]
Message-ID: <20250328050959.659508-2-mitulkumar.ajitkumar.golani@intel.com> (raw)
In-Reply-To: <20250328050959.659508-1-mitulkumar.ajitkumar.golani@intel.com>

Reduce false failures while preserving timing accuracy. Introduce
a small tolerance buffer based on refresh rate which accounts for
HW/SW latency without compromising validation on HRR panel.

--v2:
- correct refresh rate criteria.

Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
---
 tests/kms_vrr.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 47 insertions(+), 3 deletions(-)

diff --git a/tests/kms_vrr.c b/tests/kms_vrr.c
index c4bb30f6a..c7be37ff8 100644
--- a/tests/kms_vrr.c
+++ b/tests/kms_vrr.c
@@ -410,6 +410,52 @@ do_flip(data_t *data, igt_fb_t *fb)
 	igt_reset_timeout();
 }
 
+static void
+calculate_tolerance(uint64_t *threshold_hi, uint64_t *threshold_lo, uint64_t rates_ns)
+{
+	uint32_t refresh_rate = NSECS_PER_SEC/rates_ns;
+
+	if (refresh_rate < 0)
+		return;
+
+	/*
+	 * Current IGT implementation follows this sequence:
+	 * 1. Perform a page flip (`do_flip`).
+	 * 2. Wait for the flip completion event.
+	 * 3. Compare the timestamp of the flip completion event with the previous frame’s completion timestamp.
+	 * 4. Adjust CPU cycle burning based on the relative frame time.
+	 *
+	 * If a flip completes too early or too late, it is marked as out of tolerance.
+	 * As a result, additional CPU cycles are burned to match the `target_ns`.
+	 * Even if the next frame is on time, the total frame time now includes:
+	 * Burned CPU cycle time (from the previous frame) + Flip completion event time.
+	 * This leads to miscalculation, causing **false out-of-range detections**.
+	 * The impact is more significant on High Refresh Rate (HRR) panels, where: The allowed tolerance
+	 * window is smaller and more correction time is required. i.e. for 210hz (4.762ms), allowed range is
+	 * 209hz(4.784ms) to 211hz(4.739ms). This comes just 23 microsecond tolerance, which is much lesser
+	 * for accounting HW/SW latency, CPU burn cycle latency and correction logic applied in igt for
+	 * validation.
+	 *
+	 * To address this implement a Bucketing Strategy:
+	 * Provide a small tolerance buffer to allow IGT tests to account for correction. based on range of
+	 * asked refresh rate. This prevents excessive failures due to minor timing adjustments.
+	 * Although an imperical number but already IGT is living with that.
+	 * This also ensures that asked refresh rate is not too off and always catch the real HW/software 
+	 * latencies.
+	 */
+
+	if (refresh_rate <= 120) {
+		*threshold_hi = NSECS_PER_SEC / (((float)NSECS_PER_SEC / rates_ns) + 1);
+		*threshold_lo = NSECS_PER_SEC / (((float)NSECS_PER_SEC / rates_ns) - 1);
+	} else if (refresh_rate >= 120 && refresh_rate <= 240) {
+		*threshold_hi = NSECS_PER_SEC / (((float)NSECS_PER_SEC / rates_ns) + 5);
+		*threshold_lo = NSECS_PER_SEC / (((float)NSECS_PER_SEC / rates_ns) - 5);
+	} else {
+		*threshold_hi = NSECS_PER_SEC / (((float)NSECS_PER_SEC / rates_ns) + 10);
+		*threshold_lo = NSECS_PER_SEC / (((float)NSECS_PER_SEC / rates_ns) - 10);
+	}
+}
+
 /*
  * Flips at the given rate and measures against the expected value.
  * Returns the pass rate as a percentage from 0 - 100.
@@ -439,9 +485,7 @@ flip_and_measure(data_t *data, igt_output_t *output, enum pipe pipe,
 		else
 			exp_rate_ns = vtest_ns.max;
 
-		/* Allow ~1 Hz deviation for different reasons causing delay. */
-		threshold_hi[i] = NSECS_PER_SEC / (((float)NSECS_PER_SEC / exp_rate_ns) + 1);
-		threshold_lo[i] = NSECS_PER_SEC / (((float)NSECS_PER_SEC / exp_rate_ns) - 1);
+		calculate_tolerance(&threshold_hi[i], &threshold_lo[i], exp_rate_ns);
 
 		igt_info("Requested rate[%d]: %"PRIu64" ns, Expected rate between: %"PRIu64" ns to %"PRIu64" ns\n",
 				i, rates_ns[i], threshold_hi[i], threshold_lo[i]);
-- 
2.48.1


  reply	other threads:[~2025-03-28  5:12 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-28  5:09 [PATCH v2 0/2] Bucketize refresh rate tolerance Mitul Golani
2025-03-28  5:09 ` Mitul Golani [this message]
2025-03-28  6:20   ` [PATCH v2 1/2] tests/kms_vrr: " Shankar, Uma
2025-03-28 10:40   ` Naladala, Ramanaidu
2025-03-28  5:09 ` [PATCH v2 2/2] HAX patch do not merge Mitul Golani
2025-03-28  6:22 ` ✓ Xe.CI.BAT: success for Bucketize refresh rate tolerance (rev2) Patchwork
2025-03-28  6:57 ` ✗ i915.CI.BAT: failure " Patchwork
2025-03-28 18:04 ` ✗ Xe.CI.Full: " Patchwork
2025-04-06 18:00 ` Patchwork

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=20250328050959.659508-2-mitulkumar.ajitkumar.golani@intel.com \
    --to=mitulkumar.ajitkumar.golani@intel.com \
    --cc=ankit.k.nautiyal@intel.com \
    --cc=igt-dev@lists.freedesktop.org \
    --cc=ramanaidu.naladala@intel.com \
    --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