Received: by 2002:a05:7412:8d10:b0:f3:1519:9f41 with SMTP id bj16csp4950373rdb; Tue, 12 Dec 2023 14:18:59 -0800 (PST) X-Google-Smtp-Source: AGHT+IGFpIzjIPeCAiOip8Z94mXxlOkmrFpqqu51vDA5p27yedWRhmDsMZkf1esEWf56EnaLI4i5 X-Received: by 2002:a05:6a20:1612:b0:18f:97c:6162 with SMTP id l18-20020a056a20161200b0018f097c6162mr6551913pzj.95.1702419539632; Tue, 12 Dec 2023 14:18:59 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1702419539; cv=none; d=google.com; s=arc-20160816; b=pIWnQry3tGcUKYHahxrGGK5o6LOlmAa1A58l/8AE1FSL7SZJDPgDEMQZ7heMUwjQVf I0J7igrYTmfJvDq4yrVKhxgYQPCb1y3sn2jgeYmKoYsOHHqc77pyZiEh+g+H7z0uBpEk 23vHhSvVTWUMhsHm0bG3HdOPyqiWPkWNZ6gsX+/UHHJODEJfgyDmb6L1bNDI1fTdO/Am ifl7lU4foHdCv/xwt/6eEaXNWXjSfVjn+2iIEITZVqZv5VL5qMd37XI9BGyvPW90/wii QM1OOMg3vz7OzcRytfxKssItZgRHm/0PNSjFyw5MgeTm5vlMz+NTqHY7XIWeT5ynehRv Etkw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=in-reply-to:content-disposition:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:references:message-id:subject:cc :to:from:date; bh=PvY1eWl3lKFXHEN46vG5hFlccI2oevea3+BmMQLJsrc=; fh=pm9gVkp7ZZ5ob3AQI4weURE00PpRk4yDxNapfdWnepc=; b=WCokOnU8mVe/uBvLcUB5zixZpH21cG5NRPwQ6xm5kfSqcbnb2ThXHLQIVHa/wFwscw H4hyfN2Uh2GGLDiATmEOBwPKwK+rglABDZgVhNJdvYhc6K7vtfnVvbHhQEAGzzcjlrfV dAMcAtJLtVG+HrXb9tqW4LvR5Yu1pzOIa1fRo/cnwaJe2M0oJuD8VmOl58Xu7txoYuqu x4eM5KV0bToV8675V7vVG5rrM3sQdelg0j/QgFFaJxH+eoOM4oq6EQzNuCwig81Xn4We 0I4pTEHPKzkkAO6uH+rPG7kmhKHfgaMNoOUqQRfCeuhNstm6+7Q1fUbIOS5jaQAM8l4+ SfQA== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-wireless+bounces-717-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) smtp.mailfrom="linux-wireless+bounces-717-linux.lists.archive=gmail.com@vger.kernel.org" Return-Path: Received: from sy.mirrors.kernel.org (sy.mirrors.kernel.org. [2604:1380:40f1:3f00::1]) by mx.google.com with ESMTPS id j26-20020aa78dda000000b006ce420e612bsi2863015pfr.127.2023.12.12.14.18.59 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 12 Dec 2023 14:18:59 -0800 (PST) Received-SPF: pass (google.com: domain of linux-wireless+bounces-717-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-wireless+bounces-717-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) smtp.mailfrom="linux-wireless+bounces-717-linux.lists.archive=gmail.com@vger.kernel.org" Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by sy.mirrors.kernel.org (Postfix) with ESMTPS id AAA2EB2113E for ; Tue, 12 Dec 2023 22:18:51 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 33026660E3; Tue, 12 Dec 2023 22:18:47 +0000 (UTC) X-Original-To: linux-wireless@vger.kernel.org Received: from mail-pl1-f173.google.com (mail-pl1-f173.google.com [209.85.214.173]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C0E149C; Tue, 12 Dec 2023 14:18:43 -0800 (PST) Received: by mail-pl1-f173.google.com with SMTP id d9443c01a7336-1d319a7a35bso25037715ad.1; Tue, 12 Dec 2023 14:18:43 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1702419523; x=1703024323; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=PvY1eWl3lKFXHEN46vG5hFlccI2oevea3+BmMQLJsrc=; b=Sil14H59nmgOl1bCG252KjNzigSfW5HNV0fmWcIm1fdr0pfoe+ssjvZWzDY56aokdc 4ciAbTXioTPbczo8+Ha4ONDrjeDUSsa6r35hcz6c6YSQ7K0pVvcev5zEazWMoLTObfPm C/fhW35RLdW8JhdPygc4z0ijvW85gKt8hFxYH2IfQK424Cxcv3NPjBaMq9gfkeecXldk 8gA9JnquK07whHThUhX9QIBnv8OSbQ2bVNnKDDeKi6469aLKf3dLOOqGSx3rnRBVKIgC gWGiQHBt7SHkaS74ZS2y6MkhI81prVVwJ6r0c74vFt4BUsmPETD2sDZDctAGRxNuo0p5 20BQ== X-Gm-Message-State: AOJu0YxICo21o/x4xL6+wZ8gxFHqE1gwV6kcdgdFDZRG+OOCIThaTwI5 Fo8z78cW16DO76fUiDR+Zoc= X-Received: by 2002:a17:902:e54b:b0:1d0:796c:b06d with SMTP id n11-20020a170902e54b00b001d0796cb06dmr8010754plf.7.1702419523173; Tue, 12 Dec 2023 14:18:43 -0800 (PST) Received: from sultan-box.localdomain ([142.147.89.200]) by smtp.gmail.com with ESMTPSA id h8-20020a170902704800b001d0c09cc6ebsm9127532plt.92.2023.12.12.14.18.41 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 12 Dec 2023 14:18:42 -0800 (PST) Date: Tue, 12 Dec 2023 14:18:39 -0800 From: Sultan Alsawaf To: Mario Limonciello Cc: Felix Fietkau , Lorenzo Bianconi , Ryder Lee , Shayne Chen , Sean Wang , Kalle Valo , Matthias Brugger , AngeloGioacchino Del Regno , Deren Wu , Ming Yen Hsieh , Ben Greear , "open list:MEDIATEK MT76 WIRELESS LAN DRIVER" , "open list:ARM/Mediatek SoC support" , "moderated list:ARM/Mediatek SoC support" Subject: Re: [PATCH 1/2] wifi: mt76: mt7921: Disable powersaving by default Message-ID: References: <20231212090852.162787-1-mario.limonciello@amd.com> Precedence: bulk X-Mailing-List: linux-wireless@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20231212090852.162787-1-mario.limonciello@amd.com> On Tue, Dec 12, 2023 at 03:08:51AM -0600, Mario Limonciello wrote: > Several users have reported awful latency when powersaving is enabled > with certain access point combinations. It's also reported that the > powersaving feature doesn't provide an ample enough savings to justify > being enabled by default with these issues. > > Introduce a module parameter that would control the power saving > behavior. Set it to default as disabled. This mirrors what some other > WLAN drivers like iwlwifi do. > > Suggested-by: Sultan Alsawaf > Link: https://codeberg.org/Hybrid-Project-Developers/linux-tkg/blame/branch/master/mt76:-mt7921:-Disable-powersave-features-by-default.mypatch > Link: https://aur.archlinux.org/cgit/aur.git/tree/0027-mt76_-mt7921_-Disable-powersave-features-by-default.patch?h=linux-g14 > Link: https://community.frame.work/t/responded-strange-wlan-problems-with-kernel-branch-6-2/41868/4 > Signed-off-by: Mario Limonciello > --- > drivers/net/wireless/mediatek/mt76/mt7921/init.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/init.c b/drivers/net/wireless/mediatek/mt76/mt7921/init.c > index 7d6a9d746011..78d4197988c8 100644 > --- a/drivers/net/wireless/mediatek/mt76/mt7921/init.c > +++ b/drivers/net/wireless/mediatek/mt76/mt7921/init.c > @@ -10,6 +10,11 @@ > #include "../mt76_connac2_mac.h" > #include "mcu.h" > > +static bool mt7921_powersave; > +module_param_named(power_save, mt7921_powersave, bool, 0444); > +MODULE_PARM_DESC(power_save, > + "enable WiFi power management (default: disable)"); > + > static ssize_t mt7921_thermal_temp_show(struct device *dev, > struct device_attribute *attr, > char *buf) > @@ -271,11 +276,13 @@ int mt7921_register_device(struct mt792x_dev *dev) > dev->pm.idle_timeout = MT792x_PM_TIMEOUT; > dev->pm.stats.last_wake_event = jiffies; > dev->pm.stats.last_doze_event = jiffies; > - if (!mt76_is_usb(&dev->mt76)) { > + if (mt7921_powersave && !mt76_is_usb(&dev->mt76)) { > dev->pm.enable_user = true; > dev->pm.enable = true; > dev->pm.ds_enable_user = true; > dev->pm.ds_enable = true; > + } else { > + hw->wiphy->flags &= ~WIPHY_FLAG_PS_ON_BY_DEFAULT; > } > > if (!mt76_is_mmio(&dev->mt76)) > -- > 2.34.1 > A few things to note: 1. Power savings can be significant on some systems where keeping the PCIe link active consumes significant energy (e.g., Intel HX chipsets in laptops and probably desktops in general). On desktops this isn't a big deal, but on desktop-class laptops the battery impact will be noticeable. 2. This doesn't mirror iwlwifi, which has powersave enabled by default. Beacon filtering is tied to powersave in mt76, whereas it isn't in iwlwifi. Thus, disabling powersave on mt76 results in the loss of beacon filtering. This means you'll get a constant stream of interrupts from beacon frames transmitted by the AP, which can also have power implications. And iwlwifi handles powersave transitions in firmware, which allows it enter/exit powersave with very low latency. This isn't the case on mt76, which enters/exits powersave in software. 3. For insignificant/low-bandwidth traffic like ICMP to the AP, high latency is expected since the amount of traffic doesn't warrant kicking the chipset out of powersave. So although it's not pretty to look at, bad ping times to the AP aren't representative of the full user experience. That being said, given that my patch to disable powersave from over a year ago has apparently become a commonplace addition to mt76, it seems like users generally aren't happy with the current powersave UX. I agree that it should be better, though I'm not certain disabling powersave outright is the best move. Maybe the powersave behavior can be tweaked instead? The reason I disabled powersave on my mt76 hardware was because I wanted the lowest latency + highest throughput possible. I know that on smartphones, QCA chipsets exhibit the same latency issue when pinging the AP, due to powersave. But no one seems to be upset about that on their phone, so I think there's probably a way to make powersave work well for all parties. Regarding the patch itself, I think a better idea would be to tie the wiphy powersave flag to the deep sleep flag (`dev->pm.ds_enable_user`), so that users can really disable powersave through `iw` at runtime without needing to use debugfs. This would eliminate the need for a module parameter too. Also, I find it quite sad that my patch from over a year ago [1] was blatantly reauthored in that frame.work link. The commit message is even the same, word for word. :-( [1] https://github.com/kerneltoast/kernel_x86_laptop/commit/ca89780690f7492c2d357e0ed2213a1d027341ae Sultan