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

ACPI fan duty setting to enable userspace fan pwm control #365

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

curiousercreative
Copy link

@curiousercreative curiousercreative commented Oct 18, 2022

Submodule PRs,

Dependent PR:

Along with related PRs, would resolve/improve:

With the proposed changes applied (from three repos), I'm able to set a single fan's speed to 100% with the following:

echo 255 | sudo tee /sys/class/hwmon/hwmon2/pwm1

This can be improved with a convenience script that would take fan speed as a percent and apply to both fans:

~/.local/bin/fanspeed.sh 100

You can create the convenience script with the following:

cat << EOF > ~/.local/bin/fanspeed.sh
#! /usr/bin/env bash
echo "($1 * 255) / 100" | bc | sudo tee /sys/class/hwmon/hwmon2/pwm1 | sudo tee /sys/class/hwmon/hwmon2/pwm2
EOF
chmod u+x ~/.local/bin/fanspeed.sh

You may also want to use the thinkfan package to build your own fan curves in userspace. I've experimented with adding fan curves to the system76-power package as well, but found that fan smoothing was eating a lot of CPU when done in userspace and it wasn't all that smooth anyhow. Probably a combination of userspace fan curves and EC fan speed smoothing and flooring could be worth exploring.

@John-Bronson
Copy link

userspace control over the fan curves would be a huge boost in user friendliness! Appreciate the work.

@curiousercreative
Copy link
Author

curiousercreative commented Jul 11, 2023

In my current install, writing fan speeds to the sysfs is not working. I'm troubleshooting to determine where in the chain of system76_acpi_dkms > coreboot > ec the problem is. The sysfs is writable, I don't really know how to debug coreboot once it builds, and the ectool console command which would allow me to debug the ec isn't working right now.

UPDATE: I hadn't flashed the coreboot changes. I thought I had, but I think I may have run ./scripts/deps.sh after I had checked out the coreboot submodule. All is well with coreboot changes flashed.

@curiousercreative curiousercreative changed the base branch from master to tgl-u-s3 July 11, 2023 18:35
@curiousercreative curiousercreative changed the base branch from tgl-u-s3 to master July 11, 2023 18:35
@curiousercreative
Copy link
Author

freshly rebased and tested (same for EC and coreboot submodules)

@curiousercreative
Copy link
Author

curiousercreative commented Aug 18, 2023

I'd like to share how I've been using this feature on my galp5 because of course I run this firmware!

It's the hot summer months and to keep it cool (and thereby keeping energy usage and costs down) I have been running my galp5 laptop docked rather than running my desktop. I'm fortunate to have a nice 5K2K ultrawide monitor that connects over Thunderbolt, so I only have to switch the one cable to go from laptop to desktop or vice versa. My launch keyboard and webcam are connect to the monitor, so it all goes where the monitor goes.

Anyhow, the laptop is an ultralight and while I'm out and about, I like it to be nice and silent (easy on battery where the powercap is about 40% lower than AC powered) but docked I'd like to tap more of that power and run a higher fan speed floor. I've got a little script for modifying some settings for docked vs undocked use and now I've got that script setting a fan speed floor (via this PR featrue) of 40%. So now I've got a better thermal performance! If I'm doing something that is intermittently more demanding (maybe a file watcher is trigger builds), I might increase that fan speed floor to 60% and turn the music up a bit :)

@crawfxrd crawfxrd requested review from a team August 18, 2023 18:02
@leaf-node
Copy link

You may also want to use the thinkfan package to build your own fan curves in userspace. I've experimented with adding fan curves to the system76-power package as well, but found that fan smoothing was eating a lot of CPU when done in userspace and it wasn't all that smooth anyhow. Probably a combination of userspace fan curves and EC fan speed smoothing and flooring could be worth exploring.

Hey, I'm interested in your code. Does the performance issue apply when using thinkfan alone? Or does it only occur when trying to precisely smooth the fan in user space? I wonder if that question is a blocker to upstream approving your patch.

My hope is that with this change, and a thinkfan script, I'll be able to eliminate the initial very slow, then the delayed and super loud fan ramp-up. (I'm also considering throttling the max frequency to avoid this issue).

Thanks! : )

@curiousercreative
Copy link
Author

@leaf-node I don't know why userspace smoothing resulted in load. Maybe it was iowait? It'd be worth exploring an surrendering userspace implementation besides system76-power

@leaf-node
Copy link

@leaf-node I don't know why userspace smoothing resulted in load. Maybe it was iowait? It'd be worth exploring an surrendering userspace implementation besides system76-power

But is there any performance issue if one only uses the default smoothing in the EC, and uses thinkfan with this patch that adds control via /sys?

@curiousercreative
Copy link
Author

@leaf-node there shouldn't be. I believe the load I observed was a result of too frequently updating the fan speed. thinkfan or similar updating the speed at an interval of a second or more probably won't result in any noticeable load. I look forward to your report though, I only briefly tested thinkfan and fancontrol. I spent awhile trying to making system76-power work, but ultimately I've only found myself setting a fan speed floor imperatively.

@itsTheFae
Copy link

itsTheFae commented Oct 12, 2023

Recently purchased an addw3 and I must say I was hoping that fan control was already a configurable feature.
I haven't worked up the confidence to try building this for myself yet. This will be a fine place to start when I get fed up with the default fan curve and Fn+1 not being what I want.

Thanks for this fine work!

@curiousercreative
Copy link
Author

@itsTheFae building and flashing EC firmware on these is quite painless once you overcome the initial mental hurdle. You'll clone the repo, checkout the right commit, install the tooling dependencies, modify a single untracked config file (for modifying fan curve), and run a single command to build and flash. If build fails, nothing is flashed. If it succeeds, your machine powers down and when you power up you're running new EC firmware!

Judging by how long this has been sitting around without serious engagement from S76, I imagine this isn't something they want, at least not in how I've implemented it (ACPI). All to say, I wouldn't hold my breath!

@akompanas
Copy link

@curiousercreative But if this custom curve solves a temperature issue, then this is a very needed change.

I have darp8 and generally like this laptop, but due to pour thermal design I cannot really use it on, you know, my lap for more than half an hour.

I know the Fn+1 override and it solves the temperature problem, but it makes the machine sound like a vacuum cleaner.

This is a serious usability issue.

@curiousercreative
Copy link
Author

@akompanas I don't disagree with you and I'd love to see this work merged to allow userspace control over fan speeds, but you have access to the firmware and S76 has provided us with tools to modify and flash that firmware to our devices without any additional hardware.

Also, while userspace fan control would be great, your model likely needs a better default fan curve configured in firmware. I see your device has no custom curve defined so it's using this default fan curve.

@akompanas
Copy link

I’m a bit afraid to flash newest firmware because the last time I updated the newest official release my computer started crashing instead of going to sleep and I had to roll back.

@itsTheFae
Copy link

@curiousercreative That certainly makes me feel better about trying it, the initial read I did of the docs made it seem simple enough to do but the warning about brick recovery gave me some pause.

I would enjoy user-space control more than just hard-coded curves. But I'll get familiar with the process keeping it simple first. Then maybe try making the needed changes to what you have here.

@curiousercreative
Copy link
Author

curiousercreative commented Oct 16, 2023

I’m a bit afraid to flash newest firmware because the last time I updated the newest official release my computer started crashing instead of going to sleep and I had to roll back.

@akompanas you must've been working from master rather than from the last release for your device. They use a single repo for all devices, but release specific versions after thorough QA. Not to say regressions haven't happened... Try following this section labeled "Firmware version" here to be sure you're basing changes off the firmware your device currently runs.

ec: relay SFD0 and SFD1 values to fan controller and take the maximum of the EC fan duty and the ACPI fan duty
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.

5 participants