From 29b968b2af7782fe879190cd43c10261506ebc11 Mon Sep 17 00:00:00 2001 From: Paul Gortmaker Date: Sun, 5 Jun 2016 13:58:15 -0400 Subject: [PATCH 01/12] PM / clk: export symbols for existing pm_clk_<...> API fcns While trying to convert a DMA driver from bool to tristate, we encountered the following: ERROR: "pm_clk_add_clk" [drivers/dma/tegra210-adma.ko] undefined! ERROR: "pm_clk_create" [drivers/dma/tegra210-adma.ko] undefined! ERROR: "pm_clk_destroy" [drivers/dma/tegra210-adma.ko] undefined! ERROR: "pm_clk_suspend" [drivers/dma/tegra210-adma.ko] undefined! ERROR: "pm_clk_resume" [drivers/dma/tegra210-adma.ko] undefined! Since in principle there is nothing preventing these functions from being used in modular code as well as builtin, we add the export of them. We expand the scope to also include: pm_clk_add of_pm_clk_add_clks pm_clk_remove pm_clk_remove_clk pm_clk_init pm_clk_runtime_suspend pm_clk_runtime_resume pm_clk_add_notifier ...since these functions are also non-static and presumably form part of the existing API used by other drivers that may become modular in the future. Signed-off-by: Paul Gortmaker Acked-by: Pavel Machek Signed-off-by: Rafael J. Wysocki --- drivers/base/power/clock_ops.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/drivers/base/power/clock_ops.c b/drivers/base/power/clock_ops.c index 3657ac1cb801..761f5c21f9f0 100644 --- a/drivers/base/power/clock_ops.c +++ b/drivers/base/power/clock_ops.c @@ -121,6 +121,7 @@ int pm_clk_add(struct device *dev, const char *con_id) { return __pm_clk_add(dev, con_id, NULL); } +EXPORT_SYMBOL_GPL(pm_clk_add); /** * pm_clk_add_clk - Start using a device clock for power management. @@ -136,6 +137,7 @@ int pm_clk_add_clk(struct device *dev, struct clk *clk) { return __pm_clk_add(dev, NULL, clk); } +EXPORT_SYMBOL_GPL(pm_clk_add_clk); /** @@ -192,6 +194,7 @@ error: return ret; } +EXPORT_SYMBOL_GPL(of_pm_clk_add_clks); /** * __pm_clk_remove - Destroy PM clock entry. @@ -252,6 +255,7 @@ void pm_clk_remove(struct device *dev, const char *con_id) __pm_clk_remove(ce); } +EXPORT_SYMBOL_GPL(pm_clk_remove); /** * pm_clk_remove_clk - Stop using a device clock for power management. @@ -285,6 +289,7 @@ void pm_clk_remove_clk(struct device *dev, struct clk *clk) __pm_clk_remove(ce); } +EXPORT_SYMBOL_GPL(pm_clk_remove_clk); /** * pm_clk_init - Initialize a device's list of power management clocks. @@ -299,6 +304,7 @@ void pm_clk_init(struct device *dev) if (psd) INIT_LIST_HEAD(&psd->clock_list); } +EXPORT_SYMBOL_GPL(pm_clk_init); /** * pm_clk_create - Create and initialize a device's list of PM clocks. @@ -311,6 +317,7 @@ int pm_clk_create(struct device *dev) { return dev_pm_get_subsys_data(dev); } +EXPORT_SYMBOL_GPL(pm_clk_create); /** * pm_clk_destroy - Destroy a device's list of power management clocks. @@ -345,6 +352,7 @@ void pm_clk_destroy(struct device *dev) __pm_clk_remove(ce); } } +EXPORT_SYMBOL_GPL(pm_clk_destroy); /** * pm_clk_suspend - Disable clocks in a device's PM clock list. @@ -375,6 +383,7 @@ int pm_clk_suspend(struct device *dev) return 0; } +EXPORT_SYMBOL_GPL(pm_clk_suspend); /** * pm_clk_resume - Enable clocks in a device's PM clock list. @@ -400,6 +409,7 @@ int pm_clk_resume(struct device *dev) return 0; } +EXPORT_SYMBOL_GPL(pm_clk_resume); /** * pm_clk_notify - Notify routine for device addition and removal. @@ -480,6 +490,7 @@ int pm_clk_runtime_suspend(struct device *dev) return 0; } +EXPORT_SYMBOL_GPL(pm_clk_runtime_suspend); int pm_clk_runtime_resume(struct device *dev) { @@ -495,6 +506,7 @@ int pm_clk_runtime_resume(struct device *dev) return pm_generic_runtime_resume(dev); } +EXPORT_SYMBOL_GPL(pm_clk_runtime_resume); #else /* !CONFIG_PM_CLK */ @@ -598,3 +610,4 @@ void pm_clk_add_notifier(struct bus_type *bus, clknb->nb.notifier_call = pm_clk_notify; bus_register_notifier(bus, &clknb->nb); } +EXPORT_SYMBOL_GPL(pm_clk_add_notifier); From 39dd0f234fc37da071dadbd9b49fe800d62139b4 Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Mon, 30 May 2016 11:43:07 +0200 Subject: [PATCH 02/12] PM / Domains: Allow genpd to power on during system PM phases If a PM domain is powered off when the first device starts its system PM prepare phase, genpd prevents any further attempts to power on the PM domain during the following system PM phases. Not until the system PM complete phase is finalized for all devices in the PM domain, genpd again allows it to be powered on. This behaviour needs to be changed, as a subsystem/driver for a device in the same PM domain may still need to be able to serve requests in some of the system PM phases. Accordingly, it may need to runtime resume its device and thus also request the corresponding PM domain to be powered on. To deal with these scenarios, let's make the device operational in the system PM prepare phase by runtime resuming it, no matter if the PM domain is powered on or off. Changing this also enables us to remove genpd's suspend_power_off flag, as it's being used to track this condition. Additionally, we must allow the PM domain to be powered on via runtime PM during the system PM phases. This change also requires a fix in the AMD ACP (Audio CoProcessor) drm driver. It registers a genpd to model the ACP as a PM domain, but unfortunately it's also abuses genpd's "internal" suspend_power_off flag to deal with a corner case at system PM resume. More precisely, the so called SMU block powers on the ACP at system PM resume, unconditionally if it's being used or not. This may lead to that genpd's internal status of the power state, may not correctly reflect the power state of the HW after a system PM resume. Because of changing the behaviour of genpd, by runtime resuming devices in the prepare phase, the AMD ACP drm driver no longer have to deal with this corner case. So let's just drop the related code in this driver. Signed-off-by: Ulf Hansson Reviewed-by: Kevin Hilman Acked-by: Maruthi Bayyavarapu Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 84 +++++++++---------------- drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c | 23 ------- include/linux/pm_domain.h | 1 - 3 files changed, 31 insertions(+), 77 deletions(-) diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index de23b648fce3..a608f521e6fc 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -187,8 +187,7 @@ static int genpd_poweron(struct generic_pm_domain *genpd, unsigned int depth) struct gpd_link *link; int ret = 0; - if (genpd->status == GPD_STATE_ACTIVE - || (genpd->prepared_count > 0 && genpd->suspend_power_off)) + if (genpd->status == GPD_STATE_ACTIVE) return 0; /* @@ -735,21 +734,22 @@ static int pm_genpd_prepare(struct device *dev) mutex_lock(&genpd->lock); - if (genpd->prepared_count++ == 0) { + if (genpd->prepared_count++ == 0) genpd->suspended_count = 0; - genpd->suspend_power_off = genpd->status == GPD_STATE_POWER_OFF; - } mutex_unlock(&genpd->lock); - if (genpd->suspend_power_off) - return 0; - /* - * The PM domain must be in the GPD_STATE_ACTIVE state at this point, - * so genpd_poweron() will return immediately, but if the device - * is suspended (e.g. it's been stopped by genpd_stop_dev()), we need - * to make it operational. + * Even if the PM domain is powered off at this point, we can't expect + * it to remain in that state during the entire system PM suspend + * phase. Any subsystem/driver for a device in the PM domain, may still + * need to serve a request which may require the device to be runtime + * resumed and its PM domain to be powered. + * + * As we are disabling runtime PM at this point, we are preventing the + * subsystem/driver to decide themselves. For that reason, we need to + * make sure the device is operational as it may be required in some + * cases. */ pm_runtime_resume(dev); __pm_runtime_disable(dev, false); @@ -758,8 +758,7 @@ static int pm_genpd_prepare(struct device *dev) if (ret) { mutex_lock(&genpd->lock); - if (--genpd->prepared_count == 0) - genpd->suspend_power_off = false; + genpd->prepared_count--; mutex_unlock(&genpd->lock); pm_runtime_enable(dev); @@ -786,7 +785,7 @@ static int pm_genpd_suspend(struct device *dev) if (IS_ERR(genpd)) return -EINVAL; - return genpd->suspend_power_off ? 0 : pm_generic_suspend(dev); + return pm_generic_suspend(dev); } /** @@ -807,7 +806,7 @@ static int pm_genpd_suspend_late(struct device *dev) if (IS_ERR(genpd)) return -EINVAL; - return genpd->suspend_power_off ? 0 : pm_generic_suspend_late(dev); + return pm_generic_suspend_late(dev); } /** @@ -827,8 +826,7 @@ static int pm_genpd_suspend_noirq(struct device *dev) if (IS_ERR(genpd)) return -EINVAL; - if (genpd->suspend_power_off - || (dev->power.wakeup_path && genpd_dev_active_wakeup(genpd, dev))) + if (dev->power.wakeup_path && genpd_dev_active_wakeup(genpd, dev)) return 0; genpd_stop_dev(genpd, dev); @@ -860,8 +858,7 @@ static int pm_genpd_resume_noirq(struct device *dev) if (IS_ERR(genpd)) return -EINVAL; - if (genpd->suspend_power_off - || (dev->power.wakeup_path && genpd_dev_active_wakeup(genpd, dev))) + if (dev->power.wakeup_path && genpd_dev_active_wakeup(genpd, dev)) return 0; /* @@ -894,7 +891,7 @@ static int pm_genpd_resume_early(struct device *dev) if (IS_ERR(genpd)) return -EINVAL; - return genpd->suspend_power_off ? 0 : pm_generic_resume_early(dev); + return pm_generic_resume_early(dev); } /** @@ -915,7 +912,7 @@ static int pm_genpd_resume(struct device *dev) if (IS_ERR(genpd)) return -EINVAL; - return genpd->suspend_power_off ? 0 : pm_generic_resume(dev); + return pm_generic_resume(dev); } /** @@ -936,7 +933,7 @@ static int pm_genpd_freeze(struct device *dev) if (IS_ERR(genpd)) return -EINVAL; - return genpd->suspend_power_off ? 0 : pm_generic_freeze(dev); + return pm_generic_freeze(dev); } /** @@ -958,7 +955,7 @@ static int pm_genpd_freeze_late(struct device *dev) if (IS_ERR(genpd)) return -EINVAL; - return genpd->suspend_power_off ? 0 : pm_generic_freeze_late(dev); + return pm_generic_freeze_late(dev); } /** @@ -980,7 +977,7 @@ static int pm_genpd_freeze_noirq(struct device *dev) if (IS_ERR(genpd)) return -EINVAL; - return genpd->suspend_power_off ? 0 : genpd_stop_dev(genpd, dev); + return genpd_stop_dev(genpd, dev); } /** @@ -1000,8 +997,7 @@ static int pm_genpd_thaw_noirq(struct device *dev) if (IS_ERR(genpd)) return -EINVAL; - return genpd->suspend_power_off ? - 0 : genpd_start_dev(genpd, dev); + return genpd_start_dev(genpd, dev); } /** @@ -1023,7 +1019,7 @@ static int pm_genpd_thaw_early(struct device *dev) if (IS_ERR(genpd)) return -EINVAL; - return genpd->suspend_power_off ? 0 : pm_generic_thaw_early(dev); + return pm_generic_thaw_early(dev); } /** @@ -1044,7 +1040,7 @@ static int pm_genpd_thaw(struct device *dev) if (IS_ERR(genpd)) return -EINVAL; - return genpd->suspend_power_off ? 0 : pm_generic_thaw(dev); + return pm_generic_thaw(dev); } /** @@ -1072,26 +1068,13 @@ static int pm_genpd_restore_noirq(struct device *dev) * At this point suspended_count == 0 means we are being run for the * first time for the given domain in the present cycle. */ - if (genpd->suspended_count++ == 0) { + if (genpd->suspended_count++ == 0) /* * The boot kernel might put the domain into arbitrary state, * so make it appear as powered off to pm_genpd_sync_poweron(), * so that it tries to power it on in case it was really off. */ genpd->status = GPD_STATE_POWER_OFF; - if (genpd->suspend_power_off) { - /* - * If the domain was off before the hibernation, make - * sure it will be off going forward. - */ - genpd_power_off(genpd, true); - - return 0; - } - } - - if (genpd->suspend_power_off) - return 0; pm_genpd_sync_poweron(genpd, true); @@ -1110,7 +1093,6 @@ static int pm_genpd_restore_noirq(struct device *dev) static void pm_genpd_complete(struct device *dev) { struct generic_pm_domain *genpd; - bool run_complete; dev_dbg(dev, "%s()\n", __func__); @@ -1120,18 +1102,14 @@ static void pm_genpd_complete(struct device *dev) mutex_lock(&genpd->lock); - run_complete = !genpd->suspend_power_off; - if (--genpd->prepared_count == 0) - genpd->suspend_power_off = false; + genpd->prepared_count--; mutex_unlock(&genpd->lock); - if (run_complete) { - pm_generic_complete(dev); - pm_runtime_set_active(dev); - pm_runtime_enable(dev); - pm_request_idle(dev); - } + pm_generic_complete(dev); + pm_runtime_set_active(dev); + pm_runtime_enable(dev); + pm_request_idle(dev); } /** diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c index 252edba16e36..892d60fb225b 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c @@ -421,29 +421,6 @@ static int acp_suspend(void *handle) static int acp_resume(void *handle) { - int i, ret; - struct acp_pm_domain *apd; - struct amdgpu_device *adev = (struct amdgpu_device *)handle; - - /* return early if no ACP */ - if (!adev->acp.acp_genpd) - return 0; - - /* SMU block will power on ACP irrespective of ACP runtime status. - * Power off explicitly based on genpd ACP runtime status so that ACP - * hw and ACP-genpd status are in sync. - * 'suspend_power_off' represents "Power status before system suspend" - */ - if (adev->acp.acp_genpd->gpd.suspend_power_off == true) { - apd = container_of(&adev->acp.acp_genpd->gpd, - struct acp_pm_domain, gpd); - - for (i = 4; i >= 0 ; i--) { - ret = acp_suspend_tile(apd->cgs_dev, ACP_TILE_P1 + i); - if (ret) - pr_err("ACP tile %d tile suspend failed\n", i); - } - } return 0; } diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h index 39285c7bd3f5..dd5b0447d572 100644 --- a/include/linux/pm_domain.h +++ b/include/linux/pm_domain.h @@ -57,7 +57,6 @@ struct generic_pm_domain { unsigned int device_count; /* Number of devices */ unsigned int suspended_count; /* System suspend device counter */ unsigned int prepared_count; /* Suspend counter of prepared devices */ - bool suspend_power_off; /* Power status before system suspend */ int (*power_off)(struct generic_pm_domain *domain); int (*power_on)(struct generic_pm_domain *domain); struct gpd_dev_ops dev_ops; From 800188538965d90759cea13bcb4f87a214cf5c53 Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Mon, 30 May 2016 11:43:08 +0200 Subject: [PATCH 03/12] PM / Domains: Remove redundant wrapper functions for system PM Due to the previous changes in genpd, which removed the suspend_power_off flag, several of the system PM callbacks no longer do any additional checks but only invoke corresponding pm_generic_* helper functions. To clean up the code, drop these wrapper functions as they have become redundant. Instead, assign the system PM callbacks directly to the pm_generic_*() helper functions. While changing this, it has bocame clear that some of the current system PM callbacks in genpd invoke wrong driver callbacks. For example, the genpd's ->restore() callback invokes pm_generic_resume(), while that should be pm_generic_restore(). Fix that as well. Signed-off-by: Ulf Hansson Reviewed-by: Kevin Hilman Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 203 +++--------------------------------- 1 file changed, 12 insertions(+), 191 deletions(-) diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index a608f521e6fc..658eb1b229f8 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -767,48 +767,6 @@ static int pm_genpd_prepare(struct device *dev) return ret; } -/** - * pm_genpd_suspend - Suspend a device belonging to an I/O PM domain. - * @dev: Device to suspend. - * - * Suspend a device under the assumption that its pm_domain field points to the - * domain member of an object of type struct generic_pm_domain representing - * a PM domain consisting of I/O devices. - */ -static int pm_genpd_suspend(struct device *dev) -{ - struct generic_pm_domain *genpd; - - dev_dbg(dev, "%s()\n", __func__); - - genpd = dev_to_genpd(dev); - if (IS_ERR(genpd)) - return -EINVAL; - - return pm_generic_suspend(dev); -} - -/** - * pm_genpd_suspend_late - Late suspend of a device from an I/O PM domain. - * @dev: Device to suspend. - * - * Carry out a late suspend of a device under the assumption that its - * pm_domain field points to the domain member of an object of type - * struct generic_pm_domain representing a PM domain consisting of I/O devices. - */ -static int pm_genpd_suspend_late(struct device *dev) -{ - struct generic_pm_domain *genpd; - - dev_dbg(dev, "%s()\n", __func__); - - genpd = dev_to_genpd(dev); - if (IS_ERR(genpd)) - return -EINVAL; - - return pm_generic_suspend_late(dev); -} - /** * pm_genpd_suspend_noirq - Completion of suspend of device in an I/O PM domain. * @dev: Device to suspend. @@ -872,92 +830,6 @@ static int pm_genpd_resume_noirq(struct device *dev) return genpd_start_dev(genpd, dev); } -/** - * pm_genpd_resume_early - Early resume of a device in an I/O PM domain. - * @dev: Device to resume. - * - * Carry out an early resume of a device under the assumption that its - * pm_domain field points to the domain member of an object of type - * struct generic_pm_domain representing a power domain consisting of I/O - * devices. - */ -static int pm_genpd_resume_early(struct device *dev) -{ - struct generic_pm_domain *genpd; - - dev_dbg(dev, "%s()\n", __func__); - - genpd = dev_to_genpd(dev); - if (IS_ERR(genpd)) - return -EINVAL; - - return pm_generic_resume_early(dev); -} - -/** - * pm_genpd_resume - Resume of device in an I/O PM domain. - * @dev: Device to resume. - * - * Resume a device under the assumption that its pm_domain field points to the - * domain member of an object of type struct generic_pm_domain representing - * a power domain consisting of I/O devices. - */ -static int pm_genpd_resume(struct device *dev) -{ - struct generic_pm_domain *genpd; - - dev_dbg(dev, "%s()\n", __func__); - - genpd = dev_to_genpd(dev); - if (IS_ERR(genpd)) - return -EINVAL; - - return pm_generic_resume(dev); -} - -/** - * pm_genpd_freeze - Freezing a device in an I/O PM domain. - * @dev: Device to freeze. - * - * Freeze a device under the assumption that its pm_domain field points to the - * domain member of an object of type struct generic_pm_domain representing - * a power domain consisting of I/O devices. - */ -static int pm_genpd_freeze(struct device *dev) -{ - struct generic_pm_domain *genpd; - - dev_dbg(dev, "%s()\n", __func__); - - genpd = dev_to_genpd(dev); - if (IS_ERR(genpd)) - return -EINVAL; - - return pm_generic_freeze(dev); -} - -/** - * pm_genpd_freeze_late - Late freeze of a device in an I/O PM domain. - * @dev: Device to freeze. - * - * Carry out a late freeze of a device under the assumption that its - * pm_domain field points to the domain member of an object of type - * struct generic_pm_domain representing a power domain consisting of I/O - * devices. - */ -static int pm_genpd_freeze_late(struct device *dev) -{ - struct generic_pm_domain *genpd; - - dev_dbg(dev, "%s()\n", __func__); - - genpd = dev_to_genpd(dev); - if (IS_ERR(genpd)) - return -EINVAL; - - return pm_generic_freeze_late(dev); -} - /** * pm_genpd_freeze_noirq - Completion of freezing a device in an I/O PM domain. * @dev: Device to freeze. @@ -1000,49 +872,6 @@ static int pm_genpd_thaw_noirq(struct device *dev) return genpd_start_dev(genpd, dev); } -/** - * pm_genpd_thaw_early - Early thaw of device in an I/O PM domain. - * @dev: Device to thaw. - * - * Carry out an early thaw of a device under the assumption that its - * pm_domain field points to the domain member of an object of type - * struct generic_pm_domain representing a power domain consisting of I/O - * devices. - */ -static int pm_genpd_thaw_early(struct device *dev) -{ - struct generic_pm_domain *genpd; - - dev_dbg(dev, "%s()\n", __func__); - - genpd = dev_to_genpd(dev); - if (IS_ERR(genpd)) - return -EINVAL; - - return pm_generic_thaw_early(dev); -} - -/** - * pm_genpd_thaw - Thaw a device belonging to an I/O power domain. - * @dev: Device to thaw. - * - * Thaw a device under the assumption that its pm_domain field points to the - * domain member of an object of type struct generic_pm_domain representing - * a power domain consisting of I/O devices. - */ -static int pm_genpd_thaw(struct device *dev) -{ - struct generic_pm_domain *genpd; - - dev_dbg(dev, "%s()\n", __func__); - - genpd = dev_to_genpd(dev); - if (IS_ERR(genpd)) - return -EINVAL; - - return pm_generic_thaw(dev); -} - /** * pm_genpd_restore_noirq - Start of restore of device in an I/O PM domain. * @dev: Device to resume. @@ -1151,18 +980,10 @@ EXPORT_SYMBOL_GPL(pm_genpd_syscore_poweron); #else /* !CONFIG_PM_SLEEP */ #define pm_genpd_prepare NULL -#define pm_genpd_suspend NULL -#define pm_genpd_suspend_late NULL #define pm_genpd_suspend_noirq NULL -#define pm_genpd_resume_early NULL #define pm_genpd_resume_noirq NULL -#define pm_genpd_resume NULL -#define pm_genpd_freeze NULL -#define pm_genpd_freeze_late NULL #define pm_genpd_freeze_noirq NULL -#define pm_genpd_thaw_early NULL #define pm_genpd_thaw_noirq NULL -#define pm_genpd_thaw NULL #define pm_genpd_restore_noirq NULL #define pm_genpd_complete NULL @@ -1454,24 +1275,24 @@ void pm_genpd_init(struct generic_pm_domain *genpd, genpd->domain.ops.runtime_suspend = genpd_runtime_suspend; genpd->domain.ops.runtime_resume = genpd_runtime_resume; genpd->domain.ops.prepare = pm_genpd_prepare; - genpd->domain.ops.suspend = pm_genpd_suspend; - genpd->domain.ops.suspend_late = pm_genpd_suspend_late; + genpd->domain.ops.suspend = pm_generic_suspend; + genpd->domain.ops.suspend_late = pm_generic_suspend_late; genpd->domain.ops.suspend_noirq = pm_genpd_suspend_noirq; genpd->domain.ops.resume_noirq = pm_genpd_resume_noirq; - genpd->domain.ops.resume_early = pm_genpd_resume_early; - genpd->domain.ops.resume = pm_genpd_resume; - genpd->domain.ops.freeze = pm_genpd_freeze; - genpd->domain.ops.freeze_late = pm_genpd_freeze_late; + genpd->domain.ops.resume_early = pm_generic_resume_early; + genpd->domain.ops.resume = pm_generic_resume; + genpd->domain.ops.freeze = pm_generic_freeze; + genpd->domain.ops.freeze_late = pm_generic_freeze_late; genpd->domain.ops.freeze_noirq = pm_genpd_freeze_noirq; genpd->domain.ops.thaw_noirq = pm_genpd_thaw_noirq; - genpd->domain.ops.thaw_early = pm_genpd_thaw_early; - genpd->domain.ops.thaw = pm_genpd_thaw; - genpd->domain.ops.poweroff = pm_genpd_suspend; - genpd->domain.ops.poweroff_late = pm_genpd_suspend_late; + genpd->domain.ops.thaw_early = pm_generic_thaw_early; + genpd->domain.ops.thaw = pm_generic_thaw; + genpd->domain.ops.poweroff = pm_generic_poweroff; + genpd->domain.ops.poweroff_late = pm_generic_poweroff_late; genpd->domain.ops.poweroff_noirq = pm_genpd_suspend_noirq; genpd->domain.ops.restore_noirq = pm_genpd_restore_noirq; - genpd->domain.ops.restore_early = pm_genpd_resume_early; - genpd->domain.ops.restore = pm_genpd_resume; + genpd->domain.ops.restore_early = pm_generic_restore_early; + genpd->domain.ops.restore = pm_generic_restore; genpd->domain.ops.complete = pm_genpd_complete; if (genpd->flags & GENPD_FLAG_PM_CLK) { From 9b002b8f0e386966dfc2dddf47eebed3b71ef876 Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Mon, 30 May 2016 11:33:11 +0200 Subject: [PATCH 04/12] PM / Domains: Remove redundant pm_request_idle() call in genpd The PM core increases the runtime PM usage count at the system PM prepare phase. Later when the system resumes, it does a pm_runtime_put() in the complete phase, which in addition to decrementing the usage count, does the equivalent of a pm_request_idle(). Therefore the call to pm_request_idle() from within genpd's ->complete() callback is redundant, so remove it. Signed-off-by: Ulf Hansson Reviewed-by: Kevin Hilman Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 658eb1b229f8..60a9971fc474 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -938,7 +938,6 @@ static void pm_genpd_complete(struct device *dev) pm_generic_complete(dev); pm_runtime_set_active(dev); pm_runtime_enable(dev); - pm_request_idle(dev); } /** From 9f5b52747dbf83816dcd29ea1700813aeb668c0f Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Mon, 30 May 2016 11:33:12 +0200 Subject: [PATCH 05/12] PM / Runtime: Avoid resuming devices again in pm_runtime_force_resume() If the runtime PM status of the device isn't RPM_SUSPENDED, prevent the pm_runtime_force_resume() from invoking the ->runtime_resume() callback for the device, as it's not the expected behaviour from the subsystem/driver. Signed-off-by: Ulf Hansson Reviewed-by: Kevin Hilman Signed-off-by: Rafael J. Wysocki --- drivers/base/power/runtime.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index b74690418504..09e4eb1a7286 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -1506,6 +1506,9 @@ int pm_runtime_force_resume(struct device *dev) goto out; } + if (!pm_runtime_status_suspended(dev)) + goto out; + ret = pm_runtime_set_active(dev); if (ret) goto out; From 4d23a5e84806b202d9231929c9507ef7cf7a0185 Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Mon, 30 May 2016 11:33:13 +0200 Subject: [PATCH 06/12] PM / Domains: Allow runtime PM during system PM phases In cases when a PM domain isn't powered off when genpd's ->prepare() callback is invoked, genpd runtime resumes and disables runtime PM for the device. This behaviour was needed when genpd managed intermediate states during the power off sequence, as to maintain proper low power states of devices during system PM suspend/resume. Commit ba2bbfbf6307 (PM / Domains: Remove intermediate states from the power off sequence), enables genpd to improve its behaviour in that respect. The PM core disables runtime PM at __device_suspend_late() before it calls a system PM "late" callback for a device. When resuming a device, after a corresponding "early" callback has been invoked, the PM core re-enables runtime PM. By changing genpd to allow runtime PM according to the same system PM phases as the PM core, devices can be runtime resumed by their corresponding subsystem/driver when really needed. In this way, genpd no longer need to runtime resume the device from its ->prepare() callback. In most cases that avoids unnecessary and energy- wasting operations of runtime resuming devices that have nothing to do, only to runtime suspend them shortly after. Although, because of changing this behaviour in genpd and due to that genpd powers on the PM domain unconditionally in the system PM resume "noirq" phase, it could potentially cause a PM domain to stay powered on even if it's unused after the system has resumed. To avoid this, schedule a power off work when genpd's system PM ->complete() callback has been invoked for the last device in the PM domain. Signed-off-by: Ulf Hansson Reviewed-by: Kevin Hilman Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 34 ++++++++-------------------------- 1 file changed, 8 insertions(+), 26 deletions(-) diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 60a9971fc474..4cb57f3f0ee3 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -739,21 +739,6 @@ static int pm_genpd_prepare(struct device *dev) mutex_unlock(&genpd->lock); - /* - * Even if the PM domain is powered off at this point, we can't expect - * it to remain in that state during the entire system PM suspend - * phase. Any subsystem/driver for a device in the PM domain, may still - * need to serve a request which may require the device to be runtime - * resumed and its PM domain to be powered. - * - * As we are disabling runtime PM at this point, we are preventing the - * subsystem/driver to decide themselves. For that reason, we need to - * make sure the device is operational as it may be required in some - * cases. - */ - pm_runtime_resume(dev); - __pm_runtime_disable(dev, false); - ret = pm_generic_prepare(dev); if (ret) { mutex_lock(&genpd->lock); @@ -761,7 +746,6 @@ static int pm_genpd_prepare(struct device *dev) genpd->prepared_count--; mutex_unlock(&genpd->lock); - pm_runtime_enable(dev); } return ret; @@ -787,8 +771,6 @@ static int pm_genpd_suspend_noirq(struct device *dev) if (dev->power.wakeup_path && genpd_dev_active_wakeup(genpd, dev)) return 0; - genpd_stop_dev(genpd, dev); - /* * Since all of the "noirq" callbacks are executed sequentially, it is * guaranteed that this function will never run twice in parallel for @@ -827,7 +809,7 @@ static int pm_genpd_resume_noirq(struct device *dev) pm_genpd_sync_poweron(genpd, true); genpd->suspended_count--; - return genpd_start_dev(genpd, dev); + return 0; } /** @@ -849,7 +831,7 @@ static int pm_genpd_freeze_noirq(struct device *dev) if (IS_ERR(genpd)) return -EINVAL; - return genpd_stop_dev(genpd, dev); + return 0; } /** @@ -869,7 +851,7 @@ static int pm_genpd_thaw_noirq(struct device *dev) if (IS_ERR(genpd)) return -EINVAL; - return genpd_start_dev(genpd, dev); + return 0; } /** @@ -907,7 +889,7 @@ static int pm_genpd_restore_noirq(struct device *dev) pm_genpd_sync_poweron(genpd, true); - return genpd_start_dev(genpd, dev); + return 0; } /** @@ -929,15 +911,15 @@ static void pm_genpd_complete(struct device *dev) if (IS_ERR(genpd)) return; + pm_generic_complete(dev); + mutex_lock(&genpd->lock); genpd->prepared_count--; + if (!genpd->prepared_count) + genpd_queue_power_off_work(genpd); mutex_unlock(&genpd->lock); - - pm_generic_complete(dev); - pm_runtime_set_active(dev); - pm_runtime_enable(dev); } /** From 122a22377a3d8cc11cf18b20db13b02d5aad1a38 Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Mon, 30 May 2016 11:33:14 +0200 Subject: [PATCH 07/12] PM / Domains: Stop/start devices during system PM suspend/resume in genpd Not all subsystems/drivers that manages devices attached to a genpd makes use of the pm_runtime_force_suspend|resume() helper functions to deal with system PM suspend/resume. In cases like these and when genpd's ->stop|start() callbacks are used for the device, invoke the pm_runtime_force_suspend|resume() helper functions from genpd's "noirq" system PM callbacks. In this way we make sure to "stop" the device on suspend and to "start" it on resume. Signed-off-by: Ulf Hansson Reviewed-by: Kevin Hilman Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 4cb57f3f0ee3..9193aacf7b1b 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -761,6 +761,7 @@ static int pm_genpd_prepare(struct device *dev) static int pm_genpd_suspend_noirq(struct device *dev) { struct generic_pm_domain *genpd; + int ret; dev_dbg(dev, "%s()\n", __func__); @@ -771,6 +772,12 @@ static int pm_genpd_suspend_noirq(struct device *dev) if (dev->power.wakeup_path && genpd_dev_active_wakeup(genpd, dev)) return 0; + if (genpd->dev_ops.stop && genpd->dev_ops.start) { + ret = pm_runtime_force_suspend(dev); + if (ret) + return ret; + } + /* * Since all of the "noirq" callbacks are executed sequentially, it is * guaranteed that this function will never run twice in parallel for @@ -791,6 +798,7 @@ static int pm_genpd_suspend_noirq(struct device *dev) static int pm_genpd_resume_noirq(struct device *dev) { struct generic_pm_domain *genpd; + int ret = 0; dev_dbg(dev, "%s()\n", __func__); @@ -809,7 +817,10 @@ static int pm_genpd_resume_noirq(struct device *dev) pm_genpd_sync_poweron(genpd, true); genpd->suspended_count--; - return 0; + if (genpd->dev_ops.stop && genpd->dev_ops.start) + ret = pm_runtime_force_resume(dev); + + return ret; } /** @@ -824,6 +835,7 @@ static int pm_genpd_resume_noirq(struct device *dev) static int pm_genpd_freeze_noirq(struct device *dev) { struct generic_pm_domain *genpd; + int ret = 0; dev_dbg(dev, "%s()\n", __func__); @@ -831,7 +843,10 @@ static int pm_genpd_freeze_noirq(struct device *dev) if (IS_ERR(genpd)) return -EINVAL; - return 0; + if (genpd->dev_ops.stop && genpd->dev_ops.start) + ret = pm_runtime_force_suspend(dev); + + return ret; } /** @@ -844,6 +859,7 @@ static int pm_genpd_freeze_noirq(struct device *dev) static int pm_genpd_thaw_noirq(struct device *dev) { struct generic_pm_domain *genpd; + int ret = 0; dev_dbg(dev, "%s()\n", __func__); @@ -851,7 +867,10 @@ static int pm_genpd_thaw_noirq(struct device *dev) if (IS_ERR(genpd)) return -EINVAL; - return 0; + if (genpd->dev_ops.stop && genpd->dev_ops.start) + ret = pm_runtime_force_resume(dev); + + return ret; } /** @@ -864,6 +883,7 @@ static int pm_genpd_thaw_noirq(struct device *dev) static int pm_genpd_restore_noirq(struct device *dev) { struct generic_pm_domain *genpd; + int ret = 0; dev_dbg(dev, "%s()\n", __func__); @@ -889,7 +909,10 @@ static int pm_genpd_restore_noirq(struct device *dev) pm_genpd_sync_poweron(genpd, true); - return 0; + if (genpd->dev_ops.stop && genpd->dev_ops.start) + ret = pm_runtime_force_resume(dev); + + return ret; } /** From 71723f95463d284004bd0afe1825e6790a0c90d0 Mon Sep 17 00:00:00 2001 From: Linus Walleij Date: Mon, 20 Jun 2016 11:14:26 +0200 Subject: [PATCH 08/12] PM / runtime: print error when activating a child to unactive parent The code currently silently bails out with -EBUSY if you try to activate a child to an inactive parent. This typically happens when you have a runtime suspended parent and runtime resume your child, but forgot to set .ignore_children on the parent to true with pm_suspend_ignore_children(dev). Silently ignoring this error is not good as it gives rise to other strange behaviour like double-resume of devices after silently bailing out of the .runtime_resume() callback. Signed-off-by: Linus Walleij Signed-off-by: Rafael J. Wysocki --- drivers/base/power/runtime.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index b74690418504..e7ee8293055b 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -1045,10 +1045,14 @@ int __pm_runtime_set_status(struct device *dev, unsigned int status) */ if (!parent->power.disable_depth && !parent->power.ignore_children - && parent->power.runtime_status != RPM_ACTIVE) + && parent->power.runtime_status != RPM_ACTIVE) { + dev_err(dev, "runtime PM trying to activate child device %s but parent (%s) is not active\n", + dev_name(dev), + dev_name(parent)); error = -EBUSY; - else if (dev->power.runtime_status == RPM_SUSPENDED) + } else if (dev->power.runtime_status == RPM_SUSPENDED) { atomic_inc(&parent->power.child_count); + } spin_unlock(&parent->power.lock); From 498b5fdd40dd3881a3119d11bff8441cfd902850 Mon Sep 17 00:00:00 2001 From: Jon Hunter Date: Tue, 21 Jun 2016 11:33:25 +0100 Subject: [PATCH 09/12] PM / clk: Add support for adding a specific clock from device-tree Some drivers using the PM clocks framework need to add specific clocks from device-tree using a name by calling the functions of_clk_get_by_name() and then pm_clk_add_clk(). Rather than having drivers call both functions, add a helper function of_pm_clk_add_clk() that will call these functions so drivers can call a single function to add a specific clock from device-tree. Signed-off-by: Jon Hunter Signed-off-by: Rafael J. Wysocki --- drivers/base/power/clock_ops.c | 32 ++++++++++++++++++++++++++++++++ include/linux/pm_clock.h | 1 + 2 files changed, 33 insertions(+) diff --git a/drivers/base/power/clock_ops.c b/drivers/base/power/clock_ops.c index 761f5c21f9f0..8e2e4757adcb 100644 --- a/drivers/base/power/clock_ops.c +++ b/drivers/base/power/clock_ops.c @@ -140,6 +140,38 @@ int pm_clk_add_clk(struct device *dev, struct clk *clk) EXPORT_SYMBOL_GPL(pm_clk_add_clk); +/** + * of_pm_clk_add_clk - Start using a device clock for power management. + * @dev: Device whose clock is going to be used for power management. + * @name: Name of clock that is going to be used for power management. + * + * Add the clock described in the 'clocks' device-tree node that matches + * with the 'name' provided, to the list of clocks used for the power + * management of @dev. On success, returns 0. Returns a negative error + * code if the clock is not found or cannot be added. + */ +int of_pm_clk_add_clk(struct device *dev, const char *name) +{ + struct clk *clk; + int ret; + + if (!dev || !dev->of_node || !name) + return -EINVAL; + + clk = of_clk_get_by_name(dev->of_node, name); + if (IS_ERR(clk)) + return PTR_ERR(clk); + + ret = pm_clk_add_clk(dev, clk); + if (ret) { + clk_put(clk); + return ret; + } + + return 0; +} +EXPORT_SYMBOL_GPL(of_pm_clk_add_clk); + /** * of_pm_clk_add_clks - Start using device clock(s) for power management. * @dev: Device whose clock(s) is going to be used for power management. diff --git a/include/linux/pm_clock.h b/include/linux/pm_clock.h index 308d6044f153..09779b0ae720 100644 --- a/include/linux/pm_clock.h +++ b/include/linux/pm_clock.h @@ -42,6 +42,7 @@ extern int pm_clk_create(struct device *dev); extern void pm_clk_destroy(struct device *dev); extern int pm_clk_add(struct device *dev, const char *con_id); extern int pm_clk_add_clk(struct device *dev, struct clk *clk); +extern int of_pm_clk_add_clk(struct device *dev, const char *name); extern int of_pm_clk_add_clks(struct device *dev); extern void pm_clk_remove(struct device *dev, const char *con_id); extern void pm_clk_remove_clk(struct device *dev, struct clk *clk); From 7eb231c337e00735d4b553ed4ae7f9441598f028 Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Fri, 17 Jun 2016 12:27:52 +0200 Subject: [PATCH 10/12] PM / Domains: Convert pm_genpd_init() to return an error code The are already cases when pm_genpd_init() can fail. Currently we hide the failures instead of propagating an error code, which is a better method. Moreover, to prepare for future changes like moving away from using a fixed array-size of the struct genpd_power_state, to instead dynamically allocate data for it, the pm_genpd_init() API needs to be able to return an error code, as allocation can fail. Current users of the pm_genpd_init() is thus requested to start dealing with error codes. In the transition phase, users will have to live with only error messages being printed to log. Signed-off-by: Ulf Hansson Acked-by: Kevin Hilman Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 10 +++++++--- include/linux/pm_domain.h | 9 +++++---- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 9193aacf7b1b..a1f2aff33997 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -1258,12 +1258,14 @@ EXPORT_SYMBOL_GPL(pm_genpd_remove_subdomain); * @genpd: PM domain object to initialize. * @gov: PM domain governor to associate with the domain (may be NULL). * @is_off: Initial value of the domain's power_is_off field. + * + * Returns 0 on successful initialization, else a negative error code. */ -void pm_genpd_init(struct generic_pm_domain *genpd, - struct dev_power_governor *gov, bool is_off) +int pm_genpd_init(struct generic_pm_domain *genpd, + struct dev_power_governor *gov, bool is_off) { if (IS_ERR_OR_NULL(genpd)) - return; + return -EINVAL; INIT_LIST_HEAD(&genpd->master_links); INIT_LIST_HEAD(&genpd->slave_links); @@ -1321,6 +1323,8 @@ void pm_genpd_init(struct generic_pm_domain *genpd, mutex_lock(&gpd_list_lock); list_add(&genpd->gpd_list_node, &gpd_list); mutex_unlock(&gpd_list_lock); + + return 0; } EXPORT_SYMBOL_GPL(pm_genpd_init); diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h index dd5b0447d572..31fec858088c 100644 --- a/include/linux/pm_domain.h +++ b/include/linux/pm_domain.h @@ -127,8 +127,8 @@ extern int pm_genpd_add_subdomain(struct generic_pm_domain *genpd, struct generic_pm_domain *new_subdomain); extern int pm_genpd_remove_subdomain(struct generic_pm_domain *genpd, struct generic_pm_domain *target); -extern void pm_genpd_init(struct generic_pm_domain *genpd, - struct dev_power_governor *gov, bool is_off); +extern int pm_genpd_init(struct generic_pm_domain *genpd, + struct dev_power_governor *gov, bool is_off); extern struct dev_power_governor simple_qos_governor; extern struct dev_power_governor pm_domain_always_on_gov; @@ -163,9 +163,10 @@ static inline int pm_genpd_remove_subdomain(struct generic_pm_domain *genpd, { return -ENOSYS; } -static inline void pm_genpd_init(struct generic_pm_domain *genpd, - struct dev_power_governor *gov, bool is_off) +static inline int pm_genpd_init(struct generic_pm_domain *genpd, + struct dev_power_governor *gov, bool is_off) { + return -ENOSYS; } #endif From fe7450b05fddebd5a76a5ad280a5ae9a82ce336f Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Wed, 29 Jun 2016 02:53:48 +0200 Subject: [PATCH 11/12] PM / runtime: Asynchronous "idle" in pm_runtime_allow() Arjan reports that it takes a relatively long time to enable runtime PM for multiple devices at system startup, because all writes to the "control" attribute in sysfs are handled synchronously and if the device is suspended as a result of the write, it will block until that operation is complete. That may be avoided by passing the RPM_ASYNC flag to rpm_idle() in pm_runtime_allow() which will make it execute the device's "idle" callback asynchronously, so writes to "control" changing it from "on" to "auto" will return without waiting. Reported-by: Arjan van de Ven Signed-off-by: Rafael J. Wysocki Acked-by: Alan Stern Reviewed-by: Ulf Hansson Reviewed-by: Kevin Hilman --- drivers/base/power/runtime.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index e7ee8293055b..aa3ea5e25377 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -1260,7 +1260,7 @@ void pm_runtime_allow(struct device *dev) dev->power.runtime_auto = true; if (atomic_dec_and_test(&dev->power.usage_count)) - rpm_idle(dev, RPM_AUTO); + rpm_idle(dev, RPM_AUTO | RPM_ASYNC); out: spin_unlock_irq(&dev->power.lock); From 6ec39cf5cd6f77e3ff9ff1da75b758a47f939888 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Mon, 6 Jun 2016 17:25:33 +0300 Subject: [PATCH 12/12] PCI / PM: check all fields in pci_set_platform_pm() When assign new PCI platform PM operations check for all mandatory fields to prevent NULL pointer dereference. Signed-off-by: Andy Shevchenko Signed-off-by: Rafael J. Wysocki --- drivers/pci/pci.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index c8b4dbdd1bdd..badbddc683f0 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -530,8 +530,8 @@ static const struct pci_platform_pm_ops *pci_platform_pm; int pci_set_platform_pm(const struct pci_platform_pm_ops *ops) { - if (!ops->is_manageable || !ops->set_state || !ops->choose_state - || !ops->sleep_wake) + if (!ops->is_manageable || !ops->set_state || !ops->choose_state || + !ops->sleep_wake || !ops->run_wake || !ops->need_resume) return -EINVAL; pci_platform_pm = ops; return 0;