Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38755 )
Change subject: nb/amd/{agesa,pi}/acpi: include thermal zone ......................................................................
Patch Set 1:
Patch Set 1:
Sorry, there's some misunderstanding. I didn't doubt that the HTC feature is configured correctly.
I doubt that the thermal_mix.asl code is actually useful with correctly configured HTC. If I read it correctly, it would prevent HTC to become active (powers down long before that would happen).
Okay, so are you referring to this line? Return (Subtract (_CRT, K10TEMP_HOT_OFFSET))
Or I am still missing your point (or something obvious)?
That too. But also that it is using TLMT (the limit when HTC should trigger, if I understand the BWG correctly) in _CRT. ACPI says, the OS should go into S4 at _HOT and immediately shutdown at _CRT.
I don't know AMD very well. Just when I read the BWG and ACPI spec, it seems to me that TLMT would rather match _PSV.
HTC will trigger when the current temperature exceeds the TLMT, which results in performance drop and PROCHOT assertion. So it is assuming that the processor will keep running, not transitioning to S4. Indeed this is not well written, I haven't figured that out at the beginning. So _HOT should be the TLMT+<some arbitrary value, let's say 5 or 10 degrees>. And TLMT should be decreased in such a case.
Thank you for pointing this out.