[Intel-gfx] [PATCH igt] igt/prime_vgem: Split out the fine-grain coherency check

[Michał Winiarski]

On Thu, Sep 21, 2017 at 11:59:29AM +0100, Chris Wilson wrote:
> Quoting Michał Winiarski (2017-09-20 11:48:54)
> > On Tue, Sep 19, 2017 at 01:21:08PM +0100, Chris Wilson wrote:
> > > Quoting Chris Wilson (2017-09-07 15:30:55)
> > > > We don't expect every machine to be able to pass the WC/GTT coherency
> > > > check, see
> > > > 
> > > > kernel commit 3b5724d702ef24ee41ca008a1fab1cf94f3d31b5
> > > > Author: Chris Wilson <chris at chris-wilson.co.uk>
> > > > Date:   Thu Aug 18 17:16:49 2016 +0100
> > > > 
> > > >     drm/i915: Wait for writes through the GTT to land before reading back
> > > > 
> > > >     If we quickly switch from writing through the GTT to a read of the
> > > >     physical page directly with the CPU (e.g. performing relocations through
> > > >     the GTT and then running the command parser), we can observe that the
> > > >     writes are not visible to the CPU. It is not a coherency problem, as
> > > >     extensive investigations with clflush have demonstrated, but a mere
> > > >     timing issue - we have to wait for the GTT to complete it's write before
> > > >     we start our read from the CPU.
> > > > 
> > > >     The issue can be illustrated in userspace with:
> > > > 
> > > >             gtt = gem_mmap__gtt(fd, handle, 0, OBJECT_SIZE, PROT_READ | PROT_WRITE);
> > > >             cpu = gem_mmap__cpu(fd, handle, 0, OBJECT_SIZE, PROT_READ | PROT_WRITE);
> > > >             gem_set_domain(fd, handle, I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
> > > > 
> > > >             for (i = 0; i < OBJECT_SIZE / 64; i++) {
> > > >                     int x = 16*i + (i%16);
> > > >                     gtt[x] = i;
> > > >                     clflush(&cpu[x], sizeof(cpu[x]));
> > > >                     assert(cpu[x] == i);
> > > >             }
> > > > 
> > > >     Experimenting with that shows that this behaviour is indeed limited to
> > > >     recent Atom-class hardware.
> > > > 
> > > > so split out the interleave coherency check from the basic
> > > > interopability check.
> > > > 
> > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102577
> > > > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > > 
> > > Ping?
> > 
> > Mechanical change. LGTM.
> > One question though - if we do expect that coherency-gtt may fail on low-power
> > HW, perhaps we should skip it there, rather than filter through the noisy
> > results?
> 
> I don't know for sure; the test is a good discriminator for which
> platforms do need extra care (and so far big core seems reliable).
> 
> > OTOH, determining test behaviour based on particular "device id" (generation
> > really), rather than some "capability" is kind of ugly.
> 
> Yup. And such knowledge should flow from the kernel "hey, on this
> platform this ABI expectation can no longer be met".
> -Chris

Reviewed-by: Michał Winiarski <michal.winiarski at intel.com>

-Michał