LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [RFC] Dynamic fan clock divider changes (long)
@ 2004-05-16 20:28 Jean Delvare
  2004-05-16 21:36 ` Rutger Nijlunsing
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Jean Delvare @ 2004-05-16 20:28 UTC (permalink / raw)
  To: LM Sensors, LKML

Hi all,

This is a RFC about fan clock dividers in hardware monitoring chips.

### THE PROBLEM

Since the beginning of the lm78 project (now lm_sensors project, and
Linux 2.6 drivers), it has been assumed that users would be required to
manually set "fan divisors" of their hardware monitoring chipset in
order to have correct fan speed readings for their specific
configuration.

This has since become a FAQ, and is still one of the major source of
trouble and questions for fan speed measurements. This is why I am
suggesting that we could change our policy and have the proper clock
selection done by the drivers instead. I don't think I am the first one
to request that. I'm pretty sure I read a similar suggestion on this
list some times ago, but I can't remember who it came from.

The fact that various datasheets and documentation refer to it as "fan
divisors" has obviously added to the confusion. A more correct term
would be "fan clock divider", IMHO.

### ADDITIONAL POINTS TO CONSIDER

A second point to consider is that more and more people are willing to
change the speed of the fan depending of the temperature, most generally
using PWM (pulse width modulation), manual, software-driven or
hardware-driven. This means that a single divider may not be suitable
for the the full operating speed range. One particular point raised here
is that selecting the proper fan clock divider once and for all as the
driver loads is not sufficent. Also think of people plugging a new fan
afterwards, or changing fans. No way we want to force them to reload the
driver (which could be hard-built into the kernel anyway).

To add to complexity, remember that not only the measured fan speed has
to fit with the current clock divider, but the low fan speed limit has
to fit as well.

I have come to three possible implementations of various complexity,
depending on what we want to do, or not.

### WHAT YOU NEED TO KNOW

Here are a few reminders of how fan speed measurement work. It's
somewhat tricky, so even people with good knowledge are easily
mistaking. People with no knowledge wouldn't even have a chance to get
what I'm talking about without this summary ;) I'm voluntarily
simplifying a few things so as not to confuse newcomers.

Fan speeds are not measured directly. Instead, what is measured is how
much periods of clock have passed before the fan did a full revolution.
This value is stored in an 8-bit register. The actual speed (in RPM) is
given by (clock speed)*60/(register value). This means that you cannot
measure fans slower than (clock speed)*60/255. For this reason, it is
usually possible to divide the clock speed by 2, 4 or 8 (or even more)
so as to be able to measure slower fan speeds.

Usually, a low limit can be set by the user. If the fan spins under this
limit, an alarm is raised. This low limit is stored in an 8-bit
register, using a similar convention, so that the chipset can easily
compare the measured speed and the defined low limit. Note that a higher
value in the register means a slower fan, so there is an alarm condition
if the value in the measure register if *greater* than the value in the
low limit register. This may be confusing at first.

Example:
clock speed is 8kHz
the user sets the low limit to 2400 RPM
-> value in low limit register is 200 (8000*60/2400)
value read in the measure register is 150
-> real speed is 3200 RPM (8000*60/150)
-> no alarm is triggered
lower measurable speed is 1882 RPM (8000*60/255)

If we want to be able to measure a fan speed of, say, 1500RPM, we have
to set the clock divider to 2.

You may wonder why we don't directly use the greatest divider then. This
is *not* to be able to measure higher fan speeds. Even with a divider of
8, higher measurable fan speed is 60000 RPM (1000*60). This should be
sufficent for everyone ;) No, the reason is accuracy. It happens that
the measurement accuracy is proportional to the clock speed. More
precisely, the accuracy, defined as the lowest measurable RPM
difference, is computed as (speed*speed)/(clock*60). For a fan speed of
6000 RPM, the accuracy with a divider of 8 is no more than +/- 600 RPM.

This is why the choice of the best divider is important and non-trivial.
This is a lowest-measurable-speed vs accuracy-at-high-speed tradeoff.

Before I go on, please consider the following facts:

For a given fan speed, increasing the clock divider means decreasing the
value stored in the register.

It is always possible to increase the clock divider without losing low
limit value. We may have to round it but it'll fit.

If is not always possible to decrease the clock divider without losing
the low limit value. If the value in the register is >= 128, it won't
fit in an 8-bit register after we multiply it by 2 or more.

### PROPOSED IMPLEMENTATIONS

Implementation #1

Each time the user requests new data (which triggers an update of each
register value), the fan speed measurement register is analyzed. If the
value is too high (arbitrary limit to be defined), or if an overflow
flag has been set (for chipsets with such flags), the next higher clock
divider is selected (unless we already use the greatest available). The
fan min is preserved, possibly rounded. Else, if the value is too low
(arbitrary limit to be defined), the next lower clock divider is
selected (unless we already use the lowest available). The low limit is
preserved if possible, or lost and stored as 255.

This implementation solves all points mentioned above. The user doesn't
have to care, and the divider will change automatically if PWM is used,
or fan is changed. The drawback is that the selected low limit may be
lost is the process.

Concrete example (still using a 8kHz clock):

The user has a fan running at 1500 RPM with PWM, and has set low limit
to 1250 RPM. The driver will select a divider of 2 (neither 1250 nor
1500 RPM do fit with a divider of 1, lowest representable speed being
1882 RPM). Then the system goes hot and the fan goes full speed at 5000
RPM. The divider will be lowered to 1 to increase accuracy, which will
change the low limit to the lowest possible value (register == 255, i.e.
1882 RPM). Then, later, the temperature is low again and the fan returns
to low speed. The divider is back to 2, low limit is preserved to 1882,
and an alarm triggers as the measured speed is 1500 RPM.

An alternate possibility is to consider register == 255 as a special
case, which wouldn't be affected by increasing clock dividers.

In the example above, this would restore the low limit as 941 RPM
(lowest representable value with divider of 2) at the end. This won't
trigger an error, but still changed the value requested by the user.

This implementation is OK for the measured speed, but has side effects
on low limit.

Implementation #2

Same as #1, except that the low limit has to fit with the new clock
divider for a clock change to actually occur. Likewise, if the user
wants to set a new low limit which doesn't fit with the current divider,
the divider is changed.

Since the low limit is normally lower than the measured speed, this
means that the low limit is what will determine the divider. This means
that this implementation doesn't solve all the issues mentioned above.
The user still doesn't have to care, which is fine, but if the user has
a fan speed between 2000 and 5000 RPM, with low limit set to 1500 RPM,
he/she will have a "bad" accuracy at 5000 RPM (+/- 104 RPM). I see this
as the low limit "nailing" the divider ;)

The improvement here is that the clock is also changed if needed to
satisfy a requested low limit. Until now, the lowest representable speed
would have been set instead.

This is what I implemented in my new pc87360 driver (after trying #1). I
use 85 and 224 as the arbitrary limits for changing dividers.

Implementation #3

Same as #2, but we dissociate the requested low limit and the
register-stored low limit.

For example, say that the user requests a low limit of 1000 RPM but the
divider is currently 1 because it is the best tradeoff for measured
speed of 3000RPM. We will set the low limit register to 255, which
corresponds to the lowest measurable value with divider 1. But we
remember that the user wants 1000, and display this value too. If, at a
later time, the fan goes slower and the divider increases to 2, we will
remember that the user wants 1000, and store that value in the register
(now that we can). So the trick is fully invisible to the user.

The point here is that we start lying to the user. The low limit we
present isn't what is stored in the register anymore. He/she will
hopefully never notice it, but there are corner cases where it could
matter. For example, the BIOS could be messing with the chip at the same
time we are (we already saw this) and periodically force the low limit
to a given value. The user wouldn't notice (because we don't display the
speed from the actual register value) and alarms could trigger without a
visible reason. This may be because the BIOS is bad, but this would
still confuse the user. Another case is if fan speeds drops suddenly. By
the time the driver reacts and changes the clock divider, an alarm is
likely to trigger, while the user will not see any limit excess.

Each of the problems above are work-aroundable, but this will probably
significantly increase the amount of code in the driver, which is no
good.

Inplementation #4

Since implementation #2 looks rather good, and will practically result
in the clock divisor being set from the selected low limit, and never
change thereafter, choosing a constant (rather high) divider from the
very beginning may be reasonable after all. With a 8kHz clock, a divider
of 2 lets you measure speeds down to 941 RPM, which is most probably
fine for almost everyone. As far as I know, fans running below 1000 RPM
in normal conditions are quite rare. So we would simply choose an
arbitrary divider depending on the clock speed of each chipset. This is
by far the less code-expensive approach, of course, but may not be
correct in some critical cases (very slow or very fast fans).

### CONCLUSION

I think I'll stick to #2 for now. The extra code is reasonable, and I
don't really see the low accuracy at high speed as a problem. What
matters much to me is that the user shouldn't have to worry about
selecting dividers himself, and #2 does this.

Part of the extra code may be moved to i2c_sensor if several drivers use
it. Also, remember that the new policy will remove some code from the
drivers (reading and setting dividers manually) so this gives us some
margin anyway.

Comments welcome, of course.

Thanks.

-- 
Jean Delvare
http://khali.linux-fr.org/

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

* Re: [RFC] Dynamic fan clock divider changes (long)
  2004-05-16 20:28 [RFC] Dynamic fan clock divider changes (long) Jean Delvare
@ 2004-05-16 21:36 ` Rutger Nijlunsing
  2004-05-19 21:28   ` Jean Delvare
       [not found] ` <20040518021540.GB11012@earth.solarsys.private>
  2004-05-20 11:20 ` Paulo Marques
  2 siblings, 1 reply; 6+ messages in thread
From: Rutger Nijlunsing @ 2004-05-16 21:36 UTC (permalink / raw)
  To: LM Sensors, LKML

On Sun, May 16, 2004 at 10:28:09PM +0200, Jean Delvare wrote:
> Hi all,
> 
[snip]

Another implementation to add to the pool. Note I'm not convinced this
is the best solution, since I know little about the hardware
limitations...

Implementation #5

Since the divider can be programmed, it is possible to take two
measurements (all in the driver): first with the highest divider
allowed by the chipset to get an indication of the current speed, and
then a new measurement followed as close as possible to the first one
with a divider fitting the first measured speed.

Example: set divider to 64 (or highest possible divider); fan speed
gives 2500 +- 500. Not set divider to 2 or 1 and re-measure.

Advantages:
  - no 'low limit' has to be set magically
  - most accurate reading possible on the whole range
  - invisible to the user

Disadvantages:
  - no 'low limit' can be set in the hardware; low limit must be
    processed in software.
  - Update frequency of the fan speed will be halved.

Maybe-problem:
  - The 'as close as possible' must be in a small range or otherwise
    the fan speed may be dropping to fast to fall outside the
    measurable range. I do not know with which frequency the fan speed
    can be measured.

Processing the low limit in software will also solve the 'BIOS
triggering false alarms'.

> ### CONCLUSION
> 
> I think I'll stick to #2 for now. The extra code is reasonable, and I
> don't really see the low accuracy at high speed as a problem. What
> matters much to me is that the user shouldn't have to worry about
> selecting dividers himself, and #2 does this.

I agree with the stated 'I don't really see the low accuracy at high
speed as a problem', but this would suggest a simple
3-or-so-line-patch to solve the problems: just use the highest fan
divider by default. If a user really knows what he is doing, he can
change the divider himself.


-- 
Rutger Nijlunsing ---------------------------- rutger ed tux tmfweb nl
never attribute to a conspiracy which can be explained by incompetence
----------------------------------------------------------------------

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

* Re: [RFC] Dynamic fan clock divider changes (long)
  2004-05-16 21:36 ` Rutger Nijlunsing
@ 2004-05-19 21:28   ` Jean Delvare
  0 siblings, 0 replies; 6+ messages in thread
From: Jean Delvare @ 2004-05-19 21:28 UTC (permalink / raw)
  To: Rutger Nijlunsing; +Cc: sensors, linux-kernel

Hi Rutger,

> Implementation #5
> 
> Since the divider can be programmed, it is possible to take two
> measurements (all in the driver): first with the highest divider
> allowed by the chipset to get an indication of the current speed, and
> then a new measurement followed as close as possible to the first one
> with a divider fitting the first measured speed.

The idea is new to me, I have to admit it.

> Example: set divider to 64 (or highest possible divider); fan speed
> gives 2500 +- 500. Not set divider to 2 or 1 and re-measure.
> 
> Advantages:
>   - no 'low limit' has to be set magically

I'm not sure I get what you mean here.

>   - most accurate reading possible on the whole range

Correct. Same as #1 and #3 then.

>   - invisible to the user

Correct again, like all other proposed implementations.

> Disadvantages:
>   - no 'low limit' can be set in the hardware; low limit must be
>     processed in software.

Correct, same as #3. Except that with #3, the time will come when both
the fan speed measurement and the low limit fit in the same divider
value, so hardware limit can be restored. The fact that your
implementation completely disables the hardware limit is a problem.

>   - Update frequency of the fan speed will be halved.

This is another serious problem, methinks. And it doesn't quite match
your requirement of "as close as possible" above. If we simply alternate
two dividers, one per update, then there's no reason the fan speed won't
have changed inbetween. In a way, this is similar to my other proposals
in that matter, except that my implementations stick to the last "good"
value once they get it, so we don't have to change the divider often if
the fan is stable.

> Maybe-problem:
>   - The 'as close as possible' must be in a small range or otherwise
>     the fan speed may be dropping to fast to fall outside the
>     measurable range. I do not know with which frequency the fan speed
>     can be measured.

You're right here too, that's a problem. Some chips stop measuring
anything when accessed for register reads or writes. This is the reason
why all our hardware monitoring drivers cache the read values for some
time (typically 1 to 2 seconds). So reading the fan speed twice in the
same update, after changing the divider inbetween, is not an option. It
would add a very long delay to the update operation, it's just not worth
it.

> Processing the low limit in software will also solve the 'BIOS
> triggering false alarms'.

Not necessarily. If the BIOS restores an arbitrary low limit, it can
trigger. In any case, if the BIOS does this, it promises trouble, but if
we read the register value on each update and display it to the user (as
opposed to display a different one and process it in software), at least
the user will notice. This is a minor argument against your
implementation, and #3 as well.

Your proposal is very similar to #3, it seems. The idea is the same
(optimal accuracy all the time) and the problems are the same too. The
only advantage of yours is that you will get the "correct" divider value
at step 2, while it may take more than that in #3. But in the long run,
your method endlessly changes dividers while #3 stabilizes and doesn't.

> I agree with the stated 'I don't really see the low accuracy at high
> speed as a problem', but this would suggest a simple
> 3-or-so-line-patch to solve the problems: just use the highest fan
> divider by default. If a user really knows what he is doing, he can
> change the divider himself.

This is more or less implementation #4. Granted, this is by far the most
simple implementation. However, some chips have dividers up to 128. With
such a divider, readings at relatively high speed are *really* bad. At
5000 RPM you may have a resolution no better than +/- 2370 RPM, if I'm
not mistaking. We may not care about ultimate accuracy, but still... We
could decide that, for example, "32 should be enough for everyone" but
it may not be, some people have really slow fans, or fans emitting a
single tick per rotation. Deciding of one single divider will be fine
for most cases, but just not all, so this is probably not acceptable.
These people would be losing something from manual mode we have for now.

This is the reason why #2 has my preference. It does the best it can
with the user's hardware, while still being safe with regards to
hardware low limit and nasty BIOSes. The extra code required is
reasonable (IMHO), and the clock divider changes are limited to the
minimum required.

Thanks for your proposal, however.

-- 
Jean Delvare
http://khali.linux-fr.org/

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

* Re: [RFC] Dynamic fan clock divider changes (long)
       [not found] ` <20040518021540.GB11012@earth.solarsys.private>
@ 2004-05-20  9:03   ` Jean Delvare
  0 siblings, 0 replies; 6+ messages in thread
From: Jean Delvare @ 2004-05-20  9:03 UTC (permalink / raw)
  To: Mark M. Hoffman; +Cc: Sensors, LKML

> > I think I'll stick to #2 for now. The extra code is reasonable, and
> > I don't really see the low accuracy at high speed as a problem. What
> > matters much to me is that the user shouldn't have to worry about
> > selecting dividers himself, and #2 does this.
> > 
> > Part of the extra code may be moved to i2c_sensor if several drivers
> > use it. Also, remember that the new policy will remove some code
> > from the drivers (reading and setting dividers manually) so this
> > gives us some margin anyway.
> 
> The keyword in your conclusion is "policy", which reminds me of the
> phrase "mechanism, not policy".

I don't quite see how appropriate this phrase is in the context, and, to
say the truth, I'm not even sure I understand what you imply by saying
that. Could you please explain in great details why you seem to think
that my proposal is no good idea?

If the use of the word "policy" is incorrect, maybe I should have used
another. What I meant was that, if we start implementing that kind of
stuff in several drivers, we better discuss it together before, and
implement it the same, optimal way, so that we don't have to change it
afterwards, and it can be documented, and the user doesn't observe
different behaviors on different drivers. I'm trying to spare the user
some trouble, so let's not replace it with different trouble.

> The driver should behave exactly as commanded, nothing more.

Well, most hardware monitoring chip drivers do more than that, and I
don't think this should be changed. For example, drivers enable
monitoring on load and configure what needs to be. We have limited their
action to the least possible, and I think it was a clever move, but some
things definitely need to be done. Drivers may also need to reset alarm
flags after they read them. This belongs to the driver's job to do that!
Likewise, I believe that the fact that a clock divider has to be chosen
is a hardware implementation detail the user-space shouldn't even be
aware about.

> The solution to the fan divisor madness can and should be done in
> userspace.

I agree it could (the whole hardware monitoring chip drivers could).
However, the current interface doesn't allow that. The user-space would
need to know which dividers are available for each driver, and would
need to know the base clock frequency for each driver as well (or,
alternatively, we would have to export raw register readings for fan
tachometers). The amount of driver code required to do this would
probably be equal to, or even greater than, what I am proposing.

For information, my current implementation of the in-driver dynamic
clock adjustement is about 15 lines of code at update time, if you omit
comments and debugging, and 7 lines of code at low limit change time.
The current interface code to fan_div alone has about as much (about 22
lines)! If we are to export everything we need to do the same in
user-space, it will make the drivers even bigger, and add overhead at
the user-space/kernel-space interface. So why would we want to do that?

If you have a reason to believe that what I propose is not correct,
please explain why you do, with as much details as needed. I would also
want to read a concrete proposal of how we would do it your way. Simply
pretending that "it's not the correct way to do it" without additional
explanation nor proposal doesn't help me much, unfortunately.

I'm definitely surprised that you don't seem to like my proposal at all,
while I thought you were suggesting we could do something similar a few
months ago [1]. Quoting you back in January:

"(...) we'd auto-increment the fan_div until we got a non-zero reading
or we hit the max.  Then the first set to a fan_div turns the feature
off; that would allow sensors.conf/sensors -s to override."

Or do I misread you somehow?

Thanks.

[1] http://archives.andrew.net.au/lm-sensors/msg05815.html

-- 
Jean Delvare
http://khali.linux-fr.org/

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

* Re: [RFC] Dynamic fan clock divider changes (long)
  2004-05-16 20:28 [RFC] Dynamic fan clock divider changes (long) Jean Delvare
  2004-05-16 21:36 ` Rutger Nijlunsing
       [not found] ` <20040518021540.GB11012@earth.solarsys.private>
@ 2004-05-20 11:20 ` Paulo Marques
  2004-05-22 14:46   ` Jean Delvare
  2 siblings, 1 reply; 6+ messages in thread
From: Paulo Marques @ 2004-05-20 11:20 UTC (permalink / raw)
  To: LM Sensors, LKML

Jean Delvare wrote:
> The user still doesn't have to care, which is fine, but if the user has
> a fan speed between 2000 and 5000 RPM, with low limit set to 1500 RPM,
> he/she will have a "bad" accuracy at 5000 RPM (+/- 104 RPM). I see this
> as the low limit "nailing" the divider ;)

This doesn't sound so bad at all. And this seems to be the simplest approach.

> This is what I implemented in my new pc87360 driver (after trying #1). I
> use 85 and 224 as the arbitrary limits for changing dividers.

This confused me a bit. It seems that a direct consequence of implementation #2 
is that the divider will be set in a way that the low limit will be between 128 
and 255, and that there is no point in changing the divider, because it will 
only get worse. This leads directly to implementation #4. Am I missing something?

Anyway, if the user is really concerned about accuracy an average of several 
measurements should increase precision in this kind of problem. See the 
following ascii art:

clock:
     ___     ___     ___     ___     ___     ___     ___     ___
___|   |___|   |___|   |___|   |___|   |___|   |___|   |___|   |__

fan complete turn pulse:

____________________|_____________________|_____________________|

So the first measurement is 3 (assuming rising edge counting), but some of the 
period will slip into the second measurement giving a count of only 2, and so 
on. So the jitter on the counts is actually correlated with the fan speed.

Just my 2 cents,

-- 
Paulo Marques - www.grupopie.com
"In a world without walls and fences who needs windows and gates?"

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

* Re: [RFC] Dynamic fan clock divider changes (long)
  2004-05-20 11:20 ` Paulo Marques
@ 2004-05-22 14:46   ` Jean Delvare
  0 siblings, 0 replies; 6+ messages in thread
From: Jean Delvare @ 2004-05-22 14:46 UTC (permalink / raw)
  To: Paulo Marques; +Cc: sensors, linux-kernel

> > The user still doesn't have to care, which is fine, but if the user
> > has a fan speed between 2000 and 5000 RPM, with low limit set to
> > 1500 RPM, he/she will have a "bad" accuracy at 5000 RPM (+/- 104
> > RPM). I see this as the low limit "nailing" the divider ;)
> 
> This doesn't sound so bad at all. And this seems to be the simplest
> approach.

Agreed.

> > This is what I implemented in my new pc87360 driver (after trying
> > #1). I use 85 and 224 as the arbitrary limits for changing
> > dividers.
> 
> This confused me a bit. It seems that a direct consequence of
> implementation #2 is that the divider will be set in a way that the
> low limit will be between 128 and 255, and that there is no point in
> changing the divider, because it will only get worse.

You're absolutely right.

> This leads directly to implementation #4. Am I missing something?

You are. In #4, the divider is arbitrarily chosen by "us", regardless of
the user's setup. In #2, the divider is chosen according to the user's
hardware and fan use. The common point is that (after setting the low
limit for #2) the divider will no longer change (until the low limit
changes for #2). But in #2 the divider is optimal, in #4 it is
arbitrary.

This makes a big difference because we cannot possibly arbitrarily pick
a divider and not allow the user to change it, so in #4 we would have to
keep the manual interface as well, as it exists for now. For #2, we can
reasonably hope to get rid of the manual interface after some times
(once the automatic mode will have been tested and is believed to be
correct).

> Anyway, if the user is really concerned about accuracy an average of
> several measurements should increase precision in this kind of
> problem.

Yes, that's a possibility. Not sure it's even worth the extra code, but
someone motivated could do it, you're right.

Thanks for your comments :)

-- 
Jean Delvare
http://khali.linux-fr.org/

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

end of thread, other threads:[~2004-05-22 14:46 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2004-05-16 20:28 [RFC] Dynamic fan clock divider changes (long) Jean Delvare
2004-05-16 21:36 ` Rutger Nijlunsing
2004-05-19 21:28   ` Jean Delvare
     [not found] ` <20040518021540.GB11012@earth.solarsys.private>
2004-05-20  9:03   ` Jean Delvare
2004-05-20 11:20 ` Paulo Marques
2004-05-22 14:46   ` Jean Delvare

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).