[Intel-gfx] [PATCH] drm/i915/gem: Reduce ggtt_flush() from a wait-for-idle to a mere barrier flush

Chris Wilson chris at chris-wilson.co.uk
Wed Nov 20 16:42:46 UTC 2019


Since we use barriers, we need only explicitly flush those barriers to
ensure tha we can reclaim the available ggtt for ourselves. The barrier
flush was implicit inside the intel_gt_wait_for_idle() -- except because
we use i915_gem_evict from inside an active timeline during execbuf, we
could easily end up waiting upon ourselves.

v2: Wait on the barriers to ensure that any context unpinning that can
be done, will be done.

Fixes: 7936a22dd466 ("drm/i915/gt: Wait for new requests in intel_gt_retire_requests()")
Fixes: a46bfdc83fee ("drm/i915/gt: Wait for new requests in intel_gt_retire_requests()")
Testcase: igt/gem_exec_reloc/basic-range
Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
---
 .../gpu/drm/i915/gt/intel_engine_heartbeat.c  |  8 +++-
 .../gpu/drm/i915/gt/intel_engine_heartbeat.h  |  4 +-
 drivers/gpu/drm/i915/gt/selftest_context.c    | 38 ++++---------------
 .../drm/i915/gt/selftest_engine_heartbeat.c   |  7 +++-
 drivers/gpu/drm/i915/i915_gem_evict.c         | 26 ++++++++++++-
 5 files changed, 48 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
index c91fd4e4af29..0173720af05a 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
@@ -212,10 +212,14 @@ int intel_engine_pulse(struct intel_engine_cs *engine)
 	return err;
 }
 
-int intel_engine_flush_barriers(struct intel_engine_cs *engine)
+int intel_engine_flush_barriers(struct intel_engine_cs *engine,
+				struct i915_request **out)
 {
 	struct i915_request *rq;
 
+	if (out)
+		*out = NULL;
+
 	if (llist_empty(&engine->barrier_tasks))
 		return 0;
 
@@ -224,6 +228,8 @@ int intel_engine_flush_barriers(struct intel_engine_cs *engine)
 		return PTR_ERR(rq);
 
 	idle_pulse(engine, rq);
+	if (out)
+		*out = i915_request_get(rq);
 	i915_request_add(rq);
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.h b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.h
index a7b8c0f9e005..17e973d86f5c 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.h
@@ -7,6 +7,7 @@
 #ifndef INTEL_ENGINE_HEARTBEAT_H
 #define INTEL_ENGINE_HEARTBEAT_H
 
+struct i915_request;
 struct intel_engine_cs;
 
 void intel_engine_init_heartbeat(struct intel_engine_cs *engine);
@@ -18,6 +19,7 @@ void intel_engine_park_heartbeat(struct intel_engine_cs *engine);
 void intel_engine_unpark_heartbeat(struct intel_engine_cs *engine);
 
 int intel_engine_pulse(struct intel_engine_cs *engine);
-int intel_engine_flush_barriers(struct intel_engine_cs *engine);
+int intel_engine_flush_barriers(struct intel_engine_cs *engine,
+				struct i915_request **barrier);
 
 #endif /* INTEL_ENGINE_HEARTBEAT_H */
diff --git a/drivers/gpu/drm/i915/gt/selftest_context.c b/drivers/gpu/drm/i915/gt/selftest_context.c
index 3586af636304..0c0f130802fb 100644
--- a/drivers/gpu/drm/i915/gt/selftest_context.c
+++ b/drivers/gpu/drm/i915/gt/selftest_context.c
@@ -41,33 +41,6 @@ static int request_sync(struct i915_request *rq)
 	return err;
 }
 
-static int context_sync(struct intel_context *ce)
-{
-	struct intel_timeline *tl = ce->timeline;
-	int err = 0;
-
-	mutex_lock(&tl->mutex);
-	do {
-		struct dma_fence *fence;
-		long timeout;
-
-		fence = i915_active_fence_get(&tl->last_request);
-		if (!fence)
-			break;
-
-		timeout = dma_fence_wait_timeout(fence, false, HZ / 10);
-		if (timeout < 0)
-			err = timeout;
-		else
-			i915_request_retire_upto(to_request(fence));
-
-		dma_fence_put(fence);
-	} while (!err);
-	mutex_unlock(&tl->mutex);
-
-	return err;
-}
-
 static int __live_context_size(struct intel_engine_cs *engine,
 			       struct i915_gem_context *fixme)
 {
@@ -202,6 +175,7 @@ static int __live_active_context(struct intel_engine_cs *engine,
 				 struct i915_gem_context *fixme)
 {
 	unsigned long saved_heartbeat;
+	struct i915_request *barrier;
 	struct intel_context *ce;
 	int pass;
 	int err;
@@ -269,17 +243,21 @@ static int __live_active_context(struct intel_engine_cs *engine,
 	}
 
 	/* Now make sure our idle-barriers are flushed */
-	err = intel_engine_flush_barriers(engine);
+	err = intel_engine_flush_barriers(engine, &barrier);
 	if (err)
 		goto err;
 
-	err = context_sync(engine->kernel_context);
-	if (err)
+	if (i915_request_wait(barrier, 0, HZ / 5) < 0) {
+		i915_request_put(barrier);
+		err = -ETIME;
 		goto err;
+	}
+	i915_request_put(barrier);
 
 	if (!i915_active_is_idle(&ce->active)) {
 		pr_err("context is still active!");
 		err = -EINVAL;
+		goto err;
 	}
 
 	if (intel_engine_pm_is_awake(engine)) {
diff --git a/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c b/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c
index f665a0e23c61..0bd9afc20ef3 100644
--- a/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c
+++ b/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c
@@ -115,6 +115,11 @@ static int __live_idle_pulse(struct intel_engine_cs *engine,
 	return err;
 }
 
+static int wrap_engine_flush_barriers(struct intel_engine_cs *engine)
+{
+	return intel_engine_flush_barriers(engine, NULL);
+}
+
 static int live_idle_flush(void *arg)
 {
 	struct intel_gt *gt = arg;
@@ -126,7 +131,7 @@ static int live_idle_flush(void *arg)
 
 	for_each_engine(engine, gt, id) {
 		intel_engine_pm_get(engine);
-		err = __live_idle_pulse(engine, intel_engine_flush_barriers);
+		err = __live_idle_pulse(engine, wrap_engine_flush_barriers);
 		intel_engine_pm_put(engine);
 		if (err)
 			break;
diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c
index 7e62c310290f..91daf87f491e 100644
--- a/drivers/gpu/drm/i915/i915_gem_evict.c
+++ b/drivers/gpu/drm/i915/i915_gem_evict.c
@@ -28,7 +28,7 @@
 
 #include <drm/i915_drm.h>
 
-#include "gem/i915_gem_context.h"
+#include "gt/intel_engine_heartbeat.h"
 #include "gt/intel_gt_requests.h"
 
 #include "i915_drv.h"
@@ -40,6 +40,9 @@ I915_SELFTEST_DECLARE(static struct igt_evict_ctl {
 
 static int ggtt_flush(struct intel_gt *gt)
 {
+	struct intel_engine_cs *engine;
+	enum intel_engine_id id;
+
 	/*
 	 * Not everything in the GGTT is tracked via vma (otherwise we
 	 * could evict as required with minimal stalling) so we are forced
@@ -47,7 +50,26 @@ static int ggtt_flush(struct intel_gt *gt)
 	 * the hopes that we can then remove contexts and the like only
 	 * bound by their active reference.
 	 */
-	return intel_gt_wait_for_idle(gt, MAX_SCHEDULE_TIMEOUT);
+	intel_gt_retire_requests(gt);
+	for_each_engine(engine, gt, id) {
+		struct i915_request *barrier;
+		long err;
+
+		/* A barrier will unpin anything that is ready to be unpinned */
+		err = intel_engine_flush_barriers(engine, &barrier);
+		if (err)
+			return err;
+
+		err = i915_request_wait(barrier,
+					I915_WAIT_INTERRUPTIBLE,
+					MAX_SCHEDULE_TIMEOUT);
+		i915_request_put(barrier);
+		if (err)
+			return err;
+	}
+	intel_gt_retire_requests(gt);
+
+	return 0;
 }
 
 static bool
-- 
2.24.0



More information about the Intel-gfx mailing list