[PATCH 1/7] drm/i915: Flush submission tasklet after bumping priority

[PATCH 1/7] drm/i915: Flush submission tasklet after bumping priority

[Chris Wilson]

When called from process context tasklet_schedule() defers itself to
ksoftirqd. From experience this may cause unacceptable latencies of over
200ms in executing the submission tasklet, our goal is to reprioritise
the HW execution queue and trigger preemption immediately, so convert
the rcu protection over to include a kick of the tasklet.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 5ece6ae4bdff..03cd30001b5d 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -578,10 +578,10 @@ static void __fence_set_priority(struct dma_fence *fence,
 	rq = to_request(fence);
 	engine = rq->engine;
 
-	rcu_read_lock();
+	local_bh_disable(); /* RCU serialisation for set-wedged protection */
 	if (engine->schedule)
 		engine->schedule(rq, attr);
-	rcu_read_unlock();
+	local_bh_enable(); /* kick the tasklets if queues were reprioritised */
 }
 
 static void fence_set_priority(struct dma_fence *fence,
-- 
2.17.0

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

[PATCH 2/7] drm/i915: Combine set-wedged protection and tasklet kicking

[Chris Wilson]

Around request submission, we protect the call to schedule with a
premption disable loop to prevent set-wedge chaning function pointers
underneath us. This also prevents the tasklet running on the local CPU,
a trick we use immediately afterwards to forcibly execute the tasklet
after submission. We can combine the two wlog, and ensure that the
tasklet is only executed (on the local CPU) after we complete our
updates to the submission queue (and not after an intermediate,
incomplete update).

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_request.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index e4cf76ec14a6..88ee444679d4 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -1110,12 +1110,9 @@ void __i915_request_add(struct i915_request *request, bool flush_caches)
 	 * decide whether to preempt the entire chain so that it is ready to
 	 * run at the earliest possible convenience.
 	 */
-	rcu_read_lock();
+	local_bh_disable();
 	if (engine->schedule)
 		engine->schedule(request, &request->ctx->sched);
-	rcu_read_unlock();
-
-	local_bh_disable();
 	i915_sw_fence_commit(&request->submit);
 	local_bh_enable(); /* Kick the execlists tasklet if just scheduled */
 
-- 
2.17.0

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

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

[Chris Wilson]

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@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_lrc.c | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index f9f4064dec0e..56d60e830d11 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -554,7 +554,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 +564,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 +587,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);
 
@@ -745,12 +746,24 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
 	GEM_BUG_ON(execlists->first && !port_isset(execlists->port));
 
 unlock:
-	spin_unlock_irq(&engine->timeline.lock);
-
-	if (submit) {
+	if (last)
 		execlists_user_begin(execlists, execlists->port);
+
+	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));
-- 
2.17.0

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

[PATCH 4/7] drm/i915/guc: Make submission tasklet hardirq safe

[Chris Wilson]

Prepare to allow the GuC 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@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_guc_submission.c | 30 ++++++++++++++-------
 1 file changed, 21 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
index 62828e39ee26..97bd52fbd4e4 100644
--- a/drivers/gpu/drm/i915/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/intel_guc_submission.c
@@ -669,7 +669,7 @@ static inline int port_prio(const struct execlist_port *port)
 	return rq_prio(port_request(port));
 }
 
-static void guc_dequeue(struct intel_engine_cs *engine)
+static bool __guc_dequeue(struct intel_engine_cs *engine)
 {
 	struct intel_engine_execlists * const execlists = &engine->execlists;
 	struct execlist_port *port = execlists->port;
@@ -679,7 +679,8 @@ static void guc_dequeue(struct intel_engine_cs *engine)
 	bool submit = false;
 	struct rb_node *rb;
 
-	spin_lock_irq(&engine->timeline.lock);
+	lockdep_assert_held(&engine->timeline.lock);
+
 	rb = execlists->first;
 	GEM_BUG_ON(rb_first(&execlists->queue) != rb);
 
@@ -694,13 +695,13 @@ static void guc_dequeue(struct intel_engine_cs *engine)
 						     EXECLISTS_ACTIVE_PREEMPT);
 				queue_work(engine->i915->guc.preempt_wq,
 					   &preempt_work->work);
-				goto unlock;
+				return false;
 			}
 		}
 
 		port++;
 		if (port_isset(port))
-			goto unlock;
+			return false;
 	}
 	GEM_BUG_ON(port_isset(port));
 
@@ -738,19 +739,30 @@ static void guc_dequeue(struct intel_engine_cs *engine)
 done:
 	execlists->queue_priority = rb ? to_priolist(rb)->priority : INT_MIN;
 	execlists->first = rb;
