enjoy-digital / litesata

Small footprint and configurable SATA core
Other
123 stars 34 forks source link

Zero delay oscillations in Vivado XSIM simulation #18

Closed mik1234mc closed 3 years ago

mik1234mc commented 4 years ago

Dear litesata authors,

I came across your litesata IP and decided to test it. I was able to modify Python code, but for integration I would like to use more standard flow, it means to use the generated litesata Verilog backend. I used Verilog code generated by .examples/make.py build-core that is 'bist' design, but the further-described behaviour would be the same for other examples as well. I tried to setup the litesata simulation in Vivado XSIM, but I ran into troubles with the following error:

FATAL_ERROR: Iteration limit 10000 is reached. Possible zero delay oscillation detected where simulation time can not advance. Please check your source code. Note that the iteration limit can be changed using switch -maxdeltaid. Time: 0 ps  Iteration: 10000

After double-checking everything and hours of eyeballing into the Verilog netlist I found the signals which are set and read in the same process causing the zero delay oscillation in the simulation:

line 2927: transport_tx_sink_ready <= link_litesatalinktx_sink_sink_ready; // if (((transport_tx_sink_valid & transport_tx_sink_last) & transport_tx_sink_ready)) begin // should be changed to something like this: if (((transport_tx_sink_valid & transport_tx_sink_last) & link_litesatalinktx_sink_sink_ready)) begin

around line 3174: command_tx_sink_ready <= transport_tx_sink_ready; // if ((command_tx_sink_valid & command_tx_sink_ready)) begin // should be changed to something like this: if ((command_tx_sink_valid & transport_tx_sink_ready)) begin

After these changes, the zero delay oscillations disappear and simulation time may proceed.

I think this should be corrected in the migen code even though this may not be a problem for the synthesis...

Thank you for your effort, Michael

mik1234mc commented 4 years ago

just adding diff making these changes in Migen code:

diff --git a/litesata/core/command.py b/litesata/core/command.py
index 08f52b1..386c194 100644
--- a/litesata/core/command.py
+++ b/litesata/core/command.py
@@ -68,7 +68,7 @@ class LiteSATACommandTX(Module):
             transport.sink.valid.eq(sink.valid),
             transport.sink.last.eq(1),
             transport.sink.c.eq(1),
-            If(transport.sink.valid & transport.sink.ready,
+            If(sink.valid & transport.sink.ready,
                 If(is_write,
                     NextState("WAIT_DMA_ACTIVATE")
                 ).Else(
@@ -90,7 +90,7 @@ class LiteSATACommandTX(Module):
             transport.sink.valid.eq(sink.valid),
             transport.sink.last.eq((dwords_counter == (fis_max_dwords-1)) | sink.last),
             sink.ready.eq(transport.sink.ready),
-            If(sink.valid & sink.ready,
+            If(sink.valid & transport.sink.ready,
                 NextValue(dwords_counter, dwords_counter + 1),
                 If(sink.last,
                     NextState("IDLE")
diff --git a/litesata/core/transport.py b/litesata/core/transport.py
index 29e32a8..643245c 100644
--- a/litesata/core/transport.py
+++ b/litesata/core/transport.py
@@ -87,7 +87,7 @@ class LiteSATATransportTX(Module):
         fsm.act("SEND_DATA",
             data_send.eq(1),
             sink.ready.eq(link.sink.ready),
-            If(sink.valid & sink.last & sink.ready,
+            If(sink.valid & sink.last & link.sink.ready,
                 NextState("IDLE")
             )
         )
enjoy-digital commented 4 years ago

@michael-vacek: thanks for the feedback. This issue can indeed happen with XSIM and is generally worked around by setting regular_comb=False during the generation of the verilog code (https://github.com/enjoy-digital/litex/blob/master/litex/gen/fhdl/verilog.py#L385), can you try with that?

enjoy-digital commented 3 years ago

Closing since the question has been answered.