| Message ID | 20191126214704.27297-5-markus.theil@tu-ilmenau.de (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Felix Fietkau |
| Headers | show |
| Series | mt76: channel switch support for USB devices | expand |
> This patch removes a mt76_wr_copy call from the beacon path to hw. > The skb which is used in this place gets therefore build with txwi > inside its data. For mt76 usb drivers, this saves one synchronuous > copy call over usb, which lets the beacon work complete faster. > > In mmio case, there is not enough headroom to put the txwi into the > skb, it is therefore using an additional mt76_wr_copy, which is fast > over mmio. Thanks Stanislaw for pointing this out. > > Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de> > --- > .../wireless/mediatek/mt76/mt76x02_beacon.c | 20 +++++++++++++++---- > 1 file changed, 16 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c b/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c > index 1c4bdf88f712..68a4f512319e 100644 > --- a/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c > +++ b/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c > @@ -26,15 +26,27 @@ static int > mt76x02_write_beacon(struct mt76x02_dev *dev, int offset, struct sk_buff *skb) > { > int beacon_len = dev->beacon_ops->slot_size; > - struct mt76x02_txwi txwi; > > if (WARN_ON_ONCE(beacon_len < skb->len + sizeof(struct mt76x02_txwi))) > return -ENOSPC; > > - mt76x02_mac_write_txwi(dev, &txwi, skb, NULL, NULL, skb->len); > + /* USB devices already reserve enough skb headroom for txwi's. This > + * helps to save slow copies over USB. > + */ > + if (mt76_is_usb(&dev->mt76)) { > + struct mt76x02_txwi *txwi; > + > + mt76_insert_hdr_pad(skb); Do we really need mt76_insert_hdr_pad? I think beacon header should be 4B aligned. Regards, Lorenzo > + txwi = (struct mt76x02_txwi *)(skb->data - sizeof(*txwi)); > + mt76x02_mac_write_txwi(dev, txwi, skb, NULL, NULL, skb->len); > + skb_push(skb, sizeof(*txwi)); > + } else { > + struct mt76x02_txwi txwi; > > - mt76_wr_copy(dev, offset, &txwi, sizeof(txwi)); > - offset += sizeof(txwi); > + mt76x02_mac_write_txwi(dev, &txwi, skb, NULL, NULL, skb->len); > + mt76_wr_copy(dev, offset, &txwi, sizeof(txwi)); > + offset += sizeof(txwi); > + } > > mt76_wr_copy(dev, offset, skb->data, skb->len); > return 0; > -- > 2.24.0 >
On 12/17/19 10:40 AM, Lorenzo Bianconi wrote: >> This patch removes a mt76_wr_copy call from the beacon path to hw. >> The skb which is used in this place gets therefore build with txwi >> inside its data. For mt76 usb drivers, this saves one synchronuous >> copy call over usb, which lets the beacon work complete faster. >> >> In mmio case, there is not enough headroom to put the txwi into the >> skb, it is therefore using an additional mt76_wr_copy, which is fast >> over mmio. Thanks Stanislaw for pointing this out. >> >> Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de> >> --- >> .../wireless/mediatek/mt76/mt76x02_beacon.c | 20 +++++++++++++++---- >> 1 file changed, 16 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c b/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c >> index 1c4bdf88f712..68a4f512319e 100644 >> --- a/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c >> +++ b/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c >> @@ -26,15 +26,27 @@ static int >> mt76x02_write_beacon(struct mt76x02_dev *dev, int offset, struct sk_buff *skb) >> { >> int beacon_len = dev->beacon_ops->slot_size; >> - struct mt76x02_txwi txwi; >> >> if (WARN_ON_ONCE(beacon_len < skb->len + sizeof(struct mt76x02_txwi))) >> return -ENOSPC; >> >> - mt76x02_mac_write_txwi(dev, &txwi, skb, NULL, NULL, skb->len); >> + /* USB devices already reserve enough skb headroom for txwi's. This >> + * helps to save slow copies over USB. >> + */ >> + if (mt76_is_usb(&dev->mt76)) { >> + struct mt76x02_txwi *txwi; >> + >> + mt76_insert_hdr_pad(skb); > Do we really need mt76_insert_hdr_pad? I think beacon header should be 4B > aligned. > > Regards, > Lorenzo I can leave it out of course, but I don't know, if beacons from mac80211 are always 4B aligned. Regards, Markus >> + txwi = (struct mt76x02_txwi *)(skb->data - sizeof(*txwi)); >> + mt76x02_mac_write_txwi(dev, txwi, skb, NULL, NULL, skb->len); >> + skb_push(skb, sizeof(*txwi)); >> + } else { >> + struct mt76x02_txwi txwi; >> >> - mt76_wr_copy(dev, offset, &txwi, sizeof(txwi)); >> - offset += sizeof(txwi); >> + mt76x02_mac_write_txwi(dev, &txwi, skb, NULL, NULL, skb->len); >> + mt76_wr_copy(dev, offset, &txwi, sizeof(txwi)); >> + offset += sizeof(txwi); >> + } >> >> mt76_wr_copy(dev, offset, skb->data, skb->len); >> return 0; >> -- >> 2.24.0 >>
> On 12/17/19 10:40 AM, Lorenzo Bianconi wrote: > >> This patch removes a mt76_wr_copy call from the beacon path to hw. > >> The skb which is used in this place gets therefore build with txwi > >> inside its data. For mt76 usb drivers, this saves one synchronuous > >> copy call over usb, which lets the beacon work complete faster. > >> > >> In mmio case, there is not enough headroom to put the txwi into the > >> skb, it is therefore using an additional mt76_wr_copy, which is fast > >> over mmio. Thanks Stanislaw for pointing this out. > >> > >> Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de> > >> --- > >> .../wireless/mediatek/mt76/mt76x02_beacon.c | 20 +++++++++++++++---- > >> 1 file changed, 16 insertions(+), 4 deletions(-) > >> > >> diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c b/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c > >> index 1c4bdf88f712..68a4f512319e 100644 > >> --- a/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c > >> +++ b/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c > >> @@ -26,15 +26,27 @@ static int > >> mt76x02_write_beacon(struct mt76x02_dev *dev, int offset, struct sk_buff *skb) > >> { > >> int beacon_len = dev->beacon_ops->slot_size; > >> - struct mt76x02_txwi txwi; > >> > >> if (WARN_ON_ONCE(beacon_len < skb->len + sizeof(struct mt76x02_txwi))) > >> return -ENOSPC; > >> > >> - mt76x02_mac_write_txwi(dev, &txwi, skb, NULL, NULL, skb->len); > >> + /* USB devices already reserve enough skb headroom for txwi's. This > >> + * helps to save slow copies over USB. > >> + */ > >> + if (mt76_is_usb(&dev->mt76)) { > >> + struct mt76x02_txwi *txwi; > >> + > >> + mt76_insert_hdr_pad(skb); > > Do we really need mt76_insert_hdr_pad? I think beacon header should be 4B > > aligned. mt76_insert_hdr_pad takes into account just 802.11 header length and it is 24 or 28 for mgmt frames. Regards, Lorenzo > > > > Regards, > > Lorenzo > I can leave it out of course, but I don't know, if beacons from > mac80211 are always 4B aligned. > > Regards, > Markus > >> + txwi = (struct mt76x02_txwi *)(skb->data - sizeof(*txwi)); > >> + mt76x02_mac_write_txwi(dev, txwi, skb, NULL, NULL, skb->len); > >> + skb_push(skb, sizeof(*txwi)); > >> + } else { > >> + struct mt76x02_txwi txwi; > >> > >> - mt76_wr_copy(dev, offset, &txwi, sizeof(txwi)); > >> - offset += sizeof(txwi); > >> + mt76x02_mac_write_txwi(dev, &txwi, skb, NULL, NULL, skb->len); > >> + mt76_wr_copy(dev, offset, &txwi, sizeof(txwi)); > >> + offset += sizeof(txwi); > >> + } > >> > >> mt76_wr_copy(dev, offset, skb->data, skb->len); > >> return 0; > >> -- > >> 2.24.0 > >> >
On 12/18/19 2:17 PM, Lorenzo Bianconi wrote: >> On 12/17/19 10:40 AM, Lorenzo Bianconi wrote: >>>> This patch removes a mt76_wr_copy call from the beacon path to hw. >>>> The skb which is used in this place gets therefore build with txwi >>>> inside its data. For mt76 usb drivers, this saves one synchronuous >>>> copy call over usb, which lets the beacon work complete faster. >>>> >>>> In mmio case, there is not enough headroom to put the txwi into the >>>> skb, it is therefore using an additional mt76_wr_copy, which is fast >>>> over mmio. Thanks Stanislaw for pointing this out. >>>> >>>> Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de> >>>> --- >>>> .../wireless/mediatek/mt76/mt76x02_beacon.c | 20 +++++++++++++++---- >>>> 1 file changed, 16 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c b/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c >>>> index 1c4bdf88f712..68a4f512319e 100644 >>>> --- a/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c >>>> +++ b/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c >>>> @@ -26,15 +26,27 @@ static int >>>> mt76x02_write_beacon(struct mt76x02_dev *dev, int offset, struct sk_buff *skb) >>>> { >>>> int beacon_len = dev->beacon_ops->slot_size; >>>> - struct mt76x02_txwi txwi; >>>> >>>> if (WARN_ON_ONCE(beacon_len < skb->len + sizeof(struct mt76x02_txwi))) >>>> return -ENOSPC; >>>> >>>> - mt76x02_mac_write_txwi(dev, &txwi, skb, NULL, NULL, skb->len); >>>> + /* USB devices already reserve enough skb headroom for txwi's. This >>>> + * helps to save slow copies over USB. >>>> + */ >>>> + if (mt76_is_usb(&dev->mt76)) { >>>> + struct mt76x02_txwi *txwi; >>>> + >>>> + mt76_insert_hdr_pad(skb); >>> Do we really need mt76_insert_hdr_pad? I think beacon header should be 4B >>> aligned. > mt76_insert_hdr_pad takes into account just 802.11 header length and it is 24 > or 28 for mgmt frames. > > Regards, > Lorenzo Ok, thanks. Then I drop the call. Markus >>> Regards, >>> Lorenzo >> I can leave it out of course, but I don't know, if beacons from >> mac80211 are always 4B aligned. >> >> Regards, >> Markus >>>> + txwi = (struct mt76x02_txwi *)(skb->data - sizeof(*txwi)); >>>> + mt76x02_mac_write_txwi(dev, txwi, skb, NULL, NULL, skb->len); >>>> + skb_push(skb, sizeof(*txwi)); >>>> + } else { >>>> + struct mt76x02_txwi txwi; >>>> >>>> - mt76_wr_copy(dev, offset, &txwi, sizeof(txwi)); >>>> - offset += sizeof(txwi); >>>> + mt76x02_mac_write_txwi(dev, &txwi, skb, NULL, NULL, skb->len); >>>> + mt76_wr_copy(dev, offset, &txwi, sizeof(txwi)); >>>> + offset += sizeof(txwi); >>>> + } >>>> >>>> mt76_wr_copy(dev, offset, skb->data, skb->len); >>>> return 0; >>>> -- >>>> 2.24.0 >>>>
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c b/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c index 1c4bdf88f712..68a4f512319e 100644 --- a/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c +++ b/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c @@ -26,15 +26,27 @@ static int mt76x02_write_beacon(struct mt76x02_dev *dev, int offset, struct sk_buff *skb) { int beacon_len = dev->beacon_ops->slot_size; - struct mt76x02_txwi txwi; if (WARN_ON_ONCE(beacon_len < skb->len + sizeof(struct mt76x02_txwi))) return -ENOSPC; - mt76x02_mac_write_txwi(dev, &txwi, skb, NULL, NULL, skb->len); + /* USB devices already reserve enough skb headroom for txwi's. This + * helps to save slow copies over USB. + */ + if (mt76_is_usb(&dev->mt76)) { + struct mt76x02_txwi *txwi; + + mt76_insert_hdr_pad(skb); + txwi = (struct mt76x02_txwi *)(skb->data - sizeof(*txwi)); + mt76x02_mac_write_txwi(dev, txwi, skb, NULL, NULL, skb->len); + skb_push(skb, sizeof(*txwi)); + } else { + struct mt76x02_txwi txwi; - mt76_wr_copy(dev, offset, &txwi, sizeof(txwi)); - offset += sizeof(txwi); + mt76x02_mac_write_txwi(dev, &txwi, skb, NULL, NULL, skb->len); + mt76_wr_copy(dev, offset, &txwi, sizeof(txwi)); + offset += sizeof(txwi); + } mt76_wr_copy(dev, offset, skb->data, skb->len); return 0;
This patch removes a mt76_wr_copy call from the beacon path to hw. The skb which is used in this place gets therefore build with txwi inside its data. For mt76 usb drivers, this saves one synchronuous copy call over usb, which lets the beacon work complete faster. In mmio case, there is not enough headroom to put the txwi into the skb, it is therefore using an additional mt76_wr_copy, which is fast over mmio. Thanks Stanislaw for pointing this out. Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de> --- .../wireless/mediatek/mt76/mt76x02_beacon.c | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-)