coreboot-gerrit May 2024

coreboot-gerrit@coreboot.org

1 participants
3267 discussions

[S] Change in coreboot[main]: mb/google/nissa/var/sundance: Adjust the eMMC DLL delay setting accor...
by Roger Wang (Code Review) 23 May '24

23 May '24

Attention is currently required from: Dinesh Gehlot, Eric Lai, Kapil Porwal, Nick Vaccaro, Paul Menzel, Shou-Chieh Hsu, Subrata Banik. Roger Wang has posted comments on this change by Roger Wang. ( https://review.coreboot.org/c/coreboot/+/82602?usp=email ) Change subject: mb/google/nissa/var/sundance: Adjust the eMMC DLL delay setting according to Intel's suggestion ...................................................................... Patch Set 12: (2 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/82602/comment/b205894e_2b3247d8?us… : PS1, Line 8: > > `Possible unwrapped commit description (prefer a maximum 72 chars per line)` […] Done Commit Message: https://review.coreboot.org/c/coreboot/+/82602/comment/e7919cbd_09dd46c5?us… : PS11, Line 7: Tuning > Please use imperative mood. Done -- To view, visit https://review.coreboot.org/c/coreboot/+/82602?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: I29d4305bbe5f91d822d947cae942b654e80a8a57 Gerrit-Change-Number: 82602 Gerrit-PatchSet: 12 Gerrit-Owner: Roger Wang <roger2.wang(a)lcfc.corp-partner.google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Shou-Chieh Hsu <shouchieh(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-CC: Leo Chou <leo.chou(a)lcfc.corp-partner.google.com> Gerrit-CC: Melo Chuang <melo.chuang(a)lcfc.corp-partner.google.com> Gerrit-CC: Varshit Pandya <15mece14(a)nirmauni.ac.in> Gerrit-Attention: Eric Lai <ericllai(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Shou-Chieh Hsu <shouchieh(a)google.com> Gerrit-Comment-Date: Fri, 24 May 2024 00:58:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Comment-In-Reply-To: Roger Wang <roger2.wang(a)lcfc.corp-partner.google.com>

1 0

[S] Change in coreboot[main]: mb/google/nissa/var/sundance: Adjust the eMMC DLL delay setting accor...
by Roger Wang (Code Review) 23 May '24

23 May '24

Attention is currently required from: Dinesh Gehlot, Eric Lai, Kapil Porwal, Nick Vaccaro, Paul Menzel, Shou-Chieh Hsu, Subrata Banik. Hello Dinesh Gehlot, Eric Lai, Kapil Porwal, Nick Vaccaro, Paul Menzel, Shou-Chieh Hsu, Subrata Banik, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/82602?usp=email to look at the new patch set (#12). Change subject: mb/google/nissa/var/sundance: Adjust the eMMC DLL delay setting according to Intel's suggestion ...................................................................... mb/google/nissa/var/sundance: Adjust the eMMC DLL delay setting according to Intel's suggestion Currently some eMMC can't power on to OS nomally. Use the Intel provides eMMC DLL delay patch to modify some system can't boot to OS problem BUG=b:342057438 TEST=Build and check each SKU eMMC can work. Change-Id: I29d4305bbe5f91d822d947cae942b654e80a8a57 Signed-off-by: roger2.wang <roger2.wang(a)lcfc.corp-partner.google.com> --- M src/mainboard/google/brya/variants/sundance/overridetree.cb 1 file changed, 46 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/82602/12 -- To view, visit https://review.coreboot.org/c/coreboot/+/82602?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: I29d4305bbe5f91d822d947cae942b654e80a8a57 Gerrit-Change-Number: 82602 Gerrit-PatchSet: 12 Gerrit-Owner: Roger Wang <roger2.wang(a)lcfc.corp-partner.google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Shou-Chieh Hsu <shouchieh(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-CC: Leo Chou <leo.chou(a)lcfc.corp-partner.google.com> Gerrit-CC: Melo Chuang <melo.chuang(a)lcfc.corp-partner.google.com> Gerrit-CC: Varshit Pandya <15mece14(a)nirmauni.ac.in> Gerrit-Attention: Eric Lai <ericllai(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Shou-Chieh Hsu <shouchieh(a)google.com>

1 0

[S] Change in coreboot[main]: mb/google/nissa/var/sundance: Tuning eMMC DLL delay setting from Inte...
by Roger Wang (Code Review) 23 May '24

23 May '24

Attention is currently required from: Dinesh Gehlot, Eric Lai, Kapil Porwal, Nick Vaccaro, Paul Menzel, Shou-Chieh Hsu, Subrata Banik. Roger Wang has posted comments on this change by Roger Wang. ( https://review.coreboot.org/c/coreboot/+/82602?usp=email ) Change subject: mb/google/nissa/var/sundance: Tuning eMMC DLL delay setting from Intel suggestion ...................................................................... Patch Set 11: (3 comments) This change is ready for review. Patchset: PS10: > Please rebase the CL, this is currently diverged from ToT. Done Commit Message: https://review.coreboot.org/c/coreboot/+/82602/comment/c9c9b9aa_a16d171d?us… : PS1, Line 8: > `Possible unwrapped commit description (prefer a maximum 72 chars per line)` Please fix. Commit Message: https://review.coreboot.org/c/coreboot/+/82602/comment/87314522_0be0cbd0?us… : PS4, Line 8: > `Possible unwrapped commit description (prefer a maximum 72 chars per line)` Please fix. -- To view, visit https://review.coreboot.org/c/coreboot/+/82602?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: I29d4305bbe5f91d822d947cae942b654e80a8a57 Gerrit-Change-Number: 82602 Gerrit-PatchSet: 11 Gerrit-Owner: Roger Wang <roger2.wang(a)lcfc.corp-partner.google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Shou-Chieh Hsu <shouchieh(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-CC: Leo Chou <leo.chou(a)lcfc.corp-partner.google.com> Gerrit-CC: Melo Chuang <melo.chuang(a)lcfc.corp-partner.google.com> Gerrit-CC: Varshit Pandya <15mece14(a)nirmauni.ac.in> Gerrit-Attention: Eric Lai <ericllai(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Shou-Chieh Hsu <shouchieh(a)google.com> Gerrit-Comment-Date: Fri, 24 May 2024 00:54:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Shou-Chieh Hsu <shouchieh(a)google.com>

1 0

[S] Change in coreboot[main]: mb/google/nissa/var/pujjoga: Modify touchscreen IC setting from new v...
by Leo Chou (Code Review) 23 May '24

23 May '24

Attention is currently required from: Dinesh Gehlot, Eric Lai, Felix Held, Kapil Porwal, Nick Vaccaro, Roger Wang, Shou-Chieh Hsu, Subrata Banik. Leo Chou has posted comments on this change by Roger Wang. ( https://review.coreboot.org/c/coreboot/+/82427?usp=email ) Change subject: mb/google/nissa/var/pujjoga: Modify touchscreen IC setting from new vendor ...................................................................... Patch Set 5: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/82427/comment/8665ca51_f281a69d?us… : PS1, Line 9: Modify the Goodix touchscreen from new vendor. Based on keypart team provide's information > please also mentioned you remove the 3 touchscreens. Done -- To view, visit https://review.coreboot.org/c/coreboot/+/82427?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: I1e6349e80431aadf27cd72b8439b01f95348071d Gerrit-Change-Number: 82427 Gerrit-PatchSet: 5 Gerrit-Owner: Roger Wang <roger2.wang(a)lcfc.corp-partner.google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Shou-Chieh Hsu <shouchieh(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Jerry2 Huang <jerry2.huang(a)lcfc.corp-partner.google.com> Gerrit-CC: Leo Chou <leo.chou(a)lcfc.corp-partner.google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Eric Lai <ericllai(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Shou-Chieh Hsu <shouchieh(a)google.com> Gerrit-Attention: Roger Wang <roger2.wang(a)lcfc.corp-partner.google.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Fri, 24 May 2024 00:53:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Eric Lai <ericllai(a)google.com>

1 0

[M] Change in coreboot[main]: arch/arm64: Support calling a trusted monitor
by Julius Werner (Code Review) 23 May '24

23 May '24

Attention is currently required from: Arthur Heymans, Benjamin Doron, David Milosevic. Julius Werner has posted comments on this change by Benjamin Doron. ( https://review.coreboot.org/c/coreboot/+/78284?usp=email ) Change subject: arch/arm64: Support calling a trusted monitor ...................................................................... Patch Set 19: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/78284?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: I158db0b971aba722b3995d52162146aa406d1644 Gerrit-Change-Number: 78284 Gerrit-PatchSet: 19 Gerrit-Owner: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Reviewer: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Reviewer: David Milosevic <David.Milosevic(a)9elements.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: David Milosevic <David.Milosevic(a)9elements.com> Gerrit-Comment-Date: Thu, 23 May 2024 22:07:02 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[M] Change in coreboot[main]: device: drop unnecessary CHECK_REV_IN_OPROM_NAME option
by Felix Held (Code Review) 23 May '24

23 May '24

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/82598?usp=email ) Change subject: device: drop unnecessary CHECK_REV_IN_OPROM_NAME option ...................................................................... device: drop unnecessary CHECK_REV_IN_OPROM_NAME option The CHECK_REV_IN_OPROM_NAME Kconfig option was introduced to solve the problem of the PCI VID/DID combination of the Picasso iGPU not being sufficient information to know which VGA BIOS file to run, so a new function that additionally checks the PCI revision of that device was introduced. Later it turned out that there might be a case where even that isn't sufficient, so the soc_is_raven2() function is used in the remap function to always use the correct VBIOS file. Picasso is the only SoC that selected the CHECK_REV_IN_OPROM_NAME Kconfig option, so all other SoCs are unaffected by this change. Now that we use the VBIOS images with only the PCI VID and DID in the CBFS file name for Picasso, SeaBIOS will find the VBIOS with the same ID as the iGPU in CBFS and we don't need the workaround to add a third VBIOS image via VGA_BIOS_DGPU_* that has the name that SeaBIOS expects. This will result in SeaBIOS now running the VBIOS that has the same PCI VID/DID as the hardware which will be the wrong one in the RV2 silicon showing the PCO silicon PCI VID/DID, but that was also the case with the VGA_BIOS_DGPU_* workaround where the board's Kconfig just selected one of the two possible images during build time and hoped that it was the correct one for that actual hardware. The only board where this patch might cause a regression compared to the old behavior is the AMD Cereme reference board with Pollock APU, but I'm not even sure if any coreboot developer still has one of those boards, so I'm willing to accept that. To properly solve the problem with SeaBIOS using the correct VBIOS file in all cases, we'd need to generate that info during coreboot runtime and somehow pass it to SeaBIOS, but that's out of scope for this patch. TEST=On Mandolin with PCO silicon, the display output in both SeaBIOS and Ubuntu still works. Booting Windows 10 via the pre-built EDK2 payload that I'm using also resulted in the display output working. Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: Ia6de533c536044698d85404427719b8f534870fa Reviewed-on: https://review.coreboot.org/c/coreboot/+/82598 Reviewed-by: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/device/Kconfig M src/device/pci_rom.c M src/include/device/pci_rom.h M src/mainboard/amd/bilby/Kconfig M src/mainboard/amd/mandolin/Kconfig M src/soc/amd/picasso/Kconfig M src/soc/amd/picasso/graphics.c 7 files changed, 21 insertions(+), 81 deletions(-) Approvals: Matt DeVillier: Looks good to me, approved build bot (Jenkins): Verified diff --git a/src/device/Kconfig b/src/device/Kconfig index 404c73d..0fe0a09 100644 --- a/src/device/Kconfig +++ b/src/device/Kconfig @@ -846,18 +846,18 @@ depends on VGA_BIOS default "1106,3230" help - The comma-separated PCI vendor and device ID with optional revision if that - feature is enabled that would associate your vBIOS to your video card. + The comma-separated PCI vendor and device ID that would associate + your vBIOS to your video card. - Example: 1106,3230 or 1106,3230,a3 + Example: 1106,3230 In the above example 1106 is the PCI vendor ID (in hex, but without the "0x" prefix) and 3230 specifies the PCI device ID of the - video card (also in hex, without "0x" prefix). a3 specifies the revision. + video card (also in hex, without "0x" prefix). This ID needs to match the PCI VID and DID in the VGA BIOS file's header and also needs to match the value returned by map_oprom_vendev - or map_oprom_vendev_rev if the remapping feature is used. + if the remapping feature is used. Under GNU/Linux you can run `lspci -nn` to list the IDs of your PCI devices. @@ -879,23 +879,17 @@ string "Graphics device PCI IDs" depends on VGA_BIOS_SECOND help - The comma-separated PCI vendor and device ID with optional revision if that - feature is enabled that would associate your vBIOS to your video card. + The comma-separated PCI vendor and device ID that would associate + your vBIOS to your video card. - Example: 1106,3230 or 1106,3230,a3 + Example: 1106,3230 In the above example 1106 is the PCI vendor ID (in hex, but without the "0x" prefix) and 3230 specifies the PCI device ID of the - video card (also in hex, without "0x" prefix). a3 specifies the revision. + video card (also in hex, without "0x" prefix). Under GNU/Linux you can run `lspci -nn` to list the IDs of your PCI devices. -config CHECK_REV_IN_OPROM_NAME - def_bool n - help - Select this in the platform BIOS or chipset if the option rom has a revision - that needs to be checked when searching CBFS. - config VGA_BIOS_DGPU bool "Add a discrete VGA BIOS image" depends on VGA_BIOS diff --git a/src/device/pci_rom.c b/src/device/pci_rom.c index aca55d6..b9210b0 100644 --- a/src/device/pci_rom.c +++ b/src/device/pci_rom.c @@ -12,7 +12,6 @@ #include <acpi/acpigen.h> /* Rmodules don't like weak symbols. */ -void __weak map_oprom_vendev_rev(u32 *vendev, u8 *rev) { return; } u32 __weak map_oprom_vendev(u32 vendev) { return vendev; } void vga_oprom_preload(void) @@ -39,35 +38,16 @@ return cbfs_map(name, NULL); } -static void *cbfs_boot_map_optionrom_revision(uint16_t vendor, uint16_t device, uint8_t rev) -{ - char name[20] = "pciXXXX,XXXX,XX.rom"; - - snprintf(name, sizeof(name), "pci%04hx,%04hx,%02hhx.rom", vendor, device, rev); - - return cbfs_map(name, NULL); -} - struct rom_header *pci_rom_probe(const struct device *dev) { struct rom_header *rom_header = NULL; struct pci_data *rom_data; - u8 rev = pci_read_config8(dev, PCI_REVISION_ID); - u8 mapped_rev = rev; u32 vendev = (dev->vendor << 16) | dev->device; u32 mapped_vendev = vendev; /* If the ROM is in flash, then don't check the PCI device for it. */ - if (CONFIG(CHECK_REV_IN_OPROM_NAME)) { - map_oprom_vendev_rev(&mapped_vendev, &mapped_rev); - rom_header = cbfs_boot_map_optionrom_revision(mapped_vendev >> 16, - mapped_vendev & 0xffff, - mapped_rev); - } else { - mapped_vendev = map_oprom_vendev(vendev); - rom_header = cbfs_boot_map_optionrom(mapped_vendev >> 16, - mapped_vendev & 0xffff); - } + mapped_vendev = map_oprom_vendev(vendev); + rom_header = cbfs_boot_map_optionrom(mapped_vendev >> 16, mapped_vendev & 0xffff); if (rom_header) { printk(BIOS_DEBUG, "In CBFS, ROM address for %s = %p\n", diff --git a/src/include/device/pci_rom.h b/src/include/device/pci_rom.h index 19728f2..531ec18 100644 --- a/src/include/device/pci_rom.h +++ b/src/include/device/pci_rom.h @@ -55,7 +55,6 @@ void pci_rom_ssdt(const struct device *device); -void map_oprom_vendev_rev(u32 *vendev, u8 *rev); u32 map_oprom_vendev(u32 vendev); int verified_boot_should_run_oprom(struct rom_header *rom_header); diff --git a/src/mainboard/amd/bilby/Kconfig b/src/mainboard/amd/bilby/Kconfig index 19fcbe5..9573204 100644 --- a/src/mainboard/amd/bilby/Kconfig +++ b/src/mainboard/amd/bilby/Kconfig @@ -80,18 +80,6 @@ Picasso's LPC bus signals are MUXed with some of the EMMC signals. Select this option if LPC signals are required. -#TODO: remove this hack to not break graphics in combination with SeaBIOS -config VGA_BIOS_DGPU_ID - string - default "1002,15d8" - help - The default VGA BIOS PCI vendor/device ID should be set to the - result of the map_oprom_vendev() function in northbridge.c. - -config VGA_BIOS_DGPU_FILE - string - default "3rdparty/amd_blobs/picasso/PicassoGenericVbios.bin" - if !EM100 # EM100 defaults in soc/amd/common/blocks/spi/Kconfig config EFS_SPI_READ_MODE default 3 # Quad IO (1-1-4) diff --git a/src/mainboard/amd/mandolin/Kconfig b/src/mainboard/amd/mandolin/Kconfig index 252404b..ce3af3b 100644 --- a/src/mainboard/amd/mandolin/Kconfig +++ b/src/mainboard/amd/mandolin/Kconfig @@ -106,19 +106,6 @@ Picasso's LPC bus signals are MUXed with some of the EMMC signals. Select this option if LPC signals are required. -#TODO: remove this hack to not break graphics in combination with SeaBIOS -config VGA_BIOS_DGPU_ID - string - default "1002,15d8" - help - The default VGA BIOS PCI vendor/device ID should be set to the - result of the map_oprom_vendev() function in northbridge.c. - -config VGA_BIOS_DGPU_FILE - string - default "3rdparty/amd_blobs/picasso/PicassoGenericVbios.bin" if BOARD_AMD_MANDOLIN - default "3rdparty/amd_blobs/picasso/Raven2GenericVbios.bin" if BOARD_AMD_CEREME - if !EM100 # EM100 defaults in soc/amd/common/blocks/spi/Kconfig config EFS_SPI_READ_MODE default 3 # Quad IO (1-1-4) diff --git a/src/soc/amd/picasso/Kconfig b/src/soc/amd/picasso/Kconfig index d6c3fbd..f450e29 100644 --- a/src/soc/amd/picasso/Kconfig +++ b/src/soc/amd/picasso/Kconfig @@ -230,10 +230,10 @@ config VGA_BIOS_ID string - default "1002,15d8,c1" + default "1002,15d8" help The default VGA BIOS PCI vendor/device ID should be set to the - result of the map_oprom_vendev_rev() function in graphics.c. + result of the map_oprom_vendev() function in graphics.c. config VGA_BIOS_FILE string @@ -244,25 +244,18 @@ config VGA_BIOS_SECOND_ID string - default "1002,15dd,c4" + default "1002,15dd" help Some Dali and all Pollock APUs need a different VBIOS than some other Dali and all Picasso APUs, but don't always have a different PCI vendor/device IDs, so we need an alternate method to determine the - correct video BIOS. In map_oprom_vendev_rev(), we look at the return + correct video BIOS. In map_oprom_vendev(), we look at the return value of soc_is_raven2() and decide which rom to load. config VGA_BIOS_SECOND_FILE string default "3rdparty/amd_blobs/picasso/Raven2GenericVbios.bin" -config CHECK_REV_IN_OPROM_NAME - bool - default y - help - Select this in the platform BIOS or chipset if the option rom has a - revision that needs to be checked when searching CBFS. - config S3_VGA_ROM_RUN bool default n diff --git a/src/soc/amd/picasso/graphics.c b/src/soc/amd/picasso/graphics.c index 3c2e0f8..bd7ec77 100644 --- a/src/soc/amd/picasso/graphics.c +++ b/src/soc/amd/picasso/graphics.c @@ -6,20 +6,19 @@ #include <soc/soc_util.h> #include <stdint.h> -void map_oprom_vendev_rev(u32 *vendev, u8 *rev) +u32 map_oprom_vendev(u32 vendev) { - if (*vendev == PICASSO_VBIOS_VID_DID) { + if (vendev == PICASSO_VBIOS_VID_DID) { /* Check if the RV2 video bios needs to be used instead of the RV1/PCO one */ if (soc_is_raven2()) { printk(BIOS_NOTICE, "Using RV2 VBIOS.\n"); - *vendev = RAVEN2_VBIOS_VID_DID; - *rev = RAVEN2_VBIOS_REV; + return RAVEN2_VBIOS_VID_DID; } else { printk(BIOS_NOTICE, "Using RV1/PCO VBIOS.\n"); - *rev = PICASSO_VBIOS_REV; } - } else if (*vendev == RAVEN2_VBIOS_VID_DID) { + } else if (vendev == RAVEN2_VBIOS_VID_DID) { printk(BIOS_NOTICE, "Using RV2 VBIOS.\n"); - *rev = RAVEN2_VBIOS_REV; } + + return vendev; } -- To view, visit https://review.coreboot.org/c/coreboot/+/82598?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ia6de533c536044698d85404427719b8f534870fa Gerrit-Change-Number: 82598 Gerrit-PatchSet: 5 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> 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: Paul Menzel <paulepanter(a)mailbox.org>

1 0

[S] Change in coreboot[main]: mainboard/google/brox: Add FW_CONFIG and SKU definitions to support l...
by Bob Moragues (Code Review) 23 May '24

23 May '24

Attention is currently required from: Bob Moragues, Shelley Chen. Hello Bob Moragues, Shelley Chen, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/82630?usp=email to look at the new patch set (#3). Change subject: mainboard/google/brox: Add FW_CONFIG and SKU definitions to support lotso ...................................................................... mainboard/google/brox: Add FW_CONFIG and SKU definitions to support lotso BUG=b:331113852 TEST=gen config and full build Signed-off-by: Bob Moragues <moragues(a)google.com> Change-Id: I52cf42b79eff91ab2b4e98a7b5961310e60f2ea7 --- M src/mainboard/google/brox/variants/brox/overridetree.cb M src/mainboard/google/brox/variants/lotso/overridetree.cb 2 files changed, 24 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/30/82630/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/82630?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: I52cf42b79eff91ab2b4e98a7b5961310e60f2ea7 Gerrit-Change-Number: 82630 Gerrit-PatchSet: 3 Gerrit-Owner: Bob Moragues <moragues(a)chromium.org> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Bob Moragues <moragues(a)google.com> Gerrit-Attention: Shelley Chen <shchen(a)google.com> Gerrit-Attention: Bob Moragues <moragues(a)chromium.org>

1 0

[S] Change in coreboot[main]: mainboard/google/brox: Add FW_CONFIG and SKU definitions to support l...
by Bob Moragues (Code Review) 23 May '24

23 May '24

Attention is currently required from: Bob Moragues, Bob Moragues, Shelley Chen. Hello Bob Moragues, Shelley Chen, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/82630?usp=email to look at the new patch set (#2). Change subject: mainboard/google/brox: Add FW_CONFIG and SKU definitions to support lotso ...................................................................... mainboard/google/brox: Add FW_CONFIG and SKU definitions to support lotso BUG=b:331113852 TEST=gen config and full build Signed-off-by: moragues(a)google.com Change-Id: I52cf42b79eff91ab2b4e98a7b5961310e60f2ea7 --- M src/mainboard/google/brox/variants/brox/overridetree.cb M src/mainboard/google/brox/variants/lotso/overridetree.cb 2 files changed, 24 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/30/82630/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/82630?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: I52cf42b79eff91ab2b4e98a7b5961310e60f2ea7 Gerrit-Change-Number: 82630 Gerrit-PatchSet: 2 Gerrit-Owner: Bob Moragues <moragues(a)chromium.org> Gerrit-Reviewer: Bob Moragues <moragues(a)google.com> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Shelley Chen <shchen(a)google.com> Gerrit-Attention: Bob Moragues <moragues(a)google.com> Gerrit-Attention: Bob Moragues <moragues(a)chromium.org>

1 0

[S] Change in coreboot[main]: mainboard/google/brox: Add FW_CONFIG and SKU definitions to support l...
by Bob Moragues (Code Review) 23 May '24

23 May '24

Bob Moragues has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/82630?usp=email ) Change subject: mainboard/google/brox: Add FW_CONFIG and SKU definitions to support lotso ...................................................................... mainboard/google/brox: Add FW_CONFIG and SKU definitions to support lotso BUG=b:331113852 TEST=gen config and full build Change-Id: I52cf42b79eff91ab2b4e98a7b5961310e60f2ea7 --- M src/mainboard/google/brox/variants/brox/overridetree.cb M src/mainboard/google/brox/variants/lotso/overridetree.cb 2 files changed, 24 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/30/82630/1 diff --git a/src/mainboard/google/brox/variants/brox/overridetree.cb b/src/mainboard/google/brox/variants/brox/overridetree.cb index e9a467a..3a9b687 100644 --- a/src/mainboard/google/brox/variants/brox/overridetree.cb +++ b/src/mainboard/google/brox/variants/brox/overridetree.cb @@ -16,6 +16,7 @@ field AUDIO 5 7 option AUDIO_UNKNOWN 0 option AUDIO_REALTEK_ALC256 1 + option AUDIO_REALTEK_ALC3287 2 end field UFC 8 9 option UFC_NONE 0 diff --git a/src/mainboard/google/brox/variants/lotso/overridetree.cb b/src/mainboard/google/brox/variants/lotso/overridetree.cb index 7790bce..88d5efd 100644 --- a/src/mainboard/google/brox/variants/lotso/overridetree.cb +++ b/src/mainboard/google/brox/variants/lotso/overridetree.cb @@ -1,9 +1,31 @@ fw_config - field STORAGE 0 1 + field RETIMER 0 1 + option RETIMER_UNKNOWN 0 + option RETIMER_BYPASS 1 + option RETIMER_JHL8040 2 + end + field STORAGE 2 3 option STORAGE_UNKNOWN 0 option STORAGE_UFS 1 option STORAGE_NVME 2 end + field WIFI_BT 4 4 + option WIFI_BT_CNVI 0 + option WIFI_BT_PCIE 1 + end + field AUDIO 5 7 + option AUDIO_UNKNOWN 0 + option AUDIO_REALTEK_ALC256 1 + option AUDIO_REALTEK_ALC3287 2 + end + field UFC 8 9 + option UFC_NONE 0 + option UFC_OV2740 1 + end + field ISH 21 + option ISH_DISABLE 0 + option ISH_ENABLE 1 + end end chip soc/intel/alderlake -- To view, visit https://review.coreboot.org/c/coreboot/+/82630?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: I52cf42b79eff91ab2b4e98a7b5961310e60f2ea7 Gerrit-Change-Number: 82630 Gerrit-PatchSet: 1 Gerrit-Owner: Bob Moragues <moragues(a)chromium.org> Gerrit-CC: Bob Moragues <moragues(a)google.com>

1 0

[L] Change in coreboot[main]: libpayload: Add x86_64 (64-bit) support
by Julius Werner (Code Review) 23 May '24

23 May '24

Attention is currently required from: Arthur Heymans, Kapil Porwal, Subrata Banik. Julius Werner has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/81968?usp=email ) Change subject: libpayload: Add x86_64 (64-bit) support ...................................................................... Patch Set 74: (9 comments) File payloads/libpayload/arch/x86/head_64.S: https://review.coreboot.org/c/coreboot/+/81968/comment/6f9b2118_5ccf6411?us… : PS74, Line 38: * Payload Entry Point Behavior with below code. This table is okay but I think there should also be a more explicit warning that this code needs to run in both modes and certain instructions need to be avoided. How about: ``` WARNING: The payload is designed to be entered in either 32-bit protected or 64-bit long mode, and the below code (between `_entry` and `jnz _init64`) must work correctly in both modes. That means you should only use instructions that assemble to the same binary representation in either mode. Any changes to this code should be tested for both cases (entry from 32-bit and entry from 64-bit mode). ``` https://review.coreboot.org/c/coreboot/+/81968/comment/85a7afed_efe59ac7?us… : PS74, Line 41: LP_ARCH_X86_64 This column seems pointless. https://review.coreboot.org/c/coreboot/+/81968/comment/17ae0d51_f8ffa7cf?us… : PS74, Line 121: What about `fninit`? File payloads/libpayload/arch/x86/pt.S: https://review.coreboot.org/c/coreboot/+/81968/comment/fd701428_9834f4f1?us… : PS74, Line 51: .global pde_table I think these names are problematic now because you're using `pde_table` for the PDE table in the 2M paging case but for the PDPTE table in the 1GB paging case. Would probably make more sense to call these `main_page_table` and `extra_page_table` or something like that. https://review.coreboot.org/c/coreboot/+/81968/comment/b8dfa223_9c56c36a?us… : PS74, Line 65: .type init_page_table, @function Should be a similar warning here, I'd suggest: ``` WARNING: The `init_page_table` function is designed to be entered in either 32-bit protected or 64-bit long mode and must work correctly in both modes. That means you should only use instructions that assemble to the correct binary representation for either mode. We are building this with `.code64` to force the assembler to use the longer representation of the `inc` instruction (`FF r/m32` instead of `4X`), because the latter is only valid in 32-bit mode (would be a REX-prefix in 64-bit mode). When using register-indirect addressing (e.g. `mov %eax, (%rsi, %rcx, 8)`), the source register operands need to be specified in 64-bit notation (`%rsi` instead of `%esi`) to prevent the assembler from generating an extra address size prefix (`67`) that means something different in 32-bit mode. This is fine because `mov $..., %esi` will set the high 32-bits of the `%rsi` register to 0 in 64-bit mode, so it doesn't matter whether we're using to the 32-bit or 64-bit version of the register. Any changes to this code should be tested for both cases (entry from 32-bit and entry from 64-bit mode). ``` https://review.coreboot.org/c/coreboot/+/81968/comment/862f7741_9f5ab778?us… : PS74, Line 75: mov $(_PRES + _RW + _US + _A + _D), %eax I still don't understand why you're not setting `_PS` here tbh. Am I misunderstanding how this works? Table 4-17 "Format of an IA-32e Page-Directory Entry that Maps a 2-MByte Page" says "7 (PS) | Page size; must be 1 (otherwise, this entry references a page table; see Table 4-18)". Did you test this code (commenting out the `jnz` above to force it to run) and confirm that it works this way? https://review.coreboot.org/c/coreboot/+/81968/comment/269f5da9_c3723081?us… : PS74, Line 82: mov %ebx, 4(%rsi, %rcx, 8) Actually, the same argument as below also applies here: we're only writing 2MB tables for the low 4GB of memory, so we know that EAX will never overflow. Can simplify this to ``` loop_2mb: mov %eax, (%rsi, %rcx, 8) mov $0, 4(%rsi, rcx, 8) add $0x200000, %eax inc %ecx cmp %edi, %ecx jb fill_pdpe ``` https://review.coreboot.org/c/coreboot/+/81968/comment/02987c87_9a45f6d4?us… : PS74, Line 104: cmp %edx, %eax Sorry, I'm very confused about what you're doing here and what you're using EDX for (instead of EAX like with the other versions of this loop). What's the point of adding EDX to EAX and then comparing with EAX? You're not initializing EAX anywhere so it would still point to the end of the PDE table from the previous loop (and then you're adding more to it)? In general, the point of this overflow check is just to see if we need to increment EBX because EAX overflowed over the 4GB limit. In this case, since we know that EAX just walks the PDE table and libpayload (including the entire PDE table in BSS) is linked below 4GB, we can actually skip that entire check because we know that it will never cross that boundary (and EBX will never need to be anything other than 0). So in fact I think we can simplify this entire thing to: ``` mov $4, %edi mov $(_PRES + _RW + _US + _A + pde_table), %eax mov $0, %ebx mov $0, %ecx mov $pdpe_table, %esi fill_pdpe: mov %eax, (%rsi, %rcx, 8) mov $0, 4(%rsi, rcx, 8) add $4096, %eax inc %ecx cmp %edi, %ecx jb fill_pdpe ``` (I don't know if the linker supports a relocation for a construct like `mov $(_PRES + _RW + _US + _A + pde_table), %eax`. If not, then just do `mov $pde_table, %eax` and `add $(_PRES + _RW + _US + _A), %eax`.) https://review.coreboot.org/c/coreboot/+/81968/comment/c886c8f4_f07e221c?us… : PS74, Line 116: jmp leave nit: Easier to just do `ret` here? -- To view, visit https://review.coreboot.org/c/coreboot/+/81968?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: I69fda47bedf1a14807b1515c4aed6e3a1d5b8585 Gerrit-Change-Number: 81968 Gerrit-PatchSet: 74 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-CC: Kapil Porwal <kapilporwal(a)google.com> Gerrit-CC: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Thu, 23 May 2024 21:29:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit May 2024