[igt-dev] [PATCH i-g-t v2] tests/kms_cursor_legacy: Wait for an extra vblank

[Kahola, Mika]

> -----Original Message-----
> From: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com>
> Sent: Thursday, April 16, 2020 12:57 PM
> To: Kahola, Mika <mika.kahola at intel.com>; igt-dev at lists.freedesktop.org
> Subject: Re: [PATCH i-g-t v2] tests/kms_cursor_legacy: Wait for an extra vblank
> 
> On 16.4.2020 9.10, Mika Kahola wrote:
> > kms_cursor_legacy IGT subtest 2x-nonblocking-modeset-vs-cursor-atomic
> > is failing due to busyness while trying to do atomic commit. In case,
> > we are busy, let's just wait one extra vblank before continuing the
> > test.
> >
> > References: https://gitlab.freedesktop.org/drm/intel/issues/1062
> >
> > v2: Wait out for max 5 seconds for commit busyness (Juha-Pekka)
> >
> > Signed-off-by: Mika Kahola <mika.kahola at intel.com>
> > ---
> >   tests/kms_cursor_legacy.c | 6 +++++-
> >   1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/tests/kms_cursor_legacy.c b/tests/kms_cursor_legacy.c
> > index d5f95b8d..6467e646 100644
> > --- a/tests/kms_cursor_legacy.c
> > +++ b/tests/kms_cursor_legacy.c
> > @@ -927,7 +927,11 @@ static void
> > two_screens_flip_vs_cursor(igt_display_t *display, int nloops, bool
> >
> >   		if (ret == -EBUSY) {
> >   			/* Force completion on both pipes, and generate event.
> */
> > -			igt_display_commit_atomic(display, flags, NULL);
> > +			while (ret == -EBUSY) {
> > +				igt_set_timeout(5, "stuck with -EBUSY");
> 
> Timeout should be set outside while loop.
Right, otherwise we just keep resetting the timeout on each iteration.

> 
> > +				igt_wait_for_vblank(display->drm_fd, pipe);
> 
> I don't think vblank wait here does any good, that ebusy doesn't depend on
> vblank.
Actually, there is a similar functionality later in that function. It would be useful just use that routine here as well. I move this into a separate function, so code repetition is avoided.

> 
> > +				ret = igt_display_try_commit_atomic(display,
> flags, NULL);
> > +			}
> 
> You should call igt_reset_timeout() here. If not reaching next timeout block
> from here within this timeout this timer will fail the test.
> 
> >
> >   			while (nloops--) {
> >   				shared[1] = nloops & 1;
> >