From: Ben Greear <[email protected]>
This lets user turn on/off this feature. Enabling gives better
tx-rate related stats, but will cause extra driver and (maybe)
firmware work. Not sure if it actually affects performance or
not.
Signed-off-by: Ben Greear <[email protected]>
---
.../wireless/mediatek/mt76/mt7915/debugfs.c | 24 +++++++++++++++++++
.../net/wireless/mediatek/mt76/mt7915/mac.c | 3 ++-
.../wireless/mediatek/mt76/mt7915/mt7915.h | 5 ++++
3 files changed, 31 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/debugfs.c b/drivers/net/wireless/mediatek/mt76/mt7915/debugfs.c
index 91664ac63a8d..6be194f16548 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7915/debugfs.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7915/debugfs.c
@@ -109,6 +109,29 @@ mt7915_fw_debug_get(void *data, u64 *val)
DEFINE_DEBUGFS_ATTRIBUTE(fops_fw_debug, mt7915_fw_debug_get,
mt7915_fw_debug_set, "%lld\n");
+static int
+mt7915_txs_for_no_skb_set(void *data, u64 val)
+{
+ struct mt7915_dev *dev = data;
+
+ dev->txs_for_no_skb_enabled = !!val;
+
+ return 0;
+}
+
+static int
+mt7915_txs_for_no_skb_get(void *data, u64 *val)
+{
+ struct mt7915_dev *dev = data;
+
+ *val = dev->txs_for_no_skb_enabled;
+
+ return 0;
+}
+
+DEFINE_DEBUGFS_ATTRIBUTE(fops_txs_for_no_skb, mt7915_txs_for_no_skb_get,
+ mt7915_txs_for_no_skb_set, "%lld\n");
+
static void
mt7915_ampdu_stat_read_phy(struct mt7915_phy *phy,
struct seq_file *file)
@@ -344,6 +367,7 @@ int mt7915_init_debugfs(struct mt7915_dev *dev)
mt7915_queues_acq);
debugfs_create_file("tx_stats", 0400, dir, dev, &mt7915_tx_stats_fops);
debugfs_create_file("fw_debug", 0600, dir, dev, &fops_fw_debug);
+ debugfs_create_file("txs_for_no_skb", 0600, dir, dev, &fops_txs_for_no_skb);
debugfs_create_file("implicit_txbf", 0600, dir, dev,
&fops_implicit_txbf);
debugfs_create_u32("dfs_hw_pattern", 0400, dir, &dev->hw_pattern);
diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/mac.c b/drivers/net/wireless/mediatek/mt76/mt7915/mac.c
index 2228dad71657..bdcd1aae10d1 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7915/mac.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7915/mac.c
@@ -1048,7 +1048,8 @@ void mt7915_mac_write_txwi(struct mt7915_dev *dev, __le32 *txwi,
val = FIELD_PREP(MT_TXD5_PID, pid);
/* NOTE: mt7916 does NOT request TXS for 'NO_SKB' frames by default. */
- if (pid >= MT_PACKET_ID_FIRST)
+ if (pid >= MT_PACKET_ID_FIRST ||
+ (pid == MT_PACKET_ID_NO_SKB && dev->txs_for_no_skb_enabled))
val |= MT_TXD5_TX_STATUS_HOST;
txwi[5] = cpu_to_le32(val);
diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/mt7915.h b/drivers/net/wireless/mediatek/mt76/mt7915/mt7915.h
index ea6da6b4e92d..1c0a1bf91c1c 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7915/mt7915.h
+++ b/drivers/net/wireless/mediatek/mt76/mt7915/mt7915.h
@@ -223,6 +223,11 @@ struct mt7915_dev {
u16 chainmask;
u32 hif_idx;
+ /* Should we request TXS for MT_PACKET_ID_NO_SKB? Doing so gives better
+ * costs but causes a great deal more TXS packet processing by driver and
+ * creation by firmware, so may be a performance drag.
+ */
+ bool txs_for_no_skb_enabled;
struct work_struct init_work;
struct work_struct rc_work;
--
2.20.1
On 2021-08-04 15:44, [email protected] wrote:
> From: Ben Greear <[email protected]>
>
> This lets user turn on/off this feature. Enabling gives better
> tx-rate related stats, but will cause extra driver and (maybe)
> firmware work. Not sure if it actually affects performance or
> not.
>
> Signed-off-by: Ben Greear <[email protected]>
> ---
> .../wireless/mediatek/mt76/mt7915/debugfs.c | 24 +++++++++++++++++++
> .../net/wireless/mediatek/mt76/mt7915/mac.c | 3 ++-
> .../wireless/mediatek/mt76/mt7915/mt7915.h | 5 ++++
> 3 files changed, 31 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/debugfs.c b/drivers/net/wireless/mediatek/mt76/mt7915/debugfs.c
> index 91664ac63a8d..6be194f16548 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7915/debugfs.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7915/debugfs.c
> @@ -344,6 +367,7 @@ int mt7915_init_debugfs(struct mt7915_dev *dev)
> mt7915_queues_acq);
> debugfs_create_file("tx_stats", 0400, dir, dev, &mt7915_tx_stats_fops);
> debugfs_create_file("fw_debug", 0600, dir, dev, &fops_fw_debug);
> + debugfs_create_file("txs_for_no_skb", 0600, dir, dev, &fops_txs_for_no_skb);
I think this is a rather confusing name. Maybe "force_txs"?
- Felix
-
[email protected] writes:
> From: Ben Greear <[email protected]>
>
> This lets user turn on/off this feature. Enabling gives better
> tx-rate related stats, but will cause extra driver and (maybe)
> firmware work. Not sure if it actually affects performance or
> not.
>
> Signed-off-by: Ben Greear <[email protected]>
This is grey area, debugfs is not really meant to be used for users
enabling driver features.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
On 8/19/21 9:06 AM, Kalle Valo wrote:
> [email protected] writes:
>
>> From: Ben Greear <[email protected]>
>>
>> This lets user turn on/off this feature. Enabling gives better
>> tx-rate related stats, but will cause extra driver and (maybe)
>> firmware work. Not sure if it actually affects performance or
>> not.
>>
>> Signed-off-by: Ben Greear <[email protected]>
>
> This is grey area, debugfs is not really meant to be used for users
> enabling driver features.
>
What method do you suggest? Surely not trying to drive something down through
netlink for something this chipset specific?
Thanks,
Ben
--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com
Ben Greear <[email protected]> writes:
> On 8/19/21 9:06 AM, Kalle Valo wrote:
>> [email protected] writes:
>>
>>> From: Ben Greear <[email protected]>
>>>
>>> This lets user turn on/off this feature. Enabling gives better
>>> tx-rate related stats, but will cause extra driver and (maybe)
>>> firmware work. Not sure if it actually affects performance or
>>> not.
>>>
>>> Signed-off-by: Ben Greear <[email protected]>
>>
>> This is grey area, debugfs is not really meant to be used for users
>> enabling driver features.
>>
>
> What method do you suggest? Surely not trying to drive something down through
> netlink for something this chipset specific?
I think a module parameter would be a good choise for enabling this kind
of feature.
Of course the downside of a module parater is that when it's not
possible to configure this per device, only per driver. Like discussed
many times in the past, we would really need some kind more advanced
module parameters which can be per device.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
On 10/11/21 2:28 AM, Kalle Valo wrote:
> Ben Greear <[email protected]> writes:
>
>> On 8/19/21 9:06 AM, Kalle Valo wrote:
>>> [email protected] writes:
>>>
>>>> From: Ben Greear <[email protected]>
>>>>
>>>> This lets user turn on/off this feature. Enabling gives better
>>>> tx-rate related stats, but will cause extra driver and (maybe)
>>>> firmware work. Not sure if it actually affects performance or
>>>> not.
>>>>
>>>> Signed-off-by: Ben Greear <[email protected]>
>>>
>>> This is grey area, debugfs is not really meant to be used for users
>>> enabling driver features.
>>>
>>
>> What method do you suggest? Surely not trying to drive something down through
>> netlink for something this chipset specific?
>
> I think a module parameter would be a good choise for enabling this kind
> of feature.
>
> Of course the downside of a module parater is that when it's not
> possible to configure this per device, only per driver. Like discussed
> many times in the past, we would really need some kind more advanced
> module parameters which can be per device.
Enabling/disabling via debugfs is way more useful than a module parameter.
It is per-device and also changeable at run-time. Maybe you want to enable
detailed stats for a bit of time for some reason, and then go back to lower
overhead approach.
You are asking for a worse technical solution than debugfs implementation.
For the per-device module parameters, I implemented something similar for
ath10k using a 'fwcfg' file that is loaded with device options sort of like
a board file. That worked well enough for that sort of thing.
Thanks,
Ben
--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com