[Intel-gfx] [PATCH i-g-t] lib/igt_gt.c : allow changes to stop_rings mode bits

[Gore, Tim]

Tim Gore 
Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon SN3 1RJ

> -----Original Message-----
> From: Daniel Vetter [mailto:daniel.vetter at ffwll.ch] On Behalf Of Daniel
> Vetter
> Sent: Monday, July 13, 2015 10:30 AM
> To: Gore, Tim
> Cc: intel-gfx at lists.freedesktop.org; Wood, Thomas
> Subject: Re: [Intel-gfx] [PATCH i-g-t] lib/igt_gt.c : allow changes to stop_rings
> mode bits
> 
> On Fri, Jul 10, 2015 at 02:06:06PM +0100, tim.gore at intel.com wrote:
> > From: Tim Gore <tim.gore at intel.com>
> >
> > In function igt_set_stop_rings, the test
> >   igt_assert_f(flags == 0 || current == 0, ..
> >
> > will fail if we are trying to force a hang but the
> > STOP_RINGS_ALLOW_BAN or STOP_RINGS_ALLOW_ERROR bit is set.
> > With the introduction of per ring resets in the driver (in android)
> > these bits do not get cleared to zero when an individual ring is
> > reset. This causes subsequent attempt to cause a ring hang via this
> > function to fail, leading to several igt tests failing (ie
> > gem_reset_stats subtest ban-xxx etc).
> 
> Fix tdr to reset these instead?
> -Daniel
> 
I could change tdr, but why. When the TDR handles a ring hang and resets the ring,
why would it modify the flag that defines if the driver should ban a frequently hanging
context? If we get rid of the stop_rings interface, as Chris Wilson suggested, we would
still need to keep the STOP_RING_ALLOW_BAN/ALLOW_ERRORS bits in debugfs,
but you would not expect to have to re-write these bits each time there is a ring
reset. 

  Tim

> > So, modify this test to look only at the bits that are used to hang
> > the gpu rings.
> >
> > Signed-off-by: Tim Gore <tim.gore at intel.com>
> > ---
> >  lib/igt_gt.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/igt_gt.c b/lib/igt_gt.c index 8e5e076..12c56fa 100644
> > --- a/lib/igt_gt.c
> > +++ b/lib/igt_gt.c
> > @@ -345,8 +345,8 @@ void igt_set_stop_rings(enum stop_ring_flags flags)
> >  			      STOP_RING_ALLOW_ERRORS)) == 0);
> >
> >  	current = igt_get_stop_rings();
> > -	igt_assert_f(flags == 0 || current == 0,
> > -		     "previous i915_ring_stop is still 0x%x\n", current);
> > +	igt_assert_f( (flags & STOP_RING_ALL) == 0 || (current &
> STOP_RING_ALL) == 0,
> > +			     "previous i915_ring_stop is still 0x%x\n", current);
> >
> >  	stop_rings_write(flags);
> >  	current = igt_get_stop_rings();
> > --
> > 1.9.1
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch