<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">I made some fairly straightforward comments. with that, and the squash you sent, 1-6 and 8-10 are</div><div class="gmail_quote"> </div><div class="gmail_quote">Reviewed-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>></div><div class="gmail_quote"> </div><div class="gmail_quote">I'd like to see the respin of patch 7 but I fully expect to give it my R-B almost immediately.</div><div class="gmail_quote"> </div><div class="gmail_quote">--Jason </div><div class="gmail_quote"> </div><div class="gmail_quote">On Mon, Oct 16, 2017 at 11:55 AM, Chad Versace <<a href="mailto:chadversary@chromium.org" target="_blank">chadversary@chromium.org</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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 (as of patchset v9): - a little spinning cube demo APK - several Sascha demos - dEQP-VK.info.* - dEQP-VK.api.wsi.android.* (except dEQP-VK.api.wsi.android.swapchain.*.image_usage, because dEQP wants to create swapchains with VK_IMAGE_USAGE_STORAGE_BIT) - dEQP-VK.api.smoke.* - dEQP-VK.api.info.instance.* - dEQP-VK.api.info.device.* 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. v3: - Delete duplicate #include "anv_private.h". [per Tapani] - Try to fix the Android-IA build in <a href="http://Android.vulkan.mk" rel="noreferrer" target="_blank">Android.vulkan.mk</a> by following Tapani's example. v4: - Unset EXEC_OBJECT_ASYNC and set EXEC_OBJECT_WRITE on the imported gralloc buffer, just as we do for all other winsys buffers in anv_wsi.c. [found by Tapani] v5: - Really fix the Android-IA build by ensuring that <a href="http://Android.vulkan.mk" rel="noreferrer" target="_blank">Android.vulkan.mk</a> uses Mesa' vulkan.h and not Android's. Insert -I$(MESA_TOP)/include before -Iframeworks/native/vulkan/include. [for Tapani] - In vkAcquireImageANDROID, submit signal operations to the VkSemaphore and VkFence. [for zhou] v6: - Drop copy-paste duplication in vkGetSwapchainGrallocUsageANDROID(). [found by zhou] - Improve comments in vkGetSwapchainGrallocUsageANDROID(). v7: - Fix vkGetSwapchainGrallocUsageANDROID() to inspect its VkImageUsageFlags parameter. [for tfiga] - This fix regresses dEQP-VK.api.wsi.android.swapchain.*.image_usage because dEQP wants to create swapchains with VK_IMAGE_USAGE_STORAGE_BIT. v8: - Drop unneeded goto in vkAcquireImageANDROID. [for tfiga] v8.1: (minor changes) - Drop errant hunks added by rerere in anv_device.c. - Drop explicit mention of VK_ANDROID_native_buffer in anv_entrypoints_gen.py. [for jekstrand] v9: - Isolate as much Android code as possible, moving it from anv_image.c to anv_android.c. Connect the files with anv_image_from_gralloc(). Remove VkNativeBufferANDROID params from all anv_image.c funcs. [for krh] - Replace some intel_loge() with vk_errorf() in anv_android.c. - Use © in copyright line. [for krh] Reviewed-by: Tapani Pälli <<a href="mailto:tapani.palli@intel.com" target="_blank">tapani.palli@intel.com</a>> (v5) Cc: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net" target="_blank">jason@jlekstrand.net</a>> Cc: zhoucm1 <<a href="mailto:david1.zhou@amd.com" target="_blank">david1.zhou@amd.com</a>> Cc: Tomasz Figa <<a href="mailto:tfiga@chromium.org" target="_blank">tfiga@chromium.org</a>> Cc: Kristian H. Kristensen <<a href="mailto:hoegsberg@google.com" target="_blank">hoegsberg@google.com</a>> --- src/intel/<a href="http://Android.vulkan.mk" rel="noreferrer" target="_blank">Android.vulkan.mk</a> | 7 +- src/intel/Makefile.sources | 3 + src/intel/<a href="http://Makefile.vulkan.am" rel="noreferrer" target="_blank">Makefile.vulkan.am</a> | 2 + src/intel/vulkan/anv_android.c | 416 ++++++++++++++++++++++++++++++++ src/intel/vulkan/anv_entrypoints_gen.py | 4 +- src/intel/vulkan/anv_extensions.py | 1 + src/intel/vulkan/anv_image.c | 20 +- src/intel/vulkan/anv_private.h | 12 + 8 files changed, 459 insertions(+), 6 deletions(-) create mode 100644 src/intel/vulkan/anv_android.c diff --git a/src/intel/<a href="http://Android.vulkan.mk" rel="noreferrer" target="_blank">Android.vulkan.mk</a> b/src/intel/<a href="http://Android.vulkan.mk" rel="noreferrer" target="_blank">Android.vulkan.mk</a> index ee98e30c35..b9a0446f72 100644 --- a/src/intel/<a href="http://Android.vulkan.mk" rel="noreferrer" target="_blank">Android.vulkan.mk</a> +++ b/src/intel/<a href="http://Android.vulkan.mk" rel="noreferrer" target="_blank">Android.vulkan.mk</a> @@ -28,6 +28,7 @@ VK_ENTRYPOINTS_SCRIPT := $(MESA_PYTHON2) $(LOCAL_PATH)/vulkan/anv_entrypoints_ge VK_EXTENSIONS_SCRIPT := $(MESA_PYTHON2) $(LOCAL_PATH)/vulkan/anv_extensions.py VULKAN_COMMON_INCLUDES := \ + $(MESA_TOP)/include \ $(MESA_TOP)/src/mapi \ $(MESA_TOP)/src/gallium/auxiliary \ $(MESA_TOP)/src/gallium/include \ @@ -36,7 +37,8 @@ VULKAN_COMMON_INCLUDES := \ $(MESA_TOP)/src/vulkan/util \ $(MESA_TOP)/src/intel \ $(MESA_TOP)/include/drm-uapi \ - $(MESA_TOP)/src/intel/vulkan + $(MESA_TOP)/src/intel/vulkan \ + frameworks/native/vulkan/include # libmesa_anv_entrypoints with header and dummy.c # @@ -243,7 +245,8 @@ LOCAL_MODULE_CLASS := SHARED_LIBRARIES LOCAL_LDFLAGS += -Wl,--build-id=sha1 LOCAL_SRC_FILES := \ - $(VULKAN_GEM_FILES) + $(VULKAN_GEM_FILES) \ + $(VULKAN_ANDROID_FILES) LOCAL_C_INCLUDES := \ $(VULKAN_COMMON_INCLUDES) \ diff --git a/src/intel/Makefile.sources b/src/intel/Makefile.sources index da018dcc6a..5a09e6d916 100644 --- a/src/intel/Makefile.sources +++ b/src/intel/Makefile.sources @@ -231,6 +231,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/<a href="http://Makefile.vulkan.am" rel="noreferrer" target="_blank">Makefile.vulkan.am</a> b/src/intel/<a href="http://Makefile.vulkan.am" rel="noreferrer" target="_blank">Makefile.vulkan.am</a> index 0f488fc9f6..811faab556 100644 --- a/src/intel/<a href="http://Makefile.vulkan.am" rel="noreferrer" target="_blank">Makefile.vulkan.am</a> +++ b/src/intel/<a href="http://Makefile.vulkan.am" rel="noreferrer" target="_blank">Makefile.vulkan.am</a> @@ -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 0000000000..73baaf1f54 --- /dev/null +++ b/src/intel/vulkan/anv_android.c @@ -0,0 +1,416 @@ +/* + * 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" + +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, + }, + }, +}; + +/* If any bits in test_mask are set, then unset them and return true. */ +static inline bool +unmask32(uint32_t *inout_mask, uint32_t test_mask) +{ + uint32_t orig_mask = *inout_mask; + *inout_mask &= ~test_mask; + return *inout_mask != orig_mask; +} + +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 *hal_dev = malloc(sizeof(*hal_dev)); + if (!hal_dev) + return -1; + + *hal_dev = (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 = &hal_dev->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_image_from_gralloc(VkDevice device_h, + const VkImageCreateInfo *base_info, + const VkNativeBufferANDROID *gralloc_info, + const VkAllocationCallbacks *alloc, + VkImage *out_image_h) + +{ + ANV_FROM_HANDLE(anv_device, device, device_h); + VkImage image_h = VK_NULL_HANDLE; + struct anv_image *image = NULL; + struct anv_bo *bo = NULL; + VkResult result; + + struct anv_image_create_info anv_info = { + .vk_info = base_info, + .isl_extra_usage_flags = ISL_SURF_USAGE_DISABLE_AUX_BIT, + }; + + if (gralloc_info->handle->numFds != 1) { + return vk_errorf(device->instance, device, + VK_ERROR_INVALID_EXTERNAL_HANDLE_KHR, + "VkNativeBufferANDROID::handle::numFds is %d, " + "expected 1", gralloc_info->handle->numFds); + } + + /* Do not close the gralloc handle's dma_buf. The lifetime of the dma_buf + * must exceed that of the gralloc handle, and we do not own the gralloc + * handle. + */ + int dma_buf = gralloc_info->handle->data[0]; + + result = anv_bo_cache_import(device, &device->bo_cache, dma_buf, &bo); + if (result != VK_SUCCESS) { + return vk_errorf(device->instance, device, result, + "failed to import dma-buf from VkNativeBufferANDROID"); + } + + int i915_tiling = anv_gem_get_tiling(device, bo->gem_handle); + switch (i915_tiling) { + case I915_TILING_NONE: + anv_info.isl_tiling_flags = ISL_TILING_LINEAR_BIT; + break; + case I915_TILING_X: + anv_info.isl_tiling_flags = ISL_TILING_X_BIT; + break; + case I915_TILING_Y: + anv_info.isl_tiling_flags = ISL_TILING_Y0_BIT; </blockquote><div> </div><div>We have isl_tiling_for_i915_tiling to do this. You may still want the switch statement for error handling purposes but I'm not a huge fan of repeated code. How about</div><div> </div><div>if (i915_tiling < 0) {</div><div> /* Do the one error */ </div><div>} else if (i915_tiling > I915_TILING_Y) {</div><div> /* Do the other error */</div><div>}</div><div>anv_info.isl_tiling_flags = 1u << isl_tiling_from_i915_tiling(i915_tiling);</div><div> </div><div>It's not really less code but it keeps things centralized. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + break; + case -1: + result = vk_errorf(device->instance, device, + VK_ERROR_INVALID_EXTERNAL_HANDLE_KHR, + "DRM_IOCTL_I915_GEM_GET_TILING failed for " + "VkNativeBufferANDROID"); + goto fail_tiling; + default: + result = vk_errorf(device->instance, device, + VK_ERROR_INVALID_EXTERNAL_HANDLE_KHR, + "DRM_IOCTL_I915_GEM_GET_TILING returned unknown " + "tiling %d for VkNativeBufferANDROID", i915_tiling); + goto fail_tiling; + } + + enum isl_format format = anv_get_isl_format(&device->info, + base_info->format, + VK_IMAGE_ASPECT_COLOR_BIT, + base_info->tiling); </blockquote><div> </div><div>Is base_info->tiling allowed to be anything other than OPTIMAL? Not that it really matters... </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + assert(format != ISL_FORMAT_UNSUPPORTED); + + anv_info.stride = gralloc_info->stride * + (isl_format_get_layout(format)->bpb / 8); + + result = anv_image_create(device_h, &anv_info, alloc, &image_h); + image = anv_image_from_handle(image_h); + if (result != VK_SUCCESS) + goto fail_create; + + if (bo->size < image->size) { + result = vk_errorf(device, device->instance, + VK_ERROR_INVALID_EXTERNAL_HANDLE_KHR, + "dma-buf from VkNativeBufferANDROID is too small for " + "VkImage: %"PRIu64" < %"PRIu64, + bo->size, image->size); + goto fail_size; + } + + assert(image->n_planes == 1); + assert(image->planes[0].bo_offset == 0); + + image->planes[0].bo = bo; + image->planes[0].bo_is_owned = true; + + /* We need to set the WRITE flag on window system buffers so that GEM will + * know we're writing to them and synchronize uses on other rings (for + * example, if the display server uses the blitter ring). + * + * If this function fails and if the imported bo was resident in the cache, + * we should avoid updating the bo's flags. Therefore, we defer updating + * the flags until success is certain. + * + */ + bo->flags &= ~EXEC_OBJECT_ASYNC; + bo->flags |= EXEC_OBJECT_WRITE; </blockquote><div> </div><div>If Android is using sync_file, we shouldn't need this? Maybe we need it now and can delete it later? I just read the semaphore code below and answered my own question. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + + /* Don't clobber the out-parameter until success is certain. */ + *out_image_h = image_h; + + return VK_SUCCESS; + + fail_size: + anv_DestroyImage(device_h, image_h, alloc); + fail_create: + fail_tiling: + anv_bo_cache_release(device, &device->bo_cache, bo); + + return result; +} + +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); + VkResult result; + + *grallocUsage = 0; + intel_logd("%s: format=%d, usage=0x%x", __func__, format, imageUsage); + + /* WARNING: Android Nougat's libvulkan.so hardcodes the VkImageUsageFlags + * returned to applications via VkSurfaceCapabilitiesKHR::supportedUsageFlags. + * The relevant code in libvulkan/swapchain.cpp contains this fun comment: + * + * TODO(jessehall): I think these are right, but haven't thought hard + * about it. Do we need to query the driver for support of any of + * these? + * + * Any disagreement between this function and the hardcoded + * VkSurfaceCapabilitiesKHR:supportedUsageFlags causes tests + * dEQP-VK.wsi.android.swapchain.*.image_usage to fail. + */ + + const VkPhysicalDeviceImageFormatInfo2KHR image_format_info = { + .sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_IMAGE_FORMAT_INFO_2_KHR, + .format = format, + .type = VK_IMAGE_TYPE_2D, + .tiling = VK_IMAGE_TILING_OPTIMAL, + .usage = imageUsage, + }; + + VkImageFormatProperties2KHR image_format_props = { + .sType = VK_STRUCTURE_TYPE_IMAGE_FORMAT_PROPERTIES_2_KHR, + }; + + /* Check that requested format and usage are supported. */ + result = anv_GetPhysicalDeviceImageFormatProperties2KHR(phys_dev_h, + &image_format_info, &image_format_props); + if (result != VK_SUCCESS) { + return vk_errorf(device->instance, device, result, + "anv_GetPhysicalDeviceImageFormatProperties2KHR failed " + "inside %s", __func__); + } + + /* Reject STORAGE here to avoid complexity elsewhere. */ + if (imageUsage & VK_IMAGE_USAGE_STORAGE_BIT) { + return vk_errorf(device->instance, device, VK_ERROR_FORMAT_NOT_SUPPORTED, + "VK_IMAGE_USAGE_STORAGE_BIT unsupported for gralloc " + "swapchain"); + } + + if (unmask32(&imageUsage, VK_IMAGE_USAGE_TRANSFER_DST_BIT | + VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT)) + *grallocUsage |= GRALLOC_USAGE_HW_RENDER; + + if (unmask32(&imageUsage, VK_IMAGE_USAGE_TRANSFER_SRC_BIT | + VK_IMAGE_USAGE_SAMPLED_BIT | + VK_IMAGE_USAGE_INPUT_ATTACHMENT_BIT)) + *grallocUsage |= GRALLOC_USAGE_HW_TEXTURE; + + /* All VkImageUsageFlags not explicitly checked here are unsupported for + * gralloc swapchains. + */ + if (imageUsage != 0) { + return vk_errorf(device->instance, device, VK_ERROR_FORMAT_NOT_SUPPORTED, + "unsupported VkImageUsageFlags(0x%x) for gralloc " + "swapchain", imageUsage); + } + + /* The below formats support GRALLOC_USAGE_HW_FB (that is, display + * scanout). This short list of formats is univserally supported on Intel + * but is incomplete. The full set of supported formats is dependent on + * kernel and hardware. + * + * FINISHME: Advertise all display-supported formats. + */ + if (format == VK_FORMAT_B8G8R8A8_UNORM || + format == VK_FORMAT_B5G6R5_UNORM_PACK16) { + *grallocUsage |= GRALLOC_USAGE_HW_FB | + GRALLOC_USAGE_HW_COMPOSER | + GRALLOC_USAGE_EXTERNAL_DISP; + } + + 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); + 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); + } + + /* From VK_ANDROID_native_buffer's pseudo spec + * (<a href="https://source.android.com/devices/graphics/implement-vulkan" rel="noreferrer" target="_blank">https://source.android.com/devices/graphics/implement-vulkan</a>): + * + * The driver takes ownership of the fence fd and is responsible for + * closing it [...] even if vkAcquireImageANDROID fails and returns + * an error. + */ + close(nativeFenceFd); + + if (result != VK_SUCCESS) + return result; + } + + if (semaphore_h || fence_h) { + /* Thanks to implicit sync, the image is ready for GPU access. But we + * must still put the semaphore into the "submit" state; otherwise the + * client may get unexpected behavior if the client later uses it as + * a wait semaphore. + * + * Because we blocked above on the nativeFenceFd, the image is also + * ready for foreign-device access (including CPU access). But we must + * still signal the fence; otherwise the client may get unexpected + * behavior if the client later waits on it. + * + * For some values of anv_semaphore_type, we must submit the semaphore + * to execbuf in order to signal it. Likewise for anv_fence_type. + * Instead of open-coding here the signal operation for each + * anv_semaphore_type and anv_fence_type, we piggy-back on + * vkQueueSubmit. + */ </blockquote><div> </div><div>This makes me sad.... Also, it works. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + const VkSubmitInfo submit = { + .sType = VK_STRUCTURE_TYPE_SUBMIT_INFO, + .waitSemaphoreCount = 0, + .commandBufferCount = 0, + .signalSemaphoreCount = (semaphore_h ? 1 : 0), + .pSignalSemaphores = &semaphore_h, + }; + + result = anv_QueueSubmit(anv_queue_to_handle(&device->queue), 1, + &submit, fence_h); + if (result != VK_SUCCESS) { + return vk_errorf(device->instance, device, result, + "anv_QueueSubmit failed inside %s", __func__); + } + } + + return VK_SUCCESS; +} + +VkResult +anv_QueueSignalReleaseImageANDROID( + VkQueue queue, + uint32_t waitSemaphoreCount, + const VkSemaphore* pWaitSemaphores, + VkImage image, + int* pNativeFenceFd) +{ + VkResult result; + + if (waitSemaphoreCount == 0) + goto done; + + result = anv_QueueSubmit(queue, 1, + &(VkSubmitInfo) { + .sType = VK_STRUCTURE_TYPE_SUBMIT_INFO, + .waitSemaphoreCount = 1, + .pWaitSemaphores = pWaitSemaphores, + }, + (VkFence) VK_NULL_HANDLE); + if (result != VK_SUCCESS) + return result; + + done: + if (pNativeFenceFd) { + /* We can rely implicit on sync because above we submitted all + * semaphores to the queue. + */ + *pNativeFenceFd = -1; + } + + return VK_SUCCESS; +} diff --git a/src/intel/vulkan/anv_entrypoints_gen.py b/src/intel/vulkan/anv_entrypoints_gen.py index bf376a881f..334b6de54b 100644 --- a/src/intel/vulkan/anv_entrypoints_gen.py +++ b/src/intel/vulkan/anv_entrypoints_gen.py @@ -270,7 +270,9 @@ def get_entrypoints(doc, entrypoints_to_defines, start_index): if extension.attrib['name'] not in supported: continue - assert extension.attrib['supported'] == 'vulkan' + if extension.attrib['supported'] != 'vulkan': + continue + for command in extension.findall('./require/command'): enabled_commands.add(command.attrib['name']) diff --git a/src/intel/vulkan/anv_extensions.py b/src/intel/vulkan/anv_extensions.py index a828a668d6..b1e984b8cd 100644 --- a/src/intel/vulkan/anv_extensions.py +++ b/src/intel/vulkan/anv_extensions.py @@ -50,6 +50,7 @@ class Extension: # the those extension strings, then tests dEQP-VK.api.info.instance.extensions # and dEQP-VK.api.info.device fail due to the duplicated strings. EXTENSIONS = [ + Extension('VK_ANDROID_native_buffer', 5, 'ANDROID'), Extension('VK_KHR_bind_memory2', 1, True), Extension('VK_KHR_dedicated_allocation', 1, True), Extension('VK_KHR_descriptor_update_template', 1, True), diff --git a/src/intel/vulkan/anv_image.c b/src/intel/vulkan/anv_image.c index b80011e222..cd96bfd0d6 100644 --- a/src/intel/vulkan/anv_image.c +++ b/src/intel/vulkan/anv_image.c @@ -40,9 +40,10 @@ static isl_surf_usage_flags_t choose_isl_surf_usage(VkImageCreateFlags vk_create_flags, VkImageUsageFlags vk_usage, + isl_surf_usage_flags_t isl_extra_usage, VkImageAspectFlags aspect) { - isl_surf_usage_flags_t isl_usage = 0; + isl_surf_usage_flags_t isl_usage = isl_extra_usage; if (vk_usage & VK_IMAGE_USAGE_SAMPLED_BIT) isl_usage |= ISL_SURF_USAGE_TEXTURE_BIT; @@ -290,6 +291,10 @@ make_surface(const struct anv_device *dev, anv_get_format_plane(&dev->info, image->vk_format, aspect, image->tiling); struct anv_surface *anv_surf = &image->planes[plane].surface; + const isl_surf_usage_flags_t usage = + choose_isl_surf_usage(vk_info->flags, image->usage, + anv_info->isl_extra_usage_flags, aspect); + /* If an image is created as BLOCK_TEXEL_VIEW_COMPATIBLE, then we need to * fall back to linear on Broadwell and earlier because we aren't * guaranteed that we can handle offsets correctly. On Sky Lake, the @@ -316,7 +321,7 @@ make_surface(const struct anv_device *dev, .samples = vk_info->samples, .min_alignment = 0, .row_pitch = anv_info->stride, - .usage = choose_isl_surf_usage(vk_info->flags, image->usage, aspect), + .usage = usage, .tiling_flags = tiling_flags); /* isl_surf_init() will fail only if provided invalid input. Invalid input @@ -347,7 +352,7 @@ make_surface(const struct anv_device *dev, .samples = vk_info->samples, .min_alignment = 0, .row_pitch = anv_info->stride, - .usage = choose_isl_surf_usage(image->usage, image->usage, aspect), + .usage = usage, .tiling_flags = ISL_TILING_ANY_MASK); /* isl_surf_init() will fail only if provided invalid input. Invalid input @@ -547,6 +552,15 @@ anv_CreateImage(VkDevice device, const VkAllocationCallbacks *pAllocator, VkImage *pImage) { +#ifdef ANDROID + 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); +#endif + return anv_image_create(device, &(struct anv_image_create_info) { .vk_info = pCreateInfo, diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h index b51d110514..36c7220679 100644 --- a/src/intel/vulkan/anv_private.h +++ b/src/intel/vulkan/anv_private.h @@ -69,6 +69,7 @@ struct gen_l3_config; #include <vulkan/vulkan.h> #include <vulkan/vulkan_intel.h> #include <vulkan/vk_icd.h> +#include <vulkan/vk_android_native_buffer.h> #include "anv_entrypoints.h" #include "isl/isl.h" @@ -2632,6 +2633,9 @@ struct anv_image_create_info { /** An opt-in bitmask which filters an ISL-mapping of the Vulkan tiling. */ isl_tiling_flags_t isl_tiling_flags; + /** These flags will be added to any derived from VkImageCreateInfo. */ + isl_surf_usage_flags_t isl_extra_usage_flags; + uint32_t stride; }; @@ -2640,6 +2644,14 @@ VkResult anv_image_create(VkDevice _device, const VkAllocationCallbacks* alloc, VkImage *pImage); +#ifdef ANDROID +VkResult anv_image_from_gralloc(VkDevice device_h, + const VkImageCreateInfo *base_info, + const VkNativeBufferANDROID *gralloc_info, + const VkAllocationCallbacks *alloc, + VkImage *pImage); +#endif + const struct anv_surface * anv_image_get_surface_for_aspect_mask(const struct anv_image *image, VkImageAspectFlags aspect_mask); -- 2.13.0 </blockquote></div> </div></div>