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: thomas.hellstrom@intel.com, Chris Wilson <chris@chris-wilson.co.uk>
Subject: [Intel-gfx] [PATCH 11/21] drm/i915/gt: Replace intel_engine_transfer_stale_breadcrumbs
Date: Thu, 30 Jul 2020 10:37:46 +0100	[thread overview]
Message-ID: <20200730093756.16737-12-chris@chris-wilson.co.uk> (raw)
In-Reply-To: <20200730093756.16737-1-chris@chris-wilson.co.uk>

After staring at the breadcrumb enabling/cancellation and coming to the
conclusion that the cause of the mysterious stale breadcrumbs must the
act of submitting a completed requests, we can then redirect those
completed requests onto a dedicated signaled_list at the time of
construction and so eliminate intel_engine_transfer_stale_breadcrumbs().

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_breadcrumbs.c | 50 ++++++++-------------
 drivers/gpu/drm/i915/gt/intel_engine.h      |  3 --
 drivers/gpu/drm/i915/gt/intel_lrc.c         | 15 -------
 drivers/gpu/drm/i915/i915_request.c         |  5 +--
 4 files changed, 21 insertions(+), 52 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
index 3d211a0c2b5a..fbdc465a5870 100644
--- a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
+++ b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
@@ -142,16 +142,16 @@ static void add_retire(struct intel_breadcrumbs *b, struct intel_timeline *tl)
 	intel_engine_add_retire(engine, tl);
 }
 
-static void __signal_request(struct i915_request *rq, struct list_head *signals)
+static bool __signal_request(struct i915_request *rq, struct list_head *signals)
 {
-	GEM_BUG_ON(!test_bit(I915_FENCE_FLAG_SIGNAL, &rq->fence.flags));
 	clear_bit(I915_FENCE_FLAG_SIGNAL, &rq->fence.flags);
 
 	if (!__dma_fence_signal(&rq->fence))
-		return;
+		return false;
 
 	i915_request_get(rq);
 	list_add_tail(&rq->signal_link, signals);
+	return true;
 }
 
 static void signal_irq_work(struct irq_work *work)
@@ -278,32 +278,6 @@ void intel_engine_reset_breadcrumbs(struct intel_engine_cs *engine)
 	spin_unlock_irqrestore(&b->irq_lock, flags);
 }
 
-void intel_engine_transfer_stale_breadcrumbs(struct intel_engine_cs *engine,
-					     struct intel_context *ce)
-{
-	struct intel_breadcrumbs *b = &engine->breadcrumbs;
-	unsigned long flags;
-
-	spin_lock_irqsave(&b->irq_lock, flags);
-	if (!list_empty(&ce->signals)) {
-		struct i915_request *rq, *next;
-
-		/* Queue for executing the signal callbacks in the irq_work */
-		list_for_each_entry_safe(rq, next, &ce->signals, signal_link) {
-			GEM_BUG_ON(rq->engine != engine);
-			GEM_BUG_ON(!__request_completed(rq));
-
-			__signal_request(rq, &b->signaled_requests);
-		}
-
-		INIT_LIST_HEAD(&ce->signals);
-		list_del_init(&ce->signal_link);
-
-		irq_work_queue(&b->irq_work);
-	}
-	spin_unlock_irqrestore(&b->irq_lock, flags);
-}
-
 void intel_engine_fini_breadcrumbs(struct intel_engine_cs *engine)
 {
 }
