All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chris Wilson <chris@chris-wilson.co.uk>
To: intel-gfx@lists.freedesktop.org
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Subject: [PATCH 2/2] drm/i915: Look for an active kernel context before switching
Date: Wed, 23 May 2018 10:51:26 +0100	[thread overview]
Message-ID: <20180523095126.8115-2-chris@chris-wilson.co.uk> (raw)
In-Reply-To: <20180523095126.8115-1-chris@chris-wilson.co.uk>

We were not very carefully checking to see if an older request on the
engine was an earlier switch-to-kernel-context before deciding to emit a
new switch. The end result would be that we could get into a permanent
loop of trying to emit a new request to perform the switch simply to
flush the existing switch.

What we need is a means of tracking the completion of each timeline
versus the kernel context, that is to detect if a more recent request
has been submitted that would result in a switch away from the kernel
context. To realise this, we need only to look in our syncmap on the
kernel context and check that we have synchronized against all active
rings.

Reported-by: Mika Kuoppala <mika.kuoppala@intel.com>
Fixes: a89d1f921c15 ("drm/i915: Split i915_gem_timeline into individual timelines")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_context.c | 44 +++++++++++++++++++++----
 1 file changed, 38 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index b69b18ef8120..6c9301e8ef6b 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -587,18 +587,45 @@ last_request_on_engine(struct i915_timeline *timeline,
 	return NULL;
 }
 
-static bool engine_has_idle_kernel_context(struct intel_engine_cs *engine)
+static bool engine_has_kernel_context_barrier(struct intel_engine_cs *engine)
 {
-	struct list_head * const active_rings = &engine->i915->gt.active_rings;
+	struct drm_i915_private *i915 = engine->i915;
+	struct i915_timeline *barrier =
+		to_intel_context(i915->kernel_context, engine)->ring->timeline;
 	struct intel_ring *ring;
+	bool any_active = false;
 
-	lockdep_assert_held(&engine->i915->drm.struct_mutex);
+	lockdep_assert_held(&i915->drm.struct_mutex);
+	list_for_each_entry(ring, &i915->gt.active_rings, active_link) {
+		struct i915_request *rq;
+
+		if (ring->timeline == barrier)
+			continue;
 
-	list_for_each_entry(ring, active_rings, active_link) {
-		if (last_request_on_engine(ring->timeline, engine))
+		rq = last_request_on_engine(ring->timeline, engine);
+		if (!rq)
+			continue;
+
+		/*
+		 * Was this request submitted after the previous
+		 * switch-to-kernel-context?
+		 */
+		if (!i915_timeline_sync_is_later(barrier, &rq->fence))
 			return false;
+
+		any_active = true;
 	}
 
+	/*
+	 * If any other timeline was still active and behind the last barrier,
+	 * then our last switch-to-kernel-context must still be queued and
+	 * will run last (leaving the engine in the kernel context when it
+	 * eventually idles).
+	 */
+	if (any_active)
+		return true;
+
+	/* The engine is idle; check that it is idling in the kernel context. */
 	return intel_engine_has_kernel_context(engine);
 }
 
@@ -608,6 +635,7 @@ int i915_gem_switch_to_kernel_context(struct drm_i915_private *i915)
 	enum intel_engine_id id;
 
 	lockdep_assert_held(&i915->drm.struct_mutex);
+	GEM_BUG_ON(!i915->kernel_context);
 
 	i915_retire_requests(i915);
 
@@ -615,7 +643,8 @@ int i915_gem_switch_to_kernel_context(struct drm_i915_private *i915)
 		struct intel_ring *ring;
 		struct i915_request *rq;
 
-		if (engine_has_idle_kernel_context(engine))
+		GEM_BUG_ON(!to_intel_context(i915->kernel_context, engine));
+		if (engine_has_kernel_context_barrier(engine))
 			continue;
 
 		rq = i915_request_alloc(engine, i915->kernel_context);
@@ -626,6 +655,9 @@ int i915_gem_switch_to_kernel_context(struct drm_i915_private *i915)
 		list_for_each_entry(ring, &i915->gt.active_rings, active_link) {
 			struct i915_request *prev;
 
+			if (ring == rq->ring)
+				continue;
+
 			prev = last_request_on_engine(ring->timeline, engine);
 			if (prev)
 				i915_sw_fence_await_sw_fence_gfp(&rq->submit,
-- 
2.17.0

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

  reply	other threads:[~2018-05-23  9:51 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-23  9:51 [PATCH 1/2] drm/i915/selftests: Pinned the mock kernel context Chris Wilson
2018-05-23  9:51 ` Chris Wilson [this message]
2018-05-23 10:31 ` ✓ Fi.CI.BAT: success for series starting with [1/2] " Patchwork
2018-05-23 12:41 ` [PATCH 1/2] " Mika Kuoppala
2018-05-23 12:45 ` ✓ Fi.CI.IGT: success for series starting with [1/2] " Patchwork
2018-05-23 14:53 [PATCH 1/2] drm/i915/selftests: Pin " Chris Wilson
2018-05-23 14:53 ` [PATCH 2/2] drm/i915: Look for an active kernel context before switching Chris Wilson

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=20180523095126.8115-2-chris@chris-wilson.co.uk \
    --to=chris@chris-wilson.co.uk \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=mika.kuoppala@intel.com \
    /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 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.