[igt-dev] [Intel-gfx] [PATCH i-g-t] i915/gem_workarounds: Require GPU resets

[Chris Wilson]

Quoting Chris Wilson (2019-01-28 11:07:40)
> Quoting Tvrtko Ursulin (2019-01-28 11:03:30)
> > 
> > On 27/01/2019 13:06, Chris Wilson wrote:
> > > Check that we are allowed to reset the GPU prior to execution.
> > > 
> > > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > > ---
> > >   tests/i915/gem_workarounds.c | 6 +++++-
> > >   1 file changed, 5 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/tests/i915/gem_workarounds.c b/tests/i915/gem_workarounds.c
> > > index 78478ad2c..0e9ab2386 100644
> > > --- a/tests/i915/gem_workarounds.c
> > > +++ b/tests/i915/gem_workarounds.c
> > > @@ -192,7 +192,11 @@ static void check_workarounds(int fd, enum operation op, unsigned int flags)
> > >   
> > >       switch (op) {
> > >       case GPU_RESET:
> > > -             igt_force_gpu_reset(fd);
> > > +             {
> > > +                     igt_hang_t hang = igt_allow_hang(fd, ctx, 0);
> > > +                     igt_force_gpu_reset(fd);
> > > +                     igt_disallow_hang(fd, hang);
> > > +             }
> > >               break;
> > >   
> > >       case SUSPEND_RESUME:
> > > 
> > 
> > If it doesn't make sense to add the checks into igt_force_gpu_reset (so 
> > force means force), should we have igt_try_gpu_reset to avoid having to 
> > wrap it everywhere?
> 
> Ugh. igt_try_gpu_reset_with_many_many_options_depending_on_test().
> 
> I like my requires as high up in the chain as possible, I've been bitten
> too many times by hiding them.

The key benefit from doing it higher up, is that we should be doing it
as early as possible and should be more descriptive about why.
-Chris