Hi, Laurent
For several reasons it's difficult to apply patch to 2.6.25. I had to modify
ehci-sched.c by hand. I think, it's mainly because of extra whitespaces in
patch. Clean version attached.
Regards, Evgeny Marchenko
> Hi everybody,
>
> Alan Stern was kind enough to write a fix a few weeks ago for a video
> streaming issue that some of you encountered.
>
> In order to get the fix included in 2.6.26, David Brownell would like to
> know if the Linux UVC driver still suffers from the problem when used with
> 2.6.25 *without* the patch.
>
> Could you please be kind enough to test 2.6.25 and report results ?
>
> Best regards,
>
> Laurent Pinchart
>
> ---------- Forwarded Message ----------
>
> Subject: Re: ehci-hcd issue with webcam / uvcvideo driver
> Date: Monday 21 April 2008
> From: Alan Stern <[EMAIL PROTECTED]>
> To: Laurent Pinchart <[EMAIL PROTECTED]>
>
> On Mon, 21 Apr 2008, Laurent Pinchart wrote:
> > > Below is an updated version of the patch. If you can, replace the
> > > earlier version with this one and give it some good stress testing.
> > > If everything works okay, I'll submit it for inclusion in the official
> > > kernel.
> >
> > I've received several reports from people who tested your patch with the
> > Linux UVC driver. They were all positive, the patch fixed their problem
> > and introduced no regression they noticed. Could the patch be scheduled
> > for 2.6.26 ?
>
> Dave had some questions, in particular, whether your testers still
> encounter problems when running 2.6.25 without the patch. It would be
> nice to know the answer, either way.
>
> (IMO the patch ought to be merged even if the problems are no longer
> present, because what it does is basically correct. There may be a
> detail or two I missed -- it always helps to have someone else review
> changes like this -- but on the whole the idea is right.)
>
> For reference, the current version of the patch is below.
>
> Alan Stern
>
>
> No Changelog comment yet... The ideas are:
>
> (1) Never return -EL2NSYNC if URB_ISO_ASAP is set -- there's
> no reason to do so. And ehci-hcd always assumes ISO_ASAP.
>
> (2) If the CPU is delayed by IRQ latency, advance to the next
> available schedule slot.
>
> (3) Return the correct status (-EXDEV) for Iso slots which were
> submitted too late to get used.
>
> (4) Don't check for liveness of Iso slots whose frames are already
> over.
>
>
> Index: usb-2.6/drivers/usb/host/ehci-sched.c
> ===================================================================
> --- usb-2.6.orig/drivers/usb/host/ehci-sched.c
> +++ usb-2.6/drivers/usb/host/ehci-sched.c
> @@ -1349,18 +1349,27 @@ iso_stream_schedule (
> /* when's the last uframe this urb could start? */
> max = now + mod;
>
> - /* typical case: reuse current schedule. stream is still active,
> - * and no gaps from host falling behind (irq delays etc)
> + /* 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.
> */
> if (likely (!list_empty (&stream->td_list))) {
> start = stream->next_uframe;
> if (start < now)
> start += mod;
> - if (likely ((start + sched->span) < max))
> - goto ready;
> - /* else fell behind; someday, try to reschedule */
> - status = -EL2NSYNC;
> - goto fail;
> +
> + /* Fell behind (by up to twice the slop amount)? */
> + if (start >= max - 2 * 8 * SCHEDULE_SLOP)
> + start += stream->interval * DIV_ROUND_UP(
> + max - start, stream->interval) - mod;
> +
> + /* Tried to schedule too far into the future? */
> + if (unlikely((start + sched->span) >= max)) {
> + status = -EFBIG;
> + goto fail;
> + }
> + goto ready;
> }
>
> /* need to schedule; when's the next (u)frame we could start?
> @@ -1613,6 +1622,9 @@ itd_complete (
> } else if (likely ((t & EHCI_ISOC_ACTIVE) == 0)) {
> desc->status = 0;
> desc->actual_length = EHCI_ITD_LENGTH (t);
> + } else {
> + /* URB was too late */
> + desc->status = -EXDEV;
> }
> }
>
> @@ -2095,7 +2107,7 @@ done:
> static void
> scan_periodic (struct ehci_hcd *ehci)
> {
> - unsigned frame, clock, now_uframe, mod;
> + unsigned now_uframe, frame, clock, clock_frame, mod;
> unsigned modified;
>
> mod = ehci->periodic_size << 3;
> @@ -2111,6 +2123,7 @@ scan_periodic (struct ehci_hcd *ehci)
> else
> clock = now_uframe + mod - 1;
> clock %= mod;
> + clock_frame = clock >> 3;
>
> for (;;) {
> union ehci_shadow q, *q_p;
> @@ -2157,22 +2170,26 @@ restart:
> case Q_TYPE_ITD:
> /* If this ITD is still active, leave it for
> * later processing ... check the next entry.
> + * No need to check for activity unless the
> + * frame is current.
> */
> - rmb ();
> - for (uf = 0; uf < 8 && live; uf++) {
> - if (0 == (q.itd->hw_transaction [uf]
> - & ITD_ACTIVE(ehci)))
> - continue;
> - incomplete = true;
> - q_p = &q.itd->itd_next;
> - hw_p = &q.itd->hw_next;
> - type = Q_NEXT_TYPE(ehci,
> + if (frame == clock_frame && live) {
> + rmb();
> + for (uf = 0; uf < 8; uf++) {
> + if (q.itd->hw_transaction[uf] &
> + ITD_ACTIVE(ehci))
> + break;
> + }
> + if (uf < 8) {
> + incomplete = true;
> + 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;
> + q = *q_p;
> + break;
> + }
> }
> - if (uf < 8 && live)
> - break;
>
> /* Take finished ITDs out of the schedule
> * and process them: recycle, maybe report
> @@ -2189,9 +2206,12 @@ restart:
> case Q_TYPE_SITD:
> /* If this SITD is still active, leave it for
> * later processing ... check the next entry.
> + * No need to check for activity unless the
> + * frame is current.
> */
> - if ((q.sitd->hw_results & SITD_ACTIVE(ehci))
> - && live) {
> + if (frame == clock_frame && live &&
> + (q.sitd->hw_results &
> + SITD_ACTIVE(ehci))) {
> incomplete = true;
> q_p = &q.sitd->sitd_next;
> hw_p = &q.sitd->hw_next;
> @@ -2260,6 +2280,7 @@ restart:
>
> /* rescan the rest of this frame, then ... */
> clock = now;
> + clock_frame = clock >> 3;
> } else {
> now_uframe++;
> now_uframe %= mod;
>
>
> -------------------------------------------------------
> _______________________________________________
> Linux-uvc-devel mailing list
> [email protected]
> https://lists.berlios.de/mailman/listinfo/linux-uvc-devel
--- linux-2.6.25-clean/drivers/usb/host/ehci-sched.c 2008-04-22 13:54:31.000000000 +0400
+++ linux-2.6.25-uvc/drivers/usb/host/ehci-sched.c 2008-04-22 14:26:58.000000000 +0400
@@ -1354,18 +1354,27 @@
/* when's the last uframe this urb could start? */
max = now + mod;
- /* typical case: reuse current schedule. stream is still active,
- * and no gaps from host falling behind (irq delays etc)
+ /* 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.
*/
if (likely (!list_empty (&stream->td_list))) {
start = stream->next_uframe;
if (start < now)
start += mod;
- if (likely ((start + sched->span) < max))
- goto ready;
- /* else fell behind; someday, try to reschedule */
- status = -EL2NSYNC;
- goto fail;
+
+ /* Fell behind (by up to twice the slop amount)? */
+ if (start >= max - 2 * 8 * SCHEDULE_SLOP)
+ start += stream->interval * DIV_ROUND_UP(
+ max - start, stream->interval) - mod;
+
+ /* Tried to schedule too far into the future? */
+ if (unlikely((start + sched->span) >= max)) {
+ status = -EFBIG;
+ goto fail;
+ }
+ goto ready;
}
/* need to schedule; when's the next (u)frame we could start?
@@ -1618,6 +1627,9 @@
} else if (likely ((t & EHCI_ISOC_ACTIVE) == 0)) {
desc->status = 0;
desc->actual_length = EHCI_ITD_LENGTH (t);
+ } else {
+ /* URB was too late */
+ desc->status = -EXDEV;
}
}
@@ -2103,7 +2115,7 @@
static void
scan_periodic (struct ehci_hcd *ehci)
{
- unsigned frame, clock, now_uframe, mod;
+ unsigned now_uframe, frame, clock, clock_frame, mod;
unsigned modified;
mod = ehci->periodic_size << 3;
@@ -2119,6 +2131,7 @@
else
clock = now_uframe + mod - 1;
clock %= mod;
+ clock_frame = clock >> 3;
for (;;) {
union ehci_shadow q, *q_p;
@@ -2165,22 +2178,26 @@
case Q_TYPE_ITD:
/* If this ITD is still active, leave it for
* later processing ... check the next entry.
+ * No need to check for activity unless the
+ * frame is current.
*/
- rmb ();
- for (uf = 0; uf < 8 && live; uf++) {
- if (0 == (q.itd->hw_transaction [uf]
- & ITD_ACTIVE(ehci)))
- continue;
- incomplete = true;
- 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 (frame == clock_frame && live) {
+ rmb();
+ for (uf = 0; uf < 8; uf++) {
+ if (q.itd->hw_transaction[uf] &
+ ITD_ACTIVE(ehci))
+ break;
+ }
+ if (uf < 8) {
+ incomplete = true;
+ 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 && live)
- break;
/* Take finished ITDs out of the schedule
* and process them: recycle, maybe report
@@ -2197,9 +2214,12 @@
case Q_TYPE_SITD:
/* If this SITD is still active, leave it for
* later processing ... check the next entry.
+ * No need to check for activity unless the
+ * frame is current.
*/
- if ((q.sitd->hw_results & SITD_ACTIVE(ehci))
- && live) {
+ if (frame == clock_frame && live &&
+ (q.sitd->hw_results &
+ SITD_ACTIVE(ehci))) {
incomplete = true;
q_p = &q.sitd->sitd_next;
hw_p = &q.sitd->hw_next;
@@ -2268,6 +2288,7 @@
/* rescan the rest of this frame, then ... */
clock = now;
+ clock_frame = clock >> 3;
} else {
now_uframe++;
now_uframe %= mod;
_______________________________________________
Linux-uvc-devel mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/linux-uvc-devel
