LinuxLists.cc - [RFC][PATCH] PCI PM: Make new suspend-resume callbacks carry out core operations (rev. 2)

2008-12-28 15:07:35

Subject: [RFC][PATCH] PCI PM: Make new suspend-resume callbacks carry out core operations (rev. 2)

On Friday 19 December 2008, Rafael J. Wysocki wrote:> Hi Len,> > The patch below adds the callbacks that save/restore the standard PCI config> registers, change the power state of the device etc. to the new suspend-resume> callbacks (which are not yet used by device drivers), so that the drivers don't> have to worry about these operations.> > The saving and restoring of the standard PCI config registers is done with> interrupts off, the other things are done with interrupts enabled.> > The patch should apply to the current linux-next tree.
Below is a new version of the patch.
Now, the standard config spaces of devices are restored with interruptsdisabled, but only if the bridges the devices are behind are in D0. Otherwise,the operation is attempted again with interrupts enabled (presumably thebridge would be put into D0 before that happens).
Also, pci_disable_device() is not called during resume andpci_reenable_device() is used to enable the devices that were enabled duringsuspend. This allows us to automatically call pci_set_master() for devicesthat were bus masters before suspend. Additionally, the pci_reenable_device()may be overriden by the driver, by disabling the device during suspend.
Finally, bridges are handled a bit differently from the regular devices (theyare not put into low power states during suspend and pci_enable_wake() isnot called for them.
The patch should apply on top of linux-next with the patch fixingpci_update_current_state() I sent yesterday(http://marc.info/?l=linux-kernel&m=123039199607159&w=4).
Please review.
Thanks,Rafael
---There are a few standard PCI callbacks that should be used by anyPCI driver implementing suspend-resume handlers. The drivers,however, tend to use these callbacks in different ways, in differentorder or even not at all. This sometimes causes problems to appearand makes PCI suspend-resume diagnostics troublesome. Thus it seemsreasonable to make the PCI core execute the standard callbacks sothat the drivers don't have to do it.
Modify the power management framework for PCI devices so that thestandard PCI PM callbacks are executed by the core in the followingway:
* Right after executing the ->suspend() callback from the device driver's pm object (ie. with interrupts enabled) the core will execute pci_save_state() and, if the device is not a PCI bridge, the core will execute pci_prepare_to_sleep().
* Right prior to executing the ->resume_noirq() callback from the device driver's pm object (ie. with interrupts disabled), the core will execute pci_restore_state() and will update the device's pci_dev structure to reflect the actual current power state of the device. However, this is only going to succeed if the device's bus is operational (ie. its parent bridge in is the D0 power state) at that time, so the last operation has to be repeated later.
* Prior to executing the ->resume() callback from the device driver's pm object (ie. with interrupts enabled) the core will check if the device's standard config registers need to be restored and it will call pci_restore_state() if necessary. Next, if the device is not a PCI bridge, the core will disable the device's wake-up capability using pci_enable_wake(). Finally, it will execute pci_reenable_device() and, if necessary, pci_set_master(), in this order (all of this happens before the ->resume() callback is invoked).
The standard callbacks mentioned above will always be executed forall devices without drivers providing the legacy PCI PM support(ie. the ->suspend(), ->suspend_late(), ->resume_early()or ->resume() hooks in the pci_driver structure). In particular,they will be executed for devices that don't have drivers at all.
Analogous changes are made in the hibernation-related PCI PMroutines.
Signed-off-by: Rafael J. Wysocki <[email protected]>--- drivers/pci/pci-driver.c | 356 +++++++++++++++++++++++++++-------------------- drivers/pci/pci.c | 2 drivers/pci/pci.h | 1 3 files changed, 212 insertions(+), 147 deletions(-)
Index: linux-2.6/drivers/pci/pci-driver.c===================================================================--- linux-2.6.orig/drivers/pci/pci-driver.c+++ linux-2.6/drivers/pci/pci-driver.c@@ -300,98 +300,83 @@ static void pci_device_shutdown(struct d #ifdef CONFIG_PM_SLEEP -static bool pci_has_legacy_pm_support(struct pci_dev *pci_dev)-{- struct pci_driver *drv = pci_dev->driver;-- return drv && (drv->suspend || drv->suspend_late || drv->resume- || drv->resume_early);-}+/* Routines used in both the legacy and new power management callbacks */ -/*- * Default "suspend" method for devices that have no driver provided suspend,- * or not even a driver at all.- */-static void pci_default_pm_suspend(struct pci_dev *pci_dev)+static int pci_restore_standard_config(struct pci_dev *pci_dev) {- pci_save_state(pci_dev);- /*- * mark its power state as "unknown", since we don't know if- * e.g. the BIOS will change its device state when we suspend.- */- if (pci_dev->current_state == PCI_D0)+ struct pci_dev *parent = pci_dev->bus->self;+ int error = 0;++ /* Check if the device's bus is operational */+ if (!parent || parent->current_state == PCI_D0) {+ pci_restore_state(pci_dev);+ pci_update_current_state(pci_dev, PCI_D0);+ } else {+ dev_warn(&pci_dev->dev, "unable to restore config, "+ "bridge %s in low power state D%d\n", pci_name(parent),+ parent->current_state); pci_dev->current_state = PCI_UNKNOWN;-}+ error = -EAGAIN;+ } -/*- * Default "resume" method for devices that have no driver provided resume,- * or not even a driver at all (first part).- */-static void pci_default_pm_resume_early(struct pci_dev *pci_dev)-{- /* restore the PCI config space */- pci_restore_state(pci_dev);+ return error; } -/*- * Default "resume" method for devices that have no driver provided resume,- * or not even a driver at all (second part).- */-static int pci_default_pm_resume_late(struct pci_dev *pci_dev)-{- int retval;-- /* if the device was enabled before suspend, reenable */- retval = pci_reenable_device(pci_dev);- /*- * if the device was busmaster before the suspend, make it busmaster- * again- */- if (pci_dev->is_busmaster)- pci_set_master(pci_dev);-- return retval;-}+/* Legacy power management callbacks */ static int pci_legacy_suspend(struct device *dev, pm_message_t state) { struct pci_dev * pci_dev = to_pci_dev(dev); struct pci_driver * drv = pci_dev->driver;- int i = 0;+ int error = 0; if (drv && drv->suspend) {- i = drv->suspend(pci_dev, state);- suspend_report_result(drv->suspend, i);+ error = drv->suspend(pci_dev, state);+ suspend_report_result(drv->suspend, error); } else {- pci_default_pm_suspend(pci_dev);+ error = pci_save_state(pci_dev);+ /*+ * mark its power state as "unknown", since we don't know if+ * e.g. the BIOS will change its device state when we suspend.+ */+ if (!error && pci_dev->current_state == PCI_D0)+ pci_dev->current_state = PCI_UNKNOWN; }- return i;+ return error; } static int pci_legacy_suspend_late(struct device *dev, pm_message_t state) { struct pci_dev * pci_dev = to_pci_dev(dev); struct pci_driver * drv = pci_dev->driver;- int i = 0;+ int error = 0; if (drv && drv->suspend_late) {- i = drv->suspend_late(pci_dev, state);- suspend_report_result(drv->suspend_late, i);+ error = drv->suspend_late(pci_dev, state);+ suspend_report_result(drv->suspend_late, error); }- return i;+ return error; } static int pci_legacy_resume(struct device *dev) {- int error; struct pci_dev * pci_dev = to_pci_dev(dev); struct pci_driver * drv = pci_dev->driver;+ int error; if (drv && drv->resume) { error = drv->resume(pci_dev); } else {- pci_default_pm_resume_early(pci_dev);- error = pci_default_pm_resume_late(pci_dev);+ /* restore the PCI config space */+ pci_restore_state(pci_dev);+ /* if the device was enabled before suspend, reenable */+ error = pci_reenable_device(pci_dev);+ /*+ * if the device was busmaster before the suspend, make+ * it busmaster again+ */+ if (pci_dev->is_busmaster)+ pci_set_master(pci_dev); } return error; }@@ -407,6 +392,62 @@ static int pci_legacy_resume_early(struc return error; } +/* Routines used in the new power management callbacks */++static bool pci_has_legacy_pm_support(struct pci_dev *pci_dev)+{+ struct pci_driver *drv = pci_dev->driver;++ return drv && (drv->suspend || drv->suspend_late || drv->resume+ || drv->resume_early);+}++static bool pci_is_bridge(struct pci_dev *pci_dev)+{+ return !!(pci_dev->subordinate);+}++static void pci_pm_default_suspend_noirq(struct pci_dev *pci_dev)+{+ /* The device's power state may be changed by the BIOS before resume */+ if( pci_dev->current_state == PCI_D0)+ pci_dev->current_state = PCI_UNKNOWN;+}++static int pci_pm_default_resume(struct pci_dev *pci_dev)+{+ int error = 0;++ /*+ * pci_restore_standard_config() should have been called before, but it+ * would have failed if the device's bus had not been operational at+ * that time. Check it and try again if necessary.+ */+ if (pci_dev->current_state == PCI_UNKNOWN) {+ error = pci_restore_standard_config(pci_dev);+ if (error)+ return error;+ }++ pci_fixup_device(pci_fixup_resume, pci_dev);++ /* Disable wake-up capability of regular devices */+ if (!pci_is_bridge(pci_dev))+ pci_enable_wake(pci_dev, PCI_D0, false);+ /* If device was enabled before suspend, reenable it. */+ error = pci_reenable_device(pci_dev);+ /*+ * If device was a bus master before the suspend, make it a bus master+ * again.+ */+ if (pci_dev->is_busmaster)+ pci_set_master(pci_dev);++ return error;+}++/* New power management callbacks */+ static int pci_pm_prepare(struct device *dev) { struct device_driver *drv = dev->driver;@@ -434,14 +475,23 @@ static int pci_pm_suspend(struct device struct device_driver *drv = dev->driver; int error = 0; - if (drv && drv->pm) {- if (drv->pm->suspend) {- error = drv->pm->suspend(dev);- suspend_report_result(drv->pm->suspend, error);- }- } else if (pci_has_legacy_pm_support(pci_dev)) {+ if (pci_has_legacy_pm_support(pci_dev)) { error = pci_legacy_suspend(dev, PMSG_SUSPEND);+ goto Exit;+ }++ if (drv && drv->pm && drv->pm->suspend) {+ error = drv->pm->suspend(dev);+ suspend_report_result(drv->pm->suspend, error);+ }++ if (!error) {+ error = pci_save_state(pci_dev);+ if (!error && !pci_is_bridge(pci_dev))+ pci_prepare_to_sleep(pci_dev); }++ Exit: pci_fixup_device(pci_fixup_suspend, pci_dev); return error;@@ -453,17 +503,17 @@ static int pci_pm_suspend_noirq(struct d struct device_driver *drv = dev->driver; int error = 0; - if (drv && drv->pm) {- if (drv->pm->suspend_noirq) {- error = drv->pm->suspend_noirq(dev);- suspend_report_result(drv->pm->suspend_noirq, error);- }- } else if (pci_has_legacy_pm_support(pci_dev)) {- error = pci_legacy_suspend_late(dev, PMSG_SUSPEND);- } else {- pci_default_pm_suspend(pci_dev);+ if (pci_has_legacy_pm_support(pci_dev))+ return pci_legacy_suspend_late(dev, PMSG_SUSPEND);++ if (drv && drv->pm && drv->pm->suspend_noirq) {+ error = drv->pm->suspend_noirq(dev);+ suspend_report_result(drv->pm->suspend_noirq, error); } + if (!error)+ pci_pm_default_suspend_noirq(pci_dev);+ return error; } @@ -473,17 +523,16 @@ static int pci_pm_resume(struct device * struct device_driver *drv = dev->driver; int error = 0; - pci_fixup_device(pci_fixup_resume, pci_dev);-- if (drv && drv->pm) {- if (drv->pm->resume)- error = drv->pm->resume(dev);- } else if (pci_has_legacy_pm_support(pci_dev)) {- error = pci_legacy_resume(dev);- } else {- error = pci_default_pm_resume_late(pci_dev);+ if (pci_has_legacy_pm_support(pci_dev)) {+ pci_fixup_device(pci_fixup_resume, pci_dev);+ return pci_legacy_resume(dev); } + error = pci_pm_default_resume(pci_dev);++ if (!error && drv && drv->pm && drv->pm->resume)+ error = drv->pm->resume(dev);+ return error; } @@ -493,15 +542,16 @@ static int pci_pm_resume_noirq(struct de struct device_driver *drv = dev->driver; int error = 0; - pci_fixup_device(pci_fixup_resume_early, to_pci_dev(dev));+ if (pci_has_legacy_pm_support(pci_dev)) {+ pci_fixup_device(pci_fixup_resume_early, pci_dev);+ return pci_legacy_resume_early(dev);+ } - if (drv && drv->pm) {- if (drv->pm->resume_noirq)+ if (!pci_restore_standard_config(pci_dev)) {+ pci_fixup_device(pci_fixup_resume_early, pci_dev);++ if (drv && drv->pm && drv->pm->resume_noirq) error = drv->pm->resume_noirq(dev);- } else if (pci_has_legacy_pm_support(pci_dev)) {- error = pci_legacy_resume_early(dev);- } else {- pci_default_pm_resume_early(pci_dev); } return error;@@ -524,16 +574,20 @@ static int pci_pm_freeze(struct device * struct device_driver *drv = dev->driver; int error = 0; - if (drv && drv->pm) {- if (drv->pm->freeze) {- error = drv->pm->freeze(dev);- suspend_report_result(drv->pm->freeze, error);- }- } else if (pci_has_legacy_pm_support(pci_dev)) {+ if (pci_has_legacy_pm_support(pci_dev)) { error = pci_legacy_suspend(dev, PMSG_FREEZE); pci_fixup_device(pci_fixup_suspend, pci_dev);+ return error; } + if (drv && drv->pm && drv->pm->freeze) {+ error = drv->pm->freeze(dev);+ suspend_report_result(drv->pm->freeze, error);+ }++ if (!error)+ error = pci_save_state(pci_dev);+ return error; } @@ -543,17 +597,17 @@ static int pci_pm_freeze_noirq(struct de struct device_driver *drv = dev->driver; int error = 0; - if (drv && drv->pm) {- if (drv->pm->freeze_noirq) {- error = drv->pm->freeze_noirq(dev);- suspend_report_result(drv->pm->freeze_noirq, error);- }- } else if (pci_has_legacy_pm_support(pci_dev)) {- error = pci_legacy_suspend_late(dev, PMSG_FREEZE);- } else {- pci_default_pm_suspend(pci_dev);+ if (pci_has_legacy_pm_support(pci_dev))+ return pci_legacy_suspend_late(dev, PMSG_FREEZE);++ if (drv && drv->pm && drv->pm->freeze_noirq) {+ error = drv->pm->freeze_noirq(dev);+ suspend_report_result(drv->pm->freeze_noirq, error); } + if (!error)+ pci_pm_default_suspend_noirq(pci_dev);+ return error; } @@ -563,14 +617,14 @@ static int pci_pm_thaw(struct device *de struct device_driver *drv = dev->driver; int error = 0; - if (drv && drv->pm) {- if (drv->pm->thaw)- error = drv->pm->thaw(dev);- } else if (pci_has_legacy_pm_support(pci_dev)) {+ if (pci_has_legacy_pm_support(pci_dev)) { pci_fixup_device(pci_fixup_resume, pci_dev);- error = pci_legacy_resume(dev);+ return pci_legacy_resume(dev); } + if (drv && drv->pm && drv->pm->thaw)+ error = drv->pm->thaw(dev);+ return error; } @@ -580,14 +634,17 @@ static int pci_pm_thaw_noirq(struct devi struct device_driver *drv = dev->driver; int error = 0; - if (drv && drv->pm) {- if (drv->pm->thaw_noirq)- error = drv->pm->thaw_noirq(dev);- } else if (pci_has_legacy_pm_support(pci_dev)) {- pci_fixup_device(pci_fixup_resume_early, to_pci_dev(dev));- error = pci_legacy_resume_early(dev);+ if (pci_has_legacy_pm_support(pci_dev)) {+ pci_fixup_device(pci_fixup_resume_early, pci_dev);+ return pci_legacy_resume_early(dev); } + if (pci_dev->current_state == PCI_UNKNOWN)+ pci_update_current_state(pci_dev, PCI_D0);++ if (drv && drv->pm && drv->pm->thaw_noirq)+ error = drv->pm->thaw_noirq(dev);+ return error; } @@ -597,32 +654,39 @@ static int pci_pm_poweroff(struct device struct device_driver *drv = dev->driver; int error = 0; - pci_fixup_device(pci_fixup_suspend, pci_dev);-- if (drv && drv->pm) {- if (drv->pm->poweroff) {- error = drv->pm->poweroff(dev);- suspend_report_result(drv->pm->poweroff, error);- }- } else if (pci_has_legacy_pm_support(pci_dev)) {+ if (pci_has_legacy_pm_support(pci_dev)) { error = pci_legacy_suspend(dev, PMSG_HIBERNATE);+ goto Exit;+ }++ if (drv && drv->pm && drv->pm->poweroff) {+ error = drv->pm->poweroff(dev);+ suspend_report_result(drv->pm->poweroff, error);+ }++ if (!error && !pci_is_bridge(pci_dev)) {+ pci_disable_device(pci_dev);+ pci_prepare_to_sleep(pci_dev); } + Exit:+ pci_fixup_device(pci_fixup_suspend, pci_dev);+ return error; } static int pci_pm_poweroff_noirq(struct device *dev) {+ struct pci_dev *pci_dev = to_pci_dev(dev); struct device_driver *drv = dev->driver; int error = 0; - if (drv && drv->pm) {- if (drv->pm->poweroff_noirq) {- error = drv->pm->poweroff_noirq(dev);- suspend_report_result(drv->pm->poweroff_noirq, error);- }- } else if (pci_has_legacy_pm_support(to_pci_dev(dev))) {- error = pci_legacy_suspend_late(dev, PMSG_HIBERNATE);+ if (pci_has_legacy_pm_support(pci_dev))+ return pci_legacy_suspend_late(dev, PMSG_HIBERNATE);++ if (drv && drv->pm && drv->pm->poweroff_noirq) {+ error = drv->pm->poweroff_noirq(dev);+ suspend_report_result(drv->pm->poweroff_noirq, error); } return error;@@ -634,15 +698,15 @@ static int pci_pm_restore(struct device struct device_driver *drv = dev->driver; int error = 0; - if (drv && drv->pm) {- if (drv->pm->restore)- error = drv->pm->restore(dev);- } else if (pci_has_legacy_pm_support(pci_dev)) {- error = pci_legacy_resume(dev);- } else {- error = pci_default_pm_resume_late(pci_dev);+ if (pci_has_legacy_pm_support(pci_dev)) {+ pci_fixup_device(pci_fixup_resume, pci_dev);+ return pci_legacy_resume(dev); }- pci_fixup_device(pci_fixup_resume, pci_dev);++ error = pci_pm_default_resume(pci_dev);++ if (!error && drv && drv->pm && drv->pm->restore)+ error = drv->pm->restore(dev); return error; }@@ -653,17 +717,17 @@ static int pci_pm_restore_noirq(struct d struct device_driver *drv = dev->driver; int error = 0; - pci_fixup_device(pci_fixup_resume, pci_dev);+ if (pci_has_legacy_pm_support(pci_dev)) {+ pci_fixup_device(pci_fixup_resume_early, pci_dev);+ return pci_legacy_resume_early(dev);+ }++ if (!pci_restore_standard_config(pci_dev)) {+ pci_fixup_device(pci_fixup_resume_early, pci_dev); - if (drv && drv->pm) {- if (drv->pm->restore_noirq)+ if (drv && drv->pm && drv->pm->restore_noirq) error = drv->pm->restore_noirq(dev);- } else if (pci_has_legacy_pm_support(pci_dev)) {- error = pci_legacy_resume_early(dev);- } else {- pci_default_pm_resume_early(pci_dev); }- pci_fixup_device(pci_fixup_resume_early, pci_dev); return error; }Index: linux-2.6/drivers/pci/pci.c===================================================================--- linux-2.6.orig/drivers/pci/pci.c+++ linux-2.6/drivers/pci/pci.c@@ -526,7 +526,7 @@ pci_raw_set_power_state(struct pci_dev * * @dev: PCI device to handle. * @state: State to cache in case the device doesn't have the PM capability */-static void pci_update_current_state(struct pci_dev *dev, pci_power_t state)+void pci_update_current_state(struct pci_dev *dev, pci_power_t state) { if (dev->pm_cap) { u16 pmcsr;Index: linux-2.6/drivers/pci/pci.h===================================================================--- linux-2.6.orig/drivers/pci/pci.h+++ linux-2.6/drivers/pci/pci.h@@ -121,6 +121,7 @@ static inline int pci_no_d1d2(struct pci return (dev->no_d1d2 || parent_dstates); }+extern void pci_update_current_state(struct pci_dev *dev, pci_power_t state); extern int pcie_mch_quirk; extern struct device_attribute pci_dev_attrs[]; extern struct device_attribute dev_attr_cpuaffinity;????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m????????????I?

2008-12-30 22:51:47

Subject: [RFC][PATCH] PCI PM: Make new suspend-resume callbacks carry out core operations (rev. 2)

Subject: [RFC][PATCH 6/10] PCI PM: Rearrange code in pci-driver.c

Subject: [RFC][PATCH 8/10] PCI PM: Register power state of devices during initialization

Subject: [RFC][PATCH 3/10] PCI PM: Power-manage devices without drivers during suspend-resume

Subject: [RFC][PATCH 2/10] PCI PM: Add suspend counterpart of pci_reenable_device

Subject: [RFC][PATCH 9/10] PCI PM: Run default PM callbacks for all devices using new framework

Subject: [RFC][PATCH 5/10] PCI PM: Avoid touching devices behind bridges in unknown state

Subject: [RFC][PATCH 10/10] PCI PM: Put PM callbacks in the order of execution

Subject: [RFC][PATCH 4/10] PCI PM: Move pci_has_legacy_pm_support

Subject: [RFC][PATCH 7/10] PCI PM: Call pci_fixup_device from legacy routines

Subject: [RFC][PATCH 0/10] PCI PM: Make new suspend-resume callbacks carry out core operations

Subject: [RFC][PATCH 1/10] PCI PM: Fix poweroff and restore callbacks

Subject: [RFC][PATCH 5/10] PCI PM: Avoid touching devices behind bridges in unknown state (rev. 2)

Subject: Re: [RFC][PATCH 1/10] PCI PM: Fix poweroff and restore callbacks

Subject: Re: [RFC][PATCH 2/10] PCI PM: Add suspend counterpart of pci_reenable_device

Subject: Re: [RFC][PATCH 3/10] PCI PM: Power-manage devices without drivers during suspend-resume

Subject: Re: [RFC][PATCH 4/10] PCI PM: Move pci_has_legacy_pm_support

Subject: Re: [RFC][PATCH 5/10] PCI PM: Avoid touching devices behind bridges in unknown state (rev. 2)

Subject: Re: [RFC][PATCH 8/10] PCI PM: Register power state of devices during initialization

Subject: Re: [RFC][PATCH 10/10] PCI PM: Put PM callbacks in the order of execution

Subject: Re: [RFC][PATCH 6/10] PCI PM: Rearrange code in pci-driver.c

Subject: Re: [RFC][PATCH 7/10] PCI PM: Call pci_fixup_device from legacy routines

Subject: Re: [RFC][PATCH 9/10] PCI PM: Run default PM callbacks for all devices using new framework

Subject: Re: [RFC][PATCH 2/10] PCI PM: Add suspend counterpart of pci_reenable_device

Subject: Re: [RFC][PATCH 5/10] PCI PM: Avoid touching devices behind bridges in unknown state (rev. 2)

Subject: Re: [RFC][PATCH 6/10] PCI PM: Rearrange code in pci-driver.c

Subject: Re: [RFC][PATCH 5/10] PCI PM: Avoid touching devices behind bridges in unknown state (rev. 2)

Subject: Re: [RFC][PATCH 5/10] PCI PM: Avoid touching devices behind bridges in unknown state (rev. 2)