[Mesa-dev] [PATCH 20/20] anv: Implement VK_ANDROID_native_buffer (v2)
Tapani Pälli
tapani.palli at intel.com
Thu Sep 14 06:41:49 UTC 2017
On 09/14/2017 08:51 AM, Tapani Pälli wrote:
>
>
> On 09/14/2017 02:03 AM, Chad Versace wrote:
>> From: Chad Versace <chadversary at 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>
>
> This include breaks things for us because hwvulkan.h includes vulkan.h
> from android which has different content than Mesa's vulkan.h. If I make
> this:
>
> #include "hwvulkan.h"
>
> and in Android.vulkan.mk include dir
> "frameworks/native/vulkan/include/hardware", then this won't cause harm
> since I believe that causes us to use mesa's vulkan.h. Harm looks like:
>
> --- 8< ---
> In file included from
> vendor/intel/external/android_ia/mesa/src/intel/vulkan/anv_gem.c:32:
> In file included from
> vendor/intel/external/android_ia/mesa/src/intel/vulkan/anv_private.h:74:
> out/target/product/androidia_64/gen/STATIC_LIBRARIES/libmesa_anv_entrypoints_intermediates/vulkan/anv_entrypoints.h:187:11:
> error: unknown type name 'PFN_vkGetPhysicalDeviceFeatures2KHR'; did you
> mean 'PFN_vkGetPhysicalDeviceFeatures'?
> PFN_vkGetPhysicalDeviceFeatures2KHR
> GetPhysicalDeviceFeatures2KHR;
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> PFN_vkGetPhysicalDeviceFeatures
> ...
>
> --- 8< ---
>
> this same happens with bazillions of other functions not included in
> android's old header but present in Mesa's vulkan.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"
>
> no need to include twice
>
>> +
>> +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);
>> +
>
>
> FYI I'm working on to get Android-IA work with these changes. Current
> biggest issue is following:
>
>
> --- 8< ---
> out/target/product/androidia_64/obj_x86/STATIC_LIBRARIES/libmesa_vulkan_common_intermediates/vulkan/anv_entrypoints.c:826:6:
> error: field designator 'GetSwapchainGrallocUsageANDROID' does not refer
> to any field in type 'const struct anv_dispatch_table'
> .GetSwapchainGrallocUsageANDROID =
> anv_GetSwapchainGrallocUsageANDROID,
> ^
> out/target/product/androidia_64/obj_x86/STATIC_LIBRARIES/libmesa_vulkan_common_intermediates/vulkan/anv_entrypoints.c:829:6:
> error: field designator 'AcquireImageANDROID' does not refer to any
> field in type 'const struct anv_dispatch_table'
> .AcquireImageANDROID = anv_AcquireImageANDROID,
> --- 8< ---
Got this one fixed (just did not remember to update entrypoints header
..) and have Vulkan apps working. I'll send my proposed additional
changes to list for you to take a look at.
// Tapani
More information about the mesa-dev
mailing list