Patchwork therm_pm72 units, interface

login
register
mail settings
Submitter Aaro Koskinen
Date July 20, 2013, 8:33 p.m.
Message ID <20130720203346.GM14385@blackmetal.musicnaut.iki.fi>
Download mbox | patch
Permalink /patch/260479/
State Accepted
Headers show

Comments

Aaro Koskinen - July 20, 2013, 8:33 p.m.
Hi,

On Sat, Jul 20, 2013 at 09:16:49AM +1000, Benjamin Herrenschmidt wrote:
> On Fri, 2013-07-19 at 20:43 +0300, Aaro Koskinen wrote:
> > I booted a Xserve today with 3.11-rc1, and noticed the noise is coming
> > from slots-fan (PCI fan?) which is always 99%, although the slots-temp is
> > just 33. Is it on purpose the rm31 slots PID params are totally different
> > from therm_pm72 slots params? It seems like they have been copied from
> > pm72 "drive bay" PID params instead.
> 
> Could be my mistake. What happens if you change them to match the old code ?

I tried the change below (use SLOTS_PID_* values from therm_pm72.h),
and the noise level is considerably lower. The slots-fan stays at 19%,
and slots-temp rises to 42'C but not higher after running couple of hours.
Benjamin Herrenschmidt - July 20, 2013, 11:42 p.m.
On Sat, 2013-07-20 at 23:33 +0300, Aaro Koskinen wrote:
> Hi,
> 
> On Sat, Jul 20, 2013 at 09:16:49AM +1000, Benjamin Herrenschmidt wrote:
> > On Fri, 2013-07-19 at 20:43 +0300, Aaro Koskinen wrote:
> > > I booted a Xserve today with 3.11-rc1, and noticed the noise is coming
> > > from slots-fan (PCI fan?) which is always 99%, although the slots-temp is
> > > just 33. Is it on purpose the rm31 slots PID params are totally different
> > > from therm_pm72 slots params? It seems like they have been copied from
> > > pm72 "drive bay" PID params instead.
> > 
> > Could be my mistake. What happens if you change them to match the old code ?
> 
> I tried the change below (use SLOTS_PID_* values from therm_pm72.h),
> and the noise level is considerably lower. The slots-fan stays at 19%,
> and slots-temp rises to 42'C but not higher after running couple of hours.

Thanks. I'll give it a quick dbl check asap (about to head out of town
now), in the meantime, can you send it with the appropriate
Signed-off-by: etc... ?

Thanks,
Ben.

> diff --git a/drivers/macintosh/windfarm_rm31.c b/drivers/macintosh/windfarm_rm31.c
> index 0b9a79b..82fc86a 100644
> --- a/drivers/macintosh/windfarm_rm31.c
> +++ b/drivers/macintosh/windfarm_rm31.c
> @@ -439,15 +439,15 @@ static void backside_setup_pid(void)
>  
>  /* Slots fan */
>  static const struct wf_pid_param slots_param = {
> -	.interval	= 5,
> -	.history_len	= 2,
> -	.gd		= 30 << 20,
> -	.gp		= 5 << 20,
> -	.gr		= 0,
> -	.itarget	= 40 << 16,
> -	.additive	= 1,
> -	.min		= 300,
> -	.max		= 4000,
> +	.interval	= 1,
> +	.history_len	= 20,
> +	.gd		= 0,
> +	.gp		= 0,
> +	.gr		= 0x00100000,
> +	.itarget	= 3200000,
> +	.additive	= 0,
> +	.min		= 20,
> +	.max		= 100,
>  };
>  
>  static void slots_fan_tick(void)
Michel Dänzer - Aug. 2, 2013, 10:03 a.m.
On Sam, 2013-07-20 at 23:33 +0300, Aaro Koskinen wrote:
> Hi,
> 
> On Sat, Jul 20, 2013 at 09:16:49AM +1000, Benjamin Herrenschmidt wrote:
> > On Fri, 2013-07-19 at 20:43 +0300, Aaro Koskinen wrote:
> > > I booted a Xserve today with 3.11-rc1, and noticed the noise is coming
> > > from slots-fan (PCI fan?) which is always 99%, although the slots-temp is
> > > just 33. Is it on purpose the rm31 slots PID params are totally different
> > > from therm_pm72 slots params? It seems like they have been copied from
> > > pm72 "drive bay" PID params instead.
> > 
> > Could be my mistake. What happens if you change them to match the old code ?
> 
> I tried the change below (use SLOTS_PID_* values from therm_pm72.h),
> and the noise level is considerably lower. The slots-fan stays at 19%,
> and slots-temp rises to 42'C but not higher after running couple of hours.
> 
> diff --git a/drivers/macintosh/windfarm_rm31.c b/drivers/macintosh/windfarm_rm31.c
> index 0b9a79b..82fc86a 100644
> --- a/drivers/macintosh/windfarm_rm31.c
> +++ b/drivers/macintosh/windfarm_rm31.c
> @@ -439,15 +439,15 @@ static void backside_setup_pid(void)
>  
>  /* Slots fan */
>  static const struct wf_pid_param slots_param = {
> -	.interval	= 5,
> -	.history_len	= 2,
> -	.gd		= 30 << 20,
> -	.gp		= 5 << 20,
> -	.gr		= 0,
> -	.itarget	= 40 << 16,
> -	.additive	= 1,
> -	.min		= 300,
> -	.max		= 4000,
> +	.interval	= 1,
> +	.history_len	= 20,
> +	.gd		= 0,
> +	.gp		= 0,
> +	.gr		= 0x00100000,
> +	.itarget	= 3200000,
> +	.additive	= 0,
> +	.min		= 20,
> +	.max		= 100,
>  };
>  
>  static void slots_fan_tick(void)

Could a similar change fix the same problem on desktop G5s? The same
values for slots_param in windfarm_pm112.c don't help, unfortunately.
Benjamin Herrenschmidt - Aug. 2, 2013, 12:51 p.m.
On Fri, 2013-08-02 at 12:03 +0200, Michel Dänzer wrote:
> Could a similar change fix the same problem on desktop G5s? The same
> values for slots_param in windfarm_pm112.c don't help, unfortunately.

You have a 11,2 and a noisy fan ? Odd, mine(s) don't.... I can dbl check
the values vs. what Darwin uses tomorrow...

The one that is known to have a noisy slot fan is the 7,2/7,3 one
because the Apple algorithm relies on sensors on the video card to
control the slots fan and I don't have the infrastructure for that in
Linux, so I set it to a fixed speed.

But the 11,2 should be fine.

Cheers,
Ben.
Michel Dänzer - Aug. 2, 2013, 2:47 p.m.
On Fre, 2013-08-02 at 22:51 +1000, Benjamin Herrenschmidt wrote:
> On Fri, 2013-08-02 at 12:03 +0200, Michel Dänzer wrote:
> > Could a similar change fix the same problem on desktop G5s? The same
> > values for slots_param in windfarm_pm112.c don't help, unfortunately.
> 
> You have a 11,2 and a noisy fan ? Odd, mine(s) don't.... I can dbl check
> the values vs. what Darwin uses tomorrow...

Yes, see /proc/cpuinfo below.

With older kernels (currently still using Debian's 3.2.0-4-powerpc64
because of this problem), the fans go basically silent as soon as the
windfarm modules are loaded. With current kernels, the fans stay at the
level OF sets them to, until after a while they go into 'airplane mode'.

One thing I notice now is that the lines like

[    9.539173] windfarm: CPUs control loops started.
[   16.209962] windfarm: Backside control loop started.
[   16.262274] windfarm: Slots control loop started.
[   16.371642] windfarm: Drive bay control loop started.

no longer appear in dmesg with current kernels. Should they?


processor	: 0
cpu		: PPC970MP, altivec supported
clock		: 2500.000000MHz
revision	: 1.1 (pvr 0044 0101)

processor	: 1
cpu		: PPC970MP, altivec supported
clock		: 2500.000000MHz
revision	: 1.1 (pvr 0044 0101)

processor	: 2
cpu		: PPC970MP, altivec supported
clock		: 2500.000000MHz
revision	: 1.1 (pvr 0044 0101)

processor	: 3
cpu		: PPC970MP, altivec supported
clock		: 2500.000000MHz
revision	: 1.1 (pvr 0044 0101)

timebase	: 33333333
platform	: PowerMac
model		: PowerMac11,2
machine		: PowerMac11,2
motherboard	: PowerMac11,2 MacRISC4 Power Macintosh 
detected as	: 337 (PowerMac G5 Dual Core)
pmac flags	: 00000000
L2 cache	: 1024K unified
pmac-generation	: NewWorld
Aaro Koskinen - Aug. 2, 2013, 3:58 p.m.
On Fri, Aug 02, 2013 at 04:47:51PM +0200, Michel Dänzer wrote:
> On Fre, 2013-08-02 at 22:51 +1000, Benjamin Herrenschmidt wrote:
> > On Fri, 2013-08-02 at 12:03 +0200, Michel Dänzer wrote:
> > > Could a similar change fix the same problem on desktop G5s? The same
> > > values for slots_param in windfarm_pm112.c don't help, unfortunately.
> > 
> > You have a 11,2 and a noisy fan ? Odd, mine(s) don't.... I can dbl check
> > the values vs. what Darwin uses tomorrow...
> 
> Yes, see /proc/cpuinfo below.
> 
> With older kernels (currently still using Debian's 3.2.0-4-powerpc64
> because of this problem), the fans go basically silent as soon as the
> windfarm modules are loaded. With current kernels, the fans stay at the
> level OF sets them to, until after a while they go into 'airplane mode'.
> 
> One thing I notice now is that the lines like
> 
> [    9.539173] windfarm: CPUs control loops started.
> [   16.209962] windfarm: Backside control loop started.
> [   16.262274] windfarm: Slots control loop started.
> [   16.371642] windfarm: Drive bay control loop started.
> 
> no longer appear in dmesg with current kernels. Should they?

Yes. They are printed once all the sensor and control modules are
loaded and registered. If the loops are not started, it's basically
doing nothing. Maybe something goes wrong in module loading? What lsmod
is showing?

A.
Michel Dänzer - Aug. 2, 2013, 4:52 p.m.
On Fre, 2013-08-02 at 18:58 +0300, Aaro Koskinen wrote:
> On Fri, Aug 02, 2013 at 04:47:51PM +0200, Michel Dänzer wrote:
> > On Fre, 2013-08-02 at 22:51 +1000, Benjamin Herrenschmidt wrote:
> > > On Fri, 2013-08-02 at 12:03 +0200, Michel Dänzer wrote:
> > > > Could a similar change fix the same problem on desktop G5s? The same
> > > > values for slots_param in windfarm_pm112.c don't help, unfortunately.
> > > 
> > > You have a 11,2 and a noisy fan ? Odd, mine(s) don't.... I can dbl check
> > > the values vs. what Darwin uses tomorrow...
> > 
> > Yes, see /proc/cpuinfo below.
> > 
> > With older kernels (currently still using Debian's 3.2.0-4-powerpc64
> > because of this problem), the fans go basically silent as soon as the
> > windfarm modules are loaded. With current kernels, the fans stay at the
> > level OF sets them to, until after a while they go into 'airplane mode'.
> > 
> > One thing I notice now is that the lines like
> > 
> > [    9.539173] windfarm: CPUs control loops started.
> > [   16.209962] windfarm: Backside control loop started.
> > [   16.262274] windfarm: Slots control loop started.
> > [   16.371642] windfarm: Drive bay control loop started.
> > 
> > no longer appear in dmesg with current kernels. Should they?
> 
> Yes. They are printed once all the sensor and control modules are
> loaded and registered. If the loops are not started, it's basically
> doing nothing. Maybe something goes wrong in module loading? What lsmod
> is showing?

Thanks for the suggestion. The same windfarm modules were loaded in both
cases, but i2c_powermac wasn't loaded with the newer kernels. Loading it
manually fixes the problem.

How is i2c_powermac supposed to get loaded with current kernels?
Benjamin Herrenschmidt - Aug. 2, 2013, 8:47 p.m.
On Fri, 2013-08-02 at 16:47 +0200, Michel Dänzer wrote:
> On Fre, 2013-08-02 at 22:51 +1000, Benjamin Herrenschmidt wrote:
> > On Fri, 2013-08-02 at 12:03 +0200, Michel Dänzer wrote:
> > > Could a similar change fix the same problem on desktop G5s? The same
> > > values for slots_param in windfarm_pm112.c don't help, unfortunately.
> > 
> > You have a 11,2 and a noisy fan ? Odd, mine(s) don't.... I can dbl check
> > the values vs. what Darwin uses tomorrow...
> 
> Yes, see /proc/cpuinfo below.
> 
> With older kernels (currently still using Debian's 3.2.0-4-powerpc64
> because of this problem), the fans go basically silent as soon as the
> windfarm modules are loaded. With current kernels, the fans stay at the
> level OF sets them to, until after a while they go into 'airplane mode'.
> 
> One thing I notice now is that the lines like
> 
> [    9.539173] windfarm: CPUs control loops started.
> [   16.209962] windfarm: Backside control loop started.
> [   16.262274] windfarm: Slots control loop started.
> [   16.371642] windfarm: Drive bay control loop started.
> 
> no longer appear in dmesg with current kernels. Should they?

Yes. Do you have all the windfarm modules loaded ? What about
i2c-powermac ? (It's typical that the latter is missed, I think it
doesn't auto-load, which we never fixed, we used to request distros to
just built it in)

