All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
To: Denis Benato <denis.benato@linux.dev>
Cc: LKML <linux-kernel@vger.kernel.org>,
	platform-driver-x86@vger.kernel.org,
	 Hans de Goede <hdegoede@redhat.com>,
	 "Limonciello, Mario" <mario.limonciello@amd.com>,
	 "Luke D . Jones" <luke@ljones.dev>,
	Alok Tiwari <alok.a.tiwari@oracle.com>,
	 Derek John Clark <derekjohn.clark@gmail.com>,
	 Mateusz Schyboll <dragonn@op.pl>,
	Denis Benato <benato.denis96@gmail.com>,
	 porfet828@gmail.com
Subject: Re: [PATCH v16 9/9] platform/x86: asus-armoury: add ppt_* and nv_* tuning knobs
Date: Thu, 30 Oct 2025 16:44:48 +0200 (EET)	[thread overview]
Message-ID: <91cddea1-b5be-2bcc-cf64-7ac102c14aca@linux.intel.com> (raw)
In-Reply-To: <20251030130320.1287122-10-denis.benato@linux.dev>

On Thu, 30 Oct 2025, Denis Benato wrote:

> From: "Luke D. Jones" <luke@ljones.dev>
> 
> Adds the ppt_* and nv_* tuning knobs that are available via WMI methods
> and adds proper min/max levels plus defaults.
> 
> The min/max are defined by ASUS and typically gained by looking at what
> they allow in the ASUS Armoury Crate application - ASUS does not share
> the values outside of this. It could also be possible to gain the AMD
> values by use of ryzenadj and testing for the minimum stable value.
> 
> The general rule of thumb for adding to the match table is that if the
> model range has a single CPU used throughout, then the DMI match can
> omit the last letter of the model number as this is the GPU model.
> 
> If a min or max value is not provided it is assumed that the particular
> setting is not supported. for example ppt_pl2_sppt_min/max is not set.
> If a <ppt_setting>_def is not set then the default is assumed to be
> <ppt_setting>_max
> 
> It is assumed that at least AC settings are available so that the
> firmware attributes will be created - if no DC table is available
> and power is on DC, then reading the attributes is -ENODEV.
> 
> Co-developed-by: Denis Benato <denis.benato@linux.dev>
> Signed-off-by: Denis Benato <denis.benato@linux.dev>
> Signed-off-by: Luke D. Jones <luke@ljones.dev>
> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
> Tested-by: Mateusz Schyboll <dragonn@op.pl>
> Tested-by: Porfet Lillian <porfet828@gmail.com>
> ---
>  drivers/platform/x86/asus-armoury.c        |  296 ++++-
>  drivers/platform/x86/asus-armoury.h        | 1267 ++++++++++++++++++++
>  include/linux/platform_data/x86/asus-wmi.h |    3 +
>  3 files changed, 1560 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/platform/x86/asus-armoury.c b/drivers/platform/x86/asus-armoury.c
> index 63579034756a..9f0bbdc45ca0 100644
> --- a/drivers/platform/x86/asus-armoury.c
> +++ b/drivers/platform/x86/asus-armoury.c
> @@ -30,6 +30,7 @@
>  #include <linux/pci.h>
>  #include <linux/platform_data/x86/asus-wmi.h>
>  #include <linux/printk.h>
> +#include <linux/power_supply.h>
>  #include <linux/sysfs.h>
>  
>  #include "asus-armoury.h"
> @@ -48,9 +49,23 @@
>  #define ASUS_MINI_LED_2024_STRONG 0x01
>  #define ASUS_MINI_LED_2024_OFF    0x02
>  
> +/* Power tunable attribute name defines */
> +#define ATTR_PPT_PL1_SPL        "ppt_pl1_spl"
> +#define ATTR_PPT_PL2_SPPT       "ppt_pl2_sppt"
> +#define ATTR_PPT_PL3_FPPT       "ppt_pl3_fppt"
> +#define ATTR_PPT_APU_SPPT       "ppt_apu_sppt"
> +#define ATTR_PPT_PLATFORM_SPPT  "ppt_platform_sppt"
> +#define ATTR_NV_DYNAMIC_BOOST   "nv_dynamic_boost"
> +#define ATTR_NV_TEMP_TARGET     "nv_temp_target"
> +#define ATTR_NV_BASE_TGP        "nv_base_tgp"
> +#define ATTR_NV_TGP             "nv_tgp"
> +
>  #define ASUS_POWER_CORE_MASK	GENMASK(15, 8)
>  #define ASUS_PERF_CORE_MASK	GENMASK(7, 0)
>  
> +#define ASUS_ROG_TUNABLE_DC 0
> +#define ASUS_ROG_TUNABLE_AC 1
> +
>  enum cpu_core_type {
>  	CPU_CORE_PERF = 0,
>  	CPU_CORE_POWER,
> @@ -78,6 +93,19 @@ struct cpu_cores {
>  	u32 max_power_cores;
>  };
>  
> +struct rog_tunables {
> +	const struct power_limits *power_limits;
> +	u32 ppt_pl1_spl;			// cpu
> +	u32 ppt_pl2_sppt;			// cpu
> +	u32 ppt_pl3_fppt;			// cpu
> +	u32 ppt_apu_sppt;			// plat
> +	u32 ppt_platform_sppt;		// plat
> +
> +	u32 nv_dynamic_boost;
> +	u32 nv_temp_target;
> +	u32 nv_tgp;
> +};
> +
>  struct asus_armoury_priv {
>  	struct device *fw_attr_dev;
>  	struct kset *fw_attr_kset;
> @@ -98,6 +126,9 @@ struct asus_armoury_priv {
>  	struct cpu_cores *cpu_cores;
>  	bool cpu_cores_changeable;
>  
> +	/* Index 0 for DC, 1 for AC */
> +	struct rog_tunables *rog_tunables[2];
> +
>  	u32 mini_led_dev_id;
>  	u32 gpu_mux_dev_id;
>  };
> @@ -918,6 +949,15 @@ static ssize_t cores_performance_current_value_store(struct kobject *kobj,
>  ASUS_ATTR_GROUP_CORES_RW(cores_performance, "cores_performance",
>  			 "Set the max available performance cores");
>  
> +/* Define helper to access the current power mode tunable values */
> +static inline struct rog_tunables *get_current_tunables(void)
> +{
> +	if (power_supply_is_system_supplied())
> +		return asus_armoury.rog_tunables[ASUS_ROG_TUNABLE_AC];
> +
> +	return asus_armoury.rog_tunables[ASUS_ROG_TUNABLE_DC];
> +}
> +
>  static ssize_t cores_efficiency_min_value_show(struct kobject *kobj, struct kobj_attribute *attr,
>  					       char *buf)
>  {
> @@ -973,6 +1013,24 @@ ASUS_ATTR_GROUP_BOOL_RW(screen_auto_brightness, "screen_auto_brightness",
>  			"Set the panel brightness to Off<0> or On<1>");
>  ASUS_ATTR_GROUP_BOOL_RO(egpu_connected, "egpu_connected", ASUS_WMI_DEVID_EGPU_CONNECTED,
>  			"Show the eGPU connection status");
> +ASUS_ATTR_GROUP_ROG_TUNABLE(ppt_pl1_spl, ATTR_PPT_PL1_SPL, ASUS_WMI_DEVID_PPT_PL1_SPL,
> +			    "Set the CPU slow package limit");
> +ASUS_ATTR_GROUP_ROG_TUNABLE(ppt_pl2_sppt, ATTR_PPT_PL2_SPPT, ASUS_WMI_DEVID_PPT_PL2_SPPT,
> +			    "Set the CPU fast package limit");
> +ASUS_ATTR_GROUP_ROG_TUNABLE(ppt_pl3_fppt, ATTR_PPT_PL3_FPPT, ASUS_WMI_DEVID_PPT_PL3_FPPT,
> +			    "Set the CPU fastest package limit");
> +ASUS_ATTR_GROUP_ROG_TUNABLE(ppt_apu_sppt, ATTR_PPT_APU_SPPT, ASUS_WMI_DEVID_PPT_APU_SPPT,
> +			    "Set the APU package limit");
> +ASUS_ATTR_GROUP_ROG_TUNABLE(ppt_platform_sppt, ATTR_PPT_PLATFORM_SPPT, ASUS_WMI_DEVID_PPT_PLAT_SPPT,
> +			    "Set the platform package limit");
> +ASUS_ATTR_GROUP_ROG_TUNABLE(nv_dynamic_boost, ATTR_NV_DYNAMIC_BOOST, ASUS_WMI_DEVID_NV_DYN_BOOST,
> +			    "Set the Nvidia dynamic boost limit");
> +ASUS_ATTR_GROUP_ROG_TUNABLE(nv_temp_target, ATTR_NV_TEMP_TARGET, ASUS_WMI_DEVID_NV_THERM_TARGET,
> +			    "Set the Nvidia max thermal limit");
> +ASUS_ATTR_GROUP_ROG_TUNABLE(nv_tgp, "nv_tgp", ASUS_WMI_DEVID_DGPU_SET_TGP,
> +			    "Set the additional TGP on top of the base TGP");
> +ASUS_ATTR_GROUP_INT_VALUE_ONLY_RO(nv_base_tgp, ATTR_NV_BASE_TGP, ASUS_WMI_DEVID_DGPU_BASE_TGP,
> +				  "Read the base TGP value");
>  
>  /* If an attribute does not require any special case handling add it here */
>  static const struct asus_attr_group armoury_attr_groups[] = {
> @@ -983,6 +1041,16 @@ static const struct asus_attr_group armoury_attr_groups[] = {
>  	{ &cores_efficiency_attr_group, ASUS_WMI_DEVID_CORES_MAX },
>  	{ &cores_performance_attr_group, ASUS_WMI_DEVID_CORES_MAX },
>  
> +	{ &ppt_pl1_spl_attr_group, ASUS_WMI_DEVID_PPT_PL1_SPL },
> +	{ &ppt_pl2_sppt_attr_group, ASUS_WMI_DEVID_PPT_PL2_SPPT },
> +	{ &ppt_pl3_fppt_attr_group, ASUS_WMI_DEVID_PPT_PL3_FPPT },
> +	{ &ppt_apu_sppt_attr_group, ASUS_WMI_DEVID_PPT_APU_SPPT },
> +	{ &ppt_platform_sppt_attr_group, ASUS_WMI_DEVID_PPT_PLAT_SPPT },
> +	{ &nv_dynamic_boost_attr_group, ASUS_WMI_DEVID_NV_DYN_BOOST },
> +	{ &nv_temp_target_attr_group, ASUS_WMI_DEVID_NV_THERM_TARGET },
> +	{ &nv_base_tgp_attr_group, ASUS_WMI_DEVID_DGPU_BASE_TGP },
> +	{ &nv_tgp_attr_group, ASUS_WMI_DEVID_DGPU_SET_TGP },
> +
>  	{ &charge_mode_attr_group, ASUS_WMI_DEVID_CHARGE_MODE },
>  	{ &boot_sound_attr_group, ASUS_WMI_DEVID_BOOT_SOUND },
>  	{ &mcu_powersave_attr_group, ASUS_WMI_DEVID_MCU_POWERSAVE },
> @@ -991,8 +1059,75 @@ static const struct asus_attr_group armoury_attr_groups[] = {
>  	{ &screen_auto_brightness_attr_group, ASUS_WMI_DEVID_SCREEN_AUTO_BRIGHTNESS },
>  };
>  
> +/**
> + * is_power_tunable_attr - Determines if an attribute is a power-related tunable
> + * @name: The name of the attribute to check
> + *
> + * This function checks if the given attribute name is related to power tuning.
> + *
> + * Return: true if the attribute is a power-related tunable, false otherwise
> + */
> +static bool is_power_tunable_attr(const char *name)
> +{
> +	static const char * const power_tunable_attrs[] = {
> +		ATTR_PPT_PL1_SPL,	ATTR_PPT_PL2_SPPT,
> +		ATTR_PPT_PL3_FPPT,	ATTR_PPT_APU_SPPT,
> +		ATTR_PPT_PLATFORM_SPPT, ATTR_NV_DYNAMIC_BOOST,
> +		ATTR_NV_TEMP_TARGET,	ATTR_NV_BASE_TGP,
> +		ATTR_NV_TGP
> +	};
> +
> +	for (unsigned int i = 0; i < ARRAY_SIZE(power_tunable_attrs); i++) {
> +		if (!strcmp(name, power_tunable_attrs[i]))
> +			return true;
> +	}
> +
> +	return false;
> +}
> +
> +/**
> + * has_valid_limit - Checks if a power-related attribute has a valid limit value
> + * @name: The name of the attribute to check
> + * @limits: Pointer to the power_limits structure containing limit values
> + *
> + * This function checks if a power-related attribute has a valid limit value.
> + * It returns false if limits is NULL or if the corresponding limit value is zero.
> + *
> + * Return: true if the attribute has a valid limit value, false otherwise
> + */
> +static bool has_valid_limit(const char *name, const struct power_limits *limits)
> +{
> +	u32 limit_value = 0;
> +
> +	if (!limits)
> +		return false;
> +
> +	if (!strcmp(name, ATTR_PPT_PL1_SPL))
> +		limit_value = limits->ppt_pl1_spl_max;
> +	else if (!strcmp(name, ATTR_PPT_PL2_SPPT))
> +		limit_value = limits->ppt_pl2_sppt_max;
> +	else if (!strcmp(name, ATTR_PPT_PL3_FPPT))
> +		limit_value = limits->ppt_pl3_fppt_max;
> +	else if (!strcmp(name, ATTR_PPT_APU_SPPT))
> +		limit_value = limits->ppt_apu_sppt_max;
> +	else if (!strcmp(name, ATTR_PPT_PLATFORM_SPPT))
> +		limit_value = limits->ppt_platform_sppt_max;
> +	else if (!strcmp(name, ATTR_NV_DYNAMIC_BOOST))
> +		limit_value = limits->nv_dynamic_boost_max;
> +	else if (!strcmp(name, ATTR_NV_TEMP_TARGET))
> +		limit_value = limits->nv_temp_target_max;
> +	else if (!strcmp(name, ATTR_NV_BASE_TGP) ||
> +		 !strcmp(name, ATTR_NV_TGP))
> +		limit_value = limits->nv_tgp_max;
> +
> +	return limit_value > 0;
> +}
> +
>  static int asus_fw_attr_add(void)
>  {
> +	const struct power_limits *limits;
> +	bool should_create;
> +	const char *name;
>  	int err, i;
>  
>  	asus_armoury.fw_attr_dev = device_create(&firmware_attributes_class, NULL, MKDEV(0, 0),
> @@ -1049,12 +1184,29 @@ static int asus_fw_attr_add(void)
>  		if (!armoury_has_devstate(armoury_attr_groups[i].wmi_devid))
>  			continue;
>  
> -		err = sysfs_create_group(&asus_armoury.fw_attr_kset->kobj,
> -					 armoury_attr_groups[i].attr_group);
> -		if (err) {
> -			pr_err("Failed to create sysfs-group for %s\n",
> -			       armoury_attr_groups[i].attr_group->name);
> -			goto err_remove_groups;
> +		/* Always create by default, unless PPT is not present */
> +		should_create = true;
> +		name = armoury_attr_groups[i].attr_group->name;
> +
> +		/* Check if this is a power-related tunable requiring limits */
> +		if (asus_armoury.rog_tunables[ASUS_ROG_TUNABLE_AC] &&
> +		    asus_armoury.rog_tunables[ASUS_ROG_TUNABLE_AC]->power_limits &&
> +		    is_power_tunable_attr(name)) {
> +			limits = asus_armoury.rog_tunables[ASUS_ROG_TUNABLE_AC]->power_limits;

Here too a local variable would have helped.

-- 
 i.

  reply	other threads:[~2025-10-30 14:44 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-30 13:03 [PATCH v16 0/9] platform/x86: Add asus-armoury driver Denis Benato
2025-10-30 13:03 ` [PATCH v16 1/9] platform/x86: asus-wmi: export symbols used for read/write WMI Denis Benato
2025-10-30 13:03 ` [PATCH v16 2/9] platform/x86: asus-armoury: move existing tunings to asus-armoury module Denis Benato
2025-10-30 14:16   ` Ilpo Järvinen
2025-11-01 13:15     ` Denis Benato
2025-10-31  3:45   ` kernel test robot
2025-10-30 13:03 ` [PATCH v16 3/9] platform/x86: asus-armoury: add panel_hd_mode attribute Denis Benato
2025-10-30 13:03 ` [PATCH v16 4/9] platform/x86: asus-armoury: add apu-mem control support Denis Benato
2025-10-30 13:03 ` [PATCH v16 5/9] platform/x86: asus-armoury: add core count control Denis Benato
2025-10-30 13:03 ` [PATCH v16 6/9] platform/x86: asus-armoury: add screen auto-brightness toggle Denis Benato
2025-10-30 13:03 ` [PATCH v16 7/9] platform/x86: asus-wmi: deprecate bios features Denis Benato
2025-10-30 13:03 ` [PATCH v16 8/9] platform/x86: asus-wmi: rename ASUS_WMI_DEVID_PPT_FPPT Denis Benato
2025-10-30 13:03 ` [PATCH v16 9/9] platform/x86: asus-armoury: add ppt_* and nv_* tuning knobs Denis Benato
2025-10-30 14:44   ` Ilpo Järvinen [this message]
2025-10-31  2:40   ` Matthew Schwartz
2025-10-30 13:10 ` [PATCH v16 0/9] platform/x86: Add asus-armoury driver Denis Benato

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=91cddea1-b5be-2bcc-cf64-7ac102c14aca@linux.intel.com \
    --to=ilpo.jarvinen@linux.intel.com \
    --cc=alok.a.tiwari@oracle.com \
    --cc=benato.denis96@gmail.com \
    --cc=denis.benato@linux.dev \
    --cc=derekjohn.clark@gmail.com \
    --cc=dragonn@op.pl \
    --cc=hdegoede@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luke@ljones.dev \
    --cc=mario.limonciello@amd.com \
    --cc=platform-driver-x86@vger.kernel.org \
    --cc=porfet828@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.