[Mesa-dev] [PATCH 2/2] android: add vulkan build for intel

Fri Jan 20 06:26:47 UTC 2017

On 01/19/2017 06:09 PM, Emil Velikov wrote:
> On 19 January 2017 at 07:10, Tapani Pälli <tapani.palli at 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.palli at 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?

>>  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.

>> +       @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.

>> +$(intermediates)/vulkan/anv_timestamp.h:
>> +       echo "#define ANV_TIMESTAMP \"$(shell date +%s)\"" > $@
>> +
> The timestamp business is no longer needed as of
> 83548e12921b5724aa6c78c2b1efc9ff774fd7a2

Cool, I've missed this change, I'll remove this.

>> +LOCAL_EXPORT_C_INCLUDE_DIRS := \
>> +        $(intermediates)
>> +
>> +LOCAL_SHARED_LIBRARIES := libdrm_intel
>> +
>> +include $(MESA_COMMON_MK)
>> +include $(BUILD_STATIC_LIBRARY)
>> +
>> +ANV_INCLUDES := \
>> +       $(VULKAN_COMMON_INCLUDES) \
>> +       $(call generated-sources-dir-for,STATIC_LIBRARIES,libmesa_anv_entrypoints,,)/vulkan \
>> +       $(call generated-sources-dir-for,STATIC_LIBRARIES,libmesa_nir,,)/nir
>> +
>> +#
>> +# libanv for gen7
>> +#
>> +
>> +include $(CLEAR_VARS)
>> +LOCAL_MODULE := libmesa_anv_gen7
>> +LOCAL_MODULE_CLASS := STATIC_LIBRARIES
>> +
>> +LOCAL_SRC_FILES := $(GEN7_FILES)
>> +LOCAL_CFLAGS := -DGEN_VERSIONx10=70
>> +
>> +LOCAL_C_INCLUDES := $(ANV_INCLUDES)
>> +
>> +LOCAL_WHOLE_STATIC_LIBRARIES := libmesa_anv_entrypoints libmesa_genxml
>> +
>> +LOCAL_SHARED_LIBRARIES := libdrm_intel
>> +
>> +include $(MESA_COMMON_MK)
>> +include $(BUILD_STATIC_LIBRARY)
>> +
>> +#
>> +# libanv for gen75
>> +#
>> +
>> +include $(CLEAR_VARS)
>> +LOCAL_MODULE := libmesa_anv_gen75
>> +LOCAL_MODULE_CLASS := STATIC_LIBRARIES
>> +
>> +LOCAL_SRC_FILES := $(GEN75_FILES)
>> +LOCAL_CFLAGS := -DGEN_VERSIONx10=75
>> +
>> +LOCAL_C_INCLUDES := $(ANV_INCLUDES)
>> +
>> +LOCAL_WHOLE_STATIC_LIBRARIES := libmesa_anv_entrypoints libmesa_genxml
>> +
>> +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.

>> +include $(MESA_COMMON_MK)
>> +include $(BUILD_STATIC_LIBRARY)
>> +
>> +#
>> +# libanv for gen8
>> +#
>> +
>> +include $(CLEAR_VARS)
>> +LOCAL_MODULE := libmesa_anv_gen8
>> +LOCAL_MODULE_CLASS := STATIC_LIBRARIES
>> +
>> +LOCAL_SRC_FILES := $(GEN8_FILES)
>> +LOCAL_CFLAGS := -DGEN_VERSIONx10=80
>> +
>> +LOCAL_C_INCLUDES := $(ANV_INCLUDES)
>> +
>> +LOCAL_WHOLE_STATIC_LIBRARIES := libmesa_anv_entrypoints libmesa_genxml
>> +
>> +LOCAL_SHARED_LIBRARIES := libdrm_intel
>> +
>> +include $(MESA_COMMON_MK)
>> +include $(BUILD_STATIC_LIBRARY)
>> +
>> +#
>> +# libanv for gen9
>> +#
>> +
>> +include $(CLEAR_VARS)
>> +LOCAL_MODULE := libmesa_anv_gen9
>> +LOCAL_MODULE_CLASS := STATIC_LIBRARIES
>> +
>> +LOCAL_SRC_FILES := $(GEN9_FILES)
>> +LOCAL_CFLAGS := -DGEN_VERSIONx10=90
>> +
>> +LOCAL_C_INCLUDES := $(ANV_INCLUDES)
>> +
>> +LOCAL_WHOLE_STATIC_LIBRARIES := libmesa_anv_entrypoints libmesa_genxml
>> +
>> +LOCAL_SHARED_LIBRARIES := libdrm_intel
>> +
>> +include $(MESA_COMMON_MK)
>> +include $(BUILD_STATIC_LIBRARY)
>> +
>> +#
>> +# libmesa_vulkan_common
>> +#
>> +
>> +include $(CLEAR_VARS)
>> +LOCAL_MODULE := libmesa_vulkan_common
>> +LOCAL_MODULE_CLASS := STATIC_LIBRARIES
>> +
>> +intermediates := $(call local-generated-sources-dir)
>> +
>> +LOCAL_SRC_FILES := $(VULKAN_FILES)
>> +
>> +LOCAL_C_INCLUDES := \
>> +       $(ANV_INCLUDES) \
>> +       $(MESA_TOP)/src/compiler
>> +
>> +LOCAL_WHOLE_STATIC_LIBRARIES := libmesa_anv_entrypoints libmesa_genxml
>> +
>> +LOCAL_GENERATED_SOURCES += $(intermediates)/vulkan/anv_entrypoints.c
>
>> +$(intermediates)/vulkan/anv_entrypoints.c: PRIVATE_SCRIPT := $(MESA_PYTHON2) $(LOCAL_PATH)/vulkan/anv_entrypoints_gen.py
>> +$(intermediates)/vulkan/anv_entrypoints.c: PRIVATE_ARGS := code
>> +$(intermediates)/vulkan/anv_entrypoints.c:
>> +       $(call generator)
>> +
> Do we need to duplicate the rule ? If we do please add a small comment why.

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.

>> +LOCAL_SHARED_LIBRARIES := libdrm_intel
>> +
>> +include $(MESA_COMMON_MK)
>> +include $(BUILD_STATIC_LIBRARY)
>> +
>> +
>> +#
>> +# libmesa_vulkan_intel
>> +#
>> +
>> +include $(CLEAR_VARS)
>> +
>> +LOCAL_MODULE := vulkan.mesa_intel
>> +LOCAL_MODULE_CLASS := SHARED_LIBRARIES
>> +
>> +LOCAL_SRC_FILES := \
>> +       $(VULKAN_GEM_FILES)
>> +
>> +LOCAL_C_INCLUDES := \
>> +       $(VULKAN_COMMON_INCLUDES) \
>> +       $(call generated-sources-dir-for,STATIC_LIBRARIES,libmesa_vulkan_intel,,)/vulkan \
>> +       $(call generated-sources-dir-for,STATIC_LIBRARIES,libmesa_anv_entrypoints,,)/vulkan
>> +
>> +LOCAL_EXPORT_C_INCLUDE_DIRS := $(MESA_TOP)/src/intel/vulkan
>> +
>> +LOCAL_WHOLE_STATIC_LIBRARIES := \
>> +       libmesa_nir \
>> +       libmesa_isl \
>> +       libmesa_glsl \
>> +       libmesa_util \
>> +       libmesa_blorp \
>> +       libmesa_compiler \
>> +       libmesa_dricore \
>> +       libmesa_dri_common \
>> +       libmesa_intel_common \
>> +       libmesa_vulkan_common \
>> +       libmesa_anv_gen7 \
>> +       libmesa_anv_gen75 \
>> +       libmesa_anv_gen8 \
>> +       libmesa_anv_gen9 \
>> +       libmesa_i965_compiler \
>> +       libmesa_anv_entrypoints
>> +
> 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.

Thanks!

// Tapani