From: Kirill A. Korinsky Subject: Re: sys/iwx: support powersave To: Stefan Sperling Cc: OpenBSD tech , Klemens Nanni Date: Sat, 14 Feb 2026 19:32:29 +0100 On Thu, 12 Feb 2026 09:39:54 +0100, Stefan Sperling wrote: > > On Fri, Feb 06, 2026 at 09:52:48PM +0100, Kirill A. Korinsky wrote: > > On Fri, 06 Feb 2026 20:24:42 +0100, > > Stuart Henderson wrote: > > > > > > On 2026/02/06 19:45, Kirill A. Korinsky wrote: > > > > No, it follow the same pattern like iwn and wpi: to enable it, someone needs > > > > to explicity adds powersave into ifconfig args, and -powersave to disable. > > > > > > > > But I think I should document in man in this diff that it supports > > > > powersave, similar with iwn and wpi. > > > > > > Those are "older devices" as per > > > > > > powersave > > > Enable 802.11 power saving mode. This option is generally only > > > relevant to older devices where power saving is disabled by > > > default. On modern hardware, drivers will ask the firmware to > > > automatically enable any applicable power-saving features. > > > > > > I think some of the older ones were a bit unsure whether it works > > > properly on all devices (so allowing it to be disabled) but it should > > > be ok on newer ones.. > > > > > > > Now it is disabled by default and should be enabled manually, so it should > > be safe, isn't it? > > Power management in iwm/iwx is not entirely disabled by default at present. > > At present the driver is always setting the POWER_SAVE_ENA_MSK bit which > allows the device to "save power by turning off receiver and transmitter". > I assume (not having made any measurements) that this does save some battery. > It is certain that the firmware is doing something with this because while > monitoring iwx traffic we can see the device sending Null-Data frames to the > AP with the PM snooze/awake bit toggling on/off. Which is not something our > net80211 stack could do by itself. > > The proposed diff turns this feature off by default because all calls to > iwx_power_mac_update_mode() are removed (even though this function is not > deleted with this diff?) and replaced with iwx_set_ps_level() which as far > as I can tell doesn't get called if the ioctl which enables power management > does not run. So the proposed diff would probably cost people some battery > because nobody will bother running the additional command to enable power > management. > > I do think enabling support for legacy power management (DTIM) will have some > benefits, so I agree with the direction your diff is taking. If you want to > investigate further, on modern wifi networks there are additional > possibilities. See here for an overview of the state of the art in 2014: > https://mrncciew.com/2014/10/14/cwap-802-11-power-management/ > > There are probably even more features in modern standards but the intel > firmware supports all the important 2014 features mentioned there and > we leave them all disabled at present apart from POWER_SAVE_ENA_MSK. > > DTIM power management is a good start. Also supporting uAPSD and SMSP would > probably help save power quite a bit more. These newer features depend on > negotiation with the AP so having them be controlled by an ioctl which forces > PM on or off makes zero sense. > > The only use case I see for the ioctl would be to disable all power > management when using an AP such as ral(4) which cannot handle clients > using DTIM. At this point most APs in active use should be able to handle > DTIM power management as enabled by your patch (smartphones would not have > working wifi otherwise), if not also uAPSD and SMSP, etc. > So if we're going to be making use of this it should be done by default. > > By the way, I have quite a bit of diffs built up in order to support > BZ devices which is still work in progress and might clash a little > bit with your diffs, but not very much. > Thanks for the link and feedback. Here an updated patch with in this direction. I did not touch uAPSD, advanced PM flags, SMPS, or misbehaving-AP fallback. The change set is already relatively large, so I prefer to move by small steps. This patch enables power save by default for iwx by setting IEEE80211_F_PMGTON (and IEEE80211_C_PMGT), and makes iwx react to SIOCS80211POWER by switching between PM (level 3) and CAM at runtime. Index: share/man/man4/iwx.4 =================================================================== RCS file: /home/cvs/src/share/man/man4/iwx.4,v diff -u -p -r1.22 iwx.4 --- share/man/man4/iwx.4 1 Dec 2025 16:44:13 -0000 1.22 +++ share/man/man4/iwx.4 6 Feb 2026 18:33:51 -0000 @@ -76,7 +76,7 @@ driver supports protected management fra of management frames such as deauthentication, disassociation, and block-ack negotiation, on networks using WPA. .Pp -In BSS mode the driver supports background scanning; +In BSS mode the driver supports powersave mode and background scanning; see .Xr ifconfig 8 . .Pp Index: sys/dev/pci/if_iwx.c =================================================================== RCS file: /home/cvs/src/sys/dev/pci/if_iwx.c,v diff -u -p -r1.197 if_iwx.c --- sys/dev/pci/if_iwx.c 11 Feb 2026 11:14:53 -0000 1.197 +++ sys/dev/pci/if_iwx.c 14 Feb 2026 18:13:13 -0000 @@ -402,10 +402,7 @@ int iwx_flush_sta(struct iwx_softc *, st int iwx_beacon_filter_send_cmd(struct iwx_softc *, struct iwx_beacon_filter_cmd *); int iwx_update_beacon_abort(struct iwx_softc *, struct iwx_node *, int); -void iwx_power_build_cmd(struct iwx_softc *, struct iwx_node *, - struct iwx_mac_power_cmd *); -int iwx_power_mac_update_mode(struct iwx_softc *, struct iwx_node *); -int iwx_power_update_device(struct iwx_softc *); +int iwx_set_pslevel(struct iwx_softc *, int, int, int); int iwx_enable_beacon_filter(struct iwx_softc *, struct iwx_node *); int iwx_disable_beacon_filter(struct iwx_softc *); int iwx_add_sta_cmd(struct iwx_softc *, struct iwx_node *, int); @@ -6620,68 +6617,80 @@ iwx_update_beacon_abort(struct iwx_softc return iwx_beacon_filter_send_cmd(sc, &cmd); } -void -iwx_power_build_cmd(struct iwx_softc *sc, struct iwx_node *in, - struct iwx_mac_power_cmd *cmd) +int +iwx_set_pslevel(struct iwx_softc *sc, int dtim, int level, int async) { struct ieee80211com *ic = &sc->sc_ic; - struct ieee80211_node *ni = &in->in_ni; + struct iwx_device_power_cmd dcmd = { }; + struct iwx_mac_power_cmd mcmd; + struct iwx_node *in; + struct ieee80211_node *ni; + const struct iwx_pmgt *pmgt; + int range, skip_dtim, cmd_flags, err; int dtim_period, dtim_msec, keep_alive; - cmd->id_and_color = htole32(IWX_FW_CMD_ID_AND_COLOR(in->in_id, - in->in_color)); - if (ni->ni_dtimperiod) - dtim_period = ni->ni_dtimperiod; - else - dtim_period = 1; - - /* - * Regardless of power management state the driver must set - * keep alive period. FW will use it for sending keep alive NDPs - * immediately after association. Check that keep alive period - * is at least 3 * DTIM. - */ - dtim_msec = dtim_period * ni->ni_intval; - keep_alive = MAX(3 * dtim_msec, 1000 * IWX_POWER_KEEP_ALIVE_PERIOD_SEC); - keep_alive = roundup(keep_alive, 1000) / 1000; - cmd->keep_alive_seconds = htole16(keep_alive); + if (ic->ic_opmode == IEEE80211_M_MONITOR) + return 0; - if (ic->ic_opmode != IEEE80211_M_MONITOR) - cmd->flags = htole16(IWX_POWER_FLAGS_POWER_SAVE_ENA_MSK); -} + if (dtim == 0) { + dtim = 1; + skip_dtim = 0; + } else + skip_dtim = -1; -int -iwx_power_mac_update_mode(struct iwx_softc *sc, struct iwx_node *in) -{ - int err; - int ba_enable; - struct iwx_mac_power_cmd cmd; + if (dtim <= 2) + range = 0; + else if (dtim <= 10) + range = 1; + else + range = 2; - memset(&cmd, 0, sizeof(cmd)); + pmgt = &iwx_pmgt[range][level]; + if (skip_dtim == -1) + skip_dtim = pmgt->skip_dtim; - iwx_power_build_cmd(sc, in, &cmd); + if (level != 0) + dcmd.flags = htole16(IWX_DEVICE_POWER_FLAGS_POWER_SAVE_ENA_MSK); - err = iwx_send_cmd_pdu(sc, IWX_MAC_PM_POWER_TABLE, 0, - sizeof(cmd), &cmd); - if (err != 0) + cmd_flags = async ? IWX_CMD_ASYNC : 0; + err = iwx_send_cmd_pdu(sc, IWX_POWER_TABLE_CMD, cmd_flags, + sizeof(dcmd), &dcmd); + if (err) return err; - ba_enable = !!(cmd.flags & - htole16(IWX_POWER_FLAGS_POWER_MANAGEMENT_ENA_MSK)); - return iwx_update_beacon_abort(sc, in, ba_enable); -} + if ((sc->sc_flags & IWX_FLAG_MAC_ACTIVE) == 0) + return 0; -int -iwx_power_update_device(struct iwx_softc *sc) -{ - struct iwx_device_power_cmd cmd = { }; - struct ieee80211com *ic = &sc->sc_ic; + in = (void *)ic->ic_bss; + ni = &in->in_ni; - if (ic->ic_opmode != IEEE80211_M_MONITOR) - cmd.flags = htole16(IWX_DEVICE_POWER_FLAGS_POWER_SAVE_ENA_MSK); + memset(&mcmd, 0, sizeof(mcmd)); + mcmd.id_and_color = htole32(IWX_FW_CMD_ID_AND_COLOR(in->in_id, + in->in_color)); + dtim_period = ni->ni_dtimperiod ? ni->ni_dtimperiod : 1; + dtim_msec = dtim_period * ni->ni_intval; + keep_alive = MAX(3 * dtim_msec, 1000 * IWX_POWER_KEEP_ALIVE_PERIOD_SEC); + keep_alive = roundup(keep_alive, 1000) / 1000; + mcmd.keep_alive_seconds = htole16(keep_alive); - return iwx_send_cmd_pdu(sc, - IWX_POWER_TABLE_CMD, 0, sizeof(cmd), &cmd); + if (level != 0) { + mcmd.flags = htole16(IWX_POWER_FLAGS_POWER_SAVE_ENA_MSK | + IWX_POWER_FLAGS_POWER_MANAGEMENT_ENA_MSK); + mcmd.rx_data_timeout = htole32(pmgt->rxtimeout * 1024); + mcmd.tx_data_timeout = htole32(pmgt->txtimeout * 1024); + if (skip_dtim != 0) { + mcmd.flags |= htole16(IWX_POWER_FLAGS_SKIP_OVER_DTIM_MSK); + mcmd.skip_dtim_periods = skip_dtim + 1; + } + } + + err = iwx_send_cmd_pdu(sc, IWX_MAC_PM_POWER_TABLE, cmd_flags, + sizeof(mcmd), &mcmd); + if (err != 0) + return err; + + return iwx_update_beacon_abort(sc, in, !!(mcmd.flags & + htole16(IWX_POWER_FLAGS_POWER_MANAGEMENT_ENA_MSK))); } int @@ -8712,7 +8721,8 @@ iwx_run(struct iwx_softc *sc) return err; } - err = iwx_power_update_device(sc); + err = iwx_set_pslevel(sc, 0, + (ic->ic_flags & IEEE80211_F_PMGTON) ? 3 : 0, 1); if (err) { printf("%s: could not send power command (error %d)\n", DEVNAME(sc), err); @@ -8734,13 +8744,6 @@ iwx_run(struct iwx_softc *sc) if (ic->ic_opmode == IEEE80211_M_MONITOR) return 0; - err = iwx_power_mac_update_mode(sc, in); - if (err) { - printf("%s: could not update MAC power (error %d)\n", - DEVNAME(sc), err); - return err; - } - /* Start at lowest available bit-rate. Firmware will raise. */ in->in_ni.ni_txrate = 0; in->in_ni.ni_txmcs = 0; @@ -9632,7 +9635,8 @@ iwx_init_hw(struct iwx_softc *sc) goto err; } - err = iwx_power_update_device(sc); + err = iwx_set_pslevel(sc, 0, + (ic->ic_flags & IEEE80211_F_PMGTON) ? 3 : 0, 0); if (err) { printf("%s: could not send power command (error %d)\n", DEVNAME(sc), err); @@ -9986,6 +9990,7 @@ int iwx_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data) { struct iwx_softc *sc = ifp->if_softc; + struct ieee80211com *ic = &sc->sc_ic; int s, err = 0, generation = sc->sc_generation; /* @@ -10018,6 +10023,21 @@ iwx_ioctl(struct ifnet *ifp, u_long cmd, } break; + case SIOCS80211POWER: + err = ieee80211_ioctl(ifp, cmd, data); + if (err != ENETRESET) + break; + if (ic->ic_state == IEEE80211_S_RUN) { + if (ic->ic_flags & IEEE80211_F_PMGTON) + err = iwx_set_pslevel(sc, 0, 3, 0); + else /* back to CAM */ + err = iwx_set_pslevel(sc, 0, 0, 0); + } else { + /* Defer until transition to IEEE80211_S_RUN. */ + err = 0; + } + break; + default: err = ieee80211_ioctl(ifp, cmd, data); } @@ -11902,6 +11922,7 @@ iwx_attach(struct device *parent, struct ic->ic_caps = IEEE80211_C_QOS | IEEE80211_C_TX_AMPDU | /* A-MPDU */ IEEE80211_C_ADDBA_OFFLOAD | /* device sends ADDBA/DELBA frames */ + IEEE80211_C_PMGT | /* power management */ IEEE80211_C_WEP | /* WEP */ IEEE80211_C_RSN | /* WPA/RSN */ IEEE80211_C_SCANALL | /* device scans all channels at once */ @@ -11910,6 +11931,7 @@ iwx_attach(struct device *parent, struct IEEE80211_C_SHSLOT | /* short slot time supported */ IEEE80211_C_SHPREAMBLE | /* short preamble supported */ IEEE80211_C_MFP; /* management frame protection */ + ic->ic_flags |= IEEE80211_F_PMGTON; ic->ic_htcaps = IEEE80211_HTCAP_SGI20 | IEEE80211_HTCAP_SGI40; ic->ic_htcaps |= IEEE80211_HTCAP_CBW20_40; Index: sys/dev/pci/if_iwxreg.h =================================================================== RCS file: /home/cvs/src/sys/dev/pci/if_iwxreg.h,v diff -u -p -r1.58 if_iwxreg.h --- sys/dev/pci/if_iwxreg.h 1 Dec 2025 16:44:13 -0000 1.58 +++ sys/dev/pci/if_iwxreg.h 30 Jan 2026 23:52:35 -0000 @@ -5096,6 +5096,42 @@ struct iwx_mac_power_cmd { #define IWX_DEFAULT_PS_TX_DATA_TIMEOUT (100 * 1000) #define IWX_DEFAULT_PS_RX_DATA_TIMEOUT (100 * 1000) +#define IWX_NDTIMRANGES 3 +#define IWX_NPOWERLEVELS 6 +static const struct iwx_pmgt { + uint32_t rxtimeout; + uint32_t txtimeout; + int skip_dtim; +} iwx_pmgt[IWX_NDTIMRANGES][IWX_NPOWERLEVELS] = { + /* DTIM <= 2 */ + { + { 0, 0, 0 }, /* CAM */ + { 200, 500, 0 }, /* PS level 1 */ + { 200, 300, 0 }, /* PS level 2 */ + { 50, 100, 0 }, /* PS level 3 */ + { 50, 25, 1 }, /* PS level 4 */ + { 25, 25, 2 } /* PS level 5 */ + }, + /* 3 <= DTIM <= 10 */ + { + { 0, 0, 0 }, /* CAM */ + { 200, 500, 0 }, /* PS level 1 */ + { 200, 300, 0 }, /* PS level 2 */ + { 50, 100, 0 }, /* PS level 3 */ + { 50, 25, 1 }, /* PS level 4 */ + { 25, 25, 2 } /* PS level 5 */ + }, + /* DTIM >= 11 */ + { + { 0, 0, 0 }, /* CAM */ + { 200, 500, 0 }, /* PS level 1 */ + { 200, 300, 0 }, /* PS level 2 */ + { 50, 100, 0 }, /* PS level 3 */ + { 50, 25, 0 }, /* PS level 4 */ + { 25, 25, 0 } /* PS level 5 */ + } +}; + /* * struct iwx_uapsd_misbehaving_ap_notif - FW sends this notification when * associated AP is identified as improperly implementing uAPSD protocol.