drm/i915: Only reschedule the submission tasklet if preemption is possible

If we couple the scheduler more tightly with the execlists policy, we
can apply the preemption policy to the question of whether we need to
kick the tasklet at all for this priority bump.

v2: Rephrase it as a core i915 policy and not an execlists foible.
v3: Pull the kick together.

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/20190507122544.12698-1-chris@chris-wilson.co.uk

diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h
index f5b0f27cecb6cfd286032444b6303912557e1b79..06d785533502c93926fd7ba1cf0d97892a6e8216 100644 (file)
--- a/drivers/gpu/drm/i915/gt/intel_engine.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine.h
@@ -106,24 +106,6 @@ hangcheck_action_to_str(const enum intel_engine_hangcheck_action a)
 
 void intel_engines_set_scheduler_caps(struct drm_i915_private *i915);
 
-static inline bool __execlists_need_preempt(int prio, int last)
-{
-       /*
-        * Allow preemption of low -> normal -> high, but we do
-        * not allow low priority tasks to preempt other low priority
-        * tasks under the impression that latency for low priority
-        * tasks does not matter (as much as background throughput),
-        * so kiss.
-        *
-        * More naturally we would write
-        *      prio >= max(0, last);
-        * except that we wish to prevent triggering preemption at the same
-        * priority level: the task that is running should remain running
-        * to preserve FIFO ordering of dependencies.
-        */
-       return prio > max(I915_PRIORITY_NORMAL - 1, last);
-}
-
 static inline void
 execlists_set_active(struct intel_engine_execlists *execlists,
                     unsigned int bit)

diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 5580b6f1aa0cba565964fff5840106af0251beda..636df21983dd59e8ed2a883a937522272c0f2eae 100644 (file)
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -252,8 +252,8 @@ static inline bool need_preempt(const struct intel_engine_cs *engine,
         * ourselves, ignore the request.
         */
        last_prio = effective_prio(rq);
-       if (!__execlists_need_preempt(engine->execlists.queue_priority_hint,
-                                     last_prio))
+       if (!i915_scheduler_need_preempt(engine->execlists.queue_priority_hint,
+                                        last_prio))
                return false;
 
        /*

diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c b/drivers/gpu/drm/i915/gt/selftest_lrc.c
index 84538f69185b0231052afc03561cd1fead5dc272..4b042893dc0eec1d6e7087872b790b5201bdadf7 100644 (file)
--- a/drivers/gpu/drm/i915/gt/selftest_lrc.c
+++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
@@ -638,14 +638,19 @@ static struct i915_request *dummy_request(struct intel_engine_cs *engine)
        GEM_BUG_ON(i915_request_completed(rq));
 
        i915_sw_fence_init(&rq->submit, dummy_notify);
-       i915_sw_fence_commit(&rq->submit);
+       set_bit(I915_FENCE_FLAG_ACTIVE, &rq->fence.flags);
 
        return rq;
 }
 
 static void dummy_request_free(struct i915_request *dummy)
 {
+       /* We have to fake the CS interrupt to kick the next request */
+       i915_sw_fence_commit(&dummy->submit);
+
        i915_request_mark_complete(dummy);
+       dma_fence_signal(&dummy->fence);
+
        i915_sched_node_fini(&dummy->sched);
        i915_sw_fence_fini(&dummy->submit);
 

diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index e0be00c07c24206ed3b458e6803c003038dd4e5d..fa955b7b6def1e8619f2a7adce9832f5234a4fc8 100644 (file)
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -1415,9 +1415,7 @@ long i915_request_wait(struct i915_request *rq,
        if (flags & I915_WAIT_PRIORITY) {
                if (!i915_request_started(rq) && INTEL_GEN(rq->i915) >= 6)
                        gen6_rps_boost(rq);
-               local_bh_disable(); /* suspend tasklets for reprioritisation */
                i915_schedule_bump_priority(rq, I915_PRIORITY_WAIT);
-               local_bh_enable(); /* kick tasklets en masse */
        }
 
        wait.tsk = current;

diff --git a/drivers/gpu/drm/i915/i915_scheduler.c b/drivers/gpu/drm/i915/i915_scheduler.c
index 39bc4f54e2720e28aea474d3e7ead7e48c6afe43..ec22c3fe73607eb0d286ca8f470d0e51cda1f52d 100644 (file)
--- a/drivers/gpu/drm/i915/i915_scheduler.c
+++ b/drivers/gpu/drm/i915/i915_scheduler.c
@@ -261,16 +261,27 @@ sched_lock_engine(const struct i915_sched_node *node,
        return engine;
 }
 
-static bool inflight(const struct i915_request *rq,
-                    const struct intel_engine_cs *engine)
+static inline int rq_prio(const struct i915_request *rq)
 {
-       const struct i915_request *active;
+       return rq->sched.attr.priority | __NO_PREEMPTION;
+}
+
+static void kick_submission(struct intel_engine_cs *engine, int prio)
+{
+       const struct i915_request *inflight =
+               port_request(engine->execlists.port);
 
-       if (!i915_request_is_active(rq))
-               return false;
+       /*
+        * If we are already the currently executing context, don't
+        * bother evaluating if we should preempt ourselves, or if
+        * we expect nothing to change as a result of running the
+        * tasklet, i.e. we have not change the priority queue
+        * sufficiently to oust the running context.
+        */
+       if (inflight && !i915_scheduler_need_preempt(prio, rq_prio(inflight)))
+               return;
 
-       active = port_request(engine->execlists.port);
-       return active->hw_context == rq->hw_context;
+       tasklet_hi_schedule(&engine->execlists.tasklet);
 }
 
 static void __i915_schedule(struct i915_request *rq,
@@ -396,15 +407,8 @@ static void __i915_schedule(struct i915_request *rq,
 
                engine->execlists.queue_priority_hint = prio;
 
-               /*
-                * If we are already the currently executing context, don't
-                * bother evaluating if we should preempt ourselves.
-                */
-               if (inflight(node_to_request(node), engine))
-                       continue;
-
                /* Defer (tasklet) submission until after all of our updates. */
-               tasklet_hi_schedule(&engine->execlists.tasklet);
+               kick_submission(engine, prio);
        }
 
        spin_unlock(&engine->timeline.lock);

diff --git a/drivers/gpu/drm/i915/i915_scheduler.h b/drivers/gpu/drm/i915/i915_scheduler.h
index 07d243acf553b2bc9f018be31d8072e5929c9a2b..7eefccff39bf64551d717c35c86af51053bd9c50 100644 (file)
--- a/drivers/gpu/drm/i915/i915_scheduler.h
+++ b/drivers/gpu/drm/i915/i915_scheduler.h
@@ -52,4 +52,22 @@ static inline void i915_priolist_free(struct i915_priolist *p)
                __i915_priolist_free(p);
 }
 
+static inline bool i915_scheduler_need_preempt(int prio, int active)
+{
+       /*
+        * Allow preemption of low -> normal -> high, but we do
+        * not allow low priority tasks to preempt other low priority
+        * tasks under the impression that latency for low priority
+        * tasks does not matter (as much as background throughput),
+        * so kiss.
+        *
+        * More naturally we would write
+        *      prio >= max(0, last);
+        * except that we wish to prevent triggering preemption at the same
+        * priority level: the task that is running should remain running
+        * to preserve FIFO ordering of dependencies.
+        */
+       return prio > max(I915_PRIORITY_NORMAL - 1, active);
+}
+
 #endif /* _I915_SCHEDULER_H_ */

diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
index 57ed1dd4ae41796671fb50bb03e49a2dac2bff5f..380d83a2bfb6a9b74c8991586d8884f7521b11ca 100644 (file)
--- a/drivers/gpu/drm/i915/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/intel_guc_submission.c
@@ -747,7 +747,8 @@ static bool __guc_dequeue(struct intel_engine_cs *engine)
                                &engine->i915->guc.preempt_work[engine->id];
                        int prio = execlists->queue_priority_hint;
 
-                       if (__execlists_need_preempt(prio, port_prio(port))) {
+                       if (i915_scheduler_need_preempt(prio,
+                                                       port_prio(port))) {
                                execlists_set_active(execlists,
                                                     EXECLISTS_ACTIVE_PREEMPT);
                                queue_work(engine->i915->guc.preempt_wq,