Skip to content

Commit

Permalink
Fixes HB compile error
Browse files Browse the repository at this point in the history
C++ STL for certain versions of hostboot requires a
default constructor for std::pair.  This commit
adds in default constructors for the MRS data structures
and updates PDA's compression to use a map.

Change-Id: Iae07cd0e7a21330a420b291f8f4052244277937f
CQ:SW411492
Reviewed-on: http://ralgit01.raleigh.ibm.com/gerrit1/52014
Reviewed-by: Martin Gloff <mgloff@us.ibm.com>
Tested-by: FSP CI Jenkins <fsp-CI-jenkins+hostboot@us.ibm.com>
Tested-by: Jenkins Server <pfd-jenkins+hostboot@us.ibm.com>
Reviewed-by: Louis Stermole <stermole@us.ibm.com>
Tested-by: Hostboot CI <hostboot-ci+hostboot@us.ibm.com>
Reviewed-by: Jennifer A. Stofer <stofer@us.ibm.com>
Reviewed-on: http://ralgit01.raleigh.ibm.com/gerrit1/52030
Tested-by: Jenkins OP Build CI <op-jenkins+hostboot@us.ibm.com>
Tested-by: Jenkins OP HW <op-hw-jenkins+hostboot@us.ibm.com>
Reviewed-by: Christian R. Geddes <crgeddes@us.ibm.com>
  • Loading branch information
esteban012 authored and crgeddes committed Jan 18, 2018
1 parent 198f186 commit 1c2de2d
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 73 deletions.
Expand Up @@ -830,8 +830,11 @@ struct mrs00_data
///
mrs00_data( const fapi2::Target<fapi2::TARGET_TYPE_DIMM>& i_target, fapi2::ReturnCode& o_rc );

// Delete MRS00 default constructor
mrs00_data() = delete;
///
/// @brief Default constructor
/// @note Default constructor is defined to allow for the use of STL data structures
///
mrs00_data() = default;

///
/// @brief Less than operator
Expand Down Expand Up @@ -918,8 +921,11 @@ struct mrs01_data
///
mrs01_data( const fapi2::Target<fapi2::TARGET_TYPE_DIMM>& i_target, fapi2::ReturnCode& o_rc );

// Delete MRS01 default constructor
mrs01_data() = delete;
///
/// @brief Default constructor
/// @note Default constructor is defined to allow for the use of STL data structures
///
mrs01_data() = default;

///
/// @brief Less than operator
Expand Down Expand Up @@ -1017,8 +1023,11 @@ struct mrs02_data
///
mrs02_data( const fapi2::Target<fapi2::TARGET_TYPE_DIMM>& i_target, fapi2::ReturnCode& o_rc );

// Delete MRS02 default constructor
mrs02_data() = delete;
///
/// @brief Default constructor
/// @note Default constructor is defined to allow for the use of STL data structures
///
mrs02_data() = default;

///
/// @brief Less than operator
Expand Down Expand Up @@ -1094,8 +1103,11 @@ struct mrs03_data
///
mrs03_data( const fapi2::Target<fapi2::TARGET_TYPE_DIMM>& i_target, fapi2::ReturnCode& o_rc );

// Delete MRS03 default constructor
mrs03_data() = delete;
///
/// @brief Default constructor
/// @note Default constructor is defined to allow for the use of STL data structures
///
mrs03_data() = default;

///
/// @brief Less than operator
Expand Down Expand Up @@ -1196,8 +1208,11 @@ struct mrs04_data
///
mrs04_data( const fapi2::Target<fapi2::TARGET_TYPE_DIMM>& i_target, fapi2::ReturnCode& o_rc );

// Delete MRS04 default constructor
mrs04_data() = delete;
///
/// @brief Default constructor
/// @note Default constructor is defined to allow for the use of STL data structures
///
mrs04_data() = default;

///
/// @brief Less than operator
Expand Down Expand Up @@ -1319,8 +1334,11 @@ struct mrs05_data
///
mrs05_data( const fapi2::Target<fapi2::TARGET_TYPE_DIMM>& i_target, fapi2::ReturnCode& o_rc );

// Delete MRS05 default constructor
mrs05_data() = delete;
///
/// @brief Default constructor
/// @note Default constructor is defined to allow for the use of STL data structures
///
mrs05_data() = default;

///
/// @brief Less than operator
Expand Down Expand Up @@ -1430,8 +1448,11 @@ struct mrs06_data
///
mrs06_data( const fapi2::Target<fapi2::TARGET_TYPE_DIMM>& i_target, fapi2::ReturnCode& o_rc );

// Delete MRS06 default constructor
mrs06_data() = delete;
///
/// @brief Default constructor
/// @note Default constructor is defined to allow for the use of STL data structures
///
mrs06_data() = default;

///
/// @brief Less than operator
Expand Down
68 changes: 9 additions & 59 deletions src/import/chips/p9/procedures/hwp/memory/lib/dimm/ddr4/pda.H
Expand Up @@ -208,8 +208,7 @@ class commands
public:
// Typdefs to make the code more readable
typedef std::pair<fapi2::Target<fapi2::TARGET_TYPE_DIMM>, uint64_t> rank_target;
typedef std::pair<D, std::vector<uint64_t> > mrs_dram;
typedef std::vector<mrs_dram> mrs_drams_vect;
typedef std::map<D, std::vector<uint64_t> > mrs_drams;

///
/// @brief Base constructor
Expand Down Expand Up @@ -269,56 +268,7 @@ class commands
"%s does not have rank %lu", mss::c_str(i_target), i_rank);

// Does the compression
{
// If the rank/target pair does not exist simply insert a new pair
const rank_target RANK_TARGET = {i_target, i_rank};
// Note: technically this should be const auto to keep the iterator at the found position
// However, HB asumes that the const in front of the auto makes this a const_iterator
// Note: Find value from key prints an error if the key could not be found. We don't want that
// Rolling our own below
// TODO:RTC184689 Create find_iterator_from_value and find_iterator_from_key
auto l_it = std::find_if(iv_commands.begin(),
iv_commands.end(), [&RANK_TARGET](const std::pair<rank_target, mrs_drams_vect>& i_rhs)
{
return RANK_TARGET == i_rhs.first;
});

if( l_it == iv_commands.end() )
{
FAPI_INF("%s rank%lu NEW target rank info DRAM%lu", mss::c_str(i_target), i_rank, i_dram);
mrs_drams_vect l_mrs_dram = {{ i_mrs, { i_dram } }};
iv_commands.push_back( { RANK_TARGET, l_mrs_dram } );
}
// The rank/target exist
else
{
// Does the MRS exist?
auto& l_mrs_vect = l_it->second;
// Note: technically this should be const auto to keep the iterator at the found position
// However, HB asumes that the const in front of the auto makes this a const_iterator
// Note: Find value from key prints an error if the key could not be found. We don't want that
// Rolling our own below
// TODO:RTC184689 Create find_iterator_from_value and find_iterator_from_key
auto l_mrs_it = std::find_if(l_mrs_vect.begin(), l_mrs_vect.end(), [&i_mrs](const mrs_dram & i_rhs)
{
return i_mrs == i_rhs.first;
});

// No, add a new DRAM mapping
if( l_mrs_it == l_mrs_vect.end() )
{
FAPI_INF("%s rank%lu inserting new DRAM + MRS info DRAM%lu", mss::c_str(i_target), i_rank, i_dram);
const mrs_dram MRS_DRAM = { i_mrs, { i_dram } };
l_mrs_vect.push_back( MRS_DRAM );
}
// Yes, add a DRAM onto the vector
else
{
l_mrs_it->second.push_back( i_dram );
FAPI_INF("%s rank%lu pushing back DRAM%lu size %lu", mss::c_str(i_target), i_rank, i_dram, l_mrs_it->second.size());
}
}
}
iv_commands[ {i_target, i_rank}][i_mrs].push_back(i_dram);

fapi_try_exit:
return fapi2::current_err;
Expand All @@ -345,32 +295,32 @@ class commands
/// @brief Returns the command information
/// @return iv_commands
///
inline const typename std::vector<std::pair<rank_target, mrs_drams_vect>>& get() const
inline const typename std::map<rank_target, mrs_drams>& get() const
{
return iv_commands;
}

private:
// The following is a vector of target/DIMM pairs as the key to a vector of
// The following is a map of target/DIMM pairs as the key to a map of
// the MRS command as the key to the DRAM's to toggle. An explanation as to the data structure is included below
// Note: due to HB compile, a vector is used instead of a map

// PDA compression is a little complex, but is organized to allow us to minimize the number of commands run
// Each individual vector is designed to further minimize the number of commands run
// The compressed commands consist of a vector of pairs within a vector of pairs
// The outside vector, maps the DIMM/rank to the MRS command and DRAM's that need to be run
// Each individual map is designed to further minimize the number of commands run
// The compressed commands consist of a map of pairs within a map
// The outside map, maps the DIMM/rank to the MRS command and DRAM's that need to be run
// Basically, it's a list of a specific rank target with all the commands that need to be run
// The rank-specific target information allows us to just issue the enter/exit commands for PDA for each rank once
// The MRS commands to the DRAM are then looped over in the inside loop
// The inside vector has a key of the MRS and a value of a vector of DRAM's to issue the MRS to
// The inside map has a key of the MRS and a value of a map of DRAM's to issue the MRS to
// CCS does not allow the user to toggle the DQ during an MRS command
// The DQ information is stored in separate registers in the PHY
// What this means for issuing the commands is that we have to issue an invocation of CCS for each different MRS command we issue
// We can issue a single MRS command for multiple DRAM's however
// Each invocation of CCS creates a noticable increase in time, as the registers need to be configured, CCS needs to be started, and we need to poll for done
// By only entering into PDA on a DIMM-rank once and by issuing the PDA MRS's to multiple DRAM's at a time, we can save a lot of runtime
// Note: in shmoo, adding the compression reduced runtime from about 13 minutes down to 3 minutes
typename std::vector<std::pair<rank_target, mrs_drams_vect>> iv_commands;
typename std::map<rank_target, mrs_drams> iv_commands;
};

///
Expand Down

0 comments on commit 1c2de2d

Please sign in to comment.