Add read cmd, precharge all cmd, and read cmd CCS instruction and unit tests

Change-Id: I17536a120c9360580894322e5433ffcaeb7d00b0
Reviewed-on: http://ralgit01.raleigh.ibm.com/gerrit1/33723
Tested-by: Jenkins Server <pfd-jenkins+hostboot@us.ibm.com>
Tested-by: Hostboot CI <hostboot-ci+hostboot@us.ibm.com>
Reviewed-by: STEPHEN GLANCY <sglancy@us.ibm.com>
Reviewed-by: JACOB L. HARVEY <jlharvey@us.ibm.com>
Reviewed-by: Brian R. Silver <bsilver@us.ibm.com>
Reviewed-on: http://ralgit01.raleigh.ibm.com/gerrit1/33728
Tested-by: Jenkins OP Build CI <op-jenkins+hostboot@us.ibm.com>
Tested-by: FSP CI Jenkins <fsp-CI-jenkins+hostboot@us.ibm.com>
Reviewed-by: Christian R. Geddes <crgeddes@us.ibm.com>

5 files changed, 384 insertions, 73 deletions

diff --git a/src/import/chips/p9/procedures/hwp/memory/lib/ccs/ccs.H b/src/import/chips/p9/procedures/hwp/memory/lib/ccs/ccs.H
index f59ab1b71..73de8e68f 100644
--- a/src/import/chips/p9/procedures/hwp/memory/lib/ccs/ccs.H
+++ b/src/import/chips/p9/procedures/hwp/memory/lib/ccs/ccs.H
@@ -39,12 +39,15 @@
 #include <fapi2.H>
 
 #include <p9_mc_scom_addresses.H>
+#include <p9_mc_scom_addresses_fld.H>
 
 #include <lib/utils/poll.H>
 #include <lib/utils/swizzle.H>
 #include <lib/mc/port.H>
 #include <lib/shared/mss_const.H>
 
+constexpr uint64_t CKE_HIGH = 0b1111;
+
 // I have a dream that the CCS engine code can be shared among controllers. So, I drive the
 // engine from a set of traits. This might be folly. Allow me to dream. BRS
 
@@ -116,6 +119,8 @@ class ccsTraits<fapi2::TARGET_TYPE_MCBIST>
             // ARR0
             ARR0_DDR_ADDRESS_0_13 = MCBIST_CCS_INST_ARR0_00_DDR_ADDRESS_0_13,
             ARR0_DDR_ADDRESS_0_13_LEN = MCBIST_CCS_INST_ARR0_00_DDR_ADDRESS_0_13_LEN,
+            ARR0_DDR_ADDRESS_0_9 = MCBIST_CCS_INST_ARR0_00_DDR_ADDRESS_0_13, // Useful for rd/wr cmds
+            ARR0_DDR_ADDRESS_0_9_LEN = 10, // CA bits are 9:0, total length of 10
             ARR0_DDR_ADDRESS_10 = 10, // ADR10 is the 10th bit from the left in Nimbus ARR0
             ARR0_DDR_ADDRESS_17 = MCBIST_CCS_INST_ARR0_00_DDR_ADDRESS_17,
             ARR0_DDR_BANK_GROUP_1 = MCBIST_CCS_INST_ARR0_00_DDR_BANK_GROUP_1,
@@ -321,15 +326,15 @@ static void mrs_rcd_helper( fapi2::buffer<uint64_t>& i_arr0 )
     // Generic DDR4 MRS setup (RCD is an MRS)
     //
     // CKE is high Note: P8 set all 4 of these high - not sure if that's correct. BRS
-    i_arr0.insertFromRight<TT::ARR0_DDR_CKE, TT::ARR0_DDR_CKE_LEN>(0b1111);
+    i_arr0.insertFromRight<TT::ARR0_DDR_CKE, TT::ARR0_DDR_CKE_LEN>(CKE_HIGH);
 
     // ACT is high
     i_arr0.setBit<TT::ARR0_DDR_ACTN>();
 
     // RAS, CAS, WE low
-    i_arr0.clearBit<TT::ARR0_DDR_ADDRESS_16>();
-    i_arr0.clearBit<TT::ARR0_DDR_ADDRESS_15>();
-    i_arr0.clearBit<TT::ARR0_DDR_ADDRESS_14>();
+    i_arr0.clearBit<TT::ARR0_DDR_ADDRESS_16>()
+    .template clearBit<TT::ARR0_DDR_ADDRESS_15>()
+    .template clearBit<TT::ARR0_DDR_ADDRESS_14>();
 }
 
 ///
@@ -358,9 +363,9 @@ inline instruction_t<T> rcd_command( const fapi2::Target<fapi2::TARGET_TYPE_DIMM
     //
     // DDR4: Set BG1 to 0 during an MRS.
     // BG0, BA1:BA0 to 0b111 selects RCW (aka MR7).
-    rcd_boilerplate_arr0.clearBit<TT::ARR0_DDR_BANK_GROUP_1>();
-    rcd_boilerplate_arr0.insertFromRight<TT::ARR0_DDR_BANK_0_1, TT::ARR0_DDR_BANK_0_1_LEN>(0b11);
-    rcd_boilerplate_arr0.setBit<TT::ARR0_DDR_BANK_GROUP_0>();
+    rcd_boilerplate_arr0.clearBit<TT::ARR0_DDR_BANK_GROUP_1>()
+    .template insertFromRight<TT::ARR0_DDR_BANK_0_1, TT::ARR0_DDR_BANK_0_1_LEN>(0b11)
+    .template setBit<TT::ARR0_DDR_BANK_GROUP_0>();
 
     // RCD always goes to the 0th rank on the DIMM; either 0 or 4.
     return instruction_t<T>(i_target, (mss::index(i_target) == 0) ? 0 : 4, rcd_boilerplate_arr0, rcd_boilerplate_arr1);
@@ -370,7 +375,7 @@ inline instruction_t<T> rcd_command( const fapi2::Target<fapi2::TARGET_TYPE_DIMM
 /// @brief Create, initialize an MRS CCS command
 /// @tparam T the target type of the chiplet which executes the CCS instruction
 /// @tparam TT the CCS traits of the chiplet which executes the CCS instruction
-/// @param[in] i_target  the DIMM this instruction is headed for
+/// @param[in] i_target the DIMM this instruction is headed for
 /// @param[in] i_rank the rank on this dimm
 /// @param[in] i_mrs the specific MRS
 /// @return the MRS CCS instruction
@@ -379,7 +384,8 @@ inline instruction_t<T> rcd_command( const fapi2::Target<fapi2::TARGET_TYPE_DIMM
 /// in this template definition)
 ///
 template< fapi2::TargetType T, typename TT = ccsTraits<T> >
-inline instruction_t<T> mrs_command( const fapi2::Target<fapi2::TARGET_TYPE_DIMM>& i_target, const uint64_t i_rank,
+inline instruction_t<T> mrs_command( const fapi2::Target<fapi2::TARGET_TYPE_DIMM>& i_target,
+                                     const uint64_t i_rank,
                                      const uint64_t i_mrs )
 {
     fapi2::buffer<uint64_t> rcd_boilerplate_arr0;
@@ -421,7 +427,7 @@ inline instruction_t<T> des_command()
     rcd_boilerplate_arr0.setBit<TT::ARR0_DDR_ACTN>();
 
     // CKE is high Note: P8 set all 4 of these high - not sure if that's correct. BRS
-    rcd_boilerplate_arr0.insertFromRight<TT::ARR0_DDR_CKE, TT::ARR0_DDR_CKE_LEN>(0b1111);
+    rcd_boilerplate_arr0.insertFromRight<TT::ARR0_DDR_CKE, TT::ARR0_DDR_CKE_LEN>(CKE_HIGH);
 
     // ACT is high no-care
     // RAS, CAS, WE no-care
@@ -467,7 +473,7 @@ inline instruction_t<T> initial_cal_command(const uint64_t i_rp)
 /// @brief Setup ZQ Long instruction
 /// @tparam T the target type of the chiplet which executes the CCS instruction
 /// @tparam TT the CCS traits of the chiplet which executes the CCS instruction
-/// @param[in] i_target  the DIMM this instruction is headed for
+/// @param[in] i_target the DIMM this instruction is headed for
 /// @param[in] i_rank the rank on this dimm
 /// @return the MRS CCS instruction
 /// @note THIS IS DDR4 ONLY RIGHT NOW. We can (and possibly should) specialize this
@@ -475,21 +481,22 @@ inline instruction_t<T> initial_cal_command(const uint64_t i_rp)
 /// in this template definition)
 ///
 template< fapi2::TargetType T, typename TT = ccsTraits<T> >
-inline instruction_t<T> zqcl_command( const fapi2::Target<fapi2::TARGET_TYPE_DIMM>& i_target, const uint64_t i_rank )
+inline instruction_t<T> zqcl_command( const fapi2::Target<fapi2::TARGET_TYPE_DIMM>& i_target,
+                                      const uint64_t i_rank )
 {
     fapi2::buffer<uint64_t> l_boilerplate_arr0;
     fapi2::buffer<uint64_t> l_boilerplate_arr1;
 
     // CKE is high Note: P8 set all 4 of these high - not sure if that's correct. BRS
-    l_boilerplate_arr0.insertFromRight<TT::ARR0_DDR_CKE, TT::ARR0_DDR_CKE_LEN>(0b1111);
+    l_boilerplate_arr0.insertFromRight<TT::ARR0_DDR_CKE, TT::ARR0_DDR_CKE_LEN>(CKE_HIGH);
 
     // ACT is high
     l_boilerplate_arr0.setBit<TT::ARR0_DDR_ACTN>();
 
     // RAS/CAS high, WE low
-    l_boilerplate_arr0.setBit<TT::ARR0_DDR_ADDRESS_16>();
-    l_boilerplate_arr0.setBit<TT::ARR0_DDR_ADDRESS_15>();
-    l_boilerplate_arr0.clearBit<TT::ARR0_DDR_ADDRESS_14>();
+    l_boilerplate_arr0.setBit<TT::ARR0_DDR_ADDRESS_16>()
+    .template setBit<TT::ARR0_DDR_ADDRESS_15>()
+    .template clearBit<TT::ARR0_DDR_ADDRESS_14>();
 
     // ADDR10/AP is high
     l_boilerplate_arr0.setBit<TT::ARR0_DDR_ADDRESS_10>();
@@ -497,6 +504,163 @@ inline instruction_t<T> zqcl_command( const fapi2::Target<fapi2::TARGET_TYPE_DIM
     return instruction_t<T>(i_target, i_rank, l_boilerplate_arr0, l_boilerplate_arr1);
 }
 
+///
+/// @brief Setup read command helper function
+/// @tparam T the target type of the chiplet which executes the CCS instruction
+/// @tparam TT the CCS traits of the chiplet which executes the CCS instruction
+/// @param[in] i_target the DIMM this instruction is headed for
+/// @param[in] i_rank the rank on this dimm
+/// @param[in] i_bank_addr bank address bits [BG0:BG1] = [62:63] (right aligned)
+/// @param[in] i_bank_group_addr bank group address bits [BA0:BA1] = [62:63] (right aligned)
+/// @param[in] i_column_addr column address bits [A0:A9] = [54:63] (right aligned)
+/// @return the read command CCS instruction
+/// @note THIS IS DDR4 ONLY RIGHT NOW. We can (and possibly should) specialize this
+/// for the controller (Nimbus v Centaur) and then correct for DRAM generation (not included
+/// in this template definition)
+///
+template< fapi2::TargetType T, typename TT = ccsTraits<T> >
+static fapi2::buffer<uint64_t> read_cmd_boilerplate( const fapi2::Target<fapi2::TARGET_TYPE_DIMM>& i_target,
+        const uint64_t i_rank,
+        const fapi2::buffer<uint64_t>& i_bank_addr = 0,
+        const fapi2::buffer<uint64_t>& i_bank_group_addr = 0,
+        const fapi2::buffer<uint64_t>& i_column_addr = 0)
+{
+    // TODO - RTC 166175 Encapsulate command truth table in a subclass for ccs.H
+    fapi2::buffer<uint64_t> l_boilerplate_arr0;
+
+    // CKE is high Note: P8 set all 4 of these high - not sure if that's correct. AAM
+    l_boilerplate_arr0.insertFromRight<TT::ARR0_DDR_CKE,
+                                       TT::ARR0_DDR_CKE_LEN>(CKE_HIGH);
+
+    // ACT is high
+    l_boilerplate_arr0.setBit<TT::ARR0_DDR_ACTN>();
+
+    // RAS high, CAS low, WE high
+    l_boilerplate_arr0.setBit<TT::ARR0_DDR_ADDRESS_16>()
+    .template clearBit<TT::ARR0_DDR_ADDRESS_15>()
+    .template setBit<TT::ARR0_DDR_ADDRESS_14>();
+
+    l_boilerplate_arr0.insertFromRight<TT::ARR0_DDR_BANK_0_1,
+                                       TT::ARR0_DDR_BANK_0_1_LEN>(i_bank_addr);
+
+    // Bank Group takes a little effort - the bits aren't contiguous
+    constexpr uint64_t BG0_BIT = 62;
+    constexpr uint64_t BG1_BIT = 63;
+
+    l_boilerplate_arr0.writeBit<TT::ARR0_DDR_BANK_GROUP_0>(i_bank_group_addr.getBit<BG0_BIT>())
+    .template writeBit<TT::ARR0_DDR_BANK_GROUP_1>(i_bank_group_addr.getBit<BG1_BIT>());
+
+    // CA is A[0:9]
+    l_boilerplate_arr0.insertFromRight<TT::ARR0_DDR_ADDRESS_0_9,
+                                       TT::ARR0_DDR_ADDRESS_0_9_LEN>(i_column_addr);
+
+    return l_boilerplate_arr0;
+}
+
+///
+/// @brief Setup read command (Fixed BL8 or BC4) instruction
+/// @tparam T the target type of the chiplet which executes the CCS instruction
+/// @tparam TT the CCS traits of the chiplet which executes the CCS instruction
+/// @param[in] i_target the DIMM this instruction is headed for
+/// @param[in] i_rank the rank on this dimm
+/// @param[in] i_bank_addr bank address bits [BG0:BG1] = [62:63] (right aligned)
+/// @param[in] i_bank_group_addr bank group address bits [BA0:BA1] = [62:63] (right aligned)
+/// @param[in] i_column_addr column address bits [A0:A9] = [54:63] (right aligned)
+/// @return the read command CCS instruction
+/// @note THIS IS DDR4 ONLY RIGHT NOW. We can (and possibly should) specialize this
+/// for the controller (Nimbus v Centaur) and then correct for DRAM generation (not included
+/// in this template definition)
+///
+template< fapi2::TargetType T, typename TT = ccsTraits<T> >
+inline instruction_t<T> rd_command( const fapi2::Target<fapi2::TARGET_TYPE_DIMM>& i_target,
+                                    const uint64_t i_rank,
+                                    const fapi2::buffer<uint64_t>& i_bank_addr = 0,
+                                    const fapi2::buffer<uint64_t>& i_bank_group_addr = 0,
+                                    const fapi2::buffer<uint64_t>& i_column_addr = 0)
+{
+    fapi2::buffer<uint64_t> l_boilerplate_arr0;
+    fapi2::buffer<uint64_t> l_boilerplate_arr1;
+
+    l_boilerplate_arr0 = read_cmd_boilerplate<fapi2::TARGET_TYPE_MCBIST>(i_target, i_rank, i_bank_addr,
+                         i_bank_group_addr, i_column_addr);
+
+    // Setup ADDR10/AP based on read type
+    l_boilerplate_arr0.clearBit<TT::ARR0_DDR_ADDRESS_10>();
+
+    return instruction_t<T>(i_target, i_rank, l_boilerplate_arr0, l_boilerplate_arr1);
+}
+
+///
+/// @brief Setup read w/auto precharge command (Fixed BL8 or BC4) instruction
+/// @tparam T the target type of the chiplet which executes the CCS instruction
+/// @tparam TT the CCS traits of the chiplet which executes the CCS instruction
+/// @param[in] i_target the DIMM this instruction is headed for
+/// @param[in] i_rank the rank on this dimm
+/// @param[in] i_bank_addr bank address bits [BG0:BG1] = [62:63] (right aligned)
+/// @param[in] i_bank_group_addr bank group address bits [BA0:BA1] = [62:63] (right aligned)
+/// @param[in] i_column_addr column address bits [A0:A9] = [54:63] (right aligned)
+/// @return the read command CCS instruction
+/// @note THIS IS DDR4 ONLY RIGHT NOW. We can (and possibly should) specialize this
+/// for the controller (Nimbus v Centaur) and then correct for DRAM generation (not included
+/// in this template definition)
+///
+template< fapi2::TargetType T, typename TT = ccsTraits<T> >
+inline instruction_t<T> rda_command( const fapi2::Target<fapi2::TARGET_TYPE_DIMM>& i_target,
+                                     const uint64_t i_rank,
+                                     const fapi2::buffer<uint64_t>& i_bank_addr = 0,
+                                     const fapi2::buffer<uint64_t>& i_bank_group_addr = 0,
+                                     const fapi2::buffer<uint64_t>& i_column_addr = 0)
+{
+    fapi2::buffer<uint64_t> l_boilerplate_arr0;
+    fapi2::buffer<uint64_t> l_boilerplate_arr1;
+
+    l_boilerplate_arr0 = read_cmd_boilerplate<fapi2::TARGET_TYPE_MCBIST>(i_target, i_rank, i_bank_addr,
+                         i_bank_group_addr, i_column_addr);
+
+    // Setup ADDR10/AP based on read type
+    l_boilerplate_arr0.setBit<TT::ARR0_DDR_ADDRESS_10>();
+
+    return instruction_t<T>(i_target, i_rank, l_boilerplate_arr0, l_boilerplate_arr1);
+}
+
+///
+/// @brief Setup precharge all banks command instruction
+/// @tparam T the target type of the chiplet which executes the CCS instruction
+/// @tparam TT the CCS traits of the chiplet which executes the CCS instruction
+/// @param[in] i_target the DIMM this instruction is headed for
+/// @param[in] i_rank the rank on this dimm
+/// @return the precharge all banks command CCS instruction
+/// @note THIS IS DDR4 ONLY RIGHT NOW. We can (and possibly should) specialize this
+/// for the controller (Nimbus v Centaur) and then correct for DRAM generation (not included
+/// in this template definition)
+///
+template< fapi2::TargetType T, typename TT = ccsTraits<T> >
+inline instruction_t<T> precharge_all_command( const fapi2::Target<fapi2::TARGET_TYPE_DIMM>& i_target,
+        const uint64_t i_rank )
+{
+    fapi2::buffer<uint64_t> l_boilerplate_arr0;
+    fapi2::buffer<uint64_t> l_boilerplate_arr1;
+
+    // CKE is high Note: P8 set all 4 of these high - not sure if that's correct. AAM
+    l_boilerplate_arr0.insertFromRight<TT::ARR0_DDR_CKE, TT::ARR0_DDR_CKE_LEN>(CKE_HIGH);
+
+    // ACT is high
+    l_boilerplate_arr0.setBit<TT::ARR0_DDR_ACTN>();
+
+    // RAS low, CAS high, WE low
+    l_boilerplate_arr0.clearBit<TT::ARR0_DDR_ADDRESS_16>()
+    .template setBit<TT::ARR0_DDR_ADDRESS_15>()
+    .template clearBit<TT::ARR0_DDR_ADDRESS_14>();
+
+    // Setup ADDR10/AP high
+    l_boilerplate_arr0.setBit<TT::ARR0_DDR_ADDRESS_10>();
+
+    // From DDR4 Spec table 17:
+    // All other bits from the command truth table or 'V', for valid (1 or 0)
+
+    return instruction_t<T>(i_target, i_rank, l_boilerplate_arr0, l_boilerplate_arr1);
+}
+
 //
 // These functions are a little sugar to keep callers from doing the traits-dance to get the
 // appropriate bit field
@@ -554,8 +718,8 @@ inline void stop_on_err( const fapi2::Target<T>&, fapi2::buffer<uint64_t>& io_bu
 template< fapi2::TargetType T, typename TT = ccsTraits<T> >
 inline void disable_ecc( const fapi2::Target<T>&, fapi2::buffer<uint64_t>& io_buffer)
 {
-    io_buffer.setBit<TT::DISABLE_ECC_ARRAY_CHK>();
-    io_buffer.setBit<TT::DISABLE_ECC_ARRAY_CORRECTION>();
+    io_buffer.setBit<TT::DISABLE_ECC_ARRAY_CHK>()
+    .template setBit<TT::DISABLE_ECC_ARRAY_CORRECTION>();
 }
 
 ///
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 d2620e084..6e4d141ba 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
@@ -999,23 +999,17 @@ fapi2::ReturnCode mrs_load( const fapi2::Target<fapi2::TARGET_TYPE_DIMM>& i_targ
                             std::vector< ccs::instruction_t<fapi2::TARGET_TYPE_MCBIST> >& io_inst);
 
 ///
-/// @brief Makes CCS instruction to set MPR Mode
+/// @brief  Set MPR Mode
 /// @param[in] i_target a DIMM target
 /// @param[in] i_mode setting for MPR mode
-/// @param[in,out] io_inst a vector of CCS instructions we should add to
+/// @param[in,out] io_data data we are modifying MPR mode to
 /// @return FAPI2_RC_SUCCESS if and only if ok
 ///
-template< fapi2::TargetType T >
-fapi2::ReturnCode set_dram_mpr_mode(const fapi2::Target<fapi2::TARGET_TYPE_DIMM>& i_target,
-                                    const uint64_t i_rank,
-                                    const uint8_t i_mode,
-                                    std::vector< ccs::instruction_t<T> >& io_inst )
+inline fapi2::ReturnCode set_dram_mpr_mode(const fapi2::Target<fapi2::TARGET_TYPE_DIMM>& i_target,
+        const uint8_t i_mode,
+        mrs03_data& io_data)
 {
     constexpr uint64_t MAX_MPR_MODE = 0b1;
-    fapi2::ReturnCode l_rc;
-
-    mrs03_data l_data(i_target, l_rc);
-    FAPI_TRY( l_rc, "Unable to instantiate mrs03_data for %s", i_target );
 
     if(i_mode > MAX_MPR_MODE)
     {
@@ -1023,78 +1017,216 @@ fapi2::ReturnCode set_dram_mpr_mode(const fapi2::Target<fapi2::TARGET_TYPE_DIMM>
         return fapi2::FAPI2_RC_INVALID_PARAMETER;
     }
 
-    l_data.iv_mpr_mode = i_mode;
-    FAPI_TRY( mrs_engine(i_target, l_data, i_rank, mss::tmrd(), io_inst),
-              "Failed to send MRS03 on %s, rank: %d, delay (in cycles): %d",
-              i_target, i_rank, mss::tmrd());
+    // Update field if input check passes
+    io_data.iv_mpr_mode = i_mode;
 
-fapi_try_exit:
-    return fapi2::current_err;
+    return fapi2::FAPI2_RC_SUCCESS;
 }
 
 ///
-/// @brief Makes CCS instruction to set MPR Read
+/// @brief Set MPR Read
 /// @param[in] i_target a DIMM target
 /// @param[in] i_format setting for MPR read format
-/// @param[in,out] io_inst a vector of CCS instructions we should add to
+/// @param[in,out] io_data data we are modifying MPR mode to
 /// @return FAPI2_RC_SUCCESS if and only if ok
 ///
-template< fapi2::TargetType T >
-fapi2::ReturnCode set_dram_mpr_read(const fapi2::Target<fapi2::TARGET_TYPE_DIMM>& i_target,
-                                    const uint64_t i_rank,
-                                    const uint8_t i_format,
-                                    std::vector< ccs::instruction_t<T> >& io_inst )
+inline fapi2::ReturnCode set_dram_mpr_rd_format(const fapi2::Target<fapi2::TARGET_TYPE_DIMM>& i_target,
+        const uint8_t i_format,
+        mrs03_data& io_data)
 {
     constexpr uint64_t MAX_READ_FORMAT = 0b10;
 
-    fapi2::ReturnCode l_rc;
-    mrs03_data l_data(i_target, l_rc);
-    FAPI_TRY( l_rc, "Unable to instantiate mrs03_data for %s", i_target );
-
     if(i_format > MAX_READ_FORMAT)
     {
         FAPI_ERR("Invalid MPR Read Format recieved: %d. Max encoding allowed: %d.", i_format, MAX_READ_FORMAT);
         return fapi2::FAPI2_RC_INVALID_PARAMETER;
     }
 
-    l_data.iv_read_format = i_format;
-    FAPI_TRY( mrs_engine(i_target, l_data, i_rank, mss::tmrd(), io_inst),
-              "Failed to send MRS03 on %s, rank: %d, delay (in cycles): %d",
-              i_target, i_rank, mss::tmrd());
+    // Update field if input check passes
+    io_data.iv_read_format = i_format;
 
-fapi_try_exit:
-    return fapi2::current_err;
+    return fapi2::FAPI2_RC_SUCCESS;
 }
 
 ///
-/// @brief Makes CCS instruction to set MPR Read
+/// @brief Set MPR page
 /// @param[in] i_target a DIMM target
 /// @param[in] i_page setting for MPR read format
-/// @param[in,out] io_inst a vector of CCS instructions we should add to
+/// @param[in,out] io_data data we are modifying MPR mode to
 /// @return FAPI2_RC_SUCCESS if and only if ok
 ///
-template< fapi2::TargetType T >
-fapi2::ReturnCode set_dram_mpr_page(const fapi2::Target<fapi2::TARGET_TYPE_DIMM>& i_target,
-                                    const uint64_t i_rank,
-                                    const uint8_t i_page,
-                                    std::vector< ccs::instruction_t<T> >& io_inst )
+inline fapi2::ReturnCode set_dram_mpr_page(const fapi2::Target<fapi2::TARGET_TYPE_DIMM>& i_target,
+        const uint8_t i_page,
+        mrs03_data& io_data)
 {
     constexpr uint64_t MAX_PAGE = 0b11;
 
-    fapi2::ReturnCode l_rc;
-    mrs03_data l_data(i_target, l_rc);
-    FAPI_TRY( l_rc, "Unable to instantiate mrs03_data for %s", i_target );
-
     if(i_page > MAX_PAGE)
     {
         FAPI_ERR("Invalid MPR Page recieved: %d. Max encoding allowed: %d.", i_page, MAX_PAGE);
         return fapi2::FAPI2_RC_INVALID_PARAMETER;
     }
 
-    l_data.iv_mpr_page = i_page;
-    FAPI_TRY( mrs_engine(i_target, l_data, i_rank, mss::tmrd(), io_inst),
+    // Update field if input check passes
+    io_data.iv_mpr_page = i_page;
+
+    return fapi2::FAPI2_RC_SUCCESS;
+}
+
+///
+/// @brief Makes CCS instruction to set MPR Mode
+/// @tparam T TargetType of the CCS instruction
+/// @param[in] i_target a DIMM target
+/// @param[in] i_mode setting for MPR mode
+/// @param[in] i_rank DIMM rank
+/// @param[in,out] io_inst a vector of CCS instructions we should add to
+/// @return FAPI2_RC_SUCCESS if and only if ok
+///
+template< fapi2::TargetType T >
+fapi2::ReturnCode mpr_load(const fapi2::Target<fapi2::TARGET_TYPE_DIMM>& i_target,
+                           const uint8_t i_mode,
+                           const uint64_t i_rank,
+                           std::vector< ccs::instruction_t<T> >& io_inst )
+{
+    // From DDR4 spec section 4.10.3 MPR Reads:
+    // tMRD and tMOD must be satisfied after enabling/disabling MPR mode
+    const uint64_t l_delay = std::max( mss::tmod(i_target), mss::tmrd() );
+
+    mrs03_data l_data(i_target, fapi2::current_err);
+    FAPI_TRY(fapi2::current_err, "%s. Failed to initialize mrs03_data for mpr_load", mss::c_str(i_target) );
+
+    FAPI_TRY( set_dram_mpr_mode(i_target, i_mode, l_data),
+              "%s. Failed set_dram_mpr_mode() with a setting of %d",
+              mss::c_str(i_target), i_mode);
+
+    // Make MRS CCS inst
+    FAPI_TRY( mrs_engine(i_target, l_data, i_rank, l_delay, io_inst),
+              "Failed to send MRS03 on %s, rank: %d, delay (in cycles): %d",
+              mss::c_str(i_target), i_rank, l_delay);
+
+fapi_try_exit:
+    return fapi2::current_err;
+}
+
+///
+/// @brief Makes CCS instruction to set MPR Mode
+/// @tparam T TargetType of the CCS instruction
+/// @param[in] i_target a DIMM target
+/// @param[in] i_mode setting for MPR mode
+/// @param[in] i_rd_format MPR read format
+/// @param[in] i_rank DIMM rank
+/// @param[in,out] io_inst a vector of CCS instructions we should add to
+/// @return FAPI2_RC_SUCCESS if and only if ok
+///
+template< fapi2::TargetType T >
+fapi2::ReturnCode mpr_load(const fapi2::Target<fapi2::TARGET_TYPE_DIMM>& i_target,
+                           const uint8_t i_mode,
+                           const uint8_t i_rd_format,
+                           const uint64_t i_rank,
+                           std::vector< ccs::instruction_t<T> >& io_inst )
+{
+    // From DDR4 spec section 4.10.3 MPR Reads:
+    // tMRD and tMOD must be satisfied after enabling/disabling MPR mode
+    const uint64_t l_delay = std::max( mss::tmod(i_target), mss::tmrd() );
+
+    mrs03_data l_data(i_target, fapi2::current_err);
+    FAPI_TRY(fapi2::current_err, "%s. Failed to initialize mrs03_data for mpr_load", mss::c_str(i_target) );
+
+    FAPI_TRY( set_dram_mpr_mode(i_target, i_mode, l_data),
+              "%s. Failed set_dram_mpr_mode() with a setting of %d",
+              mss::c_str(i_target), i_mode);
+
+    FAPI_TRY( set_dram_mpr_rd_format(i_target, i_rd_format, l_data),
+              "%s. Failed set_dram_mpr_rd_format() with a setting of %d",
+              mss::c_str(i_target), i_rd_format);
+
+    // Make MRS CCS inst
+    FAPI_TRY( mrs_engine(i_target, l_data, i_rank, l_delay, io_inst),
               "Failed to send MRS03 on %s, rank: %d, delay (in cycles): %d",
-              i_target, i_rank, mss::tmrd());
+              mss::c_str(i_target), i_rank, l_delay);
+
+fapi_try_exit:
+    return fapi2::current_err;
+}
+
+///
+/// @brief Makes CCS instruction for an MPR read
+/// @tparam T TargetType of the CCS instruction
+/// @param[in] i_target a DIMM target
+/// @param[in] i_mode MPR location
+/// @param[in] i_rank DIMM rank
+/// @param[in,out] io_inst a vector of CCS instructions we should add to
+/// @return FAPI2_RC_SUCCESS if and only if ok
+///
+template< fapi2::TargetType T >
+fapi2::ReturnCode mpr_read( const fapi2::Target<fapi2::TARGET_TYPE_DIMM>& i_target,
+                            const uint64_t i_mpr_loc,
+                            const uint64_t i_rank,
+                            std::vector< ccs::instruction_t<T> >& io_inst )
+{
+    // Right now we only have support for RD and RDA
+    // Unclear if we want the API select the type of read command right now
+    // Note the auto precharge is ignored with MPR mode on so we just do a read cmd
+    ccs::instruction_t<T> l_inst = ccs::rd_command<T> (i_target, i_rank, i_mpr_loc);
+
+    // In MPR Mode:
+    // Reads (back-to-back) from Page 0 may use tCCD_S or tCCD_L timing between read commands
+    // Reads (back-to-back) from Pages 1, 2, or 3 may not use tCCD_S timing between read commands
+    // tCCD_L must be used for timing between read commands
+    uint8_t l_delay = 0;
+
+    if( i_mpr_loc == 0)
+    {
+        // note we are truncating a uint64 to a uint8 but since
+        // the value of tccd_s is always 4....we should be okay
+        l_delay = mss::tccd_s();
+    }
+    else
+    {
+        FAPI_TRY(eff_dram_tccd_l(i_target, l_delay), "Failed to invoke accessor for tCCD_L");
+    }
+
+    // Input type needs to be greater than IDLES_LEN, hence the cast
+    l_inst.arr1.template insertFromRight<MCBIST_CCS_INST_ARR1_00_IDLES,
+                         MCBIST_CCS_INST_ARR1_00_IDLES_LEN>(static_cast<uint64_t>(l_delay));
+
+    FAPI_INF("MPR Read CCS inst 0x%016llx:0x%016llx %s:rank %d, MPR location:%d, delay (in cycles) %d",
+             uint64_t(l_inst.arr0), uint64_t(l_inst.arr1), mss::c_str(i_target), i_rank, i_mpr_loc, l_delay);
+
+    io_inst.push_back(l_inst);
+
+fapi_try_exit:
+    return fapi2::current_err;
+}
+
+///
+/// @brief Makes CCS instruction to set precharge all command
+/// @tparam T TargetType of the CCS instruction
+/// @param[in] i_target a DIMM target
+/// @param[in] i_rank DIMM rank
+/// @param[in,out] io_inst a vector of CCS instructions we should add to
+/// @return FAPI2_RC_SUCCESS if and only if ok
+///
+template< fapi2::TargetType T >
+fapi2::ReturnCode precharge_all( const fapi2::Target<fapi2::TARGET_TYPE_DIMM>& i_target,
+                                 const uint64_t i_rank,
+                                 std::vector< ccs::instruction_t<T> >& io_inst )
+{
+    ccs::instruction_t<T> l_inst = ccs::precharge_all_command<T> (i_target, i_rank);
+
+    // From the DDR4 Spec tRP is the precharge command period
+    uint8_t l_delay = 0;
+    FAPI_TRY( eff_dram_trp(i_target, l_delay) );
+
+    // Input type needs to be greater than IDLES_LEN, hence the cast
+    l_inst.arr1.template insertFromRight<MCBIST_CCS_INST_ARR1_00_IDLES,
+                         MCBIST_CCS_INST_ARR1_00_IDLES_LEN>( static_cast<uint64_t>(l_delay));
+
+    FAPI_INF("precharge_all CCS inst 0x%016llx:0x%016llx %s:rank %d, delay (in cycles) %d",
+             uint64_t(l_inst.arr0), uint64_t(l_inst.arr1), mss::c_str(i_target), i_rank, l_delay);
+
+    // Add to CCS program
+    io_inst.push_back(l_inst);
 
 fapi_try_exit:
     return fapi2::current_err;
diff --git a/src/import/chips/p9/procedures/hwp/memory/lib/eff_config/timing.H b/src/import/chips/p9/procedures/hwp/memory/lib/eff_config/timing.H
index d009dc4fe..aa5320e3b 100644
--- a/src/import/chips/p9/procedures/hwp/memory/lib/eff_config/timing.H
+++ b/src/import/chips/p9/procedures/hwp/memory/lib/eff_config/timing.H
@@ -1340,5 +1340,15 @@ inline uint64_t tvrefdqx( const fapi2::Target<T>& i_target )
     return ns_to_cycles(i_target, tVREFDQX);
 }
 
+///
+/// @brief CAS_n to CAS_n command delay for different bank group
+/// @return constexpr value of 4 clocks
+///
+constexpr uint64_t tccd_s()
+{
+    // Per DDR4 Full spec update (79-4A) - timing requirements
+    return 4;
+}
+
 } // mss
 #endif
diff --git a/src/import/chips/p9/procedures/hwp/memory/lib/utils/swizzle.H b/src/import/chips/p9/procedures/hwp/memory/lib/utils/swizzle.H
index 126b20c27..6d2633531 100644
--- a/src/import/chips/p9/procedures/hwp/memory/lib/utils/swizzle.H
+++ b/src/import/chips/p9/procedures/hwp/memory/lib/utils/swizzle.H
@@ -106,23 +106,28 @@ static inline void reverse( T& io_buffer )
 /// @tparam DS the start bit in the destination buffer - swizzle will count up from here
 /// @tparam L how many bits to swizzle
 /// @tparam SS the start bit in the source buffer - swizzle will count down from here
+/// @tparam SB source buffer type
+/// @tparam DB destination buffer type
 /// @param[in] i_source  source buffer - these bits will be decremented
-/// @param[in] o_destination destination buffer - these bits will be incremented
+/// @param[out] o_destination destination buffer - these bits will be incremented
 /// @return reference to the destination
 ///
 template<uint64_t DS, uint64_t L, uint64_t SS, typename SB, typename DB>
-inline fapi2::buffer<DB>& swizzle( fapi2::buffer<SB> i_source, fapi2::buffer<DB>& o_destination )
+inline fapi2::buffer<DB>& swizzle( const fapi2::buffer<SB>& i_source, fapi2::buffer<DB>& o_destination )
 {
     // Reverse the destination, and then mangle the start bits to get things to line up
     fapi2::buffer<SB> l_tmp(i_source);
-    reverse(i_source);
+    reverse(l_tmp);
+
+    o_destination.template insert < DS, L, (sizeof(SB) * 8) - (SS + 1) > (SB(l_tmp));
 
 #ifdef SWIZZLE_TRACE
-    FAPI_DBG("swizzle o: 0x%016lx s: 0x%016lx d: 0x%016lx ds: %d l: %d ss: %d",
-             l_tmp, i_source, o_destination, DS, L, (sizeof(SB) * 8) - (SS + 1));
+    // s: source, r: reverse, d: destination, ds: distination start, l: length, ss: source start
+    FAPI_DBG("swizzle s: 0x%016llx, r: 0x%016llx, d: 0x%016llx, ds: %d, l: %d, ss: %d",
+             i_source, l_tmp, o_destination, DS, L, (sizeof(SB) * 8) - (SS + 1));
 #endif
 
-    return o_destination.template insert < DS, L, (sizeof(SB) * 8) - (SS + 1) > (SB(i_source));
+    return o_destination;
 }
 
 }
diff --git a/src/import/chips/p9/procedures/xml/attribute_info/memory_mcs_attributes.xml b/src/import/chips/p9/procedures/xml/attribute_info/memory_mcs_attributes.xml
index 62be56c0e..13840e996 100644
--- a/src/import/chips/p9/procedures/xml/attribute_info/memory_mcs_attributes.xml
+++ b/src/import/chips/p9/procedures/xml/attribute_info/memory_mcs_attributes.xml
@@ -1570,7 +1570,7 @@
         </description>
         <initToZero></initToZero>
         <valueType>uint8</valueType>
-        <enum>SERIAL = 0, PARALLEL = 1, STAGGERED = 2, RESERVED_TEMP= 3</enum>
+        <enum>SERIAL = 0, PARALLEL = 1, STAGGERED = 2</enum>
         <writeable/>
         <array>2</array>
         <mssAccessorName>eff_mpr_rd_format</mssAccessorName>