public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
From: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
To: Petri Latvala <petri.latvala@intel.com>,
	Tvrtko Ursulin <tursulin@ursulin.net>
Cc: Intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH i-g-t 1/7] intel-gpu-overlay: Move local perf implementation to a library
Date: Mon, 9 Oct 2017 10:54:25 +0100	[thread overview]
Message-ID: <59e14d17-c5c2-69e3-015d-a3278fb0818d@linux.intel.com> (raw)
In-Reply-To: <5407be7f-b44a-ea1d-59f8-04a1e7057938@intel.com>


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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2017-10-09  9:54 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-29 12:39 [PATCH v3 i-g-t 0/7] IGT PMU support Tvrtko Ursulin
2017-09-29 12:39 ` [PATCH i-g-t 1/7] intel-gpu-overlay: Move local perf implementation to a library Tvrtko Ursulin
2017-09-29 13:43   ` Petri Latvala
2017-10-06 15:25     ` Tvrtko Ursulin
2017-10-09  9:22       ` Petri Latvala
2017-10-09  9:54         ` Tvrtko Ursulin [this message]
2017-10-09 10:25           ` Petri Latvala
2017-09-29 12:39 ` [PATCH i-g-t 2/7] intel-gpu-overlay: Consolidate perf PMU access to library Tvrtko Ursulin
2017-09-29 12:39 ` [PATCH i-g-t 3/7] intel-gpu-overlay: Fix interrupts PMU readout Tvrtko Ursulin
2017-09-29 12:39 ` [PATCH i-g-t 4/7] intel-gpu-overlay: Catch-up to new i915 PMU Tvrtko Ursulin
2017-09-29 13:04   ` Chris Wilson
2017-09-29 12:39 ` [PATCH i-g-t 5/7] tests/perf_pmu: Tests for i915 PMU API Tvrtko Ursulin
2017-09-29 12:58   ` Petri Latvala
2017-09-29 12:39 ` [PATCH i-g-t 6/7] gem_wsim: Busy stats balancers Tvrtko Ursulin
2017-09-29 12:39 ` [PATCH i-g-t 7/7] media-bench.pl: Add busy balancers to the list Tvrtko Ursulin
2017-09-29 14:09 ` ✓ Fi.CI.BAT: success for IGT PMU support (rev5) Patchwork
2017-09-29 17:02 ` ✗ Fi.CI.IGT: warning " Patchwork
  -- strict thread matches above, loose matches on Subject: below --
2017-09-25 15:14 [PATCH i-g-t v2 0/7] IGT PMU support Tvrtko Ursulin
2017-09-25 15:14 ` [PATCH i-g-t 1/7] intel-gpu-overlay: Move local perf implementation to a library Tvrtko Ursulin
2017-09-25 15:22   ` Chris Wilson
2017-09-26 10:52     ` Tvrtko Ursulin
2017-09-26 11:07       ` Chris Wilson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=59e14d17-c5c2-69e3-015d-a3278fb0818d@linux.intel.com \
    --to=tvrtko.ursulin@linux.intel.com \
    --cc=Intel-gfx@lists.freedesktop.org \
    --cc=petri.latvala@intel.com \
    --cc=tursulin@ursulin.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox