coreboot-gerrit November 2023

coreboot-gerrit@coreboot.org

1 participants
3037 discussions

[M] Change in coreboot[main]: soc/amd/genoa: add I2C support
by Felix Held (Code Review) 20 Nov '23

20 Nov '23

Attention is currently required from: Varshit Pandya. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78900?usp=email ) Change subject: soc/amd/genoa: add I2C support ...................................................................... Set Ready For Review -- To view, visit https://review.coreboot.org/c/coreboot/+/78900?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: Iebc10de6ea5c6d441cff04e016dcec62405078c3 Gerrit-Change-Number: 78900 Gerrit-PatchSet: 5 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Varshit Pandya <pandyavarshit(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Varshit Pandya <pandyavarshit(a)gmail.com> Gerrit-Comment-Date: Mon, 20 Nov 2023 16:26:23 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: [WIP] soc/amd/genoa: add I2C pad control support
by Felix Held (Code Review) 20 Nov '23

20 Nov '23

Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/79177?usp=email ) Change subject: [WIP] soc/amd/genoa: add I2C pad control support ...................................................................... [WIP] soc/amd/genoa: add I2C pad control support Still need to find out for sure which type of pad control register is used for I2C4 and I2C5. Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: Ibc14a6de60be84460418703472293814e9a51379 --- M src/soc/amd/genoa/Kconfig M src/soc/amd/genoa/chip.h M src/soc/amd/genoa/i2c.c 3 files changed, 14 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/79177/1 diff --git a/src/soc/amd/genoa/Kconfig b/src/soc/amd/genoa/Kconfig index cbf0ddd..bca6ed4 100644 --- a/src/soc/amd/genoa/Kconfig +++ b/src/soc/amd/genoa/Kconfig @@ -21,6 +21,8 @@ select SOC_AMD_COMMON_BLOCK_DATA_FABRIC_EXTENDED_MMIO select SOC_AMD_COMMON_BLOCK_HAS_ESPI select SOC_AMD_COMMON_BLOCK_I2C + select SOC_AMD_COMMON_BLOCK_I2C_PAD_CTRL + select SOC_AMD_COMMON_BLOCK_I23C_PAD_CTRL select SOC_AMD_COMMON_BLOCK_IOMMU select SOC_AMD_COMMON_BLOCK_LPC select SOC_AMD_COMMON_BLOCK_MCAX diff --git a/src/soc/amd/genoa/chip.h b/src/soc/amd/genoa/chip.h index bca40af..5713a72 100644 --- a/src/soc/amd/genoa/chip.h +++ b/src/soc/amd/genoa/chip.h @@ -14,6 +14,7 @@ u8 i2c_scl_reset; struct dw_i2c_bus_config i2c[I2C_CTRLR_COUNT]; + struct i2c_pad_control i2c_pad[I2C_CTRLR_COUNT]; }; #endif diff --git a/src/soc/amd/genoa/i2c.c b/src/soc/amd/genoa/i2c.c index 76c1049..0a1f4fb 100644 --- a/src/soc/amd/genoa/i2c.c +++ b/src/soc/amd/genoa/i2c.c @@ -38,7 +38,17 @@ void soc_i2c_misc_init(unsigned int bus, const struct dw_i2c_bus_config *cfg) { - /* TODO: write I2C pad control registers */ + const struct soc_amd_genoa_config *config = config_of_soc(); + + if (bus >= ARRAY_SIZE(config->i2c_pad)) + return; + + /* The I/O pads of I2C0..3 are the new I23C pads and the I/O pads of I2C4..5 still are + the same I2C pads as in Picasso and Cezanne. TODO: verify if this is true */ + if (bus <= 3) + fch_i23c_pad_init(bus, cfg->speed, &config->i2c_pad[bus]); + else + fch_i2c_pad_init(bus, cfg->speed, &config->i2c_pad[bus]); } const struct soc_i2c_ctrlr_info *soc_get_i2c_ctrlr_info(size_t *num_ctrlrs) -- To view, visit https://review.coreboot.org/c/coreboot/+/79177?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: Ibc14a6de60be84460418703472293814e9a51379 Gerrit-Change-Number: 79177 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newchange

1 0

[S] Change in coreboot[main]: Documentation/mb/asus/p8z77-m: Document latest test results
by Keith Hui (Code Review) 20 Nov '23

20 Nov '23

Attention is currently required from: Angel Pons, Kevin Keijzer, Paul Menzel. Keith Hui has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78204?usp=email ) Change subject: Documentation/mb/asus/p8z77-m: Document latest test results ...................................................................... Patch Set 2: (1 comment) File Documentation/mainboard/asus/p8z77-m.md: https://review.coreboot.org/c/coreboot/+/78204/comment/b88537b0_36d92a69 : PS2, Line 106: It appears all memory modules rated for DDR3-1600 will fail to boot if : max_mem_clock_mhz is set to 800 in devicetree. > No, the *sticky* scratchpad register persists across warm reboots, so they'd work for this purpose. […] Rebase done, but I'm running into problems over there too. Take a look. -- To view, visit https://review.coreboot.org/c/coreboot/+/78204?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: I4f4c9268cd272caa83267be3f71d4a2022c26a1c Gerrit-Change-Number: 78204 Gerrit-PatchSet: 2 Gerrit-Owner: Keith Hui <buurin(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Kevin Keijzer <kevin(a)quietlife.nl> Gerrit-Attention: Kevin Keijzer <kevin(a)quietlife.nl> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 20 Nov 2023 16:09:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Kevin Keijzer <kevin(a)quietlife.nl> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Keith Hui <buurin(a)gmail.com> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: cpu/intel/model_206ax: Program Ivy Bridge defaults for MSR_PKGC_IRTL
by Patrick Rudolph (Code Review) 20 Nov '23

20 Nov '23

Attention is currently required from: Paul Menzel. Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78609?usp=email ) Change subject: cpu/intel/model_206ax: Program Ivy Bridge defaults for MSR_PKGC_IRTL ...................................................................... Patch Set 6: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/78609/comment/20fdf64d_69fc99cb : PS6, Line 12: Test: Lenovo X220 still boots. > Can it somehow be checked, if the correct latencies were programmed, and if they make a difference? It should stay in deeper C-states longer as before, but it's hard to tell if it really does as there are so many things having an influence on the actual time spend in a C-state. -- To view, visit https://review.coreboot.org/c/coreboot/+/78609?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: Ib307e3b191ba68e016cc348f82e2dccf1dc9ae16 Gerrit-Change-Number: 78609 Gerrit-PatchSet: 6 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Mon, 20 Nov 2023 16:04:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: device/pciexp_device.c: Fix setting Max Payload Size
by Patrick Rudolph (Code Review) 20 Nov '23

20 Nov '23

Attention is currently required from: Angel Pons, Arthur Heymans, Felix Held, Krystian Hebel, Michał Żygowski, Nico Huber. Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77338?usp=email ) Change subject: device/pciexp_device.c: Fix setting Max Payload Size ...................................................................... Patch Set 10: (1 comment) File src/device/pciexp_device.c: https://review.coreboot.org/c/coreboot/+/77338/comment/99518b6b_44b48c75 : PS10, Line 709: pciexp_configure_max_payload > I tried the above suggestion and it doesn't work. […] It looks like the parent of a device can be the device itself. ``` /* Walk up the chain and adjust MaxPayload to the minimum supported in the chain */ while (dev->bus && dev->bus->dev && !pciexp_device_is_root_port(dev)) { if (dev->bus->dev == dev || dev->bus->dev->path.type != DEVICE_PATH_PCI) break; /* Adjust Max_Payload_Size */ pciexp_set_max_payload_size(dev->bus->dev, root_cap, dev, cap); dev = dev->bus->dev; }; ``` Should work. -- To view, visit https://review.coreboot.org/c/coreboot/+/77338?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: I24386dc208363b7d94fea46dec25c231a3968225 Gerrit-Change-Number: 77338 Gerrit-PatchSet: 10 Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Angel Pons <angel.pons(a)9elements.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Attention: Angel Pons <angel.pons(a)9elements.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Mon, 20 Nov 2023 15:54:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Michał Żygowski <michal.zygowski(a)3mdeb.com> Comment-In-Reply-To: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: Documentation/mb/asus/p8z77-m: Document latest test results
by Angel Pons (Code Review) 20 Nov '23

