[Intel-gfx] [CI 1/3] drm/i915: Avoid the branch in computing intel_ring_space()

[Chris Wilson]

On Thu, May 04, 2017 at 04:17:13PM +0200, Michal Wajdeczko wrote:
> On Thu, May 04, 2017 at 02:08:44PM +0100, Chris Wilson wrote:
> > Exploit the power-of-two ring size to compute the space across the
> > wraparound using a mask rather than a if. Convert to unsigned integers
> > so the operation is well defined.
> > 
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=99671
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Mika Kuoppala <mika.kuoppala at intel.com>
> > Reviewed-by: Mika Kuoppala <mika.kuoppala at intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_ringbuffer.c | 23 +++++++++++----------
> >  drivers/gpu/drm/i915/intel_ringbuffer.h | 36 ++++++++++++++++++++-------------
> >  2 files changed, 34 insertions(+), 25 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > index 3ce1c87dec46..e7ef04cc071b 100644
> > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > @@ -39,12 +39,16 @@
> >   */
> >  #define LEGACY_REQUEST_SIZE 200
> >  
> > -static int __intel_ring_space(int head, int tail, int size)
> > +static unsigned int __intel_ring_space(unsigned int head,
> > +				       unsigned int tail,
> > +				       unsigned int size)
> >  {
> > -	int space = head - tail;
> > -	if (space <= 0)
> > -		space += size;
> > -	return space - I915_RING_FREE_SPACE;
> > +	/*
> > +	 * "If the Ring Buffer Head Pointer and the Tail Pointer are on the
> > +	 * same cacheline, the Head Pointer must not be greater than the Tail
> > +	 * Pointer."
> > +	 */
> > +	return (head - tail - CACHELINE_BYTES) & (size - 1);
> 
> Btw, as you exploit power-of-two ring size here, maybe it is worth to repeat
> 
> 	GEM_BUG_ON(!is_power_of_2(size));
> 
> to emphase this assumption in the code (not only in the commit message)?

I did check we had an is_power_of_2() check in intel_engine_create_ring.
Might be worth asserting here as well as there's a little disconnect
between the function and ring->size.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre