Date: Wed, 11 Aug 2010 18:26:36 GMT From: Hans Petter Selasky <hselasky@skunkworks.freebsd.org> To: Perforce Change Reviews <perforce@FreeBSD.org> Subject: PERFORCE change 182251 for review Message-ID: <201008111826.o7BIQa1d084798@skunkworks.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://p4web.freebsd.org/@@182251?ac=10 Change 182251 by hselasky@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 */
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201008111826.o7BIQa1d084798>