Convert kernel hwmon interface to use PWM (0-255) intsead of RPM v2 #268
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 12 commits
October 13, 2024 22:45
|
@MrDuartePT Have you encountered any errors in the meantime when using this version? Otherwise, please merge it. |
|
@MrDuartePT @johnfanv2 This PR broke fan speed reading/writing on Legion 7 (not slim) 2022 (16ARHA7). (It is broken in current main too). Working dmesg output: Dmesg with this commit: When trying to set the speed, it gets set to something different, I guess it's bad pwm/rpm conversion: I'm happy to help testing/debugging the fix/issue, I'm also on the Legion Discord channel. |
|
@MrDuartePT Hmm, I tried to use the GUI too but it didn't work, so I tried to use the CLI according to the README which instructed me to use such The temperatures are not read correctly. The fan speeds are close to the default (1600, 1700, ...) values but when I try to change them they are overwritten to something different (e.g For the commit before this PR, it shows the same values as in the CLI and I can succcessfully change them. |
|
Yes the readme is outdated for The difference you see in the values is because of the rounding of the pwm value and conversion again to rpm, by default the python rounds off to the nearest lower number. Also looking at the debug output after and before the pwm PR you only have 3 fan curve points, so the ec_registers are not correct for you model they need to be change. So even this PR didn't exist the fan curve control for you model is still broken. |
|
@MrDuartePT I got it, thanks. |
I think a hexdump is required. Post a hexdump with the fancurve lock and unlock in performance mode (I look again at the issue and when the fancurve is locket the 10 values appear) Also see if in custom mode (purple color) you can see the 10 fancurve values with the fancurve unlock. Also check if you have minifancurve disable |
|
@MrDuartePT There are no values when unlocked and minifancurve is disabled (in custom mode): Now in performance mode (red color): Unlocked and minifancurve disabled (10 values but I cannot set them, they are defaulting after write to the default value): EC memory hexdump
Unlocked, minifancurve enabled (only 3 values): EC memory hexdump
Locked, minifancurve enabled (only 3 values): EC memory hexdump
Don't know if it matters but I ran the above commands with the fixed ec_id and base address to match my hw: The issue exists without this patch too in current main branch. This patch attempted to fix it and hopefully make the hexdump correct. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the second version from #261
The debug messages will be removed later!