On Tue, Oct 4, 2016 at 7:38 AM, Lionel Landwerlin <[email protected]> wrote:
> Embed the xml files into the binary, so aubinator can be used from any > location. > Thank you for doing this!!! Dynamically loading the xml has a few benefits, but I think the benefits of not having to find them on disk are far bigger. > Signed-off-by: Lionel Landwerlin <[email protected]> > Cc: Sirisha Gandikota <[email protected]> > --- > src/intel/Makefile.am | 1 + > src/intel/Makefile.aubinator.am | 36 +++++++++++++++ > src/intel/Makefile.sources | 7 +++ > src/intel/tools/.gitignore | 5 +++ > src/intel/tools/aubinator.c | 97 +++++++++++++++++------------- > ----------- > src/intel/tools/decoder.c | 82 ++++++++++++++++++++-------------- > src/intel/tools/decoder.h | 4 +- > 7 files changed, 141 insertions(+), 91 deletions(-) > create mode 100644 src/intel/Makefile.aubinator.am > > diff --git a/src/intel/Makefile.am b/src/intel/Makefile.am > index 9186b5c..c3cb9fb 100644 > --- a/src/intel/Makefile.am > +++ b/src/intel/Makefile.am > @@ -52,6 +52,7 @@ BUILT_SOURCES = > CLEANFILES = > EXTRA_DIST = > > +include Makefile.aubinator.am > include Makefile.blorp.am > include Makefile.common.am > include Makefile.genxml.am > diff --git a/src/intel/Makefile.aubinator.am b/src/intel/Makefile. > aubinator.am > new file mode 100644 > index 0000000..9772700 > --- /dev/null > +++ b/src/intel/Makefile.aubinator.am > @@ -0,0 +1,36 @@ > +# Copyright © 2016 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 (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. > + > +BUILT_SOURCES += $(AUBINATOR_GENERATED_FILES) > + > +SUFFIXES = _aubinator_xml.h .xml > + > +tools/gen6_aubinator_xml.h: genxml/gen6.xml > +tools/gen7_aubinator_xml.h: genxml/gen7.xml > +tools/gen75_aubinator_xml.h: genxml/gen75.xml > +tools/gen8_aubinator_xml.h: genxml/gen8.xml > +tools/gen9_aubinator_xml.h: genxml/gen9.xml > + > +$(AUBINATOR_GENERATED_FILES): Makefile > + > +%_aubinator_xml.h: > + $(MKDIR_GEN) > + $(AM_V_GEN) xxd -i $< > $@ > We should probably check for xxd in configure.ac and use $(XXD) here. I know that it's not installed by default on Fedora, so just using it is kind-of mean. > diff --git a/src/intel/Makefile.sources b/src/intel/Makefile.sources > index 94073d2..a5c2bf0 100644 > --- a/src/intel/Makefile.sources > +++ b/src/intel/Makefile.sources > @@ -1,3 +1,10 @@ > +AUBINATOR_GENERATED_FILES = \ > + tools/gen6_aubinator_xml.h \ > + tools/gen7_aubinator_xml.h \ > + tools/gen75_aubinator_xml.h \ > + tools/gen8_aubinator_xml.h \ > + tools/gen9_aubinator_xml.h > + > BLORP_FILES = \ > blorp/blorp.c \ > blorp/blorp.h \ > diff --git a/src/intel/tools/.gitignore b/src/intel/tools/.gitignore > index 0c80a6f..c4eebde 100644 > --- a/src/intel/tools/.gitignore > +++ b/src/intel/tools/.gitignore > @@ -1 +1,6 @@ > /aubinator > +gen6_aubinator_xml.h > +gen75_aubinator_xml.h > +gen7_aubinator_xml.h > +gen8_aubinator_xml.h > +gen9_aubinator_xml.h > diff --git a/src/intel/tools/aubinator.c b/src/intel/tools/aubinator.c > index a31dcb2..83328b5 100644 > --- a/src/intel/tools/aubinator.c > +++ b/src/intel/tools/aubinator.c > @@ -35,6 +35,8 @@ > #include <sys/wait.h> > #include <sys/mman.h> > > +#include "util/macros.h" > + > #include "decoder.h" > #include "intel_aub.h" > #include "gen_disasm.h" > @@ -1059,11 +1061,24 @@ int main(int argc, char *argv[]) > { > struct gen_spec *spec; > struct aub_file *file; > - int i, pci_id = 0; > + int i; > bool found_arg_gen = false, pager = true; > - int gen_major, gen_minor; > - const char *value; > - char gen_file[256], gen_val[24]; > + const char *value, *input_file = NULL; > + char gen_val[24]; > + const struct { > + const char *name; > + int pci_id; > + } gens[] = { > + { "ivb", 0x0166 }, /* Intel(R) Ivybridge Mobile GT2 */ > + { "hsw", 0x0416 }, /* Intel(R) Haswell Mobile GT2 */ > + { "byt", 0x0155 }, /* Intel(R) Bay Trail */ > + { "bdw", 0x1616 }, /* Intel(R) HD Graphics 5500 (Broadwell GT2) */ > + { "chv", 0x22B3 }, /* Intel(R) HD Graphics (Cherryview) */ > + { "skl", 0x1912 }, /* Intel(R) HD Graphics 530 (Skylake GT2) */ > + { "kbl", 0x591D }, /* Intel(R) Kabylake GT2 */ > + { "bxt", 0x0A84 } /* Intel(R) HD Graphics (Broxton) */ > + }, *gen = NULL; > + struct gen_device_info devinfo; > > if (argc == 1) { > print_help(argv[0], stderr); > @@ -1081,8 +1096,6 @@ int main(int argc, char *argv[]) > exit(EXIT_FAILURE); > } > found_arg_gen = true; > - gen_major = 0; > - gen_minor = 0; > snprintf(gen_val, sizeof(gen_val), "%s", value); > } else if (strcmp(argv[i], "--headers") == 0) { > option_full_decode = false; > @@ -1105,6 +1118,7 @@ int main(int argc, char *argv[]) > fprintf(stderr, "unknown option %s\n", argv[i]); > exit(EXIT_FAILURE); > } > + input_file = argv[i]; > break; > } > } > @@ -1114,52 +1128,26 @@ int main(int argc, char *argv[]) > exit(EXIT_FAILURE); > } > > - if (strstr(gen_val, "ivb") != NULL) { > - /* Intel(R) Ivybridge Mobile GT2 */ > - pci_id = 0x0166; > - gen_major = 7; > - gen_minor = 0; > - } else if (strstr(gen_val, "hsw") != NULL) { > - /* Intel(R) Haswell Mobile GT2 */ > - pci_id = 0x0416; > - gen_major = 7; > - gen_minor = 5; > - } else if (strstr(gen_val, "byt") != NULL) { > - /* Intel(R) Bay Trail */ > - pci_id = 0x0155; > - gen_major = 7; > - gen_minor = 5; > - } else if (strstr(gen_val, "bdw") != NULL) { > - /* Intel(R) HD Graphics 5500 (Broadwell GT2) */ > - pci_id = 0x1616; > - gen_major = 8; > - gen_minor = 0; > - } else if (strstr(gen_val, "chv") != NULL) { > - /* Intel(R) HD Graphics (Cherryview) */ > - pci_id = 0x22B3; > - gen_major = 8; > - gen_minor = 0; > - } else if (strstr(gen_val, "skl") != NULL) { > - /* Intel(R) HD Graphics 530 (Skylake GT2) */ > - pci_id = 0x1912; > - gen_major = 9; > - gen_minor = 0; > - } else if (strstr(gen_val, "kbl") != NULL) { > - /* Intel(R) Kabylake GT2 */ > - pci_id = 0x591D; > - gen_major = 9; > - gen_minor = 0; > - } else if (strstr(gen_val, "bxt") != NULL) { > - /* Intel(R) HD Graphics (Broxton) */ > - pci_id = 0x0A84; > - gen_major = 9; > - gen_minor = 0; > - } else { > + for (i = 0; i < ARRAY_SIZE(gens); i++) { > + if (!strcmp(gen_val, gens[i].name)) { > + gen = &gens[i]; > + break; > + } > + } > I like this change but it feels like something that should be in it's own patch. > + > + if (gen == NULL) { > fprintf(stderr, "can't parse gen: %s, expected ivb, byt, hsw, " > "bdw, chv, skl, kbl or bxt\n", gen_val); > exit(EXIT_FAILURE); > } > > + if (!gen_get_device_info(gen->pci_id, &devinfo)) { > + fprintf(stderr, "can't find device information: pci_id=0x%x > name=%s\n", > + gen->pci_id, gen->name); > + exit(EXIT_FAILURE); > + } > + > + > /* Do this before we redirect stdout to pager. */ > if (option_color == COLOR_AUTO) > option_color = isatty(1) ? COLOR_ALWAYS : COLOR_NEVER; > @@ -1167,21 +1155,14 @@ int main(int argc, char *argv[]) > if (isatty(1) && pager) > setup_pager(); > > - if (gen_minor > 0) { > - snprintf(gen_file, sizeof(gen_file), "../genxml/gen%d%d.xml", > - gen_major, gen_minor); > - } else { > - snprintf(gen_file, sizeof(gen_file), "../genxml/gen%d.xml", > gen_major); > - } > - > - spec = gen_spec_load(gen_file); > - disasm = gen_disasm_create(pci_id); > + spec = gen_spec_load(&devinfo); > + disasm = gen_disasm_create(gen->pci_id); > > - if (argv[i] == NULL) { > + if (input_file == NULL) { > print_help(argv[0], stderr); > exit(EXIT_FAILURE); > } else { > - file = aub_file_open(argv[i]); > + file = aub_file_open(input_file); > } > > while (aub_file_more_stuff(file)) > diff --git a/src/intel/tools/decoder.c b/src/intel/tools/decoder.c > index be3558b..f3edc71 100644 > --- a/src/intel/tools/decoder.c > +++ b/src/intel/tools/decoder.c > @@ -32,6 +32,12 @@ > > #include "decoder.h" > > +#include "gen6_aubinator_xml.h" > +#include "gen7_aubinator_xml.h" > +#include "gen75_aubinator_xml.h" > +#include "gen8_aubinator_xml.h" > +#include "gen9_aubinator_xml.h" > + > #define XML_BUFFER_SIZE 4096 > > #define MAKE_GEN(major, minor) ( ((major) << 8) | (minor) ) > @@ -394,57 +400,69 @@ character_data(void *data, const XML_Char *s, int > len) > { > } > > +static void * > +devinfo_to_xml_data(const struct gen_device_info *devinfo, > + uint32_t *data_length) > +{ > + switch (devinfo->gen) { > + case 6: > + *data_length = sizeof(genxml_gen6_xml); > + return genxml_gen6_xml; > + case 7: > + if (devinfo->is_haswell) { > + *data_length = sizeof(genxml_gen75_xml); > + return genxml_gen75_xml; > + } > + *data_length = sizeof(genxml_gen7_xml); > + return genxml_gen7_xml; > + case 8: > + *data_length = sizeof(genxml_gen8_xml); > + return genxml_gen8_xml; > + case 9: > + *data_length = sizeof(genxml_gen9_xml); > + return genxml_gen9_xml; > + default: > + return NULL; > + } > +} > + > struct gen_spec * > -gen_spec_load(const char *filename) > +gen_spec_load(const struct gen_device_info *devinfo) > { > struct parser_context ctx; > - void *buf; > - int len; > - FILE *input; > - > - input = fopen(filename, "r"); > - printf("xml filename = %s\n", filename); > - if (input == NULL) { > - fprintf(stderr, "failed to open xml description\n"); > - exit(EXIT_FAILURE); > - } > + void *buf, *data; > + uint32_t data_length = 0; > > memset(&ctx, 0, sizeof ctx); > ctx.parser = XML_ParserCreate(NULL); > XML_SetUserData(ctx.parser, &ctx); > if (ctx.parser == NULL) { > fprintf(stderr, "failed to create parser\n"); > - fclose(input); > return NULL; > } > > XML_SetElementHandler(ctx.parser, start_element, end_element); > XML_SetCharacterDataHandler(ctx.parser, character_data); > - ctx.loc.filename = filename; > > ctx.spec = xzalloc(sizeof(*ctx.spec)); > > - do { > - buf = XML_GetBuffer(ctx.parser, XML_BUFFER_SIZE); > - len = fread(buf, 1, XML_BUFFER_SIZE, input); > - if (len < 0) { > - fprintf(stderr, "fread: %m\n"); > - fclose(input); > - return NULL; > - } > - if (XML_ParseBuffer(ctx.parser, len, len == 0) == 0) { > - fprintf(stderr, > - "Error parsing XML at line %ld col %ld: %s\n", > - XML_GetCurrentLineNumber(ctx.parser), > - XML_GetCurrentColumnNumber(ctx.parser), > - XML_ErrorString(XML_GetErrorCode(ctx.parser))); > - fclose(input); > - return NULL; > - } > - } while (len > 0); > + > + data = devinfo_to_xml_data(devinfo, &data_length); > + buf = XML_GetBuffer(ctx.parser, data_length); > + > + memcpy(buf, data, data_length); > + > + if (XML_ParseBuffer(ctx.parser, data_length, data_length == 0) == 0) { > + fprintf(stderr, > + "Error parsing XML at line %ld col %ld: %s\n", > + XML_GetCurrentLineNumber(ctx.parser), > + XML_GetCurrentColumnNumber(ctx.parser), > + XML_ErrorString(XML_GetErrorCode(ctx.parser))); > + XML_ParserFree(ctx.parser); > + return NULL; > + } > > XML_ParserFree(ctx.parser); > - fclose(input); > > return ctx.spec; > } > diff --git a/src/intel/tools/decoder.h b/src/intel/tools/decoder.h > index f688ba5..5bea9b9 100644 > --- a/src/intel/tools/decoder.h > +++ b/src/intel/tools/decoder.h > @@ -26,6 +26,8 @@ > #include <stdint.h> > #include <stdbool.h> > > +#include "common/gen_device_info.h" > + > struct gen_spec; > struct gen_group; > struct gen_field; > @@ -36,7 +38,7 @@ static inline uint32_t gen_make_gen(uint32_t major, > uint32_t minor) > } > > struct gen_group *gen_spec_find_struct(struct gen_spec *spec, const char > *name); > -struct gen_spec *gen_spec_load(const char *filename); > +struct gen_spec *gen_spec_load(const struct gen_device_info *devinfo); > uint32_t gen_spec_get_gen(struct gen_spec *spec); > struct gen_group *gen_spec_find_instruction(struct gen_spec *spec, const > uint32_t *p); > struct gen_group *gen_spec_find_register(struct gen_spec *spec, uint32_t > offset); > -- > 2.9.3 > > _______________________________________________ > mesa-dev mailing list > [email protected] > https://lists.freedesktop.org/mailman/listinfo/mesa-dev >
_______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
