Maxim Polyakov has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35166 )
Change subject: soc/skl/vr_config: add missed Icc max levels ......................................................................
soc/skl/vr_config: add missed Icc max levels
Icc_max for VR domains in Kaby Lake S processors is installed based on the processor TDP [1]. But, according to the DC Current Specifications for graphics processor [2], Icc_max GT should take several values:
+---------------------+-----+------------+ | Segment | TDP | Icc_max GT | +---------------------+-----+------------+ | Dual Core GT2/GT1 | 35W | | | Dual Core GT2 | 51W | 48 A | | Dual Core GT1 | 54W | | +---------------------+-----+------------+ | Quad Core GT2 | 35W | 35 A | +---------------------+-----+------------+ | Quad Core GT2 | 65W | 45 A | | Quad Core GT2 K-SKU | 91W | | +---------------------+-----+------------+
This patch fixes Icc_max current limits for a corresponding graphical VR domain
[1] Change-Id: I303c5dc8ed03e9a98a834a2acfb400022dfc2fde [2] 7th Generation Intel(R) Processor Families for S Platforms and Intel(R) Core(TM) X-Series Processor Family Datasheet, Volume 1, December 2018, Document Number: 335195-003
Change-Id: I19766e4f8fab6b48565b65ed4cf13efbc213e654 Signed-off-by: Maxim Polyakov max.senia.poliak@gmail.com --- M src/soc/intel/skylake/vr_config.c 1 file changed, 13 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/35166/1
diff --git a/src/soc/intel/skylake/vr_config.c b/src/soc/intel/skylake/vr_config.c index 089dd5d..fc48558 100644 --- a/src/soc/intel/skylake/vr_config.c +++ b/src/soc/intel/skylake/vr_config.c @@ -117,12 +117,12 @@ * +----------------+-------------+---------------+------+-----+ * | Domain/Setting | SA | IA | GTUS | GTS | * +----------------+-------------+---------------+------+-----+ - * | IccMax(KBL-S) | 11.1A | 100A | 45A | 45A | - * | | | ... | | | - * | | | 40A | | | + * | IccMax(KBL-S) | 11.1A | 100A | 48A | 48A | + * | | | ... | 45A | 45A | + * | | | 40A | 35A | 35A | * +----------------+-------------+---------------+------+-----+ - * | IccMax(KBL-H) | 11.1A(45W) | 68A | 55A | 55A | - * | | 6.6A(Others)| 60A | | | + * | IccMax(KBL-H) | 11.1A (45W) | 68A | 55A | 55A | + * | | 6.6A (18W) | 60A | | | * +----------------+-------------+---------------+------+-----+ * | IccMax(KBL-U/R)| 6A(U42) | 64A(U42) | 31A | 31A | * | | 4.5A(Others)| 29A(P/C) | | | @@ -139,8 +139,8 @@ uint16_t icc_max[NUM_VR_DOMAINS] = { VR_CFG_AMP(11.1), VR_CFG_AMP(40), - VR_CFG_AMP(45), - VR_CFG_AMP(45), + VR_CFG_AMP(48), + VR_CFG_AMP(48), }; if (tdp >= 54) icc_max[VR_IA_CORE] = VR_CFG_AMP(58); @@ -155,13 +155,17 @@ uint16_t icc_max[NUM_VR_DOMAINS] = { VR_CFG_AMP(11.1), VR_CFG_AMP(66), - VR_CFG_AMP(55), - VR_CFG_AMP(55), + VR_CFG_AMP(45), + VR_CFG_AMP(45), }; if (tdp >= 91) icc_max[VR_IA_CORE] = VR_CFG_AMP(100); else if (tdp >= 65) icc_max[VR_IA_CORE] = VR_CFG_AMP(79); + else if (tdp >= 35) { + icc_max[VR_GT_UNSLICED] = VR_CFG_AMP(35); + icc_max[VR_GT_SLICED] = VR_CFG_AMP(35); + }
return icc_max[domain]; }
Hello Patrick Rudolph, Angel Pons, Matt DeVillier, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35166
to look at the new patch set (#2).
Change subject: soc/skl/vr_config: add missed Icc max levels ......................................................................
soc/skl/vr_config: add missed Icc max levels
Icc_max for VR domains in Kaby Lake S processors is set based on the processor TDP [1]. But, according to the DC Current Specifications for graphics processor [2], Icc_max GT should take several values:
+---------------------+-----+------------+ | Segment | TDP | Icc_max GT | +---------------------+-----+------------+ | Dual Core GT2/GT1 | 35W | | | Dual Core GT2 | 51W | 48 A | | Dual Core GT1 | 54W | | +---------------------+-----+------------+ | Quad Core GT2 | 35W | 35 A | +---------------------+-----+------------+ | Quad Core GT2 | 65W | 45 A | | Quad Core GT2 K-SKU | 91W | | +---------------------+-----+------------+
This patch add missing current limits for a corresponding graphical VR domain
[1] Change-Id: I303c5dc8ed03e9a98a834a2acfb400022dfc2fde [2] 7th Generation Intel(R) Processor Families for S Platforms and Intel(R) Core(TM) X-Series Processor Family Datasheet, Volume 1, December 2018, Document Number: 335195-003
Change-Id: I19766e4f8fab6b48565b65ed4cf13efbc213e654 Signed-off-by: Maxim Polyakov max.senia.poliak@gmail.com --- M src/soc/intel/skylake/vr_config.c 1 file changed, 13 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/35166/2
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35166 )
Change subject: soc/skl/vr_config: add missed Icc max levels ......................................................................
Patch Set 2: Code-Review+1
See comment about using a 2D array on CB:35167
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35166 )
Change subject: soc/skl/vr_config: add missed Icc max levels ......................................................................
Patch Set 6:
(4 comments)
Tested how?
https://review.coreboot.org/c/coreboot/+/35166/6//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35166/6//COMMIT_MSG@7 PS6, Line 7: soc/skl/vr_config: add missed Icc max levels Maybe:
Increase Icc max level to 48 A
https://review.coreboot.org/c/coreboot/+/35166/6//COMMIT_MSG@26 PS6, Line 26: add adds
https://review.coreboot.org/c/coreboot/+/35166/6//COMMIT_MSG@27 PS6, Line 27: VR domain Dot/period at the end please.
https://review.coreboot.org/c/coreboot/+/35166/6/src/soc/intel/skylake/vr_co... File src/soc/intel/skylake/vr_config.c:
https://review.coreboot.org/c/coreboot/+/35166/6/src/soc/intel/skylake/vr_co... PS6, Line 143: VR_CFG_AMP(48), This should be mentioned in the commit message.
Hello Patrick Rudolph, Angel Pons, Matt DeVillier, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35166
to look at the new patch set (#7).
Change subject: soc/skl/vr_config: set Iccmax_gt depends on CPU/GT ......................................................................
soc/skl/vr_config: set Iccmax_gt depends on CPU/GT
According to the DC Current Specifications [1], the current limit for the graphical VR domain (Iccmax_gt) isn't same for different Kaby Lake S CPUs. This value should depend on the iGPU model and processor TDP:
+---------------------+-----+------------+ | Segment | TDP | Icc_max GT | +---------------------+-----+------------+ | Dual Core GT2/GT1 | 35W | | | Dual Core GT2 | 51W | 48 A | | Dual Core GT1 | 54W | | +---------------------+-----+------------+ | Quad Core GT2 | 35W | 35 A | +---------------------+-----+------------+ | Quad Core GT2 | 65W | 45 A | | Quad Core GT2 K-SKU | 91W | | +---------------------+-----+------------+
This patch adds the remaining Iccmax_gt current limit values from the documentation [1].
[1] 7th Generation Intel(R) Processor Families for S Platforms and Intel(R) Core(TM) X-Series Processor Family Datasheet, Volume 1, December 2018, Document Number: 335195-003
Change-Id: I19766e4f8fab6b48565b65ed4cf13efbc213e654 Signed-off-by: Maxim Polyakov max.senia.poliak@gmail.com --- M src/soc/intel/skylake/vr_config.c 1 file changed, 13 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/35166/7
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35166 )
Change subject: soc/skl/vr_config: set Iccmax_gt depends on CPU/GT ......................................................................
Patch Set 7: Code-Review+2
Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35166 )
Change subject: soc/skl/vr_config: set Iccmax_gt depends on CPU/GT ......................................................................
Patch Set 7:
(4 comments)
Patch Set 6:
(4 comments)
Tested how?
Tested on Asrock motherboard with Core i5-7400 (65W)
https://review.coreboot.org/c/coreboot/+/35166/6//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35166/6//COMMIT_MSG@7 PS6, Line 7: soc/skl/vr_config: add missed Icc max levels
Maybe: […]
I think a more appropriate commit message is "set Iccmax_gt depends on CPU/GT" I should explain why I use several arguments instead of one
https://review.coreboot.org/c/coreboot/+/35166/6//COMMIT_MSG@26 PS6, Line 26: add
adds
Done
https://review.coreboot.org/c/coreboot/+/35166/6//COMMIT_MSG@27 PS6, Line 27: VR domain
Dot/period at the end please.
Done
https://review.coreboot.org/c/coreboot/+/35166/6/src/soc/intel/skylake/vr_co... File src/soc/intel/skylake/vr_config.c:
https://review.coreboot.org/c/coreboot/+/35166/6/src/soc/intel/skylake/vr_co... PS6, Line 143: VR_CFG_AMP(48),
This should be mentioned in the commit message.
Done
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/35166 )
Change subject: soc/skl/vr_config: set Iccmax_gt depends on CPU/GT ......................................................................
soc/skl/vr_config: set Iccmax_gt depends on CPU/GT
According to the DC Current Specifications [1], the current limit for the graphical VR domain (Iccmax_gt) isn't same for different Kaby Lake S CPUs. This value should depend on the iGPU model and processor TDP:
+---------------------+-----+------------+ | Segment | TDP | Icc_max GT | +---------------------+-----+------------+ | Dual Core GT2/GT1 | 35W | | | Dual Core GT2 | 51W | 48 A | | Dual Core GT1 | 54W | | +---------------------+-----+------------+ | Quad Core GT2 | 35W | 35 A | +---------------------+-----+------------+ | Quad Core GT2 | 65W | 45 A | | Quad Core GT2 K-SKU | 91W | | +---------------------+-----+------------+
This patch adds the remaining Iccmax_gt current limit values from the documentation [1].
[1] 7th Generation Intel(R) Processor Families for S Platforms and Intel(R) Core(TM) X-Series Processor Family Datasheet, Volume 1, December 2018, Document Number: 335195-003
Change-Id: I19766e4f8fab6b48565b65ed4cf13efbc213e654 Signed-off-by: Maxim Polyakov max.senia.poliak@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/35166 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Patrick Rudolph siro@das-labor.org --- M src/soc/intel/skylake/vr_config.c 1 file changed, 13 insertions(+), 9 deletions(-)
Approvals: build bot (Jenkins): Verified Patrick Rudolph: Looks good to me, approved
diff --git a/src/soc/intel/skylake/vr_config.c b/src/soc/intel/skylake/vr_config.c index 089dd5d..fc48558 100644 --- a/src/soc/intel/skylake/vr_config.c +++ b/src/soc/intel/skylake/vr_config.c @@ -117,12 +117,12 @@ * +----------------+-------------+---------------+------+-----+ * | Domain/Setting | SA | IA | GTUS | GTS | * +----------------+-------------+---------------+------+-----+ - * | IccMax(KBL-S) | 11.1A | 100A | 45A | 45A | - * | | | ... | | | - * | | | 40A | | | + * | IccMax(KBL-S) | 11.1A | 100A | 48A | 48A | + * | | | ... | 45A | 45A | + * | | | 40A | 35A | 35A | * +----------------+-------------+---------------+------+-----+ - * | IccMax(KBL-H) | 11.1A(45W) | 68A | 55A | 55A | - * | | 6.6A(Others)| 60A | | | + * | IccMax(KBL-H) | 11.1A (45W) | 68A | 55A | 55A | + * | | 6.6A (18W) | 60A | | | * +----------------+-------------+---------------+------+-----+ * | IccMax(KBL-U/R)| 6A(U42) | 64A(U42) | 31A | 31A | * | | 4.5A(Others)| 29A(P/C) | | | @@ -139,8 +139,8 @@ uint16_t icc_max[NUM_VR_DOMAINS] = { VR_CFG_AMP(11.1), VR_CFG_AMP(40), - VR_CFG_AMP(45), - VR_CFG_AMP(45), + VR_CFG_AMP(48), + VR_CFG_AMP(48), }; if (tdp >= 54) icc_max[VR_IA_CORE] = VR_CFG_AMP(58); @@ -155,13 +155,17 @@ uint16_t icc_max[NUM_VR_DOMAINS] = { VR_CFG_AMP(11.1), VR_CFG_AMP(66), - VR_CFG_AMP(55), - VR_CFG_AMP(55), + VR_CFG_AMP(45), + VR_CFG_AMP(45), }; if (tdp >= 91) icc_max[VR_IA_CORE] = VR_CFG_AMP(100); else if (tdp >= 65) icc_max[VR_IA_CORE] = VR_CFG_AMP(79); + else if (tdp >= 35) { + icc_max[VR_GT_UNSLICED] = VR_CFG_AMP(35); + icc_max[VR_GT_SLICED] = VR_CFG_AMP(35); + }
return icc_max[domain]; }
-
Angel Pons (Code Review) -
Maxim Polyakov (Code Review)
-
Patrick Georgi (Code Review)
-
Patrick Rudolph (Code Review)
-
Paul Menzel (Code Review)