linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/7] PM: sleep: core: Rearrange the handling of driver power management flags
@ 2020-04-10 15:46 Rafael J. Wysocki
  2020-04-10 15:48 ` [PATCH 1/7] PM: sleep: core: Simplify the SMART_SUSPEND flag handling Rafael J. Wysocki
                   ` (8 more replies)
  0 siblings, 9 replies; 31+ messages in thread
From: Rafael J. Wysocki @ 2020-04-10 15:46 UTC (permalink / raw)
  To: Linux PM, Alan Stern
  Cc: Linux ACPI, Linux PCI, LKML, Bjorn Helgaas, Greg Kroah-Hartman,
	Mika Westerberg, Hans De Goede, Ulf Hansson

Hi Alan,

Following our recent discussion regarding the DPM_FLAG_* family of flags [1],
I have decided to follow some of your recommendations and make changes to the
core code handling those flags.

The purpose of this is basically to make the code more consistent internally,
easier to follow and better documented.

First of all, patch [1/7] changes the PM core to skip driver-level "late"
and "noirq" suspend callbacks for devices with SMART_SUSPEND set if they are
still runtime-suspended during the "late" system-wide suspend phase (without
the patch it does that only if subsystem-level late/noirq/early suspend/resume
callbacks are not present for the device, which is demonstrably inconsistent)
and updates the resume part of the code accordingly (it doesn't need to check
whether or not the subsystem-level callbacks are present any more).

The next patch, [2/7], is purely cosmetic and its only purpose is to reduce
the LOC number and move related pieces of code closer to each other.

Patch [3/7] changes the PM core so that it doesn't skip any subsystem-level
callbacks during system-wide resume (without the patch they may be skipped in
the "early resume" and "resume" phases due to LEAVE_SUSPENDED being set which
may be problematic) and to always run the driver's ->resume callback if the
corresponding subsystem-level callback is not present (without the patch it
may be skipped if LEAVE_SUSPENDED is set) to let it reverse the changes made
by the driver's ->suspend callback (which always runs too) if need be.

Patches [4-6/7] rename one function in the PM core and two driver PM flags to
make their names better reflect their purpose.

Finally, patch [7/7] updates the documentation of the driver PM flags to
reflect the new code flows.

This patch series have been available in the git branch at

 git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git \
 pm-sleep-core

for easier web and git access.

Cheers!


[1] https://lore.kernel.org/linux-pm/Pine.LNX.4.44L0.2003251631360.1724-100000@netrider.rowland.org/




^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH 1/7] PM: sleep: core: Simplify the SMART_SUSPEND flag handling
  2020-04-10 15:46 [PATCH 0/7] PM: sleep: core: Rearrange the handling of driver power management flags Rafael J. Wysocki
@ 2020-04-10 15:48 ` Rafael J. Wysocki
  2020-04-10 15:48 ` [PATCH 2/7] PM: sleep: core: Fold functions into their callers Rafael J. Wysocki
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 31+ messages in thread
From: Rafael J. Wysocki @ 2020-04-10 15:48 UTC (permalink / raw)
  To: Linux PM, Alan Stern
  Cc: Linux ACPI, Linux PCI, LKML, Bjorn Helgaas, Greg Kroah-Hartman,
	Mika Westerberg, Hans De Goede, Ulf Hansson

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

The code to handle the SMART_SUSPEND driver PM flag is hard to follow
and somewhat inconsistent with respect to devices without middle-layer
(subsystem) callbacks.

Namely, for those devices the core takes the role of a middle layer
in providing the expected ordering of execution of callbacks (under
the assumption that the drivers setting SMART_SUSPEND can reuse their
PM-runtime callbacks directly for system-wide suspend).  To that end,
it prevents driver ->suspend_late and ->suspend_noirq callbacks from
being executed for devices that are still runtime-suspended in
__device_suspend_late(), because running the same callback funtion
that was previously run by PM-runtime for them may be invalid.

However, it does that only for devices without any middle-layer
callbacks for the late/noirq/early suspend/resume phases even
though it would be simpler and more consistent to skip the
driver-lavel callbacks for all devices with SMART_SUSPEND set
that are runtime-suspended in __device_suspend_late().

Simplify the code in accordance with the above observation.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/base/power/main.c | 118 +++++++++++++++-------------------------------
 1 file changed, 39 insertions(+), 79 deletions(-)

diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index fdd508a78ffd..5d0225573bbe 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -561,24 +561,6 @@ static void dpm_watchdog_clear(struct dpm_watchdog *wd)
 
 /*------------------------- Resume routines -------------------------*/
 
-/**
- * suspend_event - Return a "suspend" message for given "resume" one.
- * @resume_msg: PM message representing a system-wide resume transition.
- */
-static pm_message_t suspend_event(pm_message_t resume_msg)
-{
-	switch (resume_msg.event) {
-	case PM_EVENT_RESUME:
-		return PMSG_SUSPEND;
-	case PM_EVENT_THAW:
-	case PM_EVENT_RESTORE:
-		return PMSG_FREEZE;
-	case PM_EVENT_RECOVER:
-		return PMSG_HIBERNATE;
-	}
-	return PMSG_ON;
-}
-
 /**
  * dev_pm_may_skip_resume - System-wide device resume optimization check.
  * @dev: Target device.
@@ -656,37 +638,36 @@ static int device_resume_noirq(struct device *dev, pm_message_t state, bool asyn
 	if (!dpm_wait_for_superior(dev, async))
 		goto Out;
 
-	skip_resume = dev_pm_may_skip_resume(dev);
-
 	callback = dpm_subsys_resume_noirq_cb(dev, state, &info);
-	if (callback)
+	if (callback) {
+		skip_resume = false;
 		goto Run;
+	}
 
+	skip_resume = dev_pm_may_skip_resume(dev);
 	if (skip_resume)
 		goto Skip;
 
-	if (dev_pm_smart_suspend_and_suspended(dev)) {
-		pm_message_t suspend_msg = suspend_event(state);
-
-		/*
-		 * If "freeze" callbacks have been skipped during a transition
-		 * related to hibernation, the subsequent "thaw" callbacks must
-		 * be skipped too or bad things may happen.  Otherwise, resume
-		 * callbacks are going to be run for the device, so its runtime
-		 * PM status must be changed to reflect the new state after the
-		 * transition under way.
-		 */
-		if (!dpm_subsys_suspend_late_cb(dev, suspend_msg, NULL) &&
-		    !dpm_subsys_suspend_noirq_cb(dev, suspend_msg, NULL)) {
-			if (state.event == PM_EVENT_THAW) {
-				skip_resume = true;
-				goto Skip;
-			} else {
-				pm_runtime_set_active(dev);
-			}
-		}
+	/*
+	 * If "freeze" driver callbacks have been skipped during hibernation,
+	 * because the device was runtime-suspended in __device_suspend_late(),
+	 * the corresponding "thaw" callbacks must be skipped too, because
+	 * running them for a runtime-suspended device may not be valid.
+	 */
+	if (dev_pm_smart_suspend_and_suspended(dev) &&
+	    state.event == PM_EVENT_THAW) {
+		skip_resume = true;
+		goto Skip;
 	}
 
+	/*
+	 * The device is going to be resumed, so set its PM-runtime status to
+	 * "active", but do that only if DPM_FLAG_SMART_SUSPEND is set to avoid
+	 * confusing drivers that don't use it.
+	 */
+	if (dev_pm_smart_suspend_and_suspended(dev))
+		pm_runtime_set_active(dev);
+
 	if (dev->driver && dev->driver->pm) {
 		info = "noirq driver ";
 		callback = pm_noirq_op(dev->driver->pm, state);
@@ -1274,32 +1255,6 @@ static pm_callback_t dpm_subsys_suspend_noirq_cb(struct device *dev,
 	return callback;
 }
 
-static bool device_must_resume(struct device *dev, pm_message_t state,
-			       bool no_subsys_suspend_noirq)
-{
-	pm_message_t resume_msg = resume_event(state);
-
-	/*
-	 * If all of the device driver's "noirq", "late" and "early" callbacks
-	 * are invoked directly by the core, the decision to allow the device to
-	 * stay in suspend can be based on its current runtime PM status and its
-	 * wakeup settings.
-	 */
-	if (no_subsys_suspend_noirq &&
-	    !dpm_subsys_suspend_late_cb(dev, state, NULL) &&
-	    !dpm_subsys_resume_early_cb(dev, resume_msg, NULL) &&
-	    !dpm_subsys_resume_noirq_cb(dev, resume_msg, NULL))
-		return !pm_runtime_status_suspended(dev) &&
-			(resume_msg.event != PM_EVENT_RESUME ||
-			 (device_can_wakeup(dev) && !device_may_wakeup(dev)));
-
-	/*
-	 * The only safe strategy here is to require that if the device may not
-	 * be left in suspend, resume callbacks must be invoked for it.
-	 */
-	return !dev->power.may_skip_resume;
-}
-
 /**
  * __device_suspend_noirq - Execute a "noirq suspend" callback for given device.
  * @dev: Device to handle.
@@ -1313,7 +1268,6 @@ static int __device_suspend_noirq(struct device *dev, pm_message_t state, bool a
 {
 	pm_callback_t callback;
 	const char *info;
-	bool no_subsys_cb = false;
 	int error = 0;
 
 	TRACE_DEVICE(dev);
@@ -1331,9 +1285,7 @@ static int __device_suspend_noirq(struct device *dev, pm_message_t state, bool a
 	if (callback)
 		goto Run;
 
-	no_subsys_cb = !dpm_subsys_suspend_late_cb(dev, state, NULL);
-
-	if (dev_pm_smart_suspend_and_suspended(dev) && no_subsys_cb)
+	if (dev_pm_smart_suspend_and_suspended(dev))
 		goto Skip;
 
 	if (dev->driver && dev->driver->pm) {
@@ -1351,13 +1303,16 @@ static int __device_suspend_noirq(struct device *dev, pm_message_t state, bool a
 Skip:
 	dev->power.is_noirq_suspended = true;
 
-	if (dev_pm_test_driver_flags(dev, DPM_FLAG_LEAVE_SUSPENDED)) {
-		dev->power.must_resume = dev->power.must_resume ||
-				atomic_read(&dev->power.usage_count) > 1 ||
-				device_must_resume(dev, state, no_subsys_cb);
-	} else {
+	/*
+	 * Skipping the resume of devices that were in use right before the
+	 * system suspend (as indicated by their PM-runtime usage counters)
+	 * would be suboptimal.  Also resume them if doing that is not allowed
+	 * to be skipped.
+	 */
+	if (atomic_read(&dev->power.usage_count) > 1 ||
+	    !(dev_pm_test_driver_flags(dev, DPM_FLAG_LEAVE_SUSPENDED) &&
+	      dev->power.may_skip_resume))
 		dev->power.must_resume = true;
-	}
 
 	if (dev->power.must_resume)
 		dpm_superior_set_must_resume(dev);
@@ -1539,9 +1494,14 @@ static int __device_suspend_late(struct device *dev, pm_message_t state, bool as
 	if (callback)
 		goto Run;
 
-	if (dev_pm_smart_suspend_and_suspended(dev) &&
-	    !dpm_subsys_suspend_noirq_cb(dev, state, NULL))
+	if (dev_pm_smart_suspend_and_suspended(dev)) {
+		/*
+		 * In principle, the resume of the device may be skippend if it
+		 * remains in runtime suspend at this point.
+		 */
+		dev->power.may_skip_resume = true;
 		goto Skip;
+	}
 
 	if (dev->driver && dev->driver->pm) {
 		info = "late driver ";
-- 
2.16.4





^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH 2/7] PM: sleep: core: Fold functions into their callers
  2020-04-10 15:46 [PATCH 0/7] PM: sleep: core: Rearrange the handling of driver power management flags Rafael J. Wysocki
  2020-04-10 15:48 ` [PATCH 1/7] PM: sleep: core: Simplify the SMART_SUSPEND flag handling Rafael J. Wysocki
@ 2020-04-10 15:48 ` Rafael J. Wysocki
  2020-04-10 15:51 ` [PATCH 3/7] PM: sleep: core: Do not skip callbacks in the resume phase Rafael J. Wysocki
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 31+ messages in thread
From: Rafael J. Wysocki @ 2020-04-10 15:48 UTC (permalink / raw)
  To: Linux PM, Alan Stern
  Cc: Linux ACPI, Linux PCI, LKML, Bjorn Helgaas, Greg Kroah-Hartman,
	Mika Westerberg, Hans De Goede, Ulf Hansson

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

Fold four functions in the PM core that each have only one caller
now into their callers.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/base/power/main.c | 198 ++++++++++++++--------------------------------
 1 file changed, 60 insertions(+), 138 deletions(-)

diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index 5d0225573bbe..75d7cdb4de9c 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -573,43 +573,6 @@ bool dev_pm_may_skip_resume(struct device *dev)
 	return !dev->power.must_resume && pm_transition.event != PM_EVENT_RESTORE;
 }
 
-static pm_callback_t dpm_subsys_resume_noirq_cb(struct device *dev,
-						pm_message_t state,
-						const char **info_p)
-{
-	pm_callback_t callback;
-	const char *info;
-
-	if (dev->pm_domain) {
-		info = "noirq power domain ";
-		callback = pm_noirq_op(&dev->pm_domain->ops, state);
-	} else if (dev->type && dev->type->pm) {
-		info = "noirq type ";
-		callback = pm_noirq_op(dev->type->pm, state);
-	} else if (dev->class && dev->class->pm) {
-		info = "noirq class ";
-		callback = pm_noirq_op(dev->class->pm, state);
-	} else if (dev->bus && dev->bus->pm) {
-		info = "noirq bus ";
-		callback = pm_noirq_op(dev->bus->pm, state);
-	} else {
-		return NULL;
-	}
-
-	if (info_p)
-		*info_p = info;
-
-	return callback;
-}
-
-static pm_callback_t dpm_subsys_suspend_noirq_cb(struct device *dev,
-						 pm_message_t state,
-						 const char **info_p);
-
-static pm_callback_t dpm_subsys_suspend_late_cb(struct device *dev,
-						pm_message_t state,
-						const char **info_p);
-
 /**
  * device_resume_noirq - Execute a "noirq resume" callback for given device.
  * @dev: Device to handle.
@@ -621,8 +584,8 @@ static pm_callback_t dpm_subsys_suspend_late_cb(struct device *dev,
  */
 static int device_resume_noirq(struct device *dev, pm_message_t state, bool async)
 {
-	pm_callback_t callback;
-	const char *info;
+	pm_callback_t callback = NULL;
+	const char *info = NULL;
 	bool skip_resume;
 	int error = 0;
 
@@ -638,7 +601,19 @@ static int device_resume_noirq(struct device *dev, pm_message_t state, bool asyn
 	if (!dpm_wait_for_superior(dev, async))
 		goto Out;
 
-	callback = dpm_subsys_resume_noirq_cb(dev, state, &info);
+	if (dev->pm_domain) {
+		info = "noirq power domain ";
+		callback = pm_noirq_op(&dev->pm_domain->ops, state);
+	} else if (dev->type && dev->type->pm) {
+		info = "noirq type ";
+		callback = pm_noirq_op(dev->type->pm, state);
+	} else if (dev->class && dev->class->pm) {
+		info = "noirq class ";
+		callback = pm_noirq_op(dev->class->pm, state);
+	} else if (dev->bus && dev->bus->pm) {
+		info = "noirq bus ";
+		callback = pm_noirq_op(dev->bus->pm, state);
+	}
 	if (callback) {
 		skip_resume = false;
 		goto Run;
@@ -791,35 +766,6 @@ void dpm_resume_noirq(pm_message_t state)
 	cpuidle_resume();
 }
 
-static pm_callback_t dpm_subsys_resume_early_cb(struct device *dev,
-						pm_message_t state,
-						const char **info_p)
-{
-	pm_callback_t callback;
-	const char *info;
-
-	if (dev->pm_domain) {
-		info = "early power domain ";
-		callback = pm_late_early_op(&dev->pm_domain->ops, state);
-	} else if (dev->type && dev->type->pm) {
-		info = "early type ";
-		callback = pm_late_early_op(dev->type->pm, state);
-	} else if (dev->class && dev->class->pm) {
-		info = "early class ";
-		callback = pm_late_early_op(dev->class->pm, state);
-	} else if (dev->bus && dev->bus->pm) {
-		info = "early bus ";
-		callback = pm_late_early_op(dev->bus->pm, state);
-	} else {
-		return NULL;
-	}
-
-	if (info_p)
-		*info_p = info;
-
-	return callback;
-}
-
 /**
  * device_resume_early - Execute an "early resume" callback for given device.
  * @dev: Device to handle.
@@ -830,8 +776,8 @@ static pm_callback_t dpm_subsys_resume_early_cb(struct device *dev,
  */
 static int device_resume_early(struct device *dev, pm_message_t state, bool async)
 {
-	pm_callback_t callback;
-	const char *info;
+	pm_callback_t callback = NULL;
+	const char *info = NULL;
 	int error = 0;
 
 	TRACE_DEVICE(dev);
@@ -846,9 +792,19 @@ static int device_resume_early(struct device *dev, pm_message_t state, bool asyn
 	if (!dpm_wait_for_superior(dev, async))
 		goto Out;
 
-	callback = dpm_subsys_resume_early_cb(dev, state, &info);
-
-	if (!callback && dev->driver && dev->driver->pm) {
+	if (dev->pm_domain) {
+		info = "early power domain ";
+		callback = pm_late_early_op(&dev->pm_domain->ops, state);
+	} else if (dev->type && dev->type->pm) {
+		info = "early type ";
+		callback = pm_late_early_op(dev->type->pm, state);
+	} else if (dev->class && dev->class->pm) {
+		info = "early class ";
+		callback = pm_late_early_op(dev->class->pm, state);
+	} else if (dev->bus && dev->bus->pm) {
+		info = "early bus ";
+		callback = pm_late_early_op(dev->bus->pm, state);
+	} else if (dev->driver && dev->driver->pm) {
 		info = "early driver ";
 		callback = pm_late_early_op(dev->driver->pm, state);
 	}
@@ -1226,35 +1182,6 @@ static void dpm_superior_set_must_resume(struct device *dev)
 	device_links_read_unlock(idx);
 }
 
-static pm_callback_t dpm_subsys_suspend_noirq_cb(struct device *dev,
-						 pm_message_t state,
-						 const char **info_p)
-{
-	pm_callback_t callback;
-	const char *info;
-
-	if (dev->pm_domain) {
-		info = "noirq power domain ";
-		callback = pm_noirq_op(&dev->pm_domain->ops, state);
-	} else if (dev->type && dev->type->pm) {
-		info = "noirq type ";
-		callback = pm_noirq_op(dev->type->pm, state);
-	} else if (dev->class && dev->class->pm) {
-		info = "noirq class ";
-		callback = pm_noirq_op(dev->class->pm, state);
-	} else if (dev->bus && dev->bus->pm) {
-		info = "noirq bus ";
-		callback = pm_noirq_op(dev->bus->pm, state);
-	} else {
-		return NULL;
-	}
-
-	if (info_p)
-		*info_p = info;
-
-	return callback;
-}
-
 /**
  * __device_suspend_noirq - Execute a "noirq suspend" callback for given device.
  * @dev: Device to handle.
@@ -1266,8 +1193,8 @@ static pm_callback_t dpm_subsys_suspend_noirq_cb(struct device *dev,
  */
 static int __device_suspend_noirq(struct device *dev, pm_message_t state, bool async)
 {
-	pm_callback_t callback;
-	const char *info;
+	pm_callback_t callback = NULL;
+	const char *info = NULL;
 	int error = 0;
 
 	TRACE_DEVICE(dev);
@@ -1281,7 +1208,19 @@ static int __device_suspend_noirq(struct device *dev, pm_message_t state, bool a
 	if (dev->power.syscore || dev->power.direct_complete)
 		goto Complete;
 
-	callback = dpm_subsys_suspend_noirq_cb(dev, state, &info);
+	if (dev->pm_domain) {
+		info = "noirq power domain ";
+		callback = pm_noirq_op(&dev->pm_domain->ops, state);
+	} else if (dev->type && dev->type->pm) {
+		info = "noirq type ";
+		callback = pm_noirq_op(dev->type->pm, state);
+	} else if (dev->class && dev->class->pm) {
+		info = "noirq class ";
+		callback = pm_noirq_op(dev->class->pm, state);
+	} else if (dev->bus && dev->bus->pm) {
+		info = "noirq bus ";
+		callback = pm_noirq_op(dev->bus->pm, state);
+	}
 	if (callback)
 		goto Run;
 
@@ -1429,35 +1368,6 @@ static void dpm_propagate_wakeup_to_parent(struct device *dev)
 	spin_unlock_irq(&parent->power.lock);
 }
 
-static pm_callback_t dpm_subsys_suspend_late_cb(struct device *dev,
-						pm_message_t state,
-						const char **info_p)
-{
-	pm_callback_t callback;
-	const char *info;
-
-	if (dev->pm_domain) {
-		info = "late power domain ";
-		callback = pm_late_early_op(&dev->pm_domain->ops, state);
-	} else if (dev->type && dev->type->pm) {
-		info = "late type ";
-		callback = pm_late_early_op(dev->type->pm, state);
-	} else if (dev->class && dev->class->pm) {
-		info = "late class ";
-		callback = pm_late_early_op(dev->class->pm, state);
-	} else if (dev->bus && dev->bus->pm) {
-		info = "late bus ";
-		callback = pm_late_early_op(dev->bus->pm, state);
-	} else {
-		return NULL;
-	}
-
-	if (info_p)
-		*info_p = info;
-
-	return callback;
-}
-
 /**
  * __device_suspend_late - Execute a "late suspend" callback for given device.
  * @dev: Device to handle.
@@ -1468,8 +1378,8 @@ static pm_callback_t dpm_subsys_suspend_late_cb(struct device *dev,
  */
 static int __device_suspend_late(struct device *dev, pm_message_t state, bool async)
 {
-	pm_callback_t callback;
-	const char *info;
+	pm_callback_t callback = NULL;
+	const char *info = NULL;
 	int error = 0;
 
 	TRACE_DEVICE(dev);
@@ -1490,7 +1400,19 @@ static int __device_suspend_late(struct device *dev, pm_message_t state, bool as
 	if (dev->power.syscore || dev->power.direct_complete)
 		goto Complete;
 
-	callback = dpm_subsys_suspend_late_cb(dev, state, &info);
+	if (dev->pm_domain) {
+		info = "late power domain ";
+		callback = pm_late_early_op(&dev->pm_domain->ops, state);
+	} else if (dev->type && dev->type->pm) {
+		info = "late type ";
+		callback = pm_late_early_op(dev->type->pm, state);
+	} else if (dev->class && dev->class->pm) {
+		info = "late class ";
+		callback = pm_late_early_op(dev->class->pm, state);
+	} else if (dev->bus && dev->bus->pm) {
+		info = "late bus ";
+		callback = pm_late_early_op(dev->bus->pm, state);
+	}
 	if (callback)
 		goto Run;
 
-- 
2.16.4





^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH 3/7] PM: sleep: core: Do not skip callbacks in the resume phase
  2020-04-10 15:46 [PATCH 0/7] PM: sleep: core: Rearrange the handling of driver power management flags Rafael J. Wysocki
  2020-04-10 15:48 ` [PATCH 1/7] PM: sleep: core: Simplify the SMART_SUSPEND flag handling Rafael J. Wysocki
  2020-04-10 15:48 ` [PATCH 2/7] PM: sleep: core: Fold functions into their callers Rafael J. Wysocki
@ 2020-04-10 15:51 ` Rafael J. Wysocki
  2020-04-10 15:51 ` [PATCH 4/7] PM: sleep: core: Rename dev_pm_may_skip_resume() Rafael J. Wysocki
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 31+ messages in thread
From: Rafael J. Wysocki @ 2020-04-10 15:51 UTC (permalink / raw)
  To: Linux PM, Alan Stern
  Cc: Linux ACPI, Linux PCI, LKML, Bjorn Helgaas, Greg Kroah-Hartman,
	Mika Westerberg, Hans De Goede, Ulf Hansson, Linux PCI,
	Linux ACPI, Bjorn Helgaas

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

The current code in device_resume_noirq() causes the entire early
resume and resume phases of device suspend to be skipped for
devices for which the noirq resume phase have been skipped (due
to the LEAVE_SUSPENDED flag being set) on the premise that those
devices should stay in runtime-suspend after system-wide resume.

However, that may not be correct in two situations.  First, the
middle layer (subsystem) noirq resume callback may be missing for
a given device, but its early resume callback may be present and it
may need to do something even if it decides to skip the driver
callback.  Second, if the device's wakeup settings were adjusted
in the suspend phase without resuming the device (that was in
runtime suspend at that time), they most likely need to be
adjusted again in the resume phase and so the driver callback
in that phase needs to be run.

For the above reason, modify the core to allow the middle layer
->resume_late callback to run even if its ->resume_noirq callback
is missing (and the core has skipped the driver-level callback
in that phase) and to allow all device callbacks to run in the
resume phase.  Also make the core set the PM-runtime status of
devices with SMART_SUSPEND set whose resume callbacks are not
skipped to "active" in the "noirq" resume phase and update the
affected subsystems (PCI and ACPI) accordingly.

After this change, middle-layer (subsystem) callbacks will always
be invoked in all phases of system suspend and resume and driver
callbacks will always run in the prepare, suspend, resume, and
complete phases for all devices.

For devices with SMART_SUSPEND set, driver callbacks will be
skipped in the late and noirq phases of system suspend if those
devices remain in runtime suspend in __device_suspend_late().
Driver callbacks will also be skipped for them during the
noirq and early phases of the "thaw" transition related to
hibernation.

For devices with LEAVE_SUSPENDED set, driver callbacks will be
skipped in the noirq and early phases of system resume if (1) the
corresponding callbacks were skipped for them during system suspend
or (2) if the middle layer (subsystem) allows the resume driver
callbacks to be skipped by setting the power.may_skip_resume flag
for the device during system suspend.

For all devices with SMART_SUSPEND set whose driver callbacks are
invoked during system resume, the PM-runtime status will be set to
"active" (by the core).

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 Documentation/power/pci.rst |  5 +--
 drivers/acpi/acpi_lpss.c    |  6 ++--
 drivers/acpi/device_pm.c    | 15 ++++----
 drivers/base/power/main.c   | 84 ++++++++++++++++++++++-----------------------
 drivers/pci/pci-driver.c    | 18 +++++-----
 5 files changed, 61 insertions(+), 67 deletions(-)

diff --git a/Documentation/power/pci.rst b/Documentation/power/pci.rst
index 0924d29636ad..a39b2461919a 100644
--- a/Documentation/power/pci.rst
+++ b/Documentation/power/pci.rst
@@ -1035,10 +1035,7 @@ This flag is checked by the PM core, but the PCI bus type informs the PM core
 which devices may be left in suspend from its perspective (that happens during
 the "noirq" phase of system-wide suspend and analogous transitions) and next it
 uses the dev_pm_may_skip_resume() helper to decide whether or not to return from
-pci_pm_resume_noirq() early, as the PM core will skip the remaining resume
-callbacks for the device during the transition under way and will set its
-runtime PM status to "suspended" if dev_pm_may_skip_resume() returns "true" for
-it.
+pci_pm_resume_noirq() and pci_pm_resume_early() upfront.
 
 3.2. Device Runtime Power Management
 ------------------------------------
diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c
index dee999938213..c4a84df6cc98 100644
--- a/drivers/acpi/acpi_lpss.c
+++ b/drivers/acpi/acpi_lpss.c
@@ -1093,6 +1093,9 @@ static int acpi_lpss_resume_early(struct device *dev)
 	if (pdata->dev_desc->resume_from_noirq)
 		return 0;
 
+	if (dev_pm_may_skip_resume(dev))
+		return 0;
+
 	return acpi_lpss_do_resume_early(dev);
 }
 
@@ -1105,9 +1108,6 @@ static int acpi_lpss_resume_noirq(struct device *dev)
 	if (dev_pm_may_skip_resume(dev))
 		return 0;
 
-	if (dev_pm_smart_suspend_and_suspended(dev))
-		pm_runtime_set_active(dev);
-
 	ret = pm_generic_resume_noirq(dev);
 	if (ret)
 		return ret;
diff --git a/drivers/acpi/device_pm.c b/drivers/acpi/device_pm.c
index b2263ec67b43..399684085f85 100644
--- a/drivers/acpi/device_pm.c
+++ b/drivers/acpi/device_pm.c
@@ -1132,14 +1132,6 @@ static int acpi_subsys_resume_noirq(struct device *dev)
 	if (dev_pm_may_skip_resume(dev))
 		return 0;
 
-	/*
-	 * Devices with DPM_FLAG_SMART_SUSPEND may be left in runtime suspend
-	 * during system suspend, so update their runtime PM status to "active"
-	 * as they will be put into D0 going forward.
-	 */
-	if (dev_pm_smart_suspend_and_suspended(dev))
-		pm_runtime_set_active(dev);
-
 	return pm_generic_resume_noirq(dev);
 }
 
@@ -1153,7 +1145,12 @@ static int acpi_subsys_resume_noirq(struct device *dev)
  */
 static int acpi_subsys_resume_early(struct device *dev)
 {
-	int ret = acpi_dev_resume(dev);
+	int ret;
+
+	if (dev_pm_may_skip_resume(dev))
+		return 0;
+
+	ret = acpi_dev_resume(dev);
 	return ret ? ret : pm_generic_resume_early(dev);
 }
 
diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index 75d7cdb4de9c..66f205d7c7a3 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -565,12 +565,21 @@ static void dpm_watchdog_clear(struct dpm_watchdog *wd)
  * dev_pm_may_skip_resume - System-wide device resume optimization check.
  * @dev: Target device.
  *
- * Checks whether or not the device may be left in suspend after a system-wide
- * transition to the working state.
+ * Driver-level resume callbacks can be skipped for @dev if its configuration is
+ * suitable for that (power.must_resume is not set) and the current transition
+ * is not the hibernation-related "restore" one (in which case all devices must
+ * be resumed) or the current transition is the hibernation-related "thaw" one
+ * and the driver-level callbacks were skipped for @dev during the corresponding
+ * "freeze" transition (which happens when DPM_FLAG_SMART_SUSPEND is set and the
+ * device remains in runtime suspend), so running the "thaw" callbacks for it
+ * may be invalid.
  */
 bool dev_pm_may_skip_resume(struct device *dev)
 {
-	return !dev->power.must_resume && pm_transition.event != PM_EVENT_RESTORE;
+	return (!dev->power.must_resume &&
+		pm_transition.event != PM_EVENT_RESTORE) ||
+	       (dev_pm_smart_suspend_and_suspended(dev) &&
+		pm_transition.event == PM_EVENT_THAW);
 }
 
 /**
@@ -601,6 +610,22 @@ static int device_resume_noirq(struct device *dev, pm_message_t state, bool asyn
 	if (!dpm_wait_for_superior(dev, async))
 		goto Out;
 
+	skip_resume = dev_pm_may_skip_resume(dev);
+	/*
+	 * If the driver callback is skipped below or by the middle layer
+	 * callback and device_resume_early() also skips the driver callback for
+	 * this device later, it needs to appear as "suspended" to PM-runtime,
+	 * so change its status accordingly.
+	 *
+	 * Otherwise, the device is going to be resumed, so set its PM-runtime
+	 * status to "active", but do that only if DPM_FLAG_SMART_SUSPEND is set
+	 * to avoid confusing drivers that don't use it.
+	 */
+	if (skip_resume)
+		pm_runtime_set_suspended(dev);
+	else if (dev_pm_smart_suspend_and_suspended(dev))
+		pm_runtime_set_active(dev);
+
 	if (dev->pm_domain) {
 		info = "noirq power domain ";
 		callback = pm_noirq_op(&dev->pm_domain->ops, state);
@@ -614,35 +639,12 @@ static int device_resume_noirq(struct device *dev, pm_message_t state, bool asyn
 		info = "noirq bus ";
 		callback = pm_noirq_op(dev->bus->pm, state);
 	}
-	if (callback) {
-		skip_resume = false;
+	if (callback)
 		goto Run;
-	}
 
-	skip_resume = dev_pm_may_skip_resume(dev);
 	if (skip_resume)
 		goto Skip;
 
-	/*
-	 * If "freeze" driver callbacks have been skipped during hibernation,
-	 * because the device was runtime-suspended in __device_suspend_late(),
-	 * the corresponding "thaw" callbacks must be skipped too, because
-	 * running them for a runtime-suspended device may not be valid.
-	 */
-	if (dev_pm_smart_suspend_and_suspended(dev) &&
-	    state.event == PM_EVENT_THAW) {
-		skip_resume = true;
-		goto Skip;
-	}
-
-	/*
-	 * The device is going to be resumed, so set its PM-runtime status to
-	 * "active", but do that only if DPM_FLAG_SMART_SUSPEND is set to avoid
-	 * confusing drivers that don't use it.
-	 */
-	if (dev_pm_smart_suspend_and_suspended(dev))
-		pm_runtime_set_active(dev);
-
 	if (dev->driver && dev->driver->pm) {
 		info = "noirq driver ";
 		callback = pm_noirq_op(dev->driver->pm, state);
@@ -654,20 +656,6 @@ static int device_resume_noirq(struct device *dev, pm_message_t state, bool asyn
 Skip:
 	dev->power.is_noirq_suspended = false;
 
-	if (skip_resume) {
-		/* Make the next phases of resume skip the device. */
-		dev->power.is_late_suspended = false;
-		dev->power.is_suspended = false;
-		/*
-		 * The device is going to be left in suspend, but it might not
-		 * have been in runtime suspend before the system suspended, so
-		 * its runtime PM status needs to be updated to avoid confusing
-		 * the runtime PM framework when runtime PM is enabled for the
-		 * device again.
-		 */
-		pm_runtime_set_suspended(dev);
-	}
-
 Out:
 	complete_all(&dev->power.completion);
 	TRACE_RESUME(error);
@@ -804,15 +792,25 @@ static int device_resume_early(struct device *dev, pm_message_t state, bool asyn
 	} else if (dev->bus && dev->bus->pm) {
 		info = "early bus ";
 		callback = pm_late_early_op(dev->bus->pm, state);
-	} else if (dev->driver && dev->driver->pm) {
+	}
+	if (callback)
+		goto Run;
+
+	if (dev_pm_may_skip_resume(dev))
+		goto Skip;
+
+	if (dev->driver && dev->driver->pm) {
 		info = "early driver ";
 		callback = pm_late_early_op(dev->driver->pm, state);
 	}
 
+Run:
 	error = dpm_run_callback(callback, dev, state, info);
+
+Skip:
 	dev->power.is_late_suspended = false;
 
- Out:
+Out:
 	TRACE_RESUME(error);
 
 	pm_runtime_enable(dev);
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index 0454ca0e4e3f..685fbf044911 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -896,14 +896,6 @@ static int pci_pm_resume_noirq(struct device *dev)
 	if (dev_pm_may_skip_resume(dev))
 		return 0;
 
-	/*
-	 * Devices with DPM_FLAG_SMART_SUSPEND may be left in runtime suspend
-	 * during system suspend, so update their runtime PM status to "active"
-	 * as they are going to be put into D0 shortly.
-	 */
-	if (dev_pm_smart_suspend_and_suspended(dev))
-		pm_runtime_set_active(dev);
-
 	/*
 	 * In the suspend-to-idle case, devices left in D0 during suspend will
 	 * stay in D0, so it is not necessary to restore or update their
@@ -928,6 +920,14 @@ static int pci_pm_resume_noirq(struct device *dev)
 	return 0;
 }
 
+static int pci_pm_resume_early(struct device *dev)
+{
+	if (dev_pm_may_skip_resume(dev))
+		return 0;
+
+	return pm_generic_resume_early(dev);
+}
+
 static int pci_pm_resume(struct device *dev)
 {
 	struct pci_dev *pci_dev = to_pci_dev(dev);
@@ -961,6 +961,7 @@ static int pci_pm_resume(struct device *dev)
 #define pci_pm_suspend_late	NULL
 #define pci_pm_suspend_noirq	NULL
 #define pci_pm_resume		NULL
+#define pci_pm_resume_early	NULL
 #define pci_pm_resume_noirq	NULL
 
 #endif /* !CONFIG_SUSPEND */
@@ -1358,6 +1359,7 @@ static const struct dev_pm_ops pci_dev_pm_ops = {
 	.suspend = pci_pm_suspend,
 	.suspend_late = pci_pm_suspend_late,
 	.resume = pci_pm_resume,
+	.resume_early = pci_pm_resume_early,
 	.freeze = pci_pm_freeze,
 	.thaw = pci_pm_thaw,
 	.poweroff = pci_pm_poweroff,
-- 
2.16.4





^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH 4/7] PM: sleep: core: Rename dev_pm_may_skip_resume()
  2020-04-10 15:46 [PATCH 0/7] PM: sleep: core: Rearrange the handling of driver power management flags Rafael J. Wysocki
                   ` (2 preceding siblings ...)
  2020-04-10 15:51 ` [PATCH 3/7] PM: sleep: core: Do not skip callbacks in the resume phase Rafael J. Wysocki
@ 2020-04-10 15:51 ` Rafael J. Wysocki
  2020-04-10 15:56 ` [PATCH 5/7] PM: sleep: core: Rename DPM_FLAG_NEVER_SKIP Rafael J. Wysocki
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 31+ messages in thread
From: Rafael J. Wysocki @ 2020-04-10 15:51 UTC (permalink / raw)
  To: Linux PM, Alan Stern
  Cc: Linux ACPI, Linux PCI, LKML, Bjorn Helgaas, Greg Kroah-Hartman,
	Mika Westerberg, Hans De Goede, Ulf Hansson, Linux ACPI,
	Linux PCI, Bjorn Helgaas

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

The name of dev_pm_may_skip_resume() may be easily confused with the
power.may_skip_resume flag which is not checked by that function, so
rename the former as dev_pm_skip_resume().

No functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 Documentation/power/pci.rst | 2 +-
 drivers/acpi/acpi_lpss.c    | 4 ++--
 drivers/acpi/device_pm.c    | 4 ++--
 drivers/base/power/main.c   | 8 ++++----
 drivers/pci/pci-driver.c    | 4 ++--
 include/linux/pm.h          | 2 +-
 6 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/Documentation/power/pci.rst b/Documentation/power/pci.rst
index a39b2461919a..aa1c7fce6cd0 100644
--- a/Documentation/power/pci.rst
+++ b/Documentation/power/pci.rst
@@ -1034,7 +1034,7 @@ device to be left in suspend after system-wide transitions to the working state.
 This flag is checked by the PM core, but the PCI bus type informs the PM core
 which devices may be left in suspend from its perspective (that happens during
 the "noirq" phase of system-wide suspend and analogous transitions) and next it
-uses the dev_pm_may_skip_resume() helper to decide whether or not to return from
+uses the dev_pm_skip_resume() helper to decide whether or not to return from
 pci_pm_resume_noirq() and pci_pm_resume_early() upfront.
 
 3.2. Device Runtime Power Management
diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c
index c4a84df6cc98..7632df1a5be3 100644
--- a/drivers/acpi/acpi_lpss.c
+++ b/drivers/acpi/acpi_lpss.c
@@ -1093,7 +1093,7 @@ static int acpi_lpss_resume_early(struct device *dev)
 	if (pdata->dev_desc->resume_from_noirq)
 		return 0;
 
-	if (dev_pm_may_skip_resume(dev))
+	if (dev_pm_skip_resume(dev))
 		return 0;
 
 	return acpi_lpss_do_resume_early(dev);
@@ -1105,7 +1105,7 @@ static int acpi_lpss_resume_noirq(struct device *dev)
 	int ret;
 
 	/* Follow acpi_subsys_resume_noirq(). */
-	if (dev_pm_may_skip_resume(dev))
+	if (dev_pm_skip_resume(dev))
 		return 0;
 
 	ret = pm_generic_resume_noirq(dev);
diff --git a/drivers/acpi/device_pm.c b/drivers/acpi/device_pm.c
index 399684085f85..b9a3ade17587 100644
--- a/drivers/acpi/device_pm.c
+++ b/drivers/acpi/device_pm.c
@@ -1129,7 +1129,7 @@ EXPORT_SYMBOL_GPL(acpi_subsys_suspend_noirq);
  */
 static int acpi_subsys_resume_noirq(struct device *dev)
 {
-	if (dev_pm_may_skip_resume(dev))
+	if (dev_pm_skip_resume(dev))
 		return 0;
 
 	return pm_generic_resume_noirq(dev);
@@ -1147,7 +1147,7 @@ static int acpi_subsys_resume_early(struct device *dev)
 {
 	int ret;
 
-	if (dev_pm_may_skip_resume(dev))
+	if (dev_pm_skip_resume(dev))
 		return 0;
 
 	ret = acpi_dev_resume(dev);
diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index 66f205d7c7a3..21187ee37b22 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -562,7 +562,7 @@ static void dpm_watchdog_clear(struct dpm_watchdog *wd)
 /*------------------------- Resume routines -------------------------*/
 
 /**
- * dev_pm_may_skip_resume - System-wide device resume optimization check.
+ * dev_pm_skip_resume - System-wide device resume optimization check.
  * @dev: Target device.
  *
  * Driver-level resume callbacks can be skipped for @dev if its configuration is
@@ -574,7 +574,7 @@ static void dpm_watchdog_clear(struct dpm_watchdog *wd)
  * device remains in runtime suspend), so running the "thaw" callbacks for it
  * may be invalid.
  */
-bool dev_pm_may_skip_resume(struct device *dev)
+bool dev_pm_skip_resume(struct device *dev)
 {
 	return (!dev->power.must_resume &&
 		pm_transition.event != PM_EVENT_RESTORE) ||
@@ -610,7 +610,7 @@ static int device_resume_noirq(struct device *dev, pm_message_t state, bool asyn
 	if (!dpm_wait_for_superior(dev, async))
 		goto Out;
 
-	skip_resume = dev_pm_may_skip_resume(dev);
+	skip_resume = dev_pm_skip_resume(dev);
 	/*
 	 * If the driver callback is skipped below or by the middle layer
 	 * callback and device_resume_early() also skips the driver callback for
@@ -796,7 +796,7 @@ static int device_resume_early(struct device *dev, pm_message_t state, bool asyn
 	if (callback)
 		goto Run;
 
-	if (dev_pm_may_skip_resume(dev))
+	if (dev_pm_skip_resume(dev))
 		goto Skip;
 
 	if (dev->driver && dev->driver->pm) {
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index 685fbf044911..ff2fc93e7a3b 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -893,7 +893,7 @@ static int pci_pm_resume_noirq(struct device *dev)
 	pci_power_t prev_state = pci_dev->current_state;
 	bool skip_bus_pm = pci_dev->skip_bus_pm;
 
-	if (dev_pm_may_skip_resume(dev))
+	if (dev_pm_skip_resume(dev))
 		return 0;
 
 	/*
@@ -922,7 +922,7 @@ static int pci_pm_resume_noirq(struct device *dev)
 
 static int pci_pm_resume_early(struct device *dev)
 {
-	if (dev_pm_may_skip_resume(dev))
+	if (dev_pm_skip_resume(dev))
 		return 0;
 
 	return pm_generic_resume_early(dev);
diff --git a/include/linux/pm.h b/include/linux/pm.h
index e057d1fa2469..d89b7099f241 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -758,7 +758,7 @@ extern int pm_generic_poweroff_late(struct device *dev);
 extern int pm_generic_poweroff(struct device *dev);
 extern void pm_generic_complete(struct device *dev);
 
-extern bool dev_pm_may_skip_resume(struct device *dev);
+extern bool dev_pm_skip_resume(struct device *dev);
 extern bool dev_pm_smart_suspend_and_suspended(struct device *dev);
 
 #else /* !CONFIG_PM_SLEEP */
-- 
2.16.4





^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH 5/7] PM: sleep: core: Rename DPM_FLAG_NEVER_SKIP
  2020-04-10 15:46 [PATCH 0/7] PM: sleep: core: Rearrange the handling of driver power management flags Rafael J. Wysocki
                   ` (3 preceding siblings ...)
  2020-04-10 15:51 ` [PATCH 4/7] PM: sleep: core: Rename dev_pm_may_skip_resume() Rafael J. Wysocki
@ 2020-04-10 15:56 ` Rafael J. Wysocki
  2020-04-10 19:38   ` Bjorn Helgaas
  2020-04-13  6:35   ` Jeff Kirsher
  2020-04-10 15:57 ` [PATCH 6/7] PM: sleep: core: Rename DPM_FLAG_LEAVE_SUSPENDED Rafael J. Wysocki
                   ` (3 subsequent siblings)
  8 siblings, 2 replies; 31+ messages in thread
From: Rafael J. Wysocki @ 2020-04-10 15:56 UTC (permalink / raw)
  To: Linux PM, Alan Stern
  Cc: Linux ACPI, Linux PCI, LKML, Bjorn Helgaas, Greg Kroah-Hartman,
	Mika Westerberg, Hans De Goede, Ulf Hansson, Linux PCI,
	Bjorn Helgaas, DRI-devel, Intel Graphics, intel-wired-lan

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

Rename DPM_FLAG_NEVER_SKIP to DPM_FLAG_NO_DIRECT_COMPLETE which
matches its purpose more closely.

No functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 Documentation/driver-api/pm/devices.rst    |  6 +++---
 Documentation/power/pci.rst                | 10 +++++-----
 drivers/base/power/main.c                  |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c    |  2 +-
 drivers/gpu/drm/i915/intel_runtime_pm.c    |  2 +-
 drivers/gpu/drm/radeon/radeon_kms.c        |  2 +-
 drivers/misc/mei/pci-me.c                  |  2 +-
 drivers/misc/mei/pci-txe.c                 |  2 +-
 drivers/net/ethernet/intel/e1000e/netdev.c |  2 +-
 drivers/net/ethernet/intel/igb/igb_main.c  |  2 +-
 drivers/net/ethernet/intel/igc/igc_main.c  |  2 +-
 drivers/pci/pcie/portdrv_pci.c             |  2 +-
 include/linux/pm.h                         |  6 +++---
 13 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/Documentation/driver-api/pm/devices.rst b/Documentation/driver-api/pm/devices.rst
index f66c7b9126ea..4ace0eba4506 100644
--- a/Documentation/driver-api/pm/devices.rst
+++ b/Documentation/driver-api/pm/devices.rst
@@ -361,9 +361,9 @@ the phases are: ``prepare``, ``suspend``, ``suspend_late``, ``suspend_noirq``.
 	runtime PM disabled.
 
 	This feature also can be controlled by device drivers by using the
-	``DPM_FLAG_NEVER_SKIP`` and ``DPM_FLAG_SMART_PREPARE`` driver power
-	management flags.  [Typically, they are set at the time the driver is
-	probed against the device in question by passing them to the
+	``DPM_FLAG_NO_DIRECT_COMPLETE`` and ``DPM_FLAG_SMART_PREPARE`` driver
+	power management flags.  [Typically, they are set at the time the driver
+	is probed against the device in question by passing them to the
 	:c:func:`dev_pm_set_driver_flags` helper function.]  If the first of
 	these flags is set, the PM core will not apply the direct-complete
 	procedure described above to the given device and, consequenty, to any
diff --git a/Documentation/power/pci.rst b/Documentation/power/pci.rst
index aa1c7fce6cd0..9e1408121bea 100644
--- a/Documentation/power/pci.rst
+++ b/Documentation/power/pci.rst
@@ -1004,11 +1004,11 @@ including the PCI bus type.  The flags should be set once at the driver probe
 time with the help of the dev_pm_set_driver_flags() function and they should not
 be updated directly afterwards.
 
-The DPM_FLAG_NEVER_SKIP flag prevents the PM core from using the direct-complete
-mechanism allowing device suspend/resume callbacks to be skipped if the device
-is in runtime suspend when the system suspend starts.  That also affects all of
-the ancestors of the device, so this flag should only be used if absolutely
-necessary.
+The DPM_FLAG_NO_DIRECT_COMPLETE flag prevents the PM core from using the
+direct-complete mechanism allowing device suspend/resume callbacks to be skipped
+if the device is in runtime suspend when the system suspend starts.  That also
+affects all of the ancestors of the device, so this flag should only be used if
+absolutely necessary.
 
 The DPM_FLAG_SMART_PREPARE flag instructs the PCI bus type to only return a
 positive value from pci_pm_prepare() if the ->prepare callback provided by the
diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index 21187ee37b22..aa9c8df9fc4b 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -1850,7 +1850,7 @@ static int device_prepare(struct device *dev, pm_message_t state)
 	spin_lock_irq(&dev->power.lock);
 	dev->power.direct_complete = state.event == PM_EVENT_SUSPEND &&
 		(ret > 0 || dev->power.no_pm_callbacks) &&
-		!dev_pm_test_driver_flags(dev, DPM_FLAG_NEVER_SKIP);
+		!dev_pm_test_driver_flags(dev, DPM_FLAG_NO_DIRECT_COMPLETE);
 	spin_unlock_irq(&dev->power.lock);
 	return 0;
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index fd1dc3236eca..a9086ea1ab60 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -191,7 +191,7 @@ int amdgpu_driver_load_kms(struct drm_device *dev, unsigned long flags)
 	}
 
 	if (adev->runpm) {
-		dev_pm_set_driver_flags(dev->dev, DPM_FLAG_NEVER_SKIP);
+		dev_pm_set_driver_flags(dev->dev, DPM_FLAG_NO_DIRECT_COMPLETE);
 		pm_runtime_use_autosuspend(dev->dev);
 		pm_runtime_set_autosuspend_delay(dev->dev, 5000);
 		pm_runtime_set_active(dev->dev);
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index ad719c9602af..9cb2d7548daa 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -549,7 +549,7 @@ void intel_runtime_pm_enable(struct intel_runtime_pm *rpm)
 	 * becaue the HDA driver may require us to enable the audio power
 	 * domain during system suspend.
 	 */
-	dev_pm_set_driver_flags(kdev, DPM_FLAG_NEVER_SKIP);
+	dev_pm_set_driver_flags(kdev, DPM_FLAG_NO_DIRECT_COMPLETE);
 
 	pm_runtime_set_autosuspend_delay(kdev, 10000); /* 10s */
 	pm_runtime_mark_last_busy(kdev);
diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c
index 58176db85952..372962358a18 100644
--- a/drivers/gpu/drm/radeon/radeon_kms.c
+++ b/drivers/gpu/drm/radeon/radeon_kms.c
@@ -158,7 +158,7 @@ int radeon_driver_load_kms(struct drm_device *dev, unsigned long flags)
 	}
 
 	if (radeon_is_px(dev)) {
-		dev_pm_set_driver_flags(dev->dev, DPM_FLAG_NEVER_SKIP);
+		dev_pm_set_driver_flags(dev->dev, DPM_FLAG_NO_DIRECT_COMPLETE);
 		pm_runtime_use_autosuspend(dev->dev);
 		pm_runtime_set_autosuspend_delay(dev->dev, 5000);
 		pm_runtime_set_active(dev->dev);
diff --git a/drivers/misc/mei/pci-me.c b/drivers/misc/mei/pci-me.c
index 3d21c38e2dbb..53f16f3bd091 100644
--- a/drivers/misc/mei/pci-me.c
+++ b/drivers/misc/mei/pci-me.c
@@ -240,7 +240,7 @@ static int mei_me_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	 * MEI requires to resume from runtime suspend mode
 	 * in order to perform link reset flow upon system suspend.
 	 */
-	dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_NEVER_SKIP);
+	dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_NO_DIRECT_COMPLETE);
 
 	/*
 	 * ME maps runtime suspend/resume to D0i states,
diff --git a/drivers/misc/mei/pci-txe.c b/drivers/misc/mei/pci-txe.c
index beacf2a2f2b5..4bf26ce61044 100644
--- a/drivers/misc/mei/pci-txe.c
+++ b/drivers/misc/mei/pci-txe.c
@@ -128,7 +128,7 @@ static int mei_txe_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	 * MEI requires to resume from runtime suspend mode
 	 * in order to perform link reset flow upon system suspend.
 	 */
-	dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_NEVER_SKIP);
+	dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_NO_DIRECT_COMPLETE);
 
 	/*
 	 * TXE maps runtime suspend/resume to own power gating states,
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index 177c6da80c57..2730b1c7dddb 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -7549,7 +7549,7 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	e1000_print_device_info(adapter);
 
-	dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_NEVER_SKIP);
+	dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_NO_DIRECT_COMPLETE);
 
 	if (pci_dev_run_wake(pdev) && hw->mac.type < e1000_pch_cnp)
 		pm_runtime_put_noidle(&pdev->dev);
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index b46bff8fe056..8bb3db2cbd41 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -3445,7 +3445,7 @@ static int igb_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		}
 	}
 
-	dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_NEVER_SKIP);
+	dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_NO_DIRECT_COMPLETE);
 
 	pm_runtime_put_noidle(&pdev->dev);
 	return 0;
diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index 69fa1ce1f927..59fc0097438f 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -4825,7 +4825,7 @@ static int igc_probe(struct pci_dev *pdev,
 	pcie_print_link_status(pdev);
 	netdev_info(netdev, "MAC: %pM\n", netdev->dev_addr);
 
-	dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_NEVER_SKIP);
+	dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_NO_DIRECT_COMPLETE);
 
 	pm_runtime_put_noidle(&pdev->dev);
 
diff --git a/drivers/pci/pcie/portdrv_pci.c b/drivers/pci/pcie/portdrv_pci.c
index 160d67c59310..3acf151ae015 100644
--- a/drivers/pci/pcie/portdrv_pci.c
+++ b/drivers/pci/pcie/portdrv_pci.c
@@ -115,7 +115,7 @@ static int pcie_portdrv_probe(struct pci_dev *dev,
 
 	pci_save_state(dev);
 
-	dev_pm_set_driver_flags(&dev->dev, DPM_FLAG_NEVER_SKIP |
+	dev_pm_set_driver_flags(&dev->dev, DPM_FLAG_NO_DIRECT_COMPLETE |
 					   DPM_FLAG_SMART_SUSPEND);
 
 	if (pci_bridge_d3_possible(dev)) {
diff --git a/include/linux/pm.h b/include/linux/pm.h
index d89b7099f241..28fd444fb5c9 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -544,7 +544,7 @@ struct pm_subsys_data {
  * These flags can be set by device drivers at the probe time.  They need not be
  * cleared by the drivers as the driver core will take care of that.
  *
- * NEVER_SKIP: Do not skip all system suspend/resume callbacks for the device.
+ * NO_DIRECT_COMPLETE: Do not apply direct-complete optimization to the device.
  * SMART_PREPARE: Check the return value of the driver's ->prepare callback.
  * SMART_SUSPEND: No need to resume the device from runtime suspend.
  * LEAVE_SUSPENDED: Avoid resuming the device during system resume if possible.
@@ -554,7 +554,7 @@ struct pm_subsys_data {
  * their ->prepare callbacks if the driver's ->prepare callback returns 0 (in
  * other words, the system suspend/resume callbacks can only be skipped for the
  * device if its driver doesn't object against that).  This flag has no effect
- * if NEVER_SKIP is set.
+ * if NO_DIRECT_COMPLETE is set.
  *
  * Setting SMART_SUSPEND instructs bus types and PM domains which may want to
  * runtime resume the device upfront during system suspend that doing so is not
@@ -565,7 +565,7 @@ struct pm_subsys_data {
  * Setting LEAVE_SUSPENDED informs the PM core and middle-layer code that the
  * driver prefers the device to be left in suspend after system resume.
  */
