[SOLVED] Artix-7 MIG DDR3 unexpected read/write result and DDR3 address question

[clin684]

Hi there,

I'm trying to get DDR3 interfacing to work on an Artix-7 evaluation board for my school project.

The Artix-7 evaluation board has a SODIMMs (MT8JTF12864HZ-1G6) of 1GB with a data width of 64 bits. It runs at a clock rate of 400 MHz, with 200 MHz PLL input clock, and a 100 MHz output clock which I used with my interface logic. I have attached the specs of the RAM the MIG 7 generated.

My test interface only does two simple write in sequence, at address 8 (x"4869") and 16 (x"ACE"), and then two simple reads at the same locations. The result is then displayed on the LCD display come with the board. When I programmed this and ran it on the board, somehow the 1st data being read is x"0000" and the 2nd data being read id x"202". After I pressed reset and let it run again, the first data become x"08CE" and the 2nd data become x"ACE". I have tried adding wait states between the write and the read and performing the read right after each write, but the result is never what I'm expecting. Does anyone what the problem is? I have copied my sample code below.

Another question I have is about the address the UI uses. It is a 28 bit address, but the ram only has 3 bits for the bank, 14 bits for the row, and 10 bits for the columns, 27 bits total. So, why is there one extra bit in the app_addr? How can I find out easily which bit I can't use?

Thanks for your generous help!
Cheng

---

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
148
149
150
151
152
153
154
155
156
157
library IEEE;
use IEEE.STD_LOGIC_1164.ALL;
use IEEE.NUMERIC_STD.ALL;
 
entity Simple_Memory is
    Port ( Clock             : in STD_LOGIC;
           Reset             : in STD_LOGIC;
 
           App_Rd_Data       : in STD_LOGIC_VECTOR (511 downto 0);
           App_Rd_Data_End   : in STD_LOGIC;
           App_Rd_Data_Valid : in STD_LOGIC;
           App_Rdy           : in STD_LOGIC;
           App_Wdf_Rdy       : in STD_LOGIC;
 
           App_Addr          : out STD_LOGIC_VECTOR (27 downto 0);
           App_Cmd           : out STD_LOGIC_VECTOR (2 downto 0);
           App_En            : out STD_LOGIC;
           App_Wdf_Data      : out STD_LOGIC_VECTOR (511 downto 0);
           App_Wdf_End       : out STD_LOGIC;
           App_Wdf_Wren      : out STD_LOGIC;
           
           Data_1_Out        : out STD_LOGIC_VECTOR (15 downto 0);
           Data_2_Out        : out STD_LOGIC_VECTOR (11 downto 0);
           Curr_State        : out STD_LOGIC_VECTOR (2 downto 0));
end Simple_Memory;
 
architecture Behavioral of Simple_Memory is
    CONSTANT READ_CMD   : STD_LOGIC_VECTOR (2 downto 0) := "001";
    CONSTANT WRITE_CMD  : STD_LOGIC_VECTOR (2 downto 0) := "000";
    
    signal data_1 : std_logic_vector (63 downto 0);
    signal data_2 : std_logic_vector (63 downto 0);
    
    type state_machine is (Init, Write_1, Write_2, Read_1, Read_Wait_1, 
            Read_2, Read_Wait_2, Done);
    signal state        : state_machine := Init;
    signal next_state   : state_machine;
begin    
    state_transition_proc : process (Clock)
    begin
        if rising_edge (Clock) then
            if Reset = '1' then
                state <= Init;
            else
                state <= next_state;
            end if;
        end if;
    end process state_transition_proc;
 
    state_proc : process (state, App_Rdy, App_Wdf_Rdy, App_Rd_Data_Valid, App_Rd_Data)
    begin
        App_En       <= '0';
        App_Wdf_Wren <= '0';
        App_Wdf_End  <= '0';
        
        App_Cmd      <= (others => '0');
        App_Addr     <= (others => '0');
        App_Wdf_Data <= (others => '0');
        
        case state is
            when Init =>
                Curr_State <= "001";
                data_1 <= (others => '0');
                data_2 <= (others => '0');
            
                if App_Rdy = '0' then
                    next_state <= Init;
                else
                    next_state <= Write_1;
                end if;
            when Write_1 =>
                Curr_State <= "010";
            
                App_Cmd <= WRITE_CMD;
                App_Addr(3 downto 0) <= x"8";
                App_En <= '1';
            
                App_Wdf_Wren <= '1';
                App_Wdf_End <= '1';
                App_Wdf_Data(15 downto 0) <= x"4869"; 
                
                if App_Rdy = '1' and App_Wdf_Rdy = '1' then
                    next_state <= Write_2;
                else
                    next_state <= Write_1;
                end if;
            when Write_2 =>
                Curr_State <= "011";
            
                App_Cmd <= WRITE_CMD;
                App_Addr(7 downto 0) <= x"10";
                App_En <= '1';
            
                App_Wdf_Wren <= '1';
                App_Wdf_End <= '1';
                App_Wdf_Data(11 downto 0) <= x"ACE";
                
                if App_Rdy = '1' and App_Wdf_Rdy = '1' then
                    next_state <= Read_1;
                else
                    next_state <= Write_2;
                end if;
            when Read_1 =>
                Curr_State <= "100";
            
                App_Cmd <= READ_CMD;
                App_Addr(3 downto 0) <= x"8";
                App_En <= '1';
                
                if App_Rdy = '1' then
                    next_state <= Read_Wait_1;
                else
                    next_state <= Read_1;
                end if;
            when Read_Wait_1 =>
                Curr_State <= "101";
                App_Cmd <= READ_CMD;
                App_Addr(3 downto 0) <= x"8";
                
                if App_Rd_Data_Valid = '1' then
                    data_1 <= App_Rd_Data(63 downto 0);
                    next_state <= Read_2;
                else
                    next_state <= Read_Wait_1;
                end if;
            when Read_2 =>
                Curr_State <= "110";
            
                App_Cmd <= READ_CMD;
                App_Addr(7 downto 0) <= x"10";
                App_En <= '1';
                
                if App_Rdy = '1' then
                    next_state <= Read_Wait_2;
                else
                    next_state <= Read_2;
                end if;
            when Read_Wait_2 =>
                Curr_State <= "111";
                App_Cmd <= READ_CMD;
                App_Addr(7 downto 0) <= x"10";
            
                if App_Rd_Data_Valid = '1' then
                    data_2 <= App_Rd_Data(63 downto 0);
                    next_state <= Done;
                else
                    next_state <= Read_Wait_2;
                end if;
            when Done =>
                Curr_State <= "000";
                next_state <= Done;
        end case;
    end process;
 
    Data_1_Out <= data_1(15 downto 0);
    Data_2_Out <= data_2(11 downto 0);
end Behavioral;

---

 

[vGoodtimes]

I seem to recall some versions of MIG adding a row bit and for implementation convenience forcing the minimum number of row bits to 1 in the case of single rank RAM.

8CE/ACE makes me think of a timing error on a data or mask line. 8CE is 1 bit different than ACE.

From what I recall, this version of MIG was a bit more forgiving than the later releases. However, you are setting app_wdf_wren and app_wren then checking if BOTH are ready to change state. There could be a case where app_rdy xor app_wdf_rdy is true. Still, I wouldn't expect to see the data that you are reading back.

Double check the read/write constants just in case. Same for design timing.

In the past, I usually waited for phy_init_done as well. Not sure if you are using it externally in some reset logic.

 

[clin684]

I seem to recall some versions of MIG adding a row bit and for implementation convenience forcing the minimum number of row bits to 1 in the case of single rank RAM.

8CE/ACE makes me think of a timing error on a data or mask line. 8CE is 1 bit different than ACE.

From what I recall, this version of MIG was a bit more forgiving than the later releases. However, you are setting app_wdf_wren and app_wren then checking if BOTH are ready to change state. There could be a case where app_rdy xor app_wdf_rdy is true. Still, I wouldn't expect to see the data that you are reading back.

Double check the read/write constants just in case. Same for design timing.

In the past, I usually waited for phy_init_done as well. Not sure if you are using it externally in some reset logic.

First of all, thanks for the help.

The masks I used in this test example are all zeros, so I concentrated on debugging the timing. I did get it to work as expected eventually. However, I noticed that in the testbench of traffic gen (the example provided by Xilinx), the app_rdy is not exactly synchronized with the clock. There's a noticeable delay in the app_rdy signal. This is also what's keep given me problems, at the end I came out with a hack to deal with this problem. Do you happend to have any idea of why this kind delay exists?

Also, when you said forcing the minimum number of row bits to 1, does that mean I have to leave that bit untouched (keep at low all time)? Since the address is in bank-row-col mode, is that extra row bit the most significant bit of the whole address or the most significant bit of the row, in this case address[24]?

Thanks again for you help!
Cheng

 

[ads-ee]

However, I noticed that in the testbench of traffic gen (the example provided by Xilinx), the app_rdy is not exactly synchronized with the clock. There's a noticeable delay in the app_rdy signal. This is also what's keep given me problems, at the end I came out with a hack to deal with this problem. Do you happend to have any idea of why this kind delay exists?

I suspect this delay is in the RTL code generating that signal and it is most likely generated from the clock but purposely delayed for some reason to move it off the clock edge (perhaps due to some interface modeling issue), or to emulate the delay from some external device (though that signal name doesn't look like it's external).

You typically see this kind of thing when someone doesn't fully understand how Verilog/VHDL schedules events, so they have to add arbitrary delays to fix simulation problems.

Yeah looking at the waveforms again I bet that app_rdy has issues due to the way the scheduling is done with the sys_clk_i and the half frequency clk, the delay to that signal was added so that there isn't some ugly delta delay issue with the edge of the app_rdy changing at the same time both clk and sys_clk_i have rising edges, I'd wager without the delay the app_rdy gets sampled too early. Probably the real issue is the way the clocks are generated (from a testbench or a clock generation model).

 

[clin684]

Okay, make sense. Thanks for the tips.

 

[vGoodtimes]

I can't match the second picture to your code. Eg, where do you have app_en= 0 AND app_wdf_en = 1? This makes me think you are designing by fixing sim errors. This should be avoided. It is easy to trick yourself by only simulating one or two test cases. This isn't to say that simulation isn't a good tool -- just that you should be able to explain your fixes beyond "it makes the waveforms line up".

In this case, I think you are saved because the UI standard allows wdf_* signals to become valid up to two cycles after the command signals. (if this is the version of MIG I am thinking of).

I don't know how you set up MIG, but it should be in the documentation and the source for MIG.

From what I recall, the rank bit location was the msb. For single rank design, it would get removed because things that used it were in if-generate blocks or ended up being constant during synthesis. But some code would want to be able to index it during elaboration. IMO, it is ok for Xilinx to do that internal to the core, but should not have exposed an unused address bit to the developer.

edit -- I wrote a different first line before realizing this sim doesn't match the fsm in the first post.

 

[clin684]

I have modified my code to go around this situation and resulting a waveform that doesn't not match the code I posted earlier.

Here's what I did:

1. I merged two process for the state machine into one process sensitive to clock edge. This does appear to make the result more staple. The data displayed on the LCD display became consistent for the first run and the runs after reset is pressed. It is also easier to debug when I simulate it in vivado since my last version of code passed the simulation but this version gives me results I see on the FPGA.

2. I break the write command into two different state. This modification is mainly used to deal with the case when the falling edge of the app_rdy happens right after the clock edge. In such a case, the command is not being read by the DDR3. So, I won't want the DDR3 to record the data. Otherwise, that data is not throw away but being used in the following writes.

3. By checking the state machine to be sensitive to clock edge also solve the case when the rising edge of the app_rdy signal happens slightly after the clock edge. The command will only be sent in the next clock cycle when the state machine detect a high in app_rdy.

4. I incorporated an additional state for reading the data. This is also mainly used to deal with the case when the falling edge of the app_rdy happens right after the clock edge. In such case, the same command would have to be resent.

I have attached my updated code in case my explanation is confusing.

Again, thanks for all your help.

library IEEE;
use IEEE.STD_LOGIC_1164.ALL;
use IEEE.NUMERIC_STD.ALL;

entity Simple_Memory is
    Port ( Clock             : in STD_LOGIC;
           Reset             : in STD_LOGIC;

           App_Rd_Data       : in STD_LOGIC_VECTOR (511 downto 0);
           App_Rd_Data_End   : in STD_LOGIC;
           App_Rd_Data_Valid : in STD_LOGIC;
           App_Rdy           : in STD_LOGIC;
           App_Wdf_Rdy       : in STD_LOGIC;

           App_Addr          : out STD_LOGIC_VECTOR (27 downto 0);
           App_Cmd           : out STD_LOGIC_VECTOR (2 downto 0);
           App_En            : out STD_LOGIC;
           App_Wdf_Data      : out STD_LOGIC_VECTOR (511 downto 0);
           App_Wdf_End       : out STD_LOGIC;
           App_Wdf_Wren      : out STD_LOGIC;
           
           Data_1_Out        : out STD_LOGIC_VECTOR (15 downto 0);
           Data_2_Out        : out STD_LOGIC_VECTOR (11 downto 0);
           Curr_State        : out STD_LOGIC_VECTOR (2 downto 0));
end Simple_Memory;

architecture Behavioral of Simple_Memory is
    CONSTANT READ_CMD   : STD_LOGIC_VECTOR (2 downto 0) := "001";
    CONSTANT WRITE_CMD  : STD_LOGIC_VECTOR (2 downto 0) := "000";
    
    signal data_1 : std_logic_vector (63 downto 0);
    signal data_2 : std_logic_vector (63 downto 0);
    
    type state_machine is (Init, Write_1, Set_Data_1, Write_2, Set_Data_2,
            Read_Prep_1, Read_1, Read_Wait_1, Read_Prep_2, Read_2, Read_Wait_2, 
            Done);
    signal state        : state_machine := Init;
    signal next_state   : state_machine;
begin
    state_proc : process (Clock)
    begin
        if rising_edge(Clock) then
        if Reset = '1' then
            state <= Init;
        else
            App_En       <= '0';
            App_Wdf_Wren <= '0';
            App_Wdf_End  <= '0';
            
            App_Cmd      <= (others => '0');
            App_Addr     <= (others => '0');
            App_Wdf_Data <= (others => '0');
            
            case state is
                when Init =>
                    Curr_State <= "001";
                    data_1 <= (others => '0');
                    data_2 <= (others => '0');
                
                    if App_Rdy = '1' then
                        state <= Write_1;
                    else
                        state <= Init;
                    end if;
                when Write_1 =>
                    Curr_State <= "010";
                    
                    if App_Rdy = '1' and App_Wdf_Rdy = '1' then
                        App_Cmd <= WRITE_CMD;
                        App_Addr(3 downto 0) <= x"0";
                        App_En <= '1';
                        
                        state <= Set_Data_1;
                    else
                        state <= Write_1;
                    end if;
                when Set_Data_1 =>
                    if App_Rdy = '0' or App_Wdf_Rdy = '0' then
                        state <= Write_1;
                    else
                        App_Wdf_Wren <= '1';
                        App_Wdf_End <= '1';
                        App_Wdf_Data(15 downto 0) <= x"4869"; 
                        
                        state <= Write_2;
                    end if;
                when Write_2 =>
                    Curr_State <= "011";
                    
                    if App_Rdy = '1' and App_Wdf_Rdy = '1' then
                        App_Cmd <= WRITE_CMD;
                        App_Addr(3 downto 0) <= x"8";
                        App_En <= '1';
                        
                        state <= Set_Data_2;
                    else
                        state <= Write_1;
                    end if;
                when Set_Data_2 =>
                    if App_Rdy = '0' or App_Wdf_Rdy = '0' then
                        state <= Write_2;
                    else
                        App_Wdf_Wren <= '1';
                        App_Wdf_End <= '1';
                        App_Wdf_Data(11 downto 0) <= x"ACE";
                        
                        state <= Read_1;
                    end if;
                when Read_1 =>
                    Curr_State <= "100";
                
                    if App_Rdy = '1' then
                        App_Cmd <= READ_CMD;
                        App_Addr(3 downto 0) <= x"0";
                        App_En <= '1';
                        
                        state <= Read_Prep_1;
                    else
                        state <= Read_1;
                    end if;
                when Read_Prep_1 =>
                    Curr_State <= "100";
                
                    if App_Rdy = '0' then
                        state <= Read_1;
                    elsif App_Rd_Data_Valid = '1' then
                        data_1 <= App_Rd_Data(63 downto 0);
                        state <= Read_2;
                    else
                        state <= Read_Wait_1;
                    end if;
                when Read_Wait_1 =>
                    Curr_State <= "101";
                    
                    if App_Rd_Data_Valid = '1' then
                        data_1 <= App_Rd_Data(63 downto 0);
                        state <= Read_2;
                    else
                        state <= Read_Wait_1;
                    end if;
                when Read_2 =>
                    Curr_State <= "110";
                
                    if App_Rdy = '1' then
                        App_Cmd <= READ_CMD;
                        App_Addr(3 downto 0) <= x"8";
                        App_En <= '1';
                        
                        state <= Read_Prep_2;
                    else
                        state <= Read_2;
                    end if;
                when Read_Prep_2 =>
                    Curr_State <= "100";
                
                    if App_Rdy = '0' then
                        state <= Read_2;
                    elsif App_Rd_Data_Valid = '1' then
                        data_2 <= App_Rd_Data(63 downto 0);
                        state <= Done;
                    else
                        state <= Read_Wait_2;
                    end if;
                when Read_Wait_2 =>
                    Curr_State <= "111";
                
                    if App_Rd_Data_Valid = '1' then
                        data_2 <= App_Rd_Data(63 downto 0);
                        state <= Done;
                    else
                        state <= Read_Wait_2;
                    end if;
                when Done =>
                    Curr_State <= "000";
                    state <= Done;
            end case;
        end if;
        end if;
    end process;

    Data_1_Out <= data_1(15 downto 0);
    Data_2_Out <= data_2(11 downto 0);
end Behavioral;

 

[aminpix]

Would you send the whole program.