[igt-dev] [PATCH i-g-t v4 3/6] lib/intel_mmio: use igt_device_map_pci_bar_region for mmap if fd is known.

[Chris Wilson]

Quoting Daniel Mrzyglod (2019-04-25 21:58:19)
>  void
> -intel_mmio_use_pci_bar(struct pci_device *pci_dev)
> +intel_mmio_use_pci_bar(struct pci_device *pci_dev, int fd)
>  {
>         uint32_t devid, gen;
>         int mmio_bar, mmio_size;
> @@ -135,14 +136,28 @@ intel_mmio_use_pci_bar(struct pci_device *pci_dev)
>         else
>                 mmio_size = 2*1024*1024;
>  
> -       error = pci_device_map_range (pci_dev,
> -                                     pci_dev->regions[mmio_bar].base_addr,
> -                                     mmio_size,
> -                                     PCI_DEV_MAP_FLAG_WRITABLE,
> -                                     &igt_global_mmio);
> +       /* It's for some legacy tools existing in tree that require
> +        * working without driver
> +        */
> +
> +       if (fd == -1) {
> +               error = pci_device_map_range(pci_dev,
> +                                            pci_dev->regions[mmio_bar].base_addr,
> +                                            mmio_size,
> +                                            PCI_DEV_MAP_FLAG_WRITABLE,
> +                                            &igt_global_mmio);
> +
> +               igt_fail_on_f(error != 0, "Couldn't map MMIO region\n");
> +       } else {
> +               /* This method is much more convenient when we have many
> +                * concurrent PCI devices
> +                */
> +               igt_global_mmio = igt_device_map_pci_bar_region(fd, mmio_bar,
> +                                                               mmio_size);
> +               igt_fail_on_f(mmio_data->igt_mmio == MAP_FAILED,
> +                             "Couldn't map MMIO region\n");

This makes no sense to me at all. We already have the pci_device we
want and the BAR we want to mmap, so why repeat ourselves?

If you didn't want that pci_dev, why are we passing around the wrong
one?
-Chris