Re: [igt-dev] [PATCH i-g-t] prime_vgem: Downgrade the severity of a single missed vblank on flipping

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Apr 04, 2019 at 08:05:37AM +0100, Chris Wilson wrote:
> Not displaying the flip on the next vblank is bad, but not the end of
> the world -- so long as that is only a temporary glitch. Give the vblank
> a few more frames to complete, and warn instead of failing if it takes
> more than one vblank interval to flip.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  tests/prime_vgem.c | 22 +++++++++++++++-------
>  1 file changed, 15 insertions(+), 7 deletions(-)
> 
> diff --git a/tests/prime_vgem.c b/tests/prime_vgem.c
> index 85a27b69f..8c0dd6c7a 100644
> --- a/tests/prime_vgem.c
> +++ b/tests/prime_vgem.c
> @@ -713,15 +713,23 @@ static void flip_to_vgem(int i915, int vgem,
>  
>  	/* And then the flip is completed as soon as it is ready */
>  	if (!hang) {
> -		union drm_wait_vblank wait;
> +		unsigned long miss;
>  
> -		memset(&wait, 0, sizeof(wait));
> -		wait.request.type = DRM_VBLANK_RELATIVE | pipe_select(0);
> -		wait.request.sequence = 10;
> -		do_or_die(drmIoctl(i915, DRM_IOCTL_WAIT_VBLANK, &wait));
> -
> -		vgem_fence_signal(vgem, fence);
> +		/* Signal fence at the start of then next vblank */
>  		get_vblank(i915, 0, DRM_VBLANK_NEXTONMISS);
> +		vgem_fence_signal(vgem, fence);
> +
> +		miss = 0;
> +		igt_until_timeout(5) {
> +			get_vblank(i915, 0, DRM_VBLANK_NEXTONMISS);
> +			if (poll(&pfd, 1, 0))
> +				break;
> +			miss++;
> +		}
> +		if (miss) {

I think miss > 1 would be correct here, since once we signal the kernel
should be able to realize that, queue the flip and execute it within 1
vblank. 16ms delay would be rather bad.

Otoh we can't expect that it'll always issue it right away, because
there's some worker and code involved plus other delays, so no matter
wether we sample the vblank counter before or after we signal the fence,
we might miss the very next vblank. Warning about a race we expect to
occasionally happen and can't prevent doesn't sound good.

With the miss > 1:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Also, is there some CI bugzilla for this? Would be good to reference that
one.
-Daniel

> +			igt_warn("Missed %lu vblanks after signaling before flip was completed\n",
> +				 miss);
> +		}
>  		igt_assert_eq(poll(&pfd, 1, 0), 1);
>  	}
>  
> -- 
> 2.20.1
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev