Changeset 27890 in vbox for trunk/src/VBox/Devices/Input

Timestamp:

Mar 31, 2010 12:57:26 PM (15 years ago)

Author:

vboxsync

Message:

USB keyboard: state machine refinements, locking, Caps Lock scancode bug

File:

• trunk/src/VBox/Devices/Input/UsbKbd.cpp (modified) (26 diffs)

trunk/src/VBox/Devices/Input/UsbKbd.cpp

@@ -128 +128 @@
     PPDMUSBINS          pUsbIns;
     /** Critical section protecting the device state. */
-    RTCRITSECT          CritSect;
+    RTCRITSECT          csLock;
 
     /** The current configuration.
@@ -162 +162 @@
     /** If no URB since last key press */
     bool                fNoUrbSinceLastPress;
+    /* If device has pending changes */
+    bool                fHasPendingChanges;
     /* Keys released since last URB */
     uint8_t             aReleasedKeys[6];
@@ -356 +358 @@
     0x34, 0x35, 0xe1, 0x31, 0x1d, 0x1b, 0x06, 0x19,
     0x05, 0x11, 0x10, 0x36, 0x37, 0x38, 0xe5, 0x55,
-    0xe2, 0x2c, 0x32, 0x3a, 0x3b, 0x3c, 0x3d, 0x3e,
+    0xe2, 0x2c, 0x39, 0x3a, 0x3b, 0x3c, 0x3d, 0x3e,
     0x3f, 0x40, 0x41, 0x42, 0x43, 0x53, 0x47, 0x5f,
     0x60, 0x61, 0x56, 0x5c, 0x5d, 0x5e, 0x57, 0x59,
@@ -438 +440 @@
 *******************************************************************************/
 
+/* Everything happens in R3 */
+/**
+ * Lock device state mutex.
+ */
+DECLINLINE(int) usbKbdLock(PUSBHID pThis)
+{
+    return RTCritSectEnter(&pThis->csLock);
+}
+
+/**
+ * Unlock device state mutex.
+ */
+DECLINLINE(void) usbKbdUnlock(PUSBHID pThis)
+{
+    RTCritSectLeave(&pThis->csLock);
+}
+
 /**
  * Initializes an URB queue.
@@ -448 +467 @@
     pQueue->ppTail = &pQueue->pHead;
 }
-
-
 
 /**
@@ -608 +625 @@
     memset(&pThis->Report, 0, sizeof(pThis->Report));
     pThis->fNoUrbSinceLastPress = false;
-    memset(&pThis->aReleasedKeys, 0, sizeof(pThis->aReleasedKeys));
+    pThis->fHasPendingChanges = false;
+    memset(&pThis->aReleasedKeys[0], 0, sizeof(pThis->aReleasedKeys));
 
     for (unsigned i = 0; i < RT_ELEMENTS(pThis->aEps); i++)
@@ -634 +652 @@
 {
     unsigned i;
-    
+
     for (i = 0; i < RT_ELEMENTS(pThis->Report.aKeys); ++i)
     {
         if (pThis->Report.aKeys[i] == u8HidCode)
         {
+            /* Remove key down. */
             pThis->Report.aKeys[i] = 0;
-            break;                              /* Remove key down. */
         }
     }
 
+#if 0
     if (i == RT_ELEMENTS(pThis->Report.aKeys))
     {
-        //            AssertMsgFailed(("Key is up but was never down!?"));
-    }
-}
+        Log(("Key 0x%x is up but was never down!?", u8HidCode));
+    }
+#endif
+}
+
+static void usbHidCommitReportReleased(PUSBHID pThis)
+{
+    unsigned i;
+    for (i=0; i < RT_ELEMENTS(pThis->aReleasedKeys); ++i)
+    {
+        if (pThis->aReleasedKeys[i] != 0)
+        {
+            usbHidUpdateReportReleased(pThis, pThis->aReleasedKeys[i]);
+            pThis->aReleasedKeys[i] = 0;
+        }
+    }
+}
+
+#ifdef DEBUG
+#define HEX_DIGIT(x) (((x) < 0xa) ? ((x) + '0') : ((x) - 0xa + 'a'))
+static void usbHidComputePressed(PUSBHIDK_REPORT pReport, char* pszBuf, unsigned uBufLen)
+{
+    unsigned i, uBufPos = 0;
+    for (i=0; i < RT_ELEMENTS(pReport->aKeys); ++i)
+    {
+        uint8_t uCode = pReport->aKeys[i];
+        if (uCode != 0)
+        {
+            if (uBufPos + 4 >= uBufLen)
+                break;
+            pszBuf[uBufPos++] = HEX_DIGIT(uCode >> 4);
+            pszBuf[uBufPos++] = HEX_DIGIT(uCode & 0xf);
+            pszBuf[uBufPos++] = ' ';
+        }
+    }
+    pszBuf[uBufPos++] = '\0';
+}
+#undef HEX_DIGIT
+#endif
 
 /**
@@ -662 +717 @@
         unsigned        i;
 
+#ifdef DEBUG
+        char            szActiveBuf[128];
+        usbHidComputePressed(pReport, szActiveBuf, sizeof szActiveBuf);
+        Log2(("Sending report with pressed keys: %s\n", szActiveBuf));
+#endif
         cbCopy = sizeof(*pReport);
         memcpy(&pUrb->abData[0], pReport, cbCopy);
         pThis->fNoUrbSinceLastPress = false;
-        for (i=0; i < RT_ELEMENTS(pThis->aReleasedKeys); ++i)
-        {
-            if (pThis->aReleasedKeys[i] != 0)
-            {
-                usbHidUpdateReportReleased(pThis, pThis->aReleasedKeys[i]);
-                pThis->aReleasedKeys[i] = 0;
-            }
-        }
+        pThis->fHasPendingChanges = false;
+        usbHidCommitReportReleased(pThis);
+#ifdef DEBUG
+        usbHidComputePressed(pReport, szActiveBuf, sizeof szActiveBuf);
+        Log2(("New state: %s\n", szActiveBuf));
+#endif
 //        LogRel(("Sent report: %x : %x %x, size %d\n", pReport->ShiftState, pReport->aKeys[0], pReport->aKeys[1], cbCopy));
         return usbHidCompleteOk(pThis, pUrb, cbCopy);
@@ -678 +736 @@
     else
     {
-        Log(("No available URB for USB kbd\n"));
+        Log2(("No available URB for USB kbd\n"));
+        pThis->fHasPendingChanges = true;
     }
     return VINF_EOF;
@@ -705 +764 @@
     PUSBHID pThis = RT_FROM_MEMBER(pInterface, USBHID, Lun0.IPort);
     PUSBHIDK_REPORT pReport = &pThis->Report;
-    uint32_t    u32Usage;
+    uint32_t    u32Usage = 0;
     uint8_t     u8HidCode;
     int         fKeyDown;
     unsigned    i;
 
-//    int rc = PDMCritSectEnter(&pThis->CritSect, VERR_SEM_BUSY);
-//    AssertReleaseRC(rc);
+    usbKbdLock(pThis);
 
     pThis->XlatState = ScancodeToHidUsage(pThis->XlatState, u8KeyCode, &u32Usage);
@@ -721 +779 @@
         u8HidCode = u32Usage & 0xFF;
 
+        Log(("key %s: 0x%x->0x%x\n",
+             fKeyDown ? "down" : "up", u8KeyCode, u8HidCode));
+
         if (fKeyDown)
         {
+            bool fAlready = false;
             for (i = 0; i < RT_ELEMENTS(pReport->aKeys); ++i)
             {
                 if (pReport->aKeys[i] == u8HidCode)
-                    break;                              /* Skip repeat events. */
-                if (pReport->aKeys[i] == 0)
                 {
-                    pReport->aKeys[i] = u8HidCode;      /* Report key down. */
-                    break;
+                    /* Skip repeat events. */
+                    fAlready = true;
+                    break;                              
                 }
             }
 
-            pThis->fNoUrbSinceLastPress = true;
-
-            if (i == RT_ELEMENTS(pReport->aKeys))
+            if (!fAlready)
             {
-                /* We ran out of room. Report error. */
-                Log(("no more room in usbHidKeyboardPutEvent\n"));
-                // @todo!!
+                for (i = 0; i < RT_ELEMENTS(pReport->aKeys); ++i)
+                {
+                    if (pReport->aKeys[i] == 0)
+                    {
+                        pReport->aKeys[i] = u8HidCode;      /* Report key down. */
+                        break;
+                    }
+                }
+                
+                pThis->fNoUrbSinceLastPress = true;
+                
+                if (i == RT_ELEMENTS(pReport->aKeys))
+                {
+                    /* We ran out of room. Report error. */
+                    Log(("no more room in usbHidKeyboardPutEvent\n"));
+                    // @todo!!
+                }
             }
         }
         else
         {
-            /* We have to avoid coalescing key presses and releases, so put all releases somewhere else */
+            /*
+             * We have to avoid coalescing key presses and releases,
+             * so put all releases somewhere else if press wasn't seen
+             * by the guest.
+             */
             if (pThis->fNoUrbSinceLastPress)
-            {                
+            {
                 for (i = 0; i < RT_ELEMENTS(pThis->aReleasedKeys); ++i)
                 {
+                    if (pThis->aReleasedKeys[i] == u8HidCode)
+                        break;
+
                     if (pThis->aReleasedKeys[i] == 0)
                     {
@@ -765 +845 @@
     }
 
-//    PDMCritSectLeave(&pThis->CritSect);
+    usbKbdUnlock(pThis);
+
     return VINF_SUCCESS;
 }
@@ -775 +856 @@
 {
     PUSBHID pThis = PDMINS_2_DATA(pUsbIns, PUSBHID);
-    LogFlow(("usbHidUrbReap/#%u: cMillies=%u\n", pUsbIns->iInstance, cMillies));
-
-    RTCritSectEnter(&pThis->CritSect);
+    //LogFlow(("usbHidUrbReap/#%u: cMillies=%u\n", pUsbIns->iInstance, cMillies));
+
+    usbKbdLock(pThis);
 
     PVUSBURB pUrb = usbHidQueueRemoveHead(&pThis->DoneQueue);
@@ -784 +865 @@
         /* Wait */
         pThis->fHaveDoneQueueWaiter = true;
-        RTCritSectLeave(&pThis->CritSect);
+        usbKbdUnlock(pThis);
 
         RTSemEventWait(pThis->hEvtDoneQueue, cMillies);
 
-        RTCritSectEnter(&pThis->CritSect);
+        usbKbdLock(pThis);
         pThis->fHaveDoneQueueWaiter = false;
 
@@ -794 +875 @@
     }
 
-    RTCritSectLeave(&pThis->CritSect);
+    usbKbdUnlock(pThis);
 
     if (pUrb)
@@ -809 +890 @@
     PUSBHID pThis = PDMINS_2_DATA(pUsbIns, PUSBHID);
     LogFlow(("usbHidUrbCancel/#%u: pUrb=%p:%s\n", pUsbIns->iInstance, pUrb, pUrb->pszDesc));
-    RTCritSectEnter(&pThis->CritSect);
+    usbKbdLock(pThis);
 
     /*
@@ -817 +898 @@
         usbHidLinkDone(pThis, pUrb);
 
-    RTCritSectLeave(&pThis->CritSect);
+    usbKbdUnlock(pThis);
     return VINF_SUCCESS;
 }
@@ -864 +945 @@
             usbHidQueueAddTail(&pThis->ToHostQueue, pUrb);
             /* If device was not set idle, sent the current report right away. */
-            if (pThis->bIdle != 0)
+            if (pThis->bIdle != 0 || pThis->fHasPendingChanges)
                 usbHidSendReport(pThis);
             LogFlow(("usbHidHandleIntrDevToHost: Sent report via %p:%s\n", pUrb, pUrb->pszDesc));
@@ -1063 +1144 @@
     PUSBHID pThis = PDMINS_2_DATA(pUsbIns, PUSBHID);
     LogFlow(("usbHidQueue/#%u: pUrb=%p:%s EndPt=%#x\n", pUsbIns->iInstance, pUrb, pUrb->pszDesc, pUrb->EndPt));
-    RTCritSectEnter(&pThis->CritSect);
+    usbKbdLock(pThis);
 
     /*
@@ -1087 +1168 @@
     }
 
-    RTCritSectLeave(&pThis->CritSect);
+    usbKbdUnlock(pThis);
     return rc;
 }
@@ -1102 +1183 @@
     if ((uEndpoint & ~0x80) < RT_ELEMENTS(pThis->aEps))
     {
-        RTCritSectEnter(&pThis->CritSect);
+        usbKbdLock(pThis);
         pThis->aEps[(uEndpoint & ~0x80)].fHalted = false;
-        RTCritSectLeave(&pThis->CritSect);
+        usbKbdUnlock(pThis);
     }
 
@@ -1131 +1212 @@
     LogFlow(("usbHidUsbSetConfiguration/#%u: bConfigurationValue=%u\n", pUsbIns->iInstance, bConfigurationValue));
     Assert(bConfigurationValue == 1);
-    RTCritSectEnter(&pThis->CritSect);
+    usbKbdLock(pThis);
 
     /*
@@ -1140 +1221 @@
     pThis->bConfigurationValue = bConfigurationValue;
 
-    RTCritSectLeave(&pThis->CritSect);
+    usbKbdUnlock(pThis);
     return VINF_SUCCESS;
 }
@@ -1163 +1244 @@
     PUSBHID pThis = PDMINS_2_DATA(pUsbIns, PUSBHID);
     LogFlow(("usbHidUsbReset/#%u:\n", pUsbIns->iInstance));
-    RTCritSectEnter(&pThis->CritSect);
+    usbKbdLock(pThis);
 
     int rc = usbHidResetWorker(pThis, NULL, false /*fSetConfig*/);
 
-    RTCritSectLeave(&pThis->CritSect);
+    usbKbdUnlock(pThis);
     return rc;
 }
@@ -1180 +1261 @@
     LogFlow(("usbHidDestruct/#%u:\n", pUsbIns->iInstance));
 
-    if (RTCritSectIsInitialized(&pThis->CritSect))
-    {
-        RTCritSectEnter(&pThis->CritSect);
-        RTCritSectLeave(&pThis->CritSect);
-        RTCritSectDelete(&pThis->CritSect);
+    if (RTCritSectIsInitialized(&pThis->csLock))
+    {
+        /* Let whoever runs in this critical section complete */
+        usbKbdLock(pThis);
+        usbKbdUnlock(pThis);
+        RTCritSectDelete(&pThis->csLock);
     }
 
@@ -1213 +1295 @@
     usbHidQueueInit(&pThis->DoneQueue);
 
-    int rc = RTCritSectInit(&pThis->CritSect);
+    int rc = RTCritSectInit(&pThis->csLock);
     AssertRCReturn(rc, rc);