Fix: AMD Ryzen CPU temps

[amz-x]

Fixes the temperature reported by AMD Ryzen CPUs.

[stsdc]

Hi, since this is a hardware issue and I can't test it, you'll have to be more descriptive on what is the issue. :)

[amz-x]

Hi @stsdc, it's been a while.

This PR fxies the AMD Ryzen CPU temperature reading which is currently incorrect compared to applications such as btop, Resources and OpenLinkHub.

After the PR / patch I can confirm that all the above mentioned applications / tools shows the same temperature reading:

[vjr]

I have a Ryzen 9950X running elementary OS v9 and the temperature is already working although I didn't compare to other tools. @amz-x What distro and version are you on?

[amz-x]

@vjr Running NixOS 26.11 (unstable) with the Pantheon desktop environment. Would you mind comparing it to other tools?

[vjr]

I can test maybe today and approve the PR if it works for me and it's okay with @stsdc

[stsdc]

Hi @amz-x, it's good to see you back!

So, I would like to know what it was like before the change?
Do you have both Tdie and Tctl in yours sysfs?

If I recall it right, the Tdie was the actual temperature on the die and Tctl was a bit modified value to trigger fans (maybe that's why it is 40deg and not ~20ish?).
That's why I specifically choose the Tdie back then, but, yeah, it could have changed since then.
From your screenshots, I can see that it is possible to have per core temperature in AMD.

Could you also paste the debug logs?

[amz-x]

Thanks, have a few more PRs coming soon...

Ok screenshot below before the PR / patch and the logs below it:

 Monitor 8.0.1 

LibPCI ver: 3.15.0 
Gtk 4.22.4

** (io.elementary.monitor:49255): CRITICAL **: 14:40:43.695: long_parse: assertion 'str != NULL' failed

** (io.elementary.monitor:49255): CRITICAL **: 14:40:43.696: long_parse: assertion 'str != NULL' failed

** (io.elementary.monitor:49255): CRITICAL **: 14:40:43.696: long_parse: assertion 'str != NULL' failed

** (io.elementary.monitor:49255): CRITICAL **: 14:40:43.696: long_parse: assertion 'str != NULL' failed

** (io.elementary.monitor:49255): CRITICAL **: 14:40:43.696: long_parse: assertion 'str != NULL' failed
WARNING: radv is not a conformant Vulkan implementation, testing use only.

(io.elementary.monitor:49255): Gtk-WARNING **: 14:40:43.971: Widget reports min height of 80 for width of 272, but overall min height of 106

(io.elementary.monitor:49255): Gtk-WARNING **: 14:40:43.971: GtkBox 0x59d864f656a0 (box) reported min height 120 and natural height 94 in measure() with for_size=381; natural size must be >= min size

[stsdc]

@amz-x sorry, I meant debug logs. To run, execute G_MESSAGES_DEBUG=all io.elementary.monitor.
So it took 31.54 degree from Tdie, as I understand

[amz-x]

@amz-x sorry, I meant debug logs. To run, execute G_MESSAGES_DEBUG=all io.elementary.monitor. So it took 31.54 degree from Tdie, as I understand

@stsdc with or without the patch?

[stsdc]

@amz-x sorry, I meant debug logs. To run, execute G_MESSAGES_DEBUG=all io.elementary.monitor. So it took 31.54 degree from Tdie, as I understand

@stsdc with or without the patch?

without the patch

[amz-x]

@stsdc uploading logs with and without patch:

• logs-with-patch.txt
• logs-without-patch.txt

[stsdc]

Looks like we must use Tccd* here, instead of Tctl

** (io.elementary.monitor:54909): DEBUG: 15:36:07.184: HwmonPathsParserCPU.vala:51: 🌡️ Parsed HWMON CPU temperature interface: Tctl
** (io.elementary.monitor:54909): DEBUG: 15:36:07.184: HwmonPathsParserCPU.vala:51: 🌡️ Parsed HWMON CPU temperature interface: Tccd1
** (io.elementary.monitor:54909): DEBUG: 15:36:07.184: HwmonPathsParserCPU.vala:51: 🌡️ Parsed HWMON CPU temperature interface: Tccd2

@amz-x what values you have on Tccds?

[amz-x]

Looks like we must use Tccd* here, instead of Tctl

** (io.elementary.monitor:54909): DEBUG: 15:36:07.184: HwmonPathsParserCPU.vala:51: 🌡️ Parsed HWMON CPU temperature interface: Tctl
** (io.elementary.monitor:54909): DEBUG: 15:36:07.184: HwmonPathsParserCPU.vala:51: 🌡️ Parsed HWMON CPU temperature interface: Tccd1
** (io.elementary.monitor:54909): DEBUG: 15:36:07.184: HwmonPathsParserCPU.vala:51: 🌡️ Parsed HWMON CPU temperature interface: Tccd2

@amz-x what values you have on Tccds?

@stsdc same temperature readings as Tctl, tested Tccd1 & Tccd2.

[stsdc]

I guess we can use Tccds then. But the best option would be to use mean of all cores, same as for Intel.

[amz-x]

I guess we can use Tccds then. But the best option would be to use mean of all cores, same as for Intel.

@stsdc updated the check for AMD CPUs.

The Tccds is reporting about ~20℃ difference.

** (io.elementary.monitor:37750): WARNING **: 10:36:51.313: CPU.vala:77: CPU temperature Tccd1 input: 28000

** (io.elementary.monitor:37750): WARNING **: 10:36:51.313: CPU.vala:77: CPU temperature Tccd2 input: 25250

** (io.elementary.monitor:37750): WARNING **: 10:36:51.313: CPU.vala:77: CPU temperature Tccd1 input: 28000

** (io.elementary.monitor:37750): WARNING **: 10:36:51.313: CPU.vala:77: CPU temperature Tccd2 input: 25250

The Tctl temperature value is more accurate to the other monitoring tools that I've tested. Updating code to not use Tccds, but rather use Tdie or Tctl.

Let me know if I need to make additional changes or modifications.

Thanks!

[stsdc]

The difference of ~20℃ between Tctl and Tdie cores or Tcdd is because Tctl is used for earlier triggering of cooling (fans).

The Tctl temperature value is more accurate to the other monitoring tools that I've tested.

Other tools shouldn't be our only source of truth. Even the author of Resources admits that Tctl is incorrect.

So, for the temperature reporting I propose this fallback chain:
coretemps → Tccd → Tdie → Tctl

I would also look for the recent changes in K10temp driver.

[stsdc]

Some docs on k10temp: https://docs.kernel.org/hwmon/k10temp.html#description

[amz-x]

@stsdc I switched to zenpower kernel module and luckily enough it provides the Tdie temperature sensor reading. So currently it looks for Tdie and then fall back to Tctl for the CPU temperature reading on AMD CPUs.

zenpower-pci-00c3
Adapter: PCI adapter
Tdie:            +42.5°C  (high = +95.0°C)
Tctl:            +42.5°C  
Tccd1:           +29.4°C  
Tccd2:           +28.0°C  
RAPL_P_Package:  23.17 W

So, for the temperature reporting I propose this fallback chain:
coretemps → Tccd → Tdie → Tctl

Agreed, however this PR is just to fix the AMD CPU temperature reading, it would require switching between k10temp and zenpower to test the differences for coretemps, Tccd, Tdie and Tctl.

I would suggest another PR (feature) to improve the overall AMD CPU temperature readings, or do you want to implement it as part of this PR?

[stsdc]

Yeah, I think that's fine for now. In the next PRs we can utilize Tccds.

[amz-x]

Yeah, I think that's fine for now. In the next PRs we can utilize Tccds.

Another sensor that we might look into?

https://docs.kernel.org/hwmon/asus_ec_sensors.html

asusec-isa-000a
Adapter: ISA adapter
CPU:          +32.0°C  
CPU Package:  +42.0°C  
Motherboard:  +27.0°C  
T_Sensor:     -60.0°C  
VRM:          +49.0°C

[stsdc]

Yes, why not? :)

I guess this is from ASUS MoBo?