| Message ID | 20190130131212.15859-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [i-g-t] i915/gem_eio: 64 batches may be too many for some devices! | expand |
Chris Wilson <chris@chris-wilson.co.uk> writes: > Actually measure how many batches we can fit into a ring before > blocking, or else we may end up hanging the device earlier than > expected! > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Mika Kuoppala <mika.kuoppala@intel.com> > --- > tests/i915/gem_eio.c | 19 +++++++++++++++---- > 1 file changed, 15 insertions(+), 4 deletions(-) > > diff --git a/tests/i915/gem_eio.c b/tests/i915/gem_eio.c > index 09059c311..534bd1899 100644 > --- a/tests/i915/gem_eio.c > +++ b/tests/i915/gem_eio.c > @@ -44,6 +44,7 @@ > #include "igt_device.h" > #include "igt_sysfs.h" > #include "sw_sync.h" > +#include "i915/gem_ring.h" > > IGT_TEST_DESCRIPTION("Test that specific ioctls report a wedged GPU (EIO)."); > > @@ -358,10 +359,15 @@ static void test_inflight(int fd, unsigned int wait) > { > int parent_fd = fd; > unsigned int engine; > + int max; > > igt_require_gem(fd); > igt_require(gem_has_exec_fence(fd)); > > + max = gem_measure_ring_inflight(fd, -1, 0); > + igt_require(max > 1); > + max = min(max - 1, 64); > + > for_each_engine(parent_fd, engine) { > const uint32_t bbe = MI_BATCH_BUFFER_END; > struct drm_i915_gem_exec_object2 obj[2]; > @@ -389,7 +395,7 @@ static void test_inflight(int fd, unsigned int wait) > execbuf.buffer_count = 2; > execbuf.flags = engine | I915_EXEC_FENCE_OUT; > > - for (unsigned int n = 0; n < ARRAY_SIZE(fence); n++) { Move the fence array to upper scope and use that in finding the max. As bonus you can remove the comment on 'conservative estimate of ring size' as apparently we weren't so convervative after all. Bugzilla ref or how did you noticed this? much hairpulling potential this has. -Mika > + for (unsigned int n = 0; n < max; n++) { > gem_execbuf_wr(fd, &execbuf); > fence[n] = execbuf.rsvd2 >> 32; > igt_assert(fence[n] != -1); > @@ -397,7 +403,7 @@ static void test_inflight(int fd, unsigned int wait) > > check_wait(fd, obj[1].handle, wait); > > - for (unsigned int n = 0; n < ARRAY_SIZE(fence); n++) { > + for (unsigned int n = 0; n < max; n++) { > igt_assert_eq(sync_fence_status(fence[n]), -EIO); > close(fence[n]); > } > @@ -418,6 +424,11 @@ static void test_inflight_suspend(int fd) > uint32_t bbe = MI_BATCH_BUFFER_END; > int fence[64]; /* conservative estimate of ring size */ > igt_spin_t *hang; > + int max; > + > + max = gem_measure_ring_inflight(fd, -1, 0); > + igt_require(max > 1); > + max = min(max - 1, 64); > > fd = gem_reopen_driver(fd); > igt_require_gem(fd); > @@ -437,7 +448,7 @@ static void test_inflight_suspend(int fd) > execbuf.buffer_count = 2; > execbuf.flags = I915_EXEC_FENCE_OUT; > > - for (unsigned int n = 0; n < ARRAY_SIZE(fence); n++) { > + for (unsigned int n = 0; n < max; n++) { > gem_execbuf_wr(fd, &execbuf); > fence[n] = execbuf.rsvd2 >> 32; > igt_assert(fence[n] != -1); > @@ -448,7 +459,7 @@ static void test_inflight_suspend(int fd) > > check_wait(fd, obj[1].handle, 10); > > - for (unsigned int n = 0; n < ARRAY_SIZE(fence); n++) { > + for (unsigned int n = 0; n < max; n++) { > igt_assert_eq(sync_fence_status(fence[n]), -EIO); > close(fence[n]); > } > -- > 2.20.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoting Mika Kuoppala (2019-01-30 14:13:57) > Chris Wilson <chris@chris-wilson.co.uk> writes: > > > Actually measure how many batches we can fit into a ring before > > blocking, or else we may end up hanging the device earlier than > > expected! > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Mika Kuoppala <mika.kuoppala@intel.com> > > --- > > tests/i915/gem_eio.c | 19 +++++++++++++++---- > > 1 file changed, 15 insertions(+), 4 deletions(-) > > > > diff --git a/tests/i915/gem_eio.c b/tests/i915/gem_eio.c > > index 09059c311..534bd1899 100644 > > --- a/tests/i915/gem_eio.c > > +++ b/tests/i915/gem_eio.c > > @@ -44,6 +44,7 @@ > > #include "igt_device.h" > > #include "igt_sysfs.h" > > #include "sw_sync.h" > > +#include "i915/gem_ring.h" > > > > IGT_TEST_DESCRIPTION("Test that specific ioctls report a wedged GPU (EIO)."); > > > > @@ -358,10 +359,15 @@ static void test_inflight(int fd, unsigned int wait) > > { > > int parent_fd = fd; > > unsigned int engine; > > + int max; > > > > igt_require_gem(fd); > > igt_require(gem_has_exec_fence(fd)); > > > > + max = gem_measure_ring_inflight(fd, -1, 0); > > + igt_require(max > 1); > > + max = min(max - 1, 64); > > + > > for_each_engine(parent_fd, engine) { > > const uint32_t bbe = MI_BATCH_BUFFER_END; > > struct drm_i915_gem_exec_object2 obj[2]; > > @@ -389,7 +395,7 @@ static void test_inflight(int fd, unsigned int wait) > > execbuf.buffer_count = 2; > > execbuf.flags = engine | I915_EXEC_FENCE_OUT; > > > > - for (unsigned int n = 0; n < ARRAY_SIZE(fence); n++) { > > Move the fence array to upper scope and use that in finding the > max. As bonus you can remove the comment on 'conservative estimate of > ring size' as apparently we weren't so convervative after all. > > > Bugzilla ref or how did you noticed this? much hairpulling > potential this has. Just recent changes to bsw. I think there's a bugzilla for it, but will have to trawl. It's just solitary failure; the device is reset before the next igt (just not subtest). -Chris
diff --git a/tests/i915/gem_eio.c b/tests/i915/gem_eio.c index 09059c311..534bd1899 100644 --- a/tests/i915/gem_eio.c +++ b/tests/i915/gem_eio.c @@ -44,6 +44,7 @@ #include "igt_device.h" #include "igt_sysfs.h" #include "sw_sync.h" +#include "i915/gem_ring.h" IGT_TEST_DESCRIPTION("Test that specific ioctls report a wedged GPU (EIO)."); @@ -358,10 +359,15 @@ static void test_inflight(int fd, unsigned int wait) { int parent_fd = fd; unsigned int engine; + int max; igt_require_gem(fd); igt_require(gem_has_exec_fence(fd)); + max = gem_measure_ring_inflight(fd, -1, 0); + igt_require(max > 1); + max = min(max - 1, 64); + for_each_engine(parent_fd, engine) { const uint32_t bbe = MI_BATCH_BUFFER_END; struct drm_i915_gem_exec_object2 obj[2]; @@ -389,7 +395,7 @@ static void test_inflight(int fd, unsigned int wait) execbuf.buffer_count = 2; execbuf.flags = engine | I915_EXEC_FENCE_OUT; - for (unsigned int n = 0; n < ARRAY_SIZE(fence); n++) { + for (unsigned int n = 0; n < max; n++) { gem_execbuf_wr(fd, &execbuf); fence[n] = execbuf.rsvd2 >> 32; igt_assert(fence[n] != -1); @@ -397,7 +403,7 @@ static void test_inflight(int fd, unsigned int wait) check_wait(fd, obj[1].handle, wait); - for (unsigned int n = 0; n < ARRAY_SIZE(fence); n++) { + for (unsigned int n = 0; n < max; n++) { igt_assert_eq(sync_fence_status(fence[n]), -EIO); close(fence[n]); } @@ -418,6 +424,11 @@ static void test_inflight_suspend(int fd) uint32_t bbe = MI_BATCH_BUFFER_END; int fence[64]; /* conservative estimate of ring size */ igt_spin_t *hang; + int max; + + max = gem_measure_ring_inflight(fd, -1, 0); + igt_require(max > 1); + max = min(max - 1, 64); fd = gem_reopen_driver(fd); igt_require_gem(fd); @@ -437,7 +448,7 @@ static void test_inflight_suspend(int fd) execbuf.buffer_count = 2; execbuf.flags = I915_EXEC_FENCE_OUT; - for (unsigned int n = 0; n < ARRAY_SIZE(fence); n++) { + for (unsigned int n = 0; n < max; n++) { gem_execbuf_wr(fd, &execbuf); fence[n] = execbuf.rsvd2 >> 32; igt_assert(fence[n] != -1); @@ -448,7 +459,7 @@ static void test_inflight_suspend(int fd) check_wait(fd, obj[1].handle, 10); - for (unsigned int n = 0; n < ARRAY_SIZE(fence); n++) { + for (unsigned int n = 0; n < max; n++) { igt_assert_eq(sync_fence_status(fence[n]), -EIO); close(fence[n]); }
Actually measure how many batches we can fit into a ring before blocking, or else we may end up hanging the device earlier than expected! Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Mika Kuoppala <mika.kuoppala@intel.com> --- tests/i915/gem_eio.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-)