coreboot-gerrit April 2023

coreboot-gerrit@coreboot.org

1 participants
5761 discussions

[S] Change in coreboot[master]: mb/facebook/fbg1701/board_mboot.h: Remove config from mb_log_list
by Frans Hendriks (Code Review) 25 Apr '23

25 Apr '23

Attention is currently required from: Erik van den Bogaert, Frans Hendriks. Hello Erik van den Bogaert, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/74750 to look at the new patch set (#4). Change subject: mb/facebook/fbg1701/board_mboot.h: Remove config from mb_log_list ...................................................................... mb/facebook/fbg1701/board_mboot.h: Remove config from mb_log_list Error message ´Cannot map compressed file config without cbfs_cache' is reported. Compressed parts of CBFS can not be used during bootblock stage. Remove config from mb_log_list. Will be added to verified items by CB:74752. BUG=NA TEST=booting and verify log on facebook FBG1701 Change-Id: Iacf023bc8b9c2ebc66137c4ea683589751a30d2f Signed-off-by: Frans Hendriks <fhendriks(a)eltan.com> --- M src/mainboard/facebook/fbg1701/board_mboot.h 1 file changed, 19 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/50/74750/4 -- To view, visit https://review.coreboot.org/c/coreboot/+/74750 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iacf023bc8b9c2ebc66137c4ea683589751a30d2f Gerrit-Change-Number: 74750 Gerrit-PatchSet: 4 Gerrit-Owner: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Erik van den Bogaert <ebogaert(a)eltan.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Erik van den Bogaert <ebogaert(a)eltan.com> Gerrit-Attention: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-MessageType: newpatchset

1 0

[S] Change in coreboot[master]: drivers/smmstore/ramstage.c: retry smmstore init up 5 times
by Kyösti Mälkki (Code Review) 25 Apr '23

25 Apr '23

Attention is currently required from: Sean Rhodes, Michał Żygowski, Patrick Rudolph, Paul Menzel, Michał Kopeć, Krystian Hebel. Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/73981 ) Change subject: drivers/smmstore/ramstage.c: retry smmstore init up 5 times ...................................................................... Patch Set 1: (1 comment) File src/drivers/smmstore/ramstage.c: https://review.coreboot.org/c/coreboot/+/73981/comment/96e0dbb7_86f2b538 PS1, Line 79: } while (retry--); > I'm afraid this can't be done, even `mdelay()` should be removed from the code. […] Maybe CB:74741 raise_io_smi() would work here. Sounds like the polling should not be hidden under drivers/smmstore. There's no retry of outb() yet, just the noop/pause part. I wonder what the argumentation was to repeat the outb() ? -- To view, visit https://review.coreboot.org/c/coreboot/+/73981 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8929af25c4f69873bbdd835fde5cb60fc324b6ab Gerrit-Change-Number: 73981 Gerrit-PatchSet: 1 Gerrit-Owner: Michał Kopeć <michal.kopec(a)3mdeb.com> Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-CC: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Michał Kopeć <michal.kopec(a)3mdeb.com> Gerrit-Attention: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Comment-Date: Tue, 25 Apr 2023 13:05:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Comment-In-Reply-To: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: Documentation/mainboard/hp: Add more about internal flashing
by Felix Held (Code Review) 25 Apr '23

25 Apr '23

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/74248 ) Change subject: Documentation/mainboard/hp: Add more about internal flashing ...................................................................... Documentation/mainboard/hp: Add more about internal flashing Add a more detailed explanation of internal flashing on the HP Compaq 8200 Elite SFF. Signed-off-by: Václav Straka <venda.straka(a)gmail.com> Change-Id: I53a697a2dd6c10fff8f287284f75d229c7c4b636 Reviewed-on: https://review.coreboot.org/c/coreboot/+/74248 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Nicholas Chin <nic.c3.14(a)gmail.com> --- M Documentation/mainboard/hp/compaq_8200_sff.md A Documentation/mainboard/hp/compaq_8200_sff_jumper.jpg 2 files changed, 105 insertions(+), 20 deletions(-) Approvals: build bot (Jenkins): Verified Nicholas Chin: Looks good to me, approved diff --git a/Documentation/mainboard/hp/compaq_8200_sff.md b/Documentation/mainboard/hp/compaq_8200_sff.md index 3e83e25..72df9e3 100644 --- a/Documentation/mainboard/hp/compaq_8200_sff.md +++ b/Documentation/mainboard/hp/compaq_8200_sff.md @@ -14,30 +14,99 @@ ## Flashing coreboot ```eval_rst -+---------------------+------------+ -| Type | Value | -+=====================+============+ -| Socketed flash | no | -+---------------------+------------+ -| Model | MX25L6406E | -+---------------------+------------+ -| Size | 8 MiB | -+---------------------+------------+ -| In circuit flashing | yes | -+---------------------+------------+ -| Package | SOIC-8 | -+---------------------+------------+ -| Write protection | No | -+---------------------+------------+ -| Dual BIOS feature | No | -+---------------------+------------+ -| Internal flashing | yes | -+---------------------+------------+ ++---------------------+-------------------------+ +| Type | Value | ++=====================+=========================+ +| Socketed flash | no | ++---------------------+-------------------------+ +| Model | MX25L6406E/MX25L6408E | ++---------------------+-------------------------+ +| Size | 8 MiB | ++---------------------+-------------------------+ +| In circuit flashing | yes | ++---------------------+-------------------------+ +| Package | SOIC-8 | ++---------------------+-------------------------+ +| Write protection | bios region | ++---------------------+-------------------------+ +| Dual BIOS feature | No | ++---------------------+-------------------------+ +| Internal flashing | yes | ++---------------------+-------------------------+ +``` + +### Flash layout +The original layout of the flash should look like this: +``` +00000000:00000fff fd +00510000:007fffff bios +00003000:0050ffff me +00001000:00002fff gbe ``` ### Internal programming The SPI flash can be accessed using [flashrom]. +```console +$ flashrom -p internal -c MX25L6406E/MX25L6408E -w coreboot.rom +``` + +After shorting the FDO jumper you gain access to the full flash, but you +still cannot write in the bios region due to SPI protected ranges. + +**Position of FDO jumper close to the IO and second fan connector** +![][compaq_8200_jumper] + +[compaq_8200_jumper]: compaq_8200_sff_jumper.jpg + +To write to the bios region you can use an [IFD Hack] originally developed +for MacBooks, but with modified values described in this guide. +You should read both guides before attempting the procedure. + +Since you can still write in the flash descriptor, you can shrink +the ME and then move the bios region into where the ME originally was. +coreboot does not by default restrict writing to any part of the flash, so +you will first flash a small coreboot build and after it boots, flash +the full one. + +The temporary flash layout with the neutered ME firmware should look like this: +``` +00000000:00000fff fd +00023000:001fffff bios +00003000:00022fff me +00001000:00002fff gbe +00200000:007fffff pd +``` + +It is very important to use these exact numbers or you will need to fix it +using external flashing, but you should already be familiar with the risks +if you got this far. + +The temporary ROM chip size to set in menuconfig is 2 MB but the default +CBFS size is too large for that, you can use up to about 0x1D0000. + +When building both the temporary and the permanent installation, don't forget +to also add the gigabit ethernet configuration when adding the flash descriptor +and ME firmware. + +You can pad the ROM to the required 8MB with zeros using: +```console +$ dd if=/dev/zero of=6M.bin bs=1024 count=6144 +$ cat coreboot.rom 6M.bin > coreboot8.rom +``` + +If you want to continue using the neutered ME firmware use this flash layout +for stage 2: +``` +00000000:00000fff fd +00023000:007fffff bios +00003000:00022fff me +00001000:00002fff gbe +``` + +If you want to use the original ME firmware use the original flash layout. + +More about flashing internally and getting the flash layout [here](../../tutorial/flashing_firmware/index.md). ### External programming @@ -74,7 +143,7 @@ | Coprocessor | Intel ME | +------------------+--------------------------------------------------+ ``` - +[IFD Hack]: https://review.coreboot.org/plugins/gitiles/coreboot/+/refs/changes/70/3877… [Compaq 8200 Elite SFF]: https://support.hp.com/us-en/document/c03414707 [HP]: https://www.hp.com/ [flashrom]: https://flashrom.org/Flashrom diff --git a/Documentation/mainboard/hp/compaq_8200_sff_jumper.jpg b/Documentation/mainboard/hp/compaq_8200_sff_jumper.jpg new file mode 100644 index 0000000..ba30746 --- /dev/null +++ b/Documentation/mainboard/hp/compaq_8200_sff_jumper.jpg Binary files differ -- To view, visit https://review.coreboot.org/c/coreboot/+/74248 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I53a697a2dd6c10fff8f287284f75d229c7c4b636 Gerrit-Change-Number: 74248 Gerrit-PatchSet: 11 Gerrit-Owner: Václav Straka Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[M] Change in coreboot[master]: [WIP] cpu/x86/smm: Poll for SMI delivery completion
by build bot (Jenkins) (Code Review) 25 Apr '23

25 Apr '23

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74741 ) Change subject: [WIP] cpu/x86/smm: Poll for SMI delivery completion ...................................................................... Patch Set 1: (3 comments) Commit Message: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-174917): https://review.coreboot.org/c/coreboot/+/74741/comment/b1977296_1fdfef6a PS1, Line 6: Possible long commit subject (prefer a maximum 65 characters) File src/cpu/x86/smi_trigger.c: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-174917): https://review.coreboot.org/c/coreboot/+/74741/comment/f17c943f_0d010658 PS1, Line 52: " mov %0, %%eax\n\t" please, no space before tabs Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-174917): https://review.coreboot.org/c/coreboot/+/74741/comment/e6a62f7d_ab081f8a PS1, Line 53: " outb %%al, %3\n\t" please, no space before tabs -- To view, visit https://review.coreboot.org/c/coreboot/+/74741 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ifc3cc23b8ec492291521319f0a2de44a45b39520 Gerrit-Change-Number: 74741 Gerrit-PatchSet: 1 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 25 Apr 2023 13:00:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: [WIP] cpu/x86/smm: Poll for SMI delivery completion
by Kyösti Mälkki (Code Review) 25 Apr '23

25 Apr '23

Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/74741 ) Change subject: [WIP] cpu/x86/smm: Poll for SMI delivery completion ...................................................................... [WIP] cpu/x86/smm: Poll for SMI delivery completion Change-Id: Ifc3cc23b8ec492291521319f0a2de44a45b39520 Signed-off-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com> --- M src/cpu/x86/smi_trigger.c M src/include/cpu/x86/smm.h 2 files changed, 61 insertions(+), 9 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/41/74741/1 diff --git a/src/cpu/x86/smi_trigger.c b/src/cpu/x86/smi_trigger.c index 470ecde..cafb121 100644 --- a/src/cpu/x86/smi_trigger.c +++ b/src/cpu/x86/smi_trigger.c @@ -5,9 +5,11 @@ #include <cpu/x86/smm.h> #include <stdint.h> -static void apmc_log(const char *fn, u8 cmd) +#define TIMEOUT_COUNT 100 + +static void prepare_smi_entry(const char *fn, const u32 cmd, const u32 params, int *timeout) { - switch (cmd) { + switch (cmd & 0xff) { case APM_CNT_ACPI_DISABLE: printk(BIOS_DEBUG, "%s: Disabling ACPI.\n", fn); break; @@ -18,37 +20,76 @@ printk(BIOS_DEBUG, "%s: Finalizing SMM.\n", fn); break; case APM_CNT_ELOG_GSMI: + *timeout = TIMEOUT_COUNT; break; case APM_CNT_SMMSTORE: + *timeout = TIMEOUT_COUNT; break; case APM_CNT_SMMINFO: break; default: - printk(BIOS_DEBUG, "%s: Unknown APMC 0x%02x.\n", fn, cmd); + printk(BIOS_ERR, "%s: Unknown APMC (eax = 0x%08x, ebx = 0x%08x)\n", + fn, cmd, params); break; } } -int apm_control(u8 cmd) +/* SMI handler is entered with commands in %eax, params in %ebx. */ +int raise_io_smi(u32 cmd, const u32 params) { + const u16 io_port = APM_CNT; + int set_timeout = 0, retries; + /* Never proceed inside SMI handler or without one. */ if (ENV_SMM || !CONFIG(HAVE_SMI_HANDLER)) return -1; - apmc_log(__func__, cmd); + prepare_smi_entry(__func__, cmd, params, &set_timeout); + retries = set_timeout; /* Now raise the SMI. */ - outb(cmd, APM_CNT); + __asm__ __volatile__ ( + " mov %0, %%eax\n\t" + " outb %%al, %3\n\t" + "1: cmp %%eax, %0\n\t" + " jne 3f\n\t" + " test %1, %1\n\t" + " jz 3f\n\t" + "2: pause\n\t" + " dec %1\n\t" + " jmp 1b\n\t" + "3:\n\t" + : "+r" (cmd), "+r" (retries) + : "b" (params), "d" (io_port) + : "%eax", "memory"); - printk(BIOS_DEBUG, "APMC done.\n"); + if (!set_timeout) { + printk(BIOS_DEBUG, "APMC done.\n"); + return 0; + } + + if (cmd & 0xff) { + printk(BIOS_ERR, "APMC timed out (%d latency).\n", set_timeout - retries); + return -1; + } + + printk(BIOS_DEBUG, "APMC done (%d latency).\n", set_timeout - retries); return 0; } +void apm_control(u8 cmd) +{ + /* Now raise the SMI. */ + raise_io_smi(cmd, 0); +} + u8 apm_get_apmc(void) { + int timeout_dummy; + /* Emulate B2 register as the FADT / Linux expects it */ u8 cmd = inb(APM_CNT); - apmc_log("SMI#", cmd); + prepare_smi_entry("SMI#", cmd, 0, &timeout_dummy); return cmd; } diff --git a/src/include/cpu/x86/smm.h b/src/include/cpu/x86/smm.h index 4c97f21..667b17a 100644 --- a/src/include/cpu/x86/smm.h +++ b/src/include/cpu/x86/smm.h @@ -34,7 +34,8 @@ #define SMM_PCI_RESOURCE_STORE_NUM_RESOURCES 6 /* Send cmd to APM_CNT with HAVE_SMI_HANDLER checking. */ -int apm_control(u8 cmd); +int raise_io_smi(u32 cmd, const u32 params); +void apm_control(u8 cmd); u8 apm_get_apmc(void); void io_trap_handler(int smif); -- To view, visit https://review.coreboot.org/c/coreboot/+/74741 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ifc3cc23b8ec492291521319f0a2de44a45b39520 Gerrit-Change-Number: 74741 Gerrit-PatchSet: 1 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-MessageType: newchange

1 0

[S] Change in coreboot[master]: soc/amd/common/block: Add missing cbfs_unmap().
by Grzegorz Bernacki (Code Review) 25 Apr '23

25 Apr '23

Attention is currently required from: Jason Glenesk, Matt DeVillier, Tim Van Patten, Fred Reitberger, Karthik Ramasubramanian, Felix Held. Grzegorz Bernacki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74716 ) Change subject: soc/amd/common/block: Add missing cbfs_unmap(). ...................................................................... Patch Set 1: (1 comment) File src/soc/amd/common/block/cpu/update_microcode.c: https://review.coreboot.org/c/coreboot/+/74716/comment/93ba50b3_17ba89c8 PS1, Line 89: cache_valid > Unrelated to these changes, but since you're in here... […] Yes, we can remove `cache_valid` and even remove `ucode_cache`. It has the same value as `ucode`. There is a bug in the patch as I should unmap `ucode_cache` not `ucode` -- To view, visit https://review.coreboot.org/c/coreboot/+/74716 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ibbebe7401e9f5a5312da1216408bf42fa2449db1 Gerrit-Change-Number: 74716 Gerrit-PatchSet: 1 Gerrit-Owner: Grzegorz Bernacki Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Konrad Adamczyk <konrada(a)google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Tim Van Patten <timvp(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Tim Van Patten <timvp(a)google.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Tue, 25 Apr 2023 11:53:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tim Van Patten <timvp(a)google.com> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: mb/google/rex/variants/baseboard/rex: Add CPU power limit values
by Sumeet R Pawnikar (Code Review) 25 Apr '23

25 Apr '23

Attention is currently required from: Tarun Tuli, Subrata Banik, Kapil Porwal. Sumeet R Pawnikar has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/74754 ) Change subject: mb/google/rex/variants/baseboard/rex: Add CPU power limit values ...................................................................... mb/google/rex/variants/baseboard/rex: Add CPU power limit values Add CPU power limit values for rex baseboard BRANCH=None BUG=b:270664854 TEST=Built and verified power limit values for 15W SKU on Rex board Change-Id: If46445157358e3e0f227e26a35b4303fc9189a4b Signed-off-by: Sumeet Pawnikar <sumeet.r.pawnikar(a)intel.com> --- M src/mainboard/google/rex/variants/baseboard/rex/Makefile.inc A src/mainboard/google/rex/variants/baseboard/rex/ramstage.c 2 files changed, 34 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/54/74754/1 diff --git a/src/mainboard/google/rex/variants/baseboard/rex/Makefile.inc b/src/mainboard/google/rex/variants/baseboard/rex/Makefile.inc index fd45b94..d9e6c9e 100644 --- a/src/mainboard/google/rex/variants/baseboard/rex/Makefile.inc +++ b/src/mainboard/google/rex/variants/baseboard/rex/Makefile.inc @@ -1 +1,2 @@ romstage-y += memory.c +romstage-y += ramstage.c diff --git a/src/mainboard/google/rex/variants/baseboard/rex/ramstage.c b/src/mainboard/google/rex/variants/baseboard/rex/ramstage.c new file mode 100644 index 0000000..b3c5406 --- /dev/null +++ b/src/mainboard/google/rex/variants/baseboard/rex/ramstage.c @@ -0,0 +1,17 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +#include <baseboard/variants.h> +#include <device/pci_ids.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 */ + /* Following values are for performance config as per document #640982 */ + { PCI_DID_INTEL_MTL_P_ID_2, 15, 10000, 15000, 57000, 57000, 114000 }, +}; + +void variant_devtree_update(void) +{ + size_t total_entries = ARRAY_SIZE(limits); + variant_update_power_limits(limits, total_entries); +} -- To view, visit https://review.coreboot.org/c/coreboot/+/74754 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If46445157358e3e0f227e26a35b4303fc9189a4b Gerrit-Change-Number: 74754 Gerrit-PatchSet: 1 Gerrit-Owner: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-MessageType: newchange

1 0

[S] Change in coreboot[master]: mb/google/rex: Add devtree update support for variants
by Sumeet R Pawnikar (Code Review) 25 Apr '23

25 Apr '23

Attention is currently required from: Tarun Tuli, Subrata Banik, Kapil Porwal, Eric Lai. Hello build bot (Jenkins), Tarun Tuli, Subrata Banik, Kapil Porwal, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/74379 to look at the new patch set (#3). Change subject: mb/google/rex: Add devtree update support for variants ...................................................................... mb/google/rex: Add devtree update support for variants Add support of variant_devtree_update() function to override devtree settings for variant boards BRANCH=None BUG=b:270664854 TEST=Built and verified power limit values for 15W SKU on Rex board Change-Id: I7953ce8dc0a247d440154902cef1007eb9358dc1 Signed-off-by: Sumeet Pawnikar <sumeet.r.pawnikar(a)intel.com> --- M src/mainboard/google/rex/mainboard.c M src/mainboard/google/rex/variants/baseboard/include/baseboard/variants.h 2 files changed, 26 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/74379/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/74379 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7953ce8dc0a247d440154902cef1007eb9358dc1 Gerrit-Change-Number: 74379 Gerrit-PatchSet: 3 Gerrit-Owner: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-MessageType: newpatchset

1 0

[S] Change in coreboot[master]: mb/google/rex: Add CPU power limit values
by Sumeet R Pawnikar (Code Review) 25 Apr '23

25 Apr '23

Attention is currently required from: Tarun Tuli, Subrata Banik, Kapil Porwal, Eric Lai. Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74379 ) Change subject: mb/google/rex: Add CPU power limit values ...................................................................... Patch Set 2: (4 comments) Patchset: PS2: > Please split this CL into 2 Cls […] Ack File src/mainboard/google/rex/mainboard.c: https://review.coreboot.org/c/coreboot/+/74379/comment/14897038_8f0d46eb PS2, Line 36: variant_devtree_update > why we need to do this override for per variant rather than baseboard ? […] Based on above comment, moving required things under baseboard rex to avoid repeat of code. File src/mainboard/google/rex/variants/rex0/ramstage.c: https://review.coreboot.org/c/coreboot/+/74379/comment/29a9f8d0_fb92eb90 PS1, Line 17: variant_update_power_limits > This might be little different from board to board based on their power and thermal design. Actually, here these are based on machine id (SKU ID) and SoC tdp value, we configure Max and Min values for PL1 and PL2. Also, we configure PL4 value. But note that OEM/ODM can modify these values based on their power and thermal cooling capabilities. File src/mainboard/google/rex/variants/rex0/ramstage.c: https://review.coreboot.org/c/coreboot/+/74379/comment/358d2931_2f865253 PS2, Line 17: } > I prefer to make it visible to know what we are filling into. […] I feel this above way of filling information is only applicable if we are sure that rex0 board won't have any other mchid i.e.CPU SKUs What if we have rex0 board with different mchid for the same TDP i.e. different 15W CPU SKUs (PCI DID). For an example PCI_DID_INTEL_MTL_P_ID_5=0x7d16 -- To view, visit https://review.coreboot.org/c/coreboot/+/74379 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7953ce8dc0a247d440154902cef1007eb9358dc1 Gerrit-Change-Number: 74379 Gerrit-PatchSet: 2 Gerrit-Owner: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Comment-Date: Tue, 25 Apr 2023 11:21:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com> Comment-In-Reply-To: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: soc/intel/common: add power limits update support for variants
by Sumeet R Pawnikar (Code Review) 25 Apr '23

25 Apr '23

Attention is currently required from: Tarun Tuli, Subrata Banik, Kapil Porwal, Eric Lai. Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74620 ) Change subject: soc/intel/common: add power limits update support for variants ...................................................................... Patch Set 1: (2 comments) File src/soc/intel/common/block/include/intelblocks/power_limit.h: https://review.coreboot.org/c/coreboot/+/74620/comment/d6579945_fa84f6db PS1, Line 45: struct cpu_power_limits { : uint16_t mchid; : u8 cpu_tdp; : unsigned int pl1_min_power; : unsigned int pl1_max_power; : unsigned int pl2_min_power; : unsigned int pl2_max_power; : unsigned int pl4_power; : }; > would you consider using this data structure inside brya as well ? https://github. […] Ack. File src/soc/intel/common/block/power_limit/power_limit.c: https://review.coreboot.org/c/coreboot/+/74620/comment/522d3f43_2ecb97f1 PS1, Line 238: variant > soc_update_power_limits is better for common code. This function would get called from varaints so prefer to keep it as it is. -- To view, visit https://review.coreboot.org/c/coreboot/+/74620 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I414715f211d816bbfad03a673ca96dd5df94caeb Gerrit-Change-Number: 74620 Gerrit-PatchSet: 1 Gerrit-Owner: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Comment-Date: Tue, 25 Apr 2023 11:19:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit April 2023