From 48715fb712512944aae0bd4154defab65d30dc12 Mon Sep 17 00:00:00 2001 From: Jacob Harvey Date: Wed, 7 Jun 2017 17:08:52 -0500 Subject: [PATCH] Turn off A17 if not needed Change-Id: I3b4b31e537586339f90ffd48b60ed93bfb531fea Reviewed-on: http://ralgit01.raleigh.ibm.com/gerrit1/41848 Dev-Ready: JACOB L. HARVEY Tested-by: Jenkins Server Tested-by: Hostboot CI Reviewed-by: STEPHEN GLANCY Reviewed-by: Louis Stermole Reviewed-by: Jennifer A. Stofer Reviewed-on: http://ralgit01.raleigh.ibm.com/gerrit1/41944 Reviewed-by: Hostboot Team Tested-by: Jenkins OP Build CI Tested-by: FSP CI Jenkins Reviewed-by: Daniel M. Crowell --- .../hwp/memory/lib/dimm/ddr4/mrs_load_ddr4.C | 59 +++++++++++++++++++ .../hwp/memory/lib/dimm/ddr4/mrs_load_ddr4.H | 33 +++++++++-- .../procedures/hwp/memory/lib/dimm/eff_dimm.C | 33 +++++++++-- .../p9/procedures/hwp/memory/lib/phy/seq.C | 1 - .../p9/procedures/hwp/memory/lib/phy/seq.H | 19 ++++-- .../memory/lib/spd/common/rcw_settings.H | 4 -- .../lib/spd/lrdimm/ddr4/lrdimm_raw_cards.C | 1 - .../lib/spd/rdimm/ddr4/rdimm_raw_cards.C | 7 --- 8 files changed, 132 insertions(+), 25 deletions(-) diff --git a/src/import/chips/p9/procedures/hwp/memory/lib/dimm/ddr4/mrs_load_ddr4.C b/src/import/chips/p9/procedures/hwp/memory/lib/dimm/ddr4/mrs_load_ddr4.C index 3805a7d5c17..22bd97ff779 100644 --- a/src/import/chips/p9/procedures/hwp/memory/lib/dimm/ddr4/mrs_load_ddr4.C +++ b/src/import/chips/p9/procedures/hwp/memory/lib/dimm/ddr4/mrs_load_ddr4.C @@ -70,6 +70,65 @@ fapi_try_exit: return fapi2::current_err; } +/// +/// @brief Helper function to determine whether the A17 is needed +/// @param[in] i_target the DIMM target +/// @param[out] o_is_needed boolean whether A17 should be turned on or off +/// @return fapi2::FAPI2_RC_SUCCESS if okay +/// @note Based off of Table 2.8 Proposed DDR4 Full spec update(79-4B) page 28 +/// +template<> +fapi2::ReturnCode is_a17_needed(const fapi2::Target& i_target, + bool& o_is_needed) +{ + uint8_t l_dram_density = 0; + uint8_t l_dram_width = 0; + + FAPI_TRY( eff_dram_density( i_target, l_dram_density) ); + FAPI_TRY( eff_dram_width( i_target, l_dram_width) ); + + o_is_needed = (l_dram_density == fapi2::ENUM_ATTR_EFF_DRAM_DENSITY_16G + && l_dram_width == fapi2::ENUM_ATTR_EFF_DRAM_WIDTH_X4) ? + true : false; + FAPI_INF("%s Turning A17 %s", mss::c_str(i_target), o_is_needed ? "on" : "off" ); + +fapi_try_exit: + return fapi2::current_err; +} + +/// +/// @brief Helper function to determine whether the A17 is needed +/// @param[in] i_target the MCA target +/// @param[out] o_is_needed boolean whether A17 should be turned on or off +/// @return fapi2::FAPI2_RC_SUCCESS if okay +/// @note Based off of Table 2.8 Proposed DDR4 Full spec update(79-4B) page 28 +/// +template<> +fapi2::ReturnCode is_a17_needed(const fapi2::Target& i_target, + bool& o_is_needed) +{ + // Set this to good in case no dimms and we're running unit tests + fapi2::current_err = fapi2::FAPI2_RC_SUCCESS; + + // Loop over the DIMMs and see if A17 is needed for one of them + // If so, we enable the parity bit in the PHY + for (const auto& l_dimm : mss::find_targets(i_target) ) + { + // Default to not used + // Using temp because we want to OR the two results together. Don't want the false to overwrite + bool l_temp = false; + FAPI_TRY( is_a17_needed( l_dimm, l_temp), "%s Failed to get a17 boolean", mss::c_str(l_dimm) ); + + if (l_temp == true) + { + o_is_needed = true; + } + } + +fapi_try_exit: + return fapi2::current_err; +} + namespace ddr4 { diff --git a/src/import/chips/p9/procedures/hwp/memory/lib/dimm/ddr4/mrs_load_ddr4.H b/src/import/chips/p9/procedures/hwp/memory/lib/dimm/ddr4/mrs_load_ddr4.H index 20e3aff35c0..98ddc889c04 100644 --- a/src/import/chips/p9/procedures/hwp/memory/lib/dimm/ddr4/mrs_load_ddr4.H +++ b/src/import/chips/p9/procedures/hwp/memory/lib/dimm/ddr4/mrs_load_ddr4.H @@ -147,11 +147,16 @@ fapi_try_exit: /// /// @brief Invert (side to side) the ADR bits of a CCS instruction +/// @tparam T the target type of the ccs instruction +/// @param[in] i_target the DIMM target of the ccs command /// @param[in] i_inst const reference to a CCS instruction. -/// @return[out] ccs instruction with the ADR bits inverted (side-to-side) +/// @param[in] l_is_a17 Boolean for whether A17 bit is enabled or not +/// @return ccs instruction with the ADR bits inverted (side-to-side) /// template -ccs::instruction_t address_invert(const ccs::instruction_t& i_inst) +ccs::instruction_t address_invert(const fapi2::Target& i_target, + const ccs::instruction_t& i_inst, + const bool i_is_a17 = false) { // Copy the input as the output doesn't all change. ccs::instruction_t i_out(i_inst); @@ -167,7 +172,12 @@ ccs::instruction_t address_invert(const ccs::instruction_t& i_inst) mss::template negate(i_out.arr0); mss::template negate(i_out.arr0); - mss::template negate(i_out.arr0); + + if (i_is_a17) + { + FAPI_INF("%s A17 is turned on, negating CCS bit A17", mss::c_str(i_target) ); + mss::template negate(i_out.arr0); + } mss::template negate(i_out.arr0); mss::template negate(i_out.arr0); @@ -245,6 +255,18 @@ fapi_try_exit: return fapi2::current_err; } +/// +/// @brief Helper function to determine whether the A17 is needed +/// @tparam T fapi2::TargetType DIMM or MCA +/// @param[in] i_target the target to check +/// @param[out] o_is_needed boolean whether A17 should be turned on or off +/// @return fapi2::FAPI2_RC_SUCCESS if okay +/// @note Based off of Table 2.8 Proposed DDR4 Full spec update(79-4B) page 28 +/// +template< fapi2::TargetType T> +fapi2::ReturnCode is_a17_needed(const fapi2::Target& i_target, + bool& o_is_needed); + /// /// @brief Helper function to decode MRS and trace CCS instructions /// @param[in] i_data the completed MRS data to send @@ -287,6 +309,7 @@ fapi2::ReturnCode mrs_engine( const fapi2::Target& i_ta { ccs::instruction_t l_inst_a_side = ccs::mrs_command(i_target, i_rank, i_data.iv_mrs); ccs::instruction_t l_inst_b_side; + bool l_is_a17 = false; // Thou shalt send 2 MRS, one for the a-side and the other inverted for the b-side. // If we're on an odd-rank then we need to mirror @@ -299,7 +322,9 @@ fapi2::ReturnCode mrs_engine( const fapi2::Target& i_ta "Failed mirroring MR%d rank %d on %s", i_data.iv_mrs, i_rank, mss::c_str(i_target) ); - l_inst_b_side = mss::address_invert(l_inst_a_side); + // So we need to see if the A17 bit is enabled. If it is we need to invert it for the CCS parity + FAPI_TRY( is_a17_needed( i_target, l_is_a17) ); + l_inst_b_side = mss::address_invert(i_target, l_inst_a_side, l_is_a17); // Not sure if we can get tricky here and only delay after the b-side MR. The question is whether the delay // is needed/assumed by the register or is purely a DRAM mandated delay. We know we can't go wrong having diff --git a/src/import/chips/p9/procedures/hwp/memory/lib/dimm/eff_dimm.C b/src/import/chips/p9/procedures/hwp/memory/lib/dimm/eff_dimm.C index c3303cc48f9..f3d7e761f2b 100644 --- a/src/import/chips/p9/procedures/hwp/memory/lib/dimm/eff_dimm.C +++ b/src/import/chips/p9/procedures/hwp/memory/lib/dimm/eff_dimm.C @@ -41,6 +41,7 @@ #include #include #include +#include #include #include #include @@ -56,6 +57,18 @@ using fapi2::TARGET_TYPE_DIMM; using fapi2::TARGET_TYPE_MCS; using fapi2::TARGET_TYPE_MCA; using fapi2::TARGET_TYPE_MCBIST; + +/// +/// @brief bit encodings for RC02 +/// From DDR4 Register v1.0 +/// +enum rc02_encode +{ + A17_POS = 7, + A17_ENABLE = 0, + A17_DISABLE = 1 +}; + /// /// @brief bit encodings for Frequencies RC08 /// @note valid frequency values for Nimbus systems @@ -1062,10 +1075,15 @@ fapi2::ReturnCode eff_dimm::dimm_rc02() { // Retrieve MCS attribute data uint8_t l_attrs_dimm_rc02[PORTS_PER_MCS][MAX_DIMM_PER_PORT] = {}; + fapi2::buffer l_temp; + + bool is_a17 = false; + FAPI_TRY( is_a17_needed( iv_dimm, is_a17), "%s Failed to get a17 boolean", mss::c_str(iv_dimm) ); + + l_temp.writeBit(is_a17 ? rc02_encode::A17_ENABLE : rc02_encode::A17_DISABLE); FAPI_TRY( eff_dimm_ddr4_rc02(iv_mcs, &l_attrs_dimm_rc02[0][0]) ); - // Update MCS attribute - l_attrs_dimm_rc02[iv_port_index][iv_dimm_index] = iv_pDecoder->iv_raw_card.iv_rc02; + l_attrs_dimm_rc02[iv_port_index][iv_dimm_index] = l_temp; FAPI_INF("%s: RC02 settting: %d", mss::c_str(iv_dimm), l_attrs_dimm_rc02[iv_port_index][iv_dimm_index] ); FAPI_TRY( FAPI_ATTR_SET(fapi2::ATTR_EFF_DIMM_DDR4_RC02, iv_mcs, l_attrs_dimm_rc02) ); @@ -1359,7 +1377,15 @@ fapi2::ReturnCode eff_dimm::dimm_rc08() // Let's set the other bits l_buffer.writeBit(RC08_PARITY_ENABLE); - l_buffer.writeBit(DA17_QA17_ENABLE); + // Now for A17 bit + { + bool l_is_a17 = false; + + FAPI_TRY( is_a17_needed( iv_dimm, l_is_a17), "%s Failed to get a17 boolean", mss::c_str(iv_dimm) ); + l_buffer.writeBit(l_is_a17 ? DA17_QA17_ENABLE : DA17_QA17_DISABLE); + + FAPI_INF("%s Turning %s DA17", mss::c_str(iv_dimm), (l_is_a17 ? "on" : "off")); + } FAPI_TRY( eff_dimm_ddr4_rc08(iv_mcs, &l_attrs_dimm_rc08[0][0]) ); l_attrs_dimm_rc08[iv_port_index][iv_dimm_index] = l_buffer; @@ -4727,5 +4753,4 @@ fapi2::ReturnCode eff_dimm::phy_seq_refresh() iv_mcs, UINT8_VECTOR_TO_1D_ARRAY(l_phy_seq_ref_enable, PORTS_PER_MCS)); } - }//mss diff --git a/src/import/chips/p9/procedures/hwp/memory/lib/phy/seq.C b/src/import/chips/p9/procedures/hwp/memory/lib/phy/seq.C index 53702bb9ac5..03387e1739b 100644 --- a/src/import/chips/p9/procedures/hwp/memory/lib/phy/seq.C +++ b/src/import/chips/p9/procedures/hwp/memory/lib/phy/seq.C @@ -83,7 +83,6 @@ uint64_t exp_helper( const uint64_t i_value ) return l_first_bit + (uint64_t(1 << l_first_bit) < i_value ? 1 : 0); } - /// /// @brief reset SEQ_TIMING0 /// @param[in] i_target fapi2 target of the port diff --git a/src/import/chips/p9/procedures/hwp/memory/lib/phy/seq.H b/src/import/chips/p9/procedures/hwp/memory/lib/phy/seq.H index 20c275f6223..c62c79108cd 100644 --- a/src/import/chips/p9/procedures/hwp/memory/lib/phy/seq.H +++ b/src/import/chips/p9/procedures/hwp/memory/lib/phy/seq.H @@ -41,6 +41,7 @@ #include #include #include +#include namespace mss { @@ -107,10 +108,13 @@ class seqTraits TODTLON_OFF_CYCLES = MCA_DDRPHY_SEQ_MEM_TIMING_PARAM2_P0_TODTLON_OFF_CYCLES, TODTLON_OFF_CYCLES_LEN = MCA_DDRPHY_SEQ_MEM_TIMING_PARAM2_P0_TODTLON_OFF_CYCLES_LEN, + PAR_A17 = MCA_DDRPHY_SEQ_CONFIG0_P0_PAR_17_MASK, DELAYED_PARITY = MCA_DDRPHY_SEQ_CONFIG0_P0_DELAYED_PAR, TWO_N_MODE = MCA_DDRPHY_SEQ_CONFIG0_P0_TWO_CYCLE_ADDR_EN, - }; + A17_IS_USED = 0b0, + A17_IS_NOT_USED = 0b1, + }; }; namespace seq @@ -400,12 +404,20 @@ fapi_try_exit: /// @return fapi2::ReturnCode FAPI2_RC_SUCCESS if ok /// template< fapi2::TargetType T, typename TT = seqTraits > -inline fapi2::ReturnCode reset_config0( const fapi2::Target& i_target ) +fapi2::ReturnCode reset_config0( const fapi2::Target& i_target ) { fapi2::buffer l_data; - l_data.writeBit(mss::two_n_mode_helper(i_target)); + // Let's figure out if the A17 bit is on/ needed for parity calculations + bool l_a17 = false; + FAPI_TRY( is_a17_needed( i_target, l_a17) ); + + { + const auto l_encoding = l_a17 ? TT::A17_IS_USED : TT::A17_IS_NOT_USED; + l_data.writeBit( l_encoding ); + } + l_data.writeBit(mss::two_n_mode_helper(i_target)); // DDR4 needs delayed partiy TK for DDR5/DDR3 ... l_data.setBit(); FAPI_TRY( write_config0(i_target, l_data), "%s failed to reset seq_config0 register via write", mss::c_str(i_target) ); @@ -424,7 +436,6 @@ fapi_try_exit: template< fapi2::TargetType T, typename TT = seqTraits > fapi2::ReturnCode reset_timing0( const fapi2::Target& i_target ); - /// /// @brief reset SEQ_TIMING1 /// @tparam T fapi2 Target Type - derived diff --git a/src/import/generic/memory/lib/spd/common/rcw_settings.H b/src/import/generic/memory/lib/spd/common/rcw_settings.H index 8f6675c4160..e9fab00e762 100644 --- a/src/import/generic/memory/lib/spd/common/rcw_settings.H +++ b/src/import/generic/memory/lib/spd/common/rcw_settings.H @@ -52,7 +52,6 @@ struct rcw_settings { uint64_t iv_rc00; uint64_t iv_rc01; - uint64_t iv_rc02; uint64_t iv_rc06_07; uint64_t iv_rc09; uint64_t iv_rc0b; @@ -98,7 +97,6 @@ struct rcw_settings /// @brief ctor /// @param[in] i_rc00 setting for register control word (RC00) /// @param[in] i_rc01 setting for register control word (RC01) - /// @param[in] i_rc02 setting for register control word (RC02) /// @param[in] i_rc06_07 setting for register control word (RCO6 & RC07) /// @param[in] i_rc09 setting for register control word (RC09) /// @param[in] i_rc0b setting for register control word (RC0B) @@ -115,7 +113,6 @@ struct rcw_settings /// constexpr rcw_settings( const uint64_t i_rc00, const uint64_t i_rc01, - const uint64_t i_rc02, const uint64_t i_rc06_07, const uint64_t i_rc09, const uint64_t i_rc0b, @@ -131,7 +128,6 @@ struct rcw_settings const uint64_t i_rcax) : iv_rc00(i_rc00), iv_rc01(i_rc01), - iv_rc02(i_rc02), iv_rc06_07(i_rc06_07), iv_rc09(i_rc09), iv_rc0b(i_rc0b), diff --git a/src/import/generic/memory/lib/spd/lrdimm/ddr4/lrdimm_raw_cards.C b/src/import/generic/memory/lib/spd/lrdimm/ddr4/lrdimm_raw_cards.C index a63138843a2..ccc90e02e38 100644 --- a/src/import/generic/memory/lib/spd/lrdimm/ddr4/lrdimm_raw_cards.C +++ b/src/import/generic/memory/lib/spd/lrdimm/ddr4/lrdimm_raw_cards.C @@ -52,7 +52,6 @@ namespace mss // TODO RTC:160116 Fill in valid RCD data for LRDIMM rcw_settings lrdimm_rc_b0( 0x00, // RC00 0x00, // RC01 (C might be the right answer) - 0x00, // RC02 0x1F, // RC06_7 0x00, // RC09 0x0E, // RC0B diff --git a/src/import/generic/memory/lib/spd/rdimm/ddr4/rdimm_raw_cards.C b/src/import/generic/memory/lib/spd/rdimm/ddr4/rdimm_raw_cards.C index 383fb7e0920..bfe95679c42 100644 --- a/src/import/generic/memory/lib/spd/rdimm/ddr4/rdimm_raw_cards.C +++ b/src/import/generic/memory/lib/spd/rdimm/ddr4/rdimm_raw_cards.C @@ -51,7 +51,6 @@ namespace mss /// rcw_settings rdimm_rc_c1( 0x02, // RC00 0x0C, // RC01 - 0x00, // RC02 0x0F, // RC06_07 0x0C, // RC09 0x0E, // RC0B @@ -72,7 +71,6 @@ rcw_settings rdimm_rc_c1( 0x02, // RC00 /// rcw_settings rdimm_rc_c2( 0x02, // RC00 0x0C, // RC01 - 0x00, // RC02 0x0F, // RC06_07 0x0C, // RC09 0x0E, // RC0B @@ -92,7 +90,6 @@ rcw_settings rdimm_rc_c2( 0x02, // RC00 /// rcw_settings rdimm_rc_a1( 0x02, // RC00 0x00, // RC01 - 0x00, // RC02 0x0F, // RC06_07 0x0C, // RC09 0x0E, // RC0B @@ -113,7 +110,6 @@ rcw_settings rdimm_rc_a1( 0x02, // RC00 /// rcw_settings rdimm_rc_b1( 0x02, // RC00 0x00, // RC01 - 0x00, // RC02 0x0F, // RC06_07 0x0C, // RC09 Could be set in eff_config for CKE power DOWN mode 0x0E, // RC0B @@ -134,7 +130,6 @@ rcw_settings rdimm_rc_b1( 0x02, // RC00 /// rcw_settings rdimm_rc_b2( 0x02, // RC00 0x00, // RC01 - 0x00, // RC02 0x0F, // RC06_07 0x0C, // RC09 0x0E, // RC0B @@ -155,7 +150,6 @@ rcw_settings rdimm_rc_b2( 0x02, // RC00 /// rcw_settings rdimm_rc_custom ( 0x02, // RC00 0x00, // RC01 - 0x00, // RC02 0x0F, // RC06_07 0x0C, // RC09 0x0E, // RC0B @@ -176,7 +170,6 @@ rcw_settings rdimm_rc_custom ( 0x02, // RC00 /// rcw_settings rdimm_rc_vbu( 0x02, // RC00 0x00, // RC01 - 0x00, // RC02 0x0F, // RC06_07 0x0C, // RC09 0x0E, // RC0B