Be (very) careful about how to add more TX DMA work.

The list-based DMA engine has the following behaviour:

* When the DMA engine is in the init state, you can write the first
  descriptor address to the QCU TxDP register and it will work.

* Then when it hits the end of the list (ie, it either hits a NULL
  link pointer, OR it hits a descriptor with VEOL set) the QCU
  stops, and the TxDP points to the last descriptor that was transmitted.

* Then when you want to transmit a new frame, you can then either:
  + write the head of the new list into TxDP, or
  + you write the head of the new list into the link pointer of the
    last completed descriptor (ie, where TxDP points), then kick
    TxE to restart transmission on that QCU>

* The hardware then will re-read the descriptor to pick up the link
  pointer and then jump to that.

Now, the quirks:

* If you write a TxDP when there's been no previous TxDP (ie, it's 0),
  it works.

* If you write a TxDP in any other instance, the TxDP write may actually
  fail.  Thus, when you start transmission, it will re-read the last
  transmitted descriptor to get the link pointer, NOT just start a new
  transmission.

So the correct thing to do here is:

* ALWAYS use the holding descriptor (ie, the last transmitted descriptor
  that we've kept safe) and use the link pointer in _THAT_ to transmit
  the next frame.

* NEVER write to the TxDP after you've done the initial write.

* .. also, don't do this whilst you're also resetting the NIC.

With this in mind, the following patch does basically the above.

* Since this encapsulates Sam's issues with the QCU behaviour w/ TDMA,
  kill the TDMA special case and replace it with the above.

* Add a new TXQ flag - PUTRUNNING - which indicates that we've started
  DMA.

* Clear that flag when DMA has been shutdown.

* Ensure that we're not restarting DMA with PUTRUNNING enabled.

* Fix the link pointer logic during TXQ drain - we should always ensure
  the link pointer does point to something if there's a list of frames.
  Having it be NULL as an indication that DMA has finished or during
  a reset causes trouble.

Now, given all of this, i want to nuke axq_link from orbit.  There's now HAL
methods to get and set the link pointer of a descriptor, so what we
should do instead is to update the right link pointer.

* If there's a holding descriptor and an empty TXQ list, set the
  link pointer of said holding descriptor to the new frame.

* If there's a non-empty TXQ list, set the link pointer of the
  last descriptor in the list to the new frame.

* Nuke axq_link from orbit.

Note:

* The AR9380 doesn't need this.  FIFO TX writes are atomic.  As long as
  we don't append to a list of frames that we've already passed to the
  hardware, all of the above doesn't apply.  The holding descriptor stuff
  is still needed to ensure the hardware can re-read a completed
  descriptor to move onto the next one, but we restart DMA by pushing in
  a new FIFO entry into the TX QCU.  That doesn't require any real
  gymnastics.

Tested:

* AR5210, AR5211, AR5212, AR5416, AR9380 - STA mode.
This commit is contained in:
Adrian Chadd 2013-05-18 18:27:53 +00:00
parent 9c91c227c7
commit 9be82a4209
Notes: svn2git 2020-12-20 02:59:44 +00:00
svn path=/head/; revision=250783
5 changed files with 198 additions and 152 deletions

View File

@ -4620,6 +4620,8 @@ ath_tx_stopdma(struct ath_softc *sc, struct ath_txq *txq)
{
struct ath_hal *ah = sc->sc_ah;
ATH_TXQ_LOCK_ASSERT(txq);
DPRINTF(sc, ATH_DEBUG_RESET,
"%s: tx queue [%u] %p, active=%d, hwpending=%d, flags 0x%08x, "
"link %p, holdingbf=%p\n",
@ -4633,6 +4635,8 @@ ath_tx_stopdma(struct ath_softc *sc, struct ath_txq *txq)
txq->axq_holdingbf);
(void) ath_hal_stoptxdma(ah, txq->axq_qnum);
/* We've stopped TX DMA, so mark this as stopped. */
txq->axq_flags &= ~ATH_TXQ_PUTRUNNING;
#ifdef ATH_DEBUG
if ((sc->sc_debug & ATH_DEBUG_RESET)
@ -4658,17 +4662,25 @@ ath_stoptxdma(struct ath_softc *sc)
__func__, sc->sc_bhalq,
(caddr_t)(uintptr_t) ath_hal_gettxbuf(ah, sc->sc_bhalq),
NULL);
/* stop the beacon queue */
(void) ath_hal_stoptxdma(ah, sc->sc_bhalq);
for (i = 0; i < HAL_NUM_TX_QUEUES; i++)
if (ATH_TXQ_SETUP(sc, i))
/* Stop the data queues */
for (i = 0; i < HAL_NUM_TX_QUEUES; i++) {
if (ATH_TXQ_SETUP(sc, i)) {
ATH_TXQ_LOCK(&sc->sc_txq[i]);
ath_tx_stopdma(sc, &sc->sc_txq[i]);
ATH_TXQ_UNLOCK(&sc->sc_txq[i]);
}
}
}
return 1;
}
#ifdef ATH_DEBUG
static void
void
ath_tx_dump(struct ath_softc *sc, struct ath_txq *txq)
{
struct ath_hal *ah = sc->sc_ah;
@ -4702,6 +4714,7 @@ ath_legacy_tx_drain(struct ath_softc *sc, ATH_RESET_TYPE reset_type)
#endif
struct ifnet *ifp = sc->sc_ifp;
int i;
struct ath_buf *bf_last;
(void) ath_stoptxdma(sc);
@ -4727,10 +4740,20 @@ ath_legacy_tx_drain(struct ath_softc *sc, ATH_RESET_TYPE reset_type)
*/
ath_txq_freeholdingbuf(sc, &sc->sc_txq[i]);
/*
* Reset the link pointer to NULL; there's
* no frames to chain DMA to.
* Setup the link pointer to be the
* _last_ buffer/descriptor in the list.
* If there's nothing in the list, set it
* to NULL.
*/
sc->sc_txq[i].axq_link = NULL;
bf_last = ATH_TXQ_LAST(&sc->sc_txq[i],
axq_q_s);
if (bf_last != NULL) {
ath_hal_gettxdesclinkptr(ah,
bf_last->bf_lastds,
&sc->sc_txq[i].axq_link);
} else {
sc->sc_txq[i].axq_link = NULL;
}
ATH_TXQ_UNLOCK(&sc->sc_txq[i]);
} else
ath_tx_draintxq(sc, &sc->sc_txq[i]);

View File

@ -642,6 +642,7 @@ ath_beacon_cabq_start_edma(struct ath_softc *sc)
/* Push the first entry into the hardware */
ath_hal_puttxbuf(sc->sc_ah, cabq->axq_qnum, bf->bf_daddr);
cabq->axq_flags |= ATH_TXQ_PUTRUNNING;
/* NB: gated by beacon so safe to start here */
ath_hal_txstart(sc->sc_ah, cabq->axq_qnum);
@ -661,6 +662,7 @@ ath_beacon_cabq_start_legacy(struct ath_softc *sc)
/* Push the first entry into the hardware */
ath_hal_puttxbuf(sc->sc_ah, cabq->axq_qnum, bf->bf_daddr);
cabq->axq_flags |= ATH_TXQ_PUTRUNNING;
/* NB: gated by beacon so safe to start here */
ath_hal_txstart(sc->sc_ah, cabq->axq_qnum);
@ -736,6 +738,16 @@ ath_beacon_generate(struct ath_softc *sc, struct ieee80211vap *vap)
* frames from a different vap.
* XXX could be slow causing us to miss DBA
*/
/*
* XXX TODO: this doesn't stop CABQ DMA - it assumes
* that since we're about to transmit a beacon, we've
* already stopped transmitting on the CABQ. But this
* doesn't at all mean that the CABQ DMA QCU will
* accept a new TXDP! So what, should we do a DMA
* stop? What if it fails?
*
* More thought is required here.
*/
ath_tx_draintxq(sc, cabq);
}
}

View File

@ -125,6 +125,8 @@ extern void ath_tx_update_tim(struct ath_softc *sc,
extern void ath_start(struct ifnet *ifp);
extern void ath_start_task(void *arg, int npending);
extern void ath_tx_dump(struct ath_softc *sc, struct ath_txq *txq);
/*
* Kick the frame TX task.
*/

View File

@ -744,6 +744,7 @@ ath_tx_handoff_hw(struct ath_softc *sc, struct ath_txq *txq,
struct ath_buf *bf)
{
struct ath_hal *ah = sc->sc_ah;
struct ath_buf *bf_first;
/*
* Insert the frame on the outbound list and pass it on
@ -759,6 +760,13 @@ ath_tx_handoff_hw(struct ath_softc *sc, struct ath_txq *txq,
KASSERT(txq->axq_qnum != ATH_TXQ_SWQ,
("ath_tx_handoff_hw called for mcast queue"));
/*
* XXX racy, should hold the PCU lock when checking this,
* and also should ensure that the TX counter is >0!
*/
KASSERT((sc->sc_inreset_cnt == 0),
("%s: TX during reset?\n", __func__));
#if 0
/*
* This causes a LOR. Find out where the PCU lock is being
@ -776,161 +784,141 @@ ath_tx_handoff_hw(struct ath_softc *sc, struct ath_txq *txq,
__func__, txq->axq_qnum,
(caddr_t)bf->bf_daddr, bf->bf_desc,
txq->axq_depth);
/* XXX axq_link needs to be set and updated! */
ATH_TXQ_INSERT_TAIL(txq, bf, bf_list);
if (bf->bf_state.bfs_aggr)
txq->axq_aggr_depth++;
/*
* There's no need to update axq_link; the hardware
* is in reset and once the reset is complete, any
* non-empty queues will simply have DMA restarted.
*/
return;
}
ATH_PCU_UNLOCK(sc);
#endif
/* For now, so not to generate whitespace diffs */
if (1) {
ATH_TXQ_LOCK(txq);
#ifdef IEEE80211_SUPPORT_TDMA
int qbusy;
ATH_TXQ_LOCK(txq);
ATH_TXQ_INSERT_TAIL(txq, bf, bf_list);
qbusy = ath_hal_txqenabled(ah, txq->axq_qnum);
/*
* XXX TODO: if there's a holdingbf, then
* ATH_TXQ_PUTRUNNING should be clear.
*
* If there is a holdingbf and the list is empty,
* then axq_link should be pointing to the holdingbf.
*
* Otherwise it should point to the last descriptor
* in the last ath_buf.
*
* In any case, we should really ensure that we
* update the previous descriptor link pointer to
* this descriptor, regardless of all of the above state.
*
* For now this is captured by having axq_link point
* to either the holdingbf (if the TXQ list is empty)
* or the end of the list (if the TXQ list isn't empty.)
* I'd rather just kill axq_link here and do it as above.
*/
ATH_KTR(sc, ATH_KTR_TX, 4,
"ath_tx_handoff: txq=%u, add bf=%p, qbusy=%d, depth=%d",
txq->axq_qnum, bf, qbusy, txq->axq_depth);
if (txq->axq_link == NULL) {
/*
* Be careful writing the address to TXDP. If
* the tx q is enabled then this write will be
* ignored. Normally this is not an issue but
* when tdma is in use and the q is beacon gated
* this race can occur. If the q is busy then
* defer the work to later--either when another
* packet comes along or when we prepare a beacon
* frame at SWBA.
*/
if (!qbusy) {
ath_hal_puttxbuf(ah, txq->axq_qnum,
bf->bf_daddr);
txq->axq_flags &= ~ATH_TXQ_PUTPENDING;
DPRINTF(sc, ATH_DEBUG_XMIT,
"%s: TXDP[%u] = %p (%p) lastds=%p depth %d\n",
__func__, txq->axq_qnum,
(caddr_t)bf->bf_daddr, bf->bf_desc,
bf->bf_lastds,
txq->axq_depth);
ATH_KTR(sc, ATH_KTR_TX, 5,
"ath_tx_handoff: TXDP[%u] = %p (%p) "
"lastds=%p depth %d",
txq->axq_qnum,
(caddr_t)bf->bf_daddr, bf->bf_desc,
bf->bf_lastds,
txq->axq_depth);
} else {
txq->axq_flags |= ATH_TXQ_PUTPENDING;
DPRINTF(sc, ATH_DEBUG_TDMA | ATH_DEBUG_XMIT,
"%s: Q%u busy, defer enable\n", __func__,
txq->axq_qnum);
ATH_KTR(sc, ATH_KTR_TX, 0, "defer enable");
}
} else {
*txq->axq_link = bf->bf_daddr;
DPRINTF(sc, ATH_DEBUG_XMIT,
"%s: link[%u](%p)=%p (%p) depth %d\n", __func__,
txq->axq_qnum, txq->axq_link,
(caddr_t)bf->bf_daddr, bf->bf_desc,
txq->axq_depth);
ATH_KTR(sc, ATH_KTR_TX, 5,
"ath_tx_handoff: link[%u](%p)=%p (%p) lastds=%p",
txq->axq_qnum, txq->axq_link,
(caddr_t)bf->bf_daddr, bf->bf_desc,
bf->bf_lastds);
/*
* Append the frame to the TX queue.
*/
ATH_TXQ_INSERT_TAIL(txq, bf, bf_list);
ATH_KTR(sc, ATH_KTR_TX, 3,
"ath_tx_handoff: non-tdma: txq=%u, add bf=%p "
"depth=%d",
txq->axq_qnum,
bf,
txq->axq_depth);
if ((txq->axq_flags & ATH_TXQ_PUTPENDING) && !qbusy) {
/*
* The q was busy when we previously tried
* to write the address of the first buffer
* in the chain. Since it's not busy now
* handle this chore. We are certain the
* buffer at the front is the right one since
* axq_link is NULL only when the buffer list
* is/was empty.
*/
ath_hal_puttxbuf(ah, txq->axq_qnum,
TAILQ_FIRST(&txq->axq_q)->bf_daddr);
txq->axq_flags &= ~ATH_TXQ_PUTPENDING;
DPRINTF(sc, ATH_DEBUG_TDMA | ATH_DEBUG_XMIT,
"%s: Q%u restarted\n", __func__,
txq->axq_qnum);
ATH_KTR(sc, ATH_KTR_TX, 4,
"ath_tx_handoff: txq[%d] restarted, bf=%p "
"daddr=%p ds=%p",
txq->axq_qnum,
bf,
(caddr_t)bf->bf_daddr,
bf->bf_desc);
}
}
#else
ATH_TXQ_INSERT_TAIL(txq, bf, bf_list);
ATH_KTR(sc, ATH_KTR_TX, 3,
"ath_tx_handoff: non-tdma: txq=%u, add bf=%p "
"depth=%d",
txq->axq_qnum,
bf,
/*
* If there's a link pointer, update it.
*
* XXX we should replace this with the above logic, just
* to kill axq_link with fire.
*/
if (txq->axq_link != NULL) {
*txq->axq_link = bf->bf_daddr;
DPRINTF(sc, ATH_DEBUG_XMIT,
"%s: link[%u](%p)=%p (%p) depth %d\n", __func__,
txq->axq_qnum, txq->axq_link,
(caddr_t)bf->bf_daddr, bf->bf_desc,
txq->axq_depth);
if (txq->axq_link == NULL) {
ath_hal_puttxbuf(ah, txq->axq_qnum, bf->bf_daddr);
DPRINTF(sc, ATH_DEBUG_XMIT,
"%s: TXDP[%u] = %p (%p) depth %d\n",
__func__, txq->axq_qnum,
(caddr_t)bf->bf_daddr, bf->bf_desc,
txq->axq_depth);
ATH_KTR(sc, ATH_KTR_TX, 5,
"ath_tx_handoff: non-tdma: TXDP[%u] = %p (%p) "
"lastds=%p depth %d",
txq->axq_qnum,
(caddr_t)bf->bf_daddr, bf->bf_desc,
bf->bf_lastds,
txq->axq_depth);
} else {
*txq->axq_link = bf->bf_daddr;
DPRINTF(sc, ATH_DEBUG_XMIT,
"%s: link[%u](%p)=%p (%p) depth %d\n", __func__,
txq->axq_qnum, txq->axq_link,
(caddr_t)bf->bf_daddr, bf->bf_desc,
txq->axq_depth);
ATH_KTR(sc, ATH_KTR_TX, 5,
"ath_tx_handoff: non-tdma: link[%u](%p)=%p (%p) "
"lastds=%d",
txq->axq_qnum, txq->axq_link,
(caddr_t)bf->bf_daddr, bf->bf_desc,
bf->bf_lastds);
}
#endif /* IEEE80211_SUPPORT_TDMA */
if (bf->bf_state.bfs_tx_queue != txq->axq_qnum) {
device_printf(sc->sc_dev,
"%s: bf=%p, bfs_tx_queue=%d, axq_qnum=%d\n",
__func__,
bf,
bf->bf_state.bfs_tx_queue,
txq->axq_qnum);
}
if (bf->bf_state.bfs_aggr)
txq->axq_aggr_depth++;
ath_hal_gettxdesclinkptr(ah, bf->bf_lastds, &txq->axq_link);
ath_hal_txstart(ah, txq->axq_qnum);
ATH_TXQ_UNLOCK(txq);
ATH_KTR(sc, ATH_KTR_TX, 1,
"ath_tx_handoff: txq=%u, txstart", txq->axq_qnum);
ATH_KTR(sc, ATH_KTR_TX, 5,
"ath_tx_handoff: non-tdma: link[%u](%p)=%p (%p) "
"lastds=%d",
txq->axq_qnum, txq->axq_link,
(caddr_t)bf->bf_daddr, bf->bf_desc,
bf->bf_lastds);
}
/*
* If we've not pushed anything into the hardware yet,
* push the head of the queue into the TxDP.
*
* Once we've started DMA, there's no guarantee that
* updating the TxDP with a new value will actually work.
* So we just don't do that - if we hit the end of the list,
* we keep that buffer around (the "holding buffer") and
* re-start DMA by updating the link pointer of _that_
* descriptor and then restart DMA.
*/
if (! (txq->axq_flags & ATH_TXQ_PUTRUNNING)) {
bf_first = TAILQ_FIRST(&txq->axq_q);
txq->axq_flags |= ATH_TXQ_PUTRUNNING;
ath_hal_puttxbuf(ah, txq->axq_qnum, bf_first->bf_daddr);
DPRINTF(sc, ATH_DEBUG_XMIT,
"%s: TXDP[%u] = %p (%p) depth %d\n",
__func__, txq->axq_qnum,
(caddr_t)bf_first->bf_daddr, bf_first->bf_desc,
txq->axq_depth);
ATH_KTR(sc, ATH_KTR_TX, 5,
"ath_tx_handoff: TXDP[%u] = %p (%p) "
"lastds=%p depth %d",
txq->axq_qnum,
(caddr_t)bf_first->bf_daddr, bf_first->bf_desc,
bf_first->bf_lastds,
txq->axq_depth);
}
/*
* Ensure that the bf TXQ matches this TXQ, so later
* checking and holding buffer manipulation is sane.
*/
if (bf->bf_state.bfs_tx_queue != txq->axq_qnum) {
device_printf(sc->sc_dev,
"%s: bf=%p, bfs_tx_queue=%d, axq_qnum=%d\n",
__func__,
bf,
bf->bf_state.bfs_tx_queue,
txq->axq_qnum);
}
/*
* Track aggregate queue depth.
*/
if (bf->bf_state.bfs_aggr)
txq->axq_aggr_depth++;
/*
* Update the link pointer.
*/
ath_hal_gettxdesclinkptr(ah, bf->bf_lastds, &txq->axq_link);
/*
* Start DMA.
*
* If we wrote a TxDP above, DMA will start from here.
*
* If DMA is running, it'll do nothing.
*
* If the DMA engine hit the end of the QCU list (ie LINK=NULL,
* or VEOL) then it stops at the last transmitted write.
* We then append a new frame by updating the link pointer
* in that descriptor and then kick TxE here; it will re-read
* that last descriptor and find the new descriptor to transmit.
*
* This is why we keep the holding descriptor around.
*/
ath_hal_txstart(ah, txq->axq_qnum);
ATH_TXQ_UNLOCK(txq);
ATH_KTR(sc, ATH_KTR_TX, 1,
"ath_tx_handoff: txq=%u, txstart", txq->axq_qnum);
}
/*
@ -945,8 +933,6 @@ ath_legacy_tx_dma_restart(struct ath_softc *sc, struct ath_txq *txq)
struct ath_buf *bf, *bf_last;
ATH_TXQ_LOCK_ASSERT(txq);
/* This is always going to be cleared, empty or not */
txq->axq_flags &= ~ATH_TXQ_PUTPENDING;
/* XXX make this ATH_TXQ_FIRST */
bf = TAILQ_FIRST(&txq->axq_q);
@ -955,7 +941,29 @@ ath_legacy_tx_dma_restart(struct ath_softc *sc, struct ath_txq *txq)
if (bf == NULL)
return;
ath_hal_puttxbuf(ah, txq->axq_qnum, bf->bf_daddr);
DPRINTF(sc, ATH_DEBUG_RESET,
"%s: Q%d: bf=%p, bf_last=%p, daddr=0x%08x\n",
__func__,
txq->axq_qnum,
bf,
bf_last,
(uint32_t) bf->bf_daddr);
if (sc->sc_debug & ATH_DEBUG_RESET)
ath_tx_dump(sc, txq);
/*
* This is called from a restart, so DMA is known to be
* completely stopped.
*/
KASSERT((!(txq->axq_flags & ATH_TXQ_PUTRUNNING)),
("%s: Q%d: called with PUTRUNNING=1\n",
__func__,
txq->axq_qnum));
ath_hal_puttxbuf(sc->sc_ah, txq->axq_qnum, bf->bf_daddr);
txq->axq_flags |= ATH_TXQ_PUTRUNNING;
ath_hal_gettxdesclinkptr(ah, bf_last->bf_lastds, &txq->axq_link);
ath_hal_txstart(ah, txq->axq_qnum);
}

View File

@ -327,7 +327,8 @@ struct ath_txq {
#define ATH_TXQ_SWQ (HAL_NUM_TX_QUEUES+1) /* qnum for s/w only queue */
u_int axq_ac; /* WME AC */
u_int axq_flags;
#define ATH_TXQ_PUTPENDING 0x0001 /* ath_hal_puttxbuf pending */
//#define ATH_TXQ_PUTPENDING 0x0001 /* ath_hal_puttxbuf pending */
#define ATH_TXQ_PUTRUNNING 0x0002 /* ath_hal_puttxbuf has been called */
u_int axq_depth; /* queue depth (stat only) */
u_int axq_aggr_depth; /* how many aggregates are queued */
u_int axq_intrcnt; /* interrupt count */