Changeset 76018 in vbox for trunk/src/VBox/Devices

Timestamp:

Dec 6, 2018 3:14:02 PM (6 years ago)

Author:

vboxsync

Message:

Audio/HDA: Cleaned up setting the stream registers at one defined place (hdaR3StreamInit()).

Location:

trunk/src/VBox/Devices/Audio

Files:

• DevHDA.cpp (modified) (7 diffs)
• HDAStream.cpp (modified) (5 diffs)
• HDAStream.h (modified) (1 diff)

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

@@ -1264 +1264 @@
     DEVHDA_LOCK_RETURN(pThis, VINF_IOM_R3_MMIO_WRITE);
 
-    PHDASTREAM pStream = hdaGetStreamFromSD(pThis, HDA_SD_NUM_FROM_REG(pThis, CBL, iReg));
-    if (pStream)
-    {
-        pStream->u32CBL = u32Value;
-        LogFlowFunc(("[SD%RU8] CBL=%RU32\n", pStream->u8SD, u32Value));
-    }
-    else
-        LogFunc(("[SD%RU8] Warning: Changing SDCBL on non-attached stream (0x%x)\n",
-                 HDA_SD_NUM_FROM_REG(pThis, CTL, iReg), u32Value));
-
     int rc = hdaRegWriteU32(pThis, iReg, u32Value);
     AssertRCSuccess(rc);
@@ -1604 +1594 @@
     DEVHDA_LOCK_RETURN(pThis, VINF_IOM_R3_MMIO_WRITE);
 
-    if (HDA_REG_IND(pThis, iReg) == u32Value) /* Value already set? */
-    { /* nothing to do */ }
-    else
-    {
-        uint8_t    uSD     = HDA_SD_NUM_FROM_REG(pThis, LVI, iReg);
+#ifdef HDA_USE_DMA_ACCESS_HANDLER
+    uint8_t uSD = HDA_SD_NUM_FROM_REG(pThis, LVI, iReg);
+
+    if (hdaGetDirFromSD(uSD) == PDMAUDIODIR_OUT)
+    {
         PHDASTREAM pStream = hdaGetStreamFromSD(pThis, uSD);
-        if (pStream)
-        {
-            /** @todo Validate LVI. */
-            pStream->u16LVI = u32Value;
-            LogFunc(("[SD%RU8] Updating LVI to %RU16\n", uSD, pStream->u16LVI));
-
-#ifdef HDA_USE_DMA_ACCESS_HANDLER
-            if (hdaGetDirFromSD(uSD) == PDMAUDIODIR_OUT)
-            {
-                /* Try registering the DMA handlers.
-                 * As we can't be sure in which order LVI + BDL base are set, try registering in both routines. */
-                if (hdaR3StreamRegisterDMAHandlers(pThis, pStream))
-                    LogFunc(("[SD%RU8] DMA logging enabled\n", pStream->u8SD));
-            }
+
+        /* Try registering the DMA handlers.
+         * As we can't be sure in which order LVI + BDL base are set, try registering in both routines. */
+        if (   pStream
+            && hdaR3StreamRegisterDMAHandlers(pThis, pStream))
+        {
+            LogFunc(("[SD%RU8] DMA logging enabled\n", pStream->u8SD));
+        }
+    }
 #endif
-        }
-        else
-            AssertMsgFailed(("[SD%RU8] Warning: Changing SDLVI on non-attached stream (0x%x)\n", uSD, u32Value));
-
-        int rc2 = hdaRegWriteU16(pThis, iReg, u32Value);
-        AssertRC(rc2);
-    }
+
+    int rc2 = hdaRegWriteU16(pThis, iReg, u32Value);
+    AssertRC(rc2);
 
     DEVHDA_UNLOCK(pThis);
@@ -1711 +1692 @@
     }
 
-    PHDASTREAM pStream = hdaGetStreamFromSD(pThis, uSD);
-    if (!pStream)
-    {
-        AssertMsgFailed(("[SD%RU8] Warning: Changing FIFOS on non-attached stream (0x%x)\n", uSD, u32Value));
-
-        int rc = hdaRegWriteU16(pThis, iReg, u32Value);
-        DEVHDA_UNLOCK(pThis);
-        return rc;
-    }
-
-    uint32_t u32FIFOS = 0;
+    uint32_t u32FIFOS;
 
     switch(u32Value)
@@ -1741 +1712 @@
     }
 
-    if (u32FIFOS)
-    {
-        pStream->u16FIFOS = u32FIFOS + 1;
-        LogFunc(("[SD%RU8] Updating FIFOS to %RU32 bytes\n", uSD, pStream->u16FIFOS));
-
-        int rc2 = hdaRegWriteU16(pThis, iReg, u32FIFOS);
-        AssertRC(rc2);
-    }
+    int rc2 = hdaRegWriteU16(pThis, iReg, u32FIFOS);
+    AssertRC(rc2);
 
     DEVHDA_UNLOCK(pThis);
@@ -2036 +2001 @@
 static int hdaRegWriteSDFMT(PHDASTATE pThis, uint32_t iReg, uint32_t u32Value)
 {
-    DEVHDA_LOCK(pThis);
-
-# ifdef LOG_ENABLED
-    if (!hdaGetStreamFromSD(pThis, HDA_SD_NUM_FROM_REG(pThis, FMT, iReg)))
-        LogFunc(("[SD%RU8] Warning: Changing SDFMT on non-attached stream (0x%x)\n",
-                 HDA_SD_NUM_FROM_REG(pThis, FMT, iReg), u32Value));
-# endif
-
+    DEVHDA_LOCK_RETURN(pThis, VINF_IOM_R3_MMIO_WRITE);
 
     /* Write the wanted stream format into the register in any case.
@@ -2056 +2014 @@
 
     DEVHDA_UNLOCK(pThis);
-    return VINF_SUCCESS; /* Never return failure. */
+    return VINF_SUCCESS; /* Always return success to the MMIO handler. */
 }
 