20 Nov '23

Attention is currently required from: Keith Hui, Kevin Keijzer, Paul Menzel. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78204?usp=email ) Change subject: Documentation/mb/asus/p8z77-m: Document latest test results ...................................................................... Patch Set 2: (1 comment) File Documentation/mainboard/asus/p8z77-m.md: https://review.coreboot.org/c/coreboot/+/78204/comment/ec943ec7_82096f8e : PS2, Line 106: It appears all memory modules rated for DDR3-1600 will fail to boot if : max_mem_clock_mhz is set to 800 in devicetree. > We need auto downclocking. […] No, the *sticky* scratchpad register persists across warm reboots, so they'd work for this purpose. Actually, I'd avoid using NVRAM - I switch between coreboot and vendor firmware from time to time, and vendor firmware resets its settings if coreboot has modified NVRAM. In theory, channels should be independent, so if one stick works on its own on channel 0 and the other stick works on its own on channel 1, both should be able to work at the same time too. But something seems to be different for dual-channel. If this discussion is going to take time, I suggest that you rebase CB:78205 out of the relation chain so that it can be submitted. -- To view, visit https://review.coreboot.org/c/coreboot/+/78204?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: I4f4c9268cd272caa83267be3f71d4a2022c26a1c Gerrit-Change-Number: 78204 Gerrit-PatchSet: 2 Gerrit-Owner: Keith Hui <buurin(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Kevin Keijzer <kevin(a)quietlife.nl> Gerrit-Attention: Kevin Keijzer <kevin(a)quietlife.nl> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Keith Hui <buurin(a)gmail.com> Gerrit-Comment-Date: Mon, 20 Nov 2023 15:40:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Kevin Keijzer <kevin(a)quietlife.nl> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Keith Hui <buurin(a)gmail.com> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: device/pciexp_device.c: Fix setting Max Payload Size
by Michał Żygowski (Code Review) 20 Nov '23

20 Nov '23

Attention is currently required from: Angel Pons, Arthur Heymans, Felix Held, Krystian Hebel, Nico Huber, Patrick Rudolph. Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77338?usp=email ) Change subject: device/pciexp_device.c: Fix setting Max Payload Size ...................................................................... Patch Set 10: (1 comment) File src/device/pciexp_device.c: https://review.coreboot.org/c/coreboot/+/77338/comment/84fe88d8_3595872e : PS10, Line 709: pciexp_configure_max_payload > instead of adding all of the new code, wouldn't it be simpler to do something like this here: […] I tried the above suggestion and it doesn't work. `dev = dev->bus->dev;` assignment break the whole chain and the pciexp_set_max_payload_size gets called with root and endp being the same devices. -- To view, visit https://review.coreboot.org/c/coreboot/+/77338?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: I24386dc208363b7d94fea46dec25c231a3968225 Gerrit-Change-Number: 77338 Gerrit-PatchSet: 10 Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Angel Pons <angel.pons(a)9elements.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Attention: Angel Pons <angel.pons(a)9elements.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Mon, 20 Nov 2023 15:03:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: nb/intel/sandybridge: Use SA devid to identify PC type
by Felix Held (Code Review) 20 Nov '23

20 Nov '23

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/78826?usp=email ) Change subject: nb/intel/sandybridge: Use SA devid to identify PC type ...................................................................... nb/intel/sandybridge: Use SA devid to identify PC type Instead of using MSR IA32_PLATFORM_ID read the SystemAgent device id to figure out the PC type. This follows the BWG which suggest to not use MSR IA32_PLATFORM_ID for system identification. Tested: Lenovo X220 still boots. Change-Id: Ibddf6c75d15ca7a99758c377ed956d483abe7ec1 Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/78826 Reviewed-by: Lean Sheng Tan <sheng.tan(a)9elements.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/cpu/intel/model_206ax/Makefile.inc D src/cpu/intel/model_206ax/common.c M src/cpu/intel/model_206ax/model_206ax.h M src/cpu/intel/model_206ax/model_206ax_init.c M src/northbridge/intel/sandybridge/common.c M src/northbridge/intel/sandybridge/early_init.c 6 files changed, 15 insertions(+), 29 deletions(-) Approvals: build bot (Jenkins): Verified Lean Sheng Tan: Looks good to me, but someone else must approve Arthur Heymans: Looks good to me, approved diff --git a/src/cpu/intel/model_206ax/Makefile.inc b/src/cpu/intel/model_206ax/Makefile.inc index cd947df..e6e6e37 100644 --- a/src/cpu/intel/model_206ax/Makefile.inc +++ b/src/cpu/intel/model_206ax/Makefile.inc @@ -9,10 +9,6 @@ ramstage-y += acpi.c -ramstage-y += common.c -romstage-y += common.c -smm-y += common.c - smm-y += finalize.c cpu_microcode_bins += $(wildcard 3rdparty/intel-microcode/intel-ucode/06-2a-*) diff --git a/src/cpu/intel/model_206ax/common.c b/src/cpu/intel/model_206ax/common.c deleted file mode 100644 index a881314..0000000 --- a/src/cpu/intel/model_206ax/common.c +++ /dev/null @@ -1,13 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ - -#include <cpu/x86/msr.h> -#include "model_206ax.h" - -int get_platform_id(void) -{ - msr_t msr; - - msr = rdmsr(IA32_PLATFORM_ID); - /* Read Platform Id Bits 52:50 */ - return (msr.hi >> 18) & 0x7; -} diff --git a/src/cpu/intel/model_206ax/model_206ax.h b/src/cpu/intel/model_206ax/model_206ax.h index 9379c29..9a2fefb 100644 --- a/src/cpu/intel/model_206ax/model_206ax.h +++ b/src/cpu/intel/model_206ax/model_206ax.h @@ -131,7 +131,6 @@ /* Configure power limits for turbo mode */ void set_power_limits(u8 power_limit_1_time); int cpu_config_tdp_levels(void); -int get_platform_id(void); static inline u8 cpu_stepping(void) { diff --git a/src/cpu/intel/model_206ax/model_206ax_init.c b/src/cpu/intel/model_206ax/model_206ax_init.c index dfe7ae5..1568372 100644 --- a/src/cpu/intel/model_206ax/model_206ax_init.c +++ b/src/cpu/intel/model_206ax/model_206ax_init.c @@ -323,9 +323,6 @@ fill_processor_name(processor_name); printk(BIOS_INFO, "CPU: %s.\n", processor_name); - /* Print platform ID */ - printk(BIOS_INFO, "CPU: platform id %x\n", get_platform_id()); - /* CPUID and features */ cpu_id = cpu_get_cpuid(); printk(BIOS_INFO, "CPU: cpuid(1) 0x%x\n", cpu_id); diff --git a/src/northbridge/intel/sandybridge/common.c b/src/northbridge/intel/sandybridge/common.c index a8f718c..a670491 100644 --- a/src/northbridge/intel/sandybridge/common.c +++ b/src/northbridge/intel/sandybridge/common.c @@ -2,14 +2,19 @@ #include <console/console.h> #include <device/device.h> -#include <cpu/intel/model_206ax/model_206ax.h> +#include <device/pci_def.h> +#include <device/pci_ops.h> #include "sandybridge.h" enum platform_type get_platform_type(void) { - const int id = get_platform_id(); - if (id != 1 && id != 4) - printk(BIOS_WARNING, "Unknown platform id 0x%x\n", id); - - return (id == 4) ? PLATFORM_MOBILE : PLATFORM_DESKTOP_SERVER; + switch (pci_s_read_config16(HOST_BRIDGE, PCI_DEVICE_ID) & 0xc) { + case 0x0: /* Desktop */ + return PLATFORM_DESKTOP_SERVER; + case 0x4: /* Mobile */ + return PLATFORM_MOBILE; + case 0x8: /* Server */ + default: + return PLATFORM_DESKTOP_SERVER; + } } diff --git a/src/northbridge/intel/sandybridge/early_init.c b/src/northbridge/intel/sandybridge/early_init.c index e3525135..a5ebf9d 100644 --- a/src/northbridge/intel/sandybridge/early_init.c +++ b/src/northbridge/intel/sandybridge/early_init.c @@ -150,14 +150,13 @@ void systemagent_early_init(void) { + const size_t is_mobile = get_platform_type() == PLATFORM_MOBILE; u32 capid0_a; u8 reg8; /* Device ID Override Enable should be done very early */ capid0_a = pci_read_config32(HOST_BRIDGE, CAPID0_A); if (capid0_a & (1 << 10)) { - const size_t is_mobile = get_platform_type() == PLATFORM_MOBILE; - reg8 = pci_read_config8(HOST_BRIDGE, DIDOR); reg8 &= ~7; /* Clear 2:0 */ @@ -167,6 +166,9 @@ pci_write_config8(HOST_BRIDGE, DIDOR, reg8); } + /* Print platform type */ + printk(BIOS_INFO, "Detected system type: %s\n", is_mobile ? "mobile" : "desktop"); + /* Setup all BARs required for early PCIe and raminit */ sandybridge_setup_bars(); -- To view, visit https://review.coreboot.org/c/coreboot/+/78826?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: Ibddf6c75d15ca7a99758c377ed956d483abe7ec1 Gerrit-Change-Number: 78826 Gerrit-PatchSet: 8 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Keith Hui <buurin(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

[XS] Change in coreboot[main]: cpu/intel/model_206ax: Lock MSR_PP_CURRENT_CONFIG
by Felix Held (Code Review) 20 Nov '23

20 Nov '23

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/78608?usp=email ) Change subject: cpu/intel/model_206ax: Lock MSR_PP_CURRENT_CONFIG ...................................................................... cpu/intel/model_206ax: Lock MSR_PP_CURRENT_CONFIG Now that those registers are only written once set the lock bit to protect it from runtime changes. TEST: Lenovo X220 still boots. Change-Id: I4c56a3cb322a0e75eb3dd366808068093928e10c Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/78608 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/cpu/intel/model_206ax/model_206ax.h M src/cpu/intel/model_206ax/model_206ax_init.c 2 files changed, 4 insertions(+), 0 deletions(-) Approvals: Arthur Heymans: Looks good to me, approved build bot (Jenkins): Verified diff --git a/src/cpu/intel/model_206ax/model_206ax.h b/src/cpu/intel/model_206ax/model_206ax.h index ecbf323..9379c29 100644 --- a/src/cpu/intel/model_206ax/model_206ax.h +++ b/src/cpu/intel/model_206ax/model_206ax.h @@ -73,8 +73,10 @@ #define PKG_POWER_LIMIT_TIME_MASK 0x7f #define MSR_PP0_CURRENT_CONFIG 0x601 +#define PP0_CURRENT_LIMIT_LOCK (1U << 31) #define PP0_CURRENT_LIMIT (112 << 3) /* 112 A */ #define MSR_PP1_CURRENT_CONFIG 0x602 +#define PP1_CURRENT_LIMIT_LOCK (1U << 31) #define PP1_CURRENT_LIMIT_SNB (35 << 3) /* 35 A */ #define PP1_CURRENT_LIMIT_IVB (50 << 3) /* 50 A */ #define MSR_PKG_POWER_SKU_UNIT 0x606 diff --git a/src/cpu/intel/model_206ax/model_206ax_init.c b/src/cpu/intel/model_206ax/model_206ax_init.c index 14a7965..dfe7ae5 100644 --- a/src/cpu/intel/model_206ax/model_206ax_init.c +++ b/src/cpu/intel/model_206ax/model_206ax_init.c @@ -206,6 +206,7 @@ msr = rdmsr(MSR_PP0_CURRENT_CONFIG); msr.lo &= ~0x1fff; msr.lo |= PP0_CURRENT_LIMIT; + msr.lo |= PP0_CURRENT_LIMIT_LOCK; wrmsr(MSR_PP0_CURRENT_CONFIG, msr); /* Secondary Plane Current Limit */ @@ -215,6 +216,7 @@ msr.lo |= PP1_CURRENT_LIMIT_IVB; else msr.lo |= PP1_CURRENT_LIMIT_SNB; + msr.lo |= PP1_CURRENT_LIMIT_LOCK; wrmsr(MSR_PP1_CURRENT_CONFIG, msr); } } -- To view, visit https://review.coreboot.org/c/coreboot/+/78608?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: I4c56a3cb322a0e75eb3dd366808068093928e10c Gerrit-Change-Number: 78608 Gerrit-PatchSet: 6 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[M] Change in coreboot[main]: cpu/intel/model_206ax: Write MSRs in scope package only once
by Felix Held (Code Review) 20 Nov '23

20 Nov '23

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/78607?usp=email ) Change subject: cpu/intel/model_206ax: Write MSRs in scope package only once ...................................................................... cpu/intel/model_206ax: Write MSRs in scope package only once Write MSRs that are in scope package only once by checking for the BSP bit. While this improves performance a bit it also has the benefit that registers can be safely locked down without the need for semaphores. TEST: Lenovo X220 still boots. Change-Id: I43f5d62d782466d2796c1df6015d43c0fbf9d031 Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/78607 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/cpu/intel/model_206ax/model_206ax_init.c 1 file changed, 66 insertions(+), 44 deletions(-) Approvals: Arthur Heymans: Looks good to me, approved build bot (Jenkins): Verified diff --git a/src/cpu/intel/model_206ax/model_206ax_init.c b/src/cpu/intel/model_206ax/model_206ax_init.c index 242746e..14a7965 100644 --- a/src/cpu/intel/model_206ax/model_206ax_init.c +++ b/src/cpu/intel/model_206ax/model_206ax_init.c @@ -16,6 +16,7 @@ #include <cpu/intel/smm_reloc.h> #include <cpu/intel/common/common.h> #include <smbios.h> +#include <smp/node.h> #include <types.h> /* Convert time in seconds to POWER_LIMIT_1_TIME MSR value */ @@ -170,45 +171,52 @@ msr.lo |= (1 << 15); // Lock C-State MSR wrmsr(MSR_PKG_CST_CONFIG_CONTROL, msr); - msr = rdmsr(MSR_MISC_PWR_MGMT); - msr.lo &= ~(1 << 0); // Enable P-state HW_ALL coordination - wrmsr(MSR_MISC_PWR_MGMT, msr); + if (boot_cpu()) { + /* + * The following MSRs are in scope 'Package', thus it's sufficient + * to write them once on one core. + */ - msr = rdmsr(MSR_POWER_CTL); - msr.lo |= (1 << 18); // Enable Energy Perf Bias MSR 0x1b0 - msr.lo |= (1 << 1); // C1E Enable - msr.lo |= (1 << 0); // Bi-directional PROCHOT# - wrmsr(MSR_POWER_CTL, msr); + msr = rdmsr(MSR_MISC_PWR_MGMT); + msr.lo &= ~(1 << 0); // Enable P-state HW_ALL coordination + wrmsr(MSR_MISC_PWR_MGMT, msr); - /* C3 Interrupt Response Time Limit */ - msr.hi = 0; - msr.lo = IRTL_VALID | IRTL_1024_NS | 0x50; - wrmsr(MSR_PKGC3_IRTL, msr); + msr = rdmsr(MSR_POWER_CTL); + msr.lo |= (1 << 18); // Enable Energy Perf Bias MSR 0x1b0 + msr.lo |= (1 << 1); // C1E Enable + msr.lo |= (1 << 0); // Bi-directional PROCHOT# + wrmsr(MSR_POWER_CTL, msr); - /* C6 Interrupt Response Time Limit */ - msr.hi = 0; - msr.lo = IRTL_VALID | IRTL_1024_NS | 0x68; - wrmsr(MSR_PKGC6_IRTL, msr); + /* C3 Interrupt Response Time Limit */ + msr.hi = 0; + msr.lo = IRTL_VALID | IRTL_1024_NS | 0x50; + wrmsr(MSR_PKGC3_IRTL, msr); - /* C7 Interrupt Response Time Limit */ - msr.hi = 0; - msr.lo = IRTL_VALID | IRTL_1024_NS | 0x6D; - wrmsr(MSR_PKGC7_IRTL, msr); + /* C6 Interrupt Response Time Limit */ + msr.hi = 0; + msr.lo = IRTL_VALID | IRTL_1024_NS | 0x68; + wrmsr(MSR_PKGC6_IRTL, msr); - /* Primary Plane Current Limit */ - msr = rdmsr(MSR_PP0_CURRENT_CONFIG); - msr.lo &= ~0x1fff; - msr.lo |= PP0_CURRENT_LIMIT; - wrmsr(MSR_PP0_CURRENT_CONFIG, msr); + /* C7 Interrupt Response Time Limit */ + msr.hi = 0; + msr.lo = IRTL_VALID | IRTL_1024_NS | 0x6D; + wrmsr(MSR_PKGC7_IRTL, msr); - /* Secondary Plane Current Limit */ - msr = rdmsr(MSR_PP1_CURRENT_CONFIG); - msr.lo &= ~0x1fff; - if (cpuid_eax(1) >= 0x30600) - msr.lo |= PP1_CURRENT_LIMIT_IVB; - else - msr.lo |= PP1_CURRENT_LIMIT_SNB; - wrmsr(MSR_PP1_CURRENT_CONFIG, msr); + /* Primary Plane Current Limit */ + msr = rdmsr(MSR_PP0_CURRENT_CONFIG); + msr.lo &= ~0x1fff; + msr.lo |= PP0_CURRENT_LIMIT; + wrmsr(MSR_PP0_CURRENT_CONFIG, msr); + + /* Secondary Plane Current Limit */ + msr = rdmsr(MSR_PP1_CURRENT_CONFIG); + msr.lo &= ~0x1fff; + if (cpuid_eax(1) >= 0x30600) + msr.lo |= PP1_CURRENT_LIMIT_IVB; + else + msr.lo |= PP1_CURRENT_LIMIT_SNB; + wrmsr(MSR_PP1_CURRENT_CONFIG, msr); + } } static void configure_thermal_target(struct device *dev) @@ -216,13 +224,20 @@ struct cpu_intel_model_206ax_config *conf = dev->bus->dev->chip_info; msr_t msr; - /* Set TCC activation offset if supported */ - msr = rdmsr(MSR_PLATFORM_INFO); - if ((msr.lo & (1 << 30)) && conf->tcc_offset) { - msr = rdmsr(MSR_TEMPERATURE_TARGET); - msr.lo &= ~(0xf << 24); /* Bits 27:24 */ - msr.lo |= (conf->tcc_offset & 0xf) << 24; - wrmsr(MSR_TEMPERATURE_TARGET, msr); + if (boot_cpu()) { + /* + * The following MSR is in scope 'Package', thus it's sufficient + * to write it once on one core. + */ + + /* Set TCC activation offset if supported */ + msr = rdmsr(MSR_PLATFORM_INFO); + if ((msr.lo & (1 << 30)) && conf->tcc_offset) { + msr = rdmsr(MSR_TEMPERATURE_TARGET); + msr.lo &= ~(0xf << 24); /* Bits 27:24 */ + msr.lo |= (conf->tcc_offset & 0xf) << 24; + wrmsr(MSR_TEMPERATURE_TARGET, msr); + } } } @@ -241,10 +256,17 @@ msr.hi = 0; wrmsr(IA32_THERM_INTERRUPT, msr); - /* Enable package critical interrupt only */ - msr.lo = 1 << 4; - msr.hi = 0; - wrmsr(IA32_PACKAGE_THERM_INTERRUPT, msr); + if (boot_cpu()) { + /* + * The following MSR is in scope 'Package', thus it's sufficient + * to write it once on one core. + */ + + /* Enable package critical interrupt only */ + msr.lo = 1 << 4; + msr.hi = 0; + wrmsr(IA32_PACKAGE_THERM_INTERRUPT, msr); + } } static void set_max_ratio(void) -- To view, visit https://review.coreboot.org/c/coreboot/+/78607?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: I43f5d62d782466d2796c1df6015d43c0fbf9d031 Gerrit-Change-Number: 78607 Gerrit-PatchSet: 6 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Keith Hui <buurin(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2023