Changeset 4ed803f1 in mainline

Timestamp:

2018-01-11T17:35:02Z (6 years ago)

Author:

Salmelu <salmelu@…>

Branches:

lfn, master, serial, ticket/834-toolchain-update, topic/msim-upgrade, topic/simplify-dev-export

Children:

69a93d02

Parents:

929599a8

git-author:

Salmelu <salmelu@…> (2018-01-11 17:29:27)

git-committer:

Salmelu <salmelu@…> (2018-01-11 17:35:02)

Message:

xhci: Rewritten isoch event handling

Traversing loop looking for finished xhci transfer is now only 1 loop traversal in average case.
Added reset timer which resets the endpoint after there is no action for a given period.

Location:

uspace/drv/bus/usb/xhci

Files:

• isoch.c (modified) (12 diffs)
• isoch.h (modified) (1 diff)

uspace/drv/bus/usb/xhci/isoch.c

@@ -84 +84 @@
 }
 
+static void isoch_reset_no_timer(xhci_endpoint_t *ep)
+{
+        xhci_isoch_t * const isoch = ep->isoch;
+        assert(fibril_mutex_is_locked(&isoch->guard));
+        /*
+         * As we cannot clear timer when we are triggered by it,
+         * we have to avoid doing it in common method.
+         */
+        fibril_timer_clear_locked(isoch->reset_timer);
+        isoch_reset(ep);
+}
+
+static void isoch_reset_timer(void *ep) {
+        xhci_isoch_t * const isoch = xhci_endpoint_get(ep)->isoch;
+        fibril_mutex_lock(&isoch->guard);
+        isoch_reset(ep);
+        fibril_mutex_unlock(&isoch->guard);
+}
+
+/*
+ * Fast transfers could trigger the reset timer before the data is processed,
+ * leading into false reset.
+ */
+#define RESET_TIMER_DELAY 100000
+static void timer_schedule_reset(xhci_endpoint_t *ep) {
+        xhci_isoch_t * const isoch = ep->isoch;
+        const suseconds_t delay = isoch->buffer_count * ep->interval * 125 + RESET_TIMER_DELAY;
+
+        fibril_timer_clear_locked(isoch->reset_timer);
+        fibril_timer_set_locked(isoch->reset_timer, delay,
+                isoch_reset_timer, ep);
+}
+
 void isoch_fini(xhci_endpoint_t *ep)
 {
@@ -92 +125 @@
                 fibril_timer_clear(isoch->feeding_timer);
                 fibril_timer_destroy(isoch->feeding_timer);
+                fibril_timer_clear(isoch->reset_timer);
+                fibril_timer_destroy(isoch->reset_timer);
         }
 
@@ -109 +144 @@
 
         isoch->feeding_timer = fibril_timer_create(&isoch->guard);
+        isoch->reset_timer = fibril_timer_create(&isoch->guard);
         if (!isoch->feeding_timer)
                 return ENOMEM;
@@ -115 +151 @@
         if(!isoch->transfers)
                 goto err;
- 
+
         for (size_t i = 0; i < isoch->buffer_count; ++i) {
                 xhci_isoch_transfer_t *transfer = &isoch->transfers[i];
@@ -124 +160 @@
 
         fibril_mutex_lock(&isoch->guard);
-        isoch_reset(ep);
+        isoch_reset_no_timer(ep);
         fibril_mutex_unlock(&isoch->guard);
 
@@ -226 +262 @@
          * to ensure correct scheduling of transfers, that are
          * buffer_count * interval away from now.
-         * Maximum interval is 8 seconds, which means we need a size of 
+         * Maximum interval is 8 seconds, which means we need a size of
          * 16 seconds. The size of MFIINDEX is 2 seconds only.
          *
@@ -301 +337 @@
                 }
         }
+
 out:
-
         if (fed) {
                 const uint8_t slot_id = xhci_device_get(ep->base.device)->slot_id;
                 const uint8_t target = xhci_endpoint_index(ep) + 1; /* EP Doorbells start at 1 */
                 hc_ring_doorbell(hc, slot_id, target);
+                /* The ring may be dead. If no event happens until the delay, reset the endpoint. */
+                timer_schedule_reset(ep);
         }
 
@@ -394 +432 @@
                 const uint8_t target = xhci_endpoint_index(ep) + 1; /* EP Doorbells start at 1 */
                 hc_ring_doorbell(hc, slot_id, target);
+                /* The ring may be dead. If no event happens until the delay, reset the endpoint. */
+                timer_schedule_reset(ep);
         }
 }
@@ -541 +581 @@
                         /* For IN, the buffer has overfilled, we empty the buffers and readd TRBs */
                         usb_log_warning("Ring over/underrun.");
-                        isoch_reset(ep);
+                        isoch_reset_no_timer(ep);
                         fibril_condvar_broadcast(&ep->isoch->avail);
                         fibril_mutex_unlock(&ep->isoch->guard);
@@ -555 +595 @@
         }
 
-        bool found_mine = false;
-        bool found_incomplete = false;
-
         /*
          * The order of delivering events is not necessarily the one we would
-         * expect. It is safer to walk the list of our 4 transfers and check
+         * expect. It is safer to walk the list of our transfers and check
          * which one it is.
+         * To minimize the amount of transfers checked, we start at dequeue pointer
+         * and exit the loop as soon as the transfer is found.
          */
-        for (size_t i = 0; i < isoch->buffer_count; ++i) {
-                xhci_isoch_transfer_t * const it = &isoch->transfers[i];
-
-                switch (it->state) {
-                case ISOCH_FILLED:
-                        found_incomplete = true;
-                        break;
-
-                case ISOCH_FED:
-                        if (it->interrupt_trb_phys != trb->parameter) {
-                                found_incomplete = true;
-                                break;
-                        }
-
+        bool found_mine = false;
+        for (size_t i = 0, di = isoch->dequeue; i < isoch->buffer_count; ++i, ++di) {
+                /* Wrap it back to 0, don't use modulo every loop traversal */
+                if (di == isoch->buffer_count) {
+                        di = 0;
+                }
+
+                xhci_isoch_transfer_t * const it = &isoch->transfers[di];
+
+                if (it->state == ISOCH_FED && it->interrupt_trb_phys == trb->parameter) {
                         usb_log_debug2("[isoch] buffer %zu completed", it - isoch->transfers);
                         it->state = ISOCH_COMPLETE;
@@ -583 +618 @@
                         found_mine = true;
                         break;
-                default:
-                        break;
                 }
         }
@@ -595 +628 @@
          * It may happen that the driver already stopped reading (writing),
          * and our buffers are filled (empty). As QEMU (and possibly others)
-         * does not send RING_UNDERRUN (OVERRUN) event, detect it here.
+         * does not send RING_UNDERRUN (OVERRUN) event, we set a timer to
+         * reset it after the buffers should have been consumed. If there
+         * is no issue, the timer will get restarted often enough.
          */
-        if (!found_incomplete) {
-                usb_log_warning("[isoch] Endpoint" XHCI_EP_FMT ": Detected "
-                    "isochronous ring %s.", XHCI_EP_ARGS(*ep),
-                    (ep->base.direction == USB_DIRECTION_IN) ? "underrun" : "overrun");
-                isoch_reset(ep);
-        }
+        timer_schedule_reset(ep);
 
         fibril_condvar_broadcast(&ep->isoch->avail);

uspace/drv/bus/usb/xhci/isoch.h

@@ -81 +81 @@
         fibril_timer_t *feeding_timer;
 
+        /** Resets endpoint if there is no traffic. */
+        fibril_timer_t *reset_timer;
+
         /** The maximum size of an isochronous transfer and therefore the size of buffers */
         size_t max_size;