Re: [Intel-gfx] [RFC i-g-t] intel_gpu_top: User friendly device listing

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

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