[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've made the cardinal sin of changing it at the last moment, if I've
broken everything I'm going to blame you :)

Semi-pushed, looks like we're already back in conflict territory.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre