All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ulf Hansson <ulf.hansson@linaro.org>
To: Wolfram Sang <wsa@the-dreams.de>,
	"Rafael J . Wysocki" <rjw@rjwysocki.net>,
	Len Brown <lenb@kernel.org>,
	linux-acpi@vger.kernel.org, linux-pm@vger.kernel.org
Cc: Kevin Hilman <khilman@kernel.org>,
	Jarkko Nikula <jarkko.nikula@linux.intel.com>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Mika Westerberg <mika.westerberg@linux.intel.com>,
	Jisheng Zhang <jszhang@marvell.com>,
	John Stultz <john.stultz@linaro.org>,
	Guodong Xu <guodong.xu@linaro.org>,
	Sumit Semwal <sumit.semwal@linaro.org>,
	Haojian Zhuang <haojian.zhuang@linaro.org>,
	linux-arm-kernel@lists.infradead.org, linux-i2c@vger.kernel.org,
	Ulf Hansson <ulf.hansson@linaro.org>
Subject: [PATCH v2 9/9] i2c: designware: Deploy the runtime PM centric approach for system sleep
Date: Wed, 23 Aug 2017 16:42:09 +0200	[thread overview]
Message-ID: <1503499329-28834-10-git-send-email-ulf.hansson@linaro.org> (raw)
In-Reply-To: <1503499329-28834-1-git-send-email-ulf.hansson@linaro.org>

Currently the device is runtime resumed in the i2c-dw-plat driver's
->suspend() callback, which is needed to manage system sleep properly.

The particular reason for the runtime resume is because the PM core may
unset the direct_complete flag for a parent device, in case its child
device are being system suspended before. This leads to that the
i2c-dw-plat driver's ->suspend() callback can be invoked when the device is
runtime suspended.

Runtime resuming the device in this scenario may be unnecessary, in case
when the device is already in its proper lower power state for system
sleep. This behaviour increases the time it takes to put the device into
low power state, but means also a waste of power.

Let's fix the behaviour by deploying the runtime PM centric path for system
sleep, via assigning the pm_runtime_force_suspend|resume() helpers as the
system sleep callbacks for the i2c-dw-plat driver.

To deploy this, we must also make sure that the direct_complete path isn't
used for the device. Therefore we drop the ->prepare() callback from the
i2c-dw-plat driver and we inform the ACPI PM domain to not try out the
direct_complete path for the device, as that is the default behaviour for
the ACPI PM domain.

Along with the runtime PM centric path, we also get some additional
improvements of the behaviour during system sleep. More precisely:

*)
In case the device is/gets runtime resumed before the device_suspend()
phase is entered, the PM core doesn't run the direct_complete path for the
device during system sleep. During system resume, this lead to that the
device will always be brought back to full power when the i2c-dw-plat
driver's ->resume() callback is invoked. This may not be necessary, thus
increasing the resume time for the device and wasting power.

In the runtime PM centric path, the pm_runtime_force_resume() helper takes
better care, as it resumes the device only in case it's really needed.
Normally this means it can be postponed to be dealt with via regular calls
to runtime PM (pm_runtime_get*()) instead. In other words, the device
remains in its low power state until someone request a new i2c transfer,
whenever that may be.

**)
In case the device is runtime suspended when the ->prepare() callback is
invoked, the direct_complete path is tried by the PM core. If it succeeds,
this leads to disabling runtime PM for the device in the device_suspend()
phase. That is a problem in cases when others may need the device to be
operational, as to be able send i2c transfers during the entire
device_suspend() phase.

By using the runtime PM centric path, we postpones runtime PM to be
disabled for the device by the PM core to the regular device_suspend_late()
phase.

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

Changes in v2:
	- Rebased.
	- Clarify changelog.

---
 drivers/i2c/busses/i2c-designware-platdrv.c | 27 ++++++---------------------
 1 file changed, 6 insertions(+), 21 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index fc6b99f..e0cbf5e 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -372,6 +372,7 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
 	if (ret)
 		goto exit_probe;
 
+	acpi_dev_disable_direct_complete(&pdev->dev);
 	return ret;
 
 exit_probe:
@@ -387,6 +388,7 @@ static int dw_i2c_plat_remove(struct platform_device *pdev)
 {
 	struct dw_i2c_dev *dev = platform_get_drvdata(pdev);
 
+	acpi_dev_enable_direct_complete(&pdev->dev);
 	pm_runtime_get_sync(&pdev->dev);
 
 	i2c_del_adapter(&dev->adapter);
@@ -413,15 +415,6 @@ static const struct of_device_id dw_i2c_of_match[] = {
 MODULE_DEVICE_TABLE(of, dw_i2c_of_match);
 #endif
 
-#ifdef CONFIG_PM_SLEEP
-static int dw_i2c_plat_prepare(struct device *dev)
-{
-	return pm_runtime_suspended(dev);
-}
-#else
-#define dw_i2c_plat_prepare	NULL
-#endif
-
 #ifdef CONFIG_PM
 static int dw_i2c_plat_runtime_suspend(struct device *dev)
 {
@@ -434,7 +427,7 @@ static int dw_i2c_plat_runtime_suspend(struct device *dev)
 	return 0;
 }
 
-static int dw_i2c_plat_resume(struct device *dev)
+static int dw_i2c_plat_runtime_resume(struct device *dev)
 {
 	struct platform_device *pdev = to_platform_device(dev);
 	struct dw_i2c_dev *i_dev = platform_get_drvdata(pdev);
@@ -445,19 +438,11 @@ static int dw_i2c_plat_resume(struct device *dev)
 	return 0;
 }
 
-#ifdef CONFIG_PM_SLEEP
-static int dw_i2c_plat_suspend(struct device *dev)
-{
-	pm_runtime_resume(dev);
-	return dw_i2c_plat_runtime_suspend(dev);
-}
-#endif
-
 static const struct dev_pm_ops dw_i2c_dev_pm_ops = {
-	.prepare = dw_i2c_plat_prepare,
-	SET_SYSTEM_SLEEP_PM_OPS(dw_i2c_plat_suspend, dw_i2c_plat_resume)
+	SET_LATE_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+				     pm_runtime_force_resume)
 	SET_RUNTIME_PM_OPS(dw_i2c_plat_runtime_suspend,
-			   dw_i2c_plat_resume,
+			   dw_i2c_plat_runtime_resume,
 			   NULL)
 };
 
-- 
2.7.4

WARNING: multiple messages have this Message-ID (diff)
From: ulf.hansson@linaro.org (Ulf Hansson)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 9/9] i2c: designware: Deploy the runtime PM centric approach for system sleep
Date: Wed, 23 Aug 2017 16:42:09 +0200	[thread overview]
Message-ID: <1503499329-28834-10-git-send-email-ulf.hansson@linaro.org> (raw)
In-Reply-To: <1503499329-28834-1-git-send-email-ulf.hansson@linaro.org>

Currently the device is runtime resumed in the i2c-dw-plat driver's
->suspend() callback, which is needed to manage system sleep properly.

The particular reason for the runtime resume is because the PM core may
unset the direct_complete flag for a parent device, in case its child
device are being system suspended before. This leads to that the
i2c-dw-plat driver's ->suspend() callback can be invoked when the device is
runtime suspended.

Runtime resuming the device in this scenario may be unnecessary, in case
when the device is already in its proper lower power state for system
sleep. This behaviour increases the time it takes to put the device into
low power state, but means also a waste of power.

Let's fix the behaviour by deploying the runtime PM centric path for system
sleep, via assigning the pm_runtime_force_suspend|resume() helpers as the
system sleep callbacks for the i2c-dw-plat driver.

To deploy this, we must also make sure that the direct_complete path isn't
used for the device. Therefore we drop the ->prepare() callback from the
i2c-dw-plat driver and we inform the ACPI PM domain to not try out the
direct_complete path for the device, as that is the default behaviour for
the ACPI PM domain.

Along with the runtime PM centric path, we also get some additional
improvements of the behaviour during system sleep. More precisely:

*)
In case the device is/gets runtime resumed before the device_suspend()
phase is entered, the PM core doesn't run the direct_complete path for the
device during system sleep. During system resume, this lead to that the
device will always be brought back to full power when the i2c-dw-plat
driver's ->resume() callback is invoked. This may not be necessary, thus
increasing the resume time for the device and wasting power.

In the runtime PM centric path, the pm_runtime_force_resume() helper takes
better care, as it resumes the device only in case it's really needed.
Normally this means it can be postponed to be dealt with via regular calls
to runtime PM (pm_runtime_get*()) instead. In other words, the device
remains in its low power state until someone request a new i2c transfer,
whenever that may be.

**)
In case the device is runtime suspended when the ->prepare() callback is
invoked, the direct_complete path is tried by the PM core. If it succeeds,
this leads to disabling runtime PM for the device in the device_suspend()
phase. That is a problem in cases when others may need the device to be
operational, as to be able send i2c transfers during the entire
device_suspend() phase.

By using the runtime PM centric path, we postpones runtime PM to be
disabled for the device by the PM core to the regular device_suspend_late()
phase.

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

Changes in v2:
	- Rebased.
	- Clarify changelog.

---
 drivers/i2c/busses/i2c-designware-platdrv.c | 27 ++++++---------------------
 1 file changed, 6 insertions(+), 21 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index fc6b99f..e0cbf5e 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -372,6 +372,7 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
 	if (ret)
 		goto exit_probe;
 
+	acpi_dev_disable_direct_complete(&pdev->dev);
 	return ret;
 
 exit_probe:
