CDC synchronization of CSRs in LiteDRAMBISTGenerator/LiteDRAMBISTChecker

[jedrzejboczar]

This issue describes problem from https://github.com/enjoy-digital/litedram/pull/165#issuecomment-599629743.

When the memory port passed to LiteDRAMBISTGenerator/LiteDRAMBISTChecker is not in sys clock domain, then synchronization is added for CSRs. The problem is that synchronization of multi-bit CSRs like base takes longer than synchronization of single-bit ones, like start. This results in a situation where, even though we write to base first and then to start, it may happend that start propagates fast enough that the LiteDRAMBISTGenerator module will use old value of base when writing first word. This is similar for LiteDRAMBISTChecker, where errors do not have final value at the moment when done is already 1.

Should we add additional synchronization of start/done to account for that?

A test for this situation has been added in https://github.com/enjoy-digital/litedram/pull/165: test_bist_csr_cdc. Simulation data from the test can be found in sim-gtkwave.zip (includes all stages of bist_test, with errors == 4 asserion commented out).

test_bist_csr_cdc fails due to (referring to the stages from bist_test method):

• 2nd checker run reports 1 error, altough it really finds 3 (errors gets updated after done); ideally there should be 4 errors, but the generator on its 2nd run ("corrupt memory") writes the first word to wrong location (using old value of base)
• 3rd checker run reports 1 error (at address 0x10, which is where generator has incorrectly written value on 2nd run); this error is picked up correctly as it happens on the first word, long before done

[enjoy-digital]

Thanks for the detailed report. The CDC has been refactored with https://github.com/enjoy-digital/litedram/commit/92e34d4d376b1f62de1d85a58a396c3fa224b67b and the test has been enabled with https://github.com/enjoy-digital/litedram/commit/c55136c17ae13a026097c9da2b88143fd1276eab and is now passing.