[Mesa-dev] [PATCH 1/2] intel: aubinator: generate a standalone binary
Ben Widawsky
ben at bwidawsk.net
Tue Oct 4 16:26:39 UTC 2016
On 16-10-04 15:38:52, Lionel Landwerlin wrote:
>Embed the xml files into the binary, so aubinator can be used from any
>location.
>
>Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
>Cc: Sirisha Gandikota <Sirisha.Gandikota at intel.com>
>---
> 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 $< > $@
>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;
>+ }
>+ }
>+
>+ if (gen == NULL) {
Since we're redoing this, I'm wondering if it makes sense to use "name" at all.
Maybe just the actual gen major/minor would be better? Just a thought.
> 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);
LGTM. Might want to run it by Kristian in terms of moving away from the XML
parsing. It's hard to tell if he did it because it seemed fun, or because it was
for some use (honestly, being able to change XML without rebuilding does seem
kind of nice to me - but I don't care much either way).
More information about the mesa-dev
mailing list