Skip to content

Commit bf42c14

Browse files
l1kbwhacks
authored andcommitted
Revert "apple-gmux: lock iGP IO to protect from vgaarb changes"
commit d6fa758 upstream. Commit 4eebd5a ("apple-gmux: lock iGP IO to protect from vgaarb changes") amended this driver's ->probe hook to lock decoding of normal (non-legacy) I/O space accesses to the integrated GPU on dual-GPU MacBook Pros. The lock stays in place until the driver is unbound. The change was made to work around an issue with the out-of-tree nvidia graphics driver (available at http://www.nvidia.com/object/unix.html). It contains the following sequence in nvidia/nv.c: #if defined(CONFIG_VGA_ARB) && !defined(NVCPU_PPC64LE) #if defined(VGA_DEFAULT_DEVICE) vga_tryget(VGA_DEFAULT_DEVICE, VGA_RSRC_LEGACY_MASK); #endif vga_set_legacy_decoding(dev, VGA_RSRC_NONE); #endif This code was reported to cause deadlocks with VFIO already in 2013: https://devtalk.nvidia.com/default/topic/545560 I've reported the issue to Nvidia developers once more in 2017: https://www.spinics.net/lists/dri-devel/msg138754.html On the MacBookPro10,1, this code apparently breaks backlight control (which is handled by apple-gmux via an I/O region starting at 0x700), as reported by Petri Hodju: https://bugzilla.kernel.org/show_bug.cgi?id=86121 I tried to replicate Petri's observations on my MacBook9,1, which uses the same Intel Ivy Bridge + Nvidia GeForce GT 650M architecture, to no avail. On my machine apple-gmux' I/O region remains accessible even with the nvidia driver loaded and commit 4eebd5a reverted. Petri reported that apple-gmux becomes accessible again after a suspend/resume cycle because the BIOS changed the VGA routing on the root port to the Nvidia GPU. Perhaps this is a BIOS issue after all that can be fixed with an update? In any case, the change made by commit 4eebd5a has turned out to cause two new issues: * Wilfried Klaebe reports a deadlock when launching Xorg because it opens /dev/vga_arbiter and calls vga_get(), but apple-gmux is holding a lock on I/O space indefinitely. It looks like apple-gmux' current behavior is an abuse of the vgaarb API as locks are not meant to be held for longer periods: https://bugzilla.kernel.org/show_bug.cgi?id=88861#c11 https://bugzilla.kernel.org/attachment.cgi?id=217541 * On dual GPU MacBook Pros introduced since 2013, the integrated GPU is powergated on boot und thus becomes invisible to Linux unless a custom EFI protocol is used to leave it powered on. (A patch exists but is not in mainline yet due to several negative side effects.) On these machines, locking I/O to the integrated GPU (as done by 4eebd5a) fails and backlight control is therefore broken: https://bugzilla.kernel.org/show_bug.cgi?id=105051 So let's revert commit 4eebd5a please. Users experiencing the issue with the proprietary nvidia driver can comment out the above- quoted problematic code as a workaround (or try updating the BIOS). Cc: Petri Hodju <[email protected]> Cc: Bjorn Helgaas <[email protected]> Cc: Bruno Prémont <[email protected]> Cc: Andy Ritger <[email protected]> Cc: Ronald Tschalär <[email protected]> Tested-by: Wilfried Klaebe <[email protected]> Signed-off-by: Lukas Wunner <[email protected]> Signed-off-by: Darren Hart (VMware) <[email protected]> [bwh: Backported to 3.16: adjust context] Signed-off-by: Ben Hutchings <[email protected]>
1 parent 21b59ba commit bf42c14

File tree

1 file changed

+1
-47
lines changed

1 file changed

+1
-47
lines changed

drivers/platform/x86/apple-gmux.c

Lines changed: 1 addition & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
#include <linux/delay.h>
2323
#include <linux/pci.h>
2424
#include <linux/vga_switcheroo.h>
25-
#include <linux/vgaarb.h>
2625
#include <acpi/video.h>
2726
#include <asm/io.h>
2827

@@ -32,7 +31,6 @@ struct apple_gmux_data {
3231
bool indexed;
3332
struct mutex index_lock;
3433

35-
struct pci_dev *pdev;
3634
struct backlight_device *bdev;
3735

3836
/* switcheroo data */
@@ -417,23 +415,6 @@ static int gmux_resume(struct device *dev)
417415
return 0;
418416
}
419417

420-
static struct pci_dev *gmux_get_io_pdev(void)
421-
{
422-
struct pci_dev *pdev = NULL;
423-
424-
while ((pdev = pci_get_class(PCI_CLASS_DISPLAY_VGA << 8, pdev))) {
425-
u16 cmd;
426-
427-
pci_read_config_word(pdev, PCI_COMMAND, &cmd);
428-
if (!(cmd & PCI_COMMAND_IO))
429-
continue;
430-
431-
return pdev;
432-
}
433-
434-
return NULL;
435-
}
436-
437418
static int gmux_probe(struct pnp_dev *pnp, const struct pnp_device_id *id)
438419
{
439420
struct apple_gmux_data *gmux_data;
@@ -444,7 +425,6 @@ static int gmux_probe(struct pnp_dev *pnp, const struct pnp_device_id *id)
444425
int ret = -ENXIO;
445426
acpi_status status;
446427
unsigned long long gpe;
447-
struct pci_dev *pdev = NULL;
448428

449429
if (apple_gmux_data)
450430
return -EBUSY;
@@ -495,31 +475,14 @@ static int gmux_probe(struct pnp_dev *pnp, const struct pnp_device_id *id)
495475
ver_minor = (version >> 16) & 0xff;
496476
ver_release = (version >> 8) & 0xff;
497477
} else {
498-
pr_info("gmux device not present or IO disabled\n");
478+
pr_info("gmux device not present\n");
499479
ret = -ENODEV;
500480
goto err_release;
501481
}
502482
}
503483
pr_info("Found gmux version %d.%d.%d [%s]\n", ver_major, ver_minor,
504484
ver_release, (gmux_data->indexed ? "indexed" : "classic"));
505485

506-
/*
507-
* Apple systems with gmux are EFI based and normally don't use
508-
* VGA. In addition changing IO+MEM ownership between IGP and dGPU
509-
* disables IO/MEM used for backlight control on some systems.
510-
* Lock IO+MEM to GPU with active IO to prevent switch.
511-
*/
512-
pdev = gmux_get_io_pdev();
513-
if (pdev && vga_tryget(pdev,
514-
VGA_RSRC_NORMAL_IO | VGA_RSRC_NORMAL_MEM)) {
515-
pr_err("IO+MEM vgaarb-locking for PCI:%s failed\n",
516-
pci_name(pdev));
517-
ret = -EBUSY;
518-
goto err_release;
519-
} else if (pdev)
520-
pr_info("locked IO for PCI:%s\n", pci_name(pdev));
521-
gmux_data->pdev = pdev;
522-
523486
memset(&props, 0, sizeof(props));
524487
props.type = BACKLIGHT_PLATFORM;
525488
props.max_brightness = gmux_read32(gmux_data, GMUX_PORT_MAX_BRIGHTNESS);
@@ -611,10 +574,6 @@ static int gmux_probe(struct pnp_dev *pnp, const struct pnp_device_id *id)
611574
err_notify:
612575
backlight_device_unregister(bdev);
613576
err_release:
614-
if (gmux_data->pdev)
615-
vga_put(gmux_data->pdev,
616-
VGA_RSRC_NORMAL_IO | VGA_RSRC_NORMAL_MEM);
617-
pci_dev_put(pdev);
618577
release_region(gmux_data->iostart, gmux_data->iolen);
619578
err_free:
620579
kfree(gmux_data);
@@ -634,11 +593,6 @@ static void gmux_remove(struct pnp_dev *pnp)
634593
&gmux_notify_handler);
635594
}
636595

637-
if (gmux_data->pdev) {
638-
vga_put(gmux_data->pdev,
639-
VGA_RSRC_NORMAL_IO | VGA_RSRC_NORMAL_MEM);
640-
pci_dev_put(gmux_data->pdev);
641-
}
642596
backlight_device_unregister(gmux_data->bdev);
643597

644598
release_region(gmux_data->iostart, gmux_data->iolen);

0 commit comments

Comments
 (0)