VirtualBox

Changeset 89771 in vbox


Ignore:
Timestamp:
Jun 18, 2021 12:58:07 AM (3 years ago)
Author:
vboxsync
Message:

DevIchAc97: Cleaned up inconsistencies in ichac97R3StreamTransfer - it is always called with the number of bytes we're supposed to transfer, including PICB considerations. Added missing ichac97R3StreamTransferUpdate call to the stream run code chwrite AC97_NABM_OFF_CR handler). bugref:9890

File:
1 edited

Legend:

Unmodified
Added
Removed

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

    r89770 r89771  
    988988 * @param   pStream             The AC'97 stream to update (shared).
    989989 * @param   pStreamCC           The AC'97 stream to update (ring-3).
    990  * @param   cbToProcessMax      Maximum of data (in bytes) to process.
     990 * @param   cbToProcess         The max amount of data to process (i.e.
     991 *                              put into / remove from the circular buffer).
     992 *                              Unless something is going seriously wrong, this
     993 *                              will always be transfer size for the current
     994 *                              period.   The current period will never be
     995 *                              larger than what can be stored in the current
     996 *                              buffer (i.e. what PICB indicates).
     997 * @param   tsNowNs             The current RTTimeNano() value.
    991998 * @param   fWriteSilence       Whether to write silence if this is an input
    992999 *                              stream (done while waiting for backend to get
    … …  
    9951002 */
    9961003static int ichac97R3StreamTransfer(PPDMDEVINS pDevIns, PAC97STATE pThis, PAC97STREAM pStream,
    997                                    PAC97STREAMR3 pStreamCC, uint32_t cbToProcessMax, bool fWriteSilence, bool fInput)
    998 {
    999     if (RT_LIKELY(cbToProcessMax > 0))
    1000         Assert(PDMAudioPropsIsSizeAligned(&pStreamCC->State.Cfg.Props, cbToProcessMax));
     1004                                   PAC97STREAMR3 pStreamCC, uint32_t cbToProcess, bool fWriteSilence, bool fInput)
     1005{
     1006    if (RT_LIKELY(cbToProcess > 0))
     1007        Assert(PDMAudioPropsIsSizeAligned(&pStreamCC->State.Cfg.Props, cbToProcess));
    10011008    else
    10021009        return VINF_SUCCESS;
    … …  
    10321039    else
    10331040    {
     1041        /** @todo Stop DMA timer when DCH is set. */
    10341042        if (pRegs->sr & AC97_SR_DCH)
    10351043        {
    … …  
    10511059    }
    10521060
    1053     /*
    1054      * How much to transfer.
    1055      */
    1056     /** @todo r=bird: This is wrong, isn't it?  We should just transfer
    1057      *        cbToProcessMax.  The caller does a RT_MIN(cbPeriod, buffer), so
    1058      *        we'll get the min number of bytes to transfer in.  It should equal
    1059      *        PICB, but it doesn't technically have to and the loop below seems to
    1060      *        think we can work with PICB=0, which this code here contratradicts.
    1061      *
    1062      *        When it was introduced in r108022, it might've made some sense...
    1063      *
    1064      *        Which is inconsistent.  I hate inconsistencies. */
    1065     uint32_t cbLeft           = pRegs->picb * PDMAudioPropsSampleSize(&pStreamCC->State.Cfg.Props);
    1066     cbLeft = RT_MIN(cbLeft, cbToProcessMax);
    1067     Log3Func(("[SD%RU8] cbToProcessMax=%#x cbLeft=%#x\n", pStream->u8SD, cbToProcessMax, cbLeft));
    1068 
    1069     /*
     1061    /*                                                           0x1ba*2 = 0x374 (884) 0x3c0
    10701062     * Transfer loop.
    10711063     */
     1064#ifdef LOG_ENABLED
    10721065    uint32_t   cbProcessedTotal = 0;
     1066#endif
    10731067    int        rc               = VINF_SUCCESS;
    10741068    PRTCIRCBUF pCircBuf         = pStreamCC->State.pCircBuf;
    10751069    AssertReturnStmt(pCircBuf, ichac97R3StreamUnlock(pStreamCC), VINF_SUCCESS);
    1076 
    1077     while (cbLeft > 0)
    1078     {
    1079         /** @todo as mentioned above, this is utter non-sense as it won't _ever_ be
    1080          *        taken, given that cbLeft == PICB * 2. */
    1081         if (!pRegs->picb) /* Get a new buffer descriptor, that is, the position is 0? */
    1082         {
    1083             Log3Func(("Fresh buffer descriptor %RU8 is empty, addr=%#x, len=%#x, skipping\n",
    1084                       pRegs->civ, pRegs->bd.addr, pRegs->bd.ctl_len));
    1085             if (pRegs->civ == pRegs->lvi)
    1086             {
    1087                 pRegs->sr |= AC97_SR_DCH; /** @todo r=andy Also set CELV? */
    1088                 pThis->bup_flag = 0;
    1089 
    1090                 rc = VINF_EOF;
    1091                 break;
    1092             }
    1093 
    1094             pRegs->sr &= ~AC97_SR_CELV;
    1095             if (ichac97R3StreamFetchNextBdle(pDevIns, pStream, pStreamCC))
    1096                 ichac97StreamUpdateSR(pDevIns, pThis, pStream, pRegs->sr | AC97_SR_BCIS);
    1097             continue;
    1098         }
    1099 
    1100         uint32_t cbChunk = cbLeft;
     1070    Assert((uint32_t)pRegs->picb * PDMAudioPropsSampleSize(&pStreamCC->State.Cfg.Props) >= cbToProcess);
     1071    Log3Func(("[SD%RU8] cbToProcess=%#x PICB=%#x/%#x\n",
     1072              pStream->u8SD, cbToProcess, pRegs->picb, pRegs->picb * PDMAudioPropsSampleSize(&pStreamCC->State.Cfg.Props)));
     1073
     1074    while (cbToProcess > 0)
     1075    {
     1076        uint32_t cbChunk = cbToProcess;
    11011077
    11021078        /*
    … …  
    11631139
    11641140        Assert(PDMAudioPropsIsSizeAligned(&pStreamCC->State.Cfg.Props, cbChunk));
    1165         Assert(cbChunk <= cbLeft);
    1166 
     1141        Assert(cbChunk <= cbToProcess);
     1142
     1143        /*
     1144         * Advance.
     1145         */
     1146        pRegs->picb          -= cbChunk / PDMAudioPropsSampleSize(&pStreamCC->State.Cfg.Props);
     1147        pRegs->bd.addr       += cbChunk;
     1148        cbToProcess          -= cbChunk;
     1149#ifdef LOG_ENABLED
    11671150        cbProcessedTotal     += cbChunk;
    1168         Assert(cbProcessedTotal <= cbToProcessMax);
    1169         cbLeft               -= cbChunk;
    1170         pRegs->picb          -= (cbChunk >> 1); /** @todo r=andy Assumes 16bit samples. */
    1171         pRegs->bd.addr       += cbChunk;
    1172 
    1173         LogFlowFunc(("[SD%RU8] cbChunk=%RU32, cbLeft=%RU32, cbTotal=%RU32, rc=%Rrc\n",
    1174                      pStream->u8SD, cbChunk, cbLeft, cbProcessedTotal, rc));
    1175 
    1176         /** @todo r=bird: again, given that cbLeft == PICB * 2, we could put this
    1177          *        outside the loop and save a lot of clutter (VINF_EOF). */
    1178         if (!pRegs->picb)
     1151#endif
     1152        LogFlowFunc(("[SD%RU8] cbChunk=%#x, cbToProcess=%#x, cbTotal=%#x picb=%#x\n",
     1153                     pStream->u8SD, cbChunk, cbToProcess, cbProcessedTotal, pRegs->picb));
     1154    }
     1155
     1156    /*
     1157     * Fetch a new buffer descriptor if we've exhausted the current one.
     1158     */
     1159    if (!pRegs->picb)
     1160    {
     1161        uint32_t fNewSr = pRegs->sr & ~AC97_SR_CELV;
     1162
     1163        if (pRegs->bd.ctl_len & AC97_BD_IOC)
     1164            fNewSr |= AC97_SR_BCIS;
     1165
     1166        if (pRegs->civ != pRegs->lvi)
     1167            fNewSr |= ichac97R3StreamFetchNextBdle(pDevIns, pStream, pStreamCC);
     1168        else
    11791169        {
    1180             uint32_t new_sr = pRegs->sr & ~AC97_SR_CELV;
    1181 
    1182             if (pRegs->bd.ctl_len & AC97_BD_IOC)
    1183             {
    1184                 new_sr |= AC97_SR_BCIS;
    1185             }
    1186 
    1187             if (pRegs->civ == pRegs->lvi)
    1188             {
    1189                 /* Did we run out of data? */
    1190                 LogFunc(("Underrun CIV (%RU8) == LVI (%RU8)\n", pRegs->civ, pRegs->lvi));
    1191 
    1192                 new_sr |= AC97_SR_LVBCI | AC97_SR_DCH | AC97_SR_CELV;
    1193                 pThis->bup_flag = (pRegs->bd.ctl_len & AC97_BD_BUP) ? BUP_LAST : 0;
    1194 
    1195                 rc = VINF_EOF;
    1196             }
    1197             else
    1198                 new_sr |= ichac97R3StreamFetchNextBdle(pDevIns, pStream, pStreamCC);
    1199 
    1200             ichac97StreamUpdateSR(pDevIns, pThis, pStream, new_sr);
     1170            LogFunc(("Underrun CIV (%RU8) == LVI (%RU8)\n", pRegs->civ, pRegs->lvi));
     1171            fNewSr |= AC97_SR_LVBCI | AC97_SR_DCH | AC97_SR_CELV;
     1172            pThis->bup_flag = (pRegs->bd.ctl_len & AC97_BD_BUP) ? BUP_LAST : 0;
     1173            /** @todo r=bird: The bup_flag isn't cleared anywhere else.  We should probably
     1174             *        do what the spec says, and keep writing zeros (silence).
     1175             *        Alternatively, we could hope the guest will pause the DMA engine
     1176             *        immediately after seeing this condition, in which case we should
     1177             *        stop the DMA timer from being re-armed. */
    12011178        }
    12021179
    1203         /* All data processed? */
    1204         if (rc == VINF_EOF)
    1205             break;
     1180        ichac97StreamUpdateSR(pDevIns, pThis, pStream, fNewSr);
    12061181    }
    12071182
    12081183    ichac97R3StreamUnlock(pStreamCC);
    1209 
    12101184    LogFlowFuncLeaveRC(rc);
    12111185    return rc;
    … …  
    28212795                             * create a lock order violation with the virtual sync time lock otherwise.
    28222796                             */
    2823                             /** @todo is ichac97R3StreamTransferUpdate called here? */
     2797                            ichac97R3StreamTransferUpdate(pDevIns, pStream, pStreamCC);
    28242798                            uint64_t const cTicksToDeadline = pStreamCC->State.cTransferTicks;
    28252799
Note: See TracChangeset for help on using the changeset viewer.

© 2024 Oracle Support Privacy / Do Not Sell My Info Terms of Use Trademark Policy Automated Access Etiquette