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

Timestamp:

Jun 18, 2021 11:07:06 AM (4 years ago)

Author:

vboxsync

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

145228

Message:

DevIchAc97: Some experiment of PICB sub-buffer reporting. bugref:9890

File:

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

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

@@ -370 +370 @@
      *  Can be 0 if no next transfer is scheduled. */
     uint64_t                tsTransferNext;
-    /** Transfer chunk size (in bytes) of a transfer period. */
-    uint32_t                cbTransferChunk;
     /** The stream's timer Hz rate.
      *  This value can can be different from the device's default Hz rate,
@@ -386 +384 @@
      *  parameters. */
     uint64_t                nsRetrySetup;
-    /** (Virtual) clock ticks per transfer. */
-    uint64_t                cTransferTicks;
     /** Timestamp (in ns) of last stream update. */
     uint64_t                tsLastUpdateNs;
@@ -452 +448 @@
     /** Bus master registers of this stream. */
     AC97BMREGS              Regs;
+
     /** The timer for pumping data thru the attached LUN drivers. */
     TMTIMERHANDLE           hTimer;
+    /** When the timer was armed (timer clock). */
+    uint64_t                uArmedTs;
+    /** (Virtual) clock ticks per transfer. */
+    uint64_t                cDmaPeriodTicks;
+    /** Transfer chunk size (in bytes) of a transfer period. */
+    uint32_t                cbDmaPeriod;
+    /** DMA period counter (for logging). */
+    uint32_t                uDmaPeriod;
+
     STAMCOUNTER             StatWriteLvi;
     STAMCOUNTER             StatWriteSr1;
@@ -1259 +1265 @@
 
     /* The amount we're supposed to be transfering in this DMA period. */
-    uint32_t cbPeriod = pStreamCC->State.cbTransferChunk;
+    uint32_t cbPeriod = pStream->cbDmaPeriod;
 
     /*
@@ -1520 +1526 @@
 
         if (cbLeftInBdle <= cbMaxPerHz)
-            pStreamCC->State.cbTransferChunk = cbLeftInBdle;
+            pStream->cbDmaPeriod = cbLeftInBdle;
         /* Try avoid leaving a very short period at the end of a buffer. */
         else if (cbLeftInBdle >= cbMaxPerHz + cbMaxPerHz / 2)
-            pStreamCC->State.cbTransferChunk = cbMaxPerHz;
+            pStream->cbDmaPeriod = cbMaxPerHz;
         else
-            pStreamCC->State.cbTransferChunk = PDMAudioPropsFloorBytesToFrame(&pStreamCC->State.Cfg.Props, cbLeftInBdle / 2);
+            pStream->cbDmaPeriod = PDMAudioPropsFloorBytesToFrame(&pStreamCC->State.Cfg.Props, cbLeftInBdle / 2);
 
         /*
          * Translate the chunk size to timer ticks.
          */
-        uint64_t const cNsXferChunk     = PDMAudioPropsBytesToNano(&pStreamCC->State.Cfg.Props, pStreamCC->State.cbTransferChunk);
-        pStreamCC->State.cTransferTicks = PDMDevHlpTimerFromNano(pDevIns, pStream->hTimer, cNsXferChunk);
-        Assert(pStreamCC->State.cTransferTicks > 0);
-
-        Log3Func(("[SD%RU8] cbLeftInBdle=%#RX32 cbMaxPerHz=%#RX32 (%RU16Hz) -> cbTransferChunk=%#RX32 cTransferTicks=%RX64\n",
-                  pStream->u8SD, cbLeftInBdle, cbMaxPerHz, pStreamCC->State.uTimerHz,
-                  pStreamCC->State.cbTransferChunk, pStreamCC->State.cTransferTicks));
+        uint64_t const cNsXferChunk = PDMAudioPropsBytesToNano(&pStreamCC->State.Cfg.Props, pStream->cbDmaPeriod);
+        pStream->cDmaPeriodTicks    = PDMDevHlpTimerFromNano(pDevIns, pStream->hTimer, cNsXferChunk);
+        Assert(pStream->cDmaPeriodTicks > 0);
+
+        Log3Func(("[SD%RU8] cbLeftInBdle=%#RX32 cbMaxPerHz=%#RX32 (%RU16Hz) -> cbDmaPeriod=%#RX32 cDmaPeriodTicks=%RX64\n",
+                  pStream->u8SD, cbLeftInBdle, cbMaxPerHz, pStreamCC->State.uTimerHz, pStream->cbDmaPeriod, pStream->cDmaPeriodTicks));
     }
 }
@@ -1552 +1557 @@
 DECLINLINE(void) ichac97R3TimerSet(PPDMDEVINS pDevIns, PAC97STREAM pStream, uint64_t cTicksToDeadline)
 {
-    int rc = PDMDevHlpTimerSetRelative(pDevIns, pStream->hTimer, cTicksToDeadline, NULL /*pu64Now*/);
+    int rc = PDMDevHlpTimerSetRelative(pDevIns, pStream->hTimer, cTicksToDeadline, &pStream->uArmedTs);
     AssertRC(rc);
 }
@@ -1579 +1584 @@
         ichac97R3StreamUpdateDma(pDevIns, pThis, pThisCC, pStream, pStreamCC, pSink);
 
+        pStream->uDmaPeriod++;
         ichac97R3StreamTransferUpdate(pDevIns, pStream, pStreamCC);
-        ichac97R3TimerSet(pDevIns, pStream, pStreamCC->State.cTransferTicks);
+        ichac97R3TimerSet(pDevIns, pStream, pStream->cDmaPeriodTicks);
     }
 
@@ -2284 +2290 @@
     if (fEnable)
     {
-        /* Reset the input pre-buffering state. */
+        /* Reset the input pre-buffering state and DMA period counter. */
         pStreamCC->State.fInputPreBuffered = false;
+        pStream->uDmaPeriod = 0;
 
         /* Set up (update) the AC'97 stream as needed. */
@@ -2527 +2534 @@
                         break;
                     case AC97_NABM_OFF_PICB:
-                        /* Position in Current Buffer */
+                        /* Position in Current Buffer
+                         * ---
+                         * We can do DMA work here if we want to give the guest a better impression of
+                         * the DMA engine of a real device.  For ring-0 we'd have to add some buffering
+                         * to AC97STREAM (4K or so), only going to ring-3 if full.  Ring-3 would commit
+                         * that buffer and write directly to the internal DMA pCircBuf.
+                         *
+                         * Checking a Linux guest (knoppix 8.6.2), I see some PIC reads each DMA cycle,
+                         * however most of these happen very very early, 1-10% into the buffer. So, I'm
+                         * not sure if it's worth it, as it'll be a big complication... */
+#if 1
                         *pu32 = pRegs->picb;
-                        Log3Func(("PICB[%d] -> %#x\n", AC97_PORT2IDX(offPort), *pu32));
+# ifdef LOG_ENABLED
+                        if (LogIs3Enabled())
+                        {
+                            uint64_t offPeriod = PDMDevHlpTimerGet(pDevIns, pStream->hTimer) - pStream->uArmedTs;
+                            Log3Func(("PICB[%d] -> %#x (%RU64 of %RU64 ticks / %RU64%% into DMA period #%RU32)\n",
+                                      AC97_PORT2IDX(offPort), *pu32, offPeriod, pStream->cDmaPeriodTicks,
+                                      pStream->cDmaPeriodTicks ? offPeriod * 100 / pStream->cDmaPeriodTicks : 0,
+                                      pStream->uDmaPeriod));
+                        }
+# endif
+#else /* For trying out sub-buffer PICB.  Will cause distortions, but can be helpful to see if it help eliminate other issues.  */
+                        if (   (pStream->Regs.cr & AC97_CR_RPBM)
+                            && !(pStream->Regs.sr & AC97_SR_DCH)
+                            && pStream->uArmedTs > 0
+                            && pStream->cDmaPeriodTicks > 0)
+                        {
+                            uint64_t const offPeriod = PDMDevHlpTimerGet(pDevIns, pStream->hTimer) - pStream->uArmedTs;
+                            uint32_t cSamples;
+                            if (offPeriod < pStream->cDmaPeriodTicks)
+                                cSamples = pStream->Regs.picb * offPeriod / pStream->cDmaPeriodTicks;
+                            else
+                                cSamples = pStream->Regs.picb;
+                            if (cSamples + 8 < pStream->Regs.picb)
+                            { /* likely */ }
+                            else if (pStream->Regs.picb > 8)
+                                cSamples = pStream->Regs.picb - 8;
+                            else
+                                cSamples = 0;
+                            *pu32 = pRegs->picb - cSamples;
+                            Log3Func(("PICB[%d] -> %#x (PICB=%#x cSamples=%#x offPeriod=%RU64 of %RU64 / %RU64%%)\n",
+                                      AC97_PORT2IDX(offPort), *pu32, pRegs->picb, cSamples, offPeriod, pStream->cDmaPeriodTicks,
+                                      offPeriod * 100 / pStream->cDmaPeriodTicks));
+                        }
+                        else
+                        {
+                            *pu32 = pRegs->picb;
+                            Log3Func(("PICB[%d] -> %#x\n", AC97_PORT2IDX(offPort), *pu32));
+                        }
+#endif
                         break;
                     default:
@@ -2695 +2750 @@
                                This means leaving the device lock to avoid virtual sync lock order issues. */
                             ichac97R3StreamTransferUpdate(pDevIns, pStream, pStreamCC);
-                            uint64_t const cTicksToDeadline = pStreamCC->State.cTransferTicks;
+                            uint64_t const cTicksToDeadline = pStream->cDmaPeriodTicks;
 
                             /** @todo Stop the DMA timer when we get into the AC97_SR_CELV situation to
@@ -2705 +2760 @@
                                      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*/);
+                            int rc2 = PDMDevHlpTimerSetRelative(pDevIns, pStream->hTimer, cTicksToDeadline, &pStream->uArmedTs);
                             AssertRC(rc2);
 #else
@@ -2794 +2848 @@
                              */
                             ichac97R3StreamTransferUpdate(pDevIns, pStream, pStreamCC);
-                            uint64_t const cTicksToDeadline = pStreamCC->State.cTransferTicks;
+                            uint64_t const cTicksToDeadline = pStream->cDmaPeriodTicks;
 
                             DEVAC97_UNLOCK(pDevIns, pThis);
@@ -2801 +2855 @@
                              *        earlier if we push it, just to reduce the lag...  For HDA we do a
                              *        DMA run immediately after the stream is enabled. */
-                            /** @todo take down the start time here.  */
-                            int rc2 = PDMDevHlpTimerSetRelative(pDevIns, pStream->hTimer, cTicksToDeadline, NULL /*pu64Now*/);
+                            int rc2 = PDMDevHlpTimerSetRelative(pDevIns, pStream->hTimer, cTicksToDeadline, &pStream->uArmedTs);
                             AssertRC(rc2);
 
@@ -3754 +3807 @@
             /* Re-arm the timer for this stream. */
             /** @todo r=aeichner This causes a VM hang upon saved state resume when NetBSD is used as a guest
-             * Stopping the timer if cTransferTicks is 0 is a workaround but needs further investigation,
+             * Stopping the timer if cDmaPeriodTicks is 0 is a workaround but needs further investigation,
              * see @bugref{9759} for more information. */
-            if (pStreamCC->State.cTransferTicks)
-                ichac97R3TimerSet(pDevIns, pStream, pStreamCC->State.cTransferTicks);
+            if (pStream->cDmaPeriodTicks)
+                ichac97R3TimerSet(pDevIns, pStream, pStream->cDmaPeriodTicks);
             else
                 PDMDevHlpTimerStop(pDevIns, pStream->hTimer);
@@ -3887 +3940 @@
     pHlp->pfnPrintf(pHlp, "  offWrite           %#RX64\n", pStreamR3->State.offWrite);
     pHlp->pfnPrintf(pHlp, "  uTimerHz           %RU16\n", pStreamR3->State.uTimerHz);
-    pHlp->pfnPrintf(pHlp, "  cTransferTicks     %RU64\n", pStreamR3->State.cTransferTicks);
-    pHlp->pfnPrintf(pHlp, "  cbTransferChunk    %#RX32\n", pStreamR3->State.cbTransferChunk);
+    pHlp->pfnPrintf(pHlp, "  cDmaPeriodTicks    %RU64\n", pStream->cDmaPeriodTicks);
+    pHlp->pfnPrintf(pHlp, "  cbDmaPeriod        %#RX32\n", pStream->cbDmaPeriod);
 }
 
@@ -4533 +4586 @@
     for (unsigned idxStream = 0; idxStream < RT_ELEMENTS(pThis->aStreams); idxStream++)
     {
-        PDMDevHlpSTAMRegisterF(pDevIns, &pThisCC->aStreams[idxStream].State.cbTransferChunk, STAMTYPE_U32, STAMVISIBILITY_USED, STAMUNIT_BYTES,
+        PDMDevHlpSTAMRegisterF(pDevIns, &pThis->aStreams[idxStream].cbDmaPeriod, STAMTYPE_U32, STAMVISIBILITY_USED, STAMUNIT_BYTES,
                                "Bytes to transfer in the current DMA period.",  "Stream%u/cbTransferChunk", idxStream);
         PDMDevHlpSTAMRegisterF(pDevIns, &pThis->aStreams[idxStream].Regs.cr, STAMTYPE_X8, STAMVISIBILITY_ALWAYS, STAMUNIT_NONE,