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

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

[Chris Wilson]

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) {
+			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

[igt-dev] ✓ Fi.CI.BAT: success for prime_vgem: Downgrade the severity of a single missed vblank on flipping

[Patchwork]

== Series Details ==

Series: prime_vgem: Downgrade the severity of a single missed vblank on flipping
URL   : https://patchwork.freedesktop.org/series/58977/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5869 -> IGTPW_2783
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/58977/revisions/1/mbox/

Known issues
------------

  Here are the changes found in IGTPW_2783 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_basic@gtt-bsd2:
    - fi-byt-clapper:     NOTRUN -> SKIP [fdo#109271] +57

  * igt@gem_exec_suspend@basic-s3:
    - fi-blb-e6850:       PASS -> INCOMPLETE [fdo#107718]

  * igt@kms_busy@basic-flip-c:
    - fi-byt-clapper:     NOTRUN -> SKIP [fdo#109271] / [fdo#109278]

  * igt@runner@aborted:
    - fi-bxt-dsi:         NOTRUN -> FAIL [fdo#109516]

  
#### Possible fixes ####

  * igt@prime_vgem@basic-fence-flip:
    - fi-ilk-650:         FAIL [fdo#104008] -> PASS

  
  [fdo#104008]: https://bugs.freedesktop.org/show_bug.cgi?id=104008
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109516]: https://bugs.freedesktop.org/show_bug.cgi?id=109516


Participating hosts (47 -> 42)
------------------------------

  Additional (2): fi-bxt-dsi fi-byt-clapper 
  Missing    (7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-bdw-samus fi-skl-6600u 


Build changes
-------------

    * IGT: IGT_4926 -> IGTPW_2783

  CI_DRM_5869: 03f8f3298b90c7f80da6a98c3eb8413d7aeaa52b @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2783: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2783/
  IGT_4926: c9a9cf357b6b2a304623790bf8dae797e12888a8 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2783/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

[Daniel Vetter]

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

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

[Chris Wilson]

Quoting Daniel Vetter (2019-04-04 09:17:06)
> 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.

Surely missing a vblank on an idle system with a full 16ms grace period
is worth a warning in CI? If we can miss under relaxed conditions, how
bad will it be under load? :)

> With the miss > 1:
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Ok, because I believe we should have other tests (like kms_flip) that
are just as sensitive to missed flips, and try to compare the timestamps
on the vblank vs walltime etc.

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

We were previously failing at the nanosleep, I do not recall seeing a
warning about the igt_assert(poll()) before, and a quick scan through
bugzilla says it hasn't been filed yet.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

[Daniel Vetter]

On Thu, Apr 4, 2019 at 10:40 AM Chris Wilson <chris@chris-wilson.co.uk> wrote:
>
> Quoting Daniel Vetter (2019-04-04 09:17:06)
> > 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.
>
> Surely missing a vblank on an idle system with a full 16ms grace period
> is worth a warning in CI? If we can miss under relaxed conditions, how
> bad will it be under load? :)

Hm, I misread the logic, coffee didn't kick in yet I guess. We wait
for the vblank first, then signal, that should indeed be enough on an
idle box. Somehow I mixed those up.

> > With the miss > 1:
> >
> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> Ok, because I believe we should have other tests (like kms_flip) that
> are just as sensitive to missed flips, and try to compare the timestamps
> on the vblank vs walltime etc.

Correcting to r-b: me as-is. Maybe add a comment that we wait for the
vblank to give the kernel almost a full frame to get things done,
which should be enough.
-Daniel

> > Also, is there some CI bugzilla for this? Would be good to reference that
> > one.
>
> We were previously failing at the nanosleep, I do not recall seeing a
> warning about the igt_assert(poll()) before, and a quick scan through
> bugzilla says it hasn't been filed yet.
> -Chris
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✗ Fi.CI.IGT: failure for prime_vgem: Downgrade the severity of a single missed vblank on flipping

[Patchwork]

== Series Details ==

Series: prime_vgem: Downgrade the severity of a single missed vblank on flipping
URL   : https://patchwork.freedesktop.org/series/58977/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_5869_full -> IGTPW_2783_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_2783_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_2783_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/58977/revisions/1/mbox/

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in IGTPW_2783_full:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_pm_rpm@cursor:
    - shard-glk:          PASS -> DMESG-WARN

  
Known issues
------------

  Here are the changes found in IGTPW_2783_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_eio@unwedge-stress:
    - shard-snb:          PASS -> FAIL [fdo#109661]

  * igt@gem_exec_schedule@preempt-other-chain-blt:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] +116

  * igt@kms_busy@extended-modeset-hang-newfb-render-a:
    - shard-kbl:          PASS -> DMESG-WARN [fdo#110222]

  * igt@kms_busy@extended-modeset-hang-newfb-render-f:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +14

  * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-a:
    - shard-snb:          NOTRUN -> DMESG-WARN [fdo#110222]

  * igt@kms_busy@extended-pageflip-hang-newfb-render-f:
    - shard-apl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-b:
    - shard-kbl:          NOTRUN -> DMESG-WARN [fdo#110222]

  * igt@kms_content_protection@legacy:
    - shard-kbl:          NOTRUN -> FAIL [fdo#108739] / [fdo#110321]

  * igt@kms_cursor_legacy@2x-long-flip-vs-cursor-atomic:
    - shard-glk:          PASS -> FAIL [fdo#104873]

  * igt@kms_frontbuffer_tracking@psr-2p-pri-indfb-multidraw:
    - shard-apl:          NOTRUN -> SKIP [fdo#109271] +13

  * igt@kms_frontbuffer_tracking@psr-2p-primscrn-shrfb-pgflip-blt:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] +8

  * igt@kms_frontbuffer_tracking@psr-2p-scndscrn-spr-indfb-onoff:
    - shard-glk:          NOTRUN -> SKIP [fdo#109271] +4

  * igt@kms_pipe_crc_basic@read-crc-pipe-d:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_setmode@basic:
    - shard-apl:          PASS -> FAIL [fdo#99912]

  * igt@kms_vblank@pipe-c-ts-continuation-dpms-suspend:
    - shard-apl:          PASS -> FAIL [fdo#104894] +2

  * igt@kms_vblank@pipe-c-ts-continuation-suspend:
    - shard-kbl:          PASS -> INCOMPLETE [fdo#103665]

  
#### Possible fixes ####

  * igt@gem_eio@in-flight-suspend:
    - shard-kbl:          INCOMPLETE [fdo#103665] -> PASS

  * igt@kms_busy@extended-pageflip-hang-newfb-render-a:
    - shard-glk:          DMESG-WARN [fdo#110222] -> PASS

  * igt@kms_flip@dpms-vs-vblank-race:
    - shard-glk:          FAIL [fdo#103060] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-pri-shrfb-draw-pwrite:
    - shard-hsw:          SKIP [fdo#109271] -> PASS

  * igt@kms_plane_alpha_blend@pipe-b-constant-alpha-max:
    - shard-glk:          FAIL [fdo#108145] -> PASS
    - shard-apl:          FAIL [fdo#108145] -> PASS
    - shard-kbl:          FAIL [fdo#108145] -> PASS

  * igt@kms_vblank@pipe-b-ts-continuation-dpms-suspend:
    - shard-kbl:          FAIL [fdo#104894] -> PASS

  * igt@kms_vblank@pipe-c-ts-continuation-modeset-hang:
    - shard-apl:          FAIL [fdo#104894] -> PASS +1

  
#### Warnings ####

  * igt@runner@aborted:
    - shard-glk:          FAIL [fdo#109373] / [k.org#202321] -> ( 2 FAIL ) [fdo#109373] / [k.org#202321]

  
  [fdo#103060]: https://bugs.freedesktop.org/show_bug.cgi?id=103060
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#104873]: https://bugs.freedesktop.org/show_bug.cgi?id=104873
  [fdo#104894]: https://bugs.freedesktop.org/show_bug.cgi?id=104894
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108739]: https://bugs.freedesktop.org/show_bug.cgi?id=108739
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109373]: https://bugs.freedesktop.org/show_bug.cgi?id=109373
  [fdo#109661]: https://bugs.freedesktop.org/show_bug.cgi?id=109661
  [fdo#110222]: https://bugs.freedesktop.org/show_bug.cgi?id=110222
  [fdo#110321]: https://bugs.freedesktop.org/show_bug.cgi?id=110321
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912
  [k.org#202321]: https://bugzilla.kernel.org/show_bug.cgi?id=202321


Participating hosts (10 -> 5)
------------------------------

  Missing    (5): shard-skl pig-hsw-4770r pig-glk-j5005 shard-iclb pig-skl-6260u 


Build changes
-------------

    * IGT: IGT_4926 -> IGTPW_2783
    * Piglit: piglit_4509 -> None

  CI_DRM_5869: 03f8f3298b90c7f80da6a98c3eb8413d7aeaa52b @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2783: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2783/
  IGT_4926: c9a9cf357b6b2a304623790bf8dae797e12888a8 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2783/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev