Re: [PATCH v2 3/7] drm/i915/execlists: Make submission tasklet hardirq safe

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 07/05/2018 14:57, Chris Wilson wrote:
Prepare to allow the execlists submission to be run from underneath a
hardirq timer context (and not just the current softirq context) as is
required for fast preemption resets and context switches.

Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
---
  drivers/gpu/drm/i915/intel_lrc.c | 42 ++++++++++++++++++++++----------
  1 file changed, 29 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index f9f4064dec0e..15c373ea5b7e 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -357,10 +357,13 @@ 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);
- spin_lock_irq(&engine->timeline.lock);
  	__unwind_incomplete_requests(engine);
-	spin_unlock_irq(&engine->timeline.lock);
+
+	spin_unlock_irqrestore(&engine->timeline.lock, flags);
  }
static inline void
@@ -554,7 +557,7 @@ static void inject_preempt_context(struct intel_engine_cs *engine)
  	execlists_set_active(&engine->execlists, EXECLISTS_ACTIVE_PREEMPT);
  }
-static void execlists_dequeue(struct intel_engine_cs *engine)
+static bool __execlists_dequeue(struct intel_engine_cs *engine)
  {
  	struct intel_engine_execlists * const execlists = &engine->execlists;
  	struct execlist_port *port = execlists->port;
@@ -564,6 +567,8 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
  	struct rb_node *rb;
  	bool submit = false;
+ lockdep_assert_held(&engine->timeline.lock);
+
  	/* Hardware submission is through 2 ports. Conceptually each port
  	 * has a (RING_START, RING_HEAD, RING_TAIL) tuple. RING_START is
  	 * static for a context, and unique to each, so we only execute
@@ -585,7 +590,6 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
  	 * and context switches) submission.
  	 */
- spin_lock_irq(&engine->timeline.lock);
  	rb = execlists->first;
  	GEM_BUG_ON(rb_first(&execlists->queue) != rb);
@@ -600,7 +604,7 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
  						EXECLISTS_ACTIVE_USER));
  		GEM_BUG_ON(!port_count(&port[0]));
  		if (port_count(&port[0]) > 1)
-			goto unlock;
+			return false;
/*
  		 * If we write to ELSP a second time before the HW has had
@@ -610,11 +614,11 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
  		 * the HW to indicate that it has had a chance to respond.
  		 */
  		if (!execlists_is_active(execlists, EXECLISTS_ACTIVE_HWACK))
-			goto unlock;
+			return false;
if (need_preempt(engine, last, execlists->queue_priority)) {
  			inject_preempt_context(engine);
-			goto unlock;
+			return false;
  		}
/*
@@ -639,7 +643,7 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
  		 * priorities of the ports haven't been switch.
  		 */
  		if (port_count(&port[1]))
-			goto unlock;
+			return false;
/*
  		 * WaIdleLiteRestore:bdw,skl
@@ -744,13 +748,25 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
  	/* We must always keep the beast fed if we have work piled up */
  	GEM_BUG_ON(execlists->first && !port_isset(execlists->port));
-unlock:
-	spin_unlock_irq(&engine->timeline.lock);
-
-	if (submit) {
+	/* Re-evaluate the executing context setup after each preemptive kick */
+	if (last)
  		execlists_user_begin(execlists, execlists->port);

Last can be non-null and submit false, so this is not equivalent.

By the looks of it makes no difference since it is OK to set the execlists user active bit multiple times. Even though the helper is called execlists_set_active_once. But the return value is not looked at.

Still, why not keep doing this when submit is true?

Regards,

Tvrtko

+
+	return submit;
+}
+
+static void execlists_dequeue(struct intel_engine_cs *engine)
+{
+	struct intel_engine_execlists * const execlists = &engine->execlists;
+	unsigned long flags;
+	bool submit;
+
+	spin_lock_irqsave(&engine->timeline.lock, flags);
+	submit = __execlists_dequeue(engine);
+	spin_unlock_irqrestore(&engine->timeline.lock, flags);
+
+	if (submit)
  		execlists_submit_ports(engine);
-	}
GEM_BUG_ON(port_isset(execlists->port) &&
  		   !execlists_is_active(execlists, EXECLISTS_ACTIVE_USER));

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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux