* [Intel-gfx] [RFC i-g-t] intel_gpu_top: User friendly device listing
@ 2020-10-14 10:48 Tvrtko Ursulin
2020-10-14 11:05 ` Chris Wilson
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Tvrtko Ursulin @ 2020-10-14 10:48 UTC (permalink / raw)
To: igt-dev; +Cc: Intel-gfx
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Adding a new device selection print type suitable for user-facing
use cases like intel_gpu_top -L and potentially lsgpu.
Instead of:
sys:/sys/devices/pci0000:00/0000:00:02.0/drm/card0
subsystem : drm
drm card : /dev/dri/card0
parent : sys:/sys/devices/pci0000:00/0000:00:02.0
sys:/sys/devices/pci0000:00/0000:00:02.0/drm/renderD128
subsystem : drm
drm render : /dev/dri/renderD128
parent : sys:/sys/devices/pci0000:00/0000:00:02.0
sys:/sys/devices/pci0000:00/0000:00:02.0
subsystem : pci
drm card : /dev/dri/card0
drm render : /dev/dri/renderD128
vendor : 8086
device : 193B
New format looks like:
card0 8086:193B drm:/dev/dri/card0
└─renderD128 drm:/dev/dri/renderD128
Advantages are more compact, more readable, one entry per GPU, shorter
string to copy and paste to intel_gpu_top -d, or respective usage.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Petri Latvala <petri.latvala@intel.com>
Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
---
lib/igt_device_scan.c | 109 +++++++++++++++++++++++++++++++++++++-----
lib/igt_device_scan.h | 1 +
tools/intel_gpu_top.c | 3 +-
3 files changed, 100 insertions(+), 13 deletions(-)
diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
index f4d43c733314..ce0ea61dda50 100644
--- a/lib/igt_device_scan.c
+++ b/lib/igt_device_scan.c
@@ -695,18 +695,26 @@ static inline void _pr_simple2(const char *k, const char *v1, const char *v2)
printf(" %-16s: %s:%s\n", k, v1, v2);
}
-static void igt_devs_print_simple(struct igt_list_head *view)
+static bool __check_empty(struct igt_list_head *view)
{
- struct igt_device *dev;
-
if (!view)
- return;
+ return true;
if (igt_list_empty(view)) {
printf("No GPU devices found\n");
- return;
+ return true;
}
+ return false;
+}
+
+static void igt_devs_print_simple(struct igt_list_head *view)
+{
+ struct igt_device *dev;
+
+ if (__check_empty(view))
+ return;
+
igt_list_for_each_entry(dev, view, link) {
printf("sys:%s\n", dev->syspath);
if (dev->subsystem)
@@ -728,6 +736,89 @@ static void igt_devs_print_simple(struct igt_list_head *view)
}
}
+static struct igt_device *
+__find_pci(struct igt_list_head *view, const char *drm)
+{
+ struct igt_device *dev;
+
+ igt_list_for_each_entry(dev, view, link) {
+ if (!is_pci_subsystem(dev) || !dev->drm_card)
+ continue;
+
+ if (!strcmp(dev->drm_card, drm))
+ return dev;
+ }
+
+ return NULL;
+}
+
+static void igt_devs_print_user(struct igt_list_head *view)
+{
+ struct igt_device *dev;
+
+ if (__check_empty(view))
+ return;
+
+ igt_list_for_each_entry(dev, view, link) {
+ unsigned int i, num_children;
+ struct igt_device *pci_dev;
+ struct igt_device *dev2;
+ char filter[64];
+ char *drm_name;
+ int ret;
+
+ if (!is_drm_subsystem(dev))
+ continue;
+ if (!dev->drm_card || dev->drm_render)
+ continue;
+
+ drm_name = rindex(dev->drm_card, '/');
+ if (!drm_name || !*++drm_name)
+ continue;
+
+ ret = snprintf(filter, sizeof(filter), "drm:%s", dev->drm_card);
+ igt_assert(ret < sizeof(filter));
+
+ pci_dev = __find_pci(view, dev->drm_card);
+ if (pci_dev)
+ printf("%-24s%4s:%4s %s\n",
+ drm_name, pci_dev->vendor, pci_dev->device,
+ filter);
+ else
+ printf("%-24s %s\n", drm_name, filter);
+
+ num_children = 0;
+ igt_list_for_each_entry(dev2, view, link) {
+ if (!is_drm_subsystem(dev2) || !dev2->drm_render)
+ continue;
+ if (strcmp(dev2->parent->syspath, dev->parent->syspath))
+ continue;
+
+ num_children++;
+ }
+
+ i = 0;
+ igt_list_for_each_entry(dev2, view, link) {
+ if (!is_drm_subsystem(dev2) || !dev2->drm_render)
+ continue;
+ if (strcmp(dev2->parent->syspath, dev->parent->syspath))
+ continue;
+
+ drm_name = rindex(dev2->drm_render, '/');
+ if (!drm_name || !*++drm_name)
+ continue;
+
+ ret = snprintf(filter, sizeof(filter), "drm:%s",
+ dev2->drm_render);
+ igt_assert(ret < sizeof(filter));
+
+ printf("%s%-22s %s\n",
+ (++i == num_children) ? "└─" : "├─",
+ drm_name, filter);
+ }
+ }
+}
+
static inline void _print_key_value(const char* k, const char *v)
{
printf("%-32s: %s\n", k, v);
@@ -752,14 +843,9 @@ static void igt_devs_print_detail(struct igt_list_head *view)
{
struct igt_device *dev;
- if (!view)
+ if (__check_empty(view))
return;
- if (igt_list_empty(view)) {
- printf("No GPU devices found\n");
- return;
- }
-
igt_list_for_each_entry(dev, view, link) {
printf("========== %s:%s ==========\n",
dev->subsystem, dev->syspath);
@@ -781,6 +867,7 @@ static struct print_func {
} print_functions[] = {
[IGT_PRINT_SIMPLE] = { .prn = igt_devs_print_simple },
[IGT_PRINT_DETAIL] = { .prn = igt_devs_print_detail },
+ [IGT_PRINT_USER] = { .prn = igt_devs_print_user },
};
/**
diff --git a/lib/igt_device_scan.h b/lib/igt_device_scan.h
index bd937d22752c..9e13ed9db406 100644
--- a/lib/igt_device_scan.h
+++ b/lib/igt_device_scan.h
@@ -37,6 +37,7 @@
enum igt_devices_print_type {
IGT_PRINT_SIMPLE,
IGT_PRINT_DETAIL,
+ IGT_PRINT_USER, /* End user friendly. */
};
#define INTEGRATED_I915_GPU_PCI_ID "0000:00:02.0"
diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
index 79a936ffbe1a..b984edc656c7 100644
--- a/tools/intel_gpu_top.c
+++ b/tools/intel_gpu_top.c
@@ -1313,7 +1313,6 @@ int main(int argc, char **argv)
unsigned int i;
int ret = 0, ch;
bool list_device = false;
- enum igt_devices_print_type printtype = IGT_PRINT_SIMPLE;
char *pmu_device, *opt_device = NULL;
struct igt_device_card card;
@@ -1388,7 +1387,7 @@ int main(int argc, char **argv)
igt_devices_scan(false);
if (list_device) {
- igt_devices_print(printtype);
+ igt_devices_print(IGT_PRINT_USER);
goto exit;
}
--
2.25.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [RFC i-g-t] intel_gpu_top: User friendly device listing
2020-10-14 10:48 [Intel-gfx] [RFC i-g-t] intel_gpu_top: User friendly device listing Tvrtko Ursulin
@ 2020-10-14 11:05 ` Chris Wilson
2020-10-14 12:01 ` [igt-dev] " Tvrtko Ursulin
2020-10-14 11:43 ` [igt-dev] ✓ Fi.CI.BAT: success for " Patchwork
2020-10-15 4:36 ` [igt-dev] " Zbigniew Kempczyński
2 siblings, 1 reply; 14+ messages in thread
From: Chris Wilson @ 2020-10-14 11:05 UTC (permalink / raw)
To: Tvrtko Ursulin, igt-dev; +Cc: Intel-gfx
Quoting Tvrtko Ursulin (2020-10-14 11:48:53)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> Adding a new device selection print type suitable for user-facing
> use cases like intel_gpu_top -L and potentially lsgpu.
>
> Instead of:
>
> sys:/sys/devices/pci0000:00/0000:00:02.0/drm/card0
> subsystem : drm
> drm card : /dev/dri/card0
> parent : sys:/sys/devices/pci0000:00/0000:00:02.0
>
> sys:/sys/devices/pci0000:00/0000:00:02.0/drm/renderD128
> subsystem : drm
> drm render : /dev/dri/renderD128
> parent : sys:/sys/devices/pci0000:00/0000:00:02.0
>
> sys:/sys/devices/pci0000:00/0000:00:02.0
> subsystem : pci
> drm card : /dev/dri/card0
> drm render : /dev/dri/renderD128
> vendor : 8086
> device : 193B
>
> New format looks like:
>
> card0 8086:193B drm:/dev/dri/card0
> └─renderD128 drm:/dev/dri/renderD128
>
> Advantages are more compact, more readable, one entry per GPU, shorter
> string to copy and paste to intel_gpu_top -d, or respective usage.
Could you present this as the typical commands and output? I think you
mean the first as the output of intel_gpu_top -L. What exactly do we now
feed to -d?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* [igt-dev] ✓ Fi.CI.BAT: success for intel_gpu_top: User friendly device listing
2020-10-14 10:48 [Intel-gfx] [RFC i-g-t] intel_gpu_top: User friendly device listing Tvrtko Ursulin
2020-10-14 11:05 ` Chris Wilson
@ 2020-10-14 11:43 ` Patchwork
2020-10-15 4:36 ` [igt-dev] " Zbigniew Kempczyński
2 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2020-10-14 11:43 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: igt-dev
[-- Attachment #1.1: Type: text/plain, Size: 5836 bytes --]
== Series Details ==
Series: intel_gpu_top: User friendly device listing
URL : https://patchwork.freedesktop.org/series/82671/
State : success
== Summary ==
CI Bug Log - changes from IGT_5815 -> IGTPW_5068
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5068/index.html
Known issues
------------
Here are the changes found in IGTPW_5068 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_flink_basic@flink-lifetime:
- fi-tgl-y: [PASS][1] -> [DMESG-WARN][2] ([i915#402])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5815/fi-tgl-y/igt@gem_flink_basic@flink-lifetime.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5068/fi-tgl-y/igt@gem_flink_basic@flink-lifetime.html
* igt@i915_module_load@reload:
- fi-icl-y: [PASS][3] -> [DMESG-WARN][4] ([i915#1982])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5815/fi-icl-y/igt@i915_module_load@reload.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5068/fi-icl-y/igt@i915_module_load@reload.html
* igt@kms_busy@basic@flip:
- fi-tgl-y: [PASS][5] -> [DMESG-WARN][6] ([i915#1982])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5815/fi-tgl-y/igt@kms_busy@basic@flip.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5068/fi-tgl-y/igt@kms_busy@basic@flip.html
* igt@kms_chamelium@hdmi-crc-fast:
- fi-kbl-7500u: [PASS][7] -> [DMESG-WARN][8] ([i915#2203])
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5815/fi-kbl-7500u/igt@kms_chamelium@hdmi-crc-fast.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5068/fi-kbl-7500u/igt@kms_chamelium@hdmi-crc-fast.html
* igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
- fi-bsw-kefka: [PASS][9] -> [DMESG-WARN][10] ([i915#1982])
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5815/fi-bsw-kefka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5068/fi-bsw-kefka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html
* igt@kms_cursor_legacy@basic-flip-after-cursor-legacy:
- fi-icl-u2: [PASS][11] -> [DMESG-WARN][12] ([i915#1982])
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5815/fi-icl-u2/igt@kms_cursor_legacy@basic-flip-after-cursor-legacy.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5068/fi-icl-u2/igt@kms_cursor_legacy@basic-flip-after-cursor-legacy.html
#### Possible fixes ####
* igt@gem_flink_basic@bad-flink:
- fi-tgl-y: [DMESG-WARN][13] ([i915#402]) -> [PASS][14]
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5815/fi-tgl-y/igt@gem_flink_basic@bad-flink.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5068/fi-tgl-y/igt@gem_flink_basic@bad-flink.html
* igt@i915_pm_rpm@basic-pci-d3-state:
- fi-bsw-kefka: [DMESG-WARN][15] ([i915#1982]) -> [PASS][16]
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5815/fi-bsw-kefka/igt@i915_pm_rpm@basic-pci-d3-state.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5068/fi-bsw-kefka/igt@i915_pm_rpm@basic-pci-d3-state.html
* igt@kms_chamelium@dp-crc-fast:
- fi-kbl-7500u: [DMESG-WARN][17] ([i915#165]) -> [PASS][18]
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5815/fi-kbl-7500u/igt@kms_chamelium@dp-crc-fast.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5068/fi-kbl-7500u/igt@kms_chamelium@dp-crc-fast.html
* igt@kms_cursor_legacy@basic-flip-before-cursor-legacy:
- fi-icl-u2: [DMESG-WARN][19] ([i915#1982]) -> [PASS][20]
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5815/fi-icl-u2/igt@kms_cursor_legacy@basic-flip-before-cursor-legacy.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5068/fi-icl-u2/igt@kms_cursor_legacy@basic-flip-before-cursor-legacy.html
#### Warnings ####
* igt@i915_pm_rpm@module-reload:
- fi-tgl-y: [DMESG-WARN][21] ([i915#2411]) -> [DMESG-WARN][22] ([i915#1982] / [i915#2411])
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5815/fi-tgl-y/igt@i915_pm_rpm@module-reload.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5068/fi-tgl-y/igt@i915_pm_rpm@module-reload.html
* igt@kms_frontbuffer_tracking@basic:
- fi-tgl-y: [FAIL][23] -> [DMESG-FAIL][24] ([i915#1982])
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5815/fi-tgl-y/igt@kms_frontbuffer_tracking@basic.html
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5068/fi-tgl-y/igt@kms_frontbuffer_tracking@basic.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[i915#165]: https://gitlab.freedesktop.org/drm/intel/issues/165
[i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
[i915#2203]: https://gitlab.freedesktop.org/drm/intel/issues/2203
[i915#2411]: https://gitlab.freedesktop.org/drm/intel/issues/2411
[i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402
Participating hosts (46 -> 40)
------------------------------
Missing (6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-byt-clapper fi-bdw-samus
Build changes
-------------
* CI: CI-20190529 -> None
* IGT: IGT_5815 -> IGTPW_5068
CI-20190529: 20190529
CI_DRM_9138: 5e4234f97efbaa30f0beb243dcf98fe0a0bb0945 @ git://anongit.freedesktop.org/gfx-ci/linux
IGTPW_5068: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5068/index.html
IGT_5815: 0c3b29498a624ad42033a219d031cb9dd475405b @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5068/index.html
[-- Attachment #1.2: Type: text/html, Size: 7347 bytes --]
[-- Attachment #2: Type: text/plain, Size: 154 bytes --]
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [RFC i-g-t] intel_gpu_top: User friendly device listing
2020-10-14 11:05 ` Chris Wilson
@ 2020-10-14 12:01 ` Tvrtko Ursulin
0 siblings, 0 replies; 14+ messages in thread
From: Tvrtko Ursulin @ 2020-10-14 12:01 UTC (permalink / raw)
To: Chris Wilson, igt-dev; +Cc: Intel-gfx
On 14/10/2020 12:05, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2020-10-14 11:48:53)
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Adding a new device selection print type suitable for user-facing
>> use cases like intel_gpu_top -L and potentially lsgpu.
>>
>> Instead of:
>>
>> sys:/sys/devices/pci0000:00/0000:00:02.0/drm/card0
>> subsystem : drm
>> drm card : /dev/dri/card0
>> parent : sys:/sys/devices/pci0000:00/0000:00:02.0
>>
>> sys:/sys/devices/pci0000:00/0000:00:02.0/drm/renderD128
>> subsystem : drm
>> drm render : /dev/dri/renderD128
>> parent : sys:/sys/devices/pci0000:00/0000:00:02.0
>>
>> sys:/sys/devices/pci0000:00/0000:00:02.0
>> subsystem : pci
>> drm card : /dev/dri/card0
>> drm render : /dev/dri/renderD128
>> vendor : 8086
>> device : 193B
>>
>> New format looks like:
>>
>> card0 8086:193B drm:/dev/dri/card0
>> └─renderD128 drm:/dev/dri/renderD128
>>
>> Advantages are more compact, more readable, one entry per GPU, shorter
>> string to copy and paste to intel_gpu_top -d, or respective usage.
>
> Could you present this as the typical commands and output? I think you
> mean the first as the output of intel_gpu_top -L. What exactly do we now
> feed to -d?
root@sc:~/igt/build# tools/intel_gpu_top -L
card0 8086:193B drm:/dev/dri/card0
└─renderD128 drm:/dev/dri/renderD128
root@sc:~/igt/build# tools/intel_gpu_top -d drm:/dev/dri/card0
I was considering adding a header, but essentially the actual format is just an idea for now. Main point is coming up with something more easily readable by the user, with no multiple entries for the same thing, and compact for easy copy & paste. Motivation was current output on a three GPU system:
sys:/sys/devices/pci0000:00/0000:00:01.0/0000:01:00.0/0000:02:01.0/0000:03:00.0/drm/card1
subsystem : drm
drm card : /dev/dri/card1
parent : sys:/sys/devices/pci0000:00/0000:00:01.0/0000:01:00.0/0000:02:01.0/0000:03:00.0
sys:/sys/devices/pci0000:00/0000:00:01.0/0000:01:00.0/0000:02:01.0/0000:03:00.0/drm/renderD129
subsystem : drm
drm render : /dev/dri/renderD129
parent : sys:/sys/devices/pci0000:00/0000:00:01.0/0000:01:00.0/0000:02:01.0/0000:03:00.0
sys:/sys/devices/pci0000:00/0000:00:01.1/0000:06:00.0/0000:07:01.0/0000:08:00.0/drm/card2
subsystem : drm
drm card : /dev/dri/card2
parent : sys:/sys/devices/pci0000:00/0000:00:01.1/0000:06:00.0/0000:07:01.0/0000:08:00.0
sys:/sys/devices/pci0000:00/0000:00:01.1/0000:06:00.0/0000:07:01.0/0000:08:00.0/drm/renderD130
subsystem : drm
drm render : /dev/dri/renderD130
parent : sys:/sys/devices/pci0000:00/0000:00:01.1/0000:06:00.0/0000:07:01.0/0000:08:00.0
sys:/sys/devices/pci0000:00/0000:00:02.0/drm/card0
subsystem : drm
drm card : /dev/dri/card0
parent : sys:/sys/devices/pci0000:00/0000:00:02.0
sys:/sys/devices/pci0000:00/0000:00:02.0/drm/renderD128
subsystem : drm
drm render : /dev/dri/renderD128
parent : sys:/sys/devices/pci0000:00/0000:00:02.0
sys:/sys/devices/pci0000:00/0000:00:01.0/0000:01:00.0/0000:02:01.0/0000:03:00.0
subsystem : pci
drm card : /dev/dri/card1
drm render : /dev/dri/renderD129
vendor : 8086
device : 4905
sys:/sys/devices/pci0000:00/0000:00:01.1/0000:06:00.0/0000:07:01.0/0000:08:00.0
subsystem : pci
drm card : /dev/dri/card2
drm render : /dev/dri/renderD130
vendor : 8086
device : 4905
sys:/sys/devices/pci0000:00/0000:00:02.0
subsystem : pci
drm card : /dev/dri/card0
drm render : /dev/dri/renderD128
vendor : 8086
device : 3E98
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [igt-dev] [Intel-gfx] [RFC i-g-t] intel_gpu_top: User friendly device listing
@ 2020-10-14 12:01 ` Tvrtko Ursulin
0 siblings, 0 replies; 14+ messages in thread
From: Tvrtko Ursulin @ 2020-10-14 12:01 UTC (permalink / raw)
To: Chris Wilson, igt-dev; +Cc: Intel-gfx
On 14/10/2020 12:05, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2020-10-14 11:48:53)
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Adding a new device selection print type suitable for user-facing
>> use cases like intel_gpu_top -L and potentially lsgpu.
>>
>> Instead of:
>>
>> sys:/sys/devices/pci0000:00/0000:00:02.0/drm/card0
>> subsystem : drm
>> drm card : /dev/dri/card0
>> parent : sys:/sys/devices/pci0000:00/0000:00:02.0
>>
>> sys:/sys/devices/pci0000:00/0000:00:02.0/drm/renderD128
>> subsystem : drm
>> drm render : /dev/dri/renderD128
>> parent : sys:/sys/devices/pci0000:00/0000:00:02.0
>>
>> sys:/sys/devices/pci0000:00/0000:00:02.0
>> subsystem : pci
>> drm card : /dev/dri/card0
>> drm render : /dev/dri/renderD128
>> vendor : 8086
>> device : 193B
>>
>> New format looks like:
>>
>> card0 8086:193B drm:/dev/dri/card0
>> └─renderD128 drm:/dev/dri/renderD128
>>
>> Advantages are more compact, more readable, one entry per GPU, shorter
>> string to copy and paste to intel_gpu_top -d, or respective usage.
>
> Could you present this as the typical commands and output? I think you
> mean the first as the output of intel_gpu_top -L. What exactly do we now
> feed to -d?
root@sc:~/igt/build# tools/intel_gpu_top -L
card0 8086:193B drm:/dev/dri/card0
└─renderD128 drm:/dev/dri/renderD128
root@sc:~/igt/build# tools/intel_gpu_top -d drm:/dev/dri/card0
I was considering adding a header, but essentially the actual format is just an idea for now. Main point is coming up with something more easily readable by the user, with no multiple entries for the same thing, and compact for easy copy & paste. Motivation was current output on a three GPU system:
sys:/sys/devices/pci0000:00/0000:00:01.0/0000:01:00.0/0000:02:01.0/0000:03:00.0/drm/card1
subsystem : drm
drm card : /dev/dri/card1
parent : sys:/sys/devices/pci0000:00/0000:00:01.0/0000:01:00.0/0000:02:01.0/0000:03:00.0
sys:/sys/devices/pci0000:00/0000:00:01.0/0000:01:00.0/0000:02:01.0/0000:03:00.0/drm/renderD129
subsystem : drm
drm render : /dev/dri/renderD129
parent : sys:/sys/devices/pci0000:00/0000:00:01.0/0000:01:00.0/0000:02:01.0/0000:03:00.0
sys:/sys/devices/pci0000:00/0000:00:01.1/0000:06:00.0/0000:07:01.0/0000:08:00.0/drm/card2
subsystem : drm
drm card : /dev/dri/card2
parent : sys:/sys/devices/pci0000:00/0000:00:01.1/0000:06:00.0/0000:07:01.0/0000:08:00.0
sys:/sys/devices/pci0000:00/0000:00:01.1/0000:06:00.0/0000:07:01.0/0000:08:00.0/drm/renderD130
subsystem : drm
drm render : /dev/dri/renderD130
parent : sys:/sys/devices/pci0000:00/0000:00:01.1/0000:06:00.0/0000:07:01.0/0000:08:00.0
sys:/sys/devices/pci0000:00/0000:00:02.0/drm/card0
subsystem : drm
drm card : /dev/dri/card0
parent : sys:/sys/devices/pci0000:00/0000:00:02.0
sys:/sys/devices/pci0000:00/0000:00:02.0/drm/renderD128
subsystem : drm
drm render : /dev/dri/renderD128
parent : sys:/sys/devices/pci0000:00/0000:00:02.0
sys:/sys/devices/pci0000:00/0000:00:01.0/0000:01:00.0/0000:02:01.0/0000:03:00.0
subsystem : pci
drm card : /dev/dri/card1
drm render : /dev/dri/renderD129
vendor : 8086
device : 4905
sys:/sys/devices/pci0000:00/0000:00:01.1/0000:06:00.0/0000:07:01.0/0000:08:00.0
subsystem : pci
drm card : /dev/dri/card2
drm render : /dev/dri/renderD130
vendor : 8086
device : 4905
sys:/sys/devices/pci0000:00/0000:00:02.0
subsystem : pci
drm card : /dev/dri/card0
drm render : /dev/dri/renderD128
vendor : 8086
device : 3E98
Regards,
Tvrtko
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [RFC i-g-t] intel_gpu_top: User friendly device listing
2020-10-14 10:48 [Intel-gfx] [RFC i-g-t] intel_gpu_top: User friendly device listing Tvrtko Ursulin
@ 2020-10-15 4:36 ` Zbigniew Kempczyński
2020-10-14 11:43 ` [igt-dev] ✓ Fi.CI.BAT: success for " Patchwork
2020-10-15 4:36 ` [igt-dev] " Zbigniew Kempczyński
2 siblings, 0 replies; 14+ messages in thread
From: Zbigniew Kempczyński @ 2020-10-15 4:36 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: igt-dev, Intel-gfx
On Wed, Oct 14, 2020 at 11:48:53AM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> Adding a new device selection print type suitable for user-facing
> use cases like intel_gpu_top -L and potentially lsgpu.
>
> Instead of:
>
> sys:/sys/devices/pci0000:00/0000:00:02.0/drm/card0
> subsystem : drm
> drm card : /dev/dri/card0
> parent : sys:/sys/devices/pci0000:00/0000:00:02.0
>
> sys:/sys/devices/pci0000:00/0000:00:02.0/drm/renderD128
> subsystem : drm
> drm render : /dev/dri/renderD128
> parent : sys:/sys/devices/pci0000:00/0000:00:02.0
>
> sys:/sys/devices/pci0000:00/0000:00:02.0
> subsystem : pci
> drm card : /dev/dri/card0
> drm render : /dev/dri/renderD128
> vendor : 8086
> device : 193B
>
> New format looks like:
>
> card0 8086:193B drm:/dev/dri/card0
> └─renderD128 drm:/dev/dri/renderD128
>
> Advantages are more compact, more readable, one entry per GPU, shorter
> string to copy and paste to intel_gpu_top -d, or respective usage.
Looks nice and more intuitive.
1. Add -s switch to handle simple print (current default in lsgpu)
2. Change to user format as default.
--
Zbigniew
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Petri Latvala <petri.latvala@intel.com>
> Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> ---
> lib/igt_device_scan.c | 109 +++++++++++++++++++++++++++++++++++++-----
> lib/igt_device_scan.h | 1 +
> tools/intel_gpu_top.c | 3 +-
> 3 files changed, 100 insertions(+), 13 deletions(-)
>
> diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
> index f4d43c733314..ce0ea61dda50 100644
> --- a/lib/igt_device_scan.c
> +++ b/lib/igt_device_scan.c
> @@ -695,18 +695,26 @@ static inline void _pr_simple2(const char *k, const char *v1, const char *v2)
> printf(" %-16s: %s:%s\n", k, v1, v2);
> }
>
> -static void igt_devs_print_simple(struct igt_list_head *view)
> +static bool __check_empty(struct igt_list_head *view)
> {
> - struct igt_device *dev;
> -
> if (!view)
> - return;
> + return true;
>
> if (igt_list_empty(view)) {
> printf("No GPU devices found\n");
> - return;
> + return true;
> }
>
> + return false;
> +}
> +
> +static void igt_devs_print_simple(struct igt_list_head *view)
> +{
> + struct igt_device *dev;
> +
> + if (__check_empty(view))
> + return;
> +
> igt_list_for_each_entry(dev, view, link) {
> printf("sys:%s\n", dev->syspath);
> if (dev->subsystem)
> @@ -728,6 +736,89 @@ static void igt_devs_print_simple(struct igt_list_head *view)
> }
> }
>
> +static struct igt_device *
> +__find_pci(struct igt_list_head *view, const char *drm)
> +{
> + struct igt_device *dev;
> +
> + igt_list_for_each_entry(dev, view, link) {
> + if (!is_pci_subsystem(dev) || !dev->drm_card)
> + continue;
> +
> + if (!strcmp(dev->drm_card, drm))
> + return dev;
> + }
> +
> + return NULL;
> +}
> +
> +static void igt_devs_print_user(struct igt_list_head *view)
> +{
> + struct igt_device *dev;
> +
> + if (__check_empty(view))
> + return;
> +
> + igt_list_for_each_entry(dev, view, link) {
> + unsigned int i, num_children;
> + struct igt_device *pci_dev;
> + struct igt_device *dev2;
> + char filter[64];
> + char *drm_name;
> + int ret;
> +
> + if (!is_drm_subsystem(dev))
> + continue;
> + if (!dev->drm_card || dev->drm_render)
> + continue;
> +
> + drm_name = rindex(dev->drm_card, '/');
> + if (!drm_name || !*++drm_name)
> + continue;
> +
> + ret = snprintf(filter, sizeof(filter), "drm:%s", dev->drm_card);
> + igt_assert(ret < sizeof(filter));
> +
> + pci_dev = __find_pci(view, dev->drm_card);
> + if (pci_dev)
> + printf("%-24s%4s:%4s %s\n",
> + drm_name, pci_dev->vendor, pci_dev->device,
> + filter);
> + else
> + printf("%-24s %s\n", drm_name, filter);
> +
> + num_children = 0;
> + igt_list_for_each_entry(dev2, view, link) {
> + if (!is_drm_subsystem(dev2) || !dev2->drm_render)
> + continue;
> + if (strcmp(dev2->parent->syspath, dev->parent->syspath))
> + continue;
> +
> + num_children++;
> + }
> +
> + i = 0;
> + igt_list_for_each_entry(dev2, view, link) {
> + if (!is_drm_subsystem(dev2) || !dev2->drm_render)
> + continue;
> + if (strcmp(dev2->parent->syspath, dev->parent->syspath))
> + continue;
> +
> + drm_name = rindex(dev2->drm_render, '/');
> + if (!drm_name || !*++drm_name)
> + continue;
> +
> + ret = snprintf(filter, sizeof(filter), "drm:%s",
> + dev2->drm_render);
> + igt_assert(ret < sizeof(filter));
> +
> + printf("%s%-22s %s\n",
> + (++i == num_children) ? "└─" : "├─",
> + drm_name, filter);
> + }
> + }
> +}
> +
> static inline void _print_key_value(const char* k, const char *v)
> {
> printf("%-32s: %s\n", k, v);
> @@ -752,14 +843,9 @@ static void igt_devs_print_detail(struct igt_list_head *view)
> {
> struct igt_device *dev;
>
> - if (!view)
> + if (__check_empty(view))
> return;
>
> - if (igt_list_empty(view)) {
> - printf("No GPU devices found\n");
> - return;
> - }
> -
> igt_list_for_each_entry(dev, view, link) {
> printf("========== %s:%s ==========\n",
> dev->subsystem, dev->syspath);
> @@ -781,6 +867,7 @@ static struct print_func {
> } print_functions[] = {
> [IGT_PRINT_SIMPLE] = { .prn = igt_devs_print_simple },
> [IGT_PRINT_DETAIL] = { .prn = igt_devs_print_detail },
> + [IGT_PRINT_USER] = { .prn = igt_devs_print_user },
> };
>
> /**
> diff --git a/lib/igt_device_scan.h b/lib/igt_device_scan.h
> index bd937d22752c..9e13ed9db406 100644
> --- a/lib/igt_device_scan.h
> +++ b/lib/igt_device_scan.h
> @@ -37,6 +37,7 @@
> enum igt_devices_print_type {
> IGT_PRINT_SIMPLE,
> IGT_PRINT_DETAIL,
> + IGT_PRINT_USER, /* End user friendly. */
> };
>
> #define INTEGRATED_I915_GPU_PCI_ID "0000:00:02.0"
> diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
> index 79a936ffbe1a..b984edc656c7 100644
> --- a/tools/intel_gpu_top.c
> +++ b/tools/intel_gpu_top.c
> @@ -1313,7 +1313,6 @@ int main(int argc, char **argv)
> unsigned int i;
> int ret = 0, ch;
> bool list_device = false;
> - enum igt_devices_print_type printtype = IGT_PRINT_SIMPLE;
> char *pmu_device, *opt_device = NULL;
> struct igt_device_card card;
>
> @@ -1388,7 +1387,7 @@ int main(int argc, char **argv)
> igt_devices_scan(false);
>
> if (list_device) {
> - igt_devices_print(printtype);
> + igt_devices_print(IGT_PRINT_USER);
> goto exit;
> }
>
> --
> 2.25.1
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [igt-dev] [RFC i-g-t] intel_gpu_top: User friendly device listing
@ 2020-10-15 4:36 ` Zbigniew Kempczyński
0 siblings, 0 replies; 14+ messages in thread
From: Zbigniew Kempczyński @ 2020-10-15 4:36 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: igt-dev, Intel-gfx, Petri Latvala, Tvrtko Ursulin
On Wed, Oct 14, 2020 at 11:48:53AM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> Adding a new device selection print type suitable for user-facing
> use cases like intel_gpu_top -L and potentially lsgpu.
>
> Instead of:
>
> sys:/sys/devices/pci0000:00/0000:00:02.0/drm/card0
> subsystem : drm
> drm card : /dev/dri/card0
> parent : sys:/sys/devices/pci0000:00/0000:00:02.0
>
> sys:/sys/devices/pci0000:00/0000:00:02.0/drm/renderD128
> subsystem : drm
> drm render : /dev/dri/renderD128
> parent : sys:/sys/devices/pci0000:00/0000:00:02.0
>
> sys:/sys/devices/pci0000:00/0000:00:02.0
> subsystem : pci
> drm card : /dev/dri/card0
> drm render : /dev/dri/renderD128
> vendor : 8086
> device : 193B
>
> New format looks like:
>
> card0 8086:193B drm:/dev/dri/card0
> └─renderD128 drm:/dev/dri/renderD128
>
> Advantages are more compact, more readable, one entry per GPU, shorter
> string to copy and paste to intel_gpu_top -d, or respective usage.
Looks nice and more intuitive.
1. Add -s switch to handle simple print (current default in lsgpu)
2. Change to user format as default.
--
Zbigniew
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Petri Latvala <petri.latvala@intel.com>
> Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> ---
> lib/igt_device_scan.c | 109 +++++++++++++++++++++++++++++++++++++-----
> lib/igt_device_scan.h | 1 +
> tools/intel_gpu_top.c | 3 +-
> 3 files changed, 100 insertions(+), 13 deletions(-)
>
> diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
> index f4d43c733314..ce0ea61dda50 100644
> --- a/lib/igt_device_scan.c
> +++ b/lib/igt_device_scan.c
> @@ -695,18 +695,26 @@ static inline void _pr_simple2(const char *k, const char *v1, const char *v2)
> printf(" %-16s: %s:%s\n", k, v1, v2);
> }
>
> -static void igt_devs_print_simple(struct igt_list_head *view)
> +static bool __check_empty(struct igt_list_head *view)
> {
> - struct igt_device *dev;
> -
> if (!view)
> - return;
> + return true;
>
> if (igt_list_empty(view)) {
> printf("No GPU devices found\n");
> - return;
> + return true;
> }
>
> + return false;
> +}
> +
> +static void igt_devs_print_simple(struct igt_list_head *view)
> +{
> + struct igt_device *dev;
> +
> + if (__check_empty(view))
> + return;
> +
> igt_list_for_each_entry(dev, view, link) {
> printf("sys:%s\n", dev->syspath);
> if (dev->subsystem)
> @@ -728,6 +736,89 @@ static void igt_devs_print_simple(struct igt_list_head *view)
> }
> }
>
> +static struct igt_device *
> +__find_pci(struct igt_list_head *view, const char *drm)
> +{
> + struct igt_device *dev;
> +
> + igt_list_for_each_entry(dev, view, link) {
> + if (!is_pci_subsystem(dev) || !dev->drm_card)
> + continue;
> +
> + if (!strcmp(dev->drm_card, drm))
> + return dev;
> + }
> +
> + return NULL;
> +}
> +
> +static void igt_devs_print_user(struct igt_list_head *view)
> +{
> + struct igt_device *dev;
> +
> + if (__check_empty(view))
> + return;
> +
> + igt_list_for_each_entry(dev, view, link) {
> + unsigned int i, num_children;
> + struct igt_device *pci_dev;
> + struct igt_device *dev2;
> + char filter[64];
> + char *drm_name;
> + int ret;
> +
> + if (!is_drm_subsystem(dev))
> + continue;
> + if (!dev->drm_card || dev->drm_render)
> + continue;
> +
> + drm_name = rindex(dev->drm_card, '/');
> + if (!drm_name || !*++drm_name)
> + continue;
> +
> + ret = snprintf(filter, sizeof(filter), "drm:%s", dev->drm_card);
> + igt_assert(ret < sizeof(filter));
> +
> + pci_dev = __find_pci(view, dev->drm_card);
> + if (pci_dev)
> + printf("%-24s%4s:%4s %s\n",
> + drm_name, pci_dev->vendor, pci_dev->device,
> + filter);
> + else
> + printf("%-24s %s\n", drm_name, filter);
> +
> + num_children = 0;
> + igt_list_for_each_entry(dev2, view, link) {
> + if (!is_drm_subsystem(dev2) || !dev2->drm_render)
> + continue;
> + if (strcmp(dev2->parent->syspath, dev->parent->syspath))
> + continue;
> +
> + num_children++;
> + }
> +
> + i = 0;
> + igt_list_for_each_entry(dev2, view, link) {
> + if (!is_drm_subsystem(dev2) || !dev2->drm_render)
> + continue;
> + if (strcmp(dev2->parent->syspath, dev->parent->syspath))
> + continue;
> +
> + drm_name = rindex(dev2->drm_render, '/');
> + if (!drm_name || !*++drm_name)
> + continue;
> +
> + ret = snprintf(filter, sizeof(filter), "drm:%s",
> + dev2->drm_render);
> + igt_assert(ret < sizeof(filter));
> +
> + printf("%s%-22s %s\n",
> + (++i == num_children) ? "└─" : "├─",
> + drm_name, filter);
> + }
> + }
> +}
> +
> static inline void _print_key_value(const char* k, const char *v)
> {
> printf("%-32s: %s\n", k, v);
> @@ -752,14 +843,9 @@ static void igt_devs_print_detail(struct igt_list_head *view)
> {
> struct igt_device *dev;
>
> - if (!view)
> + if (__check_empty(view))
> return;
>
> - if (igt_list_empty(view)) {
> - printf("No GPU devices found\n");
> - return;
> - }
> -
> igt_list_for_each_entry(dev, view, link) {
> printf("========== %s:%s ==========\n",
> dev->subsystem, dev->syspath);
> @@ -781,6 +867,7 @@ static struct print_func {
> } print_functions[] = {
> [IGT_PRINT_SIMPLE] = { .prn = igt_devs_print_simple },
> [IGT_PRINT_DETAIL] = { .prn = igt_devs_print_detail },
> + [IGT_PRINT_USER] = { .prn = igt_devs_print_user },
> };
>
> /**
> diff --git a/lib/igt_device_scan.h b/lib/igt_device_scan.h
> index bd937d22752c..9e13ed9db406 100644
> --- a/lib/igt_device_scan.h
> +++ b/lib/igt_device_scan.h
> @@ -37,6 +37,7 @@
> enum igt_devices_print_type {
> IGT_PRINT_SIMPLE,
> IGT_PRINT_DETAIL,
> + IGT_PRINT_USER, /* End user friendly. */
> };
>
> #define INTEGRATED_I915_GPU_PCI_ID "0000:00:02.0"
> diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
> index 79a936ffbe1a..b984edc656c7 100644
> --- a/tools/intel_gpu_top.c
> +++ b/tools/intel_gpu_top.c
> @@ -1313,7 +1313,6 @@ int main(int argc, char **argv)
> unsigned int i;
> int ret = 0, ch;
> bool list_device = false;
> - enum igt_devices_print_type printtype = IGT_PRINT_SIMPLE;
> char *pmu_device, *opt_device = NULL;
> struct igt_device_card card;
>
> @@ -1388,7 +1387,7 @@ int main(int argc, char **argv)
> igt_devices_scan(false);
>
> if (list_device) {
> - igt_devices_print(printtype);
> + igt_devices_print(IGT_PRINT_USER);
> goto exit;
> }
>
> --
> 2.25.1
>
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [RFC i-g-t] intel_gpu_top: User friendly device listing
2020-10-15 4:36 ` [igt-dev] " Zbigniew Kempczyński
(?)
@ 2020-10-15 8:09 ` Tvrtko Ursulin
2020-10-16 4:07 ` [igt-dev] " Zbigniew Kempczyński
-1 siblings, 1 reply; 14+ messages in thread
From: Tvrtko Ursulin @ 2020-10-15 8:09 UTC (permalink / raw)
To: Zbigniew Kempczyński; +Cc: igt-dev, Intel-gfx
On 15/10/2020 05:36, Zbigniew Kempczyński wrote:
> On Wed, Oct 14, 2020 at 11:48:53AM +0100, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Adding a new device selection print type suitable for user-facing
>> use cases like intel_gpu_top -L and potentially lsgpu.
>>
>> Instead of:
>>
>> sys:/sys/devices/pci0000:00/0000:00:02.0/drm/card0
>> subsystem : drm
>> drm card : /dev/dri/card0
>> parent : sys:/sys/devices/pci0000:00/0000:00:02.0
>>
>> sys:/sys/devices/pci0000:00/0000:00:02.0/drm/renderD128
>> subsystem : drm
>> drm render : /dev/dri/renderD128
>> parent : sys:/sys/devices/pci0000:00/0000:00:02.0
>>
>> sys:/sys/devices/pci0000:00/0000:00:02.0
>> subsystem : pci
>> drm card : /dev/dri/card0
>> drm render : /dev/dri/renderD128
>> vendor : 8086
>> device : 193B
>>
>> New format looks like:
>>
>> card0 8086:193B drm:/dev/dri/card0
>> └─renderD128 drm:/dev/dri/renderD128
>>
>> Advantages are more compact, more readable, one entry per GPU, shorter
>> string to copy and paste to intel_gpu_top -d, or respective usage.
>
> Looks nice and more intuitive.
I wasn't sure about duplication of card/render name and again in filter
string, but I have no better ideas at the moment.
Maybe one day some could replace the left column with some card names
from some database but for now above is good enough.
> 1. Add -s switch to handle simple print (current default in lsgpu)
I wanted to suggest -v (verbose) or -d (details) but both are taken in
lsgpu. -s sounds like silent or in any case not obvious what "simple"
means. Could use just long form --verbose or --details?
> 2. Change to user format as default.
Yep.
Regards,
Tvrtko
>
> --
> Zbigniew
>
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> Cc: Petri Latvala <petri.latvala@intel.com>
>> Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
>> ---
>> lib/igt_device_scan.c | 109 +++++++++++++++++++++++++++++++++++++-----
>> lib/igt_device_scan.h | 1 +
>> tools/intel_gpu_top.c | 3 +-
>> 3 files changed, 100 insertions(+), 13 deletions(-)
>>
>> diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
>> index f4d43c733314..ce0ea61dda50 100644
>> --- a/lib/igt_device_scan.c
>> +++ b/lib/igt_device_scan.c
>> @@ -695,18 +695,26 @@ static inline void _pr_simple2(const char *k, const char *v1, const char *v2)
>> printf(" %-16s: %s:%s\n", k, v1, v2);
>> }
>>
>> -static void igt_devs_print_simple(struct igt_list_head *view)
>> +static bool __check_empty(struct igt_list_head *view)
>> {
>> - struct igt_device *dev;
>> -
>> if (!view)
>> - return;
>> + return true;
>>
>> if (igt_list_empty(view)) {
>> printf("No GPU devices found\n");
>> - return;
>> + return true;
>> }
>>
>> + return false;
>> +}
>> +
>> +static void igt_devs_print_simple(struct igt_list_head *view)
>> +{
>> + struct igt_device *dev;
>> +
>> + if (__check_empty(view))
>> + return;
>> +
>> igt_list_for_each_entry(dev, view, link) {
>> printf("sys:%s\n", dev->syspath);
>> if (dev->subsystem)
>> @@ -728,6 +736,89 @@ static void igt_devs_print_simple(struct igt_list_head *view)
>> }
>> }
>>
>> +static struct igt_device *
>> +__find_pci(struct igt_list_head *view, const char *drm)
>> +{
>> + struct igt_device *dev;
>> +
>> + igt_list_for_each_entry(dev, view, link) {
>> + if (!is_pci_subsystem(dev) || !dev->drm_card)
>> + continue;
>> +
>> + if (!strcmp(dev->drm_card, drm))
>> + return dev;
>> + }
>> +
>> + return NULL;
>> +}
>> +
>> +static void igt_devs_print_user(struct igt_list_head *view)
>> +{
>> + struct igt_device *dev;
>> +
>> + if (__check_empty(view))
>> + return;
>> +
>> + igt_list_for_each_entry(dev, view, link) {
>> + unsigned int i, num_children;
>> + struct igt_device *pci_dev;
>> + struct igt_device *dev2;
>> + char filter[64];
>> + char *drm_name;
>> + int ret;
>> +
>> + if (!is_drm_subsystem(dev))
>> + continue;
>> + if (!dev->drm_card || dev->drm_render)
>> + continue;
>> +
>> + drm_name = rindex(dev->drm_card, '/');
>> + if (!drm_name || !*++drm_name)
>> + continue;
>> +
>> + ret = snprintf(filter, sizeof(filter), "drm:%s", dev->drm_card);
>> + igt_assert(ret < sizeof(filter));
>> +
>> + pci_dev = __find_pci(view, dev->drm_card);
>> + if (pci_dev)
>> + printf("%-24s%4s:%4s %s\n",
>> + drm_name, pci_dev->vendor, pci_dev->device,
>> + filter);
>> + else
>> + printf("%-24s %s\n", drm_name, filter);
>> +
>> + num_children = 0;
>> + igt_list_for_each_entry(dev2, view, link) {
>> + if (!is_drm_subsystem(dev2) || !dev2->drm_render)
>> + continue;
>> + if (strcmp(dev2->parent->syspath, dev->parent->syspath))
>> + continue;
>> +
>> + num_children++;
>> + }
>> +
>> + i = 0;
>> + igt_list_for_each_entry(dev2, view, link) {
>> + if (!is_drm_subsystem(dev2) || !dev2->drm_render)
>> + continue;
>> + if (strcmp(dev2->parent->syspath, dev->parent->syspath))
>> + continue;
>> +
>> + drm_name = rindex(dev2->drm_render, '/');
>> + if (!drm_name || !*++drm_name)
>> + continue;
>> +
>> + ret = snprintf(filter, sizeof(filter), "drm:%s",
>> + dev2->drm_render);
>> + igt_assert(ret < sizeof(filter));
>> +
>> + printf("%s%-22s %s\n",
>> + (++i == num_children) ? "└─" : "├─",
>> + drm_name, filter);
>> + }
>> + }
>> +}
>> +
>> static inline void _print_key_value(const char* k, const char *v)
>> {
>> printf("%-32s: %s\n", k, v);
>> @@ -752,14 +843,9 @@ static void igt_devs_print_detail(struct igt_list_head *view)
>> {
>> struct igt_device *dev;
>>
>> - if (!view)
>> + if (__check_empty(view))
>> return;
>>
>> - if (igt_list_empty(view)) {
>> - printf("No GPU devices found\n");
>> - return;
>> - }
>> -
>> igt_list_for_each_entry(dev, view, link) {
>> printf("========== %s:%s ==========\n",
>> dev->subsystem, dev->syspath);
>> @@ -781,6 +867,7 @@ static struct print_func {
>> } print_functions[] = {
>> [IGT_PRINT_SIMPLE] = { .prn = igt_devs_print_simple },
>> [IGT_PRINT_DETAIL] = { .prn = igt_devs_print_detail },
>> + [IGT_PRINT_USER] = { .prn = igt_devs_print_user },
>> };
>>
>> /**
>> diff --git a/lib/igt_device_scan.h b/lib/igt_device_scan.h
>> index bd937d22752c..9e13ed9db406 100644
>> --- a/lib/igt_device_scan.h
>> +++ b/lib/igt_device_scan.h
>> @@ -37,6 +37,7 @@
>> enum igt_devices_print_type {
>> IGT_PRINT_SIMPLE,
>> IGT_PRINT_DETAIL,
>> + IGT_PRINT_USER, /* End user friendly. */
>> };
>>
>> #define INTEGRATED_I915_GPU_PCI_ID "0000:00:02.0"
>> diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
>> index 79a936ffbe1a..b984edc656c7 100644
>> --- a/tools/intel_gpu_top.c
>> +++ b/tools/intel_gpu_top.c
>> @@ -1313,7 +1313,6 @@ int main(int argc, char **argv)
>> unsigned int i;
>> int ret = 0, ch;
>> bool list_device = false;
>> - enum igt_devices_print_type printtype = IGT_PRINT_SIMPLE;
>> char *pmu_device, *opt_device = NULL;
>> struct igt_device_card card;
>>
>> @@ -1388,7 +1387,7 @@ int main(int argc, char **argv)
>> igt_devices_scan(false);
>>
>> if (list_device) {
>> - igt_devices_print(printtype);
>> + igt_devices_print(IGT_PRINT_USER);
>> goto exit;
>> }
>>
>> --
>> 2.25.1
>>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [RFC i-g-t] intel_gpu_top: User friendly device listing
2020-10-15 8:09 ` [Intel-gfx] " Tvrtko Ursulin
@ 2020-10-16 4:07 ` Zbigniew Kempczyński
0 siblings, 0 replies; 14+ messages in thread
From: Zbigniew Kempczyński @ 2020-10-16 4:07 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: igt-dev, Intel-gfx
On Thu, Oct 15, 2020 at 09:09:02AM +0100, Tvrtko Ursulin wrote:
>
> On 15/10/2020 05:36, Zbigniew Kempczyński wrote:
> > On Wed, Oct 14, 2020 at 11:48:53AM +0100, Tvrtko Ursulin wrote:
> > > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > >
> > > Adding a new device selection print type suitable for user-facing
> > > use cases like intel_gpu_top -L and potentially lsgpu.
> > >
> > > Instead of:
> > >
> > > sys:/sys/devices/pci0000:00/0000:00:02.0/drm/card0
> > > subsystem : drm
> > > drm card : /dev/dri/card0
> > > parent : sys:/sys/devices/pci0000:00/0000:00:02.0
> > >
> > > sys:/sys/devices/pci0000:00/0000:00:02.0/drm/renderD128
> > > subsystem : drm
> > > drm render : /dev/dri/renderD128
> > > parent : sys:/sys/devices/pci0000:00/0000:00:02.0
> > >
> > > sys:/sys/devices/pci0000:00/0000:00:02.0
> > > subsystem : pci
> > > drm card : /dev/dri/card0
> > > drm render : /dev/dri/renderD128
> > > vendor : 8086
> > > device : 193B
> > >
> > > New format looks like:
> > >
> > > card0 8086:193B drm:/dev/dri/card0
> > > └─renderD128 drm:/dev/dri/renderD128
> > >
> > > Advantages are more compact, more readable, one entry per GPU, shorter
> > > string to copy and paste to intel_gpu_top -d, or respective usage.
> >
> > Looks nice and more intuitive.
>
> I wasn't sure about duplication of card/render name and again in filter
> string, but I have no better ideas at the moment.
>
> Maybe one day some could replace the left column with some card names from
> some database but for now above is good enough.
>
> > 1. Add -s switch to handle simple print (current default in lsgpu)
>
> I wanted to suggest -v (verbose) or -d (details) but both are taken in
> lsgpu. -s sounds like silent or in any case not obvious what "simple" means.
> Could use just long form --verbose or --details?
-v is vendor list we support in pci vendor ids, not verbose. If you have
resistence to use -s just use -i (simple). Sometimes I need to go to
device directory in /sys so I use lsgpu simple output with directory name.
--
Zbigniew
>
> > 2. Change to user format as default.
>
> Yep.
>
> Regards,
>
> Tvrtko
>
> >
> > --
> > Zbigniew
> >
> > >
> > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > > Cc: Petri Latvala <petri.latvala@intel.com>
> > > Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> > > ---
> > > lib/igt_device_scan.c | 109 +++++++++++++++++++++++++++++++++++++-----
> > > lib/igt_device_scan.h | 1 +
> > > tools/intel_gpu_top.c | 3 +-
> > > 3 files changed, 100 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
> > > index f4d43c733314..ce0ea61dda50 100644
> > > --- a/lib/igt_device_scan.c
> > > +++ b/lib/igt_device_scan.c
> > > @@ -695,18 +695,26 @@ static inline void _pr_simple2(const char *k, const char *v1, const char *v2)
> > > printf(" %-16s: %s:%s\n", k, v1, v2);
> > > }
> > > -static void igt_devs_print_simple(struct igt_list_head *view)
> > > +static bool __check_empty(struct igt_list_head *view)
> > > {
> > > - struct igt_device *dev;
> > > -
> > > if (!view)
> > > - return;
> > > + return true;
> > > if (igt_list_empty(view)) {
> > > printf("No GPU devices found\n");
> > > - return;
> > > + return true;
> > > }
> > > + return false;
> > > +}
> > > +
> > > +static void igt_devs_print_simple(struct igt_list_head *view)
> > > +{
> > > + struct igt_device *dev;
> > > +
> > > + if (__check_empty(view))
> > > + return;
> > > +
> > > igt_list_for_each_entry(dev, view, link) {
> > > printf("sys:%s\n", dev->syspath);
> > > if (dev->subsystem)
> > > @@ -728,6 +736,89 @@ static void igt_devs_print_simple(struct igt_list_head *view)
> > > }
> > > }
> > > +static struct igt_device *
> > > +__find_pci(struct igt_list_head *view, const char *drm)
> > > +{
> > > + struct igt_device *dev;
> > > +
> > > + igt_list_for_each_entry(dev, view, link) {
> > > + if (!is_pci_subsystem(dev) || !dev->drm_card)
> > > + continue;
> > > +
> > > + if (!strcmp(dev->drm_card, drm))
> > > + return dev;
> > > + }
> > > +
> > > + return NULL;
> > > +}
> > > +
> > > +static void igt_devs_print_user(struct igt_list_head *view)
> > > +{
> > > + struct igt_device *dev;
> > > +
> > > + if (__check_empty(view))
> > > + return;
> > > +
> > > + igt_list_for_each_entry(dev, view, link) {
> > > + unsigned int i, num_children;
> > > + struct igt_device *pci_dev;
> > > + struct igt_device *dev2;
> > > + char filter[64];
> > > + char *drm_name;
> > > + int ret;
> > > +
> > > + if (!is_drm_subsystem(dev))
> > > + continue;
> > > + if (!dev->drm_card || dev->drm_render)
> > > + continue;
> > > +
> > > + drm_name = rindex(dev->drm_card, '/');
> > > + if (!drm_name || !*++drm_name)
> > > + continue;
> > > +
> > > + ret = snprintf(filter, sizeof(filter), "drm:%s", dev->drm_card);
> > > + igt_assert(ret < sizeof(filter));
> > > +
> > > + pci_dev = __find_pci(view, dev->drm_card);
> > > + if (pci_dev)
> > > + printf("%-24s%4s:%4s %s\n",
> > > + drm_name, pci_dev->vendor, pci_dev->device,
> > > + filter);
> > > + else
> > > + printf("%-24s %s\n", drm_name, filter);
> > > +
> > > + num_children = 0;
> > > + igt_list_for_each_entry(dev2, view, link) {
> > > + if (!is_drm_subsystem(dev2) || !dev2->drm_render)
> > > + continue;
> > > + if (strcmp(dev2->parent->syspath, dev->parent->syspath))
> > > + continue;
> > > +
> > > + num_children++;
> > > + }
> > > +
> > > + i = 0;
> > > + igt_list_for_each_entry(dev2, view, link) {
> > > + if (!is_drm_subsystem(dev2) || !dev2->drm_render)
> > > + continue;
> > > + if (strcmp(dev2->parent->syspath, dev->parent->syspath))
> > > + continue;
> > > +
> > > + drm_name = rindex(dev2->drm_render, '/');
> > > + if (!drm_name || !*++drm_name)
> > > + continue;
> > > +
> > > + ret = snprintf(filter, sizeof(filter), "drm:%s",
> > > + dev2->drm_render);
> > > + igt_assert(ret < sizeof(filter));
> > > +
> > > + printf("%s%-22s %s\n",
> > > + (++i == num_children) ? "└─" : "├─",
> > > + drm_name, filter);
> > > + }
> > > + }
> > > +}
> > > +
> > > static inline void _print_key_value(const char* k, const char *v)
> > > {
> > > printf("%-32s: %s\n", k, v);
> > > @@ -752,14 +843,9 @@ static void igt_devs_print_detail(struct igt_list_head *view)
> > > {
> > > struct igt_device *dev;
> > > - if (!view)
> > > + if (__check_empty(view))
> > > return;
> > > - if (igt_list_empty(view)) {
> > > - printf("No GPU devices found\n");
> > > - return;
> > > - }
> > > -
> > > igt_list_for_each_entry(dev, view, link) {
> > > printf("========== %s:%s ==========\n",
> > > dev->subsystem, dev->syspath);
> > > @@ -781,6 +867,7 @@ static struct print_func {
> > > } print_functions[] = {
> > > [IGT_PRINT_SIMPLE] = { .prn = igt_devs_print_simple },
> > > [IGT_PRINT_DETAIL] = { .prn = igt_devs_print_detail },
> > > + [IGT_PRINT_USER] = { .prn = igt_devs_print_user },
> > > };
> > > /**
> > > diff --git a/lib/igt_device_scan.h b/lib/igt_device_scan.h
> > > index bd937d22752c..9e13ed9db406 100644
> > > --- a/lib/igt_device_scan.h
> > > +++ b/lib/igt_device_scan.h
> > > @@ -37,6 +37,7 @@
> > > enum igt_devices_print_type {
> > > IGT_PRINT_SIMPLE,
> > > IGT_PRINT_DETAIL,
> > > + IGT_PRINT_USER, /* End user friendly. */
> > > };
> > > #define INTEGRATED_I915_GPU_PCI_ID "0000:00:02.0"
> > > diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
> > > index 79a936ffbe1a..b984edc656c7 100644
> > > --- a/tools/intel_gpu_top.c
> > > +++ b/tools/intel_gpu_top.c
> > > @@ -1313,7 +1313,6 @@ int main(int argc, char **argv)
> > > unsigned int i;
> > > int ret = 0, ch;
> > > bool list_device = false;
> > > - enum igt_devices_print_type printtype = IGT_PRINT_SIMPLE;
> > > char *pmu_device, *opt_device = NULL;
> > > struct igt_device_card card;
> > > @@ -1388,7 +1387,7 @@ int main(int argc, char **argv)
> > > igt_devices_scan(false);
> > > if (list_device) {
> > > - igt_devices_print(printtype);
> > > + igt_devices_print(IGT_PRINT_USER);
> > > goto exit;
> > > }
> > > --
> > > 2.25.1
> > >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [igt-dev] [RFC i-g-t] intel_gpu_top: User friendly device listing
@ 2020-10-16 4:07 ` Zbigniew Kempczyński
0 siblings, 0 replies; 14+ messages in thread
From: Zbigniew Kempczyński @ 2020-10-16 4:07 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: igt-dev, Intel-gfx, Petri Latvala, Tvrtko Ursulin
On Thu, Oct 15, 2020 at 09:09:02AM +0100, Tvrtko Ursulin wrote:
>
> On 15/10/2020 05:36, Zbigniew Kempczyński wrote:
> > On Wed, Oct 14, 2020 at 11:48:53AM +0100, Tvrtko Ursulin wrote:
> > > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > >
> > > Adding a new device selection print type suitable for user-facing
> > > use cases like intel_gpu_top -L and potentially lsgpu.
> > >
> > > Instead of:
> > >
> > > sys:/sys/devices/pci0000:00/0000:00:02.0/drm/card0
> > > subsystem : drm
> > > drm card : /dev/dri/card0
> > > parent : sys:/sys/devices/pci0000:00/0000:00:02.0
> > >
> > > sys:/sys/devices/pci0000:00/0000:00:02.0/drm/renderD128
> > > subsystem : drm
> > > drm render : /dev/dri/renderD128
> > > parent : sys:/sys/devices/pci0000:00/0000:00:02.0
> > >
> > > sys:/sys/devices/pci0000:00/0000:00:02.0
> > > subsystem : pci
> > > drm card : /dev/dri/card0
> > > drm render : /dev/dri/renderD128
> > > vendor : 8086
> > > device : 193B
> > >
> > > New format looks like:
> > >
> > > card0 8086:193B drm:/dev/dri/card0
> > > └─renderD128 drm:/dev/dri/renderD128
> > >
> > > Advantages are more compact, more readable, one entry per GPU, shorter
> > > string to copy and paste to intel_gpu_top -d, or respective usage.
> >
> > Looks nice and more intuitive.
>
> I wasn't sure about duplication of card/render name and again in filter
> string, but I have no better ideas at the moment.
>
> Maybe one day some could replace the left column with some card names from
> some database but for now above is good enough.
>
> > 1. Add -s switch to handle simple print (current default in lsgpu)
>
> I wanted to suggest -v (verbose) or -d (details) but both are taken in
> lsgpu. -s sounds like silent or in any case not obvious what "simple" means.
> Could use just long form --verbose or --details?
-v is vendor list we support in pci vendor ids, not verbose. If you have
resistence to use -s just use -i (simple). Sometimes I need to go to
device directory in /sys so I use lsgpu simple output with directory name.
--
Zbigniew
>
> > 2. Change to user format as default.
>
> Yep.
>
> Regards,
>
> Tvrtko
>
> >
> > --
> > Zbigniew
> >
> > >
> > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > > Cc: Petri Latvala <petri.latvala@intel.com>
> > > Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> > > ---
> > > lib/igt_device_scan.c | 109 +++++++++++++++++++++++++++++++++++++-----
> > > lib/igt_device_scan.h | 1 +
> > > tools/intel_gpu_top.c | 3 +-
> > > 3 files changed, 100 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
> > > index f4d43c733314..ce0ea61dda50 100644
> > > --- a/lib/igt_device_scan.c
> > > +++ b/lib/igt_device_scan.c
> > > @@ -695,18 +695,26 @@ static inline void _pr_simple2(const char *k, const char *v1, const char *v2)
> > > printf(" %-16s: %s:%s\n", k, v1, v2);
> > > }
> > > -static void igt_devs_print_simple(struct igt_list_head *view)
> > > +static bool __check_empty(struct igt_list_head *view)
> > > {
> > > - struct igt_device *dev;
> > > -
> > > if (!view)
> > > - return;
> > > + return true;
> > > if (igt_list_empty(view)) {
> > > printf("No GPU devices found\n");
> > > - return;
> > > + return true;
> > > }
> > > + return false;
> > > +}
> > > +
> > > +static void igt_devs_print_simple(struct igt_list_head *view)
> > > +{
> > > + struct igt_device *dev;
> > > +
> > > + if (__check_empty(view))
> > > + return;
> > > +
> > > igt_list_for_each_entry(dev, view, link) {
> > > printf("sys:%s\n", dev->syspath);
> > > if (dev->subsystem)
> > > @@ -728,6 +736,89 @@ static void igt_devs_print_simple(struct igt_list_head *view)
> > > }
> > > }
> > > +static struct igt_device *
> > > +__find_pci(struct igt_list_head *view, const char *drm)
> > > +{
> > > + struct igt_device *dev;
> > > +
> > > + igt_list_for_each_entry(dev, view, link) {
> > > + if (!is_pci_subsystem(dev) || !dev->drm_card)
> > > + continue;
> > > +
> > > + if (!strcmp(dev->drm_card, drm))
> > > + return dev;
> > > + }
> > > +
> > > + return NULL;
> > > +}
> > > +
> > > +static void igt_devs_print_user(struct igt_list_head *view)
> > > +{
> > > + struct igt_device *dev;
> > > +
> > > + if (__check_empty(view))
> > > + return;
> > > +
> > > + igt_list_for_each_entry(dev, view, link) {
> > > + unsigned int i, num_children;
> > > + struct igt_device *pci_dev;
> > > + struct igt_device *dev2;
> > > + char filter[64];
> > > + char *drm_name;
> > > + int ret;
> > > +
> > > + if (!is_drm_subsystem(dev))
> > > + continue;
> > > + if (!dev->drm_card || dev->drm_render)
> > > + continue;
> > > +
> > > + drm_name = rindex(dev->drm_card, '/');
> > > + if (!drm_name || !*++drm_name)
> > > + continue;
> > > +
> > > + ret = snprintf(filter, sizeof(filter), "drm:%s", dev->drm_card);
> > > + igt_assert(ret < sizeof(filter));
> > > +
> > > + pci_dev = __find_pci(view, dev->drm_card);
> > > + if (pci_dev)
> > > + printf("%-24s%4s:%4s %s\n",
> > > + drm_name, pci_dev->vendor, pci_dev->device,
> > > + filter);
> > > + else
> > > + printf("%-24s %s\n", drm_name, filter);
> > > +
> > > + num_children = 0;
> > > + igt_list_for_each_entry(dev2, view, link) {
> > > + if (!is_drm_subsystem(dev2) || !dev2->drm_render)
> > > + continue;
> > > + if (strcmp(dev2->parent->syspath, dev->parent->syspath))
> > > + continue;
> > > +
> > > + num_children++;
> > > + }
> > > +
> > > + i = 0;
> > > + igt_list_for_each_entry(dev2, view, link) {
> > > + if (!is_drm_subsystem(dev2) || !dev2->drm_render)
> > > + continue;
> > > + if (strcmp(dev2->parent->syspath, dev->parent->syspath))
> > > + continue;
> > > +
> > > + drm_name = rindex(dev2->drm_render, '/');
> > > + if (!drm_name || !*++drm_name)
> > > + continue;
> > > +
> > > + ret = snprintf(filter, sizeof(filter), "drm:%s",
> > > + dev2->drm_render);
> > > + igt_assert(ret < sizeof(filter));
> > > +
> > > + printf("%s%-22s %s\n",
> > > + (++i == num_children) ? "└─" : "├─",
> > > + drm_name, filter);
> > > + }
> > > + }
> > > +}
> > > +
> > > static inline void _print_key_value(const char* k, const char *v)
> > > {
> > > printf("%-32s: %s\n", k, v);
> > > @@ -752,14 +843,9 @@ static void igt_devs_print_detail(struct igt_list_head *view)
> > > {
> > > struct igt_device *dev;
> > > - if (!view)
> > > + if (__check_empty(view))
> > > return;
> > > - if (igt_list_empty(view)) {
> > > - printf("No GPU devices found\n");
> > > - return;
> > > - }
> > > -
> > > igt_list_for_each_entry(dev, view, link) {
> > > printf("========== %s:%s ==========\n",
> > > dev->subsystem, dev->syspath);
> > > @@ -781,6 +867,7 @@ static struct print_func {
> > > } print_functions[] = {
> > > [IGT_PRINT_SIMPLE] = { .prn = igt_devs_print_simple },
> > > [IGT_PRINT_DETAIL] = { .prn = igt_devs_print_detail },
> > > + [IGT_PRINT_USER] = { .prn = igt_devs_print_user },
> > > };
> > > /**
> > > diff --git a/lib/igt_device_scan.h b/lib/igt_device_scan.h
> > > index bd937d22752c..9e13ed9db406 100644
> > > --- a/lib/igt_device_scan.h
> > > +++ b/lib/igt_device_scan.h
> > > @@ -37,6 +37,7 @@
> > > enum igt_devices_print_type {
> > > IGT_PRINT_SIMPLE,
> > > IGT_PRINT_DETAIL,
> > > + IGT_PRINT_USER, /* End user friendly. */
> > > };
> > > #define INTEGRATED_I915_GPU_PCI_ID "0000:00:02.0"
> > > diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
> > > index 79a936ffbe1a..b984edc656c7 100644
> > > --- a/tools/intel_gpu_top.c
> > > +++ b/tools/intel_gpu_top.c
> > > @@ -1313,7 +1313,6 @@ int main(int argc, char **argv)
> > > unsigned int i;
> > > int ret = 0, ch;
> > > bool list_device = false;
> > > - enum igt_devices_print_type printtype = IGT_PRINT_SIMPLE;
> > > char *pmu_device, *opt_device = NULL;
> > > struct igt_device_card card;
> > > @@ -1388,7 +1387,7 @@ int main(int argc, char **argv)
> > > igt_devices_scan(false);
> > > if (list_device) {
> > > - igt_devices_print(printtype);
> > > + igt_devices_print(IGT_PRINT_USER);
> > > goto exit;
> > > }
> > > --
> > > 2.25.1
> > >
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [RFC i-g-t] intel_gpu_top: User friendly device listing
2020-10-16 4:07 ` [igt-dev] " Zbigniew Kempczyński
@ 2020-10-16 9:11 ` Tvrtko Ursulin
-1 siblings, 0 replies; 14+ messages in thread
From: Tvrtko Ursulin @ 2020-10-16 9:11 UTC (permalink / raw)
To: Zbigniew Kempczyński; +Cc: igt-dev, Intel-gfx
On 16/10/2020 05:07, Zbigniew Kempczyński wrote:
> On Thu, Oct 15, 2020 at 09:09:02AM +0100, Tvrtko Ursulin wrote:
>>
>> On 15/10/2020 05:36, Zbigniew Kempczyński wrote:
>>> On Wed, Oct 14, 2020 at 11:48:53AM +0100, Tvrtko Ursulin wrote:
>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>
>>>> Adding a new device selection print type suitable for user-facing
>>>> use cases like intel_gpu_top -L and potentially lsgpu.
>>>>
>>>> Instead of:
>>>>
>>>> sys:/sys/devices/pci0000:00/0000:00:02.0/drm/card0
>>>> subsystem : drm
>>>> drm card : /dev/dri/card0
>>>> parent : sys:/sys/devices/pci0000:00/0000:00:02.0
>>>>
>>>> sys:/sys/devices/pci0000:00/0000:00:02.0/drm/renderD128
>>>> subsystem : drm
>>>> drm render : /dev/dri/renderD128
>>>> parent : sys:/sys/devices/pci0000:00/0000:00:02.0
>>>>
>>>> sys:/sys/devices/pci0000:00/0000:00:02.0
>>>> subsystem : pci
>>>> drm card : /dev/dri/card0
>>>> drm render : /dev/dri/renderD128
>>>> vendor : 8086
>>>> device : 193B
>>>>
>>>> New format looks like:
>>>>
>>>> card0 8086:193B drm:/dev/dri/card0
>>>> └─renderD128 drm:/dev/dri/renderD128
>>>>
>>>> Advantages are more compact, more readable, one entry per GPU, shorter
>>>> string to copy and paste to intel_gpu_top -d, or respective usage.
>>>
>>> Looks nice and more intuitive.
>>
>> I wasn't sure about duplication of card/render name and again in filter
>> string, but I have no better ideas at the moment.
>>
>> Maybe one day some could replace the left column with some card names from
>> some database but for now above is good enough.
>>
>>> 1. Add -s switch to handle simple print (current default in lsgpu)
>>
>> I wanted to suggest -v (verbose) or -d (details) but both are taken in
>> lsgpu. -s sounds like silent or in any case not obvious what "simple" means.
>> Could use just long form --verbose or --details?
>
> -v is vendor list we support in pci vendor ids, not verbose. If you have
> resistence to use -s just use -i (simple). Sometimes I need to go to
> device directory in /sys so I use lsgpu simple output with directory name.
I don't think "simple" as a term should be user facing because it is more complex than the mode I am proposing. So cannot really be called simple in my mind.
To be clear I am working under the assumption intel_gpu_top and lsgpu will be getting more and more users and as such emphasis on making them less of a developer tools should be increased. (Former had an user bug submitted in Fedora, which was one of the triggers.)
I do agree sysfs path is an interesting addition, or alternate mode. Maybe user printout should have selectable modes of drm or sysfs?
$ lsgpu --drm --filter (filter adds the drm: prefix, can be default)
card0 8086:193B drm:/dev/dri/card0
└─renderD128 drm:/dev/dri/renderD128
$ lsgpu --sysfs
card0 8086:193B /sys/devices/pci0000:00/0000:00:02.0/drm/card0
└─renderD128 /sys/devices/pci0000:00/0000:00:02.0/drm/renderD128
$ lsgpu --device
card0 8086:193B /sys/devices/pci0000:00/0000:00:02.0
└─renderD128
$ lsgpu --verbose (or --details)
... the current "simple" output ...
None of my complications from above do not fit directly into the current print type enum. I'll play with it time permitting and send another RFC.
Regards,
Tvrtko
>
> --
> Zbigniew
>
>
>>
>>> 2. Change to user format as default.
>>
>> Yep.
>>
>> Regards,
>>
>> Tvrtko
>>
>>>
>>> --
>>> Zbigniew
>>>
>>>>
>>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>> Cc: Petri Latvala <petri.latvala@intel.com>
>>>> Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
>>>> ---
>>>> lib/igt_device_scan.c | 109 +++++++++++++++++++++++++++++++++++++-----
>>>> lib/igt_device_scan.h | 1 +
>>>> tools/intel_gpu_top.c | 3 +-
>>>> 3 files changed, 100 insertions(+), 13 deletions(-)
>>>>
>>>> diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
>>>> index f4d43c733314..ce0ea61dda50 100644
>>>> --- a/lib/igt_device_scan.c
>>>> +++ b/lib/igt_device_scan.c
>>>> @@ -695,18 +695,26 @@ static inline void _pr_simple2(const char *k, const char *v1, const char *v2)
>>>> printf(" %-16s: %s:%s\n", k, v1, v2);
>>>> }
>>>> -static void igt_devs_print_simple(struct igt_list_head *view)
>>>> +static bool __check_empty(struct igt_list_head *view)
>>>> {
>>>> - struct igt_device *dev;
>>>> -
>>>> if (!view)
>>>> - return;
>>>> + return true;
>>>> if (igt_list_empty(view)) {
>>>> printf("No GPU devices found\n");
>>>> - return;
>>>> + return true;
>>>> }
>>>> + return false;
>>>> +}
>>>> +
>>>> +static void igt_devs_print_simple(struct igt_list_head *view)
>>>> +{
>>>> + struct igt_device *dev;
>>>> +
>>>> + if (__check_empty(view))
>>>> + return;
>>>> +
>>>> igt_list_for_each_entry(dev, view, link) {
>>>> printf("sys:%s\n", dev->syspath);
>>>> if (dev->subsystem)
>>>> @@ -728,6 +736,89 @@ static void igt_devs_print_simple(struct igt_list_head *view)
>>>> }
>>>> }
>>>> +static struct igt_device *
>>>> +__find_pci(struct igt_list_head *view, const char *drm)
>>>> +{
>>>> + struct igt_device *dev;
>>>> +
>>>> + igt_list_for_each_entry(dev, view, link) {
>>>> + if (!is_pci_subsystem(dev) || !dev->drm_card)
>>>> + continue;
>>>> +
>>>> + if (!strcmp(dev->drm_card, drm))
>>>> + return dev;
>>>> + }
>>>> +
>>>> + return NULL;
>>>> +}
>>>> +
>>>> +static void igt_devs_print_user(struct igt_list_head *view)
>>>> +{
>>>> + struct igt_device *dev;
>>>> +
>>>> + if (__check_empty(view))
>>>> + return;
>>>> +
>>>> + igt_list_for_each_entry(dev, view, link) {
>>>> + unsigned int i, num_children;
>>>> + struct igt_device *pci_dev;
>>>> + struct igt_device *dev2;
>>>> + char filter[64];
>>>> + char *drm_name;
>>>> + int ret;
>>>> +
>>>> + if (!is_drm_subsystem(dev))
>>>> + continue;
>>>> + if (!dev->drm_card || dev->drm_render)
>>>> + continue;
>>>> +
>>>> + drm_name = rindex(dev->drm_card, '/');
>>>> + if (!drm_name || !*++drm_name)
>>>> + continue;
>>>> +
>>>> + ret = snprintf(filter, sizeof(filter), "drm:%s", dev->drm_card);
>>>> + igt_assert(ret < sizeof(filter));
>>>> +
>>>> + pci_dev = __find_pci(view, dev->drm_card);
>>>> + if (pci_dev)
>>>> + printf("%-24s%4s:%4s %s\n",
>>>> + drm_name, pci_dev->vendor, pci_dev->device,
>>>> + filter);
>>>> + else
>>>> + printf("%-24s %s\n", drm_name, filter);
>>>> +
>>>> + num_children = 0;
>>>> + igt_list_for_each_entry(dev2, view, link) {
>>>> + if (!is_drm_subsystem(dev2) || !dev2->drm_render)
>>>> + continue;
>>>> + if (strcmp(dev2->parent->syspath, dev->parent->syspath))
>>>> + continue;
>>>> +
>>>> + num_children++;
>>>> + }
>>>> +
>>>> + i = 0;
>>>> + igt_list_for_each_entry(dev2, view, link) {
>>>> + if (!is_drm_subsystem(dev2) || !dev2->drm_render)
>>>> + continue;
>>>> + if (strcmp(dev2->parent->syspath, dev->parent->syspath))
>>>> + continue;
>>>> +
>>>> + drm_name = rindex(dev2->drm_render, '/');
>>>> + if (!drm_name || !*++drm_name)
>>>> + continue;
>>>> +
>>>> + ret = snprintf(filter, sizeof(filter), "drm:%s",
>>>> + dev2->drm_render);
>>>> + igt_assert(ret < sizeof(filter));
>>>> +
>>>> + printf("%s%-22s %s\n",
>>>> + (++i == num_children) ? "└─" : "├─",
>>>> + drm_name, filter);
>>>> + }
>>>> + }
>>>> +}
>>>> +
>>>> static inline void _print_key_value(const char* k, const char *v)
>>>> {
>>>> printf("%-32s: %s\n", k, v);
>>>> @@ -752,14 +843,9 @@ static void igt_devs_print_detail(struct igt_list_head *view)
>>>> {
>>>> struct igt_device *dev;
>>>> - if (!view)
>>>> + if (__check_empty(view))
>>>> return;
>>>> - if (igt_list_empty(view)) {
>>>> - printf("No GPU devices found\n");
>>>> - return;
>>>> - }
>>>> -
>>>> igt_list_for_each_entry(dev, view, link) {
>>>> printf("========== %s:%s ==========\n",
>>>> dev->subsystem, dev->syspath);
>>>> @@ -781,6 +867,7 @@ static struct print_func {
>>>> } print_functions[] = {
>>>> [IGT_PRINT_SIMPLE] = { .prn = igt_devs_print_simple },
>>>> [IGT_PRINT_DETAIL] = { .prn = igt_devs_print_detail },
>>>> + [IGT_PRINT_USER] = { .prn = igt_devs_print_user },
>>>> };
>>>> /**
>>>> diff --git a/lib/igt_device_scan.h b/lib/igt_device_scan.h
>>>> index bd937d22752c..9e13ed9db406 100644
>>>> --- a/lib/igt_device_scan.h
>>>> +++ b/lib/igt_device_scan.h
>>>> @@ -37,6 +37,7 @@
>>>> enum igt_devices_print_type {
>>>> IGT_PRINT_SIMPLE,
>>>> IGT_PRINT_DETAIL,
>>>> + IGT_PRINT_USER, /* End user friendly. */
>>>> };
>>>> #define INTEGRATED_I915_GPU_PCI_ID "0000:00:02.0"
>>>> diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
>>>> index 79a936ffbe1a..b984edc656c7 100644
>>>> --- a/tools/intel_gpu_top.c
>>>> +++ b/tools/intel_gpu_top.c
>>>> @@ -1313,7 +1313,6 @@ int main(int argc, char **argv)
>>>> unsigned int i;
>>>> int ret = 0, ch;
>>>> bool list_device = false;
>>>> - enum igt_devices_print_type printtype = IGT_PRINT_SIMPLE;
>>>> char *pmu_device, *opt_device = NULL;
>>>> struct igt_device_card card;
>>>> @@ -1388,7 +1387,7 @@ int main(int argc, char **argv)
>>>> igt_devices_scan(false);
>>>> if (list_device) {
>>>> - igt_devices_print(printtype);
>>>> + igt_devices_print(IGT_PRINT_USER);
>>>> goto exit;
>>>> }
>>>> --
>>>> 2.25.1
>>>>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [igt-dev] [RFC i-g-t] intel_gpu_top: User friendly device listing
@ 2020-10-16 9:11 ` Tvrtko Ursulin
0 siblings, 0 replies; 14+ messages in thread
From: Tvrtko Ursulin @ 2020-10-16 9:11 UTC (permalink / raw)
To: Zbigniew Kempczyński
Cc: igt-dev, Intel-gfx, Petri Latvala, Tvrtko Ursulin
On 16/10/2020 05:07, Zbigniew Kempczyński wrote:
> On Thu, Oct 15, 2020 at 09:09:02AM +0100, Tvrtko Ursulin wrote:
>>
>> On 15/10/2020 05:36, Zbigniew Kempczyński wrote:
>>> On Wed, Oct 14, 2020 at 11:48:53AM +0100, Tvrtko Ursulin wrote:
>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>
>>>> Adding a new device selection print type suitable for user-facing
>>>> use cases like intel_gpu_top -L and potentially lsgpu.
>>>>
>>>> Instead of:
>>>>
>>>> sys:/sys/devices/pci0000:00/0000:00:02.0/drm/card0
>>>> subsystem : drm
>>>> drm card : /dev/dri/card0
>>>> parent : sys:/sys/devices/pci0000:00/0000:00:02.0
>>>>
>>>> sys:/sys/devices/pci0000:00/0000:00:02.0/drm/renderD128
>>>> subsystem : drm
>>>> drm render : /dev/dri/renderD128
>>>> parent : sys:/sys/devices/pci0000:00/0000:00:02.0
>>>>
>>>> sys:/sys/devices/pci0000:00/0000:00:02.0
>>>> subsystem : pci
>>>> drm card : /dev/dri/card0
>>>> drm render : /dev/dri/renderD128
>>>> vendor : 8086
>>>> device : 193B
>>>>
>>>> New format looks like:
>>>>
>>>> card0 8086:193B drm:/dev/dri/card0
>>>> └─renderD128 drm:/dev/dri/renderD128
>>>>
>>>> Advantages are more compact, more readable, one entry per GPU, shorter
>>>> string to copy and paste to intel_gpu_top -d, or respective usage.
>>>
>>> Looks nice and more intuitive.
>>
>> I wasn't sure about duplication of card/render name and again in filter
>> string, but I have no better ideas at the moment.
>>
>> Maybe one day some could replace the left column with some card names from
>> some database but for now above is good enough.
>>
>>> 1. Add -s switch to handle simple print (current default in lsgpu)
>>
>> I wanted to suggest -v (verbose) or -d (details) but both are taken in
>> lsgpu. -s sounds like silent or in any case not obvious what "simple" means.
>> Could use just long form --verbose or --details?
>
> -v is vendor list we support in pci vendor ids, not verbose. If you have
> resistence to use -s just use -i (simple). Sometimes I need to go to
> device directory in /sys so I use lsgpu simple output with directory name.
I don't think "simple" as a term should be user facing because it is more complex than the mode I am proposing. So cannot really be called simple in my mind.
To be clear I am working under the assumption intel_gpu_top and lsgpu will be getting more and more users and as such emphasis on making them less of a developer tools should be increased. (Former had an user bug submitted in Fedora, which was one of the triggers.)
I do agree sysfs path is an interesting addition, or alternate mode. Maybe user printout should have selectable modes of drm or sysfs?
$ lsgpu --drm --filter (filter adds the drm: prefix, can be default)
card0 8086:193B drm:/dev/dri/card0
└─renderD128 drm:/dev/dri/renderD128
$ lsgpu --sysfs
card0 8086:193B /sys/devices/pci0000:00/0000:00:02.0/drm/card0
└─renderD128 /sys/devices/pci0000:00/0000:00:02.0/drm/renderD128
$ lsgpu --device
card0 8086:193B /sys/devices/pci0000:00/0000:00:02.0
└─renderD128
$ lsgpu --verbose (or --details)
... the current "simple" output ...
None of my complications from above do not fit directly into the current print type enum. I'll play with it time permitting and send another RFC.
Regards,
Tvrtko
>
> --
> Zbigniew
>
>
>>
>>> 2. Change to user format as default.
>>
>> Yep.
>>
>> Regards,
>>
>> Tvrtko
>>
>>>
>>> --
>>> Zbigniew
>>>
>>>>
>>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>> Cc: Petri Latvala <petri.latvala@intel.com>
>>>> Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
>>>> ---
>>>> lib/igt_device_scan.c | 109 +++++++++++++++++++++++++++++++++++++-----
>>>> lib/igt_device_scan.h | 1 +
>>>> tools/intel_gpu_top.c | 3 +-
>>>> 3 files changed, 100 insertions(+), 13 deletions(-)
>>>>
>>>> diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
>>>> index f4d43c733314..ce0ea61dda50 100644
>>>> --- a/lib/igt_device_scan.c
>>>> +++ b/lib/igt_device_scan.c
>>>> @@ -695,18 +695,26 @@ static inline void _pr_simple2(const char *k, const char *v1, const char *v2)
>>>> printf(" %-16s: %s:%s\n", k, v1, v2);
>>>> }
>>>> -static void igt_devs_print_simple(struct igt_list_head *view)
>>>> +static bool __check_empty(struct igt_list_head *view)
>>>> {
>>>> - struct igt_device *dev;
>>>> -
>>>> if (!view)
>>>> - return;
>>>> + return true;
>>>> if (igt_list_empty(view)) {
>>>> printf("No GPU devices found\n");
>>>> - return;
>>>> + return true;
>>>> }
>>>> + return false;
>>>> +}
>>>> +
>>>> +static void igt_devs_print_simple(struct igt_list_head *view)
>>>> +{
>>>> + struct igt_device *dev;
>>>> +
>>>> + if (__check_empty(view))
>>>> + return;
>>>> +
>>>> igt_list_for_each_entry(dev, view, link) {
>>>> printf("sys:%s\n", dev->syspath);
>>>> if (dev->subsystem)
>>>> @@ -728,6 +736,89 @@ static void igt_devs_print_simple(struct igt_list_head *view)
>>>> }
>>>> }
>>>> +static struct igt_device *
>>>> +__find_pci(struct igt_list_head *view, const char *drm)
>>>> +{
>>>> + struct igt_device *dev;
>>>> +
>>>> + igt_list_for_each_entry(dev, view, link) {
>>>> + if (!is_pci_subsystem(dev) || !dev->drm_card)
>>>> + continue;
>>>> +
>>>> + if (!strcmp(dev->drm_card, drm))
>>>> + return dev;
>>>> + }
>>>> +
>>>> + return NULL;
>>>> +}
>>>> +
>>>> +static void igt_devs_print_user(struct igt_list_head *view)
>>>> +{
>>>> + struct igt_device *dev;
>>>> +
>>>> + if (__check_empty(view))
>>>> + return;
>>>> +
>>>> + igt_list_for_each_entry(dev, view, link) {
>>>> + unsigned int i, num_children;
>>>> + struct igt_device *pci_dev;
>>>> + struct igt_device *dev2;
>>>> + char filter[64];
>>>> + char *drm_name;
>>>> + int ret;
>>>> +
>>>> + if (!is_drm_subsystem(dev))
>>>> + continue;
>>>> + if (!dev->drm_card || dev->drm_render)
>>>> + continue;
>>>> +
>>>> + drm_name = rindex(dev->drm_card, '/');
>>>> + if (!drm_name || !*++drm_name)
>>>> + continue;
>>>> +
>>>> + ret = snprintf(filter, sizeof(filter), "drm:%s", dev->drm_card);
>>>> + igt_assert(ret < sizeof(filter));
>>>> +
>>>> + pci_dev = __find_pci(view, dev->drm_card);
>>>> + if (pci_dev)
>>>> + printf("%-24s%4s:%4s %s\n",
>>>> + drm_name, pci_dev->vendor, pci_dev->device,
>>>> + filter);
>>>> + else
>>>> + printf("%-24s %s\n", drm_name, filter);
>>>> +
>>>> + num_children = 0;
>>>> + igt_list_for_each_entry(dev2, view, link) {
>>>> + if (!is_drm_subsystem(dev2) || !dev2->drm_render)
>>>> + continue;
>>>> + if (strcmp(dev2->parent->syspath, dev->parent->syspath))
>>>> + continue;
>>>> +
>>>> + num_children++;
>>>> + }
>>>> +
>>>> + i = 0;
>>>> + igt_list_for_each_entry(dev2, view, link) {
>>>> + if (!is_drm_subsystem(dev2) || !dev2->drm_render)
>>>> + continue;
>>>> + if (strcmp(dev2->parent->syspath, dev->parent->syspath))
>>>> + continue;
>>>> +
>>>> + drm_name = rindex(dev2->drm_render, '/');
>>>> + if (!drm_name || !*++drm_name)
>>>> + continue;
>>>> +
>>>> + ret = snprintf(filter, sizeof(filter), "drm:%s",
>>>> + dev2->drm_render);
>>>> + igt_assert(ret < sizeof(filter));
>>>> +
>>>> + printf("%s%-22s %s\n",
>>>> + (++i == num_children) ? "└─" : "├─",
>>>> + drm_name, filter);
>>>> + }
>>>> + }
>>>> +}
>>>> +
>>>> static inline void _print_key_value(const char* k, const char *v)
>>>> {
>>>> printf("%-32s: %s\n", k, v);
>>>> @@ -752,14 +843,9 @@ static void igt_devs_print_detail(struct igt_list_head *view)
>>>> {
>>>> struct igt_device *dev;
>>>> - if (!view)
>>>> + if (__check_empty(view))
>>>> return;
>>>> - if (igt_list_empty(view)) {
>>>> - printf("No GPU devices found\n");
>>>> - return;
>>>> - }
>>>> -
>>>> igt_list_for_each_entry(dev, view, link) {
>>>> printf("========== %s:%s ==========\n",
>>>> dev->subsystem, dev->syspath);
>>>> @@ -781,6 +867,7 @@ static struct print_func {
>>>> } print_functions[] = {
>>>> [IGT_PRINT_SIMPLE] = { .prn = igt_devs_print_simple },
>>>> [IGT_PRINT_DETAIL] = { .prn = igt_devs_print_detail },
>>>> + [IGT_PRINT_USER] = { .prn = igt_devs_print_user },
>>>> };
>>>> /**
>>>> diff --git a/lib/igt_device_scan.h b/lib/igt_device_scan.h
>>>> index bd937d22752c..9e13ed9db406 100644
>>>> --- a/lib/igt_device_scan.h
>>>> +++ b/lib/igt_device_scan.h
>>>> @@ -37,6 +37,7 @@
>>>> enum igt_devices_print_type {
>>>> IGT_PRINT_SIMPLE,
>>>> IGT_PRINT_DETAIL,
>>>> + IGT_PRINT_USER, /* End user friendly. */
>>>> };
>>>> #define INTEGRATED_I915_GPU_PCI_ID "0000:00:02.0"
>>>> diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
>>>> index 79a936ffbe1a..b984edc656c7 100644
>>>> --- a/tools/intel_gpu_top.c
>>>> +++ b/tools/intel_gpu_top.c
>>>> @@ -1313,7 +1313,6 @@ int main(int argc, char **argv)
>>>> unsigned int i;
>>>> int ret = 0, ch;
>>>> bool list_device = false;
>>>> - enum igt_devices_print_type printtype = IGT_PRINT_SIMPLE;
>>>> char *pmu_device, *opt_device = NULL;
>>>> struct igt_device_card card;
>>>> @@ -1388,7 +1387,7 @@ int main(int argc, char **argv)
>>>> igt_devices_scan(false);
>>>> if (list_device) {
>>>> - igt_devices_print(printtype);
>>>> + igt_devices_print(IGT_PRINT_USER);
>>>> goto exit;
>>>> }
>>>> --
>>>> 2.25.1
>>>>
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [RFC i-g-t] intel_gpu_top: User friendly device listing
2020-10-16 9:11 ` [igt-dev] " Tvrtko Ursulin
@ 2020-10-16 11:48 ` Zbigniew Kempczyński
-1 siblings, 0 replies; 14+ messages in thread
From: Zbigniew Kempczyński @ 2020-10-16 11:48 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: igt-dev, Intel-gfx
On Fri, Oct 16, 2020 at 10:11:04AM +0100, Tvrtko Ursulin wrote:
>
> On 16/10/2020 05:07, Zbigniew Kempczyński wrote:
> > On Thu, Oct 15, 2020 at 09:09:02AM +0100, Tvrtko Ursulin wrote:
> >>
> >> On 15/10/2020 05:36, Zbigniew Kempczyński wrote:
> >>> On Wed, Oct 14, 2020 at 11:48:53AM +0100, Tvrtko Ursulin wrote:
> >>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>>>
> >>>> Adding a new device selection print type suitable for user-facing
> >>>> use cases like intel_gpu_top -L and potentially lsgpu.
> >>>>
> >>>> Instead of:
> >>>>
> >>>> sys:/sys/devices/pci0000:00/0000:00:02.0/drm/card0
> >>>> subsystem : drm
> >>>> drm card : /dev/dri/card0
> >>>> parent : sys:/sys/devices/pci0000:00/0000:00:02.0
> >>>>
> >>>> sys:/sys/devices/pci0000:00/0000:00:02.0/drm/renderD128
> >>>> subsystem : drm
> >>>> drm render : /dev/dri/renderD128
> >>>> parent : sys:/sys/devices/pci0000:00/0000:00:02.0
> >>>>
> >>>> sys:/sys/devices/pci0000:00/0000:00:02.0
> >>>> subsystem : pci
> >>>> drm card : /dev/dri/card0
> >>>> drm render : /dev/dri/renderD128
> >>>> vendor : 8086
> >>>> device : 193B
> >>>>
> >>>> New format looks like:
> >>>>
> >>>> card0 8086:193B drm:/dev/dri/card0
> >>>> └─renderD128 drm:/dev/dri/renderD128
> >>>>
> >>>> Advantages are more compact, more readable, one entry per GPU, shorter
> >>>> string to copy and paste to intel_gpu_top -d, or respective usage.
> >>>
> >>> Looks nice and more intuitive.
> >>
> >> I wasn't sure about duplication of card/render name and again in filter
> >> string, but I have no better ideas at the moment.
> >>
> >> Maybe one day some could replace the left column with some card names from
> >> some database but for now above is good enough.
> >>
> >>> 1. Add -s switch to handle simple print (current default in lsgpu)
> >>
> >> I wanted to suggest -v (verbose) or -d (details) but both are taken in
> >> lsgpu. -s sounds like silent or in any case not obvious what "simple" means.
> >> Could use just long form --verbose or --details?
> >
> > -v is vendor list we support in pci vendor ids, not verbose. If you have
> > resistence to use -s just use -i (simple). Sometimes I need to go to
> > device directory in /sys so I use lsgpu simple output with directory name.
>
> I don't think "simple" as a term should be user facing because it is more complex than the mode I am proposing. So cannot really be called simple in my mind.
As lsgpu had two views they were called simple and detailed. I got no
better names previously.
>
> To be clear I am working under the assumption intel_gpu_top and lsgpu will be getting more and more users and as such emphasis on making them less of a developer tools should be increased. (Former had an user bug submitted in Fedora, which was one of the triggers.)
>
> I do agree sysfs path is an interesting addition, or alternate mode. Maybe user printout should have selectable modes of drm or sysfs?
>
> $ lsgpu --drm --filter (filter adds the drm: prefix, can be default)
>
> card0 8086:193B drm:/dev/dri/card0
> └─renderD128 drm:/dev/dri/renderD128
I would assume --drm as default so
$ lsgpu
would be enough for above (drm) view.
>
> $ lsgpu --sysfs
>
> card0 8086:193B /sys/devices/pci0000:00/0000:00:02.0/drm/card0
> └─renderD128 /sys/devices/pci0000:00/0000:00:02.0/drm/renderD128
>
Ack.
> $ lsgpu --device
>
> card0 8086:193B /sys/devices/pci0000:00/0000:00:02.0
> └─renderD128
Ack.
>
> $ lsgpu --verbose (or --details)
>
> ... the current "simple" output ...
I think we can reach consensuns with:
--verbose (simple view)
--detail (detailed view with properties)
--user (default view as you propose)
--
Zbigniew
>
> None of my complications from above do not fit directly into the current print type enum. I'll play with it time permitting and send another RFC.
>
> Regards,
>
> Tvrtko
>
> >
> > --
> > Zbigniew
> >
> >
> >>
> >>> 2. Change to user format as default.
> >>
> >> Yep.
> >>
> >> Regards,
> >>
> >> Tvrtko
> >>
> >>>
> >>> --
> >>> Zbigniew
> >>>
> >>>>
> >>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>>> Cc: Petri Latvala <petri.latvala@intel.com>
> >>>> Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> >>>> ---
> >>>> lib/igt_device_scan.c | 109 +++++++++++++++++++++++++++++++++++++-----
> >>>> lib/igt_device_scan.h | 1 +
> >>>> tools/intel_gpu_top.c | 3 +-
> >>>> 3 files changed, 100 insertions(+), 13 deletions(-)
> >>>>
> >>>> diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
> >>>> index f4d43c733314..ce0ea61dda50 100644
> >>>> --- a/lib/igt_device_scan.c
> >>>> +++ b/lib/igt_device_scan.c
> >>>> @@ -695,18 +695,26 @@ static inline void _pr_simple2(const char *k, const char *v1, const char *v2)
> >>>> printf(" %-16s: %s:%s\n", k, v1, v2);
> >>>> }
> >>>> -static void igt_devs_print_simple(struct igt_list_head *view)
> >>>> +static bool __check_empty(struct igt_list_head *view)
> >>>> {
> >>>> - struct igt_device *dev;
> >>>> -
> >>>> if (!view)
> >>>> - return;
> >>>> + return true;
> >>>> if (igt_list_empty(view)) {
> >>>> printf("No GPU devices found\n");
> >>>> - return;
> >>>> + return true;
> >>>> }
> >>>> + return false;
> >>>> +}
> >>>> +
> >>>> +static void igt_devs_print_simple(struct igt_list_head *view)
> >>>> +{
> >>>> + struct igt_device *dev;
> >>>> +
> >>>> + if (__check_empty(view))
> >>>> + return;
> >>>> +
> >>>> igt_list_for_each_entry(dev, view, link) {
> >>>> printf("sys:%s\n", dev->syspath);
> >>>> if (dev->subsystem)
> >>>> @@ -728,6 +736,89 @@ static void igt_devs_print_simple(struct igt_list_head *view)
> >>>> }
> >>>> }
> >>>> +static struct igt_device *
> >>>> +__find_pci(struct igt_list_head *view, const char *drm)
> >>>> +{
> >>>> + struct igt_device *dev;
> >>>> +
> >>>> + igt_list_for_each_entry(dev, view, link) {
> >>>> + if (!is_pci_subsystem(dev) || !dev->drm_card)
> >>>> + continue;
> >>>> +
> >>>> + if (!strcmp(dev->drm_card, drm))
> >>>> + return dev;
> >>>> + }
> >>>> +
> >>>> + return NULL;
> >>>> +}
> >>>> +
> >>>> +static void igt_devs_print_user(struct igt_list_head *view)
> >>>> +{
> >>>> + struct igt_device *dev;
> >>>> +
> >>>> + if (__check_empty(view))
> >>>> + return;
> >>>> +
> >>>> + igt_list_for_each_entry(dev, view, link) {
> >>>> + unsigned int i, num_children;
> >>>> + struct igt_device *pci_dev;
> >>>> + struct igt_device *dev2;
> >>>> + char filter[64];
> >>>> + char *drm_name;
> >>>> + int ret;
> >>>> +
> >>>> + if (!is_drm_subsystem(dev))
> >>>> + continue;
> >>>> + if (!dev->drm_card || dev->drm_render)
> >>>> + continue;
> >>>> +
> >>>> + drm_name = rindex(dev->drm_card, '/');
> >>>> + if (!drm_name || !*++drm_name)
> >>>> + continue;
> >>>> +
> >>>> + ret = snprintf(filter, sizeof(filter), "drm:%s", dev->drm_card);
> >>>> + igt_assert(ret < sizeof(filter));
> >>>> +
> >>>> + pci_dev = __find_pci(view, dev->drm_card);
> >>>> + if (pci_dev)
> >>>> + printf("%-24s%4s:%4s %s\n",
> >>>> + drm_name, pci_dev->vendor, pci_dev->device,
> >>>> + filter);
> >>>> + else
> >>>> + printf("%-24s %s\n", drm_name, filter);
> >>>> +
> >>>> + num_children = 0;
> >>>> + igt_list_for_each_entry(dev2, view, link) {
> >>>> + if (!is_drm_subsystem(dev2) || !dev2->drm_render)
> >>>> + continue;
> >>>> + if (strcmp(dev2->parent->syspath, dev->parent->syspath))
> >>>> + continue;
> >>>> +
> >>>> + num_children++;
> >>>> + }
> >>>> +
> >>>> + i = 0;
> >>>> + igt_list_for_each_entry(dev2, view, link) {
> >>>> + if (!is_drm_subsystem(dev2) || !dev2->drm_render)
> >>>> + continue;
> >>>> + if (strcmp(dev2->parent->syspath, dev->parent->syspath))
> >>>> + continue;
> >>>> +
> >>>> + drm_name = rindex(dev2->drm_render, '/');
> >>>> + if (!drm_name || !*++drm_name)
> >>>> + continue;
> >>>> +
> >>>> + ret = snprintf(filter, sizeof(filter), "drm:%s",
> >>>> + dev2->drm_render);
> >>>> + igt_assert(ret < sizeof(filter));
> >>>> +
> >>>> + printf("%s%-22s %s\n",
> >>>> + (++i == num_children) ? "└─" : "├─",
> >>>> + drm_name, filter);
> >>>> + }
> >>>> + }
> >>>> +}
> >>>> +
> >>>> static inline void _print_key_value(const char* k, const char *v)
> >>>> {
> >>>> printf("%-32s: %s\n", k, v);
> >>>> @@ -752,14 +843,9 @@ static void igt_devs_print_detail(struct igt_list_head *view)
> >>>> {
> >>>> struct igt_device *dev;
> >>>> - if (!view)
> >>>> + if (__check_empty(view))
> >>>> return;
> >>>> - if (igt_list_empty(view)) {
> >>>> - printf("No GPU devices found\n");
> >>>> - return;
> >>>> - }
> >>>> -
> >>>> igt_list_for_each_entry(dev, view, link) {
> >>>> printf("========== %s:%s ==========\n",
> >>>> dev->subsystem, dev->syspath);
> >>>> @@ -781,6 +867,7 @@ static struct print_func {
> >>>> } print_functions[] = {
> >>>> [IGT_PRINT_SIMPLE] = { .prn = igt_devs_print_simple },
> >>>> [IGT_PRINT_DETAIL] = { .prn = igt_devs_print_detail },
> >>>> + [IGT_PRINT_USER] = { .prn = igt_devs_print_user },
> >>>> };
> >>>> /**
> >>>> diff --git a/lib/igt_device_scan.h b/lib/igt_device_scan.h
> >>>> index bd937d22752c..9e13ed9db406 100644
> >>>> --- a/lib/igt_device_scan.h
> >>>> +++ b/lib/igt_device_scan.h
> >>>> @@ -37,6 +37,7 @@
> >>>> enum igt_devices_print_type {
> >>>> IGT_PRINT_SIMPLE,
> >>>> IGT_PRINT_DETAIL,
> >>>> + IGT_PRINT_USER, /* End user friendly. */
> >>>> };
> >>>> #define INTEGRATED_I915_GPU_PCI_ID "0000:00:02.0"
> >>>> diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
> >>>> index 79a936ffbe1a..b984edc656c7 100644
> >>>> --- a/tools/intel_gpu_top.c
> >>>> +++ b/tools/intel_gpu_top.c
> >>>> @@ -1313,7 +1313,6 @@ int main(int argc, char **argv)
> >>>> unsigned int i;
> >>>> int ret = 0, ch;
> >>>> bool list_device = false;
> >>>> - enum igt_devices_print_type printtype = IGT_PRINT_SIMPLE;
> >>>> char *pmu_device, *opt_device = NULL;
> >>>> struct igt_device_card card;
> >>>> @@ -1388,7 +1387,7 @@ int main(int argc, char **argv)
> >>>> igt_devices_scan(false);
> >>>> if (list_device) {
> >>>> - igt_devices_print(printtype);
> >>>> + igt_devices_print(IGT_PRINT_USER);
> >>>> goto exit;
> >>>> }
> >>>> --
> >>>> 2.25.1
> >>>>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [igt-dev] [RFC i-g-t] intel_gpu_top: User friendly device listing
@ 2020-10-16 11:48 ` Zbigniew Kempczyński
0 siblings, 0 replies; 14+ messages in thread
From: Zbigniew Kempczyński @ 2020-10-16 11:48 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: igt-dev, Intel-gfx, Petri Latvala, Tvrtko Ursulin
On Fri, Oct 16, 2020 at 10:11:04AM +0100, Tvrtko Ursulin wrote:
>
> On 16/10/2020 05:07, Zbigniew Kempczyński wrote:
> > On Thu, Oct 15, 2020 at 09:09:02AM +0100, Tvrtko Ursulin wrote:
> >>
> >> On 15/10/2020 05:36, Zbigniew Kempczyński wrote:
> >>> On Wed, Oct 14, 2020 at 11:48:53AM +0100, Tvrtko Ursulin wrote:
> >>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>>>
> >>>> Adding a new device selection print type suitable for user-facing
> >>>> use cases like intel_gpu_top -L and potentially lsgpu.
> >>>>
> >>>> Instead of:
> >>>>
> >>>> sys:/sys/devices/pci0000:00/0000:00:02.0/drm/card0
> >>>> subsystem : drm
> >>>> drm card : /dev/dri/card0
> >>>> parent : sys:/sys/devices/pci0000:00/0000:00:02.0
> >>>>
> >>>> sys:/sys/devices/pci0000:00/0000:00:02.0/drm/renderD128
> >>>> subsystem : drm
> >>>> drm render : /dev/dri/renderD128
> >>>> parent : sys:/sys/devices/pci0000:00/0000:00:02.0
> >>>>
> >>>> sys:/sys/devices/pci0000:00/0000:00:02.0
> >>>> subsystem : pci
> >>>> drm card : /dev/dri/card0
> >>>> drm render : /dev/dri/renderD128
> >>>> vendor : 8086
> >>>> device : 193B
> >>>>
> >>>> New format looks like:
> >>>>
> >>>> card0 8086:193B drm:/dev/dri/card0
> >>>> └─renderD128 drm:/dev/dri/renderD128
> >>>>
> >>>> Advantages are more compact, more readable, one entry per GPU, shorter
> >>>> string to copy and paste to intel_gpu_top -d, or respective usage.
> >>>
> >>> Looks nice and more intuitive.
> >>
> >> I wasn't sure about duplication of card/render name and again in filter
> >> string, but I have no better ideas at the moment.
> >>
> >> Maybe one day some could replace the left column with some card names from
> >> some database but for now above is good enough.
> >>
> >>> 1. Add -s switch to handle simple print (current default in lsgpu)
> >>
> >> I wanted to suggest -v (verbose) or -d (details) but both are taken in
> >> lsgpu. -s sounds like silent or in any case not obvious what "simple" means.
> >> Could use just long form --verbose or --details?
> >
> > -v is vendor list we support in pci vendor ids, not verbose. If you have
> > resistence to use -s just use -i (simple). Sometimes I need to go to
> > device directory in /sys so I use lsgpu simple output with directory name.
>
> I don't think "simple" as a term should be user facing because it is more complex than the mode I am proposing. So cannot really be called simple in my mind.
As lsgpu had two views they were called simple and detailed. I got no
better names previously.
>
> To be clear I am working under the assumption intel_gpu_top and lsgpu will be getting more and more users and as such emphasis on making them less of a developer tools should be increased. (Former had an user bug submitted in Fedora, which was one of the triggers.)
>
> I do agree sysfs path is an interesting addition, or alternate mode. Maybe user printout should have selectable modes of drm or sysfs?
>
> $ lsgpu --drm --filter (filter adds the drm: prefix, can be default)
>
> card0 8086:193B drm:/dev/dri/card0
> └─renderD128 drm:/dev/dri/renderD128
I would assume --drm as default so
$ lsgpu
would be enough for above (drm) view.
>
> $ lsgpu --sysfs
>
> card0 8086:193B /sys/devices/pci0000:00/0000:00:02.0/drm/card0
> └─renderD128 /sys/devices/pci0000:00/0000:00:02.0/drm/renderD128
>
Ack.
> $ lsgpu --device
>
> card0 8086:193B /sys/devices/pci0000:00/0000:00:02.0
> └─renderD128
Ack.
>
> $ lsgpu --verbose (or --details)
>
> ... the current "simple" output ...
I think we can reach consensuns with:
--verbose (simple view)
--detail (detailed view with properties)
--user (default view as you propose)
--
Zbigniew
>
> None of my complications from above do not fit directly into the current print type enum. I'll play with it time permitting and send another RFC.
>
> Regards,
>
> Tvrtko
>
> >
> > --
> > Zbigniew
> >
> >
> >>
> >>> 2. Change to user format as default.
> >>
> >> Yep.
> >>
> >> Regards,
> >>
> >> Tvrtko
> >>
> >>>
> >>> --
> >>> Zbigniew
> >>>
> >>>>
> >>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>>> Cc: Petri Latvala <petri.latvala@intel.com>
> >>>> Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> >>>> ---
> >>>> lib/igt_device_scan.c | 109 +++++++++++++++++++++++++++++++++++++-----
> >>>> lib/igt_device_scan.h | 1 +
> >>>> tools/intel_gpu_top.c | 3 +-
> >>>> 3 files changed, 100 insertions(+), 13 deletions(-)
> >>>>
> >>>> diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
> >>>> index f4d43c733314..ce0ea61dda50 100644
> >>>> --- a/lib/igt_device_scan.c
> >>>> +++ b/lib/igt_device_scan.c
> >>>> @@ -695,18 +695,26 @@ static inline void _pr_simple2(const char *k, const char *v1, const char *v2)
> >>>> printf(" %-16s: %s:%s\n", k, v1, v2);
> >>>> }
> >>>> -static void igt_devs_print_simple(struct igt_list_head *view)
> >>>> +static bool __check_empty(struct igt_list_head *view)
> >>>> {
> >>>> - struct igt_device *dev;
> >>>> -
> >>>> if (!view)
> >>>> - return;
> >>>> + return true;
> >>>> if (igt_list_empty(view)) {
> >>>> printf("No GPU devices found\n");
> >>>> - return;
> >>>> + return true;
> >>>> }
> >>>> + return false;
> >>>> +}
> >>>> +
> >>>> +static void igt_devs_print_simple(struct igt_list_head *view)
> >>>> +{
> >>>> + struct igt_device *dev;
> >>>> +
> >>>> + if (__check_empty(view))
> >>>> + return;
> >>>> +
> >>>> igt_list_for_each_entry(dev, view, link) {
> >>>> printf("sys:%s\n", dev->syspath);
> >>>> if (dev->subsystem)
> >>>> @@ -728,6 +736,89 @@ static void igt_devs_print_simple(struct igt_list_head *view)
> >>>> }
> >>>> }
> >>>> +static struct igt_device *
> >>>> +__find_pci(struct igt_list_head *view, const char *drm)
> >>>> +{
> >>>> + struct igt_device *dev;
> >>>> +
> >>>> + igt_list_for_each_entry(dev, view, link) {
> >>>> + if (!is_pci_subsystem(dev) || !dev->drm_card)
> >>>> + continue;
> >>>> +
> >>>> + if (!strcmp(dev->drm_card, drm))
> >>>> + return dev;
> >>>> + }
> >>>> +
> >>>> + return NULL;
> >>>> +}
> >>>> +
> >>>> +static void igt_devs_print_user(struct igt_list_head *view)
> >>>> +{
> >>>> + struct igt_device *dev;
> >>>> +
> >>>> + if (__check_empty(view))
> >>>> + return;
> >>>> +
> >>>> + igt_list_for_each_entry(dev, view, link) {
> >>>> + unsigned int i, num_children;
> >>>> + struct igt_device *pci_dev;
> >>>> + struct igt_device *dev2;
> >>>> + char filter[64];
> >>>> + char *drm_name;
> >>>> + int ret;
> >>>> +
> >>>> + if (!is_drm_subsystem(dev))
> >>>> + continue;
> >>>> + if (!dev->drm_card || dev->drm_render)
> >>>> + continue;
> >>>> +
> >>>> + drm_name = rindex(dev->drm_card, '/');
> >>>> + if (!drm_name || !*++drm_name)
> >>>> + continue;
> >>>> +
> >>>> + ret = snprintf(filter, sizeof(filter), "drm:%s", dev->drm_card);
> >>>> + igt_assert(ret < sizeof(filter));
> >>>> +
> >>>> + pci_dev = __find_pci(view, dev->drm_card);
> >>>> + if (pci_dev)
> >>>> + printf("%-24s%4s:%4s %s\n",
> >>>> + drm_name, pci_dev->vendor, pci_dev->device,
> >>>> + filter);
> >>>> + else
> >>>> + printf("%-24s %s\n", drm_name, filter);
> >>>> +
> >>>> + num_children = 0;
> >>>> + igt_list_for_each_entry(dev2, view, link) {
> >>>> + if (!is_drm_subsystem(dev2) || !dev2->drm_render)
> >>>> + continue;
> >>>> + if (strcmp(dev2->parent->syspath, dev->parent->syspath))
> >>>> + continue;
> >>>> +
> >>>> + num_children++;
> >>>> + }
> >>>> +
> >>>> + i = 0;
> >>>> + igt_list_for_each_entry(dev2, view, link) {
> >>>> + if (!is_drm_subsystem(dev2) || !dev2->drm_render)
> >>>> + continue;
> >>>> + if (strcmp(dev2->parent->syspath, dev->parent->syspath))
> >>>> + continue;
> >>>> +
> >>>> + drm_name = rindex(dev2->drm_render, '/');
> >>>> + if (!drm_name || !*++drm_name)
> >>>> + continue;
> >>>> +
> >>>> + ret = snprintf(filter, sizeof(filter), "drm:%s",
> >>>> + dev2->drm_render);
> >>>> + igt_assert(ret < sizeof(filter));
> >>>> +
> >>>> + printf("%s%-22s %s\n",
> >>>> + (++i == num_children) ? "└─" : "├─",
> >>>> + drm_name, filter);
> >>>> + }
> >>>> + }
> >>>> +}
> >>>> +
> >>>> static inline void _print_key_value(const char* k, const char *v)
> >>>> {
> >>>> printf("%-32s: %s\n", k, v);
> >>>> @@ -752,14 +843,9 @@ static void igt_devs_print_detail(struct igt_list_head *view)
> >>>> {
> >>>> struct igt_device *dev;
> >>>> - if (!view)
> >>>> + if (__check_empty(view))
> >>>> return;
> >>>> - if (igt_list_empty(view)) {
> >>>> - printf("No GPU devices found\n");
> >>>> - return;
> >>>> - }
> >>>> -
> >>>> igt_list_for_each_entry(dev, view, link) {
> >>>> printf("========== %s:%s ==========\n",
> >>>> dev->subsystem, dev->syspath);
> >>>> @@ -781,6 +867,7 @@ static struct print_func {
> >>>> } print_functions[] = {
> >>>> [IGT_PRINT_SIMPLE] = { .prn = igt_devs_print_simple },
> >>>> [IGT_PRINT_DETAIL] = { .prn = igt_devs_print_detail },
> >>>> + [IGT_PRINT_USER] = { .prn = igt_devs_print_user },
> >>>> };
> >>>> /**
> >>>> diff --git a/lib/igt_device_scan.h b/lib/igt_device_scan.h
> >>>> index bd937d22752c..9e13ed9db406 100644
> >>>> --- a/lib/igt_device_scan.h
> >>>> +++ b/lib/igt_device_scan.h
> >>>> @@ -37,6 +37,7 @@
> >>>> enum igt_devices_print_type {
> >>>> IGT_PRINT_SIMPLE,
> >>>> IGT_PRINT_DETAIL,
> >>>> + IGT_PRINT_USER, /* End user friendly. */
> >>>> };
> >>>> #define INTEGRATED_I915_GPU_PCI_ID "0000:00:02.0"
> >>>> diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
> >>>> index 79a936ffbe1a..b984edc656c7 100644
> >>>> --- a/tools/intel_gpu_top.c
> >>>> +++ b/tools/intel_gpu_top.c
> >>>> @@ -1313,7 +1313,6 @@ int main(int argc, char **argv)
> >>>> unsigned int i;
> >>>> int ret = 0, ch;
> >>>> bool list_device = false;
> >>>> - enum igt_devices_print_type printtype = IGT_PRINT_SIMPLE;
> >>>> char *pmu_device, *opt_device = NULL;
> >>>> struct igt_device_card card;
> >>>> @@ -1388,7 +1387,7 @@ int main(int argc, char **argv)
> >>>> igt_devices_scan(false);
> >>>> if (list_device) {
> >>>> - igt_devices_print(printtype);
> >>>> + igt_devices_print(IGT_PRINT_USER);
> >>>> goto exit;
> >>>> }
> >>>> --
> >>>> 2.25.1
> >>>>
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2020-10-16 11:55 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-14 10:48 [Intel-gfx] [RFC i-g-t] intel_gpu_top: User friendly device listing Tvrtko Ursulin
2020-10-14 11:05 ` Chris Wilson
2020-10-14 12:01 ` Tvrtko Ursulin
2020-10-14 12:01 ` [igt-dev] " Tvrtko Ursulin
2020-10-14 11:43 ` [igt-dev] ✓ Fi.CI.BAT: success for " Patchwork
2020-10-15 4:36 ` [Intel-gfx] [RFC i-g-t] " Zbigniew Kempczyński
2020-10-15 4:36 ` [igt-dev] " Zbigniew Kempczyński
2020-10-15 8:09 ` [Intel-gfx] " Tvrtko Ursulin
2020-10-16 4:07 ` Zbigniew Kempczyński
2020-10-16 4:07 ` [igt-dev] " Zbigniew Kempczyński
2020-10-16 9:11 ` [Intel-gfx] " Tvrtko Ursulin
2020-10-16 9:11 ` [igt-dev] " Tvrtko Ursulin
2020-10-16 11:48 ` [Intel-gfx] " Zbigniew Kempczyński
2020-10-16 11:48 ` [igt-dev] " Zbigniew Kempczyński
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.