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