From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2012-01-11 01:05:48
We just replaced the pseries platform idle loops with a cpuidle backend,
however that means that you won't get any power saving and won't return
any unused idle time to the hypervisor unless cpuidle is enabled.
Thus is should default to y when pseries is enabled. I prefer that to
a select so we can still make it modular if we want to.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
Linus, do you want to just pick that up or should I put it into powerpc.git
and ask you to pull ? I will have 2 or 3 other fixes there later today,
but I wanted to make sure you were ok with the approach with this
specific one.
On Tue, Jan 10, 2012 at 5:05 PM, Benjamin Herrenschmidt
[off-list ref] wrote:
Linus, do you want to just pick that up or should I put it into powerpc.git
and ask you to pull ? I will have 2 or 3 other fixes there later today,
but I wanted to make sure you were ok with the approach with this
specific one.
It doesn't seem to be all that different from the "default y if ACPI"
case, so I guess it works ok.
That said, I wonder if the right approach wouldn't be
default y if SUPPORT_CPU_IDLE
or something along those lines. And then both ACPI and PPC_PSERIES
could just select that instead. Because I do hate having random
board-level knowledge in something like this. I dunno.
Linus
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2012-01-11 07:05:56
On Tue, 2012-01-10 at 22:08 -0800, Linus Torvalds wrote:
On Tue, Jan 10, 2012 at 5:05 PM, Benjamin Herrenschmidt
[off-list ref] wrote:
quoted
Linus, do you want to just pick that up or should I put it into powerpc.git
and ask you to pull ? I will have 2 or 3 other fixes there later today,
but I wanted to make sure you were ok with the approach with this
specific one.
It doesn't seem to be all that different from the "default y if ACPI"
case, so I guess it works ok.
It works for my case, that's tested, but ...
That said, I wonder if the right approach wouldn't be
default y if SUPPORT_CPU_IDLE
or something along those lines. And then both ACPI and PPC_PSERIES
could just select that instead. Because I do hate having random
board-level knowledge in something like this. I dunno.
I tend to agree, I wasn't too keen on touching ACPI related stuff I
suppose it shouldn't be hard :-) Btw, what about the change:
- default ACPI
+ default y if ACPI
(To be honest I'm not sure what the first form does in details).
Oh, also, I just see that in drivers/acpi/Kconfig:
config ACPI_PROCESSOR
tristate "Processor"
select THERMAL
select CPU_IDLE
default y
Hrm... maybe I should just do the same in pseries and remove both the
"default" statements above, what do you think ?
On pSeries I'm keen to build that in rather than make it a module too
because you get no idle handling until it loads which can be
problematic. Built-in, it seems to be quite early in the link order (if
we can still trust that nowadays ...).
Cheers,
Ben.
From: Thadeu Lima de Souza Cascardo <hidden> Date: 2012-01-11 22:37:55
On Tue, Jan 10, 2012 at 03:05:35PM -0000, Benjamin Herrenschmidt wrote:
We just replaced the pseries platform idle loops with a cpuidle backend,
however that means that you won't get any power saving and won't return
any unused idle time to the hypervisor unless cpuidle is enabled.
Thus is should default to y when pseries is enabled. I prefer that to
a select so we can still make it modular if we want to.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
Linus, do you want to just pick that up or should I put it into powerpc.git
and ask you to pull ? I will have 2 or 3 other fixes there later today,
but I wanted to make sure you were ok with the approach with this
specific one.
Hi, Ben.
Note that building with CONFIG_PSERIES_IDLE=m fails.
CC [M] arch/powerpc/platforms/pseries/processor_idle.o
arch/powerpc/platforms/pseries/processor_idle.c:35: error: redefinition
of ‘update_smt_snooze_delay’
/root/linux/arch/powerpc/include/asm/system.h:230: note: previous
definition of ‘update_smt_snooze_delay’ was here
arch/powerpc/platforms/pseries/processor_idle.c:175: error: redefinition
of ‘pseries_notify_cpuidle_add_cpu’
/root/linux/arch/powerpc/include/asm/system.h:231: note: previous
definition of ‘pseries_notify_cpuidle_add_cpu’ was here
make[2]: *** [arch/powerpc/platforms/pseries/processor_idle.o] Error 1
make[1]: *** [arch/powerpc/platforms/pseries] Error 2
make: *** [arch/powerpc/platforms] Error 2
asm/system.h has empty inline implementations for
update_smt_snooze_delay and pseries_notify_cpuidle_add_cpu, which are
used when CONFIG_PSERIES_IDLE is undefined. Since those two functions
are used in core power architecture functions (store_smt_snooze_delay
at kernel/sysfs.c and smp_xics_setup_cpu at platforms/pseries/smp.c),
this requires some rework in these interactions or we should simply
disable PSERIES_IDLE to be built as a module for now.
Regards.
Cascardo.
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2012-01-11 23:06:41
On Wed, 2012-01-11 at 20:37 -0200, Thadeu Lima de Souza Cascardo wrote:
On Tue, Jan 10, 2012 at 03:05:35PM -0000, Benjamin Herrenschmidt wrote:
quoted
We just replaced the pseries platform idle loops with a cpuidle backend,
however that means that you won't get any power saving and won't return
any unused idle time to the hypervisor unless cpuidle is enabled.
Thus is should default to y when pseries is enabled. I prefer that to
a select so we can still make it modular if we want to.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
Linus, do you want to just pick that up or should I put it into powerpc.git
and ask you to pull ? I will have 2 or 3 other fixes there later today,
but I wanted to make sure you were ok with the approach with this
specific one.
Hi, Ben.
Note that building with CONFIG_PSERIES_IDLE=m fails.
Ah ok. Well, making it built-in only makes sense anyway as I said
separately, so I think I'll just select it.
Cheers,
Ben.
CC [M] arch/powerpc/platforms/pseries/processor_idle.o
arch/powerpc/platforms/pseries/processor_idle.c:35: error: redefinition
of ‘update_smt_snooze_delay’
/root/linux/arch/powerpc/include/asm/system.h:230: note: previous
definition of ‘update_smt_snooze_delay’ was here
arch/powerpc/platforms/pseries/processor_idle.c:175: error: redefinition
of ‘pseries_notify_cpuidle_add_cpu’
/root/linux/arch/powerpc/include/asm/system.h:231: note: previous
definition of ‘pseries_notify_cpuidle_add_cpu’ was here
make[2]: *** [arch/powerpc/platforms/pseries/processor_idle.o] Error 1
make[1]: *** [arch/powerpc/platforms/pseries] Error 2
make: *** [arch/powerpc/platforms] Error 2
asm/system.h has empty inline implementations for
update_smt_snooze_delay and pseries_notify_cpuidle_add_cpu, which are
used when CONFIG_PSERIES_IDLE is undefined. Since those two functions
are used in core power architecture functions (store_smt_snooze_delay
at kernel/sysfs.c and smp_xics_setup_cpu at platforms/pseries/smp.c),
this requires some rework in these interactions or we should simply
disable PSERIES_IDLE to be built as a module for now.
Regards.
Cascardo.
On 01/12/2012 04:36 AM, Benjamin Herrenschmidt wrote:
On Wed, 2012-01-11 at 20:37 -0200, Thadeu Lima de Souza Cascardo wrote:
quoted
On Tue, Jan 10, 2012 at 03:05:35PM -0000, Benjamin Herrenschmidt wrote:
quoted
We just replaced the pseries platform idle loops with a cpuidle backend,
however that means that you won't get any power saving and won't return
any unused idle time to the hypervisor unless cpuidle is enabled.
Thus is should default to y when pseries is enabled. I prefer that to
a select so we can still make it modular if we want to.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
Linus, do you want to just pick that up or should I put it into powerpc.git
and ask you to pull ? I will have 2 or 3 other fixes there later today,
but I wanted to make sure you were ok with the approach with this
specific one.
Hi, Ben.
Note that building with CONFIG_PSERIES_IDLE=m fails.
Ah ok. Well, making it built-in only makes sense anyway as I said
separately, so I think I'll just select it.
Cheers,
Ben.
quoted
CC [M] arch/powerpc/platforms/pseries/processor_idle.o
arch/powerpc/platforms/pseries/processor_idle.c:35: error: redefinition
of ‘update_smt_snooze_delay’
/root/linux/arch/powerpc/include/asm/system.h:230: note: previous
definition of ‘update_smt_snooze_delay’ was here
arch/powerpc/platforms/pseries/processor_idle.c:175: error: redefinition
of ‘pseries_notify_cpuidle_add_cpu’
/root/linux/arch/powerpc/include/asm/system.h:231: note: previous
definition of ‘pseries_notify_cpuidle_add_cpu’ was here
make[2]: *** [arch/powerpc/platforms/pseries/processor_idle.o] Error 1
make[1]: *** [arch/powerpc/platforms/pseries] Error 2
make: *** [arch/powerpc/platforms] Error 2
asm/system.h has empty inline implementations for
update_smt_snooze_delay and pseries_notify_cpuidle_add_cpu, which are
used when CONFIG_PSERIES_IDLE is undefined. Since those two functions
are used in core power architecture functions (store_smt_snooze_delay
at kernel/sysfs.c and smp_xics_setup_cpu at platforms/pseries/smp.c),
this requires some rework in these interactions or we should simply
disable PSERIES_IDLE to be built as a module for now.
Regards.
Cascardo.
The following patch disables pseries cpuidle driver to be loaded as a
module as there are build problems reported when one is trying to do so.
This is a work around for now until the problem is fixed.
arch/powerpc/platforms/pseries/Kconfig | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
@@ -122,7 +122,7 @@ config DTLSayNifyouareunsure.configPSERIES_IDLE-tristate"Cpuidle driver for pSeries platforms"+bool"Cpuidle driver for pSeries platforms"depends onCPU_IDLEdepends onPPC_PSERIESdefaulty
As pointed out, asm/system.h has empty inline implementations for
update_smt_snooze_delay and pseries_notify_cpuidle_add_cpu, which are
used when CONFIG_PSERIES_IDLE is undefined. Since those two functions
are used in core power architecture functions (store_smt_snooze_delay
at kernel/sysfs.c and smp_xics_setup_cpu at platforms/pseries/smp.c),
Going forward, the aim is to remove the dependency of using these
functions from core power architecture functions. So I am proposing the
following changes:
a) Removing update_smt_snooze_delay () and allow the pseries
idle backend driver to query snooze entry on need basis
while initializing the driver fields by exporting an api to return
snooze value for a given cpu.
b) Replacing the pseries_notify_cpuidle_add_cpu() call
by registering a cpu_notifier to take actions accordingly.
Regards,
Deepthi