From c9b7d2153fc19cd0f6c886ea832015b39141ba1d Mon Sep 17 00:00:00 2001 From: Ola Jeppsson Date: Tue, 17 May 2016 10:50:50 +0200 Subject: [PATCH 1/2] common: oh_arbiter_rr3: Add 3 port round robin arbiter NOTE: This arbiter always grant access on one port, even when there are no requests. oh_fifo_cdc deasserts its access_out when wait_in is high. Signed-off-by: Ola Jeppsson --- common/hdl/oh_arbiter_rr3.v | 73 +++++++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) create mode 100644 common/hdl/oh_arbiter_rr3.v diff --git a/common/hdl/oh_arbiter_rr3.v b/common/hdl/oh_arbiter_rr3.v new file mode 100644 index 00000000..be81bc89 --- /dev/null +++ b/common/hdl/oh_arbiter_rr3.v @@ -0,0 +1,73 @@ +//############################################################################# +//# Function: 3 port clocked round-robin arbiter # +//############################################################################# +//# Author: Ola Jeppsson # +//# SPDX-License-Identifier: MIT # +//############################################################################# + +module oh_arbiter_rr3(/*AUTOARG*/ + // Outputs + grants, + // Inputs + clk, nreset, requests + ); + + parameter CW = 4; //counter width + + input clk; //clock + input nreset; //negative reset + + input [2:0] requests; //request vector + output [2:0] grants; //grant (one hot) + + // Regs + reg [1:0] state; + reg [CW-1:0] counter; + + // Wires + wire [2:0] grants; + wire next; + + always @(posedge clk or negedge nreset) + if (!nreset) + counter <= {(CW){1'b0}}; + else + counter <= counter[CW-1:0] + 1'b1; + + /* TOKEN STATE MACHINE */ +`define R0 2'b00 /* Favour requests[0] */ +`define R1 2'b01 /* Favour requests[1] */ +`define R2 2'b10 /* Favour requests[2] */ + + assign next = nreset & ~(|(counter[CW-1:0])); + + always @(posedge next or negedge nreset) + if (!nreset) + state[1:0] <= `R2; + else + case (state[1:0]) + `R2: state[1:0] <= `R0; + `R1: state[1:0] <= `R2; + default: state[1:0] <= `R1; + endcase + + /* NOTE: We always grant access, even when there are no requests. + * oh_fifo_cdc deasserts its access_out when wait_in is high. */ + assign grants[2:0] = state[1:0] == `R2 ? + (requests[2] ? 3'b100 : + requests[0] ? 3'b001 : + requests[1] ? 3'b010 : 3'b100) + : state[1:0] == `R1 ? + (requests[1] ? 3'b010 : + requests[2] ? 3'b100 : + requests[0] ? 3'b001 : 3'b010) + : + (requests[0] ? 3'b001 : + requests[1] ? 3'b010 : + requests[2] ? 3'b100 : 3'b001); + +endmodule // oh_arbiter_rr3 + +// Local Variables: +// verilog-library-directories:(".") +// End: From 72601e7e8eadc682186599ff913abfc2d669c803 Mon Sep 17 00:00:00 2001 From: Ola Jeppsson Date: Tue, 17 May 2016 10:53:41 +0200 Subject: [PATCH 2/2] elink: etx_arbiter: Use round-robin for TX queues All tests in elink/dv/tests produces the same output as before, except for 'test_regs.emf' where the original code shows a Write-After-Read condition with the access to ERX_CFG. Tested for several hours on one board with epiphany-examples running in parallel with e-read and e-write (to bogus MMR address), no issues. I'm not 100% happy with this one, as far as I can understand the 'oh_fifo_cdc' code there's a chicken-egg problem between 'wait_in'/'access_out' when using 'access_out' as an input to an arbiter request line. I think it would be better if the FIFO also exported 'valid' (~empty). Now the arbiter must guess (round-robin) which port might have data (see note in oh_arbiter_rr3.v). Signed-off-by: Ola Jeppsson --- elink/hdl/etx_arbiter.v | 76 +++++++++++++++++------------------------ 1 file changed, 32 insertions(+), 44 deletions(-) diff --git a/elink/hdl/etx_arbiter.v b/elink/hdl/etx_arbiter.v index b71b1400..6c871c83 100644 --- a/elink/hdl/etx_arbiter.v +++ b/elink/hdl/etx_arbiter.v @@ -2,15 +2,11 @@ ######################################################################## EPIPHANY eMesh Arbiter ######################################################################## - + This block takes three FIFO inputs (write, read request, read response) - and the DMA channel, arbitrates between the active channels, and forwards - the result to the transmit output pins. - - Arbitration Priority: - 1) read responses (highest) - 2) host writes - 3) read requests from host (lowest) + and the DMA channel, arbitrates between the active channels (using round + robin), and forwards the result to the transmit output pins. + */ module etx_arbiter (/*AUTOARG*/ @@ -74,22 +70,25 @@ module etx_arbiter (/*AUTOARG*/ //######################################################################## //# Arbiter //######################################################################## - //TODO: change to round robin!!! (live lock hazard) - - oh_arbiter #(.N(3)) arbiter (.grants({txrd_grant, - txwr_grant, //highest priority - txrr_grant - }), - .requests({txrd_access, - txwr_access, - txrr_access - }) - ); + + // ???: Add fifo_not_empty/valid signals instead off tx??_access? + // oh_fifo_cdc deasserts access_out when wait_in is high. + oh_arbiter_rr3 arbiter (.clk(clk), + .nreset(nreset), + .grants({txrd_grant, + txwr_grant, + txrr_grant + }), + .requests({txrd_access, + txwr_access, + txrr_access + }) + ); oh_mux3 #(.DW(PW)) mux3(.out (etx_mux[PW-1:0]), - .in0 (txwr_packet[PW-1:0]),.sel0 (txwr_grant), - .in1 (txrd_packet[PW-1:0]),.sel1 (txrd_grant), - .in2 (txrr_packet[PW-1:0]),.sel2 (txrr_grant) + .in0 (txwr_packet[PW-1:0]),.sel0 (txwr_valid), + .in1 (txrd_packet[PW-1:0]),.sel1 (txrd_valid), + .in2 (txrr_packet[PW-1:0]),.sel2 (txrr_valid) ); //###################################################################### @@ -97,32 +96,21 @@ module etx_arbiter (/*AUTOARG*/ //###################################################################### assign etx_all_wait = (etx_wait & ~cfg_match) | (etx_cfg_wait & cfg_match); - - //Read response - assign txrr_wait = etx_all_wait; - - //Write waits on pin wr wait or cfg_wait - assign txwr_wait = etx_all_wait | - txrr_access; - - //Host read request (self throttling, one read at a time) - assign txrd_wait = etx_all_wait | - txrr_access | - txwr_access; - + + assign txrr_wait = etx_all_wait | 1'b0 | txwr_valid | txrd_valid; + assign txwr_wait = etx_all_wait | txrr_valid | 1'b0 | txrd_valid; + assign txrd_wait = etx_all_wait | txrr_valid | txwr_valid | 1'b0; + //##################################################################### //# Pipeline stage (arbiter+mux takes time..) //##################################################################### - assign access_in = txwr_grant | - txrd_grant | - txrr_grant; -/* assign access_in = (txwr_grant & ~txwr_wait) | - (txrd_grant & ~txrd_wait) | - (txrr_grant & ~txrr_wait); + assign txwr_valid = txwr_grant & txwr_access; + assign txrd_valid = txrd_grant & txrd_access; + assign txrr_valid = txrr_grant & txrr_access; + + assign access_in = txwr_valid | txrd_valid | txrr_valid; - */ - packet2emesh #(.AW(AW)) p2e (.write_in (), .datamode_in (), @@ -147,7 +135,7 @@ module etx_arbiter (/*AUTOARG*/ if (!nreset) cfg_access <= 1'b0; else if (~etx_cfg_wait) - cfg_access <= (txwr_grant | txrd_grant) & cfg_match; + cfg_access <= (txwr_valid | txrd_valid) & cfg_match; //packet always @ (posedge clk)