[PATCH i-g-t 1/2] i915/gem_exec_schedule: Avoid using borked engines

[PATCH i-g-t 1/2] i915/gem_exec_schedule: Avoid using borked engines

[Chris Wilson]

Check for viability of store-dword before use.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 tests/i915/gem_exec_schedule.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tests/i915/gem_exec_schedule.c b/tests/i915/gem_exec_schedule.c
index 4ccacba41..6e8466299 100644
--- a/tests/i915/gem_exec_schedule.c
+++ b/tests/i915/gem_exec_schedule.c
@@ -282,9 +282,11 @@ static void smoketest(int fd, unsigned ring, unsigned timeout)
 	nengine = 0;
 	if (ring == ALL_ENGINES) {
 		for_each_physical_engine(fd, engine)
-			engines[nengine++] = engine;
+			if (gem_can_store_dword(fd, engine))
+				engines[nengine++] = engine;
 	} else {
-		engines[nengine++] = ring;
+		if (gem_can_store_dword(fd, ring))
+			engines[nengine++] = ring;
 	}
 	igt_require(nengine);
 
-- 
2.22.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[igt-dev] [PATCH i-g-t 1/2] i915/gem_exec_schedule: Avoid using borked engines

[Chris Wilson]

Check for viability of store-dword before use.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 tests/i915/gem_exec_schedule.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tests/i915/gem_exec_schedule.c b/tests/i915/gem_exec_schedule.c
index 4ccacba41..6e8466299 100644
--- a/tests/i915/gem_exec_schedule.c
+++ b/tests/i915/gem_exec_schedule.c
@@ -282,9 +282,11 @@ static void smoketest(int fd, unsigned ring, unsigned timeout)
 	nengine = 0;
 	if (ring == ALL_ENGINES) {
 		for_each_physical_engine(fd, engine)
-			engines[nengine++] = engine;
+			if (gem_can_store_dword(fd, engine))
+				engines[nengine++] = engine;
 	} else {
-		engines[nengine++] = ring;
+		if (gem_can_store_dword(fd, ring))
+			engines[nengine++] = ring;
 	}
 	igt_require(nengine);
 
-- 
2.22.0

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

[PATCH i-g-t 2/2] i915/gem_ctx_engine: Drip feed requests into 'independent'

[Chris Wilson]

The intent of the test is to exercise that each channel in the engine[]
is an independent context/ring/timeline. It setups 64 channels pointing
to rcs0 and then submits one request to each in turn waiting on a
timeline that will force them to run out of submission order. They can
only run in fence order and not submission order if the timelines of
each channel are truly independent.

However, we released the fences en masse, and once the requests are
ready they are independent any may be executed in any order by the HW,
especially true with timeslicing that may reorder the requests on a
whim. So instead of releasing all requests at once, increment the
timeline step by step and check we get our results advancing. If the
requests can not be run in fence order and fall back to submission
order, we will time out waiting for our incremental results and trigger
a few GPU hangs.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110987
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 tests/i915/gem_ctx_engines.c | 39 +++++++++++++++++++++++++++---------
 1 file changed, 29 insertions(+), 10 deletions(-)

diff --git a/tests/i915/gem_ctx_engines.c b/tests/i915/gem_ctx_engines.c
index 8c66fb261..2e80d0f3e 100644
--- a/tests/i915/gem_ctx_engines.c
+++ b/tests/i915/gem_ctx_engines.c
@@ -405,6 +405,14 @@ static void execute_allforone(int i915)
 	gem_context_destroy(i915, param.ctx_id);
 }
 
+static uint32_t read_result(int timeline, uint32_t *map, int idx)
+{
+	sw_sync_timeline_inc(timeline, 1);
+	while (!READ_ONCE(map[idx]))
+		;
+	return map[idx];
+}
+
 static void independent(int i915)
 {
 #define RCS_TIMESTAMP (0x2000 + 0x358)
@@ -438,6 +446,12 @@ static void independent(int i915)
 	memset(&engines, 0, sizeof(engines)); /* All rcs0 */
 	gem_context_set_param(i915, &param);
 
+	gem_set_caching(i915, results.handle, I915_CACHING_CACHED);
+	map = gem_mmap__cpu(i915, results.handle, 0, 4096, PROT_READ);
+	gem_set_domain(i915, results.handle,
+		       I915_GEM_DOMAIN_CPU, I915_GEM_DOMAIN_CPU);
+	memset(map, 0, 4096);
+
 	for (int i = 0; i < I915_EXEC_RING_MASK + 1; i++) {
 		struct drm_i915_gem_exec_object2 obj[2] = {
 			results, /* write hazard lies! */
@@ -472,21 +486,21 @@ static void independent(int i915)
 		gem_close(i915, obj[1].handle);
 		close(execbuf.rsvd2);
 	}
-	close(timeline);
-	gem_sync(i915, results.handle);
-
-	map = gem_mmap__cpu(i915, results.handle, 0, 4096, PROT_READ);
-	gem_set_domain(i915, results.handle, I915_GEM_DOMAIN_CPU, 0);
-	gem_close(i915, results.handle);
 
-	last = map[0];
+	last = read_result(timeline, map, 0);
 	for (int i = 1; i < I915_EXEC_RING_MASK + 1; i++) {
-		igt_assert_f((map[i] - last) > 0,
-			     "Engine instance [%d] executed too late\n", i);
-		last = map[i];
+		uint32_t t = read_result(timeline, map, i);
+		igt_assert_f(t - last > 0,
+			     "Engine instance [%d] executed too late, previous timestamp %08x, now %08x\n",
+			     i, last, t);
+		last = t;
 	}
 	munmap(map, 4096);
 
+	close(timeline);
+	gem_sync(i915, results.handle);
+	gem_close(i915, results.handle);
+
 	gem_context_destroy(i915, param.ctx_id);
 }
 
@@ -500,6 +514,8 @@ igt_main
 
 		gem_require_contexts(i915);
 		igt_require(has_context_engines(i915));
+
+		igt_fork_hang_detector(i915);
 	}
 
 	igt_subtest("invalid-engines")
