Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45340 )
Change subject: soc/amd/picasso: Generate ACPI pstate and cstate objects in cb ......................................................................
Patch Set 19:
(4 comments)
https://review.coreboot.org/c/coreboot/+/45340/19/src/include/cpu/amd/msr.h File src/include/cpu/amd/msr.h:
https://review.coreboot.org/c/coreboot/+/45340/19/src/include/cpu/amd/msr.h@... PS19, Line 17: 0xFF
I found this one in the PPR to be [15:8] doc# 55570.
Ack. If this is something that is specific to Family 17h, I think it should go in picasso/include/soc/
https://review.coreboot.org/c/coreboot/+/45340/19/src/include/cpu/amd/msr.h@... PS19, Line 37: 7
In the PPR, which i used for a lot of this code cur [2:0] in PstateCtrl. […]
Bits 2:0 correspond to states 0 - 7 which makes it a total of 8. This macro is used to decide the total number of entries in pstate array which should be 8.
https://review.coreboot.org/c/coreboot/+/45340/19/src/soc/amd/picasso/acpi.c File src/soc/amd/picasso/acpi.c:
https://review.coreboot.org/c/coreboot/+/45340/19/src/soc/amd/picasso/acpi.c... PS19, Line 220: 10
I can factor in the 10*100. I kept this format as I ported over the way Agesa does the calculation.
Sorry, I don't understand the comment. If you don't change this to 1000, then, power is not really mW?
https://review.coreboot.org/c/coreboot/+/45340/19/src/soc/amd/picasso/acpi.c... PS19, Line 231: default
I believe so. I had to go back to Stoney BKDG to see the values. 0x11 is a reserved value.
For Family 17h this cannot be 0x11 as this is a 2-bit field.