Re: [Intel-gfx] [PATCH v3 i-g-t 1/2] tools: intel_aubdump: pass configuration through file descriptor

[Gandikota, Sirisha]

>-Original Message-
>From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of
>Lionel Landwerlin
>Sent: Tuesday, November 01, 2016 11:15 AM
>To: intel-gfx@lists.freedesktop.org
>Subject: [Intel-gfx] [PATCH v3 i-g-t 1/2] tools: intel_aubdump: pass 
>configuration
>through file descriptor
>
>This makes parsing options less complicated and easier to extend.
>
>v2: Fix device id parsing (atoi -> sscanf) (Sirisha)
>Combine with previous commit moving init function (Sirisha)
>
>v3: Fix behavior change between bash 4.3 & 4.4 in <<< with \n characters
>(Lionel)
>
>Signed-off-by: Lionel Landwerlin 
>---
> tools/aubdump.c| 58 
>--
> tools/intel_aubdump.in | 24 +++--
> 2 files changed, 59 insertions(+), 23 deletions(-)
>
>diff --git a/tools/aubdump.c b/tools/aubdump.c index 30dc742..f2cd2c1 100644
>--- a/tools/aubdump.c
>+++ b/tools/aubdump.c
>@@ -426,6 +426,46 @@ close(int fd)
>   return libc_close(fd);
> }
>
>+static void
>+maybe_init(void)
>+{
>+  static bool initialized = false;
>+  FILE *config;
>+  char *key, *value;
>+
>+  if (initialized)
>+  return;
>+
>+  initialized = true;
>+
>+  config = fdopen(3, "r");
>+  while (fscanf(config, "%m[^=]=%m[^\n]\n", , ) != EOF) {
>+  if (!strcmp(key, "verbose")) {
>+  verbose = 1;
>+  } else if (!strcmp(key, "device")) {
>+  fail_if(sscanf(value, "%i", ) != 1,
>+  "intel_aubdump: failed to parse device id '%s'",
>+  value);
>+  device_override = true;
>+  } else if (!strcmp(key, "file")) {
>+  filename = value;
>+  file = fopen(filename, "w+");
>+  fail_if(file == NULL,
>+  "intel_aubdump: failed to open file '%s'\n",
>+  filename);
>+  } else {
>+  fprintf(stderr, "intel_aubdump: unknown option '%s'\n",
>key);
>+  }
>+
>+  free(key);
>+  free(value);
>+  }
>+  fclose(config);
>+
>+  bos = malloc(MAX_BO_COUNT * sizeof(bos[0]));
>+  fail_if(bos == NULL, "intel_aubdump: out of memory\n"); }
>+
> int
> ioctl(int fd, unsigned long request, ...)  { @@ -447,6 +487,8 @@ ioctl(int fd,
>unsigned long request, ...)
>   }
>
>   if (fd == drm_fd) {
>+  maybe_init();
>+
>   switch (request) {
>   case DRM_IOCTL_I915_GETPARAM: {
>   struct drm_i915_getparam *getparam = argp; @@ -
>550,26 +592,10 @@ ioctl(int fd, unsigned long request, ...)  static void
> init(void)
> {
>-  const char *args = getenv("INTEL_AUBDUMP_ARGS");
>-
>   libc_close = dlsym(RTLD_NEXT, "close");
>   libc_ioctl = dlsym(RTLD_NEXT, "ioctl");
>   fail_if(libc_close == NULL || libc_ioctl == NULL,
>   "intel_aubdump: failed to get libc ioctl or close\n");
>-
>-  if (sscanf(args, "verbose=%d;file=%m[^;];device=%i",
>- , , ) != 3)
>-  filename = strdup("intel.aub");
>-  fail_if(filename == NULL, "intel_aubdump: out of memory\n");
>-
>-  if (device)
>-  device_override = true;
>-
>-  bos = malloc(MAX_BO_COUNT * sizeof(bos[0]));
>-  fail_if(bos == NULL, "intel_aubdump: out of memory\n");
>-
>-  file = fopen(filename, "w+");
>-  fail_if(file == NULL, "intel_aubdump: failed to open file '%s'\n",
>filename);
> }
>
> static int
>diff --git a/tools/intel_aubdump.in b/tools/intel_aubdump.in index
>feee23a..18fd03b 100644
>--- a/tools/intel_aubdump.in
>+++ b/tools/intel_aubdump.in
>@@ -21,29 +21,38 @@ EOF
> exit 0
> }
>
>-verbose=0
>-device=0
>+args=""
>+command=""
>+file=""
>+
>+function add_arg() {
>+arg=$1
>+args="$args$arg\n"
>+}
>
> while true; do
>   case "$1" in
> -o)
> file=$2
>+add_arg "file=${f:-$(basename ${f}).aub}"
> shift 2
> ;;
> -v)
>-verbose=1
>+add_arg "verbose=1"
> shift 1
> ;;
> -o*)
> file=${1##-o}
>+add_arg "file=${file:-$(basename ${file}).aub}"
> shift
> ;;
> --output=*)
> file=${1##--output=}
>+add_arg "file=${file:-$(basename ${file}).aub}"
> shift
> ;;
> --device=*)
>-device=${1##--device=}
>+add_arg "device=${1##--device=}"
> shift
> ;;
> --help)
>@@ -66,12 +75,13 @@ done
>
> [ -z $1 ] && show_help
>
>-file=${file:-$(basename $1).aub}
>+[ -z $file ] && add_arg "file=intel.aub"
>
> prefix=@prefix@
> exec_prefix=@exec_prefix@
> libdir=@libdir@
>
> LD_PRELOAD=${libdir}/intel_aubdump.so${LD_PPRELOAD:+:${LD_PRELOAD}} \
>-  

Re: [Intel-gfx] [PATCH v4 i-g-t 2/2] aubdump: add --command option to stream aubdump to another program

[Gandikota, Sirisha]

>-Original Message-
>From: Landwerlin, Lionel G
>Sent: Tuesday, November 01, 2016 11:15 AM
>To: intel-gfx@lists.freedesktop.org
>Cc: Landwerlin, Lionel G <lionel.g.landwer...@intel.com>; Gandikota, Sirisha
><sirisha.gandik...@intel.com>
>Subject: [PATCH v4 i-g-t 2/2] aubdump: add --command option to stream
>aubdump to another program
>
>This comes handy if you want to look at your application output without having
>to save it into a file. For example, use this with aubinator from Mesa :
>
>$ intel_aubdump -c '/path/to/aubinator --gen=hsw' my_gl_app
>
>v2: Fix handling empty command line option
>
>v3: Fix command line concatenation (again...)
>
>v4: Use execvp (Petri)
>Indentation (Petri)
>Allow recording to a file and stream to an external application (Lionel)
>
>Signed-off-by: Lionel Landwerlin <lionel.g.landwer...@intel.com>
>Cc: Sirisha Gandikota <sirisha.gandik...@intel.com>
>---
> tools/aubdump.c| 83
>--
> tools/intel_aubdump.in | 26 +++-
> 2 files changed, 98 insertions(+), 11 deletions(-)
>
>diff --git a/tools/aubdump.c b/tools/aubdump.c index f2cd2c1..f7ef699 100644
>--- a/tools/aubdump.c
>+++ b/tools/aubdump.c
>@@ -43,6 +43,10 @@
> #include "intel_aub.h"
> #include "intel_chipset.h"
>
>+#ifndef ARRAY_SIZE
>+#define ARRAY_SIZE(x) (sizeof(x)/sizeof((x)[0])) #endif
>+
> static int close_init_helper(int fd);
> static int ioctl_init_helper(int fd, unsigned long request, ...);
>
>@@ -50,8 +54,8 @@ static int (*libc_close)(int fd) = close_init_helper;  
>static int
>(*libc_ioctl)(int fd, unsigned long request, ...) = ioctl_init_helper;
>
> static int drm_fd = -1;
>-static char *filename;
>-static FILE *file;
>+static char *filename = NULL;
>+static FILE *files[2] = { NULL, NULL };
> static int gen = 0;
> static int verbose = 0;
> static const uint32_t gtt_size = 0x1; @@ -140,13 +144,28 @@
>align_u64(uint64_t v, uint64_t a)  static void  dword_out(uint32_t data)  {
>-  fwrite(, 1, 4, file);
>+  for (int i = 0; i < ARRAY_SIZE (files); i++) {
>+  if (files[i] == NULL)
>+  continue;
>+
>+  fail_if(fwrite(, 1, 4, files[i]) == 0,
>+  "Writing to output failed\n");
>+  }
> }
>
> static void
> data_out(const void *data, size_t size)  {
>-  fwrite(data, 1, size, file);
>+  if (size == 0)
>+  return;
>+
>+  for (int i = 0; i < ARRAY_SIZE (files); i++) {
>+  if (files[i] == NULL)
>+  continue;
>+
>+  fail_if(fwrite(data, 1, size, files[i]) == 0,
>+  "Writing to output failed\n");
>+  }
> }
>
> static void
>@@ -393,7 +412,10 @@ dump_execbuffer2(int fd, struct
>drm_i915_gem_execbuffer2 *execbuffer2)
>   aub_dump_ringbuffer(batch_bo->offset + execbuffer2-
>>batch_start_offset,
>   offset, ring_flag);
>
>-  fflush(file);
>+  for (int i = 0; i < ARRAY_SIZE(files); i++) {
>+  if (files[i] != NULL)
>+  fflush(files[i]);
>+  }
> }
>
> static void
>@@ -426,6 +448,40 @@ close(int fd)
>   return libc_close(fd);
> }
>
>+static FILE *
>+launch_command(char *command)
>+{
>+  int i = 0, fds[2];
>+  char **args = calloc(strlen(command), sizeof(char *));
>+  char *iter = command;
>+
>+  args[i++] = iter = command;
>+
>+  while ((iter = strstr(iter, ",")) != NULL) {
>+  *iter = '\0';
>+  iter += 1;
>+  args[i++] = iter;
>+  }
>+
>+  if (pipe(fds) == -1)
>+  return NULL;
>+
>+  switch (fork()) {
>+  case 0:
>+  dup2(fds[0], 0);
>+  fail_if(execvp(args[0], args) == -1,
>+  "intel_aubdump: failed to launch child command\n");
>+  return NULL;
>+
>+  default:
>+  free(args);
>+  return fdopen(fds[1], "w");
>+
>+  case -1:
>+  return NULL;
>+  }
>+}
>+
> static void
> maybe_init(void)
> {
>@@ -448,11 +504,16 @@ maybe_init(void)
>   value);
>   device_override = true;
>   } else if (!strcmp(key, "file")) {
>-  filename = value;
>-  file = fopen(filename, "w+");
>-  fail_if(file == NULL,
>+  filename = strdup(value);
>+

Re: [Intel-gfx] [PATCH v4 i-g-t 3/3] aubdump: add --command option to stream aubdump to another program

[Gandikota, Sirisha]

>-Original Message-
>From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of
>Lionel Landwerlin
>Sent: Thursday, October 06, 2016 8:17 AM
>To: intel-gfx@lists.freedesktop.org
>Cc: Gandikota, Sirisha <sirisha.gandik...@intel.com>
>Subject: [Intel-gfx] [PATCH v4 i-g-t 3/3] aubdump: add --command option to
>stream aubdump to another program
>
>This comes handy if you want to look at your application output without having
>to save it into a file. For example, use this with aubinator from Mesa :
>
>$ intel_aubdump -c '/path/to/aubinator --gen=hsw' my_gl_app
>
>v2: Fix handling empty command line option
>
>v3: Fix command line concatenation (again...)
>
>v4: Use execvp (Petri)
>Indentation (Petri)
>Allow recording to a file and stream to an external application (Lionel)
>
>Signed-off-by: Lionel Landwerlin <lionel.g.landwer...@intel.com>
>Cc: Sirisha Gandikota <sirisha.gandik...@intel.com>
>---
> tools/aubdump.c| 81
>--
> tools/intel_aubdump.in | 26 +++-
> 2 files changed, 97 insertions(+), 10 deletions(-)
>
>diff --git a/tools/aubdump.c b/tools/aubdump.c index e82b514..61aa83c 100644
>--- a/tools/aubdump.c
>+++ b/tools/aubdump.c
>@@ -43,6 +43,10 @@
> #include "intel_aub.h"
> #include "intel_chipset.h"
>
>+#ifndef ARRAY_SIZE
>+#define ARRAY_SIZE(x) (sizeof(x)/sizeof((x)[0])) #endif
>+
> static int close_init_helper(int fd);
> static int ioctl_init_helper(int fd, unsigned long request, ...);
>
>@@ -50,8 +54,8 @@ static int (*libc_close)(int fd) = close_init_helper;  
>static int
>(*libc_ioctl)(int fd, unsigned long request, ...) = ioctl_init_helper;
>
> static int drm_fd = -1;
>-static char *filename;
>-static FILE *file;
>+static char *filename = NULL;
>+static FILE *files[2] = { NULL, NULL };
> static int gen = 0;
> static int verbose = 0;
> static const uint32_t gtt_size = 0x1; @@ -140,13 +144,28 @@
>align_u64(uint64_t v, uint64_t a)  static void  dword_out(uint32_t data)  {
>-  fwrite(, 1, 4, file);
>+  for (int i = 0; i < ARRAY_SIZE (files); i++) {
>+  if (files[i] == NULL)
>+  continue;
>+
>+  fail_if(fwrite(, 1, 4, files[i]) == 0,
>+  "Writing to output failed\n");
>+  }
> }
>
> static void
> data_out(const void *data, size_t size)  {
>-  fwrite(data, 1, size, file);
>+  if (size == 0)
>+  return;
>+
>+  for (int i = 0; i < ARRAY_SIZE (files); i++) {
>+  if (files[i] == NULL)
>+  continue;
>+
>+  fail_if(fwrite(data, 1, size, files[i]) == 0,
>+  "Writing to output failed\n");
>+  }
> }
>
> static void
>@@ -393,7 +412,10 @@ dump_execbuffer2(int fd, struct
>drm_i915_gem_execbuffer2 *execbuffer2)
>   aub_dump_ringbuffer(batch_bo->offset + execbuffer2-
>>batch_start_offset,
>   offset, ring_flag);
>
>-  fflush(file);
>+  for (int i = 0; i < ARRAY_SIZE(files); i++) {
>+  if (files[i] != NULL)
>+  fflush(files[i]);
>+  }
> }
>
> static void
>@@ -426,6 +448,40 @@ close(int fd)
>   return libc_close(fd);
> }
>
>+static FILE *
>+launch_command(char *command)
>+{
>+  int i = 0, fds[2];
>+  char **args = calloc(strlen(command), sizeof(char *));
>+  char *iter = command;
>+
>+  args[i++] = iter = command;
>+
>+  while ((iter = strstr(iter, ",")) != NULL) {
>+  *iter = '\0';
>+  iter += 1;
>+  args[i++] = iter;
>+  }
>+
>+  if (pipe(fds) == -1)
>+  return NULL;
>+
>+  switch (fork()) {
>+  case 0:
>+  dup2(fds[0], 0);
>+  fail_if(execvp(args[0], args) == -1,
>+  "intel_aubdump: failed to launch child command\n");
>+  return NULL;
>+
>+  default:
>+  free(args);
>+  return fdopen(fds[1], "w");
>+
>+  case -1:
>+  return NULL;
>+  }
>+}
>+
> static void
> maybe_init(void)
> {
>@@ -447,10 +503,15 @@ maybe_init(void)
>   device_override = true;
>   } else if (!strcmp(key, "file")) {
>   filename = value;
>-  file = fopen(filename, "w+");
>-  fail_if(file == NULL,
>+  files[0] = fopen(filename, "w+");

Re: [Intel-gfx] [PATCH i-g-t 1/3] tools: intel_aubdump: avoid initializing multiple times

[Gandikota, Sirisha]

>-Original Message-
>From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of
>Lionel Landwerlin
>Sent: Thursday, October 06, 2016 8:17 AM
>To: intel-gfx@lists.freedesktop.org
>Subject: [Intel-gfx] [PATCH i-g-t 1/3] tools: intel_aubdump: avoid initializing
>multiple times
>
>For some reason init() seems to be called multiple times. Let's move the
>initialization to the first ioctl().
>
>Signed-off-by: Lionel Landwerlin 
>---
> tools/aubdump.c | 44 
> 1 file changed, 28 insertions(+), 16 deletions(-)
>
>diff --git a/tools/aubdump.c b/tools/aubdump.c index 30dc742..a2ac7f1 100644
>--- a/tools/aubdump.c
>+++ b/tools/aubdump.c
>@@ -426,6 +426,32 @@ close(int fd)
>   return libc_close(fd);
> }
>
>+static void
>+maybe_init(void)
>+{
>+  static bool initialized = false;
>+  const char *args = getenv("INTEL_AUBDUMP_ARGS");
>+
>+  if (initialized)
>+  return;
>+
>+  initialized = true;
>+
>+  if (sscanf(args, "verbose=%d;file=%m[^;];device=%i",
>+ , , ) != 3)
>+  filename = strdup("intel.aub");
>+  fail_if(filename == NULL, "intel_aubdump: out of memory\n");
>+
>+  if (device)
>+  device_override = true;
>+
>+  bos = malloc(MAX_BO_COUNT * sizeof(bos[0]));
>+  fail_if(bos == NULL, "intel_aubdump: out of memory\n");
>+
>+  file = fopen(filename, "w+");
>+  fail_if(file == NULL, "intel_aubdump: failed to open file '%s'\n",
>+filename); }
>+

[SG]  Since you are adding a new method anyway, I would combine patches 1 and 2 
and send only 1 patch instead. Patch 2 is also working on the same new method 
with few additional changes. Additionally, please verify if intel_aubdump works 
with --device= option. It failed for me for 0x1602 (bdw) but works 
without device option on system (hsw in my case).

> int
> ioctl(int fd, unsigned long request, ...)  { @@ -447,6 +473,8 @@ ioctl(int fd,
>unsigned long request, ...)
>   }
>
>   if (fd == drm_fd) {
>+  maybe_init();
>+
>   switch (request) {
>   case DRM_IOCTL_I915_GETPARAM: {
>   struct drm_i915_getparam *getparam = argp; @@ -
>550,26 +578,10 @@ ioctl(int fd, unsigned long request, ...)  static void
> init(void)
> {
>-  const char *args = getenv("INTEL_AUBDUMP_ARGS");
>-
>   libc_close = dlsym(RTLD_NEXT, "close");
>   libc_ioctl = dlsym(RTLD_NEXT, "ioctl");
>   fail_if(libc_close == NULL || libc_ioctl == NULL,
>   "intel_aubdump: failed to get libc ioctl or close\n");
>-
>-  if (sscanf(args, "verbose=%d;file=%m[^;];device=%i",
>- , , ) != 3)
>-  filename = strdup("intel.aub");
>-  fail_if(filename == NULL, "intel_aubdump: out of memory\n");
>-
>-  if (device)
>-  device_override = true;
>-
>-  bos = malloc(MAX_BO_COUNT * sizeof(bos[0]));
>-  fail_if(bos == NULL, "intel_aubdump: out of memory\n");
>-
>-  file = fopen(filename, "w+");
>-  fail_if(file == NULL, "intel_aubdump: failed to open file '%s'\n",
>filename);
> }
>
> static int
>--
>2.9.3
>
>___
>Intel-gfx mailing list
>Intel-gfx@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx