[PATCH 1/4] drm/i915/execlists: Assert the queue is non-empty on unsubmitting

intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed

* [PATCH 1/4] drm/i915/execlists: Assert the queue is non-empty on unsubmitting
@ 2018-08-15 14:34 Chris Wilson
  2018-08-15 14:34 ` [PATCH 2/4] drm/i915/execlists: Reset queue_priority on cancellation Chris Wilson
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Chris Wilson @ 2018-08-15 14:34 UTC (permalink / raw)
  To: intel-gfx

In the sequence

<0>[  531.960431] drv_self-4806    7.... 527402570us : intel_gpu_reset: engine_mask=1, ret=0, retry=0
<0>[  531.960431] drv_self-4806    7.... 527402571us : execlists_reset: rcs0 request global=115de, current=71133
<0>[  531.960431] drv_self-4806    7d..1 527402571us : execlists_cancel_port_requests: rcs0:port0 global=71134 (fence 826b:198), (current 71133)
<0>[  531.960431] drv_self-4806    7d..1 527402572us : execlists_cancel_port_requests: rcs0:port1 global=71135 (fence 826c:53), (current 71133)
<0>[  531.960431] drv_self-4806    7d..1 527402572us : __i915_request_unsubmit: rcs0 fence 826c:53 <- global=71135, current 71133
<0>[  531.960431] drv_self-4806    7d..1 527402579us : __i915_request_unsubmit: rcs0 fence 826b:198 <- global=71134, current 71133
<0>[  531.960431] drv_self-4806    7.... 527402613us : intel_engine_cancel_stop_cs: rcs0
<0>[  531.960431] drv_self-4806    7.... 527402624us : execlists_reset_finish: rcs0

we are missing the execlists_submission_tasklet() invocation before the
execlists_reset_fini() implying that either the queue is empty, or we
failed to schedule and run the tasklet on finish. Add an assert so we
are sure that on unsubmitting the incomplete request after reset, the
queue is indeed populated.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_lrc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 3f90c74038ef..0e90fd831c70 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -344,6 +344,7 @@ static void __unwind_incomplete_requests(struct intel_engine_cs *engine)
 			last_prio = rq_prio(rq);
 			p = lookup_priolist(engine, last_prio);
 		}
+		GEM_BUG_ON(RB_EMPTY_ROOT(&engine->execlists.queue.rb_root));
 
 		GEM_BUG_ON(p->priority != rq_prio(rq));
 		list_add(&rq->sched.link, &p->requests);
-- 
2.18.0

_______________________________________________
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

* [PATCH 2/4] drm/i915/execlists: Reset queue_priority on cancellation
  2018-08-15 14:34 [PATCH 1/4] drm/i915/execlists: Assert the queue is non-empty on unsubmitting Chris Wilson
@ 2018-08-15 14:34 ` Chris Wilson
  2018-08-15 14:34 ` [PATCH 3/4] drm/i915/execlists: Include reset depth in traces Chris Wilson
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Chris Wilson @ 2018-08-15 14:34 UTC (permalink / raw)
  To: intel-gfx

If we have an available execlists port, the queue_priority should be
INT_MIN to allow immediate direct submission. As we clear the ports in
execlists_cancel_port_requests(), we should then reset queue_priority to
show the available space.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_lrc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 0e90fd831c70..178a74e29400 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -800,6 +800,7 @@ execlists_cancel_port_requests(struct intel_engine_execlists * const execlists)
 	}
 
 	execlists_clear_all_active(execlists);
+	execlists->queue_priority = INT_MIN;
 }
 
 static void reset_csb_pointers(struct intel_engine_execlists *execlists)
-- 
2.18.0

_______________________________________________
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

* [PATCH 3/4] drm/i915/execlists: Include reset depth in traces
  2018-08-15 14:34 [PATCH 1/4] drm/i915/execlists: Assert the queue is non-empty on unsubmitting Chris Wilson
  2018-08-15 14:34 ` [PATCH 2/4] drm/i915/execlists: Reset queue_priority on cancellation Chris Wilson
@ 2018-08-15 14:34 ` Chris Wilson
  2018-08-15 14:34 ` [PATCH 4/4] drm/i915/execlists: Use direct submission to restart HW after reset Chris Wilson
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Chris Wilson @ 2018-08-15 14:34 UTC (permalink / raw)
  To: intel-gfx

Show the reset depth (the tasklet disable count) in the GEM_TRACE to
indicate when we might not expect tasklets to be flushed.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_guc_submission.c | 3 ++-
 drivers/gpu/drm/i915/intel_lrc.c            | 6 ++++--
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
index 195adbd0ebf7..8f43c2fe90ca 100644
--- a/drivers/gpu/drm/i915/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/intel_guc_submission.c
@@ -815,7 +815,8 @@ guc_reset_prepare(struct intel_engine_cs *engine)
 {
 	struct intel_engine_execlists * const execlists = &engine->execlists;
 
-	GEM_TRACE("%s\n", engine->name);
+	GEM_TRACE("%s: depth<-%d\n", engine->name,
+		  atomic_read(&execlists->tasklet.count));
 
 	/*
 	 * Prevent request submission to the hardware until we have
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 178a74e29400..257d6f5f44d1 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1834,7 +1834,8 @@ execlists_reset_prepare(struct intel_engine_cs *engine)
 	struct i915_request *request, *active;
 	unsigned long flags;
 
-	GEM_TRACE("%s\n", engine->name);
+	GEM_TRACE("%s: depth<-%d\n", engine->name,
+		  atomic_read(&execlists->tasklet.count));
 
 	/*
 	 * Prevent request submission to the hardware until we have
@@ -1982,7 +1983,8 @@ static void execlists_reset_finish(struct intel_engine_cs *engine)
 	 */
 	__tasklet_enable_sync_once(&execlists->tasklet);
 
-	GEM_TRACE("%s\n", engine->name);
+	GEM_TRACE("%s: depth->%d\n", engine->name,
+		  atomic_read(&execlists->tasklet.count));
 }
 
 static int intel_logical_ring_emit_pdps(struct i915_request *rq)
-- 
2.18.0

_______________________________________________
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

* [PATCH 4/4] drm/i915/execlists: Use direct submission to restart HW after reset
  2018-08-15 14:34 [PATCH 1/4] drm/i915/execlists: Assert the queue is non-empty on unsubmitting Chris Wilson
  2018-08-15 14:34 ` [PATCH 2/4] drm/i915/execlists: Reset queue_priority on cancellation Chris Wilson
  2018-08-15 14:34 ` [PATCH 3/4] drm/i915/execlists: Include reset depth in traces Chris Wilson
