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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxxxxxx>
> > 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@xxxxxxxxxxxxxxxxxx>
> 
> 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?
OTOH, determining test behaviour based on particular "device id" (generation
really), rather than some "capability" is kind of ugly.

-Michał

> 
> > ---
> >  tests/prime_vgem.c | 36 ++++++++++++++++++++++++++++++++----
> >  1 file changed, 32 insertions(+), 4 deletions(-)
> > 
> > diff --git a/tests/prime_vgem.c b/tests/prime_vgem.c
> > index 95557ef9..15de7993 100644
> > --- a/tests/prime_vgem.c
> > +++ b/tests/prime_vgem.c
> > @@ -232,18 +232,43 @@ static void test_gtt(int vgem, int i915)
> >                 igt_assert_eq(ptr[1024*i], ~i);
> >         munmap(ptr, scratch.size);
> >  
> > +       gem_close(i915, handle);
> > +       gem_close(vgem, scratch.handle);
> > +}
> > +
> > +static void test_gtt_interleaved(int vgem, int i915)
> > +{
> > +       struct vgem_bo scratch;
> > +       uint32_t handle;
> > +       uint32_t *ptr, *gtt;
> > +       int dmabuf, i;
> > +
> > +       scratch.width = 1024;
> > +       scratch.height = 1024;
> > +       scratch.bpp = 32;
> > +       vgem_create(vgem, &scratch);
> > +
> > +       dmabuf = prime_handle_to_fd(vgem, scratch.handle);
> > +       handle = prime_fd_to_handle(i915, dmabuf);
> > +       close(dmabuf);
> > +
> > +       /* This assumes that GTT is perfectedly coherent. On certain machines,
> > +        * it is possible for a direct acces to bypass the GTT indirection.
> > +        *
> > +        * This test may fail. It tells us how far userspace can trust
> > +        * concurrent dmabuf/i915 access.
> > +        */
> >         ptr = vgem_mmap(vgem, &scratch, PROT_WRITE);
> >         gtt = gem_mmap__gtt(i915, handle, scratch.size, PROT_WRITE);
> > -#if defined(__x86_64__)
> >         for (i = 0; i < 1024; i++) {
> >                 gtt[1024*i] = i;
> > -               __builtin_ia32_sfence();
> > +               /* The read from WC should act as a flush for the GTT wcb */
> >                 igt_assert_eq(ptr[1024*i], i);
> > +
> >                 ptr[1024*i] = ~i;
> > -               __builtin_ia32_sfence();
> > +               /* The read from GTT should act as a flush for the WC wcb */
> >                 igt_assert_eq(gtt[1024*i], ~i);
> >         }
> > -#endif
> >         munmap(gtt, scratch.size);
> >         munmap(ptr, scratch.size);
> >  
> > @@ -753,6 +778,9 @@ igt_main
> >         igt_subtest("basic-gtt")
> >                 test_gtt(vgem, i915);
> >  
> > +       igt_subtest("coherency-gtt")
> > +               test_gtt_interleaved(vgem, i915);
> > +
> >         for (e = intel_execution_engines; e->name; e++) {
> >                 igt_subtest_f("%ssync-%s",
> >                               e->exec_id == 0 ? "basic-" : "",
> > -- 
> > 2.14.1
> > 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux