Casper Chang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31331 )
Change subject: mb/google/sarien/variants/arcada: Update thermal configuration for DPTF ......................................................................
Patch Set 2:
(5 comments)
Please kindly check the comment.
https://review.coreboot.org/#/c/31331/2/src/mainboard/google/sarien/variants... File src/mainboard/google/sarien/variants/arcada/include/variant/acpi/dptf.asl:
https://review.coreboot.org/#/c/31331/2/src/mainboard/google/sarien/variants... PS2, Line 23: 108
Please, explain why it's set such a high value 108 for Skin sensor ?
The purpose of TSR0 and TSR2 critical would be preventing from serious issue like casing melting. That’s the reason we set such high value for sensors and it’s based on bandon’s (leading Windows project) experience.
https://review.coreboot.org/#/c/31331/2/src/mainboard/google/sarien/variants... PS2, Line 35: 95
Please, explain why it's set such a high value 95 for Ambient sensor ?
Same as previous one.
https://review.coreboot.org/#/c/31331/2/src/mainboard/google/sarien/variants... PS2, Line 60: 21
What's the reason for reducing it from previous value of 25W to this new value as 21W ? Did you obse […]
The value was leveraged from Bandon (leading Windows project), the 21 W would be max dynamic PL1 with same CPU type and thermal solutions.
https://review.coreboot.org/#/c/31331/2/src/mainboard/google/sarien/variants... PS2, Line 62: 28000
How did you arrive to this particular number ?
same as previous one.
https://review.coreboot.org/#/c/31331/2/src/mainboard/google/sarien/variants... PS2, Line 70: 28000
same query as previous one.
same as previous one.