Attention is currently required from: Jason Glenesk, Raul Rangel, EricKY Cheng, Matt DeVillier, Caveh Jalali, Paul Menzel, Tim Wawrzynczak, Eric Peers, Fred Reitberger, Karthikeyan Ramasubramanian, Boris Mittelberg, Felix Held.
11 comments:
Commit Message:
Thermal table switches to different modes to meet 6
user experience.
nit: Implement dynamic thermal table switching flow chart.
This commit message should also describe the various states, so we have a human-readable version of the algorithm which we can validate the code against.
File src/ec/google/chromeec/acpi/ec.asl:
#if CONFIG(FEATURE_DYNAMIC_DPTC)
Name (PRTN, 0) // Previous thermal table Label A(0) to F(5)
#endif
Move this to `src/soc/amd/common/acpi/dptc.asl`, where it's used.
File src/soc/amd/common/acpi/dptc.asl:
Done
This was not done. Raul is asking for this code to be moved out of the common `dptc.asl` that all AMD devices use into something that's specific to winterhold devices.
File src/soc/amd/common/acpi/dptc.asl:
Patch Set #21, Line 46: // AMB sensor trigger point
Please add a comment describing what conditions these checks are looking for and what the result should be.
Specifically, there is no ">50C" transition state in the diagram here:
https://docs.google.com/presentation/d/1Ifjyx_Fu39WgLjJRsUBPxXcHW0dA706Jbx5b72RTQKs/edit?resourcekey=0-pKUKwA1h6Cp2eCc2TdYcNQ#slide=id.p1
Instead, there are only >=45C and <50 transition points.
50C = 122F
Please add a comment stating the units of these temperature values. The documentation all uses Celsius, but these values are Fahrenheit.
(\_SB.PCI0.LPCB.EC0.PRTN == 3) || // Previous table is D
(\_SB.PCI0.LPCB.EC0.PRTN == 5)) // Previous table is F
Why is this checking if the previous table was D or F, when neither of those are related to desktop+lid-open? I'd expect this to check table A.
118F = 47.8C. What state is this checking?
(\_SB.PCI0.LPCB.EC0.PRTN == 2) || // Previous table is C
(\_SB.PCI0.LPCB.EC0.PRTN == 4)) // Previous table is E
Similarly, why is this state dependent on tables C/E?
Patch Set #21, Line 64: }Else{
nit: Add whitespace around `Else` and before `//`.
Patch Set #21, Line 84: }Else{// Laptop
Same
To view, visit change 68471. To unsubscribe, or for help writing mail filters, visit settings.