Richard Spiegel has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31373
Change subject: vendorcode/amd/pi: Add merlinfalcon vendor code ......................................................................
vendorcode/amd/pi: Add merlinfalcon vendor code
Merlinfalcon is essentially stoneyridge (00670F00), but using a carizo CPU. Therefore, it needs carizo AGESA (00660F01), but a stoneyridge vendor code with a small modification to gcccar.inc due to carizo AGESA requiring CAR at 0x3000 to 0x4000.
Please notice: Most of the new files are duplicate from src/vendorcode/amd/pi/00670F00, the only difference is gcccar.inc which has 1 extra instruction. I would rather have some conditional build on a config parameter, but I was not aware how to do it for an assembly file. I'm open to suggestions.
BUG=b:none. TEST=Tested later with padmelon board.
Change-Id: I01b4cdef01ba185fd0d96e6674f8b1c56307d1f2 Signed-off-by: Richard Spiegel richard.spiegel@silverbackltd.com --- A src/vendorcode/amd/pi/00670F00_CZ/AGESA.h A src/vendorcode/amd/pi/00670F00_CZ/AMD.h A src/vendorcode/amd/pi/00670F00_CZ/Include/Filecode.h A src/vendorcode/amd/pi/00670F00_CZ/Include/PlatformMemoryConfiguration.h A src/vendorcode/amd/pi/00670F00_CZ/Include/Topology.h A src/vendorcode/amd/pi/00670F00_CZ/Makefile.inc A src/vendorcode/amd/pi/00670F00_CZ/Porting.h A src/vendorcode/amd/pi/00670F00_CZ/Proc/CPU/Family/cpuFamRegisters.h A src/vendorcode/amd/pi/00670F00_CZ/Proc/CPU/Table.h A src/vendorcode/amd/pi/00670F00_CZ/Proc/CPU/cpuFamilyTranslation.h A src/vendorcode/amd/pi/00670F00_CZ/Proc/CPU/cpuRegisters.h A src/vendorcode/amd/pi/00670F00_CZ/Proc/CPU/cpuServices.h A src/vendorcode/amd/pi/00670F00_CZ/Proc/CPU/heapManager.h A src/vendorcode/amd/pi/00670F00_CZ/Proc/Common/AmdFch.h A src/vendorcode/amd/pi/00670F00_CZ/Proc/Fch/Common/FchCommonCfg.h A src/vendorcode/amd/pi/00670F00_CZ/Proc/Fch/Fch.h A src/vendorcode/amd/pi/00670F00_CZ/Proc/Fch/FchPlatform.h A src/vendorcode/amd/pi/00670F00_CZ/agesa_headers.h A src/vendorcode/amd/pi/00670F00_CZ/binaryPI/AGESA.c A src/vendorcode/amd/pi/00670F00_CZ/binaryPI/OptionsIds.h A src/vendorcode/amd/pi/00670F00_CZ/binaryPI/gcccar.inc A src/vendorcode/amd/pi/00670F00_CZ/check_for_wrapper.h A src/vendorcode/amd/pi/00670F00_CZ/gcc-intrin.h M src/vendorcode/amd/pi/Kconfig 24 files changed, 15,360 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/31373/1
Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31373 )
Change subject: vendorcode/amd/pi: Add merlinfalcon AGESA support ......................................................................
Patch Set 2:
Marshall, check if this is what you wish.
Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31373 )
Change subject: vendorcode/amd/pi: Add merlinfalcon AGESA support ......................................................................
Patch Set 2:
(2 comments)
Patch Set 2:
(2 comments)
This should be split and the commit message redone. FWIW, could probably put the Kconfig change in https://review.coreboot.org/#/c/coreboot/+/31372/
I should not place vendor code Kconfig with soc/amd/stoneyridge changes. This change is about providing AGESA to merlinfalcon, thus Kconfig and gcccar.inc changes belong here.
https://review.coreboot.org/#/c/31373/2/src/vendorcode/amd/pi/00670F00/binar... File src/vendorcode/amd/pi/00670F00/binaryPI/gcccar.inc:
https://review.coreboot.org/#/c/31373/2/src/vendorcode/amd/pi/00670F00/binar... PS2, Line 1160: # This one instruction is for merlinfalcon, harmless for stoneyridge.
I wouldn't bother with this. You could change the comment below to say it's for all cores. […]
Ok, I'll use his comment.
https://review.coreboot.org/#/c/31373/2/src/vendorcode/amd/pi/Kconfig File src/vendorcode/amd/pi/Kconfig:
https://review.coreboot.org/#/c/31373/2/src/vendorcode/amd/pi/Kconfig@46 PS2, Line 46: "3rdparty/blobs/pi/amd/00660F01/FP4/AGESA.bin"
Trying to think ahead... […]
As you mentioned, this forces the break into multiple commits. 1 commit to the blob, committing new blob to main coreboot (already needed due to video), and finally the kconfig plus gcccar.inc.
Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31373 )
Change subject: vendorcode/amd/pi: Add merlinfalcon AGESA support ......................................................................
Patch Set 3:
(2 comments)
This change is ready for review.
https://review.coreboot.org/#/c/31373/3/src/vendorcode/amd/pi/00670F00/binar... File src/vendorcode/amd/pi/00670F00/binaryPI/gcccar.inc:
https://review.coreboot.org/#/c/31373/3/src/vendorcode/amd/pi/00670F00/binar... PS3, Line 1159:
This should not be squashed here. We have separate patch: […]
Agree, I'll align with your code.
https://review.coreboot.org/#/c/31373/3/src/vendorcode/amd/pi/Kconfig File src/vendorcode/amd/pi/Kconfig:
https://review.coreboot.org/#/c/31373/3/src/vendorcode/amd/pi/Kconfig@37 PS3, Line 37: default "src/vendorcode/amd/pi/00670F00" if SOC_AMD_STONEYRIDGE_FT4
I dont' really approve the 3x 00670f00/ but I doubt that you will clean it. […]
Unfortunately it's not so easy. There are older projects using Carrizo using a different stile (separate south and north bridges) that requires 00660F01 and must be maintained. What I'm saying is that when you adopt a SOC style for Carrizo (Merlin Falcon), you must use 00670F00.
Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31373 )
Change subject: vendorcode/amd/pi: Add merlinfalcon AGESA support ......................................................................
Patch Set 5:
(1 comment)
This change is ready for review.
https://review.coreboot.org/#/c/31373/3/src/vendorcode/amd/pi/Kconfig File src/vendorcode/amd/pi/Kconfig:
https://review.coreboot.org/#/c/31373/3/src/vendorcode/amd/pi/Kconfig@37 PS3, Line 37: default "src/vendorcode/amd/pi/00670F00" if SOC_AMD_STONEYRIDGE_FT4
I dont' really approve the 3x 00670f00/... […]
Once the 00670F00 FP4 AGESA is building properly, I can go back and try with padmelon. If it works, I can replace it. Until then, it will be using 00660F01 FP4 AGESA which is known to work.
Richard Spiegel has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/31373 )
Change subject: vendorcode/amd/pi: Add merlinfalcon AGESA support ......................................................................
Abandoned
Code was developed with wrong padmelon, not using merlinfalcon. New one coming in.
-
Richard Spiegel (Code Review)