Author Topic: BrianHG_DDR3_CONTROLLER open source DDR3 controller. NEW v1.60.  (Read 92359 times)

0 Members and 1 Guest are viewing this topic.

Offline BrianHGTopic starter

  • Super Contributor
  • ***
  • Posts: 7857
  • Country: ca
Re: BrianHG_DDR3_CONTROLLER open source DDR3 controller. NEW v1.60.
« Reply #225 on: September 05, 2022, 01:51:45 am »
Good. The original sketch seemed "neater" to me anyway.

You're probably right about the approach - but Gowin IDDR/ODDR don't take vector arguments as the Altera ones seem to do. I could probably have bunched them up and used generate though, and kept it a little more generic. I'm generally of the school of thought "get it working" before "get it working well" and since there's a lot here that's new, keeping it simple where I can is "a good idea", at least to start off with. I think your abilities in the FPGA realm are orders of magnitude better than my own, so it's not surprising that we might tackle things differently :)
That's the way.
Even I use the generate for the DQ to gain access to separate OEs per bit on the Cyclone DDR primitive.  The MAX10 primitives already use 1 OE per data bit.
 

Offline SpacedCowboy

  • Frequent Contributor
  • **
  • Posts: 292
  • Country: gb
  • Aging physicist
Re: BrianHG_DDR3_CONTROLLER open source DDR3 controller. NEW v1.60.
« Reply #226 on: September 05, 2022, 07:12:09 pm »
Well, I'll go back and clean it up later - for the time being it actually fits reasonably well into the FPGA_VENDOR generate in BrianHG_DDR3_PHY_SEQ_v16.sv with a 'Gowin' section.

Anyways the code is in, and up if not actually running yet - we don't get past calibration (the simulator goes into an infinite loop - the first time I let it go for a while, thinking it was that macro in the testbench, before realization dawned...)

So the compared traces are below for the REA command after MSRs and ZQC. Warning: Image is a bit big... Gowin on top, Altera below, though the red traces ought to make that fairly clear :)

The DDR3_DQ offset seems pretty clear. FWIW, I tried adding the "shift-output-by-half-a-clock" defparam to both DQ and DQS...

Code: [Select]
            ...
            ODDR gowin_dq_oddr_inst 
                (
                .Q0(gowin_dq_out),                  // 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=out
                .CLK(DDR_CLK_WDQ)                   // write clock
                );

            // sync to negedge instead of posedge, 1/2 clock earlier output
            defparam gowin_dq_oddr_inst.TXCLK_POL   = 1'b1;
            ...
... but I didn't see any actual different traces in the sim, the outputs didn't shift across that I could see...

Then we see if the read will need a + or - 1 cycle tuning.  My code has room for - 2 clocks, + >4 clocks levels of adjustment.
What we will do here is add a localparam Gowen_read_correction = x  and add it to my set offset when the right Gowin FPGA is set for the vendor parameter.

Is this the CMD_ADD_DLY and/or RDQ_SYNC_CHAIN parameters in BrianHG_DDR3_IO_PORT_ALTERA.sv ? Or is it somewhere else ?
 

Offline BrianHGTopic starter

  • Super Contributor
  • ***
  • Posts: 7857
  • Country: ca
Re: BrianHG_DDR3_CONTROLLER open source DDR3 controller. NEW v1.60.
« Reply #227 on: September 05, 2022, 07:18:54 pm »
It appear you have a bug with you DQ output enable.
It is stuck with the output 'ON'.

This is why you see the DQ lines always green when = 16'h0000, but when the DDR3 model tries to output a 16'FFFF, you see a red 16'hXXXX as there is a bus conflict.

This is why my Altera show the blue 16'hZZZZ on the DQ before and after the DDR3's model outputs the read data pattern.

Check for the output enable logic first.
 
The following users thanked this post: SpacedCowboy

Offline SpacedCowboy

  • Frequent Contributor
  • **
  • Posts: 292
  • Country: gb
  • Aging physicist
Re: BrianHG_DDR3_CONTROLLER open source DDR3 controller. NEW v1.60.
« Reply #228 on: September 05, 2022, 08:03:17 pm »
Yep, good call - it was a polarity on the gowin TX (1'b0 == enable-output for gowin)

Ok, so DDR3_DQ are now being driven pretty much the same for both Gowin and Altera :) RDQ_h seems to be 2 clocks delayed, RDQ_l and RDQS_p{h,l} are a clock delayed. I'm not yet sure why RDQ_h ought to be different to RDQ_l - it's linked to the IDDR instantiation just the same. Perhaps this is the TXCLK_POL=1'b1 effect...

Code: [Select]
    for (x=0; x<DQ_WIDTH; x = x + 1)
        begin : gowin_DQ_bus

            wire gowin_dq_out;
            wire gowin_dq_in;
            wire gowin_dq_tx_out;

            ODDR gowin_dq_oddr_inst 
                (
                .Q0(gowin_dq_out),                  // 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
                );

            // sync to negedge instead of posedge, 1/2 clock earlier output
            defparam gowin_dq_oddr_inst.TXCLK_POL   = 1'b1;

            IDDR gowin_dq_iddr_inst 
                (
                .Q0(RDQ_h[x]),                      // SDR to app #0
                .Q1(RDQ_l[x]),     // SDR to app #1
                .D(gowin_dq_in),                    // DDR input signal
                .CLK(DDR_CLK_RDQ)                   // read 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_tx_out)               // input when 1'b1
                );

        end

Got to run to make lunch for the kids, so I'll get back to it later.

 

Offline BrianHGTopic starter

  • Super Contributor
  • ***
  • Posts: 7857
  • Country: ca
Re: BrianHG_DDR3_CONTROLLER open source DDR3 controller. NEW v1.60.
« Reply #229 on: September 05, 2022, 08:25:19 pm »
Ok, so DDR3_DQ are now being driven pretty much the same for both Gowin and Altera :) RDQ_h seems to be 2 clocks delayed, RDQ_l and RDQS_p{h,l} are a clock delayed. I'm not yet sure why RDQ_h ought to be different to RDQ_l - it's linked to the IDDR instantiation just the same. Perhaps this is the TXCLK_POL=1'b1 effect...

 TXCLK_POL=1'b1 appears to only be an adjustable delay for the output enable.  It has no effect until we write data to the DDR3.

What you have is a DDR_IN primitive configuration problem.
Take a look at the signals under the 'DDR3_PHY Data Path' divider.
These are the signals coming in directly from your DDR IN/OUT primitive, unprocessed.

Ok, for some reason, the blue b'zzzz isn't being fed through, this may be a natural consequence of Gowin's primitives, or, something wrong with your wiring.

Next, carefully analyze the RDQS_ph & RDQS_pl.  Except for the extended '0's, those seemed to have correctly read the data being generated by Mircon's DDR3 Model and actually match the output results of Altera's DDR in primitive.

Now, for the RDQ_h and RDQ_l output from the Gowin.  We can see that '_l' matches Altera, but, the '_h' is delayed 1 clock cycle.  Are there a set of parameters associated with Gowin's DDR_input primitive?  Maybe one to shift the read register by 180 degrees?  Doing so plus swapping the '_h' and '_l' read output should fix this problem.  If not, we will need to add a clocked DFF delay to all the '_l' reads in all your primitives, then instead of using a '2' read clock delay for my DDR3 routines, we will need to increase this read offset to a '3'.

(I'm assuming the h'xxxx errors in Gowin's primitive is their default response when reading a TBUFFER when in tristate when receiving a h'zzzz in simulation.  Or, you used a 'reg' instead of 'logic' or 'wire' in your home made DDR IO module.  In SystemVerilog, using the old style REG does not see and pass through the 'zzzz' sometimes.  The other choice is to report a bug or feature to Gowin regarding their primitive library not representing the IO pin's status.  Also, that the TLVDS IO reports '0' when the IO port is driven with b'z.  You may further inspect these signals in you module's wiring and right at Gowin's primitives to verify that this the case, or if it was your HDL code which has just fuzzed up the results.)
« Last Edit: September 05, 2022, 08:44:01 pm by BrianHG »
 
The following users thanked this post: SpacedCowboy

Offline BrianHGTopic starter

  • Super Contributor
  • ***
  • Posts: 7857
  • Country: ca
Re: BrianHG_DDR3_CONTROLLER open source DDR3 controller. NEW v1.60.
« Reply #230 on: September 05, 2022, 08:36:47 pm »
After you corrected the above issues, then go to this line:
BrianHG_DDR3/BrianHG_DDR3_IO_PORT_ALTERA.sv#L495

Change to this:
Code: [Select]
always_comb  RD_WINDOW = RDATA_window[RD_POS+RDQ_SYNC_CHAIN+3+CMD_ADD_DLY+(GOWIN_ENABLE*GOWIN_READ_OFFSET)]; // The extra '+3' counts for the extra latching of input to the 'RDQ_CACHE_x[0]'
The 2 new localparams, 'GOWIN_ENABLE' and 'GOWIN_READ_OFFSET' you need to place at the top after the port definitions.

GOWIN_ENABLE should = 0 unless the FGPA_VENDOR's first letter is a "G" or "g", then it should be = 1.

For now, the 'GOWIN_READ_OFFSET' should equal 2 or 3 depending on further tests.  However, make future allowance for recognizing different 'FPGA_FAMILY' options as Gowin's different FPGA silicon may have different response times for their DDR primitives.
« Last Edit: September 05, 2022, 08:45:42 pm by BrianHG »
 

Offline SpacedCowboy

  • Frequent Contributor
  • **
  • Posts: 292
  • Country: gb
  • Aging physicist
Re: BrianHG_DDR3_CONTROLLER open source DDR3 controller. NEW v1.60.
« Reply #231 on: September 05, 2022, 11:51:53 pm »
What you have is a DDR_IN primitive configuration problem.
Take a look at the signals under the 'DDR3_PHY Data Path' divider.
These are the signals coming in directly from your DDR IN/OUT primitive, unprocessed.

