All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chris Wilson <chris@chris-wilson.co.uk>
To: intel-gfx@lists.freedesktop.org
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Subject: [Intel-gfx] [PATCH 16/56] drm/i915/gt: Remove timeslice suppression
Date: Tue, 29 Dec 2020 12:01:05 +0000	[thread overview]
Message-ID: <20201229120145.26045-16-chris@chris-wilson.co.uk> (raw)
In-Reply-To: <20201229120145.26045-1-chris@chris-wilson.co.uk>

In the next patch, we remove the strict priority system and continuously
re-evaluate the relative priority of tasks. As such we need to enable
the timeslice whenever there is more than one context in the pipeline.
This simplifies the decision and removes some of the tweaks to suppress
timeslicing, allowing us to lift the timeslice enabling to a common spot
at the end of running the submission tasklet.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gt/intel_engine_types.h  |  10 --
 .../drm/i915/gt/intel_execlists_submission.c  | 159 +++++++-----------
 2 files changed, 61 insertions(+), 108 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
index fdec129a6317..f850179a583f 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
@@ -238,16 +238,6 @@ 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_execlists_submission.c b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
index c31eb7d6184c..a8544692e7e2 100644
--- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
+++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
@@ -1116,25 +1116,6 @@ static void defer_active(struct intel_engine_cs *engine)
 	defer_request(rq, i915_sched_lookup_priolist(engine, rq_prio(rq)));
 }
 
-static bool
-need_timeslice(const struct intel_engine_cs *engine,
-	       const struct i915_request *rq)
-{
-	int hint;
-
-	if (!intel_engine_has_timeslices(engine))
-		return false;
-
-	hint = max(engine->execlists.queue_priority_hint,
-		   virtual_prio(&engine->execlists));
-
-	if (!list_is_last(&rq->sched.link, &engine->active.requests))
-		hint = max(hint, rq_prio(list_next_entry(rq, sched.link)));
-
-	GEM_BUG_ON(hint >= I915_PRIORITY_UNPREEMPTABLE);
-	return hint >= effective_prio(rq);
-}
-
 static bool
 timeslice_yield(const struct intel_engine_execlists *el,
 		const struct i915_request *rq)
@@ -1154,76 +1135,68 @@ timeslice_yield(const struct intel_engine_execlists *el,
 	return rq->context->lrc.ccid == READ_ONCE(el->yield);
 }
 
-static bool
-timeslice_expired(const struct intel_engine_execlists *el,
-		  const struct i915_request *rq)
+static bool needs_timeslice(const struct intel_engine_cs *engine,
+			    const struct i915_request *rq)
 {
-	return timer_expired(&el->timer) || timeslice_yield(el, rq);
-}
+	if (!intel_engine_has_timeslices(engine))
+		return false;
 
-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 engine->execlists.queue_priority_hint;
+	/* If not currently active, or about to switch, wait for next event */
+	if (!rq || __i915_request_is_complete(rq))
+		return false;
 
-	return rq_prio(list_next_entry(rq, sched.link));
-}
+	/* We do not need to start the timeslice until after the ACK */
+	if (READ_ONCE(engine->execlists.pending[0]))
+		return false;
 
-static inline unsigned long
-timeslice(const struct intel_engine_cs *engine)
-{
-	return READ_ONCE(engine->props.timeslice_duration_ms);
+	/* If ELSP[1] is occupied, always check to see if worth slicing */
+	if (!list_is_last_rcu(&rq->sched.link, &engine->active.requests))
+		return true;
+
+	/* Otherwise, ELSP[0] is by itself, but may be waiting in the queue */
+	if (!RB_EMPTY_ROOT(&engine->execlists.queue.rb_root))
+		return true;
+
+	return !RB_EMPTY_ROOT(&engine->execlists.virtual.rb_root);
 }
 
