[alsa-devel] Fwd: [PATCH] ehci-hcd: complete iso urbs ASAP for number_of_packets != (n * 8) also

Karsten Wiese fzu at wemgehoertderstaat.de
Wed Nov 7 16:41:28 CET 2007


Hi

I just sent attached patch to usb-devel.
It makes special workaround for iso usb communication using ehci-hcd highspeed
needless.
Comments, Bug/success reports welcome!

kind regards
      Karsten

Upstream (as of 2.6.23) ehci-hcd only completes iso urbs, if the last frame
(= 8 uframes) they overlapped with has elapsed. That can be as late as when
the following urb emits its interrupt on completion.
soundcard drivers tend to work around by only transfering iso urbs with
number_of_packets = (n * 8).
Patch lets iso urbs complete asap by scanning itd always up to the elapsed
uframe. An itd's last scanned uframe is stored in the new struct ehci_itd
member uframe_scanned.
Itds stay kept in the hcd's schedule until the complete frame they cover
has elapsed, like without patch.
To make this possible, ehci->periodic_sched is changed based on the number of
active itds instead of the number of active iso urbs.

Signed-off-by: Karsten Wiese <fzu at wemgehoertderstaat.de>
---
 drivers/usb/host/ehci-sched.c |  108 ++++++++++++++++++++---------------------
 drivers/usb/host/ehci.h       |    1 +
 2 files changed, 54 insertions(+), 55 deletions(-)

diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c
index e682f23..c456b16 100644
--- a/drivers/usb/host/ehci-sched.c
+++ b/drivers/usb/host/ehci-sched.c
@@ -1481,6 +1481,8 @@ itd_link (struct ehci_hcd *ehci, unsigned frame, struct ehci_itd *itd)
 	itd->frame = frame;
 	wmb ();
 	ehci->periodic[frame] = cpu_to_hc32(ehci, itd->itd_dma | Q_TYPE_ITD);
+	if (unlikely (!ehci->periodic_sched++))
+		enable_periodic (ehci);
 }
 
 /* fit urb's itds into the selected schedule slot; activate as needed */
@@ -1553,30 +1555,30 @@ itd_link_urb (
 	urb->hcpriv = NULL;
 
 	timer_action (ehci, TIMER_IO_WATCHDOG);
-	if (unlikely (!ehci->periodic_sched++))
-		return enable_periodic (ehci);
 	return 0;
 }
 
 #define	ISO_ERRS (EHCI_ISOC_BUF_ERR | EHCI_ISOC_BABBLE | EHCI_ISOC_XACTERR)
 
-static unsigned
-itd_complete (
-	struct ehci_hcd	*ehci,
-	struct ehci_itd	*itd
-) {
+static bool
+itd_scan (
+	struct ehci_hcd *ehci,
+	struct ehci_itd	*itd,
+	unsigned uframe_after
+)
+{
 	struct urb				*urb = itd->urb;
 	struct usb_iso_packet_descriptor	*desc;
 	u32					t;
 	unsigned				uframe;
 	int					urb_index = -1;
 	struct ehci_iso_stream			*stream = itd->stream;
-	struct usb_device			*dev;
 
 	/* for each uframe with a packet */
-	for (uframe = 0; uframe < 8; uframe++) {
+	for (uframe = itd->uframe_scanned; uframe < uframe_after; uframe++) {
 		if (likely (itd->index[uframe] == -1))
 			continue;
+
 		urb_index = itd->index[uframe];
 		desc = &urb->iso_frame_desc [urb_index];
 
@@ -1603,45 +1605,53 @@ itd_complete (
 			desc->status = 0;
 			desc->actual_length = EHCI_ITD_LENGTH (t);
 		}
+
+		/* handle completion now? */
+		if (urb_index + 1 == urb->number_of_packets) {
+			struct usb_device *dev;
+
+			usb_put_urb (urb);
+			/* ASSERT: it's really the last itd for this urb
+			   struct ehci_itd	*stritd;
+			   list_for_each_entry (stritd, &stream->td_list, itd_list)
+			   BUG_ON (stritd != itd && stritd->urb == urb);
+			*/
+			itd->urb = NULL;
+
+			/* give urb back to the driver ... can be out-of-order */
+			dev = urb->dev;
+			ehci_urb_done (ehci, urb);
+			urb = NULL;
+
+			ehci_to_hcd(ehci)->self.bandwidth_isoc_reqs--;
+
+			if (unlikely (list_empty (&stream->td_list))) {
+				ehci_to_hcd(ehci)->self.bandwidth_allocated
+					-= stream->bandwidth;
+				ehci_vdbg (ehci,
+					   "deschedule devp %s ep%d%s-iso\n",
+					   dev->devpath, stream->bEndpointAddress & 0x0f,
+					   (stream->bEndpointAddress & USB_DIR_IN) ? "in" : "out");
+			}
+			iso_stream_put (ehci, stream);
+		}
 	}
 
-	usb_put_urb (urb);
-	itd->urb = NULL;
+	itd->uframe_scanned = uframe_after;
+	if (uframe_after < 8)
+		return false;
+
 	itd->stream = NULL;
+	itd->uframe_scanned = 0;
 	list_move (&itd->itd_list, &stream->free_list);
 	iso_stream_put (ehci, stream);
 
-	/* handle completion now? */
-	if (likely ((urb_index + 1) != urb->number_of_packets))
-		return 0;
-
-	/* ASSERT: it's really the last itd for this urb
-	list_for_each_entry (itd, &stream->td_list, itd_list)
-		BUG_ON (itd->urb == urb);
-	 */
-
-	/* give urb back to the driver ... can be out-of-order */
-	dev = urb->dev;
-	ehci_urb_done (ehci, urb);
-	urb = NULL;
-
 	/* defer stopping schedule; completion can submit */
 	ehci->periodic_sched--;
 	if (unlikely (!ehci->periodic_sched))
 		(void) disable_periodic (ehci);
-	ehci_to_hcd(ehci)->self.bandwidth_isoc_reqs--;
 
-	if (unlikely (list_empty (&stream->td_list))) {
-		ehci_to_hcd(ehci)->self.bandwidth_allocated
-				-= stream->bandwidth;
-		ehci_vdbg (ehci,
-			"deschedule devp %s ep%d%s-iso\n",
-			dev->devpath, stream->bEndpointAddress & 0x0f,
-			(stream->bEndpointAddress & USB_DIR_IN) ? "in" : "out");
-	}
-	iso_stream_put (ehci, stream);
-
-	return 1;
+	return true;
 }
 
 /*-------------------------------------------------------------------------*/
@@ -2156,30 +2166,18 @@ restart:
 				q = q.fstn->fstn_next;
 				break;
 			case Q_TYPE_ITD:
-				/* skip itds for later in the frame */
 				rmb ();
-				for (uf = live ? uframes : 8; uf < 8; uf++) {
-					if (0 == (q.itd->hw_transaction [uf]
-							& ITD_ACTIVE(ehci)))
-						continue;
+				modified = itd_scan(ehci, q.itd, uf = live ? uframes : 8);
+				if (!modified) {
 					q_p = &q.itd->itd_next;
 					hw_p = &q.itd->hw_next;
-					type = Q_NEXT_TYPE(ehci,
-							q.itd->hw_next);
-					q = *q_p;
-					break;
+				} else {
+					*q_p = q.itd->itd_next;
+					*hw_p = q.itd->hw_next;
 				}
-				if (uf != 8)
-					break;
-
-				/* this one's ready ... HC won't cache the
-				 * pointer for much longer, if at all.
-				 */
-				*q_p = q.itd->itd_next;
-				*hw_p = q.itd->hw_next;
 				type = Q_NEXT_TYPE(ehci, q.itd->hw_next);
+
 				wmb();
-				modified = itd_complete (ehci, q.itd);
 				q = *q_p;
 				break;
 			case Q_TYPE_SITD:
diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
index 951d69f..df35964 100644
--- a/drivers/usb/host/ehci.h
+++ b/drivers/usb/host/ehci.h
@@ -581,6 +581,7 @@ struct ehci_itd {
 	struct urb		*urb;
 	struct ehci_iso_stream	*stream;	/* endpoint's queue */
 	struct list_head	itd_list;	/* list of stream's itds */
+	unsigned		uframe_scanned;
 
 	/* any/all hw_transactions here may be used by that urb */
 	unsigned		frame;		/* where scheduled */
-- 
1.5.3.3

-------------- next part --------------
An embedded message was scrubbed...
From: Karsten Wiese <fzu at wemgehoertderstaat.de>
Subject: [PATCH] ehci-hcd: complete iso urbs ASAP for number_of_packets != (n * 8) also
Date: Wed, 7 Nov 2007 16:16:34 +0100
Size: 6675
Url: http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20071107/8ecd2c73/attachment.eml 


More information about the Alsa-devel mailing list