[Mesa-dev] [PATCH 09/10] anv: Implement VK_ANDROID_native_buffer (v9)

Jason Ekstrand jason at jlekstrand.net
Tue Oct 17 00:55:28 UTC 2017


I made some fairly straightforward comments.  with that, and the squash you
sent, 1-6 and 8-10 are

Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>

I'd like to see the respin of patch 7 but I fully expect to give it my R-B
almost immediately.

--Jason

On Mon, Oct 16, 2017 at 11:55 AM, Chad Versace <chadversary at chromium.org>
wrote:

> This implementation is correct (afaict), but takes two shortcuts
> regarding the import/export of Android sync fds.
>
>   Shortcut 1. When Android calls vkAcquireImageANDROID to import a sync
>   fd into a VkSemaphore or VkFence, the driver instead simply blocks on
>   the sync fd, then puts the VkSemaphore or VkFence into the signalled
>   state. Thanks to implicit sync, this produces correct behavior (with
>   extra latency overhead, perhaps) despite its ugliness.
>
>   Shortcut 2. When Android calls vkQueueSignalReleaseImageANDROID to
> export
>   a collection of wait semaphores as a sync fd, the driver instead
>   submits the semaphores to the queue, then returns sync fd -1, which
>   informs the caller that no additional synchronization is needed.
>   Again, thanks to implicit sync, this produces correct behavior (with
>   extra batch submission overhead) despite its ugliness.
>
> I chose to take the shortcuts instead of properly importing/exporting
> the sync fds for two reasons:
>
>   Reason 1. I've already tested this patch with dEQP and with demos
>   apps. It works. I wanted to get the tested patches into the tree now,
>   and polish the implementation afterwards.
>
>   Reason 2. I want to run this on a 3.18 kernel (gasp!). In 3.18, i915
>   supports neither Android's sync_fence, nor upstream's sync_file, nor
>   drm_syncobj. Again, I tested these patches on Android with a 3.18
>   kernel and they work.
>
> I plan to quickly follow-up with patches that remove the shortcuts and
> properly import/export the sync fds.
>
> Non-Testing
> ===========
> I did not test at all using the Android.mk buildsystem. I probably
> broke it. Please test and review that.
>
> Testing
> =======
> I tested with 64-bit ARC++ on a Skylake Chromebook and a 3.18 kernel.
> The following pass (as of patchset v9):
>
>   - a little spinning cube demo APK
>   - several Sascha demos
>   - dEQP-VK.info.*
>   - dEQP-VK.api.wsi.android.*
>       (except dEQP-VK.api.wsi.android.swapchain.*.image_usage, because
>       dEQP wants to create swapchains with VK_IMAGE_USAGE_STORAGE_BIT)
>   - dEQP-VK.api.smoke.*
>   - dEQP-VK.api.info.instance.*
>   - dEQP-VK.api.info.device.*
>
> v2:
>   - Reject VkNativeBufferANDROID if the dma-buf's size is too small for
>     the VkImage.
>   - Stop abusing VkNativeBufferANDROID by passing it to vkAllocateMemory
>     during vkCreateImage. Instead, directly import its dma-buf during
>     vkCreateImage with anv_bo_cache_import(). [for jekstrand]
>   - Rebase onto Tapani's VK_EXT_debug_report changes.
>   - Drop `CPPFLAGS += $(top_srcdir)/include/android`. The dir does not
>     exist.
>
> v3:
>   - Delete duplicate #include "anv_private.h". [per Tapani]
>   - Try to fix the Android-IA build in Android.vulkan.mk by following
>     Tapani's example.
>
> v4:
>   - Unset EXEC_OBJECT_ASYNC and set EXEC_OBJECT_WRITE on the imported
>     gralloc buffer, just as we do for all other winsys buffers in
>     anv_wsi.c. [found by Tapani]
>
> v5:
>   - Really fix the Android-IA build by ensuring that Android.vulkan.mk
>     uses Mesa' vulkan.h and not Android's.  Insert -I$(MESA_TOP)/include
>     before -Iframeworks/native/vulkan/include. [for Tapani]
>   - In vkAcquireImageANDROID, submit signal operations to the
>     VkSemaphore and VkFence. [for zhou]
>
> v6:
>   - Drop copy-paste duplication in vkGetSwapchainGrallocUsageANDROID().
>     [found by zhou]
>   - Improve comments in vkGetSwapchainGrallocUsageANDROID().
>
> v7:
>   - Fix vkGetSwapchainGrallocUsageANDROID() to inspect its
>     VkImageUsageFlags parameter. [for tfiga]
>   - This fix regresses dEQP-VK.api.wsi.android.swapchain.*.image_usage
>     because dEQP wants to create swapchains with
>     VK_IMAGE_USAGE_STORAGE_BIT.
>
> v8:
>   - Drop unneeded goto in vkAcquireImageANDROID. [for tfiga]
>
> v8.1: (minor changes)
>   - Drop errant hunks added by rerere in anv_device.c.
>   - Drop explicit mention of VK_ANDROID_native_buffer in
>     anv_entrypoints_gen.py. [for jekstrand]
>
> v9:
>   - Isolate as much Android code as possible, moving it from anv_image.c
>     to anv_android.c. Connect the files with anv_image_from_gralloc().
>     Remove VkNativeBufferANDROID params from all anv_image.c
>     funcs. [for krh]
>   - Replace some intel_loge() with vk_errorf() in anv_android.c.
>   - Use © in copyright line. [for krh]
>
> Reviewed-by: Tapani Pälli <tapani.palli at intel.com> (v5)
> Cc: Jason Ekstrand <jason at jlekstrand.net>
> Cc: zhoucm1 <david1.zhou at amd.com>
> Cc: Tomasz Figa <tfiga at chromium.org>
> Cc: Kristian H. Kristensen <hoegsberg at google.com>
> ---
>  src/intel/Android.vulkan.mk             |   7 +-
>  src/intel/Makefile.sources              |   3 +
>  src/intel/Makefile.vulkan.am            |   2 +
>  src/intel/vulkan/anv_android.c          | 416
> ++++++++++++++++++++++++++++++++
>  src/intel/vulkan/anv_entrypoints_gen.py |   4 +-
>  src/intel/vulkan/anv_extensions.py      |   1 +
>  src/intel/vulkan/anv_image.c            |  20 +-
>  src/intel/vulkan/anv_private.h          |  12 +
>  8 files changed, 459 insertions(+), 6 deletions(-)
>  create mode 100644 src/intel/vulkan/anv_android.c
>
> diff --git a/src/intel/Android.vulkan.mk b/src/intel/Android.vulkan.mk
> index ee98e30c35..b9a0446f72 100644
> --- a/src/intel/Android.vulkan.mk
> +++ b/src/intel/Android.vulkan.mk
> @@ -28,6 +28,7 @@ VK_ENTRYPOINTS_SCRIPT := $(MESA_PYTHON2)
> $(LOCAL_PATH)/vulkan/anv_entrypoints_ge
>  VK_EXTENSIONS_SCRIPT := $(MESA_PYTHON2) $(LOCAL_PATH)/vulkan/anv_exten
> sions.py
>
>  VULKAN_COMMON_INCLUDES := \
> +       $(MESA_TOP)/include \
>         $(MESA_TOP)/src/mapi \
>         $(MESA_TOP)/src/gallium/auxiliary \
>         $(MESA_TOP)/src/gallium/include \
> @@ -36,7 +37,8 @@ VULKAN_COMMON_INCLUDES := \
>         $(MESA_TOP)/src/vulkan/util \
>         $(MESA_TOP)/src/intel \
>         $(MESA_TOP)/include/drm-uapi \
> -       $(MESA_TOP)/src/intel/vulkan
> +       $(MESA_TOP)/src/intel/vulkan \
> +       frameworks/native/vulkan/include
>
>  # libmesa_anv_entrypoints with header and dummy.c
>  #
> @@ -243,7 +245,8 @@ LOCAL_MODULE_CLASS := SHARED_LIBRARIES
>  LOCAL_LDFLAGS += -Wl,--build-id=sha1
>
>  LOCAL_SRC_FILES := \
> -       $(VULKAN_GEM_FILES)
> +       $(VULKAN_GEM_FILES) \
> +       $(VULKAN_ANDROID_FILES)
>
>  LOCAL_C_INCLUDES := \
>         $(VULKAN_COMMON_INCLUDES) \
> diff --git a/src/intel/Makefile.sources b/src/intel/Makefile.sources
> index da018dcc6a..5a09e6d916 100644
> --- a/src/intel/Makefile.sources
> +++ b/src/intel/Makefile.sources
> @@ -231,6 +231,9 @@ VULKAN_FILES := \
>         vulkan/anv_wsi.c \
>         vulkan/vk_format_info.h
>
> +VULKAN_ANDROID_FILES := \
> +       vulkan/anv_android.c
> +
>  VULKAN_WSI_WAYLAND_FILES := \
>         vulkan/anv_wsi_wayland.c
>
> diff --git a/src/intel/Makefile.vulkan.am b/src/intel/Makefile.vulkan.am
> index 0f488fc9f6..811faab556 100644
> --- a/src/intel/Makefile.vulkan.am
> +++ b/src/intel/Makefile.vulkan.am
> @@ -147,8 +147,10 @@ VULKAN_LIB_DEPS = \
>         -lm
>
>  if HAVE_PLATFORM_ANDROID
> +VULKAN_CPPFLAGS += $(ANDROID_CPPFLAGS)
>  VULKAN_CFLAGS += $(ANDROID_CFLAGS)
>  VULKAN_LIB_DEPS += $(ANDROID_LIBS)
> +VULKAN_SOURCES += $(VULKAN_ANDROID_FILES)
>  endif
>
>  if HAVE_PLATFORM_X11
> diff --git a/src/intel/vulkan/anv_android.c b/src/intel/vulkan/anv_android
> .c
> new file mode 100644
> index 0000000000..73baaf1f54
> --- /dev/null
> +++ b/src/intel/vulkan/anv_android.c
> @@ -0,0 +1,416 @@
> +/*
> + * Copyright © 2017 Google
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the
> "Software"),
> + * to deal in the Software without restriction, including without
> limitation
> + * the rights to use, copy, modify, merge, publish, distribute,
> sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the
> next
> + * paragraph) shall be included in all copies or substantial portions of
> the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT
> SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> DEALINGS
> + * IN THE SOFTWARE.
> + */
> +
> +#include <hardware/gralloc.h>
> +#include <hardware/hardware.h>
> +#include <hardware/hwvulkan.h>
> +#include <vulkan/vk_android_native_buffer.h>
> +#include <vulkan/vk_icd.h>
> +#include <sync/sync.h>
> +
> +#include "anv_private.h"
> +
> +static int anv_hal_open(const struct hw_module_t* mod, const char* id,
> struct hw_device_t** dev);
> +static int anv_hal_close(struct hw_device_t *dev);
> +
> +static void UNUSED
> +static_asserts(void)
> +{
> +   STATIC_ASSERT(HWVULKAN_DISPATCH_MAGIC == ICD_LOADER_MAGIC);
> +}
> +
> +PUBLIC struct hwvulkan_module_t HAL_MODULE_INFO_SYM = {
> +   .common = {
> +      .tag = HARDWARE_MODULE_TAG,
> +      .module_api_version = HWVULKAN_MODULE_API_VERSION_0_1,
> +      .hal_api_version = HARDWARE_MAKE_API_VERSION(1, 0),
> +      .id = HWVULKAN_HARDWARE_MODULE_ID,
> +      .name = "Intel Vulkan HAL",
> +      .author = "Intel",
> +      .methods = &(hw_module_methods_t) {
> +         .open = anv_hal_open,
> +      },
> +   },
> +};
> +
> +/* If any bits in test_mask are set, then unset them and return true. */
> +static inline bool
> +unmask32(uint32_t *inout_mask, uint32_t test_mask)
> +{
> +   uint32_t orig_mask = *inout_mask;
> +   *inout_mask &= ~test_mask;
> +   return *inout_mask != orig_mask;
> +}
> +
> +static int
> +anv_hal_open(const struct hw_module_t* mod, const char* id,
> +             struct hw_device_t** dev)
> +{
> +   assert(mod == &HAL_MODULE_INFO_SYM.common);
> +   assert(strcmp(id, HWVULKAN_DEVICE_0) == 0);
> +
> +   hwvulkan_device_t *hal_dev = malloc(sizeof(*hal_dev));
> +   if (!hal_dev)
> +      return -1;
> +
> +   *hal_dev = (hwvulkan_device_t) {
> +      .common = {
> +         .tag = HARDWARE_DEVICE_TAG,
> +         .version = HWVULKAN_DEVICE_API_VERSION_0_1,
> +         .module = &HAL_MODULE_INFO_SYM.common,
> +         .close = anv_hal_close,
> +      },
> +     .EnumerateInstanceExtensionProperties =
> anv_EnumerateInstanceExtensionProperties,
> +     .CreateInstance = anv_CreateInstance,
> +     .GetInstanceProcAddr = anv_GetInstanceProcAddr,
> +   };
> +
> +   *dev = &hal_dev->common;
> +   return 0;
> +}
> +
> +static int
> +anv_hal_close(struct hw_device_t *dev)
> +{
> +   /* hwvulkan.h claims that hw_device_t::close() is never called. */
> +   return -1;
> +}
> +
> +VkResult
> +anv_image_from_gralloc(VkDevice device_h,
> +                       const VkImageCreateInfo *base_info,
> +                       const VkNativeBufferANDROID *gralloc_info,
> +                       const VkAllocationCallbacks *alloc,
> +                       VkImage *out_image_h)
> +
> +{
> +   ANV_FROM_HANDLE(anv_device, device, device_h);
> +   VkImage image_h = VK_NULL_HANDLE;
> +   struct anv_image *image = NULL;
> +   struct anv_bo *bo = NULL;
> +   VkResult result;
> +
> +   struct anv_image_create_info anv_info = {
> +      .vk_info = base_info,
> +      .isl_extra_usage_flags = ISL_SURF_USAGE_DISABLE_AUX_BIT,
> +   };
> +
> +   if (gralloc_info->handle->numFds != 1) {
> +      return vk_errorf(device->instance, device,
> +                       VK_ERROR_INVALID_EXTERNAL_HANDLE_KHR,
> +                       "VkNativeBufferANDROID::handle::numFds is %d, "
> +                       "expected 1", gralloc_info->handle->numFds);
> +   }
> +
> +   /* Do not close the gralloc handle's dma_buf. The lifetime of the
> dma_buf
> +    * must exceed that of the gralloc handle, and we do not own the
> gralloc
> +    * handle.
> +    */
> +   int dma_buf = gralloc_info->handle->data[0];
> +
> +   result = anv_bo_cache_import(device, &device->bo_cache, dma_buf, &bo);
> +   if (result != VK_SUCCESS) {
> +      return vk_errorf(device->instance, device, result,
> +                       "failed to import dma-buf from
> VkNativeBufferANDROID");
> +   }
> +
> +   int i915_tiling = anv_gem_get_tiling(device, bo->gem_handle);
> +   switch (i915_tiling) {
> +   case I915_TILING_NONE:
> +      anv_info.isl_tiling_flags = ISL_TILING_LINEAR_BIT;
> +      break;
> +   case I915_TILING_X:
> +      anv_info.isl_tiling_flags = ISL_TILING_X_BIT;
> +      break;
> +   case I915_TILING_Y:
> +      anv_info.isl_tiling_flags = ISL_TILING_Y0_BIT;
>

We have isl_tiling_for_i915_tiling to do this.  You may still want the
switch statement for error handling purposes but I'm not a huge fan of
repeated code.  How about

if (i915_tiling < 0) {
   /* Do the one error */
} else if (i915_tiling > I915_TILING_Y) {
   /* Do the other error */
}
anv_info.isl_tiling_flags = 1u << isl_tiling_from_i915_tiling(i915_tiling);

It's not really less code but it keeps things centralized.


> +      break;
> +   case -1:
> +      result = vk_errorf(device->instance, device,
> +                         VK_ERROR_INVALID_EXTERNAL_HANDLE_KHR,
> +                         "DRM_IOCTL_I915_GEM_GET_TILING failed for "
> +                         "VkNativeBufferANDROID");
> +      goto fail_tiling;
> +   default:
> +      result = vk_errorf(device->instance, device,
> +                         VK_ERROR_INVALID_EXTERNAL_HANDLE_KHR,
> +                         "DRM_IOCTL_I915_GEM_GET_TILING returned unknown
> "
> +                         "tiling %d for VkNativeBufferANDROID",
> i915_tiling);
> +      goto fail_tiling;
> +   }
> +
> +   enum isl_format format = anv_get_isl_format(&device->info,
> +                                               base_info->format,
> +                                               VK_IMAGE_ASPECT_COLOR_BIT,
> +                                               base_info->tiling);
>

Is base_info->tiling allowed to be anything other than OPTIMAL?  Not that
it really matters...


> +   assert(format != ISL_FORMAT_UNSUPPORTED);
> +
> +   anv_info.stride = gralloc_info->stride *
> +                     (isl_format_get_layout(format)->bpb / 8);
> +
> +   result = anv_image_create(device_h, &anv_info, alloc, &image_h);
> +   image = anv_image_from_handle(image_h);
> +   if (result != VK_SUCCESS)
> +      goto fail_create;
> +
> +   if (bo->size < image->size) {
> +      result = vk_errorf(device, device->instance,
> +                         VK_ERROR_INVALID_EXTERNAL_HANDLE_KHR,
> +                         "dma-buf from VkNativeBufferANDROID is too small
> for "
> +                         "VkImage: %"PRIu64" < %"PRIu64,
> +                         bo->size, image->size);
> +      goto fail_size;
> +   }
> +
> +   assert(image->n_planes == 1);
> +   assert(image->planes[0].bo_offset == 0);
> +
> +   image->planes[0].bo = bo;
> +   image->planes[0].bo_is_owned = true;
> +
> +   /* We need to set the WRITE flag on window system buffers so that GEM
> will
> +    * know we're writing to them and synchronize uses on other rings (for
> +    * example, if the display server uses the blitter ring).
> +    *
> +    * If this function fails and if the imported bo was resident in the
> cache,
> +    * we should avoid updating the bo's flags. Therefore, we defer
> updating
> +    * the flags until success is certain.
> +    *
> +    */
> +   bo->flags &= ~EXEC_OBJECT_ASYNC;
> +   bo->flags |= EXEC_OBJECT_WRITE;
>

If Android is using sync_file, we shouldn't need this?  Maybe we need it
now and can delete it later?  I just read the semaphore code below and
answered my own question.


> +
> +   /* Don't clobber the out-parameter until success is certain. */
> +   *out_image_h = image_h;
> +
> +   return VK_SUCCESS;
> +
> + fail_size:
> +   anv_DestroyImage(device_h, image_h, alloc);
> + fail_create:
> + fail_tiling:
> +   anv_bo_cache_release(device, &device->bo_cache, bo);
> +
> +   return result;
> +}
> +
> +VkResult anv_GetSwapchainGrallocUsageANDROID(
> +    VkDevice            device_h,
> +    VkFormat            format,
> +    VkImageUsageFlags   imageUsage,
> +    int*                grallocUsage)
> +{
> +   ANV_FROM_HANDLE(anv_device, device, device_h);
> +   struct anv_physical_device *phys_dev = &device->instance->physicalDev
> ice;
> +   VkPhysicalDevice phys_dev_h = anv_physical_device_to_handle(phys_dev);
> +   VkResult result;
> +
> +   *grallocUsage = 0;
> +   intel_logd("%s: format=%d, usage=0x%x", __func__, format, imageUsage);
> +
> +   /* WARNING: Android Nougat's libvulkan.so hardcodes the
> VkImageUsageFlags
> +    * returned to applications via VkSurfaceCapabilitiesKHR::supp
> ortedUsageFlags.
> +    * The relevant code in libvulkan/swapchain.cpp contains this fun
> comment:
> +    *
> +    *     TODO(jessehall): I think these are right, but haven't thought
> hard
> +    *     about it. Do we need to query the driver for support of any of
> +    *     these?
> +    *
> +    * Any disagreement between this function and the hardcoded
> +    * VkSurfaceCapabilitiesKHR:supportedUsageFlags causes tests
> +    * dEQP-VK.wsi.android.swapchain.*.image_usage to fail.
> +    */
> +
> +   const VkPhysicalDeviceImageFormatInfo2KHR image_format_info = {
> +      .sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_IMAGE_FORMAT_INFO_2_KHR,
> +      .format = format,
> +      .type = VK_IMAGE_TYPE_2D,
> +      .tiling = VK_IMAGE_TILING_OPTIMAL,
> +      .usage = imageUsage,
> +   };
> +
> +   VkImageFormatProperties2KHR image_format_props = {
> +      .sType = VK_STRUCTURE_TYPE_IMAGE_FORMAT_PROPERTIES_2_KHR,
> +   };
> +
> +   /* Check that requested format and usage are supported. */
> +   result = anv_GetPhysicalDeviceImageFormatProperties2KHR(phys_dev_h,
> +               &image_format_info, &image_format_props);
> +   if (result != VK_SUCCESS) {
> +      return vk_errorf(device->instance, device, result,
> +                       "anv_GetPhysicalDeviceImageFormatProperties2KHR
> failed "
> +                       "inside %s", __func__);
> +   }
> +
> +   /* Reject STORAGE here to avoid complexity elsewhere. */
> +   if (imageUsage & VK_IMAGE_USAGE_STORAGE_BIT) {
> +      return vk_errorf(device->instance, device,
> VK_ERROR_FORMAT_NOT_SUPPORTED,
> +                       "VK_IMAGE_USAGE_STORAGE_BIT unsupported for
> gralloc "
> +                       "swapchain");
> +   }
> +
> +   if (unmask32(&imageUsage, VK_IMAGE_USAGE_TRANSFER_DST_BIT |
> +                             VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT))
> +      *grallocUsage |= GRALLOC_USAGE_HW_RENDER;
> +
> +   if (unmask32(&imageUsage, VK_IMAGE_USAGE_TRANSFER_SRC_BIT |
> +                             VK_IMAGE_USAGE_SAMPLED_BIT |
> +                             VK_IMAGE_USAGE_INPUT_ATTACHMENT_BIT))
> +      *grallocUsage |= GRALLOC_USAGE_HW_TEXTURE;
> +
> +   /* All VkImageUsageFlags not explicitly checked here are unsupported
> for
> +    * gralloc swapchains.
> +    */
> +   if (imageUsage != 0) {
> +      return vk_errorf(device->instance, device,
> VK_ERROR_FORMAT_NOT_SUPPORTED,
> +                       "unsupported VkImageUsageFlags(0x%x) for gralloc "
> +                       "swapchain", imageUsage);
> +   }
> +
> +   /* The below formats support GRALLOC_USAGE_HW_FB (that is, display
> +    * scanout). This short list of formats is univserally supported on
> Intel
> +    * but is incomplete.  The full set of supported formats is dependent
> on
> +    * kernel and hardware.
> +    *
> +    * FINISHME: Advertise all display-supported formats.
> +    */
> +   if (format == VK_FORMAT_B8G8R8A8_UNORM ||
> +       format == VK_FORMAT_B5G6R5_UNORM_PACK16) {
> +      *grallocUsage |= GRALLOC_USAGE_HW_FB |
> +                       GRALLOC_USAGE_HW_COMPOSER |
> +                       GRALLOC_USAGE_EXTERNAL_DISP;
> +   }
> +
> +   if (*grallocUsage == 0)
> +      return VK_ERROR_FORMAT_NOT_SUPPORTED;
> +
> +   return VK_SUCCESS;
> +}
> +
> +VkResult
> +anv_AcquireImageANDROID(
> +      VkDevice            device_h,
> +      VkImage             image_h,
> +      int                 nativeFenceFd,
> +      VkSemaphore         semaphore_h,
> +      VkFence             fence_h)
> +{
> +   ANV_FROM_HANDLE(anv_device, device, device_h);
> +   VkResult result = VK_SUCCESS;
> +
> +   if (nativeFenceFd != -1) {
> +      /* As a simple, firstpass implementation of
> VK_ANDROID_native_buffer, we
> +       * block on the nativeFenceFd. This may introduce latency and is
> +       * definitiely inefficient, yet it's correct.
> +       *
> +       * FINISHME(chadv): Import the nativeFenceFd into the VkSemaphore
> and
> +       * VkFence.
> +       */
> +      if (sync_wait(nativeFenceFd, /*timeout*/ -1) < 0) {
> +         result = vk_errorf(device->instance, device,
> VK_ERROR_DEVICE_LOST,
> +                            "%s: failed to wait on nativeFenceFd=%d",
> +                            __func__, nativeFenceFd);
> +      }
> +
> +      /* From VK_ANDROID_native_buffer's pseudo spec
> +       * (https://source.android.com/devices/graphics/implement-vulkan):
> +       *
> +       *    The driver takes ownership of the fence fd and is responsible
> for
> +       *    closing it [...] even if vkAcquireImageANDROID fails and
> returns
> +       *    an error.
> +       */
> +      close(nativeFenceFd);
> +
> +      if (result != VK_SUCCESS)
> +         return result;
> +   }
> +
> +   if (semaphore_h || fence_h) {
> +      /* Thanks to implicit sync, the image is ready for GPU access.  But
> we
> +       * must still put the semaphore into the "submit" state; otherwise
> the
> +       * client may get unexpected behavior if the client later uses it as
> +       * a wait semaphore.
> +       *
> +       * Because we blocked above on the nativeFenceFd, the image is also
> +       * ready for foreign-device access (including CPU access). But we
> must
> +       * still signal the fence; otherwise the client may get unexpected
> +       * behavior if the client later waits on it.
> +       *
> +       * For some values of anv_semaphore_type, we must submit the
> semaphore
> +       * to execbuf in order to signal it.  Likewise for anv_fence_type.
> +       * Instead of open-coding here the signal operation for each
> +       * anv_semaphore_type and anv_fence_type, we piggy-back on
> +       * vkQueueSubmit.
> +       */
>

This makes me sad.... Also, it works.


> +      const VkSubmitInfo submit = {
> +         .sType = VK_STRUCTURE_TYPE_SUBMIT_INFO,
> +         .waitSemaphoreCount = 0,
> +         .commandBufferCount = 0,
> +         .signalSemaphoreCount = (semaphore_h ? 1 : 0),
> +         .pSignalSemaphores = &semaphore_h,
> +      };
> +
> +      result = anv_QueueSubmit(anv_queue_to_handle(&device->queue), 1,
> +                               &submit, fence_h);
> +      if (result != VK_SUCCESS) {
> +         return vk_errorf(device->instance, device, result,
> +                          "anv_QueueSubmit failed inside %s", __func__);
> +      }
> +   }
> +
> +   return VK_SUCCESS;
> +}
> +
> +VkResult
> +anv_QueueSignalReleaseImageANDROID(
> +      VkQueue             queue,
> +      uint32_t            waitSemaphoreCount,
> +      const VkSemaphore*  pWaitSemaphores,
> +      VkImage             image,
> +      int*                pNativeFenceFd)
> +{
> +   VkResult result;
> +
> +   if (waitSemaphoreCount == 0)
> +      goto done;
> +
> +   result = anv_QueueSubmit(queue, 1,
> +      &(VkSubmitInfo) {
> +            .sType = VK_STRUCTURE_TYPE_SUBMIT_INFO,
> +            .waitSemaphoreCount = 1,
> +            .pWaitSemaphores = pWaitSemaphores,
> +      },
> +      (VkFence) VK_NULL_HANDLE);
> +   if (result != VK_SUCCESS)
> +      return result;
> +
> + done:
> +   if (pNativeFenceFd) {
> +      /* We can rely implicit on sync because above we submitted all
> +       * semaphores to the queue.
> +       */
> +      *pNativeFenceFd = -1;
> +   }
> +
> +   return VK_SUCCESS;
> +}
> diff --git a/src/intel/vulkan/anv_entrypoints_gen.py
> b/src/intel/vulkan/anv_entrypoints_gen.py
> index bf376a881f..334b6de54b 100644
> --- a/src/intel/vulkan/anv_entrypoints_gen.py
> +++ b/src/intel/vulkan/anv_entrypoints_gen.py
> @@ -270,7 +270,9 @@ def get_entrypoints(doc, entrypoints_to_defines,
> start_index):
>          if extension.attrib['name'] not in supported:
>              continue
>
> -        assert extension.attrib['supported'] == 'vulkan'
> +        if extension.attrib['supported'] != 'vulkan':
> +            continue
> +
>          for command in extension.findall('./require/command'):
>              enabled_commands.add(command.attrib['name'])
>
> diff --git a/src/intel/vulkan/anv_extensions.py
> b/src/intel/vulkan/anv_extensions.py
> index a828a668d6..b1e984b8cd 100644
> --- a/src/intel/vulkan/anv_extensions.py
> +++ b/src/intel/vulkan/anv_extensions.py
> @@ -50,6 +50,7 @@ class Extension:
>  # the those extension strings, then tests dEQP-VK.api.info.instance.exte
> nsions
>  # and dEQP-VK.api.info.device fail due to the duplicated strings.
>  EXTENSIONS = [
> +    Extension('VK_ANDROID_native_buffer',                 5, 'ANDROID'),
>      Extension('VK_KHR_bind_memory2',                      1, True),
>      Extension('VK_KHR_dedicated_allocation',              1, True),
>      Extension('VK_KHR_descriptor_update_template',        1, True),
> diff --git a/src/intel/vulkan/anv_image.c b/src/intel/vulkan/anv_image.c
> index b80011e222..cd96bfd0d6 100644
> --- a/src/intel/vulkan/anv_image.c
> +++ b/src/intel/vulkan/anv_image.c
> @@ -40,9 +40,10 @@
>  static isl_surf_usage_flags_t
>  choose_isl_surf_usage(VkImageCreateFlags vk_create_flags,
>                        VkImageUsageFlags vk_usage,
> +                      isl_surf_usage_flags_t isl_extra_usage,
>                        VkImageAspectFlags aspect)
>  {
> -   isl_surf_usage_flags_t isl_usage = 0;
> +   isl_surf_usage_flags_t isl_usage = isl_extra_usage;
>
>     if (vk_usage & VK_IMAGE_USAGE_SAMPLED_BIT)
>        isl_usage |= ISL_SURF_USAGE_TEXTURE_BIT;
> @@ -290,6 +291,10 @@ make_surface(const struct anv_device *dev,
>        anv_get_format_plane(&dev->info, image->vk_format, aspect,
> image->tiling);
>     struct anv_surface *anv_surf = &image->planes[plane].surface;
>
> +   const isl_surf_usage_flags_t usage =
> +      choose_isl_surf_usage(vk_info->flags, image->usage,
> +                            anv_info->isl_extra_usage_flags, aspect);
> +
>     /* If an image is created as BLOCK_TEXEL_VIEW_COMPATIBLE, then we need
> to
>      * fall back to linear on Broadwell and earlier because we aren't
>      * guaranteed that we can handle offsets correctly.  On Sky Lake, the
> @@ -316,7 +321,7 @@ make_surface(const struct anv_device *dev,
>        .samples = vk_info->samples,
>        .min_alignment = 0,
>        .row_pitch = anv_info->stride,
> -      .usage = choose_isl_surf_usage(vk_info->flags, image->usage,
> aspect),
> +      .usage = usage,
>        .tiling_flags = tiling_flags);
>
>     /* isl_surf_init() will fail only if provided invalid input. Invalid
> input
> @@ -347,7 +352,7 @@ make_surface(const struct anv_device *dev,
>           .samples = vk_info->samples,
>           .min_alignment = 0,
>           .row_pitch = anv_info->stride,
> -         .usage = choose_isl_surf_usage(image->usage, image->usage,
> aspect),
> +         .usage = usage,
>           .tiling_flags = ISL_TILING_ANY_MASK);
>
>        /* isl_surf_init() will fail only if provided invalid input.
> Invalid input
> @@ -547,6 +552,15 @@ anv_CreateImage(VkDevice device,
>                  const VkAllocationCallbacks *pAllocator,
>                  VkImage *pImage)
>  {
> +#ifdef ANDROID
> +   const VkNativeBufferANDROID *gralloc_info =
> +      vk_find_struct_const(pCreateInfo->pNext, NATIVE_BUFFER_ANDROID);
> +
> +   if (gralloc_info)
> +      return anv_image_from_gralloc(device, pCreateInfo, gralloc_info,
> +                                    pAllocator, pImage);
> +#endif
> +
>     return anv_image_create(device,
>        &(struct anv_image_create_info) {
>           .vk_info = pCreateInfo,
> diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private
> .h
> index b51d110514..36c7220679 100644
> --- a/src/intel/vulkan/anv_private.h
> +++ b/src/intel/vulkan/anv_private.h
> @@ -69,6 +69,7 @@ struct gen_l3_config;
>  #include <vulkan/vulkan.h>
>  #include <vulkan/vulkan_intel.h>
>  #include <vulkan/vk_icd.h>
> +#include <vulkan/vk_android_native_buffer.h>
>
>  #include "anv_entrypoints.h"
>  #include "isl/isl.h"
> @@ -2632,6 +2633,9 @@ struct anv_image_create_info {
>     /** An opt-in bitmask which filters an ISL-mapping of the Vulkan
> tiling. */
>     isl_tiling_flags_t isl_tiling_flags;
>
> +   /** These flags will be added to any derived from VkImageCreateInfo. */
> +   isl_surf_usage_flags_t isl_extra_usage_flags;
> +
>     uint32_t stride;
>  };
>
> @@ -2640,6 +2644,14 @@ VkResult anv_image_create(VkDevice _device,
>                            const VkAllocationCallbacks* alloc,
>                            VkImage *pImage);
>
> +#ifdef ANDROID
> +VkResult anv_image_from_gralloc(VkDevice device_h,
> +                                const VkImageCreateInfo *base_info,
> +                                const VkNativeBufferANDROID *gralloc_info,
> +                                const VkAllocationCallbacks *alloc,
> +                                VkImage *pImage);
> +#endif
> +
>  const struct anv_surface *
>  anv_image_get_surface_for_aspect_mask(const struct anv_image *image,
>                                        VkImageAspectFlags aspect_mask);
> --
> 2.13.0
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20171016/c16acfb0/attachment-0001.html>


More information about the mesa-dev mailing list