On Fri, May 04, 2001, Johannes Erdfelt <[EMAIL PROTECTED]> wrote:
> On Fri, May 04, 2001, Martin Diehl <[EMAIL PROTECTED]> wrote:
> > However, now uhci looses in performance compared to usb-uhci: With my
> > usual 2 queued bulk transfers, len=2112, both to same endpoint,
> > usb-uhci provides >1MB/sec av. throughput - for uhci this number was the
> > same without this patch (until the race happened) and has now dropped to
> > about 700KB/sec.
>
> 1MB/sec is very impressive. I forget exactly what your device did, does
> it connect 2 hosts together?
>
> I can get 805KB/sec with this patch using the usbnet driver which is
> as fast as usb-uhci. I'm still testing and I'll send an email later
> with more tests.
Ok, I've done some more testing now and I found some problems. First, I
sent the wrong patch to the list. Doesn't matter since there were some
dumb bugs in the "correct" version, some of them fatal.
Here's a new updated version. Remove the previous patch first.
JE
diff -ur -X dontdiff linux-2.4.5-pre1.orig/drivers/usb/uhci-debug.h
linux-2.4.5-pre1/drivers/usb/uhci-debug.h
--- linux-2.4.5-pre1.orig/drivers/usb/uhci-debug.h Fri Apr 27 15:50:01 2001
+++ linux-2.4.5-pre1/drivers/usb/uhci-debug.h Fri May 4 15:08:23 2001
@@ -197,7 +197,7 @@
qh, qh->link, qh->element);
if (qh->element & UHCI_PTR_QH)
- out += sprintf(out, "%*s Element points to QH (bug?)\n", space, "");
+ out += sprintf(out, "%*s Element points to QH\n", space, "");
if (qh->element & UHCI_PTR_DEPTH)
out += sprintf(out, "%*s Depth traverse\n", space, "");
@@ -215,13 +215,16 @@
urbp = qh->urbp;
+ if (urbp->bqh == qh)
+ goto out;
+
head = &urbp->td_list;
tmp = head->next;
td = list_entry(tmp, struct uhci_td, list);
if (td->dma_handle != (qh->element & ~UHCI_PTR_BITS))
- out += sprintf(out, "%*s Element != First TD\n", space, "");
+ out += sprintf(out, "%*s Element != First TD\n", space, "");
while (tmp != head) {
struct uhci_td *td = list_entry(tmp, struct uhci_td, list);
@@ -258,6 +261,11 @@
}
prevactive = td->status & TD_CTRL_ACTIVE;
+ }
+
+ if (urbp->bqh) {
+ out += sprintf(out, "%*s Bottom QH:\n", space, "");
+ out += uhci_show_qh(urbp->bqh, out, len - (out - buf), space + 2);
}
if (list_empty(&urbp->queue_list) || urbp->queued)
diff -ur -X dontdiff linux-2.4.5-pre1.orig/drivers/usb/uhci.c
linux-2.4.5-pre1/drivers/usb/uhci.c
--- linux-2.4.5-pre1.orig/drivers/usb/uhci.c Wed May 2 12:25:47 2001
+++ linux-2.4.5-pre1/drivers/usb/uhci.c Fri May 4 15:01:48 2001
@@ -147,12 +147,17 @@
return 0;
}
+static inline void _uhci_set_next_interrupt(struct uhci *uhci)
+{
+ uhci->skel_term_td->status |= TD_CTRL_IOC;
+}
+
static inline void uhci_set_next_interrupt(struct uhci *uhci)
{
unsigned long flags;
spin_lock_irqsave(&uhci->frame_list_lock, flags);
- uhci->skel_term_td->status |= TD_CTRL_IOC;
+ _uhci_set_next_interrupt(uhci);
spin_unlock_irqrestore(&uhci->frame_list_lock, flags);
}
@@ -390,26 +395,31 @@
pci_pool_free(uhci->qh_pool, qh, qh->dma_handle);
}
+static void _uhci_insert_qh(struct uhci_qh *eqh, struct uhci_qh *qh)
+{
+ qh->link = eqh->link;
+ mb(); /* Ordering is important */
+ eqh->link = qh->dma_handle | UHCI_PTR_QH;
+
+ list_add(&qh->list, &eqh->list);
+}
+
static void uhci_insert_qh(struct uhci *uhci, struct uhci_qh *skelqh, struct uhci_qh
*qh)
{
- struct uhci_qh *lqh;
unsigned long flags;
+ struct uhci_qh *lqh;
spin_lock_irqsave(&uhci->frame_list_lock, flags);
/* Grab the last QH */
lqh = list_entry(skelqh->list.prev, struct uhci_qh, list);
- qh->link = lqh->link;
- mb(); /* Ordering is important */
- lqh->link = qh->dma_handle | UHCI_PTR_QH;
-
- list_add_tail(&qh->list, &skelqh->list);
+ _uhci_insert_qh(lqh, qh);
spin_unlock_irqrestore(&uhci->frame_list_lock, flags);
}
-static void uhci_remove_qh(struct uhci *uhci, struct uhci_qh *qh)
+static void _uhci_remove_qh(struct uhci *uhci, struct uhci_qh *qh)
{
unsigned long flags;
struct uhci_qh *prevqh;
@@ -422,8 +432,6 @@
qh->urbp = NULL;
- spin_lock_irqsave(&uhci->frame_list_lock, flags);
-
prevqh = list_entry(qh->list.prev, struct uhci_qh, list);
prevqh->link = qh->link;
@@ -433,20 +441,29 @@
list_del(&qh->list);
INIT_LIST_HEAD(&qh->list);
- spin_unlock_irqrestore(&uhci->frame_list_lock, flags);
-
spin_lock_irqsave(&uhci->qh_remove_list_lock, flags);
/* Check to see if the remove list is empty. Set the IOC bit */
/* to force an interrupt so we can remove the QH */
if (list_empty(&uhci->qh_remove_list))
- uhci_set_next_interrupt(uhci);
+ _uhci_set_next_interrupt(uhci);
list_add(&qh->remove_list, &uhci->qh_remove_list);
spin_unlock_irqrestore(&uhci->qh_remove_list_lock, flags);
}
+static void uhci_remove_qh(struct uhci *uhci, struct uhci_qh *qh)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&uhci->frame_list_lock, flags);
+
+ _uhci_remove_qh(uhci, qh);
+
+ spin_unlock_irqrestore(&uhci->frame_list_lock, flags);
+}
+
static int uhci_fixup_toggle(struct urb *urb, unsigned int toggle)
{
struct urb_priv *urbp = (struct urb_priv *)urb->hcpriv;
@@ -472,17 +489,59 @@
/* This function will append one URB's QH to another URB's QH. This is for */
/* USB_QUEUE_BULK support for bulk transfers and soon implicitily for */
/* control transfers */
-static void uhci_append_queued_urb(struct uhci *uhci, struct urb *eurb, struct urb
*urb)
+static int uhci_append_queued_urb(struct uhci *uhci, struct urb *eurb, struct urb
+*urb)
{
struct urb_priv *eurbp, *urbp, *furbp, *lurbp;
struct list_head *tmp;
- struct uhci_td *ftd, *lltd;
+ struct uhci_td *ftd, *lltd, *ltd;
unsigned long flags;
+ int ret = 0;
+
+ spin_lock_irqsave(&uhci->frame_list_lock, flags);
- eurbp = eurb->hcpriv;
urbp = urb->hcpriv;
- spin_lock_irqsave(&uhci->frame_list_lock, flags);
+ /* Bottom qh which will be stolen by the next URB */
+ urbp->bqh = uhci_alloc_qh(uhci, urb->dev);
+ if (!urbp->bqh) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ urbp->bqh->urbp = urbp;
+
+ ltd = list_entry(urbp->td_list.prev, struct uhci_td, list);
+ ltd->link = urbp->bqh->dma_handle | UHCI_PTR_QH;
+
+ if (!eurb) {
+ struct uhci_qh *lqh;
+
+ /* Next qh, basically a helper qh to get to continue */
+ /* processing the schedule */
+ urbp->nqh = uhci_alloc_qh(uhci, urb->dev);
+ if (!urbp->nqh) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ urbp->bqh->link = urbp->nqh->dma_handle | UHCI_PTR_QH;
+
+ /* Grab the last QH */
+ lqh = list_entry(uhci->skel_bulk_qh->list.prev, struct uhci_qh, list);
+
+ /* We need to make sure we insert these at the same time */
+ _uhci_insert_qh(lqh, urbp->qh);
+ _uhci_insert_qh(urbp->qh, urbp->nqh);
+
+ goto out;
+ }
+
+ eurbp = eurb->hcpriv;
+
+ /* Each group of queued URB's has it's own nqh */
+ urbp->nqh = eurbp->nqh;
+
+ urbp->bqh->link = urbp->nqh->dma_handle | UHCI_PTR_QH;
/* Find the first URB in the queue */
if (eurbp->queued) {
@@ -511,13 +570,16 @@
mb(); /* Make sure we flush everything */
/* Only support bulk right now, so no depth */
- lltd->link = ftd->dma_handle;
+ lurbp->bqh->element = ftd->dma_handle;
list_add_tail(&urbp->queue_list, &furbp->queue_list);
urbp->queued = 1;
+out:
spin_unlock_irqrestore(&uhci->frame_list_lock, flags);
+
+ return ret;
}
static void uhci_delete_queued_urb(struct uhci *uhci, struct urb *urb)
@@ -533,8 +595,14 @@
spin_lock_irqsave(&uhci->frame_list_lock, flags);
- if (list_empty(&urbp->queue_list))
+ if (list_empty(&urbp->queue_list)) {
+ if (urbp->nqh) {
+ _uhci_remove_qh(uhci, urbp->nqh);
+ urbp->nqh = NULL;
+ }
+
goto out;
+ }
nurbp = list_entry(urbp->queue_list.next, struct urb_priv, queue_list);
@@ -571,28 +639,19 @@
usb_pipeout(urb->pipe), toggle);
if (!urbp->queued) {
- int status;
+ struct uhci_qh *pqh;
- /* The HC may continue using the current QH if it finished */
- /* all of the TD's in this URB and may have started on the */
- /* next URB's TD's already, so we'll take over ownership */
- /* of this QH and use it instead. Don't forget to delete */
- /* the old QH first */
+ /* The next URB gets our bottom QH. We don't need the QH */
+ /* we allocated earlier anymore */
uhci_free_qh(uhci, nurbp->qh);
-
- nurbp->qh = urbp->qh;
+ nurbp->qh = urbp->bqh;
nurbp->qh->urbp = nurbp;
- urbp->qh = NULL;
+ urbp->bqh = NULL;
- /* If the last TD from the first (this) urb didn't */
- /* complete, reset qh->element to the first TD in the */
- /* next urb */
- pltd = list_entry(urbp->td_list.prev, struct uhci_td, list);
- status = uhci_status_bits(pltd->status);
- if ((status & TD_CTRL_ACTIVE) || uhci_actual_length(pltd->status) <
uhci_expected_length(pltd->info)) {
- struct uhci_td *ftd = list_entry(nurbp->td_list.next, struct
uhci_td, list);
- nurbp->qh->element = ftd->dma_handle;
- }
+ /* Grab the previous QH */
+ pqh = list_entry(nurbp->nqh->list.prev, struct uhci_qh, list);
+
+ _uhci_insert_qh(pqh, nurbp->qh);
nurbp->queued = 0;
} else {
@@ -1280,9 +1339,13 @@
/* Always assume breadth first */
uhci_insert_tds_in_qh(qh, urb, 1);
- if (urb->transfer_flags & USB_QUEUE_BULK && eurb)
- uhci_append_queued_urb(uhci, eurb, urb);
- else
+ if (urb->transfer_flags & USB_QUEUE_BULK) {
+ int ret;
+
+ ret = uhci_append_queued_urb(uhci, eurb, urb);
+ if (ret < 0)
+ return ret;
+ } else
uhci_insert_qh(uhci, uhci->skel_bulk_qh, qh);
uhci_inc_fsbr(uhci, urb);
@@ -1674,9 +1737,12 @@
uhci_delete_queued_urb(uhci, urb);
+ /* The interrupt loop will reclaim the QH's */
if (urbp->qh)
- /* The interrupt loop will reclaim the QH's */
uhci_remove_qh(uhci, urbp->qh);
+
+ if (urbp->bqh)
+ uhci_remove_qh(uhci, urbp->bqh);
}
/* FIXME: If we forcefully unlink an urb, we should reset the toggle for */
diff -ur -X dontdiff linux-2.4.5-pre1.orig/drivers/usb/uhci.h
linux-2.4.5-pre1/drivers/usb/uhci.h
--- linux-2.4.5-pre1.orig/drivers/usb/uhci.h Fri Apr 27 15:49:56 2001
+++ linux-2.4.5-pre1/drivers/usb/uhci.h Fri May 4 11:39:11 2001
@@ -331,7 +331,8 @@
dma_addr_t setup_packet_dma_handle;
dma_addr_t transfer_buffer_dma_handle;
- struct uhci_qh *qh; /* QH for this URB */
+ struct uhci_qh *qh;
+ struct uhci_qh *bqh, *nqh; /* For queued URB's */
struct list_head td_list;
int fsbr : 1; /* URB turned on FSBR */
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
http://lists.sourceforge.net/lists/listinfo/linux-usb-devel
