Re: [PATCH 1/6] drm/i915: Replace hardcoded cacheline size with macro

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Apr 03, 2014 at 07:45:40AM +0100, Chris Wilson wrote:
> On Wed, Apr 02, 2014 at 02:57:11PM -0700, Jesse Barnes wrote:
> > On Wed,  2 Apr 2014 16:36:06 +0100
> > Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > 
> > > For readibility and guess at the meaning behind the constants.
> > > 
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > ---
> > >  drivers/gpu/drm/i915/intel_ringbuffer.c | 29 ++++++++++++++++-------------
> > >  1 file changed, 16 insertions(+), 13 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > > index 785f246d28a8..475391ce671a 100644
> > > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > > @@ -33,6 +33,8 @@
> > >  #include "i915_trace.h"
> > >  #include "intel_drv.h"
> > >  
> > > +#define CACHELINE_BYTES 64
> > > +
> > 
> > Are you sure it's 64 on every gen?  It changes on the CPU side from
> > time to time... I thought it might have changed over time on the GPU
> > too but I haven't checked the specs.
> 
> The cacheline is 32bytes on gen2, 64 elsewhere. We've made a blanket
> assumption of 64 and then some random factors on top. (Some cachelines
> may be as large as 1024bytes elsewhere in the chip.) I'm not sure where
> some of the values used in the code where plucked from, Jesse?

Maybe a quick comment that this is the maximum and that gen2 has only
32bytes but aligning more isn't harmful?

Perhaps also mention that for actual cacheline flushing we _must_ use the
cl size of the cpu for otherwise we don't flush poperly. If your define
has some comment/warning to that effect I'm ok with this generalization.
And it nicely makes some of the additional lore (128bytes ?!) stick out
more.

btw the 1k thing at least on i865G is iirc just the writeout fifo between
the cpu and the gmch to paper over FSB latencies (or whatever irked hw
designers).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch