BrianHG_DDR3_CONTROLLER open source DDR3 controller. NEW v1.60.

[SpacedCowboy]

I'll comment them out for now 

Run into something else in the FIFO code, which actually looks like it might be a bug in the compiler (see attached).

Looks as though you guard against assigning si[xx] if (xx!=1) in the generate loop but the compiler is still flagging it as being written to by both continuous and procedural assigns. (Had to double the x to xx because apparently a single x in brackets is a bullet point...)

Maybe an explicit x=1 stage and a generate for x>1 ? Or if the detection is only being triggered by the variable-name, and not the variable+index, then perhaps a rename where the si[1] is 'si1', same for so...

[BrianHG]

Funny, I don't remember my minimal PHY controller calling any of my FIFOs.

Anyways, that FIFO code is never used.  My complete 16 port 'BrianHG_DDR3_CONTROLLER_v16_top.sv' which uses the 'BrianHG_DDR3_COMMANDER_v16.sv' only calls the first 'BHG_FIFO_shifter_FWFT' fifo in that source HDL.  The others arent used at all as they were extras to test FMAX optimizations.  You should not yet be touching this stuff.

These are the only source files you need for the PHY_SEQ_only controller:
BrianHG_DDR3_PLL.sv
BrianHG_DDR3_PHY_SEQ_v16.sv
BrianHG_DDR3_CMD_SEQUENCER_v16.sv
BrianHG_DDR3_IO_PORT_ALTERA.sv
BrianHG_DDR3_GEN_tCK.sv
sync_rs232_uart.v
rs232_DEBUGGER.v

[SpacedCowboy]

Huh, ok. I'm using the base code in the RS232 debug directory - which I guess instantiates a BrianHG_DDR3_CONTROLLER_v16_top module, which in turn wants the FIFO.

Am I jumping ahead too far, here ?

[ah, ok, saw the edit. I can change things around a bit]

[BrianHG]

Huh, ok. I'm using the base code in the RS232 debug directory - which I guess instantiates a BrianHG_DDR3_CONTROLLER_v16_top module, which in turn wants the FIFO.

Am I jumping ahead too far, here ?

[ah, ok, saw the edit. I can change things around a bit]

Wrong folder, go to the one I pointed out here:
https://www.eevblog.com/forum/fpga/brianhg_ddr3_controller-open-source-ddr3-controller/msg4406407/#msg4406407

The one you are pointing to uses around 5-6k logic elements and has up to 16 user read/write ports.
The '_PHY_SEQ_only_v16' one I pointed to uses only around 3k logic elements and has 1 user read/write port.

[SpacedCowboy]

So the Gowin compiler doesn't like the code here when DDR_WIDTH_ROW = 13

The last line has the assumption that DDR_WIDTH_ROW > 13 I think. I've changed it to:

Code: [Select]

// *************************************************************************
// Output the CAS address on the DDR3 A bus.
// *************************************************************************
task SET_cas();
    begin
    OUT_A[9:0]                        <= S3_CAS[9:0] ; // Column address at the beginning of a sequential burst
    if (DDR3_WIDTH_CAS==10) OUT_A[11] <= 1'b0        ; // Default 0 for additional column address.
    else                    OUT_A[11] <= S3_CAS[10]  ; // Assign the additional MSB Column address used in 4 bit DDR3 devices.
    OUT_A[10]                         <= 1'b0        ; // Disable AUTO-PRECHARGE.  We keep the banks open and precharge manually only when needed.
    OUT_A[12]                         <= 1'b1        ; // Set burst length to BL8.
    if (DDR3_WIDTH_ROW > 13)
OUT_A[DDR3_WIDTH_ROW-1:13]    <= 0           ;
    end
endtask


[BrianHG]

So the Gowin compiler doesn't like the code here when DDR_WIDTH_ROW = 13

The last line has the assumption that DDR_WIDTH_ROW > 13 I think. I've changed it to:

Code: [Select]

// *************************************************************************
// Output the CAS address on the DDR3 A bus.
// *************************************************************************
task SET_cas();
    begin
    OUT_A[9:0]                        <= S3_CAS[9:0] ; // Column address at the beginning of a sequential burst
    if (DDR3_WIDTH_CAS==10) OUT_A[11] <= 1'b0        ; // Default 0 for additional column address.
    else                    OUT_A[11] <= S3_CAS[10]  ; // Assign the additional MSB Column address used in 4 bit DDR3 devices.
    OUT_A[10]                         <= 1'b0        ; // Disable AUTO-PRECHARGE.  We keep the banks open and precharge manually only when needed.
    OUT_A[12]                         <= 1'b1        ; // Set burst length to BL8.
    if (DDR3_WIDTH_ROW > 13)
OUT_A[DDR3_WIDTH_ROW-1:13]    <= 0           ;
    end
endtask


Try:

if (DDR3_WIDTH_ROW>13)  OUT_A[DDR3_WIDTH_ROW-1:13]    <= 0           ;

Never considered testing the smallest DDR3 ram chips.

[SpacedCowboy]

Ok, is >=14 better than >13 ?

It works (well, it compiles, still in the process of incrementally adding stuff so it doesn't work yet) with either of those two. I have bigger fish to fry right now though, because it doesn't like my DQ strobes code

ERROR (CK0012) : Instance 'DDR3_PHY/BHG_DDR3_IO_PORT_GOWIN/Gowin_DQ_Strobes[0].gowin_dqs_iddr_inst' has different control net from other iologic connected to the same buffer

which is referencing:

Code: [Select]

        begin : Gowin_DQ_Strobes

            wire gowin_dqs_out;                 // Internal: ODDR->IOBUF
            wire gowin_dqs_in;                  // Internal: IOBUF->IDDR
            wire gowin_dqs_tx;                  // Internal: OE on input to ODDR

            ODDR gowin_dqs_oddr_inst 
                (
                .Q0(gowin_dqs_out),             // ODDR -> LVDS
                .Q1(gowin_dqs_tx),              // 1'b0 => output
                .D0(1'b0),                      // Input data [SDR]
                .D1(1'b1),                      // Input data [SDR]
                .TX(~OE_DQS[x]),                // Input 'output enable' 0=output
                .CLK(DDR_CLK)                   // DDR clock
                );
 
            IDDR gowin_dqs_iddr_inst 
                (
                .Q0(RDQS_pl[x]),                // SDR to app #0
                .Q1(RDQS_ph[x]),                // SDR to app #1
                .D(gowin_dqs_in),               // DDR input signal
                .CLK(DDR_CLK_RDQ) // read clock
                );

            TLVDS_IOBUF gowin_dqs_lvds_iobuf_inst
                (
                .O(gowin_dqs_in),               // LVDS -> IDDR
                .IO(DDR3_DQS_p[x]),             // +ve LVDS pad
                .IOB(DDR3_DQS_n[x]),            // -ve LVDS pad
                .I(gowin_dqs_out),              // ODDR -> LVDS
                .OEN(gowin_dqs_tx)              // input when 1'b1
                );
           
            assign RDQS_nl[x] = ~RDQS_pl[x];
            assign RDQS_nh[x] = ~RDQS_ph[x];

        end

... and I'm assuming that means it doesn't like the DDR_CLK_RDQ on the IDDR when it has DDR_CLK on the ODDR, and when both of these are connected to the TLVDS_IOBUF

[edit: yep, if I change the clocks to be the same, I don't get that error any more (I do get the same error for the DQ iobufs). Looks like mixing and matching clocks isn't on]

[BrianHG]

'different control net'?  They do not specify which or what control net they are talking about?
Before we go haywire, let's see if there are different bidir IO buffers available.
Also, what options exist.

Also, if the DQ IO passes which does use 2 different clocks, then for the DQS, you can try the manual, or usually called emulated  differential approach which I had to use for the older Cyclone devices.

Basically 2 single ended buffers in parallel, where the second IO output has the high and low 2'b01 inverted to 2'b10.


Try removing the IO buffers, just wiring the DDRs to the IO pin.
Maybe requesting the IO buffer primitive prevents the use of adjacent logic cells which may be clocked on a different net.

[SpacedCowboy]

'different control net'?  They do not specify which or what control net they are talking about?

Nope - that's the full extent of the error. There's a whole bunch of warnings in there because not everything is linked up yet (no-driver, port undriven, etc.) and a few warnings about truncation when there's calculations in a parameter (they seem to be auto-expanded to 64-bit, and then it complains when shortening them down to the size of the receiver). But nothing until now that made me think "this is serious".

Before we go haywire, let's see if there are different bidir IO buffers available.
Also, what options exist.

Without wishing to sound too gloomy, as far as I can see, the primitives aren't really that flexible - there's an IOBUF or an LVDS_IOBUF (which is further split into 'true' and 'emulated' LVDS_IOBUFs). There are no parameters to set on either of the IOBUF types, and nothing really useful on the IDDR/ODDR (init on both and clock-polarity on the ODDR).

There is an IDDR_MEM, but that's designed to work closely with the DQS (DDR3 clocking) module, which doesn't have much more documentation than the image below, as far as I can tell.

Also, if the DQ IO passes which does use 2 different clocks, then for the DQS, you can try the manual, or usually called emulated  differential approach which I had to use for the older Cyclone devices.

Basically 2 single ended buffers in parallel, where the second IO output has the high and low 2'b01 inverted to 2'b10.

DQ doesn't actually pass, it's just shielded by DQS failing first - the netlist generation bails as soon as it can't do something, and DQS happened to be in front of DQ in the .sv file. If I swap them around, I get:

ERROR (CK0012) : Instance 'DDR3_PHY/BHG_DDR3_IO_PORT_GOWIN/gowin_DQ_bus[0].gowin_dq_iddr_inst' has different control net from other iologic connected to the same buffer

I mean, this can clearly work, Gowin provide a DDR3 IP core, but presumably they are using the primitives for memory-access, and have the docs of how that works. Whether it can be done in a platform-neutral way without taking advantage of those special primitives is a different question ...

[BrianHG]

Try removing the IO buffers, just wiring the I&O DDRs directly to the IO pin.
Maybe requesting the IO buffer primitive prevents the use of adjacent logic cells which may be clocked on a different net.

Gowin's DDR3 controller is probably using delay for the write data and using the DQS as an input to clock the DQ.  However, doing this does mean there is some way to separately clock in the data, but this may be a hard wired on-die feature.  Such a setup does simplify the PLL, but locks you into a specific range of clock frequencies.

[SpacedCowboy]

Try removing the IO buffers, just wiring the DDRs to the IO pin.
Maybe requesting the IO buffer primitive prevents the use of adjacent logic cells which may be clocked on a different net.

Ok, so for DQ, removing the IOBUF does in fact make it pass compilation. The code now looks like:

Code: [Select]

    for (x=0; x<DQ_WIDTH; x = x + 1)
        begin : gowin_DQ_bus

            wire gowin_dq_tx_out;

            ODDR gowin_dq_oddr_inst 
                (
                .Q0(DDR3_DQ[x]),                  // ODDR -> IOBUF
                .Q1(gowin_dq_tx_out),               // OE   -> IOBUF, 1'b0 => output
                .D0(PIN_WDATA_PIPE_h[0][x]),        // Input data [SDR]
                .D1(PIN_WDATA_PIPE_l[0][x]),        // Input data [SDR]
                .TX(~PIN_OE_WDQ_wide[x]),           // Input 'output enable' 1'b0=out
                .CLK(DDR_CLK_WDQ)                   // write clock
                );

            IDDR gowin_dq_iddr_inst 
                (
                .Q0(RDQ_l[x]),                      // SDR to app #0
                .Q1(RDQ_h[x]),                      // SDR to app #1
                .D(DDR3_DQ[x]),                     // DDR input signal
                .CLK(DDR_CLK_RDQ)                   // read clock
                );
        end


I'm not sure if this will work in reality, or whether it'll change the timing significantly (though I think the timing delays were in the DDR stages rather than the IOBUF stage).

Maybe I'm being dense, but I can't off-hand see how the LVDS can be implemented for the DQS signals without using an LVDS_IOBUF, I ought to go read that other code you mentioned to see how you did it ... Perhaps you're "faking" the LVDS part ? I guess I'll see.

[BrianHG]

Try removing the IO buffers, just wiring the DDRs to the IO pin.
Maybe requesting the IO buffer primitive prevents the use of adjacent logic cells which may be clocked on a different net.

Ok, so for DQ, removing the IOBUF does in fact make it pass compilation. The code now looks like:

Code: [Select]

    for (x=0; x<DQ_WIDTH; x = x + 1)
        begin : gowin_DQ_bus

            wire gowin_dq_tx_out;

            ODDR gowin_dq_oddr_inst 
                (
                .Q0(DDR3_DQ[x]),                  // ODDR -> IOBUF
                .Q1(gowin_dq_tx_out),               // OE   -> IOBUF, 1'b0 => output
                .D0(PIN_WDATA_PIPE_h[0][x]),        // Input data [SDR]
                .D1(PIN_WDATA_PIPE_l[0][x]),        // Input data [SDR]
                .TX(~PIN_OE_WDQ_wide[x]),           // Input 'output enable' 1'b0=out
                .CLK(DDR_CLK_WDQ)                   // write clock
                );

            IDDR gowin_dq_iddr_inst 
                (
                .Q0(RDQ_l[x]),                      // SDR to app #0
                .Q1(RDQ_h[x]),                      // SDR to app #1
                .D(DDR3_DQ[x]),                     // DDR input signal
                .CLK(DDR_CLK_RDQ)                   // read clock
                );
        end


I'm not sure if this will work in reality, or whether it'll change the timing significantly (though I think the timing delays were in the DDR stages rather than the IOBUF stage).

Maybe I'm being dense, but I can't off-hand see how the LVDS can be implemented for the DQS signals without using an LVDS_IOBUF, I ought to go read that other code you mentioned to see how you did it ... Perhaps you're "faking" the LVDS part ? I guess I'll see.

The 'LVDS' is actually still a 1.8v LVTTL signal, just differential.
What we call 'PSEUDO DIFFERENTIAL' is just when making 1 output high, the other goes low.
For the read, I only analyze the positive pin of the DQS even though I do wire them both up just for the balance.

Also remember, that whatever we choose for the DQ & DQS, we need to match the output buffer type / wiring for the DDR3_CK and the DDR3 command controls / address lines.

This is how I wired the DDR3_CK pins using standard DDR LVTTL1.8v:
https://github.com/BrianHGinc/BrianHG-DDR3-Controller/blob/c901baa0c41ae46389940ae729cc772c8d40a8f1/BrianHG_DDR3/BrianHG_DDR3_IO_PORT_ALTERA.sv#L360-L374

And this is how I wired the DQS:
https://github.com/BrianHGinc/BrianHG-DDR3-Controller/blob/c901baa0c41ae46389940ae729cc772c8d40a8f1/BrianHG_DDR3/BrianHG_DDR3_IO_PORT_ALTERA.sv#L390-L412

As you can see, it is just 2 output pins for the CK and 2 IO pins for the DQS.
Try wiring this up and then go back to modelsim to verify functionality.
Maybe Gowin's red bus error 16'hxxxx will now be replaced with the blue tristate 16'hzzzz...

[BrianHG]

Note that the there may be a different buffer used to get 2 clocks.
Maybe 1 buffer with tristate for the output.
Then instead wire the pin to the IDDR, or another dedicated input buffer to the same pin\.
With this, it may be possible to get the differential bidirectional functionality.

(Note that altera uses their IO buffer for the output and uses a logic cell deeper in the fabric for the read data.)

[SpacedCowboy]

Ta. I’ll think about this tomorrow - off to bed right now, it’s late

[SpacedCowboy]

So I have this simulating again, as far as I can tell... (see 'gowin-v-altera').

Before I start checking it back in synthesis, quick question on coding style. I could generate (for example) the clock using:

Code: [Select]

    for (x=0; x<DDR3_NUM_CK; x = x + 1)
        begin : DDR_Clocks
            wire gowin_clock;
            wire gowin_ck_oe;

            ODDR gowin_ck_ddr_inst_p
                (
                .Q0(DDR3_CK_p[x]),      // Send +ve clock to this pin
                .Q1(gowin_ck_oe),       // Not used but save a warning
                .D0(1'b0),              // clock goes low to start
                .D1(1'b1),              // clock goes high in 2nd phase
                .TX(1'b0),              // TX=0 -> Output pin
                .CLK(DDR_CLK)           // DDR clock
                );

            assign DDR3_CK_n[x] = ~DDR3_CK_p[x];
        end

but you've been stressing how important it is to have the outputs perfectly aligned through the same path-types. Is the above ok, even though there might be an extra logic-delay for the negation in the DDR3_CK_n[] path ? Or would it be better to duplicate the ODDRs, something like:

Code: [Select]

    for (x=0; x<DDR3_NUM_CK; x = x + 1)
        begin : DDR_Clocks
            wire gowin_clock;
            wire gowin_ck_oe_p;
            wire gowin_ck_oe_n;

            ODDR gowin_ck_ddr_inst_p
                (
                .Q0(DDR3_CK_p[x]),       // Send +ve clock to this pin
                .Q1(gowin_ck_oe_p),      // Not used but save a warning
                .D0(1'b0),               // clock goes low to start
                .D1(1'b1),               // clock goes high in 2nd phase
                .TX(1'b0),               // TX=0 -> Output pin
                .CLK(DDR_CLK)            // DDR clock
                );

             ODDR gowin_ck_ddr_inst_n
                (
                .Q0(DDR3_CK_n[x]),        // Send -ve clock to this pin
                .Q1(gowin_ck_oe_n),       // Not used but save a warning
                .D0(1'b1),                // clock goes high to start
                .D1(1'b0),                // clock goes low in 2nd phase
                .TX(1'b0),                // TX=0 -> Output pin
                .CLK(DDR_CLK)             // DDR clock
                );
        end

Same thing applies for DQS and DQ of course.

[BrianHG]

Code: [Select]

            assign DDR3_CK_n[x] = ~DDR3_CK_p[x];

You are going to have 1 logic cell making the CK_P and driving that pin.  Then, an un-clocked inverter gate making the CK_N.  This will not do.  There will be a delay from the P output to the N output.

You literally need to duplicate all the logic and driver for the '_N' output so that the logic feeding the pin appears to have the same delay from the source clock.  Remember, your CK_P is the direct clocked output of a logic cell, but your CK_N isn't.

Your second solution is the better one.

Also, have you tried using the original TBUFFER, but, tying the IDDR to the pin instead of tying it to the TBUFFER's return port?  This should be easy enough for your original DQ code, just move the IDDR source pin's name.  If this compiles, it is the best solution to work with for the entire DDR design.


(I guess with the original Gowin VS Altera, the 'red' traces were trying to tell us something even though it simulated...)

[SpacedCowboy]

You are going to have 1 logic cell making the CK_P and driving that pin.  Then, an un-clocked inverter gate making the CK_N.  This will not do.  There will be a delay from the P output to the N output.

Yep, that's what I was afraid of. It wouldn't be a big deal to duplicate it all, except that I am getting an error now when I try to get DQ to synthesize, where I thought it was working last night. I'm seeing:

ERROR (CK0011) : Instance 'DDR3_PHY/BHG_DDR3_IO_PORT_GOWIN/gowin_DQ_bus[0].gowin_dq_oddr_inst'(ODDR) of module 'top' cannot drive instance 'ddr_dq_0_iobuf'(IOBUF)

That must be an inferred IOBUF because I'm not instantiating one. I will try the ODDR->IOBUF->pads and IDDR<-pads idea...

[SpacedCowboy]

[sigh]. No, wiring up the IDDR to the pads is a no-go as well.

Code: [Select]

             wire gowin_dq_in;
             wire gowin_dq_out;
             wire gowin_dq_oe;

            ODDR gowin_dq_oddr_inst 
                (
                .Q0(gowin_dq_out),                  // 2x SDR -> DDR
                .Q1(gowin_dq_oe),                   // in-phase output enable
                .D0(PIN_WDATA_PIPE_h[0][x]),        // Input data [SDR]
                .D1(PIN_WDATA_PIPE_l[0][x]),        // Input data [SDR]
                .TX(PIN_OE_WDQ_wide[x]),            // Input 'output enable' 
                .CLK(DDR_CLK_WDQ)                   // write clock
                );

            IOBUF gowin_dq_iobuf_inst
                (
                .O(gowin_dq_in),                    // IOBUF -> IDDR (unused)
                .IO(DDR3_DQ[x]),                    // DQ pad
                .I(gowin_dq_out),                   // ODDR -> IOBUF
                .OEN(~gowin_dq_oe)                  // input when 1'b1
                );


            IDDR gowin_dq_iddr_inst 
                (
                .Q0(RDQ_l[x]),                      // SDR to app #0
                .Q1(RDQ_h[x]),                      // SDR to app #1
                .D(DDR3_DQ[x]),                     // DDR input signal
                .CLK(DDR_CLK_RDQ)                   // read clock
                );

Gives me: ERROR (EX0339) : Port 'ddr_dq[15]' drives 1 pad loads(gowin_DQ_bus[0].gowin_dq_iobuf_inst) and 1 non-pad loads(pin:D inst:gowin_DQ_bus[0].gowin_dq_iddr_inst)

I'm beginning to think Gowin really don't want to support this sort of access...

[BrianHG]

There is also asking Gowin.

I do not know about Gowin, but Altera does offer us visual diagrams in the data sheet showing us what type of wiring is available to each IO pin on the FPGA fabric.  It shows us the allowable connections.  Do you have such an illustration for Gowin.  Maybe it can offer a hint...

It would be really frustrating if they only allow if from 1 PLL as we have a separate PLL for the write data.

[BrianHG]

Try adding a second IBUF from pin net, then tie that to the IDDR.

[SpacedCowboy]

There is also asking Gowin.

This is looking like the best option atm, IMHO. The last query I sent off to the FAE went unanswered, though - perhaps they realized just how 'small potatoes' I am, or are busy elsewhere with something, but I can certainly try again. A query of 'do you have an example of IO with different clocks for read and write' is something an FAE might have a stock example for...

I do not know about Gowin, but Altera does offer us visual diagrams in the data sheet showing us what type of wiring is available to each IO pin on the FPGA fabric.  It shows us the allowable connections.  Do you have such an illustration for Gowin.  Maybe it can offer a hint...

It would be really frustrating if they only allow if from 1 PLL as we have a separate PLL for the write data.

I don't know much about the Altera side of things but I've seen similar diagrams representing the internals of an IOBUF in Xilinx docs showing the various inputs and outputs that can be attached - I can't find anything with that level of detail (or at least I haven't, yet) for Gowin - their diagrams are really just pictures of the verilog/VHDL definition of the port, example attached...

Try adding a second IBUF from pin net, then tie that to the IDDR.

Worth a go...

[SpacedCowboy]

Same as before: ERROR (EX0339) : Port 'ddr_dq[15]' drives 1 pad loads(gowin_DQ_bus[0].gowin_dq_iobuf_inst) and 1 non-pad loads(pin:I inst:gowin_DQ_bus[0].gowin_dq_extra_ibuf)

Code: [Select]

             gowin_dq_in;
             wire gowin_dq_out;
             wire gowin_dq_oe;
             wire gowin_ibuf_in;


            ODDR gowin_dq_oddr_inst 
                (
                .Q0(gowin_dq_out),                  // 2x SDR -> DDR
                .Q1(gowin_dq_oe),                   // in-phase output enable
                .D0(PIN_WDATA_PIPE_h[0][x]),        // Input data [SDR]
                .D1(PIN_WDATA_PIPE_l[0][x]),        // Input data [SDR]
                .TX(PIN_OE_WDQ_wide[x]),            // Input 'output enable' 
                .CLK(DDR_CLK_WDQ)                   // write clock
                );

            IOBUF gowin_dq_iobuf_inst
                (
                .O(gowin_dq_in),                    // IOBUF -> IDDR
                .IO(DDR3_DQ[x]),                    // DQ pad
                .I(gowin_dq_out),                   // ODDR -> IOBUF
                .OEN(~gowin_dq_oe)                  // input when 1'b1
                );


             IBUF gowin_dq_extra_ibuf
                (
                .I(DDR3_DQ[x]),
                .O(gowin_ibuf_in)
                );

            IDDR gowin_dq_iddr_inst 
                (
                .Q0(RDQ_l[x]),                      // SDR to app #0
                .Q1(RDQ_h[x]),                      // SDR to app #1
                .D(gowin_ibuf_in),                  // DDR input signal from extra inserted IBUF
                .CLK(DDR_CLK_RDQ)                   // read clock
                );


[BrianHG]

Example Altera datasheet  IO illustration:

[SpacedCowboy]

Where's the 'jealous' emoji ?

[BrianHG]

Not an IO buffer, use a tristate buffer.

Then an input buffer.