* [PATCH i-g-t 2/3] Force spin-batch to cause a hang as required
2019-09-11 10:14 [PATCH i-g-t 1/3] i915/gem_eio: Race kms on/off vs reset Chris Wilson
@ 2019-09-11 10:15 ` Chris Wilson
2019-09-19 14:09 ` [igt-dev] " Andi Shyti
2019-09-11 10:15 ` [PATCH i-g-t 3/3] kms_busy: Replace fiddling with hangcheck modparam with explicit fence Chris Wilson
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Chris Wilson @ 2019-09-11 10:15 UTC (permalink / raw)
To: intel-gfx; +Cc: igt-dev
When using a spinner to trigger a hang, make it unpreemptable so that it
appears like a true hang.
References: https://bugs.freedesktop.org/show_bug.cgi?id=109661
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
tests/i915/gem_eio.c | 4 +++-
tests/i915/gem_exec_fence.c | 3 ++-
tests/kms_busy.c | 3 ++-
3 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/tests/i915/gem_eio.c b/tests/i915/gem_eio.c
index e0213c76c..e7f5d4ddb 100644
--- a/tests/i915/gem_eio.c
+++ b/tests/i915/gem_eio.c
@@ -178,7 +178,9 @@ static igt_spin_t * __spin_poll(int fd, uint32_t ctx, unsigned long flags)
struct igt_spin_factory opts = {
.ctx = ctx,
.engine = flags,
- .flags = IGT_SPIN_FAST | IGT_SPIN_FENCE_OUT,
+ .flags = (IGT_SPIN_FAST |
+ IGT_SPIN_NO_PREEMPTION |
+ IGT_SPIN_FENCE_OUT),
};
if (gem_can_store_dword(fd, opts.engine))
diff --git a/tests/i915/gem_exec_fence.c b/tests/i915/gem_exec_fence.c
index 207182922..2f04d7af4 100644
--- a/tests/i915/gem_exec_fence.c
+++ b/tests/i915/gem_exec_fence.c
@@ -331,7 +331,8 @@ static void test_fence_await(int fd, unsigned ring, unsigned flags)
spin = igt_spin_new(fd,
.engine = ring,
- .flags = IGT_SPIN_FENCE_OUT);
+ .flags = (IGT_SPIN_FENCE_OUT |
+ IGT_SPIN_NO_PREEMPTION));
igt_assert(spin->out_fence != -1);
i = 0;
diff --git a/tests/kms_busy.c b/tests/kms_busy.c
index 66f26cd08..7e5ab3d19 100644
--- a/tests/kms_busy.c
+++ b/tests/kms_busy.c
@@ -271,7 +271,8 @@ static void test_pageflip_modeset_hang(igt_display_t *dpy,
t = igt_spin_new(dpy->drm_fd,
.engine = ring,
- .dependency = fb.gem_handle);
+ .dependency = fb.gem_handle,
+ .flags = IGT_SPIN_NO_PREEMPTION);
do_or_die(drmModePageFlip(dpy->drm_fd, dpy->pipes[pipe].crtc_id, fb.fb_id, DRM_MODE_PAGE_FLIP_EVENT, &fb));
--
2.23.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [igt-dev] [PATCH i-g-t 2/3] Force spin-batch to cause a hang as required
2019-09-11 10:15 ` [PATCH i-g-t 2/3] Force spin-batch to cause a hang as required Chris Wilson
@ 2019-09-19 14:09 ` Andi Shyti
0 siblings, 0 replies; 8+ messages in thread
From: Andi Shyti @ 2019-09-19 14:09 UTC (permalink / raw)
To: Chris Wilson; +Cc: igt-dev, intel-gfx
On Wed, Sep 11, 2019 at 11:15:00AM +0100, Chris Wilson wrote:
> When using a spinner to trigger a hang, make it unpreemptable so that it
> appears like a true hang.
>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=109661
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Andi Shyti <andi.shyti@intel.com>
Andi
> ---
> tests/i915/gem_eio.c | 4 +++-
> tests/i915/gem_exec_fence.c | 3 ++-
> tests/kms_busy.c | 3 ++-
> 3 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/tests/i915/gem_eio.c b/tests/i915/gem_eio.c
> index e0213c76c..e7f5d4ddb 100644
> --- a/tests/i915/gem_eio.c
> +++ b/tests/i915/gem_eio.c
> @@ -178,7 +178,9 @@ static igt_spin_t * __spin_poll(int fd, uint32_t ctx, unsigned long flags)
> struct igt_spin_factory opts = {
> .ctx = ctx,
> .engine = flags,
> - .flags = IGT_SPIN_FAST | IGT_SPIN_FENCE_OUT,
> + .flags = (IGT_SPIN_FAST |
> + IGT_SPIN_NO_PREEMPTION |
> + IGT_SPIN_FENCE_OUT),
> };
>
> if (gem_can_store_dword(fd, opts.engine))
> diff --git a/tests/i915/gem_exec_fence.c b/tests/i915/gem_exec_fence.c
> index 207182922..2f04d7af4 100644
> --- a/tests/i915/gem_exec_fence.c
> +++ b/tests/i915/gem_exec_fence.c
> @@ -331,7 +331,8 @@ static void test_fence_await(int fd, unsigned ring, unsigned flags)
>
> spin = igt_spin_new(fd,
> .engine = ring,
> - .flags = IGT_SPIN_FENCE_OUT);
> + .flags = (IGT_SPIN_FENCE_OUT |
> + IGT_SPIN_NO_PREEMPTION));
> igt_assert(spin->out_fence != -1);
>
> i = 0;
> diff --git a/tests/kms_busy.c b/tests/kms_busy.c
> index 66f26cd08..7e5ab3d19 100644
> --- a/tests/kms_busy.c
> +++ b/tests/kms_busy.c
> @@ -271,7 +271,8 @@ static void test_pageflip_modeset_hang(igt_display_t *dpy,
>
> t = igt_spin_new(dpy->drm_fd,
> .engine = ring,
> - .dependency = fb.gem_handle);
> + .dependency = fb.gem_handle,
> + .flags = IGT_SPIN_NO_PREEMPTION);
>
> do_or_die(drmModePageFlip(dpy->drm_fd, dpy->pipes[pipe].crtc_id, fb.fb_id, DRM_MODE_PAGE_FLIP_EVENT, &fb));
>
> --
> 2.23.0
>
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH i-g-t 3/3] kms_busy: Replace fiddling with hangcheck modparam with explicit fence
2019-09-11 10:14 [PATCH i-g-t 1/3] i915/gem_eio: Race kms on/off vs reset Chris Wilson
2019-09-11 10:15 ` [PATCH i-g-t 2/3] Force spin-batch to cause a hang as required Chris Wilson
@ 2019-09-11 10:15 ` Chris Wilson
2019-09-19 14:09 ` [igt-dev] " Andi Shyti
2019-09-19 14:08 ` [igt-dev] [PATCH i-g-t 1/3] i915/gem_eio: Race kms on/off vs reset Andi Shyti
2019-09-19 14:28 ` Ville Syrjälä
3 siblings, 1 reply; 8+ messages in thread
From: Chris Wilson @ 2019-09-11 10:15 UTC (permalink / raw)
To: intel-gfx; +Cc: igt-dev
Use an explicit fence to circumvent the [i915] GPU hang detection rather
than tweak the i915 specific modparam (and remove the assertion that
such a param exists). Note, that with a bit more work, the fence could
be used be directly rather than via dirtying the fb with a dummyload.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
lib/igt_dummyload.c | 5 +++++
lib/igt_dummyload.h | 10 ++++++----
tests/kms_busy.c | 26 ++++++++++----------------
3 files changed, 21 insertions(+), 20 deletions(-)
diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c
index 0e06276af..65b5cc927 100644
--- a/lib/igt_dummyload.c
+++ b/lib/igt_dummyload.c
@@ -236,6 +236,11 @@ emit_recursive_batch(igt_spin_t *spin,
if (opts->flags & IGT_SPIN_FENCE_OUT)
execbuf->flags |= I915_EXEC_FENCE_OUT;
+ if (opts->flags & IGT_SPIN_FENCE_IN) {
+ execbuf->flags |= I915_EXEC_FENCE_IN;
+ execbuf->rsvd2 = opts->fence;
+ }
+
for (i = 0; i < nengine; i++) {
execbuf->flags &= ~ENGINE_MASK;
execbuf->flags |= flags[i];
diff --git a/lib/igt_dummyload.h b/lib/igt_dummyload.h
index bb25751ad..66837057d 100644
--- a/lib/igt_dummyload.h
+++ b/lib/igt_dummyload.h
@@ -54,12 +54,14 @@ struct igt_spin_factory {
uint32_t dependency;
unsigned int engine;
unsigned int flags;
+ int fence;
};
-#define IGT_SPIN_FENCE_OUT (1 << 0)
-#define IGT_SPIN_POLL_RUN (1 << 1)
-#define IGT_SPIN_FAST (1 << 2)
-#define IGT_SPIN_NO_PREEMPTION (1 << 3)
+#define IGT_SPIN_FENCE_IN (1 << 0)
+#define IGT_SPIN_FENCE_OUT (1 << 1)
+#define IGT_SPIN_POLL_RUN (1 << 2)
+#define IGT_SPIN_FAST (1 << 3)
+#define IGT_SPIN_NO_PREEMPTION (1 << 4)
igt_spin_t *
__igt_spin_factory(int fd, const struct igt_spin_factory *opts);
diff --git a/tests/kms_busy.c b/tests/kms_busy.c
index 7e5ab3d19..bfb3857f4 100644
--- a/tests/kms_busy.c
+++ b/tests/kms_busy.c
@@ -75,22 +75,16 @@ static void flip_to_fb(igt_display_t *dpy, int pipe,
struct pollfd pfd = { .fd = dpy->drm_fd, .events = POLLIN };
const int timeout = modeset ? 8500 : 100;
struct drm_event_vblank ev;
+ IGT_CORK_FENCE(cork);
+ igt_spin_t *t;
+ int fence;
- igt_spin_t *t = igt_spin_new(dpy->drm_fd,
- .engine = ring,
- .dependency = fb->gem_handle);
-
- if (modeset) {
- /*
- * We want to check that a modeset actually waits for the
- * spin batch to complete, but we keep a bigger timeout for
- * disable than required for flipping.
- *
- * As a result, the GPU reset code may kick in, which we neuter
- * here to be sure there's no premature completion.
- */
- igt_set_module_param_int("enable_hangcheck", 0);
- }
+ fence = igt_cork_plug(&cork, dpy->drm_fd);
+ t = igt_spin_new(dpy->drm_fd,
+ .engine = ring,
+ .fence = fence,
+ .dependency = fb->gem_handle,
+ .flags = IGT_SPIN_FENCE_IN | IGT_SPIN_NO_PREEMPTION);
igt_fork(child, 1) {
igt_assert(gem_bo_busy(dpy->drm_fd, fb->gem_handle));
@@ -116,13 +110,13 @@ static void flip_to_fb(igt_display_t *dpy, int pipe,
igt_waitchildren_timeout(5 * timeout,
"flip blocked waiting for busy bo\n");
igt_spin_end(t);
+ close(fence);
igt_assert(read(dpy->drm_fd, &ev, sizeof(ev)) == sizeof(ev));
igt_assert(poll(&pfd, 1, 0) == 0);
if (modeset) {
gem_quiescent_gpu(dpy->drm_fd);
- igt_set_module_param_int("enable_hangcheck", 1);
/* Clear old mode blob. */
igt_pipe_refresh(dpy, pipe, true);
--
2.23.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [igt-dev] [PATCH i-g-t 1/3] i915/gem_eio: Race kms on/off vs reset
2019-09-11 10:14 [PATCH i-g-t 1/3] i915/gem_eio: Race kms on/off vs reset Chris Wilson
2019-09-11 10:15 ` [PATCH i-g-t 2/3] Force spin-batch to cause a hang as required Chris Wilson
2019-09-11 10:15 ` [PATCH i-g-t 3/3] kms_busy: Replace fiddling with hangcheck modparam with explicit fence Chris Wilson
@ 2019-09-19 14:08 ` Andi Shyti
2019-09-19 14:20 ` Chris Wilson
2019-09-19 14:28 ` Ville Syrjälä
3 siblings, 1 reply; 8+ messages in thread
From: Andi Shyti @ 2019-09-19 14:08 UTC (permalink / raw)
To: Chris Wilson; +Cc: igt-dev, intel-gfx
Hi Chris,
On Wed, Sep 11, 2019 at 11:14:59AM +0100, Chris Wilson wrote:
> On older platforms, performing a GPU reset clobbers the display.
> Exercise the interactions between reset/wedge and the display and
> hopefully prevent any races creeping in.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Is Ville Cc'ed for real or just nominally?
looks straight forward,
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [igt-dev] [PATCH i-g-t 1/3] i915/gem_eio: Race kms on/off vs reset
2019-09-19 14:08 ` [igt-dev] [PATCH i-g-t 1/3] i915/gem_eio: Race kms on/off vs reset Andi Shyti
@ 2019-09-19 14:20 ` Chris Wilson
0 siblings, 0 replies; 8+ messages in thread
From: Chris Wilson @ 2019-09-19 14:20 UTC (permalink / raw)
To: Andi Shyti; +Cc: igt-dev, intel-gfx
Quoting Andi Shyti (2019-09-19 15:08:57)
> Hi Chris,
>
> On Wed, Sep 11, 2019 at 11:14:59AM +0100, Chris Wilson wrote:
> > On older platforms, performing a GPU reset clobbers the display.
> > Exercise the interactions between reset/wedge and the display and
> > hopefully prevent any races creeping in.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Is Ville Cc'ed for real or just nominally?
He's in the original cc. As it is using "Syrjälä", that means I copied
his email address from one of his commits, so it should be the right
address.
mailman naturally drops cc of list members...
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH i-g-t 1/3] i915/gem_eio: Race kms on/off vs reset
2019-09-11 10:14 [PATCH i-g-t 1/3] i915/gem_eio: Race kms on/off vs reset Chris Wilson
` (2 preceding siblings ...)
2019-09-19 14:08 ` [igt-dev] [PATCH i-g-t 1/3] i915/gem_eio: Race kms on/off vs reset Andi Shyti
@ 2019-09-19 14:28 ` Ville Syrjälä
3 siblings, 0 replies; 8+ messages in thread
From: Ville Syrjälä @ 2019-09-19 14:28 UTC (permalink / raw)
To: Chris Wilson; +Cc: igt-dev, intel-gfx
On Wed, Sep 11, 2019 at 11:14:59AM +0100, Chris Wilson wrote:
> On older platforms, performing a GPU reset clobbers the display.
> Exercise the interactions between reset/wedge and the display and
> hopefully prevent any races creeping in.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> tests/i915/gem_eio.c | 79 ++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 79 insertions(+)
>
> diff --git a/tests/i915/gem_eio.c b/tests/i915/gem_eio.c
> index 9b086a039..e0213c76c 100644
> --- a/tests/i915/gem_eio.c
> +++ b/tests/i915/gem_eio.c
> @@ -42,6 +42,8 @@
>
> #include "igt.h"
> #include "igt_device.h"
> +#include "igt_fb.h"
> +#include "igt_kms.h"
> #include "igt_stats.h"
> #include "igt_sysfs.h"
> #include "sw_sync.h"
> @@ -813,6 +815,67 @@ static void test_reset_stress(int fd, unsigned int flags)
> gem_context_destroy(fd, ctx0);
> }
>
> +/*
> + * Modesetting vs wedged
> + */
> +
> +static void display_helper(igt_display_t *dpy, int *done)
> +{
> + const int commit = dpy->is_atomic ? COMMIT_ATOMIC : COMMIT_LEGACY;
> + struct igt_fb fb = {};
> +
> + while (!READ_ONCE(*done)) {
> + drmModeModeInfoPtr mode;
> + igt_plane_t *primary;
> + igt_output_t *output;
> + int pipe;
> +
> + pipe = rand() % dpy->n_pipes;
I guess this ends up doing nothing if we get the same pipe twice in a
row? Hmm, I guess it does actually do something but since nothing
significant would change the kernel likely just downgrades it to a
fastset. But maybe that's fine.
There are some reset vs. modeset type of tests in kms_flip, but those
are rather more carefully sequenced so they are probably not so great
at finding new races.
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> + output = igt_get_single_output_for_pipe(dpy, pipe);
> + if (!output)
> + continue;
> +
> + igt_output_set_pipe(output, pipe);
> + mode = igt_output_get_mode(output);
> +
> + if (fb.width != mode->hdisplay || fb.height != mode->vdisplay) {
> + igt_remove_fb(dpy->drm_fd, &fb);
> + igt_create_pattern_fb(dpy->drm_fd,
> + mode->hdisplay, mode->vdisplay,
> + DRM_FORMAT_XRGB8888,
> + LOCAL_I915_FORMAT_MOD_X_TILED,
> + &fb);
> + }
> +
> + primary = igt_output_get_plane_type(output,
> + DRM_PLANE_TYPE_PRIMARY);
> + igt_plane_set_fb(primary, &fb);
> +
> + igt_display_commit2(dpy, commit);
> + igt_display_reset(dpy);
> + }
> +
> + igt_remove_fb(dpy->drm_fd, &fb);
> +}
> +
> +static void test_kms(int i915, igt_display_t *dpy)
> +{
> + int *shared;
> +
> + shared = mmap(NULL, 4096, PROT_WRITE, MAP_SHARED | MAP_ANON, -1, 0);
> + igt_assert(shared != MAP_FAILED);
> +
> + igt_fork(child, 1)
> + display_helper(dpy, shared);
> +
> + test_reset_stress(i915, 0);
> + test_reset_stress(i915, TEST_WEDGE);
> +
> + *shared = 1;
> + igt_waitchildren();
> + munmap(shared, 4096);
> +}
> +
> static int fd = -1;
>
> static void
> @@ -906,4 +969,20 @@ igt_main
> }
> }
> }
> +
> + igt_subtest_group {
> + igt_display_t display = {
> + .drm_fd = -1, .n_pipes = IGT_MAX_PIPES
> + };
> +
> + igt_fixture {
> + igt_device_set_master(fd);
> +
> + igt_display_require(&display, fd);
> + igt_display_require_output(&display);
> + }
> +
> + igt_subtest("kms")
> + test_kms(fd, &display);
> + }
> }
> --
> 2.23.0
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread