linux-kernel - Re: [External] : [PATCH v13 8/8] platform/x86: asus-armoury: add ppt_* and nv

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <926a5d91-e69f-cecb-90c1-64b83424b1a2@linux.intel.com>
Date: Tue, 14 Oct 2025 14:11:35 +0300 (EEST)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: Denis Benato <benato.denis96@...il.com>
cc: ALOK TIWARI <alok.a.tiwari@...cle.com>, 
    LKML <linux-kernel@...r.kernel.org>, platform-driver-x86@...r.kernel.org, 
    Hans de Goede <hdegoede@...hat.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 Tue, 14 Oct 2025, Denis Benato wrote:

> 
> On 10/13/25 21:50, Denis Benato wrote:
> > 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!
> Unfortunately taking a closer look I discovered that macro has been introduced over
> 2 years ago in commit e0b278e7b5da62c3ebb156a8b7d76a739da2d953
> "platform/x86: asus-wmi: expose dGPU and CPU tunables for ROG"
> and it is not introduced as part of this commit series.
> 
> I think it would be best to create an ah-hoc commit when this driver is merged to change the name in both,
> or do I send the name change now and rework this driver? what do you think?
> 
> Honestly given the large number of people already running this and the request of having it upstream
> the road that will make it merge sooner is the one I would like to take.

Just add a patch to the series to rename the existing define. Such a 
simple patch certainly isn't going to delay the rest, it's the large 
patches which take huge amounts of time to go through.


-- 
 i.