[PATCH i-g-t v8 1/8] lib/igt_device_scan: Introduce filtering out non-PCI devices

[Kamil Konieczny]

Hi Zbigniew,
On 2024-02-09 at 09:54:59 +0100, Zbigniew Kempczyński wrote:
> On Thu, Feb 08, 2024 at 02:52:25PM +0100, Kamil Konieczny wrote:
> > From: Dominik Karol Piątkowski <dominik.karol.piatkowski at intel.com>
> > 
> > Introduced igt_device_filter_pci function that filters out non-PCI
> > devices from IGT devices. This will help with checking multi-GPU
> > tests after they skip due to detected only one GPU, in that case
> > one can verify it after printing what GPUs are really on PCI bus.
> > Such errors can happen when using wrong env IGT_DEVICE filter.
> > 
> > v6: extending description
> > 
> > Cc: Chris Wilson <chris.p.wilson at linux.intel.com>
> > Cc: Janusz Krzysztofik <janusz.krzysztofik at linux.intel.com>
> > Signed-off-by: Dominik Karol Piątkowski <dominik.karol.piatkowski at intel.com>
> > Signed-off-by: Kamil Konieczny <kamil.konieczny at linux.intel.com>
> > ---
> >  lib/igt_device_scan.c | 21 +++++++++++++++++++++
> >  lib/igt_device_scan.h |  2 ++
> >  2 files changed, 23 insertions(+)
> > 
> > diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
> > index fbf3fa482..37ebe1964 100644
> > --- a/lib/igt_device_scan.c
> > +++ b/lib/igt_device_scan.c
> > @@ -1908,6 +1908,27 @@ const char *igt_device_filter_get(int num)
> >  	return NULL;
> >  }
> >  
> > +/**
> > + * igt_device_filter_pci
> > + *
> > + * Filter devices to PCI only.
> > + *
> > + * Returns PCI devices count.
> > + */
> > +int igt_device_filter_pci(void)
> > +{
> > +	int count = 0;
> > +	struct igt_device *dev, *tmp;
> > +
> > +	igt_list_for_each_entry_safe(dev, tmp, &igt_devs.filtered, link)
> > +		if (strcmp(dev->subsystem, "pci") != 0)
> > +			igt_list_del(&dev->link);
> 
> Device scan starts with lists 'all' and 'filtered' filled with
> all cards. If you want to eliminate non-pci cards here you should
> also free memory for dev. See igt_device_filter_apply() where
> there's comment 'Clean the filtered list'.
> 

Thank you for spotting this, I will fix it.

> > +		else
> > +			count++;
> > +
> > +	return count;
> > +}
> > +
> 
> I also wondered to fix filter_pci() and fill filtered list
> with all pci devices when filter 'pci:' (without vendor/device/card)
> is passed. Currently filter picks single matching device instead
> of filling filtered list with all matching devices.
> 
> Anyway, at the moment I see no blocker for your code. Just
> fix this minor free() nit and:
> 
> Reviewed-by: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>

I will add you r-b with the fix from above.

Regards,
Kamil

> 
> --
> Zbigniew
> 
> >  static bool igt_device_filter_apply(const char *fstr)
> >  {
> >  	struct igt_device *dev, *tmp;
> > diff --git a/lib/igt_device_scan.h b/lib/igt_device_scan.h
> > index 48690e236..908733745 100644
> > --- a/lib/igt_device_scan.h
> > +++ b/lib/igt_device_scan.h
> > @@ -81,6 +81,8 @@ int igt_device_filter_add(const char *filter);
> >  void igt_device_filter_free_all(void);
> >  const char *igt_device_filter_get(int num);
> >  
> > +int igt_device_filter_pci(void);
> > +
> >  /* Use filter to match the device and fill card structure */
> >  bool igt_device_card_match(const char *filter, struct igt_device_card *card);
> >  bool igt_device_card_match_pci(const char *filter,
> > -- 
> > 2.42.0
> >