@@ -317,6 +291,17 @@ static void insert_breadcrumb(struct i915_request *rq,
 	if (test_bit(I915_FENCE_FLAG_SIGNAL, &rq->fence.flags))
 		return;
 
+	/*
+	 * If the request is already completed, we can transfer it
+	 * straight onto a signaled list, and queue the irq worker for
+	 * its signal completion.
+	 */
+	if (__request_completed(rq)) {
+		if (__signal_request(rq, &b->signaled_requests))
+			irq_work_queue(&b->irq_work);
+		return;
+	}
+
 	__intel_breadcrumbs_arm_irq(b);
 
 	/*
@@ -344,8 +329,11 @@ static void insert_breadcrumb(struct i915_request *rq,
 	if (pos == &ce->signals) /* catch transitions from empty list */
 		list_move_tail(&ce->signal_link, &b->signalers);
 	GEM_BUG_ON(!check_signal_order(ce, rq));
-
 	set_bit(I915_FENCE_FLAG_SIGNAL, &rq->fence.flags);
+
+	/* Check after attaching to irq, interrupt may have already fired. */
+	if (__request_completed(rq))
+		irq_work_queue(&b->irq_work);
 }
 
 bool i915_request_enable_breadcrumb(struct i915_request *rq)
@@ -401,7 +389,7 @@ bool i915_request_enable_breadcrumb(struct i915_request *rq)
 
 	spin_unlock(&b->irq_lock);
 
-	return !__request_completed(rq);
+	return true;
 }
 
 void i915_request_cancel_breadcrumb(struct i915_request *rq)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h
index a9249a23903a..faf00a353e25 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine.h
@@ -237,9 +237,6 @@ intel_engine_signal_breadcrumbs(struct intel_engine_cs *engine)
 void intel_engine_reset_breadcrumbs(struct intel_engine_cs *engine);
 void intel_engine_fini_breadcrumbs(struct intel_engine_cs *engine);
 
-void intel_engine_transfer_stale_breadcrumbs(struct intel_engine_cs *engine,
-					     struct intel_context *ce);
-
 void intel_engine_print_breadcrumbs(struct intel_engine_cs *engine,
 				    struct drm_printer *p);
 
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index ae886081a431..a4959e8229ac 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -1805,18 +1805,6 @@ static bool virtual_matches(const struct virtual_engine *ve,
 	return true;
 }
 
-static void virtual_xfer_breadcrumbs(struct virtual_engine *ve)
-{
-	/*
-	 * All the outstanding signals on ve->siblings[0] must have
-	 * been completed, just pending the interrupt handler. As those
-	 * signals still refer to the old sibling (via rq->engine), we must
-	 * transfer those to the old irq_worker to keep our locking
-	 * consistent.
-	 */
-	intel_engine_transfer_stale_breadcrumbs(ve->siblings[0], &ve->context);
-}
-
 #define for_each_waiter(p__, rq__) \
 	list_for_each_entry_lockless(p__, \
 				     &(rq__)->sched.waiters_list, \
@@ -2275,9 +2263,6 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
 					virtual_update_register_offsets(regs,
 									engine);
 
-				if (!list_empty(&ve->context.signals))
-					virtual_xfer_breadcrumbs(ve);
-
 				/*
 				 * Move the bound engine to the top of the list
 				 * for future execution. We then kick this
diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index 350bf6f158cb..0cf2a10a24f1 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -592,9 +592,8 @@ bool __i915_request_submit(struct i915_request *request)
 	 */
 	__notify_execute_cb_irq(request);
 
-	if (test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, &request->fence.flags) &&
-	    !i915_request_enable_breadcrumb(request))
-		intel_engine_signal_breadcrumbs(engine);
+	if (test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, &request->fence.flags))
+		i915_request_enable_breadcrumb(request);
 
 	return result;
 }
-- 
2.20.1

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

  parent reply	other threads:[~2020-07-30  9:38 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-30  9:37 [Intel-gfx] Breadcrumbs fixes and stall avoidance Chris Wilson
2020-07-30  9:37 ` [Intel-gfx] [PATCH 01/21] drm/i915: Add a couple of missing i915_active_fini() Chris Wilson
2020-07-30  9:37 ` [Intel-gfx] [PATCH 02/21] drm/i915: Skip taking acquire mutex for no ref->active callback Chris Wilson
2020-07-30  9:37 ` [Intel-gfx] [PATCH 03/21] drm/i915: Export a preallocate variant of i915_active_acquire() Chris Wilson
2020-07-31  7:33   ` Thomas Hellström (Intel)
2020-07-30  9:37 ` [Intel-gfx] [PATCH 04/21] drm/i915: Keep the most recently used active-fence upon discard Chris Wilson
2020-07-30  9:37 ` [Intel-gfx] [PATCH 05/21] drm/i915: Make the stale cached active node available for any timeline Chris Wilson
2020-07-30  9:37 ` [Intel-gfx] [PATCH 06/21] drm/i915: Reduce locking around i915_active_acquire_preallocate_barrier() Chris Wilson
2020-07-31  7:39   ` Thomas Hellström (Intel)
2020-07-30  9:37 ` [Intel-gfx] [PATCH 07/21] drm/i915: Provide a fastpath for waiting on vma bindings Chris Wilson
2020-07-30  9:37 ` [Intel-gfx] [PATCH 08/21] drm/i915/gem: Reduce ctx->engine_mutex for reading the clone source Chris Wilson
2020-07-30  9:37 ` [Intel-gfx] [PATCH 09/21] drm/i915/gem: Reduce ctx->engines_mutex for get_engines() Chris Wilson
2020-07-30  9:37 ` [Intel-gfx] [PATCH 10/21] drm/i915: Remove requirement for holding i915_request.lock for breadcrumbs Chris Wilson
2020-07-30  9:37 ` Chris Wilson [this message]
2020-07-30  9:37 ` [Intel-gfx] [PATCH 12/21] drm/i915/gt: Only transfer the virtual context to the new engine if active Chris Wilson
2020-07-30  9:37 ` [Intel-gfx] [PATCH 13/21] drm/i915/gt: Distinguish the virtual breadcrumbs from the irq breadcrumbs Chris Wilson
2020-07-31 14:53   ` Tvrtko Ursulin
2020-07-30  9:37 ` [Intel-gfx] [PATCH 14/21] drm/i915/gt: Move intel_breadcrumbs_arm_irq earlier Chris Wilson
2020-07-30  9:37 ` [Intel-gfx] [PATCH 15/21] drm/i915/gt: Hold context/request reference while breadcrumbs are active Chris Wilson
2020-07-30  9:37 ` [Intel-gfx] [PATCH 16/21] drm/i915/gt: Track signaled breadcrumbs outside of the breadcrumb spinlock Chris Wilson
2020-07-31 15:06   ` Tvrtko Ursulin
2020-07-31 15:12     ` Chris Wilson
2020-07-31 15:21       ` Chris Wilson
2020-07-31 16:06         ` Tvrtko Ursulin
2020-07-31 17:59           ` Chris Wilson
2020-07-31 15:32       ` Tvrtko Ursulin
2020-07-30  9:37 ` [Intel-gfx] [PATCH 17/21] drm/i915/gt: Protect context lifetime with RCU Chris Wilson
2020-07-31 15:15   ` Tvrtko Ursulin
2020-07-31 15:24     ` Chris Wilson
2020-07-31 15:45       ` Tvrtko Ursulin
2020-07-30  9:37 ` [Intel-gfx] [PATCH 18/21] drm/i915/gt: Split the breadcrumb spinlock between global and contexts Chris Wilson
2020-07-30  9:37 ` [Intel-gfx] [PATCH 19/21] drm/i915: Drop i915_request.lock serialisation around await_start Chris Wilson
2020-07-30  9:37 ` [Intel-gfx] [PATCH 20/21] drm/i915: Drop i915_request.lock requirement for intel_rps_boost() Chris Wilson
2020-07-30  9:37 ` [PATCH 21/21] drm/i915/gem: Delay tracking the GEM context until it is registered Chris Wilson
2020-07-30  9:37   ` [Intel-gfx] " Chris Wilson
2020-07-30 13:45 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [01/21] drm/i915: Add a couple of missing i915_active_fini() Patchwork
2020-07-30 13:46 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2020-07-30 14:04 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2020-07-30 19:20 ` [Intel-gfx] ✓ Fi.CI.IGT: " 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=20200730093756.16737-12-chris@chris-wilson.co.uk \
    --to=chris@chris-wilson.co.uk \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=thomas.hellstrom@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.