On Tue, 11 Sep 2012, Clemens Ladisch wrote:
> > The test patch should be ready for posting soon. Would you like to try
> > it out?
>
> Yes (if I find time).
Here it is. This is actually five separate patches combined into one.
It implements the new documented behavior in ehci-hcd and adds
debugging code for performing tests. It also changes the snd-usb-audio
driver by removing the URB_ISO_ASAP flag from the data URBs.
Here's how the testing code should be used. The patch creates a file
named "test" under /sys/kernel/debug/usb/ehci/X/ where X is the PCI
device name of the particular controller (for example, 0000:00:1d.7).
You write two integer values to the test file. The first tells
ehci-hcd how many isochronous completion events to ignore and the
second tells ehci-hcd how many microseconds to delay before then
scanning for isochronous completions.
For example, with my USB audio device, the snd-usb-audio driver uses a
queue of 2 iso data URBs each containing 3 packets (usually).
Therefore I test by playing an audio file and doing:
echo 2 1000 >test
while the audio is running. The driver ignores the completion events
for the two URBs in the queue when they occur, and then handles them
1000 us after the second completion. As a result, the next URB
submitted by the sound driver is one frame too late. The submission
succeeds, but I can see the delay in a usbmon trace as well as the fact
that when the URB completes, its first packet gets a status of -EXDEV
instead of 0.
If instead I do "echo 2 3000 >test" then the next URB is submitted 3
frames too late. Since the URB contains only 3 packets, all of its
slots have already expired. The submission fails and the audio stops
playing, although the ogg123 program continues running and the synch
URBs continue to be submitted.
If the URB_ISO_ASAP flag is retained in the sound driver then the
delayed URB submission does not fail. Instead there is a brief gap and
the audio continues, now out of synchronization. In theory this gap
could be made as large as you want, although in practice the udelay()
routine has an upper limit on the size of the argument it accepts.
Alan Stern
Index: usb-3.6/drivers/usb/host/ehci.h
===================================================================
--- usb-3.6.orig/drivers/usb/host/ehci.h
+++ usb-3.6/drivers/usb/host/ehci.h
@@ -112,6 +112,7 @@ struct ehci_hcd { /* one per controlle
__u32 hcs_params; /* cached register copy */
spinlock_t lock;
enum ehci_rh_state rh_state;
+ int test1, test2;
/* general schedule support */
bool scanning:1;
@@ -143,7 +144,7 @@ struct ehci_hcd { /* one per controlle
struct ehci_qh *intr_unlink_last;
unsigned intr_unlink_cycle;
unsigned now_frame; /* frame from HC hardware */
- unsigned next_frame; /* scan periodic, start here */
+ unsigned last_iso_frame; /* last frame scanned for iso */
unsigned intr_count; /* intr activity count */
unsigned isoc_count; /* isoc activity count */
unsigned periodic_count; /* periodic activity count */
@@ -193,7 +194,6 @@ struct ehci_hcd { /* one per controlle
unsigned has_amcc_usb23:1;
unsigned need_io_watchdog:1;
unsigned amd_pll_fix:1;
- unsigned fs_i_thresh:1; /* Intel iso scheduling */
unsigned use_dummy_qh:1; /* AMD Frame List table quirk*/
unsigned has_synopsys_hc_bug:1; /* Synopsys HC */
unsigned frame_index_bug:1; /* MosChip (AKA NetMos) */
Index: usb-3.6/drivers/usb/host/ehci-sched.c
===================================================================
--- usb-3.6.orig/drivers/usb/host/ehci-sched.c
+++ usb-3.6/drivers/usb/host/ehci-sched.c
@@ -1361,7 +1361,7 @@ sitd_slot_ok (
* given EHCI_TUNE_FLS and the slop). Or, write a smarter scheduler!
*/
-#define SCHEDULE_SLOP 80 /* microframes */
+#define SCHEDULING_DELAY 40 /* microframes */
static int
iso_stream_schedule (
@@ -1370,7 +1370,7 @@ iso_stream_schedule (
struct ehci_iso_stream *stream
)
{
- u32 now, next, start, period, span;
+ u32 now, base, next, start, period, span;
int status;
unsigned mod = ehci->periodic_size << 3;
struct ehci_iso_sched *sched = urb->hcpriv;
@@ -1382,62 +1382,75 @@ iso_stream_schedule (
span <<= 3;
}
- if (span > mod - SCHEDULE_SLOP) {
- ehci_dbg (ehci, "iso request %p too long\n", urb);
- status = -EFBIG;
- goto fail;
- }
-
now = ehci_read_frame_index(ehci) & (mod - 1);
/* Typical case: reuse current schedule, stream is still active.
* Hopefully there are no gaps from the host falling behind
- * (irq delays etc), but if there are we'll take the next
- * slot in the schedule, implicitly assuming URB_ISO_ASAP.
+ * (irq delays etc). If there are, the behavior depends on
+ * whether URB_ISO_ASAP is set.
*/
if (likely (!list_empty (&stream->td_list))) {
- u32 excess;
- /* For high speed devices, allow scheduling within the
- * isochronous scheduling threshold. For full speed devices
- * and Intel PCI-based controllers, don't (work around for
- * Intel ICH9 bug).
- */
- if (!stream->highspeed && ehci->fs_i_thresh)
- next = now + ehci->i_thresh;
+ /* Take the isochronous scheduling threshold into account */
+ if (ehci->i_thresh)
+ next = now + ehci->i_thresh; /* uframe cache */
else
- next = now;
+ next = (now + 2 + 7) & ~0x07; /* full frame cache */
- /* Fell behind (by up to twice the slop amount)?
- * We decide based on the time of the last currently-scheduled
- * slot, not the time of the next available slot.
+ /*
+ * Use ehci->last_iso_frame as the base. There can't be any
+ * TDs scheduled for earlier than that.
*/
- excess = (stream->next_uframe - period - next) & (mod - 1);
- if (excess >= mod - 2 * SCHEDULE_SLOP)
- start = next + excess - mod + period *
- DIV_ROUND_UP(mod - excess, period);
- else
- start = next + excess + period;
- if (start - now >= mod) {
- ehci_dbg(ehci, "request %p would overflow (%d+%d >= %d)\n",
- urb, start - now - period, period,
- mod);
- status = -EFBIG;
+ base = ehci->last_iso_frame << 3;
+ next = (next - base) & (mod - 1);
+ start = (stream->next_uframe - base) & (mod - 1);
+
+ /* Is the schedule already full? */
+ if (unlikely(start < period)) {
+ ehci_dbg(ehci, "iso sched full %p ("
+ "%u-%u < %u mod %u)\n",
+ urb, stream->next_uframe, base,
+ period, mod);
+ status = -ENOSPC;
goto fail;
}
+
+ /* Behind the scheduling threshold? */
+ if (unlikely(start < next)) {
+
+ /* USB_ISO_ASAP: Use the first available slot */
+ if (urb->transfer_flags & URB_ISO_ASAP) {
+ start += period * DIV_ROUND_UP(next - start,
+ period);
+ }
+
+ /*
+ * Not ASAP: Use the next slot in the stream. If
+ * the entire URB falls before the threshold, fail.
+ */
+ else if (start + span - period < next) {
+ ehci_dbg(ehci, "iso urb late %p (%u+%u < %u)\n",
+ urb, start + base,
+ span - period, next + base);
+ status = -EXDEV;
+ goto fail;
+ }
+ }
+
+ start += base;
}
/* need to schedule; when's the next (u)frame we could start?
* this is bigger than ehci->i_thresh allows; scheduling itself
- * isn't free, the slop should handle reasonably slow cpus. it
+ * isn't free, the delay should handle reasonably slow cpus. it
* can also help high bandwidth if the dma and irq loads don't
* jump until after the queue is primed.
*/
else {
int done = 0;
- start = SCHEDULE_SLOP + (now & ~0x07);
- /* NOTE: assumes URB_ISO_ASAP, to limit complexity/bugs */
+ base = now & ~0x07;
+ start = base + SCHEDULING_DELAY;
/* find a uframe slot with enough bandwidth.
* Early uframes are more precious because full-speed
@@ -1464,19 +1477,16 @@ iso_stream_schedule (
/* no room in the schedule */
if (!done) {
- ehci_dbg(ehci, "iso resched full %p (now %d max %d)\n",
- urb, now, now + mod);
+ ehci_dbg(ehci, "iso sched full %p", urb);
status = -ENOSPC;
goto fail;
}
}
/* Tried to schedule too far into the future? */
- if (unlikely(start - now + span - period
- >= mod - 2 * SCHEDULE_SLOP)) {
- ehci_dbg(ehci, "request %p would overflow (%d+%d >= %d)\n",
- urb, start - now, span - period,
- mod - 2 * SCHEDULE_SLOP);
+ if (unlikely(start - base + span - period >= mod)) {
+ ehci_dbg(ehci, "request %p would overflow (%u+%u >= %u)\n",
+ urb, start - base, span - period, mod);
status = -EFBIG;
goto fail;
}
@@ -1490,7 +1500,7 @@ iso_stream_schedule (
/* Make sure scan_isoc() sees these */
if (ehci->isoc_count == 0)
- ehci->next_frame = now >> 3;
+ ehci->last_iso_frame = now >> 3;
return 0;
fail:
@@ -1708,7 +1718,7 @@ static bool itd_complete(struct ehci_hcd
urb->actual_length += desc->actual_length;
} else {
/* URB was too late */
- desc->status = -EXDEV;
+ urb->error_count++;
}
}
@@ -2081,7 +2091,7 @@ static bool sitd_complete(struct ehci_hc
t = hc32_to_cpup(ehci, &sitd->hw_results);
/* report transfer status */
- if (t & SITD_ERRS) {
+ if (unlikely(t & SITD_ERRS)) {
urb->error_count++;
if (t & SITD_STS_DBE)
desc->status = usb_pipein (urb->pipe)
@@ -2091,6 +2101,9 @@ static bool sitd_complete(struct ehci_hc
desc->status = -EOVERFLOW;
else /* XACT, MMF, etc */
desc->status = -EPROTO;
+ } else if (unlikely(t & SITD_STS_ACTIVE)) {
+ /* URB was too late */
+ urb->error_count++;
} else {
desc->status = 0;
desc->actual_length = desc->length - SITD_LENGTH(t);
@@ -2210,6 +2223,18 @@ static void scan_isoc(struct ehci_hcd *e
unsigned fmask = ehci->periodic_size - 1;
bool modified, live;
+ if (ehci->test1 > 0) {
+ --ehci->test1;
+ ehci_dbg(ehci, "counter %d\n", ehci->test1);
+ if (ehci->test1 > 0)
+ return;
+ if (ehci->test2 > 0)
+ udelay(ehci->test2);
+ ehci_dbg(ehci, "after delay\n");
+ }
+
+ /* complete the unlinking of some qh [4.15.2.3] */
+
/*
* When running, scan from last scan point up to "now"
* else clean up by scanning everything that's left.
@@ -2220,16 +2245,16 @@ static void scan_isoc(struct ehci_hcd *e
now_frame = (uf >> 3) & fmask;
live = true;
} else {
- now_frame = (ehci->next_frame - 1) & fmask;
+ now_frame = (ehci->last_iso_frame - 1) & fmask;
live = false;
}
ehci->now_frame = now_frame;
- frame = ehci->next_frame;
for (;;) {
union ehci_shadow q, *q_p;
__hc32 type, *hw_p;
+ frame = ehci->last_iso_frame;
restart:
/* scan each element in frame's queue for completions */
q_p = &ehci->pshadow [frame];
@@ -2334,7 +2359,6 @@ restart:
/* Stop when we have reached the current frame */
if (frame == now_frame)
break;
- frame = (frame + 1) & fmask;
+ ehci->last_iso_frame = (frame + 1) & fmask;
}
- ehci->next_frame = now_frame;
}
Index: usb-3.6/drivers/usb/host/ehci-pci.c
===================================================================
--- usb-3.6.orig/drivers/usb/host/ehci-pci.c
+++ usb-3.6/drivers/usb/host/ehci-pci.c
@@ -103,7 +103,6 @@ static int ehci_pci_setup(struct usb_hcd
}
break;
case PCI_VENDOR_ID_INTEL:
- ehci->fs_i_thresh = 1;
if (pdev->device == PCI_DEVICE_ID_INTEL_CE4100_USB)
hcd->has_tt = 1;
break;
Index: usb-3.6/drivers/usb/host/ehci-hcd.c
===================================================================
--- usb-3.6.orig/drivers/usb/host/ehci-hcd.c
+++ usb-3.6/drivers/usb/host/ehci-hcd.c
@@ -503,7 +503,7 @@ static int ehci_init(struct usb_hcd *hcd
/* controllers may cache some of the periodic schedule ... */
if (HCC_ISOC_CACHE(hcc_params)) // full frame cache
- ehci->i_thresh = 2 + 8;
+ ehci->i_thresh = 0;
else // N microframes cached
ehci->i_thresh = 2 + HCC_ISOC_THRES(hcc_params);
Index: usb-3.6/drivers/usb/host/ehci-dbg.c
===================================================================
--- usb-3.6.orig/drivers/usb/host/ehci-dbg.c
+++ usb-3.6/drivers/usb/host/ehci-dbg.c
@@ -1053,6 +1053,42 @@ static ssize_t debug_lpm_write(struct fi
return count;
}
+static ssize_t debug_test_write(struct file *file, const char __user *user_buf,
+ size_t count, loff_t *ppos)
+{
+ struct ehci_hcd *ehci = file->private_data;
+ char buf[50];
+ size_t len;
+ int m, n;
+ struct timeval tval;
+ unsigned tstamp;
+
+ len = min(count, sizeof(buf) - 1);
+ if (copy_from_user(buf, user_buf, len))
+ return -EFAULT;
+ buf[len] = 0;
+
+ if (sscanf(buf, "%d %d", &m, &n) != 2)
+ return -EINVAL;
+
+ spin_lock_irq(&ehci->lock);
+ ehci->test1 = m;
+ ehci->test2 = n;
+ do_gettimeofday(&tval);
+ spin_unlock_irq(&ehci->lock);
+
+ tstamp = tval.tv_sec & 0xFFF;
+ tstamp = tstamp * 1000000 + tval.tv_usec;
+ ehci_dbg(ehci, "Test write at %u\n", tstamp);
+ return count;
+}
+
+static const struct file_operations debug_test_fops = {
+ .owner = THIS_MODULE,
+ .open = simple_open,
+ .write = debug_test_write,
+};
+
static inline void create_debug_files (struct ehci_hcd *ehci)
{
struct usb_bus *bus = &ehci_to_hcd(ehci)->self;
@@ -1077,6 +1113,9 @@ static inline void create_debug_files (s
&debug_lpm_fops))
goto file_error;
+ if (!debugfs_create_file("test", 0200, ehci->debug_dir, ehci,
+ &debug_test_fops))
+ goto file_error;
return;
file_error:
Index: usb-3.6/sound/usb/endpoint.c
===================================================================
--- usb-3.6.orig/sound/usb/endpoint.c
+++ usb-3.6/sound/usb/endpoint.c
@@ -669,7 +669,7 @@ static int data_ep_set_params(struct snd
if (!u->urb->transfer_buffer)
goto out_of_memory;
u->urb->pipe = ep->pipe;
- u->urb->transfer_flags = URB_ISO_ASAP | URB_NO_TRANSFER_DMA_MAP;
+ u->urb->transfer_flags = URB_NO_TRANSFER_DMA_MAP;
u->urb->interval = 1 << ep->datainterval;
u->urb->context = u;
u->urb->complete = snd_complete_urb;
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
