Update the AMD family 17h temperature reporting based on AMD Tech Doc 56255 OSRR, section 4.2.1:
- For CPUS w/CUR_TEMP_RANGE_SEL set, scale the reported temp in the range -49..206, eg, subtract 49C
Differential D16855 Authored by gallatin on Aug 23 2018, 12:06 AM.
Details Summary Update the AMD family 17h temperature reporting based on AMD Tech Doc 56255 OSRR, section 4.2.1:
Diff Detail
Event TimelineHerald added a subscriber: imp. · View Herald TranscriptAug 23 2018, 12:06 AM2018-08-23 00:06:27 (UTC+0) Harbormaster completed remote builds in B19077: Diff 47144.Aug 23 2018, 12:06 AM2018-08-23 00:06:29 (UTC+0) Comment Actions Change looks functionally correct to me. Some style-y nitpicks below:
Comment Actions Thank you so much for the pointer to the docs!!! It is much nicer to write from real docs, rather than crib from undocumented changes in another project. Looking at the docs, it says that the range of CUR_TEMP is: 0..225C when CUR_TEMP_RANGE_SEL is not set, and -49C ... 206C when it is. So, based on my reading of the docs, our current scaling is wrong, since it does not scale from 0..225, but from 0..255. Eg, with our current scaling, max possible value, 0x7ff, would result in a temp of 255 not 225. So I've updated the scaling to reflect this. Interestingly, when bit 19 is set, the range is again 256 wide, so we can use the old scaling there. Harbormaster completed remote builds in B19098: Diff 47170.Aug 23 2018, 2:29 PM2018-08-23 14:29:40 (UTC+0) Comment Actions
Agreed!
Yeah, like I said earlier, I think that's a pretty clear typo for 255C. The temperature scale does not change between the modes and 206+49=255.
I don't believe this is correct. Comment Actions
The right document for the temperature sensor is SB-TSI (40821), which you may note is dated 2010 and has not changed from earlier sensors (i.e., amdtemp_gettemp directly above uses the same math). § 2.6 SB-TSI Temperature and Threshold Encodings:
Essentially the encoding is: one byte (8 bits) for the integer portion of the value, and the remaining 3 bits specify the fractional temperature in increments of 1/8 °C. This revision now requires changes to proceed.Aug 23 2018, 5:12 PM2018-08-23 17:12:05 (UTC+0) Harbormaster completed remote builds in B19112: Diff 47194.Aug 23 2018, 6:18 PM2018-08-23 18:18:55 (UTC+0) Comment Actions Thanks! Mostly looks good to me. A few quibbles:
gallatin marked an inline comment as done. Harbormaster completed remote builds in B19184: Diff 47299.Aug 25 2018, 11:09 PM2018-08-25 23:09:54 (UTC+0)
This revision is now accepted and ready to land.Aug 26 2018, 1:30 AM2018-08-26 01:30:19 (UTC+0) Closed by commit rS340426: amdtemp(4): Fix temperature reporting on AMD 2990WX (authored by cem). · Explain WhyNov 14 2018, 4:50 AM2018-11-14 04:50:53 (UTC+0) This revision was automatically updated to reflect the committed changes.
Revision Contents
Diff 47299 sys/dev/amdtemp/amdtemp.c
|