Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fan: Replace interpolation, smoothing with stepping #496

Merged
merged 3 commits into from
Aug 29, 2024
Merged

Conversation

crawfxrd
Copy link
Member

@crawfxrd crawfxrd commented Aug 7, 2024

Fan noise is one of the top complaints reported. The existing
interpolation and smoothing logic has not sufficiently addressed the
issues with fans changing speeds too quickly in response to rapid
changes in thermals (particularly from PECI).

This behavior can be observed by with very basic tasks, such as:

  • Powering on a system and logging into GNOME
  • Starting a GUI application such as Firefox

Replace them with a fixed step update per event interval. Fans now have
a maximum amount they change change over time (3.9%/sec) as they move
towards a target duty.

This does not address fans constantly turning on/off at lowest duty point or providing cooling at higher duties to mitigate thermal throttling.

Impact: High
Rationale: If the rate of change is too slow it will not be able to handle a rapid rise in system temperatures, causing overheating. This may lead to sudden system power-off (PROCHOT#) and damage to board components.

Test

  • Evaluate if duty rate of change is responsive enough to handle sustained loads like compilation or gaming
  • Evaluate if noise is significantly reduced without compromising heat dissipation for short loads

@crawfxrd crawfxrd force-pushed the fan-step branch 3 times, most recently from 770f18a to 2f6c260 Compare August 8, 2024 17:30
Better define the scope of the tachometer variables by moving them to
the fan module. `fan_update_duty` is renamed to `fan_event` to reflect
that it handles more than just updating the PWM duties.

Signed-off-by: Tim Crawford <[email protected]>
Signed-off-by: Tim Crawford <[email protected]>
Fan noise is one of the top complaints reported. The existing
interpolation and smoothing logic has not sufficiently addressed the
issues with fans changing speeds too quickly in response to rapid
changes in thermals (particularly from PECI).

This behavior can be observed by with very basic tasks, such as:

- Powering on a system and logging into GNOME
- Starting a GUI application such as Firefox

Replace them with a fixed step update per event interval. Fans now have
a maximum amount they change change over time (3.9%/sec) as they move
towards a target duty.

Signed-off-by: Tim Crawford <[email protected]>
@crawfxrd crawfxrd requested review from a team August 26, 2024 15:08
@garrettjwilke
Copy link

garrettjwilke commented Aug 26, 2024

build fails for gaze18:

src/board/system76/gaze18/fan.c:7: syntax error: token -> ',' ; column 11
make: *** [src/arch/8051/toolchain.mk:52: build/board/system76/gaze18/fan.rel] Error 1

@crawfxrd
Copy link
Member Author

You are attempting to build wip/fan-rework. The branch for this is fan-step.

Copy link

@garrettjwilke garrettjwilke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

built and tested on serw13, oryp12, lemp13, galp5/6

@jackpot51 jackpot51 merged commit ae63a9e into master Aug 29, 2024
48 checks passed
@jackpot51 jackpot51 deleted the fan-step branch August 29, 2024 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants