Attention is currently required from: Arthur Heymans, Christian Walter, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Patrick Rudolph, Shuo Liu, Tim Chu.
Hello Arthur Heymans, Christian Walter, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Patrick Rudolph, Tim Chu, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/81566?usp=email
to look at the new patch set (#2).
The following approvals got outdated and were removed:
Code-Review+2 by Patrick Rudolph, Verified+1 by build bot (Jenkins)
Change subject: device/device_util: Add and use is_pci_bridge()
......................................................................
device/device_util: Add and use is_pci_bridge()
Change-Id: Ied4921f7dc7e144e580d05d4f2262777aa59d895
Signed-off-by: Shuo Liu <shuo.liu(a)intel.com>
---
M src/device/device_util.c
M src/include/device/device.h
M src/soc/intel/xeon_sp/uncore_acpi.c
3 files changed, 8 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/81566/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/81566?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ied4921f7dc7e144e580d05d4f2262777aa59d895
Gerrit-Change-Number: 81566
Gerrit-PatchSet: 2
Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com>
Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Dinesh Gehlot, Eric Lai, Jamie Chen, Kapil Porwal, Nick Vaccaro, Paul Menzel, Subrata Banik, Sumeet R Pawnikar.
SH Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81614?usp=email )
Change subject: mb/google/brya/var/xol: Configure power limits by battery status
......................................................................
Patch Set 3:
(5 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/81614/comment/8b8fc9fd_25725058 :
PS2, Line 10: platform
: instabilities
> What instabilities occur? Please elaborate? Doesn’t the battery just drain faster?
Done
https://review.coreboot.org/c/coreboot/+/81614/comment/54574e55_50adca65 :
PS2, Line 18: MSR PL2: 55W, MSR PL4: 114W
> Please document the command you used to get these.
I mentioned I used PTAT.
File src/mainboard/google/brya/variants/xol/ramstage.c:
https://review.coreboot.org/c/coreboot/+/81614/comment/dfb9ff84_ca71fa31 :
PS2, Line 24:
> Log the non-zero return value?
Done
https://review.coreboot.org/c/coreboot/+/81614/comment/92086335_f751206a :
PS2, Line 28: get_soc_power_limit_config
> this is already present inside baseboard/brya. why not leveraging the same ? […]
Done
https://review.coreboot.org/c/coreboot/+/81614/comment/fa41df4c_d1f30ae3 :
PS2, Line 64: performance
> The commit message says *power*?
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/81614?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I5d71e9edde0ecbd7aaf316cd754a6ebcff9da77d
Gerrit-Change-Number: 81614
Gerrit-PatchSet: 3
Gerrit-Owner: SH Kim <sh_.kim(a)samsung.corp-partner.google.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Jamie Chen <jamie.chen(a)intel.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Eric Lai <ericllai(a)google.com>
Gerrit-Attention: Jamie Chen <jamie.chen(a)intel.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Comment-Date: Mon, 01 Apr 2024 07:17:05 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: comment
Attention is currently required from: Dinesh Gehlot, Eric Lai, Jamie Chen, Kapil Porwal, Nick Vaccaro, SH Kim, Sumeet R Pawnikar.
Hello Dinesh Gehlot, Eric Lai, Jamie Chen, Kapil Porwal, Nick Vaccaro, Subrata Banik, Sumeet R Pawnikar,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/81614?usp=email
to look at the new patch set (#3).
Change subject: mb/google/brya/var/xol: Configure power limits by battery status
......................................................................
mb/google/brya/var/xol: Configure power limits by battery status
When battery level is below critical level or battery is not present,
cpus need to run with a power optimized configuration to avoid platform
instabilities such as system power down.
This will check the current battery status and configure cpu power
limits using current PD power value.
BUG=b:328729536
TEST=built and veified MSR PL2/PL4 values using PTAT.
Original power limit configuration for Xol:
MSR PL2: 55W, MSR PL4: 114W.
[When connected 60W adaptor without battery]
Before:
MSR PL2: 55W, MSR PL4: 114W
After:
MSR PL2: 55W, MSR PL4: 60W
[When connected 45W adaptor without battery]
Before:
MSR PL2: 55W, MSR PL4: 114W
After:
MSR PL2: 45W, MSR PL4: 45W
Change-Id: I5d71e9edde0ecbd7aaf316cd754a6ebcff9da77d
Signed-off-by: Seunghwan Kim <sh_.kim(a)samsung.corp-partner.google.com>
---
M src/mainboard/google/brya/variants/baseboard/brya/ramstage.c
M src/mainboard/google/brya/variants/baseboard/include/baseboard/variants.h
M src/mainboard/google/brya/variants/xol/Makefile.mk
A src/mainboard/google/brya/variants/xol/ramstage.c
4 files changed, 65 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/81614/3
--
To view, visit https://review.coreboot.org/c/coreboot/+/81614?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I5d71e9edde0ecbd7aaf316cd754a6ebcff9da77d
Gerrit-Change-Number: 81614
Gerrit-PatchSet: 3
Gerrit-Owner: SH Kim <sh_.kim(a)samsung.corp-partner.google.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Jamie Chen <jamie.chen(a)intel.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Eric Lai <ericllai(a)google.com>
Gerrit-Attention: Jamie Chen <jamie.chen(a)intel.com>
Gerrit-Attention: SH Kim <sh_.kim(a)samsung.corp-partner.google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Arthur Heymans, Christian Walter, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Shuo Liu, Tim Chu.
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81566?usp=email )
Change subject: device/device_util: Add is_pci_bridge
......................................................................
Patch Set 1:
(1 comment)
File src/device/device_util.c:
https://review.coreboot.org/c/coreboot/+/81566/comment/a649a92c_34a4b6d3 :
PS1, Line 939: (
The outer parentheses are not needed, please remove
--
To view, visit https://review.coreboot.org/c/coreboot/+/81566?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ied4921f7dc7e144e580d05d4f2262777aa59d895
Gerrit-Change-Number: 81566
Gerrit-PatchSet: 1
Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com>
Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Comment-Date: Mon, 01 Apr 2024 07:00:01 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans, Christian Walter, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Patrick Rudolph, Tim Chu.
Shuo Liu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81570?usp=email )
Change subject: soc/intel/xeon_sp: Add soc_add_stack_mmios
......................................................................
Patch Set 1:
(3 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/81570/comment/3776a576_a24154a5 :
PS1, Line 10: unused IOAT MMIOs (CPM1 and HQM1). These resources are within stack
> How do you read the stack MMIO range? is there a HOB for that?
xSTACK_RES *sr->PciResourceMem64Base/Limit is the portion for PCI device
xSTACK_RES *sr->Mmio64Base/Limit is the total MMIO range
e.g. in CXL domain setting up, non PCI resources are used
if (sr->Mmio64Base < sr->PciResourceMem64Base) {
res = new_resource(dev, index++);
res->base = sr->Mmio64Base;
res->limit = sr->PciResourceMem64Base - 1;
res->size = res->limit - res->base + 1;
res->flags = IORESOURCE_MEM | IORESOURCE_ASSIGNED;
}
https://review.coreboot.org/c/coreboot/+/81570/comment/b089a85f_89d3067e :
PS1, Line 10: unused
> what does unused mean? When it decodes MMIO, it's not unused, is it?
in void create_ioat_domains(const union xeon_domain_path path ...), HQM1 and CPM1 MMIO64 will be always reserved, but for SKUs that is not with HQM1 and CPM1, these resources are not assigned to domains, thus forming MTRR segmentation. Will update the commit message.
File src/soc/intel/xeon_sp/chip_common.c:
https://review.coreboot.org/c/coreboot/+/81570/comment/980b072d_f3c40678 :
PS1, Line 135: while (from && (from->path.type != DEVICE_PATH_DOMAIN) &&
This check is not correct, will update.
--
To view, visit https://review.coreboot.org/c/coreboot/+/81570?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ib2a8063d8bee8fb4b5578e84ba8f9951815ffbf3
Gerrit-Change-Number: 81570
Gerrit-PatchSet: 1
Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com>
Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Comment-Date: Mon, 01 Apr 2024 06:51:48 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-MessageType: comment
Attention is currently required from: Dinesh Gehlot, Eric Lai, Jamie Chen, Kapil Porwal, Nick Vaccaro, SH Kim, Sumeet R Pawnikar.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81614?usp=email )
Change subject: mb/google/brya/var/xol: Configure power limits by battery status
......................................................................
Patch Set 2:
(4 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/81614/comment/4c3bc9bf_d12657e9 :
PS2, Line 10: platform
: instabilities
What instabilities occur? Please elaborate? Doesn’t the battery just drain faster?
https://review.coreboot.org/c/coreboot/+/81614/comment/7b847a46_41c7dc32 :
PS2, Line 18: MSR PL2: 55W, MSR PL4: 114W
Please document the command you used to get these.
File src/mainboard/google/brya/variants/xol/ramstage.c:
https://review.coreboot.org/c/coreboot/+/81614/comment/a6447c05_7635d367 :
PS2, Line 24:
Log the non-zero return value?
https://review.coreboot.org/c/coreboot/+/81614/comment/32fe9c32_772b8d24 :
PS2, Line 64: performance
The commit message says *power*?
--
To view, visit https://review.coreboot.org/c/coreboot/+/81614?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I5d71e9edde0ecbd7aaf316cd754a6ebcff9da77d
Gerrit-Change-Number: 81614
Gerrit-PatchSet: 2
Gerrit-Owner: SH Kim <sh_.kim(a)samsung.corp-partner.google.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Jamie Chen <jamie.chen(a)intel.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Eric Lai <ericllai(a)google.com>
Gerrit-Attention: Jamie Chen <jamie.chen(a)intel.com>
Gerrit-Attention: SH Kim <sh_.kim(a)samsung.corp-partner.google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Comment-Date: Mon, 01 Apr 2024 06:51:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans, Christian Walter, Cliff Huang, Jincheng Li, Johnny Lin, Jonathan Zhang, Lance Zhao, Lean Sheng Tan, Patrick Rudolph, Tim Chu, Tim Wawrzynczak.
Shuo Liu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81567?usp=email )
Change subject: soc/intel/xeon_sp: Generate root port SSDT device objects
......................................................................
Patch Set 1:
(1 comment)
File src/acpi/acpigen_pci.c:
https://review.coreboot.org/c/coreboot/+/81567/comment/34a20914_eafd741f :
PS1, Line 368: is_pci_bridge
> When it's a PCI bridge there should be a PCI driver for it, thus no need to have this separate metho […]
Sure, good suggestion, will update.
--
To view, visit https://review.coreboot.org/c/coreboot/+/81567?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I81bd5d5a2e62301543a332162a5a789e0793e18e
Gerrit-Change-Number: 81567
Gerrit-PatchSet: 1
Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Reviewer: Jincheng Li <jincheng.li(a)intel.com>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com>
Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com>
Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com>
Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Jincheng Li <jincheng.li(a)intel.com>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Comment-Date: Mon, 01 Apr 2024 06:46:15 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-MessageType: comment