coreboot-gerrit April 2023

coreboot-gerrit@coreboot.org

1 participants
5761 discussions

[M] Change in coreboot[master]: soc/intel/common: Introduce API to get the FSP Reset Status
by Subrata Banik (Code Review) 26 Apr '23

26 Apr '23

Attention is currently required from: Tarun Tuli, Derek Huang, Kapil Porwal, Arthur Heymans. Hello build bot (Jenkins), Tarun Tuli, Derek Huang, Kapil Porwal, Arthur Heymans, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/74783 to look at the new patch set (#2). Change subject: soc/intel/common: Introduce API to get the FSP Reset Status ...................................................................... soc/intel/common: Introduce API to get the FSP Reset Status This patch creates a function to read the FSP API Reset Status. This function relies on the FSP Scheduled Reset HOB which holds the reset type (warm/cold/shutdown) information along with any platform specific reset need (like global reset). Ideally FSP API should be able to return the status (both success and error code) upon exiting the FSP API but unfortunately there are some scenarios in ADL/RPL FSP where MultiPhaseSiInit API is unable to return any ERROR status. Hence, this function provides an additional hook to read the FSP reset status by reading the dedicated HOB without relying on the FSP API exit status code. Additionally, create FSP_MULTIPHASE_SI_INIT_RETURN_BROKEN config option to handle broken FSP API return status issue. Any SoC platform that selects the `FSP_MULTIPHASE_SI_INIT_RETURN_BROKEN` config will call into this newly added API to get the FSP return status from MultiPhaseSiInit. BUG=b:278665768 TEST=Able to select FSP_MULTIPHASE_SI_INIT_RETURN_BROKEN for ADL/RPL SoC code and call into this API to know the return status from MultiPhaseSiInit FSP API. Signed-off-by: Subrata Banik <subratabanik(a)google.com> Change-Id: Ief5d79736cc11a0a31ca2889128285795f8b5aae --- M src/drivers/intel/fsp2_0/Kconfig M src/soc/intel/common/fsp_reset.c M src/soc/intel/common/reset.h 3 files changed, 107 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/74783/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/74783 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ief5d79736cc11a0a31ca2889128285795f8b5aae Gerrit-Change-Number: 74783 Gerrit-PatchSet: 2 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Derek Huang <derekhuang(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Derek Huang <derekhuang(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newpatchset

1 0

[S] Change in coreboot[master]: drivers/intel/fsp2_0: Create config option to handle broken FSP API
by Subrata Banik (Code Review) 26 Apr '23

26 Apr '23

Subrata Banik has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/74782 ) Change subject: drivers/intel/fsp2_0: Create config option to handle broken FSP API ...................................................................... Abandoned -- To view, visit https://review.coreboot.org/c/coreboot/+/74782 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3b15ef5b50fe1fe73bf9ad0aa4be90aae07ebf73 Gerrit-Change-Number: 74782 Gerrit-PatchSet: 1 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Derek Huang <derekhuang(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: abandon

1 0

[S] Change in coreboot[master]: mb/google/skyrim/var/winterhold: adjust the eDP panel power sequence
by Chris Wang (Code Review) 26 Apr '23

26 Apr '23

Attention is currently required from: Jason Nien, Martin Roth. Chris Wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74790 ) Change subject: mb/google/skyrim/var/winterhold: adjust the eDP panel power sequence ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/74790/comment/921d196f_dda09865 PS1, Line 9: set Set edp_panel_t9_ms to 8ms which means it will delay 8ms > Possible repeated word: 'set' Please fix. -- To view, visit https://review.coreboot.org/c/coreboot/+/74790 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I952d05b18e29cf30256f43562a5052007c5c6268 Gerrit-Change-Number: 74790 Gerrit-PatchSet: 2 Gerrit-Owner: Chris Wang <chris.wang(a)amd.corp-partner.google.com> Gerrit-Reviewer: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Comment-Date: Wed, 26 Apr 2023 13:29:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: mb/google/skyrim/var/winterhold: adjust the eDP panel power sequence
by Chris Wang (Code Review) 26 Apr '23

26 Apr '23

Attention is currently required from: Jason Nien, Martin Roth. Hello Jason Nien, Martin Roth, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/74790 to look at the new patch set (#2). Change subject: mb/google/skyrim/var/winterhold: adjust the eDP panel power sequence ...................................................................... mb/google/skyrim/var/winterhold: adjust the eDP panel power sequence Set edp_panel_t9_ms to 8ms which means it will delay 8ms between backlight off and vary backlight off. BUG=b:271704149 BRANCH=none TEST=Build; Verify the UPD was passed to system integrated table; Signed-off-by: Chris Wang <chris.wang(a)amd.corp-partner.google.com> Change-Id: I952d05b18e29cf30256f43562a5052007c5c6268 --- M src/mainboard/google/skyrim/variants/winterhold/overridetree.cb 1 file changed, 18 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/74790/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/74790 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I952d05b18e29cf30256f43562a5052007c5c6268 Gerrit-Change-Number: 74790 Gerrit-PatchSet: 2 Gerrit-Owner: Chris Wang <chris.wang(a)amd.corp-partner.google.com> Gerrit-Reviewer: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-MessageType: newpatchset

1 0

[S] Change in coreboot[master]: drivers/intel/fsp2_0: Apply FSP Reset Status W/A for MultiPhaseSiInit
by Tarun Tuli (Code Review) 26 Apr '23

26 Apr '23

Attention is currently required from: Subrata Banik, Derek Huang, Kapil Porwal, Andrey Petrov. Tarun Tuli has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74784 ) Change subject: drivers/intel/fsp2_0: Apply FSP Reset Status W/A for MultiPhaseSiInit ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/74784 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I749c9986e17e4cbab333b29425c9a4a4ba4128fa Gerrit-Change-Number: 74784 Gerrit-PatchSet: 1 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Derek Huang <derekhuang(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Derek Huang <derekhuang(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Comment-Date: Wed, 26 Apr 2023 13:26:40 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: drivers/intel/fsp2_0: Create config option to handle broken FSP API
by Tarun Tuli (Code Review) 26 Apr '23

26 Apr '23

Attention is currently required from: Subrata Banik, Derek Huang, Kapil Porwal, Andrey Petrov. Tarun Tuli has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74782 ) Change subject: drivers/intel/fsp2_0: Create config option to handle broken FSP API ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/74782 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3b15ef5b50fe1fe73bf9ad0aa4be90aae07ebf73 Gerrit-Change-Number: 74782 Gerrit-PatchSet: 1 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Derek Huang <derekhuang(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Derek Huang <derekhuang(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Comment-Date: Wed, 26 Apr 2023 13:25:24 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: drivers/intel/fsp2_0: Inject newline after printing EFI GUID
by Tarun Tuli (Code Review) 26 Apr '23

26 Apr '23

Attention is currently required from: Subrata Banik, Derek Huang, Kapil Porwal, Andrey Petrov. Tarun Tuli has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74781 ) Change subject: drivers/intel/fsp2_0: Inject newline after printing EFI GUID ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/74781 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8d47dbc5d493f86f14a1bbcf9cb5c16c0e12b841 Gerrit-Change-Number: 74781 Gerrit-PatchSet: 1 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Derek Huang <derekhuang(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Derek Huang <derekhuang(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Comment-Date: Wed, 26 Apr 2023 13:24:41 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: soc/intel/common: Introduce API to get the FSP Reset Status
by Subrata Banik (Code Review) 26 Apr '23

26 Apr '23

Attention is currently required from: Tarun Tuli, Derek Huang, Kapil Porwal, Arthur Heymans. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74783 ) Change subject: soc/intel/common: Introduce API to get the FSP Reset Status ...................................................................... Patch Set 1: (1 comment) File src/soc/intel/common/fsp_reset.c: https://review.coreboot.org/c/coreboot/+/74783/comment/53519e0b_8c58af4a PS1, Line 54: if (!CONFIG(FSP_MULTIPHASE_SI_INIT_RETURN_BROKEN)) : return 0; > > > In the next patch the caller also checks this. > > > > yeah, i just wanted to make sure, no one directly calls into it outside the silicon_init.c file. > > > > WDYT? > > I think you can do assert(CONFIG(FSP_MULTIPHASE_SI_INIT_RETURN_BROKEN)); which would result in buildtime failure if used without Kconfig. Does that make sense? thanks for the suggestion. -- To view, visit https://review.coreboot.org/c/coreboot/+/74783 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ief5d79736cc11a0a31ca2889128285795f8b5aae Gerrit-Change-Number: 74783 Gerrit-PatchSet: 1 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Derek Huang <derekhuang(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Derek Huang <derekhuang(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Wed, 26 Apr 2023 13:24:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: soc/intel/alderlake: Add option to disable C1E
by Subrata Banik (Code Review) 26 Apr '23

26 Apr '23

Attention is currently required from: YH Lin, Tarun Tuli, Joey Peng, Paul Menzel, Nick Vaccaro. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74727 ) Change subject: soc/intel/alderlake: Add option to disable C1E ...................................................................... Patch Set 7: (1 comment) Patchset: PS7: > Hi Subrata, > > Since taeko has ADL SKUs and RPL SKUs, we would like to keep C1E enabled on ADL SKUs. > > From FspSUpd.dsc in rplfsp, I saw that C1E is enabled by default, so I think currently it is enabled on all RPL projects. let me summarise my understanding 1. We would like to keep C1E enabled for ADL SKUs 2. Disable C1E for RPL SKUs 3. We shouldn't rely on FSP UPD defaults hence, i have suggested to add logic using CPUID instead mainboard ``` if (cpu_id == CPUID_RAPTORLAKE_P_J0 || cpu_id == CPUID_RAPTORLAKE_P_Q0) s_cfg->C1e = 0; // assuming all RPL SKUs are getting covered here else s_cfg->C1e = 1; // assuming all ADL SKUs are getting covered here ``` -- To view, visit https://review.coreboot.org/c/coreboot/+/74727 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic2d2d5d6075de25141c1d08ec18838731c63a342 Gerrit-Change-Number: 74727 Gerrit-PatchSet: 7 Gerrit-Owner: Joey Peng <joey.peng(a)lcfc.corp-partner.google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: YH Lin <yueherngl(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-CC: Jerry2 Huang <jerry2.huang(a)lcfc.corp-partner.google.com> Gerrit-CC: Kevin3 Yang <kevin3.yang(a)lcfc.corp-partner.google.com> Gerrit-CC: Leo Chou <leo.chou(a)lcfc.corp-partner.google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Stanley Wu <stanley1.wu(a)lcfc.corp-partner.google.com> Gerrit-Attention: YH Lin <yueherngl(a)google.com> Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Joey Peng <joey.peng(a)lcfc.corp-partner.google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Comment-Date: Wed, 26 Apr 2023 13:23:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Joey Peng <joey.peng(a)lcfc.corp-partner.google.com> Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: soc/intel/common: Introduce API to get the FSP Reset Status
by Arthur Heymans (Code Review) 26 Apr '23

26 Apr '23

Attention is currently required from: Tarun Tuli, Subrata Banik, Derek Huang, Kapil Porwal. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74783 ) Change subject: soc/intel/common: Introduce API to get the FSP Reset Status ...................................................................... Patch Set 1: (1 comment) File src/soc/intel/common/fsp_reset.c: https://review.coreboot.org/c/coreboot/+/74783/comment/7aea58d4_eeb128f6 PS1, Line 54: if (!CONFIG(FSP_MULTIPHASE_SI_INIT_RETURN_BROKEN)) : return 0; > > In the next patch the caller also checks this. > > yeah, i just wanted to make sure, no one directly calls into it outside the silicon_init.c file. > > WDYT? I think you can do assert(CONFIG(FSP_MULTIPHASE_SI_INIT_RETURN_BROKEN)); which would result in buildtime failure if used without Kconfig. Does that make sense? -- To view, visit https://review.coreboot.org/c/coreboot/+/74783 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ief5d79736cc11a0a31ca2889128285795f8b5aae Gerrit-Change-Number: 74783 Gerrit-PatchSet: 1 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Derek Huang <derekhuang(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: 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: Derek Huang <derekhuang(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Comment-Date: Wed, 26 Apr 2023 13:20:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> 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