What is wrong with this attempt at an SDR RAM in Verilog?

Question

I have a Spartan-6 FPGA wired to the AEMIF memory interface on a TI DaVinci DM365 SoC that I control. The AEMIF is set up in Select Strobe mode. I'm trying to implement memory read/write on the FPGA over that interface, but it's not working. The hardware does work and this functionality has been working before written in VHDL (not written by me). I'm new to HDLs so maybe there is something obviously bogus here.

It's hard to tell what's actually going on as the clock is 60MHz and my scope/logic analyser struggles to go that fast.

Edit: I have since got this working. Setting drive_data in a combinatorial way meant that on read, the old memory value was driven out on the data bus, then later the sequential logic would pick up the new address and change the data during the cycle.

module main(
    input EM_A_3,
    input EM_A_7,
    input EM_CLK,
    inout [15:0] EM_D,
    input EM_nCE1,
    input EM_nOE,
    input EM_nWE
    );

wire [1:0] em_addr;
/* temporary storage for emif "registers" */
reg [15:0] mem [0:3];
reg [15:0] em_outdata;

supply0 rst;        // reset always 0 for now
wire drive_data;

initial
begin: FOO
    integer i;
    for (i = 0; i < 4; i = i + 1) begin
        mem[i] = 8'b0;
    end
    em_outdata = 8'b1;
end

// drive EM_D when CE1, OE are low, and WE is high
assign drive_data = !EM_nCE1 && !EM_nOE && EM_nWE;
assign EM_D = drive_data ? em_outdata : 8'bz;
assign em_addr = {EM_A_7, EM_A_3};

// clocked version (not working yet)
always @ (posedge EM_CLK)
begin
    if (!EM_nCE1 && !EM_nWE) begin
        mem[em_addr] <= EM_D;
    end
    if (!EM_nCE1 && !EM_nOE && EM_nWE) begin
        em_outdata <= mem[em_addr];
    end
end

endmodule

Answer 1

I got this working, with one specific change although to be honest some other edits may have helped too.

I thought about how drive_data is set up using combinatorial logic, but the outdata register is clocked. Confirmed with behavioural sim, this means that stale data is driven out during the first part of a read cycle, before the address is latched.

I "fixed" this by changing the always block that sets outdata to do so on every clock, meaning the right data gets in there during the setup phase while address is valid on the bus, before the OE strobe comes along.

I also refactored my memory bus handling into a submodule (inverting the control signals).

module emif(
    input clk,
    input [1:0] addr,
    inout [15:0] data,
    input ce,           // note these signals are active high
    input we,           // (opposite to the PCB signals)
    input oe
    );

wire drive_data;
reg [15:0] mem [0:3];
reg [15:0] em_outdata;

assign drive_data = ce && oe && !we;
assign data = drive_data ? em_outdata : 16'bz;

// writes data to small mem
always @ (posedge clk)
begin
    if (ce && we) begin
        mem[addr] <= data;
    end
end

// reads data from small mem
always @ (posedge clk)
begin
    em_outdata <= mem[addr];
end

endmodule

Answer 2

I'm not sure of the exact SDRAM timings but just commenting on your Verilog style, a couple of things might help. You need to start thinking in terms of hardware. This may be easier to understand.

assign EM_D = drive_data ? em_outdata : 16'bz;
assign drive_data = !EM_nCE1 & !EM_nOE & EM_nWE;

// writes data to small mem
always @ (posedge EM_CLK)
begin
    if (!EM_nCE1 && !EM_nWE) begin
        mem[em_addr] <= EM_D;
    end
end

// reads data from small mem
always @ (posedge EM_CLK)
begin
    if (!EM_nCE1 && !EM_nOE && EM_nWE) begin
        em_outdata <= mem[em_addr];
    end
end

// see any problem? please change this logic. it doesn't seem to go anywhere.
always @ (posedge EM_CLK)
begin
    if (!EM_nCE1 && !EM_nWE) begin
        outbit <= 1;
    end else if (!EM_nCE1 && !EM_nOE && EM_nWE) begin
        outbit <= 0;
    end else
        outbit <= X; // *** Not sure what's the default/reset cond is.
end

Also, you might want to move your initial-begin block out of this file as it is typically a simulation construct and in this case, it won't be synthesisable anyway. It's typically better to separate your simulation constructs from your synthesisable constructs.

If you want to reset the RAM contents, you can either use the FPGA tool to set the initial RAM contents or design a small block that resets the RAM at power-up.

Good luck. And let me know if it helps. :)

Answer 3

It's hard to tell what's actually going on as the clock is 60MHz and my scope/logic analyser struggles to go that fast.

Use ChipScope (Xilinx) or SignalTap (Altera). It's great for that and can give you plenty of width, if not depth.