Circular buffer design

[FvM]

Not sure why you have a pointer and an address, RAMs use addresses to access the memory array.

Good question. I would ask differently, why has your "circular buffer" read and write address inputs? It's expected to handle the memory addresses internally. Looks like an error of reasoning.

 

[design_newbie]

Thank you for the feedback , I modified the design more. So now I have only read address and write address. So I use wr_ptr and rd_ptr only as wires connected to wr_addr and rd_addr.

My issue is when rd_en is going high, I see the rd_addr "00A" in my waveform but rd_ptr is "XXX"
I checked the transcript and I have two warnings :

# Time: 40 ns Iteration: 1 Instance: /tb_circular_buffer ( Which is the time that rd_en goes high)
# ** Warning: NUMERIC_STD.TO_INTEGER: metavalue detected, returning 0

# Time: 48 ns Iteration: 1 Instance: /tb_circular_buffer/dut ( Which is the time that rd_en goes low)
# ** Warning: There is an 'U'|'X'|'W'|'Z'|'-' in an arithmetic operand, the result will be 'X'(es).

I did some search online and for the first warning it was saying the transition from std_logic_vector to integer does not happen correctly but I got no explanation, I made sure that I have a correct conversion to_integer(unsigned(rd_addr)) but still getting the warning
for the second warning, I checked all the default values which seems correct to me. I am not sure what else I need to do ?

Here is the waveform :


here is my design :

---

Code VHDL - [expand]
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
library IEEE;
use IEEE.STD_LOGIC_1164.ALL;
use IEEE.NUMERIC_STD.ALL;
use IEEE.STD_LOGIC_UNSIGNED.ALL;
 
entity circular_buffer is
    generic(
        DATA_WIDTH : integer := 8;
        ADDR_WIDTH : integer := 12 -- 2^12 = 4096 = 4k
    );
    Port ( 
        -- Input clock domain
        clk         : in  STD_LOGIC;
        rstn        : in  STD_LOGIC;
        wr_en       : in  STD_LOGIC;
        wr_addr     : in  STD_LOGIC_VECTOR (ADDR_WIDTH-1 downto 0);
        data_in     : in  STD_LOGIC_VECTOR (DATA_WIDTH-1 downto 0);
        
        -- Output clock domain
        s_clk       : in  STD_LOGIC;
        rd_en       : in  STD_LOGIC;
        rd_addr     : in  STD_LOGIC_VECTOR (ADDR_WIDTH-1 downto 0);
        data_out    : out STD_LOGIC_VECTOR (DATA_WIDTH-1 downto 0);
    full        : out STD_LOGIC;
    empty       : out STD_LOGIC;
    error       : out STD_LOGIC;
        data_ready  : out STD_LOGIC        
        );
end circular_buffer;
 
architecture Behavioral of circular_buffer is
 
constant ts_packet_size : integer := 188;
 
signal packet_count : unsigned(10 downto 0) := (others => '0'); -- 2^11 = 2048 enough for 1504
signal fifo_full    : std_logic := '0';
signal fifo_empty   : std_logic := '0';
signal ready        : std_logic := '0';
signal rd_ptr       : std_logic_vector(ADDR_WIDTH-1 downto 0) := (others => '0');
signal wr_ptr       : std_logic_vector(ADDR_WIDTH-1 downto 0) := (others => '0');
signal data_o       : std_logic_vector(DATA_WIDTH-1 downto 0) := (others => '0');
 
type memory_type is array (0 to 2**ADDR_WIDTH-1) of std_logic_vector(DATA_WIDTH-1 downto 0);
 
signal memory : memory_type := (others => (others => '0'));
 
begin
 
full       <= fifo_full;
empty      <= fifo_empty;
data_ready <= ready;
data_out   <= data_o;
 
error <= '1' when (fifo_empty='1' and rd_en='1') or (fifo_full='1' and wr_en='1') else '0';
 
process(clk, s_clk, rstn, wr_en, wr_addr, data_in, rd_en)
begin
    if rstn = '0' then
        rd_ptr      <= (others => '0');
        wr_ptr      <= (others => '0');
    packet_count<= (others => '0');
        fifo_full   <= '0';
        fifo_empty  <= '1';
    ready       <= '0';
    data_o      <= (others => '0');
 
    elsif rising_edge(clk) then
 
    wr_ptr     <= wr_addr;
 
        -- Write
        if (wr_en = '1' and fifo_full = '0') then
        memory(to_integer(unsigned(wr_ptr))) <= data_in;
            wr_ptr <= wr_ptr + 1;
 
        -- One packet is ready or not           
        if (packet_count = ts_packet_size) then
        ready <= '1';
        packet_count <= (others => '0');
        else
        ready <= '0';   
        packet_count <= packet_count + 1;   
            end if;
         end if;
    end if;
        
end process;
 
read_process : process(s_clk, rd_addr, wr_en, rd_en, memory)
begin
    if(rising_edge(s_clk)) then
 
        rd_ptr <= rd_addr;
 
        if(rstn = '1') then
            if (rd_en = '1') then                   
                  data_o <= memory(to_integer(unsigned(rd_ptr)));
                      rd_ptr <= rd_ptr - 1;
            elsif (rd_en = '0') then
              data_o <= x"00";
              rd_ptr <= rd_ptr;
            end if;
        end if;   
    end if;
end process;
 
end Behavioral;

---

and here is my testbench:

---

Code VHDL - [expand]
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
library IEEE;
use IEEE.STD_LOGIC_1164.ALL;
use IEEE.NUMERIC_STD.ALL;
use IEEE.STD_LOGIC_ARITH.ALL;
 
entity tb_circular_buffer is
end tb_circular_buffer;
 
architecture tb of tb_circular_buffer is
 
component circular_buffer
    generic(
        DATA_WIDTH  : integer := 8;
        ADDR_WIDTH  : integer := 12
    );
    port (
        clk        : in  std_logic;
        rstn       : in  std_logic;
        wr_en      : in  std_logic;
        wr_addr    : in  std_logic_vector (ADDR_WIDTH-1 downto 0);
        data_in    : in  std_logic_vector (DATA_WIDTH-1 downto 0);
    s_clk      : in std_logic;
        rd_en      : in  std_logic;
        rd_addr    : in  std_logic_vector (ADDR_WIDTH-1 downto 0);
        data_out   : out std_logic_vector (DATA_WIDTH-1 downto 0);
    full       : out std_logic;
        empty      : out std_logic;
    error      : out std_logic;
        data_ready : out std_logic
    );
end component;
 
constant DWIDTH   : integer := 8;
constant AWIDTH   : integer := 12;
 
signal clk        : std_logic;
signal rd_clk     : std_logic;
signal rstn       : std_logic;
signal wr_en      : std_logic;
signal wr_addr    : std_logic_vector (AWIDTH-1 downto 0);
signal data_in    : std_logic_vector (DWIDTH-1 downto 0);
signal rd_en      : std_logic;
signal rd_addr    : std_logic_vector (AWIDTH-1 downto 0);
signal data_out   : std_logic_vector (DWIDTH-1 downto 0);
signal full   : std_logic;
signal empty      : std_logic;
signal error      : std_logic;
signal data_ready : std_logic;
 
constant clk_period : time := 8 ns; --125MHz
 
-- Signal used to end simulator when we finished submitting our test cases
signal sim_done : boolean := false;
 
begin
    -- DUT instantiation
    dut : circular_buffer
    port map (clk        => clk,
              rstn       => rstn,
              wr_en      => wr_en,
              wr_addr    => wr_addr,
              data_in    => data_in,
          s_clk  => rd_clk,
              rd_en      => rd_en,
              rd_addr    => rd_addr,
              data_out   => data_out,
          full   => full,
          empty  => empty,
          error  => error,
              data_ready => data_ready);
 
    -- Clock generation
    write_clk_process : process
    begin
    if not sim_done then
            clk <= '1';
            wait for clk_period/2;
            clk <= '0';
            wait for clk_period/2;
    else
        wait;
    end if;
    end process;
 
 
    read_clk_process : process
    begin
    if not sim_done then
            rd_clk <= '1';
            wait for clk_period/2;
            rd_clk <= '0';
            wait for clk_period/2;
    else
        wait;
    end if;
    end process;
 
    stimuli : process
 
    -- Reset generation
    procedure async_rst is
    begin
        wait until rising_edge(clk);
        wait for clk_period;
        rstn <= '0';
        wait for clk_period;
        rstn <= '1';
    end procedure async_rst;
    
    begin
 
    -- intial state(default values)
        wr_en   <= '0';
        wr_addr <= (others => '0');
        data_in <= (others => '0');
        rd_en   <= '0';
    rd_addr <= (others => '0');
    rstn <= '0';
    wait for clk_period;
    async_rst;
 
    wait for 2*clk_period;
 
        -- single read (to check empty condition)
    rd_en   <= '1';
    rd_addr <= x"00A";
    wait until rising_edge(clk);
    assert data_out = x"AB" report "data out is not correct" severity warning;
    wait until rising_edge(clk);
    rd_en   <= '0';
 
    -- write to all locations
    for i in 1 to 4096 loop
            wr_en   <= '1';
        data_in <= conv_std_logic_vector(i,8);
        wr_addr <= conv_std_logic_vector(i-1,12);
        wait until rising_edge(clk);
                wr_en   <= '0';
    end loop;
 
    -- Instruct "clk_process" to halt execution.
 
    sim_done <= true;
 
        wait;
    end process;
end tb;

---

I appreciate any feedback.

 

[ads-ee]

You need to READ a VHDL tutorial. You still have significant mistakes in your code that unfortunately nobody has pointed out yet. I don't have the time or inclination to teach you VHDL, so find a tutorial and read it carefully.

Here are couple of issues I'll point out.

1) fix your sensitivity lists for the processes
This:

process(clk, s_clk, rstn, wr_en, wr_addr, data_in, rd_en)

should be this:

process(clk, rstn)

and this:

read_process : process(s_clk, rd_addr, wr_en, rd_en, memory)

should be this:

read_process : process(s_clk)

This is wrong it's in a different process than where you are assigning rd_ptr

    if rstn = '0' then
        rd_ptr      <= (others => '0');

same with data_o, which is also split across two processes.

A signal should only be assigned in a single process, do not assign signals in multiple processes it will cause multiple driver problems.

Here is the standard active low reset flip-flop template follow it religiously.

---

Code VHDL - [expand]
1
2
3
4
5
6
7
8
ff_template : process (clk, rst_n) -- no other signals appear in the sensitivity list for a reset flip-flop
begin
  if rst_n = '0' then
    my_ff <= '0';
  elsif rising_edge(clk) then
    -- my_ff assignment_code goes here
  end if;
end process;

---

If you want the memory to be a RAM then you need to use the proper template for a RAM implementation, as coded now you are more likely to generate a bunch of flip-flops for the memory.

I also don't understand why you are decrimenting the rd_ptr and incrimenting the wr_ptr. If you write to 0,1,2,3 addresses then you should be reading from 0,1,2,3 if you intend a circular buffer, perhaps your intention is to reverse the contents of the packet?

- - - Updated - - -

Your testbench should not be performing delays like wait for clock_period;, such statements result in race conditions with the clock generation and you end up with delta cycle problems in the simulation, where the clock captures the leading edge of the rd_en.

I mentioned before that you need to add delays to all your signals that are applied to your DUT if you want to use wait for statements in your testbench stimulus.

If you use rd_en <= '1' after 1 ns; the rd_ptr signal will go X on the trailing end of the rd_en instead of at the leading edge of rd_en.