[igt-dev] [PATCH i-g-t] lib/i915: Split igt_require_gem() into i915/

[Chris Wilson]

Quoting Petri Latvala (2020-05-07 11:07:33)
> On Thu, May 07, 2020 at 11:04:55AM +0100, Chris Wilson wrote:
> > Quoting Petri Latvala (2020-05-07 10:53:34)
> > > On Thu, May 07, 2020 at 10:45:19AM +0100, Chris Wilson wrote:
> > > > Quoting Petri Latvala (2020-05-07 10:40:35)
> > > > > On Thu, May 07, 2020 at 09:45:50AM +0100, Chris Wilson wrote:
> > > > > > +void igt_require_gem(int i915)
> > > > > > +{
> > > > > > +     int err;
> > > > > > +
> > > > > > +     igt_require_intel(i915);
> > > > > > +
> > > > > > +     /*
> > > > > > +      * We only want to use the throttle-ioctl for its -EIO reporting
> > > > > > +      * of a wedged device, not for actually waiting on outstanding
> > > > > > +      * requests! So create a new drm_file for the device that is clean.
> > > > > > +      */
> > > > > > +     i915 = gem_reopen_driver(i915);
> > > > > > +
> > > > > > +     /*
> > > > > > +      * Reset the global seqno at the start of each test. This ensures that
> > > > > > +      * the test will not wrap unless it explicitly sets up seqno wrapping
> > > > > > +      * itself, which avoids accidentally hanging when setting up long
> > > > > > +      * sequences of batches.
> > > > > > +      */
> > > > > > +     reset_device(i915);
> > > > > > +
> > > > > > +     err = 0;
> > > > > > +     if (ioctl(i915, DRM_IOCTL_I915_GEM_THROTTLE)) {
> > > > > > +             err = -errno;
> > > > > > +             igt_assume(err);
> > > > > > +     }
> > > > > > +
> > > > > > +     close(i915);
> > > > > > +
> > > > > > +     igt_require_f(err == 0, "Unresponsive i915/GEM device\n");
> > > > > 
> > > > > 
> > > > > Food for thought (not for this patch): Using igt_abort_f() for this instead?
> > > > 
> > > > Not everything requires gem? But yeah, it's pretty common to have a pile
> > > > of skips after toasting the GPU.
> > > 
> > > Generally if the GPU is toast there's really no point in testing
> > > further, even if they don't require gem. I'd rather have that in CI
> > > than flip-flopping skips. Validation is a different matter.
> > > 
> > > Martin can probably give a good opinion on that.
> > 
> > Who gets the blame for igt_abort_f? We want it like igt at runner@aborted
> > that it says couldn't continue after blah.
> 
> The test that called it.
> 
> Now, that puts the blame on the test that didn't cause the state, but
> so does igt_require.

igt_require is blameless. Filing bugs for it is just a waste of
everybody's time.
-Chris