Re: [PATCH v2 1/2] tests/kms_vrr: Bucketize refresh rate tolerance

["Naladala, Ramanaidu" <Ramanaidu.naladala@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 4820 bytes --]

Hi Mitul,

On 3/28/2025 10:39 AM, Mitul Golani wrote:
> 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) {
remove = in this if condition 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);
> +	}
> +}
Patch has style problems please check by running checkpatch.pl.

imho, Better to add one more bucket for refresh rate between 60 to 120 
to +/-2 deviation range.

from my analysis below observed,

from (60 -140) flips are in deviation range of 2, from (140 -220) flips 
are in deviation range of 5 and after 220 flips are in deviation range 
of 10.

			Number of flips on threshold
SNO 	RR 	Total flips 	1 	2 	5 	10
1 	62 	311 	307(98.7%) 			
2 	94 	471 	291(61.78%) 	458(97.24%) 		
3 	126 	631 	447(70.84%) 	587(93.02%) 		
4 	158 	791 	257(32.50%) 	521(65.86%) 	788(99.62%) 	
5 	190 	951 		504(52.99%) 	907(95.37%) 	
6 	222 	1111 			802(72.18%) 	1083(97.48%)
7 	254 	1271 			1022(80.40%) 	1249(98.26%)
8 	286 	1431 			675(47.16%) 	1160(81.06%)
9 	318 	1591 			892(56.06%) 	1393(87.56%)
10 	350 	1751 			739(42.18%) 	1591(90.81%)




> +
>   /*
>    * 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]);

[-- Attachment #2: Type: text/html, Size: 13387 bytes --]