* [PATCH] cpuidle: Default y for pseries @ 2012-01-11 1:05 Benjamin Herrenschmidt 2012-01-11 6:08 ` Linus Torvalds 2012-01-11 22:37 ` Thadeu Lima de Souza Cascardo 0 siblings, 2 replies; 6+ messages in thread From: Benjamin Herrenschmidt @ 2012-01-11 1:05 UTC (permalink / raw) To: Linus Torvalds; +Cc: linuxppc-dev, Shaohua Li, linux-pm, Venkatesh Pallipadi 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. diff --git a/drivers/cpuidle/Kconfig b/drivers/cpuidle/Kconfig index 7dbc4a8..62ca70d 100644 --- a/drivers/cpuidle/Kconfig +++ b/drivers/cpuidle/Kconfig @@ -1,7 +1,8 @@ config CPU_IDLE bool "CPU idle PM support" - default ACPI + default y if ACPI + default y if PPC_PSERIES help CPU idle is a generic framework for supporting software-controlled idle processor power management. It includes modular cross-platform ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] cpuidle: Default y for pseries 2012-01-11 1:05 [PATCH] cpuidle: Default y for pseries Benjamin Herrenschmidt @ 2012-01-11 6:08 ` Linus Torvalds 2012-01-11 7:05 ` Benjamin Herrenschmidt 2012-01-11 22:37 ` Thadeu Lima de Souza Cascardo 1 sibling, 1 reply; 6+ messages in thread From: Linus Torvalds @ 2012-01-11 6:08 UTC (permalink / raw) To: Benjamin Herrenschmidt Cc: linuxppc-dev, Shaohua Li, linux-pm, Venkatesh Pallipadi On Tue, Jan 10, 2012 at 5:05 PM, Benjamin Herrenschmidt <benh@kernel.crashing.org> 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] cpuidle: Default y for pseries 2012-01-11 6:08 ` Linus Torvalds @ 2012-01-11 7:05 ` Benjamin Herrenschmidt 0 siblings, 0 replies; 6+ messages in thread From: Benjamin Herrenschmidt @ 2012-01-11 7:05 UTC (permalink / raw) To: Linus Torvalds; +Cc: linuxppc-dev, Shaohua Li, linux-pm, Venkatesh Pallipadi On Tue, 2012-01-10 at 22:08 -0800, Linus Torvalds wrote: > On Tue, Jan 10, 2012 at 5:05 PM, Benjamin Herrenschmidt > <benh@kernel.crashing.org> 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. 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. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: cpuidle: Default y for pseries 2012-01-11 1:05 [PATCH] cpuidle: Default y for pseries Benjamin Herrenschmidt 2012-01-11 6:08 ` Linus Torvalds @ 2012-01-11 22:37 ` Thadeu Lima de Souza Cascardo 2012-01-11 23:06 ` Benjamin Herrenschmidt 1 sibling, 1 reply; 6+ messages in thread From: Thadeu Lima de Souza Cascardo @ 2012-01-11 22:37 UTC (permalink / raw) To: Benjamin Herrenschmidt Cc: Venkatesh Pallipadi, linuxppc-dev, linux-pm, Linus Torvalds, Shaohua Li 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. > > diff --git a/drivers/cpuidle/Kconfig b/drivers/cpuidle/Kconfig > index 7dbc4a8..62ca70d 100644 > --- a/drivers/cpuidle/Kconfig > +++ b/drivers/cpuidle/Kconfig > @@ -1,7 +1,8 @@ > > config CPU_IDLE > bool "CPU idle PM support" > - default ACPI > + default y if ACPI > + default y if PPC_PSERIES > help > CPU idle is a generic framework for supporting software-controlled > idle processor power management. It includes modular cross-platform ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: cpuidle: Default y for pseries 2012-01-11 22:37 ` Thadeu Lima de Souza Cascardo @ 2012-01-11 23:06 ` Benjamin Herrenschmidt 2012-01-12 13:05 ` Deepthi Dharwar 0 siblings, 1 reply; 6+ messages in thread From: Benjamin Herrenschmidt @ 2012-01-11 23:06 UTC (permalink / raw) To: Thadeu Lima de Souza Cascardo Cc: Venkatesh Pallipadi, linuxppc-dev, linux-pm, Linus Torvalds, Shaohua Li 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: > > 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. > > > > > diff --git a/drivers/cpuidle/Kconfig b/drivers/cpuidle/Kconfig > > index 7dbc4a8..62ca70d 100644 > > --- a/drivers/cpuidle/Kconfig > > +++ b/drivers/cpuidle/Kconfig > > @@ -1,7 +1,8 @@ > > > > config CPU_IDLE > > bool "CPU idle PM support" > > - default ACPI > > + default y if ACPI > > + default y if PPC_PSERIES > > help > > CPU idle is a generic framework for supporting software-controlled > > idle processor power management. It includes modular cross-platform ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: cpuidle: Default y for pseries 2012-01-11 23:06 ` Benjamin Herrenschmidt @ 2012-01-12 13:05 ` Deepthi Dharwar 0 siblings, 0 replies; 6+ messages in thread From: Deepthi Dharwar @ 2012-01-12 13:05 UTC (permalink / raw) To: Benjamin Herrenschmidt Cc: linuxppc-dev, linux-pm, Shaohua Li, Thadeu Lima de Souza Cascardo, Venkatesh Pallipadi 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: >> 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. > > 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. >> >>> >>> diff --git a/drivers/cpuidle/Kconfig b/drivers/cpuidle/Kconfig >>> index 7dbc4a8..62ca70d 100644 >>> --- a/drivers/cpuidle/Kconfig >>> +++ b/drivers/cpuidle/Kconfig >>> @@ -1,7 +1,8 @@ >>> >>> config CPU_IDLE >>> bool "CPU idle PM support" >>> - default ACPI >>> + default y if ACPI >>> + default y if PPC_PSERIES >>> help >>> CPU idle is a generic framework for supporting software-controlled >>> idle processor power management. It includes modular cross-platform 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(-) diff --git a/arch/powerpc/platforms/pseries/Kconfig b/arch/powerpc/platforms/pseries/Kconfig index ae7b6d4..31f22c1 100644 --- a/arch/powerpc/platforms/pseries/Kconfig +++ b/arch/powerpc/platforms/pseries/Kconfig @@ -122,7 +122,7 @@ config DTL Say N if you are unsure. config PSERIES_IDLE - tristate "Cpuidle driver for pSeries platforms" + bool "Cpuidle driver for pSeries platforms" depends on CPU_IDLE depends on PPC_PSERIES default y 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-01-12 13:06 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-01-11 1:05 [PATCH] cpuidle: Default y for pseries Benjamin Herrenschmidt 2012-01-11 6:08 ` Linus Torvalds 2012-01-11 7:05 ` Benjamin Herrenschmidt 2012-01-11 22:37 ` Thadeu Lima de Souza Cascardo 2012-01-11 23:06 ` Benjamin Herrenschmidt 2012-01-12 13:05 ` Deepthi Dharwar
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).