[PATCH 3/9] X86/perf: fix power:cpu_idle double end events and throw cpu_idle events from the cpuidle layer

[PATCH 3/9] X86/perf: fix power:cpu_idle double end events and throw cpu_idle events from the cpuidle layer

[Thomas Renninger]

Currently intel_idle and acpi_idle driver show double cpu_idle "exit idle"
events -> this patch fixes it and makes cpu_idle events throwing less complex.

It also introduces cpu_idle events for all architectures which use
the cpuidle subsystem, namely:
  - arch/arm/mach-at91/cpuidle.c
  - arch/arm/mach-davinci/cpuidle.c
  - arch/arm/mach-kirkwood/cpuidle.c
  - arch/arm/mach-omap2/cpuidle34xx.c
  - arch/drivers/acpi/processor_idle.c (for all cases, not only mwait)
  - arch/x86/kernel/process.c (did throw events before, but was a mess)
  - drivers/idle/intel_idle.c (did throw events before)

Convention should be:
Fire cpu_idle events inside the current pm_idle function (not somewhere
down the the callee tree) to keep things easy.

Current possible pm_idle functions in X86:
c1e_idle, poll_idle, cpuidle_idle_call, mwait_idle, default_idle
-> this is really easy is now.

This affects userspace:
The type field of the cpu_idle power event can now direclty get
mapped to:
/sys/devices/system/cpu/cpuX/cpuidle/stateX/{name,desc,usage,time,...}
instead of throwing very CPU/mwait specific values.
This change is not visible for the intel_idle driver.
For the acpi_idle driver it should only be visible if the vendor
misses out C-states in his BIOS.
Another (perf timechart) patch reads out cpuidle info of cpu_idle
events from:
/sys/.../cpuidle/stateX/*, then the cpuidle events are mapped
to the correct C-/cpuidle state again, even if e.g. vendors miss
out C-states in their BIOS and for example only export C1 and C3.
-> everything is fine.

Signed-off-by: Thomas Renninger <trenn@suse.de>
CC: Robert Schoene <robert.schoene@tu-dresden.de>
CC: Jean Pihet <j-pihet@ti.com>
CC: Arjan van de Ven <arjan@linux.intel.com>
CC: Ingo Molnar <mingo@elte.hu>
CC: Len Brown <lenb@kernel.org>
CC: Frederic Weisbecker <fweisbec@gmail.com>
CC: linux-pm@lists.linux-foundation.org
CC: linux-acpi@vger.kernel.org
CC: linux-kernel@vger.kernel.org
CC: linux-perf-users@vger.kernel.org
CC: linux-omap@vger.kernel.org
---
 arch/x86/kernel/process.c    |    6 ++++--
 arch/x86/kernel/process_32.c |    4 ----
 arch/x86/kernel/process_64.c |    6 ------
 drivers/cpuidle/cpuidle.c    |   10 ++++++++--
 drivers/idle/intel_idle.c    |    2 --
 5 files changed, 12 insertions(+), 16 deletions(-)

diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 8b0ad65..b4f9ee7 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -385,6 +385,8 @@ void default_idle(void)
 		else
 			local_irq_enable();
 		current_thread_info()->status |= TS_POLLING;
+		trace_power_end(smp_processor_id());
+		trace_cpu_idle(PWR_EVENT_EXIT, smp_processor_id());
 	} else {
 		local_irq_enable();
 		/* loop is done by the caller */
@@ -442,8 +444,6 @@ EXPORT_SYMBOL_GPL(cpu_idle_wait);
  */
 void mwait_idle_with_hints(unsigned long ax, unsigned long cx)
 {
-	trace_power_start(POWER_CSTATE, (ax>>4)+1, smp_processor_id());
-	trace_cpu_idle((ax>>4)+1, smp_processor_id());
 	if (!need_resched()) {
 		if (cpu_has(&current_cpu_data, X86_FEATURE_CLFLUSH_MONITOR))
 			clflush((void *)&current_thread_info()->flags);
@@ -470,6 +470,8 @@ static void mwait_idle(void)
 			__sti_mwait(0, 0);
 		else
 			local_irq_enable();
+		trace_power_end(smp_processor_id());
+		trace_cpu_idle(PWR_EVENT_EXIT, smp_processor_id());
 	} else
 		local_irq_enable();
 }
diff --git a/arch/x86/kernel/process_32.c b/arch/x86/kernel/process_32.c
index 4b9befa..8d12878 100644
--- a/arch/x86/kernel/process_32.c
+++ b/arch/x86/kernel/process_32.c
@@ -57,8 +57,6 @@
 #include <asm/syscalls.h>
 #include <asm/debugreg.h>
 
-#include <trace/events/power.h>
-
 asmlinkage void ret_from_fork(void) __asm__("ret_from_fork");
 
 /*
@@ -113,8 +111,6 @@ void cpu_idle(void)
 			stop_critical_timings();
 			pm_idle();
 			start_critical_timings();
-			trace_power_end(smp_processor_id());
-			trace_cpu_idle(PWR_EVENT_EXIT, smp_processor_id());
 		}
 		tick_nohz_restart_sched_tick();
 		preempt_enable_no_resched();
diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
index 4c818a7..bd387e8 100644
--- a/arch/x86/kernel/process_64.c
+++ b/arch/x86/kernel/process_64.c
@@ -51,8 +51,6 @@
 #include <asm/syscalls.h>
 #include <asm/debugreg.h>
 
-#include <trace/events/power.h>
-
 asmlinkage extern void ret_from_fork(void);
 
 DEFINE_PER_CPU(unsigned long, old_rsp);
@@ -141,10 +139,6 @@ void cpu_idle(void)
 			pm_idle();
 			start_critical_timings();
 
-			trace_power_end(smp_processor_id());
-			trace_cpu_idle(PWR_EVENT_EXIT,
-				       smp_processor_id());
-
 			/* In many cases the interrupt that ended idle
 			   has already called exit_idle. But some idle
 			   loops can be woken up without interrupt. */
diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
index 99cc8fc..4649495 100644
--- a/drivers/cpuidle/cpuidle.c
+++ b/drivers/cpuidle/cpuidle.c
@@ -96,7 +96,15 @@ static void cpuidle_idle_call(void)
 
 	/* enter the state and update stats */
 	dev->last_state = target_state;
+
+	trace_power_start(POWER_CSTATE, next_state, dev->cpu);
+	trace_cpu_idle(next_state, dev->cpu);
+
 	dev->last_residency = target_state->enter(dev, target_state);
+
+	trace_power_end(dev->cpu);
+	trace_cpu_idle(PWR_EVENT_EXIT, dev->cpu);
+
 	if (dev->last_state)
 		target_state = dev->last_state;
 
@@ -106,8 +114,6 @@ static void cpuidle_idle_call(void)
 	/* give the governor an opportunity to reflect on the outcome */
 	if (cpuidle_curr_governor->reflect)
 		cpuidle_curr_governor->reflect(dev);
