Attention is currently required from: Benjamin Doron.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49140 )
Change subject: soc/intel/skylake/acpi: Add PEP table
......................................................................
Patch Set 3:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/49140/comment/ed48f280_da161083
PS3, Line 11:
> > Not needed. SlpS0 is not asserted externally but by the SoC.
>
> And the pad doesn't need to be `_NF`?
It can be but it simply doesn't matter because your board doesn't use it.
>
> > So, the problem seems to be that Linux doesn't differentiate PC10-only and PC10+SlpS0 systems. Iow. there's a check missing here ... https://github.com/torvalds/linux/blob/master/drivers/platform/x86/intel_pm…
>
> Well, ideally the system would make it to SlpS0. I think it's correct to warn that this didn't happen. However, if the table doesn't advertise support, it can be argued that the kernel doesn't need to check the default address...
That's the point. When SlpS0 is not advertised there is no need to check/warn for S0Slp.
>
> To me, this appears deliberate, but I couldn't be sure.
>
> > Hmm, somewhere above you wrote Linux prints an error for PC10 on vendor firmware. Did you really mean PC10 or SlpS0?
>
> I meant PC10.
>
> Are ASPM and L1 substates involved? The vendor firmware has the "ASPM not supported" bit set in FADT and disables L1 substates.
Yes, both are required to get to PC10.
>
> In coreboot, I had disabled L1 substates and set ASPM to "L1" on the WLAN's root port to mitigate AER errors. I noticed now that the LAN's root port has disabled ASPM but I'm not sure why (the bit is read-write-once, I'll look into that).
Check Kconfig and devicetree for L1 states.
--
To view, visit https://review.coreboot.org/c/coreboot/+/49140
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I08d8c1fde4f447e9292a0508649f802fdc2721e1
Gerrit-Change-Number: 49140
Gerrit-PatchSet: 3
Gerrit-Owner: Benjamin Doron <benjamin.doron00(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Benjamin Doron <benjamin.doron00(a)gmail.com>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Benjamin Doron <benjamin.doron00(a)gmail.com>
Gerrit-Comment-Date: Tue, 09 Feb 2021 20:25:22 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Benjamin Doron <benjamin.doron00(a)gmail.com>
Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-MessageType: comment
Paul Fagerburg has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/50449 )
Change subject: templates: mem_parts_used.txt say that order matters
......................................................................
templates: mem_parts_used.txt say that order matters
Add comments to mem_parts_used.txt to point out that the order of
the entries matters when assigning IDs, so always add a new part
to the end of the file.
BUG=b:175898902
TEST=create a new variant of dalboz, trembyle, volteer, waddledee,
or waddledoo, and observe that mem_parts_used.txt has the new
verbiage.
Signed-off-by: Paul Fagerburg <pfagerburg(a)google.com>
Change-Id: Iffbd8e69a89b1b7c810c5d25c7a6148d459d8b02
---
M util/mainboard/google/dalboz/template/spd/mem_parts_used.txt
M util/mainboard/google/trembyle/template/spd/mem_parts_used.txt
M util/mainboard/google/volteer/template/memory/mem_parts_used.txt
M util/mainboard/google/waddledee/template/memory/mem_parts_used.txt
M util/mainboard/google/waddledoo/template/memory/mem_parts_used.txt
5 files changed, 32 insertions(+), 11 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/49/50449/1
diff --git a/util/mainboard/google/dalboz/template/spd/mem_parts_used.txt b/util/mainboard/google/dalboz/template/spd/mem_parts_used.txt
index 106a705..8124e4f 100644
--- a/util/mainboard/google/dalboz/template/spd/mem_parts_used.txt
+++ b/util/mainboard/google/dalboz/template/spd/mem_parts_used.txt
@@ -1,9 +1,11 @@
# This is a CSV file containing a list of memory parts used by this variant.
# One part per line with an optional fixed ID in column 2.
# Only include a fixed ID if it is required for legacy reasons!
-# Each part must also be listed in util/spd_tools/ddr4/global_ddr4_mem_parts.json.txt.
+# Generated IDs are dependent on the order of parts in this file,
+# so new parts must always be added at the end of the file!
+#
# Generate an updated Makefile.inc and dram_id.generated.txt by running the
-# gen_part_id tool from util/spd_tools/ddr4.
-# See util/spd_tools/ddr4/README.md for more details and instructions.
+# gen_part_id tool from util/spd_tools/{ddr4,lp4x}.
+# See util/spd_tools/{ddr4,lp4x}/README.md for more details and instructions.
# Part Name, Fixed ID (optional)
diff --git a/util/mainboard/google/trembyle/template/spd/mem_parts_used.txt b/util/mainboard/google/trembyle/template/spd/mem_parts_used.txt
index 106a705..8124e4f 100644
--- a/util/mainboard/google/trembyle/template/spd/mem_parts_used.txt
+++ b/util/mainboard/google/trembyle/template/spd/mem_parts_used.txt
@@ -1,9 +1,11 @@
# This is a CSV file containing a list of memory parts used by this variant.
# One part per line with an optional fixed ID in column 2.
# Only include a fixed ID if it is required for legacy reasons!
-# Each part must also be listed in util/spd_tools/ddr4/global_ddr4_mem_parts.json.txt.
+# Generated IDs are dependent on the order of parts in this file,
+# so new parts must always be added at the end of the file!
+#
# Generate an updated Makefile.inc and dram_id.generated.txt by running the
-# gen_part_id tool from util/spd_tools/ddr4.
-# See util/spd_tools/ddr4/README.md for more details and instructions.
+# gen_part_id tool from util/spd_tools/{ddr4,lp4x}.
+# See util/spd_tools/{ddr4,lp4x}/README.md for more details and instructions.
# Part Name, Fixed ID (optional)
diff --git a/util/mainboard/google/volteer/template/memory/mem_parts_used.txt b/util/mainboard/google/volteer/template/memory/mem_parts_used.txt
index f51b3af..e4258b5 100644
--- a/util/mainboard/google/volteer/template/memory/mem_parts_used.txt
+++ b/util/mainboard/google/volteer/template/memory/mem_parts_used.txt
@@ -1,4 +1,11 @@
# This is a CSV file containing a list of memory parts used by this variant.
+# One part per line with an optional fixed ID in column 2.
+# Only include a fixed ID if it is required for legacy reasons!
+# Generated IDs are dependent on the order of parts in this file,
+# so new parts must always be added at the end of the file!
+#
# Generate an updated Makefile.inc and dram_id.generated.txt by running the
-# gen_part_id tool from util/spd_tools/ddr4 or util/spd_tools/lp4x
+# gen_part_id tool from util/spd_tools/{ddr4,lp4x}.
# See util/spd_tools/{ddr4,lp4x}/README.md for more details and instructions.
+
+# Part Name
diff --git a/util/mainboard/google/waddledee/template/memory/mem_parts_used.txt b/util/mainboard/google/waddledee/template/memory/mem_parts_used.txt
index 59381dc..e4258b5 100644
--- a/util/mainboard/google/waddledee/template/memory/mem_parts_used.txt
+++ b/util/mainboard/google/waddledee/template/memory/mem_parts_used.txt
@@ -1,6 +1,11 @@
# This is a CSV file containing a list of memory parts used by this variant.
+# One part per line with an optional fixed ID in column 2.
+# Only include a fixed ID if it is required for legacy reasons!
+# Generated IDs are dependent on the order of parts in this file,
+# so new parts must always be added at the end of the file!
+#
# Generate an updated Makefile.inc and dram_id.generated.txt by running the
-# gen_part_id tool from util/spd_tools/lp4x
-# See util/spd_tools/lp4x/README.md for more details and instructions.
+# gen_part_id tool from util/spd_tools/{ddr4,lp4x}.
+# See util/spd_tools/{ddr4,lp4x}/README.md for more details and instructions.
# Part Name
diff --git a/util/mainboard/google/waddledoo/template/memory/mem_parts_used.txt b/util/mainboard/google/waddledoo/template/memory/mem_parts_used.txt
index 59381dc..e4258b5 100644
--- a/util/mainboard/google/waddledoo/template/memory/mem_parts_used.txt
+++ b/util/mainboard/google/waddledoo/template/memory/mem_parts_used.txt
@@ -1,6 +1,11 @@
# This is a CSV file containing a list of memory parts used by this variant.
+# One part per line with an optional fixed ID in column 2.
+# Only include a fixed ID if it is required for legacy reasons!
+# Generated IDs are dependent on the order of parts in this file,
+# so new parts must always be added at the end of the file!
+#
# Generate an updated Makefile.inc and dram_id.generated.txt by running the
-# gen_part_id tool from util/spd_tools/lp4x
-# See util/spd_tools/lp4x/README.md for more details and instructions.
+# gen_part_id tool from util/spd_tools/{ddr4,lp4x}.
+# See util/spd_tools/{ddr4,lp4x}/README.md for more details and instructions.
# Part Name
--
To view, visit https://review.coreboot.org/c/coreboot/+/50449
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iffbd8e69a89b1b7c810c5d25c7a6148d459d8b02
Gerrit-Change-Number: 50449
Gerrit-PatchSet: 1
Gerrit-Owner: Paul Fagerburg <pfagerburg(a)chromium.org>
Gerrit-MessageType: newchange
Attention is currently required from: Shreesh Chhabbi, Ravishankar Sarawadi, Duncan Laurie, Alex Levin, Raj Astekar, Patrick Rudolph.
Shreesh Chhabbi has uploaded a new patch set (#37) to the change originally created by Shreesh Chhabbi. ( https://review.coreboot.org/c/coreboot/+/49766 )
Change subject: soc/intel/tgl: Update S0ix enable mask based on SoC and mainboard design
......................................................................
soc/intel/tgl: Update S0ix enable mask based on SoC and mainboard design
This change uses the following information to determine the
appropriate S0ix states to enable as per PDG document: 607872
for TGL UP3 Rev2p2 (section 10.13):
1. SoC - UP3 v/s UP4
2. H/W design - external phy gating, external clk gating, external bypass
3. Devices enabled at runtime - CNVi, ISH
In some cases, it is recommended to use a shallower state for
S0ix even if the higher state can be achieved (e.g. with external
gating not enabled). This recommendation is because the shallower
state is determined to provide better power savings as per the
above document. Deepest state expected on tigerlake up3 based
platforms is S0i3.1.
BUG=b:177821896
TEST=
1. Build coreboot for volteer.
2. Verify "cat /sys/kernel/debug/pmc_core/substate_residencies" on delbin.
S0i2.0, S0i2.1, S0i3.0 and S0i3.1 states show as enabled.
Signed-off-by: Shreesh Chhabbi <shreesh.chhabbi(a)intel.corp-partner.google.com>
Change-Id: I5f2ac8b72d0c9b05bc02c092188d0c742cc83af9
---
M src/soc/intel/tigerlake/chip.h
M src/soc/intel/tigerlake/fsp_params.c
2 files changed, 77 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/49766/37
--
To view, visit https://review.coreboot.org/c/coreboot/+/49766
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I5f2ac8b72d0c9b05bc02c092188d0c742cc83af9
Gerrit-Change-Number: 49766
Gerrit-PatchSet: 37
Gerrit-Owner: Shreesh Chhabbi <shreesh.chhabbi(a)intel.com>
Gerrit-Reviewer: Alex Levin <levinale(a)google.com>
Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org>
Gerrit-Reviewer: Duncan Laurie <dlaurie(a)gmail.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Raj Astekar <raj.astekar(a)intel.com>
Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com>
Gerrit-Reviewer: Shreesh Chhabbi <shreesh.chhabbi(a)intel.corp-partner.google.com>
Gerrit-Reviewer: Sukumar Ghorai <sukumar.ghorai(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Shreesh Chhabbi <shreesh.chhabbi(a)intel.com>
Gerrit-Attention: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com>
Gerrit-Attention: Duncan Laurie <dlaurie(a)chromium.org>
Gerrit-Attention: Alex Levin <levinale(a)google.com>
Gerrit-Attention: Duncan Laurie <dlaurie(a)gmail.com>
Gerrit-Attention: Raj Astekar <raj.astekar(a)intel.com>
Gerrit-Attention: Shreesh Chhabbi <shreesh.chhabbi(a)intel.corp-partner.google.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Ravishankar Sarawadi, Duncan Laurie, Alex Levin, Raj Astekar, Shreesh Chhabbi, Patrick Rudolph.
Shreesh Chhabbi has uploaded a new patch set (#36) to the change originally created by Shreesh Chhabbi. ( https://review.coreboot.org/c/coreboot/+/49766 )
Change subject: soc/intel/tgl: Update S0ix enable mask based on SoC and mainboard design
......................................................................
soc/intel/tgl: Update S0ix enable mask based on SoC and mainboard design
This change uses the following information to determine the
appropriate S0ix states to enable as per PDG document: 607872
for TGL UP3 UP Rev2p2 (section 10.13):
1. SoC - UP3 v/s UP4
2. H/W design - external phy gating, external clk gating, external bypass
3. Devices enabled at runtime - CNVi, ISH
In some cases, it is recommended to use a shallower state for
S0ix even if the higher state can be achieved (e.g. with external
gating not enabled). This recommendation is because the shallower
state is determined to provide better power savings as per the
above document. Deepest state expected on tigerlake up3 based
platforms is S0i3.1.
BUG=b:177821896
TEST=
1. Build coreboot for volteer.
2. Verify "cat /sys/kernel/debug/pmc_core/substate_residencies" on delbin.
S0i2.0, S0i2.1, S0i3.0 and S0i3.1 states show as enabled.
Signed-off-by: Shreesh Chhabbi <shreesh.chhabbi(a)intel.corp-partner.google.com>
Change-Id: I5f2ac8b72d0c9b05bc02c092188d0c742cc83af9
---
M src/soc/intel/tigerlake/chip.h
M src/soc/intel/tigerlake/fsp_params.c
2 files changed, 77 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/49766/36
--
To view, visit https://review.coreboot.org/c/coreboot/+/49766
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I5f2ac8b72d0c9b05bc02c092188d0c742cc83af9
Gerrit-Change-Number: 49766
Gerrit-PatchSet: 36
Gerrit-Owner: Shreesh Chhabbi <shreesh.chhabbi(a)intel.com>
Gerrit-Reviewer: Alex Levin <levinale(a)google.com>
Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org>
Gerrit-Reviewer: Duncan Laurie <dlaurie(a)gmail.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Raj Astekar <raj.astekar(a)intel.com>
Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com>
Gerrit-Reviewer: Shreesh Chhabbi <shreesh.chhabbi(a)intel.corp-partner.google.com>
Gerrit-Reviewer: Sukumar Ghorai <sukumar.ghorai(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com>
Gerrit-Attention: Duncan Laurie <dlaurie(a)chromium.org>
Gerrit-Attention: Alex Levin <levinale(a)google.com>
Gerrit-Attention: Duncan Laurie <dlaurie(a)gmail.com>
Gerrit-Attention: Raj Astekar <raj.astekar(a)intel.com>
Gerrit-Attention: Shreesh Chhabbi <shreesh.chhabbi(a)intel.corp-partner.google.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Shreesh Chhabbi, Ravishankar Sarawadi, Duncan Laurie, Alex Levin, Raj Astekar, Patrick Rudolph.
Hello build bot (Jenkins), Furquan Shaikh, Ravishankar Sarawadi, Duncan Laurie, Alex Levin, Duncan Laurie, Sukumar Ghorai, Raj Astekar, Patrick Rudolph, Shreesh Chhabbi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/49766
to look at the new patch set (#35).
Change subject: soc/intel/tgl: Update S0ix enable mask based on SoC and mainboard design
......................................................................
soc/intel/tgl: Update S0ix enable mask based on SoC and mainboard design
This change uses the following information to determine the
appropriate S0ix states to enable as per PDG document: 607872
for TGL UP3 UP Rev2p2 (section 10.13):
1. SoC - UP3 v/s UP4
2. H/W design - external phy gating, external clk gating, external bypass
3. Devices enabled at runtime - CNVi, ISH
In some cases, it is recommended to use a shallower state for
S0ix even if the higher state can be achieved (e.g. with external
gating not enabled). This recommendation is because the shallower
state is determined to provide better power savings as per the
above document. Deepest state expected on tigerlake up3 based
platforms is S0i3.1.
BUG=b:177821896
TEST=Build coreboot for volteer. Verify that deepest
S0ix substate that is enabled is S0i3.1
Signed-off-by: Shreesh Chhabbi <shreesh.chhabbi(a)intel.corp-partner.google.com>
Change-Id: I5f2ac8b72d0c9b05bc02c092188d0c742cc83af9
---
M src/soc/intel/tigerlake/chip.h
M src/soc/intel/tigerlake/fsp_params.c
2 files changed, 77 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/49766/35
--
To view, visit https://review.coreboot.org/c/coreboot/+/49766
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I5f2ac8b72d0c9b05bc02c092188d0c742cc83af9
Gerrit-Change-Number: 49766
Gerrit-PatchSet: 35
Gerrit-Owner: Shreesh Chhabbi <shreesh.chhabbi(a)intel.com>
Gerrit-Reviewer: Alex Levin <levinale(a)google.com>
Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org>
Gerrit-Reviewer: Duncan Laurie <dlaurie(a)gmail.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Raj Astekar <raj.astekar(a)intel.com>
Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com>
Gerrit-Reviewer: Shreesh Chhabbi <shreesh.chhabbi(a)intel.corp-partner.google.com>
Gerrit-Reviewer: Sukumar Ghorai <sukumar.ghorai(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Shreesh Chhabbi <shreesh.chhabbi(a)intel.com>
Gerrit-Attention: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com>
Gerrit-Attention: Duncan Laurie <dlaurie(a)chromium.org>
Gerrit-Attention: Alex Levin <levinale(a)google.com>
Gerrit-Attention: Duncan Laurie <dlaurie(a)gmail.com>
Gerrit-Attention: Raj Astekar <raj.astekar(a)intel.com>
Gerrit-Attention: Shreesh Chhabbi <shreesh.chhabbi(a)intel.corp-partner.google.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-MessageType: newpatchset