| Message ID | 20180406100950.19033-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 06/04/2018 11:09, Chris Wilson wrote: > Tvrtko mentioned that wait_for_hang() was confusing as it does not > actually wait for the aforementioned hang, just until the request is > running and we are *ready* to inject a hang. A quick > s/wait_for_hang/wait_until_running/ removes that confusion without > having to rethink the naming scheme, immediately at least. > > Suggested-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > Cc: Michel Thierry <michel.thierry@intel.com> > --- > drivers/gpu/drm/i915/selftests/intel_hangcheck.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/selftests/intel_hangcheck.c b/drivers/gpu/drm/i915/selftests/intel_hangcheck.c > index d03abe7f8a53..8650853c8cb3 100644 > --- a/drivers/gpu/drm/i915/selftests/intel_hangcheck.c > +++ b/drivers/gpu/drm/i915/selftests/intel_hangcheck.c > @@ -322,7 +322,7 @@ static void hang_fini(struct hang *h) > flush_test(h->i915, I915_WAIT_LOCKED); > } > > -static bool wait_for_hang(struct hang *h, struct i915_request *rq) > +static bool wait_until_running(struct hang *h, struct i915_request *rq) > { > return !(wait_for_us(i915_seqno_passed(hws_seqno(h, rq), > rq->fence.seqno), > @@ -504,7 +504,7 @@ static int __igt_reset_engine(struct drm_i915_private *i915, bool active) > __i915_request_add(rq, true); > mutex_unlock(&i915->drm.struct_mutex); > > - if (!wait_for_hang(&h, rq)) { > + if (!wait_until_running(&h, rq)) { > struct drm_printer p = drm_info_printer(i915->drm.dev); > > pr_err("%s: Failed to start request %x, at %x\n", > @@ -747,7 +747,7 @@ static int __igt_reset_engines(struct drm_i915_private *i915, > __i915_request_add(rq, true); > mutex_unlock(&i915->drm.struct_mutex); > > - if (!wait_for_hang(&h, rq)) { > + if (!wait_until_running(&h, rq)) { > struct drm_printer p = drm_info_printer(i915->drm.dev); > > pr_err("%s: Failed to start request %x, at %x\n", > @@ -935,7 +935,7 @@ static int igt_wait_reset(void *arg) > i915_request_get(rq); > __i915_request_add(rq, true); > > - if (!wait_for_hang(&h, rq)) { > + if (!wait_until_running(&h, rq)) { > struct drm_printer p = drm_info_printer(i915->drm.dev); > > pr_err("%s: Failed to start request %x, at %x\n", > @@ -1066,7 +1066,7 @@ static int igt_reset_queue(void *arg) > goto fini; > } > > - if (!wait_for_hang(&h, prev)) { > + if (!wait_until_running(&h, prev)) { > struct drm_printer p = drm_info_printer(i915->drm.dev); > > pr_err("%s(%s): Failed to start request %x, at %x\n", > @@ -1177,7 +1177,7 @@ static int igt_handle_error(void *arg) > i915_request_get(rq); > __i915_request_add(rq, true); > > - if (!wait_for_hang(&h, rq)) { > + if (!wait_until_running(&h, rq)) { > struct drm_printer p = drm_info_printer(i915->drm.dev); > > pr_err("%s: Failed to start request %x, at %x\n", > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Regards, Tvrtko
Quoting Patchwork (2018-04-06 16:04:14) > == Series Details == > > Series: drm/i915/selftests: Rename wait_for_hang() to wait_until_running() > URL : https://patchwork.freedesktop.org/series/41264/ > State : failure > > == Summary == > > ---- Possible new issues: > > Test gem_pipe_control_store_loop: > Subgroup reused-buffer: > pass -> INCOMPLETE (shard-apl) <0>[ 499.756242] thermal thermal_zone0: critical temperature reached (100 C), shutting down Tomi, quick get the duster! > Test kms_atomic: > Subgroup plane_invalid_params_fence: > fail -> PASS (shard-snb) > Test kms_atomic_transition: > Subgroup plane-all-transition-fencing: > fail -> PASS (shard-snb) > Test kms_busy: > Subgroup extended-pageflip-modeset-hang-oldfb-render-b: > fail -> PASS (shard-snb) > Test kms_chv_cursor_fail: > Subgroup pipe-c-128x128-right-edge: > dmesg-warn -> PASS (shard-hsw) > Test kms_cursor_legacy: > Subgroup flip-vs-cursor-busy-crc-atomic: > fail -> PASS (shard-snb) > Subgroup flip-vs-cursor-busy-crc-legacy: > fail -> PASS (shard-snb) > Test kms_frontbuffer_tracking: > Subgroup fbc-2p-scndscrn-spr-indfb-draw-mmap-cpu: > fail -> SKIP (shard-snb) > Subgroup fbcpsr-rgb101010-draw-render: > fail -> SKIP (shard-snb) > Subgroup fbcpsr-slowdraw: > fail -> SKIP (shard-snb) > Subgroup psr-1p-offscren-pri-indfb-draw-pwrite: > fail -> SKIP (shard-snb) > Subgroup psr-1p-primscrn-pri-shrfb-draw-render: > fail -> SKIP (shard-snb) > Subgroup psr-1p-primscrn-spr-indfb-draw-mmap-wc: > fail -> SKIP (shard-snb) > Subgroup psr-2p-primscrn-cur-indfb-draw-mmap-wc: > fail -> SKIP (shard-snb) > Subgroup psr-2p-primscrn-spr-indfb-move: > fail -> SKIP (shard-snb) > Test kms_plane: > Subgroup pixel-format-pipe-b-planes: > fail -> PASS (shard-snb) > Test kms_vblank: > Subgroup pipe-a-ts-continuation-suspend: > fail -> PASS (shard-snb) > Test testdisplay: > warn -> PASS (shard-snb) Wow, that was noisy and all unconnected to the patch. Thanks for the suggestion and review, pushed. -Chris
diff --git a/drivers/gpu/drm/i915/selftests/intel_hangcheck.c b/drivers/gpu/drm/i915/selftests/intel_hangcheck.c index d03abe7f8a53..8650853c8cb3 100644 --- a/drivers/gpu/drm/i915/selftests/intel_hangcheck.c +++ b/drivers/gpu/drm/i915/selftests/intel_hangcheck.c @@ -322,7 +322,7 @@ static void hang_fini(struct hang *h) flush_test(h->i915, I915_WAIT_LOCKED); } -static bool wait_for_hang(struct hang *h, struct i915_request *rq) +static bool wait_until_running(struct hang *h, struct i915_request *rq) { return !(wait_for_us(i915_seqno_passed(hws_seqno(h, rq), rq->fence.seqno), @@ -504,7 +504,7 @@ static int __igt_reset_engine(struct drm_i915_private *i915, bool active) __i915_request_add(rq, true); mutex_unlock(&i915->drm.struct_mutex); - if (!wait_for_hang(&h, rq)) { + if (!wait_until_running(&h, rq)) { struct drm_printer p = drm_info_printer(i915->drm.dev); pr_err("%s: Failed to start request %x, at %x\n", @@ -747,7 +747,7 @@ static int __igt_reset_engines(struct drm_i915_private *i915, __i915_request_add(rq, true); mutex_unlock(&i915->drm.struct_mutex); - if (!wait_for_hang(&h, rq)) { + if (!wait_until_running(&h, rq)) { struct drm_printer p = drm_info_printer(i915->drm.dev); pr_err("%s: Failed to start request %x, at %x\n", @@ -935,7 +935,7 @@ static int igt_wait_reset(void *arg) i915_request_get(rq); __i915_request_add(rq, true); - if (!wait_for_hang(&h, rq)) { + if (!wait_until_running(&h, rq)) { struct drm_printer p = drm_info_printer(i915->drm.dev); pr_err("%s: Failed to start request %x, at %x\n", @@ -1066,7 +1066,7 @@ static int igt_reset_queue(void *arg) goto fini; } - if (!wait_for_hang(&h, prev)) { + if (!wait_until_running(&h, prev)) { struct drm_printer p = drm_info_printer(i915->drm.dev); pr_err("%s(%s): Failed to start request %x, at %x\n", @@ -1177,7 +1177,7 @@ static int igt_handle_error(void *arg) i915_request_get(rq); __i915_request_add(rq, true); - if (!wait_for_hang(&h, rq)) { + if (!wait_until_running(&h, rq)) { struct drm_printer p = drm_info_printer(i915->drm.dev); pr_err("%s: Failed to start request %x, at %x\n",
Tvrtko mentioned that wait_for_hang() was confusing as it does not actually wait for the aforementioned hang, just until the request is running and we are *ready* to inject a hang. A quick s/wait_for_hang/wait_until_running/ removes that confusion without having to rethink the naming scheme, immediately at least. Suggested-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Cc: Michel Thierry <michel.thierry@intel.com> --- drivers/gpu/drm/i915/selftests/intel_hangcheck.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)