-	trace_power_end(smp_processor_id());
-	trace_cpu_idle(PWR_EVENT_EXIT, smp_processor_id());
 }
 
 /**
diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index 56ac09d..60fa6ec 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -220,8 +220,6 @@ static int intel_idle(struct cpuidle_device *dev, struct cpuidle_state *state)
 	kt_before = ktime_get_real();
 
 	stop_critical_timings();
-	trace_power_start(POWER_CSTATE, (eax >> 4) + 1, cpu);
-	trace_cpu_idle((eax >> 4) + 1, cpu);
 	if (!need_resched()) {
 
 		__monitor((void *)&current_thread_info()->flags, 0, 0);
-- 
1.7.3.1

Re: [PATCH 3/9] X86/perf: fix power:cpu_idle double end events and throw cpu_idle events from the cpuidle layer

[Len Brown]

I'm happy to see the trace point move up into cpuidle from intel_idle.

If somebody is picking this up in a perf tree,
Acked-by: Len Brown <len.brown@intel.com>

else I can put it in the idle tree, let me know.

thanks,
Len Brown, Intel Open Source Technology Center

Re: [PATCH 3/9] X86/perf: fix power:cpu_idle double end events and throw cpu_idle events from the cpuidle layer

[Thomas Renninger]

On Wednesday 12 January 2011 07:42:59 Len Brown wrote:
> I'm happy to see the trace point move up into cpuidle from intel_idle.
> 
> If somebody is picking this up in a perf tree,
> Acked-by: Len Brown <len.brown@intel.com>
> 
> else I can put it in the idle tree, let me know.
I didn't know you have an idle tree.

If you can push this one through the idle tree that would be great.

I'll go through the rest of my patches and resubmit
in some days as soon as some things are clarified.

But I need this one to still be included in .38.
Please tell me if this could be a problem.

Thanks,

  Thomas

Re: [PATCH 3/9] X86/perf: fix power:cpu_idle double end events and throw cpu_idle events from the cpuidle layer

[Len Brown]

Applied to idle-test

git://git.kernel.org/pub/scm/linux/kernel/git/lenb/linux-idle-2.6.git idle-test

Thomas,
Please test this branch, and base your incremental patches on it.

thanks,
Len Brown, Intel Open Source Technology Center

[PATCH 4/9] cpuidle: Introduce .abbr (abbrevation) for cpuidle states

[Thomas Renninger]

and fill name, description and newly introduced abbrevation
consistently (always use snprintf) in all cpuidle drivers.

This is mainly for perf timechart. It draws vector graphics
pictures of sleep/idle state usage catching perf cpu_idle events.
The string used for the idle state must not exceed 3 chars or
you can't see much in these pictures.
The name could get used in the title, the introduced abbrevations
inside of the picture and the text must therefore be rather short.

Signed-off-by: Thomas Renninger <trenn@suse.de>
CC: lenb@kernel.org
CC: linux-acpi@vger.kernel.org
CC: linux-pm@lists.linux-foundation.org
CC: Arjan van de Ven <arjan@linux.intel.com>
CC: Ingo Molnar <mingo@elte.hu>
CC: Frederic Weisbecker <fweisbec@gmail.com>
CC: linux-kernel@vger.kernel.org
CC: linux-omap@vger.kernel.org
---
 arch/arm/mach-at91/cpuidle.c          |   12 ++++++++----
 arch/arm/mach-davinci/cpuidle.c       |   13 +++++++++----
 arch/arm/mach-kirkwood/cpuidle.c      |   12 ++++++++----
 arch/arm/mach-omap2/cpuidle34xx.c     |    3 ++-
 arch/sh/kernel/cpu/shmobile/cpuidle.c |   19 +++++++++++--------
 drivers/acpi/processor_idle.c         |    3 +++
 drivers/cpuidle/cpuidle.c             |    1 +
 drivers/cpuidle/sysfs.c               |    3 +++
 drivers/idle/intel_idle.c             |   11 +++++++++++
 include/linux/cpuidle.h               |    2 ++
 10 files changed, 58 insertions(+), 21 deletions(-)

diff --git a/arch/arm/mach-at91/cpuidle.c b/arch/arm/mach-at91/cpuidle.c
index 1cfeac1..6cdeb42 100644
--- a/arch/arm/mach-at91/cpuidle.c
+++ b/arch/arm/mach-at91/cpuidle.c
@@ -73,16 +73,20 @@ static int at91_init_cpuidle(void)
 	device->states[0].exit_latency = 1;
 	device->states[0].target_residency = 10000;
 	device->states[0].flags = CPUIDLE_FLAG_TIME_VALID;
-	strcpy(device->states[0].name, "WFI");
-	strcpy(device->states[0].desc, "Wait for interrupt");
+	snprintf(device->states[0].name, CPUIDLE_NAME_LEN, "WFI");
+	snprintf(device->states[0].desc, CPUIDLE_DESC_LEN,
+		 "Wait for interrupt");
+	snprintf(device->states[0].abbr, CPUIDLE_ABBR_LEN, "W");
 
 	/* Wait for interrupt and RAM self refresh state */
 	device->states[1].enter = at91_enter_idle;
 	device->states[1].exit_latency = 10;
 	device->states[1].target_residency = 10000;
 	device->states[1].flags = CPUIDLE_FLAG_TIME_VALID;
