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

cputemp: give RPi SoC sensor priority #3073

Closed
wants to merge 1 commit into from
Closed

cputemp: give RPi SoC sensor priority #3073

wants to merge 1 commit into from

Conversation

MilhouseVH
Copy link
Contributor

@MilhouseVH MilhouseVH commented Nov 1, 2018

See: https://forum.libreelec.tv/thread/12662-libreelec-9-0-reborn-remix-emulationstation-retroarch-dolphinqt-moonlight-chrome/?postID=102667#post102667

When an RPi is configured with dtoverlay w1-gpio gpiopin=4 (and the relevant temperature sensing hardware is connected to the GPIO pins so that a non-zero temperature is registered) then an extra temperature input becomes available which is misinterpreted as the SoC temperature.

@MilhouseVH
Copy link
Contributor Author

Updated as it turns out some x86 CPUs (ie. i5 Skylake) have /sys/class/hwmon/hwmon0/device/temp1_input which does not appear to be a CPU temp sensor (it remains at a constant ~25C which is between 30C-40C below the currently reported CPU temp, and it doesn't change with CPU or GPU workload so it maybe some sort of System board temp sensor).

@alinnastac
Copy link

alinnastac commented Nov 2, 2018

At least on my RPi, w1_therm driver creates /sys/class/hwmon/hwmon0/temp1_input for the DS18B20 sensor that is connected through w1_gpio. To fix this issue, you need to move /sys/class/thermal/thermal_zone0/temp reading at the beginning (i.e. line 37).

LE: Wait, I have this script (in its original form) in /usr/bin and it is able to read the CPU temperature correctly when I execute it with command line "cputemp cpu".
Kodi however is reporting the w1_slave temperature as being the CPU temp. /:-|

@MilhouseVH
Copy link
Contributor Author

@alinnastac are you still seeing an issue with this updated PR (19f3442)? The problem is/was that the existing cputemp script takes the reading from the GPIO sensor and not the RPI SOC sensor - this PR fixes that by reading the SOC sensor before the GPIO sensor.

@alinnastac
Copy link

alinnastac commented Nov 2, 2018

The commit you refer to makes no difference for my case because:
a) The path to the temp1_input file is slightly different than the one handled in the moved if statement (/sys/class/hwmon/hwmon0/temp1_input vs /sys/class/hwmon/hwmon0/device/temp1_input)
b) The /usr/bin/cputemp script available in my LE 9.0 RR is perfectly capable of reporting the right CPU temperature. This script does not include the commit you mention.

The only wrong CPU temperature output I get is in Kodi System Info page. I reckon that Kodi does not use the cputemp script to find out the CPU temperature, it reads the value directly from /sys/class/hwmon/hwmon0/temp1_input.

@MilhouseVH
Copy link
Contributor Author

@alinnastac

The commit you refer to makes no difference for my case because:

Correct - this PR has nothing to do with the DB18B20 sensor path which is not being read before or after the change in this PR. In fact, I had misread the forum reply and I see s/he has the same /sys path as your DB18B20, so this PR is unnecessary (and I'm going to close it).

I reckon that Kodi does not use the cputemp script to find out the CPU temperature, it reads the value directly from /sys/class/hwmon/hwmon0/temp1_input.

Yes you are correct: https://github.com/xbmc/xbmc/blob/master/xbmc/utils/CPUInfo.cpp#L276-L279, and this is the real issue being raised by the forum user.

You can add the following to advancedsettings.xml:

<advancedsettings>
  <cputempcommand>/usr/bin/cputemp</cputempcommand>
</advancedsettings>

which should resolve the issue. Or submit a PR to Kodi.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants