Adding ring buffers to remove USB bottleneck

johnnyman727 · johnnyman727 · commit dcf6a7dba584 · 2015-12-15T21:09:39.000-08:00
diff --git a/boot.mk b/boot.mk
@@ -11,5 +11,5 @@ $(TARGET)_SRC += \
   boot/usb.c \
   $(USB_PATH)/class/dfu/dfu.c
 
-$(TARGET)_LDSCRIPT = deps/sam0/linker_scripts/samd21/gcc/samd21g15a_flash.ld
-$(TARGET)_DEFINE += -D __SAMD21G15A__
+$(TARGET)_LDSCRIPT = deps/sam0/linker_scripts/samd21/gcc/samd21g18a_flash.ld
+$(TARGET)_DEFINE += -D __SAMD21G18A__
diff --git a/common/samd21g18a_firmware_partition.ld b/common/samd21g18a_firmware_partition.ld
@@ -49,7 +49,7 @@ SEARCH_DIR(.)
 /* Memory Spaces Definitions */
 MEMORY
 {
-  rom    (rx)  : ORIGIN = 0x00001000, LENGTH = 0x00008000
+  rom    (rx)  : ORIGIN = 0x00001000, LENGTH = 0x0003efff
   ram    (rwx) : ORIGIN = 0x20000000, LENGTH = 0x00001000
 }
 
diff --git a/firmware.mk b/firmware.mk
@@ -12,5 +12,5 @@ $(TARGET)_SRC += \
   firmware/usbpipe.c \
   firmware/usbserial.c \
 
-$(TARGET)_LDSCRIPT = common/samd21g15a_firmware_partition.ld
-$(TARGET)_DEFINE += -D __SAMD21G15A__
+$(TARGET)_LDSCRIPT = common/samd21g18a_firmware_partition.ld
+$(TARGET)_DEFINE += -D __SAMD21G18A__
diff --git a/firmware/usbpipe.c b/firmware/usbpipe.c
@@ -1,59 +1,103 @@
 #include "firmware.h"
-
-#define FLASH_BUFFER_SIZE 64
+#define OUT_RING_SIZE 10
+#define PACKET_SIZE 64
+u8 out_ring_buf[OUT_RING_SIZE][PACKET_SIZE];
+volatile u8 out_ring_count = 0; // Number of packets in the ring buffer
+volatile u8 out_ring_write_pos = 0; // Packet index in which we're currently receiving a packet, or will once it's free
+volatile u8 out_ring_read_pos = 0; // Packet index from which we're currently sending a packet, or will once it's filled.
+volatile u8 out_ring_short_packet = 0; // If nonzero, the ring ends with a short packet of this size
+volatile bool out_usb_pending = false;
+volatile bool out_bridge_pending = false;
 
 typedef enum {
     PIPE_DISABLE,
     PIPE_WAIT_FOR_USB,
     PIPE_WAIT_FOR_BRIDGE,
 } PipeState;
 
-PipeState pipe_state_pc_to_soc;
 PipeState pipe_state_soc_to_pc;
-USB_ALIGN u8 pipe_buffer_pc_to_soc[BRIDGE_BUF_SIZE];
 USB_ALIGN u8 pipe_buffer_soc_to_pc[BRIDGE_BUF_SIZE];
 
 void usbpipe_init() {
     usb_enable_ep(USB_EP_PIPE_OUT, USB_EP_TYPE_BULK, 64);
     usb_enable_ep(USB_EP_PIPE_IN, USB_EP_TYPE_BULK, 64);
 
-    usb_ep_start_out(USB_EP_PIPE_OUT, pipe_buffer_pc_to_soc, FLASH_BUFFER_SIZE);
-    pipe_state_pc_to_soc = PIPE_WAIT_FOR_USB;
+    usb_ep_start_out(USB_EP_PIPE_OUT, out_ring_buf[out_ring_write_pos], PACKET_SIZE);
+    out_usb_pending = true;
+    out_bridge_pending = false;
 
     bridge_start_out(BRIDGE_USB, pipe_buffer_soc_to_pc);
-    pipe_state_soc_to_pc  = PIPE_WAIT_FOR_BRIDGE;
+    pipe_state_soc_to_pc = PIPE_WAIT_FOR_BRIDGE;
 
     bridge_enable_chan(BRIDGE_USB); // Tells SPI Daemon to start connection to USB Daemon
 }
 
 void usbpipe_disable() {
     usb_disable_ep(USB_EP_PIPE_IN);
     usb_disable_ep(USB_EP_PIPE_OUT);
-    pipe_state_pc_to_soc = PIPE_DISABLE;
+    out_ring_count = 0;
+    out_ring_write_pos = 0;
+    out_ring_read_pos = 0;
+    out_ring_short_packet = 0;
     pipe_state_soc_to_pc = PIPE_DISABLE;
     bridge_disable_chan(BRIDGE_USB); // Tells SPI Daemon to close connection to USB Daemon
 }
 
-// Received from USB, send to bridge
-void pipe_usb_out_completion() {
-    if (pipe_state_pc_to_soc == PIPE_WAIT_FOR_USB) {
-        u32 len = usb_ep_out_length(USB_EP_PIPE_OUT);
-        bridge_start_in(BRIDGE_USB, pipe_buffer_pc_to_soc, len);
-        pipe_state_pc_to_soc = PIPE_WAIT_FOR_BRIDGE;
-    } else {
-        invalid();
+void out_ring_step() {
+    // If we are not currently receiving
+    // And there is an empty buffer to receive data into
+    // And there isn't an unprocessed short packet
+    if (!out_usb_pending && out_ring_count < OUT_RING_SIZE && out_ring_short_packet == 0) {
+        // Start reading data in over USB to the buffer at the correct position (up to 64 bytes)
+        usb_ep_start_out(USB_EP_PIPE_OUT, out_ring_buf[out_ring_write_pos], PACKET_SIZE);
+        // We are waiting for the transfer to complete
+        out_usb_pending = true;
     }
 
+    // If we are not waiting on a bridge transaction to complete and we have packets to send
+    if (!out_bridge_pending && out_ring_count > 0) {
+        // The size of the packet is 64 bytes (unless the below case is true)
+        u8 len = PACKET_SIZE;
+        // If we only have one outgoing packet and it is a short packet
+        if (out_ring_count == 1 && out_ring_short_packet != 0) {
+             // The length is actually a subset of a full packet
+             len = out_ring_short_packet;
+             // Reset the short packet var
+             out_ring_short_packet = 0;
+        }
+        // Start sending data to the spi daemon
+        bridge_start_in(BRIDGE_USB, out_ring_buf[out_ring_read_pos], len);
+        // We are currently waiting on the SPI
+        out_bridge_pending = true;
+    }
+}
+
+// Received from USB, send to bridge
+void pipe_usb_out_completion() {
+    // Get the length of the packet from USB
+    u32 len = usb_ep_out_length(USB_EP_PIPE_OUT);
+    // If it is less than one full packet, mark the short packet var with the length
+    if (len < PACKET_SIZE) out_ring_short_packet = len;
+    // Increase the next writable buffer by 1 (but loop to the beginning if necessary)
+    out_ring_write_pos = (out_ring_write_pos + 1) % OUT_RING_SIZE;
+    // Mark that we have one packet that needs attention
+    out_ring_count += 1;
+    // We are no longer operating over USB
+    out_usb_pending = false;
+    // Push the data to the correct place
+    out_ring_step();
 }
 
 // Finished sending on bridge, start receive from USB
 void pipe_bridge_in_completion() {
-    if (pipe_state_pc_to_soc == PIPE_WAIT_FOR_BRIDGE) {
-        usb_ep_start_out(USB_EP_PIPE_OUT, pipe_buffer_pc_to_soc, FLASH_BUFFER_SIZE);
-        pipe_state_pc_to_soc = PIPE_WAIT_FOR_USB;
-    } else {
-        invalid();
-    }
+    // Increment the location of where we will read from next (to send over USB)
+    out_ring_read_pos = (out_ring_read_pos + 1) % OUT_RING_SIZE;
+    // Decrement the number of packets that need reading
+    out_ring_count -= 1;
+    // Mark the bridge transfer as complete
+    out_bridge_pending = false;
+    // Move data along
+    out_ring_step();
 }
 
 // Received from bridge, send to USB
diff --git a/soc/spid.c b/soc/spid.c
@@ -518,42 +518,64 @@ int main(int argc, char** argv) {
 
         for (int chan=0; chan<N_CHANNEL; chan++) {
             int size = channels[chan].out_length;
+            // If the coprocessor is ready to receive, and we have data to send
             if (rx_buf[1] & (1<<chan) && size > 0) {
+                debug("coprocessor is ready to receive and we have %d bytes from channel %d", size, chan);
+                // Make this channel readable by others
                 CONN_POLL(chan).events |= POLLIN;
+                // Set the length to the size we need to send
                 transfer[desc].len = size;
+                // Point the output buffer to the correct place
                 transfer[desc].tx_buf = (unsigned long) &channels[chan].out_buf[0];
+                // Note that we will have no more data to send (once this is sent)
                 channels[chan].out_length = 0;
+                // Mark that we need to make a SPI transaction
                 desc++;
             }
 
+            // The number of bytes the coprocessor wants to send to a channel
             size = rx_buf[2+chan];
+            // Check that the channel is writable and there is data that needs to be received
             if (get_channel_bitmask_state(&channels_writable_bitmask, chan) && size > 0) {
+                debug("Channel %d is ready to have %d bytes written to it from bridge", chan, size);
+                // Set the appropriate size
                 transfer[desc].len = size;
+                // Point our receive buffer to the in buf of the appropriate channel
                 transfer[desc].rx_buf = (unsigned long) &channels[chan].in_buf[0];
+                // Mark that we need a SPI transaction to take place
                 desc++;
             }
         }
 
+        // If the previous logic designated the need for a SPI transaction
         if (desc != 0) {
             debug("Performing transfer on %i channels\n", desc);
 
+            // Make the SPI transaction
             int status = ioctl(spi_fd, SPI_IOC_MESSAGE(desc), transfer);
 
+            // Ensure there were no errors
             if (status < 0) {
               fatal("SPI_IOC_MESSAGE: data: %s", strerror(errno));
             }
 
             // Write received data to the appropriate socket
             for (int chan=0; chan<N_CHANNEL; chan++) {
+                // Get the length of the received data for this channel
                 int size = rx_buf[2+chan];
+                // Make sure that channel is writable and we have data to send to it
                 if (get_channel_bitmask_state(&channels_writable_bitmask, chan) && size > 0) {
+                    // Write this data to the pipe
                     int r = write(CONN_POLL(chan).fd, &channels[chan].in_buf[0], size);
                     debug("%i: Write %u %i\n", chan, size, r);
+                    // Ensure there were no errors
                     if (r < 0) {
                         error("Error in write %i: %s\n", chan, strerror(errno));
                     }
 
+                    // Mark we want to know when this pipe is writable again
                     CONN_POLL(chan).events |= POLLOUT;
+                    // Set the state to not writable
                     set_channel_bitmask_state(&channels_writable_bitmask, chan, false);
                 }
             }
diff --git a/soc/usbexecd.c b/soc/usbexecd.c
@@ -42,7 +42,7 @@ enum Commands {
     CMD_CLOSE_STDERR = 0x33,
 };
 
-#define debug(args...)  syslog(LOG_INFO, args)
+#define debug(args...)
 #define info(args...)   syslog(LOG_INFO, args)
 #define error(args...)  syslog(LOG_ERR, args)
 #define fatal(args...) ({ \
@@ -66,7 +66,7 @@ struct epoll_event spid_event;
 // Static array for those events to be stored
 struct epoll_event events[MAX_EPOLL_EVENTS];
 
-#define PIPE_BUF 4096
+#define INTERNAL_PIPE_BUF_SIZE 32768
 #define MAX_CTRL_ARGS 255
 #define MAX_WRITE_LEN 255
 
@@ -109,7 +109,7 @@ typedef struct {
     // The number of 'active' bytes
     int bufcount;
     // The internal buffer used for back pressure
-    char buffer[PIPE_BUF];
+    char buffer[INTERNAL_PIPE_BUF_SIZE];
 } pipebuf_t;
 
 typedef struct {
@@ -252,9 +252,9 @@ int write_from_pipebuf(pipebuf_t *pb, int fd, int write_len) {
         to_write = write_len;
 
         // If the data would go past the end of the buffer
-        if (pb->startpos + to_write > PIPE_BUF) {
+        if (pb->startpos + to_write > INTERNAL_PIPE_BUF_SIZE) {
             // Only write until the end of the buffer
-            to_write = PIPE_BUF - pb->startpos;
+            to_write = INTERNAL_PIPE_BUF_SIZE - pb->startpos;
         }
 
         // Write this batch to the file descriptor from the internal buffer
@@ -266,7 +266,7 @@ int write_from_pipebuf(pipebuf_t *pb, int fd, int write_len) {
         pb->bufcount -= written;
 
         // If the start position marker is at the buffer end
-        if (pb->startpos == PIPE_BUF) {
+        if (pb->startpos == INTERNAL_PIPE_BUF_SIZE) {
             // Set it at the start of the buffer for the next read
             pb->startpos = 0;
         }
@@ -421,9 +421,9 @@ int pipebuf_in_write_to_sock(pipebuf_t* pb, size_t num_to_write) {
         int remaining = num_to_write - written;
         int packet_write_size = (remaining < MAX_WRITE_LEN) ? remaining : MAX_WRITE_LEN;
         // Send the header so the CLI knows it's about to receive data
-        send_header(CMD_WRITE_CONTROL + pb->role, pb->id, 0, packet_write_size);
+        send_header(CMD_WRITE_CONTROL + pb->role, pb->id, packet_write_size >> 8, packet_write_size & 0xFF);
         // Send the data and increment the counter of the number of bytes written
-        pipebuf_common_debug(pb, "WRiting from stdout/stderr internal to CLI");
+        pipebuf_common_debug(pb, "Writing from stdout/stderr internal to CLI");
         debug("{%d bytes}", packet_write_size);
         written += write_from_pipebuf(pb, sock_fd, packet_write_size);
     }
@@ -459,7 +459,7 @@ void pipebuf_in_ack(pipebuf_t* pb, size_t ack_number_size) {
 
     // If this pipe buffer was previously full
     // But will now be able to send out data
-    if (pb->bufcount == PIPE_BUF && ack_size > 0) {
+    if (pb->bufcount == INTERNAL_PIPE_BUF_SIZE && ack_size > 0) {
         // Enable notifications of when the internal pipe buffer is written to
         add_pipebuf_epoll(pb);
     }
@@ -492,17 +492,17 @@ void pipebuf_in_to_internal_buffer(pipebuf_t* pb) {
     else to read */
     int r = 0;
     int try_to_read = 0;
-    int space_available = PIPE_BUF - pb->bufcount;
+    int space_available = INTERNAL_PIPE_BUF_SIZE - pb->bufcount;
     pipebuf_common_debug(pb, "stdout/stderr has data from child to be read");
     // While we have space in the pipe buffer
     while (space_available) {
         // Try to read the entire space
         try_to_read = space_available;
 
         // If the space extends past the pipe buffer end
-        if (pb->endpos + try_to_read > PIPE_BUF) {
+        if (pb->endpos + try_to_read > INTERNAL_PIPE_BUF_SIZE) {
             // Set this read to only read up to the end
-            try_to_read = PIPE_BUF - pb->endpos;
+            try_to_read = INTERNAL_PIPE_BUF_SIZE - pb->endpos;
         }
 
         pipebuf_common_debug(pb, "Attempting to write from stdout/stderr child to internal");
@@ -524,7 +524,7 @@ void pipebuf_in_to_internal_buffer(pipebuf_t* pb) {
             pb->endpos += r;
 
              // If the start position marker is at the buffer end
-            if (pb->endpos == PIPE_BUF) {
+            if (pb->endpos == INTERNAL_PIPE_BUF_SIZE) {
                 // Set it at the start of the buffer for the next read
                 pb->endpos = 0;
             }
@@ -555,7 +555,7 @@ void pipebuf_in_to_internal_buffer(pipebuf_t* pb) {
     }
 
     // If the pipe buffer is full
-    if (pb->bufcount == PIPE_BUF) {
+    if (pb->bufcount == INTERNAL_PIPE_BUF_SIZE) {
         // remove it from the epoll
         delete_pipebuf_epoll(pb);
     }
@@ -576,9 +576,9 @@ Returns: the file descriptor to communicate with the stream
 int pipebuf_out_init(pipebuf_t* pb, int id, int role) {
     int fd =  pipebuf_common_init(pb, id, role, EPOLLOUT, 1);
 
-    pipebuf_out_ack(pb, PIPE_BUF);
+    pipebuf_out_ack(pb, INTERNAL_PIPE_BUF_SIZE);
 
-    pb->credit = PIPE_BUF;
+    pb->credit = INTERNAL_PIPE_BUF_SIZE;
 
     return fd;
 }
@@ -621,9 +621,9 @@ void pipebuf_out_to_internal_buffer(pipebuf_t* pb, int read_len) {
         to_read = read_len;
 
         // If this read would read further than the end of the ring buffer
-        if (pb->endpos + read_len > PIPE_BUF) {
+        if (pb->endpos + read_len > INTERNAL_PIPE_BUF_SIZE) {
             // Only read until the end of the ring buffer
-            to_read = PIPE_BUF - pb->endpos;
+            to_read = INTERNAL_PIPE_BUF_SIZE - pb->endpos;
         }
 
         // Read from the socket into the buffer
@@ -645,7 +645,7 @@ void pipebuf_out_to_internal_buffer(pipebuf_t* pb, int read_len) {
         pb->endpos += to_read;
 
         // If the end position marker is at the buffer end
-        if (pb->endpos == PIPE_BUF) {
+        if (pb->endpos == INTERNAL_PIPE_BUF_SIZE) {
             // Set it at the start of the buffer for the next read
             pb->endpos = 0;
         }
@@ -855,12 +855,12 @@ void handle_socket_readable() {
 
         case CMD_WRITE_CONTROL:
             debug("CMD: Write to CTRL buf of process with id %d", id);
-            pipebuf_out_to_internal_buffer(&p->ctrl, header[3]);
+            pipebuf_out_to_internal_buffer(&p->ctrl, header[3] | (header[2] << 8));
             break;
 
         case CMD_WRITE_STDIN:
             debug("CMD: Write to STDIN buf of process with id %d", id);
-            pipebuf_out_to_internal_buffer(&p->stdin, header[3]);
+            pipebuf_out_to_internal_buffer(&p->stdin, header[3] | (header[2] << 8));
             break;
 
         case CMD_ACK_STDOUT:
diff --git a/test_rig.mk b/test_rig.mk
@@ -13,4 +13,4 @@ $(TARGET)_SRC += \
   test_rig/button.c \
 
 $(TARGET)_DEFINE += -D __SAMD21J18A__
-$(TARGET)_LDSCRIPT = common/samd21g15a_firmware_partition.ld
+$(TARGET)_LDSCRIPT = common/samd21g18a_firmware_partition.ld

Original file line number	Diff line number	Diff line change
`@@ -49,7 +49,7 @@ SEARCH_DIR(.)`
`49`	`49`	`/* Memory Spaces Definitions */`
`50`	`50`	`MEMORY`
`51`	`51`	`{`
`52`		`- rom (rx) : ORIGIN = 0x00001000, LENGTH = 0x00008000`
	`52`	`+ rom (rx) : ORIGIN = 0x00001000, LENGTH = 0x0003efff`
`53`	`53`	`ram (rwx) : ORIGIN = 0x20000000, LENGTH = 0x00001000`
`54`	`54`	`}`
`55`	`55`