| Message ID | 20170202151312.31406-4-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On to, 2017-02-02 at 15:13 +0000, Chris Wilson wrote: > It had only one callsite and existed to keep the code clearer. Now > having shared the wait-on-error between phases and with plans to change > the wait-for-execute in the next few patches, remove the out of line > wait loop and move it into the main body of i915_wait_request. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> <SNIP> > @@ -1080,7 +1037,30 @@ long i915_wait_request(struct drm_i915_gem_request *req, > add_wait_queue(errq, &reset); > > if (!i915_sw_fence_done(&req->execute)) { > - timeout = __i915_request_wait_for_execute(req, flags, timeout); > + DEFINE_WAIT(exec); > + > + do { > + prepare_to_wait(&req->execute.wait, &exec, state); > + if (i915_sw_fence_done(&req->execute)) > + break; > + > + if (flags & I915_WAIT_LOCKED && > + i915_reset_in_progress(&req->i915->gpu_error)) { > + __set_current_state(TASK_RUNNING); > + i915_reset(req->i915); I'm no the expert here, but this reads funnily; "if reset in progress, do reset". This was pre-existing code, so; Reviewed-by: Joonas Lahtinen <joonas.lahtine@linux.intel.com> Regards, Joonas
On Tue, Feb 14, 2017 at 01:32:37PM +0200, Joonas Lahtinen wrote: > On to, 2017-02-02 at 15:13 +0000, Chris Wilson wrote: > > It had only one callsite and existed to keep the code clearer. Now > > having shared the wait-on-error between phases and with plans to change > > the wait-for-execute in the next few patches, remove the out of line > > wait loop and move it into the main body of i915_wait_request. > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > <SNIP> > > > @@ -1080,7 +1037,30 @@ long i915_wait_request(struct drm_i915_gem_request *req, > > add_wait_queue(errq, &reset); > > > > if (!i915_sw_fence_done(&req->execute)) { > > - timeout = __i915_request_wait_for_execute(req, flags, timeout); > > + DEFINE_WAIT(exec); > > + > > + do { > > + prepare_to_wait(&req->execute.wait, &exec, state); > > + if (i915_sw_fence_done(&req->execute)) > > + break; > > + > > + if (flags & I915_WAIT_LOCKED && > > + i915_reset_in_progress(&req->i915->gpu_error)) { > > + __set_current_state(TASK_RUNNING); > > + i915_reset(req->i915); > > I'm no the expert here, but this reads funnily; "if reset in progress, > do reset". In a series-to-come, this is renamed to if (i915_reset_handoff()) i915_reset(); -Chris
diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c index 8d4f0c859fee..e4eeb5f5453c 100644 --- a/drivers/gpu/drm/i915/i915_gem_request.c +++ b/drivers/gpu/drm/i915/i915_gem_request.c @@ -988,49 +988,6 @@ bool __i915_spin_request(const struct drm_i915_gem_request *req, return false; } -static long -__i915_request_wait_for_execute(struct drm_i915_gem_request *request, - unsigned int flags, - long timeout) -{ - const int state = flags & I915_WAIT_INTERRUPTIBLE ? - TASK_INTERRUPTIBLE : TASK_UNINTERRUPTIBLE; - wait_queue_head_t *q = &request->i915->gpu_error.wait_queue; - DEFINE_WAIT(reset); - DEFINE_WAIT(wait); - - if (flags & I915_WAIT_LOCKED) - add_wait_queue(q, &reset); - - do { - prepare_to_wait(&request->execute.wait, &wait, state); - - if (i915_sw_fence_done(&request->execute)) - break; - - if (flags & I915_WAIT_LOCKED && - i915_reset_in_progress(&request->i915->gpu_error)) { - __set_current_state(TASK_RUNNING); - i915_reset(request->i915); - reset_wait_queue(q, &reset); - continue; - } - - if (signal_pending_state(state, current)) { - timeout = -ERESTARTSYS; - break; - } - - timeout = io_schedule_timeout(timeout); - } while (timeout); - finish_wait(&request->execute.wait, &wait); - - if (flags & I915_WAIT_LOCKED) - remove_wait_queue(q, &reset); - - return timeout; -} - /** * i915_wait_request - wait until execution of request has finished * @req: the request to wait upon @@ -1080,7 +1037,30 @@ long i915_wait_request(struct drm_i915_gem_request *req, add_wait_queue(errq, &reset); if (!i915_sw_fence_done(&req->execute)) { - timeout = __i915_request_wait_for_execute(req, flags, timeout); + DEFINE_WAIT(exec); + + do { + prepare_to_wait(&req->execute.wait, &exec, state); + if (i915_sw_fence_done(&req->execute)) + break; + + if (flags & I915_WAIT_LOCKED && + i915_reset_in_progress(&req->i915->gpu_error)) { + __set_current_state(TASK_RUNNING); + i915_reset(req->i915); + reset_wait_queue(errq, &reset); + continue; + } + + if (signal_pending_state(state, current)) { + timeout = -ERESTARTSYS; + break; + } + + timeout = io_schedule_timeout(timeout); + } while (timeout); + finish_wait(&req->execute.wait, &exec); + if (timeout < 0) goto complete;
It had only one callsite and existed to keep the code clearer. Now having shared the wait-on-error between phases and with plans to change the wait-for-execute in the next few patches, remove the out of line wait loop and move it into the main body of i915_wait_request. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/i915_gem_request.c | 68 ++++++++++++--------------------- 1 file changed, 24 insertions(+), 44 deletions(-)