Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for showing temp of (more) AMD chips #2051

Open
2 tasks
Caellian opened this issue Oct 2, 2024 · 7 comments
Open
2 tasks

Add support for showing temp of (more) AMD chips #2051

Caellian opened this issue Oct 2, 2024 · 7 comments
Labels
cpu related to CPU stats or system process reporting feature suggest addition of new functionality that isn't currently supported in any way good first issue straightforward enough for first-time contributors to be able to implement themselves

Comments

@Caellian
Copy link
Collaborator

Caellian commented Oct 2, 2024

Support for showing temperature of (at least) following AMD chips is missing when using acpitemp:

Originally posted by @LinuxOnTheDesktop in Discussion #2050

This thread elsewhere reveals that Conky's acpitemp cannot get the temperature of a Phenom2 quad-core chip. I find that similarly Conky cannot get the temperature on my new Framework 13-AMD, with a Ryzen 6 764OU processor. (See perhaps also this further report.)

One can write an external program to allow Conky to display the values on such AMD chips, but native Conky support would be nice!

Comment here is you find any other so they be added to the list.

@Caellian Caellian added cpu related to CPU stats or system process reporting help wanted functionality with which contributors need help to implement/fix properly feature suggest addition of new functionality that isn't currently supported in any way labels Oct 2, 2024
@Caellian
Copy link
Collaborator Author

Caellian commented Oct 2, 2024

I'm hoping @LinuxOnTheDeskop will be around to help with testing when implemented if implementer doesn't have the appropriate hardware.

This is also why this issue is tagged with "help wanted" - it would be useful if AMD users subscribed to it so they can help with testing as it's specific to hardware.

Though my guess reading a few numbers from some file somewhere should just work™.

@LinuxOnTheDesktop
Copy link

Certainly I can help to test.

And if and when I manage to create a working script (bash or lua or C) that does the job (at least for my CPU), then perhaps it will be useful to post that script. (Come to think of it, though, given how often I'll be calling the script, probably I'd better write it in . . gulp . . C. Or would a lua script perform as well, given that Conky keeps lua scripts within RAM?)

@Caellian
Copy link
Collaborator Author

Caellian commented Oct 2, 2024

Or would a lua script perform as well, given that Conky keeps lua scripts within RAM?

Very few things in the world run as good as C. JITLua is very close but it's still slower due to abstractions. The slowest part in the script will be file I/O so I don't think it matters that much what you choose given that you'll be doing very little processing. Post the script in any case as it will show what needs to be done in whichever language you land on.

Thank you for the offer to test. Also I commend you for using Framework, nice to see it in the wild.

@LinuxOnTheDesktop
Copy link

Right. I'll do lua because I'll be able to implement that much faster, and because hopefully you will render the script obsolete.

@LinuxOnTheDesktop
Copy link

LinuxOnTheDesktop commented Oct 3, 2024

The following works for me [EDIT: I am using a Framework 13-AMD batch-3 with a Ryzen 5 764OU processor] (but see caveat below).

#!/usr/bin/env lua
--[[
    NAME                ryzenCpuTemp.lua
    NEEDS               lua package, if run outside Conky.
                        The lm-sensors package
    PURPOSE             lua script for use in Conky.
    INFORMED BY         https://bbs.archlinux.org/viewtopic.php?id=299850
    NB                  This script runs every <ticksNeeded> Conky ticks.
]]

-- Optimisation, via https://gamesensical.github.io
local print, tonumber = print, tonumber

-- luacheck: ignore conky_ryzenCpuTemp conky_parse ignore conky_window

local conky_ryzenCpuTempTmp_str = ""
local txt_unknown = "?"
local ticksNeeded = 2 -- 2


--[[
====================
THE CENTRAL FUNCTION
====================
]]

-- Returns: string for Conky to print
local function conky_ryzenCpuTemp_core()
    local retStr = txt_unknown
    local f = assert(io.popen("sensors k10temp-pci-00c3 -c /dev/null", "r"))
    if f then
        -- Get 3rd line
        local line
        for _=1,3 do
            line = f:read("*l")
            if not line then break end
        end
        if line then
            -- Line should be e.g.: Tctl:         +41.0°C
            local cpuTemp
            cpuTemp = line:gsub( "(.*)+", "" )
            -- Now remove the decimal point and everything that follows it. (Need the % escape character.)
            -- We will leave it to Conky to print a (er . .) degree sign.
            cpuTemp = cpuTemp:gsub( "%.(.*)", "" )
            if cpuTemp then
                retStr = cpuTemp
            end
        end
        f:close()
    end
    return retStr
end

--[[
====================================================
THE FUNCTION CALLED BY CONKY
The function needs to be:
- non-local;
- named 'conky_<leafname-of-script-without-suffix>.
===================================================
]]
-- selene: allow(undefined_variable,unused_variable)
function conky_ryzenCpuTemp()
    if conky_window == nil then
        return
    end
    -- selene: allow(undefined_variable)
    if tonumber(conky_parse("${updates}")) % ticksNeeded == 0 then
        conky_ryzenCpuTempTmp_str = conky_ryzenCpuTemp_core()
    end
    return conky_ryzenCpuTempTmp_str
end

--[[ EOF ]]

However: I have not properly verified that the temperature the script gets really is the cpu temperature. I do not know how I could. Yet, the following points suggest that I am pulling the right number.

  • The sensor at issue is the one identified on the Arch board as being the CPU temperature (averaged across cores, or something like that).

  • That sensor is the most likely candidate among the sensors:

$ sudo apt-install lm-sensors
$ sensors
amdgpu-pci-c100
Adapter: PCI adapter
vddgfx:      844.00 mV 
vddnb:       857.00 mV 
edge:         +37.0°C  
PPT:          10.01 W  (avg =   5.13 W)

ucsi_source_psy_USBC000:004-isa-0000
Adapter: ISA adapter
in0:          20.00 V  (min =  +5.00 V, max = +13.20 V)
curr1:         3.00 A  (max =  +3.16 A)

ucsi_source_psy_USBC000:002-isa-0000
Adapter: ISA adapter
in0:           5.00 V  (min =  +5.00 V, max =  +5.00 V)
curr1:         0.00 A  (max =  +1.50 A)

nvme-pci-0200
Adapter: PCI adapter
Composite:    +33.9°C  (low  =  -5.2°C, high = +89.8°C)
                       (crit = +93.8°C)

mt7921_phy0-pci-0100
Adapter: PCI adapter
temp1:        +32.0°C  

k10temp-pci-00c3  [--------------- This is the chap! ---------------]
Adapter: PCI adapter
Tctl:         +40.1°C  

ucsi_source_psy_USBC000:003-isa-0000
Adapter: ISA adapter
in0:           5.00 V  (min =  +5.00 V, max =  +5.00 V)
curr1:         0.00 A  (max =  +1.50 A)

ucsi_source_psy_USBC000:001-isa-0000
Adapter: ISA adapter
in0:           0.00 V  (min =  +0.00 V, max =  +0.00 V)
curr1:       680.00 mA (max =  +0.00 A)

BAT1-acpi-0
Adapter: ACPI interface
in0:          17.51 V  
curr1:         0.00 A  
  • I ran the s-tui stress test (sudo apt install s-tui && sudo apt install stress && s-tui and then pressed the 'stress' button), and the value reported by my script hit the eighties (and the fan on my Framework soon started going like a turbine).

@LinuxOnTheDesktop
Copy link

LinuxOnTheDesktop commented Oct 3, 2024

@Caellian : just now I noticed that:

  • my post immediately above omitted the name of my processor;
  • my original post got the name of my processor wrong.

So, I have:

  • added the (correct) processor name to the post above;
  • added a correction on my original post.

Apologies.

@Caellian
Copy link
Collaborator Author

Caellian commented Oct 4, 2024

Thanks, so lm-sensors uses kernel drivers to get temperature information besides just sysfs which is already handled by conky. It's got a library that conky can use so we don't need to copy-paste the same code, and parts of conky sysfs handling can also then be removed in favor of library implementation.

Related code in conky is located in linux.cc.

Also, note that acpitemp variable name is maybe not the most appropriate because (as lm-sensors states), acpi is a different way of getting those readings from BIOS instead of from kernel. Maybe conky already does additional work to some degree but the variable name is kept for backwards compatibility.

@Caellian Caellian added good first issue straightforward enough for first-time contributors to be able to implement themselves and removed help wanted functionality with which contributors need help to implement/fix properly labels Oct 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cpu related to CPU stats or system process reporting feature suggest addition of new functionality that isn't currently supported in any way good first issue straightforward enough for first-time contributors to be able to implement themselves
Projects
None yet
Development

No branches or pull requests

2 participants