From b7b8eb3a1420e3e4ad10bac0832c545d3a40f35b Mon Sep 17 00:00:00 2001
From: Tristan Gingold <tristan.gingold@cern.ch>
Date: Wed, 15 Dec 2021 13:45:41 +0100
Subject: [PATCH] hydra_ram: handle corrections

---
 hdl/rtl/hydra_core.vhd        | 92 +++++++++++++++++++++--------------
 hdl/rtl/hydra_ram.vhd         | 48 +++++++++++++-----
 hdl/top/sf2-test/sf2_test.vhd |  2 +-
 3 files changed, 91 insertions(+), 51 deletions(-)

diff --git a/hdl/rtl/hydra_core.vhd b/hdl/rtl/hydra_core.vhd
index 141a4f8..f5d6972 100644
--- a/hdl/rtl/hydra_core.vhd
+++ b/hdl/rtl/hydra_core.vhd
@@ -85,6 +85,7 @@ architecture arch of hydra_core is
   signal im_data, im1_data  : std_logic_vector(31 downto 0);
   signal im_rd, im_valid : std_logic;
   signal im1_done, im1_dm_en, im1_err : std_logic;
+  signal im2_err, im2_valid : std_logic;
 
   signal dm_addr, dm_data_s, dm_data_l                            : std_logic_vector(31 downto 0);
   signal dm_data_select                                           : std_logic_vector(3 downto 0);
@@ -94,7 +95,7 @@ architecture arch of hydra_core is
   signal reg_dm_data_select         : std_logic_vector(3 downto 0);
   signal reg_dm_load, reg_dm_store  : std_logic;
 
-  signal dm_cycle_in_progress, reg_dm_is_wishbone : std_logic;
+  signal dm_cycle_in_progress, reg_dm_is_wishbone, reg_dm_is_iram : std_logic;
 
   signal dm_mem_rdata, dm_wb_rdata : std_logic_vector(31 downto 0);
   signal dm_wb_write, dm_select_wb : std_logic;
@@ -137,6 +138,7 @@ begin
       dbg_mbx_write_i  => '0',
       dbg_mbx_data_o   => open);
 
+  --  Add registers on uRV data bus
   process (clk_sys_i)
   begin
     if rising_edge(clk_sys_i) then
@@ -165,17 +167,21 @@ begin
       clk_i => clk_sys_i,
       rst_n_i => rst_n_i,
 
+      --  uRV data read from iRAM
       r1_addr_i => reg_dm_addr(g_IRAM_LOG_SIZE - 1 downto 2),
       r1_en_i   => im1_dm_en,
       r1_data_o => im1_data,
       r1_done_o => im1_done,
       r1_err_o  => im1_err,
 
+      --  uRV instructions
       r2_addr_i => im_addr(g_IRAM_LOG_SIZE - 1 downto 2),
-      r2_en_i   => im_rd, --rst_n_i,
+      r2_en_i   => im_rd,
       r2_data_o => im_data,
-      r2_done_o => im_valid,
+      r2_done_o => im2_valid,
+      r2_err_o  => im2_err,
 
+      --  iRAM initialization
       waddr_i => iram_addr,
       we_i => iram_we,
       wdata_i => iram_data,
@@ -184,12 +190,14 @@ begin
       scrubber_period_i => x"0010"
     );
 
+  im_valid <= im2_valid and not im2_err;
+
   -- 1st MByte of the mem is the RAM
   -- 1st 64KB is the IRAM.
   -- 2nd 64KB is the DRAM
   reg_dm_is_wishbone <= '1' when reg_dm_addr(31 downto 20) /= x"000" else '0';
-  dm_data_l <= dm_wb_rdata when dm_select_wb = '1' else
-               im_data when reg_dm_addr(16) = '0' else
+  reg_dm_is_iram <= '1' when reg_dm_addr(16) = '0' and reg_dm_is_wishbone = '0' else '0';
+  dm_data_l <= dm_wb_rdata when (dm_select_wb = '1' or reg_dm_is_iram = '1') else
                dm_mem_rdata;
 
   p_ram: process (clk_sys_i)
@@ -198,8 +206,12 @@ begin
     variable addr : natural range dram'range;
   begin
     if rising_edge(clk_sys_i) then
-      addr := to_integer(unsigned(reg_dm_addr(g_DRAM_LOG_SIZE - 1 downto 2)));
-      dm_mem_rdata <= dram(addr);
+      if reg_dm_load = '1' then
+        addr := to_integer(unsigned(reg_dm_addr(g_DRAM_LOG_SIZE - 1 downto 2)));
+        dm_mem_rdata <= dram(addr);
+      else
+        dm_mem_rdata <= (others => 'X');
+      end if;
       if reg_dm_store = '1' and reg_dm_addr(16) = '1' and reg_dm_is_wishbone = '0' then
         for i in 0 to 3 loop
           if reg_dm_data_select (i) = '1' then
@@ -210,10 +222,14 @@ begin
     end if;
   end process;
 
+  -- Data bus
   -- Wishbone bus arbitration / internal RAM access
   p_wishbone_master : process(clk_sys_i)
   begin
     if rising_edge(clk_sys_i) then
+      dm_store_done <= '0';
+      dm_load_done  <= '0';
+
       if rst_n_i = '0' then
         dwb_out.cyc          <= '0';
         dwb_out.stb          <= '0';
@@ -222,62 +238,64 @@ begin
         dwb_out.we           <= '0';
         dwb_out.dat          <= (others => '0');
         dm_cycle_in_progress <= '0';
-        dm_load_done         <= '0';
-        dm_store_done        <= '0';
         dm_select_wb         <= '0';
       else
-
         if dm_cycle_in_progress = '0' then
+          --  Data bus was idle.
+          dm_wb_write  <= reg_dm_store;
           if reg_dm_is_wishbone = '0' then
             --  Internal access
             dm_select_wb  <= '0';
             if reg_dm_store = '1' then
-              dm_load_done  <= '0';
               dm_store_done <= '1';
             elsif reg_dm_load = '1' then
-              dm_load_done  <= '1';
-              dm_store_done <= '0';
-            else
-              dm_store_done <= '0';
-              dm_load_done  <= '0';
+              if reg_dm_is_iram = '1' then
+                --  Need to wait for the done (may need extra cycles due to ECC)
+                dm_cycle_in_progress <= '1';
+              else
+                --  Immediate answer
+                dm_load_done  <= '1';
+              end if;
             end if;
           else
             --  Wishbone access
+            dm_select_wb <= '1';
             if reg_dm_load = '1' or reg_dm_store = '1' then
               dwb_out.cyc          <= '1';
               dwb_out.stb          <= '1';
               dwb_out.we           <= reg_dm_store;
-              dm_wb_write          <= reg_dm_store;
               dwb_out.adr          <= reg_dm_addr;
               dwb_out.dat          <= reg_dm_data_s;
               dwb_out.sel          <= reg_dm_data_select;
-              dm_load_done         <= '0';
-              dm_store_done        <= '0';
               dm_cycle_in_progress <= '1';
-            else
-              dm_store_done        <= '0';
-              dm_load_done         <= '0';
-              dm_cycle_in_progress <= '0';
             end if;
           end if;
         else
-          --  Wishbone transfer in progress.
-          if dwb_i.stall = '0' then
-            dwb_out.stb <= '0';
-          end if;
+          --  Transfer in progress
+          if dm_select_wb = '1' then
+            --  Wishbone transfer in progress.
+            if dwb_i.stall = '0' then
+              dwb_out.stb <= '0';
+            end if;
 
-          if dwb_i.ack = '1' then
-            if dm_wb_write = '0' then
-              dm_wb_rdata  <= f_x_to_zero(dwb_i.dat);
-              dm_select_wb <= '1';
+            if dwb_i.ack = '1' then
+              if dm_wb_write = '0' then
+                dm_wb_rdata  <= f_x_to_zero(dwb_i.dat);
+                dm_load_done <= '1';
+              else
+                dm_store_done <= '1';
+              end if;
+
+              dm_cycle_in_progress <= '0';
+              dwb_out.cyc          <= '0';
+            end if;
+          else
+            --  IRAM/DRAM transfer in progress
+            if im1_done = '1' and im1_err = '0' then
+              dm_wb_rdata <= im1_data;
               dm_load_done <= '1';
-            else
-              dm_store_done <= '1';
-              dm_select_wb  <= '0';
+              dm_cycle_in_progress <= '0';
             end if;
-
-            dm_cycle_in_progress <= '0';
-            dwb_out.cyc          <= '0';
           end if;
         end if;
       end if;
diff --git a/hdl/rtl/hydra_ram.vhd b/hdl/rtl/hydra_ram.vhd
index 7e41dca..5320988 100644
--- a/hdl/rtl/hydra_ram.vhd
+++ b/hdl/rtl/hydra_ram.vhd
@@ -80,15 +80,15 @@ architecture arch of hydra_ram is
 
   signal recc, rsyndrome : std_logic_vector(6 downto 0);
   signal rerr, rerr_one : std_logic;
-  signal r1_done, n_r1_done : std_logic;
-  signal r2_done, n_r2_done : std_logic;
+  signal r1_done, n_r1_done, r1_done_d : std_logic;
+  signal r2_done, n_r2_done, r2_done_d : std_logic;
   signal wdone, n_wdone : std_logic;
 
   signal n_ecc_one, n_ecc_fatal : std_logic;
   signal last_raddr, n_last_raddr, scrub_addr : std_logic_vector(g_RAM_LOG_SIZE - 1 downto 2);
   
   signal scrub_counter : unsigned(15 downto 0);
-  signal scrub_rd, scrub_done, n_scrub_done : std_logic;
+  signal scrub_rd, scrub_done, scrub_done_d, n_scrub_done : std_logic;
 
   type state_t is (S_READ, S_REWRITE);
   signal state, n_state : state_t;
