coreboot-gerrit June 2024

coreboot-gerrit@coreboot.org

1 participants
3527 discussions

[M] Change in coreboot[main]: mb: source site-local mainboard Kconfig.name and Kconfig files
by Martin L Roth (Code Review) 24 Jun '24

24 Jun '24

Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/83119?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: mb: source site-local mainboard Kconfig.name and Kconfig files ...................................................................... mb: source site-local mainboard Kconfig.name and Kconfig files This allows mainboards in site-local which use the coreboot src/ directory structure to appear in the mainboard list along with the normal mainboards without needing to be symbolically-linked into the vendor's directory. Signed-off-by: Martin Roth <gaumless(a)gmail.com> Change-Id: I4106fea7197c979e6648ebbbbaa107070c916727 --- M src/mainboard/51nb/Kconfig M src/mainboard/acer/Kconfig M src/mainboard/amd/Kconfig M src/mainboard/aoostar/Kconfig M src/mainboard/aopen/Kconfig M src/mainboard/apple/Kconfig M src/mainboard/asrock/Kconfig M src/mainboard/asus/Kconfig M src/mainboard/biostar/Kconfig M src/mainboard/bostentech/Kconfig M src/mainboard/bytedance/Kconfig M src/mainboard/cavium/Kconfig M src/mainboard/clevo/Kconfig M src/mainboard/compulab/Kconfig M src/mainboard/cwwk/Kconfig M src/mainboard/dell/Kconfig M src/mainboard/emulation/Kconfig M src/mainboard/example/Kconfig M src/mainboard/facebook/Kconfig M src/mainboard/foxconn/Kconfig M src/mainboard/framework/Kconfig M src/mainboard/getac/Kconfig M src/mainboard/gigabyte/Kconfig M src/mainboard/google/Kconfig M src/mainboard/hp/Kconfig M src/mainboard/ibase/Kconfig M src/mainboard/ibm/Kconfig M src/mainboard/intel/Kconfig M src/mainboard/inventec/Kconfig M src/mainboard/kontron/Kconfig M src/mainboard/lenovo/Kconfig M src/mainboard/libretrend/Kconfig M src/mainboard/msi/Kconfig M src/mainboard/ocp/Kconfig M src/mainboard/opencellular/Kconfig M src/mainboard/packardbell/Kconfig M src/mainboard/pcengines/Kconfig M src/mainboard/pine64/Kconfig M src/mainboard/portwell/Kconfig M src/mainboard/prodrive/Kconfig M src/mainboard/protectli/Kconfig M src/mainboard/purism/Kconfig M src/mainboard/raptor-cs/Kconfig M src/mainboard/razer/Kconfig M src/mainboard/roda/Kconfig M src/mainboard/samsung/Kconfig M src/mainboard/sapphire/Kconfig M src/mainboard/siemens/Kconfig M src/mainboard/sifive/Kconfig M src/mainboard/starlabs/Kconfig M src/mainboard/supermicro/Kconfig M src/mainboard/system76/Kconfig M src/mainboard/ti/Kconfig M src/mainboard/up/Kconfig 54 files changed, 108 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/19/83119/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/83119?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I4106fea7197c979e6648ebbbbaa107070c916727 Gerrit-Change-Number: 83119 Gerrit-PatchSet: 2 Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Varshit Pandya <pandyavarshit(a)gmail.com>

1 0

[S] Change in coreboot[main]: src/lib: Add Kconfig variable for fw_config default value
by Jérémy Compostella (Code Review) 24 Jun '24

24 Jun '24

Attention is currently required from: Anil Kumar K, Bora Guvendik, Cliff Huang, Subrata Banik, Wonkyu Kim. Jérémy Compostella has posted comments on this change by Cliff Huang. ( https://review.coreboot.org/c/coreboot/+/83153?usp=email ) Change subject: src/lib: Add Kconfig variable for fw_config default value ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/83153?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I9694433e01bdcd9ff1e550585c14ea7ccee020a3 Gerrit-Change-Number: 83153 Gerrit-PatchSet: 2 Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Comment-Date: Mon, 24 Jun 2024 17:51:37 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[S] Change in coreboot[main]: src/lib: Add Kconfig variable for fw_config default value
by Cliff Huang (Code Review) 24 Jun '24

24 Jun '24

Attention is currently required from: Anil Kumar K, Bora Guvendik, Subrata Banik, Wonkyu Kim. Hello Anil Kumar K, Bora Guvendik, Jérémy Compostella, Subrata Banik, Wonkyu Kim, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/83153?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Code-Review+1 by Anil Kumar K, Verified+1 by build bot (Jenkins) Change subject: src/lib: Add Kconfig variable for fw_config default value ...................................................................... src/lib: Add Kconfig variable for fw_config default value Set CONFIG_FW_CONFIG_DEFAULT_VALUE in config.<board> file. This default value will be used when it is not able to read via Chrome EC, CBFS or VPD. TEST=boot and check fw_config value from coreboot log Signed-off-by: Cliff Huang <cliff.huang(a)intel.com> Change-Id: I9694433e01bdcd9ff1e550585c14ea7ccee020a3 --- M src/Kconfig M src/lib/fw_config.c 2 files changed, 12 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/53/83153/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/83153?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I9694433e01bdcd9ff1e550585c14ea7ccee020a3 Gerrit-Change-Number: 83153 Gerrit-PatchSet: 2 Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>

1 0

[S] Change in coreboot[main]: src/lib: Add Kconfig variable for fw_config default value
by Cliff Huang (Code Review) 24 Jun '24

24 Jun '24

Attention is currently required from: Bora Guvendik, Subrata Banik, Wonkyu Kim. Cliff Huang has posted comments on this change by Cliff Huang. ( https://review.coreboot.org/c/coreboot/+/83153?usp=email ) Change subject: src/lib: Add Kconfig variable for fw_config default value ...................................................................... Patch Set 1: (1 comment) File src/Kconfig: https://review.coreboot.org/c/coreboot/+/83153/comment/b81afae3_6f398b6f?us… : PS1, Line 548: 0x01000 > Is there any reason why this value is no-zero value? okay. undefined value is non-zero, I will change default value to '0' ./src/include/fw_config.h: #define UNDEFINED_FW_CONFIG ~((uint64_t)0) -- To view, visit https://review.coreboot.org/c/coreboot/+/83153?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I9694433e01bdcd9ff1e550585c14ea7ccee020a3 Gerrit-Change-Number: 83153 Gerrit-PatchSet: 1 Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Comment-Date: Mon, 24 Jun 2024 17:41:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Wonkyu Kim <wonkyu.kim(a)intel.com>

1 0

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

24 Jun '24

Attention is currently required from: Bob Moragues, Karthik Ramasubramanian, Shelley Chen. Bob Moragues has posted comments on this change by Bob Moragues. ( https://review.coreboot.org/c/coreboot/+/82630?usp=email ) Change subject: mainboard/google/brox: Add FW_CONFIG and SKU definitions to support lotso ...................................................................... Patch Set 3: (2 comments) File src/mainboard/google/brox/variants/brox/overridetree.cb: https://review.coreboot.org/c/coreboot/+/82630/comment/e64e7a6e_d24a9864?us… : PS3, Line 19: option AUDIO_REALTEK_ALC3287 2 > I don't think that you want this in brox. […] I believe this was included to simply keep the the bit fields aligned between the brox reference design and lotso. I assume that this enumeration will not be used for brox. This can be removed at your discretion. File src/mainboard/google/brox/variants/lotso/overridetree.cb: https://review.coreboot.org/c/coreboot/+/82630/comment/55e55b9a_19dc6b30?us… : PS1, Line 31: hip soc/intel/alderlake : : device domain 0 on : end : : end Should this be removed? -- To view, visit https://review.coreboot.org/c/coreboot/+/82630?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I52cf42b79eff91ab2b4e98a7b5961310e60f2ea7 Gerrit-Change-Number: 82630 Gerrit-PatchSet: 3 Gerrit-Owner: Bob Moragues <moragues(a)chromium.org> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Bob Moragues <moragues(a)google.com> Gerrit-Attention: Shelley Chen <shchen(a)google.com> Gerrit-Attention: Bob Moragues <moragues(a)chromium.org> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Mon, 24 Jun 2024 17:40:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Shelley Chen <shchen(a)google.com>

1 0

[S] Change in coreboot[main]: src/lib: Add Kconfig variable for fw_config default value
by Jérémy Compostella (Code Review) 24 Jun '24

24 Jun '24

Attention is currently required from: Bora Guvendik, Cliff Huang, Subrata Banik, Wonkyu Kim. Jérémy Compostella has posted comments on this change by Cliff Huang. ( https://review.coreboot.org/c/coreboot/+/83153?usp=email ) Change subject: src/lib: Add Kconfig variable for fw_config default value ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: I am also very curious why isn't the default value 0 ? -- To view, visit https://review.coreboot.org/c/coreboot/+/83153?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I9694433e01bdcd9ff1e550585c14ea7ccee020a3 Gerrit-Change-Number: 83153 Gerrit-PatchSet: 1 Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Comment-Date: Mon, 24 Jun 2024 17:28:52 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[S] Change in coreboot[main]: src/lib: Add Kconfig variable for fw_config default value
by Wonkyu Kim (Code Review) 24 Jun '24

24 Jun '24

Attention is currently required from: Bora Guvendik, Cliff Huang, Jérémy Compostella, Subrata Banik. Wonkyu Kim has posted comments on this change by Cliff Huang. ( https://review.coreboot.org/c/coreboot/+/83153?usp=email ) Change subject: src/lib: Add Kconfig variable for fw_config default value ...................................................................... Patch Set 1: (1 comment) File src/Kconfig: https://review.coreboot.org/c/coreboot/+/83153/comment/3a8a0b6c_4066d01a?us… : PS1, Line 548: 0x01000 Is there any reason why this value is no-zero value? -- To view, visit https://review.coreboot.org/c/coreboot/+/83153?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I9694433e01bdcd9ff1e550585c14ea7ccee020a3 Gerrit-Change-Number: 83153 Gerrit-PatchSet: 1 Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Comment-Date: Mon, 24 Jun 2024 17:16:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[S] Change in coreboot[main]: src/lib: Add Kconfig variable for fw_config default value
by Anil Kumar K (Code Review) 24 Jun '24

24 Jun '24

Attention is currently required from: Bora Guvendik, Cliff Huang, Jérémy Compostella, Subrata Banik, Wonkyu Kim. Anil Kumar K has posted comments on this change by Cliff Huang. ( https://review.coreboot.org/c/coreboot/+/83153?usp=email ) Change subject: src/lib: Add Kconfig variable for fw_config default value ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/83153?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I9694433e01bdcd9ff1e550585c14ea7ccee020a3 Gerrit-Change-Number: 83153 Gerrit-PatchSet: 1 Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Comment-Date: Mon, 24 Jun 2024 17:14:56 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[S] Change in coreboot[main]: acpi/acpi: Fix Qemu's XSDT patching code
by Felix Held (Code Review) 24 Jun '24

24 Jun '24

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/83116?usp=email ) ( 3 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: acpi/acpi: Fix Qemu's XSDT patching code ...................................................................... acpi/acpi: Fix Qemu's XSDT patching code Since Qemu doesn't provide an XSDT, coreboot adds one as separate ACPI table. Qemu only provides the smaller ACPI 1.0 RSDP, but the XSDT can only fit into the bigger ACPI 2.0 RSDP. Currently the exsting RSDP is being reused, without a size check, which works fine on the first boot. However after reboot the XSDT pointer seems to be valid, even though the checksum isn't. Since the XSDT then isn't reserved again on reboot, the memory it's pointing to is reused by other tables, causing the payload/OS to see an invalid XSDT. Instead of corrupting the smaller existing RSDP, allocate a new RSDP structure and properly fill it with both, existing RSDT and XSDT. In addition return the correct length of allocated ACPI tables to the calling code. It was ommiting the size of the allocated XSDT and SSDT. TEST: Run "qemu-system-x86_64 -M q35" and reboot the virtual machine. With this patch applied XSDT is always valid from the OS point of view. Change-Id: Ie4972230c3654714f3dcbaab46a3f70152e75163 Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/83116 Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-by: Arthur Heymans <arthur(a)aheymans.xyz> Reviewed-by: Paul Menzel <paulepanter(a)mailbox.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/acpi/acpi.c 1 file changed, 11 insertions(+), 2 deletions(-) Approvals: Angel Pons: Looks good to me, approved Paul Menzel: Looks good to me, but someone else must approve Arthur Heymans: Looks good to me, approved build bot (Jenkins): Verified diff --git a/src/acpi/acpi.c b/src/acpi/acpi.c index 39eadc3..06aa038c 100644 --- a/src/acpi/acpi.c +++ b/src/acpi/acpi.c @@ -1489,6 +1489,16 @@ current = fw; current = acpi_align_current(current); if (rsdp->xsdt_address == 0) { + acpi_rsdt_t *existing_rsdt = (acpi_rsdt_t *)(uintptr_t)rsdp->rsdt_address; + + /* + * Qemu only provides a smaller ACPI 1.0 RSDP, thus + * allocate a bigger ACPI 2.0 RSDP structure. + */ + rsdp = (acpi_rsdp_t *)current; + current += sizeof(acpi_rsdp_t); + coreboot_rsdp = (uintptr_t)rsdp; + xsdt = (acpi_xsdt_t *)current; current += sizeof(acpi_xsdt_t); current = acpi_align_current(current); @@ -1497,7 +1507,6 @@ * Qemu only creates an RSDT. * Add an XSDT based on the existing RSDT entries. */ - acpi_rsdt_t *existing_rsdt = (acpi_rsdt_t *)(uintptr_t)rsdp->rsdt_address; acpi_write_rsdp(rsdp, existing_rsdt, xsdt, oem_id); acpi_write_xsdt(xsdt, oem_id, oem_table_id); /* @@ -1537,7 +1546,7 @@ acpi_add_table(rsdp, ssdt); - return fw; + return current; } dsdt_file = cbfs_map(CONFIG_CBFS_PREFIX "/dsdt.aml", &dsdt_size); -- To view, visit https://review.coreboot.org/c/coreboot/+/83116?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ie4972230c3654714f3dcbaab46a3f70152e75163 Gerrit-Change-Number: 83116 Gerrit-PatchSet: 5 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: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

[M] Change in coreboot[main]: mb/google/brox: Add support for batteryless booting
by Shelley Chen (Code Review) 24 Jun '24

24 Jun '24

Shelley Chen has submitted this change. ( https://review.coreboot.org/c/coreboot/+/83087?usp=email ) Change subject: mb/google/brox: Add support for batteryless booting ...................................................................... mb/google/brox: Add support for batteryless booting Set PsysPL2 and PsysPL3 in addition to making adjustments to PL2 and PL4 in order to prevent brownouts when we don't have a battery or have an empty battery at boot time. BUG=b:335046538,b:329722827 BRANCH=None TEST=flash Able to successfully boot on a SKU1 with 45W, 60W+ adapters and SKU2 with a 60W or higher type C adapter. 30W is still being worked on. Change-Id: Ie36f16b2c938dce29cd2130a86fc8c08f5ba0902 Signed-off-by: Shelley Chen <shchen(a)google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/83087 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Karthik Ramasubramanian <kramasub(a)google.com> --- M src/mainboard/google/brox/variants/baseboard/brox/ramstage.c M src/mainboard/google/brox/variants/brox/overridetree.cb M src/mainboard/google/brox/variants/brox/ramstage.c 3 files changed, 188 insertions(+), 33 deletions(-) Approvals: Karthik Ramasubramanian: Looks good to me, approved build bot (Jenkins): Verified diff --git a/src/mainboard/google/brox/variants/baseboard/brox/ramstage.c b/src/mainboard/google/brox/variants/baseboard/brox/ramstage.c index 7853863..be1ba08 100644 --- a/src/mainboard/google/brox/variants/baseboard/brox/ramstage.c +++ b/src/mainboard/google/brox/variants/baseboard/brox/ramstage.c @@ -5,44 +5,163 @@ #include <console/console.h> #include <device/pci_ops.h> #include <drivers/intel/dptf/chip.h> +#include <ec/google/chromeec/ec.h> #include <intelblocks/power_limit.h> #include <soc/pci_devs.h> WEAK_DEV_PTR(dptf_policy); +#define SET_PSYSPL2(e, w) ((e) * (w) / 100) +#define SET_PL2(e, w) ((e - 27) * (w) / 100) + +static bool get_sku_index(const struct cpu_power_limits *limits, size_t num_entries, + size_t *intel_idx, size_t *brox_idx) +{ + uint16_t mchid = pci_s_read_config16(PCI_DEV(0, 0, 0), PCI_DEVICE_ID); + u8 tdp = get_cpu_tdp(); + size_t i = 0; + + for (i = 0; i < ARRAY_SIZE(cpuid_to_adl); i++) { + if (mchid == cpuid_to_adl[i].cpu_id && tdp == cpuid_to_adl[i].cpu_tdp) { + *intel_idx = cpuid_to_adl[i].limits; + break; + } + } + + if (i == ARRAY_SIZE(cpuid_to_adl)) { + printk(BIOS_ERR, "Cannot find correct intel sku index (mchid = %u).\n", mchid); + return false; + } + + for (i = 0; i < num_entries; i++) { + if (mchid == limits[i].mchid && tdp == limits[i].cpu_tdp) { + *brox_idx = i; + break; + } + } + + if (i == num_entries) { + printk(BIOS_ERR, "Cannot find correct brox sku index (mchid = %u).\n", mchid); + return false; + } + + return true; +} + void variant_update_power_limits(const struct cpu_power_limits *limits, size_t num_entries) { + const struct device *policy_dev; + size_t intel_idx, brox_idx; + struct drivers_intel_dptf_config *config; + struct dptf_power_limits *settings; + config_t *conf; + struct soc_power_limits_config *soc_config; + if (!num_entries) return; - const struct device *policy_dev = DEV_PTR(dptf_policy); + policy_dev = DEV_PTR(dptf_policy); if (!policy_dev) return; - struct drivers_intel_dptf_config *config = policy_dev->chip_info; + if (!get_sku_index(limits, num_entries, &intel_idx, &brox_idx)) + return; - uint16_t mchid = pci_s_read_config16(PCI_DEV(0, 0, 0), PCI_DEVICE_ID); + config = policy_dev->chip_info; + settings = &config->controls.power_limits; + conf = config_of_soc(); + soc_config = &conf->power_limits_config[intel_idx]; + settings->pl1.min_power = limits[brox_idx].pl1_min_power; + settings->pl1.max_power = limits[brox_idx].pl1_max_power; + settings->pl2.min_power = limits[brox_idx].pl2_min_power; + settings->pl2.max_power = limits[brox_idx].pl2_max_power; - u8 tdp = get_cpu_tdp(); - - for (size_t i = 0; i < num_entries; i++) { - if (mchid == limits[i].mchid && tdp == limits[i].cpu_tdp) { - struct dptf_power_limits *settings = &config->controls.power_limits; - config_t *conf = config_of_soc(); - struct soc_power_limits_config *soc_config = conf->power_limits_config; - settings->pl1.min_power = limits[i].pl1_min_power; - settings->pl1.max_power = limits[i].pl1_max_power; - settings->pl2.min_power = limits[i].pl2_min_power; - settings->pl2.max_power = limits[i].pl2_max_power; - soc_config->tdp_pl4 = DIV_ROUND_UP(limits[i].pl4_power, - MILLIWATTS_TO_WATTS); - printk(BIOS_INFO, "Overriding power limits PL1 (%u, %u) PL2 (%u, %u) PL4 (%u)\n", - limits[i].pl1_min_power, - limits[i].pl1_max_power, - limits[i].pl2_min_power, - limits[i].pl2_max_power, - limits[i].pl4_power); - } + if (soc_config->tdp_pl2_override != 0) { + settings->pl2.max_power = soc_config->tdp_pl2_override * 1000; + settings->pl2.min_power = settings->pl2.max_power; } + + if (soc_config->tdp_pl4 == 0) + soc_config->tdp_pl4 = DIV_ROUND_UP(limits[brox_idx].pl4_power, + MILLIWATTS_TO_WATTS); +} + +/* + * Psys calculations + * + * We use the following: + * + * For USB C charger (Max Power): + * +-------------+-----+------+---------+-------+ + * | Max Power(W)| TDP | PL2 | PsysPL2 | PL3/4 | + * +-------------+-----+------+---------+-------+ + * | 30 | 15 | 17 | 25 | 25 | <--- not working yet + * | 45 | 15 | 26 | 38 | 38 | + * | 60 | 15 | 35 | 51 | 51 | + * | 110 | 15 | 55 | 94 | 96 | + * +-------------+-----+------+---------+-------+ + */ +void variant_update_psys_power_limits(const struct cpu_power_limits *limits, + const struct system_power_limits *sys_limits, + size_t num_entries, + const struct psys_config *config_psys) +{ + struct soc_power_limits_config *soc_config; + size_t intel_idx, brox_idx; + u16 volts_mv, current_ma; + enum usb_chg_type type; + u32 pl2; + u32 psyspl2 = 0; + u32 psyspl3 = 0; + u32 pl2_default; + config_t *conf; + u32 watts = 0; + int rv; + + if (!num_entries) + return; + + if (!get_sku_index(limits, num_entries, &intel_idx, &brox_idx)) + return; + + conf = config_of_soc(); + soc_config = &conf->power_limits_config[intel_idx]; + + pl2_default = DIV_ROUND_UP(limits[brox_idx].pl2_max_power, MILLIWATTS_TO_WATTS); + + /* Get AC adapter power */ + rv = google_chromeec_get_usb_pd_power_info(&type, &current_ma, &volts_mv); + + if (rv == 0 && type == USB_CHG_TYPE_PD) { + /* Detected USB-PD. Get max value of adapter */ + watts = ((u32)current_ma * volts_mv) / 1000000; + } + /* If battery is present and has enough charge, add discharge rate */ + if (CONFIG(EC_GOOGLE_CHROMEEC) && google_chromeec_is_battery_present_and_above_critical_threshold()) { + watts += 65; + } + + /* We did not detect a battery or a Type-C charger */ + if (watts == 0) { + return; + } + + /* set psyspl2 to efficiency% of adapter rating */ + psyspl2 = SET_PSYSPL2(config_psys->efficiency, watts); + psyspl3 = psyspl2; + if (watts > 60) + psyspl3 += 2; + + /* Limit PL2 if the adapter is with lower capability */ + pl2 = (psyspl2 > pl2_default) ? pl2_default : SET_PL2(config_psys->efficiency, watts); + + /* If PL4 > psyspl3, lower it */ + if (soc_config->tdp_pl4 > psyspl3) + soc_config->tdp_pl4 = psyspl3; + + /* now that we're done calculating, set everything */ + soc_config->tdp_pl2_override = pl2; + soc_config->tdp_psyspl2 = psyspl2; + soc_config->tdp_psyspl3 = psyspl3; } diff --git a/src/mainboard/google/brox/variants/brox/overridetree.cb b/src/mainboard/google/brox/variants/brox/overridetree.cb index 424d61c..21774f8 100644 --- a/src/mainboard/google/brox/variants/brox/overridetree.cb +++ b/src/mainboard/google/brox/variants/brox/overridetree.cb @@ -133,7 +133,7 @@ register "options.fan.fine_grained_control" = "1" register "options.fan.step_size" = "2" - device generic 0 on end + device generic 0 alias dptf_policy on end end end # DTT device ref igpu on diff --git a/src/mainboard/google/brox/variants/brox/ramstage.c b/src/mainboard/google/brox/variants/brox/ramstage.c index b468320..86418da 100644 --- a/src/mainboard/google/brox/variants/brox/ramstage.c +++ b/src/mainboard/google/brox/variants/brox/ramstage.c @@ -2,17 +2,53 @@ #include <baseboard/variants.h> #include <device/pci_ids.h> +#include <ec/google/chromeec/ec.h> +#include <intelblocks/power_limit.h> -const struct cpu_power_limits limits[] = { - /* SKU_ID, TDP (Watts), pl1_min, pl1_max, pl2_min, pl2_max, pl4 */ - /* All values are for performance config as per document #686872 */ - { PCI_DID_INTEL_RPL_P_ID_1, 45, 18000, 45000, 115000, 115000, 210000 }, - { PCI_DID_INTEL_RPL_P_ID_2, 28, 10000, 28000, 64000, 64000, 126000 }, - { PCI_DID_INTEL_RPL_P_ID_3, 15, 6000, 15000, 55000, 55000, 114000 }, +/* + * SKU_ID, TDP (Watts), pl1_min (milliWatts), pl1_max (milliWatts), + * pl2_min (milliWatts), pl2_max (milliWatts), pl4 (milliWatts) + * Following values are for performance config as per document #640982 + */ + +const struct cpu_power_limits performance_efficient_limits[] = { + { + .mchid = PCI_DID_INTEL_RPL_P_ID_3, + .cpu_tdp = 15, + .pl1_min_power = 6000, + .pl1_max_power = 15000, + .pl2_min_power = 55000, + .pl2_max_power = 55000, + .pl4_power = 114000 + }, + { + .mchid = PCI_DID_INTEL_RPL_P_ID_4, + .cpu_tdp = 15, + .pl1_min_power = 6000, + .pl1_max_power = 15000, + .pl2_min_power = 55000, + .pl2_max_power = 55000, + .pl4_power = 114000 + }, }; -void variant_devtree_update(void) +const struct system_power_limits sys_limits[] = { + /* SKU_ID, TDP (Watts), psys_pl2 (Watts) */ + { PCI_DID_INTEL_RPL_P_ID_3, 15, 60 }, + { PCI_DID_INTEL_RPL_P_ID_4, 15, 60 }, +}; + +const struct psys_config psys_config = { + .efficiency = 86, +}; + +void __weak variant_devtree_update(void) { - size_t total_entries = ARRAY_SIZE(limits); - variant_update_power_limits(limits, total_entries); + printk(BIOS_DEBUG, "WEAK: %s/%s called\n", __FILE__, __func__); + + const struct cpu_power_limits *limits = performance_efficient_limits; + size_t limits_size = ARRAY_SIZE(performance_efficient_limits); + + variant_update_power_limits(limits, limits_size); + variant_update_psys_power_limits(limits, sys_limits, limits_size, &psys_config); } -- To view, visit https://review.coreboot.org/c/coreboot/+/83087?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ie36f16b2c938dce29cd2130a86fc8c08f5ba0902 Gerrit-Change-Number: 83087 Gerrit-PatchSet: 6 Gerrit-Owner: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: Deepti Deshatty <deepti.deshatty(a)intel.corp-partner.google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit June 2024