Changeset 73833 in vbox for trunk/src

Timestamp:

Aug 22, 2018 3:40:27 PM (6 years ago)

Author:

vboxsync

Message:

Audio: Async I/O fixes for HDA + AC'97.

Location:

trunk/src/VBox/Devices/Audio

Files:

• DevHDA.cpp (modified) (2 diffs)
• DevIchAc97.cpp (modified) (5 diffs)
• HDAStream.cpp (modified) (9 diffs)
• HDAStream.h (modified) (1 diff)

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

@@ -1363 +1363 @@
 # ifdef VBOX_WITH_AUDIO_HDA_ASYNC_IO
         hdaR3StreamAsyncIOLock(pStream);
-        hdaR3StreamAsyncIOEnable(pStream, false /* fEnable */);
 # endif
         /* Make sure to remove the run bit before doing the actual stream reset. */
@@ -1397 +1396 @@
             if (fRun)
             {
-# ifdef VBOX_WITH_AUDIO_HDA_ASYNC_IO
-                hdaR3StreamAsyncIOEnable(pStream, fRun /* fEnable */);
-# endif
                 /* (Re-)initialize the stream with current values. */
                 rc2 = hdaR3StreamInit(pStream, pStream->u8SD);

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

@@ -839 +839 @@
         rc = ichac97R3StreamAsyncIOCreate(pThis, pStream);
     if (RT_SUCCESS(rc))
-    {
         ichac97R3StreamAsyncIOLock(pStream);
-        ichac97R3StreamAsyncIOEnable(pStream, fEnable);
-    }
 # endif
 
@@ -1288 +1285 @@
     {
         pAIO->fShutdown = false;
+        pAIO->fEnabled  = true; /* Enabled by default. */
 
         rc = RTSemEventCreate(&pAIO->Event);
@@ -1421 +1419 @@
  * The host sink(s) set the overall pace.
  *
- * This routine is called by both, the synchronous and the asynchronous, implementations.
+ * This routine is called by both, the synchronous and the asynchronous
+ * (VBOX_WITH_AUDIO_AC97_ASYNC_IO), implementations.
+ *
+ * When running synchronously, the device DMA transfers *and* the mixer sink
+ * processing is within the device timer.
+ *
+ * When running asynchronously, only the device DMA transfers are done in the
+ * device timer, whereas the mixer sink processing then is done in the stream's
+ * own async I/O thread. This thread also will call this function
+ * (with fInTimer set to @c false).
  *
  * @param   pThis               AC'97 state.
@@ -1430 +1437 @@
 static void ichac97R3StreamUpdate(PAC97STATE pThis, PAC97STREAM pStream, bool fInTimer)
 {
+    RT_NOREF(fInTimer);
+
     PAUDMIXSINK pSink = ichac97R3IndexToSink(pThis, pStream->u8SD);
     AssertPtr(pSink);
@@ -1440 +1449 @@
     if (pStream->u8SD == AC97SOUNDSOURCE_PO_INDEX) /* Output (SDO). */
     {
-#ifdef LOG_ENABLED
-        const uint64_t deltaLastUpdateNs = RTTimeNanoTS() - pStream->State.tsLastUpdateNs;
-#endif
-        pStream->State.tsLastUpdateNs = RTTimeNanoTS();
-
-        PPDMAUDIOPCMPROPS pProps = &pStream->State.Cfg.Props;
-
-        /* Make sure that we don't transfer more than we need for this slot. */
-        uint32_t cbToTransfer = DrvAudioHlpMilliToBytes(pStream->State.Cfg.Device.uSchedulingHintMs, pProps);
-
-        /* Make sure that the transfer is frame-aligned.
-         * Add one additional frame to not transfer too little because of the alignment;
-         * ichac97R3StreamTransfer() will take care of clamping to the correct value then. */
-        cbToTransfer = DrvAudioHlpBytesAlign(cbToTransfer, pProps) + PDMAUDIOPCMPROPS_F2B(pProps, 1 /* Frame */);
-
-        Log3Func(("[SD%RU8] cbToTransfer=%RU32, deltaLastUpdateNs=%RU64\n", pStream->u8SD, cbToTransfer, deltaLastUpdateNs / RT_NS_1MS));
-
-        if (   fInTimer
-            && cbToTransfer)
-        {
-            /* Do the DMA transfer. */
-            rc2 = ichac97R3StreamTransfer(pThis, pStream, cbToTransfer);
+        /* How much (guest output) data is available at the moment for the AC'97 stream? */
+        /* Only read from the AC'97 stream at the given scheduling rate. */
+        bool fDoRead = false; /* Whether to read from the AC'97 stream or not. */
+
+# ifdef VBOX_WITH_AUDIO_AC97_ASYNC_IO
+        if (fInTimer)
+# endif
+        {
+            const uint32_t cbStreamFree = ichac97R3StreamGetFree(pStream);
+            if (cbStreamFree)
+            {
+                /* Do the DMA transfer. */
+                rc2 = ichac97R3StreamTransfer(pThis, pStream, cbStreamFree);
+                AssertRC(rc2);
+            }
+
+            /* Only read from the AC'97 stream at the given scheduling rate. */
+            const uint64_t tsNowNs = RTTimeNanoTS();
+            if (tsNowNs - pStream->State.tsLastUpdateNs >= pStream->State.Cfg.Device.uSchedulingHintMs * RT_NS_1MS)
+            {
+                fDoRead = true;
+                pStream->State.tsLastUpdateNs = tsNowNs;
+            }
+        }
+
+        Log3Func(("[SD%RU8] fInTimer=%RTbool, fDoRead=%RTbool\n", pStream->u8SD, fInTimer, fDoRead));
+
+# ifdef VBOX_WITH_AUDIO_AC97_ASYNC_IO
+        if (fDoRead)
+        {
+            rc2 = ichac97R3StreamAsyncIONotify(pStream);
             AssertRC(rc2);
         }
-
-        /* Make sure that we don't write more than we need for this slot. */
-        uint32_t cbToWrite = RT_MIN(cbToTransfer, ichac97R3StreamGetUsed(pStream));
-
-        /* Make sure that the write is byte-aligned. */
-        cbToWrite = DrvAudioHlpBytesAlign(cbToWrite, pProps);
-
-        Log3Func(("[SD%RU8] cbToWrite=%RU32, deltaLastUpdateNs=%RU64\n", pStream->u8SD, cbToWrite, deltaLastUpdateNs / RT_NS_1MS));
+# endif
 
 # ifdef VBOX_WITH_AUDIO_AC97_ASYNC_IO
-        if (   fInTimer
-            && cbToWrite)
-        {
-            rc2 = ichac97R3StreamAsyncIONotify(pThis, pStream);
-            AssertRC(rc2);
-        }
-        else
+        if (!fInTimer) /* In async I/O thread */
+        {
+# else
+        if (fDoRead)
+        {
 # endif
-        {
-            if (cbToWrite)
+            const uint32_t cbSinkWritable     = AudioMixerSinkGetWritable(pSink);
+            const uint32_t cbStreamReadable   = ichac97R3StreamGetUsed(pStream);
+            const uint32_t cbToReadFromStream = RT_MIN(cbStreamReadable, cbSinkWritable);
+
+            Log3Func(("[SD%RU8] cbSinkWritable=%RU32, cbStreamReadable=%RU32\n", pStream->u8SD, cbSinkWritable, cbStreamReadable));
+
+            if (cbToReadFromStream)
             {
                 /* Read (guest output) data and write it to the stream's sink. */
-                uint32_t cbRead;
-                rc2 = ichac97R3StreamRead(pThis, pStream, pSink, cbToWrite, &cbRead);
+                rc2 = ichac97R3StreamRead(pThis, pStream, pSink, cbToReadFromStream, NULL);
                 AssertRC(rc2);
             }
-
-            /* When running synchronously, update the associated sink here.
-             * Otherwise this will be done in the device timer. */
-            rc2 = AudioMixerSinkUpdate(pSink);
-            AssertRC(rc2);
-
-        }
+        }
+
+        /* When running synchronously, update the associated sink here.
+         * Otherwise this will be done in the async I/O thread. */
+        rc2 = AudioMixerSinkUpdate(pSink);
+        AssertRC(rc2);
     }
     else /* Input (SDI). */

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

@@ -67 +67 @@
     rc = hdaR3StreamPeriodCreate(&pStream->State.Period);
     AssertRCReturn(rc, rc);
+
+    pStream->State.tsLastUpdateNs = 0;
 
 #ifdef DEBUG
@@ -378 +380 @@
         pStream->State.cTransferPendingInterrupts = 0;
         pStream->State.cbDMALeft                  = 0;
+        pStream->State.tsLastUpdateNs             = 0;
 
         const uint64_t cTicksPerHz = TMTimerGetFreq(pStream->pTimer) / pThis->u16TimerHz;
@@ -785 +788 @@
 
             Assert(cbSrc >= cbWritten);
-            Log2Func(("[SD%RU8]: %zu/%zu bytes read\n", pStream->u8SD, cbWritten, cbSrc));
+            Log2Func(("[SD%RU8]: %RU32/%zu bytes read\n", pStream->u8SD, cbWritten, cbSrc));
         }
 
@@ -891 +894 @@
     if (cbToProcess > cbToProcessMax)
     {
-        if (pStream->State.Cfg.enmDir == PDMAUDIODIR_IN)
-            LogRelMax2(64, ("HDA: Warning: DMA buffer underflow for stream #%RU8 (still %RU32 bytes needed)\n",
-                            pStream->u8SD, cbToProcess - cbToProcessMax));
-        else
-            LogRelMax2(64, ("HDA: Warning: DMA buffer overflow for stream #%RU8 (%RU32 bytes outstanding)\n",
-                            pStream->u8SD, cbToProcess - cbToProcessMax));
-
-        LogFunc(("[SD%RU8] Warning: Limiting transfer (cbToProcess=%RU32, cbToProcessMax=%RU32)\n",
+        LogFunc(("[SD%RU8] Limiting transfer (cbToProcess=%RU32, cbToProcessMax=%RU32)\n",
                  pStream->u8SD, cbToProcess, cbToProcessMax));
 
@@ -982 +978 @@
             {
                 const uint32_t cbDMAFree = (uint32_t)RTCircBufFree(pCircBuf);
-
-                if (cbDMAFree < cbDMA)
-                    LogRelMax2(64, ("HDA: Warning: DMA buffer overflow of stream #%RU8 (discarding %RU32 bytes)\n",
-                               pStream->u8SD, cbDMA - cbDMAFree));
+                Assert(cbDMAFree >= cbDMA); /* This must always hold. */
 
 #ifndef VBOX_WITH_HDA_AUDIO_INTERLEAVING_STREAMS_SUPPORT
@@ -1275 +1268 @@
  * This routine is called by both, the synchronous and the asynchronous, implementations.
  *
+ * This routine is called by both, the synchronous and the asynchronous
+ * (VBOX_WITH_AUDIO_HDA_ASYNC_IO), implementations.
+ *
+ * When running synchronously, the device DMA transfers *and* the mixer sink
+ * processing is within the device timer.
+ *
+ * When running asynchronously, only the device DMA transfers are done in the
+ * device timer, whereas the mixer sink processing then is done in the stream's
+ * own async I/O thread. This thread also will call this function
+ * (with fInTimer set to @c false).
+ *
  * @param   pStream             HDA stream to update.
  * @param   fInTimer            Whether to this function was called from the timer
@@ -1298 +1302 @@
     if (hdaGetDirFromSD(pStream->u8SD) == PDMAUDIODIR_OUT) /* Output (SDO). */
     {
-        /* Is the HDA stream ready to be written (guest output data) to? If so, by how much? */
-        const uint32_t cbFree = hdaR3StreamGetFree(pStream);
-
-        if (   fInTimer
-            && cbFree)
-        {
-            Log3Func(("[SD%RU8] cbFree=%RU32\n", pStream->u8SD, cbFree));
-
-            /* Do the DMA transfer. */
-            rc2 = hdaR3StreamTransfer(pStream, cbFree);
-            AssertRC(rc2);
-        }
-
-        /* How much (guest output) data is available at the moment for the HDA stream? */
-        uint32_t cbUsed = hdaR3StreamGetUsed(pStream);
-
-#ifdef VBOX_WITH_AUDIO_HDA_ASYNC_IO
-        if (   fInTimer
-            && cbUsed)
+        bool fDoRead = false; /* Whether to read from the HDA stream or not. */
+
+# ifdef VBOX_WITH_AUDIO_HDA_ASYNC_IO
+        if (fInTimer)
+# endif
+        {
+            const uint32_t cbStreamFree = hdaR3StreamGetFree(pStream);
+            if (cbStreamFree)
+            {
+                /* Do the DMA transfer. */
+                rc2 = hdaR3StreamTransfer(pStream, cbStreamFree);
+                AssertRC(rc2);
+            }
+
+            /* Only read from the HDA stream at the given scheduling rate. */
+            const uint64_t tsNowNs = RTTimeNanoTS();
+            if (tsNowNs - pStream->State.tsLastUpdateNs >= pStream->State.Cfg.Device.uSchedulingHintMs * RT_NS_1MS)
+            {
+                fDoRead = true;
+                pStream->State.tsLastUpdateNs = tsNowNs;
+            }
+        }
+
+        Log3Func(("[SD%RU8] fInTimer=%RTbool, fDoRead=%RTbool\n", pStream->u8SD, fInTimer, fDoRead));
+
+# ifdef VBOX_WITH_AUDIO_HDA_ASYNC_IO
+        if (fDoRead)
         {
             rc2 = hdaR3StreamAsyncIONotify(pStream);
             AssertRC(rc2);
         }
-        else
-        {
-#endif
-            const uint32_t cbSinkWritable = AudioMixerSinkGetWritable(pSink);
-
-            Log3Func(("[SD%RU8] cbWritable=%RU32, cbUsed=%RU32\n", pStream->u8SD, cbSinkWritable, cbUsed));
-
-            /* Do not write more than the sink can hold at the moment.
-             * The host sets the overall pace. */
-            if (cbUsed > cbSinkWritable)
-                cbUsed = cbSinkWritable;
-
-            if (cbUsed)
+# endif
+
+# ifdef VBOX_WITH_AUDIO_HDA_ASYNC_IO
+        if (!fInTimer) /* In async I/O thread */
+        {
+# else
+        if (fDoRead)
+        {
+# endif
+            const uint32_t cbSinkWritable     = AudioMixerSinkGetWritable(pSink);
+            const uint32_t cbStreamReadable   = hdaR3StreamGetUsed(pStream);
+            const uint32_t cbToReadFromStream = RT_MIN(cbStreamReadable, cbSinkWritable);
+
+            Log3Func(("[SD%RU8] cbSinkWritable=%RU32, cbStreamReadable=%RU32\n", pStream->u8SD, cbSinkWritable, cbStreamReadable));
+
+            if (cbToReadFromStream)
             {
                 /* Read (guest output) data and write it to the stream's sink. */
-                rc2 = hdaR3StreamRead(pStream, cbUsed, NULL /* pcbRead */);
+                rc2 = hdaR3StreamRead(pStream, cbToReadFromStream, NULL);
                 AssertRC(rc2);
             }
 
             /* When running synchronously, update the associated sink here.
-             * Otherwise this will be done in the stream's dedicated async I/O thread. */
+             * Otherwise this will be done in the async I/O thread. */
             rc2 = AudioMixerSinkUpdate(pSink);
             AssertRC(rc2);
-
-#ifdef VBOX_WITH_AUDIO_HDA_ASYNC_IO
-        }
-#endif
+        }
     }
     else /* Input (SDI). */
     {
-#ifdef VBOX_WITH_AUDIO_HDA_ASYNC_IO
+# ifdef VBOX_WITH_AUDIO_HDA_ASYNC_IO
         if (!fInTimer)
         {
-#endif
+# endif
             rc2 = AudioMixerSinkUpdate(pSink);
             AssertRC(rc2);
 
             /* Is the sink ready to be read (host input data) from? If so, by how much? */
-            uint32_t cbReadable = AudioMixerSinkGetReadable(pSink);
+            uint32_t cbSinkReadable = AudioMixerSinkGetReadable(pSink);
 
             /* How much (guest input) data is available for writing at the moment for the HDA stream? */
-            const uint32_t cbFree = hdaR3StreamGetFree(pStream);
-
-            Log3Func(("[SD%RU8] cbReadable=%RU32, cbFree=%RU32\n", pStream->u8SD, cbReadable, cbFree));
-
-            /* Do not write more than the sink can hold at the moment.
+            const uint32_t cbStreamFree = hdaR3StreamGetFree(pStream);
+
+            Log3Func(("[SD%RU8] cbSinkReadable=%RU32, cbStreamFree=%RU32\n", pStream->u8SD, cbSinkReadable, cbStreamFree));
+
+            /* Do not read more than the HDA stream can hold at the moment.
              * The host sets the overall pace. */
-            if (cbReadable > cbFree)
-                cbReadable = cbFree;
-
-            if (cbReadable)
+            if (cbSinkReadable > cbStreamFree)
+                cbSinkReadable = cbStreamFree;
+
+            if (cbSinkReadable)
             {
                 uint8_t abFIFO[HDA_FIFO_MAX + 1];
-                while (cbReadable)
+                while (cbSinkReadable)
                 {
                     uint32_t cbRead;
                     rc2 = AudioMixerSinkRead(pSink, AUDMIXOP_COPY,
-                                             abFIFO, RT_MIN(cbReadable, (uint32_t)sizeof(abFIFO)), &cbRead);
+                                             abFIFO, RT_MIN(cbSinkReadable, (uint32_t)sizeof(abFIFO)), &cbRead);
                     AssertRCBreak(rc2);
 
                     if (!cbRead)
                     {
-                        AssertMsgFailed(("Nothing read from sink, even if %RU32 bytes were (still) announced\n", cbReadable));
+                        AssertMsgFailed(("Nothing read from sink, even if %RU32 bytes were (still) announced\n", cbSinkReadable));
                         break;
                     }
@@ -1398 +1410 @@
                     }
 
-                    Assert(cbReadable >= cbRead);
-                    cbReadable -= cbRead;
+                    Assert(cbSinkReadable >= cbRead);
+                    cbSinkReadable -= cbRead;
                 }
             }
-        #if 0
-            else /* Send silence as input. */
-            {
-                cbReadable = pStream->State.cbTransferSize - pStream->State.cbTransferProcessed;
-
-                Log3Func(("[SD%RU8] Sending silence (%RU32 bytes)\n", pStream->u8SD, cbReadable));
-
-                if (cbReadable)
-                {
-                    rc2 = hdaR3StreamWrite(pStream, NULL /* Silence */, cbReadable, NULL /* pcbWritten */);
-                    AssertRC(rc2);
-                }
-            }
-        #endif
-#ifdef VBOX_WITH_AUDIO_HDA_ASYNC_IO
+# ifdef VBOX_WITH_AUDIO_HDA_ASYNC_IO
         }
         else /* fInTimer */
         {
-#endif
-            const uint64_t tsNow = RTTimeMilliTS();
-            static uint64_t s_lasti = 0;
-            if (s_lasti == 0)
-                s_lasti = tsNow;
-
-            if (tsNow - s_lasti >= 10) /** @todo Fix this properly. */
+# endif
+
+# ifdef VBOX_WITH_AUDIO_HDA_ASYNC_IO
+            const uint64_t tsNowNs = RTTimeNanoTS();
+            if (tsNowNs - pStream->State.tsLastUpdateNs >= pStream->State.Cfg.Device.uSchedulingHintMs * RT_NS_1MS)
             {
                 rc2 = hdaR3StreamAsyncIONotify(pStream);
                 AssertRC(rc2);
 
-                s_lasti = tsNow;
-            }
-
-            const uint32_t cbToTransfer = hdaR3StreamGetUsed(pStream);
-            if (cbToTransfer)
-            {
-                rc2 = hdaR3StreamTransfer(pStream, cbToTransfer);
+                pStream->State.tsLastUpdateNs = tsNowNs;
+            }
+# endif
+            const uint32_t cbStreamFree = hdaR3StreamGetUsed(pStream);
+            if (cbStreamFree)
+            {
+                rc2 = hdaR3StreamTransfer(pStream, cbStreamFree);
                 AssertRC(rc2);
             }
-#ifdef VBOX_WITH_AUDIO_HDA_ASYNC_IO
-        }
-#endif
+# ifdef VBOX_WITH_AUDIO_HDA_ASYNC_IO
+        }
+# endif
     }
 }
@@ -1754 +1750 @@
     {
         pAIO->fShutdown = false;
+        pAIO->fEnabled  = true; /* Enabled by default. */
 
         rc = RTSemEventCreate(&pAIO->Event);

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

@@ -177 +177 @@
     /** Unused, padding. */
     uint8_t                 abPadding4[2+4];
+   /** Timestamp (in ns) of last stream update. */
+    uint64_t                tsLastUpdateNs;
 } HDASTREAMSTATE;
 AssertCompileSizeAlignment(HDASTREAMSTATE, 8);