-	strcpy(device->states[1].name, "RAM_SR");
-	strcpy(device->states[1].desc, "WFI and RAM Self Refresh");
+	snprintf(device->states[1].name, CPUIDLE_NAME_LEN, "RAM SR");
+	snprintf(device->states[1].desc, CPUIDLE_DESC_LEN,
+		 "WFI and RAM Self Refresh");
+	snprintf(device->states[1].abbr, CPUIDLE_ABBR_LEN, "WSR");
 
 	if (cpuidle_register_device(device)) {
 		printk(KERN_ERR "at91_init_cpuidle: Failed registering\n");
diff --git a/arch/arm/mach-davinci/cpuidle.c b/arch/arm/mach-davinci/cpuidle.c
index bd59f31..42ad2d6 100644
--- a/arch/arm/mach-davinci/cpuidle.c
+++ b/arch/arm/mach-davinci/cpuidle.c
@@ -127,16 +127,21 @@ static int __init davinci_cpuidle_probe(struct platform_device *pdev)
 	device->states[0].exit_latency = 1;
 	device->states[0].target_residency = 10000;
 	device->states[0].flags = CPUIDLE_FLAG_TIME_VALID;
-	strcpy(device->states[0].name, "WFI");
-	strcpy(device->states[0].desc, "Wait for interrupt");
+	snprintf(device->states[0].name, CPUIDLE_NAME_LEN, "WFI");
+	snprintf(device->states[0].desc, CPUIDLE_DESC_LEN,
+		 "Wait for interrupt");
+	snprintf(device->states[0].abbr, CPUIDLE_ABBR_LEN, "W");
 
 	/* Wait for interrupt and DDR self refresh state */
 	device->states[1].enter = davinci_enter_idle;
 	device->states[1].exit_latency = 10;
 	device->states[1].target_residency = 10000;
 	device->states[1].flags = CPUIDLE_FLAG_TIME_VALID;
-	strcpy(device->states[1].name, "DDR SR");
-	strcpy(device->states[1].desc, "WFI and DDR Self Refresh");
+	snprintf(device->states[1].name, CPUIDLE_NAME_LEN, "RAM SR");
+	snprintf(device->states[1].desc, CPUIDLE_DESC_LEN,
+		 "WFI and RAM Self Refresh");
+	snprintf(device->states[1].abbr, CPUIDLE_ABBR_LEN, "WSR");
+
 	if (pdata->ddr2_pdown)
 		davinci_states[1].flags |= DAVINCI_CPUIDLE_FLAGS_DDR2_PWDN;
 	cpuidle_set_statedata(&device->states[1], &davinci_states[1]);
diff --git a/arch/arm/mach-kirkwood/cpuidle.c b/arch/arm/mach-kirkwood/cpuidle.c
index f68d33f..48eaabb 100644
--- a/arch/arm/mach-kirkwood/cpuidle.c
+++ b/arch/arm/mach-kirkwood/cpuidle.c
@@ -75,16 +75,20 @@ static int kirkwood_init_cpuidle(void)
 	device->states[0].exit_latency = 1;
 	device->states[0].target_residency = 10000;
 	device->states[0].flags = CPUIDLE_FLAG_TIME_VALID;
-	strcpy(device->states[0].name, "WFI");
-	strcpy(device->states[0].desc, "Wait for interrupt");
+	snprintf(device->states[0].name, CPUIDLE_NAME_LEN, "WFI");
+	snprintf(device->states[0].desc, CPUIDLE_DESC_LEN,
+		 "Wait for interrupt");
+	snprintf(device->states[0].abbr, CPUIDLE_ABBR_LEN, "W");
 
 	/* Wait for interrupt and DDR self refresh state */
 	device->states[1].enter = kirkwood_enter_idle;
 	device->states[1].exit_latency = 10;
 	device->states[1].target_residency = 10000;
 	device->states[1].flags = CPUIDLE_FLAG_TIME_VALID;
-	strcpy(device->states[1].name, "DDR SR");
-	strcpy(device->states[1].desc, "WFI and DDR Self Refresh");
+	snprintf(device->states[1].name, CPUIDLE_NAME_LEN, "RAM SR");
+	snprintf(device->states[1].desc, CPUIDLE_DESC_LEN,
+		 "WFI and RAM Self Refresh");
+	snprintf(device->states[1].abbr, CPUIDLE_ABBR_LEN, "WSR");
 
 	if (cpuidle_register_device(device)) {
 		printk(KERN_ERR "kirkwood_init_cpuidle: Failed registering\n");
diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
index 0d50b45..a59ac39 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -496,7 +496,8 @@ int __init omap3_idle_init(void)
 			omap3_enter_idle_bm : omap3_enter_idle;
 		if (cx->type == OMAP3_STATE_C1)
 			dev->safe_state = state;
-		sprintf(state->name, "C%d", count+1);
+		snprintf(state->name, CPUIDLE_NAME_LEN, "C%d", count+1);
+		snprintf(state->abbr, CPUIDLE_ABBR_LEN, "C%d", count+1);
 		count++;
 	}
 
diff --git a/arch/sh/kernel/cpu/shmobile/cpuidle.c b/arch/sh/kernel/cpu/shmobile/cpuidle.c
index 83972aa..9ad151d 100644
--- a/arch/sh/kernel/cpu/shmobile/cpuidle.c
+++ b/arch/sh/kernel/cpu/shmobile/cpuidle.c
@@ -75,8 +75,9 @@ void sh_mobile_setup_cpuidle(void)
 	i = CPUIDLE_DRIVER_STATE_START;
 
 	state = &dev->states[i++];
-	snprintf(state->name, CPUIDLE_NAME_LEN, "C0");
-	strncpy(state->desc, "SuperH Sleep Mode", CPUIDLE_DESC_LEN);
+	snprintf(state->name, CPUIDLE_NAME_LEN, "SuperH");
+	snprintf(state->desc, CPUIDLE_DESC_LEN, "SuperH Sleep Mode");
+	snprintf(state->abbr, CPUIDLE_ABBR_LEN, "SH");
 	state->exit_latency = 1;
 	state->target_residency = 1 * 2;
 	state->power_usage = 3;
@@ -89,9 +90,10 @@ void sh_mobile_setup_cpuidle(void)
 
 	if (sh_mobile_sleep_supported & SUSP_SH_SF) {
 		state = &dev->states[i++];
-		snprintf(state->name, CPUIDLE_NAME_LEN, "C1");
-		strncpy(state->desc, "SuperH Sleep Mode [SF]",
-			CPUIDLE_DESC_LEN);
+		snprintf(state->name, CPUIDLE_NAME_LEN, "SuperH [SF]");
+		snprintf(state->desc, CPUIDLE_DESC_LEN,
+			 "SuperH Sleep Mode [SF]");
+		snprintf(state->abbr, CPUIDLE_ABBR_LEN, "SHF");
 		state->exit_latency = 100;
 		state->target_residency = 1 * 2;
 		state->power_usage = 1;
@@ -102,9 +104,10 @@ void sh_mobile_setup_cpuidle(void)
 
 	if (sh_mobile_sleep_supported & SUSP_SH_STANDBY) {
 		state = &dev->states[i++];
-		snprintf(state->name, CPUIDLE_NAME_LEN, "C2");
-		strncpy(state->desc, "SuperH Mobile Standby Mode [SF]",
-			CPUIDLE_DESC_LEN);
+		snprintf(state->name, CPUIDLE_NAME_LEN, "SuperH Standby [SF]");
+		snprintf(state->desc, CPUIDLE_DESC_LEN,
+			 "SuperH Mobile Standby Mode [SF]");
+		snprintf(state->abbr, CPUIDLE_ABBR_LEN, "SHS");
 		state->exit_latency = 2300;
 		state->target_residency = 1 * 2;
 		state->power_usage = 1;
diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index 104ae77..b28693e 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -1016,6 +1016,7 @@ static int acpi_processor_setup_cpuidle(struct acpi_processor *pr)
 		switch (cx->type) {
 			case ACPI_STATE_C1:
 			snprintf(state->name, CPUIDLE_NAME_LEN, "C1");
+			snprintf(state->abbr, CPUIDLE_ABBR_LEN, "C1");
 			state->flags |= CPUIDLE_FLAG_SHALLOW;
 			if (cx->entry_method == ACPI_CSTATE_FFH)
 				state->flags |= CPUIDLE_FLAG_TIME_VALID;
@@ -1026,6 +1027,7 @@ static int acpi_processor_setup_cpuidle(struct acpi_processor *pr)
 
 			case ACPI_STATE_C2:
 			snprintf(state->name, CPUIDLE_NAME_LEN, "C2");
+			snprintf(state->abbr, CPUIDLE_ABBR_LEN, "C2");
 			state->flags |= CPUIDLE_FLAG_BALANCED;
 			state->flags |= CPUIDLE_FLAG_TIME_VALID;
 			state->enter = acpi_idle_enter_simple;
@@ -1034,6 +1036,7 @@ static int acpi_processor_setup_cpuidle(struct acpi_processor *pr)
 
 			case ACPI_STATE_C3:
 			snprintf(state->name, CPUIDLE_NAME_LEN, "C3");
+			snprintf(state->abbr, CPUIDLE_ABBR_LEN, "C3");
 			state->flags |= CPUIDLE_FLAG_DEEP;
 			state->flags |= CPUIDLE_FLAG_TIME_VALID;
 			state->flags |= CPUIDLE_FLAG_CHECK_BM;
diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
index 4649495..b2d2b69 100644
--- a/drivers/cpuidle/cpuidle.c
+++ b/drivers/cpuidle/cpuidle.c
@@ -268,6 +268,7 @@ static void poll_idle_init(struct cpuidle_device *dev)
 
 	snprintf(state->name, CPUIDLE_NAME_LEN, "POLL");
 	snprintf(state->desc, CPUIDLE_DESC_LEN, "CPUIDLE CORE POLL IDLE");
+	snprintf(state->abbr, CPUIDLE_ABBR_LEN, "P");
 	state->exit_latency = 0;
 	state->target_residency = 0;
 	state->power_usage = -1;
diff --git a/drivers/cpuidle/sysfs.c b/drivers/cpuidle/sysfs.c
index 0310ffa..ca7a62c 100644
--- a/drivers/cpuidle/sysfs.c
+++ b/drivers/cpuidle/sysfs.c
@@ -249,9 +249,11 @@ define_show_state_ull_function(usage)
 define_show_state_ull_function(time)
 define_show_state_str_function(name)
 define_show_state_str_function(desc)
+define_show_state_str_function(abbr)
 
 define_one_state_ro(name, show_state_name);
 define_one_state_ro(desc, show_state_desc);
+define_one_state_ro(abbr, show_state_abbr);
 define_one_state_ro(latency, show_state_exit_latency);
 define_one_state_ro(power, show_state_power_usage);
 define_one_state_ro(usage, show_state_usage);
@@ -260,6 +262,7 @@ define_one_state_ro(time, show_state_time);
 static struct attribute *cpuidle_state_default_attrs[] = {
 	&attr_name.attr,
 	&attr_desc.attr,
+	&attr_abbr.attr,
 	&attr_latency.attr,
 	&attr_power.attr,
 	&attr_usage.attr,
diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index 60fa6ec..3bb1f2b 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -91,6 +91,7 @@ static struct cpuidle_state nehalem_cstates[MWAIT_MAX_NUM_CSTATES] = {
 	{ /* MWAIT C1 */
 		.name = "NHM-C1",
 		.desc = "MWAIT 0x00",
+		.abbr = "C1",
 		.driver_data = (void *) 0x00,
 		.flags = CPUIDLE_FLAG_TIME_VALID,
 		.exit_latency = 3,
@@ -99,6 +100,7 @@ static struct cpuidle_state nehalem_cstates[MWAIT_MAX_NUM_CSTATES] = {
 	{ /* MWAIT C2 */
 		.name = "NHM-C3",
 		.desc = "MWAIT 0x10",
+		.abbr = "C3",
 		.driver_data = (void *) 0x10,
 		.flags = CPUIDLE_FLAG_TIME_VALID | CPUIDLE_FLAG_TLB_FLUSHED,
 		.exit_latency = 20,
@@ -107,6 +109,7 @@ static struct cpuidle_state nehalem_cstates[MWAIT_MAX_NUM_CSTATES] = {
 	{ /* MWAIT C3 */
 		.name = "NHM-C6",
 		.desc = "MWAIT 0x20",
+		.abbr = "C6",
 		.driver_data = (void *) 0x20,
 		.flags = CPUIDLE_FLAG_TIME_VALID | CPUIDLE_FLAG_TLB_FLUSHED,
 		.exit_latency = 200,
@@ -119,6 +122,7 @@ static struct cpuidle_state snb_cstates[MWAIT_MAX_NUM_CSTATES] = {
 	{ /* MWAIT C1 */
 		.name = "SNB-C1",
 		.desc = "MWAIT 0x00",
+		.abbr = "C1",
 		.driver_data = (void *) 0x00,
 		.flags = CPUIDLE_FLAG_TIME_VALID,
 		.exit_latency = 1,
@@ -127,6 +131,7 @@ static struct cpuidle_state snb_cstates[MWAIT_MAX_NUM_CSTATES] = {
 	{ /* MWAIT C2 */
 		.name = "SNB-C3",
 		.desc = "MWAIT 0x10",
+		.abbr = "C3",
 		.driver_data = (void *) 0x10,
 		.flags = CPUIDLE_FLAG_TIME_VALID | CPUIDLE_FLAG_TLB_FLUSHED,
 		.exit_latency = 80,
@@ -135,6 +140,7 @@ static struct cpuidle_state snb_cstates[MWAIT_MAX_NUM_CSTATES] = {
 	{ /* MWAIT C3 */
 		.name = "SNB-C6",
 		.desc = "MWAIT 0x20",
+		.abbr = "C6",
 		.driver_data = (void *) 0x20,
 		.flags = CPUIDLE_FLAG_TIME_VALID | CPUIDLE_FLAG_TLB_FLUSHED,
 		.exit_latency = 104,
@@ -143,6 +149,7 @@ static struct cpuidle_state snb_cstates[MWAIT_MAX_NUM_CSTATES] = {
 	{ /* MWAIT C4 */
 		.name = "SNB-C7",
 		.desc = "MWAIT 0x30",
+		.abbr = "C7",
 		.driver_data = (void *) 0x30,
 		.flags = CPUIDLE_FLAG_TIME_VALID | CPUIDLE_FLAG_TLB_FLUSHED,
 		.exit_latency = 109,
@@ -155,6 +162,7 @@ static struct cpuidle_state atom_cstates[MWAIT_MAX_NUM_CSTATES] = {
 	{ /* MWAIT C1 */
 		.name = "ATM-C1",
 		.desc = "MWAIT 0x00",
+		.abbr = "C1",
 		.driver_data = (void *) 0x00,
 		.flags = CPUIDLE_FLAG_TIME_VALID,
 		.exit_latency = 1,
@@ -163,6 +171,7 @@ static struct cpuidle_state atom_cstates[MWAIT_MAX_NUM_CSTATES] = {
 	{ /* MWAIT C2 */
 		.name = "ATM-C2",
 		.desc = "MWAIT 0x10",
+		.abbr = "C2",
 		.driver_data = (void *) 0x10,
 		.flags = CPUIDLE_FLAG_TIME_VALID,
 		.exit_latency = 20,
@@ -172,6 +181,7 @@ static struct cpuidle_state atom_cstates[MWAIT_MAX_NUM_CSTATES] = {
 	{ /* MWAIT C4 */
 		.name = "ATM-C4",
 		.desc = "MWAIT 0x30",
+		.abbr = "C4",
 		.driver_data = (void *) 0x30,
 		.flags = CPUIDLE_FLAG_TIME_VALID | CPUIDLE_FLAG_TLB_FLUSHED,
 		.exit_latency = 100,
@@ -181,6 +191,7 @@ static struct cpuidle_state atom_cstates[MWAIT_MAX_NUM_CSTATES] = {
 	{ /* MWAIT C6 */
 		.name = "ATM-C6",
 		.desc = "MWAIT 0x52",
+		.abbr = "C6",
 		.driver_data = (void *) 0x52,
 		.flags = CPUIDLE_FLAG_TIME_VALID | CPUIDLE_FLAG_TLB_FLUSHED,
 		.exit_latency = 140,
diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
index 1be416b..4763ef3 100644
--- a/include/linux/cpuidle.h
+++ b/include/linux/cpuidle.h
@@ -20,6 +20,7 @@
 #define CPUIDLE_STATE_MAX	8
 #define CPUIDLE_NAME_LEN	16
 #define CPUIDLE_DESC_LEN	32
+#define CPUIDLE_ABBR_LEN	3
 
 struct cpuidle_device;
 
@@ -31,6 +32,7 @@ struct cpuidle_device;
 struct cpuidle_state {
 	char		name[CPUIDLE_NAME_LEN];
 	char		desc[CPUIDLE_DESC_LEN];
+	char		abbr[CPUIDLE_ABBR_LEN];
 	void		*driver_data;
 
 	unsigned int	flags;
-- 
1.7.3.1

Re: [PATCH 4/9] cpuidle: Introduce .abbr (abbrevation) for cpuidle states

[Kevin Hilman]

Thomas Renninger <trenn@suse.de> writes:

> and fill name, description and newly introduced abbrevation
> consistently (always use snprintf) in all cpuidle drivers.
>
> This is mainly for perf timechart. It draws vector graphics
> pictures of sleep/idle state usage catching perf cpu_idle events.
> The string used for the idle state must not exceed 3 chars or
> you can't see much in these pictures.
> The name could get used in the title, the introduced abbrevations
> inside of the picture and the text must therefore be rather short.
>
> Signed-off-by: Thomas Renninger <trenn@suse.de>
> CC: lenb@kernel.org
> CC: linux-acpi@vger.kernel.org
> CC: linux-pm@lists.linux-foundation.org
> CC: Arjan van de Ven <arjan@linux.intel.com>
> CC: Ingo Molnar <mingo@elte.hu>
> CC: Frederic Weisbecker <fweisbec@gmail.com>
> CC: linux-kernel@vger.kernel.org
> CC: linux-omap@vger.kernel.org
> ---
>  arch/arm/mach-at91/cpuidle.c          |   12 ++++++++----
>  arch/arm/mach-davinci/cpuidle.c       |   13 +++++++++----
>  arch/arm/mach-kirkwood/cpuidle.c      |   12 ++++++++----
>  arch/arm/mach-omap2/cpuidle34xx.c     |    3 ++-
>  arch/sh/kernel/cpu/shmobile/cpuidle.c |   19 +++++++++++--------
>  drivers/acpi/processor_idle.c         |    3 +++
>  drivers/cpuidle/cpuidle.c             |    1 +
>  drivers/cpuidle/sysfs.c               |    3 +++
>  drivers/idle/intel_idle.c             |   11 +++++++++++
>  include/linux/cpuidle.h               |    2 ++
>  10 files changed, 58 insertions(+), 21 deletions(-)

For the davinci & OMAP changes,

Acked-by: Kevin Hilman <khilman@ti.com>


> diff --git a/arch/arm/mach-at91/cpuidle.c b/arch/arm/mach-at91/cpuidle.c
> index 1cfeac1..6cdeb42 100644
> --- a/arch/arm/mach-at91/cpuidle.c
> +++ b/arch/arm/mach-at91/cpuidle.c
> @@ -73,16 +73,20 @@ static int at91_init_cpuidle(void)
>  	device->states[0].exit_latency = 1;
>  	device->states[0].target_residency = 10000;
>  	device->states[0].flags = CPUIDLE_FLAG_TIME_VALID;
> -	strcpy(device->states[0].name, "WFI");
> -	strcpy(device->states[0].desc, "Wait for interrupt");
> +	snprintf(device->states[0].name, CPUIDLE_NAME_LEN, "WFI");
> +	snprintf(device->states[0].desc, CPUIDLE_DESC_LEN,
> +		 "Wait for interrupt");
> +	snprintf(device->states[0].abbr, CPUIDLE_ABBR_LEN, "W");
>  
>  	/* Wait for interrupt and RAM self refresh state */
>  	device->states[1].enter = at91_enter_idle;
>  	device->states[1].exit_latency = 10;
>  	device->states[1].target_residency = 10000;
>  	device->states[1].flags = CPUIDLE_FLAG_TIME_VALID;
> -	strcpy(device->states[1].name, "RAM_SR");
> -	strcpy(device->states[1].desc, "WFI and RAM Self Refresh");
> +	snprintf(device->states[1].name, CPUIDLE_NAME_LEN, "RAM SR");
> +	snprintf(device->states[1].desc, CPUIDLE_DESC_LEN,
> +		 "WFI and RAM Self Refresh");
> +	snprintf(device->states[1].abbr, CPUIDLE_ABBR_LEN, "WSR");
>  
>  	if (cpuidle_register_device(device)) {
>  		printk(KERN_ERR "at91_init_cpuidle: Failed registering\n");
> diff --git a/arch/arm/mach-davinci/cpuidle.c b/arch/arm/mach-davinci/cpuidle.c
> index bd59f31..42ad2d6 100644
> --- a/arch/arm/mach-davinci/cpuidle.c
> +++ b/arch/arm/mach-davinci/cpuidle.c
> @@ -127,16 +127,21 @@ static int __init davinci_cpuidle_probe(struct platform_device *pdev)
>  	device->states[0].exit_latency = 1;
>  	device->states[0].target_residency = 10000;
>  	device->states[0].flags = CPUIDLE_FLAG_TIME_VALID;
> -	strcpy(device->states[0].name, "WFI");
> -	strcpy(device->states[0].desc, "Wait for interrupt");
> +	snprintf(device->states[0].name, CPUIDLE_NAME_LEN, "WFI");
> +	snprintf(device->states[0].desc, CPUIDLE_DESC_LEN,
> +		 "Wait for interrupt");
> +	snprintf(device->states[0].abbr, CPUIDLE_ABBR_LEN, "W");
>  
>  	/* Wait for interrupt and DDR self refresh state */
>  	device->states[1].enter = davinci_enter_idle;
>  	device->states[1].exit_latency = 10;
>  	device->states[1].target_residency = 10000;
>  	device->states[1].flags = CPUIDLE_FLAG_TIME_VALID;
> -	strcpy(device->states[1].name, "DDR SR");
> -	strcpy(device->states[1].desc, "WFI and DDR Self Refresh");
> +	snprintf(device->states[1].name, CPUIDLE_NAME_LEN, "RAM SR");
> +	snprintf(device->states[1].desc, CPUIDLE_DESC_LEN,
> +		 "WFI and RAM Self Refresh");
> +	snprintf(device->states[1].abbr, CPUIDLE_ABBR_LEN, "WSR");
> +
>  	if (pdata->ddr2_pdown)
>  		davinci_states[1].flags |= DAVINCI_CPUIDLE_FLAGS_DDR2_PWDN;
>  	cpuidle_set_statedata(&device->states[1], &davinci_states[1]);
> diff --git a/arch/arm/mach-kirkwood/cpuidle.c b/arch/arm/mach-kirkwood/cpuidle.c
> index f68d33f..48eaabb 100644
> --- a/arch/arm/mach-kirkwood/cpuidle.c
> +++ b/arch/arm/mach-kirkwood/cpuidle.c
> @@ -75,16 +75,20 @@ static int kirkwood_init_cpuidle(void)
>  	device->states[0].exit_latency = 1;
>  	device->states[0].target_residency = 10000;
>  	device->states[0].flags = CPUIDLE_FLAG_TIME_VALID;
> -	strcpy(device->states[0].name, "WFI");
> -	strcpy(device->states[0].desc, "Wait for interrupt");
> +	snprintf(device->states[0].name, CPUIDLE_NAME_LEN, "WFI");
> +	snprintf(device->states[0].desc, CPUIDLE_DESC_LEN,
> +		 "Wait for interrupt");
> +	snprintf(device->states[0].abbr, CPUIDLE_ABBR_LEN, "W");
>  
>  	/* Wait for interrupt and DDR self refresh state */
>  	device->states[1].enter = kirkwood_enter_idle;
>  	device->states[1].exit_latency = 10;
>  	device->states[1].target_residency = 10000;
>  	device->states[1].flags = CPUIDLE_FLAG_TIME_VALID;
> -	strcpy(device->states[1].name, "DDR SR");
> -	strcpy(device->states[1].desc, "WFI and DDR Self Refresh");
> +	snprintf(device->states[1].name, CPUIDLE_NAME_LEN, "RAM SR");
> +	snprintf(device->states[1].desc, CPUIDLE_DESC_LEN,
> +		 "WFI and RAM Self Refresh");
> +	snprintf(device->states[1].abbr, CPUIDLE_ABBR_LEN, "WSR");
>  
>  	if (cpuidle_register_device(device)) {
>  		printk(KERN_ERR "kirkwood_init_cpuidle: Failed registering\n");
> diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
> index 0d50b45..a59ac39 100644
> --- a/arch/arm/mach-omap2/cpuidle34xx.c
> +++ b/arch/arm/mach-omap2/cpuidle34xx.c
> @@ -496,7 +496,8 @@ int __init omap3_idle_init(void)
>  			omap3_enter_idle_bm : omap3_enter_idle;
>  		if (cx->type == OMAP3_STATE_C1)
>  			dev->safe_state = state;
> -		sprintf(state->name, "C%d", count+1);
> +		snprintf(state->name, CPUIDLE_NAME_LEN, "C%d", count+1);
> +		snprintf(state->abbr, CPUIDLE_ABBR_LEN, "C%d", count+1);
>  		count++;
>  	}
>  
> diff --git a/arch/sh/kernel/cpu/shmobile/cpuidle.c b/arch/sh/kernel/cpu/shmobile/cpuidle.c
> index 83972aa..9ad151d 100644
> --- a/arch/sh/kernel/cpu/shmobile/cpuidle.c
> +++ b/arch/sh/kernel/cpu/shmobile/cpuidle.c
> @@ -75,8 +75,9 @@ void sh_mobile_setup_cpuidle(void)
>  	i = CPUIDLE_DRIVER_STATE_START;
>  
>  	state = &dev->states[i++];
> -	snprintf(state->name, CPUIDLE_NAME_LEN, "C0");
> -	strncpy(state->desc, "SuperH Sleep Mode", CPUIDLE_DESC_LEN);
> +	snprintf(state->name, CPUIDLE_NAME_LEN, "SuperH");
> +	snprintf(state->desc, CPUIDLE_DESC_LEN, "SuperH Sleep Mode");
> +	snprintf(state->abbr, CPUIDLE_ABBR_LEN, "SH");
>  	state->exit_latency = 1;
>  	state->target_residency = 1 * 2;
>  	state->power_usage = 3;
> @@ -89,9 +90,10 @@ void sh_mobile_setup_cpuidle(void)
>  
>  	if (sh_mobile_sleep_supported & SUSP_SH_SF) {
>  		state = &dev->states[i++];
> -		snprintf(state->name, CPUIDLE_NAME_LEN, "C1");
> -		strncpy(state->desc, "SuperH Sleep Mode [SF]",
> -			CPUIDLE_DESC_LEN);
> +		snprintf(state->name, CPUIDLE_NAME_LEN, "SuperH [SF]");
> +		snprintf(state->desc, CPUIDLE_DESC_LEN,
> +			 "SuperH Sleep Mode [SF]");
> +		snprintf(state->abbr, CPUIDLE_ABBR_LEN, "SHF");
>  		state->exit_latency = 100;
>  		state->target_residency = 1 * 2;
>  		state->power_usage = 1;
> @@ -102,9 +104,10 @@ void sh_mobile_setup_cpuidle(void)
>  
>  	if (sh_mobile_sleep_supported & SUSP_SH_STANDBY) {
>  		state = &dev->states[i++];
> -		snprintf(state->name, CPUIDLE_NAME_LEN, "C2");
> -		strncpy(state->desc, "SuperH Mobile Standby Mode [SF]",
> -			CPUIDLE_DESC_LEN);
> +		snprintf(state->name, CPUIDLE_NAME_LEN, "SuperH Standby [SF]");
> +		snprintf(state->desc, CPUIDLE_DESC_LEN,
> +			 "SuperH Mobile Standby Mode [SF]");
> +		snprintf(state->abbr, CPUIDLE_ABBR_LEN, "SHS");
>  		state->exit_latency = 2300;
>  		state->target_residency = 1 * 2;
>  		state->power_usage = 1;
> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
> index 104ae77..b28693e 100644
> --- a/drivers/acpi/processor_idle.c
> +++ b/drivers/acpi/processor_idle.c
> @@ -1016,6 +1016,7 @@ static int acpi_processor_setup_cpuidle(struct acpi_processor *pr)
>  		switch (cx->type) {
>  			case ACPI_STATE_C1:
>  			snprintf(state->name, CPUIDLE_NAME_LEN, "C1");
> +			snprintf(state->abbr, CPUIDLE_ABBR_LEN, "C1");
>  			state->flags |= CPUIDLE_FLAG_SHALLOW;
>  			if (cx->entry_method == ACPI_CSTATE_FFH)
>  				state->flags |= CPUIDLE_FLAG_TIME_VALID;
> @@ -1026,6 +1027,7 @@ static int acpi_processor_setup_cpuidle(struct acpi_processor *pr)
>  
>  			case ACPI_STATE_C2:
>  			snprintf(state->name, CPUIDLE_NAME_LEN, "C2");
> +			snprintf(state->abbr, CPUIDLE_ABBR_LEN, "C2");
>  			state->flags |= CPUIDLE_FLAG_BALANCED;
>  			state->flags |= CPUIDLE_FLAG_TIME_VALID;
>  			state->enter = acpi_idle_enter_simple;
> @@ -1034,6 +1036,7 @@ static int acpi_processor_setup_cpuidle(struct acpi_processor *pr)
>  
>  			case ACPI_STATE_C3:
>  			snprintf(state->name, CPUIDLE_NAME_LEN, "C3");
> +			snprintf(state->abbr, CPUIDLE_ABBR_LEN, "C3");
>  			state->flags |= CPUIDLE_FLAG_DEEP;
>  			state->flags |= CPUIDLE_FLAG_TIME_VALID;
>  			state->flags |= CPUIDLE_FLAG_CHECK_BM;
> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index 4649495..b2d2b69 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -268,6 +268,7 @@ static void poll_idle_init(struct cpuidle_device *dev)
>  
>  	snprintf(state->name, CPUIDLE_NAME_LEN, "POLL");
>  	snprintf(state->desc, CPUIDLE_DESC_LEN, "CPUIDLE CORE POLL IDLE");
> +	snprintf(state->abbr, CPUIDLE_ABBR_LEN, "P");
>  	state->exit_latency = 0;
>  	state->target_residency = 0;
>  	state->power_usage = -1;
> diff --git a/drivers/cpuidle/sysfs.c b/drivers/cpuidle/sysfs.c
> index 0310ffa..ca7a62c 100644
> --- a/drivers/cpuidle/sysfs.c
> +++ b/drivers/cpuidle/sysfs.c
> @@ -249,9 +249,11 @@ define_show_state_ull_function(usage)
>  define_show_state_ull_function(time)
>  define_show_state_str_function(name)
>  define_show_state_str_function(desc)
> +define_show_state_str_function(abbr)
>  
>  define_one_state_ro(name, show_state_name);
>  define_one_state_ro(desc, show_state_desc);
> +define_one_state_ro(abbr, show_state_abbr);
>  define_one_state_ro(latency, show_state_exit_latency);
>  define_one_state_ro(power, show_state_power_usage);
>  define_one_state_ro(usage, show_state_usage);
> @@ -260,6 +262,7 @@ define_one_state_ro(time, show_state_time);
>  static struct attribute *cpuidle_state_default_attrs[] = {
>  	&attr_name.attr,
>  	&attr_desc.attr,
> +	&attr_abbr.attr,
>  	&attr_latency.attr,
>  	&attr_power.attr,
>  	&attr_usage.attr,
> diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
> index 60fa6ec..3bb1f2b 100644
> --- a/drivers/idle/intel_idle.c
> +++ b/drivers/idle/intel_idle.c
> @@ -91,6 +91,7 @@ static struct cpuidle_state nehalem_cstates[MWAIT_MAX_NUM_CSTATES] = {
>  	{ /* MWAIT C1 */
>  		.name = "NHM-C1",
>  		.desc = "MWAIT 0x00",
> +		.abbr = "C1",
>  		.driver_data = (void *) 0x00,
>  		.flags = CPUIDLE_FLAG_TIME_VALID,
>  		.exit_latency = 3,
> @@ -99,6 +100,7 @@ static struct cpuidle_state nehalem_cstates[MWAIT_MAX_NUM_CSTATES] = {
>  	{ /* MWAIT C2 */
>  		.name = "NHM-C3",
>  		.desc = "MWAIT 0x10",
> +		.abbr = "C3",
>  		.driver_data = (void *) 0x10,
>  		.flags = CPUIDLE_FLAG_TIME_VALID | CPUIDLE_FLAG_TLB_FLUSHED,
>  		.exit_latency = 20,
> @@ -107,6 +109,7 @@ static struct cpuidle_state nehalem_cstates[MWAIT_MAX_NUM_CSTATES] = {
>  	{ /* MWAIT C3 */
>  		.name = "NHM-C6",
>  		.desc = "MWAIT 0x20",
> +		.abbr = "C6",
>  		.driver_data = (void *) 0x20,
>  		.flags = CPUIDLE_FLAG_TIME_VALID | CPUIDLE_FLAG_TLB_FLUSHED,
>  		.exit_latency = 200,
> @@ -119,6 +122,7 @@ static struct cpuidle_state snb_cstates[MWAIT_MAX_NUM_CSTATES] = {
>  	{ /* MWAIT C1 */
>  		.name = "SNB-C1",
>  		.desc = "MWAIT 0x00",
> +		.abbr = "C1",
>  		.driver_data = (void *) 0x00,
>  		.flags = CPUIDLE_FLAG_TIME_VALID,
>  		.exit_latency = 1,
> @@ -127,6 +131,7 @@ static struct cpuidle_state snb_cstates[MWAIT_MAX_NUM_CSTATES] = {
>  	{ /* MWAIT C2 */
>  		.name = "SNB-C3",
>  		.desc = "MWAIT 0x10",
> +		.abbr = "C3",
>  		.driver_data = (void *) 0x10,
>  		.flags = CPUIDLE_FLAG_TIME_VALID | CPUIDLE_FLAG_TLB_FLUSHED,
>  		.exit_latency = 80,
> @@ -135,6 +140,7 @@ static struct cpuidle_state snb_cstates[MWAIT_MAX_NUM_CSTATES] = {
>  	{ /* MWAIT C3 */
>  		.name = "SNB-C6",
>  		.desc = "MWAIT 0x20",
> +		.abbr = "C6",
>  		.driver_data = (void *) 0x20,
>  		.flags = CPUIDLE_FLAG_TIME_VALID | CPUIDLE_FLAG_TLB_FLUSHED,
>  		.exit_latency = 104,
> @@ -143,6 +149,7 @@ static struct cpuidle_state snb_cstates[MWAIT_MAX_NUM_CSTATES] = {
>  	{ /* MWAIT C4 */
>  		.name = "SNB-C7",
>  		.desc = "MWAIT 0x30",
> +		.abbr = "C7",
>  		.driver_data = (void *) 0x30,
>  		.flags = CPUIDLE_FLAG_TIME_VALID | CPUIDLE_FLAG_TLB_FLUSHED,
>  		.exit_latency = 109,
> @@ -155,6 +162,7 @@ static struct cpuidle_state atom_cstates[MWAIT_MAX_NUM_CSTATES] = {
>  	{ /* MWAIT C1 */
>  		.name = "ATM-C1",
>  		.desc = "MWAIT 0x00",
> +		.abbr = "C1",
>  		.driver_data = (void *) 0x00,
>  		.flags = CPUIDLE_FLAG_TIME_VALID,
>  		.exit_latency = 1,
> @@ -163,6 +171,7 @@ static struct cpuidle_state atom_cstates[MWAIT_MAX_NUM_CSTATES] = {
>  	{ /* MWAIT C2 */
>  		.name = "ATM-C2",
>  		.desc = "MWAIT 0x10",
> +		.abbr = "C2",
>  		.driver_data = (void *) 0x10,
>  		.flags = CPUIDLE_FLAG_TIME_VALID,
>  		.exit_latency = 20,
> @@ -172,6 +181,7 @@ static struct cpuidle_state atom_cstates[MWAIT_MAX_NUM_CSTATES] = {
>  	{ /* MWAIT C4 */
>  		.name = "ATM-C4",
>  		.desc = "MWAIT 0x30",
> +		.abbr = "C4",
>  		.driver_data = (void *) 0x30,
>  		.flags = CPUIDLE_FLAG_TIME_VALID | CPUIDLE_FLAG_TLB_FLUSHED,
>  		.exit_latency = 100,
> @@ -181,6 +191,7 @@ static struct cpuidle_state atom_cstates[MWAIT_MAX_NUM_CSTATES] = {
>  	{ /* MWAIT C6 */
>  		.name = "ATM-C6",
>  		.desc = "MWAIT 0x52",
> +		.abbr = "C6",
>  		.driver_data = (void *) 0x52,
>  		.flags = CPUIDLE_FLAG_TIME_VALID | CPUIDLE_FLAG_TLB_FLUSHED,
>  		.exit_latency = 140,
> diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
> index 1be416b..4763ef3 100644
> --- a/include/linux/cpuidle.h
> +++ b/include/linux/cpuidle.h
> @@ -20,6 +20,7 @@
>  #define CPUIDLE_STATE_MAX	8
>  #define CPUIDLE_NAME_LEN	16
>  #define CPUIDLE_DESC_LEN	32
> +#define CPUIDLE_ABBR_LEN	3
>  
>  struct cpuidle_device;
>  
> @@ -31,6 +32,7 @@ struct cpuidle_device;
>  struct cpuidle_state {
>  	char		name[CPUIDLE_NAME_LEN];
>  	char		desc[CPUIDLE_DESC_LEN];
> +	char		abbr[CPUIDLE_ABBR_LEN];
>  	void		*driver_data;
>  
>  	unsigned int	flags;

Re: [PATCH 4/9] cpuidle: Introduce .abbr (abbrevation) for cpuidle states

[Len Brown]

I'm not fond of inventing a new 3-character abbreviation field
for every state because display tools can't handle the existing
16-character name field.

If the display tools can only handle 3 characters,
then why not have them simply use the 1st 3 characters
of the existing name field?  If that is not unique,
then re-arrange the strings so that it is unique...

Of course the ACPI part of this patch will not apply,
as it depends on patch 1 in this series, which was erroneous.
For ACPI, the existing name field is already fine,
as C%d fits into 3 characters.

thanks,
Len Brown, Intel Open Source Technology Center

Re: [PATCH 4/9] cpuidle: Introduce .abbr (abbrevation) for cpuidle states

[Thomas Renninger]

On Wednesday 12 January 2011 07:56:45 Len Brown wrote:
> I'm not fond of inventing a new 3-character abbreviation field
> for every state because display tools can't handle the existing
> 16-character name field.
I am also not "fond" of it, but it's a sane and easy
solution and appropriate for this issue.
 
> If the display tools can only handle 3 characters,
> then why not have them simply use the 1st 3 characters
> of the existing name field?
You mean use:
C3 NHM
instead of
NHM-C3
for intel_idle?

Then userspace has to parse the two informations glued
together, get rid of the whitespace, etc.
But by sysfs convention a separate file must be used
if two data are passed to userspace which is the case here.

> If that is not unique,
> then re-arrange the strings so that it is unique...
See above, it would unnecessarily make life hard for
userspace.
Afaik sysfs entries do not consume resources as long as
they do not get accessed.

> Of course the ACPI part of this patch will not apply,
> as it depends on patch 1 in this series, which was erroneous.
> For ACPI, the existing name field is already fine,
> as C%d fits into 3 characters.
For acpi_idle only it would work, but this is (nearly) the only
one.
For example for sh arch:
name: SuperH
"SH SuperH" contains two data and should get split up into:
name: SuperH
abbr: SH

By convention the first characters of "description" could be
used, but you would again end up in userspace parsing and
double info in one sysfs file.

I hope that's enough for convincing, I'd really like to go
the .abbr way. Do you give me an acked-by for that one?

Thanks,

    Thomas

Re: [PATCH 4/9] cpuidle: Introduce .abbr (abbrevation) for cpuidle states

[Len Brown]

> > If the display tools can only handle 3 characters,
> > then why not have them simply use the 1st 3 characters
> > of the existing name field?

> You mean use:
> C3 NHM
> instead of
> NHM-C3
> for intel_idle?

Yes.
is there a reason that would be a problem?

If only 3 characters are useful,
then I'd rather see you simly do this:

#define CPUIDLE_NAME_LEN 3

> Then userspace has to parse the two informations glued
> together, get rid of the whitespace, etc.

what "two informations"?
why is kernel bloat the instrument of choice
to avoid the expense of string truncation
in user-space tools?

> But by sysfs convention a separate file must be used
> if two data are passed to userspace which is the case here.

what two data?

It is fine for a string to include space characters.

> > If that is not unique,
> > then re-arrange the strings so that it is unique...

> See above, it would unnecessarily make life hard for
> userspace.
> Afaik sysfs entries do not consume resources as long as
> they do not get accessed.

they clutter the source code,
they create  an additional name for something
that arguably already has 3 names --
the state #, the name, and the description.
I'd rather delete a few of the fields than add a 4th...

> >Of course the ACPI part of this patch will not apply,
> > as it depends on patch 1 in this series, which was erroneous.
> > For ACPI, the existing name field is already fine,
> > as C%d fits into 3 characters.

> For acpi_idle only it would work, but this is (nearly) the only
> one.
> For example for sh arch:
> name: SuperH
> "SH SuperH" contains two data and should get split up into:
> name: SuperH
> abbr: SH

acpi/arch/sh/kernel/cpu/shmobile/cpuidle.c
already uses state->name = "C0", "C1", "C2".

Just use them and be done.

> By convention the first characters of "description" could be
> used, but you would again end up in userspace parsing and
> double info in one sysfs file.
> 
> I hope that's enough for convincing, I'd really like to go
> the .abbr way. Do you give me an acked-by for that one?

No.

We should not invent an additional abbreviation field.
The tools should use the existing name field.
If the existing name field is not sufficient, then
the states should simply be enumerated and numbers
be shown in the GUI.

thanks,
Len Brown, Intel Open Source Technology Center

Re: [PATCH 4/9] cpuidle: Introduce .abbr (abbrevation) for cpuidle states

[Thomas Renninger]

On Wednesday 12 January 2011 23:25:12 Len Brown wrote:
> > > If the display tools can only handle 3 characters,
> > > then why not have them simply use the 1st 3 characters
> > > of the existing name field?
> 
> > You mean use:
> > C3 NHM
> > instead of
> > NHM-C3
> > for intel_idle?
> 
> Yes.
> is there a reason that would be a problem?
A much more elegant solution came to my mind:
I'll rewrite the perf timechart patch to do:
Title:   C-stateName1 [1] , C-stateName2 [2]
and later I will refer to the states by 1, 2, ...
Eventually a:
Description:
[1] C-stateDescription1
[2] C-stateDescription2
...
can be put somewhere as well.

Please ignore this patch.

Stupid that I didn't think of that in the first place.

Thanks,

   Thomas

Re: [PATCH 4/9] cpuidle: Introduce .abbr (abbrevation) for cpuidle states

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 606 bytes --]

On Wed, 12 Jan 2011 17:25:12 EST, Len Brown said:

> > But by sysfs convention a separate file must be used
> > if two data are passed to userspace which is the case here.
> 
> what two data?
> 
> It is fine for a string to include space characters.

I think Thomas is concerned that although when you actually read a /sys
file, you know it's one string, that fact can get easily lost and cause issues
down the road. Consider this code:

foo=`cat /sys/some/file`
bar=`cat /sys/other/file`
baz=`cat /sys/third/file`
echo $foo $bar $baz | awk '{print $2 $3}'

Suddenly your output isn't what you expected...

[-- Attachment #2: Type: application/pgp-signature, Size: 227 bytes --]

[PATCH 8/9] perf timechart: Map power:cpu_idle events to the corresponding cpuidle state

[Thomas Renninger]

Before, power:cpu_idle events were very specific X86 Intel mwait events.
This got fixed with previous patches and cpu_idle events are now thrown by
all cpuidle drivers and can be mapped to the corresponding cpuidle state
in /sys.

This patch reads out the corresponding cpuidle name of a cpu_idle event
and uses it in the title line of the chart (c-states Cx in x86, omap2
- DDR self refresh states for various arm archs).

It also reads out the corresponding abbr(eviation) and uses the string
to draw the cpu idle occurences. This needs a short (3 letter) string
to keep the overview in the chart.

Signed-off-by: Thomas Renninger <trenn@suse.de>
CC: lenb@kernel.org
CC: linux-acpi@vger.kernel.org
CC: linux-pm@lists.linux-foundation.org
CC: Arjan van de Ven <arjan@linux.intel.com>
CC: Ingo Molnar <mingo@elte.hu>
CC: linux-kernel@vger.kernel.org
CC: linux-perf-users@vger.kernel.org
CC: linux-omap@vger.kernel.org
CC: Frederic Weisbecker <fweisbec@gmail.com>
---
 tools/perf/util/include/linux/cpuidle.h |   20 ++++
 tools/perf/util/svghelper.c             |  149 +++++++++++++++++++++++++++---
 2 files changed, 154 insertions(+), 15 deletions(-)
 create mode 100644 tools/perf/util/include/linux/cpuidle.h

diff --git a/tools/perf/util/include/linux/cpuidle.h b/tools/perf/util/include/linux/cpuidle.h
new file mode 100644
index 0000000..7012f33
--- /dev/null
+++ b/tools/perf/util/include/linux/cpuidle.h
@@ -0,0 +1,20 @@
+#ifndef __PERF_CPUIDLE_H_
+#define __PERF_CPUIDLE_H_
+
+/* This comes from include/linux/cpuidle.h kernel header **********/
+#define CPUIDLE_STATE_MAX	8
+#define CPUIDLE_NAME_LEN	16
+#define CPUIDLE_DESC_LEN	32
+#define CPUIDLE_ABBR_LEN	3
+/******************************************************************/
+
+struct cpuidle_state {
+	char name[CPUIDLE_NAME_LEN + 1];
+	char abbr[CPUIDLE_ABBR_LEN + 1];
+};
+
+extern struct cpuidle_state cpuidle_states[CPUIDLE_STATE_MAX];
+
+extern unsigned int cpuidle_info_max;
+
+#endif
diff --git a/tools/perf/util/svghelper.c b/tools/perf/util/svghelper.c
index 38b9e40..5cf6c9e 100644
--- a/tools/perf/util/svghelper.c
+++ b/tools/perf/util/svghelper.c
@@ -16,8 +16,13 @@
 #include <stdlib.h>
 #include <unistd.h>
 #include <string.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+
+#include <linux/cpuidle.h>
 
 #include "svghelper.h"
+#include "debug.h"
 
 static u64 first_time, last_time;
 static u64 turbo_frequency, max_freq;
@@ -108,12 +113,14 @@ void open_svg(const char *filename, int cpus, int rows, u64 start, u64 end)
 	fprintf(svgfile, "      rect.WAITING  { fill:rgb(255,214, 48); fill-opacity:0.6; stroke-width:0;   stroke:rgb(  0,  0,  0); } \n");
 	fprintf(svgfile, "      rect.cpu      { fill:rgb(192,192,192); fill-opacity:0.2; stroke-width:0.5; stroke:rgb(128,128,128); } \n");
 	fprintf(svgfile, "      rect.pstate   { fill:rgb(128,128,128); fill-opacity:0.8; stroke-width:0; } \n");
-	fprintf(svgfile, "      rect.c1       { fill:rgb(255,214,214); fill-opacity:0.5; stroke-width:0; } \n");
-	fprintf(svgfile, "      rect.c2       { fill:rgb(255,172,172); fill-opacity:0.5; stroke-width:0; } \n");
-	fprintf(svgfile, "      rect.c3       { fill:rgb(255,130,130); fill-opacity:0.5; stroke-width:0; } \n");
-	fprintf(svgfile, "      rect.c4       { fill:rgb(255, 88, 88); fill-opacity:0.5; stroke-width:0; } \n");
-	fprintf(svgfile, "      rect.c5       { fill:rgb(255, 44, 44); fill-opacity:0.5; stroke-width:0; } \n");
-	fprintf(svgfile, "      rect.c6       { fill:rgb(255,  0,  0); fill-opacity:0.5; stroke-width:0; } \n");
+	fprintf(svgfile, "      rect.c0       { fill:rgb(102,255,102); fill-opacity:0.5; stroke-width:0; } \n");
+	fprintf(svgfile, "      rect.c1       { fill:rgb(102,255,  0); fill-opacity:0.5; stroke-width:0; } \n");
+	fprintf(svgfile, "      rect.c2       { fill:rgb(  0,255,102); fill-opacity:0.5; stroke-width:0; } \n");
+	fprintf(svgfile, "      rect.c3       { fill:rgb( 51,255, 51); fill-opacity:0.5; stroke-width:0; } \n");
+	fprintf(svgfile, "      rect.c4       { fill:rgb( 51,255,  0); fill-opacity:0.5; stroke-width:0; } \n");
+	fprintf(svgfile, "      rect.c5       { fill:rgb(  0,255, 51); fill-opacity:0.5; stroke-width:0; } \n");
+	fprintf(svgfile, "      rect.c6       { fill:rgb(  0,204,  0); fill-opacity:0.5; stroke-width:0; } \n");
+	fprintf(svgfile, "      rect.c7       { fill:rgb(  0,153,  0); fill-opacity:0.5; stroke-width:0; } \n");
 	fprintf(svgfile, "      line.pstate   { stroke:rgb(255,255,  0); stroke-opacity:0.8; stroke-width:2; } \n");
 
 	fprintf(svgfile, "    ]]>\n   </style>\n</defs>\n");
@@ -200,6 +207,81 @@ void svg_waiting(int Yslot, u64 start, u64 end)
 	fprintf(svgfile, "</g>\n");
 }
 
+/* Cpuidle info from sysfs ***************************/
+struct cpuidle_state cpuidle_states[CPUIDLE_STATE_MAX];
+unsigned int cpuidle_info_max;
+
+static void debug_dump_cpuidle_states(void)
+{
+	unsigned int state;
+
+	if (cpuidle_info_max == 0) {
+		printf("No cpuidle info retrieved from /sys\n");
+		return;
+	}
+	printf("cpuidle_info_max: %u\n", cpuidle_info_max);
+	for (state = 0; state < cpuidle_info_max; state++) {
+		printf("CPUIDLE[%u]:\n", state);
+		printf("Name: %s\n", cpuidle_states[state].name);
+		printf("Abbr: %s\n", cpuidle_states[state].abbr);
+	}
+}
+static int get_sysfs_string(const char *path, char *string,
+			     int max_string_size)
+{
+	int fd;
+	size_t numread, i;
+
+	fd = open(path, O_RDONLY);
+	if (fd == -1)
+		return -1;
+
+	numread = read(fd, string, max_string_size-1);
+	if (numread < 1) {
+		close(fd);
+		return -1;
+	}
+	for (i = 0; i < numread; i++)
+		if (string[i] == '\n')
+			string[i] = '\0';
+	string[numread] = '\0';
+	close(fd);
+	return 0;
+}
+
+#define PERF_CPUIDLE_SYS_PATH "/sys/devices/system/cpu/cpu0/cpuidle/state%u/"
+#define PERF_SYSFS_PATH_MAX 255
+
+/*
+ * Fills up cpuidle_states[CPUIDLE_STATE_MAX] info from
+ * /sys/devices/system/cpu/cpu0/cpuidle/stateX/ and sets cpuidle_info_max
+ * to found states
+ */
+static int retrieve_cpuidle_info(void)
+{
+	char path[PERF_SYSFS_PATH_MAX];
+	int state, ret;
+
+	for (state = 0; state < CPUIDLE_STATE_MAX; state++) {
+		snprintf(path, sizeof(path), PERF_CPUIDLE_SYS_PATH "name",
+			 state);
+		ret = get_sysfs_string(path, cpuidle_states[state].name,
+				       CPUIDLE_NAME_LEN + 1);
+		if (ret)
+			break;
+
+		snprintf(path, sizeof(path), PERF_CPUIDLE_SYS_PATH "abbr",
+			 state);
+		ret = get_sysfs_string(path, cpuidle_states[state].abbr,
+				       CPUIDLE_ABBR_LEN + 1);
+		if (ret)
+			break;
+	}
+	cpuidle_info_max = state;
+	return state;
+}
+/* Cpuidle info from sysfs ***************************/
+
 static char *cpu_model(void)
 {
 	static char cpu_m[255];
@@ -279,17 +361,33 @@ void svg_process(int cpu, u64 start, u64 end, const char *type, const char *name
 	fprintf(svgfile, "</g>\n");
 }
 
+/*
+ * Svg util and kernel supported max cpuidle states may differ.
+ * Cmp. with tools/perf/utils/include/linux/cpuidle.h
+ * and include/linux/cpuidle.h
+ * Currently cpuidle kernel interface and this svg tool, both support 8 states
+ */
+#define PERF_SVG_CPUIDLE_STATE_MAX 8
+
 void svg_cstate(int cpu, u64 start, u64 end, int type)
 {
 	double width;
 	char style[128];
+	static bool max_states_exceed_msg;
 
 	if (!svgfile)
 		return;
 
+	if (type > PERF_SVG_CPUIDLE_STATE_MAX) {
+		if (verbose || max_states_exceed_msg == false) {
+			max_states_exceed_msg = true;
+			printf("cpuidle state (%d) exceeding max supported "
+			       "states (%d).. ignoring\n",
+			       type, PERF_SVG_CPUIDLE_STATE_MAX);
+			return;
+		}
+	}
 
-	if (type > 6)
-		type = 6;
 	sprintf(style, "c%i", type);
 
 	fprintf(svgfile, "<rect class=\"%s\" x=\"%4.8f\" width=\"%4.8f\" y=\"%4.1f\" height=\"%4.1f\"/>\n",
@@ -452,16 +550,37 @@ static void svg_legenda_box(int X, const char *text, const char *style)
 
 void svg_legenda(void)
 {
+	unsigned int cstate, offset = 500;
+	char class[3];
+
+	retrieve_cpuidle_info();
+	if (verbose)
+		debug_dump_cpuidle_states();
+
 	if (!svgfile)
 		return;
 
-	svg_legenda_box(0,	"Running", "sample");
-	svg_legenda_box(100,	"Idle","rect.c1");
-	svg_legenda_box(200,	"Deeper Idle", "rect.c3");
-	svg_legenda_box(350,	"Deepest Idle", "rect.c6");
-	svg_legenda_box(550,	"Sleeping", "process2");
-	svg_legenda_box(650,	"Waiting for cpu", "waiting");
-	svg_legenda_box(800,	"Blocked on IO", "blocked");
+		svg_legenda_box(0,	"Running", "sample");
+		svg_legenda_box(100,	"Sleeping", "process2");
+		svg_legenda_box(200,	"Waiting for cpu", "waiting");
+		svg_legenda_box(350,	"Blocked on IO", "blocked");
+	/* trenn: Arch specific events. Only C1 exists on x86 if
+	   no cpuidle driver registered. Deeper and Deepest can get
+	   removed. Also C1 events may only get fired through cpuidle
+	   driver at some time. */
+	if (cpuidle_info_max == 0) {
+		svg_legenda_box(500,	"Idle", "c1");
+	} else {
+		for (cstate = 0; cstate < cpuidle_info_max; cstate++) {
+			sprintf(class, "c%u", cstate);
+			svg_legenda_box(offset, cpuidle_states[cstate].name,
+					class);
+			/* The box */
+			offset += 20;
+			/* The text */
+			offset += (strlen(cpuidle_states[cstate].name) * 10);
+		}
+	}
 }
 
 void svg_time_grid(void)
-- 
1.7.3.1

Re: [PATCH 8/9] perf timechart: Map power:cpu_idle events to the corresponding cpuidle state

[Thomas Renninger]

Argh, forgot guilt refresh on this one.
The changelog could be a bit more detailed by adding:

On Friday 07 January 2011 11:29:49 Thomas Renninger wrote:
> Before, power:cpu_idle events were very specific X86 Intel mwait events.
> This got fixed with previous patches and cpu_idle events are now thrown by
> all cpuidle drivers and can be mapped to the corresponding cpuidle state
> in /sys.
> 
> This patch reads out the corresponding cpuidle name of a cpu_idle event
> and uses it in the title line of the chart (c-states Cx in x86, omap2
> - DDR self refresh states for various arm archs).
> 
> It also reads out the corresponding abbr(eviation) and uses the string
> to draw the cpu idle occurences. This needs a short (3 letter) string
> to keep the overview in the chart.

All features/fixes this patch includes:
  - Read up cpuidle events from sysfs if available
    and thus make them architecture independent
  - Use (free) green color to display idle events in the chart,
    red is also used by "Blocked IO" event(s)
  - Fix wrong class="rect.cX" to class="cX" idle box svg drawing
    definitions. This fixes up black idle drawings for eog (eye of gnome)
    and firefox (inkscape somehow could handle the broken case).

    Thomas