From 542a3753053f418e7adb3700efb257228e058712 Mon Sep 17 00:00:00 2001
From: Sebastien Bourdeauducq <sb@m-labs.hk>
Date: Thu, 10 Mar 2016 09:47:29 +0800
Subject: [PATCH] rtio: remove NOP suppression capability

Back when RTIO was driving TTLs, this functionality made it simpler to use by removing some irrelevant underflows.

The same technique is not applicable to DDS and SPI, so the user will have to deal with such underflows.

This patch makes the behavior of RTIO more consistent and the code simpler.
---
 artiq/gateware/rtio/core.py           | 29 +++++----------------------
 artiq/gateware/rtio/phy/ttl_simple.py |  3 +--
 artiq/gateware/rtio/phy/wishbone.py   |  1 -
 artiq/gateware/rtio/rtlink.py         |  6 ++----
 4 files changed, 8 insertions(+), 31 deletions(-)

diff --git a/artiq/gateware/rtio/core.py b/artiq/gateware/rtio/core.py
index d1095137b..ed3f5138a 100644
--- a/artiq/gateware/rtio/core.py
+++ b/artiq/gateware/rtio/core.py
@@ -95,7 +95,7 @@ class _OutputManager(Module):
             ev_layout.append(("address", address_width))
         ev_layout.append(("timestamp", counter.width + fine_ts_width))
         # ev must be valid 1 cycle before we to account for the latency in
-        # generating replace, sequence_error and nop
+        # generating replace, sequence_error and collision
         self.ev = Record(ev_layout)
 
         self.writable = Signal()
@@ -128,7 +128,6 @@ class _OutputManager(Module):
         sequence_error = Signal()
         collision = Signal()
         any_error = Signal()
-        nop = Signal()
         if interface.enable_replace:
             # Note: replace may be asserted at the same time as collision
             # when addresses are different. In that case, it is a collision.
@@ -151,25 +150,8 @@ class _OutputManager(Module):
         else:
             self.sync.rsys += collision.eq(
                 self.ev.timestamp[fine_ts_width:] == buf.timestamp[fine_ts_width:])
-        self.comb += any_error.eq(sequence_error | collision)
-        if interface.suppress_nop:
-            # disable NOP at reset: do not suppress a first write with all 0s
-            nop_en = Signal(reset=0)
-            addresses_equal = [getattr(self.ev, a) == getattr(buf, a)
-                               for a in ("data", "address")
-                               if hasattr(self.ev, a)]
-            if addresses_equal:
-                self.sync.rsys += nop.eq(
-                    nop_en & reduce(and_, addresses_equal))
-            else:
-                self.comb.eq(nop.eq(0))
-            self.sync.rsys += [
-                # buf now contains valid data. enable NOP.
-                If(self.we & ~any_error, nop_en.eq(1)),
-                # underflows cancel the write. allow it to be retried.
-                If(self.underflow, nop_en.eq(0))
-            ]
         self.comb += [
+            any_error.eq(sequence_error | collision),
             self.sequence_error.eq(self.we & sequence_error),
             self.collision.eq(self.we & collision)
         ]
@@ -190,7 +172,7 @@ class _OutputManager(Module):
                         fifo.we.eq(1)
                     )
                 ),
-                If(self.we & ~replace & ~nop & ~any_error,
+                If(self.we & ~replace & ~any_error,
                    fifo.we.eq(1)
                 )
             )
@@ -199,7 +181,7 @@ class _OutputManager(Module):
         # Must come after read to handle concurrent read+write properly
         self.sync.rsys += [
             buf_just_written.eq(0),
-            If(self.we & ~nop & ~any_error,
+            If(self.we & ~any_error,
                 buf_just_written.eq(1),
                 buf_pending.eq(1),
                 buf.eq(self.ev)
@@ -321,8 +303,7 @@ class Channel:
 class LogChannel:
     """A degenerate channel used to log messages into the analyzer."""
     def __init__(self):
-        self.interface = rtlink.Interface(
-            rtlink.OInterface(32, suppress_nop=False))
+        self.interface = rtlink.Interface(rtlink.OInterface(32))
         self.probes = []
         self.overrides = []
 
diff --git a/artiq/gateware/rtio/phy/ttl_simple.py b/artiq/gateware/rtio/phy/ttl_simple.py
index 1cc55ac48..a640cde5b 100644
--- a/artiq/gateware/rtio/phy/ttl_simple.py
+++ b/artiq/gateware/rtio/phy/ttl_simple.py
@@ -79,8 +79,7 @@ class Inout(Module):
 
 class ClockGen(Module):
     def __init__(self, pad, ftw_width=24):
-        self.rtlink = rtlink.Interface(
-            rtlink.OInterface(ftw_width, suppress_nop=False))
+        self.rtlink = rtlink.Interface(rtlink.OInterface(ftw_width))
 
         # # #
 
diff --git a/artiq/gateware/rtio/phy/wishbone.py b/artiq/gateware/rtio/phy/wishbone.py
index bff70ef84..d8fa752c2 100644
--- a/artiq/gateware/rtio/phy/wishbone.py
+++ b/artiq/gateware/rtio/phy/wishbone.py
@@ -13,7 +13,6 @@ class RT2WB(Module):
             rtlink.OInterface(
                 len(wb.dat_w),
                 address_width + 1,
-                suppress_nop=False,
                 enable_replace=rtio_enable_replace),
             rtlink.IInterface(
                 len(wb.dat_r),
diff --git a/artiq/gateware/rtio/rtlink.py b/artiq/gateware/rtio/rtlink.py
index b8bda7aac..d52e38ad3 100644
--- a/artiq/gateware/rtio/rtlink.py
+++ b/artiq/gateware/rtio/rtlink.py
@@ -3,8 +3,7 @@ from migen import *
 
 class OInterface:
     def __init__(self, data_width, address_width=0,
-                 fine_ts_width=0, suppress_nop=True,
-                 enable_replace=True):
+                 fine_ts_width=0, enable_replace=True):
         self.stb = Signal()
         self.busy = Signal()
 
@@ -15,7 +14,6 @@ class OInterface:
         if fine_ts_width:
             self.fine_ts = Signal(fine_ts_width)
 
-        self.suppress_nop = suppress_nop
         self.enable_replace = enable_replace
 
     @classmethod
@@ -23,7 +21,7 @@ class OInterface:
         return cls(get_data_width(other),
                    get_address_width(other),
                    get_fine_ts_width(other),
-                   other.suppress_nop)
+                   other.enable_replace)
 
 
 class IInterface: