On Fri, Aug 24, 2018 at 12:12 AM Tapani Pälli <tapani.pa...@intel.com> wrote:
> > > On 08/22/2018 05:28 PM, Jason Ekstrand wrote: > > On Tue, Aug 21, 2018 at 3:27 AM Tapani Pälli <tapani.pa...@intel.com > > <mailto:tapani.pa...@intel.com>> 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 <tapani.pa...@intel.com > > <mailto:tapani.pa...@intel.com>> > > --- > > 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. > Fine by me. > > + }; > > + > > + 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 > > mesa-dev@lists.freedesktop.org <mailto: > mesa-dev@lists.freedesktop.org> > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > > >
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
