[PATCH v2 4/4] PM: sleep: Avoid unnecessary checks in device_prepare_smart_suspend()

Posted by Rafael J. Wysocki 10 months ago

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

Add an optimization (on top of previous changes) to avoid calling
pm_runtime_blocked(), which involves acquiring the device's PM spinlock,
for devices with no PM callbacks and runtime PM "blocked".

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/base/power/main.c    |   16 +++++++++-------
 drivers/base/power/runtime.c |    9 +++++++--
 include/linux/pm_runtime.h   |    4 ++--
 3 files changed, 18 insertions(+), 11 deletions(-)

--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -1796,16 +1796,14 @@
 
 	/*
 	 * The "smart suspend" feature is enabled for devices whose drivers ask
-	 * for it and for devices without PM callbacks unless runtime PM is
-	 * disabled and enabling it is blocked for them.
+	 * for it and for devices without PM callbacks.
 	 *
 	 * However, if "smart suspend" is not enabled for the device's parent
 	 * or any of its suppliers that take runtime PM into account, it cannot
 	 * be enabled for the device either.
 	 */
-	dev->power.smart_suspend = (dev->power.no_pm_callbacks ||
-		dev_pm_test_driver_flags(dev, DPM_FLAG_SMART_SUSPEND)) &&
-		!pm_runtime_blocked(dev);
+	dev->power.smart_suspend = dev->power.no_pm_callbacks ||
+		dev_pm_test_driver_flags(dev, DPM_FLAG_SMART_SUSPEND);
 
 	if (!dev_pm_smart_suspend(dev))
 		return;
@@ -1843,6 +1841,7 @@
 static int device_prepare(struct device *dev, pm_message_t state)
 {
 	int (*callback)(struct device *) = NULL;
+	bool no_runtime_pm;
 	int ret = 0;
 
 	/*
@@ -1858,7 +1857,7 @@
 	 * suspend-resume cycle is complete, so prepare to trigger a warning on
 	 * subsequent attempts to enable it.
 	 */
-	pm_runtime_block_if_disabled(dev);
+	no_runtime_pm = pm_runtime_block_if_disabled(dev);
 
 	if (dev->power.syscore)
 		return 0;
@@ -1893,7 +1892,10 @@
 		pm_runtime_put(dev);
 		return ret;
 	}
-	device_prepare_smart_suspend(dev);
+	/* Do not enable "smart suspend" for devices without runtime PM. */
+	if (!no_runtime_pm)
+		device_prepare_smart_suspend(dev);
+
 	/*
 	 * A positive return value from ->prepare() means "this device appears
 	 * to be runtime-suspended and its state is fine, so if it really is
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -1460,14 +1460,19 @@
 }
 EXPORT_SYMBOL_GPL(pm_runtime_barrier);
 
-void pm_runtime_block_if_disabled(struct device *dev)
+bool pm_runtime_block_if_disabled(struct device *dev)
 {
+	bool ret;
+
 	spin_lock_irq(&dev->power.lock);
 
-	if (dev->power.disable_depth && dev->power.last_status == RPM_INVALID)
+	ret = dev->power.disable_depth && dev->power.last_status == RPM_INVALID;
+	if (ret)
 		dev->power.last_status = RPM_BLOCKED;
 
 	spin_unlock_irq(&dev->power.lock);
+
+	return ret;
 }
 
 void pm_runtime_unblock(struct device *dev)
--- a/include/linux/pm_runtime.h
+++ b/include/linux/pm_runtime.h
@@ -77,7 +77,7 @@
 extern int pm_schedule_suspend(struct device *dev, unsigned int delay);
 extern int __pm_runtime_set_status(struct device *dev, unsigned int status);
 extern int pm_runtime_barrier(struct device *dev);
-extern void pm_runtime_block_if_disabled(struct device *dev);
+extern bool pm_runtime_block_if_disabled(struct device *dev);
 extern void pm_runtime_unblock(struct device *dev);
 extern void pm_runtime_enable(struct device *dev);
 extern void __pm_runtime_disable(struct device *dev, bool check_resume);
@@ -274,7 +274,7 @@
 static inline int __pm_runtime_set_status(struct device *dev,
 					    unsigned int status) { return 0; }
 static inline int pm_runtime_barrier(struct device *dev) { return 0; }
-static inline void pm_runtime_block_if_disabled(struct device *dev) {}
+static inline bool pm_runtime_block_if_disabled(struct device *dev) { return true; }
 static inline void pm_runtime_unblock(struct device *dev) {}
 static inline void pm_runtime_enable(struct device *dev) {}
 static inline void __pm_runtime_disable(struct device *dev, bool c) {}

Re: [PATCH v2 4/4] PM: sleep: Avoid unnecessary checks in device_prepare_smart_suspend()

Posted by Ulf Hansson 9 months, 2 weeks ago

On Tue, 18 Feb 2025 at 21:20, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Add an optimization (on top of previous changes) to avoid calling
> pm_runtime_blocked(), which involves acquiring the device's PM spinlock,
> for devices with no PM callbacks and runtime PM "blocked".
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>

Kind regards
Uffe

> ---
>  drivers/base/power/main.c    |   16 +++++++++-------
>  drivers/base/power/runtime.c |    9 +++++++--
>  include/linux/pm_runtime.h   |    4 ++--
>  3 files changed, 18 insertions(+), 11 deletions(-)
>
> --- a/drivers/base/power/main.c
> +++ b/drivers/base/power/main.c
> @@ -1796,16 +1796,14 @@
>
>         /*
>          * The "smart suspend" feature is enabled for devices whose drivers ask
> -        * for it and for devices without PM callbacks unless runtime PM is
> -        * disabled and enabling it is blocked for them.
> +        * for it and for devices without PM callbacks.
>          *
>          * However, if "smart suspend" is not enabled for the device's parent
>          * or any of its suppliers that take runtime PM into account, it cannot
>          * be enabled for the device either.
>          */
> -       dev->power.smart_suspend = (dev->power.no_pm_callbacks ||
> -               dev_pm_test_driver_flags(dev, DPM_FLAG_SMART_SUSPEND)) &&
> -               !pm_runtime_blocked(dev);
> +       dev->power.smart_suspend = dev->power.no_pm_callbacks ||
> +               dev_pm_test_driver_flags(dev, DPM_FLAG_SMART_SUSPEND);
>
>         if (!dev_pm_smart_suspend(dev))
>                 return;
> @@ -1843,6 +1841,7 @@
>  static int device_prepare(struct device *dev, pm_message_t state)
>  {
>         int (*callback)(struct device *) = NULL;
> +       bool no_runtime_pm;
>         int ret = 0;
>
>         /*
> @@ -1858,7 +1857,7 @@
>          * suspend-resume cycle is complete, so prepare to trigger a warning on
>          * subsequent attempts to enable it.
>          */
> -       pm_runtime_block_if_disabled(dev);
> +       no_runtime_pm = pm_runtime_block_if_disabled(dev);
>
>         if (dev->power.syscore)
>                 return 0;
> @@ -1893,7 +1892,10 @@
>                 pm_runtime_put(dev);
>                 return ret;
>         }
> -       device_prepare_smart_suspend(dev);
> +       /* Do not enable "smart suspend" for devices without runtime PM. */
> +       if (!no_runtime_pm)
> +               device_prepare_smart_suspend(dev);
> +
>         /*
>          * A positive return value from ->prepare() means "this device appears
>          * to be runtime-suspended and its state is fine, so if it really is
> --- a/drivers/base/power/runtime.c
> +++ b/drivers/base/power/runtime.c
> @@ -1460,14 +1460,19 @@
>  }
>  EXPORT_SYMBOL_GPL(pm_runtime_barrier);
>
> -void pm_runtime_block_if_disabled(struct device *dev)
> +bool pm_runtime_block_if_disabled(struct device *dev)
>  {
> +       bool ret;
> +
>         spin_lock_irq(&dev->power.lock);
>
> -       if (dev->power.disable_depth && dev->power.last_status == RPM_INVALID)
> +       ret = dev->power.disable_depth && dev->power.last_status == RPM_INVALID;
> +       if (ret)
>                 dev->power.last_status = RPM_BLOCKED;
>
>         spin_unlock_irq(&dev->power.lock);
> +
> +       return ret;
>  }
>
>  void pm_runtime_unblock(struct device *dev)
> --- a/include/linux/pm_runtime.h
> +++ b/include/linux/pm_runtime.h
> @@ -77,7 +77,7 @@
>  extern int pm_schedule_suspend(struct device *dev, unsigned int delay);
>  extern int __pm_runtime_set_status(struct device *dev, unsigned int status);
>  extern int pm_runtime_barrier(struct device *dev);
> -extern void pm_runtime_block_if_disabled(struct device *dev);
> +extern bool pm_runtime_block_if_disabled(struct device *dev);
>  extern void pm_runtime_unblock(struct device *dev);
>  extern void pm_runtime_enable(struct device *dev);
>  extern void __pm_runtime_disable(struct device *dev, bool check_resume);
> @@ -274,7 +274,7 @@
>  static inline int __pm_runtime_set_status(struct device *dev,
>                                             unsigned int status) { return 0; }
>  static inline int pm_runtime_barrier(struct device *dev) { return 0; }
> -static inline void pm_runtime_block_if_disabled(struct device *dev) {}
> +static inline bool pm_runtime_block_if_disabled(struct device *dev) { return true; }
>  static inline void pm_runtime_unblock(struct device *dev) {}
>  static inline void pm_runtime_enable(struct device *dev) {}
>  static inline void __pm_runtime_disable(struct device *dev, bool c) {}
>
>
>