Laptops with any of the ppt or nv tunables default to the minimum setting
on boot so we can safely assume a stored value is correct.
This patch adds storing of those values in the local struct, and enables
reading of those values back.
Secondary to the above it renames some internal variables to be more
consistent (which makes code grepping show all related parts)
Signed-off-by: Luke D. Jones <[email protected]>
---
drivers/platform/x86/asus-wmi.c | 141 +++++++++++++++++++++++++-------
1 file changed, 111 insertions(+), 30 deletions(-)
diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index e4341abb71e0..482e23b55e1e 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -272,12 +272,19 @@ struct asus_wmi {
/* Tunables provided by ASUS for gaming laptops */
bool ppt_pl2_sppt_available;
+ u32 ppt_pl2_sppt;
bool ppt_pl1_spl_available;
+ u32 ppt_pl1_spl;
bool ppt_apu_sppt_available;
- bool ppt_plat_sppt_available;
+ u32 ppt_apu_sppt;
+ bool ppt_platform_sppt_available;
+ u32 ppt_platform_sppt;
bool ppt_fppt_available;
- bool nv_dyn_boost_available;
- bool nv_temp_tgt_available;
+ u32 ppt_fppt;
+ bool nv_dynamic_boost_available;
+ u32 nv_dynamic_boost;
+ bool nv_temp_target_available;
+ u32 nv_temp_target;
bool kbd_rgb_mode_available;
u32 kbd_rgb_dev;
@@ -999,11 +1006,10 @@ static ssize_t ppt_pl2_sppt_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
{
+ struct asus_wmi *asus = dev_get_drvdata(dev);
int result, err;
u32 value;
- struct asus_wmi *asus = dev_get_drvdata(dev);
-
result = kstrtou32(buf, 10, &value);
if (result)
return result;
@@ -1022,22 +1028,31 @@ static ssize_t ppt_pl2_sppt_store(struct device *dev,
return -EIO;
}
+ asus->ppt_pl2_sppt = value;
sysfs_notify(&asus->platform_device->dev.kobj, NULL, "ppt_pl2_sppt");
return count;
}
-static DEVICE_ATTR_WO(ppt_pl2_sppt);
+
+static ssize_t ppt_pl2_sppt_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct asus_wmi *asus = dev_get_drvdata(dev);
+
+ return sysfs_emit(buf, "%d\n", asus->ppt_pl2_sppt);
+}
+static DEVICE_ATTR_RW(ppt_pl2_sppt);
/* Tunable: PPT, Intel=PL1, AMD=SPL ******************************************/
static ssize_t ppt_pl1_spl_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
{
+ struct asus_wmi *asus = dev_get_drvdata(dev);
int result, err;
u32 value;
- struct asus_wmi *asus = dev_get_drvdata(dev);
-
result = kstrtou32(buf, 10, &value);
if (result)
return result;
@@ -1056,22 +1071,30 @@ static ssize_t ppt_pl1_spl_store(struct device *dev,
return -EIO;
}
+ asus->ppt_pl1_spl = value;
sysfs_notify(&asus->platform_device->dev.kobj, NULL, "ppt_pl1_spl");
return count;
}
-static DEVICE_ATTR_WO(ppt_pl1_spl);
+static ssize_t ppt_pl1_spl_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct asus_wmi *asus = dev_get_drvdata(dev);
+
+ return sysfs_emit(buf, "%d\n", asus->ppt_pl1_spl);
+}
+static DEVICE_ATTR_RW(ppt_pl1_spl);
/* Tunable: PPT APU FPPT ******************************************************/
static ssize_t ppt_fppt_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
{
+ struct asus_wmi *asus = dev_get_drvdata(dev);
int result, err;
u32 value;
- struct asus_wmi *asus = dev_get_drvdata(dev);
-
result = kstrtou32(buf, 10, &value);
if (result)
return result;
@@ -1090,22 +1113,31 @@ static ssize_t ppt_fppt_store(struct device *dev,
return -EIO;
}
+ asus->ppt_fppt = value;
sysfs_notify(&asus->platform_device->dev.kobj, NULL, "ppt_fpu_sppt");
return count;
}
-static DEVICE_ATTR_WO(ppt_fppt);
+
+static ssize_t ppt_fppt_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct asus_wmi *asus = dev_get_drvdata(dev);
+
+ return sysfs_emit(buf, "%d\n", asus->ppt_fppt);
+}
+static DEVICE_ATTR_RW(ppt_fppt);
/* Tunable: PPT APU SPPT *****************************************************/
static ssize_t ppt_apu_sppt_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
{
+ struct asus_wmi *asus = dev_get_drvdata(dev);
int result, err;
u32 value;
- struct asus_wmi *asus = dev_get_drvdata(dev);
-
result = kstrtou32(buf, 10, &value);
if (result)
return result;
@@ -1124,22 +1156,31 @@ static ssize_t ppt_apu_sppt_store(struct device *dev,
return -EIO;
}
+ asus->ppt_apu_sppt = value;
sysfs_notify(&asus->platform_device->dev.kobj, NULL, "ppt_apu_sppt");
return count;
}
-static DEVICE_ATTR_WO(ppt_apu_sppt);
+
+static ssize_t ppt_apu_sppt_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct asus_wmi *asus = dev_get_drvdata(dev);
+
+ return sysfs_emit(buf, "%d\n", asus->ppt_apu_sppt);
+}
+static DEVICE_ATTR_RW(ppt_apu_sppt);
/* Tunable: PPT platform SPPT ************************************************/
static ssize_t ppt_platform_sppt_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
{
+ struct asus_wmi *asus = dev_get_drvdata(dev);
int result, err;
u32 value;
- struct asus_wmi *asus = dev_get_drvdata(dev);
-
result = kstrtou32(buf, 10, &value);
if (result)
return result;
@@ -1158,22 +1199,31 @@ static ssize_t ppt_platform_sppt_store(struct device *dev,
return -EIO;
}
+ asus->ppt_platform_sppt = value;
sysfs_notify(&asus->platform_device->dev.kobj, NULL, "ppt_platform_sppt");
return count;
}
-static DEVICE_ATTR_WO(ppt_platform_sppt);
+
+static ssize_t ppt_platform_sppt_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct asus_wmi *asus = dev_get_drvdata(dev);
+
+ return sysfs_emit(buf, "%d\n", asus->ppt_platform_sppt);
+}
+static DEVICE_ATTR_RW(ppt_platform_sppt);
/* Tunable: NVIDIA dynamic boost *********************************************/
static ssize_t nv_dynamic_boost_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
{
+ struct asus_wmi *asus = dev_get_drvdata(dev);
int result, err;
u32 value;
- struct asus_wmi *asus = dev_get_drvdata(dev);
-
result = kstrtou32(buf, 10, &value);
if (result)
return result;
@@ -1192,22 +1242,31 @@ static ssize_t nv_dynamic_boost_store(struct device *dev,
return -EIO;
}
+ asus->nv_dynamic_boost = value;
sysfs_notify(&asus->platform_device->dev.kobj, NULL, "nv_dynamic_boost");
return count;
}
-static DEVICE_ATTR_WO(nv_dynamic_boost);
+
+static ssize_t nv_dynamic_boost_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct asus_wmi *asus = dev_get_drvdata(dev);
+
+ return sysfs_emit(buf, "%d\n", asus->nv_dynamic_boost);
+}
+static DEVICE_ATTR_RW(nv_dynamic_boost);
/* Tunable: NVIDIA temperature target ****************************************/
static ssize_t nv_temp_target_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
{
+ struct asus_wmi *asus = dev_get_drvdata(dev);
int result, err;
u32 value;
- struct asus_wmi *asus = dev_get_drvdata(dev);
-
result = kstrtou32(buf, 10, &value);
if (result)
return result;
@@ -1226,11 +1285,21 @@ static ssize_t nv_temp_target_store(struct device *dev,
return -EIO;
}
+ asus->nv_temp_target = value;
sysfs_notify(&asus->platform_device->dev.kobj, NULL, "nv_temp_target");
return count;
}
-static DEVICE_ATTR_WO(nv_temp_target);
+
+static ssize_t nv_temp_target_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct asus_wmi *asus = dev_get_drvdata(dev);
+
+ return sysfs_emit(buf, "%d\n", asus->nv_temp_target);
+}
+static DEVICE_ATTR_RW(nv_temp_target);
/* Battery ********************************************************************/
@@ -4301,11 +4370,11 @@ static umode_t asus_sysfs_is_visible(struct kobject *kobj,
else if (attr == &dev_attr_ppt_apu_sppt.attr)
ok = asus->ppt_apu_sppt_available;
else if (attr == &dev_attr_ppt_platform_sppt.attr)
- ok = asus->ppt_plat_sppt_available;
+ ok = asus->ppt_platform_sppt_available;
else if (attr == &dev_attr_nv_dynamic_boost.attr)
- ok = asus->nv_dyn_boost_available;
+ ok = asus->nv_dynamic_boost_available;
else if (attr == &dev_attr_nv_temp_target.attr)
- ok = asus->nv_temp_tgt_available;
+ ok = asus->nv_temp_target_available;
else if (attr == &dev_attr_boot_sound.attr)
ok = asus->boot_sound_available;
else if (attr == &dev_attr_panel_od.attr)
@@ -4566,6 +4635,15 @@ static int asus_wmi_add(struct platform_device *pdev)
if (err)
goto fail_platform;
+ /* ensure defaults for tunables */
+ asus->ppt_pl2_sppt = 5;
+ asus->ppt_pl1_spl = 5;
+ asus->ppt_apu_sppt = 5;
+ asus->ppt_platform_sppt = 5;
+ asus->ppt_fppt = 5;
+ asus->nv_dynamic_boost = 5;
+ asus->nv_temp_target = 75;
+
asus->charge_mode_available = asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_CHARGE_MODE);
asus->egpu_enable_available = asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_EGPU);
asus->egpu_connect_available = asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_EGPU_CONNECTED);
@@ -4576,9 +4654,12 @@ static int asus_wmi_add(struct platform_device *pdev)
asus->ppt_pl1_spl_available = asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_PPT_PL1_SPL);
asus->ppt_fppt_available = asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_PPT_FPPT);
asus->ppt_apu_sppt_available = asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_PPT_APU_SPPT);
- asus->ppt_plat_sppt_available = asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_PPT_PLAT_SPPT);
- asus->nv_dyn_boost_available = asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_NV_DYN_BOOST);
- asus->nv_temp_tgt_available = asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_NV_THERM_TARGET);
+ asus->ppt_platform_sppt_available = asus_wmi_dev_is_present(asus,
+ ASUS_WMI_DEVID_PPT_PLAT_SPPT);
+ asus->nv_dynamic_boost_available = asus_wmi_dev_is_present(asus,
+ ASUS_WMI_DEVID_NV_DYN_BOOST);
+ asus->nv_temp_target_available = asus_wmi_dev_is_present(asus,
+ ASUS_WMI_DEVID_NV_THERM_TARGET);
asus->boot_sound_available = asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_BOOT_SOUND);
asus->panel_overdrive_available = asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_PANEL_OD);
asus->ally_mcu_usb_switch = acpi_has_method(NULL, ASUS_USB0_PWR_EC0_CSEE)
--
2.44.0
On Mon, 11 Mar 2024, Luke D. Jones wrote:
> Laptops with any of the ppt or nv tunables default to the minimum setting
> on boot so we can safely assume a stored value is correct.
>
> This patch adds storing of those values in the local struct, and enables
> reading of those values back.
>
> Secondary to the above it renames some internal variables to be more
> consistent (which makes code grepping show all related parts)
>
> Signed-off-by: Luke D. Jones <[email protected]>
> ---
> drivers/platform/x86/asus-wmi.c | 141 +++++++++++++++++++++++++-------
> 1 file changed, 111 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> index e4341abb71e0..482e23b55e1e 100644
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
> @@ -272,12 +272,19 @@ struct asus_wmi {
>
> /* Tunables provided by ASUS for gaming laptops */
> bool ppt_pl2_sppt_available;
> + u32 ppt_pl2_sppt;
> bool ppt_pl1_spl_available;
> + u32 ppt_pl1_spl;
> bool ppt_apu_sppt_available;
> - bool ppt_plat_sppt_available;
> + u32 ppt_apu_sppt;
> + bool ppt_platform_sppt_available;
> + u32 ppt_platform_sppt;
> bool ppt_fppt_available;
> - bool nv_dyn_boost_available;
> - bool nv_temp_tgt_available;
> + u32 ppt_fppt;
> + bool nv_dynamic_boost_available;
> + u32 nv_dynamic_boost;
> + bool nv_temp_target_available;
> + u32 nv_temp_target;
>
> bool kbd_rgb_mode_available;
> u32 kbd_rgb_dev;
Can you check with pahole if this structure is now full of 31-bit holes?
The benefit of keeping bool & u32 doesn't seem that big to begin with
(in visual sense because of the 1 char variation in column).
> @@ -999,11 +1006,10 @@ static ssize_t ppt_pl2_sppt_store(struct device *dev,
> struct device_attribute *attr,
> const char *buf, size_t count)
> {
> + struct asus_wmi *asus = dev_get_drvdata(dev);
> int result, err;
> u32 value;
>
> - struct asus_wmi *asus = dev_get_drvdata(dev);
> -
Please put this into own patch, it's entirely unrelated (but still useful
change!).
> result = kstrtou32(buf, 10, &value);
> if (result)
> return result;
> @@ -1022,22 +1028,31 @@ static ssize_t ppt_pl2_sppt_store(struct device *dev,
> return -EIO;
> }
>
> + asus->ppt_pl2_sppt = value;
> sysfs_notify(&asus->platform_device->dev.kobj, NULL, "ppt_pl2_sppt");
>
> return count;
> }
> -static DEVICE_ATTR_WO(ppt_pl2_sppt);
> +
> +static ssize_t ppt_pl2_sppt_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
Alignment is not correct?
> +{
> + struct asus_wmi *asus = dev_get_drvdata(dev);
> +
> + return sysfs_emit(buf, "%d\n", asus->ppt_pl2_sppt);
> +}
> +static DEVICE_ATTR_RW(ppt_pl2_sppt);
>
> /* Tunable: PPT, Intel=PL1, AMD=SPL ******************************************/
> static ssize_t ppt_pl1_spl_store(struct device *dev,
> struct device_attribute *attr,
> const char *buf, size_t count)
> {
> + struct asus_wmi *asus = dev_get_drvdata(dev);
> int result, err;
> u32 value;
>
> - struct asus_wmi *asus = dev_get_drvdata(dev);
> -
Unrelated, put to the same move patch as the other change please. I won't
mark all thse from this point on but please do them all.
--
i.
On Thu, 21 Mar 2024, at 2:13 AM, Ilpo Järvinen wrote:
> On Mon, 11 Mar 2024, Luke D. Jones wrote:
>
> > Laptops with any of the ppt or nv tunables default to the minimum setting
> > on boot so we can safely assume a stored value is correct.
> >
> > This patch adds storing of those values in the local struct, and enables
> > reading of those values back.
> >
> > Secondary to the above it renames some internal variables to be more
> > consistent (which makes code grepping show all related parts)
> >
> > Signed-off-by: Luke D. Jones <[email protected]>
> > ---
> > drivers/platform/x86/asus-wmi.c | 141 +++++++++++++++++++++++++-------
> > 1 file changed, 111 insertions(+), 30 deletions(-)
> >
> > diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> > index e4341abb71e0..482e23b55e1e 100644
> > --- a/drivers/platform/x86/asus-wmi.c
> > +++ b/drivers/platform/x86/asus-wmi.c
> > @@ -272,12 +272,19 @@ struct asus_wmi {
> >
> > /* Tunables provided by ASUS for gaming laptops */
> > bool ppt_pl2_sppt_available;
> > + u32 ppt_pl2_sppt;
> > bool ppt_pl1_spl_available;
> > + u32 ppt_pl1_spl;
> > bool ppt_apu_sppt_available;
> > - bool ppt_plat_sppt_available;
> > + u32 ppt_apu_sppt;
> > + bool ppt_platform_sppt_available;
> > + u32 ppt_platform_sppt;
> > bool ppt_fppt_available;
> > - bool nv_dyn_boost_available;
> > - bool nv_temp_tgt_available;
> > + u32 ppt_fppt;
> > + bool nv_dynamic_boost_available;
> > + u32 nv_dynamic_boost;
> > + bool nv_temp_target_available;
> > + u32 nv_temp_target;
> >
> > bool kbd_rgb_mode_available;
> > u32 kbd_rgb_dev;
>
> Can you check with pahole if this structure is now full of 31-bit holes?
>
> The benefit of keeping bool & u32 doesn't seem that big to begin with
> (in visual sense because of the 1 char variation in column).
I don't really understand what you mean here. I do most of this stuff when I have time, so because my time is limited I usually don't have opportunity to learn much more than the minimum.
On Thu, 21 Mar 2024, Luke Jones wrote:
>
>
> On Thu, 21 Mar 2024, at 2:13 AM, Ilpo Järvinen wrote:
> > On Mon, 11 Mar 2024, Luke D. Jones wrote:
> >
> > > Laptops with any of the ppt or nv tunables default to the minimum setting
> > > on boot so we can safely assume a stored value is correct.
> > >
> > > This patch adds storing of those values in the local struct, and enables
> > > reading of those values back.
> > >
> > > Secondary to the above it renames some internal variables to be more
> > > consistent (which makes code grepping show all related parts)
> > >
> > > Signed-off-by: Luke D. Jones <[email protected]>
> > > ---
> > > drivers/platform/x86/asus-wmi.c | 141 +++++++++++++++++++++++++-------
> > > 1 file changed, 111 insertions(+), 30 deletions(-)
> > >
> > > diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> > > index e4341abb71e0..482e23b55e1e 100644
> > > --- a/drivers/platform/x86/asus-wmi.c
> > > +++ b/drivers/platform/x86/asus-wmi.c
> > > @@ -272,12 +272,19 @@ struct asus_wmi {
> > >
> > > /* Tunables provided by ASUS for gaming laptops */
> > > bool ppt_pl2_sppt_available;
> > > + u32 ppt_pl2_sppt;
> > > bool ppt_pl1_spl_available;
> > > + u32 ppt_pl1_spl;
> > > bool ppt_apu_sppt_available;
> > > - bool ppt_plat_sppt_available;
> > > + u32 ppt_apu_sppt;
> > > + bool ppt_platform_sppt_available;
> > > + u32 ppt_platform_sppt;
> > > bool ppt_fppt_available;
> > > - bool nv_dyn_boost_available;
> > > - bool nv_temp_tgt_available;
> > > + u32 ppt_fppt;
> > > + bool nv_dynamic_boost_available;
> > > + u32 nv_dynamic_boost;
> > > + bool nv_temp_target_available;
> > > + u32 nv_temp_target;
> > >
> > > bool kbd_rgb_mode_available;
> > > u32 kbd_rgb_dev;
> >
> > Can you check with pahole if this structure is now full of 31-bit holes?
> >
> > The benefit of keeping bool & u32 doesn't seem that big to begin with
> > (in visual sense because of the 1 char variation in column).
>
> I don't really understand what you mean here. I do most of this stuff
> when I have time, so because my time is limited I usually don't have
> opportunity to learn much more than the minimum.
pahole is a tool with which you can look into struct layout. Compile the
driver with debug info on, and run pahole on the object file, then check
the layout of the members of that struct (mainly holes in this case).
So enable the debug info in .config if not yet enabled and then run:
$ make drivers/platform/x86/asus-wmi.o
$ pahole -C asus_wmi drivers/platform/x86/asus-wmi.o
--
i.