Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37466 )
Change subject: soc/intel/cannonlake: Add VR config for CFL and WHL ......................................................................
soc/intel/cannonlake: Add VR config for CFL and WHL
Add VR config IccMax, DC and AC loadline defaults for all CFL and WHL.
I couldn't find CNL datasheet, but it looks like only a single SoC without graphics was ever produced.
I couldn't find CML datasheet, so those are left out for now.
Used Doc #337344 and #338023 Section 7.
Change-Id: I1d2e174157d468830cc0baf2a2d8295ef61a1a63 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/soc/intel/cannonlake/include/soc/vr_config.h M src/soc/intel/cannonlake/vr_config.c 2 files changed, 236 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/37466/1
diff --git a/src/soc/intel/cannonlake/include/soc/vr_config.h b/src/soc/intel/cannonlake/include/soc/vr_config.h index 8bcf001..4cb35a0 100644 --- a/src/soc/intel/cannonlake/include/soc/vr_config.h +++ b/src/soc/intel/cannonlake/include/soc/vr_config.h @@ -56,6 +56,7 @@ };
#define VR_CFG_AMP(i) ((i) * 4) +#define VR_CFG_MOHMS(i) (uint16_t)((i) * 100)
/* VrConfig Settings for 4 domains * 0 = System Agent, 1 = IA Core, @@ -68,6 +69,22 @@ NUM_VR_DOMAINS };
+#define VR_CFG_ALL_DOMAINS_ICC(sa, ia, gt_unsl, gt_sl) \ + { \ + [VR_SYSTEM_AGENT] = VR_CFG_AMP(sa), \ + [VR_IA_CORE] = VR_CFG_AMP(ia), \ + [VR_GT_UNSLICED] = VR_CFG_AMP(gt_unsl), \ + [VR_GT_SLICED] = VR_CFG_AMP(gt_sl), \ + } + +#define VR_CFG_ALL_DOMAINS_LOADLINE(sa, ia, gt_unsl, gt_sl) \ + { \ + [VR_SYSTEM_AGENT] = VR_CFG_MOHMS(sa), \ + [VR_IA_CORE] = VR_CFG_MOHMS(ia), \ + [VR_GT_UNSLICED] = VR_CFG_MOHMS(gt_unsl), \ + [VR_GT_SLICED] = VR_CFG_MOHMS(gt_sl), \ + } + void fill_vr_domain_config(void *params, int domain, const struct vr_config *cfg);
diff --git a/src/soc/intel/cannonlake/vr_config.c b/src/soc/intel/cannonlake/vr_config.c index 25270d8..035a270 100644 --- a/src/soc/intel/cannonlake/vr_config.c +++ b/src/soc/intel/cannonlake/vr_config.c @@ -14,9 +14,13 @@ * */
+#include <device/pci_ids.h> +#include <device/pci_ops.h> #include <fsp/api.h> #include <soc/ramstage.h> #include <soc/vr_config.h> +#include <console/console.h> +#include <intelblocks/cpulib.h>
static const struct vr_config default_configs[NUM_VR_DOMAINS] = { [VR_SYSTEM_AGENT] = { @@ -28,7 +32,7 @@ .psi4enable = 1, .imon_slope = 0x0, .imon_offset = 0x0, - .icc_max = VR_CFG_AMP(6), + .icc_max = 0, .voltage_limit = 1520, }, [VR_IA_CORE] = { @@ -40,7 +44,7 @@ .psi4enable = 1, .imon_slope = 0x0, .imon_offset = 0x0, - .icc_max = VR_CFG_AMP(70), + .icc_max = 0, .voltage_limit = 1520, }, [VR_GT_UNSLICED] = { @@ -52,7 +56,7 @@ .psi4enable = 1, .imon_slope = 0x0, .imon_offset = 0x0, - .icc_max = VR_CFG_AMP(31), + .icc_max = 0, .voltage_limit = 1520, }, [VR_GT_SLICED] = { @@ -64,11 +68,207 @@ .psi4enable = 1, .imon_slope = 0x0, .imon_offset = 0x0, - .icc_max = VR_CFG_AMP(31), + .icc_max = 0, .voltage_limit = 1520, }, };
+ +static uint16_t get_sku_icc_max(int domain) +{ + const uint16_t tdp = cpu_get_power_max(); + + static uint16_t mch_id = 0; + if (!mch_id) { + struct device *dev = pcidev_path_on_root(SA_DEVFN_ROOT); + mch_id = dev ? pci_read_config16(dev, PCI_DEVICE_ID) : 0xffff; + } + + /* + * Iccmax table from Doc #337344 Section 7.2 DC Specifications for CFL. + * Iccmax table from Doc #338023 Section 7.2 DC Specifications for WHL. + * + * Platform Segment SA IA GT (GT/GTx) + * --------------------------------------------------------------------- + * CFL-U (28W) GT3 quad 8.5 64 64 + * CFL-U (28W) GT3 dual 8.5 64 64 + * + * CFL-H (45W) GT2 hex 11.1 128 0 + * CFL-H (45W) GT2 quad 11.1 64 0 + * + * CFL-S (95W) GT2 octa 11.1 193 45 + * + * CFL-S (95W) GT2 hex 11.1 138 45 + * CFL-S (65W) GT2 hex 11.1 133 45 + * CFL-S (80W) GT2 hex 11.1 133 45 + * CFL-S (35W) GT2 hex 11.1 104 35 + * + * CFL-S (91W) GT2 quad 11.1 100 45 + * CFL-S (83W) GT2 quad 11.1 100 45 + * CFL-S (71W) GT2 quad 11.1 100 45 + * CFL-S (65W) GT2 quad 11.1 79 45 + * CFL-S (62W) GT2 quad 11.1 79 45 + * CFL-S (35W) GT2 quad 11.1 66 35 + * + * CFL-S (58W) GT2 dual 11.1 79 45 + * CFL-S (54W) GT2 dual 11.1 58 45 + * CFL-S (35W) GT2 dual 11.1 40 35 + * + * CNL-U (15W) ? ? 0 + * + * WHL-U (15W) GT2 quad 6 70 31 + * WHL-U (15W) GT2 dual 6 35 31 + * + */ + + switch (mch_id) { + case PCI_DEVICE_ID_INTEL_CNL_ID_U: /* fallthrough */ + case PCI_DEVICE_ID_INTEL_CNL_ID_Y: { + uint16_t icc_max[NUM_VR_DOMAINS] = VR_CFG_ALL_DOMAINS_ICC(8.5, 64, 0, 0); + + return icc_max[domain]; + } + case PCI_DEVICE_ID_INTEL_WHL_ID_W_4: { + uint16_t icc_max[NUM_VR_DOMAINS] = VR_CFG_ALL_DOMAINS_ICC(6, 70, 31, 31); + + return icc_max[domain]; + } + case PCI_DEVICE_ID_INTEL_WHL_ID_W_2: { + uint16_t icc_max[NUM_VR_DOMAINS] = VR_CFG_ALL_DOMAINS_ICC(6, 35, 31, 31); + + return icc_max[domain]; + } + case PCI_DEVICE_ID_INTEL_CFL_ID_U: /* fallthrough */ + case PCI_DEVICE_ID_INTEL_CFL_ID_U_2: { + uint16_t icc_max[NUM_VR_DOMAINS] = VR_CFG_ALL_DOMAINS_ICC(8.5, 64, 64, 64); + + return icc_max[domain]; + } + case PCI_DEVICE_ID_INTEL_CFL_ID_H_8: /* fallthrough - undocumented */ + case PCI_DEVICE_ID_INTEL_CFL_ID_H: { /* 6 core */ + uint16_t icc_max[NUM_VR_DOMAINS] = VR_CFG_ALL_DOMAINS_ICC(11.1, 128, 0, 0); + + return icc_max[domain]; + } + case PCI_DEVICE_ID_INTEL_CFL_ID_H_4: { /* 4 core */ + uint16_t icc_max[NUM_VR_DOMAINS] = VR_CFG_ALL_DOMAINS_ICC(11.1, 64, 0, 0); + + return icc_max[domain]; + } + case PCI_DEVICE_ID_INTEL_CFL_ID_S_DT_2: { /* 2 core */ + uint16_t icc_max[NUM_VR_DOMAINS] = VR_CFG_ALL_DOMAINS_ICC(11.1, 40, 35, 35); + + if (tdp >= 54) { + if (tdp >= 58) + icc_max[VR_IA_CORE] = VR_CFG_AMP(79); + else + icc_max[VR_IA_CORE] = VR_CFG_AMP(58); + + icc_max[VR_GT_SLICED] = VR_CFG_AMP(45); + icc_max[VR_GT_UNSLICED] = VR_CFG_AMP(45); + } + + return icc_max[domain]; + } + case PCI_DEVICE_ID_INTEL_CFL_ID_S_S_8: { + uint16_t icc_max[NUM_VR_DOMAINS] = VR_CFG_ALL_DOMAINS_ICC(11.1, 193, 45, 45); + + return icc_max[domain]; + } + case PCI_DEVICE_ID_INTEL_CFL_ID_S: /* fallthrough */ + case PCI_DEVICE_ID_INTEL_CFL_ID_S_WS_6: /* fallthrough */ + case PCI_DEVICE_ID_INTEL_CFL_ID_S_S_6: { + uint16_t icc_max[NUM_VR_DOMAINS] = VR_CFG_ALL_DOMAINS_ICC(11.1, 104, 35, 35); + if (tdp > 35) { + if (tdp >= 95) { + icc_max[VR_IA_CORE] = VR_CFG_AMP(138); + } else if (tdp >= 65) { + icc_max[VR_IA_CORE] = VR_CFG_AMP(133); + } + + icc_max[VR_GT_SLICED] = VR_CFG_AMP(45); + icc_max[VR_GT_UNSLICED] = VR_CFG_AMP(45); + } + return icc_max[domain]; + } + case PCI_DEVICE_ID_INTEL_CFL_ID_S_S_4: /* fallthrough */ + case PCI_DEVICE_ID_INTEL_CFL_ID_S_WS_4: /* fallthrough */ + case PCI_DEVICE_ID_INTEL_CFL_ID_S_DT_4: { + uint16_t icc_max[NUM_VR_DOMAINS] = VR_CFG_ALL_DOMAINS_ICC(11.1, 66, 35, 35); + if (tdp > 35) { + if (tdp >= 71) { + icc_max[VR_IA_CORE] = VR_CFG_AMP(100); + } else if (tdp >= 62) { + icc_max[VR_IA_CORE] = VR_CFG_AMP(79); + } + + icc_max[VR_GT_SLICED] = VR_CFG_AMP(45); + icc_max[VR_GT_UNSLICED] = VR_CFG_AMP(45); + } + return icc_max[domain]; + } + default: + printk(BIOS_ERR, "ERROR: Unknown MCH (0x%x) in VR-config\n", mch_id); + } + return 0; +} + +static uint16_t get_sku_ac_dc_loadline(const int domain) +{ + static uint16_t mch_id = 0; + if (!mch_id) { + struct device *dev = pcidev_path_on_root(SA_DEVFN_ROOT); + mch_id = dev ? pci_read_config16(dev, PCI_DEVICE_ID) : 0xffff; + } + + switch (mch_id) { + case PCI_DEVICE_ID_INTEL_WHL_ID_W_4: /* fallthrough */ + case PCI_DEVICE_ID_INTEL_CFL_ID_H_8: /* fallthrough */ + case PCI_DEVICE_ID_INTEL_CFL_ID_H: /* fallthrough */ + case PCI_DEVICE_ID_INTEL_CFL_ID_H_4: { /* fallthrough */ + uint16_t loadline[NUM_VR_DOMAINS] = + VR_CFG_ALL_DOMAINS_LOADLINE(10.3, 1.8, 2.7, 2.7); + if (mch_id == PCI_DEVICE_ID_INTEL_WHL_ID_W_4) { + loadline[VR_GT_SLICED] = 0; /* unspecified */ + loadline[VR_GT_UNSLICED] = 0; /* unspecified */ + } + return loadline[domain]; + } + case PCI_DEVICE_ID_INTEL_WHL_ID_W_2: /* fallthrough */ + case PCI_DEVICE_ID_INTEL_CFL_ID_U: /* fallthrough */ + case PCI_DEVICE_ID_INTEL_CFL_ID_U_2: /* fallthrough */ + case PCI_DEVICE_ID_INTEL_CNL_ID_U: /* fallthrough */ + case PCI_DEVICE_ID_INTEL_CNL_ID_Y: { + uint16_t loadline[NUM_VR_DOMAINS] = + VR_CFG_ALL_DOMAINS_LOADLINE(10.3, 2.4, 2.0, 2.0); + if (mch_id == PCI_DEVICE_ID_INTEL_WHL_ID_W_2) { + loadline[VR_GT_SLICED] = 0; /* unspecified */ + loadline[VR_GT_UNSLICED] = 0; /* unspecified */ + } + return loadline[domain]; + } + case PCI_DEVICE_ID_INTEL_CFL_ID_S_S_8: { + const uint16_t loadline[NUM_VR_DOMAINS] = + VR_CFG_ALL_DOMAINS_LOADLINE(11.1, 1.6, 3.1, 3.1); + return loadline[domain]; + } + case PCI_DEVICE_ID_INTEL_CFL_ID_S: /* fallthrough */ + case PCI_DEVICE_ID_INTEL_CFL_ID_S_WS_6: /* fallthrough */ + case PCI_DEVICE_ID_INTEL_CFL_ID_S_S_6: /* fallthrough */ + case PCI_DEVICE_ID_INTEL_CFL_ID_S_S_4: /* fallthrough */ + case PCI_DEVICE_ID_INTEL_CFL_ID_S_WS_4: /* fallthrough */ + case PCI_DEVICE_ID_INTEL_CFL_ID_S_DT_4: /* fallthrough */ + case PCI_DEVICE_ID_INTEL_CFL_ID_S_DT_2: { + const uint16_t loadline[NUM_VR_DOMAINS] = + VR_CFG_ALL_DOMAINS_LOADLINE(11.1, 2.1, 3.1, 3.1); + return loadline[domain]; + } + default: + printk(BIOS_ERR, "ERROR: Unknown MCH (0x%x) in VR-config\n", mch_id); + } + return 0; +} + void fill_vr_domain_config(void *params, int domain, const struct vr_config *chip_cfg) { @@ -96,4 +296,19 @@ vr_params->VrVoltageLimit[domain] = cfg->voltage_limit; vr_params->AcLoadline[domain] = cfg->ac_loadline; vr_params->DcLoadline[domain] = cfg->dc_loadline; + + /* If board provided non-zero value, use it. */ + if (cfg->icc_max) + vr_params->IccMax[domain] = cfg->icc_max; + else + vr_params->IccMax[domain] = get_sku_icc_max(domain); + + if (cfg->ac_loadline) + vr_params->AcLoadline[domain] = cfg->ac_loadline; + else + vr_params->AcLoadline[domain] = get_sku_ac_dc_loadline(domain); + if (cfg->dc_loadline) + vr_params->DcLoadline[domain] = cfg->dc_loadline; + else + vr_params->DcLoadline[domain] = get_sku_ac_dc_loadline(domain); }
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37466 )
Change subject: soc/intel/cannonlake: Add VR config for CFL and WHL ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/37466/1/src/soc/intel/cannonlake/vr... File src/soc/intel/cannonlake/vr_config.c:
https://review.coreboot.org/c/coreboot/+/37466/1/src/soc/intel/cannonlake/vr... PS1, Line 183: if (tdp >= 95) { braces {} are not necessary for any arm of this statement
https://review.coreboot.org/c/coreboot/+/37466/1/src/soc/intel/cannonlake/vr... PS1, Line 199: if (tdp >= 71) { braces {} are not necessary for any arm of this statement
Hello Patrick Rudolph, Kane Chen, Subrata Banik, Tim Wawrzynczak, Shelley Chen, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37466
to look at the new patch set (#2).
Change subject: soc/intel/cannonlake: Add VR config for CFL and WHL ......................................................................
soc/intel/cannonlake: Add VR config for CFL and WHL
Add VR config IccMax, DC and AC loadline defaults for all CFL and WHL.
I couldn't find CNL datasheet, but it looks like only a single SoC without graphics was ever produced.
I couldn't find CML datasheet, so those are left out for now.
Used Doc #337344 and #338023 Section 7.
Change-Id: I1d2e174157d468830cc0baf2a2d8295ef61a1a63 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/soc/intel/cannonlake/include/soc/vr_config.h M src/soc/intel/cannonlake/vr_config.c 2 files changed, 246 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/37466/2
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37466 )
Change subject: soc/intel/cannonlake: Add VR config for CFL and WHL ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/37466/2/src/soc/intel/cannonlake/vr... File src/soc/intel/cannonlake/vr_config.c:
https://review.coreboot.org/c/coreboot/+/37466/2/src/soc/intel/cannonlake/vr... PS2, Line 191: if (tdp >= 95) { braces {} are not necessary for any arm of this statement
https://review.coreboot.org/c/coreboot/+/37466/2/src/soc/intel/cannonlake/vr... PS2, Line 208: if (tdp >= 71) { braces {} are not necessary for any arm of this statement
Wim Vervoorn has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37466 )
Change subject: soc/intel/cannonlake: Add VR config for CFL and WHL ......................................................................
Patch Set 2:
(8 comments)
https://review.coreboot.org/c/coreboot/+/37466/2/src/soc/intel/cannonlake/vr... File src/soc/intel/cannonlake/vr_config.c:
https://review.coreboot.org/c/coreboot/+/37466/2/src/soc/intel/cannonlake/vr... PS2, Line 33: 0x0 Why not changes these values to decimal as well.
https://review.coreboot.org/c/coreboot/+/37466/2/src/soc/intel/cannonlake/vr... PS2, Line 121: ? 13A
https://review.coreboot.org/c/coreboot/+/37466/2/src/soc/intel/cannonlake/vr... PS2, Line 121: * CNL-U (15W) ? ? 0 34A
https://review.coreboot.org/c/coreboot/+/37466/2/src/soc/intel/cannonlake/vr... PS2, Line 151: 8.5 Shouldn't you add a cast to VR_CFG_AMP(i) ((i) * 4) when values like this are used?
https://review.coreboot.org/c/coreboot/+/37466/2/src/soc/intel/cannonlake/vr... PS2, Line 305: vr_params->IccMax[domain] = cfg->icc_max; This is handled below.
https://review.coreboot.org/c/coreboot/+/37466/2/src/soc/intel/cannonlake/vr... PS2, Line 306: vr_params->VrVoltageLimit[domain] = cfg->voltage_limit; Isn't it an idea to make this conditional as well so we don't have to put 1520 in every device tree and can just leave out the value unless it should differ from the default?
https://review.coreboot.org/c/coreboot/+/37466/2/src/soc/intel/cannonlake/vr... PS2, Line 307: vr_params->AcLoadline[domain] = cfg->ac_loadline; : vr_params->DcLoadline[domain] = cfg->dc_loadline; These lines can be removed, they are handled below.
https://review.coreboot.org/c/coreboot/+/37466/2/src/soc/intel/cannonlake/vr... PS2, Line 320: if (cfg->dc_loadline) Adding a blank line before this will make it more readable.
Hello Patrick Rudolph, Wim Vervoorn, Kane Chen, Subrata Banik, Tim Wawrzynczak, Shelley Chen, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37466
to look at the new patch set (#3).
Change subject: soc/intel/cannonlake: Add VR config for CFL, CNL and WHL ......................................................................
soc/intel/cannonlake: Add VR config for CFL, CNL and WHL
Add VR config IccMax, DC and AC loadline defaults and voltage regulator maximum for all CFL, CNL and WHL. This supports mainboards with replace CPUs and provides sane defaults for boards that are missing the devicetree overwrite. Remove the default IccMax to make use of the introduced lookup-table.
Also change some hex values to decimal.
I couldn't find CML datasheet, so those are left out for now.
Used Doc #337344 and #338023 Section 7.
Change-Id: I1d2e174157d468830cc0baf2a2d8295ef61a1a63 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/soc/intel/cannonlake/include/soc/vr_config.h M src/soc/intel/cannonlake/vr_config.c 2 files changed, 262 insertions(+), 16 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/37466/3
Hello Patrick Rudolph, Wim Vervoorn, Kane Chen, Subrata Banik, Tim Wawrzynczak, Shelley Chen, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37466
to look at the new patch set (#4).
Change subject: soc/intel/cannonlake: Add VR config for CFL, CNL and WHL ......................................................................
soc/intel/cannonlake: Add VR config for CFL, CNL and WHL
Add VR config IccMax, DC and AC loadline defaults and voltage regulator maximum for all CFL, CNL and WHL. This supports mainboards with replace CPUs and provides sane defaults for boards that are missing the devicetree overwrite. Remove the default IccMax to make use of the introduced lookup-table.
Also change some hex values to decimal.
I couldn't find CML datasheet, so those are left out for now.
Used Doc #337344 and #338023 Section 7.
Change-Id: I1d2e174157d468830cc0baf2a2d8295ef61a1a63 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/soc/intel/cannonlake/include/soc/vr_config.h M src/soc/intel/cannonlake/vr_config.c 2 files changed, 263 insertions(+), 16 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/37466/4
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37466 )
Change subject: soc/intel/cannonlake: Add VR config for CFL, CNL and WHL ......................................................................
Patch Set 4:
(8 comments)
https://review.coreboot.org/c/coreboot/+/37466/2/src/soc/intel/cannonlake/vr... File src/soc/intel/cannonlake/vr_config.c:
https://review.coreboot.org/c/coreboot/+/37466/2/src/soc/intel/cannonlake/vr... PS2, Line 33: 0x0
Why not changes these values to decimal as well.
Done
https://review.coreboot.org/c/coreboot/+/37466/2/src/soc/intel/cannonlake/vr... PS2, Line 121: ?
13A
Done
https://review.coreboot.org/c/coreboot/+/37466/2/src/soc/intel/cannonlake/vr... PS2, Line 121: * CNL-U (15W) ? ? 0
34A
Done
https://review.coreboot.org/c/coreboot/+/37466/2/src/soc/intel/cannonlake/vr... PS2, Line 151: 8.5
Shouldn't you add a cast to VR_CFG_AMP(i) ((i) * 4) when values like this are used?
It will be converted to int even without a cast
https://review.coreboot.org/c/coreboot/+/37466/2/src/soc/intel/cannonlake/vr... PS2, Line 305: vr_params->IccMax[domain] = cfg->icc_max;
This is handled below.
Done
https://review.coreboot.org/c/coreboot/+/37466/2/src/soc/intel/cannonlake/vr... PS2, Line 306: vr_params->VrVoltageLimit[domain] = cfg->voltage_limit;
Isn't it an idea to make this conditional as well so we don't have to put 1520 in every device tree […]
Done
https://review.coreboot.org/c/coreboot/+/37466/2/src/soc/intel/cannonlake/vr... PS2, Line 307: vr_params->AcLoadline[domain] = cfg->ac_loadline; : vr_params->DcLoadline[domain] = cfg->dc_loadline;
These lines can be removed, they are handled below.
Done
https://review.coreboot.org/c/coreboot/+/37466/2/src/soc/intel/cannonlake/vr... PS2, Line 320: if (cfg->dc_loadline)
Adding a blank line before this will make it more readable.
Done
Wim Vervoorn has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37466 )
Change subject: soc/intel/cannonlake: Add VR config for CFL, CNL and WHL ......................................................................
Patch Set 4: Code-Review+1
Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37466 )
Change subject: soc/intel/cannonlake: Add VR config for CFL, CNL and WHL ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37466/4/src/soc/intel/cannonlake/vr... File src/soc/intel/cannonlake/vr_config.c:
https://review.coreboot.org/c/coreboot/+/37466/4/src/soc/intel/cannonlake/vr... PS4, Line 101: 64 86
Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37466 )
Change subject: soc/intel/cannonlake: Add VR config for CFL, CNL and WHL ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/c/coreboot/+/37466/4/src/soc/intel/cannonlake/vr... File src/soc/intel/cannonlake/vr_config.c:
https://review.coreboot.org/c/coreboot/+/37466/4/src/soc/intel/cannonlake/vr... PS4, Line 260: 11.1 This parameter is missed in the 337344-003 doc for the S series. Maybe we will set 10.3 here, as well as for the H and U series?
https://review.coreboot.org/c/coreboot/+/37466/4/src/soc/intel/cannonlake/vr... PS4, Line 271: 11.1 See above
Jamie Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37466 )
Change subject: soc/intel/cannonlake: Add VR config for CFL, CNL and WHL ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37466/4/src/soc/intel/cannonlake/vr... File src/soc/intel/cannonlake/vr_config.c:
https://review.coreboot.org/c/coreboot/+/37466/4/src/soc/intel/cannonlake/vr... PS4, Line 93: 338023 I cannot find those two document IDs. Do you get from Intel RDC?
Jamie Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37466 )
Change subject: soc/intel/cannonlake: Add VR config for CFL, CNL and WHL ......................................................................
Patch Set 4:
1. Could you help to add CML data as below? Iccmax table from Doc #606599 Section 7.2 DC Specifications for CML.
* Platform Segment SA IA GT (GT/GTx) * ----------------------------------------------------------------------------- * CML-U v1/v2 (15W) GT2 hexa 6 85(70) 31 * CML-U v1/v2 (15W) GT2 quad 6 85(70) 31 * CML-U v1/v2 (15W) GT2 dual 6 35 31 * CML-H (65W) GT2 octa 11.1 192(165)32 * CML-H (45W) GT2 octa 11.1 165(140)32 * CML-H (45W) GT2 hexa 11.1 140(128)32 * CML-H (45W) GT2 quad 11.1 105(86) 32 * CML-S (125W)GT2 deca 11.1 245(210)35 * CML-S (125W)GT2 octa 11.1 245(210)35 * CML-S (125W)GT2 hexa 11.1 140 35 * CML-S XeonW (80W) GT2 deca 11.1 210 35 * CML-S XeonW (80W) GT2 octa 11.1 210 35 * CML-S XeonW (80W) GT2 hexa 11.1 140 35 * CML-S (65W) GT2 deca 11.1 210(175)35 * CML-S (65W) GT2 octa 11.1 210(175)35 * CML-S (65W) GT2 hexa 11.1 140 35 * CML-S (35W) GT2 deca 11.1 140(104)35 * CML-S (35W) GT2 octa 11.1 140(104)35 * CML-S (35W) GT2 hexa 11.1 104 35 ()= if device use baseline VR design. 2. There are two kinds of VR design on CML PDG.(One is baseline, the other is performance) Could you add a config for switching two kinds of value? Ex: switch(mch_id) { ... case PCI_DEVICE_ID_INTEL_CML_ULT case PCI_DEVICE_ID_INTEL_CML_ULT_6_2: { #if CONFIG(VR_CONFIG_BASELINE) uint16_t icc_max[NUM_VR_DOMAINS] = VR_CFG_ALL_DOMAINS_ICC(6, 70, 31, 31); #else uint16_t icc_max[NUM_VR_DOMAINS] = VR_CFG_ALL_DOMAINS_ICC(6, 85, 31, 31); #endif
return icc_max[domain]; } case PCI_DEVICE_ID_INTEL_CML_ULT_2_2: { uint16_t icc_max[NUM_VR_DOMAINS] = VR_CFG_ALL_DOMAINS_ICC(6, 35, 31, 31);
return icc_max[domain]; } case PCI_DEVICE_ID_INTEL_CML_H_8_2: { uint16_t icc_max[NUM_VR_DOMAINS] = VR_CFG_ALL_DOMAINS_ICC(11.1, 105, 32, 32); if (tdp >= 54) #if CONFIG(VR_CONFIG_BASELINE) icc_max[VR_IA_CORE] = VR_CFG_AMP(165); #else icc_max[VR_IA_CORE] = VR_CFG_AMP(192); #endif else #if CONFIG(VR_CONFIG_BASELINE) icc_max[VR_IA_CORE] = VR_CFG_AMP(140); #else icc_max[VR_IA_CORE] = VR_CFG_AMP(165); #endif } ...
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37466 )
Change subject: soc/intel/cannonlake: Add VR config for CFL, CNL and WHL ......................................................................
Patch Set 4:
Hi Jamie, for CML please rebase your patch on top of this and don't use preprocessor as this is a runtime lookup table.
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37466 )
Change subject: soc/intel/cannonlake: Add VR config for CFL, CNL and WHL ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37466/4/src/soc/intel/cannonlake/vr... File src/soc/intel/cannonlake/vr_config.c:
https://review.coreboot.org/c/coreboot/+/37466/4/src/soc/intel/cannonlake/vr... PS4, Line 93: 338023
I cannot find those two document IDs. […]
Found on google: "8th Gen Intel® Core™ Processor Datasheet Volume 1 of 2"
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37466 )
Change subject: soc/intel/cannonlake: Add VR config for CFL, CNL and WHL ......................................................................
Patch Set 5:
(3 comments)
https://review.coreboot.org/c/coreboot/+/37466/5/src/soc/intel/cannonlake/vr... File src/soc/intel/cannonlake/vr_config.c:
https://review.coreboot.org/c/coreboot/+/37466/5/src/soc/intel/cannonlake/vr... PS5, Line 151: 8.5 Are you using the floating point for documentation purposes? This will be just 8 when it gets converted to a uint16_t
https://review.coreboot.org/c/coreboot/+/37466/5/src/soc/intel/cannonlake/vr... PS5, Line 190: if (tdp > 35) Above you're using "if (tdp >= 54)" but here you're using >, could you use either >= or > everywhere?
https://review.coreboot.org/c/coreboot/+/37466/5/src/soc/intel/cannonlake/vr... PS5, Line 250: uint16_t loadline[NUM_VR_DOMAINS] = : VR_CFG_ALL_DOMAINS_LOADLINE(10.3, 2.4, 2.0, 2.0); Your const usage is spotty, these should all be const. Then hopefully the compiler can turn the floating numbers into ints at compile time and not runtime.
Hello Patrick Rudolph, Wim Vervoorn, Kane Chen, Subrata Banik, Tim Wawrzynczak, Shelley Chen, build bot (Jenkins), Jamie Chen, Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37466
to look at the new patch set (#6).
Change subject: soc/intel/cannonlake: Add VR config for CFL, CNL and WHL ......................................................................
soc/intel/cannonlake: Add VR config for CFL, CNL and WHL
Add VR config IccMax, DC and AC loadline defaults and voltage regulator maximum for all CFL, CNL and WHL. This supports mainboards with replace CPUs and provides sane defaults for boards that are missing the devicetree overwrite. Remove the default IccMax to make use of the introduced lookup-table.
Also change some hex values to decimal.
I couldn't find CML datasheet, so those are left out for now.
Used Doc #337344 and #338023 Section 7.
Change-Id: I1d2e174157d468830cc0baf2a2d8295ef61a1a63 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/soc/intel/cannonlake/include/soc/vr_config.h M src/soc/intel/cannonlake/vr_config.c 2 files changed, 264 insertions(+), 17 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/37466/6
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37466 )
Change subject: soc/intel/cannonlake: Add VR config for CFL, CNL and WHL ......................................................................
Patch Set 6:
(3 comments)
https://review.coreboot.org/c/coreboot/+/37466/5/src/soc/intel/cannonlake/vr... File src/soc/intel/cannonlake/vr_config.c:
https://review.coreboot.org/c/coreboot/+/37466/5/src/soc/intel/cannonlake/vr... PS5, Line 151: 8.5
Are you using the floating point for documentation purposes? This will be just 8 when it gets conve […]
Yes, that's done for documentation purposes and the compiler will round it to the closest value (in units of 0.25A).
https://review.coreboot.org/c/coreboot/+/37466/5/src/soc/intel/cannonlake/vr... PS5, Line 190: if (tdp > 35)
Above you're using "if (tdp >= 54)" but here you're using >, could you use either >= or > everywhere […]
Done
https://review.coreboot.org/c/coreboot/+/37466/5/src/soc/intel/cannonlake/vr... PS5, Line 250: uint16_t loadline[NUM_VR_DOMAINS] = : VR_CFG_ALL_DOMAINS_LOADLINE(10.3, 2.4, 2.0, 2.0);
Your const usage is spotty, these should all be const. […]
Added cast to make sure it's rounded at compile time
Hello Patrick Rudolph, Wim Vervoorn, Kane Chen, Subrata Banik, Tim Wawrzynczak, Shelley Chen, build bot (Jenkins), Jamie Chen, Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37466
to look at the new patch set (#7).
Change subject: soc/intel/cannonlake: Add VR config for CFL, CNL and WHL ......................................................................
soc/intel/cannonlake: Add VR config for CFL, CNL and WHL
Add VR config IccMax, DC and AC loadline defaults and voltage regulator maximum for all CFL, CNL and WHL. This supports mainboards with replace CPUs and provides sane defaults for boards that are missing the devicetree overwrite. Remove the default IccMax to make use of the introduced lookup-table.
Also change some hex values to decimal.
I couldn't find CML datasheet, so those are left out for now.
Used Doc #337344 and #338023 Section 7.
Change-Id: I1d2e174157d468830cc0baf2a2d8295ef61a1a63 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/soc/intel/cannonlake/include/soc/vr_config.h M src/soc/intel/cannonlake/vr_config.c 2 files changed, 266 insertions(+), 17 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/37466/7
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37466 )
Change subject: soc/intel/cannonlake: Add VR config for CFL, CNL and WHL ......................................................................
Patch Set 6:
(3 comments)
https://review.coreboot.org/c/coreboot/+/37466/4/src/soc/intel/cannonlake/vr... File src/soc/intel/cannonlake/vr_config.c:
https://review.coreboot.org/c/coreboot/+/37466/4/src/soc/intel/cannonlake/vr... PS4, Line 101: 64
86
Done
https://review.coreboot.org/c/coreboot/+/37466/4/src/soc/intel/cannonlake/vr... PS4, Line 260: 11.1
This parameter is missed in the 337344-003 doc for the S series. Maybe we will set 10. […]
It looks like this isn't used, as the SA voltage is "fixed" on the S series. But the datasheet doesn't cover this case.
https://review.coreboot.org/c/coreboot/+/37466/4/src/soc/intel/cannonlake/vr... PS4, Line 271: 11.1
See above
Done
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37466 )
Change subject: soc/intel/cannonlake: Add VR config for CFL, CNL and WHL ......................................................................
Patch Set 7:
(2 comments)
https://review.coreboot.org/c/coreboot/+/37466/7//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37466/7//COMMIT_MSG@11 PS7, Line 11: replace replace*able*
https://review.coreboot.org/c/coreboot/+/37466/7/src/soc/intel/cannonlake/vr... File src/soc/intel/cannonlake/vr_config.c:
https://review.coreboot.org/c/coreboot/+/37466/7/src/soc/intel/cannonlake/vr... PS7, Line 311: /* If board provided non-zero value, use it. */ Could use the ternary operator:
vr_params->VrVoltageLimit[domain] = cfg->voltage_limit ? cfg->voltage_limit : get_sku_voltagelimit(domain);
If too long, it could be split like this:
vr_params->VrVoltageLimit[domain] = cfg->voltage_limit ? cfg->voltage_limit : get_sku_voltagelimit(domain);
Or like this:
vr_params->VrVoltageLimit[domain] = cfg->voltage_limit ? cfg->voltage_limit : get_sku_voltagelimit(domain);
In any case, it is more succint than an if/else. This "use X if non-zero, else use Y" pattern seems to be rather common in coreboot, maybe creating some macro for it would be useful.
Hello Patrick Rudolph, Wim Vervoorn, Kane Chen, Subrata Banik, Tim Wawrzynczak, Shelley Chen, build bot (Jenkins), Jamie Chen, Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37466
to look at the new patch set (#8).
Change subject: soc/intel/cannonlake: Add VR config for CFL, CNL and WHL ......................................................................
soc/intel/cannonlake: Add VR config for CFL, CNL and WHL
Add VR config IccMax, DC and AC loadline defaults and voltage regulator maximum for all CFL, CNL and WHL. This supports mainboards with replaceable CPUs and provides sane defaults for boards that are missing the devicetree overwrite. Remove the default IccMax to make use of the introduced lookup-table.
Also change some hex values to decimal.
I couldn't find CML datasheet, so those are left out for now.
Used Doc #337344 and #338023 Section 7.
Change-Id: I1d2e174157d468830cc0baf2a2d8295ef61a1a63 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/soc/intel/cannonlake/include/soc/vr_config.h M src/soc/intel/cannonlake/vr_config.c 2 files changed, 266 insertions(+), 17 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/37466/8
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37466 )
Change subject: soc/intel/cannonlake: Add VR config for CFL, CNL and WHL ......................................................................
Patch Set 8:
(2 comments)
https://review.coreboot.org/c/coreboot/+/37466/7//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37466/7//COMMIT_MSG@11 PS7, Line 11: replace
replace*able*
Done
https://review.coreboot.org/c/coreboot/+/37466/7/src/soc/intel/cannonlake/vr... File src/soc/intel/cannonlake/vr_config.c:
https://review.coreboot.org/c/coreboot/+/37466/7/src/soc/intel/cannonlake/vr... PS7, Line 311: /* If board provided non-zero value, use it. */
Could use the ternary operator: […]
Yes, it could be used.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37466 )
Change subject: soc/intel/cannonlake: Add VR config for CFL, CNL and WHL ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37466/7/src/soc/intel/cannonlake/vr... File src/soc/intel/cannonlake/vr_config.c:
https://review.coreboot.org/c/coreboot/+/37466/7/src/soc/intel/cannonlake/vr... PS7, Line 311: /* If board provided non-zero value, use it. */
Yes, it could be used.
:/
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37466 )
Change subject: soc/intel/cannonlake: Add VR config for CFL, CNL and WHL ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37466/7/src/soc/intel/cannonlake/vr... File src/soc/intel/cannonlake/vr_config.c:
https://review.coreboot.org/c/coreboot/+/37466/7/src/soc/intel/cannonlake/vr... PS7, Line 311: /* If board provided non-zero value, use it. */
:/
Yes, please use the ternary operator here as Angel correctly pointed out.
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37466 )
Change subject: soc/intel/cannonlake: Add VR config for CFL, CNL and WHL ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37466/7/src/soc/intel/cannonlake/vr... File src/soc/intel/cannonlake/vr_config.c:
https://review.coreboot.org/c/coreboot/+/37466/7/src/soc/intel/cannonlake/vr... PS7, Line 311: /* If board provided non-zero value, use it. */
Yes, please use the ternary operator here as Angel correctly pointed out.
Feel free to add a separate change on top of this commit.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37466 )
Change subject: soc/intel/cannonlake: Add VR config for CFL, CNL and WHL ......................................................................
Patch Set 8: Code-Review-1
Philipp Deppenwiese has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37466 )
Change subject: soc/intel/cannonlake: Add VR config for CFL, CNL and WHL ......................................................................
Patch Set 8: Code-Review+2
Philipp Deppenwiese has submitted this change. ( https://review.coreboot.org/c/coreboot/+/37466 )
Change subject: soc/intel/cannonlake: Add VR config for CFL, CNL and WHL ......................................................................
soc/intel/cannonlake: Add VR config for CFL, CNL and WHL
Add VR config IccMax, DC and AC loadline defaults and voltage regulator maximum for all CFL, CNL and WHL. This supports mainboards with replaceable CPUs and provides sane defaults for boards that are missing the devicetree overwrite. Remove the default IccMax to make use of the introduced lookup-table.
Also change some hex values to decimal.
I couldn't find CML datasheet, so those are left out for now.
Used Doc #337344 and #338023 Section 7.
Change-Id: I1d2e174157d468830cc0baf2a2d8295ef61a1a63 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/37466 Reviewed-by: Philipp Deppenwiese zaolin.daisuki@gmail.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/intel/cannonlake/include/soc/vr_config.h M src/soc/intel/cannonlake/vr_config.c 2 files changed, 266 insertions(+), 17 deletions(-)
Approvals: build bot (Jenkins): Verified Philipp Deppenwiese: Looks good to me, approved
Objections: Edward O'Callaghan: I would prefer that you didn't submit this
diff --git a/src/soc/intel/cannonlake/include/soc/vr_config.h b/src/soc/intel/cannonlake/include/soc/vr_config.h index 8bcf001..1390b17 100644 --- a/src/soc/intel/cannonlake/include/soc/vr_config.h +++ b/src/soc/intel/cannonlake/include/soc/vr_config.h @@ -55,7 +55,8 @@ uint16_t dc_loadline; };
-#define VR_CFG_AMP(i) ((i) * 4) +#define VR_CFG_AMP(i) (uint16_t)((i) * 4) +#define VR_CFG_MOHMS(i) (uint16_t)((i) * 100)
/* VrConfig Settings for 4 domains * 0 = System Agent, 1 = IA Core, @@ -68,6 +69,22 @@ NUM_VR_DOMAINS };
+#define VR_CFG_ALL_DOMAINS_ICC(sa, ia, gt_unsl, gt_sl) \ + { \ + [VR_SYSTEM_AGENT] = VR_CFG_AMP(sa), \ + [VR_IA_CORE] = VR_CFG_AMP(ia), \ + [VR_GT_UNSLICED] = VR_CFG_AMP(gt_unsl), \ + [VR_GT_SLICED] = VR_CFG_AMP(gt_sl), \ + } + +#define VR_CFG_ALL_DOMAINS_LOADLINE(sa, ia, gt_unsl, gt_sl) \ + { \ + [VR_SYSTEM_AGENT] = VR_CFG_MOHMS(sa), \ + [VR_IA_CORE] = VR_CFG_MOHMS(ia), \ + [VR_GT_UNSLICED] = VR_CFG_MOHMS(gt_unsl), \ + [VR_GT_SLICED] = VR_CFG_MOHMS(gt_sl), \ + } + void fill_vr_domain_config(void *params, int domain, const struct vr_config *cfg);
diff --git a/src/soc/intel/cannonlake/vr_config.c b/src/soc/intel/cannonlake/vr_config.c index 25270d8..7096b62 100644 --- a/src/soc/intel/cannonlake/vr_config.c +++ b/src/soc/intel/cannonlake/vr_config.c @@ -14,9 +14,13 @@ * */
+#include <device/pci_ids.h> +#include <device/pci_ops.h> #include <fsp/api.h> #include <soc/ramstage.h> #include <soc/vr_config.h> +#include <console/console.h> +#include <intelblocks/cpulib.h>
static const struct vr_config default_configs[NUM_VR_DOMAINS] = { [VR_SYSTEM_AGENT] = { @@ -26,9 +30,9 @@ .psi3threshold = VR_CFG_AMP(1), .psi3enable = 1, .psi4enable = 1, - .imon_slope = 0x0, - .imon_offset = 0x0, - .icc_max = VR_CFG_AMP(6), + .imon_slope = 0, + .imon_offset = 0, + .icc_max = 0, .voltage_limit = 1520, }, [VR_IA_CORE] = { @@ -38,9 +42,9 @@ .psi3threshold = VR_CFG_AMP(1), .psi3enable = 1, .psi4enable = 1, - .imon_slope = 0x0, - .imon_offset = 0x0, - .icc_max = VR_CFG_AMP(70), + .imon_slope = 0, + .imon_offset = 0, + .icc_max = 0, .voltage_limit = 1520, }, [VR_GT_UNSLICED] = { @@ -50,9 +54,9 @@ .psi3threshold = VR_CFG_AMP(1), .psi3enable = 1, .psi4enable = 1, - .imon_slope = 0x0, - .imon_offset = 0x0, - .icc_max = VR_CFG_AMP(31), + .imon_slope = 0, + .imon_offset = 0, + .icc_max = 0, .voltage_limit = 1520, }, [VR_GT_SLICED] = { @@ -62,13 +66,224 @@ .psi3threshold = VR_CFG_AMP(1), .psi3enable = 1, .psi4enable = 1, - .imon_slope = 0x0, - .imon_offset = 0x0, - .icc_max = VR_CFG_AMP(31), + .imon_slope = 0, + .imon_offset = 0, + .icc_max = 0, .voltage_limit = 1520, }, };
+ +static uint16_t get_sku_icc_max(int domain) +{ + const uint16_t tdp = cpu_get_power_max(); + + static uint16_t mch_id = 0, igd_id = 0; + if (!mch_id) { + struct device *dev = pcidev_path_on_root(SA_DEVFN_ROOT); + mch_id = dev ? pci_read_config16(dev, PCI_DEVICE_ID) : 0xffff; + } + if (!igd_id) { + struct device *dev = pcidev_path_on_root(SA_DEVFN_IGD); + igd_id = dev ? pci_read_config16(dev, PCI_DEVICE_ID) : 0xffff; + } + + /* + * Iccmax table from Doc #337344 Section 7.2 DC Specifications for CFL. + * Iccmax table from Doc #338023 Section 7.2 DC Specifications for WHL. + * + * Platform Segment SA IA GT (GT/GTx) + * --------------------------------------------------------------------- + * CFL-U (28W) GT3 quad 8.5 64 64 + * CFL-U (28W) GT3 dual 8.5 64 64 + * + * CFL-H (45W) GT2 hex 11.1 128 0 + * CFL-H (45W) GT2 quad 11.1 86 0 + * + * CFL-S (95W) GT2 octa 11.1 193 45 + * + * CFL-S (95W) GT2 hex 11.1 138 45 + * CFL-S (65W) GT2 hex 11.1 133 45 + * CFL-S (80W) GT2 hex 11.1 133 45 + * CFL-S (35W) GT2 hex 11.1 104 35 + * + * CFL-S (91W) GT2 quad 11.1 100 45 + * CFL-S (83W) GT2 quad 11.1 100 45 + * CFL-S (71W) GT2 quad 11.1 100 45 + * CFL-S (65W) GT2 quad 11.1 79 45 + * CFL-S (62W) GT2 quad 11.1 79 45 + * CFL-S (35W) GT2 quad 11.1 66 35 + * + * CFL-S (58W) GT2 dual 11.1 79 45 + * CFL-S (54W) GT2 dual 11.1 58 45 + * CFL-S (35W) GT2 dual 11.1 40 35 + * + * CNL-U (15W) 13 34 0 + * + * WHL-U (15W) GT2 quad 6 70 31 + * WHL-U (15W) GT2 dual 6 35 31 + * + * GT0 versions are the same as GT2/GT3, but have GT/GTx set to 0. + */ + + if (igd_id == 0xffff && ((domain == VR_GT_SLICED) || (domain == VR_GT_UNSLICED))) + return 0; + + switch (mch_id) { + case PCI_DEVICE_ID_INTEL_CNL_ID_U: /* fallthrough */ + case PCI_DEVICE_ID_INTEL_CNL_ID_Y: { + uint16_t icc_max[NUM_VR_DOMAINS] = VR_CFG_ALL_DOMAINS_ICC(13, 34, 0, 0); + + return icc_max[domain]; + } + case PCI_DEVICE_ID_INTEL_WHL_ID_W_4: { + uint16_t icc_max[NUM_VR_DOMAINS] = VR_CFG_ALL_DOMAINS_ICC(6, 70, 31, 31); + + return icc_max[domain]; + } + case PCI_DEVICE_ID_INTEL_WHL_ID_W_2: { + uint16_t icc_max[NUM_VR_DOMAINS] = VR_CFG_ALL_DOMAINS_ICC(6, 35, 31, 31); + + return icc_max[domain]; + } + case PCI_DEVICE_ID_INTEL_CFL_ID_U: /* fallthrough */ + case PCI_DEVICE_ID_INTEL_CFL_ID_U_2: { + uint16_t icc_max[NUM_VR_DOMAINS] = VR_CFG_ALL_DOMAINS_ICC(8.5, 64, 64, 64); + + return icc_max[domain]; + } + case PCI_DEVICE_ID_INTEL_CFL_ID_H_8: /* fallthrough - undocumented */ + case PCI_DEVICE_ID_INTEL_CFL_ID_H: { /* 6 core */ + uint16_t icc_max[NUM_VR_DOMAINS] = VR_CFG_ALL_DOMAINS_ICC(11.1, 128, 0, 0); + + return icc_max[domain]; + } + case PCI_DEVICE_ID_INTEL_CFL_ID_H_4: { /* 4 core */ + uint16_t icc_max[NUM_VR_DOMAINS] = VR_CFG_ALL_DOMAINS_ICC(11.1, 86, 0, 0); + + return icc_max[domain]; + } + case PCI_DEVICE_ID_INTEL_CFL_ID_S_DT_2: { /* 2 core */ + uint16_t icc_max[NUM_VR_DOMAINS] = VR_CFG_ALL_DOMAINS_ICC(11.1, 40, 35, 35); + + if (tdp >= 54) { + if (tdp >= 58) + icc_max[VR_IA_CORE] = VR_CFG_AMP(79); + else + icc_max[VR_IA_CORE] = VR_CFG_AMP(58); + + icc_max[VR_GT_SLICED] = VR_CFG_AMP(45); + icc_max[VR_GT_UNSLICED] = VR_CFG_AMP(45); + } + + return icc_max[domain]; + } + case PCI_DEVICE_ID_INTEL_CFL_ID_S_S_8: { + uint16_t icc_max[NUM_VR_DOMAINS] = VR_CFG_ALL_DOMAINS_ICC(11.1, 193, 45, 45); + + return icc_max[domain]; + } + case PCI_DEVICE_ID_INTEL_CFL_ID_S: /* fallthrough */ + case PCI_DEVICE_ID_INTEL_CFL_ID_S_WS_6: /* fallthrough */ + case PCI_DEVICE_ID_INTEL_CFL_ID_S_S_6: { + uint16_t icc_max[NUM_VR_DOMAINS] = VR_CFG_ALL_DOMAINS_ICC(11.1, 104, 35, 35); + if (tdp >= 54) { + if (tdp >= 95) + icc_max[VR_IA_CORE] = VR_CFG_AMP(138); + else if (tdp >= 65) + icc_max[VR_IA_CORE] = VR_CFG_AMP(133); + + icc_max[VR_GT_SLICED] = VR_CFG_AMP(45); + icc_max[VR_GT_UNSLICED] = VR_CFG_AMP(45); + } + + return icc_max[domain]; + } + case PCI_DEVICE_ID_INTEL_CFL_ID_S_S_4: /* fallthrough */ + case PCI_DEVICE_ID_INTEL_CFL_ID_S_WS_4: /* fallthrough */ + case PCI_DEVICE_ID_INTEL_CFL_ID_S_DT_4: { + uint16_t icc_max[NUM_VR_DOMAINS] = VR_CFG_ALL_DOMAINS_ICC(11.1, 66, 35, 35); + if (tdp >= 54) { + if (tdp >= 71) + icc_max[VR_IA_CORE] = VR_CFG_AMP(100); + else if (tdp >= 62) + icc_max[VR_IA_CORE] = VR_CFG_AMP(79); + + icc_max[VR_GT_SLICED] = VR_CFG_AMP(45); + icc_max[VR_GT_UNSLICED] = VR_CFG_AMP(45); + } + + return icc_max[domain]; + } + default: + printk(BIOS_ERR, "ERROR: Unknown MCH (0x%x) in VR-config\n", mch_id); + } + return 0; +} + +static uint16_t get_sku_ac_dc_loadline(const int domain) +{ + static uint16_t mch_id = 0; + if (!mch_id) { + struct device *dev = pcidev_path_on_root(SA_DEVFN_ROOT); + mch_id = dev ? pci_read_config16(dev, PCI_DEVICE_ID) : 0xffff; + } + + switch (mch_id) { + case PCI_DEVICE_ID_INTEL_WHL_ID_W_4: /* fallthrough */ + case PCI_DEVICE_ID_INTEL_CFL_ID_H_8: /* fallthrough */ + case PCI_DEVICE_ID_INTEL_CFL_ID_H: /* fallthrough */ + case PCI_DEVICE_ID_INTEL_CFL_ID_H_4: { /* fallthrough */ + uint16_t loadline[NUM_VR_DOMAINS] = + VR_CFG_ALL_DOMAINS_LOADLINE(10.3, 1.8, 2.7, 2.7); + if (mch_id == PCI_DEVICE_ID_INTEL_WHL_ID_W_4) { + loadline[VR_GT_SLICED] = 0; /* unspecified */ + loadline[VR_GT_UNSLICED] = 0; /* unspecified */ + } + return loadline[domain]; + } + case PCI_DEVICE_ID_INTEL_WHL_ID_W_2: /* fallthrough */ + case PCI_DEVICE_ID_INTEL_CFL_ID_U: /* fallthrough */ + case PCI_DEVICE_ID_INTEL_CFL_ID_U_2: /* fallthrough */ + case PCI_DEVICE_ID_INTEL_CNL_ID_U: /* fallthrough */ + case PCI_DEVICE_ID_INTEL_CNL_ID_Y: { + uint16_t loadline[NUM_VR_DOMAINS] = + VR_CFG_ALL_DOMAINS_LOADLINE(10.3, 2.4, 2.0, 2.0); + if (mch_id == PCI_DEVICE_ID_INTEL_WHL_ID_W_2) { + loadline[VR_GT_SLICED] = 0; /* unspecified */ + loadline[VR_GT_UNSLICED] = 0; /* unspecified */ + } + return loadline[domain]; + } + case PCI_DEVICE_ID_INTEL_CFL_ID_S_S_8: { + /* FIXME: Loadline isn't specified for S-series, using H-series default */ + const uint16_t loadline[NUM_VR_DOMAINS] = + VR_CFG_ALL_DOMAINS_LOADLINE(10.3, 1.6, 3.1, 3.1); + return loadline[domain]; + } + case PCI_DEVICE_ID_INTEL_CFL_ID_S: /* fallthrough */ + case PCI_DEVICE_ID_INTEL_CFL_ID_S_WS_6: /* fallthrough */ + case PCI_DEVICE_ID_INTEL_CFL_ID_S_S_6: /* fallthrough */ + case PCI_DEVICE_ID_INTEL_CFL_ID_S_S_4: /* fallthrough */ + case PCI_DEVICE_ID_INTEL_CFL_ID_S_WS_4: /* fallthrough */ + case PCI_DEVICE_ID_INTEL_CFL_ID_S_DT_4: /* fallthrough */ + case PCI_DEVICE_ID_INTEL_CFL_ID_S_DT_2: { + /* FIXME: Loadline isn't specified for S-series, using H-series default */ + const uint16_t loadline[NUM_VR_DOMAINS] = + VR_CFG_ALL_DOMAINS_LOADLINE(10.3, 2.1, 3.1, 3.1); + return loadline[domain]; + } + default: + printk(BIOS_ERR, "ERROR: Unknown MCH (0x%x) in VR-config\n", mch_id); + } + return 0; +} + +static uint16_t get_sku_voltagelimit(int domain) +{ + return 1520; +} + void fill_vr_domain_config(void *params, int domain, const struct vr_config *chip_cfg) { @@ -92,8 +307,25 @@ vr_params->Psi4Enable[domain] = cfg->psi4enable; vr_params->ImonSlope[domain] = cfg->imon_slope; vr_params->ImonOffset[domain] = cfg->imon_offset; - vr_params->IccMax[domain] = cfg->icc_max; - vr_params->VrVoltageLimit[domain] = cfg->voltage_limit; - vr_params->AcLoadline[domain] = cfg->ac_loadline; - vr_params->DcLoadline[domain] = cfg->dc_loadline; + + /* If board provided non-zero value, use it. */ + if (cfg->voltage_limit) + vr_params->VrVoltageLimit[domain] = cfg->voltage_limit; + else + vr_params->VrVoltageLimit[domain] = get_sku_voltagelimit(domain); + + if (cfg->icc_max) + vr_params->IccMax[domain] = cfg->icc_max; + else + vr_params->IccMax[domain] = get_sku_icc_max(domain); + + if (cfg->ac_loadline) + vr_params->AcLoadline[domain] = cfg->ac_loadline; + else + vr_params->AcLoadline[domain] = get_sku_ac_dc_loadline(domain); + + if (cfg->dc_loadline) + vr_params->DcLoadline[domain] = cfg->dc_loadline; + else + vr_params->DcLoadline[domain] = get_sku_ac_dc_loadline(domain); }
-
Angel Pons (Code Review) -
build bot (Jenkins) (Code Review)
-
Edward O'Callaghan (Code Review)
-
Jamie Chen (Code Review)
-
Maxim Polyakov (Code Review)
-
Patrick Rudolph (Code Review)
-
Philipp Deppenwiese (Code Review)
-
Tim Wawrzynczak (Code Review)
-
Wim Vervoorn (Code Review)