Re: RFC: Simplification of Power Domain Control
Hi Nishant, Rajendra,
On Fri, Jul 13, 2012 at 1:03 PM, Rajendra Nayak wrote:
> Hi Nishanth,
>
>
> On Friday 13 July 2012 03:21 PM, Menon, Nishanth wrote:
>>
>> On Thursday 05 July 2012, Rajendra Nayak wrote:
>> [..]
>>>
>>> From 5f5e4eb342110286bf719c7d9d7c1959f53e34f9 Mon Sep 17 00:00:00 2001
>>> From: Rajendra Nayak
>>> Date: Thu, 5 Jul 2012 17:33:28 +0530
>>> Subject: [RFC] ARM: OMAP: Powerdomain: control memory and logic bits
>>> internally
>>>
>>> Powerdomain framework exposes various apis for memory and logic
>>> control for powerdomains, for its users to program OSWR: Open SWitch
>>> Retention state. While in theory, there are various combinations of
>>> memory and logic states possible which can be configured as OSWR,
>>> in practice all OMAPs use just one combination. Logic lost, memory
>>> retained.
>>>
>>> This can very easily be handled within the powerdomain framework itself,
>>> without exposing all complex memory/logic control apis to upper layer
>>> drivers like cpuidle and suspend.
>>>
>>> To do this, introduce 2 new power domain states PWRDM_POWER_CSWR and
>>> PWRDM_POWER_OSWR usable by the users of powerdomain framework and
>>> make all memory/logic control apis internal to powerdomain framework.
>>> Change all users of powerdomain framework to get rid of all usage
>>> of memory/logic control apis and use the newly defined states for
>>> CSWR and OSWR with the already used powerstate control apis.
>>>
>>> Some functions (which are now made internal) are forward declared
>>> to avoid moving functions around in the file (which can be done in a
>>> later patch) to help keep the patch reviewable.
>>>
>>> Signed-off-by: Rajendra Nayak
>>
>>
>>
>> Ref: http://marc.info/?t=13396858684&r=1&w=2
>>
>> Apologies, but i've had to copypaste the original message, so inline
>> response
>> might be a bit messed up.
>>
>> From an initial port to get cpuidle working on OMAP5, my experiences
>> follow:
>
>
> Thanks for the tests and the review.
>
>>
>> a) counter handling (pm-debug.c) - we can now do better than give our
>> arcane RET:5
>> LOGIC-POWER-OFF:4 , instead we can clearly indicate OSWR, CSWR in
>> counter
>> Part of the issue also now becomes that count and time arrays are in
>> the range of
>> PWRDM_POWER_ON. They break when CSWR/OSWR is in pwrdm->state
Cf. http://marc.info/?l=linux-omap&m=133968582005062&w=2 for an
implementation of the power domains statistics using the func power
states. Of course the string format can be updated.
>>
>> b) pwrst handling this becomes a hard one to handle (as usual) when
>> comparisons of
>> while (!(pwrdm->pwrsts& (1<< pwrst))) {
>>
>> if (pwrst == PWRDM_POWER_OFF)
>> goto out;
>> pwrst--;
>> }
>>
>> with value 4, 5 -> pwrsts should either now use OSWR, CSWR definitions
>> OR we will need translate back before checks
That is correct. The proposed implementation introduces a new function
pwrdm_get_achievable_func_pwrst that walks through the _internal_
(i.e. registers) states for both the power state and the logic state.
Cf. http://marc.info/?l=linux-omap&m=133968582005061&w=2. The patch is
in RFC state and so it would be ideal to get some comments from the
knowledgeable persons.
>>
>> c) in few critical places, these mentioned error checks do silent
>> error returns - example:
>> if (!(pwrdm->pwrsts_logic_ret& (1<< pwrst)))
>>
>> return -EINVAL;
>> this bit me more than once while i tried to bring up the patch
>> we should be doing a patch which introduces a ratelimited WARN to kill the
>> bad callers.
Right!
>>
>> d) we have been lazy in programming and have been using cur_pwrst<
>> PWRDM_POWER_ON or INACTIVE etc.. and do a set of operations based off
>> that. this wont work as CSWR, OSWR> POWER_INACTIVE. (e.g. pm3 code)
That is one of the concerns with the current code. Any suggestion on that?
>
>
> All are valid issues. Some I overlooked, some like the array index
> issues due to CSWR/OSWR being defined post OFF, I knew but did not
> handle well because it was a hastily cooked up RFC to clarify our
> thoughts.
Correct. That is the problem with the RFC, which is trying to retain
most of the original code for the states handling.
>
> now that I have more feedback I will certainly wort on improving it.
>
>
>>
>> e) similar to what Jean did, omap_set_pwrdm_state will need to move
>> over from pm.c to powerdomain.c
Correct! The function could even be renamed to pwrdm_... Also the
private header file is a good concept.
>>
>> f) We probably should also will need an updated patch for
>> http://marc.info/?l=linux-omap&m=133968581105049&w=2
>
>
> Yes, certainly, they would be needed as well.
I have a new version of the patch. Cf. the tree at
https://gitorious.org/jpihet/linux (functional-pwrst branch).
>
> regards,
> Rajendra
Thanks and regards,
Jean
>
>>
>> Regards,
>> Nishanth Menon
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to
Re: RFC: Simplification of Power Domain Control
Hi Nishanth,
On Friday 13 July 2012 03:21 PM, Menon, Nishanth wrote:
On Thursday 05 July 2012, Rajendra Nayak wrote:
[..]
From 5f5e4eb342110286bf719c7d9d7c1959f53e34f9 Mon Sep 17 00:00:00 2001
From: Rajendra Nayak
Date: Thu, 5 Jul 2012 17:33:28 +0530
Subject: [RFC] ARM: OMAP: Powerdomain: control memory and logic bits internally
Powerdomain framework exposes various apis for memory and logic
control for powerdomains, for its users to program OSWR: Open SWitch
Retention state. While in theory, there are various combinations of
memory and logic states possible which can be configured as OSWR,
in practice all OMAPs use just one combination. Logic lost, memory retained.
This can very easily be handled within the powerdomain framework itself,
without exposing all complex memory/logic control apis to upper layer
drivers like cpuidle and suspend.
To do this, introduce 2 new power domain states PWRDM_POWER_CSWR and
PWRDM_POWER_OSWR usable by the users of powerdomain framework and
make all memory/logic control apis internal to powerdomain framework.
Change all users of powerdomain framework to get rid of all usage
of memory/logic control apis and use the newly defined states for
CSWR and OSWR with the already used powerstate control apis.
Some functions (which are now made internal) are forward declared
to avoid moving functions around in the file (which can be done in a
later patch) to help keep the patch reviewable.
Signed-off-by: Rajendra Nayak
Ref: http://marc.info/?t=13396858684&r=1&w=2
Apologies, but i've had to copypaste the original message, so inline response
might be a bit messed up.
From an initial port to get cpuidle working on OMAP5, my experiences follow:
Thanks for the tests and the review.
a) counter handling (pm-debug.c) - we can now do better than give our
arcane RET:5
LOGIC-POWER-OFF:4 , instead we can clearly indicate OSWR, CSWR in
counter
Part of the issue also now becomes that count and time arrays are in
the range of
PWRDM_POWER_ON. They break when CSWR/OSWR is in pwrdm->state
b) pwrst handling this becomes a hard one to handle (as usual) when
comparisons of
while (!(pwrdm->pwrsts& (1<< pwrst))) {
if (pwrst == PWRDM_POWER_OFF)
goto out;
pwrst--;
}
with value 4, 5 -> pwrsts should either now use OSWR, CSWR definitions
OR we will need translate back before checks
c) in few critical places, these mentioned error checks do silent
error returns - example:
if (!(pwrdm->pwrsts_logic_ret& (1<< pwrst)))
return -EINVAL;
this bit me more than once while i tried to bring up the patch
we should be doing a patch which introduces a ratelimited WARN to kill the
bad callers.
d) we have been lazy in programming and have been using cur_pwrst<
PWRDM_POWER_ON or INACTIVE etc.. and do a set of operations based off
that. this wont work as CSWR, OSWR> POWER_INACTIVE. (e.g. pm3 code)
All are valid issues. Some I overlooked, some like the array index
issues due to CSWR/OSWR being defined post OFF, I knew but did not
handle well because it was a hastily cooked up RFC to clarify our
thoughts.
now that I have more feedback I will certainly wort on improving it.
e) similar to what Jean did, omap_set_pwrdm_state will need to move
over from pm.c to powerdomain.c
f) We probably should also will need an updated patch for
http://marc.info/?l=linux-omap&m=133968581105049&w=2
Yes, certainly, they would be needed as well.
regards,
Rajendra
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC: Simplification of Power Domain Control
On Thursday 05 July 2012, Rajendra Nayak wrote:
[..]
> From 5f5e4eb342110286bf719c7d9d7c1959f53e34f9 Mon Sep 17 00:00:00 2001
> From: Rajendra Nayak
> Date: Thu, 5 Jul 2012 17:33:28 +0530
> Subject: [RFC] ARM: OMAP: Powerdomain: control memory and logic bits
> internally
>
> Powerdomain framework exposes various apis for memory and logic
> control for powerdomains, for its users to program OSWR: Open SWitch
> Retention state. While in theory, there are various combinations of
> memory and logic states possible which can be configured as OSWR,
> in practice all OMAPs use just one combination. Logic lost, memory retained.
>
> This can very easily be handled within the powerdomain framework itself,
> without exposing all complex memory/logic control apis to upper layer
> drivers like cpuidle and suspend.
>
> To do this, introduce 2 new power domain states PWRDM_POWER_CSWR and
> PWRDM_POWER_OSWR usable by the users of powerdomain framework and
> make all memory/logic control apis internal to powerdomain framework.
> Change all users of powerdomain framework to get rid of all usage
> of memory/logic control apis and use the newly defined states for
> CSWR and OSWR with the already used powerstate control apis.
>
> Some functions (which are now made internal) are forward declared
> to avoid moving functions around in the file (which can be done in a
> later patch) to help keep the patch reviewable.
>
> Signed-off-by: Rajendra Nayak
Ref: http://marc.info/?t=13396858684&r=1&w=2
Apologies, but i've had to copypaste the original message, so inline response
might be a bit messed up.
>From an initial port to get cpuidle working on OMAP5, my experiences follow:
a) counter handling (pm-debug.c) - we can now do better than give our
arcane RET:5
LOGIC-POWER-OFF:4 , instead we can clearly indicate OSWR, CSWR in
counter
Part of the issue also now becomes that count and time arrays are in
the range of
PWRDM_POWER_ON. They break when CSWR/OSWR is in pwrdm->state
b) pwrst handling this becomes a hard one to handle (as usual) when
comparisons of
while (!(pwrdm->pwrsts & (1 << pwrst))) {
if (pwrst == PWRDM_POWER_OFF)
goto out;
pwrst--;
}
with value 4, 5 -> pwrsts should either now use OSWR, CSWR definitions
OR we will need translate back before checks
c) in few critical places, these mentioned error checks do silent
error returns - example:
if (!(pwrdm->pwrsts_logic_ret & (1 << pwrst)))
return -EINVAL;
this bit me more than once while i tried to bring up the patch
we should be doing a patch which introduces a ratelimited WARN to kill the
bad callers.
d) we have been lazy in programming and have been using cur_pwrst <
PWRDM_POWER_ON or INACTIVE etc.. and do a set of operations based off
that. this wont work as CSWR, OSWR > POWER_INACTIVE. (e.g. pm3 code)
e) similar to what Jean did, omap_set_pwrdm_state will need to move
over from pm.c to powerdomain.c
f) We probably should also will need an updated patch for
http://marc.info/?l=linux-omap&m=133968581105049&w=2
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC: Simplification of Power Domain Control
On Thursday 05 July 2012 06:38 PM, Jean Pihet wrote: There was also some > offline discussion about this between me, Santosh Shilimkar and > Paul Walmsley. Good to know! And... ? oh, the discussion was just about the approach and there was nothing concluded. So I just cooked up this RFC so there can be some more discussion. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC: Simplification of Power Domain Control
Hi Jean, On Thursday 05 July 2012 06:36 PM, Jean Pihet wrote: Hi Rajendra, On Thu, Jul 5, 2012 at 2:47 PM, Rajendra Nayak wrote: Hi, This RFC is aimed at simplifying the powerdomain control (in software) for OMAP. Powerdomains on OMAP have been fairly complex to program (as compared to other SoCs) mainly due to the multiple memory/logic control exposed to software. These controls are limited to achieving whats know as 'OSWR: Open Switch Retention' state in OMAP. Rest of the states supported (ON, INACTIVE, CSWR, OFF) are fairly easy to program needing just a target power state to be configured. While in theory, the hardware does expose multiple ways/ combinations in which OSWR can be achieved, we in software have always been using just *one* combination all along. OSWR has always been configured as *logic lost, all memory retained* for all current OMAPs supported. With future OMAPs planning to get rid of all the memory/logic control altogether it makes sense to look at the apis exposed through the current powerdomain framework to its users (like cpuilde, suspend etc) and see if those could also be simplified. The RFC is suspend tested on OMAP4430sdp and OMAP3630Beagle-Xm. This looks very similar to the functional power states patches that I submitted a few times for review [1]. Sorry, I should have mentioned the context in which this RFC was developed. Yes, its very similar to the functional power state approach, infact some of these thoughts came up as part of the discussion between me and Santosh while doing the review for that very series. regards, Rajendra The idea is to simplify the external API to program the power domains states. The code has been used to implement the OMAP4 device OFF support (from Tero) and the per-device PM QoS implementation for OMAP [2]. [1] http://marc.info/?l=linux-omap&m=133968580905048&w=2 [2] http://marc.info/?l=linux-omap&m=133968657005566&w=2 Now that we have more than one implementation, I let it to the maintainers for decision. Paul, what do you think? A few remarks here below. Thanks for your RFC! regards, Rajendra From 5f5e4eb342110286bf719c7d9d7c1959f53e34f9 Mon Sep 17 00:00:00 2001 From: Rajendra Nayak Date: Thu, 5 Jul 2012 17:33:28 +0530 Subject: [RFC] ARM: OMAP: Powerdomain: control memory and logic bits internally Powerdomain framework exposes various apis for memory and logic control for powerdomains, for its users to program OSWR: Open SWitch Retention state. While in theory, there are various combinations of memory and logic states possible which can be configured as OSWR, in practice all OMAPs use just one combination. Logic lost, memory retained. This can very easily be handled within the powerdomain framework itself, without exposing all complex memory/logic control apis to upper layer drivers like cpuidle and suspend. To do this, introduce 2 new power domain states PWRDM_POWER_CSWR and PWRDM_POWER_OSWR usable by the users of powerdomain framework and make all memory/logic control apis internal to powerdomain framework. Change all users of powerdomain framework to get rid of all usage of memory/logic control apis and use the newly defined states for CSWR and OSWR with the already used powerstate control apis. Some functions (which are now made internal) are forward declared to avoid moving functions around in the file (which can be done in a later patch) to help keep the patch reviewable. Signed-off-by: Rajendra Nayak --- arch/arm/mach-omap2/cpuidle44xx.c|9 +--- arch/arm/mach-omap2/omap-mpuss-lowpower.c|7 +-- arch/arm/mach-omap2/pm24xx.c | 17 +-- arch/arm/mach-omap2/pm34xx.c |8 ++-- arch/arm/mach-omap2/pm44xx.c | 11 +--- arch/arm/mach-omap2/powerdomain-private.h| 17 +++ arch/arm/mach-omap2/powerdomain.c| 54 +++--- arch/arm/mach-omap2/powerdomain.h| 14 +- arch/arm/mach-omap2/powerdomains2xxx_3xxx_data.c |1 + arch/arm/mach-omap2/powerdomains2xxx_data.c |1 + arch/arm/mach-omap2/powerdomains3xxx_data.c |1 + arch/arm/mach-omap2/powerdomains44xx_data.c |1 + 12 files changed, 85 insertions(+), 56 deletions(-) create mode 100644 arch/arm/mach-omap2/powerdomain-private.h ... diff --git a/arch/arm/mach-omap2/omap-mpuss-lowpower.c b/arch/arm/mach-omap2/omap-mpuss-lowpower.c index 13670aa..41d559b 100644 --- a/arch/arm/mach-omap2/omap-mpuss-lowpower.c +++ b/arch/arm/mach-omap2/omap-mpuss-lowpower.c ... @@ -243,7 +243,7 @@ int omap4_enter_lowpower(unsigned int cpu, unsigned int power_state) case PWRDM_POWER_OFF: save_state = 1; break; - case PWRDM_POWER_RET: + case PWRDM_POWER_CSWR: default: /* * CPUx CSWR is invalid hardware state. Also CPUx OSWR @@ -262,8 +262,7 @@ int omap4_enter_lowpower(unsigned
Re: RFC: Simplification of Power Domain Control
On Thu, Jul 5, 2012 at 3:03 PM, Rajendra Nayak wrote:
> Just to give more context, the though of doing something like
> what the below RFC does came up as part of the patch review of
> Jean's "Functional power state" series.
My reply crossed yours...
> There was also some
> offline discussion about this between me, Santosh Shilimkar and
> Paul Walmsley.
Good to know!
And... ?
Jean.
>
>
> On Thursday 05 July 2012 06:17 PM, Rajendra Nayak wrote:
>>
>> Hi,
>>
>> This RFC is aimed at simplifying the powerdomain control
>> (in software) for OMAP. Powerdomains on OMAP have been
>> fairly complex to program (as compared to other SoCs)
>> mainly due to the multiple memory/logic control exposed
>> to software. These controls are limited to achieving
>> whats know as 'OSWR: Open Switch Retention' state in OMAP.
>> Rest of the states supported (ON, INACTIVE, CSWR, OFF) are
>> fairly easy to program needing just a target power state
>> to be configured.
>>
>> While in theory, the hardware does expose multiple ways/
>> combinations in which OSWR can be achieved, we in software
>> have always been using just *one* combination all along.
>> OSWR has always been configured as *logic lost, all memory
>> retained* for all current OMAPs supported.
>>
>> With future OMAPs planning to get rid of all the memory/logic
>> control altogether it makes sense to look at the apis exposed
>> through the current powerdomain framework to its users (like
>> cpuilde, suspend etc) and see if those could also be simplified.
>>
>> The RFC is suspend tested on OMAP4430sdp and
>> OMAP3630Beagle-Xm.
>>
>> regards,
>> Rajendra
>>
>>
>> From 5f5e4eb342110286bf719c7d9d7c1959f53e34f9 Mon Sep 17 00:00:00 2001
>> From: Rajendra Nayak
>> Date: Thu, 5 Jul 2012 17:33:28 +0530
>> Subject: [RFC] ARM: OMAP: Powerdomain: control memory and logic bits
>> internally
>>
>> Powerdomain framework exposes various apis for memory and logic
>> control for powerdomains, for its users to program OSWR: Open SWitch
>> Retention state. While in theory, there are various combinations of
>> memory and logic states possible which can be configured as OSWR,
>> in practice all OMAPs use just one combination. Logic lost, memory
>> retained.
>>
>> This can very easily be handled within the powerdomain framework itself,
>> without exposing all complex memory/logic control apis to upper layer
>> drivers like cpuidle and suspend.
>>
>> To do this, introduce 2 new power domain states PWRDM_POWER_CSWR and
>> PWRDM_POWER_OSWR usable by the users of powerdomain framework and
>> make all memory/logic control apis internal to powerdomain framework.
>> Change all users of powerdomain framework to get rid of all usage
>> of memory/logic control apis and use the newly defined states for
>> CSWR and OSWR with the already used powerstate control apis.
>>
>> Some functions (which are now made internal) are forward declared
>> to avoid moving functions around in the file (which can be done in a
>> later patch) to help keep the patch reviewable.
>>
>> Signed-off-by: Rajendra Nayak
>> ---
>> arch/arm/mach-omap2/cpuidle44xx.c|9 +---
>> arch/arm/mach-omap2/omap-mpuss-lowpower.c|7 +--
>> arch/arm/mach-omap2/pm24xx.c | 17 +--
>> arch/arm/mach-omap2/pm34xx.c |8 ++--
>> arch/arm/mach-omap2/pm44xx.c | 11 +---
>> arch/arm/mach-omap2/powerdomain-private.h| 17 +++
>> arch/arm/mach-omap2/powerdomain.c| 54
>> +++---
>> arch/arm/mach-omap2/powerdomain.h| 14 +-
>> arch/arm/mach-omap2/powerdomains2xxx_3xxx_data.c |1 +
>> arch/arm/mach-omap2/powerdomains2xxx_data.c |1 +
>> arch/arm/mach-omap2/powerdomains3xxx_data.c |1 +
>> arch/arm/mach-omap2/powerdomains44xx_data.c |1 +
>> 12 files changed, 85 insertions(+), 56 deletions(-)
>> create mode 100644 arch/arm/mach-omap2/powerdomain-private.h
>>
>> diff --git a/arch/arm/mach-omap2/cpuidle44xx.c
>> b/arch/arm/mach-omap2/cpuidle44xx.c
>> index be1617c..d40fe90 100644
>> --- a/arch/arm/mach-omap2/cpuidle44xx.c
>> +++ b/arch/arm/mach-omap2/cpuidle44xx.c
>> @@ -27,7 +27,6 @@
>> /* Machine specific information */
>> struct omap4_idle_statedata {
>> u32 cpu_state;
>> - u32 mpu_logic_state;
>> u32 mpu_state;
>> };
>>
>> @@ -35,17 +34,14 @@ static struct omap4_idle_statedata omap4_idle_data[] =
>> {
>> {
>> .cpu_state = PWRDM_POWER_ON,
>> .mpu_state = PWRDM_POWER_ON,
>> - .mpu_logic_state = PWRDM_POWER_RET,
>> },
>> {
>> .cpu_state = PWRDM_POWER_OFF,
>> - .mpu_state = PWRDM_POWER_RET,
>> - .mpu_logic_state = PWRDM_POWER_RET,
>> + .mpu_state = PWRDM_POWER_CSWR,
>> },
>> {
>> .cpu_state = PWRDM_POWER_OFF,
>> - .mpu_state = PWRDM_POWER_RET
Re: RFC: Simplification of Power Domain Control
Hi Rajendra, On Thu, Jul 5, 2012 at 2:47 PM, Rajendra Nayak wrote: > Hi, > > This RFC is aimed at simplifying the powerdomain control > (in software) for OMAP. Powerdomains on OMAP have been > fairly complex to program (as compared to other SoCs) > mainly due to the multiple memory/logic control exposed > to software. These controls are limited to achieving > whats know as 'OSWR: Open Switch Retention' state in OMAP. > Rest of the states supported (ON, INACTIVE, CSWR, OFF) are > fairly easy to program needing just a target power state > to be configured. > > While in theory, the hardware does expose multiple ways/ > combinations in which OSWR can be achieved, we in software > have always been using just *one* combination all along. > OSWR has always been configured as *logic lost, all memory > retained* for all current OMAPs supported. > > With future OMAPs planning to get rid of all the memory/logic > control altogether it makes sense to look at the apis exposed > through the current powerdomain framework to its users (like > cpuilde, suspend etc) and see if those could also be simplified. > > The RFC is suspend tested on OMAP4430sdp and > OMAP3630Beagle-Xm. This looks very similar to the functional power states patches that I submitted a few times for review [1]. The idea is to simplify the external API to program the power domains states. The code has been used to implement the OMAP4 device OFF support (from Tero) and the per-device PM QoS implementation for OMAP [2]. [1] http://marc.info/?l=linux-omap&m=133968580905048&w=2 [2] http://marc.info/?l=linux-omap&m=133968657005566&w=2 Now that we have more than one implementation, I let it to the maintainers for decision. Paul, what do you think? A few remarks here below. Thanks for your RFC! > > regards, > Rajendra > > > From 5f5e4eb342110286bf719c7d9d7c1959f53e34f9 Mon Sep 17 00:00:00 2001 > From: Rajendra Nayak > Date: Thu, 5 Jul 2012 17:33:28 +0530 > Subject: [RFC] ARM: OMAP: Powerdomain: control memory and logic bits > internally > > Powerdomain framework exposes various apis for memory and logic > control for powerdomains, for its users to program OSWR: Open SWitch > Retention state. While in theory, there are various combinations of > memory and logic states possible which can be configured as OSWR, > in practice all OMAPs use just one combination. Logic lost, memory retained. > > This can very easily be handled within the powerdomain framework itself, > without exposing all complex memory/logic control apis to upper layer > drivers like cpuidle and suspend. > > To do this, introduce 2 new power domain states PWRDM_POWER_CSWR and > PWRDM_POWER_OSWR usable by the users of powerdomain framework and > make all memory/logic control apis internal to powerdomain framework. > Change all users of powerdomain framework to get rid of all usage > of memory/logic control apis and use the newly defined states for > CSWR and OSWR with the already used powerstate control apis. > > Some functions (which are now made internal) are forward declared > to avoid moving functions around in the file (which can be done in a > later patch) to help keep the patch reviewable. > > Signed-off-by: Rajendra Nayak > --- > arch/arm/mach-omap2/cpuidle44xx.c|9 +--- > arch/arm/mach-omap2/omap-mpuss-lowpower.c|7 +-- > arch/arm/mach-omap2/pm24xx.c | 17 +-- > arch/arm/mach-omap2/pm34xx.c |8 ++-- > arch/arm/mach-omap2/pm44xx.c | 11 +--- > arch/arm/mach-omap2/powerdomain-private.h| 17 +++ > arch/arm/mach-omap2/powerdomain.c| 54 > +++--- > arch/arm/mach-omap2/powerdomain.h| 14 +- > arch/arm/mach-omap2/powerdomains2xxx_3xxx_data.c |1 + > arch/arm/mach-omap2/powerdomains2xxx_data.c |1 + > arch/arm/mach-omap2/powerdomains3xxx_data.c |1 + > arch/arm/mach-omap2/powerdomains44xx_data.c |1 + > 12 files changed, 85 insertions(+), 56 deletions(-) > create mode 100644 arch/arm/mach-omap2/powerdomain-private.h > ... > diff --git a/arch/arm/mach-omap2/omap-mpuss-lowpower.c > b/arch/arm/mach-omap2/omap-mpuss-lowpower.c > index 13670aa..41d559b 100644 > --- a/arch/arm/mach-omap2/omap-mpuss-lowpower.c > +++ b/arch/arm/mach-omap2/omap-mpuss-lowpower.c ... > @@ -243,7 +243,7 @@ int omap4_enter_lowpower(unsigned int cpu, unsigned int > power_state) > case PWRDM_POWER_OFF: > save_state = 1; > break; > - case PWRDM_POWER_RET: > + case PWRDM_POWER_CSWR: > default: > /* > * CPUx CSWR is invalid hardware state. Also CPUx OSWR > @@ -262,8 +262,7 @@ int omap4_enter_lowpower(unsigned int cpu, unsigned int > power_state) > * In MPUSS OSWR or device OFF, interrupt controller contest is lost. > */ > mpuss_clear_prev_logic_pwrst(); > - if ((pwrdm_read_next_pwrst
Re: RFC: Simplification of Power Domain Control
Just to give more context, the though of doing something like
what the below RFC does came up as part of the patch review of
Jean's "Functional power state" series. There was also some
offline discussion about this between me, Santosh Shilimkar and
Paul Walmsley.
On Thursday 05 July 2012 06:17 PM, Rajendra Nayak wrote:
Hi,
This RFC is aimed at simplifying the powerdomain control
(in software) for OMAP. Powerdomains on OMAP have been
fairly complex to program (as compared to other SoCs)
mainly due to the multiple memory/logic control exposed
to software. These controls are limited to achieving
whats know as 'OSWR: Open Switch Retention' state in OMAP.
Rest of the states supported (ON, INACTIVE, CSWR, OFF) are
fairly easy to program needing just a target power state
to be configured.
While in theory, the hardware does expose multiple ways/
combinations in which OSWR can be achieved, we in software
have always been using just *one* combination all along.
OSWR has always been configured as *logic lost, all memory
retained* for all current OMAPs supported.
With future OMAPs planning to get rid of all the memory/logic
control altogether it makes sense to look at the apis exposed
through the current powerdomain framework to its users (like
cpuilde, suspend etc) and see if those could also be simplified.
The RFC is suspend tested on OMAP4430sdp and
OMAP3630Beagle-Xm.
regards,
Rajendra
From 5f5e4eb342110286bf719c7d9d7c1959f53e34f9 Mon Sep 17 00:00:00 2001
From: Rajendra Nayak
Date: Thu, 5 Jul 2012 17:33:28 +0530
Subject: [RFC] ARM: OMAP: Powerdomain: control memory and logic bits internally
Powerdomain framework exposes various apis for memory and logic
control for powerdomains, for its users to program OSWR: Open SWitch
Retention state. While in theory, there are various combinations of
memory and logic states possible which can be configured as OSWR,
in practice all OMAPs use just one combination. Logic lost, memory retained.
This can very easily be handled within the powerdomain framework itself,
without exposing all complex memory/logic control apis to upper layer
drivers like cpuidle and suspend.
To do this, introduce 2 new power domain states PWRDM_POWER_CSWR and
PWRDM_POWER_OSWR usable by the users of powerdomain framework and
make all memory/logic control apis internal to powerdomain framework.
Change all users of powerdomain framework to get rid of all usage
of memory/logic control apis and use the newly defined states for
CSWR and OSWR with the already used powerstate control apis.
Some functions (which are now made internal) are forward declared
to avoid moving functions around in the file (which can be done in a
later patch) to help keep the patch reviewable.
Signed-off-by: Rajendra Nayak
---
arch/arm/mach-omap2/cpuidle44xx.c|9 +---
arch/arm/mach-omap2/omap-mpuss-lowpower.c|7 +--
arch/arm/mach-omap2/pm24xx.c | 17 +--
arch/arm/mach-omap2/pm34xx.c |8 ++--
arch/arm/mach-omap2/pm44xx.c | 11 +---
arch/arm/mach-omap2/powerdomain-private.h| 17 +++
arch/arm/mach-omap2/powerdomain.c| 54 +++---
arch/arm/mach-omap2/powerdomain.h| 14 +-
arch/arm/mach-omap2/powerdomains2xxx_3xxx_data.c |1 +
arch/arm/mach-omap2/powerdomains2xxx_data.c |1 +
arch/arm/mach-omap2/powerdomains3xxx_data.c |1 +
arch/arm/mach-omap2/powerdomains44xx_data.c |1 +
12 files changed, 85 insertions(+), 56 deletions(-)
create mode 100644 arch/arm/mach-omap2/powerdomain-private.h
diff --git a/arch/arm/mach-omap2/cpuidle44xx.c
b/arch/arm/mach-omap2/cpuidle44xx.c
index be1617c..d40fe90 100644
--- a/arch/arm/mach-omap2/cpuidle44xx.c
+++ b/arch/arm/mach-omap2/cpuidle44xx.c
@@ -27,7 +27,6 @@
/* Machine specific information */
struct omap4_idle_statedata {
u32 cpu_state;
- u32 mpu_logic_state;
u32 mpu_state;
};
@@ -35,17 +34,14 @@ static struct omap4_idle_statedata omap4_idle_data[] = {
{
.cpu_state = PWRDM_POWER_ON,
.mpu_state = PWRDM_POWER_ON,
- .mpu_logic_state = PWRDM_POWER_RET,
},
{
.cpu_state = PWRDM_POWER_OFF,
- .mpu_state = PWRDM_POWER_RET,
- .mpu_logic_state = PWRDM_POWER_RET,
+ .mpu_state = PWRDM_POWER_CSWR,
},
{
.cpu_state = PWRDM_POWER_OFF,
- .mpu_state = PWRDM_POWER_RET,
- .mpu_logic_state = PWRDM_POWER_OFF,
+ .mpu_state = PWRDM_POWER_OSWR,
},
};
@@ -95,7 +91,6 @@ static int omap4_enter_idle(struct cpuidle_device *dev,
if (cx->cpu_state == PWRDM_POWER_OFF)
cpu_pm_enter();
- pwrdm_set_logic_retst(mpu_pd, cx->mpu_logic_state);
omap_set_pwrdm_state(mpu_pd, cx->mpu_state);
/*
diff --git a/arch/arm/mach-omap2/oma
RFC: Simplification of Power Domain Control
Hi,
This RFC is aimed at simplifying the powerdomain control
(in software) for OMAP. Powerdomains on OMAP have been
fairly complex to program (as compared to other SoCs)
mainly due to the multiple memory/logic control exposed
to software. These controls are limited to achieving
whats know as 'OSWR: Open Switch Retention' state in OMAP.
Rest of the states supported (ON, INACTIVE, CSWR, OFF) are
fairly easy to program needing just a target power state
to be configured.
While in theory, the hardware does expose multiple ways/
combinations in which OSWR can be achieved, we in software
have always been using just *one* combination all along.
OSWR has always been configured as *logic lost, all memory
retained* for all current OMAPs supported.
With future OMAPs planning to get rid of all the memory/logic
control altogether it makes sense to look at the apis exposed
through the current powerdomain framework to its users (like
cpuilde, suspend etc) and see if those could also be simplified.
The RFC is suspend tested on OMAP4430sdp and
OMAP3630Beagle-Xm.
regards,
Rajendra
>From 5f5e4eb342110286bf719c7d9d7c1959f53e34f9 Mon Sep 17 00:00:00 2001
From: Rajendra Nayak
Date: Thu, 5 Jul 2012 17:33:28 +0530
Subject: [RFC] ARM: OMAP: Powerdomain: control memory and logic bits internally
Powerdomain framework exposes various apis for memory and logic
control for powerdomains, for its users to program OSWR: Open SWitch
Retention state. While in theory, there are various combinations of
memory and logic states possible which can be configured as OSWR,
in practice all OMAPs use just one combination. Logic lost, memory retained.
This can very easily be handled within the powerdomain framework itself,
without exposing all complex memory/logic control apis to upper layer
drivers like cpuidle and suspend.
To do this, introduce 2 new power domain states PWRDM_POWER_CSWR and
PWRDM_POWER_OSWR usable by the users of powerdomain framework and
make all memory/logic control apis internal to powerdomain framework.
Change all users of powerdomain framework to get rid of all usage
of memory/logic control apis and use the newly defined states for
CSWR and OSWR with the already used powerstate control apis.
Some functions (which are now made internal) are forward declared
to avoid moving functions around in the file (which can be done in a
later patch) to help keep the patch reviewable.
Signed-off-by: Rajendra Nayak
---
arch/arm/mach-omap2/cpuidle44xx.c|9 +---
arch/arm/mach-omap2/omap-mpuss-lowpower.c|7 +--
arch/arm/mach-omap2/pm24xx.c | 17 +--
arch/arm/mach-omap2/pm34xx.c |8 ++--
arch/arm/mach-omap2/pm44xx.c | 11 +---
arch/arm/mach-omap2/powerdomain-private.h| 17 +++
arch/arm/mach-omap2/powerdomain.c| 54 +++---
arch/arm/mach-omap2/powerdomain.h| 14 +-
arch/arm/mach-omap2/powerdomains2xxx_3xxx_data.c |1 +
arch/arm/mach-omap2/powerdomains2xxx_data.c |1 +
arch/arm/mach-omap2/powerdomains3xxx_data.c |1 +
arch/arm/mach-omap2/powerdomains44xx_data.c |1 +
12 files changed, 85 insertions(+), 56 deletions(-)
create mode 100644 arch/arm/mach-omap2/powerdomain-private.h
diff --git a/arch/arm/mach-omap2/cpuidle44xx.c
b/arch/arm/mach-omap2/cpuidle44xx.c
index be1617c..d40fe90 100644
--- a/arch/arm/mach-omap2/cpuidle44xx.c
+++ b/arch/arm/mach-omap2/cpuidle44xx.c
@@ -27,7 +27,6 @@
/* Machine specific information */
struct omap4_idle_statedata {
u32 cpu_state;
- u32 mpu_logic_state;
u32 mpu_state;
};
@@ -35,17 +34,14 @@ static struct omap4_idle_statedata omap4_idle_data[] = {
{
.cpu_state = PWRDM_POWER_ON,
.mpu_state = PWRDM_POWER_ON,
- .mpu_logic_state = PWRDM_POWER_RET,
},
{
.cpu_state = PWRDM_POWER_OFF,
- .mpu_state = PWRDM_POWER_RET,
- .mpu_logic_state = PWRDM_POWER_RET,
+ .mpu_state = PWRDM_POWER_CSWR,
},
{
.cpu_state = PWRDM_POWER_OFF,
- .mpu_state = PWRDM_POWER_RET,
- .mpu_logic_state = PWRDM_POWER_OFF,
+ .mpu_state = PWRDM_POWER_OSWR,
},
};
@@ -95,7 +91,6 @@ static int omap4_enter_idle(struct cpuidle_device *dev,
if (cx->cpu_state == PWRDM_POWER_OFF)
cpu_pm_enter();
- pwrdm_set_logic_retst(mpu_pd, cx->mpu_logic_state);
omap_set_pwrdm_state(mpu_pd, cx->mpu_state);
/*
diff --git a/arch/arm/mach-omap2/omap-mpuss-lowpower.c
b/arch/arm/mach-omap2/omap-mpuss-lowpower.c
index 13670aa..41d559b 100644
--- a/arch/arm/mach-omap2/omap-mpuss-lowpower.c
+++ b/arch/arm/mach-omap2/omap-mpuss-lowpower.c
@@ -125,7 +125,7 @@ static void scu_pwrst_prepare(unsigned int cpu_id, unsigned
int cpu_state)
u32 scu_pwr_st;
switch (cpu_st
