Re: [Intel-gfx] [PATCH igt 3/3] igt/gem_exec_parse: update for version 8 changes

[Chris Wilson]

On Wed, Oct 26, 2016 at 05:01:33PM +0100, Robert Bragg wrote:
> This adapts the tests to account for the parser no longer reporting
> privilege violations back to userspace as EINVAL errors (they are left
> to the HW command parser to squash the commands to NOOPS).
> 
> The interface change isn't expected to affect userspace and in fact it
> looks like the previous behaviour was liable to break userspace, such as
> Mesa which explicitly tries to observe whether OACONTROL LRIs are
> squashed to NOOPs but Mesa will abort for execbuffer errors.

That's an extremely large patch for a simple version check. The test for
LRI turning into noops is already there.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH igt 3/3] igt/gem_exec_parse: update for version 8 changes

[Robert Bragg]

This adapts the tests to account for the parser no longer reporting
privilege violations back to userspace as EINVAL errors (they are left
to the HW command parser to squash the commands to NOOPS).

The interface change isn't expected to affect userspace and in fact it
looks like the previous behaviour was liable to break userspace, such as
Mesa which explicitly tries to observe whether OACONTROL LRIs are
squashed to NOOPs but Mesa will abort for execbuffer errors.

Signed-off-by: Robert Bragg 
---
 tests/gem_exec_parse.c | 368 +++--
 1 file changed, 200 insertions(+), 168 deletions(-)

diff --git a/tests/gem_exec_parse.c b/tests/gem_exec_parse.c
index 36bf57d..4290701 100644
--- a/tests/gem_exec_parse.c
+++ b/tests/gem_exec_parse.c
@@ -34,7 +34,24 @@
 #define I915_PARAM_CMD_PARSER_VERSION   28
 #endif
 
+#define ARRAY_LEN(A) (sizeof(A) / sizeof(A[0]))
+
+#define OACONTROL 0x2360
 #define DERRMR 0x44050
+#define SO_WRITE_OFFSET_0 0x5280
+#define HSW_CS_GPR(n) (0x2600 + 8*(n))
+#define HSW_CS_GPR0 HSW_CS_GPR(0)
+#define HSW_CS_GPR1 HSW_CS_GPR(1)
+
+#define MI_LOAD_REGISTER_REG (0x2a << 23)
+#define MI_STORE_REGISTER_MEM (0x24 << 23)
+#define MI_ARB_ON_OFF (0x8 << 23)
+#define MI_DISPLAY_FLIP ((0x14 << 23) | 1)
+
+#define GFX_OP_PIPE_CONTROL((0x3<<29)|(0x3<<27)|(0x2<<24)|2)
+#define   PIPE_CONTROL_QW_WRITE(1<<14)
+#define   PIPE_CONTROL_LRI_POST_OP (1<<23)
+
 
 static int command_parser_version(int fd)
 {
@@ -50,101 +67,8 @@ static int command_parser_version(int fd)
return -1;
 }
 
-#define HSW_CS_GPR(n) (0x2600 + 8*(n))
-#define HSW_CS_GPR0 HSW_CS_GPR(0)
-#define HSW_CS_GPR1 HSW_CS_GPR(1)
-
-#define MI_LOAD_REGISTER_REG (0x2a << 23)
-#define MI_STORE_REGISTER_MEM (0x24 << 23)
-static void hsw_load_register_reg(void)
-{
-   uint32_t buf[16] = {
-   MI_LOAD_REGISTER_IMM | (5 - 2),
-   HSW_CS_GPR0,
-   0xabcdabcd,
-   HSW_CS_GPR1,
-   0xdeadbeef,
-
-   MI_STORE_REGISTER_MEM | (3 - 2),
-   HSW_CS_GPR1,
-   0, /* address0 */
-
-   MI_LOAD_REGISTER_REG | (3 - 2),
-   HSW_CS_GPR0,
-   HSW_CS_GPR1,
-
-   MI_STORE_REGISTER_MEM | (3 - 2),
-   HSW_CS_GPR1,
-   4, /* address1 */
-
-   MI_BATCH_BUFFER_END,
-   };
-   struct drm_i915_gem_execbuffer2 execbuf;
-   struct drm_i915_gem_exec_object2 obj[2];
-   struct drm_i915_gem_relocation_entry reloc[2];
-   int fd;
-
-   /* Open again to get a non-master file descriptor */
-   fd = drm_open_driver(DRIVER_INTEL);
-
-   igt_require(IS_HASWELL(intel_get_drm_devid(fd)));
-   igt_require(command_parser_version(fd) >= 7);
-
-   memset(obj, 0, sizeof(obj));
-   obj[0].handle = gem_create(fd, 4096);
-   obj[1].handle = gem_create(fd, 4096);
-   gem_write(fd, obj[1].handle, 0, buf, sizeof(buf));
-
-   memset(reloc, 0, sizeof(reloc));
-   reloc[0].offset = 7*sizeof(uint32_t);
-   reloc[0].target_handle = obj[0].handle;
-   reloc[0].delta = 0;
-   reloc[0].read_domains = I915_GEM_DOMAIN_INSTRUCTION;
-   reloc[0].write_domain = I915_GEM_DOMAIN_INSTRUCTION;
-   reloc[1].offset = 13*sizeof(uint32_t);
-   reloc[1].target_handle = obj[0].handle;
-   reloc[1].delta = sizeof(uint32_t);
-   reloc[1].read_domains = I915_GEM_DOMAIN_INSTRUCTION;
-   reloc[1].write_domain = I915_GEM_DOMAIN_INSTRUCTION;
-   obj[1].relocs_ptr = (uintptr_t)
-   obj[1].relocation_count = 2;
-
-   memset(, 0, sizeof(execbuf));
-   execbuf.buffers_ptr = (uintptr_t)obj;
-   execbuf.buffer_count = 2;
-   execbuf.batch_len = sizeof(buf);
-   execbuf.flags = I915_EXEC_RENDER;
-   gem_execbuf(fd, );
-   gem_close(fd, obj[1].handle);
-
-   gem_read(fd, obj[0].handle, 0, buf, 2*sizeof(buf[0]));
-   igt_assert_eq_u32(buf[0], 0xdeadbeef); /* before copy */
-   igt_assert_eq_u32(buf[1], 0xabcdabcd); /* after copy */
-
-   /* Now a couple of negative tests that should be filtered */
-   execbuf.buffer_count = 1;
-   execbuf.batch_len = 4*sizeof(buf[0]);
-
-   buf[0] = MI_LOAD_REGISTER_REG | (3 - 2);
-   buf[1] = HSW_CS_GPR0;
-   buf[2] = 0;
-   buf[3] = MI_BATCH_BUFFER_END;
-   gem_write(fd, obj[0].handle, 0, buf, execbuf.batch_len);
-   igt_assert_eq(__gem_execbuf(fd, ), -EINVAL);
-
-   buf[2] = DERRMR; /* master only */
-   gem_write(fd, obj[0].handle, 0, buf, execbuf.batch_len);
-   igt_assert_eq(__gem_execbuf(fd, ), -EINVAL);
-
-   buf[2] = 0x2038; /* RING_START: invalid */
-   gem_write(fd, obj[0].handle, 0, buf, execbuf.batch_len);
-   igt_assert_eq(__gem_execbuf(fd, ), -EINVAL);
-
-   close(fd);
-}
-
-static void exec_batch_patched(int fd, uint32_t cmd_bo, uint32_t *cmds,
-  int size, int patch_offset, uint64_t