soc/interconnect/wishbone: Add incrementing address burst mode to SRAM

[koluckirafal]

This pull request adds support for incrementing address burst cycles to SRAM module. Both linear and wrapped increments are supported.
Because for some designs burst might be unnecessary (e.g. no cache in CPU core), it is disabled by default. It can be turned on by passing parameter burst=True in SoCCore.add_ram() function, with shortcut available for integrated *RAM/ROM modules in the form of integrated_{rom,sram,main_ram}_burst parameters in SoCCore class.

Implementation was tested by writing a test suite and an example SoC for measuring speed difference: https://github.com/antmicro/wishbone-interconnect-burst-mode-benchmark (README contains results and instructions how to run tests).

Wishbone burst cycle documentation: https://wishbone-interconnect.readthedocs.io/en/latest/04_registered.html

[enjoy-digital]

Thanks @koluckirafal. I just did a quick first review:

• Burst support should probably be enabled at the bus level (through bus_bursting parameter that would then be propagated to memories).
• To be able to maintain this feature we'll need a unit-tests that cover the different burst mode/corner cases and would be run in LiteX-CI. Would it be possible to add it?

[koluckirafal]

• Burst support should probably be enabled at the bus level (through bus_bursting parameter that would then be propagated to memories).

Done, burst cycles support can be now enabled with bus_bursting optional parameter in SoCBusHandler. Corresponding argument in soc_core_args() is also now available.

• To be able to maintain this feature we'll need a unit-tests that cover the different burst mode/corner cases and would be run in LiteX-CI. Would it be possible to add it?

I couldn't find any tests in LiteX for Wishbone modules other than {Up,Down}Converter. Should I write tests only for SRAM or also for all other Wishbone modules?

[enjoy-digital]

Thanks for the update. The unit-tests are indeed minimal on Wishbone and that's clearly something we should improve in the future. I'm of course not asking you to write unit-tests for the other Wishbone modules but would like to have a unit-test for the SRAM that exercise the burst support (to be able to rework/maintain this code more easily in the future) Thanks!

[koluckirafal]

I added a basic unit test for SRAM write and read operations, should be enough for testing incrementing address burst cycle support - currently only linear and 4-beat wrap increments are tested.

[enjoy-digital]

Thanks for the update. I added a few review comments in the code. We should really try to simplify it when possible (otherwise it's difficult to do maintenance/support). Can you also add a test for Constant bursts? After addressing this it will be fine and I'll do the other improvement I can see myself. Thanks!

[koluckirafal]

Test for constant address burst cycle added - it works despite the lack of constant address burst support in wishbone.SRAM, because unsupported cycle types are treated as classic cycle.
About review comments, I couldn't find them, probably due to rebasing I did previously.

[enjoy-digital]

Thanks for the additional test, sorry the comments were still pending. This should be good now.

[enjoy-digital]

Thanks @koluckirafal for the updates/changes. This now looks good and can be merged. I'll maybe do another review or some changes on the burst logic, now that this is integrated correctly and with tests, it will be be easy to do this.

[enjoy-digital]

Sorry, while reading the code it's not clear to me which exact type of bursts should be supported can you list them?. It seems that adr_burst_wrap is not used, so how is done the differentiation between a Incrementing Linear Burst and Wrapped Burst?

[koluckirafal]

Differentation between linear and wrapped incrementing address bursts is done by BTE signal - it's used to select the wrap mask, which then is being used for separating the first address into initial counter value and offset.
If BTE is equal 0, then the first addess is loaded "as is" to the counter and offset register is unused; otherwise up to 4 least significant address bits (BTE == 0b11) are loaded to the offset register and are also set to 0 when setting initial counter value.
This way the same address counter logic can support both linear and {4, 8, 16}-beat wrapped increments.

adr_burst_wrap signal is probably a leftover from earlier experiments and can be removed.