[Intel-gfx] [PATCH i-g-t] tests/gem_eio: Skip in-flight-suspend on snb

[Lofstedt, Marta]

> -----Original Message-----
> From: Daniel Vetter [mailto:daniel.vetter at ffwll.ch] On Behalf Of Daniel
> Vetter
> Sent: Thursday, October 19, 2017 12:57 PM
> To: Intel Graphics Development <intel-gfx at lists.freedesktop.org>
> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>; Joonas Lahtinen
> <joonas.lahtinen at linux.intel.com>; Chris Wilson <chris at chris-wilson.co.uk>;
> Lofstedt, Marta <marta.lofstedt at intel.com>; Martin Peres
> <martin.peres at linux.intel.com>
> Subject: Re: [PATCH i-g-t] tests/gem_eio: Skip in-flight-suspend on snb
> 
> On Thu, Oct 19, 2017 at 11:51:51AM +0200, Daniel Vetter wrote:
> > CI gets upset about it resulting in an incomplete, let's skip it until
> > that's fixed to avoid havoc in the CI farm. Of course this should/will
> > be reverted as soon as we have a fix (similar to how we dealt with the
> > snb-dies-in-blt-hangs issue).
> >
> > Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> > Cc: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: "Lofstedt, Marta" <marta.lofstedt at intel.com>
> > Cc: Martin Peres <martin.peres at linux.intel.com>
> > References:
> > https://intel-gfx-ci.01.org/tree/drm-tip/igt@gem_eio@in-flight-suspend
> > .html
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=103289
> > Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> 
> For more context, since I forgot to add: I'm definitely not advertising for
> abusing igt_skip to handle problematic testcases in general. What makes this
> special here is the combo of
> - new testcase
> - old machine where we don't have priority to fix things
> 
> Hence why I think it'll make sense to treat this as a feature-like thing, where
> we simply skip if stuff doesn't work/isn't exposed on older platforms and
> shrug it off. And once someone does a free time project to fix it up, we can
> then remove the skip.
> 
> I hope that explains a bit the reasoning from my behind using skip here.

I am not buying this. 
Could you define which old machines that we are not going to care about to find out that we are having this real issue?
I also don't understand why new test-cases should be treated differently compare to the old bad behaving ones we already have.

/Marta
> 
> The other bit is that if/once Maarten figured out what's wrong with
> watermarks, we should be able to enable shard-snb reporting in CI results,
> which would be really great. Except we really can't have tests that
> incomplete, because they victimize too much else and so would need to
> blacklist until fixed one way or the other anyways.
> -Daniel
> 
> > ---
> >  tests/gem_eio.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/tests/gem_eio.c b/tests/gem_eio.c index
> > 899cb62728e3..28375e208232 100644
> > --- a/tests/gem_eio.c
> > +++ b/tests/gem_eio.c
> > @@ -218,6 +218,9 @@ static void test_inflight_suspend(int fd)
> >  	igt_require(gem_has_exec_fence(fd));
> >  	igt_require(i915_reset_control(false));
> >
> > +	igt_skip_on_f(IS_SANDYBRIDGE(intel_get_drm_devid(fd)),
> > +		      "random incompletes in CI with this test\n");
> > +
> >  	memset(obj, 0, sizeof(obj));
> >  	obj[0].flags = EXEC_OBJECT_WRITE;
> >  	obj[1].handle = gem_create(fd, 4096);
> > --
> > 2.15.0.rc1
> >
> 
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch