On Tue, 2011-03-15 at 12:51 +0100, Johannes Berg wrote:
> On Mon, 2011-03-14 at 11:16 -0700, Daniel Halperin wrote:
>
> > BUT, it looks like when we set up aggregation, we set the scheduler to
> > ALWAYS use the default maximum of 64 frames for both these parameters
> > [2] when configuring the agg queue. This is probably why the scheduler
> > is willing to send long batches and have many outstanding frames. I
> > bet that the fix is to make these parameters match the ones that come
> > down from mac80211. If I get time later, I will see if I can fix
> > this.
>
> Yeah, you're absolutely right, the window size and frame limit should
> both match the information about the aggregation session, and be limited
> to what we and the peer would like to see.
>
> The bad thing is that we set up the queue when we get
> IEEE80211_AMPDU_TX_START, but we only know the window size when we get
> IEEE80211_AMPDU_TX_OPERATIONAL. However, I think we can change this
> since the first TX to the queue will only happen after _OPERATIONAL. We
> just need to refactor iwlagn_tx_agg_start() into the parts that can
> fail, and the parts that don't.
On top of the previous patch, does this work? Note that I haven't tested
it at all yet, sorry.
johannes
---
drivers/net/wireless/iwlwifi/iwl-agn-tx.c | 47 +++++++++++++++++++-----------
drivers/net/wireless/iwlwifi/iwl-agn.c | 4 ++
drivers/net/wireless/iwlwifi/iwl-agn.h | 3 +
drivers/net/wireless/iwlwifi/iwl-dev.h | 1
4 files changed, 39 insertions(+), 16 deletions(-)
--- a/drivers/net/wireless/iwlwifi/iwl-agn-tx.c 2011-03-15 13:22:04.000000000 +0100
+++ b/drivers/net/wireless/iwlwifi/iwl-agn-tx.c 2011-03-15 13:50:34.000000000 +0100
@@ -222,13 +222,8 @@ void iwlagn_tx_queue_set_status(struct i
scd_retry ? "BA" : "AC/CMD", txq_id, tx_fifo_id);
}
-static int iwlagn_txq_agg_enable(struct iwl_priv *priv, int txq_id,
- int tx_fifo, int sta_id, int tid, u16 ssn_idx)
+static int iwlagn_txq_agg_enable(struct iwl_priv *priv, int txq_id, int sta_id, int tid)
{
- unsigned long flags;
- u16 ra_tid;
- int ret;
-
if ((IWLAGN_FIRST_AMPDU_QUEUE > txq_id) ||
(IWLAGN_FIRST_AMPDU_QUEUE +
priv->cfg->base_params->num_of_ampdu_queues <= txq_id)) {
@@ -240,12 +235,33 @@ static int iwlagn_txq_agg_enable(struct
return -EINVAL;
}
- ra_tid = BUILD_RAxTID(sta_id, tid);
-
/* Modify device's station table to Tx this TID */
- ret = iwl_sta_tx_modify_enable_tid(priv, sta_id, tid);
- if (ret)
- return ret;
+ return iwl_sta_tx_modify_enable_tid(priv, sta_id, tid);
+}
+
+void iwlagn_txq_agg_queue_setup(struct iwl_priv *priv,
+ struct ieee80211_sta *sta,
+ int tid, int frame_limit)
+{
+ int sta_id, tx_fifo, txq_id, ssn_idx;
+ u16 ra_tid;
+ unsigned long flags;
+ struct iwl_tid_data *tid_data;
+
+ sta_id = iwl_sta_id(sta);
+ if (WARN_ON(sta_id == IWL_INVALID_STATION))
+ return;
+ if (WARN_ON(tid >= MAX_TID_COUNT))
+ return;
+
+ spin_lock_irqsave(&priv->sta_lock, flags);
+ tid_data = &priv->stations[sta_id].tid[tid];
+ ssn_idx = SEQ_TO_SN(tid_data->seq_number);
+ txq_id = tid_data->agg.txq_id;
+ tx_fifo = tid_data->agg.tx_fifo;
+ spin_unlock_irqrestore(&priv->sta_lock, flags);
+
+ ra_tid = BUILD_RAxTID(sta_id, tid);
spin_lock_irqsave(&priv->lock, flags);
@@ -271,10 +287,10 @@ static int iwlagn_txq_agg_enable(struct
iwl_write_targ_mem(priv, priv->scd_base_addr +
IWLAGN_SCD_CONTEXT_QUEUE_OFFSET(txq_id) +
sizeof(u32),
- ((SCD_WIN_SIZE <<
+ ((frame_limit <<
IWLAGN_SCD_QUEUE_CTX_REG2_WIN_SIZE_POS) &
IWLAGN_SCD_QUEUE_CTX_REG2_WIN_SIZE_MSK) |
- ((SCD_FRAME_LIMIT <<
+ ((frame_limit <<
IWLAGN_SCD_QUEUE_CTX_REG2_FRAME_LIMIT_POS) &
IWLAGN_SCD_QUEUE_CTX_REG2_FRAME_LIMIT_MSK));
@@ -284,8 +300,6 @@ static int iwlagn_txq_agg_enable(struct
iwlagn_tx_queue_set_status(priv, &priv->txq[txq_id], tx_fifo, 1);
spin_unlock_irqrestore(&priv->lock, flags);
-
- return 0;
}
static int iwlagn_txq_agg_disable(struct iwl_priv *priv, u16 txq_id,
@@ -1034,10 +1048,11 @@ int iwlagn_tx_agg_start(struct iwl_priv
tid_data = &priv->stations[sta_id].tid[tid];
*ssn = SEQ_TO_SN(tid_data->seq_number);
tid_data->agg.txq_id = txq_id;
+ tid_data->agg.tx_fifo = tx_fifo;
iwl_set_swq_id(&priv->txq[txq_id], get_ac_from_tid(tid), txq_id);
spin_unlock_irqrestore(&priv->sta_lock, flags);
- ret = iwlagn_txq_agg_enable(priv, txq_id, tx_fifo, sta_id, tid, *ssn);
+ ret = iwlagn_txq_agg_enable(priv, txq_id, sta_id, tid);
if (ret)
return ret;
--- a/drivers/net/wireless/iwlwifi/iwl-agn.c 2011-03-15 13:21:53.000000000 +0100
+++ b/drivers/net/wireless/iwlwifi/iwl-agn.c 2011-03-15 13:47:49.000000000 +0100
@@ -3344,6 +3344,10 @@ int iwlagn_mac_ampdu_action(struct ieee8
}
break;
case IEEE80211_AMPDU_TX_OPERATIONAL:
+ buf_size = min_t(int, buf_size, LINK_QUAL_AGG_FRAME_LIMIT_DEF);
+
+ iwlagn_txq_agg_queue_setup(priv, sta, tid, buf_size);
+
/*
* If the limit is 0, then it wasn't initialised yet,
* use the default. We can do that since we take the
--- a/drivers/net/wireless/iwlwifi/iwl-agn.h 2011-03-15 13:50:03.000000000 +0100
+++ b/drivers/net/wireless/iwlwifi/iwl-agn.h 2011-03-15 13:50:24.000000000 +0100
@@ -202,6 +202,9 @@ int iwlagn_tx_agg_start(struct iwl_priv
struct ieee80211_sta *sta, u16 tid, u16 *ssn);
int iwlagn_tx_agg_stop(struct iwl_priv *priv, struct ieee80211_vif *vif,
struct ieee80211_sta *sta, u16 tid);
+void iwlagn_txq_agg_queue_setup(struct iwl_priv *priv,
+ struct ieee80211_sta *sta,
+ int tid, int frame_limit);
int iwlagn_txq_check_empty(struct iwl_priv *priv,
int sta_id, u8 tid, int txq_id);
void iwlagn_rx_reply_compressed_ba(struct iwl_priv *priv,
--- a/drivers/net/wireless/iwlwifi/iwl-dev.h 2011-03-15 13:48:12.000000000 +0100
+++ b/drivers/net/wireless/iwlwifi/iwl-dev.h 2011-03-15 13:48:22.000000000 +0100
@@ -417,6 +417,7 @@ struct iwl_ht_agg {
#define IWL_EMPTYING_HW_QUEUE_ADDBA 2
#define IWL_EMPTYING_HW_QUEUE_DELBA 3
u8 state;
+ u8 tx_fifo;
};
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
