Re: [PATCH] cfg80211/nl80211: add wifi tx power mode switching support

[Dan Williams]

On Thu, 2016-05-05 at 14:44 +0800, Wei-Ning Huang wrote:
> Recent new hardware has the ability to switch between tablet mode and
> clamshell mode. To optimize WiFi performance, we want to be able to
> use
> different power table between modes. This patch adds a new netlink
> message type and cfg80211_ops function to allow userspace to trigger
> a
> power mode switch for a given wireless interface.
> 
> Signed-off-by: Wei-Ning Huang <wnhuang@xxxxxxxxxxxx>
> ---
> Âinclude/net/cfg80211.hÂÂÂÂÂÂÂ| 11 +++++++++++
> Âinclude/uapi/linux/nl80211.h | 21 +++++++++++++++++++++
> Ânet/wireless/nl80211.cÂÂÂÂÂÂÂ| 16 ++++++++++++++++
> Ânet/wireless/rdev-ops.hÂÂÂÂÂÂ| 22 ++++++++++++++++++++++
> Ânet/wireless/trace.hÂÂÂÂÂÂÂÂÂ| 20 ++++++++++++++++++++
> Â5 files changed, 90 insertions(+)
> 
> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> index 9e1b24c..aa77fa0 100644
> --- a/include/net/cfg80211.h
> +++ b/include/net/cfg80211.h
> @@ -2370,6 +2370,12 @@ struct cfg80211_qos_map {
> Â * @get_tx_power: store the current TX power into the dbm variable;
> Â *	return 0 if successful
> Â *
> + * @set_tx_power_mode: set the transmit power mode. Some device have
> the ability
> + *	to transform between different mode such as clamshell and
> tablet mode.
> + *	set_tx_power_mode allows setting of different TX power
> mode at runtime.
> + * @get_tx_power_mode: store the current TX power mode into the mode
> variable;
> + *	return 0 if successful
> + *
> Â * @set_wds_peer: set the WDS peer for a WDS interface
> Â *
> Â * @rfkill_poll: polls the hw rfkill line, use cfg80211 reporting
> @@ -2631,6 +2637,11 @@ struct cfg80211_ops {
> Â	int	(*get_tx_power)(struct wiphy *wiphy, struct
> wireless_dev *wdev,
> Â				int *dbm);
> Â
> +	int	(*set_tx_power_mode)(struct wiphy *wiphy,
> +				ÂÂÂÂÂenum nl80211_tx_power_mode
> mode);
> +	int	(*get_tx_power_mode)(struct wiphy *wiphy,
> +				ÂÂÂÂÂenum nl80211_tx_power_mode
> *mode);
> +
> Â	int	(*set_wds_peer)(struct wiphy *wiphy, struct
> net_device *dev,
> Â				const u8 *addr);
> Â
> diff --git a/include/uapi/linux/nl80211.h
> b/include/uapi/linux/nl80211.h
> index 5a30a75..9b1888a 100644
> --- a/include/uapi/linux/nl80211.h
> +++ b/include/uapi/linux/nl80211.h
> @@ -1796,6 +1796,9 @@ enum nl80211_commands {
> Â *	connecting to a PCP, and in %NL80211_CMD_START_AP to start
> Â *	a PCP instead of AP. Relevant for DMG networks only.
> Â *
> + * @NL80211_ATTR_WIPHY_TX_POWER_MODE: Transmit power mode. See
> + *ÂÂÂÂÂÂ&enum nl80211_tx_power_mode for possible values.
> + *
> Â * @NUM_NL80211_ATTR: total number of nl80211_attrs available
> Â * @NL80211_ATTR_MAX: highest attribute number currently defined
> Â * @__NL80211_ATTR_AFTER_LAST: internal use
> @@ -2172,6 +2175,8 @@ enum nl80211_attrs {
> Â
> Â	NL80211_ATTR_PBSS,
> Â
> +	NL80211_ATTR_WIPHY_TX_POWER_MODE,
> +
> Â	/* add attributes here, update the policy in nl80211.c */
> Â
> Â	__NL80211_ATTR_AFTER_LAST,
> @@ -3703,6 +3708,22 @@ enum nl80211_tx_power_setting {
> Â};
> Â
> Â/**
> + * enum nl80211_tx_power_mode - TX power mode setting
> + * @NL80211_TX_POWER_LOW: general low TX power mode
> + * @NL80211_TX_POWER_MEDIUM: general medium TX power mode
> + * @NL80211_TX_POWER_HIGH: general high TX power mode
> + * @NL80211_TX_POWER_CLAMSHELL: clamshell mode TX power mode
> + * @NL80211_TX_POWER_TABLET: tablet mode TX power mode
> + */
> +enum nl80211_tx_power_mode {
> +	NL80211_TX_POWER_LOW,
> +	NL80211_TX_POWER_MEDIUM,
> +	NL80211_TX_POWER_HIGH,
> +	NL80211_TX_POWER_CLAMSHELL,
> +	NL80211_TX_POWER_TABLET,

"clamshell" and "tablet" probably mean many different things to many
different people with respect to whether or not they should do anything
with power saving or wifi. ÂI feel like a more generic interface is
needed here.

Could this be already done by:
@NL80211_ATTR_WIPHY_TX_POWER_SETTING =ÂNL80211_TX_POWER_LIMITED
@NL80211_ATTR_WIPHY_TX_POWER_LEVEL = <limit for clamshell/tablet mode>

and then the device would be able to change its TX power as it saw fit
up to that limit set by your application which figures out whether it's
in clamshell or tablet mode?

Dan

> +};
> +
> +/**
> Â * enum nl80211_packet_pattern_attr - packet pattern attribute
> Â * @__NL80211_PKTPAT_INVALID: invalid number for nested attribute
> Â * @NL80211_PKTPAT_PATTERN: the pattern, values where the mask has
> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> index 056a730..61b474d 100644
> --- a/net/wireless/nl80211.c
> +++ b/net/wireless/nl80211.c
> @@ -402,6 +402,7 @@ static const struct nla_policy
> nl80211_policy[NUM_NL80211_ATTR] = {
> Â	[NL80211_ATTR_SCHED_SCAN_DELAY] = { .type = NLA_U32 },
> Â	[NL80211_ATTR_REG_INDOOR] = { .type = NLA_FLAG },
> Â	[NL80211_ATTR_PBSS] = { .type = NLA_FLAG },
> +	[NL80211_ATTR_WIPHY_TX_POWER_MODE] = { .type = NLA_U32 },
> Â};
> Â
> Â/* policy for the key attributes */
> @@ -2218,6 +2219,21 @@ static int nl80211_set_wiphy(struct sk_buff
> *skb, struct genl_info *info)
> Â			return result;
> Â	}
> Â
> +	if (info->attrs[NL80211_ATTR_WIPHY_TX_POWER_MODE]) {
> +		enum nl80211_tx_power_mode mode;
> +		int idx = 0;
> +
> +		if (!rdev->ops->set_tx_power_mode)
> +			return -EOPNOTSUPP;
> +
> +		idx = NL80211_ATTR_WIPHY_TX_POWER_MODE;
> +		mode = nla_get_u32(info->attrs[idx]);
> +
> +		result = rdev_set_tx_power_mode(rdev, mode);
> +		if (result)
> +			return result;
> +	}
> +
> Â	if (info->attrs[NL80211_ATTR_WIPHY_ANTENNA_TX] &&
> Â	ÂÂÂÂinfo->attrs[NL80211_ATTR_WIPHY_ANTENNA_RX]) {
> Â		u32 tx_ant, rx_ant;
> diff --git a/net/wireless/rdev-ops.h b/net/wireless/rdev-ops.h
> index 8ae0c04..c3a1035 100644
> --- a/net/wireless/rdev-ops.h
> +++ b/net/wireless/rdev-ops.h
> @@ -552,6 +552,28 @@ static inline int rdev_get_tx_power(struct
> cfg80211_registered_device *rdev,
> Â	return ret;
> Â}
> Â
> +static inline int
> +rdev_set_tx_power_mode(struct cfg80211_registered_device *rdev,
> +		ÂÂÂÂÂÂÂenum nl80211_tx_power_mode mode)
> +{
> +	int ret;
> +	trace_rdev_set_tx_power_mode(&rdev->wiphy, mode);
> +	ret = rdev->ops->set_tx_power_mode(&rdev->wiphy, mode);
> +	trace_rdev_return_int(&rdev->wiphy, ret);
> +	return ret;
> +}
> +
> +static inline int
> +rdev_get_tx_power_mode(struct cfg80211_registered_device *rdev,
> +		ÂÂÂÂÂÂÂenum nl80211_tx_power_mode *mode)
> +{
> +	int ret;
> +	trace_rdev_get_tx_power_mode(&rdev->wiphy);
> +	ret = rdev->ops->get_tx_power_mode(&rdev->wiphy, mode);
> +	trace_rdev_return_int_int(&rdev->wiphy, ret, *mode);
> +	return ret;
> +}
> +
> Âstatic inline int rdev_set_wds_peer(struct
> cfg80211_registered_device *rdev,
> Â				ÂÂÂÂstruct net_device *dev, const u8
> *addr)
> Â{
> diff --git a/net/wireless/trace.h b/net/wireless/trace.h
> index 09b242b..132c8c2 100644
> --- a/net/wireless/trace.h
> +++ b/net/wireless/trace.h
> @@ -1420,6 +1420,26 @@ TRACE_EVENT(rdev_set_tx_power,
> Â		ÂÂWIPHY_PR_ARG, WDEV_PR_ARG,__entry->type, __entry-
> >mbm)
> Â);
> Â
> +DEFINE_EVENT(wiphy_only_evt, rdev_get_tx_power_mode,
> +	TP_PROTO(struct wiphy *wiphy),
> +	TP_ARGS(wiphy)
> +);
> +
> +TRACE_EVENT(rdev_set_tx_power_mode,
> +	TP_PROTO(struct wiphy *wiphy, enum nl80211_tx_power_mode
> mode),
> +	TP_ARGS(wiphy, mode),
> +	TP_STRUCT__entry(
> +		WIPHY_ENTRY
> +		__field(enum nl80211_tx_power_mode, mode)
> +	),
> +	TP_fast_assign(
> +		WIPHY_ASSIGN;
> +		__entry->mode = mode;
> +	),
> +	TP_printk(WIPHY_PR_FMT ", mode: %d",
> +		ÂÂWIPHY_PR_ARG, __entry->mode)
> +);
> +
> ÂTRACE_EVENT(rdev_return_int_int,
> Â	TP_PROTO(struct wiphy *wiphy, int func_ret, int func_fill),
> Â	TP_ARGS(wiphy, func_ret, func_fill),