PERFORCE change 182251 for review

[Hans Petter Selasky]

http://p4web.freebsd.org/@@182251?ac=10

Change 182251 by hselasky at hselasky_laptop001 on 2010/08/11 16:20:07

	USB controller (EHCI):
		- revert most of r197682 (EHCI Hardware BUG workaround).
		Implement proper solution which is to not use the TERMINATE
		pointer, but rather link to a halted TD. The initial fix was
		due to a misunderstanding about how the EHCI hardware works.
		Thanks to Alan Stern for clearing this up during a recent
		e-mail discussion.
		- this patch can increase mass storage read performance
		significantly.

Affected files ...

.. //depot/projects/usb/src/sys/dev/usb/controller/ehci.c#58 edit
.. //depot/projects/usb/src/sys/dev/usb/controller/ehci.h#18 edit

Differences ...

==== //depot/projects/usb/src/sys/dev/usb/controller/ehci.c#58 (text+ko) ====

@@ -145,7 +145,6 @@
 	uint8_t	auto_data_toggle;
 	uint8_t	setup_alt_next;
 	uint8_t	last_frame;
-	uint8_t can_use_next;
 };
 
 void
@@ -157,6 +156,9 @@
 	cb(bus, &sc->sc_hw.pframes_pc, &sc->sc_hw.pframes_pg,
 	    sizeof(uint32_t) * EHCI_FRAMELIST_COUNT, EHCI_FRAMELIST_ALIGN);
 
+	cb(bus, &sc->sc_hw.terminate_pc, &sc->sc_hw.terminate_pg,
+	    sizeof(struct ehci_qh_sub), EHCI_QH_ALIGN);
+
 	cb(bus, &sc->sc_hw.async_start_pc, &sc->sc_hw.async_start_pg,
 	    sizeof(ehci_qh_t), EHCI_QH_ALIGN);
 
@@ -310,6 +312,24 @@
 
 	sc->sc_eintrs = EHCI_NORMAL_INTRS;
 
+	if (1) {
+		struct ehci_qh_sub *qh;
+
+		usbd_get_page(&sc->sc_hw.terminate_pc, 0, &buf_res);
+
+		qh = buf_res.buffer;
+
+		sc->sc_terminate_self = htohc32(sc, buf_res.physaddr);
+
+		/* init terminate TD */
+		qh->qtd_next =
+		    htohc32(sc, EHCI_LINK_TERMINATE);
+		qh->qtd_altnext =
+		    htohc32(sc, EHCI_LINK_TERMINATE);
+		qh->qtd_status =
+		    htohc32(sc, EHCI_QTD_HALTED);
+	}
+
 	for (i = 0; i < EHCI_VIRTUAL_FRAMELIST_COUNT; i++) {
 		ehci_qh_t *qh;
 
@@ -1416,15 +1436,7 @@
 			 */
 			if (status & EHCI_QTD_ACTIVE) {
 				/* update cache */
-				if (xfer->td_transfer_cache != td) {
-					xfer->td_transfer_cache = td;
-					if (qh->qh_qtd.qtd_next & 
-					    htohc32(sc, EHCI_LINK_TERMINATE)) {
-						/* XXX - manually advance to next frame */
-						qh->qh_qtd.qtd_next = td->qtd_self;
-						usb_pc_cpu_flush(td->page_cache);
-					}
-				}
+				xfer->td_transfer_cache = td;
 				goto done;
 			}
 			/*
@@ -1634,10 +1646,12 @@
 	uint32_t average;
 	uint32_t len_old;
 	uint32_t terminate;
+	uint32_t qtd_altnext;
 	uint8_t shortpkt_old;
 	uint8_t precompute;
 
-	terminate = htohc32(temp->sc, EHCI_LINK_TERMINATE);
+	terminate = temp->sc->sc_terminate_self;
+	qtd_altnext = temp->sc->sc_terminate_self;
 	td_alt_next = NULL;
 	buf_offset = 0;
 	shortpkt_old = temp->shortpkt;
@@ -1771,23 +1785,11 @@
 			td->qtd_buffer_hi[x] = 0;
 		}
 
-		if (temp->can_use_next) {
-			if (td_next) {
-				/* link the current TD with the next one */
-				td->qtd_next = td_next->qtd_self;
-			}
-		} else {
-			/*
-			 * BUG WARNING: The EHCI HW can use the
-			 * qtd_next field instead of qtd_altnext when
-			 * a short packet is received! We work this
-			 * around in software by not queueing more
-			 * than one job/TD at a time!
-			 */
-			td->qtd_next = terminate;
+		if (td_next) {
+			/* link the current TD with the next one */
+			td->qtd_next = td_next->qtd_self;
 		}
-
-		td->qtd_altnext = terminate;
+		td->qtd_altnext = qtd_altnext;
 		td->alt_next = td_alt_next;
 
 		usb_pc_cpu_flush(td->page_cache);
@@ -1799,9 +1801,15 @@
 		/* setup alt next pointer, if any */
 		if (temp->last_frame) {
 			td_alt_next = NULL;
+			qtd_altnext = terminate;
 		} else {
 			/* we use this field internally */
 			td_alt_next = td_next;
+			if (temp->setup_alt_next) {
+				qtd_altnext = td_next->qtd_self;
+			} else {
+				qtd_altnext = terminate;
+			}
 		}
 
 		/* restore */
@@ -1846,8 +1854,6 @@
 	temp.qtd_status = 0;
 	temp.last_frame = 0;
 	temp.setup_alt_next = xfer->flags_int.short_frames_ok;
-	temp.can_use_next = (xfer->flags_int.control_xfr ||
-	    (UE_GET_DIR(xfer->endpointno) == UE_DIR_OUT));
 
 	if (xfer->flags_int.control_xfr) {
 		if (xfer->endpoint->toggle_next) {

==== //depot/projects/usb/src/sys/dev/usb/controller/ehci.h#18 (text+ko) ====

@@ -285,12 +285,14 @@
 
 struct ehci_hw_softc {
 	struct usb_page_cache pframes_pc;
+	struct usb_page_cache terminate_pc;
 	struct usb_page_cache async_start_pc;
 	struct usb_page_cache intr_start_pc[EHCI_VIRTUAL_FRAMELIST_COUNT];
 	struct usb_page_cache isoc_hs_start_pc[EHCI_VIRTUAL_FRAMELIST_COUNT];
 	struct usb_page_cache isoc_fs_start_pc[EHCI_VIRTUAL_FRAMELIST_COUNT];
 
 	struct usb_page pframes_pg;
+	struct usb_page terminate_pg;
 	struct usb_page async_start_pg;
 	struct usb_page intr_start_pg[EHCI_VIRTUAL_FRAMELIST_COUNT];
 	struct usb_page isoc_hs_start_pg[EHCI_VIRTUAL_FRAMELIST_COUNT];
@@ -329,6 +331,7 @@
 	bus_space_tag_t sc_io_tag;
 	bus_space_handle_t sc_io_hdl;
 
+	uint32_t sc_terminate_self;	/* TD short packet termination pointer */
 	uint32_t sc_eintrs;
 	uint32_t sc_cmd;		/* shadow of cmd register during
 					 * suspend */