| Message ID | 20170929123939.3312-2-tvrtko.ursulin@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 29/09/2017 14:43, Petri Latvala wrote: > On Fri, Sep 29, 2017 at 01:39:33PM +0100, Tvrtko Ursulin wrote: >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> >> Idea is to avoid duplication across multiple users in >> upcoming patches. >> >> v2: Commit message and use a separate library instead of piggy- >> backing to libintel_tools. (Chris Wilson) >> >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> --- >> lib/Makefile.am | 6 +++++- >> 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, 22 insertions(+), 12 deletions(-) >> rename overlay/perf.c => lib/igt_perf.c (94%) >> rename overlay/perf.h => lib/igt_perf.h (99%) > > > This one was more of a doozey to mesonize for a newbie. > > This is ugly but hopefully will make someone more knowledgeable point > out better ways and practices for using build targets vs. just lib > names around... > > (Now sent with X-Patchwork-Hint, hopefully patchwork doesn't get > confused) > > diff --git a/benchmarks/meson.build b/benchmarks/meson.build > index 9ab738f7..9f2672eb 100644 > --- a/benchmarks/meson.build > +++ b/benchmarks/meson.build > @@ -31,6 +31,11 @@ endif > foreach prog : benchmark_progs > # FIXME meson doesn't like binaries with the same name > # meanwhile just suffix with _bench > + link = [] > + if prog == 'gem_wsim' > + link += lib_igt_perf > + endif > executable(prog + '_bench', prog + '.c', > - dependencies : test_deps) > + dependencies : test_deps, > + link_with : link) > endforeach > diff --git a/lib/meson.build b/lib/meson.build > index 203be520..2c33493d 100644 > --- a/lib/meson.build > +++ b/lib/meson.build > @@ -178,4 +178,8 @@ lib_igt = declare_dependency(link_with : lib_igt_build, > > igt_deps = [ lib_igt ] + lib_deps > > +lib_igt_perf = static_library('igt_perf', > + ['igt_perf.c'] > +) > + > subdir('tests') > diff --git a/overlay/meson.build b/overlay/meson.build > index a92ef895..ffc011cc 100644 > --- a/overlay/meson.build > +++ b/overlay/meson.build > @@ -10,7 +10,6 @@ gpu_overlay_src = [ > 'gpu-freq.c', > 'igfx.c', > 'overlay.c', > - 'perf.c', > 'power.c', > 'rc6.c', > ] > @@ -56,5 +55,6 @@ if xrandr.found() and cairo.found() > include_directories : inc, > c_args : gpu_overlay_cflags, > dependencies : gpu_overlay_deps, > + link_with : lib_igt_perf, > install : true) > endif Grumble, can we have a switch over day where it all gets converted to meson by the people in the know, and until then not concern ourselves with a two-headed build system? At the moment it is just a distraction and time waste if everybody working on IGT has to test both build systems. I know meson is great and all that by I'd rather focus on the actual work than having to maintain parallel build systems. Especially since I am clueless on it, so it would be one more thing competing for limited brain resources. Regards, Tvrtko
On 10/06/2017 06:25 PM, Tvrtko Ursulin wrote: > > On 29/09/2017 14:43, Petri Latvala wrote: >> On Fri, Sep 29, 2017 at 01:39:33PM +0100, Tvrtko Ursulin wrote: >>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >>> >>> Idea is to avoid duplication across multiple users in >>> upcoming patches. >>> >>> v2: Commit message and use a separate library instead of piggy- >>> backing to libintel_tools. (Chris Wilson) >>> >>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >>> --- >>> lib/Makefile.am | 6 +++++- >>> 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, 22 insertions(+), 12 deletions(-) >>> rename overlay/perf.c => lib/igt_perf.c (94%) >>> rename overlay/perf.h => lib/igt_perf.h (99%) >> >> >> This one was more of a doozey to mesonize for a newbie. >> >> This is ugly but hopefully will make someone more knowledgeable point >> out better ways and practices for using build targets vs. just lib >> names around... >> >> (Now sent with X-Patchwork-Hint, hopefully patchwork doesn't get >> confused) >> >> diff --git a/benchmarks/meson.build b/benchmarks/meson.build >> index 9ab738f7..9f2672eb 100644 >> --- a/benchmarks/meson.build >> +++ b/benchmarks/meson.build >> @@ -31,6 +31,11 @@ endif >> foreach prog : benchmark_progs >> # FIXME meson doesn't like binaries with the same name >> # meanwhile just suffix with _bench >> + link = [] >> + if prog == 'gem_wsim' >> + link += lib_igt_perf >> + endif >> executable(prog + '_bench', prog + '.c', >> - dependencies : test_deps) >> + dependencies : test_deps, >> + link_with : link) >> endforeach >> diff --git a/lib/meson.build b/lib/meson.build >> index 203be520..2c33493d 100644 >> --- a/lib/meson.build >> +++ b/lib/meson.build >> @@ -178,4 +178,8 @@ lib_igt = declare_dependency(link_with : >> lib_igt_build, >> igt_deps = [ lib_igt ] + lib_deps >> +lib_igt_perf = static_library('igt_perf', >> + ['igt_perf.c'] >> +) >> + >> subdir('tests') >> diff --git a/overlay/meson.build b/overlay/meson.build >> index a92ef895..ffc011cc 100644 >> --- a/overlay/meson.build >> +++ b/overlay/meson.build >> @@ -10,7 +10,6 @@ gpu_overlay_src = [ >> 'gpu-freq.c', >> 'igfx.c', >> 'overlay.c', >> - 'perf.c', >> 'power.c', >> 'rc6.c', >> ] >> @@ -56,5 +55,6 @@ if xrandr.found() and cairo.found() >> include_directories : inc, >> c_args : gpu_overlay_cflags, >> dependencies : gpu_overlay_deps, >> + link_with : lib_igt_perf, >> install : true) >> endif > > Grumble, can we have a switch over day where it all gets converted to > meson by the people in the know, and until then not concern ourselves > with a two-headed build system? > > At the moment it is just a distraction and time waste if everybody > working on IGT has to test both build systems. > > I know meson is great and all that by I'd rather focus on the actual > work than having to maintain parallel build systems. Especially since > I am clueless on it, so it would be one more thing competing for > limited brain resources. The whole reason I've been sending meson equivalents for autotools-system changes is so that the original author doesn't have to do it. I don't expect everyone to test both, or to even make it mandatory to have the meson changes included. As for "all gets converted", the current state is that meson is able to build everything in git.
On 09/10/2017 10:22, Petri Latvala wrote: > On 10/06/2017 06:25 PM, Tvrtko Ursulin wrote: >> On 29/09/2017 14:43, Petri Latvala wrote: >>> On Fri, Sep 29, 2017 at 01:39:33PM +0100, Tvrtko Ursulin wrote: >>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >>>> >>>> Idea is to avoid duplication across multiple users in >>>> upcoming patches. >>>> >>>> v2: Commit message and use a separate library instead of piggy- >>>> backing to libintel_tools. (Chris Wilson) >>>> >>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >>>> --- >>>> lib/Makefile.am | 6 +++++- >>>> 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, 22 insertions(+), 12 deletions(-) >>>> rename overlay/perf.c => lib/igt_perf.c (94%) >>>> rename overlay/perf.h => lib/igt_perf.h (99%) >>> >>> >>> This one was more of a doozey to mesonize for a newbie. >>> >>> This is ugly but hopefully will make someone more knowledgeable point >>> out better ways and practices for using build targets vs. just lib >>> names around... >>> >>> (Now sent with X-Patchwork-Hint, hopefully patchwork doesn't get >>> confused) >>> >>> diff --git a/benchmarks/meson.build b/benchmarks/meson.build >>> index 9ab738f7..9f2672eb 100644 >>> --- a/benchmarks/meson.build >>> +++ b/benchmarks/meson.build >>> @@ -31,6 +31,11 @@ endif >>> foreach prog : benchmark_progs >>> # FIXME meson doesn't like binaries with the same name >>> # meanwhile just suffix with _bench >>> + link = [] >>> + if prog == 'gem_wsim' >>> + link += lib_igt_perf >>> + endif >>> executable(prog + '_bench', prog + '.c', >>> - dependencies : test_deps) >>> + dependencies : test_deps, >>> + link_with : link) >>> endforeach >>> diff --git a/lib/meson.build b/lib/meson.build >>> index 203be520..2c33493d 100644 >>> --- a/lib/meson.build >>> +++ b/lib/meson.build >>> @@ -178,4 +178,8 @@ lib_igt = declare_dependency(link_with : >>> lib_igt_build, >>> igt_deps = [ lib_igt ] + lib_deps >>> +lib_igt_perf = static_library('igt_perf', >>> + ['igt_perf.c'] >>> +) >>> + >>> subdir('tests') >>> diff --git a/overlay/meson.build b/overlay/meson.build >>> index a92ef895..ffc011cc 100644 >>> --- a/overlay/meson.build >>> +++ b/overlay/meson.build >>> @@ -10,7 +10,6 @@ gpu_overlay_src = [ >>> 'gpu-freq.c', >>> 'igfx.c', >>> 'overlay.c', >>> - 'perf.c', >>> 'power.c', >>> 'rc6.c', >>> ] >>> @@ -56,5 +55,6 @@ if xrandr.found() and cairo.found() >>> include_directories : inc, >>> c_args : gpu_overlay_cflags, >>> dependencies : gpu_overlay_deps, >>> + link_with : lib_igt_perf, >>> install : true) >>> endif >> >> Grumble, can we have a switch over day where it all gets converted to >> meson by the people in the know, and until then not concern ourselves >> with a two-headed build system? >> >> At the moment it is just a distraction and time waste if everybody >> working on IGT has to test both build systems. >> >> I know meson is great and all that by I'd rather focus on the actual >> work than having to maintain parallel build systems. Especially since >> I am clueless on it, so it would be one more thing competing for >> limited brain resources. > > > The whole reason I've been sending meson equivalents for > autotools-system changes is so that the original author doesn't have to > do it. I don't expect everyone to test both, or to even make it > mandatory to have the meson changes included. As for "all gets > converted", the current state is that meson is able to build everything > in git. If it is fine just to copy&paste your snippets in patches blindly I can do that. Even though I feel uncomfortable doing so (blindly), and at the same time, as I wrote above, I would prefer we only have to deal with one build system at a time. Hopefully, when I integrate these snippets in my patches, someone won't spot a problem in them and ask me to re-spin. Since that would bring us back to my point of every developer wasting time on two build system, instead of having a switch over day and be done with it. Regards, Tvrtko
diff --git a/lib/Makefile.am b/lib/Makefile.am index 6509593db1c7..e6edd9448e8d 100644 --- a/lib/Makefile.am +++ b/lib/Makefile.am @@ -7,7 +7,11 @@ include Makefile.sources libintel_tools_la_SOURCES = $(lib_source_list) -noinst_LTLIBRARIES = libintel_tools.la +libigt_perf_la_SOURCES = \ + igt_perf.c \ + igt_perf.h + +noinst_LTLIBRARIES = libintel_tools.la libigt_perf.la noinst_HEADERS = check-ndebug.h if HAVE_LIBDRM_VC4 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..e4e78bb33cd6 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/libigt_perf.la intel_gpu_overlay_SOURCES = \ chart.h \ @@ -29,8 +29,6 @@ intel_gpu_overlay_SOURCES = \ igfx.c \ overlay.h \ overlay.c \ - perf.h \ - perf.c \ power.h \ power.c \ rc6.h \ diff --git a/overlay/gem-interrupts.c b/overlay/gem-interrupts.c index 0150a1d03825..7ba54fcd487d 100644 --- a/overlay/gem-interrupts.c +++ b/overlay/gem-interrupts.c @@ -31,9 +31,10 @@ #include <string.h> #include <ctype.h> +#include "igt_perf.h" + #include "gem-interrupts.h" #include "debugfs.h" -#include "perf.h" static int perf_open(void) { diff --git a/overlay/gpu-freq.c b/overlay/gpu-freq.c index 321c93882238..7f29b1aa986e 100644 --- a/overlay/gpu-freq.c +++ b/overlay/gpu-freq.c @@ -28,9 +28,10 @@ #include <string.h> #include <stdio.h> +#include "igt_perf.h" + #include "gpu-freq.h" #include "debugfs.h" -#include "perf.h" static int perf_i915_open(int config, int group) { diff --git a/overlay/gpu-perf.c b/overlay/gpu-perf.c index f557b9f06a17..3d4a9be91a94 100644 --- a/overlay/gpu-perf.c +++ b/overlay/gpu-perf.c @@ -34,7 +34,8 @@ #include <fcntl.h> #include <errno.h> -#include "perf.h" +#include "igt_perf.h" + #include "gpu-perf.h" #include "debugfs.h" diff --git a/overlay/gpu-top.c b/overlay/gpu-top.c index 891a7ea7c0b1..06f489dfdc83 100644 --- a/overlay/gpu-top.c +++ b/overlay/gpu-top.c @@ -31,7 +31,8 @@ #include <errno.h> #include <assert.h> -#include "perf.h" +#include "igt_perf.h" + #include "igfx.h" #include "gpu-top.h" diff --git a/overlay/power.c b/overlay/power.c index 2f1521b82cd6..84d860cae40c 100644 --- a/overlay/power.c +++ b/overlay/power.c @@ -31,7 +31,8 @@ #include <time.h> #include <errno.h> -#include "perf.h" +#include "igt_perf.h" + #include "power.h" #include "debugfs.h" diff --git a/overlay/rc6.c b/overlay/rc6.c index d7047c2f4880..3175bb22308f 100644 --- a/overlay/rc6.c +++ b/overlay/rc6.c @@ -31,8 +31,9 @@ #include <time.h> #include <errno.h> +#include "igt_perf.h" + #include "rc6.h" -#include "perf.h" static int perf_i915_open(int config, int group) {