* [PATCH V4] cpufreq: suspend governors on system suspend/hibernate
@ 2013-11-27 3:39 Viresh Kumar
2013-11-28 14:23 ` Rafael J. Wysocki
0 siblings, 1 reply; 10+ messages in thread
From: Viresh Kumar @ 2013-11-27 3:39 UTC (permalink / raw)
To: rjw
Cc: linaro-kernel, patches, cpufreq, linux-pm, linux-kernel, nm,
swarren, kgene.kim, linux-samsung-soc, linux-tegra, jinchoi,
tianyu.lan, sebastian.capella, jhbird.choi, Viresh Kumar
This patch adds cpufreq callbacks to dpm_{suspend|resume}_noirq() for handling
suspend/resume of cpufreq governors.
There are multiple problems that are fixed by this patch:
- Nishanth Menon (TI) found an interesting problem on his platform, OMAP. His board
wasn't working well with suspend/resume as calls for removing non-boot CPUs
was turning out into a call to drivers ->target() which then tries to play
with regulators. But regulators and their I2C bus were already suspended and
this resulted in a failure. Many platforms have such problems, samsung, tegra,
etc.. They solved it with driver specific PM notifiers where they used to
disable their driver's ->target() routine.
- Lan Tianyu (Intel) & Jinhyuk Choi (Broadcom) found another issue where
tunables configuration for clusters/sockets with non-boot CPUs was getting
lost after suspend/resume, as we were notifying governors with
CPUFREQ_GOV_POLICY_EXIT on removal of the last cpu for that policy and so
deallocating memory for tunables. This is also fixed with this patch as we
don't allow any operation on Governors during suspend/resume now.
Reported-and-tested-by: Lan Tianyu <tianyu.lan@intel.com>
Reported-and-tested-by: Nishanth Menon <nm@ti.com>
Reported-by: Jinhyuk Choi <jinchoi@broadcom.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
This is almost same as 1/6 of V3 version of this patchset:
https://lkml.org/lkml/2013/11/25/838
This is done to get some initial fixes for 3.13. These are already tested by
both the reporters of initial problems. Tegra/exynos/s5p will keep running their
PM notifiers until v3.14, as they are currently able to work with them..
drivers/base/power/main.c | 3 +++
drivers/cpufreq/cpufreq.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++
include/linux/cpufreq.h | 8 ++++++++
3 files changed, 61 insertions(+)
diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index 1b41fca..e3219df 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -29,6 +29,7 @@
#include <linux/async.h>
#include <linux/suspend.h>
#include <trace/events/power.h>
+#include <linux/cpufreq.h>
#include <linux/cpuidle.h>
#include <linux/timer.h>
@@ -540,6 +541,7 @@ static void dpm_resume_noirq(pm_message_t state)
dpm_show_time(starttime, state, "noirq");
resume_device_irqs();
cpuidle_resume();
+ cpufreq_resume();
}
/**
@@ -955,6 +957,7 @@ static int dpm_suspend_noirq(pm_message_t state)
ktime_t starttime = ktime_get();
int error = 0;
+ cpufreq_suspend();
cpuidle_pause();
suspend_device_irqs();
mutex_lock(&dpm_list_mtx);
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 02d534d..b6c7821 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -26,6 +26,7 @@
#include <linux/module.h>
#include <linux/mutex.h>
#include <linux/slab.h>
+#include <linux/suspend.h>
#include <linux/syscore_ops.h>
#include <linux/tick.h>
#include <trace/events/power.h>
@@ -47,6 +48,9 @@ static LIST_HEAD(cpufreq_policy_list);
static DEFINE_PER_CPU(char[CPUFREQ_NAME_LEN], cpufreq_cpu_governor);
#endif
+/* Flag to suspend/resume CPUFreq governors */
+static bool cpufreq_suspended;
+
static inline bool has_target(void)
{
return cpufreq_driver->target_index || cpufreq_driver->target;
@@ -1462,6 +1466,48 @@ static struct subsys_interface cpufreq_interface = {
.remove_dev = cpufreq_remove_dev,
};
+/*
+ * Callbacks for suspending/resuming governors as some platforms can't change
+ * frequency after this point in suspend cycle. Because some of the devices
+ * (like: i2c, regulators, etc) they use for changing frequency are suspended
+ * quickly after this point.
+ */
+void cpufreq_suspend(void)
+{
+ struct cpufreq_policy *policy;
+
+ if (!has_target())
+ return;
+
+ pr_debug("%s: Suspending Governors\n", __func__);
+
+ list_for_each_entry(policy, &cpufreq_policy_list, policy_list)
+ if (__cpufreq_governor(policy, CPUFREQ_GOV_STOP))
+ pr_err("%s: Failed to stop governor for policy: %p\n",
+ __func__, policy);
+
+ cpufreq_suspended = true;
+}
+
+void cpufreq_resume(void)
+{
+ struct cpufreq_policy *policy;
+
+ if (!has_target())
+ return;
+
+ pr_debug("%s: Resuming Governors\n", __func__);
+
+ cpufreq_suspended = false;
+
+ list_for_each_entry(policy, &cpufreq_policy_list, policy_list)
+ if (__cpufreq_governor(policy, CPUFREQ_GOV_START) ||
+ __cpufreq_governor(policy,
+ CPUFREQ_GOV_LIMITS))
+ pr_err("%s: Failed to start governor for policy: %p\n",
+ __func__, policy);
+}
+
/**
* cpufreq_bp_suspend - Prepare the boot CPU for system suspend.
*
@@ -1764,6 +1810,10 @@ static int __cpufreq_governor(struct cpufreq_policy *policy,
struct cpufreq_governor *gov = NULL;
#endif
+ /* Don't start any governor operations if we are entering suspend */
+ if (cpufreq_suspended)
+ return 0;
+
if (policy->governor->max_transition_latency &&
policy->cpuinfo.transition_latency >
policy->governor->max_transition_latency) {
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index dc196bb..ee5fe9d 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -280,6 +280,14 @@ cpufreq_verify_within_cpu_limits(struct cpufreq_policy *policy)
policy->cpuinfo.max_freq);
}
+#ifdef CONFIG_CPU_FREQ
+void cpufreq_suspend(void);
+void cpufreq_resume(void);
+#else
+static inline void cpufreq_suspend(void) {}
+static inline void cpufreq_resume(void) {}
+#endif
+
/*********************************************************************
* CPUFREQ NOTIFIER INTERFACE *
*********************************************************************/
--
1.7.12.rc2.18.g61b472e
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH V4] cpufreq: suspend governors on system suspend/hibernate
2013-11-28 14:23 ` Rafael J. Wysocki
@ 2013-11-28 14:19 ` Viresh Kumar
2013-11-28 20:29 ` Rafael J. Wysocki
0 siblings, 1 reply; 10+ messages in thread
From: Viresh Kumar @ 2013-11-28 14:19 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Lists linaro-kernel, Patch Tracking, cpufreq@vger.kernel.org,
linux-pm@vger.kernel.org, Linux Kernel Mailing List,
Nishanth Menon, Stephen Warren, Kgene Kim, linux-samsung-soc,
linux-tegra, jinchoi, Lan Tianyu, Sebastian Capella,
Jonghwan Choi
On 28 November 2013 19:53, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> I don't think that the Nishanth's issue is fixed by this particular version of
> the patch, so I modified the changelog and removed a the comment above
> cpufreq_suspend()
No, it does fix his issue as we are just stopping the governors from
dpm_suspend_noirq() and not initiating any new transitions. I said we need
these calls from dpm_suspend() instead as platforms like exynos and tegra
do need to change frequency before suspending..
See:
https://lkml.org/lkml/2013/11/25/692
> (which should be a proper kerneldoc one if any, BTW).
Okay.. I will add that separately..
> I've also made some minor changes to the conditionals, because I didn't like them
> the way they were written originally.
>
> Please check the result in bleeding-edge.
Everything else is fine..
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH V4] cpufreq: suspend governors on system suspend/hibernate
2013-11-27 3:39 [PATCH V4] cpufreq: suspend governors on system suspend/hibernate Viresh Kumar
@ 2013-11-28 14:23 ` Rafael J. Wysocki
2013-11-28 14:19 ` Viresh Kumar
0 siblings, 1 reply; 10+ messages in thread
From: Rafael J. Wysocki @ 2013-11-28 14:23 UTC (permalink / raw)
To: Viresh Kumar
Cc: linaro-kernel, patches, cpufreq, linux-pm, linux-kernel, nm,
swarren, kgene.kim, linux-samsung-soc, linux-tegra, jinchoi,
tianyu.lan, sebastian.capella, jhbird.choi
On Wednesday, November 27, 2013 09:09:42 AM Viresh Kumar wrote:
> This patch adds cpufreq callbacks to dpm_{suspend|resume}_noirq() for handling
> suspend/resume of cpufreq governors.
>
> There are multiple problems that are fixed by this patch:
> - Nishanth Menon (TI) found an interesting problem on his platform, OMAP. His board
> wasn't working well with suspend/resume as calls for removing non-boot CPUs
> was turning out into a call to drivers ->target() which then tries to play
> with regulators. But regulators and their I2C bus were already suspended and
> this resulted in a failure. Many platforms have such problems, samsung, tegra,
> etc.. They solved it with driver specific PM notifiers where they used to
> disable their driver's ->target() routine.
I don't think that the Nishanth's issue is fixed by this particular version of
the patch, so I modified the changelog and removed a the comment above
cpufreq_suspend() (which should be a proper kerneldoc one if any, BTW).
I've also made some minor changes to the conditionals, because I didn't like them
the way they were written originally.
Please check the result in bleeding-edge.
Thanks!
> - Lan Tianyu (Intel) & Jinhyuk Choi (Broadcom) found another issue where
> tunables configuration for clusters/sockets with non-boot CPUs was getting
> lost after suspend/resume, as we were notifying governors with
> CPUFREQ_GOV_POLICY_EXIT on removal of the last cpu for that policy and so
> deallocating memory for tunables. This is also fixed with this patch as we
> don't allow any operation on Governors during suspend/resume now.
>
> Reported-and-tested-by: Lan Tianyu <tianyu.lan@intel.com>
> Reported-and-tested-by: Nishanth Menon <nm@ti.com>
> Reported-by: Jinhyuk Choi <jinchoi@broadcom.com>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>
> This is almost same as 1/6 of V3 version of this patchset:
>
> https://lkml.org/lkml/2013/11/25/838
>
> This is done to get some initial fixes for 3.13. These are already tested by
> both the reporters of initial problems. Tegra/exynos/s5p will keep running their
> PM notifiers until v3.14, as they are currently able to work with them..
>
> drivers/base/power/main.c | 3 +++
> drivers/cpufreq/cpufreq.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/cpufreq.h | 8 ++++++++
> 3 files changed, 61 insertions(+)
>
> diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
> index 1b41fca..e3219df 100644
> --- a/drivers/base/power/main.c
> +++ b/drivers/base/power/main.c
> @@ -29,6 +29,7 @@
> #include <linux/async.h>
> #include <linux/suspend.h>
> #include <trace/events/power.h>
> +#include <linux/cpufreq.h>
> #include <linux/cpuidle.h>
> #include <linux/timer.h>
>
> @@ -540,6 +541,7 @@ static void dpm_resume_noirq(pm_message_t state)
> dpm_show_time(starttime, state, "noirq");
> resume_device_irqs();
> cpuidle_resume();
> + cpufreq_resume();
> }
>
> /**
> @@ -955,6 +957,7 @@ static int dpm_suspend_noirq(pm_message_t state)
> ktime_t starttime = ktime_get();
> int error = 0;
>
> + cpufreq_suspend();
> cpuidle_pause();
> suspend_device_irqs();
> mutex_lock(&dpm_list_mtx);
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 02d534d..b6c7821 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -26,6 +26,7 @@
> #include <linux/module.h>
> #include <linux/mutex.h>
> #include <linux/slab.h>
> +#include <linux/suspend.h>
> #include <linux/syscore_ops.h>
> #include <linux/tick.h>
> #include <trace/events/power.h>
> @@ -47,6 +48,9 @@ static LIST_HEAD(cpufreq_policy_list);
> static DEFINE_PER_CPU(char[CPUFREQ_NAME_LEN], cpufreq_cpu_governor);
> #endif
>
> +/* Flag to suspend/resume CPUFreq governors */
> +static bool cpufreq_suspended;
> +
> static inline bool has_target(void)
> {
> return cpufreq_driver->target_index || cpufreq_driver->target;
> @@ -1462,6 +1466,48 @@ static struct subsys_interface cpufreq_interface = {
> .remove_dev = cpufreq_remove_dev,
> };
>
> +/*
> + * Callbacks for suspending/resuming governors as some platforms can't change
> + * frequency after this point in suspend cycle. Because some of the devices
> + * (like: i2c, regulators, etc) they use for changing frequency are suspended
> + * quickly after this point.
> + */
> +void cpufreq_suspend(void)
> +{
> + struct cpufreq_policy *policy;
> +
> + if (!has_target())
> + return;
> +
> + pr_debug("%s: Suspending Governors\n", __func__);
> +
> + list_for_each_entry(policy, &cpufreq_policy_list, policy_list)
> + if (__cpufreq_governor(policy, CPUFREQ_GOV_STOP))
> + pr_err("%s: Failed to stop governor for policy: %p\n",
> + __func__, policy);
> +
> + cpufreq_suspended = true;
> +}
> +
> +void cpufreq_resume(void)
> +{
> + struct cpufreq_policy *policy;
> +
> + if (!has_target())
> + return;
> +
> + pr_debug("%s: Resuming Governors\n", __func__);
> +
> + cpufreq_suspended = false;
> +
> + list_for_each_entry(policy, &cpufreq_policy_list, policy_list)
> + if (__cpufreq_governor(policy, CPUFREQ_GOV_START) ||
> + __cpufreq_governor(policy,
> + CPUFREQ_GOV_LIMITS))
> + pr_err("%s: Failed to start governor for policy: %p\n",
> + __func__, policy);
> +}
> +
> /**
> * cpufreq_bp_suspend - Prepare the boot CPU for system suspend.
> *
> @@ -1764,6 +1810,10 @@ static int __cpufreq_governor(struct cpufreq_policy *policy,
> struct cpufreq_governor *gov = NULL;
> #endif
>
> + /* Don't start any governor operations if we are entering suspend */
> + if (cpufreq_suspended)
> + return 0;
> +
> if (policy->governor->max_transition_latency &&
> policy->cpuinfo.transition_latency >
> policy->governor->max_transition_latency) {
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index dc196bb..ee5fe9d 100644
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -280,6 +280,14 @@ cpufreq_verify_within_cpu_limits(struct cpufreq_policy *policy)
> policy->cpuinfo.max_freq);
> }
>
> +#ifdef CONFIG_CPU_FREQ
> +void cpufreq_suspend(void);
> +void cpufreq_resume(void);
> +#else
> +static inline void cpufreq_suspend(void) {}
> +static inline void cpufreq_resume(void) {}
> +#endif
> +
> /*********************************************************************
> * CPUFREQ NOTIFIER INTERFACE *
> *********************************************************************/
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH V4] cpufreq: suspend governors on system suspend/hibernate
2013-11-28 14:19 ` Viresh Kumar
@ 2013-11-28 20:29 ` Rafael J. Wysocki
2013-11-29 3:26 ` Viresh Kumar
0 siblings, 1 reply; 10+ messages in thread
From: Rafael J. Wysocki @ 2013-11-28 20:29 UTC (permalink / raw)
To: Viresh Kumar
Cc: Lists linaro-kernel, Patch Tracking, cpufreq@vger.kernel.org,
linux-pm@vger.kernel.org, Linux Kernel Mailing List,
Nishanth Menon, Stephen Warren, Kgene Kim, linux-samsung-soc,
linux-tegra, jinchoi, Lan Tianyu, Sebastian Capella,
Jonghwan Choi
On Thursday, November 28, 2013 07:49:29 PM Viresh Kumar wrote:
> On 28 November 2013 19:53, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> > I don't think that the Nishanth's issue is fixed by this particular version of
> > the patch, so I modified the changelog and removed a the comment above
> > cpufreq_suspend()
>
> No, it does fix his issue as we are just stopping the governors from
> dpm_suspend_noirq() and not initiating any new transitions. I said we need
> these calls from dpm_suspend() instead as platforms like exynos and tegra
> do need to change frequency before suspending..
>
> See:
>
> https://lkml.org/lkml/2013/11/25/692
>
> > (which should be a proper kerneldoc one if any, BTW).
>
> Okay.. I will add that separately..
>
> > I've also made some minor changes to the conditionals, because I didn't like them
> > the way they were written originally.
> >
> > Please check the result in bleeding-edge.
>
> Everything else is fine..
So I'm reading this as "the version of the patch in bleeding-edge is OK".
I hope that really is the case. :-)
Thanks!
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH V4] cpufreq: suspend governors on system suspend/hibernate
2013-11-28 20:29 ` Rafael J. Wysocki
@ 2013-11-29 3:26 ` Viresh Kumar
2013-12-07 11:01 ` Bjørn Mork
0 siblings, 1 reply; 10+ messages in thread
From: Viresh Kumar @ 2013-11-29 3:26 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Lists linaro-kernel, Patch Tracking, cpufreq@vger.kernel.org,
linux-pm@vger.kernel.org, Linux Kernel Mailing List,
Nishanth Menon, Stephen Warren, Kgene Kim, linux-samsung-soc,
linux-tegra, jinchoi, Lan Tianyu, Sebastian Capella,
Jonghwan Choi
On 29 November 2013 01:59, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> So I'm reading this as "the version of the patch in bleeding-edge is OK".
>
> I hope that really is the case. :-)
Yeah, that's correct.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH V4] cpufreq: suspend governors on system suspend/hibernate
2013-11-29 3:26 ` Viresh Kumar
@ 2013-12-07 11:01 ` Bjørn Mork
2013-12-07 15:24 ` Paul Bolle
0 siblings, 1 reply; 10+ messages in thread
From: Bjørn Mork @ 2013-12-07 11:01 UTC (permalink / raw)
To: Viresh Kumar
Cc: Rafael J. Wysocki, Lists linaro-kernel, Patch Tracking,
cpufreq@vger.kernel.org, linux-pm@vger.kernel.org,
Linux Kernel Mailing List, Nishanth Menon, Stephen Warren,
Kgene Kim, linux-samsung-soc, linux-tegra, jinchoi, Lan Tianyu,
Sebastian Capella, Jonghwan Choi
Viresh Kumar <viresh.kumar@linaro.org> writes:
> On 29 November 2013 01:59, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>> So I'm reading this as "the version of the patch in bleeding-edge is OK".
>>
>> I hope that really is the case. :-)
>
> Yeah, that's correct.
Sorry to be making noise here again, but I was eager to verify that the
fixes in v3.13-rc3 were OK on my system. Unfortunately it seems this
patch completely broke suspend for me. Hibernete ended up with a blank
console and no visible activity, without ever writing any image to disk.
There really weren't that many suspects between v3.13-rc2 and v3.13-rc3,
so I went directly to reverting 5a87182aa21d ("cpufreq: suspend
governors on system suspend/hibernate") which fixed the problem. I
didn't bother debugging it further from there. I don't think there is
anything magic about my system which should make this problem specific
to it.
I am still using the acpi-cpufreq driver on an old x86_64 laptop if that
matters.
Bj√∏rn
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH V4] cpufreq: suspend governors on system suspend/hibernate
2013-12-07 11:01 ` Bjørn Mork
@ 2013-12-07 15:24 ` Paul Bolle
2013-12-08 0:34 ` Rafael J. Wysocki
0 siblings, 1 reply; 10+ messages in thread
From: Paul Bolle @ 2013-12-07 15:24 UTC (permalink / raw)
To: Bjørn Mork
Cc: Viresh Kumar, Rafael J. Wysocki, Lists linaro-kernel,
Patch Tracking, cpufreq@vger.kernel.org, linux-pm@vger.kernel.org,
Linux Kernel Mailing List, Nishanth Menon, Stephen Warren,
Kgene Kim, linux-samsung-soc, linux-tegra, jinchoi, Lan Tianyu,
Sebastian Capella, Jonghwan Choi
On Sat, 2013-12-07 at 12:01 +0100, Bjørn Mork wrote:
> Sorry to be making noise here again, but I was eager to verify that the
> fixes in v3.13-rc3 were OK on my system. Unfortunately it seems this
> patch completely broke suspend for me. Hibernete ended up with a blank
> console and no visible activity, without ever writing any image to disk.
>
> There really weren't that many suspects between v3.13-rc2 and v3.13-rc3,
> so I went directly to reverting 5a87182aa21d ("cpufreq: suspend
> governors on system suspend/hibernate") which fixed the problem. I
> didn't bother debugging it further from there. I don't think there is
> anything magic about my system which should make this problem specific
> to it.
>
> I am still using the acpi-cpufreq driver on an old x86_64 laptop if that
> matters.
On an x86 (32 bits) laptop hibernate also broke in v3.13-rc3. On that
machine hibernation itself worked, or at least seemed to work, but the
machine would basically stop after thawing (directly after loading the
hibernation image).
And reverting commit 5a87182aa21d ("cpufreq: suspend governors on system
suspend/hibernate"), on top of v3.13-rc3, also lead to a successful
hibernation/thaw cycle.
Paul Bolle
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH V4] cpufreq: suspend governors on system suspend/hibernate
2013-12-07 15:24 ` Paul Bolle
@ 2013-12-08 0:34 ` Rafael J. Wysocki
[not found] ` <3466098.C5M665z3nb-sKB8Sp2ER+y1GS7QM15AGw@public.gmane.org>
0 siblings, 1 reply; 10+ messages in thread
From: Rafael J. Wysocki @ 2013-12-08 0:34 UTC (permalink / raw)
To: Paul Bolle
Cc: Bjørn Mork, Viresh Kumar, Lists linaro-kernel,
Patch Tracking, cpufreq@vger.kernel.org, linux-pm@vger.kernel.org,
Linux Kernel Mailing List, Nishanth Menon, Stephen Warren,
Kgene Kim, linux-samsung-soc, linux-tegra, jinchoi, Lan Tianyu,
Sebastian Capella, Jonghwan Choi
On Saturday, December 07, 2013 04:24:09 PM Paul Bolle wrote:
> On Sat, 2013-12-07 at 12:01 +0100, Bjørn Mork wrote:
> > Sorry to be making noise here again, but I was eager to verify that the
> > fixes in v3.13-rc3 were OK on my system. Unfortunately it seems this
> > patch completely broke suspend for me. Hibernete ended up with a blank
> > console and no visible activity, without ever writing any image to disk.
> >
> > There really weren't that many suspects between v3.13-rc2 and v3.13-rc3,
> > so I went directly to reverting 5a87182aa21d ("cpufreq: suspend
> > governors on system suspend/hibernate") which fixed the problem. I
> > didn't bother debugging it further from there. I don't think there is
> > anything magic about my system which should make this problem specific
> > to it.
> >
> > I am still using the acpi-cpufreq driver on an old x86_64 laptop if that
> > matters.
>
> On an x86 (32 bits) laptop hibernate also broke in v3.13-rc3. On that
> machine hibernation itself worked, or at least seemed to work, but the
> machine would basically stop after thawing (directly after loading the
> hibernation image).
>
> And reverting commit 5a87182aa21d ("cpufreq: suspend governors on system
> suspend/hibernate"), on top of v3.13-rc3, also lead to a successful
> hibernation/thaw cycle.
OK, reverted.
I'll send a pull request with that revert shortly.
Thanks,
Rafael
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH V4] cpufreq: suspend governors on system suspend/hibernate
[not found] ` <3466098.C5M665z3nb-sKB8Sp2ER+y1GS7QM15AGw@public.gmane.org>
@ 2013-12-08 12:41 ` Borislav Petkov
2013-12-12 1:50 ` Rafael J. Wysocki
0 siblings, 1 reply; 10+ messages in thread
From: Borislav Petkov @ 2013-12-08 12:41 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Paul Bolle, Bjørn Mork, Viresh Kumar, Lists linaro-kernel,
Patch Tracking, cpufreq-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Linux Kernel Mailing List, Nishanth Menon, Stephen Warren,
Kgene Kim, linux-samsung-soc, linux-tegra-u79uwXL29TY76Z2rM5mHXA,
jinchoi-dY08KVG/lbpWk0Htik3J/w, Lan Tianyu, Sebastian Capella,
Jonghwan Choi
On Sun, Dec 08, 2013 at 01:34:36AM +0100, Rafael J. Wysocki wrote:
> On Saturday, December 07, 2013 04:24:09 PM Paul Bolle wrote:
> > On Sat, 2013-12-07 at 12:01 +0100, Bjørn Mork wrote:
> > > Sorry to be making noise here again, but I was eager to verify that the
> > > fixes in v3.13-rc3 were OK on my system. Unfortunately it seems this
> > > patch completely broke suspend for me. Hibernete ended up with a blank
> > > console and no visible activity, without ever writing any image to disk.
> > >
> > > There really weren't that many suspects between v3.13-rc2 and v3.13-rc3,
> > > so I went directly to reverting 5a87182aa21d ("cpufreq: suspend
> > > governors on system suspend/hibernate") which fixed the problem. I
> > > didn't bother debugging it further from there. I don't think there is
> > > anything magic about my system which should make this problem specific
> > > to it.
> > >
> > > I am still using the acpi-cpufreq driver on an old x86_64 laptop if that
> > > matters.
> >
> > On an x86 (32 bits) laptop hibernate also broke in v3.13-rc3. On that
> > machine hibernation itself worked, or at least seemed to work, but the
> > machine would basically stop after thawing (directly after loading the
> > hibernation image).
> >
> > And reverting commit 5a87182aa21d ("cpufreq: suspend governors on system
> > suspend/hibernate"), on top of v3.13-rc3, also lead to a successful
> > hibernation/thaw cycle.
>
> OK, reverted.
+1.
Grrr, I just wasted a whole morning bisecting the same issue as reported
above and can confirm the revert is the right thing to do.
Well, almost: the box almost suspends until I hear the clicking sound of
the mainboard trying to turn stuff off but then the fan remains on. When
I hit a button on the keyboard, it then turns off completely. This is
probably some nasty BIOS SMI crap which we don't have control over and
I'd guess unrelated to the issue at hand.
Anyway, Rafael, I was wondering: I have a couple of boxes here and it
would probably make a good sense to help out with testing this stuff
more - I just can't have suspend breakages.
Maybe you'd like me to give a run of your tree now and then to check
whether someone has sent you a quickly rushed brownpaper bag of
questionable nature and we can fish it out before it hits Linus. :)
So let me know.
Thanks.
--
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH V4] cpufreq: suspend governors on system suspend/hibernate
2013-12-08 12:41 ` Borislav Petkov
@ 2013-12-12 1:50 ` Rafael J. Wysocki
0 siblings, 0 replies; 10+ messages in thread
From: Rafael J. Wysocki @ 2013-12-12 1:50 UTC (permalink / raw)
To: Borislav Petkov
Cc: Paul Bolle, Bjørn Mork, Viresh Kumar, Lists linaro-kernel,
Patch Tracking, cpufreq@vger.kernel.org, linux-pm@vger.kernel.org,
Linux Kernel Mailing List, Nishanth Menon, Stephen Warren,
Kgene Kim, linux-samsung-soc, linux-tegra, jinchoi, Lan Tianyu,
Sebastian Capella, Jonghwan Choi
On Sunday, December 08, 2013 01:41:08 PM Borislav Petkov wrote:
> On Sun, Dec 08, 2013 at 01:34:36AM +0100, Rafael J. Wysocki wrote:
> > On Saturday, December 07, 2013 04:24:09 PM Paul Bolle wrote:
> > > On Sat, 2013-12-07 at 12:01 +0100, Bjørn Mork wrote:
> > > > Sorry to be making noise here again, but I was eager to verify that the
> > > > fixes in v3.13-rc3 were OK on my system. Unfortunately it seems this
> > > > patch completely broke suspend for me. Hibernete ended up with a blank
> > > > console and no visible activity, without ever writing any image to disk.
> > > >
> > > > There really weren't that many suspects between v3.13-rc2 and v3.13-rc3,
> > > > so I went directly to reverting 5a87182aa21d ("cpufreq: suspend
> > > > governors on system suspend/hibernate") which fixed the problem. I
> > > > didn't bother debugging it further from there. I don't think there is
> > > > anything magic about my system which should make this problem specific
> > > > to it.
> > > >
> > > > I am still using the acpi-cpufreq driver on an old x86_64 laptop if that
> > > > matters.
> > >
> > > On an x86 (32 bits) laptop hibernate also broke in v3.13-rc3. On that
> > > machine hibernation itself worked, or at least seemed to work, but the
> > > machine would basically stop after thawing (directly after loading the
> > > hibernation image).
> > >
> > > And reverting commit 5a87182aa21d ("cpufreq: suspend governors on system
> > > suspend/hibernate"), on top of v3.13-rc3, also lead to a successful
> > > hibernation/thaw cycle.
> >
> > OK, reverted.
>
> +1.
>
> Grrr, I just wasted a whole morning bisecting the same issue as reported
> above and can confirm the revert is the right thing to do.
>
> Well, almost: the box almost suspends until I hear the clicking sound of
> the mainboard trying to turn stuff off but then the fan remains on. When
> I hit a button on the keyboard, it then turns off completely. This is
> probably some nasty BIOS SMI crap which we don't have control over and
> I'd guess unrelated to the issue at hand.
>
> Anyway, Rafael, I was wondering: I have a couple of boxes here and it
> would probably make a good sense to help out with testing this stuff
> more - I just can't have suspend breakages.
>
> Maybe you'd like me to give a run of your tree now and then to check
> whether someone has sent you a quickly rushed brownpaper bag of
> questionable nature and we can fish it out before it hits Linus. :)
>
> So let me know.
That would be great, thanks!
My tree is not too interesting at the moment from the cpufreq testing
perspective, but I'll let you know when there's more stuff in there.
Thanks,
Rafael
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2013-12-12 1:50 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-27 3:39 [PATCH V4] cpufreq: suspend governors on system suspend/hibernate Viresh Kumar
2013-11-28 14:23 ` Rafael J. Wysocki
2013-11-28 14:19 ` Viresh Kumar
2013-11-28 20:29 ` Rafael J. Wysocki
2013-11-29 3:26 ` Viresh Kumar
2013-12-07 11:01 ` Bjørn Mork
2013-12-07 15:24 ` Paul Bolle
2013-12-08 0:34 ` Rafael J. Wysocki
[not found] ` <3466098.C5M665z3nb-sKB8Sp2ER+y1GS7QM15AGw@public.gmane.org>
2013-12-08 12:41 ` Borislav Petkov
2013-12-12 1:50 ` Rafael J. Wysocki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).