[09/10] drm/i915/gem: Allow combining submit-fences with syncobj
diff mbox series

Message ID 20200403091300.14734-9-chris@chris-wilson.co.uk
State New
Headers show
Series
  • [01/10] drm/i915/selftests: Add request throughput measurement to perf
Related show

Commit Message

Chris Wilson April 3, 2020, 9:12 a.m. UTC
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(-)

Comments

Tvrtko Ursulin April 7, 2020, 10:44 a.m. UTC | #1
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;
>   };
>   
>
Chris Wilson April 7, 2020, 10:51 a.m. UTC | #2
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
Tvrtko Ursulin April 8, 2020, 9:28 a.m. UTC | #3
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

Patch
diff mbox series

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;
 };