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
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
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
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
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
Felix Held has uploaded a new patch set (#2). ( https://review.coreboot.org/c/coreboot/+/74791 )
Change subject: arch/x86/include/pci_io_cfg: introduce PCI_IO_CONFIG_[INDEX,DATA] define
......................................................................
arch/x86/include/pci_io_cfg: introduce PCI_IO_CONFIG_[INDEX,DATA] define
Instead of having multiple instances of the same magic numbers in the
code, introduce and use the PCI_IO_CONFIG_INDEX and PCI_IO_CONFIG_DATA
definitions.
TEST=Timeless build for Mandolin results in identical image.
Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de>
Change-Id: If6f6f058180cf36cae7921ce3c7aaf1a0c75c7b9
---
M src/arch/x86/include/arch/pci_io_cfg.h
1 file changed, 31 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/74791/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/74791
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If6f6f058180cf36cae7921ce3c7aaf1a0c75c7b9
Gerrit-Change-Number: 74791
Gerrit-PatchSet: 2
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: newpatchset