Re: [igt-dev] [PATCH i-g-t 3/4] Introduce device selection lsgpu tool

[Petri Latvala <petri.latvala@intel.com>]

On Thu, Oct 24, 2019 at 02:05:14PM +0300, Arkadiusz Hiler wrote:
> From: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> 
> Tool uses device selection API to scan and display GPU devices.
> It can be used to check filter correctness as well as order
> of applying the filters (.igtrc, IGT_DEVICE and --device argument).
> 
> v2 (Arek):
>  * don't print chip as it's no longer there
>  * make it a second patch, before any alterations to igt_core or drmtest
>  * use only a single filter
> 
> Cc: Petri Latvala <petri.latvala@intel.com>
> Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> ---
>  tools/Makefile.sources |   1 +
>  tools/lsgpu.c          | 295 +++++++++++++++++++++++++++++++++++++++++
>  tools/meson.build      |   1 +
>  3 files changed, 297 insertions(+)
>  create mode 100644 tools/lsgpu.c
> 
> diff --git a/tools/Makefile.sources b/tools/Makefile.sources
> index d764895d..b7a43d47 100644
> --- a/tools/Makefile.sources
> +++ b/tools/Makefile.sources
> @@ -33,6 +33,7 @@ tools_prog_lists =		\
>  	intel_watermark		\
>  	intel_gem_info		\
>  	intel_gvtg_test     \
> +	lsgpu			\
>  	$(NULL)
>  
>  dist_bin_SCRIPTS = intel_gpu_abrt
> diff --git a/tools/lsgpu.c b/tools/lsgpu.c
> new file mode 100644
> index 00000000..e385aaa6
> --- /dev/null
> +++ b/tools/lsgpu.c
> @@ -0,0 +1,295 @@
> +/*
> + * Copyright © 2019 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + */
> +
> +#include "igt_device_scan.h"
> +#include "igt.h"
> +#include <sys/ioctl.h>
> +#include <fcntl.h>
> +#include <errno.h>
> +#include <string.h>
> +#include <signal.h>
> +#include <glib.h>
> +
> +/**
> + * SECTION:lsgpu
> + * @short_description: lsgpu
> + * @title: lsgpu
> + * @include: lsgpu.c
> + *
> + * # lsgpu
> + *
> + * The devices can be scanned and displayed using 'lsgpu' tool. Tool also
> + * displays properties and sysattrs (-p switch, means print detail) which
> + * can be used during filter implementation.
> + *
> + * Tool can also be used to try out filters.
> + * To select device use '-d' or '--device' argument like:
> + *
> + * |[<!-- language="plain" -->
> + * ./lsgpu -d 'pci:vendor=Intel'
> + * === Device filter list ===
> + * [ 0]: pci:vendor=Intel
> +
> + * === Testing device open ===
> + * subsystem   : pci
> + * drm card    : /dev/dri/card0
> + * drm render  : /dev/dri/renderD128
> + * Device /dev/dri/card0 successfully opened
> + * Device /dev/dri/renderD128 successfully opened
> + * ]|
> + *
> + * Additionally lsgpu tries to open the card and render nodes to verify
> + * permissions. It also uses IGT variable search order:
> + * - use --device first (it overrides IGT_DEVICE and .igtrc Common::Device
> + *   settings)
> + * - use IGT_DEVICE enviroment variable if no --device are passed
> + * - use .igtrc Common::Device if no --device nor IGT_DEVICE are passed
> + */
> +
> +enum {
> +	OPT_PRINT_DETAIL   = 'p',
> +	OPT_LIST_VENDORS   = 'v',
> +	OPT_LIST_FILTERS   = 'l',
> +	OPT_DEVICE         = 'd',
> +	OPT_HELP           = 'h'
> +};
> +
> +static bool g_show_vendors;
> +static bool g_list_filters;
> +static bool g_device;
> +static bool g_help;
> +static char *igt_rc_device;
> +
> +static const char *usage_str =
> +	"usage: lsgpu [options]\n\n"
> +	"Options:\n"
> +	"  -p, --print-details         Print devices with details\n"
> +	"  -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";
> +
> +static void test_device_open(struct igt_device_card *card)
> +{
> +	int fd;
> +
> +	if (!card)
> +		return;
> +
> +	fd = igt_open_card(card);
> +	if (fd >= 0) {
> +		printf("Device %s successfully opened\n", card->card);
> +		close(fd);
> +	} else {
> +		if (strlen(card->card))
> +			printf("Cannot open card %s device\n", card->card);
> +		else
> +			printf("Cannot open card device, empty name\n");
> +	}
> +
> +	fd = igt_open_render(card);
> +	if (fd >= 0) {
> +		printf("Device %s successfully opened\n", card->render);
> +		close(fd);
> +	} else {
> +		if (strlen(card->render))
> +			printf("Cannot open render %s device\n", card->render);
> +		else
> +			printf("Cannot open render device, empty name\n");
> +	}
> +}
> +
> +static void print_card(struct igt_device_card *card)
> +{
> +	if (!card)
> +		return;
> +
> +	printf("subsystem   : %s\n", card->subsystem);
> +	printf("drm card    : %s\n", card->card);
> +	printf("drm render  : %s\n", card->render);
> +}
> +
> +/* Partially copied from igt_core to simulate device selection order:
> + * 1. --device        (IGT_DEVICE and .igtrc Common::Device are ignored)
> + * 2. IGT_DEVICE env  (.igtrc Common::Device is ignored)
> + * 3. .igtrc          (overrides
> + */

Is that the reason to not use igt_load_igtrc?


> +static void common_init_config(void)
> +{
> +	char *key_file_env = NULL;
> +	char *key_file_loc = NULL;
> +	GError *error = NULL;
> +	GKeyFile *igt_key_file;
> +	int ret;
> +
> +	/* Filter count > 0, just skip .igtrc */
> +	if (igt_device_is_filter_set())
> +		return;
> +
> +	/* Determine igt config path */
> +	key_file_env = getenv("IGT_CONFIG_PATH");
> +	if (key_file_env) {
> +		key_file_loc = key_file_env;
> +	} else {
> +		key_file_loc = malloc(100);

This malloc is not free()d.


Otherwise LGTM.

Reviewed-by: Petri Latvala <petri.latvala@intel.com>
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev