MMC/SDIO: enable SDIO device to suspend/resume asynchronously
PATCH v3 1/1 MMC/SDIO: enable SDIO device to suspend/resume asynchronously
Now, PM core supports asynchronous suspend/resume mode for devices during system suspend/resume, and the power state transition of one device may be completed in separate kernel thread. PM core ensures all power state transition timing dependency between devices. This patch enables SDIO card and function devices to suspend/resume asynchronously. This will take advantage of multicore and improve system suspend/resume speed. After enabling the SDIO devices and all their child devices to suspend/resume asynchronously on ASUS T100TA, the system suspend-to-idle time is reduced from 1645ms to 1119ms, and the system resume time is reduced from 940ms to 918ms.
---
Changes in v3:
- Add test result in commit message
drivers/mmc/core/sdio.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)
drivers/mmc/core/sdio.c +4 -0
@@ -1113,6 +1113,8 @@ int mmc_attach_sdio(struct mmc_host *host)pm_runtime_enable (&card->dev); } +device_enable_async_suspend (&card->dev); +/* * The number of functions on the card is encoded inside * the ocr.
@@ -1133,6 +1135,8 @@ int mmc_attach_sdio(struct mmc_host *host)*/ if (host->caps &MMC_CAP_POWER_OFF_CARD )pm_runtime_enable (&card->sdio_func[i]->dev); + +device_enable_async_suspend (&card->sdio_func[i]->dev); }/*
-- 1.7.1
Quoted 11 lines · Zhonghui
Now, PM core supports asynchronous suspend/resume mode for devices during system suspend/resume, and the power state transition of one device may be completed in separate kernel thread. PM core ensures all power state transition timing dependency between devices. This patch enables SDIO card and function devices to suspend/resume asynchronously. This will take advantage of multicore and improve system suspend/resume speed. After enabling the SDIO devices and all their child devices to suspend/resume asynchronously on ASUS T100TA, the system suspend-to-idle time is reduced from 1645ms to 1119ms, and the system resume time is reduced from 940ms to 918ms.
Quoted 1 line
Signed-off-by Zhonghui Fu <[email protected]>
I think this is an interesting change, but I wonder if you really understand how this affects the order of how devices may be suspended/resumed?
Also, I believe you didn't answer my question for the earlier version of the patch, so let me try again.
There are a strict dependency chain when suspending/resuming devices that must be maintained. Currently this is controlled via device registration/probe order.
An SDIO func driver/device must always be suspended *before* the SDIO card device. Additionally the corresponding MMC host, must be suspended after the SDIO card device. Vice verse applies to the resume sequence.
As this patch enables asynchronous suspend, I am worried that it will break this dependency chain. What do you think?
Kind regards
Ulf Hansson
Quoted 31 lines
---
Changes in v3:
- Add test result in commit messagedrivers/mmc/core/sdio.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)drivers/mmc/core/sdio.c +4
-1@@ -1113,6 +1113,8 @@ int mmc_attach_sdio(struct mmc_host *host)pm_runtime_enable (&card->dev); } +device_enable_async_suspend (&card->dev); +/* * The number of functions on the card is encoded inside * the ocr. @@ -1133,6 +1135,8 @@ int mmc_attach_sdio(struct mmc_host *host)*/ if (host->caps &MMC_CAP_POWER_OFF_CARD )pm_runtime_enable (&card->sdio_func[i]->dev); + +device_enable_async_suspend (&card->sdio_func[i]->dev); }/* -- 1.7.1
Quoted 24 lines · Ulf Hansson
Quoted 11 lines · Zhonghui
Now, PM core supports asynchronous suspend/resume mode for devices during system suspend/resume, and the power state transition of one device may be completed in separate kernel thread. PM core ensures all power state transition timing dependency between devices. This patch enables SDIO card and function devices to suspend/resume asynchronously. This will take advantage of multicore and improve system suspend/resume speed. After enabling the SDIO devices and all their child devices to suspend/resume asynchronously on ASUS T100TA, the system suspend-to-idle time is reduced from 1645ms to 1119ms, and the system resume time is reduced from 940ms to 918ms.
Quoted 1 line
Signed-off-by Zhonghui Fu <[email protected]>I think this is an interesting change, but I wonder if you really understand how this affects the order of how devices may be suspended/resumed?
Also, I believe you didn't answer my question for the earlier version of the patch, so let me try again.
There are a strict dependency chain when suspending/resuming devices that must be maintained. Currently this is controlled via device registration/probe order.
Quoted 7 lines
An SDIO func driver/device must always be suspended *before* the SDIO card device. Additionally the corresponding MMC host, must be suspended after the SDIO card device. Vice verse applies to the resume sequence.
As this patch enables asynchronous suspend, I am worried that it will break this dependency chain. What do you think?
After enabling asynchronous suspend/resume, PM core still ensures the strict suspend/resume dependency between child and parent devices - child must be suspended before its parent, and parent must be resumed before its child. SDIO function is child of SDIO card, and SDIO card is child of MMC host, and MMC host is child of MMC controller. So the dependency chain is not broken. Actually, many devices have been using asynchronous suspend/resume mode now.
Thanks,
Zhonghui
Quoted 35 lines
Kind regards
Ulf HanssonQuoted 31 lines
---
Changes in v3:
- Add test result in commit messagedrivers/mmc/core/sdio.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)drivers/mmc/core/sdio.c +4
-1@@ -1113,6 +1113,8 @@ int mmc_attach_sdio(struct mmc_host *host)pm_runtime_enable (&card->dev); } +device_enable_async_suspend (&card->dev); +/* * The number of functions on the card is encoded inside * the ocr. @@ -1133,6 +1135,8 @@ int mmc_attach_sdio(struct mmc_host *host)*/ if (host->caps &MMC_CAP_POWER_OFF_CARD )pm_runtime_enable (&card->sdio_func[i]->dev); + +device_enable_async_suspend (&card->sdio_func[i]->dev); }/* -- 1.7.1
Quoted 26 lines · Zhonghui
Quoted 24 lines · Ulf Hansson
Quoted 11 lines · Zhonghui
Now, PM core supports asynchronous suspend/resume mode for devices during system suspend/resume, and the power state transition of one device may be completed in separate kernel thread. PM core ensures all power state transition timing dependency between devices. This patch enables SDIO card and function devices to suspend/resume asynchronously. This will take advantage of multicore and improve system suspend/resume speed. After enabling the SDIO devices and all their child devices to suspend/resume asynchronously on ASUS T100TA, the system suspend-to-idle time is reduced from 1645ms to 1119ms, and the system resume time is reduced from 940ms to 918ms.
Quoted 1 line
Signed-off-by Zhonghui Fu <[email protected]>I think this is an interesting change, but I wonder if you really understand how this affects the order of how devices may be suspended/resumed?
Also, I believe you didn't answer my question for the earlier version of the patch, so let me try again.
There are a strict dependency chain when suspending/resuming devices that must be maintained. Currently this is controlled via device registration/probe order.
Quoted 8 lines
Quoted 7 lines
An SDIO func driver/device must always be suspended *before* the SDIO card device. Additionally the corresponding MMC host, must be suspended after the SDIO card device. Vice verse applies to the resume sequence.
As this patch enables asynchronous suspend, I am worried that it will break this dependency chain. What do you think?
After enabling asynchronous suspend/resume, PM core still ensures the strict suspend/resume dependency between child and parent devices - child must be suspended before its parent, and parent must be resumed before its child. SDIO function is child of SDIO card, and SDIO card is child of MMC host, and MMC host is child of MMC controller. So the dependency chain is not broken. Actually, many devices have been using asynchronous suspend/resume mode now.
Any comments are welcome.
Thanks,
Zhonghui
Quoted 37 lines
Thanks,
ZhonghuiQuoted 34 lines
Kind regards
Ulf HanssonQuoted 31 lines
---
Changes in v3:
- Add test result in commit messagedrivers/mmc/core/sdio.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)drivers/mmc/core/sdio.c +4
-1@@ -1113,6 +1113,8 @@ int mmc_attach_sdio(struct mmc_host *host)pm_runtime_enable (&card->dev); } +device_enable_async_suspend (&card->dev); +/* * The number of functions on the card is encoded inside * the ocr. @@ -1133,6 +1135,8 @@ int mmc_attach_sdio(struct mmc_host *host)*/ if (host->caps &MMC_CAP_POWER_OFF_CARD )pm_runtime_enable (&card->sdio_func[i]->dev); + +device_enable_async_suspend (&card->sdio_func[i]->dev); }/* -- 1.7.1
Quoted 8 lines · Zhonghui
Quoted 5 lines · Ulf Hansson
Quoted 4 lines · Zhonghui
Now, PM core supports asynchronous suspend/resume mode for devices during system suspend/resume, and the power state transition of one device may be completed in separate kernel thread. PM core ensures all power state transition timing dependency between devices. This
What "timing dependency"?
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
patch enables SDIO card and function devices to suspend/resume asynchronously. This will take advantage of multicore and improve system suspend/resume speed. After enabling the SDIO devices and all their child devices to suspend/resume asynchronously on ASUS T100TA, the system suspend-to-idle time is reduced from 1645ms to 1119ms, and the system resume time is reduced from 940ms to 918ms.
Are these improvements achieved by $subject patch on its own or you need below patches:
[PATCH v3] mmc: enable mmc host device to suspend/resume asynchronously [1]
[PATCH v3] mmc/sdhci-acpi: enable sdhci-acpi device to suspend/resume asynchronously [2]
Depending if you have SD/(e)MMC card slot(s), the below patch might affect your results. So it might be a good idea to re-run your test to get some fresh data.
[PATCH 1/2] mmc: core: Make runtime resume default behavior for MMC/SD [3]
Quoted 13 lines
Quoted 13 lines
Quoted 2 lines
Signed-off-by Zhonghui Fu <[email protected]>I think this is an interesting change, but I wonder if you really understand how this affects the order of how devices may be suspended/resumed?
Also, I believe you didn't answer my question for the earlier version of the patch, so let me try again.
There are a strict dependency chain when suspending/resuming devices that must be maintained. Currently this is controlled via device registration/probe order.
Quoted 8 lines
Quoted 7 lines
An SDIO func driver/device must always be suspended *before* the SDIO card device. Additionally the corresponding MMC host, must be suspended after the SDIO card device. Vice verse applies to the resume sequence.
As this patch enables asynchronous suspend, I am worried that it will break this dependency chain. What do you think?
After enabling asynchronous suspend/resume, PM core still ensures the strict suspend/resume dependency between child and parent devices - child must be suspended before its parent, and parent must be resumed before its child. SDIO function is child of SDIO card, and SDIO card is child of MMC host, and MMC host is child of MMC controller. So the dependency chain is not broken. Actually, many devices have been using asynchronous suspend/resume mode now.
I believe your view of how the PM core works for devices that *don't* use async suspend is wrong! The PM core doesn't respect parent/child relations during the device system PM phase for these devices.
Instead it relies on that devices in the "dpm list" are ordered correctly. As I tried to describe earlier, that list is being updated during device registration and probing (there are some other special cases as well).
So, by enabling async suspend for a device it will trigger the PM core durng device system PM, to start caring about device's parent/child relations. I would appreciate if you could add some of this information to the change log, as that's *why* it should work nicely for mmc/sd/sdio to use async suspend.
Quoted 22 lines
Thanks,
ZhonghuiQuoted 19 lines
Kind regards
Ulf HanssonQuoted 15 lines
---
Changes in v3:
- Add test result in commit messagedrivers/mmc/core/sdio.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)drivers/mmc/core/sdio.c +0
-0@@ -1113,6 +1113,8 @@ int mmc_attach_sdio(struct mmc_host *host)pm_runtime_enable (&card->dev); }
Quoted 2 lines
Quoted 2 lines
drivers/mmc/core/sdio.c int mmc_attach_sdio(struct mmc_host *host) · line 1117
Quoted 2 lines
drivers/mmc/core/sdio.c +2
-0+device_enable_async_suspend (&card->dev); +
This change will also affect SDIO combo cards. That means the when there is a mmc blk device driver bound to the mmc card, its ->suspend() methods will be called asynchronously.
Have you considered that? Especially since there are nothing being mentioned about it in the change-log?
Also, within this context I am wondering why you *only* enable async suspend for SDIO cards and not all cards (SD/MMC)? Is there a problem with doing that?
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
drivers/mmc/core/sdio.c +2
-0/* * The number of functions on the card is encoded inside * the ocr. @@ -1133,6 +1135,8 @@ int mmc_attach_sdio(struct mmc_host *host)*/ if (host->caps &MMC_CAP_POWER_OFF_CARD )pm_runtime_enable (&card->sdio_func[i]->dev); + +device_enable_async_suspend (&card->sdio_func[i]->dev); }
Quoted 4 lines
Quoted 3 lines
drivers/mmc/core/sdio.c int mmc_attach_sdio(struct mmc_host *host)
Quoted 3 lines
drivers/mmc/core/sdio.c +0
-1/* -- 1.7.1
[1]
https://lkml.org/lkml/2015/11/15/83
[2]
https://lkml.org/lkml/2015/11/16/6
[3]
http://www.spinics.net/lists/linux-mmc/msg34004.html
Kind regards
Ulf Hansson
Quoted 9 lines · Ulf Hansson
Quoted 7 lines · Zhonghui
Quoted 5 lines · Ulf Hansson
Quoted 4 lines · Zhonghui
Now, PM core supports asynchronous suspend/resume mode for devices during system suspend/resume, and the power state transition of one device may be completed in separate kernel thread. PM core ensures all power state transition timing dependency between devices. This
What "timing dependency"?
Sorry, "timing" should be needless word.
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
patch enables SDIO card and function devices to suspend/resume asynchronously. This will take advantage of multicore and improve system suspend/resume speed. After enabling the SDIO devices and all their child devices to suspend/resume asynchronously on ASUS T100TA, the system suspend-to-idle time is reduced from 1645ms to 1119ms, and the system resume time is reduced from 940ms to 918ms.
Quoted 7 lines
Are these improvements achieved by $subject patch on its own or you need below patches:
[PATCH v3] mmc: enable mmc host device to suspend/resume asynchronously [1]
[PATCH v3] mmc/sdhci-acpi: enable sdhci-acpi device to suspend/resume asynchronously [2]
This patch achieves these improvements by its own.
Quoted 6 lines
Depending if you have SD/(e)MMC card slot(s), the below patch might affect your results. So it might be a good idea to re-run your test to get some fresh data.
[PATCH 1/2] mmc: core: Make runtime resume default behavior for MMC/SD [3]
I didn't find this patch in mainline kernel, where is this patch?
Quoted 18 lines
Quoted 18 lines
Quoted 18 lines
Quoted 2 lines
Signed-off-by Zhonghui Fu <[email protected]>I think this is an interesting change, but I wonder if you really understand how this affects the order of how devices may be suspended/resumed?
Also, I believe you didn't answer my question for the earlier version of the patch, so let me try again.
There are a strict dependency chain when suspending/resuming devices that must be maintained. Currently this is controlled via device registration/probe order.
An SDIO func driver/device must always be suspended *before* the SDIO card device. Additionally the corresponding MMC host, must be suspended after the SDIO card device. Vice verse applies to the resume sequence.
Quoted 7 lines
Quoted 3 lines
Quoted 2 lines
As this patch enables asynchronous suspend, I am worried that it will break this dependency chain. What do you think?
After enabling asynchronous suspend/resume, PM core still ensures the strict suspend/resume dependency between child and parent devices - child must be suspended before its parent, and parent must be resumed before its child. SDIO function is child of SDIO card, and SDIO card is child of MMC host, and MMC host is child of MMC controller. So the dependency chain is not broken. Actually, many devices have been using asynchronous suspend/resume mode now.
I believe your view of how the PM core works for devices that *don't* use async suspend is wrong! The PM core doesn't respect parent/child relations during the device system PM phase for these devices.
I agree with you for the following description. But, I never described how PM core works for devices that don't use async mode. Where did you get my view about this? I just said that PM core still ensure the dependency between child and parent devices after using async mode, I never said that the method ensuring dependency is the same for sync devices and async devices.
Quoted 33 lines
Instead it relies on that devices in the "dpm list" are ordered correctly. As I tried to describe earlier, that list is being updated during device registration and probing (there are some other special cases as well).
So, by enabling async suspend for a device it will trigger the PM core durng device system PM, to start caring about device's parent/child relations. I would appreciate if you could add some of this information to the change log, as that's *why* it should work nicely for mmc/sd/sdio to use async suspend.
Quoted 20 lines
Thanks,
ZhonghuiQuoted 18 lines
Kind regards
Ulf HanssonQuoted 15 lines
---
Changes in v3:
- Add test result in commit messagedrivers/mmc/core/sdio.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)drivers/mmc/core/sdio.c +0
-0@@ -1113,6 +1113,8 @@ int mmc_attach_sdio(struct mmc_host *host)pm_runtime_enable (&card->dev); }
Quoted 8 lines
Quoted 2 lines
Quoted 2 lines
drivers/mmc/core/sdio.c int mmc_attach_sdio(struct mmc_host *host) · line 1117
Quoted 2 lines
drivers/mmc/core/sdio.c +2
-0+device_enable_async_suspend (&card->dev); +This change will also affect SDIO combo cards. That means the when there is a mmc blk device driver bound to the mmc card, its ->suspend() methods will be called asynchronously.
Have you considered that? Especially since there are nothing being mentioned about it in the change-log?
I have considered this, this patch still work for SDIO combo cards.
Quoted 4 lines
Also, within this context I am wondering why you *only* enable async suspend for SDIO cards and not all cards (SD/MMC)? Is there a problem with doing that?
I am optimizing suspend/resume speed for some Intel's tablet platforms and focusing only on the most time-consuming device path now. I will deliver similar patch for SD/MMC card.
Quoted 29 lines
Quoted 12 lines
Quoted 12 lines
Quoted 12 lines
drivers/mmc/core/sdio.c +2
-0/* * The number of functions on the card is encoded inside * the ocr. @@ -1133,6 +1135,8 @@ int mmc_attach_sdio(struct mmc_host *host)*/ if (host->caps &MMC_CAP_POWER_OFF_CARD )pm_runtime_enable (&card->sdio_func[i]->dev); + +device_enable_async_suspend (&card->sdio_func[i]->dev); }Quoted 3 lines
Quoted 3 lines
drivers/mmc/core/sdio.c int mmc_attach_sdio(struct mmc_host *host)
Quoted 3 lines
drivers/mmc/core/sdio.c +0
-1/* -- 1.7.1 [1]
https://lkml.org/lkml/2015/11/15/83
[2]
https://lkml.org/lkml/2015/11/16/6
[3]
http://www.spinics.net/lists/linux-mmc/msg34004.htmlKind regards
Ulf Hansson
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[...]
Quoted 7 lines
Quoted 6 lines
Depending if you have SD/(e)MMC card slot(s), the below patch might affect your results. So it might be a good idea to re-run your test to get some fresh data.
[PATCH 1/2] mmc: core: Make runtime resume default behavior for MMC/SD [3]
I didn't find this patch in mainline kernel, where is this patch?
It's queued for 4.5 on my next branch via my mmc git. Thus also available in linux-next.
One should know that it's affecting MMC/SD cards and not SDIO.
Future wise I was hoping we could do something similar for SDIO, but I need to think that through a bit more.
Quoted 18 lines
Quoted 18 lines
Quoted 18 lines
Quoted 18 lines
Quoted 2 lines
Signed-off-by Zhonghui Fu <[email protected]>I think this is an interesting change, but I wonder if you really understand how this affects the order of how devices may be suspended/resumed?
Also, I believe you didn't answer my question for the earlier version of the patch, so let me try again.
There are a strict dependency chain when suspending/resuming devices that must be maintained. Currently this is controlled via device registration/probe order.
An SDIO func driver/device must always be suspended *before* the SDIO card device. Additionally the corresponding MMC host, must be suspended after the SDIO card device. Vice verse applies to the resume sequence.
Quoted 8 lines
Quoted 7 lines
Quoted 3 lines
Quoted 2 lines
As this patch enables asynchronous suspend, I am worried that it will break this dependency chain. What do you think?
After enabling asynchronous suspend/resume, PM core still ensures the strict suspend/resume dependency between child and parent devices - child must be suspended before its parent, and parent must be resumed before its child. SDIO function is child of SDIO card, and SDIO card is child of MMC host, and MMC host is child of MMC controller. So the dependency chain is not broken. Actually, many devices have been using asynchronous suspend/resume mode now.
I believe your view of how the PM core works for devices that *don't* use async suspend is wrong! The PM core doesn't respect parent/child relations during the device system PM phase for these devices.
I agree with you for the following description. But, I never described how PM core works for devices that don't use async mode. Where did you get my view about this? I just said that PM core still ensure the dependency between child and parent devices after using async mode, I never said that the method ensuring dependency is the same for sync devices and async devices.
You said it again. "PM core *still* ensures the dependency... after using async mode". I guess it depends what you mean by "still". :-)
Anyway, it doesn't matter as I think we have sorted it out now.
[...]
Quoted 4 lines
Quoted 4 lines
This change will also affect SDIO combo cards. That means the when there is a mmc blk device driver bound to the mmc card, its ->suspend() methods will be called asynchronously.
Quoted 8 lines
Quoted 2 lines
Have you considered that? Especially since there are nothing being mentioned about it in the change-log?
I have considered this, this patch still work for SDIO combo cards.
Quoted 4 lines
Also, within this context I am wondering why you *only* enable async suspend for SDIO cards and not all cards (SD/MMC)? Is there a problem with doing that?
I am optimizing suspend/resume speed for some Intel's tablet platforms and focusing only on the most time-consuming device path now. I will deliver similar patch for SD/MMC card.
Okay, thanks for sharing you plans!
I would prefer if we instead enables async suspend for all types of card devices and the sdio func devices within one patch. I suggest we use mmc_add_card() and sdio_add_func(), to where we enable async suspend for the devices.
[...]
Kind regards
Uffe