[Mesa-dev] [PATCH 00/11] intel/tools: Unify batch decoding between aubinators

[Lionel Landwerlin]

This looks really good :)
I can't find anything to nitpick :

Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>


On 13/12/17 20:05, Jason Ekstrand wrote:
> Both aubinator and aubinator_error_decode try and do the same task of
> decoding batches.  They both have code to try and decode various things
> such as shaders from the batch.  All of that code is completely different
> between the two.
>
> This little series reworks the two to use a common gen_print_batch
> function.  In order to handle dynamic states, a callback is provided that
> lets the decoder ask for a mapped buffer given a memory address.  The basic
> structure of the new code is taken from aubinator but many of the state
> decoding details are taken from aubinator_error_decode.  This new version
> should have all the goodness of both.  In particular, aubinator_error_decode
> should now properly handle 48-bit addresses (not well tested) and it should
> also be able to handle batch chaining.  Meanwhile, aubinator gains the
> field-based decoding scheme so there are a lot fewer gen-specific manual
> decoding paths.  Everyone wins!
>
> The reason I haven't converted INTEL_DEBUG=bat yet is treefold:
>
>   1) We really should come up with some mechanism to say how many dynamic
>      states you have for a given pointers packet.  We could do some sort of
>      callback but, if we want it to work for aubinator or error_decode, we
>      will need some form of batch annotations.
>
>   2) I'm not convinced I didn't break gen4-5.  (I don't think it worked in
>      either of those tools in the first place).
>
>   3) I put it in src/intel/tools because I really don't want to rewrite the
>      build system right now to put it somewhere more common.  We can't put
>      it in src/intel/common along with the other decoder stuff because it
>      depends on the compiler and we don't want i965 to depend on
>      src/intel/tools.
>
> Jason Ekstrand (11):
>    intel/decoder: Expose the raw field value in the iterator
>    intel/tools: Add the start of a generic batch decoder
>    intel/batch-decoder: Decode MEDIA_INTERFACE_DESCRIPTOR_LOAD
>    intel/batch-decoder: Decode vertex and index buffers
>    intel/batch-decoder: Decode graphics shaders
>    intel/tools: Switch aubinator_error_decode over to the gen_print_batch
>    intel/batch-decoder: Decode constants, binding tables, and samplers
>    intel/decoder: Add a gen_print_group_length helper
>    intel/batch-decoder: Decode dynamic state
>    intel/batch-decoder: Decode registers
>    intel/tools: Convert aubinator over to the common framework
>
>   src/intel/Makefile.tools.am              |   2 +
>   src/intel/common/gen_decoder.c           |  20 +-
>   src/intel/common/gen_decoder.h           |  56 +++
>   src/intel/tools/aubinator.c              | 719 ++------------------------
>   src/intel/tools/aubinator_error_decode.c | 238 ++-------
>   src/intel/tools/gen_batch_decoder.c      | 834 +++++++++++++++++++++++++++++++
>   src/intel/tools/meson.build              |   6 +-
>   7 files changed, 976 insertions(+), 899 deletions(-)
>   create mode 100644 src/intel/tools/gen_batch_decoder.c
>