What is wrong with my memory interface?!

[MattHollands]

Hi!

I am trying to use BRAM in an Artix-7 FPGA using Verilog in Vivado. I can initialise with the following code:

Code: [Select]

   wire [7:0] RAM_DO, RAM_DI;
   wire RAM_WE;
   wire [13:0] RAM_ADDR;
   
   BRAM_SINGLE_MACRO #(
          .BRAM_SIZE("36Kb"), // Target BRAM, "18Kb" or "36Kb"
          .DEVICE("7SERIES"), // Target Device: "7SERIES"
          .DO_REG(0), // Optional output register (0 or 1)
          .INIT(36'h000000000), // Initial values on output port
          .INIT_FILE ("NONE"),
          .WRITE_WIDTH(8), // Valid values are 1-72 (37-72 only valid when BRAM_SIZE="36Kb")
          .READ_WIDTH(8),  // Valid values are 1-72 (37-72 only valid when BRAM_SIZE="36Kb")
          .SRVAL(36'h000000000), // Set/Reset value for port output
          .WRITE_MODE("WRITE_FIRST") // "WRITE_FIRST", "READ_FIRST", or "NO_CHANGE"
        ) BRAM_SINGLE_MACRO_inst1 (
          .DO(RAM_DO),       // Output data, width defined by READ_WIDTH parameter
          .ADDR({3'b000,RAM_ADDR[9:0]}),   // Input address, width defined by read/write port depth
          .CLK(CLK100MHZ),     // 1-bit input clock
          .DI(RAM_DI),       // Input data port, width defined by WRITE_WIDTH parameter
          .EN(1'b1),       // 1-bit input RAM enable
          .REGCE(1'b0), // 1-bit input output register enable
          .RST(1'b0),     // 1-bit input reset
          .WE(RAM_WE)        // Input write enable, width defined by write port depth
        );
And it works totally fine. There is also a module driving RAM_ADDR, RAM_DI and RAM_WE, and reading RAM_DO. I reading/writing address from 0-511 and 512-1023. Basically, it runs in two modes where it is either reading from 0-511 and writing to 512-1023, or it is writing to 0-511 and reading from 512-1023.

Now, say I want to split these memory operations between two BRAMs, so addresses 0-511 would go to BRAM1 and addresses 512-1023 go to BRAM2. I use this code:

Code: [Select]

   wire [7:0] RAM_DO, RAM_DI, RAM_DO1, RAM_DO2;
   wire RAM_WE;
   wire [13:0] RAM_ADDR;
   assign RAM_DO = (RAM_ADDR[9] ? RAM_DO2 : RAM_DO1);
   
   BRAM_SINGLE_MACRO #(
          .BRAM_SIZE("36Kb"), // Target BRAM, "18Kb" or "36Kb"
          .DEVICE("7SERIES"), // Target Device: "7SERIES"
          .DO_REG(0), // Optional output register (0 or 1)
          .INIT(36'h000000000), // Initial values on output port
          .INIT_FILE ("NONE"),
          .WRITE_WIDTH(8), // Valid values are 1-72 (37-72 only valid when BRAM_SIZE="36Kb")
          .READ_WIDTH(8),  // Valid values are 1-72 (37-72 only valid when BRAM_SIZE="36Kb")
          .SRVAL(36'h000000000), // Set/Reset value for port output
          .WRITE_MODE("WRITE_FIRST") // "WRITE_FIRST", "READ_FIRST", or "NO_CHANGE"
        ) BRAM_SINGLE_MACRO_inst1 (
          .DO(RAM_DO1),       // Output data, width defined by READ_WIDTH parameter
          .ADDR({4'b0000,RAM_ADDR[8:0]}),   // Input address, width defined by read/write port depth
          .CLK(CLK100MHZ),     // 1-bit input clock
          .DI(RAM_DI),       // Input data port, width defined by WRITE_WIDTH parameter
          .EN(1'b1),       // 1-bit input RAM enable
          .REGCE(1'b0), // 1-bit input output register enable
          .RST(1'b0),     // 1-bit input reset
          .WE(((RAM_WE == 1) && (RAM_ADDR[9] == 0)))        // Input write enable, width defined by write port depth
        );
   
   
   BRAM_SINGLE_MACRO #(
          .BRAM_SIZE("36Kb"), // Target BRAM, "18Kb" or "36Kb"
          .DEVICE("7SERIES"), // Target Device: "7SERIES"
          .DO_REG(0), // Optional output register (0 or 1)
          .INIT(36'h000000000), // Initial values on output port
          .INIT_FILE ("NONE"),
          .WRITE_WIDTH(8), // Valid values are 1-72 (37-72 only valid when BRAM_SIZE="36Kb")
          .READ_WIDTH(8),  // Valid values are 1-72 (37-72 only valid when BRAM_SIZE="36Kb")
          .SRVAL(36'h000000000), // Set/Reset value for port output
          .WRITE_MODE("WRITE_FIRST") // "WRITE_FIRST", "READ_FIRST", or "NO_CHANGE"
        ) BRAM_SINGLE_MACRO_inst2 (
          .DO(RAM_DO2),       // Output data, width defined by READ_WIDTH parameter
          .ADDR({4'b0000,RAM_ADDR[8:0]}),   // Input address, width defined by read/write port depth
          .CLK(CLK100MHZ),     // 1-bit input clock
          .DI(RAM_DI),       // Input data port, width defined by WRITE_WIDTH parameter
          .EN(1'b1),       // 1-bit input RAM enable
          .REGCE(1'b0), // 1-bit input output register enable
          .RST(1'b0),     // 1-bit input reset
          .WE(((RAM_WE == 1) && (RAM_ADDR[9] == 1)))        // Input write enable, width defined by write port depth
        );
I can't see anything wrong with my code, but odd things are happening. If I write to addresses 512-1023, and then read addresses 0-511, I read back whatever I wrote to address 512-1023. I am really confused. Can anyone please help!

[hamster_nz]

I don't know Verilog but...  (seems to be a standard disclaimer for me, and most likely wearing thin)

It looks like you are using the RAM_ADDR[9] of the current cycle to decide which data to present from the previous cycle.

Code: [Select]

assign RAM_DO = (RAM_ADDR[9] ? RAM_DO2 : RAM_DO1);

E.g. if you have just read address 0x1FF, and you are about to access 0x200, you will use the ram_addr[9] from current address (0x200) and end up presenting the data from the block that is holding 0x200 through 0x3FF.

The solution is to remember the 'bank address' of the last cycle (in this case RAM_ADDR[9]), and use that multiplex between the different block RAM outputs.

As a side issue, it looks like your memory space is going to be 16kB. It might be more efficient to use eight 1x16k RAMs, bypassing the muxing into RAM_D0.

[MattHollands]

I don't know Verilog but...  (seems to be a standard disclaimer for me, and most likely wearing thin)

It looks like you are using the RAM_ADDR[9] of the current cycle to decide which data to present from the previous cycle.

Code: [Select]

assign RAM_DO = (RAM_ADDR[9] ? RAM_DO2 : RAM_DO1);

E.g. if you have just read address 0x1FF, and you are about to access 0x200, you will use the ram_addr[9] from current address (0x200) and end up presenting the data from the block that is holding 0x200 through 0x3FF.

The solution is to remember the 'bank address' of the last cycle (in this case RAM_ADDR[9]), and use that multiplex between the different block RAM outputs.

As a side issue, it looks like your memory space is going to be 16kB. It might be more efficient to use eight 1x16k RAMs, bypassing the muxing into RAM_D0.

The assign statement is asynchronous, ie it is combinatorial logic, therefore as soon as RAM_ADDR[9] changes, RAM_DO will change.
I agree that it makes sense in this case to use a single RAM, but in my actual application I will need ~50Kb and I am just using this case to demonstrate the issue that I am having.

[MattHollands]

No! You're right! I'm so dumb! It's because during a read cycle, I am setting up the address for the write on the next cycle which is when the read completes! How did I waste so many hours on this issue!!! Thanks!!