Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47746 )
Change subject: nb/intel/sandybridge: Refine power-down mode logic ......................................................................
nb/intel/sandybridge: Refine power-down mode logic
When memory is running at fast frequencies, power-down modes can lessen system stability. Check tXP and tXPDLL values and use safer power down modes if their values are high. Do not use APD with DLL-off on mobile: vendor firmware does not use it, and it can influence system stability.
Change-Id: Ic8e98162ca86ae454a8c951be163d58960940e0e Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/northbridge/intel/sandybridge/Kconfig M src/northbridge/intel/sandybridge/raminit_common.c M src/northbridge/intel/sandybridge/raminit_common.h 3 files changed, 41 insertions(+), 11 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/47746/1
diff --git a/src/northbridge/intel/sandybridge/Kconfig b/src/northbridge/intel/sandybridge/Kconfig index ef6dc3d..9cd522f 100644 --- a/src/northbridge/intel/sandybridge/Kconfig +++ b/src/northbridge/intel/sandybridge/Kconfig @@ -99,6 +99,15 @@ hex default 0x0
+config RAMINIT_ALWAYS_ALLOW_DLL_OFF + bool "Also enable memory DLL-off mode on desktops and servers" + default n + help + If enabled, allow enabling DLL-off mode for platforms other than + mobile. Saves power at the expense of higher exit latencies. Has + no effect on mobile platforms, where DLL-off is always allowed. + Power down is disabled for stability when running at high clocks. + config RAMINIT_ENABLE_ECC bool "Enable ECC if supported" default y diff --git a/src/northbridge/intel/sandybridge/raminit_common.c b/src/northbridge/intel/sandybridge/raminit_common.c index b06734c..d533ca8 100644 --- a/src/northbridge/intel/sandybridge/raminit_common.c +++ b/src/northbridge/intel/sandybridge/raminit_common.c @@ -688,11 +688,29 @@ iosav_run_once(channel); }
+/* Obtain optimal power down mode for current configuration */ +static enum pdwm_mode get_power_down_mode(ramctr_timing *ctrl) +{ + if (ctrl->tXP > 8) + return PDM_NONE; + + if (ctrl->tXPDLL > 32) + return PDM_PPD; + + if (CONFIG(RAMINIT_ALWAYS_ALLOW_DLL_OFF) || get_platform_type() == PLATFORM_MOBILE) + return PDM_DLL_OFF; + + return PDM_APD_PPD; +} + static u32 make_mr0(ramctr_timing *ctrl, u8 rank) { u16 mr0reg, mch_cas, mch_wr; static const u8 mch_wr_t[12] = { 1, 2, 3, 4, 0, 5, 0, 6, 0, 7, 0, 0 }; - const size_t is_mobile = get_platform_type() == PLATFORM_MOBILE; + + const enum pdwm_mode power_down = get_power_down_mode(ctrl); + + const bool slow_exit = power_down == PDM_DLL_OFF || power_down == PDM_APD_DLL_OFF;
/* Convert CAS to MCH register friendly */ if (ctrl->CAS < 12) { @@ -712,8 +730,8 @@ mr0reg |= (mch_cas & 0xe) << 3; mr0reg |= mch_wr << 9;
- /* Precharge PD - Fast (desktop) 1 or slow (mobile) 0 - mostly power-saving feature */ - mr0reg |= !is_mobile << 12; + /* Precharge PD - Use slow exit when DLL-off is used - mostly power-saving feature */ + mr0reg |= !slow_exit << 12; return mr0reg; }
@@ -2830,8 +2848,6 @@ /* FIXME: values in this function should be hardware revision-dependent */ void final_registers(ramctr_timing *ctrl) { - const bool is_mobile = get_platform_type() == PLATFORM_MOBILE; - int channel; int t1_cycles = 0, t1_ns = 0, t2_ns; int t3_ns; @@ -2848,12 +2864,8 @@ MCHBAR32(TC_OTHP_ch(channel)) = tc_othp.raw; }
- if (is_mobile) - /* APD - DLL Off, 64 DCLKs until idle, decision per rank */ - MCHBAR32(PM_PDWN_CONFIG) = 0x00000740; - else - /* APD - PPD, 64 DCLKs until idle, decision per rank */ - MCHBAR32(PM_PDWN_CONFIG) = 0x00000340; + /* 64 DCLKs until idle, decision per rank */ + MCHBAR32(PM_PDWN_CONFIG) = get_power_down_mode(ctrl) << 8 | 64;
FOR_ALL_CHANNELS MCHBAR32(PM_TRML_M_CONFIG_ch(channel)) = 0x00000aaa; diff --git a/src/northbridge/intel/sandybridge/raminit_common.h b/src/northbridge/intel/sandybridge/raminit_common.h index 050aa70..80d3074 100644 --- a/src/northbridge/intel/sandybridge/raminit_common.h +++ b/src/northbridge/intel/sandybridge/raminit_common.h @@ -273,6 +273,15 @@ */ #define MRC_CACHE_VERSION 5
+enum pdwm_mode { + PDM_NONE = 0, + PDM_APD = 1, + PDM_PPD = 2, + PDM_APD_PPD = 3, + PDM_DLL_OFF = 6, + PDM_APD_DLL_OFF = 7, +}; + typedef struct odtmap_st { u16 rttwr; u16 rttnom;
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47746 )
Change subject: nb/intel/sandybridge: Refine power-down mode logic ......................................................................
Patch Set 4: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/47746 )
Change subject: nb/intel/sandybridge: Refine power-down mode logic ......................................................................
nb/intel/sandybridge: Refine power-down mode logic
When memory is running at fast frequencies, power-down modes can lessen system stability. Check tXP and tXPDLL values and use safer power down modes if their values are high. Do not use APD with DLL-off on mobile: vendor firmware does not use it, and it can influence system stability.
Change-Id: Ic8e98162ca86ae454a8c951be163d58960940e0e Signed-off-by: Angel Pons th3fanbus@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/47746 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Arthur Heymans arthur@aheymans.xyz --- M src/northbridge/intel/sandybridge/Kconfig M src/northbridge/intel/sandybridge/raminit_common.c M src/northbridge/intel/sandybridge/raminit_common.h 3 files changed, 41 insertions(+), 11 deletions(-)
Approvals: build bot (Jenkins): Verified Arthur Heymans: Looks good to me, approved
diff --git a/src/northbridge/intel/sandybridge/Kconfig b/src/northbridge/intel/sandybridge/Kconfig index ef6dc3d..9cd522f 100644 --- a/src/northbridge/intel/sandybridge/Kconfig +++ b/src/northbridge/intel/sandybridge/Kconfig @@ -99,6 +99,15 @@ hex default 0x0
+config RAMINIT_ALWAYS_ALLOW_DLL_OFF + bool "Also enable memory DLL-off mode on desktops and servers" + default n + help + If enabled, allow enabling DLL-off mode for platforms other than + mobile. Saves power at the expense of higher exit latencies. Has + no effect on mobile platforms, where DLL-off is always allowed. + Power down is disabled for stability when running at high clocks. + config RAMINIT_ENABLE_ECC bool "Enable ECC if supported" default y diff --git a/src/northbridge/intel/sandybridge/raminit_common.c b/src/northbridge/intel/sandybridge/raminit_common.c index b06734c..d533ca8 100644 --- a/src/northbridge/intel/sandybridge/raminit_common.c +++ b/src/northbridge/intel/sandybridge/raminit_common.c @@ -688,11 +688,29 @@ iosav_run_once(channel); }
+/* Obtain optimal power down mode for current configuration */ +static enum pdwm_mode get_power_down_mode(ramctr_timing *ctrl) +{ + if (ctrl->tXP > 8) + return PDM_NONE; + + if (ctrl->tXPDLL > 32) + return PDM_PPD; + + if (CONFIG(RAMINIT_ALWAYS_ALLOW_DLL_OFF) || get_platform_type() == PLATFORM_MOBILE) + return PDM_DLL_OFF; + + return PDM_APD_PPD; +} + static u32 make_mr0(ramctr_timing *ctrl, u8 rank) { u16 mr0reg, mch_cas, mch_wr; static const u8 mch_wr_t[12] = { 1, 2, 3, 4, 0, 5, 0, 6, 0, 7, 0, 0 }; - const size_t is_mobile = get_platform_type() == PLATFORM_MOBILE; + + const enum pdwm_mode power_down = get_power_down_mode(ctrl); + + const bool slow_exit = power_down == PDM_DLL_OFF || power_down == PDM_APD_DLL_OFF;
/* Convert CAS to MCH register friendly */ if (ctrl->CAS < 12) { @@ -712,8 +730,8 @@ mr0reg |= (mch_cas & 0xe) << 3; mr0reg |= mch_wr << 9;
- /* Precharge PD - Fast (desktop) 1 or slow (mobile) 0 - mostly power-saving feature */ - mr0reg |= !is_mobile << 12; + /* Precharge PD - Use slow exit when DLL-off is used - mostly power-saving feature */ + mr0reg |= !slow_exit << 12; return mr0reg; }
@@ -2830,8 +2848,6 @@ /* FIXME: values in this function should be hardware revision-dependent */ void final_registers(ramctr_timing *ctrl) { - const bool is_mobile = get_platform_type() == PLATFORM_MOBILE; - int channel; int t1_cycles = 0, t1_ns = 0, t2_ns; int t3_ns; @@ -2848,12 +2864,8 @@ MCHBAR32(TC_OTHP_ch(channel)) = tc_othp.raw; }
- if (is_mobile) - /* APD - DLL Off, 64 DCLKs until idle, decision per rank */ - MCHBAR32(PM_PDWN_CONFIG) = 0x00000740; - else - /* APD - PPD, 64 DCLKs until idle, decision per rank */ - MCHBAR32(PM_PDWN_CONFIG) = 0x00000340; + /* 64 DCLKs until idle, decision per rank */ + MCHBAR32(PM_PDWN_CONFIG) = get_power_down_mode(ctrl) << 8 | 64;
FOR_ALL_CHANNELS MCHBAR32(PM_TRML_M_CONFIG_ch(channel)) = 0x00000aaa; diff --git a/src/northbridge/intel/sandybridge/raminit_common.h b/src/northbridge/intel/sandybridge/raminit_common.h index 050aa70..80d3074 100644 --- a/src/northbridge/intel/sandybridge/raminit_common.h +++ b/src/northbridge/intel/sandybridge/raminit_common.h @@ -273,6 +273,15 @@ */ #define MRC_CACHE_VERSION 5
+enum pdwm_mode { + PDM_NONE = 0, + PDM_APD = 1, + PDM_PPD = 2, + PDM_APD_PPD = 3, + PDM_DLL_OFF = 6, + PDM_APD_DLL_OFF = 7, +}; + typedef struct odtmap_st { u16 rttwr; u16 rttnom;
Attention is currently required from: Angel Pons.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47746?usp=email )
Change subject: nb/intel/sandybridge: Refine power-down mode logic ......................................................................
Patch Set 5:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/47746/comment/f1f21838_b73f2f2c : PS5, Line 11: Do not use APD with DLL-off on mobile: : vendor firmware does not use it, and it can influence system stability. mrc.bin does in fact use this mode on mobile...
Attention is currently required from: Angel Pons.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47746?usp=email )
Change subject: nb/intel/sandybridge: Refine power-down mode logic ......................................................................
Patch Set 5:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/47746/comment/30db0cb5_8d5ef080 : PS5, Line 11: Do not use APD with DLL-off on mobile: : vendor firmware does not use it, and it can influence system stability.
mrc.bin does in fact use this mode on mobile...
on x220 to be specific. This might be causing stability problems: https://ticket.coreboot.org/issues/121
-
Angel Pons (Code Review) -
Arthur Heymans (Code Review)
-
Patrick Georgi (Code Review)