chris wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43408 )
Change subject: mb/google/zork: add dptc support ......................................................................
Patch Set 13:
(7 comments)
https://review.coreboot.org/c/coreboot/+/43408/11/src/mainboard/google/zork/... File src/mainboard/google/zork/variants/baseboard/include/baseboard/acpi/dptc.asl:
https://review.coreboot.org/c/coreboot/+/43408/11/src/mainboard/google/zork/... PS11, Line 5: #define DPTC_THERMAL_CONTROL_LIMIT 0x3 : #define DPTC_SUSTAINED_POWER_LIMIT 0x5 : #define DPTC_FAST_PPT_LIMIT 0x6 : #define DPTC_SLOW_PPT_LIMIT 0x7 : #define DPTC_SLOW_PPT_TIME_CONSTANT 0x8 : #define DPTC_PROCHOT_L 0x9 : #define DPTC_VRM_CURRENT_LIMIT 0xb : #define DPTC_VRM_MAXIMUM_CURRENT_LIMIT 0xc : #define DPTC_VRM_SOC_CURRENT_LIMIT 0xe :
Are these register offsets? Or actual limit values?
These are the parameter ID for correspond power or thermal parameters.
https://review.coreboot.org/c/coreboot/+/43408/11/src/mainboard/google/zork/... PS11, Line 26: POF1
These are just empty functions. […]
No, It should be a dummy function and if any board needs to adjust their setting for the tablet mode. Then they should have their own POF1 in variant.
https://review.coreboot.org/c/coreboot/+/43408/11/src/mainboard/google/zork/... PS11, Line 33: Sample call
What does this mean?
A sample for the call to ALIB with DPTCi
https://review.coreboot.org/c/coreboot/+/43408/11/src/mainboard/google/zork/... PS11, Line 34: POFX
Where does this get called from?
will not call this method. it's a sample name.
https://review.coreboot.org/c/coreboot/+/43408/11/src/mainboard/google/zork/... PS11, Line 40: Store(0x07, SSZE)
Having comments about what each of these statements is doing would be helpful.
yes, the comment added.
https://review.coreboot.org/c/coreboot/+/43408/11/src/mainboard/google/zork/... PS11, Line 42: 4.8w
Does this apply to all variants using all types of processors? Picasso, Dali, Pollock?
yes, the they support and can be apply by DPTC.
https://review.coreboot.org/c/coreboot/+/43408/11/src/mainboard/google/zork/... PS11, Line 43: ALIB
Shouldn't the call to ALIB be done from SoC code? Ideally, I think we should generate the code using […]
Agree with that. But I haven't studied how to do that. maybe will have a second version in further.