In commit b7404c7ecb38 ("drm/i915: Bump ready tasks ahead of
busywaits"), I tried cutting a corner in order to not install a signal
for each of our dependencies, and only listened to requests on which we
were intending to busywait. The compromise that was made was that
instead of then being able to promite the request with a full
NOSEMAPHORE like its non-busywaiting brethren, as we had not ensured we
had cleared the semaphore chain, we settled for only using the NEWCLIENT
boost. With an over saturated system with multiple NEWCLIENTS in flight
at any time, this was found to be an inadequate promotion and left us
with a much poorer scheduling order than prior to using semaphores.

The outcome of this patch, is that all requests have NOSEMAPHORE
priority when they have no dependencies and are ready to run and not
busywait, restoring the pre-semaphore ordering on saturated systems.

Fixes: b7404c7ecb38 ("drm/i915: Bump ready tasks ahead of busywaits")
Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursu...@intel.com>
Cc: Dmitry Rogozhkin <dmitry.v.rogozh...@intel.com>
Cc: Dmitry Ermilov <dmitry.ermi...@intel.com>
---
 drivers/gpu/drm/i915/i915_request.c | 31 +++++++++++------------------
 1 file changed, 12 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_request.c 
b/drivers/gpu/drm/i915/i915_request.c
index bed213148cbb..5569dff3887b 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -569,18 +569,7 @@ semaphore_notify(struct i915_sw_fence *fence, enum 
i915_sw_fence_notify state)
 
        switch (state) {
        case FENCE_COMPLETE:
-               /*
-                * We only check a small portion of our dependencies
-                * and so cannot guarantee that there remains no
-                * semaphore chain across all. Instead of opting
-                * for the full NOSEMAPHORE boost, we go for the
-                * smaller (but still preempting) boost of
-                * NEWCLIENT. This will be enough to boost over
-                * a busywaiting request (as that cannot be
-                * NEWCLIENT) without accidentally boosting
-                * a busywait over real work elsewhere.
-                */
-               i915_schedule_bump_priority(request, I915_PRIORITY_NEWCLIENT);
+               i915_schedule_bump_priority(request, I915_PRIORITY_NOSEMAPHORE);
                break;
 
        case FENCE_FREE:
@@ -846,12 +835,6 @@ emit_semaphore_wait(struct i915_request *to,
        if (err < 0)
                return err;
 
-       err = i915_sw_fence_await_dma_fence(&to->semaphore,
-                                           &from->fence, 0,
-                                           I915_FENCE_GFP);
-       if (err < 0)
-               return err;
-
        /* Only submit our spinner after the signaler is running! */
        err = i915_request_await_execution(to, from, gfp);
        if (err)
@@ -917,8 +900,18 @@ i915_request_await_request(struct i915_request *to, struct 
i915_request *from)
                                                    &from->fence, 0,
                                                    I915_FENCE_GFP);
        }
+       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;
+       }
 
-       return ret < 0 ? ret : 0;
+       return 0;
 }
 
 int
-- 
2.20.1

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

Reply via email to