[PATCH v4 7/7] usb: gadget: f_midi: pre-allocate IN requests

[Felipe F. Tonello]

This patch introduces pre-allocation of IN endpoint USB requests. This
improves on latency (requires no usb request allocation on transmit) and avoid
several potential probles on allocating too many usb requests (which involves
DMA pool allocation problems).

This implementation also handles better multiple MIDI Gadget ports, always
processing the last processed MIDI substream if the last USB request wasn't
enought to handle the whole stream.

Signed-off-by: Felipe F. Tonello <eu@xxxxxxxxxxxxxxxxx>
---
 drivers/usb/gadget/function/f_midi.c | 173 +++++++++++++++++++++++++++--------
 drivers/usb/gadget/legacy/gmidi.c    |   2 +-
 2 files changed, 136 insertions(+), 39 deletions(-)

diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c
index a617df3..f10cf7d 100644
--- a/drivers/usb/gadget/function/f_midi.c
+++ b/drivers/usb/gadget/function/f_midi.c
@@ -23,6 +23,7 @@
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/device.h>
+#include <linux/kfifo.h>
 
 #include <sound/core.h>
 #include <sound/initval.h>
@@ -88,6 +89,9 @@ struct f_midi {
 	char *id;
 	unsigned int buflen, qlen;
 	bool in_ep_enabled;
+	DECLARE_KFIFO_PTR(in_req_fifo, struct usb_request *);
+	unsigned int in_req_num;
+	unsigned int in_last_port;
 };
 
 static inline struct f_midi *func_to_midi(struct usb_function *f)
@@ -95,7 +99,7 @@ static inline struct f_midi *func_to_midi(struct usb_function *f)
 	return container_of(f, struct f_midi, func);
 }
 
-static void f_midi_transmit(struct f_midi *midi, struct usb_request *req);
+static void f_midi_transmit(struct f_midi *midi);
 
 DECLARE_UAC_AC_HEADER_DESCRIPTOR(1);
 DECLARE_USB_MIDI_OUT_JACK_DESCRIPTOR(1);
@@ -253,7 +257,8 @@ f_midi_complete(struct usb_ep *ep, struct usb_request *req)
 		} else if (ep == midi->in_ep) {
 			/* Our transmit completed. See if there's more to go.
 			 * f_midi_transmit eats req, don't queue it again. */
-			f_midi_transmit(midi, req);
+			req->length = 0;
+			f_midi_transmit(midi);
 			return;
 		}
 		break;
@@ -264,10 +269,12 @@ f_midi_complete(struct usb_ep *ep, struct usb_request *req)
 	case -ESHUTDOWN:	/* disconnect from host */
 		VDBG(cdev, "%s gone (%d), %d/%d\n", ep->name, status,
 				req->actual, req->length);
-		if (ep == midi->out_ep)
+		if (ep == midi->out_ep) {
 			f_midi_handle_out_data(ep, req);
-
-		free_ep_req(ep, req);
+			/* We don't need to free IN requests because it's handled
+			 * by the midi->in_req_fifo. */
+			free_ep_req(ep, req);
+		}
 		return;
 
 	case -EOVERFLOW:	/* buffer overrun on read means that
@@ -336,6 +343,21 @@ static int f_midi_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
 	if (err)
 		return err;
 
+	/* pre-allocate write usb requests to use on f_midi_transmit. */
+	while (kfifo_avail(&midi->in_req_fifo)) {
+		struct usb_request *req =
+			midi_alloc_ep_req(midi->in_ep, midi->buflen);
+
+		if (req == NULL)
+			return -ENOMEM;
+
+		req->length = 0;
+		req->complete = f_midi_complete;
+
+		kfifo_put(&midi->in_req_fifo, req);
+		midi->in_req_num++;
+	}
+
 	/* allocate a bunch of read buffers and queue them all at once. */
 	for (i = 0; i < midi->qlen && err == 0; i++) {
 		struct usb_request *req =
@@ -369,6 +391,19 @@ static void f_midi_disable(struct usb_function *f)
 	usb_ep_disable(midi->in_ep);
 	usb_ep_disable(midi->out_ep);
 
+	/* release IN requests */
+	while (!kfifo_is_empty(&midi->in_req_fifo)) {
+		struct usb_request *req = NULL;
+		unsigned int len;
+
+		len = kfifo_get(&midi->in_req_fifo, &req);
+		if (len == 1)
+			free_ep_req(midi->in_ep, req);
+		else
+			ERROR(midi, "%s couldn't free usb request: something went wrong with kfifo\n",
+			      midi->in_ep->name);
+	}
+	midi->in_req_num = 0;
 	midi->in_ep_enabled = false;
 }
 
@@ -491,57 +526,111 @@ static void f_midi_transmit_byte(struct usb_request *req,
 	}
 }
 
-static void f_midi_transmit(struct f_midi *midi, struct usb_request *req)
+static void f_midi_drop_out_substreams(struct f_midi *midi)
 {
-	struct usb_ep *ep = midi->in_ep;
-	int i;
-
-	if (!ep)
-		return;
-
-	if (!req)
-		req = midi_alloc_ep_req(ep, midi->buflen);
-
-	if (!req) {
-		ERROR(midi, "%s: alloc_ep_request failed\n", __func__);
-		return;
-	}
-	req->length = 0;
-	req->complete = f_midi_complete;
+	unsigned int i;
 
 	for (i = 0; i < MAX_PORTS; i++) {
 		struct gmidi_in_port *port = midi->in_port[i];
 		struct snd_rawmidi_substream *substream = midi->in_substream[i];
 
-		if (!port || !port->active || !substream)
+		if (!port)
+			break;
+
+		if (!port->active || !substream)
 			continue;
 
-		while (req->length + 3 < midi->buflen) {
-			uint8_t b;
-			if (snd_rawmidi_transmit(substream, &b, 1) != 1) {
-				port->active = 0;
+		snd_rawmidi_drop_output(substream);
+	}
+}
+
+static void f_midi_transmit(struct f_midi *midi)
+{
+	struct usb_ep *ep = midi->in_ep;
+	bool active;
+
+	/* We only care about USB requests if IN endpoint is enabled */
+	if (!ep || !midi->in_ep_enabled)
+		goto drop_out;
+
+	do {
+		struct usb_request *req = NULL;
+		unsigned int len, i;
+
+		active = false;
+
+		/* We peek the request in order to reuse it if it fails
+		 * to enqueue on its endpoint */
+		len = kfifo_peek(&midi->in_req_fifo, &req);
+		if (len != 1) {
+			ERROR(midi, "%s: Couldn't get usb request\n", __func__);
+			goto drop_out;
+		}
+
+		if (req->length > 0) {
+			WARNING(midi, "%s: All USB requests have been used. Current queue size "
+				"is %u, consider increasing it.\n", __func__, midi->in_req_num);
+			goto drop_out;
+		}
+
+		for (i = midi->in_last_port; i < MAX_PORTS; i++) {
+			struct gmidi_in_port *port = midi->in_port[i];
+			struct snd_rawmidi_substream *substream = midi->in_substream[i];
+
+			if (!port) {
+				/* Reset counter when we reach the last available port */
+				midi->in_last_port = 0;
+				break;
+			}
+
+			if (!port->active || !substream)
+				continue;
+
+			while (req->length + 3 < midi->buflen) {
+				uint8_t b;
+
+				if (snd_rawmidi_transmit(substream, &b, 1) != 1) {
+					port->active = 0;
+					break;
+				}
+				f_midi_transmit_byte(req, port, b);
+			}
+
+			active = !!port->active;
+			/* Check if last port is still active, which means that
+			 * there is still data on that substream but this current
+			 * request run out of space. */
+			if (active) {
+				midi->in_last_port = i;
+				/* There is no need to re-iterate though midi ports. */
 				break;
 			}
-			f_midi_transmit_byte(req, port, b);
 		}
-	}
 
-	if (req->length > 0 && midi->in_ep_enabled) {
-		int err;
+		if (req->length > 0) {
+			int err;
 
-		err = usb_ep_queue(ep, req, GFP_ATOMIC);
-		if (err < 0)
-			ERROR(midi, "%s queue req: %d\n",
-			      midi->in_ep->name, err);
-	} else {
-		free_ep_req(ep, req);
-	}
+			err = usb_ep_queue(ep, req, GFP_ATOMIC);
+			if (err < 0) {
+				ERROR(midi, "%s failed to queue req: %d\n",
+				      midi->in_ep->name, err);
+				req->length = 0; /* Re-use request next time. */
+			} else {
+				/* Upon success, put request at the back of the queue. */
+				kfifo_skip(&midi->in_req_fifo);
+				kfifo_put(&midi->in_req_fifo, req);
+			}
+		}
+	} while (active);
+
+drop_out:
+	f_midi_drop_out_substreams(midi);
 }
 
 static void f_midi_in_tasklet(unsigned long data)
 {
 	struct f_midi *midi = (struct f_midi *) data;
-	f_midi_transmit(midi, NULL);
+	f_midi_transmit(midi);
 }
 
 static int f_midi_in_open(struct snd_rawmidi_substream *substream)
@@ -667,6 +756,7 @@ static int f_midi_register_card(struct f_midi *midi)
 		goto fail;
 	}
 	midi->rmidi = rmidi;
+	midi->in_last_port = 0;
 	strcpy(rmidi->name, card->shortname);
 	rmidi->info_flags = SNDRV_RAWMIDI_INFO_OUTPUT |
 			    SNDRV_RAWMIDI_INFO_INPUT |
@@ -1068,6 +1158,7 @@ static void f_midi_free(struct usb_function *f)
 	mutex_lock(&opts->lock);
 	for (i = opts->in_ports - 1; i >= 0; --i)
 		kfree(midi->in_port[i]);
+	kfifo_free(&midi->in_req_fifo);
 	kfree(midi);
 	--opts->refcnt;
 	mutex_unlock(&opts->lock);
@@ -1142,6 +1233,12 @@ static struct usb_function *f_midi_alloc(struct usb_function_instance *fi)
 	midi->buflen = opts->buflen;
 	midi->qlen = opts->qlen;
 	midi->in_ep_enabled = false;
+	midi->in_req_num = 0;
+	midi->in_last_port = 0;
+
+	if (kfifo_alloc(&midi->in_req_fifo, midi->qlen, GFP_KERNEL))
+		goto setup_fail;
+
 	++opts->refcnt;
 	mutex_unlock(&opts->lock);
 
diff --git a/drivers/usb/gadget/legacy/gmidi.c b/drivers/usb/gadget/legacy/gmidi.c
index 055390b..3522687 100644
--- a/drivers/usb/gadget/legacy/gmidi.c
+++ b/drivers/usb/gadget/legacy/gmidi.c
@@ -53,7 +53,7 @@ MODULE_PARM_DESC(buflen, "MIDI buffer length");
 
 static unsigned int qlen = 32;
 module_param(qlen, uint, S_IRUGO);
-MODULE_PARM_DESC(qlen, "USB read request queue length");
+MODULE_PARM_DESC(qlen, "USB read and write request queue length");
 
 static unsigned int in_ports = 1;
 module_param(in_ports, uint, S_IRUGO);
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/