flukejones / rog-core

Implements missing functionality for ASUS Zephyrus GX502, GX502, GX531, G712, G53, GA14/GA401, GA15/GA502 such as keyboard LED control, RGB modes, AniMe display control, and missing fn functions.
Mozilla Public License 2.0
255 stars 25 forks source link

Custom fan curve support #33

Open Yarn opened 4 years ago

Yarn commented 4 years ago

I'm planning to submit a PR for this so this ticket is mainly for figuring out the details of how it should work.

UX

rog-core cli command

rog-core --fan-curve 30c:0%,40c:1%,50c:4%,60c:10%,70c:40%,80c:60%,90c:100%,100c:100% --fan-curve-force
rog-core fan normal -f --curve '30c:0%,40c:1%,50c:4%,60c:10%,70c:40%,80c:60%,90c:100%,100c:100%'

This will set the config for the current profile and apply the fan curve. Potentially --fan-curve default to clear the custom curve.

I put --fan-curve-force as a separate parameter since that looks easy to add to the existing cli code. It may be better to split fan control into its own sub command though.


Proposed config format

"silent": {
    "min_percentage": 0,
    "max_percentage": 100,
    "no_turbo": true,
    "fan_curve": {
        "curve": "30c:0%,40c:1%,50c:4%,60c:10%,70c:40%,80c:60%,90c:100%,100c:100%",
        "force": true
    }
}

The fan curve settings will be optional (no error if they aren't present, will be saved as null).

If fan curve isn't set it will do the current behavior.

Fan curve uses the same format as atrofac. If fan curve force is false the safety limits used by atrofac/armoury crate will be enforced.

Overriding the board name will not be allowed to avoid issues with incompatibility coming back to rog-core.

Implementation

I've created a crate https://github.com/Yarn/rog_fan_curve that will handle the board specific implementation. It uses the acpi_call kernel module internally.

There is an outstanding issue with the fans changing rpm constantly (also happens on windows). https://github.com/cronosun/atrofac/issues/5

flukejones commented 4 years ago

The overall layout looks good. One thing I'd like to see is that the related data:

    "fan_curve": "30c:0%,40c:1%,50c:4%,60c:10%,70c:40%,80c:60%,90c:100%,100c:100%",
    "fan_curve_board": "GA401IV",
    "fan_curve_force": true

here is put in a struct that is Optional. This way we can prevent further clutter of the users config if acpi_call doesn't exist or they don't have a supported laptop.

Yarn commented 4 years ago

updated the original post. removed board setting as discussed in discord.

I'm thinking it might be better to have fan control as a sub command instead of a bunch of flags.

flukejones commented 4 years ago

Can you have a look at branch next please. I've refactored a lot of things and encapsulated + modularised as much as I can as a result of the coming kernel patch work. It should hopefully make things very clean and simple.

fariquelme commented 4 years ago

@Yarn How do i use your fan curve rog client?, i saw you also have a fork of rog-core with the fan curve flag implemented (branch:fan_curve). But it looks like it's missing out the latest changes, are you going to supporting this project ? i'd like to try it out.

Yarn commented 4 years ago

You'll need to join the discord for instructions on using it currently (you can find them by searching for "and change ExecStart= to the path"). The current version in my repo is pretty much just what i threw together to use myself. Note that only the GA401IV is supported currently. Other models probably work but I haven't had anyone fully confirm that yet (if you want to test another model that would be appreciated but I haven't written up the instructions outside discord yet).

The changes will get finished and merged in when I get to it (although probably only into the version needing the kernel patches).