drm/i915: Prioritise non-busywait semaphore workloads
authorChris Wilson <chris@chris-wilson.co.uk>
Fri, 1 Mar 2019 17:09:01 +0000 (17:09 +0000)
committerChris Wilson <chris@chris-wilson.co.uk>
Fri, 1 Mar 2019 17:45:11 +0000 (17:45 +0000)
We don't want to busywait on the GPU if we have other work to do. If we
give non-busywaiting workloads higher (initial) priority than workloads
that require a busywait, we will prioritise work that is ready to run
immediately. We then also have to be careful that we don't give earlier
semaphores an accidental boost because later work doesn't wait on other
rings, hence we keep a history of semaphore usage of the dependency chain.

v2: Stop rolling the bits into a chain and just use a flag in case this
request or any of our dependencies use a semaphore. The rolling around
was contagious as Tvrtko was heard to fall off his chair.

Testcase: igt/gem_exec_schedule/semaphore
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/20190301170901.8340-4-chris@chris-wilson.co.uk
drivers/gpu/drm/i915/i915_request.c
drivers/gpu/drm/i915/i915_scheduler.c
drivers/gpu/drm/i915/i915_scheduler.h
drivers/gpu/drm/i915/intel_lrc.c

index 59e30b8c4ee91450d2a5dddcd9c9a332739c52ed..bcf3c1a155e2a5f9bead03b9a312ffb5c4ab01fe 100644 (file)
@@ -813,6 +813,7 @@ emit_semaphore_wait(struct i915_request *to,
        *cs++ = 0;
 
        intel_ring_advance(to, cs);
+       to->sched.flags |= I915_SCHED_HAS_SEMAPHORE;
        return 0;
 }
 
@@ -1083,6 +1084,21 @@ void i915_request_add(struct i915_request *request)
        if (engine->schedule) {
                struct i915_sched_attr attr = request->gem_context->sched;
 
+               /*
+                * 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 (!(request->sched.flags & I915_SCHED_HAS_SEMAPHORE))
+                       attr.priority |= I915_PRIORITY_NOSEMAPHORE;
+
                /*
                 * Boost priorities to new clients (new request flows).
                 *
index 50018ad302334f27a965cce215547925f753f241..3f0a4d56bd37f5c0b56cc5a45140662da6bb76cd 100644 (file)
@@ -39,6 +39,7 @@ void i915_sched_node_init(struct i915_sched_node *node)
        INIT_LIST_HEAD(&node->waiters_list);
        INIT_LIST_HEAD(&node->link);
        node->attr.priority = I915_PRIORITY_INVALID;
+       node->flags = 0;
 }
 
 static struct i915_dependency *
@@ -69,6 +70,11 @@ bool __i915_sched_node_add_dependency(struct i915_sched_node *node,
                dep->signaler = signal;
                dep->flags = flags;
 
+               /* Keep track of whether anyone on this chain has a semaphore */
+               if (signal->flags & I915_SCHED_HAS_SEMAPHORE &&
+                   !node_started(signal))
+                       node->flags |= I915_SCHED_HAS_SEMAPHORE;
+
                ret = true;
        }
 
index 7d4a49750d92e7e13fb35fa871be2b2692f8c542..6ce450cf63fa89fefebdfecc8c549f1144d2331c 100644 (file)
@@ -24,14 +24,15 @@ enum {
        I915_PRIORITY_INVALID = INT_MIN
 };
 
-#define I915_USER_PRIORITY_SHIFT 2
+#define I915_USER_PRIORITY_SHIFT 3
 #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_NEWCLIENT        ((u8)BIT(1))
+#define I915_PRIORITY_WAIT             ((u8)BIT(0))
+#define I915_PRIORITY_NEWCLIENT                ((u8)BIT(1))
+#define I915_PRIORITY_NOSEMAPHORE      ((u8)BIT(2))
 
 #define __NO_PREEMPTION (I915_PRIORITY_WAIT)
 
@@ -74,6 +75,8 @@ struct i915_sched_node {
        struct list_head waiters_list; /* those after us, they depend upon us */
        struct list_head link;
        struct i915_sched_attr attr;
+       unsigned int flags;
+#define I915_SCHED_HAS_SEMAPHORE       BIT(0)
 };
 
 struct i915_dependency {
index 6c9479acb4331b1ba72a171b03f51bd6cf4f1330..578c8c98c718dbb60fb3d3ea2c4cfa33f0dced17 100644 (file)
 #define WA_TAIL_DWORDS 2
 #define WA_TAIL_BYTES (sizeof(u32) * WA_TAIL_DWORDS)
 
-#define ACTIVE_PRIORITY (I915_PRIORITY_NEWCLIENT)
+#define ACTIVE_PRIORITY (I915_PRIORITY_NEWCLIENT | I915_PRIORITY_NOSEMAPHORE)
 
 static int execlists_context_deferred_alloc(struct i915_gem_context *ctx,
                                            struct intel_engine_cs *engine,