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

Timestamp:

Jun 24, 2021 9:51:51 AM (3 years ago)

Author:

vboxsync

Message:

DevHda: Calculate cMsSchedulingHint based on the actual scheduling rather than some unused and obsolete timer frequency value. Removed the TimerHz config value. Cleanups. bugref:9890

Location:

trunk/src/VBox/Devices/Audio

Files:

• DevHda.cpp (modified) (2 diffs)
• DevHda.h (modified) (1 diff)
• DevHdaCommon.h (modified) (1 diff)
• DevHdaStream.cpp (modified) (5 diffs)
• DevHdaStream.h (modified) (3 diffs)

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

@@ -4753 +4753 @@
                                   "|BufSizeOutMs"
                                   "|InitialDelayMs"
-                                  "|TimerHz"
                                   "|DebugEnabled"
                                   "|DebugPathOut",
@@ -4777 +4776 @@
         return PDMDEV_SET_ERROR(pDevIns, VERR_OUT_OF_RANGE,
                                 N_("HDA configuration error: 'BufSizeOutMs' is out of bound, max 2000 ms"));
-
-    /** @todo uTimerHz isn't used for anything anymore. */
-    rc = pHlp->pfnCFGMQueryU16Def(pCfg, "TimerHz", &pThis->uTimerHz, HDA_TIMER_HZ_DEFAULT);
-    if (RT_FAILURE(rc))
-        return PDMDEV_SET_ERROR(pDevIns, rc,
-                                N_("HDA configuration error: failed to read Hertz (Hz) rate as unsigned integer"));
-    if (pThis->uTimerHz != HDA_TIMER_HZ_DEFAULT)
-        LogRel(("HDA: Using custom device timer rate (%RU16Hz)\n", pThis->uTimerHz));
 
     /** @devcfgm{hda,InitialDelayMs,uint16_t,0,256,12,ms}

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

@@ -129 +129 @@
     /** Current IRQ level. */
     uint8_t                 u8IRQL;
-    uint8_t                 abPadding[4];
-    /** The device timer Hz rate. Defaults to HDA_TIMER_HZ_DEFAULT. */
-    uint16_t                uTimerHz;
+    uint8_t                 abPadding[4 + 2];
     /** Number of milliseconds to delay kicking off the AIO when a stream starts.
      * @sa InitialDelayMs config value.  */

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

@@ -107 +107 @@
  */
 
-/** Default timer frequency (in Hz).
- *
- *  20 Hz now seems enough for most setups, even with load on the guest.
- *  Raising the rate will produce more I/O load on the guest and therefore
- *  also will affect the performance.
- */
-#define HDA_TIMER_HZ_DEFAULT        100
-
-/** Default position adjustment (in audio samples).
- *
- * For snd_hda_intel (Linux guests), the first BDL entry always is being used as
- * so-called BDL adjustment, which can vary, and is being used for chipsets which
- * misbehave and/or are incorrectly implemented.
- *
- * The BDL adjustment entry *always* has the IOC (Interrupt on Completion) bit set.
- *
- * For Intel Baytrail / Braswell implementations the BDL default adjustment is 32 frames, whereas
- * for ICH / PCH it's only one (1) frame.
- *
- * See default_bdl_pos_adj() and snd_hdac_stream_setup_periods() for more information.
- *
- * By default we apply some simple heuristics in hdaStreamInit().
- */
-#define HDA_POS_ADJUST_DEFAULT      0
-
-/** HDA's (fixed) audio frame size in bytes.
- *  We only support 16-bit stereo frames at the moment. */
-#define HDA_FRAME_SIZE_DEFAULT      4
-
 /** Offset of the SD0 register map. */
 #define HDA_REG_DESC_SD0_BASE       0x80

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

@@ -651 +651 @@
 
     /*
-     * Calculate the transfer Hz for use in the circular buffer calculation.
+     * Calculate the transfer Hz for use in the circular buffer calculation
+     * and the average period for the scheduling hint.
      */
     uint32_t cbMaxPeriod = 0;
     uint32_t cbMinPeriod = UINT32_MAX;
+    uint64_t cTicks      = 0;
     uint32_t cPeriods    = 0;
-    for (uint32_t i = 0; i < pStreamShared->State.cSchedule; i++)
+    for (uint32_t i = pStreamShared->State.cSchedulePrologue; i < pStreamShared->State.cSchedule; i++)
     {
         uint32_t cbPeriod = pStreamShared->State.aSchedule[i].cbPeriod;
@@ -662 +664 @@
         cbMinPeriod = RT_MIN(cbMinPeriod, cbPeriod);
         cPeriods   += pStreamShared->State.aSchedule[i].cLoops;
-    }
+        cTicks     += pStreamShared->State.aSchedule[i].cPeriodTicks * pStreamShared->State.aSchedule[i].cLoops;
+    }
+    /* Only consider the prologue in relation to the max period. */
+    for (uint32_t i = 0; i < pStreamShared->State.cSchedulePrologue; i++)
+        cbMaxPeriod = RT_MAX(cbMaxPeriod, pStreamShared->State.aSchedule[i].cbPeriod);
+
+    AssertLogRelReturn(cPeriods > 0, VERR_INTERNAL_ERROR_3);
     uint64_t const cbTransferPerSec  = RT_MAX(PDMAudioPropsFramesToBytes(&pCfg->Props, pCfg->Props.uHz),
                                               4096 /* zero div prevention: min is 6kHz, picked 4k in case I'm mistaken */);
@@ -675 +683 @@
 
     pStreamShared->State.cbAvgTransfer = (uint32_t)(cbTotal + cPeriods - 1) / cPeriods;
+
+    /* Calculate the average scheduling period length in nanoseconds. */
+    uint64_t const cTimerResolution = PDMDevHlpTimerGetFreq(pDevIns, pStreamShared->hTimer);
+    Assert(cTimerResolution <= UINT32_MAX);
+    uint64_t const cNsPerPeriod     = ASMMultU64ByU32DivByU32(cTicks / cPeriods, RT_NS_1SEC, cTimerResolution);
+    AssertLogRelReturn(cNsPerPeriod > 0, VERR_INTERNAL_ERROR_3);
 
     /* For input streams we must determin a pre-buffering requirement.
@@ -687 +701 @@
      * Set up data transfer stuff.
      */
-
-    /* Assign the global device rate to the stream I/O timer as default. */
-    pStreamShared->State.uTimerIoHz = pThis->uTimerHz;
-    ASSERT_GUEST_LOGREL_MSG_STMT(pStreamShared->State.uTimerIoHz,
-                                 ("I/O timer Hz rate for stream #%RU8 is invalid\n", uSD),
-                                 pStreamShared->State.uTimerIoHz = HDA_TIMER_HZ_DEFAULT);
-
     /* Set I/O scheduling hint for the backends. */
-    /** @todo r=bird: derive this from the schedule instead of using the
-     *        uTimerIoHz, as that's almost pure non-sense now. */
-    pCfg->Device.cMsSchedulingHint = RT_MS_1SEC / pStreamShared->State.uTimerIoHz;
+    pCfg->Device.cMsSchedulingHint = cNsPerPeriod > RT_NS_1MS ? (cNsPerPeriod + RT_NS_1MS / 2) / RT_NS_1MS : 1;
     LogRel2(("HDA: Stream #%RU8 set scheduling hint for the backends to %RU32ms\n", uSD, pCfg->Device.cMsSchedulingHint));
-
 
     /* Make sure to also update the stream's DMA counter (based on its current LPIB value). */
     /** @todo r=bird: We use LPIB as-is here, so if it's not zero we have to
-     *        locate the right place in the schedule and whatnot... */
+     * locate the right place in the schedule and whatnot...
+     *
+     * This is a similar scenario as when loading state saved, btw.
+     */
     if (HDA_STREAM_REG(pThis, LPIB, uSD) != 0)
         LogRel2(("HDA: Warning! Stream #%RU8 is set up with LPIB=%#RX32 instead of zero!\n", uSD, HDA_STREAM_REG(pThis, LPIB, uSD)));
@@ -755 +762 @@
      *       channels we don't support / need to save space.
      */
-    uint32_t msCircBuf = RT_MS_1SEC * 6 / RT_MIN(uTransferHz, pStreamShared->State.uTimerIoHz);
-    msCircBuf = RT_MAX(msCircBuf, pThis->msInitialDelay + RT_MS_1SEC * 6 / uTransferHz);
-
-    uint32_t cbCircBuf = PDMAudioPropsMilliToBytes(&pCfg->Props, msCircBuf);
+    uint32_t cbCircBuf = PDMAudioPropsMilliToBytes(&pCfg->Props, pThis->msInitialDelay + RT_MS_1SEC * 6 / uTransferHz);
     LogRel2(("HDA: Stream #%RU8 default ring buffer size is %RU32 bytes / %RU64 ms\n",
              uSD, cbCircBuf, PDMAudioPropsBytesToMilli(&pCfg->Props, cbCircBuf)));

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

@@ -96 +96 @@
     /** Flag indicating if the stream is in running state or not. */
     volatile bool           fRunning;
-    /** The stream's I/O timer Hz rate. */
-    uint16_t                uTimerIoHz;
     /** How many interrupts are pending due to
      *  BDLE interrupt-on-completion (IOC) bits set. */
     uint8_t                 cTransferPendingInterrupts;
-    /** Unused, padding. */
-    uint8_t                 abPadding1[2];
     /** Input streams only: Set when we switch from feeding the guest silence and
      *  commits to proving actual audio input bytes. */
@@ -108 +104 @@
     /** Input streams only: The number of bytes we need to prebuffer. */
     uint32_t                cbInputPreBuffer;
-    uint32_t                u32Padding2;
     /** Timestamp (absolute, in timer ticks) of the last DMA data transfer.
      * @note This is used for wall clock (WALCLK) calculations.  */
@@ -159 +154 @@
     /** Current loop number within the current scheduling step.  */
     uint32_t                idxScheduleLoop;
+
+    /** Aligning the next members on 16 bytes. */
+    uint64_t                u64Padding;
 
     /** Buffer descriptors and additional timer scheduling state.