Ben.

> 
> processor	: 0
> cpu		: PPC970MP, altivec supported
> clock		: 2500.000000MHz
> revision	: 1.1 (pvr 0044 0101)
> 
> processor	: 1
> cpu		: PPC970MP, altivec supported
> clock		: 2500.000000MHz
> revision	: 1.1 (pvr 0044 0101)
> 
> processor	: 2
> cpu		: PPC970MP, altivec supported
> clock		: 2500.000000MHz
> revision	: 1.1 (pvr 0044 0101)
> 
> processor	: 3
> cpu		: PPC970MP, altivec supported
> clock		: 2500.000000MHz
> revision	: 1.1 (pvr 0044 0101)
> 
> timebase	: 33333333
> platform	: PowerMac
> model		: PowerMac11,2
> machine		: PowerMac11,2
> motherboard	: PowerMac11,2 MacRISC4 Power Macintosh 
> detected as	: 337 (PowerMac G5 Dual Core)
> pmac flags	: 00000000
> L2 cache	: 1024K unified
> pmac-generation	: NewWorld
> 
>
Ben Hutchings - Aug. 2, 2013, 8:56 p.m.
On Sat, 2013-08-03 at 06:47 +1000, Benjamin Herrenschmidt wrote:
> On Fri, 2013-08-02 at 16:47 +0200, Michel Dänzer wrote:
> > On Fre, 2013-08-02 at 22:51 +1000, Benjamin Herrenschmidt wrote:
> > > On Fri, 2013-08-02 at 12:03 +0200, Michel Dänzer wrote:
> > > > Could a similar change fix the same problem on desktop G5s? The same
> > > > values for slots_param in windfarm_pm112.c don't help, unfortunately.
> > > 
> > > You have a 11,2 and a noisy fan ? Odd, mine(s) don't.... I can dbl check
> > > the values vs. what Darwin uses tomorrow...
> > 
> > Yes, see /proc/cpuinfo below.
> > 
> > With older kernels (currently still using Debian's 3.2.0-4-powerpc64
> > because of this problem), the fans go basically silent as soon as the
> > windfarm modules are loaded. With current kernels, the fans stay at the
> > level OF sets them to, until after a while they go into 'airplane mode'.
> > 
> > One thing I notice now is that the lines like
> > 
> > [    9.539173] windfarm: CPUs control loops started.
> > [   16.209962] windfarm: Backside control loop started.
> > [   16.262274] windfarm: Slots control loop started.
> > [   16.371642] windfarm: Drive bay control loop started.
> > 
> > no longer appear in dmesg with current kernels. Should they?
> 
> Yes. Do you have all the windfarm modules loaded ? What about
> i2c-powermac ? (It's typical that the latter is missed, I think it
> doesn't auto-load, which we never fixed, we used to request distros to
> just built it in)

We built it as a module in 3.2 and we still do.  The regression
apparently occurred between 3.8 and 3.9.

Ben.
Benjamin Herrenschmidt - Aug. 2, 2013, 9:04 p.m.
On Fri, 2013-08-02 at 22:56 +0200, Ben Hutchings wrote:
> > Yes. Do you have all the windfarm modules loaded ? What about
> > i2c-powermac ? (It's typical that the latter is missed, I think it
> > doesn't auto-load, which we never fixed, we used to request distros
> to
> > just built it in)
> 
> We built it as a module in 3.2 and we still do.  The regression
> apparently occurred between 3.8 and 3.9.

Interesting. Maybe the old MODULE_ALIAS statement in there no longer
work....

Ben.

Patch

diff --git a/drivers/macintosh/windfarm_rm31.c b/drivers/macintosh/windfarm_rm31.c
index 0b9a79b..82fc86a 100644
--- a/drivers/macintosh/windfarm_rm31.c
+++ b/drivers/macintosh/windfarm_rm31.c
@@ -439,15 +439,15 @@  static void backside_setup_pid(void)
 
 /* Slots fan */
 static const struct wf_pid_param slots_param = {
-	.interval	= 5,
-	.history_len	= 2,
-	.gd		= 30 << 20,
-	.gp		= 5 << 20,
-	.gr		= 0,
-	.itarget	= 40 << 16,
-	.additive	= 1,
-	.min		= 300,
-	.max		= 4000,
+	.interval	= 1,
+	.history_len	= 20,
+	.gd		= 0,
+	.gp		= 0,
+	.gr		= 0x00100000,
+	.itarget	= 3200000,
+	.additive	= 0,
+	.min		= 20,
+	.max		= 100,
 };
 
 static void slots_fan_tick(void)