Hi Thomas,

2014-11-30 14:30 GMT+03:00 Thomas Hühn <tho...@net.t-labs.tu-berlin.de>:
> From my point of view the call-path of the functions look like this:
>
> ath9k_init_queues(struct ath_softc *sc)
>
>        -> ath_txq_setup(sc, ATH9K_TX_QUEUE_DATA, i) with subtype-remapping
> via
>
> qi.tqi_subtype = subtype_txq_to_hwq[subtype];
>
> -> ath9k_hw_setuptxqueue(ah, qtype, &qi);
>
> And the subtyperemapping reflects the correct AC class mapping to ath9k hw
> queues.

tqi_subtype field doesn't define hw queue. Quick grep show only 6
lines in the whole driver, where this field is accessed. Actual hw
queue number stored in axq_qnum field of ath_txq as documented in
ath9k.h

> In xmit.c function ath_txq_setup() defines and initializes the mapping with:
>
> static const int subtype_txq_to_hwq[] = {
>
> [IEEE80211_AC_BE] = ATH_TXQ_AC_BE,
> [IEEE80211_AC_BK] = ATH_TXQ_AC_BK,
> [IEEE80211_AC_VI] = ATH_TXQ_AC_VI,
> [IEEE80211_AC_VO] = ATH_TXQ_AC_VO,
> };
>
> from mac80211.h:
>
> 101 /**
> 102  * enum ieee80211_ac_numbers - AC numbers as used in mac80211
> 103  * @IEEE80211_AC_VO: voice
> 104  * @IEEE80211_AC_VI: video
> 105  * @IEEE80211_AC_BE: best effort
> 106  * @IEEE80211_AC_BK: background
> 107  */
> 108 enum ieee80211_ac_numbers {
> 109         IEEE80211_AC_VO         = 0,
> 110         IEEE80211_AC_VI         = 1,
> 111         IEEE80211_AC_BE         = 2,
> 112         IEEE80211_AC_BK         = 3,
> 113 };
> 114 #define IEEE80211_NUM_ACS       4
>
> from ath9k/hw.h:
>
> 218 enum ath_hw_txq_subtype {
> 219         ATH_TXQ_AC_BE = 0,
> 220         ATH_TXQ_AC_BK = 1,
> 221         ATH_TXQ_AC_VI = 2,
> 222         ATH_TXQ_AC_VO = 3,
> 223 };
>
> this translates to the following mapping of AC to hw queues:
>
> static const int subtype_txq_to_hwq[] = {
> [2] = 0,
> [3] = 1,
> [1] = 2,
> [0] = 3,
> };
>

This table confuse me too. But if you trace where the table and the
tqi_subtype field is used, then you face, that this table means
nothing for hw queue selection.

>
> That is how I understand the source code and yes it looks a bit complicated
> but function wise seems to be correct.
> Or do I miss something here ?
>

Actual hw queue selection occurs inside the ath9k_hw_setuptxqueue()
function (in mac.c). Stripped version looks like:

int ath9k_hw_setuptxqueue(struct ath_hw *ah, enum ath9k_tx_queue type,
                          const struct ath9k_tx_queue_info *qinfo)
{
        int q;
[stripped]
                for (q = 0; q < ATH9K_NUM_TX_QUEUES; q++)
                        if (ah->txq[q].tqi_type ==
                            ATH9K_TX_QUEUE_INACTIVE)
                                break;
[stripped]
        return q;
}

So, as Huber wrote, hw queue number really depends on call order. And
existing call order gives us the following result:

~# cat /sys/kernel/debug/ieee80211/phy0/ath9k/queues
(VO):  qnum: 0 qdepth:  0 ampdu-depth:  0 pending:   0 stopped: 0
(VI):  qnum: 1 qdepth:  0 ampdu-depth:  0 pending:   0 stopped: 0
(BE):  qnum: 2 qdepth:  0 ampdu-depth:  0 pending:   0 stopped: 0
(BK):  qnum: 3 qdepth:  0 ampdu-depth:  0 pending:   0 stopped: 0
(CAB): qnum: 8 qdepth:  0 ampdu-depth:  0 pending:   0 stopped: 0

Just in case, I checked ath5k driver and it contains exactly the same code.

Let's wait a comment from maintainers.

> Greetings from Berlin
> Thomas
>
>
>
>
> Am 27.11.2014 um 13:04 schrieb Hubert Feurstein <h.feurst...@gmail.com>:
>
> Hi Sujith,
>
> 2014-11-26 13:25 GMT+01:00 Sujith Manoharan <suj...@msujith.org>:
>
> Hubert Feurstein wrote:
>
> Well, in fact it does. To understand my post you have to know the
> behaviour of ath_txq_setup. The point here is that on the first call
> of ath_txq_setup(..ATH9K_TX_QUEUE_DATA..), hw-queue 0 is returned. On
> the second call hw-queue 1 is returned, ... . And IEEE80211_AC_VO = 0,
> so hw-queue 0 is assigned, which is wrong in my opinion. But the right
> place to fix this would be ath_txq_setup itself.
>
>
> The mapping has been changed recently and I think it might be a problem,
> since this is what the datasheet says:
>
> "The mapping of physical DCUs to absolute channel access priorities is fixed
> and
> cannot be altered by software."
>
>
> Yes I know, and isn't this the reason why there is
> sc->tx.txq_map[<AC>] in place, to map the AC to the proper HW queue in
> software. So sc->tx.txq_map[IEEE80211_AC_VO] should return the
> higher-prio-queue 3 and not the low-prio-queue 0. In FreeBSD it is
> that way, in Linux it is the opposite, because in Linux
> IEEE80211_AC_VO=0 and in FreeBSD WME_AC_VO=3.
>
> Hubert

-- 
BR,
Sergey
_______________________________________________
ath9k-devel mailing list
ath9k-devel@lists.ath9k.org
https://lists.ath9k.org/mailman/listinfo/ath9k-devel

Reply via email to