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

Wed Jul 3 10:10:03 UTC 2019

On Wed, 2019-07-03 at 09:45 +0100, Vasilev, Oleg wrote:
> 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.

Code has been written to allow different filter types, so I added that
filter because it differs from 'drm' filter. I wanted also expose that
code is easily extensible. From my point of view 'pci' filter will be
used, but adding new filter should be done easily. 

> 
> > 
> > > > 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'. 

For Intel there's no problem. We have single vendor id. AMD has two 
vendor id registered in PCI vendor space - 1002 and 1022 so we I have
to add separate alias for each one (device id can equal). I guess 
'amdati' and 'amd' regarding pci.ids would be ok.  

> > > 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 at intel.com>
> > > > Cc: Arkadiusz Hiler <arkadiusz.hiler at intel.com>
> > > > Cc: Daniel Vetter <daniel at ffwll.ch>
> > > > Cc: Petri Latvala <petri.latvala at 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

Adding separate --device2, --device3 is rigid but I won't complain. If 
you think it is better than for example:
--device ';pci=...', which allows also would allow automatic selection 
for primary device. From my point of view ';' separating selectors are
more flexible solution.

Zbigniew