| Message ID | 20170217183536.26100-2-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On pe, 2017-02-17 at 18:35 +0000, Chris Wilson wrote: > The code currently assumes that all fence arrays it sees are the normal > signal-on-all variety, and decomposes the array into its individual > fences so that it can extract the native i915 fences. If the fence array > is using signal-on-any, we should not decompose as we must not wait on > them all, just the first in *that* set. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > CC: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> <SNIP> > > @@ -696,7 +696,8 @@ i915_gem_request_await_dma_fence(struct drm_i915_gem_request *req, > if (dma_fence_is_i915(fence)) > return i915_gem_request_await_request(req, to_request(fence)); > > - if (!dma_fence_is_array(fence)) { > + if (!dma_fence_is_array(fence) || > + test_bit(DMA_FENCE_ARRAY_SIGNAL_ANY, &fence->flags)) { Smells like a helper function? While that helper is finding the way upstream; Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Regards, Joonas
On Mon, Feb 20, 2017 at 01:32:42PM +0200, Joonas Lahtinen wrote: > On pe, 2017-02-17 at 18:35 +0000, Chris Wilson wrote: > > The code currently assumes that all fence arrays it sees are the normal > > signal-on-all variety, and decomposes the array into its individual > > fences so that it can extract the native i915 fences. If the fence array > > is using signal-on-any, we should not decompose as we must not wait on > > them all, just the first in *that* set. > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > CC: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > <SNIP> > > > > > @@ -696,7 +696,8 @@ i915_gem_request_await_dma_fence(struct drm_i915_gem_request *req, > > if (dma_fence_is_i915(fence)) > > return i915_gem_request_await_request(req, to_request(fence)); > > > > - if (!dma_fence_is_array(fence)) { > > + if (!dma_fence_is_array(fence) || > > + test_bit(DMA_FENCE_ARRAY_SIGNAL_ANY, &fence->flags)) { > > Smells like a helper function? While that helper is finding the way > upstream; Blurgh. enum dma_fence_array_signal_mode { DMA_FENCE_ARRAY_SIGNAL_ON_ALL = 0, DMA_FENCE_ARRAY_SIGNAL_ON_ANY, }; enum dma_fence_array_signal_mode dma_fence_array_get_signaling_mode(struct dma_fence_array *array) { return test_bit(DMA_FENCE_ARRAY_SIGNAL_ANY, &array->base.flags) ? DMA_FENCE_ARRAY_SIGNAL_ON_ANY : DMA_FENCE_ARRAY_SIGNAL_ON_ALL; } if (!dma_fence_is_array(fence) || dma_fence_array_get_signaling_mode(to_dma_fence_array(fence)) == DMA_FENCE_ARRAY_SIGNAL_ON_ANY) Hmm. Not happy with that yet. -Chris
Op 20-02-17 om 13:03 schreef Chris Wilson: > On Mon, Feb 20, 2017 at 01:32:42PM +0200, Joonas Lahtinen wrote: >> On pe, 2017-02-17 at 18:35 +0000, Chris Wilson wrote: >>> The code currently assumes that all fence arrays it sees are the normal >>> signal-on-all variety, and decomposes the array into its individual >>> fences so that it can extract the native i915 fences. If the fence array >>> is using signal-on-any, we should not decompose as we must not wait on >>> them all, just the first in *that* set. >>> >>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> >>> CC: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> >>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> <SNIP> >> >>> @@ -696,7 +696,8 @@ i915_gem_request_await_dma_fence(struct drm_i915_gem_request *req, >>> if (dma_fence_is_i915(fence)) >>> return i915_gem_request_await_request(req, to_request(fence)); >>> >>> - if (!dma_fence_is_array(fence)) { >>> + if (!dma_fence_is_array(fence) || >>> + test_bit(DMA_FENCE_ARRAY_SIGNAL_ANY, &fence->flags)) { >> Smells like a helper function? While that helper is finding the way >> upstream; > Blurgh. > > enum dma_fence_array_signal_mode { > DMA_FENCE_ARRAY_SIGNAL_ON_ALL = 0, > DMA_FENCE_ARRAY_SIGNAL_ON_ANY, > }; > > enum dma_fence_array_signal_mode > dma_fence_array_get_signaling_mode(struct dma_fence_array *array) > { > return test_bit(DMA_FENCE_ARRAY_SIGNAL_ANY, &array->base.flags) ? > DMA_FENCE_ARRAY_SIGNAL_ON_ANY : DMA_FENCE_ARRAY_SIGNAL_ON_ALL; > } > > if (!dma_fence_is_array(fence) || > dma_fence_array_get_signaling_mode(to_dma_fence_array(fence)) == DMA_FENCE_ARRAY_SIGNAL_ON_ANY) > > Hmm. Not happy with that yet. > -Chris > bool dma_fence_array_signal_on_any with a WARN_ON(!dma_fence_is_array(fence))?
On ma, 2017-02-20 at 13:12 +0100, Maarten Lankhorst wrote: > Op 20-02-17 om 13:03 schreef Chris Wilson: > > > > On Mon, Feb 20, 2017 at 01:32:42PM +0200, Joonas Lahtinen wrote: > > > > > > Smells like a helper function? While that helper is finding the way > > > upstream; > > Blurgh. > > > > enum dma_fence_array_signal_mode { > > DMA_FENCE_ARRAY_SIGNAL_ON_ALL = 0, > > DMA_FENCE_ARRAY_SIGNAL_ON_ANY, > > }; > > > > enum dma_fence_array_signal_mode > > dma_fence_array_get_signaling_mode(struct dma_fence_array *array) > > { > > return test_bit(DMA_FENCE_ARRAY_SIGNAL_ANY, &array->base.flags) ? > > DMA_FENCE_ARRAY_SIGNAL_ON_ANY : DMA_FENCE_ARRAY_SIGNAL_ON_ALL; > > } > > > > if (!dma_fence_is_array(fence) || > > dma_fence_array_get_signaling_mode(to_dma_fence_array(fence)) == DMA_FENCE_ARRAY_SIGNAL_ON_ANY) > > > > Hmm. Not happy with that yet. > > -Chris > > > bool dma_fence_array_signal_on_any with a WARN_ON(!dma_fence_is_array(fence))? I was also thinking along these lines. Regards, Joonas
diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c index 2f6cfa47dc61..2ab96c35cc5e 100644 --- a/drivers/gpu/drm/i915/i915_gem_request.c +++ b/drivers/gpu/drm/i915/i915_gem_request.c @@ -696,7 +696,8 @@ i915_gem_request_await_dma_fence(struct drm_i915_gem_request *req, if (dma_fence_is_i915(fence)) return i915_gem_request_await_request(req, to_request(fence)); - if (!dma_fence_is_array(fence)) { + if (!dma_fence_is_array(fence) || + test_bit(DMA_FENCE_ARRAY_SIGNAL_ANY, &fence->flags)) { ret = i915_sw_fence_await_dma_fence(&req->submit, fence, I915_FENCE_TIMEOUT, GFP_KERNEL);
The code currently assumes that all fence arrays it sees are the normal signal-on-all variety, and decomposes the array into its individual fences so that it can extract the native i915 fences. If the fence array is using signal-on-any, we should not decompose as we must not wait on them all, just the first in *that* set. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> --- drivers/gpu/drm/i915/i915_gem_request.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)