* [PATCH 10/15] gma500: don't register the ACPI video bus
From: Len Brown @ 2012-06-04 5:40 UTC (permalink / raw)
To: linux-acpi, linux-pm; +Cc: Len Brown, linux-kernel, Alan Cox
In-Reply-To: <c5971456964290da7e98222892797b71ef793e62.1338786831.git.len.brown@intel.com>
From: Alan Cox <alan@linux.intel.com>
We are not yet ready for this and it makes a mess on some devices.
Signed-off-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
drivers/gpu/drm/gma500/psb_drv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c
index c34adf9..09af2ff 100644
--- a/drivers/gpu/drm/gma500/psb_drv.c
+++ b/drivers/gpu/drm/gma500/psb_drv.c
@@ -349,7 +349,7 @@ static int psb_driver_load(struct drm_device *dev, unsigned long chipset)
PSB_WSGX32(0x30000000, PSB_CR_BIF_3D_REQ_BASE);
/* igd_opregion_init(&dev_priv->opregion_dev); */
- acpi_video_register();
+/* acpi_video_register(); */
if (dev_priv->lid_state)
psb_lid_timer_init(dev_priv);
--
1.7.11.rc0.100.g5498c5f
^ permalink raw reply related
* [PATCH 11/15] ACPI video: use after input_unregister_device()
From: Len Brown @ 2012-06-04 5:40 UTC (permalink / raw)
To: linux-acpi, linux-pm; +Cc: Len Brown, linux-kernel, Dan Carpenter
In-Reply-To: <c5971456964290da7e98222892797b71ef793e62.1338786831.git.len.brown@intel.com>
From: Dan Carpenter <dan.carpenter@oracle.com>
We can't use "input" anymore after calling input_unregister_device().
The call to input_free_device() is a double free. The normal way to
deal with this is to make input_register_device() the last function
called in the function.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Acked-by: Dmitry Torokhov <dtor@mail.ru>
Signed-off-by: Len Brown <len.brown@intel.com>
---
drivers/acpi/video.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 609262d..a576575 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -1687,10 +1687,6 @@ static int acpi_video_bus_add(struct acpi_device *device)
set_bit(KEY_BRIGHTNESS_ZERO, input->keybit);
set_bit(KEY_DISPLAY_OFF, input->keybit);
- error = input_register_device(input);
- if (error)
- goto err_stop_video;
-
printk(KERN_INFO PREFIX "%s [%s] (multi-head: %s rom: %s post: %s)\n",
ACPI_VIDEO_DEVICE_NAME, acpi_device_bid(device),
video->flags.multihead ? "yes" : "no",
@@ -1701,12 +1697,16 @@ static int acpi_video_bus_add(struct acpi_device *device)
video->pm_nb.priority = 0;
error = register_pm_notifier(&video->pm_nb);
if (error)
- goto err_unregister_input_dev;
+ goto err_stop_video;
+
+ error = input_register_device(input);
+ if (error)
+ goto err_unregister_pm_notifier;
return 0;
- err_unregister_input_dev:
- input_unregister_device(input);
+ err_unregister_pm_notifier:
+ unregister_pm_notifier(&video->pm_nb);
err_stop_video:
acpi_video_bus_stop_devices(video);
err_free_input_dev:
--
1.7.11.rc0.100.g5498c5f
^ permalink raw reply related
* [PATCH 12/15] tools/power turbostat: fix un-intended affinity of forked program
From: Len Brown @ 2012-06-04 5:40 UTC (permalink / raw)
To: linux-acpi, linux-pm; +Cc: linux-kernel, Len Brown
In-Reply-To: <c5971456964290da7e98222892797b71ef793e62.1338786831.git.len.brown@intel.com>
From: Len Brown <len.brown@intel.com>
Linux 3.4 included a modification to turbostat to
lower cross-call overhead by using scheduler affinity:
15aaa34654831e98dd76f7738b6c7f5d05a66430
(tools turbostat: reduce measurement overhead due to IPIs)
In the use-case where turbostat forks a child program,
that change had the un-intended side-effect of binding
the child to the last cpu in the system.
This change removed the binding before forking the child.
This is a back-port of a fix already included in turbostat v2.
Signed-off-by: Len Brown <len.brown@intel.com>
---
tools/power/x86/turbostat/turbostat.c | 28 ++++++++++++++++++++++++++--
1 file changed, 26 insertions(+), 2 deletions(-)
diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
index ab2f682..d5d6a3d 100644
--- a/tools/power/x86/turbostat/turbostat.c
+++ b/tools/power/x86/turbostat/turbostat.c
@@ -73,8 +73,8 @@ int backwards_count;
char *progname;
int num_cpus;
-cpu_set_t *cpu_mask;
-size_t cpu_mask_size;
+cpu_set_t *cpu_present_set, *cpu_mask;
+size_t cpu_present_setsize, cpu_mask_size;
struct counters {
unsigned long long tsc; /* per thread */
@@ -103,6 +103,12 @@ struct timeval tv_even;
struct timeval tv_odd;
struct timeval tv_delta;
+int mark_cpu_present(int pkg, int core, int cpu)
+{
+ CPU_SET_S(cpu, cpu_present_setsize, cpu_present_set);
+ return 0;
+}
+
/*
* cpu_mask_init(ncpus)
*
@@ -118,6 +124,18 @@ void cpu_mask_init(int ncpus)
}
cpu_mask_size = CPU_ALLOC_SIZE(ncpus);
CPU_ZERO_S(cpu_mask_size, cpu_mask);
+
+ /*
+ * Allocate and initialize cpu_present_set
+ */
+ cpu_present_set = CPU_ALLOC(ncpus);
+ if (cpu_present_set == NULL) {
+ perror("CPU_ALLOC");
+ exit(3);
+ }
+ cpu_present_setsize = CPU_ALLOC_SIZE(ncpus);
+ CPU_ZERO_S(cpu_present_setsize, cpu_present_set);
+ for_all_cpus(mark_cpu_present);
}
void cpu_mask_uninit()
@@ -125,6 +143,9 @@ void cpu_mask_uninit()
CPU_FREE(cpu_mask);
cpu_mask = NULL;
cpu_mask_size = 0;
+ CPU_FREE(cpu_present_set);
+ cpu_present_set = NULL;
+ cpu_present_setsize = 0;
}
int cpu_migrate(int cpu)
@@ -1047,6 +1068,9 @@ int fork_it(char **argv)
int retval;
pid_t child_pid;
get_counters(cnt_even);
+
+ /* clear affinity side-effect of get_counters() */
+ sched_setaffinity(0, cpu_present_setsize, cpu_present_set);
gettimeofday(&tv_even, (struct timezone *)NULL);
child_pid = fork();
--
1.7.11.rc0.100.g5498c5f
^ permalink raw reply related
* [PATCH 13/15] tools/power turbostat: fix IVB support
From: Len Brown @ 2012-06-04 5:40 UTC (permalink / raw)
To: linux-acpi, linux-pm; +Cc: linux-kernel, Len Brown
In-Reply-To: <c5971456964290da7e98222892797b71ef793e62.1338786831.git.len.brown@intel.com>
From: Len Brown <len.brown@intel.com>
Initial IVB support went into turbostat in Linux-3.1:
553575f1ae048aa44682b46b3c51929a0b3ad337
(tools turbostat: recognize and run properly on IVB)
However, when running on IVB, turbostat would fail
to report the new couters added with SNB, c7, pc2 and pc7.
So in scenarios where these counters are non-zero on IVB,
turbostat would report erroneous residencey results.
In particular c7 time would be added to c1 time,
since c1 time is calculated as "that which is left over".
Also, turbostat reports MHz capabilities when passed
the "-v" option, and it would incorrectly report 133MHz
bclk instead of 100MHz bclk for IVB, which would inflate
GHz reported with that option.
This patch is a backport of a fix already included in turbostat v2.
Signed-off-by: Len Brown <len.brown@intel.com>
---
tools/power/x86/turbostat/turbostat.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
index d5d6a3d..16de7ad 100644
--- a/tools/power/x86/turbostat/turbostat.c
+++ b/tools/power/x86/turbostat/turbostat.c
@@ -933,6 +933,8 @@ int is_snb(unsigned int family, unsigned int model)
switch (model) {
case 0x2A:
case 0x2D:
+ case 0x3A: /* IVB */
+ case 0x3D: /* IVB Xeon */
return 1;
}
return 0;
--
1.7.11.rc0.100.g5498c5f
^ permalink raw reply related
* [PATCH 14/15] drivers: acpi: Fix dependency for ACPI_HOTPLUG_CPU
From: Len Brown @ 2012-06-04 5:40 UTC (permalink / raw)
To: linux-acpi, linux-pm
Cc: linux-kernel, Fabio Estevam, Fabio Estevam, Len Brown
In-Reply-To: <c5971456964290da7e98222892797b71ef793e62.1338786831.git.len.brown@intel.com>
From: Fabio Estevam <festevam@gmail.com>
Fix the following build warning:
warning: (ACPI_HOTPLUG_CPU) selects ACPI_CONTAINER which has unmet direct dependencies (ACPI && EXPERIMENTAL)
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
drivers/acpi/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 47768ff..8099895 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -208,7 +208,7 @@ config ACPI_IPMI
config ACPI_HOTPLUG_CPU
bool
- depends on ACPI_PROCESSOR && HOTPLUG_CPU
+ depends on EXPERIMENTAL && ACPI_PROCESSOR && HOTPLUG_CPU
select ACPI_CONTAINER
default y
--
1.7.11.rc0.100.g5498c5f
^ permalink raw reply related
* [PATCH 15/15] ACPI: fix acpi_bus.h build warnings when ACPI is not enabled
From: Len Brown @ 2012-06-04 5:40 UTC (permalink / raw)
To: linux-acpi, linux-pm; +Cc: linux-kernel, Len Brown
In-Reply-To: <c5971456964290da7e98222892797b71ef793e62.1338786831.git.len.brown@intel.com>
From: Len Brown <len.brown@intel.com>
introduced in Linux-3.5-rc1 by
66886d6f8c9bcdee3d7fce5796dcffd6b4bc0b48
(ACPI: Add stubs for (un)register_acpi_bus_type)
Fix header file warnings when CONFIG_ACPI is not enabled:
include/acpi/acpi_bus.h:443:42: warning: 'struct acpi_bus_type' declared inside parameter list
include/acpi/acpi_bus.h:443:42: warning: its scope is only this definition or declaration, which is probably not
include/acpi/acpi_bus.h:444:44: warning: 'struct acpi_bus_type' declared inside parameter list
Signed-off-by: Len Brown <len.brown@intel.com>
---
include/acpi/acpi_bus.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index b0d6282..9e6e1c6 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -440,8 +440,8 @@ static inline int acpi_pm_device_sleep_wake(struct device *dev, bool enable)
#else /* CONFIG_ACPI */
-static int register_acpi_bus_type(struct acpi_bus_type *bus) { return 0; }
-static int unregister_acpi_bus_type(struct acpi_bus_type *bus) { return 0; }
+static inline int register_acpi_bus_type(void *bus) { return 0; }
+static inline int unregister_acpi_bus_type(void *bus) { return 0; }
#endif /* CONFIG_ACPI */
--
1.7.11.rc0.100.g5498c5f
^ permalink raw reply related
* Re: [linux-pm] [PATCH 0/2] bug fixes for coupled cpuidle
From: Kevin Hilman @ 2012-06-05 18:12 UTC (permalink / raw)
To: Len Brown
Cc: Colin Cross, linux-kernel, Amit Kucheria, linux-pm,
Arjan van de Ven, linux-arm-kernel
In-Reply-To: <4FC9A9BA.1020904@kernel.org>
Hi Len,
Len Brown <lenb@kernel.org> writes:
> On 05/18/2012 02:05 PM, Colin Cross wrote:
>
>> The last modifications made to the coupled cpuidle patches introduced
>> two bugs that I missed during testing. The online count was never
>> initialized, causing coupled idle to always wait and never enter the
>> ready loop. That hid the second bug, the ready count could never be
>> decremented after exiting idle.
>>
>> Len, these two patches could be squashed into patch 3 of the original
>> set. If you do squash them, you could also add Rafael's tags to the
>> set (Reviewed-by on 1 and 2, acked-by on 3).
>
>
> squash & tags update done.
>
>> Or I can reupload the
>> whole stack as v5 if you prefer.
>
>
> no need.
Hmm, after the problems with the pull request, it looks like you dropped
the coupled CPUidle series completely. It's a shame that this has been
reviewed, tested and queued for so long only to see it dropped at the
last minute.
Instead of the squash, could you just queue the original series (that
has been in linux-next for some time) and then submit the fixes in
$SUBJECT series for v3.5-rc? That way we could still get this support
in for 3.5, which many of us are waiting for.
Thanks,
Kevin
^ permalink raw reply
* Re: Suspend/resume regressions on Lenovo S10-3
From: Deepthi Dharwar @ 2012-06-06 12:51 UTC (permalink / raw)
To: Dave Hansen, Linux PM mailing list
Cc: Kevin Hilman, Len Brown, Arnd Bergmann, Tomas M., LKML,
Srivatsa S. Bhat, preeti, linux-pm, Ferenc Wagner,
Arjan van de Ven, Jean Pihet
In-Reply-To: <4FC2DA8C.7050401@linux.vnet.ibm.com>
On 05/28/2012 07:23 AM, Dave Hansen wrote:
> I have a Lenovo S10-3 Atom netbook. It's always had some amount of
> trouble working with the intel_idle driver, so I usually compile that
> out an use the acpi one. However, just after 3.1, suspend/resume broke.
> 'echo mem > /sys/power/state' would hang before suspending. I bisected
> it down to the commits around:
>
> e978aa7d7d57d04eb5f88a7507c4fb98577def77 / v3.1-1-ge978aa7
>
> by Deepthi. But, current mainline (v3.4-07644-g07acfc2) hangs with a
> different symptom: it suspends, but hangs on resume from suspend. I
> think _that_ delta in the behavior was caused by:
>
> 3439a8da16bcad6b0982ece938c9f8299bb53584
>
> ACPI / cpuidle: Remove acpi_idle_suspend (to fix suspend
> regression)
>
> It's a bit of a pain to bisect these two different things in parallel.
> I was trying to tell git bisect 'good' on working suspend/resume, 'bad'
> on the hang during resume, and 'skip' on the hangs _during_ suspend. 83
> kernels in, I'm not sure that's working very well. :)
>
> Deepthi, do you have any idea why your patches broke me in the first
> place? Perhaps we should fix that regression first before we go on and
> try to figure out what changed to let it suspend again, but break later.
Hi Dave,
Sorry about my patches breaking your suspend-resume.
I, basically tried out building and booting 3.1 kernel with
my patch set to reproduce the failure. I could clearly
see suspend not happening. It turns out to be
a bug with my first patch in global registration
series submitted earlier.
e978aa7d7d57d04eb5f88a7507c4fb98577def77 / v3.1-1-ge978aa7
The following patch, fixes the suspend issues
seen on my laptop due to earlier cpuidle cleanup
(Lenevo T420 booting with acpi_idle enabled).
Can you please give this a try
on top of my patch set (without Rafael's fix)
and see if it fixes the problem for you.
I am not reverting acpi_idle_suspend flag and
hopefully it should resume fine too.
---
This patch fixes suspend-resume issue seen in the kernel 3.1
series using acpi_idle_driver because of cpuidle global
registration cleanup.
Here, when acpi_idle_suspend flag was set ( during suspend)
the interrupts were not getting enabled in acpi_idle_enter_bm()
routine which was causing the system to hang.
Signed-off-by: Deepthi Dharwar <deepthi@linux.vnet.ibm.com>
---
drivers/acpi/processor_idle.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index 24fe3af..6e35293 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -895,8 +895,9 @@ static int acpi_idle_enter_bm(struct cpuidle_device *dev,
if (unlikely(!pr))
return -EINVAL;
-
if (acpi_idle_suspend) {
+ local_irq_disable();
+ local_irq_enable();
cpu_relax();
return -EINVAL;
}
Cheers,
Deepthi
^ permalink raw reply related
* Re: Suspend/resume regressions on Lenovo S10-3
From: Rafael J. Wysocki @ 2012-06-06 13:43 UTC (permalink / raw)
To: Deepthi Dharwar
Cc: Kevin Hilman, Len Brown, Arnd Bergmann, Linux PM mailing list,
Tomas M., LKML, Dave Hansen, Srivatsa S. Bhat, preeti, linux-pm,
Ferenc Wagner, Arjan van de Ven, Jean Pihet
In-Reply-To: <4FCF5249.2070600@linux.vnet.ibm.com>
On Wednesday, June 06, 2012, Deepthi Dharwar wrote:
> On 05/28/2012 07:23 AM, Dave Hansen wrote:
>
> > I have a Lenovo S10-3 Atom netbook. It's always had some amount of
> > trouble working with the intel_idle driver, so I usually compile that
> > out an use the acpi one. However, just after 3.1, suspend/resume broke.
> > 'echo mem > /sys/power/state' would hang before suspending. I bisected
> > it down to the commits around:
> >
> > e978aa7d7d57d04eb5f88a7507c4fb98577def77 / v3.1-1-ge978aa7
> >
> > by Deepthi. But, current mainline (v3.4-07644-g07acfc2) hangs with a
> > different symptom: it suspends, but hangs on resume from suspend. I
> > think _that_ delta in the behavior was caused by:
> >
> > 3439a8da16bcad6b0982ece938c9f8299bb53584
> >
> > ACPI / cpuidle: Remove acpi_idle_suspend (to fix suspend
> > regression)
> >
> > It's a bit of a pain to bisect these two different things in parallel.
> > I was trying to tell git bisect 'good' on working suspend/resume, 'bad'
> > on the hang during resume, and 'skip' on the hangs _during_ suspend. 83
> > kernels in, I'm not sure that's working very well. :)
> >
> > Deepthi, do you have any idea why your patches broke me in the first
> > place? Perhaps we should fix that regression first before we go on and
> > try to figure out what changed to let it suspend again, but break later.
>
>
> Hi Dave,
>
> Sorry about my patches breaking your suspend-resume.
>
> I, basically tried out building and booting 3.1 kernel with
> my patch set to reproduce the failure. I could clearly
> see suspend not happening. It turns out to be
> a bug with my first patch in global registration
> series submitted earlier.
>
> e978aa7d7d57d04eb5f88a7507c4fb98577def77 / v3.1-1-ge978aa7
>
> The following patch, fixes the suspend issues
> seen on my laptop due to earlier cpuidle cleanup
> (Lenevo T420 booting with acpi_idle enabled).
> Can you please give this a try
> on top of my patch set (without Rafael's fix)
> and see if it fixes the problem for you.
> I am not reverting acpi_idle_suspend flag and
> hopefully it should resume fine too.
>
> ---
>
> This patch fixes suspend-resume issue seen in the kernel 3.1
> series using acpi_idle_driver because of cpuidle global
> registration cleanup.
> Here, when acpi_idle_suspend flag was set ( during suspend)
> the interrupts were not getting enabled in acpi_idle_enter_bm()
> routine which was causing the system to hang.
>
>
> Signed-off-by: Deepthi Dharwar <deepthi@linux.vnet.ibm.com>
>
> ---
> drivers/acpi/processor_idle.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
> index 24fe3af..6e35293 100644
> --- a/drivers/acpi/processor_idle.c
> +++ b/drivers/acpi/processor_idle.c
> @@ -895,8 +895,9 @@ static int acpi_idle_enter_bm(struct cpuidle_device *dev,
> if (unlikely(!pr))
> return -EINVAL;
>
> -
> if (acpi_idle_suspend) {
> + local_irq_disable();
> + local_irq_enable();
> cpu_relax();
> return -EINVAL;
> }
May I say this is ugly? Why can't we track the status of interrupts
properly here?
Rafael
^ permalink raw reply
* Re: Suspend/resume regressions on Lenovo S10-3
From: Dave Hansen @ 2012-06-06 14:48 UTC (permalink / raw)
To: Deepthi Dharwar
Cc: Linux PM mailing list, LKML, Tomas M., Ferenc Wagner,
Arnd Bergmann, Rafael J. Wysocki, Len Brown, Kevin Hilman,
Arjan van de Ven, Jean Pihet, Trinabh Gupta, linux-pm,
Srivatsa S. Bhat, preeti
In-Reply-To: <4FCF5249.2070600@linux.vnet.ibm.com>
On 06/06/2012 05:51 AM, Deepthi Dharwar wrote:
> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
> index 24fe3af..6e35293 100644
> --- a/drivers/acpi/processor_idle.c
> +++ b/drivers/acpi/processor_idle.c
> @@ -895,8 +895,9 @@ static int acpi_idle_enter_bm(struct cpuidle_device *dev,
> if (unlikely(!pr))
> return -EINVAL;
>
> -
> if (acpi_idle_suspend) {
> + local_irq_disable();
> + local_irq_enable();
> cpu_relax();
> return -EINVAL;
> }
Heh, that is quite the hack. :)
However, it does at least work around my problem: I can suspend again
with 46bcfad + your patch.
^ permalink raw reply
* Re: [linux-pm] Suspend/resume regressions on Lenovo S10-3
From: Alan Stern @ 2012-06-06 15:09 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Deepthi Dharwar, Kevin Hilman, Len Brown, Arnd Bergmann,
Linux PM mailing list, Tomas M., LKML, Dave Hansen,
Srivatsa S. Bhat, preeti, linux-pm, Ferenc Wagner,
Arjan van de Ven, Jean Pihet
In-Reply-To: <201206061543.03537.rjw@sisk.pl>
On Wed, 6 Jun 2012, Rafael J. Wysocki wrote:
> > --- a/drivers/acpi/processor_idle.c
> > +++ b/drivers/acpi/processor_idle.c
> > @@ -895,8 +895,9 @@ static int acpi_idle_enter_bm(struct cpuidle_device *dev,
> > if (unlikely(!pr))
> > return -EINVAL;
> >
> > -
> > if (acpi_idle_suspend) {
> > + local_irq_disable();
> > + local_irq_enable();
> > cpu_relax();
> > return -EINVAL;
> > }
>
> May I say this is ugly? Why can't we track the status of interrupts
> properly here?
It's not just ugly; it's illogical. What reason could there possibly
be for disabling interrupts and then enabling them again without doing
anything in between?
Alan Stern
^ permalink raw reply
* Re: Suspend/resume regressions on Lenovo S10-3
From: Deepthi Dharwar @ 2012-06-06 16:45 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Kevin Hilman, Len Brown, Arnd Bergmann, Linux PM mailing list,
Tomas M., LKML, Dave Hansen, Srivatsa S. Bhat, preeti, linux-pm,
Ferenc Wagner, Arjan van de Ven, Jean Pihet
In-Reply-To: <201206061543.03537.rjw@sisk.pl>
On 06/06/2012 07:13 PM, Rafael J. Wysocki wrote:
> On Wednesday, June 06, 2012, Deepthi Dharwar wrote:
>> On 05/28/2012 07:23 AM, Dave Hansen wrote:
>>
>>> I have a Lenovo S10-3 Atom netbook. It's always had some amount of
>>> trouble working with the intel_idle driver, so I usually compile that
>>> out an use the acpi one. However, just after 3.1, suspend/resume broke.
>>> 'echo mem > /sys/power/state' would hang before suspending. I bisected
>>> it down to the commits around:
>>>
>>> e978aa7d7d57d04eb5f88a7507c4fb98577def77 / v3.1-1-ge978aa7
>>>
>>> by Deepthi. But, current mainline (v3.4-07644-g07acfc2) hangs with a
>>> different symptom: it suspends, but hangs on resume from suspend. I
>>> think _that_ delta in the behavior was caused by:
>>>
>>> 3439a8da16bcad6b0982ece938c9f8299bb53584
>>>
>>> ACPI / cpuidle: Remove acpi_idle_suspend (to fix suspend
>>> regression)
>>>
>>> It's a bit of a pain to bisect these two different things in parallel.
>>> I was trying to tell git bisect 'good' on working suspend/resume, 'bad'
>>> on the hang during resume, and 'skip' on the hangs _during_ suspend. 83
>>> kernels in, I'm not sure that's working very well. :)
>>>
>>> Deepthi, do you have any idea why your patches broke me in the first
>>> place? Perhaps we should fix that regression first before we go on and
>>> try to figure out what changed to let it suspend again, but break later.
>>
>>
>> Hi Dave,
>>
>> Sorry about my patches breaking your suspend-resume.
>>
>> I, basically tried out building and booting 3.1 kernel with
>> my patch set to reproduce the failure. I could clearly
>> see suspend not happening. It turns out to be
>> a bug with my first patch in global registration
>> series submitted earlier.
>>
>> e978aa7d7d57d04eb5f88a7507c4fb98577def77 / v3.1-1-ge978aa7
>>
>> The following patch, fixes the suspend issues
>> seen on my laptop due to earlier cpuidle cleanup
>> (Lenevo T420 booting with acpi_idle enabled).
>> Can you please give this a try
>> on top of my patch set (without Rafael's fix)
>> and see if it fixes the problem for you.
>> I am not reverting acpi_idle_suspend flag and
>> hopefully it should resume fine too.
>>
>> ---
>>
>> This patch fixes suspend-resume issue seen in the kernel 3.1
>> series using acpi_idle_driver because of cpuidle global
>> registration cleanup.
>> Here, when acpi_idle_suspend flag was set ( during suspend)
>> the interrupts were not getting enabled in acpi_idle_enter_bm()
>> routine which was causing the system to hang.
>>
>>
>> Signed-off-by: Deepthi Dharwar <deepthi@linux.vnet.ibm.com>
>>
>> ---
>> drivers/acpi/processor_idle.c | 3 ++-
>> 1 files changed, 2 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
>> index 24fe3af..6e35293 100644
>> --- a/drivers/acpi/processor_idle.c
>> +++ b/drivers/acpi/processor_idle.c
>> @@ -895,8 +895,9 @@ static int acpi_idle_enter_bm(struct cpuidle_device *dev,
>> if (unlikely(!pr))
>> return -EINVAL;
>>
>> -
>> if (acpi_idle_suspend) {
>> + local_irq_disable();
>> + local_irq_enable();
>> cpu_relax();
>> return -EINVAL;
>> }
>
> May I say this is ugly? Why can't we track the status of interrupts
> properly here?
I agree. Just the irq_enable call should do the trick.
Once the cpu enters idle via cpu_idle call, irqs are disabled. We
need to enable them if acpi_idle_suspend is set.
Else we'll see a hang during suspend.
The patch that was posted was just for workaround patch.
I shall do some more testing with just irq_enable call
and then shall post the fix which can be considered
for inclusion.
Cheers
Deepthi
^ permalink raw reply
* Re: Suspend/resume regressions on Lenovo S10-3
From: Rafael J. Wysocki @ 2012-06-08 6:30 UTC (permalink / raw)
To: Deepthi Dharwar
Cc: Dave Hansen, Linux PM mailing list, LKML, Tomas M., Ferenc Wagner,
Arnd Bergmann, Len Brown, Kevin Hilman, Arjan van de Ven,
Jean Pihet, Trinabh Gupta, linux-pm, Srivatsa S. Bhat, preeti
In-Reply-To: <4FCF8936.5030909@linux.vnet.ibm.com>
On Wednesday, June 06, 2012, Deepthi Dharwar wrote:
> On 06/06/2012 07:13 PM, Rafael J. Wysocki wrote:
>
> > On Wednesday, June 06, 2012, Deepthi Dharwar wrote:
> >> On 05/28/2012 07:23 AM, Dave Hansen wrote:
> >>
> >>> I have a Lenovo S10-3 Atom netbook. It's always had some amount of
> >>> trouble working with the intel_idle driver, so I usually compile that
> >>> out an use the acpi one. However, just after 3.1, suspend/resume broke.
> >>> 'echo mem > /sys/power/state' would hang before suspending. I bisected
> >>> it down to the commits around:
> >>>
> >>> e978aa7d7d57d04eb5f88a7507c4fb98577def77 / v3.1-1-ge978aa7
> >>>
> >>> by Deepthi. But, current mainline (v3.4-07644-g07acfc2) hangs with a
> >>> different symptom: it suspends, but hangs on resume from suspend. I
> >>> think _that_ delta in the behavior was caused by:
> >>>
> >>> 3439a8da16bcad6b0982ece938c9f8299bb53584
> >>>
> >>> ACPI / cpuidle: Remove acpi_idle_suspend (to fix suspend
> >>> regression)
> >>>
> >>> It's a bit of a pain to bisect these two different things in parallel.
> >>> I was trying to tell git bisect 'good' on working suspend/resume, 'bad'
> >>> on the hang during resume, and 'skip' on the hangs _during_ suspend. 83
> >>> kernels in, I'm not sure that's working very well. :)
> >>>
> >>> Deepthi, do you have any idea why your patches broke me in the first
> >>> place? Perhaps we should fix that regression first before we go on and
> >>> try to figure out what changed to let it suspend again, but break later.
> >>
> >>
> >> Hi Dave,
> >>
> >> Sorry about my patches breaking your suspend-resume.
> >>
> >> I, basically tried out building and booting 3.1 kernel with
> >> my patch set to reproduce the failure. I could clearly
> >> see suspend not happening. It turns out to be
> >> a bug with my first patch in global registration
> >> series submitted earlier.
> >>
> >> e978aa7d7d57d04eb5f88a7507c4fb98577def77 / v3.1-1-ge978aa7
> >>
> >> The following patch, fixes the suspend issues
> >> seen on my laptop due to earlier cpuidle cleanup
> >> (Lenevo T420 booting with acpi_idle enabled).
> >> Can you please give this a try
> >> on top of my patch set (without Rafael's fix)
> >> and see if it fixes the problem for you.
> >> I am not reverting acpi_idle_suspend flag and
> >> hopefully it should resume fine too.
> >>
> >> ---
> >>
> >> This patch fixes suspend-resume issue seen in the kernel 3.1
> >> series using acpi_idle_driver because of cpuidle global
> >> registration cleanup.
> >> Here, when acpi_idle_suspend flag was set ( during suspend)
> >> the interrupts were not getting enabled in acpi_idle_enter_bm()
> >> routine which was causing the system to hang.
> >>
> >>
> >> Signed-off-by: Deepthi Dharwar <deepthi@linux.vnet.ibm.com>
> >>
> >> ---
> >> drivers/acpi/processor_idle.c | 3 ++-
> >> 1 files changed, 2 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
> >> index 24fe3af..6e35293 100644
> >> --- a/drivers/acpi/processor_idle.c
> >> +++ b/drivers/acpi/processor_idle.c
> >> @@ -895,8 +895,9 @@ static int acpi_idle_enter_bm(struct cpuidle_device *dev,
> >> if (unlikely(!pr))
> >> return -EINVAL;
> >>
> >> -
> >> if (acpi_idle_suspend) {
> >> + local_irq_disable();
> >> + local_irq_enable();
> >> cpu_relax();
> >> return -EINVAL;
> >> }
> >
> > May I say this is ugly? Why can't we track the status of interrupts
> > properly here?
>
>
>
> I agree. Just the irq_enable call should do the trick.
> Once the cpu enters idle via cpu_idle call, irqs are disabled. We
> need to enable them if acpi_idle_suspend is set.
> Else we'll see a hang during suspend.
>
> The patch that was posted was just for workaround patch.
> I shall do some more testing with just irq_enable call
> and then shall post the fix which can be considered
> for inclusion.
I see. OK, then, looking forward to seeing the final patch. :-)
Thanks,
Rafael
^ permalink raw reply
* Re: Suspend/resume regressions on Lenovo S10-3
From: Srivatsa S. Bhat @ 2012-06-08 6:41 UTC (permalink / raw)
To: Deepthi Dharwar
Cc: Rafael J. Wysocki, Dave Hansen, Linux PM mailing list, LKML,
Tomas M., Ferenc Wagner, Arnd Bergmann, Len Brown, Kevin Hilman,
Arjan van de Ven, Jean Pihet, Trinabh Gupta, linux-pm, preeti,
Alan Stern
In-Reply-To: <201206061543.03537.rjw@sisk.pl>
On 06/06/2012 07:13 PM, Rafael J. Wysocki wrote:
> On Wednesday, June 06, 2012, Deepthi Dharwar wrote:
>> On 05/28/2012 07:23 AM, Dave Hansen wrote:
>>
>>> I have a Lenovo S10-3 Atom netbook. It's always had some amount of
>>> trouble working with the intel_idle driver, so I usually compile that
>>> out an use the acpi one. However, just after 3.1, suspend/resume broke.
>>> 'echo mem > /sys/power/state' would hang before suspending. I bisected
>>> it down to the commits around:
>>>
>>> e978aa7d7d57d04eb5f88a7507c4fb98577def77 / v3.1-1-ge978aa7
>>>
>>> by Deepthi. But, current mainline (v3.4-07644-g07acfc2) hangs with a
>>> different symptom: it suspends, but hangs on resume from suspend. I
>>> think _that_ delta in the behavior was caused by:
>>>
>>> 3439a8da16bcad6b0982ece938c9f8299bb53584
>>>
>>> ACPI / cpuidle: Remove acpi_idle_suspend (to fix suspend
>>> regression)
>>>
>>> It's a bit of a pain to bisect these two different things in parallel.
>>> I was trying to tell git bisect 'good' on working suspend/resume, 'bad'
>>> on the hang during resume, and 'skip' on the hangs _during_ suspend. 83
>>> kernels in, I'm not sure that's working very well. :)
>>>
>>> Deepthi, do you have any idea why your patches broke me in the first
>>> place? Perhaps we should fix that regression first before we go on and
>>> try to figure out what changed to let it suspend again, but break later.
>>
>>
>> Hi Dave,
>>
>> Sorry about my patches breaking your suspend-resume.
>>
>> I, basically tried out building and booting 3.1 kernel with
>> my patch set to reproduce the failure. I could clearly
>> see suspend not happening. It turns out to be
>> a bug with my first patch in global registration
>> series submitted earlier.
>>
>> e978aa7d7d57d04eb5f88a7507c4fb98577def77 / v3.1-1-ge978aa7
>>
>> The following patch, fixes the suspend issues
>> seen on my laptop due to earlier cpuidle cleanup
>> (Lenevo T420 booting with acpi_idle enabled).
>> Can you please give this a try
>> on top of my patch set (without Rafael's fix)
>> and see if it fixes the problem for you.
>> I am not reverting acpi_idle_suspend flag and
>> hopefully it should resume fine too.
>>
>> ---
>>
>> This patch fixes suspend-resume issue seen in the kernel 3.1
>> series using acpi_idle_driver because of cpuidle global
>> registration cleanup.
>> Here, when acpi_idle_suspend flag was set ( during suspend)
>> the interrupts were not getting enabled in acpi_idle_enter_bm()
>> routine which was causing the system to hang.
>>
>>
>> Signed-off-by: Deepthi Dharwar <deepthi@linux.vnet.ibm.com>
>>
>> ---
>> drivers/acpi/processor_idle.c | 3 ++-
>> 1 files changed, 2 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
>> index 24fe3af..6e35293 100644
>> --- a/drivers/acpi/processor_idle.c
>> +++ b/drivers/acpi/processor_idle.c
>> @@ -895,8 +895,9 @@ static int acpi_idle_enter_bm(struct cpuidle_device *dev,
>> if (unlikely(!pr))
>> return -EINVAL;
>>
>> -
>> if (acpi_idle_suspend) {
>> + local_irq_disable();
>> + local_irq_enable();
>> cpu_relax();
>> return -EINVAL;
>> }
>
> May I say this is ugly? Why can't we track the status of interrupts
> properly here?
>
Btw, Deepthi, when you are modifying this to keep track of interrupt enabled/
disabled status, I think it would be worthwhile to also add a WARN_ON() in
cpu_idle() inside arch/x86/kernel/process.c, just like how ARM and sh do it.
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 735279e..1ca7e1a 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -459,6 +459,9 @@ void cpu_idle(void)
if (cpuidle_idle_call())
pm_idle();
+ /* The idle routine must return with IRQs enabled. */
+ WARN_ON(irqs_disabled());
+
rcu_idle_exit();
start_critical_timings();
[If we had done this earlier, we could have caught the bug right when the
patch went in :-)]
Regards,
Srivatsa S. Bhat
^ permalink raw reply related
* [RFC 1/4] cpuidle: define the enter function in the driver structure
From: Daniel Lezcano @ 2012-06-08 16:02 UTC (permalink / raw)
To: lenb; +Cc: linux-pm, linux-acpi, linux-kernel, linaro-dev
We have the state index passed as parameter to the 'enter' function.
Most of the drivers assign their 'enter' functions several times in
the cpuidle_state structure, as we have the index, we can delegate
to the driver to handle their own callback array.
That will have the benefit of removing multiple lines of code in the
different drivers.
In order to smoothly modify the driver, the 'enter' function are in
the driver structure and in the cpuidle state structure. That will
let the time to modify the different drivers one by one.
So the 'cpuidle_enter' function checks if the 'enter' callback is
assigned in the driver structure and use it, otherwise it invokes
the 'enter' assigned to the cpuidle_state.
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
drivers/cpuidle/cpuidle.c | 4 +++-
include/linux/cpuidle.h | 1 +
2 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
index d90519c..155dee7 100644
--- a/drivers/cpuidle/cpuidle.c
+++ b/drivers/cpuidle/cpuidle.c
@@ -46,7 +46,9 @@ static inline int cpuidle_enter(struct cpuidle_device *dev,
struct cpuidle_driver *drv, int index)
{
struct cpuidle_state *target_state = &drv->states[index];
- return target_state->enter(dev, drv, index);
+
+ return drv->enter(dev, drv, index) ? drv->enter(dev, drv, index) :
+ target_state->enter(dev, drv, index);
}
static inline int cpuidle_enter_tk(struct cpuidle_device *dev,
diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
index 6c26a3d..d82e169 100644
--- a/include/linux/cpuidle.h
+++ b/include/linux/cpuidle.h
@@ -130,6 +130,7 @@ struct cpuidle_driver {
struct cpuidle_state states[CPUIDLE_STATE_MAX];
int state_count;
int safe_state_index;
+ int (*enter)(struct cpuidle_device *, struct cpuidle_driver *, int);
};
#ifdef CONFIG_CPU_IDLE
--
1.7.5.4
^ permalink raw reply related
* [RFC 2/4] cpuidle: move enter_dead to the driver structure
From: Daniel Lezcano @ 2012-06-08 16:02 UTC (permalink / raw)
To: lenb; +Cc: linux-acpi, linux-pm, linaro-dev, linux-kernel
In-Reply-To: <1339171365-4098-1-git-send-email-daniel.lezcano@linaro.org>
The 'enter_dead' function is only used for processor_idle.c
and the same function is used several times. We fall into the
same abuse with the multiple callbacks for the same function.
This patch fixes that by moving the 'enter_dead' function to the
driver structure. A flag CPUIDLE_FLAG_DEAD_VALID has been added
to handle the callback conditional invokation.
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
drivers/acpi/processor_idle.c | 7 +++----
drivers/cpuidle/cpuidle.c | 4 ++--
include/linux/cpuidle.h | 25 +++++++++++++++++--------
3 files changed, 22 insertions(+), 14 deletions(-)
diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index f3decb3..5590ab4 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -996,6 +996,7 @@ static int acpi_idle_enter_bm(struct cpuidle_device *dev,
struct cpuidle_driver acpi_idle_driver = {
.name = "acpi_idle",
.owner = THIS_MODULE,
+ .enter_dead = acpi_idle_play_dead,
};
/**
@@ -1104,16 +1105,14 @@ static int acpi_processor_setup_cpuidle_states(struct acpi_processor *pr)
case ACPI_STATE_C1:
if (cx->entry_method == ACPI_CSTATE_FFH)
state->flags |= CPUIDLE_FLAG_TIME_VALID;
-
+ state->flags |= CPUIDLE_FLAG_DEAD_VALID;
state->enter = acpi_idle_enter_c1;
- state->enter_dead = acpi_idle_play_dead;
drv->safe_state_index = count;
break;
case ACPI_STATE_C2:
- state->flags |= CPUIDLE_FLAG_TIME_VALID;
+ state->flags |= CPUIDLE_FLAG_TIME_VALID | CPUIDLE_FLAG_DEAD_VALID;
state->enter = acpi_idle_enter_simple;
- state->enter_dead = acpi_idle_play_dead;
drv->safe_state_index = count;
break;
diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
index 155dee7..68bf115 100644
--- a/drivers/cpuidle/cpuidle.c
+++ b/drivers/cpuidle/cpuidle.c
@@ -81,14 +81,14 @@ int cpuidle_play_dead(void)
for (i = CPUIDLE_DRIVER_STATE_START; i < drv->state_count; i++) {
struct cpuidle_state *s = &drv->states[i];
- if (s->power_usage < power_usage && s->enter_dead) {
+ if (s->power_usage < power_usage && (s->flags & CPUIDLE_FLAG_DEAD_VALID)) {
power_usage = s->power_usage;
dead_state = i;
}
}
if (dead_state != -1)
- return drv->states[dead_state].enter_dead(dev, dead_state);
+ return drv->enter_dead(dev, dead_state);
return -ENODEV;
}
diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
index d82e169..f5915e9 100644
--- a/include/linux/cpuidle.h
+++ b/include/linux/cpuidle.h
@@ -49,16 +49,24 @@ struct cpuidle_state {
unsigned int disable;
int (*enter) (struct cpuidle_device *dev,
- struct cpuidle_driver *drv,
- int index);
-
- int (*enter_dead) (struct cpuidle_device *dev, int index);
+ struct cpuidle_driver *drv,
+ int index);
};
-/* Idle State Flags */
-#define CPUIDLE_FLAG_TIME_VALID (0x01) /* is residency time measurable? */
-
-#define CPUIDLE_DRIVER_FLAGS_MASK (0xFFFF0000)
+/********************************************************************************
+ * Idle State Flags: *
+ * CPUIDLE_FLAG_TIME_VALID : the drivers could measure the state residency *
+ * time and store it in the 'target_residency'field. *
+ * This flag will tell the cpuidle core to use this *
+ * value to compute an accurate prediction. *
+ * *
+ * CPUIDLE_FLAG_DEAD_VALID : the driver may want to deal with dead cpus, tell *
+ * the cpuidle core the specified state can use the *
+ * enter_dead function. *
+ * *
+ *******************************************************************************/
+#define CPUIDLE_FLAG_TIME_VALID 0x01
+#define CPUIDLE_FLAG_DEAD_VALID 0x02
/**
* cpuidle_get_statedata - retrieves private driver state data
@@ -131,6 +139,7 @@ struct cpuidle_driver {
int state_count;
int safe_state_index;
int (*enter)(struct cpuidle_device *, struct cpuidle_driver *, int);
+ int (*enter_dead)(struct cpuidle_device *, int);
};
#ifdef CONFIG_CPU_IDLE
--
1.7.5.4
^ permalink raw reply related
* [RFC 3/4] cpuidle : move tlb flag to the cpuidle header
From: Daniel Lezcano @ 2012-06-08 16:02 UTC (permalink / raw)
To: lenb; +Cc: linux-pm, linux-acpi, linux-kernel, linaro-dev
In-Reply-To: <1339171365-4098-1-git-send-email-daniel.lezcano@linaro.org>
Move this specific flag to the header file.
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
drivers/idle/intel_idle.c | 8 --------
include/linux/cpuidle.h | 7 +++++++
2 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index d0f59c3..3456fc9 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -100,14 +100,6 @@ static int intel_idle(struct cpuidle_device *dev,
static struct cpuidle_state *cpuidle_state_table;
/*
- * Set this flag for states where the HW flushes the TLB for us
- * and so we don't need cross-calls to keep it consistent.
- * If this flag is set, SW flushes the TLB, so even if the
- * HW doesn't do the flushing, this flag is safe to use.
- */
-#define CPUIDLE_FLAG_TLB_FLUSHED 0x10000
-
-/*
* States are indexed by the cstate number,
* which is also the index into the MWAIT hint array.
* Thus C0 is a dummy.
diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
index f5915e9..aafacd4 100644
--- a/include/linux/cpuidle.h
+++ b/include/linux/cpuidle.h
@@ -64,9 +64,16 @@ struct cpuidle_state {
* the cpuidle core the specified state can use the *
* enter_dead function. *
* *
+ * CPUIDLE_FLAG_TLB_FLUSHED : Set this flag for states where the HW flushes the *
+ * TLB for us and so we don't need cross-calls to *
+ * keep it consistent. If this flag is set, SW *
+ * flushes the TLB, so even if the HW doesn't do the *
+ * flushing, this flag is safe to use. *
+ * *
*******************************************************************************/
#define CPUIDLE_FLAG_TIME_VALID 0x01
#define CPUIDLE_FLAG_DEAD_VALID 0x02
+#define CPUIDLE_FLAG_TLB_FLUSHED 0x04
/**
* cpuidle_get_statedata - retrieves private driver state data
--
1.7.5.4
^ permalink raw reply related
* [RFC 4/4] cpuidle: replace the 'disable' field by a flag
From: Daniel Lezcano @ 2012-06-08 16:02 UTC (permalink / raw)
To: lenb-DgEjT+Ai2ygdnm+yROfE0A
Cc: linux-acpi-u79uwXL29TY76Z2rM5mHXA,
linux-pm-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
linaro-dev-cunTk1MwBs8s++Sfvej+rw,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1339171365-4098-1-git-send-email-daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
We have a flag field for each cpuidle state but we don't use it
for the 'disabled' states. Let's remove the integer field and use
the flag field.
Signed-off-by: Daniel Lezcano <daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
drivers/cpuidle/cpuidle.c | 1 -
| 5 +++--
drivers/cpuidle/sysfs.c | 4 ++--
include/linux/cpuidle.h | 4 +++-
4 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
index 68bf115..3068b65 100644
--- a/drivers/cpuidle/cpuidle.c
+++ b/drivers/cpuidle/cpuidle.c
@@ -267,7 +267,6 @@ static void poll_idle_init(struct cpuidle_driver *drv)
state->power_usage = -1;
state->flags = 0;
state->enter = poll_idle;
- state->disable = 0;
}
#else
static void poll_idle_init(struct cpuidle_driver *drv) {}
--git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c
index 0633575..c1fe87b 100644
--- a/drivers/cpuidle/governors/menu.c
+++ b/drivers/cpuidle/governors/menu.c
@@ -281,7 +281,8 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev)
* unless the timer is happening really really soon.
*/
if (data->expected_us > 5 &&
- drv->states[CPUIDLE_DRIVER_STATE_START].disable == 0)
+ !(drv->states[CPUIDLE_DRIVER_STATE_START].flags &
+ CPUIDLE_FLAG_DISABLED))
data->last_state_idx = CPUIDLE_DRIVER_STATE_START;
/*
@@ -291,7 +292,7 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev)
for (i = CPUIDLE_DRIVER_STATE_START; i < drv->state_count; i++) {
struct cpuidle_state *s = &drv->states[i];
- if (s->disable)
+ if (s->flags & CPUIDLE_FLAG_DISABLED)
continue;
if (s->target_residency > data->predicted_us)
continue;
diff --git a/drivers/cpuidle/sysfs.c b/drivers/cpuidle/sysfs.c
index 88032b4..a9a5f05 100644
--- a/drivers/cpuidle/sysfs.c
+++ b/drivers/cpuidle/sysfs.c
@@ -245,9 +245,9 @@ static ssize_t store_state_##_name(struct cpuidle_state *state, \
if (err) \
return err; \
if (value) \
- state->disable = 1; \
+ state->flags |= CPUIDLE_FLAG_DISABLED; \
else \
- state->disable = 0; \
+ state->flags &= ~CPUIDLE_FLAG_DISABLED; \
return size; \
}
diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
index aafacd4..65f4c52 100644
--- a/include/linux/cpuidle.h
+++ b/include/linux/cpuidle.h
@@ -46,7 +46,6 @@ struct cpuidle_state {
unsigned int exit_latency; /* in US */
int power_usage; /* in mW */
unsigned int target_residency; /* in US */
- unsigned int disable;
int (*enter) (struct cpuidle_device *dev,
struct cpuidle_driver *drv,
@@ -70,10 +69,13 @@ struct cpuidle_state {
* flushes the TLB, so even if the HW doesn't do the *
* flushing, this flag is safe to use. *
* *
+ * CPUIDLE_FLAG_DISABLE : the state is disabled if this flag is set *
+ * *
*******************************************************************************/
#define CPUIDLE_FLAG_TIME_VALID 0x01
#define CPUIDLE_FLAG_DEAD_VALID 0x02
#define CPUIDLE_FLAG_TLB_FLUSHED 0x04
+#define CPUIDLE_FLAG_DISABLED 0x08
/**
* cpuidle_get_statedata - retrieves private driver state data
--
1.7.5.4
^ permalink raw reply related
* Re: [linux-pm] [RFC 1/4] cpuidle: define the enter function in the driver structure
From: Deepthi Dharwar @ 2012-06-08 17:33 UTC (permalink / raw)
To: Daniel Lezcano; +Cc: lenb, linux-acpi, linux-pm, linaro-dev, linux-kernel
In-Reply-To: <1339171365-4098-1-git-send-email-daniel.lezcano@linaro.org>
Hi Daniel,
On 06/08/2012 09:32 PM, Daniel Lezcano wrote:
> We have the state index passed as parameter to the 'enter' function.
> Most of the drivers assign their 'enter' functions several times in
> the cpuidle_state structure, as we have the index, we can delegate
> to the driver to handle their own callback array.
>
> That will have the benefit of removing multiple lines of code in the
> different drivers.
>
> In order to smoothly modify the driver, the 'enter' function are in
> the driver structure and in the cpuidle state structure. That will
> let the time to modify the different drivers one by one.
> So the 'cpuidle_enter' function checks if the 'enter' callback is
> assigned in the driver structure and use it, otherwise it invokes
> the 'enter' assigned to the cpuidle_state.
Currently, the backend driver initializes
all the cpuidle states supported on the platform,
and each state can have its own enter routine
which can be unique This is a clean approach.
By moving the enter routine into the driver,
we are enforcing in having only one enter state.
There is unnecessary overhead involved
in calling a wrapper routine just to
index into the right idle state routine
for many platforms at runtime.
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
> drivers/cpuidle/cpuidle.c | 4 +++-
> include/linux/cpuidle.h | 1 +
> 2 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index d90519c..155dee7 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -46,7 +46,9 @@ static inline int cpuidle_enter(struct cpuidle_device *dev,
> struct cpuidle_driver *drv, int index)
> {
> struct cpuidle_state *target_state = &drv->states[index];
> - return target_state->enter(dev, drv, index);
> +
> + return drv->enter(dev, drv, index) ? drv->enter(dev, drv, index) :
> + target_state->enter(dev, drv, index);
> }
>
> static inline int cpuidle_enter_tk(struct cpuidle_device *dev,
> diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
> index 6c26a3d..d82e169 100644
> --- a/include/linux/cpuidle.h
> +++ b/include/linux/cpuidle.h
> @@ -130,6 +130,7 @@ struct cpuidle_driver {
> struct cpuidle_state states[CPUIDLE_STATE_MAX];
> int state_count;
> int safe_state_index;
> + int (*enter)(struct cpuidle_device *, struct cpuidle_driver *, int);
> };
>
> #ifdef CONFIG_CPU_IDLE
Cheers,
Deepthi
^ permalink raw reply
* Re: [RFC 1/4] cpuidle: define the enter function in the driver structure
From: Daniel Lezcano @ 2012-06-08 21:34 UTC (permalink / raw)
To: Deepthi Dharwar; +Cc: linux-acpi, linux-pm, linaro-dev, linux-kernel
In-Reply-To: <4FD23752.6090101@linux.vnet.ibm.com>
On 06/08/2012 07:33 PM, Deepthi Dharwar wrote:
> Hi Daniel,
Hi Deepthi,
> On 06/08/2012 09:32 PM, Daniel Lezcano wrote:
>
>> We have the state index passed as parameter to the 'enter' function.
>> Most of the drivers assign their 'enter' functions several times in
>> the cpuidle_state structure, as we have the index, we can delegate
>> to the driver to handle their own callback array.
>>
>> That will have the benefit of removing multiple lines of code in the
>> different drivers.
>>
>> In order to smoothly modify the driver, the 'enter' function are in
>> the driver structure and in the cpuidle state structure. That will
>> let the time to modify the different drivers one by one.
>> So the 'cpuidle_enter' function checks if the 'enter' callback is
>> assigned in the driver structure and use it, otherwise it invokes
>> the 'enter' assigned to the cpuidle_state.
>
>
> Currently, the backend driver initializes
> all the cpuidle states supported on the platform,
> and each state can have its own enter routine
> which can be unique This is a clean approach.
Yes, I perfectly understood the purpose of this field but as clean it is
it does not make sense as it is not used in this way. If it is supposed
to be done in the way you are describing here, we should have the same
number of states and enter functions. Here it is how it is used:
--------------------------------------------------
| Arch | nr states | nr enter function |
--------------------------------------------------
| x86 (nehalem) | 3 | 1 |
--------------------------------------------------
| x86 (snb) | 4 | 1 |
--------------------------------------------------
| x86 (atom) | 4 | 1 |
--------------------------------------------------
| ARM tegra | 1 | 1 |
--------------------------------------------------
| ARM omap3 | 7 | 2 |
--------------------------------------------------
| ARM omap4 | 3 | 1 |
--------------------------------------------------
| ARM ux500 | 2 | 1 |
--------------------------------------------------
| ARM shmobile | 1 | 1 |
--------------------------------------------------
| ARM davinci | 2 | 1 |
--------------------------------------------------
| ARM at91 | 2 | 1 |
--------------------------------------------------
| ARM s3c64xx | 1 | 1 |
--------------------------------------------------
| ARM exynos | 2 | 1 |
--------------------------------------------------
| ARM kirkwood | 2 | 1 |
--------------------------------------------------
| SH | 3 | 1 |
--------------------------------------------------
| PPC | 2 | 2 |
--------------------------------------------------
| | | |
| TOTAL | 39 | 17 |
| | | |
--------------------------------------------------
As you can see most of the enter functions are only used as one.
The Omap3 cpuidle driver enter function for C2 calls the enter function
of C1. Other arch, already use a table of callbacks or the index.
> By moving the enter routine into the driver,
> we are enforcing in having only one enter state.
> There is unnecessary overhead involved
> in calling a wrapper routine just to
> index into the right idle state routine
> for many platforms at runtime.
I don't agree. For the sake of encapsulated code, we duplicate n-times a
field and that is not used in this way. It is quite easy to have in the
driver specific code a common enter function to ventilate to the right
routine without adding extra overhead and let the common code use a
single enter routine (which is already the case today).
^ permalink raw reply
* [RFC PATCH 0/12] generic thermal layer enhancement
From: Zhang Rui @ 2012-06-11 3:19 UTC (permalink / raw)
To: linux-pm, linux-acpi@vger.kernel.org
Hi, all,
this patch set is made to enhance the current generic thermal layer.
It fixes several gaps discussed in
http://marc.info/?l=linux-acpi&m=133836783425764&w=2
and introduces a simple arbitrator to fix the "one cooling devices
referenced by multiple trip points in multiple thermal zones" problem.
The whole idea is
1) make sure we have multiple cooling states for both active cooling and
passive cooling devices. (patch 1,2,3)
2) remove passive specific requirement, aka, tc1/tc2, and
introduce .get_trend() instead, for both active and passive cooling
algorithm. (patch 4,5)
3) introduce new function thermal_zone_trip_update(), this contains the
code for the general cooling algorithm. (patch 6)
4) rename some thermal structures. Use thermal_instance instead of
thermal_cooling_device_instance, as this structure is used to
describe the behavior of a certain cooling device for a certain trip
point in a certain thermal zone.
thermal_zone_device has a list of all the thermal instances in this
thermal zone so that it can update them when the temperature changes.
thermal_cooling_device has a list of all the thermal instances for
this cooling device so that it can make decision which cooling state
should be in when the temperature changes. (patch 7,8,9,10)
5) introduce a simple arbitrator. (patch 11)
When temperature changes, we update a thermal zone in two stages,
a) thermal_zone_trip_point() is the general cooling algorithm to
update the thermal instances in the thermal zone device
b) thermal_zone_do_update() is the arbitrator for the cooling device
to choose the deepest cooling state required to enter.
6) unify the code for both passive and active cooling.
TODO, add locking mechanism. I know this is important but as this patch
set changes the thermal layer a lot, I just want to make sure I'm in the
right direction before going on.
BTW, in theory, they should make no change to the behavior of the
current generic thermal layer users. But I have just done the build
test.
Any comments are welcome.
thanks,
rui
--------------------------------------------------------------------
drivers/acpi/thermal.c | 76 +++++++--
drivers/platform/x86/acerhdf.c | 4 +-
drivers/platform/x86/intel_mid_thermal.c | 2 +-
drivers/thermal/spear_thermal.c | 2 +-
drivers/thermal/thermal_sys.c | 293
++++++++++++++++++------------
include/linux/thermal.h | 22 ++-
7 files changed, 271 insertions(+), 148 deletions(-)
^ permalink raw reply
* [RFC PATCH 1/12] thermal: introduce multiple cooling states support for Active cooling devices
From: Zhang Rui @ 2012-06-11 3:19 UTC (permalink / raw)
To: linux-pm, linux-acpi@vger.kernel.org
Introduce multiple cooling states support for Active cooling devices.
This is because general active cooling devices, like fans,
may have multiple speeds, which can be mapped to different cooling states.
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
drivers/thermal/thermal_sys.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
Index: rtd3/drivers/thermal/thermal_sys.c
===================================================================
--- rtd3.orig/drivers/thermal/thermal_sys.c
+++ rtd3/drivers/thermal/thermal_sys.c
@@ -1018,6 +1018,7 @@ void thermal_zone_device_update(struct t
enum thermal_trip_type trip_type;
struct thermal_cooling_device_instance *instance;
struct thermal_cooling_device *cdev;
+ unsigned long cur_state, max_state;
mutex_lock(&tz->lock);
@@ -1057,10 +1058,17 @@ void thermal_zone_device_update(struct t
cdev = instance->cdev;
+ cdev->ops->get_cur_state(cdev, &cur_state);
+ cdev->ops->get_max_state(cdev, &max_state);
+
if (temp >= trip_temp)
- cdev->ops->set_cur_state(cdev, 1);
+ cur_state = cur_state < max_state ?
+ (cur_state + 1) : max_state;
else
- cdev->ops->set_cur_state(cdev, 0);
+ cur_state = cur_state > 0 ?
+ (cur_state - 1) : 0;
+
+ cdev->ops->set_cur_state(cdev, cur_state);
}
break;
case THERMAL_TRIP_PASSIVE:
^ permalink raw reply
* [ RFC PATCH 2/12] thermal: Introduce cooling states range support for a certain trip point
From: Zhang Rui @ 2012-06-11 3:19 UTC (permalink / raw)
To: linux-pm, linux-acpi@vger.kernel.org
Introduce cooling states range support for a certain trip point.
As the active cooling devices can have multiple cooling states,
we may want only several cooling states for a certain trip point,
and other cooling states for other active trip points.
To do this, we should be able to describe the cooling device
behavior for a certain trip point, rather than for the entire thermal zone.
And when updating thermal zone, we need to check the upper and lower limit
to make sure the cooling device is set to the proper cooling state.
Note that this patch will not bring any different behavior as
upper limit is set to max_state and lower limit is set to 0
in this patch, for now.
Next patch will set these to real values.
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
drivers/thermal/thermal_sys.c | 25 +++++++++++++++++++++----
1 file changed, 21 insertions(+), 4 deletions(-)
Index: rtd3/drivers/thermal/thermal_sys.c
===================================================================
--- rtd3.orig/drivers/thermal/thermal_sys.c
+++ rtd3/drivers/thermal/thermal_sys.c
@@ -41,12 +41,19 @@ MODULE_AUTHOR("Zhang Rui");
MODULE_DESCRIPTION("Generic thermal management sysfs support");
MODULE_LICENSE("GPL");
+/*
+ * This structure is used to describe the behavior of
+ * a certain cooling device on a certain trip point
+ * in a certain thermal zone
+ */
struct thermal_cooling_device_instance {
int id;
char name[THERMAL_NAME_LENGTH];
struct thermal_zone_device *tz;
struct thermal_cooling_device *cdev;
int trip;
+ unsigned long upper; /* Highest cooling state for this trip point */
+ unsigned long lower; /* Lowest cooling state for this trip point */
char attr_name[THERMAL_NAME_LENGTH];
struct device_attribute attr;
struct list_head node;
@@ -759,6 +766,7 @@ int thermal_zone_bind_cooling_device(str
struct thermal_cooling_device_instance *pos;
struct thermal_zone_device *pos1;
struct thermal_cooling_device *pos2;
+ unsigned long max_state;
int result;
if (trip >= tz->trips || (trip < 0 && trip != THERMAL_TRIPS_NONE))
@@ -783,6 +791,11 @@ int thermal_zone_bind_cooling_device(str
dev->tz = tz;
dev->cdev = cdev;
dev->trip = trip;
+
+ cdev->ops->get_max_state(dev, &max_state);
+ dev->upper = max_state;
+ dev->lower = 0;
+
result = get_idr(&tz->idr, &tz->lock, &dev->id);
if (result)
goto free_mem;
@@ -1062,11 +1075,15 @@ void thermal_zone_device_update(struct t
cdev->ops->get_max_state(cdev, &max_state);
if (temp >= trip_temp)
- cur_state = cur_state < max_state ?
- (cur_state + 1) : max_state;
+ cur_state =
+ cur_state < instance->upper ?
+ (cur_state + 1) :
+ instance->upper;
else
- cur_state = cur_state > 0 ?
- (cur_state - 1) : 0;
+ cur_state =
+ cur_state > instance->lower ?
+ (cur_state - 1) :
+ instance->lower;
cdev->ops->set_cur_state(cdev, cur_state);
}
^ permalink raw reply
* [RFC PATCH 3/12] thermal: set upper and lower limits when binding
From: Zhang Rui @ 2012-06-11 3:20 UTC (permalink / raw)
To: linux-pm, linux-acpi@vger.kernel.org
Set upper and lower limits when binding
a thermal cooling device to a thermal zone device.
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
drivers/acpi/thermal.c | 34 ++++++++++++++++++++++++----------
drivers/platform/x86/acerhdf.c | 2 +-
drivers/thermal/thermal_sys.c | 20 ++++++++++++++------
include/linux/thermal.h | 3 ++-
4 files changed, 41 insertions(+), 18 deletions(-)
Index: rtd3/drivers/thermal/thermal_sys.c
===================================================================
--- rtd3.orig/drivers/thermal/thermal_sys.c
+++ rtd3/drivers/thermal/thermal_sys.c
@@ -248,7 +248,7 @@ passive_store(struct device *dev, struct
sizeof("Processor")))
thermal_zone_bind_cooling_device(tz,
THERMAL_TRIPS_NONE,
- cdev);
+ cdev, -1, -1);
}
mutex_unlock(&thermal_list_lock);
if (!tz->passive_delay)
@@ -760,7 +760,8 @@ static void thermal_zone_device_check(st
*/
int thermal_zone_bind_cooling_device(struct thermal_zone_device *tz,
int trip,
- struct thermal_cooling_device *cdev)
+ struct thermal_cooling_device *cdev,
+ long upper, long lower)
{
struct thermal_cooling_device_instance *dev;
struct thermal_cooling_device_instance *pos;
@@ -784,6 +785,15 @@ int thermal_zone_bind_cooling_device(str
if (tz != pos1 || cdev != pos2)
return -EINVAL;
+ cdev->ops->get_max_state(cdev, &max_state);
+
+ /* lower default 0, upper default max_state */
+ lower = lower < 0 ? 0 : lower;
+ upper = upper < 0 ? max_state : upper;
+
+ if (lower > upper || upper > max_state)
+ return -EINVAL;
+
dev =
kzalloc(sizeof(struct thermal_cooling_device_instance), GFP_KERNEL);
if (!dev)
@@ -791,10 +801,8 @@ int thermal_zone_bind_cooling_device(str
dev->tz = tz;
dev->cdev = cdev;
dev->trip = trip;
-
- cdev->ops->get_max_state(dev, &max_state);
- dev->upper = max_state;
- dev->lower = 0;
+ dev->upper = upper;
+ dev->lower = lower;
result = get_idr(&tz->idr, &tz->lock, &dev->id);
if (result)
Index: rtd3/include/linux/thermal.h
===================================================================
--- rtd3.orig/include/linux/thermal.h
+++ rtd3/include/linux/thermal.h
@@ -143,7 +143,8 @@ struct thermal_zone_device *thermal_zone
void thermal_zone_device_unregister(struct thermal_zone_device *);
int thermal_zone_bind_cooling_device(struct thermal_zone_device *, int,
- struct thermal_cooling_device *);
+ struct thermal_cooling_device *,
+ long, long);
int thermal_zone_unbind_cooling_device(struct thermal_zone_device *, int,
struct thermal_cooling_device *);
void thermal_zone_device_update(struct thermal_zone_device *);
Index: rtd3/drivers/acpi/thermal.c
===================================================================
--- rtd3.orig/drivers/acpi/thermal.c
+++ rtd3/drivers/acpi/thermal.c
@@ -729,11 +729,9 @@ static int thermal_notify(struct thermal
return 0;
}
-typedef int (*cb)(struct thermal_zone_device *, int,
- struct thermal_cooling_device *);
static int acpi_thermal_cooling_device_cb(struct thermal_zone_device *thermal,
struct thermal_cooling_device *cdev,
- cb action)
+ int bind)
{
struct acpi_device *device = cdev->devdata;
struct acpi_thermal *tz = thermal->devdata;
@@ -758,7 +756,14 @@ static int acpi_thermal_cooling_device_c
handle = tz->trips.passive.devices.handles[i];
status = acpi_bus_get_device(handle, &dev);
if (ACPI_SUCCESS(status) && (dev == device)) {
- result = action(thermal, trip, cdev);
+ if (bind)
+ result =
+ thermal_zone_bind_cooling_device(
+ thermal, trip, cdev, -1, -1);
+ else
+ result =
+ thermal_zone_unbind_cooling_device(
+ thermal, trip, cdev);
if (result)
goto failed;
}
@@ -775,7 +780,13 @@ static int acpi_thermal_cooling_device_c
handle = tz->trips.active[i].devices.handles[j];
status = acpi_bus_get_device(handle, &dev);
if (ACPI_SUCCESS(status) && (dev == device)) {
- result = action(thermal, trip, cdev);
+ if (bind)
+ result =
+ thermal_zone_bind_cooling_device(
+ thermal, trip, cdev, -1, -1);
+ else
+ result = thermal_zone_unbind_cooling_device(
+ thermal, trip, cdev);
if (result)
goto failed;
}
@@ -786,7 +797,12 @@ static int acpi_thermal_cooling_device_c
handle = tz->devices.handles[i];
status = acpi_bus_get_device(handle, &dev);
if (ACPI_SUCCESS(status) && (dev == device)) {
- result = action(thermal, -1, cdev);
+ if (bind)
+ result = thermal_zone_bind_cooling_device(thermal,
+ -1, cdev, -1, -1);
+ else
+ result = thermal_zone_unbind_cooling_device(thermal,
+ -1, cdev);
if (result)
goto failed;
}
@@ -800,16 +816,14 @@ static int
acpi_thermal_bind_cooling_device(struct thermal_zone_device *thermal,
struct thermal_cooling_device *cdev)
{
- return acpi_thermal_cooling_device_cb(thermal, cdev,
- thermal_zone_bind_cooling_device);
+ return acpi_thermal_cooling_device_cb(thermal, cdev, 1);
}
static int
acpi_thermal_unbind_cooling_device(struct thermal_zone_device *thermal,
struct thermal_cooling_device *cdev)
{
- return acpi_thermal_cooling_device_cb(thermal, cdev,
- thermal_zone_unbind_cooling_device);
+ return acpi_thermal_cooling_device_cb(thermal, cdev, 0);
}
static const struct thermal_zone_device_ops acpi_thermal_zone_ops = {
Index: rtd3/drivers/platform/x86/acerhdf.c
===================================================================
--- rtd3.orig/drivers/platform/x86/acerhdf.c
+++ rtd3/drivers/platform/x86/acerhdf.c
@@ -329,7 +329,7 @@ static int acerhdf_bind(struct thermal_z
if (cdev != cl_dev)
return 0;
- if (thermal_zone_bind_cooling_device(thermal, 0, cdev)) {
+ if (thermal_zone_bind_cooling_device(thermal, 0, cdev, -1, -1)) {
pr_err("error binding cooling dev\n");
return -EINVAL;
}
^ permalink raw reply
* [RFC PATCH 4/12] thermal: introduce .get_trend() callback
From: Zhang Rui @ 2012-06-11 3:20 UTC (permalink / raw)
To: linux-pm, linux-acpi@vger.kernel.org
tc1 and tc2 are ACPI platform specific concepts, introduce
.get_trend() as a more general solution.
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
drivers/acpi/thermal.c | 32 ++++++++++++++++++++++++++++++++
drivers/thermal/thermal_sys.c | 19 +++++++++++++++++--
include/linux/thermal.h | 9 +++++++++
3 files changed, 58 insertions(+), 2 deletions(-)
Index: rtd3/drivers/acpi/thermal.c
===================================================================
--- rtd3.orig/drivers/acpi/thermal.c
+++ rtd3/drivers/acpi/thermal.c
@@ -706,6 +706,37 @@ static int thermal_get_crit_temp(struct
return -EINVAL;
}
+static int thermal_get_trend(struct thermal_zone_device *thermal,
+ int trip, enum thermal_trend *trend)
+{
+ struct acpi_thermal *tz = thermal->devdata;
+ enum thermal_trip_type type;
+ unsigned long trip_temp;
+ int i;
+
+ if (thermal_get_trip_type(thermal, trip, &type))
+ return -EINVAL;
+
+ /* Only PASSIVE trip points need TREND */
+ if (type != THERMAL_TRIP_PASSIVE)
+ return -EINVAL;
+
+ if (thermal_get_trip_temp(thermal, trip, &trip_temp))
+ return -EINVAL;
+
+ /*
+ * tz->temperature has already been updated by generic thermal layer,
+ * before this callback being invoked
+ */
+ i = (tz->trips.passive.tc1 * (tz->temperature - tz->last_temperature))
+ + (tz->trips.passive.tc2 * (tz->temperature - trip_temp));
+
+ *trend = i > 0 ? THERMAL_TREND_RAISING :
+ (i < 0 ? THERMAL_TREND_DROPPING : THERMAL_TREND_NONE);
+ return 0;
+}
+
+
static int thermal_notify(struct thermal_zone_device *thermal, int trip,
enum thermal_trip_type trip_type)
{
@@ -835,6 +866,7 @@ static const struct thermal_zone_device_
.get_trip_type = thermal_get_trip_type,
.get_trip_temp = thermal_get_trip_temp,
.get_crit_temp = thermal_get_crit_temp,
+ .get_trend = thermal_get_trend,
.notify = thermal_notify,
};
Index: rtd3/drivers/thermal/thermal_sys.c
===================================================================
--- rtd3.orig/drivers/thermal/thermal_sys.c
+++ rtd3/drivers/thermal/thermal_sys.c
@@ -657,6 +657,18 @@ thermal_remove_hwmon_sysfs(struct therma
}
#endif
+static void thermal_get_trend (struct thermal_zone_device *tz,
+ int trip, enum thermal_trend *trend)
+{
+ if (tz->ops->get_trend)
+ if (!tz->ops->get_trend(tz, trip, trend))
+ return;
+
+ *trend = tz->temperature >= tz->last_temperature ?
+ THERMAL_TREND_RAISING : THERMAL_TREND_DROPPING;
+ return;
+}
+
static void thermal_zone_device_set_polling(struct thermal_zone_device *tz,
int delay)
{
@@ -691,6 +703,8 @@ static void thermal_zone_device_passive(
if (temp >= trip_temp) {
tz->passive = true;
+ thermal_get_trend(tz, trip, (enum thermal_trend *)&trend);
+
trend = (tz->tc1 * (temp - tz->last_temperature)) +
(tz->tc2 * (temp - trip_temp));
@@ -1049,6 +1063,9 @@ void thermal_zone_device_update(struct t
goto leave;
}
+ tz->last_temperature = tz->temperature;
+ tz->temperature = temp;
+
for (count = 0; count < tz->trips; count++) {
tz->ops->get_trip_type(tz, count, &trip_type);
tz->ops->get_trip_temp(tz, count, &trip_temp);
@@ -1108,8 +1125,6 @@ void thermal_zone_device_update(struct t
thermal_zone_device_passive(tz, temp, tz->forced_passive,
THERMAL_TRIPS_NONE);
- tz->last_temperature = temp;
-
leave:
if (tz->passive)
thermal_zone_device_set_polling(tz, tz->passive_delay);
Index: rtd3/include/linux/thermal.h
===================================================================
--- rtd3.orig/include/linux/thermal.h
+++ rtd3/include/linux/thermal.h
@@ -44,6 +44,12 @@ enum thermal_trip_type {
THERMAL_TRIP_CRITICAL,
};
+enum thermal_trend {
+ THERMAL_TREND_NONE,
+ THERMAL_TREND_RAISING,
+ THERMAL_TREND_DROPPING,
+};
+
struct thermal_zone_device_ops {
int (*bind) (struct thermal_zone_device *,
struct thermal_cooling_device *);
@@ -59,6 +65,8 @@ struct thermal_zone_device_ops {
int (*get_trip_temp) (struct thermal_zone_device *, int,
unsigned long *);
int (*get_crit_temp) (struct thermal_zone_device *, unsigned long *);
+ int (*get_trend) (struct thermal_zone_device *, int,
+ enum thermal_trend *);
int (*notify) (struct thermal_zone_device *, int,
enum thermal_trip_type);
};
@@ -95,6 +103,7 @@ struct thermal_zone_device {
int tc2;
int passive_delay;
int polling_delay;
+ int temperature;
int last_temperature;
bool passive;
unsigned int forced_passive;
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox