[Intel-gfx] [PATCH 3/3] drm/i915: Check the ring is empty when declaring the engines are idle

[Chris Wilson]

On Wed, May 31, 2017 at 05:01:53PM +0300, Mika Kuoppala wrote:
> Chris Wilson <chris at chris-wilson.co.uk> writes:
> 
> > On Tue, May 30, 2017 at 03:33:41PM +0300, Mika Kuoppala wrote:
> >> Chris Wilson <chris at chris-wilson.co.uk> writes:
> >> 
> >> > As another precaution when testing whether the CS engine is actually
> >> > idle, also inspect the ring's HEAD/TAIL registers, which should be equal
> >> > when there are no commands left to execute by the GPU.
> >> >
> >> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> >> > Cc: Mika Kuoppala <mika.kuoppala at intel.com>
> >> > ---
> >> >  drivers/gpu/drm/i915/intel_engine_cs.c | 5 +++++
> >> >  1 file changed, 5 insertions(+)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> >> > index 699f2d3861c7..bc38bd128b76 100644
> >> > --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> >> > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> >> > @@ -1212,6 +1212,11 @@ static bool ring_is_idle(struct intel_engine_cs *engine)
> >> >  
> >> >  	intel_runtime_pm_get(dev_priv);
> >> >  
> >> > +	/* First check that no commands are left in the ring */
> >> > +	if ((I915_READ_HEAD(engine) & HEAD_ADDR) !=
> >> > +	    (I915_READ_TAIL(engine) & TAIL_ADDR))
> >> > +		idle = false;
> >> > +
> >> 
> >> You are already certain that is not idle so why not goto out?
> >
> > In this case I could argue that extra path for the jump is not worth it.
> > It saves a mmio read, yes, but will any one notice?
> and one write :P
> 
> > It boils down to is it easier to read as:
> >
> 
> Sold. It is easier to read as is. 3/3 is
> 
> Reviewed-by: Mika Kuoppala <mika.kuoppala at intel.com>

Thanks, now pushed. Hopefully it never spots an error!
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre