Changeset 53783 in vbox for trunk/src/VBox/Devices/Graphics/DevVGA-SVGA.cpp

Timestamp:

Jan 13, 2015 12:31:52 PM (10 years ago)

Author:

vboxsync

Message:

VMSVGA: FIFO stats; wait on the work-to-do-semaphore instead of sleeping when we're short on data in the FIFO (rare); Signal the semaphore before entering the FIFO loop so we don't delay work unecessarily when resumed.

File:

• trunk/src/VBox/Devices/Graphics/DevVGA-SVGA.cpp (modified) (18 diffs)

trunk/src/VBox/Devices/Graphics/DevVGA-SVGA.cpp

@@ -109 +109 @@
     STAMPROFILE             StatR3CmdDrawPrimitive;
     STAMPROFILE             StatR3CmdSurfaceDMA;
+
+    STAMCOUNTER             StatFifoCommands;
+    STAMCOUNTER             StatFifoErrors;
+    STAMCOUNTER             StatFifoUnkCmds;
+    STAMCOUNTER             StatFifoTodoTimeout;
+    STAMCOUNTER             StatFifoTodoWoken;
+    STAMPROFILE             StatFifoStalls;
+
 } VMSVGASTATE, *PVMSVGASTATE;
 
@@ -1041 +1049 @@
 
     case SVGA_REG_DEPTH:
-        /* @todo read-only?? */
+        /** @todo read-only?? */
         break;
 
@@ -1874 +1882 @@
  * @retval  NULL on FIFO error.
  *
- * @param   pThread         The calling PDM thread handle.
  * @param   cbPayloadReq    The number of bytes of payload requested.
  * @param   pFIFO           The FIFO.
@@ -1884 +1891 @@
  * @param   pcbAlreadyRead  How much payload we've already read into the bounce
  *                          buffer. (We will NEVER re-read anything.)
+ * @param   pThread         The calling PDM thread handle.
+ * @param   pSVGAState      Pointer to the ring-3 only SVGA state data. For
+ *                          statistics collection.
  */
-static void *vmsvgaFIFOGetCmdPayload(PPDMTHREAD pThread, uint32_t cbPayloadReq, uint32_t volatile *pFIFO,
-                                    uint32_t offCurrentCmd, uint32_t offFifoMin, uint32_t offFifoMax,
-                                    uint8_t *pbBounceBuf, uint32_t *pcbAlreadyRead)
+static void *vmsvgaFIFOGetCmdPayload(uint32_t cbPayloadReq, uint32_t volatile *pFIFO,
+                                     uint32_t offCurrentCmd, uint32_t offFifoMin, uint32_t offFifoMax,
+                                     uint8_t *pbBounceBuf, uint32_t *pcbAlreadyRead,
+                                     PPDMTHREAD pThread, PVGASTATE pThis, PVMSVGASTATE pSVGAState)
 {
     Assert(pbBounceBuf);
@@ -1913 +1924 @@
      */
     uint32_t const cbFifoCmd = offFifoMax - offFifoMin;
-    AssertMsgReturn(cbPayloadReq <= cbFifoCmd, ("cbPayloadReq=%#x cbFifoCmd=%#x\n", cbPayloadReq, cbFifoCmd), NULL);
+    AssertMsgReturnStmt(cbPayloadReq <= cbFifoCmd, ("cbPayloadReq=%#x cbFifoCmd=%#x\n", cbPayloadReq, cbFifoCmd),
+                        STAM_REL_COUNTER_INC(&pSVGAState->StatFifoErrors),
+                        NULL);
 
     /*
@@ -1933 +1946 @@
         else
         {
+            STAM_REL_COUNTER_INC(&pSVGAState->StatFifoErrors);
             LogRelMax(16, ("vmsvgaFIFOGetCmdPayload: Invalid offNextCmd=%#x (offFifoMin=%#x offFifoMax=%#x)\n",
                            offNextCmd, offFifoMin, offFifoMax));
@@ -1947 +1961 @@
         else
         {
+            STAM_REL_COUNTER_INC(&pSVGAState->StatFifoErrors);
             LogRelMax(16, ("vmsvgaFIFOGetCmdPayload: Invalid offNextCmd=%#x (offFifoMin=%#x offFifoMax=%#x)\n",
                            offNextCmd, offFifoMin, offFifoMax));
@@ -1958 +1973 @@
          * Insufficient, must wait for it to arrive.
          */
-        for (;;)
+        STAM_REL_PROFILE_START(&pSVGAState->StatFifoStalls, Stall);
+        for (uint32_t i = 0;; i++)
         {
             if (pThread->enmState != PDMTHREADSTATE_RUNNING)
+            {
+                STAM_REL_PROFILE_STOP(&pSVGAState->StatFifoStalls, Stall);
                 return (void *)(uintptr_t)1;
-            Log(("Guest still copying (%x vs %x) current %x next %x stop %x; sleep a bit\n",
-                 cbPayloadReq, cbAfter + cbBefore, offCurrentCmd, offNextCmd, pFIFO[SVGA_FIFO_STOP]));
-            RTThreadSleep(1);
-            /** @todo release counter.   */
+            }
+            Log(("Guest still copying (%x vs %x) current %x next %x stop %x loop %u; sleep a bit\n",
+                 cbPayloadReq, cbAfter + cbBefore, offCurrentCmd, offNextCmd, pFIFO[SVGA_FIFO_STOP], i));
+
+            SUPSemEventWaitNoResume(pThis->svga.pSupDrvSession, pThis->svga.FIFORequestSem, i < 16 ? 1 : 2);
 
             offNextCmd = pFIFO[SVGA_FIFO_NEXT_CMD];
@@ -1982 +2001 @@
                 break;
         }
+        STAM_REL_PROFILE_STOP(&pSVGAState->StatFifoStalls, Stall);
     }
 
@@ -2030 +2050 @@
 
     /*
+     * Signal the semaphore to make sure we don't wait for 250 after a
+     * suspend & resume scenario (see vmsvgaFIFOGetCmdPayload).
+     */
+    SUPSemEventSignal(pThis->svga.pSupDrvSession, pThis->svga.FIFORequestSem);
+
+    /*
      * Allocate a bounce buffer for command we get from the FIFO.
      * (All code must return via the end of the function to free this buffer.)
@@ -2053 +2079 @@
             if (pFIFO[SVGA_FIFO_NEXT_CMD] == pFIFO[SVGA_FIFO_STOP])
                 continue;
+            STAM_REL_COUNTER_INC(&pSVGAState->StatFifoTodoTimeout);
 
             Log(("vmsvgaFIFOLoop: timeout\n"));
         }
+        else if (pFIFO[SVGA_FIFO_NEXT_CMD] != pFIFO[SVGA_FIFO_STOP])
+            STAM_REL_COUNTER_INC(&pSVGAState->StatFifoTodoWoken);
+
         Log(("vmsvgaFIFOLoop: enabled=%d configured=%d busy=%d\n", pThis->svga.fEnabled, pThis->svga.fConfigured, pThis->svga.pFIFOR3[SVGA_FIFO_BUSY]));
         Log(("vmsvgaFIFOLoop: min  %x max  %x\n", pFIFO[SVGA_FIFO_MIN], pFIFO[SVGA_FIFO_MAX]));
@@ -2137 +2167 @@
             LogRelMax(8, ("vmsvgaFIFOLoop: Bad fifo: min=%#x stop=%#x max=%#x\n", offFifoMin, offCurrentCmd, offFifoMax));
             /** @todo Should we clear SVGA_FIFO_BUSY here like we do above? */
+            STAM_REL_COUNTER_INC(&pSVGAState->StatFifoErrors);
             continue;
         }
@@ -2156 +2187 @@
 # define VMSVGAFIFO_GET_CMD_BUFFER_BREAK(a_PtrVar, a_Type, a_cbPayloadReq) \
             if (1) { \
-                (a_PtrVar) = (a_Type *)vmsvgaFIFOGetCmdPayload(pThread, (a_cbPayloadReq), pFIFO, \
-                                                               offCurrentCmd, offFifoMin, offFifoMax, \
-                                                               pbBounceBuf, &cbPayload); \
+                (a_PtrVar) = (a_Type *)vmsvgaFIFOGetCmdPayload((a_cbPayloadReq), pFIFO, offCurrentCmd, offFifoMin, offFifoMax, \
+                                                               pbBounceBuf, &cbPayload, pThread, pThis, pSVGAState); \
                 if (RT_UNLIKELY((uintptr_t)(a_PtrVar) < 2)) { if ((uintptr_t)(a_PtrVar) == 1) continue; break; } \
             } else do {} while (0)
@@ -2423 +2453 @@
                 PGMR pGMR = &pSVGAState->aGMR[pCmd->gmrId];
                 AssertBreak(pCmd->offsetPages + pCmd->numPages <= pGMR->cMaxPages);
-                AssertBreak(!pCmd->offsetPages || pGMR->paDesc); /* @todo */
+                AssertBreak(!pCmd->offsetPages || pGMR->paDesc); /** @todo */
 
                 /* Save the old page descriptors as an array of page addresses (>> PAGE_SHIFT) */
@@ -2652 +2682 @@
             }
 
+            /** @todo SVGA_CMD_RECT_COPY  - see with ubuntu */
+
             default:
 # ifdef VBOX_WITH_VMSVGA3D
@@ -2995 +3027 @@
                         /* context id + surface id? */
                         break;
+
+                    default:
+                        STAM_REL_COUNTER_INC(&pSVGAState->StatFifoUnkCmds);
+                        AssertFailed();
+                        break;
                     }
                 }
                 else
 # endif // VBOX_WITH_VMSVGA3D
+                {
+                    STAM_REL_COUNTER_INC(&pSVGAState->StatFifoUnkCmds);
                     AssertFailed();
+                }
             }
 
@@ -3012 +3052 @@
             }
             ASMAtomicWriteU32(&pFIFO[SVGA_FIFO_STOP], offCurrentCmd);
+            STAM_REL_COUNTER_INC(&pSVGAState->StatFifoCommands);
 
             /* FIFO progress might trigger an interrupt. */
@@ -3747 +3788 @@
      * Statistics.
      */
-    STAM_REG(pVM, &pSVGAState->StatR3CmdPresent,       STAMTYPE_PROFILE, "/Devices/VMSVGA/3d/Cmd/Present",  STAMUNIT_TICKS_PER_CALL, "Profiling of Present.");
-    STAM_REG(pVM, &pSVGAState->StatR3CmdDrawPrimitive, STAMTYPE_PROFILE, "/Devices/VMSVGA/3d/Cmd/DrawPrimitive",  STAMUNIT_TICKS_PER_CALL, "Profiling of DrawPrimitive.");
-    STAM_REG(pVM, &pSVGAState->StatR3CmdSurfaceDMA,    STAMTYPE_PROFILE, "/Devices/VMSVGA/3d/Cmd/SurfaceDMA",  STAMUNIT_TICKS_PER_CALL, "Profiling of SurfaceDMA.");
+    STAM_REG(pVM, &pSVGAState->StatR3CmdPresent,        STAMTYPE_PROFILE, "/Devices/VMSVGA/3d/Cmd/Present",  STAMUNIT_TICKS_PER_CALL, "Profiling of Present.");
+    STAM_REG(pVM, &pSVGAState->StatR3CmdDrawPrimitive,  STAMTYPE_PROFILE, "/Devices/VMSVGA/3d/Cmd/DrawPrimitive",  STAMUNIT_TICKS_PER_CALL, "Profiling of DrawPrimitive.");
+    STAM_REG(pVM, &pSVGAState->StatR3CmdSurfaceDMA,     STAMTYPE_PROFILE, "/Devices/VMSVGA/3d/Cmd/SurfaceDMA",  STAMUNIT_TICKS_PER_CALL, "Profiling of SurfaceDMA.");
+    STAM_REL_REG(pVM, &pSVGAState->StatFifoCommands,    STAMTYPE_COUNTER, "/Devices/VMSVGA/FifoCommands",  STAMUNIT_OCCURENCES, "FIFO command counter.");
+    STAM_REL_REG(pVM, &pSVGAState->StatFifoErrors,      STAMTYPE_COUNTER, "/Devices/VMSVGA/FifoErrors",  STAMUNIT_OCCURENCES, "FIFO error counter.");
+    STAM_REL_REG(pVM, &pSVGAState->StatFifoUnkCmds,     STAMTYPE_COUNTER, "/Devices/VMSVGA/FifoUnknownCommands",  STAMUNIT_OCCURENCES, "FIFO unknown command counter.");
+    STAM_REL_REG(pVM, &pSVGAState->StatFifoTodoTimeout, STAMTYPE_COUNTER, "/Devices/VMSVGA/FifoTodoTimeout",  STAMUNIT_OCCURENCES, "Number of times we discovered pending work after a wait timeout.");
+    STAM_REL_REG(pVM, &pSVGAState->StatFifoTodoWoken,   STAMTYPE_COUNTER, "/Devices/VMSVGA/FifoTodoWoken",  STAMUNIT_OCCURENCES, "Number of times we discovered pending work after being woken up.");
+    STAM_REL_REG(pVM, &pSVGAState->StatFifoStalls,      STAMTYPE_PROFILE, "/Devices/VMSVGA/FifoStalls",  STAMUNIT_TICKS_PER_CALL, "Profiling of FIFO stalls (waiting for guest to finish copying data).");
 
     return VINF_SUCCESS;