Changeset 87319 in vbox

Timestamp:

Jan 20, 2021 10:44:22 AM (4 years ago)

Author:

vboxsync

Message:

Audio/HDA: Implemented FIFO watermark support. ticketoem2ref:36

Location:

trunk/src/VBox/Devices/Audio

Files:

• DevHDA.cpp (modified) (3 diffs)
• DevHDACommon.cpp (modified) (3 diffs)
• HDAStream.cpp (modified) (10 diffs)
• HDAStream.h (modified) (2 diffs)

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

@@ -536 +536 @@
 
 
-
-
-/**
- * Retrieves the number of bytes of a FIFOW register.
- *
- * @return Number of bytes of a given FIFOW register.
- */
-DECLINLINE(uint8_t) hdaSDFIFOWToBytes(uint32_t u32RegFIFOW)
-{
-    uint32_t cb;
-    switch (u32RegFIFOW)
-    {
-        case HDA_SDFIFOW_8B:  cb = 8;  break;
-        case HDA_SDFIFOW_16B: cb = 16; break;
-        case HDA_SDFIFOW_32B: cb = 32; break;
-        default:              cb = 0;  break;
-    }
-
-    Assert(RT_IS_POWER_OF_TWO(cb));
-    return cb;
-}
-
 #ifdef IN_RING3
 /**
@@ -1563 +1541 @@
     }
 
-    uint32_t u32FIFOW = 0;
+    uint32_t u16FIFOW = 0;
     switch (u32Value)
     {
@@ -1569 +1547 @@
         case HDA_SDFIFOW_16B:
         case HDA_SDFIFOW_32B:
-            u32FIFOW = u32Value;
+            u16FIFOW = u32Value;
             break;
         default:
             ASSERT_GUEST_LOGREL_MSG_FAILED(("Guest tried writing unsupported FIFOW (0x%zx) to stream #%RU8, defaulting to 32 bytes\n",
                                             u32Value, idxStream));
-            u32FIFOW = HDA_SDFIFOW_32B;
+            u16FIFOW = HDA_SDFIFOW_32B;
             break;
     }
 
-    pThis->aStreams[idxStream].u16FIFOW = hdaSDFIFOWToBytes(u32FIFOW);
-    LogFunc(("[SD%zu] Updating FIFOW to %u bytes\n", idxStream, pThis->aStreams[idxStream].u16FIFOW));
-    return hdaRegWriteU16(pDevIns, pThis, iReg, u32FIFOW);
+    pThis->aStreams[idxStream].u8FIFOW = hdaSDFIFOWToBytes(u16FIFOW);
+    LogFunc(("[SD%zu] Updating FIFOW to %RU8 bytes\n", idxStream, pThis->aStreams[idxStream].u8FIFOW));
+    return hdaRegWriteU16(pDevIns, pThis, iReg, u16FIFOW);
 }
 

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

@@ -79 +79 @@
         pThis->u8IRQL = 0;
     }
+}
+
+/**
+ * Retrieves the number of bytes of a FIFOW register.
+ *
+ * @return Number of bytes of a given FIFOW register.
+ * @param  u16RegFIFOS         FIFOW register to convert.
+ */
+uint8_t hdaSDFIFOWToBytes(uint16_t u16RegFIFOW)
+{
+    uint32_t cb;
+    switch (u16RegFIFOW)
+    {
+        case HDA_SDFIFOW_8B:  cb = 8;  break;
+        case HDA_SDFIFOW_16B: cb = 16; break;
+        case HDA_SDFIFOW_32B: cb = 32; break;
+        default:
+            AssertFailedStmt(cb = 32); /* Paranoia. */
+            break;
+    }
+
+    Assert(RT_IS_POWER_OF_TWO(cb));
+    return cb;
 }
 
@@ -334 +357 @@
     while (cbLeft)
     {
-        uint32_t cbChunk = RT_MIN(cbLeft, pStreamShared->u16FIFOS);
+        uint32_t cbChunk = RT_MIN(cbLeft, pStreamShared->u8FIFOS);
 
         rc = PDMDevHlpPhysRead(pDevIns, GCPhysChunk, (uint8_t *)pvBuf + cbReadTotal, cbChunk);
@@ -409 +432 @@
     while (cbLeft)
     {
-        uint32_t cbChunk = RT_MIN(cbLeft, pStreamShared->u16FIFOS);
+        uint32_t cbChunk = RT_MIN(cbLeft, pStreamShared->u8FIFOS);
 
         /* Sanity checks. */

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

@@ -242 +242 @@
     const uint16_t u16LVI     = HDA_STREAM_REG(pThis, LVI, uSD);
     const uint32_t u32CBL     = HDA_STREAM_REG(pThis, CBL, uSD);
-    const uint16_t u16FIFOS   = HDA_STREAM_REG(pThis, FIFOS, uSD) + 1;
+    const uint8_t  u8FIFOS    = HDA_STREAM_REG(pThis, FIFOS, uSD) + 1;
+          uint8_t  u8FIFOW    = hdaSDFIFOWToBytes(HDA_STREAM_REG(pThis, FIFOW, uSD));
     const uint16_t u16FMT     = HDA_STREAM_REG(pThis, FMT, uSD);
 
@@ -250 +251 @@
         || !u16LVI
         || !u32CBL
-        || !u16FIFOS
+        || !u8FIFOS
+        || !u8FIFOW
         || !u16FMT)
     {
@@ -326 +328 @@
         && u16LVI     == pStreamShared->u16LVI
         && u32CBL     == pStreamShared->u32CBL
-        && u16FIFOS   == pStreamShared->u16FIFOS
+        && u8FIFOS    == pStreamShared->u8FIFOS
+        && u8FIFOW    == pStreamShared->u8FIFOW
         && u16FMT     == pStreamShared->u16FMT)
     {
@@ -332 +335 @@
         return VINF_NO_CHANGE;
     }
+
+    /* Make sure the guest behaves regarding the stream's FIFO. */
+    ASSERT_GUEST_LOGREL_MSG_STMT(u8FIFOW <= u8FIFOS,
+                                 ("Guest tried setting a bigger FIFOW (%RU8) than FIFOS (%RU8), limiting\n", u8FIFOW, u8FIFOS),
+                                 u8FIFOW = u8FIFOS /* ASSUMES that u8FIFOS has been validated. */);
 
     pStreamShared->u8SD       = uSD;
@@ -339 +347 @@
     pStreamShared->u16LVI     = u16LVI;
     pStreamShared->u32CBL     = u32CBL;
-    pStreamShared->u16FIFOS   = u16FIFOS;
+    pStreamShared->u8FIFOS    = u8FIFOS;
+    pStreamShared->u8FIFOW    = u8FIFOW;
     pStreamShared->u16FMT     = u16FMT;
 
@@ -387 +396 @@
         pCfg->Device.cMsSchedulingHint = 1000 /* ms */ / pStreamShared->State.uTimerHz;
 
-    LogFunc(("[SD%RU8] DMA @ 0x%x (%RU32 bytes), LVI=%RU16, FIFOS=%RU16\n",
-             uSD, pStreamShared->u64BDLBase, pStreamShared->u32CBL, pStreamShared->u16LVI, pStreamShared->u16FIFOS));
+    LogFunc(("[SD%RU8] DMA @ 0x%x (%RU32 bytes), LVI=%RU16, FIFOS=%RU8\n",
+             uSD, pStreamShared->u64BDLBase, pStreamShared->u32CBL, pStreamShared->u16LVI, pStreamShared->u8FIFOS));
 
     if (RT_SUCCESS(rc))
@@ -987 +996 @@
     Assert(pStreamShared->u64BDLBase);
     Assert(pStreamShared->u32CBL);
-    Assert(pStreamShared->u16FIFOS);
+    Assert(pStreamShared->u8FIFOS);
 
     /* State sanity checks. */
@@ -1021 +1030 @@
     {
         /* Limit the chunk to the stream's FIFO size and what's left to process. */
-        uint32_t cbChunk = RT_MIN(cbLeft, pStreamShared->u16FIFOS);
+        uint32_t cbChunk = RT_MIN(cbLeft, pStreamShared->u8FIFOS);
 
         /* Limit the chunk to the remaining data of the current BDLE. */
@@ -1238 +1247 @@
         if (cbDMA)
         {
+            const size_t cbCircBufUsed = RTCircBufUsed(pCircBuf);
+
+            Log3Func(("[SD%RU8] cbDMA=%RU32, cbUsed=%zu, uFIFOW=%RU8, uFIFOS=%RU8\n",
+                      uSD, cbDMA, cbCircBufUsed, pStreamShared->u8FIFOW, pStreamShared->u8FIFOS));
+
             /* We always increment the position of DMA buffer counter because we're always reading
              * into an intermediate DMA buffer. */
@@ -1253 +1267 @@
             }
 
-            /*
-             * Update the stream's current position.
-             * Do this as accurate and close to the actual data transfer as possible.
-             * All guetsts rely on this, depending on the mechanism they use (LPIB register or DMA counters).
-             */
-            uint32_t cbStreamPos = hdaR3StreamGetPosition(pThis, pStreamShared);
-            if (cbStreamPos == pStreamShared->u32CBL)
-                cbStreamPos = 0;
-
-            hdaR3StreamSetPosition(pStreamShared, pDevIns, pThis, cbStreamPos + cbDMA);
+            /* Only set the new DMA position if we at least reached the stream's FIFO watermark.
+             * This by default is 32 bytes. */
+            if (cbCircBufUsed >= pStreamShared->u8FIFOW)
+            {
+                /*
+                 * Update the stream's current position.
+                 * Do this as accurate and close to the actual data transfer as possible.
+                 * All guetsts rely on this, depending on the mechanism they use (LPIB register or DMA counters).
+                 */
+                uint32_t cbStreamPos = hdaR3StreamGetPosition(pThis, pStreamShared);
+                if (cbStreamPos == pStreamShared->u32CBL)
+                    cbStreamPos = 0;
+
+                hdaR3StreamSetPosition(pStreamShared, pDevIns, pThis, cbStreamPos + cbDMA);
+            }
         }
 

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

@@ -203 +203 @@
      *  Will be updated in hdaR3StreamInit(). */
     uint16_t                    u16FMT;
-    /** FIFO Size (FIFOS).
+    /** FIFO Size (checked + translated in bytes, FIFOS).
      *  Maximum number of bytes that may have been DMA'd into
      *  memory but not yet transmitted on the link.
      *
      *  Will be updated in hdaR3StreamInit(). */
-    uint16_t                    u16FIFOS;
-    /** FIFO Watermark. */
-    uint16_t                    u16FIFOW;
+    uint8_t                     u8FIFOS;
+    /** FIFO Watermark (checked + translated in bytes, FIFOW). */
+    uint8_t                     u8FIFOW;
+    uint8_t                     abPadding1[2];
     /** FIFO scratch buffer, to avoid intermediate (re-)allocations. */
     uint8_t                     abFIFO[HDA_FIFO_MAX + 1];
@@ -216 +217 @@
      *  Will be updated in hdaR3StreamInit(). */
     uint16_t                    u16LVI;
-    uint16_t                    au16Padding1[2];
     /** The timer for pumping data thru the attached LUN drivers. */
     TMTIMERHANDLE               hTimer;