@@ -387,6 +388,7 @@ static int dw_i2c_plat_remove(struct platform_device *pdev)
 {
 	struct dw_i2c_dev *dev = platform_get_drvdata(pdev);
 
+	acpi_dev_enable_direct_complete(&pdev->dev);
 	pm_runtime_get_sync(&pdev->dev);
 
 	i2c_del_adapter(&dev->adapter);
@@ -413,15 +415,6 @@ static const struct of_device_id dw_i2c_of_match[] = {
 MODULE_DEVICE_TABLE(of, dw_i2c_of_match);
 #endif
 
-#ifdef CONFIG_PM_SLEEP
-static int dw_i2c_plat_prepare(struct device *dev)
-{
-	return pm_runtime_suspended(dev);
-}
-#else
-#define dw_i2c_plat_prepare	NULL
-#endif
-
 #ifdef CONFIG_PM
 static int dw_i2c_plat_runtime_suspend(struct device *dev)
 {
@@ -434,7 +427,7 @@ static int dw_i2c_plat_runtime_suspend(struct device *dev)
 	return 0;
 }
 
-static int dw_i2c_plat_resume(struct device *dev)
+static int dw_i2c_plat_runtime_resume(struct device *dev)
 {
 	struct platform_device *pdev = to_platform_device(dev);
 	struct dw_i2c_dev *i_dev = platform_get_drvdata(pdev);
@@ -445,19 +438,11 @@ static int dw_i2c_plat_resume(struct device *dev)
 	return 0;
 }
 
-#ifdef CONFIG_PM_SLEEP
-static int dw_i2c_plat_suspend(struct device *dev)
-{
-	pm_runtime_resume(dev);
-	return dw_i2c_plat_runtime_suspend(dev);
-}
-#endif
-
 static const struct dev_pm_ops dw_i2c_dev_pm_ops = {
-	.prepare = dw_i2c_plat_prepare,
-	SET_SYSTEM_SLEEP_PM_OPS(dw_i2c_plat_suspend, dw_i2c_plat_resume)
+	SET_LATE_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+				     pm_runtime_force_resume)
 	SET_RUNTIME_PM_OPS(dw_i2c_plat_runtime_suspend,
-			   dw_i2c_plat_resume,
+			   dw_i2c_plat_runtime_resume,
 			   NULL)
 };
 
-- 
2.7.4

  parent reply	other threads:[~2017-08-23 14:42 UTC|newest]

Thread overview: 94+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-23 14:42 [PATCH v2 0/9] PM / ACPI / i2c: Deploy runtime PM centric path for system sleep Ulf Hansson
2017-08-23 14:42 ` Ulf Hansson
2017-08-23 14:42 ` [PATCH v2 1/9] PM / ACPI: Restore acpi_subsys_complete() Ulf Hansson
2017-08-23 14:42   ` Ulf Hansson
2017-08-23 22:41   ` Rafael J. Wysocki
2017-08-23 22:41     ` Rafael J. Wysocki
2017-08-23 14:42 ` [PATCH v2 2/9] PM / Sleep: Remove pm_complete_with_resume_check() Ulf Hansson
2017-08-23 14:42   ` Ulf Hansson
2017-08-23 14:42 ` [PATCH v2 3/9] PM / ACPI: Split code validating need for runtime resume in ->prepare() Ulf Hansson
2017-08-23 14:42   ` Ulf Hansson
2017-08-23 14:42 ` [PATCH v2 4/9] PM / ACPI: Split acpi_lpss_suspend_late|resume_early() Ulf Hansson
2017-08-23 14:42   ` Ulf Hansson
2017-08-23 14:42 ` [PATCH v2 5/9] PM / ACPI: Provide option to disable direct_complete for ACPI devices Ulf Hansson
2017-08-23 14:42   ` Ulf Hansson
2017-08-23 23:39   ` Rafael J. Wysocki
2017-08-23 23:39     ` Rafael J. Wysocki
2017-08-24  0:13     ` Rafael J. Wysocki
2017-08-24  0:13       ` Rafael J. Wysocki
2017-08-24  0:20       ` Rafael J. Wysocki
2017-08-24  0:20         ` Rafael J. Wysocki
2017-08-24  1:03         ` Rafael J. Wysocki
2017-08-24  1:03           ` Rafael J. Wysocki
2017-08-24  9:15           ` Ulf Hansson
2017-08-24  9:15             ` Ulf Hansson
2017-08-24 16:35             ` Rafael J. Wysocki
2017-08-24 16:35               ` Rafael J. Wysocki
2017-08-24 21:50               ` Rafael J. Wysocki
2017-08-24 21:50                 ` Rafael J. Wysocki
2017-08-25 13:42                 ` Rafael J. Wysocki
2017-08-25 13:42                   ` Rafael J. Wysocki
2017-08-28  1:30                   ` Rafael J. Wysocki
2017-08-28  1:30                     ` Rafael J. Wysocki
2017-08-28  8:31                     ` Ulf Hansson
2017-08-28  8:31                       ` Ulf Hansson
2017-08-28 12:39                       ` Rafael J. Wysocki
2017-08-28 12:39                         ` Rafael J. Wysocki
2017-08-28 12:54                         ` Ulf Hansson
2017-08-28 12:54                           ` Ulf Hansson
2017-08-28 13:40                           ` Rafael J. Wysocki
2017-08-28 13:40                             ` Rafael J. Wysocki
2017-08-28 14:24                             ` Ulf Hansson
2017-08-28 14:24                               ` Ulf Hansson
2017-08-28 21:14                               ` Rafael J. Wysocki
2017-08-28 21:14                                 ` Rafael J. Wysocki
2017-08-25  9:28               ` Ulf Hansson
2017-08-25  9:28                 ` Ulf Hansson
2017-08-25 12:23                 ` Rafael J. Wysocki
2017-08-25 12:23                   ` Rafael J. Wysocki
2017-08-24  8:19     ` Ulf Hansson
2017-08-24  8:19       ` Ulf Hansson
2017-08-24 14:57       ` Rafael J. Wysocki
2017-08-24 14:57         ` Rafael J. Wysocki
2017-08-25  9:04         ` Ulf Hansson
2017-08-25  9:04           ` Ulf Hansson
2017-08-23 14:42 ` [PATCH v2 6/9] PM / ACPI: Enable the runtime PM centric approach for system sleep Ulf Hansson
2017-08-23 14:42   ` Ulf Hansson
2017-08-23 14:42 ` [PATCH v2 7/9] PM / ACPI: Avoid runtime resuming device in acpi_subsys_suspend|freeze() Ulf Hansson
2017-08-23 14:42   ` Ulf Hansson
2017-08-23 14:42 ` [PATCH v2 8/9] i2c: designware: Don't resume device in the ->complete() callback Ulf Hansson
2017-08-23 14:42   ` Ulf Hansson
2017-08-23 14:42 ` Ulf Hansson [this message]
2017-08-23 14:42   ` [PATCH v2 9/9] i2c: designware: Deploy the runtime PM centric approach for system sleep Ulf Hansson
2017-08-25 14:10 ` [PATCH v2 0/9] PM / ACPI / i2c: Deploy runtime PM centric path " Jarkko Nikula
2017-08-25 14:10   ` Jarkko Nikula
2017-08-29  0:18 ` [PATCH 0/3] PM / ACPI / i2c: Runtime PM aware system sleep handling Rafael J. Wysocki
2017-08-29  0:18   ` Rafael J. Wysocki
2017-08-29  0:20   ` [PATCH 1/3] PM / core: Add SAFE_SUSPEND driver flag Rafael J. Wysocki
2017-08-29  0:20     ` Rafael J. Wysocki
2017-08-29 14:57     ` Ulf Hansson
2017-08-29 14:57       ` Ulf Hansson
2017-08-29 15:02       ` Rafael J. Wysocki
2017-08-29 15:02         ` Rafael J. Wysocki
2017-08-29  0:59   ` [PATCH 2/3] PM / ACPI: Use SAFE_SUSPEND in the generic ACPI PM domain Rafael J. Wysocki
2017-08-29  0:59     ` Rafael J. Wysocki
2017-08-29  0:59   ` [PATCH 3/3] PM: i2c-designware-platdrv: System sleep handling rework Rafael J. Wysocki
2017-08-29  0:59     ` Rafael J. Wysocki
2017-08-29 16:38     ` Rafael J. Wysocki
2017-08-29 16:38       ` Rafael J. Wysocki
2017-08-29 16:40       ` Rafael J. Wysocki
2017-08-29 16:40         ` Rafael J. Wysocki
2017-08-29 10:29   ` [PATCH 0/3] PM / ACPI / i2c: Runtime PM aware system sleep handling Johannes Stezenbach
2017-08-29 10:29     ` Johannes Stezenbach
2017-08-29 11:44     ` Ulf Hansson
2017-08-29 11:44       ` Ulf Hansson
2017-08-29 13:53       ` Johannes Stezenbach
2017-08-29 13:53         ` Johannes Stezenbach
2017-08-29 14:43       ` Rafael J. Wysocki
2017-08-29 14:43         ` Rafael J. Wysocki
2017-08-29 15:05         ` Ulf Hansson
2017-08-29 15:05           ` Ulf Hansson
2017-08-29 16:44           ` Rafael J. Wysocki
2017-08-29 16:44             ` Rafael J. Wysocki
2017-08-29 14:49     ` Rafael J. Wysocki
2017-08-29 14:49       ` Rafael J. Wysocki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1503499329-28834-10-git-send-email-ulf.hansson@linaro.org \
    --to=ulf.hansson@linaro.org \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=guodong.xu@linaro.org \
    --cc=haojian.zhuang@linaro.org \
    --cc=jarkko.nikula@linux.intel.com \
    --cc=john.stultz@linaro.org \
    --cc=jszhang@marvell.com \
    --cc=khilman@kernel.org \
    --cc=lenb@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=mika.westerberg@linux.intel.com \
    --cc=rjw@rjwysocki.net \
    --cc=sumit.semwal@linaro.org \
    --cc=wsa@the-dreams.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.