| Message ID | 20160914065250.15482-2-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On ke, 2016-09-14 at 07:52 +0100, Chris Wilson wrote: > +int > +i915_gem_request_await_fence(struct drm_i915_gem_request *req, > + struct fence *fence) > +{ > + struct fence_array *array; > + int ret; > + int i; > + > + if (test_bit(FENCE_FLAG_SIGNALED_BIT, &fence->flags)) > + return 0; > + > + if (fence_is_i915(fence)) > + return i915_gem_request_await_request(req, to_request(fence)); > + > + if (!fence_is_array(fence)) { > + ret = i915_sw_fence_await_dma_fence(&req->submit, > + fence, 10*HZ, Magic 10*HZ, give it a define it is not temporary. > + GFP_KERNEL); > + return ret < 0 ? ret : 0; > + } > + > + array = to_fence_array(fence); > + for (i = 0; i < array->num_fences; i++) { > + struct fence *child = array->fences[i]; > + > + if (fence_is_i915(child)) > + ret = i915_gem_request_await_request(req, > + to_request(child)); > + else > + ret = i915_sw_fence_await_dma_fence(&req->submit, > + child, 10*HZ, > + GFP_KERNEL); This chunk repeats from above, make it a small __i915_gem_request_await_fence function. > + if (ret < 0) > + return ret; How about the failure case when we're half bound, no need to tear down? No uses in this patch so hard to evaluate. Regards, Joonas
On Wed, Sep 14, 2016 at 10:37:18AM +0300, Joonas Lahtinen wrote: > On ke, 2016-09-14 at 07:52 +0100, Chris Wilson wrote: > > + array = to_fence_array(fence); > > + for (i = 0; i < array->num_fences; i++) { > > + struct fence *child = array->fences[i]; > > + > > + if (fence_is_i915(child)) > > + ret = i915_gem_request_await_request(req, > > + to_request(child)); > > + else > > + ret = i915_sw_fence_await_dma_fence(&req->submit, > > + child, 10*HZ, > > + GFP_KERNEL); > > This chunk repeats from above, make it a small > __i915_gem_request_await_fence function. It's not a repeat. If it were, we could just recurse. Except we don't know how deep the fence-array could go. > > + if (ret < 0) > > + return ret; > > How about the failure case when we're half bound, no need to tear down? We submit the partial request that doesn't execute anything more than the context setup. The next request in the timeline with then inherit the context setup from this request. Same rule as everything else handling errors during request construction. -Chris
diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c index 40978bc12ceb..624d71bf4518 100644 --- a/drivers/gpu/drm/i915/i915_gem_request.c +++ b/drivers/gpu/drm/i915/i915_gem_request.c @@ -23,6 +23,7 @@ */ #include <linux/prefetch.h> +#include <linux/fence-array.h> #include "i915_drv.h" @@ -494,6 +495,45 @@ i915_gem_request_await_request(struct drm_i915_gem_request *to, return 0; } +int +i915_gem_request_await_fence(struct drm_i915_gem_request *req, + struct fence *fence) +{ + struct fence_array *array; + int ret; + int i; + + if (test_bit(FENCE_FLAG_SIGNALED_BIT, &fence->flags)) + return 0; + + if (fence_is_i915(fence)) + return i915_gem_request_await_request(req, to_request(fence)); + + if (!fence_is_array(fence)) { + ret = i915_sw_fence_await_dma_fence(&req->submit, + fence, 10*HZ, + GFP_KERNEL); + return ret < 0 ? ret : 0; + } + + array = to_fence_array(fence); + for (i = 0; i < array->num_fences; i++) { + struct fence *child = array->fences[i]; + + if (fence_is_i915(child)) + ret = i915_gem_request_await_request(req, + to_request(child)); + else + ret = i915_sw_fence_await_dma_fence(&req->submit, + child, 10*HZ, + GFP_KERNEL); + if (ret < 0) + return ret; + } + + return 0; +} + /** * i915_gem_request_await_object - set this request to (async) wait upon a bo * diff --git a/drivers/gpu/drm/i915/i915_gem_request.h b/drivers/gpu/drm/i915/i915_gem_request.h index 974bd7bcc801..c85a3d82febf 100644 --- a/drivers/gpu/drm/i915/i915_gem_request.h +++ b/drivers/gpu/drm/i915/i915_gem_request.h @@ -214,6 +214,8 @@ int i915_gem_request_await_object(struct drm_i915_gem_request *to, struct drm_i915_gem_object *obj, bool write); +int i915_gem_request_await_fence(struct drm_i915_gem_request *req, + struct fence *fence); void __i915_add_request(struct drm_i915_gem_request *req, bool flush_caches); #define i915_add_request(req) \
We will need to wait on DMA completion (as signaled via struct fence) before executing our i915_gem_request. Therefore we want to expose a method for adding the await on the fence itself to the request. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/i915_gem_request.c | 40 +++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/i915_gem_request.h | 2 ++ 2 files changed, 42 insertions(+)