coreboot-gerrit May 2021

coreboot-gerrit@coreboot.org

1 participants
6047 discussions

Change in coreboot[master]: vc/amd/fsp/cezanne: Add support for AMD_FSP_PCIE_DEVFUNC_REMAP_HOB_GUID
by Raul Rangel (Code Review) 03 May '21

03 May '21

Raul Rangel has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/52869 ) Change subject: vc/amd/fsp/cezanne: Add support for AMD_FSP_PCIE_DEVFUNC_REMAP_HOB_GUID ...................................................................... vc/amd/fsp/cezanne: Add support for AMD_FSP_PCIE_DEVFUNC_REMAP_HOB_GUID This HOB describes the PCI routing table. It will be consumed by coreboot to generate the _PRT ACPI object. BUG=b:184766519, b:184766197 TEST=Dump guybrush ACPI table and verify it looks correct. Cq-Depend: chrome-internal:3794981 Signed-off-by: Raul E Rangel <rrangel(a)chromium.org> Change-Id: Ib790004b88dfaf7671534f657c7735f6718114db --- M src/vendorcode/amd/fsp/cezanne/FspGuids.h A src/vendorcode/amd/fsp/cezanne/pcie_routing_hob.h 2 files changed, 84 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/69/52869/1 diff --git a/src/vendorcode/amd/fsp/cezanne/FspGuids.h b/src/vendorcode/amd/fsp/cezanne/FspGuids.h index 7f7a91d..c26ee24 100644 --- a/src/vendorcode/amd/fsp/cezanne/FspGuids.h +++ b/src/vendorcode/amd/fsp/cezanne/FspGuids.h @@ -13,4 +13,8 @@ GUID_INIT(0xa21f7ab5, 0x6a89, 0x4df2, \ 0xb9, 0x19, 0x51, 0xad, 0x95, 0x50, 0x5b, 0xd8) +#define AMD_FSP_PCIE_DEVFUNC_REMAP_HOB_GUID \ + GUID_INIT(0x00D54AA7, 0x0002, 0x47F5, \ + 0x00, 0x78, 0x08, 0x57, 0x00, 0x00, 0xA4, 0x11) + #endif /* __FSP_GUIDS__ */ diff --git a/src/vendorcode/amd/fsp/cezanne/pcie_routing_hob.h b/src/vendorcode/amd/fsp/cezanne/pcie_routing_hob.h new file mode 100644 index 0000000..87e5674 --- /dev/null +++ b/src/vendorcode/amd/fsp/cezanne/pcie_routing_hob.h @@ -0,0 +1,80 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#ifndef _VC_AMD_FSP_PCIE_ROUTING_HOB_H_ +#define _VC_AMD_FSP_PCIE_ROUTING_HOB_H_ + +#include <console/console.h> +#include <fsp/util.h> +#include <FspGuids.h> +#include <types.h> + +/** + * Each PCI bridge has its INTx lines routed to one of the GNB IOAPIC PCI + * groups. Each group has 4 interrupts. The INTx lines can be swizzled before + * being routed to the IOAPIC. If the IOAPIC redirection entry is masked, the + * interrupt is reduced modulo 8 onto INT[A-H] and forwarded to the FCH IOAPIC. + **/ +struct pcie_routing_info { + uint8_t devfn; + uint8_t group; + uint8_t swizzle; + uint8_t irq; +}; + +enum pcie_swizzle_pin { + PIN_A, + PIN_B, + PIN_C, + PIN_D, +}; + +static const uint8_t pcie_swizzle_table[][4] = { + {PIN_A, PIN_B, PIN_C, PIN_D}, + {PIN_B, PIN_C, PIN_D, PIN_A}, + {PIN_C, PIN_D, PIN_A, PIN_B}, + {PIN_D, PIN_A, PIN_B, PIN_C}, +}; + +static inline const struct pcie_routing_info *get_pcie_routing_info(unsigned int devfn) +{ + const struct pcie_routing_info *routing_info; + size_t hob_size = 0, entries = 0; + + routing_info = fsp_find_extension_hob_by_guid(AMD_FSP_PCIE_DEVFUNC_REMAP_HOB_GUID.b, + &hob_size); + + if (routing_info == NULL || hob_size == 0) { + printk(BIOS_ERR, "Couldn't find PCIe routing HOB.\n"); + return NULL; + } + + entries = hob_size / sizeof(*routing_info); + + for (size_t i = 0; i < entries; ++i, ++routing_info) + if (routing_info->devfn == devfn) + return routing_info; + + printk(BIOS_ERR, "Failed to find PCIe routing info for dev: %#x, fn: %#x\n", + PCI_SLOT(devfn), PCI_FUNC(devfn)); + + return NULL; +} + +static inline unsigned int pcie_calculate_irq(const struct pcie_routing_info *routing_info, + unsigned int pin) +{ + unsigned int irq; + + if (routing_info->swizzle > ARRAY_SIZE(pcie_swizzle_table)) + die("%s: swizzle %u out of bounds\n", __func__, routing_info->swizzle); + + if (pin > ARRAY_SIZE(pcie_swizzle_table[routing_info->swizzle])) + die("%s: pin %u out of bounds\n", __func__, pin); + + irq = routing_info->group * 4; + irq += pcie_swizzle_table[routing_info->swizzle][pin]; + + return irq; +} + +#endif /* _VC_AMD_FSP_PCIE_ROUTING_HOB_H_ */ -- To view, visit https://review.coreboot.org/c/coreboot/+/52869 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib790004b88dfaf7671534f657c7735f6718114db Gerrit-Change-Number: 52869 Gerrit-PatchSet: 1 Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: soc/amd/cezanne: Generate PCI routing table
by Raul Rangel (Code Review) 03 May '21

03 May '21

Attention is currently required from: Jason Glenesk, Marshall Dawson, Eric Peers, Yu-hsuan Hsu, Karthik Ramasubramanian, Felix Held. Hello build bot (Jenkins), Jason Glenesk, Marshall Dawson, Felix Held, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/51556 to look at the new patch set (#2). Change subject: soc/amd/cezanne: Generate PCI routing table ...................................................................... soc/amd/cezanne: Generate PCI routing table We use the FSP PCI routing HOB to construct the ACPI _PRT table. This code was based off of picasso's pcie_gpp.c. The eventual goal is to make picasso's FSP export the same HOB and then we can move this code into amd/common. BUG=b:184766519 TEST=Dump guybrush ACPI table and verify it looks correct. Signed-off-by: Raul E Rangel <rrangel(a)chromium.org> Change-Id: Idb559335435a95e73640e6d7fb224e16e0592326 --- A src/soc/amd/cezanne/acpi/pci_int.asl M src/soc/amd/cezanne/acpi/soc.asl M src/soc/amd/cezanne/pcie_gpp.c 3 files changed, 307 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/56/51556/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/51556 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Idb559335435a95e73640e6d7fb224e16e0592326 Gerrit-Change-Number: 51556 Gerrit-PatchSet: 2 Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Eric Peers <epeers(a)google.com> Gerrit-CC: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-CC: Matt Papageorge <matthewpapa07(a)gmail.com> Gerrit-CC: Yu-hsuan Hsu <yuhsuan(a)google.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Eric Peers <epeers(a)google.com> Gerrit-Attention: Yu-hsuan Hsu <yuhsuan(a)google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: drivers/intel/fsp2_0: Add timestamps for loading FSPM & FSPS
by Furquan Shaikh (Code Review) 03 May '21

03 May '21

Attention is currently required from: Martin Roth, Andrey Petrov, Patrick Rudolph. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52867 ) Change subject: drivers/intel/fsp2_0: Add timestamps for loading FSPM & FSPS ...................................................................... Patch Set 1: (2 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/52867/comment/275140c8_95b65c31 PS1, Line 15: timestamps. Can you please add a snippet of the timestamps here? File src/commonlib/include/commonlib/timestamp_serialized.h: https://review.coreboot.org/c/coreboot/+/52867/comment/216d58a6_63270893 PS1, Line 122: INITRAM MEMORY_INIT to keep it consistent with TS_FSP_MEMORY_INIT_START and TS_FSP_MEMORY_INIT_END -- To view, visit https://review.coreboot.org/c/coreboot/+/52867 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5796d4cdd512799c2eafee45a8ef561de5258b91 Gerrit-Change-Number: 52867 Gerrit-PatchSet: 1 Gerrit-Owner: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Mon, 03 May 2021 23:02:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: drivers/intel/fsp2_0: Add mb hooks before & after FSP calls
by Furquan Shaikh (Code Review) 03 May '21

03 May '21

Attention is currently required from: Martin Roth, Subrata Banik, Andrey Petrov, Patrick Rudolph. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52866 ) Change subject: drivers/intel/fsp2_0: Add mb hooks before & after FSP calls ...................................................................... Patch Set 1: (6 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/52866/comment/b2617892_e9afda7f PS1, Line 9: There are currently various callbacks and hooks for chipsets and : mainboards in various places around the FSP calls, but I need some : mainboard hooks immediately before and after the FSP calls. From the follow-up changes, it looks like you are using these callbacks for configuring pads before the FSP call. This is currently achieved by using the calls: `platform_fsp_memory_init_params_cb` for FSP-M `platform_fsp_silicon_init_params_cb` for FSP-S What is the significance of performing the GPIO initialization just before the jump to FSP-M/S? Not completely against having the hooks, but currently there is one function call into SoC (which in turn can call mainboard) that allows each to perform any initialization that is required before the FSP calls. It also ensures that proper ordering is maintained between SoC and mainboard w.r.t. initializing UPD params. https://review.coreboot.org/c/coreboot/+/52866/comment/03fa8d3c_dc488d85 PS1, Line 13: This allows for GPIO initialization before the calls as required That can be achieved by using the call chain for platform_fsp_memory_init_params_cb and platform_fsp_silicon_init_params_cb https://review.coreboot.org/c/coreboot/+/52866/comment/7c069d98_873671aa PS1, Line 14: easy analysis and updates of register changes within the FSP. If this is the intent, I think a better place to put the callbacks would be fsp_debug_{before|after}_{memory|silicon}_init(). It will allow you to dump or check any registers or configurations before and after FSP calls. Example: Recently the debug path was updated to snapshot and verify any GPIO changes across FSP calls: https://review.coreboot.org/plugins/gitiles/coreboot/+/refs/heads/master/sr…. Similarly, other hooks can be added as required. File src/drivers/intel/fsp2_0/include/fsp/api.h: https://review.coreboot.org/c/coreboot/+/52866/comment/03b8c29e_7f78ac43 PS1, Line 75: print values This is better suited for the debug path in my opinion. https://review.coreboot.org/c/coreboot/+/52866/comment/2175e625_b9b139d2 PS1, Line 75: save and restore registers This seems like a problem with the FSP implementation itself. If there are certain things FSP must not touch, then we should be pushing the SoC vendor to fix that rather than adding permanent workarounds in coreboot. I understand that we have to deal with such situations and we have intentionally kept this outside the FSP driver to ensure that those actually get addressed rather than accepting it as the right solution. Example: CB:52248 https://review.coreboot.org/c/coreboot/+/52866/comment/e0ce83da_9e29667a PS1, Line 75: other initialization Other initialization can already be handled via the callback path we already have. -- To view, visit https://review.coreboot.org/c/coreboot/+/52866 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8dc6eb95fa8a1114234cfd7467507992c25669f1 Gerrit-Change-Number: 52866 Gerrit-PatchSet: 1 Gerrit-Owner: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Mon, 03 May 2021 22:55:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/skylake: Drop EXCLUDE_NATIVE_SD_INTERFACE
by Furquan Shaikh (Code Review) 03 May '21

03 May '21

Attention is currently required from: Felix Singer, Nico Huber, Martin Roth, Tim Wawrzynczak, Angel Pons, Subrata Banik. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52789 ) Change subject: soc/intel/skylake: Drop EXCLUDE_NATIVE_SD_INTERFACE ...................................................................... Patch Set 5: (1 comment) File src/soc/intel/skylake/acpi/scs.asl: https://review.coreboot.org/c/coreboot/+/52789/comment/e243d712_4e70dd5f PS5, Line 120: 0x4c4 > That's an interesting thought. My understanding was that with the _ADR > above this "Device" would only be treated as present if it matches an > enumerated PCI device in the OS. My understanding matches what you outlined here. However, I think the device will be enumerated in the OS unless it is power and clock gated before jumping into OS. This is where `ScsSdCardEnabled` seems to play a role. If the mainboard has initialized devicetree correctly to mark SD controller as `off`, then it should be power and clock gated by FSP, thus preventing it from showing up on the PCI bus. > Would the OS try to switch power states of a PCI device that it can't see? My understanding is that if the OS doesn't see the device, then it won't really switch the power states for the device. -- To view, visit https://review.coreboot.org/c/coreboot/+/52789 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id07f59b5252cf3862724d6ff3d7c54d51ccfd9a6 Gerrit-Change-Number: 52789 Gerrit-PatchSet: 5 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Comment-Date: Mon, 03 May 2021 22:39:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: *x86: Support x2apic mode
by Matt DeVillier (Code Review) 03 May '21

03 May '21

Attention is currently required from: Lance Zhao, Shaunak Saha, Nico Huber, Furquan Shaikh, Wonkyu Kim, Ravishankar Sarawadi, Srinidhi N Kaushik, Angel Pons, Subrata Banik. Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52696 ) Change subject: *x86: Support x2apic mode ...................................................................... Patch Set 2: Code-Review+1 (1 comment) Patchset: PS2: fixes booting issues on google/kahlee introduced by CB:51723, but also has the same commit title which is a bit problematic. perhaps 'fix support for x2apic mode' and include that it resolves issues in the previous commit -- To view, visit https://review.coreboot.org/c/coreboot/+/52696 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia28f60a077182c3753f6ba9fbdd141f951d39b37 Gerrit-Change-Number: 52696 Gerrit-PatchSet: 2 Gerrit-Owner: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Lance Zhao Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Reviewer: Shaunak Saha <shaunak.saha(a)intel.com> Gerrit-Reviewer: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.com> Gerrit-Reviewer: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.corp-partner.google.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Lance Zhao Gerrit-Attention: Shaunak Saha <shaunak.saha(a)intel.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Attention: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Attention: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.corp-partner.google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.com> Gerrit-Comment-Date: Mon, 03 May 2021 22:33:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/skylake: Drop EXCLUDE_NATIVE_SD_INTERFACE
by Nico Huber (Code Review) 03 May '21

03 May '21

Attention is currently required from: Felix Singer, Furquan Shaikh, Martin Roth, Tim Wawrzynczak, Angel Pons, Subrata Banik. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52789 ) Change subject: soc/intel/skylake: Drop EXCLUDE_NATIVE_SD_INTERFACE ...................................................................... Patch Set 5: (1 comment) File src/soc/intel/skylake/acpi/scs.asl: https://review.coreboot.org/c/coreboot/+/52789/comment/d35ec6e6_c8c4cb22 PS5, Line 120: 0x4c4 > Did you confirm that the mainboards do not use these GPIOs for some other purpose? Else, it would be […] That's an interesting thought. My understanding was that with the _ADR above this "Device" would only be treated as present if it matches an enumerated PCI device in the OS. The spec calls this an augmented device descriptor. But it doesn't mention what that implies. Would the OS try to switch power states of a PCI device that it can't see? What I could find in the description of _STA: "If a device object describes a device that is not on an enumerable bus and the device object does not have an _STA object, then OSPM assumes that the device is present, enabled, shown in the UI, and functioning." I always interpreted this that we can expect for a PCI device that the OS is supposed to check its presence. But I couldn't find more details about it. -- To view, visit https://review.coreboot.org/c/coreboot/+/52789 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id07f59b5252cf3862724d6ff3d7c54d51ccfd9a6 Gerrit-Change-Number: 52789 Gerrit-PatchSet: 5 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Comment-Date: Mon, 03 May 2021 22:27:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/guybrush: Update power-on timings for PCIe devices
by build bot (Jenkins) (Code Review) 03 May '21

03 May '21

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52868 ) Change subject: mb/google/guybrush: Update power-on timings for PCIe devices ...................................................................... Patch Set 1: (1 comment) File src/mainboard/google/guybrush/bootblock.c: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-118281): https://review.coreboot.org/c/coreboot/+/52868/comment/d1d734ab_47d9db8a PS1, Line 42: void bootblock_mainboard_init(void){ open brace '{' following function definitions go on the next line -- To view, visit https://review.coreboot.org/c/coreboot/+/52868 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2d0b812b654b0cd317a2c8c1ce554e850c96be44 Gerrit-Change-Number: 52868 Gerrit-PatchSet: 1 Gerrit-Owner: Martin Roth <martinroth(a)google.com> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 03 May 2021 22:25:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/guybrush: Update power-on timings for PCIe devices
by Martin Roth (Code Review) 03 May '21

03 May '21

Martin Roth has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/52868 ) Change subject: mb/google/guybrush: Update power-on timings for PCIe devices ...................................................................... mb/google/guybrush: Update power-on timings for PCIe devices This configures the PCIe GPIOs in the correct sequence to meet the power-on timings. WLAN_AUX_RESET is active-high, and the FSP currently only works with active-low reset lines. BUG=b:184796302, b:184598323 TEST=Verify timings between GPIO init sections. All available modules are present after training. Signed-off-by: Martin Roth <martinroth(a)chromium.org> Change-Id: I2d0b812b654b0cd317a2c8c1ce554e850c96be44 --- M src/mainboard/google/guybrush/Makefile.inc M src/mainboard/google/guybrush/bootblock.c M src/mainboard/google/guybrush/port_descriptors.c A src/mainboard/google/guybrush/romstage.c M src/mainboard/google/guybrush/variants/baseboard/Makefile.inc M src/mainboard/google/guybrush/variants/baseboard/gpio.c M src/mainboard/google/guybrush/variants/baseboard/include/baseboard/variants.h 7 files changed, 86 insertions(+), 8 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/68/52868/1 diff --git a/src/mainboard/google/guybrush/Makefile.inc b/src/mainboard/google/guybrush/Makefile.inc index d4eeaf5..ecd031a 100644 --- a/src/mainboard/google/guybrush/Makefile.inc +++ b/src/mainboard/google/guybrush/Makefile.inc @@ -11,6 +11,7 @@ endif romstage-y += port_descriptors.c +romstage-y += romstage.c ramstage-y += mainboard.c ramstage-y += ec.c diff --git a/src/mainboard/google/guybrush/bootblock.c b/src/mainboard/google/guybrush/bootblock.c index 5b18d37..46240bc 100644 --- a/src/mainboard/google/guybrush/bootblock.c +++ b/src/mainboard/google/guybrush/bootblock.c @@ -38,3 +38,15 @@ if (variant_has_fpmcu()) variant_fpmcu_reset(); } + +void bootblock_mainboard_init(void){ + size_t num_gpios; + const struct soc_amd_gpio *gpios; + + /* + * Set GPIOs to bring up power for WWAN & WLAN. This needs to happen + * at the end of bootblock to meet timing requirements. + */ + gpios = variant_bootblock_gpio_table(&num_gpios); + program_gpios(gpios, num_gpios); +} diff --git a/src/mainboard/google/guybrush/port_descriptors.c b/src/mainboard/google/guybrush/port_descriptors.c index cc1fa1a..78801bd 100644 --- a/src/mainboard/google/guybrush/port_descriptors.c +++ b/src/mainboard/google/guybrush/port_descriptors.c @@ -15,7 +15,6 @@ .function_number = 1, .turn_off_unused_lanes = true, .clk_req = CLK_REQ0, - .gpio_group_id = GPIO_29, .port_params = {PP_PSPP_AC, 0x133, PP_PSPP_DC, 0x122} }, { /* SD */ diff --git a/src/mainboard/google/guybrush/romstage.c b/src/mainboard/google/guybrush/romstage.c new file mode 100644 index 0000000..6b3448f --- /dev/null +++ b/src/mainboard/google/guybrush/romstage.c @@ -0,0 +1,17 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +#include <baseboard/variants.h> +#include <fsp/api.h> + +void mb_pre_fspm_init(void) +{ + /* + * Initialize the AUX PCIe reset lines to meet the power-on + * timings. + */ + size_t num_gpios; + const struct soc_amd_gpio *gpios; + + gpios = variant_pcie_gpio_table(&num_gpios); + program_gpios(gpios, num_gpios); +} diff --git a/src/mainboard/google/guybrush/variants/baseboard/Makefile.inc b/src/mainboard/google/guybrush/variants/baseboard/Makefile.inc index 88aa1a4..c8d007d 100644 --- a/src/mainboard/google/guybrush/variants/baseboard/Makefile.inc +++ b/src/mainboard/google/guybrush/variants/baseboard/Makefile.inc @@ -2,6 +2,7 @@ bootblock-y += helpers.c romstage-y += tpm_tis.c +romstage-y += gpio.c ramstage-y += gpio.c ramstage-y += tpm_tis.c diff --git a/src/mainboard/google/guybrush/variants/baseboard/gpio.c b/src/mainboard/google/guybrush/variants/baseboard/gpio.c index 528ace1..a33ed24 100644 --- a/src/mainboard/google/guybrush/variants/baseboard/gpio.c +++ b/src/mainboard/google/guybrush/variants/baseboard/gpio.c @@ -7,6 +7,7 @@ #include <soc/gpio.h> /* GPIO configuration in ramstage*/ +/* Please make sure that *ALL* GPIOs are configured in this table */ static const struct soc_amd_gpio base_gpio_table[] = { /* PWR_BTN_L */ PAD_NF(GPIO_0, PWR_BTN_L, PULL_NONE), @@ -40,7 +41,7 @@ /* SOC_SAR_INT_L */ PAD_INT(GPIO_17, PULL_NONE, EDGE_LOW, STATUS_DELIVERY), /* WWAN_AUX_RESET_L */ - PAD_GPO(GPIO_18, LOW), + PAD_GPO(GPIO_18, HIGH), /* I2C3_SCL */ PAD_NF(GPIO_19, I2C3_SCL, PULL_NONE), /* I2C3_SDA */ @@ -60,7 +61,7 @@ /* PCIE_RST1_L */ PAD_NF(GPIO_27, PCIE_RST1_L, PULL_NONE), /* GPIO_28: Not available */ - /* WLAN_AUX_RESET */ + /* WLAN_AUX_RESET (Active HIGH)*/ PAD_GPO(GPIO_29, LOW), /* ESPI_CS_L */ PAD_NF(GPIO_30, ESPI_CS_L, PULL_NONE), @@ -171,10 +172,7 @@ PAD_GPO(GPIO_6, HIGH), /* EN_PWR_WWAN_X */ PAD_GPO(GPIO_8, HIGH), - /* WWAN_DISABLE */ - PAD_GPO(GPIO_85, LOW), - /* WLAN_DISABLE */ - PAD_GPO(GPIO_130, LOW), + /* GSC_SOC_INT_L */ PAD_INT(GPIO_3, PULL_NONE, EDGE_LOW, STATUS_DELIVERY), /* I2C3_SCL */ @@ -203,11 +201,55 @@ PAD_NF(GPIO_143, UART0_TXD, PULL_NONE), }; +/* + * For the correct power sequencing, there's a minimum of 20ms between Early + * GPIO configuration and bootblock_gpio_table. Currently there's about 23ms + * between the two calls. The first call is within 1ms of the start of bootblock + * and will be moved to psp_verstage when that starts running. The bootblock + * GPIO initialization happens roughly 3.5ms before the end of bootblock. + */ +static const struct soc_amd_gpio bootblock_gpio_table[] = { + /* WWAN_DISABLE */ + PAD_GPO(GPIO_85, LOW), + /* WLAN_DISABLE */ + PAD_GPO(GPIO_130, LOW), +}; + /* GPIO configuration for sleep */ static const struct soc_amd_gpio sleep_gpio_table[] = { /* TODO: Fill sleep gpio configuration */ }; +/* + * The pcie_gpio configuration needs to happen at least 30ms after the + * bootblock_gpio_table initialization and 100ms after the early_gpio_table + * call. Even after verstage is moved to the PSP, this will still happen + * roughly 120ms after bootblock. This includes the time to load romstage + * and FSP-M. + */ +static const struct soc_amd_gpio pcie_gpio_table[] = { + /* WWAN_AUX_RESET_L */ + PAD_GPO(GPIO_18, HIGH), + /* WLAN_AUX_RESET (ACTIVE HIGH) */ + PAD_GPO(GPIO_29, LOW), + /* SSD_AUX_RESET_L */ + PAD_GPO(GPIO_40, HIGH), + /* SD_AUX_RESET_L */ + PAD_GPO(GPIO_70, HIGH), +}; + +const struct soc_amd_gpio *__weak variant_pcie_gpio_table(size_t *size) +{ + *size = ARRAY_SIZE(pcie_gpio_table); + return pcie_gpio_table; +} + +const struct soc_amd_gpio *__weak variant_bootblock_gpio_table(size_t *size) +{ + *size = ARRAY_SIZE(bootblock_gpio_table); + return bootblock_gpio_table; +} + const struct soc_amd_gpio *__weak variant_base_gpio_table(size_t *size) { *size = ARRAY_SIZE(base_gpio_table); diff --git a/src/mainboard/google/guybrush/variants/baseboard/include/baseboard/variants.h b/src/mainboard/google/guybrush/variants/baseboard/include/baseboard/variants.h index 5fb7462..a80d38e 100644 --- a/src/mainboard/google/guybrush/variants/baseboard/include/baseboard/variants.h +++ b/src/mainboard/google/guybrush/variants/baseboard/include/baseboard/variants.h @@ -18,9 +18,15 @@ */ const struct soc_amd_gpio *variant_override_gpio_table(size_t *size); -/* This function provides early GPIO init in bootblock or psp. */ +/* This function provides early GPIO init in early bootblock or psp. */ const struct soc_amd_gpio *variant_early_gpio_table(size_t *size); +/* This function provides GPIO settings at the end of bootblock. */ +const struct soc_amd_gpio *variant_bootblock_gpio_table(size_t *size); + +/* This function provides GPIO settings before PCIe enumeration. */ +const struct soc_amd_gpio *variant_pcie_gpio_table(size_t *size); + /* This function provides GPIO settings before entering sleep. */ const struct soc_amd_gpio *variant_sleep_gpio_table(size_t *size); -- To view, visit https://review.coreboot.org/c/coreboot/+/52868 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2d0b812b654b0cd317a2c8c1ce554e850c96be44 Gerrit-Change-Number: 52868 Gerrit-PatchSet: 1 Gerrit-Owner: Martin Roth <martinroth(a)google.com> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: drivers/intel/fsp2_0: Add timestamps for loading FSPM & FSPS
by Martin Roth (Code Review) 03 May '21

03 May '21

Attention is currently required from: Andrey Petrov, Patrick Rudolph. Martin Roth has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/52867 ) Change subject: drivers/intel/fsp2_0: Add timestamps for loading FSPM & FSPS ...................................................................... drivers/intel/fsp2_0: Add timestamps for loading FSPM & FSPS The loads of the FSPM and FSPS binaries are not insignificant amounts of time, and without these timestamps, it's not clear what's going on in those time blocks. For FSPM, the timestamps can run together to make it look like that time is still part of the romstage init time. BUG=None TEST=Build & Boot guybrush, look at timestamps. Signed-off-by: Martin Roth <martinroth(a)chromium.org> Change-Id: I5796d4cdd512799c2eafee45a8ef561de5258b91 --- M src/commonlib/include/commonlib/timestamp_serialized.h M src/drivers/intel/fsp2_0/memory_init.c M src/drivers/intel/fsp2_0/silicon_init.c 3 files changed, 8 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/52867/1 diff --git a/src/commonlib/include/commonlib/timestamp_serialized.h b/src/commonlib/include/commonlib/timestamp_serialized.h index a572150..f202fbb 100644 --- a/src/commonlib/include/commonlib/timestamp_serialized.h +++ b/src/commonlib/include/commonlib/timestamp_serialized.h @@ -119,6 +119,8 @@ TS_FSP_AFTER_END_OF_FIRMWARE = 961, TS_FSP_MULTI_PHASE_SI_INIT_START = 962, TS_FSP_MULTI_PHASE_SI_INIT_END = 963, + TS_FSP_INITRAM_LOAD = 970, + TS_FSP_SILICON_INIT_LOAD = 971, /* 1000+ reserved for payloads (1000-1200: ChromeOS depthcharge) */ @@ -261,6 +263,10 @@ { TS_FSP_BEFORE_END_OF_FIRMWARE, "calling FspNotify(EndOfFirmware)" }, { TS_FSP_AFTER_END_OF_FIRMWARE, "returning from FspNotify(EndOfFirmware)" }, + + { TS_FSP_INITRAM_LOAD, "Loading FSP-M" }, + { TS_FSP_SILICON_INIT_LOAD, "Loading FSP-S" }, + { TS_START_POSTCAR, "start of postcar" }, { TS_END_POSTCAR, "end of postcar" }, }; diff --git a/src/drivers/intel/fsp2_0/memory_init.c b/src/drivers/intel/fsp2_0/memory_init.c index c5d560a..1d48973 100644 --- a/src/drivers/intel/fsp2_0/memory_init.c +++ b/src/drivers/intel/fsp2_0/memory_init.c @@ -387,6 +387,7 @@ _car_unallocated_start - _car_region_start, 0); memranges_insert(memmap, (uintptr_t)_program, REGION_SIZE(program), 0); + timestamp_add_now(TS_FSP_INITRAM_LOAD); if (fsp_load_component(&fspld, hdr) != CB_SUCCESS) die("FSPM not available or failed to load!\n"); diff --git a/src/drivers/intel/fsp2_0/silicon_init.c b/src/drivers/intel/fsp2_0/silicon_init.c index 707a380..625451b 100644 --- a/src/drivers/intel/fsp2_0/silicon_init.c +++ b/src/drivers/intel/fsp2_0/silicon_init.c @@ -228,6 +228,7 @@ void fsp_silicon_init(void) { + timestamp_add_now(TS_FSP_SILICON_INIT_LOAD); fsps_load(); mb_pre_fsps_init(); -- To view, visit https://review.coreboot.org/c/coreboot/+/52867 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5796d4cdd512799c2eafee45a8ef561de5258b91 Gerrit-Change-Number: 52867 Gerrit-PatchSet: 1 Gerrit-Owner: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newchange

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit May 2021