| Message ID | 20200403091300.14734-9-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/10] drm/i915/selftests: Add request throughput measurement to perf | expand |
On 03/04/2020 10:12, Chris Wilson wrote: > Fixes: a88b6e4cbafd ("drm/i915: Allow specification of parallel execbuf") It looks like new uapi on the technical level, even though from a higher level it is just an application of existing uapi across more modes, so why fixes and who is the consumer? Regards, Tvrtko > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > --- > drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 10 +++++++--- > include/uapi/drm/i915_drm.h | 7 ++++--- > 2 files changed, 11 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > index bf1b5399ffa3..5c1c5a9eced4 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > @@ -2299,7 +2299,7 @@ get_fence_array(struct drm_i915_gem_execbuffer2 *args, > BUILD_BUG_ON(~(ARCH_KMALLOC_MINALIGN - 1) & > ~__I915_EXEC_FENCE_UNKNOWN_FLAGS); > > - fences[n] = ptr_pack_bits(syncobj, fence.flags, 2); > + fences[n] = ptr_pack_bits(syncobj, fence.flags, 3); > } > > return fences; > @@ -2330,7 +2330,7 @@ await_fence_array(struct i915_execbuffer *eb, > struct dma_fence *fence; > unsigned int flags; > > - syncobj = ptr_unpack_bits(fences[n], &flags, 2); > + syncobj = ptr_unpack_bits(fences[n], &flags, 3); > if (!(flags & I915_EXEC_FENCE_WAIT)) > continue; > > @@ -2354,7 +2354,11 @@ await_fence_array(struct i915_execbuffer *eb, > spin_unlock(&syncobj->lock); > } > > - err = i915_request_await_dma_fence(eb->request, fence); > + if (flags & I915_EXEC_FENCE_WAIT_SUBMIT) > + err = i915_request_await_execution(eb->request, fence, > + eb->engine->bond_execute); > + else > + err = i915_request_await_dma_fence(eb->request, fence); > dma_fence_put(fence); > if (err < 0) > return err; > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h > index 14b67cd6b54b..704dd0e3bc1d 100644 > --- a/include/uapi/drm/i915_drm.h > +++ b/include/uapi/drm/i915_drm.h > @@ -1040,9 +1040,10 @@ struct drm_i915_gem_exec_fence { > */ > __u32 handle; > > -#define I915_EXEC_FENCE_WAIT (1<<0) > -#define I915_EXEC_FENCE_SIGNAL (1<<1) > -#define __I915_EXEC_FENCE_UNKNOWN_FLAGS (-(I915_EXEC_FENCE_SIGNAL << 1)) > +#define I915_EXEC_FENCE_WAIT (1u << 0) > +#define I915_EXEC_FENCE_SIGNAL (1u << 1) > +#define I915_EXEC_FENCE_WAIT_SUBMIT (1u << 2) > +#define __I915_EXEC_FENCE_UNKNOWN_FLAGS (-(I915_EXEC_FENCE_WAIT_SUBMIT << 1)) > __u32 flags; > }; > >
Quoting Tvrtko Ursulin (2020-04-07 11:44:45) > > On 03/04/2020 10:12, Chris Wilson wrote: > > Fixes: a88b6e4cbafd ("drm/i915: Allow specification of parallel execbuf") > > It looks like new uapi on the technical level, even though from a higher > level it is just an application of existing uapi across more modes, so > why fixes and who is the consumer? Submitting semaphores from userspace for batches under construction [passed between processes via syncobj/sync-file]. iris has a bug where it is trying to wait on a future fence to be submitted and cannot -- but we already have the uapi to handle that elsewhere. -Chris
On 07/04/2020 11:51, Chris Wilson wrote: > Quoting Tvrtko Ursulin (2020-04-07 11:44:45) >> >> On 03/04/2020 10:12, Chris Wilson wrote: >>> Fixes: a88b6e4cbafd ("drm/i915: Allow specification of parallel execbuf") >> >> It looks like new uapi on the technical level, even though from a higher >> level it is just an application of existing uapi across more modes, so >> why fixes and who is the consumer? > > Submitting semaphores from userspace for batches under construction > [passed between processes via syncobj/sync-file]. iris has a bug where > it is trying to wait on a future fence to be submitted and cannot -- > but we already have the uapi to handle that elsewhere. I am all for consistent uapi and this looks simple enough to me. Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Regards, Tvrtko
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c index bf1b5399ffa3..5c1c5a9eced4 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c @@ -2299,7 +2299,7 @@ get_fence_array(struct drm_i915_gem_execbuffer2 *args, BUILD_BUG_ON(~(ARCH_KMALLOC_MINALIGN - 1) & ~__I915_EXEC_FENCE_UNKNOWN_FLAGS); - fences[n] = ptr_pack_bits(syncobj, fence.flags, 2); + fences[n] = ptr_pack_bits(syncobj, fence.flags, 3); } return fences; @@ -2330,7 +2330,7 @@ await_fence_array(struct i915_execbuffer *eb, struct dma_fence *fence; unsigned int flags; - syncobj = ptr_unpack_bits(fences[n], &flags, 2); + syncobj = ptr_unpack_bits(fences[n], &flags, 3); if (!(flags & I915_EXEC_FENCE_WAIT)) continue; @@ -2354,7 +2354,11 @@ await_fence_array(struct i915_execbuffer *eb, spin_unlock(&syncobj->lock); } - err = i915_request_await_dma_fence(eb->request, fence); + if (flags & I915_EXEC_FENCE_WAIT_SUBMIT) + err = i915_request_await_execution(eb->request, fence, + eb->engine->bond_execute); + else + err = i915_request_await_dma_fence(eb->request, fence); dma_fence_put(fence); if (err < 0) return err; diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index 14b67cd6b54b..704dd0e3bc1d 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -1040,9 +1040,10 @@ struct drm_i915_gem_exec_fence { */ __u32 handle; -#define I915_EXEC_FENCE_WAIT (1<<0) -#define I915_EXEC_FENCE_SIGNAL (1<<1) -#define __I915_EXEC_FENCE_UNKNOWN_FLAGS (-(I915_EXEC_FENCE_SIGNAL << 1)) +#define I915_EXEC_FENCE_WAIT (1u << 0) +#define I915_EXEC_FENCE_SIGNAL (1u << 1) +#define I915_EXEC_FENCE_WAIT_SUBMIT (1u << 2) +#define __I915_EXEC_FENCE_UNKNOWN_FLAGS (-(I915_EXEC_FENCE_WAIT_SUBMIT << 1)) __u32 flags; };
Fixes: a88b6e4cbafd ("drm/i915: Allow specification of parallel execbuf") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com> --- drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 10 +++++++--- include/uapi/drm/i915_drm.h | 7 ++++--- 2 files changed, 11 insertions(+), 6 deletions(-)