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

[Petri Latvala <petri.latvala@intel.com>]

On Mon, Oct 09, 2017 at 10:54:25AM +0100, Tvrtko Ursulin wrote:
> 
> 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.


Up to you how you want to merge, we can make the meson changes
separately. Especially with this one, it wasn't a simple "file added /
removed" change.


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