From: Preeti U Murthy <hidden> Date: 2015-01-19 11:33:04
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, clump
the common parts of device tree parsing into one chunk.
Signed-off-by: Preeti U Murthy <redacted>
---
drivers/cpuidle/cpuidle-powernv.c | 39 ++++++++++++++++++++++++-------------
1 file changed, 25 insertions(+), 14 deletions(-)
@@ -162,7 +162,8 @@ static int powernv_add_idle_states(void)intdt_idle_states;const__be32*idle_state_flags;const__be32*idle_state_latency;-u32len_flags,flags,latency_ns;+const__be32*idle_state_residency;+u32len_flags,flags,latency_ns,residency_ns;inti;/* Currently we have snooze statically defined */
@@ -186,14 +187,21 @@ static int powernv_add_idle_states(void)returnnr_idle_states;}+idle_state_residency=of_get_property(power_mgt,+"ibm,cpu-idle-state-residency-ns",NULL);+if(!idle_state_residency){+pr_warn("DT-PowerMgmt: missing ibm,cpu-idle-state-residency-ns\n");+pr_warn("Falling back to default values\n");+}+dt_idle_states=len_flags/sizeof(u32);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-*targetresidencytobe10xexit_latency+/*+*Cpuidleacceptsexit_latencyandtarget_residencyinus.+*Usedefaulttarget_residencyvaluesiff/wdoesnotexposeit.*/latency_ns=be32_to_cpu(idle_state_latency[i]);if(flags&OPAL_PM_NAP_ENABLED){
@@ -201,12 +209,8 @@ static int powernv_add_idle_states(void)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=-((unsignedint)latency_ns)/1000;-powernv_states[nr_idle_states].target_residency=-((unsignedint)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||
@@ -215,13 +219,20 @@ static int powernv_add_idle_states(void)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=-((unsignedint)latency_ns)/1000;-powernv_states[nr_idle_states].target_residency=-((unsignedint)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=+((unsignedint)latency_ns)/1000;++if(idle_state_residency){+residency_ns=be32_to_cpu(idle_state_residency[i]);+powernv_states[nr_idle_states].target_residency=+((unsignedint)residency_ns)/1000;+}++nr_idle_states++;}returnnr_idle_states;
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2015-01-20 05:45:10
On Mon, 2015-19-01 at 11:32:51 UTC, Preeti U Murthy wrote:
quoted hunk
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, clump
the common parts of device tree parsing into one chunk.
Signed-off-by: Preeti U Murthy <redacted>
---
drivers/cpuidle/cpuidle-powernv.c | 39 ++++++++++++++++++++++++-------------
1 file changed, 25 insertions(+), 14 deletions(-)
@@ -162,7 +162,8 @@ static int powernv_add_idle_states(void)intdt_idle_states;const__be32*idle_state_flags;const__be32*idle_state_latency;-u32len_flags,flags,latency_ns;+const__be32*idle_state_residency;+u32len_flags,flags,latency_ns,residency_ns;inti;/* Currently we have snooze statically defined */
@@ -186,14 +187,21 @@ static int powernv_add_idle_states(void)returnnr_idle_states;}+idle_state_residency=of_get_property(power_mgt,+"ibm,cpu-idle-state-residency-ns",NULL);+if(!idle_state_residency){+pr_warn("DT-PowerMgmt: missing ibm,cpu-idle-state-residency-ns\n");+pr_warn("Falling back to default values\n");+}
This would be better done with something like:
rc = of_read_property_u32(power_mgt, "ibm,cpu-idle-state-residency-ns", &residency_ns);
if (!rc) {
pr_info("cpuidle-powernv: missing ibm,cpu-idle-state-residency-ns\n");
residency_ns = 300000;
}
cheers
From: Preeti U Murthy <hidden> Date: 2015-01-20 11:27:08
On 01/20/2015 11:15 AM, Michael Ellerman wrote:
On Mon, 2015-19-01 at 11:32:51 UTC, Preeti U Murthy wrote:
quoted
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, clump
the common parts of device tree parsing into one chunk.
Signed-off-by: Preeti U Murthy <redacted>
---
drivers/cpuidle/cpuidle-powernv.c | 39 ++++++++++++++++++++++++-------------
1 file changed, 25 insertions(+), 14 deletions(-)
@@ -162,7 +162,8 @@ static int powernv_add_idle_states(void)intdt_idle_states;const__be32*idle_state_flags;const__be32*idle_state_latency;-u32len_flags,flags,latency_ns;+const__be32*idle_state_residency;+u32len_flags,flags,latency_ns,residency_ns;inti;/* Currently we have snooze statically defined */
@@ -186,14 +187,21 @@ static int powernv_add_idle_states(void)returnnr_idle_states;}+idle_state_residency=of_get_property(power_mgt,+"ibm,cpu-idle-state-residency-ns",NULL);+if(!idle_state_residency){+pr_warn("DT-PowerMgmt: missing ibm,cpu-idle-state-residency-ns\n");+pr_warn("Falling back to default values\n");+}
This would be better done with something like:
rc = of_read_property_u32(power_mgt, "ibm,cpu-idle-state-residency-ns", &residency_ns);
if (!rc) {
pr_info("cpuidle-powernv: missing ibm,cpu-idle-state-residency-ns\n");
residency_ns = 300000;
}
This looks like a better API, but the default residency values are different
for each idle state. So perhaps a patch like the below ?
--------------------------Start Patch------------------------------------------
From: Preeti U Murthy <redacted>
---
drivers/cpuidle/cpuidle-powernv.c | 54 +++++++++++++++++++++++--------------
1 file changed, 33 insertions(+), 21 deletions(-)
@@ -159,9 +160,9 @@ static int powernv_add_idle_states(void)intnr_idle_states=1;/* Snooze */intdt_idle_states;const__be32*idle_state_flags;-const__be32*idle_state_latency;-u32len_flags,flags,latency_ns;-inti;+u32len_flags,flags;+u32*latency_ns,*residency_ns;+inti,rc;/* Currently we have snooze statically defined */
@@ -177,34 +178,39 @@ static int powernv_add_idle_states(void)returnnr_idle_states;}-idle_state_latency=of_get_property(power_mgt,-"ibm,cpu-idle-state-latencies-ns",NULL);-if(!idle_state_latency){+dt_idle_states=len_flags/sizeof(u32);++latency_ns=kzalloc(sizeof(*latency_ns)*dt_idle_states,GFP_KERNEL);+rc=of_property_read_u32(power_mgt,+"ibm,cpu-idle-state-latencies-ns",latency_ns);+if(rc){pr_warn("DT-PowerMgmt: missing ibm,cpu-idle-state-latencies-ns\n");returnnr_idle_states;}-dt_idle_states=len_flags/sizeof(u32);+residency_ns=kzalloc(sizeof(*residency_ns)*dt_idle_states,GFP_KERNEL);+rc=of_property_read_u32(power_mgt,+"ibm,cpu-idle-state-residency-ns",residency_ns);+if(rc){+pr_warn("DT-PowerMgmt: missing ibm,cpu-idle-state-residency-ns\n");+pr_warn("Falling back to default values\n");+}+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-*targetresidencytobe10xexit_latency+/*+*Cpuidleacceptsexit_latencyandtarget_residencyinus.+*Usedefaulttarget_residencyvaluesiff/wdoesnotexposeit.*/-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=-((unsignedint)latency_ns)/1000;-powernv_states[nr_idle_states].target_residency=-((unsignedint)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||
@@ -213,13 +219,19 @@ static int powernv_add_idle_states(void)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=-((unsignedint)latency_ns)/1000;-powernv_states[nr_idle_states].target_residency=-((unsignedint)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=+((unsignedint)latency_ns[i])/1000;++if(!rc){+powernv_states[nr_idle_states].target_residency=+((unsignedint)residency_ns[i])/1000;+}++nr_idle_states++;}returnnr_idle_states;
From: luigi burdo <hidden> Date: 2015-01-20 11:57:34
Hello all LinuxPPc Developer,
need to report a issue that im facing from kernel 3.16 and up.
The fans of powermac g5 quad start boosting like a turbo jet after the system is
been loaded and up.
Usually this become after about 30-60 seconds when the system is loaded.
This issue start with Xorg loaded or only in teminal system (gdm3 not loaded)
This issue come with system load with minimum cpu usage 5% and less too.
During the closing of the system (reboot) the fans goes in right silents mode.
I had been tested and compiled the kernel with all the modules of fan controls for Apple G5
without a positive result.
Kernel tested 3.16.ctk2 (debian wheezy 7.8), 3.18.3 and 3.19.rc5
Im using Debian Wheezy 7.6 (up 7.8) PPC on Quad G5 (970mp) Nvidia 7800gtx and RadeonHD 6570
Dont face this issue with old kernels version 3.10.65 and pre
(didnt test the 3.12 and 3.14)
Need to report to Michel D. from amd.com . The RadeonHD 6570 now is working on PowerMac G5 too
look like the Xorg bigendian issue related Evergreen / Northern is been fixed ;-)
Thanks alot for your jobs guys
Hope soon i will able to help in X5000 hardware too
PPC Rulez ... and sorry for my "perfect english"
Luigi Burdo
From: Michel Dänzer <hidden> Date: 2015-01-21 06:40:00
On 20.01.2015 20:52, luigi burdo wrote:
Hello all LinuxPPc Developer,
need to report a issue that im facing from kernel 3.16 and up.
The fans of powermac g5 quad start boosting like a turbo jet after the
system is
been loaded and up.
Usually this become after about 30-60 seconds when the system is loaded.
You need to load the i2c_powermac kernel module.
Need to report to Michel D. from amd.com . The RadeonHD 6570 now is
working on PowerMac G5 too
look like the Xorg bigendian issue related Evergreen / Northern is
been fixed ;-)
Xorg itself has always been working, only OpenGL can be problematic.
Have you verified r600_dri.so is used for that?
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
From: luigi burdo <hidden> Date: 2015-01-21 08:13:39
Hi Mike, thank you for your reply .
yes im sorry i was fixed the issue yesterday night,
forcing the module i2c_powermac loaded from etc/modules
stange it was not loaded by default like it was before.
in any way *fixed :-)
About the RadeonHD , yes you are true, have the Mesa/Gallium
working is problematic but what is not problematic in the good ppc world ? :-)
Right now im using the software resterized i didnt had the time
to check everythings regarding the gpu because i was need
first fix the turbine noise in my home made by powermac (or my wife had been kill me)
The good thing i see all the firmware -nonfree loaded without errors
before not and with lubuntu 14.04ts i dint have a good rasterizing too on 6570hd.
Forget to mention, before for have the radeonhd working on g5 quad i was need the X1900gt in couple with 4xxx and 6xxx hd,
now im using it in couple with and Nvidia with apple firmware. i think this is the best advance for make people
from the ppc apple community swap finnally their gpu.
I hope soon will be able to have the mesa gallium working there for advance the communty .
here couple of screenshots.
http://i1249.photobucket.com/albums/hh511/tlosm/BenchmarkG5/1_zps025bc7ed.jpghttp://i1249.photobucket.com/albums/hh511/tlosm/BenchmarkG5/2_zps84cf4cac.jpghttp://i1249.photobucket.com/albums/hh511/tlosm/BenchmarkG5/3_zpsfded5971.jpg
Need to Notice to Alexander Graf the Kvm and Kvm-pr are running great too.
Thank you very mutch to all the LinuxPPC Devs
Luigi Burdo
Date: Wed, 21 Jan 2015 15:39:50 +0900
From: michel@daenzer.net
To: intermediadc@hotmail.com
Subject: Re: PowerMac G5 Quad Issue reporting
CC: linuxppc-dev@lists.ozlabs.org; linux-kernel@vger.kernel.org
On 20.01.2015 20:52, luigi burdo wrote:
quoted
Hello all LinuxPPc Developer,
need to report a issue that im facing from kernel 3.16 and up.
The fans of powermac g5 quad start boosting like a turbo jet after the
system is
been loaded and up.
Usually this become after about 30-60 seconds when the system is loaded.
You need to load the i2c_powermac kernel module.
quoted
Need to report to Michel D. from amd.com . The RadeonHD 6570 now is
working on PowerMac G5 too
look like the Xorg bigendian issue related Evergreen / Northern is
been fixed ;-)
Xorg itself has always been working, only OpenGL can be problematic.
Have you verified r600_dri.so is used for that?
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
I don't think this is worth a warning seeing as we know there are firmwares out
there which do not have the property.
quoted
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++;
That looks wrong? Or do you mean to do that?
Are you pointing at removing the lines that populate the exit_latency ?
I read the latency value from the DT outside of these conditions because
it is guaranteed to be present at this point. We return back from this
function early on if we do not find it in the DT.
If you are pointing at the lines that remove increment of
nr_idle_states, it has to be done so towards the end of the iteration
because we are yet to populate the exit_latency and target_residency
outside of these conditions using this index.
Regards
Preeti U Murthy
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2015-01-27 06:28:16
On Tue, 2015-01-27 at 09:32 +0530, Preeti U Murthy wrote:
On 01/23/2015 10:29 AM, Michael Ellerman wrote:
quoted
On Tue, 2015-20-01 at 11:26:49 UTC, Preeti U Murthy wrote:
quoted
@@ -177,34 +178,39 @@ static int powernv_add_idle_states(void)
quoted
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++;
That looks wrong? Or do you mean to do that?
If you are pointing at the lines that remove increment of
nr_idle_states, it has to be done so towards the end of the iteration
because we are yet to populate the exit_latency and target_residency
outside of these conditions using this index.
Oh OK I think I get it, it's not clear at all.
The resulting code is:
for (i = 0; i < dt_idle_states; i++) {
flags = be32_to_cpu(idle_state_flags[i]);
/*
* Cpuidle accepts exit_latency and target_residency in us.
* Use default target_residency values if f/w does not expose it.
*/
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].target_residency = 100;
powernv_states[nr_idle_states].enter = &nap_loop;
}
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].target_residency = 300000;
powernv_states[nr_idle_states].enter = &fastsleep_loop;
}
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++;
}
Which looks like you could potentially overwrite the first set of values, "Nap"
with "FastSleep". But I think what you're going to tell me is that flags can
never satisfy both conditions, so we will only ever execute either the "Nap"
case OR the "FastSleep".
If so please change the code to make that obvious, ie. by using else if.
cheers