-
Notifications
You must be signed in to change notification settings - Fork 207
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
timing lookup table typos? #47
Comments
I think you meant the .cpp files DDR4.cpp and DDR3.cpp. Yes, seems like you are right about the typos. It should definitely be WR-to-WR timing as RD-to-WR timing is defined later. We will get it fixed. Thanks for the heads up. |
@arthasSin
I'd appreciate it if you could check whether the following code is correct.
|
@hmhmhey yes, we are aware of that issue. I will go over all timing parameters soon to make sure everything is in order. Your changes make sense but I am not sure if it is entirely correct. I will need to check the DDR4 specification. Thanks. |
@hmhmhey Your proposed change is incorrect. The correct timing constraints are defined at the rank level, and hold regardless of whether you hit the same or a different bank group (because tCL is trivially larger than both tCCDS and tCCDL). At the rank level, technically tCCDS needs to be replaced with tBL. In practice you'll find that the two are both equal to 4 for DDR4. tCCDS is lower bound by tBL, and any higher value for tCCDS implies the clock frequency is poorly chosen. I have opened a pull request (#57) that corrects several timing issues in DDR4. One of the patches addresses the issue you pointed out by removing the four items wholesale. Another patch corrects the definition of RD->WR issue latency by replacing tCCDS with tBL. Feel free to check out my DDR4-fix branch if you wish to test my adjustments. |
Pull request #57 has just been merged, meaning @hmhmhey's issue has been resolved. As for the original issue posted by @stevekulick , my current experimental set-up has only a single rank. As such, I am unable to test or verify any changes in that area. |
DDR4.h (DDR3.h as well)
// CAS <-> CAS (between sibling ranks)
t[int(Command::RD)].push_back({Command::RD, 1, s.nBL + s.nRTRS, true});
t[int(Command::RD)].push_back({Command::RDA, 1, s.nBL + s.nRTRS, true});
t[int(Command::RDA)].push_back({Command::RD, 1, s.nBL + s.nRTRS, true});
t[int(Command::RDA)].push_back({Command::RDA, 1, s.nBL + s.nRTRS, true});
The text was updated successfully, but these errors were encountered: