From xiphmont@gmail.com Wed Sep 27 23:58:14 2006 Message-ID: <806dafc20609272358h67f5e636m33122d86079e420d@mail.gmail.com> Date: Thu, 28 Sep 2006 02:58:09 -0400 From: "Christopher \"Monty\" Montgomery" To: linux-usb-devel@lists.sourceforge.net Subject: [PATCH 1/15] USB: ehci-hcd: make ehci_iso_stream instances more persistent Cc: greg@kroah.com, david-b@pacbell.net, xiphmont@gmail.com Content-Disposition: inline patch 1: This patch slightly refactors isoch stream cleanup such that stream state is more persistent; it is instantiated at first transfer and not released until endpoint shutdown. This is to give isoch transfers something persistent to associate with bandwidth budget reservations later. Signed-off-by: Christopher "Monty" Montgomery Cc: David Brownell Signed-off-by: Greg Kroah-Hartman --- drivers/usb/host/ehci-hcd.c | 108 ++++++++++++++++++++++++------------------ drivers/usb/host/ehci-sched.c | 12 +++- drivers/usb/host/ehci.h | 8 +++ 3 files changed, 79 insertions(+), 49 deletions(-) --- gregkh-2.6.orig/drivers/usb/host/ehci-hcd.c +++ gregkh-2.6/drivers/usb/host/ehci-hcd.c @@ -812,62 +812,80 @@ ehci_endpoint_disable (struct usb_hcd *h struct ehci_hcd *ehci = hcd_to_ehci (hcd); unsigned long flags; struct ehci_qh *qh, *tmp; + struct ehci_iso_stream *iso; - /* ASSERT: any requests/urbs are being unlinked */ - /* ASSERT: nobody can be submitting urbs for this any more */ + /* ASSERT: any requests/urbs are being unlinked */ + /* ASSERT: nobody can be submitting urbs for this any more */ -rescan: spin_lock_irqsave (&ehci->lock, flags); - qh = ep->hcpriv; - if (!qh) - goto done; + iso = EHCI_EP_STREAM(ep); - /* endpoints can be iso streams. for now, we don't - * accelerate iso completions ... so spin a while. - */ - if (qh->hw_info1 == 0) { - ehci_vdbg (ehci, "iso delay\n"); - goto idle_timeout; - } + if (iso){ + /* for now, we don't accelerate iso completions ... so spin + a while. */ + + while(iso->refcount>1){ + spin_unlock_irqrestore (&ehci->lock, flags); + schedule_timeout_uninterruptible(1); + spin_lock_irqsave (&ehci->lock, flags); + } - if (!HC_IS_RUNNING (hcd->state)) - qh->qh_state = QH_STATE_IDLE; - switch (qh->qh_state) { - case QH_STATE_LINKED: - for (tmp = ehci->async->qh_next.qh; - tmp && tmp != qh; - tmp = tmp->qh_next.qh) - continue; - /* periodic qh self-unlinks on empty */ - if (!tmp) - goto nogood; - unlink_async (ehci, qh); - /* FALL THROUGH */ - case QH_STATE_UNLINK: /* wait for hw to finish? */ -idle_timeout: + /* we want to be sure completions deref the stream to 1, + then we finally pull the plug here */ + iso_stream_put(ehci,iso); + ep->hcpriv = NULL; spin_unlock_irqrestore (&ehci->lock, flags); - schedule_timeout_uninterruptible(1); - goto rescan; - case QH_STATE_IDLE: /* fully unlinked */ - if (list_empty (&qh->qtd_list)) { - qh_put (qh); + return; + } + + while ( (qh = EHCI_EP_QH(ep)) ){ + + if (!HC_IS_RUNNING (hcd->state)) + qh->qh_state = QH_STATE_IDLE; + switch (qh->qh_state) { + case QH_STATE_LINKED: + for (tmp = ehci->async->qh_next.qh; + tmp && tmp != qh; + tmp = tmp->qh_next.qh) + continue; + + /* periodic qh self-unlinks on empty */ + if (!tmp) goto error; + unlink_async (ehci, qh); + /* FALL THROUGH */ + + case QH_STATE_UNLINK: /* wait for hw to finish? */ + case QH_STATE_UNLINK_WAIT: + + spin_unlock_irqrestore (&ehci->lock, flags); + schedule_timeout_uninterruptible(1); + spin_lock_irqsave (&ehci->lock, flags); break; + + case QH_STATE_IDLE: /* fully unlinked */ + + if (list_empty (&qh->qtd_list)) { + qh_put (qh); + ep->hcpriv = NULL; + break; + } + /* else FALL THROUGH */ + default: + goto error; } - /* else FALL THROUGH */ - default: -nogood: - /* caller was supposed to have unlinked any requests; - * that's not our job. just leak this memory. - */ - ehci_err (ehci, "qh %p (#%02x) state %d%s\n", - qh, ep->desc.bEndpointAddress, qh->qh_state, - list_empty (&qh->qtd_list) ? "" : "(has tds)"); - break; } - ep->hcpriv = NULL; -done: + spin_unlock_irqrestore (&ehci->lock, flags); return; + +error: + /* caller was supposed to have unlinked any requests; + * that's not our job. just leak this memory. + */ + ehci_err (ehci, "qh %p (#%02x) state %d%s\n", + qh, ep->desc.bEndpointAddress, qh->qh_state, + list_empty (&qh->qtd_list) ? "" : "(has tds)"); + ep->hcpriv = NULL; } static int ehci_get_frame (struct usb_hcd *hcd) --- gregkh-2.6.orig/drivers/usb/host/ehci-sched.c +++ gregkh-2.6/drivers/usb/host/ehci-sched.c @@ -959,10 +959,14 @@ iso_stream_put(struct ehci_hcd *ehci, st { stream->refcount--; - /* free whenever just a dev->ep reference remains. - * not like a QH -- no persistent state (toggle, halt) - */ - if (stream->refcount == 1) { + /* don't free on last descriptor; free when endpoint disable + finally releases last refcount. Although it is technically + broken for an endpoint driver to submit its streaming + descriptors such that a new one appears after the old one + ends, it is only punishing the users to insist on breaking + these drivers when it's not necessary to do so. This saves + substantial overhead in that case.*/ + if (stream->refcount == 0) { int is_in; // BUG_ON (!list_empty(&stream->td_list)); --- gregkh-2.6.orig/drivers/usb/host/ehci.h +++ gregkh-2.6/drivers/usb/host/ehci.h @@ -604,6 +604,14 @@ struct ehci_fstn { /*-------------------------------------------------------------------------*/ +#define EHCI_EP_QH(x) (((x)->desc.bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) \ + != USB_ENDPOINT_XFER_ISOC ? (x)->hcpriv : NULL) +#define EHCI_EP_STREAM(x) (((x)->desc.bmAttributes & \ + USB_ENDPOINT_XFERTYPE_MASK) == \ + USB_ENDPOINT_XFER_ISOC ? (x)->hcpriv : NULL) + +/*-------------------------------------------------------------------------*/ + #ifdef CONFIG_USB_EHCI_ROOT_HUB_TT /*