@ 2018-08-15 14:34 ` Chris Wilson
  2018-08-15 15:19 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/4] drm/i915/execlists: Assert the queue is non-empty on unsubmitting Patchwork
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Chris Wilson @ 2018-08-15 14:34 UTC (permalink / raw)
  To: intel-gfx

We can bypass ksoftirqd for restarting our submission queue after a GPU
reset and so avoid any inscrutable delays.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_guc_submission.c | 24 +++++++++++++++++++++
 drivers/gpu/drm/i915/intel_lrc.c            |  2 +-
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
index 8f43c2fe90ca..d9f9b12a0213 100644
--- a/drivers/gpu/drm/i915/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/intel_guc_submission.c
@@ -842,6 +842,29 @@ guc_reset_prepare(struct intel_engine_cs *engine)
 	return i915_gem_find_active_request(engine);
 }
 
+static void guc_reset_finish(struct intel_engine_cs *engine)
+{
+	struct intel_engine_execlists * const execlists = &engine->execlists;
+
+	/* After a GPU reset, we may have requests to replay */
+	if (!RB_EMPTY_ROOT(&execlists->queue.rb_root))
+		tasklet_schedule(&execlists->tasklet);
+
+	/*
+	 * Flush the tasklet while we still have the forcewake to be sure
+	 * that it is not allowed to sleep before we restart and reload a
+	 * context.
+	 *
+	 * As before (with execlists_reset_prepare) we rely on the caller
+	 * serialising multiple attempts to reset so that we know that we
+	 * are the only one manipulating tasklet state.
+	 */
+	__tasklet_enable_sync_once(&execlists->tasklet);
+
+	GEM_TRACE("%s: depth->%d\n", engine->name,
+		  atomic_read(&execlists->tasklet.count));
+}
+
 /*
  * Everything below here is concerned with setup & teardown, and is
  * therefore not part of the somewhat time-critical batch-submission
@@ -1289,6 +1312,7 @@ static void guc_set_default_submission(struct intel_engine_cs *engine)
 	engine->unpark = guc_submission_unpark;
 
 	engine->reset.prepare = guc_reset_prepare;
+	engine->reset.finish = guc_reset_finish;
 
 	engine->flags &= ~I915_ENGINE_SUPPORTS_STATS;
 }
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 257d6f5f44d1..4631d38f0b6e 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1970,7 +1970,7 @@ static void execlists_reset_finish(struct intel_engine_cs *engine)
 
 	/* After a GPU reset, we may have requests to replay */
 	if (!RB_EMPTY_ROOT(&execlists->queue.rb_root))
-		tasklet_schedule(&execlists->tasklet);
+		execlists_submission_tasklet(execlists->tasklet.data);
 
 	/*
 	 * Flush the tasklet while we still have the forcewake to be sure
-- 
2.18.0

_______________________________________________
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 series starting with [1/4] drm/i915/execlists: Assert the queue is non-empty on unsubmitting
  2018-08-15 14:34 [PATCH 1/4] drm/i915/execlists: Assert the queue is non-empty on unsubmitting Chris Wilson
                   ` (2 preceding siblings ...)
  2018-08-15 14:34 ` [PATCH 4/4] drm/i915/execlists: Use direct submission to restart HW after reset Chris Wilson
@ 2018-08-15 15:19 ` Patchwork
  2018-08-15 15:36 ` ✓ Fi.CI.BAT: success " Patchwork
  2018-08-15 19:27 ` ✓ Fi.CI.IGT: " Patchwork
  5 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2018-08-15 15:19 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/4] drm/i915/execlists: Assert the queue is non-empty on unsubmitting
URL   : https://patchwork.freedesktop.org/series/48269/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
472fc90cb853 drm/i915/execlists: Assert the queue is non-empty on unsubmitting
-:9: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#9: 
<0>[  531.960431] drv_self-4806    7.... 527402570us : intel_gpu_reset: engine_mask=1, ret=0, retry=0

total: 0 errors, 1 warnings, 0 checks, 7 lines checked
1717e4384796 drm/i915/execlists: Reset queue_priority on cancellation
da4b9a58392d drm/i915/execlists: Include reset depth in traces
ed6c526c3084 drm/i915/execlists: Use direct submission to restart HW after reset

_______________________________________________
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: success for series starting with [1/4] drm/i915/execlists: Assert the queue is non-empty on unsubmitting
  2018-08-15 14:34 [PATCH 1/4] drm/i915/execlists: Assert the queue is non-empty on unsubmitting Chris Wilson
                   ` (3 preceding siblings ...)
  2018-08-15 15:19 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/4] drm/i915/execlists: Assert the queue is non-empty on unsubmitting Patchwork
@ 2018-08-15 15:36 ` Patchwork
  2018-08-15 19:27 ` ✓ Fi.CI.IGT: " Patchwork
  5 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2018-08-15 15:36 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/4] drm/i915/execlists: Assert the queue is non-empty on unsubmitting
URL   : https://patchwork.freedesktop.org/series/48269/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4672 -> Patchwork_9956 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/48269/revisions/1/mbox/

== Known issues ==

  Here are the changes found in Patchwork_9956 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    {igt@amdgpu/amd_prime@amd-to-i915}:
      {fi-kbl-8809g}:     NOTRUN -> FAIL (fdo#107341)

    igt@drv_selftest@live_hangcheck:
      fi-skl-guc:         PASS -> DMESG-FAIL (fdo#107174)
      fi-skl-6600u:       PASS -> DMESG-FAIL (fdo#107174, fdo#106560)
      fi-kbl-guc:         PASS -> DMESG-FAIL (fdo#106947)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
      {fi-byt-clapper}:   PASS -> FAIL (fdo#107362, fdo#103191) +1

    igt@prime_vgem@basic-fence-flip:
      fi-ilk-650:         PASS -> FAIL (fdo#104008)

    
    ==== Possible fixes ====

    {igt@amdgpu/amd_basic@userptr}:
      {fi-kbl-8809g}:     INCOMPLETE (fdo#107402) -> PASS

    igt@kms_frontbuffer_tracking@basic:
      fi-hsw-peppy:       DMESG-FAIL (fdo#102614) -> PASS
      {fi-byt-clapper}:   FAIL (fdo#103167) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      {fi-cfl-8109u}:     INCOMPLETE (fdo#106070) -> PASS

    {igt@kms_psr@primary_mmap_gtt}:
      fi-cnl-psr:         DMESG-WARN (fdo#107372) -> 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#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
  fdo#106070 https://bugs.freedesktop.org/show_bug.cgi?id=106070
  fdo#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560
  fdo#106947 https://bugs.freedesktop.org/show_bug.cgi?id=106947
  fdo#107174 https://bugs.freedesktop.org/show_bug.cgi?id=107174
  fdo#107341 https://bugs.freedesktop.org/show_bug.cgi?id=107341
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107372 https://bugs.freedesktop.org/show_bug.cgi?id=107372
  fdo#107402 https://bugs.freedesktop.org/show_bug.cgi?id=107402


== Participating hosts (53 -> 48) ==

  Missing    (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * Linux: CI_DRM_4672 -> Patchwork_9956

  CI_DRM_4672: 2a55ea192b780b798b8c5bf2e458694ba54a312d @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4598: 9c0f04355107a8693650b16756b6343a78501138 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9956: ed6c526c3084f8cfc3e2b0e21bacc513f49228d5 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

ed6c526c3084 drm/i915/execlists: Use direct submission to restart HW after reset
da4b9a58392d drm/i915/execlists: Include reset depth in traces
1717e4384796 drm/i915/execlists: Reset queue_priority on cancellation
472fc90cb853 drm/i915/execlists: Assert the queue is non-empty on unsubmitting

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9956/issues.html
_______________________________________________
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.IGT: success for series starting with [1/4] drm/i915/execlists: Assert the queue is non-empty on unsubmitting
  2018-08-15 14:34 [PATCH 1/4] drm/i915/execlists: Assert the queue is non-empty on unsubmitting Chris Wilson
                   ` (4 preceding siblings ...)
  2018-08-15 15:36 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-08-15 19:27 ` Patchwork
  5 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2018-08-15 19:27 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/4] drm/i915/execlists: Assert the queue is non-empty on unsubmitting
URL   : https://patchwork.freedesktop.org/series/48269/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4672_full -> Patchwork_9956_full =

== Summary - SUCCESS ==

  No regressions found.

  

== Known issues ==

  Here are the changes found in Patchwork_9956_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-shrfb-plflip-blt:
      shard-glk:          PASS -> FAIL (fdo#103167)

    igt@kms_setmode@basic:
      shard-glk:          PASS -> FAIL (fdo#99912)

    
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4672 -> Patchwork_9956

  CI_DRM_4672: 2a55ea192b780b798b8c5bf2e458694ba54a312d @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4598: 9c0f04355107a8693650b16756b6343a78501138 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9956: ed6c526c3084f8cfc3e2b0e21bacc513f49228d5 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9956/shards.html
_______________________________________________
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:[~2018-08-15 19:27 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-08-15 14:34 [PATCH 1/4] drm/i915/execlists: Assert the queue is non-empty on unsubmitting Chris Wilson
2018-08-15 14:34 ` [PATCH 2/4] drm/i915/execlists: Reset queue_priority on cancellation Chris Wilson
2018-08-15 14:34 ` [PATCH 3/4] drm/i915/execlists: Include reset depth in traces Chris Wilson
2018-08-15 14:34 ` [PATCH 4/4] drm/i915/execlists: Use direct submission to restart HW after reset Chris Wilson
2018-08-15 15:19 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/4] drm/i915/execlists: Assert the queue is non-empty on unsubmitting Patchwork
2018-08-15 15:36 ` ✓ Fi.CI.BAT: success " Patchwork
2018-08-15 19:27 ` ✓ Fi.CI.IGT: " Patchwork

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).