rfnoc: Add clock selection to blocks

During registration, blocks must now specify which clock they are using
for the timebase (i.e., for timed commands) and for the ctrlport (this
is used to determine the length of sleeps and polls). For example, the
X300 provides bus_clk and radio_clk; typically, the former is used for
the control port, and the latter for the timebase clock.
Another virtual clock is called "__graph__", and it means the clock is
derived from property propagation via the graph.

The actual clocks are provided by the mb_iface. It has two new API
calls: get_timebase_clock() and get_ctrlport_clock(), which take an
argument as to which clock exactly is requested. On block
initialization, those clock_iface objects are copied into the block
controller.

The get_tick_rate() API call for blocks now exclusively checks the
timebase clock_iface, and will no longer cache the current tick rate in
a separate _tick_rate member variable. Block controllers can't manually
modify the clock_iface, unless they also have access to the
mb_controller (like the radio block), and that mb_controller has
provided said access.

This commit also adds the clock selection API changes to the DDC block,
the Null block, and the default block.

14 files changed, 133 insertions, 63 deletions

diff --git a/host/include/uhd/rfnoc/noc_block_base.hpp b/host/include/uhd/rfnoc/noc_block_base.hpp
index 45a1f5738..d9659862c 100644
--- a/host/include/uhd/rfnoc/noc_block_base.hpp
+++ b/host/include/uhd/rfnoc/noc_block_base.hpp
@@ -100,7 +100,7 @@ public:
      *
      * Note there is only ever one time base (or tick rate) per block.
      */
-    double get_tick_rate() const { return _tick_rate; }
+    double get_tick_rate() const;
 
     /*! Return the arguments that were passed into this block from the framework
      */
@@ -215,11 +215,11 @@ private:
     // for all in- and output edges.
     std::vector<property_t<double>> _tick_rate_props;
 
-    //! The actual tick rate of the current time base
-    double _tick_rate;
+    //! Reference to the ctrlport clock_iface object shared with the register_iface
+    std::shared_ptr<clock_iface> _ctrlport_clock_iface;
 
-    //! Reference to the clock_iface object shared with the register_iface
-    std::shared_ptr<clock_iface> _clock_iface;
+    //! Reference to the timebase clock_iface object shared with the register_iface
+    std::shared_ptr<clock_iface> _tb_clock_iface;
 
     //! Stores a reference to this block's motherboard's controller, if this
     // block had requested and was granted access
diff --git a/host/include/uhd/rfnoc/noc_block_make_args.hpp b/host/include/uhd/rfnoc/noc_block_make_args.hpp
index 8a5fbd46f..7ed191079 100644
--- a/host/include/uhd/rfnoc/noc_block_make_args.hpp
+++ b/host/include/uhd/rfnoc/noc_block_make_args.hpp
@@ -40,8 +40,11 @@ struct noc_block_base::make_args_t
     //! Register interface to this block's register space
     register_iface::sptr reg_iface;
 
-    //! Clock interface object that is shared with the reg_iface
-    std::shared_ptr<clock_iface> clk_iface;
+    //! Timebase clock interface object that is shared with the reg_iface
+    std::shared_ptr<clock_iface> tb_clk_iface;
+
+    //! Controlport clock interface object that is shared with the reg_iface
+    std::shared_ptr<clock_iface> ctrlport_clk_iface;
 
     //! Reference to the motherboard controller associated with this block.
     //
diff --git a/host/include/uhd/rfnoc/registry.hpp b/host/include/uhd/rfnoc/registry.hpp
index cc07d5c65..9815ce3c6 100644
--- a/host/include/uhd/rfnoc/registry.hpp
+++ b/host/include/uhd/rfnoc/registry.hpp
@@ -15,16 +15,17 @@
 
 //! This macro must be placed inside a block implementation file
 // after the class definition
-#define UHD_RFNOC_BLOCK_REGISTER_DIRECT(CLASS_NAME, NOC_ID, BLOCK_NAME)   \
-    uhd::rfnoc::noc_block_base::sptr CLASS_NAME##_make(                   \
-        uhd::rfnoc::noc_block_base::make_args_ptr make_args)              \
-    {                                                                     \
-        return std::make_shared<CLASS_NAME##_impl>(std::move(make_args)); \
-    }                                                                     \
-    UHD_STATIC_BLOCK(register_rfnoc_##CLASS_NAME)                         \
-    {                                                                     \
-        uhd::rfnoc::registry::register_block_direct(                      \
-            NOC_ID, BLOCK_NAME, &CLASS_NAME##_make);                      \
+#define UHD_RFNOC_BLOCK_REGISTER_DIRECT(                                   \
+    CLASS_NAME, NOC_ID, BLOCK_NAME, TB_CLOCK, CTRL_CLOCK)                  \
+    uhd::rfnoc::noc_block_base::sptr CLASS_NAME##_make(                    \
+        uhd::rfnoc::noc_block_base::make_args_ptr make_args)               \
+    {                                                                      \
+        return std::make_shared<CLASS_NAME##_impl>(std::move(make_args));  \
+    }                                                                      \
+    UHD_STATIC_BLOCK(register_rfnoc_##CLASS_NAME)                          \
+    {                                                                      \
+        uhd::rfnoc::registry::register_block_direct(                       \
+            NOC_ID, BLOCK_NAME, TB_CLOCK, CTRL_CLOCK, &CLASS_NAME##_make); \
     }
 
 #define UHD_RFNOC_BLOCK_REQUEST_MB_ACCESS(NOC_ID)              \
@@ -62,6 +63,8 @@ public:
      */
     static void register_block_direct(noc_block_base::noc_id_t noc_id,
         const std::string& block_name,
+        const std::string& timebase_clock,
+        const std::string& ctrlport_clock,
         factory_t factory_fn);
 
     /*! Register a block that does use a block descriptor file
diff --git a/host/lib/include/uhdlib/rfnoc/clock_iface.hpp b/host/lib/include/uhdlib/rfnoc/clock_iface.hpp
index ae61a645e..06e54e67c 100644
--- a/host/lib/include/uhdlib/rfnoc/clock_iface.hpp
+++ b/host/lib/include/uhdlib/rfnoc/clock_iface.hpp
@@ -12,12 +12,15 @@
 #include <uhd/utils/log.hpp>
 #include <atomic>
 #include <string>
+#include <memory>
 
 namespace uhd { namespace rfnoc {
 
 class clock_iface
 {
 public:
+    using sptr = std::shared_ptr<clock_iface>;
+
     clock_iface(const std::string& name) : _name(name), _is_mutable(true)
     {
         _is_running = false;
diff --git a/host/lib/include/uhdlib/rfnoc/factory.hpp b/host/lib/include/uhdlib/rfnoc/factory.hpp
index 8d1fb27a0..be42a57e5 100644
--- a/host/lib/include/uhdlib/rfnoc/factory.hpp
+++ b/host/lib/include/uhdlib/rfnoc/factory.hpp
@@ -12,6 +12,14 @@
 
 namespace uhd { namespace rfnoc {
 
+struct block_factory_info_t
+{
+    std::string block_name;
+    std::string timebase_clk;
+    std::string ctrlport_clk;
+    registry::factory_t factory_fn;
+};
+
 /*! Container for factory functionality
  */
 class factory
@@ -19,11 +27,10 @@ class factory
 public:
     /*! Return a factory function for an RFNoC block based on the Noc-ID
      *
-     * \returns a pair: factory function, and block name
+     * \returns a block_factory_info_t object
      * \throws uhd::lookup_error if no block is found
      */
-    static std::pair<registry::factory_t, std::string>
-    get_block_factory(noc_block_base::noc_id_t noc_id);
+    static block_factory_info_t get_block_factory(noc_block_base::noc_id_t noc_id);
 
     /*! Check if this block has requested access to the motherboard controller
      */
diff --git a/host/lib/include/uhdlib/rfnoc/mb_iface.hpp b/host/lib/include/uhdlib/rfnoc/mb_iface.hpp
index cca8dcab8..0a2790a34 100644
--- a/host/lib/include/uhdlib/rfnoc/mb_iface.hpp
+++ b/host/lib/include/uhdlib/rfnoc/mb_iface.hpp
@@ -9,6 +9,7 @@
 
 #include <uhdlib/rfnoc/chdr_ctrl_xport.hpp>
 #include <uhdlib/rfnoc/rfnoc_common.hpp>
+#include <uhdlib/rfnoc/clock_iface.hpp>
 #include <memory>
 
 namespace uhd { namespace rfnoc {
@@ -67,6 +68,10 @@ public:
      */
     virtual void reset_network() = 0;
 
+    /*! Return a reference to a clock iface
+     */
+    virtual std::shared_ptr<clock_iface> get_clock_iface(const std::string& clock_name) = 0;
+
     /*! Create a control transport
      *
      * This is usually called once per motherboard, since there is only one
diff --git a/host/lib/rfnoc/block_control.cpp b/host/lib/rfnoc/block_control.cpp
index 78c390001..e6d5cd31d 100644
--- a/host/lib/rfnoc/block_control.cpp
+++ b/host/lib/rfnoc/block_control.cpp
@@ -21,4 +21,5 @@ public:
     }
 };
 
-UHD_RFNOC_BLOCK_REGISTER_DIRECT(block_control, DEFAULT_NOC_ID, DEFAULT_BLOCK_NAME)
+UHD_RFNOC_BLOCK_REGISTER_DIRECT(
+    block_control, DEFAULT_NOC_ID, DEFAULT_BLOCK_NAME, CLOCK_KEY_GRAPH, "bus_clk")
diff --git a/host/lib/rfnoc/client_zero.cpp b/host/lib/rfnoc/client_zero.cpp
index aaecdcbf5..6d0e47203 100644
--- a/host/lib/rfnoc/client_zero.cpp
+++ b/host/lib/rfnoc/client_zero.cpp
@@ -220,9 +220,10 @@ client_zero::sptr client_zero::make(chdr_ctrl_endpoint& chdr_ctrl_ep, sep_id_t d
     static constexpr uint16_t CLIENT_ZERO_PORT         = 0;
     static constexpr size_t CLIENT_ZERO_BUFF_CAPACITY  = 32;
     static constexpr size_t CLIENT_ZERO_MAX_ASYNC_MSGS = 0;
-    static clock_iface client_zero_clk{"client_zero"};
+    // This clock_iface doesn't really matter because client_zero doesn't do timed
+    // commands
+    static clock_iface client_zero_clk("client_zero", 100e6, true);
     client_zero_clk.set_running(true); // Client zero clock must be always-on.
-    client_zero_clk.set_freq(100e6); // The freq is unused. No timed ops or sleeps.
 
     return std::make_shared<client_zero>(chdr_ctrl_ep.get_ctrlport_ep(dst_epid,
         CLIENT_ZERO_PORT,
diff --git a/host/lib/rfnoc/ddc_block_control.cpp b/host/lib/rfnoc/ddc_block_control.cpp
index 0b6fd7d89..56bb8cc5a 100644
--- a/host/lib/rfnoc/ddc_block_control.cpp
+++ b/host/lib/rfnoc/ddc_block_control.cpp
@@ -516,4 +516,5 @@ private:
     std::vector<property_t<double>> _freq;
 };
 
-UHD_RFNOC_BLOCK_REGISTER_DIRECT(ddc_block_control, 0xDDC00000, "DDC")
+UHD_RFNOC_BLOCK_REGISTER_DIRECT(
+    ddc_block_control, 0xDDC00000, "DDC", CLOCK_KEY_GRAPH, "bus_clk")
diff --git a/host/lib/rfnoc/noc_block_base.cpp b/host/lib/rfnoc/noc_block_base.cpp
index c0d8cf2a9..68093d9b1 100644
--- a/host/lib/rfnoc/noc_block_base.cpp
+++ b/host/lib/rfnoc/noc_block_base.cpp
@@ -23,21 +23,28 @@ noc_block_base::noc_block_base(make_args_ptr make_args)
     , _block_id(make_args->block_id)
     , _num_input_ports(make_args->num_input_ports)
     , _num_output_ports(make_args->num_output_ports)
-    , _clock_iface(make_args->clk_iface)
+    , _ctrlport_clock_iface(make_args->ctrlport_clk_iface)
+    , _tb_clock_iface(make_args->tb_clk_iface)
     , _mb_controller(std::move(make_args->mb_control))
     , _block_args(make_args->args)
     , _tree(make_args->tree)
 {
+    RFNOC_LOG_TRACE(
+        "Using timebase clock: `" << _tb_clock_iface->get_name() << "'. Current frequency: "
+                                 << (_tb_clock_iface->get_freq() / 1e6) << " MHz");
+    RFNOC_LOG_TRACE("Using ctrlport clock: `"
+                    << _ctrlport_clock_iface->get_name() << "'. Current frequency: "
+                    << (_ctrlport_clock_iface->get_freq() / 1e6) << " MHz");
     // First, create one tick_rate property for every port
     _tick_rate_props.reserve(get_num_input_ports() + get_num_output_ports());
     for (size_t input_port = 0; input_port < get_num_input_ports(); input_port++) {
         _tick_rate_props.push_back(property_t<double>(PROP_KEY_TICK_RATE,
-            DEFAULT_TICK_RATE,
+            _tb_clock_iface->get_freq(),
             {res_source_info::INPUT_EDGE, input_port}));
     }
     for (size_t output_port = 0; output_port < get_num_output_ports(); output_port++) {
         _tick_rate_props.push_back(property_t<double>(PROP_KEY_TICK_RATE,
-            DEFAULT_TICK_RATE,
+            _tb_clock_iface->get_freq(),
             {res_source_info::OUTPUT_EDGE, output_port}));
     }
     // Register all the tick_rate properties and create a default resolver
@@ -51,14 +58,16 @@ noc_block_base::noc_block_base(make_args_ptr make_args)
         auto prop_refs_copy = prop_refs;
         add_property_resolver(
             {&prop}, std::move(prop_refs_copy), [this, source_prop = &prop]() {
+                // _set_tick_rate() will update _tick_rate, but only if that's
+                // a valid operation for this block
+                this->_set_tick_rate(source_prop->get());
+                // Now, _tick_rate is valid and we will pass its value to all
+                // tick_rate properties
                 for (property_t<double>& tick_rate_prop : _tick_rate_props) {
-                    tick_rate_prop = source_prop->get();
+                    tick_rate_prop = get_tick_rate();
                 }
-                this->_set_tick_rate(source_prop->get());
             });
     }
-    // Now enable this clock iface
-    _clock_iface->set_running(true);
 }
 
 noc_block_base::~noc_block_base()
@@ -89,13 +98,19 @@ void noc_block_base::set_num_output_ports(const size_t num_ports)
 }
 
 
+double noc_block_base::get_tick_rate() const
+{
+    return _tb_clock_iface->get_freq();
+}
+
 void noc_block_base::set_tick_rate(const double tick_rate)
 {
-    if (_tick_rate == tick_rate) {
+    if (tick_rate == get_tick_rate()) {
         return;
     }
     // Update this node
-    _set_tick_rate(tick_rate);
+    RFNOC_LOG_TRACE("Setting tb clock freq to " << tick_rate/1e6 << " MHz");
+    _tb_clock_iface->set_freq(tick_rate);
     // Now trigger property propagation
     if (!_tick_rate_props.empty()) {
         auto src_info = _tick_rate_props.at(0).get_src_info();
@@ -105,12 +120,21 @@ void noc_block_base::set_tick_rate(const double tick_rate)
 
 void noc_block_base::_set_tick_rate(const double tick_rate)
 {
-    if (tick_rate == _tick_rate) {
+    if (tick_rate == get_tick_rate()) {
+        return;
+    }
+    if (tick_rate <= 0) {
+        RFNOC_LOG_WARNING("Attempting to set tick rate to 0. Skipping.")
         return;
     }
-    RFNOC_LOG_TRACE("Updating tick rate to " << (tick_rate / 1e6) << " MHz");
-    _clock_iface->set_freq(tick_rate);
-    _tick_rate = tick_rate;
+    if (_tb_clock_iface->get_name() == CLOCK_KEY_GRAPH) {
+        RFNOC_LOG_TRACE("Updating tick rate to " << (tick_rate / 1e6) << " MHz");
+        _tb_clock_iface->set_freq(tick_rate);
+    } else {
+        RFNOC_LOG_WARNING("Cannot change tick rate to "
+                          << (tick_rate / 1e6)
+                          << " MHz, this clock is not configurable by the graph!");
+    }
 }
 
 void noc_block_base::shutdown()
diff --git a/host/lib/rfnoc/null_block_control.cpp b/host/lib/rfnoc/null_block_control.cpp
index 21042453d..443c2f3d7 100644
--- a/host/lib/rfnoc/null_block_control.cpp
+++ b/host/lib/rfnoc/null_block_control.cpp
@@ -181,4 +181,5 @@ private:
     uint32_t _item_width;
 };
 
-UHD_RFNOC_BLOCK_REGISTER_DIRECT(null_block_control, 0x00000001, "NullSrcSink")
+UHD_RFNOC_BLOCK_REGISTER_DIRECT(
+    null_block_control, 0x00000001, "NullSrcSink", CLOCK_KEY_GRAPH, "bus_clk")
diff --git a/host/lib/rfnoc/registry_factory.cpp b/host/lib/rfnoc/registry_factory.cpp
index d03cb183a..117b60e96 100644
--- a/host/lib/rfnoc/registry_factory.cpp
+++ b/host/lib/rfnoc/registry_factory.cpp
@@ -24,9 +24,8 @@ using namespace uhd::rfnoc;
 //   descriptor file
 //
 // This is the direct registry:
-using block_direct_reg_t = std::unordered_map<noc_block_base::noc_id_t,
-    std::tuple<std::string /* block_name */,
-        registry::factory_t>>;
+using block_direct_reg_t =
+    std::unordered_map<noc_block_base::noc_id_t, block_factory_info_t>;
 UHD_SINGLETON_FCN(block_direct_reg_t, get_direct_block_registry);
 //
 // This is the descriptor registry:
@@ -54,6 +53,8 @@ UHD_SINGLETON_FCN(
  *****************************************************************************/
 void registry::register_block_direct(noc_block_base::noc_id_t noc_id,
     const std::string& block_name,
+    const std::string& timebase_clock,
+    const std::string& ctrlport_clock,
     factory_t factory_fn)
 {
     if (get_direct_block_registry().count(noc_id)) {
@@ -63,8 +64,9 @@ void registry::register_block_direct(noc_block_base::noc_id_t noc_id,
             << std::hex << noc_id << std::dec << std::endl;
         return;
     }
-    get_direct_block_registry().emplace(
-        noc_id, std::make_tuple(block_name, std::move(factory_fn)));
+    get_direct_block_registry().emplace(noc_id,
+        block_factory_info_t{
+            block_name, timebase_clock, ctrlport_clock, std::move(factory_fn)});
 }
 
 void registry::register_block_descriptor(
@@ -96,8 +98,7 @@ void registry::request_mb_access(const std::string& block_key)
 /******************************************************************************
  * Factory functions
  *****************************************************************************/
-std::pair<registry::factory_t, std::string> factory::get_block_factory(
-    noc_block_base::noc_id_t noc_id)
+block_factory_info_t factory::get_block_factory(noc_block_base::noc_id_t noc_id)
 {
     // First, check the descriptor registry
     // FIXME TODO
@@ -109,8 +110,7 @@ std::pair<registry::factory_t, std::string> factory::get_block_factory(
                 << std::hex << std::setw(sizeof(noc_block_base::noc_id_t) * 2) << noc_id);
         noc_id = DEFAULT_NOC_ID;
     }
-    auto& block_info = get_direct_block_registry().at(noc_id);
-    return {std::get<1>(block_info), std::get<0>(block_info)};
+    return get_direct_block_registry().at(noc_id);
 }
 
 bool factory::has_requested_mb_access(noc_block_base::noc_id_t noc_id)
diff --git a/host/lib/rfnoc/rfnoc_graph.cpp b/host/lib/rfnoc/rfnoc_graph.cpp
index 5b2277284..04b13a374 100644
--- a/host/lib/rfnoc/rfnoc_graph.cpp
+++ b/host/lib/rfnoc/rfnoc_graph.cpp
@@ -184,27 +184,47 @@ private:
             // Iterate through and register each of the blocks in this mboard
             for (size_t portno = 0; portno < num_blocks; ++portno) {
                 auto noc_id = mb_cz->get_noc_id(portno + first_block_port);
-                auto block_factory_pair = factory::get_block_factory(noc_id);
+                auto block_factory_info = factory::get_block_factory(noc_id);
                 auto block_info = mb_cz->get_block_info(portno + first_block_port);
                 block_id_t block_id(mb_idx,
-                    block_factory_pair.second,
-                    block_count_map[block_factory_pair.second]++);
-                auto clk_iface = std::make_shared<clock_iface>(block_id.to_string() + "_clock");
+                    block_factory_info.block_name,
+                    block_count_map[block_factory_info.block_name]++);
+                // Get access to the clock interface objects. We have some rules
+                // here:
+                // - The ctrlport clock must always be provided through the
+                //   BSP via mb_iface
+                // - The timebase clock can be set to "graph", which means the
+                //   block takes care of the timebase itself (via property
+                //   propagation). In that case, we generate a clock iface
+                //   object on the fly here.
+                // - In all other cases, the BSP must provide us that clock
+                //   iface object through the mb_iface
+                auto ctrlport_clk_iface =
+                    mb.get_clock_iface(block_factory_info.ctrlport_clk);
+                auto tb_clk_iface = (block_factory_info.timebase_clk == CLOCK_KEY_GRAPH) ?
+                    std::make_shared<clock_iface>(CLOCK_KEY_GRAPH) :
+                    mb.get_clock_iface(block_factory_info.timebase_clk);
+                // A "graph" clock is always "running"
+                if (block_factory_info.timebase_clk == CLOCK_KEY_GRAPH) {
+                    tb_clk_iface->set_running(true);
+                }
                 auto block_reg_iface = _gsm->get_block_register_iface(ctrl_sep_addr,
                     portno,
-                    *clk_iface.get(),
-                    *clk_iface.get());
+                    *ctrlport_clk_iface.get(),
+                    *tb_clk_iface.get());
                 auto make_args_uptr = std::make_unique<noc_block_base::make_args_t>();
                 make_args_uptr->noc_id = noc_id;
                 make_args_uptr->block_id = block_id;
                 make_args_uptr->num_input_ports = block_info.num_inputs;
                 make_args_uptr->num_output_ports = block_info.num_outputs;
                 make_args_uptr->reg_iface = block_reg_iface;
-                make_args_uptr->clk_iface = clk_iface;
+                make_args_uptr->tb_clk_iface = tb_clk_iface;
+                make_args_uptr->ctrlport_clk_iface = ctrlport_clk_iface;
                 make_args_uptr->mb_control = (factory::has_requested_mb_access(noc_id) ? _mb_controllers.at(mb_idx) : nullptr);
                 make_args_uptr->tree = _tree->subtree("/mboards/0"); /* FIXME Get the block's subtree */
                 make_args_uptr->args = dev_addr; // TODO filter the device args
-                _block_registry->register_block(block_factory_pair.first(std::move(make_args_uptr)));
+                _block_registry->register_block(
+                    block_factory_info.factory_fn(std::move(make_args_uptr)));
                 _xbar_block_config[block_id.to_string()] = {
                     portno, noc_id, block_id.get_block_count()};
             }
diff --git a/host/tests/rfnoc_blocks_test.cpp b/host/tests/rfnoc_blocks_test.cpp
index b69b6f9f8..13fc15848 100644
--- a/host/tests/rfnoc_blocks_test.cpp
+++ b/host/tests/rfnoc_blocks_test.cpp
@@ -24,14 +24,15 @@ noc_block_base::make_args_ptr make_make_args(noc_block_base::noc_id_t noc_id,
     const size_t n_inputs,
     const size_t n_outputs)
 {
-    auto make_args              = std::make_unique<noc_block_base::make_args_t>();
-    make_args->noc_id           = noc_id;
-    make_args->num_input_ports  = n_inputs;
-    make_args->num_output_ports = n_outputs;
-    make_args->reg_iface        = std::make_shared<mock_reg_iface_t>();
-    make_args->block_id         = block_id;
-    make_args->clk_iface        = std::make_shared<clock_iface>("MOCK_CLOCK");
-    make_args->tree             = uhd::property_tree::make();
+    auto make_args                = std::make_unique<noc_block_base::make_args_t>();
+    make_args->noc_id             = noc_id;
+    make_args->num_input_ports    = n_inputs;
+    make_args->num_output_ports   = n_outputs;
+    make_args->reg_iface          = std::make_shared<mock_reg_iface_t>();
+    make_args->block_id           = block_id;
+    make_args->ctrlport_clk_iface = std::make_shared<clock_iface>("MOCK_CLOCK");
+    make_args->tb_clk_iface       = std::make_shared<clock_iface>("MOCK_CLOCK");
+    make_args->tree               = uhd::property_tree::make();
     return make_args;
 }