@@ -97,13 +97,27 @@ begin
   p_ram: process (clk_i)
   is
     variable iram : t_ram39_type(0 to RAM_WSIZE - 1);
+    variable d : std_logic_vector(38 downto 0);
+    variable err : std_logic_vector(38 downto 0) := (0 => '1', others => '0');
+    variable sim_cnt : natural;
   begin
     if rising_edge(clk_i) then
       if wen = '1' then
         iram (to_integer(unsigned(waddr))) := wdata_ecc;
       end if;
       if ren = '1' then
-        rdata_ecc <= iram (to_integer(unsigned(raddr)));
+        d := iram (to_integer(unsigned(raddr)));
+
+        --  Simulate errors.
+        if sim_cnt < 7 then
+          sim_cnt := sim_cnt + 1;
+        else
+          sim_cnt := 0;
+          d := d xor err;
+          err := err(err'left - 1 downto 0) & err(err'left);
+        end if;
+
+        rdata_ecc <= d;
       else
         rdata_ecc <= (others => 'X');
       end if;
@@ -133,7 +147,7 @@ begin
         scrub_addr <= (others => '0');
         scrub_rd <= '0';
       else
-        if scrub_done = '1' then
+        if scrub_done = '1' and rerr = '0' then
           scrub_counter <= unsigned(scrubber_period_i);
           scrub_addr <= std_logic_vector(unsigned(scrub_addr) + 1);
           scrub_rd <= '0';
@@ -148,9 +162,9 @@ begin
     end if;
   end process;
 
-  p_ctrl: process (state, r1_done, r2_done, rerr, rdata_ecc, rsyndrome, last_raddr,
-                   r1_en_i, r1_addr_i, we_i, waddr_i, wdata_i, wforce_i,
-                   r2_en_i, r2_addr_i, scrub_rd, scrub_addr, scrub_done)
+  p_ctrl: process (state, r1_done, r2_done, rerr, rerr_one, rdata_ecc, rsyndrome, last_raddr, recc,
+                   r1_en_i, r1_addr_i, we_i, waddr_i, wdata_i, wforce_i, r1_done_d, r2_done_d,
+                   r2_en_i, r2_addr_i, scrub_rd, scrub_addr, scrub_done, scrub_done_d, rst_n_i)
   begin
     wen <= '0';
     waddr <= (others => 'X');
@@ -169,7 +183,7 @@ begin
 
     case state is
       when S_READ =>
-        if (r2_done = '1' or r1_done = '1') and rerr = '1' then
+        if (r2_done = '1' or r1_done = '1' or scrub_done = '1') and rerr = '1' then
           --  There was an error on the last access.
           --  Write to fix it.
           if rerr_one = '1' then
@@ -210,8 +224,8 @@ begin
           ren <= '1';
           n_r2_done <= '1';
           n_last_raddr <= r2_addr_i;
-        else
-          -- scrub if idle.
+        elsif rst_n_i = '1' then
+          -- scrub if idle (but not during reset)
           raddr <= scrub_addr;
           ren <= '1';
           n_scrub_done <= '1';
@@ -221,8 +235,11 @@ begin
         --  Reread
         raddr <= last_raddr;
         ren <= '1';
-        n_r1_done <= '1';
+        n_r1_done <= r1_done_d;
+        n_r2_done <= r2_done_d;
+        n_scrub_done <= scrub_done_d;
         n_last_raddr <= last_raddr;
+        n_state <= S_READ;
     end case;
   end process;
 
@@ -231,18 +248,23 @@ begin
     if rising_edge(clk_i) then
       if rst_n_i = '0' then
         r1_done <= '0';
+        r1_done_d <= '0';
         r2_done <= '0';
+        r2_done_d <= '0';
         scrub_done <= '0';
+        scrub_done_d <= '0';
         state <= S_READ;
         last_raddr <= (others => 'X');
 
         ecc_one_o <= '0';
         ecc_fatal_o <= '0';
       else
+        r1_done_d <= r1_done;
         r1_done <= n_r1_done;
+        r2_done_d <= r2_done;
         r2_done <= n_r2_done;
+        scrub_done_d <= scrub_done;
         scrub_done <= n_scrub_done;
-        assert ((r2_done or scrub_done) and rerr) = '0' severity error; --  TODO
 
         state <= n_state;
         last_raddr <= n_last_raddr;
diff --git a/hdl/top/sf2-test/sf2_test.vhd b/hdl/top/sf2-test/sf2_test.vhd
index 6107d77..f580898 100644
--- a/hdl/top/sf2-test/sf2_test.vhd
+++ b/hdl/top/sf2-test/sf2_test.vhd
@@ -69,7 +69,7 @@ architecture behav of sf2_test is
   signal iram_we : std_logic;
   signal iram_data : std_logic_vector(31 downto 0);
 
-  signal cpu_rst_n : std_logic;
+  signal cpu_rst_n : std_logic := '0';
 
   signal dwb_out : t_wishbone_master_out;
   signal dwb_in  : t_wishbone_master_in;
-- 
GitLab