* [PATCH] drm/i915: Always kick the execlists tasklet after reset
@ 2019-03-13 16:28 Chris Wilson
2019-03-14 1:38 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Chris Wilson @ 2019-03-13 16:28 UTC (permalink / raw)
To: intel-gfx
With direct submission being disabled while the reset in progress, we
have a small window where we may forgo the submission of a new request
and not notice its addition during execlists_reset_finish. To close this
window, always schedule the submission tasklet on coming out of reset to
catch any residual work.
<6> [333.144082] i915: Running intel_hangcheck_live_selftests/igt_reset_engines
<3> [333.296927] i915_reset_engine(rcs0:idle): failed to idle after reset
<6> [333.296932] i915 0000:00:02.0: [drm] rcs0
<6> [333.296934] i915 0000:00:02.0: [drm] Hangcheck 0:a9ddf7a5 [4157 ms]
<6> [333.296936] i915 0000:00:02.0: [drm] Reset count: 36048 (global 754)
<6> [333.296938] i915 0000:00:02.0: [drm] Requests:
<6> [333.296997] i915 0000:00:02.0: [drm] RING_START: 0x00000000
<6> [333.296999] i915 0000:00:02.0: [drm] RING_HEAD: 0x00000000
<6> [333.297001] i915 0000:00:02.0: [drm] RING_TAIL: 0x00000000
<6> [333.297003] i915 0000:00:02.0: [drm] RING_CTL: 0x00000000
<6> [333.297005] i915 0000:00:02.0: [drm] RING_MODE: 0x00000200 [idle]
<6> [333.297007] i915 0000:00:02.0: [drm] RING_IMR: fffffeff
<6> [333.297010] i915 0000:00:02.0: [drm] ACTHD: 0x00000000_00000000
<6> [333.297012] i915 0000:00:02.0: [drm] BBADDR: 0x00000000_00000000
<6> [333.297015] i915 0000:00:02.0: [drm] DMA_FADDR: 0x00000000_00000000
<6> [333.297017] i915 0000:00:02.0: [drm] IPEIR: 0x00000000
<6> [333.297019] i915 0000:00:02.0: [drm] IPEHR: 0x00000000
<6> [333.297021] i915 0000:00:02.0: [drm] Execlist status: 0x00000001 00000000
<6> [333.297023] i915 0000:00:02.0: [drm] Execlist CSB read 5, write 5 [mmio:7], tasklet queued? no (enabled)
<6> [333.297025] i915 0000:00:02.0: [drm] ELSP[0] idle
<6> [333.297027] i915 0000:00:02.0: [drm] ELSP[1] idle
<6> [333.297028] i915 0000:00:02.0: [drm] HW active? 0x0
<6> [333.297044] i915 0000:00:02.0: [drm] Queue priority hint: -8186
<6> [333.297067] i915 0000:00:02.0: [drm] Q 2afac:5f2+ prio=-8186 @ 50ms: (null)
<6> [333.297068] i915 0000:00:02.0: [drm] HWSP:
<6> [333.297071] i915 0000:00:02.0: [drm] [0000] 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
<6> [333.297073] i915 0000:00:02.0: [drm] *
<6> [333.297075] i915 0000:00:02.0: [drm] [0040] 00000001 00000000 00000018 00000002 00000001 00000000 00000018 00000000
<6> [333.297077] i915 0000:00:02.0: [drm] [0060] 00000001 00000000 00008002 00000002 00000000 00000000 00000000 00000005
<6> [333.297079] i915 0000:00:02.0: [drm] [0080] 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
<6> [333.297081] i915 0000:00:02.0: [drm] *
<6> [333.297083] i915 0000:00:02.0: [drm] [00c0] 00000000 00000000 00000000 00000000 a9ddf7a5 00000000 00000000 00000000
<6> [333.297085] i915 0000:00:02.0: [drm] [00e0] 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
<6> [333.297087] i915 0000:00:02.0: [drm] *
<6> [333.297089] i915 0000:00:02.0: [drm] Idle? no
<6> [333.297090] i915_reset_engine(rcs0:idle): 3000 resets
<3> [333.297092] i915/intel_hangcheck_live_selftests: igt_reset_engines failed with error -5
<3> [333.455460] i915 0000:00:02.0: Failed to idle engines, declaring wedged!
...
<0> [333.491294] i915_sel-4916 1.... 333262143us : i915_reset_engine: rcs0 flags=4
<0> [333.491328] i915_sel-4916 1.... 333262143us : execlists_reset_prepare: rcs0: depth<-0
<0> [333.491362] i915_sel-4916 1.... 333262143us : intel_engine_stop_cs: rcs0
<0> [333.491396] i915_sel-4916 1d..1 333262144us : process_csb: rcs0 cs-irq head=5, tail=5
<0> [333.491424] i915_sel-4916 1.... 333262145us : intel_gpu_reset: engine_mask=1
<0> [333.491454] kworker/-214 5.... 333262184us : i915_gem_switch_to_kernel_context: awake?=yes
<0> [333.491487] kworker/-214 5.... 333262192us : i915_request_add: rcs0 fence 2afac:1522
<0> [333.491520] kworker/-214 5.... 333262193us : i915_request_add: marking (null) as active
<0> [333.491553] i915_sel-4916 1.... 333262199us : intel_engine_cancel_stop_cs: rcs0
<0> [333.491587] i915_sel-4916 1.... 333262199us : execlists_reset_finish: rcs0: depth->0
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
drivers/gpu/drm/i915/i915_gem.h | 7 ++++++-
drivers/gpu/drm/i915/intel_lrc.c | 3 ++-
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.h b/drivers/gpu/drm/i915/i915_gem.h
index 74a2ddc1b52f..5c073fe73664 100644
--- a/drivers/gpu/drm/i915/i915_gem.h
+++ b/drivers/gpu/drm/i915/i915_gem.h
@@ -82,7 +82,7 @@ void i915_gem_unpark(struct drm_i915_private *i915);
static inline void __tasklet_disable_sync_once(struct tasklet_struct *t)
{
- if (atomic_inc_return(&t->count) == 1)
+ if (!atomic_fetch_inc(&t->count))
tasklet_unlock_wait(t);
}
@@ -91,4 +91,9 @@ static inline bool __tasklet_is_enabled(const struct tasklet_struct *t)
return !atomic_read(&t->count);
}
+static inline bool __tasklet_enable(struct tasklet_struct *t)
+{
+ return atomic_dec_and_test(&t->count);
+}
+
#endif /* __I915_GEM_H__ */
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index dc3de09c7586..b2d0e16645c7 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -2030,7 +2030,8 @@ static void execlists_reset_finish(struct intel_engine_cs *engine)
if (!RB_EMPTY_ROOT(&execlists->queue.rb_root))
execlists->tasklet.func(execlists->tasklet.data);
- tasklet_enable(&execlists->tasklet);
+ if (__tasklet_enable(&execlists->tasklet))
+ tasklet_hi_schedule(&execlists->tasklet);
GEM_TRACE("%s: depth->%d\n", engine->name,
atomic_read(&execlists->tasklet.count));
}
--
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] 7+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Always kick the execlists tasklet after reset
2019-03-13 16:28 [PATCH] drm/i915: Always kick the execlists tasklet after reset Chris Wilson
@ 2019-03-14 1:38 ` Patchwork
2019-03-14 1:58 ` ✗ Fi.CI.BAT: failure " Patchwork
2019-03-15 10:10 ` [PATCH] " Mika Kuoppala
2 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2019-03-14 1:38 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Always kick the execlists tasklet after reset
URL : https://patchwork.freedesktop.org/series/57947/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
1e079bce5d0e drm/i915: Always kick the execlists tasklet after reset
-:12: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#12:
<6> [333.144082] i915: Running intel_hangcheck_live_selftests/igt_reset_engines
total: 0 errors, 1 warnings, 0 checks, 26 lines checked
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* ✗ Fi.CI.BAT: failure for drm/i915: Always kick the execlists tasklet after reset
2019-03-13 16:28 [PATCH] drm/i915: Always kick the execlists tasklet after reset Chris Wilson
2019-03-14 1:38 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
@ 2019-03-14 1:58 ` Patchwork
2019-03-15 10:10 ` [PATCH] " Mika Kuoppala
2 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2019-03-14 1:58 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Always kick the execlists tasklet after reset
URL : https://patchwork.freedesktop.org/series/57947/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_5742 -> Patchwork_12452
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_12452 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_12452, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://patchwork.freedesktop.org/api/1.0/series/57947/revisions/1/mbox/
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_12452:
### IGT changes ###
#### Possible regressions ####
* igt@gem_basic@bad-close:
- fi-icl-u3: PASS -> INCOMPLETE
Known issues
------------
Here are the changes found in Patchwork_12452 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@amdgpu/amd_basic@userptr:
- fi-whl-u: NOTRUN -> SKIP [fdo#109271] +41
* igt@amdgpu/amd_cs_nop@fork-gfx0:
- fi-icl-u2: NOTRUN -> SKIP [fdo#109315] +17
* igt@gem_ctx_create@basic-files:
- fi-gdg-551: NOTRUN -> SKIP [fdo#109271] +106
* igt@gem_exec_basic@readonly-bsd1:
- fi-snb-2520m: NOTRUN -> SKIP [fdo#109271] +57
- fi-icl-u2: NOTRUN -> SKIP [fdo#109276] +7
* igt@gem_exec_parse@basic-allowed:
- fi-icl-u2: NOTRUN -> SKIP [fdo#109289] +1
* igt@i915_selftest@live_contexts:
- fi-icl-u2: NOTRUN -> DMESG-FAIL [fdo#108569]
* igt@kms_busy@basic-flip-a:
- fi-gdg-551: NOTRUN -> FAIL [fdo#103182] +1
* igt@kms_busy@basic-flip-c:
- fi-gdg-551: NOTRUN -> SKIP [fdo#109271] / [fdo#109278]
- fi-snb-2520m: NOTRUN -> SKIP [fdo#109271] / [fdo#109278]
* igt@kms_chamelium@dp-edid-read:
- fi-icl-u2: NOTRUN -> SKIP [fdo#109316] +2
* igt@kms_chamelium@hdmi-edid-read:
- fi-hsw-peppy: NOTRUN -> SKIP [fdo#109271] +46
* igt@kms_chamelium@vga-hpd-fast:
- fi-icl-u2: NOTRUN -> SKIP [fdo#109309] +1
* igt@kms_force_connector_basic@prune-stale-modes:
- fi-icl-u2: NOTRUN -> SKIP [fdo#109285] +3
* igt@kms_frontbuffer_tracking@basic:
- fi-hsw-peppy: NOTRUN -> DMESG-FAIL [fdo#102614] / [fdo#107814]
- fi-icl-u2: NOTRUN -> FAIL [fdo#103167]
* igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
- fi-byt-clapper: PASS -> FAIL [fdo#103191] / [fdo#107362] +1
* igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
- fi-bdw-5557u: PASS -> FAIL [fdo#103375]
* igt@kms_psr@cursor_plane_move:
- fi-whl-u: NOTRUN -> FAIL [fdo#107383] +3
* igt@runner@aborted:
- fi-bxt-dsi: NOTRUN -> FAIL [fdo#109516]
#### Possible fixes ####
* igt@i915_selftest@live_evict:
- fi-bsw-kefka: DMESG-WARN [fdo#107709] -> PASS
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
[fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
[fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
[fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
[fdo#103375]: https://bugs.freedesktop.org/show_bug.cgi?id=103375
[fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
[fdo#107383]: https://bugs.freedesktop.org/show_bug.cgi?id=107383
[fdo#107709]: https://bugs.freedesktop.org/show_bug.cgi?id=107709
[fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
[fdo#107814]: https://bugs.freedesktop.org/show_bug.cgi?id=107814
[fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
[fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
[fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
[fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
[fdo#109309]: https://bugs.freedesktop.org/show_bug.cgi?id=109309
[fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
[fdo#109316]: https://bugs.freedesktop.org/show_bug.cgi?id=109316
[fdo#109516]: https://bugs.freedesktop.org/show_bug.cgi?id=109516
Participating hosts (42 -> 43)
------------------------------
Additional (6): fi-bxt-dsi fi-hsw-peppy fi-icl-u2 fi-snb-2520m fi-whl-u fi-gdg-551
Missing (5): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-bdw-samus
Build changes
-------------
* Linux: CI_DRM_5742 -> Patchwork_12452
CI_DRM_5742: 519a3f67aadea184cb97720c838946c1ba81c875 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4884: c46051337b972f8b5a302afb6f603df06fea527d @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_12452: 1e079bce5d0e294b81e58491893b112a9527569f @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
1e079bce5d0e drm/i915: Always kick the execlists tasklet after reset
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12452/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: Always kick the execlists tasklet after reset
2019-03-13 16:28 [PATCH] drm/i915: Always kick the execlists tasklet after reset Chris Wilson
2019-03-14 1:38 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2019-03-14 1:58 ` ✗ Fi.CI.BAT: failure " Patchwork
@ 2019-03-15 10:10 ` Mika Kuoppala
2019-03-15 10:14 ` Chris Wilson
2 siblings, 1 reply; 7+ messages in thread
From: Mika Kuoppala @ 2019-03-15 10:10 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
Chris Wilson <chris@chris-wilson.co.uk> writes:
> With direct submission being disabled while the reset in progress, we
> have a small window where we may forgo the submission of a new request
> and not notice its addition during execlists_reset_finish. To close this
> window, always schedule the submission tasklet on coming out of reset to
> catch any residual work.
>
> <6> [333.144082] i915: Running intel_hangcheck_live_selftests/igt_reset_engines
> <3> [333.296927] i915_reset_engine(rcs0:idle): failed to idle after reset
> <6> [333.296932] i915 0000:00:02.0: [drm] rcs0
> <6> [333.296934] i915 0000:00:02.0: [drm] Hangcheck 0:a9ddf7a5 [4157 ms]
> <6> [333.296936] i915 0000:00:02.0: [drm] Reset count: 36048 (global 754)
> <6> [333.296938] i915 0000:00:02.0: [drm] Requests:
> <6> [333.296997] i915 0000:00:02.0: [drm] RING_START: 0x00000000
> <6> [333.296999] i915 0000:00:02.0: [drm] RING_HEAD: 0x00000000
> <6> [333.297001] i915 0000:00:02.0: [drm] RING_TAIL: 0x00000000
> <6> [333.297003] i915 0000:00:02.0: [drm] RING_CTL: 0x00000000
> <6> [333.297005] i915 0000:00:02.0: [drm] RING_MODE: 0x00000200 [idle]
> <6> [333.297007] i915 0000:00:02.0: [drm] RING_IMR: fffffeff
> <6> [333.297010] i915 0000:00:02.0: [drm] ACTHD: 0x00000000_00000000
> <6> [333.297012] i915 0000:00:02.0: [drm] BBADDR: 0x00000000_00000000
> <6> [333.297015] i915 0000:00:02.0: [drm] DMA_FADDR: 0x00000000_00000000
> <6> [333.297017] i915 0000:00:02.0: [drm] IPEIR: 0x00000000
> <6> [333.297019] i915 0000:00:02.0: [drm] IPEHR: 0x00000000
> <6> [333.297021] i915 0000:00:02.0: [drm] Execlist status: 0x00000001 00000000
> <6> [333.297023] i915 0000:00:02.0: [drm] Execlist CSB read 5, write 5 [mmio:7], tasklet queued? no (enabled)
> <6> [333.297025] i915 0000:00:02.0: [drm] ELSP[0] idle
> <6> [333.297027] i915 0000:00:02.0: [drm] ELSP[1] idle
> <6> [333.297028] i915 0000:00:02.0: [drm] HW active? 0x0
> <6> [333.297044] i915 0000:00:02.0: [drm] Queue priority hint: -8186
> <6> [333.297067] i915 0000:00:02.0: [drm] Q 2afac:5f2+ prio=-8186 @ 50ms: (null)
> <6> [333.297068] i915 0000:00:02.0: [drm] HWSP:
> <6> [333.297071] i915 0000:00:02.0: [drm] [0000] 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> <6> [333.297073] i915 0000:00:02.0: [drm] *
> <6> [333.297075] i915 0000:00:02.0: [drm] [0040] 00000001 00000000 00000018 00000002 00000001 00000000 00000018 00000000
> <6> [333.297077] i915 0000:00:02.0: [drm] [0060] 00000001 00000000 00008002 00000002 00000000 00000000 00000000 00000005
> <6> [333.297079] i915 0000:00:02.0: [drm] [0080] 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> <6> [333.297081] i915 0000:00:02.0: [drm] *
> <6> [333.297083] i915 0000:00:02.0: [drm] [00c0] 00000000 00000000 00000000 00000000 a9ddf7a5 00000000 00000000 00000000
> <6> [333.297085] i915 0000:00:02.0: [drm] [00e0] 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> <6> [333.297087] i915 0000:00:02.0: [drm] *
> <6> [333.297089] i915 0000:00:02.0: [drm] Idle? no
> <6> [333.297090] i915_reset_engine(rcs0:idle): 3000 resets
> <3> [333.297092] i915/intel_hangcheck_live_selftests: igt_reset_engines failed with error -5
> <3> [333.455460] i915 0000:00:02.0: Failed to idle engines, declaring wedged!
> ...
> <0> [333.491294] i915_sel-4916 1.... 333262143us : i915_reset_engine: rcs0 flags=4
> <0> [333.491328] i915_sel-4916 1.... 333262143us : execlists_reset_prepare: rcs0: depth<-0
> <0> [333.491362] i915_sel-4916 1.... 333262143us : intel_engine_stop_cs: rcs0
> <0> [333.491396] i915_sel-4916 1d..1 333262144us : process_csb: rcs0 cs-irq head=5, tail=5
> <0> [333.491424] i915_sel-4916 1.... 333262145us : intel_gpu_reset: engine_mask=1
> <0> [333.491454] kworker/-214 5.... 333262184us : i915_gem_switch_to_kernel_context: awake?=yes
> <0> [333.491487] kworker/-214 5.... 333262192us : i915_request_add: rcs0 fence 2afac:1522
> <0> [333.491520] kworker/-214 5.... 333262193us : i915_request_add: marking (null) as active
> <0> [333.491553] i915_sel-4916 1.... 333262199us : intel_engine_cancel_stop_cs: rcs0
> <0> [333.491587] i915_sel-4916 1.... 333262199us : execlists_reset_finish: rcs0: depth->0
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem.h | 7 ++++++-
> drivers/gpu/drm/i915/intel_lrc.c | 3 ++-
> 2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.h b/drivers/gpu/drm/i915/i915_gem.h
> index 74a2ddc1b52f..5c073fe73664 100644
> --- a/drivers/gpu/drm/i915/i915_gem.h
> +++ b/drivers/gpu/drm/i915/i915_gem.h
> @@ -82,7 +82,7 @@ void i915_gem_unpark(struct drm_i915_private *i915);
>
> static inline void __tasklet_disable_sync_once(struct tasklet_struct *t)
> {
> - if (atomic_inc_return(&t->count) == 1)
> + if (!atomic_fetch_inc(&t->count))
> tasklet_unlock_wait(t);
> }
>
> @@ -91,4 +91,9 @@ static inline bool __tasklet_is_enabled(const struct tasklet_struct *t)
> return !atomic_read(&t->count);
> }
>
> +static inline bool __tasklet_enable(struct tasklet_struct *t)
> +{
> + return atomic_dec_and_test(&t->count);
> +}
> +
> #endif /* __I915_GEM_H__ */
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index dc3de09c7586..b2d0e16645c7 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -2030,7 +2030,8 @@ static void execlists_reset_finish(struct intel_engine_cs *engine)
> if (!RB_EMPTY_ROOT(&execlists->queue.rb_root))
> execlists->tasklet.func(execlists->tasklet.data);
>
> - tasklet_enable(&execlists->tasklet);
> + if (__tasklet_enable(&execlists->tasklet))
> + tasklet_hi_schedule(&execlists->tasklet);
Why not just go fully unconditional, enable and schedule?
-Mika
> GEM_TRACE("%s: depth->%d\n", engine->name,
> atomic_read(&execlists->tasklet.count));
> }
> --
> 2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: Always kick the execlists tasklet after reset
2019-03-15 10:10 ` [PATCH] " Mika Kuoppala
@ 2019-03-15 10:14 ` Chris Wilson
2019-03-15 10:39 ` Mika Kuoppala
0 siblings, 1 reply; 7+ messages in thread
From: Chris Wilson @ 2019-03-15 10:14 UTC (permalink / raw)
To: Mika Kuoppala, intel-gfx
Quoting Mika Kuoppala (2019-03-15 10:10:20)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> > +static inline bool __tasklet_enable(struct tasklet_struct *t)
> > +{
> > + return atomic_dec_and_test(&t->count);
> > +}
> > +
> > #endif /* __I915_GEM_H__ */
> > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> > index dc3de09c7586..b2d0e16645c7 100644
> > --- a/drivers/gpu/drm/i915/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/intel_lrc.c
> > @@ -2030,7 +2030,8 @@ static void execlists_reset_finish(struct intel_engine_cs *engine)
> > if (!RB_EMPTY_ROOT(&execlists->queue.rb_root))
> > execlists->tasklet.func(execlists->tasklet.data);
> >
> > - tasklet_enable(&execlists->tasklet);
> > + if (__tasklet_enable(&execlists->tasklet))
> > + tasklet_hi_schedule(&execlists->tasklet);
>
> Why not just go fully unconditional, enable and schedule?
If we schedule before we finish the reset, the tasklet busyspins, get's
kicked to ksoftirqd, which then busyspins for its timeslice and gets
bumped around by the scheduler until finally ready.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: Always kick the execlists tasklet after reset
2019-03-15 10:14 ` Chris Wilson
@ 2019-03-15 10:39 ` Mika Kuoppala
2019-03-15 10:51 ` Chris Wilson
0 siblings, 1 reply; 7+ messages in thread
From: Mika Kuoppala @ 2019-03-15 10:39 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
Chris Wilson <chris@chris-wilson.co.uk> writes:
> Quoting Mika Kuoppala (2019-03-15 10:10:20)
>> Chris Wilson <chris@chris-wilson.co.uk> writes:
>> > +static inline bool __tasklet_enable(struct tasklet_struct *t)
>> > +{
>> > + return atomic_dec_and_test(&t->count);
>> > +}
>> > +
>> > #endif /* __I915_GEM_H__ */
>> > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
>> > index dc3de09c7586..b2d0e16645c7 100644
>> > --- a/drivers/gpu/drm/i915/intel_lrc.c
>> > +++ b/drivers/gpu/drm/i915/intel_lrc.c
>> > @@ -2030,7 +2030,8 @@ static void execlists_reset_finish(struct intel_engine_cs *engine)
>> > if (!RB_EMPTY_ROOT(&execlists->queue.rb_root))
>> > execlists->tasklet.func(execlists->tasklet.data);
>> >
>> > - tasklet_enable(&execlists->tasklet);
>> > + if (__tasklet_enable(&execlists->tasklet))
>> > + tasklet_hi_schedule(&execlists->tasklet);
>>
>> Why not just go fully unconditional, enable and schedule?
>
> If we schedule before we finish the reset, the tasklet busyspins, get's
> kicked to ksoftirqd, which then busyspins for its timeslice and gets
> bumped around by the scheduler until finally ready.
Hmm stealing the whole timeslice is rude. Just wanted
to weight in the extra trickery. Random thought was
to advocate for execlists_tasklet_enable|disable,
but perhaps we are not there yet.
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: Always kick the execlists tasklet after reset
2019-03-15 10:39 ` Mika Kuoppala
@ 2019-03-15 10:51 ` Chris Wilson
0 siblings, 0 replies; 7+ messages in thread
From: Chris Wilson @ 2019-03-15 10:51 UTC (permalink / raw)
To: Mika Kuoppala, intel-gfx
Quoting Mika Kuoppala (2019-03-15 10:39:25)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
>
> > Quoting Mika Kuoppala (2019-03-15 10:10:20)
> >> Chris Wilson <chris@chris-wilson.co.uk> writes:
> >> > +static inline bool __tasklet_enable(struct tasklet_struct *t)
> >> > +{
> >> > + return atomic_dec_and_test(&t->count);
> >> > +}
> >> > +
> >> > #endif /* __I915_GEM_H__ */
> >> > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> >> > index dc3de09c7586..b2d0e16645c7 100644
> >> > --- a/drivers/gpu/drm/i915/intel_lrc.c
> >> > +++ b/drivers/gpu/drm/i915/intel_lrc.c
> >> > @@ -2030,7 +2030,8 @@ static void execlists_reset_finish(struct intel_engine_cs *engine)
> >> > if (!RB_EMPTY_ROOT(&execlists->queue.rb_root))
> >> > execlists->tasklet.func(execlists->tasklet.data);
> >> >
> >> > - tasklet_enable(&execlists->tasklet);
> >> > + if (__tasklet_enable(&execlists->tasklet))
> >> > + tasklet_hi_schedule(&execlists->tasklet);
> >>
> >> Why not just go fully unconditional, enable and schedule?
> >
> > If we schedule before we finish the reset, the tasklet busyspins, get's
> > kicked to ksoftirqd, which then busyspins for its timeslice and gets
> > bumped around by the scheduler until finally ready.
>
> Hmm stealing the whole timeslice is rude. Just wanted
> to weight in the extra trickery. Random thought was
> to advocate for execlists_tasklet_enable|disable,
> but perhaps we are not there yet.
Keep sowing those seeds!
By the way, remind yourself on how our execlists submission works...
https://patchwork.freedesktop.org/patch/291941/?series=57942&rev=2
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-03-15 10:51 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-13 16:28 [PATCH] drm/i915: Always kick the execlists tasklet after reset Chris Wilson
2019-03-14 1:38 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2019-03-14 1:58 ` ✗ Fi.CI.BAT: failure " Patchwork
2019-03-15 10:10 ` [PATCH] " Mika Kuoppala
2019-03-15 10:14 ` Chris Wilson
2019-03-15 10:39 ` Mika Kuoppala
2019-03-15 10:51 ` Chris Wilson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.