[Mesa-dev] [PATCH 20/20] anv: Implement VK_ANDROID_native_buffer (v2)
Chad Versace
chadversary at chromium.org
Thu Sep 14 21:39:43 UTC 2017
On Thu 14 Sep 2017, Tapani Pälli wrote:
>
>
> 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
> > > +
> > > +#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.
Replacing #include <hardware/hwvulkan.h> with #include "hwvulkan.h"
requires changing the Autotools build too.
Instead, I think a better fix is to ensure that
$(MESA_TOP)/include/vulkan appears before any Android framework paths in
the list of include paths. If understand correctly, GCC searches the
include dirs in the order given on the cmdline, and cmdline dirs precede
any built-in system dirs.
See v3 of this patch.
> > > +
> > > +#include "anv_private.h"
> > > +
> > > +
> > > +#include "anv_private.h"
> >
> > no need to include twice
Thanks, and fixed in v3.
> > > +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.
Hi Tapani, I don't want the Android-IA build to break in the middle of
my patch series. So I took your changes and cherry-picked them,
hunk-by-hunk, into my patches. I resent those revised patches with you
CC'd. Please test.
More information about the mesa-dev
mailing list