From 2e80d2310fd8a28c2a9812bfb7f88b6684543f0b Mon Sep 17 00:00:00 2001 From: Andreas Kurth Date: Tue, 20 Apr 2021 11:01:27 +0200 Subject: [PATCH 1/7] Increment development version towards next minor release --- VERSION | 2 +- axi.core | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/VERSION b/VERSION index 808150e88..d870653a9 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.28.1-dev +0.29.0-dev diff --git a/axi.core b/axi.core index 9f7a5f08f..f076292bc 100644 --- a/axi.core +++ b/axi.core @@ -1,6 +1,6 @@ CAPI=2: -name : pulp-platform.org::axi:0.28.1-dev +name : pulp-platform.org::axi:0.29.0-dev filesets: rtl: From 9728a0ba4b23e52a220b6c4fbf2023327d5d9817 Mon Sep 17 00:00:00 2001 From: Andreas Kurth Date: Tue, 20 Apr 2021 10:34:49 +0200 Subject: [PATCH 2/7] axi_demux: Add `UniqueIds` parameter --- doc/axi_demux.md | 11 +++++ src/axi_demux.sv | 103 +++++++++++++++++++++++++++++------------------ 2 files changed, 75 insertions(+), 39 deletions(-) diff --git a/doc/axi_demux.md b/doc/axi_demux.md index 267a4d93e..3d67a2bbe 100644 --- a/doc/axi_demux.md +++ b/doc/axi_demux.md @@ -26,6 +26,7 @@ This demultiplexer is configured through the parameters listed in the following | `NoMstPorts` | `int unsigned` | The number of AXI master ports of the demultiplexer (in other words, how many AXI slave modules can be attached). | | `MaxTrans` | `int unsigned` | The slave port can have at most this many transactions [in flight](../doc#in-flight). | | `AxiLookBits` | `int unsigned` | The number of ID bits (starting at the least significant) the demultiplexer uses to determine the uniqueness of an AXI ID (see section *Ordering and Stalls* below). This value has to be less or equal than `AxiIdWidth`. | +| `UniqueIds` | `bit` | If you can guarantee that the ID of each transaction is always unique among all in-flight transactions in the same direction, setting this parameter to `1'b1` simplifies the demultiplexer (see section *Ordering and Stalls* below). Defaults to `1'b0`. | | `FallThrough` | `bit` | Routing decisions on the AW channel fall through to the W channel. Enabling this allows the demultiplexer to accept a W beat in the same cycle as the corresponding AW beat, but it increases the combinatorial path of the W channel with logic from `slv_aw_select_i`. | | `SpillXX` | `bit` | Inserts one spill register on the respective channel (AW, W, B, AR, and R) before the demultiplexer. | @@ -56,6 +57,16 @@ When the demultiplexer receives two transactions with the same ID and direction The reason for this behavior are AXI ordering constraints, see the [documentation of the crossbar](axi_xbar.md#ordering-and-stalls) for details. +There are use cases that do not require the demultiplexer to keep track of and enforce this ordering, and the `UniqueIds` parameter can be set to specialize the demultiplexer for these cases: +`UniqueIds` may be set to `1'b1` if and only if +- each transaction has an ID that is unique among all in-flight transactions in the same direction; +- or for any ID, all transactions with that ID target the same master port as all other transactions with the same ID and direction; +- or both. + +Setting the `UniqueIds` parameter to `1'b1` when those conditions are not always met leads to undefined behavior. + +Setting the `UniqueIds` parameter to `1'b1` reduces the area complexity of the demultiplexer from `O(2^I)` to `O(I)`, where `I` is the ID width. + ### Implementation `2 * 2^AxiLookBits` counters track the number of [in-flight](../doc#in-flight) transactions. That is, for each ID in the (potentially) reduced set of IDs of `AxiLookBits` bits, there is one counter for write transactions and one for read transactions. Each counter can count up to (and including) `MaxTrans`, and there is a register that holds the index of the master port to which a counter is assigned. diff --git a/src/axi_demux.sv b/src/axi_demux.sv index bd2c79bd7..270ee05d9 100644 --- a/src/axi_demux.sv +++ b/src/axi_demux.sv @@ -28,6 +28,7 @@ module axi_demux #( parameter int unsigned NoMstPorts = 32'd0, parameter int unsigned MaxTrans = 32'd8, parameter int unsigned AxiLookBits = 32'd3, + parameter bit UniqueIds = 1'b0, parameter bit FallThrough = 1'b0, parameter bit SpillAw = 1'b1, parameter bit SpillW = 1'b0, @@ -223,26 +224,37 @@ module axi_demux #( // prevent further pushing `FFLARN(lock_aw_valid_q, lock_aw_valid_d, load_aw_lock, '0, clk_i, rst_ni) - axi_demux_id_counters #( - .AxiIdBits ( AxiLookBits ), - .CounterWidth ( IdCounterWidth ), - .mst_port_select_t ( select_t ) - ) i_aw_id_counter ( - .clk_i ( clk_i ), - .rst_ni ( rst_ni ), - .lookup_axi_id_i ( slv_aw_chan_select.aw_chan.id[0+:AxiLookBits] ), - .lookup_mst_select_o ( lookup_aw_select ), - .lookup_mst_select_occupied_o ( aw_select_occupied ), - .full_o ( aw_id_cnt_full ), - .inject_axi_id_i ( '0 ), - .inject_i ( 1'b0 ), - .push_axi_id_i ( slv_aw_chan_select.aw_chan.id[0+:AxiLookBits] ), - .push_mst_select_i ( slv_aw_chan_select.aw_select ), - .push_i ( aw_push ), - .pop_axi_id_i ( slv_b_chan.id[0+:AxiLookBits] ), - .pop_i ( slv_b_valid & slv_b_ready ) - ); - // pop from ID counter on outward transaction + if (UniqueIds) begin : gen_unique_ids_aw + // If the `UniqueIds` parameter is set, each write transaction has an ID that is unique among + // all in-flight write transactions, or all write transactions with a given ID target the same + // master port as all write transactions with the same ID, or both. This means that the + // signals that are driven by the ID counters if this parameter is not set can instead be + // derived from existing signals. The ID counters can therefore be omitted. + assign lookup_aw_select = slv_aw_chan_select.aw_select; + assign aw_select_occupied = 1'b0; + assign aw_id_cnt_full = 1'b0; + end else begin : gen_aw_id_counter + axi_demux_id_counters #( + .AxiIdBits ( AxiLookBits ), + .CounterWidth ( IdCounterWidth ), + .mst_port_select_t ( select_t ) + ) i_aw_id_counter ( + .clk_i ( clk_i ), + .rst_ni ( rst_ni ), + .lookup_axi_id_i ( slv_aw_chan_select.aw_chan.id[0+:AxiLookBits] ), + .lookup_mst_select_o ( lookup_aw_select ), + .lookup_mst_select_occupied_o ( aw_select_occupied ), + .full_o ( aw_id_cnt_full ), + .inject_axi_id_i ( '0 ), + .inject_i ( 1'b0 ), + .push_axi_id_i ( slv_aw_chan_select.aw_chan.id[0+:AxiLookBits] ), + .push_mst_select_i ( slv_aw_chan_select.aw_select ), + .push_i ( aw_push ), + .pop_axi_id_i ( slv_b_chan.id[0+:AxiLookBits] ), + .pop_i ( slv_b_valid & slv_b_ready ) + ); + // pop from ID counter on outward transaction + end // FIFO to save W selection fifo_v3 #( @@ -391,25 +403,36 @@ module axi_demux #( // this ff is needed so that ar does not get de-asserted if an atop gets injected `FFLARN(lock_ar_valid_q, lock_ar_valid_d, load_ar_lock, '0, clk_i, rst_ni) - axi_demux_id_counters #( - .AxiIdBits ( AxiLookBits ), - .CounterWidth ( IdCounterWidth ), - .mst_port_select_t ( select_t ) - ) i_ar_id_counter ( - .clk_i ( clk_i ), - .rst_ni ( rst_ni ), - .lookup_axi_id_i ( slv_ar_chan_select.ar_chan.id[0+:AxiLookBits] ), - .lookup_mst_select_o ( lookup_ar_select ), - .lookup_mst_select_occupied_o ( ar_select_occupied ), - .full_o ( ar_id_cnt_full ), - .inject_axi_id_i ( slv_aw_chan_select.aw_chan.id[0+:AxiLookBits] ), - .inject_i ( atop_inject ), - .push_axi_id_i ( slv_ar_chan_select.ar_chan.id[0+:AxiLookBits] ), - .push_mst_select_i ( slv_ar_chan_select.ar_select ), - .push_i ( ar_push ), - .pop_axi_id_i ( slv_r_chan.id[0+:AxiLookBits] ), - .pop_i ( slv_r_valid & slv_r_ready & slv_r_chan.last ) - ); + if (UniqueIds) begin : gen_unique_ids_ar + // If the `UniqueIds` parameter is set, each read transaction has an ID that is unique among + // all in-flight read transactions, or all read transactions with a given ID target the same + // master port as all read transactions with the same ID, or both. This means that the + // signals that are driven by the ID counters if this parameter is not set can instead be + // derived from existing signals. The ID counters can therefore be omitted. + assign lookup_ar_select = slv_ar_chan_select.ar_select; + assign ar_select_occupied = 1'b0; + assign ar_id_cnt_full = 1'b0; + end else begin : gen_ar_id_counter + axi_demux_id_counters #( + .AxiIdBits ( AxiLookBits ), + .CounterWidth ( IdCounterWidth ), + .mst_port_select_t ( select_t ) + ) i_ar_id_counter ( + .clk_i ( clk_i ), + .rst_ni ( rst_ni ), + .lookup_axi_id_i ( slv_ar_chan_select.ar_chan.id[0+:AxiLookBits] ), + .lookup_mst_select_o ( lookup_ar_select ), + .lookup_mst_select_occupied_o ( ar_select_occupied ), + .full_o ( ar_id_cnt_full ), + .inject_axi_id_i ( slv_aw_chan_select.aw_chan.id[0+:AxiLookBits] ), + .inject_i ( atop_inject ), + .push_axi_id_i ( slv_ar_chan_select.ar_chan.id[0+:AxiLookBits] ), + .push_mst_select_i ( slv_ar_chan_select.ar_select ), + .push_i ( ar_push ), + .pop_axi_id_i ( slv_r_chan.id[0+:AxiLookBits] ), + .pop_i ( slv_r_valid & slv_r_ready & slv_r_chan.last ) + ); + end //-------------------------------------- // R Channel @@ -674,6 +697,7 @@ module axi_demux_intf #( parameter int unsigned NO_MST_PORTS = 32'd3, parameter int unsigned MAX_TRANS = 32'd8, parameter int unsigned AXI_LOOK_BITS = 32'd3, + parameter bit UNIQUE_IDS = 1'b0, parameter bit FALL_THROUGH = 1'b0, parameter bit SPILL_AW = 1'b1, parameter bit SPILL_W = 1'b0, @@ -731,6 +755,7 @@ module axi_demux_intf #( .NoMstPorts ( NO_MST_PORTS ), .MaxTrans ( MAX_TRANS ), .AxiLookBits ( AXI_LOOK_BITS ), + .UniqueIds ( UNIQUE_IDS ), .FallThrough ( FALL_THROUGH ), .SpillAw ( SPILL_AW ), .SpillW ( SPILL_W ), From 92dda754361eb8ec253a8e8cbe72bff7509c5f4b Mon Sep 17 00:00:00 2001 From: Andreas Kurth Date: Tue, 20 Apr 2021 10:40:18 +0200 Subject: [PATCH 3/7] axi_xbar: Add `UniqueIds` parameter --- doc/axi_xbar.md | 1 + scripts/axi_intercon_gen.py | 1 + src/axi_pkg.sv | 1 + src/axi_xbar.sv | 1 + test/tb_axi_xbar.sv | 1 + 5 files changed, 5 insertions(+) diff --git a/doc/axi_xbar.md b/doc/axi_xbar.md index 98770c533..ca0c12108 100644 --- a/doc/axi_xbar.md +++ b/doc/axi_xbar.md @@ -49,6 +49,7 @@ The crossbar is configured through the `Cfg` parameter with a `axi_pkg::xbar_cfg | `LatencyMode` | `enum logic [9:0]` | Latency on the individual channels, defined in detail in section *Pipelining and Latency* below. | | `AxiIdWidthSlvPorts` | `int unsigned` | The AXI ID width of the slave ports. | | `AxiIdUsedSlvPorts` | `int unsigned` | The number of slave port ID bits (starting at the least significant) the crossbar uses to determine the uniqueness of an AXI ID (see section *Ordering and Stalls* below). This value has to be less or equal than `AxiIdWidthSlvPorts`. | +| `UniqueIds` | `bit` | If you can guarantee that the ID of each transaction is always unique among all in-flight transactions in the same direction, setting this parameter to `1'b1` simplifies the crossbar. See the [`axi_demux` documentation](axi_demux#ordering-and-stalls) for details. | | `AxiAddrWidth` | `int unsigned` | The AXI address width. | | `AxiDataWidth` | `int unsigned` | The AXI data width. | | `NoAddrRules` | `int unsigned` | The number of address map rules. | diff --git a/scripts/axi_intercon_gen.py b/scripts/axi_intercon_gen.py index 8f6b02544..af5b3a209 100644 --- a/scripts/axi_intercon_gen.py +++ b/scripts/axi_intercon_gen.py @@ -351,6 +351,7 @@ def write(self): LatencyMode: axi_pkg::CUT_ALL_AX, AxiIdWidthSlvPorts: AxiIdWidthMasters, AxiIdUsedSlvPorts: AxiIdUsed, + UniqueIds: 1'b0, AxiAddrWidth: AxiAddrWidth, AxiDataWidth: AxiDataWidth, NoAddrRules: NoSlaves diff --git a/src/axi_pkg.sv b/src/axi_pkg.sv index 712e4ca34..92ede558c 100644 --- a/src/axi_pkg.sv +++ b/src/axi_pkg.sv @@ -401,6 +401,7 @@ package axi_pkg; xbar_latency_e LatencyMode; int unsigned AxiIdWidthSlvPorts; int unsigned AxiIdUsedSlvPorts; + bit UniqueIds; int unsigned AxiAddrWidth; int unsigned AxiDataWidth; int unsigned NoAddrRules; diff --git a/src/axi_xbar.sv b/src/axi_xbar.sv index ae1e6ecad..d66cd97d5 100644 --- a/src/axi_xbar.sv +++ b/src/axi_xbar.sv @@ -141,6 +141,7 @@ module axi_xbar #( .NoMstPorts ( Cfg.NoMstPorts + 1 ), .MaxTrans ( Cfg.MaxMstTrans ), .AxiLookBits ( Cfg.AxiIdUsedSlvPorts ), + .UniqueIds ( Cfg.UniqueIds ), .FallThrough ( Cfg.FallThrough ), .SpillAw ( Cfg.LatencyMode[9] ), .SpillW ( Cfg.LatencyMode[8] ), diff --git a/test/tb_axi_xbar.sv b/test/tb_axi_xbar.sv index b1acd3abc..595bd6ad3 100644 --- a/test/tb_axi_xbar.sv +++ b/test/tb_axi_xbar.sv @@ -55,6 +55,7 @@ module tb_axi_xbar #( LatencyMode: axi_pkg::CUT_ALL_AX, AxiIdWidthSlvPorts: AxiIdWidthMasters, AxiIdUsedSlvPorts: AxiIdUsed, + UniqueIds: 1'b0, AxiAddrWidth: AxiAddrWidth, AxiDataWidth: AxiDataWidth, NoAddrRules: 8 From f0cc14213f9198144cc763f499e5c4763ee85afb Mon Sep 17 00:00:00 2001 From: Andreas Kurth Date: Wed, 21 Apr 2021 18:05:21 +0200 Subject: [PATCH 4/7] axi_test (axi_rand_master): Add support for unique IDs --- src/axi_test.sv | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/axi_test.sv b/src/axi_test.sv index bd35e08bc..4fe92be75 100644 --- a/src/axi_test.sv +++ b/src/axi_test.sv @@ -620,6 +620,9 @@ package axi_test; parameter bit AXI_BURST_FIXED = 1'b1, parameter bit AXI_BURST_INCR = 1'b1, parameter bit AXI_BURST_WRAP = 1'b0, + parameter bit UNIQUE_IDS = 1'b0, // guarantee that the ID of each transaction is + // unique among all in-flight transactions in the + // same direction // Dependent parameters, do not override. parameter int AXI_STRB_WIDTH = DW/8, parameter int N_AXI_IDS = 2**IW @@ -968,6 +971,12 @@ package axi_test; r_flight_cnt[beat.ax_id] != 0 || w_flight_cnt[beat.ax_id] !=0 )) return 1'b0; end + if (UNIQUE_IDS) begin + // This master may only emit transactions with an ID that is unique among all in-flight + // transactions in the same direction. + if (is_read && r_flight_cnt[beat.ax_id] != 0) return 1'b0; + if (!is_read && w_flight_cnt[beat.ax_id] != 0) return 1'b0; + end // There is no reason why this ID would be illegal, so it is legal. return 1'b1; endfunction From 4516a9b7bdc4f42dfefc3366a04b3f15253ef412 Mon Sep 17 00:00:00 2001 From: Andreas Kurth Date: Wed, 21 Apr 2021 18:07:55 +0200 Subject: [PATCH 5/7] test/tb_axi_xbar: Test unique ID configuration This configuration is tested in addition to the configuration without unqiue IDs. To keep the overall TB runtime comparable even though we have doubled the number of tested configurations, the number of transactions per master module is halved. --- scripts/run_vsim.sh | 5 ++++- test/tb_axi_xbar.sv | 14 ++++++++------ 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/scripts/run_vsim.sh b/scripts/run_vsim.sh index 1435038fa..f177e77e7 100755 --- a/scripts/run_vsim.sh +++ b/scripts/run_vsim.sh @@ -106,7 +106,10 @@ exec_test() { axi_xbar) for Atop in 0 1; do for Exclusive in 0 1; do - call_vsim tb_axi_xbar -gTbEnAtop=$Atop -gTbEnExcl=$Exclusive + for UniqueIds in 0 1; do + call_vsim tb_axi_xbar -gTbEnAtop=$Atop -gTbEnExcl=$Exclusive \ + -gTbUniqueIds=$UniqueIds + done done done ;; diff --git a/test/tb_axi_xbar.sv b/test/tb_axi_xbar.sv index 595bd6ad3..195f0dde5 100644 --- a/test/tb_axi_xbar.sv +++ b/test/tb_axi_xbar.sv @@ -23,15 +23,16 @@ `include "axi/assign.svh" module tb_axi_xbar #( - parameter bit TbEnAtop = 1'b1, // enable atomic operations (ATOPs) - parameter bit TbEnExcl = 1'b0 // enable exclusive accesses + parameter bit TbEnAtop = 1'b1, // enable atomic operations (ATOPs) + parameter bit TbEnExcl = 1'b0, // enable exclusive accesses + parameter bit TbUniqueIds = 1'b0 // restrict to only unique IDs ); // Dut parameters localparam int unsigned NoMasters = 6; // How many Axi Masters there are localparam int unsigned NoSlaves = 8; // How many Axi Slaves there are // Random master no Transactions - localparam int unsigned NoWrites = 250; // How many writes per master - localparam int unsigned NoReads = 250; // How many reads per master + localparam int unsigned NoWrites = 125; // How many writes per master + localparam int unsigned NoReads = 125; // How many reads per master // timing parameters localparam time CyclTime = 10ns; localparam time ApplTime = 2ns; @@ -55,7 +56,7 @@ module tb_axi_xbar #( LatencyMode: axi_pkg::CUT_ALL_AX, AxiIdWidthSlvPorts: AxiIdWidthMasters, AxiIdUsedSlvPorts: AxiIdUsed, - UniqueIds: 1'b0, + UniqueIds: TbUniqueIds, AxiAddrWidth: AxiAddrWidth, AxiDataWidth: AxiDataWidth, NoAddrRules: 8 @@ -108,7 +109,8 @@ module tb_axi_xbar #( .MAX_READ_TXNS ( 20 ), .MAX_WRITE_TXNS ( 20 ), .AXI_EXCLS ( TbEnExcl ), - .AXI_ATOPS ( TbEnAtop ) + .AXI_ATOPS ( TbEnAtop ), + .UNIQUE_IDS ( TbUniqueIds ) ) axi_rand_master_t; typedef axi_test::axi_rand_slave #( // AXI interface parameters From e0aa7b2d37c5097d225dea0e3a27c16b8c152a12 Mon Sep 17 00:00:00 2001 From: Andreas Kurth Date: Thu, 22 Apr 2021 17:05:49 +0200 Subject: [PATCH 6/7] Changelog: Add `UniqueIds` to `axi_xbar` (#172) --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index bfd241891..104940a40 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,13 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Added ### Changed +- `axi_xbar` and `axi_demux`: Add support for unique IDs by adding a `UniqueIds` parameter to both + modules (#172). If you can guarantee that the ID of each transaction is always unique among all + in-flight transactions in the same direction, setting the `UniqueIds` parameter to `1'b1` + simplifies the demultiplexer (see documentation of `axi_demux` for details). This change is + backward-compatible on `axi_demux` (because the default value of the new parameter is `1'b0`). + As `axi_xbar` is configured with the `xbar_cfg_t` `struct`, this change is *not + backward-compatible* for `axi_xbar` (except for `xbar_cfg_t`s initialized with a `default` part). ### Fixed - `axi_test::axi_rand_master`: Refactor ID legalization into common function to simplify the From cd917ca5fe0f21eee24c0da5a6a6e49a9ad80577 Mon Sep 17 00:00:00 2001 From: Andreas Kurth Date: Wed, 28 Apr 2021 19:04:51 +0200 Subject: [PATCH 7/7] doc (demux): State one condition for `UniqueIds` more precisely --- doc/axi_demux.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/axi_demux.md b/doc/axi_demux.md index 3d67a2bbe..4b8b964ed 100644 --- a/doc/axi_demux.md +++ b/doc/axi_demux.md @@ -60,7 +60,7 @@ The reason for this behavior are AXI ordering constraints, see the [documentatio There are use cases that do not require the demultiplexer to keep track of and enforce this ordering, and the `UniqueIds` parameter can be set to specialize the demultiplexer for these cases: `UniqueIds` may be set to `1'b1` if and only if - each transaction has an ID that is unique among all in-flight transactions in the same direction; -- or for any ID, all transactions with that ID target the same master port as all other transactions with the same ID and direction; +- or for any ID, all transactions with that ID target the same master port as all other in-flight transactions with the same ID and direction; - or both. Setting the `UniqueIds` parameter to `1'b1` when those conditions are not always met leads to undefined behavior.