Changeset 89853 in vbox

Timestamp:

Jun 23, 2021 1:19:38 PM (4 years ago)

Author:

vboxsync

svn:sync-xref-src-repo-rev:

145310

Message:

DevHda: Added a StatDmaSkippedPendingBcis statistics counter like AC97 already has, and added a @todo trying to guess why we're skipping (== screwing up) DMA periods when BCIS is set. bugref:9890

Location:

trunk/src/VBox/Devices/Audio

Files:

• DevHda.cpp (modified) (1 diff)
• DevHdaStream.cpp (modified) (4 diffs)
• DevHdaStream.h (modified) (2 diffs)

trunk/src/VBox/Devices/Audio/DevHda.cpp

@@ -5107 +5107 @@
                                    "Number of bytes of silence added to cope with underruns.", "Stream%u/DMABufferSilence", idxStream);
         }
+        PDMDevHlpSTAMRegisterF(pDevIns, &pThisCC->aStreams[idxStream].State.StatDmaSkippedPendingBcis, STAMTYPE_COUNTER, STAMVISIBILITY_USED, STAMUNIT_COUNT,
+                               "DMA transfer period skipped because of BCIS pending.", "Stream%u/DMASkippedPendingBCIS", idxStream);
+
         PDMDevHlpSTAMRegisterF(pDevIns, &pThisCC->aStreams[idxStream].State.offRead, STAMTYPE_U64, STAMVISIBILITY_USED, STAMUNIT_BYTES,
                                "Virtual internal buffer read position.",    "Stream%u/offRead", idxStream);

trunk/src/VBox/Devices/Audio/DevHdaStream.cpp

@@ -1211 +1211 @@
  * @param   pThis           The shared HDA device state.
  * @param   pStreamShared   HDA stream to update (shared).
+ * @param   pStreamR3       HDA stream to update (ring-3).
  * @param   uSD             The stream ID (for asserting).
  * @param   tsNowNs         The current RTTimeNano() value.
  * @param   pszFunction     The function name (for logging).
  */
-DECLINLINE(bool) hdaR3StreamDoDmaPrologue(PHDASTATE pThis, PHDASTREAM pStreamShared, uint8_t uSD,
+DECLINLINE(bool) hdaR3StreamDoDmaPrologue(PHDASTATE pThis, PHDASTREAM pStreamShared, PHDASTREAMR3 pStreamR3, uint8_t uSD,
                                           uint64_t tsNowNs, const char *pszFunction)
 {
@@ -1236 +1237 @@
     else
     {
+        /** @todo r=bird: This is a bit fishy.  We should make effort the reschedule
+         *        the transfer immediately after the guest clears the interrupt.
+         *        The same fishy code is present in AC'97 with just a little
+         *        explanation as here, see {9890#c95}.
+         *
+         *        The reasoning is probably that the developer noticed some windows
+         *        versions don't like having their BCIS interrupts bundled.  There were
+         *        comments to that effect elsewhere, probably as a result of a fixed
+         *        uTimerHz approach to DMA scheduling.  However, pausing DMA for a
+         *        period isn't going to help us with the host backends, as they don't
+         *        pause and will want samples ASAP.  So, we should at least unpause
+         *        DMA as quickly as we possible when BCIS is cleared.  We might even
+         *        not skip it iff the DMA work here doesn't involve raising any IOC,
+         *        which is possible although unlikely. */
         Log3(("%s: [SD%RU8] BCIS bit set, skipping transfer\n", pszFunction, uSD));
+        STAM_REL_COUNTER_INC(&pStreamR3->State.StatDmaSkippedPendingBcis);
+        Log(("%s: [SD%RU8] BCIS bit set, skipping transfer\n", pszFunction, uSD));
 # ifdef HDA_STRICT
         /* Timing emulation bug or guest is misbehaving -- let me know. */
@@ -1387 +1404 @@
      * Common prologue.
      */
-    if (hdaR3StreamDoDmaPrologue(pThis, pStreamShared, uSD, tsNowNs, "hdaR3StreamDoDmaInput"))
+    if (hdaR3StreamDoDmaPrologue(pThis, pStreamShared, pStreamR3, uSD, tsNowNs, "hdaR3StreamDoDmaInput"))
     { /* likely */ }
     else
@@ -1596 +1613 @@
      * Common prologue.
      */
-    if (hdaR3StreamDoDmaPrologue(pThis, pStreamShared, uSD, tsNowNs, "hdaR3StreamDoDmaOutput"))
+    if (hdaR3StreamDoDmaPrologue(pThis, pStreamShared, pStreamR3, uSD, tsNowNs, "hdaR3StreamDoDmaOutput"))
     { /* likely */ }
     else

trunk/src/VBox/Devices/Audio/DevHdaStream.h

@@ -290 +290 @@
         /** Number of bytes involved in unresolved flow errors. */
         STAMCOUNTER             StatDmaFlowErrorBytes;
+        /** DMA skipped because buffer interrupt pending. */
+        STAMCOUNTER             StatDmaSkippedPendingBcis;
 
         STAMPROFILE             StatStart;
@@ -298 +300 @@
     /** Debug bits. */
     HDASTREAMDEBUG              Dbg;
-    uint64_t                    au64Alignment[2+4];
+    uint64_t                    au64Alignment[1+4];
 } HDASTREAMR3;
 AssertCompileSizeAlignment(HDASTREAMR3, 64);