Karthik Ramasubramanian has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/39477 )
Change subject: mb/google/dedede: Enable Speed Shift Technology ......................................................................
mb/google/dedede: Enable Speed Shift Technology
BUG=b:151281860 TEST=Boot the mainboard. Ensure that cpufreq driver to configure P-states is enabled in kernel.
Change-Id: Id65d7981501c2f282e564bfc140f8d499d5713e8 Signed-off-by: Karthikeyan Ramasubramanian kramasub@google.com --- M src/mainboard/google/dedede/variants/baseboard/devicetree.cb 1 file changed, 3 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/39477/1
diff --git a/src/mainboard/google/dedede/variants/baseboard/devicetree.cb b/src/mainboard/google/dedede/variants/baseboard/devicetree.cb index 994e96e..3941ea8 100644 --- a/src/mainboard/google/dedede/variants/baseboard/devicetree.cb +++ b/src/mainboard/google/dedede/variants/baseboard/devicetree.cb @@ -115,6 +115,9 @@ register "DdiPortBDdc" = "1" register "DdiPortCDdc" = "1"
+ # Enable Speed Shift Technology support + register "speed_shift_enable" = "1" + # Intel Common SoC Config #+-------------------+---------------------------+ #| Field | Value |
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39477 )
Change subject: mb/google/dedede: Enable Speed Shift Technology ......................................................................
Patch Set 1: Code-Review+2
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39477 )
Change subject: mb/google/dedede: Enable Speed Shift Technology ......................................................................
Patch Set 1: Code-Review+2
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39477 )
Change subject: mb/google/dedede: Enable Speed Shift Technology ......................................................................
Patch Set 1: Code-Review-1
observed instablity with this, please hold the merge
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39477 )
Change subject: mb/google/dedede: Enable Speed Shift Technology ......................................................................
Patch Set 1:
Patch Set 1: Code-Review-1
observed instablity with this, please hold the merge
updated details in b:151408614
Furquan Shaikh has removed a vote from this change. ( https://review.coreboot.org/c/coreboot/+/39477 )
Change subject: mb/google/dedede: Enable Speed Shift Technology ......................................................................
Removed Code-Review+2 by Furquan Shaikh furquan@google.com
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39477 )
Change subject: mb/google/dedede: Enable Speed Shift Technology ......................................................................
Patch Set 1: -Code-Review
Hello build bot (Jenkins), Furquan Shaikh, Justin TerAvest, Tim Wawrzynczak, Aamir Bohra,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39477
to look at the new patch set (#3).
Change subject: mb/google/dedede: Enable Intel Speed Shift Technology ......................................................................
mb/google/dedede: Enable Intel Speed Shift Technology
Enable Intel Speed Shift Technology (ISST) by default. Disable ISST in waddledee and waddledoo variants on early phases.
BUG=b:151281860 TEST=Build and Boot the mainboard. Ensure that cpufreq driver to configure P-states is enabled in kernel on boards where board version is provisioned.
Change-Id: Id65d7981501c2f282e564bfc140f8d499d5713e8 Signed-off-by: Karthikeyan Ramasubramanian kramasub@google.com --- M src/mainboard/google/dedede/mainboard.c M src/mainboard/google/dedede/variants/baseboard/devicetree.cb M src/mainboard/google/dedede/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/dedede/variants/waddledee/Makefile.inc A src/mainboard/google/dedede/variants/waddledee/variant.c M src/mainboard/google/dedede/variants/waddledoo/Makefile.inc A src/mainboard/google/dedede/variants/waddledoo/variant.c 7 files changed, 53 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/39477/3
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39477 )
Change subject: mb/google/dedede: Enable Intel Speed Shift Technology ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39477/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39477/3//COMMIT_MSG@13 PS3, Line 13: Boot boot
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39477 )
Change subject: mb/google/dedede: Enable Intel Speed Shift Technology ......................................................................
Patch Set 3:
Just curious, is it worth adding this, in the hopes that it will not be needed later? Is it critical that all builds have speed shift enabled? Or can you live with a slower boot for a while?
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39477 )
Change subject: mb/google/dedede: Enable Intel Speed Shift Technology ......................................................................
Patch Set 3:
Patch Set 3:
Just curious, is it worth adding this, in the hopes that it will not be needed later? Is it critical that all builds have speed shift enabled? Or can you live with a slower boot for a while?
We have an active mix of older and newer builds. In older builds if you have this enabled there is a known system instability. Hence we want it disabled for the benefit of those using older builds.
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Furquan Shaikh, Justin TerAvest, Tim Wawrzynczak, Aamir Bohra,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39477
to look at the new patch set (#4).
Change subject: mb/google/dedede: Enable Intel Speed Shift Technology ......................................................................
mb/google/dedede: Enable Intel Speed Shift Technology
Enable Intel Speed Shift Technology (ISST) by default. Disable ISST in waddledee and waddledoo variants on early phases.
BUG=b:151281860 TEST=Build and Boot the mainboard. Ensure that cpufreq driver to configure P-states is enabled in kernel on boards where board version is provisioned.
Change-Id: Id65d7981501c2f282e564bfc140f8d499d5713e8 Signed-off-by: Karthikeyan Ramasubramanian kramasub@google.com --- M src/mainboard/google/dedede/mainboard.c M src/mainboard/google/dedede/variants/baseboard/devicetree.cb M src/mainboard/google/dedede/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/dedede/variants/waddledee/Makefile.inc A src/mainboard/google/dedede/variants/waddledee/variant.c M src/mainboard/google/dedede/variants/waddledoo/Makefile.inc A src/mainboard/google/dedede/variants/waddledoo/variant.c 7 files changed, 56 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/39477/4
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Furquan Shaikh, Justin TerAvest, Tim Wawrzynczak, Aamir Bohra,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39477
to look at the new patch set (#5).
Change subject: mb/google/dedede: Enable Intel Speed Shift Technology ......................................................................
mb/google/dedede: Enable Intel Speed Shift Technology
Enable Intel Speed Shift Technology (ISST) by default. Disable ISST in waddledee and waddledoo variants on early phases.
BUG=b:151281860 TEST=Build and boot the mainboard. Ensure that cpufreq driver to configure P-states is enabled in kernel on boards where board version is provisioned.
Change-Id: Id65d7981501c2f282e564bfc140f8d499d5713e8 Signed-off-by: Karthikeyan Ramasubramanian kramasub@google.com --- M src/mainboard/google/dedede/mainboard.c M src/mainboard/google/dedede/variants/baseboard/devicetree.cb M src/mainboard/google/dedede/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/dedede/variants/waddledee/Makefile.inc A src/mainboard/google/dedede/variants/waddledee/variant.c M src/mainboard/google/dedede/variants/waddledoo/Makefile.inc A src/mainboard/google/dedede/variants/waddledoo/variant.c 7 files changed, 56 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/39477/5
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39477 )
Change subject: mb/google/dedede: Enable Intel Speed Shift Technology ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39477/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39477/3//COMMIT_MSG@13 PS3, Line 13: Boot
boot
Done
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39477 )
Change subject: mb/google/dedede: Enable Intel Speed Shift Technology ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39477/5/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/waddledee/variant.c:
https://review.coreboot.org/c/coreboot/+/39477/5/src/mainboard/google/dedede... PS5, Line 13: where board version is not populated. Does it always return -1 or can the return value be 0 and board_ver = 0xffffffff?
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39477 )
Change subject: mb/google/dedede: Enable Intel Speed Shift Technology ......................................................................
Patch Set 5:
Not to be too much of a stickler, but this is the whole reason for mainboard_silicon_init_params(FSP_S_CONFIG *params). It gives you a chance to update FSP_S UPDs before FSP-S runs. And if 'dee and 'doo are all the variants you have right now, you could just leave the check and setting of ISST in mainboard.c for now.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39477 )
Change subject: mb/google/dedede: Enable Intel Speed Shift Technology ......................................................................
Patch Set 5:
Patch Set 5:
Not to be too much of a stickler, but this is the whole reason for mainboard_silicon_init_params(FSP_S_CONFIG *params). It gives you a chance to update FSP_S UPDs before FSP-S runs. And if 'dee and 'doo are all the variants you have right now, you could just leave the check and setting of ISST in mainboard.c for now.
My mistake, I had thought FSP-S was setting up ISST.
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39477 )
Change subject: mb/google/dedede: Enable Intel Speed Shift Technology ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39477/5/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/waddledee/variant.c:
https://review.coreboot.org/c/coreboot/+/39477/5/src/mainboard/google/dedede... PS5, Line 13: where board version is not populated.
Does it always return -1 or can the return value be 0 and board_ver = 0xffffffff?
If EC returns error, get board version returns -1. EC returns error on the boards where board version is not populated.
EC Logs: 2020-05-20 11:44:07 [1.679129 Failed (1) getting board version] 2020-05-20 11:44:07 [1.679664 HC 0x06 err 2]
BIOS logs: EC returned error result code 2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39477 )
Change subject: mb/google/dedede: Enable Intel Speed Shift Technology ......................................................................
Patch Set 5:
(2 comments)
Patch Set 5:
Not to be too much of a stickler, but this is the whole reason for mainboard_silicon_init_params(FSP_S_CONFIG *params). It gives you a chance to update FSP_S UPDs before FSP-S runs. And if 'dee and 'doo are all the variants you have right now, you could just leave the check and setting of ISST in mainboard.c for now.
I think what Karthik identified was yyou
https://review.coreboot.org/c/coreboot/+/39477/5/src/mainboard/google/dedede... File src/mainboard/google/dedede/mainboard.c:
https://review.coreboot.org/c/coreboot/+/39477/5/src/mainboard/google/dedede... PS5, Line 12: /* Nothing to override */ It would be helpful to add a comment indicating when this is required to be implemented by a variant. It would be helpful as we add new variants to ensure that they don't end up copying waddledee/waddledoo.
https://review.coreboot.org/c/coreboot/+/39477/5/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/waddledee/variant.c:
https://review.coreboot.org/c/coreboot/+/39477/5/src/mainboard/google/dedede... PS5, Line 13: where board version is not populated.
If EC returns error, get board version returns -1. […]
Thanks for checking Karthik!
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39477 )
Change subject: mb/google/dedede: Enable Intel Speed Shift Technology ......................................................................
Patch Set 5: Code-Review+2
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39477 )
Change subject: mb/google/dedede: Enable Intel Speed Shift Technology ......................................................................
Patch Set 5:
Patch Set 5:
(2 comments)
Patch Set 5:
Not to be too much of a stickler, but this is the whole reason for mainboard_silicon_init_params(FSP_S_CONFIG *params). It gives you a chance to update FSP_S UPDs before FSP-S runs. And if 'dee and 'doo are all the variants you have right now, you could just leave the check and setting of ISST in mainboard.c for now.
I think what Karthik identified was yyou
Karthik and I chatted about this, it was a misunderstanding on my part.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39477 )
Change subject: mb/google/dedede: Enable Intel Speed Shift Technology ......................................................................
Patch Set 5:
Patch Set 5:
Patch Set 5:
(2 comments)
Patch Set 5:
Not to be too much of a stickler, but this is the whole reason for mainboard_silicon_init_params(FSP_S_CONFIG *params). It gives you a chance to update FSP_S UPDs before FSP-S runs. And if 'dee and 'doo are all the variants you have right now, you could just leave the check and setting of ISST in mainboard.c for now.
I think what Karthik identified was yyou
Karthik and I chatted about this, it was a misunderstanding on my part.
Woops.. I had typed that and removed it after you posted your latest comment, but somehow it didn't get removed completely.
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39477 )
Change subject: mb/google/dedede: Enable Intel Speed Shift Technology ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39477/5/src/mainboard/google/dedede... File src/mainboard/google/dedede/mainboard.c:
https://review.coreboot.org/c/coreboot/+/39477/5/src/mainboard/google/dedede... PS5, Line 12: /* Nothing to override */
It would be helpful to add a comment indicating when this is required to be implemented by a variant […]
Agreed. I will add a comment.
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth, Justin TerAvest, Tim Wawrzynczak, Aamir Bohra,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39477
to look at the new patch set (#6).
Change subject: mb/google/dedede: Enable Intel Speed Shift Technology ......................................................................
mb/google/dedede: Enable Intel Speed Shift Technology
Enable Intel Speed Shift Technology (ISST) by default. Disable ISST in waddledee and waddledoo variants on early phases.
BUG=b:151281860 TEST=Build and boot the mainboard. Ensure that cpufreq driver to configure P-states is enabled in kernel on boards where board version is provisioned.
Change-Id: Id65d7981501c2f282e564bfc140f8d499d5713e8 Signed-off-by: Karthikeyan Ramasubramanian kramasub@google.com --- M src/mainboard/google/dedede/mainboard.c M src/mainboard/google/dedede/variants/baseboard/devicetree.cb M src/mainboard/google/dedede/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/dedede/variants/waddledee/Makefile.inc A src/mainboard/google/dedede/variants/waddledee/variant.c M src/mainboard/google/dedede/variants/waddledoo/Makefile.inc A src/mainboard/google/dedede/variants/waddledoo/variant.c 7 files changed, 59 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/39477/6
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39477 )
Change subject: mb/google/dedede: Enable Intel Speed Shift Technology ......................................................................
Patch Set 6:
(2 comments)
https://review.coreboot.org/c/coreboot/+/39477/5/src/mainboard/google/dedede... File src/mainboard/google/dedede/mainboard.c:
https://review.coreboot.org/c/coreboot/+/39477/5/src/mainboard/google/dedede... PS5, Line 12: /* Nothing to override */
Agreed. I will add a comment.
Done
https://review.coreboot.org/c/coreboot/+/39477/5/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/waddledee/variant.c:
https://review.coreboot.org/c/coreboot/+/39477/5/src/mainboard/google/dedede... PS5, Line 13: where board version is not populated.
Thanks for checking Karthik!
Done
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39477 )
Change subject: mb/google/dedede: Enable Intel Speed Shift Technology ......................................................................
Patch Set 6: Code-Review+2
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39477 )
Change subject: mb/google/dedede: Enable Intel Speed Shift Technology ......................................................................
Patch Set 6: Code-Review+2
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39477 )
Change subject: mb/google/dedede: Enable Intel Speed Shift Technology ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39477/6/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/waddledee/variant.c:
https://review.coreboot.org/c/coreboot/+/39477/6/src/mainboard/google/dedede... PS6, Line 17: Do we really required two varinat files, one under waddledee/variant.c and waddledoo/variant.c I am not finiding any difference between these two. Is it possible to move this variant.c file under baseboard OR am I missing here something ?
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39477 )
Change subject: mb/google/dedede: Enable Intel Speed Shift Technology ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39477/6/src/mainboard/google/dedede... File src/mainboard/google/dedede/mainboard.c:
https://review.coreboot.org/c/coreboot/+/39477/6/src/mainboard/google/dedede... PS6, Line 10: __weak void variant_isst_override(void) Do we need this weak function? Why not put the code inside mainboard_config_isst() directly?
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39477 )
Change subject: mb/google/dedede: Enable Intel Speed Shift Technology ......................................................................
Patch Set 6:
(2 comments)
https://review.coreboot.org/c/coreboot/+/39477/6/src/mainboard/google/dedede... File src/mainboard/google/dedede/mainboard.c:
https://review.coreboot.org/c/coreboot/+/39477/6/src/mainboard/google/dedede... PS6, Line 10: __weak void variant_isst_override(void)
Do we need this weak function? Why not put the code inside mainboard_config_isst() directly?
Only 2 variant boards and that too only in initial board phases use early revisions of Silicon. Also not every variant board that does not have the board version populated use the early Silicon revision. Hence the weak override is added.
https://review.coreboot.org/c/coreboot/+/39477/6/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/waddledee/variant.c:
https://review.coreboot.org/c/coreboot/+/39477/6/src/mainboard/google/dedede... PS6, Line 17:
Do we really required two varinat files, one under waddledee/variant.c and waddledoo/variant.c […]
Only 2 variant boards and that too only in initial board phases use early revisions of Silicon. Also not every variant board that does not have the board version populated use the early Silicon revision. Hence the weak override is added.
Moving it to the baseboard will apply for all new boards, which is not expected.
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39477 )
Change subject: mb/google/dedede: Enable Intel Speed Shift Technology ......................................................................
Patch Set 6: Code-Review+1
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39477 )
Change subject: mb/google/dedede: Enable Intel Speed Shift Technology ......................................................................
Patch Set 6:
(2 comments)
https://review.coreboot.org/c/coreboot/+/39477/6/src/mainboard/google/dedede... File src/mainboard/google/dedede/mainboard.c:
https://review.coreboot.org/c/coreboot/+/39477/6/src/mainboard/google/dedede... PS6, Line 10: __weak void variant_isst_override(void)
Only 2 variant boards and that too only in initial board phases use early revisions of Silicon. […]
Done
https://review.coreboot.org/c/coreboot/+/39477/6/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/waddledee/variant.c:
https://review.coreboot.org/c/coreboot/+/39477/6/src/mainboard/google/dedede... PS6, Line 17:
Only 2 variant boards and that too only in initial board phases use early revisions of Silicon. […]
Done
Furquan Shaikh has submitted this change. ( https://review.coreboot.org/c/coreboot/+/39477 )
Change subject: mb/google/dedede: Enable Intel Speed Shift Technology ......................................................................
mb/google/dedede: Enable Intel Speed Shift Technology
Enable Intel Speed Shift Technology (ISST) by default. Disable ISST in waddledee and waddledoo variants on early phases.
BUG=b:151281860 TEST=Build and boot the mainboard. Ensure that cpufreq driver to configure P-states is enabled in kernel on boards where board version is provisioned.
Change-Id: Id65d7981501c2f282e564bfc140f8d499d5713e8 Signed-off-by: Karthikeyan Ramasubramanian kramasub@google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/39477 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Furquan Shaikh furquan@google.com Reviewed-by: Aamir Bohra aamir.bohra@intel.com Reviewed-by: Sumeet R Pawnikar sumeet.r.pawnikar@intel.com --- M src/mainboard/google/dedede/mainboard.c M src/mainboard/google/dedede/variants/baseboard/devicetree.cb M src/mainboard/google/dedede/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/dedede/variants/waddledee/Makefile.inc A src/mainboard/google/dedede/variants/waddledee/variant.c M src/mainboard/google/dedede/variants/waddledoo/Makefile.inc A src/mainboard/google/dedede/variants/waddledoo/variant.c 7 files changed, 59 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved Sumeet R Pawnikar: Looks good to me, but someone else must approve Aamir Bohra: Looks good to me, approved
diff --git a/src/mainboard/google/dedede/mainboard.c b/src/mainboard/google/dedede/mainboard.c index c503a86..fe89527 100644 --- a/src/mainboard/google/dedede/mainboard.c +++ b/src/mainboard/google/dedede/mainboard.c @@ -1,11 +1,25 @@ /* SPDX-License-Identifier: GPL-2.0-or-later */
#include <acpi/acpi.h> +#include <bootstate.h> #include <baseboard/variants.h> #include <device/device.h> #include <ec/ec.h> #include <vendorcode/google/chromeos/chromeos.h>
+__weak void variant_isst_override(void) +{ + /* + * Implement the override only if the board uses very early/initial revisions of + * Silicon. Otherwise nothing to override. + */ +} + +static void mainboard_config_isst(void *unused) +{ + variant_isst_override(); +} + static void mainboard_init(void *chip_info) { const struct pad_config *pads; @@ -37,3 +51,6 @@ .init = mainboard_init, .enable_dev = mainboard_enable, }; + +/* Configure ISST before CPU initialization */ +BOOT_STATE_INIT_ENTRY(BS_PRE_DEVICE, BS_ON_ENTRY, mainboard_config_isst, NULL); diff --git a/src/mainboard/google/dedede/variants/baseboard/devicetree.cb b/src/mainboard/google/dedede/variants/baseboard/devicetree.cb index c891e6e..1b42dfb 100644 --- a/src/mainboard/google/dedede/variants/baseboard/devicetree.cb +++ b/src/mainboard/google/dedede/variants/baseboard/devicetree.cb @@ -133,6 +133,9 @@ register "DdiPortBDdc" = "1" register "DdiPortCDdc" = "1"
+ # Enable Speed Shift Technology support + register "speed_shift_enable" = "1" + # Intel Common SoC Config #+-------------------+---------------------------+ #| Field | Value | diff --git a/src/mainboard/google/dedede/variants/baseboard/include/baseboard/variants.h b/src/mainboard/google/dedede/variants/baseboard/include/baseboard/variants.h index 3fdc782..5d8355b 100644 --- a/src/mainboard/google/dedede/variants/baseboard/include/baseboard/variants.h +++ b/src/mainboard/google/dedede/variants/baseboard/include/baseboard/variants.h @@ -39,4 +39,7 @@ */ bool variant_mem_is_half_populated(void);
+/* Variant Intel Speed Shift Technology override */ +void variant_isst_override(void); + #endif /*__BASEBOARD_VARIANTS_H__ */ diff --git a/src/mainboard/google/dedede/variants/waddledee/Makefile.inc b/src/mainboard/google/dedede/variants/waddledee/Makefile.inc index dfb97ba..aaa65e2 100644 --- a/src/mainboard/google/dedede/variants/waddledee/Makefile.inc +++ b/src/mainboard/google/dedede/variants/waddledee/Makefile.inc @@ -4,3 +4,5 @@ SPD_SOURCES += empty #0b0001
romstage-y += memory.c + +ramstage-y += variant.c diff --git a/src/mainboard/google/dedede/variants/waddledee/variant.c b/src/mainboard/google/dedede/variants/waddledee/variant.c new file mode 100644 index 0000000..ac3cf58 --- /dev/null +++ b/src/mainboard/google/dedede/variants/waddledee/variant.c @@ -0,0 +1,16 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +#include <baseboard/variants.h> +#include <chip.h> +#include <device/device.h> +#include <ec/google/chromeec/ec.h> + +void variant_isst_override(void) +{ + config_t *cfg = config_of_soc(); + uint32_t board_ver; + + /* Override/Disable ISST in boards where board version is not populated. */ + if (google_chromeec_get_board_version(&board_ver)) + cfg->speed_shift_enable = 0; +} diff --git a/src/mainboard/google/dedede/variants/waddledoo/Makefile.inc b/src/mainboard/google/dedede/variants/waddledoo/Makefile.inc index 922c314..11bbbd6 100644 --- a/src/mainboard/google/dedede/variants/waddledoo/Makefile.inc +++ b/src/mainboard/google/dedede/variants/waddledoo/Makefile.inc @@ -4,3 +4,5 @@ SPD_SOURCES += SPD_LPDDR4X_200b_8Gb_4267_DDP_1x16 #0b0001
romstage-y += memory.c + +ramstage-y += variant.c diff --git a/src/mainboard/google/dedede/variants/waddledoo/variant.c b/src/mainboard/google/dedede/variants/waddledoo/variant.c new file mode 100644 index 0000000..ac3cf58 --- /dev/null +++ b/src/mainboard/google/dedede/variants/waddledoo/variant.c @@ -0,0 +1,16 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +#include <baseboard/variants.h> +#include <chip.h> +#include <device/device.h> +#include <ec/google/chromeec/ec.h> + +void variant_isst_override(void) +{ + config_t *cfg = config_of_soc(); + uint32_t board_ver; + + /* Override/Disable ISST in boards where board version is not populated. */ + if (google_chromeec_get_board_version(&board_ver)) + cfg->speed_shift_enable = 0; +}
-
Aamir Bohra (Code Review) -
Angel Pons (Code Review)
-
Furquan Shaikh (Code Review)
-
Karthik Ramasubramanian (Code Review)
-
Paul Menzel (Code Review)
-
Sumeet R Pawnikar (Code Review)
-
Tim Wawrzynczak (Code Review)