* [PATCH 0/3] ARM: tegra114: cpuidle: add power down state
@ 2013-05-30 11:19 ` Joseph Lo
0 siblings, 0 replies; 22+ messages in thread
From: Joseph Lo @ 2013-05-30 11:19 UTC (permalink / raw)
To: Stephen Warren
Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Joseph Lo
This series introduce CPU core power down state for CPU idle. When CPU go
into this state, it saves it's context and needs a proper configuration
in flow controller to power gate the CPU when CPU runs into WFI
instruction. And the CPU also needs to set the IRQ as CPU power down idle
wake up event in flow controller.
To prevent race conditions and ensure proper interrupt routing on
Cortex-A15 CPUs when they are power-gated, add a CPU PM notifier
call-back to reprogram the GIC CPU interface on PM entry. The
GIC CPU interface will be reset back to its normal state by
the common GIC CPU PM exit callback when the CPU wakes up.
Joseph Lo (3):
ARM: tegra114: Reprogram GIC CPU interface to bypass IRQ on CPU PM
entry
ARM: tegra114: add low level support for CPU idle powered-down mode
ARM: tegra114: cpuidle: add powered-down state
arch/arm/mach-tegra/cpuidle-tegra114.c | 62 +++++++++++++++++++++++++++++++++-
arch/arm/mach-tegra/flowctrl.h | 2 ++
arch/arm/mach-tegra/irq.c | 40 ++++++++++++++++++++++
arch/arm/mach-tegra/sleep-tegra30.S | 2 ++
4 files changed, 105 insertions(+), 1 deletion(-)
--
1.8.3
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 0/3] ARM: tegra114: cpuidle: add power down state
@ 2013-05-30 11:19 ` Joseph Lo
0 siblings, 0 replies; 22+ messages in thread
From: Joseph Lo @ 2013-05-30 11:19 UTC (permalink / raw)
To: linux-arm-kernel
This series introduce CPU core power down state for CPU idle. When CPU go
into this state, it saves it's context and needs a proper configuration
in flow controller to power gate the CPU when CPU runs into WFI
instruction. And the CPU also needs to set the IRQ as CPU power down idle
wake up event in flow controller.
To prevent race conditions and ensure proper interrupt routing on
Cortex-A15 CPUs when they are power-gated, add a CPU PM notifier
call-back to reprogram the GIC CPU interface on PM entry. The
GIC CPU interface will be reset back to its normal state by
the common GIC CPU PM exit callback when the CPU wakes up.
Joseph Lo (3):
ARM: tegra114: Reprogram GIC CPU interface to bypass IRQ on CPU PM
entry
ARM: tegra114: add low level support for CPU idle powered-down mode
ARM: tegra114: cpuidle: add powered-down state
arch/arm/mach-tegra/cpuidle-tegra114.c | 62 +++++++++++++++++++++++++++++++++-
arch/arm/mach-tegra/flowctrl.h | 2 ++
arch/arm/mach-tegra/irq.c | 40 ++++++++++++++++++++++
arch/arm/mach-tegra/sleep-tegra30.S | 2 ++
4 files changed, 105 insertions(+), 1 deletion(-)
--
1.8.3
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 1/3] ARM: tegra114: Reprogram GIC CPU interface to bypass IRQ on CPU PM entry
2013-05-30 11:19 ` Joseph Lo
@ 2013-05-30 11:19 ` Joseph Lo
-1 siblings, 0 replies; 22+ messages in thread
From: Joseph Lo @ 2013-05-30 11:19 UTC (permalink / raw)
To: Stephen Warren
Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Joseph Lo
There is a difference between GICv1 and v2 when CPU in power management
mode (aka CPU power down on Tegra). For GICv1, IRQ/FIQ interrupt lines
going to CPU are same lines which are also used for wake-interrupt.
Therefore, we cannot disable the GIC CPU interface if we need to use same
interrupts for CPU wake purpose. This creates a race condition for CPU
power off entry. Also, in GICv1, disabling GICv1 CPU interface puts GICv1
into bypass mode such that incoming legacy IRQ/FIQ are sent to CPU, which
means disabling GIC CPU interface doesn't really disable IRQ/FIQ to CPU.
GICv2 provides a wake IRQ/FIQ (for wake-event purpose), which are not
disabled by GIC CPU interface. This is done by adding a bypass override
capability when the interrupts are disabled at the CPU interface. To
support this, there are four bits about IRQ/FIQ BypassDisable in CPU
interface Control Register. When the IRQ/FIQ not being driver by the
CPU interface, each interrupt output signal can be deasserted rather
than being driven by the legacy interrupt input.
So the wake-event can be used as wakeup signals to SoC (system power
controller).
To prevent race conditions and ensure proper interrupt routing on
Cortex-A15 CPUs when they are power-gated, add a CPU PM notifier
call-back to reprogram the GIC CPU interface on PM entry. The
GIC CPU interface will be reset back to its normal state by
the common GIC CPU PM exit callback when the CPU wakes up.
Based on the work by: Scott Williams <scwilliams-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Joseph Lo <josephl-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
arch/arm/mach-tegra/irq.c | 40 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 40 insertions(+)
diff --git a/arch/arm/mach-tegra/irq.c b/arch/arm/mach-tegra/irq.c
index 0de4eed..1a74d56 100644
--- a/arch/arm/mach-tegra/irq.c
+++ b/arch/arm/mach-tegra/irq.c
@@ -18,10 +18,12 @@
*/
#include <linux/kernel.h>
+#include <linux/cpu_pm.h>
#include <linux/interrupt.h>
#include <linux/irq.h>
#include <linux/io.h>
#include <linux/of.h>
+#include <linux/of_address.h>
#include <linux/irqchip/arm-gic.h>
#include <linux/syscore_ops.h>
@@ -65,6 +67,7 @@ static u32 cpu_ier[TEGRA_MAX_NUM_ICTLRS];
static u32 cpu_iep[TEGRA_MAX_NUM_ICTLRS];
static u32 ictlr_wake_mask[TEGRA_MAX_NUM_ICTLRS];
+static void __iomem *tegra_gic_cpu_base;
#endif
bool tegra_pending_sgi(void)
@@ -213,8 +216,43 @@ int tegra_legacy_irq_syscore_init(void)
return 0;
}
+
+static int tegra_gic_notifier(struct notifier_block *self,
+ unsigned long cmd, void *v)
+{
+ switch (cmd) {
+ case CPU_PM_ENTER:
+ writel_relaxed(0x1E0, tegra_gic_cpu_base + GIC_CPU_CTRL);
+ break;
+ }
+
+ return NOTIFY_OK;
+}
+
+static struct notifier_block tegra_gic_notifier_block = {
+ .notifier_call = tegra_gic_notifier,
+};
+
+static const struct of_device_id tegra114_dt_gic_match[] __initconst = {
+ { .compatible = "arm,cortex-a15-gic" },
+ { }
+};
+
+static void tegra114_gic_cpu_pm_registration(void)
+{
+ struct device_node *dn;
+
+ dn = of_find_matching_node(NULL, tegra114_dt_gic_match);
+ if (!dn)
+ return;
+
+ tegra_gic_cpu_base = of_iomap(dn, 1);
+
+ cpu_pm_register_notifier(&tegra_gic_notifier_block);
+}
#else
#define tegra_set_wake NULL
+static void tegra114_gic_cpu_pm_registration(void) { }
#endif
void __init tegra_init_irq(void)
@@ -252,4 +290,6 @@ void __init tegra_init_irq(void)
if (!of_have_populated_dt())
gic_init(0, 29, distbase,
IO_ADDRESS(TEGRA_ARM_PERIF_BASE + 0x100));
+
+ tegra114_gic_cpu_pm_registration();
}
--
1.8.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 1/3] ARM: tegra114: Reprogram GIC CPU interface to bypass IRQ on CPU PM entry
@ 2013-05-30 11:19 ` Joseph Lo
0 siblings, 0 replies; 22+ messages in thread
From: Joseph Lo @ 2013-05-30 11:19 UTC (permalink / raw)
To: linux-arm-kernel
There is a difference between GICv1 and v2 when CPU in power management
mode (aka CPU power down on Tegra). For GICv1, IRQ/FIQ interrupt lines
going to CPU are same lines which are also used for wake-interrupt.
Therefore, we cannot disable the GIC CPU interface if we need to use same
interrupts for CPU wake purpose. This creates a race condition for CPU
power off entry. Also, in GICv1, disabling GICv1 CPU interface puts GICv1
into bypass mode such that incoming legacy IRQ/FIQ are sent to CPU, which
means disabling GIC CPU interface doesn't really disable IRQ/FIQ to CPU.
GICv2 provides a wake IRQ/FIQ (for wake-event purpose), which are not
disabled by GIC CPU interface. This is done by adding a bypass override
capability when the interrupts are disabled at the CPU interface. To
support this, there are four bits about IRQ/FIQ BypassDisable in CPU
interface Control Register. When the IRQ/FIQ not being driver by the
CPU interface, each interrupt output signal can be deasserted rather
than being driven by the legacy interrupt input.
So the wake-event can be used as wakeup signals to SoC (system power
controller).
To prevent race conditions and ensure proper interrupt routing on
Cortex-A15 CPUs when they are power-gated, add a CPU PM notifier
call-back to reprogram the GIC CPU interface on PM entry. The
GIC CPU interface will be reset back to its normal state by
the common GIC CPU PM exit callback when the CPU wakes up.
Based on the work by: Scott Williams <scwilliams@nvidia.com>
Signed-off-by: Joseph Lo <josephl@nvidia.com>
---
arch/arm/mach-tegra/irq.c | 40 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 40 insertions(+)
diff --git a/arch/arm/mach-tegra/irq.c b/arch/arm/mach-tegra/irq.c
index 0de4eed..1a74d56 100644
--- a/arch/arm/mach-tegra/irq.c
+++ b/arch/arm/mach-tegra/irq.c
@@ -18,10 +18,12 @@
*/
#include <linux/kernel.h>
+#include <linux/cpu_pm.h>
#include <linux/interrupt.h>
#include <linux/irq.h>
#include <linux/io.h>
#include <linux/of.h>
+#include <linux/of_address.h>
#include <linux/irqchip/arm-gic.h>
#include <linux/syscore_ops.h>
@@ -65,6 +67,7 @@ static u32 cpu_ier[TEGRA_MAX_NUM_ICTLRS];
static u32 cpu_iep[TEGRA_MAX_NUM_ICTLRS];
static u32 ictlr_wake_mask[TEGRA_MAX_NUM_ICTLRS];
+static void __iomem *tegra_gic_cpu_base;
#endif
bool tegra_pending_sgi(void)
@@ -213,8 +216,43 @@ int tegra_legacy_irq_syscore_init(void)
return 0;
}
+
+static int tegra_gic_notifier(struct notifier_block *self,
+ unsigned long cmd, void *v)
+{
+ switch (cmd) {
+ case CPU_PM_ENTER:
+ writel_relaxed(0x1E0, tegra_gic_cpu_base + GIC_CPU_CTRL);
+ break;
+ }
+
+ return NOTIFY_OK;
+}
+
+static struct notifier_block tegra_gic_notifier_block = {
+ .notifier_call = tegra_gic_notifier,
+};
+
+static const struct of_device_id tegra114_dt_gic_match[] __initconst = {
+ { .compatible = "arm,cortex-a15-gic" },
+ { }
+};
+
+static void tegra114_gic_cpu_pm_registration(void)
+{
+ struct device_node *dn;
+
+ dn = of_find_matching_node(NULL, tegra114_dt_gic_match);
+ if (!dn)
+ return;
+
+ tegra_gic_cpu_base = of_iomap(dn, 1);
+
+ cpu_pm_register_notifier(&tegra_gic_notifier_block);
+}
#else
#define tegra_set_wake NULL
+static void tegra114_gic_cpu_pm_registration(void) { }
#endif
void __init tegra_init_irq(void)
@@ -252,4 +290,6 @@ void __init tegra_init_irq(void)
if (!of_have_populated_dt())
gic_init(0, 29, distbase,
IO_ADDRESS(TEGRA_ARM_PERIF_BASE + 0x100));
+
+ tegra114_gic_cpu_pm_registration();
}
--
1.8.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 2/3] ARM: tegra114: add low level support for CPU idle powered-down mode
2013-05-30 11:19 ` Joseph Lo
@ 2013-05-30 11:19 ` Joseph Lo
-1 siblings, 0 replies; 22+ messages in thread
From: Joseph Lo @ 2013-05-30 11:19 UTC (permalink / raw)
To: Stephen Warren
Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Joseph Lo
The flow controller would take care the power sequence when CPU idle in
powered-down mode. It powered gate the CPU when CPU runs into WFI
instruction. And wake up the CPU when event be triggered.
The sequence is below.
* setting wfi bitmap for the CPU as the halt event in the
FLOW_CTRL_CPU_HALT_REG to monitor the CPU running into WFI,then power
gate it
* setting IRQ and FIQ as wake up event to wake up CPU when event triggered
Signed-off-by: Joseph Lo <josephl-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
arch/arm/mach-tegra/flowctrl.h | 2 ++
arch/arm/mach-tegra/sleep-tegra30.S | 2 ++
2 files changed, 4 insertions(+)
diff --git a/arch/arm/mach-tegra/flowctrl.h b/arch/arm/mach-tegra/flowctrl.h
index 7a29bae..e56a950 100644
--- a/arch/arm/mach-tegra/flowctrl.h
+++ b/arch/arm/mach-tegra/flowctrl.h
@@ -28,6 +28,8 @@
#define FLOW_CTRL_SCLK_RESUME (1 << 27)
#define FLOW_CTRL_HALT_CPU_IRQ (1 << 10)
#define FLOW_CTRL_HALT_CPU_FIQ (1 << 8)
+#define FLOW_CTRL_HALT_GIC_IRQ (1 << 9)
+#define FLOW_CTRL_HALT_GIC_FIQ (1 << 8)
#define FLOW_CTRL_CPU0_CSR 0x8
#define FLOW_CTRL_CSR_INTR_FLAG (1 << 15)
#define FLOW_CTRL_CSR_EVENT_FLAG (1 << 14)
diff --git a/arch/arm/mach-tegra/sleep-tegra30.S b/arch/arm/mach-tegra/sleep-tegra30.S
index ada8821..5877f26 100644
--- a/arch/arm/mach-tegra/sleep-tegra30.S
+++ b/arch/arm/mach-tegra/sleep-tegra30.S
@@ -99,6 +99,8 @@ flow_ctrl_setting_for_lp2:
cmp r10, #TEGRA30
moveq r3, #FLOW_CTRL_WAIT_FOR_INTERRUPT @ For LP2
movne r3, #FLOW_CTRL_WAITEVENT
+ orrne r3, r3, #FLOW_CTRL_HALT_GIC_IRQ
+ orrne r3, r3, #FLOW_CTRL_HALT_GIC_FIQ
flow_ctrl_done:
cmp r10, #TEGRA30
str r3, [r2]
--
1.8.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 2/3] ARM: tegra114: add low level support for CPU idle powered-down mode
@ 2013-05-30 11:19 ` Joseph Lo
0 siblings, 0 replies; 22+ messages in thread
From: Joseph Lo @ 2013-05-30 11:19 UTC (permalink / raw)
To: linux-arm-kernel
The flow controller would take care the power sequence when CPU idle in
powered-down mode. It powered gate the CPU when CPU runs into WFI
instruction. And wake up the CPU when event be triggered.
The sequence is below.
* setting wfi bitmap for the CPU as the halt event in the
FLOW_CTRL_CPU_HALT_REG to monitor the CPU running into WFI,then power
gate it
* setting IRQ and FIQ as wake up event to wake up CPU when event triggered
Signed-off-by: Joseph Lo <josephl@nvidia.com>
---
arch/arm/mach-tegra/flowctrl.h | 2 ++
arch/arm/mach-tegra/sleep-tegra30.S | 2 ++
2 files changed, 4 insertions(+)
diff --git a/arch/arm/mach-tegra/flowctrl.h b/arch/arm/mach-tegra/flowctrl.h
index 7a29bae..e56a950 100644
--- a/arch/arm/mach-tegra/flowctrl.h
+++ b/arch/arm/mach-tegra/flowctrl.h
@@ -28,6 +28,8 @@
#define FLOW_CTRL_SCLK_RESUME (1 << 27)
#define FLOW_CTRL_HALT_CPU_IRQ (1 << 10)
#define FLOW_CTRL_HALT_CPU_FIQ (1 << 8)
+#define FLOW_CTRL_HALT_GIC_IRQ (1 << 9)
+#define FLOW_CTRL_HALT_GIC_FIQ (1 << 8)
#define FLOW_CTRL_CPU0_CSR 0x8
#define FLOW_CTRL_CSR_INTR_FLAG (1 << 15)
#define FLOW_CTRL_CSR_EVENT_FLAG (1 << 14)
diff --git a/arch/arm/mach-tegra/sleep-tegra30.S b/arch/arm/mach-tegra/sleep-tegra30.S
index ada8821..5877f26 100644
--- a/arch/arm/mach-tegra/sleep-tegra30.S
+++ b/arch/arm/mach-tegra/sleep-tegra30.S
@@ -99,6 +99,8 @@ flow_ctrl_setting_for_lp2:
cmp r10, #TEGRA30
moveq r3, #FLOW_CTRL_WAIT_FOR_INTERRUPT @ For LP2
movne r3, #FLOW_CTRL_WAITEVENT
+ orrne r3, r3, #FLOW_CTRL_HALT_GIC_IRQ
+ orrne r3, r3, #FLOW_CTRL_HALT_GIC_FIQ
flow_ctrl_done:
cmp r10, #TEGRA30
str r3, [r2]
--
1.8.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 3/3] ARM: tegra114: cpuidle: add powered-down state
2013-05-30 11:19 ` Joseph Lo
@ 2013-05-30 11:19 ` Joseph Lo
-1 siblings, 0 replies; 22+ messages in thread
From: Joseph Lo @ 2013-05-30 11:19 UTC (permalink / raw)
To: Stephen Warren
Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Joseph Lo
This supports CPU core power down on each CPU when CPU idle. When CPU go
into this state, it saves it's context and needs a proper configuration
in flow controller to power gate the CPU when CPU runs into WFI
instruction. And the CPU also needs to set the IRQ as CPU power down idle
wake up event in flow controller.
Signed-off-by: Joseph Lo <josephl-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
arch/arm/mach-tegra/cpuidle-tegra114.c | 62 +++++++++++++++++++++++++++++++++-
1 file changed, 61 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-tegra/cpuidle-tegra114.c b/arch/arm/mach-tegra/cpuidle-tegra114.c
index 1d1c602..e7d21f5 100644
--- a/arch/arm/mach-tegra/cpuidle-tegra114.c
+++ b/arch/arm/mach-tegra/cpuidle-tegra114.c
@@ -17,19 +17,79 @@
#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/suspend.h>
+#include <asm/smp_plat.h>
+
+#include "pm.h"
+#include "sleep.h"
+
+#ifdef CONFIG_PM_SLEEP
+static int tegra114_idle_power_down(struct cpuidle_device *dev,
+ struct cpuidle_driver *drv,
+ int index);
+#define TEGRA114_MAX_STATES 2
+#else
+#define TEGRA114_MAX_STATES 1
+#endif
static struct cpuidle_driver tegra_idle_driver = {
.name = "tegra_idle",
.owner = THIS_MODULE,
- .state_count = 1,
+ .state_count = TEGRA114_MAX_STATES,
.states = {
[0] = ARM_CPUIDLE_WFI_STATE_PWR(600),
+#ifdef CONFIG_PM_SLEEP
+ [1] = {
+ .enter = tegra114_idle_power_down,
+ .exit_latency = 500,
+ .target_residency = 1000,
+ .power_usage = 0,
+ .flags = CPUIDLE_FLAG_TIME_VALID,
+ .name = "powered-down",
+ .desc = "CPU power gated",
+ },
+#endif
},
};
+#ifdef CONFIG_PM_SLEEP
+static int tegra114_idle_power_down(struct cpuidle_device *dev,
+ struct cpuidle_driver *drv,
+ int index)
+{
+ u32 cpu = is_smp() ? cpu_logical_map(dev->cpu) : dev->cpu;
+
+ local_fiq_disable();
+
+ tegra_set_cpu_in_lp2(cpu);
+ cpu_pm_enter();
+
+ clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &dev->cpu);
+ smp_wmb();
+
+ cpu_suspend(0, tegra30_sleep_cpu_secondary_finish);
+
+ clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &dev->cpu);
+
+ cpu_pm_exit();
+ tegra_clear_cpu_in_lp2(cpu);
+
+ local_fiq_enable();
+
+ smp_rmb();
+
+ return index;
+}
+#endif
+
int __init tegra114_cpuidle_init(void)
{
+#ifdef CONFIG_PM_SLEEP
+ tegra_tear_down_cpu = tegra30_tear_down_cpu;
+#endif
return cpuidle_register(&tegra_idle_driver, NULL);
}
--
1.8.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 3/3] ARM: tegra114: cpuidle: add powered-down state
@ 2013-05-30 11:19 ` Joseph Lo
0 siblings, 0 replies; 22+ messages in thread
From: Joseph Lo @ 2013-05-30 11:19 UTC (permalink / raw)
To: linux-arm-kernel
This supports CPU core power down on each CPU when CPU idle. When CPU go
into this state, it saves it's context and needs a proper configuration
in flow controller to power gate the CPU when CPU runs into WFI
instruction. And the CPU also needs to set the IRQ as CPU power down idle
wake up event in flow controller.
Signed-off-by: Joseph Lo <josephl@nvidia.com>
---
arch/arm/mach-tegra/cpuidle-tegra114.c | 62 +++++++++++++++++++++++++++++++++-
1 file changed, 61 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-tegra/cpuidle-tegra114.c b/arch/arm/mach-tegra/cpuidle-tegra114.c
index 1d1c602..e7d21f5 100644
--- a/arch/arm/mach-tegra/cpuidle-tegra114.c
+++ b/arch/arm/mach-tegra/cpuidle-tegra114.c
@@ -17,19 +17,79 @@
#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/suspend.h>
+#include <asm/smp_plat.h>
+
+#include "pm.h"
+#include "sleep.h"
+
+#ifdef CONFIG_PM_SLEEP
+static int tegra114_idle_power_down(struct cpuidle_device *dev,
+ struct cpuidle_driver *drv,
+ int index);
+#define TEGRA114_MAX_STATES 2
+#else
+#define TEGRA114_MAX_STATES 1
+#endif
static struct cpuidle_driver tegra_idle_driver = {
.name = "tegra_idle",
.owner = THIS_MODULE,
- .state_count = 1,
+ .state_count = TEGRA114_MAX_STATES,
.states = {
[0] = ARM_CPUIDLE_WFI_STATE_PWR(600),
+#ifdef CONFIG_PM_SLEEP
+ [1] = {
+ .enter = tegra114_idle_power_down,
+ .exit_latency = 500,
+ .target_residency = 1000,
+ .power_usage = 0,
+ .flags = CPUIDLE_FLAG_TIME_VALID,
+ .name = "powered-down",
+ .desc = "CPU power gated",
+ },
+#endif
},
};
+#ifdef CONFIG_PM_SLEEP
+static int tegra114_idle_power_down(struct cpuidle_device *dev,
+ struct cpuidle_driver *drv,
+ int index)
+{
+ u32 cpu = is_smp() ? cpu_logical_map(dev->cpu) : dev->cpu;
+
+ local_fiq_disable();
+
+ tegra_set_cpu_in_lp2(cpu);
+ cpu_pm_enter();
+
+ clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &dev->cpu);
+ smp_wmb();
+
+ cpu_suspend(0, tegra30_sleep_cpu_secondary_finish);
+
+ clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &dev->cpu);
+
+ cpu_pm_exit();
+ tegra_clear_cpu_in_lp2(cpu);
+
+ local_fiq_enable();
+
+ smp_rmb();
+
+ return index;
+}
+#endif
+
int __init tegra114_cpuidle_init(void)
{
+#ifdef CONFIG_PM_SLEEP
+ tegra_tear_down_cpu = tegra30_tear_down_cpu;
+#endif
return cpuidle_register(&tegra_idle_driver, NULL);
}
--
1.8.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH 3/3] ARM: tegra114: cpuidle: add powered-down state
2013-05-30 11:19 ` Joseph Lo
@ 2013-05-30 14:35 ` Daniel Lezcano
-1 siblings, 0 replies; 22+ messages in thread
From: Daniel Lezcano @ 2013-05-30 14:35 UTC (permalink / raw)
To: Joseph Lo
Cc: Stephen Warren, linux-tegra, linux-arm-kernel, Linux PM mailing list
On 05/30/2013 01:19 PM, Joseph Lo wrote:
> This supports CPU core power down on each CPU when CPU idle. When CPU go
> into this state, it saves it's context and needs a proper configuration
> in flow controller to power gate the CPU when CPU runs into WFI
> instruction. And the CPU also needs to set the IRQ as CPU power down idle
> wake up event in flow controller.
>
> Signed-off-by: Joseph Lo <josephl@nvidia.com>
> ---
Hi Joseph,
a new flag has been added in the cpuidle framework to let this one to
handle the timer broadcast : CPUIDLE_FLAG_TIMER_STOP
It is the commit b60e6a0eb0273132cbb60a9806abf5f47a4aee1c
You can get rid of the clockevent notify stuff by adding this flag to
the state.
Also, can you clarify why tegra3 code is used in tegra114 ?
> arch/arm/mach-tegra/cpuidle-tegra114.c | 62 +++++++++++++++++++++++++++++++++-
> 1 file changed, 61 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-tegra/cpuidle-tegra114.c b/arch/arm/mach-tegra/cpuidle-tegra114.c
> index 1d1c602..e7d21f5 100644
> --- a/arch/arm/mach-tegra/cpuidle-tegra114.c
> +++ b/arch/arm/mach-tegra/cpuidle-tegra114.c
> @@ -17,19 +17,79 @@
> #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/suspend.h>
> +#include <asm/smp_plat.h>
> +
> +#include "pm.h"
> +#include "sleep.h"
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int tegra114_idle_power_down(struct cpuidle_device *dev,
> + struct cpuidle_driver *drv,
> + int index);
Isn't possible to move the driver declaration after the
tegra114_idle_power_down function, before the init function, so we
prevent a forward declaration ?
> +#define TEGRA114_MAX_STATES 2
> +#else
> +#define TEGRA114_MAX_STATES 1
> +#endif
>
> static struct cpuidle_driver tegra_idle_driver = {
> .name = "tegra_idle",
> .owner = THIS_MODULE,
> - .state_count = 1,
> + .state_count = TEGRA114_MAX_STATES,
> .states = {
> [0] = ARM_CPUIDLE_WFI_STATE_PWR(600),
> +#ifdef CONFIG_PM_SLEEP
> + [1] = {
> + .enter = tegra114_idle_power_down,
> + .exit_latency = 500,
> + .target_residency = 1000,
> + .power_usage = 0,
> + .flags = CPUIDLE_FLAG_TIME_VALID,
> + .name = "powered-down",
> + .desc = "CPU power gated",
> + },
> +#endif
> },
> };
>
> +#ifdef CONFIG_PM_SLEEP
> +static int tegra114_idle_power_down(struct cpuidle_device *dev,
> + struct cpuidle_driver *drv,
> + int index)
> +{
> + u32 cpu = is_smp() ? cpu_logical_map(dev->cpu) : dev->cpu;
IMO, it is up to the tegra_set_cpu_in_lp2 / tegra_clear_cpu_in_lp2
functions to do the cpu map conversion instead of adding this into a
routine working with logical cpu.
> + local_fiq_disable();
> +
> + tegra_set_cpu_in_lp2(cpu);
> + cpu_pm_enter();
> +
> + clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &dev->cpu);
> + smp_wmb();
Why do you need a memory barrier here ?
> + cpu_suspend(0, tegra30_sleep_cpu_secondary_finish);
> +
> + clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &dev->cpu);
> +
> + cpu_pm_exit();
> + tegra_clear_cpu_in_lp2(cpu);
> +
> + local_fiq_enable();
> +
> + smp_rmb();
Why do you need a memory barrier here ?
> + return index;
> +}
> +#endif
> +
> int __init tegra114_cpuidle_init(void)
> {
> +#ifdef CONFIG_PM_SLEEP
> + tegra_tear_down_cpu = tegra30_tear_down_cpu;
> +#endif
Doesn't this code should go in the pm.c file ?
> return cpuidle_register(&tegra_idle_driver, NULL);
> }
>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 3/3] ARM: tegra114: cpuidle: add powered-down state
@ 2013-05-30 14:35 ` Daniel Lezcano
0 siblings, 0 replies; 22+ messages in thread
From: Daniel Lezcano @ 2013-05-30 14:35 UTC (permalink / raw)
To: linux-arm-kernel
On 05/30/2013 01:19 PM, Joseph Lo wrote:
> This supports CPU core power down on each CPU when CPU idle. When CPU go
> into this state, it saves it's context and needs a proper configuration
> in flow controller to power gate the CPU when CPU runs into WFI
> instruction. And the CPU also needs to set the IRQ as CPU power down idle
> wake up event in flow controller.
>
> Signed-off-by: Joseph Lo <josephl@nvidia.com>
> ---
Hi Joseph,
a new flag has been added in the cpuidle framework to let this one to
handle the timer broadcast : CPUIDLE_FLAG_TIMER_STOP
It is the commit b60e6a0eb0273132cbb60a9806abf5f47a4aee1c
You can get rid of the clockevent notify stuff by adding this flag to
the state.
Also, can you clarify why tegra3 code is used in tegra114 ?
> arch/arm/mach-tegra/cpuidle-tegra114.c | 62 +++++++++++++++++++++++++++++++++-
> 1 file changed, 61 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-tegra/cpuidle-tegra114.c b/arch/arm/mach-tegra/cpuidle-tegra114.c
> index 1d1c602..e7d21f5 100644
> --- a/arch/arm/mach-tegra/cpuidle-tegra114.c
> +++ b/arch/arm/mach-tegra/cpuidle-tegra114.c
> @@ -17,19 +17,79 @@
> #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/suspend.h>
> +#include <asm/smp_plat.h>
> +
> +#include "pm.h"
> +#include "sleep.h"
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int tegra114_idle_power_down(struct cpuidle_device *dev,
> + struct cpuidle_driver *drv,
> + int index);
Isn't possible to move the driver declaration after the
tegra114_idle_power_down function, before the init function, so we
prevent a forward declaration ?
> +#define TEGRA114_MAX_STATES 2
> +#else
> +#define TEGRA114_MAX_STATES 1
> +#endif
>
> static struct cpuidle_driver tegra_idle_driver = {
> .name = "tegra_idle",
> .owner = THIS_MODULE,
> - .state_count = 1,
> + .state_count = TEGRA114_MAX_STATES,
> .states = {
> [0] = ARM_CPUIDLE_WFI_STATE_PWR(600),
> +#ifdef CONFIG_PM_SLEEP
> + [1] = {
> + .enter = tegra114_idle_power_down,
> + .exit_latency = 500,
> + .target_residency = 1000,
> + .power_usage = 0,
> + .flags = CPUIDLE_FLAG_TIME_VALID,
> + .name = "powered-down",
> + .desc = "CPU power gated",
> + },
> +#endif
> },
> };
>
> +#ifdef CONFIG_PM_SLEEP
> +static int tegra114_idle_power_down(struct cpuidle_device *dev,
> + struct cpuidle_driver *drv,
> + int index)
> +{
> + u32 cpu = is_smp() ? cpu_logical_map(dev->cpu) : dev->cpu;
IMO, it is up to the tegra_set_cpu_in_lp2 / tegra_clear_cpu_in_lp2
functions to do the cpu map conversion instead of adding this into a
routine working with logical cpu.
> + local_fiq_disable();
> +
> + tegra_set_cpu_in_lp2(cpu);
> + cpu_pm_enter();
> +
> + clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &dev->cpu);
> + smp_wmb();
Why do you need a memory barrier here ?
> + cpu_suspend(0, tegra30_sleep_cpu_secondary_finish);
> +
> + clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &dev->cpu);
> +
> + cpu_pm_exit();
> + tegra_clear_cpu_in_lp2(cpu);
> +
> + local_fiq_enable();
> +
> + smp_rmb();
Why do you need a memory barrier here ?
> + return index;
> +}
> +#endif
> +
> int __init tegra114_cpuidle_init(void)
> {
> +#ifdef CONFIG_PM_SLEEP
> + tegra_tear_down_cpu = tegra30_tear_down_cpu;
> +#endif
Doesn't this code should go in the pm.c file ?
> return cpuidle_register(&tegra_idle_driver, NULL);
> }
>
--
<http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/3] ARM: tegra114: cpuidle: add powered-down state
2013-05-30 14:35 ` Daniel Lezcano
@ 2013-05-31 9:12 ` Joseph Lo
-1 siblings, 0 replies; 22+ messages in thread
From: Joseph Lo @ 2013-05-31 9:12 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Stephen Warren, linux-tegra-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Linux PM mailing list
Hi Daniel,
Thanks for your review.
On Thu, 2013-05-30 at 22:35 +0800, Daniel Lezcano wrote:
> On 05/30/2013 01:19 PM, Joseph Lo wrote:
> > This supports CPU core power down on each CPU when CPU idle. When CPU go
> > into this state, it saves it's context and needs a proper configuration
> > in flow controller to power gate the CPU when CPU runs into WFI
> > instruction. And the CPU also needs to set the IRQ as CPU power down idle
> > wake up event in flow controller.
> >
> > Signed-off-by: Joseph Lo <josephl-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> > ---
>
> Hi Joseph,
>
> a new flag has been added in the cpuidle framework to let this one to
> handle the timer broadcast : CPUIDLE_FLAG_TIMER_STOP
>
> It is the commit b60e6a0eb0273132cbb60a9806abf5f47a4aee1c
>
> You can get rid of the clockevent notify stuff by adding this flag to
> the state.
>
I just tested this flag on Tegra20, 30 and 114. It is only OK on
Tegra20. It caused a warning message below on Tegra30/114 at boot time.
I gave it a quick check I found it can't clean
tick_broadcast_pending_mask sometimes if apply the flag. But I keep the
code right now it's always OK. Not sure why?
[ 25.629539] ------------[ cut here ]------------
[ 25.629559] WARNING: CPU: 2 PID: 0 at
kernel/time/tick-broadcast.c:578 tick_broadcast_oneshot_control
+0x1a4/0x1d0()
[ 25.629565] Modules linked in:
[ 25.629574] CPU: 2 PID: 0 Comm: swapper/2 Not tainted
3.10.0-rc3-next-20130529+ #15
[ 25.629601] [<c00148f4>] (unwind_backtrace+0x0/0xf8) from
[<c0011040>] (show_stack+0x10/0x14)
[ 25.629616] [<c0011040>] (show_stack+0x10/0x14) from [<c0504248>]
(dump_stack+0x80/0xc4)
[ 25.629633] [<c0504248>] (dump_stack+0x80/0xc4) from [<c00231ac>]
(warn_slowpath_common+0x64/0x88)
[ 25.629646] [<c00231ac>] (warn_slowpath_common+0x64/0x88) from
[<c00231ec>] (warn_slowpath_null+0x1c/0x24)
[ 25.629657] [<c00231ec>] (warn_slowpath_null+0x1c/0x24) from
[<c00667f8>] (tick_broadcast_oneshot_control+0x1a4/0x1d0)
[ 25.629670] [<c00667f8>] (tick_broadcast_oneshot_control+0x1a4/0x1d0)
from [<c0065cd0>] (tick_notify+0x240/0x40c)
[ 25.629685] [<c0065cd0>] (tick_notify+0x240/0x40c) from [<c0044724>]
(notifier_call_chain+0x44/0x84)
[ 25.629699] [<c0044724>] (notifier_call_chain+0x44/0x84) from
[<c0044828>] (raw_notifier_call_chain+0x18/0x20)
[ 25.629712] [<c0044828>] (raw_notifier_call_chain+0x18/0x20) from
[<c00650cc>] (clockevents_notify+0x28/0x170)
[ 25.629726] [<c00650cc>] (clockevents_notify+0x28/0x170) from
[<c033f1f0>] (cpuidle_idle_call+0x11c/0x168)
[ 25.629739] [<c033f1f0>] (cpuidle_idle_call+0x11c/0x168) from
[<c000ea94>] (arch_cpu_idle+0x8/0x38)
[ 25.629755] [<c000ea94>] (arch_cpu_idle+0x8/0x38) from [<c005ea80>]
(cpu_startup_entry+0x60/0x134)
[ 25.629767] [<c005ea80>] (cpu_startup_entry+0x60/0x134) from
[<804fe9a4>] (0x804fe9a4)
[ 25.629772] ---[ end trace 5484e77e2531bccc ]---
> Also, can you clarify why tegra3 code is used in tegra114 ?
>
Yes, because the flow controller that was used to control CPU power is
similar for both SoCs. The function is shared for Tegra30/114.
>
> > arch/arm/mach-tegra/cpuidle-tegra114.c | 62 +++++++++++++++++++++++++++++++++-
> > 1 file changed, 61 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/mach-tegra/cpuidle-tegra114.c b/arch/arm/mach-tegra/cpuidle-tegra114.c
> > index 1d1c602..e7d21f5 100644
> > --- a/arch/arm/mach-tegra/cpuidle-tegra114.c
> > +++ b/arch/arm/mach-tegra/cpuidle-tegra114.c
> > @@ -17,19 +17,79 @@
> > #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/suspend.h>
> > +#include <asm/smp_plat.h>
> > +
> > +#include "pm.h"
> > +#include "sleep.h"
> > +
> > +#ifdef CONFIG_PM_SLEEP
> > +static int tegra114_idle_power_down(struct cpuidle_device *dev,
> > + struct cpuidle_driver *drv,
> > + int index);
>
> Isn't possible to move the driver declaration after the
> tegra114_idle_power_down function, before the init function, so we
> prevent a forward declaration ?
>
> > +#define TEGRA114_MAX_STATES 2
> > +#else
> > +#define TEGRA114_MAX_STATES 1
> > +#endif
> >
> > static struct cpuidle_driver tegra_idle_driver = {
> > .name = "tegra_idle",
> > .owner = THIS_MODULE,
> > - .state_count = 1,
> > + .state_count = TEGRA114_MAX_STATES,
> > .states = {
> > [0] = ARM_CPUIDLE_WFI_STATE_PWR(600),
> > +#ifdef CONFIG_PM_SLEEP
> > + [1] = {
> > + .enter = tegra114_idle_power_down,
> > + .exit_latency = 500,
> > + .target_residency = 1000,
> > + .power_usage = 0,
> > + .flags = CPUIDLE_FLAG_TIME_VALID,
> > + .name = "powered-down",
> > + .desc = "CPU power gated",
> > + },
> > +#endif
> > },
> > };
> >
> > +#ifdef CONFIG_PM_SLEEP
> > +static int tegra114_idle_power_down(struct cpuidle_device *dev,
> > + struct cpuidle_driver *drv,
> > + int index)
> > +{
> > + u32 cpu = is_smp() ? cpu_logical_map(dev->cpu) : dev->cpu;
>
> IMO, it is up to the tegra_set_cpu_in_lp2 / tegra_clear_cpu_in_lp2
> functions to do the cpu map conversion instead of adding this into a
> routine working with logical cpu.
>
Good idea, I can create another patch to do that.
> > + local_fiq_disable();
> > +
> > + tegra_set_cpu_in_lp2(cpu);
> > + cpu_pm_enter();
> > +
> > + clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &dev->cpu);
> > + smp_wmb();
>
> Why do you need a memory barrier here ?
>
Yeah, Looks I don't have any static data that needs a barrier to sync
data for SMP here. Will remove them.
> > + cpu_suspend(0, tegra30_sleep_cpu_secondary_finish);
> > +
> > + clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &dev->cpu);
> > +
> > + cpu_pm_exit();
> > + tegra_clear_cpu_in_lp2(cpu);
> > +
> > + local_fiq_enable();
> > +
> > + smp_rmb();
>
> Why do you need a memory barrier here ?
>
> > + return index;
> > +}
> > +#endif
> > +
> > int __init tegra114_cpuidle_init(void)
> > {
> > +#ifdef CONFIG_PM_SLEEP
> > + tegra_tear_down_cpu = tegra30_tear_down_cpu;
> > +#endif
>
> Doesn't this code should go in the pm.c file ?
>
Yes, I had a plan to do that too. Currently It had a timing issue about
this. Because the CPU idle driver was registered by device_initcall, but
the PM init function was registered at late init. I need to sync the
init time first.
OK, will do that too.
Thanks,
Joseph
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 3/3] ARM: tegra114: cpuidle: add powered-down state
@ 2013-05-31 9:12 ` Joseph Lo
0 siblings, 0 replies; 22+ messages in thread
From: Joseph Lo @ 2013-05-31 9:12 UTC (permalink / raw)
To: linux-arm-kernel
Hi Daniel,
Thanks for your review.
On Thu, 2013-05-30 at 22:35 +0800, Daniel Lezcano wrote:
> On 05/30/2013 01:19 PM, Joseph Lo wrote:
> > This supports CPU core power down on each CPU when CPU idle. When CPU go
> > into this state, it saves it's context and needs a proper configuration
> > in flow controller to power gate the CPU when CPU runs into WFI
> > instruction. And the CPU also needs to set the IRQ as CPU power down idle
> > wake up event in flow controller.
> >
> > Signed-off-by: Joseph Lo <josephl@nvidia.com>
> > ---
>
> Hi Joseph,
>
> a new flag has been added in the cpuidle framework to let this one to
> handle the timer broadcast : CPUIDLE_FLAG_TIMER_STOP
>
> It is the commit b60e6a0eb0273132cbb60a9806abf5f47a4aee1c
>
> You can get rid of the clockevent notify stuff by adding this flag to
> the state.
>
I just tested this flag on Tegra20, 30 and 114. It is only OK on
Tegra20. It caused a warning message below on Tegra30/114 at boot time.
I gave it a quick check I found it can't clean
tick_broadcast_pending_mask sometimes if apply the flag. But I keep the
code right now it's always OK. Not sure why?
[ 25.629539] ------------[ cut here ]------------
[ 25.629559] WARNING: CPU: 2 PID: 0 at
kernel/time/tick-broadcast.c:578 tick_broadcast_oneshot_control
+0x1a4/0x1d0()
[ 25.629565] Modules linked in:
[ 25.629574] CPU: 2 PID: 0 Comm: swapper/2 Not tainted
3.10.0-rc3-next-20130529+ #15
[ 25.629601] [<c00148f4>] (unwind_backtrace+0x0/0xf8) from
[<c0011040>] (show_stack+0x10/0x14)
[ 25.629616] [<c0011040>] (show_stack+0x10/0x14) from [<c0504248>]
(dump_stack+0x80/0xc4)
[ 25.629633] [<c0504248>] (dump_stack+0x80/0xc4) from [<c00231ac>]
(warn_slowpath_common+0x64/0x88)
[ 25.629646] [<c00231ac>] (warn_slowpath_common+0x64/0x88) from
[<c00231ec>] (warn_slowpath_null+0x1c/0x24)
[ 25.629657] [<c00231ec>] (warn_slowpath_null+0x1c/0x24) from
[<c00667f8>] (tick_broadcast_oneshot_control+0x1a4/0x1d0)
[ 25.629670] [<c00667f8>] (tick_broadcast_oneshot_control+0x1a4/0x1d0)
from [<c0065cd0>] (tick_notify+0x240/0x40c)
[ 25.629685] [<c0065cd0>] (tick_notify+0x240/0x40c) from [<c0044724>]
(notifier_call_chain+0x44/0x84)
[ 25.629699] [<c0044724>] (notifier_call_chain+0x44/0x84) from
[<c0044828>] (raw_notifier_call_chain+0x18/0x20)
[ 25.629712] [<c0044828>] (raw_notifier_call_chain+0x18/0x20) from
[<c00650cc>] (clockevents_notify+0x28/0x170)
[ 25.629726] [<c00650cc>] (clockevents_notify+0x28/0x170) from
[<c033f1f0>] (cpuidle_idle_call+0x11c/0x168)
[ 25.629739] [<c033f1f0>] (cpuidle_idle_call+0x11c/0x168) from
[<c000ea94>] (arch_cpu_idle+0x8/0x38)
[ 25.629755] [<c000ea94>] (arch_cpu_idle+0x8/0x38) from [<c005ea80>]
(cpu_startup_entry+0x60/0x134)
[ 25.629767] [<c005ea80>] (cpu_startup_entry+0x60/0x134) from
[<804fe9a4>] (0x804fe9a4)
[ 25.629772] ---[ end trace 5484e77e2531bccc ]---
> Also, can you clarify why tegra3 code is used in tegra114 ?
>
Yes, because the flow controller that was used to control CPU power is
similar for both SoCs. The function is shared for Tegra30/114.
>
> > arch/arm/mach-tegra/cpuidle-tegra114.c | 62 +++++++++++++++++++++++++++++++++-
> > 1 file changed, 61 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/mach-tegra/cpuidle-tegra114.c b/arch/arm/mach-tegra/cpuidle-tegra114.c
> > index 1d1c602..e7d21f5 100644
> > --- a/arch/arm/mach-tegra/cpuidle-tegra114.c
> > +++ b/arch/arm/mach-tegra/cpuidle-tegra114.c
> > @@ -17,19 +17,79 @@
> > #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/suspend.h>
> > +#include <asm/smp_plat.h>
> > +
> > +#include "pm.h"
> > +#include "sleep.h"
> > +
> > +#ifdef CONFIG_PM_SLEEP
> > +static int tegra114_idle_power_down(struct cpuidle_device *dev,
> > + struct cpuidle_driver *drv,
> > + int index);
>
> Isn't possible to move the driver declaration after the
> tegra114_idle_power_down function, before the init function, so we
> prevent a forward declaration ?
>
> > +#define TEGRA114_MAX_STATES 2
> > +#else
> > +#define TEGRA114_MAX_STATES 1
> > +#endif
> >
> > static struct cpuidle_driver tegra_idle_driver = {
> > .name = "tegra_idle",
> > .owner = THIS_MODULE,
> > - .state_count = 1,
> > + .state_count = TEGRA114_MAX_STATES,
> > .states = {
> > [0] = ARM_CPUIDLE_WFI_STATE_PWR(600),
> > +#ifdef CONFIG_PM_SLEEP
> > + [1] = {
> > + .enter = tegra114_idle_power_down,
> > + .exit_latency = 500,
> > + .target_residency = 1000,
> > + .power_usage = 0,
> > + .flags = CPUIDLE_FLAG_TIME_VALID,
> > + .name = "powered-down",
> > + .desc = "CPU power gated",
> > + },
> > +#endif
> > },
> > };
> >
> > +#ifdef CONFIG_PM_SLEEP
> > +static int tegra114_idle_power_down(struct cpuidle_device *dev,
> > + struct cpuidle_driver *drv,
> > + int index)
> > +{
> > + u32 cpu = is_smp() ? cpu_logical_map(dev->cpu) : dev->cpu;
>
> IMO, it is up to the tegra_set_cpu_in_lp2 / tegra_clear_cpu_in_lp2
> functions to do the cpu map conversion instead of adding this into a
> routine working with logical cpu.
>
Good idea, I can create another patch to do that.
> > + local_fiq_disable();
> > +
> > + tegra_set_cpu_in_lp2(cpu);
> > + cpu_pm_enter();
> > +
> > + clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &dev->cpu);
> > + smp_wmb();
>
> Why do you need a memory barrier here ?
>
Yeah, Looks I don't have any static data that needs a barrier to sync
data for SMP here. Will remove them.
> > + cpu_suspend(0, tegra30_sleep_cpu_secondary_finish);
> > +
> > + clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &dev->cpu);
> > +
> > + cpu_pm_exit();
> > + tegra_clear_cpu_in_lp2(cpu);
> > +
> > + local_fiq_enable();
> > +
> > + smp_rmb();
>
> Why do you need a memory barrier here ?
>
> > + return index;
> > +}
> > +#endif
> > +
> > int __init tegra114_cpuidle_init(void)
> > {
> > +#ifdef CONFIG_PM_SLEEP
> > + tegra_tear_down_cpu = tegra30_tear_down_cpu;
> > +#endif
>
> Doesn't this code should go in the pm.c file ?
>
Yes, I had a plan to do that too. Currently It had a timing issue about
this. Because the CPU idle driver was registered by device_initcall, but
the PM init function was registered at late init. I need to sync the
init time first.
OK, will do that too.
Thanks,
Joseph
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/3] ARM: tegra114: cpuidle: add powered-down state
2013-05-31 9:12 ` Joseph Lo
@ 2013-05-31 9:27 ` Daniel Lezcano
-1 siblings, 0 replies; 22+ messages in thread
From: Daniel Lezcano @ 2013-05-31 9:27 UTC (permalink / raw)
To: Joseph Lo
Cc: Stephen Warren, linux-tegra, linux-arm-kernel, Linux PM mailing list
On 05/31/2013 11:12 AM, Joseph Lo wrote:
> Hi Daniel,
>
> Thanks for your review.
>
> On Thu, 2013-05-30 at 22:35 +0800, Daniel Lezcano wrote:
>> On 05/30/2013 01:19 PM, Joseph Lo wrote:
>>> This supports CPU core power down on each CPU when CPU idle. When CPU go
>>> into this state, it saves it's context and needs a proper configuration
>>> in flow controller to power gate the CPU when CPU runs into WFI
>>> instruction. And the CPU also needs to set the IRQ as CPU power down idle
>>> wake up event in flow controller.
>>>
>>> Signed-off-by: Joseph Lo <josephl@nvidia.com>
>>> ---
>>
>> Hi Joseph,
>>
>> a new flag has been added in the cpuidle framework to let this one to
>> handle the timer broadcast : CPUIDLE_FLAG_TIMER_STOP
>>
>> It is the commit b60e6a0eb0273132cbb60a9806abf5f47a4aee1c
>>
>> You can get rid of the clockevent notify stuff by adding this flag to
>> the state.
>>
>
> I just tested this flag on Tegra20, 30 and 114. It is only OK on
> Tegra20. It caused a warning message below on Tegra30/114 at boot time.
>
> I gave it a quick check I found it can't clean
> tick_broadcast_pending_mask sometimes if apply the flag. But I keep the
> code right now it's always OK. Not sure why?
Is it possible you didn't intialized the timer broadcast with
CLOCK_EVT_NOTIFY_BROADCAST_ON in the driver and then this one is
actually disabled for you driver ?
> [ 25.629539] ------------[ cut here ]------------
> [ 25.629559] WARNING: CPU: 2 PID: 0 at
> kernel/time/tick-broadcast.c:578 tick_broadcast_oneshot_control
> +0x1a4/0x1d0()
> [ 25.629565] Modules linked in:
> [ 25.629574] CPU: 2 PID: 0 Comm: swapper/2 Not tainted
> 3.10.0-rc3-next-20130529+ #15
> [ 25.629601] [<c00148f4>] (unwind_backtrace+0x0/0xf8) from
> [<c0011040>] (show_stack+0x10/0x14)
> [ 25.629616] [<c0011040>] (show_stack+0x10/0x14) from [<c0504248>]
> (dump_stack+0x80/0xc4)
> [ 25.629633] [<c0504248>] (dump_stack+0x80/0xc4) from [<c00231ac>]
> (warn_slowpath_common+0x64/0x88)
> [ 25.629646] [<c00231ac>] (warn_slowpath_common+0x64/0x88) from
> [<c00231ec>] (warn_slowpath_null+0x1c/0x24)
> [ 25.629657] [<c00231ec>] (warn_slowpath_null+0x1c/0x24) from
> [<c00667f8>] (tick_broadcast_oneshot_control+0x1a4/0x1d0)
> [ 25.629670] [<c00667f8>] (tick_broadcast_oneshot_control+0x1a4/0x1d0)
> from [<c0065cd0>] (tick_notify+0x240/0x40c)
> [ 25.629685] [<c0065cd0>] (tick_notify+0x240/0x40c) from [<c0044724>]
> (notifier_call_chain+0x44/0x84)
> [ 25.629699] [<c0044724>] (notifier_call_chain+0x44/0x84) from
> [<c0044828>] (raw_notifier_call_chain+0x18/0x20)
> [ 25.629712] [<c0044828>] (raw_notifier_call_chain+0x18/0x20) from
> [<c00650cc>] (clockevents_notify+0x28/0x170)
> [ 25.629726] [<c00650cc>] (clockevents_notify+0x28/0x170) from
> [<c033f1f0>] (cpuidle_idle_call+0x11c/0x168)
> [ 25.629739] [<c033f1f0>] (cpuidle_idle_call+0x11c/0x168) from
> [<c000ea94>] (arch_cpu_idle+0x8/0x38)
> [ 25.629755] [<c000ea94>] (arch_cpu_idle+0x8/0x38) from [<c005ea80>]
> (cpu_startup_entry+0x60/0x134)
> [ 25.629767] [<c005ea80>] (cpu_startup_entry+0x60/0x134) from
> [<804fe9a4>] (0x804fe9a4)
> [ 25.629772] ---[ end trace 5484e77e2531bccc ]---
>
>
>> Also, can you clarify why tegra3 code is used in tegra114 ?
>>
> Yes, because the flow controller that was used to control CPU power is
> similar for both SoCs. The function is shared for Tegra30/114.
>
>>
>>> arch/arm/mach-tegra/cpuidle-tegra114.c | 62 +++++++++++++++++++++++++++++++++-
>>> 1 file changed, 61 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm/mach-tegra/cpuidle-tegra114.c b/arch/arm/mach-tegra/cpuidle-tegra114.c
>>> index 1d1c602..e7d21f5 100644
>>> --- a/arch/arm/mach-tegra/cpuidle-tegra114.c
>>> +++ b/arch/arm/mach-tegra/cpuidle-tegra114.c
>>> @@ -17,19 +17,79 @@
>>> #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/suspend.h>
>>> +#include <asm/smp_plat.h>
>>> +
>>> +#include "pm.h"
>>> +#include "sleep.h"
>>> +
>>> +#ifdef CONFIG_PM_SLEEP
>>> +static int tegra114_idle_power_down(struct cpuidle_device *dev,
>>> + struct cpuidle_driver *drv,
>>> + int index);
>>
>> Isn't possible to move the driver declaration after the
>> tegra114_idle_power_down function, before the init function, so we
>> prevent a forward declaration ?
>>
>>> +#define TEGRA114_MAX_STATES 2
>>> +#else
>>> +#define TEGRA114_MAX_STATES 1
>>> +#endif
>>>
>>> static struct cpuidle_driver tegra_idle_driver = {
>>> .name = "tegra_idle",
>>> .owner = THIS_MODULE,
>>> - .state_count = 1,
>>> + .state_count = TEGRA114_MAX_STATES,
>>> .states = {
>>> [0] = ARM_CPUIDLE_WFI_STATE_PWR(600),
>>> +#ifdef CONFIG_PM_SLEEP
>>> + [1] = {
>>> + .enter = tegra114_idle_power_down,
>>> + .exit_latency = 500,
>>> + .target_residency = 1000,
>>> + .power_usage = 0,
>>> + .flags = CPUIDLE_FLAG_TIME_VALID,
>>> + .name = "powered-down",
>>> + .desc = "CPU power gated",
>>> + },
>>> +#endif
>>> },
>>> };
>>>
>>> +#ifdef CONFIG_PM_SLEEP
>>> +static int tegra114_idle_power_down(struct cpuidle_device *dev,
>>> + struct cpuidle_driver *drv,
>>> + int index)
>>> +{
>>> + u32 cpu = is_smp() ? cpu_logical_map(dev->cpu) : dev->cpu;
>>
>> IMO, it is up to the tegra_set_cpu_in_lp2 / tegra_clear_cpu_in_lp2
>> functions to do the cpu map conversion instead of adding this into a
>> routine working with logical cpu.
>>
> Good idea, I can create another patch to do that.
>
>>> + local_fiq_disable();
>>> +
>>> + tegra_set_cpu_in_lp2(cpu);
>>> + cpu_pm_enter();
>>> +
>>> + clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &dev->cpu);
>>> + smp_wmb();
>>
>> Why do you need a memory barrier here ?
>>
> Yeah, Looks I don't have any static data that needs a barrier to sync
> data for SMP here. Will remove them.
>
>>> + cpu_suspend(0, tegra30_sleep_cpu_secondary_finish);
>>> +
>>> + clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &dev->cpu);
>>> +
>>> + cpu_pm_exit();
>>> + tegra_clear_cpu_in_lp2(cpu);
>>> +
>>> + local_fiq_enable();
>>> +
>>> + smp_rmb();
>>
>> Why do you need a memory barrier here ?
>>
>>> + return index;
>>> +}
>>> +#endif
>>> +
>>> int __init tegra114_cpuidle_init(void)
>>> {
>>> +#ifdef CONFIG_PM_SLEEP
>>> + tegra_tear_down_cpu = tegra30_tear_down_cpu;
>>> +#endif
>>
>> Doesn't this code should go in the pm.c file ?
>>
> Yes, I had a plan to do that too. Currently It had a timing issue about
> this. Because the CPU idle driver was registered by device_initcall, but
> the PM init function was registered at late init. I need to sync the
> init time first.
> OK, will do that too.
>
> Thanks,
> Joseph
>
>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 3/3] ARM: tegra114: cpuidle: add powered-down state
@ 2013-05-31 9:27 ` Daniel Lezcano
0 siblings, 0 replies; 22+ messages in thread
From: Daniel Lezcano @ 2013-05-31 9:27 UTC (permalink / raw)
To: linux-arm-kernel
On 05/31/2013 11:12 AM, Joseph Lo wrote:
> Hi Daniel,
>
> Thanks for your review.
>
> On Thu, 2013-05-30 at 22:35 +0800, Daniel Lezcano wrote:
>> On 05/30/2013 01:19 PM, Joseph Lo wrote:
>>> This supports CPU core power down on each CPU when CPU idle. When CPU go
>>> into this state, it saves it's context and needs a proper configuration
>>> in flow controller to power gate the CPU when CPU runs into WFI
>>> instruction. And the CPU also needs to set the IRQ as CPU power down idle
>>> wake up event in flow controller.
>>>
>>> Signed-off-by: Joseph Lo <josephl@nvidia.com>
>>> ---
>>
>> Hi Joseph,
>>
>> a new flag has been added in the cpuidle framework to let this one to
>> handle the timer broadcast : CPUIDLE_FLAG_TIMER_STOP
>>
>> It is the commit b60e6a0eb0273132cbb60a9806abf5f47a4aee1c
>>
>> You can get rid of the clockevent notify stuff by adding this flag to
>> the state.
>>
>
> I just tested this flag on Tegra20, 30 and 114. It is only OK on
> Tegra20. It caused a warning message below on Tegra30/114 at boot time.
>
> I gave it a quick check I found it can't clean
> tick_broadcast_pending_mask sometimes if apply the flag. But I keep the
> code right now it's always OK. Not sure why?
Is it possible you didn't intialized the timer broadcast with
CLOCK_EVT_NOTIFY_BROADCAST_ON in the driver and then this one is
actually disabled for you driver ?
> [ 25.629539] ------------[ cut here ]------------
> [ 25.629559] WARNING: CPU: 2 PID: 0 at
> kernel/time/tick-broadcast.c:578 tick_broadcast_oneshot_control
> +0x1a4/0x1d0()
> [ 25.629565] Modules linked in:
> [ 25.629574] CPU: 2 PID: 0 Comm: swapper/2 Not tainted
> 3.10.0-rc3-next-20130529+ #15
> [ 25.629601] [<c00148f4>] (unwind_backtrace+0x0/0xf8) from
> [<c0011040>] (show_stack+0x10/0x14)
> [ 25.629616] [<c0011040>] (show_stack+0x10/0x14) from [<c0504248>]
> (dump_stack+0x80/0xc4)
> [ 25.629633] [<c0504248>] (dump_stack+0x80/0xc4) from [<c00231ac>]
> (warn_slowpath_common+0x64/0x88)
> [ 25.629646] [<c00231ac>] (warn_slowpath_common+0x64/0x88) from
> [<c00231ec>] (warn_slowpath_null+0x1c/0x24)
> [ 25.629657] [<c00231ec>] (warn_slowpath_null+0x1c/0x24) from
> [<c00667f8>] (tick_broadcast_oneshot_control+0x1a4/0x1d0)
> [ 25.629670] [<c00667f8>] (tick_broadcast_oneshot_control+0x1a4/0x1d0)
> from [<c0065cd0>] (tick_notify+0x240/0x40c)
> [ 25.629685] [<c0065cd0>] (tick_notify+0x240/0x40c) from [<c0044724>]
> (notifier_call_chain+0x44/0x84)
> [ 25.629699] [<c0044724>] (notifier_call_chain+0x44/0x84) from
> [<c0044828>] (raw_notifier_call_chain+0x18/0x20)
> [ 25.629712] [<c0044828>] (raw_notifier_call_chain+0x18/0x20) from
> [<c00650cc>] (clockevents_notify+0x28/0x170)
> [ 25.629726] [<c00650cc>] (clockevents_notify+0x28/0x170) from
> [<c033f1f0>] (cpuidle_idle_call+0x11c/0x168)
> [ 25.629739] [<c033f1f0>] (cpuidle_idle_call+0x11c/0x168) from
> [<c000ea94>] (arch_cpu_idle+0x8/0x38)
> [ 25.629755] [<c000ea94>] (arch_cpu_idle+0x8/0x38) from [<c005ea80>]
> (cpu_startup_entry+0x60/0x134)
> [ 25.629767] [<c005ea80>] (cpu_startup_entry+0x60/0x134) from
> [<804fe9a4>] (0x804fe9a4)
> [ 25.629772] ---[ end trace 5484e77e2531bccc ]---
>
>
>> Also, can you clarify why tegra3 code is used in tegra114 ?
>>
> Yes, because the flow controller that was used to control CPU power is
> similar for both SoCs. The function is shared for Tegra30/114.
>
>>
>>> arch/arm/mach-tegra/cpuidle-tegra114.c | 62 +++++++++++++++++++++++++++++++++-
>>> 1 file changed, 61 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm/mach-tegra/cpuidle-tegra114.c b/arch/arm/mach-tegra/cpuidle-tegra114.c
>>> index 1d1c602..e7d21f5 100644
>>> --- a/arch/arm/mach-tegra/cpuidle-tegra114.c
>>> +++ b/arch/arm/mach-tegra/cpuidle-tegra114.c
>>> @@ -17,19 +17,79 @@
>>> #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/suspend.h>
>>> +#include <asm/smp_plat.h>
>>> +
>>> +#include "pm.h"
>>> +#include "sleep.h"
>>> +
>>> +#ifdef CONFIG_PM_SLEEP
>>> +static int tegra114_idle_power_down(struct cpuidle_device *dev,
>>> + struct cpuidle_driver *drv,
>>> + int index);
>>
>> Isn't possible to move the driver declaration after the
>> tegra114_idle_power_down function, before the init function, so we
>> prevent a forward declaration ?
>>
>>> +#define TEGRA114_MAX_STATES 2
>>> +#else
>>> +#define TEGRA114_MAX_STATES 1
>>> +#endif
>>>
>>> static struct cpuidle_driver tegra_idle_driver = {
>>> .name = "tegra_idle",
>>> .owner = THIS_MODULE,
>>> - .state_count = 1,
>>> + .state_count = TEGRA114_MAX_STATES,
>>> .states = {
>>> [0] = ARM_CPUIDLE_WFI_STATE_PWR(600),
>>> +#ifdef CONFIG_PM_SLEEP
>>> + [1] = {
>>> + .enter = tegra114_idle_power_down,
>>> + .exit_latency = 500,
>>> + .target_residency = 1000,
>>> + .power_usage = 0,
>>> + .flags = CPUIDLE_FLAG_TIME_VALID,
>>> + .name = "powered-down",
>>> + .desc = "CPU power gated",
>>> + },
>>> +#endif
>>> },
>>> };
>>>
>>> +#ifdef CONFIG_PM_SLEEP
>>> +static int tegra114_idle_power_down(struct cpuidle_device *dev,
>>> + struct cpuidle_driver *drv,
>>> + int index)
>>> +{
>>> + u32 cpu = is_smp() ? cpu_logical_map(dev->cpu) : dev->cpu;
>>
>> IMO, it is up to the tegra_set_cpu_in_lp2 / tegra_clear_cpu_in_lp2
>> functions to do the cpu map conversion instead of adding this into a
>> routine working with logical cpu.
>>
> Good idea, I can create another patch to do that.
>
>>> + local_fiq_disable();
>>> +
>>> + tegra_set_cpu_in_lp2(cpu);
>>> + cpu_pm_enter();
>>> +
>>> + clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &dev->cpu);
>>> + smp_wmb();
>>
>> Why do you need a memory barrier here ?
>>
> Yeah, Looks I don't have any static data that needs a barrier to sync
> data for SMP here. Will remove them.
>
>>> + cpu_suspend(0, tegra30_sleep_cpu_secondary_finish);
>>> +
>>> + clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &dev->cpu);
>>> +
>>> + cpu_pm_exit();
>>> + tegra_clear_cpu_in_lp2(cpu);
>>> +
>>> + local_fiq_enable();
>>> +
>>> + smp_rmb();
>>
>> Why do you need a memory barrier here ?
>>
>>> + return index;
>>> +}
>>> +#endif
>>> +
>>> int __init tegra114_cpuidle_init(void)
>>> {
>>> +#ifdef CONFIG_PM_SLEEP
>>> + tegra_tear_down_cpu = tegra30_tear_down_cpu;
>>> +#endif
>>
>> Doesn't this code should go in the pm.c file ?
>>
> Yes, I had a plan to do that too. Currently It had a timing issue about
> this. Because the CPU idle driver was registered by device_initcall, but
> the PM init function was registered at late init. I need to sync the
> init time first.
> OK, will do that too.
>
> Thanks,
> Joseph
>
>
--
<http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/3] ARM: tegra114: cpuidle: add powered-down state
2013-05-31 9:27 ` Daniel Lezcano
@ 2013-05-31 10:47 ` Joseph Lo
-1 siblings, 0 replies; 22+ messages in thread
From: Joseph Lo @ 2013-05-31 10:47 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Stephen Warren, linux-tegra-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Linux PM mailing list
On Fri, 2013-05-31 at 17:27 +0800, Daniel Lezcano wrote:
> On 05/31/2013 11:12 AM, Joseph Lo wrote:
> > Hi Daniel,
> >
> > Thanks for your review.
> >
> > On Thu, 2013-05-30 at 22:35 +0800, Daniel Lezcano wrote:
> >> On 05/30/2013 01:19 PM, Joseph Lo wrote:
> >>> This supports CPU core power down on each CPU when CPU idle. When CPU go
> >>> into this state, it saves it's context and needs a proper configuration
> >>> in flow controller to power gate the CPU when CPU runs into WFI
> >>> instruction. And the CPU also needs to set the IRQ as CPU power down idle
> >>> wake up event in flow controller.
> >>>
> >>> Signed-off-by: Joseph Lo <josephl-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> >>> ---
> >>
> >> Hi Joseph,
> >>
> >> a new flag has been added in the cpuidle framework to let this one to
> >> handle the timer broadcast : CPUIDLE_FLAG_TIMER_STOP
> >>
> >> It is the commit b60e6a0eb0273132cbb60a9806abf5f47a4aee1c
> >>
> >> You can get rid of the clockevent notify stuff by adding this flag to
> >> the state.
> >>
> >
> > I just tested this flag on Tegra20, 30 and 114. It is only OK on
> > Tegra20. It caused a warning message below on Tegra30/114 at boot time.
> >
> > I gave it a quick check I found it can't clean
> > tick_broadcast_pending_mask sometimes if apply the flag. But I keep the
> > code right now it's always OK. Not sure why?
>
> Is it possible you didn't intialized the timer broadcast with
> CLOCK_EVT_NOTIFY_BROADCAST_ON in the driver and then this one is
> actually disabled for you driver ?
>
I found if I don't apply the CPUIDLE_FLAG_TIMER_STOP flag, the function
"cpuidle_setup_broadcast_timer" (drivers/cpuidle/driver.c) would not be
called. Then it's OK.
If I apply this flag (with no CONFIG_CPU_IDLE_MULTIPLE_DRIVERS), the
cpuidle_register_driver only register for CPU0. Then the
"cpuidle_setup_broadcast_timer" only turn on for CPU0. I think this
might be the root cause. Not sure this also can reproduce on the other
SoCs?
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 3/3] ARM: tegra114: cpuidle: add powered-down state
@ 2013-05-31 10:47 ` Joseph Lo
0 siblings, 0 replies; 22+ messages in thread
From: Joseph Lo @ 2013-05-31 10:47 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, 2013-05-31 at 17:27 +0800, Daniel Lezcano wrote:
> On 05/31/2013 11:12 AM, Joseph Lo wrote:
> > Hi Daniel,
> >
> > Thanks for your review.
> >
> > On Thu, 2013-05-30 at 22:35 +0800, Daniel Lezcano wrote:
> >> On 05/30/2013 01:19 PM, Joseph Lo wrote:
> >>> This supports CPU core power down on each CPU when CPU idle. When CPU go
> >>> into this state, it saves it's context and needs a proper configuration
> >>> in flow controller to power gate the CPU when CPU runs into WFI
> >>> instruction. And the CPU also needs to set the IRQ as CPU power down idle
> >>> wake up event in flow controller.
> >>>
> >>> Signed-off-by: Joseph Lo <josephl@nvidia.com>
> >>> ---
> >>
> >> Hi Joseph,
> >>
> >> a new flag has been added in the cpuidle framework to let this one to
> >> handle the timer broadcast : CPUIDLE_FLAG_TIMER_STOP
> >>
> >> It is the commit b60e6a0eb0273132cbb60a9806abf5f47a4aee1c
> >>
> >> You can get rid of the clockevent notify stuff by adding this flag to
> >> the state.
> >>
> >
> > I just tested this flag on Tegra20, 30 and 114. It is only OK on
> > Tegra20. It caused a warning message below on Tegra30/114 at boot time.
> >
> > I gave it a quick check I found it can't clean
> > tick_broadcast_pending_mask sometimes if apply the flag. But I keep the
> > code right now it's always OK. Not sure why?
>
> Is it possible you didn't intialized the timer broadcast with
> CLOCK_EVT_NOTIFY_BROADCAST_ON in the driver and then this one is
> actually disabled for you driver ?
>
I found if I don't apply the CPUIDLE_FLAG_TIMER_STOP flag, the function
"cpuidle_setup_broadcast_timer" (drivers/cpuidle/driver.c) would not be
called. Then it's OK.
If I apply this flag (with no CONFIG_CPU_IDLE_MULTIPLE_DRIVERS), the
cpuidle_register_driver only register for CPU0. Then the
"cpuidle_setup_broadcast_timer" only turn on for CPU0. I think this
might be the root cause. Not sure this also can reproduce on the other
SoCs?
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/3] ARM: tegra114: cpuidle: add powered-down state
2013-05-31 10:47 ` Joseph Lo
@ 2013-05-31 11:31 ` Joseph Lo
-1 siblings, 0 replies; 22+ messages in thread
From: Joseph Lo @ 2013-05-31 11:31 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Stephen Warren, linux-tegra-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Linux PM mailing list
On Fri, 2013-05-31 at 18:47 +0800, Joseph Lo wrote:
> On Fri, 2013-05-31 at 17:27 +0800, Daniel Lezcano wrote:
> > On 05/31/2013 11:12 AM, Joseph Lo wrote:
> > > Hi Daniel,
> > >
> > > Thanks for your review.
> > >
> > > On Thu, 2013-05-30 at 22:35 +0800, Daniel Lezcano wrote:
> > >> On 05/30/2013 01:19 PM, Joseph Lo wrote:
> > >>> This supports CPU core power down on each CPU when CPU idle. When CPU go
> > >>> into this state, it saves it's context and needs a proper configuration
> > >>> in flow controller to power gate the CPU when CPU runs into WFI
> > >>> instruction. And the CPU also needs to set the IRQ as CPU power down idle
> > >>> wake up event in flow controller.
> > >>>
> > >>> Signed-off-by: Joseph Lo <josephl-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> > >>> ---
> > >>
> > >> Hi Joseph,
> > >>
> > >> a new flag has been added in the cpuidle framework to let this one to
> > >> handle the timer broadcast : CPUIDLE_FLAG_TIMER_STOP
> > >>
> > >> It is the commit b60e6a0eb0273132cbb60a9806abf5f47a4aee1c
> > >>
> > >> You can get rid of the clockevent notify stuff by adding this flag to
> > >> the state.
> > >>
> > >
> > > I just tested this flag on Tegra20, 30 and 114. It is only OK on
> > > Tegra20. It caused a warning message below on Tegra30/114 at boot time.
> > >
> > > I gave it a quick check I found it can't clean
> > > tick_broadcast_pending_mask sometimes if apply the flag. But I keep the
> > > code right now it's always OK. Not sure why?
> >
> > Is it possible you didn't intialized the timer broadcast with
> > CLOCK_EVT_NOTIFY_BROADCAST_ON in the driver and then this one is
> > actually disabled for you driver ?
> >
> I found if I don't apply the CPUIDLE_FLAG_TIMER_STOP flag, the function
> "cpuidle_setup_broadcast_timer" (drivers/cpuidle/driver.c) would not be
> called. Then it's OK.
>
> If I apply this flag (with no CONFIG_CPU_IDLE_MULTIPLE_DRIVERS), the
> cpuidle_register_driver only register for CPU0. Then the
> "cpuidle_setup_broadcast_timer" only turn on for CPU0. I think this
> might be the root cause. Not sure this also can reproduce on the other
> SoCs?
>
Looks it's not related to CLOCK_EVT_NOTIFY_BROADCAST_ON. I still saw the
warning message even enable CONFIG_CPU_IDLE_MULTIPLE_DRIVERS. But if I
move the clockevent_notify of CLOCK_EVT_NOTIFY_BROADCAST_EXIT from idle
framework back to driver, then everything is OK.
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 3/3] ARM: tegra114: cpuidle: add powered-down state
@ 2013-05-31 11:31 ` Joseph Lo
0 siblings, 0 replies; 22+ messages in thread
From: Joseph Lo @ 2013-05-31 11:31 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, 2013-05-31 at 18:47 +0800, Joseph Lo wrote:
> On Fri, 2013-05-31 at 17:27 +0800, Daniel Lezcano wrote:
> > On 05/31/2013 11:12 AM, Joseph Lo wrote:
> > > Hi Daniel,
> > >
> > > Thanks for your review.
> > >
> > > On Thu, 2013-05-30 at 22:35 +0800, Daniel Lezcano wrote:
> > >> On 05/30/2013 01:19 PM, Joseph Lo wrote:
> > >>> This supports CPU core power down on each CPU when CPU idle. When CPU go
> > >>> into this state, it saves it's context and needs a proper configuration
> > >>> in flow controller to power gate the CPU when CPU runs into WFI
> > >>> instruction. And the CPU also needs to set the IRQ as CPU power down idle
> > >>> wake up event in flow controller.
> > >>>
> > >>> Signed-off-by: Joseph Lo <josephl@nvidia.com>
> > >>> ---
> > >>
> > >> Hi Joseph,
> > >>
> > >> a new flag has been added in the cpuidle framework to let this one to
> > >> handle the timer broadcast : CPUIDLE_FLAG_TIMER_STOP
> > >>
> > >> It is the commit b60e6a0eb0273132cbb60a9806abf5f47a4aee1c
> > >>
> > >> You can get rid of the clockevent notify stuff by adding this flag to
> > >> the state.
> > >>
> > >
> > > I just tested this flag on Tegra20, 30 and 114. It is only OK on
> > > Tegra20. It caused a warning message below on Tegra30/114 at boot time.
> > >
> > > I gave it a quick check I found it can't clean
> > > tick_broadcast_pending_mask sometimes if apply the flag. But I keep the
> > > code right now it's always OK. Not sure why?
> >
> > Is it possible you didn't intialized the timer broadcast with
> > CLOCK_EVT_NOTIFY_BROADCAST_ON in the driver and then this one is
> > actually disabled for you driver ?
> >
> I found if I don't apply the CPUIDLE_FLAG_TIMER_STOP flag, the function
> "cpuidle_setup_broadcast_timer" (drivers/cpuidle/driver.c) would not be
> called. Then it's OK.
>
> If I apply this flag (with no CONFIG_CPU_IDLE_MULTIPLE_DRIVERS), the
> cpuidle_register_driver only register for CPU0. Then the
> "cpuidle_setup_broadcast_timer" only turn on for CPU0. I think this
> might be the root cause. Not sure this also can reproduce on the other
> SoCs?
>
Looks it's not related to CLOCK_EVT_NOTIFY_BROADCAST_ON. I still saw the
warning message even enable CONFIG_CPU_IDLE_MULTIPLE_DRIVERS. But if I
move the clockevent_notify of CLOCK_EVT_NOTIFY_BROADCAST_EXIT from idle
framework back to driver, then everything is OK.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/3] ARM: tegra114: cpuidle: add powered-down state
2013-05-31 11:31 ` Joseph Lo
@ 2013-05-31 11:44 ` Joseph Lo
-1 siblings, 0 replies; 22+ messages in thread
From: Joseph Lo @ 2013-05-31 11:44 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Stephen Warren, linux-tegra, linux-arm-kernel, Linux PM mailing list
On Fri, 2013-05-31 at 19:31 +0800, Joseph Lo wrote:
> On Fri, 2013-05-31 at 18:47 +0800, Joseph Lo wrote:
> > On Fri, 2013-05-31 at 17:27 +0800, Daniel Lezcano wrote:
> > > On 05/31/2013 11:12 AM, Joseph Lo wrote:
> > > > Hi Daniel,
> > > >
> > > > Thanks for your review.
> > > >
> > > > On Thu, 2013-05-30 at 22:35 +0800, Daniel Lezcano wrote:
> > > >> On 05/30/2013 01:19 PM, Joseph Lo wrote:
> > > >>> This supports CPU core power down on each CPU when CPU idle. When CPU go
> > > >>> into this state, it saves it's context and needs a proper configuration
> > > >>> in flow controller to power gate the CPU when CPU runs into WFI
> > > >>> instruction. And the CPU also needs to set the IRQ as CPU power down idle
> > > >>> wake up event in flow controller.
> > > >>>
> > > >>> Signed-off-by: Joseph Lo <josephl@nvidia.com>
> > > >>> ---
> > > >>
> > > >> Hi Joseph,
> > > >>
> > > >> a new flag has been added in the cpuidle framework to let this one to
> > > >> handle the timer broadcast : CPUIDLE_FLAG_TIMER_STOP
> > > >>
> > > >> It is the commit b60e6a0eb0273132cbb60a9806abf5f47a4aee1c
> > > >>
> > > >> You can get rid of the clockevent notify stuff by adding this flag to
> > > >> the state.
> > > >>
> > > >
> > > > I just tested this flag on Tegra20, 30 and 114. It is only OK on
> > > > Tegra20. It caused a warning message below on Tegra30/114 at boot time.
> > > >
> > > > I gave it a quick check I found it can't clean
> > > > tick_broadcast_pending_mask sometimes if apply the flag. But I keep the
> > > > code right now it's always OK. Not sure why?
> > >
> > > Is it possible you didn't intialized the timer broadcast with
> > > CLOCK_EVT_NOTIFY_BROADCAST_ON in the driver and then this one is
> > > actually disabled for you driver ?
> > >
> > I found if I don't apply the CPUIDLE_FLAG_TIMER_STOP flag, the function
> > "cpuidle_setup_broadcast_timer" (drivers/cpuidle/driver.c) would not be
> > called. Then it's OK.
> >
> > If I apply this flag (with no CONFIG_CPU_IDLE_MULTIPLE_DRIVERS), the
> > cpuidle_register_driver only register for CPU0. Then the
> > "cpuidle_setup_broadcast_timer" only turn on for CPU0. I think this
> > might be the root cause. Not sure this also can reproduce on the other
> > SoCs?
> >
> Looks it's not related to CLOCK_EVT_NOTIFY_BROADCAST_ON. I still saw the
> warning message even enable CONFIG_CPU_IDLE_MULTIPLE_DRIVERS. But if I
> move the clockevent_notify of CLOCK_EVT_NOTIFY_BROADCAST_EXIT from idle
> framework back to driver, then everything is OK.
>
Once I move the clockevent_notify of CLOCK_EVT_NOTIFY_BROADCAST_EXIT
right after "local_irq_enable" in the
"cpuidle_enter_state" (drivers/cpuidle/cpuidle.c). Then the warning
message shows up.
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 3/3] ARM: tegra114: cpuidle: add powered-down state
@ 2013-05-31 11:44 ` Joseph Lo
0 siblings, 0 replies; 22+ messages in thread
From: Joseph Lo @ 2013-05-31 11:44 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, 2013-05-31 at 19:31 +0800, Joseph Lo wrote:
> On Fri, 2013-05-31 at 18:47 +0800, Joseph Lo wrote:
> > On Fri, 2013-05-31 at 17:27 +0800, Daniel Lezcano wrote:
> > > On 05/31/2013 11:12 AM, Joseph Lo wrote:
> > > > Hi Daniel,
> > > >
> > > > Thanks for your review.
> > > >
> > > > On Thu, 2013-05-30 at 22:35 +0800, Daniel Lezcano wrote:
> > > >> On 05/30/2013 01:19 PM, Joseph Lo wrote:
> > > >>> This supports CPU core power down on each CPU when CPU idle. When CPU go
> > > >>> into this state, it saves it's context and needs a proper configuration
> > > >>> in flow controller to power gate the CPU when CPU runs into WFI
> > > >>> instruction. And the CPU also needs to set the IRQ as CPU power down idle
> > > >>> wake up event in flow controller.
> > > >>>
> > > >>> Signed-off-by: Joseph Lo <josephl@nvidia.com>
> > > >>> ---
> > > >>
> > > >> Hi Joseph,
> > > >>
> > > >> a new flag has been added in the cpuidle framework to let this one to
> > > >> handle the timer broadcast : CPUIDLE_FLAG_TIMER_STOP
> > > >>
> > > >> It is the commit b60e6a0eb0273132cbb60a9806abf5f47a4aee1c
> > > >>
> > > >> You can get rid of the clockevent notify stuff by adding this flag to
> > > >> the state.
> > > >>
> > > >
> > > > I just tested this flag on Tegra20, 30 and 114. It is only OK on
> > > > Tegra20. It caused a warning message below on Tegra30/114 at boot time.
> > > >
> > > > I gave it a quick check I found it can't clean
> > > > tick_broadcast_pending_mask sometimes if apply the flag. But I keep the
> > > > code right now it's always OK. Not sure why?
> > >
> > > Is it possible you didn't intialized the timer broadcast with
> > > CLOCK_EVT_NOTIFY_BROADCAST_ON in the driver and then this one is
> > > actually disabled for you driver ?
> > >
> > I found if I don't apply the CPUIDLE_FLAG_TIMER_STOP flag, the function
> > "cpuidle_setup_broadcast_timer" (drivers/cpuidle/driver.c) would not be
> > called. Then it's OK.
> >
> > If I apply this flag (with no CONFIG_CPU_IDLE_MULTIPLE_DRIVERS), the
> > cpuidle_register_driver only register for CPU0. Then the
> > "cpuidle_setup_broadcast_timer" only turn on for CPU0. I think this
> > might be the root cause. Not sure this also can reproduce on the other
> > SoCs?
> >
> Looks it's not related to CLOCK_EVT_NOTIFY_BROADCAST_ON. I still saw the
> warning message even enable CONFIG_CPU_IDLE_MULTIPLE_DRIVERS. But if I
> move the clockevent_notify of CLOCK_EVT_NOTIFY_BROADCAST_EXIT from idle
> framework back to driver, then everything is OK.
>
Once I move the clockevent_notify of CLOCK_EVT_NOTIFY_BROADCAST_EXIT
right after "local_irq_enable" in the
"cpuidle_enter_state" (drivers/cpuidle/cpuidle.c). Then the warning
message shows up.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/3] ARM: tegra114: cpuidle: add powered-down state
2013-05-31 11:44 ` Joseph Lo
@ 2013-05-31 13:18 ` Daniel Lezcano
-1 siblings, 0 replies; 22+ messages in thread
From: Daniel Lezcano @ 2013-05-31 13:18 UTC (permalink / raw)
To: Joseph Lo
Cc: Stephen Warren, linux-tegra-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Linux PM mailing list
On 05/31/2013 01:44 PM, Joseph Lo wrote:
> On Fri, 2013-05-31 at 19:31 +0800, Joseph Lo wrote:
>> On Fri, 2013-05-31 at 18:47 +0800, Joseph Lo wrote:
>>> On Fri, 2013-05-31 at 17:27 +0800, Daniel Lezcano wrote:
>>>> On 05/31/2013 11:12 AM, Joseph Lo wrote:
>>>>> Hi Daniel,
>>>>>
>>>>> Thanks for your review.
>>>>>
>>>>> On Thu, 2013-05-30 at 22:35 +0800, Daniel Lezcano wrote:
>>>>>> On 05/30/2013 01:19 PM, Joseph Lo wrote:
>>>>>>> This supports CPU core power down on each CPU when CPU idle. When CPU go
>>>>>>> into this state, it saves it's context and needs a proper configuration
>>>>>>> in flow controller to power gate the CPU when CPU runs into WFI
>>>>>>> instruction. And the CPU also needs to set the IRQ as CPU power down idle
>>>>>>> wake up event in flow controller.
>>>>>>>
>>>>>>> Signed-off-by: Joseph Lo <josephl-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>>>>>>> ---
>>>>>>
>>>>>> Hi Joseph,
>>>>>>
>>>>>> a new flag has been added in the cpuidle framework to let this one to
>>>>>> handle the timer broadcast : CPUIDLE_FLAG_TIMER_STOP
>>>>>>
>>>>>> It is the commit b60e6a0eb0273132cbb60a9806abf5f47a4aee1c
>>>>>>
>>>>>> You can get rid of the clockevent notify stuff by adding this flag to
>>>>>> the state.
>>>>>>
>>>>>
>>>>> I just tested this flag on Tegra20, 30 and 114. It is only OK on
>>>>> Tegra20. It caused a warning message below on Tegra30/114 at boot time.
>>>>>
>>>>> I gave it a quick check I found it can't clean
>>>>> tick_broadcast_pending_mask sometimes if apply the flag. But I keep the
>>>>> code right now it's always OK. Not sure why?
>>>>
>>>> Is it possible you didn't intialized the timer broadcast with
>>>> CLOCK_EVT_NOTIFY_BROADCAST_ON in the driver and then this one is
>>>> actually disabled for you driver ?
>>>>
>>> I found if I don't apply the CPUIDLE_FLAG_TIMER_STOP flag, the function
>>> "cpuidle_setup_broadcast_timer" (drivers/cpuidle/driver.c) would not be
>>> called. Then it's OK.
>>>
>>> If I apply this flag (with no CONFIG_CPU_IDLE_MULTIPLE_DRIVERS), the
>>> cpuidle_register_driver only register for CPU0. Then the
>>> "cpuidle_setup_broadcast_timer" only turn on for CPU0. I think this
>>> might be the root cause. Not sure this also can reproduce on the other
>>> SoCs?
>>>
>> Looks it's not related to CLOCK_EVT_NOTIFY_BROADCAST_ON. I still saw the
>> warning message even enable CONFIG_CPU_IDLE_MULTIPLE_DRIVERS. But if I
>> move the clockevent_notify of CLOCK_EVT_NOTIFY_BROADCAST_EXIT from idle
>> framework back to driver, then everything is OK.
>>
> Once I move the clockevent_notify of CLOCK_EVT_NOTIFY_BROADCAST_EXIT
> right after "local_irq_enable" in the
> "cpuidle_enter_state" (drivers/cpuidle/cpuidle.c). Then the warning
> message shows up.
Is it possible you pastebin you kernel config file ? I would like to
check a couple of things before investigating the code.
Thanks
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 3/3] ARM: tegra114: cpuidle: add powered-down state
@ 2013-05-31 13:18 ` Daniel Lezcano
0 siblings, 0 replies; 22+ messages in thread
From: Daniel Lezcano @ 2013-05-31 13:18 UTC (permalink / raw)
To: linux-arm-kernel
On 05/31/2013 01:44 PM, Joseph Lo wrote:
> On Fri, 2013-05-31 at 19:31 +0800, Joseph Lo wrote:
>> On Fri, 2013-05-31 at 18:47 +0800, Joseph Lo wrote:
>>> On Fri, 2013-05-31 at 17:27 +0800, Daniel Lezcano wrote:
>>>> On 05/31/2013 11:12 AM, Joseph Lo wrote:
>>>>> Hi Daniel,
>>>>>
>>>>> Thanks for your review.
>>>>>
>>>>> On Thu, 2013-05-30 at 22:35 +0800, Daniel Lezcano wrote:
>>>>>> On 05/30/2013 01:19 PM, Joseph Lo wrote:
>>>>>>> This supports CPU core power down on each CPU when CPU idle. When CPU go
>>>>>>> into this state, it saves it's context and needs a proper configuration
>>>>>>> in flow controller to power gate the CPU when CPU runs into WFI
>>>>>>> instruction. And the CPU also needs to set the IRQ as CPU power down idle
>>>>>>> wake up event in flow controller.
>>>>>>>
>>>>>>> Signed-off-by: Joseph Lo <josephl@nvidia.com>
>>>>>>> ---
>>>>>>
>>>>>> Hi Joseph,
>>>>>>
>>>>>> a new flag has been added in the cpuidle framework to let this one to
>>>>>> handle the timer broadcast : CPUIDLE_FLAG_TIMER_STOP
>>>>>>
>>>>>> It is the commit b60e6a0eb0273132cbb60a9806abf5f47a4aee1c
>>>>>>
>>>>>> You can get rid of the clockevent notify stuff by adding this flag to
>>>>>> the state.
>>>>>>
>>>>>
>>>>> I just tested this flag on Tegra20, 30 and 114. It is only OK on
>>>>> Tegra20. It caused a warning message below on Tegra30/114 at boot time.
>>>>>
>>>>> I gave it a quick check I found it can't clean
>>>>> tick_broadcast_pending_mask sometimes if apply the flag. But I keep the
>>>>> code right now it's always OK. Not sure why?
>>>>
>>>> Is it possible you didn't intialized the timer broadcast with
>>>> CLOCK_EVT_NOTIFY_BROADCAST_ON in the driver and then this one is
>>>> actually disabled for you driver ?
>>>>
>>> I found if I don't apply the CPUIDLE_FLAG_TIMER_STOP flag, the function
>>> "cpuidle_setup_broadcast_timer" (drivers/cpuidle/driver.c) would not be
>>> called. Then it's OK.
>>>
>>> If I apply this flag (with no CONFIG_CPU_IDLE_MULTIPLE_DRIVERS), the
>>> cpuidle_register_driver only register for CPU0. Then the
>>> "cpuidle_setup_broadcast_timer" only turn on for CPU0. I think this
>>> might be the root cause. Not sure this also can reproduce on the other
>>> SoCs?
>>>
>> Looks it's not related to CLOCK_EVT_NOTIFY_BROADCAST_ON. I still saw the
>> warning message even enable CONFIG_CPU_IDLE_MULTIPLE_DRIVERS. But if I
>> move the clockevent_notify of CLOCK_EVT_NOTIFY_BROADCAST_EXIT from idle
>> framework back to driver, then everything is OK.
>>
> Once I move the clockevent_notify of CLOCK_EVT_NOTIFY_BROADCAST_EXIT
> right after "local_irq_enable" in the
> "cpuidle_enter_state" (drivers/cpuidle/cpuidle.c). Then the warning
> message shows up.
Is it possible you pastebin you kernel config file ? I would like to
check a couple of things before investigating the code.
Thanks
--
<http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2013-05-31 13:18 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-30 11:19 [PATCH 0/3] ARM: tegra114: cpuidle: add power down state Joseph Lo
2013-05-30 11:19 ` Joseph Lo
[not found] ` <1369912782-30663-1-git-send-email-josephl-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2013-05-30 11:19 ` [PATCH 1/3] ARM: tegra114: Reprogram GIC CPU interface to bypass IRQ on CPU PM entry Joseph Lo
2013-05-30 11:19 ` Joseph Lo
2013-05-30 11:19 ` [PATCH 2/3] ARM: tegra114: add low level support for CPU idle powered-down mode Joseph Lo
2013-05-30 11:19 ` Joseph Lo
2013-05-30 11:19 ` [PATCH 3/3] ARM: tegra114: cpuidle: add powered-down state Joseph Lo
2013-05-30 11:19 ` Joseph Lo
2013-05-30 14:35 ` Daniel Lezcano
2013-05-30 14:35 ` Daniel Lezcano
[not found] ` <51A763BD.6070001-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2013-05-31 9:12 ` Joseph Lo
2013-05-31 9:12 ` Joseph Lo
2013-05-31 9:27 ` Daniel Lezcano
2013-05-31 9:27 ` Daniel Lezcano
[not found] ` <51A86D11.8060305-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2013-05-31 10:47 ` Joseph Lo
2013-05-31 10:47 ` Joseph Lo
[not found] ` <1369997278.7804.73.camel-yx3yKKdKkHfc7b1ADBJPm0n48jw8i0AO@public.gmane.org>
2013-05-31 11:31 ` Joseph Lo
2013-05-31 11:31 ` Joseph Lo
2013-05-31 11:44 ` Joseph Lo
2013-05-31 11:44 ` Joseph Lo
[not found] ` <1370000680.7804.83.camel-yx3yKKdKkHfc7b1ADBJPm0n48jw8i0AO@public.gmane.org>
2013-05-31 13:18 ` Daniel Lezcano
2013-05-31 13:18 ` Daniel Lezcano
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.