| Message ID | 54BE3B79.9070004@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
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
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?
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.jpg http://i1249.photobucket.com/albums/hh511/tlosm/BenchmarkG5/2_zps84cf4cac.jpg http://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: > > 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 > _______________________________________________ > Linuxppc-dev mailing list > Linuxppc-dev@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/linuxppc-dev
On Tue, 2015-20-01 at 11:26:49 UTC, Preeti U Murthy wrote: > @@ -177,34 +178,39 @@ static int powernv_add_idle_states(void) > return nr_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); That's only reading the first value. If you want to read the full property you need the _array version. > + if (rc) { > pr_warn("DT-PowerMgmt: missing ibm,cpu-idle-state-latencies-ns\n"); You missed my hint that "DT-PowerMgmt" is a weird and ugly prefix. Can you use "cpuidle-powernv:" instead? > > - 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"); I don't think this is worth a warning seeing as we know there are firmwares out there which do not have the property. > 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? cheers
On 01/23/2015 10:29 AM, Michael Ellerman wrote: > On Tue, 2015-20-01 at 11:26:49 UTC, Preeti U Murthy wrote: >> @@ -177,34 +178,39 @@ static int powernv_add_idle_states(void) >> return nr_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); > > That's only reading the first value. > > If you want to read the full property you need the _array version. Right, thanks for pointing this out. > >> + if (rc) { >> pr_warn("DT-PowerMgmt: missing ibm,cpu-idle-state-latencies-ns\n"); > > You missed my hint that "DT-PowerMgmt" is a weird and ugly prefix. Can you use > "cpuidle-powernv:" instead? Yes will change this. > >> >> - 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"); > > I don't think this is worth a warning seeing as we know there are firmwares out > there which do not have the property. > >> 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 > > cheers >
On Tue, 2015-01-27 at 09:32 +0530, Preeti U Murthy wrote: > On 01/23/2015 10:29 AM, Michael Ellerman wrote: > > On Tue, 2015-20-01 at 11:26:49 UTC, Preeti U Murthy wrote: > >> @@ -177,34 +178,39 @@ static int powernv_add_idle_states(void) > > > >> 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
diff --git a/drivers/cpuidle/cpuidle-powernv.c b/drivers/cpuidle/cpuidle-powernv.c index aedec09..2031560 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,9 +160,9 @@ 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 */ @@ -177,34 +178,39 @@ static int powernv_add_idle_states(void) return nr_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"); return nr_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 - * 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 || @@ -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 = - ((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++; } return nr_idle_states;