Jason Glenesk 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 11:
(7 comments)
https://review.coreboot.org/c/coreboot/+/45340/10/src/include/cpu/amd/msr.h File src/include/cpu/amd/msr.h:
https://review.coreboot.org/c/coreboot/+/45340/10/src/include/cpu/amd/msr.h@... PS10, Line 14:
maybe add another white space.
I will submit cleanup in a follow up review and set them all to 2 spaces.
https://review.coreboot.org/c/coreboot/+/45340/10/src/include/cpu/amd/msr.h@... PS10, Line 16:
maybe 2 whitespaces
Ack
https://review.coreboot.org/c/coreboot/+/45340/10/src/include/cpu/amd/msr.h@... PS10, Line 17:
also here ?
Ack
https://review.coreboot.org/c/coreboot/+/45340/10/src/include/cpu/amd/msr.h@... PS10, Line 34:
2 whitespaces?
Ack
https://review.coreboot.org/c/coreboot/+/45340/10/src/include/cpu/amd/msr.h@... PS10, Line 50: #define PSTATE_DEF_HI_ENABLE_SHIFT 31 : #define PSTATE_DEF_HI_ENABLE_MASK (0x1 << PSTATE_DEF_HI_ENABLE_SHIFT) : #define PSTATE_DEF_LO_CUR_DIV_SHIFT 30 : #define PSTATE_DEF_LO_CUR_DIV_MASK (0x3 << PSTATE_DEF_LO_CUR_DIV_SHIFT) : #define PSTATE_DEF_LO_CUR_VAL_SHIFT 22 : #define PSTATE_DEF_LO_CUR_VAL_MASK (0xFF << PSTATE_DEF_LO_CUR_VAL_SHIFT) : #define PSTATE_DEF_LO_CORE_VID_SHIFT 14 : #define PSTATE_DEF_LO_CORE_VID_MASK (0xFF << PSTATE_DEF_LO_CORE_VID_SHIFT) : #define PSTATE_DEF_LO_FREQ_DIV_SHIFT 8 : #define PSTATE_DEF_LO_FREQ_DIV_MASK (0x3F << PSTATE_DEF_LO_FREQ_DIV_SHIFT) : #define PSTATE_DEF_LO_FREQ_DIV_MIN 0x8 : #define PSTATE_DEF_LO_FREQ_DIV_MAX 0x3E : #define PSTATE_DEF_LO_FREQ_MUL_SHIFT 0 : #define PSTATE_DEF_LO_FREQ_MUL_MASK (0xFF << PSTATE_DEF_LO_FREQ_MUL_SHIFT) : #define PSTATE_DEF_LO_CORE_FREQ_BASE
2 whitespaces?
Ack
https://review.coreboot.org/c/coreboot/+/45340/10/src/include/cpu/amd/msr.h@... PS10, Line 71:
also here ?
Ack
https://review.coreboot.org/c/coreboot/+/45340/7/src/soc/amd/picasso/Kconfig File src/soc/amd/picasso/Kconfig:
https://review.coreboot.org/c/coreboot/+/45340/7/src/soc/amd/picasso/Kconfig... PS7, Line 345: AMD recommends the ACPI _PSD object to be configured to cause : cores to transition between p-states independently. A vendor may : choose to generate _PSD object to allow cores to transition together.
I didn't know initially. I updated in patch 10.
Done