From eec39e441c29dd636bbdf5390af765cd9db6b380 Mon Sep 17 00:00:00 2001
From: Chris Wilson <chris@chris-wilson.co.uk>
Date: Thu, 7 May 2020 16:23:38 +0100
Subject: [PATCH] drm/i915: Remove wait priority boosting
Git-commit: eec39e441c29dd636bbdf5390af765cd9db6b380
Patch-mainline: v5.8-rc1
References: bsc#1174737
Upon waiting a request (when asked), we gave that request a small
priority boost, not enough for it to cause preemption, but enough for it
to be scheduled next before all equals. We also used that bit to give
new clients a small priority boost, similar to FQ_CODEL, such that we
favoured short interactive tasks ahead of long running streams.
However, this is causing lots of complications with timeslicing where we
both want to honour the boost and yet ignore it. Those complications
cause unexpected user behaviour (tasks not being timesliced and run
concurrently as epxected), and the easiest way to resolve that is to
remove the boost. Hopefully, we can find a compromise again if we need
to, but in theory timeslicing itself and future more advanced schedulers
should give us the interactivity boost we seek.
Testcase: igt/gem_exec_schedule/lateslice
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200507152338.7452-3-chris@chris-wilson.co.uk
Acked-by: Takashi Iwai <tiwai@suse.de>
---
drivers/gpu/drm/i915/gt/intel_lrc.c | 4 +---
drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 2 +-
drivers/gpu/drm/i915/i915_priolist_types.h | 7 ++-----
drivers/gpu/drm/i915/i915_request.c | 3 ---
drivers/gpu/drm/i915/i915_scheduler.c | 12 +-----------
5 files changed, 5 insertions(+), 23 deletions(-)
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -295,9 +295,7 @@ static int effective_prio(const struct i
if (__i915_request_has_started(rq))
prio |= I915_PRIORITY_NOSEMAPHORE;
- /* Restrict mere WAIT boosts from triggering preemption */
- BUILD_BUG_ON(__NO_PREEMPTION & ~I915_PRIORITY_MASK); /* only internal */
- return prio | __NO_PREEMPTION;
+ return prio;
}
static int queue_prio(const struct intel_engine_execlists *execlists)
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -508,7 +508,7 @@ static void guc_submit(struct intel_engi
static inline int rq_prio(const struct i915_request *rq)
{
- return rq->sched.attr.priority | __NO_PREEMPTION;
+ return rq->sched.attr.priority;
}
static struct i915_request *schedule_in(struct i915_request *rq, int idx)
--- a/drivers/gpu/drm/i915/i915_priolist_types.h
+++ b/drivers/gpu/drm/i915/i915_priolist_types.h
@@ -18,14 +18,13 @@ enum {
I915_PRIORITY_MAX = I915_CONTEXT_MAX_USER_PRIORITY + 1,
};
-#define I915_USER_PRIORITY_SHIFT 2
+#define I915_USER_PRIORITY_SHIFT 1
#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_WAIT ((u8)BIT(0))
-#define I915_PRIORITY_NOSEMAPHORE ((u8)BIT(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)
@@ -40,8 +39,6 @@ enum {
*/
#define I915_PRIORITY_UNPREEMPTABLE INT_MAX
-#define __NO_PREEMPTION (I915_PRIORITY_WAIT)
-
struct i915_priolist {
struct list_head requests[I915_PRIORITY_COUNT];
struct rb_node node;
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -1409,8 +1409,6 @@ void i915_request_add(struct i915_reques
* 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);
@@ -1617,7 +1615,6 @@ long i915_request_wait(struct i915_reque
if (flags & I915_WAIT_PRIORITY) {
if (!i915_request_started(rq) && INTEL_GEN(rq->i915) >= 6)
gen6_rps_boost(rq);
- i915_schedule_bump_priority(rq, I915_PRIORITY_WAIT);
}
wait.tsk = current;
--- a/drivers/gpu/drm/i915/i915_scheduler.c
+++ b/drivers/gpu/drm/i915/i915_scheduler.c
@@ -174,7 +174,7 @@ sched_lock_engine(const struct i915_sche
static inline int rq_prio(const struct i915_request *rq)
{
- return rq->sched.attr.priority | __NO_PREEMPTION;
+ return rq->sched.attr.priority;
}
static inline bool need_preempt(int prio, int active)
@@ -429,16 +429,6 @@ bool __i915_sched_node_add_dependency(st
list_add(&dep->signal_link, &node->signalers_list);
list_add_rcu(&dep->wait_link, &signal->waiters_list);
- /*
- * As we do not allow WAIT to preempt inflight requests,
- * once we have executed a request, along with triggering
- * any execution callbacks, we must preserve its ordering
- * within the non-preemptible FIFO.
- */
- BUILD_BUG_ON(__NO_PREEMPTION & ~I915_PRIORITY_MASK);
- if (flags & I915_DEPENDENCY_EXTERNAL)
- __bump_priority(signal, __NO_PREEMPTION);
-
ret = true;
}