[Mesa-dev] [PATCH] vc4: Introduce XML-based packet header generation like Intel's.

[Emil Velikov]

Hi Eric,

On 28 June 2017 at 19:18, Eric Anholt <eric at anholt.net> wrote:
> I really liked this idea, as it should help with management of packet
> parsing tools like the CL dump.  The python script is forked off of theirs
> because our packets are byte-based instead of dwords, and the changes to
> do so while avoiding performance regressions due to unaligned accesses
> were quite invasive.
> ---
>
> I'm hoping for an ack from Jason or Kenneth on the genxml script fork
> to the new location, and an Android test from Rob.  Full branch using
> the XML stuff is at vc4-xml of my Mesa tree.
>
Even though I'm not one of the above people, I've spotted a few things
which I hope you'll find useful.

> --- a/src/mesa/Android.libmesa_git_sha1.mk
> +++ b/src/broadcom/Android.genxml.mk

> +# ---------------------------------------
> +# Build libmesa_genxml
Does not match with actual name below. Update or simply drop the comment?

> +# ---------------------------------------
>
>  include $(CLEAR_VARS)
>
> -LOCAL_MODULE := libmesa_git_sha1
> +LOCAL_MODULE := libmesa_broadcom_genxml
>
Nicely done on renaming the lib. Otherwise it would have conflicted
with the Intel one or gone unnoticed if one builds w/o Intel.

> +$(intermediates)/genxml/v3d_packet_v21_pack.h: PRIVATE_SCRIPT := $(MESA_PYTHON2) $(LOCAL_PATH)/cle/gen_pack_header.py
> +$(intermediates)/genxml/v3d_packet_v21_pack.h: PRIVATE_XML := $(LOCAL_PATH)/cle/v3d_packet_v21.xml
> +$(intermediates)/genxml/v3d_packet_v21_pack.h: $(LOCAL_PATH)/cle/v3d_packet_v21.xml $(LOCAL_PATH)/cle/gen_pack_header.py
Based on the .gitignore and Autotools build you want s/genxml/cle/ here.

> +       $(call header-gen)
> +
> +LOCAL_EXPORT_C_INCLUDE_DIRS := \
> +       $(MESA_TOP)/src/intel \
s/intel/broadcom/ ?


> --- /dev/null
> +++ b/src/broadcom/Makefile.sources
> @@ -0,0 +1,12 @@
> +BROADCOM_GENXML_GENERATED_FILES = \
> +       cle/v3d_packet_v21_pack.h \
> +       $()
> +
> +BROADCOM_GENXML_XML_FILES = \
> +       cle/v3d_packet_v21.xml \
> +       $()
> +
> +BROADCOM_FILES = \
> +       cle/v3d_packet_helpers.h \
> +       $()
> +
Can I bribe you to drop the $() lines?


> --- /dev/null
> +++ b/src/broadcom/cle/gen_pack_header.py
> @@ -0,0 +1,547 @@
> +#!/usr/bin/env python2
Please drop this line. Afaict the script should be python3 safe, plus
we use manually invoke correct version of python and strip the file
execute bit.


-Emil