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@xxxxxxxxx>
> > >
> > > 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@xxxxxxxxx>
> > > Cc: Petri Latvala <petri.latvala@xxxxxxxxx>
> > > Cc: Zbigniew Kempczyński <zbigniew.kempczynski@xxxxxxxxx>
> > > ---
> > > 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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
