Hi Chad, On the topic of keep this in the driver vs separate module - I think it makes sense to keep it internal.
Sure radv/others won't be able to reuse the code [that easily], at the same time it gives greater control and one could make more optimal decisions. On 2 September 2017 at 09:17, Chad Versace <[email protected]> wrote: > diff --git a/src/intel/Makefile.sources b/src/intel/Makefile.sources > index f6a69f65455..e4a371cbe3f 100644 > --- a/src/intel/Makefile.sources > +++ b/src/intel/Makefile.sources > @@ -228,6 +228,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..bc4b12768ad 100644 > --- a/src/intel/Makefile.vulkan.am > +++ b/src/intel/Makefile.vulkan.am > @@ -147,8 +147,12 @@ VULKAN_LIB_DEPS = \ > -lm > > if HAVE_PLATFORM_ANDROID > +VULKAN_CPPFLAGS += \ > + -I$(top_srcdir)/include/android \ > + $(ANDROID_CPPFLAGS) > VULKAN_CFLAGS += $(ANDROID_CFLAGS) > VULKAN_LIB_DEPS += $(ANDROID_LIBS) > +VULKAN_SOURCES += $(VULKAN_ANDROID_FILES) > endif > I think we'll need a simialar hunk for Android. Once people are settled on the approach myself or Tapani can propose something. > 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..24274510cd6 > --- /dev/null > +++ b/src/intel/vulkan/anv_android.c > +#include <vulkan/vk_icd.h> > + > +static void UNUSED > +static_asserts(void) > +{ > + STATIC_ASSERT(HWVULKAN_DISPATCH_MAGIC == ICD_LOADER_MAGIC); > +} Seems like a left over? With that gone, there should be no need for vk_icd.h. > + > +PUBLIC struct hwvulkan_module_t HAL_MODULE_INFO_SYM = { Unrelated: any ideas why these are not const? All the HAL exports I've seen always lack the notation. > +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[]) { Nit: Please stay consistent with other anv_QueueSubmit users. Namely, move "1," on the previous line and use a pointer &(VkSubmitInfo), dropping the sentinel. > + { > + .sType = VK_STRUCTURE_TYPE_SUBMIT_INFO, > + .waitSemaphoreCount = 1, > + .pWaitSemaphores = pWaitSemaphores, > + }, > + }, > + (VkFence) VK_NULL_HANDLE); > @@ -1440,10 +1440,20 @@ VkResult anv_AllocateMemory( > struct anv_device_memory *mem; > VkResult result = VK_SUCCESS; > > + /* VK_ANDROID_native_buffer defines VkNativeBufferANDROID as an extension > + * of VkImageCreateInfo. We abuse the struct by chaining it to > + * VkMemoryAllocateInfo in the implementation of vkCreateImage. > + */ > + const VkNativeBufferANDROID *gralloc_info = NULL; > +#ifdef ANDROID On one hand I'm thinking that we could drop the ifdef guard here and the rest of the patch. At the same time any solution will lead to a few annoying issues: - The -I$(top_srcdir)/include/android is Android only -> breaking the !Android builds - The header within ^^ is unconditionally included The above two can be resolved trivially by a) conditionally including the header, or b) making the -I unconditional That in itself will lead to other issues: a) - VkNativeBufferANDROID is unknown at combile time - WA with redeclaration Leading to build warnings since that's a C11 feature - Matt fixed a few warnings like that recently. b) - missing headers - system/window.h at least (from vk_android_native_buffer.h) Personal preference is to: - conditionally include the header, providing a fallback redeclaration - silence the compiler to not emit warnings Then we could even drop all (but one) of the ifdef ANDROID guards ;-) How does that sound? Emil _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
