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".
v2: ret needs to be signed, otherwise ERR_PTR will just store a large
positive value on 64bit machines. Noticed by Jani Nikula.
Signed-Off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
---
drivers/gpu/vga/vgaarb.c | 26 +++++++++++++++++++++++++-
1 files changed, 25 insertions(+), 1 deletions(-)
diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c
index 3df8fc0..c0b1271 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;
@@ -172,6 +176,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;
+ int ret;
u32 flags = 0;
/* Account for "normal" resources to lock. If we decode the legacy,
@@ -264,6 +269,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 +299,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 +391,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 +530,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 +600,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
