[igt-dev] [PATCH i-g-t v2] lib/hang_ctx: Make use of dummyload library to create recursive batch

[igt-dev] [PATCH i-g-t v2] lib/hang_ctx: Make use of dummyload library to create recursive batch

[Antonio Argenziano]

An hanging batch is nothing more than a spinning batch that never gets
stopped, so re-use the routines implemented in dummyload.c.

v2:
	- Let caller decide spin loop size (Chris)
	- Now builds with meson.

Signed-off-by: Antonio Argenziano <antonio.argenziano@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 lib/igt_gt.c                 | 65 +++++++++-----------------------------------
 lib/igt_gt.h                 |  3 +-
 lib/meson.build              |  1 +
 tests/gem_concurrent_all.c   |  6 ++--
 tests/gem_ctx_exec.c         |  2 +-
 tests/gem_mmap_gtt.c         |  2 +-
 tests/gem_pread_after_blit.c |  2 +-
 tests/gem_reloc_vs_gpu.c     |  2 +-
 tests/gem_ringfill.c         |  2 +-
 tests/gem_shrink.c           |  2 +-
 tests/kms_flip.c             |  2 +-
 tests/kms_vblank.c           |  2 +-
 12 files changed, 27 insertions(+), 64 deletions(-)

diff --git a/lib/igt_gt.c b/lib/igt_gt.c
index 4569fd36..4ca4beef 100644
--- a/lib/igt_gt.c
+++ b/lib/igt_gt.c
@@ -40,6 +40,7 @@
 #include "ioctl_wrappers.h"
 #include "intel_reg.h"
 #include "intel_chipset.h"
+#include "igt_dummyload.h"
 
 /**
  * SECTION:igt_gt
@@ -264,14 +265,10 @@ igt_hang_t igt_hang_ctx(int fd,
 			unsigned flags,
 			uint64_t *offset)
 {
-	struct drm_i915_gem_relocation_entry reloc;
-	struct drm_i915_gem_execbuffer2 execbuf;
-	struct drm_i915_gem_exec_object2 exec;
+	igt_spin_t *spinning_batch;
+	struct igt_spin_factory opts = {};
 	struct drm_i915_gem_context_param param;
-	uint32_t b[16];
 	unsigned ban;
-	unsigned len;
-	int gen;
 
 	igt_require_hang_ring(fd, ring);
 
@@ -295,58 +292,22 @@ igt_hang_t igt_hang_ctx(int fd,
 	if ((flags & HANG_ALLOW_BAN) == 0)
 		context_set_ban(fd, ctx, 0);
 
-	memset(&reloc, 0, sizeof(reloc));
-	memset(&exec, 0, sizeof(exec));
-	memset(&execbuf, 0, sizeof(execbuf));
-
-	exec.handle = gem_create(fd, 4096);
-	exec.relocation_count = 1;
-	exec.relocs_ptr = to_user_pointer(&reloc);
-
-	memset(b, 0xc5, sizeof(b));
-
-	len = 0;
-	gen = intel_gen(intel_get_drm_devid(fd));
-	if (gen >= 8) {
-		b[len++] = MI_BATCH_BUFFER_START | 1 << 8 | 1;
-		b[len++] = 0;
-		b[len++] = 0;
-	} else if (gen >= 6) {
-		b[len++] = MI_BATCH_BUFFER_START | 1 << 8;
-		b[len++] = 0;
-	} else {
-		b[len++] = MI_BATCH_BUFFER_START | 2 << 6;
-		b[len] = 0;
-		if (gen < 4) {
-			b[len] |= 1;
-			reloc.delta = 1;
-		}
-		len++;
-	}
-	b[len++] = MI_BATCH_BUFFER_END;
-	b[len] = MI_NOOP;
-	gem_write(fd, exec.handle, 0, b, sizeof(b));
-
-	reloc.offset = sizeof(uint32_t);
-	reloc.target_handle = exec.handle;
-	reloc.read_domains = I915_GEM_DOMAIN_COMMAND;
-
-	execbuf.buffers_ptr = to_user_pointer(&exec);
-	execbuf.buffer_count = 1;
-	execbuf.flags = ring;
-	i915_execbuffer2_set_context_id(execbuf, ctx);
-	gem_execbuf(fd, &execbuf);
+	opts.ctx = ctx;
+	opts.engine = ring;
+	opts.flags |= (flags & HANG_SPIN_FAST) ? IGT_SPIN_FAST : 0; /* hangs faster */
+	spinning_batch = igt_spin_batch_factory(fd, &opts);
 
 	if (offset)
-		*offset = exec.offset;
+		*offset = (*spinning_batch).obj[1].offset; /* The batch is the last object */
 
-	return (igt_hang_t){ exec.handle, ctx, ban, flags };
+	return (igt_hang_t){ (*spinning_batch).obj[1].handle, ctx, ban, flags };
 }
 
 /**
  * igt_hang_ring:
  * @fd: open i915 drm file descriptor
  * @ring: execbuf ring flag
+ * @flags: set of flags to control execution
  *
  * This helper function injects a hanging batch into @ring. It returns a
  * #igt_hang_t structure which must be passed to igt_post_hang_ring() for
@@ -355,9 +316,9 @@ igt_hang_t igt_hang_ctx(int fd,
  * Returns:
  * Structure with helper internal state for igt_post_hang_ring().
  */
-igt_hang_t igt_hang_ring(int fd, int ring)
+igt_hang_t igt_hang_ring(int fd, int ring, unsigned flags)
 {
-	return igt_hang_ctx(fd, 0, ring, 0, NULL);
+	return igt_hang_ctx(fd, 0, ring, flags, NULL);
 }
 
 /**
@@ -425,7 +386,7 @@ hang_helper_process(pid_t pid, int fd)
 			exit(0);
 
 		igt_post_hang_ring(fd,
-				   igt_hang_ring(fd, I915_EXEC_DEFAULT));
+				   igt_hang_ring(fd, I915_EXEC_DEFAULT, 0));
 
 		sleep(1);
 	}
diff --git a/lib/igt_gt.h b/lib/igt_gt.h
index d44b7552..12a83a1b 100644
--- a/lib/igt_gt.h
+++ b/lib/igt_gt.h
@@ -50,8 +50,9 @@ igt_hang_t igt_hang_ctx(int fd,
 			uint64_t *offset);
 #define HANG_ALLOW_BAN 1
 #define HANG_ALLOW_CAPTURE 2
+#define HANG_SPIN_FAST 3
 
-igt_hang_t igt_hang_ring(int fd, int ring);
+igt_hang_t igt_hang_ring(int fd, int ring, unsigned flags);
 void igt_post_hang_ring(int fd, igt_hang_t arg);
 
 void igt_force_gpu_reset(int fd);
diff --git a/lib/meson.build b/lib/meson.build
index 1a355414..045f4a13 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -134,6 +134,7 @@ lib_igt_chipset = declare_dependency(link_with : lin_igt_chipset_build,
 
 lib_igt_perf_build = static_library('igt_perf',
 	['igt_perf.c'],
+    dependencies: [libdrm],
 	include_directories : inc)
 
 lib_igt_perf = declare_dependency(link_with : lib_igt_perf_build,
diff --git a/tests/gem_concurrent_all.c b/tests/gem_concurrent_all.c
index 3a1097ba..d327f78a 100644
--- a/tests/gem_concurrent_all.c
+++ b/tests/gem_concurrent_all.c
@@ -936,12 +936,12 @@ static igt_hang_t no_hang(void)
 
 static igt_hang_t bcs_hang(void)
 {
-	return igt_hang_ring(fd, I915_EXEC_BLT);
+	return igt_hang_ring(fd, I915_EXEC_BLT, 0);
 }
 
 static igt_hang_t rcs_hang(void)
 {
-	return igt_hang_ring(fd, I915_EXEC_RENDER);
+	return igt_hang_ring(fd, I915_EXEC_RENDER, 0);
 }
 
 static igt_hang_t all_hang(void)
@@ -961,7 +961,7 @@ static igt_hang_t all_hang(void)
 	execbuf.buffer_count = 1;
 
 	for_each_physical_engine(fd, engine) {
-		hang = igt_hang_ring(fd, engine);
+		hang = igt_hang_ring(fd, engine, 0);
 
 		execbuf.flags = engine;
 		__gem_execbuf(fd, &execbuf);
diff --git a/tests/gem_ctx_exec.c b/tests/gem_ctx_exec.c
index 1f8ed64d..8b126b38 100644
--- a/tests/gem_ctx_exec.c
+++ b/tests/gem_ctx_exec.c
@@ -195,7 +195,7 @@ igt_main
 		 * the last context is leaked at every reset.
 		 */
 		for (i = 0; i < 20; i++) {
-			igt_hang_t hang = igt_hang_ring(fd, I915_EXEC_RENDER);
+			igt_hang_t hang = igt_hang_ring(fd, I915_EXEC_RENDER, 0);
 			igt_assert(exec(fd, handle, I915_EXEC_RENDER, 0) == 0);
 			igt_assert(exec(fd, handle, I915_EXEC_RENDER, ctx_id) == 0);
 			igt_post_hang_ring(fd, hang);
diff --git a/tests/gem_mmap_gtt.c b/tests/gem_mmap_gtt.c
index fd60b8ff..43797f73 100644
--- a/tests/gem_mmap_gtt.c
+++ b/tests/gem_mmap_gtt.c
@@ -382,7 +382,7 @@ test_hang(int fd)
 		gem_close(fd, handle);
 	}
 
-	hang = igt_hang_ring(fd, I915_EXEC_RENDER);
+	hang = igt_hang_ring(fd, I915_EXEC_RENDER, 0);
 
 	do {
 		for (i = 0; i < OBJECT_SIZE / 64; i++) {
diff --git a/tests/gem_pread_after_blit.c b/tests/gem_pread_after_blit.c
index 6ef3ca20..1763ea6e 100644
--- a/tests/gem_pread_after_blit.c
+++ b/tests/gem_pread_after_blit.c
@@ -127,7 +127,7 @@ static igt_hang_t no_hang(int fd)
 
 static igt_hang_t bcs_hang(int fd)
 {
-	return igt_hang_ring(fd, batch->gen >= 6 ? I915_EXEC_BLT : I915_EXEC_DEFAULT);
+	return igt_hang_ring(fd, batch->gen >= 6 ? I915_EXEC_BLT : I915_EXEC_DEFAULT, 0);
 }
 
 static void do_test(int fd, int cache_level,
diff --git a/tests/gem_reloc_vs_gpu.c b/tests/gem_reloc_vs_gpu.c
index d421e434..c589ea63 100644
--- a/tests/gem_reloc_vs_gpu.c
+++ b/tests/gem_reloc_vs_gpu.c
@@ -190,7 +190,7 @@ static igt_hang_t no_hang(int fd)
 
 static igt_hang_t bcs_hang(int fd)
 {
-	return igt_hang_ring(fd, I915_EXEC_BLT);
+	return igt_hang_ring(fd, I915_EXEC_BLT, 0);
 }
 
 static void do_test(int fd, bool faulting_reloc,
diff --git a/tests/gem_ringfill.c b/tests/gem_ringfill.c
index c728e1cd..ceaee8c8 100644
--- a/tests/gem_ringfill.c
+++ b/tests/gem_ringfill.c
@@ -188,7 +188,7 @@ static void run_test(int fd, unsigned ring, unsigned flags, unsigned timeout)
 
 	memset(&hang, 0, sizeof(hang));
 	if (flags & HANG)
-		hang = igt_hang_ring(fd, ring & ~(3<<13));
+		hang = igt_hang_ring(fd, ring & ~(3<<13), 0);
 
 	if (flags & (CHILD | FORKED | BOMB)) {
 		int nchild;
diff --git a/tests/gem_shrink.c b/tests/gem_shrink.c
index 929e0426..94b833f5 100644
--- a/tests/gem_shrink.c
+++ b/tests/gem_shrink.c
@@ -208,7 +208,7 @@ static void hang(int fd, uint64_t alloc)
 		gem_execbuf(fd, &execbuf);
 	}
 
-	gem_close(fd, igt_hang_ring(fd, 0).handle);
+	gem_close(fd, igt_hang_ring(fd, 0, 0).handle);
 	for (int i = 0; i <= count; i++)
 		gem_madvise(fd, obj[i].handle, I915_MADV_DONTNEED);
 	munmap(obj, obj_size);
diff --git a/tests/kms_flip.c b/tests/kms_flip.c
index 3d6fe948..290bdd30 100644
--- a/tests/kms_flip.c
+++ b/tests/kms_flip.c
@@ -616,7 +616,7 @@ static void set_y_tiling(struct test_output *o, int fb_idx)
 
 static igt_hang_t hang_gpu(int fd)
 {
-	return igt_hang_ring(fd, I915_EXEC_DEFAULT);
+	return igt_hang_ring(fd, I915_EXEC_DEFAULT, 0);
 }
 
 static void unhang_gpu(int fd, igt_hang_t hang)
diff --git a/tests/kms_vblank.c b/tests/kms_vblank.c
index 2bee49de..676231f9 100644
--- a/tests/kms_vblank.c
+++ b/tests/kms_vblank.c
@@ -128,7 +128,7 @@ static void run_test(data_t *data, void (*testfunc)(data_t *, int, int))
 		 igt_output_name(output));
 
 	if (!(data->flags & NOHANG))
-		hang = igt_hang_ring(fd, I915_EXEC_DEFAULT);
+		hang = igt_hang_ring(fd, I915_EXEC_DEFAULT, 0);
 
 	if (data->flags & BUSY) {
 		union drm_wait_vblank vbl;
-- 
2.16.2

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v2] lib/hang_ctx: Make use of dummyload library to create recursive batch

[Chris Wilson]

Quoting Antonio Argenziano (2018-07-10 16:31:55)
> An hanging batch is nothing more than a spinning batch that never gets
> stopped, so re-use the routines implemented in dummyload.c.
> 
> v2:
>         - Let caller decide spin loop size (Chris)
>         - Now builds with meson.
> 
> Signed-off-by: Antonio Argenziano <antonio.argenziano@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  lib/igt_gt.c                 | 65 +++++++++-----------------------------------
>  lib/igt_gt.h                 |  3 +-
>  lib/meson.build              |  1 +
>  tests/gem_concurrent_all.c   |  6 ++--
>  tests/gem_ctx_exec.c         |  2 +-
>  tests/gem_mmap_gtt.c         |  2 +-
>  tests/gem_pread_after_blit.c |  2 +-
>  tests/gem_reloc_vs_gpu.c     |  2 +-
>  tests/gem_ringfill.c         |  2 +-
>  tests/gem_shrink.c           |  2 +-
>  tests/kms_flip.c             |  2 +-
>  tests/kms_vblank.c           |  2 +-
>  12 files changed, 27 insertions(+), 64 deletions(-)
> 
> diff --git a/lib/igt_gt.c b/lib/igt_gt.c
> index 4569fd36..4ca4beef 100644
> --- a/lib/igt_gt.c
> +++ b/lib/igt_gt.c
> @@ -40,6 +40,7 @@
>  #include "ioctl_wrappers.h"
>  #include "intel_reg.h"
>  #include "intel_chipset.h"
> +#include "igt_dummyload.h"
>  
>  /**
>   * SECTION:igt_gt
> @@ -264,14 +265,10 @@ igt_hang_t igt_hang_ctx(int fd,
>                         unsigned flags,
>                         uint64_t *offset)
>  {
> -       struct drm_i915_gem_relocation_entry reloc;
> -       struct drm_i915_gem_execbuffer2 execbuf;
> -       struct drm_i915_gem_exec_object2 exec;
> +       igt_spin_t *spinning_batch;
> +       struct igt_spin_factory opts = {};
>         struct drm_i915_gem_context_param param;
> -       uint32_t b[16];
>         unsigned ban;
> -       unsigned len;
> -       int gen;
>  
>         igt_require_hang_ring(fd, ring);
>  
> @@ -295,58 +292,22 @@ igt_hang_t igt_hang_ctx(int fd,
>         if ((flags & HANG_ALLOW_BAN) == 0)
>                 context_set_ban(fd, ctx, 0);
>  
> -       memset(&reloc, 0, sizeof(reloc));
> -       memset(&exec, 0, sizeof(exec));
> -       memset(&execbuf, 0, sizeof(execbuf));
> -
> -       exec.handle = gem_create(fd, 4096);
> -       exec.relocation_count = 1;
> -       exec.relocs_ptr = to_user_pointer(&reloc);
> -
> -       memset(b, 0xc5, sizeof(b));
> -
> -       len = 0;
> -       gen = intel_gen(intel_get_drm_devid(fd));
> -       if (gen >= 8) {
> -               b[len++] = MI_BATCH_BUFFER_START | 1 << 8 | 1;
> -               b[len++] = 0;
> -               b[len++] = 0;
> -       } else if (gen >= 6) {
> -               b[len++] = MI_BATCH_BUFFER_START | 1 << 8;
> -               b[len++] = 0;
> -       } else {
> -               b[len++] = MI_BATCH_BUFFER_START | 2 << 6;
> -               b[len] = 0;
> -               if (gen < 4) {
> -                       b[len] |= 1;
> -                       reloc.delta = 1;
> -               }
> -               len++;
> -       }
> -       b[len++] = MI_BATCH_BUFFER_END;
> -       b[len] = MI_NOOP;
> -       gem_write(fd, exec.handle, 0, b, sizeof(b));
> -
> -       reloc.offset = sizeof(uint32_t);
> -       reloc.target_handle = exec.handle;
> -       reloc.read_domains = I915_GEM_DOMAIN_COMMAND;
> -
> -       execbuf.buffers_ptr = to_user_pointer(&exec);
> -       execbuf.buffer_count = 1;
> -       execbuf.flags = ring;
> -       i915_execbuffer2_set_context_id(execbuf, ctx);
> -       gem_execbuf(fd, &execbuf);
> +       opts.ctx = ctx;
> +       opts.engine = ring;
> +       opts.flags |= (flags & HANG_SPIN_FAST) ? IGT_SPIN_FAST : 0; /* hangs faster */
> +       spinning_batch = igt_spin_batch_factory(fd, &opts);
>  
>         if (offset)
> -               *offset = exec.offset;
> +               *offset = (*spinning_batch).obj[1].offset; /* The batch is the last object */

K&R came up with this shorthand I think you should use, ->

> diff --git a/lib/igt_gt.h b/lib/igt_gt.h
> index d44b7552..12a83a1b 100644
> --- a/lib/igt_gt.h
> +++ b/lib/igt_gt.h
> @@ -50,8 +50,9 @@ igt_hang_t igt_hang_ctx(int fd,
>                         uint64_t *offset);
>  #define HANG_ALLOW_BAN 1
>  #define HANG_ALLOW_CAPTURE 2
> +#define HANG_SPIN_FAST 3

Must be a power-of-two, or wrap in BIT() which doesn't exist yet...

Overall I am still dubious there is any reason to set this flag. The
ratelimiting step here isn't the speed at which the batch spins, but the
time for hangcheck to kick in. Where a fast spinner is interesting is
the latency in responding to terminating the spinner (which never
happens for a hang!).
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v2] lib/hang_ctx: Make use of dummyload library to create recursive batch

[Antonio Argenziano]

On 10/07/18 08:38, Chris Wilson wrote:
> Quoting Antonio Argenziano (2018-07-10 16:31:55)
>> An hanging batch is nothing more than a spinning batch that never gets
>> stopped, so re-use the routines implemented in dummyload.c.
>>
>> v2:
>>          - Let caller decide spin loop size (Chris)
>>          - Now builds with meson.
>>
>> Signed-off-by: Antonio Argenziano <antonio.argenziano@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> ---
>>   lib/igt_gt.c                 | 65 +++++++++-----------------------------------
>>   lib/igt_gt.h                 |  3 +-
>>   lib/meson.build              |  1 +
>>   tests/gem_concurrent_all.c   |  6 ++--
>>   tests/gem_ctx_exec.c         |  2 +-
>>   tests/gem_mmap_gtt.c         |  2 +-
>>   tests/gem_pread_after_blit.c |  2 +-
>>   tests/gem_reloc_vs_gpu.c     |  2 +-
>>   tests/gem_ringfill.c         |  2 +-
>>   tests/gem_shrink.c           |  2 +-
>>   tests/kms_flip.c             |  2 +-
>>   tests/kms_vblank.c           |  2 +-
>>   12 files changed, 27 insertions(+), 64 deletions(-)
>>
>> diff --git a/lib/igt_gt.c b/lib/igt_gt.c
>> index 4569fd36..4ca4beef 100644
>> --- a/lib/igt_gt.c
>> +++ b/lib/igt_gt.c
>> @@ -40,6 +40,7 @@
>>   #include "ioctl_wrappers.h"
>>   #include "intel_reg.h"
>>   #include "intel_chipset.h"
>> +#include "igt_dummyload.h"
>>   
>>   /**
>>    * SECTION:igt_gt
>> @@ -264,14 +265,10 @@ igt_hang_t igt_hang_ctx(int fd,
>>                          unsigned flags,
>>                          uint64_t *offset)
>>   {
>> -       struct drm_i915_gem_relocation_entry reloc;
>> -       struct drm_i915_gem_execbuffer2 execbuf;
>> -       struct drm_i915_gem_exec_object2 exec;
>> +       igt_spin_t *spinning_batch;
>> +       struct igt_spin_factory opts = {};
>>          struct drm_i915_gem_context_param param;
>> -       uint32_t b[16];
>>          unsigned ban;
>> -       unsigned len;
>> -       int gen;
>>   
>>          igt_require_hang_ring(fd, ring);
>>   
>> @@ -295,58 +292,22 @@ igt_hang_t igt_hang_ctx(int fd,
>>          if ((flags & HANG_ALLOW_BAN) == 0)
>>                  context_set_ban(fd, ctx, 0);
>>   
>> -       memset(&reloc, 0, sizeof(reloc));
>> -       memset(&exec, 0, sizeof(exec));
>> -       memset(&execbuf, 0, sizeof(execbuf));
>> -
>> -       exec.handle = gem_create(fd, 4096);
>> -       exec.relocation_count = 1;
>> -       exec.relocs_ptr = to_user_pointer(&reloc);
>> -
>> -       memset(b, 0xc5, sizeof(b));
>> -
>> -       len = 0;
>> -       gen = intel_gen(intel_get_drm_devid(fd));
>> -       if (gen >= 8) {
>> -               b[len++] = MI_BATCH_BUFFER_START | 1 << 8 | 1;
>> -               b[len++] = 0;
>> -               b[len++] = 0;
>> -       } else if (gen >= 6) {
>> -               b[len++] = MI_BATCH_BUFFER_START | 1 << 8;
>> -               b[len++] = 0;
>> -       } else {
>> -               b[len++] = MI_BATCH_BUFFER_START | 2 << 6;
>> -               b[len] = 0;
>> -               if (gen < 4) {
>> -                       b[len] |= 1;
>> -                       reloc.delta = 1;
>> -               }
>> -               len++;
>> -       }
>> -       b[len++] = MI_BATCH_BUFFER_END;
>> -       b[len] = MI_NOOP;
>> -       gem_write(fd, exec.handle, 0, b, sizeof(b));
>> -
>> -       reloc.offset = sizeof(uint32_t);
>> -       reloc.target_handle = exec.handle;
>> -       reloc.read_domains = I915_GEM_DOMAIN_COMMAND;
>> -
>> -       execbuf.buffers_ptr = to_user_pointer(&exec);
>> -       execbuf.buffer_count = 1;
>> -       execbuf.flags = ring;
>> -       i915_execbuffer2_set_context_id(execbuf, ctx);
>> -       gem_execbuf(fd, &execbuf);
>> +       opts.ctx = ctx;
>> +       opts.engine = ring;
>> +       opts.flags |= (flags & HANG_SPIN_FAST) ? IGT_SPIN_FAST : 0; /* hangs faster */
>> +       spinning_batch = igt_spin_batch_factory(fd, &opts);
>>   
>>          if (offset)
>> -               *offset = exec.offset;
>> +               *offset = (*spinning_batch).obj[1].offset; /* The batch is the last object */
> 
> K&R came up with this shorthand I think you should use, ->

Witchcraft!

> 
>> diff --git a/lib/igt_gt.h b/lib/igt_gt.h
>> index d44b7552..12a83a1b 100644
>> --- a/lib/igt_gt.h
>> +++ b/lib/igt_gt.h
>> @@ -50,8 +50,9 @@ igt_hang_t igt_hang_ctx(int fd,
>>                          uint64_t *offset);
>>   #define HANG_ALLOW_BAN 1
>>   #define HANG_ALLOW_CAPTURE 2
>> +#define HANG_SPIN_FAST 3
> 
> Must be a power-of-two, or wrap in BIT() which doesn't exist yet...

Should probably be more awake when I do this things...

> 
> Overall I am still dubious there is any reason to set this flag. The
> ratelimiting step here isn't the speed at which the batch spins, but the
> time for hangcheck to kick in. Where a fast spinner is interesting is
> the latency in responding to terminating the spinner (which never
> happens for a hang!).

The flag makes sense to me because hangcheck will always sample the same 
head on a very tight loop like the fast spinner. The current 
implementation of hangcheck will also declare the engine hung much 
quicker. In general I think we should expose different hanging batch to 
the driver to make sure it can react to the different situations.

> -Chris
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✓ Fi.CI.BAT: success for lib/hang_ctx: Make use of dummyload library to create recursive batch (rev2)

[Patchwork]

== Series Details ==

Series: lib/hang_ctx: Make use of dummyload library to create recursive batch (rev2)
URL   : https://patchwork.freedesktop.org/series/46209/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4463 -> IGTPW_1551 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/46209/revisions/2/mbox/

== Known issues ==

  Here are the changes found in IGTPW_1551 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
      fi-bxt-dsi:         PASS -> INCOMPLETE (fdo#103927)

    
    ==== Possible fixes ====

    igt@kms_frontbuffer_tracking@basic:
      fi-hsw-peppy:       DMESG-FAIL (fdo#102614, fdo#106103) -> PASS

    igt@prime_vgem@basic-fence-flip:
      fi-ilk-650:         FAIL (fdo#104008) -> PASS

    
  fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
  fdo#106103 https://bugs.freedesktop.org/show_bug.cgi?id=106103


== Participating hosts (47 -> 42) ==

  Missing    (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * IGT: IGT_4544 -> IGTPW_1551

  CI_DRM_4463: 756ded1fe53d1449d239c6b34f97e03e478a8a38 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1551: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1551/
  IGT_4544: 764160f214cd916ddb79408b9f28ac0ad2df40e0 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1551/issues.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v2] lib/hang_ctx: Make use of dummyload library to create recursive batch

[Chris Wilson]

Quoting Antonio Argenziano (2018-07-10 16:48:38)
> On 10/07/18 08:38, Chris Wilson wrote:
> > Quoting Antonio Argenziano (2018-07-10 16:31:55)
> > Overall I am still dubious there is any reason to set this flag. The
> > ratelimiting step here isn't the speed at which the batch spins, but the
> > time for hangcheck to kick in. Where a fast spinner is interesting is
> > the latency in responding to terminating the spinner (which never
> > happens for a hang!).
> 
> The flag makes sense to me because hangcheck will always sample the same 
> head on a very tight loop like the fast spinner. The current 
> implementation of hangcheck will also declare the engine hung much 
> quicker.

True, but I don't think much more than 1 or 2 extra ticks. In the grand
scheme of things, using the pollable spinner + manual hang injection is
far far far far far far quicker and precise.

> In general I think we should expose different hanging batch to 
> the driver to make sure it can react to the different situations.

Harder to call (where do you draw the line between meaningful and
hanging batch), but such construction is better done inside the test
than inside a general purpose library (as it is no longer general
purpose).
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✗ Fi.CI.IGT: failure for lib/hang_ctx: Make use of dummyload library to create recursive batch (rev2)

[Patchwork]

== Series Details ==

Series: lib/hang_ctx: Make use of dummyload library to create recursive batch (rev2)
URL   : https://patchwork.freedesktop.org/series/46209/
State : failure

== Summary ==

= CI Bug Log - changes from IGT_4544_full -> IGTPW_1551_full =

== Summary - FAILURE ==

  Serious unknown changes coming with IGTPW_1551_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_1551_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/46209/revisions/2/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in IGTPW_1551_full:

  === IGT changes ===

    ==== Possible regressions ====

    igt@gem_exec_schedule@preemptive-hang-bsd1:
      shard-kbl:          PASS -> FAIL +4

    igt@gem_exec_schedule@preemptive-hang-render:
      shard-apl:          PASS -> FAIL +3

    igt@gem_exec_schedule@preemptive-hang-vebox:
      shard-glk:          PASS -> FAIL +3

    igt@kms_pipe_crc_basic@hang-read-crc-pipe-b:
      shard-snb:          PASS -> DMESG-FAIL

    
    ==== Warnings ====

    igt@gem_exec_schedule@deep-vebox:
      shard-kbl:          SKIP -> PASS +1

    igt@kms_busy@extended-pageflip-hang-oldfb-render-a:
      shard-snb:          PASS -> SKIP +2

    
== Known issues ==

  Here are the changes found in IGTPW_1551_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_cursor_legacy@2x-long-nonblocking-modeset-vs-cursor-atomic:
      shard-glk:          PASS -> FAIL (fdo#106509)

    igt@kms_cursor_legacy@2x-nonblocking-modeset-vs-cursor-atomic:
      shard-glk:          PASS -> FAIL (fdo#105454, fdo#106509)

    igt@kms_cursor_legacy@cursor-vs-flip-atomic:
      shard-hsw:          PASS -> FAIL (fdo#103355)

    igt@kms_cursor_legacy@pipe-a-forked-move:
      shard-snb:          PASS -> INCOMPLETE (fdo#105411)

    igt@kms_flip@plain-flip-fb-recreate-interruptible:
      shard-glk:          PASS -> FAIL (fdo#100368) +1

    igt@kms_flip_tiling@flip-to-y-tiled:
      shard-glk:          PASS -> FAIL (fdo#107161, fdo#103822)

    igt@kms_flip_tiling@flip-x-tiled:
      shard-glk:          PASS -> FAIL (fdo#107161)

    igt@kms_vblank@pipe-b-ts-continuation-suspend:
      shard-kbl:          PASS -> INCOMPLETE (fdo#103665)

    igt@testdisplay:
      shard-glk:          PASS -> INCOMPLETE (k.org#198133, fdo#103359)

    
    ==== Possible fixes ====

    igt@kms_flip_tiling@flip-y-tiled:
      shard-glk:          FAIL (fdo#107161, fdo#103822) -> PASS

    igt@pm_rpm@system-suspend:
      shard-kbl:          INCOMPLETE (fdo#103665) -> PASS

    
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#103355 https://bugs.freedesktop.org/show_bug.cgi?id=103355
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#103822 https://bugs.freedesktop.org/show_bug.cgi?id=103822
  fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
  fdo#105454 https://bugs.freedesktop.org/show_bug.cgi?id=105454
  fdo#106509 https://bugs.freedesktop.org/show_bug.cgi?id=106509
  fdo#107161 https://bugs.freedesktop.org/show_bug.cgi?id=107161
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * IGT: IGT_4544 -> IGTPW_1551
    * Linux: CI_DRM_4453 -> CI_DRM_4463

  CI_DRM_4453: 940e07c2a136b53965e4a6728e01e1638f5ba2de @ git://anongit.freedesktop.org/gfx-ci/linux
  CI_DRM_4463: 756ded1fe53d1449d239c6b34f97e03e478a8a38 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1551: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1551/
  IGT_4544: 764160f214cd916ddb79408b9f28ac0ad2df40e0 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1551/shards.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v2] lib/hang_ctx: Make use of dummyload library to create recursive batch

[Antonio Argenziano]

On 10/07/18 08:59, Chris Wilson wrote:
> Quoting Antonio Argenziano (2018-07-10 16:48:38)
>> On 10/07/18 08:38, Chris Wilson wrote:
>>> Quoting Antonio Argenziano (2018-07-10 16:31:55)
>>> Overall I am still dubious there is any reason to set this flag. The
>>> ratelimiting step here isn't the speed at which the batch spins, but the
>>> time for hangcheck to kick in. Where a fast spinner is interesting is
>>> the latency in responding to terminating the spinner (which never
>>> happens for a hang!).
>>
>> The flag makes sense to me because hangcheck will always sample the same
>> head on a very tight loop like the fast spinner. The current
>> implementation of hangcheck will also declare the engine hung much
>> quicker.
> 
> True, but I don't think much more than 1 or 2 extra ticks. In the grand
> scheme of things, using the pollable spinner + manual hang injection is
> far far far far far far quicker and precise.

Agree. Maybe this should be our preferred method for triggering hangs.

> 
>> In general I think we should expose different hanging batch to
>> the driver to make sure it can react to the different situations.
> 
> Harder to call (where do you draw the line between meaningful and
> hanging batch), but such construction is better done inside the test
> than inside a general purpose library (as it is no longer general
> purpose).

Makes sense, I was thinking of just some basic tests anyway.

Antonio

> -Chris
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev