| 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
| ||
|
Message-ID: <CALiyAomC-QXuMWs2i50SK2f=F7Te5QTbx1f1i-MWhF=STLmmEw@mail.gmail.com>
Date: Sun, 5 Jan 2025 16:49:30 +0530
From: Hridesh MG <hridesh699@...il.com>
To: Kurt Borja <kuurtb@...il.com>
Cc: Hans de Goede <hdegoede@...hat.com>, Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
Armin Wolf <W_Armin@....de>, platform-driver-x86@...r.kernel.org,
linux-kernel@...r.kernel.org, Shuah Khan <skhan@...uxfoundation.org>
Subject: Re: [PATCH v2 1/3] platform/x86: acer-wmi: use WMI calls for platform
profile handling
On Sat, Jan 4, 2025 at 10:43 PM Kurt Borja <kuurtb@...il.com> wrote:
>
> On Sat, Jan 04, 2025 at 08:59:20PM +0530, Hridesh MG wrote:
> > Improve the platform profile handling by using WMI calls to fetch the
> > current platform profile instead of directly accessing it from the EC.
> > This is beneficial because the EC address differs for certain laptops.
> >
> > Link: https://lore.kernel.org/platform-driver-x86/d7be714c-3103-42ee-ad15-223a3fe67f80@gmx.de/
> > Co-developed-by: Armin Wolf <W_Armin@....de>
> > Signed-off-by: Armin Wolf <W_Armin@....de>
> > Signed-off-by: Hridesh MG <hridesh699@...il.com>
>
> Hi Hridesh,
>
> > ---
> > drivers/platform/x86/acer-wmi.c | 189 ++++++++++++++++++++++++++++------------
> > 1 file changed, 133 insertions(+), 56 deletions(-)
> >
> > diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
> > index b3043d78a7b38a7b773da5ecd4846ca11e8595f5..5370056fb2d03a768162f2f1643ef27dc6deafa8 100644
> > --- a/drivers/platform/x86/acer-wmi.c
> > +++ b/drivers/platform/x86/acer-wmi.c
> > @@ -31,6 +31,7 @@
> > #include <acpi/video.h>
> > #include <linux/hwmon.h>
> > #include <linux/units.h>
> > +#include <linux/unaligned.h>
> > #include <linux/bitfield.h>
> >
> > MODULE_AUTHOR("Carlos Corbacho");
> > @@ -68,8 +69,11 @@ MODULE_LICENSE("GPL");
> > #define ACER_WMID_GET_GAMING_SYS_INFO_METHODID 5
> > #define ACER_WMID_SET_GAMING_FAN_BEHAVIOR 14
> > #define ACER_WMID_SET_GAMING_MISC_SETTING_METHODID 22
> > +#define ACER_WMID_GET_GAMING_MISC_SETTING_METHODID 23
> >
> > -#define ACER_PREDATOR_V4_THERMAL_PROFILE_EC_OFFSET 0x54
> > +#define ACER_GAMING_MISC_SETTING_STATUS_MASK GENMASK_ULL(7, 0)
> > +#define ACER_GAMING_MISC_SETTING_INDEX_MASK GENMASK_ULL(7, 0)
> > +#define ACER_GAMING_MISC_SETTING_VALUE_MASK GENMASK_ULL(15, 8)
> >
> > #define ACER_PREDATOR_V4_RETURN_STATUS_BIT_MASK GENMASK_ULL(7, 0)
> > #define ACER_PREDATOR_V4_SENSOR_INDEX_BIT_MASK GENMASK_ULL(15, 8)
> > @@ -115,6 +119,13 @@ enum acer_wmi_predator_v4_sensor_id {
> > ACER_WMID_SENSOR_GPU_TEMPERATURE = 0x0A,
> > };
> >
> > +enum acer_wmi_gaming_misc_setting {
> > + ACER_WMID_MISC_SETTING_OC_1 = 0x0005,
> > + ACER_WMID_MISC_SETTING_OC_2 = 0x0007,
>
> These OC settings should be added only if you add support for them.
>
> I noticed acer_toggle_turbo() uses these settings. For consistency, I
> think it should be refactored to use WMID_gaming_set_misc_setting()
> instead of WMID_gaming_set_u64().
Yeah I agree. Actually, now that we have this function, this
particular case in WMID_gaming_set_u64() is redundant, so can I remove
it? (sorry if this is a dumb question)
switch (cap) {
case ACER_CAP_TURBO_OC:
method_id = ACER_WMID_SET_GAMING_MISC_SETTING_METHODID;
break;
}
> > + ACER_WMID_MISC_SETTING_SUPPORTED_PROFILES = 0x000A,
>
> This should be added in patch [2/3].
>
> > + ACER_WMID_MISC_SETTING_PLATFORM_PROFILE = 0x000B,
> > +};
> > +
> > static const struct key_entry acer_wmi_keymap[] __initconst = {
> > {KE_KEY, 0x01, {KEY_WLAN} }, /* WiFi */
> > {KE_KEY, 0x03, {KEY_WLAN} }, /* WiFi */
> > @@ -751,20 +762,12 @@ static bool platform_profile_support;
> > */
> > static int last_non_turbo_profile;
> >
> > -enum acer_predator_v4_thermal_profile_ec {
> > - ACER_PREDATOR_V4_THERMAL_PROFILE_ECO = 0x04,
> > - ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO = 0x03,
> > - ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE = 0x02,
> > - ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET = 0x01,
> > - ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED = 0x00,
> > -};
> > -
> > -enum acer_predator_v4_thermal_profile_wmi {
> > - ACER_PREDATOR_V4_THERMAL_PROFILE_ECO_WMI = 0x060B,
> > - ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO_WMI = 0x050B,
> > - ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE_WMI = 0x040B,
> > - ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET_WMI = 0x0B,
> > - ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED_WMI = 0x010B,
> > +enum acer_predator_v4_thermal_profile {
> > + ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET = 0x00,
> > + ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED = 0x01,
> > + ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE = 0x04,
> > + ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO = 0x05,
> > + ACER_PREDATOR_V4_THERMAL_PROFILE_ECO = 0x06,
> > };
> >
> > /* Find which quirks are needed for a particular vendor/ model pair */
> > @@ -1477,6 +1480,45 @@ WMI_gaming_execute_u64(u32 method_id, u64 in, u64 *out)
> > return status;
> > }
> >
> > +static int WMI_gaming_execute_u32_u64(u32 method_id, u32 in, u64 *out)
> > +{
> > + struct acpi_buffer result = { ACPI_ALLOCATE_BUFFER, NULL };
> > + struct acpi_buffer input = {
> > + .length = sizeof(in),
> > + .pointer = &in,
> > + };
> > + union acpi_object *obj;
> > + acpi_status status;
> > + int ret = 0;
> > +
> > + status = wmi_evaluate_method(WMID_GUID4, 0, method_id, &input, &result);
> > + if (ACPI_FAILURE(status))
> > + return -EIO;
> > +
> > + obj = result.pointer;
> > + if (obj && out) {
> > + switch (obj->type) {
> > + case ACPI_TYPE_INTEGER:
> > + *out = obj->integer.value;
> > + break;
> > + case ACPI_TYPE_BUFFER:
> > + if (obj->buffer.length < sizeof(*out))
> > + ret = -ENOMSG;
> > + else
> > + *out = get_unaligned_le64(obj->buffer.pointer);
> > +
> > + break;
> > + default:
> > + ret = -ENOMSG;
> > + break;
> > + }
> > + }
> > +
> > + kfree(obj);
> > +
> > + return ret;
> > +}
> > +
> > static acpi_status WMID_gaming_set_u64(u64 value, u32 cap)
> > {
> > u32 method_id = 0;
> > @@ -1565,6 +1607,48 @@ static void WMID_gaming_set_fan_mode(u8 fan_mode)
> > WMID_gaming_set_u64(gpu_fan_config2 | gpu_fan_config1 << 16, ACER_CAP_TURBO_FAN);
> > }
> >
> > +static int WMID_gaming_set_misc_setting(enum acer_wmi_gaming_misc_setting setting, u8 value)
> > +{
> > + acpi_status status;
> > + u64 input = 0;
> > + u64 result;
> > +
> > + input |= FIELD_PREP(ACER_GAMING_MISC_SETTING_INDEX_MASK, setting);
> > + input |= FIELD_PREP(ACER_GAMING_MISC_SETTING_VALUE_MASK, value);
> > +
> > + status = WMI_gaming_execute_u64(ACER_WMID_SET_GAMING_MISC_SETTING_METHODID, input, &result);
> > + if (ACPI_FAILURE(status))
> > + return -EIO;
> > +
> > + /* The return status must be zero for the operation to have succeeded */
> > + if (FIELD_GET(ACER_GAMING_MISC_SETTING_STATUS_MASK, result))
> > + return -EIO;
> > +
> > + return 0;
> > +}
> > +
> > +static int WMID_gaming_get_misc_setting(enum acer_wmi_gaming_misc_setting setting, u8 *value)
> > +{
> > + u64 input = 0;
> > + u64 result;
> > + int ret;
> > +
> > + input |= FIELD_PREP(ACER_GAMING_MISC_SETTING_INDEX_MASK, setting);
> > +
> > + ret = WMI_gaming_execute_u32_u64(ACER_WMID_GET_GAMING_MISC_SETTING_METHODID, input,
> > + &result);
> > + if (ret < 0)
> > + return ret;
> > +
> > + /* The return status must be zero for the operation to have succeeded */
> > + if (FIELD_GET(ACER_GAMING_MISC_SETTING_STATUS_MASK, result))
> > + return -EIO;
> > +
> > + *value = FIELD_GET(ACER_GAMING_MISC_SETTING_VALUE_MASK, result);
> > +
> > + return 0;
> > +}
> > +
> > /*
> > * Generic Device (interface-independent)
> > */
> > @@ -1833,9 +1917,8 @@ acer_predator_v4_platform_profile_get(struct platform_profile_handler *pprof,
> > u8 tp;
> > int err;
> >
> > - err = ec_read(ACER_PREDATOR_V4_THERMAL_PROFILE_EC_OFFSET, &tp);
> > -
> > - if (err < 0)
> > + err = WMID_gaming_get_misc_setting(ACER_WMID_MISC_SETTING_PLATFORM_PROFILE, &tp);
> > + if (err)
> > return err;
> >
> > switch (tp) {
> > @@ -1865,36 +1948,33 @@ static int
> > acer_predator_v4_platform_profile_set(struct platform_profile_handler *pprof,
> > enum platform_profile_option profile)
> > {
> > - int tp;
> > - acpi_status status;
> > + int tp, err;
> >
> > switch (profile) {
> > case PLATFORM_PROFILE_PERFORMANCE:
> > - tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO_WMI;
> > + tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO;
> > break;
> > case PLATFORM_PROFILE_BALANCED_PERFORMANCE:
> > - tp = ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE_WMI;
> > + tp = ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE;
> > break;
> > case PLATFORM_PROFILE_BALANCED:
> > - tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED_WMI;
> > + tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
> > break;
> > case PLATFORM_PROFILE_QUIET:
> > - tp = ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET_WMI;
> > + tp = ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET;
> > break;
> > case PLATFORM_PROFILE_LOW_POWER:
> > - tp = ACER_PREDATOR_V4_THERMAL_PROFILE_ECO_WMI;
> > + tp = ACER_PREDATOR_V4_THERMAL_PROFILE_ECO;
> > break;
> > default:
> > return -EOPNOTSUPP;
> > }
> >
> > - status = WMI_gaming_execute_u64(
> > - ACER_WMID_SET_GAMING_MISC_SETTING_METHODID, tp, NULL);
> > -
> > - if (ACPI_FAILURE(status))
> > - return -EIO;
> > + err = WMID_gaming_set_misc_setting(ACER_WMID_MISC_SETTING_PLATFORM_PROFILE, tp);
> > + if (err)
> > + return err;
> >
> > - if (tp != ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO_WMI)
> > + if (tp != ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO)
> > last_non_turbo_profile = tp;
> >
> > return 0;
> > @@ -1923,6 +2003,7 @@ static int acer_platform_profile_setup(struct platform_device *device)
> > set_bit(PLATFORM_PROFILE_LOW_POWER,
> > platform_profile_handler.choices);
> >
> > +
>
> Please, drop this extra line.
>
> ~ Kurt
>
> > err = platform_profile_register(&platform_profile_handler);
> > if (err)
> > return err;
> > @@ -1931,7 +2012,7 @@ static int acer_platform_profile_setup(struct platform_device *device)
> >
> > /* Set default non-turbo profile */
> > last_non_turbo_profile =
> > - ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED_WMI;
> > + ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
> > }
> > return 0;
> > }
> > @@ -1946,12 +2027,10 @@ static int acer_thermal_profile_change(void)
> > u8 current_tp;
> > int tp, err;
> > u64 on_AC;
> > - acpi_status status;
> > -
> > - err = ec_read(ACER_PREDATOR_V4_THERMAL_PROFILE_EC_OFFSET,
> > - ¤t_tp);
> >
> > - if (err < 0)
> > + err = WMID_gaming_get_misc_setting(ACER_WMID_MISC_SETTING_PLATFORM_PROFILE,
> > + ¤t_tp);
> > + if (err)
> > return err;
> >
> > /* Check power source */
> > @@ -1962,54 +2041,52 @@ static int acer_thermal_profile_change(void)
> > switch (current_tp) {
> > case ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO:
> > if (!on_AC)
> > - tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED_WMI;
> > + tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
> > else if (cycle_gaming_thermal_profile)
> > - tp = ACER_PREDATOR_V4_THERMAL_PROFILE_ECO_WMI;
> > + tp = ACER_PREDATOR_V4_THERMAL_PROFILE_ECO;
> > else
> > tp = last_non_turbo_profile;
> > break;
> > case ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE:
> > if (!on_AC)
> > - tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED_WMI;
> > + tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
> > else
> > - tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO_WMI;
> > + tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO;
> > break;
> > case ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED:
> > if (!on_AC)
> > - tp = ACER_PREDATOR_V4_THERMAL_PROFILE_ECO_WMI;
> > + tp = ACER_PREDATOR_V4_THERMAL_PROFILE_ECO;
> > else if (cycle_gaming_thermal_profile)
> > - tp = ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE_WMI;
> > + tp = ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE;
> > else
> > - tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO_WMI;
> > + tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO;
> > break;
> > case ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET:
> > if (!on_AC)
> > - tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED_WMI;
> > + tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
> > else if (cycle_gaming_thermal_profile)
> > - tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED_WMI;
> > + tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
> > else
> > - tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO_WMI;
> > + tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO;
> > break;
> > case ACER_PREDATOR_V4_THERMAL_PROFILE_ECO:
> > if (!on_AC)
> > - tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED_WMI;
> > + tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
> > else if (cycle_gaming_thermal_profile)
> > - tp = ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET_WMI;
> > + tp = ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET;
> > else
> > - tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO_WMI;
> > + tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO;
> > break;
> > default:
> > return -EOPNOTSUPP;
> > }
> >
> > - status = WMI_gaming_execute_u64(
> > - ACER_WMID_SET_GAMING_MISC_SETTING_METHODID, tp, NULL);
> > -
> > - if (ACPI_FAILURE(status))
> > - return -EIO;
> > + err = WMID_gaming_set_misc_setting(ACER_WMID_MISC_SETTING_PLATFORM_PROFILE, tp);
> > + if (err)
> > + return err;
> >
> > /* Store non-turbo profile for turbo mode toggle*/
> > - if (tp != ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO_WMI)
> > + if (tp != ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO)
> > last_non_turbo_profile = tp;
> >
> > platform_profile_notify(&platform_profile_handler);
--
Thanks,
Hridesh MG
Powered by blists - more mailing lists