-static unsigned long active_timeslice(const struct intel_engine_cs *engine)
+static bool
+timeslice_expired(struct intel_engine_cs *engine, const struct i915_request *rq)
 {
-	const struct intel_engine_execlists *execlists = &engine->execlists;
-	const struct i915_request *rq = *execlists->active;
+	const struct intel_engine_execlists *el = &engine->execlists;
 
-	if (!rq || __i915_request_is_complete(rq))
-		return 0;
+	if (i915_request_has_nopreempt(rq) && __i915_request_has_started(rq))
+		return false;
 
-	if (READ_ONCE(execlists->switch_priority_hint) < effective_prio(rq))
-		return 0;
+	if (!needs_timeslice(engine, rq))
+		return false;
 
-	return timeslice(engine);
+	return timer_expired(&el->timer) || timeslice_yield(el, rq);
 }
 
-static void set_timeslice(struct intel_engine_cs *engine)
+static unsigned long timeslice(const struct intel_engine_cs *engine)
 {
-	unsigned long duration;
-
-	if (!intel_engine_has_timeslices(engine))
-		return;
-
-	duration = active_timeslice(engine);
-	ENGINE_TRACE(engine, "bump timeslicing, interval:%lu", duration);
-
-	set_timer_ms(&engine->execlists.timer, duration);
+	return READ_ONCE(engine->props.timeslice_duration_ms);
 }
 
-static void start_timeslice(struct intel_engine_cs *engine, int prio)
+static void start_timeslice(struct intel_engine_cs *engine)
 {
-	struct intel_engine_execlists *execlists = &engine->execlists;
+	struct intel_engine_execlists *el = &engine->execlists;
 	unsigned long duration;
 
-	if (!intel_engine_has_timeslices(engine))
-		return;
-
-	WRITE_ONCE(execlists->switch_priority_hint, prio);
-	if (prio == INT_MIN)
-		return;
-
-	if (timer_pending(&execlists->timer))
-		return;
+	/* Disable the timer if there is nothing to switch to */
+	duration = 0;
+	if (needs_timeslice(engine, *el->active)) {
+		if (el->timer.expires) {
+			if (!timer_pending(&el->timer))
+				tasklet_hi_schedule(&engine->execlists.tasklet);
+			return;
+		}
 
-	duration = timeslice(engine);
-	ENGINE_TRACE(engine,
-		     "start timeslicing, prio:%d, interval:%lu",
-		     prio, duration);
+		duration = timeslice(engine);
+	}
 
-	set_timer_ms(&execlists->timer, duration);
+	set_timer_ms(&el->timer, duration);
 }
 
 static void record_preemption(struct intel_engine_execlists *execlists)
@@ -1336,16 +1309,16 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
 			__unwind_incomplete_requests(engine);
 
 			last = NULL;
-		} else if (need_timeslice(engine, last) &&
-			   timeslice_expired(execlists, last)) {
+		} else if (timeslice_expired(engine, last)) {
 			ENGINE_TRACE(engine,
-				     "expired last=%llx:%lld, prio=%d, hint=%d, yield?=%s\n",
-				     last->fence.context,
-				     last->fence.seqno,
-				     last->sched.attr.priority,
+				     "expired:%s last=%llx:%lld, prio=%d, hint=%d, yield?=%s\n",
+				     yesno(timer_expired(&execlists->timer)),
+				     last->fence.context, last->fence.seqno,
+				     rq_prio(last),
 				     execlists->queue_priority_hint,
 				     yesno(timeslice_yield(execlists, last)));
 
+			cancel_timer(&execlists->timer);
 			ring_set_paused(engine, 1);
 			defer_active(engine);
 
@@ -1381,7 +1354,6 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
 				 * of timeslices, our queue might be.
 				 */
 				spin_unlock(&engine->active.lock);
-				start_timeslice(engine, queue_prio(execlists));
 				return;
 			}
 		}
@@ -1410,7 +1382,6 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
 		if (last && !can_merge_rq(last, rq)) {
 			spin_unlock(&ve->base.active.lock);
 			spin_unlock(&engine->active.lock);
-			start_timeslice(engine, rq_prio(rq));
 			return; /* leave this for another sibling */
 		}
 
@@ -1574,29 +1545,23 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
 	execlists->queue_priority_hint = queue_prio(execlists);
 	spin_unlock(&engine->active.lock);
 
-	if (submit) {
-		/*
-		 * Skip if we ended up with exactly the same set of requests,
-		 * e.g. trying to timeslice a pair of ordered contexts
-		 */
-		if (!memcmp(execlists->active,
-			    execlists->pending,
-			    (port - execlists->pending) * sizeof(*port)))
-			goto skip_submit;
-
+	/*
+	 * We can skip poking the HW if we ended up with exactly the same set
+	 * of requests as currently running, e.g. trying to timeslice a pair
+	 * of ordered contexts.
+	 */
+	if (submit &&
+	    memcmp(execlists->active,
+		   execlists->pending,
+		   (port - execlists->pending) * sizeof(*port))) {
 		*port = NULL;
 		while (port-- != execlists->pending)
 			execlists_schedule_in(*port, port - execlists->pending);
 
-		execlists->switch_priority_hint =
-			switch_prio(engine, *execlists->pending);
-
 		WRITE_ONCE(execlists->yield, -1);
 		set_preempt_timeout(engine, *execlists->active);
 		execlists_submit_ports(engine);
 	} else {
-		start_timeslice(engine, execlists->queue_priority_hint);
-skip_submit:
 		ring_set_paused(engine, 0);
 		while (port-- != execlists->pending)
 			i915_request_put(*port);
@@ -1954,8 +1919,6 @@ process_csb(struct intel_engine_cs *engine, struct i915_request **inactive)
 		}
 	} while (head != tail);
 
-	set_timeslice(engine);
-
 	/*
 	 * Gen11 has proven to fail wrt global observation point between
 	 * entry and tail update, failing on the ordering and thus
@@ -1968,6 +1931,7 @@ process_csb(struct intel_engine_cs *engine, struct i915_request **inactive)
 	 * invalidation before.
 	 */
 	invalidate_csb_entries(&buf[0], &buf[num_entries - 1]);
+	cancel_timer(&execlists->timer);
 
 	return inactive;
 }
@@ -2386,8 +2350,10 @@ static void execlists_submission_tasklet(unsigned long data)
 		execlists_reset(engine, msg);
 	}
 
-	if (!engine->execlists.pending[0])
+	if (!engine->execlists.pending[0]) {
 		execlists_dequeue_irq(engine);
+		start_timeslice(engine);
+	}
 
 	post_process_csb(post, inactive);
 	rcu_read_unlock();
@@ -3864,9 +3830,6 @@ void intel_execlists_show_requests(struct intel_engine_cs *engine,
 		show_request(m, last, "\t\t", 0);
 	}
 
-	if (execlists->switch_priority_hint != INT_MIN)
-		drm_printf(m, "\t\tSwitch priority hint: %d\n",
-			   READ_ONCE(execlists->switch_priority_hint));
 	if (execlists->queue_priority_hint != INT_MIN)
 		drm_printf(m, "\t\tQueue priority hint: %d\n",
 			   READ_ONCE(execlists->queue_priority_hint));
-- 
2.20.1

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

  parent reply	other threads:[~2020-12-29 12:03 UTC|newest]

Thread overview: 65+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-29 12:00 [Intel-gfx] [PATCH 01/56] drm/i915/gt: Restore ce->signal flush before releasing virtual engine Chris Wilson
2020-12-29 12:00 ` [Intel-gfx] [PATCH 02/56] drm/i915/gt: Only retire on the last breadcrumb if the last request Chris Wilson
2020-12-29 12:00 ` [Intel-gfx] [PATCH 03/56] drm/i915/gt: Cancel submitted requests upon context reset Chris Wilson
2020-12-30 21:07   ` Mika Kuoppala
2020-12-29 12:00 ` [Intel-gfx] [PATCH 04/56] drm/i915/gt: Pull context closure check from request submit to schedule-in Chris Wilson
2020-12-29 12:00 ` [Intel-gfx] [PATCH 05/56] drm/i915/gem: Peek at the inflight context Chris Wilson
2020-12-29 12:00 ` [Intel-gfx] [PATCH 06/56] drm/i915: Mark up protected uses of 'i915_request_completed' Chris Wilson
2020-12-29 12:00 ` [Intel-gfx] [PATCH 07/56] drm/i915: Drop i915_request.lock serialisation around await_start Chris Wilson
2020-12-29 12:00 ` [Intel-gfx] [PATCH 08/56] drm/i915: Drop i915_request.lock requirement for intel_rps_boost() Chris Wilson
2020-12-29 12:00 ` [Intel-gfx] [PATCH 09/56] drm/i915/gem: Reduce ctx->engine_mutex for reading the clone source Chris Wilson
2020-12-29 12:00 ` [Intel-gfx] [PATCH 10/56] drm/i915/gem: Reduce ctx->engines_mutex for get_engines() Chris Wilson
2020-12-29 12:01 ` [Intel-gfx] [PATCH 11/56] drm/i915: Reduce test_and_set_bit to set_bit in i915_request_submit() Chris Wilson
2020-12-29 12:01 ` [Intel-gfx] [PATCH 12/56] drm/i915/gt: Drop atomic for engine->fw_active tracking Chris Wilson
2020-12-29 12:01 ` [Intel-gfx] [PATCH 13/56] drm/i915/gt: Extract busy-stats for ring-scheduler Chris Wilson
2020-12-29 12:01 ` [Intel-gfx] [PATCH 14/56] drm/i915/gt: Convert stats.active to plain unsigned int Chris Wilson
2020-12-29 12:01 ` [Intel-gfx] [PATCH 15/56] drm/i915/gt: Do not suspend bonded requests if one hangs Chris Wilson
2020-12-29 12:01 ` Chris Wilson [this message]
2020-12-29 12:01 ` [Intel-gfx] [PATCH 17/56] drm/i915/gt: Skip over completed active execlists, again Chris Wilson
2020-12-29 12:01 ` [Intel-gfx] [PATCH 18/56] drm/i915: Strip out internal priorities Chris Wilson
2020-12-29 12:01 ` [Intel-gfx] [PATCH 19/56] drm/i915: Remove I915_USER_PRIORITY_SHIFT Chris Wilson
2020-12-29 12:01 ` [Intel-gfx] [PATCH 20/56] drm/i915: Replace engine->schedule() with a known request operation Chris Wilson
2020-12-29 12:01 ` [Intel-gfx] [PATCH 21/56] drm/i915: Teach the i915_dependency to use a double-lock Chris Wilson
2020-12-29 12:01 ` [Intel-gfx] [PATCH 22/56] drm/i915: Restructure priority inheritance Chris Wilson
2020-12-29 12:01 ` [Intel-gfx] [PATCH 23/56] drm/i915/selftests: Measure set-priority duration Chris Wilson
2020-12-29 12:01 ` [Intel-gfx] [PATCH 24/56] drm/i915/selftests: Exercise priority inheritance around an engine loop Chris Wilson
2020-12-29 12:01 ` [Intel-gfx] [PATCH 25/56] drm/i915: Improve DFS for priority inheritance Chris Wilson
2020-12-29 12:01 ` [Intel-gfx] [PATCH 26/56] drm/i915: Extract request submission from execlists Chris Wilson
2020-12-29 12:01 ` [Intel-gfx] [PATCH 27/56] drm/i915: Extract request rewinding " Chris Wilson
2020-12-29 12:01 ` [Intel-gfx] [PATCH 28/56] drm/i915: Extract request suspension from the execlists backend Chris Wilson
2020-12-29 12:01 ` [Intel-gfx] [PATCH 29/56] drm/i915: Extract the ability to defer and rerun a request later Chris Wilson
2020-12-29 12:01 ` [Intel-gfx] [PATCH 30/56] drm/i915: Fix the iterative dfs for defering requests Chris Wilson
2020-12-29 12:01 ` [Intel-gfx] [PATCH 31/56] drm/i915: Move common active lists from engine to i915_scheduler Chris Wilson
2020-12-29 12:01 ` [Intel-gfx] [PATCH 32/56] drm/i915: Move scheduler queue Chris Wilson
2020-12-29 12:01 ` [Intel-gfx] [PATCH 33/56] drm/i915: Move tasklet from execlists to sched Chris Wilson
2020-12-29 12:01 ` [Intel-gfx] [PATCH 34/56] drm/i915: Replace priolist rbtree with a skiplist Chris Wilson
2020-12-29 12:01 ` [Intel-gfx] [PATCH 35/56] drm/i915: Wrap cmpxchg64 with try_cmpxchg64() helper Chris Wilson
2020-12-29 12:01 ` [Intel-gfx] [PATCH 36/56] drm/i915: Fair low-latency scheduling Chris Wilson
2021-01-07 16:05   ` Matthew Brost
2021-01-07 16:45     ` Chris Wilson
2021-01-07 17:10       ` Matthew Brost
2020-12-29 12:01 ` [Intel-gfx] [PATCH 37/56] drm/i915/gt: Specify a deadline for the heartbeat Chris Wilson
2020-12-29 12:01 ` [Intel-gfx] [PATCH 38/56] drm/i915: Extend the priority boosting for the display with a deadline Chris Wilson
2020-12-29 12:01 ` [Intel-gfx] [PATCH 39/56] drm/i915/gt: Support virtual engine queues Chris Wilson
2020-12-29 12:01 ` [Intel-gfx] [PATCH 40/56] drm/i915: Move saturated workload detection back to the context Chris Wilson
2020-12-29 12:01 ` [Intel-gfx] [PATCH 41/56] drm/i915: Bump default timeslicing quantum to 5ms Chris Wilson
2020-12-29 12:01 ` [Intel-gfx] [PATCH 42/56] drm/i915/gt: Wrap intel_timeline.has_initial_breadcrumb Chris Wilson
2020-12-29 12:01 ` [Intel-gfx] [PATCH 43/56] drm/i915/gt: Track timeline GGTT offset separately from subpage offset Chris Wilson
2020-12-29 12:01 ` [Intel-gfx] [PATCH 44/56] drm/i915/gt: Add timeline "mode" Chris Wilson
2020-12-29 12:01 ` [Intel-gfx] [PATCH 45/56] drm/i915/gt: Use indices for writing into relative timelines Chris Wilson
2020-12-29 12:01 ` [Intel-gfx] [PATCH 46/56] drm/i915/selftests: Exercise relative timeline modes Chris Wilson
2020-12-29 12:01 ` [Intel-gfx] [PATCH 47/56] drm/i915/gt: Use ppHWSP for unshared non-semaphore related timelines Chris Wilson
2020-12-29 12:01 ` [Intel-gfx] [PATCH 48/56] Restore "drm/i915: drop engine_pin/unpin_breadcrumbs_irq" Chris Wilson
2020-12-29 12:01 ` [Intel-gfx] [PATCH 49/56] drm/i915/gt: Couple tasklet scheduling for all CS interrupts Chris Wilson
2020-12-29 12:01 ` [Intel-gfx] [PATCH 50/56] drm/i915/gt: Support creation of 'internal' rings Chris Wilson
2020-12-29 12:01 ` [Intel-gfx] [PATCH 51/56] drm/i915/gt: Use client timeline address for seqno writes Chris Wilson
2020-12-29 12:01 ` [Intel-gfx] [PATCH 52/56] drm/i915/gt: Infrastructure for ring scheduling Chris Wilson
2020-12-29 18:34   ` kernel test robot
2020-12-29 18:34     ` kernel test robot
2020-12-29 12:01 ` [Intel-gfx] [PATCH 53/56] drm/i915/gt: Enable busy-stats for ring-scheduler Chris Wilson
2020-12-29 12:01 ` [Intel-gfx] [PATCH 54/56] drm/i915/gt: Implement ring scheduler for gen6/7 Chris Wilson
2020-12-29 12:01 ` [Intel-gfx] [PATCH 55/56] drm/i915/gt: Enable ring scheduling " Chris Wilson
2020-12-29 12:01 ` [Intel-gfx] [PATCH 56/56] drm/i915/gt: Limit C-states while waiting for requests Chris Wilson
2020-12-29 12:20 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [01/56] drm/i915/gt: Restore ce->signal flush before releasing virtual engine Patchwork
2020-12-29 12:22 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2020-12-29 12:49 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201229120145.26045-16-chris@chris-wilson.co.uk \
    --to=chris@chris-wilson.co.uk \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.