[Mesa-dev] [RFC] anv: Use DRM sync objects for external semaphores when available

[Chris Wilson]

On Wed, Apr 12, 2017 at 11:12:31AM -0700, Jason Ekstrand wrote:
> This patch is based on the work that's already been on the list for some
> time to implement VK_KHX_external_semaphore.  The difference here is that
> this patch makes us able to use the new DRM syncobj API cooked up by Dave
> Airlie.  This patch seems to work ok assuming a Kernel API I cooked up that
> has yet to get any review and very hackish kernel patch on top of Dave's
> stuff.  Mostly, I'm sending it to the list so that people can get an idea
> of what the implementation looks like so that we can get the ball rolling
> on getting the final kernel API in place.
> 
> Cc: Chad Versace <chadversary at chromium.org>
> Cc: Dave Airlie <airlied at redhat.com>
> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> 
> ---
>  src/intel/vulkan/anv_batch_chain.c | 72 +++++++++++++++++++++++++++++++++++
>  src/intel/vulkan/anv_device.c      |  3 ++
>  src/intel/vulkan/anv_private.h     |  8 ++++
>  src/intel/vulkan/anv_queue.c       | 77 ++++++++++++++++++++++++++++++--------
>  4 files changed, 144 insertions(+), 16 deletions(-)
> 
> diff --git a/src/intel/vulkan/anv_batch_chain.c b/src/intel/vulkan/anv_batch_chain.c
> index 0a86ae5..46c4078 100644
> --- a/src/intel/vulkan/anv_batch_chain.c
> +++ b/src/intel/vulkan/anv_batch_chain.c
> @@ -953,6 +953,21 @@ anv_cmd_buffer_add_secondary(struct anv_cmd_buffer *primary,
>                           &secondary->surface_relocs, 0);
>  }
>  
> +struct drm_i915_gem_exec_fence {
> +	/**
> +	 * User's handle for a dma-fence to wait on or signal.
> +	 */
> +	__u32 handle;
> +
> +	__u32 rsvd1;
> +
> +#define I915_EXEC_FENCE_WAIT            (1<<0)
> +#define I915_EXEC_FENCE_SIGNAL          (1<<1)
> +	__u64 flags;

u32 handle;
u32 flags;

Whether or not to leave u64 rsvd[]? But we can just as easily override
the semaphore array with new abi.

> +};
> +
> +#define I915_EXEC_FENCE_ARRAY   (1<<18)
> +
>  struct anv_execbuf {
>     struct drm_i915_gem_execbuffer2           execbuf;
>  
> @@ -962,6 +977,10 @@ struct anv_execbuf {
>  
>     /* Allocated length of the 'objects' and 'bos' arrays */
>     uint32_t                                  array_length;
> +
> +   uint32_t                                  fence_count;
> +   uint32_t                                  fence_array_length;
> +   struct drm_i915_gem_exec_fence *          fences;
>  };
>  
>  static void
> @@ -976,6 +995,7 @@ anv_execbuf_finish(struct anv_execbuf *exec,
>  {
>     vk_free(alloc, exec->objects);
>     vk_free(alloc, exec->bos);
> +   vk_free(alloc, exec->fences);
>  }
>  
>  static VkResult
> @@ -1040,6 +1060,9 @@ anv_execbuf_add_bo(struct anv_execbuf *exec,
>        obj->flags = flags | (bo->is_winsys_bo ? EXEC_OBJECT_WRITE : 0);
>        obj->rsvd1 = 0;
>        obj->rsvd2 = 0;

/* Vk provides explicit synchronisation primitives to the user, so
 * disable GEM's implicit synchronisation.
 *
 * Except for two cases:
 *
 *  (a) window system buffers are shared with an independent renderer,
 *      but we may not be sharing explicit fences and so rely on implicit
 *      synchronisation between ourselves;
 *
 *  (b) we build some explicit synchronisation primitives using writable
 *      bo i.e. exposing GEM's implicit synchronsiation as a first
 *      class Vk object.
 */

Something like that?

> +      if (!(obj->flags & EXEC_OBJECT_WRITE))
> +         obj->flags |= EXEC_OBJECT_ASYNC;
>     }
>  
>     if (relocs != NULL && obj->relocation_count == 0) {
> @@ -1060,6 +1083,33 @@ anv_execbuf_add_bo(struct anv_execbuf *exec,
>     return VK_SUCCESS;
>  }
>  
> +static VkResult
> +anv_execbuf_add_syncobj(struct anv_execbuf *exec,
> +                        uint32_t handle,
> +                        uint32_t flags,
> +                        const VkAllocationCallbacks *alloc)
> +{
> +   if (exec->fence_count >= exec->fence_array_length) {
> +      uint32_t new_len = MAX2(exec->fence_array_length * 2, 64);
> +
> +      struct drm_i915_gem_exec_fence *new_fences =
> +         vk_realloc(alloc, exec->fences, new_len * sizeof(*new_fences),
> +                    8, VK_SYSTEM_ALLOCATION_SCOPE_COMMAND);
> +      if (new_fences == NULL)
> +         return vk_error(VK_ERROR_OUT_OF_HOST_MEMORY);
> +
> +      exec->fences = new_fences;
> +      exec->fence_array_length = new_len;
> +   }
> +
> +   exec->fences[exec->fence_count] = (struct drm_i915_gem_exec_fence) {
> +      .handle = handle,
> +      .flags = flags,
> +   };
> +
> +   return VK_SUCCESS;
> +}
> +
>  static void
>  anv_cmd_buffer_process_relocs(struct anv_cmd_buffer *cmd_buffer,
>                                struct anv_reloc_list *list)
> @@ -1444,6 +1494,14 @@ anv_cmd_buffer_execbuf(struct anv_device *device,
>           impl->fd = -1;
>           break;
>  
> +      case ANV_SEMAPHORE_TYPE_DRM_SYNCOBJ:
> +         result = anv_execbuf_add_syncobj(&execbuf, impl->syncobj,
> +                                          I915_EXEC_FENCE_WAIT,
> +                                          &device->alloc);
> +         if (result != VK_SUCCESS)
> +            return result;
> +         break;
> +
>        default:
>           break;
>        }
> @@ -1478,6 +1536,14 @@ anv_cmd_buffer_execbuf(struct anv_device *device,
>           need_out_fence = true;
>           break;
>  
> +      case ANV_SEMAPHORE_TYPE_DRM_SYNCOBJ:
> +         result = anv_execbuf_add_syncobj(&execbuf, impl->syncobj,
> +                                          I915_EXEC_FENCE_SIGNAL,
> +                                          &device->alloc);
> +         if (result != VK_SUCCESS)
> +            return result;
> +         break;
> +
>        default:
>           break;
>        }
> @@ -1491,6 +1557,12 @@ anv_cmd_buffer_execbuf(struct anv_device *device,
>        setup_empty_execbuf(&execbuf, device);
>     }
>  
> +   if (execbuf.fence_count > 0) {
> +      execbuf.execbuf.flags |= I915_EXEC_FENCE_ARRAY;
> +      execbuf.execbuf.num_cliprects = execbuf.fence_count;
> +      execbuf.execbuf.cliprects_ptr = (uintptr_t) execbuf.fences;
> +   }

That looks very neat.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre