On Wed, May 31, 2017 at 05:01:53PM +0300, Mika Kuoppala wrote:
> Chris Wilson <[email protected]> writes:
> 
> > On Tue, May 30, 2017 at 03:33:41PM +0300, Mika Kuoppala wrote:
> >> Chris Wilson <[email protected]> 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 <[email protected]>
> >> > Cc: Mika Kuoppala <[email protected]>
> >> > ---
> >> >  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 <[email protected]>

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

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to