<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Fri, Aug 24, 2018 at 12:12 AM Tapani Pälli <<a href="mailto:tapani.palli@intel.com">tapani.palli@intel.com</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> On 08/22/2018 05:28 PM, Jason Ekstrand wrote: > On Tue, Aug 21, 2018 at 3:27 AM Tapani Pälli <<a href="mailto:tapani.palli@intel.com" target="_blank">tapani.palli@intel.com</a> > <mailto:<a href="mailto:tapani.palli@intel.com" target="_blank">tapani.palli@intel.com</a>>> wrote: > > v2: add support for non-image buffers (AHARDWAREBUFFER_FORMAT_BLOB) > v3: properly handle usage bits when creating from image > > Signed-off-by: Tapani Pälli <<a href="mailto:tapani.palli@intel.com" target="_blank">tapani.palli@intel.com</a> > <mailto:<a href="mailto:tapani.palli@intel.com" target="_blank">tapani.palli@intel.com</a>>> > --- > src/intel/vulkan/anv_android.c | 149 > +++++++++++++++++++++++++++++++++++++++++ > src/intel/vulkan/anv_device.c | 46 ++++++++++++- > src/intel/vulkan/anv_private.h | 18 +++++ > 3 files changed, 212 insertions(+), 1 deletion(-) > > diff --git a/src/intel/vulkan/anv_android.c > b/src/intel/vulkan/anv_android.c > index 7d0eb588e2b..6f90649847d 100644 > --- a/src/intel/vulkan/anv_android.c > +++ b/src/intel/vulkan/anv_android.c > @@ -195,6 +195,155 @@ anv_GetAndroidHardwareBufferPropertiesANDROID( > return VK_SUCCESS; > } > > +VkResult > +anv_GetMemoryAndroidHardwareBufferANDROID( > + VkDevice device_h, > + const VkMemoryGetAndroidHardwareBufferInfoANDROID *pInfo, > + struct AHardwareBuffer **pBuffer) > +{ > + ANV_FROM_HANDLE(anv_device_memory, mem, pInfo->memory); > + > + /* Some quotes from Vulkan spec: > + * > + * "If the device memory was created by importing an Android > hardware > + * buffer, vkGetMemoryAndroidHardwareBufferANDROID must return > that same > + * Android hardware buffer object." > + * > + * > "VK_EXTERNAL_MEMORY_HANDLE_TYPE_ANDROID_HARDWARE_BUFFER_BIT_ANDROID must > + * have been included in > VkExportMemoryAllocateInfoKHR::handleTypes when > + * memory was created." > + */ > + if (mem->ahw) { > + *pBuffer = mem->ahw; > + /* Increase refcount. */ > + AHardwareBuffer_acquire(mem->ahw); > + return VK_SUCCESS; > + } > + > + return VK_ERROR_INVALID_EXTERNAL_HANDLE_KHR; > +} > + > +/* > + * Called from anv_AllocateMemory when import AHardwareBuffer. > + */ > +VkResult > +anv_import_ahw_memory(VkDevice device_h, > + struct anv_device_memory *mem, > + const > VkImportAndroidHardwareBufferInfoANDROID *info) > +{ > + ANV_FROM_HANDLE(anv_device, device, device_h); > + > + /* Get a description of buffer contents. */ > + AHardwareBuffer_Desc desc; > + AHardwareBuffer_describe(info->buffer, &desc); > + VkResult result = VK_SUCCESS; > + > + /* Import from AHardwareBuffer to anv_device_memory. */ > + const native_handle_t *handle = > + AHardwareBuffer_getNativeHandle(info->buffer); > + > + int dma_buf = (handle && handle->numFds) ? handle->data[0] : -1; > + if (dma_buf < 0) > + return VK_ERROR_INVALID_EXTERNAL_HANDLE_KHR; > + > + uint64_t bo_flags = 0; > + if (device->instance->physicalDevice.supports_48bit_addresses) > + bo_flags |= EXEC_OBJECT_SUPPORTS_48B_ADDRESS; > + if (device->instance->physicalDevice.use_softpin) > + bo_flags |= EXEC_OBJECT_PINNED; > + > + result = anv_bo_cache_import(device, &device->bo_cache, > + dma_buf, bo_flags, &mem->bo); > + if (result != VK_SUCCESS) > + return result; > + > + /* "If the vkAllocateMemory command succeeds, the implementation > must > + * acquire a reference to the imported hardware buffer, which it > must > + * release when the device memory object is freed. If the > command fails, > + * the implementation must not retain a reference." > + */ > + AHardwareBuffer_acquire(info->buffer); > + mem->ahw = info->buffer; > + > + return result; > +} > + > +VkResult > +anv_create_ahw_memory(VkDevice device_h, > + struct anv_device_memory *mem, > + const VkMemoryAllocateInfo *pAllocateInfo) > +{ > + ANV_FROM_HANDLE(anv_device, dev, device_h); > + > + const VkMemoryDedicatedAllocateInfo *dedicated_info = > + vk_find_struct_const(pAllocateInfo->pNext, > + MEMORY_DEDICATED_ALLOCATE_INFO); > + > + uint32_t w = 0; > + uint32_t h = 1; > + uint32_t format = 0; > + uint64_t usage = 0; > + > + /* If caller passed dedicated information. */ > + if (dedicated_info && dedicated_info->image) { > + ANV_FROM_HANDLE(anv_image, image, dedicated_info->image); > + w = image->extent.width; > + h = image->extent.height; > + format = android_format_from_vk(image->vk_format); > + > + /* Construct usage mask from image usage bits, see > + * 'AHardwareBuffer Usage Equivalence' in spec. > + */ > + if (image->usage & VK_IMAGE_USAGE_SAMPLED_BIT) > + usage |= AHARDWAREBUFFER_USAGE_GPU_SAMPLED_IMAGE; > + > + if (image->usage & VK_IMAGE_USAGE_INPUT_ATTACHMENT_BIT) > + usage |= AHARDWAREBUFFER_USAGE_GPU_SAMPLED_IMAGE; > + > + if (image->usage & VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT) > + usage |= AHARDWAREBUFFER_USAGE_GPU_COLOR_OUTPUT; > + > + if (image->create_flags & VK_IMAGE_CREATE_CUBE_COMPATIBLE_BIT) > + usage |= AHARDWAREBUFFER_USAGE_GPU_CUBE_MAP; > + > + if (image->create_flags & VK_IMAGE_CREATE_PROTECTED_BIT) > + usage |= AHARDWAREBUFFER_USAGE_PROTECTED_CONTENT; > + > + /* No usage bits set, set at least one GPU usage. */ > + if (usage == 0) > + usage |= AHARDWAREBUFFER_USAGE_GPU_SAMPLED_IMAGE; > + > + } else if (dedicated_info && dedicated_info->buffer) { > + ANV_FROM_HANDLE(anv_buffer, buffer, dedicated_info->buffer); > + w = buffer->size; > + format = AHARDWAREBUFFER_FORMAT_BLOB; > + usage = AHARDWAREBUFFER_USAGE_CPU_READ_OFTEN | > + AHARDWAREBUFFER_USAGE_CPU_WRITE_OFTEN; > + } else { > + w = pAllocateInfo->allocationSize; > + format = AHARDWAREBUFFER_FORMAT_BLOB; > + usage = AHARDWAREBUFFER_USAGE_CPU_READ_OFTEN | > + AHARDWAREBUFFER_USAGE_CPU_WRITE_OFTEN; > + } > + > + struct AHardwareBuffer *ahw = NULL; > + struct AHardwareBuffer_Desc desc = { > + .width = w, > + .height = h, > + .layers = 1, > + .format = format, > + .usage = usage, > + .stride = 0, > > > If the image is linear (not sure if that's possible), we could give them > a valid stride. It seems allocator will always anyway calculate the stride so I could just drop setting it here. </blockquote><div> </div><div>Fine by me. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> > + }; > + > + if (AHardwareBuffer_allocate(&desc, &ahw) != 0) > + return VK_ERROR_OUT_OF_HOST_MEMORY; > + > + mem->ahw = ahw; > + > + return VK_SUCCESS; > +} > + > VkResult > anv_image_from_gralloc(VkDevice device_h, > const VkImageCreateInfo *base_info, > diff --git a/src/intel/vulkan/anv_device.c > b/src/intel/vulkan/anv_device.c > index d8b67b54d63..54919112d08 100644 > --- a/src/intel/vulkan/anv_device.c > +++ b/src/intel/vulkan/anv_device.c > @@ -2161,14 +2161,53 @@ VkResult anv_AllocateMemory( > > if (pdevice->use_softpin) > bo_flags |= EXEC_OBJECT_PINNED; > + mem->ahw = NULL; > + > + /* Check if we need to support Android HW buffer export. If so, > + * create AHardwareBuffer and import memory from it. > + */ > + bool android_export = false; > + const VkExportMemoryAllocateInfo *export_info = > + vk_find_struct_const(pAllocateInfo->pNext, > + EXPORT_MEMORY_ALLOCATE_INFO); > + > + if (export_info && export_info->handleTypes & > + > VK_EXTERNAL_MEMORY_HANDLE_TYPE_ANDROID_HARDWARE_BUFFER_BIT_ANDROID) > + android_export = true; > + > + /* Android memory import. */ > + const struct VkImportAndroidHardwareBufferInfoANDROID *ahw_info = > + vk_find_struct_const(pAllocateInfo->pNext, > + > IMPORT_ANDROID_HARDWARE_BUFFER_INFO_ANDROID); > > const VkImportMemoryFdInfoKHR *fd_info = > vk_find_struct_const(pAllocateInfo->pNext, > IMPORT_MEMORY_FD_INFO_KHR); > > + /* Android export support required. */ > + if (android_export) { > +#ifdef ANDROID > + result = anv_create_ahw_memory(_device, mem, pAllocateInfo); > + if (result != VK_SUCCESS) > + goto fail; > + > + const struct VkImportAndroidHardwareBufferInfoANDROID > import_info = { > + .buffer = mem->ahw, > + }; > + result = anv_import_ahw_memory(_device, mem, &import_info); > + if (result != VK_SUCCESS) > + goto fail; > +#endif > + } else if (ahw_info) { > +#ifdef ANDROID > + > + result = anv_import_ahw_memory(_device, mem, ahw_info); > + if (result != VK_SUCCESS) > + goto fail; > /* The Vulkan spec permits handleType to be 0, in which case > the struct is > * ignored. > */ > - if (fd_info && fd_info->handleType) { > +#endif > + } else if (fd_info && fd_info->handleType) { > > > Two comments here. First, I think this would be cleaner if we only had > one #ifdef: > > if (android_export || ahw_info) { > #ifdef ANDROID > #else > assert(!"Mesa was not built with android support"); > #endif > } > > Second, I think you want the order the other way around so it's > > if (ahw_info) { > } else if (android_export) { > } else { > unreachable(); > } > > Otherwise, if they import a ANativeHardwareBuffer that they intend to > re-export, they'll end up with both ahw_info != NULL and android_export > and it'll choose to create a new one instead of importing. > OK thanks for the comments! I'll refactor this and changes proposed to later patches and get back. > /* At the moment, we support only the below handle types. */ > assert(fd_info->handleType == > VK_EXTERNAL_MEMORY_HANDLE_TYPE_OPAQUE_FD_BIT || > @@ -2313,6 +2352,11 @@ void anv_FreeMemory( > > anv_bo_cache_release(device, &device->bo_cache, mem->bo); > > +#ifdef ANDROID > + if (mem->ahw) > + AHardwareBuffer_release(mem->ahw); > +#endif > + > vk_free2(&device->alloc, pAllocator, mem); > } > > diff --git a/src/intel/vulkan/anv_private.h > b/src/intel/vulkan/anv_private.h > index e6984e84afa..56dee607dbe 100644 > --- a/src/intel/vulkan/anv_private.h > +++ b/src/intel/vulkan/anv_private.h > @@ -85,6 +85,11 @@ struct gen_l3_config; > #include "common/intel_log.h" > #include "wsi_common.h" > > +#ifdef ANDROID > +#include <vndk/hardware_buffer.h> > +#include "vulkan/vulkan_android.h" > +#endif > + > /* anv Virtual Memory Layout > * ========================= > * > @@ -1354,6 +1359,11 @@ struct anv_device_memory { > struct anv_memory_type * type; > VkDeviceSize map_size; > void * map; > + > + /* If set, we are holding reference to AHardwareBuffer > + * which we must release when memory is freed. > + */ > + struct AHardwareBuffer * ahw; > }; > > /** > @@ -3043,6 +3053,14 @@ VkResult anv_image_from_gralloc(VkDevice > device_h, > const VkNativeBufferANDROID > *gralloc_info, > const VkAllocationCallbacks *alloc, > VkImage *pImage); > + > +VkResult anv_import_ahw_memory(VkDevice device_h, > + struct anv_device_memory *mem, > + const > VkImportAndroidHardwareBufferInfoANDROID *info); > + > +VkResult anv_create_ahw_memory(VkDevice device_h, > + struct anv_device_memory *mem, > + const VkMemoryAllocateInfo > *pAllocateInfo); > #endif > > const struct anv_surface * > -- > 2.14.4 > > _______________________________________________ > mesa-dev mailing list > <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a> <mailto:<a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a>> > <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> > </blockquote></div></div>