On Fri 15 Sep 2017, zhoucm1 wrote: > > > On 2017年09月14日 07:03, Chad Versace wrote: > > From: Chad Versace <chadvers...@chromium.org> > > > > 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: > > > > a little spinning cube demo APK > > dEQP-VK.info.* > > dEQP-VK.api.smoke.* > > dEQP-VK.api.info.instance.* > > dEQP-VK.api.info.device.* > > dEQP-VK.api.wsi.android.* > > > > 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. > > --- > > src/intel/Makefile.sources | 3 + > > src/intel/Makefile.vulkan.am | 2 + > > src/intel/vulkan/anv_android.c | 245 > > ++++++++++++++++++++++++++++++++ > > src/intel/vulkan/anv_device.c | 12 +- > > src/intel/vulkan/anv_entrypoints_gen.py | 10 +- > > src/intel/vulkan/anv_extensions.py | 1 + > > src/intel/vulkan/anv_image.c | 141 ++++++++++++++++-- > > src/intel/vulkan/anv_private.h | 1 + > > 8 files changed, 405 insertions(+), 10 deletions(-) > > create mode 100644 src/intel/vulkan/anv_android.c > > > > diff --git a/src/intel/Makefile.sources b/src/intel/Makefile.sources > > index 8ca50ff622b..6f2dfa91e20 100644 > > --- a/src/intel/Makefile.sources > > +++ b/src/intel/Makefile.sources > > @@ -229,6 +229,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/Makefile.vulkan.am b/src/intel/Makefile.vulkan.am > > index d1b1132ed2e..e9c824f717b 100644 > > --- a/src/intel/Makefile.vulkan.am > > +++ b/src/intel/Makefile.vulkan.am > > @@ -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 00000000000..6b19ace4d2d > > --- /dev/null > > +++ b/src/intel/vulkan/anv_android.c > > @@ -0,0 +1,245 @@ > > +/* > > + * 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" > > + > > + > > +#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, > > + }, > > + }, > > +}; > > + > > +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 *vkdev = malloc(sizeof(*vkdev)); > > + if (!vkdev) > > + return -1; > > + > > + *vkdev = (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 = &vkdev->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_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); > > + VkFormatProperties props; > > + > > + static const VkFormat fb_formats[] = { > > + VK_FORMAT_B8G8R8A8_UNORM, > > + VK_FORMAT_B5G6R5_UNORM_PACK16, > > + }; > > + > > + *grallocUsage = 0; > > + anv_GetPhysicalDeviceFormatProperties(phys_dev_h, format, &props); > > + > > + for (uint32_t i = 0; i < ARRAY_SIZE(fb_formats); ++i) { > > + if (format == fb_formats[i]) { > > + /* TODO(chadv): Query these properties from isl. */ > > + *grallocUsage |= GRALLOC_USAGE_HW_FB | > > + GRALLOC_USAGE_HW_COMPOSER | > > + GRALLOC_USAGE_EXTERNAL_DISP; > > + break; > > + } > > + } > > + > > + if ((props.optimalTilingFeatures & VK_FORMAT_FEATURE_SAMPLED_IMAGE_BIT)) > > + *grallocUsage |= GRALLOC_USAGE_HW_TEXTURE; > > + > > + if ((props.optimalTilingFeatures & > > VK_FORMAT_FEATURE_COLOR_ATTACHMENT_BIT)) > > + *grallocUsage |= GRALLOC_USAGE_HW_RENDER; > > + > > + if ((props.optimalTilingFeatures & VK_FORMAT_FEATURE_SAMPLED_IMAGE_BIT)) > > + *grallocUsage |= GRALLOC_USAGE_HW_TEXTURE; > > + > > + 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); > > + ANV_FROM_HANDLE(anv_semaphore, semaphore, semaphore_h); > > + ANV_FROM_HANDLE(anv_fence, fence, fence_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); > > + goto done; > > + } > > + } > > + > > + if (semaphore) { > > + /* It's illegal to import into an existing import. */ > > + assert(semaphore->temporary.type == ANV_SEMAPHORE_TYPE_NONE); > > + > > + /* It's also illegal here to import into an in-flight semaphore. > > + * Therefore, thanks to implicit sync, there's nothing to do here > > + * because we have already blocked on the nativeFenceFd. > > + */
> the semaphore here at least should be set to signaled state, otherwise, the > submission with semaphore_wait could have some problems. Good point. I'll send a v5 in a few minutes. _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev