Cache the JEDEC ID value in SPDAccessor and improve SPD detector logging. Caching the JEDEC ID speeds up DRAM detection significantly.

ResourceManager.cpp

@@ -1197,8 +1197,7 @@ void ResourceManager::DetectDevicesCoroutine()
 
             for(unsigned int i2c_detector_idx = 0; i2c_detector_idx < i2c_dimm_device_detectors.size() && detection_is_required.load(); i2c_detector_idx++)
             {
-                if(i2c_dimm_device_detectors[i2c_detector_idx].dimm_type == dimm_type &&
+                if((i2c_dimm_device_detectors[i2c_detector_idx].dimm_type == dimm_type) && is_jedec_in_slots(slots, i2c_dimm_device_detectors[i2c_detector_idx].jedec_id))
-                   is_jedec_in_slots(slots, i2c_dimm_device_detectors[i2c_detector_idx].jedec_id))
                 {
                     detection_string = i2c_dimm_device_detectors[i2c_detector_idx].name.c_str();
 

SPDAccessor/SPDDetector.cpp

@@ -37,8 +37,6 @@ void SPDDetector::detect_memory_type()
 {
     SPDAccessor *accessor;
 
-    LOG_DEBUG("[SPDDetector] Probing DRAM on bus %d address 0x%02x", bus->bus_id, address);
-
     /*---------------------------------------------------------*\
     | On Linux, attempt to use the ee1004 or spd5118 drivers to |
     | access SPD on DDR4 and DDR5, respectively                 |
@@ -46,10 +44,12 @@ void SPDDetector::detect_memory_type()
 #ifdef __linux__
     if(EE1004Accessor::isAvailable(bus, address))
     {
+        LOG_DEBUG("[SPDDetector] Probing DRAM using EE1004 Accessor on bus %d, address 0x%02x", bus->bus_id, address);
         accessor = new EE1004Accessor(bus, address);
     }
     else if(SPD5118Accessor::isAvailable(bus, address))
     {
+        LOG_DEBUG("[SPDDetector] Probing DRAM using SPD5118 Accessor on bus %d, address 0x%02x", bus->bus_id, address);
         accessor = new SPD5118Accessor(bus, address);
     }
     else
@@ -65,6 +65,7 @@ void SPDDetector::detect_memory_type()
      || mem_type == SPD_LPDDR4X_SDRAM)
     &&  DDR4DirectAccessor::isAvailable(bus, address))
     {
+        LOG_DEBUG("[SPDDetector] Probing DRAM using DDR4 Direct Accessor on bus %d, address 0x%02x", bus->bus_id, address);
         accessor = new DDR4DirectAccessor(bus, address);
     }
     else if((mem_type == SPD_RESERVED
@@ -72,6 +73,7 @@ void SPDDetector::detect_memory_type()
           || mem_type == SPD_LPDDR5_SDRAM)
          &&  DDR5DirectAccessor::isAvailable(bus, address))
     {
+        LOG_DEBUG("[SPDDetector] Probing DRAM using DDR5 Direct Accessor on bus %d, address 0x%02x", bus->bus_id, address);
         accessor = new DDR5DirectAccessor(bus, address);
     }
     /*---------------------------------------------------------*\
@@ -80,7 +82,7 @@ void SPDDetector::detect_memory_type()
     \*---------------------------------------------------------*/
     else if(mem_type == SPD_RESERVED)
     {
-        LOG_TRACE("[SPDDetector] Probing memory type older than DDR4");
+        LOG_DEBUG("[SPDDetector] Probing DRAM older than DDR4 on bus %d, address 0x%02x", bus->bus_id, address);
 
         int value = bus->i2c_smbus_read_byte_data(address, 0x02);
 
@@ -110,6 +112,7 @@ void SPDDetector::detect_memory_type()
     \*---------------------------------------------------------*/
     else
     {
+        LOG_DEBUG("[SPDDetector] Memory type could not be determined for bus %d, address 0x%02x", bus->bus_id, address);
         valid = false;
         return;
     }

SPDAccessor/SPDWrapper.cpp

@@ -17,6 +17,20 @@ SPDWrapper::SPDWrapper(const SPDWrapper &wrapper)
     }
     this->address = wrapper.address;
     this->mem_type = wrapper.mem_type;
+
+    /*-----------------------------------------------------*\
+    | Read the JEDEC ID and cache its value                 |
+    | This saves a significant amount of time over reading  |
+    | the JEDEC ID each time it is accessed                 |
+    \*-----------------------------------------------------*/
+    if(accessor == nullptr)
+    {
+        jedec_id_val = 0x0000;
+    }
+    else
+    {
+        jedec_id_val = accessor->jedec_id();
+    }
 }
 
 SPDWrapper::SPDWrapper(const SPDDetector &detector)
@@ -24,8 +38,24 @@ SPDWrapper::SPDWrapper(const SPDDetector &detector)
     this->address = detector.spd_address();
     this->mem_type = detector.memory_type();
 
-    // Allocate a new accessor
+    /*-----------------------------------------------------*\
+    | Allocate a new accessor                               |
+    \*-----------------------------------------------------*/
     this->accessor = SPDAccessor::for_memory_type(this->mem_type, detector.smbus(), this->address);
+
+    /*-----------------------------------------------------*\
+    | Read the JEDEC ID and cache its value                 |
+    | This saves a significant amount of time over reading  |
+    | the JEDEC ID each time it is accessed                 |
+    \*-----------------------------------------------------*/
+    if(accessor == nullptr)
+    {
+        jedec_id_val = 0x0000;
+    }
+    else
+    {
+        jedec_id_val = accessor->jedec_id();
+    }
 }
 
 SPDWrapper::~SPDWrapper()
@@ -45,11 +75,7 @@ int SPDWrapper::index()
 
 uint16_t SPDWrapper::jedec_id()
 {
-    if(accessor == nullptr)
+    return jedec_id_val;
-    {
-        return 0x0000;
-    }
-    return accessor->jedec_id();
 }
 
 uint8_t SPDWrapper::manufacturer_data(uint16_t index)
@@ -61,12 +87,16 @@ uint8_t SPDWrapper::manufacturer_data(uint16_t index)
     return accessor->manufacturer_data(index);
 }
 
-/*-------------------------------------------------------------------------*\
+/*---------------------------------------------------------*\
-| Helper functions for easier collection handling.                          |
+| Helper functions for easier collection handling.          |
-\*-------------------------------------------------------------------------*/
+\*---------------------------------------------------------*/
 
 bool is_jedec_in_slots(std::vector<SPDWrapper> &slots, uint16_t jedec_id)
 {
+    /*-----------------------------------------------------*\
+    | Search through all SPD slots to see if any have the   |
+    | desired JEDEC ID                                      |
+    \*-----------------------------------------------------*/
     for(SPDWrapper &slot : slots)
     {
         if(slot.jedec_id() == jedec_id)
@@ -81,6 +111,10 @@ std::vector<SPDWrapper*> slots_with_jedec(std::vector<SPDWrapper> &slots, uint16
 {
     std::vector<SPDWrapper*> matching_slots;
 
+    /*-----------------------------------------------------*\
+    | Search through all SPD slots and build a list of all  |
+    | slots matching the desired JEDEC ID                   |
+    \*-----------------------------------------------------*/
     for(SPDWrapper &slot : slots)
     {
         if(slot.jedec_id() == jedec_id)

SPDAccessor/SPDWrapper.h

@@ -28,6 +28,7 @@ class SPDWrapper
   private:
     SPDAccessor *accessor = nullptr;
     uint8_t address;
+    uint16_t jedec_id_val;
     SPDMemoryType mem_type;
 };