Hello Tim Chen,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/40828
to review the following change.
Change subject: mb/google/hatch/vr/puff: Add psys_pmax calculation ......................................................................
mb/google/hatch/vr/puff: Add psys_pmax calculation
This patch adds psys_pmax calculation. There are two types of power sources. One is barrel jack and the other is USB TYPE-C. The voltage level is fixed for a barrel jack while TYPE-C may vary depending on power ratings. We need to get voltage information from EC and calculate correct psys_pmax value. The psys_pmax needs to be set before FSP-S since FSP-S will handle the setting passing to pcode, so move the routine ahead to variant_ramstage_init.
BUG=b:151972149 TEST=emerge-puff coreboot chromeos-bootimage check firmware log and ensure psys_pmax is passed to FSP check the data from dump_intel_rapl_consumption in the OS and ensure the power data is close to an external power meter.
Change-Id: Iff767d4b44a01e766258345545438a54a16d1af5 Signed-off-by: Tim Chen tim-chen@quanta.corp-partner.google.com --- M src/mainboard/google/hatch/variants/duffy/mainboard.c M src/mainboard/google/hatch/variants/kaisa/mainboard.c 2 files changed, 74 insertions(+), 36 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/28/40828/1
diff --git a/src/mainboard/google/hatch/variants/duffy/mainboard.c b/src/mainboard/google/hatch/variants/duffy/mainboard.c index 3f74c96..ceeb0c5 100644 --- a/src/mainboard/google/hatch/variants/duffy/mainboard.c +++ b/src/mainboard/google/hatch/variants/duffy/mainboard.c @@ -31,23 +31,6 @@ stopwatch_duration_msecs(&sw)); }
-void variant_ramstage_init(void) -{ - static const long display_timeout_ms = 3000; - - /* This is reconfigured back to whatever FSP-S expects by - gpio_configure_pads. */ - gpio_input(GPIO_HDMI_HPD); - gpio_input(GPIO_DP_HPD); - if (display_init_required() - && !gpio_get(GPIO_HDMI_HPD) - && !gpio_get(GPIO_DP_HPD)) { - /* This has to be done before FSP-S runs. */ - if (google_chromeec_wait_for_displayport(display_timeout_ms)) - wait_for_hpd(GPIO_DP_HPD, display_timeout_ms); - } -} - /* * For type-C chargers, set PL2 to 90% of max power to account for * cable loss and FET Rdson loss in the path from the source. @@ -86,6 +69,24 @@ * | n (U22) | 29 | .9n | .9n | x(43) | * +-------------+-----+---------+---------+-------+ */ + +/* + * Psys_pmax considerations + * + * Given the hardware design in puff, the serial shunt resistor is 0.01ohm. + * The full scale of hardware PSYS signal 0.8v maps to system current 9.6A + * instead of real system power. The equation is shown below: + * PSYS = 0.8v = (0.01ohm x Iinput) x 50 (INA213, gain 50V/V) x 15k/(15k + 75k) + * Hence, Iinput (Amps) = 9.6A + * Since there is no voltage information from PSYS, different voltage input + * would map to different Psys_pmax settings: + * For Type-C 15V, the Psys_pmax sholud be 15v x 9.6A = 144W + * For Type-C 20V, the Psys_pmax should be 20v x 9.6A = 192W + * For a barral jack, the Psys_pmax should be 19v x 9.6A = 182.4W + */ +#define PSYS_IMAX 9600 +#define BJ_VOLTS_MV 19000 + static void mainboard_set_power_limits(config_t *conf) { enum usb_chg_type type; @@ -108,15 +109,33 @@ /* set minimum duty cycle */ conf->tdp_psyspl3_dutycycle = PUFF_MIN_DUTYCYCLE; conf->tdp_pl4 = SET_PSYSPL2(psyspl2); + } else { + /* Input type is barrel jack */ + volts_mv = BJ_VOLTS_MV; } + /* voltage unit is milliVolts and current is in milliAmps */ + conf->psys_pmax = (u16)(((u32)PSYS_IMAX * volts_mv) / 1000000);
conf->tdp_pl2_override = PUFF_PL2; /* set psyspl2 to 90% of max adapter power */ conf->tdp_psyspl2 = SET_PSYSPL2(psyspl2); }
-void variant_mainboard_enable(struct device *dev) +void variant_ramstage_init(void) { + static const long display_timeout_ms = 3000; config_t *conf = config_of_soc(); + + /* This is reconfigured back to whatever FSP-S expects by gpio_configure_pads. */ + gpio_input(GPIO_HDMI_HPD); + gpio_input(GPIO_DP_HPD); + if (display_init_required() + && !gpio_get(GPIO_HDMI_HPD) + && !gpio_get(GPIO_DP_HPD)) { + /* This has to be done before FSP-S runs. */ + if (google_chromeec_wait_for_displayport(display_timeout_ms)) + wait_for_hpd(GPIO_DP_HPD, display_timeout_ms); + } + /* Psys_pmax needs to be setup before FSP-S */ mainboard_set_power_limits(conf); } diff --git a/src/mainboard/google/hatch/variants/kaisa/mainboard.c b/src/mainboard/google/hatch/variants/kaisa/mainboard.c index 3f74c96..ceeb0c5 100644 --- a/src/mainboard/google/hatch/variants/kaisa/mainboard.c +++ b/src/mainboard/google/hatch/variants/kaisa/mainboard.c @@ -31,23 +31,6 @@ stopwatch_duration_msecs(&sw)); }
-void variant_ramstage_init(void) -{ - static const long display_timeout_ms = 3000; - - /* This is reconfigured back to whatever FSP-S expects by - gpio_configure_pads. */ - gpio_input(GPIO_HDMI_HPD); - gpio_input(GPIO_DP_HPD); - if (display_init_required() - && !gpio_get(GPIO_HDMI_HPD) - && !gpio_get(GPIO_DP_HPD)) { - /* This has to be done before FSP-S runs. */ - if (google_chromeec_wait_for_displayport(display_timeout_ms)) - wait_for_hpd(GPIO_DP_HPD, display_timeout_ms); - } -} - /* * For type-C chargers, set PL2 to 90% of max power to account for * cable loss and FET Rdson loss in the path from the source. @@ -86,6 +69,24 @@ * | n (U22) | 29 | .9n | .9n | x(43) | * +-------------+-----+---------+---------+-------+ */ + +/* + * Psys_pmax considerations + * + * Given the hardware design in puff, the serial shunt resistor is 0.01ohm. + * The full scale of hardware PSYS signal 0.8v maps to system current 9.6A + * instead of real system power. The equation is shown below: + * PSYS = 0.8v = (0.01ohm x Iinput) x 50 (INA213, gain 50V/V) x 15k/(15k + 75k) + * Hence, Iinput (Amps) = 9.6A + * Since there is no voltage information from PSYS, different voltage input + * would map to different Psys_pmax settings: + * For Type-C 15V, the Psys_pmax sholud be 15v x 9.6A = 144W + * For Type-C 20V, the Psys_pmax should be 20v x 9.6A = 192W + * For a barral jack, the Psys_pmax should be 19v x 9.6A = 182.4W + */ +#define PSYS_IMAX 9600 +#define BJ_VOLTS_MV 19000 + static void mainboard_set_power_limits(config_t *conf) { enum usb_chg_type type; @@ -108,15 +109,33 @@ /* set minimum duty cycle */ conf->tdp_psyspl3_dutycycle = PUFF_MIN_DUTYCYCLE; conf->tdp_pl4 = SET_PSYSPL2(psyspl2); + } else { + /* Input type is barrel jack */ + volts_mv = BJ_VOLTS_MV; } + /* voltage unit is milliVolts and current is in milliAmps */ + conf->psys_pmax = (u16)(((u32)PSYS_IMAX * volts_mv) / 1000000);
conf->tdp_pl2_override = PUFF_PL2; /* set psyspl2 to 90% of max adapter power */ conf->tdp_psyspl2 = SET_PSYSPL2(psyspl2); }
-void variant_mainboard_enable(struct device *dev) +void variant_ramstage_init(void) { + static const long display_timeout_ms = 3000; config_t *conf = config_of_soc(); + + /* This is reconfigured back to whatever FSP-S expects by gpio_configure_pads. */ + gpio_input(GPIO_HDMI_HPD); + gpio_input(GPIO_DP_HPD); + if (display_init_required() + && !gpio_get(GPIO_HDMI_HPD) + && !gpio_get(GPIO_DP_HPD)) { + /* This has to be done before FSP-S runs. */ + if (google_chromeec_wait_for_displayport(display_timeout_ms)) + wait_for_hpd(GPIO_DP_HPD, display_timeout_ms); + } + /* Psys_pmax needs to be setup before FSP-S */ mainboard_set_power_limits(conf); }
Tim Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40828 )
Change subject: mb/google/hatch/vr/puff: Add psys_pmax calculation ......................................................................
Patch Set 1:
Do Kaisa and Duffy need this CL? This copies from https://review.coreboot.org/c/coreboot/+/40482 .
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40828 )
Change subject: mb/google/hatch/vr/puff: Add psys_pmax calculation ......................................................................
Patch Set 1: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/40828 )
Change subject: mb/google/hatch/vr/puff: Add psys_pmax calculation ......................................................................
mb/google/hatch/vr/puff: Add psys_pmax calculation
This patch adds psys_pmax calculation. There are two types of power sources. One is barrel jack and the other is USB TYPE-C. The voltage level is fixed for a barrel jack while TYPE-C may vary depending on power ratings. We need to get voltage information from EC and calculate correct psys_pmax value. The psys_pmax needs to be set before FSP-S since FSP-S will handle the setting passing to pcode, so move the routine ahead to variant_ramstage_init.
BUG=b:151972149 TEST=emerge-puff coreboot chromeos-bootimage check firmware log and ensure psys_pmax is passed to FSP check the data from dump_intel_rapl_consumption in the OS and ensure the power data is close to an external power meter.
Change-Id: Iff767d4b44a01e766258345545438a54a16d1af5 Signed-off-by: Tim Chen tim-chen@quanta.corp-partner.google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/40828 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Edward O'Callaghan quasisec@chromium.org --- M src/mainboard/google/hatch/variants/duffy/mainboard.c M src/mainboard/google/hatch/variants/kaisa/mainboard.c 2 files changed, 74 insertions(+), 36 deletions(-)
Approvals: build bot (Jenkins): Verified Edward O'Callaghan: Looks good to me, approved
diff --git a/src/mainboard/google/hatch/variants/duffy/mainboard.c b/src/mainboard/google/hatch/variants/duffy/mainboard.c index 3f74c96..ceeb0c5 100644 --- a/src/mainboard/google/hatch/variants/duffy/mainboard.c +++ b/src/mainboard/google/hatch/variants/duffy/mainboard.c @@ -31,23 +31,6 @@ stopwatch_duration_msecs(&sw)); }
-void variant_ramstage_init(void) -{ - static const long display_timeout_ms = 3000; - - /* This is reconfigured back to whatever FSP-S expects by - gpio_configure_pads. */ - gpio_input(GPIO_HDMI_HPD); - gpio_input(GPIO_DP_HPD); - if (display_init_required() - && !gpio_get(GPIO_HDMI_HPD) - && !gpio_get(GPIO_DP_HPD)) { - /* This has to be done before FSP-S runs. */ - if (google_chromeec_wait_for_displayport(display_timeout_ms)) - wait_for_hpd(GPIO_DP_HPD, display_timeout_ms); - } -} - /* * For type-C chargers, set PL2 to 90% of max power to account for * cable loss and FET Rdson loss in the path from the source. @@ -86,6 +69,24 @@ * | n (U22) | 29 | .9n | .9n | x(43) | * +-------------+-----+---------+---------+-------+ */ + +/* + * Psys_pmax considerations + * + * Given the hardware design in puff, the serial shunt resistor is 0.01ohm. + * The full scale of hardware PSYS signal 0.8v maps to system current 9.6A + * instead of real system power. The equation is shown below: + * PSYS = 0.8v = (0.01ohm x Iinput) x 50 (INA213, gain 50V/V) x 15k/(15k + 75k) + * Hence, Iinput (Amps) = 9.6A + * Since there is no voltage information from PSYS, different voltage input + * would map to different Psys_pmax settings: + * For Type-C 15V, the Psys_pmax sholud be 15v x 9.6A = 144W + * For Type-C 20V, the Psys_pmax should be 20v x 9.6A = 192W + * For a barral jack, the Psys_pmax should be 19v x 9.6A = 182.4W + */ +#define PSYS_IMAX 9600 +#define BJ_VOLTS_MV 19000 + static void mainboard_set_power_limits(config_t *conf) { enum usb_chg_type type; @@ -108,15 +109,33 @@ /* set minimum duty cycle */ conf->tdp_psyspl3_dutycycle = PUFF_MIN_DUTYCYCLE; conf->tdp_pl4 = SET_PSYSPL2(psyspl2); + } else { + /* Input type is barrel jack */ + volts_mv = BJ_VOLTS_MV; } + /* voltage unit is milliVolts and current is in milliAmps */ + conf->psys_pmax = (u16)(((u32)PSYS_IMAX * volts_mv) / 1000000);
conf->tdp_pl2_override = PUFF_PL2; /* set psyspl2 to 90% of max adapter power */ conf->tdp_psyspl2 = SET_PSYSPL2(psyspl2); }
-void variant_mainboard_enable(struct device *dev) +void variant_ramstage_init(void) { + static const long display_timeout_ms = 3000; config_t *conf = config_of_soc(); + + /* This is reconfigured back to whatever FSP-S expects by gpio_configure_pads. */ + gpio_input(GPIO_HDMI_HPD); + gpio_input(GPIO_DP_HPD); + if (display_init_required() + && !gpio_get(GPIO_HDMI_HPD) + && !gpio_get(GPIO_DP_HPD)) { + /* This has to be done before FSP-S runs. */ + if (google_chromeec_wait_for_displayport(display_timeout_ms)) + wait_for_hpd(GPIO_DP_HPD, display_timeout_ms); + } + /* Psys_pmax needs to be setup before FSP-S */ mainboard_set_power_limits(conf); } diff --git a/src/mainboard/google/hatch/variants/kaisa/mainboard.c b/src/mainboard/google/hatch/variants/kaisa/mainboard.c index 3f74c96..ceeb0c5 100644 --- a/src/mainboard/google/hatch/variants/kaisa/mainboard.c +++ b/src/mainboard/google/hatch/variants/kaisa/mainboard.c @@ -31,23 +31,6 @@ stopwatch_duration_msecs(&sw)); }
-void variant_ramstage_init(void) -{ - static const long display_timeout_ms = 3000; - - /* This is reconfigured back to whatever FSP-S expects by - gpio_configure_pads. */ - gpio_input(GPIO_HDMI_HPD); - gpio_input(GPIO_DP_HPD); - if (display_init_required() - && !gpio_get(GPIO_HDMI_HPD) - && !gpio_get(GPIO_DP_HPD)) { - /* This has to be done before FSP-S runs. */ - if (google_chromeec_wait_for_displayport(display_timeout_ms)) - wait_for_hpd(GPIO_DP_HPD, display_timeout_ms); - } -} - /* * For type-C chargers, set PL2 to 90% of max power to account for * cable loss and FET Rdson loss in the path from the source. @@ -86,6 +69,24 @@ * | n (U22) | 29 | .9n | .9n | x(43) | * +-------------+-----+---------+---------+-------+ */ + +/* + * Psys_pmax considerations + * + * Given the hardware design in puff, the serial shunt resistor is 0.01ohm. + * The full scale of hardware PSYS signal 0.8v maps to system current 9.6A + * instead of real system power. The equation is shown below: + * PSYS = 0.8v = (0.01ohm x Iinput) x 50 (INA213, gain 50V/V) x 15k/(15k + 75k) + * Hence, Iinput (Amps) = 9.6A + * Since there is no voltage information from PSYS, different voltage input + * would map to different Psys_pmax settings: + * For Type-C 15V, the Psys_pmax sholud be 15v x 9.6A = 144W + * For Type-C 20V, the Psys_pmax should be 20v x 9.6A = 192W + * For a barral jack, the Psys_pmax should be 19v x 9.6A = 182.4W + */ +#define PSYS_IMAX 9600 +#define BJ_VOLTS_MV 19000 + static void mainboard_set_power_limits(config_t *conf) { enum usb_chg_type type; @@ -108,15 +109,33 @@ /* set minimum duty cycle */ conf->tdp_psyspl3_dutycycle = PUFF_MIN_DUTYCYCLE; conf->tdp_pl4 = SET_PSYSPL2(psyspl2); + } else { + /* Input type is barrel jack */ + volts_mv = BJ_VOLTS_MV; } + /* voltage unit is milliVolts and current is in milliAmps */ + conf->psys_pmax = (u16)(((u32)PSYS_IMAX * volts_mv) / 1000000);
conf->tdp_pl2_override = PUFF_PL2; /* set psyspl2 to 90% of max adapter power */ conf->tdp_psyspl2 = SET_PSYSPL2(psyspl2); }
-void variant_mainboard_enable(struct device *dev) +void variant_ramstage_init(void) { + static const long display_timeout_ms = 3000; config_t *conf = config_of_soc(); + + /* This is reconfigured back to whatever FSP-S expects by gpio_configure_pads. */ + gpio_input(GPIO_HDMI_HPD); + gpio_input(GPIO_DP_HPD); + if (display_init_required() + && !gpio_get(GPIO_HDMI_HPD) + && !gpio_get(GPIO_DP_HPD)) { + /* This has to be done before FSP-S runs. */ + if (google_chromeec_wait_for_displayport(display_timeout_ms)) + wait_for_hpd(GPIO_DP_HPD, display_timeout_ms); + } + /* Psys_pmax needs to be setup before FSP-S */ mainboard_set_power_limits(conf); }
-
Edward O'Callaghan (Code Review) -
Patrick Georgi (Code Review)
-
Tim Chen (Code Review)