* [PATCH 04/11] video/aperture: use generic code to figure out the vga default device [not found] <20230111154112.90575-1-daniel.vetter@ffwll.ch> @ 2023-01-11 15:41 ` Daniel Vetter 2023-01-11 15:59 ` Thomas Zimmermann 0 siblings, 1 reply; 3+ messages in thread From: Daniel Vetter @ 2023-01-11 15:41 UTC (permalink / raw) To: DRI Development Cc: Intel Graphics Development, LKML, Daniel Vetter, Daniel Vetter, Thomas Zimmermann, Javier Martinez Canillas, Helge Deller, linux-fbdev, Bjorn Helgaas, linux-pci 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. Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Cc: Thomas Zimmermann <tzimmermann@suse.de> Cc: Javier Martinez Canillas <javierm@redhat.com> Cc: Helge Deller <deller@gmx.de> Cc: linux-fbdev@vger.kernel.org Cc: Bjorn Helgaas <bhelgaas@google.com> Cc: linux-pci@vger.kernel.org --- 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 41e77de1ea82..3d8c925c7365 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(); for (bar = 0; bar < PCI_STD_NUM_BARS; ++bar) { if (!(pci_resource_flags(pdev, bar) & IORESOURCE_MEM)) -- 2.39.0 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 04/11] video/aperture: use generic code to figure out the vga default device 2023-01-11 15:41 ` [PATCH 04/11] video/aperture: use generic code to figure out the vga default device Daniel Vetter @ 2023-01-11 15:59 ` Thomas Zimmermann 2023-01-11 16:51 ` Daniel Vetter 0 siblings, 1 reply; 3+ messages in thread From: Thomas Zimmermann @ 2023-01-11 15:59 UTC (permalink / raw) To: Daniel Vetter, DRI Development Cc: Intel Graphics Development, LKML, Daniel Vetter, Javier Martinez Canillas, Helge Deller, linux-fbdev, Bjorn Helgaas, linux-pci [-- Attachment #1.1: Type: text/plain, Size: 2779 bytes --] Hi Am 11.01.23 um 16:41 schrieb Daniel Vetter: > 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. > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > Cc: Thomas Zimmermann <tzimmermann@suse.de> > Cc: Javier Martinez Canillas <javierm@redhat.com> > Cc: Helge Deller <deller@gmx.de> > Cc: linux-fbdev@vger.kernel.org > Cc: Bjorn Helgaas <bhelgaas@google.com> > Cc: linux-pci@vger.kernel.org > --- > 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 41e77de1ea82..3d8c925c7365 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(); vga_default_device() is part of vgaarb and can return NULL. [1] That new test is likely to be incorrect on many systems. I suggest to implement a helper like fb_is_primary_device() on x86: it uses the default VGA if set, or falls back to the original test. [2] It's noteworthy that on most architectures, fb_is_primary_device() returns 0. But at least on Sparc [3] and some Parisc [4] machines, it does not. I've long wanted to rework this helper anyway. So this is a good opportunity. Best regards Thomas [1] https://elixir.bootlin.com/linux/latest/source/include/linux/vgaarb.h#L69 [2] https://elixir.bootlin.com/linux/latest/source/arch/x86/video/fbdev.c#L14 [3] https://elixir.bootlin.com/linux/latest/source/arch/sparc/include/asm/fb.h#L18 [4] https://elixir.bootlin.com/linux/latest/source/drivers/video/console/sticore.c#L1153 > > for (bar = 0; bar < PCI_STD_NUM_BARS; ++bar) { > if (!(pci_resource_flags(pdev, bar) & IORESOURCE_MEM)) -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 840 bytes --] ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 04/11] video/aperture: use generic code to figure out the vga default device 2023-01-11 15:59 ` Thomas Zimmermann @ 2023-01-11 16:51 ` Daniel Vetter 0 siblings, 0 replies; 3+ messages in thread From: Daniel Vetter @ 2023-01-11 16:51 UTC (permalink / raw) To: Thomas Zimmermann Cc: Daniel Vetter, DRI Development, Intel Graphics Development, LKML, Daniel Vetter, Javier Martinez Canillas, Helge Deller, linux-fbdev, Bjorn Helgaas, linux-pci On Wed, Jan 11, 2023 at 04:59:30PM +0100, Thomas Zimmermann wrote: > Hi > > Am 11.01.23 um 16:41 schrieb Daniel Vetter: > > 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. > > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > Cc: Thomas Zimmermann <tzimmermann@suse.de> > > Cc: Javier Martinez Canillas <javierm@redhat.com> > > Cc: Helge Deller <deller@gmx.de> > > Cc: linux-fbdev@vger.kernel.org > > Cc: Bjorn Helgaas <bhelgaas@google.com> > > Cc: linux-pci@vger.kernel.org > > --- > > 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 41e77de1ea82..3d8c925c7365 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(); > > vga_default_device() is part of vgaarb and can return NULL. [1] That new > test is likely to be incorrect on many systems. > > I suggest to implement a helper like fb_is_primary_device() on x86: it uses > the default VGA if set, or falls back to the original test. [2] > > It's noteworthy that on most architectures, fb_is_primary_device() returns > 0. But at least on Sparc [3] and some Parisc [4] machines, it does not. Afaik these neither do legacy vga nor sysfb, so we should be fine. I'll augment the commit message. > I've long wanted to rework this helper anyway. So this is a good > opportunity. Hm yeah that sounds like a good thing to copy. I'm honestly not sure whether it's needed, but at least it shouldn't hurt. I thought at least that the boot default device should be set on all pci architectures. I'll also reorder this with the previous patch to avoid the compile fail. -Daniel > > Best regards > Thomas > > [1] > https://elixir.bootlin.com/linux/latest/source/include/linux/vgaarb.h#L69 > [2] > https://elixir.bootlin.com/linux/latest/source/arch/x86/video/fbdev.c#L14 > [3] https://elixir.bootlin.com/linux/latest/source/arch/sparc/include/asm/fb.h#L18 > [4] https://elixir.bootlin.com/linux/latest/source/drivers/video/console/sticore.c#L1153 > > > > for (bar = 0; bar < PCI_STD_NUM_BARS; ++bar) { > > if (!(pci_resource_flags(pdev, bar) & IORESOURCE_MEM)) > > -- > Thomas Zimmermann > Graphics Driver Developer > SUSE Software Solutions Germany GmbH > Maxfeldstr. 5, 90409 Nürnberg, Germany > (HRB 36809, AG Nürnberg) > Geschäftsführer: Ivo Totev -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-01-11 16:52 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20230111154112.90575-1-daniel.vetter@ffwll.ch> 2023-01-11 15:41 ` [PATCH 04/11] video/aperture: use generic code to figure out the vga default device Daniel Vetter 2023-01-11 15:59 ` Thomas Zimmermann 2023-01-11 16:51 ` Daniel Vetter
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).