Skip to contentSkip to conversation

vgaarb: Add support for setting the default video device

PATCH In review Matthew Garrett 5 participants 11 messages Apr 16, 2012 – Apr 20, 2012

Subsystems (6)
DRM drivers x86 DRM misc PCI +2 more subsystems
lore
Share & tools
lore

Matthew Garrett

author
#0 lore raw

PATCH v2 1/4 vgaarb: Add support for setting the default video device

The default VGA device is a somewhat fluid concept on platforms with multiple GPUs. Add support for setting it so switching code can update things appropriately, and make sure that the sysfs code returns the right device if it's changed.

Signed-off-by Matthew Garrett <[email protected]>
Cc [email protected]
Cc [email protected]

---
Updated to fix builds when __ARCH_HAS_VGA_DEFAULT_DEVICE is false.

 drivers/gpu/vga/vgaarb.c |    7 +++++++
 drivers/pci/pci-sysfs.c  |    5 +++++
 include/linux/vgaarb.h   |    2 ++
 3 files changed, 14 insertions(+)
drivers/gpu/vga/vgaarb.c +7 -0
index 111d956..e223b96 100644
@@ -136,6 +136,11 @@ struct pci_dev *vga_default_device(void)
 {
 	return vga_default;
 }
+
+void vga_set_default_device(struct pci_dev *pdev)
+{
+	vga_default = pdev;
+}
 #endif
 
 static inline void vga_irq_set_state(struct vga_device *vgadev, bool state)

@@ -605,10 +610,12 @@ static bool vga_arbiter_del_pci_device(struct pci_dev *pdev)
 		goto bail;
 	}
 
+#ifndef __ARCH_HAS_VGA_DEFAULT_DEVICE
 	if (vga_default == pdev) {
 		pci_dev_put(vga_default);
 		vga_default = NULL;
 	}
+#endif
 
 	if (vgadev->decodes & (VGA_RSRC_LEGACY_IO | VGA_RSRC_LEGACY_MEM))
 		vga_decode_count--;
drivers/pci/pci-sysfs.c +5 -0
index a3cd8ca..7dd9f2b 100644
@@ -27,6 +27,7 @@
 #include <linux/security.h>
 #include <linux/pci-aspm.h>
 #include <linux/slab.h>
+#include <linux/vgaarb.h>
 #include "pci.h"
 
 static int sysfs_initialized;	/* = 0 */

@@ -414,6 +415,10 @@ static ssize_t
 boot_vga_show(struct device *dev, struct device_attribute *attr, char *buf)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
+	struct pci_dev *vga_dev = vga_default_device();
+
+	if (vga_dev)
+		return sprintf(buf, "%u\n", (pdev == vga_dev));
 
 	return sprintf(buf, "%u\n",
 		!!(pdev->resource[PCI_ROM_RESOURCE].flags &
include/linux/vgaarb.h +2 -0
index 9c3120d..759a25b 100644
@@ -31,6 +31,7 @@
 #ifndef LINUX_VGA_H
 #define LINUX_VGA_H
 
+#include <video/vga.h>
 
 /* Legacy VGA regions */
 #define VGA_RSRC_NONE	       0x00

@@ -181,6 +182,7 @@ extern void vga_put(struct pci_dev *pdev, unsigned int rsrc);
 
 #ifndef __ARCH_HAS_VGA_DEFAULT_DEVICE
 extern struct pci_dev *vga_default_device(void);
+extern void vga_set_default_device(struct pci_dev *pdev);
 #endif
 
 /**
Signature · 1 line
1.7.10
author
#1 lore raw

PATCH v2 2/4 vga-switcheroo: Use vga_default_device()

vga-switcheroo currently changes the default VGA device by fiddling with the IORESOURCE_ROM_SHADOW flag on the device. This isn't strictly accurate, since there's no guarantee that switching also changes the ROM decoding. Switch over to using the vgaarb functions for this.

Signed-off-by Matthew Garrett <[email protected]>
Cc [email protected]
---
 drivers/gpu/vga/vga_switcheroo.c |    9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
drivers/gpu/vga/vga_switcheroo.c +5 -4
index 9a2c805..44c664e 100644
@@ -28,6 +28,8 @@
 #include <linux/pci.h>
 #include <linux/vga_switcheroo.h>
 
+#include <linux/vgaarb.h>
+
 struct vga_switcheroo_client {
 	struct pci_dev *pdev;
 	struct fb_info *fb_info;

@@ -140,7 +142,7 @@ int vga_switcheroo_register_client(struct pci_dev *pdev,
 	vgasr_priv.clients[index].reprobe = reprobe;
 	vgasr_priv.clients[index].can_switch = can_switch;
 	vgasr_priv.clients[index].id = -1;
-	if (pdev->resource[PCI_ROM_RESOURCE].flags & IORESOURCE_ROM_SHADOW)
+	if (pdev == vga_default_device())
 		vgasr_priv.clients[index].active = true;
 
 	vgasr_priv.registered_clients |= (1 << index);

@@ -248,9 +250,8 @@ static int vga_switchto_stage1(struct vga_switcheroo_client *new_client)
 	if (new_client->pwr_state == VGA_SWITCHEROO_OFF)
 		vga_switchon(new_client);
 
-	/* swap shadow resource to denote boot VGA device has changed so X starts on new device */
-	active->pdev->resource[PCI_ROM_RESOURCE].flags &= ~IORESOURCE_ROM_SHADOW;
-	new_client->pdev->resource[PCI_ROM_RESOURCE].flags |= IORESOURCE_ROM_SHADOW;
+	vga_set_default_device(new_client->pdev);
+
 	return 0;
 }
Signature · 1 line
1.7.10
author
#2 lore raw

PATCH v2 3/4 x86: Use vga_default_device() when determining whether an fb is primary

In reply to Matthew Garrett

IORESOURCE_ROM_SHADOW is not necessarily an indication that the hardware is the primary device. Add support for using the vgaarb functions and fall back if nothing's set them.

Signed-off-by Matthew Garrett <[email protected]>
Cc [email protected]
Cc [email protected]
---
 arch/x86/video/fbdev.c |   20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)
arch/x86/video/fbdev.c +15 -5
index c5ffb6a..d5644bb 100644
@@ -9,24 +9,34 @@
 #include <linux/fb.h>
 #include <linux/pci.h>
 #include <linux/module.h>
+#include <linux/vgaarb.h>
 
 int fb_is_primary_device(struct fb_info *info)
 {
 	struct device *device = info->device;
 	struct pci_dev *pci_dev = NULL;
+	struct pci_dev *default_device = vga_default_device();
 	struct resource *res = NULL;
-	int retval = 0;
 
 	if (device)
 		pci_dev = to_pci_dev(device);
 
-	if (pci_dev)
-		res = &pci_dev->resource[PCI_ROM_RESOURCE];
+	if (!pci_dev)
+		return 0;
+
+	if (default_device) {
+		if (pci_dev == default_device)
+			return 1;
+		else
+			return 0;
+	}
+
+	res = &pci_dev->resource[PCI_ROM_RESOURCE];
 
 	if (res && res->flags & IORESOURCE_ROM_SHADOW)
-		retval = 1;
+		return 1;
 
-	return retval;
+	return 0;
 }
 EXPORT_SYMBOL(fb_is_primary_device);
 MODULE_LICENSE("GPL");
Signature · 1 line
1.7.10
author
#3 lore raw

PATCH v2 4/4 efifb: Implement vga_default_device()

In reply to Matthew Garrett

EFI doesn't typically make use of the legacy VGA ROM, but it may still be configured to pass that through to a given video device. This may lead to an inaccurate choice of default video device. Add support to efifb to pick out the correct active video device.

Signed-off-by Matthew Garrett <[email protected]>
Cc [email protected]
Cc [email protected]
---
 arch/x86/include/asm/vga.h |    6 ++++
 drivers/video/efifb.c      |   77 ++++++++++++++++++++++++++++++++------------
 2 files changed, 63 insertions(+), 20 deletions(-)
arch/x86/include/asm/vga.h +6 -0
index c4b9dc2..2723c07 100644
@@ -17,4 +17,10 @@
 #define vga_readb(x) (*(x))
 #define vga_writeb(x, y) (*(y) = (x))
 
+#if CONFIG_FB_EFI
+#define __ARCH_HAS_VGA_DEFAULT_DEVICE
+extern struct pci_dev *vga_default_device(void);
+extern void vga_set_default_device(struct pci_dev *pdev);
+#endif
+
 #endif /* _ASM_X86_VGA_H */
drivers/video/efifb.c +57 -20

98 lines not on this page · raw message

Signature · 1 line
1.7.10
maintainer
#4 lore raw

Re: [PATCH V2 4/4] efifb: Implement vga_default_device()

Quoted 18 lines · Matthew Garrett

EFI doesn't typically make use of the legacy VGA ROM, but it may still be configured to pass that through to a given video device. This may lead to an inaccurate choice of default video device. Add support to efifb to pick out the correct active video device.

Signed-off-by Matthew Garrett <[email protected]>
Cc [email protected]
Cc [email protected]

---
 arch/x86/include/asm/vga.h |    6 ++++
 drivers/video/efifb.c      |   77 ++++++++++++++++++++++++++++++++------------

 2 files changed, 63 insertions(+), 20 deletions(-)
arch/x86/include/asm/vga.h +0 -0
index c4b9dc2..2723c07 100644

arch/x86/include/asm/vga.h line 20

Quoted 4 lines
arch/x86/include/asm/vga.h +1 -0
@@ -17,4 +17,10 @@
  #define vga_readb(x) (*(x))
  #define vga_writeb(x, y) (*(y) = (x))
 
+#if CONFIG_FB_EFI

^ #ifdef?

Dave.

Acked-by
#6 lore raw
Quoted 4 lines · H. Peter Anvin
Acked-by H. Peter Anvin <[email protected]>

... on this patchset. I'd like to see it together though, so it's OK for me if Dave wants to take it, or I can take it.

Well, I was CCed so here's my Ack :-)

Acked-by Benjamin Herrenschmidt <[email protected]>

BTW. We should hack a version of vbetool that uses the arbiter interfaces ... not that I want vbetool to live but it's proved handy once or twice diagnosing problems...

Cheers,
Ben.

#7 lore raw
In reply to Matthew Garrett

include/linux/vgaarb.h line 34

Quoted 7 lines · Matthew Garrett
include/linux/vgaarb.h +1 -0
@@ -31,6 +31,7 @@
  #ifndef LINUX_VGA_H
  #define LINUX_VGA_H
 
+#include <video/vga.h>

Is this include really needed? x86_64/allmodconfig builds fine without it.

It causes m68k/allmodconfig to fail with:

In file included from include/linux/vgaarb.h:35,

                 from drivers/gpu/drm/drm_irq.c:43:

include/video/vga.h: In function ‘vga_mm_r’:
include/video/vga.h:242: error: implicit declaration of function ‘z_readb’ include/video/vga.h: In function ‘vga_mm_w’:
include/video/vga.h:247: error: implicit declaration of function ‘z_writeb’ include/video/vga.h: In function ‘vga_mm_w_fast’:
include/video/vga.h:253: error: implicit declaration of function ‘z_writew’ make[1]: *** [drivers/gpu/drm/drm_irq.o] Error 1

http://kisskb.ellerman.id.au/kisskb/buildresult/6127136/

Currently <video/vga.h> has a crude hack to make cirrusfb work on Amiga, but that's the only VGA support m68k has.
There's also no <asm/vga.h>, so m68k/allmodconfig with CONFIG_AMIGA disabled now fails with:

In file included from include/linux/vgaarb.h:34,

                 from drivers/gpu/drm/drm_irq.c:42:

include/video/vga.h:23:21: error: asm/vga.h: No such file or directory make[1]: *** [drivers/gpu/drm/drm_irq.o] Error 1

Removing the #include <video/vga.h> fixes both cases.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds

#8 lore raw
Quoted 30 lines · Geert Uytterhoeven

include/linux/vgaarb.h line 34

Quoted 7 lines · Matthew Garrett
include/linux/vgaarb.h +1 -0
@@ -31,6 +31,7 @@
  #ifndef LINUX_VGA_H
  #define LINUX_VGA_H
 
+#include <video/vga.h>

Is this include really needed? x86_64/allmodconfig builds fine without it.

It causes m68k/allmodconfig to fail with:

In file included from include/linux/vgaarb.h:35,
                from drivers/gpu/drm/drm_irq.c:43:
include/video/vga.h: In function ‘vga_mm_r’:
include/video/vga.h:242: error: implicit declaration of function ‘z_readb’ include/video/vga.h: In function ‘vga_mm_w’:
include/video/vga.h:247: error: implicit declaration of function ‘z_writeb’ include/video/vga.h: In function ‘vga_mm_w_fast’:
include/video/vga.h:253: error: implicit declaration of function ‘z_writew’ make[1]: *** [drivers/gpu/drm/drm_irq.o] Error 1

http://kisskb.ellerman.id.au/kisskb/buildresult/6127136/

Currently <video/vga.h> has a crude hack to make cirrusfb work on Amiga, but that's the only VGA support m68k has.
There's also no <asm/vga.h>, so m68k/allmodconfig with CONFIG_AMIGA disabled now fails with:

Quoted 6 lines

In file included from include/linux/vgaarb.h:34,
                from drivers/gpu/drm/drm_irq.c:42:
include/video/vga.h:23:21: error: asm/vga.h: No such file or directory make[1]: *** [drivers/gpu/drm/drm_irq.o] Error 1

Removing the #include <video/vga.h> fixes both cases.

Anyway, I'll move the hack for cirrusfb on Amiga to <asm/vga.h> on m68k.

You still may want to remove the include of <video/vga.h> if it's not needed.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds

maintainer
#9 lore raw
Quoted 36 lines · Geert Uytterhoeven
Quoted 35 lines · Geert Uytterhoeven

include/linux/vgaarb.h line 34

Quoted 7 lines · Matthew Garrett
include/linux/vgaarb.h +1 -0
@@ -31,6 +31,7 @@
  #ifndef LINUX_VGA_H
  #define LINUX_VGA_H
 
+#include <video/vga.h>

Is this include really needed? x86_64/allmodconfig builds fine without it.

It causes m68k/allmodconfig to fail with:

In file included from include/linux/vgaarb.h:35,
                from drivers/gpu/drm/drm_irq.c:43:
include/video/vga.h: In function ‘vga_mm_r’:
include/video/vga.h:242: error: implicit declaration of function ‘z_readb’ include/video/vga.h: In function ‘vga_mm_w’:
include/video/vga.h:247: error: implicit declaration of function ‘z_writeb’ include/video/vga.h: In function ‘vga_mm_w_fast’:
include/video/vga.h:253: error: implicit declaration of function ‘z_writew’ make[1]: *** [drivers/gpu/drm/drm_irq.o] Error 1

http://kisskb.ellerman.id.au/kisskb/buildresult/6127136/

Currently <video/vga.h> has a crude hack to make cirrusfb work on Amiga, but that's the only VGA support m68k has.
There's also no <asm/vga.h>, so m68k/allmodconfig with CONFIG_AMIGA disabled now fails with:

In file included from include/linux/vgaarb.h:34,
                from drivers/gpu/drm/drm_irq.c:42:
include/video/vga.h:23:21: error: asm/vga.h: No such file or directory make[1]: *** [drivers/gpu/drm/drm_irq.o] Error 1

Quoted 5 lines
Quoted 1 line

Removing the #include <video/vga.h> fixes both cases.

Anyway, I'll move the hack for cirrusfb on Amiga to <asm/vga.h> on m68k.

You still may want to remove the include of <video/vga.h> if it's not needed.

I don't think its necessary, I'm dropping it here. should be gone in -next soon after.

Dave.

maintainer
#10 lore raw
Quoted 38 lines · Dave Airlie
Quoted 36 lines · Geert Uytterhoeven
Quoted 35 lines · Geert Uytterhoeven

include/linux/vgaarb.h line 34

Quoted 7 lines · Matthew Garrett
include/linux/vgaarb.h +1 -0
@@ -31,6 +31,7 @@
  #ifndef LINUX_VGA_H
  #define LINUX_VGA_H
 
+#include <video/vga.h>

Is this include really needed? x86_64/allmodconfig builds fine without it.

It causes m68k/allmodconfig to fail with:

In file included from include/linux/vgaarb.h:35,
                from drivers/gpu/drm/drm_irq.c:43:
include/video/vga.h: In function ‘vga_mm_r’:
include/video/vga.h:242: error: implicit declaration of function ‘z_readb’ include/video/vga.h: In function ‘vga_mm_w’:
include/video/vga.h:247: error: implicit declaration of function ‘z_writeb’ include/video/vga.h: In function ‘vga_mm_w_fast’:
include/video/vga.h:253: error: implicit declaration of function ‘z_writew’ make[1]: *** [drivers/gpu/drm/drm_irq.o] Error 1

http://kisskb.ellerman.id.au/kisskb/buildresult/6127136/

Currently <video/vga.h> has a crude hack to make cirrusfb work on Amiga, but that's the only VGA support m68k has.
There's also no <asm/vga.h>, so m68k/allmodconfig with CONFIG_AMIGA disabled now fails with:

In file included from include/linux/vgaarb.h:34,
                from drivers/gpu/drm/drm_irq.c:42:
include/video/vga.h:23:21: error: asm/vga.h: No such file or directory make[1]: *** [drivers/gpu/drm/drm_irq.o] Error 1

Quoted 9 lines
Quoted 5 lines
Quoted 1 line

Removing the #include <video/vga.h> fixes both cases.

Anyway, I'll move the hack for cirrusfb on Amiga to <asm/vga.h> on m68k.

You still may want to remove the include of <video/vga.h> if it's not needed.

I don't think its necessary, I'm dropping it here. should be gone in -next soon after.

Of course then I read the rest of the patches, and yes patch 4/4 would need it so we pick up the __ARCH_HAS define, unless we can think of a better place to put that.

Dave.

Quoted 1 line

Dave.