Re: [PATCH i-g-t v3] lib/intel_chipset: Extend intel_get_pci_device to PCI accelerator class

["Dixit, Ashutosh" <ashutosh.dixit@intel.com>]

On Thu, 18 Jun 2026 13:30:49 -0700, Kamil Konieczny wrote:
>

Hi Kamil,

In v5 I have basically done what you've suggested below. Please check.

Thanks.
--
Ashutosh


> On 2026-06-16 at 07:54:58 -0700, Ashutosh Dixit wrote:
> > In addition to PCI display class devices, Intel devices can also be PCI
> > accelerator class devices. Extend intel_get_pci_device() to also find these
> > devices.
> >
> > v2: Introduce intel_get_pci_device_disp for those tools who specifically
> >     want only display class devices (Kamiil)
> > v3: s/intel_get_pci_device_disp/intel_get_pci_device_display/
> >
> > Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> > ---
> >  lib/intel_chipset.c             | 82 +++++++++++++++++++++++----------
> >  lib/intel_chipset.h             |  1 +
> >  tools/intel_audio_dump.c        |  2 +-
> >  tools/intel_backlight.c         |  2 +-
> >  tools/intel_display_bandwidth.c |  4 +-
> >  tools/intel_display_poller.c    |  4 +-
> >  tools/intel_panel_fitter.c      |  2 +-
> >  7 files changed, 65 insertions(+), 32 deletions(-)
> >
> > diff --git a/lib/intel_chipset.c b/lib/intel_chipset.c
> > index 760faede24..8bf3d52b77 100644
> > --- a/lib/intel_chipset.c
> > +++ b/lib/intel_chipset.c
> > @@ -63,16 +63,34 @@
> >   */
> >  enum pch_type intel_pch;
> >
> > -/**
> > - * intel_get_pci_device:
> > - *
> > - * Looks up the main graphics pci device using libpciaccess.
> > - *
> > - * Returns:
> > - * The pci_device, exits the program on any failures.
> > - */
> > -struct pci_device *
> > -intel_get_pci_device(void)
> > +static struct pci_device *intel_pci_device_by_class(uint8_t class)
> > +{
> > +	struct pci_device *pci_dev;
> > +	struct pci_device_iterator *iter;
> > +	struct pci_id_match match;
> > +	int error;
> > +
> > +	error = igt_pci_system_init();
> > +	igt_fail_on_f(error != 0, "Couldn't initialize PCI system\n");
> > +
> > +	match.vendor_id = 0x8086; /* Intel */
> > +	match.device_id = PCI_MATCH_ANY;
> > +	match.subvendor_id = PCI_MATCH_ANY;
> > +	match.subdevice_id = PCI_MATCH_ANY;
> > +
> > +	match.device_class = class << 16;
> > +	match.device_class_mask = 0xff << 16;
> > +
> > +	match.match_data = 0;
> > +
> > +	iter = pci_id_match_iterator_create(&match);
> > +	pci_dev = pci_device_next(iter);
> > +	pci_iterator_destroy(iter);
>
> Also there was a device probe, you need it here when
> pci_dev != NULL
>
> Add a check for vendor id, just like in previous implementation,
>	if (pci_dev && pci_dev->vendor_id != 0x8086)
>		err here
>
>
> > +
> > +	return pci_dev;
> > +}
> > +
> > +static struct pci_device *__intel_get_pci_device(bool accel)
>
> Please remove this function, see below.
>
> >  {
> >	struct pci_device *pci_dev;
> >	int error;
> > @@ -85,22 +103,12 @@ intel_get_pci_device(void)
> >	 * walk the entire PCI bus for a matching device. */
> >	pci_dev = pci_device_find_by_slot(0, 0, 2, 0);
> >	if (pci_dev == NULL || pci_dev->vendor_id != 0x8086) {
> > -		struct pci_device_iterator *iter;
> > -		struct pci_id_match match;
> > +		/* PCI display class (0x3) devices */
> > +		pci_dev = intel_pci_device_by_class(0x3);
> >
> > -		match.vendor_id = 0x8086; /* Intel */
> > -		match.device_id = PCI_MATCH_ANY;
> > -		match.subvendor_id = PCI_MATCH_ANY;
> > -		match.subdevice_id = PCI_MATCH_ANY;
> > -
> > -		match.device_class = 0x3 << 16;
> > -		match.device_class_mask = 0xff << 16;
> > -
> > -		match.match_data = 0;
> > -
> > -		iter = pci_id_match_iterator_create(&match);
> > -		pci_dev = pci_device_next(iter);
> > -		pci_iterator_destroy(iter);
> > +		if (!pci_dev && accel)
> > +			/* PCI accelerator class (0x12) devices */
> > +			pci_dev = intel_pci_device_by_class(0x12);
> >	}
> >	igt_require_f(pci_dev, "Couldn't find Intel graphics card\n");
> >
> > @@ -114,6 +122,30 @@ intel_get_pci_device(void)
> >	return pci_dev;
> >  }
> >
> > +/**
> > + * intel_get_pci_device:
> > + *
> > + * Looks up display and accelerator class pci devices using libpciaccess
> > + *
> > + * Returns: The pci_device, exits the program on any failures
> > + */
> > +struct pci_device *intel_get_pci_device(void)
> > +{
>
> Just encode it here:
>	struct pci_device *pci_dev = intel_pci_device_by_class(0x3);
>
>	if (!pci_dev)
>		pci_dev = intel_pci_device_by_class(0x12);
>
>	if (!pci_dev) {
>		drop error here just like in previous implementation
>	}
>
>	return pci_dev;
> > +}
> > +
> > +/**
> > + * intel_get_pci_device_disp:
>
> s/disp/display/
>
> > + *
> > + * Looks up display class pci device using libpciaccess
> > + *
> > + * Returns: The pci_device, exits the program on any failures
> > + */
> > +struct pci_device *intel_get_pci_device_display(void)
> > +{
> > +	return __intel_get_pci_device(false);
>
> Here use helper directly:
>
>	pci_dev = intel_pci_device_by_class(0x3);
>
> and add error check igt_require(...)
>
> Regards,
> Kamil
>
> > +}
> > +
> >  static uint32_t __i915_get_drm_devid(int fd)
> >  {
> >	struct drm_i915_getparam gp;
> > diff --git a/lib/intel_chipset.h b/lib/intel_chipset.h
> > index 3fcc5b18d6..018cfaba5e 100644
> > --- a/lib/intel_chipset.h
> > +++ b/lib/intel_chipset.h
> > @@ -36,6 +36,7 @@
> >  #define BIT(x) (1ul <<(x))
> >
> >  struct pci_device *intel_get_pci_device(void);
> > +struct pci_device *intel_get_pci_device_display(void);
> >  uint32_t intel_get_drm_devid(int fd);
> >
> >  struct intel_device_info {
> > diff --git a/tools/intel_audio_dump.c b/tools/intel_audio_dump.c
> > index 287dbd4759..5761fc1bbc 100644
> > --- a/tools/intel_audio_dump.c
> > +++ b/tools/intel_audio_dump.c
> > @@ -2468,7 +2468,7 @@ int main(int argc, char **argv)
> >	struct pci_device *pci_dev;
> >	struct intel_mmio_data mmio_data;
> >
> > -	pci_dev = intel_get_pci_device();
> > +	pci_dev = intel_get_pci_device_display();
> >	devid = pci_dev->device_id; /* XXX not true when mapping! */
> >
> >	do_self_tests();
> > diff --git a/tools/intel_backlight.c b/tools/intel_backlight.c
> > index edf060224f..1478583e97 100644
> > --- a/tools/intel_backlight.c
> > +++ b/tools/intel_backlight.c
> > @@ -41,7 +41,7 @@ int main(int argc, char** argv)
> >	struct intel_mmio_data mmio_data;
> >	uint32_t current, max;
> >
> > -	intel_mmio_use_pci_bar(&mmio_data, intel_get_pci_device());
> > +	intel_mmio_use_pci_bar(&mmio_data, intel_get_pci_device_display());
> >
> >	current = INREG(BLC_PWM_CPU_CTL) & BACKLIGHT_DUTY_CYCLE_MASK;
> >	max = INREG(BLC_PWM_PCH_CTL2) >> 16;
> > diff --git a/tools/intel_display_bandwidth.c b/tools/intel_display_bandwidth.c
> > index b798f8b652..193f309d39 100644
> > --- a/tools/intel_display_bandwidth.c
> > +++ b/tools/intel_display_bandwidth.c
> > @@ -152,14 +152,14 @@ int main(int argc, char *argv[])
> >		}
> >	}
> >
> > -	devid = intel_get_pci_device()->device_id;
> > +	devid = intel_get_pci_device_display()->device_id;
> >
> >	if (!has_de_power2(devid)) {
> >		fprintf(stderr, "Display bandwidth counter not available\n");
> >		return 2;
> >	}
> >
> > -	intel_register_access_init(&mmio_data, intel_get_pci_device(), 0);
> > +	intel_register_access_init(&mmio_data, intel_get_pci_device_display(), 0);
> >
> >	if (has_de_power2_abox0_abox1(devid))
> >		measure_de_power2_abox0_abox1(devid, sleep_duration);
> > diff --git a/tools/intel_display_poller.c b/tools/intel_display_poller.c
> > index 6338f22223..df333597e7 100644
> > --- a/tools/intel_display_poller.c
> > +++ b/tools/intel_display_poller.c
> > @@ -1508,7 +1508,7 @@ int main(int argc, char *argv[])
> >		}
> >	}
> >
> > -	devid = intel_get_pci_device()->device_id;
> > +	devid = intel_get_pci_device_display()->device_id;
> >
> >	/*
> >	 * check if the requires registers are
> > @@ -1677,7 +1677,7 @@ int main(int argc, char *argv[])
> >		break;
> >	}
> >
> > -	intel_register_access_init(&mmio_data, intel_get_pci_device(), 0);
> > +	intel_register_access_init(&mmio_data, intel_get_pci_device_display(), 0);
> >
> >	if (auto_scanline_offset)
> >		scanline_offset = default_scanline_offset(devid, pipe);
> > diff --git a/tools/intel_panel_fitter.c b/tools/intel_panel_fitter.c
> > index d9555d5c67..2234eac6c4 100644
> > --- a/tools/intel_panel_fitter.c
> > +++ b/tools/intel_panel_fitter.c
> > @@ -280,7 +280,7 @@ int main (int argc, char *argv[])
> >	       "with overscan compensation properties: it is just a temporary "
> >	       "solution that may or may not work. Use it at your own risk.\n");
> >
> > -	pci_dev = intel_get_pci_device();
> > +	pci_dev = intel_get_pci_device_display();
> >	intel_register_access_init(&mmio_data, pci_dev, 0);
> >	devid = pci_dev->device_id;
> >
> > --
> > 2.54.0
> >