[Mesa-dev] [PATCH 10/15] anv/android: support creating images from external format
Tapani Pälli
tapani.palli at intel.com
Fri Dec 14 12:02:39 UTC 2018
On 12/11/18 3:27 PM, Lionel Landwerlin wrote:
> 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?
Yes makes sense, I've added following asserts and following comment:
"We must have image allocated or imported at this point. According to
the specification, external images must have been bound to memory before
calling GetImageMemoryRequirements."
>
>> *
>> 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 :)
done
>
>> + 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