public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH 1/9] drm/i915/execlists: Avoid kicking priority on the current context
@ 2018-08-30 10:07 Chris Wilson
  2018-08-30 10:07 ` [PATCH 2/9] drm/i915: Missed interrupt simulation is no more, tell the world Chris Wilson
                   ` (12 more replies)
  0 siblings, 13 replies; 16+ messages in thread
From: Chris Wilson @ 2018-08-30 10:07 UTC (permalink / raw)
  To: intel-gfx

If the request is currently on the HW (in port 0), then we do not need
to kick the submission tasklet to evaluate whether we should be
preempting itself in order to execute it again.

In the case that was annoying me:

   execlists_schedule: rq(18:211173).prio=0 -> 2
   need_preempt: last(18:211174).prio=0, queue.prio=2

We are bumping the priority of the first of a pair of requests running
in the current context. Then when evaluating preempt, we would see that
that our priority request is higher than the last executing request in
ELSP0 and so trigger preemption, not realising that our intended request
was already executing.

v2: As we assume state of the execlists->port[] that is only valid while
we hold the timeline lock we have to repeat some earlier tests that on
the validity of the node.
v3: Wrap guc submission under the timeline.lock as is now the way of all
things.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_submission.c | 18 +++------
 drivers/gpu/drm/i915/intel_lrc.c            | 41 +++++++++++++++------
 2 files changed, 36 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
index 07b9d313b019..7b878790228a 100644
--- a/drivers/gpu/drm/i915/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/intel_guc_submission.c
@@ -771,19 +771,8 @@ static bool __guc_dequeue(struct intel_engine_cs *engine)
 
 static void guc_dequeue(struct intel_engine_cs *engine)
 {
-	unsigned long flags;
-	bool submit;
-
-	local_irq_save(flags);
-
-	spin_lock(&engine->timeline.lock);
-	submit = __guc_dequeue(engine);
-	spin_unlock(&engine->timeline.lock);
-
-	if (submit)
+	if ( __guc_dequeue(engine))
 		guc_submit(engine);
-
-	local_irq_restore(flags);
 }
 
 static void guc_submission_tasklet(unsigned long data)
@@ -792,6 +781,9 @@ static void guc_submission_tasklet(unsigned long data)
 	struct intel_engine_execlists * const execlists = &engine->execlists;
 	struct execlist_port *port = execlists->port;
 	struct i915_request *rq;
+	unsigned long flags;
+
+	spin_lock_irqsave(&engine->timeline.lock, flags);
 
 	rq = port_request(port);
 	while (rq && i915_request_completed(rq)) {
@@ -815,6 +807,8 @@ static void guc_submission_tasklet(unsigned long data)
 
 	if (!execlists_is_active(execlists, EXECLISTS_ACTIVE_PREEMPT))
 		guc_dequeue(engine);
+
+	spin_unlock_irqrestore(&engine->timeline.lock, flags);
 }
 
 static struct i915_request *
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index f8ceb9c99dd6..4e0df633fd35 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -355,13 +355,8 @@ execlists_unwind_incomplete_requests(struct intel_engine_execlists *execlists)
 {
 	struct intel_engine_cs *engine =
 		container_of(execlists, typeof(*engine), execlists);
-	unsigned long flags;
-
-	spin_lock_irqsave(&engine->timeline.lock, flags);
 
 	__unwind_incomplete_requests(engine);
-
-	spin_unlock_irqrestore(&engine->timeline.lock, flags);
 }
 
 static inline void
@@ -1233,9 +1228,13 @@ static void execlists_schedule(struct i915_request *request,
 
 		engine = sched_lock_engine(node, engine);
 
+		/* Recheck after acquiring the engine->timeline.lock */
 		if (prio <= node->attr.priority)
 			continue;
 
+		if (i915_sched_node_signaled(node))
+			continue;
+
 		node->attr.priority = prio;
 		if (!list_empty(&node->link)) {
 			if (last != engine) {
@@ -1244,14 +1243,34 @@ static void execlists_schedule(struct i915_request *request,
 			}
 			GEM_BUG_ON(pl->priority != prio);
 			list_move_tail(&node->link, &pl->requests);
+		} else {
+			/*
+			 * If the request is not in the priolist queue because
+			 * it is not yet runnable, then it doesn't contribute
+			 * to our preemption decisions. On the other hand,
+			 * if the request is on the HW, it too is not in the
+			 * queue; but in that case we may still need to reorder
+			 * the inflight requests.
+			 */
+			if (!i915_sw_fence_done(&sched_to_request(node)->submit))
+				continue;
 		}
 
-		if (prio > engine->execlists.queue_priority &&
-		    i915_sw_fence_done(&sched_to_request(node)->submit)) {
-			/* defer submission until after all of our updates */
-			__update_queue(engine, prio);
-			tasklet_hi_schedule(&engine->execlists.tasklet);
-		}
+		if (prio <= engine->execlists.queue_priority)
+			continue;
+
+		/*
+		 * If we are already the currently executing context, don't
+		 * bother evaluating if we should preempt ourselves.
+		 */
+		if (sched_to_request(node)->global_seqno &&
+		    i915_seqno_passed(port_request(engine->execlists.port)->global_seqno,
+				      sched_to_request(node)->global_seqno))
+			continue;
+
+		/* Defer (tasklet) submission until after all of our updates. */
+		__update_queue(engine, prio);
+		tasklet_hi_schedule(&engine->execlists.tasklet);
 	}
 
 	spin_unlock_irq(&engine->timeline.lock);
-- 
2.19.0.rc1

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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2018-09-05  9:01 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-08-30 10:07 [PATCH 1/9] drm/i915/execlists: Avoid kicking priority on the current context Chris Wilson
2018-08-30 10:07 ` [PATCH 2/9] drm/i915: Missed interrupt simulation is no more, tell the world Chris Wilson
2018-08-30 10:07 ` [PATCH 3/9] drm/i915/selftests: Basic stress test for rapid context switching Chris Wilson
2018-09-05  7:24   ` [Intel-gfx] " Dan Carpenter
2018-09-05  8:50   ` [PATCH] " Chris Wilson
2018-08-30 10:07 ` [PATCH 4/9] drm/i915/execlists: Delay updating ring register state after resume Chris Wilson
2018-08-30 10:07 ` [PATCH 5/9] drm/i915/execlists: Use coherent writes into the context image Chris Wilson
2018-08-30 10:07 ` [PATCH 6/9] drm/i915/execlists: Onion unwind for logical_ring_init() failure Chris Wilson
2018-08-30 10:07 ` [PATCH 7/9] drm/i915: Report the number of closed vma held by each context in debugfs Chris Wilson
2018-08-30 10:07 ` [PATCH 8/9] drm/i915: Remove debugfs/i915_ppgtt_info Chris Wilson
2018-08-30 10:07 ` [PATCH 9/9] drm/i915/execlists: Assert the queue is non-empty on unsubmitting Chris Wilson
2018-08-30 12:15 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/9] drm/i915/execlists: Avoid kicking priority on the current context Patchwork
2018-08-30 12:17 ` ✗ Fi.CI.SPARSE: " Patchwork
2018-08-30 12:34 ` ✓ Fi.CI.BAT: success " Patchwork
2018-08-30 16:27 ` ✓ Fi.CI.IGT: " Patchwork
2018-09-05  9:01 ` ✗ Fi.CI.BAT: failure for series starting with [1/9] drm/i915/execlists: Avoid kicking priority on the current context (rev2) Patchwork

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox