[PATCH 1/8] drm/i915/execlists: Avoid sync calls during park

[PATCH 1/8] drm/i915/execlists: Avoid sync calls during park

[Chris Wilson]

Since we allow ourselves to use non-process context during parking, we
cannot allow ourselves to sleep and in particular cannot call
del_timer_sync() -- but we can use a plain del_timer().

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111375
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gt/intel_lrc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index bb74954889dd..b97047d58d3d 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -2728,7 +2728,7 @@ static u32 *gen8_emit_fini_breadcrumb_rcs(struct i915_request *request, u32 *cs)
 
 static void execlists_park(struct intel_engine_cs *engine)
 {
-	del_timer_sync(&engine->execlists.timer);
+	del_timer(&engine->execlists.timer);
 }
 
 void intel_execlists_set_default_submission(struct intel_engine_cs *engine)
-- 
2.23.0.rc1

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

[PATCH 2/8] drm/i915/selftests: Prevent the timeslice expiring during suppression tests

[Chris Wilson]

When testing whether we prevent suppressing preemption, it helps to
avoid a time slice expiring prematurely.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111108
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gt/selftest_lrc.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c b/drivers/gpu/drm/i915/gt/selftest_lrc.c
index 91f1c9012489..b797be1627e9 100644
--- a/drivers/gpu/drm/i915/gt/selftest_lrc.c
+++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
@@ -913,6 +913,8 @@ static int live_suppress_self_preempt(void *arg)
 			goto err_wedged;
 		}
 
+		/* Keep postponing the timer to avoid premature slicing */
+		mod_timer(&engine->execlists.timer, jiffies + HZ);
 		for (depth = 0; depth < 8; depth++) {
 			rq_b = spinner_create_request(&b.spin,
 						      b.ctx, engine,
@@ -938,7 +940,8 @@ static int live_suppress_self_preempt(void *arg)
 		igt_spinner_end(&a.spin);
 
 		if (engine->execlists.preempt_hang.count) {
-			pr_err("Preemption recorded x%d, depth %d; should have been suppressed!\n",
+			pr_err("Preemption on %s recorded x%d, depth %d; should have been suppressed!\n",
+			       engine->name,
 			       engine->execlists.preempt_hang.count,
 			       depth);
 			err = -EINVAL;
-- 
2.23.0.rc1

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

[PATCH 3/8] drm/i915/guc: Use a local cancel_port_requests

[Chris Wilson]

Since execlista and the guc have diverged in their port tracking, we
cannot simply reuse the execlists cancellation code as it leads to
unbalanced reference counting. Use a local simpler routine for the guc.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_engine.h        |  3 ---
 drivers/gpu/drm/i915/gt/intel_lrc.c           |  6 ++---
 .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 23 +++++++++++--------
 3 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h
index e1228b0e577f..4b6a1cf80706 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine.h
@@ -136,9 +136,6 @@ execlists_active(const struct intel_engine_execlists *execlists)
 	return READ_ONCE(*execlists->active);
 }
 
-void
-execlists_cancel_port_requests(struct intel_engine_execlists * const execlists);
-
 struct i915_request *
 execlists_unwind_incomplete_requests(struct intel_engine_execlists *execlists);
 
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index b97047d58d3d..5c26c4ae139b 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -1297,8 +1297,8 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
 	}
 }
 
-void
-execlists_cancel_port_requests(struct intel_engine_execlists * const execlists)
+static void
+cancel_port_requests(struct intel_engine_execlists * const execlists)
 {
 	struct i915_request * const *port, *rq;
 
@@ -2355,7 +2355,7 @@ static void __execlists_reset(struct intel_engine_cs *engine, bool stalled)
 
 unwind:
 	/* Push back any incomplete requests for replay after the reset. */
-	execlists_cancel_port_requests(execlists);
+	cancel_port_requests(execlists);
 	__unwind_incomplete_requests(engine);
 }
 
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 8b83750cf96c..5bf838223cf9 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -536,11 +536,7 @@ static struct i915_request *schedule_in(struct i915_request *rq, int idx)
 {
 	trace_i915_request_in(rq, idx);
 
-	if (!rq->hw_context->inflight)
-		rq->hw_context->inflight = rq->engine;
-	intel_context_inflight_inc(rq->hw_context);
 	intel_gt_pm_get(rq->engine->gt);
-
 	return i915_request_get(rq);
 }
 
@@ -548,10 +544,6 @@ static void schedule_out(struct i915_request *rq)
 {
 	trace_i915_request_out(rq);
 
-	intel_context_inflight_dec(rq->hw_context);
-	if (!intel_context_inflight_count(rq->hw_context))
-		rq->hw_context->inflight = NULL;
-
 	intel_gt_pm_put(rq->engine->gt);
 	i915_request_put(rq);
 }
@@ -655,6 +647,17 @@ static void guc_reset_prepare(struct intel_engine_cs *engine)
 	__tasklet_disable_sync_once(&execlists->tasklet);
 }
 
+static void
+cancel_port_requests(struct intel_engine_execlists * const execlists)
+{
+	struct i915_request * const *port, *rq;
+
+	for (port = execlists->active; (rq = *port); port++)
+		schedule_out(rq);
+	execlists->active =
+		memset(execlists->inflight, 0, sizeof(execlists->inflight));
+}
+
 static void guc_reset(struct intel_engine_cs *engine, bool stalled)
 {
 	struct intel_engine_execlists * const execlists = &engine->execlists;
@@ -663,7 +666,7 @@ static void guc_reset(struct intel_engine_cs *engine, bool stalled)
 
 	spin_lock_irqsave(&engine->active.lock, flags);
 
-	execlists_cancel_port_requests(execlists);
+	cancel_port_requests(execlists);
 
 	/* Push back any incomplete requests for replay after the reset. */
 	rq = execlists_unwind_incomplete_requests(execlists);
@@ -706,7 +709,7 @@ static void guc_cancel_requests(struct intel_engine_cs *engine)
 	spin_lock_irqsave(&engine->active.lock, flags);
 
 	/* Cancel the requests on the HW and clear the ELSP tracker. */
-	execlists_cancel_port_requests(execlists);
+	cancel_port_requests(execlists);
 
 	/* Mark all executing requests as skipped. */
 	list_for_each_entry(rq, &engine->active.requests, sched.link) {
-- 
2.23.0.rc1

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

[PATCH 4/8] drm/i915: Push the wakeref->count deferral to the backend

[Chris Wilson]

If the backend wishes to defer the wakeref parking, make it responsible
for unlocking the wakeref (i.e. bumping the counter). This allows it to
time the unlock much more carefully in case it happens to needs the
wakeref to be active during its deferral.

For instance, during engine parking we may choose to emit an idle
barrier (a request). To do so, we borrow the engine->kernel_context
timeline and to ensure exclusive access we keep the
engine->wakeref.count as 0. However, to submit that request to HW may
require a intel_engine_pm_get() (e.g. to keep the submission tasklet
alive) and before we allow that we have to rewake our wakeref to avoid a
recursive deadlock.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_engine_pm.c |  8 ++-
 drivers/gpu/drm/i915/i915_request.c       | 66 ++++++++++++-----------
 drivers/gpu/drm/i915/i915_request.h       |  2 +
 drivers/gpu/drm/i915/i915_scheduler.c     |  3 +-
 drivers/gpu/drm/i915/intel_wakeref.c      |  4 +-
 drivers/gpu/drm/i915/intel_wakeref.h      | 11 ++++
 6 files changed, 56 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
index 6b15e3335dd6..ad37c9808c1f 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
@@ -68,9 +68,13 @@ static bool switch_to_kernel_context(struct intel_engine_cs *engine)
 
 	/* Check again on the next retirement. */
 	engine->wakeref_serial = engine->serial + 1;
-
 	i915_request_add_active_barriers(rq);
+
+	rq->sched.attr.priority = INT_MAX; /* Preemption barrier */
+
 	__i915_request_commit(rq);
+	__intel_wakeref_defer_park(&engine->wakeref);
+	__i915_request_queue(rq, NULL);
 
 	return false;
 }
@@ -98,7 +102,7 @@ static int __engine_park(struct intel_wakeref *wf)
 	intel_engine_pool_park(&engine->pool);
 
 	/* Must be reset upon idling, or we may miss the busy wakeup. */
-	GEM_BUG_ON(engine->execlists.queue_priority_hint != INT_MIN);
+	engine->execlists.queue_priority_hint = INT_MIN;
 
 	if (engine->park)
 		engine->park(engine);
diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index 43175bada09e..4703aab3ae21 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -1186,6 +1186,12 @@ struct i915_request *__i915_request_commit(struct i915_request *rq)
 		list_add(&ring->active_link, &rq->i915->gt.active_rings);
 	rq->emitted_jiffies = jiffies;
 
+	return prev;
+}
+
+void __i915_request_queue(struct i915_request *rq,
+			  const struct i915_sched_attr *attr)
+{
 	/*
 	 * Let the backend know a new request has arrived that may need
 	 * to adjust the existing execution schedule due to a high priority
@@ -1199,43 +1205,15 @@ struct i915_request *__i915_request_commit(struct i915_request *rq)
 	 */
 	local_bh_disable();
 	i915_sw_fence_commit(&rq->semaphore);
-	if (engine->schedule) {
-		struct i915_sched_attr attr = rq->gem_context->sched;
-
-		/*
-		 * Boost actual workloads past semaphores!
-		 *
-		 * With semaphores we spin on one engine waiting for another,
-		 * simply to reduce the latency of starting our work when
-		 * the signaler completes. However, if there is any other
-		 * work that we could be doing on this engine instead, that
-		 * is better utilisation and will reduce the overall duration
-		 * of the current work. To avoid PI boosting a semaphore
-		 * far in the distance past over useful work, we keep a history
-		 * of any semaphore use along our dependency chain.
-		 */
-		if (!(rq->sched.flags & I915_SCHED_HAS_SEMAPHORE_CHAIN))
-			attr.priority |= I915_PRIORITY_NOSEMAPHORE;
-
-		/*
-		 * Boost priorities to new clients (new request flows).
-		 *
-		 * Allow interactive/synchronous clients to jump ahead of
-		 * the bulk clients. (FQ_CODEL)
-		 */
-		if (list_empty(&rq->sched.signalers_list))
-			attr.priority |= I915_PRIORITY_WAIT;
-
-		engine->schedule(rq, &attr);
-	}
+	if (attr && rq->engine->schedule)
+		rq->engine->schedule(rq, attr);
 	i915_sw_fence_commit(&rq->submit);
 	local_bh_enable(); /* Kick the execlists tasklet if just scheduled */
-
-	return prev;
 }
 
 void i915_request_add(struct i915_request *rq)
 {
+	struct i915_sched_attr attr = rq->gem_context->sched;
 	struct i915_request *prev;
 
 	lockdep_assert_held(&rq->timeline->mutex);
@@ -1245,6 +1223,32 @@ void i915_request_add(struct i915_request *rq)
 
 	prev = __i915_request_commit(rq);
 
+	/*
+	 * Boost actual workloads past semaphores!
+	 *
+	 * With semaphores we spin on one engine waiting for another,
+	 * simply to reduce the latency of starting our work when
+	 * the signaler completes. However, if there is any other
+	 * work that we could be doing on this engine instead, that
+	 * is better utilisation and will reduce the overall duration
+	 * of the current work. To avoid PI boosting a semaphore
+	 * far in the distance past over useful work, we keep a history
+	 * of any semaphore use along our dependency chain.
+	 */
+	if (!(rq->sched.flags & I915_SCHED_HAS_SEMAPHORE_CHAIN))
+		attr.priority |= I915_PRIORITY_NOSEMAPHORE;
+
+	/*
+	 * Boost priorities to new clients (new request flows).
+	 *
+	 * Allow interactive/synchronous clients to jump ahead of
+	 * the bulk clients. (FQ_CODEL)
+	 */
+	if (list_empty(&rq->sched.signalers_list))
+		attr.priority |= I915_PRIORITY_WAIT;
+
+	__i915_request_queue(rq, &attr);
+
 	/*
 	 * In typical scenarios, we do not expect the previous request on
 	 * the timeline to be still tracked by timeline->last_request if it
diff --git a/drivers/gpu/drm/i915/i915_request.h b/drivers/gpu/drm/i915/i915_request.h
index 313df3c37158..fec1d5f17c94 100644
--- a/drivers/gpu/drm/i915/i915_request.h
+++ b/drivers/gpu/drm/i915/i915_request.h
@@ -251,6 +251,8 @@ struct i915_request * __must_check
 i915_request_create(struct intel_context *ce);
 
 struct i915_request *__i915_request_commit(struct i915_request *request);
+void __i915_request_queue(struct i915_request *rq,
+			  const struct i915_sched_attr *attr);
 
 void i915_request_retire_upto(struct i915_request *rq);
 
diff --git a/drivers/gpu/drm/i915/i915_scheduler.c b/drivers/gpu/drm/i915/i915_scheduler.c
index 0bd452e851d8..7b84ebca2901 100644
--- a/drivers/gpu/drm/i915/i915_scheduler.c
+++ b/drivers/gpu/drm/i915/i915_scheduler.c
@@ -349,8 +349,7 @@ void i915_schedule_bump_priority(struct i915_request *rq, unsigned int bump)
 	unsigned long flags;
 
 	GEM_BUG_ON(bump & ~I915_PRIORITY_MASK);
-
-	if (READ_ONCE(rq->sched.attr.priority) == I915_PRIORITY_INVALID)
+	if (READ_ONCE(rq->sched.attr.priority) & bump)
 		return;
 
 	spin_lock_irqsave(&schedule_lock, flags);
diff --git a/drivers/gpu/drm/i915/intel_wakeref.c b/drivers/gpu/drm/i915/intel_wakeref.c
index d4443e81c1c8..868cc78048d0 100644
--- a/drivers/gpu/drm/i915/intel_wakeref.c
+++ b/drivers/gpu/drm/i915/intel_wakeref.c
@@ -57,12 +57,10 @@ static void ____intel_wakeref_put_last(struct intel_wakeref *wf)
 	if (!atomic_dec_and_test(&wf->count))
 		goto unlock;
 
+	/* ops->put() must reschedule its own release on error/deferral */
 	if (likely(!wf->ops->put(wf))) {
 		rpm_put(wf);
 		wake_up_var(&wf->wakeref);
-	} else {
-		/* ops->put() must schedule its own release on deferral */
-		atomic_set_release(&wf->count, 1);
 	}
 
 unlock:
diff --git a/drivers/gpu/drm/i915/intel_wakeref.h b/drivers/gpu/drm/i915/intel_wakeref.h
index 535a3a12864b..5f0c972a80fb 100644
--- a/drivers/gpu/drm/i915/intel_wakeref.h
+++ b/drivers/gpu/drm/i915/intel_wakeref.h
@@ -163,6 +163,17 @@ intel_wakeref_is_active(const struct intel_wakeref *wf)
 	return READ_ONCE(wf->wakeref);
 }
 
+/**
+ * __intel_wakeref_defer_park: Defer the current park callback
+ * @wf: the wakeref
+ */
+static inline void
+__intel_wakeref_defer_park(struct intel_wakeref *wf)
+{
+	INTEL_WAKEREF_BUG_ON(atomic_read(&wf->count));
+	atomic_set_release(&wf->count, 1);
+}
+
 /**
  * intel_wakeref_wait_for_idle: Wait until the wakeref is idle
  * @wf: the wakeref
-- 
2.23.0.rc1

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

[PATCH 5/8] drm/i915/gt: Save/restore interrupts around breadcrumb disable

[Chris Wilson]

Stop assuming we only get called with irqs-on for disarming the
breadcrumbs, and do a full save/restore spin_lock_irq.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gt/intel_breadcrumbs.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
index e1bbc9b428cd..90db41d173df 100644
--- a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
+++ b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
@@ -67,14 +67,15 @@ static void __intel_breadcrumbs_disarm_irq(struct intel_breadcrumbs *b)
 void intel_engine_disarm_breadcrumbs(struct intel_engine_cs *engine)
 {
 	struct intel_breadcrumbs *b = &engine->breadcrumbs;
+	unsigned long flags;
 
 	if (!b->irq_armed)
 		return;
 
-	spin_lock_irq(&b->irq_lock);
+	spin_lock_irqsave(&b->irq_lock, flags);
 	if (b->irq_armed)
 		__intel_breadcrumbs_disarm_irq(b);
-	spin_unlock_irq(&b->irq_lock);
+	spin_unlock_irqrestore(&b->irq_lock, flags);
 }
 
 static inline bool __request_completed(const struct i915_request *rq)
-- 
2.23.0.rc1

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

[PATCH 6/8] drm/i915/guc: Keep the engine awake until the tasklet is idle

[Chris Wilson]

For the guc, we need to keep the engine awake (and not parked) and not
just the gt. If we let the engine park, we disable the irq and stop
processing the tasklet, leaving state outstanding inside the tasklet.

The downside is, of course, we now have to wait until the tasklet is run
before we consider the engine idle.

Reported-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 5bf838223cf9..52edfe8d1c60 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -534,9 +534,10 @@ static inline int rq_prio(const struct i915_request *rq)
 
 static struct i915_request *schedule_in(struct i915_request *rq, int idx)
 {
+	GEM_BUG_ON(!intel_engine_pm_is_awake(rq->engine));
 	trace_i915_request_in(rq, idx);
 
-	intel_gt_pm_get(rq->engine->gt);
+	intel_engine_pm_get(rq->engine);
 	return i915_request_get(rq);
 }
 
@@ -544,7 +545,7 @@ static void schedule_out(struct i915_request *rq)
 {
 	trace_i915_request_out(rq);
 
-	intel_gt_pm_put(rq->engine->gt);
+	intel_engine_pm_put(rq->engine);
 	i915_request_put(rq);
 }
 
@@ -610,8 +611,6 @@ static void guc_submission_tasklet(unsigned long data)
 	struct i915_request **port, *rq;
 	unsigned long flags;
 
-	spin_lock_irqsave(&engine->active.lock, flags);
-
 	for (port = execlists->inflight; (rq = *port); port++) {
 		if (!i915_request_completed(rq))
 			break;
@@ -624,8 +623,8 @@ static void guc_submission_tasklet(unsigned long data)
 		memmove(execlists->inflight, port, rem * sizeof(*port));
 	}
 
+	spin_lock_irqsave(&engine->active.lock, flags);
 	__guc_dequeue(engine);
-
 	spin_unlock_irqrestore(&engine->active.lock, flags);
 }
 
-- 
2.23.0.rc1

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

[PATCH 7/8] drm/i915/gt: Use the local engine wakeref when checking RING registers

[Chris Wilson]

Now that we can atomically acquire the engine wakeref, make use of it
when check whether the RING registers are idle.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gt/intel_engine_cs.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 7d174af30f8c..c7b241417ee1 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -1008,16 +1008,12 @@ void intel_engine_get_instdone(struct intel_engine_cs *engine,
 
 static bool ring_is_idle(struct intel_engine_cs *engine)
 {
-	struct drm_i915_private *dev_priv = engine->i915;
-	intel_wakeref_t wakeref;
 	bool idle = true;
 
 	if (I915_SELFTEST_ONLY(!engine->mmio_base))
 		return true;
 
-	/* If the whole device is asleep, the engine must be idle */
-	wakeref = intel_runtime_pm_get_if_in_use(&dev_priv->runtime_pm);
-	if (!wakeref)
+	if (!intel_engine_pm_get_if_awake(engine))
 		return true;
 
 	/* First check that no commands are left in the ring */
@@ -1026,11 +1022,11 @@ static bool ring_is_idle(struct intel_engine_cs *engine)
 		idle = false;
 
 	/* No bit for gen2, so assume the CS parser is idle */
-	if (INTEL_GEN(dev_priv) > 2 &&
+	if (INTEL_GEN(engine->i915) > 2 &&
 	    !(ENGINE_READ(engine, RING_MI_MODE) & MODE_IDLE))
 		idle = false;
 
-	intel_runtime_pm_put(&dev_priv->runtime_pm, wakeref);
+	intel_engine_pm_put(engine);
 
 	return idle;
 }
-- 
2.23.0.rc1

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

[PATCH 8/8] drm/i915/execlists: Lift process_csb() out of the irq-off spinlock

[Chris Wilson]

If we only call process_csb() from the tasklet, though we lose the
ability to bypass ksoftirqd interrupt processing on direct submission paths,
we can push it out of the irq-off spinlock.

The penalty is that we then allow schedule_out to be called concurrently
with schedule_in requiring us to handle the usage count (baked into the
pointer itself) atomically.

As we do kick the tasklets (via local_bh_enable()) after our submission,
there is a possibility there to see if we can pull the local softirq
processing back from the ksoftirqd.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gt/intel_context_types.h |   4 +-
 drivers/gpu/drm/i915/gt/intel_engine_cs.c     |   2 +-
 drivers/gpu/drm/i915/gt/intel_lrc.c           | 114 +++++++++++-------
 drivers/gpu/drm/i915/i915_utils.h             |  20 ++-
 4 files changed, 80 insertions(+), 60 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h b/drivers/gpu/drm/i915/gt/intel_context_types.h
index a632b20ec4d8..d8ce266c049f 100644
--- a/drivers/gpu/drm/i915/gt/intel_context_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_context_types.h
@@ -41,9 +41,7 @@ struct intel_context {
 	struct intel_engine_cs *engine;
 	struct intel_engine_cs *inflight;
 #define intel_context_inflight(ce) ptr_mask_bits((ce)->inflight, 2)
-#define intel_context_inflight_count(ce)  ptr_unmask_bits((ce)->inflight, 2)
-#define intel_context_inflight_inc(ce) ptr_count_inc(&(ce)->inflight)
-#define intel_context_inflight_dec(ce) ptr_count_dec(&(ce)->inflight)
+#define intel_context_inflight_count(ce) ptr_unmask_bits((ce)->inflight, 2)
 
 	struct i915_address_space *vm;
 	struct i915_gem_context *gem_context;
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index c7b241417ee1..13a569907c3d 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -1459,7 +1459,7 @@ int intel_enable_engine_stats(struct intel_engine_cs *engine)
 
 		for (port = execlists->pending; (rq = *port); port++) {
 			/* Exclude any contexts already counted in active */
-			if (intel_context_inflight_count(rq->hw_context) == 1)
+			if (!intel_context_inflight_count(rq->hw_context))
 				engine->stats.active++;
 		}
 
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 5c26c4ae139b..a65d78b125a0 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -547,27 +547,39 @@ execlists_context_status_change(struct i915_request *rq, unsigned long status)
 				   status, rq);
 }
 
+static inline struct intel_engine_cs *
+__execlists_schedule_in(struct i915_request *rq)
+{
+	struct intel_engine_cs * const engine = rq->engine;
+	struct intel_context * const ce = rq->hw_context;
+
+	intel_context_get(ce);
+
+	intel_gt_pm_get(engine->gt);
+	execlists_context_status_change(rq, INTEL_CONTEXT_SCHEDULE_IN);
+	intel_engine_context_in(engine);
+
+	return engine;
+}
+
 static inline struct i915_request *
 execlists_schedule_in(struct i915_request *rq, int idx)
 {
-	struct intel_context *ce = rq->hw_context;
-	int count;
+	struct intel_context * const ce = rq->hw_context;
+	struct intel_engine_cs *old;
 
+	GEM_BUG_ON(!intel_engine_pm_is_awake(rq->engine));
 	trace_i915_request_in(rq, idx);
 
-	count = intel_context_inflight_count(ce);
-	if (!count) {
-		intel_context_get(ce);
-		ce->inflight = rq->engine;
-
-		intel_gt_pm_get(ce->inflight->gt);
-		execlists_context_status_change(rq, INTEL_CONTEXT_SCHEDULE_IN);
-		intel_engine_context_in(ce->inflight);
-	}
+	old = READ_ONCE(ce->inflight);
+	do {
+		if (!old) {
+			WRITE_ONCE(ce->inflight, __execlists_schedule_in(rq));
+			break;
+		}
+	} while (!try_cmpxchg(&ce->inflight, &old, ptr_inc(old)));
 
-	intel_context_inflight_inc(ce);
 	GEM_BUG_ON(intel_context_inflight(ce) != rq->engine);
-
 	return i915_request_get(rq);
 }
 
@@ -581,35 +593,45 @@ static void kick_siblings(struct i915_request *rq, struct intel_context *ce)
 }
 
 static inline void
-execlists_schedule_out(struct i915_request *rq)
+__execlists_schedule_out(struct i915_request *rq)
 {
-	struct intel_context *ce = rq->hw_context;
+	struct intel_engine_cs * const engine = rq->engine;
+	struct intel_context * const ce = rq->hw_context;
 
-	GEM_BUG_ON(!intel_context_inflight_count(ce));
+	intel_engine_context_out(engine);
+	execlists_context_status_change(rq, INTEL_CONTEXT_SCHEDULE_OUT);
+	intel_gt_pm_put(engine->gt);
 
-	trace_i915_request_out(rq);
+	/*
+	 * If this is part of a virtual engine, its next request may
+	 * have been blocked waiting for access to the active context.
+	 * We have to kick all the siblings again in case we need to
+	 * switch (e.g. the next request is not runnable on this
+	 * engine). Hopefully, we will already have submitted the next
+	 * request before the tasklet runs and do not need to rebuild
+	 * each virtual tree and kick everyone again.
+	 */
+	if (ce->engine != engine)
+		kick_siblings(rq, ce);
 
-	intel_context_inflight_dec(ce);
-	if (!intel_context_inflight_count(ce)) {
-		intel_engine_context_out(ce->inflight);
-		execlists_context_status_change(rq, INTEL_CONTEXT_SCHEDULE_OUT);
-		intel_gt_pm_put(ce->inflight->gt);
+	intel_context_put(ce);
+}
 
-		/*
-		 * If this is part of a virtual engine, its next request may
-		 * have been blocked waiting for access to the active context.
-		 * We have to kick all the siblings again in case we need to
-		 * switch (e.g. the next request is not runnable on this
-		 * engine). Hopefully, we will already have submitted the next
-		 * request before the tasklet runs and do not need to rebuild
-		 * each virtual tree and kick everyone again.
-		 */
-		ce->inflight = NULL;
-		if (rq->engine != ce->engine)
-			kick_siblings(rq, ce);
+static inline void
+execlists_schedule_out(struct i915_request *rq)
+{
+	struct intel_context * const ce = rq->hw_context;
+	struct intel_engine_cs *cur, *old;
 
-		intel_context_put(ce);
-	}
+	trace_i915_request_out(rq);
+	GEM_BUG_ON(intel_context_inflight(ce) != rq->engine);
+
+	old = READ_ONCE(ce->inflight);
+	do
+		cur = ptr_unmask_bits(old, 2) ? ptr_dec(old) : NULL;
+	while (!try_cmpxchg(&ce->inflight, &old, cur));
+	if (!cur)
+		__execlists_schedule_out(rq);
 
 	i915_request_put(rq);
 }
@@ -684,6 +706,9 @@ assert_pending_valid(const struct intel_engine_execlists *execlists,
 
 	trace_ports(execlists, msg, execlists->pending);
 
+	if (!execlists->pending[0])
+		return false;
+
 	if (execlists->pending[execlists_num_ports(execlists)])
 		return false;
 
@@ -1356,7 +1381,6 @@ static void process_csb(struct intel_engine_cs *engine)
 	const u8 num_entries = execlists->csb_size;
 	u8 head, tail;
 
-	lockdep_assert_held(&engine->active.lock);
 	GEM_BUG_ON(USES_GUC_SUBMISSION(engine->i915));
 
 	/*
@@ -1427,15 +1451,14 @@ static void process_csb(struct intel_engine_cs *engine)
 				       execlists->pending,
 				       execlists_num_ports(execlists) *
 				       sizeof(*execlists->pending));
-			execlists->pending[0] = NULL;
-
-			trace_ports(execlists, "promoted", execlists->active);
 
 			if (enable_timeslice(engine))
 				mod_timer(&execlists->timer, jiffies + 1);
 
 			if (!inject_preempt_hang(execlists))
 				ring_set_paused(engine, 0);
+
+			WRITE_ONCE(execlists->pending[0], NULL);
 			break;
 
 		case CSB_COMPLETE: /* port0 completed, advanced to port1 */
@@ -1479,8 +1502,6 @@ static void process_csb(struct intel_engine_cs *engine)
 static void __execlists_submission_tasklet(struct intel_engine_cs *const engine)
 {
 	lockdep_assert_held(&engine->active.lock);
-
-	process_csb(engine);
 	if (!engine->execlists.pending[0])
 		execlists_dequeue(engine);
 }
@@ -1494,9 +1515,12 @@ static void execlists_submission_tasklet(unsigned long data)
 	struct intel_engine_cs * const engine = (struct intel_engine_cs *)data;
 	unsigned long flags;
 
-	spin_lock_irqsave(&engine->active.lock, flags);
-	__execlists_submission_tasklet(engine);
-	spin_unlock_irqrestore(&engine->active.lock, flags);
+	process_csb(engine);
+	if (!engine->execlists.pending[0]) {
+		spin_lock_irqsave(&engine->active.lock, flags);
+		__execlists_submission_tasklet(engine);
+		spin_unlock_irqrestore(&engine->active.lock, flags);
+	}
 }
 
 static void execlists_submission_timer(struct timer_list *timer)
diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h
index d652ba5d2320..562f756da421 100644
--- a/drivers/gpu/drm/i915/i915_utils.h
+++ b/drivers/gpu/drm/i915/i915_utils.h
@@ -161,17 +161,15 @@ __check_struct_size(size_t base, size_t arr, size_t count, size_t *size)
 	((typeof(ptr))((unsigned long)(ptr) | __bits));			\
 })
 
-#define ptr_count_dec(p_ptr) do {					\
-	typeof(p_ptr) __p = (p_ptr);					\
-	unsigned long __v = (unsigned long)(*__p);			\
-	*__p = (typeof(*p_ptr))(--__v);					\
-} while (0)
-
-#define ptr_count_inc(p_ptr) do {					\
-	typeof(p_ptr) __p = (p_ptr);					\
-	unsigned long __v = (unsigned long)(*__p);			\
-	*__p = (typeof(*p_ptr))(++__v);					\
-} while (0)
+#define ptr_dec(ptr) ({							\
+	unsigned long __v = (unsigned long)(ptr);			\
+	(typeof(ptr))(__v - 1);						\
+})
+
+#define ptr_inc(ptr) ({							\
+	unsigned long __v = (unsigned long)(ptr);			\
+	(typeof(ptr))(__v + 1);						\
+})
 
 #define page_mask_bits(ptr) ptr_mask_bits(ptr, PAGE_SHIFT)
 #define page_unmask_bits(ptr) ptr_unmask_bits(ptr, PAGE_SHIFT)
-- 
2.23.0.rc1

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

Re: [PATCH 1/8] drm/i915/execlists: Avoid sync calls during park

[Mika Kuoppala]

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

> Since we allow ourselves to use non-process context during parking, we
> cannot allow ourselves to sleep and in particular cannot call
> del_timer_sync() -- but we can use a plain del_timer().
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111375
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/gt/intel_lrc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index bb74954889dd..b97047d58d3d 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -2728,7 +2728,7 @@ static u32 *gen8_emit_fini_breadcrumb_rcs(struct i915_request *request, u32 *cs)
>  
>  static void execlists_park(struct intel_engine_cs *engine)
>  {
> -	del_timer_sync(&engine->execlists.timer);
> +	del_timer(&engine->execlists.timer);

There will be another sync point then somewhere else or not needed?

Also are irq safe timers where we could do a sync deletion. 

So my question is why the need for a sync point disappeared?

-Mika
>  }
>  
>  void intel_execlists_set_default_submission(struct intel_engine_cs *engine)
> -- 
> 2.23.0.rc1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/8] drm/i915/execlists: Avoid sync calls during park

[Chris Wilson]

Quoting Mika Kuoppala (2019-08-12 10:27:16)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > Since we allow ourselves to use non-process context during parking, we
> > cannot allow ourselves to sleep and in particular cannot call
> > del_timer_sync() -- but we can use a plain del_timer().
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111375
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/gt/intel_lrc.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> > index bb74954889dd..b97047d58d3d 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> > @@ -2728,7 +2728,7 @@ static u32 *gen8_emit_fini_breadcrumb_rcs(struct i915_request *request, u32 *cs)
> >  
> >  static void execlists_park(struct intel_engine_cs *engine)
> >  {
> > -     del_timer_sync(&engine->execlists.timer);
> > +     del_timer(&engine->execlists.timer);
> 
> There will be another sync point then somewhere else or not needed?

Not required, as it means the timer if currently running and will just
kick the tasklet (as it does today). The tasklet running after we park
is not a huge issue as it doesn't touch HW -- it checks a CPU mapping
and in the process drains the GT wakeref.
 
> Also are irq safe timers where we could do a sync deletion. 
> 
> So my question is why the need for a sync point disappeared?

We didn't use it correctly to begin with :) To complete the sync, we
should have put a tasklet_kill(&execlists->tasklet); afterwards.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/8] drm/i915/selftests: Prevent the timeslice expiring during suppression tests

[Mika Kuoppala]

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

> When testing whether we prevent suppressing preemption, it helps to
> avoid a time slice expiring prematurely.
>

I did look the test and it does call schedule on it's own.

So what we want to do is to postpone the defacto schedule tick
provided by driver not to mess our own schedule? (which we
use to check that no preemption does occur with equal
priorities?)

Just trying to figure out if I got the test framework right :O
-Mika


> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111108
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/gt/selftest_lrc.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c b/drivers/gpu/drm/i915/gt/selftest_lrc.c
> index 91f1c9012489..b797be1627e9 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
> @@ -913,6 +913,8 @@ static int live_suppress_self_preempt(void *arg)
>  			goto err_wedged;
>  		}
>  
> +		/* Keep postponing the timer to avoid premature slicing */
> +		mod_timer(&engine->execlists.timer, jiffies + HZ);
>  		for (depth = 0; depth < 8; depth++) {
>  			rq_b = spinner_create_request(&b.spin,
>  						      b.ctx, engine,
> @@ -938,7 +940,8 @@ static int live_suppress_self_preempt(void *arg)
>  		igt_spinner_end(&a.spin);
>  
>  		if (engine->execlists.preempt_hang.count) {
> -			pr_err("Preemption recorded x%d, depth %d; should have been suppressed!\n",
> +			pr_err("Preemption on %s recorded x%d, depth %d; should have been suppressed!\n",
> +			       engine->name,
>  			       engine->execlists.preempt_hang.count,
>  			       depth);
>  			err = -EINVAL;
> -- 
> 2.23.0.rc1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/8] drm/i915/execlists: Avoid sync calls during park

[Mika Kuoppala]

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

> Quoting Mika Kuoppala (2019-08-12 10:27:16)
>> Chris Wilson <chris@chris-wilson.co.uk> writes:
>> 
>> > Since we allow ourselves to use non-process context during parking, we
>> > cannot allow ourselves to sleep and in particular cannot call
>> > del_timer_sync() -- but we can use a plain del_timer().
>> >
>> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111375
>> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> > ---
>> >  drivers/gpu/drm/i915/gt/intel_lrc.c | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
>> > index bb74954889dd..b97047d58d3d 100644
>> > --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
>> > +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
>> > @@ -2728,7 +2728,7 @@ static u32 *gen8_emit_fini_breadcrumb_rcs(struct i915_request *request, u32 *cs)
>> >  
>> >  static void execlists_park(struct intel_engine_cs *engine)
>> >  {
>> > -     del_timer_sync(&engine->execlists.timer);
>> > +     del_timer(&engine->execlists.timer);
>> 
>> There will be another sync point then somewhere else or not needed?
>
> Not required, as it means the timer if currently running and will just
> kick the tasklet (as it does today). The tasklet running after we park
> is not a huge issue as it doesn't touch HW -- it checks a CPU mapping
> and in the process drains the GT wakeref.
>  
>> Also are irq safe timers where we could do a sync deletion. 
>> 
>> So my question is why the need for a sync point disappeared?
>
> We didn't use it correctly to begin with :) To complete the sync, we
> should have put a tasklet_kill(&execlists->tasklet); afterwards.

Ok,
So no need for fancey irq safe timers either.

Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/8] drm/i915/selftests: Prevent the timeslice expiring during suppression tests

[Chris Wilson]

Quoting Mika Kuoppala (2019-08-12 10:39:01)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > When testing whether we prevent suppressing preemption, it helps to
> > avoid a time slice expiring prematurely.
> >
> 
> I did look the test and it does call schedule on it's own.
> 
> So what we want to do is to postpone the defacto schedule tick
> provided by driver not to mess our own schedule? (which we
> use to check that no preemption does occur with equal
> priorities?)

The test is trying to look at our mechanics to ensure that we don't
cause preemptions where we simply put back the same request. As such, we
have a marker in the preemption code that we are trying to avoid, and
must control the scheduling to exclude all other events than the one we
are injecting.

The timeslice could expire and reverse A,B (to B,A) such that our
promotion of A does (correctly) cause a preemption that we expect never
to need.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/8] drm/i915/selftests: Prevent the timeslice expiring during suppression tests

[Mika Kuoppala]

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

> Quoting Mika Kuoppala (2019-08-12 10:39:01)
>> Chris Wilson <chris@chris-wilson.co.uk> writes:
>> 
>> > When testing whether we prevent suppressing preemption, it helps to
>> > avoid a time slice expiring prematurely.
>> >
>> 
>> I did look the test and it does call schedule on it's own.
>> 
>> So what we want to do is to postpone the defacto schedule tick
>> provided by driver not to mess our own schedule? (which we
>> use to check that no preemption does occur with equal
>> priorities?)
>
> The test is trying to look at our mechanics to ensure that we don't
> cause preemptions where we simply put back the same request. As such, we
> have a marker in the preemption code that we are trying to avoid, and
> must control the scheduling to exclude all other events than the one we
> are injecting.
>
> The timeslice could expire and reverse A,B (to B,A) such that our
> promotion of A does (correctly) cause a preemption that we expect never
> to need.

If there will be more users, then we can consider
disable|enable_reschedule_timer or similar.

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

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

Re: [PATCH 6/8] drm/i915/guc: Keep the engine awake until the tasklet is idle

[Chris Wilson]

Quoting Chris Wilson (2019-08-12 10:10:43)
> For the guc, we need to keep the engine awake (and not parked) and not
> just the gt. If we let the engine park, we disable the irq and stop
> processing the tasklet, leaving state outstanding inside the tasklet.
> 
> The downside is, of course, we now have to wait until the tasklet is run
> before we consider the engine idle.

Fwiw, because of this I think it may be preferable to keep to using GT
pm for the tasklet; and apply Daniele's patch to keep
NEEDS_BREADCRUMB_TASKLET set (which is the right thing to do anyway now
that we stop switching between submission modes).
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm/i915/execlists: Lift process_csb() out of the irq-off spinlock

[Chris Wilson]

If we only call process_csb() from the tasklet, though we lose the
ability to bypass ksoftirqd interrupt processing on direct submission paths,
we can push it out of the irq-off spinlock.

The penalty is that we then allow schedule_out to be called concurrently
with schedule_in requiring us to handle the usage count (baked into the
pointer itself) atomically.

As we do kick the tasklets (via local_bh_enable()) after our submission,
there is a possibility there to see if we can pull the local softirq
processing back from the ksoftirqd.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
We have to restrict enable_timeslice() to only consider the information
under its control (i.e. execlists->active[])
---
 drivers/gpu/drm/i915/gt/intel_context_types.h |   4 +-
 drivers/gpu/drm/i915/gt/intel_engine_cs.c     |   2 +-
 drivers/gpu/drm/i915/gt/intel_lrc.c           | 130 +++++++++++-------
 drivers/gpu/drm/i915/i915_utils.h             |  20 ++-
 4 files changed, 94 insertions(+), 62 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h b/drivers/gpu/drm/i915/gt/intel_context_types.h
index a632b20ec4d8..d8ce266c049f 100644
--- a/drivers/gpu/drm/i915/gt/intel_context_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_context_types.h
@@ -41,9 +41,7 @@ struct intel_context {
 	struct intel_engine_cs *engine;
 	struct intel_engine_cs *inflight;
 #define intel_context_inflight(ce) ptr_mask_bits((ce)->inflight, 2)
-#define intel_context_inflight_count(ce)  ptr_unmask_bits((ce)->inflight, 2)
-#define intel_context_inflight_inc(ce) ptr_count_inc(&(ce)->inflight)
-#define intel_context_inflight_dec(ce) ptr_count_dec(&(ce)->inflight)
+#define intel_context_inflight_count(ce) ptr_unmask_bits((ce)->inflight, 2)
 
 	struct i915_address_space *vm;
 	struct i915_gem_context *gem_context;
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index c7b241417ee1..13a569907c3d 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -1459,7 +1459,7 @@ int intel_enable_engine_stats(struct intel_engine_cs *engine)
 
 		for (port = execlists->pending; (rq = *port); port++) {
 			/* Exclude any contexts already counted in active */
-			if (intel_context_inflight_count(rq->hw_context) == 1)
+			if (!intel_context_inflight_count(rq->hw_context))
 				engine->stats.active++;
 		}
 
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 5c26c4ae139b..945f3acc2e75 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -547,27 +547,39 @@ execlists_context_status_change(struct i915_request *rq, unsigned long status)
 				   status, rq);
 }
 
+static inline struct intel_engine_cs *
+__execlists_schedule_in(struct i915_request *rq)
+{
+	struct intel_engine_cs * const engine = rq->engine;
+	struct intel_context * const ce = rq->hw_context;
+
+	intel_context_get(ce);
+
+	intel_gt_pm_get(engine->gt);
+	execlists_context_status_change(rq, INTEL_CONTEXT_SCHEDULE_IN);
+	intel_engine_context_in(engine);
+
+	return engine;
+}
+
 static inline struct i915_request *
 execlists_schedule_in(struct i915_request *rq, int idx)
 {
-	struct intel_context *ce = rq->hw_context;
-	int count;
+	struct intel_context * const ce = rq->hw_context;
+	struct intel_engine_cs *old;
 
+	GEM_BUG_ON(!intel_engine_pm_is_awake(rq->engine));
 	trace_i915_request_in(rq, idx);
 
-	count = intel_context_inflight_count(ce);
-	if (!count) {
-		intel_context_get(ce);
-		ce->inflight = rq->engine;
-
-		intel_gt_pm_get(ce->inflight->gt);
-		execlists_context_status_change(rq, INTEL_CONTEXT_SCHEDULE_IN);
-		intel_engine_context_in(ce->inflight);
-	}
+	old = READ_ONCE(ce->inflight);
+	do {
+		if (!old) {
+			WRITE_ONCE(ce->inflight, __execlists_schedule_in(rq));
+			break;
+		}
+	} while (!try_cmpxchg(&ce->inflight, &old, ptr_inc(old)));
 
-	intel_context_inflight_inc(ce);
 	GEM_BUG_ON(intel_context_inflight(ce) != rq->engine);
-
 	return i915_request_get(rq);
 }
 
@@ -581,35 +593,45 @@ static void kick_siblings(struct i915_request *rq, struct intel_context *ce)
 }
 
 static inline void
-execlists_schedule_out(struct i915_request *rq)
+__execlists_schedule_out(struct i915_request *rq)
 {
-	struct intel_context *ce = rq->hw_context;
+	struct intel_engine_cs * const engine = rq->engine;
+	struct intel_context * const ce = rq->hw_context;
 
-	GEM_BUG_ON(!intel_context_inflight_count(ce));
+	intel_engine_context_out(engine);
+	execlists_context_status_change(rq, INTEL_CONTEXT_SCHEDULE_OUT);
+	intel_gt_pm_put(engine->gt);
 
-	trace_i915_request_out(rq);
+	/*
+	 * If this is part of a virtual engine, its next request may
+	 * have been blocked waiting for access to the active context.
+	 * We have to kick all the siblings again in case we need to
+	 * switch (e.g. the next request is not runnable on this
+	 * engine). Hopefully, we will already have submitted the next
+	 * request before the tasklet runs and do not need to rebuild
+	 * each virtual tree and kick everyone again.
+	 */
+	if (ce->engine != engine)
+		kick_siblings(rq, ce);
 
-	intel_context_inflight_dec(ce);
-	if (!intel_context_inflight_count(ce)) {
-		intel_engine_context_out(ce->inflight);
-		execlists_context_status_change(rq, INTEL_CONTEXT_SCHEDULE_OUT);
-		intel_gt_pm_put(ce->inflight->gt);
+	intel_context_put(ce);
+}
 
-		/*
-		 * If this is part of a virtual engine, its next request may
-		 * have been blocked waiting for access to the active context.
-		 * We have to kick all the siblings again in case we need to
-		 * switch (e.g. the next request is not runnable on this
-		 * engine). Hopefully, we will already have submitted the next
-		 * request before the tasklet runs and do not need to rebuild
-		 * each virtual tree and kick everyone again.
-		 */
-		ce->inflight = NULL;
-		if (rq->engine != ce->engine)
-			kick_siblings(rq, ce);
+static inline void
+execlists_schedule_out(struct i915_request *rq)
+{
+	struct intel_context * const ce = rq->hw_context;
+	struct intel_engine_cs *cur, *old;
 
-		intel_context_put(ce);
-	}
+	trace_i915_request_out(rq);
+	GEM_BUG_ON(intel_context_inflight(ce) != rq->engine);
+
+	old = READ_ONCE(ce->inflight);
+	do
+		cur = ptr_unmask_bits(old, 2) ? ptr_dec(old) : NULL;
+	while (!try_cmpxchg(&ce->inflight, &old, cur));
+	if (!cur)
+		__execlists_schedule_out(rq);
 
 	i915_request_put(rq);
 }
@@ -684,6 +706,9 @@ assert_pending_valid(const struct intel_engine_execlists *execlists,
 
 	trace_ports(execlists, msg, execlists->pending);
 
+	if (!execlists->pending[0])
+		return false;
+
 	if (execlists->pending[execlists_num_ports(execlists)])
 		return false;
 
@@ -944,9 +969,21 @@ need_timeslice(struct intel_engine_cs *engine, const struct i915_request *rq)
 static bool
 enable_timeslice(struct intel_engine_cs *engine)
 {
-	struct i915_request *last = last_active(&engine->execlists);
+	struct i915_request * const *port;
+	int hint;
+
+	port = engine->execlists.active;
+	while (port[0] && i915_request_completed(port[0]))
+		port++;
+	if (!port[0])
+		return false;
 
-	return last && need_timeslice(engine, last);
+	hint = engine->execlists.queue_priority_hint;
+	if (port[1])
+		hint = max(rq_prio(port[1]), hint);
+
+	/* Compare the two end-points as an unlocked approximation */
+	return hint >= effective_prio(port[0]);
 }
 
 static void record_preemption(struct intel_engine_execlists *execlists)
@@ -1356,7 +1393,6 @@ static void process_csb(struct intel_engine_cs *engine)
 	const u8 num_entries = execlists->csb_size;
 	u8 head, tail;
 
-	lockdep_assert_held(&engine->active.lock);
 	GEM_BUG_ON(USES_GUC_SUBMISSION(engine->i915));
 
 	/*
@@ -1427,15 +1463,14 @@ static void process_csb(struct intel_engine_cs *engine)
 				       execlists->pending,
 				       execlists_num_ports(execlists) *
 				       sizeof(*execlists->pending));
-			execlists->pending[0] = NULL;
-
-			trace_ports(execlists, "promoted", execlists->active);
 
 			if (enable_timeslice(engine))
 				mod_timer(&execlists->timer, jiffies + 1);
 
 			if (!inject_preempt_hang(execlists))
 				ring_set_paused(engine, 0);
+
+			WRITE_ONCE(execlists->pending[0], NULL);
 			break;
 
 		case CSB_COMPLETE: /* port0 completed, advanced to port1 */
@@ -1479,8 +1514,6 @@ static void process_csb(struct intel_engine_cs *engine)
 static void __execlists_submission_tasklet(struct intel_engine_cs *const engine)
 {
 	lockdep_assert_held(&engine->active.lock);
-
-	process_csb(engine);
 	if (!engine->execlists.pending[0])
 		execlists_dequeue(engine);
 }
@@ -1494,9 +1527,12 @@ static void execlists_submission_tasklet(unsigned long data)
 	struct intel_engine_cs * const engine = (struct intel_engine_cs *)data;
 	unsigned long flags;
 
-	spin_lock_irqsave(&engine->active.lock, flags);
-	__execlists_submission_tasklet(engine);
-	spin_unlock_irqrestore(&engine->active.lock, flags);
+	process_csb(engine);
+	if (!engine->execlists.pending[0]) {
+		spin_lock_irqsave(&engine->active.lock, flags);
+		__execlists_submission_tasklet(engine);
+		spin_unlock_irqrestore(&engine->active.lock, flags);
+	}
 }
 
 static void execlists_submission_timer(struct timer_list *timer)
diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h
index d652ba5d2320..562f756da421 100644
--- a/drivers/gpu/drm/i915/i915_utils.h
+++ b/drivers/gpu/drm/i915/i915_utils.h
@@ -161,17 +161,15 @@ __check_struct_size(size_t base, size_t arr, size_t count, size_t *size)
 	((typeof(ptr))((unsigned long)(ptr) | __bits));			\
 })
 
-#define ptr_count_dec(p_ptr) do {					\
-	typeof(p_ptr) __p = (p_ptr);					\
-	unsigned long __v = (unsigned long)(*__p);			\
-	*__p = (typeof(*p_ptr))(--__v);					\
-} while (0)
-
-#define ptr_count_inc(p_ptr) do {					\
-	typeof(p_ptr) __p = (p_ptr);					\
-	unsigned long __v = (unsigned long)(*__p);			\
-	*__p = (typeof(*p_ptr))(++__v);					\
-} while (0)
+#define ptr_dec(ptr) ({							\
+	unsigned long __v = (unsigned long)(ptr);			\
+	(typeof(ptr))(__v - 1);						\
+})
+
+#define ptr_inc(ptr) ({							\
+	unsigned long __v = (unsigned long)(ptr);			\
+	(typeof(ptr))(__v + 1);						\
+})
 
 #define page_mask_bits(ptr) ptr_mask_bits(ptr, PAGE_SHIFT)
 #define page_unmask_bits(ptr) ptr_unmask_bits(ptr, PAGE_SHIFT)
-- 
2.23.0.rc1

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

Re: [PATCH 7/8] drm/i915/gt: Use the local engine wakeref when checking RING registers

[Mika Kuoppala]

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

> Now that we can atomically acquire the engine wakeref, make use of it
> when check whether the RING registers are idle.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

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

> ---
>  drivers/gpu/drm/i915/gt/intel_engine_cs.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index 7d174af30f8c..c7b241417ee1 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -1008,16 +1008,12 @@ void intel_engine_get_instdone(struct intel_engine_cs *engine,
>  
>  static bool ring_is_idle(struct intel_engine_cs *engine)
>  {
> -	struct drm_i915_private *dev_priv = engine->i915;
> -	intel_wakeref_t wakeref;
>  	bool idle = true;
>  
>  	if (I915_SELFTEST_ONLY(!engine->mmio_base))
>  		return true;
>  
> -	/* If the whole device is asleep, the engine must be idle */
> -	wakeref = intel_runtime_pm_get_if_in_use(&dev_priv->runtime_pm);
> -	if (!wakeref)
> +	if (!intel_engine_pm_get_if_awake(engine))
>  		return true;
>  
>  	/* First check that no commands are left in the ring */
> @@ -1026,11 +1022,11 @@ static bool ring_is_idle(struct intel_engine_cs *engine)
>  		idle = false;
>  
>  	/* No bit for gen2, so assume the CS parser is idle */
> -	if (INTEL_GEN(dev_priv) > 2 &&
> +	if (INTEL_GEN(engine->i915) > 2 &&
>  	    !(ENGINE_READ(engine, RING_MI_MODE) & MODE_IDLE))
>  		idle = false;
>  
> -	intel_runtime_pm_put(&dev_priv->runtime_pm, wakeref);
> +	intel_engine_pm_put(engine);
>  
>  	return idle;
>  }
> -- 
> 2.23.0.rc1
_______________________________________________
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/8] drm/i915/execlists: Avoid sync calls during park (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [1/8] drm/i915/execlists: Avoid sync calls during park (rev2)
URL   : https://patchwork.freedesktop.org/series/65080/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
aa05f673a683 drm/i915/execlists: Avoid sync calls during park
56311376df00 drm/i915/selftests: Prevent the timeslice expiring during suppression tests
2a6559c60e40 drm/i915/guc: Use a local cancel_port_requests
c4f51bd32424 drm/i915: Push the wakeref->count deferral to the backend
2963aca747f2 drm/i915/gt: Save/restore interrupts around breadcrumb disable
f82033348344 drm/i915/guc: Keep the engine awake until the tasklet is idle
01856543d38e drm/i915/gt: Use the local engine wakeref when checking RING registers
78d72ec3911b drm/i915/execlists: Lift process_csb() out of the irq-off spinlock
-:8: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#8: 
ability to bypass ksoftirqd interrupt processing on direct submission paths,

total: 0 errors, 1 warnings, 0 checks, 243 lines checked

_______________________________________________
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/8] drm/i915/execlists: Avoid sync calls during park (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [1/8] drm/i915/execlists: Avoid sync calls during park (rev2)
URL   : https://patchwork.freedesktop.org/series/65080/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm/i915/execlists: Avoid sync calls during park
Okay!

Commit: drm/i915/selftests: Prevent the timeslice expiring during suppression tests
Okay!

Commit: drm/i915/guc: Use a local cancel_port_requests
Okay!

Commit: drm/i915: Push the wakeref->count deferral to the backend
Okay!

Commit: drm/i915/gt: Save/restore interrupts around breadcrumb disable
Okay!

Commit: drm/i915/guc: Keep the engine awake until the tasklet is idle
Okay!

Commit: drm/i915/gt: Use the local engine wakeref when checking RING registers
Okay!

Commit: drm/i915/execlists: Lift process_csb() out of the irq-off spinlock
+drivers/gpu/drm/i915/gt/intel_lrc.c:983:24: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/gt/intel_lrc.c:983:24: warning: expression using sizeof(void)
-./drivers/gpu/drm/i915/i915_utils.h:262:16: warning: expression using sizeof(void)
-./drivers/gpu/drm/i915/i915_utils.h:262:16: warning: expression using sizeof(void)
-./drivers/gpu/drm/i915/i915_utils.h:262:16: warning: expression using sizeof(void)
-./drivers/gpu/drm/i915/i915_utils.h:262:16: warning: expression using sizeof(void)
-./drivers/gpu/drm/i915/i915_utils.h:262:16: warning: expression using sizeof(void)
-./drivers/gpu/drm/i915/i915_utils.h:262:16: warning: expression using sizeof(void)
-./drivers/gpu/drm/i915/i915_utils.h:262:16: warning: expression using sizeof(void)
-./drivers/gpu/drm/i915/i915_utils.h:262:16: warning: expression using sizeof(void)
+./drivers/gpu/drm/i915/i915_utils.h:260:16: warning: expression using sizeof(void)
+./drivers/gpu/drm/i915/i915_utils.h:260:16: warning: expression using sizeof(void)
+./drivers/gpu/drm/i915/i915_utils.h:260:16: warning: expression using sizeof(void)
+./drivers/gpu/drm/i915/i915_utils.h:260:16: warning: expression using sizeof(void)
+./drivers/gpu/drm/i915/i915_utils.h:260:16: warning: expression using sizeof(void)
+./drivers/gpu/drm/i915/i915_utils.h:260:16: warning: expression using sizeof(void)
+./drivers/gpu/drm/i915/i915_utils.h:260:16: warning: expression using sizeof(void)
+./drivers/gpu/drm/i915/i915_utils.h:260:16: warning: expression using sizeof(void)
-drivers/gpu/drm/i915/selftests/../i915_utils.h:262:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_utils.h:260:16: warning: expression using sizeof(void)

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

✓ Fi.CI.BAT: success for series starting with [1/8] drm/i915/execlists: Avoid sync calls during park (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [1/8] drm/i915/execlists: Avoid sync calls during park (rev2)
URL   : https://patchwork.freedesktop.org/series/65080/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6685 -> Patchwork_13984
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13984/

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_fence@basic-wait-default:
    - fi-icl-u3:          [PASS][1] -> [DMESG-WARN][2] ([fdo#107724])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6685/fi-icl-u3/igt@gem_exec_fence@basic-wait-default.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13984/fi-icl-u3/igt@gem_exec_fence@basic-wait-default.html

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-blb-e6850:       [PASS][3] -> [INCOMPLETE][4] ([fdo#107718])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6685/fi-blb-e6850/igt@gem_exec_suspend@basic-s4-devices.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13984/fi-blb-e6850/igt@gem_exec_suspend@basic-s4-devices.html

  * igt@i915_selftest@live_execlists:
    - fi-skl-gvtdvm:      [PASS][5] -> [DMESG-FAIL][6] ([fdo#111108])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6685/fi-skl-gvtdvm/igt@i915_selftest@live_execlists.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13984/fi-skl-gvtdvm/igt@i915_selftest@live_execlists.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7567u:       [PASS][7] -> [FAIL][8] ([fdo#109485])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6685/fi-kbl-7567u/igt@kms_chamelium@hdmi-hpd-fast.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13984/fi-kbl-7567u/igt@kms_chamelium@hdmi-hpd-fast.html

  * igt@kms_frontbuffer_tracking@basic:
    - fi-icl-u2:          [PASS][9] -> [FAIL][10] ([fdo#103167])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6685/fi-icl-u2/igt@kms_frontbuffer_tracking@basic.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13984/fi-icl-u2/igt@kms_frontbuffer_tracking@basic.html

  * igt@prime_vgem@basic-fence-flip:
    - fi-kbl-7500u:       [PASS][11] -> [SKIP][12] ([fdo#109271]) +23 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6685/fi-kbl-7500u/igt@prime_vgem@basic-fence-flip.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13984/fi-kbl-7500u/igt@prime_vgem@basic-fence-flip.html

  
#### Possible fixes ####

  * igt@gem_mmap_gtt@basic-read-write:
    - fi-icl-u3:          [DMESG-WARN][13] ([fdo#107724]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6685/fi-icl-u3/igt@gem_mmap_gtt@basic-read-write.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13984/fi-icl-u3/igt@gem_mmap_gtt@basic-read-write.html

  * igt@i915_selftest@live_active:
    - fi-bsw-n3050:       [DMESG-WARN][15] ([fdo#111373]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6685/fi-bsw-n3050/igt@i915_selftest@live_active.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13984/fi-bsw-n3050/igt@i915_selftest@live_active.html

  * igt@kms_chamelium@dp-crc-fast:
    - fi-cml-u2:          [FAIL][17] ([fdo#109483]) -> [PASS][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6685/fi-cml-u2/igt@kms_chamelium@dp-crc-fast.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13984/fi-cml-u2/igt@kms_chamelium@dp-crc-fast.html

  
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109483]: https://bugs.freedesktop.org/show_bug.cgi?id=109483
  [fdo#109485]: https://bugs.freedesktop.org/show_bug.cgi?id=109485
  [fdo#111108]: https://bugs.freedesktop.org/show_bug.cgi?id=111108
  [fdo#111373]: https://bugs.freedesktop.org/show_bug.cgi?id=111373


Participating hosts (55 -> 47)
------------------------------

  Missing    (8): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-y fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_6685 -> Patchwork_13984

  CI-20190529: 20190529
  CI_DRM_6685: acabc817e999dd7a158654fb207f7e61d68295f9 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5127: f43f5fa12ac1b93febfe3eeb9e9985f5f3e2eff0 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13984: 78d72ec3911b38f273d4d782fa15bf43f2ad72af @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

78d72ec3911b drm/i915/execlists: Lift process_csb() out of the irq-off spinlock
01856543d38e drm/i915/gt: Use the local engine wakeref when checking RING registers
f82033348344 drm/i915/guc: Keep the engine awake until the tasklet is idle
2963aca747f2 drm/i915/gt: Save/restore interrupts around breadcrumb disable
c4f51bd32424 drm/i915: Push the wakeref->count deferral to the backend
2a6559c60e40 drm/i915/guc: Use a local cancel_port_requests
56311376df00 drm/i915/selftests: Prevent the timeslice expiring during suppression tests
aa05f673a683 drm/i915/execlists: Avoid sync calls during park

== Logs ==

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

Re: [PATCH] drm/i915/execlists: Lift process_csb() out of the irq-off spinlock

[kbuild test robot]

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

Hi Chris,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on drm-intel/for-linux-next]
[cannot apply to v5.3-rc4]
[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-execlists-Lift-process_csb-out-of-the-irq-off-spinlock/20190812-211057
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: i386-defconfig (attached as .config)
compiler: gcc-7 (Debian 7.4.0-10) 7.4.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c: In function 'schedule_in':
>> drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c:541:2: error: implicit declaration of function 'intel_context_inflight_inc'; did you mean 'intel_context_inflight_count'? [-Werror=implicit-function-declaration]
     intel_context_inflight_inc(rq->hw_context);
     ^~~~~~~~~~~~~~~~~~~~~~~~~~
     intel_context_inflight_count
   drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c: In function 'schedule_out':
>> drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c:551:2: error: implicit declaration of function 'intel_context_inflight_dec'; did you mean 'intel_context_inflight'? [-Werror=implicit-function-declaration]
     intel_context_inflight_dec(rq->hw_context);
     ^~~~~~~~~~~~~~~~~~~~~~~~~~
     intel_context_inflight
   cc1: some warnings being treated as errors

vim +541 drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c

77f0d0e925e8a0 drivers/gpu/drm/i915/i915_guc_submission.c        Chris Wilson 2017-05-17  534  
22b7a426bbe1eb drivers/gpu/drm/i915/intel_guc_submission.c       Chris Wilson 2019-06-20  535  static struct i915_request *schedule_in(struct i915_request *rq, int idx)
2a694feb93556e drivers/gpu/drm/i915/intel_guc_submission.c       Chris Wilson 2018-04-03  536  {
22b7a426bbe1eb drivers/gpu/drm/i915/intel_guc_submission.c       Chris Wilson 2019-06-20  537  	trace_i915_request_in(rq, idx);
22b7a426bbe1eb drivers/gpu/drm/i915/intel_guc_submission.c       Chris Wilson 2019-06-20  538  
22b7a426bbe1eb drivers/gpu/drm/i915/intel_guc_submission.c       Chris Wilson 2019-06-20  539  	if (!rq->hw_context->inflight)
22b7a426bbe1eb drivers/gpu/drm/i915/intel_guc_submission.c       Chris Wilson 2019-06-20  540  		rq->hw_context->inflight = rq->engine;
22b7a426bbe1eb drivers/gpu/drm/i915/intel_guc_submission.c       Chris Wilson 2019-06-20 @541  	intel_context_inflight_inc(rq->hw_context);
c7302f204490f3 drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c Chris Wilson 2019-08-08  542  	intel_gt_pm_get(rq->engine->gt);
22b7a426bbe1eb drivers/gpu/drm/i915/intel_guc_submission.c       Chris Wilson 2019-06-20  543  
22b7a426bbe1eb drivers/gpu/drm/i915/intel_guc_submission.c       Chris Wilson 2019-06-20  544  	return i915_request_get(rq);
2a694feb93556e drivers/gpu/drm/i915/intel_guc_submission.c       Chris Wilson 2018-04-03  545  }
2a694feb93556e drivers/gpu/drm/i915/intel_guc_submission.c       Chris Wilson 2018-04-03  546  
22b7a426bbe1eb drivers/gpu/drm/i915/intel_guc_submission.c       Chris Wilson 2019-06-20  547  static void schedule_out(struct i915_request *rq)
2a694feb93556e drivers/gpu/drm/i915/intel_guc_submission.c       Chris Wilson 2018-04-03  548  {
22b7a426bbe1eb drivers/gpu/drm/i915/intel_guc_submission.c       Chris Wilson 2019-06-20  549  	trace_i915_request_out(rq);
22b7a426bbe1eb drivers/gpu/drm/i915/intel_guc_submission.c       Chris Wilson 2019-06-20  550  
22b7a426bbe1eb drivers/gpu/drm/i915/intel_guc_submission.c       Chris Wilson 2019-06-20 @551  	intel_context_inflight_dec(rq->hw_context);
22b7a426bbe1eb drivers/gpu/drm/i915/intel_guc_submission.c       Chris Wilson 2019-06-20  552  	if (!intel_context_inflight_count(rq->hw_context))
22b7a426bbe1eb drivers/gpu/drm/i915/intel_guc_submission.c       Chris Wilson 2019-06-20  553  		rq->hw_context->inflight = NULL;
22b7a426bbe1eb drivers/gpu/drm/i915/intel_guc_submission.c       Chris Wilson 2019-06-20  554  
c7302f204490f3 drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c Chris Wilson 2019-08-08  555  	intel_gt_pm_put(rq->engine->gt);
22b7a426bbe1eb drivers/gpu/drm/i915/intel_guc_submission.c       Chris Wilson 2019-06-20  556  	i915_request_put(rq);
2a694feb93556e drivers/gpu/drm/i915/intel_guc_submission.c       Chris Wilson 2018-04-03  557  }
2a694feb93556e drivers/gpu/drm/i915/intel_guc_submission.c       Chris Wilson 2018-04-03  558  

:::::: The code at line 541 was first introduced by commit
:::::: 22b7a426bbe1ebe1520f92da4cd1617d1e1b5fc4 drm/i915/execlists: Preempt-to-busy

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

---
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: 28082 bytes --]

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

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

✓ Fi.CI.IGT: success for series starting with [1/8] drm/i915/execlists: Avoid sync calls during park (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [1/8] drm/i915/execlists: Avoid sync calls during park (rev2)
URL   : https://patchwork.freedesktop.org/series/65080/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6685_full -> Patchwork_13984_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_isolation@vecs0-s3:
    - shard-apl:          [PASS][1] -> [DMESG-WARN][2] ([fdo#108566]) +4 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6685/shard-apl4/igt@gem_ctx_isolation@vecs0-s3.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13984/shard-apl8/igt@gem_ctx_isolation@vecs0-s3.html

  * igt@gem_exec_schedule@wide-bsd:
    - shard-iclb:         [PASS][3] -> [SKIP][4] ([fdo#111325]) +2 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6685/shard-iclb6/igt@gem_exec_schedule@wide-bsd.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13984/shard-iclb2/igt@gem_exec_schedule@wide-bsd.html

  * igt@i915_pm_rpm@system-suspend:
    - shard-skl:          [PASS][5] -> [INCOMPLETE][6] ([fdo#104108] / [fdo#107807])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6685/shard-skl8/igt@i915_pm_rpm@system-suspend.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13984/shard-skl7/igt@i915_pm_rpm@system-suspend.html

  * igt@kms_cursor_crc@pipe-c-cursor-128x128-sliding:
    - shard-apl:          [PASS][7] -> [INCOMPLETE][8] ([fdo#103927]) +2 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6685/shard-apl3/igt@kms_cursor_crc@pipe-c-cursor-128x128-sliding.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13984/shard-apl6/igt@kms_cursor_crc@pipe-c-cursor-128x128-sliding.html

  * igt@kms_flip@dpms-vs-vblank-race-interruptible:
    - shard-glk:          [PASS][9] -> [FAIL][10] ([fdo#103060])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6685/shard-glk8/igt@kms_flip@dpms-vs-vblank-race-interruptible.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13984/shard-glk3/igt@kms_flip@dpms-vs-vblank-race-interruptible.html

  * igt@kms_flip@flip-vs-panning-vs-hang:
    - shard-hsw:          [PASS][11] -> [INCOMPLETE][12] ([fdo#103540])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6685/shard-hsw5/igt@kms_flip@flip-vs-panning-vs-hang.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13984/shard-hsw2/igt@kms_flip@flip-vs-panning-vs-hang.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render:
    - shard-iclb:         [PASS][13] -> [FAIL][14] ([fdo#103167]) +5 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6685/shard-iclb2/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13984/shard-iclb3/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render.html

  * igt@kms_frontbuffer_tracking@fbc-1p-rte:
    - shard-iclb:         [PASS][15] -> [FAIL][16] ([fdo#103167] / [fdo#110378])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6685/shard-iclb6/igt@kms_frontbuffer_tracking@fbc-1p-rte.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13984/shard-iclb6/igt@kms_frontbuffer_tracking@fbc-1p-rte.html

  * igt@kms_frontbuffer_tracking@psr-suspend:
    - shard-skl:          [PASS][17] -> [INCOMPLETE][18] ([fdo#104108] / [fdo#106978])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6685/shard-skl8/igt@kms_frontbuffer_tracking@psr-suspend.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13984/shard-skl10/igt@kms_frontbuffer_tracking@psr-suspend.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
    - shard-skl:          [PASS][19] -> [INCOMPLETE][20] ([fdo#104108])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6685/shard-skl6/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13984/shard-skl1/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          [PASS][21] -> [FAIL][22] ([fdo#108145] / [fdo#110403])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6685/shard-skl7/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13984/shard-skl5/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html

  * igt@kms_vblank@pipe-b-ts-continuation-suspend:
    - shard-kbl:          [PASS][23] -> [INCOMPLETE][24] ([fdo#103665])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6685/shard-kbl7/igt@kms_vblank@pipe-b-ts-continuation-suspend.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13984/shard-kbl4/igt@kms_vblank@pipe-b-ts-continuation-suspend.html

  * igt@prime_vgem@fence-wait-bsd2:
    - shard-iclb:         [PASS][25] -> [SKIP][26] ([fdo#109276]) +17 similar issues
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6685/shard-iclb1/igt@prime_vgem@fence-wait-bsd2.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13984/shard-iclb5/igt@prime_vgem@fence-wait-bsd2.html

  
#### Possible fixes ####

  * igt@gem_ctx_shared@exec-single-timeline-bsd:
    - shard-iclb:         [SKIP][27] ([fdo#110841]) -> [PASS][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6685/shard-iclb1/igt@gem_ctx_shared@exec-single-timeline-bsd.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13984/shard-iclb3/igt@gem_ctx_shared@exec-single-timeline-bsd.html

  * igt@gem_exec_blt@normal:
    - shard-apl:          [INCOMPLETE][29] ([fdo#103927]) -> [PASS][30] +1 similar issue
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6685/shard-apl3/igt@gem_exec_blt@normal.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13984/shard-apl7/igt@gem_exec_blt@normal.html

  * igt@gem_exec_schedule@preempt-bsd:
    - shard-iclb:         [SKIP][31] ([fdo#111325]) -> [PASS][32] +2 similar issues
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6685/shard-iclb4/igt@gem_exec_schedule@preempt-bsd.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13984/shard-iclb8/igt@gem_exec_schedule@preempt-bsd.html

  * igt@gem_softpin@noreloc-s3:
    - shard-apl:          [DMESG-WARN][33] ([fdo#108566]) -> [PASS][34] +5 similar issues
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6685/shard-apl3/igt@gem_softpin@noreloc-s3.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13984/shard-apl4/igt@gem_softpin@noreloc-s3.html

  * igt@i915_suspend@debugfs-reader:
    - shard-skl:          [INCOMPLETE][35] ([fdo#104108]) -> [PASS][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6685/shard-skl9/igt@i915_suspend@debugfs-reader.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13984/shard-skl8/igt@i915_suspend@debugfs-reader.html

  * igt@kms_flip@flip-vs-suspend:
    - shard-skl:          [INCOMPLETE][37] ([fdo#109507]) -> [PASS][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6685/shard-skl3/igt@kms_flip@flip-vs-suspend.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13984/shard-skl10/igt@kms_flip@flip-vs-suspend.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-render:
    - shard-iclb:         [FAIL][39] ([fdo#103167]) -> [PASS][40] +1 similar issue
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6685/shard-iclb2/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-render.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13984/shard-iclb2/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-render.html

  * igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min:
    - shard-skl:          [FAIL][41] ([fdo#108145]) -> [PASS][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6685/shard-skl3/igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13984/shard-skl3/igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min.html

  * igt@kms_psr@psr2_cursor_render:
    - shard-iclb:         [SKIP][43] ([fdo#109441]) -> [PASS][44] +2 similar issues
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6685/shard-iclb6/igt@kms_psr@psr2_cursor_render.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13984/shard-iclb2/igt@kms_psr@psr2_cursor_render.html

  * igt@perf_pmu@rc6-runtime-pm-long:
    - shard-apl:          [FAIL][45] ([fdo#105010]) -> [PASS][46]
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6685/shard-apl5/igt@perf_pmu@rc6-runtime-pm-long.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13984/shard-apl6/igt@perf_pmu@rc6-runtime-pm-long.html

  * igt@prime_busy@hang-bsd2:
    - shard-iclb:         [SKIP][47] ([fdo#109276]) -> [PASS][48] +17 similar issues
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6685/shard-iclb8/igt@prime_busy@hang-bsd2.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13984/shard-iclb1/igt@prime_busy@hang-bsd2.html

  
#### Warnings ####

  * igt@gem_ctx_isolation@vcs1-nonpriv:
    - shard-iclb:         [FAIL][49] ([fdo#111329]) -> [SKIP][50] ([fdo#109276])
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6685/shard-iclb2/igt@gem_ctx_isolation@vcs1-nonpriv.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13984/shard-iclb3/igt@gem_ctx_isolation@vcs1-nonpriv.html

  * igt@gem_mocs_settings@mocs-reset-bsd2:
    - shard-iclb:         [FAIL][51] ([fdo#111330]) -> [SKIP][52] ([fdo#109276])
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6685/shard-iclb2/igt@gem_mocs_settings@mocs-reset-bsd2.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13984/shard-iclb3/igt@gem_mocs_settings@mocs-reset-bsd2.html

  * igt@gem_mocs_settings@mocs-settings-bsd2:
    - shard-iclb:         [SKIP][53] ([fdo#109276]) -> [FAIL][54] ([fdo#111330])
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6685/shard-iclb7/igt@gem_mocs_settings@mocs-settings-bsd2.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13984/shard-iclb4/igt@gem_mocs_settings@mocs-settings-bsd2.html

  
  [fdo#103060]: https://bugs.freedesktop.org/show_bug.cgi?id=103060
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#105010]: https://bugs.freedesktop.org/show_bug.cgi?id=105010
  [fdo#106978]: https://bugs.freedesktop.org/show_bug.cgi?id=106978
  [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109507]: https://bugs.freedesktop.org/show_bug.cgi?id=109507
  [fdo#110378]: https://bugs.freedesktop.org/show_bug.cgi?id=110378
  [fdo#110403]: https://bugs.freedesktop.org/show_bug.cgi?id=110403
  [fdo#110841]: https://bugs.freedesktop.org/show_bug.cgi?id=110841
  [fdo#111325]: https://bugs.freedesktop.org/show_bug.cgi?id=111325
  [fdo#111329]: https://bugs.freedesktop.org/show_bug.cgi?id=111329
  [fdo#111330]: https://bugs.freedesktop.org/show_bug.cgi?id=111330


Participating hosts (10 -> 10)
------------------------------

  No changes in participating hosts


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_6685 -> Patchwork_13984

  CI-20190529: 20190529
  CI_DRM_6685: acabc817e999dd7a158654fb207f7e61d68295f9 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5127: f43f5fa12ac1b93febfe3eeb9e9985f5f3e2eff0 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13984: 78d72ec3911b38f273d4d782fa15bf43f2ad72af @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

Re: [PATCH 6/8] drm/i915/guc: Keep the engine awake until the tasklet is idle

[Daniele Ceraolo Spurio]

On 8/12/19 3:44 AM, Chris Wilson wrote:
> Quoting Chris Wilson (2019-08-12 10:10:43)
>> For the guc, we need to keep the engine awake (and not parked) and not
>> just the gt. If we let the engine park, we disable the irq and stop
>> processing the tasklet, leaving state outstanding inside the tasklet.
>>
>> The downside is, of course, we now have to wait until the tasklet is run
>> before we consider the engine idle.
> 
> Fwiw, because of this I think it may be preferable to keep to using GT
> pm for the tasklet; and apply Daniele's patch to keep
> NEEDS_BREADCRUMB_TASKLET set (which is the right thing to do anyway now
> that we stop switching between submission modes).
> -Chris
> 

Given that the GuC submission code is about to undergo a rework I 
believe it'd be better to keep the fix contained to the GuC side of 
things for now and avoid impacting the more general request paths (i.e. 
patch 4 in this series, unless you still want that for other reasons). 
I'll clean up and send the other patch.

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

Re: [PATCH 6/8] drm/i915/guc: Keep the engine awake until the tasklet is idle

[Chris Wilson]

Quoting Daniele Ceraolo Spurio (2019-08-12 21:38:39)
> 
> 
> On 8/12/19 3:44 AM, Chris Wilson wrote:
> > Quoting Chris Wilson (2019-08-12 10:10:43)
> >> For the guc, we need to keep the engine awake (and not parked) and not
> >> just the gt. If we let the engine park, we disable the irq and stop
> >> processing the tasklet, leaving state outstanding inside the tasklet.
> >>
> >> The downside is, of course, we now have to wait until the tasklet is run
> >> before we consider the engine idle.
> > 
> > Fwiw, because of this I think it may be preferable to keep to using GT
> > pm for the tasklet; and apply Daniele's patch to keep
> > NEEDS_BREADCRUMB_TASKLET set (which is the right thing to do anyway now
> > that we stop switching between submission modes).
> > -Chris
> > 
> 
> Given that the GuC submission code is about to undergo a rework I 
> believe it'd be better to keep the fix contained to the GuC side of 
> things for now and avoid impacting the more general request paths (i.e. 
> patch 4 in this series, unless you still want that for other reasons). 
> I'll clean up and send the other patch.

Oh, we need that anyway :)
https://bugs.freedesktop.org/show_bug.cgi?id=111378

And it actually clarified some of the heartbeat code, so it's an
eventual win.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm/i915/execlists: Lift process_csb() out of the irq-off spinlock

[Chris Wilson]

If we only call process_csb() from the tasklet, though we lose the
ability to bypass ksoftirqd interrupt processing on direct submission
paths, we can push it out of the irq-off spinlock.

The penalty is that we then allow schedule_out to be called concurrently
with schedule_in requiring us to handle the usage count (baked into the
pointer itself) atomically.

As we do kick the tasklets (via local_bh_enable()) after our submission,
there is a possibility there to see if we can pull the local softirq
processing back from the ksoftirqd.

v2: Store the 'switch_priority_hint' on submission, so that we can
safely check during process_csb().

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 drivers/gpu/drm/i915/gt/intel_context_types.h |   4 +-
 drivers/gpu/drm/i915/gt/intel_engine_cs.c     |   2 +-
 drivers/gpu/drm/i915/gt/intel_engine_types.h  |  10 ++
 drivers/gpu/drm/i915/gt/intel_lrc.c           | 136 +++++++++++-------
 drivers/gpu/drm/i915/i915_utils.h             |  20 ++-
 5 files changed, 108 insertions(+), 64 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h b/drivers/gpu/drm/i915/gt/intel_context_types.h
index a632b20ec4d8..d8ce266c049f 100644
--- a/drivers/gpu/drm/i915/gt/intel_context_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_context_types.h
@@ -41,9 +41,7 @@ struct intel_context {
 	struct intel_engine_cs *engine;
 	struct intel_engine_cs *inflight;
 #define intel_context_inflight(ce) ptr_mask_bits((ce)->inflight, 2)
-#define intel_context_inflight_count(ce)  ptr_unmask_bits((ce)->inflight, 2)
-#define intel_context_inflight_inc(ce) ptr_count_inc(&(ce)->inflight)
-#define intel_context_inflight_dec(ce) ptr_count_dec(&(ce)->inflight)
+#define intel_context_inflight_count(ce) ptr_unmask_bits((ce)->inflight, 2)
 
 	struct i915_address_space *vm;
 	struct i915_gem_context *gem_context;
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 957f27a2ec97..ba457c1c7dc0 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -1459,7 +1459,7 @@ int intel_enable_engine_stats(struct intel_engine_cs *engine)
 
 		for (port = execlists->pending; (rq = *port); port++) {
 			/* Exclude any contexts already counted in active */
-			if (intel_context_inflight_count(rq->hw_context) == 1)
+			if (!intel_context_inflight_count(rq->hw_context))
 				engine->stats.active++;
 		}
 
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
index 9965a32601d6..5441aa9cb863 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
@@ -204,6 +204,16 @@ struct intel_engine_execlists {
 	 */
 	unsigned int port_mask;
 
+	/**
+	 * @switch_priority_hint: Second context priority.
+	 *
+	 * We submit multiple contexts to the HW simultaneously and would
+	 * like to occasionally switch between them to emulate timeslicing.
+	 * To know when timeslicing is suitable, we track the priority of
+	 * the context submitted second.
+	 */
+	int switch_priority_hint;
+
 	/**
 	 * @queue_priority_hint: Highest pending priority.
 	 *
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index e9863f4d826b..2978cf16fb9b 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -547,27 +547,39 @@ execlists_context_status_change(struct i915_request *rq, unsigned long status)
 				   status, rq);
 }
 
+static inline struct intel_engine_cs *
+__execlists_schedule_in(struct i915_request *rq)
+{
+	struct intel_engine_cs * const engine = rq->engine;
+	struct intel_context * const ce = rq->hw_context;
+
+	intel_context_get(ce);
+
+	intel_gt_pm_get(engine->gt);
+	execlists_context_status_change(rq, INTEL_CONTEXT_SCHEDULE_IN);
+	intel_engine_context_in(engine);
+
+	return engine;
+}
+
 static inline struct i915_request *
 execlists_schedule_in(struct i915_request *rq, int idx)
 {
-	struct intel_context *ce = rq->hw_context;
-	int count;
+	struct intel_context * const ce = rq->hw_context;
+	struct intel_engine_cs *old;
 
+	GEM_BUG_ON(!intel_engine_pm_is_awake(rq->engine));
 	trace_i915_request_in(rq, idx);
 
-	count = intel_context_inflight_count(ce);
-	if (!count) {
-		intel_context_get(ce);
-		ce->inflight = rq->engine;
-
-		intel_gt_pm_get(ce->inflight->gt);
-		execlists_context_status_change(rq, INTEL_CONTEXT_SCHEDULE_IN);
-		intel_engine_context_in(ce->inflight);
-	}
+	old = READ_ONCE(ce->inflight);
+	do {
+		if (!old) {
+			WRITE_ONCE(ce->inflight, __execlists_schedule_in(rq));
+			break;
+		}
+	} while (!try_cmpxchg(&ce->inflight, &old, ptr_inc(old)));
 
-	intel_context_inflight_inc(ce);
 	GEM_BUG_ON(intel_context_inflight(ce) != rq->engine);
-
 	return i915_request_get(rq);
 }
 
@@ -581,35 +593,45 @@ static void kick_siblings(struct i915_request *rq, struct intel_context *ce)
 }
 
 static inline void
-execlists_schedule_out(struct i915_request *rq)
+__execlists_schedule_out(struct i915_request *rq,
+			 struct intel_engine_cs * const engine)
 {
-	struct intel_context *ce = rq->hw_context;
+	struct intel_context * const ce = rq->hw_context;
 
-	GEM_BUG_ON(!intel_context_inflight_count(ce));
+	intel_engine_context_out(engine);
+	execlists_context_status_change(rq, INTEL_CONTEXT_SCHEDULE_OUT);
+	intel_gt_pm_put(engine->gt);
 
-	trace_i915_request_out(rq);
+	/*
+	 * If this is part of a virtual engine, its next request may
+	 * have been blocked waiting for access to the active context.
+	 * We have to kick all the siblings again in case we need to
+	 * switch (e.g. the next request is not runnable on this
+	 * engine). Hopefully, we will already have submitted the next
+	 * request before the tasklet runs and do not need to rebuild
+	 * each virtual tree and kick everyone again.
+	 */
+	if (ce->engine != engine)
+		kick_siblings(rq, ce);
 
-	intel_context_inflight_dec(ce);
-	if (!intel_context_inflight_count(ce)) {
-		intel_engine_context_out(ce->inflight);
-		execlists_context_status_change(rq, INTEL_CONTEXT_SCHEDULE_OUT);
-		intel_gt_pm_put(ce->inflight->gt);
+	intel_context_put(ce);
+}
 
-		/*
-		 * If this is part of a virtual engine, its next request may
-		 * have been blocked waiting for access to the active context.
-		 * We have to kick all the siblings again in case we need to
-		 * switch (e.g. the next request is not runnable on this
-		 * engine). Hopefully, we will already have submitted the next
-		 * request before the tasklet runs and do not need to rebuild
-		 * each virtual tree and kick everyone again.
-		 */
-		ce->inflight = NULL;
-		if (rq->engine != ce->engine)
-			kick_siblings(rq, ce);
+static inline void
+execlists_schedule_out(struct i915_request *rq)
+{
+	struct intel_context * const ce = rq->hw_context;
+	struct intel_engine_cs *cur, *old;
 
-		intel_context_put(ce);
-	}
+	trace_i915_request_out(rq);
+	GEM_BUG_ON(intel_context_inflight(ce) != rq->engine);
+
+	old = READ_ONCE(ce->inflight);
+	do
+		cur = ptr_unmask_bits(old, 2) ? ptr_dec(old) : NULL;
+	while (!try_cmpxchg(&ce->inflight, &old, cur));
+	if (!cur)
+		__execlists_schedule_out(rq, old);
 
 	i915_request_put(rq);
 }
@@ -684,6 +706,9 @@ assert_pending_valid(const struct intel_engine_execlists *execlists,
 
 	trace_ports(execlists, msg, execlists->pending);
 
+	if (!execlists->pending[0])
+		return false;
+
 	if (execlists->pending[execlists_num_ports(execlists)])
 		return false;
 
@@ -941,12 +966,24 @@ need_timeslice(struct intel_engine_cs *engine, const struct i915_request *rq)
 	return hint >= effective_prio(rq);
 }
 
+static int
+switch_prio(struct intel_engine_cs *engine, const struct i915_request *rq)
+{
+	if (list_is_last(&rq->sched.link, &engine->active.requests))
+		return INT_MIN;
+
+	return rq_prio(list_next_entry(rq, sched.link));
+}
+
 static bool
-enable_timeslice(struct intel_engine_cs *engine)
+enable_timeslice(const struct intel_engine_execlists *execlists)
 {
-	struct i915_request *last = last_active(&engine->execlists);
+	const struct i915_request *rq = *execlists->active;
 
-	return last && need_timeslice(engine, last);
+	if (i915_request_completed(rq))
+		return false;
+
+	return execlists->switch_priority_hint >= effective_prio(rq);
 }
 
 static void record_preemption(struct intel_engine_execlists *execlists)
@@ -1292,6 +1329,8 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
 		*port = execlists_schedule_in(last, port - execlists->pending);
 		memset(port + 1, 0, (last_port - port) * sizeof(*port));
 		execlists_submit_ports(engine);
+		execlists->switch_priority_hint =
+			switch_prio(engine, *execlists->pending);
 	} else {
 		ring_set_paused(engine, 0);
 	}
@@ -1356,7 +1395,6 @@ static void process_csb(struct intel_engine_cs *engine)
 	const u8 num_entries = execlists->csb_size;
 	u8 head, tail;
 
-	lockdep_assert_held(&engine->active.lock);
 	GEM_BUG_ON(USES_GUC_SUBMISSION(engine->i915));
 
 	/*
@@ -1427,15 +1465,14 @@ static void process_csb(struct intel_engine_cs *engine)
 				       execlists->pending,
 				       execlists_num_ports(execlists) *
 				       sizeof(*execlists->pending));
-			execlists->pending[0] = NULL;
 
-			trace_ports(execlists, "promoted", execlists->active);
-
-			if (enable_timeslice(engine))
+			if (enable_timeslice(execlists))
 				mod_timer(&execlists->timer, jiffies + 1);
 
 			if (!inject_preempt_hang(execlists))
 				ring_set_paused(engine, 0);
+
+			WRITE_ONCE(execlists->pending[0], NULL);
 			break;
 
 		case CSB_COMPLETE: /* port0 completed, advanced to port1 */
@@ -1479,8 +1516,6 @@ static void process_csb(struct intel_engine_cs *engine)
 static void __execlists_submission_tasklet(struct intel_engine_cs *const engine)
 {
 	lockdep_assert_held(&engine->active.lock);
-
-	process_csb(engine);
 	if (!engine->execlists.pending[0])
 		execlists_dequeue(engine);
 }
@@ -1494,9 +1529,12 @@ static void execlists_submission_tasklet(unsigned long data)
 	struct intel_engine_cs * const engine = (struct intel_engine_cs *)data;
 	unsigned long flags;
 
-	spin_lock_irqsave(&engine->active.lock, flags);
-	__execlists_submission_tasklet(engine);
-	spin_unlock_irqrestore(&engine->active.lock, flags);
+	process_csb(engine);
+	if (!READ_ONCE(engine->execlists.pending[0])) {
+		spin_lock_irqsave(&engine->active.lock, flags);
+		__execlists_submission_tasklet(engine);
+		spin_unlock_irqrestore(&engine->active.lock, flags);
+	}
 }
 
 static void execlists_submission_timer(struct timer_list *timer)
diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h
index d652ba5d2320..562f756da421 100644
--- a/drivers/gpu/drm/i915/i915_utils.h
+++ b/drivers/gpu/drm/i915/i915_utils.h
@@ -161,17 +161,15 @@ __check_struct_size(size_t base, size_t arr, size_t count, size_t *size)
 	((typeof(ptr))((unsigned long)(ptr) | __bits));			\
 })
 
-#define ptr_count_dec(p_ptr) do {					\
-	typeof(p_ptr) __p = (p_ptr);					\
-	unsigned long __v = (unsigned long)(*__p);			\
-	*__p = (typeof(*p_ptr))(--__v);					\
-} while (0)
-
-#define ptr_count_inc(p_ptr) do {					\
-	typeof(p_ptr) __p = (p_ptr);					\
-	unsigned long __v = (unsigned long)(*__p);			\
-	*__p = (typeof(*p_ptr))(++__v);					\
-} while (0)
+#define ptr_dec(ptr) ({							\
+	unsigned long __v = (unsigned long)(ptr);			\
+	(typeof(ptr))(__v - 1);						\
+})
+
+#define ptr_inc(ptr) ({							\
+	unsigned long __v = (unsigned long)(ptr);			\
+	(typeof(ptr))(__v + 1);						\
+})
 
 #define page_mask_bits(ptr) ptr_mask_bits(ptr, PAGE_SHIFT)
 #define page_unmask_bits(ptr) ptr_unmask_bits(ptr, PAGE_SHIFT)
-- 
2.23.0.rc1

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