[Mesa-dev] [PATCH 20/20] anv: Implement VK_ANDROID_native_buffer (v2)
Tapani Pälli
tapani.palli at intel.com
Thu Sep 14 05:51:36 UTC 2017
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< ---
// Tapani
More information about the mesa-dev
mailing list