[PATCH 0/7] PM / core: Direct handling of DPM_FLAG_SMART_SUSPEND and DPM_FLAG_LEAVE_SUSPENDED

[PATCH 0/7] PM / core: Direct handling of DPM_FLAG_SMART_SUSPEND and DPM_FLAG_LEAVE_SUSPENDED

[Rafael J. Wysocki]

On Sunday, December 10, 2017 12:55:23 AM CET Rafael J. Wysocki wrote:
> Hi All,
> 
> This series is a follow-up for
> 
> https://marc.info/?l=linux-doc&m=151101644105835&w=2
> 
> Patches[1-3/6] from the above have been reviewed and agreed on, so
> they are in linux-next now and here's a next version of the rest.
> 
> Patches [1-2/4] are preparatory.  The first one is just really small
> code duplication avoidance on top of this recent fix:
> 
> https://patchwork.kernel.org/patch/10097563/
> 
> and the second one simply moves some code to separate functions.
> 
> Patch [3/4] causes the PM core to carry out some optimizations for
> drivers of devices with DPM_FLAG_SMART_SUSPEND set whose "late"
> and "noirq" suspend (or equivalent) driver callbacks are invoked
> directly by the core.
> 
> The underlying observation is that if the device is suspended (via
> runtime PM) during the "late suspend" phase of a system transition,
> invoking the "late" and "noirq" callbacks from the driver for it is not
> going to make it more suspended, so to speak, so it doesn't make sense to
> invoke them at all.
> 
> [That optimization is only done for devices with DPM_FLAG_SMART_SUSPEND
> set, because drivers setting that flag are expected to be prepared for
> skipping their "late" and "noirq" callbacks if the device is already
> suspended.]
> 
> Patch [4/4] makes the core do an analogous thing for devices with
> DPM_FLAG_LEAVE_SUSPENDED set whose "noirq" and "early" resume (or
> equivalent) driver callbacks are directly invoked by the core.
> 
> In that case the observation is that if such devices can be left in
> suspend after the system transition to the working state, running
> resume callbacks from their drivers is simply not necessary.
> 
> Pathes [3-4/4] have been reoredered and reworked a bit since the last
> iteration, so they are regarded as new.
> 
> The series is on top of the linux-next branch of the linux-pm.git tree
> that should be merged into linux-next on Monday.
> 
> [I have developed debug bus type and driver modules to test that code,
> but they are not ready to be made available at this point.]

Resending the original series (except for the first patch that has been
applied already) along with some driver code changes based on them as
requested by Ulf.

The driver patches are an intel-lpss change (already reviewed), two
modifications of i2c-designware-platdrv (posted previously but slightly
changed since then) and a PCIe port driver change.

At this point patches [1-3/7] are pretty much on the way in and the driver
material depends on review comments (it is pointless to apply [4/7] without
[5-6/7], so it depends on them in my view).

I'm sending this from a system running with all of the series applied, although
admittedly not using i2c-designware-platdrv.  However, one of my test machines
has this one and I haven't seen any adverse effects of these changes on it so
far.

Thanks,
Rafael

[PATCH 1/7] PM / core: Add helpers for subsystem callback selection

[Rafael J. Wysocki]

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

Add helper routines to find and return a suitable subsystem callback
during the "noirq" phases of system suspend/resume (or analogous)
transitions as well as during the "late" phase of system suspend and
the "early" phase of system resume (or analogous) transitions.

The helpers will be called from additional sites going forward.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/base/power/main.c |  188 +++++++++++++++++++++++++++++++---------------
 1 file changed, 128 insertions(+), 60 deletions(-)

Index: linux-pm/drivers/base/power/main.c
===================================================================
--- linux-pm.orig/drivers/base/power/main.c
+++ linux-pm/drivers/base/power/main.c
@@ -551,6 +551,35 @@ bool dev_pm_may_skip_resume(struct devic
 	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;
+}
+
 /**
  * device_resume_noirq - Execute a "noirq resume" callback for given device.
  * @dev: Device to handle.
@@ -562,8 +591,8 @@ bool dev_pm_may_skip_resume(struct devic
  */
 static int device_resume_noirq(struct device *dev, pm_message_t state, bool async)
 {
-	pm_callback_t callback = NULL;
-	const char *info = NULL;
+	pm_callback_t callback;
+	const char *info;
 	int error = 0;
 
 	TRACE_DEVICE(dev);
@@ -577,19 +606,7 @@ static int device_resume_noirq(struct de
 
 	dpm_wait_for_superior(dev, async);
 
-	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);
-	}
+	callback = dpm_subsys_resume_noirq_cb(dev, state, &info);
 
 	if (!callback && dev->driver && dev->driver->pm) {
 		info = "noirq driver ";
@@ -704,6 +721,35 @@ void dpm_resume_noirq(pm_message_t state
 	dpm_noirq_end();
 }
 
+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.
@@ -714,8 +760,8 @@ void dpm_resume_noirq(pm_message_t state
  */
 static int device_resume_early(struct device *dev, pm_message_t state, bool async)
 {
-	pm_callback_t callback = NULL;
-	const char *info = NULL;
+	pm_callback_t callback;
+	const char *info;
 	int error = 0;
 
 	TRACE_DEVICE(dev);
@@ -729,19 +775,7 @@ static int device_resume_early(struct de
 
 	dpm_wait_for_superior(dev, async);
 
-	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);
-	}
+	callback = dpm_subsys_resume_early_cb(dev, state, &info);
 
 	if (!callback && dev->driver && dev->driver->pm) {
 		info = "early driver ";
@@ -1128,6 +1162,35 @@ static void dpm_superior_set_must_resume
 	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.
@@ -1139,8 +1202,8 @@ static void dpm_superior_set_must_resume
  */
 static int __device_suspend_noirq(struct device *dev, pm_message_t state, bool async)
 {
-	pm_callback_t callback = NULL;
-	const char *info = NULL;
+	pm_callback_t callback;
+	const char *info;
 	int error = 0;
 
 	TRACE_DEVICE(dev);
@@ -1159,19 +1222,7 @@ static int __device_suspend_noirq(struct
 	if (dev->power.syscore || dev->power.direct_complete)
 		goto Complete;
 
-	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);
-	}
+	callback = dpm_subsys_suspend_noirq_cb(dev, state, &info);
 
 	if (!callback && dev->driver && dev->driver->pm) {
 		info = "noirq driver ";
@@ -1306,6 +1357,35 @@ int dpm_suspend_noirq(pm_message_t state
 	return ret;
 }
 
+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.
@@ -1316,8 +1396,8 @@ int dpm_suspend_noirq(pm_message_t state
  */
 static int __device_suspend_late(struct device *dev, pm_message_t state, bool async)
 {
-	pm_callback_t callback = NULL;
-	const char *info = NULL;
+	pm_callback_t callback;
+	const char *info;
 	int error = 0;
 
 	TRACE_DEVICE(dev);
@@ -1338,19 +1418,7 @@ static int __device_suspend_late(struct
 	if (dev->power.syscore || dev->power.direct_complete)
 		goto Complete;
 
-	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);
-	}
+	callback = dpm_subsys_suspend_late_cb(dev, state, &info);
 
 	if (!callback && dev->driver && dev->driver->pm) {
 		info = "late driver ";

[PATCH 2/7] PM / core: Direct DPM_FLAG_SMART_SUSPEND optimization

[Rafael J. Wysocki]

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

Make the PM core avoid invoking the "late" and "noirq" system-wide
suspend (or analogous) callbacks provided by device drivers directly
for devices with DPM_FLAG_SMART_SUSPEND set that are in runtime
suspend during the "late" and "noirq" phases of system-wide suspend
(or analogous) transitions.  That is only done for devices without
any middle-layer "late" and "noirq" suspend callbacks (to avoid
confusing the middle layer if there is one).

The underlying observation is that runtime PM is disabled for devices
during the "late" and "noirq" system-wide suspend phases, so if they
remain in runtime suspend from the "late" phase forward, it doesn't
make sense to invoke the "late" and "noirq" callbacks provided by
the drivers for them (arguably, the device is already suspended and
in the right state).  Thus, if the remaining driver suspend callbacks
are to be invoked directly by the core, they can be skipped.

This change really makes it possible for, say, platform device
drivers to re-use runtime PM suspend and resume callbacks by
pointing ->suspend_late and ->resume_early, respectively (and
possibly the analogous hibernation-related callback pointers too),
to them without adding any extra "is the device already suspended?"
type of checks to the callback routines, as long as they will be
invoked directly by the core.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 Documentation/driver-api/pm/devices.rst |   18 +++---
 drivers/base/power/main.c               |   85 +++++++++++++++++++++++++++++---
 2 files changed, 88 insertions(+), 15 deletions(-)

Index: linux-pm/Documentation/driver-api/pm/devices.rst
===================================================================
--- linux-pm.orig/Documentation/driver-api/pm/devices.rst
+++ linux-pm/Documentation/driver-api/pm/devices.rst
@@ -777,14 +777,16 @@ The driver can indicate that by setting
 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.  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" status for runtime PM in that case.
+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.
 
 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.txt`.
Index: linux-pm/drivers/base/power/main.c
===================================================================
--- linux-pm.orig/drivers/base/power/main.c
+++ linux-pm/drivers/base/power/main.c
@@ -540,6 +540,24 @@ void dev_pm_skip_next_resume_phases(stru
 }
 
 /**
+ * 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.
  *
@@ -580,6 +598,14 @@ static pm_callback_t dpm_subsys_resume_n
 	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.
@@ -607,13 +633,40 @@ static int device_resume_noirq(struct de
 	dpm_wait_for_superior(dev, async);
 
 	callback = dpm_subsys_resume_noirq_cb(dev, state, &info);
+	if (callback)
+		goto Run;
 
-	if (!callback && dev->driver && dev->driver->pm) {
+	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) {
+				dev_pm_skip_next_resume_phases(dev);
+				goto Skip;
+			} else {
+				pm_runtime_set_active(dev);
+			}
+		}
+	}
+
+	if (dev->driver && dev->driver->pm) {
 		info = "noirq driver ";
 		callback = pm_noirq_op(dev->driver->pm, state);
 	}
 
+Run:
 	error = dpm_run_callback(callback, dev, state, info);
+
+Skip:
 	dev->power.is_noirq_suspended = false;
 
 	if (dev_pm_may_skip_resume(dev)) {
@@ -628,7 +681,7 @@ static int device_resume_noirq(struct de
 		dev_pm_skip_next_resume_phases(dev);
 	}
 
- Out:
+Out:
 	complete_all(&dev->power.completion);
 	TRACE_RESUME(error);
 	return error;
@@ -1223,18 +1276,26 @@ static int __device_suspend_noirq(struct
 		goto Complete;
 
 	callback = dpm_subsys_suspend_noirq_cb(dev, state, &info);
+	if (callback)
+		goto Run;
 
-	if (!callback && dev->driver && dev->driver->pm) {
+	if (dev_pm_smart_suspend_and_suspended(dev) &&
+	    !dpm_subsys_suspend_late_cb(dev, state, NULL))
+		goto Skip;
+
+	if (dev->driver && dev->driver->pm) {
 		info = "noirq driver ";
 		callback = pm_noirq_op(dev->driver->pm, state);
 	}
 
+Run:
 	error = dpm_run_callback(callback, dev, state, info);
 	if (error) {
 		async_error = error;
 		goto Complete;
 	}
 
+Skip:
 	dev->power.is_noirq_suspended = true;
 
 	if (dev_pm_test_driver_flags(dev, DPM_FLAG_LEAVE_SUSPENDED)) {
@@ -1419,17 +1480,27 @@ static int __device_suspend_late(struct
 		goto Complete;
 
 	callback = dpm_subsys_suspend_late_cb(dev, state, &info);
+	if (callback)
+		goto Run;
 
-	if (!callback && dev->driver && dev->driver->pm) {
+	if (dev_pm_smart_suspend_and_suspended(dev) &&
+	    !dpm_subsys_suspend_noirq_cb(dev, state, NULL))
+		goto Skip;
+
+	if (dev->driver && dev->driver->pm) {
 		info = "late driver ";
 		callback = pm_late_early_op(dev->driver->pm, state);
 	}
 
+Run:
 	error = dpm_run_callback(callback, dev, state, info);
-	if (!error)
-		dev->power.is_late_suspended = true;
-	else
+	if (error) {
 		async_error = error;
+		goto Complete;
+	}
+
+Skip:
+	dev->power.is_late_suspended = true;
 
 Complete:
 	TRACE_SUSPEND(error);

[PATCH 3/7] PM / core: Direct DPM_FLAG_LEAVE_SUSPENDED handling

[Rafael J. Wysocki]

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

Make the PM core handle DPM_FLAG_LEAVE_SUSPENDED directly for
devices whose "noirq", "late" and "early" driver callbacks are
invoked directly by it.

Namely, make it skip all of the system-wide resume callbacks for
such devices with DPM_FLAG_LEAVE_SUSPENDED set if (1) they are in
runtime suspend during the "noirq" phase of system-wide suspend
(or analogous) transitions or (2) the system transition under way is
a proper suspend (rather than anything related to hibernation) and
the device's wakeup settings are compatible with runtime PM (that
is, the device cannot generate wakeup signals at all or it is
allowed to wake up the system from sleep).

The underlying observation is that if either (1) or (2) takes place,
it simply is not necessary to invoke any resume callbacks for the
device that can be left in suspend.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 Documentation/driver-api/pm/devices.rst |    9 +++++
 drivers/base/power/main.c               |   51 +++++++++++++++++++++++++-------
 2 files changed, 49 insertions(+), 11 deletions(-)

Index: linux-pm/Documentation/driver-api/pm/devices.rst
===================================================================
--- linux-pm.orig/Documentation/driver-api/pm/devices.rst
+++ linux-pm/Documentation/driver-api/pm/devices.rst
@@ -816,3 +816,12 @@ appropriate in its "noirq" resume callba
 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_LEAVE_SUSPENDED`` 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).
Index: linux-pm/drivers/base/power/main.c
===================================================================
--- linux-pm.orig/drivers/base/power/main.c
+++ linux-pm/drivers/base/power/main.c
@@ -619,6 +619,7 @@ static int device_resume_noirq(struct de
 {
 	pm_callback_t callback;
 	const char *info;
+	bool skip_resume;
 	int error = 0;
 
 	TRACE_DEVICE(dev);
@@ -632,10 +633,15 @@ static int device_resume_noirq(struct de
 
 	dpm_wait_for_superior(dev, async);
 
+	skip_resume = dev_pm_may_skip_resume(dev);
+
 	callback = dpm_subsys_resume_noirq_cb(dev, state, &info);
 	if (callback)
 		goto Run;
 
+	if (skip_resume)
+		goto Skip;
+
 	if (dev_pm_smart_suspend_and_suspended(dev)) {
 		pm_message_t suspend_msg = suspend_event(state);
 
@@ -650,7 +656,7 @@ static int device_resume_noirq(struct de
 		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) {
-				dev_pm_skip_next_resume_phases(dev);
+				skip_resume = true;
 				goto Skip;
 			} else {
 				pm_runtime_set_active(dev);
@@ -669,7 +675,7 @@ Run:
 Skip:
 	dev->power.is_noirq_suspended = false;
 
-	if (dev_pm_may_skip_resume(dev)) {
+	if (skip_resume) {
 		/*
 		 * The device is going to be left in suspend, but it might not
 		 * have been in runtime suspend before the system suspended, so
@@ -1244,6 +1250,32 @@ static pm_callback_t dpm_subsys_suspend_
 	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.
@@ -1257,6 +1289,7 @@ static int __device_suspend_noirq(struct
 {
 	pm_callback_t callback;
 	const char *info;
+	bool no_subsys_cb = false;
 	int error = 0;
 
 	TRACE_DEVICE(dev);
@@ -1279,8 +1312,9 @@ static int __device_suspend_noirq(struct
 	if (callback)
 		goto Run;
 
-	if (dev_pm_smart_suspend_and_suspended(dev) &&
-	    !dpm_subsys_suspend_late_cb(dev, state, NULL))
+	no_subsys_cb = !dpm_subsys_suspend_late_cb(dev, state, NULL);
+
+	if (dev_pm_smart_suspend_and_suspended(dev) && no_subsys_cb)
 		goto Skip;
 
 	if (dev->driver && dev->driver->pm) {
@@ -1299,14 +1333,9 @@ Skip:
 	dev->power.is_noirq_suspended = true;
 
 	if (dev_pm_test_driver_flags(dev, DPM_FLAG_LEAVE_SUSPENDED)) {
-		/*
-		 * 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.
-		 */
 		dev->power.must_resume = dev->power.must_resume ||
-					!dev->power.may_skip_resume ||
-					atomic_read(&dev->power.usage_count) > 1;
+				atomic_read(&dev->power.usage_count) > 1 ||
+				device_must_resume(dev, state, no_subsys_cb);
 	} else {
 		dev->power.must_resume = true;
 	}

[PATCH 4/7] PM / mfd: intel-lpss: Use DPM_FLAG_SMART_SUSPEND

[Rafael J. Wysocki]

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

Make the intel-lpss driver set DPM_FLAG_SMART_SUSPEND for its
devices which will allow them to stay in runtime suspend during
system suspend unless they need to be reconfigured for some reason.

Also make it avoid resuming its child devices if they have
DPM_FLAG_SMART_SUSPEND set to allow them to remain in runtime
suspend during system suspend.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mfd/intel-lpss.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Index: linux-pm/drivers/mfd/intel-lpss.c
===================================================================
--- linux-pm.orig/drivers/mfd/intel-lpss.c
+++ linux-pm/drivers/mfd/intel-lpss.c
@@ -450,6 +450,8 @@ int intel_lpss_probe(struct device *dev,
 	if (ret)
 		goto err_remove_ltr;
 
+	dev_pm_set_driver_flags(dev, DPM_FLAG_SMART_SUSPEND);
+
 	return 0;
 
 err_remove_ltr:
@@ -478,7 +480,9 @@ EXPORT_SYMBOL_GPL(intel_lpss_remove);
 
 static int resume_lpss_device(struct device *dev, void *data)
 {
-	pm_runtime_resume(dev);
+	if (!dev_pm_test_driver_flags(dev, DPM_FLAG_SMART_SUSPEND))
+		pm_runtime_resume(dev);
+
 	return 0;
 }
 

[PATCH 5/7] PM: i2c-designware-platdrv: Use DPM_FLAG_SMART_PREPARE

[Rafael J. Wysocki]

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

Modify i2c-designware-platdrv to set DPM_FLAG_SMART_PREPARE for its
devices and return 0 from the system suspend ->prepare callback
if the device has an ACPI companion object in order to tell the PM
core and middle layers to avoid skipping system suspend/resume
callbacks for the device in that case (which may be problematic,
because the device may be accessed during suspend and resume of
other devices via I2C operation regions then).

Also the pm_runtime_suspended() check in dw_i2c_plat_prepare()
is not necessary any more, because the core does it when setting
power.direct_complete for the device, so drop it.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/i2c/busses/i2c-designware-platdrv.c |   10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

Index: linux-pm/drivers/i2c/busses/i2c-designware-platdrv.c
===================================================================
--- linux-pm.orig/drivers/i2c/busses/i2c-designware-platdrv.c
+++ linux-pm/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -372,6 +372,8 @@ static int dw_i2c_plat_probe(struct plat
 	ACPI_COMPANION_SET(&adap->dev, ACPI_COMPANION(&pdev->dev));
 	adap->dev.of_node = pdev->dev.of_node;
 
+	dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_SMART_PREPARE);
+
 	/* The code below assumes runtime PM to be disabled. */
 	WARN_ON(pm_runtime_enabled(&pdev->dev));
 
@@ -435,7 +437,13 @@ MODULE_DEVICE_TABLE(of, dw_i2c_of_match)
 #ifdef CONFIG_PM_SLEEP
 static int dw_i2c_plat_prepare(struct device *dev)
 {
-	return pm_runtime_suspended(dev);
+	/*
+	 * If the ACPI companion device object is present for this device, it
+	 * may be accessed during suspend and resume of other devices via I2C
+	 * operation regions, so tell the PM core and middle layers to avoid
+	 * skipping system suspend/resume callbacks for it in that case.
+	 */
+	return !has_acpi_companion(dev);
 }
 
 static void dw_i2c_plat_complete(struct device *dev)

[PATCH 6/7] PM: i2c-designware-platdrv: Optimize power management

[Rafael J. Wysocki]

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

Optimize the power management in i2c-designware-platdrv by making it
set the DPM_FLAG_SMART_SUSPEND and DPM_FLAG_LEAVE_SUSPENDED which
allows some code to be dropped from its PM callbacks.

First, setting DPM_FLAG_SMART_SUSPEND causes the intel-lpss driver
to avoid resuming i2c-designware-platdrv devices in its ->prepare
callback, so they can stay in runtime suspend after that point even
if the direct-complete feature is not used for them.

It also causes the ACPI PM domain and the PM core to avoid invoking
"late" and "noirq" suspend callbacks for these devices if they are
in runtime suspend at the beginning of the "late" phase of device
suspend during system suspend.  That guarantees dw_i2c_plat_suspend()
to be called for a device only if it is not in runtime suspend.

Moreover, it causes the device's runtime PM status to be set to
"active" after calling dw_i2c_plat_resume() for it, so the
driver doesn't need internal flags to avoid invoking either
dw_i2c_plat_suspend() or dw_i2c_plat_resume() twice in a row.

Second, setting DPM_FLAG_LEAVE_SUSPENDED enables the optimization
allowing the device to stay suspended after system resume under
suitable conditions, so again the driver doesn't need to take
care of that by itself.

Accordingly, the internal "suspended" and "skip_resume" flags
used by the driver are not necessary any more, so drop them and
simplify the driver's PM callbacks.

Additionally, notice that dw_i2c_plat_complete() only needs to
schedule runtime PM resume for the device if platform firmware
has been involved in resuming the system, so make it call
pm_resume_via_firmware() to check that.  Also make it check the
runtime PM status of the device instead of its direct_complete
flag which also works if the device remained suspended due to
the DPM_FLAG_LEAVE_SUSPENDED driver flag.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/i2c/busses/i2c-designware-core.h    |    2 -
 drivers/i2c/busses/i2c-designware-platdrv.c |   31 ++++++++++------------------
 2 files changed, 12 insertions(+), 21 deletions(-)

Index: linux-pm/drivers/i2c/busses/i2c-designware-core.h
===================================================================
--- linux-pm.orig/drivers/i2c/busses/i2c-designware-core.h
+++ linux-pm/drivers/i2c/busses/i2c-designware-core.h
@@ -280,8 +280,6 @@ struct dw_i2c_dev {
 	int			(*acquire_lock)(struct dw_i2c_dev *dev);
 	void			(*release_lock)(struct dw_i2c_dev *dev);
 	bool			pm_disabled;
-	bool			suspended;
-	bool			skip_resume;
 	void			(*disable)(struct dw_i2c_dev *dev);
 	void			(*disable_int)(struct dw_i2c_dev *dev);
 	int			(*init)(struct dw_i2c_dev *dev);
Index: linux-pm/drivers/i2c/busses/i2c-designware-platdrv.c
===================================================================
--- linux-pm.orig/drivers/i2c/busses/i2c-designware-platdrv.c
+++ linux-pm/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -42,6 +42,7 @@
 #include <linux/reset.h>
 #include <linux/sched.h>
 #include <linux/slab.h>
+#include <linux/suspend.h>
 
 #include "i2c-designware-core.h"
 
@@ -372,7 +373,10 @@ static int dw_i2c_plat_probe(struct plat
 	ACPI_COMPANION_SET(&adap->dev, ACPI_COMPANION(&pdev->dev));
 	adap->dev.of_node = pdev->dev.of_node;
 
-	dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_SMART_PREPARE);
+	dev_pm_set_driver_flags(&pdev->dev,
+				DPM_FLAG_SMART_PREPARE |
+				DPM_FLAG_SMART_SUSPEND |
+				DPM_FLAG_LEAVE_SUSPENDED);
 
 	/* The code below assumes runtime PM to be disabled. */
 	WARN_ON(pm_runtime_enabled(&pdev->dev));
@@ -448,7 +452,13 @@ static int dw_i2c_plat_prepare(struct de
 
 static void dw_i2c_plat_complete(struct device *dev)
 {
-	if (dev->power.direct_complete)
+	/*
+	 * The device can only be in runtime suspend at this point if it has not
+	 * been resumed throughout the ending system suspend/resume cycle, so if
+	 * the platform firmware might mess up with it, request the runtime PM
+	 * framework to resume it.
+	 */
+	if (pm_runtime_suspended(dev) && pm_resume_via_firmware())
 		pm_request_resume(dev);
 }
 #else
@@ -461,16 +471,9 @@ static int dw_i2c_plat_suspend(struct de
 {
 	struct dw_i2c_dev *i_dev = dev_get_drvdata(dev);
 
-	if (i_dev->suspended) {
-		i_dev->skip_resume = true;
-		return 0;
-	}
-
 	i_dev->disable(i_dev);
 	i2c_dw_plat_prepare_clk(i_dev, false);
 
-	i_dev->suspended = true;
-
 	return 0;
 }
 
@@ -478,19 +481,9 @@ static int dw_i2c_plat_resume(struct dev
 {
 	struct dw_i2c_dev *i_dev = dev_get_drvdata(dev);
 
-	if (!i_dev->suspended)
-		return 0;
-
-	if (i_dev->skip_resume) {
-		i_dev->skip_resume = false;
-		return 0;
-	}
-
 	i2c_dw_plat_prepare_clk(i_dev, true);
 	i_dev->init(i_dev);
 
-	i_dev->suspended = false;
-
 	return 0;
 }
 

[PATCH 7/7] PCI / PM: Use SMART_SUSPEND and LEAVE_SUSPENDED flags for PCIe ports

[Rafael J. Wysocki]

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

Make the PCIe port driver set DPM_FLAG_SMART_SUSPEND and
DPM_FLAG_LEAVE_SUSPENDED for the devices handled by it to benefit
from the opportunistic optimizations in the PCI layer enabled by
these flags.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/pci/pcie/portdrv_pci.c |    3 +++
 1 file changed, 3 insertions(+)

Index: linux-pm/drivers/pci/pcie/portdrv_pci.c
===================================================================
--- linux-pm.orig/drivers/pci/pcie/portdrv_pci.c
+++ linux-pm/drivers/pci/pcie/portdrv_pci.c
@@ -150,6 +150,9 @@ static int pcie_portdrv_probe(struct pci
 
 	pci_save_state(dev);
 
+	dev_pm_set_driver_flags(&dev->dev, DPM_FLAG_SMART_SUSPEND |
+					   DPM_FLAG_LEAVE_SUSPENDED);
+
 	if (pci_bridge_d3_possible(dev)) {
 		/*
 		 * Keep the port resumed 100ms to make sure things like

Re: [PATCH 7/7] PCI / PM: Use SMART_SUSPEND and LEAVE_SUSPENDED flags for PCIe ports

[Bjorn Helgaas]

On Wed, Jan 03, 2018 at 01:38:27AM +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Make the PCIe port driver set DPM_FLAG_SMART_SUSPEND and
> DPM_FLAG_LEAVE_SUSPENDED for the devices handled by it to benefit
> from the opportunistic optimizations in the PCI layer enabled by
> these flags.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

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

Please merge this along with the rest of the series.

> ---
>  drivers/pci/pcie/portdrv_pci.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> Index: linux-pm/drivers/pci/pcie/portdrv_pci.c
> ===================================================================
> --- linux-pm.orig/drivers/pci/pcie/portdrv_pci.c
> +++ linux-pm/drivers/pci/pcie/portdrv_pci.c
> @@ -150,6 +150,9 @@ static int pcie_portdrv_probe(struct pci
>  
>  	pci_save_state(dev);
>  
> +	dev_pm_set_driver_flags(&dev->dev, DPM_FLAG_SMART_SUSPEND |
> +					   DPM_FLAG_LEAVE_SUSPENDED);
> +
>  	if (pci_bridge_d3_possible(dev)) {
>  		/*
>  		 * Keep the port resumed 100ms to make sure things like
> 

Re: [PATCH 7/7] PCI / PM: Use SMART_SUSPEND and LEAVE_SUSPENDED flags for PCIe ports

[Rafael J. Wysocki]

On Thu, Jan 4, 2018 at 11:14 PM, Bjorn Helgaas <helgaas@kernel.org> wrote:
> On Wed, Jan 03, 2018 at 01:38:27AM +0100, Rafael J. Wysocki wrote:
>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>
>> Make the PCIe port driver set DPM_FLAG_SMART_SUSPEND and
>> DPM_FLAG_LEAVE_SUSPENDED for the devices handled by it to benefit
>> from the opportunistic optimizations in the PCI layer enabled by
>> these flags.
>>
>> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
>
> Please merge this along with the rest of the series.

I will, thank you!

Re: [PATCH 5/7] PM: i2c-designware-platdrv: Use DPM_FLAG_SMART_PREPARE

[Jarkko Nikula]

On 01/03/2018 02:35 AM, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Modify i2c-designware-platdrv to set DPM_FLAG_SMART_PREPARE for its
> devices and return 0 from the system suspend ->prepare callback
> if the device has an ACPI companion object in order to tell the PM
> core and middle layers to avoid skipping system suspend/resume
> callbacks for the device in that case (which may be problematic,
> because the device may be accessed during suspend and resume of
> other devices via I2C operation regions then).
> 
> Also the pm_runtime_suspended() check in dw_i2c_plat_prepare()
> is not necessary any more, because the core does it when setting
> power.direct_complete for the device, so drop it.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>   drivers/i2c/busses/i2c-designware-platdrv.c |   10 +++++++++-
>   1 file changed, 9 insertions(+), 1 deletion(-)
> 
Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>

Re: [PATCH 6/7] PM: i2c-designware-platdrv: Optimize power management

[Jarkko Nikula]

On 01/03/2018 02:37 AM, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Optimize the power management in i2c-designware-platdrv by making it
> set the DPM_FLAG_SMART_SUSPEND and DPM_FLAG_LEAVE_SUSPENDED which
> allows some code to be dropped from its PM callbacks.
> 
> First, setting DPM_FLAG_SMART_SUSPEND causes the intel-lpss driver
> to avoid resuming i2c-designware-platdrv devices in its ->prepare
> callback, so they can stay in runtime suspend after that point even
> if the direct-complete feature is not used for them.
> 
> It also causes the ACPI PM domain and the PM core to avoid invoking
> "late" and "noirq" suspend callbacks for these devices if they are
> in runtime suspend at the beginning of the "late" phase of device
> suspend during system suspend.  That guarantees dw_i2c_plat_suspend()
> to be called for a device only if it is not in runtime suspend.
> 
> Moreover, it causes the device's runtime PM status to be set to
> "active" after calling dw_i2c_plat_resume() for it, so the
> driver doesn't need internal flags to avoid invoking either
> dw_i2c_plat_suspend() or dw_i2c_plat_resume() twice in a row.
> 
> Second, setting DPM_FLAG_LEAVE_SUSPENDED enables the optimization
> allowing the device to stay suspended after system resume under
> suitable conditions, so again the driver doesn't need to take
> care of that by itself.
> 
> Accordingly, the internal "suspended" and "skip_resume" flags
> used by the driver are not necessary any more, so drop them and
> simplify the driver's PM callbacks.
> 
> Additionally, notice that dw_i2c_plat_complete() only needs to
> schedule runtime PM resume for the device if platform firmware
> has been involved in resuming the system, so make it call
> pm_resume_via_firmware() to check that.  Also make it check the
> runtime PM status of the device instead of its direct_complete
> flag which also works if the device remained suspended due to
> the DPM_FLAG_LEAVE_SUSPENDED driver flag.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>   drivers/i2c/busses/i2c-designware-core.h    |    2 -
>   drivers/i2c/busses/i2c-designware-platdrv.c |   31 ++++++++++------------------
>   2 files changed, 12 insertions(+), 21 deletions(-)
> 
This doesn't apply to linux-next due 0326f9f801b2 ("i2c: designware: 
rename i2c_dw_plat_prepare_clk to i2c_dw_prepare_clk"). It was trivial 
to fix which I did locally for testing.

Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>

Re: [PATCH 0/7] PM / core: Direct handling of DPM_FLAG_SMART_SUSPEND and DPM_FLAG_LEAVE_SUSPENDED

[Jarkko Nikula]

On 01/03/2018 02:29 AM, Rafael J. Wysocki wrote:
> Resending the original series (except for the first patch that has been
> applied already) along with some driver code changes based on them as
> requested by Ulf.
> 
> The driver patches are an intel-lpss change (already reviewed), two
> modifications of i2c-designware-platdrv (posted previously but slightly
> changed since then) and a PCIe port driver change.
> 
> At this point patches [1-3/7] are pretty much on the way in and the driver
> material depends on review comments (it is pointless to apply [4/7] without
> [5-6/7], so it depends on them in my view).
> 
> I'm sending this from a system running with all of the series applied, although
> admittedly not using i2c-designware-platdrv.  However, one of my test machines
> has this one and I haven't seen any adverse effects of these changes on it so
> far.
> 
Both i2cdetect and hexdump from touchscreen and touchpad devices were 
working fine. I used linux-next next-20180108 and patches 4-7/7.

Tested-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>

Re: [PATCH 5/7] PM: i2c-designware-platdrv: Use DPM_FLAG_SMART_PREPARE

[Wolfram Sang]

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

On Mon, Jan 08, 2018 at 04:31:49PM +0200, Jarkko Nikula wrote:
> On 01/03/2018 02:35 AM, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > 
> > Modify i2c-designware-platdrv to set DPM_FLAG_SMART_PREPARE for its
> > devices and return 0 from the system suspend ->prepare callback
> > if the device has an ACPI companion object in order to tell the PM
> > core and middle layers to avoid skipping system suspend/resume
> > callbacks for the device in that case (which may be problematic,
> > because the device may be accessed during suspend and resume of
> > other devices via I2C operation regions then).
> > 
> > Also the pm_runtime_suspended() check in dw_i2c_plat_prepare()
> > is not necessary any more, because the core does it when setting
> > power.direct_complete for the device, so drop it.
> > 
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> >   drivers/i2c/busses/i2c-designware-platdrv.c |   10 +++++++++-
> >   1 file changed, 9 insertions(+), 1 deletion(-)
> > 
> Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>

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


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

Re: [PATCH 6/7] PM: i2c-designware-platdrv: Optimize power management

[Wolfram Sang]

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

On Mon, Jan 08, 2018 at 04:31:58PM +0200, Jarkko Nikula wrote:
> On 01/03/2018 02:37 AM, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > 
> > Optimize the power management in i2c-designware-platdrv by making it
> > set the DPM_FLAG_SMART_SUSPEND and DPM_FLAG_LEAVE_SUSPENDED which
> > allows some code to be dropped from its PM callbacks.
> > 
> > First, setting DPM_FLAG_SMART_SUSPEND causes the intel-lpss driver
> > to avoid resuming i2c-designware-platdrv devices in its ->prepare
> > callback, so they can stay in runtime suspend after that point even
> > if the direct-complete feature is not used for them.
> > 
> > It also causes the ACPI PM domain and the PM core to avoid invoking
> > "late" and "noirq" suspend callbacks for these devices if they are
> > in runtime suspend at the beginning of the "late" phase of device
> > suspend during system suspend.  That guarantees dw_i2c_plat_suspend()
> > to be called for a device only if it is not in runtime suspend.
> > 
> > Moreover, it causes the device's runtime PM status to be set to
> > "active" after calling dw_i2c_plat_resume() for it, so the
> > driver doesn't need internal flags to avoid invoking either
> > dw_i2c_plat_suspend() or dw_i2c_plat_resume() twice in a row.
> > 
> > Second, setting DPM_FLAG_LEAVE_SUSPENDED enables the optimization
> > allowing the device to stay suspended after system resume under
> > suitable conditions, so again the driver doesn't need to take
> > care of that by itself.
> > 
> > Accordingly, the internal "suspended" and "skip_resume" flags
> > used by the driver are not necessary any more, so drop them and
> > simplify the driver's PM callbacks.
> > 
> > Additionally, notice that dw_i2c_plat_complete() only needs to
> > schedule runtime PM resume for the device if platform firmware
> > has been involved in resuming the system, so make it call
> > pm_resume_via_firmware() to check that.  Also make it check the
> > runtime PM status of the device instead of its direct_complete
> > flag which also works if the device remained suspended due to
> > the DPM_FLAG_LEAVE_SUSPENDED driver flag.
> > 
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> >   drivers/i2c/busses/i2c-designware-core.h    |    2 -
> >   drivers/i2c/busses/i2c-designware-platdrv.c |   31 ++++++++++------------------
> >   2 files changed, 12 insertions(+), 21 deletions(-)
> > 
> This doesn't apply to linux-next due 0326f9f801b2 ("i2c: designware: rename
> i2c_dw_plat_prepare_clk to i2c_dw_prepare_clk"). It was trivial to fix which
> I did locally for testing.
> 
> Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>

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


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

Re: [PATCH 0/7] PM / core: Direct handling of DPM_FLAG_SMART_SUSPEND and DPM_FLAG_LEAVE_SUSPENDED

[Rafael J. Wysocki]

On Monday, January 8, 2018 3:33:07 PM CET Jarkko Nikula wrote:
> On 01/03/2018 02:29 AM, Rafael J. Wysocki wrote:
> > Resending the original series (except for the first patch that has been
> > applied already) along with some driver code changes based on them as
> > requested by Ulf.
> > 
> > The driver patches are an intel-lpss change (already reviewed), two
> > modifications of i2c-designware-platdrv (posted previously but slightly
> > changed since then) and a PCIe port driver change.
> > 
> > At this point patches [1-3/7] are pretty much on the way in and the driver
> > material depends on review comments (it is pointless to apply [4/7] without
> > [5-6/7], so it depends on them in my view).
> > 
> > I'm sending this from a system running with all of the series applied, although
> > admittedly not using i2c-designware-platdrv.  However, one of my test machines
> > has this one and I haven't seen any adverse effects of these changes on it so
> > far.
> > 
> Both i2cdetect and hexdump from touchscreen and touchpad devices were 
> working fine. I used linux-next next-20180108 and patches 4-7/7.
> 
> Tested-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>

Thank you!