On 08/02/2017 17:54, Chris Wilson wrote:
On Wed, Feb 08, 2017 at 05:28:39PM +0000, Tvrtko Ursulin wrote:
On 08/02/2017 16:54, Chris Wilson wrote:
We first wait for a request to be submitted to hw and assigned a seqno,
before we can wait for the hw to signal completion (otherwise we don't
know the hw id we need to wait upon). Whilst waiting for the request to
be submitted, we may exceed the user's timeout and need to propagate the
error back.
Reported-by: Tvrtko Ursulin <tvrtko.ursu...@intel.com>
Fixes: 4680816be336 ("drm/i915: Wait first for submission, before waiting for
request completion")
Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursu...@intel.com>
Cc: Joonas Lahtinen <joonas.lahti...@linux.intel.com>
Cc: <drm-intel-fi...@lists.freedesktop.org> # v4.10-rc1+
---
drivers/gpu/drm/i915/i915_gem_request.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_gem_request.c
b/drivers/gpu/drm/i915/i915_gem_request.c
index 72b7f7d9461d..69aff559cf8e 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -1084,6 +1084,9 @@ long i915_wait_request(struct drm_i915_gem_request *req,
if (timeout < 0)
goto complete;
+ if (!timeout)
+ return -ETIME;
+
GEM_BUG_ON(!i915_sw_fence_done(&req->execute));
}
GEM_BUG_ON(!i915_sw_fence_done(&req->submit));
Perhaps "else if" would be more typical, but still OK for a fix.
What I did later on in the series, was
diff --git a/drivers/gpu/drm/i915/i915_gem_request.c
b/drivers/gpu/drm/i915/i915_gem_request.c
index 72b7f7d9461d..c33f537f02b3 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -1026,7 +1026,11 @@ __i915_request_wait_for_execute(struct
drm_i915_gem_request *request,
}
timeout = io_schedule_timeout(timeout);
- } while (timeout);
+ if (!timeout) {
+ timeout = -ETIME;
+ break;
+ }
+ } while (1);
finish_wait(&request->execute.wait, &wait);
if (flags & I915_WAIT_LOCKED)
That seemed like a more consistent pattern to use. Care to consider that as a
v2?
Don't like to see "while (1)", how about:
return timeout == 0 ? -ETIME : timeout;
For the exit of __i915_request_wait_for_execute?
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
