Re: [igt-dev] [EARLY RFC 1/1] Introduce new method of device selection

Vasilev, Oleg
2019-07-03T08:45:07Z

[-- Attachment #1.1: Type: text/plain, Size: 5247 bytes --]

On Tue, 2019-07-02 at 08:38 +0100, Kempczynski, Zbigniew wrote:
> On Mon, 2019-07-01 at 21:57 +0100, Vasilev, Oleg wrote:
> > Hi,
> >
> > First of all, thanks for working on this. This will definitely be
> > useful.
> >
> > On Mon, 2019-07-01 at 15:03 +0200, Zbigniew Kempczyński wrote:
> > > Change adds device selection based on implemented filters.
> > > Currently
> > > drm, dev and pci filters were added to address different device
> > > selection possibilities.
> > >
> > > drm filter allows selection /dev/dri/cardN node by using filter
> > > drm:card=N
> > > where N is number of card.
> > >
> > > dev filter allows selection of N-th device in all gpus avilable
> > > (PCI devices at the moment). Regardless of driver load time
> > > and /dev/dri/cardN this filter will select same device until
> > > new card will be added. Filter syntax is as follows:
> > > dev:card=N
> >
> > I am not sure I understand the purpose of this method. The first is
> > mostly for debugging. The third one is used to specify the exact
> > device. Why do we need this?
>
> Let we have two cards, first on pci slot 0000:00:01.0, second
> on 0000:00:02.0. Driver module load time makes influence which device
> will have /dev/dri/card0 and which /dev/dri/card1.
> Filter 'drm:card=0' always will return you /dev/dri/card0.
> Filter 'dev:card=0' will return you /dev/dri/card0 or /dev/dri/card1
> because it will find first card on PCI bus.

Nah, I still don't like it much. It wouldn't work for hotplug devices.
But if someone would use it for local debugging, I don't mind.

>
> > > pci filter gives vendor / device / card number selection. Only
> > > cards which match this filter will be returned. Filter syntax:
> > > pci:vendor=xxxx,device=yyyy,card=N
> > > where xxxx, yyyy are hex numbers which identify vendor and
> > > device,
> > > card selects N-th matching card.
> > > New device selection uses --device filter or IGT_DEVICE
> > > environent
> > > variable.
> >
> > I think, we are still missing a method for specifying the device in
> > a
> > human-readable format. Is there any reliable way of projecting
> > strings
> > like "intel", "amd", "vkms", etc to an actual device, or we still
> > need
> > to maintain this mapping inside IGT runner?
>
> Adding '8086' -> 'intel' mapping and so on is easy. Small problem
> occurs
> for handling device, especially that set is larger and how to select
> that
> (string matching, what to do if device is not on the list?). I'd
> crazy idea
> to convert pci.ids to table but this is overkill IMHO for this case.
> And in
> this case we need to put data to library in compiled form because I
> guess we
> don't want to depend on pciutils package (or whichever contains
> pci.ids).

I think, maintaining this table inside IGT should be fine. We have a
finite number of vendors, who want to run IGT on their devices.

I suppose, we can even incorporate those human-readable names into the
same syntax, such that 'pci:vendor=intel' would be an alias to
'pci:vendor=8086'.

> > Also, should IGT_FORCE_DRIVER be kept? Seems like IGT_FORCE_DRIVER
> > and
> > IGT_DEVICE serve quite a similar purpose.
>
> IGT_FORCE_DRIVER ensures that this driver module is used for device
> opened with DRIVER_ANY. I didn't wanted to change this behavior for
> legacy
> usage. That's why I proposed new IGT_DEVICE to leave old device
> selection
> intact.

I think IGT_FORCE_DRIVER is not broadly used, and can be dropped. But
let others clarify this.

> > > Tool 'lsgpu' which uses device scanning feature was added.
> > >
> > > Signed-off-by: Zbigniew Kempczyński <
> > > zbigniew.kempczynski@intel.com>
> > > Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > > Cc: Daniel Vetter <daniel@ffwll.ch>
> > > Cc: Petri Latvala <petri.latvala@intel.com>
> >
> > BTW, about multiple devices tests and
> > drm_open_driver(DRIVER_INTEL).
> > One approach could be this:
> >
> > 1. We add another instance of this --device property, called for
> > example --device2, which would correspond to an auxiliary device.
> > This
> > approach allows us to have the same generic device selection
> > mechanism
> > in all places. Also, if we ever have the test involving 3 devices,
> > this
> > can scale :)
> > 2. With new selection mechanism active
> > fd = drm_open_driver(DRIVER_SOMETHING)
> > is an alias for
> > fd = drm_open_driver(DRIVER_ANY)
> > igt_require_something(fd)
> > Oleg
>
> If we need to handle more than one device it is possible to
> reimplement
> parsing of --device filter to something like this:
>
> --device pci:vendor=1234,card=0;pci:vendor=2345,card=2;...
>
> Splitting against ';' would extract separate device filters giving
> array
> as an output. Then access to device using array index would give you
> expected card.

I would rather have separate parameters/envvars for different devices.
It is more readable, we easier to copy-paste parameters, envvars for
different devices can come from different places. We might want to
specify only secondary device, and for primary rely on automatic
selection

> Zbigniew

Oleg

[-- Attachment #1.2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3261 bytes --]

[-- Attachment #2: Type: text/plain, Size: 153 bytes --]