Re: xhci patch 20150623

[takahiro hayashi <t.hash425%gmail.com@localhost>]

On 2015/06/24 07:17, takahiro hayashi wrote:
> On 2015/06/23 17:01, takahiro hayashi wrote:
> > Hi,
> >
> > On 2015/06/23 16:12, Nick Hudson wrote:
> > > On 06/23/15 07:43, takahiro hayashi wrote:
> [snip]
> > > > + USB keyboard interaction is laggy and annoying.
> > >
> > > Any idea why keyboard is laggy?
> >
> > Currently I'm not sure.
>
> Something is wrong with interval value modification of
> interrupt endpoint in xhci_configure_endpoint().
> Maybe it does not satisfy the requirement in xHCI 6.2.3.6.

Attached patch should fix the problem.


--
t-hash

--- src/sys/dev/usb/xhci.c.orig	2015-06-14 20:29:13.000000000 +0900
+++ src/sys/dev/usb/xhci.c	2015-06-25 12:06:29.000000000 +0900
@@ -1251,15 +1251,36 @@ xhci_configure_endpoint(struct usbd_pipe
 		DPRINTFN(4, "setting HS MaxBurst %u", maxb, 0, 0, 0);
 	}
 
+	if (pipe->up_interval != USBD_DEFAULT_INTERVAL)
+		ival = pipe->up_interval;
+
 	switch (xfertype) {
 	case UE_INTERRUPT:
-		/* 6.2.3.6  */
+		/* xHCI 6.2.3.6 Table 65, USB 2.0 9.6.6 */
 		if (speed == USB_SPEED_LOW || speed == USB_SPEED_FULL) {
-			ival = ival > 10 ? 10 : ival;
-			ival = ival < 3 ? 3 : ival;
+			int i;
+
+			/*
+			 * round ival down to "the nearest base 2 multiple of
+			 * bInterval * 8".
+			 * bInterval is at most 255 as its type is uByte.
+			 * 255(ms) = 2040(x 125us) < 2^11, so start with 11.
+			 */
+			for (i = 11; i > 0; i--) {
+#if 0
+				DPRINTFN(4, "ival*8 %u vs 2^%d(%d)",
+				    ival*8, i, 1<<i, 0);
+#endif
+				if ((ival * 8) >= (1 << i))
+					break;
+			}
+			ival = i;
 		} else {
-			ival = ival > 15 ? 15 : ival;
+			/* Interval = bInterval-1 for SS/HS */
+			ival--;
 		}
+		DPRINTFN(4, "ival %u", ival, 0, 0, 0);
+
 		if (USB_IS_SS(speed)) {
 			if (maxb > 0)
 				mps = 1024;
@@ -1282,12 +1303,11 @@ xhci_configure_endpoint(struct usbd_pipe
 		break;
 #ifdef notyet
 	case UE_ISOCHRONOUS:
-		if (speed == USB_SPEED_FULL) {
-			ival = ival > 18 ? 18 : ival;
-			ival = ival < 3 ? 3 : ival;
-		} else {
-			ival = ival > 15 ? 15 : ival;
-		}
+		if (speed == USB_SPEED_FULL)
+			ival += 3; /* 1ms -> 125us */
+		ival--;
+		DPRINTFN(4, "ival %u", ival, 0, 0, 0);
+
 		if (USB_IS_SS(speed)) {
 			mps = 1024;
 		} else {