On 01/24/2017 11:18 AM, Tapani Pälli wrote:


On 01/20/2017 03:09 PM, Emil Velikov wrote:
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.

Fair enough I've moved this, will be sending new version.

 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.

ok


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

After reading these I'm still a bit puzzled what you mean here but let's
have some IRC time about this?

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

ok


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
?

It might be the case. I have noticed other slight differences, for
example 'anv_CreateSwapchainKHR' has HIDDEN visibility in the resulting
Android binary but has DEFAULT visibility on my regular desktop build,
probably the reason why dEQP is not so happy on those swapchain tests :)

Doh forget about this, this function was bad example. This symbol (and other swapchain functions) is exposed and implemented in Android frameworks libvulkan.so.


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
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to