Attention is currently required from: Nico Huber, Tim Wawrzynczak, Subrata Banik, Sridhar Siricilla, Michael Niewöhner, Patrick Rudolph.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59359 )
Change subject: soc/intel/common: Implements ACPI CPPCv3 package to support hybrid core
......................................................................
Patch Set 1:
(2 comments)
File src/soc/intel/common/block/acpi/acpi/globalnvs.asl:
https://review.coreboot.org/c/coreboot/+/59359/comment/800d088f_4d0a476f
PS1, Line 28: SFBC, 16, // 0x48 - 0x49 Indicates Scaling factor for Big core
: SFSC, 16, // 0x50 - 0x51 Indicates Scaling Factor for Small Core
: NMFQ, 16, // 0x52 - 0x53 Indicates Nominal Frequency
: CORE, 32, // 0x54 - 0x57 Each marked bit indicates Big Core corresponding to core index
: INFS, 8, // 0x58 - Nominal Frequency is supported
:
> but with coreboot producing AML at runtime, it is rarely actually required to use GNVS, and we prefer to keep away from that when possible. in this case, I don't think I see a good reason that it has to be in NVS.
Right, you can just define a namespace in the ssdt generator using acpigen_write_name_x
File src/soc/intel/common/block/acpi/cpu_hybrid.c:
https://review.coreboot.org/c/coreboot/+/59359/comment/bd7c24db_039390ce
PS1, Line 66: pscope = (char *) malloc(12);
You can just allocate these on the stack. No need for malloc.
--
To view, visit https://review.coreboot.org/c/coreboot/+/59359
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Icd5ea9e70bebd1e66d3cea2bcf8a6678e5cc95ca
Gerrit-Change-Number: 59359
Gerrit-PatchSet: 1
Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Sridhar Siricilla <sridhar.siricilla(a)intel.corp-partner.google.com>
Gerrit-CC: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Thu, 18 Nov 2021 08:48:15 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-MessageType: comment
Attention is currently required from: Furquan Shaikh, Sridhar Siricilla, Patrick Rudolph.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59357 )
Change subject: cpu/intel: Get cpu index based on LAPIC id
......................................................................
Patch Set 1:
(3 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/59357/comment/23a2d4d1_9f6a884e
PS1, Line 9: The patch adds a method to provide cpu index based on LAPCI Id.
: The function determines the cpu's LAPIC id and returns the position of the
: cpu's LAPIC id among the other cores' LAPIC ids.
Explain why you need it here.
It's also not clear to me what 'position' means. Something like counting lapic up to the lapic id of the cpu calling the function is better.
https://review.coreboot.org/c/coreboot/+/59357/comment/cafd4e0d_676a40c5
PS1, Line 13: TEST=Verified on Brya
You're adding a function without calling it, so what exactly is verified?
File src/cpu/x86/mp_init.c:
https://review.coreboot.org/c/coreboot/+/59357/comment/fabb37b4_7fe91cd9
PS1, Line 217: cpus_dev[i]->path.apic.apic_id
It looks like this is most certainly not valid for i == 0. Probably best that you dev_find_lapic(0) and loop over all siblings using a while loop.
--
To view, visit https://review.coreboot.org/c/coreboot/+/59357
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If18116472aaa78cfa88350f313c246a28f67303d
Gerrit-Change-Number: 59357
Gerrit-PatchSet: 1
Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Reviewer: Furquan Shaikh <furquan.m.shaikh(a)gmail.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur.heymans(a)9elements.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Sridhar Siricilla <sridhar.siricilla(a)intel.corp-partner.google.com>
Gerrit-Attention: Furquan Shaikh <furquan.m.shaikh(a)gmail.com>
Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Thu, 18 Nov 2021 08:32:17 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Lance Zhao, Tim Wawrzynczak, Sridhar Siricilla, Michael Niewöhner.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59358 )
Change subject: src/include/acpi: Move CPPC_PACKAGE_NAME macro definition
......................................................................
Patch Set 1:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/59358/comment/786e5c9a_cd220e43
PS1, Line 9: The patch move CPPC_PACKAGE_NAME macro definition from acpi/acpigen.c
: to include/acpi/acpigen.h.
commit messages should say why you do things, not just what you do.
--
To view, visit https://review.coreboot.org/c/coreboot/+/59358
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic547445cdbe2b1a3efe44390bd127f577386e7fc
Gerrit-Change-Number: 59358
Gerrit-PatchSet: 1
Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Reviewer: Lance Zhao
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur.heymans(a)9elements.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Sridhar Siricilla <sridhar.siricilla(a)intel.corp-partner.google.com>
Gerrit-Attention: Lance Zhao
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Comment-Date: Thu, 18 Nov 2021 08:14:47 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Tim Wawrzynczak.
Malik Hsu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59367 )
Change subject: mb/google/brya/variants/primus: add fw_config_probe for ALC5682I-VS
......................................................................
Patch Set 4:
(2 comments)
File src/mainboard/google/brya/variants/primus/variant.c:
https://review.coreboot.org/c/coreboot/+/59367/comment/1f911c1c_b1b56ed2
PS3, Line 25: if (fw_config_probe(FW_CONFIG(AUDIO, MAX98360_ALC5682I_VS_I2S))) {
> nit: blank line before this one
Done
https://review.coreboot.org/c/coreboot/+/59367/comment/177b261e_ac27f3bd
PS3, Line 27: audio_codec->enabled = 1;
> do you still want to enable it regardless of the probe?
Yes, after testing, it is needed.
--
To view, visit https://review.coreboot.org/c/coreboot/+/59367
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I0d5b95e89154b2cb6b371f24cc1b151c23ff642f
Gerrit-Change-Number: 59367
Gerrit-PatchSet: 4
Gerrit-Owner: Malik Hsu <malik_hsu(a)wistron.corp-partner.google.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: YH Lin <yueherngl(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Casper Chang <casper_chang(a)wistron.corp-partner.google.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Comment-Date: Thu, 18 Nov 2021 08:13:28 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Francois Toguo Fotso, Tim Wawrzynczak, Patrick Rudolph.
Kane Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59355 )
Change subject: soc/intel/alderlake: Save/restore BAR registers when extract cpu crashlog
......................................................................
Patch Set 6:
(1 comment)
This change is ready for review.
Patchset:
PS6:
Hi Tim,
Instead of removing tmp bar assignment, i would like to do save/restore BAR0/BAR1 before and after crashlog capture.
It's just in case the bar is not assigned by CB some day in the future and also prevent it occupy pmc mmio in later bios boot.
--
To view, visit https://review.coreboot.org/c/coreboot/+/59355
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib7e6772be51ec4f26ef31fed6cb2bddef8ffc6be
Gerrit-Change-Number: 59355
Gerrit-PatchSet: 6
Gerrit-Owner: Kane Chen <kane.chen(a)intel.corp-partner.google.com>
Gerrit-Reviewer: Francois Toguo Fotso <francois.toguo.fotso(a)intel.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Francois Toguo Fotso <francois.toguo.fotso(a)intel.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Thu, 18 Nov 2021 06:35:49 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Jonathan Zhang, Paul Menzel.
Christian Walter has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58979 )
Change subject: doc/mb/ocp: update Delta Lake documentation
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/58979
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I3eaa765bf9918988b4b5cb01e8607a5c27b9bd17
Gerrit-Change-Number: 58979
Gerrit-PatchSet: 2
Gerrit-Owner: Jonathan Zhang <jonzhang(a)fb.com>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jonathan Zhang <jonzhang(a)fb.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Comment-Date: Thu, 18 Nov 2021 06:35:08 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Raul Rangel, Andrey Pronin, Rob Barnes.
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58870 )
Change subject: soc/amd/psp_verstage: Init TPM on S0i3 resume
......................................................................
Patch Set 18: Code-Review+2
(1 comment)
File src/soc/amd/common/psp_verstage/Kconfig:
https://review.coreboot.org/c/coreboot/+/58870/comment/cb6b1959_1665c143
PS18, Line 12: default VBOOT_STARTS_BEFORE_BOOTBLOCK
Nit: Not for this change, but a follow-up change. We can keep it disabled by default and enable it only in Cezanne.
Currently this enables PSP_INIT_TPM_ON_S0I3_RESUME for both Zork and Guybrush. Since S0i3 is supported only in Cezanne/Guybrush, no impact to Zork.
--
To view, visit https://review.coreboot.org/c/coreboot/+/58870
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ie511928da6a8b4be62621fd2c4c31a8d1e724d48
Gerrit-Change-Number: 58870
Gerrit-PatchSet: 18
Gerrit-Owner: Rob Barnes <robbarnes(a)google.com>
Gerrit-Reviewer: Andrey Pronin <apronin(a)chromium.org>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Kangheui Won <khwon(a)chromium.org>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Andrey Pronin <apronin(a)chromium.org>
Gerrit-Attention: Rob Barnes <robbarnes(a)google.com>
Gerrit-Attention: Andrey Pronin <apronin(a)chromium.org>
Gerrit-Comment-Date: Thu, 18 Nov 2021 05:50:19 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Adam Liu has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/59414 )
Change subject: mainboard/google/brya: Enable dev screen in bios-stage for Brask
......................................................................
mainboard/google/brya: Enable dev screen in bios-stage for Brask
Add Kconfig item ENABLE_TCSS_DISPLAY_DETECTION.
TEST=Build with the VBT provided in issue b:199490251. Check the dev screen in bios-stage.
BUG=b:199490251, b:206014054
Signed-off-by: Adam Liu <adam.liu(a)quanta.corp-partner.google.com>
Change-Id: I5f34be030a6d819a0e93a2d479c4ff41bb14cfe2
---
M src/mainboard/google/brya/Kconfig
1 file changed, 1 insertion(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/59414/1
diff --git a/src/mainboard/google/brya/Kconfig b/src/mainboard/google/brya/Kconfig
index 70729c9..a5ea566 100644
--- a/src/mainboard/google/brya/Kconfig
+++ b/src/mainboard/google/brya/Kconfig
@@ -7,6 +7,7 @@
config BOARD_GOOGLE_BASEBOARD_BRASK
def_bool n
select SPD_CACHE_IN_FMAP
+ select ENABLE_TCSS_DISPLAY_DETECTION
if BOARD_GOOGLE_BASEBOARD_BRYA || BOARD_GOOGLE_BASEBOARD_BRASK
--
To view, visit https://review.coreboot.org/c/coreboot/+/59414
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I5f34be030a6d819a0e93a2d479c4ff41bb14cfe2
Gerrit-Change-Number: 59414
Gerrit-PatchSet: 1
Gerrit-Owner: Adam Liu <adam.liu(a)quanta.corp-partner.google.com>
Gerrit-MessageType: newchange
Attention is currently required from: Rizwan Qureshi, Meera Ravindranath.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59392 )
Change subject: mb/intel/adlrvp: Enable CPU PCIe RP 2
......................................................................
Patch Set 2: Code-Review+2
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/59392/comment/c1356599_c91e51c0
PS2, Line 9: commit:3fd39467b Fix S0ix regression
> Clksrc 3 disable is needed for S0ix regression. […]
Note: if you disable the CLKSRC from strap as well, then also you are doing things statically which is similar to disable in coreboot. You need solution that is dynamic. Do you want to compromise on the capability to have device connected to CPU PCIe ?
I guess we wish to manage dynamic clock gating based on implementing IPC to PMC. Myself and Tim are working towards the implementation for CPU PCIe as here b:197983574
--
To view, visit https://review.coreboot.org/c/coreboot/+/59392
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I0b8b76a5537d8b80777cb7588ce6b22281af7882
Gerrit-Change-Number: 59392
Gerrit-PatchSet: 2
Gerrit-Owner: Meera Ravindranath <meera.ravindranath(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Attention: Meera Ravindranath <meera.ravindranath(a)intel.com>
Gerrit-Comment-Date: Thu, 18 Nov 2021 05:34:51 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Comment-In-Reply-To: Meera Ravindranath <meera.ravindranath(a)intel.com>
Gerrit-MessageType: comment