sdram: I/O timing tweaks, hopefully for the better...

Let the clock arrive at the SDRAM a bit sooner than 180° out of
phase. Use an explicit I/O delays to retard the clock otherwise using
the same phase as the internal clock (as opposed to the opposite.)

Combine with an explicit I/O delay on the input and latch directly
into the target registers. As a side benefit, we get the data one
cycle earlier.

This seems to be far more reliable on at least my board. Hopefully on
others as well.

fpga/max80.sdc

@@ -27,12 +27,13 @@ set sdram_clk [get_clocks pll|*|clk\[0\]]
 set cpu_clk   [get_clocks pll|*|clk\[1\]]
 set vid_clk   [get_clocks pll|*|clk\[2\]]
 
-# The SDRAM ack signal is delayed by a minimum of 1 SDRAM plus one CPU
-# cycle by the sdram_mem_ready logic, so make it a multicycle path.
-#set_multicycle_path -from [get_registers sdram:sdram|rd1*] \
-#    -to [get_registers picorv32:cpu|*] -start -setup 3
-#set_multicycle_path -from [get_registers sdram:sdram|rd1*] \
-#    -to [get_registers picorv32:cpu|*] -start -hold 2
+# SDRAM I/O constraints
+# set_max_skew -to [get_ports sr_*] 0.500ns
+set sr_data_out [remove_from_collection [get_ports sr_*] sr_clk]
+set sr_data_in  [get_ports sr_dq\[*\]]
+set_max_skew -to [get_ports sr_*] 0.100ns
+# set_output_delay -clock $sdram_clk 1.500ns [get_ports sr_clk]
+set_input_delay  -clock $sdram_clk 0.500ns  $sr_data_in
 
 # Anything that feeds into a synchronizer is by definition
 # asynchronous, but encode it as allowing multicycle of one

fpga/output_files/max80.map

@@ -10,7 +10,7 @@ Quad-Serial configuration device dummy clock cycle: 8
 
 Notes:
 
-- Data checksum for this conversion is 0xF76C999F
+- Data checksum for this conversion is 0xF76A3AFA
 
 - All the addresses in this file are byte addresses
 

fpga/sdram.sv

@@ -160,8 +160,8 @@ module sdram
    // isn't clear it buys us a whole lot.
    ddio_out sr_clk_out (
 			.aclr ( 1'b0 ),
-			.datain_h ( 1'b0 ),
-			.datain_l ( 1'b1 ),
+			.datain_h ( 1'b1 ),
+			.datain_l ( 1'b0 ),
 			.outclock ( clk ),
 			.dataout ( sr_clk )
 			);
@@ -179,11 +179,6 @@ module sdram
    reg			    dram_d_en; // Drive data out
    assign		    sr_dq = dram_d_en ? dram_d : 16'hzzzz;
 
-   // I/O cell input register for SDRAM data
-   reg [15:0]		    dram_q;
-   always @(posedge clk)
-     dram_q <= sr_dq;
-
    // State machine and counters
    reg [t_refi_lg2-2:0]	      rfsh_ctr;  // Refresh timer
    wire			      rfsh_ctr_msb = rfsh_ctr[t_refi_lg2-2];
@@ -202,12 +197,12 @@ module sdram
  	 st_init_mrd,		// MRD register write during initialization
 	 st_idle,		// Idle state: all banks precharged
 	 st_rfsh,
-	 st_rd,
-	 st_wr,
+	 st_rd_wr,
 	 st_pre_idle
    } state_t;
    state_t state = st_reset;
-
+   reg 		is_write;
+   
    always @(posedge clk or negedge rst_n)
      if (~rst_n)
        begin
@@ -264,6 +259,7 @@ module sdram
 	  op_zero       <= 1'b0;
 	  init_op_ctr   <= 2'b00;
 	  state         <= st_reset;
+	  is_write      <= 1'bx;
 
 	  rack0         <= 1'b0;
 	  rready0       <= 1'b1;
@@ -275,6 +271,7 @@ module sdram
 
 	  wdata_q       <= 32'hxxxx_xxxx;
 	  be_q          <= 4'bxxxx;
+	  col_addr      <= 10'hxxx;
        end
      else
        begin
@@ -284,12 +281,12 @@ module sdram
 	  // Note: dram_ba are preserved
 	  dram_a        <= 13'hxxxx;
 	  dram_dqm      <= 2'b00;
-	  dram_d        <= 16'hxxxx;
+	  dram_d        <= 16'haaaa;
 	  dram_cmd      <= cmd_nop;
 
 	  dram_d_en     <= 1'b1; // Don't float except during read
 
-	  if (state != st_rd && state != st_wr)
+	  if (state != st_rd_wr)
 	    begin
 	       rack0 <= 1'b0;
 	       wack0 <= 1'b0;
@@ -344,6 +341,10 @@ module sdram
 
 	    st_idle:
 	      begin
+		 is_write <= 1'bx;
+		 be_q     <= 4'bxxxx;
+		 wdata_q  <= 32'hxxxx_xxxx;
+
 		 // A data transaction starts with ACTIVE command;
 		 // a refresh transaction starts with REFRESH.
 		 // Port 0 has the highest priority, then
@@ -351,6 +352,8 @@ module sdram
 		 // is started opportunistically if nothing is
 		 // pending and the refresh counter is no less than
 		 // half expired.
+		 dram_d <= 16'hbbbb;
+		 
 		 casez ( {rrq0|wrq0, rrq1|wrq1, rfsh_prio} )
 		   4'b1???:
 		     begin
@@ -361,17 +364,18 @@ module sdram
 			col_addr     <= a0[9:0];
 			if ( wrq0 )
 			  begin
-			     state   <= st_wr;
-			     wack0   <= 1'b1;
-			     wdata_q <= {16'hxxxx, wd0, wd0};
-			     be_q    <= {2'b00, a0[0], ~a0[0]};
+			     state    <= st_rd_wr;
+			     wack0    <= 1'b1;
+			     wdata_q  <= {16'hxxxx, wd0, wd0};
+			     be_q     <= {2'b00, a0[0], ~a0[0]};
+			     is_write <= 1'b1;
 			  end
 			else
 			  begin
-			     state   <= st_rd;
-			     rack0   <= 1'b1;
-			     rready0 <= 1'b0;
-			     be_q    <= 4'b1111;
+			     state    <= st_rd_wr;
+			     rack0    <= 1'b1;
+			     rready0  <= 1'b0;
+			     is_write <= 1'b0;
 			  end
 		     end
 		   4'b010?:
@@ -383,17 +387,18 @@ module sdram
 			col_addr     <= { a1[9:2], 2'b00 };
 			if ( wrq1 )
 			  begin
-			     state   <= st_wr;
-			     wack1   <= 1'b1;
-			     wdata_q <= wd1;
-			     be_q    <= wstrb1;
+			     state    <= st_rd_wr;
+			     wack1    <= 1'b1;
+			     wdata_q  <= wd1;
+			     be_q     <= wstrb1;
+			     is_write <= 1'b1;
 			  end
 			else
 			  begin
-			     state   <= st_rd;
-			     rack1   <= 1'b1;
-			     rready1 <= 1'b0;
-			     be_q    <= 4'b1111;
+			     state    <= st_rd_wr;
+			     rack1    <= 1'b1;
+			     rready1  <= 1'b0;
+			     is_write <= 1'b0;
 			  end
 		     end
 		   4'b0?1?, 4'b0001:
@@ -416,55 +421,71 @@ module sdram
 		   state <= st_idle;
 	      end
 
-	    st_rd, st_wr:
+	    st_rd_wr:
 	      begin
-		 dram_d_en <= (state == st_wr);
+		 dram_d_en <= is_write;
+		 dram_dqm  <= {2{is_write}};
+		 dram_d <= 16'hcccc;
 
 		 // Commands
 		 //
 		 // This assumes:
-		 // tRCD = 3
-		 // rRRD = 2
-		 // CL   = 3
+		 // tRCD =  3
+		 // rRRD =  2
+		 // CL   =  3
 		 // tRC  = 10
+		 // tRAS =  7
+		 // tRP  =  3
 		 //
 		 case (op_cycle)
 		   2: begin
-		      dram_a[10]   <= 1'b1; // Auto precharge
+		      dram_a[10]   <= 1'b0; // No auto precharge
 		      dram_a[8:0]  <= col_addr[9:1];
-		      dram_cmd     <= (state == st_wr) ? cmd_wr : cmd_rd;
+		      dram_cmd     <= is_write ? cmd_wr : cmd_rd;
 		      dram_d       <= wdata_q[15:0];
-		      wdata_q      <= { 16'hxxxx, wdata_q[31:16] };
-		      dram_dqm     <= ~be_q[1:0];
-		      be_q         <= { 2'bxx, be_q[3:2] };
+		      dram_dqm     <= {2{is_write}} & ~be_q[1:0];
+		      wdata_q      <= { 16'hdddd, wdata_q[31:16] };
+		      be_q         <= { 2'hxx, be_q[3:2] };
 		   end
 		   3: begin
 		      dram_d       <= wdata_q[15:0];
-		      dram_dqm     <= ~be_q[1:0];
-		      wdata_q      <= 32'hxxxx_xxxx;
+		      dram_dqm     <= {2{is_write}} & ~be_q[1:0];
+		      wdata_q      <= { 16'heeee, wdata_q[31:16] };
 		      be_q         <= 4'bxxxx;
 		   end
+		   6: begin
+		      // Earliest legal cycle to precharge
+		      // It seems auto precharge violates tRAS(?)
+		      // so do it explicitly.
+		      dram_a[10]   <= 1'b1; // One bank
+		      dram_cmd     <= cmd_pre;
+		   end
 		   // CL+2 cycles after the read command
+		   // The +2 accounts for internal and I/O delays
 		   7: begin
 		      if (rack0)
-			rd0 <= col_addr[0] ? dram_q[15:8] : dram_q[7:0];
+			rd0 <= col_addr[0] ? sr_dq[15:8] : sr_dq[7:0];
 		      rready0 <= rready0 | rack0;
 		      if (rack1)
-			rd1[15:0] <= dram_q;
+			rd1[15:0] <= sr_dq;
 		   end
 		   8: begin
 		      if (rack1)
-			rd1[31:16] <= dram_q;
+			rd1[31:16] <= sr_dq;
 		      rready1 <= rready1 | rack1;
-		      // Last cycle before tRC is a separate state
-		      // so that rack/wack will be cleared
+
 		      state <= st_pre_idle;
 		   end
 		 endcase // case (op_cycle)
-	      end // case: st_rd, st_wr
+	      end // case: st_rd_wr
 
 	    st_pre_idle:
 	      begin
+		 // Last cycle before tRC is a separate state
+		 // so that rack/wack will be cleared
+
+		 dram_d_en <= is_write;
+		 dram_dqm  <= {2{is_write}};
 		 state <= st_idle;
 	      end
 	  endcase // case(state)

fw/hello.c

@@ -37,7 +37,8 @@ static inline void write_check(volatile uint32_t *p, uint32_t v)
 
 #define ERROR_RATELIMIT 8
 
-#define SOME_NUMBER 0x78dacecb
+#define A 0x45c11ba1		/* Arbitrary odd constant */
+#define B 0x78dacecb		/* Arbitrary constant */
 
 static void test_sdram(void)
 {
@@ -61,7 +62,7 @@ static void test_sdram(void)
 	write_check(p, ~0);
 	write_check(p, ~w);
 	write_check(p, w);
-	write_check(p, w + SOME_NUMBER);
+	write_check(p, A*w + B);
 
 	if (!(++n & 0x3ffff)) {
 	    CONSOLE = err_char;
@@ -78,7 +79,7 @@ static void test_sdram(void)
 	uint32_t a = w + SDRAM_ADDR;
 	volatile uint32_t *p = (volatile uint32_t *)a;
 
-	data_check(p, w + SOME_NUMBER);
+	data_check(p, A*w + B);
 
 	if (!(++n & 0x3ffff)) {
 	    CONSOLE = err_char;