coreboot-gerrit October 2023

coreboot-gerrit@coreboot.org

1 participants
3354 discussions

[S] Change in coreboot[main]: {commonlib, libpayload}: Add "has_external display" in coreboot table
by Tim Van Patten (Code Review) 03 Oct '23

03 Oct '23

Attention is currently required from: Andrey Petrov, Arthur Heymans, Julius Werner, Karthik Ramasubramanian, Nick Vaccaro, Subrata Banik, Yu-Ping Wu. Tim Van Patten has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77796?usp=email ) Change subject: {commonlib, libpayload}: Add "has_external display" in coreboot table ...................................................................... Patch Set 5: (2 comments) File payloads/libpayload/include/coreboot_tables.h: https://review.coreboot.org/c/coreboot/+/77796/comment/a903f448_07b17b08 : PS5, Line 224: uint8_t Should these be `u8` as well, to be consistent with the other structs in this file? https://review.coreboot.org/c/coreboot/+/77796/comment/9a6a44ac_36730dd1 : PS5, Line 246: struct cb_framebuffer_flags flags; Similar to `lb_framebuffer`, I suggest adding a `u8` padding here as well, to make it clearer where the next field should go to prevent any holes in this struct. -- To view, visit https://review.coreboot.org/c/coreboot/+/77796?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I0fa7eee4c5a50371a7a66c6ca1ac2c7d046d010b Gerrit-Change-Number: 77796 Gerrit-PatchSet: 5 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-CC: Tim Van Patten <timvp(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Tue, 03 Oct 2023 16:44:40 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: cpu/intel/model_206ax: Only use supported C-states
by Patrick Rudolph (Code Review) 03 Oct '23

03 Oct '23

Attention is currently required from: Kyösti Mälkki, Paul Menzel. Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78194?usp=email ) Change subject: cpu/intel/model_206ax: Only use supported C-states ...................................................................... Patch Set 3: (2 comments) File src/cpu/intel/model_206ax/acpi.c: https://review.coreboot.org/c/coreboot/+/78194/comment/0b6e3864_1382529c : PS2, Line 106: acpi_cstate_map[count].ctype = i + 1; > I don't see the .ctype field filled anymore. Good catch. Fixed. https://review.coreboot.org/c/coreboot/+/78194/comment/020ca888_3ec0689f : PS2, Line 97: uint8_t h, l; > state, substate ? If you changed the previous function on Felix's request. Done -- To view, visit https://review.coreboot.org/c/coreboot/+/78194?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Iaaee050e0ce3c29c12e97f5819a29f485a7946c2 Gerrit-Change-Number: 78194 Gerrit-PatchSet: 3 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Comment-Date: Tue, 03 Oct 2023 16:44:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: cpu/intel/model_206ax: Print supported C-states
by Patrick Rudolph (Code Review) 03 Oct '23

03 Oct '23

Attention is currently required from: Felix Held, Kyösti Mälkki. Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78133?usp=email ) Change subject: cpu/intel/model_206ax: Print supported C-states ...................................................................... Patch Set 6: (7 comments) File src/arch/x86/cpu_common.c: https://review.coreboot.org/c/coreboot/+/78133/comment/74904756_29946a39 : PS5, Line 196: int cpu_get_c_state_support(const int lvl) > Is the input parameter 'lvl' really a 'state'. […] Moved to separate commit. https://review.coreboot.org/c/coreboot/+/78133/comment/1f384839_f762b51e : PS5, Line 201: 1 > might be good to have a define for this bit Done File src/cpu/intel/model_206ax/acpi.c: https://review.coreboot.org/c/coreboot/+/78133/comment/d1f78068_cbe5f1eb : PS5, Line 103: h, l > it would make the code easier to read if you'd use 'state' and 'substate' as variable names Done https://review.coreboot.org/c/coreboot/+/78133/comment/d6b6a6b2_6bd18fa9 : PS5, Line 104: > maybe add a comment that C0 is always supported; was wondering why i was initialized as 1, but that […] yes, added a comment to clarify. https://review.coreboot.org/c/coreboot/+/78133/comment/8132fb79_55fde0dc : PS5, Line 109: > l > i'd drop this or make it != 0 and drop the + 1 in line 106 too; at least for me that would make the […] That would not result in the same behaviour. cpu_get_c_state_support doesn't return a boolean, but an integer of supported sub-levels. Also the ACPI resource holds a C state starting at C1, so +1 must be added to get the correct CPU C-state. https://review.coreboot.org/c/coreboot/+/78133/comment/8ad8f63c_37e22b37 : PS5, Line 110: strcpy(&str[strlen(str)], c_state_names[i]); : strcpy(&str[strlen(str)], " "); > i'd just use printk directly instead of assembling a string and then printing it. […] I ported the code from MPinit, which runs in parallel and made a mess when using printk, but here it's fine. https://review.coreboot.org/c/coreboot/+/78133/comment/de684a05_dc51bc55 : PS5, Line 128: print_supported_cstates(); > While there is a call to cpu_get_c_state_support() with cpuid() calls, ACPI table creation only runs […] Sounds good. Will do on the next push. -- To view, visit https://review.coreboot.org/c/coreboot/+/78133?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I713712a1a104714cbf3091782e564e7e784cf21d Gerrit-Change-Number: 78133 Gerrit-PatchSet: 6 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Tue, 03 Oct 2023 16:43:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: mb/google/rex: Configure ISH UART TX/RX as NC
by Bernardo Perez Priego (Code Review) 03 Oct '23

03 Oct '23

Attention is currently required from: Bernardo Perez Priego, Cliff Huang, Eran Mitrani, Jakub Czapiga, Kapil Porwal, Li1 Feng, Subrata Banik, Tarun. Hello Cliff Huang, Eran Mitrani, Jakub Czapiga, Kapil Porwal, Li1 Feng, Subrata Banik, Tarun, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/78049?usp=email to look at the new patch set (#4). The following approvals got outdated and were removed: Code-Review+1 by Bernardo Perez Priego, Code-Review+1 by Li1 Feng, Code-Review+2 by Subrata Banik, Verified+1 by build bot (Jenkins) The change is no longer submittable: Code-Review and Verified are unsatisfied now. Change subject: mb/google/rex: Configure ISH UART TX/RX as NC ...................................................................... mb/google/rex: Configure ISH UART TX/RX as NC This patch reverses ISH UART pin configuration to allow ISH to enter into suspend mode. This UART port is for debugging purposes. BUG=b:302612549 TEST=On Google/rex platform with ISH enabled, do suspend_stress_test This test must pass Signed-off-by: Bernardo Perez Priego <bernardo.perez.priego(a)intel.com> Change-Id: I8aba45420744a3990e1f9637c3b31ea2e0f78f87 --- M src/mainboard/google/rex/variants/rex0/fw_config.c 1 file changed, 0 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/49/78049/4 -- To view, visit https://review.coreboot.org/c/coreboot/+/78049?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I8aba45420744a3990e1f9637c3b31ea2e0f78f87 Gerrit-Change-Number: 78049 Gerrit-PatchSet: 4 Gerrit-Owner: Bernardo Perez Priego <bernardo.perez.priego(a)intel.com> Gerrit-Reviewer: Bernardo Perez Priego <bernardo.perez.priego(a)intel.com> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Li1 Feng <li1.feng(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Tanu Malhotra <tanu.malhotra(a)intel.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Li1 Feng <li1.feng(a)intel.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Bernardo Perez Priego <bernardo.perez.priego(a)intel.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

[XS] Change in coreboot[main]: sb/intel/bd82x6x/pch: Mark static devices hidden
by Patrick Rudolph (Code Review) 03 Oct '23

03 Oct '23

Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/78230?usp=email ) Change subject: sb/intel/bd82x6x/pch: Mark static devices hidden ...................................................................... sb/intel/bd82x6x/pch: Mark static devices hidden When hiding static devices mark them as hidden so the PCI enumeration no longer complains about them being missing. Test: PCI: Disabled southbridge devices no longer appear in "Leftover static devices:" log. Change-Id: Iae70072a85b62a456102190a5f72f4d652ad6d5a Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> --- M src/southbridge/intel/bd82x6x/pch.c 1 file changed, 4 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/30/78230/1 diff --git a/src/southbridge/intel/bd82x6x/pch.c b/src/southbridge/intel/bd82x6x/pch.c index c9e66de..b523f63 100644 --- a/src/southbridge/intel/bd82x6x/pch.c +++ b/src/southbridge/intel/bd82x6x/pch.c @@ -131,6 +131,10 @@ /* Set bit in function disable register to hide this device */ static void pch_hide_devfn(unsigned int devfn) { + struct device *dev = pcidev_path_on_root(devfn); + if (dev) + dev->hidden = true; + switch (devfn) { case PCI_DEVFN(20, 0): /* xHCI */ if (pch_silicon_type() == PCH_TYPE_PPT) { -- To view, visit https://review.coreboot.org/c/coreboot/+/78230?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Iae70072a85b62a456102190a5f72f4d652ad6d5a Gerrit-Change-Number: 78230 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-MessageType: newchange

1 0

[XS] Change in coreboot[main]: sb/intel/bd82x6x: Warn about slow PCIe downstream devices
by Patrick Rudolph (Code Review) 03 Oct '23

03 Oct '23

Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/78229?usp=email ) Change subject: sb/intel/bd82x6x: Warn about slow PCIe downstream devices ...................................................................... sb/intel/bd82x6x: Warn about slow PCIe downstream devices Warn when a device took longer than usual to appear. Use the PDS bit to detect if a root port has a downstream device connected and warn if enumeration failed. Test: On Lenovo X220 all PCIe device are visible, thus the added code path is never taken. Change-Id: I86b498b89d672b239d9951e116dc3680030666a6 Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> --- M src/southbridge/intel/bd82x6x/pcie.c 1 file changed, 4 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/29/78229/1 diff --git a/src/southbridge/intel/bd82x6x/pcie.c b/src/southbridge/intel/bd82x6x/pcie.c index 59ef2cb..f1817ba 100644 --- a/src/southbridge/intel/bd82x6x/pcie.c +++ b/src/southbridge/intel/bd82x6x/pcie.c @@ -234,6 +234,10 @@ /* Normal PCIe Scan */ pciexp_scan_bridge(dev); } + if ((pci_read_config32(dev, D28Fx_SLSTS) & PDS) && + !dev_is_active_bridge(dev)) + printk(BIOS_WARNING, "%s: Has a slow downstream device. Enumeration failed.\n", + dev_path(dev)); /* Late Power Management init after bridge device enumeration */ pch_pcie_pm_late(dev); -- To view, visit https://review.coreboot.org/c/coreboot/+/78229?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I86b498b89d672b239d9951e116dc3680030666a6 Gerrit-Change-Number: 78229 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-MessageType: newchange

1 0

[S] Change in coreboot[main]: sb/intel/bd82x6x: Improve SLCAP
by Patrick Rudolph (Code Review) 03 Oct '23

03 Oct '23

Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/78228?usp=email ) Change subject: sb/intel/bd82x6x: Improve SLCAP ...................................................................... sb/intel/bd82x6x: Improve SLCAP - Set the 'Hotplug Capable' bit and 'Hot Plug Surprise' bit in SLCAP for hotplugable PCIe slots. - Assign unique slot number. Test: System still boots and all PCIe devices are working. Change-Id: I03aeb0a1ff0041901acc20fe700d3f7995d22366 Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> --- M src/southbridge/intel/bd82x6x/pch.h M src/southbridge/intel/bd82x6x/pcie.c 2 files changed, 9 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/28/78228/1 diff --git a/src/southbridge/intel/bd82x6x/pch.h b/src/southbridge/intel/bd82x6x/pch.h index 727204f..44c723d 100644 --- a/src/southbridge/intel/bd82x6x/pch.h +++ b/src/southbridge/intel/bd82x6x/pch.h @@ -87,6 +87,7 @@ # define SI (1 << 8) #define D28Fx_LCTL 0x50 # define LD (1 << 4) +#define D28Fx_SLCAP 0x54 #define D28Fx_SLSTS 0x58 # define PDS (1 << 22) diff --git a/src/southbridge/intel/bd82x6x/pcie.c b/src/southbridge/intel/bd82x6x/pcie.c index 206894d..59ef2cb 100644 --- a/src/southbridge/intel/bd82x6x/pcie.c +++ b/src/southbridge/intel/bd82x6x/pcie.c @@ -43,6 +43,7 @@ { u16 link_width_p0, link_width_p4; u8 slot_power_limit = 10; /* 10W for x1 */ + static u8 slot_number; u32 reg32; u8 reg8; @@ -137,11 +138,16 @@ pci_write_config32(dev, 0x4c, reg32); /* Set slot power limit as configured above */ - reg32 = pci_read_config32(dev, 0x54); + reg32 = pci_read_config32(dev, D28Fx_SLCAP); + reg32 &= ~(3 << 5); + if (pci_is_hotplugable(dev)) + reg32 |= (3 << 5); reg32 &= ~((1 << 15) | (1 << 16)); /* 16:15 = Slot power scale */ reg32 &= ~(0xff << 7); /* 14:7 = Slot power limit */ reg32 |= (slot_power_limit << 7); - pci_write_config32(dev, 0x54, reg32); + reg32 &= ~(0x1fff << 19); + reg32 |= (slot_number++ << 19); + pci_write_config32(dev, D28Fx_SLCAP, reg32); } static void pch_pcie_pm_late(struct device *dev) -- To view, visit https://review.coreboot.org/c/coreboot/+/78228?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I03aeb0a1ff0041901acc20fe700d3f7995d22366 Gerrit-Change-Number: 78228 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-MessageType: newchange

1 0

[S] Change in coreboot[main]: sb/intel/bd82x6x: Use helper for PCIe hotplug
by Patrick Rudolph (Code Review) 03 Oct '23

03 Oct '23

Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/78227?usp=email ) Change subject: sb/intel/bd82x6x: Use helper for PCIe hotplug ...................................................................... sb/intel/bd82x6x: Use helper for PCIe hotplug Introduce pci_is_hotplugable is helper to find hotpluggable PCIe devices. Test: PCI express slot is still marked as the only hotpluggable PCIe root port. Change-Id: I25aae540ff2ffa3ec5b93ed9caa838b4e50048d2 Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> --- M src/southbridge/intel/bd82x6x/pcie.c 1 file changed, 9 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/27/78227/1 diff --git a/src/southbridge/intel/bd82x6x/pcie.c b/src/southbridge/intel/bd82x6x/pcie.c index 6f082e8..206894d 100644 --- a/src/southbridge/intel/bd82x6x/pcie.c +++ b/src/southbridge/intel/bd82x6x/pcie.c @@ -32,6 +32,13 @@ return NULL; } +static bool pci_is_hotplugable(struct device *dev) +{ + struct southbridge_intel_bd82x6x_config *config = dev->chip_info; + + return config && config->pcie_hotplug_map[PCI_FUNC(dev->path.pci.devfn)]; +} + static void pch_pcie_pm_early(struct device *dev) { u16 link_width_p0, link_width_p4; @@ -179,7 +186,6 @@ static void pci_init(struct device *dev) { u16 reg16; - struct southbridge_intel_bd82x6x_config *config = dev->chip_info; printk(BIOS_DEBUG, "Initializing PCH PCIe bridge.\n"); @@ -202,7 +208,7 @@ pci_write_config16(dev, 0x1e, reg16); /* Enable expresscard hotplug events. */ - if (config->pcie_hotplug_map[PCI_FUNC(dev->path.pci.devfn)]) { + if (pci_is_hotplugable(dev)) { pci_or_config32(dev, 0xd8, 1 << 30); pci_write_config16(dev, 0x42, 0x142); } @@ -216,9 +222,7 @@ static void pch_pciexp_scan_bridge(struct device *dev) { - struct southbridge_intel_bd82x6x_config *config = dev->chip_info; - - if (CONFIG(PCIEXP_HOTPLUG) && config->pcie_hotplug_map[PCI_FUNC(dev->path.pci.devfn)]) { + if (CONFIG(PCIEXP_HOTPLUG) && pci_is_hotplugable(dev)) { pciexp_hotplug_scan_bridge(dev); } else { /* Normal PCIe Scan */ -- To view, visit https://review.coreboot.org/c/coreboot/+/78227?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I25aae540ff2ffa3ec5b93ed9caa838b4e50048d2 Gerrit-Change-Number: 78227 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-MessageType: newchange

1 0

[S] Change in coreboot[main]: sb/intel/bd82x6x: Disable unused PCIe root ports
by Patrick Rudolph (Code Review) 03 Oct '23

03 Oct '23

Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/78226?usp=email ) Change subject: sb/intel/bd82x6x: Disable unused PCIe root ports ...................................................................... sb/intel/bd82x6x: Disable unused PCIe root ports Follow the PCH BIOS spec more closely by porting the broadwell and braswell PCIe downstream device detection. - Set "Slot Implemented" bit. - Read SLSTS BIT22 to detect connected downstream devices as done on braswell. - Disable unused PCIe slots that are not hotplugable. - Set BIT26 in register 0x338 and wait for bits in register 0x328 to clear as done on broadwell. Test: Tested on Lenovo X220. Unused root ports are disabled and port that are in used or marked hot-plug are kept enabled. TODO: Test on more devices and measure powersavings. Change-Id: I8ccfcab2e0e4faba8322755a4f8c2108d9b007ac Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> --- M src/southbridge/intel/bd82x6x/pch.c M src/southbridge/intel/bd82x6x/pch.h 2 files changed, 43 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/26/78226/1 diff --git a/src/southbridge/intel/bd82x6x/pch.c b/src/southbridge/intel/bd82x6x/pch.c index ae8ed9b..c9e66de 100644 --- a/src/southbridge/intel/bd82x6x/pch.c +++ b/src/southbridge/intel/bd82x6x/pch.c @@ -6,6 +6,7 @@ #include <device/pci.h> #include <device/pci_ops.h> #include <string.h> +#include <timer.h> #include "chip.h" #include "pch.h" @@ -296,6 +297,39 @@ sizeof(new_hotplug_map)); } +static void check_device_present(struct device *dev) +{ + struct southbridge_intel_bd82x6x_config *config = dev->chip_info; + + struct stopwatch timeout; + bool present, hot_plugable; + + /* Set slot implemented. */ + pci_or_config16(dev, D28Fx_XCAP, SI); + + hot_plugable = config && config->pcie_hotplug_map[PCI_FUNC(dev->path.pci.devfn)]; + present = !!(pci_read_config32(dev, D28Fx_SLSTS) & PDS); + + printk(BIOS_DEBUG, "%s: Found %s downstream device on %shot pluggable port\n", + dev_path(dev), present ? "a" : "no", hot_plugable ? "" : "non "); + + if (!present && !hot_plugable) { + /* No device present. */ + stopwatch_init_usecs_expire(&timeout, 50 * 1000); + pci_or_config32(dev, 0x338, 1 << 26); + + while (!stopwatch_expired(&timeout)) { + if ((pci_read_config32(dev, 0x328) & (0x1f << 23)) == 0) + break; + udelay(100); + } + dev->enabled = 0; + } else if (present && !hot_plugable && !dev->enabled) { + /* Port is disabled, but device present. Disable link. */ + pci_or_config32(dev, D28Fx_LCTL, LD); + } +} + /* Special handling for PCIe Root Port devices */ static void pch_pcie_enable(struct device *dev) { @@ -304,6 +338,8 @@ if (!config) return; + check_device_present(dev); + /* * Save a copy of the Root Port Function Number map when * starting to walk the list of PCIe Root Ports so it can diff --git a/src/southbridge/intel/bd82x6x/pch.h b/src/southbridge/intel/bd82x6x/pch.h index 6d164ea..727204f 100644 --- a/src/southbridge/intel/bd82x6x/pch.h +++ b/src/southbridge/intel/bd82x6x/pch.h @@ -83,6 +83,13 @@ #define PCH_PCIE_DEV_SLOT 28 #define PCH_PCIE_DEV(_func) PCI_DEV(0, PCH_PCIE_DEV_SLOT, _func) +#define D28Fx_XCAP 0x42 +# define SI (1 << 8) +#define D28Fx_LCTL 0x50 +# define LD (1 << 4) +#define D28Fx_SLSTS 0x58 +# define PDS (1 << 22) + /* PCI Configuration Space (D20:F0): xHCI */ #define PCH_XHCI_DEV PCI_DEV(0, 0x14, 0) -- To view, visit https://review.coreboot.org/c/coreboot/+/78226?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I8ccfcab2e0e4faba8322755a4f8c2108d9b007ac Gerrit-Change-Number: 78226 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-MessageType: newchange

1 0

[XS] Change in coreboot[main]: sb/intel/bd82x6x/early_pch: Follow PCH BIOS spec
by Patrick Rudolph (Code Review) 03 Oct '23

03 Oct '23

Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/78225?usp=email ) Change subject: sb/intel/bd82x6x/early_pch: Follow PCH BIOS spec ...................................................................... sb/intel/bd82x6x/early_pch: Follow PCH BIOS spec PCH BIOS spec says that BIOS must clear BIT26 in register 0x338 in PEI, as done on lynxpoint. Copy and adapt the lynxpoint code to do the same on bd82x6x. Test: System still boots and all PCIe ports are fully working. Change-Id: I865818c0c22194fffcb2bbdf8c43737b0dce2307 Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> --- M src/southbridge/intel/bd82x6x/early_pch.c M src/southbridge/intel/bd82x6x/pch.h 2 files changed, 7 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/25/78225/1 diff --git a/src/southbridge/intel/bd82x6x/early_pch.c b/src/southbridge/intel/bd82x6x/early_pch.c index e4e002d..e5b0f3d 100644 --- a/src/southbridge/intel/bd82x6x/early_pch.c +++ b/src/southbridge/intel/bd82x6x/early_pch.c @@ -137,6 +137,9 @@ { pci_write_config8(SOUTHBRIDGE, 0xa6, pci_read_config8(SOUTHBRIDGE, 0xa6) | 2); + for (uint8_t i = 0; i < 8; i++) + pci_update_config32(PCH_PCIE_DEV(i), 0x338, ~(1 << 26), 0); + RCBA32(CIR1) = 0x00109000; RCBA32(REC); // !!! = 0x00000000 RCBA32(REC) = 0x40000000; diff --git a/src/southbridge/intel/bd82x6x/pch.h b/src/southbridge/intel/bd82x6x/pch.h index 8face06..6d164ea 100644 --- a/src/southbridge/intel/bd82x6x/pch.h +++ b/src/southbridge/intel/bd82x6x/pch.h @@ -74,12 +74,15 @@ #define PCH_EHCI2_DEV PCI_DEV(0, 0x1a, 0) #define PCH_ME_DEV PCI_DEV(0, 0x16, 0) #define PCH_THERMAL_DEV PCI_DEV(0, 0x1f, 6) -#define PCH_PCIE_DEV_SLOT 28 #define PCH_IOAPIC_PCI_BUS 250 #define PCH_IOAPIC_PCI_SLOT 31 #define PCH_HPET_PCI_BUS 250 #define PCH_HPET_PCI_SLOT 15 +/* PCI Configuration Space (D28:F0): PCI2PCI */ +#define PCH_PCIE_DEV_SLOT 28 +#define PCH_PCIE_DEV(_func) PCI_DEV(0, PCH_PCIE_DEV_SLOT, _func) + /* PCI Configuration Space (D20:F0): xHCI */ #define PCH_XHCI_DEV PCI_DEV(0, 0x14, 0) -- To view, visit https://review.coreboot.org/c/coreboot/+/78225?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I865818c0c22194fffcb2bbdf8c43737b0dce2307 Gerrit-Change-Number: 78225 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-MessageType: newchange

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit October 2023