dual-port, writable, READ_FIRST, memory pattern not recognized

[jordens]

No mention of it being detected as a TDP pattern by Vivado (when WRITE_FIRST it is detected)

Used in artiq/suservo:

https://github.com/m-labs/artiq/blob/7d4a103a43a2cc4c1787eaa7aff2f37de8738050/artiq/gateware/suservo/iir.py#L347 https://github.com/m-labs/artiq/blob/7d4a103a43a2cc4c1787eaa7aff2f37de8738050/artiq/gateware/rtio/phy/servo.py#L34-L37

diff --git a/artiq/gateware/rtio/phy/servo.py b/artiq/gateware/rtio/phy/servo.py
index eb1f8495..864c82eb 100644
--- a/artiq/gateware/rtio/phy/servo.py
+++ b/artiq/gateware/rtio/phy/servo.py
@@ -28,12 +28,10 @@ class RTServoMem(Module):
     interface."""
     def __init__(self, w, servo):
         m_coeff = servo.iir.m_coeff.get_port(write_capable=True,
-                mode=READ_FIRST,
-                we_granularity=w.coeff, clock_domain="rio")
+                mode=READ_FIRST, we_granularity=w.coeff, clock_domain="rio")
         assert len(m_coeff.we) == 2
         m_state = servo.iir.m_state.get_port(write_capable=True,
-                # mode=READ_FIRST,
-                clock_domain="rio")
+                mode=READ_FIRST, clock_domain="rio")
         self.specials += m_state, m_coeff

         # just expose the w.coeff (18) MSBs of state
diff --git a/artiq/gateware/suservo/iir.py b/artiq/gateware/suservo/iir.py
index bd4c7dda..3d0ab0b3 100644
--- a/artiq/gateware/suservo/iir.py
+++ b/artiq/gateware/suservo/iir.py
@@ -344,7 +344,7 @@ class IIR(Module):
         ]

         m_coeff = self.m_coeff.get_port()
-        m_state = self.m_state.get_port(write_capable=True)  # mode=READ_FIRST
+        m_state = self.m_state.get_port(write_capable=True, mode=READ_FIRST)
         self.specials += m_state, m_coeff

         dsp = DSP(w)

Verilog:

...

reg [24:0] m_state[0:271];
reg [24:0] memdat_6;
reg [24:0] memdat_7;
always @(posedge rio_phy_clk) begin
        if (main_iir_m_state_we)
                m_state[main_iir_m_state_adr] <= main_iir_m_state_dat_w;
        memdat_6 <= m_state[main_iir_m_state_adr];
end

always @(posedge rio_clk) begin
        if (main_mem_m_state_we)
                m_state[main_mem_m_state_adr] <= main_mem_m_state_dat_w;
        memdat_7 <= m_state[main_mem_m_state_adr];
end

assign main_iir_m_state_dat_r = memdat_6;
assign main_mem_m_state_dat_r = memdat_7;

...

[nkrackow]

I came across this bug, too. Only mode=READ_FIRST leads to vivado inferring a TDP ram. mode=WRITE_FIRST leads to distributed ram which somehow shows different behavior than a write first TDP. Specifically vivado would sometimes omit the write enable signal and set it to constant high. NextPnR had no problem correctly inferring a write first TDP.