[Mesa-dev] [PATCH 20/20] anv: Implement VK_ANDROID_native_buffer (v2)

Chad Versace chadversary at chromium.org
Mon Sep 18 21:22:56 UTC 2017


On Fri 15 Sep 2017, zhoucm1 wrote:
> 
> 
> On 2017年09月14日 07:03, Chad Versace wrote:
> > From: Chad Versace <chadversary at chromium.org>
> > 
> > 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:
> > 
> >    a little spinning cube demo APK
> >    dEQP-VK.info.*
> >    dEQP-VK.api.smoke.*
> >    dEQP-VK.api.info.instance.*
> >    dEQP-VK.api.info.device.*
> >    dEQP-VK.api.wsi.android.*
> > 
> > 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.
> > ---
> >   src/intel/Makefile.sources              |   3 +
> >   src/intel/Makefile.vulkan.am            |   2 +
> >   src/intel/vulkan/anv_android.c          | 245 ++++++++++++++++++++++++++++++++
> >   src/intel/vulkan/anv_device.c           |  12 +-
> >   src/intel/vulkan/anv_entrypoints_gen.py |  10 +-
> >   src/intel/vulkan/anv_extensions.py      |   1 +
> >   src/intel/vulkan/anv_image.c            | 141 ++++++++++++++++--
> >   src/intel/vulkan/anv_private.h          |   1 +
> >   8 files changed, 405 insertions(+), 10 deletions(-)
> >   create mode 100644 src/intel/vulkan/anv_android.c
> > 
> > diff --git a/src/intel/Makefile.sources b/src/intel/Makefile.sources
> > index 8ca50ff622b..6f2dfa91e20 100644
> > --- a/src/intel/Makefile.sources
> > +++ b/src/intel/Makefile.sources
> > @@ -229,6 +229,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 d1b1132ed2e..e9c824f717b 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 00000000000..6b19ace4d2d
> > --- /dev/null
> > +++ b/src/intel/vulkan/anv_android.c
> > @@ -0,0 +1,245 @@
> > +/*
> > + * 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"
> > +
> > +
> > +#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,
> > +      },
> > +   },
> > +};
> > +
> > +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 *vkdev = malloc(sizeof(*vkdev));
> > +   if (!vkdev)
> > +      return -1;
> > +
> > +   *vkdev = (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 = &vkdev->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_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->physicalDevice;
> > +   VkPhysicalDevice phys_dev_h = anv_physical_device_to_handle(phys_dev);
> > +   VkFormatProperties props;
> > +
> > +   static const VkFormat fb_formats[] = {
> > +      VK_FORMAT_B8G8R8A8_UNORM,
> > +      VK_FORMAT_B5G6R5_UNORM_PACK16,
> > +   };
> > +
> > +   *grallocUsage = 0;
> > +   anv_GetPhysicalDeviceFormatProperties(phys_dev_h, format, &props);
> > +
> > +   for (uint32_t i = 0; i < ARRAY_SIZE(fb_formats); ++i) {
> > +      if (format == fb_formats[i]) {
> > +         /* TODO(chadv): Query these properties from isl. */
> > +          *grallocUsage |= GRALLOC_USAGE_HW_FB |
> > +                           GRALLOC_USAGE_HW_COMPOSER |
> > +                           GRALLOC_USAGE_EXTERNAL_DISP;
> > +          break;
> > +      }
> > +   }
> > +
> > +  if ((props.optimalTilingFeatures & VK_FORMAT_FEATURE_SAMPLED_IMAGE_BIT))
> > +     *grallocUsage |= GRALLOC_USAGE_HW_TEXTURE;
> > +
> > +  if ((props.optimalTilingFeatures & VK_FORMAT_FEATURE_COLOR_ATTACHMENT_BIT))
> > +     *grallocUsage |= GRALLOC_USAGE_HW_RENDER;
> > +
> > +  if ((props.optimalTilingFeatures & VK_FORMAT_FEATURE_SAMPLED_IMAGE_BIT))
> > +     *grallocUsage |= GRALLOC_USAGE_HW_TEXTURE;
> > +
> > +  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);
> > +   ANV_FROM_HANDLE(anv_semaphore, semaphore, semaphore_h);
> > +   ANV_FROM_HANDLE(anv_fence, fence, fence_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);
> > +         goto done;
> > +      }
> > +   }
> > +
> > +   if (semaphore) {
> > +      /* It's illegal to import into an existing import. */
> > +      assert(semaphore->temporary.type == ANV_SEMAPHORE_TYPE_NONE);
> > +
> > +      /* It's also illegal here to import into an in-flight semaphore.
> > +       * Therefore, thanks to implicit sync, there's nothing to do here
> > +       * because we have already blocked on the nativeFenceFd.
> > +       */

> the semaphore here at least should be set to signaled state, otherwise, the
> submission with semaphore_wait could  have some problems.

Good point. I'll send a v5 in a few minutes.


More information about the mesa-dev mailing list