Piotr Kleinschmidt has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35906 )
Change subject: mb/pcengines/apu2: use older AGESA with different headers ......................................................................
mb/pcengines/apu2: use older AGESA with different headers
PC Engines apu2 platforms use AGESA 1.0.0.4 which uses different AGESA.h header than newer AGESA versions.
Change-Id: I3d85ee14e35dae8079e8d552b6530a3867f65876 Signed-off-by: Piotr Kleinschmidt piotr.kleins@gmail.com --- M src/mainboard/pcengines/apu2/Kconfig M src/vendorcode/amd/pi/00730F01/AGESA.h 2 files changed, 12 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/35906/1
diff --git a/src/mainboard/pcengines/apu2/Kconfig b/src/mainboard/pcengines/apu2/Kconfig index 6c3958b..5a61ed1 100644 --- a/src/mainboard/pcengines/apu2/Kconfig +++ b/src/mainboard/pcengines/apu2/Kconfig @@ -114,4 +114,8 @@ int default 128
+config AGESA_USE_OLD_HEADER + bool + default y + endif # BOARD_PCENGINES_APU2 diff --git a/src/vendorcode/amd/pi/00730F01/AGESA.h b/src/vendorcode/amd/pi/00730F01/AGESA.h index c25b631..6053f3c 100644 --- a/src/vendorcode/amd/pi/00730F01/AGESA.h +++ b/src/vendorcode/amd/pi/00730F01/AGESA.h @@ -775,6 +775,7 @@ DP_VS_0_4V_9_5DB = 0x18 ///< 0x18 } DP_FIXED_VOLT_SWING_TYPE;
+#if CONFIG(AGESA_USE_OLD_HEADER) /// Alternative DRAM MAC typedef enum { MAC_UNTESTEDMAC, ///< Assign 0 to Untested MAC @@ -785,6 +786,7 @@ MAC_300k, ///< Assign 5 to 300k MAC_200k, ///< Assign 6 to 200k } DRAM_MAXIMUM_ACTIVATE_COUNT; +#endif
// Macro for statically initializing various structures #define PCIE_ENGINE_DATA_INITIALIZER(mType, mStartLane, mEndLane) {mType, mStartLane, mEndLane} @@ -1547,7 +1549,9 @@ ///< 667 (MHz) ///< 800 (MHz) ///< and so on... +#if CONFIG(AGESA_USE_OLD_HEADER) OUT UINT8 Mac; ///< Maximum Activate Count +#endif OUT UINT8 CasL; ///< CAS latency DCT setting (busclocks) OUT UINT8 Trcd; ///< DCT Trcd (busclocks) OUT UINT8 Trp; ///< DCT Trp (busclocks) @@ -1803,6 +1807,7 @@ ///< ///< @BldCfgItem{BLDCFG_MEMORY_POWER_DOWN}
+#if CONFIG(AGESA_USE_OLD_HEADER) // Dram Mac Default
IN UINT8 DramMacDefault; ///< Default Maximum Activate Count @@ -1818,6 +1823,7 @@ ///< @BldCfgItem{BLDCFG_MEMORY_EXTENDED_TEMPERATURE_RANGE} // Extended temperature range
+#endif // Online Spare
IN BOOLEAN EnableOnLineSpareCtl; ///< Chip Select Spare Control bit 0. @@ -2721,8 +2727,10 @@ IN BOOLEAN CfgMemoryEnableNodeInterleaving; ///< Memory Enable Node Interleaving. IN BOOLEAN CfgMemoryChannelInterleaving; ///< Memory Channel Interleaving. IN BOOLEAN CfgMemoryPowerDown; ///< Memory Power Down. +#if CONFIG(AGESA_USE_OLD_HEADER) IN UINT8 CfgMemoryMacDefault; ///< Memory DRAM MAC Default IN BOOLEAN CfgMemoryExtendedTemperatureRange; ///< Memory Extended Temperature Range +#endif IN UINT32 CfgPowerDownMode; ///< Power Down Mode. IN BOOLEAN CfgOnlineSpare; ///< Online Spare. IN BOOLEAN CfgMemoryParityEnable; ///< Memory Parity Enable.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35906 )
Change subject: mb/pcengines/apu2: use older AGESA with different headers ......................................................................
Patch Set 1:
Does it make sense to have support for different versions of a binaryPI in coreboot for the same hardware? Why not upgrade to a newer binaryPI?
Piotr Kleinschmidt has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35906 )
Change subject: mb/pcengines/apu2: use older AGESA with different headers ......................................................................
Patch Set 1:
Patch Set 1:
Does it make sense to have support for different versions of a binaryPI in coreboot for the same hardware? Why not upgrade to a newer binaryPI?
apu2 platforms have issues with newer binaryPI versions. That's why we stayed with 1.0.0.4
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35906 )
Change subject: mb/pcengines/apu2: use older AGESA with different headers ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/35906/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35906/1//COMMIT_MSG@11 PS1, Line 11: Is this just a precaution or does this fix real problems?
https://review.coreboot.org/c/coreboot/+/35906/1/src/mainboard/pcengines/apu... File src/mainboard/pcengines/apu2/Kconfig:
https://review.coreboot.org/c/coreboot/+/35906/1/src/mainboard/pcengines/apu... PS1, Line 117: config AGESA_USE_OLD_HEADER Use the version 1_0_0_4 instead of old?
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35906 )
Change subject: mb/pcengines/apu2: use older AGESA with different headers ......................................................................
Patch Set 1:
(1 comment)
Patch Set 1:
Patch Set 1:
Does it make sense to have support for different versions of a binaryPI in coreboot for the same hardware? Why not upgrade to a newer binaryPI?
apu2 platforms have issues with newer binaryPI versions. That's why we stayed with 1.0.0.4
I don't think having binaries + headers depend on a board is a good solution for upstream coreboot, as it creates maintainability issues. Not my call to make though.
https://review.coreboot.org/c/coreboot/+/35906/1/src/mainboard/pcengines/apu... File src/mainboard/pcengines/apu2/Kconfig:
https://review.coreboot.org/c/coreboot/+/35906/1/src/mainboard/pcengines/apu... PS1, Line 117: AGESA_USE_OLD_HEADER I'd be more specific about the version here.
Hello Kyösti Mälkki, Patrick Rudolph, Piotr Król, build bot (Jenkins), Michał Żygowski,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35906
to look at the new patch set (#2).
Change subject: mb/pcengines/apu2: use AGESA 1.0.0.4 with different AGESA.h headers ......................................................................
mb/pcengines/apu2: use AGESA 1.0.0.4 with different AGESA.h headers
PC Engines apu2 platforms use AGESA 1.0.0.4 which uses different AGESA.h header than newer AGESA versions.
Change-Id: I3d85ee14e35dae8079e8d552b6530a3867f65876 Signed-off-by: Piotr Kleinschmidt piotr.kleins@gmail.com --- M src/mainboard/pcengines/apu2/Kconfig M src/vendorcode/amd/pi/00730F01/AGESA.h 2 files changed, 12 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/35906/2
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35906 )
Change subject: mb/pcengines/apu2: use AGESA 1.0.0.4 with different AGESA.h headers ......................................................................
Patch Set 2:
(1 comment)
Patch Set 1:
(1 comment)
Patch Set 1:
Patch Set 1:
Does it make sense to have support for different versions of a binaryPI in coreboot for the same hardware? Why not upgrade to a newer binaryPI?
apu2 platforms have issues with newer binaryPI versions. That's why we stayed with 1.0.0.4
I don't think having binaries + headers depend on a board is a good solution for upstream coreboot, as it creates maintainability issues. Not my call to make though.
Resolving issues with closed source binaries isn't trivial. If we had a solution for the default AGESA 1.0.0.A we would have definitely fix it in mainboard code. I understand your concerns, but what options do we have at this moment? We may only drop this change and hope nothing blows up (fortunately nothing bad happened because of those headers - yet).
https://review.coreboot.org/c/coreboot/+/35906/1/src/mainboard/pcengines/apu... File src/mainboard/pcengines/apu2/Kconfig:
https://review.coreboot.org/c/coreboot/+/35906/1/src/mainboard/pcengines/apu... PS1, Line 117: AGESA_USE_OLD_HEADER
I'd be more specific about the version here.
How about some help section here that describes the problem a little and point to specific commit hash?
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35906 )
Change subject: mb/pcengines/apu2: use AGESA 1.0.0.4 with different AGESA.h headers ......................................................................
Patch Set 2:
seems like the logic is backwards here, if you want to prevent those fields from being included when using the 1.0.04 binary
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35906 )
Change subject: mb/pcengines/apu2: use AGESA 1.0.0.4 with different AGESA.h headers ......................................................................
Patch Set 2:
Patch Set 2:
seems like the logic is backwards here, if you want to prevent those fields from being included when using the 1.0.04 binary
Exactly. We basically want to carve out the changes introduced in https://review.coreboot.org/c/coreboot/+/11225/ just for PC Engines apu2 platform.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35906 )
Change subject: mb/pcengines/apu2: use AGESA 1.0.0.4 with different AGESA.h headers ......................................................................
Patch Set 2:
IMHO, the header files belong where the blob lives (blobs repo). This makes it much easier to keep them synchronized!
In this case, we should also revive the 1.0.0.4 binary on blobs' master branch. If we have per-version dirs there, we'd also have a place for the proper header files, without fragile preprocessor guards.
Hello Kyösti Mälkki, Patrick Rudolph, Piotr Król, build bot (Jenkins), Michał Żygowski,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35906
to look at the new patch set (#3).
Change subject: mb/pcengines/apu2: use AGESA 1.0.0.4 with adjusted AGESA header ......................................................................
mb/pcengines/apu2: use AGESA 1.0.0.4 with adjusted AGESA header
PC Engines apu2 platform uses AGESA 1.0.0.4, because upstream AGESA 1.0.0.A doesn't work on apu2 - platform doesn't boot. To properly utilize AGESA 1.0.0.4 we need to adjust AGESA header to state, which is compatible with AGESA 1.0.0.4 version.
Change-Id: I3d85ee14e35dae8079e8d552b6530a3867f65876 Signed-off-by: Piotr Kleinschmidt piotr.kleins@gmail.com --- M src/mainboard/pcengines/apu2/Kconfig M src/vendorcode/amd/pi/00730F01/AGESA.h 2 files changed, 12 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/35906/3
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35906 )
Change subject: mb/pcengines/apu2: use AGESA 1.0.0.4 with adjusted AGESA header ......................................................................
Patch Set 3:
Patch Set 2:
IMHO, the header files belong where the blob lives (blobs repo). This makes it much easier to keep them synchronized!
In this case, we should also revive the 1.0.0.4 binary on blobs' master branch. If we have per-version dirs there, we'd also have a place for the proper header files, without fragile preprocessor guards.
The 1.0.0.4 is present in mainboard/pcengines/apu2. The idea with having the headers in blobs repo like FSP is also good. I just didn't suggest that to Piotr since I have no idea about the rules of committing to blobs repo.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35906 )
Change subject: mb/pcengines/apu2: use AGESA 1.0.0.4 with adjusted AGESA header ......................................................................
Patch Set 3:
Patch Set 3:
Patch Set 2:
IMHO, the header files belong where the blob lives (blobs repo). This makes it much easier to keep them synchronized!
In this case, we should also revive the 1.0.0.4 binary on blobs' master branch. If we have per-version dirs there, we'd also have a place for the proper header files, without fragile preprocessor guards.
The 1.0.0.4 is present in mainboard/pcengines/apu2. The idea with having the headers in blobs repo like FSP is also good. I just didn't suggest that to Piotr since I have no idea about the rules of committing to blobs repo.
The license of the header permits to do that (redistribution), so go ahead!
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35906 )
Change subject: mb/pcengines/apu2: use AGESA 1.0.0.4 with adjusted AGESA header ......................................................................
Patch Set 3:
Patch Set 3:
Patch Set 3:
Patch Set 2:
IMHO, the header files belong where the blob lives (blobs repo). This makes it much easier to keep them synchronized!
In this case, we should also revive the 1.0.0.4 binary on blobs' master branch. If we have per-version dirs there, we'd also have a place for the proper header files, without fragile preprocessor guards.
The 1.0.0.4 is present in mainboard/pcengines/apu2. The idea with having the headers in blobs repo like FSP is also good. I just didn't suggest that to Piotr since I have no idea about the rules of committing to blobs repo.
The license of the header permits to do that (redistribution), so go ahead!
If this approach is more elegant and desirable, we will do that then.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35906 )
Change subject: mb/pcengines/apu2: use AGESA 1.0.0.4 with adjusted AGESA header ......................................................................
Patch Set 3:
IMHO, the header files belong where the blob lives (blobs repo). This makes it much easier to keep them synchronized!
In this case, we should also revive the 1.0.0.4 binary on blobs' master branch. If we have per-version dirs there, we'd also have a place for the proper header files, without fragile preprocessor guards.
The 1.0.0.4 is present in mainboard/pcengines/apu2. The idea with having the headers in blobs repo like FSP is also good. I just didn't suggest that to Piotr since I have no idea about the rules of committing to blobs repo.
Workflow is the same as for the main repo. It just needs somebody who is authorized to accept the change (Verified+1 isn't given automatically).
The license of the header permits to do that (redistribution), so go ahead!
If this approach is more elegant and desirable, we will do that then.
I think it is :)
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35906 )
Change subject: mb/pcengines/apu2: use AGESA 1.0.0.4 with adjusted AGESA header ......................................................................
Patch Set 3:
https://review.coreboot.org/c/blobs/+/10979
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35906 )
Change subject: mb/pcengines/apu2: use AGESA 1.0.0.4 with adjusted AGESA header ......................................................................
Patch Set 3:
Hmmm, well. If the header files are not in the same repo, and we have only one submodule pointer, it seems impossible to keep multiple versions sync'ed. Do I miss something? any better suggestions?
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35906 )
Change subject: mb/pcengines/apu2: use AGESA 1.0.0.4 with adjusted AGESA header ......................................................................
Patch Set 3:
Patch Set 3:
Hmmm, well. If the header files are not in the same repo, and we have only one submodule pointer, it seems impossible to keep multiple versions sync'ed. Do I miss something? any better suggestions?
How much of the headers are version specific anyway? Is there a clear cut somewhere?
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35906 )
Change subject: mb/pcengines/apu2: use AGESA 1.0.0.4 with adjusted AGESA header ......................................................................
Patch Set 3:
Patch Set 3:
Patch Set 3:
Hmmm, well. If the header files are not in the same repo, and we have only one submodule pointer, it seems impossible to keep multiple versions sync'ed. Do I miss something? any better suggestions?
How much of the headers are version specific anyway? Is there a clear cut somewhere?
This patch should be already the clear cut. Pointed the commit updating the headers in previously comments.
Michał Żygowski has uploaded a new patch set (#4) to the change originally created by Piotr Kleinschmidt. ( https://review.coreboot.org/c/coreboot/+/35906 )
Change subject: mb/pcengines/apu2: use AGESA 1.0.0.4 with adjusted AGESA header ......................................................................
mb/pcengines/apu2: use AGESA 1.0.0.4 with adjusted AGESA header
PC Engines apu2 platform uses AGESA 1.0.0.4, because upstream AGESA 1.0.0.A doesn't work on apu2 - platform doesn't boot. To properly utilize AGESA 1.0.0.4 we need to adjust AGESA header to state, which is compatible with AGESA 1.0.0.4 version.
Cut out the changes introduced in CB:11225 exclusively for apu2 board.
Change-Id: I3d85ee14e35dae8079e8d552b6530a3867f65876 Signed-off-by: Piotr Kleinschmidt piotr.kleins@gmail.com --- M src/mainboard/pcengines/apu2/Kconfig M src/vendorcode/amd/pi/00730F01/AGESA.h 2 files changed, 12 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/35906/4
Michał Żygowski has uploaded a new patch set (#5) to the change originally created by Piotr Kleinschmidt. ( https://review.coreboot.org/c/coreboot/+/35906 )
Change subject: mb/pcengines/apu2: use AGESA 1.0.0.4 with adjusted AGESA header ......................................................................
mb/pcengines/apu2: use AGESA 1.0.0.4 with adjusted AGESA header
PC Engines apu2 platform uses AGESA 1.0.0.4, because upstream AGESA 1.0.0.A doesn't work on apu2 - the platform doesn't boot. To properly utilize AGESA 1.0.0.4 we need to adjust AGESA header to state, which is compatible with AGESA 1.0.0.4 version.
Cut out the changes introduced in CB:11225 exclusively for apu2 board.
TEST=boot PC Engines apu2 and launch Debian Linux
Change-Id: I3d85ee14e35dae8079e8d552b6530a3867f65876 Signed-off-by: Piotr Kleinschmidt piotr.kleins@gmail.com Signed-off-by: Michał Żygowski michal.zygowski@3mdeb.com --- M src/mainboard/pcengines/apu2/Kconfig M src/vendorcode/amd/pi/00730F01/AGESA.h 2 files changed, 12 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/35906/5
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35906 )
Change subject: mb/pcengines/apu2: use AGESA 1.0.0.4 with adjusted AGESA header ......................................................................
Patch Set 6: Code-Review+1
Not a fan of the #if's. But I don't want to block this. Somebody who's actually using this header file, or knows how likely future updates are, should decide, though.
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35906 )
Change subject: mb/pcengines/apu2: use AGESA 1.0.0.4 with adjusted AGESA header ......................................................................
Patch Set 6:
I do not plan to keep the #if's forever. I would like to put them temporarily until I manage to workaround the issue with AGESA 1.0.0.A
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35906 )
Change subject: mb/pcengines/apu2: use AGESA 1.0.0.4 with adjusted AGESA header ......................................................................
Patch Set 6:
Sorry for the further delay. If there are no objections / additional comments in the next few days, I'll just ack it.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35906 )
Change subject: mb/pcengines/apu2: use AGESA 1.0.0.4 with adjusted AGESA header ......................................................................
Patch Set 6:
There are some unresolved comments on patch set 1. Though, looks they were addressed already.
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35906 )
Change subject: mb/pcengines/apu2: use AGESA 1.0.0.4 with adjusted AGESA header ......................................................................
Patch Set 6:
Patch Set 6:
There are some unresolved comments on patch set 1. Though, looks they were addressed already.
Yes, I missed that. Thank you, Nico.
Michał Żygowski has uploaded a new patch set (#7) to the change originally created by Piotr Kleinschmidt. ( https://review.coreboot.org/c/coreboot/+/35906 )
Change subject: mb/pcengines/apu2: use AGESA 1.0.0.4 with adjusted AGESA header ......................................................................
mb/pcengines/apu2: use AGESA 1.0.0.4 with adjusted AGESA header
PC Engines apu2 platform uses AGESA 1.0.0.4, because upstream AGESA 1.0.0.A doesn't work on apu2 - the platform doesn't boot. To properly utilize AGESA 1.0.0.4 we need to adjust AGESA header to state, which is compatible with AGESA 1.0.0.4 version.
Cut out the changes introduced in CB:11225 exclusively for apu2 board.
TEST=boot PC Engines apu2 and launch Debian Linux
Change-Id: I3d85ee14e35dae8079e8d552b6530a3867f65876 Signed-off-by: Piotr Kleinschmidt piotr.kleins@gmail.com Signed-off-by: Michał Żygowski michal.zygowski@3mdeb.com --- M src/mainboard/pcengines/apu2/Kconfig M src/vendorcode/amd/pi/00730F01/AGESA.h 2 files changed, 18 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/35906/7
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35906 )
Change subject: mb/pcengines/apu2: use AGESA 1.0.0.4 with adjusted AGESA header ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35906/7/src/mainboard/pcengines/apu... File src/mainboard/pcengines/apu2/Kconfig:
https://review.coreboot.org/c/coreboot/+/35906/7/src/mainboard/pcengines/apu... PS7, Line 123: workarounds the problem, however some headers changes between blob trailing whitespace
Michał Żygowski has uploaded a new patch set (#8) to the change originally created by Piotr Kleinschmidt. ( https://review.coreboot.org/c/coreboot/+/35906 )
Change subject: mb/pcengines/apu2: use AGESA 1.0.0.4 with adjusted AGESA header ......................................................................
mb/pcengines/apu2: use AGESA 1.0.0.4 with adjusted AGESA header
PC Engines apu2 platform uses AGESA 1.0.0.4, because upstream AGESA 1.0.0.A doesn't work on apu2 - the platform doesn't boot. To properly utilize AGESA 1.0.0.4 we need to adjust AGESA header to state, which is compatible with AGESA 1.0.0.4 version.
Cut out the changes introduced in CB:11225 exclusively for apu2 board.
TEST=boot PC Engines apu2 and launch Debian Linux
Change-Id: I3d85ee14e35dae8079e8d552b6530a3867f65876 Signed-off-by: Piotr Kleinschmidt piotr.kleins@gmail.com Signed-off-by: Michał Żygowski michal.zygowski@3mdeb.com --- M src/mainboard/pcengines/apu2/Kconfig M src/vendorcode/amd/pi/00730F01/AGESA.h 2 files changed, 18 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/35906/8
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35906 )
Change subject: mb/pcengines/apu2: use AGESA 1.0.0.4 with adjusted AGESA header ......................................................................
Patch Set 8:
(3 comments)
https://review.coreboot.org/c/coreboot/+/35906/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35906/1//COMMIT_MSG@11 PS1, Line 11:
Is this just a precaution or does this fix real problems?
Just a precaution.
https://review.coreboot.org/c/coreboot/+/35906/1/src/mainboard/pcengines/apu... File src/mainboard/pcengines/apu2/Kconfig:
https://review.coreboot.org/c/coreboot/+/35906/1/src/mainboard/pcengines/apu... PS1, Line 117: AGESA_USE_OLD_HEADER
How about some help section here that describes the problem a little and point to specific commit ha […]
Done
https://review.coreboot.org/c/coreboot/+/35906/1/src/mainboard/pcengines/apu... PS1, Line 117: config AGESA_USE_OLD_HEADER
Use the version 1_0_0_4 instead of old?
Done
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35906 )
Change subject: mb/pcengines/apu2: use AGESA 1.0.0.4 with adjusted AGESA header ......................................................................
Patch Set 8: Code-Review+2
Nico Huber has submitted this change. ( https://review.coreboot.org/c/coreboot/+/35906 )
Change subject: mb/pcengines/apu2: use AGESA 1.0.0.4 with adjusted AGESA header ......................................................................
mb/pcengines/apu2: use AGESA 1.0.0.4 with adjusted AGESA header
PC Engines apu2 platform uses AGESA 1.0.0.4, because upstream AGESA 1.0.0.A doesn't work on apu2 - the platform doesn't boot. To properly utilize AGESA 1.0.0.4 we need to adjust AGESA header to state, which is compatible with AGESA 1.0.0.4 version.
Cut out the changes introduced in CB:11225 exclusively for apu2 board.
TEST=boot PC Engines apu2 and launch Debian Linux
Change-Id: I3d85ee14e35dae8079e8d552b6530a3867f65876 Signed-off-by: Piotr Kleinschmidt piotr.kleins@gmail.com Signed-off-by: Michał Żygowski michal.zygowski@3mdeb.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/35906 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Nico Huber nico.h@gmx.de --- M src/mainboard/pcengines/apu2/Kconfig M src/vendorcode/amd/pi/00730F01/AGESA.h 2 files changed, 18 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved
diff --git a/src/mainboard/pcengines/apu2/Kconfig b/src/mainboard/pcengines/apu2/Kconfig index 8c713e5..501d583 100644 --- a/src/mainboard/pcengines/apu2/Kconfig +++ b/src/mainboard/pcengines/apu2/Kconfig @@ -114,4 +114,14 @@ int default 128
+config AGESA_USE_1_0_0_4_HEADER + bool + default y + help + Due to a bug in AGESA 1.0.0.A affecting boards without UMA, it is + impossible to use the newest blob. Using an older 1.0.0.4 blob + workarounds the problem, however some headers changes between blob + revisions. This option removes the changes in headers introduced + with AGESA 1.0.0.A to fit the 1.0.0.4 revision. + endif # BOARD_PCENGINES_APU2 diff --git a/src/vendorcode/amd/pi/00730F01/AGESA.h b/src/vendorcode/amd/pi/00730F01/AGESA.h index c25b631..5a3ee5b 100644 --- a/src/vendorcode/amd/pi/00730F01/AGESA.h +++ b/src/vendorcode/amd/pi/00730F01/AGESA.h @@ -775,6 +775,7 @@ DP_VS_0_4V_9_5DB = 0x18 ///< 0x18 } DP_FIXED_VOLT_SWING_TYPE;
+#if CONFIG(AGESA_USE_1_0_0_4_HEADER) /// Alternative DRAM MAC typedef enum { MAC_UNTESTEDMAC, ///< Assign 0 to Untested MAC @@ -785,6 +786,7 @@ MAC_300k, ///< Assign 5 to 300k MAC_200k, ///< Assign 6 to 200k } DRAM_MAXIMUM_ACTIVATE_COUNT; +#endif
// Macro for statically initializing various structures #define PCIE_ENGINE_DATA_INITIALIZER(mType, mStartLane, mEndLane) {mType, mStartLane, mEndLane} @@ -1547,7 +1549,9 @@ ///< 667 (MHz) ///< 800 (MHz) ///< and so on... +#if CONFIG(AGESA_USE_1_0_0_4_HEADER) OUT UINT8 Mac; ///< Maximum Activate Count +#endif OUT UINT8 CasL; ///< CAS latency DCT setting (busclocks) OUT UINT8 Trcd; ///< DCT Trcd (busclocks) OUT UINT8 Trp; ///< DCT Trp (busclocks) @@ -1803,6 +1807,7 @@ ///< ///< @BldCfgItem{BLDCFG_MEMORY_POWER_DOWN}
+#if CONFIG(AGESA_USE_1_0_0_4_HEADER) // Dram Mac Default
IN UINT8 DramMacDefault; ///< Default Maximum Activate Count @@ -1818,6 +1823,7 @@ ///< @BldCfgItem{BLDCFG_MEMORY_EXTENDED_TEMPERATURE_RANGE} // Extended temperature range
+#endif // Online Spare
IN BOOLEAN EnableOnLineSpareCtl; ///< Chip Select Spare Control bit 0. @@ -2721,8 +2727,10 @@ IN BOOLEAN CfgMemoryEnableNodeInterleaving; ///< Memory Enable Node Interleaving. IN BOOLEAN CfgMemoryChannelInterleaving; ///< Memory Channel Interleaving. IN BOOLEAN CfgMemoryPowerDown; ///< Memory Power Down. +#if CONFIG(AGESA_USE_1_0_0_4_HEADER) IN UINT8 CfgMemoryMacDefault; ///< Memory DRAM MAC Default IN BOOLEAN CfgMemoryExtendedTemperatureRange; ///< Memory Extended Temperature Range +#endif IN UINT32 CfgPowerDownMode; ///< Power Down Mode. IN BOOLEAN CfgOnlineSpare; ///< Online Spare. IN BOOLEAN CfgMemoryParityEnable; ///< Memory Parity Enable.
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35906 )
Change subject: mb/pcengines/apu2: use AGESA 1.0.0.4 with adjusted AGESA header ......................................................................
Patch Set 9:
Automatic boot test returned (PASS/FAIL/TOTAL): 3/0/3 Emulation targets: EMULATION_QEMU_X86_Q35 using payload TianoCore : SUCCESS : https://lava.9esec.io/r/465 EMULATION_QEMU_X86_Q35 using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/464 EMULATION_QEMU_X86_I440FX using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/463
Please note: This test is under development and might not be accurate at all!
-
9elements QA (Code Review) -
Arthur Heymans (Code Review)
-
build bot (Jenkins) (Code Review)
-
Kyösti Mälkki (Code Review)
-
Matt DeVillier (Code Review)
-
Michał Żygowski (Code Review)
-
Nico Huber (Code Review)
-
Paul Menzel (Code Review)
-
Piotr Kleinschmidt (Code Review)