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

[Daniel Vetter]

On Mon, Jul 13, 2015 at 09:43:11AM +0000, Gore, Tim wrote:
> 
> 
> 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. 

The fix current hang recover code to no reset this, add some grace period,
then push this patch to igt. We don't have full-blown abi guarantees for
debugfs/igt stuff, but I want at least a few months (really last released
kernel&igt) of backwards/forward compatibility. And inconsistent behaviour
isn't great imo.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch