---
# System prepended metadata

title: Lab3 FIR code review
tags: [SoC Lab]

---

# Lab3 FIR code review

Appreciate Jiin Lai for reviewing my lab3 verilog code and some suggestions.

[TOC]

## 1. AXI-Lite misunderstanding

- In Lab3, since we only load in coefficients for one time, so I let `awready` and `wready` be constant high.
- It may be ok in lab3, but it will have some errors in furture design.

![螢幕擷取畫面 2023-11-20 175520](https://hackmd.io/_uploads/HyrGpiOVa.png)

- Here is my improved design:

![檔案_000 (1)](https://hackmd.io/_uploads/SyTlHO9ET.png)
![檔案_000](https://hackmd.io/_uploads/H1vnEd946.png)


``` verilog=69
    assign tmp_rvalid = (arvalid | rvalid & ~rready); // set by ARVALID, reset by RREADY
    always @(posedge axis_clk) RVALID  <= (tmp_rvalid)?        1 : 0;
    always @(posedge axis_clk) ARREADY <= (arvalid)?           1 : 0;
    always @(posedge axis_clk) AWREADY <= (awvalid && wvalid)? 1 : 0;    
    always @(posedge axis_clk) WREADY  <= (awvalid && wvalid)? 1 : 0;

    assign awready = AWREADY;
    assign arready = ARREADY; 
    assign wready  = WREADY;
    assign rvalid  = RVALID;
```
### awready/wready

1. `awvalid` means that the write-in address can be used.
2. `wvalid` means that the write-in data can be used.
3. We need both address and data when we write data into BRAM.
- Therefore, for `awready` and `wready`, they can assert **after** `awvalid` and `wvalid` are sampled.
---

### arready/rvalid

1. `rvalid` may fall after `rready` is asserted (reset by rready).
2. Should be asserted after `arvalid` is sampled.
- Therefore, we can known that `rvalid` is set by `arvalid` and reset by `rready`.
- `arready` asserted after `arvalid` is sampled.
---

## 2. Xin/Yout 3T

- `sm_tvalid` asserted 3T later than `ss_tready`

![螢幕擷取畫面 2023-11-20 183346](https://hackmd.io/_uploads/S1MQ8nuEa.png)

### Explanation

1. We can run the HLS co-simulation of lab2 FIR, and we will get the waveform of FIR without operation pipelining.

![螢幕擷取畫面 2023-11-20 201344](https://hackmd.io/_uploads/r1EcaaOEa.png)

2. After we pipeline our operation like following:

![螢幕擷取畫面 2023-11-20 201714](https://hackmd.io/_uploads/BJFLA6_V6.png)

3. The waveform should be like:

![螢幕擷取畫面 2023-11-20 201814](https://hackmd.io/_uploads/HkXqATO4a.png)

4. The reason of 3T delay:

![檔案_000 (3)](https://hackmd.io/_uploads/rJuRbJtE6.png)

### Verilog code:

![螢幕擷取畫面 2023-11-20 202105](https://hackmd.io/_uploads/HJIbzJKVa.png)

## 3. Handle ap_ctrl independently

### Before:

![螢幕擷取畫面 2023-11-20 223102](https://hackmd.io/_uploads/SJY36JFET.png)

### After:

``` verilog=80
    always @* begin
        /*----- ap_idle -----*/
        if (ap_state == `AP_IDLE)
            ap_ctrl[2] = 1;
        else
            ap_ctrl[2] = 0;
    
        /*----- ap_start ----*/
        if (ap_state == `AP_IDLE && awaddr = 12'd0
        && wdata[0] == 1 && tlast_cnt != data_length)
            ap_ctrl[0] = 1;
        else
            ap_ctrl[0] = 0;
        
        /*----- ap_done -----*/
        if (sm_tvalid && sm_tlast) // since TLAST cannot be independently used without qualified by VALID.
            ap_ctrl[1] = 1;
        else if (ap_state == `AP_DONE)
            ap_ctrl[1] = 1;
        else
            ap_ctrl[1] = 0;
    end

```
---
## 4. Status signal v.s. Control signal

- The TVALID and TREADY handshake determines when information is passed across the interface
- Note: All information (TLAST, TKEEP, TSTROB … ) can not be independently used without qualified by TVALID
- **We should distinguish Status signal v.s. Control signal. Status signal must be qualified by Control signal before used.**

``` verilog=213
    always @* begin
        case (ss_state)
        `SS_IDLE:
        begin
            if (ss_tvalid && ss_tlast) begin
                next_ss_state = `SS_DONE;
                ss_idle       = 1;
            end
            else begin
                next_ss_state = `SS_IDLE;
                ss_idle       = 1;
            end
        end
        `SS_DONE:
        begin
            if (ss_tvalid) begin
                next_ss_state = `SS_IDLE;
                ss_idle       = 1;
            end
            else begin
                next_ss_state = `SS_DONE;
                ss_idle       = 0;
            end
        end
    end
```
## 5. Tolerate the latency of Xn, Yn

![螢幕擷取畫面 2023-11-20 232918](https://hackmd.io/_uploads/SJCUixtVT.png)
