Merge in some fixes from the if_ath_tx branch.

* Close down some of the kickpcu races, where the interrupt handler
  can and will run concurrently with the taskqueue.
* Close down the TXQ active/completed race between the interrupt
  handler and the concurrently running tx completion taskqueue
  function.
* Add some tx and rx interrupt count tracking, for debugging.
* Fix the kickpcu logic in ath_rx_proc() to not simply drain and
  restart the TX queue - instead, assume the hardware isn't
  (too) confused and just restart RX DMA. This may break on
  previous chipsets, so if it does I'll add a HAL flag and
  conditionally handle this (ie, for broken chipsets, I'll
  just restore the "stop PCU / flush things / restart PCU"
  logic.)
* Misc stuff

Sponsored by:	Hobnob, Inc.

3 files changed, 125 insertions, 33 deletions

diff --git a/sys/dev/ath/if_ath.c b/sys/dev/ath/if_ath.c
index 2c6e84a..e2d6a84 100644
--- a/sys/dev/ath/if_ath.c
+++ b/sys/dev/ath/if_ath.c
@@ -1304,6 +1304,7 @@ ath_intr(void *arg)
 	struct ifnet *ifp = sc->sc_ifp;
 	struct ath_hal *ah = sc->sc_ah;
 	HAL_INT status = 0;
+	uint32_t txqs;
 
 	if (sc->sc_invalid) {
 		/*
@@ -1377,7 +1378,7 @@ ath_intr(void *arg)
 			}
 		}
 		if (status & HAL_INT_RXEOL) {
-			int imask = sc->sc_imask;
+			int imask;
 			/*
 			 * NB: the hardware should re-read the link when
 			 *     RXE bit is written, but it doesn't work at
@@ -1393,26 +1394,55 @@ ath_intr(void *arg)
 			 * by a call to ath_reset() somehow, the
 			 * interrupt mask will be correctly reprogrammed.
 			 */
+			ATH_LOCK(sc);
+			imask = sc->sc_imask;
 			imask &= ~(HAL_INT_RXEOL | HAL_INT_RXORN);
 			ath_hal_intrset(ah, imask);
 			/*
+			 * Only blank sc_rxlink if we've not yet kicked
+			 * the PCU.
+			 *
+			 * This isn't entirely correct - the correct solution
+			 * would be to have a PCU lock and engage that for
+			 * the duration of the PCU fiddling; which would include
+			 * running the RX process. Otherwise we could end up
+			 * messing up the RX descriptor chain and making the
+			 * RX desc list much shorter.
+			 */
+			if (! sc->sc_kickpcu)
+				sc->sc_rxlink = NULL;
+			sc->sc_kickpcu = 1;
+			ATH_UNLOCK(sc);
+			/*
 			 * Enqueue an RX proc, to handled whatever
 			 * is in the RX queue.
 			 * This will then kick the PCU.
 			 */
 			taskqueue_enqueue(sc->sc_tq, &sc->sc_rxtask);
-			sc->sc_rxlink = NULL;
-			sc->sc_kickpcu = 1;
 		}
 		if (status & HAL_INT_TXURN) {
 			sc->sc_stats.ast_txurn++;
 			/* bump tx trigger level */
 			ath_hal_updatetxtriglevel(ah, AH_TRUE);
 		}
-		if (status & HAL_INT_RX)
+		if (status & HAL_INT_RX) {
+			sc->sc_stats.ast_rx_intr++;
 			taskqueue_enqueue(sc->sc_tq, &sc->sc_rxtask);
-		if (status & HAL_INT_TX)
+		}
+		if (status & HAL_INT_TX) {
+			sc->sc_stats.ast_tx_intr++;
+			/*
+			 * Grab all the currently set bits in the HAL txq bitmap
+			 * and blank them. This is the only place we should be
+			 * doing this.
+			 */
+			ATH_LOCK(sc);
+			txqs = 0xffffffff;
+			ath_hal_gettxintrtxqs(sc->sc_ah, &txqs);
+			sc->sc_txq_active |= txqs;
+			ATH_UNLOCK(sc);
 			taskqueue_enqueue(sc->sc_tq, &sc->sc_txtask);
+		}
 		if (status & HAL_INT_BMISS) {
 			sc->sc_stats.ast_bmiss++;
 			taskqueue_enqueue(sc->sc_tq, &sc->sc_bmisstask);
@@ -1433,7 +1463,16 @@ ath_intr(void *arg)
 			 * clear whatever condition caused the interrupt.
 			 */
 			ath_hal_mibevent(ah, &sc->sc_halstats);
-			ath_hal_intrset(ah, sc->sc_imask);
+			/*
+			 * Don't reset the interrupt if we've just
+			 * kicked the PCU, or we may get a nested
+			 * RXEOL before the rxproc has had a chance
+			 * to run.
+			 */
+			ATH_LOCK(sc);
+			if (sc->sc_kickpcu == 0)
+				ath_hal_intrset(ah, sc->sc_imask);
+			ATH_UNLOCK(sc);
 		}
 		if (status & HAL_INT_RXORN) {
 			/* NB: hal marks HAL_INT_FATAL when RXORN is fatal */
@@ -1609,6 +1648,13 @@ ath_init(void *arg)
 	sc->sc_beacons = 0;
 
 	/*
+	 * Initial aggregation settings.
+	 */
+	sc->sc_hwq_limit = ATH_AGGR_MIN_QDEPTH;
+	sc->sc_tid_hwq_lo = ATH_AGGR_SCHED_LOW;
+	sc->sc_tid_hwq_hi = ATH_AGGR_SCHED_HIGH;
+
+	/*
 	 * Setup the hardware after reset: the key cache
 	 * is filled as needed and the receive engine is
 	 * set going.  Frame transmit is handled entirely
@@ -3478,6 +3524,7 @@ ath_rx_proc(void *arg, int npending)
 	HAL_STATUS status;
 	int16_t nf;
 	u_int64_t tsf;
+	int npkts = 0;
 
 	DPRINTF(sc, ATH_DEBUG_RX_PROC, "%s: pending %u\n", __func__, npending);
 	ngood = 0;
@@ -3537,6 +3584,7 @@ ath_rx_proc(void *arg, int npending)
 			break;
 
 		TAILQ_REMOVE(&sc->sc_rxbuf, bf, bf_list);
+		npkts++;
 
 		/* These aren't specifically errors */
 		if (rs->rs_flags & HAL_RX_GI)
@@ -3823,17 +3871,20 @@ rx_next:
 	 * been an RXEOL condition.
 	 */
 	if (sc->sc_kickpcu) {
-		sc->sc_kickpcu = 0;
-		ath_stoprecv(sc);
-		sc->sc_imask |= (HAL_INT_RXEOL | HAL_INT_RXORN);
-		if (ath_startrecv(sc) != 0) {
-			if_printf(ifp,
-			    "%s: couldn't restart RX after RXEOL; resetting\n",
-			    __func__);
-			ath_reset(ifp);
-			return;
-		}
+		device_printf(sc->sc_dev, "%s: kickpcu; handled %d packets\n",
+		    __func__, npkts);
+
+		/* XXX rxslink? */
+		bf = TAILQ_FIRST(&sc->sc_rxbuf);
+		ath_hal_putrxbuf(ah, bf->bf_daddr);
+		ath_hal_rxena(ah);		/* enable recv descriptors */
+		ath_mode_init(sc);		/* set filters, etc. */
+		ath_hal_startpcurecv(ah);	/* re-enable PCU/DMA engine */
+
+		ATH_LOCK(sc);
 		ath_hal_intrset(ah, sc->sc_imask);
+		sc->sc_kickpcu = 0;
+		ATH_UNLOCK(sc);
 	}
 
 	if ((ifp->if_drv_flags & IFF_DRV_OACTIVE) == 0) {
@@ -4218,13 +4269,7 @@ ath_tx_processq(struct ath_softc *sc, struct ath_txq *txq)
 	return nacked;
 }
 
-static __inline int
-txqactive(struct ath_hal *ah, int qnum)
-{
-	u_int32_t txqs = 1<<qnum;
-	ath_hal_gettxintrtxqs(ah, &txqs);
-	return (txqs & (1<<qnum));
-}
+#define	TXQACTIVE(t, q)		( (t) & (1 << (q)))
 
 /*
  * Deferred processing of transmit interrupt; special-cased
@@ -4235,10 +4280,17 @@ ath_tx_proc_q0(void *arg, int npending)
 {
 	struct ath_softc *sc = arg;
 	struct ifnet *ifp = sc->sc_ifp;
+	uint32_t txqs;
 
-	if (txqactive(sc->sc_ah, 0) && ath_tx_processq(sc, &sc->sc_txq[0]))
+	ATH_LOCK(sc);
+	txqs = sc->sc_txq_active;
+	sc->sc_txq_active &= ~txqs;
+	ATH_UNLOCK(sc);
+
+	if (TXQACTIVE(txqs, 0) && ath_tx_processq(sc, &sc->sc_txq[0]))
+		/* XXX why is lastrx updated in tx code? */
 		sc->sc_lastrx = ath_hal_gettsf64(sc->sc_ah);
-	if (txqactive(sc->sc_ah, sc->sc_cabq->axq_qnum))
+	if (TXQACTIVE(txqs, sc->sc_cabq->axq_qnum))
 		ath_tx_processq(sc, sc->sc_cabq);
 	ifp->if_drv_flags &= ~IFF_DRV_OACTIVE;
 	sc->sc_wd_timer = 0;
@@ -4259,20 +4311,26 @@ ath_tx_proc_q0123(void *arg, int npending)
 	struct ath_softc *sc = arg;
 	struct ifnet *ifp = sc->sc_ifp;
 	int nacked;
+	uint32_t txqs;
+
+	ATH_LOCK(sc);
+	txqs = sc->sc_txq_active;
+	sc->sc_txq_active &= ~txqs;
+	ATH_UNLOCK(sc);
 
 	/*
 	 * Process each active queue.
 	 */
 	nacked = 0;
-	if (txqactive(sc->sc_ah, 0))
+	if (TXQACTIVE(txqs, 0))
 		nacked += ath_tx_processq(sc, &sc->sc_txq[0]);
-	if (txqactive(sc->sc_ah, 1))
+	if (TXQACTIVE(txqs, 1))
 		nacked += ath_tx_processq(sc, &sc->sc_txq[1]);
-	if (txqactive(sc->sc_ah, 2))
+	if (TXQACTIVE(txqs, 2))
 		nacked += ath_tx_processq(sc, &sc->sc_txq[2]);
-	if (txqactive(sc->sc_ah, 3))
+	if (TXQACTIVE(txqs, 3))
 		nacked += ath_tx_processq(sc, &sc->sc_txq[3]);
-	if (txqactive(sc->sc_ah, sc->sc_cabq->axq_qnum))
+	if (TXQACTIVE(txqs, sc->sc_cabq->axq_qnum))
 		ath_tx_processq(sc, sc->sc_cabq);
 	if (nacked)
 		sc->sc_lastrx = ath_hal_gettsf64(sc->sc_ah);
@@ -4295,13 +4353,19 @@ ath_tx_proc(void *arg, int npending)
 	struct ath_softc *sc = arg;
 	struct ifnet *ifp = sc->sc_ifp;
 	int i, nacked;
+	uint32_t txqs;
+
+	ATH_LOCK(sc);
+	txqs = sc->sc_txq_active;
+	sc->sc_txq_active &= ~txqs;
+	ATH_UNLOCK(sc);
 
 	/*
 	 * Process each active queue.
 	 */
 	nacked = 0;
 	for (i = 0; i < HAL_NUM_TX_QUEUES; i++)
-		if (ATH_TXQ_SETUP(sc, i) && txqactive(sc->sc_ah, i))
+		if (ATH_TXQ_SETUP(sc, i) && TXQACTIVE(txqs, i))
 			nacked += ath_tx_processq(sc, &sc->sc_txq[i]);
 	if (nacked)
 		sc->sc_lastrx = ath_hal_gettsf64(sc->sc_ah);
diff --git a/sys/dev/ath/if_ath_tx.h b/sys/dev/ath/if_ath_tx.h
index e181f7a..c66f61b 100644
--- a/sys/dev/ath/if_ath_tx.h
+++ b/sys/dev/ath/if_ath_tx.h
@@ -31,6 +31,24 @@
 #ifndef	__IF_ATH_TX_H__
 #define	__IF_ATH_TX_H__
 
+/*
+ * How 'busy' to try and keep the hardware txq
+ */
+#define	ATH_AGGR_MIN_QDEPTH		2
+
+/*
+ * Watermark for scheduling TIDs in order to maximise aggregation.
+ *
+ * If hwq_depth is greater than this, don't schedule the TID
+ * for packet scheduling - the hardware is already busy servicing
+ * this TID.
+ *
+ * If hwq_depth is less than this, schedule the TID for packet
+ * scheduling in the completion handler.
+ */
+#define	ATH_AGGR_SCHED_HIGH		4
+#define	ATH_AGGR_SCHED_LOW		2
+
 extern void ath_freetx(struct mbuf *m);
 extern void ath_txfrag_cleanup(struct ath_softc *sc, ath_bufhead *frags,
     struct ieee80211_node *ni);
diff --git a/sys/dev/ath/if_athvar.h b/sys/dev/ath/if_athvar.h
index ba30f50..21829a62 100644
--- a/sys/dev/ath/if_athvar.h
+++ b/sys/dev/ath/if_athvar.h
@@ -312,7 +312,6 @@ struct ath_txq {
 	TAILQ_REMOVE(&(_tq)->axq_q, _elm, _field); \
 	(_tq)->axq_depth--; \
 } while (0)
-/* NB: this does not do the "head empty check" that STAILQ_LAST does */
 #define	ATH_TXQ_LAST(_tq, _field)	TAILQ_LAST(&(_tq)->axq_q, _field)
 
 struct ath_vap {
@@ -397,7 +396,6 @@ struct ath_softc {
 				sc_setcca   : 1,/* set/clr CCA with TDMA */
 				sc_resetcal : 1,/* reset cal state next trip */
 				sc_rxslink  : 1,/* do self-linked final descriptor */
-				sc_kickpcu  : 1,/* kick PCU RX on next RX proc */
 				sc_rxtsf32  : 1;/* RX dec TSF is 32 bits */
 	uint32_t		sc_eerd;	/* regdomain from EEPROM */
 	uint32_t		sc_eecc;	/* country code from EEPROM */
@@ -424,7 +422,19 @@ struct ath_softc {
 	u_int			sc_fftxqmin;	/* min frames before staging */
 	u_int			sc_fftxqmax;	/* max frames before drop */
 	u_int			sc_txantenna;	/* tx antenna (fixed or auto) */
+
 	HAL_INT			sc_imask;	/* interrupt mask copy */
+	/*
+	 * These are modified in the interrupt handler as well as
+	 * the task queues and other contexts. Thus these must be
+	 * protected by a mutex, or they could clash.
+	 *
+	 * For now, access to these is behind the ATH_LOCK,
+	 * just to save time.
+	 */
+	uint32_t		sc_txq_active;	/* bitmap of active TXQs */
+	uint32_t		sc_kickpcu;	/* whether to kick the PCU */
+
 	u_int			sc_keymax;	/* size of key cache */
 	u_int8_t		sc_keymap[ATH_KEYBYTES];/* key use bit map */