public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
From: John.C.Harrison@Intel.com
To: Intel-GFX@Lists.FreeDesktop.Org
Subject: [PATCH v6 21/34] drm/i915: Support for 'unflushed' ring idle
Date: Wed, 20 Apr 2016 18:13:39 +0100	[thread overview]
Message-ID: <1461172435-4256-22-git-send-email-John.C.Harrison@Intel.com> (raw)
In-Reply-To: <1461172435-4256-1-git-send-email-John.C.Harrison@Intel.com>

From: John Harrison <John.C.Harrison@Intel.com>

When the seqno wraps around zero, the entire GPU is forced to be idle
for some reason (possibly only to work around issues with hardware
semaphores but no-one seems too sure!). This causes a problem if the
force idle occurs at an inopportune moment such as in the middle of
submitting a batch buffer. Specifically, it would lead to recursive
submits - submitting work requires a new seqno, the new seqno requires
idling the ring, idling the ring requires submitting work, submitting
work requires a new seqno...

This change adds a 'flush' parameter to the idle function call which
specifies whether the scheduler queues should be flushed out. I.e. is
the call intended to just idle the ring as it is right now (no flush)
or is it intended to force all outstanding work out of the system
(with flush).

In the seqno wrap case, pending work is not an issue because the next
operation will be to submit it. However, in other cases, the intention
is to make sure everything that could be done has been done.

v6: Updated to newer nightly (lots of ring -> engine renaming).

Added kerneldoc for intel_engine_idle().

Wrapped boolean 'flush' parameter with an _flush() macro.
[review feedback from Joonas Lahtinen]

For: VIZ-1587
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_gem.c         |  2 +-
 drivers/gpu/drm/i915/intel_lrc.c        |  2 +-
 drivers/gpu/drm/i915/intel_ringbuffer.c | 31 +++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_ringbuffer.h |  4 +++-
 4 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index ebc7f0a..1737f13 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3896,7 +3896,7 @@ int i915_gpu_idle(struct drm_device *dev)
 			i915_add_request_no_flush(req);
 		}
 
-		ret = intel_engine_idle(engine);
+		ret = intel_engine_idle_flush(engine);
 		if (ret)
 			return ret;
 	}
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 0a4ef61..d67b08b 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1091,7 +1091,7 @@ void intel_logical_ring_stop(struct intel_engine_cs *engine)
 	if (!intel_engine_initialized(engine))
 		return;
 
-	ret = intel_engine_idle(engine);
+	ret = intel_engine_idle_flush(engine);
 	if (ret && !i915_reset_in_progress(&to_i915(engine->dev)->gpu_error))
 		DRM_ERROR("failed to quiesce %s whilst cleaning up: %d\n",
 			  engine->name, ret);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 6021655..f5bcd24 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2359,10 +2359,37 @@ static void __wrap_ring_buffer(struct intel_ringbuffer *ringbuf)
 	intel_ring_update_space(ringbuf);
 }
 
-int intel_engine_idle(struct intel_engine_cs *engine)
+/**
+ * __intel_engine_idle - Force the engine to be idle.
+ * @engine: Engine to be idled
+ * @flush: Should queued scheduler work also be flushed
+ * Waits for all outstanding requests that have been sent to the given engine
+ * to complete. Can optionally also force all unsent requests that are queued
+ * in the scheduler to be sent first.
+ * Returns zero on success otherwise a negative error code.
+ *
+ * NB: Flushing can lead to recursion if called at the wrong time. E.g. flush
+ * causes the scheduler to submit requests to the hardware, submitting
+ * requests requires allocating a new seqno, when the seqno wraps around it
+ * idles the engine, idling with flush causes the scheduler to submit requests...
+ */
+int __intel_engine_idle(struct intel_engine_cs *engine, bool flush)
 {
 	struct drm_i915_gem_request *req;
 	uint32_t flags;
+	int ret;
+
+	/*
+	 * NB: Must not flush the scheduler if this idle request is from
+	 * within an execbuff submission (i.e. due to 'get_seqno' calling
+	 * 'wrap_seqno' calling 'idle'). As that would lead to recursive
+	 * flushes!
+	 */
+	if (flush) {
+		ret = i915_scheduler_flush(engine, true);
+		if (ret)
+			return ret;
+	}
 
 	/* Wait upon the last request to be completed */
 	if (list_empty(&engine->request_list))
@@ -3202,7 +3229,7 @@ intel_stop_engine(struct intel_engine_cs *engine)
 	if (!intel_engine_initialized(engine))
 		return;
 
-	ret = intel_engine_idle(engine);
+	ret = intel_engine_idle_flush(engine);
 	if (ret && !i915_reset_in_progress(&to_i915(engine->dev)->gpu_error))
 		DRM_ERROR("failed to quiesce %s whilst cleaning up: %d\n",
 			  engine->name, ret);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index c75c5e1..2e7daef 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -474,7 +474,9 @@ void intel_ring_update_space(struct intel_ringbuffer *ringbuf);
 int intel_ring_space(struct intel_ringbuffer *ringbuf);
 bool intel_engine_stopped(struct intel_engine_cs *engine);
 
-int __must_check intel_engine_idle(struct intel_engine_cs *engine);
+#define intel_engine_idle(engine)           __intel_engine_idle((engine), false)
+#define intel_engine_idle_flush(engine)     __intel_engine_idle((engine), true)
+int __must_check __intel_engine_idle(struct intel_engine_cs *engine, bool flush);
 void intel_ring_init_seqno(struct intel_engine_cs *engine, u32 seqno);
 int intel_ring_flush_all_caches(struct drm_i915_gem_request *req);
 int intel_ring_invalidate_all_caches(struct drm_i915_gem_request *req);
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  parent reply	other threads:[~2016-04-20 17:14 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-20 17:13 [PATCH v6 00/34] GPU scheduler for i915 driver John.C.Harrison
2016-04-20 17:13 ` [PATCH v6 01/34] drm/i915: Add total count to context status debugfs output John.C.Harrison
2016-04-20 17:13 ` [PATCH v6 02/34] drm/i915: Prelude to splitting i915_gem_do_execbuffer in two John.C.Harrison
2016-04-20 17:13 ` [PATCH v6 03/34] drm/i915: Split i915_dem_do_execbuffer() in half John.C.Harrison
2016-04-20 17:13 ` [PATCH v6 04/34] drm/i915: Cache request pointer in *_submission_final() John.C.Harrison
2016-04-20 17:13 ` [PATCH v6 05/34] drm/i915: Re-instate request->uniq because it is extremely useful John.C.Harrison
2016-04-20 17:13 ` [PATCH v6 06/34] drm/i915: Start of GPU scheduler John.C.Harrison
2016-06-10 16:24   ` Tvrtko Ursulin
2016-04-20 17:13 ` [PATCH v6 07/34] drm/i915: Disable hardware semaphores when GPU scheduler is enabled John.C.Harrison
2016-04-20 17:13 ` [PATCH v6 08/34] drm/i915: Force MMIO flips when scheduler enabled John.C.Harrison
2016-04-20 17:13 ` [PATCH v6 09/34] drm/i915: Added scheduler hook when closing DRM file handles John.C.Harrison
2016-04-20 17:13 ` [PATCH v6 10/34] drm/i915: Added scheduler hook into i915_gem_request_notify() John.C.Harrison
2016-04-20 17:13 ` [PATCH v6 11/34] drm/i915: Added deferred work handler for scheduler John.C.Harrison
2016-06-10 16:29   ` Tvrtko Ursulin
2016-04-20 17:13 ` [PATCH v6 12/34] drm/i915: Redirect execbuffer_final() via scheduler John.C.Harrison
2016-04-20 17:13 ` [PATCH v6 13/34] drm/i915: Keep the reserved space mechanism happy John.C.Harrison
2016-04-20 17:13 ` [PATCH v6 14/34] drm/i915: Added tracking/locking of batch buffer objects John.C.Harrison
2016-04-20 17:13 ` [PATCH v6 15/34] drm/i915: Hook scheduler node clean up into retire requests John.C.Harrison
2016-04-20 17:13 ` [PATCH v6 16/34] drm/i915: Added scheduler support to __wait_request() calls John.C.Harrison
2016-04-20 17:13 ` [PATCH v6 17/34] drm/i915: Added scheduler support to page fault handler John.C.Harrison
2016-04-20 17:13 ` [PATCH v6 18/34] drm/i915: Added scheduler flush calls to ring throttle and idle functions John.C.Harrison
2016-04-20 17:13 ` [PATCH v6 19/34] drm/i915: Add scheduler hook to GPU reset John.C.Harrison
2016-04-20 17:13 ` [PATCH v6 20/34] drm/i915: Added a module parameter to allow the scheduler to be disabled John.C.Harrison
2016-04-20 17:13 ` John.C.Harrison [this message]
2016-04-20 17:13 ` [PATCH v6 22/34] drm/i915: Defer seqno allocation until actual hardware submission time John.C.Harrison
2016-04-20 17:13 ` [PATCH v6 23/34] drm/i915: Added trace points to scheduler John.C.Harrison
2016-04-20 17:13 ` [PATCH v6 24/34] drm/i915: Added scheduler queue throttling by DRM file handle John.C.Harrison
2016-05-06 13:19   ` John.C.Harrison
2016-04-20 17:13 ` [PATCH v6 25/34] drm/i915: Added debugfs interface to scheduler tuning parameters John.C.Harrison
2016-04-20 17:13 ` [PATCH v6 26/34] drm/i915: Add early exit to execbuff_final() if insufficient ring space John.C.Harrison
2016-04-20 17:13 ` [PATCH v6 27/34] drm/i915: Added scheduler statistic reporting to debugfs John.C.Harrison
2016-05-06 13:21   ` John.C.Harrison
2016-04-20 17:13 ` [PATCH v6 28/34] drm/i915: Add scheduler support functions for TDR John.C.Harrison
2016-04-20 17:13 ` [PATCH v6 29/34] drm/i915: Enable GPU scheduler by default John.C.Harrison
2016-04-20 17:13 ` [PATCH v6 30/34] drm/i915: Add scheduling priority to per-context parameters John.C.Harrison
2016-04-20 17:13 ` [PATCH v6 31/34] drm/i915: Add support for retro-actively banning batch buffers John.C.Harrison
2016-04-20 17:13 ` [PATCH v6 32/34] drm/i915: Allow scheduler to manage inter-ring object synchronisation John.C.Harrison
2016-04-20 17:13 ` [PATCH v6 33/34] drm/i915: Added debug state dump facilities to scheduler John.C.Harrison
2016-04-20 17:13 ` [PATCH v6 34/34] drm/i915: Scheduler state dump via debugfs John.C.Harrison
2016-04-20 17:13 ` [PATCH 1/1] drm/i915: Add wrapper for context priority interface John.C.Harrison
2016-04-20 17:13 ` [PATCH 1/2] igt/gem_ctx_param_basic: Updated to support scheduler " John.C.Harrison
2016-04-20 17:13 ` [PATCH 2/2] igt/gem_scheduler: Add gem_scheduler test John.C.Harrison
2016-04-21  9:43 ` ✓ Fi.CI.BAT: success for GPU scheduler for i915 driver (rev2) Patchwork
2016-04-22 15:37 ` [PATCH v6 00/34] GPU scheduler for i915 driver John Harrison
2016-04-23  9:57 ` ✗ Fi.CI.BAT: failure for GPU scheduler for i915 driver (rev2) Patchwork
2016-04-25  9:54 ` [PATCH v6 00/34] GPU scheduler for i915 driver Chris Wilson
2016-04-25 11:55   ` John Harrison
2016-04-26 13:20   ` Daniel Vetter
2016-05-05 11:54     ` John Harrison
2016-05-09  9:49 ` ✗ Fi.CI.BAT: warning for GPU scheduler for i915 driver (rev4) Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1461172435-4256-22-git-send-email-John.C.Harrison@Intel.com \
    --to=john.c.harrison@intel.com \
    --cc=Intel-GFX@Lists.FreeDesktop.Org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox