Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43408 )
Change subject: soc/amd/picasso: add dptc support ......................................................................
Patch Set 20: Code-Review+1
(5 comments)
Minor nits. But LGTM overall.
https://review.coreboot.org/c/coreboot/+/43408/20//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43408/20//COMMIT_MSG@18 PS20, Line 18: Generated ASL code from SSDT Can you please add the generated SSDT code for the DPTC routine here in the commit message?
https://review.coreboot.org/c/coreboot/+/43408/20/src/soc/amd/picasso/chip.h File src/soc/amd/picasso/chip.h:
https://review.coreboot.org/c/coreboot/+/43408/20/src/soc/amd/picasso/chip.h... PS20, Line 93: * nit: space before *
https://review.coreboot.org/c/coreboot/+/43408/20/src/soc/amd/picasso/chip.h... PS20, Line 94: nit: One space is sufficient.
https://review.coreboot.org/c/coreboot/+/43408/20/src/soc/amd/picasso/chip.h... PS20, Line 95: fast_ppt_limit_tablet_mode I think it would be good to add a comment indicating that these limits are used only when dptc_enable is set to 1.
https://review.coreboot.org/c/coreboot/+/43408/20/src/soc/amd/picasso/root_c... File src/soc/amd/picasso/root_complex.c:
https://review.coreboot.org/c/coreboot/+/43408/20/src/soc/amd/picasso/root_c... PS20, Line 38: .size = sizeof(struct dptc_input), \ : .params = { \ : { \ : .id = SUSTAINED_POWER_LIMIT_PARAM_ID, \ : .value = _sustained, \ : }, \ : { \ : .id = FAST_PPT_LIMIT_PARAM_ID, \ : .value = _fast, \ : }, \ : { \ : .id = SLOW_PPT_LIMIT_PARAM_ID, \ : .value = _slow, \ : }, nit: this whole block can be shifted left by one tab.