Matt DeVillier has submitted this change. ( https://review.coreboot.org/c/coreboot/+/39980 )
Change subject: soc/intel/skylake: vr_config: enable PSI3 and PSI4 by default
......................................................................
soc/intel/skylake: vr_config: enable PSI3 and PSI4 by default
There are boards that do not need a specific domain_vr_config because
the defaults provided by the soc code are sufficient. Currently, this
means that these boards can't benefit from lower power states (PSI 3
and 4) because the settings default to being disabled since at the time
the defaults have been defined (2015) there were bugs in FSP in this
regard.
Set the default values of psiXenable to 1 for boards that do not have a
domain_vr_config setting in their devicetree, just like Cannon Lake
does.
Boards that have a domain_vr_config and set their specific settings are
not affected at all. Currently, there are only three boards that have
no domain_vr_config:
- supermicro/x11-lga1151-series
These boards have a MPS MP2955 which we can assume support for PS3
(the MP2965 and MP2935 support it, too).
S-series CPUs with a 1151 socket do not have C9/C10 but only C8 and
since only C10 makes use of PS4, those CPUs won't ever request PS4.
That means we do not need to disable it explicitly for these boards.
- 51nb/x210:
Needs testing and/or VR datasheet check for PS3/PS4 support
Change-Id: I5b5fd9fb3b9b89e80c47f15d706e2dd62dcc0748
Signed-off-by: Michael Niewöhner <foss(a)mniewoehner.de>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/39980
Reviewed-by: Matt DeVillier <matt.devillier(a)gmail.com>
Reviewed-by: Benjamin Doron <benjamin.doron00(a)gmail.com>
Reviewed-by: Angel Pons <th3fanbus(a)gmail.com>
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
---
M src/soc/intel/skylake/vr_config.c
1 file changed, 9 insertions(+), 9 deletions(-)
Approvals:
build bot (Jenkins): Verified
Matt DeVillier: Looks good to me, approved
Angel Pons: Looks good to me, approved
Benjamin Doron: Looks good to me, but someone else must approve
diff --git a/src/soc/intel/skylake/vr_config.c b/src/soc/intel/skylake/vr_config.c
index 98d2513..57c55ec 100644
--- a/src/soc/intel/skylake/vr_config.c
+++ b/src/soc/intel/skylake/vr_config.c
@@ -21,15 +21,15 @@
#include <console/console.h>
#include <intelblocks/cpulib.h>
-/* Default values for domain configuration. PSI3 and PSI4 are disabled. */
+/* Default values for domain configuration. */
static const struct vr_config default_configs[NUM_VR_DOMAINS] = {
[VR_SYSTEM_AGENT] = {
.vr_config_enable = 1,
.psi1threshold = VR_CFG_AMP(20),
.psi2threshold = VR_CFG_AMP(4),
.psi3threshold = VR_CFG_AMP(1),
- .psi3enable = 0,
- .psi4enable = 0,
+ .psi3enable = 1,
+ .psi4enable = 1,
.imon_slope = 0,
.imon_offset = 0,
.icc_max = 0,
@@ -40,8 +40,8 @@
.psi1threshold = VR_CFG_AMP(20),
.psi2threshold = VR_CFG_AMP(5),
.psi3threshold = VR_CFG_AMP(1),
- .psi3enable = 0,
- .psi4enable = 0,
+ .psi3enable = 1,
+ .psi4enable = 1,
.imon_slope = 0,
.imon_offset = 0,
.icc_max = 0,
@@ -52,8 +52,8 @@
.psi1threshold = VR_CFG_AMP(20),
.psi2threshold = VR_CFG_AMP(5),
.psi3threshold = VR_CFG_AMP(1),
- .psi3enable = 0,
- .psi4enable = 0,
+ .psi3enable = 1,
+ .psi4enable = 1,
.imon_slope = 0,
.imon_offset = 0,
.icc_max = 0,
@@ -64,8 +64,8 @@
.psi1threshold = VR_CFG_AMP(20),
.psi2threshold = VR_CFG_AMP(5),
.psi3threshold = VR_CFG_AMP(1),
- .psi3enable = 0,
- .psi4enable = 0,
+ .psi3enable = 1,
+ .psi4enable = 1,
.imon_slope = 0,
.imon_offset = 0,
.icc_max = 0,
--
To view, visit https://review.coreboot.org/c/coreboot/+/39980
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I5b5fd9fb3b9b89e80c47f15d706e2dd62dcc0748
Gerrit-Change-Number: 39980
Gerrit-PatchSet: 8
Gerrit-Owner: Michael Niewöhner
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Benjamin Doron <benjamin.doron00(a)gmail.com>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Matthew Garrett <mjg59(a)google.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged
Philipp Deppenwiese has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39127 )
Change subject: soc/intel/tigerlake: Add function to dump ME firmware status information
......................................................................
Patch Set 10:
Hey guys. Can we implement this in src/soc/intel/common/block/cse/cse.c instead so we have a common code base. Other we would duplicate the code base
--
To view, visit https://review.coreboot.org/c/coreboot/+/39127
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I831a51f9f482425bd3b97ef1d2404b1d06844d07
Gerrit-Change-Number: 39127
Gerrit-PatchSet: 10
Gerrit-Owner: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com>
Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com>
Gerrit-Reviewer: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Poornima Tom <poornima.tom(a)intel.com>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-CC: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com>
Gerrit-Comment-Date: Fri, 03 Apr 2020 15:33:07 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/39967 )
Change subject: soc/intel/common/pch/lockdown: Lock at end of BS_POST_DEVICE
......................................................................
soc/intel/common/pch/lockdown: Lock at end of BS_POST_DEVICE
There's no need to lock it early. Lock it after device initialization
was finished.
Change-Id: I76cca20bc8dc2a7bdc5f16525919f7ef081df798
Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com>
---
M src/soc/intel/common/pch/lockdown/lockdown.c
1 file changed, 1 insertion(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/39967/1
diff --git a/src/soc/intel/common/pch/lockdown/lockdown.c b/src/soc/intel/common/pch/lockdown/lockdown.c
index de45643..684dcee 100644
--- a/src/soc/intel/common/pch/lockdown/lockdown.c
+++ b/src/soc/intel/common/pch/lockdown/lockdown.c
@@ -100,5 +100,4 @@
soc_lockdown_config(chipset_lockdown);
}
-BOOT_STATE_INIT_ENTRY(BS_DEV_RESOURCES, BS_ON_EXIT, platform_lockdown_config,
- NULL);
+BOOT_STATE_INIT_ENTRY(BS_POST_DEVICE, BS_ON_EXIT, platform_lockdown_config, NULL);
--
To view, visit https://review.coreboot.org/c/coreboot/+/39967
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I76cca20bc8dc2a7bdc5f16525919f7ef081df798
Gerrit-Change-Number: 39967
Gerrit-PatchSet: 1
Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-MessageType: newchange
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30360 )
Change subject: mb/libretrend/lt1000: Add Libretrend LT1000 mainboard
......................................................................
Patch Set 19:
(1 comment)
https://review.coreboot.org/c/coreboot/+/30360/19/Documentation/mainboard/i…
File Documentation/mainboard/index.md:
https://review.coreboot.org/c/coreboot/+/30360/19/Documentation/mainboard/i…
PS19, Line 77:
> CB:40118
Sorry, I maintained order by the headers:
## Lenovo > ## Libretrend > ### Nehalem series
--
To view, visit https://review.coreboot.org/c/coreboot/+/30360
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I32fc8a7d3177ba379d04ad8b87adefcfca2b0fab
Gerrit-Change-Number: 30360
Gerrit-PatchSet: 19
Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-CC: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Fri, 03 Apr 2020 13:42:10 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39980 )
Change subject: soc/intel/skylake: vr_config: enable PSI3 and PSI4 by default
......................................................................
Patch Set 7:
> Patch Set 7:
>
> > Patch Set 7:
> >
> > > Patch Set 7: Code-Review+2
> > >
> > > > Patch Set 7:
> > > >
> > > > > Patch Set 6:
> > > > >
> > > > > whoops, looks like I pushed an older patch when I rebased another change, I'll fix that
> > > >
> > > > Ah, alright. Not a problem (maybe -1 was unnecessary)
> > >
> > > A -1 is not a blocker. In fact, it's a good way to indicate mild dissatisfaction with a change, as it stands out in a field of green scores.
> >
> > but -1 is not a thing that should be used just because of a wrong comment IMHO
>
> There are no real guidelines, what -1 means. So do not interpret too much into it, besides another iteration is needed. -1 scores will get removed with the next iteration, so in my opinion it’s a good way to prevent accidental submission of the change-set. Though, as comments need to be explicitly marked as resolved nowadays, -1 are not needed for that anymore.
> Though, as comments need to be explicitly marked as resolved nowadays,
Yeah, that's what I actually meant but didn't say :)
--
To view, visit https://review.coreboot.org/c/coreboot/+/39980
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I5b5fd9fb3b9b89e80c47f15d706e2dd62dcc0748
Gerrit-Change-Number: 39980
Gerrit-PatchSet: 7
Gerrit-Owner: Michael Niewöhner
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Benjamin Doron <benjamin.doron00(a)gmail.com>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Matthew Garrett <mjg59(a)google.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Fri, 03 Apr 2020 12:25:52 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39980 )
Change subject: soc/intel/skylake: vr_config: enable PSI3 and PSI4 by default
......................................................................
Patch Set 7:
> Patch Set 7:
>
> > Patch Set 7: Code-Review+2
> >
> > > Patch Set 7:
> > >
> > > > Patch Set 6:
> > > >
> > > > whoops, looks like I pushed an older patch when I rebased another change, I'll fix that
> > >
> > > Ah, alright. Not a problem (maybe -1 was unnecessary)
> >
> > A -1 is not a blocker. In fact, it's a good way to indicate mild dissatisfaction with a change, as it stands out in a field of green scores.
>
> but -1 is not a thing that should be used just because of a wrong comment IMHO
There are no real guidelines, what -1 means. So do not interpret too much into it, besides another iteration is needed. -1 scores will get removed with the next iteration, so in my opinion it’s a good way to prevent accidental submission of the change-set. Though, as comments need to be explicitly marked as resolved nowadays, -1 are not needed for that anymore.
--
To view, visit https://review.coreboot.org/c/coreboot/+/39980
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I5b5fd9fb3b9b89e80c47f15d706e2dd62dcc0748
Gerrit-Change-Number: 39980
Gerrit-PatchSet: 7
Gerrit-Owner: Michael Niewöhner
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Benjamin Doron <benjamin.doron00(a)gmail.com>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Matthew Garrett <mjg59(a)google.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Fri, 03 Apr 2020 11:36:53 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30360 )
Change subject: mb/libretrend/lt1000: Add Libretrend LT1000 mainboard
......................................................................
Patch Set 19:
(1 comment)
https://review.coreboot.org/c/coreboot/+/30360/19/Documentation/mainboard/i…
File Documentation/mainboard/index.md:
https://review.coreboot.org/c/coreboot/+/30360/19/Documentation/mainboard/i…
PS19, Line 77:
> Please maintain order, you stole all the ThinkPads below from Lenovo.
CB:40118
--
To view, visit https://review.coreboot.org/c/coreboot/+/30360
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I32fc8a7d3177ba379d04ad8b87adefcfca2b0fab
Gerrit-Change-Number: 30360
Gerrit-PatchSet: 19
Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-CC: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Fri, 03 Apr 2020 11:27:06 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Gerrit-MessageType: comment