[Intel-gfx] [PATCH i-g-t 1/7] intel-gpu-overlay: Move local perf implementation to a library

[Chris Wilson]

Quoting Tvrtko Ursulin (2017-09-26 11:52:28)
> 
> On 25/09/2017 16:22, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2017-09-25 16:14:56)
> >> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> >>
> >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> >> ---
> >>   lib/Makefile.sources             | 2 ++
> >>   overlay/perf.c => lib/igt_perf.c | 2 +-
> >>   overlay/perf.h => lib/igt_perf.h | 2 ++
> >>   overlay/Makefile.am              | 6 ++----
> >>   overlay/gem-interrupts.c         | 3 ++-
> >>   overlay/gpu-freq.c               | 3 ++-
> >>   overlay/gpu-perf.c               | 3 ++-
> >>   overlay/gpu-top.c                | 3 ++-
> >>   overlay/power.c                  | 3 ++-
> >>   overlay/rc6.c                    | 3 ++-
> >>   10 files changed, 19 insertions(+), 11 deletions(-)
> >>   rename overlay/perf.c => lib/igt_perf.c (94%)
> >>   rename overlay/perf.h => lib/igt_perf.h (99%)
> >>
> >> diff --git a/lib/Makefile.sources b/lib/Makefile.sources
> >> index 53fdb54cbfa5..c031cb502469 100644
> >> --- a/lib/Makefile.sources
> >> +++ b/lib/Makefile.sources
> >> @@ -16,6 +16,8 @@ lib_source_list =             \
> >>          igt_gt.h                \
> >>          igt_gvt.c               \
> >>          igt_gvt.h               \
> >> +       igt_perf.c              \
> >> +       igt_perf.h              \
> >>          igt_primes.c            \
> >>          igt_primes.h            \
> >>          igt_rand.c              \
> >> diff --git a/overlay/perf.c b/lib/igt_perf.c
> >> similarity index 94%
> >> rename from overlay/perf.c
> >> rename to lib/igt_perf.c
> >> index b8fdc675c587..45cccff0ae53 100644
> >> --- a/overlay/perf.c
> >> +++ b/lib/igt_perf.c
> >> @@ -3,7 +3,7 @@
> >>   #include <unistd.h>
> >>   #include <stdlib.h>
> >>   
> >> -#include "perf.h"
> >> +#include "igt_perf.h"
> >>   
> >>   uint64_t i915_type_id(void)
> >>   {
> >> diff --git a/overlay/perf.h b/lib/igt_perf.h
> >> similarity index 99%
> >> rename from overlay/perf.h
> >> rename to lib/igt_perf.h
> >> index c44e65f9734c..a80b311cd1d1 100644
> >> --- a/overlay/perf.h
> >> +++ b/lib/igt_perf.h
> >> @@ -1,6 +1,8 @@
> >>   #ifndef I915_PERF_H
> >>   #define I915_PERF_H
> >>   
> >> +#include <stdint.h>
> >> +
> >>   #include <linux/perf_event.h>
> >>   
> >>   #define I915_SAMPLE_BUSY       0
> >> diff --git a/overlay/Makefile.am b/overlay/Makefile.am
> >> index 5472514efc16..c66a80f4e571 100644
> >> --- a/overlay/Makefile.am
> >> +++ b/overlay/Makefile.am
> >> @@ -4,8 +4,8 @@ endif
> >>   
> >>   AM_CPPFLAGS = -I.
> >>   AM_CFLAGS = $(DRM_CFLAGS) $(PCIACCESS_CFLAGS) $(CWARNFLAGS) \
> >> -       $(CAIRO_CFLAGS) $(OVERLAY_CFLAGS) $(WERROR_CFLAGS)
> >> -LDADD = $(DRM_LIBS) $(PCIACCESS_LIBS) $(CAIRO_LIBS) $(OVERLAY_LIBS)
> >> +       $(CAIRO_CFLAGS) $(OVERLAY_CFLAGS) $(WERROR_CFLAGS) -I$(srcdir)/../lib
> >> +LDADD = $(DRM_LIBS) $(PCIACCESS_LIBS) $(CAIRO_LIBS) $(OVERLAY_LIBS) $(top_builddir)/lib/libintel_tools.la
> > 
> > It was a conscious decision that tools wouldn't pull in the test lib. We
> > are nowhere close to having a stable library api, i.e. a long way from
> > being installable. An issue if this is turned into a .so, which has
> > happened in a nearby universe.
> 
> I don't exactly follow why we would care about stable library API since 
> no outside projects will be linking to our libraries.

Hah. Just wait, if we install a .so, expect the complaints to start
rolling in. :-p

> But anyway, I'll 
> try to make overlay not link wit libintel_tools but just statically 
> embed the perf file. Ok with that?

I don't mind if we do start using a .so, I just advise that if we do, we
don't just package up the current libigt.so as it is a mess of an API
(not a library, more a collection of hacks).

For this case, I would create a noinst_LTLIBRARY = libigt_perf.la
Might as well use automake/libtool for one good thing :)
-Chris