[Intel-gfx] [PATCH 1/5] agp/intel: Serialise after GTT updates

[Ville Syrjälä]

On Fri, Feb 06, 2015 at 09:32:29AM +0100, Daniel Vetter wrote:
> On Thu, Feb 05, 2015 at 04:11:00PM -0800, Jesse Barnes wrote:
> > On Wed, 14 Jan 2015 11:20:55 +0000
> > Chris Wilson <chris at chris-wilson.co.uk> wrote:
> > 
> > > diff --git a/drivers/char/agp/intel-gtt.c
> > > b/drivers/char/agp/intel-gtt.c index 92aa43fa8d70..15685ca39193 100644
> > > --- a/drivers/char/agp/intel-gtt.c
> > > +++ b/drivers/char/agp/intel-gtt.c
> > > @@ -225,7 +225,7 @@ static int i810_insert_dcache_entries(struct
> > > agp_memory *mem, off_t pg_start,
> > > intel_private.driver->write_entry(addr, i, type);
> > >  	}
> > > -	readl(intel_private.gtt+i-1);
> > > +	readl(intel_private.gtt+pg_start);
> > 
> > Any idea why?  This one scares me...  is it that the read is being
> > serviced from the WC buffer w/o being flushed?  Or is the compiler
> > optimizing the last read based on the previous write?
> > 
> > Writing a non-sequential address should also cause a flush, but I don't
> > remember the rules for reads.  We should get this figured out while we
> > have an easy way to reproduce and a willing tester.
> 
> Yeah agreed, but apparently a full mb(); is good enough too. So that's
> what has landed.

I was first wondering if we need something like this for gen6+ too, but
then I relalized the UC GFX_FLUSH_CNTL access should cause the WC flush
already.

Hmm, except we don't do it for clear_range(), but I guess that's not a
huge issue, just means someone could clobber some other memory besides
the scratch page if accidentally writing to a cleared area of the ggtt.

-- 
Ville Syrjälä
Intel OTC