gh-667: Modify message when using amd-pstate-epp #674
Conversation
|
cc @LDprg |
|
|
auto_cpufreq/core.py
Outdated
| if amd_value == "active" and scaling_driver_value == "amd-pstate-epp": | ||
| print("CPU turbo is controlled by amd-pstate-epp driver") | ||
| else: | ||
| print("Warning: CPU turbo is not available") |
There was a problem hiding this comment.
I am not sure if more status can exist at this moment which can set the turbo.
FYI, I am not the AMD expert :)
If "yes," we should make it to set the turbo if the user wants.
auto_cpufreq/core.py
Outdated
| @@ -267,13 +267,23 @@ def turbo(value: bool = None): | |||
| """ | |||
| p_state = Path("/sys/devices/system/cpu/intel_pstate/no_turbo") | |||
| cpufreq = Path("/sys/devices/system/cpu/cpufreq/boost") | |||
| amd_pstate = Path("/sys/devices/system/cpu/amd_pstate/status") | |||
| scaling_driver = Path("/sys/devices/system/cpu/cpu0/cpufreq/scaling_driver") | |||
There was a problem hiding this comment.
Don't need to check scaling_driver. Just amd_pstate/status is enough
auto_cpufreq/core.py
Outdated
|
|
||
| if p_state.exists(): | ||
| inverse = True | ||
| f = p_state | ||
| elif cpufreq.exists(): | ||
| f = cpufreq | ||
| inverse = False | ||
| elif amd_pstate.exists() and scaling_driver.exists(): |
There was a problem hiding this comment.
remove scaling_driver.exists()
auto_cpufreq/core.py
Outdated
| else: | ||
| print("Warning: CPU turbo is not available") |
There was a problem hiding this comment.
These can be removed since this else statement will never be reached. the cpufreq Path does exist on amd-pstate so it would be caught by the preceding elif statement
auto_cpufreq/core.py
Outdated
| scaling_driver_value = scaling_driver.read_text().strip() | ||
| if amd_value == "active" and scaling_driver_value == "amd-pstate-epp": |
There was a problem hiding this comment.
remove the scaling_driver check since if amd_pstate/status equals "active", then the loaded driver must be amd-pstate-epp
There was a problem hiding this comment.
also this isn't really important but I think this entire if statement doesn't actually do anything since the cpufreq Path only doesn't exist on amd-pstate-epp but I think we should leave the if statement since it makes it more clear what is actually happening.
There was a problem hiding this comment.
hmm, by the way, what for amd_pstate == guided or amd_pstate == passive?
There was a problem hiding this comment.
Self-reply for guided mode: #602 (comment)
if amd_pstate=guided is set, then /sys/devices/system/cpu/cpufreq/boost is set.
|
Looks good, but still since Also i forgot to mention, the |
auto_cpufreq/core.py
Outdated
| if value == "active": | ||
| print("CPU turbo is controlled by amd-pstate-epp driver") | ||
| return False | ||
| # Should not reach here. |
There was a problem hiding this comment.
I prefer to raise error here, but not sure about this project convention :)
There was a problem hiding this comment.
Because following code will require f, but f is None at this moment.
There was a problem hiding this comment.
I get what you're saying. Its true but since it never reaches there, it shouldn't matter, but I suppose for future proofing, we can unindent the return False
|
Thanks for the changes. You'll be credited as part of the next release. |
Hi, First of all, auto-cpufreq is super useful for my laptop, thank you for working on this.
While I read the relevant threads, it looks like setting turbo can be delegated to the amd-pstate-epp.
Please let me know if I knew wrong :)