| Message ID | 1502742720-6036-1-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Quoting Daniel Vetter (2017-08-14 21:32:00) > The goal of these subtests was just to delay a kms operation a little > bit. The goal wasn't to > - spin until the operation was completed. That results in a gpu hang, > and gpu hangs need igt_hang for safety. > - complete it before the operation. That's just pointless. It didn't use the arbitrary timeout to avoid the race condition of it finishing before the flip was queued. > Fix it by using the timeout support. Find the path that was broken and fix it. At a guess just the pair of TEST_DPMS need to consider how exactly they want to interact with the busy fb. -Chris
Op 14-08-17 om 22:32 schreef Daniel Vetter: > The goal of these subtests was just to delay a kms operation a little > bit. The goal wasn't to > - spin until the operation was completed. That results in a gpu hang, > and gpu hangs need igt_hang for safety. > - complete it before the operation. That's just pointless. > > Fix it by using the timeout support. > > This was all broken in the initial conversation: > > commit 96ec8cb3b5ec1fc2927d6cff8e09930082301d7e > Author: Abdiel Janulgue <abdiel.janulgue@linux.intel.com> > Date: Sat Oct 29 01:01:05 2016 +0300 > > igt/kms_flip: Use new igt_spin_batch > > Note that part of the damage was removed already in > > commit b0081ea9cb7d39234fd0fcc64dd56ed4f5d50b05 > Author: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Date: Wed Aug 9 11:11:52 2017 +0200 > > tests/kms_flip: Remove $engine-flip-vs-dpms/modeset > > Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com> > Cc: Abdiel Janulgue <abdiel.janulgue@linux.intel.com> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > --- > tests/kms_flip.c | 21 +++++++++------------ > 1 file changed, 9 insertions(+), 12 deletions(-) Use the NSEC_PER_SEC macro for readability? Otherwise looks sane.. Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > diff --git a/tests/kms_flip.c b/tests/kms_flip.c > index ea860e191a67..6d315b3793a8 100644 > --- a/tests/kms_flip.c > +++ b/tests/kms_flip.c > @@ -693,13 +693,19 @@ static unsigned int run_test_step(struct test_output *o) > if (!(o->flags & TEST_SINGLE_BUFFER)) > o->current_fb_id = !o->current_fb_id; > > - if (o->flags & TEST_WITH_DUMMY_BCS) > + if (o->flags & TEST_WITH_DUMMY_BCS) { > spin_bcs = igt_spin_batch_new(drm_fd, I915_EXEC_BLT, > o->fb_info[o->current_fb_id].gem_handle); > + igt_spin_batch_set_timeout(spin_bcs, > + 1000*1000*1000); > + } > > - if (o->flags & TEST_WITH_DUMMY_RCS) > + if (o->flags & TEST_WITH_DUMMY_RCS) { > spin_rcs = igt_spin_batch_new(drm_fd, I915_EXEC_RENDER, > o->fb_info[o->current_fb_id].gem_handle); > + igt_spin_batch_set_timeout(spin_rcs, > + 1000*1000*1000); > + } > > if (o->flags & TEST_FB_RECREATE) > recreate_fb(o); > @@ -789,13 +795,8 @@ static unsigned int run_test_step(struct test_output *o) > igt_assert(__wait_for_vblank(TEST_VBLANK_BLOCK, o->pipe, 1, 0, &reply) == 0); > } > > - if (do_flip) { > + if (do_flip) > do_or_die(do_page_flip(o, new_fb_id, !(o->flags & TEST_NOEVENT))); > - if (spin_rcs) > - igt_spin_batch_end(spin_rcs); > - if (spin_bcs) > - igt_spin_batch_end(spin_bcs); > - } > > if (o->flags & TEST_FENCE_STRESS) > emit_fence_stress(o); > @@ -809,10 +810,6 @@ static unsigned int run_test_step(struct test_output *o) > vbl_reply.ts.tv_usec); > completed_events = EVENT_VBLANK; > } > - if (spin_rcs) > - igt_spin_batch_end(spin_rcs); > - if (spin_bcs) > - igt_spin_batch_end(spin_bcs); > } > if (spin_rcs) > igt_spin_batch_free(drm_fd, spin_rcs);
diff --git a/tests/kms_flip.c b/tests/kms_flip.c index ea860e191a67..6d315b3793a8 100644 --- a/tests/kms_flip.c +++ b/tests/kms_flip.c @@ -693,13 +693,19 @@ static unsigned int run_test_step(struct test_output *o) if (!(o->flags & TEST_SINGLE_BUFFER)) o->current_fb_id = !o->current_fb_id; - if (o->flags & TEST_WITH_DUMMY_BCS) + if (o->flags & TEST_WITH_DUMMY_BCS) { spin_bcs = igt_spin_batch_new(drm_fd, I915_EXEC_BLT, o->fb_info[o->current_fb_id].gem_handle); + igt_spin_batch_set_timeout(spin_bcs, + 1000*1000*1000); + } - if (o->flags & TEST_WITH_DUMMY_RCS) + if (o->flags & TEST_WITH_DUMMY_RCS) { spin_rcs = igt_spin_batch_new(drm_fd, I915_EXEC_RENDER, o->fb_info[o->current_fb_id].gem_handle); + igt_spin_batch_set_timeout(spin_rcs, + 1000*1000*1000); + } if (o->flags & TEST_FB_RECREATE) recreate_fb(o); @@ -789,13 +795,8 @@ static unsigned int run_test_step(struct test_output *o) igt_assert(__wait_for_vblank(TEST_VBLANK_BLOCK, o->pipe, 1, 0, &reply) == 0); } - if (do_flip) { + if (do_flip) do_or_die(do_page_flip(o, new_fb_id, !(o->flags & TEST_NOEVENT))); - if (spin_rcs) - igt_spin_batch_end(spin_rcs); - if (spin_bcs) - igt_spin_batch_end(spin_bcs); - } if (o->flags & TEST_FENCE_STRESS) emit_fence_stress(o); @@ -809,10 +810,6 @@ static unsigned int run_test_step(struct test_output *o) vbl_reply.ts.tv_usec); completed_events = EVENT_VBLANK; } - if (spin_rcs) - igt_spin_batch_end(spin_rcs); - if (spin_bcs) - igt_spin_batch_end(spin_bcs); } if (spin_rcs) igt_spin_batch_free(drm_fd, spin_rcs);
The goal of these subtests was just to delay a kms operation a little bit. The goal wasn't to - spin until the operation was completed. That results in a gpu hang, and gpu hangs need igt_hang for safety. - complete it before the operation. That's just pointless. Fix it by using the timeout support. This was all broken in the initial conversation: commit 96ec8cb3b5ec1fc2927d6cff8e09930082301d7e Author: Abdiel Janulgue <abdiel.janulgue@linux.intel.com> Date: Sat Oct 29 01:01:05 2016 +0300 igt/kms_flip: Use new igt_spin_batch Note that part of the damage was removed already in commit b0081ea9cb7d39234fd0fcc64dd56ed4f5d50b05 Author: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Date: Wed Aug 9 11:11:52 2017 +0200 tests/kms_flip: Remove $engine-flip-vs-dpms/modeset Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com> Cc: Abdiel Janulgue <abdiel.janulgue@linux.intel.com> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> --- tests/kms_flip.c | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-)