[PATCH 16/46] drm/i915: Move common active lists from engine to i915_scheduler

Chris Wilson chris at chris-wilson.co.uk
Sat Jan 30 12:52:07 UTC 2021


Extract the scheduler lists into a related structure, stop sprawling
over struct intel_engine_cs. Also transfer the responsibility of tracing
from ENGINE_TRACE() to SCHED_TRACE().

Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gt/intel_engine_cs.c     | 30 ++------
 drivers/gpu/drm/i915/gt/intel_engine_types.h  |  8 +--
 .../drm/i915/gt/intel_execlists_submission.c  |  7 +-
 drivers/gpu/drm/i915/gt/mock_engine.c         |  6 +-
 drivers/gpu/drm/i915/i915_request.h           |  8 ++-
 drivers/gpu/drm/i915/i915_scheduler.c         | 72 +++++++++++++++----
 drivers/gpu/drm/i915/i915_scheduler.h         | 13 +++-
 drivers/gpu/drm/i915/i915_scheduler_types.h   | 18 +++++
 .../gpu/drm/i915/selftests/i915_scheduler.c   |  1 +
 9 files changed, 111 insertions(+), 52 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 1b7661386dee..7c627e8d54d1 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -576,8 +576,6 @@ void intel_engine_init_execlists(struct intel_engine_cs *engine)
 
 	execlists->queue_priority_hint = INT_MIN;
 	execlists->queue = RB_ROOT_CACHED;
-
-	i915_sched_init_ipi(&execlists->ipi);
 }
 
 static void cleanup_status_page(struct intel_engine_cs *engine)
@@ -693,7 +691,11 @@ static int engine_setup_common(struct intel_engine_cs *engine)
 		goto err_status;
 	}
 
-	intel_engine_init_active(engine, ENGINE_PHYSICAL);
+	i915_sched_init_engine(&engine->active,
+			       engine->i915->drm.dev,
+			       engine->name,
+			       engine->mask,
+			       ENGINE_PHYSICAL);
 	intel_engine_init_execlists(engine);
 	intel_engine_init_cmd_parser(engine);
 	intel_engine_init__pm(engine);
@@ -761,28 +763,6 @@ static int measure_breadcrumb_dw(struct intel_context *ce)
 	return dw;
 }
 
-void
-intel_engine_init_active(struct intel_engine_cs *engine, unsigned int subclass)
-{
-	INIT_LIST_HEAD(&engine->active.requests);
-	INIT_LIST_HEAD(&engine->active.hold);
-
-	spin_lock_init(&engine->active.lock);
-	lockdep_set_subclass(&engine->active.lock, subclass);
-
-	/*
-	 * Due to an interesting quirk in lockdep's internal debug tracking,
-	 * after setting a subclass we must ensure the lock is used. Otherwise,
-	 * nr_unused_locks is incremented once too often.
-	 */
-#ifdef CONFIG_DEBUG_LOCK_ALLOC
-	local_irq_disable();
-	lock_map_acquire(&engine->active.lock.dep_map);
-	lock_map_release(&engine->active.lock.dep_map);
-	local_irq_enable();
-#endif
-}
-
 static struct intel_context *
 create_pinned_context(struct intel_engine_cs *engine,
 		      unsigned int hwsp,
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
index 9105b7769635..36bcd85cc73b 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
@@ -258,8 +258,6 @@ struct intel_engine_execlists {
 	struct rb_root_cached queue;
 	struct rb_root_cached virtual;
 
-	struct i915_sched_ipi ipi;
-
 	/**
 	 * @csb_write: control register for Context Switch buffer
 	 *
@@ -329,11 +327,7 @@ struct intel_engine_cs {
 
 	struct intel_sseu sseu;
 
-	struct {
-		spinlock_t lock;
-		struct list_head requests;
-		struct list_head hold; /* ready requests, but on hold */
-	} active;
+	struct i915_sched_engine active;
 
 	/* keep a request in reserve for a [pm] barrier under oom */
 	struct i915_request *request_pool;
diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
index 195d7a21b4cd..21a5375af399 100644
--- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
+++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
@@ -3347,7 +3347,6 @@ intel_execlists_create_virtual(struct intel_engine_cs **siblings,
 
 	snprintf(ve->base.name, sizeof(ve->base.name), "virtual");
 
-	intel_engine_init_active(&ve->base, ENGINE_VIRTUAL);
 	intel_engine_init_execlists(&ve->base);
 
 	ve->base.cops = &virtual_context_ops;
@@ -3433,6 +3432,12 @@ intel_execlists_create_virtual(struct intel_engine_cs **siblings,
 
 	ve->base.flags |= I915_ENGINE_IS_VIRTUAL;
 
+	i915_sched_init_engine(&ve->base.active,
+			       ve->base.i915->drm.dev,
+			       ve->base.name,
+			       ve->base.mask,
+			       ENGINE_VIRTUAL);
+
 	virtual_engine_initial_hint(ve);
 	return &ve->context;
 
diff --git a/drivers/gpu/drm/i915/gt/mock_engine.c b/drivers/gpu/drm/i915/gt/mock_engine.c
index cf1269e74998..350191479c30 100644
--- a/drivers/gpu/drm/i915/gt/mock_engine.c
+++ b/drivers/gpu/drm/i915/gt/mock_engine.c
@@ -327,7 +327,11 @@ int mock_engine_init(struct intel_engine_cs *engine)
 {
 	struct intel_context *ce;
 
-	intel_engine_init_active(engine, ENGINE_MOCK);
+	i915_sched_init_engine(&engine->active,
+			       engine->i915->drm.dev,
+			       engine->name,
+			       engine->mask,
+			       ENGINE_MOCK);
 	intel_engine_init_execlists(engine);
 	intel_engine_init__pm(engine);
 	intel_engine_init_retire(engine);
diff --git a/drivers/gpu/drm/i915/i915_request.h b/drivers/gpu/drm/i915/i915_request.h
index c0bd4cb8786a..a8dd7120e182 100644
--- a/drivers/gpu/drm/i915/i915_request.h
+++ b/drivers/gpu/drm/i915/i915_request.h
@@ -51,11 +51,13 @@ struct i915_capture_list {
 	struct i915_vma *vma;
 };
 
+#define RQ_FMT "%llx:%lld"
+#define RQ_ARG(rq) (rq) ? (rq)->fence.context : 0, (rq) ? (rq)->fence.seqno : 0
+
 #define RQ_TRACE(rq, fmt, ...) do {					\
 	const struct i915_request *rq__ = (rq);				\
-	ENGINE_TRACE(rq__->engine, "fence %llx:%lld, current %d " fmt,	\
-		     rq__->fence.context, rq__->fence.seqno,		\
-		     hwsp_seqno(rq__), ##__VA_ARGS__);			\
+	ENGINE_TRACE(rq__->engine, "fence " RQ_FMT ", current %d " fmt,	\
+		     RQ_ARG(rq__), hwsp_seqno(rq__), ##__VA_ARGS__);	\
 } while (0)
 
 enum {
diff --git a/drivers/gpu/drm/i915/i915_scheduler.c b/drivers/gpu/drm/i915/i915_scheduler.c
index 694ca3a3b563..0a86f16f3cc8 100644
--- a/drivers/gpu/drm/i915/i915_scheduler.c
+++ b/drivers/gpu/drm/i915/i915_scheduler.c
@@ -85,12 +85,46 @@ static void ipi_schedule(struct work_struct *wrk)
 	} while (rq);
 }
 
-void i915_sched_init_ipi(struct i915_sched_ipi *ipi)
+static void i915_sched_init_ipi(struct i915_sched_ipi *ipi)
 {
 	INIT_WORK(&ipi->work, ipi_schedule);
 	ipi->list = NULL;
 }
 
+void i915_sched_init_engine(struct i915_sched_engine *se,
+			    struct device *dev,
+			    const char *name,
+			    unsigned long mask,
+			    unsigned int subclass)
+{
+	spin_lock_init(&se->lock);
+	lockdep_set_subclass(&se->lock, subclass);
+
+#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM)
+	se->dbg.dev = dev;
+	se->dbg.name = name;
+#endif
+
+	se->mask = mask;
+
+	INIT_LIST_HEAD(&se->requests);
+	INIT_LIST_HEAD(&se->hold);
+
+	i915_sched_init_ipi(&se->ipi);
+
+	/*
+	 * Due to an interesting quirk in lockdep's internal debug tracking,
+	 * after setting a subclass we must ensure the lock is used. Otherwise,
+	 * nr_unused_locks is incremented once too often.
+	 */
+#ifdef CONFIG_DEBUG_LOCK_ALLOC
+	local_irq_disable();
+	lock_map_acquire(&se->lock.dep_map);
+	lock_map_release(&se->lock.dep_map);
+	local_irq_enable();
+#endif
+}
+
 static void __ipi_add(struct i915_request *rq)
 {
 #define STUB ((struct i915_request *)1)
@@ -114,13 +148,13 @@ static void __ipi_add(struct i915_request *rq)
 	}
 
 	/* Carefully insert ourselves into the head of the llist */
-	first = READ_ONCE(engine->execlists.ipi.list);
+	first = READ_ONCE(engine->active.ipi.list);
 	do {
 		rq->sched.ipi_link = ptr_pack_bits(first, 1, 1);
-	} while (!try_cmpxchg(&engine->execlists.ipi.list, &first, rq));
+	} while (!try_cmpxchg(&engine->active.ipi.list, &first, rq));
 
 	if (!first)
-		queue_work(system_unbound_wq, &engine->execlists.ipi.work);
+		queue_work(system_unbound_wq, &engine->active.ipi.work);
 }
 
 /*
@@ -302,12 +336,11 @@ static void kick_submission(struct intel_engine_cs *engine,
 	if (inflight->context == rq->context)
 		return;
 
-	ENGINE_TRACE(engine,
-		     "bumping queue-priority-hint:%d for rq:%llx:%lld, inflight:%llx:%lld prio %d\n",
-		     prio,
-		     rq->fence.context, rq->fence.seqno,
-		     inflight->fence.context, inflight->fence.seqno,
-		     inflight->sched.attr.priority);
+	SCHED_TRACE(&engine->active,
+		    "bumping queue-priority-hint:%d for rq:" RQ_FMT ", inflight:" RQ_FMT " prio %d\n",
+		    prio,
+		    RQ_ARG(rq), RQ_ARG(inflight),
+		    inflight->sched.attr.priority);
 
 	engine->execlists.queue_priority_hint = prio;
 	if (need_preempt(prio, rq_prio(inflight)))
@@ -332,6 +365,9 @@ static void __i915_request_set_priority(struct i915_request *rq, int prio)
 	struct list_head *pos = &rq->sched.signalers_list;
 	struct list_head *plist;
 
+	SCHED_TRACE(&engine->active, "PI for " RQ_FMT ", prio:%d\n",
+		    RQ_FMT(rq), prio);
+
 	plist = lookup_priolist(engine, prio);
 
 	/*
@@ -471,6 +507,8 @@ void __i915_sched_defer_request(struct intel_engine_cs *engine,
 	struct i915_request *rn;
 	LIST_HEAD(dfs);
 
+	SCHED_TRACE(&engine->active, "defer request " RQ_FMT "\n", RQ_ARG(rq));
+
 	lockdep_assert_held(&engine->active.lock);
 	GEM_BUG_ON(!test_bit(I915_FENCE_FLAG_PQUEUE, &rq->fence.flags));
 
@@ -598,6 +636,8 @@ void i915_request_enqueue(struct i915_request *rq)
 	unsigned long flags;
 	bool kick = false;
 
+	SCHED_TRACE(&engine->active, "queue request " RQ_FMT "\n", RQ_ARG(rq));
+
 	/* Will be called from irq-context when using foreign fences. */
 	spin_lock_irqsave(&engine->active.lock, flags);
 	GEM_BUG_ON(test_bit(I915_FENCE_FLAG_PQUEUE, &rq->fence.flags));
@@ -658,6 +698,10 @@ __i915_sched_rewind_requests(struct intel_engine_cs *engine)
 		active = rq;
 	}
 
+	SCHED_TRACE(&engine->active,
+		    "rewind requests, active request " RQ_FMT "\n",
+		    RQ_ARG(active));
+
 	return active;
 }
 
@@ -675,8 +719,8 @@ bool __i915_sched_suspend_request(struct intel_engine_cs *engine,
 	if (i915_request_on_hold(rq))
 		return false;
 
-	ENGINE_TRACE(engine, "suspending request %llx:%lld\n",
-		     rq->fence.context, rq->fence.seqno);
+	SCHED_TRACE(&engine->active, "suspending request " RQ_FMT "\n",
+		    RQ_ARG(rq));
 
 	/*
 	 * Transfer this request onto the hold queue to prevent it
@@ -756,8 +800,8 @@ void __i915_sched_resume_request(struct intel_engine_cs *engine,
 	if (!i915_request_on_hold(rq))
 		return;
 
-	ENGINE_TRACE(engine, "resuming request %llx:%lld\n",
-		     rq->fence.context, rq->fence.seqno);
+	SCHED_TRACE(&engine->active, "resuming request " RQ_FMT "\n",
+		    RQ_ARG(rq));
 
 	/*
 	 * Move this request back to the priority queue, and all of its
diff --git a/drivers/gpu/drm/i915/i915_scheduler.h b/drivers/gpu/drm/i915/i915_scheduler.h
index 00ce0a9d519d..346a738c86b5 100644
--- a/drivers/gpu/drm/i915/i915_scheduler.h
+++ b/drivers/gpu/drm/i915/i915_scheduler.h
@@ -16,6 +16,13 @@
 struct drm_printer;
 struct intel_engine_cs;
 
+#define SCHED_TRACE(se, fmt, ...) do {					\
+	const struct i915_sched_engine *se__ __maybe_unused = (se);	\
+	GEM_TRACE("%s sched:%s: " fmt,					\
+		  dev_name(se__->dbg.dev), se__->dbg.name,		\
+		  ##__VA_ARGS__);					\
+} while (0)
+
 #define priolist_for_each_request(it, plist) \
 	list_for_each_entry(it, &(plist)->requests, sched.link)
 
@@ -36,7 +43,11 @@ int i915_sched_node_add_dependency(struct i915_sched_node *node,
 
 void i915_sched_node_retire(struct i915_sched_node *node);
 
-void i915_sched_init_ipi(struct i915_sched_ipi *ipi);
+void i915_sched_init_engine(struct i915_sched_engine *se,
+			    struct device *dev,
+			    const char *name,
+			    unsigned long mask,
+			    unsigned int subclass);
 
 void i915_request_set_priority(struct i915_request *request, int prio);
 
diff --git a/drivers/gpu/drm/i915/i915_scheduler_types.h b/drivers/gpu/drm/i915/i915_scheduler_types.h
index 60a3aa743ab8..3cf60e60dc29 100644
--- a/drivers/gpu/drm/i915/i915_scheduler_types.h
+++ b/drivers/gpu/drm/i915/i915_scheduler_types.h
@@ -20,6 +20,24 @@ struct i915_sched_ipi {
 	struct work_struct work;
 };
 
+struct i915_sched_engine {
+	spinlock_t lock; /* protects the scheduling lists and queue */
+
+	unsigned long mask; /* available scheduling channels */
+
+	struct list_head requests; /* active request, on HW */
+	struct list_head hold; /* ready requests, but on hold */
+
+	struct i915_sched_ipi ipi;
+
+#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM)
+	struct {
+		struct device *dev;
+		const char *name;
+	} dbg;
+#endif
+};
+
 struct i915_sched_attr {
 	/**
 	 * @priority: execution and service priority
diff --git a/drivers/gpu/drm/i915/selftests/i915_scheduler.c b/drivers/gpu/drm/i915/selftests/i915_scheduler.c
index acc666f755d7..a70b6d035d0a 100644
--- a/drivers/gpu/drm/i915/selftests/i915_scheduler.c
+++ b/drivers/gpu/drm/i915/selftests/i915_scheduler.c
@@ -884,6 +884,7 @@ int i915_scheduler_perf_selftests(struct drm_i915_private *i915)
 		T(i915_priolist),
 		T(i915_sched_attr),
 		T(i915_sched_node),
+		T(i915_sched_engine),
 		T(i915_dependency),
 #undef T
 		{}
-- 
2.20.1



More information about the Intel-gfx-trybot mailing list