[Intel-gfx] [PATCH igt 10/10] igt/gem_eio: i915.reset is no longer a boolean

[Michał Winiarski]

On Fri, Jul 28, 2017 at 02:50:30PM +0100, Chris Wilson wrote:
> Quoting Michał Winiarski (2017-07-28 14:36:46)
> > On Fri, Jul 28, 2017 at 01:08:08PM +0100, Chris Wilson wrote:
> > > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > > ---
> > >  tests/gem_eio.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/tests/gem_eio.c b/tests/gem_eio.c
> > > index 3c826626..15120842 100644
> > > --- a/tests/gem_eio.c
> > > +++ b/tests/gem_eio.c
> > > @@ -53,7 +53,7 @@ static bool i915_reset_control(bool enable)
> > >       fd = open(path, O_RDWR);
> > >       igt_require(fd >= 0);
> > >  
> > > -     ret = write(fd, &"NY"[enable], 1) == 1;
> > > +     ret = write(fd, &"01"[enable], 1) == 1;
> > 
> > That's a fun way of doing "itoa()" :) 
> > How about using helpers? We have igt_sysfs_set_parameter.
> > 
> > Other thing is the fact that we're leaving the machine in a non-default state
> > (global reset rather than per-engine reset).
> 
> If you haven't noticed, igt already does exist in a reset=[01] universe.
> We are aaiting on the per-engine reset tests to actually enable
> i915.reset=2. For coverage we need both, even i915.reset=2 may fallback
> to i915.reset=1 in some unlikely situations.
> 
> > We should actually restore the old value after the test.
> > I can also see that's "by design" (hang detector), but if we change the
> > behaviour of hang detector in the future, we may forget to modify gem_eio.
> 
> That was the flavour of irc conversation yesterday. Instead of using
> module parameters, we create a debugfs interface. The parameter will
> only be set/adjusted while the debugfs fd was held open, so that we
> would get automatic cleanup on process exit. It also means we should be
> able to kill a few more module options that shouldn't exist to tempt
> users.
> -Chris

I was just worried about potentially adding noise to other tests. If we plan to
remove the module parameter, it won't happen.

With or without the helper:

Reviewed-by: Michał Winiarski <michal.winiarski at intel.com>

-Michał