Re: [igt-dev] [PATCH i-g-t 4/4] Add device selection in IGT

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

On Thu, Oct 24, 2019 at 02:05:15PM +0300, Arkadiusz Hiler wrote:
> From: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> 
> New IGT command line argument --device, IGT_DEVICE enviroment
> and .igtrc Common::Device were added to allow selecting device
> using device selection API.
> 
> v2 (Arek):
>  * remove functions acting on igt_device_card
>  * 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>
> ---
>  .../igt-gpu-tools/igt_test_programs.xml       |  7 ++
>  lib/drmtest.c                                 | 94 +++++++++++++++++--
>  lib/drmtest.h                                 |  2 +
>  lib/igt_core.c                                | 47 ++++++++++
>  4 files changed, 142 insertions(+), 8 deletions(-)
> 
> diff --git a/docs/reference/igt-gpu-tools/igt_test_programs.xml b/docs/reference/igt-gpu-tools/igt_test_programs.xml
> index b64ba474..fcce1458 100644
> --- a/docs/reference/igt-gpu-tools/igt_test_programs.xml
> +++ b/docs/reference/igt-gpu-tools/igt_test_programs.xml
> @@ -43,6 +43,13 @@
>              </para></listitem>
>            </varlistentry>
>  
> +          <varlistentry>
> +            <term><option>--device filter</option></term>
> +            <listitem><para>
> +                select device using filter (see "Device selection" for details)
> +            </para></listitem>
> +          </varlistentry>
> +
>            <varlistentry>
>              <term><option>--help-description</option></term>
>              <listitem><para>
> diff --git a/lib/drmtest.c b/lib/drmtest.c
> index c379a7b7..43471625 100644
> --- a/lib/drmtest.c
> +++ b/lib/drmtest.c
> @@ -55,6 +55,7 @@
>  #include "igt_gt.h"
>  #include "igt_kmod.h"
>  #include "igt_sysfs.h"
> +#include "igt_device_scan.h"
>  #include "version.h"
>  #include "config.h"
>  #include "intel_reg.h"
> @@ -266,14 +267,9 @@ static int __search_and_open(const char *base, int offset, unsigned int chipset)
>  	return -1;
>  }
>  
> -static int __open_driver(const char *base, int offset, unsigned int chipset)
> +static void __try_modprobe(unsigned int chipset)
>  {
>  	static pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
> -	int fd;
> -
> -	fd = __search_and_open(base, offset, chipset);
> -	if (fd != -1)
> -		return fd;
>  
>  	pthread_mutex_lock(&mutex);
>  	for (const struct module *m = modules; m->module; m++) {
> @@ -285,26 +281,108 @@ static int __open_driver(const char *base, int offset, unsigned int chipset)
>  		}
>  	}
>  	pthread_mutex_unlock(&mutex);
> +}
> +
> +static int __open_driver(const char *base, int offset, unsigned int chipset)
> +{
> +	int fd;
> +
> +	fd = __search_and_open(base, offset, chipset);
> +	if (fd != -1)
> +		return fd;
> +
> +	__try_modprobe(chipset);
>  
>  	return __search_and_open(base, offset, chipset);
>  }
>  
> +static int __open_driver_exact(const char *name, unsigned int chipset)
> +{
> +	int fd;
> +
> +	fd = open_device(name, chipset);
> +	if (fd != -1)
> +		return fd;
> +
> +	__try_modprobe(chipset);
> +
> +	return open_device(name, chipset);
> +}
> +
> +/*
> + * A helper to get the first marching card in case a filter is set.
> + * It does all the extra logging around the filters for us.
> + *
> + * @card: pointer to the igt_device_card structure to be filled
> + * when a card is found.
> + *
> + * Returns:
> + * True if card according to the added filter was found,
> + * false othwerwise.
> + */
> +static bool __get_the_first_card(struct igt_device_card *card)
> +{
> +	const char *filter;
> +
> +	if (igt_device_is_filter_set()) {
> +		filter = igt_device_filter_get();
> +		igt_info("Looking for devices to open using filter: %s\n", filter);
> +
> +		if (igt_device_card_match(filter, card)) {
> +			igt_info("Filter matched %s | %s\n", card->card, card->render);
> +			return true;
> +		}
> +
> +		igt_warn("No card matches the filter!\n");
> +	}
> +
> +	return false;
> +}
> +
>  /**
>   * __drm_open_driver:
>   * @chipset: OR'd flags for each chipset to search, eg. #DRIVER_INTEL
>   *
> - * Open the first DRM device we can find, searching up to 16 device nodes
> + * Function opens device in the following order:
> + * 1. when --device arguments are present device scanning will be executed,
> + * then filter argument is used to find matching one.
> + * 2. compatibility mode - open the first DRM device we can find,
> + * searching up to 16 device nodes.
>   *
>   * Returns:
>   * An open DRM fd or -1 on error
>   */
>  int __drm_open_driver(int chipset)
>  {
> +	if (igt_device_is_filter_set()) {
> +		bool found;
> +		struct igt_device_card card;
> +
> +		found = __get_the_first_card(&card);
> +
> +		if (!found || !strlen(card.card))
> +			return -1;
> +
> +		return __open_driver_exact(card.card, chipset);
> +	}
> +
>  	return __open_driver("/dev/dri/card", 0, chipset);
>  }
>  
> -static int __drm_open_driver_render(int chipset)
> +int __drm_open_driver_render(int chipset)
>  {
> +	if (igt_device_is_filter_set()) {
> +		bool found;
> +		struct igt_device_card card;
> +
> +		found = __get_the_first_card(&card);
> +
> +		if (!found || !strlen(card.render))
> +			return -1;
> +
> +		return __open_driver_exact(card.render, chipset);
> +	}
> +
>  	return __open_driver("/dev/dri/renderD", 128, chipset);
>  }
>  
> diff --git a/lib/drmtest.h b/lib/drmtest.h
> index 614f57e6..7a985b26 100644
> --- a/lib/drmtest.h
> +++ b/lib/drmtest.h
> @@ -50,6 +50,7 @@
>  #define DRIVER_AMDGPU	(1 << 3)
>  #define DRIVER_V3D	(1 << 4)
>  #define DRIVER_PANFROST	(1 << 5)
> +
>  /*
>   * Exclude DRVER_VGEM from DRIVER_ANY since if you run on a system
>   * with vgem as well as a supported driver, you can end up with a
> @@ -81,6 +82,7 @@ int drm_open_driver(int chipset);
>  int drm_open_driver_master(int chipset);
>  int drm_open_driver_render(int chipset);
>  int __drm_open_driver(int chipset);
> +int __drm_open_driver_render(int chipset);
>  
>  void gem_quiescent_gpu(int fd);
>  
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index 67b76025..94121f99 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -71,6 +71,7 @@
>  #include "igt_sysrq.h"
>  #include "igt_rc.h"
>  #include "igt_list.h"
> +#include "igt_device_scan.h"
>  
>  #define UNW_LOCAL_ONLY
>  #include <libunwind.h>
> @@ -245,6 +246,9 @@
>   *	[Common]
>   *	FrameDumpPath=/tmp # The path to dump frames that fail comparison checks
>   *
> + *	&num; Device selection filter
> + *	Device=pci:vendor=8086,card=0;vgem:
> + *
>   *	&num; The following section is used for configuring the Device Under Test.
>   *	&num; It is not mandatory and allows overriding default values.
>   *	[DUT]
> @@ -304,6 +308,7 @@ enum {
>  	OPT_DEBUG,
>  	OPT_INTERACTIVE_DEBUG,
>  	OPT_SKIP_CRC,
> +	OPT_DEVICE,
>  	OPT_HELP = 'h'
>  };
>  
> @@ -320,6 +325,7 @@ static pthread_mutex_t log_buffer_mutex = PTHREAD_MUTEX_INITIALIZER;
>  GKeyFile *igt_key_file;
>  
>  char *igt_frame_dump_path;
> +char *igt_rc_device;
>  
>  static bool stderr_needs_sentinel = false;
>  
> @@ -644,6 +650,7 @@ static void print_usage(const char *help_str, bool output_on_stderr)
>  		   "  --skip-crc-compare\n"
>  		   "  --help-description\n"
>  		   "  --describe\n"
> +		   "  --device filter\n"
>  		   "  --help|-h\n");
>  	if (help_str)
>  		fprintf(f, "%s\n", help_str);
> @@ -733,6 +740,31 @@ static void common_init_config(void)
>  
>  	if (ret != 0)
>  		igt_set_autoresume_delay(ret);
> +
> +	/* Adding filters, order .igtrc, IGT_DEVICE, --device filter */
> +	if (igt_device_is_filter_set())
> +		igt_debug("Notice: using --device filters:\n");
> +	else {
> +		if (igt_rc_device) {
> +			igt_debug("Notice: using IGT_DEVICE env:\n");
> +		} else {
> +			igt_rc_device =	g_key_file_get_string(igt_key_file,
> +							      "Common",
> +							      "Device", &error);
> +			g_clear_error(&error);
> +			if (igt_rc_device)
> +				igt_debug("Notice: using .igtrc "
> +					  "Common::Device:\n");
> +		}
> +		if (igt_rc_device) {
> +			igt_device_filter_set(igt_rc_device);
> +			free(igt_rc_device);
> +			igt_rc_device = NULL;
> +		}
> +	}
> +
> +	if (igt_device_is_filter_set())
> +		igt_debug("[%s]\n",  igt_device_filter_get());

                                   ^  extra space


With that and the earlier comments,

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