Re: [PATCH i-g-t v2] tests/kms_setmode: Relax vblank timing tolerance from 1 scanline to 1%

["Jason-JH Lin (林睿祥)" <Jason-JH.Lin@mediatek.com>]

On Thu, 2026-04-16 at 18:29 +0300, Ville Syrjälä wrote:
> On Thu, Apr 16, 2026 at 06:00:58PM +0300, Ville Syrjälä wrote:
> > On Thu, Apr 16, 2026 at 10:45:28PM +0800, Jason-JH Lin wrote:
> > > The original check_timings() validation required vblank intervals
> > > to be
> > > within 1 scanline (~15us for 1080p60) which is overly strict and
> > > causes
> > > false failures due to userspace measurement overhead.
> > > 
> > > Changes:
> > > - Keep original strict checks (1 scanline, 1.718 sigma) as
> > > warnings only
> > > - Add relaxed validation with 1% relative tolerance for pass/fail
> > > - Use 3 sigma threshold (99.7% confidence) instead of 1.718 sigma
> > > (90%)
> > > 
> > > Rationale:
> > > Hardware specs (VESA/HDMI/DP) allow ±0.5% pixel clock tolerance,
> > > which
> > > translates to ±0.5% frame time due to inverse relationship.
> > > However,
> > > userspace measurement via read() syscall adds ~0.3-0.5% overhead
> > > from:
> > 
> > read() overhead is irrelevant here. This is measuring the accuracy
> > of the kernel generated vblank event timestamps. If you are having
> > problems with this then that would indicate your kernel driver
> > isn't
> > generating particularly good timestamps, or it's using a dotclock
> > that
> > doesn't quite match what userspace specified originally.
> > 
> > > - Interrupt latency and scheduling delays (5-50 us)
> > > - System call overhead (1-5 us)
> > > - Variable delays depending on system load
> > > 
> > > Total tolerance: ±0.5% (hardware) + ~0.5% (software) = ±1.0%
> > > 
> > > This makes the test ~11x more tolerant while still detecting real
> > > timing
> > > issues and aligning with industry hardware specifications.
> > > 
> > > Signed-off-by: Jason-JH Lin <jason-jh.lin@mediatek.com>
> > > Suggested-by: Jeevain B <jeevan.b@intel.com>
> > > ---
> > >  tests/kms_setmode.c | 71 +++++++++++++++++++++++++++++++++++++--
> > > ------
> > >  1 file changed, 59 insertions(+), 12 deletions(-)
> > > 
> > > diff --git a/tests/kms_setmode.c b/tests/kms_setmode.c
> > > index 1f2849bc26cf..b452f8a9e997 100644
> > > --- a/tests/kms_setmode.c
> > > +++ b/tests/kms_setmode.c
> > > @@ -495,6 +495,7 @@ static bool check_timings(int crtc_idx, const
> > > drmModeModeInfo *kmode)
> > >  	double mean;
> > >  	double stddev;
> > >  	int n;
> > > +	bool strict_failed = false;
> > >  
> > >  	memset(&wait, 0, sizeof(wait));
> > >  	wait.request.type = kmstest_get_vbl_flag(crtc_idx);
> > > @@ -563,10 +564,9 @@ static bool check_timings(int crtc_idx,
> > > const drmModeModeInfo *kmode)
> > >  
> > >  	/* 99.7% samples within one scanline on each side of
> > > mean */
> > >  	if (accuracy >= line_time(kmode)) {
> > > -		igt_info("vblank accuracy (%.3fus, %.1f%%) worse
> > > than a scanline (%.3fus)\n",
> > > -			  accuracy, 100 * accuracy / mean,
> > > line_time(kmode));
> > > -
> > > -		return false;
> > > +		igt_warn("vblank accuracy (%.3fus, %.1f%%) worse
> > > than a scanline (%.3fus)\n",
> > > +			 accuracy, 100 * accuracy / mean,
> > > line_time(kmode));
> > > +		strict_failed = true;
> 
> Failing this would indicate the driver isn't generating particularly
> good timestamps.
> 
> > >  	}
> > >  
> > >  	/* At least 90% of frame times fall within the one
> > > scanline on each
> > > @@ -591,14 +591,61 @@ static bool check_timings(int crtc_idx,
> > > const drmModeModeInfo *kmode)
> > >  	 *
> > > https://urldefense.com/v3/__https://en.wikipedia.org/wiki/Standard_deviation*Rules_for_normally_distributed_data__;Iw!!CTRNKA9wMg0ARbw!mn4DeRxVpp3Y-PmcRLBfMYD4u4qjPzmx5JvBQ1PYYCBiTI_ygLNR5s_T01CTM6Ti-vnDSzzPxB509bsBcxgFBERpcqqRgSO5QA$
> > >  
> > >  	 */
> > >  	if (fabs(mean - expected) >= max(line_time(kmode), 1.718
> > > * stddev)) {
> 
> And assuming the timestamps were good, failing this would indicate
> the
> actual timings don't quite match what userspace specified (eg.
> dotclock
> could be a bit off). I suppose we should allow that VESA specified 
> .5% error here.

Thank you for the review.

You are right that read() overhead is irrelevant here since
check_timings() is measuring kernel generated vblank event timestamps.
I will remove that part of the rationale.

Regarding the two checks:

For Check 1 (timestamp accuracy >= line_time): You pointed out that
failing this indicates the kernel driver isn't generating good
timestamps. We will investigate and fix MTK's vblank timestamp
implementation at the kernel driver level  rather than relaxing
the test.

For Check 2 (mean deviation from expected): I will update the patch
to only relax this check to allow the VESA specified ±0.5% pixel clock
tolerance, and remove the read() overhead rationale from the commit
message.

Regards,
Jason-JH Lin