Re: [PATCH i-g-t 4/5] lsgpu: Add filter type print-out selection

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Nov 16, 2020 at 05:26:52PM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> 
> 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:vendor=8086,device=193B,card=0
> └─renderD128
> 
> v2:
>  * Fix pci filter format.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> Suggested-by: Zbigniew Kempczyński <zbigniew.kempczynski@xxxxxxxxx>
> Cc: Petri Latvala <petri.latvala@xxxxxxxxx>
> ---
>  lib/igt_device_scan.c | 83 ++++++++++++++++++++++++++++++++-----------
>  lib/igt_device_scan.h | 15 ++++++--
>  tools/intel_gpu_top.c |  6 +++-
>  tools/lsgpu.c         | 32 +++++++++++++----
>  4 files changed, 106 insertions(+), 30 deletions(-)
> 
> diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
> index ecb8db297f86..e97f7163ba70 100644
> --- a/lib/igt_device_scan.c
> +++ b/lib/igt_device_scan.c
> @@ -777,7 +777,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;
>  
> @@ -821,7 +823,38 @@ __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:vendor=%s,device=%s,card=%d",
> +				       dev->vendor, dev->device,
> +				       dev->gpu_index);
> +			igt_assert(ret < len);
> +		}
> +		break;
> +	};

You could leave single assert after the switch but this is minor nit.

> +}
> +
> +static void
> +igt_devs_print_user(struct igt_list_head *view,
> +		    const struct igt_devices_print_format *fmt)
>  {
>  	struct igt_device *dev;
>  
> @@ -834,7 +867,6 @@ static void igt_devs_print_user(struct igt_list_head *view)
>  		struct igt_device *dev2;
>  		char filter[256];
>  		char *drm_name;
> -		int ret;
>  
>  		if (!is_drm_subsystem(dev))
>  			continue;
> @@ -845,16 +877,21 @@ 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));
> -
>  		pci_dev = __find_pci(view, dev->drm_card);
> -		if (pci_dev)
> +
> +		if (fmt->option == IGT_PRINT_PCI && !pci_dev)
> +			continue;
> +
> +		if (pci_dev) {
> +			__print_filter(filter, sizeof(filter), fmt, pci_dev,
> +				       false);
>  			printf("%-24s%4s:%4s    %s\n",
>  			       drm_name, pci_dev->vendor, pci_dev->device,
>  			       filter);
> -		else
> +		} else {
> +			__print_filter(filter, sizeof(filter), fmt, dev, false);
>  			printf("%-24s             %s\n", drm_name, filter);
> +		}
>  
>  		num_children = 0;
>  		igt_list_for_each_entry(dev2, view, link) {
> @@ -877,13 +914,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");
> +			}
>  		}
>  	}
>  }
> @@ -908,7 +947,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;
>  
> @@ -932,7 +974,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 },
> @@ -941,15 +984,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
>

Ok, haven't tracked other issues and LGTM:

Reviewed-by: Zbigniew Kempczyński <zbigniew.kempczynski@xxxxxxxxx>

--
Zbigniew 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux