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

[Arkadiusz Hiler]

On Mon, Nov 18, 2019 at 02:14:04PM +0200, Petri Latvala wrote:
> On Thu, Oct 24, 2019 at 02:05:15PM +0300, Arkadiusz Hiler wrote:
> > From: Zbigniew Kempczyński <zbigniew.kempczynski at 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 at intel.com>
> > Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
> > Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler at 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.
> 
> Cards can do what?

Right, it should have said "fleeing"...

"A helper to ge tthe first fleeing card"

> 'matching' might be more appropriate.

Ah, right :-)

> > + * 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);
> >  }
> 
> I'm a little confused how this all works with IGT_FORCE_DRIVER?

IGT_FORCE_DRIVER will work if no filter is used. We may consider
deprecating this later on.

> And how does this work for tests that open two different drivers?

This will break, but this is IMO ok and they are the prefect thing to
get us working on API for opening multiple devices :-)

There was some proposal in the original patches by Zbigniew, but IMO the
API requires a little bit more work and though from a consumer POV
before getting it in.