From 18e4af04d2183e99e6808f55dcef30f66ac0b155 Mon Sep 17 00:00:00 2001
From: Chris Wilson <chris@chris-wilson.co.uk>
Date: Wed, 13 May 2020 18:35:04 +0100
Subject: [PATCH] drm/i915: Drop no-semaphore boosting
Git-commit: 18e4af04d2183e99e6808f55dcef30f66ac0b155
Patch-mainline: v5.8-rc1
References: bsc#1174737
Now that we have fast timeslicing on semaphores, we no longer need to
prioritise none-semaphore work as we will yield any work blocked on a
semaphore to the next in the queue. Previously with no timeslicing,
blocking on the semaphore caused extremely bad scheduling with multiple
clients utilising multiple rings. Now, there is no impact and we can
remove the complication.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200513173504.28322-1-chris@chris-wilson.co.uk
Acked-by: Takashi Iwai <tiwai@suse.de>
---
drivers/gpu/drm/i915/gt/intel_lrc.c | 9 ----
drivers/gpu/drm/i915/gt/selftest_context.c | 1
drivers/gpu/drm/i915/i915_priolist_types.h | 4 --
drivers/gpu/drm/i915/i915_request.c | 53 ++--------------------------
drivers/gpu/drm/i915/i915_request.h | 1
drivers/gpu/drm/i915/i915_scheduler.c | 11 ++---
drivers/gpu/drm/i915/i915_scheduler_types.h | 3 -
7 files changed, 12 insertions(+), 70 deletions(-)
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -286,15 +286,6 @@ static int effective_prio(const struct i
if (i915_request_has_nopreempt(rq))
prio = I915_PRIORITY_UNPREEMPTABLE;
- /*
- * On unwinding the active request, we give it a priority bump
- * if it has completed waiting on any semaphore. If we know that
- * the request has already started, we can prevent an unwanted
- * preempt-to-idle cycle by taking that into account now.
- */
- if (__i915_request_has_started(rq))
- prio |= I915_PRIORITY_NOSEMAPHORE;
-
return prio;
}
--- a/drivers/gpu/drm/i915/gt/selftest_context.c
+++ b/drivers/gpu/drm/i915/gt/selftest_context.c
@@ -23,6 +23,7 @@ static int request_sync(struct i915_requ
/* Opencode i915_request_add() so we can keep the timeline locked. */
__i915_request_commit(rq);
+ rq->sched.attr.priority = I915_PRIORITY_BARRIER;
__i915_request_queue(rq, NULL);
timeout = i915_request_wait(rq, 0, HZ / 10);
--- a/drivers/gpu/drm/i915/i915_priolist_types.h
+++ b/drivers/gpu/drm/i915/i915_priolist_types.h
@@ -18,14 +18,12 @@ enum {
I915_PRIORITY_MAX = I915_CONTEXT_MAX_USER_PRIORITY + 1,
};
-#define I915_USER_PRIORITY_SHIFT 1
+#define I915_USER_PRIORITY_SHIFT 0
#define I915_USER_PRIORITY(x) ((x) << I915_USER_PRIORITY_SHIFT)
#define I915_PRIORITY_COUNT BIT(I915_USER_PRIORITY_SHIFT)
#define I915_PRIORITY_MASK (I915_PRIORITY_COUNT - 1)
-#define I915_PRIORITY_NOSEMAPHORE ((u8)BIT(0))
-
/* Smallest priority value that cannot be bumped. */
#define I915_PRIORITY_INVALID (INT_MIN | (u8)I915_PRIORITY_MASK)
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -370,8 +370,6 @@ __await_execution(struct i915_request *r
}
spin_unlock_irq(&signal->lock);
- /* Copy across semaphore status as we need the same behaviour */
- rq->sched.flags |= signal->sched.flags;
return 0;
}
@@ -499,10 +497,8 @@ void __i915_request_unsubmit(struct i915
spin_unlock(&request->lock);
/* We've already spun, don't charge on resubmitting. */
- if (request->sched.semaphores && i915_request_started(request)) {
- request->sched.attr.priority |= I915_PRIORITY_NOSEMAPHORE;
+ if (request->sched.semaphores && i915_request_started(request))
request->sched.semaphores = 0;
- }
/*
* We don't need to wake_up any waiters on request->execute, they
@@ -560,15 +556,6 @@ submit_notify(struct i915_sw_fence *fenc
return NOTIFY_DONE;
}
-static void irq_semaphore_cb(struct irq_work *wrk)
-{
- struct i915_request *rq =
- container_of(wrk, typeof(*rq), semaphore_work);
-
- i915_schedule_bump_priority(rq, I915_PRIORITY_NOSEMAPHORE);
- i915_request_put(rq);
-}
-
static int __i915_sw_fence_call
semaphore_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
{
@@ -576,11 +563,6 @@ semaphore_notify(struct i915_sw_fence *f
switch (state) {
case FENCE_COMPLETE:
- if (!(READ_ONCE(rq->sched.attr.priority) & I915_PRIORITY_NOSEMAPHORE)) {
- i915_request_get(rq);
- init_irq_work(&rq->semaphore_work, irq_semaphore_cb);
- irq_work_queue(&rq->semaphore_work);
- }
break;
case FENCE_FREE:
@@ -922,6 +904,8 @@ emit_semaphore_wait(struct i915_request
struct i915_request *from,
gfp_t gfp)
{
+ struct i915_sw_fence *wait = &to->submit;
+
/*
* If this or its dependents are waiting on an external fence
* that may fail catastrophically, then we want to avoid using
@@ -946,11 +930,10 @@ emit_semaphore_wait(struct i915_request
goto await_fence;
to->sched.semaphores |= from->engine->mask;
- to->sched.flags |= I915_SCHED_HAS_SEMAPHORE_CHAIN;
- return 0;
+ wait = &to->semaphore;
await_fence:
- return i915_sw_fence_await_dma_fence(&to->submit,
+ return i915_sw_fence_await_dma_fence(wait,
&from->fence, 0,
I915_FENCE_GFP);
}
@@ -989,17 +972,6 @@ i915_request_await_request(struct i915_r
if (ret < 0)
return ret;
- if (to->sched.flags & I915_SCHED_HAS_SEMAPHORE_CHAIN) {
- ret = i915_sw_fence_await_dma_fence(&to->semaphore,
- &from->fence, 0,
- I915_FENCE_GFP);
- if (ret < 0)
- return ret;
- }
-
- if (from->sched.flags & I915_SCHED_HAS_EXTERNAL_CHAIN)
- to->sched.flags |= I915_SCHED_HAS_EXTERNAL_CHAIN;
-
return 0;
}
@@ -1449,21 +1421,6 @@ void i915_request_add(struct i915_reques
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
--- a/drivers/gpu/drm/i915/i915_request.h
+++ b/drivers/gpu/drm/i915/i915_request.h
@@ -160,7 +160,6 @@ struct i915_request {
};
struct list_head execute_cb;
struct i915_sw_fence semaphore;
- struct irq_work semaphore_work;
/*
* A list of everyone we wait upon, and everyone who waits upon us.
--- a/drivers/gpu/drm/i915/i915_scheduler.c
+++ b/drivers/gpu/drm/i915/i915_scheduler.c
@@ -51,11 +51,11 @@ static void assert_priolists(struct inte
GEM_BUG_ON(rb_first_cached(&execlists->queue) !=
rb_first(&execlists->queue.rb_root));
- last_prio = (INT_MAX >> I915_USER_PRIORITY_SHIFT) + 1;
+ last_prio = INT_MAX;
for (rb = rb_first_cached(&execlists->queue); rb; rb = rb_next(rb)) {
const struct i915_priolist *p = to_priolist(rb);
- GEM_BUG_ON(p->priority >= last_prio);
+ GEM_BUG_ON(p->priority > last_prio);
last_prio = p->priority;
GEM_BUG_ON(!p->used);
@@ -420,15 +420,12 @@ bool __i915_sched_node_add_dependency(st
dep->waiter = node;
dep->flags = flags;
- /* Keep track of whether anyone on this chain has a semaphore */
- if (signal->flags & I915_SCHED_HAS_SEMAPHORE_CHAIN &&
- !node_started(signal))
- node->flags |= I915_SCHED_HAS_SEMAPHORE_CHAIN;
-
/* All set, now publish. Beware the lockless walkers. */
list_add(&dep->signal_link, &node->signalers_list);
list_add_rcu(&dep->wait_link, &signal->waiters_list);
+ /* Propagate the chains */
+ node->flags |= signal->flags;
ret = true;
}
--- a/drivers/gpu/drm/i915/i915_scheduler_types.h
+++ b/drivers/gpu/drm/i915/i915_scheduler_types.h
@@ -56,8 +56,7 @@ struct i915_sched_node {
struct list_head link;
struct i915_sched_attr attr;
unsigned int flags;
-#define I915_SCHED_HAS_SEMAPHORE_CHAIN BIT(0)
-#define I915_SCHED_HAS_EXTERNAL_CHAIN BIT(1)
+#define I915_SCHED_HAS_EXTERNAL_CHAIN BIT(0)
intel_engine_mask_t semaphores;
};