Device3: Fix block control flushing

- Factor out the _start_drain helper function, which starts flushing
data out of the block. We should always be disabling flow control after
we write to the flushing registers.
- Always attempt to flush when calling the _flush function. Previously,
we would check if data appeared to be moving when _flush was called,
and only write to the flush registers if data was moving. However, if
data is stuck for some reason (for example, the block ran out of flow
control credits), this check will give us a false positive, and we
won't flush. Instead, we need to always begin the flushing process,
then check those counters, and return once the counters stop changing.

Note: we need to start flushing before disabling flow control so that
the flushed data isn't flooded onto the crossbar.

Co-authored-by: michael-west <michael.west@ettus.com>
Co-authored-by: Sugandha Gupta <sugandha.gupta@ettus.com>

2 files changed, 28 insertions, 18 deletions

diff --git a/host/include/uhd/rfnoc/block_ctrl_base.hpp b/host/include/uhd/rfnoc/block_ctrl_base.hpp
index 0405ed8b7..b3c9c53d6 100644
--- a/host/include/uhd/rfnoc/block_ctrl_base.hpp
+++ b/host/include/uhd/rfnoc/block_ctrl_base.hpp
@@ -401,6 +401,9 @@ private:
     //! Helper to create a lambda to read tick rate
     double get_command_tick_rate(const size_t port);
 
+    //! Helper to start flushing for this block
+    void _start_drain(const size_t port = 0);
+
     //! Helper to flush any in-flight packets for this block
     bool _flush(const size_t port = 0);
 
diff --git a/host/lib/rfnoc/block_ctrl_base.cpp b/host/lib/rfnoc/block_ctrl_base.cpp
index ba4db7a61..dd89433a9 100644
--- a/host/lib/rfnoc/block_ctrl_base.cpp
+++ b/host/lib/rfnoc/block_ctrl_base.cpp
@@ -155,9 +155,7 @@ block_ctrl_base::~block_ctrl_base()
            // packets that are in flight, for now and until the setting is
            // disabled. This prevents long-running blocks without a tear-down
            // mechanism to gracefully flush.
-           const size_t port = get_ctrl_ports().front();
-           sr_write(SR_CLEAR_TX_FC, 0x2, port);    // Disconnect TX data-path
-           sr_write(SR_CLEAR_RX_FC, 0x2, port);    // Disconnect RX data-path
+           _start_drain(get_ctrl_ports().front());
        }
        _tree->remove(_root_path);
     )
@@ -589,9 +587,19 @@ stream_sig_t block_ctrl_base::_resolve_port_def(const blockdef::port_t &port_def
     return stream_sig;
 }
 
+void block_ctrl_base::_start_drain(const size_t port)
+{
+    // Begin flushing data out of the block by writing to the flushing
+    // registers, then disabling flow control. We do this because we don't know
+    // what state the flow-control module was left in in the previous run
+    sr_write(SR_CLEAR_TX_FC, 0x2, port);
+    sr_write(SR_CLEAR_RX_FC, 0x2, port);
+    sr_write(SR_FLOW_CTRL_EN, 0, port);
+}
+
 bool block_ctrl_base::_flush(const size_t port)
 {
-    UHD_LOG_DEBUG(unique_id(), "block_ctrl_base::_flush()");
+    UHD_LOG_TRACE(unique_id(), "block_ctrl_base::_flush (port=" << port << ")");
 
     auto is_data_streaming = [this](int time_ms) -> bool {
         // noc_shell has 2 16-bit counters (one for TX and one for RX) in the top
@@ -605,14 +613,9 @@ bool block_ctrl_base::_flush(const size_t port)
         return (new_cnts != old_cnts);
     };
 
-    // Initial check for activity
-    // We use a 10ms window to check for activity which detects a stream with approx
-    // 100 packets per second
-    constexpr int INITIAL_CHK_WINDOW_MS = 10;
-    if (not is_data_streaming(INITIAL_CHK_WINDOW_MS)) return true;
-
-    UHD_LOG_DEBUG(unique_id(), "block_ctrl_base::_flush(recovery mode)");
-    // We noticed streaming data. This is most likely because the last
+    // We always want to try flushing out data. This is done by starting to
+    // drain the data out of the block, then checking if counts have changed.
+    // If a change is detected, this is most likely because the last
     // session terminated abnormally or if logic in a noc_block is
     // misbehaving. This is a situation that we may not be able to
     // recover from because we are in a partially initialized state.
@@ -624,21 +627,25 @@ bool block_ctrl_base::_flush(const size_t port)
     // - This block which might be finishing up with its data output
     constexpr int FLUSH_TIMEOUT_MS = 2000;  // This is approximate
     bool success = false;
-    sr_write(SR_CLEAR_TX_FC, 0x2, port);    // Disconnect TX data-path
-    sr_write(SR_CLEAR_RX_FC, 0x2, port);    // Disconnect RX data-path
+    _start_drain(port);
     for (int i = 0; i < FLUSH_TIMEOUT_MS/10; i++) {
         if (not is_data_streaming(10)) {
             success = true;
             break;
         }
     }
+    // Stop flushing
     sr_write(SR_CLEAR_TX_FC, 0x0, port);    // Enable TX data-path
     sr_write(SR_CLEAR_RX_FC, 0x0, port);    // Enable RX data-path
 
-    UHD_LOGGER_WARNING(unique_id()) <<
-        "This block seems to be busy most likely due to the abnormal termination of a previous session. " <<
-        "Attempted recovery but it may not have worked depending on the behavior of other blocks in the design. " <<
-        "Please restart the application.";
+    if (not success) {
+        // Print a warning only if data was still flushing
+        // after the timeout elapsed
+        UHD_LOGGER_WARNING(unique_id()) <<
+            "This block seems to be busy most likely due to the abnormal termination of a previous "
+            "session. Attempted recovery but it may not have worked depending on the behavior of "
+            "other blocks in the design. Please restart the application.";
+    }
     return success;
 }