[i-g-t,v6,4/4] igt/kms_busy.c: Use new igt_spin_batch
diff mbox

Message ID 1479147882-27003-5-git-send-email-abdiel.janulgue@linux.intel.com
State New
Headers show

Commit Message

Abdiel Janulgue Nov. 14, 2016, 6:24 p.m. UTC
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
---
 tests/kms_busy.c | 75 +++-----------------------------------------------------
 1 file changed, 4 insertions(+), 71 deletions(-)

Comments

Tomeu Vizoso Nov. 15, 2016, 8:45 a.m. UTC | #1
Hi Abdiel,

here running the whole of kms_busy causes all subtests after the first
one to be skipped due to:

Test requirement not met in function __real_main164, file
../../intel-gpu-tools/tests/kms_busy.c:195:
Test requirement: gem_has_ring(display.drm_fd, e->exec_id | e->flags)

If I run the subtests individually, the do get to run, so I guess
there's some issue with cleanup.

Btw, do you know why patchwork doesn't know about v6?

https://patchwork.freedesktop.org/series/15155/

I think it would be good to make sure that all commit messages contain
the why of the change, and a cover letter as the one generated by
git-send-email would be good to have, even if with very succinct
contents.

Thanks,

Tomeu

On 14 November 2016 at 19:24, Abdiel Janulgue
<abdiel.janulgue@linux.intel.com> wrote:
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
> ---
>  tests/kms_busy.c | 75 +++-----------------------------------------------------
>  1 file changed, 4 insertions(+), 71 deletions(-)
>
> diff --git a/tests/kms_busy.c b/tests/kms_busy.c
> index b555f99..d0fe412 100644
> --- a/tests/kms_busy.c
> +++ b/tests/kms_busy.c
> @@ -78,73 +78,6 @@ static void do_cleanup_display(igt_display_t *dpy)
>         igt_display_commit2(dpy, dpy->is_atomic ? COMMIT_ATOMIC : COMMIT_LEGACY);
>  }
>
> -static uint32_t *
> -make_fb_busy(igt_display_t *dpy, unsigned ring, const struct igt_fb *fb)
> -{
> -       const int gen = intel_gen(intel_get_drm_devid(dpy->drm_fd));
> -       struct drm_i915_gem_exec_object2 obj[2];
> -#define SCRATCH 0
> -#define BATCH 1
> -       struct drm_i915_gem_relocation_entry reloc[2];
> -       struct drm_i915_gem_execbuffer2 execbuf;
> -       uint32_t *batch;
> -       int i;
> -
> -       memset(&execbuf, 0, sizeof(execbuf));
> -       execbuf.buffers_ptr = (uintptr_t)obj;
> -       execbuf.buffer_count = 2;
> -       execbuf.flags = ring;
> -
> -       memset(obj, 0, sizeof(obj));
> -       obj[SCRATCH].handle = fb->gem_handle;
> -
> -       obj[BATCH].handle = gem_create(dpy->drm_fd, 4096);
> -       obj[BATCH].relocs_ptr = (uintptr_t)reloc;
> -       obj[BATCH].relocation_count = 2;
> -       memset(reloc, 0, sizeof(reloc));
> -       reloc[0].target_handle = obj[BATCH].handle; /* recurse */
> -       reloc[0].presumed_offset = 0;
> -       reloc[0].offset = sizeof(uint32_t);
> -       reloc[0].delta = 0;
> -       reloc[0].read_domains = I915_GEM_DOMAIN_COMMAND;
> -       reloc[0].write_domain = 0;
> -
> -       batch = gem_mmap__wc(dpy->drm_fd,
> -                            obj[BATCH].handle, 0, 4096, PROT_WRITE);
> -       gem_set_domain(dpy->drm_fd, obj[BATCH].handle,
> -                      I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
> -
> -       batch[i = 0] = MI_BATCH_BUFFER_START;
> -       if (gen >= 8) {
> -               batch[i] |= 1 << 8 | 1;
> -               batch[++i] = 0;
> -               batch[++i] = 0;
> -       } else if (gen >= 6) {
> -               batch[i] |= 1 << 8;
> -               batch[++i] = 0;
> -       } else {
> -               batch[i] |= 2 << 6;
> -               batch[++i] = 0;
> -               if (gen < 4) {
> -                       batch[i] |= 1;
> -                       reloc[0].delta = 1;
> -               }
> -       }
> -
> -       /* dummy write to fb */
> -       reloc[1].target_handle = obj[SCRATCH].handle;
> -       reloc[1].presumed_offset = 0;
> -       reloc[1].offset = 1024;
> -       reloc[1].delta = 0;
> -       reloc[1].read_domains = I915_GEM_DOMAIN_RENDER;
> -       reloc[1].write_domain = I915_GEM_DOMAIN_RENDER;
> -
> -       gem_execbuf(dpy->drm_fd, &execbuf);
> -       gem_close(dpy->drm_fd, obj[BATCH].handle);
> -
> -       return batch;
> -}
> -
>  static void finish_fb_busy(uint32_t *batch, int msecs)
>  {
>         struct timespec tv = { 0, msecs * 1000 * 1000 };
> @@ -165,9 +98,8 @@ static void flip_to_fb(igt_display_t *dpy, int pipe,
>         struct pollfd pfd = { .fd = dpy->drm_fd, .events = POLLIN };
>         struct timespec tv = { 1, 0 };
>         struct drm_event_vblank ev;
> -       uint32_t *batch;
> -
> -       batch = make_fb_busy(dpy, ring, fb);
> +       igt_spin_t t = igt_spin_batch(dpy->drm_fd, -1,
> +                                     ring, fb->gem_handle);
>         igt_fork(child, 1) {
>                 igt_assert(gem_bo_busy(dpy->drm_fd, fb->gem_handle));
>                 do_or_die(drmModePageFlip(dpy->drm_fd,
> @@ -179,7 +111,8 @@ static void flip_to_fb(igt_display_t *dpy, int pipe,
>         }
>         igt_assert_f(nanosleep(&tv, NULL) == -1,
>                      "flip to %s blocked waiting for busy fb", name);
> -       finish_fb_busy(batch, 2*TIMEOUT);
> +       finish_fb_busy(t.batch, 2*TIMEOUT);
> +       igt_post_spin_batch(dpy->drm_fd, t);
>         igt_waitchildren();
>         igt_assert(read(dpy->drm_fd, &ev, sizeof(ev)) == sizeof(ev));
>         igt_assert(poll(&pfd, 1, 0) == 0);
> --
> 2.7.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Abdiel Janulgue Nov. 15, 2016, 1:19 p.m. UTC | #2
Hi

On 15.11.2016 10:45, Tomeu Vizoso wrote:
> Hi Abdiel,
> 
> here running the whole of kms_busy causes all subtests after the first
> one to be skipped due to:
> 
> Test requirement not met in function __real_main164, file
> ../../intel-gpu-tools/tests/kms_busy.c:195:
> Test requirement: gem_has_ring(display.drm_fd, e->exec_id | e->flags)
> 
> If I run the subtests individually, the do get to run, so I guess
> there's some issue with cleanup.

The recursive batch with dependency bo works fine with BDW, SKL and up.
It's broken on HSW :( Found the problem, sending a fix soon.

Thanks for taking time to review this!

> 
> Btw, do you know why patchwork doesn't know about v6?
> 
> https://patchwork.freedesktop.org/series/15155/
> 
> I think it would be good to make sure that all commit messages contain
> the why of the change, and a cover letter as the one generated by
> git-send-email would be good to have, even if with very succinct
> contents.

Forgot to include the subject in git-send-email. Sorry about that...

> 
> Thanks,
> 
> Tomeu
> 
> On 14 November 2016 at 19:24, Abdiel Janulgue
> <abdiel.janulgue@linux.intel.com> wrote:
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
>> ---
>>  tests/kms_busy.c | 75 +++-----------------------------------------------------
>>  1 file changed, 4 insertions(+), 71 deletions(-)
>>
>> diff --git a/tests/kms_busy.c b/tests/kms_busy.c
>> index b555f99..d0fe412 100644
>> --- a/tests/kms_busy.c
>> +++ b/tests/kms_busy.c
>> @@ -78,73 +78,6 @@ static void do_cleanup_display(igt_display_t *dpy)
>>         igt_display_commit2(dpy, dpy->is_atomic ? COMMIT_ATOMIC : COMMIT_LEGACY);
>>  }
>>
>> -static uint32_t *
>> -make_fb_busy(igt_display_t *dpy, unsigned ring, const struct igt_fb *fb)
>> -{
>> -       const int gen = intel_gen(intel_get_drm_devid(dpy->drm_fd));
>> -       struct drm_i915_gem_exec_object2 obj[2];
>> -#define SCRATCH 0
>> -#define BATCH 1
>> -       struct drm_i915_gem_relocation_entry reloc[2];
>> -       struct drm_i915_gem_execbuffer2 execbuf;
>> -       uint32_t *batch;
>> -       int i;
>> -
>> -       memset(&execbuf, 0, sizeof(execbuf));
>> -       execbuf.buffers_ptr = (uintptr_t)obj;
>> -       execbuf.buffer_count = 2;
>> -       execbuf.flags = ring;
>> -
>> -       memset(obj, 0, sizeof(obj));
>> -       obj[SCRATCH].handle = fb->gem_handle;
>> -
>> -       obj[BATCH].handle = gem_create(dpy->drm_fd, 4096);
>> -       obj[BATCH].relocs_ptr = (uintptr_t)reloc;
>> -       obj[BATCH].relocation_count = 2;
>> -       memset(reloc, 0, sizeof(reloc));
>> -       reloc[0].target_handle = obj[BATCH].handle; /* recurse */
>> -       reloc[0].presumed_offset = 0;
>> -       reloc[0].offset = sizeof(uint32_t);
>> -       reloc[0].delta = 0;
>> -       reloc[0].read_domains = I915_GEM_DOMAIN_COMMAND;
>> -       reloc[0].write_domain = 0;
>> -
>> -       batch = gem_mmap__wc(dpy->drm_fd,
>> -                            obj[BATCH].handle, 0, 4096, PROT_WRITE);
>> -       gem_set_domain(dpy->drm_fd, obj[BATCH].handle,
>> -                      I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
>> -
>> -       batch[i = 0] = MI_BATCH_BUFFER_START;
>> -       if (gen >= 8) {
>> -               batch[i] |= 1 << 8 | 1;
>> -               batch[++i] = 0;
>> -               batch[++i] = 0;
>> -       } else if (gen >= 6) {
>> -               batch[i] |= 1 << 8;
>> -               batch[++i] = 0;
>> -       } else {
>> -               batch[i] |= 2 << 6;
>> -               batch[++i] = 0;
>> -               if (gen < 4) {
>> -                       batch[i] |= 1;
>> -                       reloc[0].delta = 1;
>> -               }
>> -       }
>> -
>> -       /* dummy write to fb */
>> -       reloc[1].target_handle = obj[SCRATCH].handle;
>> -       reloc[1].presumed_offset = 0;
>> -       reloc[1].offset = 1024;
>> -       reloc[1].delta = 0;
>> -       reloc[1].read_domains = I915_GEM_DOMAIN_RENDER;
>> -       reloc[1].write_domain = I915_GEM_DOMAIN_RENDER;
>> -
>> -       gem_execbuf(dpy->drm_fd, &execbuf);
>> -       gem_close(dpy->drm_fd, obj[BATCH].handle);
>> -
>> -       return batch;
>> -}
>> -
>>  static void finish_fb_busy(uint32_t *batch, int msecs)
>>  {
>>         struct timespec tv = { 0, msecs * 1000 * 1000 };
>> @@ -165,9 +98,8 @@ static void flip_to_fb(igt_display_t *dpy, int pipe,
>>         struct pollfd pfd = { .fd = dpy->drm_fd, .events = POLLIN };
>>         struct timespec tv = { 1, 0 };
>>         struct drm_event_vblank ev;
>> -       uint32_t *batch;
>> -
>> -       batch = make_fb_busy(dpy, ring, fb);
>> +       igt_spin_t t = igt_spin_batch(dpy->drm_fd, -1,
>> +                                     ring, fb->gem_handle);
>>         igt_fork(child, 1) {
>>                 igt_assert(gem_bo_busy(dpy->drm_fd, fb->gem_handle));
>>                 do_or_die(drmModePageFlip(dpy->drm_fd,
>> @@ -179,7 +111,8 @@ static void flip_to_fb(igt_display_t *dpy, int pipe,
>>         }
>>         igt_assert_f(nanosleep(&tv, NULL) == -1,
>>                      "flip to %s blocked waiting for busy fb", name);
>> -       finish_fb_busy(batch, 2*TIMEOUT);
>> +       finish_fb_busy(t.batch, 2*TIMEOUT);
>> +       igt_post_spin_batch(dpy->drm_fd, t);
>>         igt_waitchildren();
>>         igt_assert(read(dpy->drm_fd, &ev, sizeof(ev)) == sizeof(ev));
>>         igt_assert(poll(&pfd, 1, 0) == 0);
>> --
>> 2.7.4
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>

Patch
diff mbox

diff --git a/tests/kms_busy.c b/tests/kms_busy.c
index b555f99..d0fe412 100644
--- a/tests/kms_busy.c
+++ b/tests/kms_busy.c
@@ -78,73 +78,6 @@  static void do_cleanup_display(igt_display_t *dpy)
 	igt_display_commit2(dpy, dpy->is_atomic ? COMMIT_ATOMIC : COMMIT_LEGACY);
 }
 
-static uint32_t *
-make_fb_busy(igt_display_t *dpy, unsigned ring, const struct igt_fb *fb)
-{
-	const int gen = intel_gen(intel_get_drm_devid(dpy->drm_fd));
-	struct drm_i915_gem_exec_object2 obj[2];
-#define SCRATCH 0
-#define BATCH 1
-	struct drm_i915_gem_relocation_entry reloc[2];
-	struct drm_i915_gem_execbuffer2 execbuf;
-	uint32_t *batch;
-	int i;
-
-	memset(&execbuf, 0, sizeof(execbuf));
-	execbuf.buffers_ptr = (uintptr_t)obj;
-	execbuf.buffer_count = 2;
-	execbuf.flags = ring;
-
-	memset(obj, 0, sizeof(obj));
-	obj[SCRATCH].handle = fb->gem_handle;
-
-	obj[BATCH].handle = gem_create(dpy->drm_fd, 4096);
-	obj[BATCH].relocs_ptr = (uintptr_t)reloc;
-	obj[BATCH].relocation_count = 2;
-	memset(reloc, 0, sizeof(reloc));
-	reloc[0].target_handle = obj[BATCH].handle; /* recurse */
-	reloc[0].presumed_offset = 0;
-	reloc[0].offset = sizeof(uint32_t);
-	reloc[0].delta = 0;
-	reloc[0].read_domains = I915_GEM_DOMAIN_COMMAND;
-	reloc[0].write_domain = 0;
-
-	batch = gem_mmap__wc(dpy->drm_fd,
-			     obj[BATCH].handle, 0, 4096, PROT_WRITE);
-	gem_set_domain(dpy->drm_fd, obj[BATCH].handle,
-		       I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
-
-	batch[i = 0] = MI_BATCH_BUFFER_START;
-	if (gen >= 8) {
-		batch[i] |= 1 << 8 | 1;
-		batch[++i] = 0;
-		batch[++i] = 0;
-	} else if (gen >= 6) {
-		batch[i] |= 1 << 8;
-		batch[++i] = 0;
-	} else {
-		batch[i] |= 2 << 6;
-		batch[++i] = 0;
-		if (gen < 4) {
-			batch[i] |= 1;
-			reloc[0].delta = 1;
-		}
-	}
-
-	/* dummy write to fb */
-	reloc[1].target_handle = obj[SCRATCH].handle;
-	reloc[1].presumed_offset = 0;
-	reloc[1].offset = 1024;
-	reloc[1].delta = 0;
-	reloc[1].read_domains = I915_GEM_DOMAIN_RENDER;
-	reloc[1].write_domain = I915_GEM_DOMAIN_RENDER;
-
-	gem_execbuf(dpy->drm_fd, &execbuf);
-	gem_close(dpy->drm_fd, obj[BATCH].handle);
-
-	return batch;
-}
-
 static void finish_fb_busy(uint32_t *batch, int msecs)
 {
 	struct timespec tv = { 0, msecs * 1000 * 1000 };
@@ -165,9 +98,8 @@  static void flip_to_fb(igt_display_t *dpy, int pipe,
 	struct pollfd pfd = { .fd = dpy->drm_fd, .events = POLLIN };
 	struct timespec tv = { 1, 0 };
 	struct drm_event_vblank ev;
-	uint32_t *batch;
-
-	batch = make_fb_busy(dpy, ring, fb);
+	igt_spin_t t = igt_spin_batch(dpy->drm_fd, -1,
+				      ring, fb->gem_handle);
 	igt_fork(child, 1) {
 		igt_assert(gem_bo_busy(dpy->drm_fd, fb->gem_handle));
 		do_or_die(drmModePageFlip(dpy->drm_fd,
@@ -179,7 +111,8 @@  static void flip_to_fb(igt_display_t *dpy, int pipe,
 	}
 	igt_assert_f(nanosleep(&tv, NULL) == -1,
 		     "flip to %s blocked waiting for busy fb", name);
-	finish_fb_busy(batch, 2*TIMEOUT);
+	finish_fb_busy(t.batch, 2*TIMEOUT);
+	igt_post_spin_batch(dpy->drm_fd, t);
 	igt_waitchildren();
 	igt_assert(read(dpy->drm_fd, &ev, sizeof(ev)) == sizeof(ev));
 	igt_assert(poll(&pfd, 1, 0) == 0);