[Mesa-dev] [PATCH] android: vulkan: add support for libmesa_vulkan_{util, wsi}

Tue Feb 28 16:05:28 UTC 2017

On Tuesday, 2017-02-28 14:06:14 +0000, Emil Velikov wrote:
> On 28 February 2017 at 11:58, Eric Engestrom <eric.engestrom at imgtec.com> wrote:
> > On Tuesday, 2017-02-28 01:31:01 +0100, Mauro Rossi wrote:
> >> Thanks a lot for your feedbacks.
> >> Here follows final version.
> >> Mauro
> >>
> >> From d0db1d0fd07fd0b5fb7580bb8dddf109389f28ba Mon Sep 17 00:00:00 2001
> >> From: Mauro Rossi <issor.oruam at gmail.com>
> >> Date: Tue, 28 Feb 2017 01:24:41 +0100
> >> Subject: [PATCH] android: vulkan: add support for libmesa_vulkan_util
> >>
> >> The following changes are implemented:
> >>
> >> Add src/vulkan/Android.mk to build libmesa_vulkan_util
> >> Android.mk: add src/vulkan to SUBDIR to build new module
> >> intel/vulkan: fix libmesa_vulkan_util,vk_enum_to_str.h dependencies
> >> Add -o OUTPUT_PATH option in src/vulkan/util/gen_enum_to_str.py script
> >> Use -o OUTPUT_PATH option in automake generation rules for vk_enum_to_str.{c,h}
> >>
> >> Fixes: e9dcb17 "vulkan/util: Add generator for enum_to_str functions"
> >> Fixes: 8e03250 "vulkan: Combine wsi and util makefiles"
> >> ---
> >>  Android.mk                         |  3 ++-
> >>  src/intel/vulkan/Android.mk        |  8 ++++--
> >>  src/vulkan/Android.mk              | 55 ++++++++++++++++++++++++++++++++++++++
> >>  src/vulkan/Makefile.am             |  2 +-
> >>  src/vulkan/util/gen_enum_to_str.py |  9 +++++--
> >>  5 files changed, 71 insertions(+), 6 deletions(-)
> >>  create mode 100644 src/vulkan/Android.mk
> >>
> >> diff --git a/Android.mk b/Android.mk
> >> index 4168b4d..bb70321 100644
> >> --- a/Android.mk
> >> +++ b/Android.mk
> >> @@ -92,7 +92,8 @@ SUBDIRS := \
> >>   src/egl \
> >>   src/amd \
> >>   src/intel \
> >> - src/mesa/drivers/dri
> >> + src/mesa/drivers/dri \
> >> + src/vulkan
> >>
> >>  INC_DIRS := $(call all-named-subdir-makefiles,$(SUBDIRS))
> >>
> >> diff --git a/src/intel/vulkan/Android.mk b/src/intel/vulkan/Android.mk
> >> index 1e53970..a6a7d26 100644
> >> --- a/src/intel/vulkan/Android.mk
> >> +++ b/src/intel/vulkan/Android.mk
> >> @@ -74,7 +74,8 @@ include $(BUILD_STATIC_LIBRARY)
> >>  ANV_INCLUDES := \
> >>   $(VULKAN_COMMON_INCLUDES) \
> >>   $(call generated-sources-dir-for,STATIC_LIBRARIES,libmesa_anv_entrypoints,,) \
> >> - $(call generated-sources-dir-for,STATIC_LIBRARIES,libmesa_nir,,)/nir
> >> + $(call generated-sources-dir-for,STATIC_LIBRARIES,libmesa_nir,,)/nir \
> >> + $(call generated-sources-dir-for,STATIC_LIBRARIES,libmesa_vulkan_util,,)/util
> >>
> >>  #
> >>  # libanv for gen7
> >> @@ -172,7 +173,10 @@ LOCAL_C_INCLUDES := \
> >>   $(ANV_INCLUDES) \
> >>   $(MESA_TOP)/src/compiler
> >>
> >> -LOCAL_WHOLE_STATIC_LIBRARIES := libmesa_anv_entrypoints libmesa_genxml
> >> +LOCAL_WHOLE_STATIC_LIBRARIES := \
> >> + libmesa_anv_entrypoints \
> >> + libmesa_genxml \
> >> + libmesa_vulkan_util
> >>
> >>  LOCAL_GENERATED_SOURCES += $(intermediates)/anv_entrypoints.c
> >>
> >> diff --git a/src/vulkan/Android.mk b/src/vulkan/Android.mk
> >> new file mode 100644
> >> index 0000000..871d7f3
> >> --- /dev/null
> >> +++ b/src/vulkan/Android.mk
> >> @@ -0,0 +1,55 @@
> >> +# Copyright © 2017 Mauro Rossi <issor.oruam at gmail.com>
> >> +#
> >> +# 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 (including the next
> >> +# paragraph) 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)
> >> +
> >> +# Import variables
> >> +include $(LOCAL_PATH)/Makefile.sources
> >> +
> >> +#
> >> +# libmesa_vulkan_util
> >> +#
> >> +
> >> +include $(CLEAR_VARS)
> >> +LOCAL_MODULE := libmesa_vulkan_util
> >> +LOCAL_MODULE_CLASS := STATIC_LIBRARIES
> >> +
> >> +intermediates := $(call local-generated-sources-dir)
> >> +
> >> +LOCAL_C_INCLUDES := \
> >> + $(MESA_TOP)/include/vulkan
> >> +
> >> +LOCAL_GENERATED_SOURCES := \
> >> + $(intermediates)/util/vk_enum_to_str.c \
> >> + $(intermediates)/util/vk_enum_to_str.h
> >> +
> >> +vulkan_api_xml = $(MESA_TOP)/src/vulkan/registry/vk.xml
> >> +
> >> +$(LOCAL_GENERATED_SOURCES):
> >> $(MESA_TOP)/src/vulkan/util/gen_enum_to_str.py $(vulkan_api_xml)
> >> + @echo "target Generated: $(PRIVATE_MODULE) <= $(notdir $(@))"
> >> + @mkdir -p $(dir $@)
> >> + $(hide) $(MESA_PYTHON2)
> >> $(MESA_TOP)/src/vulkan/util/gen_enum_to_str.py -o
> >> $(intermediates)/util
> >> +
> >> +LOCAL_EXPORT_C_INCLUDE_DIRS := \
> >> +        $(intermediates)
> >> +
> >> +include $(MESA_COMMON_MK)
> >> +include $(BUILD_STATIC_LIBRARY)
> >> diff --git a/src/vulkan/Makefile.am b/src/vulkan/Makefile.am
> >> index 5cdffbf..019da13 100644
> >> --- a/src/vulkan/Makefile.am
> >> +++ b/src/vulkan/Makefile.am
> >> @@ -17,7 +17,7 @@ BUILT_SOURCES = \
> >>
> >>  util/vk_enum_to_str.c util/vk_enum_to_str.h: util/gen_enum_to_str.py
> >> $(vulkan_api_xml)
> >>   $(MKDIR_GEN)
> >> - $(PYTHON_GEN) $(srcdir)/util/gen_enum_to_str.py
> >> + $(PYTHON_GEN) $(srcdir)/util/gen_enum_to_str.py -o
> >> $(top_builddir)/src/vulkan/util
> >
> > Just like the other Makefile patch a few days ago, this rule will run
> > the python script twice, script which writes both files.
> > Not performant, race condition, etc.
> >
> > The clean fix is to give `-o $@` to the script (ie. the target of the rule)
> > and add an `if` in the script to only write the file given.
> >
> > The quick and dirty fix is to add a rule forcing the serialisation, like
> > this:
> >   util/vk_enum_to_str.c: util/vk_enum_to_str.h
> > This fixes the race condition, but still writes both files twice for no
> > reason.
> >
> Are you sure that will generate them twice - can you elaborate a bit ?

You can try it yourself with this makefile:
----8<----
all: foo bar
foo bar:
	echo $@
---->8----

$ make
echo foo
foo
echo bar
bar

It runs both targets, regardless of the fact they have the same rule.

> 
> We can always purging the one of the two files from BUILT_SOURCES,
> which should do it right ?

It would, but then you'd also loose the dependency. If you were to
modify that file (testing a change for instance), `make` wouldn't
pick up the change anymore.

> 
> In either case I think those are simpler workarounds than writing more
> python code ;-)
> I think that we want to fix the whole tree in one go, otherwise people
> end up copying "the wrong" ones.

Agreed :)

> 
> -Emil