bbswitch_dev: disable runtime PM on removal

Call pm_runtime_forbid() to balance it with pm_runtime_allow(), this
ensures that the runtime usage counter before loading and after
unloading match.

README.md

@@ -165,3 +165,9 @@ issues on this module in the issue tracker and provide the following details:
 
 Upload the generated tarball on the above Launchpad URL and provide a link to
 the comment containing your report.
+
+TODO
+----
+With the new PCI device approach, if load_state=0, then starting bumblebeed will
+unload bbswitch. Fix Bumblebee not to unload the module when the PM method is
+bbswitch and the loaded module is bbswitch.

bbswitch.c

@@ -8,6 +8,16 @@
  * # echo ON > /proc/acpi/bbswitch
  * Get status
  * # cat /proc/acpi/bbswitch
+ *
+ * Note: only one PCI driver (bbswitch, nouveau, etc.) can bind to a PCI device.
+ * When turning a device OFF, bbswitch tries to bind itself to the PCI device.
+ * When turning a device ON, bbswitch unbinds a device (if it was bound) before
+ * returning from the write.
+ *
+ * TODO is this true?
+ * The new dual module approach is used to allow for backwards compatibility,
+ * Bumblebee unloads any driver that is loaded for a device, that would however
+ * result in unloading the main bbswitch module.
  */
 /*
  *  Copyright (C) 2011-2013 Bumblebee Project
@@ -35,13 +45,6 @@
 #include <linux/suspend.h>
 #include <linux/seq_file.h>
 #include <linux/pm_runtime.h>
-#include <linux/pm_domain.h>
-#include <linux/vga_switcheroo.h>
-#include <linux/version.h>
-
-#if LINUX_VERSION_CODE < KERNEL_VERSION(3,8,0)
-#   define ACPI_HANDLE DEVICE_ACPI_HANDLE
-#endif
 
 #define BBSWITCH_VERSION "0.8"
 
@@ -93,15 +96,11 @@ http://lxr.linux.no/#linux+v3.1.5/drivers/gpu/drm/i915/intel_acpi.c
 #define DSM_TYPE_NVIDIA         2
 static int dsm_type = DSM_TYPE_UNSUPPORTED;
 
-/* The cached name of the discrete device (of the form "0000:01:00.0"). */
-static char dis_dev_name[16];
-/* dis_dev is non-NULL iff it is currently bound by bbswitch (and off). */
 static struct pci_dev *dis_dev;
 static acpi_handle dis_handle;
 
-/* The PM domain that wraps the PCI device, used to ensure that power is
- * available before the device is put in D0. ("Nvidia" DSM and PR3). */
-static struct dev_pm_domain pm_domain;
+/* whether the card was off before suspend or not; on: 0, off: 1 */
+static int dis_before_suspend_disabled;
 
 static char *buffer_to_string(const char *buffer, size_t n, char *target) {
     int i;
@@ -208,6 +207,26 @@ static int bbswitch_optimus_dsm(void) {
     return 0;
 }
 
+// Windows 8/8.1/10 do not use DSM to put the device in D3cold state,
+// instead it disables power resources on the parent PCIe port device.
+static bool has_pr3_support(void) {
+    acpi_handle parent_handle;
+    struct acpi_device *ad = NULL;
+
+    if (ACPI_FAILURE(acpi_get_parent(dis_handle, &parent_handle))) {
+        pr_warn("Failed to obtain the parent device\n");
+        return false;
+    }
+
+    acpi_bus_get_device(parent_handle, &ad);
+    if (!ad) {
+        pr_warn("Failed to obtain an ACPI device for handle\n");
+        return false;
+    }
+
+    return ad->power.flags.power_resources;
+}
+
 static int bbswitch_acpi_off(void) {
     if (dsm_type == DSM_TYPE_NVIDIA) {
         char args[] = {2, 0, 0, 0};
@@ -240,219 +259,82 @@ static int bbswitch_acpi_on(void) {
 
 // Returns 1 if the card is disabled, 0 if enabled
 static int is_card_disabled(void) {
-    /* Assume that the device is disabled when our PCI driver found a device. */
-    return dis_dev != NULL;
+    u32 cfg_word;
+    // read first config word which contains Vendor and Device ID. If all bits
+    // are enabled, the device is assumed to be off
+    pci_read_config_dword(dis_dev, 0, &cfg_word);
+    // if one of the bits is not enabled (the card is enabled), the inverted
+    // result will be non-zero and hence logical not will make it 0 ("false")
+    return !~cfg_word;
 }
 
+static void bbswitch_off(void) {
+    if (is_card_disabled())
+        return;
 
-/* Power source handling. */
-
-static int bbswitch_pmd_runtime_suspend(struct device *dev)
-{
-    int ret;
-
-    pr_debug("Preparing for runtime suspend.\n");
-
-    /* Put the device in D3. */
-    ret = dev->bus->pm->runtime_suspend(dev);
-    if (ret)
-        return ret;
-
-    bbswitch_acpi_off();
-    /* TODO For PR3, disable them. */
-    return 0;
-}
-
-static int bbswitch_pmd_runtime_resume(struct device *dev)
-{
-    pr_info("enabling discrete graphics\n");
-
-    bbswitch_acpi_on();
-    /* TODO For PR3, enable them. */
-
-    /* Now ensure that the device is actually put in D0 by PCI. */
-    return dev->bus->pm->runtime_resume(dev);
-}
-
-static void bbswitch_pmd_set(struct device *dev)
-{
-    pm_domain.ops = *dev->bus->pm;
-    pm_domain.ops.runtime_resume = bbswitch_pmd_runtime_resume;
-    pm_domain.ops.runtime_suspend = bbswitch_pmd_runtime_suspend;
-    dev_pm_domain_set(dev, &pm_domain);
-}
-
-
-/* Nvidia device itself. */
-
-static int bbswitch_pci_runtime_suspend(struct device *dev)
-{
-    struct pci_dev *pdev = to_pci_dev(dev);
+    // to prevent the system from possibly locking up, don't disable the device
+    // if it's still in use by a driver (i.e. nouveau or nvidia)
+    if (dis_dev->driver) {
+        pr_warn("device %s is in use by driver '%s', refusing OFF\n",
+            dev_name(&dis_dev->dev), dis_dev->driver->name);
+        return;
+    }
 
     pr_info("disabling discrete graphics\n");
 
-    /* Ensure that the audio driver knows not to touch us. */
-    vga_switcheroo_set_dynamic_switch(pdev, VGA_SWITCHEROO_OFF);
-
-    bbswitch_optimus_dsm();
-
-    /* Save state now that the device is still awake, makes PCI layer happy */
-    pci_save_state(pdev);
-    /* TODO if _PR3 is supported, should this be PCI_D3hot? */
-    pci_set_power_state(pdev, PCI_D3hot);
-    return 0;
-}
-
-static int bbswitch_pci_runtime_resume(struct device *dev)
-{
-    struct pci_dev *pdev = to_pci_dev(dev);
-
-    pr_debug("Finishing runtime resume.\n");
-
-    /* Resume audio driver. */
-    vga_switcheroo_set_dynamic_switch(pdev, VGA_SWITCHEROO_ON);
-    return 0;
-}
-
-static const struct dev_pm_ops bbswitch_pci_pm_ops = {
-    .runtime_suspend = bbswitch_pci_runtime_suspend,
-    .runtime_resume = bbswitch_pci_runtime_resume,
-    /* No runtime_idle callback, the default zero delay is sufficient. */
-};
-
-static int bbswitch_switcheroo_switchto(enum vga_switcheroo_client_id id)
-{
-    /* We do not support switching, only power on/off. */
-    return -ENOSYS;
-}
-
-static enum vga_switcheroo_client_id bbswitch_switcheroo_get_client_id(struct pci_dev *pdev)
-{
-    /* Our registered client is always the discrete GPU. */
-    return VGA_SWITCHEROO_DIS;
-}
-
-static const struct vga_switcheroo_handler bbswitch_handler = {
-    .switchto = bbswitch_switcheroo_switchto,
-    .get_client_id = bbswitch_switcheroo_get_client_id,
-};
-
-
-static void bbswitch_switcheroo_set_gpu_state(struct pci_dev *pdev, enum vga_switcheroo_state state)
-{
-    /* Nothing to do, we handle the PM domain ourselves. Perhaps we can add
-     * backwards compatibility with older kernels in this way and workaround
-     * bugs? */
-    pr_debug("set_gpu_state to %s\n", state == VGA_SWITCHEROO_ON ? "ON" : "OFF");
-}
-
-static bool bbswitch_switcheroo_can_switch(struct pci_dev *pdev)
-{
-    /* We do not support switching between IGD/DIS. */
-    return false;
-}
-
-static const struct vga_switcheroo_client_ops bbswitch_switcheroo_ops = {
-    .set_gpu_state = bbswitch_switcheroo_set_gpu_state,
-    .can_switch = bbswitch_switcheroo_can_switch,
-};
-
-static int bbswitch_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
-{
-    /* Only bind to the device which we discovered before. */
-    if (strcmp(dev_name(&pdev->dev), dis_dev_name))
-        return -ENODEV;
-
-    pr_debug("Found PCI device\n");
-    dis_dev = pdev;
-
-    bbswitch_pmd_set(&pdev->dev);
-#if LINUX_VERSION_CODE >= KERNEL_VERSION(4,5,0)
-    vga_switcheroo_register_handler(&bbswitch_handler, 0);
-#else
-    vga_switcheroo_register_handler(&bbswitch_handler);
-#endif
-    vga_switcheroo_register_client(pdev, &bbswitch_switcheroo_ops, true);
-
-    /* Prevent kernel from detaching the PCI device for some devices that
-     * generate hotplug events. The graphics card is typically not physically
-     * removable. */
-    pci_ignore_hotplug(pdev);
-
-    pm_runtime_set_active(&pdev->dev); /* clear any errors */
-    /* Use autosuspend to avoid lspci waking up the device multiple times. */
-    pm_runtime_set_autosuspend_delay(&pdev->dev, 2000);
-    pm_runtime_use_autosuspend(&pdev->dev);
-    pm_runtime_allow(&pdev->dev);
-    pm_runtime_put_autosuspend(&pdev->dev);
-    return 0;
-}
-
-static void bbswitch_pci_remove(struct pci_dev *pdev)
-{
-    pr_debug("Removing PCI device\n");
-
-    pm_runtime_get_noresume(&pdev->dev);
-    pm_runtime_dont_use_autosuspend(&pdev->dev);
-    pm_runtime_forbid(&pdev->dev);
-
-    vga_switcheroo_unregister_client(pdev);
-    vga_switcheroo_unregister_handler();
-    dev_pm_domain_set(&pdev->dev, NULL);
-
-    dis_dev = NULL;
-}
-
-static const struct pci_device_id pciidlist[] = {
-    { PCI_DEVICE(PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID),
-      PCI_CLASS_DISPLAY_VGA << 8, 0xffff00 },
-    { PCI_DEVICE(PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID),
-      PCI_CLASS_DISPLAY_3D << 8, 0xffff00 },
-    { 0, 0, 0 },
-};
-
-static struct pci_driver bbswitch_pci_driver = {
-    .name = KBUILD_MODNAME,
-    .id_table = pciidlist,
-    .probe  = bbswitch_pci_probe,
-    .remove = bbswitch_pci_remove,
-    .driver.pm = &bbswitch_pci_pm_ops,
-};
-
-
-static void bbswitch_off(void) {
-    int ret;
-
-    /* Do nothing if the device was disabled before. */
-    if (dis_dev)
-        return;
-
-    ret = pci_register_driver(&bbswitch_pci_driver);
-    if (ret) {
-        pr_warn("Cannot register PCI device\n");
+    if (bbswitch_optimus_dsm()) {
+        pr_warn("Optimus ACPI call failed, the device is not disabled\n");
         return;
     }
 
-    /* If the probe failed, remove the driver such that it can be reprobed on
-     * the next registration. */
-    if (!dis_dev) {
-#if 0
-        /* TODO discover the other driver name if possible. */
-        pr_warn("device %s is in use by driver '%s', refusing OFF\n",
-            dev_name(&dis_dev->dev), dis_dev->driver->name);
-#endif
+    pci_save_state(dis_dev);
+    pci_clear_master(dis_dev);
+    pci_disable_device(dis_dev);
+    do {
+        struct acpi_device *ad = NULL;
+        int r;
 
-        pr_warn("Could not bind to device, is it in use by an other driver?\n");
-        pci_unregister_driver(&bbswitch_pci_driver);
-    }
+        r = acpi_bus_get_device(dis_handle, &ad);
+        if (r || !ad) {
+            pr_warn("Cannot get ACPI device for PCI device\n");
+            break;
+        }
+        if (ad->power.state == ACPI_STATE_UNKNOWN) {
+            pr_debug("ACPI power state is unknown, forcing D0\n");
+            ad->power.state = ACPI_STATE_D0;
+        }
+    } while (0);
+    pci_set_power_state(dis_dev, PCI_D3cold);
+
+    if (bbswitch_acpi_off())
+        pr_warn("The discrete card could not be disabled by a _DSM call\n");
 }
 
 static void bbswitch_on(void) {
-    /* Do nothing if no device exists that was previously disabled. */
-    if (!dis_dev)
+    if (!is_card_disabled())
         return;
 
-    pci_unregister_driver(&bbswitch_pci_driver);
+    pr_info("enabling discrete graphics\n");
+
+    if (bbswitch_acpi_on())
+        pr_warn("The discrete card could not be enabled by a _DSM call\n");
+
+    pci_set_power_state(dis_dev, PCI_D0);
+    pci_restore_state(dis_dev);
+    if (pci_enable_device(dis_dev))
+        pr_warn("failed to enable %s\n", dev_name(&dis_dev->dev));
+    pci_set_master(dis_dev);
+}
+
+/* power bus so we can read PCI configuration space */
+static void dis_dev_get(void) {
+    if (dis_dev->bus && dis_dev->bus->self)
+        pm_runtime_get_sync(&dis_dev->bus->self->dev);
+}
+
+static void dis_dev_put(void) {
+    if (dis_dev->bus && dis_dev->bus->self)
+        pm_runtime_put_sync(&dis_dev->bus->self->dev);
 }
 
 static ssize_t bbswitch_proc_write(struct file *fp, const char __user *buff,
@@ -465,25 +347,64 @@ static ssize_t bbswitch_proc_write(struct file *fp, const char __user *buff,
     if (copy_from_user(cmd, buff, len))
         return -EFAULT;
 
+    dis_dev_get();
+
     if (strncmp(cmd, "OFF", 3) == 0)
         bbswitch_off();
 
     if (strncmp(cmd, "ON", 2) == 0)
         bbswitch_on();
 
+    dis_dev_put();
+
     return len;
 }
 
 static int bbswitch_proc_show(struct seq_file *seqfp, void *p) {
     // show the card state. Example output: 0000:01:00:00 ON
-    seq_printf(seqfp, "%s %s\n", dis_dev_name,
+    dis_dev_get();
+    seq_printf(seqfp, "%s %s\n", dev_name(&dis_dev->dev),
              is_card_disabled() ? "OFF" : "ON");
+    dis_dev_put();
     return 0;
 }
 static int bbswitch_proc_open(struct inode *inode, struct file *file) {
     return single_open(file, bbswitch_proc_show, NULL);
 }
 
+static int bbswitch_pm_handler(struct notifier_block *nbp,
+    unsigned long event_type, void *p) {
+    switch (event_type) {
+    case PM_HIBERNATION_PREPARE:
+    case PM_SUSPEND_PREPARE:
+        dis_dev_get();
+        dis_before_suspend_disabled = is_card_disabled();
+        // enable the device before suspend to avoid the PCI config space from
+        // being saved incorrectly
+        if (dis_before_suspend_disabled)
+            bbswitch_on();
+        dis_dev_put();
+        break;
+    case PM_POST_HIBERNATION:
+    case PM_POST_SUSPEND:
+    case PM_POST_RESTORE:
+        // after suspend, the card is on, but if it was off before suspend,
+        // disable it again
+        if (dis_before_suspend_disabled) {
+            dis_dev_get();
+            bbswitch_off();
+            dis_dev_put();
+        }
+        break;
+    case PM_RESTORE_PREPARE:
+        // deliberately don't do anything as it does not occur before suspend
+        // nor hibernate, but before restoring a saved image. In that case,
+        // either PM_POST_HIBERNATION or PM_POST_RESTORE will be called
+        break;
+    }
+    return 0;
+}
+
 static struct file_operations bbswitch_fops = {
     .open   = bbswitch_proc_open,
     .read   = seq_read,
@@ -492,6 +413,10 @@ static struct file_operations bbswitch_fops = {
     .release= single_release
 };
 
+static struct notifier_block nb = {
+    .notifier_call = &bbswitch_pm_handler
+};
+
 static int __init bbswitch_init(void) {
     struct proc_dir_entry *acpi_entry;
     struct pci_dev *pdev = NULL;
@@ -508,7 +433,13 @@ static int __init bbswitch_init(void) {
             pci_class != PCI_CLASS_DISPLAY_3D)
             continue;
 
+#ifdef ACPI_HANDLE
+        /* since Linux 3.8 */
         handle = ACPI_HANDLE(&pdev->dev);
+#else
+        /* removed since Linux 3.13 */
+        handle = DEVICE_ACPI_HANDLE(&pdev->dev);
+#endif
         if (!handle) {
             pr_warn("cannot find ACPI handle for VGA device %s\n",
                 dev_name(&pdev->dev));
@@ -522,7 +453,7 @@ static int __init bbswitch_init(void) {
             pr_info("Found integrated VGA device %s: %s\n",
                 dev_name(&pdev->dev), (char *)buf.pointer);
         } else {
-            strlcpy(dis_dev_name, dev_name(&pdev->dev), sizeof(dis_dev_name));
+            dis_dev = pdev;
             dis_handle = handle;
             pr_info("Found discrete VGA device %s: %s\n",
                 dev_name(&pdev->dev), (char *)buf.pointer);
@@ -530,12 +461,14 @@ static int __init bbswitch_init(void) {
         kfree(buf.pointer);
     }
 
-    if (dis_handle == NULL) {
+    if (dis_dev == NULL) {
         pr_err("No discrete VGA device found\n");
         return -ENODEV;
     }
 
-    if (!skip_optimus_dsm &&
+    if (has_pr3_support()) {
+        pr_info("skipping _DSM as _PR3 support is detected\n");
+    } else if (!skip_optimus_dsm &&
             has_dsm_func(acpi_optimus_dsm_muid, 0x100, 0x1A)) {
         dsm_type = DSM_TYPE_OPTIMUS;
         pr_info("detected an Optimus _DSM function\n");
@@ -562,11 +495,25 @@ static int __init bbswitch_init(void) {
         return -ENOMEM;
     }
 
-    if (load_state == CARD_OFF)
+    dis_dev_get();
+
+    if (!is_card_disabled()) {
+        /* We think the card is enabled, so ensure the kernel does as well */
+        if (pci_enable_device(dis_dev))
+            pr_warn("failed to enable %s\n", dev_name(&dis_dev->dev));
+    }
+
+    if (load_state == CARD_ON)
+        bbswitch_on();
+    else if (load_state == CARD_OFF)
         bbswitch_off();
 
     pr_info("Succesfully loaded. Discrete card %s is %s\n",
-        dis_dev_name, is_card_disabled() ? "off" : "on");
+        dev_name(&dis_dev->dev), is_card_disabled() ? "off" : "on");
+
+    dis_dev_put();
+
+    register_pm_notifier(&nb);
 
     return 0;
 }
@@ -574,8 +521,20 @@ static int __init bbswitch_init(void) {
 static void __exit bbswitch_exit(void) {
     remove_proc_entry("bbswitch", acpi_root_dir);
 
-    bbswitch_on();
-    pr_info("Unloaded\n");
+    dis_dev_get();
+
+    if (unload_state == CARD_ON)
+        bbswitch_on();
+    else if (unload_state == CARD_OFF)
+        bbswitch_off();
+
+    pr_info("Unloaded. Discrete card %s is %s\n",
+        dev_name(&dis_dev->dev), is_card_disabled() ? "off" : "on");
+
+    dis_dev_put();
+
+    if (nb.notifier_call)
+        unregister_pm_notifier(&nb);
 }
 
 module_init(bbswitch_init);

bbswitch_dev.c

@@ -0,0 +1,100 @@
+/*
+ * TODO merge into main bbswitch module.
+ * TODO on ON call device_release_driver
+ * TODO how to bind to a specific device from kernel space? Can't use
+ *      driver_probe_device (https://lkml.org/lkml/2014/2/14/628). Maybe use
+ *      driver_override or new_id/bind/remove_id from userspace?
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/pci.h>
+#include <linux/module.h>
+#include <linux/pm_runtime.h>
+#include <linux/delay.h>
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Toggle the discrete graphics card (PCI driver)");
+MODULE_AUTHOR("Peter Wu <peter@lekensteyn.nl>");
+
+static int bbswitch_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
+{
+    /* TODO how to discover devices? */
+    /* Prevent kernel from detaching the PCI device for some devices that
+     * generate hotplug events. The graphics card is typically not physically
+     * removable. */
+    pci_ignore_hotplug(dev);
+
+    pm_runtime_set_active(&dev->dev); /* clear any errors */
+    /* Use autosuspend to avoid lspci waking up the device multiple times. */
+    pm_runtime_set_autosuspend_delay(&dev->dev, 2000);
+    pm_runtime_use_autosuspend(&dev->dev);
+    pm_runtime_allow(&dev->dev);
+    pm_runtime_put_autosuspend(&dev->dev);
+    return 0;
+}
+
+static void bbswitch_pci_remove(struct pci_dev *dev)
+{
+    pm_runtime_get_noresume(&dev->dev);
+    pm_runtime_dont_use_autosuspend(&dev->dev);
+    pm_runtime_forbid(&dev->dev);
+}
+
+static int bbswitch_runtime_suspend(struct device *dev) {
+    struct pci_dev *pdev = to_pci_dev(dev);
+
+    pr_info("disabling discrete graphics\n");
+
+    /* TODO if _PR3 is not supported, call Optimus DSM here. */
+    /* TODO for v1 Optimus, call DSM here. */
+
+    /* Save state now that the device is still awake, makes PCI layer happy */
+    pci_save_state(pdev);
+    /* TODO if _PR3 is supported, should this be PCI_D3hot? */
+    pci_set_power_state(pdev, PCI_D3cold);
+    return 0;
+}
+
+static int bbswitch_runtime_resume(struct device *dev) {
+    pr_info("enabling discrete graphics\n");
+
+    /* TODO for v1 Optimus, call DSM here. */
+    /* Nothing to do for Optimus, the PCI layer already moved into D0 state. */
+    return 0;
+}
+
+static struct dev_pm_ops bbswitch_pm_ops = {
+    .runtime_suspend = bbswitch_runtime_suspend,
+    .runtime_resume = bbswitch_runtime_resume,
+    /* No runtime_idle callback, the default zero delay is sufficient. */
+};
+
+static struct pci_driver bbswitch_pci_driver = {
+    .name = KBUILD_MODNAME,
+    .id_table = NULL, /* will be added dynamically */
+    .probe  = bbswitch_pci_probe,
+    .remove = bbswitch_pci_remove,
+    .driver.pm = &bbswitch_pm_ops,
+};
+
+static int __init bbswitch_dev_init(void) {
+    int ret;
+
+    ret = pci_register_driver(&bbswitch_pci_driver);
+#if 0
+    ret = pci_add_dynid(&bbswitch_pci_driver, PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID,
+            PCI_ANY_ID, PCI_ANY_ID, PCI_BASE_CLASS_DISPLAY << 16, 0xff0000);
+#endif
+
+    return ret;
+}
+
+static void __exit bbswitch_dev_exit(void) {
+    pci_unregister_driver(&bbswitch_pci_driver);
+}
+
+module_init(bbswitch_dev_init);
+module_exit(bbswitch_dev_exit);
+
+/* vim: set sw=4 ts=4 et: */