Re: [Intel-gfx] [igt-dev] [PATCH i-g-t v6] intel-gpu-top: Rewrite the tool to be safe to use
On 4 April 2018 at 16:26, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin
>
> intel-gpu-top is a dangerous tool which can hang machines due unsafe mmio
> register access. This patch rewrites it to use only PMU.
>
> Only overall command streamer busyness and GPU global data such as power
> and frequencies are included in this new version.
>
> For access to more GPU functional unit level data, an OA metric based tool
> like gpu-top should be used instead.
>
> v2:
> * Sort engines by class and instance.
> * Do not wait for one sampling period to display something on screen.
> * Move code out of the asserts. (Rinat Ibragimov)
> * Continuously adapt to terminal size. (Rinat Ibragimov)
>
> v3:
> * Change layout and precision of some field. (Chris Wilson)
> Eero Tamminen:
> * Use more user friendly engine names.
> * Don't error out if a counter is missing.
> * Add IMC read/write bandwidth.
> * Report minimum required kernel version.
>
> v4:
> * Really support 4.16 by skipping of missing engines.
> * Simpler and less hacky float printing.
> * Preserve copyright header. (Antonio Argenziano)
> * Simplify engines_ptr macro. (Rinat Ibragimov)
>
> v5:
> * Get RAPL unit from sysfs.
> * Consolidate sysfs paths with a macro.
> * Tidy error handling by carrying over and reporting errno.
> * Check against console height on all prints.
> * More readable minimum kernel version message. (Eero Tamminen)
> * Column banner for per engine stats. (Eero Tamminen)
>
> v6:
> * Man page update. (Eero Tamminen)
>
> Signed-off-by: Tvrtko Ursulin
> Cc: Chris Wilson
> Cc: Lionel Landwerlin
> Cc: Petri Latvala
> Cc: Eero Tamminen
> Cc: Rinat Ibragimov
> Reviewed-by: Lionel Landwerlin # v1
> Reviewed-by: Chris Wilson # v0.5
> ---
> lib/igt_perf.c|6 +
> lib/igt_perf.h|1 +
> man/intel_gpu_top.rst | 41 +-
> tools/Makefile.am |2 +
> tools/intel_gpu_top.c | 1250
> +++--
> tools/meson.build |6 +-
> 6 files changed, 719 insertions(+), 587 deletions(-)
>
> diff --git a/lib/igt_perf.c b/lib/igt_perf.c
> index 99d82ea51c9b..e3dec2cc29c7 100644
> --- a/lib/igt_perf.c
> +++ b/lib/igt_perf.c
> @@ -69,3 +69,9 @@ int igt_perf_open(uint64_t type, uint64_t config)
> return _perf_open(type, config, -1,
> PERF_FORMAT_TOTAL_TIME_ENABLED);
> }
> +
> +int igt_perf_open_group(uint64_t type, uint64_t config, int group)
> +{
> + return _perf_open(type, config, group,
> + PERF_FORMAT_TOTAL_TIME_ENABLED | PERF_FORMAT_GROUP);
> +}
> diff --git a/lib/igt_perf.h b/lib/igt_perf.h
> index 614ea5d23fa6..e00718f4769a 100644
> --- a/lib/igt_perf.h
> +++ b/lib/igt_perf.h
> @@ -55,5 +55,6 @@ uint64_t i915_type_id(void);
> int perf_i915_open(uint64_t config);
> int perf_i915_open_group(uint64_t config, int group);
> int igt_perf_open(uint64_t type, uint64_t config);
> +int igt_perf_open_group(uint64_t type, uint64_t config, int group);
>
> #endif /* I915_PERF_H */
> diff --git a/man/intel_gpu_top.rst b/man/intel_gpu_top.rst
> index a5f7175bb1a0..19c712307d28 100644
> --- a/man/intel_gpu_top.rst
> +++ b/man/intel_gpu_top.rst
> @@ -7,9 +7,9 @@ Display a top-like summary of Intel GPU usage
> -
> .. include:: defs.rst
> :Author: IGT Developers
> -:Date: 2016-03-01
> +:Date: 2018-04-04
> :Version: |PACKAGE_STRING|
> -:Copyright: 2009,2011,2012,2016 Intel Corporation
> +:Copyright: 2009,2011,2012,2016,2018 Intel Corporation
> :Manual section: |MANUAL_SECTION|
> :Manual group: |MANUAL_GROUP|
>
> @@ -21,42 +21,25 @@ SYNOPSIS
> DESCRIPTION
> ===
>
> -**intel_gpu_top** is a tool to display usage information of an Intel GPU. It
> -requires root privilege to map the graphics device.
> +**intel_gpu_top** is a tool to display usage information on Intel GPU's.
> +
> +The tool gathers data using perf performance counters (PMU) exposed by i915
> and other platform drivers like RAPL (power) and Uncore IMC (memory
> bandwidth).
>
> OPTIONS
> ===
>
> --s SAMPLES
> -Number of samples to acquire per second.
> -
> --o FILE
> -Collect usage statistics to FILE. If file is "-", run non-interactively
> -and output statistics to stdout.
> -
> --e COMMAND
> -Execute COMMAND to profile, and leave when it is finished. Note that the
> -entire command with all parameters should be included as one parameter.
> +-s
> +Refresh period in milliseconds.
>
> -h
> -Show usage notes.
> +Show help text.
>
> -EXAMPLES
> -
> -
> -intel_gpu_top -o "cairo-trace-gvim.log" -s 100 -e "cairo-perf-trace
> /tmp/gvim"
> -Run cairo-perf-trace with /tmp/gvim trace, non-interactively, saving the
> -statistics into cairo-trace-gvim.log file, and collecting 100 samples per
> -second.
> -
> -Note that idle units are not displayed, so an entirely idle GPU will only
> -display the ring status and header.
> +LIMITATIONS
> +
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t v6] intel-gpu-top: Rewrite the tool to be safe to use
Ping?
>Понедельник, 9 апреля 2018, 15:26 +03:00 от Tvrtko Ursulin
>:
>
>
>[Adding some people to Cc for more ack/nack type feedback.]
>
>Executive question is ack or nack on replacing intel_gpu_top with a new
>implementation which uses only perf PMU for counter gathering.
>
>A short history on how this came to be:
>
>There was a recent external patch contribution from Rinat Ibragimov to
>support more platforms from the existing intel_gpu_top. But as the tool
>is not safe to use Chris Wilson suggested to maybe just replace it.
>
>As it happens I had a good start to do this quickly and cheaply, in the
>form of one prototype I did recently, which only needed ripping some
>bits out, and polishing the rest.
>
>Eero and Rinat kindly did a lot of platform coverage testing and the
>rewrite seems ready for next steps.
>
>I need to stress that as the commit notes, the new tool has a slightly
>different scope as that it doesn't expose GPU functional level data, but
>only overall stats like power, frequencies, RC6, interrupts, IMC memory
>bandwidth and per command streamer busyness, mi_semaphore and mi_event
>waits. My thinking was that for more functional level profiling gpu-top
>(OA) should be used.
>
>Also the "run a command" and CSV output features are not not supported
>since both can be done directly via perf stat.
>
>Regards,
>
>Tvrtko
>
>On 04/04/2018 16:26, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin < tvrtko.ursu...@intel.com >
>>
>> intel-gpu-top is a dangerous tool which can hang machines due unsafe mmio
>> register access. This patch rewrites it to use only PMU.
>>
>> Only overall command streamer busyness and GPU global data such as power
>> and frequencies are included in this new version.
>>
>> For access to more GPU functional unit level data, an OA metric based tool
>> like gpu-top should be used instead.
>>
>> v2:
>> * Sort engines by class and instance.
>> * Do not wait for one sampling period to display something on screen.
>> * Move code out of the asserts. (Rinat Ibragimov)
>> * Continuously adapt to terminal size. (Rinat Ibragimov)
>>
>> v3:
>> * Change layout and precision of some field. (Chris Wilson)
>> Eero Tamminen:
>> * Use more user friendly engine names.
>> * Don't error out if a counter is missing.
>> * Add IMC read/write bandwidth.
>> * Report minimum required kernel version.
>>
>> v4:
>> * Really support 4.16 by skipping of missing engines.
>> * Simpler and less hacky float printing.
>> * Preserve copyright header. (Antonio Argenziano)
>> * Simplify engines_ptr macro. (Rinat Ibragimov)
>>
>> v5:
>> * Get RAPL unit from sysfs.
>> * Consolidate sysfs paths with a macro.
>> * Tidy error handling by carrying over and reporting errno.
>> * Check against console height on all prints.
>> * More readable minimum kernel version message. (Eero Tamminen)
>> * Column banner for per engine stats. (Eero Tamminen)
>>
>> v6:
>> * Man page update. (Eero Tamminen)
>>
>> Signed-off-by: Tvrtko Ursulin < tvrtko.ursu...@intel.com >
>> Cc: Chris Wilson < ch...@chris-wilson.co.uk >
>> Cc: Lionel Landwerlin < lionel.g.landwer...@intel.com >
>> Cc: Petri Latvala < petri.latv...@intel.com >
>> Cc: Eero Tamminen < eero.t.tammi...@intel.com >
>> Cc: Rinat Ibragimov < ibragimovri...@mail.ru >
>> Reviewed-by: Lionel Landwerlin < lionel.g.landwer...@intel.com > # v1
>> Reviewed-by: Chris Wilson < ch...@chris-wilson.co.uk > # v0.5
>> ---
>> lib/igt_perf.c|6 +
>> lib/igt_perf.h|1 +
>> man/intel_gpu_top.rst | 41 +-
>> tools/Makefile.am |2 +
>> tools/intel_gpu_top.c | 1250
>> +++--
>> tools/meson.build |6 +-
>> 6 files changed, 719 insertions(+), 587 deletions(-)
>>
>> diff --git a/lib/igt_perf.c b/lib/igt_perf.c
>> index 99d82ea51c9b..e3dec2cc29c7 100644
>> --- a/lib/igt_perf.c
>> +++ b/lib/igt_perf.c
>> @@ -69,3 +69,9 @@ int igt_perf_open(uint64_t type, uint64_t config)
>> return _perf_open(type, config, -1,
>> PERF_FORMAT_TOTAL_TIME_ENABLED);
>> }
>> +
>> +int igt_perf_open_group(uint64_t type, uint64_t config, int group)
>> +{
>> +return _perf_open(type, config, group,
>> + PERF_FORMAT_TOTAL_TIME_ENABLED | PERF_FORMAT_GROUP);
>> +}
>> diff --git a/lib/igt_perf.h b/lib/igt_perf.h
>> index 614ea5d23fa6..e00718f4769a 100644
>> --- a/lib/igt_perf.h
>> +++ b/lib/igt_perf.h
>> @@ -55,5 +55,6 @@ uint64_t i915_type_id(void);
>> int perf_i915_open(uint64_t config);
>> int perf_i915_open_group(uint64_t config, int group);
>> int igt_perf_open(uint64_t type, uint64_t config);
>> +int igt_perf_open_group(uint64_t type, uint64_t config, int group);
>>
>> #endif /* I915_PERF_H */
>> diff --git a/man/intel_gpu_top.rst b/man/intel_gpu_top.rst
>> index a5f7175bb1a0..19c712307d28 100644
>> --- a/man/intel_gpu_top.rst
>> +++ b/man/intel_gpu_top.rst
>> @@ -7,9 +7,9 @@ Display a top-like summary of Intel GPU usage
>> ---
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t v6] intel-gpu-top: Rewrite the tool to be safe to use
[Adding some people to Cc for more ack/nack type feedback.]
Executive question is ack or nack on replacing intel_gpu_top with a new
implementation which uses only perf PMU for counter gathering.
A short history on how this came to be:
There was a recent external patch contribution from Rinat Ibragimov to
support more platforms from the existing intel_gpu_top. But as the tool
is not safe to use Chris Wilson suggested to maybe just replace it.
As it happens I had a good start to do this quickly and cheaply, in the
form of one prototype I did recently, which only needed ripping some
bits out, and polishing the rest.
Eero and Rinat kindly did a lot of platform coverage testing and the
rewrite seems ready for next steps.
I need to stress that as the commit notes, the new tool has a slightly
different scope as that it doesn't expose GPU functional level data, but
only overall stats like power, frequencies, RC6, interrupts, IMC memory
bandwidth and per command streamer busyness, mi_semaphore and mi_event
waits. My thinking was that for more functional level profiling gpu-top
(OA) should be used.
Also the "run a command" and CSV output features are not not supported
since both can be done directly via perf stat.
Regards,
Tvrtko
On 04/04/2018 16:26, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin
intel-gpu-top is a dangerous tool which can hang machines due unsafe mmio
register access. This patch rewrites it to use only PMU.
Only overall command streamer busyness and GPU global data such as power
and frequencies are included in this new version.
For access to more GPU functional unit level data, an OA metric based tool
like gpu-top should be used instead.
v2:
* Sort engines by class and instance.
* Do not wait for one sampling period to display something on screen.
* Move code out of the asserts. (Rinat Ibragimov)
* Continuously adapt to terminal size. (Rinat Ibragimov)
v3:
* Change layout and precision of some field. (Chris Wilson)
Eero Tamminen:
* Use more user friendly engine names.
* Don't error out if a counter is missing.
* Add IMC read/write bandwidth.
* Report minimum required kernel version.
v4:
* Really support 4.16 by skipping of missing engines.
* Simpler and less hacky float printing.
* Preserve copyright header. (Antonio Argenziano)
* Simplify engines_ptr macro. (Rinat Ibragimov)
v5:
* Get RAPL unit from sysfs.
* Consolidate sysfs paths with a macro.
* Tidy error handling by carrying over and reporting errno.
* Check against console height on all prints.
* More readable minimum kernel version message. (Eero Tamminen)
* Column banner for per engine stats. (Eero Tamminen)
v6:
* Man page update. (Eero Tamminen)
Signed-off-by: Tvrtko Ursulin
Cc: Chris Wilson
Cc: Lionel Landwerlin
Cc: Petri Latvala
Cc: Eero Tamminen
Cc: Rinat Ibragimov
Reviewed-by: Lionel Landwerlin # v1
Reviewed-by: Chris Wilson # v0.5
---
lib/igt_perf.c|6 +
lib/igt_perf.h|1 +
man/intel_gpu_top.rst | 41 +-
tools/Makefile.am |2 +
tools/intel_gpu_top.c | 1250 +++--
tools/meson.build |6 +-
6 files changed, 719 insertions(+), 587 deletions(-)
diff --git a/lib/igt_perf.c b/lib/igt_perf.c
index 99d82ea51c9b..e3dec2cc29c7 100644
--- a/lib/igt_perf.c
+++ b/lib/igt_perf.c
@@ -69,3 +69,9 @@ int igt_perf_open(uint64_t type, uint64_t config)
return _perf_open(type, config, -1,
PERF_FORMAT_TOTAL_TIME_ENABLED);
}
+
+int igt_perf_open_group(uint64_t type, uint64_t config, int group)
+{
+ return _perf_open(type, config, group,
+ PERF_FORMAT_TOTAL_TIME_ENABLED | PERF_FORMAT_GROUP);
+}
diff --git a/lib/igt_perf.h b/lib/igt_perf.h
index 614ea5d23fa6..e00718f4769a 100644
--- a/lib/igt_perf.h
+++ b/lib/igt_perf.h
@@ -55,5 +55,6 @@ uint64_t i915_type_id(void);
int perf_i915_open(uint64_t config);
int perf_i915_open_group(uint64_t config, int group);
int igt_perf_open(uint64_t type, uint64_t config);
+int igt_perf_open_group(uint64_t type, uint64_t config, int group);
#endif /* I915_PERF_H */
diff --git a/man/intel_gpu_top.rst b/man/intel_gpu_top.rst
index a5f7175bb1a0..19c712307d28 100644
--- a/man/intel_gpu_top.rst
+++ b/man/intel_gpu_top.rst
@@ -7,9 +7,9 @@ Display a top-like summary of Intel GPU usage
-
.. include:: defs.rst
:Author: IGT Developers
-:Date: 2016-03-01
+:Date: 2018-04-04
:Version: |PACKAGE_STRING|
-:Copyright: 2009,2011,2012,2016 Intel Corporation
+:Copyright: 2009,2011,2012,2016,2018 Intel Corporation
:Manual section: |MANUAL_SECTION|
:Manual group: |MANUAL_GROUP|
@@ -21,42 +21,25 @@ SYNOPSIS
DESCRIPTION
===
-**intel_gpu_top** is a tool to display usage information of an Intel GPU. It
-requires root privilege to map the graphics device.
+**intel_gpu_top** is a tool to display usage information on Intel GPU
