frontend/avalon: properly implement bursts

[hansfbaier]

Read and write bursts are now implemented properly, ie without gaps on the native ports.

[hansfbaier]

Wait, this did not verify on hardware.

[hansfbaier]

Problem is, the downconverter does not look good It creates separate requests, and the addresses are messed up. That also explains why I had to make a whole bunch of exceptions in the single request, because the downconverter behaves quite differently from the others. I think I have to write my own, because fixing the downconverter might break the other frontends.

[Edit] Aaah. dut_cmd_payload_addr comes too late.

[hansfbaier]

I added test cases for up and downconverter bursts (not sure if I got those right yet, if you have time to take a look at those so see if my assumptions are correct). The Upconverter test does not terminate that, so only the downconverter test is relevant. After the latest fixes I pushed, the downconverter test now looks like this: Which seems good to me at the Moment, but what ends up in memory is this:

0x00000000: 0xdead
0x00000001: 0xee00
0x00000002: 0xc0ff
0x00000003: 0x3210
0x00000004: 0x7654

which seems to be somewhat random parts of the written data. Also noteworthy that the cmd port requests of the downconverter are separated, but the wdata port requests are now. Might that confuse the native interface?

[enjoy-digital]

@hansfbaier: I don't see obvious issue here on the traces. This could be worth testing this with a litex_sim/verilator simulation and LiteDRAM included. This would allow you to have more visibility.

[hansfbaier]

Removing WIP, ready for review, all tests passed and verified in hardware. Burst reads work now as they should: And burst writes too:

[enjoy-digital]

Thanks @hansfbaier, this looks good and is merged. I'll maybe just do some minor cosmetic changes while also reviewing it more closely. Thanks also for the testbenches, this is very useful to do these minor changes with confidence I'm not breaking the functionality.

[enjoy-digital]

Hi @hansfbaier,

I started reviewing more closely and reworking a bit the code in https://github.com/enjoy-digital/litedram/tree/avalon_frontend_review_cleanup: The FSM is doing multiple things at once which is make the code difficult to read so I'm trying to decouple things a bit.

To allow me to go further, can you just describe the LiteDRAM DownConverter issue that you seems to workaround in the code (specific way to send the cmd/data in the FSM for this case). I could have a look at improving the LiteDRAM DownConverter on this aspect to avoid complicated code in the Avalon Frontend.

Thanks,

Florent

[hansfbaier]

It is because it needs the data on the wdata port be valid at the same time as on the command port. Good way to find out in detail is delete all downconvert conditions and then run the tests.

[hansfbaier]

Also port.cmd.vaild needs to go low, after port.cmd.ready has been asserted.

[hansfbaier]

@enjoy-digital I made a couple of nice GTKW files, in case you want to look at the traces of the tests https://github.com/hansfbaier/litedram/tree/test_gtkw

[enjoy-digital]

Thanks @hansfbaier, I indeed saw the simulation was hanging when disabling this. I'll continue looking at the code, we can probably simplify it even more with a better decoupling of the CMD / Data paths. The testbenches you created are very useful for this.

[enjoy-digital]

I've been able to remove the downconvert specific cases by doing a small modification to the DownConverter and also did other simplification (Always go through the Wdata FIFO, remove the Cmd FIFO, etc...). This is here: https://github.com/enjoy-digital/litedram/tree/avalon_frontend_review_cleanup. I'll wait for CI to be green before merging and would also be interested by your feedback (if you are already testing this on hardware).

[hansfbaier]

Yes it works, and I think the average read latency got better too. It used to be 14 cycles and now it is 11.

[hansfbaier]

Hmm wait. In the Menu core for some reason the DDR3 went dead. Yes, waitrequest and write are stuck at one.

With the old version it works.

[hansfbaier]

That is a regression. I have seen this in hardware before, but unfortunately I forgot the context.

[hansfbaier]

49f48130e7ca207679ada342dfaadf11e13a430d is a good revision

[hansfbaier]

a86bd6c3d0cac3fe574f4e996f2a05f1cd0af281 broke it. Looks like cmd_fifo is actually useful.

[enjoy-digital]

Thanks @hansfbaier, I just did other simplifications of the code still in https://github.com/enjoy-digital/litedram/tree/avalon_frontend_review_cleanup. I think we now have something that is easy to understand and will be easy to maintain in the future. This is passing the testbenches but that's possible the issue you see on hardware is not covered by the testbenches. Would you mind testing again this version on hardware? Thanks.

[hansfbaier]

@enjoy-digital Unfortunately, it isn't working. I wonder why since all the tests pass. Litescope pretty much complete silence

[hansfbaier]

As long as the scaler is not activated (MiSTer executable not run), I can see writes by the scaler, which seem to work, but no trigger on reads.

[hansfbaier]

If I trigger on read directly before starting the MiSTer executable, which activates the scaler, I get a working read request: and subsequent writes.

[hansfbaier]

After that it goes dead (no triggers on neiter reads nor writes)

[hansfbaier]

And the screen looks like this (animation freezes). No moving noise pattern anymore.

[hansfbaier]

I have to say this is a downconverting case, scaler port is 64 bits wide.

        __   _ __      _  __
       / /  (_) /____ | |/_/
      / /__/ / __/ -_)>  <
     /____/_/\__/\__/_/|_|
   Build your hardware, easily!

 (c) Copyright 2012-2023 Enjoy-Digital
 (c) Copyright 2007-2015 M-Labs

 BIOS CRC passed (d4c43b78)

 LiteX git sha1: 57bffbbb

--=============== SoC ==================--
CPU:        FemtoRV-STANDARD @ 125MHz
BUS:        WISHBONE 32-bit @ 4GiB
CSR:        32-bit data
ROM:        32.0KiB
SRAM:       4.0KiB
SDRAM:      128.0MiB 8-bit @ 1000MT/s (CL-8 CWL-6)
MAIN-RAM:   128.0MiB

--========== Initialization ============--
Initializing SDRAM @0x40000000...
Switching SDRAM to software control.
Read leveling:
  m0, b00: |00000000000000000000000000000000| delays: -
  m0, b01: |00000000000000000000000000000000| delays: -
  m0, b02: |11111000000000000000000000000000| delays: 02+-02
  m0, b03: |00000001111111111100000000000000| delays: 11+-05
  m0, b04: |00000000000000000001111111111100| delays: 24+-05
  m0, b05: |00000000000000000000000000000000| delays: -
  m0, b06: |00000000000000000000000000000000| delays: -
  m0, b07: |00000000000000000000000000000000| delays: -
  best: m0, b03 delays: 12+-05
Switching SDRAM to hardware control.
Memtest at 0x40000000 (2.0MiB)...
  Write: 0x40000000-0x40200000 2.0MiB     
   Read: 0x40000000-0x40200000 2.0MiB     
Memtest OK
Memspeed at 0x40000000 (Sequential, 2.0MiB)...
  Write speed: 41.1MiB/s
   Read speed: 25.3MiB/s

--============== Boot ==================--
Booting from serial...
Press Q or ESC to abort boot completely.
sL5DdSMmkekro
             Timeout
No boot medium found

--============= Console ================--

litex> sdram_test
Memtest at 0x40000000 (4.0MiB)...
  Write: 0x40000000-0x40400000 4.0MiB     
   Read: 0x40000000-0x40400000 4.0MiB     
Memtest OK

[hansfbaier]

This time also the Template core is broken. Seems to work for a split second and then goes dead. I wonder how to trigger the failure case.

[enjoy-digital]

@hansfbaier: Thanks for the feedback/test. This could be a case that was hidden previously or a back pressure issue. I'll try to also do some tests on hardware or add a bit of randomness on the testbenches.

[hansfbaier]

@enjoy-digital I think I remember having those lockup issues until I introduced separate command/wdata FIFOs.

[hansfbaier]

@enjoy-digital Which also is somewhat confirmed by the observation that it breaks exactly with the commit that undoes that. So that might give a clue for what to look out for.

[enjoy-digital]

@hansfbaier: I just added back a FIFO on the Cmd with https://github.com/enjoy-digital/litedram/commit/ec6d2467c7c16c87fc8e986bdb9d43195c620201, this could be worth doing a test on hardware.

[hansfbaier]

@enjoy-digital Still the same symptoms, I'm afraid.

[hansfbaier]

@enjoy-digital It might be the premature termination of bursts. In the original MiSTer sys there was this module in between the Altera avalon bus and the scaler. https://github.com/MiSTer-devel/Intv_MiSTer/blob/master/sys/f2sdram_safe_terminator.sv The issues might have to do something with it, maybe the old code better handles abnormal burst termination.

[hansfbaier]

Hmm concluding from the comments this is rather about HPS avalon bus safety when new bitstreams are loaded. Since we don't have an HPS avalon bus and the avalon bus we have goes away with the new core, this should be be an issue.

[hansfbaier]

@enjoy-digital Might be instructive to compare current head with the last working revision https://github.com/enjoy-digital/litedram/commit/49f48130e7ca207679ada342dfaadf11e13a430d and see where the qualitative difference might be

[hansfbaier]

Even with version https://github.com/enjoy-digital/litedram/commit/49f48130e7ca207679ada342dfaadf11e13a430d I actually have problems in QBert.

[hansfbaier]

I just tried revision 4a8b102c7ef598c6f73203e3ed8c3b14590cf569 (built QBert with it, which makes full use of rotation and scaler), and it works as well as the original branch. I have to correct myself. The MiSTeX cores use an upconverter not a downconverter though. (DDR3 is 8 bit, avalon is 64 bit).