[PATCH] vgaarb: call pci_disable|enable_device on decoding vga devices

Daniel Vetter daniel.vetter at ffwll.ch
Fri Jun 8 09:20:39 PDT 2012


There's the neat little problem that some systems die if vga decoding
isn't decoded anywhere, because linux disabled that pci device.

Atm we solve that problem by simple not calling pci_disable_device at
driver unregister time for drm pci devices. Which isn't to great
because it leaks a pci_enable_device reference on module reload and
also is rather inconsistent, since the driver load codcode in
drm/drm_pci.c _does_ call pci_disable_device on the load error path.

To fix this issue, let the vga arbiter hold a pci_enable_device
reference on its own when the vga device decodes anything. This leaves
the issue still open when the vga arbiter isn't loaded/compiled in,
but I guess since drm module unload is riddled with dragons it's ok to
say "just don't do this".

Signed-Off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
---
 drivers/gpu/vga/vgaarb.c |   27 +++++++++++++++++++++++++--
 1 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c
index 3df8fc0..82f19a1 100644
--- a/drivers/gpu/vga/vgaarb.c
+++ b/drivers/gpu/vga/vgaarb.c
@@ -49,7 +49,11 @@
 static void vga_arbiter_notify_clients(void);
 /*
  * We keep a list of all vga devices in the system to speed
- * up the various operations of the arbiter
+ * up the various operations of the arbiter. Note that vgaarb keeps a
+ * pci_enable_device reference for all vga devices with owns != 0. This is to
+ * avoid drm devices from killing the system when they call pci_disable_device
+ * on module unload (as they should), because some systems don't like it if no
+ * one decodes vga.
  */
 struct vga_device {
 	struct list_head list;
@@ -171,7 +175,7 @@ static struct vga_device *__vga_tryget(struct vga_device *vgadev,
 {
 	unsigned int wants, legacy_wants, match;
 	struct vga_device *conflict;
-	unsigned int pci_bits;
+	unsigned int pci_bits, ret;
 	u32 flags = 0;
 
 	/* Account for "normal" resources to lock. If we decode the legacy,
@@ -264,6 +268,10 @@ static struct vga_device *__vga_tryget(struct vga_device *vgadev,
 
 		pci_set_vga_state(conflict->pdev, false, pci_bits, flags);
 		conflict->owns &= ~lwants;
+
+		if (conflict->owns == 0)
+			pci_disable_device(conflict->pdev);
+
 		/* If he also owned non-legacy, that is no longer the case */
 		if (lwants & VGA_RSRC_LEGACY_MEM)
 			conflict->owns &= ~VGA_RSRC_NORMAL_MEM;
@@ -290,6 +298,12 @@ enable_them:
 	if (!!(wants & VGA_RSRC_LEGACY_MASK))
 		flags |= PCI_VGA_STATE_CHANGE_BRIDGE;
 
+	if (vgadev->owns == 0) {
+		ret = pci_enable_device(vgadev->pdev);
+		if (ret)
+			return ERR_PTR(ret);
+	}
+
 	pci_set_vga_state(vgadev->pdev, true, pci_bits, flags);
 
 	if (!vgadev->bridge_has_one_vga) {
@@ -376,6 +390,8 @@ int vga_get(struct pci_dev *pdev, unsigned int rsrc, int interruptible)
 		if (conflict == NULL)
 			break;
 
+		if (IS_ERR(conflict))
+			return PTR_ERR(conflict);
 
 		/* We have a conflict, we wait until somebody kicks the
 		 * work queue. Currently we have one work queue that we
@@ -513,6 +529,7 @@ static bool vga_arbiter_add_pci_device(struct pci_dev *pdev)
 	unsigned long flags;
 	struct pci_bus *bus;
 	struct pci_dev *bridge;
+	int ret;
 	u16 cmd;
 
 	/* Only deal with VGA class devices */
@@ -582,6 +599,12 @@ static bool vga_arbiter_add_pci_device(struct pci_dev *pdev)
 
 	vga_arbiter_check_bridge_sharing(vgadev);
 
+	if (vgadev->owns != 0) {
+		ret = pci_enable_device(vgadev->pdev);
+		if (ret)
+			goto fail;
+	}
+
 	/* Add to the list */
 	list_add(&vgadev->list, &vga_list);
 	vga_count++;
-- 
1.7.7.6



More information about the dri-devel mailing list