All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V4] cpuidle/powernv: Read target_residency value of idle states from DT if available
@ 2015-02-03  6:20 Preeti U Murthy
  2015-02-17  7:59 ` Preeti U Murthy
  0 siblings, 1 reply; 10+ messages in thread
From: Preeti U Murthy @ 2015-02-03  6:20 UTC (permalink / raw)
  To: mpe; +Cc: stewart, rafael.j.wysocki, linuxppc-dev, linux-kernel, linux-pm

The device tree now exposes the residency values for different idle states. Read
these values instead of calculating residency from the latency values. The values
exposed in the DT are validated for optimal power efficiency. However to maintain
compatibility with the older firmware code which does not expose residency
values, use default values as a fallback mechanism. While at it, use better
APIs to parse the powermgmt device tree node.

Signed-off-by: Preeti U Murthy <preeti@linux.vnet.ibm.com>
Acked-by: Stewart Smith <stewart@linux.vnet.ibm.com>
Acked-by: Michael Ellerman <mpe@ellerman.id.au>
---
Changes from the previous versions: https://lkml.org/lkml/2015/2/2/12
1. Used better APIs
2. Code Cleanups
3. Reverted back to the V2 of this patch as suggested in the above link,
in addition to a few cleanups and carried forward the Acks from V2

 drivers/cpuidle/cpuidle-powernv.c |   71 ++++++++++++++++++++++---------------
 1 file changed, 42 insertions(+), 29 deletions(-)

diff --git a/drivers/cpuidle/cpuidle-powernv.c b/drivers/cpuidle/cpuidle-powernv.c
index 2726663..3aa27a0 100644
--- a/drivers/cpuidle/cpuidle-powernv.c
+++ b/drivers/cpuidle/cpuidle-powernv.c
@@ -13,6 +13,7 @@
 #include <linux/notifier.h>
 #include <linux/clockchips.h>
 #include <linux/of.h>
+#include <linux/slab.h>
 
 #include <asm/machdep.h>
 #include <asm/firmware.h>
@@ -161,69 +162,81 @@ static int powernv_add_idle_states(void)
 	int nr_idle_states = 1; /* Snooze */
 	int dt_idle_states;
 	const __be32 *idle_state_flags;
-	const __be32 *idle_state_latency;
-	u32 len_flags, flags, latency_ns;
-	int i;
+	u32 len_flags, flags;
+	u32 *latency_ns, *residency_ns;
+	int i, rc;
 
 	/* Currently we have snooze statically defined */
 
 	power_mgt = of_find_node_by_path("/ibm,opal/power-mgt");
 	if (!power_mgt) {
 		pr_warn("opal: PowerMgmt Node not found\n");
-		return nr_idle_states;
+		goto out;
 	}
 
-	idle_state_flags = of_get_property(power_mgt, "ibm,cpu-idle-state-flags", &len_flags);
+	idle_state_flags = of_get_property(power_mgt,
+				"ibm,cpu-idle-state-flags", &len_flags);
 	if (!idle_state_flags) {
-		pr_warn("DT-PowerMgmt: missing ibm,cpu-idle-state-flags\n");
-		return nr_idle_states;
+		pr_warn("cpuidle-powernv : missing
+				ibm,cpu-idle-state-flags in DT\n");
+		goto out;
 	}
 
-	idle_state_latency = of_get_property(power_mgt,
-			"ibm,cpu-idle-state-latencies-ns", NULL);
-	if (!idle_state_latency) {
-		pr_warn("DT-PowerMgmt: missing ibm,cpu-idle-state-latencies-ns\n");
-		return nr_idle_states;
+	dt_idle_states = len_flags / sizeof(u32);
+
+	latency_ns = kzalloc(sizeof(*latency_ns) * dt_idle_states, GFP_KERNEL);
+	rc = of_property_read_u32_array(power_mgt,
+		"ibm,cpu-idle-state-latencies-ns", latency_ns, dt_idle_states);
+	if (rc) {
+		pr_warn("cpuidle-powernv: missing
+				ibm,cpu-idle-state-latencies-ns in DT\n");
+		goto out_free_latency;
 	}
 
-	dt_idle_states = len_flags / sizeof(u32);
+	residency_ns = kzalloc(sizeof(*residency_ns) * dt_idle_states, GFP_KERNEL);
+	rc = of_property_read_u32_array(power_mgt,
+		"ibm,cpu-idle-state-residency-ns", residency_ns, dt_idle_states);
 
 	for (i = 0; i < dt_idle_states; i++) {
 
 		flags = be32_to_cpu(idle_state_flags[i]);
 
-		/* Cpuidle accepts exit_latency in us and we estimate
-		 * target residency to be 10x exit_latency
+		/*
+		 * Cpuidle accepts exit_latency and target_residency in us.
+		 * Use default target_residency values if f/w does not expose it.
 		 */
-		latency_ns = be32_to_cpu(idle_state_latency[i]);
 		if (flags & OPAL_PM_NAP_ENABLED) {
 			/* Add NAP state */
 			strcpy(powernv_states[nr_idle_states].name, "Nap");
 			strcpy(powernv_states[nr_idle_states].desc, "Nap");
 			powernv_states[nr_idle_states].flags = 0;
-			powernv_states[nr_idle_states].exit_latency =
-					((unsigned int)latency_ns) / 1000;
-			powernv_states[nr_idle_states].target_residency =
-					((unsigned int)latency_ns / 100);
+			powernv_states[nr_idle_states].target_residency = 100;
 			powernv_states[nr_idle_states].enter = &nap_loop;
-			nr_idle_states++;
-		}
-
-		if (flags & OPAL_PM_SLEEP_ENABLED ||
+		} else if (flags & OPAL_PM_SLEEP_ENABLED ||
 			flags & OPAL_PM_SLEEP_ENABLED_ER1) {
 			/* Add FASTSLEEP state */
 			strcpy(powernv_states[nr_idle_states].name, "FastSleep");
 			strcpy(powernv_states[nr_idle_states].desc, "FastSleep");
 			powernv_states[nr_idle_states].flags = CPUIDLE_FLAG_TIMER_STOP;
-			powernv_states[nr_idle_states].exit_latency =
-					((unsigned int)latency_ns) / 1000;
-			powernv_states[nr_idle_states].target_residency =
-					((unsigned int)latency_ns / 100);
+			powernv_states[nr_idle_states].target_residency = 300000;
 			powernv_states[nr_idle_states].enter = &fastsleep_loop;
-			nr_idle_states++;
 		}
+
+		powernv_states[nr_idle_states].exit_latency =
+				((unsigned int)latency_ns[i]) / 1000;
+
+		if (!rc) {
+			powernv_states[nr_idle_states].target_residency =
+				((unsigned int)residency_ns[i]) / 1000;
+		}
+
+		nr_idle_states++;
 	}
 
+	kfree(residency_ns);
+out_free_latency:
+	kfree(latency_ns);
+out:
 	return nr_idle_states;
 }
 


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

* Re: [PATCH V4] cpuidle/powernv: Read target_residency value of idle states from DT if available
  2015-02-03  6:20 [PATCH V4] cpuidle/powernv: Read target_residency value of idle states from DT if available Preeti U Murthy
@ 2015-02-17  7:59 ` Preeti U Murthy
  2015-02-17 17:53     ` Rafael J. Wysocki
  0 siblings, 1 reply; 10+ messages in thread
From: Preeti U Murthy @ 2015-02-17  7:59 UTC (permalink / raw)
  To: mpe, rafael.j.wysocki; +Cc: stewart, linuxppc-dev, linux-kernel, linux-pm

Hi Rafael,

Can you please pull this patch ?
Another patch based on top of this is waiting to be pulled:
[PATCH] driver/cpuidle-powernv: Avoid endianness conversions while parsing DT

Regards
Preeti U Murthy

On 02/03/2015 11:50 AM, Preeti U Murthy wrote:
> The device tree now exposes the residency values for different idle states. Read
> these values instead of calculating residency from the latency values. The values
> exposed in the DT are validated for optimal power efficiency. However to maintain
> compatibility with the older firmware code which does not expose residency
> values, use default values as a fallback mechanism. While at it, use better
> APIs to parse the powermgmt device tree node.
> 
> Signed-off-by: Preeti U Murthy <preeti@linux.vnet.ibm.com>
> Acked-by: Stewart Smith <stewart@linux.vnet.ibm.com>
> Acked-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
> Changes from the previous versions: https://lkml.org/lkml/2015/2/2/12
> 1. Used better APIs
> 2. Code Cleanups
> 3. Reverted back to the V2 of this patch as suggested in the above link,
> in addition to a few cleanups and carried forward the Acks from V2
> 
>  drivers/cpuidle/cpuidle-powernv.c |   71 ++++++++++++++++++++++---------------
>  1 file changed, 42 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/cpuidle/cpuidle-powernv.c b/drivers/cpuidle/cpuidle-powernv.c
> index 2726663..3aa27a0 100644
> --- a/drivers/cpuidle/cpuidle-powernv.c
> +++ b/drivers/cpuidle/cpuidle-powernv.c
> @@ -13,6 +13,7 @@
>  #include <linux/notifier.h>
>  #include <linux/clockchips.h>
>  #include <linux/of.h>
> +#include <linux/slab.h>
>  
>  #include <asm/machdep.h>
>  #include <asm/firmware.h>
> @@ -161,69 +162,81 @@ static int powernv_add_idle_states(void)
>  	int nr_idle_states = 1; /* Snooze */
>  	int dt_idle_states;
>  	const __be32 *idle_state_flags;
> -	const __be32 *idle_state_latency;
> -	u32 len_flags, flags, latency_ns;
> -	int i;
> +	u32 len_flags, flags;
> +	u32 *latency_ns, *residency_ns;
> +	int i, rc;
>  
>  	/* Currently we have snooze statically defined */
>  
>  	power_mgt = of_find_node_by_path("/ibm,opal/power-mgt");
>  	if (!power_mgt) {
>  		pr_warn("opal: PowerMgmt Node not found\n");
> -		return nr_idle_states;
> +		goto out;
>  	}
>  
> -	idle_state_flags = of_get_property(power_mgt, "ibm,cpu-idle-state-flags", &len_flags);
> +	idle_state_flags = of_get_property(power_mgt,
> +				"ibm,cpu-idle-state-flags", &len_flags);
>  	if (!idle_state_flags) {
> -		pr_warn("DT-PowerMgmt: missing ibm,cpu-idle-state-flags\n");
> -		return nr_idle_states;
> +		pr_warn("cpuidle-powernv : missing
> +				ibm,cpu-idle-state-flags in DT\n");
> +		goto out;
>  	}
>  
> -	idle_state_latency = of_get_property(power_mgt,
> -			"ibm,cpu-idle-state-latencies-ns", NULL);
> -	if (!idle_state_latency) {
> -		pr_warn("DT-PowerMgmt: missing ibm,cpu-idle-state-latencies-ns\n");
> -		return nr_idle_states;
> +	dt_idle_states = len_flags / sizeof(u32);
> +
> +	latency_ns = kzalloc(sizeof(*latency_ns) * dt_idle_states, GFP_KERNEL);
> +	rc = of_property_read_u32_array(power_mgt,
> +		"ibm,cpu-idle-state-latencies-ns", latency_ns, dt_idle_states);
> +	if (rc) {
> +		pr_warn("cpuidle-powernv: missing
> +				ibm,cpu-idle-state-latencies-ns in DT\n");
> +		goto out_free_latency;
>  	}
>  
> -	dt_idle_states = len_flags / sizeof(u32);
> +	residency_ns = kzalloc(sizeof(*residency_ns) * dt_idle_states, GFP_KERNEL);
> +	rc = of_property_read_u32_array(power_mgt,
> +		"ibm,cpu-idle-state-residency-ns", residency_ns, dt_idle_states);
>  
>  	for (i = 0; i < dt_idle_states; i++) {
>  
>  		flags = be32_to_cpu(idle_state_flags[i]);
>  
> -		/* Cpuidle accepts exit_latency in us and we estimate
> -		 * target residency to be 10x exit_latency
> +		/*
> +		 * Cpuidle accepts exit_latency and target_residency in us.
> +		 * Use default target_residency values if f/w does not expose it.
>  		 */
> -		latency_ns = be32_to_cpu(idle_state_latency[i]);
>  		if (flags & OPAL_PM_NAP_ENABLED) {
>  			/* Add NAP state */
>  			strcpy(powernv_states[nr_idle_states].name, "Nap");
>  			strcpy(powernv_states[nr_idle_states].desc, "Nap");
>  			powernv_states[nr_idle_states].flags = 0;
> -			powernv_states[nr_idle_states].exit_latency =
> -					((unsigned int)latency_ns) / 1000;
> -			powernv_states[nr_idle_states].target_residency =
> -					((unsigned int)latency_ns / 100);
> +			powernv_states[nr_idle_states].target_residency = 100;
>  			powernv_states[nr_idle_states].enter = &nap_loop;
> -			nr_idle_states++;
> -		}
> -
> -		if (flags & OPAL_PM_SLEEP_ENABLED ||
> +		} else if (flags & OPAL_PM_SLEEP_ENABLED ||
>  			flags & OPAL_PM_SLEEP_ENABLED_ER1) {
>  			/* Add FASTSLEEP state */
>  			strcpy(powernv_states[nr_idle_states].name, "FastSleep");
>  			strcpy(powernv_states[nr_idle_states].desc, "FastSleep");
>  			powernv_states[nr_idle_states].flags = CPUIDLE_FLAG_TIMER_STOP;
> -			powernv_states[nr_idle_states].exit_latency =
> -					((unsigned int)latency_ns) / 1000;
> -			powernv_states[nr_idle_states].target_residency =
> -					((unsigned int)latency_ns / 100);
> +			powernv_states[nr_idle_states].target_residency = 300000;
>  			powernv_states[nr_idle_states].enter = &fastsleep_loop;
> -			nr_idle_states++;
>  		}
> +
> +		powernv_states[nr_idle_states].exit_latency =
> +				((unsigned int)latency_ns[i]) / 1000;
> +
> +		if (!rc) {
> +			powernv_states[nr_idle_states].target_residency =
> +				((unsigned int)residency_ns[i]) / 1000;
> +		}
> +
> +		nr_idle_states++;
>  	}
>  
> +	kfree(residency_ns);
> +out_free_latency:
> +	kfree(latency_ns);
> +out:
>  	return nr_idle_states;
>  }
>  
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
> 


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

* Re: [PATCH V4] cpuidle/powernv: Read target_residency value of idle states from DT if available
  2015-02-17  7:59 ` Preeti U Murthy
@ 2015-02-17 17:53     ` Rafael J. Wysocki
  0 siblings, 0 replies; 10+ messages in thread
From: Rafael J. Wysocki @ 2015-02-17 17:53 UTC (permalink / raw)
  To: Preeti U Murthy
  Cc: mpe, rafael.j.wysocki, stewart, linuxppc-dev, linux-kernel, linux-pm

On Tuesday, February 17, 2015 01:29:10 PM Preeti U Murthy wrote:
> Hi Rafael,

Hi,

> Can you please pull this patch ?

Sorry, I wasn't sure whether or not the patch was for me to take and then
I've been traveling lately.  Applied now.

> Another patch based on top of this is waiting to be pulled:
> [PATCH] driver/cpuidle-powernv: Avoid endianness conversions while parsing DT

OK, have you posted it already?


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

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

* Re: [PATCH V4] cpuidle/powernv: Read target_residency value of idle states from DT if available
@ 2015-02-17 17:53     ` Rafael J. Wysocki
  0 siblings, 0 replies; 10+ messages in thread
From: Rafael J. Wysocki @ 2015-02-17 17:53 UTC (permalink / raw)
  To: Preeti U Murthy
  Cc: stewart, linux-pm, rafael.j.wysocki, linux-kernel, linuxppc-dev

On Tuesday, February 17, 2015 01:29:10 PM Preeti U Murthy wrote:
> Hi Rafael,

Hi,

> Can you please pull this patch ?

Sorry, I wasn't sure whether or not the patch was for me to take and then
I've been traveling lately.  Applied now.

> Another patch based on top of this is waiting to be pulled:
> [PATCH] driver/cpuidle-powernv: Avoid endianness conversions while parsing DT

OK, have you posted it already?


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

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

* Re: [PATCH V4] cpuidle/powernv: Read target_residency value of idle states from DT if available
  2015-02-17 17:53     ` Rafael J. Wysocki
@ 2015-02-18  4:43       ` Preeti U Murthy
  -1 siblings, 0 replies; 10+ messages in thread
From: Preeti U Murthy @ 2015-02-18  4:43 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: stewart, linux-pm, rafael.j.wysocki, linux-kernel, linuxppc-dev,
	Michael Ellerman



On 02/17/2015 11:23 PM, Rafael J. Wysocki wrote:
> On Tuesday, February 17, 2015 01:29:10 PM Preeti U Murthy wrote:
>> Hi Rafael,
> 
> Hi,
> 
>> Can you please pull this patch ?
> 
> Sorry, I wasn't sure whether or not the patch was for me to take and then
> I've been traveling lately.  Applied now.

Hi Rafael,

I saw that the kbuild bot reported build issues around pr_warn() in this patch.
I have fixed this and below is the corrected patch. Do let me know if I have
to send it out as a separate version/RESEND tag.

> 
>> Another patch based on top of this is waiting to be pulled:
>> [PATCH] driver/cpuidle-powernv: Avoid endianness conversions while parsing DT
> 
> OK, have you posted it already?
Yes I have. https://lkml.org/lkml/2015/2/3/24. This patch is also waiting to
be pulled by you. There was some confusion around who should pull this, but
since it all the code is in the driver, we thought it would be best if you pulled it.

Thanks

--------------------------- PATCH -------------------------------------

[PATCH V4] cpuidle/powernv: Read target_residency value of idle states from DT if available

From: Preeti U Murthy <preeti@linux.vnet.ibm.com>

The device tree now exposes the residency values for different idle states. Read
these values instead of calculating residency from the latency values. The values
exposed in the DT are validated for optimal power efficiency. However to maintain
compatibility with the older firmware code which does not expose residency
values, use default values as a fallback mechanism. While at it, use better
APIs to parse the powermgmt device tree node.

Signed-off-by: Preeti U Murthy <preeti@linux.vnet.ibm.com>
Acked-by: Stewart Smith <stewart@linux.vnet.ibm.com>
Acked-by: Michael Ellerman <mpe@ellerman.id.au>
---
 drivers/cpuidle/cpuidle-powernv.c |   69 +++++++++++++++++++++----------------
 1 file changed, 40 insertions(+), 29 deletions(-)

diff --git a/drivers/cpuidle/cpuidle-powernv.c b/drivers/cpuidle/cpuidle-powernv.c
index aedec09..30d4229 100644
--- a/drivers/cpuidle/cpuidle-powernv.c
+++ b/drivers/cpuidle/cpuidle-powernv.c
@@ -13,6 +13,7 @@
 #include <linux/notifier.h>
 #include <linux/clockchips.h>
 #include <linux/of.h>
+#include <linux/slab.h>
 
 #include <asm/machdep.h>
 #include <asm/firmware.h>
@@ -159,69 +160,79 @@ static int powernv_add_idle_states(void)
 	int nr_idle_states = 1; /* Snooze */
 	int dt_idle_states;
 	const __be32 *idle_state_flags;
-	const __be32 *idle_state_latency;
-	u32 len_flags, flags, latency_ns;
-	int i;
+	u32 len_flags, flags;
+	u32 *latency_ns, *residency_ns;
+	int i, rc;
 
 	/* Currently we have snooze statically defined */
 
 	power_mgt = of_find_node_by_path("/ibm,opal/power-mgt");
 	if (!power_mgt) {
 		pr_warn("opal: PowerMgmt Node not found\n");
-		return nr_idle_states;
+		goto out;
 	}
 
-	idle_state_flags = of_get_property(power_mgt, "ibm,cpu-idle-state-flags", &len_flags);
+	idle_state_flags = of_get_property(power_mgt,
+				"ibm,cpu-idle-state-flags", &len_flags);
 	if (!idle_state_flags) {
-		pr_warn("DT-PowerMgmt: missing ibm,cpu-idle-state-flags\n");
-		return nr_idle_states;
+		pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-flags in DT\n");
+		goto out;
 	}
 
-	idle_state_latency = of_get_property(power_mgt,
-			"ibm,cpu-idle-state-latencies-ns", NULL);
-	if (!idle_state_latency) {
-		pr_warn("DT-PowerMgmt: missing ibm,cpu-idle-state-latencies-ns\n");
-		return nr_idle_states;
+	dt_idle_states = len_flags / sizeof(u32);
+
+	latency_ns = kzalloc(sizeof(*latency_ns) * dt_idle_states, GFP_KERNEL);
+	rc = of_property_read_u32_array(power_mgt,
+		"ibm,cpu-idle-state-latencies-ns", latency_ns, dt_idle_states);
+	if (rc) {
+		pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-latencies-ns in DT\n");
+		goto out_free_latency;
 	}
 
-	dt_idle_states = len_flags / sizeof(u32);
+	residency_ns = kzalloc(sizeof(*residency_ns) * dt_idle_states, GFP_KERNEL);
+	rc = of_property_read_u32_array(power_mgt,
+		"ibm,cpu-idle-state-residency-ns", residency_ns, dt_idle_states);
 
 	for (i = 0; i < dt_idle_states; i++) {
 
 		flags = be32_to_cpu(idle_state_flags[i]);
 
-		/* Cpuidle accepts exit_latency in us and we estimate
-		 * target residency to be 10x exit_latency
+		/*
+		 * Cpuidle accepts exit_latency and target_residency in us.
+		 * Use default target_residency values if f/w does not expose it.
 		 */
-		latency_ns = be32_to_cpu(idle_state_latency[i]);
 		if (flags & OPAL_PM_NAP_ENABLED) {
 			/* Add NAP state */
 			strcpy(powernv_states[nr_idle_states].name, "Nap");
 			strcpy(powernv_states[nr_idle_states].desc, "Nap");
 			powernv_states[nr_idle_states].flags = 0;
-			powernv_states[nr_idle_states].exit_latency =
-					((unsigned int)latency_ns) / 1000;
-			powernv_states[nr_idle_states].target_residency =
-					((unsigned int)latency_ns / 100);
+			powernv_states[nr_idle_states].target_residency = 100;
 			powernv_states[nr_idle_states].enter = &nap_loop;
-			nr_idle_states++;
-		}
-
-		if (flags & OPAL_PM_SLEEP_ENABLED ||
+		} else if (flags & OPAL_PM_SLEEP_ENABLED ||
 			flags & OPAL_PM_SLEEP_ENABLED_ER1) {
 			/* Add FASTSLEEP state */
 			strcpy(powernv_states[nr_idle_states].name, "FastSleep");
 			strcpy(powernv_states[nr_idle_states].desc, "FastSleep");
 			powernv_states[nr_idle_states].flags = CPUIDLE_FLAG_TIMER_STOP;
-			powernv_states[nr_idle_states].exit_latency =
-					((unsigned int)latency_ns) / 1000;
-			powernv_states[nr_idle_states].target_residency =
-					((unsigned int)latency_ns / 100);
+			powernv_states[nr_idle_states].target_residency = 300000;
 			powernv_states[nr_idle_states].enter = &fastsleep_loop;
-			nr_idle_states++;
 		}
+
+		powernv_states[nr_idle_states].exit_latency =
+				((unsigned int)latency_ns[i]) / 1000;
+
+		if (!rc) {
+			powernv_states[nr_idle_states].target_residency =
+				((unsigned int)residency_ns[i]) / 1000;
+		}
+
+		nr_idle_states++;
 	}
 
+	kfree(residency_ns);
+out_free_latency:
+	kfree(latency_ns);
+out:
 	return nr_idle_states;
 }
 

Regards
Preeti U Murthy


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

