htop
htop copied to clipboard
Add core temperatures for Rockchip RK3588 SoC
My attempt to map SoC core temperatures to virtual cores. Rather a hack than a well reasoned change. There must be a proper way to determine chipset so temperature mapping can be done if condition is met (a refactor will be needed).
Also, data[0] is not set. We have two candidates here, center_thermal and soc_thermal values.
SoC Thermal Drivers
sensors utility bundled with lm-sensors package detected the following thermal drivers
npu_thermal-virtual-0
Adapter: Virtual device
temp1: +35.2 C
center_thermal-virtual-0
Adapter: Virtual device
temp1: +35.2 C
bigcore1_thermal-virtual-0
Adapter: Virtual device
temp1: +36.1 C
soc_thermal-virtual-0
Adapter: Virtual device
temp1: +36.1 C (crit = +115.0 C)
nvme-pci-0100
Adapter: PCI adapter
Composite: +41.9 C (low = -0.1 C, high = +71.8 C)
(crit = +89.8 C)
gpu_thermal-virtual-0
Adapter: Virtual device
temp1: +35.2 C
littlecore_thermal-virtual-0
Adapter: Virtual device
temp1: +36.1 C
bigcore0_thermal-virtual-0
Adapter: Virtual device
temp1: +36.1 C
Future Additions
- [ ] NPU temperature
- [ ] GPU temperature
References
This PR addresses #1404
I think, instead of hacks on top of other hacks, the overall libsensors-to-cpucore mapping should be addressed properly.
I think, instead of hacks on top of other hacks, the overall libsensors-to-cpucore mapping should be addressed properly.
I believe the libsensors-to-cpu-core mapping shouldn't be handled in htop's side. Perhaps we can ask this question in libsensors upstream to see if there is a portable way to describe the mappings.
My impression is that libsensors.conf is supposed to do the job. Or, maybe, ACPI thermal zone (even though I personally hate ACPI). Once we have a "protocol" to read the mapping from, we would no longer need these chipset-specific hacks.
@Explorer09 Mind to open a discussion with the libsensors people and link it here for reference? It's not just this bug, but quite a few more, that basically boil down to how to perform this mapping. TIA.
Maybe it would be better to create some sort of naming convention for the temperature channels used for the cpu cores.
I am strongly against adding driver-specific code to libsensors, it was done once and proved to be an maintainance nightmare.
With a proper naming convention in place, sensors.conf can be used to do the necessary adjustments.
We'd also strongly prefer not to deal with any of the driver-specific stuff in htop. And honestly, there are two places things could go: Either into sysfs alongside each sensor providing the core/thread information we need (can figure the rest from procfs -> cpuinfo). Or we get this information from the library we use for fetching the temperature information.
Both have their drawbacks, but the alternative would be, that each user of lmsensors would have to write their own code to figure this out instead, which is effectively worse …
@segabor Please squash your commits in this PR. Cf. the styleguide for details.
The latest rebase looks strange …
Sorry for the mess @BenBE , hope it's fixed now
@segabor highly appreacted as I have many rk3588...
wohoo - so happy that my thank you triggered a merge ;)
thank you for this, i easily adapted this for Qualcomm SDM845 (Oneplus 6 enchilada running PostmarketOS with mainline kernel), it reports temperatures in similar way. Although i could imagine this getting really redundant if you wanted to support all the other possible Qualcomm chips. Here in case someone wants it.
thank you for this, i easily adapted this for Qualcomm SDM845 (Oneplus 6 enchilada running PostmarketOS with mainline kernel), it reports temperatures in similar way. Although i could imagine this getting really redundant if you wanted to support all the other possible Qualcomm chips. Here in case someone wants it.
Could you please link the exact commit for future reference? That way you allow people even when your repo at some point contains more commits to easily find the exact change you are referring to in your comment. TIA.
Also looking at your commit I notice a few improvements that could/should be made:
- Use
sscanfin the code assigning the actual values to avoid redundancy. Alternatively you couldString_startsWith(str, "cpu") && String_eq(str + 4, "_thermal")and pull the actual index (with range check) fromstr[3]-'0'+1if it's a valid digit. - Proper alignment in the list of possible drivers at the top.