Message ID | 20170223074422.4125-12-chris@chris-wilson.co.uk (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On 23/02/2017 07:44, Chris Wilson wrote: > After the request is cancelled, we then need to remove it from the > global execution timeline and return it to the context timeline, the > inverse of submit_request(). > > v2: Move manipulation of struct intel_wait to helpers > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_gem_request.c | 55 ++++++++++++++++++++++++++++++++ > drivers/gpu/drm/i915/i915_gem_request.h | 3 ++ > drivers/gpu/drm/i915/intel_breadcrumbs.c | 17 ++++++++-- > 3 files changed, 73 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c > index 5ed52521397f..76e31cd7840e 100644 > --- a/drivers/gpu/drm/i915/i915_gem_request.c > +++ b/drivers/gpu/drm/i915/i915_gem_request.c > @@ -441,6 +441,55 @@ void i915_gem_request_submit(struct drm_i915_gem_request *request) > spin_unlock_irqrestore(&engine->timeline->lock, flags); > } > > +void __i915_gem_request_unsubmit(struct drm_i915_gem_request *request) > +{ > + struct intel_engine_cs *engine = request->engine; > + struct intel_timeline *timeline; > + > + assert_spin_locked(&engine->timeline->lock); > + > + /* Only unwind in reverse order, required so that the per-context list > + * is kept in seqno/ring order. > + */ > + GEM_BUG_ON(request->global_seqno != engine->timeline->seqno); > + engine->timeline->seqno--; > + > + /* We may be recursing from the signal callback of another i915 fence */ > + spin_lock_nested(&request->lock, SINGLE_DEPTH_NESTING); > + request->global_seqno = 0; > + if (test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, &request->fence.flags)) > + intel_engine_cancel_signaling(request); > + spin_unlock(&request->lock); > + > + /* Transfer back from the global per-engine timeline to per-context */ > + timeline = request->timeline; > + GEM_BUG_ON(timeline == engine->timeline); > + > + spin_lock(&timeline->lock); > + list_move(&request->link, &timeline->requests); > + spin_unlock(&timeline->lock); > + > + /* We don't need to wake_up any waiters on request->execute, they > + * will get woken by any other event or us re-adding this request > + * to the engine timeline (__i915_gem_request_submit()). The waiters > + * should be quite adapt at finding that the request now has a new > + * global_seqno to the one they went to sleep on. > + */ > +} > + > +void i915_gem_request_unsubmit(struct drm_i915_gem_request *request) > +{ > + struct intel_engine_cs *engine = request->engine; > + unsigned long flags; > + > + /* Will be called from irq-context when using foreign fences. */ > + spin_lock_irqsave(&engine->timeline->lock, flags); > + > + __i915_gem_request_unsubmit(request); > + > + spin_unlock_irqrestore(&engine->timeline->lock, flags); > +} > + > static int __i915_sw_fence_call > submit_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state) > { > @@ -1036,6 +1085,7 @@ long i915_wait_request(struct drm_i915_gem_request *req, > > intel_wait_init(&wait); > > +restart: > reset_wait_queue(&req->execute, &exec); > if (!intel_wait_update_request(&wait, req)) { > do { > @@ -1134,6 +1184,11 @@ long i915_wait_request(struct drm_i915_gem_request *req, > /* Only spin if we know the GPU is processing this request */ > if (i915_spin_request(req, state, 2)) > break; > + > + if (!intel_wait_check_request(&wait, req)) { > + intel_engine_remove_wait(req->engine, &wait); > + goto restart; > + } > } > > intel_engine_remove_wait(req->engine, &wait); > diff --git a/drivers/gpu/drm/i915/i915_gem_request.h b/drivers/gpu/drm/i915/i915_gem_request.h > index b81f6709905c..5f73d8c0a38a 100644 > --- a/drivers/gpu/drm/i915/i915_gem_request.h > +++ b/drivers/gpu/drm/i915/i915_gem_request.h > @@ -274,6 +274,9 @@ void __i915_add_request(struct drm_i915_gem_request *req, bool flush_caches); > void __i915_gem_request_submit(struct drm_i915_gem_request *request); > void i915_gem_request_submit(struct drm_i915_gem_request *request); > > +void __i915_gem_request_unsubmit(struct drm_i915_gem_request *request); > +void i915_gem_request_unsubmit(struct drm_i915_gem_request *request); > + > struct intel_rps_client; > #define NO_WAITBOOST ERR_PTR(-1) > #define IS_RPS_CLIENT(p) (!IS_ERR(p)) > diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c > index dd39e4f7a560..027c93e34c97 100644 > --- a/drivers/gpu/drm/i915/intel_breadcrumbs.c > +++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c > @@ -453,7 +453,12 @@ void intel_engine_remove_wait(struct intel_engine_cs *engine, > spin_unlock_irq(&b->lock); > } > > -static bool signal_complete(struct drm_i915_gem_request *request) > +static bool signal_valid(const struct drm_i915_gem_request *request) > +{ > + return intel_wait_check_request(&request->signaling.wait, request); > +} > + > +static bool signal_complete(const struct drm_i915_gem_request *request) > { > if (!request) > return false; > @@ -462,7 +467,7 @@ static bool signal_complete(struct drm_i915_gem_request *request) > * signalled that this wait is already completed. > */ > if (intel_wait_complete(&request->signaling.wait)) > - return true; > + return signal_valid(request); > > /* Carefully check if the request is complete, giving time for the > * seqno to be visible or if the GPU hung. > @@ -542,13 +547,21 @@ static int intel_breadcrumbs_signaler(void *arg) > > i915_gem_request_put(request); > } else { > + DEFINE_WAIT(exec); > + > if (kthread_should_stop()) { > GEM_BUG_ON(request); > break; > } > > + if (request) > + add_wait_queue(&request->execute, &exec); > + > schedule(); > > + if (request) > + remove_wait_queue(&request->execute, &exec); > + > if (kthread_should_park()) > kthread_parkme(); > } > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Regards, Tvrtko
diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c index 5ed52521397f..76e31cd7840e 100644 --- a/drivers/gpu/drm/i915/i915_gem_request.c +++ b/drivers/gpu/drm/i915/i915_gem_request.c @@ -441,6 +441,55 @@ void i915_gem_request_submit(struct drm_i915_gem_request *request) spin_unlock_irqrestore(&engine->timeline->lock, flags); } +void __i915_gem_request_unsubmit(struct drm_i915_gem_request *request) +{ + struct intel_engine_cs *engine = request->engine; + struct intel_timeline *timeline; + + assert_spin_locked(&engine->timeline->lock); + + /* Only unwind in reverse order, required so that the per-context list + * is kept in seqno/ring order. + */ + GEM_BUG_ON(request->global_seqno != engine->timeline->seqno); + engine->timeline->seqno--; + + /* We may be recursing from the signal callback of another i915 fence */ + spin_lock_nested(&request->lock, SINGLE_DEPTH_NESTING); + request->global_seqno = 0; + if (test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, &request->fence.flags)) + intel_engine_cancel_signaling(request); + spin_unlock(&request->lock); + + /* Transfer back from the global per-engine timeline to per-context */ + timeline = request->timeline; + GEM_BUG_ON(timeline == engine->timeline); + + spin_lock(&timeline->lock); + list_move(&request->link, &timeline->requests); + spin_unlock(&timeline->lock); + + /* We don't need to wake_up any waiters on request->execute, they + * will get woken by any other event or us re-adding this request + * to the engine timeline (__i915_gem_request_submit()). The waiters + * should be quite adapt at finding that the request now has a new + * global_seqno to the one they went to sleep on. + */ +} + +void i915_gem_request_unsubmit(struct drm_i915_gem_request *request) +{ + struct intel_engine_cs *engine = request->engine; + unsigned long flags; + + /* Will be called from irq-context when using foreign fences. */ + spin_lock_irqsave(&engine->timeline->lock, flags); + + __i915_gem_request_unsubmit(request); + + spin_unlock_irqrestore(&engine->timeline->lock, flags); +} + static int __i915_sw_fence_call submit_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state) { @@ -1036,6 +1085,7 @@ long i915_wait_request(struct drm_i915_gem_request *req, intel_wait_init(&wait); +restart: reset_wait_queue(&req->execute, &exec); if (!intel_wait_update_request(&wait, req)) { do { @@ -1134,6 +1184,11 @@ long i915_wait_request(struct drm_i915_gem_request *req, /* Only spin if we know the GPU is processing this request */ if (i915_spin_request(req, state, 2)) break; + + if (!intel_wait_check_request(&wait, req)) { + intel_engine_remove_wait(req->engine, &wait); + goto restart; + } } intel_engine_remove_wait(req->engine, &wait); diff --git a/drivers/gpu/drm/i915/i915_gem_request.h b/drivers/gpu/drm/i915/i915_gem_request.h index b81f6709905c..5f73d8c0a38a 100644 --- a/drivers/gpu/drm/i915/i915_gem_request.h +++ b/drivers/gpu/drm/i915/i915_gem_request.h @@ -274,6 +274,9 @@ void __i915_add_request(struct drm_i915_gem_request *req, bool flush_caches); void __i915_gem_request_submit(struct drm_i915_gem_request *request); void i915_gem_request_submit(struct drm_i915_gem_request *request); +void __i915_gem_request_unsubmit(struct drm_i915_gem_request *request); +void i915_gem_request_unsubmit(struct drm_i915_gem_request *request); + struct intel_rps_client; #define NO_WAITBOOST ERR_PTR(-1) #define IS_RPS_CLIENT(p) (!IS_ERR(p)) diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c index dd39e4f7a560..027c93e34c97 100644 --- a/drivers/gpu/drm/i915/intel_breadcrumbs.c +++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c @@ -453,7 +453,12 @@ void intel_engine_remove_wait(struct intel_engine_cs *engine, spin_unlock_irq(&b->lock); } -static bool signal_complete(struct drm_i915_gem_request *request) +static bool signal_valid(const struct drm_i915_gem_request *request) +{ + return intel_wait_check_request(&request->signaling.wait, request); +} + +static bool signal_complete(const struct drm_i915_gem_request *request) { if (!request) return false; @@ -462,7 +467,7 @@ static bool signal_complete(struct drm_i915_gem_request *request) * signalled that this wait is already completed. */ if (intel_wait_complete(&request->signaling.wait)) - return true; + return signal_valid(request); /* Carefully check if the request is complete, giving time for the * seqno to be visible or if the GPU hung. @@ -542,13 +547,21 @@ static int intel_breadcrumbs_signaler(void *arg) i915_gem_request_put(request); } else { + DEFINE_WAIT(exec); + if (kthread_should_stop()) { GEM_BUG_ON(request); break; } + if (request) + add_wait_queue(&request->execute, &exec); + schedule(); + if (request) + remove_wait_queue(&request->execute, &exec); + if (kthread_should_park()) kthread_parkme(); }
After the request is cancelled, we then need to remove it from the global execution timeline and return it to the context timeline, the inverse of submit_request(). v2: Move manipulation of struct intel_wait to helpers Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> --- drivers/gpu/drm/i915/i915_gem_request.c | 55 ++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/i915_gem_request.h | 3 ++ drivers/gpu/drm/i915/intel_breadcrumbs.c | 17 ++++++++-- 3 files changed, 73 insertions(+), 2 deletions(-)