@@ -2065 +2023 @@
     DEVHDA_LOCK(pThis);
 
+# ifdef HDA_USE_DMA_ACCESS_HANDLER
+    if (hdaGetDirFromSD(uSD) == PDMAUDIODIR_OUT)
+    {
+        PHDASTREAM pStream = hdaGetStreamFromSD(pThis, uSD);
+
+        /* Try registering the DMA handlers.
+         * As we can't be sure in which order LVI + BDL base are set, try registering in both routines. */
+        if (   pStream
+            && hdaR3StreamRegisterDMAHandlers(pThis, pStream))
+        {
+            LogFunc(("[SD%RU8] DMA logging enabled\n", pStream->u8SD));
+        }
+    }
+# else
+    RT_NOREF(uSD);
+# endif
+
     int rc2 = hdaRegWriteU32(pThis, iReg, u32Value);
     AssertRC(rc2);
-
-    PHDASTREAM pStream = hdaGetStreamFromSD(pThis, uSD);
-    if (!pStream)
-    {
-        DEVHDA_UNLOCK(pThis);
-        return VINF_SUCCESS;
-    }
-
-    /* Update BDL base. */
-    pStream->u64BDLBase = RT_MAKE_U64(HDA_STREAM_REG(pThis, BDPL, uSD),
-                                      HDA_STREAM_REG(pThis, BDPU, uSD));
-
-# ifdef HDA_USE_DMA_ACCESS_HANDLER
-    if (hdaGetDirFromSD(uSD) == PDMAUDIODIR_OUT)
-    {
-        /* Try registering the DMA handlers.
-         * As we can't be sure in which order LVI + BDL base are set, try registering in both routines. */
-        if (hdaR3StreamRegisterDMAHandlers(pThis, pStream))
-            LogFunc(("[SD%RU8] DMA logging enabled\n", pStream->u8SD));
-    }
-# endif
-
-    LogFlowFunc(("[SD%RU8] BDLBase=0x%x\n", pStream->u8SD, pStream->u64BDLBase));
 
     DEVHDA_UNLOCK(pThis);

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

@@ -203 +203 @@
     const uint32_t u32CBL     = HDA_STREAM_REG(pThis, CBL, uSD);
     const uint16_t u16FIFOS   = HDA_STREAM_REG(pThis, FIFOS, uSD) + 1;
-    const uint32_t u32FMT     = HDA_STREAM_REG(pThis, FMT, uSD);
+    const uint16_t u16FMT     = HDA_STREAM_REG(pThis, FMT, uSD);
 
     /* Is the bare minimum set of registers configured for the stream?
@@ -211 +211 @@
         || !u32CBL
         || !u16FIFOS
-        || !u32FMT)
+        || !u16FMT)
     {
         LogFunc(("[SD%RU8] Registers not set up yet, skipping (re-)initialization\n", uSD));
@@ -218 +218 @@
 
     PDMAUDIOPCMPROPS Props;
-    int rc = hdaR3SDFMTToPCMProps(u32FMT, &Props);
+    int rc = hdaR3SDFMTToPCMProps(u16FMT, &Props);
     if (RT_FAILURE(rc))
     {
@@ -262 +262 @@
         && u32CBL     == pStream->u32CBL
         && u16FIFOS   == pStream->u16FIFOS
-        && DrvAudioHlpPCMPropsAreEqual(&Props, &pStream->State.Cfg.Props))
+        && u16FMT     == pStream->u16FMT)
     {
         LogFunc(("[SD%RU8] No format change, skipping (re-)initialization\n", uSD));
@@ -269 +269 @@
 
     pStream->u8SD       = uSD;
+
+    /* Update all register copies so that we later know that something has changed. */
     pStream->u64BDLBase = u64BDLBase;
     pStream->u16LVI     = u16LVI;
     pStream->u32CBL     = u32CBL;
     pStream->u16FIFOS   = u16FIFOS;
+    pStream->u16FMT     = u16FMT;
 
     PPDMAUDIOSTREAMCFG pCfg = &pStream->State.Cfg;

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

@@ -201 +201 @@
     uint8_t                  u8Channel;
     uint8_t                  Padding0[6];
-    /** DMA base address (SDnBDPU - SDnBDPL). */
+    /** DMA base address (SDnBDPU - SDnBDPL).
+     *  Will be updated in hdaR3StreamInit(). */
     uint64_t                 u64BDLBase;
     /** Cyclic Buffer Length (SDnCBL).
-     *  Represents the size of the ring buffer. */
+     *  Represents the size of the ring buffer.
+     *  Will be updated in hdaR3StreamInit(). */
     uint32_t                 u32CBL;
-    /** Format (SDnFMT). */
+    /** Format (SDnFMT).
+     *  Will be updated in hdaR3StreamInit(). */
     uint16_t                 u16FMT;
     /** FIFO Size (FIFOS).
      *  Maximum number of bytes that may have been DMA'd into
-     *  memory but not yet transmitted on the link. */
+     *  memory but not yet transmitted on the link.
+     *
+     *  Will be updated in hdaR3StreamInit(). */
     uint16_t                 u16FIFOS;
     /** FIFO Watermark. */
     uint16_t                 u16FIFOW;
-    /** Last Valid Index (SDnLVI). */
+    /** Last Valid Index (SDnLVI).
+     *  Will be updated in hdaR3StreamInit(). */
     uint16_t                 u16LVI;
     uint16_t                 Padding1[2];