Attention is currently required from: Andrey Petrov, Felix Held, Fred Reitberger, Intel coreboot Reviewers, Jason Glenesk, Jérémy Compostella, Matt DeVillier, Patrick Rudolph, Ronak Kanabar.
Hello Andrey Petrov, Felix Held, Fred Reitberger, Intel coreboot Reviewers, Jason Glenesk, Jérémy Compostella, Matt DeVillier, Ronak Kanabar, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/86299?usp=email
to look at the new patch set (#4).
The following approvals got outdated and were removed:
Code-Review+1 by Felix Held, Code-Review+1 by Jérémy Compostella, Verified+1 by build bot (Jenkins)
Change subject: soc/amd/common/block/graphics: Use vbt_get()
......................................................................
soc/amd/common/block/graphics: Use vbt_get()
Implement vbt_get() on AMD and return the VBIOS location. This allows
to drop the hardcoded addresses used in various places and return an
address in DRAM that is reserved for FSP use.
TEST: amd/birman+ still gets passed the correct VBIOS address.
Change-Id: I92d76fc4df88fbce792b9d7c912c6799617704a0
Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com>
---
M src/drivers/intel/fsp2_0/silicon_init.c
M src/soc/amd/cezanne/fsp_s_params.c
M src/soc/amd/common/block/graphics/graphics.c
A src/soc/amd/common/block/include/amdblocks/vbt.h
M src/soc/amd/glinda/fsp_s_params.c
M src/soc/amd/mendocino/fsp_s_params.c
M src/soc/amd/phoenix/fsp_s_params.c
M src/soc/amd/picasso/fsp_s_params.c
8 files changed, 46 insertions(+), 20 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/99/86299/4
--
To view, visit https://review.coreboot.org/c/coreboot/+/86299?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I92d76fc4df88fbce792b9d7c912c6799617704a0
Gerrit-Change-Number: 86299
Gerrit-PatchSet: 4
Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Attention is currently required from: Patrick Rudolph.
Andy Ebrahiem has posted comments on this change by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/86383?usp=email )
Change subject: device/pci_rom: Use correct endian conversion
......................................................................
Patch Set 2: Code-Review+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/86383?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I571be97a930ad018e1d1316117cefe5bd1c68f9b
Gerrit-Change-Number: 86383
Gerrit-PatchSet: 2
Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Andy Ebrahiem <ahmet.ebrahiem(a)9elements.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Comment-Date: Thu, 13 Feb 2025 12:08:18 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: Patrick Rudolph.
Felix Held has posted comments on this change by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/86383?usp=email )
Change subject: device/pci_rom: Use correct endian conversion
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/86383?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I571be97a930ad018e1d1316117cefe5bd1c68f9b
Gerrit-Change-Number: 86383
Gerrit-PatchSet: 2
Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Comment-Date: Thu, 13 Feb 2025 11:48:53 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: Andrey Petrov, Fred Reitberger, Intel coreboot Reviewers, Jason Glenesk, Jérémy Compostella, Matt DeVillier, Patrick Rudolph, Ronak Kanabar.
Felix Held has posted comments on this change by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/86299?usp=email )
Change subject: soc/amd/common/block/graphics: Use vbt_get()
......................................................................
Patch Set 3: Code-Review+1
(2 comments)
File src/soc/amd/common/block/graphics/graphics.c:
https://review.coreboot.org/c/coreboot/+/86299/comment/9ad37442_e032bd1b?us… :
PS3, Line 149: /*
: * On AMD platforms the VBT is called ATOMBIOS and is always part of the
: * VGA Option ROM. As part of the FSP GOP init the ATOMBIOS tables are
: * updated in place. Thus the VBIOS must be loaded into RAM before FSP GOP
: * runs. The address of the VBIOS must be passed to FSP-S using UPDs, but
: * loading of the VBIOS can be delayed until before FSP AFTER_PCI_ENUM
: * notify is called. FSP expects a pointer to the PCI option rom instead
: * a pointer to the ATOMBIOS table directly.
: */
ah, this comment gets moved to the header file, so it's not just deleted
https://review.coreboot.org/c/coreboot/+/86299/comment/931f09b1_a53ba55b?us… :
PS3, Line 151: if (!CONFIG(RUN_FSP_GOP))
> It's aligned to `src/soc/intel/common/vbt.c`.
hmm, consistency vs code being slightly easier to read. in this case, i'd probably rather go with the code being a bit easier to read, but i don't have a too strong opinion here
--
To view, visit https://review.coreboot.org/c/coreboot/+/86299?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I92d76fc4df88fbce792b9d7c912c6799617704a0
Gerrit-Change-Number: 86299
Gerrit-PatchSet: 3
Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Comment-Date: Thu, 13 Feb 2025 11:45:41 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Comment-In-Reply-To: Jérémy Compostella <jeremy.compostella(a)intel.com>
Attention is currently required from: Ana Carolina Cabral, Fred Reitberger, Jason Glenesk, Matt DeVillier, Patrick Rudolph.
Felix Held has posted comments on this change by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/86300?usp=email )
The change is no longer submittable: All-Comments-Resolved and Code-Review are unsatisfied now.
Change subject: soc/amd/common/block/graphics: Support non VGA IGDs
......................................................................
Patch Set 3: Code-Review+1
(1 comment)
File src/soc/amd/common/block/graphics/graphics.c:
https://review.coreboot.org/c/coreboot/+/86300/comment/7905277e_384851b3?us… :
PS3, Line 258: PCI_VGA_RAM_IMAGE_START
this comment probably needs to be updated; same in line 263 and 279
--
To view, visit https://review.coreboot.org/c/coreboot/+/86300?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I6ab28aab74f3169d45d7d852a37ddfcfc75b7c88
Gerrit-Change-Number: 86300
Gerrit-PatchSet: 3
Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Ana Carolina Cabral <ana.cpmelo95(a)gmail.com>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Attention: Ana Carolina Cabral <ana.cpmelo95(a)gmail.com>
Gerrit-Comment-Date: Thu, 13 Feb 2025 11:40:17 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Attention is currently required from: Ana Carolina Cabral, Fred Reitberger, Jason Glenesk, Matt DeVillier, Patrick Rudolph.
Felix Held has posted comments on this change by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/86300?usp=email )
Change subject: soc/amd/common/block/graphics: Support non VGA IGDs
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/86300?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I6ab28aab74f3169d45d7d852a37ddfcfc75b7c88
Gerrit-Change-Number: 86300
Gerrit-PatchSet: 3
Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Ana Carolina Cabral <ana.cpmelo95(a)gmail.com>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Attention: Ana Carolina Cabral <ana.cpmelo95(a)gmail.com>
Gerrit-Comment-Date: Thu, 13 Feb 2025 11:37:26 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: Vladimir Serbinenko.
Felix Held has posted comments on this change by Vladimir Serbinenko. ( https://review.coreboot.org/c/coreboot/+/81507?usp=email )
Change subject: Support smmstore in RW_LEGACY
......................................................................
Patch Set 3:
(1 comment)
File src/drivers/smmstore/store.c:
https://review.coreboot.org/c/coreboot/+/81507/comment/5e205d6b_3af519c7?us… :
PS3, Line 60: && cbfs_lookup(&rw_legacy_rdev, "smmstore.bin", &mdata, &data_offset, NULL) == 0) {
it seems that this new code isn't checking the alignment and size requirements. the smmstore in flash needs to start right on the beginning of an erase block and needs to end right at the end of an erase block so that we can be sure that doing an erase to the smmstore doesn't clobber anything outside of it
--
To view, visit https://review.coreboot.org/c/coreboot/+/81507?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I86a617782f187adcce4429ea41ac40a9df58d6d3
Gerrit-Change-Number: 81507
Gerrit-PatchSet: 3
Gerrit-Owner: Vladimir Serbinenko <phcoder(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-CC: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Vladimir Serbinenko <phcoder(a)gmail.com>
Gerrit-Comment-Date: Thu, 13 Feb 2025 11:29:25 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Yidi Lin, Yu-Ping Wu.
Jarried Lin has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/blobs/+/86380?usp=email )
Change subject: soc/mediatek/mt8196: update mtk_fsp_ramstage to 16174.22.0
......................................................................
Patch Set 3:
(5 comments)
Commit Message:
https://review.coreboot.org/c/blobs/+/86380/comment/b522b4b7_f9587e3e?usp=e… :
PS2, Line 7: update mtk_fsp_ramstage to version: 16189.0.0
> `Update mtk_fsp_ramstage to 16189.0. […]
Done
https://review.coreboot.org/c/blobs/+/86380/comment/175224f3_b8edec95?usp=e… :
PS2, Line 7: 16189.0.0
> Why not use the binary built from the ChromeOS FW branch, which should be more stable?
Done
https://review.coreboot.org/c/blobs/+/86380/comment/f3272ae2_d5b161b6?usp=e… :
PS2, Line 13: rauru
> We don't need this in the rauru FW branch, because ChromeOS builds always build mtk-fsp from source.
Done
File soc/mediatek/mt8196/mtk_fsp_ramstage_release_notes.txt:
https://review.coreboot.org/c/blobs/+/86380/comment/b45b886c_d3067b82?usp=e… :
PS2, Line 5: Include
> Included
Done
https://review.coreboot.org/c/blobs/+/86380/comment/3eaad0a8_8da69539?usp=e… :
PS2, Line 7: - CL:*7980492 mt8196/kraken_v3: Call dsb() after writing to CSRAM
> The CL order in the v1.0 release notes is the opposite: from latest to oldest. Please be consistent.
Done
--
To view, visit https://review.coreboot.org/c/blobs/+/86380?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: blobs
Gerrit-Branch: main
Gerrit-Change-Id: Ic785a04e576065a250f55a78105abf42b61a15a7
Gerrit-Change-Number: 86380
Gerrit-PatchSet: 3
Gerrit-Owner: Jarried Lin <jarried.lin(a)mediatek.com>
Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Attention: Yidi Lin <yidilin(a)google.com>
Gerrit-Comment-Date: Thu, 13 Feb 2025 11:27:15 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Yu-Ping Wu <yupingso(a)google.com>
Attention is currently required from: Jarried Lin, Yidi Lin.
Hello Yidi Lin, Yu-Ping Wu,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/blobs/+/86380?usp=email
to look at the new patch set (#3).
The following approvals got outdated and were removed:
Code-Review+2 by Yidi Lin
Change subject: soc/mediatek/mt8196: update mtk_fsp_ramstage to 16174.22.0
......................................................................
soc/mediatek/mt8196: update mtk_fsp_ramstage to 16174.22.0
Update mtk_fsp_ramstage.elf includes:
- Disable clock for Crypto hardware.
- Add build version info.
TEST=Build pass
BUG=b:317009620
Change-Id: Ic785a04e576065a250f55a78105abf42b61a15a7
Signed-off-by: Jarried Lin <jarried.lin(a)mediatek.corp-partner.google.com>
---
M soc/mediatek/mt8196/README.md
M soc/mediatek/mt8196/mtk_fsp_ramstage.elf
M soc/mediatek/mt8196/mtk_fsp_ramstage.elf.md5
M soc/mediatek/mt8196/mtk_fsp_ramstage_release_notes.txt
4 files changed, 13 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/blobs refs/changes/80/86380/3
--
To view, visit https://review.coreboot.org/c/blobs/+/86380?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: blobs
Gerrit-Branch: main
Gerrit-Change-Id: Ic785a04e576065a250f55a78105abf42b61a15a7
Gerrit-Change-Number: 86380
Gerrit-PatchSet: 3
Gerrit-Owner: Jarried Lin <jarried.lin(a)mediatek.com>
Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Attention: Yidi Lin <yidilin(a)google.com>
Gerrit-Attention: Jarried Lin <jarried.lin(a)mediatek.com>
Attention is currently required from: Felix Held, Fred Reitberger, Intel coreboot Reviewers, Jason Glenesk, Matt DeVillier.
Sean Rhodes has posted comments on this change by Sean Rhodes. ( https://review.coreboot.org/c/coreboot/+/86399?usp=email )
Change subject: soc/{intel,amd}/gpio: Correct SOC GPIO OP
......................................................................
Set Ready For Review
--
To view, visit https://review.coreboot.org/c/coreboot/+/86399?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Id03d7ca0e328917fc8379adb967a2a122c7ff85f
Gerrit-Change-Number: 86399
Gerrit-PatchSet: 1
Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Thu, 13 Feb 2025 10:44:48 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No