My project is as follows: I want to save the pixel values coming via UART to BRAM first, then pass them through an image processing filter, and send them back via UART. Currently, I want this filter to be used not as an image processing filter but for shifting right. That is, I want it to divide the incoming pixel values by 2 and send them back. I managed to run this without using BRAM. However, when BRAM is involved, I’m not sure if my FSM (Finite State Machine) and BRAM work together properly.

I created a Simple Dual Port BRAM by reading the Vivado Design Suite User Guide Synthesis. The main difference was that a separate always block was used to write to and read from BRAM. So, I did the same.

module imfilter
    #(parameter D_BITS = 8, // RAM WIDTH
                N = 400) // RAM DEPTH
    (
    input logic i_clk,
    input logic reset,
    //(*DONT_TOUCH = "true"*)
    input logic [31:0] bleng, //byte length of array
    input logic [D_BITS - 1:0] i_data,
    input logic i_valid, // write en signal
    input logic i_rdy,   // read en signal
    output logic [D_BITS - 1:0] o_data,
    output logic o_send
    );
    
    localparam RAM_PERFORMANCE = "LOW_LATENCY"; // Select "HIGH_PERFORMANCE" or "LOW_LATENCY" 
    localparam INIT_FILE = "";

    typedef enum logic [1:0] {
        IDLE = 2'b00,
        SAVE2MEM = 2'b01,
        SEND = 2'b10
        //STOP = 2'b10
    } state_t;
    /*(*DONT_TOUCH = "true"*)*/ //(* ram_style = "block" *)
    logic [D_BITS - 1:0] img [0:N - 1];
    logic [D_BITS - 1:0] img_data, dout;
    logic [$clog2(N) - 1:0] addra = 0;  // write address  index
    logic [$clog2(N) - 1:0] addrb = 0;  // read address index
    //(*DONT_TOUCH = "true"*)
    state_t state_reg;
    
    

    always_ff @(posedge i_clk) begin      
        if (i_valid) begin
            img[addra] <= i_data; end
        if(i_rdy) begin
            dout <= img[addrb]; end
    end    

    always_ff @(posedge i_clk) begin
        if(reset) begin
            state_reg <= SAVE2MEM;
            o_send <= 0;;
            addra <= 0;
            addrb <= 0;
        end else begin    
        o_send <= 0;
        case(state_reg) 
            IDLE: begin
                if(bleng) begin
                    state_reg <= SAVE2MEM; end
            end
            SAVE2MEM: begin
                if(i_valid) begin
                   addra <= addra + 1;        
                   if(addra == bleng - 1) begin
                       addra <= 0;
                       state_reg <= SEND; end
                end        
            end
            SEND: begin
                if(i_rdy) begin
                    o_send <= 1;
                    addrb <= addrb + 1; 
                    if(addrb == bleng - 1) begin
                        state_reg <= IDLE;
                        addrb <= 0; end
                end else begin
                    o_send <= 0; end
            end

            default: begin
                state_reg <= IDLE;
            end
        endcase end
    end
    
    assign o_data = (i_rdy)? dout : {D_BITS{1'bz}};

    
endmodule

Now, here are my questions:

1. Do you think there is any issue with this code? Have I properly combined FSM with BRAM?
2. I don’t want to directly send the value from BRAM to UART, but I want to subject it to a specific operation. Can I directly do this operation while assigning to the dout variable like dout <= img[addrb] >> 1? Or would it be more appropriate to assign it to dout and do this in the FSM?
3. I looked at Vivado’s sample codes, but there was nothing that resets the output data when the output register is not used. That’s why I decided to let it go to high impedance if the read operation is not enabled. Do you think this is correct? How to reset output data whenever reset button is pressed?