diff mbox

[02/04] OMAP3: PM: Prevent AUTO_RET and AUTO_OFF being enabled simultaneously

Message ID 1245153154-1876-2-git-send-email-rnayak@ti.com (mailing list archive)
State Accepted
Delegated to: Kevin Hilman
Headers show

Commit Message

Rajendra Nayak June 16, 2009, 11:52 a.m. UTC
There is a design requirement in OMAP3 that Auto_RET and AUTO_OFF
should not be set together. The PRCM FSM  has been coded assuming
that SW will set either auto_ret or auto_off bit depending on
whether the core has been programmed to go into open switched
logic retention state or OFF state. They are mutually exclusive.
A similar issue will exist if SW sets  auto_ret= auto_off=1 and
auto_sleep = 1 in the PRM voltage CTRL register

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
---
 arch/arm/mach-omap2/pm34xx.c |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

Comments

Jouni Hogander June 16, 2009, 12:39 p.m. UTC | #1
ext Rajendra Nayak <rnayak@ti.com> writes:

> There is a design requirement in OMAP3 that Auto_RET and AUTO_OFF
> should not be set together. The PRCM FSM  has been coded assuming
> that SW will set either auto_ret or auto_off bit depending on
> whether the core has been programmed to go into open switched
> logic retention state or OFF state. They are mutually exclusive.

So we don't have to do this if closed switch retention is used? (This
is what is currently used in linux-omap:pm)

> A similar issue will exist if SW sets  auto_ret= auto_off=1 and
> auto_sleep = 1 in the PRM voltage CTRL register
>
> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
> ---
>  arch/arm/mach-omap2/pm34xx.c |   12 ++++++++----
>  1 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index a9ef670..d4225b4 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -392,6 +392,10 @@ void omap_sram_idle(void)
>  					     OMAP3_PRM_VOLTCTRL_OFFSET);
>  			omap3_core_save_context();
>  			omap3_prcm_save_context();
> +		} else if (core_next_state == PWRDM_POWER_RET) {
> +			prm_set_mod_reg_bits(OMAP3430_AUTO_RET,
> +						OMAP3430_GR_MOD,
> +						OMAP3_PRM_VOLTCTRL_OFFSET);
>  		}
>  		/* Enable IO-PAD and IO-CHAIN wakeups */
>  		prm_set_mod_reg_bits(OMAP3430_EN_IO, WKUP_MOD, PM_WKEN);
> @@ -446,6 +450,10 @@ void omap_sram_idle(void)
>  			prm_clear_mod_reg_bits(OMAP3430_AUTO_OFF,
>  					       OMAP3430_GR_MOD,
>  					       OMAP3_PRM_VOLTCTRL_OFFSET);
> +		else if (core_next_state == PWRDM_POWER_RET)
> +			prm_clear_mod_reg_bits(OMAP3430_AUTO_RET,
> +						OMAP3430_GR_MOD,
> +						OMAP3_PRM_VOLTCTRL_OFFSET);
>  		/* Enable smartreflex after WFI */
>  		enable_smartreflex(SR1);
>  		enable_smartreflex(SR2);
> @@ -1128,10 +1136,6 @@ static void __init configure_vc(void)
>  				OMAP3430_GR_MOD,
>  				OMAP3_PRM_VC_I2C_CFG_OFFSET);
>  
> -	/* Setup value for voltctrl */
> -	prm_write_mod_reg(OMAP3430_AUTO_RET,
> -			  OMAP3430_GR_MOD, OMAP3_PRM_VOLTCTRL_OFFSET);
> -
>  	/* Write setup times */
>  	prm_write_mod_reg(prm_setup.clksetup, OMAP3430_GR_MOD,
>  			OMAP3_PRM_CLKSETUP_OFFSET);
> -- 
> 1.5.4.7
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rajendra Nayak June 16, 2009, 1:16 p.m. UTC | #2
IA0KDQo+LS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj5Gcm9tOiBIw7ZnYW5kZXIgSm91bmkg
W21haWx0bzpqb3VuaS5ob2dhbmRlckBub2tpYS5jb21dIA0KPlNlbnQ6IFR1ZXNkYXksIEp1bmUg
MTYsIDIwMDkgNjoxMCBQTQ0KPlRvOiBOYXlhaywgUmFqZW5kcmENCj5DYzogbGludXgtb21hcEB2
Z2VyLmtlcm5lbC5vcmc7IERlcnJpY2ssIERhdmlkOyBXb29kcnVmZiwgUmljaGFyZA0KPlN1Ympl
Y3Q6IFJlOiBbUEFUQ0ggMDIvMDRdIE9NQVAzOiBQTTogUHJldmVudCBBVVRPX1JFVCBhbmQgDQo+
QVVUT19PRkYgYmVpbmcgZW5hYmxlZCBzaW11bHRhbmVvdXNseQ0KPg0KPmV4dCBSYWplbmRyYSBO
YXlhayA8cm5heWFrQHRpLmNvbT4gd3JpdGVzOg0KPg0KPj4gVGhlcmUgaXMgYSBkZXNpZ24gcmVx
dWlyZW1lbnQgaW4gT01BUDMgdGhhdCBBdXRvX1JFVCBhbmQgQVVUT19PRkYNCj4+IHNob3VsZCBu
b3QgYmUgc2V0IHRvZ2V0aGVyLiBUaGUgUFJDTSBGU00gIGhhcyBiZWVuIGNvZGVkIGFzc3VtaW5n
DQo+PiB0aGF0IFNXIHdpbGwgc2V0IGVpdGhlciBhdXRvX3JldCBvciBhdXRvX29mZiBiaXQgZGVw
ZW5kaW5nIG9uDQo+PiB3aGV0aGVyIHRoZSBjb3JlIGhhcyBiZWVuIHByb2dyYW1tZWQgdG8gZ28g
aW50byBvcGVuIHN3aXRjaGVkDQo+PiBsb2dpYyByZXRlbnRpb24gc3RhdGUgb3IgT0ZGIHN0YXRl
LiBUaGV5IGFyZSBtdXR1YWxseSBleGNsdXNpdmUuDQo+DQo+U28gd2UgZG9uJ3QgaGF2ZSB0byBk
byB0aGlzIGlmIGNsb3NlZCBzd2l0Y2ggcmV0ZW50aW9uIGlzIHVzZWQ/IChUaGlzDQo+aXMgd2hh
dCBpcyBjdXJyZW50bHkgdXNlZCBpbiBsaW51eC1vbWFwOnBtKQ0KDQpDdXJyZW50bHkgaW4gdGhl
IHBtIGJyYW5jaCBBVVRPX1JFVCBpcyBlbmFibGVkIGF0IGluaXQgYW5kIGtlcHQNCmVuYWJsZWQu
IFdoaWxlIGF0dGVtcHRpbmcgYSBPRkYgc3RhdGUgQVVUT19PRkYgaXMgZW5hYmxlZCBhbHNvDQps
ZWF2aW5nIEFVVE9fUkVUIGFuZCBBVVRPX09GRiBib3RoIGVuYWJsZWQuDQoNCj4NCj4+IEEgc2lt
aWxhciBpc3N1ZSB3aWxsIGV4aXN0IGlmIFNXIHNldHMgIGF1dG9fcmV0PSBhdXRvX29mZj0xIGFu
ZA0KPj4gYXV0b19zbGVlcCA9IDEgaW4gdGhlIFBSTSB2b2x0YWdlIENUUkwgcmVnaXN0ZXINCj4+
DQo+PiBTaWduZWQtb2ZmLWJ5OiBSYWplbmRyYSBOYXlhayA8cm5heWFrQHRpLmNvbT4NCj4+IC0t
LQ0KPj4gIGFyY2gvYXJtL21hY2gtb21hcDIvcG0zNHh4LmMgfCAgIDEyICsrKysrKysrLS0tLQ0K
Pj4gIDEgZmlsZXMgY2hhbmdlZCwgOCBpbnNlcnRpb25zKCspLCA0IGRlbGV0aW9ucygtKQ0KPj4N
Cj4+IGRpZmYgLS1naXQgYS9hcmNoL2FybS9tYWNoLW9tYXAyL3BtMzR4eC5jIA0KPmIvYXJjaC9h
cm0vbWFjaC1vbWFwMi9wbTM0eHguYw0KPj4gaW5kZXggYTllZjY3MC4uZDQyMjViNCAxMDA2NDQN
Cj4+IC0tLSBhL2FyY2gvYXJtL21hY2gtb21hcDIvcG0zNHh4LmMNCj4+ICsrKyBiL2FyY2gvYXJt
L21hY2gtb21hcDIvcG0zNHh4LmMNCj4+IEBAIC0zOTIsNiArMzkyLDEwIEBAIHZvaWQgb21hcF9z
cmFtX2lkbGUodm9pZCkNCj4+ICAJCQkJCSAgICAgT01BUDNfUFJNX1ZPTFRDVFJMX09GRlNFVCk7
DQo+PiAgCQkJb21hcDNfY29yZV9zYXZlX2NvbnRleHQoKTsNCj4+ICAJCQlvbWFwM19wcmNtX3Nh
dmVfY29udGV4dCgpOw0KPj4gKwkJfSBlbHNlIGlmIChjb3JlX25leHRfc3RhdGUgPT0gUFdSRE1f
UE9XRVJfUkVUKSB7DQo+PiArCQkJcHJtX3NldF9tb2RfcmVnX2JpdHMoT01BUDM0MzBfQVVUT19S
RVQsDQo+PiArCQkJCQkJT01BUDM0MzBfR1JfTU9ELA0KPj4gKwkJCQkJCQ0KPk9NQVAzX1BSTV9W
T0xUQ1RSTF9PRkZTRVQpOw0KPj4gIAkJfQ0KPj4gIAkJLyogRW5hYmxlIElPLVBBRCBhbmQgSU8t
Q0hBSU4gd2FrZXVwcyAqLw0KPj4gIAkJcHJtX3NldF9tb2RfcmVnX2JpdHMoT01BUDM0MzBfRU5f
SU8sIFdLVVBfTU9ELCBQTV9XS0VOKTsNCj4+IEBAIC00NDYsNiArNDUwLDEwIEBAIHZvaWQgb21h
cF9zcmFtX2lkbGUodm9pZCkNCj4+ICAJCQlwcm1fY2xlYXJfbW9kX3JlZ19iaXRzKE9NQVAzNDMw
X0FVVE9fT0ZGLA0KPj4gIAkJCQkJICAgICAgIE9NQVAzNDMwX0dSX01PRCwNCj4+ICAJCQkJCSAg
ICAgICANCj5PTUFQM19QUk1fVk9MVENUUkxfT0ZGU0VUKTsNCj4+ICsJCWVsc2UgaWYgKGNvcmVf
bmV4dF9zdGF0ZSA9PSBQV1JETV9QT1dFUl9SRVQpDQo+PiArCQkJcHJtX2NsZWFyX21vZF9yZWdf
Yml0cyhPTUFQMzQzMF9BVVRPX1JFVCwNCj4+ICsJCQkJCQlPTUFQMzQzMF9HUl9NT0QsDQo+PiAr
CQkJCQkJDQo+T01BUDNfUFJNX1ZPTFRDVFJMX09GRlNFVCk7DQo+PiAgCQkvKiBFbmFibGUgc21h
cnRyZWZsZXggYWZ0ZXIgV0ZJICovDQo+PiAgCQllbmFibGVfc21hcnRyZWZsZXgoU1IxKTsNCj4+
ICAJCWVuYWJsZV9zbWFydHJlZmxleChTUjIpOw0KPj4gQEAgLTExMjgsMTAgKzExMzYsNiBAQCBz
dGF0aWMgdm9pZCBfX2luaXQgY29uZmlndXJlX3ZjKHZvaWQpDQo+PiAgCQkJCU9NQVAzNDMwX0dS
X01PRCwNCj4+ICAJCQkJT01BUDNfUFJNX1ZDX0kyQ19DRkdfT0ZGU0VUKTsNCj4+ICANCj4+IC0J
LyogU2V0dXAgdmFsdWUgZm9yIHZvbHRjdHJsICovDQo+PiAtCXBybV93cml0ZV9tb2RfcmVnKE9N
QVAzNDMwX0FVVE9fUkVULA0KPj4gLQkJCSAgT01BUDM0MzBfR1JfTU9ELCBPTUFQM19QUk1fVk9M
VENUUkxfT0ZGU0VUKTsNCj4+IC0NCj4+ICAJLyogV3JpdGUgc2V0dXAgdGltZXMgKi8NCj4+ICAJ
cHJtX3dyaXRlX21vZF9yZWcocHJtX3NldHVwLmNsa3NldHVwLCBPTUFQMzQzMF9HUl9NT0QsDQo+
PiAgCQkJT01BUDNfUFJNX0NMS1NFVFVQX09GRlNFVCk7DQo+PiAtLSANCj4+IDEuNS40LjcNCj4+
DQo+PiAtLQ0KPj4gVG8gdW5zdWJzY3JpYmUgZnJvbSB0aGlzIGxpc3Q6IHNlbmQgdGhlIGxpbmUg
InVuc3Vic2NyaWJlIA0KPmxpbnV4LW9tYXAiIGluDQo+PiB0aGUgYm9keSBvZiBhIG1lc3NhZ2Ug
dG8gbWFqb3Jkb21vQHZnZXIua2VybmVsLm9yZw0KPj4gTW9yZSBtYWpvcmRvbW8gaW5mbyBhdCAg
aHR0cDovL3ZnZXIua2VybmVsLm9yZy9tYWpvcmRvbW8taW5mby5odG1sDQo+DQo+LS0gDQo+Sm91
bmkgSMO2Z2FuZGVyDQo+DQo+
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kevin Hilman June 16, 2009, 2:28 p.m. UTC | #3
"Nayak, Rajendra" <rnayak@ti.com> writes:

>From: Högander Jouni [mailto:jouni.hogander@nokia.com] 
>>
>>ext Rajendra Nayak <rnayak@ti.com> writes:
>>
>>> There is a design requirement in OMAP3 that Auto_RET and AUTO_OFF
>>> should not be set together. The PRCM FSM  has been coded assuming
>>> that SW will set either auto_ret or auto_off bit depending on
>>> whether the core has been programmed to go into open switched
>>> logic retention state or OFF state. They are mutually exclusive.
>>
>>So we don't have to do this if closed switch retention is used? (This
>>is what is currently used in linux-omap:pm)
>
> Currently in the pm branch AUTO_RET is enabled at init and kept
> enabled. While attempting a OFF state AUTO_OFF is enabled also
> leaving AUTO_RET and AUTO_OFF both enabled.

A little more clarification needed, in particular whether or how
this affects closed-switch retention.

The description above states that this problem affects OSWR and OFF.
linux-omap PM only uses CSWR, so based on the description of the PRCM
FSM above, for CSWR, we should never be setting AUTO_RET when using
CSWR.

Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kevin Hilman June 16, 2009, 5:28 p.m. UTC | #4
Hi David,

Thanks for the extra clarifications.  Is this Table you sent in the
TRM somewhere?  I don't find it in my ES3.1 NDA TRM ver. P.

For the various C-states in CPUidle, we do indeed have various
combinations of MPU and CORE state, from cpuidle34xx.c:

/* omap3_init_power_states - Initialises the OMAP3 specific C states.
 *
 * Below is the desciption of each C state.
 * 	C1 . MPU WFI + Core active
 *	C2 . MPU WFI + Core inactive
 *	C3 . MPU CSWR + Core inactive
 *	C4 . MPU OFF + Core inactive
 *	C5 . MPU CSWR + Core CSWR
 *	C6 . MPU OFF + Core CSWR
 *	C7 . MPU OFF + Core OFF
 */

Maybe the right solution is to have the PRM_VOLTCTRL value associated
with the C-state so we minimize the amount of conditional logic we
have to do in idle loop itself.  Otherwise tracking all these
combinations in the idle loop could get ugly.  We also don't currently
do anything with AUTO_SLEEP, and it looks like we should.

But, getting back to the original patch, based on the table you sent,
the description in the original patch makes even less sense to me.  It
says basically that AUTO_RET and AUTO_OFF are mutually exclusive.  The
table suggests otherwise, and shows that you'll never hit 0V unless
both are set.

So I still think the original patch description needs an update.

Kevin

"Derrick, David" <dderrick@ti.com> writes:

> Kevin,
>
>  
>
> Not sure this change has been done yet. But basically when you have mis-matched
> power states between the MPU and CORE, you may run into a situation where the
> voltage does not scale. As long as both MPU and CORE go into the same power
> state then there is not a problem. See the following table to better
> understand:
>
>  
>
>                                                                                                                          
> Table 1.          AUTO Voltage Scaling
>
> +----------------------------------------------------------------------------------------------------+
> |MPU  Power     |CORE Power State |AUTO_OFF    |AUTO_RETENTION    |AUTO_SLEEP    |VDD1    |VDD2      |
> |State          |                 |            |                  |              |        |          |
> |---------------+-----------------+------------+------------------+--------------+--------+----------|
> |      OFF      |       OFF       |     0      |        1         |      1       |  1.0V  |   0.9V   |
> |---------------+-----------------+------------+------------------+--------------+--------+----------|
> |      OFF      |       OFF       |     0      |        1         |      0       |  0.9V  |   0.9V   |
> |---------------+-----------------+------------+------------------+--------------+--------+----------|
> |      OFF      |       OFF       |     1      |        1         |      1       |0V/0.6V*|0V/0.6mV* |
> |---------------+-----------------+------------+------------------+--------------+--------+----------|
> |      OFF      |  OSWR or CSWR   |     0      |        1         |      1       |  1.0V  |   0.9V   |
> |---------------+-----------------+------------+------------------+--------------+--------+----------|
> |      OFF      |  OSWR or CSWR   |     0      |        1         |      0       |  0.9V  |   0.9V   |
> |---------------+-----------------+------------+------------------+--------------+--------+----------|
> |      OFF      |  OSWR or CSWR   |     1      |        1         |      1       |  1.2V  |  1.15V   |
> |---------------+-----------------+------------+------------------+--------------+--------+----------|
> |     CSWR      |  ON (INACTIVE)  |     0      |        1         |      0       |  0.9V  |  1.15V   |
> |---------------+-----------------+------------+------------------+--------------+--------+----------|
> |     CSWR      |  ON (INACTIVE)  |     0      |        1         |      1       |  0.9V  |   1.0V   |
> |---------------+-----------------+------------+------------------+--------------+--------+----------|
> |     CSWR      |  ON (INACTIVE)  |     0      |        0         |      1       |  1.0V  |   1.0V   |
> +----------------------------------------------------------------------------------------------------+
>
> * Using SYS_OFFMODE / Using I2C4
>
>  
>
>  
>
> You need to dynamically enable and disable the auto voltage scaling (AUTO_OFF,
> AUTO_RET and AUTO_SLEEP) based on which sleep configuration you are going into
> for MPU and CORE.
>
>  
>
> Regards,
>
> David
>
>  
>
> -----Original Message-----
> From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
> Sent: Tuesday, June 16, 2009 9:28 AM
> To: Nayak, Rajendra
> Cc: Högander Jouni; linux-omap@vger.kernel.org; Derrick, David; Woodruff,
> Richard
> Subject: Re: [PATCH 02/04] OMAP3: PM: Prevent AUTO_RET and AUTO_OFF being
> enabled simultaneously
>
>  
>
> "Nayak, Rajendra" <rnayak@ti.com> writes:
>
>  
>
>>From: Högander Jouni [mailto:jouni.hogander@nokia.com]
>
>>> 
>
>>>ext Rajendra Nayak <rnayak@ti.com> writes:
>
>>> 
>
>>>> There is a design requirement in OMAP3 that Auto_RET and AUTO_OFF
>
>>>> should not be set together. The PRCM FSM  has been coded assuming
>
>>>> that SW will set either auto_ret or auto_off bit depending on
>
>>>> whether the core has been programmed to go into open switched
>
>>>> logic retention state or OFF state. They are mutually exclusive.
>
>>> 
>
>>>So we don't have to do this if closed switch retention is used? (This
>
>>>is what is currently used in linux-omap:pm)
>
>> 
>
>> Currently in the pm branch AUTO_RET is enabled at init and kept
>
>> enabled. While attempting a OFF state AUTO_OFF is enabled also
>
>> leaving AUTO_RET and AUTO_OFF both enabled.
>
>  
>
> A little more clarification needed, in particular whether or how
>
> this affects closed-switch retention.
>
>  
>
> The description above states that this problem affects OSWR and OFF.
>
> linux-omap PM only uses CSWR, so based on the description of the PRCM
>
> FSM above, for CSWR, we should never be setting AUTO_RET when using
>
> CSWR.
>
>  
>
> Kevin
>
>  
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rajendra Nayak June 17, 2009, 6:27 a.m. UTC | #5
>-----Original Message-----
>From: Kevin Hilman [mailto:khilman@deeprootsystems.com] 
>Sent: Tuesday, June 16, 2009 7:58 PM
>To: Nayak, Rajendra
>Cc: Högander Jouni; linux-omap@vger.kernel.org; Derrick, 
>David; Woodruff, Richard
>Subject: Re: [PATCH 02/04] OMAP3: PM: Prevent AUTO_RET and 
>AUTO_OFF being enabled simultaneously
>
>"Nayak, Rajendra" <rnayak@ti.com> writes:
>
>>From: Högander Jouni [mailto:jouni.hogander@nokia.com] 
>>>
>>>ext Rajendra Nayak <rnayak@ti.com> writes:
>>>
>>>> There is a design requirement in OMAP3 that Auto_RET and AUTO_OFF
>>>> should not be set together. The PRCM FSM  has been coded assuming
>>>> that SW will set either auto_ret or auto_off bit depending on
>>>> whether the core has been programmed to go into open switched
>>>> logic retention state or OFF state. They are mutually exclusive.
>>>
>>>So we don't have to do this if closed switch retention is used? (This
>>>is what is currently used in linux-omap:pm)
>>
>> Currently in the pm branch AUTO_RET is enabled at init and kept
>> enabled. While attempting a OFF state AUTO_OFF is enabled also
>> leaving AUTO_RET and AUTO_OFF both enabled.
>
>A little more clarification needed, in particular whether or how
>this affects closed-switch retention.

Sorry, that was a wrong description. AUTO_RET could be used to scale the voltage
down even in case of CSWR and not just OSWR.

The idea of this patch was to enable auto voltage scaling only for the last 3 C states.

 *	C5 . MPU CSWR + Core CSWR - AUTO RET enabled
 *	C6 . MPU OFF + Core CSWR - AUTO RET enabled
 *	C7 . MPU OFF + Core OFF - AUTO OFF enabled.

For the rest of the Lower C states the decision of not enabling AUTO RET/OFF was to keep
the latency for such states down.

Especially with SR enabled having AUTO RET/OFF enabled for say  a MPU RET/CORE inactive
state would mean a 2 level voltage change. First to scale from SR autocompensated level to the
actual OPP voltage level (That's done when you disable SR) and then from the there to the
AUTO RET level.
So say you hit MPU RET/CORE inactive state at OPP3 (1.2v) which has a SR autocompensated voltage
to 1.08v, then you would scale from 1.08->1.2->0.975.
That might be too much of additional latency to incure in case of a MPU RET/CORE inactive
state. 

>
>The description above states that this problem affects OSWR and OFF.
>linux-omap PM only uses CSWR, so based on the description of the PRCM
>FSM above, for CSWR, we should never be setting AUTO_RET when using
>CSWR.
>
>Kevin
>
>--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Derrick, David June 17, 2009, 4:52 p.m. UTC | #6
Hi Kevin,

Setting auto voltage scaling with C-states is good idea.

AUTO_SLEEP is when you idle with power state set to "ON" - otherwise known as INACTIVE.

Sorry for the confusion on the table. I did not show all combinations. You will get 0V in OFF mode with just AUTO_OFF set, the other 2 (AUTO_RET and AUTO_SLEEP don't have to be set but do not hurt if they are set). My point was to show that when MPU and CORE do not have the same power state settings then you need to pay attention to the AUTO voltage settings otherwise you may not get voltage scaling. If MPU and CORE have the same power state then you are OK setting all three (AUTO_OFF, AUTO_RET and AUTO_SLEEP). 

Regards,
David

-----Original Message-----
From: Kevin Hilman [mailto:khilman@deeprootsystems.com] 
Sent: Tuesday, June 16, 2009 12:28 PM
To: Derrick, David
Cc: Nayak, Rajendra; Högander Jouni; linux-omap@vger.kernel.org; Woodruff, Richard
Subject: Re: [PATCH 02/04] OMAP3: PM: Prevent AUTO_RET and AUTO_OFF being enabled simultaneously

Hi David,

Thanks for the extra clarifications.  Is this Table you sent in the
TRM somewhere?  I don't find it in my ES3.1 NDA TRM ver. P.

For the various C-states in CPUidle, we do indeed have various
combinations of MPU and CORE state, from cpuidle34xx.c:

/* omap3_init_power_states - Initialises the OMAP3 specific C states.
 *
 * Below is the desciption of each C state.
 * 	C1 . MPU WFI + Core active
 *	C2 . MPU WFI + Core inactive
 *	C3 . MPU CSWR + Core inactive
 *	C4 . MPU OFF + Core inactive
 *	C5 . MPU CSWR + Core CSWR
 *	C6 . MPU OFF + Core CSWR
 *	C7 . MPU OFF + Core OFF
 */

Maybe the right solution is to have the PRM_VOLTCTRL value associated
with the C-state so we minimize the amount of conditional logic we
have to do in idle loop itself.  Otherwise tracking all these
combinations in the idle loop could get ugly.  We also don't currently
do anything with AUTO_SLEEP, and it looks like we should.

But, getting back to the original patch, based on the table you sent,
the description in the original patch makes even less sense to me.  It
says basically that AUTO_RET and AUTO_OFF are mutually exclusive.  The
table suggests otherwise, and shows that you'll never hit 0V unless
both are set.

So I still think the original patch description needs an update.

Kevin

"Derrick, David" <dderrick@ti.com> writes:

> Kevin,
>
>  
>
> Not sure this change has been done yet. But basically when you have mis-matched
> power states between the MPU and CORE, you may run into a situation where the
> voltage does not scale. As long as both MPU and CORE go into the same power
> state then there is not a problem. See the following table to better
> understand:
>
>  
>
>                                                                                                                          
> Table 1.          AUTO Voltage Scaling
>
> +----------------------------------------------------------------------------------------------------+
> |MPU  Power     |CORE Power State |AUTO_OFF    |AUTO_RETENTION    |AUTO_SLEEP    |VDD1    |VDD2      |
> |State          |                 |            |                  |              |        |          |
> |---------------+-----------------+------------+------------------+--------------+--------+----------|
> |      OFF      |       OFF       |     0      |        1         |      1       |  1.0V  |   0.9V   |
> |---------------+-----------------+------------+------------------+--------------+--------+----------|
> |      OFF      |       OFF       |     0      |        1         |      0       |  0.9V  |   0.9V   |
> |---------------+-----------------+------------+------------------+--------------+--------+----------|
> |      OFF      |       OFF       |     1      |        1         |      1       |0V/0.6V*|0V/0.6mV* |
> |---------------+-----------------+------------+------------------+--------------+--------+----------|
> |      OFF      |  OSWR or CSWR   |     0      |        1         |      1       |  1.0V  |   0.9V   |
> |---------------+-----------------+------------+------------------+--------------+--------+----------|
> |      OFF      |  OSWR or CSWR   |     0      |        1         |      0       |  0.9V  |   0.9V   |
> |---------------+-----------------+------------+------------------+--------------+--------+----------|
> |      OFF      |  OSWR or CSWR   |     1      |        1         |      1       |  1.2V  |  1.15V   |
> |---------------+-----------------+------------+------------------+--------------+--------+----------|
> |     CSWR      |  ON (INACTIVE)  |     0      |        1         |      0       |  0.9V  |  1.15V   |
> |---------------+-----------------+------------+------------------+--------------+--------+----------|
> |     CSWR      |  ON (INACTIVE)  |     0      |        1         |      1       |  0.9V  |   1.0V   |
> |---------------+-----------------+------------+------------------+--------------+--------+----------|
> |     CSWR      |  ON (INACTIVE)  |     0      |        0         |      1       |  1.0V  |   1.0V   |
> +----------------------------------------------------------------------------------------------------+
>
> * Using SYS_OFFMODE / Using I2C4
>
>  
>
>  
>
> You need to dynamically enable and disable the auto voltage scaling (AUTO_OFF,
> AUTO_RET and AUTO_SLEEP) based on which sleep configuration you are going into
> for MPU and CORE.
>
>  
>
> Regards,
>
> David
>
>  
>
> -----Original Message-----
> From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
> Sent: Tuesday, June 16, 2009 9:28 AM
> To: Nayak, Rajendra
> Cc: Högander Jouni; linux-omap@vger.kernel.org; Derrick, David; Woodruff,
> Richard
> Subject: Re: [PATCH 02/04] OMAP3: PM: Prevent AUTO_RET and AUTO_OFF being
> enabled simultaneously
>
>  
>
> "Nayak, Rajendra" <rnayak@ti.com> writes:
>
>  
>
>>From: Högander Jouni [mailto:jouni.hogander@nokia.com]
>
>>> 
>
>>>ext Rajendra Nayak <rnayak@ti.com> writes:
>
>>> 
>
>>>> There is a design requirement in OMAP3 that Auto_RET and AUTO_OFF
>
>>>> should not be set together. The PRCM FSM  has been coded assuming
>
>>>> that SW will set either auto_ret or auto_off bit depending on
>
>>>> whether the core has been programmed to go into open switched
>
>>>> logic retention state or OFF state. They are mutually exclusive.
>
>>> 
>
>>>So we don't have to do this if closed switch retention is used? (This
>
>>>is what is currently used in linux-omap:pm)
>
>> 
>
>> Currently in the pm branch AUTO_RET is enabled at init and kept
>
>> enabled. While attempting a OFF state AUTO_OFF is enabled also
>
>> leaving AUTO_RET and AUTO_OFF both enabled.
>
>  
>
> A little more clarification needed, in particular whether or how
>
> this affects closed-switch retention.
>
>  
>
> The description above states that this problem affects OSWR and OFF.
>
> linux-omap PM only uses CSWR, so based on the description of the PRCM
>
> FSM above, for CSWR, we should never be setting AUTO_RET when using
>
> CSWR.
>
>  
>
> Kevin
>
>  

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kevin Hilman June 29, 2009, 9:45 p.m. UTC | #7
"Nayak, Rajendra" <rnayak@ti.com> writes:

>>"Nayak, Rajendra" <rnayak@ti.com> writes:
>>
>>>From: Högander Jouni [mailto:jouni.hogander@nokia.com] 
>>>>
>>>>ext Rajendra Nayak <rnayak@ti.com> writes:
>>>>
>>>>> There is a design requirement in OMAP3 that Auto_RET and AUTO_OFF
>>>>> should not be set together. The PRCM FSM  has been coded assuming
>>>>> that SW will set either auto_ret or auto_off bit depending on
>>>>> whether the core has been programmed to go into open switched
>>>>> logic retention state or OFF state. They are mutually exclusive.
>>>>
>>>>So we don't have to do this if closed switch retention is used? (This
>>>>is what is currently used in linux-omap:pm)
>>>
>>> Currently in the pm branch AUTO_RET is enabled at init and kept
>>> enabled. While attempting a OFF state AUTO_OFF is enabled also
>>> leaving AUTO_RET and AUTO_OFF both enabled.
>>
>>A little more clarification needed, in particular whether or how
>>this affects closed-switch retention.
>
> Sorry, that was a wrong description. AUTO_RET could be used to scale the voltage
> down even in case of CSWR and not just OSWR.
>
> The idea of this patch was to enable auto voltage scaling only for the last 3 C states.
>
>  *	C5 . MPU CSWR + Core CSWR - AUTO RET enabled
>  *	C6 . MPU OFF + Core CSWR - AUTO RET enabled
>  *	C7 . MPU OFF + Core OFF - AUTO OFF enabled.
>
> For the rest of the Lower C states the decision of not enabling AUTO
> RET/OFF was to keep the latency for such states down.
>
> Especially with SR enabled having AUTO RET/OFF enabled for say a MPU
> RET/CORE inactive state would mean a 2 level voltage change. First
> to scale from SR autocompensated level to the actual OPP voltage
> level (That's done when you disable SR) and then from the there to
> the AUTO RET level.  So say you hit MPU RET/CORE inactive state at
> OPP3 (1.2v) which has a SR autocompensated voltage to 1.08v, then
> you would scale from 1.08->1.2->0.975.  That might be too much of
> additional latency to incure in case of a MPU RET/CORE inactive
> state.
>

OK, I've updated the patch description on this one and will push to PM
branch (and pm-2.6.29)

Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index a9ef670..d4225b4 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -392,6 +392,10 @@  void omap_sram_idle(void)
 					     OMAP3_PRM_VOLTCTRL_OFFSET);
 			omap3_core_save_context();
 			omap3_prcm_save_context();
+		} else if (core_next_state == PWRDM_POWER_RET) {
+			prm_set_mod_reg_bits(OMAP3430_AUTO_RET,
+						OMAP3430_GR_MOD,
+						OMAP3_PRM_VOLTCTRL_OFFSET);
 		}
 		/* Enable IO-PAD and IO-CHAIN wakeups */
 		prm_set_mod_reg_bits(OMAP3430_EN_IO, WKUP_MOD, PM_WKEN);
@@ -446,6 +450,10 @@  void omap_sram_idle(void)
 			prm_clear_mod_reg_bits(OMAP3430_AUTO_OFF,
 					       OMAP3430_GR_MOD,
 					       OMAP3_PRM_VOLTCTRL_OFFSET);
+		else if (core_next_state == PWRDM_POWER_RET)
+			prm_clear_mod_reg_bits(OMAP3430_AUTO_RET,
+						OMAP3430_GR_MOD,
+						OMAP3_PRM_VOLTCTRL_OFFSET);
 		/* Enable smartreflex after WFI */
 		enable_smartreflex(SR1);
 		enable_smartreflex(SR2);
@@ -1128,10 +1136,6 @@  static void __init configure_vc(void)
 				OMAP3430_GR_MOD,
 				OMAP3_PRM_VC_I2C_CFG_OFFSET);
 
-	/* Setup value for voltctrl */
-	prm_write_mod_reg(OMAP3430_AUTO_RET,
-			  OMAP3430_GR_MOD, OMAP3_PRM_VOLTCTRL_OFFSET);
-
 	/* Write setup times */
 	prm_write_mod_reg(prm_setup.clksetup, OMAP3430_GR_MOD,
 			OMAP3_PRM_CLKSETUP_OFFSET);