soc: work on PipeCon connection for S4NoC

Author: schoeberl

README.md

@@ -5,16 +5,13 @@
 Collection of different communication methods for chip mulitprocessors.
 
 This repo shall include all the multicore communication we have done in
-[T-CREST](https://github.com/t-crest) as a standalone repo to make the
-work more useful.
+[T-CREST](https://github.com/t-crest) as a standalone repo to make the work more useful.
 
 We use a simple rd/wr/address/data/rdy interface, which maps
-dirctly to the Patmos OCPcore interface (A command is acked in the next
+directly to the Patmos OCPcore interface (A command is acked in the next
 clock cycle or later, IO devices need to be ready to accept a command
 during the ack cycle).
 
-The S4NOC has currently a slightly different interface (no rdy needed).
-
 The repo also contains a Wishbone wrapper.
 
 We plan to  provide a bridge to AXI, as AXI is not so super nice.
@@ -68,18 +65,15 @@ class PipeConIO(private val addrWidth: Int) extends Bundle {
 }
 ```
 
-```PipeCon``` itself is an abstract class, just containing  the interface:
+```PipeCon``` itself is an abstract class, just containing the interface:
 
 ```scala
 abstract class PipeCon(addrWidth: Int) extends Module {
-  val io = IO(new Bundle {
-    val cpuPort = new PipeConIO(addrWidth)
-  })
-  val cp = io.cpuPort
+   val cpuPort = IO(new PipeConIO(addrWidth))
 }
 ```
 
-The main rules define PipeCon:
+The main rules that define PipeCon:
 
  * There are two transactions: read and write
  * The transaction command is valid for a single clock cycle
@@ -195,17 +189,17 @@ To analyze memory issues (e.g., increase the heap size with Xmx) use a ```.sbtop
 ## TODO
 
  * [x] Use and document the PipeCon, direction from slave
- * [ ] Use a better name for the PipeCon interface (not io)
- * [ ] Wrapper for OCP (in Patmos)
+ * [x] Use a better name for the PipeCon interface (not io)
+ * [x] Wrapper for OCP (in Patmos)
  * [ ] Integrate a simple multicore device with T-CREST
-   * A multicore "Hello World"
+   * A multicore "Hello World" also for the handbook
  * [ ] Run S4NOC with T-CREST
  * [ ] Move (copy) the fair arbiter from the Chisel book into this repo
    * [ ] Write a test for the arbiter (or delegate it)
  * [ ] Use that arbiter for access to the serial port in T-CREST (using the ip-contributions version)
  * [ ] Do traffic generators like in the Dally book
  * [ ] Provide blocking in the CPU interface if not data available or TX FIFO full
-   * [x] DONE 
+   * [x] DONE
    * [ ] There is one in the Chisel book, compare them, maybe make them the same
  * [ ] Get one-way memory back in here
  * [ ] AXI wrapper

src/main/scala/s4noc/S4NoCTop.scala

@@ -21,7 +21,7 @@ class S4NoCTop(conf: Config) extends Module  {
   for (i <- 0 until conf.n) {
     val ci = Module(new PipeConS4NoC(conf.width, Entry(UInt(conf.width.W))))
     s4noc.io.networkPort(i) <> ci.rv
-    io.cpuPorts(i) <> ci.io.cpuPort
+    io.cpuPorts(i) <> ci.cpuPort
   }
 
   val cntReg = RegInit(0.U(32.W))

src/main/scala/s4noc/iotypes.scala renamed to src/main/scala/s4noc/ports.scala

@@ -4,8 +4,8 @@ import chisel3._
 
 
 /**
-  * Channel directions
-  */
+ * Channel directions
+ */
 object Const {
   val NORTH = 0
   val EAST = 1

src/main/scala/soc/HardwareLock.scala

@@ -18,18 +18,18 @@ class HardwareLock() extends PipeCon(2) {
   val lockReg = RegInit(false.B)
   val ackReg = RegInit(false.B)
   val readReg = RegInit(0.U)
-  ackReg := cp.wr || cp.rd
-  cp.ack := ackReg
+  ackReg := cpuPort.wr || cpuPort.rd
+  cpuPort.ack := ackReg
 
-  when (cp.rd) {
+  when (cpuPort.rd) {
     readReg := 0.U
     when (!lockReg) {
       lockReg := true.B
       readReg := 1.U
     }
   }
-  when (cp.wr) {
+  when (cpuPort.wr) {
     lockReg := false.B
   }
-  cp.rdData := readReg
+  cpuPort.rdData := readReg
 }

src/main/scala/soc/HelloDevice.scala

@@ -14,24 +14,24 @@ class HelloDevice(coreId: Int) extends PipeCon(3) {
   // TODO: following five lines are duplicated in PipeConRV, back to PipeCon class?
   val addrReg = RegInit(0.U(3.W))
   val ackReg = RegInit(false.B)
-  when (cp.rd) {
-    addrReg := cp.address
+  when (cpuPort.rd) {
+    addrReg := cpuPort.address
   }
-  cp.ack := ackReg
+  cpuPort.ack := ackReg
 
   val nr = coreId.U(32.W)
   val reg = RegInit(0.U(32.W))
-  when (cp.wr) {
-    reg := cp.wrData
+  when (cpuPort.wr) {
+    reg := cpuPort.wrData
   }
-  ackReg := cp.wr || cp.rd
-  cp.rdData := Mux(addrReg === 4.U, nr, reg)
+  ackReg := cpuPort.wr || cpuPort.rd
+  cpuPort.rdData := Mux(addrReg === 4.U, nr, reg)
 }
 
 class MultiCoreHello(nrCores: Int) extends MultiCoreDevice(nrCores, 3) {
 
   for(i <- 0 until nrCores) {
     val d = Module(new HelloDevice(i))
-    d.io.cpuPort <> ports(i)
+    d.cpuPort <> ports(i)
   }
 }

src/main/scala/soc/PipeCon.scala

@@ -7,10 +7,9 @@ import chisel3._
   *
   */
 abstract class PipeCon(addrWidth: Int) extends Module {
-  val io = IO(new Bundle {
-    val cpuPort = new PipeConIO(addrWidth)
-  })
-  val cp = io.cpuPort
+
+  val cpuPort = IO(new PipeConIO(addrWidth))
+
   assert(addrWidth >= 2, "Address width needs some size for byte addresses")
 }
 

src/main/scala/soc/PipeConRV.scala

@@ -13,6 +13,7 @@ import s4noc.Entry
   * 8: write receiver, read sender
   * TODO: compare with Chisel book version
   * TODO: make it generic and do a subtype for s4noc
+  * TODO: have some better tests
   */
 class PipeConRV[T <: Data](private val addrWidth: Int, private val dt: T, s4noc: Boolean = false) extends PipeCon(addrWidth) {
 
@@ -22,12 +23,21 @@ class PipeConRV[T <: Data](private val addrWidth: Int, private val dt: T, s4noc:
   val rx = rv.rx
 
   val ackReg = RegInit(false.B)
-  cp.ack := ackReg
+  ackReg := false.B
+  cpuPort.ack := ackReg
+  // just look at lower bits
+  val address = cpuPort.address(3, 0)
 
   val status = rx.valid ## tx.ready
 
-  // Two additional registers, used by S4NOC for addressing of nodes
-  val txDestReg = RegInit(0.U(8.W))
+  // This is ugly: two different transmit registers for two different types
+  // dt should be usable here...
+  val txDataReg = RegInit(0.U.asTypeOf(dt))
+  val e = Wire(Entry(UInt(32.W)))
+  e.data := 0.U
+  e.core := 0.U
+  val txS4NocReg = RegInit(e)
+  // Aditional register, used by S4NOC for addressing of nodes
   val rxSourceReg = RegInit(0.U(8.W))
   // TODO: detect Entry type
   // println("Type: " + dt.isInstanceOf[Entry[UInt(32.W)]])
@@ -36,42 +46,43 @@ class PipeConRV[T <: Data](private val addrWidth: Int, private val dt: T, s4noc:
   rx.ready := false.B
   tx.valid := false.B
   if (s4noc) {
-    val e = Wire(new Entry(UInt(32.W)))
-    e.data := cp.wrData
-    e.core := txDestReg
-    rv.tx.bits := e
-    cp.rdData := rv.rx.bits.asTypeOf(Entry(UInt(32.W))).data
+    cpuPort.rdData := rx.bits.asTypeOf(Entry(UInt(32.W))).data
+    tx.bits := txS4NocReg
   } else {
-    rv.tx.bits := cp.wrData
-    cp.rdData := rv.rx.bits
+    cpuPort.rdData := rx.bits
+    tx.bits := txDataReg
   }
 
   val idle :: readStatus :: readSource :: readWait :: writeWait :: Nil = Enum(5)
   val stateReg = RegInit(idle)
-  val wrDataReg = Reg(UInt(32.W))
 
   def idleReaction() = {
-    when (cp.wr) {
-      // printf("Write %d to %d\n", cp.wrData, cp.address)
-      when (cp.address === 8.U) {
-        txDestReg := cp.wrData
+    when (cpuPort.wr) {
+      // printf("Write %d to %d\n", cp.wrData, address)
+      when (address === 8.U) {
+        txS4NocReg.core := cpuPort.wrData
         ackReg := true.B
-      } .otherwise {
+      } .elsewhen (address === 4.U) {
+        /* lets not have this single cycle write for now
         tx.valid := true.B
         when (tx.ready) {
           ackReg := true.B
         } .otherwise {
-          wrDataReg := cp.wrData
+          txReg.data := cpuPort.wrData
           stateReg := writeWait
         }
+         */
+        txDataReg := cpuPort.wrData
+        txS4NocReg.data := cpuPort.wrData
+        stateReg := writeWait
       }
     }
-    when (cp.rd) {
-      // printf("Read from %d\n", cp.address)
-      when (cp.address === 0.U) {
+    when (cpuPort.rd) {
+      // printf("Read from %d\n", address)
+      when (address === 0.U) {
         stateReg := readStatus
         ackReg := true.B
-      } .elsewhen (cp.address === 8.U) {
+      } .elsewhen (address === 8.U) {
         stateReg := readSource
         ackReg := true.B
       } .otherwise {
@@ -80,19 +91,20 @@ class PipeConRV[T <: Data](private val addrWidth: Int, private val dt: T, s4noc:
     }
   }
 
-  ackReg := false.B
   switch(stateReg) {
     is (idle) {
       idleReaction()
     }
     is (readStatus) {
-      cp.rdData := status
+      cpuPort.rdData := status
       stateReg := idle
+      ackReg := false.B
       idleReaction()
     }
     is (readSource) {
-      cp.rdData := rxSourceReg
+      cpuPort.rdData := rxSourceReg
       stateReg := idle
+      ackReg := false.B
       idleReaction()
     }
     is (readWait) {
@@ -103,19 +115,14 @@ class PipeConRV[T <: Data](private val addrWidth: Int, private val dt: T, s4noc:
           rxSourceReg := rx.bits.asTypeOf(Entry(UInt(32.W))).core
         }
         // this is different from write - check
-        cp.ack := true.B
+        cpuPort.ack := true.B
+        ackReg := false.B
         idleReaction()
       }
     }
 
     is (writeWait) {
       tx.valid := true.B
-      if (s4noc) {
-        val e = tx.bits.asTypeOf(Entry(UInt(32.W)))
-        e.data := wrDataReg
-      } else {
-        tx.bits := wrDataReg
-      }
       when (tx.ready) {
         stateReg := idle
         ackReg := true.B

src/main/scala/wishbone/WBHardwareLock.scala

@@ -11,7 +11,7 @@ class WBHardwareLock() extends WishboneDevice(2) {
 
   val wrapper = Module(new Wrapper(2))
   val hello = Module(new HardwareLock())
-  wrapper.cpuIf.cpuPort <> hello.cp
+  wrapper.cpuIf.cpuPort <> hello.cpuPort
   io.port <> wrapper.io.port
 }
 

src/main/scala/wishbone/WrappedHello.scala

@@ -11,7 +11,7 @@ class WrappedHello() extends WishboneDevice(4) {
 
   val wrapper = Module(new Wrapper(4))
   val hello = Module(new HelloDevice(42))
-  wrapper.cpuIf.cpuPort <> hello.cp
+  wrapper.cpuIf.cpuPort <> hello.cpuPort
   io.port <> wrapper.io.port
 }
 

src/test/scala/soc/HelloTest.scala

@@ -14,7 +14,7 @@ class HelloTest extends AnyFlatSpec with ChiselScalatestTester {
 
         def step() = d.clock.step()
 
-        val cp = d.io.cpuPort
+        val cp = d.cpuPort
         cp.ack.expect(false.B)
 
         // set some default values
@@ -130,7 +130,7 @@ class HelloTest extends AnyFlatSpec with ChiselScalatestTester {
       dut => {
         def step() = dut.clock.step()
 
-        val helper = new MemoryMappedIOHelper(dut.cp, dut.clock)
+        val helper = new MemoryMappedIOHelper(dut.cpuPort, dut.clock)
 
         helper.setTimeOut(10)
         helper.read(4)