* [PATCH 1/7] ARM: tegra: cpuidle: separate cpuidle driver for different chips
2012-10-08 10:26 [PATCH 0/7] ARM: tegra30: cpuidle: add LP2 support Joseph Lo
@ 2012-10-08 10:26 ` Joseph Lo
2012-10-09 22:22 ` Stephen Warren
2012-10-08 10:26 ` [PATCH 2/7] ARM: tegra: cpuidle: add LP2 resume function Joseph Lo
` (6 subsequent siblings)
7 siblings, 1 reply; 50+ messages in thread
From: Joseph Lo @ 2012-10-08 10:26 UTC (permalink / raw)
To: linux-arm-kernel
The different Tegra chips may have different CPU idle states and data.
Individual CPU idle driver make it more easy to maintain.
Signed-off-by: Joseph Lo <josephl@nvidia.com>
---
arch/arm/mach-tegra/Makefile | 6 +++
.../mach-tegra/{cpuidle.c => cpuidle-tegra20.c} | 23 ++++-----
.../mach-tegra/{cpuidle.c => cpuidle-tegra30.c} | 26 ++++-------
arch/arm/mach-tegra/cpuidle.c | 47 +++++--------------
arch/arm/mach-tegra/cpuidle.h | 32 +++++++++++++
5 files changed, 71 insertions(+), 63 deletions(-)
copy arch/arm/mach-tegra/{cpuidle.c => cpuidle-tegra20.c} (72%)
copy arch/arm/mach-tegra/{cpuidle.c => cpuidle-tegra30.c} (65%)
create mode 100644 arch/arm/mach-tegra/cpuidle.h
diff --git a/arch/arm/mach-tegra/Makefile b/arch/arm/mach-tegra/Makefile
index e6929c6..9b80c1e 100644
--- a/arch/arm/mach-tegra/Makefile
+++ b/arch/arm/mach-tegra/Makefile
@@ -14,9 +14,15 @@ obj-$(CONFIG_ARCH_TEGRA_2x_SOC) += tegra20_clocks.o
obj-$(CONFIG_ARCH_TEGRA_2x_SOC) += tegra20_clocks_data.o
obj-$(CONFIG_ARCH_TEGRA_2x_SOC) += tegra2_emc.o
obj-$(CONFIG_ARCH_TEGRA_2x_SOC) += sleep-tegra20.o
+ifeq ($(CONFIG_CPU_IDLE),y)
+obj-$(CONFIG_ARCH_TEGRA_2x_SOC) += cpuidle-tegra20.o
+endif
obj-$(CONFIG_ARCH_TEGRA_3x_SOC) += tegra30_clocks.o
obj-$(CONFIG_ARCH_TEGRA_3x_SOC) += tegra30_clocks_data.o
obj-$(CONFIG_ARCH_TEGRA_3x_SOC) += sleep-tegra30.o
+ifeq ($(CONFIG_CPU_IDLE),y)
+obj-$(CONFIG_ARCH_TEGRA_3x_SOC) += cpuidle-tegra30.o
+endif
obj-$(CONFIG_SMP) += platsmp.o headsmp.o
obj-$(CONFIG_SMP) += reset.o
obj-$(CONFIG_HOTPLUG_CPU) += hotplug.o
diff --git a/arch/arm/mach-tegra/cpuidle.c b/arch/arm/mach-tegra/cpuidle-tegra20.c
similarity index 72%
copy from arch/arm/mach-tegra/cpuidle.c
copy to arch/arm/mach-tegra/cpuidle-tegra20.c
index 4e0b07c..e2fc26a 100644
--- a/arch/arm/mach-tegra/cpuidle.c
+++ b/arch/arm/mach-tegra/cpuidle-tegra20.c
@@ -1,24 +1,22 @@
/*
- * arch/arm/mach-tegra/cpuidle.c
+ * CPU idle driver for Tegra20 CPUs
*
- * CPU idle driver for Tegra CPUs
- *
- * Copyright (c) 2010-2012, NVIDIA Corporation.
+ * Copyright (c) 2010-2012, NVIDIA Corporation. All rights reserved.
* Copyright (c) 2011 Google, Inc.
* Author: Colin Cross <ccross@android.com>
* Gary King <gking@nvidia.com>
*
- * Rework for 3.3 by Peter De Schrijver <pdeschrijver@nvidia.com>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
* more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
#include <linux/kernel.h>
@@ -27,7 +25,7 @@
#include <asm/cpuidle.h>
-struct cpuidle_driver tegra_idle_driver = {
+static struct cpuidle_driver tegra_idle_driver = {
.name = "tegra_idle",
.owner = THIS_MODULE,
.en_core_tk_irqen = 1,
@@ -39,7 +37,7 @@ struct cpuidle_driver tegra_idle_driver = {
static DEFINE_PER_CPU(struct cpuidle_device, tegra_idle_device);
-static int __init tegra_cpuidle_init(void)
+int __init tegra20_cpuidle_init(void)
{
int ret;
unsigned int cpu;
@@ -66,4 +64,3 @@ static int __init tegra_cpuidle_init(void)
}
return 0;
}
-device_initcall(tegra_cpuidle_init);
diff --git a/arch/arm/mach-tegra/cpuidle.c b/arch/arm/mach-tegra/cpuidle-tegra30.c
similarity index 65%
copy from arch/arm/mach-tegra/cpuidle.c
copy to arch/arm/mach-tegra/cpuidle-tegra30.c
index 4e0b07c..0f85df8 100644
--- a/arch/arm/mach-tegra/cpuidle.c
+++ b/arch/arm/mach-tegra/cpuidle-tegra30.c
@@ -1,24 +1,19 @@
/*
- * arch/arm/mach-tegra/cpuidle.c
+ * CPU idle driver for Tegra30 CPUs
*
- * CPU idle driver for Tegra CPUs
+ * Copyright (c) 2012, NVIDIA Corporation. All rights reserved.
*
- * Copyright (c) 2010-2012, NVIDIA Corporation.
- * Copyright (c) 2011 Google, Inc.
- * Author: Colin Cross <ccross@android.com>
- * Gary King <gking@nvidia.com>
- *
- * Rework for 3.3 by Peter De Schrijver <pdeschrijver@nvidia.com>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
* more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
#include <linux/kernel.h>
@@ -27,7 +22,7 @@
#include <asm/cpuidle.h>
-struct cpuidle_driver tegra_idle_driver = {
+static struct cpuidle_driver tegra_idle_driver = {
.name = "tegra_idle",
.owner = THIS_MODULE,
.en_core_tk_irqen = 1,
@@ -39,7 +34,7 @@ struct cpuidle_driver tegra_idle_driver = {
static DEFINE_PER_CPU(struct cpuidle_device, tegra_idle_device);
-static int __init tegra_cpuidle_init(void)
+int __init tegra30_cpuidle_init(void)
{
int ret;
unsigned int cpu;
@@ -66,4 +61,3 @@ static int __init tegra_cpuidle_init(void)
}
return 0;
}
-device_initcall(tegra_cpuidle_init);
diff --git a/arch/arm/mach-tegra/cpuidle.c b/arch/arm/mach-tegra/cpuidle.c
index 4e0b07c..d065139 100644
--- a/arch/arm/mach-tegra/cpuidle.c
+++ b/arch/arm/mach-tegra/cpuidle.c
@@ -23,47 +23,26 @@
#include <linux/kernel.h>
#include <linux/module.h>
-#include <linux/cpuidle.h>
-#include <asm/cpuidle.h>
-
-struct cpuidle_driver tegra_idle_driver = {
- .name = "tegra_idle",
- .owner = THIS_MODULE,
- .en_core_tk_irqen = 1,
- .state_count = 1,
- .states = {
- [0] = ARM_CPUIDLE_WFI_STATE_PWR(600),
- },
-};
-
-static DEFINE_PER_CPU(struct cpuidle_device, tegra_idle_device);
+#include "fuse.h"
+#include "cpuidle.h"
static int __init tegra_cpuidle_init(void)
{
int ret;
- unsigned int cpu;
- struct cpuidle_device *dev;
- struct cpuidle_driver *drv = &tegra_idle_driver;
- ret = cpuidle_register_driver(&tegra_idle_driver);
- if (ret) {
- pr_err("CPUidle driver registration failed\n");
- return ret;
+ switch (tegra_chip_id) {
+ case TEGRA20:
+ ret = tegra20_cpuidle_init();
+ break;
+ case TEGRA30:
+ ret = tegra30_cpuidle_init();
+ break;
+ default:
+ ret = -ENODEV;
+ break;
}
- for_each_possible_cpu(cpu) {
- dev = &per_cpu(tegra_idle_device, cpu);
- dev->cpu = cpu;
-
- dev->state_count = drv->state_count;
- ret = cpuidle_register_device(dev);
- if (ret) {
- pr_err("CPU%u: CPUidle device registration failed\n",
- cpu);
- return ret;
- }
- }
- return 0;
+ return ret;
}
device_initcall(tegra_cpuidle_init);
diff --git a/arch/arm/mach-tegra/cpuidle.h b/arch/arm/mach-tegra/cpuidle.h
new file mode 100644
index 0000000..496204d
--- /dev/null
+++ b/arch/arm/mach-tegra/cpuidle.h
@@ -0,0 +1,32 @@
+/*
+ * Copyright (c) 2012, NVIDIA Corporation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __MACH_TEGRA_CPUIDLE_H
+#define __MACH_TEGRA_CPUIDLE_H
+
+#ifdef CONFIG_ARCH_TEGRA_2x_SOC
+int tegra20_cpuidle_init(void);
+#else
+static inline int tegra20_cpuidle_init(void) { return -ENODEV; }
+#endif
+
+#ifdef CONFIG_ARCH_TEGRA_3x_SOC
+int tegra30_cpuidle_init(void);
+#else
+static inline int tegra30_cpuidle_init(void) { return -ENODEV; }
+#endif
+
+#endif
--
1.7.0.4
^ permalink raw reply related [flat|nested] 50+ messages in thread* [PATCH 1/7] ARM: tegra: cpuidle: separate cpuidle driver for different chips
2012-10-08 10:26 ` [PATCH 1/7] ARM: tegra: cpuidle: separate cpuidle driver for different chips Joseph Lo
@ 2012-10-09 22:22 ` Stephen Warren
2012-10-11 6:42 ` Joseph Lo
0 siblings, 1 reply; 50+ messages in thread
From: Stephen Warren @ 2012-10-09 22:22 UTC (permalink / raw)
To: linux-arm-kernel
On 10/08/2012 04:26 AM, Joseph Lo wrote:
> The different Tegra chips may have different CPU idle states and data.
> Individual CPU idle driver make it more easy to maintain.
> diff --git a/arch/arm/mach-tegra/cpuidle.c b/arch/arm/mach-tegra/cpuidle-tegra20.c
> similarity index 72%
> copy from arch/arm/mach-tegra/cpuidle.c
> copy to arch/arm/mach-tegra/cpuidle-tegra20.c
> index 4e0b07c..e2fc26a 100644
> --- a/arch/arm/mach-tegra/cpuidle.c
> +++ b/arch/arm/mach-tegra/cpuidle-tegra20.c
> @@ -1,24 +1,22 @@
> /*
> - * arch/arm/mach-tegra/cpuidle.c
> + * CPU idle driver for Tegra20 CPUs
> *
> - * CPU idle driver for Tegra CPUs
> - *
> - * Copyright (c) 2010-2012, NVIDIA Corporation.
> + * Copyright (c) 2010-2012, NVIDIA Corporation. All rights reserved.
> * Copyright (c) 2011 Google, Inc.
> * Author: Colin Cross <ccross@android.com>
> * Gary King <gking@nvidia.com>
> *
> - * Rework for 3.3 by Peter De Schrijver <pdeschrijver@nvidia.com>
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License as published by
> - * the Free Software Foundation; either version 2 of the License, or
> - * (at your option) any later version.
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> *
> * This program is distributed in the hope that it will be useful, but WITHOUT
> * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> * more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> */
While that is the correct (c) header for new work submitted upstream,
given this file is existing, just renamed and tweaked a bit, I don't
believe you want to be changing the (c) header at all. Same for
cpuidle-tegra30.c.
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 1/7] ARM: tegra: cpuidle: separate cpuidle driver for different chips
2012-10-09 22:22 ` Stephen Warren
@ 2012-10-11 6:42 ` Joseph Lo
0 siblings, 0 replies; 50+ messages in thread
From: Joseph Lo @ 2012-10-11 6:42 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, 2012-10-10 at 06:22 +0800, Stephen Warren wrote:
> On 10/08/2012 04:26 AM, Joseph Lo wrote:
> > The different Tegra chips may have different CPU idle states and data.
> > Individual CPU idle driver make it more easy to maintain.
>
> > diff --git a/arch/arm/mach-tegra/cpuidle.c b/arch/arm/mach-tegra/cpuidle-tegra20.c
> > similarity index 72%
> > copy from arch/arm/mach-tegra/cpuidle.c
> > copy to arch/arm/mach-tegra/cpuidle-tegra20.c
> > index 4e0b07c..e2fc26a 100644
> > --- a/arch/arm/mach-tegra/cpuidle.c
> > +++ b/arch/arm/mach-tegra/cpuidle-tegra20.c
> > @@ -1,24 +1,22 @@
> > /*
> > - * arch/arm/mach-tegra/cpuidle.c
> > + * CPU idle driver for Tegra20 CPUs
> > *
> > - * CPU idle driver for Tegra CPUs
> > - *
> > - * Copyright (c) 2010-2012, NVIDIA Corporation.
> > + * Copyright (c) 2010-2012, NVIDIA Corporation. All rights reserved.
> > * Copyright (c) 2011 Google, Inc.
> > * Author: Colin Cross <ccross@android.com>
> > * Gary King <gking@nvidia.com>
> > *
> > - * Rework for 3.3 by Peter De Schrijver <pdeschrijver@nvidia.com>
> > - *
> > - * This program is free software; you can redistribute it and/or modify
> > - * it under the terms of the GNU General Public License as published by
> > - * the Free Software Foundation; either version 2 of the License, or
> > - * (at your option) any later version.
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms and conditions of the GNU General Public License,
> > + * version 2, as published by the Free Software Foundation.
> > *
> > * This program is distributed in the hope that it will be useful, but WITHOUT
> > * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> > * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> > * more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> > */
>
> While that is the correct (c) header for new work submitted upstream,
> given this file is existing, just renamed and tweaked a bit, I don't
> believe you want to be changing the (c) header at all. Same for
> cpuidle-tegra30.c.
OK. I would just remove this line.
- * arch/arm/mach-tegra/cpuidle.c
Thanks,
Joseph
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 2/7] ARM: tegra: cpuidle: add LP2 resume function
2012-10-08 10:26 [PATCH 0/7] ARM: tegra30: cpuidle: add LP2 support Joseph Lo
2012-10-08 10:26 ` [PATCH 1/7] ARM: tegra: cpuidle: separate cpuidle driver for different chips Joseph Lo
@ 2012-10-08 10:26 ` Joseph Lo
2012-10-09 22:29 ` Stephen Warren
2012-10-08 10:26 ` [PATCH 3/7] ARM: tegra30: cpuidle: add LP2 driver for secondary CPUs Joseph Lo
` (5 subsequent siblings)
7 siblings, 1 reply; 50+ messages in thread
From: Joseph Lo @ 2012-10-08 10:26 UTC (permalink / raw)
To: linux-arm-kernel
LP2 is one of the Tegra low power states that supports power gating both
CPU cores and GICs. Adding a resume function for taking care the CPUs that
resume from LP2. This function was been hooked to reset handler. We take
care everything here before go into kernel.
Based on the work by:
Scott Williams <scwilliams@nvidia.com>
Colin Cross <ccross@android.com>
Gary King <gking@nvidia.com>
Signed-off-by: Joseph Lo <josephl@nvidia.com>
---
arch/arm/mach-tegra/headsmp.S | 58 +++++++++++++++++++++++++++++++++++++++++
arch/arm/mach-tegra/reset.c | 6 ++++
arch/arm/mach-tegra/sleep.h | 1 +
3 files changed, 65 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-tegra/headsmp.S b/arch/arm/mach-tegra/headsmp.S
index 6addc78..abf86b3 100644
--- a/arch/arm/mach-tegra/headsmp.S
+++ b/arch/arm/mach-tegra/headsmp.S
@@ -69,6 +69,53 @@ ENTRY(tegra_secondary_startup)
b secondary_startup
ENDPROC(tegra_secondary_startup)
+#ifdef CONFIG_PM_SLEEP
+/*
+ * tegra_resume
+ *
+ * CPU boot vector when restarting the a CPU following
+ * an LP2 transition. Also branched to by LP0 and LP1 resume after
+ * re-enabling sdram.
+ */
+ENTRY(tegra_resume)
+ bl v7_invalidate_l1
+ /* Enable coresight */
+ mov32 r0, 0xC5ACCE55
+ mcr p14, 0, r0, c7, c12, 6
+
+ cpu_id r0
+ cmp r0, #0 @ CPU0?
+ bne cpu_resume @ no
+
+ /* Are we on Tegra20? */
+ mov32 r6, TEGRA_APB_MISC_BASE
+ ldr r0, [r6, #APB_MISC_GP_HIDREV]
+ and r0, r0, #0xff00
+ cmp r0, #(0x20 << 8)
+ beq 1f
+#ifdef CONFIG_ARCH_TEGRA_3x_SOC
+ /* Clear the flow controller flags for this CPU. */
+ mov32 r2, TEGRA_FLOW_CTRL_BASE + FLOW_CTRL_CPU0_CSR @ CPU0 CSR
+ ldr r1, [r2]
+ /* Clear event & intr flag */
+ orr r1, r1, \
+ #FLOW_CTRL_CSR_INTR_FLAG | FLOW_CTRL_CSR_EVENT_FLAG
+ movw r0, #0x0FFD @ enable, cluster_switch, immed, & bitmaps
+ bic r1, r1, r0
+ str r1, [r2]
+#endif
+1:
+
+ /* enable SCU */
+ mov32 r0, TEGRA_ARM_PERIF_BASE
+ ldr r1, [r0]
+ orr r1, r1, #1
+ str r1, [r0]
+
+ b cpu_resume
+ENDPROC(tegra_resume)
+#endif
+
.align L1_CACHE_SHIFT
ENTRY(__tegra_cpu_reset_handler_start)
@@ -122,6 +169,17 @@ ENTRY(__tegra_cpu_reset_handler)
1:
#endif
+ /* Waking up from LP2? */
+ ldr r9, [r12, #RESET_DATA(MASK_LP2)]
+ tst r9, r11 @ if in_lp2
+ beq __is_not_lp2
+ ldr lr, [r12, #RESET_DATA(STARTUP_LP2)]
+ cmp lr, #0
+ bleq __die @ no LP2 startup handler
+ bx lr
+
+__is_not_lp2:
+
#ifdef CONFIG_SMP
/*
* Can only be secondary boot (initial or hotplug) but CPU 0
diff --git a/arch/arm/mach-tegra/reset.c b/arch/arm/mach-tegra/reset.c
index 5beb7eb..c48548f 100644
--- a/arch/arm/mach-tegra/reset.c
+++ b/arch/arm/mach-tegra/reset.c
@@ -26,6 +26,7 @@
#include <mach/irammap.h>
#include "reset.h"
+#include "sleep.h"
#include "fuse.h"
#define TEGRA_IRAM_RESET_BASE (TEGRA_IRAM_BASE + \
@@ -80,5 +81,10 @@ void __init tegra_cpu_reset_handler_init(void)
virt_to_phys((void *)tegra_secondary_startup);
#endif
+#ifdef CONFIG_PM_SLEEP
+ __tegra_cpu_reset_handler_data[TEGRA_RESET_STARTUP_LP2] =
+ virt_to_phys((void *)tegra_resume);
+#endif
+
tegra_cpu_reset_handler_enable();
}
diff --git a/arch/arm/mach-tegra/sleep.h b/arch/arm/mach-tegra/sleep.h
index e25a7cd..c9dec37 100644
--- a/arch/arm/mach-tegra/sleep.h
+++ b/arch/arm/mach-tegra/sleep.h
@@ -72,6 +72,7 @@
dsb
.endm
#else
+void tegra_resume(void);
#ifdef CONFIG_HOTPLUG_CPU
void tegra20_hotplug_init(void);
--
1.7.0.4
^ permalink raw reply related [flat|nested] 50+ messages in thread* [PATCH 2/7] ARM: tegra: cpuidle: add LP2 resume function
2012-10-08 10:26 ` [PATCH 2/7] ARM: tegra: cpuidle: add LP2 resume function Joseph Lo
@ 2012-10-09 22:29 ` Stephen Warren
2012-10-11 7:08 ` Joseph Lo
0 siblings, 1 reply; 50+ messages in thread
From: Stephen Warren @ 2012-10-09 22:29 UTC (permalink / raw)
To: linux-arm-kernel
On 10/08/2012 04:26 AM, Joseph Lo wrote:
> LP2 is one of the Tegra low power states that supports power gating both
> CPU cores and GICs. Adding a resume function for taking care the CPUs that
> resume from LP2. This function was been hooked to reset handler. We take
> care everything here before go into kernel.
> diff --git a/arch/arm/mach-tegra/headsmp.S b/arch/arm/mach-tegra/headsmp.S
> +ENTRY(tegra_resume)
...
> + /* Are we on Tegra20? */
> + mov32 r6, TEGRA_APB_MISC_BASE
> + ldr r0, [r6, #APB_MISC_GP_HIDREV]
> + and r0, r0, #0xff00
> + cmp r0, #(0x20 << 8)
> + beq 1f
> +#ifdef CONFIG_ARCH_TEGRA_3x_SOC
> + /* Clear the flow controller flags for this CPU. */
> + mov32 r2, TEGRA_FLOW_CTRL_BASE + FLOW_CTRL_CPU0_CSR @ CPU0 CSR
> + ldr r1, [r2]
> + /* Clear event & intr flag */
> + orr r1, r1, \
> + #FLOW_CTRL_CSR_INTR_FLAG | FLOW_CTRL_CSR_EVENT_FLAG
> + movw r0, #0x0FFD @ enable, cluster_switch, immed, & bitmaps
> + bic r1, r1, r0
> + str r1, [r2]
> +#endif
> +1:
couldn't that entire quoted chunk go inside the ifdef; all of reading
the HIDREV register and the 1: label?
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 2/7] ARM: tegra: cpuidle: add LP2 resume function
2012-10-09 22:29 ` Stephen Warren
@ 2012-10-11 7:08 ` Joseph Lo
0 siblings, 0 replies; 50+ messages in thread
From: Joseph Lo @ 2012-10-11 7:08 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, 2012-10-10 at 06:29 +0800, Stephen Warren wrote:
> On 10/08/2012 04:26 AM, Joseph Lo wrote:
> > LP2 is one of the Tegra low power states that supports power gating both
> > CPU cores and GICs. Adding a resume function for taking care the CPUs that
> > resume from LP2. This function was been hooked to reset handler. We take
> > care everything here before go into kernel.
>
> > diff --git a/arch/arm/mach-tegra/headsmp.S b/arch/arm/mach-tegra/headsmp.S
>
> > +ENTRY(tegra_resume)
> ...
> > + /* Are we on Tegra20? */
> > + mov32 r6, TEGRA_APB_MISC_BASE
> > + ldr r0, [r6, #APB_MISC_GP_HIDREV]
> > + and r0, r0, #0xff00
> > + cmp r0, #(0x20 << 8)
> > + beq 1f
> > +#ifdef CONFIG_ARCH_TEGRA_3x_SOC
> > + /* Clear the flow controller flags for this CPU. */
> > + mov32 r2, TEGRA_FLOW_CTRL_BASE + FLOW_CTRL_CPU0_CSR @ CPU0 CSR
> > + ldr r1, [r2]
> > + /* Clear event & intr flag */
> > + orr r1, r1, \
> > + #FLOW_CTRL_CSR_INTR_FLAG | FLOW_CTRL_CSR_EVENT_FLAG
> > + movw r0, #0x0FFD @ enable, cluster_switch, immed, & bitmaps
> > + bic r1, r1, r0
> > + str r1, [r2]
> > +#endif
> > +1:
>
> couldn't that entire quoted chunk go inside the ifdef; all of reading
> the HIDREV register and the 1: label?
Ah. Yes, it's OK. Will do.
Thanks,
Joseph
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 3/7] ARM: tegra30: cpuidle: add LP2 driver for secondary CPUs
2012-10-08 10:26 [PATCH 0/7] ARM: tegra30: cpuidle: add LP2 support Joseph Lo
2012-10-08 10:26 ` [PATCH 1/7] ARM: tegra: cpuidle: separate cpuidle driver for different chips Joseph Lo
2012-10-08 10:26 ` [PATCH 2/7] ARM: tegra: cpuidle: add LP2 resume function Joseph Lo
@ 2012-10-08 10:26 ` Joseph Lo
2012-10-08 16:35 ` Lorenzo Pieralisi
2012-10-09 22:38 ` Stephen Warren
2012-10-08 10:26 ` [PATCH 4/7] ARM: tegra30: common: enable csite clock Joseph Lo
` (4 subsequent siblings)
7 siblings, 2 replies; 50+ messages in thread
From: Joseph Lo @ 2012-10-08 10:26 UTC (permalink / raw)
To: linux-arm-kernel
This supports power-gated (LP2) idle on secondary CPUs for Tegra30.
The secondary CPUs can go into LP2 state independently. When CPU goes
into LP2 state, it saves it's state and puts itself to flow controlled
WFI state. After that, it will been power gated.
Based on the work by:
Scott Williams <scwilliams@nvidia.com>
Signed-off-by: Joseph Lo <josephl@nvidia.com>
---
arch/arm/mach-tegra/Makefile | 1 +
arch/arm/mach-tegra/cpuidle-tegra30.c | 79 +++++++++++++++++++++++++++++-
arch/arm/mach-tegra/pm.c | 88 +++++++++++++++++++++++++++++++++
arch/arm/mach-tegra/pm.h | 30 +++++++++++
arch/arm/mach-tegra/reset.h | 9 +++
arch/arm/mach-tegra/sleep-tegra30.S | 26 ++++++++++
arch/arm/mach-tegra/sleep.S | 66 ++++++++++++++++++++++++
arch/arm/mach-tegra/sleep.h | 2 +
8 files changed, 300 insertions(+), 1 deletions(-)
create mode 100644 arch/arm/mach-tegra/pm.c
create mode 100644 arch/arm/mach-tegra/pm.h
diff --git a/arch/arm/mach-tegra/Makefile b/arch/arm/mach-tegra/Makefile
index 9b80c1e..6f224f7 100644
--- a/arch/arm/mach-tegra/Makefile
+++ b/arch/arm/mach-tegra/Makefile
@@ -8,6 +8,7 @@ obj-y += pmc.o
obj-y += flowctrl.o
obj-y += powergate.o
obj-y += apbio.o
+obj-y += pm.o
obj-$(CONFIG_CPU_IDLE) += cpuidle.o
obj-$(CONFIG_CPU_IDLE) += sleep.o
obj-$(CONFIG_ARCH_TEGRA_2x_SOC) += tegra20_clocks.o
diff --git a/arch/arm/mach-tegra/cpuidle-tegra30.c b/arch/arm/mach-tegra/cpuidle-tegra30.c
index 0f85df8..842fef4 100644
--- a/arch/arm/mach-tegra/cpuidle-tegra30.c
+++ b/arch/arm/mach-tegra/cpuidle-tegra30.c
@@ -19,21 +19,94 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/cpuidle.h>
+#include <linux/cpu_pm.h>
+#include <linux/clockchips.h>
#include <asm/cpuidle.h>
+#include <asm/proc-fns.h>
+#include <asm/suspend.h>
+
+#include "pm.h"
+#include "sleep.h"
+
+#ifdef CONFIG_PM_SLEEP
+static int tegra30_idle_lp2(struct cpuidle_device *dev,
+ struct cpuidle_driver *drv,
+ int index);
+#endif
static struct cpuidle_driver tegra_idle_driver = {
.name = "tegra_idle",
.owner = THIS_MODULE,
.en_core_tk_irqen = 1,
- .state_count = 1,
+ .state_count = 2,
.states = {
[0] = ARM_CPUIDLE_WFI_STATE_PWR(600),
+#ifdef CONFIG_PM_SLEEP
+ [1] = {
+ .enter = tegra30_idle_lp2,
+ .exit_latency = 2000,
+ .target_residency = 2200,
+ .power_usage = 0,
+ .flags = CPUIDLE_FLAG_TIME_VALID,
+ .name = "LP2",
+ .desc = "CPU power-gate",
+ },
+#endif
},
};
static DEFINE_PER_CPU(struct cpuidle_device, tegra_idle_device);
+#ifdef CONFIG_PM_SLEEP
+static bool tegra30_idle_enter_lp2_cpu_n(struct cpuidle_device *dev,
+ struct cpuidle_driver *drv,
+ int index)
+{
+#ifdef CONFIG_SMP
+ clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &dev->cpu);
+
+ smp_wmb();
+
+ save_cpu_arch_register();
+
+ cpu_suspend(0, tegra30_sleep_cpu_secondary_finish);
+
+ restore_cpu_arch_register();
+
+ clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &dev->cpu);
+#endif
+
+ return true;
+}
+
+static int __cpuinit tegra30_idle_lp2(struct cpuidle_device *dev,
+ struct cpuidle_driver *drv,
+ int index)
+{
+ bool entered_lp2 = false;
+
+ local_fiq_disable();
+
+ tegra_set_cpu_in_lp2(dev->cpu);
+ cpu_pm_enter();
+
+ if (dev->cpu == 0)
+ cpu_do_idle();
+ else
+ entered_lp2 = tegra30_idle_enter_lp2_cpu_n(dev, drv, index);
+
+ cpu_pm_exit();
+ tegra_clear_cpu_in_lp2(dev->cpu);
+
+ local_fiq_enable();
+
+ smp_rmb();
+
+ return (entered_lp2) ? index : 0;
+}
+#endif
+
int __init tegra30_cpuidle_init(void)
{
int ret;
@@ -41,6 +114,10 @@ int __init tegra30_cpuidle_init(void)
struct cpuidle_device *dev;
struct cpuidle_driver *drv = &tegra_idle_driver;
+#ifndef CONFIG_PM_SLEEP
+ drv->state_count = 1; /* support clockgating only */
+#endif
+
ret = cpuidle_register_driver(&tegra_idle_driver);
if (ret) {
pr_err("CPUidle driver registration failed\n");
diff --git a/arch/arm/mach-tegra/pm.c b/arch/arm/mach-tegra/pm.c
new file mode 100644
index 0000000..4655383
--- /dev/null
+++ b/arch/arm/mach-tegra/pm.c
@@ -0,0 +1,88 @@
+/*
+ * CPU complex suspend & resume functions for Tegra SoCs
+ *
+ * Copyright (c) 2009-2012, NVIDIA Corporation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/kernel.h>
+#include <linux/spinlock.h>
+#include <linux/io.h>
+#include <linux/cpumask.h>
+
+#include <mach/iomap.h>
+
+#include "reset.h"
+
+#ifdef CONFIG_PM_SLEEP
+static unsigned int g_diag_reg;
+static DEFINE_SPINLOCK(tegra_lp2_lock);
+static cpumask_t tegra_in_lp2;
+
+void save_cpu_arch_register(void)
+{
+ /*read diagnostic register*/
+ asm("mrc p15, 0, %0, c15, c0, 1" : "=r"(g_diag_reg) : : "cc");
+ return;
+}
+
+void restore_cpu_arch_register(void)
+{
+ /*write diagnostic register*/
+ asm("mcr p15, 0, %0, c15, c0, 1" : : "r"(g_diag_reg) : "cc");
+ return;
+}
+
+void __cpuinit tegra_clear_cpu_in_lp2(int cpu)
+{
+ spin_lock(&tegra_lp2_lock);
+ BUG_ON(!cpumask_test_cpu(cpu, &tegra_in_lp2));
+ cpumask_clear_cpu(cpu, &tegra_in_lp2);
+
+ /*
+ * Update the IRAM copy used by the reset handler. The IRAM copy
+ * can't use used directly by cpumask_clear_cpu() because it uses
+ * LDREX/STREX which requires the addressed location to be inner
+ * cacheable and sharable which IRAM isn't.
+ */
+ writel(tegra_in_lp2.bits[0], tegra_cpu_lp2_mask);
+ dsb();
+
+ spin_unlock(&tegra_lp2_lock);
+}
+
+bool __cpuinit tegra_set_cpu_in_lp2(int cpu)
+{
+ bool last_cpu = false;
+
+ spin_lock(&tegra_lp2_lock);
+ BUG_ON(cpumask_test_cpu(cpu, &tegra_in_lp2));
+ cpumask_set_cpu(cpu, &tegra_in_lp2);
+
+ /*
+ * Update the IRAM copy used by the reset handler. The IRAM copy
+ * can't use used directly by cpumask_set_cpu() because it uses
+ * LDREX/STREX which requires the addressed location to be inner
+ * cacheable and sharable which IRAM isn't.
+ */
+ writel(tegra_in_lp2.bits[0], tegra_cpu_lp2_mask);
+ dsb();
+
+ if ((cpu == 0) && cpumask_equal(&tegra_in_lp2, cpu_online_mask))
+ last_cpu = true;
+
+ spin_unlock(&tegra_lp2_lock);
+ return last_cpu;
+}
+#endif
diff --git a/arch/arm/mach-tegra/pm.h b/arch/arm/mach-tegra/pm.h
new file mode 100644
index 0000000..21858d6
--- /dev/null
+++ b/arch/arm/mach-tegra/pm.h
@@ -0,0 +1,30 @@
+/*
+ * Copyright (C) 2010 Google, Inc.
+ * Copyright (c) 2010-2012 NVIDIA Corporation. All rights reserved.
+ *
+ * Author:
+ * Colin Cross <ccross@google.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef _MACH_TEGRA_PM_H_
+#define _MACH_TEGRA_PM_H_
+
+void save_cpu_arch_register(void);
+void restore_cpu_arch_register(void);
+
+void tegra_clear_cpu_in_lp2(int cpu);
+bool tegra_set_cpu_in_lp2(int cpu);
+
+#endif /* _MACH_TEGRA_PM_H_ */
diff --git a/arch/arm/mach-tegra/reset.h b/arch/arm/mach-tegra/reset.h
index de88bf8..234cd6b 100644
--- a/arch/arm/mach-tegra/reset.h
+++ b/arch/arm/mach-tegra/reset.h
@@ -29,6 +29,8 @@
#ifndef __ASSEMBLY__
+#include <mach/irammap.h>
+
extern unsigned long __tegra_cpu_reset_handler_data[TEGRA_RESET_DATA_SIZE];
void __tegra_cpu_reset_handler_start(void);
@@ -36,6 +38,13 @@ void __tegra_cpu_reset_handler(void);
void __tegra_cpu_reset_handler_end(void);
void tegra_secondary_startup(void);
+#ifdef CONFIG_PM_SLEEP
+#define tegra_cpu_lp2_mask \
+ (IO_ADDRESS(TEGRA_IRAM_BASE + TEGRA_IRAM_RESET_HANDLER_OFFSET + \
+ ((u32)&__tegra_cpu_reset_handler_data[TEGRA_RESET_MASK_LP2] - \
+ (u32)__tegra_cpu_reset_handler_start)))
+#endif
+
#define tegra_cpu_reset_handler_offset \
((u32)__tegra_cpu_reset_handler - \
(u32)__tegra_cpu_reset_handler_start)
diff --git a/arch/arm/mach-tegra/sleep-tegra30.S b/arch/arm/mach-tegra/sleep-tegra30.S
index 777d9ce..67b1cba 100644
--- a/arch/arm/mach-tegra/sleep-tegra30.S
+++ b/arch/arm/mach-tegra/sleep-tegra30.S
@@ -17,6 +17,8 @@
#include <linux/linkage.h>
#include <asm/assembler.h>
+#include <asm/asm-offsets.h>
+#include <asm/glue-cache.h>
#include <mach/iomap.h>
@@ -82,6 +84,7 @@ delay_1:
ldr r3, [r1] @ read CSR
str r3, [r1] @ clear CSR
tst r0, #TEGRA30_POWER_HOTPLUG_SHUTDOWN
+ moveq r3, #FLOW_CTRL_WAIT_FOR_INTERRUPT @ For LP2
movne r3, #FLOW_CTRL_WAITEVENT @ For hotplug
str r3, [r2]
ldr r0, [r2]
@@ -105,3 +108,26 @@ wfe_war:
ENDPROC(tegra30_cpu_shutdown)
#endif
+
+#ifdef CONFIG_PM_SLEEP
+/*
+ * tegra30_sleep_cpu_secondary_finish(unsigned long v2p)
+ *
+ * Enters LP2 on secondary CPU by exiting coherency and powergating the CPU.
+ */
+ENTRY(tegra30_sleep_cpu_secondary_finish)
+ mov r7, lr
+
+ /* Flush and disable the L1 data cache */
+ bl tegra_flush_l1_cache
+
+ /* Turn off SMP coherency */
+ exit_smp r4, r5
+
+ /* Powergate this CPU. */
+ mov r0, #0 @ power mode flags (!hotplug)
+ bl tegra30_cpu_shutdown
+ mov r0, #1 @ never return here
+ mov pc, r7
+ENDPROC(tegra30_sleep_cpu_secondary_finish)
+#endif
diff --git a/arch/arm/mach-tegra/sleep.S b/arch/arm/mach-tegra/sleep.S
index ea81554..7748fdb 100644
--- a/arch/arm/mach-tegra/sleep.S
+++ b/arch/arm/mach-tegra/sleep.S
@@ -25,9 +25,75 @@
#include <linux/linkage.h>
#include <asm/assembler.h>
+#include <asm/cp15.h>
#include <mach/iomap.h>
#include "flowctrl.h"
#include "sleep.h"
+#ifdef CONFIG_PM_SLEEP
+/*
+ * tegra_flush_l1_cache
+ *
+ * clean & invalidate the L1 cache
+ *
+ * The flush_cache_all flushes all caches within level of coherence, this
+ * may not be desired if all we need is to flush L1 only. Therefore this
+ * function is implemented to flush the L1 cache only.
+ *
+ * Disable is needed before flush to prevent allocation during flush.
+ * When cache is disabled, we cannot push to stack.
+ *
+ * Corrupted registers: r0-r7, r9-r11
+ */
+
+ENTRY(tegra_flush_l1_cache)
+ stmfd sp!, {r4-r5, r7, r9-r11, lr}
+ dmb @ ensure ordering
+
+ /* Disable the data cache */
+ mrc p15, 0, r2, c1, c0, 0
+ bic r2, r2, #CR_C
+ dsb
+ mcr p15, 0, r2, c1, c0, 0
+
+ /* Flush data cache */
+ mov r10, #0
+#ifdef CONFIG_PREEMPT
+ save_and_disable_irqs_notrace r9 @ make cssr&csidr read atomic
+#endif
+ mcr p15, 2, r10, c0, c0, 0 @ select current cache level in cssr
+ isb @ isb to sych the new cssr&csidr
+ mrc p15, 1, r1, c0, c0, 0 @ read the new csidr
+#ifdef CONFIG_PREEMPT
+ restore_irqs_notrace r9
+#endif
+ and r2, r1, #7 @ extract the length of the cache lines
+ add r2, r2, #4 @ add 4 (line length offset)
+ ldr r4, =0x3ff
+ ands r4, r4, r1, lsr #3 @ find maximum number on the way size
+ clz r5, r4 @ find bit position of way size increment
+ ldr r7, =0x7fff
+ ands r7, r7, r1, lsr #13 @ extract max number of the index size
+loop2:
+ mov r9, r4 @ create working copy of max way size
+loop3:
+ orr r11, r10, r9, lsl r5 @ factor way and cache number into r11
+ orr r11, r11, r7, lsl r2 @ factor index number into r11
+ mcr p15, 0, r11, c7, c14, 2 @ clean & invalidate by set/way
+ subs r9, r9, #1 @ decrement the way
+ bge loop3
+ subs r7, r7, #1 @ decrement the index
+ bge loop2
+finished:
+ mov r10, #0 @ swith back to cache level 0
+ mcr p15, 2, r10, c0, c0, 0 @ select current cache level in cssr
+ dsb
+ isb
+
+ ldmfd sp!, {r4-r5, r7, r9-r11, lr}
+ mov pc, lr
+ENDPROC(tegra_flush_l1_cache)
+
+#endif
diff --git a/arch/arm/mach-tegra/sleep.h b/arch/arm/mach-tegra/sleep.h
index c9dec37..220fbd1 100644
--- a/arch/arm/mach-tegra/sleep.h
+++ b/arch/arm/mach-tegra/sleep.h
@@ -82,5 +82,7 @@ static inline void tegra20_hotplug_init(void) {}
static inline void tegra30_hotplug_init(void) {}
#endif
+int tegra30_sleep_cpu_secondary_finish(unsigned long);
+
#endif
#endif
--
1.7.0.4
^ permalink raw reply related [flat|nested] 50+ messages in thread* [PATCH 3/7] ARM: tegra30: cpuidle: add LP2 driver for secondary CPUs
2012-10-08 10:26 ` [PATCH 3/7] ARM: tegra30: cpuidle: add LP2 driver for secondary CPUs Joseph Lo
@ 2012-10-08 16:35 ` Lorenzo Pieralisi
2012-10-09 4:13 ` Joseph Lo
2012-10-09 22:38 ` Stephen Warren
1 sibling, 1 reply; 50+ messages in thread
From: Lorenzo Pieralisi @ 2012-10-08 16:35 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Oct 08, 2012 at 11:26:17AM +0100, Joseph Lo wrote:
> This supports power-gated (LP2) idle on secondary CPUs for Tegra30.
> The secondary CPUs can go into LP2 state independently. When CPU goes
> into LP2 state, it saves it's state and puts itself to flow controlled
> WFI state. After that, it will been power gated.
>
> Based on the work by:
> Scott Williams <scwilliams@nvidia.com>
>
> Signed-off-by: Joseph Lo <josephl@nvidia.com>
> ---
> arch/arm/mach-tegra/Makefile | 1 +
> arch/arm/mach-tegra/cpuidle-tegra30.c | 79 +++++++++++++++++++++++++++++-
> arch/arm/mach-tegra/pm.c | 88 +++++++++++++++++++++++++++++++++
> arch/arm/mach-tegra/pm.h | 30 +++++++++++
> arch/arm/mach-tegra/reset.h | 9 +++
> arch/arm/mach-tegra/sleep-tegra30.S | 26 ++++++++++
> arch/arm/mach-tegra/sleep.S | 66 ++++++++++++++++++++++++
> arch/arm/mach-tegra/sleep.h | 2 +
> 8 files changed, 300 insertions(+), 1 deletions(-)
> create mode 100644 arch/arm/mach-tegra/pm.c
> create mode 100644 arch/arm/mach-tegra/pm.h
>
> diff --git a/arch/arm/mach-tegra/Makefile b/arch/arm/mach-tegra/Makefile
> index 9b80c1e..6f224f7 100644
> --- a/arch/arm/mach-tegra/Makefile
> +++ b/arch/arm/mach-tegra/Makefile
> @@ -8,6 +8,7 @@ obj-y += pmc.o
> obj-y += flowctrl.o
> obj-y += powergate.o
> obj-y += apbio.o
> +obj-y += pm.o
> obj-$(CONFIG_CPU_IDLE) += cpuidle.o
> obj-$(CONFIG_CPU_IDLE) += sleep.o
> obj-$(CONFIG_ARCH_TEGRA_2x_SOC) += tegra20_clocks.o
> diff --git a/arch/arm/mach-tegra/cpuidle-tegra30.c b/arch/arm/mach-tegra/cpuidle-tegra30.c
> index 0f85df8..842fef4 100644
> --- a/arch/arm/mach-tegra/cpuidle-tegra30.c
> +++ b/arch/arm/mach-tegra/cpuidle-tegra30.c
> @@ -19,21 +19,94 @@
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/cpuidle.h>
> +#include <linux/cpu_pm.h>
> +#include <linux/clockchips.h>
>
> #include <asm/cpuidle.h>
> +#include <asm/proc-fns.h>
> +#include <asm/suspend.h>
> +
> +#include "pm.h"
> +#include "sleep.h"
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int tegra30_idle_lp2(struct cpuidle_device *dev,
> + struct cpuidle_driver *drv,
> + int index);
> +#endif
>
> static struct cpuidle_driver tegra_idle_driver = {
> .name = "tegra_idle",
> .owner = THIS_MODULE,
> .en_core_tk_irqen = 1,
> - .state_count = 1,
> + .state_count = 2,
> .states = {
> [0] = ARM_CPUIDLE_WFI_STATE_PWR(600),
> +#ifdef CONFIG_PM_SLEEP
> + [1] = {
> + .enter = tegra30_idle_lp2,
> + .exit_latency = 2000,
> + .target_residency = 2200,
> + .power_usage = 0,
> + .flags = CPUIDLE_FLAG_TIME_VALID,
> + .name = "LP2",
> + .desc = "CPU power-gate",
> + },
> +#endif
> },
> };
>
> static DEFINE_PER_CPU(struct cpuidle_device, tegra_idle_device);
>
> +#ifdef CONFIG_PM_SLEEP
> +static bool tegra30_idle_enter_lp2_cpu_n(struct cpuidle_device *dev,
> + struct cpuidle_driver *drv,
> + int index)
> +{
> +#ifdef CONFIG_SMP
> + clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &dev->cpu);
> +
> + smp_wmb();
> +
> + save_cpu_arch_register();
> +
> + cpu_suspend(0, tegra30_sleep_cpu_secondary_finish);
> +
> + restore_cpu_arch_register();
> +
> + clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &dev->cpu);
> +#endif
Can't you factor out this #ifdef out using an inline function ?
> +
> + return true;
> +}
> +
> +static int __cpuinit tegra30_idle_lp2(struct cpuidle_device *dev,
> + struct cpuidle_driver *drv,
> + int index)
> +{
> + bool entered_lp2 = false;
> +
> + local_fiq_disable();
> +
> + tegra_set_cpu_in_lp2(dev->cpu);
> + cpu_pm_enter();
> +
> + if (dev->cpu == 0)
Logical cpu 0 ? Or you need a HW cpu 0 check here ? If you boot on a CPU
that is different from HW CPU 0 (do not know if that's possible) you
might have a problem.
[...]
> +bool __cpuinit tegra_set_cpu_in_lp2(int cpu)
> +{
> + bool last_cpu = false;
> +
> + spin_lock(&tegra_lp2_lock);
> + BUG_ON(cpumask_test_cpu(cpu, &tegra_in_lp2));
> + cpumask_set_cpu(cpu, &tegra_in_lp2);
> +
> + /*
> + * Update the IRAM copy used by the reset handler. The IRAM copy
> + * can't use used directly by cpumask_set_cpu() because it uses
> + * LDREX/STREX which requires the addressed location to be inner
> + * cacheable and sharable which IRAM isn't.
> + */
> + writel(tegra_in_lp2.bits[0], tegra_cpu_lp2_mask);
> + dsb();
> +
> + if ((cpu == 0) && cpumask_equal(&tegra_in_lp2, cpu_online_mask))
> + last_cpu = true;
For cpu == 0, see above.
[...]
> +ENTRY(tegra_flush_l1_cache)
> + stmfd sp!, {r4-r5, r7, r9-r11, lr}
> + dmb @ ensure ordering
> +
> + /* Disable the data cache */
> + mrc p15, 0, r2, c1, c0, 0
> + bic r2, r2, #CR_C
> + dsb
> + mcr p15, 0, r2, c1, c0, 0
> +
> + /* Flush data cache */
> + mov r10, #0
> +#ifdef CONFIG_PREEMPT
> + save_and_disable_irqs_notrace r9 @ make cssr&csidr read atomic
> +#endif
> + mcr p15, 2, r10, c0, c0, 0 @ select current cache level in cssr
> + isb @ isb to sych the new cssr&csidr
> + mrc p15, 1, r1, c0, c0, 0 @ read the new csidr
> +#ifdef CONFIG_PREEMPT
> + restore_irqs_notrace r9
> +#endif
> + and r2, r1, #7 @ extract the length of the cache lines
> + add r2, r2, #4 @ add 4 (line length offset)
> + ldr r4, =0x3ff
> + ands r4, r4, r1, lsr #3 @ find maximum number on the way size
> + clz r5, r4 @ find bit position of way size increment
> + ldr r7, =0x7fff
> + ands r7, r7, r1, lsr #13 @ extract max number of the index size
> +loop2:
> + mov r9, r4 @ create working copy of max way size
> +loop3:
> + orr r11, r10, r9, lsl r5 @ factor way and cache number into r11
> + orr r11, r11, r7, lsl r2 @ factor index number into r11
> + mcr p15, 0, r11, c7, c14, 2 @ clean & invalidate by set/way
> + subs r9, r9, #1 @ decrement the way
> + bge loop3
> + subs r7, r7, #1 @ decrement the index
> + bge loop2
> +finished:
> + mov r10, #0 @ swith back to cache level 0
> + mcr p15, 2, r10, c0, c0, 0 @ select current cache level in cssr
> + dsb
> + isb
This code is already in the kernel in cache-v7.S, please use that.
We are just adding the new LoUIS API that probably does what you
want, even though for Tegra, that is an A9 based platform I fail to
understand why Level of Coherency differs from L1.
Can you explain to me please why Level of Coherency (LoC) is != from L1
on Tegra ?
Thanks,
Lorenzo
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH 3/7] ARM: tegra30: cpuidle: add LP2 driver for secondary CPUs
2012-10-08 16:35 ` Lorenzo Pieralisi
@ 2012-10-09 4:13 ` Joseph Lo
2012-10-09 8:38 ` Lorenzo Pieralisi
0 siblings, 1 reply; 50+ messages in thread
From: Joseph Lo @ 2012-10-09 4:13 UTC (permalink / raw)
To: linux-arm-kernel
Hi Lorenzo,
Thanks for your review.
On Tue, 2012-10-09 at 00:35 +0800, Lorenzo Pieralisi wrote:
> On Mon, Oct 08, 2012 at 11:26:17AM +0100, Joseph Lo wrote:
> > This supports power-gated (LP2) idle on secondary CPUs for Tegra30.
> > The secondary CPUs can go into LP2 state independently. When CPU goes
> > into LP2 state, it saves it's state and puts itself to flow controlled
> > WFI state. After that, it will been power gated.
> >
> > Based on the work by:
> > Scott Williams <scwilliams@nvidia.com>
> >
> > Signed-off-by: Joseph Lo <josephl@nvidia.com>
> > ---
...
> > +#ifdef CONFIG_PM_SLEEP
> > +static bool tegra30_idle_enter_lp2_cpu_n(struct cpuidle_device *dev,
> > + struct cpuidle_driver *drv,
> > + int index)
> > +{
> > +#ifdef CONFIG_SMP
> > + clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &dev->cpu);
> > +
> > + smp_wmb();
> > +
> > + save_cpu_arch_register();
> > +
> > + cpu_suspend(0, tegra30_sleep_cpu_secondary_finish);
> > +
> > + restore_cpu_arch_register();
> > +
> > + clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &dev->cpu);
> > +#endif
>
> Can't you factor out this #ifdef out using an inline function ?
>
OK. Will do.
> > +
> > + return true;
> > +}
> > +
> > +static int __cpuinit tegra30_idle_lp2(struct cpuidle_device *dev,
> > + struct cpuidle_driver *drv,
> > + int index)
> > +{
> > + bool entered_lp2 = false;
> > +
> > + local_fiq_disable();
> > +
> > + tegra_set_cpu_in_lp2(dev->cpu);
> > + cpu_pm_enter();
> > +
> > + if (dev->cpu == 0)
>
> Logical cpu 0 ? Or you need a HW cpu 0 check here ? If you boot on a CPU
> that is different from HW CPU 0 (do not know if that's possible) you
> might have a problem.
>
> [...]
>
For Tegra20 & Tegra30, it's always physical CPU 0 here. And the CPU0 was
always the first boot CPU. I will change to
cpu = cpu_logical_map(dev->cpu);
Thanks for your remind.
> > +bool __cpuinit tegra_set_cpu_in_lp2(int cpu)
> > +{
> > + bool last_cpu = false;
> > +
> > + spin_lock(&tegra_lp2_lock);
> > + BUG_ON(cpumask_test_cpu(cpu, &tegra_in_lp2));
> > + cpumask_set_cpu(cpu, &tegra_in_lp2);
> > +
> > + /*
> > + * Update the IRAM copy used by the reset handler. The IRAM copy
> > + * can't use used directly by cpumask_set_cpu() because it uses
> > + * LDREX/STREX which requires the addressed location to be inner
> > + * cacheable and sharable which IRAM isn't.
> > + */
> > + writel(tegra_in_lp2.bits[0], tegra_cpu_lp2_mask);
> > + dsb();
> > +
> > + if ((cpu == 0) && cpumask_equal(&tegra_in_lp2, cpu_online_mask))
> > + last_cpu = true;
>
> For cpu == 0, see above.
>
> [...]
>
Will use cpu_logical_map to get the physical CPU first, thanks.
> > +ENTRY(tegra_flush_l1_cache)
> > + stmfd sp!, {r4-r5, r7, r9-r11, lr}
> > + dmb @ ensure ordering
> > +
> > + /* Disable the data cache */
> > + mrc p15, 0, r2, c1, c0, 0
> > + bic r2, r2, #CR_C
> > + dsb
> > + mcr p15, 0, r2, c1, c0, 0
> > +
> > + /* Flush data cache */
> > + mov r10, #0
> > +#ifdef CONFIG_PREEMPT
> > + save_and_disable_irqs_notrace r9 @ make cssr&csidr read atomic
> > +#endif
> > + mcr p15, 2, r10, c0, c0, 0 @ select current cache level in cssr
> > + isb @ isb to sych the new cssr&csidr
> > + mrc p15, 1, r1, c0, c0, 0 @ read the new csidr
> > +#ifdef CONFIG_PREEMPT
> > + restore_irqs_notrace r9
> > +#endif
> > + and r2, r1, #7 @ extract the length of the cache lines
> > + add r2, r2, #4 @ add 4 (line length offset)
> > + ldr r4, =0x3ff
> > + ands r4, r4, r1, lsr #3 @ find maximum number on the way size
> > + clz r5, r4 @ find bit position of way size increment
> > + ldr r7, =0x7fff
> > + ands r7, r7, r1, lsr #13 @ extract max number of the index size
> > +loop2:
> > + mov r9, r4 @ create working copy of max way size
> > +loop3:
> > + orr r11, r10, r9, lsl r5 @ factor way and cache number into r11
> > + orr r11, r11, r7, lsl r2 @ factor index number into r11
> > + mcr p15, 0, r11, c7, c14, 2 @ clean & invalidate by set/way
> > + subs r9, r9, #1 @ decrement the way
> > + bge loop3
> > + subs r7, r7, #1 @ decrement the index
> > + bge loop2
> > +finished:
> > + mov r10, #0 @ swith back to cache level 0
> > + mcr p15, 2, r10, c0, c0, 0 @ select current cache level in cssr
> > + dsb
> > + isb
>
> This code is already in the kernel in cache-v7.S, please use that.
> We are just adding the new LoUIS API that probably does what you
> want, even though for Tegra, that is an A9 based platform I fail to
> understand why Level of Coherency differs from L1.
>
> Can you explain to me please why Level of Coherency (LoC) is != from L1
> on Tegra ?
>
Thanks for introducing the new LoUIS cache API. Did LoUIS been changed
by other HW? I checked the new LoUIS API. If LoUIS == 0, it means inner
shareable then it do nothing just return. But I need to flush L1 data
cache here to sync the coherency before CPU be power gated. And disable
data cache before flush is needed.
I can tell you the sequence that why we just do L1 data cache flush
here. Maybe I need to change the comment to "flush to point of
coherency" not "level of coherency".
For secondary CPUs:
* after cpu_suspend
* disable data cache and flush L1 data cache
* Turn off SMP coherency
* power gate CPU
For CPU0:
* outer_disable (flush and disable L2)
* cpu_suspend
* disable data cache and flush L1 data cache
* Turn off SMP coherency
* Turn off MMU
* shut off the CPU rail
So we only do flush to PoC.
And changing the sequence of secondary CPUs to belows maybe more
suitable?
* after cpu_suspend
* disable data cache and call to v7_flush_dcache_all
* Turn off SMP coherency
* power gate CPU
How do you think?
Thanks,
Joseph
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH 3/7] ARM: tegra30: cpuidle: add LP2 driver for secondary CPUs
2012-10-09 4:13 ` Joseph Lo
@ 2012-10-09 8:38 ` Lorenzo Pieralisi
2012-10-09 9:18 ` Joseph Lo
0 siblings, 1 reply; 50+ messages in thread
From: Lorenzo Pieralisi @ 2012-10-09 8:38 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Oct 09, 2012 at 05:13:15AM +0100, Joseph Lo wrote:
[...]
> > > +ENTRY(tegra_flush_l1_cache)
> > > + stmfd sp!, {r4-r5, r7, r9-r11, lr}
> > > + dmb @ ensure ordering
> > > +
> > > + /* Disable the data cache */
> > > + mrc p15, 0, r2, c1, c0, 0
> > > + bic r2, r2, #CR_C
> > > + dsb
> > > + mcr p15, 0, r2, c1, c0, 0
> > > +
> > > + /* Flush data cache */
> > > + mov r10, #0
> > > +#ifdef CONFIG_PREEMPT
> > > + save_and_disable_irqs_notrace r9 @ make cssr&csidr read atomic
> > > +#endif
> > > + mcr p15, 2, r10, c0, c0, 0 @ select current cache level in cssr
> > > + isb @ isb to sych the new cssr&csidr
> > > + mrc p15, 1, r1, c0, c0, 0 @ read the new csidr
> > > +#ifdef CONFIG_PREEMPT
> > > + restore_irqs_notrace r9
> > > +#endif
> > > + and r2, r1, #7 @ extract the length of the cache lines
> > > + add r2, r2, #4 @ add 4 (line length offset)
> > > + ldr r4, =0x3ff
> > > + ands r4, r4, r1, lsr #3 @ find maximum number on the way size
> > > + clz r5, r4 @ find bit position of way size increment
> > > + ldr r7, =0x7fff
> > > + ands r7, r7, r1, lsr #13 @ extract max number of the index size
> > > +loop2:
> > > + mov r9, r4 @ create working copy of max way size
> > > +loop3:
> > > + orr r11, r10, r9, lsl r5 @ factor way and cache number into r11
> > > + orr r11, r11, r7, lsl r2 @ factor index number into r11
> > > + mcr p15, 0, r11, c7, c14, 2 @ clean & invalidate by set/way
> > > + subs r9, r9, #1 @ decrement the way
> > > + bge loop3
> > > + subs r7, r7, #1 @ decrement the index
> > > + bge loop2
> > > +finished:
> > > + mov r10, #0 @ swith back to cache level 0
> > > + mcr p15, 2, r10, c0, c0, 0 @ select current cache level in cssr
> > > + dsb
> > > + isb
> >
> > This code is already in the kernel in cache-v7.S, please use that.
> > We are just adding the new LoUIS API that probably does what you
> > want, even though for Tegra, that is an A9 based platform I fail to
> > understand why Level of Coherency differs from L1.
> >
> > Can you explain to me please why Level of Coherency (LoC) is != from L1
> > on Tegra ?
> >
>
> Thanks for introducing the new LoUIS cache API. Did LoUIS been changed
> by other HW? I checked the new LoUIS API. If LoUIS == 0, it means inner
> shareable then it do nothing just return. But I need to flush L1 data
> cache here to sync the coherency before CPU be power gated. And disable
> data cache before flush is needed.
I understand that, that's why I am asking. To me LoUIS and LoC should
both be the same for A9 based platforms and they should both represent
a cache level that *includes* L1.
Can you provide me with the CLIDR value for Tegra3 please ?
>
> I can tell you the sequence that why we just do L1 data cache flush
> here. Maybe I need to change the comment to "flush to point of
> coherency" not "level of coherency".
>
> For secondary CPUs:
> * after cpu_suspend
> * disable data cache and flush L1 data cache
^(1)
> * Turn off SMP coherency
^(2)
Two steps above, one assembly function, no access to any data whatsoever
please.
> * power gate CPU
>
> For CPU0:
> * outer_disable (flush and disable L2)
I guess L2 cannot be retained on Tegra ?
> * cpu_suspend
> * disable data cache and flush L1 data cache
> * Turn off SMP coherency
> * Turn off MMU
> * shut off the CPU rail
>
> So we only do flush to PoC.
>
> And changing the sequence of secondary CPUs to belows maybe more
> suitable?
Yes, basically because the net effect should be identical.
Lorenzo
> * after cpu_suspend
> * disable data cache and call to v7_flush_dcache_all
> * Turn off SMP coherency
> * power gate CPU
>
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH 3/7] ARM: tegra30: cpuidle: add LP2 driver for secondary CPUs
2012-10-09 8:38 ` Lorenzo Pieralisi
@ 2012-10-09 9:18 ` Joseph Lo
2012-10-09 9:42 ` Lorenzo Pieralisi
0 siblings, 1 reply; 50+ messages in thread
From: Joseph Lo @ 2012-10-09 9:18 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, 2012-10-09 at 16:38 +0800, Lorenzo Pieralisi wrote:
> On Tue, Oct 09, 2012 at 05:13:15AM +0100, Joseph Lo wrote:
>
> [...]
>
> > > > +ENTRY(tegra_flush_l1_cache)
> > > > + stmfd sp!, {r4-r5, r7, r9-r11, lr}
> > > > + dmb @ ensure ordering
> > > > +
> > > > + /* Disable the data cache */
> > > > + mrc p15, 0, r2, c1, c0, 0
> > > > + bic r2, r2, #CR_C
> > > > + dsb
> > > > + mcr p15, 0, r2, c1, c0, 0
> > > > +
> > > > + /* Flush data cache */
> > > > + mov r10, #0
> > > > +#ifdef CONFIG_PREEMPT
> > > > + save_and_disable_irqs_notrace r9 @ make cssr&csidr read atomic
> > > > +#endif
> > > > + mcr p15, 2, r10, c0, c0, 0 @ select current cache level in cssr
> > > > + isb @ isb to sych the new cssr&csidr
> > > > + mrc p15, 1, r1, c0, c0, 0 @ read the new csidr
> > > > +#ifdef CONFIG_PREEMPT
> > > > + restore_irqs_notrace r9
> > > > +#endif
> > > > + and r2, r1, #7 @ extract the length of the cache lines
> > > > + add r2, r2, #4 @ add 4 (line length offset)
> > > > + ldr r4, =0x3ff
> > > > + ands r4, r4, r1, lsr #3 @ find maximum number on the way size
> > > > + clz r5, r4 @ find bit position of way size increment
> > > > + ldr r7, =0x7fff
> > > > + ands r7, r7, r1, lsr #13 @ extract max number of the index size
> > > > +loop2:
> > > > + mov r9, r4 @ create working copy of max way size
> > > > +loop3:
> > > > + orr r11, r10, r9, lsl r5 @ factor way and cache number into r11
> > > > + orr r11, r11, r7, lsl r2 @ factor index number into r11
> > > > + mcr p15, 0, r11, c7, c14, 2 @ clean & invalidate by set/way
> > > > + subs r9, r9, #1 @ decrement the way
> > > > + bge loop3
> > > > + subs r7, r7, #1 @ decrement the index
> > > > + bge loop2
> > > > +finished:
> > > > + mov r10, #0 @ swith back to cache level 0
> > > > + mcr p15, 2, r10, c0, c0, 0 @ select current cache level in cssr
> > > > + dsb
> > > > + isb
> > >
> > > This code is already in the kernel in cache-v7.S, please use that.
> > > We are just adding the new LoUIS API that probably does what you
> > > want, even though for Tegra, that is an A9 based platform I fail to
> > > understand why Level of Coherency differs from L1.
> > >
> > > Can you explain to me please why Level of Coherency (LoC) is != from L1
> > > on Tegra ?
> > >
> >
> > Thanks for introducing the new LoUIS cache API. Did LoUIS been changed
> > by other HW? I checked the new LoUIS API. If LoUIS == 0, it means inner
> > shareable then it do nothing just return. But I need to flush L1 data
> > cache here to sync the coherency before CPU be power gated. And disable
> > data cache before flush is needed.
>
> I understand that, that's why I am asking. To me LoUIS and LoC should
> both be the same for A9 based platforms and they should both represent
> a cache level that *includes* L1.
>
> Can you provide me with the CLIDR value for Tegra3 please ?
>
I just checked the CLIDR of both Tegra20 and Tegra30. It's 0x09200003.
It looks I can use this new LoUIS api, right? :)
I will give it a try and update to next version. Thanks.
> >
> > I can tell you the sequence that why we just do L1 data cache flush
> > here. Maybe I need to change the comment to "flush to point of
> > coherency" not "level of coherency".
> >
> > For secondary CPUs:
> > * after cpu_suspend
> > * disable data cache and flush L1 data cache
> ^(1)
> > * Turn off SMP coherency
> ^(2)
>
> Two steps above, one assembly function, no access to any data whatsoever
> please.
>
OK. Will do this and update to next version.
> > * power gate CPU
> >
> > For CPU0:
> > * outer_disable (flush and disable L2)
>
> I guess L2 cannot be retained on Tegra ?
>
Yes.
I will give it a test and update later.
Thanks,
Joseph
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH 3/7] ARM: tegra30: cpuidle: add LP2 driver for secondary CPUs
2012-10-09 9:18 ` Joseph Lo
@ 2012-10-09 9:42 ` Lorenzo Pieralisi
0 siblings, 0 replies; 50+ messages in thread
From: Lorenzo Pieralisi @ 2012-10-09 9:42 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Oct 09, 2012 at 10:18:57AM +0100, Joseph Lo wrote:
> On Tue, 2012-10-09 at 16:38 +0800, Lorenzo Pieralisi wrote:
> > On Tue, Oct 09, 2012 at 05:13:15AM +0100, Joseph Lo wrote:
> >
> > [...]
> >
> > > > > +ENTRY(tegra_flush_l1_cache)
> > > > > + stmfd sp!, {r4-r5, r7, r9-r11, lr}
> > > > > + dmb @ ensure ordering
> > > > > +
> > > > > + /* Disable the data cache */
> > > > > + mrc p15, 0, r2, c1, c0, 0
> > > > > + bic r2, r2, #CR_C
> > > > > + dsb
> > > > > + mcr p15, 0, r2, c1, c0, 0
> > > > > +
> > > > > + /* Flush data cache */
> > > > > + mov r10, #0
> > > > > +#ifdef CONFIG_PREEMPT
> > > > > + save_and_disable_irqs_notrace r9 @ make cssr&csidr read atomic
> > > > > +#endif
> > > > > + mcr p15, 2, r10, c0, c0, 0 @ select current cache level in cssr
> > > > > + isb @ isb to sych the new cssr&csidr
> > > > > + mrc p15, 1, r1, c0, c0, 0 @ read the new csidr
> > > > > +#ifdef CONFIG_PREEMPT
> > > > > + restore_irqs_notrace r9
> > > > > +#endif
> > > > > + and r2, r1, #7 @ extract the length of the cache lines
> > > > > + add r2, r2, #4 @ add 4 (line length offset)
> > > > > + ldr r4, =0x3ff
> > > > > + ands r4, r4, r1, lsr #3 @ find maximum number on the way size
> > > > > + clz r5, r4 @ find bit position of way size increment
> > > > > + ldr r7, =0x7fff
> > > > > + ands r7, r7, r1, lsr #13 @ extract max number of the index size
> > > > > +loop2:
> > > > > + mov r9, r4 @ create working copy of max way size
> > > > > +loop3:
> > > > > + orr r11, r10, r9, lsl r5 @ factor way and cache number into r11
> > > > > + orr r11, r11, r7, lsl r2 @ factor index number into r11
> > > > > + mcr p15, 0, r11, c7, c14, 2 @ clean & invalidate by set/way
> > > > > + subs r9, r9, #1 @ decrement the way
> > > > > + bge loop3
> > > > > + subs r7, r7, #1 @ decrement the index
> > > > > + bge loop2
> > > > > +finished:
> > > > > + mov r10, #0 @ swith back to cache level 0
> > > > > + mcr p15, 2, r10, c0, c0, 0 @ select current cache level in cssr
> > > > > + dsb
> > > > > + isb
> > > >
> > > > This code is already in the kernel in cache-v7.S, please use that.
> > > > We are just adding the new LoUIS API that probably does what you
> > > > want, even though for Tegra, that is an A9 based platform I fail to
> > > > understand why Level of Coherency differs from L1.
> > > >
> > > > Can you explain to me please why Level of Coherency (LoC) is != from L1
> > > > on Tegra ?
> > > >
> > >
> > > Thanks for introducing the new LoUIS cache API. Did LoUIS been changed
> > > by other HW? I checked the new LoUIS API. If LoUIS == 0, it means inner
> > > shareable then it do nothing just return. But I need to flush L1 data
> > > cache here to sync the coherency before CPU be power gated. And disable
> > > data cache before flush is needed.
> >
> > I understand that, that's why I am asking. To me LoUIS and LoC should
> > both be the same for A9 based platforms and they should both represent
> > a cache level that *includes* L1.
> >
> > Can you provide me with the CLIDR value for Tegra3 please ?
> >
>
> I just checked the CLIDR of both Tegra20 and Tegra30. It's 0x09200003.
> It looks I can use this new LoUIS api, right? :)
You do not have to, since LoUIS == LoC, but that would be appreciated.
If you execute:
v7_flush_dcache_all
That would do the same thing on current Tegra(s).
If you want to reuse the same code for future A15/A7 processors then yes,
v7_flush_dcache_louis
is what you want to call, and that works for current Tegra(s) as well since,
as I mentioned LoUIS == LoC there.
Overall, using the new LoUIS API for single core shutdown is the best
option, since it should work for current and future cores.
Thanks,
Lorenzo
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 3/7] ARM: tegra30: cpuidle: add LP2 driver for secondary CPUs
2012-10-08 10:26 ` [PATCH 3/7] ARM: tegra30: cpuidle: add LP2 driver for secondary CPUs Joseph Lo
2012-10-08 16:35 ` Lorenzo Pieralisi
@ 2012-10-09 22:38 ` Stephen Warren
2012-10-11 9:15 ` Joseph Lo
1 sibling, 1 reply; 50+ messages in thread
From: Stephen Warren @ 2012-10-09 22:38 UTC (permalink / raw)
To: linux-arm-kernel
On 10/08/2012 04:26 AM, Joseph Lo wrote:
> This supports power-gated (LP2) idle on secondary CPUs for Tegra30.
> The secondary CPUs can go into LP2 state independently. When CPU goes
> into LP2 state, it saves it's state and puts itself to flow controlled
> WFI state. After that, it will been power gated.
> diff --git a/arch/arm/mach-tegra/cpuidle-tegra30.c b/arch/arm/mach-tegra/cpuidle-tegra30.c
> static struct cpuidle_driver tegra_idle_driver = {
> .name = "tegra_idle",
> .owner = THIS_MODULE,
> .en_core_tk_irqen = 1,
> - .state_count = 1,
> + .state_count = 2,
Doesn't that assignment need to be ifdef'd just like the array entry
setup below:
> .states = {
> [0] = ARM_CPUIDLE_WFI_STATE_PWR(600),
> +#ifdef CONFIG_PM_SLEEP
> + [1] = {
> + .enter = tegra30_idle_lp2,
> + .exit_latency = 2000,
> + .target_residency = 2200,
> + .power_usage = 0,
> + .flags = CPUIDLE_FLAG_TIME_VALID,
> + .name = "LP2",
> + .desc = "CPU power-gate",
> + },
> +#endif
> },
> };
> @@ -41,6 +114,10 @@ int __init tegra30_cpuidle_init(void)
> struct cpuidle_device *dev;
> struct cpuidle_driver *drv = &tegra_idle_driver;
>
> +#ifndef CONFIG_PM_SLEEP
> + drv->state_count = 1; /* support clockgating only */
> +#endif
Oh, I see it's done here. Just fixing the static initialization seems a
lot simpler?
> diff --git a/arch/arm/mach-tegra/pm.c b/arch/arm/mach-tegra/pm.c
> +void __cpuinit tegra_clear_cpu_in_lp2(int cpu)
> +{
> + spin_lock(&tegra_lp2_lock);
> + BUG_ON(!cpumask_test_cpu(cpu, &tegra_in_lp2));
> + cpumask_clear_cpu(cpu, &tegra_in_lp2);
> +
> + /*
> + * Update the IRAM copy used by the reset handler. The IRAM copy
> + * can't use used directly by cpumask_clear_cpu() because it uses
> + * LDREX/STREX which requires the addressed location to be inner
> + * cacheable and sharable which IRAM isn't.
> + */
> + writel(tegra_in_lp2.bits[0], tegra_cpu_lp2_mask);
> + dsb();
Why not /just/ store the data in IRAM, and read/write directly to it,
rather than maintaining an SDRAM-based copy of it?
Then, wouldn't the body of this function be simply:
spin_lock();
BUG_ON(!(tegra_cpu_lp2_mask & BIT(cpu)));
tegra_cpu_lp2_mask |= BIT(cpu);
spin_unlock();
> +bool __cpuinit tegra_set_cpu_in_lp2(int cpu)
Similar comment here.
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH 3/7] ARM: tegra30: cpuidle: add LP2 driver for secondary CPUs
2012-10-09 22:38 ` Stephen Warren
@ 2012-10-11 9:15 ` Joseph Lo
2012-10-11 16:24 ` Stephen Warren
0 siblings, 1 reply; 50+ messages in thread
From: Joseph Lo @ 2012-10-11 9:15 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, 2012-10-10 at 06:38 +0800, Stephen Warren wrote:
> On 10/08/2012 04:26 AM, Joseph Lo wrote:
> > This supports power-gated (LP2) idle on secondary CPUs for Tegra30.
> > The secondary CPUs can go into LP2 state independently. When CPU goes
> > into LP2 state, it saves it's state and puts itself to flow controlled
> > WFI state. After that, it will been power gated.
>
> > diff --git a/arch/arm/mach-tegra/cpuidle-tegra30.c b/arch/arm/mach-tegra/cpuidle-tegra30.c
>
> > static struct cpuidle_driver tegra_idle_driver = {
> > .name = "tegra_idle",
> > .owner = THIS_MODULE,
> > .en_core_tk_irqen = 1,
> > - .state_count = 1,
> > + .state_count = 2,
>
> Doesn't that assignment need to be ifdef'd just like the array entry
> setup below:
>
> > .states = {
> > [0] = ARM_CPUIDLE_WFI_STATE_PWR(600),
> > +#ifdef CONFIG_PM_SLEEP
> > + [1] = {
> > + .enter = tegra30_idle_lp2,
> > + .exit_latency = 2000,
> > + .target_residency = 2200,
> > + .power_usage = 0,
> > + .flags = CPUIDLE_FLAG_TIME_VALID,
> > + .name = "LP2",
> > + .desc = "CPU power-gate",
> > + },
> > +#endif
> > },
> > };
>
> > @@ -41,6 +114,10 @@ int __init tegra30_cpuidle_init(void)
> > struct cpuidle_device *dev;
> > struct cpuidle_driver *drv = &tegra_idle_driver;
> >
> > +#ifndef CONFIG_PM_SLEEP
> > + drv->state_count = 1; /* support clockgating only */
> > +#endif
>
> Oh, I see it's done here. Just fixing the static initialization seems a
> lot simpler?
>
OK. Will do.
> > diff --git a/arch/arm/mach-tegra/pm.c b/arch/arm/mach-tegra/pm.c
>
> > +void __cpuinit tegra_clear_cpu_in_lp2(int cpu)
> > +{
> > + spin_lock(&tegra_lp2_lock);
> > + BUG_ON(!cpumask_test_cpu(cpu, &tegra_in_lp2));
> > + cpumask_clear_cpu(cpu, &tegra_in_lp2);
> > +
> > + /*
> > + * Update the IRAM copy used by the reset handler. The IRAM copy
> > + * can't use used directly by cpumask_clear_cpu() because it uses
> > + * LDREX/STREX which requires the addressed location to be inner
> > + * cacheable and sharable which IRAM isn't.
> > + */
> > + writel(tegra_in_lp2.bits[0], tegra_cpu_lp2_mask);
> > + dsb();
>
> Why not /just/ store the data in IRAM, and read/write directly to it,
> rather than maintaining an SDRAM-based copy of it?
>
> Then, wouldn't the body of this function be simply:
>
> spin_lock();
> BUG_ON(!(tegra_cpu_lp2_mask & BIT(cpu)));
> tegra_cpu_lp2_mask |= BIT(cpu);
> spin_unlock();
>
It may not simple like this. To maintain it identical to a cpumask. It
may look likes below. Because I need to compare it with cpu_online_mask.
void __cpuinit tegra_clear_cpu_in_lp2(int phy_cpu_id)
{
cpumask_t *iram_cpu_lp2_mask = tegra_cpu_lp2_mask;
unsigned long *addr;
spin_lock(&tegra_lp2_lock);
addr = cpumask_bits(iram_cpu_lp2_mask);
BUG_ON(!(1UL &
(addr[BIT_WORD(phy_cpu_id)]
>> (phy_cpu_id & (BITS_PER_LONG-1)))));
*(addr + BIT_WORD(phy_cpu_id)) &= ~BIT_MASK(phy_cpu_id);
spin_unlock(&tegra_lp2_lock);
}
> > +bool __cpuinit tegra_set_cpu_in_lp2(int cpu)
>
> Similar comment here.
bool __cpuinit tegra_set_cpu_in_lp2(int phy_cpu_id)
{
bool last_cpu = false;
cpumask_t *iram_cpu_lp2_mask = tegra_cpu_lp2_mask;
unsigned long *addr;
spin_lock(&tegra_lp2_lock);
addr = cpumask_bits(iram_cpu_lp2_mask);
BUG_ON((1UL &
(addr[BIT_WORD(phy_cpu_id)]
>> (phy_cpu_id & (BITS_PER_LONG-1)))));
*(addr + BIT_WORD(phy_cpu_id)) |= BIT_MASK(phy_cpu_id);
if ((phy_cpu_id == 0) &&
cpumask_equal(iram_cpu_lp2_mask, cpu_online_mask))
last_cpu = true;
spin_unlock(&tegra_lp2_lock);
return last_cpu;
}
So I think the original version should more easy to understand. How do
you think?
Thanks,
Joseph
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH 3/7] ARM: tegra30: cpuidle: add LP2 driver for secondary CPUs
2012-10-11 9:15 ` Joseph Lo
@ 2012-10-11 16:24 ` Stephen Warren
2012-10-12 3:21 ` Joseph Lo
0 siblings, 1 reply; 50+ messages in thread
From: Stephen Warren @ 2012-10-11 16:24 UTC (permalink / raw)
To: linux-arm-kernel
On 10/11/2012 03:15 AM, Joseph Lo wrote:
> On Wed, 2012-10-10 at 06:38 +0800, Stephen Warren wrote:
>> On 10/08/2012 04:26 AM, Joseph Lo wrote:
>>> This supports power-gated (LP2) idle on secondary CPUs for Tegra30.
>>> The secondary CPUs can go into LP2 state independently. When CPU goes
>>> into LP2 state, it saves it's state and puts itself to flow controlled
>>> WFI state. After that, it will been power gated.
>>
>>> diff --git a/arch/arm/mach-tegra/cpuidle-tegra30.c b/arch/arm/mach-tegra/cpuidle-tegra30.c
>>
>>> static struct cpuidle_driver tegra_idle_driver = {
>>> .name = "tegra_idle",
>>> .owner = THIS_MODULE,
>>> .en_core_tk_irqen = 1,
>>> - .state_count = 1,
>>> + .state_count = 2,
>>
>> Doesn't that assignment need to be ifdef'd just like the array entry
>> setup below:
>>
>>> .states = {
>>> [0] = ARM_CPUIDLE_WFI_STATE_PWR(600),
>>> +#ifdef CONFIG_PM_SLEEP
>>> + [1] = {
>>> + .enter = tegra30_idle_lp2,
>>> + .exit_latency = 2000,
>>> + .target_residency = 2200,
>>> + .power_usage = 0,
>>> + .flags = CPUIDLE_FLAG_TIME_VALID,
>>> + .name = "LP2",
>>> + .desc = "CPU power-gate",
>>> + },
>>> +#endif
>>> },
>>> };
>>
>>> @@ -41,6 +114,10 @@ int __init tegra30_cpuidle_init(void)
>>> struct cpuidle_device *dev;
>>> struct cpuidle_driver *drv = &tegra_idle_driver;
>>>
>>> +#ifndef CONFIG_PM_SLEEP
>>> + drv->state_count = 1; /* support clockgating only */
>>> +#endif
>>
>> Oh, I see it's done here. Just fixing the static initialization seems a
>> lot simpler?
>>
> OK. Will do.
>
>>> diff --git a/arch/arm/mach-tegra/pm.c b/arch/arm/mach-tegra/pm.c
>>
>>> +void __cpuinit tegra_clear_cpu_in_lp2(int cpu)
>>> +{
>>> + spin_lock(&tegra_lp2_lock);
>>> + BUG_ON(!cpumask_test_cpu(cpu, &tegra_in_lp2));
>>> + cpumask_clear_cpu(cpu, &tegra_in_lp2);
>>> +
>>> + /*
>>> + * Update the IRAM copy used by the reset handler. The IRAM copy
>>> + * can't use used directly by cpumask_clear_cpu() because it uses
>>> + * LDREX/STREX which requires the addressed location to be inner
>>> + * cacheable and sharable which IRAM isn't.
>>> + */
>>> + writel(tegra_in_lp2.bits[0], tegra_cpu_lp2_mask);
>>> + dsb();
>>
>> Why not /just/ store the data in IRAM, and read/write directly to it,
>> rather than maintaining an SDRAM-based copy of it?
>>
>> Then, wouldn't the body of this function be simply:
>>
>> spin_lock();
>> BUG_ON(!(tegra_cpu_lp2_mask & BIT(cpu)));
>> tegra_cpu_lp2_mask |= BIT(cpu);
>> spin_unlock();
>>
>
> It may not simple like this. To maintain it identical to a cpumask. It
> may look likes below. Because I need to compare it with cpu_online_mask.
Oh, the comparison against cpu_online_mask() is what I was missing. I
guess that offline CPUs don't go into LP2, so you can't just check that
tegra_cpu_lp2_mask == (1 << num_cpus()) - 1.
One way to avoid that might be to maintain a cpu_in_lp2_count variable
alongside the mask, and simply compare that against num_online_cpus()
rather than comparing the two masks. At least that would avoid the
following line:
>>> + writel(tegra_in_lp2.bits[0], tegra_cpu_lp2_mask);
... making use of knowledge of the internal structure of the struct
cpumask type.
However, given the comparison requirement, either way is probably fine.
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH 3/7] ARM: tegra30: cpuidle: add LP2 driver for secondary CPUs
2012-10-11 16:24 ` Stephen Warren
@ 2012-10-12 3:21 ` Joseph Lo
[not found] ` <87sj8vr517.fsf@amiettinen-lnx.nvidia.com>
0 siblings, 1 reply; 50+ messages in thread
From: Joseph Lo @ 2012-10-12 3:21 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, 2012-10-12 at 00:24 +0800, Stephen Warren wrote:
> On 10/11/2012 03:15 AM, Joseph Lo wrote:
> > On Wed, 2012-10-10 at 06:38 +0800, Stephen Warren wrote:
> >> On 10/08/2012 04:26 AM, Joseph Lo wrote:
> >>> This supports power-gated (LP2) idle on secondary CPUs for Tegra30.
> >>> The secondary CPUs can go into LP2 state independently. When CPU goes
> >>> into LP2 state, it saves it's state and puts itself to flow controlled
> >>> WFI state. After that, it will been power gated.
> >>
> >>> diff --git a/arch/arm/mach-tegra/pm.c b/arch/arm/mach-tegra/pm.c
> >>
> >>> +void __cpuinit tegra_clear_cpu_in_lp2(int cpu)
> >>> +{
> >>> + spin_lock(&tegra_lp2_lock);
> >>> + BUG_ON(!cpumask_test_cpu(cpu, &tegra_in_lp2));
> >>> + cpumask_clear_cpu(cpu, &tegra_in_lp2);
> >>> +
> >>> + /*
> >>> + * Update the IRAM copy used by the reset handler. The IRAM copy
> >>> + * can't use used directly by cpumask_clear_cpu() because it uses
> >>> + * LDREX/STREX which requires the addressed location to be inner
> >>> + * cacheable and sharable which IRAM isn't.
> >>> + */
> >>> + writel(tegra_in_lp2.bits[0], tegra_cpu_lp2_mask);
> >>> + dsb();
> >>
> >> Why not /just/ store the data in IRAM, and read/write directly to it,
> >> rather than maintaining an SDRAM-based copy of it?
> >>
> >> Then, wouldn't the body of this function be simply:
> >>
> >> spin_lock();
> >> BUG_ON(!(tegra_cpu_lp2_mask & BIT(cpu)));
> >> tegra_cpu_lp2_mask |= BIT(cpu);
> >> spin_unlock();
> >>
> >
> > It may not simple like this. To maintain it identical to a cpumask. It
> > may look likes below. Because I need to compare it with cpu_online_mask.
>
> Oh, the comparison against cpu_online_mask() is what I was missing. I
> guess that offline CPUs don't go into LP2, so you can't just check that
> tegra_cpu_lp2_mask == (1 << num_cpus()) - 1.
>
> One way to avoid that might be to maintain a cpu_in_lp2_count variable
> alongside the mask, and simply compare that against num_online_cpus()
> rather than comparing the two masks. At least that would avoid the
> following line:
>
> >>> + writel(tegra_in_lp2.bits[0], tegra_cpu_lp2_mask);
>
> ... making use of knowledge of the internal structure of the struct
> cpumask type.
>
> However, given the comparison requirement, either way is probably fine.
OK. I got your idea now. And I rechecked it today. The method that you
suggested original could satisfy the usage here. It could maintain the
CPUs go into LP2 as a cpumask. I just verified it. Will update in next
version.
Thanks,
Joseph
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 4/7] ARM: tegra30: common: enable csite clock
2012-10-08 10:26 [PATCH 0/7] ARM: tegra30: cpuidle: add LP2 support Joseph Lo
` (2 preceding siblings ...)
2012-10-08 10:26 ` [PATCH 3/7] ARM: tegra30: cpuidle: add LP2 driver for secondary CPUs Joseph Lo
@ 2012-10-08 10:26 ` Joseph Lo
2012-10-09 22:38 ` Stephen Warren
2012-10-08 10:26 ` [PATCH 5/7] ARM: tegra30: clocks: add CPU low-power function into tegra_cpu_car_ops Joseph Lo
` (3 subsequent siblings)
7 siblings, 1 reply; 50+ messages in thread
From: Joseph Lo @ 2012-10-08 10:26 UTC (permalink / raw)
To: linux-arm-kernel
Enable csite (debug and trace controller) clock at init to prevent it
be disabled. And this also the necessary clock for CPU be brought up or
resumed from a power-gate low power state (e.g., LP2).
Signed-off-by: Joseph Lo <josephl@nvidia.com>
---
arch/arm/mach-tegra/common.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-tegra/common.c b/arch/arm/mach-tegra/common.c
index 0b0a5f5..4a2dd98 100644
--- a/arch/arm/mach-tegra/common.c
+++ b/arch/arm/mach-tegra/common.c
@@ -104,6 +104,7 @@ static __initdata struct tegra_clk_init_table tegra30_clk_init_table[] = {
{ "clk_m", NULL, 0, true },
{ "pll_p", "clk_m", 408000000, true },
{ "pll_p_out1", "pll_p", 9600000, true },
+ { "csite", NULL, 0, true },
{ NULL, NULL, 0, 0},
};
#endif
--
1.7.0.4
^ permalink raw reply related [flat|nested] 50+ messages in thread* [PATCH 4/7] ARM: tegra30: common: enable csite clock
2012-10-08 10:26 ` [PATCH 4/7] ARM: tegra30: common: enable csite clock Joseph Lo
@ 2012-10-09 22:38 ` Stephen Warren
2012-10-11 10:28 ` Joseph Lo
0 siblings, 1 reply; 50+ messages in thread
From: Stephen Warren @ 2012-10-09 22:38 UTC (permalink / raw)
To: linux-arm-kernel
On 10/08/2012 04:26 AM, Joseph Lo wrote:
> Enable csite (debug and trace controller) clock at init to prevent it
> be disabled. And this also the necessary clock for CPU be brought up or
> resumed from a power-gate low power state (e.g., LP2).
Does it make sense to enable this clock only when entering LP2? Or do we
really need to keep it on 100% of the time?
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 4/7] ARM: tegra30: common: enable csite clock
2012-10-09 22:38 ` Stephen Warren
@ 2012-10-11 10:28 ` Joseph Lo
0 siblings, 0 replies; 50+ messages in thread
From: Joseph Lo @ 2012-10-11 10:28 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, 2012-10-10 at 06:38 +0800, Stephen Warren wrote:
> On 10/08/2012 04:26 AM, Joseph Lo wrote:
> > Enable csite (debug and trace controller) clock at init to prevent it
> > be disabled. And this also the necessary clock for CPU be brought up or
> > resumed from a power-gate low power state (e.g., LP2).
>
> Does it make sense to enable this clock only when entering LP2? Or do we
> really need to keep it on 100% of the time?
Hmmm. I am not sure does the RealView or Lauterbach ICE can still attach
to the target if the clock is not available. Or even it can be attached
but some functions of the debugger may not work. Because the CoreSight
is a very fancy debug module.
I only test it with a light weight debugger (OpenOCD). It still can work
without turning on the CoreSight clock. Maybe it just because the
openocd debugger only using the jtag interface.
Anyway, I think we need the csite always on for the developer that using
RealView or Lauterbach ICE.
Thanks,
Joseph
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 5/7] ARM: tegra30: clocks: add CPU low-power function into tegra_cpu_car_ops
2012-10-08 10:26 [PATCH 0/7] ARM: tegra30: cpuidle: add LP2 support Joseph Lo
` (3 preceding siblings ...)
2012-10-08 10:26 ` [PATCH 4/7] ARM: tegra30: common: enable csite clock Joseph Lo
@ 2012-10-08 10:26 ` Joseph Lo
2012-10-08 10:26 ` [PATCH 6/7] ARM: tegra30: flowctrl: add cpu_suspend_exter/exit function Joseph Lo
` (2 subsequent siblings)
7 siblings, 0 replies; 50+ messages in thread
From: Joseph Lo @ 2012-10-08 10:26 UTC (permalink / raw)
To: linux-arm-kernel
Add suspend, resume and rail_off_ready API into tegra_cpu_car_ops. These
functions were used for CPU low-power state maintenance (e.g., LP2). One
thing needs to notice the rail_off_ready API only availalbe for cpu_g
cluster not cpu_lp cluster.
Signed-off-by: Joseph Lo <josephl@nvidia.com>
---
arch/arm/mach-tegra/tegra30_clocks.c | 107 ++++++++++++++++++++++++++++++++++
arch/arm/mach-tegra/tegra_cpu_car.h | 37 ++++++++++++
2 files changed, 144 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-tegra/tegra30_clocks.c b/arch/arm/mach-tegra/tegra30_clocks.c
index 5cd502c..23f513b 100644
--- a/arch/arm/mach-tegra/tegra30_clocks.c
+++ b/arch/arm/mach-tegra/tegra30_clocks.c
@@ -32,6 +32,7 @@
#include <asm/clkdev.h>
#include <mach/iomap.h>
+#include <mach/powergate.h>
#include "clock.h"
#include "fuse.h"
@@ -310,6 +311,31 @@
#define CPU_CLOCK(cpu) (0x1 << (8 + cpu))
#define CPU_RESET(cpu) (0x1111ul << (cpu))
+#define CLK_RESET_CCLK_BURST 0x20
+#define CLK_RESET_CCLK_DIVIDER 0x24
+#define CLK_RESET_PLLX_BASE 0xe0
+#define CLK_RESET_PLLX_MISC 0xe4
+
+#define CLK_RESET_SOURCE_CSITE 0x1d4
+
+#define CLK_RESET_CCLK_BURST_POLICY_SHIFT 28
+#define CLK_RESET_CCLK_RUN_POLICY_SHIFT 4
+#define CLK_RESET_CCLK_IDLE_POLICY_SHIFT 0
+#define CLK_RESET_CCLK_IDLE_POLICY 1
+#define CLK_RESET_CCLK_RUN_POLICY 2
+#define CLK_RESET_CCLK_BURST_POLICY_PLLX 8
+
+#ifdef CONFIG_PM_SLEEP
+static struct cpu_clk_suspend_context {
+ u32 pllx_misc;
+ u32 pllx_base;
+
+ u32 cpu_burst;
+ u32 clk_csite_src;
+ u32 cclk_divider;
+} tegra30_cpu_clk_sctx;
+#endif
+
/**
* Structure defining the fields for USB UTMI clocks Parameters.
*/
@@ -2281,12 +2307,93 @@ static void tegra30_disable_cpu_clock(u32 cpu)
reg_clk_base + TEGRA_CLK_RST_CONTROLLER_CLK_CPU_CMPLX);
}
+#ifdef CONFIG_PM_SLEEP
+static bool tegra30_cpu_rail_off_ready(void)
+{
+ unsigned int cpu_rst_status;
+ int cpu_pwr_status;
+
+ cpu_rst_status = readl(reg_clk_base +
+ TEGRA30_CLK_RST_CONTROLLER_CPU_CMPLX_STATUS);
+ cpu_pwr_status = tegra_powergate_is_powered(TEGRA_POWERGATE_CPU1) ||
+ tegra_powergate_is_powered(TEGRA_POWERGATE_CPU2) ||
+ tegra_powergate_is_powered(TEGRA_POWERGATE_CPU3);
+
+ if (((cpu_rst_status & 0xE) != 0xE) || cpu_pwr_status)
+ return false;
+
+ return true;
+}
+
+static void tegra30_cpu_clock_suspend(void)
+{
+ /* switch coresite to clk_m, save off original source */
+ tegra30_cpu_clk_sctx.clk_csite_src =
+ readl(reg_clk_base + CLK_RESET_SOURCE_CSITE);
+ writel(3<<30, reg_clk_base + CLK_RESET_SOURCE_CSITE);
+
+ tegra30_cpu_clk_sctx.cpu_burst =
+ readl(reg_clk_base + CLK_RESET_CCLK_BURST);
+ tegra30_cpu_clk_sctx.pllx_base =
+ readl(reg_clk_base + CLK_RESET_PLLX_BASE);
+ tegra30_cpu_clk_sctx.pllx_misc =
+ readl(reg_clk_base + CLK_RESET_PLLX_MISC);
+ tegra30_cpu_clk_sctx.cclk_divider =
+ readl(reg_clk_base + CLK_RESET_CCLK_DIVIDER);
+}
+
+static void tegra30_cpu_clock_resume(void)
+{
+ unsigned int reg, policy;
+
+ /* Is CPU complex already running on PLLX? */
+ reg = readl(reg_clk_base + CLK_RESET_CCLK_BURST);
+ policy = (reg >> CLK_RESET_CCLK_BURST_POLICY_SHIFT) & 0xF;
+
+ if (policy == CLK_RESET_CCLK_IDLE_POLICY)
+ reg = (reg >> CLK_RESET_CCLK_IDLE_POLICY_SHIFT) & 0xF;
+ else if (policy == CLK_RESET_CCLK_RUN_POLICY)
+ reg = (reg >> CLK_RESET_CCLK_RUN_POLICY_SHIFT) & 0xF;
+ else
+ BUG();
+
+ if (reg != CLK_RESET_CCLK_BURST_POLICY_PLLX) {
+ /* restore PLLX settings if CPU is on different PLL */
+ writel(tegra30_cpu_clk_sctx.pllx_misc,
+ reg_clk_base + CLK_RESET_PLLX_MISC);
+ writel(tegra30_cpu_clk_sctx.pllx_base,
+ reg_clk_base + CLK_RESET_PLLX_BASE);
+
+ /* wait for PLL stabilization if PLLX was enabled */
+ if (tegra30_cpu_clk_sctx.pllx_base & (1 << 30))
+ udelay(300);
+ }
+
+ /*
+ * Restore original burst policy setting for calls resulting from CPU
+ * LP2 in idle or system suspend.
+ */
+ writel(tegra30_cpu_clk_sctx.cclk_divider,
+ reg_clk_base + CLK_RESET_CCLK_DIVIDER);
+ writel(tegra30_cpu_clk_sctx.cpu_burst,
+ reg_clk_base + CLK_RESET_CCLK_BURST);
+
+ writel(tegra30_cpu_clk_sctx.clk_csite_src,
+ reg_clk_base + CLK_RESET_SOURCE_CSITE);
+}
+#endif
+
static struct tegra_cpu_car_ops tegra30_cpu_car_ops = {
.wait_for_reset = tegra30_wait_cpu_in_reset,
.put_in_reset = tegra30_put_cpu_in_reset,
.out_of_reset = tegra30_cpu_out_of_reset,
.enable_clock = tegra30_enable_cpu_clock,
.disable_clock = tegra30_disable_cpu_clock,
+#ifdef CONFIG_PM_SLEEP
+ .rail_off_ready = tegra30_cpu_rail_off_ready,
+ .suspend = tegra30_cpu_clock_suspend,
+ .resume = tegra30_cpu_clock_resume,
+#endif
};
void __init tegra30_cpu_car_ops_init(void)
diff --git a/arch/arm/mach-tegra/tegra_cpu_car.h b/arch/arm/mach-tegra/tegra_cpu_car.h
index 30d063a..9764d31 100644
--- a/arch/arm/mach-tegra/tegra_cpu_car.h
+++ b/arch/arm/mach-tegra/tegra_cpu_car.h
@@ -30,6 +30,12 @@
* CPU clock un-gate
* disable_clock:
* CPU clock gate
+ * rail_off_ready:
+ * CPU is ready for rail off
+ * suspend:
+ * save the clock settings when CPU go into low-power state
+ * resume:
+ * restore the clock settings when CPU exit low-power state
*/
struct tegra_cpu_car_ops {
void (*wait_for_reset)(u32 cpu);
@@ -37,6 +43,11 @@ struct tegra_cpu_car_ops {
void (*out_of_reset)(u32 cpu);
void (*enable_clock)(u32 cpu);
void (*disable_clock)(u32 cpu);
+#ifdef CONFIG_PM_SLEEP
+ bool (*rail_off_ready)(void);
+ void (*suspend)(void);
+ void (*resume)(void);
+#endif
};
extern struct tegra_cpu_car_ops *tegra_cpu_car_ops;
@@ -81,6 +92,32 @@ static inline void tegra_disable_cpu_clock(u32 cpu)
tegra_cpu_car_ops->disable_clock(cpu);
}
+#ifdef CONFIG_PM_SLEEP
+static inline bool tegra_cpu_rail_off_ready(void)
+{
+ if (WARN_ON(!tegra_cpu_car_ops->rail_off_ready))
+ return false;
+
+ return tegra_cpu_car_ops->rail_off_ready();
+}
+
+static inline void tegra_cpu_clock_suspend(void)
+{
+ if (WARN_ON(!tegra_cpu_car_ops->suspend))
+ return;
+
+ tegra_cpu_car_ops->suspend();
+}
+
+static inline void tegra_cpu_clock_resume(void)
+{
+ if (WARN_ON(!tegra_cpu_car_ops->resume))
+ return;
+
+ tegra_cpu_car_ops->resume();
+}
+#endif
+
void tegra20_cpu_car_ops_init(void);
void tegra30_cpu_car_ops_init(void);
--
1.7.0.4
^ permalink raw reply related [flat|nested] 50+ messages in thread* [PATCH 6/7] ARM: tegra30: flowctrl: add cpu_suspend_exter/exit function
2012-10-08 10:26 [PATCH 0/7] ARM: tegra30: cpuidle: add LP2 support Joseph Lo
` (4 preceding siblings ...)
2012-10-08 10:26 ` [PATCH 5/7] ARM: tegra30: clocks: add CPU low-power function into tegra_cpu_car_ops Joseph Lo
@ 2012-10-08 10:26 ` Joseph Lo
2012-10-08 10:26 ` [PATCH 7/7] ARM: tegra30: cpuidle: add LP2 driver for CPU0 Joseph Lo
2012-10-09 22:26 ` [PATCH 0/7] ARM: tegra30: cpuidle: add LP2 support Stephen Warren
7 siblings, 0 replies; 50+ messages in thread
From: Joseph Lo @ 2012-10-08 10:26 UTC (permalink / raw)
To: linux-arm-kernel
The flow controller can help CPU to go into suspend mode (deep power
saving mode). When CPU go into deep power saving mode, it needs some
careful settings before getting into and after leaving. The enter and
exit functions do that by configuring appropriate mode for flow
controller.
Signed-off-by: Joseph Lo <josephl@nvidia.com>
---
arch/arm/mach-tegra/flowctrl.c | 47 ++++++++++++++++++++++++++++++++++++++++
arch/arm/mach-tegra/flowctrl.h | 8 ++++++
2 files changed, 55 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-tegra/flowctrl.c b/arch/arm/mach-tegra/flowctrl.c
index f07488e..5967b08 100644
--- a/arch/arm/mach-tegra/flowctrl.c
+++ b/arch/arm/mach-tegra/flowctrl.c
@@ -21,6 +21,7 @@
#include <linux/init.h>
#include <linux/kernel.h>
#include <linux/io.h>
+#include <linux/cpumask.h>
#include <mach/iomap.h>
@@ -51,6 +52,14 @@ static void flowctrl_update(u8 offset, u32 value)
readl_relaxed(addr);
}
+u32 flowctrl_read_cpu_csr(unsigned int cpuid)
+{
+ u8 offset = flowctrl_offset_cpu_csr[cpuid];
+ void __iomem *addr = IO_ADDRESS(TEGRA_FLOW_CTRL_BASE) + offset;
+
+ return readl(addr);
+}
+
void flowctrl_write_cpu_csr(unsigned int cpuid, u32 value)
{
return flowctrl_update(flowctrl_offset_cpu_csr[cpuid], value);
@@ -60,3 +69,41 @@ void flowctrl_write_cpu_halt(unsigned int cpuid, u32 value)
{
return flowctrl_update(flowctrl_offset_halt_cpu[cpuid], value);
}
+
+void flowctrl_cpu_suspend_enter(unsigned int cpuid)
+{
+ unsigned int reg;
+ int i;
+
+ reg = flowctrl_read_cpu_csr(cpuid);
+ reg &= ~TEGRA30_FLOW_CTRL_CSR_WFE_BITMAP; /* clear wfe bitmap */
+ reg &= ~TEGRA30_FLOW_CTRL_CSR_WFI_BITMAP; /* clear wfi bitmap */
+ reg |= FLOW_CTRL_CSR_INTR_FLAG; /* clear intr flag */
+ reg |= FLOW_CTRL_CSR_EVENT_FLAG; /* clear event flag */
+ reg |= TEGRA30_FLOW_CTRL_CSR_WFI_CPU0 << cpuid; /* pwr gating on wfi */
+ reg |= FLOW_CTRL_CSR_ENABLE; /* pwr gating */
+ flowctrl_write_cpu_csr(cpuid, reg);
+
+ for (i = 0; i < num_possible_cpus(); i++) {
+ if (i == cpuid)
+ continue;
+ reg = flowctrl_read_cpu_csr(i);
+ reg |= FLOW_CTRL_CSR_EVENT_FLAG;
+ reg |= FLOW_CTRL_CSR_INTR_FLAG;
+ flowctrl_write_cpu_csr(i, reg);
+ }
+}
+
+void flowctrl_cpu_suspend_exit(unsigned int cpuid)
+{
+ unsigned int reg;
+
+ /* Disable powergating via flow controller for CPU0 */
+ reg = flowctrl_read_cpu_csr(cpuid);
+ reg &= ~TEGRA30_FLOW_CTRL_CSR_WFE_BITMAP; /* clear wfe bitmap */
+ reg &= ~TEGRA30_FLOW_CTRL_CSR_WFI_BITMAP; /* clear wfi bitmap */
+ reg &= ~FLOW_CTRL_CSR_ENABLE; /* clear enable */
+ reg |= FLOW_CTRL_CSR_INTR_FLAG; /* clear intr */
+ reg |= FLOW_CTRL_CSR_EVENT_FLAG; /* clear event */
+ flowctrl_write_cpu_csr(cpuid, reg);
+}
diff --git a/arch/arm/mach-tegra/flowctrl.h b/arch/arm/mach-tegra/flowctrl.h
index 1942817..0798dec 100644
--- a/arch/arm/mach-tegra/flowctrl.h
+++ b/arch/arm/mach-tegra/flowctrl.h
@@ -34,9 +34,17 @@
#define FLOW_CTRL_HALT_CPU1_EVENTS 0x14
#define FLOW_CTRL_CPU1_CSR 0x18
+#define TEGRA30_FLOW_CTRL_CSR_WFI_CPU0 (1 << 8)
+#define TEGRA30_FLOW_CTRL_CSR_WFE_BITMAP (0xF << 4)
+#define TEGRA30_FLOW_CTRL_CSR_WFI_BITMAP (0xF << 8)
+
#ifndef __ASSEMBLY__
+u32 flowctrl_read_cpu_csr(unsigned int cpuid);
void flowctrl_write_cpu_csr(unsigned int cpuid, u32 value);
void flowctrl_write_cpu_halt(unsigned int cpuid, u32 value);
+
+void flowctrl_cpu_suspend_enter(unsigned int cpuid);
+void flowctrl_cpu_suspend_exit(unsigned int cpuid);
#endif
#endif
--
1.7.0.4
^ permalink raw reply related [flat|nested] 50+ messages in thread* [PATCH 7/7] ARM: tegra30: cpuidle: add LP2 driver for CPU0
2012-10-08 10:26 [PATCH 0/7] ARM: tegra30: cpuidle: add LP2 support Joseph Lo
` (5 preceding siblings ...)
2012-10-08 10:26 ` [PATCH 6/7] ARM: tegra30: flowctrl: add cpu_suspend_exter/exit function Joseph Lo
@ 2012-10-08 10:26 ` Joseph Lo
2012-10-09 22:49 ` Stephen Warren
2012-10-09 22:26 ` [PATCH 0/7] ARM: tegra30: cpuidle: add LP2 support Stephen Warren
7 siblings, 1 reply; 50+ messages in thread
From: Joseph Lo @ 2012-10-08 10:26 UTC (permalink / raw)
To: linux-arm-kernel
The cpuidle LP2 is a power gating idle mode. It support power gating
vdd_cpu rail after all cpu cores in LP2. For Tegra30, the CPU0 must
be last one to go into LP2. We need to take care and make sure whole
secondary CPUs were in LP2 by checking CPU and power gate status.
After that, the CPU0 can go into LP2 safely. Then power gating the
CPU rail.
Base on the work by:
Scott Williams <scwilliams@nvidia.com>
Signed-off-by: Joseph Lo <josephl@nvidia.com>
---
arch/arm/mach-tegra/cpuidle-tegra30.c | 39 ++++++++-
arch/arm/mach-tegra/pm.c | 152 +++++++++++++++++++++++++++++++++
arch/arm/mach-tegra/pm.h | 3 +
arch/arm/mach-tegra/sleep-tegra30.S | 44 ++++++++++
arch/arm/mach-tegra/sleep.S | 44 ++++++++++
arch/arm/mach-tegra/sleep.h | 2 +
6 files changed, 280 insertions(+), 4 deletions(-)
diff --git a/arch/arm/mach-tegra/cpuidle-tegra30.c b/arch/arm/mach-tegra/cpuidle-tegra30.c
index 842fef4..1fd938b 100644
--- a/arch/arm/mach-tegra/cpuidle-tegra30.c
+++ b/arch/arm/mach-tegra/cpuidle-tegra30.c
@@ -28,6 +28,7 @@
#include "pm.h"
#include "sleep.h"
+#include "tegra_cpu_car.h"
#ifdef CONFIG_PM_SLEEP
static int tegra30_idle_lp2(struct cpuidle_device *dev,
@@ -59,6 +60,28 @@ static struct cpuidle_driver tegra_idle_driver = {
static DEFINE_PER_CPU(struct cpuidle_device, tegra_idle_device);
#ifdef CONFIG_PM_SLEEP
+static bool tegra30_idle_enter_lp2_cpu_0(struct cpuidle_device *dev,
+ struct cpuidle_driver *drv,
+ int index)
+{
+ struct cpuidle_state *state = &drv->states[index];
+ u32 cpu_on_time = state->exit_latency;
+ u32 cpu_off_time = state->target_residency - state->exit_latency;
+
+ if (num_online_cpus() > 1 && !tegra_cpu_rail_off_ready()) {
+ cpu_do_idle();
+ return false;
+ }
+
+ clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &dev->cpu);
+
+ tegra_idle_lp2_last(cpu_on_time, cpu_off_time);
+
+ clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &dev->cpu);
+
+ return true;
+}
+
static bool tegra30_idle_enter_lp2_cpu_n(struct cpuidle_device *dev,
struct cpuidle_driver *drv,
int index)
@@ -85,16 +108,22 @@ static int __cpuinit tegra30_idle_lp2(struct cpuidle_device *dev,
int index)
{
bool entered_lp2 = false;
+ bool last_cpu;
local_fiq_disable();
- tegra_set_cpu_in_lp2(dev->cpu);
+ last_cpu = tegra_set_cpu_in_lp2(dev->cpu);
cpu_pm_enter();
- if (dev->cpu == 0)
- cpu_do_idle();
- else
+ if (dev->cpu == 0) {
+ if (last_cpu)
+ entered_lp2 = tegra30_idle_enter_lp2_cpu_0(dev, drv,
+ index);
+ else
+ cpu_do_idle();
+ } else {
entered_lp2 = tegra30_idle_enter_lp2_cpu_n(dev, drv, index);
+ }
cpu_pm_exit();
tegra_clear_cpu_in_lp2(dev->cpu);
@@ -116,6 +145,8 @@ int __init tegra30_cpuidle_init(void)
#ifndef CONFIG_PM_SLEEP
drv->state_count = 1; /* support clockgating only */
+#else
+ tegra_tear_down_cpu = tegra30_tear_down_cpu;
#endif
ret = cpuidle_register_driver(&tegra_idle_driver);
diff --git a/arch/arm/mach-tegra/pm.c b/arch/arm/mach-tegra/pm.c
index 4655383..2e71bad 100644
--- a/arch/arm/mach-tegra/pm.c
+++ b/arch/arm/mach-tegra/pm.c
@@ -20,15 +20,38 @@
#include <linux/spinlock.h>
#include <linux/io.h>
#include <linux/cpumask.h>
+#include <linux/delay.h>
+#include <linux/cpu_pm.h>
+#include <linux/clk.h>
+#include <linux/err.h>
+
+#include <asm/smp_plat.h>
+#include <asm/cacheflush.h>
+#include <asm/suspend.h>
+#include <asm/idmap.h>
+#include <asm/proc-fns.h>
+#include <asm/tlbflush.h>
#include <mach/iomap.h>
#include "reset.h"
+#include "flowctrl.h"
+#include "sleep.h"
+#include "tegra_cpu_car.h"
+
+#define TEGRA_POWER_CPU_PWRREQ_OE (1 << 16) /* CPU pwr req enable */
+
+#define PMC_CTRL 0x0
+#define PMC_CPUPWRGOOD_TIMER 0xc8
+#define PMC_CPUPWROFF_TIMER 0xcc
#ifdef CONFIG_PM_SLEEP
static unsigned int g_diag_reg;
static DEFINE_SPINLOCK(tegra_lp2_lock);
static cpumask_t tegra_in_lp2;
+static void __iomem *pmc = IO_ADDRESS(TEGRA_PMC_BASE);
+static struct clk *tegra_pclk;
+void (*tegra_tear_down_cpu)(void);
void save_cpu_arch_register(void)
{
@@ -44,6 +67,96 @@ void restore_cpu_arch_register(void)
return;
}
+static void set_power_timers(unsigned long us_on, unsigned long us_off)
+{
+ unsigned long long ticks;
+ unsigned long long pclk;
+ unsigned long rate;
+ static unsigned long tegra_last_pclk;
+
+ if (tegra_pclk == NULL) {
+ tegra_pclk = clk_get_sys(NULL, "pclk");
+ if (IS_ERR(tegra_pclk)) {
+ /*
+ * pclk not been init or not exist.
+ * Use sclk to take the place of it.
+ * The default setting was pclk=sclk.
+ */
+ tegra_pclk = clk_get_sys(NULL, "sclk");
+ }
+ }
+
+ rate = clk_get_rate(tegra_pclk);
+
+ if (WARN_ON_ONCE(rate <= 0))
+ pclk = 100000000;
+ else
+ pclk = rate;
+
+ if ((rate != tegra_last_pclk)) {
+ ticks = (us_on * pclk) + 999999ull;
+ do_div(ticks, 1000000);
+ writel((unsigned long)ticks, pmc + PMC_CPUPWRGOOD_TIMER);
+
+ ticks = (us_off * pclk) + 999999ull;
+ do_div(ticks, 1000000);
+ writel((unsigned long)ticks, pmc + PMC_CPUPWROFF_TIMER);
+ wmb();
+ }
+ tegra_last_pclk = pclk;
+}
+
+/*
+ * restore_cpu_complex
+ *
+ * restores cpu clock setting, clears flow controller
+ *
+ * Always called on CPU 0.
+ */
+static void restore_cpu_complex(void)
+{
+ int cpu = smp_processor_id();
+
+ BUG_ON(cpu != 0);
+
+#ifdef CONFIG_SMP
+ cpu = cpu_logical_map(cpu);
+#endif
+
+ /* Restore the CPU clock settings */
+ tegra_cpu_clock_resume();
+
+ flowctrl_cpu_suspend_exit(cpu);
+
+ restore_cpu_arch_register();
+}
+
+/*
+ * suspend_cpu_complex
+ *
+ * saves pll state for use by restart_plls, prepares flow controller for
+ * transition to suspend state
+ *
+ * Must always be called on cpu 0.
+ */
+static void suspend_cpu_complex(void)
+{
+ int cpu = smp_processor_id();
+
+ BUG_ON(cpu != 0);
+
+#ifdef CONFIG_SMP
+ cpu = cpu_logical_map(cpu);
+#endif
+
+ /* Save the CPU clock settings */
+ tegra_cpu_clock_suspend();
+
+ flowctrl_cpu_suspend_enter(cpu);
+
+ save_cpu_arch_register();
+}
+
void __cpuinit tegra_clear_cpu_in_lp2(int cpu)
{
spin_lock(&tegra_lp2_lock);
@@ -85,4 +198,43 @@ bool __cpuinit tegra_set_cpu_in_lp2(int cpu)
spin_unlock(&tegra_lp2_lock);
return last_cpu;
}
+
+static int tegra_sleep_cpu(unsigned long v2p)
+{
+ /* Switch to the identity mapping. */
+ cpu_switch_mm(idmap_pgd, &init_mm);
+
+ /* Flush the TLB. */
+ local_flush_tlb_all();
+
+ tegra_sleep_cpu_finish(v2p);
+
+ /* should never here */
+ BUG();
+
+ return 0;
+}
+
+void tegra_idle_lp2_last(u32 cpu_on_time, u32 cpu_off_time)
+{
+ u32 mode;
+
+ /* Only the last cpu down does the final suspend steps */
+ mode = readl(pmc + PMC_CTRL);
+ mode |= TEGRA_POWER_CPU_PWRREQ_OE;
+ writel(mode, pmc + PMC_CTRL);
+
+ set_power_timers(cpu_on_time, cpu_off_time);
+
+ cpu_cluster_pm_enter();
+ suspend_cpu_complex();
+ flush_cache_all();
+ outer_disable();
+
+ cpu_suspend(PHYS_OFFSET - PAGE_OFFSET, &tegra_sleep_cpu);
+
+ outer_resume();
+ restore_cpu_complex();
+ cpu_cluster_pm_exit();
+}
#endif
diff --git a/arch/arm/mach-tegra/pm.h b/arch/arm/mach-tegra/pm.h
index 21858d6..63ffe02 100644
--- a/arch/arm/mach-tegra/pm.h
+++ b/arch/arm/mach-tegra/pm.h
@@ -27,4 +27,7 @@ void restore_cpu_arch_register(void);
void tegra_clear_cpu_in_lp2(int cpu);
bool tegra_set_cpu_in_lp2(int cpu);
+void tegra_idle_lp2_last(u32 cpu_on_time, u32 cpu_off_time);
+extern void (*tegra_tear_down_cpu)(void);
+
#endif /* _MACH_TEGRA_PM_H_ */
diff --git a/arch/arm/mach-tegra/sleep-tegra30.S b/arch/arm/mach-tegra/sleep-tegra30.S
index 67b1cba..aef5ce7 100644
--- a/arch/arm/mach-tegra/sleep-tegra30.S
+++ b/arch/arm/mach-tegra/sleep-tegra30.S
@@ -130,4 +130,48 @@ ENTRY(tegra30_sleep_cpu_secondary_finish)
mov r0, #1 @ never return here
mov pc, r7
ENDPROC(tegra30_sleep_cpu_secondary_finish)
+
+/*
+ * tegra30_tear_down_cpu
+ *
+ * Switches the CPU to enter sleep.
+ */
+ENTRY(tegra30_tear_down_cpu)
+ mov32 r6, TEGRA_FLOW_CTRL_BASE
+
+ b tegra30_enter_sleep
+ENDPROC(tegra30_tear_down_cpu)
+
+/*
+ * tegra30_enter_sleep
+ *
+ * uses flow controller to enter sleep state
+ * executes from IRAM with SDRAM in selfrefresh when target state is LP0 or LP1
+ * executes from SDRAM with target state is LP2
+ * r6 = TEGRA_FLOW_CTRL_BASE
+ */
+tegra30_enter_sleep:
+ cpu_id r1
+
+ cpu_to_csr_reg r2, r1
+ ldr r0, [r6, r2]
+ orr r0, r0, #FLOW_CTRL_CSR_INTR_FLAG | FLOW_CTRL_CSR_EVENT_FLAG
+ orr r0, r0, #FLOW_CTRL_CSR_ENABLE
+ str r0, [r6, r2]
+
+ mov r0, #FLOW_CTRL_WAIT_FOR_INTERRUPT
+ orr r0, r0, #FLOW_CTRL_HALT_CPU_IRQ | FLOW_CTRL_HALT_CPU_FIQ
+ cpu_to_halt_reg r2, r1
+ str r0, [r6, r2]
+ dsb
+ ldr r0, [r6, r2] /* memory barrier */
+
+halted:
+ isb
+ dsb
+ wfi /* CPU should be power gated here */
+
+ /* !!!FIXME!!! Implement halt failure handler */
+ b halted
+
#endif
diff --git a/arch/arm/mach-tegra/sleep.S b/arch/arm/mach-tegra/sleep.S
index 7748fdb..3ab1e82 100644
--- a/arch/arm/mach-tegra/sleep.S
+++ b/arch/arm/mach-tegra/sleep.S
@@ -25,6 +25,7 @@
#include <linux/linkage.h>
#include <asm/assembler.h>
+#include <asm/cache.h>
#include <asm/cp15.h>
#include <mach/iomap.h>
@@ -96,4 +97,47 @@ finished:
mov pc, lr
ENDPROC(tegra_flush_l1_cache)
+/*
+ * tegra_sleep_cpu_finish(unsigned long v2p)
+ *
+ * enters suspend in LP2 by turning off the mmu and jumping to
+ * tegra?_tear_down_cpu
+ */
+ENTRY(tegra_sleep_cpu_finish)
+ /* Flush and disable the L1 data cache */
+ bl tegra_flush_l1_cache
+ /* Trun off coherency */
+ exit_smp r4, r5
+
+ mov32 r6, tegra_tear_down_cpu
+ ldr r1, [r6]
+ add r1, r1, r0
+
+ mov32 r3, tegra_shut_off_mmu
+ add r3, r3, r0
+ mov r0, r1
+
+ mov pc, r3
+ENDPROC(tegra_sleep_cpu_finish)
+
+/*
+ * tegra_shut_off_mmu
+ *
+ * r0 = physical address to jump to with mmu off
+ *
+ * called with VA=PA mapping
+ * turns off MMU, icache, dcache and branch prediction
+ */
+ .align L1_CACHE_SHIFT
+ .pushsection .idmap.text, "ax"
+ENTRY(tegra_shut_off_mmu)
+ mrc p15, 0, r3, c1, c0, 0
+ movw r2, #CR_I | CR_Z | CR_C | CR_M
+ bic r3, r3, r2
+ dsb
+ mcr p15, 0, r3, c1, c0, 0
+ isb
+ mov pc, r0
+ENDPROC(tegra_shut_off_mmu)
+ .popsection
#endif
diff --git a/arch/arm/mach-tegra/sleep.h b/arch/arm/mach-tegra/sleep.h
index 220fbd1..001920f 100644
--- a/arch/arm/mach-tegra/sleep.h
+++ b/arch/arm/mach-tegra/sleep.h
@@ -73,6 +73,7 @@
.endm
#else
void tegra_resume(void);
+int tegra_sleep_cpu_finish(unsigned long);
#ifdef CONFIG_HOTPLUG_CPU
void tegra20_hotplug_init(void);
@@ -83,6 +84,7 @@ static inline void tegra30_hotplug_init(void) {}
#endif
int tegra30_sleep_cpu_secondary_finish(unsigned long);
+void tegra30_tear_down_cpu(void);
#endif
#endif
--
1.7.0.4
^ permalink raw reply related [flat|nested] 50+ messages in thread* [PATCH 7/7] ARM: tegra30: cpuidle: add LP2 driver for CPU0
2012-10-08 10:26 ` [PATCH 7/7] ARM: tegra30: cpuidle: add LP2 driver for CPU0 Joseph Lo
@ 2012-10-09 22:49 ` Stephen Warren
2012-10-11 11:24 ` Joseph Lo
0 siblings, 1 reply; 50+ messages in thread
From: Stephen Warren @ 2012-10-09 22:49 UTC (permalink / raw)
To: linux-arm-kernel
On 10/08/2012 04:26 AM, Joseph Lo wrote:
> The cpuidle LP2 is a power gating idle mode. It support power gating
> vdd_cpu rail after all cpu cores in LP2. For Tegra30, the CPU0 must
> be last one to go into LP2. We need to take care and make sure whole
> secondary CPUs were in LP2 by checking CPU and power gate status.
> After that, the CPU0 can go into LP2 safely. Then power gating the
> CPU rail.
> diff --git a/arch/arm/mach-tegra/cpuidle-tegra30.c b/arch/arm/mach-tegra/cpuidle-tegra30.c
> +static bool tegra30_idle_enter_lp2_cpu_0(struct cpuidle_device *dev,
> + struct cpuidle_driver *drv,
> + int index)
> +{
> + struct cpuidle_state *state = &drv->states[index];
> + u32 cpu_on_time = state->exit_latency;
> + u32 cpu_off_time = state->target_residency - state->exit_latency;
> +
> + if (num_online_cpus() > 1 && !tegra_cpu_rail_off_ready()) {
Should that be || not &&?
Isn't the "num_online_cpus() > 1" condition effectively checked at the
call site, i.e. in tegra30_idle_lp2() below via the if (last_cpu) check?
> @@ -85,16 +108,22 @@ static int __cpuinit tegra30_idle_lp2(struct cpuidle_device *dev,
> int index)
> {
> bool entered_lp2 = false;
> + bool last_cpu;
>
> local_fiq_disable();
>
> + last_cpu = tegra_set_cpu_in_lp2(dev->cpu);
> + if (dev->cpu == 0) {
> + if (last_cpu)
> + entered_lp2 = tegra30_idle_enter_lp2_cpu_0(dev, drv,
> + index);
> + else
> + cpu_do_idle();
> + } else {
> entered_lp2 = tegra30_idle_enter_lp2_cpu_n(dev, drv, index);
> + }
Hmm. That means that if the last CPU to enter LP2 is e.g. CPU1, then
even though all CPUs are now in LP2, the complex as a whole doesn't
enter LP2. Is there a way to make the cluster as a whole enter LP2 in
this case? Isn't that what coupled cpuidle is for?
> diff --git a/arch/arm/mach-tegra/pm.c b/arch/arm/mach-tegra/pm.c
> +static void set_power_timers(unsigned long us_on, unsigned long us_off)
> + if (tegra_pclk == NULL) {
> + tegra_pclk = clk_get_sys(NULL, "pclk");
> + if (IS_ERR(tegra_pclk)) {
> + /*
> + * pclk not been init or not exist.
> + * Use sclk to take the place of it.
> + * The default setting was pclk=sclk.
> + */
> + tegra_pclk = clk_get_sys(NULL, "sclk");
> + }
> + }
That's a little odd. Surely the HW has pclk or it doesn't? Why use
different clocks at different times for what is apparently the same thing?
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH 7/7] ARM: tegra30: cpuidle: add LP2 driver for CPU0
2012-10-09 22:49 ` Stephen Warren
@ 2012-10-11 11:24 ` Joseph Lo
2012-10-11 16:37 ` Stephen Warren
0 siblings, 1 reply; 50+ messages in thread
From: Joseph Lo @ 2012-10-11 11:24 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, 2012-10-10 at 06:49 +0800, Stephen Warren wrote:
> On 10/08/2012 04:26 AM, Joseph Lo wrote:
> > The cpuidle LP2 is a power gating idle mode. It support power gating
> > vdd_cpu rail after all cpu cores in LP2. For Tegra30, the CPU0 must
> > be last one to go into LP2. We need to take care and make sure whole
> > secondary CPUs were in LP2 by checking CPU and power gate status.
> > After that, the CPU0 can go into LP2 safely. Then power gating the
> > CPU rail.
>
> > diff --git a/arch/arm/mach-tegra/cpuidle-tegra30.c b/arch/arm/mach-tegra/cpuidle-tegra30.c
>
> > +static bool tegra30_idle_enter_lp2_cpu_0(struct cpuidle_device *dev,
> > + struct cpuidle_driver *drv,
> > + int index)
> > +{
> > + struct cpuidle_state *state = &drv->states[index];
> > + u32 cpu_on_time = state->exit_latency;
> > + u32 cpu_off_time = state->target_residency - state->exit_latency;
> > +
> > + if (num_online_cpus() > 1 && !tegra_cpu_rail_off_ready()) {
>
> Should that be || not &&?
>
> Isn't the "num_online_cpus() > 1" condition effectively checked at the
> call site, i.e. in tegra30_idle_lp2() below via the if (last_cpu) check?
>
Should be "&&" here.
Because we need to check if there are still multi CPUs online, then we
need to make sure all the secondary CPUs be power gated first. After all
the secondary CPUs been power gated, the CPU0 could go into LP2 and the
CPU rail could be shut off.
If all the secondary CPUs been hot plugged, then the "num_online_cpus()
>1" would be always false. Then the CPU0 can go into LP2 directly.
So it was used to check are there multi cpus online or not? It's
difference with the last_cpu check below. The last_cpu was used to check
all the CPUs were in LP2 process or not. If the CPU0 is the last one
went into LP2 process, then it would be true.
So the point here is. We can avoid to check the power status of the
secodarys CPUs if they be unplugged.
> > @@ -85,16 +108,22 @@ static int __cpuinit tegra30_idle_lp2(struct cpuidle_device *dev,
> > int index)
> > {
> > bool entered_lp2 = false;
> > + bool last_cpu;
> >
> > local_fiq_disable();
> >
> > + last_cpu = tegra_set_cpu_in_lp2(dev->cpu);
> > + if (dev->cpu == 0) {
> > + if (last_cpu)
> > + entered_lp2 = tegra30_idle_enter_lp2_cpu_0(dev, drv,
> > + index);
> > + else
> > + cpu_do_idle();
> > + } else {
> > entered_lp2 = tegra30_idle_enter_lp2_cpu_n(dev, drv, index);
> > + }
>
> Hmm. That means that if the last CPU to enter LP2 is e.g. CPU1, then
> even though all CPUs are now in LP2, the complex as a whole doesn't
> enter LP2. Is there a way to make the cluster as a whole enter LP2 in
> this case? Isn't that what coupled cpuidle is for?
>
It may look like the coupled cpuidle can satisfy the usage here. But it
didn't. Please check the criteria of coupled cpuidle.
/*
* To use coupled cpuidle states, a cpuidle driver must:
*
* Set struct cpuidle_device.coupled_cpus to the mask of all
* coupled cpus, usually the same as cpu_possible_mask if all cpus
* are part of the same cluster. The coupled_cpus mask must be
* set in the struct cpuidle_device for each cpu.
*
* Set struct cpuidle_device.safe_state to a state that is not a
* coupled state. This is usually WFI.
*
* Set CPUIDLE_FLAG_COUPLED in struct cpuidle_state.flags for each
* state that affects multiple cpus.
*
* Provide a struct cpuidle_state.enter function for each state
* that affects multiple cpus. This function is guaranteed to be
* called on all cpus at approximately the same time. The driver
* should ensure that the cpus all abort together if any cpu tries
* to abort once the function is called. The function should return
* with interrupts still disabled.
*/
The Tegra30 can support the secondary CPUs go into LP2 (power-gate)
independently. The limitation of the CPU0 is the CPU0 must be the last
one to go into LP2 to shut off CPU rail.
It also no need for every CPU to leave LP2 at the same time. The CPU0
would be always the first one that woken up from LP2. But all the other
secondary CPUs can still keep in LP2. One of the secondary CPUs can also
be woken up alone, if the CPU0 already up.
> > diff --git a/arch/arm/mach-tegra/pm.c b/arch/arm/mach-tegra/pm.c
>
> > +static void set_power_timers(unsigned long us_on, unsigned long us_off)
>
> > + if (tegra_pclk == NULL) {
> > + tegra_pclk = clk_get_sys(NULL, "pclk");
> > + if (IS_ERR(tegra_pclk)) {
> > + /*
> > + * pclk not been init or not exist.
> > + * Use sclk to take the place of it.
> > + * The default setting was pclk=sclk.
> > + */
> > + tegra_pclk = clk_get_sys(NULL, "sclk");
> > + }
> > + }
>
> That's a little odd. Surely the HW has pclk or it doesn't? Why use
> different clocks at different times for what is apparently the same thing?
It just because the "pclk" is not available on the Tegra30's clock
framework but Tegra20 right now.
Thanks,
Joseph
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH 7/7] ARM: tegra30: cpuidle: add LP2 driver for CPU0
2012-10-11 11:24 ` Joseph Lo
@ 2012-10-11 16:37 ` Stephen Warren
2012-10-11 16:48 ` Colin Cross
2012-10-12 7:07 ` Joseph Lo
0 siblings, 2 replies; 50+ messages in thread
From: Stephen Warren @ 2012-10-11 16:37 UTC (permalink / raw)
To: linux-arm-kernel
On 10/11/2012 05:24 AM, Joseph Lo wrote:
> On Wed, 2012-10-10 at 06:49 +0800, Stephen Warren wrote:
>> On 10/08/2012 04:26 AM, Joseph Lo wrote:
>>> The cpuidle LP2 is a power gating idle mode. It support power gating
>>> vdd_cpu rail after all cpu cores in LP2. For Tegra30, the CPU0 must
>>> be last one to go into LP2. We need to take care and make sure whole
>>> secondary CPUs were in LP2 by checking CPU and power gate status.
>>> After that, the CPU0 can go into LP2 safely. Then power gating the
>>> CPU rail.
>>
>>> diff --git a/arch/arm/mach-tegra/cpuidle-tegra30.c b/arch/arm/mach-tegra/cpuidle-tegra30.c
>>
>>> +static bool tegra30_idle_enter_lp2_cpu_0(struct cpuidle_device *dev,
>>> + struct cpuidle_driver *drv,
>>> + int index)
>>> +{
>>> + struct cpuidle_state *state = &drv->states[index];
>>> + u32 cpu_on_time = state->exit_latency;
>>> + u32 cpu_off_time = state->target_residency - state->exit_latency;
>>> +
>>> + if (num_online_cpus() > 1 && !tegra_cpu_rail_off_ready()) {
>>
>> Should that be || not &&?
>>
>> Isn't the "num_online_cpus() > 1" condition effectively checked at the
>> call site, i.e. in tegra30_idle_lp2() below via the if (last_cpu) check?
>>
>
> Should be "&&" here.
> Because we need to check if there are still multi CPUs online, then we
> need to make sure all the secondary CPUs be power gated first. After all
> the secondary CPUs been power gated, the CPU0 could go into LP2 and the
> CPU rail could be shut off.
> If all the secondary CPUs been hot plugged, then the "num_online_cpus()
>> 1" would be always false. Then the CPU0 can go into LP2 directly.
>
> So it was used to check are there multi cpus online or not? It's
> difference with the last_cpu check below. The last_cpu was used to check
> all the CPUs were in LP2 process or not. If the CPU0 is the last one
> went into LP2 process, then it would be true.
>
> So the point here is. We can avoid to check the power status of the
> secodarys CPUs if they be unplugged.
OK, so this condition is about ignoring the result of
tegra_cpu_rail_off_ready() if there is only 1 CPU online. That makes
sense, since we know in that case there cannot be any other CPUs to
check if they're in LP2 or not.
But what about the case where 2 CPUs are online and 2 offline. In that
case, num_online_cpus() > 1, so the call to tegra_cpu_rail_off_ready()
is skipped. Yet, with 2 CPUs online, we do need to check whichever other
CPU is online to see if it's in LP2 or not.
I think what we need to do is the following:
cpus_in_lp2_mask = generate_mask_from_pmc_registers();
if (cpus_in_lp2_mask != cpus_online_mask) {
cpu_do_idle();
return;
}
enter lp2;
right?
>>> @@ -85,16 +108,22 @@ static int __cpuinit tegra30_idle_lp2(struct cpuidle_device *dev,
>>> int index)
>>> {
>>> bool entered_lp2 = false;
>>> + bool last_cpu;
>>>
>>> local_fiq_disable();
>>>
>>> + last_cpu = tegra_set_cpu_in_lp2(dev->cpu);
>>> + if (dev->cpu == 0) {
>>> + if (last_cpu)
>>> + entered_lp2 = tegra30_idle_enter_lp2_cpu_0(dev, drv,
>>> + index);
>>> + else
>>> + cpu_do_idle();
>>> + } else {
>>> entered_lp2 = tegra30_idle_enter_lp2_cpu_n(dev, drv, index);
>>> + }
>>
>> Hmm. That means that if the last CPU to enter LP2 is e.g. CPU1, then
>> even though all CPUs are now in LP2, the complex as a whole doesn't
>> enter LP2. Is there a way to make the cluster as a whole enter LP2 in
>> this case? Isn't that what coupled cpuidle is for?
>
> It may look like the coupled cpuidle can satisfy the usage here. But it
> didn't. Please check the criteria of coupled cpuidle.
What about the first part of the question. What happens if:
CPU3 enters LP2
CPU2 enters LP2
CPU0 enters LP2
CPU1 enters LP2
Since CPU1 is not CPU0, tegra30_idle_enter_lp2_cpu_n() is called, and
hence I think the whole CPU complex is never rail-gated (just each CPU
is power-gated) even though all CPUs are in LP2 and the complex could be
rail-gated. Isn't this missing out on power-savings?
So, we either need to:
a) Make tegra30_idle_enter_lp2_cpu_n() rail-gate if the last CPU is
entering LP2, and then I'm not sure the implementation would be any
different to tegra30_idle_enter_lp2_cpu_0, would it?
b) If CPUn can't trigger rail-gating, then when CPUn is the last to
enter LP2 of the whole complex, it needs to IPI to CPU0 to tell it to
rail-gate, and simply power-gate itself. I believe this IPI interaction
is exactly what coupled cpuidle is about, isn't it?
> /*
> * To use coupled cpuidle states, a cpuidle driver must:
> *
> * Set struct cpuidle_device.coupled_cpus to the mask of all
> * coupled cpus, usually the same as cpu_possible_mask if all cpus
> * are part of the same cluster. The coupled_cpus mask must be
> * set in the struct cpuidle_device for each cpu.
> *
> * Set struct cpuidle_device.safe_state to a state that is not a
> * coupled state. This is usually WFI.
> *
> * Set CPUIDLE_FLAG_COUPLED in struct cpuidle_state.flags for each
> * state that affects multiple cpus.
> *
> * Provide a struct cpuidle_state.enter function for each state
> * that affects multiple cpus. This function is guaranteed to be
> * called on all cpus at approximately the same time. The driver
> * should ensure that the cpus all abort together if any cpu tries
> * to abort once the function is called. The function should return
> * with interrupts still disabled.
> */
>
> The Tegra30 can support the secondary CPUs go into LP2 (power-gate)
> independently.
I think that just means that the safe state for CPUn (i.e. not CPU0) can
do better than WFI on Tegra30, even though it can't on Tegra20.
> The limitation of the CPU0 is the CPU0 must be the last
> one to go into LP2 to shut off CPU rail.
>
> It also no need for every CPU to leave LP2 at the same time. The CPU0
> would be always the first one that woken up from LP2. But all the other
> secondary CPUs can still keep in LP2. One of the secondary CPUs can also
> be woken up alone, if the CPU0 already up.
That seems like an implementation detail. Perhaps coupled cpuidle needs
to be enhanced to best support Tegra30?
>>> diff --git a/arch/arm/mach-tegra/pm.c b/arch/arm/mach-tegra/pm.c
>>
>>> +static void set_power_timers(unsigned long us_on, unsigned long us_off)
>>
>>> + if (tegra_pclk == NULL) {
>>> + tegra_pclk = clk_get_sys(NULL, "pclk");
>>> + if (IS_ERR(tegra_pclk)) {
>>> + /*
>>> + * pclk not been init or not exist.
>>> + * Use sclk to take the place of it.
>>> + * The default setting was pclk=sclk.
>>> + */
>>> + tegra_pclk = clk_get_sys(NULL, "sclk");
>>> + }
>>> + }
>>
>> That's a little odd. Surely the HW has pclk or it doesn't? Why use
>> different clocks at different times for what is apparently the same thing?
>
> It just because the "pclk" is not available on the Tegra30's clock
> framework but Tegra20 right now.
We should just fix that instead of working around it then. I assume it's
a simple matter of adding the appropriate clock definition?
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH 7/7] ARM: tegra30: cpuidle: add LP2 driver for CPU0
2012-10-11 16:37 ` Stephen Warren
@ 2012-10-11 16:48 ` Colin Cross
2012-10-12 7:11 ` Joseph Lo
` (2 more replies)
2012-10-12 7:07 ` Joseph Lo
1 sibling, 3 replies; 50+ messages in thread
From: Colin Cross @ 2012-10-11 16:48 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Oct 11, 2012 at 9:37 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 10/11/2012 05:24 AM, Joseph Lo wrote:
>> On Wed, 2012-10-10 at 06:49 +0800, Stephen Warren wrote:
>>> On 10/08/2012 04:26 AM, Joseph Lo wrote:
>>>> The cpuidle LP2 is a power gating idle mode. It support power gating
>>>> vdd_cpu rail after all cpu cores in LP2. For Tegra30, the CPU0 must
>>>> be last one to go into LP2. We need to take care and make sure whole
>>>> secondary CPUs were in LP2 by checking CPU and power gate status.
>>>> After that, the CPU0 can go into LP2 safely. Then power gating the
>>>> CPU rail.
<snip>
>>>> @@ -85,16 +108,22 @@ static int __cpuinit tegra30_idle_lp2(struct cpuidle_device *dev,
>>>> int index)
>>>> {
>>>> bool entered_lp2 = false;
>>>> + bool last_cpu;
>>>>
>>>> local_fiq_disable();
>>>>
>>>> + last_cpu = tegra_set_cpu_in_lp2(dev->cpu);
>>>> + if (dev->cpu == 0) {
>>>> + if (last_cpu)
>>>> + entered_lp2 = tegra30_idle_enter_lp2_cpu_0(dev, drv,
>>>> + index);
>>>> + else
>>>> + cpu_do_idle();
>>>> + } else {
>>>> entered_lp2 = tegra30_idle_enter_lp2_cpu_n(dev, drv, index);
>>>> + }
>>>
>>> Hmm. That means that if the last CPU to enter LP2 is e.g. CPU1, then
>>> even though all CPUs are now in LP2, the complex as a whole doesn't
>>> enter LP2. Is there a way to make the cluster as a whole enter LP2 in
>>> this case? Isn't that what coupled cpuidle is for?
>>
>> It may look like the coupled cpuidle can satisfy the usage here. But it
>> didn't. Please check the criteria of coupled cpuidle.
>
> What about the first part of the question. What happens if:
>
> CPU3 enters LP2
> CPU2 enters LP2
> CPU0 enters LP2
> CPU1 enters LP2
>
> Since CPU1 is not CPU0, tegra30_idle_enter_lp2_cpu_n() is called, and
> hence I think the whole CPU complex is never rail-gated (just each CPU
> is power-gated) even though all CPUs are in LP2 and the complex could be
> rail-gated. Isn't this missing out on power-savings?
>
> So, we either need to:
>
> a) Make tegra30_idle_enter_lp2_cpu_n() rail-gate if the last CPU is
> entering LP2, and then I'm not sure the implementation would be any
> different to tegra30_idle_enter_lp2_cpu_0, would it?
>
> b) If CPUn can't trigger rail-gating, then when CPUn is the last to
> enter LP2 of the whole complex, it needs to IPI to CPU0 to tell it to
> rail-gate, and simply power-gate itself. I believe this IPI interaction
> is exactly what coupled cpuidle is about, isn't it?
>
>> /*
>> * To use coupled cpuidle states, a cpuidle driver must:
>> *
>> * Set struct cpuidle_device.coupled_cpus to the mask of all
>> * coupled cpus, usually the same as cpu_possible_mask if all cpus
>> * are part of the same cluster. The coupled_cpus mask must be
>> * set in the struct cpuidle_device for each cpu.
>> *
>> * Set struct cpuidle_device.safe_state to a state that is not a
>> * coupled state. This is usually WFI.
>> *
>> * Set CPUIDLE_FLAG_COUPLED in struct cpuidle_state.flags for each
>> * state that affects multiple cpus.
>> *
>> * Provide a struct cpuidle_state.enter function for each state
>> * that affects multiple cpus. This function is guaranteed to be
>> * called on all cpus at approximately the same time. The driver
>> * should ensure that the cpus all abort together if any cpu tries
>> * to abort once the function is called. The function should return
>> * with interrupts still disabled.
>> */
>>
>> The Tegra30 can support the secondary CPUs go into LP2 (power-gate)
>> independently.
>
> I think that just means that the safe state for CPUn (i.e. not CPU0) can
> do better than WFI on Tegra30, even though it can't on Tegra20.
Exactly.
>> The limitation of the CPU0 is the CPU0 must be the last
>> one to go into LP2 to shut off CPU rail.
>>
>> It also no need for every CPU to leave LP2 at the same time. The CPU0
>> would be always the first one that woken up from LP2. But all the other
>> secondary CPUs can still keep in LP2. One of the secondary CPUs can also
>> be woken up alone, if the CPU0 already up.
>
> That seems like an implementation detail. Perhaps coupled cpuidle needs
> to be enhanced to best support Tegra30?
As is, coupled cpuidle will work on Tegra30, but it will unnecessarily
wake up the secondary cpus during the transitions to off and back on
again. Those cpus will immediately go back to single-cpu LP2, so it
may not be a big deal, but there is a small optimization I've
discussed with a few other people that could avoid waking them up. I
suggest adding an extra pre-idle hook to the Tegra30 that is called by
coupled cpuidle on the last cpu to go down. It would return a cpumask
of cpus that have been prepared for idle by guaranteeing that they
will not wake up from an interrupt, and therefore don't need to be
woken up for the transitions. I haven't worked with a cpu that needs
this optimization yet, so I haven't done it.
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH 7/7] ARM: tegra30: cpuidle: add LP2 driver for CPU0
2012-10-11 16:48 ` Colin Cross
@ 2012-10-12 7:11 ` Joseph Lo
2012-10-12 7:40 ` Joseph Lo
2012-10-12 7:54 ` Shawn Guo
2 siblings, 0 replies; 50+ messages in thread
From: Joseph Lo @ 2012-10-12 7:11 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, 2012-10-12 at 00:48 +0800, Colin Cross wrote:
> On Thu, Oct 11, 2012 at 9:37 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> > On 10/11/2012 05:24 AM, Joseph Lo wrote:
> >> On Wed, 2012-10-10 at 06:49 +0800, Stephen Warren wrote:
> >>> On 10/08/2012 04:26 AM, Joseph Lo wrote:
> >>>> The cpuidle LP2 is a power gating idle mode. It support power gating
> >>>> vdd_cpu rail after all cpu cores in LP2. For Tegra30, the CPU0 must
> >>>> be last one to go into LP2. We need to take care and make sure whole
> >>>> secondary CPUs were in LP2 by checking CPU and power gate status.
> >>>> After that, the CPU0 can go into LP2 safely. Then power gating the
> >>>> CPU rail.
>
> <snip>
>
> >>>> @@ -85,16 +108,22 @@ static int __cpuinit tegra30_idle_lp2(struct cpuidle_device *dev,
> >>>> int index)
> >>>> {
> >>>> bool entered_lp2 = false;
> >>>> + bool last_cpu;
> >>>>
> >>>> local_fiq_disable();
> >>>>
> >>>> + last_cpu = tegra_set_cpu_in_lp2(dev->cpu);
> >>>> + if (dev->cpu == 0) {
> >>>> + if (last_cpu)
> >>>> + entered_lp2 = tegra30_idle_enter_lp2_cpu_0(dev, drv,
> >>>> + index);
> >>>> + else
> >>>> + cpu_do_idle();
> >>>> + } else {
> >>>> entered_lp2 = tegra30_idle_enter_lp2_cpu_n(dev, drv, index);
> >>>> + }
> >>>
> >>> Hmm. That means that if the last CPU to enter LP2 is e.g. CPU1, then
> >>> even though all CPUs are now in LP2, the complex as a whole doesn't
> >>> enter LP2. Is there a way to make the cluster as a whole enter LP2 in
> >>> this case? Isn't that what coupled cpuidle is for?
> >>
> >> It may look like the coupled cpuidle can satisfy the usage here. But it
> >> didn't. Please check the criteria of coupled cpuidle.
> >
> > What about the first part of the question. What happens if:
> >
> > CPU3 enters LP2
> > CPU2 enters LP2
> > CPU0 enters LP2
> > CPU1 enters LP2
> >
> > Since CPU1 is not CPU0, tegra30_idle_enter_lp2_cpu_n() is called, and
> > hence I think the whole CPU complex is never rail-gated (just each CPU
> > is power-gated) even though all CPUs are in LP2 and the complex could be
> > rail-gated. Isn't this missing out on power-savings?
> >
> > So, we either need to:
> >
> > a) Make tegra30_idle_enter_lp2_cpu_n() rail-gate if the last CPU is
> > entering LP2, and then I'm not sure the implementation would be any
> > different to tegra30_idle_enter_lp2_cpu_0, would it?
> >
> > b) If CPUn can't trigger rail-gating, then when CPUn is the last to
> > enter LP2 of the whole complex, it needs to IPI to CPU0 to tell it to
> > rail-gate, and simply power-gate itself. I believe this IPI interaction
> > is exactly what coupled cpuidle is about, isn't it?
> >
> >> /*
> >> * To use coupled cpuidle states, a cpuidle driver must:
> >> *
> >> * Set struct cpuidle_device.coupled_cpus to the mask of all
> >> * coupled cpus, usually the same as cpu_possible_mask if all cpus
> >> * are part of the same cluster. The coupled_cpus mask must be
> >> * set in the struct cpuidle_device for each cpu.
> >> *
> >> * Set struct cpuidle_device.safe_state to a state that is not a
> >> * coupled state. This is usually WFI.
> >> *
> >> * Set CPUIDLE_FLAG_COUPLED in struct cpuidle_state.flags for each
> >> * state that affects multiple cpus.
> >> *
> >> * Provide a struct cpuidle_state.enter function for each state
> >> * that affects multiple cpus. This function is guaranteed to be
> >> * called on all cpus at approximately the same time. The driver
> >> * should ensure that the cpus all abort together if any cpu tries
> >> * to abort once the function is called. The function should return
> >> * with interrupts still disabled.
> >> */
> >>
> >> The Tegra30 can support the secondary CPUs go into LP2 (power-gate)
> >> independently.
> >
> > I think that just means that the safe state for CPUn (i.e. not CPU0) can
> > do better than WFI on Tegra30, even though it can't on Tegra20.
>
> Exactly.
>
> >> The limitation of the CPU0 is the CPU0 must be the last
> >> one to go into LP2 to shut off CPU rail.
> >>
> >> It also no need for every CPU to leave LP2 at the same time. The CPU0
> >> would be always the first one that woken up from LP2. But all the other
> >> secondary CPUs can still keep in LP2. One of the secondary CPUs can also
> >> be woken up alone, if the CPU0 already up.
> >
> > That seems like an implementation detail. Perhaps coupled cpuidle needs
> > to be enhanced to best support Tegra30?
>
> As is, coupled cpuidle will work on Tegra30, but it will unnecessarily
> wake up the secondary cpus during the transitions to off and back on
> again. Those cpus will immediately go back to single-cpu LP2, so it
> may not be a big deal, but there is a small optimization I've
> discussed with a few other people that could avoid waking them up. I
> suggest adding an extra pre-idle hook to the Tegra30 that is called by
> coupled cpuidle on the last cpu to go down. It would return a cpumask
> of cpus that have been prepared for idle by guaranteeing that they
> will not wake up from an interrupt, and therefore don't need to be
> woken up for the transitions. I haven't worked with a cpu that needs
> this optimization yet, so I haven't done it.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH 7/7] ARM: tegra30: cpuidle: add LP2 driver for CPU0
2012-10-11 16:48 ` Colin Cross
2012-10-12 7:11 ` Joseph Lo
@ 2012-10-12 7:40 ` Joseph Lo
2012-10-12 7:54 ` Shawn Guo
2 siblings, 0 replies; 50+ messages in thread
From: Joseph Lo @ 2012-10-12 7:40 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, 2012-10-12 at 00:48 +0800, Colin Cross wrote:
> On Thu, Oct 11, 2012 at 9:37 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> > On 10/11/2012 05:24 AM, Joseph Lo wrote:
> >> On Wed, 2012-10-10 at 06:49 +0800, Stephen Warren wrote:
> >>> On 10/08/2012 04:26 AM, Joseph Lo wrote:
> >>>> The cpuidle LP2 is a power gating idle mode. It support power gating
> >>>> vdd_cpu rail after all cpu cores in LP2. For Tegra30, the CPU0 must
> >>>> be last one to go into LP2. We need to take care and make sure whole
> >>>> secondary CPUs were in LP2 by checking CPU and power gate status.
> >>>> After that, the CPU0 can go into LP2 safely. Then power gating the
> >>>> CPU rail.
>
> <snip>
>
> >>>> @@ -85,16 +108,22 @@ static int __cpuinit tegra30_idle_lp2(struct cpuidle_device *dev,
> >>>> int index)
> >>>> {
> >>>> bool entered_lp2 = false;
> >>>> + bool last_cpu;
> >>>>
> >>>> local_fiq_disable();
> >>>>
> >>>> + last_cpu = tegra_set_cpu_in_lp2(dev->cpu);
> >>>> + if (dev->cpu == 0) {
> >>>> + if (last_cpu)
> >>>> + entered_lp2 = tegra30_idle_enter_lp2_cpu_0(dev, drv,
> >>>> + index);
> >>>> + else
> >>>> + cpu_do_idle();
> >>>> + } else {
> >>>> entered_lp2 = tegra30_idle_enter_lp2_cpu_n(dev, drv, index);
> >>>> + }
> >>>
> >>> Hmm. That means that if the last CPU to enter LP2 is e.g. CPU1, then
> >>> even though all CPUs are now in LP2, the complex as a whole doesn't
> >>> enter LP2. Is there a way to make the cluster as a whole enter LP2 in
> >>> this case? Isn't that what coupled cpuidle is for?
> >>
> >> It may look like the coupled cpuidle can satisfy the usage here. But it
> >> didn't. Please check the criteria of coupled cpuidle.
> >
> > What about the first part of the question. What happens if:
> >
> > CPU3 enters LP2
> > CPU2 enters LP2
> > CPU0 enters LP2
> > CPU1 enters LP2
> >
> > Since CPU1 is not CPU0, tegra30_idle_enter_lp2_cpu_n() is called, and
> > hence I think the whole CPU complex is never rail-gated (just each CPU
> > is power-gated) even though all CPUs are in LP2 and the complex could be
> > rail-gated. Isn't this missing out on power-savings?
> >
> > So, we either need to:
> >
> > a) Make tegra30_idle_enter_lp2_cpu_n() rail-gate if the last CPU is
> > entering LP2, and then I'm not sure the implementation would be any
> > different to tegra30_idle_enter_lp2_cpu_0, would it?
> >
> > b) If CPUn can't trigger rail-gating, then when CPUn is the last to
> > enter LP2 of the whole complex, it needs to IPI to CPU0 to tell it to
> > rail-gate, and simply power-gate itself. I believe this IPI interaction
> > is exactly what coupled cpuidle is about, isn't it?
> >
> >> /*
> >> * To use coupled cpuidle states, a cpuidle driver must:
> >> *
> >> * Set struct cpuidle_device.coupled_cpus to the mask of all
> >> * coupled cpus, usually the same as cpu_possible_mask if all cpus
> >> * are part of the same cluster. The coupled_cpus mask must be
> >> * set in the struct cpuidle_device for each cpu.
> >> *
> >> * Set struct cpuidle_device.safe_state to a state that is not a
> >> * coupled state. This is usually WFI.
> >> *
> >> * Set CPUIDLE_FLAG_COUPLED in struct cpuidle_state.flags for each
> >> * state that affects multiple cpus.
> >> *
> >> * Provide a struct cpuidle_state.enter function for each state
> >> * that affects multiple cpus. This function is guaranteed to be
> >> * called on all cpus at approximately the same time. The driver
> >> * should ensure that the cpus all abort together if any cpu tries
> >> * to abort once the function is called. The function should return
> >> * with interrupts still disabled.
> >> */
> >>
> >> The Tegra30 can support the secondary CPUs go into LP2 (power-gate)
> >> independently.
> >
> > I think that just means that the safe state for CPUn (i.e. not CPU0) can
> > do better than WFI on Tegra30, even though it can't on Tegra20.
>
> Exactly.
>
> >> The limitation of the CPU0 is the CPU0 must be the last
> >> one to go into LP2 to shut off CPU rail.
> >>
> >> It also no need for every CPU to leave LP2 at the same time. The CPU0
> >> would be always the first one that woken up from LP2. But all the other
> >> secondary CPUs can still keep in LP2. One of the secondary CPUs can also
> >> be woken up alone, if the CPU0 already up.
> >
> > That seems like an implementation detail. Perhaps coupled cpuidle needs
> > to be enhanced to best support Tegra30?
>
> As is, coupled cpuidle will work on Tegra30, but it will unnecessarily
> wake up the secondary cpus during the transitions to off and back on
> again. Those cpus will immediately go back to single-cpu LP2, so it
> may not be a big deal, but there is a small optimization I've
> discussed with a few other people that could avoid waking them up. I
> suggest adding an extra pre-idle hook to the Tegra30 that is called by
> coupled cpuidle on the last cpu to go down. It would return a cpumask
> of cpus that have been prepared for idle by guaranteeing that they
> will not wake up from an interrupt, and therefore don't need to be
> woken up for the transitions. I haven't worked with a cpu that needs
> this optimization yet, so I haven't done it.
Hi Colin,
Thanks for your update. I understand what you are talk about. Actually,
I had tried it in the background for both Tegra20 and Tegra30. But I can
just ran several times of coupled cpuidle state. Then it dead locked in
the coupled cpuidle state. It's look like a race condition and dead lock
by missing IPI. I am sure the GIC was on and the IPI be fired. But
sometimes the CPU just can't catch the IPI. And I had a WAR for it by
doing a very short local_irq_enable and local_irq_disable in the coupled
cpuidle ready waiting loop. Then everything works. Not sure, did you
meet this before? Any hint about this?
Very interesting about "guaranteeing that they will not wake up from an
interrupt", did these CPUs go into power-gete cpuidle mode by wfe? If
the CPUs not wake up from an interrupt when doing cpuidle, how did them
be woke up? By sending event to these CPUs?
Thanks,
Joseph
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH 7/7] ARM: tegra30: cpuidle: add LP2 driver for CPU0
2012-10-11 16:48 ` Colin Cross
2012-10-12 7:11 ` Joseph Lo
2012-10-12 7:40 ` Joseph Lo
@ 2012-10-12 7:54 ` Shawn Guo
2012-10-12 8:24 ` Joseph Lo
2 siblings, 1 reply; 50+ messages in thread
From: Shawn Guo @ 2012-10-12 7:54 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Oct 11, 2012 at 09:48:45AM -0700, Colin Cross wrote:
> As is, coupled cpuidle will work on Tegra30, but it will unnecessarily
> wake up the secondary cpus during the transitions to off and back on
> again. Those cpus will immediately go back to single-cpu LP2,
I'm sure coupled cpuidle will work like that. We have the following
code at the end of cpuidle_enter_state_coupled() to wait until all
coupled cpus have exited idle.
while (!cpuidle_coupled_no_cpus_ready(coupled))
cpu_relax();
The cpu woken up during the transitions will just loop there until all
other 3 cpus exit from idle function.
Shawn
> so it
> may not be a big deal, but there is a small optimization I've
> discussed with a few other people that could avoid waking them up. I
> suggest adding an extra pre-idle hook to the Tegra30 that is called by
> coupled cpuidle on the last cpu to go down. It would return a cpumask
> of cpus that have been prepared for idle by guaranteeing that they
> will not wake up from an interrupt, and therefore don't need to be
> woken up for the transitions. I haven't worked with a cpu that needs
> this optimization yet, so I haven't done it.
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH 7/7] ARM: tegra30: cpuidle: add LP2 driver for CPU0
2012-10-12 7:54 ` Shawn Guo
@ 2012-10-12 8:24 ` Joseph Lo
2012-10-12 8:30 ` Shawn Guo
2012-10-12 20:46 ` [PATCH 7/7] ARM: tegra30: cpuidle: add LP2 driver for CPU0 Colin Cross
0 siblings, 2 replies; 50+ messages in thread
From: Joseph Lo @ 2012-10-12 8:24 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, 2012-10-12 at 15:54 +0800, Shawn Guo wrote:
> On Thu, Oct 11, 2012 at 09:48:45AM -0700, Colin Cross wrote:
> > As is, coupled cpuidle will work on Tegra30, but it will unnecessarily
> > wake up the secondary cpus during the transitions to off and back on
> > again. Those cpus will immediately go back to single-cpu LP2,
>
> I'm sure coupled cpuidle will work like that. We have the following
> code at the end of cpuidle_enter_state_coupled() to wait until all
> coupled cpus have exited idle.
>
> while (!cpuidle_coupled_no_cpus_ready(coupled))
> cpu_relax();
>
> The cpu woken up during the transitions will just loop there until all
> other 3 cpus exit from idle function.
>
Did this a good idea if the CPU was been woken up from an interrupt but
it still needed to wait all other CPUs been woken up then it could
handle the interrupt?
Thanks,
Joseph
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 7/7] ARM: tegra30: cpuidle: add LP2 driver for CPU0
2012-10-12 8:24 ` Joseph Lo
@ 2012-10-12 8:30 ` Shawn Guo
2012-10-12 20:50 ` Colin Cross
2012-10-12 20:46 ` [PATCH 7/7] ARM: tegra30: cpuidle: add LP2 driver for CPU0 Colin Cross
1 sibling, 1 reply; 50+ messages in thread
From: Shawn Guo @ 2012-10-12 8:30 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Oct 12, 2012 at 04:24:24PM +0800, Joseph Lo wrote:
> On Fri, 2012-10-12 at 15:54 +0800, Shawn Guo wrote:
> > On Thu, Oct 11, 2012 at 09:48:45AM -0700, Colin Cross wrote:
> > > As is, coupled cpuidle will work on Tegra30, but it will unnecessarily
> > > wake up the secondary cpus during the transitions to off and back on
> > > again. Those cpus will immediately go back to single-cpu LP2,
> >
> > I'm sure coupled cpuidle will work like that. We have the following
> > code at the end of cpuidle_enter_state_coupled() to wait until all
> > coupled cpus have exited idle.
> >
> > while (!cpuidle_coupled_no_cpus_ready(coupled))
> > cpu_relax();
> >
> > The cpu woken up during the transitions will just loop there until all
> > other 3 cpus exit from idle function.
> >
>
> Did this a good idea if the CPU was been woken up from an interrupt but
> it still needed to wait all other CPUs been woken up then it could
> handle the interrupt?
>
This is how coupled cpuidle gets implemented right now. And that's
why I see RCU stall warning reported on that cpu when I tried coupled
cpuidle on imx6q (CA9 Quad).
Shawn
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 7/7] ARM: tegra30: cpuidle: add LP2 driver for CPU0
2012-10-12 8:30 ` Shawn Guo
@ 2012-10-12 20:50 ` Colin Cross
2012-10-15 16:28 ` Use coupled cpuidle on imx6q Shawn Guo
0 siblings, 1 reply; 50+ messages in thread
From: Colin Cross @ 2012-10-12 20:50 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Oct 12, 2012 at 1:30 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> On Fri, Oct 12, 2012 at 04:24:24PM +0800, Joseph Lo wrote:
>> On Fri, 2012-10-12 at 15:54 +0800, Shawn Guo wrote:
>> > On Thu, Oct 11, 2012 at 09:48:45AM -0700, Colin Cross wrote:
>> > > As is, coupled cpuidle will work on Tegra30, but it will unnecessarily
>> > > wake up the secondary cpus during the transitions to off and back on
>> > > again. Those cpus will immediately go back to single-cpu LP2,
>> >
>> > I'm sure coupled cpuidle will work like that. We have the following
>> > code at the end of cpuidle_enter_state_coupled() to wait until all
>> > coupled cpus have exited idle.
>> >
>> > while (!cpuidle_coupled_no_cpus_ready(coupled))
>> > cpu_relax();
>> >
>> > The cpu woken up during the transitions will just loop there until all
>> > other 3 cpus exit from idle function.
>> >
>>
>> Did this a good idea if the CPU was been woken up from an interrupt but
>> it still needed to wait all other CPUs been woken up then it could
>> handle the interrupt?
>>
> This is how coupled cpuidle gets implemented right now. And that's
> why I see RCU stall warning reported on that cpu when I tried coupled
> cpuidle on imx6q (CA9 Quad).
Current coupled cpuidle requires all cpus to wake up together and go
back to the idle governor to select a new state, because that's what
all available ARM cpus did when I wrote it. You should be able to
implement coupled idle on a cpu that does not need to wake all the
cpus if you wake them anyways, and then later you can optimize it to
avoid the extra wakeups when the extension to coupled cpuidle that I
discussed previously gets implemented.
^ permalink raw reply [flat|nested] 50+ messages in thread
* Use coupled cpuidle on imx6q
2012-10-12 20:50 ` Colin Cross
@ 2012-10-15 16:28 ` Shawn Guo
2012-10-15 22:58 ` Colin Cross
0 siblings, 1 reply; 50+ messages in thread
From: Shawn Guo @ 2012-10-15 16:28 UTC (permalink / raw)
To: linux-arm-kernel
Changed subject (was: Re: [PATCH 7/7] ARM: tegra30: cpuidle: add LP2
driver for CPU0) ...
On Fri, Oct 12, 2012 at 01:50:35PM -0700, Colin Cross wrote:
> Current coupled cpuidle requires all cpus to wake up together and go
> back to the idle governor to select a new state, because that's what
> all available ARM cpus did when I wrote it. You should be able to
> implement coupled idle on a cpu that does not need to wake all the
> cpus if you wake them anyways,
Can you please elaborate it a little bit? I do not quite understand
what you mean.
Basically, imx6q has a low-power mode named WAIT. When all 4 cores
are in WFI, imx6q will go into WAIT mode, and whenever there is a
wakeup interrupt, it will exit WAIT mode. Software can configure
hardware behavior during WAIT mode, clock gating or power gating for
ARM core.
I'm trying to implement this low-power mode with coupled cpuidle.
I initially have the following code as the coupled cpuidle enter
function for clock gating case.
static int imx6q_enter_wait_coupled(struct cpuidle_device *dev,
struct cpuidle_driver *drv, int index)
{
int cpu = dev->cpu;
clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &cpu);
if (atomic_inc_return(&master) == num_online_cpus())
imx6q_set_lpm(WAIT_UNCLOCKED);
cpu_do_idle();
if (atomic_dec_return(&master) == num_online_cpus() - 1)
imx6q_set_lpm(WAIT_CLOCKED);
clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &cpu);
return index;
}
However I think there are a couple of problems. The first one is the
extra wakeups you have mentioned. When last cpu is in call
imx6q_set_lpm(), some of the first 3 cpus that are already in WFI may
exit from there. The second one is when hardware exits WAIT mode and
has ARM clock resumed, some cpus may stay in WFI if scheduler has
nothing to wake them up. This breaks the requirement of coupled cpuidle
that all cpus need to exit together. I can force the function to work
around the problems by adding cpuidle_coupled_parallel_barrier() just
above cpu_do_idle() and arch_send_wakeup_ipi_mask(cpu_online_mask)
right after imx6q_set_lpm(WAIT_CLOCKED). But I doubt it's appropriate.
So I rewrite the function as below to not use coupled cpuidle, and it
works fine.
static int imx6q_enter_wait(struct cpuidle_device *dev,
struct cpuidle_driver *drv, int index)
{
int cpu = dev->cpu;
clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &cpu);
if (atomic_inc_return(&master) == num_online_cpus()) {
/*
* With this lock, we prevent the other cpu to exit and
* enter this function again and become the master.
*/
if (!spin_trylock(&master_lock))
goto idle;
imx6q_set_lpm(WAIT_UNCLOCKED);
cpu_do_idle();
imx6q_set_lpm(WAIT_CLOCKED);
spin_unlock(&master_lock);
goto out;
}
idle:
cpu_do_idle();
out:
atomic_dec(&master);
clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &cpu);
return index;
}
For power gating case, it may better fit coupled cpuidle usage model,
since all the cores will have to run resume routine and thus will not
be in WFI when hardware exits from WAIT mode. But we still need to
work out the extra wakeup issue which will also be seen in this case.
> and then later you can optimize it to
> avoid the extra wakeups when the extension to coupled cpuidle that I
> discussed previously gets implemented.
Do you have a pointer to the initial discussion for the extension, or
you have a draft patch for that already (hopefully:)?
Shawn
^ permalink raw reply [flat|nested] 50+ messages in thread* Use coupled cpuidle on imx6q
2012-10-15 16:28 ` Use coupled cpuidle on imx6q Shawn Guo
@ 2012-10-15 22:58 ` Colin Cross
0 siblings, 0 replies; 50+ messages in thread
From: Colin Cross @ 2012-10-15 22:58 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Oct 15, 2012 at 9:28 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> Changed subject (was: Re: [PATCH 7/7] ARM: tegra30: cpuidle: add LP2
> driver for CPU0) ...
>
> On Fri, Oct 12, 2012 at 01:50:35PM -0700, Colin Cross wrote:
>> Current coupled cpuidle requires all cpus to wake up together and go
>> back to the idle governor to select a new state, because that's what
>> all available ARM cpus did when I wrote it. You should be able to
>> implement coupled idle on a cpu that does not need to wake all the
>> cpus if you wake them anyways,
>
> Can you please elaborate it a little bit? I do not quite understand
> what you mean.
In the current coupled cpuidle implementation, no cpu will return back
to the scheduler until all cpus have returned from their idle
function. That means you need to force each cpu to wake up whenever
one of them wakes up. This is a limitation in the current coupled
cpuidle design due to all existing coupled cpuidle implemenations
having it as a hardware requirement, but is not a hardware requirement
for your WAIT mode. Until somebody fixes that, you have to fake
wakeups to the other cpus.
> Basically, imx6q has a low-power mode named WAIT. When all 4 cores
> are in WFI, imx6q will go into WAIT mode, and whenever there is a
> wakeup interrupt, it will exit WAIT mode. Software can configure
> hardware behavior during WAIT mode, clock gating or power gating for
> ARM core.
>
> I'm trying to implement this low-power mode with coupled cpuidle.
> I initially have the following code as the coupled cpuidle enter
> function for clock gating case.
>
> static int imx6q_enter_wait_coupled(struct cpuidle_device *dev,
> struct cpuidle_driver *drv, int index)
> {
> int cpu = dev->cpu;
>
> clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &cpu);
>
> if (atomic_inc_return(&master) == num_online_cpus())
> imx6q_set_lpm(WAIT_UNCLOCKED);
>
> cpu_do_idle();
>
> if (atomic_dec_return(&master) == num_online_cpus() - 1)
> imx6q_set_lpm(WAIT_CLOCKED);
>
> clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &cpu);
> return index;
> }
>
> However I think there are a couple of problems. The first one is the
> extra wakeups you have mentioned. When last cpu is in call
> imx6q_set_lpm(), some of the first 3 cpus that are already in WFI may
> exit from there. The second one is when hardware exits WAIT mode and
> has ARM clock resumed, some cpus may stay in WFI if scheduler has
> nothing to wake them up. This breaks the requirement of coupled cpuidle
> that all cpus need to exit together. I can force the function to work
> around the problems by adding cpuidle_coupled_parallel_barrier() just
> above cpu_do_idle() and arch_send_wakeup_ipi_mask(cpu_online_mask)
> right after imx6q_set_lpm(WAIT_CLOCKED). But I doubt it's appropriate.
That is the appropriate way of getting it working with today's coupled cpuidle.
> So I rewrite the function as below to not use coupled cpuidle, and it
> works fine.
>
> static int imx6q_enter_wait(struct cpuidle_device *dev,
> struct cpuidle_driver *drv, int index)
> {
> int cpu = dev->cpu;
>
> clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &cpu);
>
> if (atomic_inc_return(&master) == num_online_cpus()) {
> /*
> * With this lock, we prevent the other cpu to exit and
> * enter this function again and become the master.
> */
> if (!spin_trylock(&master_lock))
> goto idle;
>
> imx6q_set_lpm(WAIT_UNCLOCKED);
> cpu_do_idle();
> imx6q_set_lpm(WAIT_CLOCKED);
>
> spin_unlock(&master_lock);
> goto out;
> }
>
> idle:
> cpu_do_idle();
> out:
> atomic_dec(&master);
> clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &cpu);
> return index;
> }
Coupled cpuidle provides two features, and you only need one of them
for your clock gating mode. The more complex one is sequencing, which
you don't need since your hardware can check if all cpus are in WFI
for you. The other feature is voting, to make sure all cpus can
accept the exit latency of the deeper idle state. The
atomic_inc_return is handling voting in your example, but it can't
handle one cpu requesting power gating while the other requests clock
gating.
> For power gating case, it may better fit coupled cpuidle usage model,
> since all the cores will have to run resume routine and thus will not
> be in WFI when hardware exits from WAIT mode. But we still need to
> work out the extra wakeup issue which will also be seen in this case.
Yes, but using coupled cpuidle for both clock gating and power gating
will allow you to get into clock gating when one cpu wants clock
gating and the other wants power gating.
>> and then later you can optimize it to
>> avoid the extra wakeups when the extension to coupled cpuidle that I
>> discussed previously gets implemented.
>
> Do you have a pointer to the initial discussion for the extension, or
> you have a draft patch for that already (hopefully:)?
I can't find the initial discussion, and I'm not going to have a
chance to work on it for at least a few weeks. The basic idea is:
Add "preidle" and "postidle" function pointers to the coupled cpuidle
states (better names TBD).
When all cpus are waiting, and before sending any pokes, call the
preidle hook on the last cpu to start waiting. It must only be called
on one cpu. The preidle hook is responsible for disabling wakeups on
secondary cpus so they can't wake up, and returning a mask of the cpus
it has disabled.
The coupled core can increment ready count by the number of cpus that
are disabled and skip poking them.
After coupled idle returns (or when aborting coupled idle), call the
postidle hook with the mask returned by the pre-idle hook, which
re-enables wakeups on secondary cpus and returns a mask of cpus that
are not waking because they are already in a state that can handle
interrupts (WFI). The calling cpu will then decrement the ready count
by the number of cpus that are not waking.
There are some tricky synchronization problems to solve:
Only one cpu can call the preidle and postidle hooks. On some
hardware, they may not even be the same cpu.
A cpu that is left in the WFI state by the postidle hook is back in
the "waiting but not ready" state, and the current coupled cpuidle
code does not support going from waiting->ready->waiting.
Once the postidle hook has returned, some cpus may be able to
immediately handle interrupts and return to the scheduler.
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 7/7] ARM: tegra30: cpuidle: add LP2 driver for CPU0
2012-10-12 8:24 ` Joseph Lo
2012-10-12 8:30 ` Shawn Guo
@ 2012-10-12 20:46 ` Colin Cross
1 sibling, 0 replies; 50+ messages in thread
From: Colin Cross @ 2012-10-12 20:46 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Oct 12, 2012 at 1:24 AM, Joseph Lo <josephl@nvidia.com> wrote:
> On Fri, 2012-10-12 at 15:54 +0800, Shawn Guo wrote:
>> On Thu, Oct 11, 2012 at 09:48:45AM -0700, Colin Cross wrote:
>> > As is, coupled cpuidle will work on Tegra30, but it will unnecessarily
>> > wake up the secondary cpus during the transitions to off and back on
>> > again. Those cpus will immediately go back to single-cpu LP2,
>>
>> I'm sure coupled cpuidle will work like that. We have the following
>> code at the end of cpuidle_enter_state_coupled() to wait until all
>> coupled cpus have exited idle.
>>
>> while (!cpuidle_coupled_no_cpus_ready(coupled))
>> cpu_relax();
>>
>> The cpu woken up during the transitions will just loop there until all
>> other 3 cpus exit from idle function.
>>
>
> Did this a good idea if the CPU was been woken up from an interrupt but
> it still needed to wait all other CPUs been woken up then it could
> handle the interrupt?
Each cpu enables its local interrupts between when it clears it's
ready flag and when it enters the loop above, so it will already be
handling interrupts.
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 7/7] ARM: tegra30: cpuidle: add LP2 driver for CPU0
2012-10-11 16:37 ` Stephen Warren
2012-10-11 16:48 ` Colin Cross
@ 2012-10-12 7:07 ` Joseph Lo
2012-10-12 21:04 ` Stephen Warren
1 sibling, 1 reply; 50+ messages in thread
From: Joseph Lo @ 2012-10-12 7:07 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, 2012-10-12 at 00:37 +0800, Stephen Warren wrote:
> On 10/11/2012 05:24 AM, Joseph Lo wrote:
> > On Wed, 2012-10-10 at 06:49 +0800, Stephen Warren wrote:
> >> On 10/08/2012 04:26 AM, Joseph Lo wrote:
> >>> The cpuidle LP2 is a power gating idle mode. It support power gating
> >>> vdd_cpu rail after all cpu cores in LP2. For Tegra30, the CPU0 must
> >>> be last one to go into LP2. We need to take care and make sure whole
> >>> secondary CPUs were in LP2 by checking CPU and power gate status.
> >>> After that, the CPU0 can go into LP2 safely. Then power gating the
> >>> CPU rail.
> >>
> >>> diff --git a/arch/arm/mach-tegra/cpuidle-tegra30.c b/arch/arm/mach-tegra/cpuidle-tegra30.c
> >>
> >>> +static bool tegra30_idle_enter_lp2_cpu_0(struct cpuidle_device *dev,
> >>> + struct cpuidle_driver *drv,
> >>> + int index)
> >>> +{
> >>> + struct cpuidle_state *state = &drv->states[index];
> >>> + u32 cpu_on_time = state->exit_latency;
> >>> + u32 cpu_off_time = state->target_residency - state->exit_latency;
> >>> +
> >>> + if (num_online_cpus() > 1 && !tegra_cpu_rail_off_ready()) {
> >>
> >> Should that be || not &&?
> >>
> >> Isn't the "num_online_cpus() > 1" condition effectively checked at the
> >> call site, i.e. in tegra30_idle_lp2() below via the if (last_cpu) check?
> >>
> >
> > Should be "&&" here.
> > Because we need to check if there are still multi CPUs online, then we
> > need to make sure all the secondary CPUs be power gated first. After all
> > the secondary CPUs been power gated, the CPU0 could go into LP2 and the
> > CPU rail could be shut off.
> > If all the secondary CPUs been hot plugged, then the "num_online_cpus()
> >> 1" would be always false. Then the CPU0 can go into LP2 directly.
> >
> > So it was used to check are there multi cpus online or not? It's
> > difference with the last_cpu check below. The last_cpu was used to check
> > all the CPUs were in LP2 process or not. If the CPU0 is the last one
> > went into LP2 process, then it would be true.
> >
> > So the point here is. We can avoid to check the power status of the
> > secodarys CPUs if they be unplugged.
>
> OK, so this condition is about ignoring the result of
> tegra_cpu_rail_off_ready() if there is only 1 CPU online. That makes
> sense, since we know in that case there cannot be any other CPUs to
> check if they're in LP2 or not.
>
> But what about the case where 2 CPUs are online and 2 offline. In that
> case, num_online_cpus() > 1, so the call to tegra_cpu_rail_off_ready()
> is skipped. Yet, with 2 CPUs online, we do need to check whichever other
> CPU is online to see if it's in LP2 or not.
>
> I think what we need to do is the following:
>
> cpus_in_lp2_mask = generate_mask_from_pmc_registers();
> if (cpus_in_lp2_mask != cpus_online_mask) {
> cpu_do_idle();
> return;
> }
> enter lp2;
>
> right?
>
We are not only check the cpu_in_lp2_mask here. The most important thing
here was to check all the other secondary CPUs were already been power
gated. We need to check the physical power status of the CPUs not
logical status.
When there are only 2 CPU online, the "num_online_cpus() > 1" would be
true and the "tegra_cpu_rail_off_ready" still would be checked.
> >>> @@ -85,16 +108,22 @@ static int __cpuinit tegra30_idle_lp2(struct cpuidle_device *dev,
> >>> int index)
> >>> {
> >>> bool entered_lp2 = false;
> >>> + bool last_cpu;
> >>>
> >>> local_fiq_disable();
> >>>
> >>> + last_cpu = tegra_set_cpu_in_lp2(dev->cpu);
> >>> + if (dev->cpu == 0) {
> >>> + if (last_cpu)
> >>> + entered_lp2 = tegra30_idle_enter_lp2_cpu_0(dev, drv,
> >>> + index);
> >>> + else
> >>> + cpu_do_idle();
> >>> + } else {
> >>> entered_lp2 = tegra30_idle_enter_lp2_cpu_n(dev, drv, index);
> >>> + }
> >>
> >> Hmm. That means that if the last CPU to enter LP2 is e.g. CPU1, then
> >> even though all CPUs are now in LP2, the complex as a whole doesn't
> >> enter LP2. Is there a way to make the cluster as a whole enter LP2 in
> >> this case? Isn't that what coupled cpuidle is for?
> >
> > It may look like the coupled cpuidle can satisfy the usage here. But it
> > didn't. Please check the criteria of coupled cpuidle.
>
> What about the first part of the question. What happens if:
>
> CPU3 enters LP2
> CPU2 enters LP2
> CPU0 enters LP2
> CPU1 enters LP2
>
> Since CPU1 is not CPU0, tegra30_idle_enter_lp2_cpu_n() is called, and
> hence I think the whole CPU complex is never rail-gated (just each CPU
> is power-gated) even though all CPUs are in LP2 and the complex could be
> rail-gated. Isn't this missing out on power-savings?
Yes, indeed.
>
> So, we either need to:
>
> a) Make tegra30_idle_enter_lp2_cpu_n() rail-gate if the last CPU is
> entering LP2, and then I'm not sure the implementation would be any
> different to tegra30_idle_enter_lp2_cpu_0, would it?
>
No, we need to make sure the CPU0 is the last one go into LP2 state and
all other CPUs already be power gated. This is the only safe way to shut
off vdd_cpu rail (HW limitation).
> b) If CPUn can't trigger rail-gating, then when CPUn is the last to
> enter LP2 of the whole complex, it needs to IPI to CPU0 to tell it to
> rail-gate, and simply power-gate itself. I believe this IPI interaction
> is exactly what coupled cpuidle is about, isn't it?
>
Yes, indeed. Actually, I had tried the coupled cpuidle on Tegra20. I
knew it a lot. But I met issues when porting it. It looks like a race
condition and becomes a dead lock caused by IPI missing. Anyway, we can
talk about it more detail when I try to upstream the coupled cpuidle
support for Tegra later.
> > /*
> > * To use coupled cpuidle states, a cpuidle driver must:
> > *
> > * Set struct cpuidle_device.coupled_cpus to the mask of all
> > * coupled cpus, usually the same as cpu_possible_mask if all cpus
> > * are part of the same cluster. The coupled_cpus mask must be
> > * set in the struct cpuidle_device for each cpu.
> > *
> > * Set struct cpuidle_device.safe_state to a state that is not a
> > * coupled state. This is usually WFI.
> > *
> > * Set CPUIDLE_FLAG_COUPLED in struct cpuidle_state.flags for each
> > * state that affects multiple cpus.
> > *
> > * Provide a struct cpuidle_state.enter function for each state
> > * that affects multiple cpus. This function is guaranteed to be
> > * called on all cpus at approximately the same time. The driver
> > * should ensure that the cpus all abort together if any cpu tries
> > * to abort once the function is called. The function should return
> > * with interrupts still disabled.
> > */
> >
> > The Tegra30 can support the secondary CPUs go into LP2 (power-gate)
> > independently.
>
> I think that just means that the safe state for CPUn (i.e. not CPU0) can
> do better than WFI on Tegra30, even though it can't on Tegra20.
>
Yes, I understood.
> > The limitation of the CPU0 is the CPU0 must be the last
> > one to go into LP2 to shut off CPU rail.
> >
> > It also no need for every CPU to leave LP2 at the same time. The CPU0
> > would be always the first one that woken up from LP2. But all the other
> > secondary CPUs can still keep in LP2. One of the secondary CPUs can also
> > be woken up alone, if the CPU0 already up.
>
> That seems like an implementation detail. Perhaps coupled cpuidle needs
> to be enhanced to best support Tegra30?
>
Yes. If the coupled cpuidle could support the CPUs can randomly leave
the coupled state, it would be perfect.
> >>> diff --git a/arch/arm/mach-tegra/pm.c b/arch/arm/mach-tegra/pm.c
> >>
> >>> +static void set_power_timers(unsigned long us_on, unsigned long us_off)
> >>
> >>> + if (tegra_pclk == NULL) {
> >>> + tegra_pclk = clk_get_sys(NULL, "pclk");
> >>> + if (IS_ERR(tegra_pclk)) {
> >>> + /*
> >>> + * pclk not been init or not exist.
> >>> + * Use sclk to take the place of it.
> >>> + * The default setting was pclk=sclk.
> >>> + */
> >>> + tegra_pclk = clk_get_sys(NULL, "sclk");
> >>> + }
> >>> + }
> >>
> >> That's a little odd. Surely the HW has pclk or it doesn't? Why use
> >> different clocks at different times for what is apparently the same thing?
> >
> > It just because the "pclk" is not available on the Tegra30's clock
> > framework but Tegra20 right now.
>
> We should just fix that instead of working around it then. I assume it's
> a simple matter of adding the appropriate clock definition?
OK. I will add the "pclk" clock interface for Tegra30.
Thanks,
Joseph
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH 7/7] ARM: tegra30: cpuidle: add LP2 driver for CPU0
2012-10-12 7:07 ` Joseph Lo
@ 2012-10-12 21:04 ` Stephen Warren
2012-10-15 7:56 ` Joseph Lo
0 siblings, 1 reply; 50+ messages in thread
From: Stephen Warren @ 2012-10-12 21:04 UTC (permalink / raw)
To: linux-arm-kernel
On 10/12/2012 01:07 AM, Joseph Lo wrote:
> On Fri, 2012-10-12 at 00:37 +0800, Stephen Warren wrote:
>> On 10/11/2012 05:24 AM, Joseph Lo wrote:
>>> On Wed, 2012-10-10 at 06:49 +0800, Stephen Warren wrote:
>>>> On 10/08/2012 04:26 AM, Joseph Lo wrote:
>>>>> The cpuidle LP2 is a power gating idle mode. It support power gating
>>>>> vdd_cpu rail after all cpu cores in LP2. For Tegra30, the CPU0 must
>>>>> be last one to go into LP2. We need to take care and make sure whole
>>>>> secondary CPUs were in LP2 by checking CPU and power gate status.
>>>>> After that, the CPU0 can go into LP2 safely. Then power gating the
>>>>> CPU rail.
>>>>
>>>>> diff --git a/arch/arm/mach-tegra/cpuidle-tegra30.c b/arch/arm/mach-tegra/cpuidle-tegra30.c
>>>>
>>>>> +static bool tegra30_idle_enter_lp2_cpu_0(struct cpuidle_device *dev,
>>>>> + struct cpuidle_driver *drv,
>>>>> + int index)
>>>>> +{
>>>>> + struct cpuidle_state *state = &drv->states[index];
>>>>> + u32 cpu_on_time = state->exit_latency;
>>>>> + u32 cpu_off_time = state->target_residency - state->exit_latency;
>>>>> +
>>>>> + if (num_online_cpus() > 1 && !tegra_cpu_rail_off_ready()) {
>>>>
>>>> Should that be || not &&?
>>>>
>>>> Isn't the "num_online_cpus() > 1" condition effectively checked at the
>>>> call site, i.e. in tegra30_idle_lp2() below via the if (last_cpu) check?
>>>>
>>>
>>> Should be "&&" here.
>>> Because we need to check if there are still multi CPUs online, then we
>>> need to make sure all the secondary CPUs be power gated first. After all
>>> the secondary CPUs been power gated, the CPU0 could go into LP2 and the
>>> CPU rail could be shut off.
>>> If all the secondary CPUs been hot plugged, then the "num_online_cpus()
>>>> 1" would be always false. Then the CPU0 can go into LP2 directly.
>>>
>>> So it was used to check are there multi cpus online or not? It's
>>> difference with the last_cpu check below. The last_cpu was used to check
>>> all the CPUs were in LP2 process or not. If the CPU0 is the last one
>>> went into LP2 process, then it would be true.
>>>
>>> So the point here is. We can avoid to check the power status of the
>>> secodarys CPUs if they be unplugged.
>>
>> OK, so this condition is about ignoring the result of
>> tegra_cpu_rail_off_ready() if there is only 1 CPU online. That makes
>> sense, since we know in that case there cannot be any other CPUs to
>> check if they're in LP2 or not.
>>
>> But what about the case where 2 CPUs are online and 2 offline. In that
>> case, num_online_cpus() > 1, so the call to tegra_cpu_rail_off_ready()
>> is skipped.
Uggh. I'm not thinking straight, so what I said there was backwards.
>> b) If CPUn can't trigger rail-gating, then when CPUn is the last to
>> enter LP2 of the whole complex, it needs to IPI to CPU0 to tell it to
>> rail-gate, and simply power-gate itself. I believe this IPI interaction
>> is exactly what coupled cpuidle is about, isn't it?
>
> Yes, indeed. Actually, I had tried the coupled cpuidle on Tegra20. I
> knew it a lot. But I met issues when porting it. It looks like a race
> condition and becomes a dead lock caused by IPI missing. Anyway, we can
> talk about it more detail when I try to upstream the coupled cpuidle
> support for Tegra later.
Hmm. That sounds a little churny. Why can't we just use coupled cpuidle
right from the start if the plan is to use it eventually anyway? From
other comments, it sounds like you even already have the code basically
working, but just need to iron out a bug or two?
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH 7/7] ARM: tegra30: cpuidle: add LP2 driver for CPU0
2012-10-12 21:04 ` Stephen Warren
@ 2012-10-15 7:56 ` Joseph Lo
2012-10-15 15:59 ` Stephen Warren
0 siblings, 1 reply; 50+ messages in thread
From: Joseph Lo @ 2012-10-15 7:56 UTC (permalink / raw)
To: linux-arm-kernel
On Sat, 2012-10-13 at 05:04 +0800, Stephen Warren wrote:
> On 10/12/2012 01:07 AM, Joseph Lo wrote:
> > On Fri, 2012-10-12 at 00:37 +0800, Stephen Warren wrote:
> >> On 10/11/2012 05:24 AM, Joseph Lo wrote:
> >>> On Wed, 2012-10-10 at 06:49 +0800, Stephen Warren wrote:
> >>>> On 10/08/2012 04:26 AM, Joseph Lo wrote:
> >>>>> The cpuidle LP2 is a power gating idle mode. It support power gating
> >>>>> vdd_cpu rail after all cpu cores in LP2. For Tegra30, the CPU0 must
> >>>>> be last one to go into LP2. We need to take care and make sure whole
> >>>>> secondary CPUs were in LP2 by checking CPU and power gate status.
> >>>>> After that, the CPU0 can go into LP2 safely. Then power gating the
> >>>>> CPU rail.
> >>>>
> >>>>> diff --git a/arch/arm/mach-tegra/cpuidle-tegra30.c b/arch/arm/mach-tegra/cpuidle-tegra30.c
> >> b) If CPUn can't trigger rail-gating, then when CPUn is the last to
> >> enter LP2 of the whole complex, it needs to IPI to CPU0 to tell it to
> >> rail-gate, and simply power-gate itself. I believe this IPI interaction
> >> is exactly what coupled cpuidle is about, isn't it?
> >
> > Yes, indeed. Actually, I had tried the coupled cpuidle on Tegra20. I
> > knew it a lot. But I met issues when porting it. It looks like a race
> > condition and becomes a dead lock caused by IPI missing. Anyway, we can
> > talk about it more detail when I try to upstream the coupled cpuidle
> > support for Tegra later.
>
> Hmm. That sounds a little churny. Why can't we just use coupled cpuidle
> right from the start if the plan is to use it eventually anyway? From
> other comments, it sounds like you even already have the code basically
> working, but just need to iron out a bug or two?
Stephen,
Even though we have plan to use coupled cpuidle, I still prefer to go
with the LP2 driver first. Then adding one more patch to support coupled
cpuidle based on LP2 driver. This is good for history. And if there is
any issue, it's more easy to roll back to the stable one.
There is still one thing you should know. Because we are planning to
upstream "CPUquiet" framework that is a CPU auto hotplug mechanism. It
will auto hotplug the CPUs depends on the system is busy or not. So when
system is idle, there will be only one CPU online (i.e, CPU0). The
secondary CPUs will all be hot plugged (i.e, offline and power gate). We
need to think about do we still need coupled cpuidle on Tegra30 if we
are going to use "CPUquiet".
And Yes, I already had a work version on Tegra20 but I need to iron out
the IPI missing issue first. The LP2 driver of Tegra20 also not yet be
upstreamed.
Thanks,
Joseph
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 7/7] ARM: tegra30: cpuidle: add LP2 driver for CPU0
2012-10-15 7:56 ` Joseph Lo
@ 2012-10-15 15:59 ` Stephen Warren
2012-10-15 22:33 ` Colin Cross
2012-10-16 8:06 ` Peter De Schrijver
0 siblings, 2 replies; 50+ messages in thread
From: Stephen Warren @ 2012-10-15 15:59 UTC (permalink / raw)
To: linux-arm-kernel
On 10/15/2012 01:56 AM, Joseph Lo wrote:
> On Sat, 2012-10-13 at 05:04 +0800, Stephen Warren wrote:
>> On 10/12/2012 01:07 AM, Joseph Lo wrote:
>>> On Fri, 2012-10-12 at 00:37 +0800, Stephen Warren wrote:
>>>> On 10/11/2012 05:24 AM, Joseph Lo wrote:
>>>>> On Wed, 2012-10-10 at 06:49 +0800, Stephen Warren wrote:
>>>>>> On 10/08/2012 04:26 AM, Joseph Lo wrote:
>>>>>>> The cpuidle LP2 is a power gating idle mode. It support power gating
>>>>>>> vdd_cpu rail after all cpu cores in LP2. For Tegra30, the CPU0 must
>>>>>>> be last one to go into LP2. We need to take care and make sure whole
>>>>>>> secondary CPUs were in LP2 by checking CPU and power gate status.
>>>>>>> After that, the CPU0 can go into LP2 safely. Then power gating the
>>>>>>> CPU rail.
>>>>>>
>>>>>>> diff --git a/arch/arm/mach-tegra/cpuidle-tegra30.c b/arch/arm/mach-tegra/cpuidle-tegra30.c
>>>> b) If CPUn can't trigger rail-gating, then when CPUn is the last to
>>>> enter LP2 of the whole complex, it needs to IPI to CPU0 to tell it to
>>>> rail-gate, and simply power-gate itself. I believe this IPI interaction
>>>> is exactly what coupled cpuidle is about, isn't it?
>>>
>>> Yes, indeed. Actually, I had tried the coupled cpuidle on Tegra20. I
>>> knew it a lot. But I met issues when porting it. It looks like a race
>>> condition and becomes a dead lock caused by IPI missing. Anyway, we can
>>> talk about it more detail when I try to upstream the coupled cpuidle
>>> support for Tegra later.
>>
>> Hmm. That sounds a little churny. Why can't we just use coupled cpuidle
>> right from the start if the plan is to use it eventually anyway? From
>> other comments, it sounds like you even already have the code basically
>> working, but just need to iron out a bug or two?
>
> Stephen,
>
> Even though we have plan to use coupled cpuidle, I still prefer to go
> with the LP2 driver first. Then adding one more patch to support coupled
> cpuidle based on LP2 driver. This is good for history. And if there is
> any issue, it's more easy to roll back to the stable one.
I don't think that implementing it one way and then changing to a
different way will benefit history at all. It'll make the history more
complicated. What exactly is the problem with just using coupled cpuidle
from the start? If we did merge this implementation now, then switch to
coupled cpuidle later, when do you think the switch would happen?
> There is still one thing you should know. Because we are planning to
> upstream "CPUquiet" framework that is a CPU auto hotplug mechanism. It
> will auto hotplug the CPUs depends on the system is busy or not. So when
> system is idle, there will be only one CPU online (i.e, CPU0). The
> secondary CPUs will all be hot plugged (i.e, offline and power gate). We
> need to think about do we still need coupled cpuidle on Tegra30 if we
> are going to use "CPUquiet".
CPUquiet isn't relevant at all. First, a user may presumably disable
CPUquiet's Kconfig option (it had better have one, and the system had
better work with it disabled). Second, even if CPUquiet is enabled, I
don't imagine there is a 100% guarantee that hot(un)plug will happen
before cpuidle kicks in, is there? Finally, is there any guarantee that
CPUquiet will actually be accepted upstream?
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 7/7] ARM: tegra30: cpuidle: add LP2 driver for CPU0
2012-10-15 15:59 ` Stephen Warren
@ 2012-10-15 22:33 ` Colin Cross
2012-10-16 8:13 ` Peter De Schrijver
2012-10-16 8:06 ` Peter De Schrijver
1 sibling, 1 reply; 50+ messages in thread
From: Colin Cross @ 2012-10-15 22:33 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Oct 15, 2012 at 8:59 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 10/15/2012 01:56 AM, Joseph Lo wrote:
>> There is still one thing you should know. Because we are planning to
>> upstream "CPUquiet" framework that is a CPU auto hotplug mechanism. It
>> will auto hotplug the CPUs depends on the system is busy or not. So when
>> system is idle, there will be only one CPU online (i.e, CPU0). The
>> secondary CPUs will all be hot plugged (i.e, offline and power gate). We
>> need to think about do we still need coupled cpuidle on Tegra30 if we
>> are going to use "CPUquiet".
>
> CPUquiet isn't relevant at all. First, a user may presumably disable
> CPUquiet's Kconfig option (it had better have one, and the system had
> better work with it disabled). Second, even if CPUquiet is enabled, I
> don't imagine there is a 100% guarantee that hot(un)plug will happen
> before cpuidle kicks in, is there? Finally, is there any guarantee that
> CPUquiet will actually be accepted upstream?
CPUquiet is a glorified hotplug governor, and hotplug governors have
been widely rejected upstream, so I wouldn't plan on seeing it
accepted.
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 7/7] ARM: tegra30: cpuidle: add LP2 driver for CPU0
2012-10-15 22:33 ` Colin Cross
@ 2012-10-16 8:13 ` Peter De Schrijver
0 siblings, 0 replies; 50+ messages in thread
From: Peter De Schrijver @ 2012-10-16 8:13 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Oct 16, 2012 at 12:33:07AM +0200, Colin Cross wrote:
> On Mon, Oct 15, 2012 at 8:59 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> > On 10/15/2012 01:56 AM, Joseph Lo wrote:
> >> There is still one thing you should know. Because we are planning to
> >> upstream "CPUquiet" framework that is a CPU auto hotplug mechanism. It
> >> will auto hotplug the CPUs depends on the system is busy or not. So when
> >> system is idle, there will be only one CPU online (i.e, CPU0). The
> >> secondary CPUs will all be hot plugged (i.e, offline and power gate). We
> >> need to think about do we still need coupled cpuidle on Tegra30 if we
> >> are going to use "CPUquiet".
> >
> > CPUquiet isn't relevant at all. First, a user may presumably disable
> > CPUquiet's Kconfig option (it had better have one, and the system had
> > better work with it disabled). Second, even if CPUquiet is enabled, I
> > don't imagine there is a 100% guarantee that hot(un)plug will happen
> > before cpuidle kicks in, is there? Finally, is there any guarantee that
> > CPUquiet will actually be accepted upstream?
>
> CPUquiet is a glorified hotplug governor, and hotplug governors have
> been widely rejected upstream, so I wouldn't plan on seeing it
> accepted.
Note that nothing in CPUquiet enforces the use of hotplug. It assumes there
is a way to put a CPU in a quiet state which means it doesn't get interrupted,
doesn't do any work and doesn't respond to IPIs. Currently only hotplug
provides this state, but it's possible to provide a driver which uses a
different mechanism to provide the same state. We need this state to be able
to switch to the low power cluster which has only 1 CPU. IPIs to non-existing
cores would hang the system.
Cheers,
Peter.
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 7/7] ARM: tegra30: cpuidle: add LP2 driver for CPU0
2012-10-15 15:59 ` Stephen Warren
2012-10-15 22:33 ` Colin Cross
@ 2012-10-16 8:06 ` Peter De Schrijver
2012-10-16 17:03 ` Stephen Warren
1 sibling, 1 reply; 50+ messages in thread
From: Peter De Schrijver @ 2012-10-16 8:06 UTC (permalink / raw)
To: linux-arm-kernel
> > Even though we have plan to use coupled cpuidle, I still prefer to go
> > with the LP2 driver first. Then adding one more patch to support coupled
> > cpuidle based on LP2 driver. This is good for history. And if there is
> > any issue, it's more easy to roll back to the stable one.
>
> I don't think that implementing it one way and then changing to a
> different way will benefit history at all. It'll make the history more
> complicated. What exactly is the problem with just using coupled cpuidle
> from the start? If we did merge this implementation now, then switch to
> coupled cpuidle later, when do you think the switch would happen?
>
Before we consider doing this, I think we should have some idea on how
frequently we run into the situation where CPU0 is idle but a secondary
core is not. Depending on that we can then decide how useful coupled cpuidle
would be for us.
Cheers,
Peter.
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 7/7] ARM: tegra30: cpuidle: add LP2 driver for CPU0
2012-10-16 8:06 ` Peter De Schrijver
@ 2012-10-16 17:03 ` Stephen Warren
2012-10-18 9:24 ` Peter De Schrijver
0 siblings, 1 reply; 50+ messages in thread
From: Stephen Warren @ 2012-10-16 17:03 UTC (permalink / raw)
To: linux-arm-kernel
On 10/16/2012 02:06 AM, Peter De Schrijver wrote:
>>> Even though we have plan to use coupled cpuidle, I still prefer to go
>>> with the LP2 driver first. Then adding one more patch to support coupled
>>> cpuidle based on LP2 driver. This is good for history. And if there is
>>> any issue, it's more easy to roll back to the stable one.
>>
>> I don't think that implementing it one way and then changing to a
>> different way will benefit history at all. It'll make the history more
>> complicated. What exactly is the problem with just using coupled cpuidle
>> from the start? If we did merge this implementation now, then switch to
>> coupled cpuidle later, when do you think the switch would happen?
>
> Before we consider doing this, I think we should have some idea on how
> frequently we run into the situation where CPU0 is idle but a secondary
> core is not. Depending on that we can then decide how useful coupled cpuidle
> would be for us.
Would it not be 75% of the time where we have 1 of 4 CPUs active? At
least, that's assuming that all work is evenly distributed amongst CPUs,
and hence it's random which CPU is the last to go idle, but perhaps
that's not the case if CPU0 is somehow special workload-wise?
But yes, some stats gathered from real-world use-cases could well be
interesting.
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 7/7] ARM: tegra30: cpuidle: add LP2 driver for CPU0
2012-10-16 17:03 ` Stephen Warren
@ 2012-10-18 9:24 ` Peter De Schrijver
2012-10-25 14:08 ` Peter De Schrijver
0 siblings, 1 reply; 50+ messages in thread
From: Peter De Schrijver @ 2012-10-18 9:24 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Oct 16, 2012 at 07:03:43PM +0200, Stephen Warren wrote:
> On 10/16/2012 02:06 AM, Peter De Schrijver wrote:
> >>> Even though we have plan to use coupled cpuidle, I still prefer to go
> >>> with the LP2 driver first. Then adding one more patch to support coupled
> >>> cpuidle based on LP2 driver. This is good for history. And if there is
> >>> any issue, it's more easy to roll back to the stable one.
> >>
> >> I don't think that implementing it one way and then changing to a
> >> different way will benefit history at all. It'll make the history more
> >> complicated. What exactly is the problem with just using coupled cpuidle
> >> from the start? If we did merge this implementation now, then switch to
> >> coupled cpuidle later, when do you think the switch would happen?
> >
> > Before we consider doing this, I think we should have some idea on how
> > frequently we run into the situation where CPU0 is idle but a secondary
> > core is not. Depending on that we can then decide how useful coupled cpuidle
> > would be for us.
>
> Would it not be 75% of the time where we have 1 of 4 CPUs active? At
> least, that's assuming that all work is evenly distributed amongst CPUs,
> and hence it's random which CPU is the last to go idle, but perhaps
> that's not the case if CPU0 is somehow special workload-wise?
>
Depends, at least it used to be possible to tune the scheduler to prefer
CPU0 if the workload can run on a single CPU.
Cheers,
Peter.
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 7/7] ARM: tegra30: cpuidle: add LP2 driver for CPU0
2012-10-18 9:24 ` Peter De Schrijver
@ 2012-10-25 14:08 ` Peter De Schrijver
0 siblings, 0 replies; 50+ messages in thread
From: Peter De Schrijver @ 2012-10-25 14:08 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Oct 18, 2012 at 11:24:40AM +0200, Peter De Schrijver wrote:
> On Tue, Oct 16, 2012 at 07:03:43PM +0200, Stephen Warren wrote:
> > On 10/16/2012 02:06 AM, Peter De Schrijver wrote:
> > >>> Even though we have plan to use coupled cpuidle, I still prefer to go
> > >>> with the LP2 driver first. Then adding one more patch to support coupled
> > >>> cpuidle based on LP2 driver. This is good for history. And if there is
> > >>> any issue, it's more easy to roll back to the stable one.
> > >>
> > >> I don't think that implementing it one way and then changing to a
> > >> different way will benefit history at all. It'll make the history more
> > >> complicated. What exactly is the problem with just using coupled cpuidle
> > >> from the start? If we did merge this implementation now, then switch to
> > >> coupled cpuidle later, when do you think the switch would happen?
> > >
> > > Before we consider doing this, I think we should have some idea on how
> > > frequently we run into the situation where CPU0 is idle but a secondary
> > > core is not. Depending on that we can then decide how useful coupled cpuidle
> > > would be for us.
> >
> > Would it not be 75% of the time where we have 1 of 4 CPUs active? At
> > least, that's assuming that all work is evenly distributed amongst CPUs,
> > and hence it's random which CPU is the last to go idle, but perhaps
> > that's not the case if CPU0 is somehow special workload-wise?
> >
>
> Depends, at least it used to be possible to tune the scheduler to prefer
> CPU0 if the workload can run on a single CPU.
>
I just noticed https://lwn.net/Articles/518834/. If we can configure this to
prefer CPU0 in case all work can be done on a single core, we shouldn't
hit the case were a secondary CPU is the only active CPU for a significant
period of time.
Cheers,
Peter.
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 0/7] ARM: tegra30: cpuidle: add LP2 support
2012-10-08 10:26 [PATCH 0/7] ARM: tegra30: cpuidle: add LP2 support Joseph Lo
` (6 preceding siblings ...)
2012-10-08 10:26 ` [PATCH 7/7] ARM: tegra30: cpuidle: add LP2 driver for CPU0 Joseph Lo
@ 2012-10-09 22:26 ` Stephen Warren
2012-10-11 6:39 ` Joseph Lo
7 siblings, 1 reply; 50+ messages in thread
From: Stephen Warren @ 2012-10-09 22:26 UTC (permalink / raw)
To: linux-arm-kernel
On 10/08/2012 04:26 AM, Joseph Lo wrote:
> The CPU idle LP2 is a power gating idle mode for Tegra30. It supports the
> secondary CPUs (i.e., CPU1-CPU3) to go into LP2 dynamically. When any of
> the secondary CPUs go into LP2, it can be power gated alone. There is a
> limitation on CPU0. The CPU0 can go into LP2 only when all secondary CPUs
> are already in LP2. After CPU0 is in LP2, the CPU rail can be turned off.
>
> Verified on Seaboard(Tegra20) and Cardhu(Tegra30).
What's the most comprehensive way to verify this? I booted Cardhu with
these patches applied and saw that all CPU cores did enter both idle
states. However, I'm unsure what the best way to stress the system is,
i.e. how would I stress and test for correct handling of all the L2
caching/SMP coherency issues, etc.
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH 0/7] ARM: tegra30: cpuidle: add LP2 support
2012-10-09 22:26 ` [PATCH 0/7] ARM: tegra30: cpuidle: add LP2 support Stephen Warren
@ 2012-10-11 6:39 ` Joseph Lo
0 siblings, 0 replies; 50+ messages in thread
From: Joseph Lo @ 2012-10-11 6:39 UTC (permalink / raw)
To: linux-arm-kernel
Hi Stephen,
Thanks for review and sorry for late response due to national holiday
yesterday.
On Wed, 2012-10-10 at 06:26 +0800, Stephen Warren wrote:
> On 10/08/2012 04:26 AM, Joseph Lo wrote:
> > The CPU idle LP2 is a power gating idle mode for Tegra30. It supports the
> > secondary CPUs (i.e., CPU1-CPU3) to go into LP2 dynamically. When any of
> > the secondary CPUs go into LP2, it can be power gated alone. There is a
> > limitation on CPU0. The CPU0 can go into LP2 only when all secondary CPUs
> > are already in LP2. After CPU0 is in LP2, the CPU rail can be turned off.
> >
> > Verified on Seaboard(Tegra20) and Cardhu(Tegra30).
>
> What's the most comprehensive way to verify this? I booted Cardhu with
> these patches applied and saw that all CPU cores did enter both idle
> states. However, I'm unsure what the best way to stress the system is,
> i.e. how would I stress and test for correct handling of all the L2
> caching/SMP coherency issues, etc.
Yes, we need a pattern to verify this. The idea is try to make data
traffic busy and the CPU can still fall into LP2. So I used a software
video decoding process to verify this. You can image how the data
traffic it is. The raw video data from file system to memory. Memory to
memory access during video decoding process. The L1/L2 cache and TLB
maintenance procedure can be verified.
To verify this, you need to get a mplayer and "lower resolution" video
clips. Using DVD resolution would be good choice. Then creating two
decoding processes in the background. Please do check the system can
fall into LP2. If not, please try to lower the fps value. Finally, using
loop to keep the decoding process always in the background.
The criteria to pass the verification. The system need to keep alive
over a weekend at least.
Thanks,
Joseph
^ permalink raw reply [flat|nested] 50+ messages in thread