* Re: [PATCH V4] cpuidle/powernv: Read target_residency value of idle states from DT if available
@ 2015-02-18  4:43       ` Preeti U Murthy
  0 siblings, 0 replies; 10+ messages in thread
From: Preeti U Murthy @ 2015-02-18  4:43 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: stewart, linux-pm, rafael.j.wysocki, linux-kernel, linuxppc-dev



On 02/17/2015 11:23 PM, Rafael J. Wysocki wrote:
> On Tuesday, February 17, 2015 01:29:10 PM Preeti U Murthy wrote:
>> Hi Rafael,
> 
> Hi,
> 
>> Can you please pull this patch ?
> 
> Sorry, I wasn't sure whether or not the patch was for me to take and then
> I've been traveling lately.  Applied now.

Hi Rafael,

I saw that the kbuild bot reported build issues around pr_warn() in this patch.
I have fixed this and below is the corrected patch. Do let me know if I have
to send it out as a separate version/RESEND tag.

> 
>> Another patch based on top of this is waiting to be pulled:
>> [PATCH] driver/cpuidle-powernv: Avoid endianness conversions while parsing DT
> 
> OK, have you posted it already?
Yes I have. https://lkml.org/lkml/2015/2/3/24. This patch is also waiting to
be pulled by you. There was some confusion around who should pull this, but
since it all the code is in the driver, we thought it would be best if you pulled it.

Thanks

--------------------------- PATCH -------------------------------------

[PATCH V4] cpuidle/powernv: Read target_residency value of idle states from DT if available

From: Preeti U Murthy <preeti@linux.vnet.ibm.com>

The device tree now exposes the residency values for different idle states. Read
these values instead of calculating residency from the latency values. The values
exposed in the DT are validated for optimal power efficiency. However to maintain
compatibility with the older firmware code which does not expose residency
values, use default values as a fallback mechanism. While at it, use better
APIs to parse the powermgmt device tree node.

Signed-off-by: Preeti U Murthy <preeti@linux.vnet.ibm.com>
Acked-by: Stewart Smith <stewart@linux.vnet.ibm.com>
Acked-by: Michael Ellerman <mpe@ellerman.id.au>
---
 drivers/cpuidle/cpuidle-powernv.c |   69 +++++++++++++++++++++----------------
 1 file changed, 40 insertions(+), 29 deletions(-)

diff --git a/drivers/cpuidle/cpuidle-powernv.c b/drivers/cpuidle/cpuidle-powernv.c
index aedec09..30d4229 100644
--- a/drivers/cpuidle/cpuidle-powernv.c
+++ b/drivers/cpuidle/cpuidle-powernv.c
@@ -13,6 +13,7 @@
 #include <linux/notifier.h>
 #include <linux/clockchips.h>
 #include <linux/of.h>
+#include <linux/slab.h>
 
 #include <asm/machdep.h>
 #include <asm/firmware.h>
@@ -159,69 +160,79 @@ static int powernv_add_idle_states(void)
 	int nr_idle_states = 1; /* Snooze */
 	int dt_idle_states;
 	const __be32 *idle_state_flags;
-	const __be32 *idle_state_latency;
-	u32 len_flags, flags, latency_ns;
-	int i;
+	u32 len_flags, flags;
+	u32 *latency_ns, *residency_ns;
+	int i, rc;
 
 	/* Currently we have snooze statically defined */
 
 	power_mgt = of_find_node_by_path("/ibm,opal/power-mgt");
 	if (!power_mgt) {
 		pr_warn("opal: PowerMgmt Node not found\n");
-		return nr_idle_states;
+		goto out;
 	}
 
-	idle_state_flags = of_get_property(power_mgt, "ibm,cpu-idle-state-flags", &len_flags);
+	idle_state_flags = of_get_property(power_mgt,
+				"ibm,cpu-idle-state-flags", &len_flags);
 	if (!idle_state_flags) {
-		pr_warn("DT-PowerMgmt: missing ibm,cpu-idle-state-flags\n");
-		return nr_idle_states;
+		pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-flags in DT\n");
+		goto out;
 	}
 
-	idle_state_latency = of_get_property(power_mgt,
-			"ibm,cpu-idle-state-latencies-ns", NULL);
-	if (!idle_state_latency) {
-		pr_warn("DT-PowerMgmt: missing ibm,cpu-idle-state-latencies-ns\n");
-		return nr_idle_states;
+	dt_idle_states = len_flags / sizeof(u32);
+
+	latency_ns = kzalloc(sizeof(*latency_ns) * dt_idle_states, GFP_KERNEL);
+	rc = of_property_read_u32_array(power_mgt,
+		"ibm,cpu-idle-state-latencies-ns", latency_ns, dt_idle_states);
+	if (rc) {
+		pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-latencies-ns in DT\n");
+		goto out_free_latency;
 	}
 
-	dt_idle_states = len_flags / sizeof(u32);
+	residency_ns = kzalloc(sizeof(*residency_ns) * dt_idle_states, GFP_KERNEL);
+	rc = of_property_read_u32_array(power_mgt,
+		"ibm,cpu-idle-state-residency-ns", residency_ns, dt_idle_states);
 
 	for (i = 0; i < dt_idle_states; i++) {
 
 		flags = be32_to_cpu(idle_state_flags[i]);
 
-		/* Cpuidle accepts exit_latency in us and we estimate
-		 * target residency to be 10x exit_latency
+		/*
+		 * Cpuidle accepts exit_latency and target_residency in us.
+		 * Use default target_residency values if f/w does not expose it.
 		 */
-		latency_ns = be32_to_cpu(idle_state_latency[i]);
 		if (flags & OPAL_PM_NAP_ENABLED) {
 			/* Add NAP state */
 			strcpy(powernv_states[nr_idle_states].name, "Nap");
 			strcpy(powernv_states[nr_idle_states].desc, "Nap");
 			powernv_states[nr_idle_states].flags = 0;
-			powernv_states[nr_idle_states].exit_latency =
-					((unsigned int)latency_ns) / 1000;
-			powernv_states[nr_idle_states].target_residency =
-					((unsigned int)latency_ns / 100);
+			powernv_states[nr_idle_states].target_residency = 100;
 			powernv_states[nr_idle_states].enter = &nap_loop;
-			nr_idle_states++;
-		}
-
-		if (flags & OPAL_PM_SLEEP_ENABLED ||
+		} else if (flags & OPAL_PM_SLEEP_ENABLED ||
 			flags & OPAL_PM_SLEEP_ENABLED_ER1) {
 			/* Add FASTSLEEP state */
 			strcpy(powernv_states[nr_idle_states].name, "FastSleep");
 			strcpy(powernv_states[nr_idle_states].desc, "FastSleep");
 			powernv_states[nr_idle_states].flags = CPUIDLE_FLAG_TIMER_STOP;
-			powernv_states[nr_idle_states].exit_latency =
-					((unsigned int)latency_ns) / 1000;
-			powernv_states[nr_idle_states].target_residency =
-					((unsigned int)latency_ns / 100);
+			powernv_states[nr_idle_states].target_residency = 300000;
 			powernv_states[nr_idle_states].enter = &fastsleep_loop;
-			nr_idle_states++;
 		}
+
+		powernv_states[nr_idle_states].exit_latency =
+				((unsigned int)latency_ns[i]) / 1000;
+
+		if (!rc) {
+			powernv_states[nr_idle_states].target_residency =
+				((unsigned int)residency_ns[i]) / 1000;
+		}
+
+		nr_idle_states++;
 	}
 
+	kfree(residency_ns);
+out_free_latency:
+	kfree(latency_ns);
+out:
 	return nr_idle_states;
 }
 

Regards
Preeti U Murthy

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

* Re: [PATCH V4] cpuidle/powernv: Read target_residency value of idle states from DT if available
  2015-02-18  4:43       ` Preeti U Murthy
@ 2015-02-18  5:51         ` Rafael J. Wysocki
  -1 siblings, 0 replies; 10+ messages in thread
From: Rafael J. Wysocki @ 2015-02-18  5:51 UTC (permalink / raw)
  To: Preeti U Murthy
  Cc: stewart, linux-pm, rafael.j.wysocki, linux-kernel, linuxppc-dev,
	Michael Ellerman

On Wednesday, February 18, 2015 10:13:23 AM Preeti U Murthy wrote:
> 
> On 02/17/2015 11:23 PM, Rafael J. Wysocki wrote:
> > On Tuesday, February 17, 2015 01:29:10 PM Preeti U Murthy wrote:
> >> Hi Rafael,
> > 
> > Hi,
> > 
> >> Can you please pull this patch ?
> > 
> > Sorry, I wasn't sure whether or not the patch was for me to take and then
> > I've been traveling lately.  Applied now.
> 
> Hi Rafael,
> 
> I saw that the kbuild bot reported build issues around pr_warn() in this patch.
> I have fixed this and below is the corrected patch. Do let me know if I have
> to send it out as a separate version/RESEND tag.

I've applied it, thanks!

> >> Another patch based on top of this is waiting to be pulled:
> >> [PATCH] driver/cpuidle-powernv: Avoid endianness conversions while parsing DT
> > 
> > OK, have you posted it already?
> Yes I have. https://lkml.org/lkml/2015/2/3/24.

OK, so this one does not apply for me on top of the other.  Can you rebase it
on top of my current linux-next branch and resend it, please?

Also I'd appreciate it if someone could ACK DT-related stuff for me.

> This patch is also waiting to be pulled by you. There was some confusion
> around who should pull this, but since it all the code is in the driver, we
> thought it would be best if you pulled it.

Well, OK.  If you want me to apply a patch, send it To: me and not To: someone
else.  If you send it To: someone else, I'll ignore it.

Also ask people to review and ACK things you want me to pull, especially if
that's not stuff I'm particularly familiar with (like related to DT bindings
specific to your platform etc.).


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

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

* Re: [PATCH V4] cpuidle/powernv: Read target_residency value of idle states from DT if available
@ 2015-02-18  5:51         ` Rafael J. Wysocki
  0 siblings, 0 replies; 10+ messages in thread
From: Rafael J. Wysocki @ 2015-02-18  5:51 UTC (permalink / raw)
  To: Preeti U Murthy
  Cc: stewart, linux-pm, rafael.j.wysocki, linux-kernel, linuxppc-dev

On Wednesday, February 18, 2015 10:13:23 AM Preeti U Murthy wrote:
> 
> On 02/17/2015 11:23 PM, Rafael J. Wysocki wrote:
> > On Tuesday, February 17, 2015 01:29:10 PM Preeti U Murthy wrote:
> >> Hi Rafael,
> > 
> > Hi,
> > 
> >> Can you please pull this patch ?
> > 
> > Sorry, I wasn't sure whether or not the patch was for me to take and then
> > I've been traveling lately.  Applied now.
> 
> Hi Rafael,
> 
> I saw that the kbuild bot reported build issues around pr_warn() in this patch.
> I have fixed this and below is the corrected patch. Do let me know if I have
> to send it out as a separate version/RESEND tag.

I've applied it, thanks!

> >> Another patch based on top of this is waiting to be pulled:
> >> [PATCH] driver/cpuidle-powernv: Avoid endianness conversions while parsing DT
> > 
> > OK, have you posted it already?
> Yes I have. https://lkml.org/lkml/2015/2/3/24.

OK, so this one does not apply for me on top of the other.  Can you rebase it
on top of my current linux-next branch and resend it, please?

Also I'd appreciate it if someone could ACK DT-related stuff for me.

> This patch is also waiting to be pulled by you. There was some confusion
> around who should pull this, but since it all the code is in the driver, we
> thought it would be best if you pulled it.

Well, OK.  If you want me to apply a patch, send it To: me and not To: someone
else.  If you send it To: someone else, I'll ignore it.

Also ask people to review and ACK things you want me to pull, especially if
that's not stuff I'm particularly familiar with (like related to DT bindings
specific to your platform etc.).


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

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

* Re: [PATCH V4] cpuidle/powernv: Read target_residency value of idle states from DT if available
  2015-02-18  5:51         ` Rafael J. Wysocki
@ 2015-02-18  6:18           ` Preeti U Murthy
  -1 siblings, 0 replies; 10+ messages in thread
From: Preeti U Murthy @ 2015-02-18  6:18 UTC (permalink / raw)
  To: Rafael J. Wysocki, Michael Ellerman
  Cc: stewart, linux-pm, rafael.j.wysocki, linux-kernel, linuxppc-dev



On 02/18/2015 11:21 AM, Rafael J. Wysocki wrote:
> On Wednesday, February 18, 2015 10:13:23 AM Preeti U Murthy wrote:
>>
>> On 02/17/2015 11:23 PM, Rafael J. Wysocki wrote:
>>> On Tuesday, February 17, 2015 01:29:10 PM Preeti U Murthy wrote:
>>>> Hi Rafael,
>>>
>>> Hi,
>>>
>>>> Can you please pull this patch ?
>>>
>>> Sorry, I wasn't sure whether or not the patch was for me to take and then
>>> I've been traveling lately.  Applied now.
>>
>> Hi Rafael,
>>
>> I saw that the kbuild bot reported build issues around pr_warn() in this patch.
>> I have fixed this and below is the corrected patch. Do let me know if I have
>> to send it out as a separate version/RESEND tag.
> 
> I've applied it, thanks!
> 
>>>> Another patch based on top of this is waiting to be pulled:
>>>> [PATCH] driver/cpuidle-powernv: Avoid endianness conversions while parsing DT
>>>
>>> OK, have you posted it already?
>> Yes I have. https://lkml.org/lkml/2015/2/3/24.
> 
> OK, so this one does not apply for me on top of the other.  Can you rebase it
> on top of my current linux-next branch and resend it, please?
> 
> Also I'd appreciate it if someone could ACK DT-related stuff for me.

Yes Ill get this done.
> 
>> This patch is also waiting to be pulled by you. There was some confusion
>> around who should pull this, but since it all the code is in the driver, we
>> thought it would be best if you pulled it.
> 
> Well, OK.  If you want me to apply a patch, send it To: me and not To: someone
> else.  If you send it To: someone else, I'll ignore it.
> 
> Also ask people to review and ACK things you want me to pull, especially if
> that's not stuff I'm particularly familiar with (like related to DT bindings
> specific to your platform etc.).
> 

Sure Ill do that. Thanks!

Regards
Preeti U Murthy
> 


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

* Re: [PATCH V4] cpuidle/powernv: Read target_residency value of idle states from DT if available
@ 2015-02-18  6:18           ` Preeti U Murthy
  0 siblings, 0 replies; 10+ messages in thread
From: Preeti U Murthy @ 2015-02-18  6:18 UTC (permalink / raw)
  To: Rafael J. Wysocki, Michael Ellerman
  Cc: stewart, rafael.j.wysocki, linuxppc-dev, linux-kernel, linux-pm



On 02/18/2015 11:21 AM, Rafael J. Wysocki wrote:
> On Wednesday, February 18, 2015 10:13:23 AM Preeti U Murthy wrote:
>>
>> On 02/17/2015 11:23 PM, Rafael J. Wysocki wrote:
>>> On Tuesday, February 17, 2015 01:29:10 PM Preeti U Murthy wrote:
>>>> Hi Rafael,
>>>
>>> Hi,
>>>
>>>> Can you please pull this patch ?
>>>
>>> Sorry, I wasn't sure whether or not the patch was for me to take and then
>>> I've been traveling lately.  Applied now.
>>
>> Hi Rafael,
>>
>> I saw that the kbuild bot reported build issues around pr_warn() in this patch.
>> I have fixed this and below is the corrected patch. Do let me know if I have
>> to send it out as a separate version/RESEND tag.
> 
> I've applied it, thanks!
> 
>>>> Another patch based on top of this is waiting to be pulled:
>>>> [PATCH] driver/cpuidle-powernv: Avoid endianness conversions while parsing DT
>>>
>>> OK, have you posted it already?
>> Yes I have. https://lkml.org/lkml/2015/2/3/24.
> 
> OK, so this one does not apply for me on top of the other.  Can you rebase it
> on top of my current linux-next branch and resend it, please?
> 
> Also I'd appreciate it if someone could ACK DT-related stuff for me.

Yes Ill get this done.
> 
>> This patch is also waiting to be pulled by you. There was some confusion
>> around who should pull this, but since it all the code is in the driver, we
>> thought it would be best if you pulled it.
> 
> Well, OK.  If you want me to apply a patch, send it To: me and not To: someone
> else.  If you send it To: someone else, I'll ignore it.
> 
> Also ask people to review and ACK things you want me to pull, especially if
> that's not stuff I'm particularly familiar with (like related to DT bindings
> specific to your platform etc.).
> 

Sure Ill do that. Thanks!

Regards
Preeti U Murthy
> 

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

end of thread, other threads:[~2015-02-18  6:20 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-03  6:20 [PATCH V4] cpuidle/powernv: Read target_residency value of idle states from DT if available Preeti U Murthy
2015-02-17  7:59 ` Preeti U Murthy
2015-02-17 17:53   ` Rafael J. Wysocki
2015-02-17 17:53     ` Rafael J. Wysocki
2015-02-18  4:43     ` Preeti U Murthy
2015-02-18  4:43       ` Preeti U Murthy
2015-02-18  5:51       ` Rafael J. Wysocki
2015-02-18  5:51         ` Rafael J. Wysocki
2015-02-18  6:18         ` Preeti U Murthy
2015-02-18  6:18           ` Preeti U Murthy

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.