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 25:
(5 comments)
https://review.coreboot.org/c/coreboot/+/45340/24/src/soc/amd/picasso/acpi.c File src/soc/amd/picasso/acpi.c:
https://review.coreboot.org/c/coreboot/+/45340/24/src/soc/amd/picasso/acpi.c... PS24, Line 330: .ctype = 2, : .latency = 400, : .power = 0, : .resource = { : .space_id = ACPI_ADDRESS_SPACE_IO, : .bit_width = 8, : .bit_offset = 0, : .addrl = 0, /* Address will be initialized later : from MSR_CSTATE_ADDRESS */ : .addrh = 0, : .access_size = ACPI_ACCESS_SIZE_BYTE_ACCESS, : }, : },
Same here.
Done
https://review.coreboot.org/c/coreboot/+/45340/24/src/soc/amd/picasso/acpi.c... PS24, Line 379: cstate_info[1].resource.addrl = : (rdmsr(MSR_CSTATE_ADDRESS).lo & MSR_CSTATE_ADDRESS_MASK) + 1;
Moved out of the loop, but in order to make this const, i will have to modify the helper function. […]
Done
https://review.coreboot.org/c/coreboot/+/45340/24/src/soc/amd/picasso/includ... File src/soc/amd/picasso/include/soc/msr.h:
https://review.coreboot.org/c/coreboot/+/45340/24/src/soc/amd/picasso/includ... PS24, Line 3: /* This file applies to AMD64 products. : * The definitions come from the AMD64 Programmers Manual vol2 : * Revision 3.30 and/or the device's PPR.
This is not really correct. […]
Done
https://review.coreboot.org/c/coreboot/+/45340/24/src/soc/amd/picasso/includ... PS24, Line 11: #include <cpu/amd/msr.h>
Why is this included here? This is required by picasso/acpi. […]
Done
https://review.coreboot.org/c/coreboot/+/45340/24/src/soc/amd/picasso/includ... PS24, Line 32: #define SERIAL_VID_DECODE_MICROVOLTS 6250 : #define SERIAL_VID_MAX_MICROVOLTS 1550000L
Is this also Family 17h specific?
I moved this back into common