Deepthi Dharwar | 19 Aug 06:27 2013
Picon

[RFC PATCH V3 0/5] powerpc/cpuidle: Generic POWERPC cpuidle driver enabled for POWER and POWERNV platforms

This patch series consolidates the backend cpuidle driver for pseries
and powernv platforms and also enables the new drivers/cpuidle/cpuidle-powerpc.c 
to include other powerpc drivers with minimal code duplication.

Current existing backend driver for pseries has been moved to drivers/cpuidle 
and has been extended to accommodate powernv idle power mgmt states. 
As seen in V1 of this patch series, having a separate powernv backend driver 
results in too much code duplication, which is less elegant and can pose 
maintenance problems going further.

Using the cpuidle framework to exploit platform low power idle states
management can take advantage of advanced heuristics, tunables and features 
provided by framework. The statistics and tracing infrastructure provided 
by the cpuidle framework also helps in enabling power management 
related tools and help tune the system and applications.

Earlier in 3.3 kernel, pseries idle state management was modified to 
exploit the cpuidle framework and the end goal of this patch is to have powernv
platform also to hook its idle states into cpuidle framework with minimal 
code duplication between both platforms. This result is a generic powerpc 
backend driver currently enabled for pseries and powernv platforms and which
can be extended to accommodate other powerpc archs in the future.

This series aims to maintain compatibility and functionality to existing pseries 
and powernv idle cpu management code. There are no new functions or idle 
states added as part of this series. This can be extended by adding more 
states to this existing framework.

With this patch series, the powernv cpuidle functionalities are on-par with 
pSeries idle management. Other POWERPC platforms can use this patch series 
(Continue reading)

Deepthi Dharwar | 19 Aug 06:28 2013
Picon

[RFC PATCH V3 1/5] pseries/cpuidle: Remove dependency of pseries.h file

As a part of pseries_idle cleanup to make the backend driver
code common to both pseries and powernv.
Remove non-essential smt_snooze_delay declaration in pseries.h
header file and pseries.h file inclusion in
pseries/processor_idle.c

Signed-off-by: Deepthi Dharwar <deepthi <at> linux.vnet.ibm.com>
---
 arch/powerpc/platforms/pseries/processor_idle.c |    1 -
 arch/powerpc/platforms/pseries/pseries.h        |    3 ---
 2 files changed, 4 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/processor_idle.c b/arch/powerpc/platforms/pseries/processor_idle.c
index 4644efa0..ca70279 100644
--- a/arch/powerpc/platforms/pseries/processor_idle.c
+++ b/arch/powerpc/platforms/pseries/processor_idle.c
 <at>  <at>  -20,7 +20,6  <at>  <at> 
 #include <asm/runlatch.h>

 #include "plpar_wrappers.h"
-#include "pseries.h"

 struct cpuidle_driver pseries_idle_driver = {
 	.name             = "pseries_idle",
diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h
index c2a3a25..d1b07e6 100644
--- a/arch/powerpc/platforms/pseries/pseries.h
+++ b/arch/powerpc/platforms/pseries/pseries.h
 <at>  <at>  -60,9 +60,6  <at>  <at>  extern struct device_node *dlpar_configure_connector(u32);
 extern int dlpar_attach_node(struct device_node *);
(Continue reading)

Deepthi Dharwar | 19 Aug 06:28 2013
Picon

[RFC PATCH V3 2/5] pseries: Move plpar_wrapper.h to powerpc common include/asm location.

As a part of pseries_idle backend driver cleanup to make
the code common to both pseries and powernv platforms, it
is necessary to move the backend-driver code to drivers/cpuidle.

As a pre-requisite for that, it is essential to move plpar_wrapper.h
to include/asm.

Signed-off-by: Deepthi Dharwar <deepthi <at> linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/plpar_wrappers.h       |  325 +++++++++++++++++++++++
 arch/powerpc/platforms/pseries/cmm.c            |    3 
 arch/powerpc/platforms/pseries/dtl.c            |    3 
 arch/powerpc/platforms/pseries/hotplug-cpu.c    |    3 
 arch/powerpc/platforms/pseries/hvconsole.c      |    2 
 arch/powerpc/platforms/pseries/iommu.c          |    3 
 arch/powerpc/platforms/pseries/kexec.c          |    2 
 arch/powerpc/platforms/pseries/lpar.c           |    2 
 arch/powerpc/platforms/pseries/plpar_wrappers.h |  324 -----------------------
 arch/powerpc/platforms/pseries/processor_idle.c |    3 
 arch/powerpc/platforms/pseries/setup.c          |    2 
 arch/powerpc/platforms/pseries/smp.c            |    2 
 12 files changed, 336 insertions(+), 338 deletions(-)
 create mode 100644 arch/powerpc/include/asm/plpar_wrappers.h
 delete mode 100644 arch/powerpc/platforms/pseries/plpar_wrappers.h

diff --git a/arch/powerpc/include/asm/plpar_wrappers.h b/arch/powerpc/include/asm/plpar_wrappers.h
new file mode 100644
index 0000000..e2f84d6
--- /dev/null
+++ b/arch/powerpc/include/asm/plpar_wrappers.h
(Continue reading)

Deepthi Dharwar | 19 Aug 06:28 2013
Picon

[RFC PATCH V3 3/5] powerpc/cpuidle: Generic powerpc backend cpuidle driver.

This patch involves moving the current pseries_idle backend driver code
from pseries/processor_idle.c to drivers/cpuidle/cpuidle-powerpc.c,
and making the backend code generic enough to be able to  extend this
driver code for complete powerpc platform to exploit the cpuidle framework.

It enables the support for pseries platform, such that going forward the same code
with minimal efforts can be re-used for a common driver on powernv
and can be further extended to support cpuidle idle state mgmt for the rest
of the powerpc platforms. This removes a lot of code duplicacy, making
the code elegant.

Signed-off-by: Deepthi Dharwar <deepthi <at> linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/paca.h                 |   23 +
 arch/powerpc/include/asm/processor.h            |    2 
 arch/powerpc/platforms/pseries/Kconfig          |    9 -
 arch/powerpc/platforms/pseries/Makefile         |    1 
 arch/powerpc/platforms/pseries/processor_idle.c |  360 -----------------------
 drivers/cpuidle/Kconfig                         |    7 
 drivers/cpuidle/Makefile                        |    2 
 drivers/cpuidle/cpuidle-powerpc.c               |  361 +++++++++++++++++++++++
 8 files changed, 394 insertions(+), 371 deletions(-)
 delete mode 100644 arch/powerpc/platforms/pseries/processor_idle.c
 create mode 100644 drivers/cpuidle/cpuidle-powerpc.c

diff --git a/arch/powerpc/include/asm/paca.h b/arch/powerpc/include/asm/paca.h
index 77c91e7..7bd83ff 100644
--- a/arch/powerpc/include/asm/paca.h
+++ b/arch/powerpc/include/asm/paca.h
 <at>  <at>  -175,6 +175,29  <at>  <at>  extern void setup_paca(struct paca_struct *new_paca);
(Continue reading)

Wang Dongsheng-B40534 | 19 Aug 07:52 2013

RE: [RFC PATCH V3 3/5] powerpc/cpuidle: Generic powerpc backend cpuidle driver.

I think we should move the states and handle function to arch/power/platform*
The states and handle function is belong to backend driver, not for this, different platform have
different state.
Different platforms to make their own deal with these states.

I think we cannot put all the status of different platforms and handler in this driver.

> diff --git a/drivers/cpuidle/Kconfig b/drivers/cpuidle/Kconfig
> index 0e2cd5c..99ee5d4 100644
> --- a/drivers/cpuidle/Kconfig
> +++ b/drivers/cpuidle/Kconfig
>  <at>  <at>  -42,6 +42,13  <at>  <at>  config CPU_IDLE_ZYNQ
>  	help
>  	  Select this to enable cpuidle on Xilinx Zynq processors.
> 
> +config CPU_IDLE_POWERPC
> +	bool "CPU Idle driver for POWERPC platforms"
> +	depends on PPC64

Why not PPC?

> +	default y
> +        help
> +          Select this option to enable processor idle state management
> +	  for POWERPC platform.
>  endif
> 
>  config ARCH_NEEDS_CPU_IDLE_COUPLED
> diff --git a/drivers/cpuidle/Makefile b/drivers/cpuidle/Makefile
> index 8767a7b..d12e205 100644
(Continue reading)

Deepthi Dharwar | 19 Aug 12:18 2013
Picon

Re: [RFC PATCH V3 3/5] powerpc/cpuidle: Generic powerpc backend cpuidle driver.


Hi Dongsheng,

On 08/19/2013 11:22 AM, Wang Dongsheng-B40534 wrote:
> I think we should move the states and handle function to arch/power/platform*
> The states and handle function is belong to backend driver, not for this, different platform have
different state.
> Different platforms to make their own deal with these states.
> 
> I think we cannot put all the status of different platforms and handler in this driver.

The idea here is a single powerpc back-end driver, which does a runtime
detection of the platform it is running and choose the right
idle states table. This was one of outcome of V2 discussion.

I feel there is no harm in keeping the state information in the same
file. We do have x86, which has all its variants information in one
file. One place will have all the idle consolidated information of
all the platform variants. If community does feel, we need to
have just the states information in arch specific file, we can do so.

>> diff --git a/drivers/cpuidle/Kconfig b/drivers/cpuidle/Kconfig
>> index 0e2cd5c..99ee5d4 100644
>> --- a/drivers/cpuidle/Kconfig
>> +++ b/drivers/cpuidle/Kconfig
>>  <at>  <at>  -42,6 +42,13  <at>  <at>  config CPU_IDLE_ZYNQ
>>  	help
>>  	  Select this to enable cpuidle on Xilinx Zynq processors.
>>
>> +config CPU_IDLE_POWERPC
(Continue reading)

Scott Wood | 19 Aug 20:17 2013

Re: [RFC PATCH V3 3/5] powerpc/cpuidle: Generic powerpc backend cpuidle driver.

On Mon, 2013-08-19 at 15:48 +0530, Deepthi Dharwar wrote:
> Hi Dongsheng,
> 
> On 08/19/2013 11:22 AM, Wang Dongsheng-B40534 wrote:
> > I think we should move the states and handle function to arch/power/platform*
> > The states and handle function is belong to backend driver, not for this, different platform have
different state.
> > Different platforms to make their own deal with these states.
> > 
> > I think we cannot put all the status of different platforms and handler in this driver.
> 
> The idea here is a single powerpc back-end driver, which does a runtime
> detection of the platform it is running and choose the right
> idle states table. This was one of outcome of V2 discussion.

I see a lot more in there than just detecting a platform and choosing a
driver.

> I feel there is no harm in keeping the state information in the same
> file. We do have x86, which has all its variants information in one
> file. One place will have all the idle consolidated information of
> all the platform variants. If community does feel, we need to
> have just the states information in arch specific file, we can do so.

What actual functionality is common to all powerpc but not common to
other arches?

> >> diff --git a/drivers/cpuidle/Kconfig b/drivers/cpuidle/Kconfig
> >> index 0e2cd5c..99ee5d4 100644
> >> --- a/drivers/cpuidle/Kconfig
(Continue reading)

Deepthi Dharwar | 21 Aug 06:53 2013
Picon

Re: [RFC PATCH V3 3/5] powerpc/cpuidle: Generic powerpc backend cpuidle driver.

On 08/19/2013 11:47 PM, Scott Wood wrote:
> On Mon, 2013-08-19 at 15:48 +0530, Deepthi Dharwar wrote:
>> Hi Dongsheng,
>>
>> On 08/19/2013 11:22 AM, Wang Dongsheng-B40534 wrote:
>>> I think we should move the states and handle function to arch/power/platform*
>>> The states and handle function is belong to backend driver, not for this, different platform have
different state.
>>> Different platforms to make their own deal with these states.
>>>
>>> I think we cannot put all the status of different platforms and handler in this driver.
>>
>> The idea here is a single powerpc back-end driver, which does a runtime
>> detection of the platform it is running and choose the right
>> idle states table. This was one of outcome of V2 discussion.
> 
> I see a lot more in there than just detecting a platform and choosing a
> driver.
> 
>> I feel there is no harm in keeping the state information in the same
>> file. We do have x86, which has all its variants information in one
>> file. One place will have all the idle consolidated information of
>> all the platform variants. If community does feel, we need to
>> have just the states information in arch specific file, we can do so.
> 
> What actual functionality is common to all powerpc but not common to
> other arches?
> 
>>>> diff --git a/drivers/cpuidle/Kconfig b/drivers/cpuidle/Kconfig
>>>> index 0e2cd5c..99ee5d4 100644
(Continue reading)

Scott Wood | 21 Aug 22:08 2013

Re: [RFC PATCH V3 3/5] powerpc/cpuidle: Generic powerpc backend cpuidle driver.

On Wed, 2013-08-21 at 10:23 +0530, Deepthi Dharwar wrote:
> On 08/19/2013 11:47 PM, Scott Wood wrote:
> > On Mon, 2013-08-19 at 15:48 +0530, Deepthi Dharwar wrote:
> >> Hi Dongsheng,
> >>
> >> On 08/19/2013 11:22 AM, Wang Dongsheng-B40534 wrote:
> >>> I think we should move the states and handle function to arch/power/platform*
> >>> The states and handle function is belong to backend driver, not for this, different platform have
different state.
> >>> Different platforms to make their own deal with these states.
> >>>
> >>> I think we cannot put all the status of different platforms and handler in this driver.
> >>
> >> The idea here is a single powerpc back-end driver, which does a runtime
> >> detection of the platform it is running and choose the right
> >> idle states table. This was one of outcome of V2 discussion.
> > 
> > I see a lot more in there than just detecting a platform and choosing a
> > driver.
> > 
> >> I feel there is no harm in keeping the state information in the same
> >> file. We do have x86, which has all its variants information in one
> >> file. One place will have all the idle consolidated information of
> >> all the platform variants. If community does feel, we need to
> >> have just the states information in arch specific file, we can do so.
> > 
> > What actual functionality is common to all powerpc but not common to
> > other arches?

No answer?
(Continue reading)

Deepthi Dharwar | 22 Aug 07:50 2013
Picon

Re: [RFC PATCH V3 3/5] powerpc/cpuidle: Generic powerpc backend cpuidle driver.

On 08/22/2013 01:38 AM, Scott Wood wrote:
> On Wed, 2013-08-21 at 10:23 +0530, Deepthi Dharwar wrote:
>> On 08/19/2013 11:47 PM, Scott Wood wrote:
>>> On Mon, 2013-08-19 at 15:48 +0530, Deepthi Dharwar wrote:
>>>> Hi Dongsheng,
>>>>
>>>> On 08/19/2013 11:22 AM, Wang Dongsheng-B40534 wrote:
>>>>> I think we should move the states and handle function to arch/power/platform*
>>>>> The states and handle function is belong to backend driver, not for this, different platform have
different state.
>>>>> Different platforms to make their own deal with these states.
>>>>>
>>>>> I think we cannot put all the status of different platforms and handler in this driver.
>>>>
>>>> The idea here is a single powerpc back-end driver, which does a runtime
>>>> detection of the platform it is running and choose the right
>>>> idle states table. This was one of outcome of V2 discussion.
>>>
>>> I see a lot more in there than just detecting a platform and choosing a
>>> driver.
>>>
>>>> I feel there is no harm in keeping the state information in the same
>>>> file. We do have x86, which has all its variants information in one
>>>> file. One place will have all the idle consolidated information of
>>>> all the platform variants. If community does feel, we need to
>>>> have just the states information in arch specific file, we can do so.
>>>
>>> What actual functionality is common to all powerpc but not common to
>>> other arches?
> 
(Continue reading)

Benjamin Herrenschmidt | 22 Aug 07:58 2013

Re: [RFC PATCH V3 3/5] powerpc/cpuidle: Generic powerpc backend cpuidle driver.

On Thu, 2013-08-22 at 11:20 +0530, Deepthi Dharwar wrote:
> But if having cpuidle backend-driver separately for other powerpc arcs
> makes sense such that each one have their own state information etc
> then it makes sense to name the files as cpuidle-power.c,
> cpuilde-ppc32.c and so on.

If by "power" you mean IBM POWER machines/CPUs, then make it
cpuidle-ibm-power or cpuidle-book3s64 maybe to clarify what families it
affects.

Cheers
Ben.
Deepthi Dharwar | 22 Aug 08:41 2013
Picon

Re: [RFC PATCH V3 3/5] powerpc/cpuidle: Generic powerpc backend cpuidle driver.

On 08/22/2013 11:28 AM, Benjamin Herrenschmidt wrote:
> On Thu, 2013-08-22 at 11:20 +0530, Deepthi Dharwar wrote:
>> But if having cpuidle backend-driver separately for other powerpc arcs
>> makes sense such that each one have their own state information etc
>> then it makes sense to name the files as cpuidle-power.c,
>> cpuilde-ppc32.c and so on.
> 
> If by "power" you mean IBM POWER machines/CPUs, then make it
> cpuidle-ibm-power or cpuidle-book3s64 maybe to clarify what families it
> affects.

Sure. Thanks :)

Regards,
Deepthi

> Cheers
> Ben.
> 
> 
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev <at> lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
> 
> 
Deepthi Dharwar | 22 Aug 08:41 2013
Picon

Re: [RFC PATCH V3 3/5] powerpc/cpuidle: Generic powerpc backend cpuidle driver.

On 08/22/2013 11:28 AM, Benjamin Herrenschmidt wrote:
> On Thu, 2013-08-22 at 11:20 +0530, Deepthi Dharwar wrote:
>> But if having cpuidle backend-driver separately for other powerpc arcs
>> makes sense such that each one have their own state information etc
>> then it makes sense to name the files as cpuidle-power.c,
>> cpuilde-ppc32.c and so on.
> 
> If by "power" you mean IBM POWER machines/CPUs, then make it
> cpuidle-ibm-power or cpuidle-book3s64 maybe to clarify what families it
> affects.

Sure. Thanks :)

Regards,
Deepthi

> Cheers
> Ben.
> 
> 
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev <at> lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
> 
> 
Scott Wood | 22 Aug 23:24 2013

Re: [RFC PATCH V3 3/5] powerpc/cpuidle: Generic powerpc backend cpuidle driver.

On Thu, 2013-08-22 at 11:20 +0530, Deepthi Dharwar wrote:
> On 08/22/2013 01:38 AM, Scott Wood wrote:
> > On Wed, 2013-08-21 at 10:23 +0530, Deepthi Dharwar wrote:
> >> On 08/19/2013 11:47 PM, Scott Wood wrote:
> >>> What actual functionality is common to all powerpc but not common to
> >>> other arches?
> > 
> 
> The functionality here is idle states on powerpc  like the snooze loop
> that is common.
> Also, the basic registration of the driver, hotplug notifier etc for
> powerpc.

The snooze loop uses things like SPRN_PURR, get_lppaca(), and CTRL which
aren't common to all PPC (they might be common to all book3s64).  I also
don't see any hook for the low power mode entry -- is "snooze" just a
busy loop plus the de-emphasis stuff like HMT and CTRL[RUN]?  I'm not
familiar with the term "snooze" in this context.  I don't think we'd use
anything like that on our chips; we'd always at least "wait" or "doze"
depending on the chip.

It's not clear what is powerpc-specific about the notifier -- perhaps it
should go in drivers/cpuidle/.

> > The way forward is to give this file a more appropriate name based on
> > the hardware that it actually targets -- and to refactor it so that the
> > answer to that question is not complicated.
> 
> Sure, thanks.
> Our idea was to have POWER archs idle states merged at the first go.
(Continue reading)

Deepthi Dharwar | 23 Aug 12:11 2013
Picon

Re: [RFC PATCH V3 3/5] powerpc/cpuidle: Generic powerpc backend cpuidle driver.

On 08/23/2013 02:54 AM, Scott Wood wrote:
> On Thu, 2013-08-22 at 11:20 +0530, Deepthi Dharwar wrote:
>> On 08/22/2013 01:38 AM, Scott Wood wrote:
>>> On Wed, 2013-08-21 at 10:23 +0530, Deepthi Dharwar wrote:
>>>> On 08/19/2013 11:47 PM, Scott Wood wrote:
>>>>> What actual functionality is common to all powerpc but not common to
>>>>> other arches?
>>>
>>
>> The functionality here is idle states on powerpc  like the snooze loop
>> that is common.
>> Also, the basic registration of the driver, hotplug notifier etc for
>> powerpc.
> 
> The snooze loop uses things like SPRN_PURR, get_lppaca(), and CTRL which
> aren't common to all PPC (they might be common to all book3s64).  I also
> don't see any hook for the low power mode entry -- is "snooze" just a
> busy loop plus the de-emphasis stuff like HMT and CTRL[RUN]?  I'm not
> familiar with the term "snooze" in this context.  I don't think we'd use
> anything like that on our chips; we'd always at least "wait" or "doze"
> depending on the chip.
>

Duly noted. Lot of stuff are common across book3s64. So my later
versions of this patchset does just that. (V5 posted out yesterday).
The driver is common only to IBM-POWER platform. Other PPC variants
can have their own driver.

> It's not clear what is powerpc-specific about the notifier -- perhaps it
> should go in drivers/cpuidle/.
(Continue reading)

Deepthi Dharwar | 23 Aug 12:11 2013
Picon

Re: [RFC PATCH V3 3/5] powerpc/cpuidle: Generic powerpc backend cpuidle driver.

On 08/23/2013 02:54 AM, Scott Wood wrote:
> On Thu, 2013-08-22 at 11:20 +0530, Deepthi Dharwar wrote:
>> On 08/22/2013 01:38 AM, Scott Wood wrote:
>>> On Wed, 2013-08-21 at 10:23 +0530, Deepthi Dharwar wrote:
>>>> On 08/19/2013 11:47 PM, Scott Wood wrote:
>>>>> What actual functionality is common to all powerpc but not common to
>>>>> other arches?
>>>
>>
>> The functionality here is idle states on powerpc  like the snooze loop
>> that is common.
>> Also, the basic registration of the driver, hotplug notifier etc for
>> powerpc.
> 
> The snooze loop uses things like SPRN_PURR, get_lppaca(), and CTRL which
> aren't common to all PPC (they might be common to all book3s64).  I also
> don't see any hook for the low power mode entry -- is "snooze" just a
> busy loop plus the de-emphasis stuff like HMT and CTRL[RUN]?  I'm not
> familiar with the term "snooze" in this context.  I don't think we'd use
> anything like that on our chips; we'd always at least "wait" or "doze"
> depending on the chip.
>

Duly noted. Lot of stuff are common across book3s64. So my later
versions of this patchset does just that. (V5 posted out yesterday).
The driver is common only to IBM-POWER platform. Other PPC variants
can have their own driver.

> It's not clear what is powerpc-specific about the notifier -- perhaps it
> should go in drivers/cpuidle/.
(Continue reading)

Benjamin Herrenschmidt | 22 Aug 07:58 2013

Re: [RFC PATCH V3 3/5] powerpc/cpuidle: Generic powerpc backend cpuidle driver.

On Thu, 2013-08-22 at 11:20 +0530, Deepthi Dharwar wrote:
> But if having cpuidle backend-driver separately for other powerpc arcs
> makes sense such that each one have their own state information etc
> then it makes sense to name the files as cpuidle-power.c,
> cpuilde-ppc32.c and so on.

If by "power" you mean IBM POWER machines/CPUs, then make it
cpuidle-ibm-power or cpuidle-book3s64 maybe to clarify what families it
affects.

Cheers
Ben.
Scott Wood | 22 Aug 23:24 2013

Re: [RFC PATCH V3 3/5] powerpc/cpuidle: Generic powerpc backend cpuidle driver.

On Thu, 2013-08-22 at 11:20 +0530, Deepthi Dharwar wrote:
> On 08/22/2013 01:38 AM, Scott Wood wrote:
> > On Wed, 2013-08-21 at 10:23 +0530, Deepthi Dharwar wrote:
> >> On 08/19/2013 11:47 PM, Scott Wood wrote:
> >>> What actual functionality is common to all powerpc but not common to
> >>> other arches?
> > 
> 
> The functionality here is idle states on powerpc  like the snooze loop
> that is common.
> Also, the basic registration of the driver, hotplug notifier etc for
> powerpc.

The snooze loop uses things like SPRN_PURR, get_lppaca(), and CTRL which
aren't common to all PPC (they might be common to all book3s64).  I also
don't see any hook for the low power mode entry -- is "snooze" just a
busy loop plus the de-emphasis stuff like HMT and CTRL[RUN]?  I'm not
familiar with the term "snooze" in this context.  I don't think we'd use
anything like that on our chips; we'd always at least "wait" or "doze"
depending on the chip.

It's not clear what is powerpc-specific about the notifier -- perhaps it
should go in drivers/cpuidle/.

> > The way forward is to give this file a more appropriate name based on
> > the hardware that it actually targets -- and to refactor it so that the
> > answer to that question is not complicated.
> 
> Sure, thanks.
> Our idea was to have POWER archs idle states merged at the first go.
(Continue reading)

Deepthi Dharwar | 22 Aug 07:50 2013
Picon

Re: [RFC PATCH V3 3/5] powerpc/cpuidle: Generic powerpc backend cpuidle driver.

On 08/22/2013 01:38 AM, Scott Wood wrote:
> On Wed, 2013-08-21 at 10:23 +0530, Deepthi Dharwar wrote:
>> On 08/19/2013 11:47 PM, Scott Wood wrote:
>>> On Mon, 2013-08-19 at 15:48 +0530, Deepthi Dharwar wrote:
>>>> Hi Dongsheng,
>>>>
>>>> On 08/19/2013 11:22 AM, Wang Dongsheng-B40534 wrote:
>>>>> I think we should move the states and handle function to arch/power/platform*
>>>>> The states and handle function is belong to backend driver, not for this, different platform have
different state.
>>>>> Different platforms to make their own deal with these states.
>>>>>
>>>>> I think we cannot put all the status of different platforms and handler in this driver.
>>>>
>>>> The idea here is a single powerpc back-end driver, which does a runtime
>>>> detection of the platform it is running and choose the right
>>>> idle states table. This was one of outcome of V2 discussion.
>>>
>>> I see a lot more in there than just detecting a platform and choosing a
>>> driver.
>>>
>>>> I feel there is no harm in keeping the state information in the same
>>>> file. We do have x86, which has all its variants information in one
>>>> file. One place will have all the idle consolidated information of
>>>> all the platform variants. If community does feel, we need to
>>>> have just the states information in arch specific file, we can do so.
>>>
>>> What actual functionality is common to all powerpc but not common to
>>> other arches?
> 
(Continue reading)

Scott Wood | 21 Aug 22:08 2013

Re: [RFC PATCH V3 3/5] powerpc/cpuidle: Generic powerpc backend cpuidle driver.

On Wed, 2013-08-21 at 10:23 +0530, Deepthi Dharwar wrote:
> On 08/19/2013 11:47 PM, Scott Wood wrote:
> > On Mon, 2013-08-19 at 15:48 +0530, Deepthi Dharwar wrote:
> >> Hi Dongsheng,
> >>
> >> On 08/19/2013 11:22 AM, Wang Dongsheng-B40534 wrote:
> >>> I think we should move the states and handle function to arch/power/platform*
> >>> The states and handle function is belong to backend driver, not for this, different platform have
different state.
> >>> Different platforms to make their own deal with these states.
> >>>
> >>> I think we cannot put all the status of different platforms and handler in this driver.
> >>
> >> The idea here is a single powerpc back-end driver, which does a runtime
> >> detection of the platform it is running and choose the right
> >> idle states table. This was one of outcome of V2 discussion.
> > 
> > I see a lot more in there than just detecting a platform and choosing a
> > driver.
> > 
> >> I feel there is no harm in keeping the state information in the same
> >> file. We do have x86, which has all its variants information in one
> >> file. One place will have all the idle consolidated information of
> >> all the platform variants. If community does feel, we need to
> >> have just the states information in arch specific file, we can do so.
> > 
> > What actual functionality is common to all powerpc but not common to
> > other arches?

No answer?
(Continue reading)

Deepthi Dharwar | 21 Aug 06:53 2013
Picon

Re: [RFC PATCH V3 3/5] powerpc/cpuidle: Generic powerpc backend cpuidle driver.

On 08/19/2013 11:47 PM, Scott Wood wrote:
> On Mon, 2013-08-19 at 15:48 +0530, Deepthi Dharwar wrote:
>> Hi Dongsheng,
>>
>> On 08/19/2013 11:22 AM, Wang Dongsheng-B40534 wrote:
>>> I think we should move the states and handle function to arch/power/platform*
>>> The states and handle function is belong to backend driver, not for this, different platform have
different state.
>>> Different platforms to make their own deal with these states.
>>>
>>> I think we cannot put all the status of different platforms and handler in this driver.
>>
>> The idea here is a single powerpc back-end driver, which does a runtime
>> detection of the platform it is running and choose the right
>> idle states table. This was one of outcome of V2 discussion.
> 
> I see a lot more in there than just detecting a platform and choosing a
> driver.
> 
>> I feel there is no harm in keeping the state information in the same
>> file. We do have x86, which has all its variants information in one
>> file. One place will have all the idle consolidated information of
>> all the platform variants. If community does feel, we need to
>> have just the states information in arch specific file, we can do so.
> 
> What actual functionality is common to all powerpc but not common to
> other arches?
> 
>>>> diff --git a/drivers/cpuidle/Kconfig b/drivers/cpuidle/Kconfig
>>>> index 0e2cd5c..99ee5d4 100644
(Continue reading)

Scott Wood | 19 Aug 20:17 2013

Re: [RFC PATCH V3 3/5] powerpc/cpuidle: Generic powerpc backend cpuidle driver.

On Mon, 2013-08-19 at 15:48 +0530, Deepthi Dharwar wrote:
> Hi Dongsheng,
> 
> On 08/19/2013 11:22 AM, Wang Dongsheng-B40534 wrote:
> > I think we should move the states and handle function to arch/power/platform*
> > The states and handle function is belong to backend driver, not for this, different platform have
different state.
> > Different platforms to make their own deal with these states.
> > 
> > I think we cannot put all the status of different platforms and handler in this driver.
> 
> The idea here is a single powerpc back-end driver, which does a runtime
> detection of the platform it is running and choose the right
> idle states table. This was one of outcome of V2 discussion.

I see a lot more in there than just detecting a platform and choosing a
driver.

> I feel there is no harm in keeping the state information in the same
> file. We do have x86, which has all its variants information in one
> file. One place will have all the idle consolidated information of
> all the platform variants. If community does feel, we need to
> have just the states information in arch specific file, we can do so.

What actual functionality is common to all powerpc but not common to
other arches?

> >> diff --git a/drivers/cpuidle/Kconfig b/drivers/cpuidle/Kconfig
> >> index 0e2cd5c..99ee5d4 100644
> >> --- a/drivers/cpuidle/Kconfig
(Continue reading)

Deepthi Dharwar | 19 Aug 12:18 2013
Picon

Re: [RFC PATCH V3 3/5] powerpc/cpuidle: Generic powerpc backend cpuidle driver.


Hi Dongsheng,

On 08/19/2013 11:22 AM, Wang Dongsheng-B40534 wrote:
> I think we should move the states and handle function to arch/power/platform*
> The states and handle function is belong to backend driver, not for this, different platform have
different state.
> Different platforms to make their own deal with these states.
> 
> I think we cannot put all the status of different platforms and handler in this driver.

The idea here is a single powerpc back-end driver, which does a runtime
detection of the platform it is running and choose the right
idle states table. This was one of outcome of V2 discussion.

I feel there is no harm in keeping the state information in the same
file. We do have x86, which has all its variants information in one
file. One place will have all the idle consolidated information of
all the platform variants. If community does feel, we need to
have just the states information in arch specific file, we can do so.

>> diff --git a/drivers/cpuidle/Kconfig b/drivers/cpuidle/Kconfig
>> index 0e2cd5c..99ee5d4 100644
>> --- a/drivers/cpuidle/Kconfig
>> +++ b/drivers/cpuidle/Kconfig
>>  <at>  <at>  -42,6 +42,13  <at>  <at>  config CPU_IDLE_ZYNQ
>>  	help
>>  	  Select this to enable cpuidle on Xilinx Zynq processors.
>>
>> +config CPU_IDLE_POWERPC
(Continue reading)

Wang Dongsheng-B40534 | 19 Aug 07:52 2013

RE: [RFC PATCH V3 3/5] powerpc/cpuidle: Generic powerpc backend cpuidle driver.

I think we should move the states and handle function to arch/power/platform*
The states and handle function is belong to backend driver, not for this, different platform have
different state.
Different platforms to make their own deal with these states.

I think we cannot put all the status of different platforms and handler in this driver.

> diff --git a/drivers/cpuidle/Kconfig b/drivers/cpuidle/Kconfig
> index 0e2cd5c..99ee5d4 100644
> --- a/drivers/cpuidle/Kconfig
> +++ b/drivers/cpuidle/Kconfig
>  <at>  <at>  -42,6 +42,13  <at>  <at>  config CPU_IDLE_ZYNQ
>  	help
>  	  Select this to enable cpuidle on Xilinx Zynq processors.
> 
> +config CPU_IDLE_POWERPC
> +	bool "CPU Idle driver for POWERPC platforms"
> +	depends on PPC64

Why not PPC?

> +	default y
> +        help
> +          Select this option to enable processor idle state management
> +	  for POWERPC platform.
>  endif
> 
>  config ARCH_NEEDS_CPU_IDLE_COUPLED
> diff --git a/drivers/cpuidle/Makefile b/drivers/cpuidle/Makefile
> index 8767a7b..d12e205 100644
(Continue reading)

Deepthi Dharwar | 19 Aug 06:28 2013
Picon

[RFC PATCH V3 4/5] powerpc/cpuidle: Enable powernv cpuidle support.

The following patch extends the current powerpc backend
idle driver to the powernv platform.

Signed-off-by: Deepthi Dharwar <deepthi <at> linux.vnet.ibm.com>
---
 drivers/cpuidle/cpuidle-powerpc.c |   36 +++++++++++++++++++++++++++++++++---
 1 file changed, 33 insertions(+), 3 deletions(-)

diff --git a/drivers/cpuidle/cpuidle-powerpc.c b/drivers/cpuidle/cpuidle-powerpc.c
index 5756085..e1cf599 100644
--- a/drivers/cpuidle/cpuidle-powerpc.c
+++ b/drivers/cpuidle/cpuidle-powerpc.c
 <at>  <at>  -53,7 +53,9  <at>  <at>  static int snooze_loop(struct cpuidle_device *dev,
 {
 	unsigned long in_purr;

-	idle_loop_prolog(&in_purr);
+	if (firmware_has_feature(FW_FEATURE_SPLPAR))
+		idle_loop_prolog(&in_purr);
+
 	local_irq_enable();
 	set_thread_flag(TIF_POLLING_NRFLAG);

 <at>  <at>  -67,7 +69,8  <at>  <at>  static int snooze_loop(struct cpuidle_device *dev,
 	clear_thread_flag(TIF_POLLING_NRFLAG);
 	smp_mb();

-	idle_loop_epilog(in_purr);
+	if (firmware_has_feature(FW_FEATURE_SPLPAR))
+		idle_loop_epilog(in_purr);
(Continue reading)

Deepthi Dharwar | 19 Aug 06:28 2013
Picon

[RFC PATCH V3 5/5] powernv/cpuidle: Enable idle powernv cpu to call into the cpuidle framework.

This patch enables idle cpu on the powernv platform  to hook on to the cpuidle
framework, if available, else call on to default idle platform
code.

Signed-off-by: Deepthi Dharwar <deepthi <at> linux.vnet.ibm.com>
---
 arch/powerpc/platforms/powernv/setup.c |   14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/powernv/setup.c b/arch/powerpc/platforms/powernv/setup.c
index 84438af..fc62f21 100644
--- a/arch/powerpc/platforms/powernv/setup.c
+++ b/arch/powerpc/platforms/powernv/setup.c
 <at>  <at>  -25,6 +25,7  <at>  <at> 
 #include <linux/of.h>
 #include <linux/interrupt.h>
 #include <linux/bug.h>
+#include <linux/cpuidle.h>

 #include <asm/machdep.h>
 #include <asm/firmware.h>
 <at>  <at>  -175,6 +176,17  <at>  <at>  static void __init pnv_setup_machdep_rtas(void)
 }
 #endif /* CONFIG_PPC_POWERNV_RTAS */

+void powernv_idle(void)
+{
+	/* Hook to cpuidle framework if available, else
+	 * call on default platform idle code
+	 */
(Continue reading)

Deepthi Dharwar | 19 Aug 06:28 2013
Picon

[RFC PATCH V3 1/5] pseries/cpuidle: Remove dependency of pseries.h file

As a part of pseries_idle cleanup to make the backend driver
code common to both pseries and powernv.
Remove non-essential smt_snooze_delay declaration in pseries.h
header file and pseries.h file inclusion in
pseries/processor_idle.c

Signed-off-by: Deepthi Dharwar <deepthi <at> linux.vnet.ibm.com>
---
 arch/powerpc/platforms/pseries/processor_idle.c |    1 -
 arch/powerpc/platforms/pseries/pseries.h        |    3 ---
 2 files changed, 4 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/processor_idle.c b/arch/powerpc/platforms/pseries/processor_idle.c
index 4644efa0..ca70279 100644
--- a/arch/powerpc/platforms/pseries/processor_idle.c
+++ b/arch/powerpc/platforms/pseries/processor_idle.c
 <at>  <at>  -20,7 +20,6  <at>  <at> 
 #include <asm/runlatch.h>

 #include "plpar_wrappers.h"
-#include "pseries.h"

 struct cpuidle_driver pseries_idle_driver = {
 	.name             = "pseries_idle",
diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h
index c2a3a25..d1b07e6 100644
--- a/arch/powerpc/platforms/pseries/pseries.h
+++ b/arch/powerpc/platforms/pseries/pseries.h
 <at>  <at>  -60,9 +60,6  <at>  <at>  extern struct device_node *dlpar_configure_connector(u32);
 extern int dlpar_attach_node(struct device_node *);
(Continue reading)

Deepthi Dharwar | 19 Aug 06:28 2013
Picon

[RFC PATCH V3 2/5] pseries: Move plpar_wrapper.h to powerpc common include/asm location.

As a part of pseries_idle backend driver cleanup to make
the code common to both pseries and powernv platforms, it
is necessary to move the backend-driver code to drivers/cpuidle.

As a pre-requisite for that, it is essential to move plpar_wrapper.h
to include/asm.

Signed-off-by: Deepthi Dharwar <deepthi <at> linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/plpar_wrappers.h       |  325 +++++++++++++++++++++++
 arch/powerpc/platforms/pseries/cmm.c            |    3 
 arch/powerpc/platforms/pseries/dtl.c            |    3 
 arch/powerpc/platforms/pseries/hotplug-cpu.c    |    3 
 arch/powerpc/platforms/pseries/hvconsole.c      |    2 
 arch/powerpc/platforms/pseries/iommu.c          |    3 
 arch/powerpc/platforms/pseries/kexec.c          |    2 
 arch/powerpc/platforms/pseries/lpar.c           |    2 
 arch/powerpc/platforms/pseries/plpar_wrappers.h |  324 -----------------------
 arch/powerpc/platforms/pseries/processor_idle.c |    3 
 arch/powerpc/platforms/pseries/setup.c          |    2 
 arch/powerpc/platforms/pseries/smp.c            |    2 
 12 files changed, 336 insertions(+), 338 deletions(-)
 create mode 100644 arch/powerpc/include/asm/plpar_wrappers.h
 delete mode 100644 arch/powerpc/platforms/pseries/plpar_wrappers.h

diff --git a/arch/powerpc/include/asm/plpar_wrappers.h b/arch/powerpc/include/asm/plpar_wrappers.h
new file mode 100644
index 0000000..e2f84d6
--- /dev/null
+++ b/arch/powerpc/include/asm/plpar_wrappers.h
(Continue reading)

Deepthi Dharwar | 19 Aug 06:28 2013
Picon

[RFC PATCH V3 3/5] powerpc/cpuidle: Generic powerpc backend cpuidle driver.

This patch involves moving the current pseries_idle backend driver code
from pseries/processor_idle.c to drivers/cpuidle/cpuidle-powerpc.c,
and making the backend code generic enough to be able to  extend this
driver code for complete powerpc platform to exploit the cpuidle framework.

It enables the support for pseries platform, such that going forward the same code
with minimal efforts can be re-used for a common driver on powernv
and can be further extended to support cpuidle idle state mgmt for the rest
of the powerpc platforms. This removes a lot of code duplicacy, making
the code elegant.

Signed-off-by: Deepthi Dharwar <deepthi <at> linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/paca.h                 |   23 +
 arch/powerpc/include/asm/processor.h            |    2 
 arch/powerpc/platforms/pseries/Kconfig          |    9 -
 arch/powerpc/platforms/pseries/Makefile         |    1 
 arch/powerpc/platforms/pseries/processor_idle.c |  360 -----------------------
 drivers/cpuidle/Kconfig                         |    7 
 drivers/cpuidle/Makefile                        |    2 
 drivers/cpuidle/cpuidle-powerpc.c               |  361 +++++++++++++++++++++++
 8 files changed, 394 insertions(+), 371 deletions(-)
 delete mode 100644 arch/powerpc/platforms/pseries/processor_idle.c
 create mode 100644 drivers/cpuidle/cpuidle-powerpc.c

diff --git a/arch/powerpc/include/asm/paca.h b/arch/powerpc/include/asm/paca.h
index 77c91e7..7bd83ff 100644
--- a/arch/powerpc/include/asm/paca.h
+++ b/arch/powerpc/include/asm/paca.h
 <at>  <at>  -175,6 +175,29  <at>  <at>  extern void setup_paca(struct paca_struct *new_paca);
(Continue reading)

Deepthi Dharwar | 19 Aug 06:28 2013
Picon

[RFC PATCH V3 4/5] powerpc/cpuidle: Enable powernv cpuidle support.

The following patch extends the current powerpc backend
idle driver to the powernv platform.

Signed-off-by: Deepthi Dharwar <deepthi <at> linux.vnet.ibm.com>
---
 drivers/cpuidle/cpuidle-powerpc.c |   36 +++++++++++++++++++++++++++++++++---
 1 file changed, 33 insertions(+), 3 deletions(-)

diff --git a/drivers/cpuidle/cpuidle-powerpc.c b/drivers/cpuidle/cpuidle-powerpc.c
index 5756085..e1cf599 100644
--- a/drivers/cpuidle/cpuidle-powerpc.c
+++ b/drivers/cpuidle/cpuidle-powerpc.c
 <at>  <at>  -53,7 +53,9  <at>  <at>  static int snooze_loop(struct cpuidle_device *dev,
 {
 	unsigned long in_purr;

-	idle_loop_prolog(&in_purr);
+	if (firmware_has_feature(FW_FEATURE_SPLPAR))
+		idle_loop_prolog(&in_purr);
+
 	local_irq_enable();
 	set_thread_flag(TIF_POLLING_NRFLAG);

 <at>  <at>  -67,7 +69,8  <at>  <at>  static int snooze_loop(struct cpuidle_device *dev,
 	clear_thread_flag(TIF_POLLING_NRFLAG);
 	smp_mb();

-	idle_loop_epilog(in_purr);
+	if (firmware_has_feature(FW_FEATURE_SPLPAR))
+		idle_loop_epilog(in_purr);
(Continue reading)

Deepthi Dharwar | 19 Aug 06:28 2013
Picon

[RFC PATCH V3 5/5] powernv/cpuidle: Enable idle powernv cpu to call into the cpuidle framework.

This patch enables idle cpu on the powernv platform  to hook on to the cpuidle
framework, if available, else call on to default idle platform
code.

Signed-off-by: Deepthi Dharwar <deepthi <at> linux.vnet.ibm.com>
---
 arch/powerpc/platforms/powernv/setup.c |   14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/powernv/setup.c b/arch/powerpc/platforms/powernv/setup.c
index 84438af..fc62f21 100644
--- a/arch/powerpc/platforms/powernv/setup.c
+++ b/arch/powerpc/platforms/powernv/setup.c
 <at>  <at>  -25,6 +25,7  <at>  <at> 
 #include <linux/of.h>
 #include <linux/interrupt.h>
 #include <linux/bug.h>
+#include <linux/cpuidle.h>

 #include <asm/machdep.h>
 #include <asm/firmware.h>
 <at>  <at>  -175,6 +176,17  <at>  <at>  static void __init pnv_setup_machdep_rtas(void)
 }
 #endif /* CONFIG_PPC_POWERNV_RTAS */

+void powernv_idle(void)
+{
+	/* Hook to cpuidle framework if available, else
+	 * call on default platform idle code
+	 */
(Continue reading)


Gmane