Simplify Calyx AXI wrapper -- xVALID signals and reg invokes
#1846
+36
−177
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -72,32 +72,13 @@ component m_arread_channel( | |
|
|
||
| ARVALID = is_arvalid.out; | ||
|
|
||
| // this asserts valid and defines all inputs correctly | ||
| // because valid should not be deasserted until handshake occurs | ||
| // this all needs to be one group | ||
| // this contains blocking logic previously in its own group | ||
| group do_ar_transfer { | ||
| //assert ARVALID as long as this is the first time we are asserting it | ||
| is_arvalid.in = !arvalid_was_high.out ? 1'b1; | ||
|
|
||
| // TODO(nathanielnrn): in theory should be able to get rid of arvalid_was_high | ||
| // but for now we will be explicit and reduce this in generation maybe. Not sure | ||
|
|
@@ -134,25 +115,6 @@ component m_arread_channel( | |
| do_ar_transfer[done] = bt_reg.out; | ||
| } | ||
|
|
||
| group txn_incr { | ||
| txn_adder.left = txn_count.out; | ||
| txn_adder.right = 32'b1; | ||
|
|
@@ -172,16 +134,19 @@ component m_arread_channel( | |
| control{ | ||
| //XXX(nathanielnrn): What is best way to offer more flexiblity beyond just a counter? | ||
| seq{ | ||
| invoke txn_count(in=32'b0)(); | ||
| // TODO(nathanielnrn):Parametrize this. | ||
| // 7 is good for word wide data bus and 8 element vec_add We'd | ||
| // expect 8 transfers. Number of transfers that occur is ARLEN + 1 | ||
| invoke txn_len(in=8'd7)(); | ||
| while perform_reads.out with check_reads_done{ | ||
| seq{ | ||
| par { | ||
| invoke bt_reg(in=1'b0)(); | ||
| invoke arvalid_was_high(in=1'b0)(); | ||
| } | ||
| do_ar_transfer; | ||
| invoke is_arvalid(in=1'b0)(); | ||
| txn_incr; | ||
| } | ||
| } | ||
|
|
@@ -223,6 +188,7 @@ component m_read_channel( | |
| curr_addr_adder = std_add(64); | ||
|
|
||
| // block_transfer reg to avoid combinational loops | ||
| // Used to block any servicing until handshake occurs. | ||
| bt_reg = std_reg(1); | ||
|
|
||
| } | ||
|
|
@@ -231,19 +197,6 @@ component m_read_channel( | |
| RREADY = is_rdy.out; | ||
| data_received.read_en = 1'b0; | ||
|
|
||
| // NOTE: xVALID signals must be high until xREADY is high as well, so this works | ||
| // because if xREADY is high (is_rdy.out) then RVALID being high makes 1 flip | ||
| // and group will be done by bt_reg.out | ||
|
|
@@ -306,10 +259,10 @@ component m_read_channel( | |
| } | ||
| } | ||
| control{ | ||
| invoke n_RLAST(in=1'b1)(); //init n_RLAST | ||
| while n_RLAST.out{ | ||
| seq{ | ||
| invoke bt_reg(in=1'b0)(); //reset bt_reg | ||
| block_transfer; | ||
| receive_r_transfer; | ||
| incr_curr_addr; | ||
|
|
@@ -473,32 +426,13 @@ component m_awwrite_channel( | |
|
|
||
| AWVALID = is_awvalid.out; | ||
|
|
||
| // this asserts valid and defines all inputs correctly | ||
| // because valid should not be deasserted until handshake occurs | ||
| // this all needs to be one group | ||
| // this contains blocking logic previously in its own group | ||
| group do_aw_transfer { | ||
| //assert AWVALID | ||
| is_awvalid.in = !awvalid_was_high.out ? 1'b1; | ||
|
There was a problem hiding this comment. The same thing as |
||
|
|
||
| // TODO(nathanielnrn): in theory should be able to get rid of awvalid_was_high | ||
| // but for now we will be explicit and reduce this in generation maybe. Not sure | ||
|
|
@@ -540,22 +474,6 @@ component m_awwrite_channel( | |
| } | ||
|
|
||
|
|
||
|
|
||
| group txn_incr { | ||
| txn_adder.left = txn_count.out; | ||
|
|
@@ -575,16 +493,17 @@ component m_awwrite_channel( | |
| control{ | ||
| //XXX(nathanielnrn): What is best way to offer more flexiblity beyond just a counter? | ||
| seq{ | ||
| invoke txn_count(in=32'b0)(); | ||
| //TODO(nathanielnrn):parameterize this number. 7(+1) gives us 8 elements | ||
| invoke txn_len(in=8'd7)(); | ||
| while perform_write_txns.out with check_writes_done{ | ||
| seq{ | ||
| par { | ||
| invoke bt_reg(in=1'b0)(); | ||
| invoke awvalid_was_high(in=1'b0)(); | ||
| } | ||
| do_aw_transfer; | ||
| invoke is_awvalid(in=1'b0)(); | ||
| txn_incr; | ||
| } | ||
| } | ||
|
|
@@ -638,35 +557,15 @@ component m_write_channel( | |
|
|
||
| WVALID = wvalid.out; | ||
|
|
||
| //NOTE: xVALID signals must be high until xREADY is high as well, so this works | ||
| //because if xREADY is high (is_rdy.out) then RVALID being high makes 1 flip | ||
| //and group will be done by bt_reg.out | ||
| group do_write_transfer { | ||
| //set RREADY high | ||
| //TODO (nathanielnrn): technically we can make RREADY depend on on RVALID (but not vice versa). | ||
| //Could we simplify this we just making things ready when we are in | ||
|
|
||
| //NOTE: wvalid.in = 1'b1; does not work, it leaves WVALID high for 2 cycles | ||
| // this both asserts and deasserts one cycle later | ||
| wvalid.in = !(wvalid.out & WREADY & wvalid_was_high.out) ? 1'b1; | ||
| // TODO(nathanielnrn): Can prob get rid of wvalid_was_high | ||
|
|
@@ -675,7 +574,7 @@ component m_write_channel( | |
|
|
||
| //set to 1 after valid has been high even once | ||
| wvalid_was_high.in = 1'b1; | ||
| wvalid_was_high.write_en = !(wvalid.out & WREADY & !wvalid_was_high.out) ? 1'b1; | ||
|
There was a problem hiding this comment. This partitions assignment to |
||
|
|
||
|
|
||
| // set data output based on curr_addr register | ||
|
|
@@ -720,12 +619,11 @@ component m_write_channel( | |
| control{ | ||
| seq{ | ||
|
|
||
| invoke curr_addr(in=64'b0)(); //reset curr_addr | ||
| invoke n_finished_last_trnsfr(in=1'b1)(); //init reg | ||
| while n_finished_last_trnsfr.out{ | ||
| seq{ | ||
| invoke bt_reg(in=1'b0)(); | ||
| do_write_transfer; | ||
| par{ | ||
| incr_curr_addr; | ||
|
|
@@ -757,11 +655,6 @@ component m_bresp_channel( | |
| } | ||
| wires{ | ||
| BREADY = bready.out; | ||
|
|
||
| // TODO(nathanielnrn): This is probably very unoptimal and takes multiple | ||
| // cycles to simply do a handshake. Can probably be much better | ||
|
|
@@ -779,7 +672,7 @@ component m_bresp_channel( | |
| } | ||
| control{ | ||
| seq{ | ||
| invoke bt_reg(in=1'b0)(); | ||
| block_transfer; | ||
| } | ||
| } | ||
|
|
@@ -1003,49 +896,15 @@ component main( | |
| m2_WID = 1'b0; | ||
| m2_BID = 1'b0; | ||
|
|
||
| } | ||
| control{ | ||
| seq{ | ||
| //read stuff | ||
| par{ | ||
| //init base_addresses | ||
| invoke base_addr_A0(in = 64'b0)(); | ||
| invoke base_addr_B0(in = 64'b0)(); | ||
| invoke base_addr_Sum0(in = 64'b0)(); | ||
| } | ||
| par{ | ||
| seq{ | ||
|
|
@@ -1063,7 +922,7 @@ component main( | |
| ARBURST = m0_ARBURST | ||
| ); | ||
|
|
||
| invoke curr_addr_A0(in = base_addr_A0.out)(); //set curr_addr to base_address | ||
|
|
||
| invoke A0_read_channel[data_received = A0, curr_addr = curr_addr_A0] | ||
| ( | ||
|
|
@@ -1093,7 +952,7 @@ component main( | |
| ARBURST = m1_ARBURST | ||
| ); | ||
|
|
||
| invoke curr_addr_B0(in = base_addr_B0.out)(); //set curr_addr to base_address | ||
|
|
||
| invoke B0_read_channel[data_received = B0, curr_addr = curr_addr_B0] | ||
| ( | ||
|
|
@@ -1121,7 +980,7 @@ component main( | |
| ARBURST = m2_ARBURST | ||
| ); | ||
|
|
||
| invoke curr_addr_Sum0(in = base_addr_Sum0.out)(); //set curr_addr to base_address | ||
|
|
||
| invoke Sum0_read_channel[data_received = Sum0, curr_addr = curr_addr_Sum0] | ||
| ( | ||
|
|
@@ -1162,7 +1021,7 @@ component main( | |
| AWPROT = m0_AWPROT | ||
| ); | ||
|
|
||
| invoke curr_addr_A0(in = base_addr_A0.out)(); //set curr_addr to base_address | ||
|
|
||
| invoke A0_write_channel[internal_mem = A0, curr_addr = curr_addr_A0, max_trnsfrs = max_trnsfrs] | ||
| ( | ||
|
|
@@ -1192,7 +1051,7 @@ component main( | |
| AWPROT = m1_AWPROT | ||
| ); | ||
|
|
||
| invoke curr_addr_B0(in = base_addr_B0.out)(); //set curr_addr to base_address | ||
|
|
||
| invoke B0_write_channel[internal_mem = B0, curr_addr = curr_addr_B0, max_trnsfrs = max_trnsfrs] | ||
| ( | ||
|
|
@@ -1223,7 +1082,7 @@ component main( | |
| AWPROT = m2_AWPROT | ||
| ); | ||
|
|
||
| invoke curr_addr_Sum0(in = base_addr_Sum0.out)(); //set curr_addr to base_address | ||
|
|
||
| invoke Sum0_write_channel[internal_mem = Sum0, curr_addr = curr_addr_Sum0, max_trnsfrs = max_trnsfrs] | ||
| ( | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems to me like
is_arvalid.out & ARREADYwill always be true becauseis_arvalid.outis always 0 as we calldeassert_valin the control sequence:So we can simplify to just
!arvalid_was_high.outas our guard