* [PATCH i-g-t] i915/gem_eio: 64 batches may be too many for some devices!
@ 2019-01-30 13:12 Chris Wilson
2019-01-30 14:13 ` Mika Kuoppala
2019-01-30 14:21 ` Chris Wilson
0 siblings, 2 replies; 5+ messages in thread
From: Chris Wilson @ 2019-01-30 13:12 UTC (permalink / raw)
To: intel-gfx; +Cc: igt-dev, Mika Kuoppala
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++) {
+ 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
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH i-g-t] i915/gem_eio: 64 batches may be too many for some devices!
2019-01-30 13:12 [PATCH i-g-t] i915/gem_eio: 64 batches may be too many for some devices! Chris Wilson
@ 2019-01-30 14:13 ` Mika Kuoppala
2019-01-30 14:18 ` Chris Wilson
2019-01-30 14:21 ` Chris Wilson
1 sibling, 1 reply; 5+ messages in thread
From: Mika Kuoppala @ 2019-01-30 14:13 UTC (permalink / raw)
To: Chris Wilson, intel-gfx; +Cc: igt-dev
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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH i-g-t] i915/gem_eio: 64 batches may be too many for some devices!
2019-01-30 14:13 ` Mika Kuoppala
@ 2019-01-30 14:18 ` Chris Wilson
0 siblings, 0 replies; 5+ messages in thread
From: Chris Wilson @ 2019-01-30 14:18 UTC (permalink / raw)
To: Mika Kuoppala, intel-gfx; +Cc: igt-dev
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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH i-g-t] i915/gem_eio: 64 batches may be too many for some devices!
2019-01-30 13:12 [PATCH i-g-t] i915/gem_eio: 64 batches may be too many for some devices! Chris Wilson
2019-01-30 14:13 ` Mika Kuoppala
@ 2019-01-30 14:21 ` Chris Wilson
2019-01-30 14:27 ` Mika Kuoppala
1 sibling, 1 reply; 5+ messages in thread
From: Chris Wilson @ 2019-01-30 14:21 UTC (permalink / raw)
To: intel-gfx; +Cc: igt-dev, Mika Kuoppala
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!
v2: Mostly conservative.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109014
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
---
tests/i915/gem_eio.c | 23 +++++++++++++++++------
1 file changed, 17 insertions(+), 6 deletions(-)
diff --git a/tests/i915/gem_eio.c b/tests/i915/gem_eio.c
index 09059c311..61054a07e 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,16 +359,21 @@ static void test_inflight(int fd, unsigned int wait)
{
int parent_fd = fd;
unsigned int engine;
+ int fence[64]; /* mostly conservative estimate of ring size */
+ 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, ARRAY_SIZE(fence));
+
for_each_engine(parent_fd, engine) {
const uint32_t bbe = MI_BATCH_BUFFER_END;
struct drm_i915_gem_exec_object2 obj[2];
struct drm_i915_gem_execbuffer2 execbuf;
igt_spin_t *hang;
- int fence[64]; /* conservative estimate of ring size */
fd = gem_reopen_driver(parent_fd);
igt_require_gem(fd);
@@ -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]);
}
@@ -416,8 +422,13 @@ static void test_inflight_suspend(int fd)
struct drm_i915_gem_execbuffer2 execbuf;
struct drm_i915_gem_exec_object2 obj[2];
uint32_t bbe = MI_BATCH_BUFFER_END;
- int fence[64]; /* conservative estimate of ring size */
+ int fence[64]; /* mostly 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, ARRAY_SIZE(fence));
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
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH i-g-t] i915/gem_eio: 64 batches may be too many for some devices!
2019-01-30 14:21 ` Chris Wilson
@ 2019-01-30 14:27 ` Mika Kuoppala
0 siblings, 0 replies; 5+ messages in thread
From: Mika Kuoppala @ 2019-01-30 14:27 UTC (permalink / raw)
To: Chris Wilson, intel-gfx; +Cc: igt-dev
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!
>
> v2: Mostly conservative.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109014
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
> tests/i915/gem_eio.c | 23 +++++++++++++++++------
> 1 file changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/tests/i915/gem_eio.c b/tests/i915/gem_eio.c
> index 09059c311..61054a07e 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,16 +359,21 @@ static void test_inflight(int fd, unsigned int wait)
> {
> int parent_fd = fd;
> unsigned int engine;
> + int fence[64]; /* mostly conservative estimate of ring size */
> + 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, ARRAY_SIZE(fence));
> +
> for_each_engine(parent_fd, engine) {
> const uint32_t bbe = MI_BATCH_BUFFER_END;
> struct drm_i915_gem_exec_object2 obj[2];
> struct drm_i915_gem_execbuffer2 execbuf;
> igt_spin_t *hang;
> - int fence[64]; /* conservative estimate of ring size */
>
> fd = gem_reopen_driver(parent_fd);
> igt_require_gem(fd);
> @@ -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]);
> }
> @@ -416,8 +422,13 @@ static void test_inflight_suspend(int fd)
> struct drm_i915_gem_execbuffer2 execbuf;
> struct drm_i915_gem_exec_object2 obj[2];
> uint32_t bbe = MI_BATCH_BUFFER_END;
> - int fence[64]; /* conservative estimate of ring size */
> + int fence[64]; /* mostly 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, ARRAY_SIZE(fence));
>
> 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-01-30 14:27 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-01-30 13:12 [PATCH i-g-t] i915/gem_eio: 64 batches may be too many for some devices! Chris Wilson
2019-01-30 14:13 ` Mika Kuoppala
2019-01-30 14:18 ` Chris Wilson
2019-01-30 14:21 ` Chris Wilson
2019-01-30 14:27 ` Mika Kuoppala
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox