Change in coreboot[master]: nb/amd/agesa: select ROMSTAGE_CACHED_CBMEM
Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37198 ) Change subject: nb/amd/agesa: select ROMSTAGE_CACHED_CBMEM ...................................................................... nb/amd/agesa: select ROMSTAGE_CACHED_CBMEM AGESA sets up MTRR's to cache the whole dram, so decompressing postcar stage to cbmem should be fast and is now selected by default. Change-Id: I62ffe1bd646e9ddad77be240f030601790f4da4b Signed-off-by: Arthur Heymans <arthur@aheymans.xyz> --- M src/northbridge/amd/agesa/family14/Kconfig M src/northbridge/amd/agesa/family15tn/Kconfig M src/northbridge/amd/agesa/family16kb/Kconfig 3 files changed, 3 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/98/37198/1 diff --git a/src/northbridge/amd/agesa/family14/Kconfig b/src/northbridge/amd/agesa/family14/Kconfig index 173714f..cfafb23 100644 --- a/src/northbridge/amd/agesa/family14/Kconfig +++ b/src/northbridge/amd/agesa/family14/Kconfig @@ -14,6 +14,7 @@ ## config NORTHBRIDGE_AMD_AGESA_FAMILY14 bool + select ROMSTAGE_CACHED_CBMEM if NORTHBRIDGE_AMD_AGESA_FAMILY14 diff --git a/src/northbridge/amd/agesa/family15tn/Kconfig b/src/northbridge/amd/agesa/family15tn/Kconfig index a0841eb..e6a469f 100644 --- a/src/northbridge/amd/agesa/family15tn/Kconfig +++ b/src/northbridge/amd/agesa/family15tn/Kconfig @@ -14,6 +14,7 @@ ## config NORTHBRIDGE_AMD_AGESA_FAMILY15_TN bool + select ROMSTAGE_CACHED_CBMEM if NORTHBRIDGE_AMD_AGESA_FAMILY15_TN diff --git a/src/northbridge/amd/agesa/family16kb/Kconfig b/src/northbridge/amd/agesa/family16kb/Kconfig index 2be2fd3..e34ef60 100644 --- a/src/northbridge/amd/agesa/family16kb/Kconfig +++ b/src/northbridge/amd/agesa/family16kb/Kconfig @@ -15,6 +15,7 @@ ## config NORTHBRIDGE_AMD_AGESA_FAMILY16_KB bool + select ROMSTAGE_CACHED_CBMEM if NORTHBRIDGE_AMD_AGESA_FAMILY16_KB -- To view, visit https://review.coreboot.org/c/coreboot/+/37198 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I62ffe1bd646e9ddad77be240f030601790f4da4b Gerrit-Change-Number: 37198 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-MessageType: newchange
Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/37198 to look at the new patch set (#3). Change subject: nb/amd/agesa: select ROMSTAGE_CACHED_CBMEM ...................................................................... nb/amd/agesa: select ROMSTAGE_CACHED_CBMEM AGESA sets up MTRR's to cache the whole dram, so decompressing postcar stage to cbmem should be fast and is now selected by default. Change-Id: I62ffe1bd646e9ddad77be240f030601790f4da4b Signed-off-by: Arthur Heymans <arthur@aheymans.xyz> --- M src/northbridge/amd/agesa/family14/Kconfig M src/northbridge/amd/agesa/family15tn/Kconfig M src/northbridge/amd/agesa/family16kb/Kconfig 3 files changed, 3 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/98/37198/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/37198 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I62ffe1bd646e9ddad77be240f030601790f4da4b Gerrit-Change-Number: 37198 Gerrit-PatchSet: 3 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-MessageType: newpatchset
Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/37198 to look at the new patch set (#7). Change subject: nb/amd/agesa: select ROMSTAGE_CACHED_CBMEM ...................................................................... nb/amd/agesa: select ROMSTAGE_CACHED_CBMEM AGESA sets up MTRR's to cache the whole dram, so decompressing postcar stage to cbmem should be fast and is now selected by default. Postcar stage will now be compressed by default. Change-Id: I62ffe1bd646e9ddad77be240f030601790f4da4b Signed-off-by: Arthur Heymans <arthur@aheymans.xyz> --- M src/northbridge/amd/agesa/family14/Kconfig M src/northbridge/amd/agesa/family15tn/Kconfig M src/northbridge/amd/agesa/family16kb/Kconfig 3 files changed, 3 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/98/37198/7 -- To view, visit https://review.coreboot.org/c/coreboot/+/37198 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I62ffe1bd646e9ddad77be240f030601790f4da4b Gerrit-Change-Number: 37198 Gerrit-PatchSet: 7 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-MessageType: newpatchset
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37198 ) Change subject: nb/amd/agesa: select ROMSTAGE_CACHED_CBMEM ...................................................................... Patch Set 7: In #coreboot@irc.freenode.net, Arthur asked for tests on AGESA platforms. -- To view, visit https://review.coreboot.org/c/coreboot/+/37198 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I62ffe1bd646e9ddad77be240f030601790f4da4b Gerrit-Change-Number: 37198 Gerrit-PatchSet: 7 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Krystian Hebel <krystian.hebel@3mdeb.com> Gerrit-CC: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-CC: Mike Banon <mikebdp2@gmail.com> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Tue, 26 May 2020 14:28:28 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Mike Banon has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37198 ) Change subject: nb/amd/agesa: select ROMSTAGE_CACHED_CBMEM ...................................................................... Patch Set 7:
Patch Set 7:
In #coreboot@irc.freenode.net, Arthur asked for tests on AGESA platforms.
Thanks, hopefully I'll find the time to test it on 15h and 16h -- To view, visit https://review.coreboot.org/c/coreboot/+/37198 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I62ffe1bd646e9ddad77be240f030601790f4da4b Gerrit-Change-Number: 37198 Gerrit-PatchSet: 7 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Krystian Hebel <krystian.hebel@3mdeb.com> Gerrit-CC: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-CC: Mike Banon <mikebdp2@gmail.com> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Wed, 27 May 2020 09:20:37 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37198 ) Change subject: nb/amd/agesa: select ROMSTAGE_CACHED_CBMEM ...................................................................... Patch Set 7: Code-Review+1 So far it's good on fam14. Postcar is decompressed during boot and does not cause issues. -- To view, visit https://review.coreboot.org/c/coreboot/+/37198 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I62ffe1bd646e9ddad77be240f030601790f4da4b Gerrit-Change-Number: 37198 Gerrit-PatchSet: 7 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Krystian Hebel <krystian.hebel@3mdeb.com> Gerrit-CC: Mike Banon <mikebdp2@gmail.com> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Wed, 27 May 2020 11:28:42 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37198 ) Change subject: nb/amd/agesa: select ROMSTAGE_CACHED_CBMEM ...................................................................... Patch Set 7: That AGESA (or any blob) manipulates MTRRs is discouraged and I understand such behaviour was removed for amd/stoneyridge. Also, src/vendorcode/amd/agesa/f14/Legacy/Proc/hobTransfer.c shows there is some back-and-forth copying of AGESA heap. The code is form of CAR migration with intermediate buffer kept as uncacheable. -- To view, visit https://review.coreboot.org/c/coreboot/+/37198 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I62ffe1bd646e9ddad77be240f030601790f4da4b Gerrit-Change-Number: 37198 Gerrit-PatchSet: 7 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Marshall Dawson <marshall.dawson@amd.corp-partner.google.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Krystian Hebel <krystian.hebel@3mdeb.com> Gerrit-CC: Kyösti Mälkki <kyosti.malkki@gmail.com> Gerrit-CC: Mike Banon <mikebdp2@gmail.com> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Wed, 27 May 2020 13:12:47 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37198 ) Change subject: nb/amd/agesa: select ROMSTAGE_CACHED_CBMEM ...................................................................... Patch Set 7:
Patch Set 7:
That AGESA (or any blob) manipulates MTRRs is discouraged and I understand such behaviour was removed for amd/stoneyridge.
Also, src/vendorcode/amd/agesa/f14/Legacy/Proc/hobTransfer.c shows there is some back-and-forth copying of AGESA heap. The code is form of CAR migration with intermediate buffer kept as uncacheable.
Right. I suppose the commit message is not entirely accurate then. What I meant was that the location where cbmem ends up will be set up as WB by AGESA and that this is recommended if one wants to decompress+clflush postcar (+ other things in cbmem) there. -- To view, visit https://review.coreboot.org/c/coreboot/+/37198 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I62ffe1bd646e9ddad77be240f030601790f4da4b Gerrit-Change-Number: 37198 Gerrit-PatchSet: 7 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Marshall Dawson <marshall.dawson@amd.corp-partner.google.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Krystian Hebel <krystian.hebel@3mdeb.com> Gerrit-CC: Kyösti Mälkki <kyosti.malkki@gmail.com> Gerrit-CC: Mike Banon <mikebdp2@gmail.com> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Wed, 27 May 2020 13:27:39 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37198 ) Change subject: nb/amd/agesa: select ROMSTAGE_CACHED_CBMEM ...................................................................... Patch Set 7: On the ASRock E350M1: 4.12-374-g0013623b7c: ``` 27 entries total: 0:1st timestamp 7,931 11:start of bootblock 14,414 (6,482) 12:end of bootblock 15,624 (1,210) 13:starting to load romstage 16,214 (589) 14:finished loading romstage 16,301 (87) 1:start of romstage 16,605 (303) 2:before ram initialization 129,739 (113,134) 3:after ram initialization 411,357 (281,617) 4:end of romstage 426,279 (14,921) 100:start of postcar 426,418 (138) 101:end of postcar 426,418 (0) 8:starting to load ramstage 426,528 (109) 15:starting LZMA decompress (ignore for x86) 426,564 (36) 16:finished LZMA decompress (ignore for x86) 518,200 (91,636) 9:finished loading ramstage 518,247 (46) 10:start of ramstage 518,269 (21) 30:device enumeration 522,196 (3,926) 40:device configuration 659,462 (137,266) 50:device enable 672,386 (12,924) 60:device initialization 672,859 (473) 70:device setup done 687,333 (14,474) 75:cbmem post 932,112 (244,778) 80:write tables 932,113 (1) 85:finalize chips 973,568 (41,454) 90:load payload 973,572 (4) 15:starting LZMA decompress (ignore for x86) 974,055 (483) 16:finished LZMA decompress (ignore for x86) 1,027,445 (53,389) 99:selfboot jump 1,027,502 (57) Total Time: 1,019,558 ```` Your change-set rebased on origin/master (`cpu/x86/cache.h` added for `clflush_supported()`): ``` 28 entries total: 0:1st timestamp 7,886 11:start of bootblock 13,556 (5,670) 12:end of bootblock 14,705 (1,148) 13:starting to load romstage 15,285 (580) 14:finished loading romstage 15,371 (86) 1:start of romstage 15,652 (280) 2:before ram initialization 129,582 (113,930) 3:after ram initialization 414,538 (284,955) 17:starting LZ4 decompress (ignore for x86) 421,713 (7,175) 18:finished LZ4 decompress (ignore for x86) 427,201 (5,487) 100:start of postcar 428,672 (1,471) 101:end of postcar 428,672 (0) 8:starting to load ramstage 428,783 (110) 15:starting LZMA decompress (ignore for x86) 428,819 (36) 16:finished LZMA decompress (ignore for x86) 520,412 (91,593) 9:finished loading ramstage 520,459 (46) 10:start of ramstage 520,480 (21) 30:device enumeration 524,407 (3,927) 40:device configuration 661,704 (137,297) 50:device enable 674,636 (12,931) 60:device initialization 675,105 (468) 70:device setup done 689,625 (14,520) 75:cbmem post 953,917 (264,291) 80:write tables 953,918 (1) 85:finalize chips 995,266 (41,348) 90:load payload 995,271 (4) 15:starting LZMA decompress (ignore for x86) 995,755 (483) 16:finished LZMA decompress (ignore for x86) 1,049,208 (53,453) 99:selfboot jump 1,049,265 (57) Total Time: 1,041,368 ``` -- To view, visit https://review.coreboot.org/c/coreboot/+/37198 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I62ffe1bd646e9ddad77be240f030601790f4da4b Gerrit-Change-Number: 37198 Gerrit-PatchSet: 7 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Marshall Dawson <marshall.dawson@amd.corp-partner.google.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Krystian Hebel <krystian.hebel@3mdeb.com> Gerrit-CC: Kyösti Mälkki <kyosti.malkki@gmail.com> Gerrit-CC: Mike Banon <mikebdp2@gmail.com> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Thu, 28 May 2020 21:06:35 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37198 ) Change subject: nb/amd/agesa: select ROMSTAGE_CACHED_CBMEM ...................................................................... Patch Set 7:
Patch Set 7:
On the ASRock E350M1:
4.12-374-g0013623b7c:
``` 27 entries total:
0:1st timestamp 7,931 11:start of bootblock 14,414 (6,482) 12:end of bootblock 15,624 (1,210) 13:starting to load romstage 16,214 (589) 14:finished loading romstage 16,301 (87) 1:start of romstage 16,605 (303) 2:before ram initialization 129,739 (113,134) 3:after ram initialization 411,357 (281,617) 4:end of romstage 426,279 (14,921) 100:start of postcar 426,418 (138) 101:end of postcar 426,418 (0) 8:starting to load ramstage 426,528 (109) 15:starting LZMA decompress (ignore for x86) 426,564 (36) 16:finished LZMA decompress (ignore for x86) 518,200 (91,636) 9:finished loading ramstage 518,247 (46) 10:start of ramstage 518,269 (21) 30:device enumeration 522,196 (3,926) 40:device configuration 659,462 (137,266) 50:device enable 672,386 (12,924) 60:device initialization 672,859 (473) 70:device setup done 687,333 (14,474) 75:cbmem post 932,112 (244,778) 80:write tables 932,113 (1) 85:finalize chips 973,568 (41,454) 90:load payload 973,572 (4) 15:starting LZMA decompress (ignore for x86) 974,055 (483) 16:finished LZMA decompress (ignore for x86) 1,027,445 (53,389) 99:selfboot jump 1,027,502 (57)
Total Time: 1,019,558 ````
Your change-set rebased on origin/master (`cpu/x86/cache.h` added for `clflush_supported()`):
``` 28 entries total:
0:1st timestamp 7,886 11:start of bootblock 13,556 (5,670) 12:end of bootblock 14,705 (1,148) 13:starting to load romstage 15,285 (580) 14:finished loading romstage 15,371 (86) 1:start of romstage 15,652 (280) 2:before ram initialization 129,582 (113,930) 3:after ram initialization 414,538 (284,955) 17:starting LZ4 decompress (ignore for x86) 421,713 (7,175) 18:finished LZ4 decompress (ignore for x86) 427,201 (5,487) 100:start of postcar 428,672 (1,471) 101:end of postcar 428,672 (0) 8:starting to load ramstage 428,783 (110) 15:starting LZMA decompress (ignore for x86) 428,819 (36) 16:finished LZMA decompress (ignore for x86) 520,412 (91,593) 9:finished loading ramstage 520,459 (46) 10:start of ramstage 520,480 (21) 30:device enumeration 524,407 (3,927) 40:device configuration 661,704 (137,297) 50:device enable 674,636 (12,931) 60:device initialization 675,105 (468) 70:device setup done 689,625 (14,520) 75:cbmem post 953,917 (264,291) 80:write tables 953,918 (1) 85:finalize chips 995,266 (41,348) 90:load payload 995,271 (4) 15:starting LZMA decompress (ignore for x86) 995,755 (483) 16:finished LZMA decompress (ignore for x86) 1,049,208 (53,453) 99:selfboot jump 1,049,265 (57)
Total Time: 1,041,368 ```
Looks like this improves loading postcar stage with ~1ms. Thanks for testing. -- To view, visit https://review.coreboot.org/c/coreboot/+/37198 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I62ffe1bd646e9ddad77be240f030601790f4da4b Gerrit-Change-Number: 37198 Gerrit-PatchSet: 7 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Marshall Dawson <marshall.dawson@amd.corp-partner.google.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Krystian Hebel <krystian.hebel@3mdeb.com> Gerrit-CC: Kyösti Mälkki <kyosti.malkki@gmail.com> Gerrit-CC: Mike Banon <mikebdp2@gmail.com> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Thu, 28 May 2020 21:27:04 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37198 ) Change subject: nb/amd/agesa: select ROMSTAGE_CACHED_CBMEM ...................................................................... Patch Set 7:
Looks like this improves loading postcar stage with ~1ms. Thanks for testing.
Which timestamps exactly did you compare? It looked like 22ms overall increment to me. -- To view, visit https://review.coreboot.org/c/coreboot/+/37198 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I62ffe1bd646e9ddad77be240f030601790f4da4b Gerrit-Change-Number: 37198 Gerrit-PatchSet: 7 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Marshall Dawson <marshall.dawson@amd.corp-partner.google.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Krystian Hebel <krystian.hebel@3mdeb.com> Gerrit-CC: Kyösti Mälkki <kyosti.malkki@gmail.com> Gerrit-CC: Mike Banon <mikebdp2@gmail.com> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Thu, 28 May 2020 21:31:50 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37198 ) Change subject: nb/amd/agesa: select ROMSTAGE_CACHED_CBMEM ...................................................................... Patch Set 7:
Patch Set 7:
Looks like this improves loading postcar stage with ~1ms. Thanks for testing.
Which timestamps exactly did you compare? It looked like 22ms overall increment to me.
The difference between '3:after ram initialization' and '100:start of postcar' which this change affects. -- To view, visit https://review.coreboot.org/c/coreboot/+/37198 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I62ffe1bd646e9ddad77be240f030601790f4da4b Gerrit-Change-Number: 37198 Gerrit-PatchSet: 7 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Marshall Dawson <marshall.dawson@amd.corp-partner.google.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Krystian Hebel <krystian.hebel@3mdeb.com> Gerrit-CC: Kyösti Mälkki <kyosti.malkki@gmail.com> Gerrit-CC: Mike Banon <mikebdp2@gmail.com> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Thu, 28 May 2020 21:45:16 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37198 ) Change subject: nb/amd/agesa: select ROMSTAGE_CACHED_CBMEM ...................................................................... Patch Set 7:
Patch Set 7:
Patch Set 7:
Looks like this improves loading postcar stage with ~1ms. Thanks for testing.
Which timestamps exactly did you compare? It looked like 22ms overall increment to me.
The difference between '3:after ram initialization' and '100:start of postcar' which this change affects.
1ms improvement is just noise here in my opinion, specially when adding new dependency on having AGESA do MTRR setups. -- To view, visit https://review.coreboot.org/c/coreboot/+/37198 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I62ffe1bd646e9ddad77be240f030601790f4da4b Gerrit-Change-Number: 37198 Gerrit-PatchSet: 7 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Marshall Dawson <marshall.dawson@amd.corp-partner.google.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Krystian Hebel <krystian.hebel@3mdeb.com> Gerrit-CC: Kyösti Mälkki <kyosti.malkki@gmail.com> Gerrit-CC: Mike Banon <mikebdp2@gmail.com> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Thu, 28 May 2020 21:57:27 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37198 ) Change subject: nb/amd/agesa: select ROMSTAGE_CACHED_CBMEM ...................................................................... Patch Set 7:
Patch Set 7:
Patch Set 7:
Patch Set 7:
Looks like this improves loading postcar stage with ~1ms. Thanks for testing.
Which timestamps exactly did you compare? It looked like 22ms overall increment to me.
The difference between '3:after ram initialization' and '100:start of postcar' which this change affects.
1ms improvement is just noise here in my opinion, specially when adding new dependency on having AGESA do MTRR setups.
I think we already mostly keep AGESA's MTRR setup be we work around it by using more MTRR's: UC on regular boot to make sure cbmem hits memory before invd in postcar and WB on S3 resume to speed up things. 1ms on ~14ms is indeed just noise here. The advantages are: - No need to punch an UC hole with MTRR's for cbmem to make sure postcar hits memory before invd - postcar is compressed: 16380 vs 12461 (lz4 compressed) - (TODO)Some of the code can be simplified/removed if one can safely assume the presence of clflush. -- To view, visit https://review.coreboot.org/c/coreboot/+/37198 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I62ffe1bd646e9ddad77be240f030601790f4da4b Gerrit-Change-Number: 37198 Gerrit-PatchSet: 7 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Marshall Dawson <marshall.dawson@amd.corp-partner.google.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Krystian Hebel <krystian.hebel@3mdeb.com> Gerrit-CC: Kyösti Mälkki <kyosti.malkki@gmail.com> Gerrit-CC: Mike Banon <mikebdp2@gmail.com> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Thu, 28 May 2020 22:17:13 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37198 ) Change subject: nb/amd/agesa: select ROMSTAGE_CACHED_CBMEM ...................................................................... Patch Set 7: For a better overview: ``` 3:after ram initialization 411,357 (281,617) 4:end of romstage 426,279 (14,921) 100:start of postcar 426,418 (138) ``` ``` 3:after ram initialization 414,538 (284,955) 17:starting LZ4 decompress (ignore for x86) 421,713 (7,175) 18:finished LZ4 decompress (ignore for x86) 427,201 (5,487) 100:start of postcar 428,672 (1,471) ``` (By the way, *end of romstage* is missing.) If I remember correctly, the 22 ms overall decrease is due to the difference in *cbmem post*, which accesses the flash ROM chip, and reads with the used chip has a lot of timing variations. (I think, Kyösti even verified it with the datasheet.) -- To view, visit https://review.coreboot.org/c/coreboot/+/37198 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I62ffe1bd646e9ddad77be240f030601790f4da4b Gerrit-Change-Number: 37198 Gerrit-PatchSet: 7 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Marshall Dawson <marshall.dawson@amd.corp-partner.google.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Krystian Hebel <krystian.hebel@3mdeb.com> Gerrit-CC: Kyösti Mälkki <kyosti.malkki@gmail.com> Gerrit-CC: Mike Banon <mikebdp2@gmail.com> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Fri, 29 May 2020 10:46:56 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Mike Banon has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37198 ) Change subject: nb/amd/agesa: select ROMSTAGE_CACHED_CBMEM ...................................................................... Patch Set 7: While building for G505S (15h) I got: CC romstage/southbridge/amd/agesa/hudson/smbus.o CC romstage/southbridge/amd/agesa/hudson/smbus_spd.o CC romstage/vendorcode/amd/agesa/common/agesa-entry.o LINK cbfs/fallback/romstage.debug OBJCOPY cbfs/fallback/romstage.elf make: *** No rule to make target '".car.data"', needed by 'build/coreboot.pre'. Stop. But that's maybe because I selected a "Compress postcar with LZ4 (COMPRESS_POSTCAR) [Y/n/?] (NEW) y". Now will try without it -- To view, visit https://review.coreboot.org/c/coreboot/+/37198 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I62ffe1bd646e9ddad77be240f030601790f4da4b Gerrit-Change-Number: 37198 Gerrit-PatchSet: 7 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Marshall Dawson <marshall.dawson@amd.corp-partner.google.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Krystian Hebel <krystian.hebel@3mdeb.com> Gerrit-CC: Kyösti Mälkki <kyosti.malkki@gmail.com> Gerrit-CC: Mike Banon <mikebdp2@gmail.com> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Sat, 30 May 2020 16:11:38 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Mike Banon has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37198 ) Change subject: nb/amd/agesa: select ROMSTAGE_CACHED_CBMEM ...................................................................... Patch Set 7:
Patch Set 7:
While building for G505S (15h) I got:
CC romstage/southbridge/amd/agesa/hudson/smbus.o CC romstage/southbridge/amd/agesa/hudson/smbus_spd.o CC romstage/vendorcode/amd/agesa/common/agesa-entry.o LINK cbfs/fallback/romstage.debug OBJCOPY cbfs/fallback/romstage.elf make: *** No rule to make target '".car.data"', needed by 'build/coreboot.pre'. Stop.
But that's maybe because I selected a "Compress postcar with LZ4 (COMPRESS_POSTCAR) [Y/n/?] (NEW) y". Now will try without it
Hmmm, even if I answer N to this question, - still getting this error. -- To view, visit https://review.coreboot.org/c/coreboot/+/37198 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I62ffe1bd646e9ddad77be240f030601790f4da4b Gerrit-Change-Number: 37198 Gerrit-PatchSet: 7 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Marshall Dawson <marshall.dawson@amd.corp-partner.google.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Krystian Hebel <krystian.hebel@3mdeb.com> Gerrit-CC: Kyösti Mälkki <kyosti.malkki@gmail.com> Gerrit-CC: Mike Banon <mikebdp2@gmail.com> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Sat, 30 May 2020 16:15:48 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Mike Banon has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37198 ) Change subject: nb/amd/agesa: select ROMSTAGE_CACHED_CBMEM ...................................................................... Patch Set 7: Code-Review-1
Patch Set 7:
Patch Set 7:
While building for G505S (15h) I got:
CC romstage/southbridge/amd/agesa/hudson/smbus.o CC romstage/southbridge/amd/agesa/hudson/smbus_spd.o CC romstage/vendorcode/amd/agesa/common/agesa-entry.o LINK cbfs/fallback/romstage.debug OBJCOPY cbfs/fallback/romstage.elf make: *** No rule to make target '".car.data"', needed by 'build/coreboot.pre'. Stop.
But that's maybe because I selected a "Compress postcar with LZ4 (COMPRESS_POSTCAR) [Y/n/?] (NEW) y". Now will try without it
Hmmm, even if I answer N to this question, - still getting this error.
1. GCC (COMPILER_GCC)
1. Disabled (NO_STAGE_CACHE)
My full config is the latest revision of CB:32352 . After copying it to ./.config , I get these questions, most of which are auto-answered. Hope this is enough info for you to fix this change. I put -1 for now, but will be happy to upgrade it later Local version string (LOCALVERSION) [] Compiler to use 2. LLVM/clang (TESTING ONLY - Not currently working) (COMPILER_LLVM_CLANG) choice[1-2?]: 1 Allow building with any toolchain (ANY_TOOLCHAIN) [N/y/?] n Use ccache to speed up (re)compilation (CCACHE) [N/y/?] n Generate flashmap descriptor parser using flex and bison (FMD_GENPARSER) [N/y/?] n Generate SCONFIG & BINCFG parser using flex and bison (UTIL_GENPARSER) [N/y/?] n Use CMOS for configuration values (USE_OPTION_TABLE) [N/y/?] n Compress ramstage with LZMA (COMPRESS_RAMSTAGE) [Y/n/?] y Compress postcar with LZ4 (COMPRESS_POSTCAR) [Y/n/?] (NEW) n Include the coreboot .config file into the ROM image (INCLUDE_CONFIG_FILE) [Y/n/?] y Create a table of timestamps collected during boot (COLLECT_TIMESTAMPS) [Y/n/?] y Print the timestamp values on the console (TIMESTAMPS_ON_CONSOLE) [Y/n/?] y Allow use of binary-only repository (USE_BLOBS) [N/y/?] n Code coverage support (COVERAGE) [N/y/?] n Undefined behavior sanitizer support (UBSAN) [N/y/?] n Stage Cache for ACPI S3 resume 2. CBMEM (CBMEM_STAGE_CACHE) choice[1-2]: 1 Update existing coreboot.rom image (UPDATE_IMAGE) [N/y/?] n Add a bootsplash image (BOOTSPLASH_IMAGE) [N/y/?] n -- To view, visit https://review.coreboot.org/c/coreboot/+/37198 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I62ffe1bd646e9ddad77be240f030601790f4da4b Gerrit-Change-Number: 37198 Gerrit-PatchSet: 7 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Marshall Dawson <marshall.dawson@amd.corp-partner.google.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: Mike Banon <mikebdp2@gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Krystian Hebel <krystian.hebel@3mdeb.com> Gerrit-CC: Kyösti Mälkki <kyosti.malkki@gmail.com> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Sat, 30 May 2020 16:51:28 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37198 ) Change subject: nb/amd/agesa: select ROMSTAGE_CACHED_CBMEM ...................................................................... Patch Set 7:
Patch Set 7:
While building for G505S (15h) I got:
CC romstage/southbridge/amd/agesa/hudson/smbus.o CC romstage/southbridge/amd/agesa/hudson/smbus_spd.o CC romstage/vendorcode/amd/agesa/common/agesa-entry.o LINK cbfs/fallback/romstage.debug OBJCOPY cbfs/fallback/romstage.elf make: *** No rule to make target '".car.data"', needed by 'build/coreboot.pre'. Stop.
But that's maybe because I selected a "Compress postcar with LZ4 (COMPRESS_POSTCAR) [Y/n/?] (NEW) y". Now will try without it
Either downgrade to make or apply https://review.coreboot.org/c/coreboot/+/38790 -- To view, visit https://review.coreboot.org/c/coreboot/+/37198 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I62ffe1bd646e9ddad77be240f030601790f4da4b Gerrit-Change-Number: 37198 Gerrit-PatchSet: 7 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Marshall Dawson <marshall.dawson@amd.corp-partner.google.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: Mike Banon <mikebdp2@gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Krystian Hebel <krystian.hebel@3mdeb.com> Gerrit-CC: Kyösti Mälkki <kyosti.malkki@gmail.com> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Sat, 30 May 2020 17:10:37 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Mike Banon has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37198 ) Change subject: nb/amd/agesa: select ROMSTAGE_CACHED_CBMEM ...................................................................... Patch Set 7:
Patch Set 7:
Patch Set 7:
While building for G505S (15h) I got:
CC romstage/southbridge/amd/agesa/hudson/smbus.o CC romstage/southbridge/amd/agesa/hudson/smbus_spd.o CC romstage/vendorcode/amd/agesa/common/agesa-entry.o LINK cbfs/fallback/romstage.debug OBJCOPY cbfs/fallback/romstage.elf make: *** No rule to make target '".car.data"', needed by 'build/coreboot.pre'. Stop.
But that's maybe because I selected a "Compress postcar with LZ4 (COMPRESS_POSTCAR) [Y/n/?] (NEW) y". Now will try without it
Either downgrade to make or apply https://review.coreboot.org/c/coreboot/+/38790
Board boots fine until SeaBIOS, then I don't get any graphics in Linux. Here's my coreboot boot log captured with FT232H adapter - https://pastebin.com/g3LyVBZx . Maybe it's just an unfortunate parent of this change and the change itself is fine. Please rebase this change series on top of a stable commit like e6fb1344ed9188e19be4b54bdf1a76680b8c4523 or master (if it's okay currently), then I could retest this. Meanwhile, need to unbrick a laptop... -- To view, visit https://review.coreboot.org/c/coreboot/+/37198 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I62ffe1bd646e9ddad77be240f030601790f4da4b Gerrit-Change-Number: 37198 Gerrit-PatchSet: 7 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Marshall Dawson <marshall.dawson@amd.corp-partner.google.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: Mike Banon <mikebdp2@gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Krystian Hebel <krystian.hebel@3mdeb.com> Gerrit-CC: Kyösti Mälkki <kyosti.malkki@gmail.com> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Sat, 30 May 2020 18:37:42 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Mike Banon has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37198 ) Change subject: nb/amd/agesa: select ROMSTAGE_CACHED_CBMEM ...................................................................... Patch Set 7:
Patch Set 7:
Patch Set 7:
Patch Set 7:
While building for G505S (15h) I got:
CC romstage/southbridge/amd/agesa/hudson/smbus.o CC romstage/southbridge/amd/agesa/hudson/smbus_spd.o CC romstage/vendorcode/amd/agesa/common/agesa-entry.o LINK cbfs/fallback/romstage.debug OBJCOPY cbfs/fallback/romstage.elf make: *** No rule to make target '".car.data"', needed by 'build/coreboot.pre'. Stop.
But that's maybe because I selected a "Compress postcar with LZ4 (COMPRESS_POSTCAR) [Y/n/?] (NEW) y". Now will try without it
Either downgrade to make or apply https://review.coreboot.org/c/coreboot/+/38790
Board boots fine until SeaBIOS, then I don't get any graphics in Linux. Here's my coreboot boot log captured with FT232H adapter - https://pastebin.com/g3LyVBZx . Maybe it's just an unfortunate parent of this change and the change itself is fine. Please rebase this change series on top of a stable commit like e6fb1344ed9188e19be4b54bdf1a76680b8c4523 or master (if it's okay currently), then I could retest this. Meanwhile, need to unbrick a laptop...
On weekends I have some time for unbricking, so if you have new ideas about how to make this change bootable - please go ahead, and I'll test again happily. -- To view, visit https://review.coreboot.org/c/coreboot/+/37198 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I62ffe1bd646e9ddad77be240f030601790f4da4b Gerrit-Change-Number: 37198 Gerrit-PatchSet: 7 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: Mike Banon <mikebdp2@gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Reviewer: wrong ID - Marshall Dawson <marshall.dawson@amd.corp-partner.google.com> Gerrit-CC: Krystian Hebel <krystian.hebel@3mdeb.com> Gerrit-CC: Kyösti Mälkki <kyosti.malkki@gmail.com> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Sun, 07 Jun 2020 12:59:50 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37198 ) Change subject: nb/amd/agesa: select ROMSTAGE_CACHED_CBMEM ...................................................................... Patch Set 7: So, I89fdc6066027447bf72968c66e6f5eb5fbb630c7 [1] works for you? I’d say payload problems are unrelated to postcar changes. [1]: https://review.coreboot.org/c/coreboot/+/36674/8 -- To view, visit https://review.coreboot.org/c/coreboot/+/37198 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I62ffe1bd646e9ddad77be240f030601790f4da4b Gerrit-Change-Number: 37198 Gerrit-PatchSet: 7 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: Mike Banon <mikebdp2@gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Reviewer: wrong ID - Marshall Dawson <marshall.dawson@amd.corp-partner.google.com> Gerrit-CC: Krystian Hebel <krystian.hebel@3mdeb.com> Gerrit-CC: Kyösti Mälkki <kyosti.malkki@gmail.com> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Sun, 07 Jun 2020 14:12:32 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Mike Banon has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37198 ) Change subject: nb/amd/agesa: select ROMSTAGE_CACHED_CBMEM ...................................................................... Patch Set 7:
Patch Set 7:
So, I89fdc6066027447bf72968c66e6f5eb5fbb630c7 [1] works for you?
I’d say payload problems are unrelated to postcar changes.
This [1] doesn't work as well, I'll put -1 and share a boot log there. I don't think these are the payload problems: 1) The problems happen even if I choose the same SeaBIOS build 2) It could be that something related to graphic init (resources allocation, AtomBIOS OpROM loading, etc.) got corrupted - i.e. AtomBIOS got overwritten by something else, so the graphics in Linux got broken, or something else like a resource conflict. These problems aren't new - happened a couple times in a coreboot master and have been quickly resolved. It's unlikely these two changes got a problematic parent commit - but if Arthur thinks it could be really a case, hope he rebases these changes on top of a good parent - or fixes the code body of changes to make them work -- To view, visit https://review.coreboot.org/c/coreboot/+/37198 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I62ffe1bd646e9ddad77be240f030601790f4da4b Gerrit-Change-Number: 37198 Gerrit-PatchSet: 7 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: Mike Banon <mikebdp2@gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Reviewer: wrong ID - Marshall Dawson <marshall.dawson@amd.corp-partner.google.com> Gerrit-CC: Krystian Hebel <krystian.hebel@3mdeb.com> Gerrit-CC: Kyösti Mälkki <kyosti.malkki@gmail.com> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Sun, 07 Jun 2020 16:40:53 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Mike Banon has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37198 ) Change subject: nb/amd/agesa: select ROMSTAGE_CACHED_CBMEM ...................................................................... Patch Set 7: To investigate a problem of this change (almost no graphics (super distorted) on GRUB screen and no graphics at all in Linux), you could compare a "fail" bootlog here https://pastebin.com/g3LyVBZx with a good log at CB:41924 https://pastebin.com/u0RXtQvQ . This problem may be just a side effect exposing the issue with this code or a parent's change code (not clear yet) -- To view, visit https://review.coreboot.org/c/coreboot/+/37198 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I62ffe1bd646e9ddad77be240f030601790f4da4b Gerrit-Change-Number: 37198 Gerrit-PatchSet: 7 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: Mike Banon <mikebdp2@gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Reviewer: wrong ID - Marshall Dawson <marshall.dawson@amd.corp-partner.google.com> Gerrit-CC: Krystian Hebel <krystian.hebel@3mdeb.com> Gerrit-CC: Kyösti Mälkki <kyosti.malkki@gmail.com> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Sun, 07 Jun 2020 16:55:59 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Hello build bot (Jenkins), Michał Żygowski, Marshall Dawson, Mike Banon, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/37198 to look at the new patch set (#8). Change subject: nb/amd/agesa: select ROMSTAGE_CACHED_CBMEM ...................................................................... nb/amd/agesa: select ROMSTAGE_CACHED_CBMEM AGESA sets up MTRR's to cache the whole dram, so decompressing postcar stage to cbmem should be fast and is now selected by default. Postcar stage will now be compressed by default. Change-Id: I62ffe1bd646e9ddad77be240f030601790f4da4b Signed-off-by: Arthur Heymans <arthur@aheymans.xyz> --- M src/northbridge/amd/agesa/family14/Kconfig M src/northbridge/amd/agesa/family15tn/Kconfig M src/northbridge/amd/agesa/family16kb/Kconfig 3 files changed, 3 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/98/37198/8 -- To view, visit https://review.coreboot.org/c/coreboot/+/37198 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I62ffe1bd646e9ddad77be240f030601790f4da4b Gerrit-Change-Number: 37198 Gerrit-PatchSet: 8 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Marshall Dawson <marshall.dawson@amd.corp-partner.google.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: Mike Banon <mikebdp2@gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Krystian Hebel <krystian.hebel@3mdeb.com> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-MessageType: newpatchset
Attention is currently required from: Arthur Heymans. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37198 ) Change subject: nb/amd/agesa: select ROMSTAGE_CACHED_CBMEM ...................................................................... Patch Set 14: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/37198 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I62ffe1bd646e9ddad77be240f030601790f4da4b Gerrit-Change-Number: 37198 Gerrit-PatchSet: 14 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Marshall Dawson <marshall.dawson@amd.corp-partner.google.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: Mike Banon <mikebdp2@gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Krystian Hebel <krystian.hebel@3mdeb.com> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Attention: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Comment-Date: Tue, 23 Mar 2021 00:08:23 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans. Hello build bot (Jenkins), Michał Żygowski, Marshall Dawson, Angel Pons, Mike Banon, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/37198 to look at the new patch set (#15). Change subject: nb/amd/agesa: default to COMPRESS_POSTCAR ...................................................................... nb/amd/agesa: default to COMPRESS_POSTCAR AGESA sets up MTRR's to cache the whole dram, so decompressing postcar stage to cbmem should be fast and is now selected by default. Postcar stage will now be compressed by default. Change-Id: I62ffe1bd646e9ddad77be240f030601790f4da4b Signed-off-by: Arthur Heymans <arthur@aheymans.xyz> --- M src/northbridge/amd/agesa/family14/Kconfig M src/northbridge/amd/agesa/family15tn/Kconfig M src/northbridge/amd/agesa/family16kb/Kconfig 3 files changed, 15 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/98/37198/15 -- To view, visit https://review.coreboot.org/c/coreboot/+/37198 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I62ffe1bd646e9ddad77be240f030601790f4da4b Gerrit-Change-Number: 37198 Gerrit-PatchSet: 15 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Marshall Dawson <marshall.dawson@amd.corp-partner.google.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: Mike Banon <mikebdp2@gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Krystian Hebel <krystian.hebel@3mdeb.com> Gerrit-CC: Paul Menzel <paulepanter@mailbox.org> Gerrit-Attention: Arthur Heymans <arthur@aheymans.xyz> Gerrit-MessageType: newpatchset
Attention is currently required from: Arthur Heymans, Paul Menzel. Mike Banon has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37198?usp=email ) Change subject: nb/amd/agesa: default to COMPRESS_POSTCAR ...................................................................... Patch Set 21: (1 comment) Patchset: PS21: Arthur, please note that this patch is going to be "auto-abandoned" unless you'll rebase it on a similar parent or something... -- To view, visit https://review.coreboot.org/c/coreboot/+/37198?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I62ffe1bd646e9ddad77be240f030601790f4da4b Gerrit-Change-Number: 37198 Gerrit-PatchSet: 21 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Marshall Dawson <marshall.dawson@amd.corp-partner.google.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: Mike Banon <mikebdp2@gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter@mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Krystian Hebel <krystian.hebel@3mdeb.com> Gerrit-Attention: Paul Menzel <paulepanter@mailbox.org> Gerrit-Attention: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Comment-Date: Thu, 03 Aug 2023 22:13:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
Arthur Heymans has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/37198?usp=email ) Change subject: nb/amd/agesa: default to COMPRESS_POSTCAR ...................................................................... Abandoned -- To view, visit https://review.coreboot.org/c/coreboot/+/37198?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I62ffe1bd646e9ddad77be240f030601790f4da4b Gerrit-Change-Number: 37198 Gerrit-PatchSet: 21 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Marshall Dawson <marshall.dawson@amd.corp-partner.google.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: Mike Banon <mikebdp2@gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter@mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Krystian Hebel <krystian.hebel@3mdeb.com> Gerrit-CC: Stefan Reinauer <stefan.reinauer@coreboot.org> Gerrit-MessageType: abandon
participants (6)
-
Angel Pons (Code Review) -
Arthur Heymans (Code Review)
-
Kyösti Mälkki (Code Review)
-
Michał Żygowski (Code Review)
-
Mike Banon (Code Review)
-
Paul Menzel (Code Review)