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

Jesse Barnes
2015-09-21T16:57:22Z

On 09/21/2015 09:53 AM, Ville Syrjälä wrote:
> On Mon, Sep 21, 2015 at 09:26:11AM -0700, Jesse Barnes wrote:
>> On 09/18/2015 12:26 PM, Ville Syrjälä wrote:
>>> On Fri, Sep 18, 2015 at 07:44:34PM +0100, Chris Wilson wrote:
>>> 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.
>>
>> If you mean the top level readX_relaxed() functions, those were more
>> about re-ordering on a large I/O fabric than compiler re-ordering. I'm
>> not sure how the compiler handles things these days; I thought maybe the
>> volatile accesses would imply a re-order barrier as well, since they
>> could have side effects the compiler can't anticipate (i.e. re-ordering
>> even read after read could change behavior). But it would be good to
>> double check... and according to memory-barriers.txt we should be safe
>> there, from the readX_relaxed() section:
>>
>> "Note that relaxed accesses to the same peripheral are guaranteed to be
>> ordered with respect to each other."
>
> We have
> #define barrier() asm volatile("" ::: "memory")
>
> so I take it someone definitely thinks just volatile isn't enough to
> keep the compiler in check.

Yeah, maybe with respect to other, non-I/O targeted reads & writes
things get messier...

Jesse