-#define DPM_FLAG_NEVER_SKIP		BIT(0)
+#define DPM_FLAG_NO_DIRECT_COMPLETE	BIT(0)
 #define DPM_FLAG_SMART_PREPARE		BIT(1)
 #define DPM_FLAG_SMART_SUSPEND		BIT(2)
 #define DPM_FLAG_LEAVE_SUSPENDED	BIT(3)
-- 
2.16.4





^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH 6/7] PM: sleep: core: Rename DPM_FLAG_LEAVE_SUSPENDED
  2020-04-10 15:46 [PATCH 0/7] PM: sleep: core: Rearrange the handling of driver power management flags Rafael J. Wysocki
                   ` (4 preceding siblings ...)
  2020-04-10 15:56 ` [PATCH 5/7] PM: sleep: core: Rename DPM_FLAG_NEVER_SKIP Rafael J. Wysocki
@ 2020-04-10 15:57 ` Rafael J. Wysocki
  2020-04-15  9:51   ` Wolfram Sang
  2020-04-10 15:58 ` [PATCH 7/7] Documentation: PM: sleep: Update driver flags documentation Rafael J. Wysocki
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 31+ messages in thread
From: Rafael J. Wysocki @ 2020-04-10 15:57 UTC (permalink / raw)
  To: Linux PM, Alan Stern
  Cc: Linux ACPI, Linux PCI, LKML, Bjorn Helgaas, Greg Kroah-Hartman,
	Mika Westerberg, Hans De Goede, Ulf Hansson, Linux ACPI,
	Linux I2C, Linux PCI

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

Rename DPM_FLAG_LEAVE_SUSPENDED to DPM_FLAG_MAY_SKIP_RESUME which
matches its purpose more closely.

No functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 Documentation/driver-api/pm/devices.rst     | 4 ++--
 Documentation/power/pci.rst                 | 2 +-
 drivers/acpi/acpi_tad.c                     | 2 +-
 drivers/base/power/main.c                   | 2 +-
 drivers/i2c/busses/i2c-designware-platdrv.c | 2 +-
 include/linux/pm.h                          | 6 +++---
 6 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/Documentation/driver-api/pm/devices.rst b/Documentation/driver-api/pm/devices.rst
index 4ace0eba4506..f342c7549b4c 100644
--- a/Documentation/driver-api/pm/devices.rst
+++ b/Documentation/driver-api/pm/devices.rst
@@ -803,7 +803,7 @@ general.]
 However, it often is desirable to leave devices in suspend after system
 transitions to the working state, especially if those devices had been in
 runtime suspend before the preceding system-wide suspend (or analogous)
-transition.  Device drivers can use the ``DPM_FLAG_LEAVE_SUSPENDED`` flag to
+transition.  Device drivers can use the ``DPM_FLAG_MAY_SKIP_RESUME`` flag to
 indicate to the PM core (and middle-layer code) that they prefer the specific
 devices handled by them to be left suspended and they have no problems with
 skipping their system-wide resume callbacks for this reason.  Whether or not the
@@ -825,7 +825,7 @@ device really can be left in suspend.
 
 For devices whose "noirq", "late" and "early" driver callbacks are invoked
 directly by the PM core, all of the system-wide resume callbacks are skipped if
-``DPM_FLAG_LEAVE_SUSPENDED`` is set and the device is in runtime suspend during
+``DPM_FLAG_MAY_SKIP_RESUME`` is set and the device is in runtime suspend during
 the ``suspend_noirq`` (or analogous) phase or the transition under way is a
 proper system suspend (rather than anything related to hibernation) and the
 device's wakeup settings are suitable for runtime PM (that is, it cannot
diff --git a/Documentation/power/pci.rst b/Documentation/power/pci.rst
index 9e1408121bea..f09b382b4621 100644
--- a/Documentation/power/pci.rst
+++ b/Documentation/power/pci.rst
@@ -1029,7 +1029,7 @@ into D0 going forward), but if it is in runtime suspend in pci_pm_thaw_noirq(),
 the function will set the power.direct_complete flag for it (to make the PM core
 skip the subsequent "thaw" callbacks for it) and return.
 
-Setting the DPM_FLAG_LEAVE_SUSPENDED flag means that the driver prefers the
+Setting the DPM_FLAG_MAY_SKIP_RESUME flag means that the driver prefers the
 device to be left in suspend after system-wide transitions to the working state.
 This flag is checked by the PM core, but the PCI bus type informs the PM core
 which devices may be left in suspend from its perspective (that happens during
diff --git a/drivers/acpi/acpi_tad.c b/drivers/acpi/acpi_tad.c
index 33a4bcdaa4d7..7d45cce0c3c1 100644
--- a/drivers/acpi/acpi_tad.c
+++ b/drivers/acpi/acpi_tad.c
@@ -624,7 +624,7 @@ static int acpi_tad_probe(struct platform_device *pdev)
 	 */
 	device_init_wakeup(dev, true);
 	dev_pm_set_driver_flags(dev, DPM_FLAG_SMART_SUSPEND |
-				     DPM_FLAG_LEAVE_SUSPENDED);
+				     DPM_FLAG_MAY_SKIP_RESUME);
 	/*
 	 * The platform bus type layer tells the ACPI PM domain powers up the
 	 * device, so set the runtime PM status of it to "active".
diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index aa9c8df9fc4b..b6f785024b24 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -1247,7 +1247,7 @@ static int __device_suspend_noirq(struct device *dev, pm_message_t state, bool a
 	 * to be skipped.
 	 */
 	if (atomic_read(&dev->power.usage_count) > 1 ||
-	    !(dev_pm_test_driver_flags(dev, DPM_FLAG_LEAVE_SUSPENDED) &&
+	    !(dev_pm_test_driver_flags(dev, DPM_FLAG_MAY_SKIP_RESUME) &&
 	      dev->power.may_skip_resume))
 		dev->power.must_resume = true;
 
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index c98befe2a92e..b6270e69f853 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -357,7 +357,7 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
 	dev_pm_set_driver_flags(&pdev->dev,
 				DPM_FLAG_SMART_PREPARE |
 				DPM_FLAG_SMART_SUSPEND |
-				DPM_FLAG_LEAVE_SUSPENDED);
+				DPM_FLAG_MAY_SKIP_RESUME);
 
 	/* The code below assumes runtime PM to be disabled. */
 	WARN_ON(pm_runtime_enabled(&pdev->dev));
diff --git a/include/linux/pm.h b/include/linux/pm.h
index 28fd444fb5c9..f545666120d0 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -547,7 +547,7 @@ struct pm_subsys_data {
  * NO_DIRECT_COMPLETE: Do not apply direct-complete optimization to the device.
  * SMART_PREPARE: Check the return value of the driver's ->prepare callback.
  * SMART_SUSPEND: No need to resume the device from runtime suspend.
- * LEAVE_SUSPENDED: Avoid resuming the device during system resume if possible.
+ * MAY_SKIP_RESUME: Avoid resuming the device during system resume if possible.
  *
  * Setting SMART_PREPARE instructs bus types and PM domains which may want
  * system suspend/resume callbacks to be skipped for the device to return 0 from
@@ -562,13 +562,13 @@ struct pm_subsys_data {
  * invocations of the ->suspend_late and ->suspend_noirq callbacks provided by
  * the driver if they decide to leave the device in runtime suspend.
  *
- * Setting LEAVE_SUSPENDED informs the PM core and middle-layer code that the
+ * Setting MAY_SKIP_RESUME informs the PM core and middle-layer code that the
  * driver prefers the device to be left in suspend after system resume.
  */
 #define DPM_FLAG_NO_DIRECT_COMPLETE	BIT(0)
 #define DPM_FLAG_SMART_PREPARE		BIT(1)
 #define DPM_FLAG_SMART_SUSPEND		BIT(2)
-#define DPM_FLAG_LEAVE_SUSPENDED	BIT(3)
+#define DPM_FLAG_MAY_SKIP_RESUME	BIT(3)
 
 struct dev_pm_info {
 	pm_message_t		power_state;
-- 
2.16.4





^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH 7/7] Documentation: PM: sleep: Update driver flags documentation
  2020-04-10 15:46 [PATCH 0/7] PM: sleep: core: Rearrange the handling of driver power management flags Rafael J. Wysocki
                   ` (5 preceding siblings ...)
  2020-04-10 15:57 ` [PATCH 6/7] PM: sleep: core: Rename DPM_FLAG_LEAVE_SUSPENDED Rafael J. Wysocki
@ 2020-04-10 15:58 ` Rafael J. Wysocki
  2020-04-13 13:11 ` [PATCH 0/7] PM: sleep: core: Rearrange the handling of driver power management flags Hans de Goede
  2020-04-18 16:23 ` [PATCH v2 0/9] " Rafael J. Wysocki
  8 siblings, 0 replies; 31+ messages in thread
From: Rafael J. Wysocki @ 2020-04-10 15:58 UTC (permalink / raw)
  To: Linux PM, Alan Stern
  Cc: Linux ACPI, Linux PCI, LKML, Bjorn Helgaas, Greg Kroah-Hartman,
	Mika Westerberg, Hans De Goede, Ulf Hansson, Bjorn Helgaas,
	Linux PCI, Linux Documentation

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

Update the documentation of the driver flags for system-wide power
management to reflect the current code flows and be more consistent.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 Documentation/driver-api/pm/devices.rst | 118 +++++++++++++++++++-------------
 Documentation/power/pci.rst             |  43 ++++++------
 include/linux/pm.h                      |  24 ++-----
 3 files changed, 98 insertions(+), 87 deletions(-)

diff --git a/Documentation/driver-api/pm/devices.rst b/Documentation/driver-api/pm/devices.rst
index f342c7549b4c..39360c2cc54d 100644
--- a/Documentation/driver-api/pm/devices.rst
+++ b/Documentation/driver-api/pm/devices.rst
@@ -776,58 +776,82 @@ Some bus types and PM domains have a policy to resume all devices from runtime
 suspend upfront in their ``->suspend`` callbacks, but that may not be really
 necessary if the driver of the device can cope with runtime-suspended devices.
 The driver can indicate that by setting ``DPM_FLAG_SMART_SUSPEND`` in
-:c:member:`power.driver_flags` at the probe time, by passing it to the
-:c:func:`dev_pm_set_driver_flags` helper.  That also may cause middle-layer code
+:c:member:`power.driver_flags` at the probe time with the help of the
+:c:func:`dev_pm_set_driver_flags` helper routine.
+
+However, setting that flag also causes the PM core and middle-layer code
 (bus types, PM domains etc.) to skip the ``->suspend_late`` and
 ``->suspend_noirq`` callbacks provided by the driver if the device remains in
-runtime suspend at the beginning of the ``suspend_late`` phase of system-wide
-suspend (or in the ``poweroff_late`` phase of hibernation), when runtime PM
-has been disabled for it, under the assumption that its state should not change
-after that point until the system-wide transition is over (the PM core itself
-does that for devices whose "noirq", "late" and "early" system-wide PM callbacks
-are executed directly by it).  If that happens, the driver's system-wide resume
-callbacks, if present, may still be invoked during the subsequent system-wide
-resume transition and the device's runtime power management status may be set
-to "active" before enabling runtime PM for it, so the driver must be prepared to
-cope with the invocation of its system-wide resume callbacks back-to-back with
-its ``->runtime_suspend`` one (without the intervening ``->runtime_resume`` and
-so on) and the final state of the device must reflect the "active" runtime PM
-status in that case.
+runtime suspend during the ``suspend_late`` phase of system-wide suspend (or
+during the ``poweroff_late`` or ``freeze_late`` phase of hibernation),
+after runtime PM was disabled for it.  [Without doing that, the same driver
+callback might be executed twice in a row for the same device, which would not
+be valid in general.]  If the middle-layer system-wide PM callbacks are present
+for the device, they are responsible for doing the above, and if they are not
+present, the PM core takes care of it.
+
+In addition, with ``DPM_FLAG_SMART_SUSPEND`` set, the driver's ``->thaw_late``
+and ``->thaw_noirq`` callbacks are skipped if the device remained in runtime
+suspend during the preceding "freeze" transition related to hibernation.
+[Again, if the middle-layer callbacks are present for the device, they are
+responsible for doing that, or the PM core takes care of it otherwise.]
 
 During system-wide resume from a sleep state it's easiest to put devices into
 the full-power state, as explained in :file:`Documentation/power/runtime_pm.rst`.
 [Refer to that document for more information regarding this particular issue as
 well as for information on the device runtime power management framework in
-general.]
-
-However, it often is desirable to leave devices in suspend after system
-transitions to the working state, especially if those devices had been in
+general.]  However, it often is desirable to leave devices in suspend after
+system transitions to the working state, especially if those devices had been in
 runtime suspend before the preceding system-wide suspend (or analogous)
-transition.  Device drivers can use the ``DPM_FLAG_MAY_SKIP_RESUME`` flag to
-indicate to the PM core (and middle-layer code) that they prefer the specific
-devices handled by them to be left suspended and they have no problems with
-skipping their system-wide resume callbacks for this reason.  Whether or not the
-devices will actually be left in suspend may depend on their state before the
-given system suspend-resume cycle and on the type of the system transition under
-way.  In particular, devices are not left suspended if that transition is a
-restore from hibernation, as device states are not guaranteed to be reflected
-by the information stored in the hibernation image in that case.
-
-The middle-layer code involved in the handling of the device is expected to
-indicate to the PM core if the device may be left in suspend by setting its
-:c:member:`power.may_skip_resume` status bit which is checked by the PM core
-during the "noirq" phase of the preceding system-wide suspend (or analogous)
-transition.  The middle layer is then responsible for handling the device as
-appropriate in its "noirq" resume callback, which is executed regardless of
-whether or not the device is left suspended, but the other resume callbacks
-(except for ``->complete``) will be skipped automatically by the PM core if the
-device really can be left in suspend.
-
-For devices whose "noirq", "late" and "early" driver callbacks are invoked
-directly by the PM core, all of the system-wide resume callbacks are skipped if
-``DPM_FLAG_MAY_SKIP_RESUME`` is set and the device is in runtime suspend during
-the ``suspend_noirq`` (or analogous) phase or the transition under way is a
-proper system suspend (rather than anything related to hibernation) and the
-device's wakeup settings are suitable for runtime PM (that is, it cannot
-generate wakeup signals at all or it is allowed to wake up the system from
-sleep).
+transition.
+
+To that end, device drivers can use the ``DPM_FLAG_MAY_SKIP_RESUME`` flag to
+indicate to the PM core and middle-layer code that they allow their "noirq" and
+"early" resume callbacks to be skipped if the device can be left in suspend
+after system-wide PM transitions to the working state.  Whether or not that is
+the case generally depend on the state of the device before the given system
+suspend-resume cycle and on the type of the system transition under way.
+In particular, driver callbacks are never skipped during the "restore"
+transition related to hibernation (because the actual states of devices at that
+time are not known in general, all of them are put into the full-power state
+then and configured to reflect their "active" working-state settings).
+
+The middle-layer code is expected to indicate to the PM core whether or not
+skipping the driver's "noirq" and "early" resume callbacks for a given device
+makes sense from its perspective by either setting or clearing the device's
+:c:member:`power.may_skip_resume` status bit (checked by the PM core during the
+"noirq" phase of the suspend, or analogous, transition).  Setting that status
+bit along with the ``DPM_FLAG_MAY_SKIP_RESUME`` flag is necessary, but
+generally not sufficient, for the driver's "noirq" and "early" callbacks to be
+skipped.  Whether or not they should be skipped can be determined by evaluating
+the :c:func:`dev_pm_skip_resume` helper function.
+
+If that function returns ``true``, the driver's "noirq" and "early" resume
+callbacks should be skipped.  Otherwise, if the device was runtime-suspended
+during the preceding system-wide suspend transition, its runtime PM status needs
+to be set to "active" which is taken care of by the PM core.
+
+If the ``DPM_FLAG_MAY_SKIP_RESUME`` flag is not set for a device, but
+``DPM_FLAG_SMART_SUSPEND`` is set and the driver's "late" and "noirq" suspend
+callbacks are skipped, its system-wide "noirq" and "early" resume callbacks, if
+present, are invoked as usual and the device's runtime PM status is set to
+"active" by the PM core before enabling runtime PM for it, so the driver must be
+prepared to cope with the invocation of its system-wide resume callbacks
+back-to-back with its ``->runtime_suspend`` one (without the intervening
+``->runtime_resume`` and system-wide suspend callbacks) and the final state of
+the device must reflect the "active" runtime PM status in that case.  [Note that
+this is not a problem at all if the driver's ``->suspend_late`` callback pointer
+points to the same function as its ``->runtime_suspend`` one and its
+``->resume_early`` callback pointer points to the same function as the
+``->runtime_resume`` one, while none of the other system-wide suspend-resume
+callbacks of the driver are present, for example.]
+
+Likewise, if ``DPM_FLAG_MAY_SKIP_RESUME`` is set for a device, its driver's
+system-wide "noirq" and "early" resume callbacks may be skipped while its "late"
+and "noirq" suspend callbacks may have been executed (in principle, regardless
+of whether or not ``DPM_FLAG_SMART_SUSPEND`` is set).  In that case, the driver
+needs to be able to cope with the invocation of its ``->runtime_resume``
+callback back-to-back with its "late" and "noirq" suspend ones.  [Again, that
+is not a concern if the driver sets both ``DPM_FLAG_SMART_SUSPEND`` and
+``DPM_FLAG_MAY_SKIP_RESUME`` and uses the same pair of suspend/resume callback
+functions for runtime PM and system-wide suspend/resume.]
diff --git a/Documentation/power/pci.rst b/Documentation/power/pci.rst
index f09b382b4621..1831e431f725 100644
--- a/Documentation/power/pci.rst
+++ b/Documentation/power/pci.rst
@@ -1010,32 +1010,33 @@ if the device is in runtime suspend when the system suspend starts.  That also
 affects all of the ancestors of the device, so this flag should only be used if
 absolutely necessary.
 
-The DPM_FLAG_SMART_PREPARE flag instructs the PCI bus type to only return a
-positive value from pci_pm_prepare() if the ->prepare callback provided by the
+The DPM_FLAG_SMART_PREPARE flag causes the PCI bus type to return a positive
+value from pci_pm_prepare() only if the ->prepare callback provided by the
 driver of the device returns a positive value.  That allows the driver to opt
-out from using the direct-complete mechanism dynamically.
+out from using the direct-complete mechanism dynamically (whereas setting
+DPM_FLAG_NO_DIRECT_COMPLETE means permanent opt-out).
 
 The DPM_FLAG_SMART_SUSPEND flag tells the PCI bus type that from the driver's
 perspective the device can be safely left in runtime suspend during system
 suspend.  That causes pci_pm_suspend(), pci_pm_freeze() and pci_pm_poweroff()
-to skip resuming the device from runtime suspend unless there are PCI-specific
-reasons for doing that.  Also, it causes pci_pm_suspend_late/noirq(),
-pci_pm_freeze_late/noirq() and pci_pm_poweroff_late/noirq() to return early
-if the device remains in runtime suspend in the beginning of the "late" phase
-of the system-wide transition under way.  Moreover, if the device is in
-runtime suspend in pci_pm_resume_noirq() or pci_pm_restore_noirq(), its runtime
-power management status will be changed to "active" (as it is going to be put
-into D0 going forward), but if it is in runtime suspend in pci_pm_thaw_noirq(),
-the function will set the power.direct_complete flag for it (to make the PM core
-skip the subsequent "thaw" callbacks for it) and return.
-
-Setting the DPM_FLAG_MAY_SKIP_RESUME flag means that the driver prefers the
-device to be left in suspend after system-wide transitions to the working state.
-This flag is checked by the PM core, but the PCI bus type informs the PM core
-which devices may be left in suspend from its perspective (that happens during
-the "noirq" phase of system-wide suspend and analogous transitions) and next it
-uses the dev_pm_skip_resume() helper to decide whether or not to return from
-pci_pm_resume_noirq() and pci_pm_resume_early() upfront.
+to avoid resuming the device from runtime suspend unless there are PCI-specific
+reasons for doing that.  Also, it causes pci_pm_suspend_late/noirq() and
+pci_pm_poweroff_late/noirq() to return early if the device remains in runtime
+suspend during the "late" phase of the system-wide transition under way.
+Moreover, if the device is in runtime suspend in pci_pm_resume_noirq() or
+pci_pm_restore_noirq(), its runtime PM status will be changed to "active" (as it
+is going to be put into D0 going forward).
+
+Setting the DPM_FLAG_MAY_SKIP_RESUME flag means that the driver allows its
+"noirq" and "early" resume callbacks to be skipped if the device can be left
+in suspend after a system-wide transition into the working state.  This flag is
+taken into consideration by the PM core along with the power.may_skip_resume
+status bit of the device which is set by pci_pm_suspend_noirq() in certain
+situations.  If the PM core determines that the driver's "noirq" and "early"
+resume callbacks should be skipped, the dev_pm_skip_resume() helper function
+will return "true" and that will cause pci_pm_resume_noirq() and
+pci_pm_resume_early() to return upfront without touching the device and
+executing the driver callbacks.
 
 3.2. Device Runtime Power Management
 ------------------------------------
diff --git a/include/linux/pm.h b/include/linux/pm.h
index f545666120d0..9542bbc5f938 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -545,25 +545,11 @@ struct pm_subsys_data {
  * cleared by the drivers as the driver core will take care of that.
  *
  * NO_DIRECT_COMPLETE: Do not apply direct-complete optimization to the device.
- * SMART_PREPARE: Check the return value of the driver's ->prepare callback.
- * SMART_SUSPEND: No need to resume the device from runtime suspend.
- * MAY_SKIP_RESUME: Avoid resuming the device during system resume if possible.
- *
- * Setting SMART_PREPARE instructs bus types and PM domains which may want
- * system suspend/resume callbacks to be skipped for the device to return 0 from
- * their ->prepare callbacks if the driver's ->prepare callback returns 0 (in
- * other words, the system suspend/resume callbacks can only be skipped for the
- * device if its driver doesn't object against that).  This flag has no effect
- * if NO_DIRECT_COMPLETE is set.
- *
- * Setting SMART_SUSPEND instructs bus types and PM domains which may want to
- * runtime resume the device upfront during system suspend that doing so is not
- * necessary from the driver's perspective.  It also may cause them to skip
- * invocations of the ->suspend_late and ->suspend_noirq callbacks provided by
- * the driver if they decide to leave the device in runtime suspend.
- *
- * Setting MAY_SKIP_RESUME informs the PM core and middle-layer code that the
- * driver prefers the device to be left in suspend after system resume.
+ * SMART_PREPARE: Take the driver ->prepare callback return value into account.
+ * SMART_SUSPEND: Avoid resuming the device from runtime suspend.
+ * MAY_SKIP_RESUME: Allow driver "noirq" and "early" callbacks to be skipped.
+ *
+ * See Documentation/driver-api/pm/devices.rst for details.
  */
 #define DPM_FLAG_NO_DIRECT_COMPLETE	BIT(0)
 #define DPM_FLAG_SMART_PREPARE		BIT(1)
-- 
2.16.4





^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 5/7] PM: sleep: core: Rename DPM_FLAG_NEVER_SKIP
  2020-04-10 15:56 ` [PATCH 5/7] PM: sleep: core: Rename DPM_FLAG_NEVER_SKIP Rafael J. Wysocki
@ 2020-04-10 19:38   ` Bjorn Helgaas
  2020-04-13  6:35   ` Jeff Kirsher
  1 sibling, 0 replies; 31+ messages in thread
From: Bjorn Helgaas @ 2020-04-10 19:38 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Linux PM, Alan Stern, Linux ACPI, Linux PCI, LKML,
	Greg Kroah-Hartman, Mika Westerberg, Hans De Goede, Ulf Hansson,
	DRI-devel, Intel Graphics, intel-wired-lan

On Fri, Apr 10, 2020 at 05:56:13PM +0200, Rafael J. Wysocki wrote:
> From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> 
> Rename DPM_FLAG_NEVER_SKIP to DPM_FLAG_NO_DIRECT_COMPLETE which
> matches its purpose more closely.
> 
> No functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Acked-by: Bjorn Helgaas <bhelgaas@google.com> # for PCI parts

> ---
>  Documentation/driver-api/pm/devices.rst    |  6 +++---
>  Documentation/power/pci.rst                | 10 +++++-----
>  drivers/base/power/main.c                  |  2 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c    |  2 +-
>  drivers/gpu/drm/i915/intel_runtime_pm.c    |  2 +-
>  drivers/gpu/drm/radeon/radeon_kms.c        |  2 +-
>  drivers/misc/mei/pci-me.c                  |  2 +-
>  drivers/misc/mei/pci-txe.c                 |  2 +-
>  drivers/net/ethernet/intel/e1000e/netdev.c |  2 +-
>  drivers/net/ethernet/intel/igb/igb_main.c  |  2 +-
>  drivers/net/ethernet/intel/igc/igc_main.c  |  2 +-
>  drivers/pci/pcie/portdrv_pci.c             |  2 +-
>  include/linux/pm.h                         |  6 +++---
>  13 files changed, 21 insertions(+), 21 deletions(-)
> 
> diff --git a/Documentation/driver-api/pm/devices.rst b/Documentation/driver-api/pm/devices.rst
> index f66c7b9126ea..4ace0eba4506 100644
> --- a/Documentation/driver-api/pm/devices.rst
> +++ b/Documentation/driver-api/pm/devices.rst
> @@ -361,9 +361,9 @@ the phases are: ``prepare``, ``suspend``, ``suspend_late``, ``suspend_noirq``.
>  	runtime PM disabled.

Minor question about a preceding paragraph that ends:

  In that case, the ``->complete`` callback will be invoked directly
  after the ``->prepare`` callback and is entirely responsible for
  putting the device into a consistent state as appropriate.

What does" a consistent state as appropriate" mean?  I know this is
generic documentation at a high level, so maybe there's no good
explanation for "consistent state," but I don't know what to imagine
there.

And what does "as appropriate" mean?  Would it change the meaning to
drop those two words, or are there situations where it's not
appropriate to put the device into a consistent state?  Or maybe it's
just that the type of device determines what the consistent state is?

>  	This feature also can be controlled by device drivers by using the
> -	``DPM_FLAG_NEVER_SKIP`` and ``DPM_FLAG_SMART_PREPARE`` driver power
> -	management flags.  [Typically, they are set at the time the driver is
> -	probed against the device in question by passing them to the
> +	``DPM_FLAG_NO_DIRECT_COMPLETE`` and ``DPM_FLAG_SMART_PREPARE`` driver
> +	power management flags.  [Typically, they are set at the time the driver
> +	is probed against the device in question by passing them to the
>  	:c:func:`dev_pm_set_driver_flags` helper function.]  If the first of
>  	these flags is set, the PM core will not apply the direct-complete
>  	procedure described above to the given device and, consequenty, to any

s/consequenty/consequently/

Drive-by comment: I looked for a definition of "direct-complete".  The
closest I found is a couple paragraphs above this, where it says "Note
that this direct-complete procedure ...," but that leaves me to try to
reconstruct the definition from the preceding text.

AFAICT, going to freeze, standby, or memory sleep includes these
callbacks:

  ->prepare
  ->suspend
  ->suspend_late
  ->suspend_noirq
  ->complete         (not mentioned in the list of phases)

And "direct-complete" means we skip the suspend, suspend_late,
and suspend_noirq callbacks so we only use these:

  ->prepare
  ->complete

And apparently we skip those callbacks for device X if ->prepare() for
X and all its descendents returns a positive value AND they are all
runtime-suspended, except if a driver for X or a descendent sets
DPM_FLAG_NO_DIRECT_COMPLETE.

Bjorn

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 5/7] PM: sleep: core: Rename DPM_FLAG_NEVER_SKIP
  2020-04-10 15:56 ` [PATCH 5/7] PM: sleep: core: Rename DPM_FLAG_NEVER_SKIP Rafael J. Wysocki
  2020-04-10 19:38   ` Bjorn Helgaas
@ 2020-04-13  6:35   ` Jeff Kirsher
  1 sibling, 0 replies; 31+ messages in thread
From: Jeff Kirsher @ 2020-04-13  6:35 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Linux PM, Alan Stern, Linux ACPI, Linux PCI, LKML, Bjorn Helgaas,
	Greg Kroah-Hartman, Mika Westerberg, Hans De Goede, Ulf Hansson,
	DRI-devel, Intel Graphics, intel-wired-lan

On Fri, Apr 10, 2020 at 9:03 AM Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>
> From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
>
> Rename DPM_FLAG_NEVER_SKIP to DPM_FLAG_NO_DIRECT_COMPLETE which
> matches its purpose more closely.
>
> No functional impact.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

For the driver changes to e1000e, igb and igc.

> ---
>  Documentation/driver-api/pm/devices.rst    |  6 +++---
>  Documentation/power/pci.rst                | 10 +++++-----
>  drivers/base/power/main.c                  |  2 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c    |  2 +-
>  drivers/gpu/drm/i915/intel_runtime_pm.c    |  2 +-
>  drivers/gpu/drm/radeon/radeon_kms.c        |  2 +-
>  drivers/misc/mei/pci-me.c                  |  2 +-
>  drivers/misc/mei/pci-txe.c                 |  2 +-
>  drivers/net/ethernet/intel/e1000e/netdev.c |  2 +-
>  drivers/net/ethernet/intel/igb/igb_main.c  |  2 +-
>  drivers/net/ethernet/intel/igc/igc_main.c  |  2 +-
>  drivers/pci/pcie/portdrv_pci.c             |  2 +-
>  include/linux/pm.h                         |  6 +++---
>  13 files changed, 21 insertions(+), 21 deletions(-)
>

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 0/7] PM: sleep: core: Rearrange the handling of driver power management flags
  2020-04-10 15:46 [PATCH 0/7] PM: sleep: core: Rearrange the handling of driver power management flags Rafael J. Wysocki
                   ` (6 preceding siblings ...)
  2020-04-10 15:58 ` [PATCH 7/7] Documentation: PM: sleep: Update driver flags documentation Rafael J. Wysocki
@ 2020-04-13 13:11 ` Hans de Goede
  2020-04-18 16:23 ` [PATCH v2 0/9] " Rafael J. Wysocki
  8 siblings, 0 replies; 31+ messages in thread
From: Hans de Goede @ 2020-04-13 13:11 UTC (permalink / raw)
  To: Rafael J. Wysocki, Linux PM, Alan Stern
  Cc: Linux ACPI, Linux PCI, LKML, Bjorn Helgaas, Greg Kroah-Hartman,
	Mika Westerberg, Ulf Hansson

Hi,

On 4/10/20 5:46 PM, Rafael J. Wysocki wrote:
> Hi Alan,
> 
> Following our recent discussion regarding the DPM_FLAG_* family of flags [1],
> I have decided to follow some of your recommendations and make changes to the
> core code handling those flags.
> 
> The purpose of this is basically to make the code more consistent internally,
> easier to follow and better documented.
> 
> First of all, patch [1/7] changes the PM core to skip driver-level "late"
> and "noirq" suspend callbacks for devices with SMART_SUSPEND set if they are
> still runtime-suspended during the "late" system-wide suspend phase (without
> the patch it does that only if subsystem-level late/noirq/early suspend/resume
> callbacks are not present for the device, which is demonstrably inconsistent)
> and updates the resume part of the code accordingly (it doesn't need to check
> whether or not the subsystem-level callbacks are present any more).
> 
> The next patch, [2/7], is purely cosmetic and its only purpose is to reduce
> the LOC number and move related pieces of code closer to each other.
> 
> Patch [3/7] changes the PM core so that it doesn't skip any subsystem-level
> callbacks during system-wide resume (without the patch they may be skipped in
> the "early resume" and "resume" phases due to LEAVE_SUSPENDED being set which
> may be problematic) and to always run the driver's ->resume callback if the
> corresponding subsystem-level callback is not present (without the patch it
> may be skipped if LEAVE_SUSPENDED is set) to let it reverse the changes made
> by the driver's ->suspend callback (which always runs too) if need be.
> 
> Patches [4-6/7] rename one function in the PM core and two driver PM flags to
> make their names better reflect their purpose.
> 
> Finally, patch [7/7] updates the documentation of the driver PM flags to
> reflect the new code flows.
> 
> This patch series have been available in the git branch at
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git \
>   pm-sleep-core
> 
> for easier web and git access.

The series looks sane to me at a first look. I've added it to my local
tree for testing on all the pesky Bay and Cherry Trail devices I have
and which always cause trouble in this area.

Regards,

Hans


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 6/7] PM: sleep: core: Rename DPM_FLAG_LEAVE_SUSPENDED
  2020-04-10 15:57 ` [PATCH 6/7] PM: sleep: core: Rename DPM_FLAG_LEAVE_SUSPENDED Rafael J. Wysocki
@ 2020-04-15  9:51   ` Wolfram Sang
  0 siblings, 0 replies; 31+ messages in thread
From: Wolfram Sang @ 2020-04-15  9:51 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Linux PM, Alan Stern, Linux ACPI, Linux PCI, LKML, Bjorn Helgaas,
	Greg Kroah-Hartman, Mika Westerberg, Hans De Goede, Ulf Hansson,
	Linux I2C

[-- Attachment #1: Type: text/plain, Size: 392 bytes --]

On Fri, Apr 10, 2020 at 05:57:49PM +0200, Rafael J. Wysocki wrote:
> From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> 
> Rename DPM_FLAG_LEAVE_SUSPENDED to DPM_FLAG_MAY_SKIP_RESUME which
> matches its purpose more closely.
> 
> No functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Acked-by: Wolfram Sang <wsa@the-dreams.de> # for I2C


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v2 0/9] PM: sleep: core: Rearrange the handling of driver power management flags
  2020-04-10 15:46 [PATCH 0/7] PM: sleep: core: Rearrange the handling of driver power management flags Rafael J. Wysocki
                   ` (7 preceding siblings ...)
  2020-04-13 13:11 ` [PATCH 0/7] PM: sleep: core: Rearrange the handling of driver power management flags Hans de Goede
@ 2020-04-18 16:23 ` Rafael J. Wysocki
  2020-04-18 16:29   ` [PATCH v2 1/9] PM: sleep: core: Simplify the SMART_SUSPEND flag handling Rafael J. Wysocki
                     ` (12 more replies)
  8 siblings, 13 replies; 31+ messages in thread
From: Rafael J. Wysocki @ 2020-04-18 16:23 UTC (permalink / raw)
  To: Linux PM, Alan Stern
  Cc: Linux ACPI, Linux PCI, LKML, Bjorn Helgaas, Greg Kroah-Hartman,
	Mika Westerberg, Hans De Goede, Ulf Hansson

Hi,

This is an update including some fixes and extra patches based on the
continuation of the discussion [1].

On Friday, April 10, 2020 5:46:27 PM CEST Rafael J. Wysocki wrote:
> Hi Alan,
> 
> Following our recent discussion regarding the DPM_FLAG_* family of flags [1],
> I have decided to follow some of your recommendations and make changes to the
> core code handling those flags.
> 
> The purpose of this is basically to make the code more consistent internally,
> easier to follow and better documented.
> 
> First of all, patch [1/7] changes the PM core to skip driver-level "late"
> and "noirq" suspend callbacks for devices with SMART_SUSPEND set if they are
> still runtime-suspended during the "late" system-wide suspend phase (without
> the patch it does that only if subsystem-level late/noirq/early suspend/resume
> callbacks are not present for the device, which is demonstrably inconsistent)
> and updates the resume part of the code accordingly (it doesn't need to check
> whether or not the subsystem-level callbacks are present any more).
> 
> The next patch, [2/7], is purely cosmetic and its only purpose is to reduce
> the LOC number and move related pieces of code closer to each other.

The first two patches have not changed.

> Patch [3/7] changes the PM core so that it doesn't skip any subsystem-level
> callbacks during system-wide resume (without the patch they may be skipped in
> the "early resume" and "resume" phases due to LEAVE_SUSPENDED being set which
> may be problematic) and to always run the driver's ->resume callback if the
> corresponding subsystem-level callback is not present (without the patch it
> may be skipped if LEAVE_SUSPENDED is set) to let it reverse the changes made
> by the driver's ->suspend callback (which always runs too) if need be.

The difference between this one and patch [3/9] in the v2 is the fixed
definition of dev_pm_may_skip_resume(), renamed to dev_pm_skip_resume() by
one of the next patches.

Patch [4/9] changes the handling of the power.may_skip_resume flag to set it
to 'true' by default and updates the subsystems aware of it to clear it when
they don't want devices to stay in suspend.

> Patches [4-6/7] rename one function in the PM core and two driver PM flags to
> make their names better reflect their purpose.

These are patches [5/9] and [7-8/9] in the v2 and patch [6/9] renames
dev_pm_smart_suspend_and_suspended() to dev_pm_skip_suspend().

> Finally, patch [7/7] updates the documentation of the driver PM flags to
> reflect the new code flows.

This patch [9/9] now and it has been updated to reflect the new code changes.

The pm-sleep-core branch:

  git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git \
  pm-sleep-core

contains the v2 now.

Cheers!


[1] https://lore.kernel.org/linux-pm/Pine.LNX.4.44L0.2003251631360.1724-100000@netrider.rowland.org/




^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v2 1/9] PM: sleep: core: Simplify the SMART_SUSPEND flag handling
  2020-04-18 16:23 ` [PATCH v2 0/9] " Rafael J. Wysocki
@ 2020-04-18 16:29   ` Rafael J. Wysocki
  2020-04-18 16:51   ` [PATCH v2 2/9] PM: sleep: core: Fold functions into their callers Rafael J. Wysocki
                     ` (11 subsequent siblings)
  12 siblings, 0 replies; 31+ messages in thread
From: Rafael J. Wysocki @ 2020-04-18 16:29 UTC (permalink / raw)
  To: Linux PM, Alan Stern
  Cc: Linux ACPI, Linux PCI, LKML, Bjorn Helgaas, Greg Kroah-Hartman,
	Mika Westerberg, Hans De Goede, Ulf Hansson

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

The code to handle the SMART_SUSPEND driver PM flag is hard to follow
and somewhat inconsistent with respect to devices without middle-layer
(subsystem) callbacks.

Namely, for those devices the core takes the role of a middle layer
in providing the expected ordering of execution of callbacks (under
the assumption that the drivers setting SMART_SUSPEND can reuse their
PM-runtime callbacks directly for system-wide suspend).  To that end,
it prevents driver ->suspend_late and ->suspend_noirq callbacks from
being executed for devices that are still runtime-suspended in
__device_suspend_late(), because running the same callback funtion
that was previously run by PM-runtime for them may be invalid.

However, it does that only for devices without any middle-layer
callbacks for the late/noirq/early suspend/resume phases even
though it would be simpler and more consistent to skip the
driver-lavel callbacks for all devices with SMART_SUSPEND set
that are runtime-suspended in __device_suspend_late().

Simplify the code in accordance with the above observation.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

No changes from the first iteration.

---
 drivers/base/power/main.c | 118 +++++++++++++++-------------------------------
 1 file changed, 39 insertions(+), 79 deletions(-)

diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index fdd508a78ffd..5d0225573bbe 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -561,24 +561,6 @@ static void dpm_watchdog_clear(struct dpm_watchdog *wd)
 
 /*------------------------- Resume routines -------------------------*/
 
-/**
- * suspend_event - Return a "suspend" message for given "resume" one.
- * @resume_msg: PM message representing a system-wide resume transition.
- */
-static pm_message_t suspend_event(pm_message_t resume_msg)
-{
-	switch (resume_msg.event) {
-	case PM_EVENT_RESUME:
-		return PMSG_SUSPEND;
-	case PM_EVENT_THAW:
-	case PM_EVENT_RESTORE:
-		return PMSG_FREEZE;
-	case PM_EVENT_RECOVER:
-		return PMSG_HIBERNATE;
-	}
-	return PMSG_ON;
-}
-
 /**
  * dev_pm_may_skip_resume - System-wide device resume optimization check.
  * @dev: Target device.
@@ -656,37 +638,36 @@ static int device_resume_noirq(struct device *dev, pm_message_t state, bool asyn
 	if (!dpm_wait_for_superior(dev, async))
 		goto Out;
 
-	skip_resume = dev_pm_may_skip_resume(dev);
-
 	callback = dpm_subsys_resume_noirq_cb(dev, state, &info);
-	if (callback)
+	if (callback) {
+		skip_resume = false;
 		goto Run;
+	}
 
+	skip_resume = dev_pm_may_skip_resume(dev);
 	if (skip_resume)
 		goto Skip;
 
-	if (dev_pm_smart_suspend_and_suspended(dev)) {
-		pm_message_t suspend_msg = suspend_event(state);
-
-		/*
-		 * If "freeze" callbacks have been skipped during a transition
-		 * related to hibernation, the subsequent "thaw" callbacks must
-		 * be skipped too or bad things may happen.  Otherwise, resume
-		 * callbacks are going to be run for the device, so its runtime
-		 * PM status must be changed to reflect the new state after the
-		 * transition under way.
-		 */
-		if (!dpm_subsys_suspend_late_cb(dev, suspend_msg, NULL) &&
-		    !dpm_subsys_suspend_noirq_cb(dev, suspend_msg, NULL)) {
-			if (state.event == PM_EVENT_THAW) {
-				skip_resume = true;
-				goto Skip;
-			} else {
-				pm_runtime_set_active(dev);
-			}
-		}
+	/*
+	 * If "freeze" driver callbacks have been skipped during hibernation,
+	 * because the device was runtime-suspended in __device_suspend_late(),
+	 * the corresponding "thaw" callbacks must be skipped too, because
+	 * running them for a runtime-suspended device may not be valid.
+	 */
+	if (dev_pm_smart_suspend_and_suspended(dev) &&
+	    state.event == PM_EVENT_THAW) {
+		skip_resume = true;
+		goto Skip;
 	}
 
+	/*
+	 * The device is going to be resumed, so set its PM-runtime status to
+	 * "active", but do that only if DPM_FLAG_SMART_SUSPEND is set to avoid
+	 * confusing drivers that don't use it.
+	 */
+	if (dev_pm_smart_suspend_and_suspended(dev))
+		pm_runtime_set_active(dev);
+
 	if (dev->driver && dev->driver->pm) {
 		info = "noirq driver ";
 		callback = pm_noirq_op(dev->driver->pm, state);
@@ -1274,32 +1255,6 @@ static pm_callback_t dpm_subsys_suspend_noirq_cb(struct device *dev,
 	return callback;
 }
 
-static bool device_must_resume(struct device *dev, pm_message_t state,
-			       bool no_subsys_suspend_noirq)
-{
-	pm_message_t resume_msg = resume_event(state);
-
-	/*
-	 * If all of the device driver's "noirq", "late" and "early" callbacks
-	 * are invoked directly by the core, the decision to allow the device to
-	 * stay in suspend can be based on its current runtime PM status and its
-	 * wakeup settings.
-	 */
-	if (no_subsys_suspend_noirq &&
-	    !dpm_subsys_suspend_late_cb(dev, state, NULL) &&
-	    !dpm_subsys_resume_early_cb(dev, resume_msg, NULL) &&
-	    !dpm_subsys_resume_noirq_cb(dev, resume_msg, NULL))
-		return !pm_runtime_status_suspended(dev) &&
-			(resume_msg.event != PM_EVENT_RESUME ||
-			 (device_can_wakeup(dev) && !device_may_wakeup(dev)));
-
-	/*
-	 * The only safe strategy here is to require that if the device may not
-	 * be left in suspend, resume callbacks must be invoked for it.
-	 */
-	return !dev->power.may_skip_resume;
-}
-
 /**
  * __device_suspend_noirq - Execute a "noirq suspend" callback for given device.
  * @dev: Device to handle.
@@ -1313,7 +1268,6 @@ static int __device_suspend_noirq(struct device *dev, pm_message_t state, bool a
 {
 	pm_callback_t callback;
 	const char *info;
-	bool no_subsys_cb = false;
 	int error = 0;
 
 	TRACE_DEVICE(dev);
@@ -1331,9 +1285,7 @@ static int __device_suspend_noirq(struct device *dev, pm_message_t state, bool a
 	if (callback)
 		goto Run;
 
-	no_subsys_cb = !dpm_subsys_suspend_late_cb(dev, state, NULL);
-
-	if (dev_pm_smart_suspend_and_suspended(dev) && no_subsys_cb)
+	if (dev_pm_smart_suspend_and_suspended(dev))
 		goto Skip;
 
 	if (dev->driver && dev->driver->pm) {
@@ -1351,13 +1303,16 @@ static int __device_suspend_noirq(struct device *dev, pm_message_t state, bool a
 Skip:
 	dev->power.is_noirq_suspended = true;
 
-	if (dev_pm_test_driver_flags(dev, DPM_FLAG_LEAVE_SUSPENDED)) {
-		dev->power.must_resume = dev->power.must_resume ||
-				atomic_read(&dev->power.usage_count) > 1 ||
-				device_must_resume(dev, state, no_subsys_cb);
-	} else {
+	/*
+	 * Skipping the resume of devices that were in use right before the
+	 * system suspend (as indicated by their PM-runtime usage counters)
+	 * would be suboptimal.  Also resume them if doing that is not allowed
+	 * to be skipped.
+	 */
+	if (atomic_read(&dev->power.usage_count) > 1 ||
+	    !(dev_pm_test_driver_flags(dev, DPM_FLAG_LEAVE_SUSPENDED) &&
+	      dev->power.may_skip_resume))
 		dev->power.must_resume = true;
-	}
 
 	if (dev->power.must_resume)
 		dpm_superior_set_must_resume(dev);
@@ -1539,9 +1494,14 @@ static int __device_suspend_late(struct device *dev, pm_message_t state, bool as
 	if (callback)
 		goto Run;
 
-	if (dev_pm_smart_suspend_and_suspended(dev) &&
-	    !dpm_subsys_suspend_noirq_cb(dev, state, NULL))
+	if (dev_pm_smart_suspend_and_suspended(dev)) {
+		/*
+		 * In principle, the resume of the device may be skippend if it
+		 * remains in runtime suspend at this point.
+		 */
+		dev->power.may_skip_resume = true;
 		goto Skip;
+	}
 
 	if (dev->driver && dev->driver->pm) {
 		info = "late driver ";
-- 
2.16.4





^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v2 2/9] PM: sleep: core: Fold functions into their callers
  2020-04-18 16:23 ` [PATCH v2 0/9] " Rafael J. Wysocki
  2020-04-18 16:29   ` [PATCH v2 1/9] PM: sleep: core: Simplify the SMART_SUSPEND flag handling Rafael J. Wysocki
@ 2020-04-18 16:51   ` Rafael J. Wysocki
  2020-04-18 16:52   ` [PATCH v2 3/9] PM: sleep: core: Do not skip callbacks in the resume phase Rafael J. Wysocki
                     ` (10 subsequent siblings)
  12 siblings, 0 replies; 31+ messages in thread
From: Rafael J. Wysocki @ 2020-04-18 16:51 UTC (permalink / raw)
  To: Linux PM, Alan Stern
  Cc: Linux ACPI, Linux PCI, LKML, Bjorn Helgaas, Greg Kroah-Hartman,
	Mika Westerberg, Hans De Goede, Ulf Hansson

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

Fold four functions in the PM core that each have only one caller
now into their callers.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

No changes from the first iteration.

---
 drivers/base/power/main.c | 198 ++++++++++++++--------------------------------
 1 file changed, 60 insertions(+), 138 deletions(-)

diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index 5d0225573bbe..75d7cdb4de9c 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -573,43 +573,6 @@ bool dev_pm_may_skip_resume(struct device *dev)
 	return !dev->power.must_resume && pm_transition.event != PM_EVENT_RESTORE;
 }
 
-static pm_callback_t dpm_subsys_resume_noirq_cb(struct device *dev,
-						pm_message_t state,
-						const char **info_p)
-{
-	pm_callback_t callback;
-	const char *info;
-
-	if (dev->pm_domain) {
-		info = "noirq power domain ";
-		callback = pm_noirq_op(&dev->pm_domain->ops, state);
-	} else if (dev->type && dev->type->pm) {
-		info = "noirq type ";
-		callback = pm_noirq_op(dev->type->pm, state);
-	} else if (dev->class && dev->class->pm) {
-		info = "noirq class ";
-		callback = pm_noirq_op(dev->class->pm, state);
-	} else if (dev->bus && dev->bus->pm) {
-		info = "noirq bus ";
-		callback = pm_noirq_op(dev->bus->pm, state);
-	} else {
-		return NULL;
-	}
-
-	if (info_p)
-		*info_p = info;
-
-	return callback;
-}
-
-static pm_callback_t dpm_subsys_suspend_noirq_cb(struct device *dev,
-						 pm_message_t state,
-						 const char **info_p);
-
-static pm_callback_t dpm_subsys_suspend_late_cb(struct device *dev,
-						pm_message_t state,
-						const char **info_p);
-
 /**
  * device_resume_noirq - Execute a "noirq resume" callback for given device.
  * @dev: Device to handle.
@@ -621,8 +584,8 @@ static pm_callback_t dpm_subsys_suspend_late_cb(struct device *dev,
  */
 static int device_resume_noirq(struct device *dev, pm_message_t state, bool async)
 {
-	pm_callback_t callback;
-	const char *info;
+	pm_callback_t callback = NULL;
+	const char *info = NULL;
 	bool skip_resume;
 	int error = 0;
 
@@ -638,7 +601,19 @@ static int device_resume_noirq(struct device *dev, pm_message_t state, bool asyn
 	if (!dpm_wait_for_superior(dev, async))
 		goto Out;
 
-	callback = dpm_subsys_resume_noirq_cb(dev, state, &info);
+	if (dev->pm_domain) {
+		info = "noirq power domain ";
+		callback = pm_noirq_op(&dev->pm_domain->ops, state);
+	} else if (dev->type && dev->type->pm) {
+		info = "noirq type ";
+		callback = pm_noirq_op(dev->type->pm, state);
+	} else if (dev->class && dev->class->pm) {
+		info = "noirq class ";
+		callback = pm_noirq_op(dev->class->pm, state);
+	} else if (dev->bus && dev->bus->pm) {
+		info = "noirq bus ";
+		callback = pm_noirq_op(dev->bus->pm, state);
+	}
 	if (callback) {
 		skip_resume = false;
 		goto Run;
@@ -791,35 +766,6 @@ void dpm_resume_noirq(pm_message_t state)
 	cpuidle_resume();
 }
 
-static pm_callback_t dpm_subsys_resume_early_cb(struct device *dev,
-						pm_message_t state,
-						const char **info_p)
-{
-	pm_callback_t callback;
-	const char *info;
-
-	if (dev->pm_domain) {
-		info = "early power domain ";
-		callback = pm_late_early_op(&dev->pm_domain->ops, state);
-	} else if (dev->type && dev->type->pm) {
-		info = "early type ";
-		callback = pm_late_early_op(dev->type->pm, state);
-	} else if (dev->class && dev->class->pm) {
-		info = "early class ";
-		callback = pm_late_early_op(dev->class->pm, state);
-	} else if (dev->bus && dev->bus->pm) {
-		info = "early bus ";
-		callback = pm_late_early_op(dev->bus->pm, state);
-	} else {
-		return NULL;
-	}
-
-	if (info_p)
-		*info_p = info;
-
-	return callback;
-}
-
 /**
  * device_resume_early - Execute an "early resume" callback for given device.
  * @dev: Device to handle.
@@ -830,8 +776,8 @@ static pm_callback_t dpm_subsys_resume_early_cb(struct device *dev,
  */
 static int device_resume_early(struct device *dev, pm_message_t state, bool async)
 {
-	pm_callback_t callback;
-	const char *info;
+	pm_callback_t callback = NULL;
+	const char *info = NULL;
 	int error = 0;
 
 	TRACE_DEVICE(dev);
@@ -846,9 +792,19 @@ static int device_resume_early(struct device *dev, pm_message_t state, bool asyn
 	if (!dpm_wait_for_superior(dev, async))
 		goto Out;
 
-	callback = dpm_subsys_resume_early_cb(dev, state, &info);
-
-	if (!callback && dev->driver && dev->driver->pm) {
+	if (dev->pm_domain) {
+		info = "early power domain ";
+		callback = pm_late_early_op(&dev->pm_domain->ops, state);
+	} else if (dev->type && dev->type->pm) {
+		info = "early type ";
+		callback = pm_late_early_op(dev->type->pm, state);
+	} else if (dev->class && dev->class->pm) {
+		info = "early class ";
+		callback = pm_late_early_op(dev->class->pm, state);
+	} else if (dev->bus && dev->bus->pm) {
+		info = "early bus ";
+		callback = pm_late_early_op(dev->bus->pm, state);
+	} else if (dev->driver && dev->driver->pm) {
 		info = "early driver ";
 		callback = pm_late_early_op(dev->driver->pm, state);
 	}
@@ -1226,35 +1182,6 @@ static void dpm_superior_set_must_resume(struct device *dev)
 	device_links_read_unlock(idx);
 }
 
-static pm_callback_t dpm_subsys_suspend_noirq_cb(struct device *dev,
-						 pm_message_t state,
-						 const char **info_p)
-{
-	pm_callback_t callback;
-	const char *info;
-
-	if (dev->pm_domain) {
-		info = "noirq power domain ";
-		callback = pm_noirq_op(&dev->pm_domain->ops, state);
-	} else if (dev->type && dev->type->pm) {
-		info = "noirq type ";
-		callback = pm_noirq_op(dev->type->pm, state);
-	} else if (dev->class && dev->class->pm) {
-		info = "noirq class ";
-		callback = pm_noirq_op(dev->class->pm, state);
-	} else if (dev->bus && dev->bus->pm) {
-		info = "noirq bus ";
-		callback = pm_noirq_op(dev->bus->pm, state);
-	} else {
-		return NULL;
-	}
-
-	if (info_p)
-		*info_p = info;
-
-	return callback;
-}
-
 /**
  * __device_suspend_noirq - Execute a "noirq suspend" callback for given device.
  * @dev: Device to handle.
@@ -1266,8 +1193,8 @@ static pm_callback_t dpm_subsys_suspend_noirq_cb(struct device *dev,
  */
 static int __device_suspend_noirq(struct device *dev, pm_message_t state, bool async)
 {
-	pm_callback_t callback;
-	const char *info;
+	pm_callback_t callback = NULL;
+	const char *info = NULL;
 	int error = 0;
 
 	TRACE_DEVICE(dev);
@@ -1281,7 +1208,19 @@ static int __device_suspend_noirq(struct device *dev, pm_message_t state, bool a
 	if (dev->power.syscore || dev->power.direct_complete)
 		goto Complete;
 
-	callback = dpm_subsys_suspend_noirq_cb(dev, state, &info);
+	if (dev->pm_domain) {
+		info = "noirq power domain ";
+		callback = pm_noirq_op(&dev->pm_domain->ops, state);
+	} else if (dev->type && dev->type->pm) {
+		info = "noirq type ";
+		callback = pm_noirq_op(dev->type->pm, state);
+	} else if (dev->class && dev->class->pm) {
+		info = "noirq class ";
+		callback = pm_noirq_op(dev->class->pm, state);
+	} else if (dev->bus && dev->bus->pm) {
+		info = "noirq bus ";
+		callback = pm_noirq_op(dev->bus->pm, state);
+	}
 	if (callback)
 		goto Run;
 
@@ -1429,35 +1368,6 @@ static void dpm_propagate_wakeup_to_parent(struct device *dev)
 	spin_unlock_irq(&parent->power.lock);
 }
 
-static pm_callback_t dpm_subsys_suspend_late_cb(struct device *dev,
-						pm_message_t state,
-						const char **info_p)
-{
-	pm_callback_t callback;
-	const char *info;
-
-	if (dev->pm_domain) {
-		info = "late power domain ";
-		callback = pm_late_early_op(&dev->pm_domain->ops, state);
-	} else if (dev->type && dev->type->pm) {
-		info = "late type ";
-		callback = pm_late_early_op(dev->type->pm, state);
-	} else if (dev->class && dev->class->pm) {
-		info = "late class ";
-		callback = pm_late_early_op(dev->class->pm, state);
-	} else if (dev->bus && dev->bus->pm) {
-		info = "late bus ";
-		callback = pm_late_early_op(dev->bus->pm, state);
-	} else {
-		return NULL;
-	}
-
-	if (info_p)
-		*info_p = info;
-
-	return callback;
-}
-
 /**
  * __device_suspend_late - Execute a "late suspend" callback for given device.
  * @dev: Device to handle.
@@ -1468,8 +1378,8 @@ static pm_callback_t dpm_subsys_suspend_late_cb(struct device *dev,
  */
 static int __device_suspend_late(struct device *dev, pm_message_t state, bool async)
 {
-	pm_callback_t callback;
-	const char *info;
+	pm_callback_t callback = NULL;
+	const char *info = NULL;
 	int error = 0;
 
 	TRACE_DEVICE(dev);
@@ -1490,7 +1400,19 @@ static int __device_suspend_late(struct device *dev, pm_message_t state, bool as
 	if (dev->power.syscore || dev->power.direct_complete)
 		goto Complete;
 
-	callback = dpm_subsys_suspend_late_cb(dev, state, &info);
+	if (dev->pm_domain) {
+		info = "late power domain ";
+		callback = pm_late_early_op(&dev->pm_domain->ops, state);
+	} else if (dev->type && dev->type->pm) {
+		info = "late type ";
+		callback = pm_late_early_op(dev->type->pm, state);
+	} else if (dev->class && dev->class->pm) {
+		info = "late class ";
+		callback = pm_late_early_op(dev->class->pm, state);
+	} else if (dev->bus && dev->bus->pm) {
+		info = "late bus ";
+		callback = pm_late_early_op(dev->bus->pm, state);
+	}
 	if (callback)
 		goto Run;
 
-- 
2.16.4





^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v2 3/9] PM: sleep: core: Do not skip callbacks in the resume phase
  2020-04-18 16:23 ` [PATCH v2 0/9] " Rafael J. Wysocki
  2020-04-18 16:29   ` [PATCH v2 1/9] PM: sleep: core: Simplify the SMART_SUSPEND flag handling Rafael J. Wysocki
  2020-04-18 16:51   ` [PATCH v2 2/9] PM: sleep: core: Fold functions into their callers Rafael J. Wysocki
@ 2020-04-18 16:52   ` Rafael J. Wysocki
  2020-04-18 16:52   ` [PATCH v2 4/9] PM: sleep: core: Rework the power.may_skip_resume handling Rafael J. Wysocki
                     ` (9 subsequent siblings)
  12 siblings, 0 replies; 31+ messages in thread
From: Rafael J. Wysocki @ 2020-04-18 16:52 UTC (permalink / raw)
  To: Linux PM, Alan Stern
  Cc: Linux ACPI, Linux PCI, LKML, Bjorn Helgaas, Greg Kroah-Hartman,
	Mika Westerberg, Hans De Goede, Ulf Hansson

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

The current code in device_resume_noirq() causes the entire early
resume and resume phases of device suspend to be skipped for
devices for which the noirq resume phase have been skipped (due
to the LEAVE_SUSPENDED flag being set) on the premise that those
devices should stay in runtime-suspend after system-wide resume.

However, that may not be correct in two situations.  First, the
middle layer (subsystem) noirq resume callback may be missing for
a given device, but its early resume callback may be present and it
may need to do something even if it decides to skip the driver
callback.  Second, if the device's wakeup settings were adjusted
in the suspend phase without resuming the device (that was in
runtime suspend at that time), they most likely need to be
adjusted again in the resume phase and so the driver callback
in that phase needs to be run.

For the above reason, modify the core to allow the middle layer
->resume_late callback to run even if its ->resume_noirq callback
is missing (and the core has skipped the driver-level callback
in that phase) and to allow all device callbacks to run in the
resume phase.  Also make the core set the PM-runtime status of
devices with SMART_SUSPEND set whose resume callbacks are not
skipped to "active" in the "noirq" resume phase and update the
affected subsystems (PCI and ACPI) accordingly.

After this change, middle-layer (subsystem) callbacks will always
be invoked in all phases of system suspend and resume and driver
callbacks will always run in the prepare, suspend, resume, and
complete phases for all devices.

For devices with SMART_SUSPEND set, driver callbacks will be
skipped in the late and noirq phases of system suspend if those
devices remain in runtime suspend in __device_suspend_late().
Driver callbacks will also be skipped for them during the
noirq and early phases of the "thaw" transition related to
hibernation in that case.

Setting LEAVE_SUSPENDED means that the driver allows its callbacks
to be skipped in the noirq and early phases of system resume, but
some additional conditions need to be met for that to happen (among
other things, the power.may_skip_resume flag needs to be set for the
device during system suspend for the driver callbacks to be skipped
during the subsequent resume transition).

For all devices with SMART_SUSPEND set whose driver callbacks are
invoked during system resume, the PM-runtime status will be set to
"active" (by the core).

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

-> v2:
  * Fixed definition of dev_pm_may_skip_resume().
  * Updated changelog.

---
 Documentation/power/pci.rst |  5 +--
 drivers/acpi/acpi_lpss.c    |  6 ++--
 drivers/acpi/device_pm.c    | 15 ++++----
 drivers/base/power/main.c   | 85 ++++++++++++++++++++++-----------------------
 drivers/pci/pci-driver.c    | 18 +++++-----
 5 files changed, 62 insertions(+), 67 deletions(-)

diff --git a/Documentation/power/pci.rst b/Documentation/power/pci.rst
index 0924d29636ad..a39b2461919a 100644
--- a/Documentation/power/pci.rst
+++ b/Documentation/power/pci.rst
@@ -1035,10 +1035,7 @@ This flag is checked by the PM core, but the PCI bus type informs the PM core
 which devices may be left in suspend from its perspective (that happens during
 the "noirq" phase of system-wide suspend and analogous transitions) and next it
 uses the dev_pm_may_skip_resume() helper to decide whether or not to return from
-pci_pm_resume_noirq() early, as the PM core will skip the remaining resume
-callbacks for the device during the transition under way and will set its
-runtime PM status to "suspended" if dev_pm_may_skip_resume() returns "true" for
-it.
+pci_pm_resume_noirq() and pci_pm_resume_early() upfront.
 
 3.2. Device Runtime Power Management
 ------------------------------------
diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c
index dee999938213..c4a84df6cc98 100644
--- a/drivers/acpi/acpi_lpss.c
+++ b/drivers/acpi/acpi_lpss.c
@@ -1093,6 +1093,9 @@ static int acpi_lpss_resume_early(struct device *dev)
 	if (pdata->dev_desc->resume_from_noirq)
 		return 0;
 
+	if (dev_pm_may_skip_resume(dev))
+		return 0;
+
 	return acpi_lpss_do_resume_early(dev);
 }
 
@@ -1105,9 +1108,6 @@ static int acpi_lpss_resume_noirq(struct device *dev)
 	if (dev_pm_may_skip_resume(dev))
 		return 0;
 
-	if (dev_pm_smart_suspend_and_suspended(dev))
-		pm_runtime_set_active(dev);
-
 	ret = pm_generic_resume_noirq(dev);
 	if (ret)
 		return ret;
diff --git a/drivers/acpi/device_pm.c b/drivers/acpi/device_pm.c
index b2263ec67b43..399684085f85 100644
--- a/drivers/acpi/device_pm.c
+++ b/drivers/acpi/device_pm.c
@@ -1132,14 +1132,6 @@ static int acpi_subsys_resume_noirq(struct device *dev)
 	if (dev_pm_may_skip_resume(dev))
 		return 0;
 
-	/*
-	 * Devices with DPM_FLAG_SMART_SUSPEND may be left in runtime suspend
-	 * during system suspend, so update their runtime PM status to "active"
-	 * as they will be put into D0 going forward.
-	 */
-	if (dev_pm_smart_suspend_and_suspended(dev))
-		pm_runtime_set_active(dev);
-
 	return pm_generic_resume_noirq(dev);
 }
 
@@ -1153,7 +1145,12 @@ static int acpi_subsys_resume_noirq(struct device *dev)
  */
 static int acpi_subsys_resume_early(struct device *dev)
 {
-	int ret = acpi_dev_resume(dev);
+	int ret;
+
+	if (dev_pm_may_skip_resume(dev))
+		return 0;
+
+	ret = acpi_dev_resume(dev);
 	return ret ? ret : pm_generic_resume_early(dev);
 }
 
diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index 75d7cdb4de9c..25b0302188d8 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -565,12 +565,22 @@ static void dpm_watchdog_clear(struct dpm_watchdog *wd)
  * dev_pm_may_skip_resume - System-wide device resume optimization check.
  * @dev: Target device.
  *
- * Checks whether or not the device may be left in suspend after a system-wide
- * transition to the working state.
+ * Return:
+ * - %false if the transition under way is RESTORE.
+ * - The return value of dev_pm_smart_suspend_and_suspended() if the transition
+ *   under way is THAW.
+ * - The logical negation of %power.must_resume otherwise (that is, when the
+ *   transition under way is RESUME).
  */
 bool dev_pm_may_skip_resume(struct device *dev)
 {
-	return !dev->power.must_resume && pm_transition.event != PM_EVENT_RESTORE;
+	if (pm_transition.event == PM_EVENT_RESTORE)
+		return false;
+
+	if (pm_transition.event == PM_EVENT_THAW)
+		return dev_pm_smart_suspend_and_suspended(dev);
+
+	return !dev->power.must_resume;
 }
 
 /**
@@ -601,6 +611,22 @@ static int device_resume_noirq(struct device *dev, pm_message_t state, bool asyn
 	if (!dpm_wait_for_superior(dev, async))
 		goto Out;
 
+	skip_resume = dev_pm_may_skip_resume(dev);
+	/*
+	 * If the driver callback is skipped below or by the middle layer
+	 * callback and device_resume_early() also skips the driver callback for
+	 * this device later, it needs to appear as "suspended" to PM-runtime,
+	 * so change its status accordingly.
+	 *
+	 * Otherwise, the device is going to be resumed, so set its PM-runtime
+	 * status to "active", but do that only if DPM_FLAG_SMART_SUSPEND is set
+	 * to avoid confusing drivers that don't use it.
+	 */
+	if (skip_resume)
+		pm_runtime_set_suspended(dev);
+	else if (dev_pm_smart_suspend_and_suspended(dev))
+		pm_runtime_set_active(dev);
+
 	if (dev->pm_domain) {
 		info = "noirq power domain ";
 		callback = pm_noirq_op(&dev->pm_domain->ops, state);
@@ -614,35 +640,12 @@ static int device_resume_noirq(struct device *dev, pm_message_t state, bool asyn
 		info = "noirq bus ";
 		callback = pm_noirq_op(dev->bus->pm, state);
 	}
-	if (callback) {
-		skip_resume = false;
+	if (callback)
 		goto Run;
-	}
 
-	skip_resume = dev_pm_may_skip_resume(dev);
 	if (skip_resume)
 		goto Skip;
 
-	/*
-	 * If "freeze" driver callbacks have been skipped during hibernation,
-	 * because the device was runtime-suspended in __device_suspend_late(),
-	 * the corresponding "thaw" callbacks must be skipped too, because
-	 * running them for a runtime-suspended device may not be valid.
-	 */
-	if (dev_pm_smart_suspend_and_suspended(dev) &&
-	    state.event == PM_EVENT_THAW) {
-		skip_resume = true;
-		goto Skip;
-	}
-
-	/*
-	 * The device is going to be resumed, so set its PM-runtime status to
-	 * "active", but do that only if DPM_FLAG_SMART_SUSPEND is set to avoid
-	 * confusing drivers that don't use it.
-	 */
-	if (dev_pm_smart_suspend_and_suspended(dev))
-		pm_runtime_set_active(dev);
-
 	if (dev->driver && dev->driver->pm) {
 		info = "noirq driver ";
 		callback = pm_noirq_op(dev->driver->pm, state);
@@ -654,20 +657,6 @@ static int device_resume_noirq(struct device *dev, pm_message_t state, bool asyn
 Skip:
 	dev->power.is_noirq_suspended = false;
 
-	if (skip_resume) {
-		/* Make the next phases of resume skip the device. */
-		dev->power.is_late_suspended = false;
-		dev->power.is_suspended = false;
-		/*
-		 * The device is going to be left in suspend, but it might not
-		 * have been in runtime suspend before the system suspended, so
-		 * its runtime PM status needs to be updated to avoid confusing
-		 * the runtime PM framework when runtime PM is enabled for the
-		 * device again.
-		 */
-		pm_runtime_set_suspended(dev);
-	}
-
 Out:
 	complete_all(&dev->power.completion);
 	TRACE_RESUME(error);
@@ -804,15 +793,25 @@ static int device_resume_early(struct device *dev, pm_message_t state, bool asyn
 	} else if (dev->bus && dev->bus->pm) {
 		info = "early bus ";
 		callback = pm_late_early_op(dev->bus->pm, state);
-	} else if (dev->driver && dev->driver->pm) {
+	}
+	if (callback)
+		goto Run;
+
+	if (dev_pm_may_skip_resume(dev))
+		goto Skip;
+
+	if (dev->driver && dev->driver->pm) {
 		info = "early driver ";
 		callback = pm_late_early_op(dev->driver->pm, state);
 	}
 
+Run:
 	error = dpm_run_callback(callback, dev, state, info);
+
+Skip:
 	dev->power.is_late_suspended = false;
 
- Out:
+Out:
 	TRACE_RESUME(error);
 
 	pm_runtime_enable(dev);
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index 0454ca0e4e3f..685fbf044911 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -896,14 +896,6 @@ static int pci_pm_resume_noirq(struct device *dev)
 	if (dev_pm_may_skip_resume(dev))
 		return 0;
 
-	/*
-	 * Devices with DPM_FLAG_SMART_SUSPEND may be left in runtime suspend
-	 * during system suspend, so update their runtime PM status to "active"
-	 * as they are going to be put into D0 shortly.
-	 */
-	if (dev_pm_smart_suspend_and_suspended(dev))
-		pm_runtime_set_active(dev);
-
 	/*
 	 * In the suspend-to-idle case, devices left in D0 during suspend will
 	 * stay in D0, so it is not necessary to restore or update their
@@ -928,6 +920,14 @@ static int pci_pm_resume_noirq(struct device *dev)
 	return 0;
 }
 
+static int pci_pm_resume_early(struct device *dev)
+{
+	if (dev_pm_may_skip_resume(dev))
+		return 0;
+
+	return pm_generic_resume_early(dev);
+}
+
 static int pci_pm_resume(struct device *dev)
 {
 	struct pci_dev *pci_dev = to_pci_dev(dev);
@@ -961,6 +961,7 @@ static int pci_pm_resume(struct device *dev)
 #define pci_pm_suspend_late	NULL
 #define pci_pm_suspend_noirq	NULL
 #define pci_pm_resume		NULL
+#define pci_pm_resume_early	NULL
 #define pci_pm_resume_noirq	NULL
 
 #endif /* !CONFIG_SUSPEND */
@@ -1358,6 +1359,7 @@ static const struct dev_pm_ops pci_dev_pm_ops = {
 	.suspend = pci_pm_suspend,
 	.suspend_late = pci_pm_suspend_late,
 	.resume = pci_pm_resume,
+	.resume_early = pci_pm_resume_early,
 	.freeze = pci_pm_freeze,
 	.thaw = pci_pm_thaw,
 	.poweroff = pci_pm_poweroff,
-- 
2.16.4





^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v2 4/9] PM: sleep: core: Rework the power.may_skip_resume handling
  2020-04-18 16:23 ` [PATCH v2 0/9] " Rafael J. Wysocki
                     ` (2 preceding siblings ...)
  2020-04-18 16:52   ` [PATCH v2 3/9] PM: sleep: core: Do not skip callbacks in the resume phase Rafael J. Wysocki
@ 2020-04-18 16:52   ` Rafael J. Wysocki
  2020-04-18 16:52   ` [PATCH v2 5/9] PM: sleep: core: Rename dev_pm_may_skip_resume() Rafael J. Wysocki
                     ` (8 subsequent siblings)
  12 siblings, 0 replies; 31+ messages in thread
From: Rafael J. Wysocki @ 2020-04-18 16:52 UTC (permalink / raw)
  To: Linux PM, Alan Stern
  Cc: Linux ACPI, Linux PCI, LKML, Bjorn Helgaas, Greg Kroah-Hartman,
	Mika Westerberg, Hans De Goede, Ulf Hansson

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

Because the power.may_skip_resume device status bit is taken
into account in combination with the DPM_FLAG_LEAVE_SUSPENDED
driver flag, it can be set to 'true' for all devices in the
"suspend" phase of a suspend-resume cycle, so do that.

Then, neither the PM core nor the middle-layer (sybsystem) code
handling it needs to set it to 'true' any more and it just has
to be cleared if there is a reason to avoid skipping the "noirq"
and "early" resume callbacks provided by the driver, so update
the code in question accordingly.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

New patch.

---
 drivers/acpi/device_pm.c  |  8 +++-----
 drivers/base/power/main.c | 10 ++--------
 drivers/pci/pci-driver.c  |  8 +++-----
 3 files changed, 8 insertions(+), 18 deletions(-)

diff --git a/drivers/acpi/device_pm.c b/drivers/acpi/device_pm.c
index 399684085f85..1b02d7dc7d34 100644
--- a/drivers/acpi/device_pm.c
+++ b/drivers/acpi/device_pm.c
@@ -1100,10 +1100,8 @@ int acpi_subsys_suspend_noirq(struct device *dev)
 {
 	int ret;
 
-	if (dev_pm_smart_suspend_and_suspended(dev)) {
-		dev->power.may_skip_resume = true;
+	if (dev_pm_smart_suspend_and_suspended(dev))
 		return 0;
-	}
 
 	ret = pm_generic_suspend_noirq(dev);
 	if (ret)
@@ -1116,8 +1114,8 @@ int acpi_subsys_suspend_noirq(struct device *dev)
 	 * acpi_subsys_complete() to take care of fixing up the device's state
 	 * anyway, if need be.
 	 */
-	dev->power.may_skip_resume = device_may_wakeup(dev) ||
-					!device_can_wakeup(dev);
+	if (device_can_wakeup(dev) && !device_may_wakeup(dev))
+		dev->power.may_skip_resume = false;
 
 	return 0;
 }
diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index 25b0302188d8..5adf0be6aa47 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -1415,14 +1415,8 @@ static int __device_suspend_late(struct device *dev, pm_message_t state, bool as
 	if (callback)
 		goto Run;
 
-	if (dev_pm_smart_suspend_and_suspended(dev)) {
-		/*
-		 * In principle, the resume of the device may be skippend if it
-		 * remains in runtime suspend at this point.
-		 */
-		dev->power.may_skip_resume = true;
+	if (dev_pm_smart_suspend_and_suspended(dev))
 		goto Skip;
-	}
 
 	if (dev->driver && dev->driver->pm) {
 		info = "late driver ";
@@ -1647,7 +1641,7 @@ static int __device_suspend(struct device *dev, pm_message_t state, bool async)
 		dev->power.direct_complete = false;
 	}
 
-	dev->power.may_skip_resume = false;
+	dev->power.may_skip_resume = true;
 	dev->power.must_resume = false;
 
 	dpm_watchdog_set(&wd, dev);
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index 685fbf044911..ce220b1987df 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -789,10 +789,8 @@ static int pci_pm_suspend_noirq(struct device *dev)
 	struct pci_dev *pci_dev = to_pci_dev(dev);
 	const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
 
-	if (dev_pm_smart_suspend_and_suspended(dev)) {
-		dev->power.may_skip_resume = true;
+	if (dev_pm_smart_suspend_and_suspended(dev))
 		return 0;
-	}
 
 	if (pci_has_legacy_pm_support(pci_dev))
 		return pci_legacy_suspend_late(dev, PMSG_SUSPEND);
@@ -880,8 +878,8 @@ static int pci_pm_suspend_noirq(struct device *dev)
 	 * pci_pm_complete() to take care of fixing up the device's state
 	 * anyway, if need be.
 	 */
-	dev->power.may_skip_resume = device_may_wakeup(dev) ||
-					!device_can_wakeup(dev);
+	if (device_can_wakeup(dev) && !device_may_wakeup(dev))
+		dev->power.may_skip_resume = false;
 
 	return 0;
 }
-- 
2.16.4





^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v2 5/9] PM: sleep: core: Rename dev_pm_may_skip_resume()
  2020-04-18 16:23 ` [PATCH v2 0/9] " Rafael J. Wysocki
                     ` (3 preceding siblings ...)
  2020-04-18 16:52   ` [PATCH v2 4/9] PM: sleep: core: Rework the power.may_skip_resume handling Rafael J. Wysocki
@ 2020-04-18 16:52   ` Rafael J. Wysocki
  2020-04-18 16:52   ` [PATCH v2 6/9] PM: sleep: core: Rename dev_pm_smart_suspend_and_suspended() Rafael J. Wysocki
                     ` (7 subsequent siblings)
  12 siblings, 0 replies; 31+ messages in thread
From: Rafael J. Wysocki @ 2020-04-18 16:52 UTC (permalink / raw)
  To: Linux PM, Alan Stern
  Cc: Linux ACPI, Linux PCI, LKML, Bjorn Helgaas, Greg Kroah-Hartman,
	Mika Westerberg, Hans De Goede, Ulf Hansson

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

The name of dev_pm_may_skip_resume() may be easily confused with the
power.may_skip_resume flag which is not checked by that function, so
rename the former as dev_pm_skip_resume().

No functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

-> v2:
   * Rebased.

---
 Documentation/power/pci.rst | 2 +-
 drivers/acpi/acpi_lpss.c    | 4 ++--
 drivers/acpi/device_pm.c    | 4 ++--
 drivers/base/power/main.c   | 8 ++++----
 drivers/pci/pci-driver.c    | 4 ++--
 include/linux/pm.h          | 2 +-
 6 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/Documentation/power/pci.rst b/Documentation/power/pci.rst
index a39b2461919a..aa1c7fce6cd0 100644
--- a/Documentation/power/pci.rst
+++ b/Documentation/power/pci.rst
@@ -1034,7 +1034,7 @@ device to be left in suspend after system-wide transitions to the working state.
 This flag is checked by the PM core, but the PCI bus type informs the PM core
 which devices may be left in suspend from its perspective (that happens during
 the "noirq" phase of system-wide suspend and analogous transitions) and next it
-uses the dev_pm_may_skip_resume() helper to decide whether or not to return from
+uses the dev_pm_skip_resume() helper to decide whether or not to return from
 pci_pm_resume_noirq() and pci_pm_resume_early() upfront.
 
 3.2. Device Runtime Power Management
diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c
index c4a84df6cc98..7632df1a5be3 100644
--- a/drivers/acpi/acpi_lpss.c
+++ b/drivers/acpi/acpi_lpss.c
@@ -1093,7 +1093,7 @@ static int acpi_lpss_resume_early(struct device *dev)
 	if (pdata->dev_desc->resume_from_noirq)
 		return 0;
 
-	if (dev_pm_may_skip_resume(dev))
+	if (dev_pm_skip_resume(dev))
 		return 0;
 
 	return acpi_lpss_do_resume_early(dev);
@@ -1105,7 +1105,7 @@ static int acpi_lpss_resume_noirq(struct device *dev)
 	int ret;
 
 	/* Follow acpi_subsys_resume_noirq(). */
-	if (dev_pm_may_skip_resume(dev))
+	if (dev_pm_skip_resume(dev))
 		return 0;
 
 	ret = pm_generic_resume_noirq(dev);
diff --git a/drivers/acpi/device_pm.c b/drivers/acpi/device_pm.c
index 1b02d7dc7d34..8c2a091728a9 100644
--- a/drivers/acpi/device_pm.c
+++ b/drivers/acpi/device_pm.c
@@ -1127,7 +1127,7 @@ EXPORT_SYMBOL_GPL(acpi_subsys_suspend_noirq);
  */
 static int acpi_subsys_resume_noirq(struct device *dev)
 {
-	if (dev_pm_may_skip_resume(dev))
+	if (dev_pm_skip_resume(dev))
 		return 0;
 
 	return pm_generic_resume_noirq(dev);
@@ -1145,7 +1145,7 @@ static int acpi_subsys_resume_early(struct device *dev)
 {
 	int ret;
 
-	if (dev_pm_may_skip_resume(dev))
+	if (dev_pm_skip_resume(dev))
 		return 0;
 
 	ret = acpi_dev_resume(dev);
diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index 5adf0be6aa47..f98eced0f200 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -562,7 +562,7 @@ static void dpm_watchdog_clear(struct dpm_watchdog *wd)
 /*------------------------- Resume routines -------------------------*/
 
 /**
- * dev_pm_may_skip_resume - System-wide device resume optimization check.
+ * dev_pm_skip_resume - System-wide device resume optimization check.
  * @dev: Target device.
  *
  * Return:
@@ -572,7 +572,7 @@ static void dpm_watchdog_clear(struct dpm_watchdog *wd)
  * - The logical negation of %power.must_resume otherwise (that is, when the
  *   transition under way is RESUME).
  */
-bool dev_pm_may_skip_resume(struct device *dev)
+bool dev_pm_skip_resume(struct device *dev)
 {
 	if (pm_transition.event == PM_EVENT_RESTORE)
 		return false;
@@ -611,7 +611,7 @@ static int device_resume_noirq(struct device *dev, pm_message_t state, bool asyn
 	if (!dpm_wait_for_superior(dev, async))
 		goto Out;
 
-	skip_resume = dev_pm_may_skip_resume(dev);
+	skip_resume = dev_pm_skip_resume(dev);
 	/*
 	 * If the driver callback is skipped below or by the middle layer
 	 * callback and device_resume_early() also skips the driver callback for
@@ -797,7 +797,7 @@ static int device_resume_early(struct device *dev, pm_message_t state, bool asyn
 	if (callback)
 		goto Run;
 
-	if (dev_pm_may_skip_resume(dev))
+	if (dev_pm_skip_resume(dev))
 		goto Skip;
 
 	if (dev->driver && dev->driver->pm) {
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index ce220b1987df..decf82595340 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -891,7 +891,7 @@ static int pci_pm_resume_noirq(struct device *dev)
 	pci_power_t prev_state = pci_dev->current_state;
 	bool skip_bus_pm = pci_dev->skip_bus_pm;
 
-	if (dev_pm_may_skip_resume(dev))
+	if (dev_pm_skip_resume(dev))
 		return 0;
 
 	/*
@@ -920,7 +920,7 @@ static int pci_pm_resume_noirq(struct device *dev)
 
 static int pci_pm_resume_early(struct device *dev)
 {
-	if (dev_pm_may_skip_resume(dev))
+	if (dev_pm_skip_resume(dev))
 		return 0;
 
 	return pm_generic_resume_early(dev);
diff --git a/include/linux/pm.h b/include/linux/pm.h
index e057d1fa2469..d89b7099f241 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -758,7 +758,7 @@ extern int pm_generic_poweroff_late(struct device *dev);
 extern int pm_generic_poweroff(struct device *dev);
 extern void pm_generic_complete(struct device *dev);
 
-extern bool dev_pm_may_skip_resume(struct device *dev);
+extern bool dev_pm_skip_resume(struct device *dev);
 extern bool dev_pm_smart_suspend_and_suspended(struct device *dev);
 
 #else /* !CONFIG_PM_SLEEP */
-- 
2.16.4





^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v2 6/9] PM: sleep: core: Rename dev_pm_smart_suspend_and_suspended()
  2020-04-18 16:23 ` [PATCH v2 0/9] " Rafael J. Wysocki
                     ` (4 preceding siblings ...)
  2020-04-18 16:52   ` [PATCH v2 5/9] PM: sleep: core: Rename dev_pm_may_skip_resume() Rafael J. Wysocki
@ 2020-04-18 16:52   ` Rafael J. Wysocki
  2020-04-18 16:53   ` [PATCH v2 7/9] PM: sleep: core: Rename DPM_FLAG_NEVER_SKIP Rafael J. Wysocki
                     ` (6 subsequent siblings)
  12 siblings, 0 replies; 31+ messages in thread
From: Rafael J. Wysocki @ 2020-04-18 16:52 UTC (permalink / raw)
  To: Linux PM, Alan Stern
  Cc: Linux ACPI, Linux PCI, LKML, Bjorn Helgaas, Greg Kroah-Hartman,
	Mika Westerberg, Hans De Goede, Ulf Hansson

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

Because all callers of dev_pm_smart_suspend_and_suspended use it only
for checking whether or not to skip driver suspend callbacks for a
device, rename it to dev_pm_skip_suspend() in analogy with
dev_pm_skip_resume().

No functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

New patch.

---
 drivers/acpi/acpi_lpss.c          |  6 +++---
 drivers/acpi/device_pm.c          |  8 ++++----
 drivers/base/power/main.c         | 13 ++++++-------
 drivers/pci/hotplug/pciehp_core.c |  2 +-
 drivers/pci/pci-driver.c          |  8 ++++----
 include/linux/pm.h                |  2 +-
 6 files changed, 19 insertions(+), 20 deletions(-)

diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c
index 7632df1a5be3..5e2bfbcf526f 100644
--- a/drivers/acpi/acpi_lpss.c
+++ b/drivers/acpi/acpi_lpss.c
@@ -1041,7 +1041,7 @@ static int acpi_lpss_do_suspend_late(struct device *dev)
 {
 	int ret;
 
-	if (dev_pm_smart_suspend_and_suspended(dev))
+	if (dev_pm_skip_suspend(dev))
 		return 0;
 
 	ret = pm_generic_suspend_late(dev);
@@ -1169,7 +1169,7 @@ static int acpi_lpss_poweroff_late(struct device *dev)
 {
 	struct lpss_private_data *pdata = acpi_driver_data(ACPI_COMPANION(dev));
 
-	if (dev_pm_smart_suspend_and_suspended(dev))
+	if (dev_pm_skip_suspend(dev))
 		return 0;
 
 	if (pdata->dev_desc->resume_from_noirq)
@@ -1182,7 +1182,7 @@ static int acpi_lpss_poweroff_noirq(struct device *dev)
 {
 	struct lpss_private_data *pdata = acpi_driver_data(ACPI_COMPANION(dev));
 
-	if (dev_pm_smart_suspend_and_suspended(dev))
+	if (dev_pm_skip_suspend(dev))
 		return 0;
 
 	if (pdata->dev_desc->resume_from_noirq) {
diff --git a/drivers/acpi/device_pm.c b/drivers/acpi/device_pm.c
index 8c2a091728a9..ae234d731d42 100644
--- a/drivers/acpi/device_pm.c
+++ b/drivers/acpi/device_pm.c
@@ -1084,7 +1084,7 @@ int acpi_subsys_suspend_late(struct device *dev)
 {
 	int ret;
 
-	if (dev_pm_smart_suspend_and_suspended(dev))
+	if (dev_pm_skip_suspend(dev))
 		return 0;
 
 	ret = pm_generic_suspend_late(dev);
@@ -1100,7 +1100,7 @@ int acpi_subsys_suspend_noirq(struct device *dev)
 {
 	int ret;
 
-	if (dev_pm_smart_suspend_and_suspended(dev))
+	if (dev_pm_skip_suspend(dev))
 		return 0;
 
 	ret = pm_generic_suspend_noirq(dev);
@@ -1213,7 +1213,7 @@ static int acpi_subsys_poweroff_late(struct device *dev)
 {
 	int ret;
 
-	if (dev_pm_smart_suspend_and_suspended(dev))
+	if (dev_pm_skip_suspend(dev))
 		return 0;
 
 	ret = pm_generic_poweroff_late(dev);
@@ -1229,7 +1229,7 @@ static int acpi_subsys_poweroff_late(struct device *dev)
  */
 static int acpi_subsys_poweroff_noirq(struct device *dev)
 {
-	if (dev_pm_smart_suspend_and_suspended(dev))
+	if (dev_pm_skip_suspend(dev))
 		return 0;
 
 	return pm_generic_poweroff_noirq(dev);
diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index f98eced0f200..3170d93e29f9 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -567,8 +567,7 @@ static void dpm_watchdog_clear(struct dpm_watchdog *wd)
  *
  * Return:
  * - %false if the transition under way is RESTORE.
- * - The return value of dev_pm_smart_suspend_and_suspended() if the transition
- *   under way is THAW.
+ * - Return value of dev_pm_skip_suspend() if the transition under way is THAW.
  * - The logical negation of %power.must_resume otherwise (that is, when the
  *   transition under way is RESUME).
  */
@@ -578,7 +577,7 @@ bool dev_pm_skip_resume(struct device *dev)
 		return false;
 
 	if (pm_transition.event == PM_EVENT_THAW)
-		return dev_pm_smart_suspend_and_suspended(dev);
+		return dev_pm_skip_suspend(dev);
 
 	return !dev->power.must_resume;
 }
@@ -624,7 +623,7 @@ static int device_resume_noirq(struct device *dev, pm_message_t state, bool asyn
 	 */
 	if (skip_resume)
 		pm_runtime_set_suspended(dev);
-	else if (dev_pm_smart_suspend_and_suspended(dev))
+	else if (dev_pm_skip_suspend(dev))
 		pm_runtime_set_active(dev);
 
 	if (dev->pm_domain) {
@@ -1223,7 +1222,7 @@ static int __device_suspend_noirq(struct device *dev, pm_message_t state, bool a
 	if (callback)
 		goto Run;
 
-	if (dev_pm_smart_suspend_and_suspended(dev))
+	if (dev_pm_skip_suspend(dev))
 		goto Skip;
 
 	if (dev->driver && dev->driver->pm) {
@@ -1415,7 +1414,7 @@ static int __device_suspend_late(struct device *dev, pm_message_t state, bool as
 	if (callback)
 		goto Run;
 
-	if (dev_pm_smart_suspend_and_suspended(dev))
+	if (dev_pm_skip_suspend(dev))
 		goto Skip;
 
 	if (dev->driver && dev->driver->pm) {
@@ -2003,7 +2002,7 @@ void device_pm_check_callbacks(struct device *dev)
 	spin_unlock_irq(&dev->power.lock);
 }
 
-bool dev_pm_smart_suspend_and_suspended(struct device *dev)
+bool dev_pm_skip_suspend(struct device *dev)
 {
 	return dev_pm_test_driver_flags(dev, DPM_FLAG_SMART_SUSPEND) &&
 		pm_runtime_status_suspended(dev);
diff --git a/drivers/pci/hotplug/pciehp_core.c b/drivers/pci/hotplug/pciehp_core.c
index 312cc45c44c7..bf779f291f15 100644
--- a/drivers/pci/hotplug/pciehp_core.c
+++ b/drivers/pci/hotplug/pciehp_core.c
@@ -275,7 +275,7 @@ static int pciehp_suspend(struct pcie_device *dev)
 	 * If the port is already runtime suspended we can keep it that
 	 * way.
 	 */
-	if (dev_pm_smart_suspend_and_suspended(&dev->port->dev))
+	if (dev_pm_skip_suspend(&dev->port->dev))
 		return 0;
 
 	pciehp_disable_interrupt(dev);
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index decf82595340..da6510af1221 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -776,7 +776,7 @@ static int pci_pm_suspend(struct device *dev)
 
 static int pci_pm_suspend_late(struct device *dev)
 {
-	if (dev_pm_smart_suspend_and_suspended(dev))
+	if (dev_pm_skip_suspend(dev))
 		return 0;
 
 	pci_fixup_device(pci_fixup_suspend, to_pci_dev(dev));
@@ -789,7 +789,7 @@ static int pci_pm_suspend_noirq(struct device *dev)
 	struct pci_dev *pci_dev = to_pci_dev(dev);
 	const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
 
-	if (dev_pm_smart_suspend_and_suspended(dev))
+	if (dev_pm_skip_suspend(dev))
 		return 0;
 
 	if (pci_has_legacy_pm_support(pci_dev))
@@ -1126,7 +1126,7 @@ static int pci_pm_poweroff(struct device *dev)
 
 static int pci_pm_poweroff_late(struct device *dev)
 {
-	if (dev_pm_smart_suspend_and_suspended(dev))
+	if (dev_pm_skip_suspend(dev))
 		return 0;
 
 	pci_fixup_device(pci_fixup_suspend, to_pci_dev(dev));
@@ -1139,7 +1139,7 @@ static int pci_pm_poweroff_noirq(struct device *dev)
 	struct pci_dev *pci_dev = to_pci_dev(dev);
 	const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
 
-	if (dev_pm_smart_suspend_and_suspended(dev))
+	if (dev_pm_skip_suspend(dev))
 		return 0;
 
 	if (pci_has_legacy_pm_support(pci_dev))
diff --git a/include/linux/pm.h b/include/linux/pm.h
index d89b7099f241..8c59a7f0bcf4 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -759,7 +759,7 @@ extern int pm_generic_poweroff(struct device *dev);
 extern void pm_generic_complete(struct device *dev);
 
 extern bool dev_pm_skip_resume(struct device *dev);
-extern bool dev_pm_smart_suspend_and_suspended(struct device *dev);
+extern bool dev_pm_skip_suspend(struct device *dev);
 
 #else /* !CONFIG_PM_SLEEP */
 
-- 
2.16.4





^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v2 7/9] PM: sleep: core: Rename DPM_FLAG_NEVER_SKIP
  2020-04-18 16:23 ` [PATCH v2 0/9] " Rafael J. Wysocki
                     ` (5 preceding siblings ...)
  2020-04-18 16:52   ` [PATCH v2 6/9] PM: sleep: core: Rename dev_pm_smart_suspend_and_suspended() Rafael J. Wysocki
@ 2020-04-18 16:53   ` Rafael J. Wysocki
  2020-04-22 20:10     ` Alex Deucher
  2020-04-18 16:53   ` [PATCH v2 8/9] PM: sleep: core: Rename DPM_FLAG_LEAVE_SUSPENDED Rafael J. Wysocki
                     ` (5 subsequent siblings)
  12 siblings, 1 reply; 31+ messages in thread
From: Rafael J. Wysocki @ 2020-04-18 16:53 UTC (permalink / raw)
  To: Linux PM, Alan Stern
  Cc: Linux ACPI, Linux PCI, LKML, Bjorn Helgaas, Greg Kroah-Hartman,
	Mika Westerberg, Hans De Goede, Ulf Hansson, DRI-devel,
	Intel Graphics, intel-wired-lan

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

Rename DPM_FLAG_NEVER_SKIP to DPM_FLAG_NO_DIRECT_COMPLETE which
matches its purpose more closely.

No functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Acked-by: Bjorn Helgaas <bhelgaas@google.com> # for PCI parts
Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---

-> v2:
   * Rebased.
   * Added tags received so far.

---
 Documentation/driver-api/pm/devices.rst    |  6 +++---
 Documentation/power/pci.rst                | 10 +++++-----
 drivers/base/power/main.c                  |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c    |  2 +-
 drivers/gpu/drm/i915/intel_runtime_pm.c    |  2 +-
 drivers/gpu/drm/radeon/radeon_kms.c        |  2 +-
 drivers/misc/mei/pci-me.c                  |  2 +-
 drivers/misc/mei/pci-txe.c                 |  2 +-
 drivers/net/ethernet/intel/e1000e/netdev.c |  2 +-
 drivers/net/ethernet/intel/igb/igb_main.c  |  2 +-
 drivers/net/ethernet/intel/igc/igc_main.c  |  2 +-
 drivers/pci/pcie/portdrv_pci.c             |  2 +-
 include/linux/pm.h                         |  6 +++---
 13 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/Documentation/driver-api/pm/devices.rst b/Documentation/driver-api/pm/devices.rst
index f66c7b9126ea..4ace0eba4506 100644
--- a/Documentation/driver-api/pm/devices.rst
+++ b/Documentation/driver-api/pm/devices.rst
@@ -361,9 +361,9 @@ the phases are: ``prepare``, ``suspend``, ``suspend_late``, ``suspend_noirq``.
 	runtime PM disabled.
 
 	This feature also can be controlled by device drivers by using the
-	``DPM_FLAG_NEVER_SKIP`` and ``DPM_FLAG_SMART_PREPARE`` driver power
-	management flags.  [Typically, they are set at the time the driver is
-	probed against the device in question by passing them to the
+	``DPM_FLAG_NO_DIRECT_COMPLETE`` and ``DPM_FLAG_SMART_PREPARE`` driver
+	power management flags.  [Typically, they are set at the time the driver
+	is probed against the device in question by passing them to the
 	:c:func:`dev_pm_set_driver_flags` helper function.]  If the first of
 	these flags is set, the PM core will not apply the direct-complete
 	procedure described above to the given device and, consequenty, to any
diff --git a/Documentation/power/pci.rst b/Documentation/power/pci.rst
index aa1c7fce6cd0..9e1408121bea 100644
--- a/Documentation/power/pci.rst
+++ b/Documentation/power/pci.rst
@@ -1004,11 +1004,11 @@ including the PCI bus type.  The flags should be set once at the driver probe
 time with the help of the dev_pm_set_driver_flags() function and they should not
 be updated directly afterwards.
 
-The DPM_FLAG_NEVER_SKIP flag prevents the PM core from using the direct-complete
-mechanism allowing device suspend/resume callbacks to be skipped if the device
-is in runtime suspend when the system suspend starts.  That also affects all of
-the ancestors of the device, so this flag should only be used if absolutely
-necessary.
+The DPM_FLAG_NO_DIRECT_COMPLETE flag prevents the PM core from using the
+direct-complete mechanism allowing device suspend/resume callbacks to be skipped
+if the device is in runtime suspend when the system suspend starts.  That also
+affects all of the ancestors of the device, so this flag should only be used if
+absolutely necessary.
 
 The DPM_FLAG_SMART_PREPARE flag instructs the PCI bus type to only return a
 positive value from pci_pm_prepare() if the ->prepare callback provided by the
diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index 3170d93e29f9..dbc1e5e7346b 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -1844,7 +1844,7 @@ static int device_prepare(struct device *dev, pm_message_t state)
 	spin_lock_irq(&dev->power.lock);
 	dev->power.direct_complete = state.event == PM_EVENT_SUSPEND &&
 		(ret > 0 || dev->power.no_pm_callbacks) &&
-		!dev_pm_test_driver_flags(dev, DPM_FLAG_NEVER_SKIP);
+		!dev_pm_test_driver_flags(dev, DPM_FLAG_NO_DIRECT_COMPLETE);
 	spin_unlock_irq(&dev->power.lock);
 	return 0;
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index fd1dc3236eca..a9086ea1ab60 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -191,7 +191,7 @@ int amdgpu_driver_load_kms(struct drm_device *dev, unsigned long flags)
 	}
 
 	if (adev->runpm) {
-		dev_pm_set_driver_flags(dev->dev, DPM_FLAG_NEVER_SKIP);
+		dev_pm_set_driver_flags(dev->dev, DPM_FLAG_NO_DIRECT_COMPLETE);
 		pm_runtime_use_autosuspend(dev->dev);
 		pm_runtime_set_autosuspend_delay(dev->dev, 5000);
 		pm_runtime_set_active(dev->dev);
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index ad719c9602af..9cb2d7548daa 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -549,7 +549,7 @@ void intel_runtime_pm_enable(struct intel_runtime_pm *rpm)
 	 * becaue the HDA driver may require us to enable the audio power
 	 * domain during system suspend.
 	 */
-	dev_pm_set_driver_flags(kdev, DPM_FLAG_NEVER_SKIP);
+	dev_pm_set_driver_flags(kdev, DPM_FLAG_NO_DIRECT_COMPLETE);
 
 	pm_runtime_set_autosuspend_delay(kdev, 10000); /* 10s */
 	pm_runtime_mark_last_busy(kdev);
diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c
index 58176db85952..372962358a18 100644
--- a/drivers/gpu/drm/radeon/radeon_kms.c
+++ b/drivers/gpu/drm/radeon/radeon_kms.c
@@ -158,7 +158,7 @@ int radeon_driver_load_kms(struct drm_device *dev, unsigned long flags)
 	}
 
 	if (radeon_is_px(dev)) {
-		dev_pm_set_driver_flags(dev->dev, DPM_FLAG_NEVER_SKIP);
+		dev_pm_set_driver_flags(dev->dev, DPM_FLAG_NO_DIRECT_COMPLETE);
 		pm_runtime_use_autosuspend(dev->dev);
 		pm_runtime_set_autosuspend_delay(dev->dev, 5000);
 		pm_runtime_set_active(dev->dev);
diff --git a/drivers/misc/mei/pci-me.c b/drivers/misc/mei/pci-me.c
index 3d21c38e2dbb..53f16f3bd091 100644
--- a/drivers/misc/mei/pci-me.c
+++ b/drivers/misc/mei/pci-me.c
@@ -240,7 +240,7 @@ static int mei_me_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	 * MEI requires to resume from runtime suspend mode
 	 * in order to perform link reset flow upon system suspend.
 	 */
-	dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_NEVER_SKIP);
+	dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_NO_DIRECT_COMPLETE);
 
 	/*
 	 * ME maps runtime suspend/resume to D0i states,
diff --git a/drivers/misc/mei/pci-txe.c b/drivers/misc/mei/pci-txe.c
index beacf2a2f2b5..4bf26ce61044 100644
--- a/drivers/misc/mei/pci-txe.c
+++ b/drivers/misc/mei/pci-txe.c
@@ -128,7 +128,7 @@ static int mei_txe_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	 * MEI requires to resume from runtime suspend mode
 	 * in order to perform link reset flow upon system suspend.
 	 */
-	dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_NEVER_SKIP);
+	dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_NO_DIRECT_COMPLETE);
 
 	/*
 	 * TXE maps runtime suspend/resume to own power gating states,
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index 177c6da80c57..2730b1c7dddb 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -7549,7 +7549,7 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	e1000_print_device_info(adapter);
 
-	dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_NEVER_SKIP);
+	dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_NO_DIRECT_COMPLETE);
 
 	if (pci_dev_run_wake(pdev) && hw->mac.type < e1000_pch_cnp)
 		pm_runtime_put_noidle(&pdev->dev);
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index b46bff8fe056..8bb3db2cbd41 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -3445,7 +3445,7 @@ static int igb_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		}
 	}
 
-	dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_NEVER_SKIP);
+	dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_NO_DIRECT_COMPLETE);
 
 	pm_runtime_put_noidle(&pdev->dev);
 	return 0;
diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index 69fa1ce1f927..59fc0097438f 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -4825,7 +4825,7 @@ static int igc_probe(struct pci_dev *pdev,
 	pcie_print_link_status(pdev);
 	netdev_info(netdev, "MAC: %pM\n", netdev->dev_addr);
 
-	dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_NEVER_SKIP);
+	dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_NO_DIRECT_COMPLETE);
 
 	pm_runtime_put_noidle(&pdev->dev);
 
diff --git a/drivers/pci/pcie/portdrv_pci.c b/drivers/pci/pcie/portdrv_pci.c
index 160d67c59310..3acf151ae015 100644
--- a/drivers/pci/pcie/portdrv_pci.c
+++ b/drivers/pci/pcie/portdrv_pci.c
@@ -115,7 +115,7 @@ static int pcie_portdrv_probe(struct pci_dev *dev,
 
 	pci_save_state(dev);
 
-	dev_pm_set_driver_flags(&dev->dev, DPM_FLAG_NEVER_SKIP |
+	dev_pm_set_driver_flags(&dev->dev, DPM_FLAG_NO_DIRECT_COMPLETE |
 					   DPM_FLAG_SMART_SUSPEND);
 
 	if (pci_bridge_d3_possible(dev)) {
diff --git a/include/linux/pm.h b/include/linux/pm.h
index 8c59a7f0bcf4..cdb8fbd6ab18 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -544,7 +544,7 @@ struct pm_subsys_data {
  * These flags can be set by device drivers at the probe time.  They need not be
  * cleared by the drivers as the driver core will take care of that.
  *
- * NEVER_SKIP: Do not skip all system suspend/resume callbacks for the device.
+ * NO_DIRECT_COMPLETE: Do not apply direct-complete optimization to the device.
  * SMART_PREPARE: Check the return value of the driver's ->prepare callback.
  * SMART_SUSPEND: No need to resume the device from runtime suspend.
  * LEAVE_SUSPENDED: Avoid resuming the device during system resume if possible.
@@ -554,7 +554,7 @@ struct pm_subsys_data {
  * their ->prepare callbacks if the driver's ->prepare callback returns 0 (in
  * other words, the system suspend/resume callbacks can only be skipped for the
  * device if its driver doesn't object against that).  This flag has no effect
- * if NEVER_SKIP is set.
+ * if NO_DIRECT_COMPLETE is set.
  *
  * Setting SMART_SUSPEND instructs bus types and PM domains which may want to
  * runtime resume the device upfront during system suspend that doing so is not
@@ -565,7 +565,7 @@ struct pm_subsys_data {
  * Setting LEAVE_SUSPENDED informs the PM core and middle-layer code that the
  * driver prefers the device to be left in suspend after system resume.
  */
-#define DPM_FLAG_NEVER_SKIP		BIT(0)
+#define DPM_FLAG_NO_DIRECT_COMPLETE	BIT(0)
 #define DPM_FLAG_SMART_PREPARE		BIT(1)
 #define DPM_FLAG_SMART_SUSPEND		BIT(2)
 #define DPM_FLAG_LEAVE_SUSPENDED	BIT(3)
-- 
2.16.4





^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v2 8/9] PM: sleep: core: Rename DPM_FLAG_LEAVE_SUSPENDED
  2020-04-18 16:23 ` [PATCH v2 0/9] " Rafael J. Wysocki
                     ` (6 preceding siblings ...)
  2020-04-18 16:53   ` [PATCH v2 7/9] PM: sleep: core: Rename DPM_FLAG_NEVER_SKIP Rafael J. Wysocki
@ 2020-04-18 16:53   ` Rafael J. Wysocki
  2020-04-18 16:55   ` [PATCH v2 9/9] Documentation: PM: sleep: Update driver flags documentation Rafael J. Wysocki
                     ` (4 subsequent siblings)
  12 siblings, 0 replies; 31+ messages in thread
From: Rafael J. Wysocki @ 2020-04-18 16:53 UTC (permalink / raw)
  To: Linux PM, Alan Stern
  Cc: Linux ACPI, Linux PCI, LKML, Bjorn Helgaas, Greg Kroah-Hartman,
	Mika Westerberg, Hans De Goede, Ulf Hansson, Linux I2C

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

Rename DPM_FLAG_LEAVE_SUSPENDED to DPM_FLAG_MAY_SKIP_RESUME which
matches its purpose more closely.

No functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Acked-by: Wolfram Sang <wsa@the-dreams.de> # for I2C
---

-> v2:
   * Rebased.
   * Added tags received so far.

---
 Documentation/driver-api/pm/devices.rst     | 4 ++--
 Documentation/power/pci.rst                 | 2 +-
 drivers/acpi/acpi_tad.c                     | 2 +-
 drivers/base/power/main.c                   | 2 +-
 drivers/i2c/busses/i2c-designware-platdrv.c | 2 +-
 include/linux/pm.h                          | 6 +++---
 6 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/Documentation/driver-api/pm/devices.rst b/Documentation/driver-api/pm/devices.rst
index 4ace0eba4506..f342c7549b4c 100644
--- a/Documentation/driver-api/pm/devices.rst
+++ b/Documentation/driver-api/pm/devices.rst
@@ -803,7 +803,7 @@ general.]
 However, it often is desirable to leave devices in suspend after system
 transitions to the working state, especially if those devices had been in
 runtime suspend before the preceding system-wide suspend (or analogous)
-transition.  Device drivers can use the ``DPM_FLAG_LEAVE_SUSPENDED`` flag to
+transition.  Device drivers can use the ``DPM_FLAG_MAY_SKIP_RESUME`` flag to
 indicate to the PM core (and middle-layer code) that they prefer the specific
 devices handled by them to be left suspended and they have no problems with
 skipping their system-wide resume callbacks for this reason.  Whether or not the
@@ -825,7 +825,7 @@ device really can be left in suspend.
 
 For devices whose "noirq", "late" and "early" driver callbacks are invoked
 directly by the PM core, all of the system-wide resume callbacks are skipped if
-``DPM_FLAG_LEAVE_SUSPENDED`` is set and the device is in runtime suspend during
+``DPM_FLAG_MAY_SKIP_RESUME`` is set and the device is in runtime suspend during
 the ``suspend_noirq`` (or analogous) phase or the transition under way is a
 proper system suspend (rather than anything related to hibernation) and the
 device's wakeup settings are suitable for runtime PM (that is, it cannot
diff --git a/Documentation/power/pci.rst b/Documentation/power/pci.rst
index 9e1408121bea..f09b382b4621 100644
--- a/Documentation/power/pci.rst
+++ b/Documentation/power/pci.rst
@@ -1029,7 +1029,7 @@ into D0 going forward), but if it is in runtime suspend in pci_pm_thaw_noirq(),
 the function will set the power.direct_complete flag for it (to make the PM core
 skip the subsequent "thaw" callbacks for it) and return.
 
-Setting the DPM_FLAG_LEAVE_SUSPENDED flag means that the driver prefers the
+Setting the DPM_FLAG_MAY_SKIP_RESUME flag means that the driver prefers the
 device to be left in suspend after system-wide transitions to the working state.
 This flag is checked by the PM core, but the PCI bus type informs the PM core
 which devices may be left in suspend from its perspective (that happens during
diff --git a/drivers/acpi/acpi_tad.c b/drivers/acpi/acpi_tad.c
index 33a4bcdaa4d7..7d45cce0c3c1 100644
--- a/drivers/acpi/acpi_tad.c
+++ b/drivers/acpi/acpi_tad.c
@@ -624,7 +624,7 @@ static int acpi_tad_probe(struct platform_device *pdev)
 	 */
 	device_init_wakeup(dev, true);
 	dev_pm_set_driver_flags(dev, DPM_FLAG_SMART_SUSPEND |
-				     DPM_FLAG_LEAVE_SUSPENDED);
+				     DPM_FLAG_MAY_SKIP_RESUME);
 	/*
 	 * The platform bus type layer tells the ACPI PM domain powers up the
 	 * device, so set the runtime PM status of it to "active".
diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index dbc1e5e7346b..aaa4aaf41d27 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -1247,7 +1247,7 @@ static int __device_suspend_noirq(struct device *dev, pm_message_t state, bool a
 	 * to be skipped.
 	 */
 	if (atomic_read(&dev->power.usage_count) > 1 ||
-	    !(dev_pm_test_driver_flags(dev, DPM_FLAG_LEAVE_SUSPENDED) &&
+	    !(dev_pm_test_driver_flags(dev, DPM_FLAG_MAY_SKIP_RESUME) &&
 	      dev->power.may_skip_resume))
 		dev->power.must_resume = true;
 
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index c98befe2a92e..b6270e69f853 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -357,7 +357,7 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
 	dev_pm_set_driver_flags(&pdev->dev,
 				DPM_FLAG_SMART_PREPARE |
 				DPM_FLAG_SMART_SUSPEND |
-				DPM_FLAG_LEAVE_SUSPENDED);
+				DPM_FLAG_MAY_SKIP_RESUME);
 
 	/* The code below assumes runtime PM to be disabled. */
 	WARN_ON(pm_runtime_enabled(&pdev->dev));
diff --git a/include/linux/pm.h b/include/linux/pm.h
index cdb8fbd6ab18..35796fc49e7a 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -547,7 +547,7 @@ struct pm_subsys_data {
  * NO_DIRECT_COMPLETE: Do not apply direct-complete optimization to the device.
  * SMART_PREPARE: Check the return value of the driver's ->prepare callback.
  * SMART_SUSPEND: No need to resume the device from runtime suspend.
- * LEAVE_SUSPENDED: Avoid resuming the device during system resume if possible.
+ * MAY_SKIP_RESUME: Avoid resuming the device during system resume if possible.
  *
  * Setting SMART_PREPARE instructs bus types and PM domains which may want
  * system suspend/resume callbacks to be skipped for the device to return 0 from
@@ -562,13 +562,13 @@ struct pm_subsys_data {
  * invocations of the ->suspend_late and ->suspend_noirq callbacks provided by
  * the driver if they decide to leave the device in runtime suspend.
  *
- * Setting LEAVE_SUSPENDED informs the PM core and middle-layer code that the
+ * Setting MAY_SKIP_RESUME informs the PM core and middle-layer code that the
  * driver prefers the device to be left in suspend after system resume.
  */
 #define DPM_FLAG_NO_DIRECT_COMPLETE	BIT(0)
 #define DPM_FLAG_SMART_PREPARE		BIT(1)
 #define DPM_FLAG_SMART_SUSPEND		BIT(2)
-#define DPM_FLAG_LEAVE_SUSPENDED	BIT(3)
+#define DPM_FLAG_MAY_SKIP_RESUME	BIT(3)
 
 struct dev_pm_info {
 	pm_message_t		power_state;
-- 
2.16.4





^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v2 9/9] Documentation: PM: sleep: Update driver flags documentation
  2020-04-18 16:23 ` [PATCH v2 0/9] " Rafael J. Wysocki
                     ` (7 preceding siblings ...)
  2020-04-18 16:53   ` [PATCH v2 8/9] PM: sleep: core: Rename DPM_FLAG_LEAVE_SUSPENDED Rafael J. Wysocki
@ 2020-04-18 16:55   ` Rafael J. Wysocki
  2020-04-18 18:00   ` [PATCH v2 0/9] PM: sleep: core: Rearrange the handling of driver power management flags Alan Stern
                     ` (3 subsequent siblings)
  12 siblings, 0 replies; 31+ messages in thread
From: Rafael J. Wysocki @ 2020-04-18 16:55 UTC (permalink / raw)
  To: Linux PM, Alan Stern
  Cc: Linux ACPI, Linux PCI, LKML, Bjorn Helgaas, Greg Kroah-Hartman,
	Mika Westerberg, Hans De Goede, Ulf Hansson

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

Update the documentation of the driver flags for system-wide power
management to reflect the current code flows and be more consistent.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

-> v2:
   * Updated to reflect changes new in v2.

---
 Documentation/driver-api/pm/devices.rst | 139 +++++++++++++++++++++-----------
 Documentation/power/pci.rst             |  43 +++++-----
 include/linux/pm.h                      |  24 ++----
 3 files changed, 119 insertions(+), 87 deletions(-)

diff --git a/Documentation/driver-api/pm/devices.rst b/Documentation/driver-api/pm/devices.rst
index f342c7549b4c..782cb37073a3 100644
--- a/Documentation/driver-api/pm/devices.rst
+++ b/Documentation/driver-api/pm/devices.rst
@@ -772,62 +772,107 @@ the state of devices (possibly except for resuming them from runtime suspend)
 from their ``->prepare`` and ``->suspend`` callbacks (or equivalent) *before*
 invoking device drivers' ``->suspend`` callbacks (or equivalent).
 
+.. _smart_suspend_flag:
+
+The ``DPM_FLAG_SMART_SUSPEND`` Driver Flag
+------------------------------------------
+
 Some bus types and PM domains have a policy to resume all devices from runtime
 suspend upfront in their ``->suspend`` callbacks, but that may not be really
 necessary if the driver of the device can cope with runtime-suspended devices.
 The driver can indicate that by setting ``DPM_FLAG_SMART_SUSPEND`` in
-:c:member:`power.driver_flags` at the probe time, by passing it to the
-:c:func:`dev_pm_set_driver_flags` helper.  That also may cause middle-layer code
+:c:member:`power.driver_flags` at the probe time with the help of the
+:c:func:`dev_pm_set_driver_flags` helper routine.
+
+However, setting that flag also causes the PM core and middle-layer code
 (bus types, PM domains etc.) to skip the ``->suspend_late`` and
 ``->suspend_noirq`` callbacks provided by the driver if the device remains in
-runtime suspend at the beginning of the ``suspend_late`` phase of system-wide
-suspend (or in the ``poweroff_late`` phase of hibernation), when runtime PM
-has been disabled for it, under the assumption that its state should not change
-after that point until the system-wide transition is over (the PM core itself
-does that for devices whose "noirq", "late" and "early" system-wide PM callbacks
-are executed directly by it).  If that happens, the driver's system-wide resume
-callbacks, if present, may still be invoked during the subsequent system-wide
-resume transition and the device's runtime power management status may be set
-to "active" before enabling runtime PM for it, so the driver must be prepared to
-cope with the invocation of its system-wide resume callbacks back-to-back with
-its ``->runtime_suspend`` one (without the intervening ``->runtime_resume`` and
-so on) and the final state of the device must reflect the "active" runtime PM
-status in that case.
+runtime suspend during the ``suspend_late`` phase of system-wide suspend (or
+during the ``poweroff_late`` or ``freeze_late`` phase of hibernation),
+after runtime PM was disabled for it.  [Without doing that, the same driver
+callback might be executed twice in a row for the same device, which would not
+be valid in general.]  If the middle-layer system-wide PM callbacks are present
+for the device, they are responsible for doing the above, and the PM core takes
+care of it otherwise.
+
+In addition, with ``DPM_FLAG_SMART_SUSPEND`` set, the driver's ``->thaw_late``
+and ``->thaw_noirq`` callbacks are skipped if the device remained in runtime
+suspend during the preceding "freeze" transition related to hibernation.
+Again, if the middle-layer callbacks are present for the device, they are
+responsible for doing that, or the PM core takes care of it otherwise.
+
+
+The ``DPM_FLAG_MAY_SKIP_RESUME`` Driver Flag
+--------------------------------------------
 
 During system-wide resume from a sleep state it's easiest to put devices into
 the full-power state, as explained in :file:`Documentation/power/runtime_pm.rst`.
 [Refer to that document for more information regarding this particular issue as
 well as for information on the device runtime power management framework in
-general.]
-
-However, it often is desirable to leave devices in suspend after system
-transitions to the working state, especially if those devices had been in
+general.]  However, it often is desirable to leave devices in suspend after
+system transitions to the working state, especially if those devices had been in
 runtime suspend before the preceding system-wide suspend (or analogous)
-transition.  Device drivers can use the ``DPM_FLAG_MAY_SKIP_RESUME`` flag to
-indicate to the PM core (and middle-layer code) that they prefer the specific
-devices handled by them to be left suspended and they have no problems with
-skipping their system-wide resume callbacks for this reason.  Whether or not the
-devices will actually be left in suspend may depend on their state before the
-given system suspend-resume cycle and on the type of the system transition under
-way.  In particular, devices are not left suspended if that transition is a
-restore from hibernation, as device states are not guaranteed to be reflected
-by the information stored in the hibernation image in that case.
-
-The middle-layer code involved in the handling of the device is expected to
-indicate to the PM core if the device may be left in suspend by setting its
-:c:member:`power.may_skip_resume` status bit which is checked by the PM core
-during the "noirq" phase of the preceding system-wide suspend (or analogous)
-transition.  The middle layer is then responsible for handling the device as
-appropriate in its "noirq" resume callback, which is executed regardless of
-whether or not the device is left suspended, but the other resume callbacks
-(except for ``->complete``) will be skipped automatically by the PM core if the
-device really can be left in suspend.
-
-For devices whose "noirq", "late" and "early" driver callbacks are invoked
-directly by the PM core, all of the system-wide resume callbacks are skipped if
-``DPM_FLAG_MAY_SKIP_RESUME`` is set and the device is in runtime suspend during
-the ``suspend_noirq`` (or analogous) phase or the transition under way is a
-proper system suspend (rather than anything related to hibernation) and the
-device's wakeup settings are suitable for runtime PM (that is, it cannot
-generate wakeup signals at all or it is allowed to wake up the system from
-sleep).
+transition.
+
+To that end, device drivers can use the ``DPM_FLAG_MAY_SKIP_RESUME`` flag to
+indicate to the PM core and middle-layer code that they allow their "noirq" and
+"early" resume callbacks to be skipped if the device can be left in suspend
+after system-wide PM transitions to the working state.  Whether or not that is
+the case generally depends on the state of the device before the given system
+suspend-resume cycle and on the type of the system transition under way.
+In particular, the "restore" and "thaw" transitions related to hibernation are
+not affected by ``DPM_FLAG_MAY_SKIP_RESUME`` at all.  [All devices are always
+resumed during the "restore" transition and whether or not any driver callbacks
+are skipped during the "freeze" transition depends whether or not the
+``DPM_FLAG_SMART_SUSPEND`` flag is set (see `above <smart_suspend_flag_>`_).]
+
+The ``DPM_FLAG_MAY_SKIP_RESUME`` flag is taken into account in combination with
+the :c:member:`power.may_skip_resume` status bit set by the PM core during the
+"suspend" phase of suspend-type transitions.  If the driver or the middle layer
+has a reason to prevent the driver's "noirq" and "early" resume callbacks from
+being skipped during the subsequent resume transition of the system, it should
+clear :c:member:`power.may_skip_resume` in its ``->suspend``, ``->suspend_late``
+or ``->suspend_noirq`` callback.  [Note that the drivers setting
+``DPM_FLAG_SMART_SUSPEND`` need to clear :c:member:`power.may_skip_resume` in
+their ``->suspend`` callback in case the other two are skipped.]
+
+Setting the :c:member:`power.may_skip_resume` status bit along with the
+``DPM_FLAG_MAY_SKIP_RESUME`` flag is necessary, but generally not sufficient,
+for the driver's "noirq" and "early" resume callbacks to be skipped.  Whether or
+not they should be skipped can be determined by evaluating the
+:c:func:`dev_pm_skip_resume` helper function.
+
+If that function returns ``true``, the driver's "noirq" and "early" resume
+callbacks should be skipped and the device's runtime PM status will be set to
+"suspended" by the PM core.  Otherwise, if the device was runtime-suspended
+during the preceding system-wide suspend transition and
+``DPM_FLAG_SMART_SUSPEND`` is set for it, its runtime PM status will be set to
+"active" by the PM core.  [Hence, the drivers that do not set
+``DPM_FLAG_SMART_SUSPEND`` should not expect the runtime PM status of their
+devices to be changed from "suspended" to "active" by the PM core during
+system-wide resume-type transitions.]
+
+If the ``DPM_FLAG_MAY_SKIP_RESUME`` flag is not set for a device, but
+``DPM_FLAG_SMART_SUSPEND`` is set and the driver's "late" and "noirq" suspend
+callbacks are skipped, its system-wide "noirq" and "early" resume callbacks, if
+present, are invoked as usual and the device's runtime PM status is set to
+"active" by the PM core before enabling runtime PM for it.  In that case, the
+driver must be prepared to cope with the invocation of its system-wide resume
+callbacks back-to-back with its ``->runtime_suspend`` one (without the
+intervening ``->runtime_resume`` and system-wide suspend callbacks) and the
+final state of the device must reflect the "active" runtime PM status in that
+case.  [Note that this is not a problem at all if the driver's
+``->suspend_late`` callback pointer points to the same function as its
+``->runtime_suspend`` one and its ``->resume_early`` callback pointer points to
+the same function as the ``->runtime_resume`` one, while none of the other
+system-wide suspend-resume callbacks of the driver are present, for example.]
+
+Likewise, if ``DPM_FLAG_MAY_SKIP_RESUME`` is set for a device, its driver's
+system-wide "noirq" and "early" resume callbacks may be skipped while its "late"
+and "noirq" suspend callbacks may have been executed (in principle, regardless
+of whether or not ``DPM_FLAG_SMART_SUSPEND`` is set).  In that case, the driver
+needs to be able to cope with the invocation of its ``->runtime_resume``
+callback back-to-back with its "late" and "noirq" suspend ones.  [For instance,
+that is not a concern if the driver sets both ``DPM_FLAG_SMART_SUSPEND`` and
+``DPM_FLAG_MAY_SKIP_RESUME`` and uses the same pair of suspend/resume callback
+functions for runtime PM and system-wide suspend/resume.]
diff --git a/Documentation/power/pci.rst b/Documentation/power/pci.rst
index f09b382b4621..1831e431f725 100644
--- a/Documentation/power/pci.rst
+++ b/Documentation/power/pci.rst
@@ -1010,32 +1010,33 @@ if the device is in runtime suspend when the system suspend starts.  That also
 affects all of the ancestors of the device, so this flag should only be used if
 absolutely necessary.
 
-The DPM_FLAG_SMART_PREPARE flag instructs the PCI bus type to only return a
-positive value from pci_pm_prepare() if the ->prepare callback provided by the
+The DPM_FLAG_SMART_PREPARE flag causes the PCI bus type to return a positive
+value from pci_pm_prepare() only if the ->prepare callback provided by the
 driver of the device returns a positive value.  That allows the driver to opt
-out from using the direct-complete mechanism dynamically.
+out from using the direct-complete mechanism dynamically (whereas setting
+DPM_FLAG_NO_DIRECT_COMPLETE means permanent opt-out).
 
 The DPM_FLAG_SMART_SUSPEND flag tells the PCI bus type that from the driver's
 perspective the device can be safely left in runtime suspend during system
 suspend.  That causes pci_pm_suspend(), pci_pm_freeze() and pci_pm_poweroff()
-to skip resuming the device from runtime suspend unless there are PCI-specific
-reasons for doing that.  Also, it causes pci_pm_suspend_late/noirq(),
-pci_pm_freeze_late/noirq() and pci_pm_poweroff_late/noirq() to return early
-if the device remains in runtime suspend in the beginning of the "late" phase
-of the system-wide transition under way.  Moreover, if the device is in
-runtime suspend in pci_pm_resume_noirq() or pci_pm_restore_noirq(), its runtime
-power management status will be changed to "active" (as it is going to be put
-into D0 going forward), but if it is in runtime suspend in pci_pm_thaw_noirq(),
-the function will set the power.direct_complete flag for it (to make the PM core
-skip the subsequent "thaw" callbacks for it) and return.
-
-Setting the DPM_FLAG_MAY_SKIP_RESUME flag means that the driver prefers the
-device to be left in suspend after system-wide transitions to the working state.
-This flag is checked by the PM core, but the PCI bus type informs the PM core
-which devices may be left in suspend from its perspective (that happens during
-the "noirq" phase of system-wide suspend and analogous transitions) and next it
-uses the dev_pm_skip_resume() helper to decide whether or not to return from
-pci_pm_resume_noirq() and pci_pm_resume_early() upfront.
+to avoid resuming the device from runtime suspend unless there are PCI-specific
+reasons for doing that.  Also, it causes pci_pm_suspend_late/noirq() and
+pci_pm_poweroff_late/noirq() to return early if the device remains in runtime
+suspend during the "late" phase of the system-wide transition under way.
+Moreover, if the device is in runtime suspend in pci_pm_resume_noirq() or
+pci_pm_restore_noirq(), its runtime PM status will be changed to "active" (as it
+is going to be put into D0 going forward).
+
+Setting the DPM_FLAG_MAY_SKIP_RESUME flag means that the driver allows its
+"noirq" and "early" resume callbacks to be skipped if the device can be left
+in suspend after a system-wide transition into the working state.  This flag is
+taken into consideration by the PM core along with the power.may_skip_resume
+status bit of the device which is set by pci_pm_suspend_noirq() in certain
+situations.  If the PM core determines that the driver's "noirq" and "early"
+resume callbacks should be skipped, the dev_pm_skip_resume() helper function
+will return "true" and that will cause pci_pm_resume_noirq() and
+pci_pm_resume_early() to return upfront without touching the device and
+executing the driver callbacks.
 
 3.2. Device Runtime Power Management
 ------------------------------------
diff --git a/include/linux/pm.h b/include/linux/pm.h
index 35796fc49e7a..121c104a4090 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -545,25 +545,11 @@ struct pm_subsys_data {
  * cleared by the drivers as the driver core will take care of that.
  *
  * NO_DIRECT_COMPLETE: Do not apply direct-complete optimization to the device.
- * SMART_PREPARE: Check the return value of the driver's ->prepare callback.
- * SMART_SUSPEND: No need to resume the device from runtime suspend.
- * MAY_SKIP_RESUME: Avoid resuming the device during system resume if possible.
- *
- * Setting SMART_PREPARE instructs bus types and PM domains which may want
- * system suspend/resume callbacks to be skipped for the device to return 0 from
- * their ->prepare callbacks if the driver's ->prepare callback returns 0 (in
- * other words, the system suspend/resume callbacks can only be skipped for the
- * device if its driver doesn't object against that).  This flag has no effect
- * if NO_DIRECT_COMPLETE is set.
- *
- * Setting SMART_SUSPEND instructs bus types and PM domains which may want to
- * runtime resume the device upfront during system suspend that doing so is not
- * necessary from the driver's perspective.  It also may cause them to skip
- * invocations of the ->suspend_late and ->suspend_noirq callbacks provided by
- * the driver if they decide to leave the device in runtime suspend.
- *
- * Setting MAY_SKIP_RESUME informs the PM core and middle-layer code that the
- * driver prefers the device to be left in suspend after system resume.
+ * SMART_PREPARE: Take the driver ->prepare callback return value into account.
+ * SMART_SUSPEND: Avoid resuming the device from runtime suspend.
+ * MAY_SKIP_RESUME: Allow driver "noirq" and "early" callbacks to be skipped.
+ *
+ * See Documentation/driver-api/pm/devices.rst for details.
  */
 #define DPM_FLAG_NO_DIRECT_COMPLETE	BIT(0)
 #define DPM_FLAG_SMART_PREPARE		BIT(1)
-- 
2.16.4





^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v2 0/9] PM: sleep: core: Rearrange the handling of driver power management flags
  2020-04-18 16:23 ` [PATCH v2 0/9] " Rafael J. Wysocki
                     ` (8 preceding siblings ...)
  2020-04-18 16:55   ` [PATCH v2 9/9] Documentation: PM: sleep: Update driver flags documentation Rafael J. Wysocki
@ 2020-04-18 18:00   ` Alan Stern
  2020-04-18 18:08     ` Rafael J. Wysocki
  2020-04-19 14:43   ` Alan Stern
                     ` (2 subsequent siblings)
  12 siblings, 1 reply; 31+ messages in thread
From: Alan Stern @ 2020-04-18 18:00 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Linux PM, Linux ACPI, Linux PCI, LKML, Bjorn Helgaas,
	Greg Kroah-Hartman, Mika Westerberg, Hans De Goede, Ulf Hansson

On Sat, 18 Apr 2020, Rafael J. Wysocki wrote:

> Hi,
> 
> This is an update including some fixes and extra patches based on the
> continuation of the discussion [1].

I haven't checked the updates in detail yet.  However, it seems that 
dev_pm_skip_suspend() and dev_pm_skip_resume() should be EXPORTed, 
since they are intended to be used by subsystems, which may be in 
modules.

Alan Stern


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v2 0/9] PM: sleep: core: Rearrange the handling of driver power management flags
  2020-04-18 18:00   ` [PATCH v2 0/9] PM: sleep: core: Rearrange the handling of driver power management flags Alan Stern
@ 2020-04-18 18:08     ` Rafael J. Wysocki
  2020-04-18 19:41       ` Alan Stern
  0 siblings, 1 reply; 31+ messages in thread
From: Rafael J. Wysocki @ 2020-04-18 18:08 UTC (permalink / raw)
  To: Alan Stern
  Cc: Rafael J. Wysocki, Linux PM, Linux ACPI, Linux PCI, LKML,
	Bjorn Helgaas, Greg Kroah-Hartman, Mika Westerberg,
	Hans De Goede, Ulf Hansson

On Sat, Apr 18, 2020 at 8:00 PM Alan Stern <stern@rowland.harvard.edu> wrote:
>
> On Sat, 18 Apr 2020, Rafael J. Wysocki wrote:
>
> > Hi,
> >
> > This is an update including some fixes and extra patches based on the
> > continuation of the discussion [1].
>
> I haven't checked the updates in detail yet.  However, it seems that
> dev_pm_skip_suspend() and dev_pm_skip_resume() should be EXPORTed,
> since they are intended to be used by subsystems, which may be in
> modules.

OK, so what about an extra patch to export them?

Currently there are no modular users of these functions.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v2 0/9] PM: sleep: core: Rearrange the handling of driver power management flags
  2020-04-18 18:08     ` Rafael J. Wysocki
@ 2020-04-18 19:41       ` Alan Stern
  0 siblings, 0 replies; 31+ messages in thread
From: Alan Stern @ 2020-04-18 19:41 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Rafael J. Wysocki, Linux PM, Linux ACPI, Linux PCI, LKML,
	Bjorn Helgaas, Greg Kroah-Hartman, Mika Westerberg,
	Hans De Goede, Ulf Hansson

On Sat, 18 Apr 2020, Rafael J. Wysocki wrote:

> On Sat, Apr 18, 2020 at 8:00 PM Alan Stern <stern@rowland.harvard.edu> wrote:
> >
> > On Sat, 18 Apr 2020, Rafael J. Wysocki wrote:
> >
> > > Hi,
> > >
> > > This is an update including some fixes and extra patches based on the
> > > continuation of the discussion [1].
> >
> > I haven't checked the updates in detail yet.  However, it seems that
> > dev_pm_skip_suspend() and dev_pm_skip_resume() should be EXPORTed,
> > since they are intended to be used by subsystems, which may be in
> > modules.
> 
> OK, so what about an extra patch to export them?
> 
> Currently there are no modular users of these functions.

Ah, all right.  So when/if I want to use them, I will submit such a 
patch.

Alan Stern


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v2 0/9] PM: sleep: core: Rearrange the handling of driver power management flags
  2020-04-18 16:23 ` [PATCH v2 0/9] " Rafael J. Wysocki
                     ` (9 preceding siblings ...)
  2020-04-18 18:00   ` [PATCH v2 0/9] PM: sleep: core: Rearrange the handling of driver power management flags Alan Stern
@ 2020-04-19 14:43   ` Alan Stern
  2020-04-19 15:13     ` Rafael J. Wysocki
  2020-04-21 10:30   ` Ulf Hansson
  2020-04-23 17:07   ` Bjorn Helgaas
  12 siblings, 1 reply; 31+ messages in thread
From: Alan Stern @ 2020-04-19 14:43 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Linux PM, Linux ACPI, Linux PCI, LKML, Bjorn Helgaas,
	Greg Kroah-Hartman, Mika Westerberg, Hans De Goede, Ulf Hansson

On Sat, 18 Apr 2020, Rafael J. Wysocki wrote:

> Hi,
> 
> This is an update including some fixes and extra patches based on the
> continuation of the discussion [1].

The new code in pm.h and main.c all looks good.  Please add my
Acked-by: or Suggested-by: to the portions of the patches that affect
those files.

It's nice to see that, aside from the documentation, the patches ended
up removing more lines than they added.

Alan Stern


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v2 0/9] PM: sleep: core: Rearrange the handling of driver power management flags
  2020-04-19 14:43   ` Alan Stern
@ 2020-04-19 15:13     ` Rafael J. Wysocki
  0 siblings, 0 replies; 31+ messages in thread
From: Rafael J. Wysocki @ 2020-04-19 15:13 UTC (permalink / raw)
  To: Alan Stern
  Cc: Rafael J. Wysocki, Linux PM, Linux ACPI, Linux PCI, LKML,
	Bjorn Helgaas, Greg Kroah-Hartman, Mika Westerberg,
	Hans De Goede, Ulf Hansson

On Sun, Apr 19, 2020 at 4:43 PM Alan Stern <stern@rowland.harvard.edu> wrote:
>
> On Sat, 18 Apr 2020, Rafael J. Wysocki wrote:
>
> > Hi,
> >
> > This is an update including some fixes and extra patches based on the
> > continuation of the discussion [1].
>
> The new code in pm.h and main.c all looks good.  Please add my
> Acked-by: or Suggested-by: to the portions of the patches that affect
> those files.

I will, thank you!

> It's nice to see that, aside from the documentation, the patches ended
> up removing more lines than they added.

Indeed.

Cheers!

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v2 0/9] PM: sleep: core: Rearrange the handling of driver power management flags
  2020-04-18 16:23 ` [PATCH v2 0/9] " Rafael J. Wysocki
                     ` (10 preceding siblings ...)
  2020-04-19 14:43   ` Alan Stern
@ 2020-04-21 10:30   ` Ulf Hansson
  2020-04-21 11:32     ` Rafael J. Wysocki
  2020-04-23 17:07   ` Bjorn Helgaas
  12 siblings, 1 reply; 31+ messages in thread
From: Ulf Hansson @ 2020-04-21 10:30 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Linux PM, Alan Stern, Linux ACPI, Linux PCI, LKML, Bjorn Helgaas,
	Greg Kroah-Hartman, Mika Westerberg, Hans De Goede

On Sat, 18 Apr 2020 at 19:11, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>
> Hi,
>
> This is an update including some fixes and extra patches based on the
> continuation of the discussion [1].
>
> On Friday, April 10, 2020 5:46:27 PM CEST Rafael J. Wysocki wrote:
> > Hi Alan,
> >
> > Following our recent discussion regarding the DPM_FLAG_* family of flags [1],
> > I have decided to follow some of your recommendations and make changes to the
> > core code handling those flags.
> >
> > The purpose of this is basically to make the code more consistent internally,
> > easier to follow and better documented.
> >
> > First of all, patch [1/7] changes the PM core to skip driver-level "late"
> > and "noirq" suspend callbacks for devices with SMART_SUSPEND set if they are
> > still runtime-suspended during the "late" system-wide suspend phase (without
> > the patch it does that only if subsystem-level late/noirq/early suspend/resume
> > callbacks are not present for the device, which is demonstrably inconsistent)
> > and updates the resume part of the code accordingly (it doesn't need to check
> > whether or not the subsystem-level callbacks are present any more).
> >
> > The next patch, [2/7], is purely cosmetic and its only purpose is to reduce
> > the LOC number and move related pieces of code closer to each other.
>
> The first two patches have not changed.
>
> > Patch [3/7] changes the PM core so that it doesn't skip any subsystem-level
> > callbacks during system-wide resume (without the patch they may be skipped in
> > the "early resume" and "resume" phases due to LEAVE_SUSPENDED being set which
> > may be problematic) and to always run the driver's ->resume callback if the
> > corresponding subsystem-level callback is not present (without the patch it
> > may be skipped if LEAVE_SUSPENDED is set) to let it reverse the changes made
> > by the driver's ->suspend callback (which always runs too) if need be.
>
> The difference between this one and patch [3/9] in the v2 is the fixed
> definition of dev_pm_may_skip_resume(), renamed to dev_pm_skip_resume() by
> one of the next patches.
>
> Patch [4/9] changes the handling of the power.may_skip_resume flag to set it
> to 'true' by default and updates the subsystems aware of it to clear it when
> they don't want devices to stay in suspend.
>
> > Patches [4-6/7] rename one function in the PM core and two driver PM flags to
> > make their names better reflect their purpose.
>
> These are patches [5/9] and [7-8/9] in the v2 and patch [6/9] renames
> dev_pm_smart_suspend_and_suspended() to dev_pm_skip_suspend().
>
> > Finally, patch [7/7] updates the documentation of the driver PM flags to
> > reflect the new code flows.
>
> This patch [9/9] now and it has been updated to reflect the new code changes.
>
> The pm-sleep-core branch:
>
>   git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git \
>   pm-sleep-core
>
> contains the v2 now.
>
> Cheers!

Rafael, apologize for taking some time to review and respond. I
noticed you have queued this up on your next branch by now, good.

In any case, I have looked through the series and I think it looks good, thanks!

Kind regards
Uffe

>
>
> [1] https://lore.kernel.org/linux-pm/Pine.LNX.4.44L0.2003251631360.1724-100000@netrider.rowland.org/
>
>
>

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v2 0/9] PM: sleep: core: Rearrange the handling of driver power management flags
  2020-04-21 10:30   ` Ulf Hansson
@ 2020-04-21 11:32     ` Rafael J. Wysocki
  0 siblings, 0 replies; 31+ messages in thread
From: Rafael J. Wysocki @ 2020-04-21 11:32 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Rafael J. Wysocki, Linux PM, Alan Stern, Linux ACPI, Linux PCI,
	LKML, Bjorn Helgaas, Greg Kroah-Hartman, Mika Westerberg,
	Hans De Goede

On Tue, Apr 21, 2020 at 12:30 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> On Sat, 18 Apr 2020 at 19:11, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> >
> > Hi,
> >
> > This is an update including some fixes and extra patches based on the
> > continuation of the discussion [1].
> >
> > On Friday, April 10, 2020 5:46:27 PM CEST Rafael J. Wysocki wrote:
> > > Hi Alan,
> > >
> > > Following our recent discussion regarding the DPM_FLAG_* family of flags [1],
> > > I have decided to follow some of your recommendations and make changes to the
> > > core code handling those flags.
> > >
> > > The purpose of this is basically to make the code more consistent internally,
> > > easier to follow and better documented.
> > >
> > > First of all, patch [1/7] changes the PM core to skip driver-level "late"
> > > and "noirq" suspend callbacks for devices with SMART_SUSPEND set if they are
> > > still runtime-suspended during the "late" system-wide suspend phase (without
> > > the patch it does that only if subsystem-level late/noirq/early suspend/resume
> > > callbacks are not present for the device, which is demonstrably inconsistent)
> > > and updates the resume part of the code accordingly (it doesn't need to check
> > > whether or not the subsystem-level callbacks are present any more).
> > >
> > > The next patch, [2/7], is purely cosmetic and its only purpose is to reduce
> > > the LOC number and move related pieces of code closer to each other.
> >
> > The first two patches have not changed.
> >
> > > Patch [3/7] changes the PM core so that it doesn't skip any subsystem-level
> > > callbacks during system-wide resume (without the patch they may be skipped in
> > > the "early resume" and "resume" phases due to LEAVE_SUSPENDED being set which
> > > may be problematic) and to always run the driver's ->resume callback if the
> > > corresponding subsystem-level callback is not present (without the patch it
> > > may be skipped if LEAVE_SUSPENDED is set) to let it reverse the changes made
> > > by the driver's ->suspend callback (which always runs too) if need be.
> >
> > The difference between this one and patch [3/9] in the v2 is the fixed
> > definition of dev_pm_may_skip_resume(), renamed to dev_pm_skip_resume() by
> > one of the next patches.
> >
> > Patch [4/9] changes the handling of the power.may_skip_resume flag to set it
> > to 'true' by default and updates the subsystems aware of it to clear it when
> > they don't want devices to stay in suspend.
> >
> > > Patches [4-6/7] rename one function in the PM core and two driver PM flags to
> > > make their names better reflect their purpose.
> >
> > These are patches [5/9] and [7-8/9] in the v2 and patch [6/9] renames
> > dev_pm_smart_suspend_and_suspended() to dev_pm_skip_suspend().
> >
> > > Finally, patch [7/7] updates the documentation of the driver PM flags to
> > > reflect the new code flows.
> >
> > This patch [9/9] now and it has been updated to reflect the new code changes.
> >
> > The pm-sleep-core branch:
> >
> >   git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git \
> >   pm-sleep-core
> >
> > contains the v2 now.
> >
> > Cheers!
>
> Rafael, apologize for taking some time to review and respond. I
> noticed you have queued this up on your next branch by now, good.
>
> In any case, I have looked through the series and I think it looks good, thanks!

Thanks for letting me know!

Cheers!

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v2 7/9] PM: sleep: core: Rename DPM_FLAG_NEVER_SKIP
  2020-04-18 16:53   ` [PATCH v2 7/9] PM: sleep: core: Rename DPM_FLAG_NEVER_SKIP Rafael J. Wysocki
@ 2020-04-22 20:10     ` Alex Deucher
  0 siblings, 0 replies; 31+ messages in thread
From: Alex Deucher @ 2020-04-22 20:10 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Linux PM, Alan Stern, Ulf Hansson, Hans De Goede,
	Greg Kroah-Hartman, Intel Graphics, LKML, DRI-devel, Linux ACPI,
	Bjorn Helgaas, Linux PCI, intel-wired-lan, Mika Westerberg

On Sat, Apr 18, 2020 at 1:11 PM Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>
> From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
>
> Rename DPM_FLAG_NEVER_SKIP to DPM_FLAG_NO_DIRECT_COMPLETE which
> matches its purpose more closely.
>
> No functional impact.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Acked-by: Bjorn Helgaas <bhelgaas@google.com> # for PCI parts
> Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

Acked-by: Alex Deucher <alexander.deucher@amd.com>
for radeon and amdgpu

Alex

> ---
>
> -> v2:
>    * Rebased.
>    * Added tags received so far.
>
> ---
>  Documentation/driver-api/pm/devices.rst    |  6 +++---
>  Documentation/power/pci.rst                | 10 +++++-----
>  drivers/base/power/main.c                  |  2 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c    |  2 +-
>  drivers/gpu/drm/i915/intel_runtime_pm.c    |  2 +-
>  drivers/gpu/drm/radeon/radeon_kms.c        |  2 +-
>  drivers/misc/mei/pci-me.c                  |  2 +-
>  drivers/misc/mei/pci-txe.c                 |  2 +-
>  drivers/net/ethernet/intel/e1000e/netdev.c |  2 +-
>  drivers/net/ethernet/intel/igb/igb_main.c  |  2 +-
>  drivers/net/ethernet/intel/igc/igc_main.c  |  2 +-
>  drivers/pci/pcie/portdrv_pci.c             |  2 +-
>  include/linux/pm.h                         |  6 +++---
>  13 files changed, 21 insertions(+), 21 deletions(-)
>
> diff --git a/Documentation/driver-api/pm/devices.rst b/Documentation/driver-api/pm/devices.rst
> index f66c7b9126ea..4ace0eba4506 100644
> --- a/Documentation/driver-api/pm/devices.rst
> +++ b/Documentation/driver-api/pm/devices.rst
> @@ -361,9 +361,9 @@ the phases are: ``prepare``, ``suspend``, ``suspend_late``, ``suspend_noirq``.
>         runtime PM disabled.
>
>         This feature also can be controlled by device drivers by using the
> -       ``DPM_FLAG_NEVER_SKIP`` and ``DPM_FLAG_SMART_PREPARE`` driver power
> -       management flags.  [Typically, they are set at the time the driver is
> -       probed against the device in question by passing them to the
> +       ``DPM_FLAG_NO_DIRECT_COMPLETE`` and ``DPM_FLAG_SMART_PREPARE`` driver
> +       power management flags.  [Typically, they are set at the time the driver
> +       is probed against the device in question by passing them to the
>         :c:func:`dev_pm_set_driver_flags` helper function.]  If the first of
>         these flags is set, the PM core will not apply the direct-complete
>         procedure described above to the given device and, consequenty, to any
> diff --git a/Documentation/power/pci.rst b/Documentation/power/pci.rst
> index aa1c7fce6cd0..9e1408121bea 100644
> --- a/Documentation/power/pci.rst
> +++ b/Documentation/power/pci.rst
> @@ -1004,11 +1004,11 @@ including the PCI bus type.  The flags should be set once at the driver probe
>  time with the help of the dev_pm_set_driver_flags() function and they should not
>  be updated directly afterwards.
>
> -The DPM_FLAG_NEVER_SKIP flag prevents the PM core from using the direct-complete
> -mechanism allowing device suspend/resume callbacks to be skipped if the device
> -is in runtime suspend when the system suspend starts.  That also affects all of
> -the ancestors of the device, so this flag should only be used if absolutely
> -necessary.
> +The DPM_FLAG_NO_DIRECT_COMPLETE flag prevents the PM core from using the
> +direct-complete mechanism allowing device suspend/resume callbacks to be skipped
> +if the device is in runtime suspend when the system suspend starts.  That also
> +affects all of the ancestors of the device, so this flag should only be used if
> +absolutely necessary.
>
>  The DPM_FLAG_SMART_PREPARE flag instructs the PCI bus type to only return a
>  positive value from pci_pm_prepare() if the ->prepare callback provided by the
> diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
> index 3170d93e29f9..dbc1e5e7346b 100644
> --- a/drivers/base/power/main.c
> +++ b/drivers/base/power/main.c
> @@ -1844,7 +1844,7 @@ static int device_prepare(struct device *dev, pm_message_t state)
>         spin_lock_irq(&dev->power.lock);
>         dev->power.direct_complete = state.event == PM_EVENT_SUSPEND &&
>                 (ret > 0 || dev->power.no_pm_callbacks) &&
> -               !dev_pm_test_driver_flags(dev, DPM_FLAG_NEVER_SKIP);
> +               !dev_pm_test_driver_flags(dev, DPM_FLAG_NO_DIRECT_COMPLETE);
>         spin_unlock_irq(&dev->power.lock);
>         return 0;
>  }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index fd1dc3236eca..a9086ea1ab60 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -191,7 +191,7 @@ int amdgpu_driver_load_kms(struct drm_device *dev, unsigned long flags)
>         }
>
>         if (adev->runpm) {
> -               dev_pm_set_driver_flags(dev->dev, DPM_FLAG_NEVER_SKIP);
> +               dev_pm_set_driver_flags(dev->dev, DPM_FLAG_NO_DIRECT_COMPLETE);
>                 pm_runtime_use_autosuspend(dev->dev);
>                 pm_runtime_set_autosuspend_delay(dev->dev, 5000);
>                 pm_runtime_set_active(dev->dev);
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index ad719c9602af..9cb2d7548daa 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -549,7 +549,7 @@ void intel_runtime_pm_enable(struct intel_runtime_pm *rpm)
>          * becaue the HDA driver may require us to enable the audio power
>          * domain during system suspend.
>          */
> -       dev_pm_set_driver_flags(kdev, DPM_FLAG_NEVER_SKIP);
> +       dev_pm_set_driver_flags(kdev, DPM_FLAG_NO_DIRECT_COMPLETE);
>
>         pm_runtime_set_autosuspend_delay(kdev, 10000); /* 10s */
>         pm_runtime_mark_last_busy(kdev);
> diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c
> index 58176db85952..372962358a18 100644
> --- a/drivers/gpu/drm/radeon/radeon_kms.c
> +++ b/drivers/gpu/drm/radeon/radeon_kms.c
> @@ -158,7 +158,7 @@ int radeon_driver_load_kms(struct drm_device *dev, unsigned long flags)
>         }
>
>         if (radeon_is_px(dev)) {
> -               dev_pm_set_driver_flags(dev->dev, DPM_FLAG_NEVER_SKIP);
> +               dev_pm_set_driver_flags(dev->dev, DPM_FLAG_NO_DIRECT_COMPLETE);
>                 pm_runtime_use_autosuspend(dev->dev);
>                 pm_runtime_set_autosuspend_delay(dev->dev, 5000);
>                 pm_runtime_set_active(dev->dev);
> diff --git a/drivers/misc/mei/pci-me.c b/drivers/misc/mei/pci-me.c
> index 3d21c38e2dbb..53f16f3bd091 100644
> --- a/drivers/misc/mei/pci-me.c
> +++ b/drivers/misc/mei/pci-me.c
> @@ -240,7 +240,7 @@ static int mei_me_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>          * MEI requires to resume from runtime suspend mode
>          * in order to perform link reset flow upon system suspend.
>          */
> -       dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_NEVER_SKIP);
> +       dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_NO_DIRECT_COMPLETE);
>
>         /*
>          * ME maps runtime suspend/resume to D0i states,
> diff --git a/drivers/misc/mei/pci-txe.c b/drivers/misc/mei/pci-txe.c
> index beacf2a2f2b5..4bf26ce61044 100644
> --- a/drivers/misc/mei/pci-txe.c
> +++ b/drivers/misc/mei/pci-txe.c
> @@ -128,7 +128,7 @@ static int mei_txe_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>          * MEI requires to resume from runtime suspend mode
>          * in order to perform link reset flow upon system suspend.
>          */
> -       dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_NEVER_SKIP);
> +       dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_NO_DIRECT_COMPLETE);
>
>         /*
>          * TXE maps runtime suspend/resume to own power gating states,
> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
> index 177c6da80c57..2730b1c7dddb 100644
> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> @@ -7549,7 +7549,7 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>
>         e1000_print_device_info(adapter);
>
> -       dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_NEVER_SKIP);
> +       dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_NO_DIRECT_COMPLETE);
>
>         if (pci_dev_run_wake(pdev) && hw->mac.type < e1000_pch_cnp)
>                 pm_runtime_put_noidle(&pdev->dev);
> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
> index b46bff8fe056..8bb3db2cbd41 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -3445,7 +3445,7 @@ static int igb_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>                 }
>         }
>
> -       dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_NEVER_SKIP);
> +       dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_NO_DIRECT_COMPLETE);
>
>         pm_runtime_put_noidle(&pdev->dev);
>         return 0;
> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
> index 69fa1ce1f927..59fc0097438f 100644
> --- a/drivers/net/ethernet/intel/igc/igc_main.c
> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> @@ -4825,7 +4825,7 @@ static int igc_probe(struct pci_dev *pdev,
>         pcie_print_link_status(pdev);
>         netdev_info(netdev, "MAC: %pM\n", netdev->dev_addr);
>
> -       dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_NEVER_SKIP);
> +       dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_NO_DIRECT_COMPLETE);
>
>         pm_runtime_put_noidle(&pdev->dev);
>
> diff --git a/drivers/pci/pcie/portdrv_pci.c b/drivers/pci/pcie/portdrv_pci.c
> index 160d67c59310..3acf151ae015 100644
> --- a/drivers/pci/pcie/portdrv_pci.c
> +++ b/drivers/pci/pcie/portdrv_pci.c
> @@ -115,7 +115,7 @@ static int pcie_portdrv_probe(struct pci_dev *dev,
>
>         pci_save_state(dev);
>
> -       dev_pm_set_driver_flags(&dev->dev, DPM_FLAG_NEVER_SKIP |
> +       dev_pm_set_driver_flags(&dev->dev, DPM_FLAG_NO_DIRECT_COMPLETE |
>                                            DPM_FLAG_SMART_SUSPEND);
>
>         if (pci_bridge_d3_possible(dev)) {
> diff --git a/include/linux/pm.h b/include/linux/pm.h
> index 8c59a7f0bcf4..cdb8fbd6ab18 100644
> --- a/include/linux/pm.h
> +++ b/include/linux/pm.h
> @@ -544,7 +544,7 @@ struct pm_subsys_data {
>   * These flags can be set by device drivers at the probe time.  They need not be
>   * cleared by the drivers as the driver core will take care of that.
>   *
> - * NEVER_SKIP: Do not skip all system suspend/resume callbacks for the device.
> + * NO_DIRECT_COMPLETE: Do not apply direct-complete optimization to the device.
>   * SMART_PREPARE: Check the return value of the driver's ->prepare callback.
>   * SMART_SUSPEND: No need to resume the device from runtime suspend.
>   * LEAVE_SUSPENDED: Avoid resuming the device during system resume if possible.
> @@ -554,7 +554,7 @@ struct pm_subsys_data {
>   * their ->prepare callbacks if the driver's ->prepare callback returns 0 (in
>   * other words, the system suspend/resume callbacks can only be skipped for the
>   * device if its driver doesn't object against that).  This flag has no effect
> - * if NEVER_SKIP is set.
> + * if NO_DIRECT_COMPLETE is set.
>   *
>   * Setting SMART_SUSPEND instructs bus types and PM domains which may want to
>   * runtime resume the device upfront during system suspend that doing so is not
> @@ -565,7 +565,7 @@ struct pm_subsys_data {
>   * Setting LEAVE_SUSPENDED informs the PM core and middle-layer code that the
>   * driver prefers the device to be left in suspend after system resume.
>   */
> -#define DPM_FLAG_NEVER_SKIP            BIT(0)
> +#define DPM_FLAG_NO_DIRECT_COMPLETE    BIT(0)
>  #define DPM_FLAG_SMART_PREPARE         BIT(1)
>  #define DPM_FLAG_SMART_SUSPEND         BIT(2)
>  #define DPM_FLAG_LEAVE_SUSPENDED       BIT(3)
> --
> 2.16.4
>
>
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v2 0/9] PM: sleep: core: Rearrange the handling of driver power management flags
  2020-04-18 16:23 ` [PATCH v2 0/9] " Rafael J. Wysocki
                     ` (11 preceding siblings ...)
  2020-04-21 10:30   ` Ulf Hansson
@ 2020-04-23 17:07   ` Bjorn Helgaas
  12 siblings, 0 replies; 31+ messages in thread
From: Bjorn Helgaas @ 2020-04-23 17:07 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Linux PM, Alan Stern, Linux ACPI, Linux PCI, LKML,
	Greg Kroah-Hartman, Mika Westerberg, Hans De Goede, Ulf Hansson

On Sat, Apr 18, 2020 at 06:23:08PM +0200, Rafael J. Wysocki wrote:
> Hi,
> 
> This is an update including some fixes and extra patches based on the
> continuation of the discussion [1].
> 
> On Friday, April 10, 2020 5:46:27 PM CEST Rafael J. Wysocki wrote:
> > Hi Alan,
> > 
> > Following our recent discussion regarding the DPM_FLAG_* family of flags [1],
> > I have decided to follow some of your recommendations and make changes to the
> > core code handling those flags.
> > 
> > The purpose of this is basically to make the code more consistent internally,
> > easier to follow and better documented.
> > 
> > First of all, patch [1/7] changes the PM core to skip driver-level "late"
> > and "noirq" suspend callbacks for devices with SMART_SUSPEND set if they are
> > still runtime-suspended during the "late" system-wide suspend phase (without
> > the patch it does that only if subsystem-level late/noirq/early suspend/resume
> > callbacks are not present for the device, which is demonstrably inconsistent)
> > and updates the resume part of the code accordingly (it doesn't need to check
> > whether or not the subsystem-level callbacks are present any more).
> > 
> > The next patch, [2/7], is purely cosmetic and its only purpose is to reduce
> > the LOC number and move related pieces of code closer to each other.
> 
> The first two patches have not changed.
> 
> > Patch [3/7] changes the PM core so that it doesn't skip any subsystem-level
> > callbacks during system-wide resume (without the patch they may be skipped in
> > the "early resume" and "resume" phases due to LEAVE_SUSPENDED being set which
> > may be problematic) and to always run the driver's ->resume callback if the
> > corresponding subsystem-level callback is not present (without the patch it
> > may be skipped if LEAVE_SUSPENDED is set) to let it reverse the changes made
> > by the driver's ->suspend callback (which always runs too) if need be.
> 
> The difference between this one and patch [3/9] in the v2 is the fixed
> definition of dev_pm_may_skip_resume(), renamed to dev_pm_skip_resume() by
> one of the next patches.
> 
> Patch [4/9] changes the handling of the power.may_skip_resume flag to set it
> to 'true' by default and updates the subsystems aware of it to clear it when
> they don't want devices to stay in suspend.
> 
> > Patches [4-6/7] rename one function in the PM core and two driver PM flags to
> > make their names better reflect their purpose.
> 
> These are patches [5/9] and [7-8/9] in the v2 and patch [6/9] renames
> dev_pm_smart_suspend_and_suspended() to dev_pm_skip_suspend().
> 
> > Finally, patch [7/7] updates the documentation of the driver PM flags to
> > reflect the new code flows.
> 
> This patch [9/9] now and it has been updated to reflect the new code changes.
> 
> The pm-sleep-core branch:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git \
>   pm-sleep-core
> 
> contains the v2 now.

For the drivers/pci parts:

Acked-by: Bjorn Helgaas <bhelgaas@google.com>


^ permalink raw reply	[flat|nested] 31+ messages in thread

end of thread, other threads:[~2020-04-23 17:07 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-10 15:46 [PATCH 0/7] PM: sleep: core: Rearrange the handling of driver power management flags Rafael J. Wysocki
2020-04-10 15:48 ` [PATCH 1/7] PM: sleep: core: Simplify the SMART_SUSPEND flag handling Rafael J. Wysocki
2020-04-10 15:48 ` [PATCH 2/7] PM: sleep: core: Fold functions into their callers Rafael J. Wysocki
2020-04-10 15:51 ` [PATCH 3/7] PM: sleep: core: Do not skip callbacks in the resume phase Rafael J. Wysocki
2020-04-10 15:51 ` [PATCH 4/7] PM: sleep: core: Rename dev_pm_may_skip_resume() Rafael J. Wysocki
2020-04-10 15:56 ` [PATCH 5/7] PM: sleep: core: Rename DPM_FLAG_NEVER_SKIP Rafael J. Wysocki
2020-04-10 19:38   ` Bjorn Helgaas
2020-04-13  6:35   ` Jeff Kirsher
2020-04-10 15:57 ` [PATCH 6/7] PM: sleep: core: Rename DPM_FLAG_LEAVE_SUSPENDED Rafael J. Wysocki
2020-04-15  9:51   ` Wolfram Sang
2020-04-10 15:58 ` [PATCH 7/7] Documentation: PM: sleep: Update driver flags documentation Rafael J. Wysocki
2020-04-13 13:11 ` [PATCH 0/7] PM: sleep: core: Rearrange the handling of driver power management flags Hans de Goede
2020-04-18 16:23 ` [PATCH v2 0/9] " Rafael J. Wysocki
2020-04-18 16:29   ` [PATCH v2 1/9] PM: sleep: core: Simplify the SMART_SUSPEND flag handling Rafael J. Wysocki
2020-04-18 16:51   ` [PATCH v2 2/9] PM: sleep: core: Fold functions into their callers Rafael J. Wysocki
2020-04-18 16:52   ` [PATCH v2 3/9] PM: sleep: core: Do not skip callbacks in the resume phase Rafael J. Wysocki
2020-04-18 16:52   ` [PATCH v2 4/9] PM: sleep: core: Rework the power.may_skip_resume handling Rafael J. Wysocki
2020-04-18 16:52   ` [PATCH v2 5/9] PM: sleep: core: Rename dev_pm_may_skip_resume() Rafael J. Wysocki
2020-04-18 16:52   ` [PATCH v2 6/9] PM: sleep: core: Rename dev_pm_smart_suspend_and_suspended() Rafael J. Wysocki
2020-04-18 16:53   ` [PATCH v2 7/9] PM: sleep: core: Rename DPM_FLAG_NEVER_SKIP Rafael J. Wysocki
2020-04-22 20:10     ` Alex Deucher
2020-04-18 16:53   ` [PATCH v2 8/9] PM: sleep: core: Rename DPM_FLAG_LEAVE_SUSPENDED Rafael J. Wysocki
2020-04-18 16:55   ` [PATCH v2 9/9] Documentation: PM: sleep: Update driver flags documentation Rafael J. Wysocki
2020-04-18 18:00   ` [PATCH v2 0/9] PM: sleep: core: Rearrange the handling of driver power management flags Alan Stern
2020-04-18 18:08     ` Rafael J. Wysocki
2020-04-18 19:41       ` Alan Stern
2020-04-19 14:43   ` Alan Stern
2020-04-19 15:13     ` Rafael J. Wysocki
2020-04-21 10:30   ` Ulf Hansson
2020-04-21 11:32     ` Rafael J. Wysocki
2020-04-23 17:07   ` Bjorn Helgaas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).