[igt-dev] [PATCH i-g-t 4/6] tools: add i915 perf recorder tool

[Chris Wilson]

Quoting Lionel Landwerlin (2020-02-18 20:14:12)
> On 18/02/2020 21:49, Lionel Landwerlin wrote:
> > On 18/02/2020 21:36, Chris Wilson wrote:
> >> Quoting Lionel Landwerlin (2020-02-18 19:25:08)
> >>> On 18/02/2020 20:57, Chris Wilson wrote:
> >>>> Quoting Lionel Landwerlin (2020-02-17 14:21:53)
> >>>>> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
> >>>>> ---
> >>>>>    lib/i915-perf.pc.in                           |    1 +
> >>>>>    lib/i915/perf_data.h                          |  118 ++
> >>>>>    lib/meson.build                               |    1 +
> >>>>>    lib/tests/i915_perf_data_alignment.c          |   40 +
> >>>>>    lib/tests/meson.build                         |    1 +
> >>>>>    tools/i915-perf/i915_perf_control.c           |  133 +++
> >>>>>    tools/i915-perf/i915_perf_recorder.c          | 1052 
> >>>>> +++++++++++++++++
> >>>>>    tools/i915-perf/i915_perf_recorder_commands.h |   39 +
> >>>>>    tools/i915-perf/meson.build                   |   11 +
> >>>>>    9 files changed, 1396 insertions(+)
> >>>>>    create mode 100644 lib/i915/perf_data.h
> >>>>>    create mode 100644 lib/tests/i915_perf_data_alignment.c
> >>>>>    create mode 100644 tools/i915-perf/i915_perf_control.c
> >>>>>    create mode 100644 tools/i915-perf/i915_perf_recorder.c
> >>>>>    create mode 100644 tools/i915-perf/i915_perf_recorder_commands.h
> >>>>>
> >>>>> diff --git a/lib/i915-perf.pc.in b/lib/i915-perf.pc.in
> >>>>> index e72efdc3..8cfa3dbe 100644
> >>>>> --- a/lib/i915-perf.pc.in
> >>>>> +++ b/lib/i915-perf.pc.in
> >>>>> @@ -6,5 +6,6 @@ includedir=@includedir@
> >>>>>    Name: i915-perf
> >>>>>    Description: i915 perf library
> >>>>>    Version: @version@
> >>>>> +Requires: libdrm
> >>>> You only use the uAPI headers, and they are in include/drm-uapi/
> >>>> Right?
> >>>> -Chris
> >>> For IGT yeah, but for another application using this library, it needs
> >>> the dependency.
> >> Isn't that a Requires.private:
> >> No linkage required, but headers are?
> >> -Chris
> >
> > The headers are a structure pointer from i915_drm.h
> >
> > But I can probably forward declare this and potentially drop the need 
> > for any dep :)
> >
> >
> > -Lionel 
> 
> 
> Arg I was wrong, one of the packet in the perf_data.h embeds the 
> topology structure from i915_drm.h.
> 
> 
> I guess it could be rewritten just in there. What do you think?

Pull in the header via pkg-config, we should stick to a single canonical
header for the outside world. I think it is possible for our .pc to
avoid the linkage; we just risk Eero complaining about the million links
if we get it wrong.
-Chris