Re: [PATCH] drm/i915: Double check ring is idle before declaring the GPU wedged

[Damien Lespiau <damien.lespiau@intel.com>]

On Mon, Aug 11, 2014 at 10:35:25AM +0100, Chris Wilson wrote:
> On Mon, Aug 11, 2014 at 10:30:09AM +0100, Damien Lespiau wrote:
> > On Mon, Aug 11, 2014 at 09:21:35AM +0100, Chris Wilson wrote:
> > > During ring initialisation, sometimes we observe, though not in
> > > production hardware, that the idle flag is not set even though the ring
> > > is empty. Double check before giving up.
> > > 
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: Damien Lespiau <damien.lespiau@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_ringbuffer.c | 7 ++++++-
> > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > > index a0831c309eab..d72d5e0e693d 100644
> > > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > > @@ -467,7 +467,12 @@ static bool stop_ring(struct intel_engine_cs *ring)
> > >  		I915_WRITE_MODE(ring, _MASKED_BIT_ENABLE(STOP_RING));
> > >  		if (wait_for((I915_READ_MODE(ring) & MODE_IDLE) != 0, 1000)) {
> > >  			DRM_ERROR("%s : timed out trying to stop ring\n", ring->name);
> > > -			return false;
> > > +			/* Sometimes we observe that the idle flag is not
> > > +			 * set even though the ring is empty. So double
> > > +			 * check before giving up.
> > > +			 */
> > > +			if (I915_READ_HEAD(ring) != I915_READ_TAIL(ring))
> > > +				return false;
> > 
> > That means we propably want to just put the user visible error message
> > there as well?
> 
> It is still a 1 second timeout, so having a warning there that something
> is wrong is important I thought.

Ah, I missed the "not in production hw" bits of the commit message, It
may mean simulation and then the guess is that flag is not implemented.
If that makes us not totally give up, I guess that's something.

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

-- 
Damien