Daniel Vetter <daniel.vetter@xxxxxxxx> writes:
> Since vgaarb has been promoted to be a core piece of the pci subsystem
> we don't have to open code random guesses anymore, we actually know
> this in a platform agnostic way, and there's no need for an x86
> specific hack. See also 1d38fe6ee6a8 ("PCI/VGA: Move vgaarb to
> drivers/pci")
>
> This should not result in any functional change, and the non-x86
> multi-gpu pci systems are probably rare enough to not matter (I don't
> know of any tbh). But it's a nice cleanup, so let's do it.
>
> There's been a few questions on previous iterations on dri-devel and
> irc:
>
> - fb_is_primary_device() seems to be yet another implementation of
> this theme, and at least on x86 it checks for both
> vga_default_device OR rom shadowing. There shouldn't ever be a case
> where rom shadowing gives any additional hints about the boot vga
> device, but if there is then the default vga selection in vgaarb
> should probably be fixed. And not special-case checks replicated all
> over.
>
Agreed and if there are regressions reported then could be added there.
> - Thomas also brought up that on most !x86 systems
> fb_is_primary_device() returns 0, except on sparc/parisc. But these
> 2 special cases are about platform specific devices and not pci, so
> shouldn't have any interactions.
>
> - Furthermore fb_is_primary_device() is a bit a red herring since it's
> only used to select the right fbdev driver for fbcon, and not for
> the fw handover dance which the aperture helpers handle. At least
> for x86 we might want to look into unifying them, but that's a
> separate thing.
>
> v2: Extend commit message trying to summarize various discussions.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx>
> Cc: Thomas Zimmermann <tzimmermann@xxxxxxx>
> Cc: Javier Martinez Canillas <javierm@xxxxxxxxxx>
> Cc: Helge Deller <deller@xxxxxx>
> Cc: linux-fbdev@xxxxxxxxxxxxxxx
> Cc: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
> Cc: linux-pci@xxxxxxxxxxxxxxx
> ---
> drivers/video/aperture.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/video/aperture.c b/drivers/video/aperture.c
> index b009468ffdff..8835d3bc39bf 100644
> --- a/drivers/video/aperture.c
> +++ b/drivers/video/aperture.c
> @@ -324,13 +324,11 @@ EXPORT_SYMBOL(aperture_remove_conflicting_devices);
> */
> int aperture_remove_conflicting_pci_devices(struct pci_dev *pdev, const char *name)
> {
> - bool primary = false;
> + bool primary;
> resource_size_t base, size;
> int bar, ret;
>
> -#ifdef CONFIG_X86
> - primary = pdev->resource[PCI_ROM_RESOURCE].flags & IORESOURCE_ROM_SHADOW;
> -#endif
> + primary = pdev == vga_default_device();
>
Maybe enclose the check in parenthesis to make it easier to read ?
Reviewed-by: Javier Martinez Canillas <javierm@xxxxxxxxxx>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
