From 0c7376ada9508141becec9b897d73b65ce66a15a Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Fri, 7 Jun 2019 00:32:31 +0200 Subject: PCI: PM: Replace pci_dev_keep_suspended() with two functions The code in pci_dev_keep_suspended() is relatively hard to follow due to the negative checks in it and in its callers and the function has a possible side-effect (disabling the PME) which doesn't really match its role. For this reason, move the PME disabling from pci_dev_keep_suspended() to a separate function and change the semantics (and name) of the rest of it, so that 'true' is returned when the device needs to be resumed (and not the other way around). Change the callers of pci_dev_keep_suspended() accordingly. While at it, make the code flow in pci_pm_poweroff() reflect the pci_pm_suspend() more closely to avoid arbitrary differences between them. This is a cosmetic change with no intention to alter behavior. Signed-off-by: Rafael J. Wysocki Reviewed-by: Mika Westerberg --- drivers/pci/pci-driver.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) (limited to 'drivers/pci/pci-driver.c') diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c index 5eadbc3d0969..dd656cf19a12 100644 --- a/drivers/pci/pci-driver.c +++ b/drivers/pci/pci-driver.c @@ -679,6 +679,7 @@ static bool pci_has_legacy_pm_support(struct pci_dev *pci_dev) static int pci_pm_prepare(struct device *dev) { struct device_driver *drv = dev->driver; + struct pci_dev *pci_dev = to_pci_dev(dev); if (drv && drv->pm && drv->pm->prepare) { int error = drv->pm->prepare(dev); @@ -688,7 +689,15 @@ static int pci_pm_prepare(struct device *dev) if (!error && dev_pm_test_driver_flags(dev, DPM_FLAG_SMART_PREPARE)) return 0; } - return pci_dev_keep_suspended(to_pci_dev(dev)); + if (pci_dev_need_resume(pci_dev)) + return 0; + + /* + * The PME setting needs to be adjusted here in case the direct-complete + * optimization is used with respect to this device. + */ + pci_dev_adjust_pme(pci_dev); + return 1; } static void pci_pm_complete(struct device *dev) @@ -758,9 +767,11 @@ static int pci_pm_suspend(struct device *dev) * better to resume the device from runtime suspend here. */ if (!dev_pm_test_driver_flags(dev, DPM_FLAG_SMART_SUSPEND) || - !pci_dev_keep_suspended(pci_dev)) { + pci_dev_need_resume(pci_dev)) { pm_runtime_resume(dev); pci_dev->state_saved = false; + } else { + pci_dev_adjust_pme(pci_dev); } if (pm->suspend) { @@ -1108,10 +1119,13 @@ static int pci_pm_poweroff(struct device *dev) /* The reason to do that is the same as in pci_pm_suspend(). */ if (!dev_pm_test_driver_flags(dev, DPM_FLAG_SMART_SUSPEND) || - !pci_dev_keep_suspended(pci_dev)) + pci_dev_need_resume(pci_dev)) { pm_runtime_resume(dev); + pci_dev->state_saved = false; + } else { + pci_dev_adjust_pme(pci_dev); + } - pci_dev->state_saved = false; if (pm->poweroff) { int error; -- cgit v1.2.3 From b51033e06c2ebbad322370f4a35c84488e61b342 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Tue, 25 Jun 2019 14:09:12 +0200 Subject: PCI: PM/ACPI: Refresh all stale power state data in pci_pm_complete() In pci_pm_complete() there are checks to decide whether or not to resume devices that were left in runtime-suspend during the preceding system-wide transition into a sleep state. They involve checking the current power state of the device and comparing it with the power state of it set before the preceding system-wide transition, but the platform component of the device's power state is not handled correctly in there. Namely, on platforms with ACPI, the device power state information needs to be updated with care, so that the reference counters of power resources used by the device (if any) are set to ensure that the refreshed power state of it will be maintained going forward. To that end, introduce a new ->refresh_state() platform PM callback for PCI devices, for asking the platform to refresh the device power state data and ensure that the corresponding power state will be maintained going forward, make it invoke acpi_device_update_power() (for devices with ACPI PM) on platforms with ACPI and make pci_pm_complete() use it, through a new pci_refresh_power_state() wrapper function. Fixes: a0d2a959d3da (PCI: Avoid unnecessary resume after direct-complete) Signed-off-by: Rafael J. Wysocki Reviewed-by: Mika Westerberg --- drivers/pci/pci-acpi.c | 9 +++++++++ drivers/pci/pci-driver.c | 9 ++++++++- drivers/pci/pci.c | 21 +++++++++++++++++++++ drivers/pci/pci.h | 4 ++++ 4 files changed, 42 insertions(+), 1 deletion(-) (limited to 'drivers/pci/pci-driver.c') diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c index 2abe0eeafb53..45049f558860 100644 --- a/drivers/pci/pci-acpi.c +++ b/drivers/pci/pci-acpi.c @@ -692,6 +692,14 @@ static pci_power_t acpi_pci_get_power_state(struct pci_dev *dev) return state_conv[state]; } +static void acpi_pci_refresh_power_state(struct pci_dev *dev) +{ + struct acpi_device *adev = ACPI_COMPANION(&dev->dev); + + if (adev && acpi_device_power_manageable(adev)) + acpi_device_update_power(adev, NULL); +} + static int acpi_pci_propagate_wakeup(struct pci_bus *bus, bool enable) { while (bus->parent) { @@ -749,6 +757,7 @@ static const struct pci_platform_pm_ops acpi_pci_platform_pm = { .is_manageable = acpi_pci_power_manageable, .set_state = acpi_pci_set_power_state, .get_state = acpi_pci_get_power_state, + .refresh_state = acpi_pci_refresh_power_state, .choose_state = acpi_pci_choose_state, .set_wakeup = acpi_pci_wakeup, .need_resume = acpi_pci_need_resume, diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c index bd097ea5925c..3149f11ca265 100644 --- a/drivers/pci/pci-driver.c +++ b/drivers/pci/pci-driver.c @@ -710,7 +710,14 @@ static void pci_pm_complete(struct device *dev) if (pm_runtime_suspended(dev) && pm_resume_via_firmware()) { pci_power_t pre_sleep_state = pci_dev->current_state; - pci_update_current_state(pci_dev, pci_dev->current_state); + pci_refresh_power_state(pci_dev); + /* + * On platforms with ACPI this check may also trigger for + * devices sharing power resources if one of those power + * resources has been activated as a result of a change of the + * power state of another device sharing it. However, in that + * case it is also better to resume the device, in general. + */ if (pci_dev->current_state < pre_sleep_state) pm_request_resume(dev); } diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index e34fb2b3c466..b1f563916036 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -777,6 +777,12 @@ static inline pci_power_t platform_pci_get_power_state(struct pci_dev *dev) return pci_platform_pm ? pci_platform_pm->get_state(dev) : PCI_UNKNOWN; } +static inline void platform_pci_refresh_power_state(struct pci_dev *dev) +{ + if (pci_platform_pm && pci_platform_pm->refresh_state) + pci_platform_pm->refresh_state(dev); +} + static inline pci_power_t platform_pci_choose_state(struct pci_dev *dev) { return pci_platform_pm ? @@ -937,6 +943,21 @@ void pci_update_current_state(struct pci_dev *dev, pci_power_t state) } } +/** + * pci_refresh_power_state - Refresh the given device's power state data + * @dev: Target PCI device. + * + * Ask the platform to refresh the devices power state information and invoke + * pci_update_current_state() to update its current PCI power state. + */ +void pci_refresh_power_state(struct pci_dev *dev) +{ + if (platform_pci_power_manageable(dev)) + platform_pci_refresh_power_state(dev); + + pci_update_current_state(dev, dev->current_state); +} + /** * pci_power_up - Put the given device into D0 forcibly * @dev: PCI device to power up diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h index 7f9fd93270ed..5db6f985f16d 100644 --- a/drivers/pci/pci.h +++ b/drivers/pci/pci.h @@ -51,6 +51,8 @@ int pci_bus_error_reset(struct pci_dev *dev); * * @get_state: queries the platform firmware for a device's current power state * + * @refresh_state: asks the platform to refresh the device's power state data + * * @choose_state: returns PCI power state of given device preferred by the * platform; to be used during system-wide transitions from a * sleeping state to the working state and vice versa @@ -69,6 +71,7 @@ struct pci_platform_pm_ops { bool (*is_manageable)(struct pci_dev *dev); int (*set_state)(struct pci_dev *dev, pci_power_t state); pci_power_t (*get_state)(struct pci_dev *dev); + void (*refresh_state)(struct pci_dev *dev); pci_power_t (*choose_state)(struct pci_dev *dev); int (*set_wakeup)(struct pci_dev *dev, bool enable); bool (*need_resume)(struct pci_dev *dev); @@ -76,6 +79,7 @@ struct pci_platform_pm_ops { int pci_set_platform_pm(const struct pci_platform_pm_ops *ops); void pci_update_current_state(struct pci_dev *dev, pci_power_t state); +void pci_refresh_power_state(struct pci_dev *dev); void pci_power_up(struct pci_dev *dev); void pci_disable_enabled_device(struct pci_dev *dev); int pci_finish_runtime_suspend(struct pci_dev *dev); -- cgit v1.2.3