radeontop icon indicating copy to clipboard operation
radeontop copied to clipboard

Implemented temperature sensor reading

Open mmsoft3 opened this issue 3 years ago • 20 comments

GPU temperature is easily available in the drm interface. In FreeBSD, there is no other tool available to monitor GPU temperature.

mmsoft3 avatar Jul 01 '21 22:07 mmsoft3

What is this: "%.1fgit cloneC GPU Temperature %6.2f%%"

Also, this looks like it'd break Radeon.

clbr avatar Jul 02 '21 05:07 clbr

The indent on the later part in ticks.c is wrong.

clbr avatar Jul 02 '21 05:07 clbr

Looks like it'd also break amdgpu when on older DRM.

clbr avatar Jul 02 '21 05:07 clbr

Does the ui.c part even compile? temp is declared before temp_c but it uses temp_c...

clbr avatar Jul 02 '21 05:07 clbr

Dump part is missing.

Altogether, this needs a lot of work.

clbr avatar Jul 02 '21 05:07 clbr

Thanks for your comments. I updated the radeon part, however I cannot test it since I do not have such a board. Also updated the dump part and fixed some typos

mmsoft3 avatar Jul 02 '21 10:07 mmsoft3

Thanks. Please squash the commits into one.

clbr avatar Jul 02 '21 11:07 clbr

Looks like it'd also break amdgpu when on older DRM.

How do you think it will break? Temp sensor data has been exposed at the same time as memory and shader clock data that is already used by radeontop

mmsoft3 avatar Jul 02 '21 11:07 mmsoft3

In that case the code was calling a function pointer of random garbage. There was no setting it to NULL and checking for NULL at that point.

clbr avatar Jul 02 '21 11:07 clbr

well, this is a common problem also with the other sensors data as well. e.g.: In case, line 80: (!(ret = getsclk_amdgpu(&out32))) gives false for some reason, the function pointer is also uninitialized garbage.

mmsoft3 avatar Jul 02 '21 12:07 mmsoft3

No, it's not; that is initialized in detect.c.

clbr avatar Jul 02 '21 12:07 clbr

do you want to have them all similar or should the NULL check remain there?

mmsoft3 avatar Jul 02 '21 12:07 mmsoft3

Existing style would be best.

clbr avatar Jul 02 '21 12:07 clbr

as discussed, unified the initialization of function pointers and squashed all commits into one.

Would appreciate to see it in upcoming releases.

mmsoft3 avatar Jul 02 '21 21:07 mmsoft3

Ok, I tested it on radeon. The values are all wrong, and the indent in dump.c is still wrong. The NULL check in dump.c is still present. Also please remove the utf-8 degree symbol in the comment in detect.c.

The values are wrong because you don't take into account k. You must multiply by that, and use a correct formula otherwise.

Have you even tested this? First the non-compiling ui.c, and now the wrong values. The values will be wrong on amdgpu as well, not just radeon, since the formula is the same for both!

clbr avatar Jul 03 '21 04:07 clbr

Of course I tested and it’s working not only for me with correct values as you can see here: https://forums.freebsd.org/threads/amdgpu-is-there-a-way-to-read-amd-gpus-temperatures-and-or-power-usage.76245/#post-520161

That’s why I said I can only test for amdgpu, not radeon. So maybe the formula for radeon needs to be different from amdgpu.

What values do you get from a radeon board?

mmsoft3 avatar Jul 03 '21 07:07 mmsoft3

Try changing the tick rate. -t 50 for example.

clbr avatar Jul 03 '21 16:07 clbr

Temperature reading does not match with that of hardinfo.

image

mmaravillo avatar Jan 07 '23 20:01 mmaravillo

Any update on this? Would love to see my GPU temperature with radeontop.

octal-illumination avatar Jun 11 '23 18:06 octal-illumination

Why putting this in radeontop while all the correct radeon temperatures, power usage and so on are available via the hwmon subsystem? I'm pretty sure hwmon and the userspace tool lm_sensors is available on freebsd.

wgottwalt avatar Mar 13 '24 06:03 wgottwalt