[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f352e00c-50d9-4c13-941c-d6e254c44072@gmail.com>
Date: Mon, 13 Oct 2025 21:50:46 +0200
From: Denis Benato <benato.denis96@...il.com>
To: ALOK TIWARI <alok.a.tiwari@...cle.com>, linux-kernel@...r.kernel.org
Cc: platform-driver-x86@...r.kernel.org, Hans de Goede <hdegoede@...hat.com>,
Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
"Limonciello, Mario" <mario.limonciello@....com>,
"Luke D . Jones" <luke@...nes.dev>,
Derek John Clark <derekjohn.clark@...il.com>,
Mateusz Schyboll <dragonn@...pl>, porfet828@...il.com
Subject: Re: [External] : [PATCH v13 8/8] platform/x86: asus-armoury: add
ppt_* and nv_* tuning knobs
On 10/13/25 20:25, ALOK TIWARI wrote:
>
>
> On 10/13/2025 11:35 PM, Denis Benato wrote:
>> From: "Luke D. Jones" <luke@...nes.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.
>>
>> Signed-off-by: Denis Benato <benato.denis96@...il.com>
>> Signed-off-by: Luke D. Jones <luke@...nes.dev>
>> Reviewed-by: Mario Limonciello <mario.limonciello@....com>
>> Tested-by: Mateusz Schyboll <dragonn@...pl>
>> ---
>> drivers/platform/x86/asus-armoury.c | 296 ++++-
>> drivers/platform/x86/asus-armoury.h | 1210 ++++++++++++++++++++
>> include/linux/platform_data/x86/asus-wmi.h | 3 +
>> 3 files changed, 1503 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/platform/x86/asus-armoury.c b/drivers/platform/x86/asus-armoury.c
>> index e27f964aebf8..918aea6fba1e 100644
>> --- a/drivers/platform/x86/asus-armoury.c
>> +++ b/drivers/platform/x86/asus-armoury.c
>> @@ -27,6 +27,7 @@
>> #include <linux/mutex.h>
>> #include <linux/platform_data/x86/asus-wmi.h>
>> #include <linux/printk.h>
>> +#include <linux/power_supply.h>
>> #include <linux/types.h>
>> #include "asus-armoury.h"
>> @@ -45,6 +46,17 @@
>> #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)
>> @@ -73,11 +85,26 @@ 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;
>> +};
>> +
>> static struct asus_armoury_priv {
>> struct device *fw_attr_dev;
>> struct kset *fw_attr_kset;
>> struct cpu_cores *cpu_cores;
>> + /* Index 0 for DC, 1 for AC */
>> + struct rog_tunables *rog_tunables[2];
>> u32 mini_led_dev_id;
>> u32 gpu_mux_dev_id;
>> /*
>> @@ -719,7 +746,34 @@ static ssize_t cores_efficiency_current_value_store(struct kobject *kobj,
>> ATTR_GROUP_CORES_RW(cores_efficiency, "cores_efficiency",
>> "Set the max available efficiency cores");
>> +/* Define helper to access the current power mode tunable values */
>> +static inline struct rog_tunables *get_current_tunables(void)
>> +{
>> + return asus_armoury
>> + .rog_tunables[power_supply_is_system_supplied() ? 1 : 0];
>> +}
>> +
>> /* Simple attribute creation */
>> +ATTR_GROUP_ROG_TUNABLE(ppt_pl1_spl, ATTR_PPT_PL1_SPL, ASUS_WMI_DEVID_PPT_PL1_SPL,
>> + "Set the CPU slow package limit");
>> +ATTR_GROUP_ROG_TUNABLE(ppt_pl2_sppt, ATTR_PPT_PL2_SPPT, ASUS_WMI_DEVID_PPT_PL2_SPPT,
>> + "Set the CPU fast package limit");
>> +ATTR_GROUP_ROG_TUNABLE(ppt_pl3_fppt, ATTR_PPT_PL3_FPPT, ASUS_WMI_DEVID_PPT_FPPT,
>
> why not ASUS_WMI_DEVID_PPT_PL3_FPPT ?
>
I simply didn't touch anything that was not brought up, but I see that it appears to be a more consistent name.
Will use that name for v14, thanks!
>> + "Set the CPU fastest package limit");
>> +ATTR_GROUP_ROG_TUNABLE(ppt_apu_sppt, ATTR_PPT_APU_SPPT, ASUS_WMI_DEVID_PPT_APU_SPPT,
>> + "Set the APU package limit");
>> +ATTR_GROUP_ROG_TUNABLE(ppt_platform_sppt, ATTR_PPT_PLATFORM_SPPT, ASUS_WMI_DEVID_PPT_PLAT_SPPT,
>> + "Set the platform package limit");
>> +ATTR_GROUP_ROG_TUNABLE(nv_dynamic_boost, ATTR_NV_DYNAMIC_BOOST, ASUS_WMI_DEVID_NV_DYN_BOOST,
>> + "Set the Nvidia dynamic boost limit");
>> +ATTR_GROUP_ROG_TUNABLE(nv_temp_target, ATTR_NV_TEMP_TARGET, ASUS_WMI_DEVID_NV_THERM_TARGET,
>> + "Set the Nvidia max thermal limit");
>> +ATTR_GROUP_ROG_TUNABLE(nv_tgp, "nv_tgp", ASUS_WMI_DEVID_DGPU_SET_TGP,
>> + "Set the additional TGP on top of the base TGP");
>> +ATTR_GROUP_INT_VALUE_ONLY_RO(nv_base_tgp, ATTR_NV_BASE_TGP, ASUS_WMI_DEVID_DGPU_BASE_TGP,
>> + "Read the base TGP value");
>> +
>> +
>> ATTR_GROUP_ENUM_INT_RO(charge_mode, "charge_mode", ASUS_WMI_DEVID_CHARGE_MODE, "0;1;2",
>> "Show the current mode of charging");
>> @@ -746,6 +800,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_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 },
>> @@ -754,8 +818,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),
>> @@ -812,12 +943,30 @@ static int asus_fw_attr_add(void)
>> if (!asus_wmi_is_present(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[1] && asus_armoury.rog_tunables[1]->power_limits &&
>> + is_power_tunable_attr(name)) {
>> + limits = asus_armoury.rog_tunables[1]->power_limits;
>> + /* Check only AC, if DC is not present then AC won't be either */
>> + should_create = has_valid_limit(name, limits);
>> + if (!should_create) {
>> + pr_debug("Missing max value on %s for tunable: %s\n",
>> + dmi_get_system_info(DMI_BOARD_NAME), name);
>
> dmi_get_system_info can return NULL
ouch! v14 here I come.
>
>> + }
>> + }
>> +
>> + if (should_create) {
>> + 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;
>> + }
>> }
>> }
>>
> Thanks,
> Alok
Thanks,
Denis B.
Powered by blists - more mailing lists