[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5419D7BD.30003@linaro.org>
Date: Wed, 17 Sep 2014 11:49:33 -0700
From: Daniel Lezcano <daniel.lezcano@...aro.org>
To: Nishanth Menon <nm@...com>,
Santosh Shilimkar <santosh.shilimkar@...com>,
Tony Lindgren <tony@...mide.com>,
Tero Kristo <t-kristo@...com>, Paul Walmsley <paul@...an.com>
CC: Kevin Hilman <khilman@...prootsystems.com>,
linux-arm-kernel@...ts.infradead.org, linux-omap@...r.kernel.org,
linux-kernel@...r.kernel.org, Keerthy <j-keerthy@...com>,
Benoît Cousson
<bcousson@...libre.com>
Subject: Re: [PATCH 08/10] ARM: OMAP5/DRA7: PM: cpuidle MPU CSWR support
On 08/22/2014 07:02 AM, Nishanth Menon wrote:
> From: Santosh Shilimkar <santosh.shilimkar@...com>
>
> Add OMAP5/DRA74/72 CPUIDLE support.
>
> This patch adds MPUSS low power states in cpuidle.
>
> C1 - CPU0 WFI + CPU1 WFI + MPU ON
> C2 - CPU0 RET + CPU1 RET + MPU CSWR
>
> Tested on DRA74/72-EVM for C1 and C2 states.
>
> NOTE: DRA7 does not do voltage scaling as part of retention transition
> and has Mercury which speeds up transition paths - Latency numbers are
> based on measurements done by toggling GPIOs.
>
> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@...com>
> [ j-keerthy@...com rework on 3.14]
> Signed-off-by: Keerthy <j-keerthy@...com>
> [nm@...com: updates based on profiling, OMAP5 squashed]
> Signed-off-by: Nishanth Menon <nm@...com>
> ---
> arch/arm/mach-omap2/cpuidle44xx.c | 82 ++++++++++++++++++++++++++++++++++++-
> arch/arm/mach-omap2/pm44xx.c | 2 +-
> 2 files changed, 82 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/cpuidle44xx.c b/arch/arm/mach-omap2/cpuidle44xx.c
> index 2498ab0..8ad4f44 100644
> --- a/arch/arm/mach-omap2/cpuidle44xx.c
> +++ b/arch/arm/mach-omap2/cpuidle44xx.c
> @@ -22,6 +22,7 @@
> #include "common.h"
> #include "pm.h"
> #include "prm.h"
> +#include "soc.h"
> #include "clockdomain.h"
>
> #define MAX_CPUS 2
> @@ -31,6 +32,7 @@ struct idle_statedata {
> u32 cpu_state;
> u32 mpu_logic_state;
> u32 mpu_state;
> + u32 mpu_state_vote;
> };
>
> static struct idle_statedata omap4_idle_data[] = {
> @@ -51,12 +53,26 @@ static struct idle_statedata omap4_idle_data[] = {
> },
> };
>
> +static struct idle_statedata dra7_idle_data[] = {
> + {
> + .cpu_state = PWRDM_POWER_ON,
> + .mpu_state = PWRDM_POWER_ON,
> + .mpu_logic_state = PWRDM_POWER_ON,
> + },
> + {
> + .cpu_state = PWRDM_POWER_RET,
> + .mpu_state = PWRDM_POWER_RET,
> + .mpu_logic_state = PWRDM_POWER_RET,
> + },
> +};
> +
> static struct powerdomain *mpu_pd, *cpu_pd[MAX_CPUS];
> static struct clockdomain *cpu_clkdm[MAX_CPUS];
>
> static atomic_t abort_barrier;
> static bool cpu_done[MAX_CPUS];
> static struct idle_statedata *state_ptr = &omap4_idle_data[0];
> +static DEFINE_RAW_SPINLOCK(mpu_lock);
>
> /* Private functions */
>
> @@ -78,6 +94,32 @@ static int omap_enter_idle_simple(struct cpuidle_device *dev,
> return index;
> }
>
> +static int omap_enter_idle_smp(struct cpuidle_device *dev,
> + struct cpuidle_driver *drv,
> + int index)
> +{
> + struct idle_statedata *cx = state_ptr + index;
> + unsigned long flag;
> +
> + raw_spin_lock_irqsave(&mpu_lock, flag);
Why do you need this spin_lock_irqsave ? Aren't the local irqs already
disabled ?
> + cx->mpu_state_vote++;
> + if (cx->mpu_state_vote == num_online_cpus()) {
> + pwrdm_set_logic_retst(mpu_pd, cx->mpu_logic_state);
> + omap_set_pwrdm_state(mpu_pd, cx->mpu_state);
> + }
> + raw_spin_unlock_irqrestore(&mpu_lock, flag);
> +
> + omap4_enter_lowpower(dev->cpu, cx->cpu_state);
> +
> + raw_spin_lock_irqsave(&mpu_lock, flag);
> + if (cx->mpu_state_vote == num_online_cpus())
> + omap_set_pwrdm_state(mpu_pd, PWRDM_POWER_ON);
> + cx->mpu_state_vote--;
> + raw_spin_unlock_irqrestore(&mpu_lock, flag);
I am not sure that will work. What happens if a cpu exits idle and then
re-enter idle immediately ?
Could you try a long run of this little program:
https://git.linaro.org/power/pm-qa.git/blob/HEAD:/cpuidle/cpuidle_killer.c
> + return index;
> +}
> +
> static int omap_enter_idle_coupled(struct cpuidle_device *dev,
> struct cpuidle_driver *drv,
> int index)
> @@ -224,6 +266,34 @@ static struct cpuidle_driver omap4_idle_driver = {
> .safe_state_index = 0,
> };
>
> +static struct cpuidle_driver dra7_idle_driver = {
> + .name = "dra7_idle",
> + .owner = THIS_MODULE,
> + .states = {
> + {
> + /* C1 - CPU0 ON + CPU1 ON + MPU ON */
> + .exit_latency = 2 + 2,
> + .target_residency = 5,
> + .flags = CPUIDLE_FLAG_TIME_VALID,
> + .enter = omap_enter_idle_simple,
> + .name = "C1",
> + .desc = "CPUx WFI, MPUSS ON"
> + },
> + {
> + /* C2 - CPU0 RET + CPU1 RET + MPU CSWR */
> + .exit_latency = 48 + 60,
> + .target_residency = 100,
> + .flags = CPUIDLE_FLAG_TIME_VALID
> + | CPUIDLE_FLAG_TIMER_STOP,
> + .enter = omap_enter_idle_smp,
> + .name = "C2",
> + .desc = "CPUx CSWR, MPUSS CSWR",
> + },
> + },
> + .state_count = ARRAY_SIZE(dra7_idle_data),
> + .safe_state_index = 0,
> +};
> +
> /* Public functions */
>
> /**
> @@ -234,6 +304,16 @@ static struct cpuidle_driver omap4_idle_driver = {
> */
> int __init omap4_idle_init(void)
> {
> + struct cpuidle_driver *idle_driver;
> +
> + if (soc_is_dra7xx() || soc_is_omap54xx()) {
> + state_ptr = &dra7_idle_data[0];
> + idle_driver = &dra7_idle_driver;
> + } else {
> + state_ptr = &omap4_idle_data[0];
> + idle_driver = &omap4_idle_driver;
> + }
> +
> mpu_pd = pwrdm_lookup("mpu_pwrdm");
> cpu_pd[0] = pwrdm_lookup("cpu0_pwrdm");
> cpu_pd[1] = pwrdm_lookup("cpu1_pwrdm");
> @@ -248,5 +328,5 @@ int __init omap4_idle_init(void)
> /* Configure the broadcast timer on each cpu */
> on_each_cpu(omap_setup_broadcast_timer, NULL, 1);
>
> - return cpuidle_register(&omap4_idle_driver, cpu_online_mask);
> + return cpuidle_register(idle_driver, cpu_online_mask);
> }
> diff --git a/arch/arm/mach-omap2/pm44xx.c b/arch/arm/mach-omap2/pm44xx.c
> index c063833..1d22162 100644
> --- a/arch/arm/mach-omap2/pm44xx.c
> +++ b/arch/arm/mach-omap2/pm44xx.c
> @@ -293,7 +293,7 @@ int __init omap4_pm_init(void)
> /* Overwrite the default cpu_do_idle() */
> arm_pm_idle = omap_default_idle;
>
> - if (cpu_is_omap44xx())
> + if (cpu_is_omap44xx() || soc_is_dra7xx() || soc_is_omap54xx())
> omap4_idle_init();
>
> err2:
>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists