Re: [PATCH i-g-t v1 2/2] tools/intel_reg: Create default accelerator access

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

On Thu, 18 Jun 2026 04:50:36 -0700, Kamil Konieczny wrote:
>
> Hi Dixit,,
> On 2026-06-17 at 15:09:53 -0700, Dixit, Ashutosh wrote:
> > On Wed, 17 Jun 2026 13:25:58 -0700, Kamil Konieczny wrote:
> > >
> > > Among computing devices there are GPU and accelerators and the
> > > intel_reg tool worked only with former ones. Create new way for
> > > finding a compute device and when no Intel GPU is found, then
> > > search for Intel accelerator. Also, inform user about which type
> > > of device will be accessed.
> > >
> > > Cc: Ashutosh Dixit <ashutosh.dixit@intel.com>
> > > Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> > > ---
> > >  tools/intel_reg.c | 12 ++++++++++++
> > >  1 file changed, 12 insertions(+)
> > >
> > > diff --git a/tools/intel_reg.c b/tools/intel_reg.c
> > > index 49afe91c0..b4f8cd0fd 100644
> > > --- a/tools/intel_reg.c
> > > +++ b/tools/intel_reg.c
> > > @@ -1382,6 +1382,18 @@ int main(int argc, char *argv[])
> > >				return EXIT_FAILURE;
> > >		} else {
> > >			config.pci_dev = intel_get_pci_device();
> > > +			if (config.pci_dev) {
> > > +				fprintf(stderr, "Found Intel GPU PCI device 0x%x\n", config.pci_dev->device_id);
> > > +			} else {
> > > +				config.pci_dev = intel_get_pci_accelerator_device();
> > > +				if (config.pci_dev)
> > > +					fprintf(stderr, "Found Intel accelerator PCI device 0x%x\n", config.pci_dev->device_id);
> > > +			}
> > > +
> > > +			if (!config.pci_dev) {
> > > +				fprintf(stderr, "Cannot find Intel GPU nor accelerator device\n");
> > > +				return EXIT_FAILURE;
> > > +			}
> >
> > So what I am not following is why you want to preserve
> > intel_get_pci_device() to just display class devices? In
> > https://patchwork.freedesktop.org/series/168385/ intel_get_pci_device() is
> > extended to both display and acceleretor class devices. So this kind of
> > modification to the individual tools is not needed at all.
>
> But existing tools willl not work for accelerators?
> They are meant for display/GPU only.
> cd tools
> grep -l intel_get_pci_device *c
> intel_audio_dump.c
> intel_backlight.c
> intel_display_bandwidth.c
> intel_display_poller.c
> intel_forcewaked.c
> intel_gpu_time.c
> intel_gtt.c
> intel_infoframes.c
> intel_lid.c
> intel_panel_fitter.c
> intel_reg.c
> intel_reg_checker.c
> intel_watermark.c

https://patchwork.freedesktop.org/patch/733334/?series=168385&rev=4

According to me only the tools calling intel_get_pci_device_display() work
only with display/gpu.

See also my comments here:
https://lore.kernel.org/igt-dev/87wlvyzbn0.wl-ashutosh.dixit@intel.com/

> Which ones from above could work? Or can you add to cc few devs
> from Intel accelerators drivers and ask him/her for opinion?

I don't think we should be spending time refining this at this time. If
something is not completely right it can be fixed later. We just need to
provide the correct lib functions in lib/ at this time.

> >
> > Note that it is not just intel_reg. Similar modification will need to all
> > tools which currently call intel_get_pci_device(). They will now all have
> > to be changed to add the intel_get_pci_accelerator_device() calls,
> > something which is not needed with
> > https://patchwork.freedesktop.org/series/168385/ (especially rev1 of that
> > series). Because we would want most of these tools to "just work" with any
> > intel device (display or accelerator).
> >
> > See also my comments here:
> > https://lore.kernel.org/igt-dev/87wlvyzbn0.wl-ashutosh.dixit@intel.com/
> >
> > Anyway, let's see what other reviewers have to say about this.
>
> Well, I just do not like indeterminancy of that function, it was
> returning whatever a first device was on PCI bus. When it
> searched for display only it was ok, now you never know which
> one will be.
> Second no-go is that accelerators could be non-GPU based and
> then almost all of our tools will not work.

Note that accelerators are mostly the same as GPU, except maybe for display
part, so your assumption "almost all of our tools will not work" is wrong.

In any case, if you want to keep intel_get_pci_device() for display, you
need to provide a function, such as intel_get_pci_device_any(), which will
work for both display and accelerator. We can't have each tool modified to
add this code, this should be provided as a function in lib/.

I prefer this naming scheme:

- intel_get_pci_device() or intel_get_pci_device_any(): for both display and
  accelerator

- intel_get_pci_device_display() for display.

- intel_get_pci_device_accelerator() for accelerator (according to me there
  is no use cases for only accelerator at this time).

> >
> > Thanks.
> > --
> > Ashutosh
> >
> > >		}
> > >
> > >		config.devid = config.pci_dev->device_id;
> > > --
> > > 2.54.0
> > >