On Wed, Mar 16, 2016 at 01:52:10PM +0100, Michał Winiarski wrote: > Test description suggested that all platforms were testing qword writes, > while in fact only gen4-gen5 did. > > v2: Test dword/qword writes for all available platforms > > Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > Signed-off-by: Michał Winiarski <michal.winiarski@xxxxxxxxx> > --- > tests/gem_pipe_control_store_loop.c | 49 +++++++++++++++++++++++++------------ > 1 file changed, 33 insertions(+), 16 deletions(-) > > diff --git a/tests/gem_pipe_control_store_loop.c b/tests/gem_pipe_control_store_loop.c > index a155ad1..cab3ed5 100644 > --- a/tests/gem_pipe_control_store_loop.c > +++ b/tests/gem_pipe_control_store_loop.c > @@ -26,7 +26,7 @@ > */ > > /* > - * Testcase: (TLB-)Coherency of pipe_control QW writes > + * Testcase: (TLB-)Coherency of pipe_control writes > * > * Writes a counter-value into an always newly allocated target bo (by disabling > * buffer reuse). Decently trashes on tlb inconsistencies, too. > @@ -43,7 +43,7 @@ > #include "drm.h" > #include "intel_bufmgr.h" > > -IGT_TEST_DESCRIPTION("Test (TLB-)Coherency of pipe_control QW writes."); > +IGT_TEST_DESCRIPTION("Test (TLB-)Coherency of pipe_control writes."); > > static drm_intel_bufmgr *bufmgr; > struct intel_batchbuffer *batch; > @@ -60,13 +60,20 @@ uint32_t devid; > #define PIPE_CONTROL_CS_STALL (1<<20) > #define PIPE_CONTROL_GLOBAL_GTT (1<<2) /* in addr dword */ > > +#define PIPE_CONTROL_STATE_BUFFER_REUSED (1 << 0) > +#define PIPE_CONTROL_STATE_QWORD_WRITE (1 << 1) > +#define PIPE_CONTROL_STATE_ALL_FLAGS (PIPE_CONTROL_STATE_BUFFER_REUSED | \ > + PIPE_CONTROL_STATE_QWORD_WRITE) Let's not use PIPE_CONTROL for test flags. That was very confusing! > + > /* Like the store dword test, but we create new command buffers each time */ > static void > -store_pipe_control_loop(bool preuse_buffer) > +store_pipe_control_loop(uint32_t flags) > { > int i, val = 0; > uint32_t *buf; > drm_intel_bo *target_bo; > + const bool preuse_buffer = flags & PIPE_CONTROL_STATE_BUFFER_REUSED; > + const bool qword_write = flags & PIPE_CONTROL_STATE_QWORD_WRITE; > > for (i = 0; i < SLOW_QUICK(0x10000, 4); i++) { > /* we want to check tlb consistency of the pipe_control target, > @@ -98,15 +105,16 @@ store_pipe_control_loop(bool preuse_buffer) > * creating new batchbuffers - with buffer reuse disabled, the > * support code will do that for us. */ > if (batch->gen >= 8) { > - BEGIN_BATCH(4, 1); > - OUT_BATCH(GFX_OP_PIPE_CONTROL + 1); > + BEGIN_BATCH(4 + qword_write, 1); > + OUT_BATCH(GFX_OP_PIPE_CONTROL + 1 + qword_write); > OUT_BATCH(PIPE_CONTROL_WRITE_IMMEDIATE); > OUT_RELOC_FENCED(target_bo, > I915_GEM_DOMAIN_INSTRUCTION, I915_GEM_DOMAIN_INSTRUCTION, > PIPE_CONTROL_GLOBAL_GTT); > OUT_BATCH(val); /* write data */ > + if (qword_write) > + OUT_BATCH(~val); /* high dword */ > ADVANCE_BATCH(); Let's put a MI_NOOP | 0xabcd here to catch if the HW is reading past the end of the packet and writing the upper dword anyway. > - > } else if (batch->gen >= 6) { > /* work-around hw issue, see intel_emit_post_sync_nonzero_flush > * in mesa sources. */ > @@ -118,24 +126,27 @@ store_pipe_control_loop(bool preuse_buffer) > OUT_BATCH(0); /* write data */ > ADVANCE_BATCH(); > > - BEGIN_BATCH(4, 1); > - OUT_BATCH(GFX_OP_PIPE_CONTROL); > + BEGIN_BATCH(4 + qword_write, 1); > + OUT_BATCH(GFX_OP_PIPE_CONTROL + qword_write); > OUT_BATCH(PIPE_CONTROL_WRITE_IMMEDIATE); > OUT_RELOC(target_bo, > I915_GEM_DOMAIN_INSTRUCTION, I915_GEM_DOMAIN_INSTRUCTION, > PIPE_CONTROL_GLOBAL_GTT); > OUT_BATCH(val); /* write data */ > + if (qword_write) > + OUT_BATCH(~val); /* high dword */ > ADVANCE_BATCH(); > } else if (batch->gen >= 4) { > - BEGIN_BATCH(4, 1); > + BEGIN_BATCH(3 + qword_write, 1); > OUT_BATCH(GFX_OP_PIPE_CONTROL | PIPE_CONTROL_WC_FLUSH | > PIPE_CONTROL_TC_FLUSH | > - PIPE_CONTROL_WRITE_IMMEDIATE | 2); > + PIPE_CONTROL_WRITE_IMMEDIATE | (1 + qword_write)); Ideally we should remove the extra flushes that are not required for emitting the write. Part of the test is that the kernel is emitting adequate flushes for a standalone pipecontrol batch. > OUT_RELOC(target_bo, > I915_GEM_DOMAIN_INSTRUCTION, I915_GEM_DOMAIN_INSTRUCTION, > PIPE_CONTROL_GLOBAL_GTT); Fiddle sticks. Get rid of the libdrm layer so that I can control the placement of the batch completely to double check that the kernel w/a happens with the batch and not by random happenstance because of intel_batchbuffer.c > OUT_BATCH(val); > - OUT_BATCH(0xdeadbeef); > + if (qword_write) > + OUT_BATCH(~val); /* high dword */ > ADVANCE_BATCH(); > } > > @@ -145,6 +156,8 @@ store_pipe_control_loop(bool preuse_buffer) > > buf = target_bo->virtual; > igt_assert(buf[0] == val); > + if (qword_write) > + igt_assert(buf[1] == ~val); else igt_assert_eq_u32(buf[1], 0); We also want to test CPU coherency, since these instructions are advertising as being compatable with snooped writes. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx