[Intel-gfx] [PATCH 35/43] drm/i915: Move __raw_i915_read8() & co. into i915_drv.h

[Ville Syrjälä]

On Fri, Sep 18, 2015 at 07:44:34PM +0100, Chris Wilson wrote:
> On Fri, Sep 18, 2015 at 09:37:38PM +0300, Ville Syrjälä wrote:
> > On Fri, Sep 18, 2015 at 07:33:50PM +0100, Chris Wilson wrote:
> > > On Fri, Sep 18, 2015 at 09:23:11PM +0300, Ville Syrjälä wrote:
> > > > On Fri, Sep 18, 2015 at 06:42:17PM +0100, Chris Wilson wrote:
> > > > > On Fri, Sep 18, 2015 at 08:03:48PM +0300, ville.syrjala at linux.intel.com wrote:
> > > > > > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > > > > 
> > > > > > We have a few users of the raw register acces functions outside
> > > > > > intel_uncore.c, so let's just move the functions into intel_drv.h.
> > > > > 
> > > > > I would rather see those external users converted to
> > > > > I915_READ_FW/I915_WRITE_FW etc. You will then, no doubt, want to convert
> > > > > those _FW macro definitions over to the uncore set.
> > > > > 
> > > > > Also due to how we write and post our accesses, the raw functions can be
> > > > > the _relaxed variants.
> > > > 
> > > > Hmm. I think the only difference with the relaxed vs. not would be
> > > > potential compiler reordering of memory accesses vs. mmio. So if we
> > > > start using the relaxed versions we may need to start sprinkling
> > > > barriers around.
> > > 
> > > Yes. We have been working under that assumption (weak ordering of writes),
> > > or at least I hope we all have been...
> > 
> > Well, looking at the irq code that uses the _FW, what would prevent the
> > compiler from eg. reorder the seqno read to happen before the IIR read?
> > Well in practice function calls are involved, so there's a barrier
> > there, but say we would want to read the seqno straight from the irq
> > handler...
> 
> What seqno read? We definitely don't want to be doing those expensive
> reads from an irq handler...

Yeah I guess that was a crappy example. Trying to think of a better one,
I figure execlist status could be read from memory, but apparently
that's just mmio. I guess the context update + ELSP write is something
we do from the irq handler, but there are plenty of barriers between
those two it seems. Maybe there's no good example here.

> 
> Anyway, I thought we had strongly ordered reads on x64/x32?

The cpu won't reoder reads vs. reads, or writes vs. writes for that
matter if you ignore the nt stuff and whatnot. But AFAIU the whole
point of _relaxed() on x86 is that it allows the compiler to reoder
memory vs. mmio any which way it wants.

-- 
Ville Syrjälä
Intel OTC