From ad7a6e60f214acfa9dc1f1100e689476a0e93228 Mon Sep 17 00:00:00 2001 From: Adam Honse Date: Fri, 27 Jun 2025 00:30:06 -0500 Subject: [PATCH] Cache the JEDEC ID value in SPDAccessor and improve SPD detector logging. Caching the JEDEC ID speeds up DRAM detection significantly. --- ResourceManager.cpp | 3 +-- SPDAccessor/SPDDetector.cpp | 9 ++++--- SPDAccessor/SPDWrapper.cpp | 52 ++++++++++++++++++++++++++++++------- SPDAccessor/SPDWrapper.h | 1 + 4 files changed, 51 insertions(+), 14 deletions(-) diff --git a/ResourceManager.cpp b/ResourceManager.cpp index d06c4739..9c627d8d 100644 --- a/ResourceManager.cpp +++ b/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 && - is_jedec_in_slots(slots, i2c_dimm_device_detectors[i2c_detector_idx].jedec_id)) + 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)) { detection_string = i2c_dimm_device_detectors[i2c_detector_idx].name.c_str(); diff --git a/SPDAccessor/SPDDetector.cpp b/SPDAccessor/SPDDetector.cpp index dc5e3bad..f3a850a1 100644 --- a/SPDAccessor/SPDDetector.cpp +++ b/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; } diff --git a/SPDAccessor/SPDWrapper.cpp b/SPDAccessor/SPDWrapper.cpp index 48da3176..7db22f01 100644 --- a/SPDAccessor/SPDWrapper.cpp +++ b/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 0x0000; - } - return accessor->jedec_id(); + return jedec_id_val; } 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 &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 slots_with_jedec(std::vector &slots, uint16 { std::vector 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) diff --git a/SPDAccessor/SPDWrapper.h b/SPDAccessor/SPDWrapper.h index 34c15bef..b615e0e2 100644 --- a/SPDAccessor/SPDWrapper.h +++ b/SPDAccessor/SPDWrapper.h @@ -28,6 +28,7 @@ class SPDWrapper private: SPDAccessor *accessor = nullptr; uint8_t address; + uint16_t jedec_id_val; SPDMemoryType mem_type; };