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

Timestamp:

Jun 16, 2021 8:38:13 PM (4 years ago)

Author:

vboxsync

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

145202

Message:

DevIchAc97: Restored the BDLE fetching to the LVI write w/ AC97_SR_DCH set condition, and added re-arming the timer. bugref:9890

File:

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

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

@@ -2656 +2656 @@
                     case AC97_NABM_OFF_LVI:
                         DEVAC97_LOCK_RETURN(pDevIns, pThis, VINF_IOM_R3_IOPORT_WRITE);
-                        if (   (pRegs->cr & AC97_CR_RPBM)
-                            && (pRegs->sr & AC97_SR_DCH))
+                        if (   !(pRegs->sr & AC97_SR_DCH)
+                            || !(pRegs->cr & AC97_CR_RPBM))
                         {
+                            pRegs->lvi = u32 % AC97_MAX_BDLE;
+                            DEVAC97_UNLOCK(pDevIns, pThis);
+                            Log3Func(("[SD%RU8] LVI <- %#x\n", pStream->u8SD, u32));
+                        }
+                        else
+                        {
+#ifdef IN_RING3
+                            /* Recover from underflow situation where CIV caught up with LVI
+                               and the DMA processing stopped.  We clear the status condition,
+                               update LVI and then try to load the next BDLE.  Unfortunately,
+                               we cannot do this from ring-3 as much of the BDLE state is
+                               ring-3 only. */
                             pRegs->sr &= ~(AC97_SR_DCH | AC97_SR_CELV);
-                            pRegs->civ = pRegs->piv;
-                            pRegs->piv = (pRegs->piv + 1) % AC97_MAX_BDLE;
-                            /** @todo r=bird: there used to be a ichac97StreamFetchBDLE here, but it was
-                             * removed in r128222 without any helpful clue in the commit message
-                             * ("Audio/AC97: Added more code for transfer calculation for helping with A/V
-                             * synchronization"). There wasn't and isn't any clue to the logic here in the
-                             * source code either, of course, so at least there is some consistency. :-)
-                             *
-                             * The old code with the fetch was probably correct, though, as we're now
-                             * skipping one BDLE (CIV is incremented here, but since PICB is still zero
-                             * we'll increment CIV again in the DMA transfer loop - the way I read it).
-                             *
-                             * I found no special handling of DCH in the linux code, but that doesn't prove
-                             * anything as they might just expect the device never to get into this underrun
-                             * situation...  Ditto windows 8.1 AC'97 sample driver. */
+                            pRegs->lvi = u32 % AC97_MAX_BDLE;
+                            if (ichac97R3StreamFetchNextBdle(pDevIns, pStream, pStreamCC))
+                                ichac97StreamUpdateSR(pDevIns, pThis, pStream, pRegs->sr | AC97_SR_BCIS);
+
+                            /* We now have to re-arm the DMA timer according to the new BDLE length.
+                               This means leaving the device lock to avoid virtual sync lock order issues. */
+                            ichac97R3StreamTransferUpdate(pDevIns, pStream, pStreamCC);
+                            uint64_t const cTicksToDeadline = pStreamCC->State.cTransferTicks;
+
+                            /** @todo Stop the DMA timer when we get into the AC97_SR_CELV situation to
+                             *        avoid potential race here. */
+                            DEVAC97_UNLOCK(pDevIns, pThis);
+
+                            LogFunc(("[SD%RU8] LVI <- %#x; CIV=%#x PIV=%#x SR=%#x cTicksToDeadline=%#RX64 [recovering]\n",
+                                     pStream->u8SD, u32, pRegs->civ, pRegs->piv, pRegs->sr, cTicksToDeadline));
+
+                            /** @todo take down the start time here.  */
+                            int rc2 = PDMDevHlpTimerSetRelative(pDevIns, pStream->hTimer, cTicksToDeadline, NULL /*pu64Now*/);
+                            AssertRC(rc2);
+#else
+                            rc = VINF_IOM_R3_IOPORT_WRITE;
+#endif
                         }
-                        pRegs->lvi = u32 % AC97_MAX_BDLE;
-                        DEVAC97_UNLOCK(pDevIns, pThis);
-                        Log3Func(("[SD%RU8] LVI <- %#x\n", pStream->u8SD, u32));
                         break;
 
@@ -2721 +2737 @@
                             {
                                 /** @todo r=bird: How do we prevent the guest from triggering enable more
-                                 *        than once? */
+                                 *        than once?  Only take action if the RBMP bit changed. Duh^3 */
                                 Log3Func(("[SD%RU8] Enable\n", pStream->u8SD));
 
@@ -2738 +2754 @@
                                  * create a lock order violation with the virtual sync time lock otherwise.
                                  */
+                                /** @todo is ichac97R3StreamTransferUpdate called here? */
                                 uint64_t const cTicksToDeadline = pStreamCC->State.cTransferTicks;