On Mon, Aug 14, 2017 at 09:50:01PM +0100, Chris Wilson wrote:
> Quoting Daniel Vetter (2017-08-14 21:32:00)
> > The goal of these subtests was just to delay a kms operation a little
> > bit. The goal wasn't to
> > - spin until the operation was completed. That results in a gpu hang,
> > and gpu hangs need igt_hang for safety.
> > - complete it before the operation. That's just pointless.
> >
> > Fix it by using the timeout support.
> >
> > This was all broken in the initial conversation:
> >
> > commit 96ec8cb3b5ec1fc2927d6cff8e09930082301d7e
> > Author: Abdiel Janulgue <abdiel.janulgue@xxxxxxxxxxxxxxx>
> > Date: Sat Oct 29 01:01:05 2016 +0300
> >
> > igt/kms_flip: Use new igt_spin_batch
> >
> > Note that part of the damage was removed already in
>
> Ah no, that patch removed the timeouts! I remember there being
> set_timeout in there and was baffled to see them gone.
>
> Indeed, the breakage is
>
> > commit b0081ea9cb7d39234fd0fcc64dd56ed4f5d50b05
> > Author: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> > Date: Wed Aug 9 11:11:52 2017 +0200
> >
> > tests/kms_flip: Remove $engine-flip-vs-dpms/modeset
>
>
> due to
>
> diff --git a/tests/kms_flip.c b/tests/kms_flip.c
> index ede5fd2b..ea860e19 100644
> --- a/tests/kms_flip.c
> +++ b/tests/kms_flip.c
> @@ -755,13 +755,8 @@ static unsigned int run_test_step(struct test_output *o)
> if (o->flags & TEST_MODESET)
> igt_assert(set_mode(o, o->fb_ids[o->current_fb_id], 0, 0) == 0);
>
> - if (o->flags & TEST_DPMS) {
> - if (spin_rcs)
> - igt_spin_batch_set_timeout(spin_rcs, NSEC_PER_SEC);
> - if (spin_bcs)
> - igt_spin_batch_set_timeout(spin_bcs, NSEC_PER_SEC);
> + if (o->flags & TEST_DPMS)
This doesn't fix the case where TEST_DPMS is not set. I seen hangs
everywhere, because we only stop the spinning _after_ the operation is
completed. The only way for the operation to complete is by a gpu reset,
and resetting a live spinner on snb blt is not a good idea (it kills the
box eventually).
And yes there's a a race with just setting a timeout, but we have other
checks that make sure an atomic flip doesn't take one full second. So I
don't think the race is relevant in practice for testing (because it would
indicate a broken kernel, and we catch those bugs already).
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
