Blocking in a worker is ok, that's what the unbound_wq is for. And it
unifies the paths between the blocking and nonblocking commit, giving
me just one path where I have to implement the deadlock avoidance
trickery in the next patch.

I first tried to implement the following patch without this rework, but
force-completing i915_sw_fence creates some serious challenges around
properly cleaning things up. So wasn't a feasible short-term approach.
Another approach would be to simple keep track of all pending atomic
commit work items and manually queue them from the reset code. With the
caveat that double-queue in case we race with the i915_sw_fence must be
avoided. Given all that, taking the cost of a double schedule in atomic
for the short-term fix is the best approach, but can be changed in the
future of course.

v2: Amend commit message (Chris).

v3: Add comment explaining why we do nothing in the sw_fence complete
callback (Michel).

Reviewed-by: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
Cc: Chris Wilson <ch...@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuopp...@intel.com>
Cc: Joonas Lahtinen <joonas.lahti...@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursu...@intel.com> (v2)
Cc: Michel Thierry <michel.thie...@intel.com>
Reviewed-by: Michel Thierry <michel.thie...@intel.com>
Signed-off-by: Daniel Vetter <daniel.vet...@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index b2c220ba2575..da8d0d3b2bc2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12082,6 +12082,8 @@ static void intel_atomic_commit_tail(struct 
drm_atomic_state *state)
        unsigned crtc_vblank_mask = 0;
        int i;
 
+       i915_sw_fence_wait(&intel_state->commit_ready);
+
        drm_atomic_helper_wait_for_dependencies(state);
 
        if (intel_state->modeset)
@@ -12247,10 +12249,8 @@ intel_atomic_commit_ready(struct i915_sw_fence *fence,
 
        switch (notify) {
        case FENCE_COMPLETE:
-               if (state->base.commit_work.func)
-                       queue_work(system_unbound_wq, &state->base.commit_work);
+               /* we do blocking waits in the worker, nothing to do here */
                break;
-
        case FENCE_FREE:
                {
                        struct intel_atomic_helper *helper =
@@ -12352,14 +12352,14 @@ static int intel_atomic_commit(struct drm_device *dev,
        }
 
        drm_atomic_state_get(state);
-       INIT_WORK(&state->commit_work,
-                 nonblock ? intel_atomic_commit_work : NULL);
+       INIT_WORK(&state->commit_work, intel_atomic_commit_work);
 
        i915_sw_fence_commit(&intel_state->commit_ready);
-       if (!nonblock) {
-               i915_sw_fence_wait(&intel_state->commit_ready);
+       if (nonblock)
+               queue_work(system_unbound_wq, &state->commit_work);
+       else
                intel_atomic_commit_tail(state);
-       }
+
 
        return 0;
 }
-- 
2.13.3

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

Reply via email to