Re: [Intel-gfx] [PATCH 01/18] drm/i915: Support asynchronous waits on struct fence from i915_gem_request

2016-09-19 Thread Chris Wilson
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(>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

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 01/18] drm/i915: Support asynchronous waits on struct fence from i915_gem_request

2016-09-14 Thread Joonas Lahtinen
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, >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(>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(>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
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx