Re: [PATCHv2] OMAP3: CPUIdle: prevent CORE from going off if doing so would reset an active clockdomain

2011-03-07 Thread Kevin Hilman 
Hi Tero,

Tero Kristo tero.kri...@nokia.com writes:

 On OMAP3 SoCs, if the CORE powerdomain enters off-mode, many other
 parts of the chip will be reset.  If those parts of the chip are busy,
 the reset will disrupt them, causing unpredictable and generally
 undesirable results. This reset is caused by the core domain wakeup
 (COREDOMAINWKUP_RST), and it can occur for example in a case where
 some peripheral domain is in retention or inactive state and core
 enters off. This will result in the peripheral domain being reset.

 Signed-off-by: Tero Kristo tero.kri...@nokia.com
 Cc: Paul Walmsley p...@pwsan.com
 Cc: Kevin Hilman khil...@deeprootsystems.com
 Cc: Santosh Shilimkar santosh.shilim...@ti.com
 Cc: Vishwanath Sripathy vishwanath...@ti.com
 ---

Turns out this one suffers from same problem as the the other DSS one
because of sleepdeps.  

In today's linux-omap because of sleepdeps, at least DSS will always
report ON during this check, and I think USB does as well.  The result
is that core powerdomain will never hit off-mode.

I'm going to drop this one from my 2.6.39 queue for now as well until
we have a cleaner way to check powerdomain idle status.

Kevin

  arch/arm/mach-omap2/cpuidle34xx.c |   40 +++-
  1 files changed, 38 insertions(+), 2 deletions(-)

 diff --git a/arch/arm/mach-omap2/cpuidle34xx.c 
 b/arch/arm/mach-omap2/cpuidle34xx.c
 index f3e043f..d31b858 100644
 --- a/arch/arm/mach-omap2/cpuidle34xx.c
 +++ b/arch/arm/mach-omap2/cpuidle34xx.c
 @@ -61,7 +61,7 @@ struct omap3_processor_cx {
  struct omap3_processor_cx omap3_power_states[OMAP3_MAX_STATES];
  struct omap3_processor_cx current_cx_state;
  struct powerdomain *mpu_pd, *core_pd, *per_pd;
 -struct powerdomain *cam_pd;
 +struct powerdomain *cam_pd, *dss_pd, *iva2_pd, *sgx_pd, *usb_pd;
  
  /*
   * The latencies/thresholds for various C states have
 @@ -235,7 +235,7 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,
  {
   struct cpuidle_state *new_state = next_valid_state(dev, state);
   u32 core_next_state, per_next_state = 0, per_saved_state = 0;
 - u32 cam_state;
 + u32 cam_state, dss_state, iva2_state, sgx_state, usb_state;
   struct omap3_processor_cx *cx;
   int ret;
  
 @@ -256,6 +256,8 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,
*its own code.
*/
  
 + /* XXX Add CORE-active check here */
 +
   /*
* Prevent idle completely if CAM is active.
* CAM does not have wakeup capability in OMAP3.
 @@ -275,6 +277,36 @@ static int omap3_enter_idle_bm(struct cpuidle_device 
 *dev,
   (core_next_state  PWRDM_POWER_RET))
   per_next_state = PWRDM_POWER_RET;
  
 + /* XXX Add prevent-PER-off check here */
 +
 + /*
 +  * If we are attempting CORE off, check if any other powerdomains
 +  * are at retention or higher. CORE off causes chipwide reset which
 +  * would reset these domains also.
 +  */
 + if (core_next_state == PWRDM_POWER_OFF) {
 + iva2_state = pwrdm_read_pwrst(iva2_pd);
 + sgx_state = pwrdm_read_pwrst(sgx_pd);
 + usb_state = pwrdm_read_pwrst(usb_pd);
 + dss_state = pwrdm_read_pwrst(dss_pd);

Because of existing sleepdeps, some of these may be still on during the
idle path  (especially DSS and USB in mainline kernels.)

 + if (cam_state  PWRDM_POWER_OFF ||
 + dss_state  PWRDM_POWER_OFF ||
 + iva2_state  PWRDM_POWER_OFF ||
 + per_next_state  PWRDM_POWER_OFF ||
 + sgx_state  PWRDM_POWER_OFF ||
 + usb_state  PWRDM_POWER_OFF)
 + core_next_state = PWRDM_POWER_RET;
 + }
 +
 + /* Fallback to new target core/mpu state */
 + while (cx-core_state  core_next_state) {
 + state--;
 + cx = cpuidle_get_statedata(state);
 + }
 +
 + new_state = state;
 +
   /* Are we changing PER target state? */
   if (per_next_state != per_saved_state)
   pwrdm_set_next_pwrst(per_pd, per_next_state);
 @@ -489,6 +521,10 @@ int __init omap3_idle_init(void)
   core_pd = pwrdm_lookup(core_pwrdm);
   per_pd = pwrdm_lookup(per_pwrdm);
   cam_pd = pwrdm_lookup(cam_pwrdm);
 + dss_pd = pwrdm_lookup(dss_pwrdm);
 + iva2_pd = pwrdm_lookup(iva2_pwrdm);
 + sgx_pd = pwrdm_lookup(sgx_pwrdm);
 + usb_pd = pwrdm_lookup(usbhost_pwrdm);
  
   omap_init_power_states();
   cpuidle_register_driver(omap3_idle_driver);
--
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: [PATCHv2] OMAP3: CPUIdle: prevent CORE from going off if doing so would reset an active clockdomain

2011-03-03 Thread Kevin Hilman 
Hi Tero,

On Fri, 2011-01-21 at 12:37 +0200, Tero Kristo wrote:
 On OMAP3 SoCs, if the CORE powerdomain enters off-mode, many other
 parts of the chip will be reset.  If those parts of the chip are busy,
 the reset will disrupt them, causing unpredictable and generally
 undesirable results. This reset is caused by the core domain wakeup
 (COREDOMAINWKUP_RST), and it can occur for example in a case where
 some peripheral domain is in retention or inactive state and core
 enters off. This will result in the peripheral domain being reset.

Found a minor bug here...

 Signed-off-by: Tero Kristo tero.kri...@nokia.com
 Cc: Paul Walmsley p...@pwsan.com
 Cc: Kevin Hilman khil...@deeprootsystems.com
 Cc: Santosh Shilimkar santosh.shilim...@ti.com
 Cc: Vishwanath Sripathy vishwanath...@ti.com
 ---
  arch/arm/mach-omap2/cpuidle34xx.c |   40 +++-
  1 files changed, 38 insertions(+), 2 deletions(-)
 
 diff --git a/arch/arm/mach-omap2/cpuidle34xx.c 
 b/arch/arm/mach-omap2/cpuidle34xx.c
 index f3e043f..d31b858 100644
 --- a/arch/arm/mach-omap2/cpuidle34xx.c
 +++ b/arch/arm/mach-omap2/cpuidle34xx.c
 @@ -61,7 +61,7 @@ struct omap3_processor_cx {
  struct omap3_processor_cx omap3_power_states[OMAP3_MAX_STATES];
  struct omap3_processor_cx current_cx_state;
  struct powerdomain *mpu_pd, *core_pd, *per_pd;
 -struct powerdomain *cam_pd;
 +struct powerdomain *cam_pd, *dss_pd, *iva2_pd, *sgx_pd, *usb_pd;
  
  /*
   * The latencies/thresholds for various C states have
 @@ -235,7 +235,7 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,
  {
   struct cpuidle_state *new_state = next_valid_state(dev, state);
   u32 core_next_state, per_next_state = 0, per_saved_state = 0;
 - u32 cam_state;
 + u32 cam_state, dss_state, iva2_state, sgx_state, usb_state;
   struct omap3_processor_cx *cx;
   int ret;
  
 @@ -256,6 +256,8 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,
*its own code.
*/
  
 + /* XXX Add CORE-active check here */
 +
   /*
* Prevent idle completely if CAM is active.
* CAM does not have wakeup capability in OMAP3.
 @@ -275,6 +277,36 @@ static int omap3_enter_idle_bm(struct cpuidle_device 
 *dev,
   (core_next_state  PWRDM_POWER_RET))
   per_next_state = PWRDM_POWER_RET;
  
 + /* XXX Add prevent-PER-off check here */
 +
 + /*
 +  * If we are attempting CORE off, check if any other powerdomains
 +  * are at retention or higher. CORE off causes chipwide reset which
 +  * would reset these domains also.
 +  */
 + if (core_next_state == PWRDM_POWER_OFF) {
 + iva2_state = pwrdm_read_pwrst(iva2_pd);
 + sgx_state = pwrdm_read_pwrst(sgx_pd);
 + usb_state = pwrdm_read_pwrst(usb_pd);
 + dss_state = pwrdm_read_pwrst(dss_pd);
 +
 + if (cam_state  PWRDM_POWER_OFF ||
 + dss_state  PWRDM_POWER_OFF ||
 + iva2_state  PWRDM_POWER_OFF ||
 + per_next_state  PWRDM_POWER_OFF ||
 + sgx_state  PWRDM_POWER_OFF ||
 + usb_state  PWRDM_POWER_OFF)
 + core_next_state = PWRDM_POWER_RET;
 + }
 +
 + /* Fallback to new target core/mpu state */
 + while (cx-core_state  core_next_state) {
 + state--;

This can potentially pick a state that has its 'valid' flag set to zero,
for example, a state with MPU/CORE set to off-mode, but
enable_off_mode=0.

The fix is to also add a check for cx-valid in the while test.  Will
fold that fix into original patch and queue for 2.6.39.

Kevin

 + cx = cpuidle_get_statedata(state);
 + }
 +
 + new_state = state;
 +
   /* Are we changing PER target state? */
   if (per_next_state != per_saved_state)
   pwrdm_set_next_pwrst(per_pd, per_next_state);
 @@ -489,6 +521,10 @@ int __init omap3_idle_init(void)
   core_pd = pwrdm_lookup(core_pwrdm);
   per_pd = pwrdm_lookup(per_pwrdm);
   cam_pd = pwrdm_lookup(cam_pwrdm);
 + dss_pd = pwrdm_lookup(dss_pwrdm);
 + iva2_pd = pwrdm_lookup(iva2_pwrdm);
 + sgx_pd = pwrdm_lookup(sgx_pwrdm);
 + usb_pd = pwrdm_lookup(usbhost_pwrdm);
  
   omap_init_power_states();
   cpuidle_register_driver(omap3_idle_driver);


--
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

[PATCHv2] OMAP3: CPUIdle: prevent CORE from going off if doing so would reset an active clockdomain

2011-01-21 Thread Tero Kristo 
On OMAP3 SoCs, if the CORE powerdomain enters off-mode, many other
parts of the chip will be reset.  If those parts of the chip are busy,
the reset will disrupt them, causing unpredictable and generally
undesirable results. This reset is caused by the core domain wakeup
(COREDOMAINWKUP_RST), and it can occur for example in a case where
some peripheral domain is in retention or inactive state and core
enters off. This will result in the peripheral domain being reset.

Signed-off-by: Tero Kristo tero.kri...@nokia.com
Cc: Paul Walmsley p...@pwsan.com
Cc: Kevin Hilman khil...@deeprootsystems.com
Cc: Santosh Shilimkar santosh.shilim...@ti.com
Cc: Vishwanath Sripathy vishwanath...@ti.com
---
 arch/arm/mach-omap2/cpuidle34xx.c |   40 +++-
 1 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/cpuidle34xx.c 
b/arch/arm/mach-omap2/cpuidle34xx.c
index f3e043f..d31b858 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -61,7 +61,7 @@ struct omap3_processor_cx {
 struct omap3_processor_cx omap3_power_states[OMAP3_MAX_STATES];
 struct omap3_processor_cx current_cx_state;
 struct powerdomain *mpu_pd, *core_pd, *per_pd;
-struct powerdomain *cam_pd;
+struct powerdomain *cam_pd, *dss_pd, *iva2_pd, *sgx_pd, *usb_pd;
 
 /*
  * The latencies/thresholds for various C states have
@@ -235,7 +235,7 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,
 {
struct cpuidle_state *new_state = next_valid_state(dev, state);
u32 core_next_state, per_next_state = 0, per_saved_state = 0;
-   u32 cam_state;
+   u32 cam_state, dss_state, iva2_state, sgx_state, usb_state;
struct omap3_processor_cx *cx;
int ret;
 
@@ -256,6 +256,8 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,
 *its own code.
 */
 
+   /* XXX Add CORE-active check here */
+
/*
 * Prevent idle completely if CAM is active.
 * CAM does not have wakeup capability in OMAP3.
@@ -275,6 +277,36 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,
(core_next_state  PWRDM_POWER_RET))
per_next_state = PWRDM_POWER_RET;
 
+   /* XXX Add prevent-PER-off check here */
+
+   /*
+* If we are attempting CORE off, check if any other powerdomains
+* are at retention or higher. CORE off causes chipwide reset which
+* would reset these domains also.
+*/
+   if (core_next_state == PWRDM_POWER_OFF) {
+   iva2_state = pwrdm_read_pwrst(iva2_pd);
+   sgx_state = pwrdm_read_pwrst(sgx_pd);
+   usb_state = pwrdm_read_pwrst(usb_pd);
+   dss_state = pwrdm_read_pwrst(dss_pd);
+
+   if (cam_state  PWRDM_POWER_OFF ||
+   dss_state  PWRDM_POWER_OFF ||
+   iva2_state  PWRDM_POWER_OFF ||
+   per_next_state  PWRDM_POWER_OFF ||
+   sgx_state  PWRDM_POWER_OFF ||
+   usb_state  PWRDM_POWER_OFF)
+   core_next_state = PWRDM_POWER_RET;
+   }
+
+   /* Fallback to new target core/mpu state */
+   while (cx-core_state  core_next_state) {
+   state--;
+   cx = cpuidle_get_statedata(state);
+   }
+
+   new_state = state;
+
/* Are we changing PER target state? */
if (per_next_state != per_saved_state)
pwrdm_set_next_pwrst(per_pd, per_next_state);
@@ -489,6 +521,10 @@ int __init omap3_idle_init(void)
core_pd = pwrdm_lookup(core_pwrdm);
per_pd = pwrdm_lookup(per_pwrdm);
cam_pd = pwrdm_lookup(cam_pwrdm);
+   dss_pd = pwrdm_lookup(dss_pwrdm);
+   iva2_pd = pwrdm_lookup(iva2_pwrdm);
+   sgx_pd = pwrdm_lookup(sgx_pwrdm);
+   usb_pd = pwrdm_lookup(usbhost_pwrdm);
 
omap_init_power_states();
cpuidle_register_driver(omap3_idle_driver);
-- 
1.7.1

--
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: [PATCHv2] OMAP3: CPUIdle: prevent CORE from going off if doing so would reset an active clockdomain

2011-01-21 Thread Santosh Shilimkar 
 -Original Message-
 From: Tero Kristo [mailto:tero.kri...@nokia.com]
 Sent: Friday, January 21, 2011 4:07 PM
 To: linux-omap@vger.kernel.org
 Cc: Paul Walmsley; Kevin Hilman; Santosh Shilimkar; Vishwanath
 Sripathy
 Subject: [PATCHv2] OMAP3: CPUIdle: prevent CORE from going off if
 doing so would reset an active clockdomain

 On OMAP3 SoCs, if the CORE powerdomain enters off-mode, many other
 parts of the chip will be reset.  If those parts of the chip are
 busy,
 the reset will disrupt them, causing unpredictable and generally
 undesirable results. This reset is caused by the core domain wakeup
 (COREDOMAINWKUP_RST), and it can occur for example in a case where
 some peripheral domain is in retention or inactive state and core
 enters off. This will result in the peripheral domain being reset.

 Signed-off-by: Tero Kristo tero.kri...@nokia.com
 Cc: Paul Walmsley p...@pwsan.com
 Cc: Kevin Hilman khil...@deeprootsystems.com
 Cc: Santosh Shilimkar santosh.shilim...@ti.com
 Cc: Vishwanath Sripathy vishwanath...@ti.com
 ---
Thanks for update Tero.
If you like,
Acked-by: Santosh Shilimkar santosh.shilim...@ti.com
--
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: [PATCHv2] OMAP3: CPUIdle: prevent CORE from going off if doing so would reset an active clockdomain

2011-01-21 Thread Kevin Hilman 
Tero Kristo tero.kri...@nokia.com writes:

 On OMAP3 SoCs, if the CORE powerdomain enters off-mode, many other
 parts of the chip will be reset.  If those parts of the chip are busy,
 the reset will disrupt them, causing unpredictable and generally
 undesirable results. This reset is caused by the core domain wakeup
 (COREDOMAINWKUP_RST), and it can occur for example in a case where
 some peripheral domain is in retention or inactive state and core
 enters off. This will result in the peripheral domain being reset.

 Signed-off-by: Tero Kristo tero.kri...@nokia.com
 Cc: Paul Walmsley p...@pwsan.com
 Cc: Kevin Hilman khil...@deeprootsystems.com
 Cc: Santosh Shilimkar santosh.shilim...@ti.com
 Cc: Vishwanath Sripathy vishwanath...@ti.com

Thanks Tero.

Will add Paul's reviewed-by and Santosh's ack and queue for 2.6.39 in my
pm-next branch.

Kevin

 ---
  arch/arm/mach-omap2/cpuidle34xx.c |   40 +++-
  1 files changed, 38 insertions(+), 2 deletions(-)

 diff --git a/arch/arm/mach-omap2/cpuidle34xx.c 
 b/arch/arm/mach-omap2/cpuidle34xx.c
 index f3e043f..d31b858 100644
 --- a/arch/arm/mach-omap2/cpuidle34xx.c
 +++ b/arch/arm/mach-omap2/cpuidle34xx.c
 @@ -61,7 +61,7 @@ struct omap3_processor_cx {
  struct omap3_processor_cx omap3_power_states[OMAP3_MAX_STATES];
  struct omap3_processor_cx current_cx_state;
  struct powerdomain *mpu_pd, *core_pd, *per_pd;
 -struct powerdomain *cam_pd;
 +struct powerdomain *cam_pd, *dss_pd, *iva2_pd, *sgx_pd, *usb_pd;
  
  /*
   * The latencies/thresholds for various C states have
 @@ -235,7 +235,7 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,
  {
   struct cpuidle_state *new_state = next_valid_state(dev, state);
   u32 core_next_state, per_next_state = 0, per_saved_state = 0;
 - u32 cam_state;
 + u32 cam_state, dss_state, iva2_state, sgx_state, usb_state;
   struct omap3_processor_cx *cx;
   int ret;
  
 @@ -256,6 +256,8 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,
*its own code.
*/
  
 + /* XXX Add CORE-active check here */
 +
   /*
* Prevent idle completely if CAM is active.
* CAM does not have wakeup capability in OMAP3.
 @@ -275,6 +277,36 @@ static int omap3_enter_idle_bm(struct cpuidle_device 
 *dev,
   (core_next_state  PWRDM_POWER_RET))
   per_next_state = PWRDM_POWER_RET;
  
 + /* XXX Add prevent-PER-off check here */
 +
 + /*
 +  * If we are attempting CORE off, check if any other powerdomains
 +  * are at retention or higher. CORE off causes chipwide reset which
 +  * would reset these domains also.
 +  */
 + if (core_next_state == PWRDM_POWER_OFF) {
 + iva2_state = pwrdm_read_pwrst(iva2_pd);
 + sgx_state = pwrdm_read_pwrst(sgx_pd);
 + usb_state = pwrdm_read_pwrst(usb_pd);
 + dss_state = pwrdm_read_pwrst(dss_pd);
 +
 + if (cam_state  PWRDM_POWER_OFF ||
 + dss_state  PWRDM_POWER_OFF ||
 + iva2_state  PWRDM_POWER_OFF ||
 + per_next_state  PWRDM_POWER_OFF ||
 + sgx_state  PWRDM_POWER_OFF ||
 + usb_state  PWRDM_POWER_OFF)
 + core_next_state = PWRDM_POWER_RET;
 + }
 +
 + /* Fallback to new target core/mpu state */
 + while (cx-core_state  core_next_state) {
 + state--;
 + cx = cpuidle_get_statedata(state);
 + }
 +
 + new_state = state;
 +
   /* Are we changing PER target state? */
   if (per_next_state != per_saved_state)
   pwrdm_set_next_pwrst(per_pd, per_next_state);
 @@ -489,6 +521,10 @@ int __init omap3_idle_init(void)
   core_pd = pwrdm_lookup(core_pwrdm);
   per_pd = pwrdm_lookup(per_pwrdm);
   cam_pd = pwrdm_lookup(cam_pwrdm);
 + dss_pd = pwrdm_lookup(dss_pwrdm);
 + iva2_pd = pwrdm_lookup(iva2_pwrdm);
 + sgx_pd = pwrdm_lookup(sgx_pwrdm);
 + usb_pd = pwrdm_lookup(usbhost_pwrdm);
  
   omap_init_power_states();
   cpuidle_register_driver(omap3_idle_driver);
--
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