Re: [Intel-gfx] [PATCH i-g-t v5] overlay: parse tracepoints from sysfs to figure out fields' location

[Lionel Landwerlin]

On 19/12/17 15:15, Chris Wilson wrote:

Quoting Lionel Landwerlin (2017-12-19 14:26:11)

With changes going to drm-tip, the tracepoints field locations are
going to change. This change introduces a tracepoint parser (using a
peg parser) which lets us figure out field positions on the fly.

v2: Fix automake build (Lionel)

v3: Make overlay build conditional on peg (Petri)
 Make wait_end callback more readable (Chris)
 Drop tracepoint_id(), instead parsing from format file (Lionel)

v4: Fix existing configure.ac issue with overlay build (Petri)

v5: Silence unused function (Lionel)

Signed-off-by: Lionel Landwerlin 

For the build system changes:
Acked-by: Petri Latvala 

If it works, it works.
Acked-by: Chris Wilson 

One of the next steps would be failing if the tracepoint doesn't contain
the desired data.
-Chris

It will fail if the parsing fails. Now if someone removes a field from 
the kernel, yeah, we're a bit screwed...


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t v5] overlay: parse tracepoints from sysfs to figure out fields' location

[Chris Wilson]

Quoting Lionel Landwerlin (2017-12-19 14:26:11)
> With changes going to drm-tip, the tracepoints field locations are
> going to change. This change introduces a tracepoint parser (using a
> peg parser) which lets us figure out field positions on the fly.
> 
> v2: Fix automake build (Lionel)
> 
> v3: Make overlay build conditional on peg (Petri)
> Make wait_end callback more readable (Chris)
> Drop tracepoint_id(), instead parsing from format file (Lionel)
> 
> v4: Fix existing configure.ac issue with overlay build (Petri)
> 
> v5: Silence unused function (Lionel)
> 
> Signed-off-by: Lionel Landwerlin 
> 
> For the build system changes:
> Acked-by: Petri Latvala 

If it works, it works.
Acked-by: Chris Wilson 

One of the next steps would be failing if the tracepoint doesn't contain
the desired data.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH i-g-t v5] overlay: parse tracepoints from sysfs to figure out fields' location

[Lionel Landwerlin]

With changes going to drm-tip, the tracepoints field locations are
going to change. This change introduces a tracepoint parser (using a
peg parser) which lets us figure out field positions on the fly.

v2: Fix automake build (Lionel)

v3: Make overlay build conditional on peg (Petri)
Make wait_end callback more readable (Chris)
Drop tracepoint_id(), instead parsing from format file (Lionel)

v4: Fix existing configure.ac issue with overlay build (Petri)

v5: Silence unused function (Lionel)

Signed-off-by: Lionel Landwerlin 

For the build system changes:
Acked-by: Petri Latvala 
---
 configure.ac  |   9 ++-
 overlay/Makefile.am   |   5 ++
 overlay/gpu-perf.c| 164 --
 overlay/meson.build   |  13 +++-
 overlay/tracepoint_format.leg |  35 +
 5 files changed, 186 insertions(+), 40 deletions(-)
 create mode 100644 overlay/tracepoint_format.leg

diff --git a/configure.ac b/configure.ac
index 8740f7a4..50c1a024 100644
--- a/configure.ac
+++ b/configure.ac
@@ -146,6 +146,13 @@ if test x"$build_x86" = xyes; then
AS_IF([test x"$LEX" != "x:" -a x"$YACC" != xyacc],
[enable_assembler=yes],
[enable_assembler=no])
+
+   AC_CHECK_TOOL([LEG], [leg])
+   if test "x$LEG" != "xleg"; then
+   AC_MSG_NOTICE([leg command missing, disabling overlay; try : 
apt-get install peg])
+   enable_overlay_xvlib="no"
+   enable_overlay_xlib="no"
+   fi
 else
enable_overlay_xvlib="no"
enable_overlay_xlib="no"
@@ -158,7 +165,7 @@ AM_CONDITIONAL(BUILD_ASSEMBLER, [test "x$enable_assembler" 
= xyes])
 
 AM_CONDITIONAL(BUILD_OVERLAY_XVLIB, [test "x$enable_overlay_xvlib" = xyes])
 AM_CONDITIONAL(BUILD_OVERLAY_XLIB, [test "x$enable_overlay_xlib" = xyes])
-AM_CONDITIONAL(BUILD_OVERLAY, [test "x$enable_overlay_xlib" = xyes -o 
"x$enable_overlay_xvlib"])
+AM_CONDITIONAL(BUILD_OVERLAY, [test "x$enable_overlay_xlib" = xyes -o 
"x$enable_overlay_xvlib" = "xyes])
 if test x$enable_overlay_xvlib = xyes; then
AC_DEFINE(HAVE_OVERLAY_XVLIB, 1, [Enable XV backend])
 fi
diff --git a/overlay/Makefile.am b/overlay/Makefile.am
index fca04cae..0f553b7c 100644
--- a/overlay/Makefile.am
+++ b/overlay/Makefile.am
@@ -1,7 +1,12 @@
 if BUILD_OVERLAY
 bin_PROGRAMS = intel-gpu-overlay
+
+BUILT_SOURCES = tracepoint_format.h
 endif
 
+tracepoint_format.h: tracepoint_format.leg
+   $(LEG) -o $@ $<
+
 AM_CPPFLAGS = -I. -I$(top_srcdir)/include/drm-uapi
 AM_CFLAGS = $(DRM_CFLAGS) $(PCIACCESS_CFLAGS) $(CWARNFLAGS) \
$(CAIRO_CFLAGS) $(OVERLAY_CFLAGS) $(WERROR_CFLAGS) -I$(srcdir)/../lib
diff --git a/overlay/gpu-perf.c b/overlay/gpu-perf.c
index 3d4a9be9..0abd937e 100644
--- a/overlay/gpu-perf.c
+++ b/overlay/gpu-perf.c
@@ -57,39 +57,125 @@ struct sample_event {
uint64_t time;
uint64_t id;
uint32_t raw_size;
-   uint32_t raw_hdr0;
-   uint32_t raw_hdr1;
-   uint32_t raw[0];
+   uint8_t tracepoint_data[0];
 };
 
 enum {
-   DEVICE = 0,
-   CTX,
-   ENGINE,
-   CTX_SEQNO,
-   GLOBAL_SEQNO
+   TP_GEM_REQUEST_ADD,
+   TP_GEM_REQUEST_WAIT_BEGIN,
+   TP_GEM_REQUEST_WAIT_END,
+   TP_FLIP_COMPLETE,
+   TP_GEM_RING_SYNC_TO,
+   TP_GEM_RING_SWITCH_CONTEXT,
+
+   TP_NB
 };
 
-static uint64_t tracepoint_id(const char *sys, const char *name)
+struct tracepoint {
+   const char *name;
+   int event_id;
+
+   struct {
+   char name[128];
+   int offset;
+   int size;
+   int is_signed;
+   } fields[20];
+   int n_fields;
+
+   int device_field;
+   int ctx_field;
+   int ring_field;
+   int seqno_field;
+   int global_seqno_field;
+   int plane_field;
+} tracepoints[TP_NB] = {
+   [TP_GEM_REQUEST_ADD] = { .name = "i915/i915_gem_request_add", },
+   [TP_GEM_REQUEST_WAIT_BEGIN]  = { .name = 
"i915/i915_gem_request_wait_begin", },
+   [TP_GEM_REQUEST_WAIT_END]= { .name = 
"i915/i915_gem_request_wait_end", },
+   [TP_FLIP_COMPLETE]   = { .name = "i915/flip_complete", },
+   [TP_GEM_RING_SYNC_TO]= { .name = "i915/gem_ring_sync_to", },
+   [TP_GEM_RING_SWITCH_CONTEXT] = { .name = 
"i915/gem_ring_switch_context", },
+};
+
+union parser_value {
+char *string;
+int integer;
+};
+
+struct parser_ctx {
+   struct tracepoint *tp;
+   FILE *fp;
+};
+
+#define YY_CTX_LOCAL
+#define YY_CTX_MEMBERS struct parser_ctx ctx;
+#define YYSTYPE union parser_value
+#define YY_LOCAL(T) static __attribute__((unused)) T
+#define YY_PARSE(T) static T
+#define YY_INPUT(yy, buf, result, max) \
+   {   \
+   int c = getc(yy->ctx.fp);   \
+   result = (EOF == c) ? 0 : (*(buf)= c, 1);