[igt-dev] [PATCH i-g-t] i915/gem_exec_schedule: Check deps along implicit inter-engine semaphores

[igt-dev] [PATCH i-g-t] i915/gem_exec_schedule: Check deps along implicit inter-engine semaphores

[Chris Wilson]

Given an implicit semaphore from one engine to the next, check that if
we skip the wait on that semaphore the following batch although
submitted early (as it depends along the single engine timeline) is not
executed ahead of its dependency.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Matthew Brost <matthew.brost@intel.com>
---
 tests/i915/gem_exec_schedule.c | 72 ++++++++++++++++++++++++++++++++++
 1 file changed, 72 insertions(+)

diff --git a/tests/i915/gem_exec_schedule.c b/tests/i915/gem_exec_schedule.c
index 3590b739a..5e18561d4 100644
--- a/tests/i915/gem_exec_schedule.c
+++ b/tests/i915/gem_exec_schedule.c
@@ -603,6 +603,76 @@ static void semaphore_resolve(int i915)
 	gem_context_destroy(i915, outer);
 }
 
+static void semaphore_noskip(int i915)
+{
+	unsigned int engine, other;
+	uint32_t ctx;
+
+	igt_assert(intel_get_drm_devid(i915) >= 8);
+
+	ctx = gem_context_create(i915);
+
+	for_each_physical_engine(i915, engine) {
+	for_each_physical_engine(i915, other) {
+		struct drm_i915_gem_exec_object2 obj[3];
+		struct drm_i915_gem_execbuffer2 eb;
+		igt_spin_t *chain, *spin;
+		uint32_t handle, cancel;
+		uint32_t *cs, *map;
+
+		if (other == engine || !gem_can_store_dword(i915, other))
+			continue;
+
+		chain = __igt_spin_new(i915, .engine = engine);
+
+		spin = __igt_spin_new(i915, .engine = other);
+		igt_spin_end(spin); /* we just want its address for later */
+		gem_sync(i915, spin->handle);
+		igt_spin_reset(spin);
+
+		handle = gem_create(i915, 4096);
+		cs = map = gem_mmap__cpu(i915, handle, 0, 4096, PROT_WRITE);
+
+		/* Cancel the following spinner */
+		*cs++ = MI_STORE_DWORD_IMM;
+		*cs++ = spin->obj[1].offset + offset_in_page(spin->condition);
+		*cs++ = 0;
+		*cs++ = MI_BATCH_BUFFER_END;
+
+		*cs++ = MI_BATCH_BUFFER_END;
+		munmap(map, 4096);
+
+		memset(obj, 0, sizeof(obj));
+		obj[0] = chain->obj[1];
+		obj[1] = spin->obj[1];
+		obj[2].handle = handle;
+		memset(&eb, 0, sizeof(eb));
+		eb.buffer_count = 3;
+		eb.buffers_ptr = to_user_pointer(obj);
+		eb.rsvd1 = ctx;
+		gem_execbuf(i915, &eb);
+
+		memset(obj, 0, sizeof(obj));
+		obj[0].handle = handle;
+		obj[0].flags = EXEC_OBJECT_WRITE;
+		obj[1] = spin->obj[1];
+		memset(&eb, 0, sizeof(eb));
+		eb.buffer_count = 2;
+		eb.buffers_ptr = to_user_pointer(obj);
+		gem_execbuf(i915, &eb);
+
+		igt_spin_set_timeout(chain, NSEC_PER_SEC / 100);
+		gem_sync(i915, spin->handle); /* To hang unless cancel runs! */
+
+		gem_close(i915, handle);
+		igt_spin_free(i915, spin);
+		igt_spin_free(i915, chain);
+	}
+	}
+
+	gem_context_destroy(i915, ctx);
+}
+
 static void reorder(int fd, unsigned ring, unsigned flags)
 #define EQUAL 1
 {
@@ -1592,6 +1662,8 @@ igt_main
 			semaphore_codependency(fd);
 		igt_subtest("semaphore-resolve")
 			semaphore_resolve(fd);
+		igt_subtest("semaphore-noskip")
+			semaphore_noskip(fd);
 
 		igt_subtest("smoketest-all")
 			smoketest(fd, ALL_ENGINES, 30);
-- 
2.20.1

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

[igt-dev] [PATCH i-g-t] i915/gem_exec_schedule: Check deps along implicit inter-engine semaphores

[Chris Wilson]

Given an implicit semaphore from one engine to the next, check that if
we skip the wait on that semaphore the following batch although
submitted early (as it depends along the single engine timeline) is not
executed ahead of its dependency.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Matthew Brost <matthew.brost@intel.com>
---
Use the other engines!
---
 tests/i915/gem_exec_schedule.c | 74 ++++++++++++++++++++++++++++++++++
 1 file changed, 74 insertions(+)

diff --git a/tests/i915/gem_exec_schedule.c b/tests/i915/gem_exec_schedule.c
index 3590b739a..d8badfd0b 100644
--- a/tests/i915/gem_exec_schedule.c
+++ b/tests/i915/gem_exec_schedule.c
@@ -603,6 +603,78 @@ static void semaphore_resolve(int i915)
 	gem_context_destroy(i915, outer);
 }
 
+static void semaphore_noskip(int i915)
+{
+	unsigned int engine, other;
+	uint32_t ctx;
+
+	igt_assert(intel_get_drm_devid(i915) >= 8);
+
+	ctx = gem_context_create(i915);
+
+	for_each_physical_engine(i915, engine) {
+	for_each_physical_engine(i915, other) {
+		struct drm_i915_gem_exec_object2 obj[3];
+		struct drm_i915_gem_execbuffer2 eb;
+		igt_spin_t *chain, *spin;
+		uint32_t handle, cancel;
+		uint32_t *cs, *map;
+
+		if (other == engine || !gem_can_store_dword(i915, other))
+			continue;
+
+		chain = __igt_spin_new(i915, .engine = engine);
+
+		spin = __igt_spin_new(i915, .engine = other);
+		igt_spin_end(spin); /* we just want its address for later */
+		gem_sync(i915, spin->handle);
+		igt_spin_reset(spin);
+
+		handle = gem_create(i915, 4096);
+		cs = map = gem_mmap__cpu(i915, handle, 0, 4096, PROT_WRITE);
+
+		/* Cancel the following spinner */
+		*cs++ = MI_STORE_DWORD_IMM;
+		*cs++ = spin->obj[1].offset + offset_in_page(spin->condition);
+		*cs++ = 0;
+		*cs++ = MI_BATCH_BUFFER_END;
+
+		*cs++ = MI_BATCH_BUFFER_END;
+		munmap(map, 4096);
+
+		memset(obj, 0, sizeof(obj));
+		obj[0] = chain->obj[1];
+		obj[1] = spin->obj[1];
+		obj[2].handle = handle;
+		memset(&eb, 0, sizeof(eb));
+		eb.buffer_count = 3;
+		eb.buffers_ptr = to_user_pointer(obj);
+		eb.rsvd1 = ctx;
+		eb.flags = other;
+		gem_execbuf(i915, &eb);
+
+		memset(obj, 0, sizeof(obj));
+		obj[0].handle = handle;
+		obj[0].flags = EXEC_OBJECT_WRITE;
+		obj[1] = spin->obj[1];
+		memset(&eb, 0, sizeof(eb));
+		eb.buffer_count = 2;
+		eb.buffers_ptr = to_user_pointer(obj);
+		eb.flags = other;
+		gem_execbuf(i915, &eb);
+
+		igt_spin_set_timeout(chain, NSEC_PER_SEC / 100);
+		gem_sync(i915, spin->handle); /* To hang unless cancel runs! */
+
+		gem_close(i915, handle);
+		igt_spin_free(i915, spin);
+		igt_spin_free(i915, chain);
+	}
+	}
+
+	gem_context_destroy(i915, ctx);
+}
+
 static void reorder(int fd, unsigned ring, unsigned flags)
 #define EQUAL 1
 {
@@ -1592,6 +1664,8 @@ igt_main
 			semaphore_codependency(fd);
 		igt_subtest("semaphore-resolve")
 			semaphore_resolve(fd);
+		igt_subtest("semaphore-noskip")
+			semaphore_noskip(fd);
 
 		igt_subtest("smoketest-all")
 			smoketest(fd, ALL_ENGINES, 30);
-- 
2.20.1

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

[igt-dev] [PATCH i-g-t] i915/gem_exec_schedule: Check deps along implicit inter-engine semaphores

[Chris Wilson]

Given an implicit semaphore from one engine to the next, check that if
we skip the wait on that semaphore the following batch although
submitted early (as it depends along the single engine timeline) is not
executed ahead of its dependency.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Matthew Brost <matthew.brost@intel.com>
---
Make sure there is actually a semaphore (read-read doesn't generate an
interengine dependency!)
---
 tests/i915/gem_exec_schedule.c | 75 ++++++++++++++++++++++++++++++++++
 1 file changed, 75 insertions(+)

diff --git a/tests/i915/gem_exec_schedule.c b/tests/i915/gem_exec_schedule.c
index 3590b739a..a6b74b3b6 100644
--- a/tests/i915/gem_exec_schedule.c
+++ b/tests/i915/gem_exec_schedule.c
@@ -603,6 +603,79 @@ static void semaphore_resolve(int i915)
 	gem_context_destroy(i915, outer);
 }
 
+static void semaphore_noskip(int i915)
+{
+	unsigned int engine, other;
+	uint32_t ctx;
+
+	igt_assert(intel_get_drm_devid(i915) >= 8);
+
+	ctx = gem_context_create(i915);
+
+	for_each_physical_engine(i915, engine) {
+	for_each_physical_engine(i915, other) {
+		struct drm_i915_gem_exec_object2 obj[3];
+		struct drm_i915_gem_execbuffer2 eb;
+		igt_spin_t *chain, *spin;
+		uint32_t handle, cancel;
+		uint32_t *cs, *map;
+
+		if (other == engine || !gem_can_store_dword(i915, other))
+			continue;
+
+		chain = __igt_spin_new(i915, .engine = engine);
+
+		spin = __igt_spin_new(i915, .engine = other);
+		igt_spin_end(spin); /* we just want its address for later */
+		gem_sync(i915, spin->handle);
+		igt_spin_reset(spin);
+
+		handle = gem_create(i915, 4096);
+		cs = map = gem_mmap__cpu(i915, handle, 0, 4096, PROT_WRITE);
+
+		/* Cancel the following spinner */
+		*cs++ = MI_STORE_DWORD_IMM;
+		*cs++ = spin->obj[1].offset + offset_in_page(spin->condition);
+		*cs++ = 0;
+		*cs++ = MI_BATCH_BUFFER_END;
+
+		*cs++ = MI_BATCH_BUFFER_END;
+		munmap(map, 4096);
+
+		memset(obj, 0, sizeof(obj));
+		obj[0] = chain->obj[1];
+		obj[0].flags |= EXEC_OBJECT_WRITE;
+		obj[1] = spin->obj[1];
+		obj[2].handle = handle;
+		memset(&eb, 0, sizeof(eb));
+		eb.buffer_count = 3;
+		eb.buffers_ptr = to_user_pointer(obj);
+		eb.rsvd1 = ctx;
+		eb.flags = other;
+		gem_execbuf(i915, &eb);
+
+		memset(obj, 0, sizeof(obj));
+		obj[0].handle = handle;
+		obj[0].flags = EXEC_OBJECT_WRITE;
+		obj[1] = spin->obj[1];
+		memset(&eb, 0, sizeof(eb));
+		eb.buffer_count = 2;
+		eb.buffers_ptr = to_user_pointer(obj);
+		eb.flags = other;
+		gem_execbuf(i915, &eb);
+
+		igt_spin_set_timeout(chain, NSEC_PER_SEC / 100);
+		gem_sync(i915, spin->handle); /* To hang unless cancel runs! */
+
+		gem_close(i915, handle);
+		igt_spin_free(i915, spin);
+		igt_spin_free(i915, chain);
+	}
+	}
+
+	gem_context_destroy(i915, ctx);
+}
+
 static void reorder(int fd, unsigned ring, unsigned flags)
 #define EQUAL 1
 {
@@ -1592,6 +1665,8 @@ igt_main
 			semaphore_codependency(fd);
 		igt_subtest("semaphore-resolve")
 			semaphore_resolve(fd);
+		igt_subtest("semaphore-noskip")
+			semaphore_noskip(fd);
 
 		igt_subtest("smoketest-all")
 			smoketest(fd, ALL_ENGINES, 30);
-- 
2.20.1

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

[igt-dev] ✗ Fi.CI.BAT: failure for i915/gem_exec_schedule: Check deps along implicit inter-engine semaphores (rev3)

[Patchwork]

== Series Details ==

Series: i915/gem_exec_schedule: Check deps along implicit inter-engine semaphores (rev3)
URL   : https://patchwork.freedesktop.org/series/59894/
State : failure

== Summary ==

Applying: i915/gem_exec_schedule: Check deps along implicit inter-engine semaphores
Patch failed at 0001 i915/gem_exec_schedule: Check deps along implicit inter-engine semaphores
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

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

Re: [igt-dev] [PATCH i-g-t] i915/gem_exec_schedule: Check deps along implicit inter-engine semaphores

[Daniele Ceraolo Spurio]

On 4/24/19 8:47 AM, Chris Wilson wrote:
> Given an implicit semaphore from one engine to the next, check that if
> we skip the wait on that semaphore the following batch although
> submitted early (as it depends along the single engine timeline) is not
> executed ahead of its dependency.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>

Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

some nitpicks below.

> ---
> Make sure there is actually a semaphore (read-read doesn't generate an
> interengine dependency!)
> ---
>   tests/i915/gem_exec_schedule.c | 75 ++++++++++++++++++++++++++++++++++
>   1 file changed, 75 insertions(+)
> 
> diff --git a/tests/i915/gem_exec_schedule.c b/tests/i915/gem_exec_schedule.c
> index 3590b739a..a6b74b3b6 100644
> --- a/tests/i915/gem_exec_schedule.c
> +++ b/tests/i915/gem_exec_schedule.c
> @@ -603,6 +603,79 @@ static void semaphore_resolve(int i915)
>   	gem_context_destroy(i915, outer);
>   }
>   
> +static void semaphore_noskip(int i915)
> +{
> +	unsigned int engine, other;
> +	uint32_t ctx;
> +
> +	igt_assert(intel_get_drm_devid(i915) >= 8);

I would move this to igt_require since the subtest_group only requires 
scheduler & priority and even if we currently don't enable those 
pre-gen8 I don't think we can assume we never will.

> +
> +	ctx = gem_context_create(i915);
> +
> +	for_each_physical_engine(i915, engine) {
> +	for_each_physical_engine(i915, other) {

Is it worth hiding the double loop in its own macro? e.g.:

for_each_physical_engine_pair(i915, engine1, engine2)

> +		struct drm_i915_gem_exec_object2 obj[3];
> +		struct drm_i915_gem_execbuffer2 eb;
> +		igt_spin_t *chain, *spin;
> +		uint32_t handle, cancel;
> +		uint32_t *cs, *map;
> +
> +		if (other == engine || !gem_can_store_dword(i915, other))
> +			continue;
> +
> +		chain = __igt_spin_new(i915, .engine = engine);
> +
> +		spin = __igt_spin_new(i915, .engine = other);
> +		igt_spin_end(spin); /* we just want its address for later */
> +		gem_sync(i915, spin->handle);
> +		igt_spin_reset(spin);
> +
> +		handle = gem_create(i915, 4096);
> +		cs = map = gem_mmap__cpu(i915, handle, 0, 4096, PROT_WRITE);
> +
> +		/* Cancel the following spinner */
> +		*cs++ = MI_STORE_DWORD_IMM;
> +		*cs++ = spin->obj[1].offset + offset_in_page(spin->condition);
> +		*cs++ = 0;
> +		*cs++ = MI_BATCH_BUFFER_END;
> +
> +		*cs++ = MI_BATCH_BUFFER_END;
> +		munmap(map, 4096);
> +
> +		memset(obj, 0, sizeof(obj));
> +		obj[0] = chain->obj[1];
> +		obj[0].flags |= EXEC_OBJECT_WRITE;
> +		obj[1] = spin->obj[1];
> +		obj[2].handle = handle;
> +		memset(&eb, 0, sizeof(eb));
> +		eb.buffer_count = 3;
> +		eb.buffers_ptr = to_user_pointer(obj);
> +		eb.rsvd1 = ctx;
> +		eb.flags = other;
> +		gem_execbuf(i915, &eb);
> +
> +		memset(obj, 0, sizeof(obj));
> +		obj[0].handle = handle;
> +		obj[0].flags = EXEC_OBJECT_WRITE;
> +		obj[1] = spin->obj[1];
> +		memset(&eb, 0, sizeof(eb));
> +		eb.buffer_count = 2;
> +		eb.buffers_ptr = to_user_pointer(obj);
> +		eb.flags = other;
> +		gem_execbuf(i915, &eb);
> +
> +		igt_spin_set_timeout(chain, NSEC_PER_SEC / 100);
> +		gem_sync(i915, spin->handle); /* To hang unless cancel runs! */
> +
> +		gem_close(i915, handle);
> +		igt_spin_free(i915, spin);
> +		igt_spin_free(i915, chain);
> +	}
> +	}
> +
> +	gem_context_destroy(i915, ctx);
> +}
> +
>   static void reorder(int fd, unsigned ring, unsigned flags)
>   #define EQUAL 1
>   {
> @@ -1592,6 +1665,8 @@ igt_main
>   			semaphore_codependency(fd);
>   		igt_subtest("semaphore-resolve")
>   			semaphore_resolve(fd);

semaphore_resolve isn't merged yet ;)

Daniele

> +		igt_subtest("semaphore-noskip")
> +			semaphore_noskip(fd);
>   
>   		igt_subtest("smoketest-all")
>   			smoketest(fd, ALL_ENGINES, 30);
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t] i915/gem_exec_schedule: Check deps along implicit inter-engine semaphores

[Chris Wilson]

Quoting Daniele Ceraolo Spurio (2019-04-24 18:53:08)
> 
> 
> On 4/24/19 8:47 AM, Chris Wilson wrote:
> > Given an implicit semaphore from one engine to the next, check that if
> > we skip the wait on that semaphore the following batch although
> > submitted early (as it depends along the single engine timeline) is not
> > executed ahead of its dependency.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > Cc: Matthew Brost <matthew.brost@intel.com>
> 
> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> 
> some nitpicks below.
> 
> > ---
> > Make sure there is actually a semaphore (read-read doesn't generate an
> > interengine dependency!)
> > ---
> >   tests/i915/gem_exec_schedule.c | 75 ++++++++++++++++++++++++++++++++++
> >   1 file changed, 75 insertions(+)
> > 
> > diff --git a/tests/i915/gem_exec_schedule.c b/tests/i915/gem_exec_schedule.c
> > index 3590b739a..a6b74b3b6 100644
> > --- a/tests/i915/gem_exec_schedule.c
> > +++ b/tests/i915/gem_exec_schedule.c
> > @@ -603,6 +603,79 @@ static void semaphore_resolve(int i915)
> >       gem_context_destroy(i915, outer);
> >   }
> >   
> > +static void semaphore_noskip(int i915)
> > +{
> > +     unsigned int engine, other;
> > +     uint32_t ctx;
> > +
> > +     igt_assert(intel_get_drm_devid(i915) >= 8);
> 
> I would move this to igt_require since the subtest_group only requires 
> scheduler & priority and even if we currently don't enable those 
> pre-gen8 I don't think we can assume we never will.

Sure. It's copy-and-paste from the previous test that was using
MI_SEMAPHORE_WAIT. This test could be written to be gen agnostic,
so might as well.

> > +
> > +     ctx = gem_context_create(i915);
> > +
> > +     for_each_physical_engine(i915, engine) {
> > +     for_each_physical_engine(i915, other) {
> 
> Is it worth hiding the double loop in its own macro? e.g.:
> 
> for_each_physical_engine_pair(i915, engine1, engine2)

I'm still waiting for Andi to rewrite the iterators :)
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev