Re: [PATCH 4/7] drm/i915: Combine multiple internal plists into the same i915_priolist bucket

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

 




On 25/09/2018 09:32, Chris Wilson wrote:
As we are about to allow ourselves to slightly bump the user priority
into a few different sublevels, packthose internal priority lists
into the same i915_priolist to keep the rbtree compact and avoid having
to allocate the default user priority even after the internal bumping.
The downside to having an requests[] rather than a node per active list,
is that we then have to walk over the empty higher priority lists. To
compensate, we track the active buckets and use a small bitmap to skip
over any inactive ones.

v2: Use MASK of internal levels to simplify our usage.

Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
---
  drivers/gpu/drm/i915/intel_engine_cs.c      |  6 +-
  drivers/gpu/drm/i915/intel_guc_submission.c | 12 ++-
  drivers/gpu/drm/i915/intel_lrc.c            | 87 ++++++++++++++-------
  drivers/gpu/drm/i915/intel_ringbuffer.h     | 13 ++-
  4 files changed, 80 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index 217ed3ee1cab..83f2f7774c1f 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -1534,10 +1534,10 @@ void intel_engine_dump(struct intel_engine_cs *engine,
  	count = 0;
  	drm_printf(m, "\t\tQueue priority: %d\n", execlists->queue_priority);
  	for (rb = rb_first_cached(&execlists->queue); rb; rb = rb_next(rb)) {
-		struct i915_priolist *p =
-			rb_entry(rb, typeof(*p), node);
+		struct i915_priolist *p = rb_entry(rb, typeof(*p), node);
+		int i;
- list_for_each_entry(rq, &p->requests, sched.link) {
+		priolist_for_each_request(rq, p, i) {
  			if (count++ < MAX_REQUESTS_TO_SHOW - 1)
  				print_request(m, rq, "\t\tQ ");
  			else
diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
index 4874a212754c..ac862b42f6a1 100644
--- a/drivers/gpu/drm/i915/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/intel_guc_submission.c
@@ -746,30 +746,28 @@ static bool __guc_dequeue(struct intel_engine_cs *engine)
  	while ((rb = rb_first_cached(&execlists->queue))) {
  		struct i915_priolist *p = to_priolist(rb);
  		struct i915_request *rq, *rn;
+		int i;
- list_for_each_entry_safe(rq, rn, &p->requests, sched.link) {
+		priolist_for_each_request_consume(rq, rn, p, i) {
  			if (last && rq->hw_context != last->hw_context) {
-				if (port == last_port) {
-					__list_del_many(&p->requests,
-							&rq->sched.link);
+				if (port == last_port)
  					goto done;
-				}
if (submit)
  					port_assign(port, last);
  				port++;
  			}
- INIT_LIST_HEAD(&rq->sched.link);
+			list_del_init(&rq->sched.link);
__i915_request_submit(rq);
  			trace_i915_request_in(rq, port_index(port, execlists));
+
  			last = rq;
  			submit = true;
  		}
rb_erase_cached(&p->node, &execlists->queue);
-		INIT_LIST_HEAD(&p->requests);
  		if (p->priority != I915_PRIORITY_NORMAL)
  			kmem_cache_free(engine->i915->priorities, p);
  	}
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index e8de250c3413..b1b3f67d1120 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -259,14 +259,49 @@ intel_lr_context_descriptor_update(struct i915_gem_context *ctx,
  	ce->lrc_desc = desc;
  }
-static struct i915_priolist *
+static void assert_priolists(struct intel_engine_execlists * const execlists,
+			     int queue_priority)
+{
+	struct rb_node *rb;
+	int last_prio, i;
+
+	if (!IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM))
+		return;
+
+	GEM_BUG_ON(rb_first_cached(&execlists->queue) !=
+		   rb_first(&execlists->queue.rb_root));
+
+	last_prio = (queue_priority >> I915_USER_PRIORITY_SHIFT) + 1;
+	for (rb = rb_first_cached(&execlists->queue); rb; rb = rb_next(rb)) {
+		struct i915_priolist *p = to_priolist(rb);
+
+		GEM_BUG_ON(p->priority >= last_prio);
+		last_prio = p->priority;
+
+		GEM_BUG_ON(!p->used);
+		for (i = 0; i < ARRAY_SIZE(p->requests); i++) {
+			if (list_empty(&p->requests[i]))
+				continue;
+
+			GEM_BUG_ON(!(p->used & BIT(i)));
+		}
+	}
+}
+
+static struct list_head *
  lookup_priolist(struct intel_engine_cs *engine, int prio)
  {
  	struct intel_engine_execlists * const execlists = &engine->execlists;
  	struct i915_priolist *p;
  	struct rb_node **parent, *rb;
  	bool first = true;
+	int idx, i;
+
+	assert_priolists(execlists, INT_MAX);
+ /* buckets sorted from highest [in slot 0] to lowest priority */
+	idx = I915_PRIORITY_MASK - (prio & I915_PRIORITY_MASK);

How about:

#define I915_INTERNAL_PRIO(p) (p & I915_PRIORITY_MASK)

idx = I915_PRIORITY_COUNT - 1 - I915_INTERNAL_PRIO(prio); ?

Regards,

Tvrtko

+	prio >>= I915_USER_PRIORITY_SHIFT;
  	if (unlikely(execlists->no_priolist))
  		prio = I915_PRIORITY_NORMAL;
@@ -283,7 +318,7 @@ lookup_priolist(struct intel_engine_cs *engine, int prio)
  			parent = &rb->rb_right;
  			first = false;
  		} else {
-			return p;
+			goto out;
  		}
  	}
@@ -309,11 +344,15 @@ lookup_priolist(struct intel_engine_cs *engine, int prio)
  	}
p->priority = prio;
-	INIT_LIST_HEAD(&p->requests);
+	for (i = 0; i < ARRAY_SIZE(p->requests); i++)
+		INIT_LIST_HEAD(&p->requests[i]);
  	rb_link_node(&p->node, rb, parent);
  	rb_insert_color_cached(&p->node, &execlists->queue, first);
+	p->used = 0;
- return p;
+out:
+	p->used |= BIT(idx);
+	return &p->requests[idx];
  }
static void unwind_wa_tail(struct i915_request *rq)
@@ -325,7 +364,7 @@ static void unwind_wa_tail(struct i915_request *rq)
  static void __unwind_incomplete_requests(struct intel_engine_cs *engine)
  {
  	struct i915_request *rq, *rn;
-	struct i915_priolist *uninitialized_var(p);
+	struct list_head *uninitialized_var(pl);
  	int last_prio = I915_PRIORITY_INVALID;
lockdep_assert_held(&engine->timeline.lock);
@@ -342,12 +381,11 @@ static void __unwind_incomplete_requests(struct intel_engine_cs *engine)
  		GEM_BUG_ON(rq_prio(rq) == I915_PRIORITY_INVALID);
  		if (rq_prio(rq) != last_prio) {
  			last_prio = rq_prio(rq);
-			p = lookup_priolist(engine, last_prio);
+			pl = lookup_priolist(engine, last_prio);
  		}
  		GEM_BUG_ON(RB_EMPTY_ROOT(&engine->execlists.queue.rb_root));
- GEM_BUG_ON(p->priority != rq_prio(rq));
-		list_add(&rq->sched.link, &p->requests);
+		list_add(&rq->sched.link, pl);
  	}
  }
@@ -665,8 +703,9 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
  	while ((rb = rb_first_cached(&execlists->queue))) {
  		struct i915_priolist *p = to_priolist(rb);
  		struct i915_request *rq, *rn;
+		int i;
- list_for_each_entry_safe(rq, rn, &p->requests, sched.link) {
+		priolist_for_each_request_consume(rq, rn, p, i) {
  			/*
  			 * Can we combine this request with the current port?
  			 * It has to be the same context/ringbuffer and not
@@ -685,11 +724,8 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
  				 * combine this request with the last, then we
  				 * are done.
  				 */
-				if (port == last_port) {
-					__list_del_many(&p->requests,
-							&rq->sched.link);
+				if (port == last_port)
  					goto done;
-				}
/*
  				 * If GVT overrides us we only ever submit
@@ -699,11 +735,8 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
  				 * request) to the second port.
  				 */
  				if (ctx_single_port_submission(last->hw_context) ||
-				    ctx_single_port_submission(rq->hw_context)) {
-					__list_del_many(&p->requests,
-							&rq->sched.link);
+				    ctx_single_port_submission(rq->hw_context))
  					goto done;
-				}
GEM_BUG_ON(last->hw_context == rq->hw_context); @@ -714,15 +747,16 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
  				GEM_BUG_ON(port_isset(port));
  			}
- INIT_LIST_HEAD(&rq->sched.link);
+			list_del_init(&rq->sched.link);
+
  			__i915_request_submit(rq);
  			trace_i915_request_in(rq, port_index(port, execlists));
+
  			last = rq;
  			submit = true;
  		}
rb_erase_cached(&p->node, &execlists->queue);
-		INIT_LIST_HEAD(&p->requests);
  		if (p->priority != I915_PRIORITY_NORMAL)
  			kmem_cache_free(engine->i915->priorities, p);
  	}
@@ -746,6 +780,7 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
  	 */
  	execlists->queue_priority =
  		port != execlists->port ? rq_prio(last) : INT_MIN;
+	assert_priolists(execlists, execlists->queue_priority);
if (submit) {
  		port_assign(port, last);
@@ -857,16 +892,16 @@ static void execlists_cancel_requests(struct intel_engine_cs *engine)
  	/* Flush the queued requests to the timeline list (for retiring). */
  	while ((rb = rb_first_cached(&execlists->queue))) {
  		struct i915_priolist *p = to_priolist(rb);
+		int i;
- list_for_each_entry_safe(rq, rn, &p->requests, sched.link) {
-			INIT_LIST_HEAD(&rq->sched.link);
+		priolist_for_each_request_consume(rq, rn, p, i) {
+			list_del_init(&rq->sched.link);
dma_fence_set_error(&rq->fence, -EIO);
  			__i915_request_submit(rq);
  		}
rb_erase_cached(&p->node, &execlists->queue);
-		INIT_LIST_HEAD(&p->requests);
  		if (p->priority != I915_PRIORITY_NORMAL)
  			kmem_cache_free(engine->i915->priorities, p);
  	}
@@ -1072,8 +1107,7 @@ static void queue_request(struct intel_engine_cs *engine,
  			  struct i915_sched_node *node,
  			  int prio)
  {
-	list_add_tail(&node->link,
-		      &lookup_priolist(engine, prio)->requests);
+	list_add_tail(&node->link, lookup_priolist(engine, prio));
  }
static void __update_queue(struct intel_engine_cs *engine, int prio)
@@ -1143,7 +1177,7 @@ sched_lock_engine(struct i915_sched_node *node, struct intel_engine_cs *locked)
  static void execlists_schedule(struct i915_request *request,
  			       const struct i915_sched_attr *attr)
  {
-	struct i915_priolist *uninitialized_var(pl);
+	struct list_head *uninitialized_var(pl);
  	struct intel_engine_cs *engine, *last;
  	struct i915_dependency *dep, *p;
  	struct i915_dependency stack;
@@ -1242,8 +1276,7 @@ static void execlists_schedule(struct i915_request *request,
  				pl = lookup_priolist(engine, prio);
  				last = engine;
  			}
-			GEM_BUG_ON(pl->priority != prio);
-			list_move_tail(&node->link, &pl->requests);
+			list_move_tail(&node->link, pl);
  		} else {
  			/*
  			 * If the request is not in the priolist queue because
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 2dfa585712c2..1534de5bb852 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -190,11 +190,22 @@ enum intel_engine_id {
  };
struct i915_priolist {
+	struct list_head requests[I915_PRIORITY_COUNT];
  	struct rb_node node;
-	struct list_head requests;
+	unsigned long used;
  	int priority;
  };
+#define priolist_for_each_request(it, plist, idx) \
+	for (idx = 0; idx < ARRAY_SIZE((plist)->requests); idx++) \
+		list_for_each_entry(it, &(plist)->requests[idx], sched.link)
+
+#define priolist_for_each_request_consume(it, n, plist, idx) \
+	for (; (idx = ffs((plist)->used)); (plist)->used &= ~BIT(idx - 1)) \
+		list_for_each_entry_safe(it, n, \
+					 &(plist)->requests[idx - 1], \
+					 sched.link)
+
  struct st_preempt_hang {
  	struct completion completion;
  	bool inject_hang;

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




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

  Powered by Linux