Ok, for some reason, the blue b'zzzz isn't being fed through, this may be a natural consequence of Gowin's primitives, or, something wrong with your wiring.

Next, carefully analyze the RDQS_ph & RDQS_pl.  Except for the extended '0's, those seemed to have correctly read the data being generated by Mircon's DDR3 Model and actually match the output results of Altera's DDR in primitive.

Now, for the RDQ_h and RDQ_l output from the Gowin.  We can see that '_l' matches Altera, but, the '_h' is delayed 1 clock cycle.  Are there a set of parameters associated with Gowin's DDR_input primitive?  Maybe one to shift the read register by 180 degrees?  Doing so plus swapping the '_h' and '_l' read output should fix this problem.  If not, we will need to add a clocked DFF delay to all the '_l' reads in all your primitives, then instead of using a '2' read clock delay for my DDR3 routines, we will need to increase this read offset to a '3'.

(I'm assuming the h'xxxx errors in Gowin's primitive is their default response when reading a TBUFFER when in tristate when receiving a h'zzzz in simulation.  Or, you used a 'reg' instead of 'logic' or 'wire' in your home made DDR IO module.  In SystemVerilog, using the old style REG does not see and pass through the 'zzzz' sometimes.  The other choice is to report a bug or feature to Gowin regarding their primitive library not representing the IO pin's status.  Also, that the TLVDS IO reports '0' when the IO port is driven with b'z.  You may further inspect these signals in you module's wiring and right at Gowin's primitives to verify that this the case, or if it was your HDL code which has just fuzzed up the results.)

The entirety of the DQ wiring is in the 'code' section a post or 3 up, there aren't any regs, just wires, and it's a straight map from pad to port (well, via the IOBUF/IDDR). Looking at the IDDR primitive, there are no parameters other than 'init=1'b0/1'b1' for Q0,Q1 (both defaulting to 0).

What is shown though is the timing diagram (attached), and it certainly looks as though Q0 and Q1 are supposed to be in-sync, not one delayed from the other. I checked the DDR3_RD_CLK clock to see if it had somehow gone awry and we were missing a 'beat', but it's lock-synced to DDR3_CLK in both Altera and Gowin sims.

I pulled out the internal wire 'gowin_dq_in' that transfers the data from the IOBUF:O output to the IDDR:I input, and that looks just fine - it's mirroring the DDR3_DQ signal in the simulation, so I think it's definitely something in the IDDR rather than some io-buffer delay.

Delving a bit deeper, stripping out the reset-control and initialization, the verilog library primitive for an IDDR looks like:
Code: [Select]
module IDDR(output Q0, output Q1, input D, input CLK);
  reg Q0_oreg, Q1_oreg,Q0_reg, Q1_reg;

  assign Q0 = Q0_reg;
  assign Q1 = Q1_reg;

  always @(posedge CLK) begin
      Q0_oreg <= D;
      Q0_reg <= Q0_oreg;
      Q1_reg <= Q1_oreg;
  end

  always @(negedge CLK) begin
      Q1_oreg <= D;
  end
endmodule

I pulled out one of the 16 IDDR bits into the simulation waveform (they're all either '1' or '0' at the same time, so one bit is representative). The four signals above are in the image 'gowin-iddr-sim' just under the cyan 'gowin_dq_in' trace.  If we start looking (@neg-edge on DDR_CLK_RDQ) at the yellow cursor, then Q1_oreg is set to 0. One half clock later (@posedge) ...
  • Q0_oreg is set to D at the end of the clock-cycle
  • Q0_reg is set to Q0_oreg at the end of the clock-cycle
  • Q1_reg is set to Q1_oreg at the end of the clock-cycle
It looks as though that logic can only work if it gets a positive clock-edge as the first clock-edge into the IDDR. If it gets a negedge first (as we're seeing), Q0_reg will necessarily be delayed by a clock. Not sure if this is a bug in the simulation library, or an actual hardware limitation. I guess I can ask Gowin...

 

Offline BrianHGTopic starter

  • Super Contributor
  • ***
  • Posts: 7857
  • Country: ca
Re: BrianHG_DDR3_CONTROLLER open source DDR3 controller. NEW v1.60.
« Reply #232 on: September 06, 2022, 12:02:32 am »
Ok, you will need to add your own DFF delay reg for all the '_l' to align the data output.

Remember, this is for all the reads, the DQ and DQS.

Also, please, please, please fix your waveform grid period.  Counting clocks from in to out has messed up and I do not know how to calculate the proper read window offset without clear visuals.
 

Offline BrianHGTopic starter

  • Super Contributor
  • ***
  • Posts: 7857
  • Country: ca
Re: BrianHG_DDR3_CONTROLLER open source DDR3 controller. NEW v1.60.
« Reply #233 on: September 06, 2022, 12:10:26 am »
You can also try reset / powerup the read clock to 180deg and swap the high and low to fix the problem.
This would save the extra delay reg.
 

Offline SpacedCowboy

  • Frequent Contributor
  • **
  • Posts: 292
  • Country: gb
  • Aging physicist
Re: BrianHG_DDR3_CONTROLLER open source DDR3 controller. NEW v1.60.
« Reply #234 on: September 06, 2022, 05:02:59 pm »
Okay, a bit stumped at the moment.

I've fixed up RDQ_{h,l} so they're being changed at the same time. I just put the DFF in place for the time being, I'll maybe play with clock-retiming later. So the trace below shows them changing correctly at the cursor - as before the Gowin one is on top, Altera below.

I experimented with the GOWIN_ENABLE/GOWIN_READ_OFFSET parameter, and matching the RD_WINDOW signal to be one clock after RDQ_{l,h} are available seems to require a GOWIN_READ_OFFSET of 3, so that's what's being shown in the screenshot, with the GOWIN_READ_OFFSET parameter being logged as the 2nd-to-top yellow trace.

It still wasn't reading correctly, so I logged RDQ_POS, and that seemed to be starting its incrementing one clock earlier, after RD_WINDOW goes high, than for the Altera design.

So I logged the RDQ_CACHE_{l,h} and you can see them below. RDQ_CACHE_l is being populated one clock before RDQ_CACHE_h, so I assumed I'd missed something with the DFF I'd just inserted. I still think it must be related to that, but I don't understand how... As far as I can tell, RDQ_CACHE_{l,h} are only updated in module BrianHG_DDR3_IO_PORT_ALTERA in the following code:

Code: [Select]
always_ff @(posedge DDR_CLK_RDQ) begin
                                            RDQ_CACHE_h[0]    <= RDQ_h;                       // Shift in the input read data
                                            RDQ_CACHE_l[0]    <= RDQ_l;                       // Shift in the input read data
    for (int i=0; i<(3+RDQ_SYNC_CHAIN);i++) RDQ_CACHE_h[i+1]  <= RDQ_CACHE_h[i];              // Shift the input across the cache
    for (int i=0; i<(3+RDQ_SYNC_CHAIN);i++) RDQ_CACHE_l[i+1]  <= RDQ_CACHE_l[i];              // Shift the input across the cache
    ...

So their only dependency ought to be RDQ_{l,h} and I can see both of those changing in sync with each other. So how can RDQ_CACHE_l be being changed one clock earlier than RDQ_CACHE_h ? Or vice-versa, RDQ_CACHE_h losing a clock over RDQ_CACHE_l, I guess ...

[This is all actually in the DDR3_IO_PORT_GOWIN module of course, but this logic is identical to the DDR3_IO_PORT_ALTERA module, the only changes I've made are to the DDR/LVDS instantiations (and the RD_WINDOW modification for GOWIN_ENABLE/GOWIN_READ_OFFSET).]

I thought it might be stale data from a previous run, but AFAICT the setup_phy_v16.do (which I run every time and let it call run_phy_v16.do) deletes everything in work/ as a precursor to doing anything at all... I'm sure it's something stupid I'm doing, but at least right now, I'm just not seeing it.

 

Offline BrianHGTopic starter

  • Super Contributor
  • ***
  • Posts: 7857
  • Country: ca
Re: BrianHG_DDR3_CONTROLLER open source DDR3 controller. NEW v1.60.
« Reply #235 on: September 06, 2022, 06:41:17 pm »
You did not need to bring up the RDQ_caches.
Now I cant follow what's going on with that mess.

It's the RDATA_store which is taking the buffer 1 clock early, so it has the wrong data in the buffer chain.

Try a 4 or 5 for the Gowin delay.

Just swap the read _h and _l at the instantiation of you DDR buffer and let my auto PLL tuning auto discover the 180 degree error as it will tune the Gowin PLL through the 180deg mode anyways.  Don't forget to do the same for the read DQS.

Having your read PLL reset set to 180 just means a few less cycles during simulation startup.

 

Offline SpacedCowboy

  • Frequent Contributor
  • **
  • Posts: 292
  • Country: gb
  • Aging physicist
Re: BrianHG_DDR3_CONTROLLER open source DDR3 controller. NEW v1.60.
« Reply #236 on: September 06, 2022, 07:46:41 pm »
Ok, you want to see something awesome ? Look at the attached :)

Took out the DFF's, inverted the returned RDQ_{_h,_l} and RDQS_{pl,ph}, reset GOWIN_READ_OFFSET to 3, and ran setup_phy_v16.do, which .... ran to completion.

The first few times through the training, RDQ_{h,l} are offset, with RDQ_l trailing by a clock. Then they match up as you can see in the image, and we get the training pattern back. Then the rest of the test bench runs - log attached :)

It's not entirely sweetness and light - there's a lot of tDSS violations on DQS and DQS_n bits on WRITE ops, and for some reason the test bench logs for Altera and Gowin start with different values, which might be indicative of a missed write-cycle or could be to do with the different phase of the two clocks, I haven't checked yet. This is the case right at the start all the way through to the end:
Code: [Select]
Gowin:
# BrianHG_DDR3_PHY_SEQ_v16_tb.sdramddr3_0.cmd_task: at time 20603750.0 ps INFO: Load Mode 3 MultiPurpose Register Select = Pre-defined pattern
# BrianHG_DDR3_PHY_SEQ_v16_tb.sdramddr3_0.cmd_task: at time 20603750.0 ps INFO: Load Mode 3 MultiPurpose Register Enable = Disabled
# BrianHG_DDR3_PHY_SEQ_v16_tb.sdramddr3_0.cmd_task: at time 20793750.0 ps INFO: Activate  bank 0 row 0040
# BrianHG_DDR3_PHY_SEQ_v16_tb.sdramddr3_0.cmd_task: at time 20811250.0 ps INFO: Write     bank 0 col 000, auto precharge 0
# BrianHG_DDR3_PHY_SEQ_v16_tb.sdramddr3_0.main: at time 20818750.0 ps INFO: Sync On Die Termination Rtt_NOM =         40 Ohm
# BrianHG_DDR3_PHY_SEQ_v16_tb.sdramddr3_0.cmd_task: at time 20821250.0 ps INFO: Write     bank 0 col 080, auto precharge 0
# BrianHG_DDR3_PHY_SEQ_v16_tb.sdramddr3_0.data_task: at time 20825000.0 ps INFO: WRITE @ DQS= bank = 0 row = 0040 col = 00000000 data = 4567
# BrianHG_DDR3_PHY_SEQ_v16_tb.sdramddr3_0.data_task: at time 20826250.0 ps ERROR: tDSS violation on DQS   bit           0
# BrianHG_DDR3_PHY_SEQ_v16_tb.sdramddr3_0.data_task: at time 20826250.0 ps ERROR: tDSS violation on DQS   bit           1
# BrianHG_DDR3_PHY_SEQ_v16_tb.sdramddr3_0.data_task: at time 20826250.0 ps ERROR: tDSS violation on DQS_N bit           0
# BrianHG_DDR3_PHY_SEQ_v16_tb.sdramddr3_0.data_task: at time 20826250.0 ps ERROR: tDSS violation on DQS_N bit           1
# BrianHG_DDR3_PHY_SEQ_v16_tb.sdramddr3_0.data_task: at time 20826250.0 ps INFO: WRITE @ DQS= bank = 0 row = 0040 col = 00000001 data = 89ab
# BrianHG_DDR3_PHY_SEQ_v16_tb.sdramddr3_0.data_task: at time 20827500.0 ps INFO: WRITE @ DQS= bank = 0 row = 0040 col = 00000002 data = cdef

vs Altera:
Code: [Select]
# BrianHG_DDR3_PHY_SEQ_v16_tb.sdramddr3_0.cmd_task: at time 14978750.0 ps INFO: Load Mode 3 MultiPurpose Register Select = Pre-defined pattern
# BrianHG_DDR3_PHY_SEQ_v16_tb.sdramddr3_0.cmd_task: at time 14978750.0 ps INFO: Load Mode 3 MultiPurpose Register Enable = Disabled
# BrianHG_DDR3_PHY_SEQ_v16_tb.sdramddr3_0.cmd_task: at time 15168750.0 ps INFO: Activate  bank 0 row 0040
# BrianHG_DDR3_PHY_SEQ_v16_tb.sdramddr3_0.cmd_task: at time 15186250.0 ps INFO: Write     bank 0 col 000, auto precharge 0
# BrianHG_DDR3_PHY_SEQ_v16_tb.sdramddr3_0.main: at time 15193750.0 ps INFO: Sync On Die Termination Rtt_NOM =         40 Ohm
# BrianHG_DDR3_PHY_SEQ_v16_tb.sdramddr3_0.cmd_task: at time 15196250.0 ps INFO: Write     bank 0 col 080, auto precharge 0
# BrianHG_DDR3_PHY_SEQ_v16_tb.sdramddr3_0.data_task: at time 15200000.0 ps INFO: WRITE @ DQS= bank = 0 row = 0040 col = 00000000 data = 0123
# BrianHG_DDR3_PHY_SEQ_v16_tb.sdramddr3_0.data_task: at time 15201250.0 ps INFO: WRITE @ DQS= bank = 0 row = 0040 col = 00000001 data = 4567
# BrianHG_DDR3_PHY_SEQ_v16_tb.sdramddr3_0.data_task: at time 15202500.0 ps INFO: WRITE @ DQS= bank = 0 row = 0040 col = 00000002 data = 89ab

Most of the writes work, but some don't, and return xxxx where they return data for the Altera log. There *are* some entries in the Altera log where xxxx is returned, but given the high 'row=0284' value, I'm guessing those are supposed to fail.
Code: [Select]
Gowin:
# BrianHG_DDR3_PHY_SEQ_v16_tb.sdramddr3_0.data_task: at time 21083750.0 ps INFO: WRITE @ DQS= bank = 7 row = 0004 col = 0000001f data = xxxx
...
# BrianHG_DDR3_PHY_SEQ_v16_tb.sdramddr3_0.data_task: at time 21483750.0 ps INFO: READ @ DQS= bank = 7 row = 0004 col = 0000001f data = xxxx

vs Altera:
# BrianHG_DDR3_PHY_SEQ_v16_tb.sdramddr3_0.data_task: at time 15458750.0 ps INFO: WRITE @ DQS= bank = 7 row = 0004 col = 0000001f data = 8888
...
# BrianHG_DDR3_PHY_SEQ_v16_tb.sdramddr3_0.data_task: at time 15663750.0 ps INFO: READ @ DQS= bank = 7 row = 0004 col = 0000001f data = 8888

Anyway, this is fantastic :) Thanks for all the help, inspiration, and guidance. Hopefully clearing up the remaining things above isn't too arduous :)
« Last Edit: September 06, 2022, 07:50:45 pm by SpacedCowboy »
 

Offline SpacedCowboy

  • Frequent Contributor
  • **
  • Posts: 292
  • Country: gb
  • Aging physicist
Re: BrianHG_DDR3_CONTROLLER open source DDR3 controller. NEW v1.60.
« Reply #237 on: September 06, 2022, 08:11:17 pm »
And just to keep it real, here's the traces of where the write-op is failing.
 

Offline BrianHGTopic starter

  • Super Contributor
  • ***
  • Posts: 7857
  • Country: ca
Re: BrianHG_DDR3_CONTROLLER open source DDR3 controller. NEW v1.60.
« Reply #238 on: September 06, 2022, 08:45:08 pm »
Note that if we read a memory address which has never been written to, Micron's DDR3 model does return to us an h'xxxx.  I have 1 such deliberate read in my testbench.

If any writes fail (including partial bits), then there will probably be multiple reads with h'xxxx in them.

It looks like we need to inspect the 'OE' for the DQS and the OE for the write data and compare it to the matched Altera reference version.  Once again, I have controls in my code to make +/-1 adjustments here, individually for the DQS and DQ oe's.

The DQS write is on a different phase than the write DQ data.
We need to compare this to the Altera's output of the same written byte.  You may just need to adjust Gowin's TX enable phase shift parameter so that the DQS gets asserted at the right time.

Please zoom the Altera and Gowin's matching write position bug with only my master recommended outputs shown in the waveform as this is all the needed debug information.
« Last Edit: September 06, 2022, 08:47:54 pm by BrianHG »
 

Offline BrianHGTopic starter

  • Super Contributor
  • ***
  • Posts: 7857
  • Country: ca
Re: BrianHG_DDR3_CONTROLLER open source DDR3 controller. NEW v1.60.
« Reply #239 on: September 06, 2022, 09:11:44 pm »
*** PRO-TIP - Double click on the:

# BrianHG_DDR3_PHY_SEQ_v16_tb.sdramddr3_0.data_task: at time 20826250.0 ps ERROR: tDSS violation on DQS   bit           0

and ModelSim's waveform view will center the waveform display at that point.

I've attached what I need to see from you.
You also may need to Zoom into the error if it appears to match my Altera example.

(Note that I already see your simple mistake, but let's see if you can figure it out...)
« Last Edit: September 06, 2022, 09:15:04 pm by BrianHG »
 

Offline SpacedCowboy

  • Frequent Contributor
  • **
  • Posts: 292
  • Country: gb
  • Aging physicist
Re: BrianHG_DDR3_CONTROLLER open source DDR3 controller. NEW v1.60.
« Reply #240 on: September 06, 2022, 11:26:13 pm »
So I can see a few differences (see 'compare-gowin-altera) ...

  • The Gowin DDR3_DQS_p is out of phase with the Altera one (high-low-high-... rather than low-high-low...
  • RDQS_ph (and presumably _l but it's hard to tell) is a half-clock later in the Gowin simulation
  • RDQ_{l,h} are a half-clock later too

The last two are on the read-path, and since we're seeing problems with write-ops, I doubt it's that. The test-bench started running to completion when I inverted the phase of the signals for DQS and DQ, so I'd really like to keep that unless we *must* revert it...

Ah-ha. I forgot to change around the DQS clock-values (ie: setting D0/D1) when I inverted the polarity of the input-path DQS. See DQS-generation.

(runs simulation)

Yep. That passes with flying colours!
  • The only xxxx's are the expected ones
  • No more tDSS violations
  • We start storing data with 0123, just like the Altera simulation

Log attached, but this looks pretty much perfect to me :)

Thanks again for all the help Brian :)
 

Offline BrianHGTopic starter

  • Super Contributor
  • ***
  • Posts: 7857
  • Country: ca
Re: BrianHG_DDR3_CONTROLLER open source DDR3 controller. NEW v1.60.
« Reply #241 on: September 06, 2022, 11:51:39 pm »
The DDR3_DQS_p output during a write must match the DDR3_CK_p phase...
See attachment 1.

In fact, the hard wired D0/D1 inputs to the DDR_out primitive should match clock DDR3_CK's inputs.
(Your above snapshot still has it backwards, though your swap in the code should have fixed it, so I assume it's an old screenshot.)

I hope you kept everything as a positive edge clock...

Now you need to inspect the read data coming out in my second snapshot to verify a match between Altera and Gowin.  Also, inspect the 2 different 'Transcript logs' and verify that all the read and write match perfectly.  There might only be a difference in the time index location as Gowin's power-up tuning time may be different.

Continue scrolling to the right to verify a match between Gowin and Altera.

If everything is ok, clean up you code and add your necessary comments.
Remember that we may still have a bad value for the read offset as the simulator allows for large discrepancies.  I bet the true figure will be 2, not 3 once in hardware.

Next, place into your hardware and let's see what needs to be done with the .sdc file.

« Last Edit: September 07, 2022, 12:00:40 am by BrianHG »
 

Offline SpacedCowboy

  • Frequent Contributor
  • **
  • Posts: 292
  • Country: gb
  • Aging physicist
Re: BrianHG_DDR3_CONTROLLER open source DDR3 controller. NEW v1.60.
« Reply #242 on: September 07, 2022, 01:05:39 am »
So the clock generators do match up - DDR3 clock is:
Code: [Select]
            ODDR gowin_ck_ddr_inst
                (
                .Q0(gowin_clock),       // Send clock via LVDS buffer
                .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 input clock
                );
           
            TLVDS_OBUF gowin_ck_lvds_inst
                (
                .I(gowin_clock),
                .O(DDR3_CK_p[x]),
                .OB(DDR3_CK_n[x])   
                );

and DQS clock is
Code: [Select]
            ODDR gowin_dqs_oddr_inst 
                (
                .Q0(gowin_dqs_out),             // ODDR -> IVDS
                .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
                );

When you say "The DDR3_DQS_p output during a write must match the DDR3_CK_p phase..." - both signals are synced so they change hi->lo->hi at the same time, is that what you mean here by phase, or are you talking about timings of sequences relative to each other ?

As for reads, I think I'm matching your results here, see 'tb-reads'. It looks the same to me, and the read-values from the log-files seem to match up as far as I can tell. I probably ought to write a script to compare them, to account for human error...

I just ran the Gowin simulation without any extra delays(see 'gowin-without-delays') and it worked fine - the transcript logs are the same (modulo timestamps) and all the reads & writes produce the expected values. So maybe making other fixes has abrogated any need for that delay, and the signal timings actually look more like that of the Altera snapshot you made (regarding the 'phase' of DDR3_DQS_p and DDR3_CK_p for example).



 

Offline BrianHGTopic starter

  • Super Contributor
  • ***
  • Posts: 7857
  • Country: ca
Re: BrianHG_DDR3_CONTROLLER open source DDR3 controller. NEW v1.60.
« Reply #243 on: September 07, 2022, 02:26:42 am »
When you say "The DDR3_DQS_p output during a write must match the DDR3_CK_p phase..." - both signals are synced so they change hi->lo->hi at the same time, is that what you mean here by phase, or are you talking about timings of sequences relative to each other ?
Just look at my above screenshot ' hint1.png '.  The 2 clock output phases I highlighted are a perfect match.
They both go high and low at the same time.

Next, see if you can get your hardware working with my RS232 debugger reference design.
IE: you will need a PC with a LVTTL <-> RS232(USB) adapter and my RS232_debugger.exe program.

Quote
I just ran the Gowin simulation without any extra delays(see 'gowin-without-delays') and it worked fine - the transcript logs are the same (modulo timestamps) and all the reads & writes produce the expected values. So maybe making other fixes has abrogated any need for that delay, and the signal timings actually look more like that of the Altera snapshot you made (regarding the 'phase' of DDR3_DQS_p and DDR3_CK_p for example).
This is the nature of using Modelsim, my code resets the read reference in the middle until the true DQS coming from the DDR3 model is received.  So, a too early/small a value will work.  When real hardware and testing the DDR3 running at 300MHz, 400MHz, 500MHz and 600MHz, you may need that 1 or 2 extra delays, otherwise powerup initialization wont always work properly.  On the other hand, if you added 2 to the Altera simulation, it probably will never power-up as the read will begin too late.
« Last Edit: September 07, 2022, 02:35:05 am by BrianHG »
 

Offline SpacedCowboy

  • Frequent Contributor
  • **
  • Posts: 292
  • Country: gb
  • Aging physicist
Re: BrianHG_DDR3_CONTROLLER open source DDR3 controller. NEW v1.60.
« Reply #244 on: September 07, 2022, 02:37:04 am »
Ok, so the signaling in ‘gowin-without-delays.png’ is a match for ‘hint1.png’ as far as I can see. Since the no-extra-delay code seems to work, I’m going to keep that for now.

[edit] crossed over in time there. I see, well, I’ll keep the code in, but define the delay to be zero, and when/if it needs to be changed, we can take a view then.
« Last Edit: September 07, 2022, 02:39:39 am by SpacedCowboy »
 

Offline BrianHGTopic starter

  • Super Contributor
  • ***
  • Posts: 7857
  • Country: ca
Re: BrianHG_DDR3_CONTROLLER open source DDR3 controller. NEW v1.60.
« Reply #245 on: September 08, 2022, 07:20:55 pm »
Did Gowin compile the design?
 

Offline SpacedCowboy

  • Frequent Contributor
  • **
  • Posts: 292
  • Country: gb
  • Aging physicist
Re: BrianHG_DDR3_CONTROLLER open source DDR3 controller. NEW v1.60.
« Reply #246 on: September 08, 2022, 08:54:13 pm »
Haven't had a chance to do anything much more. Life getting in the way - had an MRI scan yesterday... Work is busy too, so it'll be this weekend probably before I start trying to get the DDR3 to compile on the chip.

The hardware turned up, and I can see it in the dev environment - needed to download a separate a chip-specific update for the programmer, but it appears to work now.  I got the "dev-board" to go with the SO-DIMM so I have access to easy pins to make an RS232 interface with.

The board itself is basically just the FPGA, some DDR3, and the power circuitry - which is perfect for me - no long list of buttons and switches with a paltry 10 GPIO!
 

Offline BrianHGTopic starter

  • Super Contributor
  • ***
  • Posts: 7857
  • Country: ca
Re: BrianHG_DDR3_CONTROLLER open source DDR3 controller. NEW v1.60.
« Reply #247 on: September 09, 2022, 02:43:58 am »
You will need to verify all the DDR3 specification parameters first.

These from this source file:
PHY only <-> RS232-Debugger
 
The following users thanked this post: SpacedCowboy

Offline SpacedCowboy

  • Frequent Contributor
  • **
  • Posts: 292
  • Country: gb
  • Aging physicist
Re: BrianHG_DDR3_CONTROLLER open source DDR3 controller. NEW v1.60.
« Reply #248 on: September 09, 2022, 11:20:17 pm »
Slowly starting to make a start on writing something the Gowin IDE will like.

You will need to verify all the DDR3 specification parameters first.

These from this source file:
PHY only <-> RS232-Debugger

Yep, it's a 1Gbit, 64Mbx16 SK-hynix HSTQ1G63EFR-PBC which isn't exactly mainstream, but as long as it works...

The Gowin example code has 14 address pins mapped out, but the datasheet for the chip says it uses A0->A12. I'm assuming they mapped out the pins on the board so they could replace with a x8 or a 2Gbit chip easier. The -PBC indicates its CL6 at 800MHz and CL7 at 1066MHz. It claims to be DDR3-1600 11-11-11.

Anyway, in 'top.sv' I have
Code: [Select]
// Use 1066/187E, 1333/-15E, 1600/-125, 1866/-107, or 2133/093.
parameter string     DDR3_SPEED_GRADE        = "-125",

// Use 0,1,2,4 or 8.  (0=512mb) Caution: Must be correct as ram chip size
// affects the tRFC REFRESH period.
parameter int        DDR3_SIZE_GB            = 1,

// Use 8 or 16.  The width of each DDR3 ram chip.
parameter int        DDR3_WIDTH_DQ           = 16,

// 1, 2, or 4 for the number of DDR3 RAM chips.
parameter int        DDR3_NUM_CHIPS          = 1,

// Select the number of DDR3_CLK & DDR3_CLK# output pairs. 
// Add 1 for every DDR3 Ram chip.
// These are placed on a DDR DQ or DDR CK# IO output pins.
parameter int        DDR3_NUM_CK             = (DDR3_NUM_CHIPS),

// Use for the number of bits to address each row.
parameter int        DDR3_WIDTH_ADDR         = 13,  // A0-12

// Use for the number of bits to address each bank.
parameter int        DDR3_WIDTH_BANK         = 3,

// Use for the number of bits to address each column.
parameter int        DDR3_WIDTH_CAS          = 10,   // A0-9


The Gowin verilog compiler is complaining (see attached, just warnings) about these lines because of the initialization. Is that initialization just for simulation purposes, or do I have to worry about these ?
 

Offline BrianHGTopic starter

  • Super Contributor
  • ***
  • Posts: 7857
  • Country: ca
Re: BrianHG_DDR3_CONTROLLER open source DDR3 controller. NEW v1.60.
« Reply #249 on: September 09, 2022, 11:32:55 pm »
Ahhh, Gowin doesn't allow default power-up values specified in the IO port list.
You need to do it the old fashioned way, define the ports, then externally declare that those nets are logic & their initial power-up value.
(Maybe Gowin's compiler may have a 'SystemVerilog default version 20xx' flag which may allow such declarations.)

Anyways, try re-compiling with the ' =0, ' removed.  It should still work.
Even simulating should work with the annoying red #'hxx just after power-up until the ports get their first assignment.
 


Share me

Digg  Facebook  SlashDot  Delicious  Technorati  Twitter  Google  Yahoo
Smf