| Message ID | 20170223074422.4125-16-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 23/02/2017 07:44, Chris Wilson wrote: > As we handoff the GPU reset to the waiter, we need to check we don't > miss a wakeup if it has already been sent prior to us starting the wait. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/i915_gem_request.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c > index ee601e13e28a..eaa333a35f7c 100644 > --- a/drivers/gpu/drm/i915/i915_gem_request.c > +++ b/drivers/gpu/drm/i915/i915_gem_request.c > @@ -1081,8 +1081,10 @@ long i915_wait_request(struct drm_i915_gem_request *req, > trace_i915_gem_request_wait_begin(req, flags); > > add_wait_queue(&req->execute, &exec); > - if (flags & I915_WAIT_LOCKED) > + if (flags & I915_WAIT_LOCKED) { > add_wait_queue(errq, &reset); > + __i915_wait_request_check_and_reset(req); > + } > > intel_wait_init(&wait); > > @@ -1120,7 +1122,8 @@ long i915_wait_request(struct drm_i915_gem_request *req, > goto complete; > > set_current_state(state); > - if (intel_engine_add_wait(req->engine, &wait)) > + if (intel_engine_add_wait(req->engine, &wait) || > + flags & I915_WAIT_LOCKED) > /* In order to check that we haven't missed the interrupt > * as we enabled it, we need to kick ourselves to do a > * coherent check on the seqno before we sleep. > Didn't figure this second bit - what's happening here? Regards, Tvrtko
On Thu, Feb 23, 2017 at 12:35:38PM +0000, Tvrtko Ursulin wrote: > > On 23/02/2017 07:44, Chris Wilson wrote: > >As we handoff the GPU reset to the waiter, we need to check we don't > >miss a wakeup if it has already been sent prior to us starting the wait. > > > >Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > >--- > > drivers/gpu/drm/i915/i915_gem_request.c | 7 +++++-- > > 1 file changed, 5 insertions(+), 2 deletions(-) > > > >diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c > >index ee601e13e28a..eaa333a35f7c 100644 > >--- a/drivers/gpu/drm/i915/i915_gem_request.c > >+++ b/drivers/gpu/drm/i915/i915_gem_request.c > >@@ -1081,8 +1081,10 @@ long i915_wait_request(struct drm_i915_gem_request *req, > > trace_i915_gem_request_wait_begin(req, flags); > > > > add_wait_queue(&req->execute, &exec); > >- if (flags & I915_WAIT_LOCKED) > >+ if (flags & I915_WAIT_LOCKED) { > > add_wait_queue(errq, &reset); > >+ __i915_wait_request_check_and_reset(req); > >+ } > > > > intel_wait_init(&wait); > > > >@@ -1120,7 +1122,8 @@ long i915_wait_request(struct drm_i915_gem_request *req, > > goto complete; > > > > set_current_state(state); > >- if (intel_engine_add_wait(req->engine, &wait)) > >+ if (intel_engine_add_wait(req->engine, &wait) || > >+ flags & I915_WAIT_LOCKED) > > /* In order to check that we haven't missed the interrupt > > * as we enabled it, we need to kick ourselves to do a > > * coherent check on the seqno before we sleep. > > > > Didn't figure this second bit - what's happening here? As we are now on the error/reset waitqueue from the start and have just set ourselves back to TASK_INTERRUPTIBLE (overriding any wake up from reset) before we actually sleep we need to double check for the signal from i915_reset. -Chris
On 23/02/2017 12:41, Chris Wilson wrote: > On Thu, Feb 23, 2017 at 12:35:38PM +0000, Tvrtko Ursulin wrote: >> >> On 23/02/2017 07:44, Chris Wilson wrote: >>> As we handoff the GPU reset to the waiter, we need to check we don't >>> miss a wakeup if it has already been sent prior to us starting the wait. >>> >>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> >>> --- >>> drivers/gpu/drm/i915/i915_gem_request.c | 7 +++++-- >>> 1 file changed, 5 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c >>> index ee601e13e28a..eaa333a35f7c 100644 >>> --- a/drivers/gpu/drm/i915/i915_gem_request.c >>> +++ b/drivers/gpu/drm/i915/i915_gem_request.c >>> @@ -1081,8 +1081,10 @@ long i915_wait_request(struct drm_i915_gem_request *req, >>> trace_i915_gem_request_wait_begin(req, flags); >>> >>> add_wait_queue(&req->execute, &exec); >>> - if (flags & I915_WAIT_LOCKED) >>> + if (flags & I915_WAIT_LOCKED) { >>> add_wait_queue(errq, &reset); >>> + __i915_wait_request_check_and_reset(req); >>> + } >>> >>> intel_wait_init(&wait); >>> >>> @@ -1120,7 +1122,8 @@ long i915_wait_request(struct drm_i915_gem_request *req, >>> goto complete; >>> >>> set_current_state(state); >>> - if (intel_engine_add_wait(req->engine, &wait)) >>> + if (intel_engine_add_wait(req->engine, &wait) || >>> + flags & I915_WAIT_LOCKED) >>> /* In order to check that we haven't missed the interrupt >>> * as we enabled it, we need to kick ourselves to do a >>> * coherent check on the seqno before we sleep. >>> >> >> Didn't figure this second bit - what's happening here? > > As we are now on the error/reset waitqueue from the start and have just > set ourselves back to TASK_INTERRUPTIBLE (overriding any wake up from > reset) before we actually sleep we need to double check for the signal > from i915_reset. Makes sense yes. 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 ee601e13e28a..eaa333a35f7c 100644 --- a/drivers/gpu/drm/i915/i915_gem_request.c +++ b/drivers/gpu/drm/i915/i915_gem_request.c @@ -1081,8 +1081,10 @@ long i915_wait_request(struct drm_i915_gem_request *req, trace_i915_gem_request_wait_begin(req, flags); add_wait_queue(&req->execute, &exec); - if (flags & I915_WAIT_LOCKED) + if (flags & I915_WAIT_LOCKED) { add_wait_queue(errq, &reset); + __i915_wait_request_check_and_reset(req); + } intel_wait_init(&wait); @@ -1120,7 +1122,8 @@ long i915_wait_request(struct drm_i915_gem_request *req, goto complete; set_current_state(state); - if (intel_engine_add_wait(req->engine, &wait)) + if (intel_engine_add_wait(req->engine, &wait) || + flags & I915_WAIT_LOCKED) /* In order to check that we haven't missed the interrupt * as we enabled it, we need to kick ourselves to do a * coherent check on the seqno before we sleep.
As we handoff the GPU reset to the waiter, we need to check we don't miss a wakeup if it has already been sent prior to us starting the wait. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/i915_gem_request.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)