All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH igt] igt/prime_vgem: Split out the fine-grain coherency check
@ 2017-09-07 14:30 Chris Wilson
  2017-09-07 15:22 ` ✓ Fi.CI.BAT: success for " Patchwork
  2017-09-19 12:21 ` [PATCH igt] " Chris Wilson
  0 siblings, 2 replies; 6+ messages in thread
From: Chris Wilson @ 2017-09-07 14:30 UTC (permalink / raw)
  To: intel-gfx

We don't expect every machine to be able to pass the WC/GTT coherency
check, see

kernel commit 3b5724d702ef24ee41ca008a1fab1cf94f3d31b5
Author: Chris Wilson <chris@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@chris-wilson.co.uk>
---
 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* ✓ Fi.CI.BAT: success for igt/prime_vgem: Split out the fine-grain coherency check
  2017-09-07 14:30 [PATCH igt] igt/prime_vgem: Split out the fine-grain coherency check Chris Wilson
@ 2017-09-07 15:22 ` Patchwork
  2017-09-19 12:21 ` [PATCH igt] " Chris Wilson
  1 sibling, 0 replies; 6+ messages in thread
From: Patchwork @ 2017-09-07 15:22 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: igt/prime_vgem: Split out the fine-grain coherency check
URL   : https://patchwork.freedesktop.org/series/29952/
State : success

== Summary ==

IGT patchset tested on top of latest successful build
4a1c8daff2005e2cbfe980d63bc0a0fb09cb017d igt/gem_ringfill: Prime execbuf before measuring ring size

with latest DRM-Tip kernel build CI_DRM_3054
00f9b49384df drm-tip: 2017y-09m-07d-09h-58m-50s UTC integration manifest

Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-atomic:
                fail       -> PASS       (fi-snb-2600) fdo#100215
Test kms_flip:
        Subgroup basic-flip-vs-modeset:
                skip       -> PASS       (fi-skl-x1585l) fdo#101781
Test pm_rpm:
        Subgroup basic-pci-d3-state:
                skip       -> PASS       (fi-cfl-s) fdo#102294
Test drv_module_reload:
        Subgroup basic-reload-inject:
                dmesg-fail -> DMESG-WARN (fi-cfl-s) k.org#196765

fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215
fdo#101781 https://bugs.freedesktop.org/show_bug.cgi?id=101781
fdo#102294 https://bugs.freedesktop.org/show_bug.cgi?id=102294
k.org#196765 https://bugzilla.kernel.org/show_bug.cgi?id=196765

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:464s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:444s
fi-blb-e6850     total:289  pass:224  dwarn:1   dfail:0   fail:0   skip:64  time:362s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:567s
fi-bwr-2160      total:289  pass:184  dwarn:0   dfail:0   fail:0   skip:105 time:254s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:528s
fi-byt-j1900     total:289  pass:254  dwarn:1   dfail:0   fail:0   skip:34  time:525s
fi-byt-n2820     total:289  pass:250  dwarn:1   dfail:0   fail:0   skip:38  time:515s
fi-cfl-s         total:289  pass:250  dwarn:4   dfail:0   fail:0   skip:35  time:473s
fi-elk-e7500     total:289  pass:230  dwarn:0   dfail:0   fail:0   skip:59  time:443s
fi-glk-2a        total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:614s
fi-hsw-4770      total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:446s
fi-hsw-4770r     total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:434s
fi-ilk-650       total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:424s
fi-ivb-3520m     total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:509s
fi-ivb-3770      total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:476s
fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:0   skip:24  time:514s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:603s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:606s
fi-pnv-d510      total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:525s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:473s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:541s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:516s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:444s
fi-skl-x1585l    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:518s
fi-snb-2520m     total:289  pass:251  dwarn:0   dfail:0   fail:0   skip:38  time:567s
fi-snb-2600      total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:415s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_159/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH igt] igt/prime_vgem: Split out the fine-grain coherency check
  2017-09-07 14:30 [PATCH igt] igt/prime_vgem: Split out the fine-grain coherency check Chris Wilson
  2017-09-07 15:22 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2017-09-19 12:21 ` Chris Wilson
  2017-09-20 10:48   ` Michał Winiarski
  1 sibling, 1 reply; 6+ messages in thread
From: Chris Wilson @ 2017-09-19 12:21 UTC (permalink / raw)
  To: intel-gfx

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@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@chris-wilson.co.uk>

Ping?

> ---
>  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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH igt] igt/prime_vgem: Split out the fine-grain coherency check
  2017-09-19 12:21 ` [PATCH igt] " Chris Wilson
@ 2017-09-20 10:48   ` Michał Winiarski
  2017-09-21 10:59     ` Chris Wilson
  0 siblings, 1 reply; 6+ messages in thread
From: Michał Winiarski @ 2017-09-20 10:48 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

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@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@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?
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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH igt] igt/prime_vgem: Split out the fine-grain coherency check
  2017-09-20 10:48   ` Michał Winiarski
@ 2017-09-21 10:59     ` Chris Wilson
  2017-09-21 11:36       ` Michał Winiarski
  0 siblings, 1 reply; 6+ messages in thread
From: Chris Wilson @ 2017-09-21 10:59 UTC (permalink / raw)
  To: Michał Winiarski; +Cc: intel-gfx

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@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@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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH igt] igt/prime_vgem: Split out the fine-grain coherency check
  2017-09-21 10:59     ` Chris Wilson
@ 2017-09-21 11:36       ` Michał Winiarski
  0 siblings, 0 replies; 6+ messages in thread
From: Michał Winiarski @ 2017-09-21 11:36 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

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@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@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@intel.com>

-Michał
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-09-21 11:36 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-07 14:30 [PATCH igt] igt/prime_vgem: Split out the fine-grain coherency check Chris Wilson
2017-09-07 15:22 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-09-19 12:21 ` [PATCH igt] " Chris Wilson
2017-09-20 10:48   ` Michał Winiarski
2017-09-21 10:59     ` Chris Wilson
2017-09-21 11:36       ` Michał Winiarski

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.