Changeset 89661 in vbox for trunk/src/VBox/Devices/Audio

Timestamp:

Jun 13, 2021 11:08:07 PM (4 years ago)

Author:

vboxsync

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

145106

Message:

DevIchAc97: Docs, TODOs, and some ichac97R3StreamUpdateDma refactoring preps. bugref:9890

File:

• trunk/src/VBox/Devices/Audio/DevIchAc97.cpp (modified) (9 diffs)

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

@@ -304 +304 @@
 /**
  * Buffer Descriptor List Entry (BDLE).
+ *
+ * (See section 3.2.1 in Intel document number 252751-001, or section 1.2.2.1 in
+ * Intel document number 302349-003.)
  */
 typedef struct AC97BDLE
@@ -319 +322 @@
 /**
  * Bus master register set for an audio stream.
+ *
+ * (See section 16.2 in Intel document 301473-002, or section 2.2 in Intel
+ * document 302349-003.)
  */
 typedef struct AC97BMREGS
@@ -326 +332 @@
     uint8_t                 lvi;        /**< rw 0, Last valid index. */
     uint16_t                sr;         /**< rw 1, Status register. */
-    uint16_t                picb;       /**< ro 0, Position in current buffer (in samples). */
+    uint16_t                picb;       /**< ro 0, Position in current buffer (samples left to process). */
     uint8_t                 piv;        /**< ro 0, Prefetched index value. */
     uint8_t                 cr;         /**< rw 0, Control register. */
@@ -1265 +1271 @@
                                      PAC97STREAM pStream, PAC97STREAMR3 pStreamCC)
 {
-    int         rc2;
-    PAUDMIXSINK pSink = ichac97R3IndexToSink(pThisCC, pStream->u8SD);
+    /*
+     * Make sure we're running and got an active mixer sink.
+     */
+    PAUDMIXSINK pSink = ichac97R3IndexToSink(pThisCC, pStream->u8SD); /** @todo caller will need + check this too afterwards... */
     AssertPtr(pSink);
-    if (AudioMixerSinkIsActive(pSink))
-    {
-        if (pStreamCC->State.Cfg.enmDir == PDMAUDIODIR_OUT) /* Output (SDO). */
+    if (RT_LIKELY(AudioMixerSinkIsActive(pSink)))
+    { /* likely */ }
+    else
+        return;
+
+    int rc2;
+
+    /*
+     * Output streams (SDO).
+     */
+    if (pStreamCC->State.Cfg.enmDir == PDMAUDIODIR_OUT)
+    {
+        uint32_t cbStreamFree = ichac97R3StreamGetFree(pStreamCC);
+        if (cbStreamFree)
+        { /* likely */ }
+        else
         {
-            uint32_t cbStreamFree = ichac97R3StreamGetFree(pStreamCC);
-            if (cbStreamFree)
-            { /* likely */ }
-            else
-            {
-                /** @todo Record this as a statistic. Try make some space available.    */
-            }
-            if (cbStreamFree)
-            {
-                Log3Func(("[SD%RU8] PICB=%zu (%RU64ms), cbFree=%zu (%RU64ms), cbTransferChunk=%zu (%RU64ms)\n",
-                          pStream->u8SD,
-                          (pStream->Regs.picb << 1), PDMAudioPropsBytesToMilli(&pStreamCC->State.Cfg.Props, pStream->Regs.picb << 1),
-                          cbStreamFree, PDMAudioPropsBytesToMilli(&pStreamCC->State.Cfg.Props, cbStreamFree),
-                          pStreamCC->State.cbTransferChunk, PDMAudioPropsBytesToMilli(&pStreamCC->State.Cfg.Props, pStreamCC->State.cbTransferChunk)));
-
-                /* Do the DMA transfer. */
-                rc2 = ichac97R3StreamTransfer(pDevIns, pThis, pStream, pStreamCC,
-                                              RT_MIN(pStreamCC->State.cbTransferChunk, cbStreamFree));
-                AssertRC(rc2);
-
-                pStreamCC->State.tsLastUpdateNs = RTTimeNanoTS();
-            }
-
+            /** @todo Record this as a statistic. Try make some space available.    */
+        }
+        if (cbStreamFree)
+        {
+            Log3Func(("[SD%RU8] PICB=%zu (%RU64ms), cbFree=%zu (%RU64ms), cbTransferChunk=%zu (%RU64ms)\n",
+                      pStream->u8SD,
+                      (pStream->Regs.picb << 1), PDMAudioPropsBytesToMilli(&pStreamCC->State.Cfg.Props, pStream->Regs.picb << 1),
+                      cbStreamFree, PDMAudioPropsBytesToMilli(&pStreamCC->State.Cfg.Props, cbStreamFree),
+                      pStreamCC->State.cbTransferChunk, PDMAudioPropsBytesToMilli(&pStreamCC->State.Cfg.Props, pStreamCC->State.cbTransferChunk)));
+
+            /* Do the DMA transfer. */
+            rc2 = ichac97R3StreamTransfer(pDevIns, pThis, pStream, pStreamCC,
+                                          RT_MIN(pStreamCC->State.cbTransferChunk, cbStreamFree));
+            AssertRC(rc2);
+
+            pStreamCC->State.tsLastUpdateNs = RTTimeNanoTS();
+        }
+
+        rc2 = AudioMixerSinkSignalUpdateJob(pSink);
+        AssertRC(rc2);
+    }
+    else /* Input (SDI). */
+    {
+#if 0 /* bird: I just love when crusial code like this with no explanation.  This just causing AIO
+   *       skipping a DMA timer cycle if the timer callback is a bit quicker than the 'hint' (see HDA/9890).   */
+        const uint64_t tsNowNs = RTTimeNanoTS();
+        if (tsNowNs - pStreamCC->State.tsLastUpdateNs >= pStreamCC->State.Cfg.Device.cMsSchedulingHint * RT_NS_1MS)
+        {
             rc2 = AudioMixerSinkSignalUpdateJob(pSink);
             AssertRC(rc2);
+
+            pStreamCC->State.tsLastUpdateNs = tsNowNs;
         }
-        else /* Input (SDI). */
+#endif
+
+        uint32_t cbStreamUsed = ichac97R3StreamGetUsed(pStreamCC);
+        if (cbStreamUsed)
+        { /* likey */ }
+        else
         {
-#if 0 /* bird: I just love when crusial code like this with no explanation.  This just causing AIO
-       *       skipping a DMA timer cycle if the timer callback is a bit quicker than the 'hint' (see HDA/9890).   */
-            const uint64_t tsNowNs = RTTimeNanoTS();
-            if (tsNowNs - pStreamCC->State.tsLastUpdateNs >= pStreamCC->State.Cfg.Device.cMsSchedulingHint * RT_NS_1MS)
-            {
-                rc2 = AudioMixerSinkSignalUpdateJob(pSink);
-                AssertRC(rc2);
-
-                pStreamCC->State.tsLastUpdateNs = tsNowNs;
-            }
-#endif
-
-            uint32_t cbStreamUsed = ichac97R3StreamGetUsed(pStreamCC);
-            if (cbStreamUsed)
-            { /* likey */ }
-            else
-            {
-                /** @todo Record this as a statistic. Try pull some data into the DMA buffer.*/
-            }
-
-            if (cbStreamUsed)
-            {
-                /* When running synchronously, do the DMA data transfers here.
-                 * Otherwise this will be done in the stream's async I/O thread. */
-                rc2 = ichac97R3StreamTransfer(pDevIns, pThis, pStream, pStreamCC, cbStreamUsed);
-                AssertRC(rc2);
-            }
-
-            /*
-             * We should always kick the AIO thread.
-             */
-            /** @todo This isn't entirely ideal.  If we get into an underrun situation,
-             *        we ideally want the AIO thread to run right before the DMA timer
-             *        rather than right after it ran. */
-            Log5Func(("Notifying AIO thread\n"));
-            rc2 = AudioMixerSinkSignalUpdateJob(pSink);
+            /** @todo Record this as a statistic. Try pull some data into the DMA buffer.*/
+        }
+
+        if (cbStreamUsed)
+        {
+            /* When running synchronously, do the DMA data transfers here.
+             * Otherwise this will be done in the stream's async I/O thread. */
+            rc2 = ichac97R3StreamTransfer(pDevIns, pThis, pStream, pStreamCC, cbStreamUsed);
             AssertRC(rc2);
-            pStreamCC->State.tsLastUpdateNs = RTTimeNanoTS();
         }
+
+        /*
+         * We should always kick the AIO thread.
+         */
+        /** @todo This isn't entirely ideal.  If we get into an underrun situation,
+         *        we ideally want the AIO thread to run right before the DMA timer
+         *        rather than right after it ran. */
+        Log5Func(("Notifying AIO thread\n"));
+        rc2 = AudioMixerSinkSignalUpdateJob(pSink);
+        AssertRC(rc2);
+        pStreamCC->State.tsLastUpdateNs = RTTimeNanoTS();
     }
 }
@@ -3268 +3283 @@
                     {
                         ichac97MixerSet(pThis, AC97_PCM_Front_DAC_Rate, 0xbb80); /* Set default (48000 Hz). */
+                        /** @todo r=bird: Why reopen it now?  Can't we put that off till it's
+                         *        actually used? */
                         ichac97R3StreamReOpen(pDevIns, pThis, pThisCC, &pThis->aStreams[AC97SOUNDSOURCE_PO_INDEX],
                                               &pThisCC->aStreams[AC97SOUNDSOURCE_PO_INDEX], true /* fForce */);
 
                         ichac97MixerSet(pThis, AC97_PCM_LR_ADC_Rate, 0xbb80); /* Set default (48000 Hz). */
+                        /** @todo r=bird: Why reopen it now?  Can't we put that off till it's
+                         *        actually used? */
                         ichac97R3StreamReOpen(pDevIns, pThis, pThisCC, &pThis->aStreams[AC97SOUNDSOURCE_PI_INDEX],
                                               &pThisCC->aStreams[AC97SOUNDSOURCE_PI_INDEX], true /* fForce */);
@@ -3284 +3303 @@
                     {
                         ichac97MixerSet(pThis, AC97_MIC_ADC_Rate, 0xbb80); /* Set default (48000 Hz). */
+                        /** @todo r=bird: Why reopen it now?  Can't we put that off till it's
+                         *        actually used? */
                         ichac97R3StreamReOpen(pDevIns, pThis, pThisCC, &pThis->aStreams[AC97SOUNDSOURCE_MC_INDEX],
                                               &pThisCC->aStreams[AC97SOUNDSOURCE_MC_INDEX], true /* fForce */);
@@ -3302 +3323 @@
                         LogRel2(("AC97: Setting front DAC rate to 0x%x\n", u32));
                         ichac97MixerSet(pThis, offPort, u32);
+                        /** @todo r=bird: Why reopen it now?  Can't we put that off till it's
+                         *        actually used? */
                         ichac97R3StreamReOpen(pDevIns, pThis, pThisCC, &pThis->aStreams[AC97SOUNDSOURCE_PO_INDEX],
                                               &pThisCC->aStreams[AC97SOUNDSOURCE_PO_INDEX], true /* fForce */);
@@ -3317 +3340 @@
                         LogRel2(("AC97: Setting microphone ADC rate to 0x%x\n", u32));
                         ichac97MixerSet(pThis, offPort, u32);
+                        /** @todo r=bird: Why reopen it now?  Can't we put that off till it's
+                         *        actually used? */
                         ichac97R3StreamReOpen(pDevIns, pThis, pThisCC, &pThis->aStreams[AC97SOUNDSOURCE_MC_INDEX],
                                               &pThisCC->aStreams[AC97SOUNDSOURCE_MC_INDEX], true /* fForce */);
@@ -3332 +3357 @@
                         LogRel2(("AC97: Setting line-in ADC rate to 0x%x\n", u32));
                         ichac97MixerSet(pThis, offPort, u32);
+                        /** @todo r=bird: Why reopen it now?  Can't we put that off till it's
+                         *        actually used? */
                         ichac97R3StreamReOpen(pDevIns, pThis, pThisCC, &pThis->aStreams[AC97SOUNDSOURCE_PI_INDEX],
                                               &pThisCC->aStreams[AC97SOUNDSOURCE_PI_INDEX], true /* fForce */);