[igt-dev] [PATCH i-g-t 2/2] i915/treewide: Replace gem_mmap__gtt() by gem_mmap__device_coherent()

[Chris Wilson]

Quoting Dixit, Ashutosh (2020-01-12 02:57:22)
> On Sat, 11 Jan 2020 00:44:59 -0800, Chris Wilson wrote:
> >
> > Here, we want OFFSET_WC etc added. So leave this style for a later patch,
> > and focus on just converting the ones where we want any old mapping to
> > device_coherent(). Another patch can focus on converting any __cpu maps
> > that are only used for writes to device_coherent.
> 
> Thanks for the feedback Chris, makes sense, will rework and repost.
> 
> Some time ago, Dixit, Ashutosh wrote:
> > On Sat, 11 Jan 2020 00:40:32 -0800, Chris Wilson wrote:
> > > Quoting Ashutosh Dixit (2020-01-11 05:36:52)
> > > > In order to preserve legacy behavior for devices with a mappable
> > > > aperture try a GTT mapping first. Use a WC mapping only for devices
> > > > without a mappable aperture.
> > >
> > > No...? It's just historical legacy from before we even had WC available.
> > > Since we've had WC for basic mmappings, we prefer WC since it is not as
> > > scarce and as precious a resource. (Not that it matters for most tests
> > > since GTT thrashing in basic igt is unheard of!)
> 
> About this, I was assuming if we were using WC instead of GTT on legacy
> (Gen < 6), we would have to set read/write domains to WC too, unless you
> are saying it's not required? (From what I know, for later Gen's, setting
> the domain to either GTT or WC is identical).

The complication is just in mixing GTT and WC mappings [including
implicit paths via reloc/pwrite/pread). And technically that matters for
all gen, as GTT is only coherent on a few platforms.  But it really only
matters if you follow a GTT write with a WC read (GTT write followed by
WC write could also technically race). The answer is not to mix a
device_coherent mapping with any other GEM access [on the same cacheline],
and you can rely on set_domain(GTT).
-Chris