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

[Karsten Wiese]

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 <***@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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
linux-usb-***@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

[David Brownell]

Post by Karsten Wiese
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.

Looks interesting, but could you make this apply to *BOTH* types of
ISO transfer descriptor? You did it for ITDs, but not Split-ITDs.

Plus there are two non-technical problems with this patch. First,
it doesn't apply against 2.6.24-rc2-git; second, it triggers lots
of messages from scripts/checkpatch.pl ... both issues need to get
fixed before I can properly review this.

- Dave

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
linux-usb-***@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

[Karsten Wiese]

Post by David Brownell
Looks interesting, but could you make this apply to *BOTH* types of
ISO transfer descriptor? You did it for ITDs, but not Split-ITDs.

Done. Split-ITD changes untested.

Post by David Brownell
Plus there are two non-technical problems with this patch. First,
it doesn't apply against 2.6.24-rc2-git; second, it triggers lots
of messages from scripts/checkpatch.pl ... both issues need to get
fixed before I can properly review this.

Fixed.

Karsten

------------------------------------->

From: Karsten Wiese <***@wemgehoertderstaat.de>

ehci-hcd: complete iso urbs ASAP for number_of_packets != (n * 8) also

Upstream 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 ITDs 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.
SITD handling is unchanged, only sitd_complete() is renamed to sitd_scan() and
the active check is moved to sitd_scan() to make the "case Q_TYPE_ITD:" and
"case Q_TYPE_SITD:" sections under scan_periodic() look alike.

Signed-off-by: Karsten Wiese <***@wemgehoertderstaat.de>
---
drivers/usb/host/ehci-sched.c | 156 ++++++++++++++++++++---------------------
drivers/usb/host/ehci.h | 3 +-
2 files changed, 76 insertions(+), 83 deletions(-)

diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c
index 80d99bc..e647842 100644
--- a/drivers/usb/host/ehci-sched.c
+++ b/drivers/usb/host/ehci-sched.c
@@ -1486,6 +1486,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 */
@@ -1558,30 +1560,28 @@ 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++) {
+ struct usb_device *dev;
+
if (likely (itd->index[uframe] == -1))
continue;
+
urb_index = itd->index[uframe];
desc = &urb->iso_frame_desc [urb_index];

@@ -1608,45 +1608,54 @@ itd_complete (
desc->status = 0;
desc->actual_length = EHCI_ITD_LENGTH (t);
}
- }

- usb_put_urb (urb);
- itd->urb = NULL;
- itd->stream = NULL;
- 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;
+ /* handle completion now? */
+ if (urb_index + 1 != urb->number_of_packets)
+ continue;

- /* 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);
- */
+ 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, 0);
- urb = NULL;
+ /* give urb back to the driver ... can be out-of-order */
+ dev = urb->dev;
+ ehci_urb_done(ehci, urb, 0);
+ 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--;
+ ehci_to_hcd(ehci)->self.bandwidth_isoc_reqs--;

- if (unlikely (list_empty (&stream->td_list))) {
- ehci_to_hcd(ehci)->self.bandwidth_allocated
+ 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");
+ 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);
}
+
+ 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);

- return 1;
+ /* defer stopping schedule; completion can submit */
+ ehci->periodic_sched--;
+ if (unlikely(!ehci->periodic_sched))
+ disable_periodic(ehci);
+
+ return true;
}

/*-------------------------------------------------------------------------*/
@@ -1950,11 +1959,9 @@ sitd_link_urb (
#define SITD_ERRS (SITD_STS_ERR | SITD_STS_DBE | SITD_STS_BABBLE \
| SITD_STS_XACT | SITD_STS_MMF)

-static unsigned
-sitd_complete (
- struct ehci_hcd *ehci,
- struct ehci_sitd *sitd
-) {
+static bool
+sitd_scan(struct ehci_hcd *ehci, struct ehci_sitd *sitd, int live)
+{
struct urb *urb = sitd->urb;
struct usb_iso_packet_descriptor *desc;
u32 t;
@@ -1962,9 +1969,12 @@ sitd_complete (
struct ehci_iso_stream *stream = sitd->stream;
struct usb_device *dev;

+ t = hc32_to_cpup(ehci, &sitd->hw_results);
+ if (t & SITD_STS_ACTIVE && live)
+ return false;
+
urb_index = sitd->index;
desc = &urb->iso_frame_desc [urb_index];
- t = hc32_to_cpup(ehci, &sitd->hw_results);

/* report transfer status */
if (t & SITD_ERRS) {
@@ -1991,7 +2001,7 @@ sitd_complete (

/* handle completion now? */
if ((urb_index + 1) != urb->number_of_packets)
- return 0;
+ return true;

/* ASSERT: it's really the last sitd for this urb
list_for_each_entry (sitd, &stream->td_list, sitd_list)
@@ -2019,7 +2029,7 @@ sitd_complete (
}
iso_stream_put (ehci, stream);

- return 1;
+ return true;
}


@@ -2091,13 +2101,11 @@ sitd_submit (struct ehci_hcd *ehci, struct urb *urb, gfp_t mem_flags)
return -ENOSYS;
}

-static inline unsigned
-sitd_complete (
- struct ehci_hcd *ehci,
- struct ehci_sitd *sitd
-) {
- ehci_err (ehci, "sitd_complete %p?\n", sitd);
- return 0;
+static inline bool
+sitd_scan(struct ehci_hcd *ehci, struct ehci_sitd *sitd)
+{
+ ehci_err(ehci, "%s %p?\n", __FUNCTION__, sitd);
+ return false;
}

#endif /* USB_EHCI_SPLIT_ISO */
@@ -2175,47 +2183,33 @@ 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:
- if ((q.sitd->hw_results & SITD_ACTIVE(ehci))
- && live) {
+ modified = sitd_scan(ehci, q.sitd, live);
+ if (!modified) {
q_p = &q.sitd->sitd_next;
hw_p = &q.sitd->hw_next;
- type = Q_NEXT_TYPE(ehci,
- q.sitd->hw_next);
- q = *q_p;
- break;
+ } else {
+ *q_p = q.sitd->sitd_next;
+ *hw_p = q.sitd->hw_next;
}
- *q_p = q.sitd->sitd_next;
- *hw_p = q.sitd->hw_next;
type = Q_NEXT_TYPE(ehci, q.sitd->hw_next);
+
wmb();
- modified = sitd_complete (ehci, q.sitd);
q = *q_p;
break;
default:
diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
index 951d69f..156c02f 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 */
@@ -615,8 +616,6 @@ struct ehci_sitd {
#define SITD_STS_MMF (1 << 2) /* incomplete split transaction */
#define SITD_STS_STS (1 << 1) /* split transaction state */

-#define SITD_ACTIVE(ehci) cpu_to_hc32(ehci, SITD_STS_ACTIVE)
-
__hc32 hw_buf [2]; /* EHCI table 3-12 */
__hc32 hw_backpointer; /* EHCI table 3-13 */
__hc32 hw_buf_hi [2]; /* Appendix B */

[Karsten Wiese]

Post by Karsten Wiese
Fixed.

erm, might unnecesarily rescan and the !USB_EHCI_SPLIT_ISO case misses an
argument. Will repost.

Karsten

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
linux-usb-***@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

[Karsten Wiese]

Post by Karsten Wiese
erm, might unnecesarily rescan and the !USB_EHCI_SPLIT_ISO case misses an
argument. Will repost.

Here goes take 3: It makes the

if (unlikely (modified))
goto restart;

part of scan_periodic() work like upstream again and corrects the
!USB_EHCI_SPLIT_ISO issue.

Karsten

---------------------------->

From: Karsten Wiese <***@wemgehoertderstaat.de>

ehci-hcd: complete iso urbs ASAP for number_of_packets != (n * 8) also

Upstream 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 ITDs 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.
SITD handling is unchanged, only sitd_complete() is renamed to sitd_scan() and
the active check is moved to sitd_scan() to make the "case Q_TYPE_ITD:" and
"case Q_TYPE_SITD:" sections under scan_periodic() look alike.

Signed-off-by: Karsten Wiese <***@wemgehoertderstaat.de>
---
drivers/usb/host/ehci-sched.c | 157 +++++++++++++++++++++--------------------
drivers/usb/host/ehci.h | 3 +-
2 files changed, 80 insertions(+), 80 deletions(-)

diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c
index 80d99bc..2ae5c0a 100644
--- a/drivers/usb/host/ehci-sched.c
+++ b/drivers/usb/host/ehci-sched.c
@@ -1486,6 +1486,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 */
@@ -1558,30 +1560,32 @@ 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)
+/* return values used by itd_scan() and sitd_scan() */
+#define XITD_ELAPSED 1
+#define URB_COMPLETE 2

+#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
-) {
+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;
+ unsigned complete = 0;

/* for each uframe with a packet */
- for (uframe = 0; uframe < 8; uframe++) {
+ for (uframe = itd->uframe_scanned; uframe < uframe_after; uframe++) {
+ struct usb_device *dev;
+
if (likely (itd->index[uframe] == -1))
continue;
+
urb_index = itd->index[uframe];
desc = &urb->iso_frame_desc [urb_index];

@@ -1608,45 +1612,55 @@ itd_complete (
desc->status = 0;
desc->actual_length = EHCI_ITD_LENGTH (t);
}
- }

- usb_put_urb (urb);
- itd->urb = NULL;
- itd->stream = NULL;
- 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;
+ /* handle completion now? */
+ if (urb_index + 1 != urb->number_of_packets)
+ continue;

- /* 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);
- */
+ 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, 0);
- urb = NULL;
+ /* give urb back to the driver ... can be out-of-order */
+ dev = urb->dev;
+ ehci_urb_done(ehci, urb, 0);
+ urb = NULL;
+ complete = URB_COMPLETE;

- /* 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--;
+ ehci_to_hcd(ehci)->self.bandwidth_isoc_reqs--;

- if (unlikely (list_empty (&stream->td_list))) {
- ehci_to_hcd(ehci)->self.bandwidth_allocated
+ 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");
+ 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);
}
+
+ itd->uframe_scanned = uframe_after;
+ if (uframe_after < 8)
+ return complete;
+
+ itd->stream = NULL;
+ itd->uframe_scanned = 0;
+ list_move(&itd->itd_list, &stream->free_list);
iso_stream_put (ehci, stream);

- return 1;
+ /* defer stopping schedule; completion can submit */
+ ehci->periodic_sched--;
+ if (unlikely(!ehci->periodic_sched))
+ disable_periodic(ehci);
+
+ return complete | XITD_ELAPSED;
}

/*-------------------------------------------------------------------------*/
@@ -1951,10 +1965,8 @@ sitd_link_urb (
| SITD_STS_XACT | SITD_STS_MMF)

static unsigned
-sitd_complete (
- struct ehci_hcd *ehci,
- struct ehci_sitd *sitd
-) {
+sitd_scan(struct ehci_hcd *ehci, struct ehci_sitd *sitd, int live)
+{
struct urb *urb = sitd->urb;
struct usb_iso_packet_descriptor *desc;
u32 t;
@@ -1962,9 +1974,12 @@ sitd_complete (
struct ehci_iso_stream *stream = sitd->stream;
struct usb_device *dev;

+ t = hc32_to_cpup(ehci, &sitd->hw_results);
+ if (t & SITD_STS_ACTIVE && live)
+ return 0;
+
urb_index = sitd->index;
desc = &urb->iso_frame_desc [urb_index];
- t = hc32_to_cpup(ehci, &sitd->hw_results);

/* report transfer status */
if (t & SITD_ERRS) {
@@ -1991,7 +2006,7 @@ sitd_complete (

/* handle completion now? */
if ((urb_index + 1) != urb->number_of_packets)
- return 0;
+ return XITD_ELAPSED;

/* ASSERT: it's really the last sitd for this urb
list_for_each_entry (sitd, &stream->td_list, sitd_list)
@@ -2019,7 +2034,7 @@ sitd_complete (
}
iso_stream_put (ehci, stream);

- return 1;
+ return XITD_ELAPSED | URB_COMPLETE;
}


@@ -2092,11 +2107,9 @@ sitd_submit (struct ehci_hcd *ehci, struct urb *urb, gfp_t mem_flags)
}

static inline unsigned
-sitd_complete (
- struct ehci_hcd *ehci,
- struct ehci_sitd *sitd
-) {
- ehci_err (ehci, "sitd_complete %p?\n", sitd);
+sitd_scan(struct ehci_hcd *ehci, struct ehci_sitd *sitd, int live)
+{
+ ehci_err(ehci, "%s %p?\n", __FUNCTION__, sitd);
return 0;
}

@@ -2175,47 +2188,35 @@ 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 & XITD_ELAPSED) {
+ modified &= ~XITD_ELAPSED;
+ *q_p = q.itd->itd_next;
+ *hw_p = q.itd->hw_next;
+ } else {
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;
}
- 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:
- if ((q.sitd->hw_results & SITD_ACTIVE(ehci))
- && live) {
+ modified = sitd_scan(ehci, q.sitd, live);
+ if (modified & XITD_ELAPSED) {
+ modified &= ~XITD_ELAPSED;
+ *q_p = q.sitd->sitd_next;
+ *hw_p = q.sitd->hw_next;
+ } else {
q_p = &q.sitd->sitd_next;
hw_p = &q.sitd->hw_next;
- type = Q_NEXT_TYPE(ehci,
- q.sitd->hw_next);
- q = *q_p;
- break;
}
- *q_p = q.sitd->sitd_next;
- *hw_p = q.sitd->hw_next;
type = Q_NEXT_TYPE(ehci, q.sitd->hw_next);
+
wmb();
- modified = sitd_complete (ehci, q.sitd);
q = *q_p;
break;
default:
diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
index 951d69f..156c02f 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 */
@@ -615,8 +616,6 @@ struct ehci_sitd {
#define SITD_STS_MMF (1 << 2) /* incomplete split transaction */
#define SITD_STS_STS (1 << 1) /* split transaction state */

-#define SITD_ACTIVE(ehci) cpu_to_hc32(ehci, SITD_STS_ACTIVE)
-
__hc32 hw_buf [2]; /* EHCI table 3-12 */
__hc32 hw_backpointer; /* EHCI table 3-13 */
__hc32 hw_buf_hi [2]; /* Appendix B */
--
1.5.3.3


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
linux-usb-***@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

[David Brownell]

Post by Karsten Wiese
ITDs stay kept in the hcd's schedule until the complete frame they cover
has elapsed, like without patch.

Did you try removing them immediately, and find some problem?

I've never been sure how that hardware schedule caching thing
is supposed to work. Ideally it would only apply to (s)ITDs
that were active, and only to the fields used to perform the
data transfer ... rather than the schedule tree itself. If
it cached the tree (pointers to next (s)ITD or QH) that could
be trouble; ditto caching records for "expired" descriptors.

- Dave


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
linux-usb-***@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

[Karsten Wiese]

Post by David Brownell

Post by Karsten Wiese
ITDs stay kept in the hcd's schedule until the complete frame they cover
has elapsed, like without patch.

Did you try removing them immediately, and find some problem?

Yes, don't remember the exact symptoms though.
I clearly remember it started to work , if ITDs were left in the hardware
schedule, until frame elapsed (uframe % 8 == 0).

Post by David Brownell
I've never been sure how that hardware schedule caching thing
is supposed to work. Ideally it would only apply to (s)ITDs
that were active, and only to the fields used to perform the
data transfer ... rather than the schedule tree itself. If
it cached the tree (pointers to next (s)ITD or QH) that could
be trouble; ditto caching records for "expired" descriptors.

Seams to me as if it cached the tree. Maybe thats implentation
dependant. I test on VT8237 now and found the same once on a 06ish
intel laptop.

Karsten

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
linux-usb-***@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

[David Brownell]

Can you give this updated version a try? There were some glitches
in the hardware schedule updating vs (s)ITD recycling, plus you only
relocated part of the ITD completion path. I also updated the patch
description.

This version is just compile-tested.

- Dave

======================
From: Karsten Wiese <***@wemgehoertderstaat.de>

ehci-hcd: complete ISO URBs sooner

This patch lets some ISO URBs complete sooner, by scanning ITDs and sITDs up
to the current microframe and then reporting completions immediately.

Previously some completions for high speed transfers wouldn't be reported
right away, because the code waited until a complete frame was scanned and
such a scan might not happen immediately. Soundcard drivers have used a
partial work around for this, transferring ISO urbs using entire frames.

ISO transfers now activate/deactivate the periodic schedule based on when
an DMA descriptor is added to the schedule, rather than slightly later when
the URB is fully scheduled. The scanning logic is simplified a bit, making
the three results distinct: a descriptor may still need processing; or it
may have completed, either with or without an URB completion callback.

---
drivers/usb/host/ehci-sched.c | 181 ++++++++++++++++++++++--------------------
drivers/usb/host/ehci.h | 3
2 files changed, 96 insertions(+), 88 deletions(-)

--- g26.orig/drivers/usb/host/ehci-sched.c 2007-10-12 16:27:05.000000000 -0700
+++ g26/drivers/usb/host/ehci-sched.c 2007-11-11 23:56:43.000000000 -0800
@@ -1486,6 +1486,8 @@ itd_link (struct ehci_hcd *ehci, unsigne
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 */
@@ -1558,30 +1560,33 @@ itd_link_urb (
urb->hcpriv = NULL;

timer_action (ehci, TIMER_IO_WATCHDOG);
- if (unlikely (!ehci->periodic_sched++))
- return enable_periodic (ehci);
return 0;
}

+/* mask for return values used by itd_scan() and sitd_scan() */
+#define XITD_UNLINK (1 << 0) /* needs unlink from hw schedule */
+#define URB_COMPLETE (1 << 1) /* schedule may have new entries */
+
#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
-) {
+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;
+ unsigned complete = 0;

/* for each uframe with a packet */
- for (uframe = 0; uframe < 8; uframe++) {
+ for (uframe = itd->uframe_scanned; uframe < uframe_after; uframe++) {
+ struct usb_device *dev;
+
if (likely (itd->index[uframe] == -1))
continue;
+
urb_index = itd->index[uframe];
desc = &urb->iso_frame_desc [urb_index];

@@ -1607,46 +1612,66 @@ itd_complete (
} else if (likely ((t & EHCI_ISOC_ACTIVE) == 0)) {
desc->status = 0;
desc->actual_length = EHCI_ITD_LENGTH (t);
+ } else {
+ /* SHOULD NOT HAPPEN!! */
+ WARN_ON(1);
}
- }

- usb_put_urb (urb);
- itd->urb = NULL;
- itd->stream = NULL;
- 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;
+ /* handle completion now? */
+ if (urb_index + 1 != urb->number_of_packets)
+ continue;

- /* 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);
- */
+#if 1
+ /* ASSERT: it's really the last itd for this urb */
+ {
+ struct ehci_itd *tmp;

- /* give urb back to the driver ... can be out-of-order */
- dev = urb->dev;
- ehci_urb_done(ehci, urb, 0);
- urb = NULL;
+ list_for_each_entry(tmp, &stream->td_list, itd_list)
+ BUG_ON(tmp != itd && tmp->urb == urb);
+ }
+#endif

- /* 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--;
+ /* give urb back to the driver ... can be out-of-order */
+ dev = urb->dev;
+ ehci_urb_done(ehci, urb, 0);
+ urb = NULL;
+ complete = URB_COMPLETE;
+
+ /* deferred schedule disable; completion often submits */
+ ehci->periodic_sched--;
+ if (unlikely(!ehci->periodic_sched))
+ 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);
+ goto recycle;
+ }
+ itd->uframe_scanned = uframe_after;

- 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");
+ /* If this ITD still has active transfers, leave it scheduled */
+ while (uframe < 8) {
+ if (itd->index[uframe++] != -1)
+ return complete;
}
- iso_stream_put (ehci, stream);

- return 1;
+recycle:
+ usb_put_urb(urb);
+ itd->urb = NULL;
+ itd->stream = NULL;
+ list_move(&itd->itd_list, &stream->free_list);
+ iso_stream_put(ehci, stream);
+
+ return complete | XITD_UNLINK;
}

/*-------------------------------------------------------------------------*/
@@ -1878,6 +1903,8 @@ sitd_link (struct ehci_hcd *ehci, unsign
sitd->frame = frame;
wmb ();
ehci->periodic[frame] = cpu_to_hc32(ehci, sitd->sitd_dma | Q_TYPE_SITD);
+ if (unlikely(!ehci->periodic_sched++))
+ return enable_periodic(ehci);
}

/* fit urb's sitds into the selected schedule slot; activate as needed */
@@ -1940,8 +1967,6 @@ sitd_link_urb (
urb->hcpriv = NULL;

timer_action (ehci, TIMER_IO_WATCHDOG);
- if (!ehci->periodic_sched++)
- return enable_periodic (ehci);
return 0;
}

@@ -1951,10 +1976,8 @@ sitd_link_urb (
| SITD_STS_XACT | SITD_STS_MMF)

static unsigned
-sitd_complete (
- struct ehci_hcd *ehci,
- struct ehci_sitd *sitd
-) {
+sitd_scan(struct ehci_hcd *ehci, struct ehci_sitd *sitd, int live)
+{
struct urb *urb = sitd->urb;
struct usb_iso_packet_descriptor *desc;
u32 t;
@@ -1962,9 +1985,12 @@ sitd_complete (
struct ehci_iso_stream *stream = sitd->stream;
struct usb_device *dev;

+ t = hc32_to_cpup(ehci, &sitd->hw_results);
+ if ((t & SITD_STS_ACTIVE) && live)
+ return 0;
+
urb_index = sitd->index;
desc = &urb->iso_frame_desc [urb_index];
- t = hc32_to_cpup(ehci, &sitd->hw_results);

/* report transfer status */
if (t & SITD_ERRS) {
@@ -1991,7 +2017,7 @@ sitd_complete (

/* handle completion now? */
if ((urb_index + 1) != urb->number_of_packets)
- return 0;
+ return XITD_UNLINK;

/* ASSERT: it's really the last sitd for this urb
list_for_each_entry (sitd, &stream->td_list, sitd_list)
@@ -2019,7 +2045,7 @@ sitd_complete (
}
iso_stream_put (ehci, stream);

- return 1;
+ return XITD_UNLINK | URB_COMPLETE;
}


@@ -2092,11 +2118,9 @@ sitd_submit (struct ehci_hcd *ehci, stru
}

static inline unsigned
-sitd_complete (
- struct ehci_hcd *ehci,
- struct ehci_sitd *sitd
-) {
- ehci_err (ehci, "sitd_complete %p?\n", sitd);
+sitd_scan(struct ehci_hcd *ehci, struct ehci_sitd *sitd, int live)
+{
+ ehci_err(ehci, "%s %p?\n", __FUNCTION__, sitd);
return 0;
}

@@ -2148,7 +2172,6 @@ restart:
modified = 0;

while (q.ptr != NULL) {
- unsigned uf;
union ehci_shadow temp;
int live;

@@ -2175,47 +2198,33 @@ 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;
+ type = Q_NEXT_TYPE(ehci, q.itd->hw_next);
+ modified = itd_scan(ehci, q.itd,
+ live ? uframes : 8);
+ if (modified & XITD_UNLINK) {
+ modified &= ~XITD_UNLINK;
+ *q_p = q.itd->itd_next;
+ *hw_p = q.itd->hw_next;
+ wmb();
+ } else {
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;
}
- 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:
- if ((q.sitd->hw_results & SITD_ACTIVE(ehci))
- && live) {
+ type = Q_NEXT_TYPE(ehci, q.sitd->hw_next);
+ modified = sitd_scan(ehci, q.sitd, live);
+ if (modified & XITD_UNLINK) {
+ modified &= ~XITD_UNLINK;
+ *q_p = q.sitd->sitd_next;
+ *hw_p = q.sitd->hw_next;
+ wmb();
+ } else {
q_p = &q.sitd->sitd_next;
hw_p = &q.sitd->hw_next;
- type = Q_NEXT_TYPE(ehci,
- q.sitd->hw_next);
- q = *q_p;
- break;
}
- *q_p = q.sitd->sitd_next;
- *hw_p = q.sitd->hw_next;
- type = Q_NEXT_TYPE(ehci, q.sitd->hw_next);
- wmb();
- modified = sitd_complete (ehci, q.sitd);
q = *q_p;
break;
default:
--- g26.orig/drivers/usb/host/ehci.h 2007-11-11 15:10:19.000000000 -0800
+++ g26/drivers/usb/host/ehci.h 2007-11-11 19:34:24.000000000 -0800
@@ -589,6 +589,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 */
@@ -623,8 +624,6 @@ struct ehci_sitd {
#define SITD_STS_MMF (1 << 2) /* incomplete split transaction */
#define SITD_STS_STS (1 << 1) /* split transaction state */

-#define SITD_ACTIVE(ehci) cpu_to_hc32(ehci, SITD_STS_ACTIVE)
-
__hc32 hw_buf [2]; /* EHCI table 3-12 */
__hc32 hw_backpointer; /* EHCI table 3-13 */
__hc32 hw_buf_hi [2]; /* Appendix B */


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
linux-usb-***@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

[Karsten Wiese]

Post by David Brownell
Can you give this updated version a try? There were some glitches
in the hardware schedule updating vs (s)ITD recycling, plus you only
relocated part of the ITD completion path. I also updated the patch
description.

Wrongly gives back empty in urb uframes to the soundcarddriver.
My patch changes ehci->periodic_sched urb wise for SITD transactions
and ITD wise for high_speed iso.
ITD wise that makes the "tree-ish hardware-caching" possible.
Is it glitchy for SITDs because its wrong somehow or because its
inconvinient to differ from ITD mode?

Post by David Brownell
--- g26.orig/drivers/usb/host/ehci-sched.c 2007-10-12 16:27:05.000000000 -0700
+++ g26/drivers/usb/host/ehci-sched.c 2007-11-11 23:56:43.000000000 -0800
@@ -1486,6 +1486,8 @@ itd_link (struct ehci_hcd *ehci, unsigne
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 */
@@ -1558,30 +1560,33 @@ itd_link_urb (
urb->hcpriv = NULL;
timer_action (ehci, TIMER_IO_WATCHDOG);
- if (unlikely (!ehci->periodic_sched++))
- return enable_periodic (ehci);
return 0;
}

Here you add to ehci->periodic_sched per ITD.

Post by David Brownell
+/* mask for return values used by itd_scan() and sitd_scan() */
+#define XITD_UNLINK (1 << 0) /* needs unlink from hw schedule */
+#define URB_COMPLETE (1 << 1) /* schedule may have new entries */
+
#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
-) {
+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;
+ unsigned complete = 0;
/* for each uframe with a packet */
- for (uframe = 0; uframe < 8; uframe++) {
+ for (uframe = itd->uframe_scanned; uframe < uframe_after; uframe++) {
+ struct usb_device *dev;
+
if (likely (itd->index[uframe] == -1))
continue;
+
urb_index = itd->index[uframe];
desc = &urb->iso_frame_desc [urb_index];
@@ -1607,46 +1612,66 @@ itd_complete (
} else if (likely ((t & EHCI_ISOC_ACTIVE) == 0)) {
desc->status = 0;
desc->actual_length = EHCI_ITD_LENGTH (t);
+ } else {
+ /* SHOULD NOT HAPPEN!! */
+ WARN_ON(1);
}
- }
- usb_put_urb (urb);
- itd->urb = NULL;
- itd->stream = NULL;
- 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;
+ /* handle completion now? */
+ if (urb_index + 1 != urb->number_of_packets)
+ continue;
- /* 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);
- */
+#if 1
+ /* ASSERT: it's really the last itd for this urb */
+ {
+ struct ehci_itd *tmp;
- /* give urb back to the driver ... can be out-of-order */
- dev = urb->dev;
- ehci_urb_done(ehci, urb, 0);
- urb = NULL;
+ list_for_each_entry(tmp, &stream->td_list, itd_list)
+ BUG_ON(tmp != itd && tmp->urb == urb);
+ }
+#endif
- /* 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--;
+ /* give urb back to the driver ... can be out-of-order */
+ dev = urb->dev;
+ ehci_urb_done(ehci, urb, 0);
+ urb = NULL;
+ complete = URB_COMPLETE;
+
+ /* deferred schedule disable; completion often submits */
+ ehci->periodic_sched--;
+ if (unlikely(!ehci->periodic_sched))
+ disable_periodic(ehci);

here ehci->periodic_sched-- is done per urb. That wasn't intended I guess?

Post by David Brownell
+ 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);
+ goto recycle;
+ }
+ itd->uframe_scanned = uframe_after;
- 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");
+ /* If this ITD still has active transfers, leave it scheduled */
+ while (uframe < 8) {
+ if (itd->index[uframe++] != -1)
+ return complete;
}
- iso_stream_put (ehci, stream);
- return 1;
+ usb_put_urb(urb);
+ itd->urb = NULL;
+ itd->stream = NULL;
+ list_move(&itd->itd_list, &stream->free_list);
+ iso_stream_put(ehci, stream);
+
+ return complete | XITD_UNLINK;
}
/*-------------------------------------------------------------------------*/
@@ -1878,6 +1903,8 @@ sitd_link (struct ehci_hcd *ehci, unsign
sitd->frame = frame;
wmb ();
ehci->periodic[frame] = cpu_to_hc32(ehci, sitd->sitd_dma | Q_TYPE_SITD);
+ if (unlikely(!ehci->periodic_sched++))
+ return enable_periodic(ehci);
}
/* fit urb's sitds into the selected schedule slot; activate as needed */
@@ -1940,8 +1967,6 @@ sitd_link_urb (
urb->hcpriv = NULL;
timer_action (ehci, TIMER_IO_WATCHDOG);
- if (!ehci->periodic_sched++)
- return enable_periodic (ehci);
return 0;
}

ehci->periodic_sched++ per SITD.
Really needed or just nicer to mimic TDI mode?

Post by David Brownell
@@ -1951,10 +1976,8 @@ sitd_link_urb (
| SITD_STS_XACT | SITD_STS_MMF)
static unsigned
-sitd_complete (
- struct ehci_hcd *ehci,
- struct ehci_sitd *sitd
-) {
+sitd_scan(struct ehci_hcd *ehci, struct ehci_sitd *sitd, int live)
+{
struct urb *urb = sitd->urb;
struct usb_iso_packet_descriptor *desc;
u32 t;
@@ -1962,9 +1985,12 @@ sitd_complete (
struct ehci_iso_stream *stream = sitd->stream;
struct usb_device *dev;
+ t = hc32_to_cpup(ehci, &sitd->hw_results);
+ if ((t & SITD_STS_ACTIVE) && live)
+ return 0;
+
urb_index = sitd->index;
desc = &urb->iso_frame_desc [urb_index];
- t = hc32_to_cpup(ehci, &sitd->hw_results);
/* report transfer status */
if (t & SITD_ERRS) {
@@ -1991,7 +2017,7 @@ sitd_complete (
/* handle completion now? */
if ((urb_index + 1) != urb->number_of_packets)
- return 0;
+ return XITD_UNLINK;
/* ASSERT: it's really the last sitd for this urb
list_for_each_entry (sitd, &stream->td_list, sitd_list)
@@ -2019,7 +2045,7 @@ sitd_complete (
}
iso_stream_put (ehci, stream);
- return 1;
+ return XITD_UNLINK | URB_COMPLETE;
}
@@ -2092,11 +2118,9 @@ sitd_submit (struct ehci_hcd *ehci, stru
}
static inline unsigned
-sitd_complete (
- struct ehci_hcd *ehci,
- struct ehci_sitd *sitd
-) {
- ehci_err (ehci, "sitd_complete %p?\n", sitd);
+sitd_scan(struct ehci_hcd *ehci, struct ehci_sitd *sitd, int live)
+{
+ ehci_err(ehci, "%s %p?\n", __FUNCTION__, sitd);
return 0;
}
modified = 0;
while (q.ptr != NULL) {
- unsigned uf;
union ehci_shadow temp;
int live;
q = q.fstn->fstn_next;
break;
- /* 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;
+ type = Q_NEXT_TYPE(ehci, q.itd->hw_next);
+ modified = itd_scan(ehci, q.itd,
+ live ? uframes : 8);
+ if (modified & XITD_UNLINK) {
+ modified &= ~XITD_UNLINK;
+ *q_p = q.itd->itd_next;
+ *hw_p = q.itd->hw_next;
+ wmb();

Put wmb() here to keep hardware happy, don't need it in the else branch?

I've got two more patches here to reduce struct ehci_itd size by removing
index[] and usecs[]. They size ehci_itd down to 160bytes.
Thats 2 32byte alignment steps here on x86_64.
Should I post "next take" with wmb() tweeks and ehci_idt slimming?
Add per SITD ehci->periodic_sched++?

Karsten

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
linux-usb-***@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

[Karsten Wiese]

Post by Karsten Wiese
Is it glitchy for SITDs because its wrong somehow or because its
inconvinient to differ from ITD mode?

Found a big glitch: put (s)idt on the freelist, then complete urb,
then update schedule based on a (s)itd which is reused by submition
during completion.

Karsten

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
linux-usb-***@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

[Karsten Wiese]

Hi David!

Differences in this version:

In patch 1:
- itd_complete() split into 3 functions, assures schedule is updated
before ITD is recycled and lets
- updates to ehci->periodic_sched happen per urb again
- SITD handling unchanged. Didn't find anything to improve.

New patches 2 & 3
- Reduce dma-memory usage by slimming struct ehci_itd.
On x86_64 patch 1 makes ehci_itd grow 1 32byte alignment step,
patch 2 & 3 let ehci_itd take 1 32byte alignment less each.

kind regards,
Karsten

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
linux-usb-***@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

[David Brownell]

Post by Karsten Wiese
Hi David!
- itd_complete() split into 3 functions, assures schedule is updated
before ITD is recycled and lets
- updates to ehci->periodic_sched happen per urb again
- SITD handling unchanged. Didn't find anything to improve.

I've been looking that over, and not liking the complexity.
Seems to me a far simpler patch should solve this. I'll
post it in a minute.

Post by Karsten Wiese
New patches 2 & 3
- Reduce dma-memory usage by slimming struct ehci_itd.
On x86_64 patch 1 makes ehci_itd grow 1 32byte alignment step,
patch 2 & 3 let ehci_itd take 1 32byte alignment less each.

These are nice. I never liked how bloated the ITD structure
had gotten. :)

I already signed off on an updated version of #3 ... I'll split
out the fix in #2 from your patch #2 and send it along too.

- Dave

p.s. further traffic to linux-***@vger ...

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
linux-usb-***@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

[Karsten Wiese]

ehci_itd's member index[8] stored the offset in an urb's
iso_frame_desc[] for status updates while in itd_scan().
Replace member index[] by new member urb_index.
urb_index is updated in itd_scan() to provide the same
information as index[uframe] did before.
Saves 6*4=24 bytes.

Signed-off-by: Karsten Wiese <***@wemgehoertderstaat.de>
---
drivers/usb/host/ehci-dbg.c | 6 ++----
drivers/usb/host/ehci-sched.c | 25 +++++++++++--------------
drivers/usb/host/ehci.h | 4 +++-
3 files changed, 16 insertions(+), 19 deletions(-)

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index c9cc441..ef6ea56 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -165,10 +165,8 @@ dbg_itd (const char *label, struct ehci_hcd *ehci, struct ehci_itd *itd)
hc32_to_cpu(ehci, itd->hw_bufp[4]),
hc32_to_cpu(ehci, itd->hw_bufp[5]),
hc32_to_cpu(ehci, itd->hw_bufp[6]));
- ehci_dbg (ehci, " index: %d %d %d %d %d %d %d %d\n",
- itd->index[0], itd->index[1], itd->index[2],
- itd->index[3], itd->index[4], itd->index[5],
- itd->index[6], itd->index[7]);
+ ehci_dbg(ehci, " urb_index: %d uframe_scan: %d\n",
+ itd->urb_index, itd->uframe_scan);
}

static void __maybe_unused
diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c
index 2d4fda5..38a7901 100644
--- a/drivers/usb/host/ehci-sched.c
+++ b/drivers/usb/host/ehci-sched.c
@@ -1430,17 +1430,12 @@ static inline void
itd_init(struct ehci_hcd *ehci, struct ehci_iso_stream *stream,
struct ehci_itd *itd)
{
- int i;
-
/* it's been recently zeroed */
itd->hw_next = EHCI_LIST_END(ehci);
itd->hw_bufp [0] = stream->buf0;
itd->hw_bufp [1] = stream->buf1;
itd->hw_bufp [2] = stream->buf2;

- for (i = 0; i < 8; i++)
- itd->index[i] = -1;
-
/* All other fields are filled when scheduling */
}

@@ -1458,9 +1453,6 @@ itd_patch(

// BUG_ON (pg == 6 && uf->cross);

- uframe &= 0x07;
- itd->index [uframe] = index;
-
itd->hw_transaction[uframe] = uf->transaction;
itd->hw_transaction[uframe] |= cpu_to_hc32(ehci, pg << 12);
itd->hw_bufp[pg] |= cpu_to_hc32(ehci, uf->bufp & ~(u32)0);
@@ -1519,6 +1511,7 @@ itd_link_urb (

/* fill iTDs uframe by uframe */
for (packet = 0, itd = NULL; packet < urb->number_of_packets; ) {
+ uframe = next_uframe & 0x07;
if (itd == NULL) {
/* ASSERT: we have all necessary itds */
// BUG_ON (list_empty (&iso_sched->td_list));
@@ -1531,9 +1524,10 @@ itd_link_urb (
itd->stream = iso_stream_get (stream);
itd->urb = usb_get_urb (urb);
itd_init (ehci, stream, itd);
+ itd->uframe_scan = uframe;
+ itd->urb_index = packet;
}

- uframe = next_uframe & 0x07;
frame = next_uframe >> 3;

itd->usecs [uframe] = stream->usecs;
@@ -1576,17 +1570,18 @@ itd_scan(struct ehci_hcd *ehci, struct ehci_itd *itd, unsigned uframe_after)
struct usb_iso_packet_descriptor *desc;
u32 t;
unsigned uframe;
- int urb_index = -1;
+ int urb_index;
struct ehci_iso_stream *stream = itd->stream;
unsigned complete = 0;

/* for each uframe with a packet */
- for (uframe = itd->uframe_scan; uframe < uframe_after; uframe++) {
+ urb_index = itd->urb_index;
+ uframe = itd->uframe_scan;
+ for (; uframe < uframe_after; uframe += stream->interval) {

- if (likely (itd->index[uframe] == -1))
+ if (!urb)
continue;

- urb_index = itd->index[uframe];
desc = &urb->iso_frame_desc [urb_index];

t = hc32_to_cpup(ehci, &itd->hw_transaction [uframe]);
@@ -1616,8 +1611,9 @@ itd_scan(struct ehci_hcd *ehci, struct ehci_itd *itd, unsigned uframe_after)
WARN_ON(1);
}

+ urb_index++;
/* handle completion now? */
- if (urb_index + 1 != urb->number_of_packets)
+ if (urb_index < urb->number_of_packets)
continue;

#if 1
@@ -1636,6 +1632,7 @@ itd_scan(struct ehci_hcd *ehci, struct ehci_itd *itd, unsigned uframe_after)

if (uframe < 8) {
itd->uframe_scan = uframe;
+ itd->urb_index = urb_index;
return complete;
}

diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
index dcd031a..135dfae 100644
--- a/drivers/usb/host/ehci.h
+++ b/drivers/usb/host/ehci.h
@@ -581,12 +581,14 @@ struct ehci_itd {
struct urb *urb;
struct ehci_iso_stream *stream; /* endpoint's queue */
struct list_head itd_list; /* list of stream's itds */
+
+ /* at which uframe to look at which index next in itd_scan() */
unsigned uframe_scan;
+ unsigned urb_index; /* in urb->iso_frame_desc[] */

/* any/all hw_transactions here may be used by that urb */
unsigned frame; /* where scheduled */
unsigned pg;
- unsigned index[8]; /* in urb->iso_frame_desc */
u8 usecs[8];
} __attribute__ ((aligned (32)));
--
1.5.3.3


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
linux-usb-***@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

[Karsten Wiese]

This patch lets some ISO URBs complete sooner, by scanning ITDs up
to the current microframe and then reporting completions immediately.

Previously some completions for high speed transfers wouldn't be reported
right away, because the code waited until a complete frame was scanned and
such a scan might not happen immediately. Soundcard drivers have used a
partial work around for this, transferring ISO urbs using entire frames.

itd_complete() is split into 3 functions:
itd_scan() updates the urb's iso_frame_desc and returns a bitfield
indicating if the ITD's frame has elapsed (uframe % 8 == 0) or the urb can
be completed.
itd_complete() completes the urb and sets itd->urb = NULL.
itd_recycle() puts the ITD on the freelist.
If itd->urb == NULL, it also updates ehci->periodic_sched and appropiately
calls disable_periodic(ehci).

scan_periodic() is changed for "case Q_TYPE_ITD:" to always call
itd_scan() and itd_complete() or itd_recycle() based on the value itd_scan()
returned.

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

diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c
index 80d99bc..2d4fda5 100644
--- a/drivers/usb/host/ehci-sched.c
+++ b/drivers/usb/host/ehci-sched.c
@@ -1563,25 +1563,29 @@ itd_link_urb (
return 0;
}

+/* mask for return values used by itd_scan() and sitd_scan() */
+#define XITD_UNLINK (1 << 0) /* needs unlink from hw schedule */
+#define URB_COMPLETE (1 << 1) /* schedule may have new entries */
+
#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
-) {
+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;
+ unsigned complete = 0;

/* for each uframe with a packet */
- for (uframe = 0; uframe < 8; uframe++) {
+ for (uframe = itd->uframe_scan; uframe < uframe_after; uframe++) {
+
if (likely (itd->index[uframe] == -1))
continue;
+
urb_index = itd->index[uframe];
desc = &urb->iso_frame_desc [urb_index];

@@ -1607,46 +1611,75 @@ itd_complete (
} else if (likely ((t & EHCI_ISOC_ACTIVE) == 0)) {
desc->status = 0;
desc->actual_length = EHCI_ITD_LENGTH (t);
+ } else {
+ /* SHOULD NOT HAPPEN!! */
+ WARN_ON(1);
}
+
+ /* handle completion now? */
+ if (urb_index + 1 != urb->number_of_packets)
+ continue;
+
+#if 1
+ /* ASSERT: it's really the last itd for this urb */
+ {
+ struct ehci_itd *tmp;
+
+ list_for_each_entry(tmp, &stream->td_list, itd_list)
+ BUG_ON(tmp != itd && tmp->urb == urb);
+ }
+#endif
+
+ urb = NULL;
+ complete = URB_COMPLETE;
}

- usb_put_urb (urb);
- itd->urb = NULL;
- itd->stream = NULL;
- list_move (&itd->itd_list, &stream->free_list);
- iso_stream_put (ehci, stream);
+ if (uframe < 8) {
+ itd->uframe_scan = uframe;
+ return complete;
+ }

- /* handle completion now? */
- if (likely ((urb_index + 1) != urb->number_of_packets))
- return 0;
+ return complete | XITD_UNLINK;
+}

- /* 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 */
+static void
+itd_complete(struct ehci_hcd *ehci, struct ehci_itd *itd)
+{
+ usb_put_urb(itd->urb);

- /* give urb back to the driver ... can be out-of-order */
- dev = urb->dev;
- ehci_urb_done(ehci, urb, 0);
- urb = NULL;
+ ehci_urb_done(ehci, itd->urb, 0);
+ itd->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--;
+ iso_stream_put(ehci, itd->stream);
+}

- if (unlikely (list_empty (&stream->td_list))) {
+static void
+itd_recycle(struct ehci_hcd *ehci, struct ehci_itd *itd)
+{
+ struct ehci_iso_stream *stream = itd->stream;
+
+ usb_put_urb(itd->urb);
+ if (unlikely(!itd->urb)) {
+ /* deferred schedule disable; completion often submits */
+ ehci->periodic_sched--;
+ if (unlikely(!ehci->periodic_sched))
+ disable_periodic(ehci);
+ }
+ itd->stream = NULL;
+ list_move(&itd->itd_list, &stream->free_list);
+ 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");
+ ehci_vdbg(ehci,
+ "deschedule devp %s ep%d%s-iso\n",
+ stream->udev->devpath,
+ stream->bEndpointAddress & 0x0f,
+ (stream->bEndpointAddress & USB_DIR_IN)
+ ? "in" : "out");
}
iso_stream_put (ehci, stream);
-
- return 1;
}

/*-------------------------------------------------------------------------*/
@@ -2148,7 +2181,6 @@ restart:
modified = 0;

while (q.ptr != NULL) {
- unsigned uf;
union ehci_shadow temp;
int live;

@@ -2175,30 +2207,23 @@ 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;
+ type = Q_NEXT_TYPE(ehci, q.itd->hw_next);
+ modified = itd_scan(ehci, q.itd,
+ live ? uframes : 8);
+ if (modified & URB_COMPLETE)
+ itd_complete(ehci, q.itd);
+
+ if (modified & XITD_UNLINK) {
+ modified &= ~XITD_UNLINK;
+ *q_p = q.itd->itd_next;
+ *hw_p = q.itd->hw_next;
+ wmb();
+ itd_recycle(ehci, q.itd);
+ } else {
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;
}
- 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..dcd031a 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_scan;

/* any/all hw_transactions here may be used by that urb */
unsigned frame; /* where scheduled */
--
1.5.3.3


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
linux-usb-***@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

[Karsten Wiese]

usecs[] was used in periodic_usecs(), indexed by uframe.
For an ITD's unused uframes it was 0, else it contained the value of
itd->stream->usecs.
To check if an ITD's uframe is used, we can also test
itd->hw_transaction[uframe]: It contains != 0 no matter what endianess
itd->hw_transaction[]'s type is for used uframes.
Replace the access to usecs[uframe] in periodic_usecs() by an access to
itd->stream->usecs for used uframes. Else don't add to periodic_usecs()'s
result.
Saves 8bytes, makes struct ehci_itd use 160 Bytes on x86_64, if
"ehci-hcd: complete ISO URBs sooner" and
"ehci-hcd: slimm struct ehci_itd" were applied before ontop of 2.6.24-rc2.

Signed-off-by: Karsten Wiese <***@wemgehoertderstaat.de>
---
drivers/usb/host/ehci-sched.c | 4 ++--
drivers/usb/host/ehci.h | 1 -
2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c
index 38a7901..c41d348 100644
--- a/drivers/usb/host/ehci-sched.c
+++ b/drivers/usb/host/ehci-sched.c
@@ -119,7 +119,8 @@ periodic_usecs (struct ehci_hcd *ehci, unsigned frame, unsigned uframe)
q = &q->fstn->fstn_next;
break;
case Q_TYPE_ITD:
- usecs += q->itd->usecs [uframe];
+ if (q->itd->hw_transaction[uframe])
+ usecs += q->itd->stream->usecs;
hw_p = &q->itd->hw_next;
q = &q->itd->itd_next;
break;
@@ -1530,7 +1531,6 @@ itd_link_urb (

frame = next_uframe >> 3;

- itd->usecs [uframe] = stream->usecs;
itd_patch(ehci, itd, iso_sched, packet, uframe);

next_uframe += stream->interval;
diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
index 135dfae..2ec0bae 100644
--- a/drivers/usb/host/ehci.h
+++ b/drivers/usb/host/ehci.h
@@ -589,7 +589,6 @@ struct ehci_itd {
/* any/all hw_transactions here may be used by that urb */
unsigned frame; /* where scheduled */
unsigned pg;
- u8 usecs[8];
} __attribute__ ((aligned (32)));

/*-------------------------------------------------------------------------*/
--
1.5.3.3


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
linux-usb-***@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

[David Brownell]

Post by Karsten Wiese

Post by Karsten Wiese
Is it glitchy for SITDs because its wrong somehow or because its
inconvinient to differ from ITD mode?

Found a big glitch: put (s)idt on the freelist, then complete urb,
then update schedule based on a (s)itd which is reused by submition
during completion.

Both SITD and ITD? You had re-organized the two differently:

itd_scan() ... check "finished" microframes, maybe call done(),
then recycle, then return,

sitd_scan() ... check "finished" uframes, recycle, maybe done(),
then return

I've yet to check your very latest patch though, but that's what
was wrong in the last version I did look at in detail.

- Dave

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
linux-usb-***@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel