Thanks for this Mark,
CCing Lorenzo as this doesn't seem to have come through on the linaro-devs list.
Cheers,
Steve
On 15 Jan 2013, at 13:35, Mark Hambleton mark.hambleton@broadcom.com wrote:
Take the TC2 version of CPUIdle and remove TC2 specific line. Move the file to drivers/cpuidle. Introduce the concept of "arm,generic" compatible drivers to avoid the need for a dummy dts node.
Build the new CPUIdle based on CONFIG_BIG_LITTLE.
Signed-off-by: mark mahamble@broadcom.com
Hi Nicolas / Dave,
After plugging in our ASIC support to the new big.LITTLE frameworks I realised that the TC2 CPUIdle driver was fundamentally generic, this patch moves it from mach-vexpress into drivers/cpuidle and then checks for the presence of a compatible node in the devicetree instead of checking for an SPC.
I haven't added a new config define as I figured it was specific to the big.LITTLE framework, so just used that instead.
I have tested the change on TC2 and am looking at extending the DCSCB code to make it work on fastmodels.
Any comments?
Regards,
Mark
arch/arm/boot/dts/vexpress-v2p-ca15-tc2.dts | 2 +- arch/arm/mach-vexpress/Kconfig | 12 -- arch/arm/mach-vexpress/Makefile | 1 - arch/arm/mach-vexpress/cpuidle-tc2.c | 184 --------------------------- drivers/cpuidle/Makefile | 2 +- drivers/cpuidle/arm_big_little.c | 183 ++++++++++++++++++++++++++ 6 files changed, 185 insertions(+), 199 deletions(-) delete mode 100644 arch/arm/mach-vexpress/cpuidle-tc2.c create mode 100755 drivers/cpuidle/arm_big_little.c
diff --git a/arch/arm/boot/dts/vexpress-v2p-ca15-tc2.dts b/arch/arm/boot/dts/vexpress-v2p-ca15-tc2.dts index 3b7cd86..b261bde 100644 --- a/arch/arm/boot/dts/vexpress-v2p-ca15-tc2.dts +++ b/arch/arm/boot/dts/vexpress-v2p-ca15-tc2.dts @@ -15,7 +15,7 @@ model = "V2P-CA15_CA7"; arm,hbi = <0x249>; arm,vexpress,site = <0xf>;
compatible = "arm,vexpress,v2p-ca15,tc2", "arm,vexpress,v2p-ca15", "arm,vexpress";
compatible = "arm,vexpress,v2p-ca15,tc2", "arm,vexpress,v2p-ca15", "arm,vexpress", "arm,generic"; interrupt-parent = <&gic>; #address-cells = <2>; #size-cells = <2>;diff --git a/arch/arm/mach-vexpress/Kconfig b/arch/arm/mach-vexpress/Kconfig index f302fa9..3fd44c4 100644 --- a/arch/arm/mach-vexpress/Kconfig +++ b/arch/arm/mach-vexpress/Kconfig @@ -61,18 +61,6 @@ config ARCH_VEXPRESS_TC2 help Support for CPU and cluster power management on TC2.
-config VEXPRESS_TC2_CPUIDLE
bool "cpuidle support for TC2 test-chip (EXPERIMENTAL)"depends on EXPERIMENTALdepends on CPU_IDLE && PM && ARCH_VEXPRESS_TC2select ARM_CPU_SUSPENDselect ARM_SPChelpProvides code that enables CPU idle power management on theTC2 testchip. It enables the CPU idle driver so that the kernelcan enter cluster power down states provided by the powercontroller.config ARCH_VEXPRESS_DCSCB bool "Dual Cluster System Control Block (DCSCB) support" depends on BIG_LITTLE diff --git a/arch/arm/mach-vexpress/Makefile b/arch/arm/mach-vexpress/Makefile index 05c6fed..b10390e 100644 --- a/arch/arm/mach-vexpress/Makefile +++ b/arch/arm/mach-vexpress/Makefile @@ -10,4 +10,3 @@ obj-$(CONFIG_ARCH_VEXPRESS_DCSCB) += dcscb.o dcscb_setup.o obj-$(CONFIG_ARCH_VEXPRESS_TC2) += tc2_pm.o tc2_pm_setup.o obj-$(CONFIG_SMP) += platsmp.o obj-$(CONFIG_HOTPLUG_CPU) += hotplug.o -obj-$(CONFIG_VEXPRESS_TC2_CPUIDLE) += cpuidle-tc2.o diff --git a/arch/arm/mach-vexpress/cpuidle-tc2.c b/arch/arm/mach-vexpress/cpuidle-tc2.c deleted file mode 100644 index 1f1efc4..0000000 --- a/arch/arm/mach-vexpress/cpuidle-tc2.c +++ /dev/null @@ -1,184 +0,0 @@ -/*
- TC2 CPU idle driver.
- Copyright (C) 2012 ARM Ltd.
- Author: Lorenzo Pieralisi lorenzo.pieralisi@arm.com
- This program is free software; you can redistribute it and/or modify
- it under the terms of the GNU General Public License version 2 as
- published by the Free Software Foundation.
- */
-#include <linux/arm-cci.h> -#include <linux/bitmap.h> -#include <linux/cpuidle.h> -#include <linux/cpu_pm.h> -#include <linux/clockchips.h> -#include <linux/debugfs.h> -#include <linux/hrtimer.h> -#include <linux/kernel.h> -#include <linux/module.h> -#include <linux/tick.h> -#include <linux/vexpress.h> -#include <asm/bL_entry.h> -#include <asm/cpuidle.h> -#include <asm/cputype.h> -#include <asm/idmap.h> -#include <asm/proc-fns.h> -#include <asm/suspend.h>
-#include <mach/motherboard.h>
-static int tc2_cpuidle_simple_enter(struct cpuidle_device *dev,
struct cpuidle_driver *drv, int index)-{
ktime_t time_start, time_end;s64 diff;time_start = ktime_get();cpu_do_idle();time_end = ktime_get();local_irq_enable();diff = ktime_to_us(ktime_sub(time_end, time_start));if (diff > INT_MAX)diff = INT_MAX;dev->last_residency = (int) diff;return index;-}
-static int tc2_enter_powerdown(struct cpuidle_device *dev,
struct cpuidle_driver *drv, int idx);-static struct cpuidle_state tc2_cpuidle_set[] __initdata = {
[0] = {.enter = tc2_cpuidle_simple_enter,.exit_latency = 1,.target_residency = 1,.power_usage = UINT_MAX,.flags = CPUIDLE_FLAG_TIME_VALID,.name = "WFI",.desc = "ARM WFI",},[1] = {.enter = tc2_enter_powerdown,.exit_latency = 300,.target_residency = 1000,.flags = CPUIDLE_FLAG_TIME_VALID,.name = "C1",.desc = "ARM power down",},-};
-struct cpuidle_driver tc2_idle_driver = {
.name = "tc2_idle",.owner = THIS_MODULE,.safe_state_index = 0-};
-static DEFINE_PER_CPU(struct cpuidle_device, tc2_idle_dev);
-static int notrace tc2_powerdown_finisher(unsigned long arg) -{
unsigned int mpidr = read_cpuid_mpidr();unsigned int cluster = (mpidr >> 8) & 0xf;unsigned int cpu = mpidr & 0xf;bL_set_entry_vector(cpu, cluster, cpu_resume);bL_cpu_suspend(0); /* 0 should be replaced with better value here */return 1;-}
-/*
- tc2_enter_powerdown - Programs CPU to enter the specified state
- @dev: cpuidle device
- @drv: The target state to be programmed
- @idx: state index
- Called from the CPUidle framework to program the device to the
- specified target state selected by the governor.
- */
-static int tc2_enter_powerdown(struct cpuidle_device *dev,
struct cpuidle_driver *drv, int idx)-{
struct timespec ts_preidle, ts_postidle, ts_idle;int ret;/* Used to keep track of the total time in idle */getnstimeofday(&ts_preidle);BUG_ON(!irqs_disabled());cpu_pm_enter();clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &dev->cpu);ret = cpu_suspend((unsigned long) dev, tc2_powerdown_finisher);if (ret)BUG();bL_cpu_powered_up();clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &dev->cpu);cpu_pm_exit();getnstimeofday(&ts_postidle);local_irq_enable();ts_idle = timespec_sub(ts_postidle, ts_preidle);dev->last_residency = ts_idle.tv_nsec / NSEC_PER_USEC +ts_idle.tv_sec * USEC_PER_SEC;return idx;-}
-/*
- tc2_idle_init
- Registers the TC2 specific cpuidle driver with the cpuidle
- framework with the valid set of states.
- */
-int __init tc2_idle_init(void) -{
struct cpuidle_device *dev;int i, cpu_id;struct cpuidle_driver *drv = &tc2_idle_driver;if (!vexpress_spc_check_loaded()) {pr_info("TC2 CPUidle not registered because no SPC found\n");return -ENODEV;}drv->state_count = (sizeof(tc2_cpuidle_set) /sizeof(struct cpuidle_state));for (i = 0; i < drv->state_count; i++) {memcpy(&drv->states[i], &tc2_cpuidle_set[i],sizeof(struct cpuidle_state));}cpuidle_register_driver(drv);for_each_cpu(cpu_id, cpu_online_mask) {pr_err("CPUidle for CPU%d registered\n", cpu_id);dev = &per_cpu(tc2_idle_dev, cpu_id);dev->cpu = cpu_id;dev->state_count = drv->state_count;if (cpuidle_register_device(dev)) {printk(KERN_ERR "%s: Cpuidle register device failed\n",__func__);return -EIO;}}return 0;-}
-late_initcall(tc2_idle_init); diff --git a/drivers/cpuidle/Makefile b/drivers/cpuidle/Makefile index 03ee874..398ece7 100644 --- a/drivers/cpuidle/Makefile +++ b/drivers/cpuidle/Makefile @@ -4,5 +4,5 @@
obj-y += cpuidle.o driver.o governor.o sysfs.o governors/ obj-$(CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED) += coupled.o
+obj-$(CONFIG_BIG_LITTLE) += arm_big_little.o obj-$(CONFIG_CPU_IDLE_CALXEDA) += cpuidle-calxeda.o diff --git a/drivers/cpuidle/arm_big_little.c b/drivers/cpuidle/arm_big_little.c new file mode 100755 index 0000000..b97ebe0 --- /dev/null +++ b/drivers/cpuidle/arm_big_little.c @@ -0,0 +1,183 @@ +/*
- big.LITTLE CPU idle driver.
- Copyright (C) 2012 ARM Ltd.
- Author: Lorenzo Pieralisi lorenzo.pieralisi@arm.com
- This program is free software; you can redistribute it and/or modify
- it under the terms of the GNU General Public License version 2 as
- published by the Free Software Foundation.
- */
+#include <linux/arm-cci.h> +#include <linux/bitmap.h> +#include <linux/cpuidle.h> +#include <linux/cpu_pm.h> +#include <linux/clockchips.h> +#include <linux/debugfs.h> +#include <linux/hrtimer.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/tick.h> +#include <linux/vexpress.h> +#include <asm/bL_entry.h> +#include <asm/cpuidle.h> +#include <asm/cputype.h> +#include <asm/idmap.h> +#include <asm/proc-fns.h> +#include <asm/suspend.h> +#include <linux/of.h>
+static int bl_cpuidle_simple_enter(struct cpuidle_device *dev,
struct cpuidle_driver *drv, int index)+{
ktime_t time_start, time_end;s64 diff;time_start = ktime_get();cpu_do_idle();time_end = ktime_get();local_irq_enable();diff = ktime_to_us(ktime_sub(time_end, time_start));if (diff > INT_MAX)diff = INT_MAX;dev->last_residency = (int) diff;return index;+}
+static int bl_enter_powerdown(struct cpuidle_device *dev,
struct cpuidle_driver *drv, int idx);+static struct cpuidle_state bl_cpuidle_set[] __initdata = {
[0] = {.enter = bl_cpuidle_simple_enter,.exit_latency = 1,.target_residency = 1,.power_usage = UINT_MAX,.flags = CPUIDLE_FLAG_TIME_VALID,.name = "WFI",.desc = "ARM WFI",},[1] = {.enter = bl_enter_powerdown,.exit_latency = 300,.target_residency = 1000,.flags = CPUIDLE_FLAG_TIME_VALID,.name = "C1",.desc = "ARM power down",},+};
+struct cpuidle_driver bl_idle_driver = {
.name = "bl_idle",.owner = THIS_MODULE,.safe_state_index = 0+};
+static DEFINE_PER_CPU(struct cpuidle_device, bl_idle_dev);
+static int notrace bl_powerdown_finisher(unsigned long arg) +{
unsigned int mpidr = read_cpuid_mpidr();unsigned int cluster = (mpidr >> 8) & 0xf;unsigned int cpu = mpidr & 0xf;bL_set_entry_vector(cpu, cluster, cpu_resume);bL_cpu_suspend(0); /* 0 should be replaced with better value here */return 1;+}
+/*
- bl_enter_powerdown - Programs CPU to enter the specified state
- @dev: cpuidle device
- @drv: The target state to be programmed
- @idx: state index
- Called from the CPUidle framework to program the device to the
- specified target state selected by the governor.
- */
+static int bl_enter_powerdown(struct cpuidle_device *dev,
struct cpuidle_driver *drv, int idx)+{
struct timespec ts_preidle, ts_postidle, ts_idle;int ret;/* Used to keep track of the total time in idle */getnstimeofday(&ts_preidle);BUG_ON(!irqs_disabled());cpu_pm_enter();clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &dev->cpu);ret = cpu_suspend((unsigned long) dev, bl_powerdown_finisher);if (ret)BUG();bL_cpu_powered_up();clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &dev->cpu);cpu_pm_exit();getnstimeofday(&ts_postidle);local_irq_enable();ts_idle = timespec_sub(ts_postidle, ts_preidle);dev->last_residency = ts_idle.tv_nsec / NSEC_PER_USEC +ts_idle.tv_sec * USEC_PER_SEC;return idx;+}
+/*
- bl_idle_init
- Registers the bl specific cpuidle driver with the cpuidle
- framework with the valid set of states.
- */
+int __init bl_idle_init(void) +{
struct cpuidle_device *dev;int i, cpu_id;struct cpuidle_driver *drv = &bl_idle_driver;if (!of_find_compatible_node(NULL, NULL, "arm,generic")) {pr_info("%s: No compatible node found\n", __func__);return -ENODEV;}drv->state_count = (sizeof(bl_cpuidle_set) /sizeof(struct cpuidle_state));for (i = 0; i < drv->state_count; i++) {memcpy(&drv->states[i], &bl_cpuidle_set[i],sizeof(struct cpuidle_state));}cpuidle_register_driver(drv);for_each_cpu(cpu_id, cpu_online_mask) {pr_err("CPUidle for CPU%d registered\n", cpu_id);dev = &per_cpu(bl_idle_dev, cpu_id);dev->cpu = cpu_id;dev->state_count = drv->state_count;if (cpuidle_register_device(dev)) {printk(KERN_ERR "%s: Cpuidle register device failed\n",__func__);return -EIO;}}return 0;+}
+late_initcall(bl_idle_init);
1.7.9.5
-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
On Tue, Jan 15, 2013 at 04:22:22PM +0000, Steve Bannister wrote:
Thanks for this Mark,
CCing Lorenzo as this doesn't seem to have come through on the linaro-devs list.
Cheers,
Steve
On 15 Jan 2013, at 13:35, Mark Hambleton mark.hambleton@broadcom.com wrote:
Take the TC2 version of CPUIdle and remove TC2 specific line. Move the file to drivers/cpuidle. Introduce the concept of "arm,generic" compatible drivers to avoid the need for a dummy dts node.
Build the new CPUIdle based on CONFIG_BIG_LITTLE.
Signed-off-by: mark mahamble@broadcom.com
Hi Nicolas / Dave,
After plugging in our ASIC support to the new big.LITTLE frameworks I realised that the TC2 CPUIdle driver was fundamentally generic, this patch moves it from mach-vexpress into drivers/cpuidle and then checks for the presence of a compatible node in the devicetree instead of checking for an SPC.
I haven't added a new config define as I figured it was specific to the big.LITTLE framework, so just used that instead.
I have tested the change on TC2 and am looking at extending the DCSCB code to make it work on fastmodels.
Any comments?
Regards,
Mark
[...]
diff --git a/drivers/cpuidle/Makefile b/drivers/cpuidle/Makefile index 03ee874..398ece7 100644 --- a/drivers/cpuidle/Makefile +++ b/drivers/cpuidle/Makefile @@ -4,5 +4,5 @@
obj-y += cpuidle.o driver.o governor.o sysfs.o governors/ obj-$(CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED) += coupled.o
+obj-$(CONFIG_BIG_LITTLE) += arm_big_little.o
There is nothing big.LITTLE specific in all of this, so arm_idle.c would be better.
obj-$(CONFIG_CPU_IDLE_CALXEDA) += cpuidle-calxeda.o diff --git a/drivers/cpuidle/arm_big_little.c b/drivers/cpuidle/arm_big_little.c new file mode 100755 index 0000000..b97ebe0 --- /dev/null +++ b/drivers/cpuidle/arm_big_little.c @@ -0,0 +1,183 @@ +/*
- big.LITTLE CPU idle driver.
See above.
- Copyright (C) 2012 ARM Ltd.
- Author: Lorenzo Pieralisi lorenzo.pieralisi@arm.com
Please copy me in next time.
- This program is free software; you can redistribute it and/or modify
- it under the terms of the GNU General Public License version 2 as
- published by the Free Software Foundation.
- */
+#include <linux/arm-cci.h> +#include <linux/bitmap.h> +#include <linux/cpuidle.h> +#include <linux/cpu_pm.h> +#include <linux/clockchips.h> +#include <linux/debugfs.h> +#include <linux/hrtimer.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/tick.h> +#include <linux/vexpress.h> +#include <asm/bL_entry.h> +#include <asm/cpuidle.h> +#include <asm/cputype.h> +#include <asm/idmap.h> +#include <asm/proc-fns.h> +#include <asm/suspend.h> +#include <linux/of.h>
+static int bl_cpuidle_simple_enter(struct cpuidle_device *dev,
struct cpuidle_driver *drv, int index)+{
ktime_t time_start, time_end;s64 diff;time_start = ktime_get();cpu_do_idle();time_end = ktime_get();local_irq_enable();diff = ktime_to_us(ktime_sub(time_end, time_start));if (diff > INT_MAX)diff = INT_MAX;dev->last_residency = (int) diff;return index;+}
+static int bl_enter_powerdown(struct cpuidle_device *dev,
struct cpuidle_driver *drv, int idx);+static struct cpuidle_state bl_cpuidle_set[] __initdata = {
[0] = {.enter = bl_cpuidle_simple_enter,.exit_latency = 1,.target_residency = 1,.power_usage = UINT_MAX,.flags = CPUIDLE_FLAG_TIME_VALID,.name = "WFI",.desc = "ARM WFI",},[1] = {.enter = bl_enter_powerdown,.exit_latency = 300,.target_residency = 1000,.flags = CPUIDLE_FLAG_TIME_VALID,.name = "C1",.desc = "ARM power down",},+};
First problem, these states are not standard, in particular target_residency, exit_latency and the enter function. We could have a stab at initializing them from DT (or provide a way to probe them from HW like Intel does), that's the only way to make this stuff generic.
+struct cpuidle_driver bl_idle_driver = {
.name = "bl_idle",.owner = THIS_MODULE,.safe_state_index = 0+};
Different clusters have different drivers. This can be solved with DT and a couple of bindings.
+static DEFINE_PER_CPU(struct cpuidle_device, bl_idle_dev);
+static int notrace bl_powerdown_finisher(unsigned long arg) +{
unsigned int mpidr = read_cpuid_mpidr();unsigned int cluster = (mpidr >> 8) & 0xf;unsigned int cpu = mpidr & 0xf;bL_set_entry_vector(cpu, cluster, cpu_resume);
This might not be what we want. We might want to resume from an address different from cpu_resume (although it might well be standardized since the power API allows to carry out platform specific operations before jumping back to the kernel so I reckon this can be and will be generic).
bL_cpu_suspend(0); /* 0 should be replaced with better value here */return 1;+}
+/*
- bl_enter_powerdown - Programs CPU to enter the specified state
- @dev: cpuidle device
- @drv: The target state to be programmed
- @idx: state index
- Called from the CPUidle framework to program the device to the
- specified target state selected by the governor.
- */
+static int bl_enter_powerdown(struct cpuidle_device *dev,
struct cpuidle_driver *drv, int idx)+{
struct timespec ts_preidle, ts_postidle, ts_idle;int ret;/* Used to keep track of the total time in idle */getnstimeofday(&ts_preidle);BUG_ON(!irqs_disabled());cpu_pm_enter();clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &dev->cpu);ret = cpu_suspend((unsigned long) dev, bl_powerdown_finisher);
This is where things start getting a little more complicated. If we do not know what the power state implies (e.g. L2 retained or not, global timer or emulated local timers, GIC state lost in low-power or not) we end up saving/restoring state or executing actions that may be useless.
In Intel world executing "mwait" does the trick for all C-states. Here we need to define what a state is and what it implies to carry out the required actions in a generic way, provided we can pull it off.
PSCI and the power API are a MAJOR step in this direction.
if (ret)BUG();bL_cpu_powered_up();clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &dev->cpu);cpu_pm_exit();getnstimeofday(&ts_postidle);local_irq_enable();ts_idle = timespec_sub(ts_postidle, ts_preidle);dev->last_residency = ts_idle.tv_nsec / NSEC_PER_USEC +ts_idle.tv_sec * USEC_PER_SEC;return idx;+}
+/*
- bl_idle_init
- Registers the bl specific cpuidle driver with the cpuidle
- framework with the valid set of states.
- */
+int __init bl_idle_init(void) +{
struct cpuidle_device *dev;int i, cpu_id;struct cpuidle_driver *drv = &bl_idle_driver;if (!of_find_compatible_node(NULL, NULL, "arm,generic")) {pr_info("%s: No compatible node found\n", __func__);return -ENODEV;}drv->state_count = (sizeof(bl_cpuidle_set) /sizeof(struct cpuidle_state));for (i = 0; i < drv->state_count; i++) {memcpy(&drv->states[i], &bl_cpuidle_set[i],sizeof(struct cpuidle_state));}cpuidle_register_driver(drv);for_each_cpu(cpu_id, cpu_online_mask) {pr_err("CPUidle for CPU%d registered\n", cpu_id);dev = &per_cpu(bl_idle_dev, cpu_id);dev->cpu = cpu_id;dev->state_count = drv->state_count;if (cpuidle_register_device(dev)) {printk(KERN_ERR "%s: Cpuidle register device failed\n",__func__);return -EIO;}}
This should be improved to cater for multiple drivers but that's not my major concern.
Lorenzo
-
Lorenzo Pieralisi -
Steve Bannister