enjoy-digital / litedram

Small footprint and configurable DRAM core
Other
365 stars 115 forks source link

Test: add tests for BIST modules with different access patterns #165

Closed jedrzejboczar closed 4 years ago

jedrzejboczar commented 4 years ago

This PR adds tests for _LiteDRAMBISTGenerator, _LiteDRAMPatternGenerator, _LiteDRAMBISTChecker, _LiteDRAMPatternChecker. Test data definitions are created in setUp and then used as reference in different test.

It tests current pattern addressing (not https://github.com/enjoy-digital/litedram/issues/164).

If you would prefer to wait for all the BIST related tests, than I can just commit to this PR.

enjoy-digital commented 4 years ago

Thanks. If you still have tests you want to do for the BIST and it's not blocking you if this PR is not merged for now, you can add then to this PR yes. I'll try to review it soon.

jedrzejboczar commented 4 years ago

I've added tests with memory using clock domain asynchronous to sys and it seems that there is a problem with synchronization in BIST modules.

BusSynchronizers take much more time for base and error signals to propagate, then for start and done signals, so e.g. a write to base followed by a write to start results in the first word being written by generator using old value of base before the new value propagates through the synchronization logic.

This of course can be fixed by adding delays, but I wonder if maybe we should add some kind of synchronization on start and done signals that ensures that all other values get propagate before.

More information:

The tests have the same sequence as before - write, read, write to another location (corrupt few values written previously), read first location (errors), read new locaton (ok). With this setup test_bist_csr_cdc fails due to:

If you want to take a look, I attach a gtkwave file for that test (removed asserion for errors == 4 to have full simulatin): sim-gtkwave.zip

jedrzejboczar commented 4 years ago

I've added the remaining BIST tests from https://github.com/enjoy-digital/litedram/issues/156, so you can review the tests when you have some time. test_bist_csr_cdc fails as I described in https://github.com/enjoy-digital/litedram/pull/165#issuecomment-599629743.

enjoy-digital commented 4 years ago

Thanks, this seems fine and we can probably merge with a green Travis-CI. Can you just:

enjoy-digital commented 4 years ago

Thanks, merged.