[igt-dev] [RFC 3/3] lsgpu: Add filter type print-out selection
Zbigniew Kempczyński
zbigniew.kempczynski at intel.com
Tue Nov 10 11:02:41 UTC 2020
On Mon, Nov 09, 2020 at 10:48:11AM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>
> In the previous patch we switched the lsgpu output to a short and user
> friendly format but some users will need a shorthand for getting other
> types of device selection filters than the defaut drm.
>
> Add some command line switches to enable this:
>
> $ lsgpu
> card0 8086:193B drm:/dev/dri/card0
> └─renderD128 drm:/dev/dri/renderD128
>
> $ lsgpu --sysfs
> card0 8086:193B sys:/sys/devices/pci0000:00/0000:00:02.0/drm/card0
> └─renderD128 sys:/sys/devices/pci0000:00/0000:00:02.0/drm/renderD128
>
> $ lsgpu --pci
> card0 8086:193B pci:/sys/devices/pci0000:00/0000:00:02.0
> └─renderD128
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> Suggested-by: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
> Cc: Petri Latvala <petri.latvala at intel.com>
> ---
> lib/igt_device_scan.c | 69 ++++++++++++++++++++++++++++++++-----------
> lib/igt_device_scan.h | 15 ++++++++--
> tools/intel_gpu_top.c | 6 +++-
> tools/lsgpu.c | 32 +++++++++++++++-----
> 4 files changed, 95 insertions(+), 27 deletions(-)
>
> diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
> index e66ccdc25aeb..54b565b68524 100644
> --- a/lib/igt_device_scan.c
> +++ b/lib/igt_device_scan.c
> @@ -748,7 +748,9 @@ static bool __check_empty(struct igt_list_head *view)
> return false;
> }
>
> -static void igt_devs_print_simple(struct igt_list_head *view)
> +static void
> +igt_devs_print_simple(struct igt_list_head *view,
> + const struct igt_devices_print_format *fmt)
> {
> struct igt_device *dev;
>
> @@ -792,7 +794,36 @@ __find_pci(struct igt_list_head *view, const char *drm)
> return NULL;
> }
>
> -static void igt_devs_print_user(struct igt_list_head *view)
> +static void __print_filter(char *buf, int len,
> + const struct igt_devices_print_format *fmt,
> + struct igt_device *dev,
> + bool render)
> +{
> + int ret;
> +
> + switch (fmt->option) {
> + case IGT_PRINT_DRM:
> + ret = snprintf(buf, len, "drm:%s",
> + render ? dev->drm_render : dev->drm_card);
> + igt_assert(ret < len);
> + break;
> + case IGT_PRINT_SYSFS:
> + ret = snprintf(buf, len, "sys:%s", dev->syspath);
> + igt_assert(ret < len);
> + break;
> + case IGT_PRINT_PCI:
> + if (!render) {
> + ret = snprintf(buf, len, "pci:%s",
> + dev->parent->syspath);
> + igt_assert(ret < len);
> + }
> + break;
> + };
> +}
> +
> +static void
> +igt_devs_print_user(struct igt_list_head *view,
> + const struct igt_devices_print_format *fmt)
> {
> struct igt_device *dev;
>
> @@ -805,7 +836,6 @@ static void igt_devs_print_user(struct igt_list_head *view)
> struct igt_device *dev2;
> char filter[64];
Too small, for two machines I got (printed returned size) :
(lsgpu:15601) igt_device_scan-INFO: path: /sys/devices/pci0000:00/0000:00:01.0/0000:01:00.0/0000:02:01.0/0000:03:00.0/drm/card0, ret: 89, len: 64
(lsgpu:15601) igt_device_scan-CRITICAL: Test assertion failure function __print_filter, file ../lib/igt_device_scan.c:815:
(lsgpu:15601) igt_device_scan-CRITICAL: Failed assertion: ret < len
I would extend that to at least 128 or 256 (in this case we can completely
forget about the path length).
--
Zbigniew
> char *drm_name;
> - int ret;
>
> if (!is_drm_subsystem(dev))
> continue;
> @@ -816,8 +846,7 @@ static void igt_devs_print_user(struct igt_list_head *view)
> if (!drm_name || !*++drm_name)
> continue;
>
> - ret = snprintf(filter, sizeof(filter), "drm:%s", dev->drm_card);
> - igt_assert(ret < sizeof(filter));
> + __print_filter(filter, sizeof(filter), fmt, dev, false);
>
> pci_dev = __find_pci(view, dev->drm_card);
> if (pci_dev)
> @@ -848,13 +877,15 @@ static void igt_devs_print_user(struct igt_list_head *view)
> 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);
> + printf("%s%-22s",
> + (++i == num_children) ? "└─" : "├─", drm_name);
> + if (fmt->option != IGT_PRINT_PCI) {
> + __print_filter(filter, sizeof(filter), fmt,
> + dev2, true);
> + printf(" %s\n", filter);
> + } else {
> + printf("\n");
> + }
> }
> }
> }
> @@ -879,7 +910,10 @@ static void print_ht(GHashTable *ht)
> g_list_free(keys);
> }
>
> -static void igt_devs_print_detail(struct igt_list_head *view)
> +static void
> +igt_devs_print_detail(struct igt_list_head *view,
> + const struct igt_devices_print_format *fmt)
> +
> {
> struct igt_device *dev;
>
> @@ -903,7 +937,8 @@ static void igt_devs_print_detail(struct igt_list_head *view)
> }
>
> static struct print_func {
> - void (*prn)(struct igt_list_head *view);
> + void (*prn)(struct igt_list_head *view,
> + const struct igt_devices_print_format *);
> } print_functions[] = {
> [IGT_PRINT_SIMPLE] = { .prn = igt_devs_print_simple },
> [IGT_PRINT_DETAIL] = { .prn = igt_devs_print_detail },
> @@ -912,15 +947,15 @@ static struct print_func {
>
> /**
> * igt_devices_print
> - * @printtype: IGT_PRINT_SIMPLE or IGT_PRINT_DETAIL
> + * @fmt: Print format as specified by struct igt_devices_print_format
> *
> * Function can be used by external tool to print device array in simple
> * or detailed form. This function is added here to avoid exposing
> * internal implementation data structures.
> */
> -void igt_devices_print(enum igt_devices_print_type printtype)
> +void igt_devices_print(const struct igt_devices_print_format *fmt)
> {
> - print_functions[printtype].prn(&igt_devs.filtered);
> + print_functions[fmt->type].prn(&igt_devs.filtered, fmt);
> }
>
> /**
> diff --git a/lib/igt_device_scan.h b/lib/igt_device_scan.h
> index 9822c22cb69c..bb4be72345fb 100644
> --- a/lib/igt_device_scan.h
> +++ b/lib/igt_device_scan.h
> @@ -35,11 +35,22 @@
> #include <unistd.h>
>
> enum igt_devices_print_type {
> - IGT_PRINT_SIMPLE,
> + IGT_PRINT_SIMPLE = 0,
> IGT_PRINT_DETAIL,
> IGT_PRINT_USER, /* End user friendly. */
> };
>
> +enum igt_devices_print_option {
> + IGT_PRINT_DRM = 0,
> + IGT_PRINT_SYSFS,
> + IGT_PRINT_PCI,
> +};
> +
> +struct igt_devices_print_format {
> + enum igt_devices_print_type type;
> + enum igt_devices_print_option option;
> +};
> +
> #define INTEGRATED_I915_GPU_PCI_ID "0000:00:02.0"
> #define PCI_SLOT_NAME_SIZE 12
> struct igt_device_card {
> @@ -51,7 +62,7 @@ struct igt_device_card {
>
> void igt_devices_scan(bool force);
>
> -void igt_devices_print(enum igt_devices_print_type printtype);
> +void igt_devices_print(const struct igt_devices_print_format *fmt);
> void igt_devices_print_vendors(void);
> void igt_device_print_filter_types(void);
>
> diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
> index 5230472d2af4..07f88d555dc8 100644
> --- a/tools/intel_gpu_top.c
> +++ b/tools/intel_gpu_top.c
> @@ -1387,7 +1387,11 @@ int main(int argc, char **argv)
> igt_devices_scan(false);
>
> if (list_device) {
> - igt_devices_print(IGT_PRINT_USER);
> + struct igt_devices_print_format fmt = {
> + .type = IGT_PRINT_USER
> + };
> +
> + igt_devices_print(&fmt);
> goto exit;
> }
>
> diff --git a/tools/lsgpu.c b/tools/lsgpu.c
> index 3b234b73361a..169ab0c29e50 100644
> --- a/tools/lsgpu.c
> +++ b/tools/lsgpu.c
> @@ -91,7 +91,11 @@ static const char *usage_str =
> " -v, --list-vendors List recognized vendors\n"
> " -l, --list-filter-types List registered device filters types\n"
> " -d, --device filter Device filter, can be given multiple times\n"
> - " -h, --help Show this help message and exit\n";
> + " -h, --help Show this help message and exit\n"
> + "\nOptions valid for default print out mode only:\n"
> + " --drm Show DRM filters (default) for each device\n"
> + " --sysfs Show sysfs filters for each device\n"
> + " --pci Show PCI filters for each device\n";
>
> static void test_device_open(struct igt_device_card *card)
> {
> @@ -153,6 +157,9 @@ static char *get_device_from_rc(void)
> int main(int argc, char *argv[])
> {
> static struct option long_options[] = {
> + {"drm", no_argument, NULL, 0},
> + {"sysfs", no_argument, NULL, 1},
> + {"pci", no_argument, NULL, 2},
> {"print-simple", no_argument, NULL, OPT_PRINT_SIMPLE},
> {"print-detail", no_argument, NULL, OPT_PRINT_DETAIL},
> {"list-vendors", no_argument, NULL, OPT_LIST_VENDORS},
> @@ -163,17 +170,19 @@ int main(int argc, char *argv[])
> };
> int c, index = 0;
> char *env_device = NULL, *opt_device = NULL, *rc_device = NULL;
> - enum igt_devices_print_type printtype = IGT_PRINT_USER;
> + struct igt_devices_print_format fmt = {
> + .type = IGT_PRINT_USER,
> + };
>
> while ((c = getopt_long(argc, argv, "spvld:h",
> long_options, &index)) != -1) {
> switch(c) {
>
> case OPT_PRINT_SIMPLE:
> - printtype = IGT_PRINT_SIMPLE;
> + fmt.type = IGT_PRINT_SIMPLE;
> break;
> case OPT_PRINT_DETAIL:
> - printtype = IGT_PRINT_DETAIL;
> + fmt.type = IGT_PRINT_DETAIL;
> break;
> case OPT_LIST_VENDORS:
> g_show_vendors = true;
> @@ -187,6 +196,15 @@ int main(int argc, char *argv[])
> case OPT_HELP:
> g_help = true;
> break;
> + case 0:
> + fmt.option = IGT_PRINT_DRM;
> + break;
> + case 1:
> + fmt.option = IGT_PRINT_SYSFS;
> + break;
> + case 2:
> + fmt.option = IGT_PRINT_PCI;
> + break;
> }
> }
>
> @@ -239,14 +257,14 @@ int main(int argc, char *argv[])
> printf("Device detail:\n");
> print_card(&card);
> test_device_open(&card);
> - if (printtype == IGT_PRINT_DETAIL) {
> + if (fmt.type == IGT_PRINT_DETAIL) {
> printf("\n");
> - igt_devices_print(printtype);
> + igt_devices_print(&fmt);
> }
> printf("-------------------------------------------\n");
>
> } else {
> - igt_devices_print(printtype);
> + igt_devices_print(&fmt);
> }
>
> free(rc_device);
> --
> 2.25.1
>
More information about the igt-dev
mailing list