hi, > I'd like to keep it simple stupid. If you want a stepping, you'll end up > with a lot of code to handle hysteresis - which is, don't slow down the > fan as soon as the temp is going under one limit (or the other way) or > your fan will change speed every two seconds. Which is very > annoying to hear...
hmm, see, personally I find it more annoying to have no fan at all because I'm just reading mail or writing some absolutely crazy ocaml code while heat gradually builds up. then suddenly, out of nowhere, the fan kicks in really loud. the only choice right now is to set the fan_speed to a lower value, however in that case it becomes more likely to reach lim+8 rather quickly and then the fan reeaally kicks in... I'd just like some more configuration options so that I can decide on my own what's annoying and what's not... but it's right, the module basically does its job pretty well, it's just the stepping that I miss... therefore I've slightly modified it to make it more configurable (patch against 2.6.9 attached) 1) you get 3 new parameters: limit_override: like limit_adjust, but sets the temperature at which we force full speed fan_step: the step in fan speed that we want to have step_degrees: the temperature step after which we increase the fan_speed by fan_step. thus, we get stepping starting at limit_adjust, but once limit_override is reached, we force full speed again. the hysteresis problem is not really addressed, instead we will simply "lag" one step when reducing the fan speed in order to prevent cases where temp falls one step, we reduce the fan speed, then immediately the temp jumps up a step again -> we'll rather wait until the temp drops 2 steps and then we reduce the fan speed by one step. At lim-2, the fan stops completely. If you do not set the new parameters upon module insertion or via sysfs, the module behaves just like the original one does... I've just tried a step_degrees = 1, fan_step = 20, fan_speed = 60, limit_adjust = -3 and limit_override = 6. for my taste, it works the way I want it to right now (on a 12" powerbook), although the parameters surely can be tweaked again... when you start up cpu-intensive tasks, the fan speed gradually increases (but that's what I wanted), when you are doing non-intensive tasks, the fan only runs at low speeds like 60 or 80, being really quiet. "Jumping" between two values hasn't happened yet... still, I outcommented the printk when setting a new fan speed in order to keep the syslog clean... I'd recommend to try it out, if you like it I may submit it... regards, georg kaindl
--- linux-2.6.9/drivers/macintosh/therm_adt746x.c.orig 2004-10-26 19:06:04.699402232 +0200 +++ linux-2.6.9/drivers/macintosh/therm_adt746x.c 2004-10-26 19:05:05.506400936 +0200 @@ -48,6 +48,9 @@ static u8 default_limits_chip[3] = {80, static int limit_adjust = 0; static int fan_speed = -1; +static int limit_override = 8; +static int fan_step = 0; +static int step_degrees = 2; MODULE_AUTHOR("Colin Leroy <[EMAIL PROTECTED]>"); MODULE_DESCRIPTION("Driver for ADT746x thermostat in iBook G4 and Powerbook G4 Alu"); @@ -57,12 +60,20 @@ MODULE_PARM(limit_adjust,"i"); MODULE_PARM_DESC(limit_adjust,"Adjust maximum temperatures (50 cpu, 70 gpu) by N degrees."); MODULE_PARM(fan_speed,"i"); MODULE_PARM_DESC(fan_speed,"Specify fan speed (0-255) when lim < temp < lim+8 (default 128)"); +MODULE_PARM(limit_override,"i"); +MODULE_PARM_DESC(limit_override,"Temperature offset at which the fans should be set to full speed"); +MODULE_PARM(fan_step,"i"); +MODULE_PARM_DESC(fan_step,"Increment to fan_speed that is applied at every <step_degrees> degree increment in temperature"); +MODULE_PARM(step_degrees,"i"); +MODULE_PARM_DESC(step_degrees,"The temperature step after which the fan_speed gets increased by <fan_step>, or decreased respectively"); + struct thermostat { struct i2c_client clt; u8 cached_temp[3]; u8 initial_limits[3]; u8 limits[3]; + u8 limits_override[3]; int last_speed[2]; int overriding[2]; }; @@ -203,9 +214,12 @@ static void write_fan_speed(struct therm if (speed == -1) printk(KERN_INFO "adt746x: Setting speed to: automatic for %s fan.\n", fan?"GPU":"CPU"); + /* too noisy on the syslog, therefore commented out */ + /* else printk(KERN_INFO "adt746x: Setting speed to: %d for %s fan.\n", speed, fan?"GPU":"CPU"); + */ } else return; @@ -232,6 +246,7 @@ static int monitor_task(void *arg) struct thermostat* th = arg; u8 temps[3]; u8 lims[3]; + u8 lims_override[3]; int i; #ifdef DEBUG int mfan_speed; @@ -248,8 +263,9 @@ static int monitor_task(void *arg) if (fan_speed != -1) { #endif for (i = 0; i < 3; i++) { - temps[i] = read_reg(th, TEMP_REG[i]); - lims[i] = th->limits[i]; + temps[i] = read_reg(th, TEMP_REG[i]); + lims[i] = th->limits[i]; + lims_override[i] = th->limits_override[i]; } #ifndef DEBUG } @@ -259,20 +275,28 @@ static int monitor_task(void *arg) for (i = 1; i < 3; i++) { /* we don't care about local sensor */ int started = 0; int fan_number = (therm_type == ADT7460 && i == 2); - int var = temps[i] - lims[i]; - if (var > 8) { + int var = temps[i] - lims[i]; + int var_o = temps[i] - lims_override[i]; + if (var_o > 0) { if (th->overriding[fan_number] == 0) - printk(KERN_INFO "adt746x: Limit exceeded by %d, overriding specified fan speed for %s.\n", - var, fan_number?"GPU":"CPU"); + printk(KERN_INFO "adt746x: Override-Limit exceeded by %d, overriding specified fan speed for %s.\n", + var_o, fan_number?"GPU":"CPU"); th->overriding[fan_number] = 1; write_fan_speed(th, 255, fan_number); started = 1; - } else if ((!th->overriding[fan_number] || var < 6) && var > 0) { + } else if ((!th->overriding[fan_number] || var_o < -2) && var > 0) { + int new_fan_speed = fan_speed + (var - 1)/step_degrees * fan_step; + + /* don't decrease the fan speed immediately, let the temp drop another step_degrees first */ + /* note that the case in which temp < lim-2 is not handled here but in the next if statement */ + if (new_fan_speed == th->last_speed[fan_number]-fan_step) continue; + else if (new_fan_speed < th->last_speed[fan_number]) new_fan_speed += fan_step; + if (th->overriding[fan_number] == 1) - printk(KERN_INFO "adt746x: Limit exceeded by %d, setting speed to specified for %s.\n", - var, fan_number?"GPU":"CPU"); + printk(KERN_INFO "adt746x: Limit exceeded by %d, setting speed to %d for %s.\n", + var, new_fan_speed, fan_number?"GPU":"CPU"); th->overriding[fan_number] = 0; - write_fan_speed(th, fan_speed, fan_number); + write_fan_speed(th, new_fan_speed, fan_number); started = 1; } else if (var < -1) { /* don't stop iBook fan if GPU is cold and CPU is not @@ -325,6 +349,7 @@ set_limit(struct thermostat *th, int i) /* set our limits to normal */ th->limits[i] = default_limits_local[i] + limit_adjust; + th->limits_override[i] = default_limits_local[i] + limit_override; } static int @@ -375,6 +400,10 @@ attach_one_thermostat(struct i2c_adapter th->initial_limits[0], th->initial_limits[1], th->initial_limits[2], th->limits[0], th->limits[1], th->limits[2]); + printk(KERN_INFO "adt746x: limit_adjust = %d, limit_override = %d, " + "fan_speed = %d, fan_step = %d, step_degrees = %d\n", + limit_adjust, limit_override, fan_speed, fan_step, step_degrees); + thermostat = th; if (i2c_attach_client(&th->clt)) { @@ -426,28 +455,31 @@ static ssize_t show_##name(struct device ); \ } -#define BUILD_STORE_FUNC_DEG(name, data) \ +#define BUILD_STORE_FUNC_DEG(name, data, logstr) \ static ssize_t store_##name(struct device *dev, const char *buf, size_t n) \ { \ int val; \ int i; \ val = simple_strtol(buf, NULL, 10); \ - printk(KERN_INFO "Adjusting limits by %d°C\n", val); \ - limit_adjust = val; \ + printk(KERN_INFO "adt746x: " logstr, val); \ + name = val; \ for (i=0; i < 3; i++) \ set_limit(thermostat, i); \ return n; \ } -#define BUILD_STORE_FUNC_INT(name, data) \ +#define BUILD_STORE_FUNC_INT(name, data, logstr) \ static ssize_t store_##name(struct device *dev, const char *buf, size_t n) \ { \ u32 val; \ val = simple_strtoul(buf, NULL, 10); \ if (val < 0 || val > 255) \ return -EINVAL; \ - printk(KERN_INFO "Setting specified fan speed to %d\n", val); \ + printk(KERN_INFO "adt746x: " logstr, val); \ data = val; \ + if (step_degrees == 0) { \ + step_degrees = 1; \ + } \ return n; \ } @@ -455,14 +487,22 @@ BUILD_SHOW_FUNC_INT(cpu_temperature, (r BUILD_SHOW_FUNC_INT(gpu_temperature, (read_reg(thermostat, TEMP_REG[2]))) BUILD_SHOW_FUNC_INT(cpu_limit, thermostat->limits[1]) BUILD_SHOW_FUNC_INT(gpu_limit, thermostat->limits[2]) +BUILD_SHOW_FUNC_INT(cpu_limit_override, thermostat->limits_override[1]) +BUILD_SHOW_FUNC_INT(gpu_limit_override, thermostat->limits_override[2]) BUILD_SHOW_FUNC_INT(specified_fan_speed, fan_speed) +BUILD_SHOW_FUNC_INT(specified_fan_step, fan_step) +BUILD_SHOW_FUNC_INT(specified_step_degrees, step_degrees) BUILD_SHOW_FUNC_FAN(cpu_fan_speed, 0) BUILD_SHOW_FUNC_FAN(gpu_fan_speed, 1) -BUILD_STORE_FUNC_INT(specified_fan_speed,fan_speed) +BUILD_STORE_FUNC_INT(specified_fan_speed,fan_speed,"Setting specified fan speed to %d\n") +BUILD_STORE_FUNC_INT(specified_fan_step,fan_step,"Setting specified fan step to %d\n") +BUILD_STORE_FUNC_INT(specified_step_degrees,step_degrees,"Setting fan speed increment temperature threshold to %d\n") BUILD_SHOW_FUNC_INT(limit_adjust, limit_adjust) -BUILD_STORE_FUNC_DEG(limit_adjust, thermostat) +BUILD_SHOW_FUNC_INT(limit_override, limit_override) +BUILD_STORE_FUNC_DEG(limit_adjust, thermostat,"Adjusting limits by %d°C\n") +BUILD_STORE_FUNC_DEG(limit_override, thermostat,"Adjusting override limits by %d°C\n") static DEVICE_ATTR(cpu_temperature, S_IRUGO, show_cpu_temperature,NULL); @@ -472,10 +512,17 @@ static DEVICE_ATTR(cpu_limit, S_IRUGO, show_cpu_limit, NULL); static DEVICE_ATTR(gpu_limit, S_IRUGO, show_gpu_limit, NULL); +static DEVICE_ATTR(cpu_limit_override, S_IRUGO, + show_cpu_limit_override,NULL); +static DEVICE_ATTR(gpu_limit_override, S_IRUGO, + show_gpu_limit_override,NULL); static DEVICE_ATTR(specified_fan_speed, S_IRUSR|S_IWUSR|S_IRGRP|S_IROTH, show_specified_fan_speed,store_specified_fan_speed); +static DEVICE_ATTR(specified_fan_step, S_IRUSR|S_IWUSR|S_IRGRP|S_IROTH, + show_specified_fan_step,store_specified_fan_step); + static DEVICE_ATTR(cpu_fan_speed, S_IRUGO, show_cpu_fan_speed, NULL); static DEVICE_ATTR(gpu_fan_speed, S_IRUGO, @@ -484,6 +531,11 @@ static DEVICE_ATTR(gpu_fan_speed, S_IRUG static DEVICE_ATTR(limit_adjust, S_IRUSR|S_IWUSR|S_IRGRP|S_IROTH, show_limit_adjust, store_limit_adjust); +static DEVICE_ATTR(limit_override, S_IRUSR|S_IWUSR|S_IRGRP|S_IROTH, + show_limit_override, store_limit_override); + +static DEVICE_ATTR(specified_step_degrees,S_IRUSR|S_IWUSR|S_IRGRP|S_IROTH, + show_specified_step_degrees, store_specified_step_degrees); static int __init thermostat_init(void) @@ -521,9 +573,14 @@ thermostat_init(void) device_create_file(&of_dev->dev, &dev_attr_gpu_temperature); device_create_file(&of_dev->dev, &dev_attr_cpu_limit); device_create_file(&of_dev->dev, &dev_attr_gpu_limit); + device_create_file(&of_dev->dev, &dev_attr_cpu_limit_override); + device_create_file(&of_dev->dev, &dev_attr_gpu_limit_override); device_create_file(&of_dev->dev, &dev_attr_limit_adjust); device_create_file(&of_dev->dev, &dev_attr_specified_fan_speed); device_create_file(&of_dev->dev, &dev_attr_cpu_fan_speed); + device_create_file(&of_dev->dev, &dev_attr_specified_fan_step); + device_create_file(&of_dev->dev, &dev_attr_limit_override); + device_create_file(&of_dev->dev, &dev_attr_specified_step_degrees); if(therm_type == ADT7460) device_create_file(&of_dev->dev, &dev_attr_gpu_fan_speed);