* [PATCH][v3] mfd: intel-lpss: Avoid resuming runtime-suspended lpss unnecessarily
@ 2016-09-29 5:01 Chen Yu
2016-09-30 0:14 ` Rafael J. Wysocki
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Chen Yu @ 2016-09-29 5:01 UTC (permalink / raw)
To: linux-pm
Cc: Pavel Machek, Len Brown, linux-kernel, Chen Yu, Andy Shevchenko,
Mika Westerberg, Rafael J . Wysocki, Lee Jones
We have report that the intel_lpss_prepare() takes too much time during
suspend, and this is because we first resume the devices from runtime
suspend by resume_lpss_device(), to make sure they are in proper state
before system suspend, which takes 100ms for each LPSS devices(PCI power
state from D3_cold to D0). And since resume_lpss_device() resumes the
devices synchronously, we might get huge latency if we have many
LPSS devices.
So first try is to use pm_request_resume() instead, to make the runtime
resume process asynchronously. Unfortunately the asynchronous runtime
resume relies on pm_wq, which is freezed at early stage. So we choose
another method, that is to avoid resuming runtime-suspended devices,
if they are already runtime suspended. This is safe because for LPSS
driver, the runtime suspend and system suspend are of the same
hook - i.e., intel_lpss_suspend(). And moreover, this device is
neither runtime wakeup source nor system wakeup source.
Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Cc: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Chen Yu <yu.c.chen@intel.com>
---
drivers/mfd/intel-lpss.c | 9 +++++++++
include/linux/pm.h | 7 +++++++
2 files changed, 16 insertions(+)
diff --git a/drivers/mfd/intel-lpss.c b/drivers/mfd/intel-lpss.c
index 41b1138..2583db8 100644
--- a/drivers/mfd/intel-lpss.c
+++ b/drivers/mfd/intel-lpss.c
@@ -485,6 +485,15 @@ static int resume_lpss_device(struct device *dev, void *data)
int intel_lpss_prepare(struct device *dev)
{
/*
+ * This is safe because:
+ * 1. The runtime suspend and system suspend
+ * are of the same hook.
+ * 2. This device is neither runtime wakeup source
+ * nor system wakeup source.
+ */
+ if (pm_runtime_status_suspended(dev))
+ return DPM_DIRECT_COMPLETE;
+ /*
* Resume both child devices before entering system sleep. This
* ensures that they are in proper state before they get suspended.
*/
diff --git a/include/linux/pm.h b/include/linux/pm.h
index 06eb353..4a788b4 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -786,4 +786,11 @@ enum dpm_order {
DPM_ORDER_DEV_LAST,
};
+/*
+ * Return this from system suspend/hibernation ->prepare() callback to
+ * request the core to leave the device runtime-suspended during system
+ * suspend if possible.
+ */
+#define DPM_DIRECT_COMPLETE 1
+
#endif /* _LINUX_PM_H */
--
2.7.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH][v3] mfd: intel-lpss: Avoid resuming runtime-suspended lpss unnecessarily
2016-09-29 5:01 [PATCH][v3] mfd: intel-lpss: Avoid resuming runtime-suspended lpss unnecessarily Chen Yu
@ 2016-09-30 0:14 ` Rafael J. Wysocki
2016-09-30 0:29 ` Lee Jones
2016-10-04 14:55 ` Lee Jones
2016-12-16 23:19 ` [PATCH v3] " Lukas Wunner
2 siblings, 1 reply; 8+ messages in thread
From: Rafael J. Wysocki @ 2016-09-30 0:14 UTC (permalink / raw)
To: Chen Yu, Lee Jones
Cc: Linux PM, Pavel Machek, Len Brown, Linux Kernel Mailing List,
Andy Shevchenko, Mika Westerberg, Rafael J . Wysocki
Hi Lee,
On Thu, Sep 29, 2016 at 7:01 AM, Chen Yu <yu.c.chen@intel.com> wrote:
> We have report that the intel_lpss_prepare() takes too much time during
> suspend, and this is because we first resume the devices from runtime
> suspend by resume_lpss_device(), to make sure they are in proper state
> before system suspend, which takes 100ms for each LPSS devices(PCI power
> state from D3_cold to D0). And since resume_lpss_device() resumes the
> devices synchronously, we might get huge latency if we have many
> LPSS devices.
>
> So first try is to use pm_request_resume() instead, to make the runtime
> resume process asynchronously. Unfortunately the asynchronous runtime
> resume relies on pm_wq, which is freezed at early stage. So we choose
> another method, that is to avoid resuming runtime-suspended devices,
> if they are already runtime suspended. This is safe because for LPSS
> driver, the runtime suspend and system suspend are of the same
> hook - i.e., intel_lpss_suspend(). And moreover, this device is
> neither runtime wakeup source nor system wakeup source.
>
> Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Cc: Lee Jones <lee.jones@linaro.org>
> Signed-off-by: Chen Yu <yu.c.chen@intel.com>
If this is fine with you and you'd like to apply it, please feel free
to add my ACK to it.
Alternatively, if you'd prefer me to apply it, please let me know.
> ---
> drivers/mfd/intel-lpss.c | 9 +++++++++
> include/linux/pm.h | 7 +++++++
> 2 files changed, 16 insertions(+)
>
> diff --git a/drivers/mfd/intel-lpss.c b/drivers/mfd/intel-lpss.c
> index 41b1138..2583db8 100644
> --- a/drivers/mfd/intel-lpss.c
> +++ b/drivers/mfd/intel-lpss.c
> @@ -485,6 +485,15 @@ static int resume_lpss_device(struct device *dev, void *data)
> int intel_lpss_prepare(struct device *dev)
> {
> /*
> + * This is safe because:
> + * 1. The runtime suspend and system suspend
> + * are of the same hook.
> + * 2. This device is neither runtime wakeup source
> + * nor system wakeup source.
> + */
> + if (pm_runtime_status_suspended(dev))
> + return DPM_DIRECT_COMPLETE;
> + /*
> * Resume both child devices before entering system sleep. This
> * ensures that they are in proper state before they get suspended.
> */
> diff --git a/include/linux/pm.h b/include/linux/pm.h
> index 06eb353..4a788b4 100644
> --- a/include/linux/pm.h
> +++ b/include/linux/pm.h
> @@ -786,4 +786,11 @@ enum dpm_order {
> DPM_ORDER_DEV_LAST,
> };
>
> +/*
> + * Return this from system suspend/hibernation ->prepare() callback to
> + * request the core to leave the device runtime-suspended during system
> + * suspend if possible.
> + */
> +#define DPM_DIRECT_COMPLETE 1
> +
> #endif /* _LINUX_PM_H */
> --
Thanks,
Rafael
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH][v3] mfd: intel-lpss: Avoid resuming runtime-suspended lpss unnecessarily
2016-09-30 0:14 ` Rafael J. Wysocki
@ 2016-09-30 0:29 ` Lee Jones
2016-09-30 0:31 ` Rafael J. Wysocki
0 siblings, 1 reply; 8+ messages in thread
From: Lee Jones @ 2016-09-30 0:29 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Chen Yu, Linux PM, Pavel Machek, Len Brown,
Linux Kernel Mailing List, Andy Shevchenko, Mika Westerberg,
Rafael J . Wysocki
On Fri, 30 Sep 2016, Rafael J. Wysocki wrote:
> Hi Lee,
>
> On Thu, Sep 29, 2016 at 7:01 AM, Chen Yu <yu.c.chen@intel.com> wrote:
> > We have report that the intel_lpss_prepare() takes too much time during
> > suspend, and this is because we first resume the devices from runtime
> > suspend by resume_lpss_device(), to make sure they are in proper state
> > before system suspend, which takes 100ms for each LPSS devices(PCI power
> > state from D3_cold to D0). And since resume_lpss_device() resumes the
> > devices synchronously, we might get huge latency if we have many
> > LPSS devices.
> >
> > So first try is to use pm_request_resume() instead, to make the runtime
> > resume process asynchronously. Unfortunately the asynchronous runtime
> > resume relies on pm_wq, which is freezed at early stage. So we choose
> > another method, that is to avoid resuming runtime-suspended devices,
> > if they are already runtime suspended. This is safe because for LPSS
> > driver, the runtime suspend and system suspend are of the same
> > hook - i.e., intel_lpss_suspend(). And moreover, this device is
> > neither runtime wakeup source nor system wakeup source.
> >
> > Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
> > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > Cc: Lee Jones <lee.jones@linaro.org>
> > Signed-off-by: Chen Yu <yu.c.chen@intel.com>
>
> If this is fine with you and you'd like to apply it, please feel free
> to add my ACK to it.
>
> Alternatively, if you'd prefer me to apply it, please let me know.
You want this in for v3.9?
I just started applying patches for v3.10.
If you're certain there are 0% chance of regressions, I will still
apply this for v3.9 with your Ack.
> > ---
> > drivers/mfd/intel-lpss.c | 9 +++++++++
> > include/linux/pm.h | 7 +++++++
> > 2 files changed, 16 insertions(+)
> >
> > diff --git a/drivers/mfd/intel-lpss.c b/drivers/mfd/intel-lpss.c
> > index 41b1138..2583db8 100644
> > --- a/drivers/mfd/intel-lpss.c
> > +++ b/drivers/mfd/intel-lpss.c
> > @@ -485,6 +485,15 @@ static int resume_lpss_device(struct device *dev, void *data)
> > int intel_lpss_prepare(struct device *dev)
> > {
> > /*
> > + * This is safe because:
> > + * 1. The runtime suspend and system suspend
> > + * are of the same hook.
> > + * 2. This device is neither runtime wakeup source
> > + * nor system wakeup source.
> > + */
> > + if (pm_runtime_status_suspended(dev))
> > + return DPM_DIRECT_COMPLETE;
> > + /*
> > * Resume both child devices before entering system sleep. This
> > * ensures that they are in proper state before they get suspended.
> > */
> > diff --git a/include/linux/pm.h b/include/linux/pm.h
> > index 06eb353..4a788b4 100644
> > --- a/include/linux/pm.h
> > +++ b/include/linux/pm.h
> > @@ -786,4 +786,11 @@ enum dpm_order {
> > DPM_ORDER_DEV_LAST,
> > };
> >
> > +/*
> > + * Return this from system suspend/hibernation ->prepare() callback to
> > + * request the core to leave the device runtime-suspended during system
> > + * suspend if possible.
> > + */
> > +#define DPM_DIRECT_COMPLETE 1
> > +
> > #endif /* _LINUX_PM_H */
> > --
>
> Thanks,
> Rafael
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH][v3] mfd: intel-lpss: Avoid resuming runtime-suspended lpss unnecessarily
2016-09-30 0:29 ` Lee Jones
@ 2016-09-30 0:31 ` Rafael J. Wysocki
2016-10-04 14:55 ` Lee Jones
0 siblings, 1 reply; 8+ messages in thread
From: Rafael J. Wysocki @ 2016-09-30 0:31 UTC (permalink / raw)
To: Lee Jones
Cc: Rafael J. Wysocki, Chen Yu, Linux PM, Pavel Machek, Len Brown,
Linux Kernel Mailing List, Andy Shevchenko, Mika Westerberg,
Rafael J . Wysocki
On Fri, Sep 30, 2016 at 2:29 AM, Lee Jones <lee.jones@linaro.org> wrote:
> On Fri, 30 Sep 2016, Rafael J. Wysocki wrote:
>
>> Hi Lee,
>>
>> On Thu, Sep 29, 2016 at 7:01 AM, Chen Yu <yu.c.chen@intel.com> wrote:
>> > We have report that the intel_lpss_prepare() takes too much time during
>> > suspend, and this is because we first resume the devices from runtime
>> > suspend by resume_lpss_device(), to make sure they are in proper state
>> > before system suspend, which takes 100ms for each LPSS devices(PCI power
>> > state from D3_cold to D0). And since resume_lpss_device() resumes the
>> > devices synchronously, we might get huge latency if we have many
>> > LPSS devices.
>> >
>> > So first try is to use pm_request_resume() instead, to make the runtime
>> > resume process asynchronously. Unfortunately the asynchronous runtime
>> > resume relies on pm_wq, which is freezed at early stage. So we choose
>> > another method, that is to avoid resuming runtime-suspended devices,
>> > if they are already runtime suspended. This is safe because for LPSS
>> > driver, the runtime suspend and system suspend are of the same
>> > hook - i.e., intel_lpss_suspend(). And moreover, this device is
>> > neither runtime wakeup source nor system wakeup source.
>> >
>> > Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>> > Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
>> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> > Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
>> > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>> > Cc: Lee Jones <lee.jones@linaro.org>
>> > Signed-off-by: Chen Yu <yu.c.chen@intel.com>
>>
>> If this is fine with you and you'd like to apply it, please feel free
>> to add my ACK to it.
>>
>> Alternatively, if you'd prefer me to apply it, please let me know.
>
> You want this in for v3.9?
I'd rather queue it up for 4.10 (assuming that the above and below
major version numbers are simply off by one by mistake).
> I just started applying patches for v3.10.
>
> If you're certain there are 0% chance of regressions, I will still
> apply this for v3.9 with your Ack.
4.10 should be fine.
>> > ---
>> > drivers/mfd/intel-lpss.c | 9 +++++++++
>> > include/linux/pm.h | 7 +++++++
>> > 2 files changed, 16 insertions(+)
>> >
>> > diff --git a/drivers/mfd/intel-lpss.c b/drivers/mfd/intel-lpss.c
>> > index 41b1138..2583db8 100644
>> > --- a/drivers/mfd/intel-lpss.c
>> > +++ b/drivers/mfd/intel-lpss.c
>> > @@ -485,6 +485,15 @@ static int resume_lpss_device(struct device *dev, void *data)
>> > int intel_lpss_prepare(struct device *dev)
>> > {
>> > /*
>> > + * This is safe because:
>> > + * 1. The runtime suspend and system suspend
>> > + * are of the same hook.
>> > + * 2. This device is neither runtime wakeup source
>> > + * nor system wakeup source.
>> > + */
>> > + if (pm_runtime_status_suspended(dev))
>> > + return DPM_DIRECT_COMPLETE;
>> > + /*
>> > * Resume both child devices before entering system sleep. This
>> > * ensures that they are in proper state before they get suspended.
>> > */
>> > diff --git a/include/linux/pm.h b/include/linux/pm.h
>> > index 06eb353..4a788b4 100644
>> > --- a/include/linux/pm.h
>> > +++ b/include/linux/pm.h
>> > @@ -786,4 +786,11 @@ enum dpm_order {
>> > DPM_ORDER_DEV_LAST,
>> > };
>> >
>> > +/*
>> > + * Return this from system suspend/hibernation ->prepare() callback to
>> > + * request the core to leave the device runtime-suspended during system
>> > + * suspend if possible.
>> > + */
>> > +#define DPM_DIRECT_COMPLETE 1
>> > +
>> > #endif /* _LINUX_PM_H */
>> > --
Thanks,
Rafael
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH][v3] mfd: intel-lpss: Avoid resuming runtime-suspended lpss unnecessarily
2016-09-30 0:31 ` Rafael J. Wysocki
@ 2016-10-04 14:55 ` Lee Jones
0 siblings, 0 replies; 8+ messages in thread
From: Lee Jones @ 2016-10-04 14:55 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Chen Yu, Linux PM, Pavel Machek, Len Brown,
Linux Kernel Mailing List, Andy Shevchenko, Mika Westerberg,
Rafael J . Wysocki
On Fri, 30 Sep 2016, Rafael J. Wysocki wrote:
> On Fri, Sep 30, 2016 at 2:29 AM, Lee Jones <lee.jones@linaro.org> wrote:
> > On Fri, 30 Sep 2016, Rafael J. Wysocki wrote:
> >
> >> Hi Lee,
> >>
> >> On Thu, Sep 29, 2016 at 7:01 AM, Chen Yu <yu.c.chen@intel.com> wrote:
> >> > We have report that the intel_lpss_prepare() takes too much time during
> >> > suspend, and this is because we first resume the devices from runtime
> >> > suspend by resume_lpss_device(), to make sure they are in proper state
> >> > before system suspend, which takes 100ms for each LPSS devices(PCI power
> >> > state from D3_cold to D0). And since resume_lpss_device() resumes the
> >> > devices synchronously, we might get huge latency if we have many
> >> > LPSS devices.
> >> >
> >> > So first try is to use pm_request_resume() instead, to make the runtime
> >> > resume process asynchronously. Unfortunately the asynchronous runtime
> >> > resume relies on pm_wq, which is freezed at early stage. So we choose
> >> > another method, that is to avoid resuming runtime-suspended devices,
> >> > if they are already runtime suspended. This is safe because for LPSS
> >> > driver, the runtime suspend and system suspend are of the same
> >> > hook - i.e., intel_lpss_suspend(). And moreover, this device is
> >> > neither runtime wakeup source nor system wakeup source.
> >> >
> >> > Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >> > Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> >> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> >> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> >> > Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
> >> > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >> > Cc: Lee Jones <lee.jones@linaro.org>
> >> > Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> >>
> >> If this is fine with you and you'd like to apply it, please feel free
> >> to add my ACK to it.
> >>
> >> Alternatively, if you'd prefer me to apply it, please let me know.
> >
> > You want this in for v3.9?
>
> I'd rather queue it up for 4.10 (assuming that the above and below
> major version numbers are simply off by one by mistake).
Yes, of course they are off by one.
This is what happens when you conduct mail duties with jetlag. :)
> > I just started applying patches for v3.10.
> >
> > If you're certain there are 0% chance of regressions, I will still
> > apply this for v3.9 with your Ack.
>
> 4.10 should be fine.
Great.
> >> > ---
> >> > drivers/mfd/intel-lpss.c | 9 +++++++++
> >> > include/linux/pm.h | 7 +++++++
> >> > 2 files changed, 16 insertions(+)
> >> >
> >> > diff --git a/drivers/mfd/intel-lpss.c b/drivers/mfd/intel-lpss.c
> >> > index 41b1138..2583db8 100644
> >> > --- a/drivers/mfd/intel-lpss.c
> >> > +++ b/drivers/mfd/intel-lpss.c
> >> > @@ -485,6 +485,15 @@ static int resume_lpss_device(struct device *dev, void *data)
> >> > int intel_lpss_prepare(struct device *dev)
> >> > {
> >> > /*
> >> > + * This is safe because:
> >> > + * 1. The runtime suspend and system suspend
> >> > + * are of the same hook.
> >> > + * 2. This device is neither runtime wakeup source
> >> > + * nor system wakeup source.
> >> > + */
> >> > + if (pm_runtime_status_suspended(dev))
> >> > + return DPM_DIRECT_COMPLETE;
> >> > + /*
> >> > * Resume both child devices before entering system sleep. This
> >> > * ensures that they are in proper state before they get suspended.
> >> > */
> >> > diff --git a/include/linux/pm.h b/include/linux/pm.h
> >> > index 06eb353..4a788b4 100644
> >> > --- a/include/linux/pm.h
> >> > +++ b/include/linux/pm.h
> >> > @@ -786,4 +786,11 @@ enum dpm_order {
> >> > DPM_ORDER_DEV_LAST,
> >> > };
> >> >
> >> > +/*
> >> > + * Return this from system suspend/hibernation ->prepare() callback to
> >> > + * request the core to leave the device runtime-suspended during system
> >> > + * suspend if possible.
> >> > + */
> >> > +#define DPM_DIRECT_COMPLETE 1
> >> > +
> >> > #endif /* _LINUX_PM_H */
> >> > --
>
> Thanks,
> Rafael
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH][v3] mfd: intel-lpss: Avoid resuming runtime-suspended lpss unnecessarily
2016-09-29 5:01 [PATCH][v3] mfd: intel-lpss: Avoid resuming runtime-suspended lpss unnecessarily Chen Yu
2016-09-30 0:14 ` Rafael J. Wysocki
@ 2016-10-04 14:55 ` Lee Jones
2016-12-16 23:19 ` [PATCH v3] " Lukas Wunner
2 siblings, 0 replies; 8+ messages in thread
From: Lee Jones @ 2016-10-04 14:55 UTC (permalink / raw)
To: Chen Yu
Cc: linux-pm, Pavel Machek, Len Brown, linux-kernel, Andy Shevchenko,
Mika Westerberg, Rafael J . Wysocki
On Thu, 29 Sep 2016, Chen Yu wrote:
> We have report that the intel_lpss_prepare() takes too much time during
> suspend, and this is because we first resume the devices from runtime
> suspend by resume_lpss_device(), to make sure they are in proper state
> before system suspend, which takes 100ms for each LPSS devices(PCI power
> state from D3_cold to D0). And since resume_lpss_device() resumes the
> devices synchronously, we might get huge latency if we have many
> LPSS devices.
>
> So first try is to use pm_request_resume() instead, to make the runtime
> resume process asynchronously. Unfortunately the asynchronous runtime
> resume relies on pm_wq, which is freezed at early stage. So we choose
> another method, that is to avoid resuming runtime-suspended devices,
> if they are already runtime suspended. This is safe because for LPSS
> driver, the runtime suspend and system suspend are of the same
> hook - i.e., intel_lpss_suspend(). And moreover, this device is
> neither runtime wakeup source nor system wakeup source.
>
> Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Cc: Lee Jones <lee.jones@linaro.org>
> Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> ---
> drivers/mfd/intel-lpss.c | 9 +++++++++
> include/linux/pm.h | 7 +++++++
> 2 files changed, 16 insertions(+)
Applied for v4.10, thanks.
> diff --git a/drivers/mfd/intel-lpss.c b/drivers/mfd/intel-lpss.c
> index 41b1138..2583db8 100644
> --- a/drivers/mfd/intel-lpss.c
> +++ b/drivers/mfd/intel-lpss.c
> @@ -485,6 +485,15 @@ static int resume_lpss_device(struct device *dev, void *data)
> int intel_lpss_prepare(struct device *dev)
> {
> /*
> + * This is safe because:
> + * 1. The runtime suspend and system suspend
> + * are of the same hook.
> + * 2. This device is neither runtime wakeup source
> + * nor system wakeup source.
> + */
> + if (pm_runtime_status_suspended(dev))
> + return DPM_DIRECT_COMPLETE;
> + /*
> * Resume both child devices before entering system sleep. This
> * ensures that they are in proper state before they get suspended.
> */
> diff --git a/include/linux/pm.h b/include/linux/pm.h
> index 06eb353..4a788b4 100644
> --- a/include/linux/pm.h
> +++ b/include/linux/pm.h
> @@ -786,4 +786,11 @@ enum dpm_order {
> DPM_ORDER_DEV_LAST,
> };
>
> +/*
> + * Return this from system suspend/hibernation ->prepare() callback to
> + * request the core to leave the device runtime-suspended during system
> + * suspend if possible.
> + */
> +#define DPM_DIRECT_COMPLETE 1
> +
> #endif /* _LINUX_PM_H */
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] mfd: intel-lpss: Avoid resuming runtime-suspended lpss unnecessarily
2016-09-29 5:01 [PATCH][v3] mfd: intel-lpss: Avoid resuming runtime-suspended lpss unnecessarily Chen Yu
2016-09-30 0:14 ` Rafael J. Wysocki
2016-10-04 14:55 ` Lee Jones
@ 2016-12-16 23:19 ` Lukas Wunner
2017-01-03 12:29 ` Lee Jones
2 siblings, 1 reply; 8+ messages in thread
From: Lukas Wunner @ 2016-12-16 23:19 UTC (permalink / raw)
To: Lee Jones
Cc: linux-pm, linux-kernel, Andy Shevchenko, Mika Westerberg,
Rafael J . Wysocki, Chen Yu
Hi Lee,
you wrote on Oct 4 that you applied the below patch for v4.10.
However I don't see the patch on your for-mfd-next branch.
Maybe the patch slipped between the cracks, please double-check.
I would like to use the DPM_DIRECT_COMPLETE macro introduced by it
in a patch of my own.
I also don't see a pull from you in Linus' tree yet. Please note
that this merge window will be shorter than usual and pulls should
be sent out before coming Friday.
Thanks,
Lukas
On Thu, Sep 29, 2016 at 01:01:26PM +0800, Chen Yu wrote:
> We have report that the intel_lpss_prepare() takes too much time during
> suspend, and this is because we first resume the devices from runtime
> suspend by resume_lpss_device(), to make sure they are in proper state
> before system suspend, which takes 100ms for each LPSS devices(PCI power
> state from D3_cold to D0). And since resume_lpss_device() resumes the
> devices synchronously, we might get huge latency if we have many
> LPSS devices.
>
> So first try is to use pm_request_resume() instead, to make the runtime
> resume process asynchronously. Unfortunately the asynchronous runtime
> resume relies on pm_wq, which is freezed at early stage. So we choose
> another method, that is to avoid resuming runtime-suspended devices,
> if they are already runtime suspended. This is safe because for LPSS
> driver, the runtime suspend and system suspend are of the same
> hook - i.e., intel_lpss_suspend(). And moreover, this device is
> neither runtime wakeup source nor system wakeup source.
>
> Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Cc: Lee Jones <lee.jones@linaro.org>
> Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> ---
> drivers/mfd/intel-lpss.c | 9 +++++++++
> include/linux/pm.h | 7 +++++++
> 2 files changed, 16 insertions(+)
>
> diff --git a/drivers/mfd/intel-lpss.c b/drivers/mfd/intel-lpss.c
> index 41b1138..2583db8 100644
> --- a/drivers/mfd/intel-lpss.c
> +++ b/drivers/mfd/intel-lpss.c
> @@ -485,6 +485,15 @@ static int resume_lpss_device(struct device *dev, void *data)
> int intel_lpss_prepare(struct device *dev)
> {
> /*
> + * This is safe because:
> + * 1. The runtime suspend and system suspend
> + * are of the same hook.
> + * 2. This device is neither runtime wakeup source
> + * nor system wakeup source.
> + */
> + if (pm_runtime_status_suspended(dev))
> + return DPM_DIRECT_COMPLETE;
> + /*
> * Resume both child devices before entering system sleep. This
> * ensures that they are in proper state before they get suspended.
> */
> diff --git a/include/linux/pm.h b/include/linux/pm.h
> index 06eb353..4a788b4 100644
> --- a/include/linux/pm.h
> +++ b/include/linux/pm.h
> @@ -786,4 +786,11 @@ enum dpm_order {
> DPM_ORDER_DEV_LAST,
> };
>
> +/*
> + * Return this from system suspend/hibernation ->prepare() callback to
> + * request the core to leave the device runtime-suspended during system
> + * suspend if possible.
> + */
> +#define DPM_DIRECT_COMPLETE 1
> +
> #endif /* _LINUX_PM_H */
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] mfd: intel-lpss: Avoid resuming runtime-suspended lpss unnecessarily
2016-12-16 23:19 ` [PATCH v3] " Lukas Wunner
@ 2017-01-03 12:29 ` Lee Jones
0 siblings, 0 replies; 8+ messages in thread
From: Lee Jones @ 2017-01-03 12:29 UTC (permalink / raw)
To: Lukas Wunner
Cc: linux-pm, linux-kernel, Andy Shevchenko, Mika Westerberg,
Rafael J . Wysocki, Chen Yu
On Sat, 17 Dec 2016, Lukas Wunner wrote:
> you wrote on Oct 4 that you applied the below patch for v4.10.
> However I don't see the patch on your for-mfd-next branch.
> Maybe the patch slipped between the cracks, please double-check.
> I would like to use the DPM_DIRECT_COMPLETE macro introduced by it
> in a patch of my own.
Chen asked me to remove the patch.
> I also don't see a pull from you in Linus' tree yet. Please note
> that this merge window will be shorter than usual and pulls should
> be sent out before coming Friday.
Right, I was on vacation. All sorted now.
> On Thu, Sep 29, 2016 at 01:01:26PM +0800, Chen Yu wrote:
> > We have report that the intel_lpss_prepare() takes too much time during
> > suspend, and this is because we first resume the devices from runtime
> > suspend by resume_lpss_device(), to make sure they are in proper state
> > before system suspend, which takes 100ms for each LPSS devices(PCI power
> > state from D3_cold to D0). And since resume_lpss_device() resumes the
> > devices synchronously, we might get huge latency if we have many
> > LPSS devices.
> >
> > So first try is to use pm_request_resume() instead, to make the runtime
> > resume process asynchronously. Unfortunately the asynchronous runtime
> > resume relies on pm_wq, which is freezed at early stage. So we choose
> > another method, that is to avoid resuming runtime-suspended devices,
> > if they are already runtime suspended. This is safe because for LPSS
> > driver, the runtime suspend and system suspend are of the same
> > hook - i.e., intel_lpss_suspend(). And moreover, this device is
> > neither runtime wakeup source nor system wakeup source.
> >
> > Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
> > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > Cc: Lee Jones <lee.jones@linaro.org>
> > Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> > ---
> > drivers/mfd/intel-lpss.c | 9 +++++++++
> > include/linux/pm.h | 7 +++++++
> > 2 files changed, 16 insertions(+)
> >
> > diff --git a/drivers/mfd/intel-lpss.c b/drivers/mfd/intel-lpss.c
> > index 41b1138..2583db8 100644
> > --- a/drivers/mfd/intel-lpss.c
> > +++ b/drivers/mfd/intel-lpss.c
> > @@ -485,6 +485,15 @@ static int resume_lpss_device(struct device *dev, void *data)
> > int intel_lpss_prepare(struct device *dev)
> > {
> > /*
> > + * This is safe because:
> > + * 1. The runtime suspend and system suspend
> > + * are of the same hook.
> > + * 2. This device is neither runtime wakeup source
> > + * nor system wakeup source.
> > + */
> > + if (pm_runtime_status_suspended(dev))
> > + return DPM_DIRECT_COMPLETE;
> > + /*
> > * Resume both child devices before entering system sleep. This
> > * ensures that they are in proper state before they get suspended.
> > */
> > diff --git a/include/linux/pm.h b/include/linux/pm.h
> > index 06eb353..4a788b4 100644
> > --- a/include/linux/pm.h
> > +++ b/include/linux/pm.h
> > @@ -786,4 +786,11 @@ enum dpm_order {
> > DPM_ORDER_DEV_LAST,
> > };
> >
> > +/*
> > + * Return this from system suspend/hibernation ->prepare() callback to
> > + * request the core to leave the device runtime-suspended during system
> > + * suspend if possible.
> > + */
> > +#define DPM_DIRECT_COMPLETE 1
> > +
> > #endif /* _LINUX_PM_H */
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-01-03 12:25 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-29 5:01 [PATCH][v3] mfd: intel-lpss: Avoid resuming runtime-suspended lpss unnecessarily Chen Yu
2016-09-30 0:14 ` Rafael J. Wysocki
2016-09-30 0:29 ` Lee Jones
2016-09-30 0:31 ` Rafael J. Wysocki
2016-10-04 14:55 ` Lee Jones
2016-10-04 14:55 ` Lee Jones
2016-12-16 23:19 ` [PATCH v3] " Lukas Wunner
2017-01-03 12:29 ` Lee Jones
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.