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

Mitigate thermal issues with oryp7 #179

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

MilesBHuff
Copy link

@MilesBHuff MilesBHuff commented Apr 20, 2021

The oryp7 (and oryp6, which has the same everything (apart from the GPU)) has abysmal CPU thermals, reaching over 90'C at just 15% CPU use. Yet, in spite of this, and in spite of the fact that the heat pipe is shared with the GPU, only one fan turns on to cool the CPU. This PR adds the new config option from #177 to the oryp7, thus helping resolve the above issue.

The biggest problem with the fans, though, is undoubtedly the fact that they are not set to turn on until long after CPU throttling has already begun. Fans should reach full speed before the CPU starts throttling; and throttling is supposed to be a last resort, not a first one. In this vein, I have added the vastly improved fan curve from #135. This will prevent throttling, increase performance, and give lower temps.
However, it will also worsen the sudden and harsh spinup so-often complained-about in reviews of the oryp7. In order to resolve this, interpolation will need to be fixed and re-enabled (as in #139). However, the scope of that such a change is beyond the intention of this PR.
EDIT: I have gone ahead and just hard-coded the interpolations into the config. Good-enough, for now.
EDIT: The worsened spinup above was actually mostly due to the HEATUP/COOLDOWN settings from #135 being far too rapid.

@MilesBHuff MilesBHuff changed the title Fix fan issues with oryp7 Mitigate thermal issues with oryp7 Apr 20, 2021
@MilesBHuff
Copy link
Author

MilesBHuff commented Apr 20, 2021

You don't need to merge this PR as-is. This is largely a call to please implement sane defaults for the fan curves, ideally for all System76 computers. The fans are mentioned as a negative in every single Oryx Pro 6/7 review I've seen.
This PR works perfectly fine on my oryp7, and significantly lowers temperatures at idle. It lowers temperatures at low loads some, but the computer still starts throttling very quickly. I'm definitely going to have to repaste the CPU.

@curiousercreative
Copy link
Contributor

@MilesBHuff I'd suggest making this a draft PR if it's not ready for review. You can find a "converting to draft" text link in the right sidebar.

@MilesBHuff
Copy link
Author

Dropped e35f6a5 and squashed f077127. I'd say this is ready for review now.

@MilesBHuff
Copy link
Author

MilesBHuff commented Apr 20, 2021

I've set the floor speed to 25% to avoid the rattling that happened after #163 was fixed the first time. Unfortunately, the fan still rattles momentarily when turning off.

I've also halved the cooldown time, to avoid the fan blowing at 100% for over 10 seconds when the CPU is at 45'C (This happens every boot.). The new cooldown setting works nicely, but I'm thinking 12 or 13 might be better.

@MilesBHuff MilesBHuff marked this pull request as draft April 20, 2021 15:46
@MilesBHuff MilesBHuff marked this pull request as ready for review April 20, 2021 15:50
@MilesBHuff
Copy link
Author

MilesBHuff commented Apr 21, 2021

I've just given a full breakdown of the thought-processes behind this PR in #180.

@MilesBHuff
Copy link
Author

MilesBHuff commented May 1, 2021

Rebased onto latest code (now that #177 has been merged), and added support for smoothing (from #190 -- make sure to merge that before merging this).

@MilesBHuff
Copy link
Author

Rebased onto latest code (now that #180 has been merged).

@MilesBHuff MilesBHuff marked this pull request as ready for review June 16, 2021 06:08
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.

None yet

2 participants