[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6f4bc109-00d0-47b0-a581-b96a6152545c@amd.com>
Date: Mon, 27 May 2024 21:19:10 -0500
From: "Limonciello, Mario" <mario.limonciello@....com>
To: "Luke D. Jones" <luke@...nes.dev>, hdegoede@...hat.com
Cc: ilpo.jarvinen@...ux.intel.com, corentin.chary@...il.com,
platform-driver-x86@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 8/9] platform/x86: asus-wmi: add apu_mem setting
On 5/27/2024 8:36 PM, Luke D. Jones wrote:
> Exposes the APU memory setting available on a few ASUS models such as
> the ROG Ally.
>
> Signed-off-by: Luke D. Jones <luke@...nes.dev>
> ---
> .../ABI/testing/sysfs-platform-asus-wmi | 8 ++
> drivers/platform/x86/asus-wmi.c | 109 ++++++++++++++++++
> include/linux/platform_data/x86/asus-wmi.h | 3 +
> 3 files changed, 120 insertions(+)
>
> diff --git a/Documentation/ABI/testing/sysfs-platform-asus-wmi b/Documentation/ABI/testing/sysfs-platform-asus-wmi
> index ac881e72e374..d221a3bc1a81 100644
> --- a/Documentation/ABI/testing/sysfs-platform-asus-wmi
> +++ b/Documentation/ABI/testing/sysfs-platform-asus-wmi
> @@ -245,3 +245,11 @@ Description:
> Show the maximum performance and efficiency core countin format
> 0x[E][P] where [E] is the efficiency core count, and [P] is
> the perfromance core count.
> +
> +What: /sys/devices/platform/<platform>/apu_mem
> +Date: Jun 2024
> +KernelVersion: 6.11
> +Contact: "Luke Jones" <luke@...nes.dev>
> +Description:
> + Set the maximum available system memory for the APU.
> + * Min=0, Max=8
What is the unit? It seems like multiples of something?
> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> index f62a36dfcd4b..4b5fbae8c563 100644
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
> @@ -855,6 +855,112 @@ static DEVICE_ATTR_RW(cores_enabled);
> WMI_SIMPLE_SHOW(cores_max, "0x%x\n", ASUS_WMI_DEVID_CORES_MAX);
> static DEVICE_ATTR_RO(cores_max);
>
> +/* Device memory available to APU */
> +
> +static ssize_t apu_mem_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct asus_wmi *asus = dev_get_drvdata(dev);
> + int err;
> + u32 mem;
> +
> + err = asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_APU_MEM, &mem);
> + if (err < 0)
> + return err;
> +
> + switch (mem) {
> + case 256:
> + mem = 0;
> + break;
> + case 258:
> + mem = 1;
> + break;
> + case 259:
> + mem = 2;
> + break;
> + case 260:
> + mem = 3;
> + break;
> + case 261:
> + mem = 4;
> + break;
> + case 262:
> + mem = 8;
> + break;
> + case 263:
> + mem = 5;
> + break;
> + case 264:
> + mem = 6;
> + break;
> + case 265:
> + mem = 7;
> + break;
> + default:
> + mem = 4;
> + break;
> + }
> +
> + return sysfs_emit(buf, "%d\n", mem);
> +}
> +
> +static ssize_t apu_mem_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 mem;
> +
> + result = kstrtou32(buf, 10, &mem);
> + if (result)
> + return result;
> +
> + switch (mem) {
> + case 0:
> + mem = 0;
> + break;
> + case 1:
> + mem = 258;
> + break;
> + case 2:
> + mem = 259;
> + break;
> + case 3:
> + mem = 260;
> + break;
> + case 4:
> + mem = 261;
> + break;
> + case 5:
> + mem = 263;
> + break;
> + case 6:
> + mem = 264;
> + break;
> + case 7:
> + mem = 265;
> + break;
> + case 8:
> + mem = 262;
Is case 8 a mistake, or intentionally out of order?
> + break;
> + default:
> + return -EIO;
> + }
> +
> + err = asus_wmi_set_devstate(ASUS_WMI_DEVID_APU_MEM, mem, &result);
> + if (err) {
> + pr_warn("Failed to set apu_mem: %d\n", err);
> + return err;
> + }
> +
> + pr_info("APU memory changed, reboot required\n");
If you're logging something into the logs for this, I'd say make it more
useful.
"APU memory changed to %d MB"
> + sysfs_notify(&asus->platform_device->dev.kobj, NULL, "apu_mem");
So this is a case that the BIOS attributes API I mentioned before would
be REALLY useful. There is a pending_reboot sysfs file that userspace
can query to know if a given setting requires a reboot or not.
Fwupd also uses this attribute to know to delay BIOS updates until the
system has been rebooted.
> +
> + return count;
> +}
> +static DEVICE_ATTR_RW(apu_mem);
> +
> /* Tablet mode ****************************************************************/
>
> static void asus_wmi_tablet_mode_get_state(struct asus_wmi *asus)
> @@ -4100,6 +4206,7 @@ static struct attribute *platform_attributes[] = {
> &dev_attr_panel_fhd.attr,
> &dev_attr_cores_enabled.attr,
> &dev_attr_cores_max.attr,
> + &dev_attr_apu_mem.attr,
> &dev_attr_mini_led_mode.attr,
> &dev_attr_available_mini_led_mode.attr,
> NULL
> @@ -4176,6 +4283,8 @@ static umode_t asus_sysfs_is_visible(struct kobject *kobj,
> else if (attr == &dev_attr_cores_enabled.attr
> || attr == &dev_attr_cores_max.attr)
> ok = asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_CORES_SET);
> + else if (attr == &dev_attr_apu_mem.attr)
> + ok = asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_APU_MEM);
> else if (attr == &dev_attr_mini_led_mode.attr)
> ok = asus->mini_led_dev_id != 0;
> else if (attr == &dev_attr_available_mini_led_mode.attr)
> diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
> index 5a56e7e97785..efe608861e55 100644
> --- a/include/linux/platform_data/x86/asus-wmi.h
> +++ b/include/linux/platform_data/x86/asus-wmi.h
> @@ -121,6 +121,9 @@
> /* Maximum Intel E-core and P-core availability */
> #define ASUS_WMI_DEVID_CORES_MAX 0x001200D3
>
> +/* Set the memory available to the APU */
> +#define ASUS_WMI_DEVID_APU_MEM 0x000600C1
> +
> /* MCU powersave mode */
> #define ASUS_WMI_DEVID_MCU_POWERSAVE 0x001200E2
>
Powered by blists - more mailing lists