[PATCH v1] tests/kms_flip: Assert with twice of frame time while waiting fro vblank

[Golani, Mitulkumar Ajitkumar]

> -----Original Message-----
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Sent: Friday, October 11, 2024 3:26 PM
> To: Golani, Mitulkumar Ajitkumar <mitulkumar.ajitkumar.golani at intel.com>
> Cc: igt-dev at lists.freedesktop.org; Modem, Bhanuprakash
> <bhanuprakash.modem at intel.com>; Nautiyal, Ankit K
> <ankit.k.nautiyal at intel.com>
> Subject: Re: [PATCH v1] tests/kms_flip: Assert with twice of frame time while
> waiting fro vblank
> 
> On Fri, Oct 11, 2024 at 11:51:46AM +0530, Mitul Golani wrote:
> > Instead of hardcoding to 500us, wait for twice of mode frame time
> > while waiting for vblank. Motivation of this change is to remove hard
> > coding of 500us, which comes out to be more stringent and ambiguous vs
> > changing modeline.
> >
> > Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani at intel.com>
> > ---
> >  tests/kms_flip.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tests/kms_flip.c b/tests/kms_flip.c index
> > cbabbe74f..decffa961 100755
> > --- a/tests/kms_flip.c
> > +++ b/tests/kms_flip.c
> > @@ -921,7 +921,7 @@ static bool run_test_step(struct test_output *o,
> unsigned int *events)
> >  					    0, &reply));
> >  		end = gettime_us();
> >  		igt_debug("Vblank took %luus\n", end - start);
> > -		igt_assert(end - start < 500);
> > +		igt_assert(end - start < 2 * mode_frame_time(o));
> 
> The test is literally called TEST_VBLANK_EXPIRED_SEQ, which tells us that it's
> expecting the target vblank seq to have expired already, and thus the response
> should be "immediate". It doesn't matter at all what kind of mode is in use.
> 

Hello Ville,

Thank for pointing this out, you are right I mistakenly overlooked the criteria "!(o->pending_events & EVENT_VBLANK)"

In certain test failure cases, the assertion indicates that the vblank exceeded 500 usec. However, based on the drmWaitVblank event logs, the vblank interrupt occurs within the expected period of 16.666ms for a 60Hz panel. This suggests that the vblank is not actually delayed from driver. Do you recommend any changes in igt to address such delays?

Regards,
Mitul

> >  		if (reply.sequence != exp_seq) {
> >  			igt_debug("unexpected vblank seq %u, should be
> %u\n",
> >  				  reply.sequence, exp_seq);
> > --
> > 2.46.0
> 
> --
> Ville Syrjälä
> Intel