On Tue, Dec 19, 2017 at 12:00:58PM +0000, Lionel Landwerlin wrote:
> 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)
>
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@xxxxxxxxx>
> ---
> configure.ac | 7 ++
> overlay/Makefile.am | 5 ++
> overlay/gpu-perf.c | 163 ++++++++++++++++++++++++++++++++----------
> overlay/meson.build | 15 +++-
> overlay/tracepoint_format.leg | 35 +++++++++
> 5 files changed, 185 insertions(+), 40 deletions(-)
> create mode 100644 overlay/tracepoint_format.leg
>
> diff --git a/configure.ac b/configure.ac
> index 8740f7a4..2d2ed8ce 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"
My earlier testing was insufficient. There's a bug lurking that we need to fix at the same time:
@@ -165,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..057e83eb 100644
> --- a/overlay/gpu-perf.c
> +++ b/overlay/gpu-perf.c
> @@ -57,39 +57,124 @@ 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_PARSE(T) static T
> +#define YY_INPUT(yy, buf, result, max) \
> + { \
> + int c = getc(yy->ctx.fp); \
> + result = (EOF == c) ? 0 : (*(buf)= c, 1); \
> + if (EOF != c) { \
> + yyprintf((stderr, "<%c>", c)); \
> + } \
> + }
> +
> +#include "tracepoint_format.h"
> +
> +static int
> +tracepoint_id(int tp_id)
> {
> + struct tracepoint *tp = &tracepoints[tp_id];
> + yycontext ctx;
> char buf[1024];
> - int fd, n;
>
> - snprintf(buf, sizeof(buf), "%s/tracing/events/%s/%s/id", debugfs_path, sys, name);
> - fd = open(buf, 0);
> - if (fd < 0)
> - return 0;
> - n = read(fd, buf, sizeof(buf)-1);
> - close(fd);
> - if (n < 0)
> + /* Already parsed? */
> + if (tp->event_id != 0)
> + return tp->event_id;
> +
> + snprintf(buf, sizeof(buf), "%s/tracing/events/%s/format",
> + debugfs_path, tp->name);
> +
> + memset(&ctx, 0, sizeof(ctx));
> + ctx.ctx.tp = tp;
> + ctx.ctx.fp = fopen(buf, "r");
> +
> + if (ctx.ctx.fp == NULL)
> return 0;
>
> - buf[n] = '\0';
> - return strtoull(buf, 0, 0);
> + if (yyparse(&ctx)) {
> + for (int f = 0; f < tp->n_fields; f++) {
> + if (!strcmp(tp->fields[f].name, "device")) {
> + tp->device_field = f;
> + } else if (!strcmp(tp->fields[f].name, "ctx")) {
> + tp->ctx_field = f;
> + } else if (!strcmp(tp->fields[f].name, "ring")) {
> + tp->ring_field = f;
> + } else if (!strcmp(tp->fields[f].name, "seqno")) {
> + tp->seqno_field = f;
> + } else if (!strcmp(tp->fields[f].name, "global_seqno")) {
> + tp->global_seqno_field = f;
> + } else if (!strcmp(tp->fields[f].name, "plane")) {
> + tp->plane_field = f;
> + }
> + }
> + } else
> + tp->event_id = tp->n_fields = 0;
> +
> + yyrelease(&ctx);
> + fclose(ctx.ctx.fp);
> +
> + return tp->event_id;
> }
>
> -static int perf_tracepoint_open(struct gpu_perf *gp,
> - const char *sys, const char *name,
> +#define READ_TP_FIELD_U32(sample, tp_id, field_name) \
> + (*(const uint32_t *)((sample)->tracepoint_data + \
> + tracepoints[tp_id].fields[ \
> + tracepoints[tp_id].field_name##_field].offset))
> +
> +static int perf_tracepoint_open(struct gpu_perf *gp, int tp_id,
> int (*func)(struct gpu_perf *, const void *))
> {
> struct perf_event_attr attr;
> @@ -99,7 +184,7 @@ static int perf_tracepoint_open(struct gpu_perf *gp,
> memset(&attr, 0, sizeof (attr));
>
> attr.type = PERF_TYPE_TRACEPOINT;
> - attr.config = tracepoint_id(sys, name);
> + attr.config = tracepoint_id(tp_id);
> if (attr.config == 0)
> return ENOENT;
>
> @@ -227,7 +312,7 @@ static int request_add(struct gpu_perf *gp, const void *event)
> if (comm == NULL)
> return 0;
>
> - comm->nr_requests[sample->raw[ENGINE]]++;
> + comm->nr_requests[READ_TP_FIELD_U32(sample, TP_GEM_REQUEST_ADD, ring)]++;
> return 1;
> }
>
> @@ -235,7 +320,7 @@ static int flip_complete(struct gpu_perf *gp, const void *event)
> {
> const struct sample_event *sample = event;
>
> - gp->flip_complete[sample->raw[0]]++;
> + gp->flip_complete[READ_TP_FIELD_U32(sample, TP_FLIP_COMPLETE, plane)]++;
> return 1;
> }
>
> @@ -243,7 +328,7 @@ static int ctx_switch(struct gpu_perf *gp, const void *event)
> {
> const struct sample_event *sample = event;
>
> - gp->ctx_switch[sample->raw[ENGINE]]++;
> + gp->ctx_switch[READ_TP_FIELD_U32(sample, TP_GEM_RING_SWITCH_CONTEXT, ring)]++;
> return 1;
> }
>
> @@ -278,11 +363,11 @@ static int wait_begin(struct gpu_perf *gp, const void *event)
>
> wait->comm = comm;
> wait->comm->active = true;
> - wait->context = sample->raw[ENGINE];
> - wait->seqno = sample->raw[CTX_SEQNO];
> + wait->context = READ_TP_FIELD_U32(sample, TP_GEM_REQUEST_WAIT_BEGIN, ctx);
> + wait->seqno = READ_TP_FIELD_U32(sample, TP_GEM_REQUEST_WAIT_BEGIN, seqno);
> wait->time = sample->time;
> - wait->next = gp->wait[sample->raw[CTX]];
> - gp->wait[sample->raw[CTX]] = wait;
> + wait->next = gp->wait[READ_TP_FIELD_U32(sample, TP_GEM_REQUEST_WAIT_BEGIN, ring)];
> + gp->wait[READ_TP_FIELD_U32(sample, TP_GEM_REQUEST_WAIT_BEGIN, ring)] = wait;
>
> return 0;
> }
> @@ -291,10 +376,12 @@ static int wait_end(struct gpu_perf *gp, const void *event)
> {
> const struct sample_event *sample = event;
> struct gpu_perf_time *wait, **prev;
> + uint32_t engine = READ_TP_FIELD_U32(sample, TP_GEM_REQUEST_WAIT_END, ring);
> + uint32_t context = READ_TP_FIELD_U32(sample, TP_GEM_REQUEST_WAIT_END, ctx);
> + uint32_t seqno = READ_TP_FIELD_U32(sample, TP_GEM_REQUEST_WAIT_END, seqno);
>
> - for (prev = &gp->wait[sample->raw[ENGINE]]; (wait = *prev) != NULL; prev = &wait->next) {
> - if (wait->context != sample->raw[CTX] ||
> - wait->seqno != sample->raw[CTX_SEQNO])
> + for (prev = &gp->wait[engine]; (wait = *prev) != NULL; prev = &wait->next) {
> + if (wait->context != context || wait->seqno != seqno)
> continue;
>
> wait->comm->wait_time += sample->time - wait->time;
> @@ -314,12 +401,12 @@ void gpu_perf_init(struct gpu_perf *gp, unsigned flags)
> gp->nr_cpus = sysconf(_SC_NPROCESSORS_ONLN);
> gp->page_size = getpagesize();
>
> - perf_tracepoint_open(gp, "i915", "i915_gem_request_add", request_add);
> - if (perf_tracepoint_open(gp, "i915", "i915_gem_request_wait_begin", wait_begin) == 0)
> - perf_tracepoint_open(gp, "i915", "i915_gem_request_wait_end", wait_end);
> - perf_tracepoint_open(gp, "i915", "i915_flip_complete", flip_complete);
> - perf_tracepoint_open(gp, "i915", "i915_gem_ring_sync_to", ring_sync);
> - perf_tracepoint_open(gp, "i915", "i915_gem_ring_switch_context", ctx_switch);
> + perf_tracepoint_open(gp, TP_GEM_REQUEST_ADD, request_add);
> + if (perf_tracepoint_open(gp, TP_GEM_REQUEST_WAIT_BEGIN, wait_begin) == 0)
> + perf_tracepoint_open(gp, TP_GEM_REQUEST_WAIT_END, wait_end);
> + perf_tracepoint_open(gp, TP_FLIP_COMPLETE, flip_complete);
> + perf_tracepoint_open(gp, TP_GEM_RING_SYNC_TO, ring_sync);
> + perf_tracepoint_open(gp, TP_GEM_RING_SWITCH_CONTEXT, ctx_switch);
>
> if (gp->nr_events == 0) {
> gp->error = "i915.ko tracepoints not available";
> diff --git a/overlay/meson.build b/overlay/meson.build
> index afacff5e..57d8fa61 100644
> --- a/overlay/meson.build
> +++ b/overlay/meson.build
> @@ -26,7 +26,7 @@ gpu_overlay_deps = [ realtime, math, cairo, pciaccess, libdrm,
>
> both_x11_src = ''
>
> -gpu_overlay_cflags = []
> +gpu_overlay_cflags = [ '-g3', '-O0' ]
Why is this needed?
Can you silence this warning?
In file included from ../overlay/gpu-perf.c:124:0:
overlay/tracepoint_format.h:242:15: warning: ‘yyAccept’ defined but not used [-Wunused-function]
YY_LOCAL(int) yyAccept(yycontext *yy, int tp0)
^~~~~~~~
For the build system changes,
Acked-by: Petri Latvala <petri.latvala@xxxxxxxxx>
--
Petri Latvala
> if xv.found() and x11.found() and xext.found() and dri2proto.found()
> both_x11_src = 'x11/position.c'
> gpu_overlay_src += [
> @@ -51,7 +51,18 @@ gpu_overlay_src += both_x11_src
>
> gpu_overlay_src += 'kms/kms-overlay.c'
>
> -if xrandr.found() and cairo.found()
> +leg = find_program('leg', required : false)
> +if leg.found()
> + leg_file = custom_target('tracepoint_format',
> + output: 'tracepoint_format.h',
> + input: 'tracepoint_format.leg',
> + command: [leg, '-P', '-o', '@OUTPUT@', '@INPUT@'])
> + gpu_overlay_src += leg_file
> +else
> + warning('leg command not found, disabling overlay; try : apt-get install peg')
> +endif
> +
> +if leg.found() and xrandr.found() and cairo.found()
> executable('intel-gpu-overlay', gpu_overlay_src,
> include_directories : inc,
> c_args : gpu_overlay_cflags,
> diff --git a/overlay/tracepoint_format.leg b/overlay/tracepoint_format.leg
> new file mode 100644
> index 00000000..7a09149d
> --- /dev/null
> +++ b/overlay/tracepoint_format.leg
> @@ -0,0 +1,35 @@
> +TracepointFmt =
> + 'name' ':' Space n:PropertyName EndLine
> + { free(v.string); }
> + 'ID:' Space v:Number EndLine
> + { yy->ctx.tp->event_id = v.integer; }
> + 'format:' EndLine
> + Field+
> + 'print fmt:' [^.]* !.
> +
> +Field = Space (Property ';' Space)+ EndLine
> + { yy->ctx.tp->n_fields++; }
> + | EndLine
> +
> +Property = 'offset' ':' v:Number
> + { yy->ctx.tp->fields[yy->ctx.tp->n_fields].offset = v.integer; }
> + | 'size' ':' v:Number
> + { yy->ctx.tp->fields[yy->ctx.tp->n_fields].size = v.integer; }
> + | 'signed' ':' v:Number
> + { yy->ctx.tp->fields[yy->ctx.tp->n_fields].is_signed = v.integer != 0; }
> + | 'field' ':' v:PropertyValue
> + { snprintf(yy->ctx.tp->fields[yy->ctx.tp->n_fields].name,
> + sizeof(yy->ctx.tp->fields[yy->ctx.tp->n_fields].name),
> + "%s", strrchr(v.string, ' ') + 1); free(v.string); }
> + | n:PropertyName ':' v:PropertyValue
> + { free(n.string); free(v.string); }
> +
> +PropertyName = < [A-Za-z0-9_]+ >
> + { $$.string = strdup(yytext); }
> +PropertyValue = < [^;]+ >
> + { $$.string = strdup(yytext); }
> +Number = < [0-9]+ >
> + { $$.integer = atoi(yytext); }
> +
> +EndLine = [\n]
> +Space = [ \t]*
> --
> 2.15.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
