[Mesa-dev] [PATCH v3 4/8] anv: Implement support for exporting semaphores as FENCE_FD

Lionel Landwerlin lionel.g.landwerlin at intel.com
Sun Aug 13 11:37:23 UTC 2017


On 04/08/17 18:24, Jason Ekstrand wrote:
> ---
>   src/intel/vulkan/anv_batch_chain.c | 57 +++++++++++++++++++++++++++++--
>   src/intel/vulkan/anv_device.c      |  1 +
>   src/intel/vulkan/anv_gem.c         | 36 ++++++++++++++++++++
>   src/intel/vulkan/anv_private.h     | 23 +++++++++----
>   src/intel/vulkan/anv_queue.c       | 69 ++++++++++++++++++++++++++++++++++++--
>   5 files changed, 175 insertions(+), 11 deletions(-)
>
> diff --git a/src/intel/vulkan/anv_batch_chain.c b/src/intel/vulkan/anv_batch_chain.c
> index 65fe366..7a84bbd 100644
> --- a/src/intel/vulkan/anv_batch_chain.c
> +++ b/src/intel/vulkan/anv_batch_chain.c
> @@ -1416,11 +1416,13 @@ anv_cmd_buffer_execbuf(struct anv_device *device,
>      struct anv_execbuf execbuf;
>      anv_execbuf_init(&execbuf);
>   
> +   int in_fence = -1;
>      VkResult result = VK_SUCCESS;
>      for (uint32_t i = 0; i < num_in_semaphores; i++) {
>         ANV_FROM_HANDLE(anv_semaphore, semaphore, in_semaphores[i]);
> -      assert(semaphore->temporary.type == ANV_SEMAPHORE_TYPE_NONE);
> -      struct anv_semaphore_impl *impl = &semaphore->permanent;
> +      struct anv_semaphore_impl *impl =
> +         semaphore->temporary.type != ANV_SEMAPHORE_TYPE_NONE ?
> +         &semaphore->temporary : &semaphore->permanent;

I know you're not enabling this until patch 8, but for consistency, 
shouldn't this be part of patch 1?

>   
>         switch (impl->type) {
>         case ANV_SEMAPHORE_TYPE_BO:
> @@ -1429,11 +1431,29 @@ anv_cmd_buffer_execbuf(struct anv_device *device,
>            if (result != VK_SUCCESS)
>               return result;
>            break;
> +
> +      case ANV_SEMAPHORE_TYPE_SYNC_FILE:
> +         if (in_fence == -1) {
> +            in_fence = impl->fd;
> +         } else {
> +            int merge = anv_gem_sync_file_merge(device, in_fence, impl->fd);
> +            if (merge == -1)
> +               return vk_error(VK_ERROR_INVALID_EXTERNAL_HANDLE_KHR);
> +
> +            close(impl->fd);
> +            close(in_fence);
> +            in_fence = merge;
> +         }
> +
> +         impl->fd = -1;
> +         break;
> +
>         default:
>            break;
>         }
>      }
>   
> +   bool need_out_fence = false;
>      for (uint32_t i = 0; i < num_out_semaphores; i++) {
>         ANV_FROM_HANDLE(anv_semaphore, semaphore, out_semaphores[i]);
>   
> @@ -1459,6 +1479,11 @@ anv_cmd_buffer_execbuf(struct anv_device *device,
>            if (result != VK_SUCCESS)
>               return result;
>            break;
> +
> +      case ANV_SEMAPHORE_TYPE_SYNC_FILE:
> +         need_out_fence = true;
> +         break;
> +
>         default:
>            break;
>         }
> @@ -1472,9 +1497,19 @@ anv_cmd_buffer_execbuf(struct anv_device *device,
>         setup_empty_execbuf(&execbuf, device);
>      }
>   
> +   if (in_fence != -1) {
> +      execbuf.execbuf.flags |= I915_EXEC_FENCE_IN;
> +      execbuf.execbuf.rsvd2 |= (uint32_t)in_fence;
> +   }
> +
> +   if (need_out_fence)
> +      execbuf.execbuf.flags |= I915_EXEC_FENCE_OUT;
>   
>      result = anv_device_execbuf(device, &execbuf.execbuf, execbuf.bos);
>   
> +   /* Execbuf does not consume the in_fence.  It's our job to close it. */
> +   close(in_fence);
> +
>      for (uint32_t i = 0; i < num_in_semaphores; i++) {
>         ANV_FROM_HANDLE(anv_semaphore, semaphore, in_semaphores[i]);
>         /* From the Vulkan 1.0.53 spec:
> @@ -1489,6 +1524,24 @@ anv_cmd_buffer_execbuf(struct anv_device *device,
>         anv_semaphore_reset_temporary(device, semaphore);
>      }
>   
> +   if (result == VK_SUCCESS && need_out_fence) {
> +      int out_fence = execbuf.execbuf.rsvd2 >> 32;
> +      for (uint32_t i = 0; i < num_out_semaphores; i++) {
> +         ANV_FROM_HANDLE(anv_semaphore, semaphore, out_semaphores[i]);
> +         /* Out fences can't have temporary state because that would imply
> +          * that we imported a sync file and are trying to signal it.
> +          */
> +         assert(semaphore->temporary.type == ANV_SEMAPHORE_TYPE_NONE);
> +         struct anv_semaphore_impl *impl = &semaphore->permanent;
> +
> +         if (impl->type == ANV_SEMAPHORE_TYPE_SYNC_FILE) {
> +            assert(impl->fd == -1);
> +            impl->fd = dup(out_fence);
> +         }
> +      }
> +      close(out_fence);
> +   }
> +
>      anv_execbuf_finish(&execbuf, &device->alloc);
>   
>      return result;
> diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c
> index e82e1e9..3c5f78c 100644
> --- a/src/intel/vulkan/anv_device.c
> +++ b/src/intel/vulkan/anv_device.c
> @@ -337,6 +337,7 @@ anv_physical_device_init(struct anv_physical_device *device,
>         goto fail;
>   
>      device->has_exec_async = anv_gem_get_param(fd, I915_PARAM_HAS_EXEC_ASYNC);
> +   device->has_exec_fence = anv_gem_get_param(fd, I915_PARAM_HAS_EXEC_FENCE);
>   
>      bool swizzled = anv_gem_get_bit6_swizzle(fd, I915_TILING_X);
>   
> diff --git a/src/intel/vulkan/anv_gem.c b/src/intel/vulkan/anv_gem.c
> index 36692f5..5b68e9b 100644
> --- a/src/intel/vulkan/anv_gem.c
> +++ b/src/intel/vulkan/anv_gem.c
> @@ -22,6 +22,7 @@
>    */
>   
>   #include <sys/ioctl.h>
> +#include <sys/types.h>
>   #include <sys/mman.h>
>   #include <string.h>
>   #include <errno.h>
> @@ -400,3 +401,38 @@ anv_gem_fd_to_handle(struct anv_device *device, int fd)
>   
>      return args.handle;
>   }
> +
> +#ifndef SYNC_IOC_MAGIC
> +/* duplicated from linux/sync_file.h to avoid build-time depnedency

nit: dependency

> + * on new (v4.7) kernel headers.  Once distro's are mostly using
> + * something newer than v4.7 drop this and #include <linux/sync_file.h>
> + * instead.
> + */
> +struct sync_merge_data {
> +   char  name[32];
> +   __s32 fd2;
> +   __s32 fence;
> +   __u32 flags;
> +   __u32 pad;
> +};
> +
> +#define SYNC_IOC_MAGIC '>'
> +#define SYNC_IOC_MERGE _IOWR(SYNC_IOC_MAGIC, 3, struct sync_merge_data)
> +#endif
> +
> +int
> +anv_gem_sync_file_merge(struct anv_device *device, int fd1, int fd2)
> +{
> +   const char name[] = "anv merge fence";
> +   struct sync_merge_data args = {
> +      .fd2 = fd2,
> +      .fence = -1,
> +   };
> +   memcpy(args.name, name, sizeof(name));
> +
> +   int ret = anv_ioctl(fd1, SYNC_IOC_MERGE, &args);
> +   if (ret == -1)
> +      return -1;
> +
> +   return args.fence;
> +}
> diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h
> index bc67bb6..5c7b3b4 100644
> --- a/src/intel/vulkan/anv_private.h
> +++ b/src/intel/vulkan/anv_private.h
> @@ -652,6 +652,7 @@ struct anv_physical_device {
>       struct isl_device                           isl_dev;
>       int                                         cmd_parser_version;
>       bool                                        has_exec_async;
> +    bool                                        has_exec_fence;
>   
>       uint32_t                                    eu_total;
>       uint32_t                                    subslice_total;
> @@ -810,6 +811,7 @@ uint32_t anv_gem_fd_to_handle(struct anv_device *device, int fd);
>   int anv_gem_set_caching(struct anv_device *device, uint32_t gem_handle, uint32_t caching);
>   int anv_gem_set_domain(struct anv_device *device, uint32_t gem_handle,
>                          uint32_t read_domains, uint32_t write_domain);
> +int anv_gem_sync_file_merge(struct anv_device *device, int fd1, int fd2);
>   
>   VkResult anv_bo_init_new(struct anv_bo *bo, struct anv_device *device, uint64_t size);
>   
> @@ -1735,17 +1737,26 @@ enum anv_semaphore_type {
>      ANV_SEMAPHORE_TYPE_NONE = 0,
>      ANV_SEMAPHORE_TYPE_DUMMY,
>      ANV_SEMAPHORE_TYPE_BO,
> +   ANV_SEMAPHORE_TYPE_SYNC_FILE,
>   };
>   
>   struct anv_semaphore_impl {
>      enum anv_semaphore_type type;
>   
> -   /* A BO representing this semaphore when type == ANV_SEMAPHORE_TYPE_BO.
> -    * This BO will be added to the object list on any execbuf2 calls for
> -    * which this semaphore is used as a wait or signal fence.  When used as
> -    * a signal fence, the EXEC_OBJECT_WRITE flag will be set.
> -    */
> -   struct anv_bo *bo;
> +   union {
> +      /* A BO representing this semaphore when type == ANV_SEMAPHORE_TYPE_BO.
> +       * This BO will be added to the object list on any execbuf2 calls for
> +       * which this semaphore is used as a wait or signal fence.  When used as
> +       * a signal fence, the EXEC_OBJECT_WRITE flag will be set.
> +       */
> +      struct anv_bo *bo;
> +
> +      /* The sync file descriptor when type == AKV_SEMAPHORE_TYPE_SYNC_FILE.
> +       * If the semaphore is in the unsignaled state due to either just being
> +       * created or because it has been used for a wait, fd will be -1.
> +       */
> +      int fd;
> +   };
>   };
>   
>   struct anv_semaphore {
> diff --git a/src/intel/vulkan/anv_queue.c b/src/intel/vulkan/anv_queue.c
> index 039dfd7..1f27028 100644
> --- a/src/intel/vulkan/anv_queue.c
> +++ b/src/intel/vulkan/anv_queue.c
> @@ -571,6 +571,11 @@ VkResult anv_CreateSemaphore(
>          * EXEC_OBJECT_ASYNC bit set.
>          */
>         assert(!(semaphore->permanent.bo->flags & EXEC_OBJECT_ASYNC));
> +   } else if (handleTypes & VK_EXTERNAL_SEMAPHORE_HANDLE_TYPE_SYNC_FD_BIT_KHR) {
> +      assert(handleTypes == VK_EXTERNAL_SEMAPHORE_HANDLE_TYPE_SYNC_FD_BIT_KHR);
> +
> +      semaphore->permanent.type = ANV_SEMAPHORE_TYPE_SYNC_FILE;
> +      semaphore->permanent.fd = -1;
>      } else {
>         assert(!"Unknown handle type");
>         vk_free2(&device->alloc, pAllocator, semaphore);
> @@ -597,6 +602,10 @@ anv_semaphore_impl_cleanup(struct anv_device *device,
>      case ANV_SEMAPHORE_TYPE_BO:
>         anv_bo_cache_release(device, &device->bo_cache, impl->bo);
>         return;
> +
> +   case ANV_SEMAPHORE_TYPE_SYNC_FILE:
> +      close(impl->fd);
> +      return;
>      }
>   
>      unreachable("Invalid semaphore type");
> @@ -635,6 +644,8 @@ void anv_GetPhysicalDeviceExternalSemaphorePropertiesKHR(
>       const VkPhysicalDeviceExternalSemaphoreInfoKHR* pExternalSemaphoreInfo,
>       VkExternalSemaphorePropertiesKHR*           pExternalSemaphoreProperties)
>   {
> +   ANV_FROM_HANDLE(anv_physical_device, device, physicalDevice);
> +
>      switch (pExternalSemaphoreInfo->handleType) {
>      case VK_EXTERNAL_SEMAPHORE_HANDLE_TYPE_OPAQUE_FD_BIT_KHR:
>         pExternalSemaphoreProperties->exportFromImportedHandleTypes =
> @@ -644,13 +655,27 @@ void anv_GetPhysicalDeviceExternalSemaphorePropertiesKHR(
>         pExternalSemaphoreProperties->externalSemaphoreFeatures =
>            VK_EXTERNAL_SEMAPHORE_FEATURE_EXPORTABLE_BIT_KHR |
>            VK_EXTERNAL_SEMAPHORE_FEATURE_IMPORTABLE_BIT_KHR;
> +      return;
> +
> +   case VK_EXTERNAL_SEMAPHORE_HANDLE_TYPE_SYNC_FD_BIT_KHR:
> +      if (device->has_exec_fence) {
> +         pExternalSemaphoreProperties->exportFromImportedHandleTypes = 0;
> +         pExternalSemaphoreProperties->compatibleHandleTypes =
> +            VK_EXTERNAL_SEMAPHORE_HANDLE_TYPE_SYNC_FD_BIT_KHR;
> +         pExternalSemaphoreProperties->externalSemaphoreFeatures =
> +            VK_EXTERNAL_SEMAPHORE_FEATURE_EXPORTABLE_BIT_KHR |
> +            VK_EXTERNAL_SEMAPHORE_FEATURE_IMPORTABLE_BIT_KHR;
> +         return;
> +      }
>         break;
>   
>      default:
> -      pExternalSemaphoreProperties->exportFromImportedHandleTypes = 0;
> -      pExternalSemaphoreProperties->compatibleHandleTypes = 0;
> -      pExternalSemaphoreProperties->externalSemaphoreFeatures = 0;
> +      break;
>      }
> +
> +   pExternalSemaphoreProperties->exportFromImportedHandleTypes = 0;
> +   pExternalSemaphoreProperties->compatibleHandleTypes = 0;
> +   pExternalSemaphoreProperties->externalSemaphoreFeatures = 0;
>   }
>   
>   VkResult anv_ImportSemaphoreFdKHR(
> @@ -682,6 +707,13 @@ VkResult anv_ImportSemaphoreFdKHR(
>         break;
>      }
>   
> +   case VK_EXTERNAL_SEMAPHORE_HANDLE_TYPE_SYNC_FD_BIT_KHR:
> +      new_impl = (struct anv_semaphore_impl) {
> +         .type = ANV_SEMAPHORE_TYPE_SYNC_FILE,
> +         .fd = fd,
> +      };
> +      break;
> +
>      default:
>         return vk_error(VK_ERROR_INVALID_EXTERNAL_HANDLE_KHR);
>      }
> @@ -690,6 +722,9 @@ VkResult anv_ImportSemaphoreFdKHR(
>         anv_semaphore_impl_cleanup(device, &semaphore->temporary);
>         semaphore->temporary = new_impl;
>      } else {
> +      /* SYNC_FILE must be a temporary import */
> +      assert(new_impl.type != ANV_SEMAPHORE_TYPE_SYNC_FILE);
> +
>         anv_semaphore_impl_cleanup(device, &semaphore->permanent);
>         semaphore->permanent = new_impl;
>      }
> @@ -719,6 +754,34 @@ VkResult anv_GetSemaphoreFdKHR(
>            return result;
>         break;
>   
> +   case ANV_SEMAPHORE_TYPE_SYNC_FILE:
> +      /* There are two reasons why this could happen:
> +       *
> +       *  1) The user is trying to export without submitting something that
> +       *     signals the semaphore.  If this is the case, it's their bug so
> +       *     what we return here doesn't matter.
> +       *
> +       *  2) The kernel didn't give us a file descriptor.  The most likely
> +       *     reason for this is running out of file descriptors.
> +       */
> +      if (impl->fd < 0)
> +         return vk_error(VK_ERROR_TOO_MANY_OBJECTS);
> +
> +      *pFd = impl->fd;
> +
> +      /* From the Vulkan 1.0.53 spec:
> +       *
> +       *    "...exporting a semaphore payload to a handle with copy
> +       *    transference has the same side effects on the source
> +       *    semaphore’s payload as executing a semaphore wait operation."
> +       *
> +       * In other words, it may still be a SYNC_FD semaphore, but it's now
> +       * considered to have been waited on and no longer has a sync file
> +       * attached.
> +       */
> +      impl->fd = -1;
> +      return VK_SUCCESS;
> +
>      default:
>         return vk_error(VK_ERROR_INVALID_EXTERNAL_HANDLE_KHR);
>      }




More information about the mesa-dev mailing list