[PATCH/RFT 0/3] ARM: OMAP: PM: reduce overhead of pwrdm pre/post transitions

[PATCH/RFT 0/3] ARM: OMAP: PM: reduce overhead of pwrdm pre/post transitions

[Kevin Hilman]

Here's a first pass attempt to reduce the overhead of the pre/post
transitions by allowing them to be called per powerdomain and making
them conditional on powerdomain transtions.

This can be used for testing/measurements to see the reduction the
latencies involved in entering/exiting idle with and without these
patches.

Kevin Hilman (3):
  ARM: OMAP2+: powerdomain: allow pre/post transtion to be per pwrdm
  ARM: OMAP3: PM: call pre/post transition per powerdomain
  ARM: OMAP3: PM: cleanup cam_pwrdm leftovers

 arch/arm/mach-omap2/omap-mpuss-lowpower.c |    4 ++--
 arch/arm/mach-omap2/pm34xx.c              |   14 ++++++++------
 arch/arm/mach-omap2/powerdomain.c         |   16 ++++++++++++----
 arch/arm/mach-omap2/powerdomain.h         |    4 ++--
 4 files changed, 24 insertions(+), 14 deletions(-)

-- 
1.7.9.2

[PATCH/RFT 1/3] ARM: OMAP2+: powerdomain: allow pre/post transtion to be per pwrdm

[Kevin Hilman]

Iteration over all power domains in the idle path is unnecessary since
only power domains that are transitioning need to be accounted for.
Also PRCM register accesses are known to be expensive, so the
additional latency added to the idle path is signficiant.

In order allow the pre/post transitions to be isolated and called
per-pwrdm, change the API so passing in a specific power domain will
trigger the pre/post transtion accounting for only that specific power
domain.  Passing NULL means iterating over all power domains as is
current behavior.

Signed-off-by: Kevin Hilman <khilman@ti.com>
---
 arch/arm/mach-omap2/omap-mpuss-lowpower.c |    4 ++--
 arch/arm/mach-omap2/pm34xx.c              |    4 ++--
 arch/arm/mach-omap2/powerdomain.c         |   16 ++++++++++++----
 arch/arm/mach-omap2/powerdomain.h         |    4 ++--
 4 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/arch/arm/mach-omap2/omap-mpuss-lowpower.c b/arch/arm/mach-omap2/omap-mpuss-lowpower.c
index 13670aa..e35a86b 100644
--- a/arch/arm/mach-omap2/omap-mpuss-lowpower.c
+++ b/arch/arm/mach-omap2/omap-mpuss-lowpower.c
@@ -255,7 +255,7 @@ int omap4_enter_lowpower(unsigned int cpu, unsigned int power_state)
 		return -ENXIO;
 	}
 
-	pwrdm_pre_transition();
+	pwrdm_pre_transition(NULL);
 
 	/*
 	 * Check MPUSS next state and save interrupt controller if needed.
@@ -287,7 +287,7 @@ int omap4_enter_lowpower(unsigned int cpu, unsigned int power_state)
 	wakeup_cpu = smp_processor_id();
 	set_cpu_next_pwrst(wakeup_cpu, PWRDM_POWER_ON);
 
-	pwrdm_post_transition();
+	pwrdm_post_transition(NULL);
 
 	return 0;
 }
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 703bd10..2451b90 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -307,7 +307,7 @@ void omap_sram_idle(void)
 			omap3_enable_io_chain();
 	}
 
-	pwrdm_pre_transition();
+	pwrdm_pre_transition(NULL);
 
 	/* PER */
 	if (per_next_state < PWRDM_POWER_ON) {
@@ -372,7 +372,7 @@ void omap_sram_idle(void)
 	}
 	omap3_intc_resume_idle();
 
-	pwrdm_post_transition();
+	pwrdm_post_transition(NULL);
 
 	/* PER */
 	if (per_next_state < PWRDM_POWER_ON) {
diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
index 96ad3dbe..0baf8c3 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -991,15 +991,23 @@ int pwrdm_clkdm_state_switch(struct clockdomain *clkdm)
 	return -EINVAL;
 }
 
-int pwrdm_pre_transition(void)
+int pwrdm_pre_transition(struct powerdomain *pwrdm)
 {
-	pwrdm_for_each(_pwrdm_pre_transition_cb, NULL);
+	if (pwrdm) 
+		_pwrdm_pre_transition_cb(pwrdm, NULL);
+	else 
+		pwrdm_for_each(_pwrdm_pre_transition_cb, NULL);
+
 	return 0;
 }
 
-int pwrdm_post_transition(void)
+int pwrdm_post_transition(struct powerdomain *pwrdm)
 {
-	pwrdm_for_each(_pwrdm_post_transition_cb, NULL);
+	if (pwrdm)
+		_pwrdm_post_transition_cb(pwrdm, NULL);
+	else
+		pwrdm_for_each(_pwrdm_post_transition_cb, NULL);
+
 	return 0;
 }
 
diff --git a/arch/arm/mach-omap2/powerdomain.h b/arch/arm/mach-omap2/powerdomain.h
index 0d72a8a..a468de4 100644
--- a/arch/arm/mach-omap2/powerdomain.h
+++ b/arch/arm/mach-omap2/powerdomain.h
@@ -214,8 +214,8 @@ int pwrdm_wait_transition(struct powerdomain *pwrdm);
 
 int pwrdm_state_switch(struct powerdomain *pwrdm);
 int pwrdm_clkdm_state_switch(struct clockdomain *clkdm);
-int pwrdm_pre_transition(void);
-int pwrdm_post_transition(void);
+int pwrdm_pre_transition(struct powerdomain *pwrdm);
+int pwrdm_post_transition(struct powerdomain *pwrdm);
 int pwrdm_set_lowpwrstchange(struct powerdomain *pwrdm);
 int pwrdm_get_context_loss_count(struct powerdomain *pwrdm);
 bool pwrdm_can_ever_lose_context(struct powerdomain *pwrdm);
-- 
1.7.9.2

Re: [PATCH/RFT 1/3] ARM: OMAP2+: powerdomain: allow pre/post transtion to be per pwrdm

[Shilimkar, Santosh]

On Tue, Apr 24, 2012 at 7:53 PM, Kevin Hilman <khilman@ti.com> wrote:
> Iteration over all power domains in the idle path is unnecessary since
> only power domains that are transitioning need to be accounted for.
> Also PRCM register accesses are known to be expensive, so the
> additional latency added to the idle path is signficiant.
>
> In order allow the pre/post transitions to be isolated and called
> per-pwrdm, change the API so passing in a specific power domain will
> trigger the pre/post transtion accounting for only that specific power
> domain.  Passing NULL means iterating over all power domains as is
> current behavior.
>
> Signed-off-by: Kevin Hilman <khilman@ti.com>
> ---
>  arch/arm/mach-omap2/omap-mpuss-lowpower.c |    4 ++--
>  arch/arm/mach-omap2/pm34xx.c              |    4 ++--
>  arch/arm/mach-omap2/powerdomain.c         |   16 ++++++++++++----
>  arch/arm/mach-omap2/powerdomain.h         |    4 ++--
>  4 files changed, 18 insertions(+), 10 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/omap-mpuss-lowpower.c b/arch/arm/mach-omap2/omap-mpuss-lowpower.c
> index 13670aa..e35a86b 100644
> --- a/arch/arm/mach-omap2/omap-mpuss-lowpower.c
> +++ b/arch/arm/mach-omap2/omap-mpuss-lowpower.c
> @@ -255,7 +255,7 @@ int omap4_enter_lowpower(unsigned int cpu, unsigned int power_state)
>                return -ENXIO;
>        }
>
> -       pwrdm_pre_transition();
> +       pwrdm_pre_transition(NULL);
>
>        /*
>         * Check MPUSS next state and save interrupt controller if needed.
> @@ -287,7 +287,7 @@ int omap4_enter_lowpower(unsigned int cpu, unsigned int power_state)
>        wakeup_cpu = smp_processor_id();
>        set_cpu_next_pwrst(wakeup_cpu, PWRDM_POWER_ON);
>
> -       pwrdm_post_transition();
> +       pwrdm_post_transition(NULL);
>
>        return 0;
>  }

Now since the   pwrdm_[pre\post]_transition() takes
power domain pointer, OMAP4 can limit this
to CPU and MPUSS PD's only.

Will create that patch on top of this series and test
it out.

Thanks for the API change.

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

Re: [PATCH/RFT 1/3] ARM: OMAP2+: powerdomain: allow pre/post transtion to be per pwrdm

[Shilimkar, Santosh]

On Tue, Apr 24, 2012 at 7:53 PM, Kevin Hilman <khilman@ti.com> wrote:
> Iteration over all power domains in the idle path is unnecessary since
> only power domains that are transitioning need to be accounted for.
> Also PRCM register accesses are known to be expensive, so the
> additional latency added to the idle path is signficiant.
>
> In order allow the pre/post transitions to be isolated and called
> per-pwrdm, change the API so passing in a specific power domain will
> trigger the pre/post transtion accounting for only that specific power
> domain.  Passing NULL means iterating over all power domains as is
> current behavior.
>
> Signed-off-by: Kevin Hilman <khilman@ti.com>
> ---
>  arch/arm/mach-omap2/omap-mpuss-lowpower.c |    4 ++--
>  arch/arm/mach-omap2/pm34xx.c              |    4 ++--
>  arch/arm/mach-omap2/powerdomain.c         |   16 ++++++++++++----
>  arch/arm/mach-omap2/powerdomain.h         |    4 ++--
>  4 files changed, 18 insertions(+), 10 deletions(-)
>
[...]

> diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
> index 96ad3dbe..0baf8c3 100644
> --- a/arch/arm/mach-omap2/powerdomain.c
> +++ b/arch/arm/mach-omap2/powerdomain.c
> @@ -991,15 +991,23 @@ int pwrdm_clkdm_state_switch(struct clockdomain *clkdm)
>        return -EINVAL;
>  }
>
> -int pwrdm_pre_transition(void)
> +int pwrdm_pre_transition(struct powerdomain *pwrdm)
>  {
> -       pwrdm_for_each(_pwrdm_pre_transition_cb, NULL);
> +       if (pwrdm)
> +               _pwrdm_pre_transition_cb(pwrdm, NULL);
> +       else
> +               pwrdm_for_each(_pwrdm_pre_transition_cb, NULL);
> +
>        return 0;
>  }
>
Minor nit. Noticed couple of whitespace errors in the patch.

-------
ERROR: trailing whitespace
#82: FILE: arch/arm/mach-omap2/powerdomain.c:996:
+^Iif (pwrdm) $

ERROR: trailing whitespace
#84: FILE: arch/arm/mach-omap2/powerdomain.c:998:
+^Ielse $

total: 2 errors, 0 warnings, 69 lines checked
-------

Updated patch below with that fixed.

Regards
Santosh

From 2782503adcf91142d2aee4bafe29989095ece3ba Mon Sep 17 00:00:00 2001
From: Kevin Hilman <khilman@ti.com>
Date: Wed, 25 Apr 2012 14:05:21 +0530
Subject: [PATCH 1/1] ARM: OMAP2+: powerdomain: allow pre/post transtion to be
 per pwrdm

Iteration over all power domains in the idle path is unnecessary since
only power domains that are transitioning need to be accounted for.
Also PRCM register accesses are known to be expensive, so the
additional latency added to the idle path is signficiant.

In order allow the pre/post transitions to be isolated and called
per-pwrdm, change the API so passing in a specific power domain will
trigger the pre/post transtion accounting for only that specific power
domain.  Passing NULL means iterating over all power domains as is
current behavior.

Signed-off-by: Kevin Hilman <khilman@ti.com>
---
 arch/arm/mach-omap2/omap-mpuss-lowpower.c |    4 ++--
 arch/arm/mach-omap2/pm34xx.c              |    4 ++--
 arch/arm/mach-omap2/powerdomain.c         |   16 ++++++++++++----
 arch/arm/mach-omap2/powerdomain.h         |    4 ++--
 4 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/arch/arm/mach-omap2/omap-mpuss-lowpower.c
b/arch/arm/mach-omap2/omap-mpuss-lowpower.c
index 13670aa..e35a86b 100644
--- a/arch/arm/mach-omap2/omap-mpuss-lowpower.c
+++ b/arch/arm/mach-omap2/omap-mpuss-lowpower.c
@@ -255,7 +255,7 @@ int omap4_enter_lowpower(unsigned int cpu,
unsigned int power_state)
 		return -ENXIO;
 	}

-	pwrdm_pre_transition();
+	pwrdm_pre_transition(NULL);

 	/*
 	 * Check MPUSS next state and save interrupt controller if needed.
@@ -287,7 +287,7 @@ int omap4_enter_lowpower(unsigned int cpu,
unsigned int power_state)
 	wakeup_cpu = smp_processor_id();
 	set_cpu_next_pwrst(wakeup_cpu, PWRDM_POWER_ON);

-	pwrdm_post_transition();
+	pwrdm_post_transition(NULL);

 	return 0;
 }
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 703bd10..2451b90 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -307,7 +307,7 @@ void omap_sram_idle(void)
 			omap3_enable_io_chain();
 	}

-	pwrdm_pre_transition();
+	pwrdm_pre_transition(NULL);

 	/* PER */
 	if (per_next_state < PWRDM_POWER_ON) {
@@ -372,7 +372,7 @@ void omap_sram_idle(void)
 	}
 	omap3_intc_resume_idle();

-	pwrdm_post_transition();
+	pwrdm_post_transition(NULL);

 	/* PER */
 	if (per_next_state < PWRDM_POWER_ON) {
diff --git a/arch/arm/mach-omap2/powerdomain.c
b/arch/arm/mach-omap2/powerdomain.c
index 96ad3dbe..bb6780d 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -991,15 +991,23 @@ int pwrdm_clkdm_state_switch(struct clockdomain *clkdm)
 	return -EINVAL;
 }

-int pwrdm_pre_transition(void)
+int pwrdm_pre_transition(struct powerdomain *pwrdm)
 {
-	pwrdm_for_each(_pwrdm_pre_transition_cb, NULL);
+	if (pwrdm)
+		_pwrdm_pre_transition_cb(pwrdm, NULL);
+	else
+		pwrdm_for_each(_pwrdm_pre_transition_cb, NULL);
+
 	return 0;
 }

-int pwrdm_post_transition(void)
+int pwrdm_post_transition(struct powerdomain *pwrdm)
 {
-	pwrdm_for_each(_pwrdm_post_transition_cb, NULL);
+	if (pwrdm)
+		_pwrdm_post_transition_cb(pwrdm, NULL);
+	else
+		pwrdm_for_each(_pwrdm_post_transition_cb, NULL);
+
 	return 0;
 }

diff --git a/arch/arm/mach-omap2/powerdomain.h
b/arch/arm/mach-omap2/powerdomain.h
index 0d72a8a..a468de4 100644
--- a/arch/arm/mach-omap2/powerdomain.h
+++ b/arch/arm/mach-omap2/powerdomain.h
@@ -214,8 +214,8 @@ int pwrdm_wait_transition(struct powerdomain *pwrdm);

 int pwrdm_state_switch(struct powerdomain *pwrdm);
 int pwrdm_clkdm_state_switch(struct clockdomain *clkdm);
-int pwrdm_pre_transition(void);
-int pwrdm_post_transition(void);
+int pwrdm_pre_transition(struct powerdomain *pwrdm);
+int pwrdm_post_transition(struct powerdomain *pwrdm);
 int pwrdm_set_lowpwrstchange(struct powerdomain *pwrdm);
 int pwrdm_get_context_loss_count(struct powerdomain *pwrdm);
 bool pwrdm_can_ever_lose_context(struct powerdomain *pwrdm);
-- 
1.7.5.4
--
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

Re: [PATCH/RFT 1/3] ARM: OMAP2+: powerdomain: allow pre/post transtion to be per pwrdm

[Paul Walmsley]

On Tue, 24 Apr 2012, Kevin Hilman wrote:

> Iteration over all power domains in the idle path is unnecessary since
> only power domains that are transitioning need to be accounted for.
> Also PRCM register accesses are known to be expensive, so the
> additional latency added to the idle path is signficiant.
> 
> In order allow the pre/post transitions to be isolated and called
> per-pwrdm, change the API so passing in a specific power domain will
> trigger the pre/post transtion accounting for only that specific power
> domain.  Passing NULL means iterating over all power domains as is
> current behavior.
> 
> Signed-off-by: Kevin Hilman <khilman@ti.com>

Based on a quick glance, it looks good to me:

Acked-by: Paul Walmsley <paul@pwsan.com>

Want to queue this one?


- Paul

Re: [PATCH/RFT 1/3] ARM: OMAP2+: powerdomain: allow pre/post transtion to be per pwrdm

[Kevin Hilman]

Paul Walmsley <paul@pwsan.com> writes:

> On Tue, 24 Apr 2012, Kevin Hilman wrote:
>
>> Iteration over all power domains in the idle path is unnecessary since
>> only power domains that are transitioning need to be accounted for.
>> Also PRCM register accesses are known to be expensive, so the
>> additional latency added to the idle path is signficiant.
>> 
>> In order allow the pre/post transitions to be isolated and called
>> per-pwrdm, change the API so passing in a specific power domain will
>> trigger the pre/post transtion accounting for only that specific power
>> domain.  Passing NULL means iterating over all power domains as is
>> current behavior.
>> 
>> Signed-off-by: Kevin Hilman <khilman@ti.com>
>
> Based on a quick glance, it looks good to me:
>
> Acked-by: Paul Walmsley <paul@pwsan.com>
>
> Want to queue this one?

Yes, I'll queue this along with the others.

Thanks,

Kevin

[PATCH/RFT 2/3] ARM: OMAP3: PM: call pre/post transition per powerdomain

[Kevin Hilman]

We only need to call the pre/post transtion methods when we know the
power state is changing.  First, split up the pre/post transition
calls to be per-powerdomain, and then make them conditional on whether
the power domain is actually changing states.

Signed-off-by: Kevin Hilman <khilman@ti.com>
---
 arch/arm/mach-omap2/pm34xx.c |   12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 2451b90..74a7f8c 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -307,16 +307,19 @@ void omap_sram_idle(void)
 			omap3_enable_io_chain();
 	}
 
-	pwrdm_pre_transition(NULL);
+	if (mpu_next_state < PWRDM_POWER_ON)
+		pwrdm_pre_transition(mpu_pwrdm);
 
 	/* PER */
 	if (per_next_state < PWRDM_POWER_ON) {
+		pwrdm_pre_transition(per_pwrdm);
 		per_going_off = (per_next_state == PWRDM_POWER_OFF) ? 1 : 0;
 		omap2_gpio_prepare_for_idle(per_going_off);
 	}
 
 	/* CORE */
 	if (core_next_state < PWRDM_POWER_ON) {
+		pwrdm_pre_transition(core_pwrdm);
 		if (core_next_state == PWRDM_POWER_OFF) {
 			omap3_core_save_context();
 			omap3_cm_save_context();
@@ -369,15 +372,15 @@ void omap_sram_idle(void)
 			omap2_prm_clear_mod_reg_bits(OMAP3430_AUTO_OFF_MASK,
 					       OMAP3430_GR_MOD,
 					       OMAP3_PRM_VOLTCTRL_OFFSET);
+		pwrdm_post_transition(core_pwrdm);
 	}
 	omap3_intc_resume_idle();
 
-	pwrdm_post_transition(NULL);
-
 	/* PER */
 	if (per_next_state < PWRDM_POWER_ON) {
 		per_prev_state = pwrdm_read_prev_pwrst(per_pwrdm);
 		omap2_gpio_resume_after_idle();
+		pwrdm_post_transition(per_pwrdm);
 	}
 
 	/* Disable IO-PAD and IO-CHAIN wakeup */
@@ -390,6 +393,9 @@ void omap_sram_idle(void)
 			omap3_disable_io_chain();
 	}
 
+	if (mpu_next_state < PWRDM_POWER_ON)
+		pwrdm_post_transition(mpu_pwrdm);
+
 	clkdm_allow_idle(mpu_pwrdm->pwrdm_clkdms[0]);
 }
 
-- 
1.7.9.2

Re: [PATCH/RFT 2/3] ARM: OMAP3: PM: call pre/post transition per powerdomain

[Kevin Hilman]

On 04/24/2012 07:23 AM, Kevin Hilman wrote:
> We only need to call the pre/post transtion methods when we know the
> power state is changing.  First, split up the pre/post transition
> calls to be per-powerdomain, and then make them conditional on whether
> the power domain is actually changing states.
>
> Signed-off-by: Kevin Hilman<khilman@ti.com>

This patch introduced a regression where the transition counts for NEON 
were no longer accurate because I forgot to call the notifiers for NEON 
when calling them for MPU.

The diff below will be folded into this patch before submitting for v3.6.

Kevin

diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 8bd7daf..6226d10 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -306,8 +306,10 @@ void omap_sram_idle(void)
  			omap3_enable_io_chain();
  	}

-	if (mpu_next_state < PWRDM_POWER_ON)
+	if (mpu_next_state < PWRDM_POWER_ON) {
  		pwrdm_pre_transition(mpu_pwrdm);
+		pwrdm_pre_transition(neon_pwrdm);
+	}

  	/* PER */
  	if (per_next_state < PWRDM_POWER_ON) {
@@ -391,8 +393,10 @@ void omap_sram_idle(void)
  			omap3_disable_io_chain();
  	}

-	if (mpu_next_state < PWRDM_POWER_ON)
+	if (mpu_next_state < PWRDM_POWER_ON) {
  		pwrdm_post_transition(mpu_pwrdm);
+		pwrdm_post_transition(neon_pwrdm);
+	}
  }

  static void omap3_pm_idle(void)

[PATCH/RFT 3/3] ARM: OMAP3: PM: cleanup cam_pwrdm leftovers

[Kevin Hilman]

commit e7410cf7 (02fdb03e69699f26e1370d0e51593dbc8a4e5265) moved
mangement of cam_pwrdm to CPUidle but left some remnants behind,
namely the call to clkcm_allo_idle() for the clockdomains in the MPU
pwrdm.  Remove these since they are not necessary and cause unwanted
latency in the idle path.

Cc: Tero Kristo <Tero Kristo <t-kristo@ti.com>
Signed-off-by: Kevin Hilman <khilman@ti.com>
---
 arch/arm/mach-omap2/pm34xx.c |    4 ----
 1 file changed, 4 deletions(-)

diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 74a7f8c..66ff828 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -70,7 +70,6 @@ void (*omap3_do_wfi_sram)(void);
 
 static struct powerdomain *mpu_pwrdm, *neon_pwrdm;
 static struct powerdomain *core_pwrdm, *per_pwrdm;
-static struct powerdomain *cam_pwrdm;
 
 static void omap3_enable_io_chain(void)
 {
@@ -395,8 +394,6 @@ void omap_sram_idle(void)
 
 	if (mpu_next_state < PWRDM_POWER_ON)
 		pwrdm_post_transition(mpu_pwrdm);
-
-	clkdm_allow_idle(mpu_pwrdm->pwrdm_clkdms[0]);
 }
 
 static void omap3_pm_idle(void)
@@ -759,7 +756,6 @@ static int __init omap3_pm_init(void)
 	neon_pwrdm = pwrdm_lookup("neon_pwrdm");
 	per_pwrdm = pwrdm_lookup("per_pwrdm");
 	core_pwrdm = pwrdm_lookup("core_pwrdm");
-	cam_pwrdm = pwrdm_lookup("cam_pwrdm");
 
 	neon_clkdm = clkdm_lookup("neon_clkdm");
 	mpu_clkdm = clkdm_lookup("mpu_clkdm");
-- 
1.7.9.2

Re: [PATCH/RFT 3/3] ARM: OMAP3: PM: cleanup cam_pwrdm leftovers

[Tero Kristo]

Looks good to me, acked.

-Tero

On Tue, 2012-04-24 at 07:23 -0700, Kevin Hilman wrote:
> commit e7410cf7 (02fdb03e69699f26e1370d0e51593dbc8a4e5265) moved
> mangement of cam_pwrdm to CPUidle but left some remnants behind,
> namely the call to clkcm_allo_idle() for the clockdomains in the MPU
> pwrdm.  Remove these since they are not necessary and cause unwanted
> latency in the idle path.
> 
> Cc: Tero Kristo <Tero Kristo <t-kristo@ti.com>
> Signed-off-by: Kevin Hilman <khilman@ti.com>
> ---
>  arch/arm/mach-omap2/pm34xx.c |    4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index 74a7f8c..66ff828 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -70,7 +70,6 @@ void (*omap3_do_wfi_sram)(void);
>  
>  static struct powerdomain *mpu_pwrdm, *neon_pwrdm;
>  static struct powerdomain *core_pwrdm, *per_pwrdm;
> -static struct powerdomain *cam_pwrdm;
>  
>  static void omap3_enable_io_chain(void)
>  {
> @@ -395,8 +394,6 @@ void omap_sram_idle(void)
>  
>  	if (mpu_next_state < PWRDM_POWER_ON)
>  		pwrdm_post_transition(mpu_pwrdm);
> -
> -	clkdm_allow_idle(mpu_pwrdm->pwrdm_clkdms[0]);
>  }
>  
>  static void omap3_pm_idle(void)
> @@ -759,7 +756,6 @@ static int __init omap3_pm_init(void)
>  	neon_pwrdm = pwrdm_lookup("neon_pwrdm");
>  	per_pwrdm = pwrdm_lookup("per_pwrdm");
>  	core_pwrdm = pwrdm_lookup("core_pwrdm");
> -	cam_pwrdm = pwrdm_lookup("cam_pwrdm");
>  
>  	neon_clkdm = clkdm_lookup("neon_clkdm");
>  	mpu_clkdm = clkdm_lookup("mpu_clkdm");

Re: [PATCH/RFT 3/3] ARM: OMAP3: PM: cleanup cam_pwrdm leftovers

[Kevin Hilman]

Tero Kristo <t-kristo@ti.com> writes:

> Looks good to me, acked.

Thanks.

Kevin

>
> On Tue, 2012-04-24 at 07:23 -0700, Kevin Hilman wrote:
>> commit e7410cf7 (02fdb03e69699f26e1370d0e51593dbc8a4e5265) moved
>> mangement of cam_pwrdm to CPUidle but left some remnants behind,
>> namely the call to clkcm_allo_idle() for the clockdomains in the MPU
>> pwrdm.  Remove these since they are not necessary and cause unwanted
>> latency in the idle path.
>> 
>> Cc: Tero Kristo <Tero Kristo <t-kristo@ti.com>
>> Signed-off-by: Kevin Hilman <khilman@ti.com>
>> ---
>>  arch/arm/mach-omap2/pm34xx.c |    4 ----
>>  1 file changed, 4 deletions(-)
>> 
>> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
>> index 74a7f8c..66ff828 100644
>> --- a/arch/arm/mach-omap2/pm34xx.c
>> +++ b/arch/arm/mach-omap2/pm34xx.c
>> @@ -70,7 +70,6 @@ void (*omap3_do_wfi_sram)(void);
>>  
>>  static struct powerdomain *mpu_pwrdm, *neon_pwrdm;
>>  static struct powerdomain *core_pwrdm, *per_pwrdm;
>> -static struct powerdomain *cam_pwrdm;
>>  
>>  static void omap3_enable_io_chain(void)
>>  {
>> @@ -395,8 +394,6 @@ void omap_sram_idle(void)
>>  
>>  	if (mpu_next_state < PWRDM_POWER_ON)
>>  		pwrdm_post_transition(mpu_pwrdm);
>> -
>> -	clkdm_allow_idle(mpu_pwrdm->pwrdm_clkdms[0]);
>>  }
>>  
>>  static void omap3_pm_idle(void)
>> @@ -759,7 +756,6 @@ static int __init omap3_pm_init(void)
>>  	neon_pwrdm = pwrdm_lookup("neon_pwrdm");
>>  	per_pwrdm = pwrdm_lookup("per_pwrdm");
>>  	core_pwrdm = pwrdm_lookup("core_pwrdm");
>> -	cam_pwrdm = pwrdm_lookup("cam_pwrdm");
>>  
>>  	neon_clkdm = clkdm_lookup("neon_clkdm");
>>  	mpu_clkdm = clkdm_lookup("mpu_clkdm");
>
>
> --
> 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

Re: [PATCH/RFT 3/3] ARM: OMAP3: PM: cleanup cam_pwrdm leftovers

[Grazvydas Ignotas]

On Tue, Apr 24, 2012 at 5:23 PM, Kevin Hilman <khilman@ti.com> wrote:
> commit e7410cf7 (02fdb03e69699f26e1370d0e51593dbc8a4e5265) moved
> mangement of cam_pwrdm to CPUidle but left some remnants behind,
> namely the call to clkcm_allo_idle() for the clockdomains in the MPU
> pwrdm.  Remove these since they are not necessary and cause unwanted
> latency in the idle path.

Only noticed this now; what does
02fdb03e69699f26e1370d0e51593dbc8a4e5265 refer to? Doesn't seem to be
resolving to anything here.


-- 
Gražvydas
--
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

Re: [PATCH/RFT 3/3] ARM: OMAP3: PM: cleanup cam_pwrdm leftovers

[Kevin Hilman]

Grazvydas Ignotas <notasas@gmail.com> writes:

> On Tue, Apr 24, 2012 at 5:23 PM, Kevin Hilman <khilman@ti.com> wrote:
>> commit e7410cf7 (02fdb03e69699f26e1370d0e51593dbc8a4e5265) moved
>> mangement of cam_pwrdm to CPUidle but left some remnants behind,
>> namely the call to clkcm_allo_idle() for the clockdomains in the MPU
>> pwrdm.  Remove these since they are not necessary and cause unwanted
>> latency in the idle path.
>
> Only noticed this now; what does
> 02fdb03e69699f26e1370d0e51593dbc8a4e5265 refer to? Doesn't seem to be
> resolving to anything here.

Oops, copy/paste problem.  I've updated the changelog locally.  Updated
patch below.

Thanks for pointing it out,

Kevin


From 77d4c33d506938896cb87fb4034e3b65fe5e30f0 Mon Sep 17 00:00:00 2001
From: Kevin Hilman <khilman@ti.com>
Date: Mon, 16 Apr 2012 17:53:51 -0700
Subject: [PATCH] ARM: OMAP3: PM: cleanup cam_pwrdm leftovers

commit e7410cf7 (OMAP3: PM: move device-specific special cases from PM
core into CPUidle) moved mangement of cam_pwrdm to CPUidle but left
some remnants behind, namely the call to clkcm_allo_idle() for the
clockdomains in the MPU pwrdm.  Remove these since they are not
necessary and cause unwanted latency in the idle path.

Acked-by: Tero Kristo <Tero Kristo <t-kristo@ti.com>
Reviewed-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Tested-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Tested-by: Grazvydas Ignotas <notasas@gmail.com>
Signed-off-by: Kevin Hilman <khilman@ti.com>
---
 arch/arm/mach-omap2/pm34xx.c |    4 ----
 1 file changed, 4 deletions(-)

diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 74a7f8c..66ff828 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -70,7 +70,6 @@ void (*omap3_do_wfi_sram)(void);
 
 static struct powerdomain *mpu_pwrdm, *neon_pwrdm;
 static struct powerdomain *core_pwrdm, *per_pwrdm;
-static struct powerdomain *cam_pwrdm;
 
 static void omap3_enable_io_chain(void)
 {
@@ -395,8 +394,6 @@ void omap_sram_idle(void)
 
 	if (mpu_next_state < PWRDM_POWER_ON)
 		pwrdm_post_transition(mpu_pwrdm);
-
-	clkdm_allow_idle(mpu_pwrdm->pwrdm_clkdms[0]);
 }
 
 static void omap3_pm_idle(void)
@@ -759,7 +756,6 @@ static int __init omap3_pm_init(void)
 	neon_pwrdm = pwrdm_lookup("neon_pwrdm");
 	per_pwrdm = pwrdm_lookup("per_pwrdm");
 	core_pwrdm = pwrdm_lookup("core_pwrdm");
-	cam_pwrdm = pwrdm_lookup("cam_pwrdm");
 
 	neon_clkdm = clkdm_lookup("neon_clkdm");
 	mpu_clkdm = clkdm_lookup("mpu_clkdm");
-- 
1.7.9.2

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

Re: [PATCH/RFT 0/3] ARM: OMAP: PM: reduce overhead of pwrdm pre/post transitions

[Grazvydas Ignotas]

On Tue, Apr 24, 2012 at 5:23 PM, Kevin Hilman <khilman@ti.com> wrote:
> Here's a first pass attempt to reduce the overhead of the pre/post
> transitions by allowing them to be called per powerdomain and making
> them conditional on powerdomain transtions.
>
> This can be used for testing/measurements to see the reduction the
> latencies involved in entering/exiting idle with and without these
> patches.

Seems to have similar effect as my earlier test of simply commenting
out pre/post transitions (~4.4MB/s), so this definitely helps.

Tested-by: Grazvydas Ignotas <notasas@gmail.com>

-- 
Gražvydas
--
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

Re: [PATCH/RFT 0/3] ARM: OMAP: PM: reduce overhead of pwrdm pre/post transitions

[Shilimkar, Santosh]

On Tue, Apr 24, 2012 at 7:53 PM, Kevin Hilman <khilman@ti.com> wrote:
> Here's a first pass attempt to reduce the overhead of the pre/post
> transitions by allowing them to be called per powerdomain and making
> them conditional on powerdomain transtions.
>
> This can be used for testing/measurements to see the reduction the
> latencies involved in entering/exiting idle with and without these
> patches.
>
> Kevin Hilman (3):
>  ARM: OMAP2+: powerdomain: allow pre/post transtion to be per pwrdm
>  ARM: OMAP3: PM: call pre/post transition per powerdomain
>  ARM: OMAP3: PM: cleanup cam_pwrdm leftovers
>
I have reviewed and tested this series on OMAP4 with coupleidle and system
wide supsned. It continues to work as expected. I have done an additional
patch(end of email)  as mentioned to take advantage of per powerdomain
pre/post transition API update.

