public inbox for linux-acpi@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ACPI, CPU_IDLE: Add more information about C-states
@ 2008-02-12  1:50 Venki Pallipadi
  2008-02-14  5:10 ` Len Brown
  0 siblings, 1 reply; 7+ messages in thread
From: Venki Pallipadi @ 2008-02-12  1:50 UTC (permalink / raw)
  To: Len Brown; +Cc: linux-acpi

From: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>

Add a new sysfs entry under cpuidle states. desc - can be used by driver to
communicate to userspace any specific information about the state.
This helps in identifying the exact hardware C-states behind the ACPI C-state
definition.

Idea is to export this through powertop, which will help to map the C-state
reported by powertop to actual hardware C-state.

Signed-off-by: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>

Index: linux-2.6.25-rc/arch/x86/kernel/acpi/cstate.c
===================================================================
--- linux-2.6.25-rc.orig/arch/x86/kernel/acpi/cstate.c
+++ linux-2.6.25-rc/arch/x86/kernel/acpi/cstate.c
@@ -126,6 +126,8 @@ int acpi_processor_ffh_cstate_probe(unsi
 		printk(KERN_DEBUG "Monitor-Mwait will be used to enter C-%d "
 		       "state\n", cx->type);
 	}
+	snprintf(cx->desc, ACPI_CX_DESC_LEN, "ACPI FFH INTEL MWAIT 0x%x",
+		 cx->address);
 
 out:
 	set_cpus_allowed(current, saved_mask);
Index: linux-2.6.25-rc/drivers/acpi/processor_idle.c
===================================================================
--- linux-2.6.25-rc.orig/drivers/acpi/processor_idle.c
+++ linux-2.6.25-rc/drivers/acpi/processor_idle.c
@@ -917,11 +917,16 @@ static int acpi_processor_get_power_info
 				 * Otherwise, ignore this info and continue.
 				 */
 				cx.entry_method = ACPI_CSTATE_HALT;
+				snprintf(cx.desc, ACPI_CX_DESC_LEN, "ACPI HLT");
 			} else {
 				continue;
 			}
+		} else {
+			snprintf(cx.desc, ACPI_CX_DESC_LEN, "ACPI IOPORT 0x%x",
+				 cx.address);
 		}
 
+
 		obj = &(element->package.elements[2]);
 		if (obj->type != ACPI_TYPE_INTEGER)
 			continue;
@@ -1620,6 +1625,11 @@ static int acpi_processor_setup_cpuidle(
 		return -EINVAL;
 	}
 
+	for (i = 0; i < CPUIDLE_STATE_MAX; i++) {
+		dev->states[i].name[0] = '\0';
+		dev->states[i].desc[0] = '\0';
+	}
+
 	for (i = 1; i < ACPI_PROCESSOR_MAX_POWER && i <= max_cstate; i++) {
 		cx = &pr->power.states[i];
 		state = &dev->states[count];
@@ -1636,6 +1646,7 @@ static int acpi_processor_setup_cpuidle(
 		cpuidle_set_statedata(state, cx);
 
 		snprintf(state->name, CPUIDLE_NAME_LEN, "C%d", i);
+		strncpy(state->desc, cx->desc, CPUIDLE_DESC_LEN);
 		state->exit_latency = cx->latency;
 		state->target_residency = cx->latency * 6;
 		state->power_usage = cx->power;
Index: linux-2.6.25-rc/drivers/cpuidle/sysfs.c
===================================================================
--- linux-2.6.25-rc.orig/drivers/cpuidle/sysfs.c
+++ linux-2.6.25-rc/drivers/cpuidle/sysfs.c
@@ -218,16 +218,23 @@ static ssize_t show_state_##_name(struct
 	return sprintf(buf, "%u\n", state->_name);\
 }
 
-static ssize_t show_state_name(struct cpuidle_state *state, char *buf)
-{
-	return sprintf(buf, "%s\n", state->name);
+#define define_show_state_str_function(_name) \
+static ssize_t show_state_##_name(struct cpuidle_state *state, char *buf) \
+{ \
+	if (state->_name[0] == '\0')\
+		return sprintf(buf, "<null>\n");\
+	return sprintf(buf, "%s\n", state->_name);\
 }
 
 define_show_state_function(exit_latency)
 define_show_state_function(power_usage)
 define_show_state_function(usage)
 define_show_state_function(time)
+define_show_state_str_function(name)
+define_show_state_str_function(desc)
+
 define_one_state_ro(name, show_state_name);
+define_one_state_ro(desc, show_state_desc);
 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);
@@ -235,6 +242,7 @@ define_one_state_ro(time, show_state_tim
 
 static struct attribute *cpuidle_state_default_attrs[] = {
 	&attr_name.attr,
+	&attr_desc.attr,
 	&attr_latency.attr,
 	&attr_power.attr,
 	&attr_usage.attr,
Index: linux-2.6.25-rc/include/acpi/processor.h
===================================================================
--- linux-2.6.25-rc.orig/include/acpi/processor.h
+++ linux-2.6.25-rc/include/acpi/processor.h
@@ -32,9 +32,11 @@
 #define DOMAIN_COORD_TYPE_SW_ANY	0xfd
 #define DOMAIN_COORD_TYPE_HW_ALL	0xfe
 
-#define ACPI_CSTATE_SYSTEMIO	(0)
-#define ACPI_CSTATE_FFH		(1)
-#define ACPI_CSTATE_HALT	(2)
+#define ACPI_CSTATE_SYSTEMIO	0
+#define ACPI_CSTATE_FFH		1
+#define ACPI_CSTATE_HALT	2
+
+#define ACPI_CX_DESC_LEN	32
 
 /* Power Management */
 
@@ -74,6 +76,7 @@ struct acpi_processor_cx {
 	u64 time;
 	struct acpi_processor_cx_policy promotion;
 	struct acpi_processor_cx_policy demotion;
+	char desc[ACPI_CX_DESC_LEN];
 };
 
 struct acpi_processor_power {
Index: linux-2.6.25-rc/include/linux/cpuidle.h
===================================================================
--- linux-2.6.25-rc.orig/include/linux/cpuidle.h
+++ linux-2.6.25-rc/include/linux/cpuidle.h
@@ -19,6 +19,7 @@
 
 #define CPUIDLE_STATE_MAX	8
 #define CPUIDLE_NAME_LEN	16
+#define CPUIDLE_DESC_LEN	32
 
 struct cpuidle_device;
 
@@ -29,6 +30,7 @@ struct cpuidle_device;
 
 struct cpuidle_state {
 	char		name[CPUIDLE_NAME_LEN];
+	char		desc[CPUIDLE_DESC_LEN];
 	void		*driver_data;
 
 	unsigned int	flags;
Index: linux-2.6.25-rc/drivers/cpuidle/cpuidle.c
===================================================================
--- linux-2.6.25-rc.orig/drivers/cpuidle/cpuidle.c
+++ linux-2.6.25-rc/drivers/cpuidle/cpuidle.c
@@ -219,7 +219,8 @@ static void poll_idle_init(struct cpuidl
 
 	cpuidle_set_statedata(state, NULL);
 
-	snprintf(state->name, CPUIDLE_NAME_LEN, "C0 (poll idle)");
+	snprintf(state->name, CPUIDLE_NAME_LEN, "C0");
+	snprintf(state->desc, CPUIDLE_DESC_LEN, "CPUIDLE CORE POLL IDLE");
 	state->exit_latency = 0;
 	state->target_residency = 0;
 	state->power_usage = -1;

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] ACPI, CPU_IDLE: Add more information about C-states
  2008-02-12  1:50 [PATCH] ACPI, CPU_IDLE: Add more information about C-states Venki Pallipadi
@ 2008-02-14  5:10 ` Len Brown
  2008-02-14  8:09   ` Len Brown
  0 siblings, 1 reply; 7+ messages in thread
From: Len Brown @ 2008-02-14  5:10 UTC (permalink / raw)
  To: Venki Pallipadi; +Cc: linux-acpi

applied

thanks,
-len

On Monday 11 February 2008 20:50, Venki Pallipadi wrote:
> From: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
> 
> Add a new sysfs entry under cpuidle states. desc - can be used by driver to
> communicate to userspace any specific information about the state.
> This helps in identifying the exact hardware C-states behind the ACPI C-state
> definition.
> 
> Idea is to export this through powertop, which will help to map the C-state
> reported by powertop to actual hardware C-state.
> 
> Signed-off-by: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
> 
> Index: linux-2.6.25-rc/arch/x86/kernel/acpi/cstate.c
> ===================================================================
> --- linux-2.6.25-rc.orig/arch/x86/kernel/acpi/cstate.c
> +++ linux-2.6.25-rc/arch/x86/kernel/acpi/cstate.c
> @@ -126,6 +126,8 @@ int acpi_processor_ffh_cstate_probe(unsi
>  		printk(KERN_DEBUG "Monitor-Mwait will be used to enter C-%d "
>  		       "state\n", cx->type);
>  	}
> +	snprintf(cx->desc, ACPI_CX_DESC_LEN, "ACPI FFH INTEL MWAIT 0x%x",
> +		 cx->address);
>  
>  out:
>  	set_cpus_allowed(current, saved_mask);
> Index: linux-2.6.25-rc/drivers/acpi/processor_idle.c
> ===================================================================
> --- linux-2.6.25-rc.orig/drivers/acpi/processor_idle.c
> +++ linux-2.6.25-rc/drivers/acpi/processor_idle.c
> @@ -917,11 +917,16 @@ static int acpi_processor_get_power_info
>  				 * Otherwise, ignore this info and continue.
>  				 */
>  				cx.entry_method = ACPI_CSTATE_HALT;
> +				snprintf(cx.desc, ACPI_CX_DESC_LEN, "ACPI HLT");
>  			} else {
>  				continue;
>  			}
> +		} else {
> +			snprintf(cx.desc, ACPI_CX_DESC_LEN, "ACPI IOPORT 0x%x",
> +				 cx.address);
>  		}
>  
> +
>  		obj = &(element->package.elements[2]);
>  		if (obj->type != ACPI_TYPE_INTEGER)
>  			continue;
> @@ -1620,6 +1625,11 @@ static int acpi_processor_setup_cpuidle(
>  		return -EINVAL;
>  	}
>  
> +	for (i = 0; i < CPUIDLE_STATE_MAX; i++) {
> +		dev->states[i].name[0] = '\0';
> +		dev->states[i].desc[0] = '\0';
> +	}
> +
>  	for (i = 1; i < ACPI_PROCESSOR_MAX_POWER && i <= max_cstate; i++) {
>  		cx = &pr->power.states[i];
>  		state = &dev->states[count];
> @@ -1636,6 +1646,7 @@ static int acpi_processor_setup_cpuidle(
>  		cpuidle_set_statedata(state, cx);
>  
>  		snprintf(state->name, CPUIDLE_NAME_LEN, "C%d", i);
> +		strncpy(state->desc, cx->desc, CPUIDLE_DESC_LEN);
>  		state->exit_latency = cx->latency;
>  		state->target_residency = cx->latency * 6;
>  		state->power_usage = cx->power;
> Index: linux-2.6.25-rc/drivers/cpuidle/sysfs.c
> ===================================================================
> --- linux-2.6.25-rc.orig/drivers/cpuidle/sysfs.c
> +++ linux-2.6.25-rc/drivers/cpuidle/sysfs.c
> @@ -218,16 +218,23 @@ static ssize_t show_state_##_name(struct
>  	return sprintf(buf, "%u\n", state->_name);\
>  }
>  
> -static ssize_t show_state_name(struct cpuidle_state *state, char *buf)
> -{
> -	return sprintf(buf, "%s\n", state->name);
> +#define define_show_state_str_function(_name) \
> +static ssize_t show_state_##_name(struct cpuidle_state *state, char *buf) \
> +{ \
> +	if (state->_name[0] == '\0')\
> +		return sprintf(buf, "<null>\n");\
> +	return sprintf(buf, "%s\n", state->_name);\
>  }
>  
>  define_show_state_function(exit_latency)
>  define_show_state_function(power_usage)
>  define_show_state_function(usage)
>  define_show_state_function(time)
> +define_show_state_str_function(name)
> +define_show_state_str_function(desc)
> +
>  define_one_state_ro(name, show_state_name);
> +define_one_state_ro(desc, show_state_desc);
>  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);
> @@ -235,6 +242,7 @@ define_one_state_ro(time, show_state_tim
>  
>  static struct attribute *cpuidle_state_default_attrs[] = {
>  	&attr_name.attr,
> +	&attr_desc.attr,
>  	&attr_latency.attr,
>  	&attr_power.attr,
>  	&attr_usage.attr,
> Index: linux-2.6.25-rc/include/acpi/processor.h
> ===================================================================
> --- linux-2.6.25-rc.orig/include/acpi/processor.h
> +++ linux-2.6.25-rc/include/acpi/processor.h
> @@ -32,9 +32,11 @@
>  #define DOMAIN_COORD_TYPE_SW_ANY	0xfd
>  #define DOMAIN_COORD_TYPE_HW_ALL	0xfe
>  
> -#define ACPI_CSTATE_SYSTEMIO	(0)
> -#define ACPI_CSTATE_FFH		(1)
> -#define ACPI_CSTATE_HALT	(2)
> +#define ACPI_CSTATE_SYSTEMIO	0
> +#define ACPI_CSTATE_FFH		1
> +#define ACPI_CSTATE_HALT	2
> +
> +#define ACPI_CX_DESC_LEN	32
>  
>  /* Power Management */
>  
> @@ -74,6 +76,7 @@ struct acpi_processor_cx {
>  	u64 time;
>  	struct acpi_processor_cx_policy promotion;
>  	struct acpi_processor_cx_policy demotion;
> +	char desc[ACPI_CX_DESC_LEN];
>  };
>  
>  struct acpi_processor_power {
> Index: linux-2.6.25-rc/include/linux/cpuidle.h
> ===================================================================
> --- linux-2.6.25-rc.orig/include/linux/cpuidle.h
> +++ linux-2.6.25-rc/include/linux/cpuidle.h
> @@ -19,6 +19,7 @@
>  
>  #define CPUIDLE_STATE_MAX	8
>  #define CPUIDLE_NAME_LEN	16
> +#define CPUIDLE_DESC_LEN	32
>  
>  struct cpuidle_device;
>  
> @@ -29,6 +30,7 @@ struct cpuidle_device;
>  
>  struct cpuidle_state {
>  	char		name[CPUIDLE_NAME_LEN];
> +	char		desc[CPUIDLE_DESC_LEN];
>  	void		*driver_data;
>  
>  	unsigned int	flags;
> Index: linux-2.6.25-rc/drivers/cpuidle/cpuidle.c
> ===================================================================
> --- linux-2.6.25-rc.orig/drivers/cpuidle/cpuidle.c
> +++ linux-2.6.25-rc/drivers/cpuidle/cpuidle.c
> @@ -219,7 +219,8 @@ static void poll_idle_init(struct cpuidl
>  
>  	cpuidle_set_statedata(state, NULL);
>  
> -	snprintf(state->name, CPUIDLE_NAME_LEN, "C0 (poll idle)");
> +	snprintf(state->name, CPUIDLE_NAME_LEN, "C0");
> +	snprintf(state->desc, CPUIDLE_DESC_LEN, "CPUIDLE CORE POLL IDLE");
>  	state->exit_latency = 0;
>  	state->target_residency = 0;
>  	state->power_usage = -1;
> -
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] ACPI, CPU_IDLE: Add more information about C-states
  2008-02-14  5:10 ` Len Brown
@ 2008-02-14  8:09   ` Len Brown
  2008-02-14 16:52     ` Pallipadi, Venkatesh
  0 siblings, 1 reply; 7+ messages in thread
From: Len Brown @ 2008-02-14  8:09 UTC (permalink / raw)
  To: Venki Pallipadi; +Cc: linux-acpi

I'm confused by state0 -- why are usage and time non-zero --
is this due to some residency there during initialization?

I don't know if I like the looks of 2^32 - 1 as the power number...

-Len

[root@t61 cpuidle]# cat /proc/acpi/processor/CPU0/power
active state:            C0
max_cstate:              C8
bus master activity:     00000000
maximum allowed latency: 2000000000 usec
states:
    C1:                  type[C1] promotion[--] demotion[--] latency[001] usage[00000000] duration[00000000000000000000]
    C2:                  type[C2] promotion[--] demotion[--] latency[001] usage[00000161] duration[00000000000000018522]
    C3:                  type[C3] promotion[--] demotion[--] latency[017] usage[00088238] duration[00000000002957934702]
[root@t61 cpuidle]# grep . */*
state0/desc:CPUIDLE CORE POLL IDLE
state0/latency:0
state0/name:C0
state0/power:4294967295
state0/time:11623
state0/usage:6
state1/desc:ACPI FFH INTEL MWAIT 0x0
state1/latency:1
state1/name:C1
state1/power:1000
state1/time:0
state1/usage:0
state2/desc:ACPI FFH INTEL MWAIT 0x10
state2/latency:1
state2/name:C2
state2/power:500
state2/time:5104
state2/usage:163
state3/desc:ACPI FFH INTEL MWAIT 0x20
state3/latency:17
state3/name:C3
state3/power:250
state3/time:830738297
state3/usage:89280

of course this isnt' new with this patch -- 2.6.25-rc1 looks as below:

[root@t61 cpuidle]# cat /proc/acpi/processor/CPU0/power
active state:            C0
max_cstate:              C8
bus master activity:     00000000
maximum allowed latency: 2000000000 usec
states:
    C1:                  type[C1] promotion[--] demotion[--] latency[001] usage[00000000] duration[00000000000000000000]
    C2:                  type[C2] promotion[--] demotion[--] latency[001] usage[00000202] duration[00000000000000028172]
    C3:                  type[C3] promotion[--] demotion[--] latency[017] usage[00026011] duration[00000000000264174466]
[root@t61 cpuidle]# grep . */*
state0/latency:0
state0/name:C0 (poll idle)
state0/power:4294967295
state0/time:35713
state0/usage:6
state1/latency:1
state1/name:C1
state1/power:1000
state1/time:0
state1/usage:0
state2/latency:1
state2/name:C2
state2/power:500
state2/time:7771
state2/usage:203
state3/latency:17
state3/name:C3
state3/power:250
state3/time:74950731
state3/usage:26286

and 2.6.24 showed no C0, as expected.

[root@t61 cpuidle]# cat /proc/acpi/processor/CPU0/power
active state:            C0
max_cstate:              C8
bus master activity:     00000000
maximum allowed latency: 2000 usec
states:
    C1:                  type[C1] promotion[--] demotion[--] latency[001] usage[00000030] duration[00000000000000000000]
    C2:                  type[C2] promotion[--] demotion[--] latency[001] usage[00000714] duration[00000000000000531448]
    C3:                  type[C3] promotion[--] demotion[--] latency[017] usage[00020563] duration[00000000000168147357]
[root@t61 cpuidle]# grep . */*
state0/latency:1
state0/name:C1
state0/power:1000
state0/time:0
state0/usage:30
state1/latency:1
state1/name:C2
state1/power:500
state1/time:148317
state1/usage:719
state2/latency:17
state2/name:C3
state2/power:250
state2/time:48238649
state2/usage:21034
[root@t61 cpuidle]#                        

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH] ACPI, CPU_IDLE: Add more information about C-states
  2008-02-14  8:09   ` Len Brown
@ 2008-02-14 16:52     ` Pallipadi, Venkatesh
  2008-02-14 20:41       ` Len Brown
  0 siblings, 1 reply; 7+ messages in thread
From: Pallipadi, Venkatesh @ 2008-02-14 16:52 UTC (permalink / raw)
  To: Len Brown; +Cc: linux-acpi


'usage' is a counter that counts every time we get into poll_idle
routine. I also try to keep track of time spent in poll_idle, until
need_resched sends us out. That time is accumulated in 'time'.
Just have those to be consistent with other state and to be sure that
powertop kind of tools wont get confused.

I agree with the 'power' number. I did not like that 4294967295 there.
But, I had to put some number higher than other C-states, maximum
possible seemed logical. May be I can change the sysfs interface to
print <NULL> or something like that for power?

Thanks,
Venki

>-----Original Message-----
>From: Len Brown [mailto:lenb@kernel.org] 
>Sent: Thursday, February 14, 2008 12:09 AM
>To: Pallipadi, Venkatesh
>Cc: linux-acpi@vger.kernel.org
>Subject: Re: [PATCH] ACPI, CPU_IDLE: Add more information 
>about C-states
>
>I'm confused by state0 -- why are usage and time non-zero --
>is this due to some residency there during initialization?
>
>I don't know if I like the looks of 2^32 - 1 as the power number...
>
>-Len
>
>[root@t61 cpuidle]# cat /proc/acpi/processor/CPU0/power
>active state:            C0
>max_cstate:              C8
>bus master activity:     00000000
>maximum allowed latency: 2000000000 usec
>states:
>    C1:                  type[C1] promotion[--] demotion[--] 
>latency[001] usage[00000000] duration[00000000000000000000]
>    C2:                  type[C2] promotion[--] demotion[--] 
>latency[001] usage[00000161] duration[00000000000000018522]
>    C3:                  type[C3] promotion[--] demotion[--] 
>latency[017] usage[00088238] duration[00000000002957934702]
>[root@t61 cpuidle]# grep . */*
>state0/desc:CPUIDLE CORE POLL IDLE
>state0/latency:0
>state0/name:C0
>state0/power:4294967295
>state0/time:11623
>state0/usage:6
>state1/desc:ACPI FFH INTEL MWAIT 0x0
>state1/latency:1
>state1/name:C1
>state1/power:1000
>state1/time:0
>state1/usage:0
>state2/desc:ACPI FFH INTEL MWAIT 0x10
>state2/latency:1
>state2/name:C2
>state2/power:500
>state2/time:5104
>state2/usage:163
>state3/desc:ACPI FFH INTEL MWAIT 0x20
>state3/latency:17
>state3/name:C3
>state3/power:250
>state3/time:830738297
>state3/usage:89280
>
>of course this isnt' new with this patch -- 2.6.25-rc1 looks as below:
>
>[root@t61 cpuidle]# cat /proc/acpi/processor/CPU0/power
>active state:            C0
>max_cstate:              C8
>bus master activity:     00000000
>maximum allowed latency: 2000000000 usec
>states:
>    C1:                  type[C1] promotion[--] demotion[--] 
>latency[001] usage[00000000] duration[00000000000000000000]
>    C2:                  type[C2] promotion[--] demotion[--] 
>latency[001] usage[00000202] duration[00000000000000028172]
>    C3:                  type[C3] promotion[--] demotion[--] 
>latency[017] usage[00026011] duration[00000000000264174466]
>[root@t61 cpuidle]# grep . */*
>state0/latency:0
>state0/name:C0 (poll idle)
>state0/power:4294967295
>state0/time:35713
>state0/usage:6
>state1/latency:1
>state1/name:C1
>state1/power:1000
>state1/time:0
>state1/usage:0
>state2/latency:1
>state2/name:C2
>state2/power:500
>state2/time:7771
>state2/usage:203
>state3/latency:17
>state3/name:C3
>state3/power:250
>state3/time:74950731
>state3/usage:26286
>
>and 2.6.24 showed no C0, as expected.
>
>[root@t61 cpuidle]# cat /proc/acpi/processor/CPU0/power
>active state:            C0
>max_cstate:              C8
>bus master activity:     00000000
>maximum allowed latency: 2000 usec
>states:
>    C1:                  type[C1] promotion[--] demotion[--] 
>latency[001] usage[00000030] duration[00000000000000000000]
>    C2:                  type[C2] promotion[--] demotion[--] 
>latency[001] usage[00000714] duration[00000000000000531448]
>    C3:                  type[C3] promotion[--] demotion[--] 
>latency[017] usage[00020563] duration[00000000000168147357]
>[root@t61 cpuidle]# grep . */*
>state0/latency:1
>state0/name:C1
>state0/power:1000
>state0/time:0
>state0/usage:30
>state1/latency:1
>state1/name:C2
>state1/power:500
>state1/time:148317
>state1/usage:719
>state2/latency:17
>state2/name:C3
>state2/power:250
>state2/time:48238649
>state2/usage:21034
>[root@t61 cpuidle]#                        
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] ACPI, CPU_IDLE: Add more information about C-states
  2008-02-14 16:52     ` Pallipadi, Venkatesh
@ 2008-02-14 20:41       ` Len Brown
  2008-02-15  0:42         ` Pallipadi, Venkatesh
  0 siblings, 1 reply; 7+ messages in thread
From: Len Brown @ 2008-02-14 20:41 UTC (permalink / raw)
  To: Pallipadi, Venkatesh; +Cc: linux-acpi

On Thursday 14 February 2008 11:52, Pallipadi, Venkatesh wrote:
> 
> 'usage' is a counter that counts every time we get into poll_idle
> routine. I also try to keep track of time spent in poll_idle, until
> need_resched sends us out. That time is accumulated in 'time'.
> Just have those to be consistent with other state and to be sure that
> powertop kind of tools wont get confused.

yes, the consistency is good.
But my question is when and why we enter poll_idle at all.

> I agree with the 'power' number. I did not like that 4294967295 there.
> But, I had to put some number higher than other C-states, maximum
> possible seemed logical. May be I can change the sysfs interface to
> print <NULL> or something like that for power?

I dunno.  Somehow I like 0 to signify an unknown quantity
rather than a huge number.  If somebody actually looks at it
with a program,  they're going to have to special case either of them,
and 0 seems simpler.

thanks,
-Len


> >
> >I'm confused by state0 -- why are usage and time non-zero --
> >is this due to some residency there during initialization?
> >
> >I don't know if I like the looks of 2^32 - 1 as the power number...
> >
> >-Len
> >
> >[root@t61 cpuidle]# cat /proc/acpi/processor/CPU0/power
> >active state:            C0
> >max_cstate:              C8
> >bus master activity:     00000000
> >maximum allowed latency: 2000000000 usec
> >states:
> >    C1:                  type[C1] promotion[--] demotion[--] 
> >latency[001] usage[00000000] duration[00000000000000000000]
> >    C2:                  type[C2] promotion[--] demotion[--] 
> >latency[001] usage[00000161] duration[00000000000000018522]
> >    C3:                  type[C3] promotion[--] demotion[--] 
> >latency[017] usage[00088238] duration[00000000002957934702]
> >[root@t61 cpuidle]# grep . */*
> >state0/desc:CPUIDLE CORE POLL IDLE
> >state0/latency:0
> >state0/name:C0
> >state0/power:4294967295
> >state0/time:11623
> >state0/usage:6
> >state1/desc:ACPI FFH INTEL MWAIT 0x0
> >state1/latency:1
> >state1/name:C1
> >state1/power:1000
> >state1/time:0
> >state1/usage:0
> >state2/desc:ACPI FFH INTEL MWAIT 0x10
> >state2/latency:1
> >state2/name:C2
> >state2/power:500
> >state2/time:5104
> >state2/usage:163
> >state3/desc:ACPI FFH INTEL MWAIT 0x20
> >state3/latency:17
> >state3/name:C3
> >state3/power:250
> >state3/time:830738297
> >state3/usage:89280
> >
> >of course this isnt' new with this patch -- 2.6.25-rc1 looks as below:
> >
> >[root@t61 cpuidle]# cat /proc/acpi/processor/CPU0/power
> >active state:            C0
> >max_cstate:              C8
> >bus master activity:     00000000
> >maximum allowed latency: 2000000000 usec
> >states:
> >    C1:                  type[C1] promotion[--] demotion[--] 
> >latency[001] usage[00000000] duration[00000000000000000000]
> >    C2:                  type[C2] promotion[--] demotion[--] 
> >latency[001] usage[00000202] duration[00000000000000028172]
> >    C3:                  type[C3] promotion[--] demotion[--] 
> >latency[017] usage[00026011] duration[00000000000264174466]
> >[root@t61 cpuidle]# grep . */*
> >state0/latency:0
> >state0/name:C0 (poll idle)
> >state0/power:4294967295
> >state0/time:35713
> >state0/usage:6
> >state1/latency:1
> >state1/name:C1
> >state1/power:1000
> >state1/time:0
> >state1/usage:0
> >state2/latency:1
> >state2/name:C2
> >state2/power:500
> >state2/time:7771
> >state2/usage:203
> >state3/latency:17
> >state3/name:C3
> >state3/power:250
> >state3/time:74950731
> >state3/usage:26286
> >
> >and 2.6.24 showed no C0, as expected.
> >
> >[root@t61 cpuidle]# cat /proc/acpi/processor/CPU0/power
> >active state:            C0
> >max_cstate:              C8
> >bus master activity:     00000000
> >maximum allowed latency: 2000 usec
> >states:
> >    C1:                  type[C1] promotion[--] demotion[--] 
> >latency[001] usage[00000030] duration[00000000000000000000]
> >    C2:                  type[C2] promotion[--] demotion[--] 
> >latency[001] usage[00000714] duration[00000000000000531448]
> >    C3:                  type[C3] promotion[--] demotion[--] 
> >latency[017] usage[00020563] duration[00000000000168147357]
> >[root@t61 cpuidle]# grep . */*
> >state0/latency:1
> >state0/name:C1
> >state0/power:1000
> >state0/time:0
> >state0/usage:30
> >state1/latency:1
> >state1/name:C2
> >state1/power:500
> >state1/time:148317
> >state1/usage:719
> >state2/latency:17
> >state2/name:C3
> >state2/power:250
> >state2/time:48238649
> >state2/usage:21034
> >[root@t61 cpuidle]#                        
> >
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH] ACPI, CPU_IDLE: Add more information about C-states
  2008-02-14 20:41       ` Len Brown
@ 2008-02-15  0:42         ` Pallipadi, Venkatesh
  2008-02-15  4:52           ` Len Brown
  0 siblings, 1 reply; 7+ messages in thread
From: Pallipadi, Venkatesh @ 2008-02-15  0:42 UTC (permalink / raw)
  To: Len Brown; +Cc: linux-acpi

 

>-----Original Message-----
>From: Len Brown [mailto:lenb@kernel.org] 
>Sent: Thursday, February 14, 2008 12:41 PM
>To: Pallipadi, Venkatesh
>Cc: linux-acpi@vger.kernel.org
>Subject: Re: [PATCH] ACPI, CPU_IDLE: Add more information 
>about C-states
>
>On Thursday 14 February 2008 11:52, Pallipadi, Venkatesh wrote:
>> 
>> 'usage' is a counter that counts every time we get into poll_idle
>> routine. I also try to keep track of time spent in poll_idle, until
>> need_resched sends us out. That time is accumulated in 'time'.
>> Just have those to be consistent with other state and to be sure that
>> powertop kind of tools wont get confused.
>
>yes, the consistency is good.
>But my question is when and why we enter poll_idle at all.

Ohh. Ok. May be the ladder governor used it for a while, before we
switch to menu governor. It should not increase later once system is
fully up? Do you see it increasing later?

>
>> I agree with the 'power' number. I did not like that 
>4294967295 there.
>> But, I had to put some number higher than other C-states, maximum
>> possible seemed logical. May be I can change the sysfs interface to
>> print <NULL> or something like that for power?
>
>I dunno.  Somehow I like 0 to signify an unknown quantity
>rather than a huge number.  If somebody actually looks at it
>with a program,  they're going to have to special case either of them,
>and 0 seems simpler.
>

Yes. But, if we have a 0W Cn state in future? :-).

Thanks,
Venki

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] ACPI, CPU_IDLE: Add more information about C-states
  2008-02-15  0:42         ` Pallipadi, Venkatesh
@ 2008-02-15  4:52           ` Len Brown
  0 siblings, 0 replies; 7+ messages in thread
From: Len Brown @ 2008-02-15  4:52 UTC (permalink / raw)
  To: Pallipadi, Venkatesh; +Cc: linux-acpi

On Thursday 14 February 2008 19:42, Pallipadi, Venkatesh wrote:
> 
> >-----Original Message-----
> >From: Len Brown [mailto:lenb@kernel.org] 
> >Sent: Thursday, February 14, 2008 12:41 PM
> >To: Pallipadi, Venkatesh
> >Cc: linux-acpi@vger.kernel.org
> >Subject: Re: [PATCH] ACPI, CPU_IDLE: Add more information 
> >about C-states
> >
> >On Thursday 14 February 2008 11:52, Pallipadi, Venkatesh wrote:
> >> 
> >> 'usage' is a counter that counts every time we get into poll_idle
> >> routine. I also try to keep track of time spent in poll_idle, until
> >> need_resched sends us out. That time is accumulated in 'time'.
> >> Just have those to be consistent with other state and to be sure that
> >> powertop kind of tools wont get confused.
> >
> >yes, the consistency is good.
> >But my question is when and why we enter poll_idle at all.
> 
> Ohh. Ok. May be the ladder governor used it for a while, before we
> switch to menu governor. It should not increase later once system is
> fully up? Do you see it increasing later?

i haven't noticed it increase, but will look.

> >
> >> I agree with the 'power' number. I did not like that 
> >4294967295 there.
> >> But, I had to put some number higher than other C-states, maximum
> >> possible seemed logical. May be I can change the sysfs interface to
> >> print <NULL> or something like that for power?
> >
> >I dunno.  Somehow I like 0 to signify an unknown quantity
> >rather than a huge number.  If somebody actually looks at it
> >with a program,  they're going to have to special case either of them,
> >and 0 seems simpler.
> >
> 
> Yes. But, if we have a 0W Cn state in future? :-).

actually, the entire field is really worthless.
we all know that BIOS writers just take the example code
and use it multiple systems w/o changing these values.
Perhaps we should re-cosider exposing this field,
as it is mis-leading, at best...

-len

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2008-02-15  4:53 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-02-12  1:50 [PATCH] ACPI, CPU_IDLE: Add more information about C-states Venki Pallipadi
2008-02-14  5:10 ` Len Brown
2008-02-14  8:09   ` Len Brown
2008-02-14 16:52     ` Pallipadi, Venkatesh
2008-02-14 20:41       ` Len Brown
2008-02-15  0:42         ` Pallipadi, Venkatesh
2008-02-15  4:52           ` Len Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox