[Mesa-dev] [PATCH 10/15] anv/android: support creating images from external format

Lionel Landwerlin lionel.g.landwerlin at intel.com
Tue Dec 11 13:27:38 UTC 2018


On 27/11/2018 10:53, Tapani Pälli wrote:
> Since we don't know the exact format at creation time, some initialization
> is done only when bound with memory in vkBindImageMemory.
>
> v2: demand dedicated allocation in vkGetImageMemoryRequirements2 if
>      image has external format
>
> v3: refactor prepare_ahw_image, support vkBindImageMemory2,
>      calculate stride correctly for rgb(x) surfaces, rename as
>      'resolve_ahw_image'
>
> v4: rebase to b43f955037c changes
>
> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>


Another couple of suggestions belower, otherwise :


Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>


> ---
>   src/intel/vulkan/anv_android.c       |  36 ++++++++++
>   src/intel/vulkan/anv_android.h       |   8 +++
>   src/intel/vulkan/anv_android_stubs.c |  10 +++
>   src/intel/vulkan/anv_device.c        |   2 +-
>   src/intel/vulkan/anv_image.c         | 103 ++++++++++++++++++++++++++-
>   src/intel/vulkan/anv_private.h       |   4 ++
>   6 files changed, 160 insertions(+), 3 deletions(-)
>
> diff --git a/src/intel/vulkan/anv_android.c b/src/intel/vulkan/anv_android.c
> index bdc720214c4..3bc2b63b1c9 100644
> --- a/src/intel/vulkan/anv_android.c
> +++ b/src/intel/vulkan/anv_android.c
> @@ -356,6 +356,42 @@ anv_create_ahw_memory(VkDevice device_h,
>      return VK_SUCCESS;
>   }
>   
> +VkResult
> +anv_image_from_external(
> +   VkDevice device_h,
> +   const VkImageCreateInfo *base_info,
> +   const struct VkExternalMemoryImageCreateInfo *create_info,
> +   const VkAllocationCallbacks *alloc,
> +   VkImage *out_image_h)
> +{
> +   ANV_FROM_HANDLE(anv_device, device, device_h);
> +
> +   const struct VkExternalFormatANDROID *ext_info =
> +      vk_find_struct_const(base_info->pNext, EXTERNAL_FORMAT_ANDROID);
> +
> +   if (ext_info && ext_info->externalFormat != 0) {
> +      assert(base_info->format == VK_FORMAT_UNDEFINED);
> +      assert(base_info->imageType == VK_IMAGE_TYPE_2D);
> +      assert(base_info->usage == VK_IMAGE_USAGE_SAMPLED_BIT);
> +      assert(base_info->tiling == VK_IMAGE_TILING_OPTIMAL);
> +   }
> +
> +   struct anv_image_create_info anv_info = {
> +      .vk_info = base_info,
> +      .isl_extra_usage_flags = ISL_SURF_USAGE_DISABLE_AUX_BIT,
> +      .external_format = true,
> +   };
> +
> +   VkImage image_h;
> +   VkResult result = anv_image_create(device_h, &anv_info, alloc, &image_h);
> +   if (result != VK_SUCCESS)
> +      return result;
> +
> +   *out_image_h = image_h;
> +
> +   return VK_SUCCESS;
> +}
> +
>   VkResult
>   anv_image_from_gralloc(VkDevice device_h,
>                          const VkImageCreateInfo *base_info,
> diff --git a/src/intel/vulkan/anv_android.h b/src/intel/vulkan/anv_android.h
> index 01f0e856291..d5c073126e3 100644
> --- a/src/intel/vulkan/anv_android.h
> +++ b/src/intel/vulkan/anv_android.h
> @@ -29,6 +29,8 @@
>   #include <vulkan/vk_android_native_buffer.h>
>   
>   struct anv_device_memory;
> +struct anv_device;
> +struct anv_image;
>   
>   VkResult anv_image_from_gralloc(VkDevice device_h,
>                                   const VkImageCreateInfo *base_info,
> @@ -36,6 +38,12 @@ VkResult anv_image_from_gralloc(VkDevice device_h,
>                                   const VkAllocationCallbacks *alloc,
>                                   VkImage *pImage);
>   
> +VkResult anv_image_from_external(VkDevice device_h,
> +                                 const VkImageCreateInfo *base_info,
> +                                 const struct VkExternalMemoryImageCreateInfo *create_info,
> +                                 const VkAllocationCallbacks *alloc,
> +                                 VkImage *out_image_h);
> +
>   uint64_t anv_ahw_usage_from_vk_usage(const VkImageCreateFlags vk_create,
>                                        const VkImageUsageFlags vk_usage);
>   
> diff --git a/src/intel/vulkan/anv_android_stubs.c b/src/intel/vulkan/anv_android_stubs.c
> index ccc16500494..a34afc198a1 100644
> --- a/src/intel/vulkan/anv_android_stubs.c
> +++ b/src/intel/vulkan/anv_android_stubs.c
> @@ -55,3 +55,13 @@ anv_create_ahw_memory(VkDevice device_h,
>   {
>      return VK_ERROR_EXTENSION_NOT_PRESENT;
>   }
> +
> +VkResult
> +anv_image_from_external(VkDevice device_h,
> +                        const VkImageCreateInfo *base_info,
> +                        const struct VkExternalMemoryImageCreateInfo *create_info,
> +                        const VkAllocationCallbacks *alloc,
> +                        VkImage *out_image_h)
> +{
> +   return VK_ERROR_EXTENSION_NOT_PRESENT;
> +}
> diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c
> index a1ee9315956..5777de96d80 100644
> --- a/src/intel/vulkan/anv_device.c
> +++ b/src/intel/vulkan/anv_device.c
> @@ -2768,7 +2768,7 @@ void anv_GetImageMemoryRequirements2(
>         switch (ext->sType) {
>         case VK_STRUCTURE_TYPE_MEMORY_DEDICATED_REQUIREMENTS: {
>            VkMemoryDedicatedRequirements *requirements = (void *)ext;
> -         if (image->needs_set_tiling) {
> +         if (image->needs_set_tiling || image->external_format) {
>               /* If we need to set the tiling for external consumers, we need a
>                * dedicated allocation.


It's tempting to add an assert(image->size > 0) in the 
GetImageMemoryRequirements*() functions.

With a comment about external format images. What do you think?


>                *
> diff --git a/src/intel/vulkan/anv_image.c b/src/intel/vulkan/anv_image.c
> index 59e9d007831..79777efe456 100644
> --- a/src/intel/vulkan/anv_image.c
> +++ b/src/intel/vulkan/anv_image.c
> @@ -594,6 +594,15 @@ anv_image_create(VkDevice _device,
>      image->drm_format_mod = isl_mod_info ? isl_mod_info->modifier :
>                                             DRM_FORMAT_MOD_INVALID;
>   
> +   /* In case of external format, We don't know format yet,
> +    * so skip the rest for now.
> +    */
> +   if (create_info->external_format) {
> +      image->external_format = true;
> +      *pImage = anv_image_to_handle(image);
> +      return VK_SUCCESS;
> +   }
> +
>      const struct anv_format *format = anv_get_format(image->vk_format);
>      assert(format != NULL);
>   
> @@ -635,9 +644,16 @@ anv_CreateImage(VkDevice device,
>                   const VkAllocationCallbacks *pAllocator,
>                   VkImage *pImage)
>   {
> +   const struct VkExternalMemoryImageCreateInfo *create_info =
> +      vk_find_struct_const(pCreateInfo->pNext, EXTERNAL_MEMORY_IMAGE_CREATE_INFO);
> +
> +   if (create_info && create_info->handleTypes &
> +       VK_EXTERNAL_MEMORY_HANDLE_TYPE_ANDROID_HARDWARE_BUFFER_BIT_ANDROID)


I've been told to wrap the a & b expressions into (a & b), so just 
passing on the advice :)


> +      return anv_image_from_external(device, pCreateInfo, create_info,
> +                                     pAllocator, pImage);
> +
>      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);
> @@ -690,6 +706,83 @@ static void anv_image_bind_memory_plane(struct anv_device *device,
>      };
>   }
>   
> +/* We are binding AHardwareBuffer. Get a description, resolve the
> + * format and prepare anv_image properly.
> + */
> +static void
> +resolve_ahw_image(struct anv_device *device,
> +                  struct anv_image *image,
> +                  struct anv_device_memory *mem)
> +{
> +#ifdef ANDROID
> +   assert(mem->ahw);
> +   AHardwareBuffer_Desc desc;
> +   AHardwareBuffer_describe(mem->ahw, &desc);
> +
> +   /* Check tiling. */
> +   int i915_tiling = anv_gem_get_tiling(device, mem->bo->gem_handle);
> +   VkImageTiling vk_tiling;
> +   isl_tiling_flags_t isl_tiling_flags = 0;
> +
> +   switch (i915_tiling) {
> +   case I915_TILING_NONE:
> +      vk_tiling = VK_IMAGE_TILING_LINEAR;
> +      isl_tiling_flags = ISL_TILING_LINEAR_BIT;
> +      break;
> +   case I915_TILING_X:
> +      vk_tiling = VK_IMAGE_TILING_OPTIMAL;
> +      isl_tiling_flags = ISL_TILING_X_BIT;
> +      break;
> +   case I915_TILING_Y:
> +      vk_tiling = VK_IMAGE_TILING_OPTIMAL;
> +      isl_tiling_flags = ISL_TILING_Y0_BIT;
> +      break;
> +   case -1:
> +   default:
> +      unreachable("Invalid tiling flags.");
> +   }
> +
> +   assert(vk_tiling == VK_IMAGE_TILING_LINEAR ||
> +          vk_tiling == VK_IMAGE_TILING_OPTIMAL);
> +
> +   /* Check format. */
> +   VkFormat vk_format = vk_format_from_android(desc.format);
> +   enum isl_format isl_fmt = anv_get_isl_format(&device->info,
> +                                                vk_format,
> +                                                VK_IMAGE_ASPECT_COLOR_BIT,
> +                                                vk_tiling);
> +   assert(format != ISL_FORMAT_UNSUPPORTED);
> +
> +   /* Handle RGB(X)->RGBA fallback. */
> +   switch (desc.format) {
> +   case AHARDWAREBUFFER_FORMAT_R8G8B8_UNORM:
> +   case AHARDWAREBUFFER_FORMAT_R8G8B8X8_UNORM:
> +      if (isl_format_is_rgb(isl_fmt))
> +         isl_fmt = isl_format_rgb_to_rgba(isl_fmt);
> +      break;
> +   }
> +
> +   /* Now we are able to fill anv_image fields properly and create
> +    * isl_surface for it.
> +    */
> +   image->vk_format = vk_format;
> +   image->format = anv_get_format(vk_format);
> +   image->aspects = vk_format_aspects(image->vk_format);
> +   image->n_planes = image->format->n_planes;
> +   image->ccs_e_compatible = false;
> +
> +   uint32_t stride = desc.stride *
> +                     (isl_format_get_layout(isl_fmt)->bpb / 8);
> +
> +   uint32_t b;
> +   for_each_bit(b, image->aspects) {
> +      VkResult r = make_surface(device, image, stride, isl_tiling_flags,
> +                                ISL_SURF_USAGE_DISABLE_AUX_BIT, (1 << b));
> +      assert(r == VK_SUCCESS);
> +   }
> +#endif
> +}
> +
>   VkResult anv_BindImageMemory(
>       VkDevice                                    _device,
>       VkImage                                     _image,
> @@ -700,6 +793,9 @@ VkResult anv_BindImageMemory(
>      ANV_FROM_HANDLE(anv_device_memory, mem, _memory);
>      ANV_FROM_HANDLE(anv_image, image, _image);
>   
> +   if (mem->ahw)
> +      resolve_ahw_image(device, image, mem);
> +
>      uint32_t aspect_bit;
>      anv_foreach_image_aspect_bit(aspect_bit, image, image->aspects) {
>         uint32_t plane =
> @@ -721,8 +817,11 @@ VkResult anv_BindImageMemory2(
>         const VkBindImageMemoryInfo *bind_info = &pBindInfos[i];
>         ANV_FROM_HANDLE(anv_device_memory, mem, bind_info->memory);
>         ANV_FROM_HANDLE(anv_image, image, bind_info->image);
> -      VkImageAspectFlags aspects = image->aspects;
>   
> +      if (mem->ahw)
> +         resolve_ahw_image(device, image, mem);
> +
> +      VkImageAspectFlags aspects = image->aspects;
>         vk_foreach_struct_const(s, bind_info->pNext) {
>            switch (s->sType) {
>            case VK_STRUCTURE_TYPE_BIND_IMAGE_PLANE_MEMORY_INFO: {
> diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h
> index bd899dcb828..893c5da7abc 100644
> --- a/src/intel/vulkan/anv_private.h
> +++ b/src/intel/vulkan/anv_private.h
> @@ -2706,6 +2706,9 @@ struct anv_image {
>       */
>      bool ccs_e_compatible;
>   
> +   /* Image was created with external format. */
> +   bool external_format;
> +
>      /**
>       * Image subsurfaces
>       *
> @@ -3080,6 +3083,7 @@ struct anv_image_create_info {
>      isl_surf_usage_flags_t isl_extra_usage_flags;
>   
>      uint32_t stride;
> +   bool external_format;
>   };
>   
>   VkResult anv_image_create(VkDevice _device,




More information about the mesa-dev mailing list