Thanks for the series. it certainly removes the big over-head we had before
with the pre-post APIs.

FWIW,
Reviewed-tested-by: Santosh Shilimkar <santosh.shilimkar@ti.com>

Regards
Santosh

From e535b0d8948ed44732c6128ff4236cfe32d2f40a Mon Sep 17 00:00:00 2001
From: Santosh Shilimkar <santosh.shilimkar@ti.com>
Date: Wed, 25 Apr 2012 17:04:08 +0530
Subject: [PATCH] ARM: OMAP4: PM: call pre/post transition per powerdomain

Iteration over all power domains in the idle path is unnecessary since
only power domains that are transitioning need to be accounted for.

Update OMAP4 low power code accordingly.

Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Kevin Hilman <khilman@ti.com>
---
There is one issue though about MPUSS powerdomain accounting. It will
get called on both CPUs and might result in duplicate pm debug counter
update. There is no functional issue with that as such but needs to
be cleaned up.

Will address that issue in a seperate series.

 arch/arm/mach-omap2/omap-mpuss-lowpower.c |   22 ++++++++++++++++++++--
 1 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/omap-mpuss-lowpower.c
b/arch/arm/mach-omap2/omap-mpuss-lowpower.c
index e35a86b..3a709b2 100644
--- a/arch/arm/mach-omap2/omap-mpuss-lowpower.c
+++ b/arch/arm/mach-omap2/omap-mpuss-lowpower.c
@@ -117,6 +117,20 @@ static inline void clear_cpu_prev_pwrst(unsigned
int cpu_id)
 }

 /*
+ * CPU powerdomain pre/post transition.
+ */
+static inline void cpu_pwrdm_pre_post_transition(unsigned int cpu_id,
+				bool pre_transition)
+{
+	struct omap4_cpu_pm_info *pm_info = &per_cpu(omap4_pm_info, cpu_id);
+
+	if (pre_transition)
+		pwrdm_pre_transition(pm_info->pwrdm);
+	else
+		pwrdm_post_transition(pm_info->pwrdm);
+}
+
+/*
  * Store the SCU power status value to scratchpad memory
  */
 static void scu_pwrst_prepare(unsigned int cpu_id, unsigned int cpu_state)
@@ -255,7 +269,7 @@ int omap4_enter_lowpower(unsigned int cpu,
unsigned int power_state)
 		return -ENXIO;
 	}

-	pwrdm_pre_transition(NULL);
+	pwrdm_pre_transition(mpuss_pd);

 	/*
 	 * Check MPUSS next state and save interrupt controller if needed.
@@ -266,6 +280,7 @@ int omap4_enter_lowpower(unsigned int cpu,
unsigned int power_state)
 		(pwrdm_read_logic_retst(mpuss_pd) == PWRDM_POWER_OFF))
 		save_state = 2;

+	cpu_pwrdm_pre_post_transition(cpu, 1);
 	cpu_clear_prev_logic_pwrst(cpu);
 	set_cpu_next_pwrst(cpu, power_state);
 	set_cpu_wakeup_addr(cpu, virt_to_phys(omap4_cpu_resume));
@@ -285,9 +300,10 @@ int omap4_enter_lowpower(unsigned int cpu,
unsigned int power_state)
 	 * domain transition
 	 */
 	wakeup_cpu = smp_processor_id();
+	cpu_pwrdm_pre_post_transition(wakeup_cpu, 0);
 	set_cpu_next_pwrst(wakeup_cpu, PWRDM_POWER_ON);

-	pwrdm_post_transition(NULL);
+	pwrdm_post_transition(mpuss_pd);

 	return 0;
 }
@@ -307,6 +323,7 @@ int __cpuinit omap4_hotplug_cpu(unsigned int cpu,
unsigned int power_state)
 	if (power_state == PWRDM_POWER_OFF)
 		cpu_state = 1;

+	cpu_pwrdm_pre_post_transition(cpu, 1);
 	clear_cpu_prev_pwrst(cpu);
 	set_cpu_next_pwrst(cpu, power_state);
 	set_cpu_wakeup_addr(cpu, virt_to_phys(omap_secondary_startup));
@@ -319,6 +336,7 @@ int __cpuinit omap4_hotplug_cpu(unsigned int cpu,
unsigned int power_state)
 	 */
 	omap4_finish_suspend(cpu_state);

+	cpu_pwrdm_pre_post_transition(wakeup_cpu, 0);
 	set_cpu_next_pwrst(cpu, PWRDM_POWER_ON);
 	return 0;
 }
-- 
1.7.5.4
--
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