Reduce crossing and queue depths to save space and ease timing

src/main/scala/devices/xilinx/xilinxml507mig/XilinxML507MIG.scala

@@ -20,7 +20,7 @@ class MemoryController extends BlackBox {
         val ddr2 = new MemoryDDR2IO
 
         val request_addr = Input(UInt(28.W))
-        val request_read = Input(Bool())
+        val request_type = Input(Bool())
         val request_data = Input(UInt(256.W))
         val request_mask = Input(UInt(32.W))
         val request_valid = Input(Bool())
@@ -34,11 +34,20 @@ class MemoryController extends BlackBox {
     override def desiredName: String = "memory_controller"
 }
 
+class ResponseQueueIO extends Bundle {
+    val read = Bool()
+    val source = UInt()
+    val size = UInt()
+}
+
 class XilinxML507MIGToTL(c: XilinxML507MIGParams)(implicit p: Parameters) extends LazyModule with HasCrossing {
     // Corresponds to MIG interface with 64 bit width and a burst length of 4
     val width = 256
     val beatBytes = width/8 // 32 byte (half a cache-line, fragmented)
-    val crossing = AsynchronousCrossing(8)
+
+    val address_range = AddressRange.fromSets(c.address).head
+    require(log2Ceil(address_range.size) == 28, "Max 256MiB DIMMs supported")
+    val crossing = AsynchronousCrossing(1)
 
     val device = new MemoryDevice
     val node = TLManagerNode(
@@ -58,6 +67,7 @@ class XilinxML507MIGToTL(c: XilinxML507MIGParams)(implicit p: Parameters) extend
     // We could possibly also support supportsPutPartial, as we need support
     // for masks anyway because of the possibility of transfers smaller that
     // the data width (size signal, see below).
+    // Seems we can: TL$7.3
 
     lazy val module = new LazyModuleImp(this) {
         val io = IO(new Bundle {
@@ -72,7 +82,7 @@ class XilinxML507MIGToTL(c: XilinxML507MIGParams)(implicit p: Parameters) extend
         // in: TLBundle, edge: TLEdgeIn
         val (in, edge) = node.in(0)
 
-        // Due to the Fragmenter defined above, all messages are 32 bytes or
+        // Due to the TLFragmenter defined below, all messages are 32 bytes or
         // smaller. The data signal of the TL channels is also 32 bytes, so
         // all messages will be transfered in a single beat.
         // Also, TL guarantees (see TL$4.6) that the payload of a data message
@@ -80,34 +90,76 @@ class XilinxML507MIGToTL(c: XilinxML507MIGParams)(implicit p: Parameters) extend
         // byte data signal, data[7:0] will always have address 0x***00000 and
         // data[255:247] address 0x***11111. It is also guaranteed that the
         // mask bits always correctly reflect the active bytes inside the beat
-        // with respect to the size and address.
-        // So we can directly forward the mask, (relative) address and possibly
-        // data to the MIG interface.
-        // Put requests can be acknowledged as soon as they are latched into
-        // the write fifo of the MIG (possibly combinatorily).
-        // For read requests, we have to store the source id and size in a
-        // queue for later acknowledgment.
-        // We are ready if both the MIG and the response data queue are not
-        // full.
+        // with respect to the size and address. So we can directly forward
+        // the mask, (relative) address and data to the MIG interface.
 
-        // Widths of the A channel:
-        // addressBits: 32
-        // dataBits:    256
-        // sourceBits:  6
-        // sinkBits:    1
-        // sizeBits:    3
+        // An AddressSet is always aligned, so we don't need to subtract the
+        // base address, we can just take the lower bits. The lowest 5 bits
+        // are used for indexing the 32 byte word of the MIG.
+        val address = in.a.bits.address(27, 0) & "hFFFFFE0".U
 
-        // source (from): in.a.bits.source
-        // adresse (to): edgeIn.address(in.a.bits)
-        // size: edgeIn.size(in.a.bits)
-        // isPut: edgeIn.hasData(in.a.bits)
+        // Save the source, size and type of the requests in a queue so we
+        // can synthesize the right responses in fifo order. The length also
+        // determines the maximum number of in-flight requests.
+        val ack_queue = Module(new Queue(new ResponseQueueIO, 2))
 
-        // bits kommt von Decoupled: ready, valid + bits
+        // Pass data directly to the controller
+        controller.io.request_addr := address
+        controller.io.request_type := !edge.hasData(in.a.bits)
+        controller.io.request_data := in.a.bits.data
+        // TL uses high to indicate valid data while mig uses low
+        controller.io.request_mask := ~ in.a.bits.mask
 
-        println("a parameters: " + in.a.bits.params)
+        ack_queue.io.enq.bits.read   := !edge.hasData(in.a.bits)
+        ack_queue.io.enq.bits.source := in.a.bits.source
+        ack_queue.io.enq.bits.size   := in.a.bits.size
+
+        // We are ready when the controller and the queue input are ready
+        in.a.ready := controller.io.request_ready && ack_queue.io.enq.ready
+        // Both queues only latch data if the other is ready, so that data
+        // is latched into both queues or not at all
+        controller.io.request_valid := in.a.valid && ack_queue.io.enq.ready
+        ack_queue.io.enq.valid      := in.a.valid && controller.io.request_ready
+
+
+        // We have to buffer the responses from the MIG as it has no internal
+        // buffer and will output its read responses only for one cycle. To
+        // avoid losing any responses, this queue *must* be at least as wide
+        // as the ack queue, so that we can catch all responses, even if the
+        // ack queue is completely filled with read requests.
+        val response_queue = Module(new Queue(controller.io.response_data, 2))
+
+        response_queue.io.enq.bits  := controller.io.response_data
+        response_queue.io.enq.valid := controller.io.response_valid
+        // MIG does not support delaying a response, so we ignore enq.ready.
+        // This will result in lost reads and returning wrong data in further
+        // AccessAckData messages, so this must be avoided (see above).
+
+        // Acks may or may not contain data depending on the request, but we
+        // can always pass the data, even if it is invalid in the write case,
+        // because it is ignored for AccessAck responses
+        val response_read = ack_queue.io.deq.bits.read
+        in.d.bits.opcode := Mux(response_read, TLMessages.AccessAckData, TLMessages.AccessAck)
+        in.d.bits.param  := UInt(0) // reserved, must be 0
+        in.d.bits.size   := ack_queue.io.deq.bits.size
+        in.d.bits.source := ack_queue.io.deq.bits.source
+        in.d.bits.sink   := UInt(0) // ignored
+        in.d.bits.data   := response_queue.io.deq.bits
+        in.d.bits.error  := Bool(false)
+
+        // The data is valid when the ack queue data is valid (write case) or
+        // when the ack *and* response queues are valid (read case)
+        in.d.valid       := ack_queue.io.deq.valid && (!response_read ||
+                            response_queue.io.deq.valid)
+        // Let the ack queue dequeue when the master is ready (write case) or
+        // when the master is ready *and* there is a valid response (read case)
+        ack_queue.io.deq.ready      := in.d.ready && (!response_read ||
+                                       response_queue.io.deq.valid)
+        // Let the response queue dequeue when the master is ready and there
+        // is a valid read ack waiting
+        response_queue.io.deq.ready := in.d.ready && response_read &&
+                                       ack_queue.io.deq.valid
 
-        in.a.ready := Bool(false)
-        in.d.valid := Bool(false)
 
         // Tie off unused channels
         in.b.valid := Bool(false)

src/main/scala/ip/xilinx/Xilinx.scala

@@ -114,6 +114,17 @@ class ml507_dvi_clock extends BlackBox {
   }
 }
 
+class ml507_ddr2_clock extends BlackBox {
+  val io = new Bundle {
+    val CLKIN_P_IN = Clock(INPUT)
+    val CLKIN_N_IN = Clock(INPUT)
+    val CLK0_OUT   = Clock(OUTPUT)
+    val CLK90_OUT  = Clock(OUTPUT)
+    val CLKDV_OUT  = Clock(OUTPUT)
+    val LOCKED_OUT = Bool(OUTPUT)
+  }
+}
+
 //-------------------------------------------------------------------------
 // vc707_sys_clock_mmcm
 //-------------------------------------------------------------------------

src/main/scala/shell/xilinx/ML507Shell.scala

@@ -16,7 +16,7 @@ import sifive.blocks.devices.chiplink._
 import sifive.blocks.devices.terminal._
 
 import sifive.fpgashells.devices.xilinx.xilinxml507mig._
-import sifive.fpgashells.ip.xilinx.{PowerOnResetFPGAOnly, sdio_spi_bridge, ml507_dvi_clock, ml507_sys_clock, vc707reset}
+import sifive.fpgashells.ip.xilinx.{PowerOnResetFPGAOnly, sdio_spi_bridge, ml507_ddr2_clock, ml507_dvi_clock, ml507_sys_clock, vc707reset}
 
 //-------------------------------------------------------------------------
 // ML507Shell
@@ -57,6 +57,10 @@ abstract class ML507Shell(implicit val p: Parameters) extends RawModule {
   // 100Mhz sysclk
   val sys_clock            = IO(Input(Clock()))
 
+  // 200MHz ddrclk
+  val ddr_clock_p          = IO(Input(Clock()))
+  val ddr_clock_n          = IO(Input(Clock()))
+
   // active high async reset
   val reset                = IO(Input(Bool()))
 
@@ -97,6 +101,12 @@ abstract class ML507Shell(implicit val p: Parameters) extends RawModule {
   val dvi_clock       = Wire(Clock())
   val dvi_reset       = Wire(Bool())
 
+  val ddr_clk0        = Wire(Clock())
+  val ddr_clk90       = Wire(Clock())
+  val ddr_clkdiv0     = Wire(Clock())
+  val ddr_clk_locked  = Wire(Bool())
+  val ddr_reset       = Wire(Bool())
+
   val sd_spi_sck      = Wire(Bool())
   val sd_spi_cs       = Wire(Bool())
   val sd_spi_dq_i     = Wire(Vec(4, Bool()))
@@ -126,9 +136,19 @@ abstract class ML507Shell(implicit val p: Parameters) extends RawModule {
   ml507_dvi_clock.io.CLKIN_IN := sys_clock
   dvi_clock := ml507_dvi_clock.io.CLKFX_OUT
 
+  // 200 MHz (DDR2 and IDELAY clock)
+  val ml507_ddr2_clock = Module(new ml507_ddr2_clock)
+  ml507_ddr2_clock.io.CLKIN_P_IN := ddr_clock_p
+  ml507_ddr2_clock.io.CLKIN_N_IN := ddr_clock_n
+  ddr_clk0 := ml507_ddr2_clock.io.CLK0_OUT
+  ddr_clk90 := ml507_ddr2_clock.io.CLK90_OUT
+  ddr_clkdiv0 := ml507_ddr2_clock.io.CLKDV_OUT
+  ddr_clk_locked := ml507_ddr2_clock.io.LOCKED_OUT
+
   // Clocks locked?
   clk_locked := ml507_sys_clock.io.LOCKED_OUT &
-                ml507_dvi_clock.io.LOCKED_OUT
+                ml507_dvi_clock.io.LOCKED_OUT &
+                ddr_clk_locked
 
   //-----------------------------------------------------------------------
   // System reset
@@ -140,7 +160,8 @@ abstract class ML507Shell(implicit val p: Parameters) extends RawModule {
   val safe_reset = Module(new vc707reset)
 
   safe_reset.io.areset := do_reset
-  safe_reset.io.clock1 := dut_clock
+  safe_reset.io.clock1 := ddr_clk0
+  ddr_reset            := safe_reset.io.reset1
   safe_reset.io.clock2 := dut_clock
   safe_reset.io.clock3 := dvi_clock
   dvi_reset            := safe_reset.io.reset3
@@ -167,6 +188,13 @@ abstract class ML507Shell(implicit val p: Parameters) extends RawModule {
 
   def connectDDRMemory(dut: HasMemoryML507ModuleImp): Unit = {
     ddr2 <> dut.ddr2
+
+    dut.ddr_sys.clk0 := ddr_clk0
+    dut.ddr_sys.clk90 := ddr_clk90
+    dut.ddr_sys.clkdiv0 := ddr_clkdiv0
+    dut.ddr_sys.clk_locked := ddr_clk_locked
+    dut.ddr_sys.clk_idelay := ddr_clk0
+    dut.ddr_sys.reset := ddr_reset
   }
 
   //-----------------------------------------------------------------------