coreboot-gerrit February 2025

coreboot-gerrit@coreboot.org

1 participants
2886 discussions

[XS] Change in coreboot[main]: drivers/intel/gma: Don't advertise support for opregion mailbox #2
by Matt DeVillier (Code Review) 04 Feb '25

04 Feb '25

Attention is currently required from: Intel coreboot Reviewers. Matt DeVillier has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/86276?usp=email ) Change subject: drivers/intel/gma: Don't advertise support for opregion mailbox #2 ...................................................................... drivers/intel/gma: Don't advertise support for opregion mailbox #2 IGD mailbox #2, Software SCI Interface, is not supported by coreboot currently, as it requires supporting the Get BIOS Data (GBDA) and System BIOS Callbacks (SBCB) interfaces. Since coreboot doesn't support these, don't advertise mailbox #2 support. This eliminates an error with the Linux display drivers: "SWSCI request timed out" TEST=build/boot Linux 6.9, Win11 on starlabs/starlite_adl Change-Id: I8efcf9c5d384b6e0ce159d65cb1497c2e2e47f42 Signed-off-by: Matt DeVillier <matt.devillier(a)gmail.com> --- M src/drivers/intel/gma/opregion.h 1 file changed, 1 insertion(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/76/86276/1 diff --git a/src/drivers/intel/gma/opregion.h b/src/drivers/intel/gma/opregion.h index 7b6a85d..2de9213 100644 --- a/src/drivers/intel/gma/opregion.h +++ b/src/drivers/intel/gma/opregion.h @@ -41,8 +41,7 @@ #define IGD_MBOX4 (1 << 3) #define IGD_MBOX5 (1 << 4) -#define MAILBOXES_MOBILE (IGD_MBOX1 | IGD_MBOX2 | IGD_MBOX3 | \ - IGD_MBOX4 | IGD_MBOX5) +#define MAILBOXES_MOBILE (IGD_MBOX1 | IGD_MBOX3 | IGD_MBOX4 | IGD_MBOX5) #define MAILBOXES_DESKTOP (IGD_MBOX2 | IGD_MBOX4) #define SBIOS_VERSION_SIZE 32 -- To view, visit https://review.coreboot.org/c/coreboot/+/86276?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I8efcf9c5d384b6e0ce159d65cb1497c2e2e47f42 Gerrit-Change-Number: 86276 Gerrit-PatchSet: 1 Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com>

1 0

[S] Change in coreboot[main]: drivers/intel/gma: Fix alignment of extended VBT in opregion
by Matt DeVillier (Code Review) 04 Feb '25

04 Feb '25

Matt DeVillier has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/86275?usp=email ) Change subject: drivers/intel/gma: Fix alignment of extended VBT in opregion ...................................................................... drivers/intel/gma: Fix alignment of extended VBT in opregion Intel's reference implementation in Slimbootloader pads the area allocated for the extended VBT to the nearest 512-byte boundary, which strongly suggests that the Windows driver expects the same. TEST=build/boot Linux 6.9, Win11 on starlabs/starlite_adl, verify VBT read properly by OS. Change-Id: Ib3784eea6eb929ffec9672fc123b833c11c057e8 Signed-off-by: Matt DeVillier <matt.devillier(a)gmail.com> --- M src/drivers/intel/gma/opregion.c 1 file changed, 7 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/86275/1 diff --git a/src/drivers/intel/gma/opregion.c b/src/drivers/intel/gma/opregion.c index f97d373..37a8413 100644 --- a/src/drivers/intel/gma/opregion.c +++ b/src/drivers/intel/gma/opregion.c @@ -287,7 +287,7 @@ * values correctly for the opregion. */ static void opregion_add_ext_vbt(igd_opregion_t *opregion, uint8_t *ext_vbt, - optionrom_vbt_t *vbt) + optionrom_vbt_t *vbt, size_t ext_vbt_size) { opregion_header_t *header = &opregion->header; /* Copy VBT into extended VBT region (at offset 8 KiB) */ @@ -301,7 +301,7 @@ else opregion->mailbox3.rvda = (uintptr_t)ext_vbt; - opregion->mailbox3.rvds = vbt->hdr_vbt_size; + opregion->mailbox3.rvds = ext_vbt_size; } /* Initialize IGD OpRegion, called from ACPI code and OS drivers */ @@ -311,6 +311,7 @@ struct region_device rdev; optionrom_vbt_t *vbt = NULL; size_t opregion_size = sizeof(igd_opregion_t); + size_t ext_vbt_size; if (acpi_is_wakeup_s3()) return intel_gma_restore_opregion(); @@ -331,7 +332,9 @@ } /* Add the space for the extended VBT header even if it's not used */ - opregion_size += vbt->hdr_vbt_size; + /* Align the VBT to nearest 512 byte boundary */ + ext_vbt_size = (vbt->hdr_vbt_size & (uint32_t)~(0x1ff)) + 0x200; + opregion_size += ext_vbt_size; opregion = cbmem_add(CBMEM_ID_IGD_OPREGION, opregion_size); if (!opregion) { @@ -353,7 +356,7 @@ if (is_ext_vbt_required(opregion, vbt)) { /* Place extended VBT just after opregion */ uint8_t *ext_vbt = (uint8_t *)opregion + sizeof(*opregion); - opregion_add_ext_vbt(opregion, ext_vbt, vbt); + opregion_add_ext_vbt(opregion, ext_vbt, vbt, ext_vbt_size); } else { /* Raw VBT size which can fit in gvd1 */ memcpy(opregion->vbt.gvd1, vbt, vbt->hdr_vbt_size); -- To view, visit https://review.coreboot.org/c/coreboot/+/86275?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ib3784eea6eb929ffec9672fc123b833c11c057e8 Gerrit-Change-Number: 86275 Gerrit-PatchSet: 1 Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com>

1 0

[XS] Change in coreboot[main]: soc/amd/cezanne/chipset.cb: Enable gpp_bridge_[a/b/c] by default
by Maximilian Brune (Code Review) 04 Feb '25

04 Feb '25

Attention is currently required from: Felix Held, Fred Reitberger, Jason Glenesk, Jason Nien, Martin Roth, Matt DeVillier. Hello Felix Held, Fred Reitberger, Jason Glenesk, Jason Nien, Martin Roth, Matt DeVillier, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/86272?usp=email to look at the new patch set (#2). Change subject: soc/amd/cezanne/chipset.cb: Enable gpp_bridge_[a/b/c] by default ...................................................................... soc/amd/cezanne/chipset.cb: Enable gpp_bridge_[a/b/c] by default TODO justification Change-Id: Ie34bb2abc0211963b2613d1b50b1767df31c1062 Signed-off-by: Maximilian Brune <maximilian.brune(a)9elements.com> --- M src/mainboard/google/guybrush/variants/baseboard/devicetree.cb M src/soc/amd/cezanne/chipset.cb 2 files changed, 5 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/86272/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/86272?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: Ie34bb2abc0211963b2613d1b50b1767df31c1062 Gerrit-Change-Number: 86272 Gerrit-PatchSet: 2 Gerrit-Owner: Maximilian Brune <maximilian.brune(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: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>

1 0

[S] Change in coreboot[main]: soc/amd/phoenix/chipset_*: Enable gpp_bridge_[a/b/c] by default
by Maximilian Brune (Code Review) 04 Feb '25

04 Feb '25

Attention is currently required from: Felix Held, Fred Reitberger, Jason Glenesk, Matt DeVillier. Hello Felix Held, Fred Reitberger, Jason Glenesk, Matt DeVillier, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/86271?usp=email to look at the new patch set (#2). Change subject: soc/amd/phoenix/chipset_*: Enable gpp_bridge_[a/b/c] by default ...................................................................... soc/amd/phoenix/chipset_*: Enable gpp_bridge_[a/b/c] by default TODO justification Change-Id: Ic226fd93b431467c7fa3a53140102ff4fd327f40 Signed-off-by: Maximilian Brune <maximilian.brune(a)9elements.com> --- M src/mainboard/google/myst/variants/baseboard/devicetree.cb M src/soc/amd/phoenix/chipset_fsp.cb M src/soc/amd/phoenix/chipset_opensil.cb 3 files changed, 7 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/71/86271/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/86271?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: Ic226fd93b431467c7fa3a53140102ff4fd327f40 Gerrit-Change-Number: 86271 Gerrit-PatchSet: 2 Gerrit-Owner: Maximilian Brune <maximilian.brune(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-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.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>

1 0

[M] Change in coreboot[main]: soc/amd/common: Support sbin ucode files
by Maximilian Brune (Code Review) 04 Feb '25

04 Feb '25

Attention is currently required from: Felix Held, Fred Reitberger, Jason Glenesk, Martin Roth, Matt DeVillier, Raul Rangel. Maximilian Brune has posted comments on this change by Fred Reitberger. ( https://review.coreboot.org/c/coreboot/+/68122?usp=email ) Change subject: soc/amd/common: Support sbin ucode files ...................................................................... Patch Set 9: (3 comments) File src/soc/amd/common/block/cpu/Makefile.inc: https://review.coreboot.org/c/coreboot/+/68122/comment/fea04cf9_59821a39?us… : PS3, Line 19: Function > I agree, but the problem is that in order to write proper makefile rules we would need to know the n […] Done File src/soc/amd/common/block/cpu/Makefile.mk: https://review.coreboot.org/c/coreboot/+/68122/comment/804eef3f_1e655de0?us… : PS9, Line 52: extract-bytes = $(shell echo $(shell od -A n -N $(2) -j $(3) -t $(4) $(1))) > one of the minor concerns I had with this patch and `od` directly is that we are assuming this alway […] Done https://review.coreboot.org/c/coreboot/+/68122/comment/d7c922a7_1ac0dcb1?us… : PS9, Line 52: extract-bytes = $(shell echo $(shell od -A n -N $(2) -j $(3) -t $(4) $(1))) > for consistency, the call to `od` here should probably use the long options format instead of the sh […] good catch -- To view, visit https://review.coreboot.org/c/coreboot/+/68122?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: I29768ea19543bdc76662e687f59bf31b76f555ae Gerrit-Change-Number: 68122 Gerrit-PatchSet: 9 Gerrit-Owner: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Martin Roth <martin.roth(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: Tue, 04 Feb 2025 16:24:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Raul Rangel <rrangel(a)chromium.org> Comment-In-Reply-To: Maximilian Brune <maximilian.brune(a)9elements.com> Comment-In-Reply-To: Fred Reitberger <reitbergerfred(a)gmail.com>

1 0

[M] Change in coreboot[main]: soc/amd/common: Support sbin ucode files
by Maximilian Brune (Code Review) 04 Feb '25

04 Feb '25

Attention is currently required from: Felix Held, Jason Glenesk, Martin Roth, Matt DeVillier, Maximilian Brune, Raul Rangel. Maximilian Brune has uploaded a new patch set (#10) to the change originally created by Fred Reitberger. ( https://review.coreboot.org/c/coreboot/+/68122?usp=email ) The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: soc/amd/common: Support sbin ucode files ...................................................................... soc/amd/common: Support sbin ucode files Recent PI releases have been distributing the ucode patch files as sbin files instead of bin files. The sbin uses a 256 byte amd_fw_header to wrap the bin file. Offset 0x14 of the header is the size field. The can be extracted with od to get the size of the ucode bin file. The bin file can then be extracted with dd and placed in the build directory for inclusion as a cbfs file. In the case where both an sbin and bin ucode file are present, the bin file will be added and a note will print at the start of the build about the sbin file being skipped. TEST=builds with only bin, only sbin, non-matching bin and sbin, matching bin and sbin files Signed-off-by: Fred Reitberger <reitbergerfred(a)gmail.com> Signed-off-by: Maximilian Brune <maximilian.brune(a)9elements.com> Change-Id: I29768ea19543bdc76662e687f59bf31b76f555ae --- M src/soc/amd/common/Makefile.mk M src/soc/amd/common/block/cpu/Makefile.mk 2 files changed, 45 insertions(+), 13 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/22/68122/10 -- To view, visit https://review.coreboot.org/c/coreboot/+/68122?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: I29768ea19543bdc76662e687f59bf31b76f555ae Gerrit-Change-Number: 68122 Gerrit-PatchSet: 10 Gerrit-Owner: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>

1 0

[M] Change in coreboot[main]: soc/amd/common: Support sbin ucode files
by Fred Reitberger (Code Review) 04 Feb '25

04 Feb '25

Attention is currently required from: Felix Held, Jason Glenesk, Martin Roth, Matt DeVillier, Maximilian Brune, Raul Rangel. Fred Reitberger has posted comments on this change by Fred Reitberger. ( https://review.coreboot.org/c/coreboot/+/68122?usp=email ) Change subject: soc/amd/common: Support sbin ucode files ...................................................................... Patch Set 9: (3 comments) File src/soc/amd/common/block/cpu/Makefile.mk: https://review.coreboot.org/c/coreboot/+/68122/comment/07ee330a_ac0fdcdf?us… : PS9, Line 32: # if there is both a sbin and bin microcode only include the bin one to keep the old behaviour > This only covers the case if there is both an sbin and bin (like currently in mendocino). […] Acknowledged https://review.coreboot.org/c/coreboot/+/68122/comment/f8c50815_fd6ca647?us… : PS9, Line 52: extract-bytes = $(shell echo $(shell od -A n -N $(2) -j $(3) -t $(4) $(1))) one of the minor concerns I had with this patch and `od` directly is that we are assuming this always will run on a little endian build machine. That's probably a safe assumption, but if we really need to be safe it looks like there is a `--endian=little` flag that might do the right thing on a big endian machine? https://review.coreboot.org/c/coreboot/+/68122/comment/5d989b3f_c27b9b03?us… : PS9, Line 52: extract-bytes = $(shell echo $(shell od -A n -N $(2) -j $(3) -t $(4) $(1))) for consistency, the call to `od` here should probably use the long options format instead of the short, to match the call above on line 28 ex: use `--address-radix` instead of `-A`, etc -- To view, visit https://review.coreboot.org/c/coreboot/+/68122?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: I29768ea19543bdc76662e687f59bf31b76f555ae Gerrit-Change-Number: 68122 Gerrit-PatchSet: 9 Gerrit-Owner: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Tue, 04 Feb 2025 16:15:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Maximilian Brune <maximilian.brune(a)9elements.com> Comment-In-Reply-To: Fred Reitberger <reitbergerfred(a)gmail.com>

1 0

[S] Change in coreboot[main]: soc/amd/mendocino/chipset_*: Enable gpp_bridge_[a/b/c] by default
by Maximilian Brune (Code Review) 04 Feb '25

04 Feb '25

Attention is currently required from: Felix Held, Fred Reitberger, Jason Glenesk, Jason Nien, Martin Roth, Matt DeVillier, Maximilian Brune. Hello Felix Held, Fred Reitberger, Jason Glenesk, Jason Nien, Martin Roth, Matt DeVillier, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/86273?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Verified-1 by build bot (Jenkins) Change subject: soc/amd/mendocino/chipset_*: Enable gpp_bridge_[a/b/c] by default ...................................................................... soc/amd/mendocino/chipset_*: Enable gpp_bridge_[a/b/c] by default TODO justification Change-Id: Ife30f73495d44c98717e147602de10f5a6a89358 Signed-off-by: Maximilian Brune <maximilian.brune(a)9elements.com> --- M src/soc/amd/mendocino/chipset_mendocino.cb M src/soc/amd/mendocino/chipset_rembrandt.cb 2 files changed, 5 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/86273/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/86273?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: Ife30f73495d44c98717e147602de10f5a6a89358 Gerrit-Change-Number: 86273 Gerrit-PatchSet: 2 Gerrit-Owner: Maximilian Brune <maximilian.brune(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: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.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: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>

1 0

[S] Change in coreboot[main]: soc/amd/common/block/pci: Ignore disabled bridges
by Maximilian Brune (Code Review) 04 Feb '25

04 Feb '25

Attention is currently required from: Felix Held, Fred Reitberger, Jason Glenesk, Matt DeVillier, Patrick Rudolph. Maximilian Brune has posted comments on this change by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/86216?usp=email ) Change subject: soc/amd/common/block/pci: Ignore disabled bridges ...................................................................... Patch Set 1: (1 comment) File src/soc/amd/common/block/pci/pcie_gpp.c: https://review.coreboot.org/c/coreboot/+/86216/comment/3c780fa1_90234348?us… : PS1, Line 58: static void pcie_internal_gpp_enable(struct device *dev) : { : assert(dev->enabled); : if (!dev->enabled) { : /* : * Cannot hide PPB devices! Must keep them enabled to : * make sure PCI enumeration runs and avoids resource conflicts. : */ : dev->enabled = true; : dev->hidden = true; : dev->ops = &amd_pcie_disabled_gpp_ops; : } : } > Maybe we must enable them in the chipset devicetree. That would be the easiest solution. […] iirc @felix-coreboot@felixheld.de said that they are mandatory. As a response I created a patch for the chipsets: https://review.coreboot.org/c/coreboot/+/86270/1 Felix could you confirm? -- To view, visit https://review.coreboot.org/c/coreboot/+/86216?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: Ia97098c860aa6cd6b0b59aa7b1c6f7c239e41983 Gerrit-Change-Number: 86216 Gerrit-PatchSet: 1 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: Maximilian Brune <maximilian.brune(a)9elements.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: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Tue, 04 Feb 2025 16:03:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Rudolph <patrick.rudolph(a)9elements.com> Comment-In-Reply-To: Maximilian Brune <maximilian.brune(a)9elements.com>

1 0

[S] Change in coreboot[main]: soc/amd/mendocino/chipset_*: Enable gpp_bridge_[a/b/c] by default
by Maximilian Brune (Code Review) 04 Feb '25

04 Feb '25

Attention is currently required from: Felix Held, Fred Reitberger, Jason Glenesk, Jason Nien, Martin Roth, Matt DeVillier. Maximilian Brune has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/86273?usp=email ) Change subject: soc/amd/mendocino/chipset_*: Enable gpp_bridge_[a/b/c] by default ...................................................................... soc/amd/mendocino/chipset_*: Enable gpp_bridge_[a/b/c] by default TODO justification Change-Id: Ife30f73495d44c98717e147602de10f5a6a89358 Signed-off-by: Maximilian Brune <maximilian.brune(a)9elements.com> --- M src/mainboard/google/skyrim/variants/baseboard/devicetree.cb M src/soc/amd/mendocino/chipset_mendocino.cb M src/soc/amd/mendocino/chipset_rembrandt.cb 3 files changed, 6 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/86273/1 diff --git a/src/mainboard/google/skyrim/variants/baseboard/devicetree.cb b/src/mainboard/google/skyrim/variants/baseboard/devicetree.cb index fe6537f..302d289 100644 --- a/src/mainboard/google/skyrim/variants/baseboard/devicetree.cb +++ b/src/mainboard/google/skyrim/variants/baseboard/devicetree.cb @@ -173,6 +173,7 @@ device ref acp on end # Audio Processor (ACP) device ref mp2 on end # Sensor Fusion Hub (MP2) end + device ref gpp_bridge_b on # Internal GPP Bridge 1 to Bus B device ref gpp_bridge_c on # Internal GPP Bridge 2 to Bus C device ref xhci_2 on # USB 2.0 (USB2) ops xhci_pci_ops diff --git a/src/soc/amd/mendocino/chipset_mendocino.cb b/src/soc/amd/mendocino/chipset_mendocino.cb index 6ee1e63..3458cae 100644 --- a/src/soc/amd/mendocino/chipset_mendocino.cb +++ b/src/soc/amd/mendocino/chipset_mendocino.cb @@ -16,7 +16,7 @@ device pci 02.4 alias gpp_bridge_3 hidden ops amd_external_pcie_gpp_ops end device pci 08.0 on end # Dummy device function, do not disable - device pci 08.1 alias gpp_bridge_a off # Internal GPP Bridge 0 to Bus A + device pci 08.1 alias gpp_bridge_a on # Internal GPP Bridge 0 to Bus A ops amd_internal_pcie_gpp_ops device pci 0.0 alias gfx off ops amd_graphics_ops end # Internal GPU (GFX) device pci 0.1 alias gfx_hda off end # Display HD Audio Controller (GFXAZ) @@ -65,7 +65,7 @@ device pci 0.6 alias hda off end # Audio Processor HD Audio Controller (main AZ) device pci 0.7 alias mp2 off end # Sensor Fusion Hub (MP2) end - device pci 08.3 alias gpp_bridge_c off # Internal GPP Bridge 2 to Bus C + device pci 08.3 alias gpp_bridge_c on # Internal GPP Bridge 2 to Bus C ops amd_internal_pcie_gpp_ops device pci 0.0 alias xhci_2 off end # Might also be a dummy device with different PCI DID # When using this as XHCI2, the mainboard devicetree needs to add ops xhci_pci_ops diff --git a/src/soc/amd/mendocino/chipset_rembrandt.cb b/src/soc/amd/mendocino/chipset_rembrandt.cb index ef3bfa9..aef0a89 100644 --- a/src/soc/amd/mendocino/chipset_rembrandt.cb +++ b/src/soc/amd/mendocino/chipset_rembrandt.cb @@ -18,7 +18,7 @@ device pci 02.6 alias gpp_bridge_5 hidden ops amd_external_pcie_gpp_ops end device pci 08.0 on end # Dummy device function, do not disable - device pci 08.1 alias gpp_bridge_a off # Internal GPP Bridge 0 to Bus A + device pci 08.1 alias gpp_bridge_a on # Internal GPP Bridge 0 to Bus A ops amd_internal_pcie_gpp_ops device pci 0.0 alias gfx off ops amd_graphics_ops end # Internal GPU (GFX) device pci 0.1 alias gfx_hda off end # Display HD Audio Controller (GFXAZ) @@ -67,8 +67,8 @@ device pci 0.6 alias hda off end # Audio Processor HD Audio Controller (main AZ) device pci 0.7 alias mp2 off end # Sensor Fusion Hub (MP2) end - device pci 08.2 alias gpp_bridge_b off ops amd_internal_pcie_gpp_ops end # Internal GPP Bridge 1 to Bus B - device pci 08.3 alias gpp_bridge_c off # Internal GPP Bridge 2 to Bus C + device pci 08.2 alias gpp_bridge_b on ops amd_internal_pcie_gpp_ops end # Internal GPP Bridge 1 to Bus B + device pci 08.3 alias gpp_bridge_c on # Internal GPP Bridge 2 to Bus C ops amd_internal_pcie_gpp_ops device pci 0.0 alias xhci_2 off end # Might also be a dummy device with different PCI DID # When using this as XHCI2, the mainboard devicetree needs to add ops xhci_pci_ops -- To view, visit https://review.coreboot.org/c/coreboot/+/86273?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ife30f73495d44c98717e147602de10f5a6a89358 Gerrit-Change-Number: 86273 Gerrit-PatchSet: 1 Gerrit-Owner: Maximilian Brune <maximilian.brune(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: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit February 2025