[usbdev] Switch to single port packet buffer

Switch to SPRAM implementation of the usbdev packet buffer. Collisions are very infrequent and we can just delay the TL-UL access. Retain the read data on the usbdev side because the packet engine operates on bytes and relies upon rdata remaining static throughout the use of the 32-bit read word. Signed-off-by: Adrian Lees <[email protected]>
lowRISC · Jan 17, 2024 · 8daa9cb · 8daa9cb
1 parent d53236c
commit 8daa9cb
Show file tree

Hide file tree

Showing 13 changed files with 153 additions and 121 deletions.
diff --git a/hw/ip/usbdev/data/usbdev.hjson b/hw/ip/usbdev/data/usbdev.hjson
@@ -145,8 +145,8 @@
       struct:  "logic",
       width:   "1"
     },
-    { struct:  "ram_2p_cfg",
-      package: "prim_ram_2p_pkg",
+    { struct:  "ram_1p_cfg",
+      package: "prim_ram_1p_pkg",
       type:    "uni",
       name:    "ram_cfg",
       act:     "rcv"

diff --git a/hw/ip/usbdev/rtl/usbdev.sv b/hw/ip/usbdev/rtl/usbdev.sv
@@ -67,7 +67,7 @@ module usbdev
   output logic       usb_ref_pulse_o,
 
   // memory configuration
-  input prim_ram_2p_pkg::ram_2p_cfg_t ram_cfg_i,
+  input prim_ram_1p_pkg::ram_1p_cfg_t ram_cfg_i,
 
   // Interrupts
   output logic       intr_pkt_received_o, // Packet received
@@ -121,15 +121,17 @@ module usbdev
   tlul_pkg::tl_h2d_t tl_sram_h2d;
   tlul_pkg::tl_d2h_t tl_sram_d2h;
 
-  // Dual-port SRAM Interface: Refer prim_ram_2p_adv.sv
+  // Software access to the Packet Buffer RAM
   logic              sw_mem_a_req;
+  logic              sw_mem_a_gnt;
   logic              sw_mem_a_write;
   logic [SramAw-1:0] sw_mem_a_addr;
   logic [SramDw-1:0] sw_mem_a_wdata;
   logic              sw_mem_a_rvalid;
   logic [SramDw-1:0] sw_mem_a_rdata;
   logic [1:0]        sw_mem_a_rerror;
 
+  // usbdev hardware access to the Packet Buffer RAM
   logic              usb_mem_b_req;
   logic              usb_mem_b_write;
   logic [SramAw-1:0] usb_mem_b_addr;
@@ -582,6 +584,7 @@ module usbdev
 
     // Tie off unused signals
     assign sw_mem_a_req    = '0;
+    assign sw_mem_a_gnt    = '0;
     assign sw_mem_a_write  = '0;
     assign sw_mem_a_addr   = '0;
     assign sw_mem_a_wdata  = '0;
@@ -614,7 +617,7 @@ module usbdev
       .en_ifetch_i (prim_mubi_pkg::MuBi4False),
       .req_o       (sw_mem_a_req),
       .req_type_o  (),
-      .gnt_i       (sw_mem_a_req),  //Always grant when request
+      .gnt_i       (sw_mem_a_gnt),
       .we_o        (sw_mem_a_write),
       .addr_o      (sw_mem_a_addr),
       .wdata_o     (sw_mem_a_wdata),
@@ -625,8 +628,56 @@ module usbdev
       .rerror_i    (sw_mem_a_rerror)
     );
 
+    // Single Port RAM implementation, which will award the `usb` port absolute priority and
+    // delay `sw` access by in the event of a collision.
+    //
+    // In practice the `usb` access to memory is sporadic (4x oversampling of bits, and read/write
+    // operations transfer 32 bits so on average the probability of collision is just 1/128 even
+    // during active USB traffic, if the TL-UL interface were active on every cycle).
+
+    // usb access has absolute priority, followed by any deferred write, and then any sw access.
+    wire               mem_req   = usb_mem_b_req | sw_mem_a_req;
+    wire               mem_write = usb_mem_b_req ? usb_mem_b_write : sw_mem_a_write;
+    wire  [SramAw-1:0] mem_addr  = usb_mem_b_req ? usb_mem_b_addr  : sw_mem_a_addr;
+    wire  [SramDw-1:0] mem_wdata = usb_mem_b_req ? usb_mem_b_wdata : sw_mem_a_wdata;
+    logic              mem_rvalid;
+    logic [SramDw-1:0] mem_rdata;
+    logic [1:0]        mem_rerror;
+    logic              mem_rsteering;
+
+    // Always grant when no `usb` request.
+    assign sw_mem_a_gnt = !usb_mem_b_req;
+
+    // `usb` relies upon its read data remaining static after read.
+    logic              mem_b_read_q;
+    logic [SramDw-1:0] mem_b_rdata_q;
+
+    // Remember granted read accesses.
+    // NOTE: No pipelining within the RAM model.
+    always_ff @(posedge clk_i or negedge rst_ni) begin
+      if (!rst_ni) begin
+        mem_rsteering <= 1'b0;
+        mem_b_read_q  <= 1'b0;
+        mem_b_rdata_q <= {SramDw{1'b0}};
+      end else begin
+        mem_rsteering <= usb_mem_b_req;
+        mem_b_read_q  <= usb_mem_b_req & !usb_mem_b_write;
+        // Capture the `usb` read data.
+        if (mem_b_read_q)
+          mem_b_rdata_q <= mem_rdata;
+      end
+    end
+
+    // Read responses.
+    assign sw_mem_a_rvalid  = mem_rvalid & !mem_rsteering;
+    assign sw_mem_a_rerror  = {2{sw_mem_a_rvalid}} & mem_rerror;
+    // We may safely return the read data to both (no security implications), but `usb` rdata
+    // must be held static after the read, and be unaffected by `sw` reads.
+    assign sw_mem_a_rdata  = mem_rdata;
+    assign usb_mem_b_rdata = mem_b_read_q ? mem_rdata : mem_b_rdata_q;
+
     // SRAM Wrapper
-    prim_ram_2p_adv #(
+    prim_ram_1p_adv #(
       .Depth (SramDepth),
       .Width (SramDw),    // 32 x 512 --> 2kB
       .DataBitsPerMask(8),
@@ -635,29 +686,21 @@ module usbdev
       .EnableParity        (0),
       .EnableInputPipeline (0),
       .EnableOutputPipeline(0)
-    ) u_memory_2p (
+    ) u_memory_1p (
       .clk_i,
       .rst_ni,
-      .a_req_i    (sw_mem_a_req),
-      .a_write_i  (sw_mem_a_write),
-      .a_addr_i   (sw_mem_a_addr),
-      .a_wdata_i  (sw_mem_a_wdata),
-      .a_wmask_i  ({SramDw{1'b1}}),
-      .a_rvalid_o (sw_mem_a_rvalid),
-      .a_rdata_o  (sw_mem_a_rdata),
-      .a_rerror_o (sw_mem_a_rerror),
-
-      .b_req_i    (usb_mem_b_req),
-      .b_write_i  (usb_mem_b_write),
-      .b_addr_i   (usb_mem_b_addr),
-      .b_wdata_i  (usb_mem_b_wdata),
-      .b_wmask_i  ({SramDw{1'b1}}),
-      .b_rvalid_o (),
-      .b_rdata_o  (usb_mem_b_rdata),
-      .b_rerror_o (),
+
+      .req_i      (mem_req),
+      .write_i    (mem_write),
+      .addr_i     (mem_addr),
+      .wdata_i    (mem_wdata),
+      .wmask_i    ({SramDw{1'b1}}),
+      .rdata_o    (mem_rdata),
+      .rvalid_o   (mem_rvalid),
+      .rerror_o   (mem_rerror),
       .cfg_i      (ram_cfg_i)
     );
-  end
+  end : gen_no_stubbed_memory
 
   logic [NumAlerts-1:0] alert_test, alerts;
 

diff --git a/hw/ip/usbdev/usbdev.core b/hw/ip/usbdev/usbdev.core
@@ -12,6 +12,7 @@ filesets:
       - lowrisc:tlul:socket_1n
       - lowrisc:prim:edge_detector
       - lowrisc:prim:ram_2p_adv
+      - lowrisc:prim:ram_1p_adv
       - lowrisc:ip:usb_fs_nb_pe
       - lowrisc:ip:usbdev_pkg
     files:

diff --git a/hw/top_earlgrey/data/autogen/top_earlgrey.gen.hjson b/hw/top_earlgrey/data/autogen/top_earlgrey.gen.hjson
@@ -2596,14 +2596,14 @@
         }
         {
           name: ram_cfg
-          struct: ram_2p_cfg
-          package: prim_ram_2p_pkg
+          struct: ram_1p_cfg
+          package: prim_ram_1p_pkg
           type: uni
           act: rcv
           width: 1
           inst_name: usbdev
           default: ""
-          top_signame: ast_usb_ram_2p_cfg
+          top_signame: ast_usb_ram_1p_cfg
           index: -1
         }
         {
@@ -7983,17 +7983,17 @@
           conn_type: true
         }
         {
-          struct: ram_2p_cfg
-          package: prim_ram_2p_pkg
+          struct: ram_1p_cfg
+          package: prim_ram_1p_pkg
           type: uni
-          name: usb_ram_2p_cfg
+          name: usb_ram_1p_cfg
           act: rcv
           inst_name: ast
           width: 1
           default: ""
           end_idx: -1
           top_type: broadcast
-          top_signame: ast_usb_ram_2p_cfg
+          top_signame: ast_usb_ram_1p_cfg
           index: -1
           external: true
           conn_type: true
@@ -8053,7 +8053,7 @@
       [
         spi_device.ram_cfg
       ]
-      ast.usb_ram_2p_cfg:
+      ast.usb_ram_1p_cfg:
       [
         usbdev.ram_cfg
       ]
@@ -8640,7 +8640,7 @@
       ast.obs_ctrl: obs_ctrl
       ast.ram_1p_cfg: ram_1p_cfg
       ast.spi_ram_2p_cfg: spi_ram_2p_cfg
-      ast.usb_ram_2p_cfg: usb_ram_2p_cfg
+      ast.usb_ram_1p_cfg: usb_ram_1p_cfg
       ast.rom_cfg: rom_cfg
       clkmgr_aon.jitter_en: clk_main_jitter_en
       clkmgr_aon.io_clk_byp_req: io_clk_byp_req
@@ -16670,14 +16670,14 @@
       }
       {
         name: ram_cfg
-        struct: ram_2p_cfg
-        package: prim_ram_2p_pkg
+        struct: ram_1p_cfg
+        package: prim_ram_1p_pkg
         type: uni
         act: rcv
         width: 1
         inst_name: usbdev
         default: ""
-        top_signame: ast_usb_ram_2p_cfg
+        top_signame: ast_usb_ram_1p_cfg
         index: -1
       }
       {
@@ -20369,17 +20369,17 @@
         conn_type: true
       }
       {
-        struct: ram_2p_cfg
-        package: prim_ram_2p_pkg
+        struct: ram_1p_cfg
+        package: prim_ram_1p_pkg
         type: uni
-        name: usb_ram_2p_cfg
+        name: usb_ram_1p_cfg
         act: rcv
         inst_name: ast
         width: 1
         default: ""
         end_idx: -1
         top_type: broadcast
-        top_signame: ast_usb_ram_2p_cfg
+        top_signame: ast_usb_ram_1p_cfg
         index: -1
         external: true
         conn_type: true
@@ -20516,16 +20516,16 @@
         netname: ast_spi_ram_2p_cfg
       }
       {
-        package: prim_ram_2p_pkg
-        struct: ram_2p_cfg
-        signame: usb_ram_2p_cfg_i
+        package: prim_ram_1p_pkg
+        struct: ram_1p_cfg
+        signame: usb_ram_1p_cfg_i
         width: 1
         type: uni
         default: ""
         direction: in
         conn_type: true
         index: -1
-        netname: ast_usb_ram_2p_cfg
+        netname: ast_usb_ram_1p_cfg
       }
       {
         package: prim_rom_pkg
@@ -21104,15 +21104,15 @@
         default: prim_ram_2p_pkg::RAM_2P_CFG_DEFAULT
       }
       {
-        package: prim_ram_2p_pkg
-        struct: ram_2p_cfg
-        signame: ast_usb_ram_2p_cfg
+        package: prim_ram_1p_pkg
+        struct: ram_1p_cfg
+        signame: ast_usb_ram_1p_cfg
         width: 1
         type: uni
         end_idx: -1
         act: rcv
         suffix: ""
-        default: prim_ram_2p_pkg::RAM_2P_CFG_DEFAULT
+        default: prim_ram_1p_pkg::RAM_1P_CFG_DEFAULT
       }
       {
         package: prim_rom_pkg

diff --git a/hw/top_earlgrey/data/top_earlgrey.hjson b/hw/top_earlgrey/data/top_earlgrey.hjson
@@ -854,10 +854,10 @@
           act:     "rcv"
         },
 
-        { struct:  "ram_2p_cfg",
-          package: "prim_ram_2p_pkg",
+        { struct:  "ram_1p_cfg",
+          package: "prim_ram_1p_pkg",
           type:    "uni",
-          name:    "usb_ram_2p_cfg",
+          name:    "usb_ram_1p_cfg",
           // The activity direction for a port inter-signal is "opposite" of
           // what the external module actually needs.
           act:     "rcv"
@@ -899,7 +899,7 @@
                                    'sram_ctrl_ret_aon.cfg',
                                    'rv_core_ibex.ram_cfg'],
       'ast.spi_ram_2p_cfg'      : ['spi_device.ram_cfg'],
-      'ast.usb_ram_2p_cfg'      : ['usbdev.ram_cfg'],
+      'ast.usb_ram_1p_cfg'      : ['usbdev.ram_cfg'],
       'ast.rom_cfg'             : ['rom_ctrl.rom_cfg'],
       'alert_handler.crashdump' : ['rstmgr_aon.alert_dump'],
       'alert_handler.esc_rx'    : ['rv_core_ibex.esc_rx',
@@ -1103,7 +1103,7 @@
         'ast.obs_ctrl'                    : 'obs_ctrl',
         'ast.ram_1p_cfg'                  : 'ram_1p_cfg',
         'ast.spi_ram_2p_cfg'              : 'spi_ram_2p_cfg',
-        'ast.usb_ram_2p_cfg'              : 'usb_ram_2p_cfg',
+        'ast.usb_ram_1p_cfg'              : 'usb_ram_1p_cfg',
         'ast.rom_cfg'                     : 'rom_cfg',
         'clkmgr_aon.jitter_en'            : 'clk_main_jitter_en',
         'clkmgr_aon.io_clk_byp_req'       : 'io_clk_byp_req',

diff --git a/hw/top_earlgrey/dv/tb/chip_hier_macros.svh b/hw/top_earlgrey/dv/tb/chip_hier_macros.svh
@@ -60,4 +60,4 @@
 `define OTP_MEM_HIER          `OTP_GENERIC_HIER.u_prim_ram_1p_adv.u_mem.`MEM_ARRAY_SUB
 `define OTBN_IMEM_HIER        `OTBN_HIER.u_imem.u_prim_ram_1p_adv.u_mem.`MEM_ARRAY_SUB
 `define OTBN_DMEM_HIER        `OTBN_HIER.u_dmem.u_prim_ram_1p_adv.u_mem.`MEM_ARRAY_SUB
-`define USBDEV_BUF_HIER       `USBDEV_HIER.gen_no_stubbed_memory.u_memory_2p.i_prim_ram_2p_async_adv.u_mem.`MEM_ARRAY_SUB
+`define USBDEV_BUF_HIER       `USBDEV_HIER.gen_no_stubbed_memory.u_memory_1p.u_mem.`MEM_ARRAY_SUB
diff --git a/hw/top_earlgrey/rtl/autogen/chip_earlgrey_asic.sv b/hw/top_earlgrey/rtl/autogen/chip_earlgrey_asic.sv
@@ -790,7 +790,7 @@ module chip_earlgrey_asic #(
 
   prim_ram_1p_pkg::ram_1p_cfg_t ram_1p_cfg;
   prim_ram_2p_pkg::ram_2p_cfg_t spi_ram_2p_cfg;
-  prim_ram_2p_pkg::ram_2p_cfg_t usb_ram_2p_cfg;
+  prim_ram_1p_pkg::ram_1p_cfg_t usb_ram_1p_cfg;
   prim_rom_pkg::rom_cfg_t rom_cfg;
 
   // conversion from ast structure to memory centric structures
@@ -805,18 +805,15 @@ module chip_earlgrey_asic #(
               }
   };
 
-  // this maps as follows:
-  // assign usb_ram_2p_cfg = {10'h000, ram_2p_cfg_i.a_ram_fcfg, ram_2p_cfg_i.b_ram_fcfg};
-  assign usb_ram_2p_cfg = '{
-    a_ram_lcfg: '{
-                   cfg_en: ast_ram_2p_fcfg.marg_en_a,
-                   cfg:    ast_ram_2p_fcfg.marg_a
-                 },
-    b_ram_lcfg: '{
-                   cfg_en: ast_ram_2p_fcfg.marg_en_b,
-                   cfg:    ast_ram_2p_fcfg.marg_b
-                 },
-    default: '0
+  assign usb_ram_1p_cfg = '{
+    ram_cfg: '{
+                cfg_en: ast_ram_1p_cfg.marg_en,
+                cfg:    ast_ram_1p_cfg.marg
+              },
+    rf_cfg:  '{
+                cfg_en: ast_rf_cfg.marg_en,
+                cfg:    ast_rf_cfg.marg
+              }
   };
 
   // this maps as follows:
@@ -1171,7 +1168,7 @@ module chip_earlgrey_asic #(
     // Memory attributes
     .ram_1p_cfg_i                 ( ram_1p_cfg                 ),
     .spi_ram_2p_cfg_i             ( spi_ram_2p_cfg             ),
-    .usb_ram_2p_cfg_i             ( usb_ram_2p_cfg             ),
+    .usb_ram_1p_cfg_i             ( usb_ram_1p_cfg             ),
 
     .rom_cfg_i                    ( rom_cfg                    ),