| Message ID | 9f9152c11c6178bbac53fd3d0ef0d38173ad4b0b.1472554278.git.lukas@wunner.de (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On Wednesday, August 31, 2016 08:15:18 AM Lukas Wunner wrote: > We currently perform a mandatory runtime resume of all PCI devices on > ->shutdown. However it is pointless to wake devices only to immediately > power them down afterwards. (Or have the firmware reset them, in case > of a reboot.) > > It seems there are only two cases when a runtime resume is actually > necessary: If the driver has declared a ->shutdown callback or if kexec > is in progress. > > Constrain resume of a device to these cases and let it slumber > otherwise, thereby conserving energy and speeding up shutdown. > > Cc: Rafael J. Wysocki <rjw@rjwysocki.net> > Signed-off-by: Lukas Wunner <lukas@wunner.de> > --- > drivers/pci/pci-driver.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c > index fd4b9c4..09a4e56 100644 > --- a/drivers/pci/pci-driver.c > +++ b/drivers/pci/pci-driver.c > @@ -459,6 +459,11 @@ static void pci_device_shutdown(struct device *dev) > struct pci_dev *pci_dev = to_pci_dev(dev); > struct pci_driver *drv = pci_dev->driver; > > + /* Fast path for suspended devices */ > + if (pm_runtime_suspended(dev) && (!drv || !drv->shutdown) && > + !kexec_in_progress) What happens if runtime suspend or resume of the device happens here? > + return; > + > pm_runtime_resume(dev); > > if (drv && drv->shutdown) > Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Sep 14, 2016 at 02:29:52AM +0200, Rafael J. Wysocki wrote: > On Wednesday, August 31, 2016 08:15:18 AM Lukas Wunner wrote: > > We currently perform a mandatory runtime resume of all PCI devices on > > ->shutdown. However it is pointless to wake devices only to immediately > > power them down afterwards. (Or have the firmware reset them, in case > > of a reboot.) > > > > It seems there are only two cases when a runtime resume is actually > > necessary: If the driver has declared a ->shutdown callback or if kexec > > is in progress. > > > > Constrain resume of a device to these cases and let it slumber > > otherwise, thereby conserving energy and speeding up shutdown. > > > > Cc: Rafael J. Wysocki <rjw@rjwysocki.net> > > Signed-off-by: Lukas Wunner <lukas@wunner.de> > > --- > > drivers/pci/pci-driver.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c > > index fd4b9c4..09a4e56 100644 > > --- a/drivers/pci/pci-driver.c > > +++ b/drivers/pci/pci-driver.c > > @@ -459,6 +459,11 @@ static void pci_device_shutdown(struct device *dev) > > struct pci_dev *pci_dev = to_pci_dev(dev); > > struct pci_driver *drv = pci_dev->driver; > > > > + /* Fast path for suspended devices */ > > + if (pm_runtime_suspended(dev) && (!drv || !drv->shutdown) && > > + !kexec_in_progress) > > What happens if runtime suspend or resume of the device happens here? You're right, good point. How about disabling runtime PM then, like this: /* Fast path for suspended devices */ if (pm_runtime_status_suspended(dev) && (!drv || !drv->shutdown) && !kexec_in_progress) { pm_runtime_disable(dev); if (pm_runtime_status_suspended(dev)) return; pm_runtime_enable(dev); } All dependents (children, and in the future, consumers) should already have been treated at that point, due to the ordering of devices_kset->list. Thanks! Lukas -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Sep 15, 2016 at 3:11 PM, Lukas Wunner <lukas@wunner.de> wrote: > On Wed, Sep 14, 2016 at 02:29:52AM +0200, Rafael J. Wysocki wrote: >> On Wednesday, August 31, 2016 08:15:18 AM Lukas Wunner wrote: >> > We currently perform a mandatory runtime resume of all PCI devices on >> > ->shutdown. However it is pointless to wake devices only to immediately >> > power them down afterwards. (Or have the firmware reset them, in case >> > of a reboot.) >> > >> > It seems there are only two cases when a runtime resume is actually >> > necessary: If the driver has declared a ->shutdown callback or if kexec >> > is in progress. >> > >> > Constrain resume of a device to these cases and let it slumber >> > otherwise, thereby conserving energy and speeding up shutdown. >> > >> > Cc: Rafael J. Wysocki <rjw@rjwysocki.net> >> > Signed-off-by: Lukas Wunner <lukas@wunner.de> >> > --- >> > drivers/pci/pci-driver.c | 5 +++++ >> > 1 file changed, 5 insertions(+) >> > >> > diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c >> > index fd4b9c4..09a4e56 100644 >> > --- a/drivers/pci/pci-driver.c >> > +++ b/drivers/pci/pci-driver.c >> > @@ -459,6 +459,11 @@ static void pci_device_shutdown(struct device *dev) >> > struct pci_dev *pci_dev = to_pci_dev(dev); >> > struct pci_driver *drv = pci_dev->driver; >> > >> > + /* Fast path for suspended devices */ >> > + if (pm_runtime_suspended(dev) && (!drv || !drv->shutdown) && >> > + !kexec_in_progress) >> >> What happens if runtime suspend or resume of the device happens here? > > You're right, good point. How about disabling runtime PM then, like this: > > /* Fast path for suspended devices */ > if (pm_runtime_status_suspended(dev) && (!drv || !drv->shutdown) && > !kexec_in_progress) { > pm_runtime_disable(dev); > if (pm_runtime_status_suspended(dev)) > return; > pm_runtime_enable(dev); > } > > All dependents (children, and in the future, consumers) should already > have been treated at that point, due to the ordering of devices_kset->list. That should work. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c index fd4b9c4..09a4e56 100644 --- a/drivers/pci/pci-driver.c +++ b/drivers/pci/pci-driver.c @@ -459,6 +459,11 @@ static void pci_device_shutdown(struct device *dev) struct pci_dev *pci_dev = to_pci_dev(dev); struct pci_driver *drv = pci_dev->driver; + /* Fast path for suspended devices */ + if (pm_runtime_suspended(dev) && (!drv || !drv->shutdown) && + !kexec_in_progress) + return; + pm_runtime_resume(dev); if (drv && drv->shutdown)
We currently perform a mandatory runtime resume of all PCI devices on ->shutdown. However it is pointless to wake devices only to immediately power them down afterwards. (Or have the firmware reset them, in case of a reboot.) It seems there are only two cases when a runtime resume is actually necessary: If the driver has declared a ->shutdown callback or if kexec is in progress. Constrain resume of a device to these cases and let it slumber otherwise, thereby conserving energy and speeding up shutdown. Cc: Rafael J. Wysocki <rjw@rjwysocki.net> Signed-off-by: Lukas Wunner <lukas@wunner.de> --- drivers/pci/pci-driver.c | 5 +++++ 1 file changed, 5 insertions(+)