On 20 January 2017 at 10:12, Tapani Pälli <tapani.pa...@intel.com> wrote: > On 01/20/2017 08:26 AM, Tapani Pälli wrote: >> On 01/19/2017 06:09 PM, Emil Velikov wrote: >>> >>> On 19 January 2017 at 07:10, Tapani Pälli <tapani.pa...@intel.com> wrote: >>>> >>>> fixes to issues spotted by Emil Velikov: >>>> >>>> - set ANV_TIMESTAMP corretly >>>> - fix typo with VULKAN_GEM_FILES >>>> >>>> v2: update to use Makefile.sources under vulkan >>>> instead of having own >>>> >>>> v3: update to changes to generate from vk.xml >>>> (commit c7fc310) >>>> >>>> Signed-off-by: Tapani Pälli <tapani.pa...@intel.com> >>>> --- >>>> >>>> Note, this patch does not equal to 'Android support'. Patch provides >>>> build system >>>> support so that we can start building driver in Android-IA and going >>>> further to >>>> develop required support. In order to trigger build, you need to add >>>> vulkan.mesa_intel >>>> to PRODUCT_PACKAGES list for the build. >>>> >>>> src/intel/Android.mk | 1 + >>>> src/intel/Android.vulkan.mk | 257 >>>> ++++++++++++++++++++++++++++++++++++++++++++ >>> >>> I'm leaning towards having git mv Android.vulkan.mk vulkan/Android.mk >>> >>> Afaict both area identical from speed POV on Android, and it'll be >>> symmetrical to the Autoconf ones. >> >> >> That is true but all the other mk's are in src/intel so I wanted to >> follow the current way of things. Do you think we should move blorp, >> common, genxml, isl mk's as well? >> Atm blorp, common, genxml and isl are both "sub"makefiles, on autoconf and android. With the Vulkan one being a standalone one since it depends on mesa/drivers/dri/i965. Where possible I'd rather keep things analogous across autoconf <> android <> scons since it gives a nice ref. point when porting things across build systems plus it makes the build bits less overall.
>>>> 2 files changed, 258 insertions(+) >>>> create mode 100644 src/intel/Android.vulkan.mk >>>> >>>> diff --git a/src/intel/Android.mk b/src/intel/Android.mk >>>> index 3d501ab..7cb2bb9 100644 >>>> --- a/src/intel/Android.mk >>>> +++ b/src/intel/Android.mk >>>> @@ -29,3 +29,4 @@ include $(LOCAL_PATH)/Android.blorp.mk >>>> include $(LOCAL_PATH)/Android.common.mk >>>> include $(LOCAL_PATH)/Android.genxml.mk >>>> include $(LOCAL_PATH)/Android.isl.mk >>>> +include $(LOCAL_PATH)/Android.vulkan.mk >>>> diff --git a/src/intel/Android.vulkan.mk b/src/intel/Android.vulkan.mk >>>> new file mode 100644 >>>> index 0000000..bd2626d >>>> --- /dev/null >>>> +++ b/src/intel/Android.vulkan.mk >>>> @@ -0,0 +1,257 @@ >>>> +# Copyright © 2017 Intel Corporation >>>> +# >>>> +# 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 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. >>>> + >>>> +LOCAL_PATH := $(call my-dir) >>>> + >>>> +include $(CLEAR_VARS) >>>> +include $(LOCAL_PATH)/vulkan/Makefile.sources >>> >>> Then this file gets a s|vulkan/|| + all the addprefix disappears. >>> >>>> + >>>> +VULKAN_FILES := $(addprefix vulkan/, $(VULKAN_FILES)) >>>> +VULKAN_GEM_FILES := $(addprefix vulkan/, $(VULKAN_GEM_FILES)) >>>> +VULKAN_GENERATED_FILES := vulkan/anv_entrypoints.h >>>> vulkan/anv_timestamp.h >>>> + >>>> +GEN7_FILES := $(addprefix vulkan/, $(GEN7_FILES)) >>>> +GEN75_FILES := $(addprefix vulkan/, $(GEN75_FILES)) >>>> +GEN8_FILES := $(addprefix vulkan/, $(GEN8_FILES)) >>>> +GEN9_FILES := $(addprefix vulkan/, $(GEN9_FILES)) >>>> + >>>> +VULKAN_SOURCES := \ >>>> + $(VULKAN_GENERATED_FILES) \ >>>> + $(VULKAN_FILES) >>>> + >>>> +VULKAN_HEADERS := \ >>>> + $(MESA_TOP)/include/vulkan/vk_platform.h \ >>>> + $(MESA_TOP)/include/vulkan/vulkan.h \ >>>> + $(MESA_TOP)/include/vulkan/vulkan_intel.h >>>> + >>>> +VULKAN_COMMON_INCLUDES := \ >>>> + $(VULKAN_HEADERS) \ >>>> + $(MESA_TOP)/src/mapi \ >>>> + $(MESA_TOP)/src/gallium/auxiliary \ >>>> + $(MESA_TOP)/src/gallium/include \ >>>> + $(MESA_TOP)/src/mesa \ >>>> + $(MESA_TOP)/src/mesa/drivers/dri/common \ >>>> + $(MESA_TOP)/src/mesa/drivers/dri/i965 \ >>>> + $(MESA_TOP)/src/vulkan/wsi \ >>>> + $(MESA_TOP)/src/intel/vulkan >>>> + >>>> +# >>>> +# libmesa_anv_entrypoints with header and dummy.c >>>> +# >>>> + >>>> +include $(CLEAR_VARS) >>>> +LOCAL_MODULE := libmesa_anv_entrypoints >>>> +LOCAL_MODULE_CLASS := STATIC_LIBRARIES >>>> + >>>> +intermediates := $(call local-generated-sources-dir) >>>> + >>>> +LOCAL_C_INCLUDES := \ >>>> + $(VULKAN_COMMON_INCLUDES) \ >>>> + $(intermediates)/vulkan >>>> + >>>> +LOCAL_GENERATED_SOURCES += $(addprefix $(intermediates)/, >>>> $(VULKAN_GENERATED_FILES)) >>>> +LOCAL_GENERATED_SOURCES += $(intermediates)/vulkan/dummy.c >>>> + >>>> +$(intermediates)/vulkan/dummy.c: >>> >>> Memory is failing: Why do we need a dummy.c at all ? >> >> >> This was about duplicate symbols when linking against >> libmesa_anv_entrypoints later so we could not have the actual c here. I >> can't recall more details. >> Just remembered it: Since we require the header (alongside the C file) by multiple targets and having each anv_gen* create its own instance of the header is redundant and/or race prone (not 100% on this one). Thus we create an empty static library which pulls the dependencies appropriately. Adding the anv_entrypoint.c will result in duplicate symbol when folding the multiple anv_gen* (each one will have a copy) into the final binary. Please add a comment with as much or as little that sound reasonable. >> >>>> + @mkdir -p $(dir $@) >>>> + @echo "Gen Dummy: $(PRIVATE_MODULE) <= $(notdir $(@))" >>>> + $(hide) touch $@ >>>> + >>>> +define generator >>>> + @mkdir -p $(dir $@) >>>> + $(hide) cat $(MESA_TOP)/src/vulkan/registry/vk.xml | >>>> $(PRIVATE_SCRIPT) $(PRIVATE_ARGS) > $@ >>>> +endef >>>> + >>>> +$(intermediates)/vulkan/anv_entrypoints.h: >>>> $(intermediates)/vulkan/dummy.c >>>> +$(intermediates)/vulkan/anv_entrypoints.h: PRIVATE_SCRIPT := >>>> $(MESA_PYTHON2) $(LOCAL_PATH)/vulkan/anv_entrypoints_gen.py >>>> +$(intermediates)/vulkan/anv_entrypoints.h: PRIVATE_ARGS := header >>>> +$(intermediates)/vulkan/anv_entrypoints.h: >>>> + $(call generator) >>>> + >>> >>> Humble poke if we can share generation rules and/or includes as >>> elaborated here [1] ? >>> Definately no a blocker, just _really_ nice to have. >> >> >> You missed [1] .. these are quite small but yeah if sharing between some >> other mk is possible then yes we should go for it. >> Seems like I also missed T in "not". On the generation rules: https://lists.freedesktop.org/archives/mesa-dev/2016-June/120341.html Also skim through IGT's lib/Makefile.sources and related files On the includes de-duplication one could use LOCAL_C{PP,}FLAGS as mentioned in the documentation https://developer.android.com/ndk/guides/android_mk.html >>>> +LOCAL_SHARED_LIBRARIES := libdrm_intel >>>> + >>> >>> Out of curiosity: is the final binary linked against libdrm_intel ? We >>> don't really use it, but Android build dictates that using the above >>> is the correct thing to do... afaict. >> >> >> We need this to have intel_aub.h (included by brw_context.h), otherwise >> build fails and says that 'intel_aub.h' is not found. There might be >> others too but this is the first failure. >> Yes we need the C/CPP flags, but I'd imagine we end up over-linking against libdrm_intel. Not a huge deal either way - was just a question. >> Hmm what do you mean my duplication? This is unique rule for generating >> anv_entrypoints.c while we had almost similar one to generate >> anv_entrypoints.h. >> Brain freeze - thanks for the clarification :-) >>> Do we need the _WHOLE_ version here ? Skimming through the list (and >>> order) shouldn't getting it closer to the autoconf one help you >>> resolve things ? >>> If _WHOLE_ must stay please mention where/what any of the circular >>> dependencies that mandates it. >> >> >> You are right, it looks like it can be removed. I will test this and >> comment back on this if there is some problem removing it. > > > Did not work out, without WHOLE I get following error: > > VulkanHAL: Could not find vk_icdGetInstanceProcAddr symbol. undefined > symbol: vk_icdGetInstanceProcAddr > IMHO the linker should be smart enough to not discard symbols annotated with default visibility. Perhaps something else is defining PUBLIC in a different manner, thus ours (in macros.h) does not kick in ? Not sure how much one should bother really, but dropping _WHOLE should shave a few KiB off the final binary. It was just an idea, don't read too much into it. Thanks Emil _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