-	if (submit) {
+	if (submit)
 		port_assign(port, last);
+	if (last)
 		execlists_user_begin(execlists, execlists->port);
-		guc_submit(engine);
-	}
 
 	/* We must always keep the beast fed if we have work piled up */
 	GEM_BUG_ON(port_isset(execlists->port) &&
 		   !execlists_is_active(execlists, EXECLISTS_ACTIVE_USER));
 	GEM_BUG_ON(execlists->first && !port_isset(execlists->port));
 
-unlock:
-	spin_unlock_irq(&engine->timeline.lock);
+	return submit;
+}
+
+static void guc_dequeue(struct intel_engine_cs *engine)
+{
+	unsigned long flags;
+	bool submit;
+
+	spin_lock_irqsave(&engine->timeline.lock, flags);
+	submit = __guc_dequeue(engine);
+	spin_unlock_irqrestore(&engine->timeline.lock, flags);
+
+	if (submit)
+		guc_submit(engine);
 }
 
 static void guc_submission_tasklet(unsigned long data)
-- 
2.17.0

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

[PATCH 5/7] drm/i915/execlists: Direct submit onto idle engines

[Chris Wilson]

Bypass using the tasklet to submit the first request to HW, as the
tasklet may be deferred unto ksoftirqd and at a minimum will add in
excess of 10us (and maybe tens of milliseconds) to our execution
latency. This latency reduction is most notable when execution flows
between engines.

Suggested-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
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 | 10 ++--
 drivers/gpu/drm/i915/intel_lrc.c            | 52 +++++++++++++++++----
 drivers/gpu/drm/i915/intel_ringbuffer.h     |  7 +++
 3 files changed, 56 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
index 97bd52fbd4e4..fc0799897162 100644
--- a/drivers/gpu/drm/i915/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/intel_guc_submission.c
@@ -754,12 +754,16 @@ static bool __guc_dequeue(struct intel_engine_cs *engine)
 
 static void guc_dequeue(struct intel_engine_cs *engine)
 {
-	unsigned long flags;
+	unsigned long uninitialized_var(flags);
 	bool submit;
 
-	spin_lock_irqsave(&engine->timeline.lock, flags);
+	if (!intel_engine_direct_submit(engine))
+		spin_lock_irqsave(&engine->timeline.lock, flags);
+
 	submit = __guc_dequeue(engine);
-	spin_unlock_irqrestore(&engine->timeline.lock, flags);
+
+	if (!intel_engine_direct_submit(engine))
+		spin_unlock_irqrestore(&engine->timeline.lock, flags);
 
 	if (submit)
 		guc_submit(engine);
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 56d60e830d11..fbe0a7ec8522 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -599,6 +599,8 @@ static bool __execlists_dequeue(struct intel_engine_cs *engine)
 		 */
 		GEM_BUG_ON(!execlists_is_active(execlists,
 						EXECLISTS_ACTIVE_USER));
+		GEM_BUG_ON(execlists_is_active(execlists,
+					       EXECLISTS_ACTIVE_PREEMPT));
 		GEM_BUG_ON(!port_count(&port[0]));
 		if (port_count(&port[0]) > 1)
 			goto unlock;
@@ -755,12 +757,16 @@ static bool __execlists_dequeue(struct intel_engine_cs *engine)
 static void execlists_dequeue(struct intel_engine_cs *engine)
 {
 	struct intel_engine_execlists * const execlists = &engine->execlists;
-	unsigned long flags;
+	unsigned long uninitialized_var(flags);
 	bool submit;
 
-	spin_lock_irqsave(&engine->timeline.lock, flags);
+	if (!intel_engine_direct_submit(engine))
+		spin_lock_irqsave(&engine->timeline.lock, flags);
+
 	submit = __execlists_dequeue(engine);
-	spin_unlock_irqrestore(&engine->timeline.lock, flags);
+
+	if (!intel_engine_direct_submit(engine))
+		spin_unlock_irqrestore(&engine->timeline.lock, flags);
 
 	if (submit)
 		execlists_submit_ports(engine);
@@ -1160,16 +1166,41 @@ static void queue_request(struct intel_engine_cs *engine,
 		      &lookup_priolist(engine, node, prio)->requests);
 }
 
-static void __submit_queue(struct intel_engine_cs *engine, int prio)
+static void __wakeup_queue(struct intel_engine_cs *engine, int prio)
 {
 	engine->execlists.queue_priority = prio;
+}
+
+static void __schedule_queue(struct intel_engine_cs *engine)
+{
 	tasklet_hi_schedule(&engine->execlists.tasklet);
 }
 
+static void __submit_queue(struct intel_engine_cs *engine)
+{
+	struct intel_engine_execlists * const execlists = &engine->execlists;
+
+	GEM_BUG_ON(!engine->i915->gt.awake);
+
+	/* Directly submit the first request to reduce the initial latency */
+	if (!port_isset(execlists->port) &&
+	    tasklet_trylock(&execlists->tasklet)) {
+		engine->flags |= I915_ENGINE_DIRECT_SUBMIT;
+		execlists->tasklet.func(execlists->tasklet.data);
+		engine->flags &= ~I915_ENGINE_DIRECT_SUBMIT;
+		tasklet_unlock(&execlists->tasklet);
+		return;
+	}
+
+	__schedule_queue(engine);
+}
+
 static void submit_queue(struct intel_engine_cs *engine, int prio)
 {
-	if (prio > engine->execlists.queue_priority)
-		__submit_queue(engine, prio);
+	if (prio > engine->execlists.queue_priority) {
+		__wakeup_queue(engine, prio);
+		__submit_queue(engine);
+	}
 }
 
 static void execlists_submit_request(struct i915_request *request)
@@ -1181,10 +1212,9 @@ static void execlists_submit_request(struct i915_request *request)
 	spin_lock_irqsave(&engine->timeline.lock, flags);
 
 	queue_request(engine, &request->sched, rq_prio(request));
-	submit_queue(engine, rq_prio(request));
-
 	GEM_BUG_ON(!engine->execlists.first);
 	GEM_BUG_ON(list_empty(&request->sched.link));
+	submit_queue(engine, rq_prio(request));
 
 	spin_unlock_irqrestore(&engine->timeline.lock, flags);
 }
@@ -1306,8 +1336,10 @@ static void execlists_schedule(struct i915_request *request,
 		}
 
 		if (prio > engine->execlists.queue_priority &&
-		    i915_sw_fence_done(&sched_to_request(node)->submit))
-			__submit_queue(engine, prio);
+		    i915_sw_fence_done(&sched_to_request(node)->submit)) {
+			__wakeup_queue(engine, prio);
+			__schedule_queue(engine);
+		}
 	}
 
 	spin_unlock_irq(&engine->timeline.lock);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 010750e8ee44..f5545391d76a 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -569,6 +569,7 @@ struct intel_engine_cs {
 #define I915_ENGINE_NEEDS_CMD_PARSER BIT(0)
 #define I915_ENGINE_SUPPORTS_STATS   BIT(1)
 #define I915_ENGINE_HAS_PREEMPTION   BIT(2)
+#define I915_ENGINE_DIRECT_SUBMIT    BIT(3)
 	unsigned int flags;
 
 	/*
@@ -646,6 +647,12 @@ intel_engine_has_preemption(const struct intel_engine_cs *engine)
 	return engine->flags & I915_ENGINE_HAS_PREEMPTION;
 }
 
+static inline bool
+intel_engine_direct_submit(const struct intel_engine_cs *engine)
+{
+	return engine->flags & I915_ENGINE_DIRECT_SUBMIT;
+}
+
 static inline bool __execlists_need_preempt(int prio, int last)
 {
 	return prio > max(0, last);
-- 
2.17.0

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

[PATCH 6/7] drm/i915/execlists: Direct submission from irq handler

[Chris Wilson]

Continuing the themem of bypassing ksoftirqd latency, also first try to
directly submit from the CS interrupt handler to clear the ELSP and
queue the next.

In the past, we have been hesitant to do this as the context switch
processing has been quite heavy, requiring forcewaked mmio. However, as
we now can read the GPU state from the cacheable HWSP, it is relatively
cheap!

Suggested-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c         | 10 ++++------
 drivers/gpu/drm/i915/intel_ringbuffer.h | 11 +++++++++++
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index f9bc3aaa90d0..ce5efb0cbb88 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1465,11 +1465,9 @@ gen8_cs_irq_handler(struct intel_engine_cs *engine, u32 iir)
 	struct intel_engine_execlists * const execlists = &engine->execlists;
 	bool tasklet = false;
 
-	if (iir & GT_CONTEXT_SWITCH_INTERRUPT) {
-		if (READ_ONCE(engine->execlists.active))
-			tasklet = !test_and_set_bit(ENGINE_IRQ_EXECLIST,
-						    &engine->irq_posted);
-	}
+	if (iir & GT_CONTEXT_SWITCH_INTERRUPT && READ_ONCE(execlists->active))
+		tasklet = !test_and_set_bit(ENGINE_IRQ_EXECLIST,
+					    &engine->irq_posted);
 
 	if (iir & GT_RENDER_USER_INTERRUPT) {
 		notify_ring(engine);
@@ -1477,7 +1475,7 @@ gen8_cs_irq_handler(struct intel_engine_cs *engine, u32 iir)
 	}
 
 	if (tasklet)
-		tasklet_hi_schedule(&execlists->tasklet);
+		execlists_tasklet(execlists);
 }
 
 static void gen8_gt_irq_ack(struct drm_i915_private *i915,
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index f5545391d76a..670b0837e2f6 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -717,6 +717,17 @@ execlists_port_complete(struct intel_engine_execlists * const execlists,
 	return port;
 }
 
+static inline void
+execlists_tasklet(struct intel_engine_execlists * const execlists)
+{
+	if (tasklet_trylock(&execlists->tasklet)) {
+		execlists->tasklet.func(execlists->tasklet.data);
+		tasklet_unlock(&execlists->tasklet);
+	} else {
+		tasklet_hi_schedule(&execlists->tasklet);
+	}
+}
+
 static inline unsigned int
 intel_engine_flag(const struct intel_engine_cs *engine)
 {
-- 
2.17.0

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

[PATCH 7/7] drm/i915: Speed up idle detection by kicking the tasklets

[Chris Wilson]

We rely on ksoftirqd to run in a timely fashion in order to drain the
execlists queue. Quite frequently, it does not. In some cases we may see
latencies of over 200ms triggering our idle timeouts and forcing us to
declare the driver wedged!

Thus we can speed up idle detection by bypassing ksoftirqd in these
cases and flush our tasklet to confirm if we are indeed still waiting
for the ELSP to drain.

v2: Put the execlists.first check back; it is required for handling
reset!

References: https://bugs.freedesktop.org/show_bug.cgi?id=106373
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_engine_cs.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index 70325e0824e3..0f45d6826c8e 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -945,10 +945,19 @@ bool intel_engine_is_idle(struct intel_engine_cs *engine)
 		return true;
 
 	/* Waiting to drain ELSP? */
-	if (READ_ONCE(engine->execlists.active))
-		return false;
+	if (READ_ONCE(engine->execlists.active)) {
+		/*
+		 * ksoftirqd has notorious latency that may cause us to
+		 * timeout while waiting for the engine to idle as we wait for
+		 * ksoftirqd to run the execlists tasklet to drain the ELSP.
+		 * If we are expecting a context switch from the GPU, check now.
+		 */
+		execlists_tasklet(&engine->execlists);
+		if (READ_ONCE(engine->execlists.active))
+			return false;
+	}
 
-	/* ELSP is empty, but there are ready requests? */
+	/* ELSP is empty, but there are ready requests? E.g. after reset */
 	if (READ_ONCE(engine->execlists.first))
 		return false;
 
-- 
2.17.0

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

✗ Fi.CI.CHECKPATCH: warning for series starting with [1/7] drm/i915: Flush submission tasklet after bumping priority

[Patchwork]

== Series Details ==

Series: series starting with [1/7] drm/i915: Flush submission tasklet after bumping priority
URL   : https://patchwork.freedesktop.org/series/42800/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
003f117f2171 drm/i915: Flush submission tasklet after bumping priority
a81b5d17707b drm/i915: Combine set-wedged protection and tasklet kicking
-:7: WARNING:TYPO_SPELLING: 'premption' may be misspelled - perhaps 'preemption'?
#7: 
premption disable loop to prevent set-wedge chaning function pointers

total: 0 errors, 1 warnings, 0 checks, 13 lines checked
e4c720c38170 drm/i915/execlists: Make submission tasklet hardirq safe
260be6c60570 drm/i915/guc: Make submission tasklet hardirq safe
52010b9a0967 drm/i915/execlists: Direct submit onto idle engines
-:25: WARNING:FUNCTION_ARGUMENTS: function definition argument 'flags' should also have an identifier name
#25: FILE: drivers/gpu/drm/i915/intel_guc_submission.c:757:
+	unsigned long uninitialized_var(flags);

-:58: WARNING:FUNCTION_ARGUMENTS: function definition argument 'flags' should also have an identifier name
#58: FILE: drivers/gpu/drm/i915/intel_lrc.c:760:
+	unsigned long uninitialized_var(flags);

total: 0 errors, 2 warnings, 0 checks, 132 lines checked
1a157fc92a41 drm/i915/execlists: Direct submission from irq handler
813ac493c9e1 drm/i915: Speed up idle detection by kicking the tasklets

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

✗ Fi.CI.SPARSE: warning for series starting with [1/7] drm/i915: Flush submission tasklet after bumping priority

[Patchwork]

== Series Details ==

Series: series starting with [1/7] drm/i915: Flush submission tasklet after bumping priority
URL   : https://patchwork.freedesktop.org/series/42800/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Commit: drm/i915: Flush submission tasklet after bumping priority
Okay!

Commit: drm/i915: Combine set-wedged protection and tasklet kicking
Okay!

Commit: drm/i915/execlists: Make submission tasklet hardirq safe
Okay!

Commit: drm/i915/guc: Make submission tasklet hardirq safe
Okay!

Commit: drm/i915/execlists: Direct submit onto idle engines
+drivers/gpu/drm/i915/intel_guc_submission.c:766:39: warning: context imbalance in 'guc_dequeue' - unexpected unlock
+drivers/gpu/drm/i915/intel_lrc.c:769:39: warning: context imbalance in 'execlists_dequeue' - unexpected unlock
-O:drivers/gpu/drm/i915/intel_ringbuffer.h:651:23: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/intel_ringbuffer.h:651:23: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_ringbuffer.h:658:23: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_ringbuffer.h:658:23: warning: expression using sizeof(void)

Commit: drm/i915/execlists: Direct submission from irq handler
Okay!

Commit: drm/i915: Speed up idle detection by kicking the tasklets
Okay!

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

[PATCH v3] drm/i915: Speed up idle detection by kicking the tasklets

[Chris Wilson]

We rely on ksoftirqd to run in a timely fashion in order to drain the
execlists queue. Quite frequently, it does not. In some cases we may see
latencies of over 200ms triggering our idle timeouts and forcing us to
declare the driver wedged!

Thus we can speed up idle detection by bypassing ksoftirqd in these
cases and flush our tasklet to confirm if we are indeed still waiting
for the ELSP to drain.

v2: Put the execlists.first check back; it is required for handling
reset!
v3: Follow Mika's suggestion to try and limit kicking the tasklet to
only when we expect it to make a difference, i.e. in catch up after a CS
interrupt, and not just execute it everytime as that is likely just to
cover over our own bugs.

References: https://bugs.freedesktop.org/show_bug.cgi?id=106373
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_engine_cs.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index 70325e0824e3..27f6b30e032f 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -944,11 +944,20 @@ bool intel_engine_is_idle(struct intel_engine_cs *engine)
 	if (I915_SELFTEST_ONLY(engine->breadcrumbs.mock))
 		return true;
 
+	/*
+	 * ksoftirqd has notorious latency that may cause us to
+	 * timeout while waiting for the engine to idle as we wait for
+	 * ksoftirqd to run the execlists tasklet to drain the ELSP.
+	 * If we are expecting a context switch from the GPU, check now.
+	 */
+	if (test_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted))
+		execlists_tasklet(&engine->execlists);
+
 	/* Waiting to drain ELSP? */
 	if (READ_ONCE(engine->execlists.active))
 		return false;
 
-	/* ELSP is empty, but there are ready requests? */
+	/* ELSP is empty, but there are ready requests? E.g. after reset */
 	if (READ_ONCE(engine->execlists.first))
 		return false;
 
-- 
2.17.0

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

✗ Fi.CI.BAT: failure for series starting with [1/7] drm/i915: Flush submission tasklet after bumping priority

[Patchwork]

== Series Details ==

Series: series starting with [1/7] drm/i915: Flush submission tasklet after bumping priority
URL   : https://patchwork.freedesktop.org/series/42800/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4150 -> Patchwork_8923 =

== Summary - FAILURE ==

  Serious unknown changes coming with Patchwork_8923 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_8923, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/42800/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_8923:

  === IGT changes ===

    ==== Possible regressions ====

    igt@gem_exec_parallel@basic:
      fi-skl-guc:         PASS -> INCOMPLETE

    
== Known issues ==

  Here are the changes found in Patchwork_8923 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_suspend@basic-s4-devices:
      fi-kbl-7500u:       PASS -> DMESG-WARN (fdo#105128)

    igt@gem_ringfill@basic-default-hang:
      fi-pnv-d510:        NOTRUN -> DMESG-WARN (fdo#101600)

    igt@kms_flip@basic-flip-vs-modeset:
      fi-glk-j4005:       PASS -> DMESG-WARN (fdo#106097)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-snb-2520m:       PASS -> INCOMPLETE (fdo#103713)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
      fi-cnl-y3:          PASS -> DMESG-WARN (fdo#104951)

    
    ==== Possible fixes ====

    igt@kms_frontbuffer_tracking@basic:
      fi-hsw-4200u:       DMESG-FAIL (fdo#106103, fdo#102614) -> PASS

    igt@kms_pipe_crc_basic@read-crc-pipe-b:
      fi-skl-6700k2:      FAIL (fdo#103191, fdo#104724) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-glk-j4005:       DMESG-WARN (fdo#106097) -> PASS

    
  fdo#101600 https://bugs.freedesktop.org/show_bug.cgi?id=101600
  fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
  fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724
  fdo#104951 https://bugs.freedesktop.org/show_bug.cgi?id=104951
  fdo#105128 https://bugs.freedesktop.org/show_bug.cgi?id=105128
  fdo#106097 https://bugs.freedesktop.org/show_bug.cgi?id=106097
  fdo#106103 https://bugs.freedesktop.org/show_bug.cgi?id=106103


== Participating hosts (38 -> 37) ==

  Additional (2): fi-kbl-7560u fi-pnv-d510 
  Missing    (3): fi-ctg-p8600 fi-ilk-m540 fi-skl-6700hq 


== Build changes ==

    * Linux: CI_DRM_4150 -> Patchwork_8923

  CI_DRM_4150: 93d32416ba4b1dae9451fec28aaa71915d770f51 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4463: 91b5a3ef5516b29584ea4567b0f5ffa18219b29f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8923: 813ac493c9e18c7e744dca193d7e512c38b41850 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4463: 33e58d5583eb7ed3966a1b905f875a1dfa959f6b @ git://anongit.freedesktop.org/piglit


== Linux commits ==

813ac493c9e1 drm/i915: Speed up idle detection by kicking the tasklets
1a157fc92a41 drm/i915/execlists: Direct submission from irq handler
52010b9a0967 drm/i915/execlists: Direct submit onto idle engines
260be6c60570 drm/i915/guc: Make submission tasklet hardirq safe
e4c720c38170 drm/i915/execlists: Make submission tasklet hardirq safe
a81b5d17707b drm/i915: Combine set-wedged protection and tasklet kicking
003f117f2171 drm/i915: Flush submission tasklet after bumping priority

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8923/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.CHECKPATCH: warning for series starting with [1/7] drm/i915: Flush submission tasklet after bumping priority (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [1/7] drm/i915: Flush submission tasklet after bumping priority (rev2)
URL   : https://patchwork.freedesktop.org/series/42800/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
a337a1138169 drm/i915: Flush submission tasklet after bumping priority
3fff19362e88 drm/i915: Combine set-wedged protection and tasklet kicking
-:7: WARNING:TYPO_SPELLING: 'premption' may be misspelled - perhaps 'preemption'?
#7: 
premption disable loop to prevent set-wedge chaning function pointers

total: 0 errors, 1 warnings, 0 checks, 13 lines checked
9a183196dc0c drm/i915/execlists: Make submission tasklet hardirq safe
26afd02dbd15 drm/i915/guc: Make submission tasklet hardirq safe
dbee74d26584 drm/i915/execlists: Direct submit onto idle engines
-:25: WARNING:FUNCTION_ARGUMENTS: function definition argument 'flags' should also have an identifier name
#25: FILE: drivers/gpu/drm/i915/intel_guc_submission.c:757:
+	unsigned long uninitialized_var(flags);

-:58: WARNING:FUNCTION_ARGUMENTS: function definition argument 'flags' should also have an identifier name
#58: FILE: drivers/gpu/drm/i915/intel_lrc.c:760:
+	unsigned long uninitialized_var(flags);

total: 0 errors, 2 warnings, 0 checks, 132 lines checked
637900fb273c drm/i915/execlists: Direct submission from irq handler
a6ecf3114c11 drm/i915: Speed up idle detection by kicking the tasklets

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

✗ Fi.CI.SPARSE: warning for series starting with [1/7] drm/i915: Flush submission tasklet after bumping priority (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [1/7] drm/i915: Flush submission tasklet after bumping priority (rev2)
URL   : https://patchwork.freedesktop.org/series/42800/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Commit: drm/i915: Flush submission tasklet after bumping priority
Okay!

Commit: drm/i915: Combine set-wedged protection and tasklet kicking
Okay!

Commit: drm/i915/execlists: Make submission tasklet hardirq safe
Okay!

Commit: drm/i915/guc: Make submission tasklet hardirq safe
Okay!

Commit: drm/i915/execlists: Direct submit onto idle engines
+drivers/gpu/drm/i915/intel_guc_submission.c:766:39: warning: context imbalance in 'guc_dequeue' - unexpected unlock
+drivers/gpu/drm/i915/intel_lrc.c:769:39: warning: context imbalance in 'execlists_dequeue' - unexpected unlock
-O:drivers/gpu/drm/i915/intel_ringbuffer.h:651:23: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/intel_ringbuffer.h:651:23: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_ringbuffer.h:658:23: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_ringbuffer.h:658:23: warning: expression using sizeof(void)

Commit: drm/i915/execlists: Direct submission from irq handler
Okay!

Commit: drm/i915: Speed up idle detection by kicking the tasklets
Okay!

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

✗ Fi.CI.BAT: failure for series starting with [1/7] drm/i915: Flush submission tasklet after bumping priority (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [1/7] drm/i915: Flush submission tasklet after bumping priority (rev2)
URL   : https://patchwork.freedesktop.org/series/42800/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4150 -> Patchwork_8924 =

== Summary - FAILURE ==

  Serious unknown changes coming with Patchwork_8924 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_8924, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/42800/revisions/2/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_8924:

  === IGT changes ===

    ==== Possible regressions ====

    igt@gem_exec_gttfill@basic:
      fi-skl-guc:         PASS -> INCOMPLETE

    
== Known issues ==

  Here are the changes found in Patchwork_8924 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_ringfill@basic-default-hang:
      fi-pnv-d510:        NOTRUN -> DMESG-WARN (fdo#101600)

    igt@kms_chamelium@hdmi-hpd-fast:
      fi-kbl-7500u:       SKIP -> FAIL (fdo#102672, fdo#103841)

    
    ==== Possible fixes ====

    igt@kms_frontbuffer_tracking@basic:
      fi-hsw-4200u:       DMESG-FAIL (fdo#102614, fdo#106103) -> PASS

    igt@kms_pipe_crc_basic@read-crc-pipe-b:
      fi-skl-6700k2:      FAIL (fdo#103191, fdo#104724) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-glk-j4005:       DMESG-WARN (fdo#106097) -> PASS

    
  fdo#101600 https://bugs.freedesktop.org/show_bug.cgi?id=101600
  fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
  fdo#102672 https://bugs.freedesktop.org/show_bug.cgi?id=102672
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#103841 https://bugs.freedesktop.org/show_bug.cgi?id=103841
  fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724
  fdo#106097 https://bugs.freedesktop.org/show_bug.cgi?id=106097
  fdo#106103 https://bugs.freedesktop.org/show_bug.cgi?id=106103


== Participating hosts (38 -> 37) ==

  Additional (2): fi-kbl-7560u fi-pnv-d510 
  Missing    (3): fi-ctg-p8600 fi-ilk-m540 fi-skl-6700hq 


== Build changes ==

    * Linux: CI_DRM_4150 -> Patchwork_8924

  CI_DRM_4150: 93d32416ba4b1dae9451fec28aaa71915d770f51 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4463: 91b5a3ef5516b29584ea4567b0f5ffa18219b29f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8924: a6ecf3114c11797b6f46d2e9fb7172d69a676305 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4463: 33e58d5583eb7ed3966a1b905f875a1dfa959f6b @ git://anongit.freedesktop.org/piglit


== Linux commits ==

a6ecf3114c11 drm/i915: Speed up idle detection by kicking the tasklets
637900fb273c drm/i915/execlists: Direct submission from irq handler
dbee74d26584 drm/i915/execlists: Direct submit onto idle engines
26afd02dbd15 drm/i915/guc: Make submission tasklet hardirq safe
9a183196dc0c drm/i915/execlists: Make submission tasklet hardirq safe
3fff19362e88 drm/i915: Combine set-wedged protection and tasklet kicking
a337a1138169 drm/i915: Flush submission tasklet after bumping priority

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8924/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/7] drm/i915: Flush submission tasklet after bumping priority

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> When called from process context tasklet_schedule() defers itself to
> ksoftirqd. From experience this may cause unacceptable latencies of over
> 200ms in executing the submission tasklet, our goal is to reprioritise
> the HW execution queue and trigger preemption immediately, so convert

I would add 'execlists preemption' even tho the context points to it,
as we are also quite intimately entangled here with kernel preemption
too.

> the rcu protection over to include a kick of the tasklet.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 5ece6ae4bdff..03cd30001b5d 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -578,10 +578,10 @@ static void __fence_set_priority(struct dma_fence *fence,
>  	rq = to_request(fence);
>  	engine = rq->engine;
>  
> -	rcu_read_lock();
> +	local_bh_disable(); /* RCU serialisation for set-wedged protection */
>  	if (engine->schedule)
>  		engine->schedule(rq, attr);
> -	rcu_read_unlock();
> +	local_bh_enable(); /* kick the tasklets if queues were reprioritised */

Do we end up calling the tasklet->func() explicitly on the end of
engine->schedule()?

Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

>  }
>  
>  static void fence_set_priority(struct dma_fence *fence,
> -- 
> 2.17.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/7] drm/i915: Flush submission tasklet after bumping priority

[Chris Wilson]

Quoting Mika Kuoppala (2018-05-07 12:15:58)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > When called from process context tasklet_schedule() defers itself to
> > ksoftirqd. From experience this may cause unacceptable latencies of over
> > 200ms in executing the submission tasklet, our goal is to reprioritise
> > the HW execution queue and trigger preemption immediately, so convert
> 
> I would add 'execlists preemption' even tho the context points to it,
> as we are also quite intimately entangled here with kernel preemption
> too.

I'll settle for HW preemption.
 
> > the rcu protection over to include a kick of the tasklet.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_gem.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > index 5ece6ae4bdff..03cd30001b5d 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -578,10 +578,10 @@ static void __fence_set_priority(struct dma_fence *fence,
> >       rq = to_request(fence);
> >       engine = rq->engine;
> >  
> > -     rcu_read_lock();
> > +     local_bh_disable(); /* RCU serialisation for set-wedged protection */
> >       if (engine->schedule)
> >               engine->schedule(rq, attr);
> > -     rcu_read_unlock();
> > +     local_bh_enable(); /* kick the tasklets if queues were reprioritised */
> 
> Do we end up calling the tasklet->func() explicitly on the end of
> engine->schedule()?

We do tasklet_hi_schedule(). Well in the future, we might either call
 ->func() directly or schedule. In the former case, no softirq is raised
 and local_bh_enable is just a slightly heavier preempt_disable().
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/7] drm/i915: Combine set-wedged protection and tasklet kicking

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Around request submission, we protect the call to schedule with a
> premption disable loop to prevent set-wedge chaning function pointers

s/premption/preemption
s/chaning/changing

Also RCU read critical sections can be preempted if CONFIG_PREEMPT_RCU
so it looks like we should have had explicit preempt_disable()
before the rcu_read_lock(), if it is the preempt disable we rely on?

-Mika

> underneath us. This also prevents the tasklet running on the local CPU,
> a trick we use immediately afterwards to forcibly execute the tasklet
> after submission. We can combine the two wlog, and ensure that the
> tasklet is only executed (on the local CPU) after we complete our
> updates to the submission queue (and not after an intermediate,
> incomplete update).
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_request.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
> index e4cf76ec14a6..88ee444679d4 100644
> --- a/drivers/gpu/drm/i915/i915_request.c
> +++ b/drivers/gpu/drm/i915/i915_request.c
> @@ -1110,12 +1110,9 @@ void __i915_request_add(struct i915_request *request, bool flush_caches)
>  	 * decide whether to preempt the entire chain so that it is ready to
>  	 * run at the earliest possible convenience.
>  	 */
> -	rcu_read_lock();
> +	local_bh_disable();
>  	if (engine->schedule)
>  		engine->schedule(request, &request->ctx->sched);
> -	rcu_read_unlock();
> -
> -	local_bh_disable();
>  	i915_sw_fence_commit(&request->submit);
>  	local_bh_enable(); /* Kick the execlists tasklet if just scheduled */
>  
> -- 
> 2.17.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/7] drm/i915: Combine set-wedged protection and tasklet kicking

[Chris Wilson]

Quoting Mika Kuoppala (2018-05-07 13:09:20)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > Around request submission, we protect the call to schedule with a
> > premption disable loop to prevent set-wedge chaning function pointers
> 
> s/premption/preemption
> s/chaning/changing
> 
> Also RCU read critical sections can be preempted if CONFIG_PREEMPT_RCU
> so it looks like we should have had explicit preempt_disable()
> before the rcu_read_lock(), if it is the preempt disable we rely on?

No, my mistake in not knowing about PREEMPT_RCU. So we need both. Sigh.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3] drm/i915: Speed up idle detection by kicking the tasklets

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2965 bytes --]

Hi Chris,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on v4.17-rc4 next-20180504]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Chris-Wilson/drm-i915-Speed-up-idle-detection-by-kicking-the-tasklets/20180507-184057
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: i386-randconfig-x0-05071422 (attached as .config)
compiler: gcc-5 (Debian 5.5.0-3) 5.4.1 20171010
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/gpu//drm/i915/intel_engine_cs.c: In function 'intel_engine_is_idle':
>> drivers/gpu//drm/i915/intel_engine_cs.c:954:3: error: implicit declaration of function 'execlists_tasklet' [-Werror=implicit-function-declaration]
      execlists_tasklet(&engine->execlists);
      ^
   cc1: some warnings being treated as errors

vim +/execlists_tasklet +954 drivers/gpu//drm/i915/intel_engine_cs.c

   923	
   924	/**
   925	 * intel_engine_is_idle() - Report if the engine has finished process all work
   926	 * @engine: the intel_engine_cs
   927	 *
   928	 * Return true if there are no requests pending, nothing left to be submitted
   929	 * to hardware, and that the engine is idle.
   930	 */
   931	bool intel_engine_is_idle(struct intel_engine_cs *engine)
   932	{
   933		struct drm_i915_private *dev_priv = engine->i915;
   934	
   935		/* More white lies, if wedged, hw state is inconsistent */
   936		if (i915_terminally_wedged(&dev_priv->gpu_error))
   937			return true;
   938	
   939		/* Any inflight/incomplete requests? */
   940		if (!i915_seqno_passed(intel_engine_get_seqno(engine),
   941				       intel_engine_last_submit(engine)))
   942			return false;
   943	
   944		if (I915_SELFTEST_ONLY(engine->breadcrumbs.mock))
   945			return true;
   946	
   947		/*
   948		 * ksoftirqd has notorious latency that may cause us to
   949		 * timeout while waiting for the engine to idle as we wait for
   950		 * ksoftirqd to run the execlists tasklet to drain the ELSP.
   951		 * If we are expecting a context switch from the GPU, check now.
   952		 */
   953		if (test_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted))
 > 954			execlists_tasklet(&engine->execlists);
   955	
   956		/* Waiting to drain ELSP? */
   957		if (READ_ONCE(engine->execlists.active))
   958			return false;
   959	
   960		/* ELSP is empty, but there are ready requests? E.g. after reset */
   961		if (READ_ONCE(engine->execlists.first))
   962			return false;
   963	
   964		/* Ring stopped? */
   965		if (!ring_is_idle(engine))
   966			return false;
   967	
   968		return true;
   969	}
   970	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30195 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

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