@@ -519,4 +535,7 @@ igt_main
 
 	igt_subtest("independent")
 		independent(i915);
+
+	igt_fixture
+		igt_stop_hang_detector();
 }
-- 
2.22.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[igt-dev] [PATCH i-g-t 2/2] i915/gem_ctx_engine: Drip feed requests into 'independent'

[Chris Wilson]

The intent of the test is to exercise that each channel in the engine[]
is an independent context/ring/timeline. It setups 64 channels pointing
to rcs0 and then submits one request to each in turn waiting on a
timeline that will force them to run out of submission order. They can
only run in fence order and not submission order if the timelines of
each channel are truly independent.

However, we released the fences en masse, and once the requests are
ready they are independent any may be executed in any order by the HW,
especially true with timeslicing that may reorder the requests on a
whim. So instead of releasing all requests at once, increment the
timeline step by step and check we get our results advancing. If the
requests can not be run in fence order and fall back to submission
order, we will time out waiting for our incremental results and trigger
a few GPU hangs.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110987
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 tests/i915/gem_ctx_engines.c | 39 +++++++++++++++++++++++++++---------
 1 file changed, 29 insertions(+), 10 deletions(-)

diff --git a/tests/i915/gem_ctx_engines.c b/tests/i915/gem_ctx_engines.c
index 8c66fb261..2e80d0f3e 100644
--- a/tests/i915/gem_ctx_engines.c
+++ b/tests/i915/gem_ctx_engines.c
@@ -405,6 +405,14 @@ static void execute_allforone(int i915)
 	gem_context_destroy(i915, param.ctx_id);
 }
 
+static uint32_t read_result(int timeline, uint32_t *map, int idx)
+{
+	sw_sync_timeline_inc(timeline, 1);
+	while (!READ_ONCE(map[idx]))
+		;
+	return map[idx];
+}
+
 static void independent(int i915)
 {
 #define RCS_TIMESTAMP (0x2000 + 0x358)
@@ -438,6 +446,12 @@ static void independent(int i915)
 	memset(&engines, 0, sizeof(engines)); /* All rcs0 */
 	gem_context_set_param(i915, &param);
 
+	gem_set_caching(i915, results.handle, I915_CACHING_CACHED);
+	map = gem_mmap__cpu(i915, results.handle, 0, 4096, PROT_READ);
+	gem_set_domain(i915, results.handle,
+		       I915_GEM_DOMAIN_CPU, I915_GEM_DOMAIN_CPU);
+	memset(map, 0, 4096);
+
 	for (int i = 0; i < I915_EXEC_RING_MASK + 1; i++) {
 		struct drm_i915_gem_exec_object2 obj[2] = {
 			results, /* write hazard lies! */
@@ -472,21 +486,21 @@ static void independent(int i915)
 		gem_close(i915, obj[1].handle);
 		close(execbuf.rsvd2);
 	}
-	close(timeline);
-	gem_sync(i915, results.handle);
-
-	map = gem_mmap__cpu(i915, results.handle, 0, 4096, PROT_READ);
-	gem_set_domain(i915, results.handle, I915_GEM_DOMAIN_CPU, 0);
-	gem_close(i915, results.handle);
 
-	last = map[0];
+	last = read_result(timeline, map, 0);
 	for (int i = 1; i < I915_EXEC_RING_MASK + 1; i++) {
-		igt_assert_f((map[i] - last) > 0,
-			     "Engine instance [%d] executed too late\n", i);
-		last = map[i];
+		uint32_t t = read_result(timeline, map, i);
+		igt_assert_f(t - last > 0,
+			     "Engine instance [%d] executed too late, previous timestamp %08x, now %08x\n",
+			     i, last, t);
+		last = t;
 	}
 	munmap(map, 4096);
 
+	close(timeline);
+	gem_sync(i915, results.handle);
+	gem_close(i915, results.handle);
+
 	gem_context_destroy(i915, param.ctx_id);
 }
 
@@ -500,6 +514,8 @@ igt_main
 
 		gem_require_contexts(i915);
 		igt_require(has_context_engines(i915));
+
+		igt_fork_hang_detector(i915);
 	}
 
 	igt_subtest("invalid-engines")
@@ -519,4 +535,7 @@ igt_main
 
 	igt_subtest("independent")
 		independent(i915);
+
+	igt_fixture
+		igt_stop_hang_detector();
 }
-- 
2.22.0

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

Re: [igt-dev] [PATCH i-g-t 1/2] i915/gem_exec_schedule: Avoid using borked engines

[Andi Shyti]

Hi Chris,

On Tue, Jul 30, 2019 at 10:24:53PM +0100, Chris Wilson wrote:
> Check for viability of store-dword before use.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Reviewed-by: Andi Shyti <andi.shyti@intel.com>

Andi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [PATCH i-g-t 1/2] i915/gem_exec_schedule: Avoid using borked engines

[Andi Shyti]

Hi Chris,

On Tue, Jul 30, 2019 at 10:24:53PM +0100, Chris Wilson wrote:
> Check for viability of store-dword before use.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Reviewed-by: Andi Shyti <andi.shyti@intel.com>

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