Hi, Thank you for your patch. Some other changes have just landed to the hp-wmi driver, so for the next version please base your patch on my review-hans branch: https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans On 3/13/22 20:05, Enver Balalic wrote: > As it turns out, these laptops have 2 thermal profile versions. > A previous patch added support for v0, this patch adds support > for v1 thermal policies that are in use on some devices. > We obtain the thermal policy version by querying the get system > design data WMI call and looking at the fourth byte it returns, > except if the system board DMI Board ID is in a specific array > that the windows command center app overrides to thermal policy > v0 for some reason. > > Signed-off-by: Enver Balalic <balalic.enver@xxxxxxxxx> > --- > drivers/platform/x86/hp-wmi.c | 69 +++++++++++++++++++++++++++++++---- > 1 file changed, 62 insertions(+), 7 deletions(-) > > diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c > index 48a46466f086..ed5c729f8da1 100644 > --- a/drivers/platform/x86/hp-wmi.c > +++ b/drivers/platform/x86/hp-wmi.c > @@ -61,6 +61,14 @@ static const char * const omen_thermal_profile_boards[] = { > "8917", "8918", "8949", "894A", "89EB" > }; > > +/* DMI Board names of Omen laptops that are specifically set to be thermal > + * profile version 0 by the Omen Command Center app, regardless of what > + * the get system design information WMI call returns > + */ > +static const char *const omen_thermal_profile_force_v0_boards[] = { > + "8607", "8746", "8747", "8749", "874A", "8748" > +}; > + > enum hp_wmi_radio { > HPWMI_WIFI = 0x0, > HPWMI_BLUETOOTH = 0x1, > @@ -115,6 +123,7 @@ enum hp_wmi_gm_commandtype { > HPWMI_SET_PERFORMANCE_MODE = 0x1A, > HPWMI_FAN_SPEED_MAX_GET_QUERY = 0x26, > HPWMI_FAN_SPEED_MAX_SET_QUERY = 0x27, > + HPWMI_GET_SYSTEM_DESIGN_DATA = 0x28, > }; > > enum hp_wmi_command { > @@ -149,6 +158,12 @@ enum hp_wireless2_bits { > HPWMI_POWER_FW_OR_HW = HPWMI_POWER_BIOS | HPWMI_POWER_HARD, > }; > > +enum hp_thermal_profile_omen_v1 { > + HP_OMEN_V1_THERMAL_PROFILE_DEFAULT = 0x30, > + HP_OMEN_V1_THERMAL_PROFILE_PERFORMANCE = 0x31, > + HP_OMEN_V1_THERMAL_PROFILE_COOL = 0x50, > +}; > + Maybe also rename the V0 values to HP_OMEN_V0_THERMAL_PROFILE_? for consistency ? > enum hp_thermal_profile_omen { > HP_OMEN_THERMAL_PROFILE_DEFAULT = 0x00, > HP_OMEN_THERMAL_PROFILE_PERFORMANCE = 0x01, > @@ -360,9 +375,6 @@ static int omen_thermal_profile_set(int mode) > char buffer[2] = {0, mode}; > int ret; > > - if (mode < 0 || mode > 2) > - return -EINVAL; > - > ret = hp_wmi_perform_query(HPWMI_SET_PERFORMANCE_MODE, HPWMI_GM, > &buffer, sizeof(buffer), sizeof(buffer)); > > @@ -384,6 +396,30 @@ static bool is_omen_thermal_profile(void) > board_name) >= 0; > } > > +static int omen_get_thermal_policy_version(void) > +{ > + unsigned char buffer[8] = { 0 }; > + int ret; > + > + const char *board_name = dmi_get_system_info(DMI_BOARD_NAME); > + > + if (board_name) { > + int matches = match_string(omen_thermal_profile_force_v0_boards, > + ARRAY_SIZE(omen_thermal_profile_force_v0_boards), > + board_name); > + if (matches >= 0) > + return 1; This seems wrong, shouldn't this be "return 0" so that the v0 interface actually gets used, rather then the v1 interface? > + } > + > + ret = hp_wmi_perform_query(HPWMI_GET_SYSTEM_DESIGN_DATA, HPWMI_GM, > + &buffer, sizeof(buffer), sizeof(buffer)); > + > + if (ret) > + return ret < 0 ? ret : -EINVAL; > + > + return buffer[3]; > +} > + > static int omen_thermal_profile_get(void) > { > u8 data; > @@ -1009,12 +1045,15 @@ static int platform_profile_omen_get(struct platform_profile_handler *pprof, > > switch (tp) { > case HP_OMEN_THERMAL_PROFILE_PERFORMANCE: > + case HP_OMEN_V1_THERMAL_PROFILE_PERFORMANCE: > *profile = PLATFORM_PROFILE_PERFORMANCE; > break; > case HP_OMEN_THERMAL_PROFILE_DEFAULT: > + case HP_OMEN_V1_THERMAL_PROFILE_DEFAULT: > *profile = PLATFORM_PROFILE_BALANCED; > break; > case HP_OMEN_THERMAL_PROFILE_COOL: > + case HP_OMEN_V1_THERMAL_PROFILE_COOL: > *profile = PLATFORM_PROFILE_COOL; > break; > default: > @@ -1027,23 +1066,38 @@ static int platform_profile_omen_get(struct platform_profile_handler *pprof, > static int platform_profile_omen_set(struct platform_profile_handler *pprof, > enum platform_profile_option profile) > { > - int err, tp; > + int err, tp, tp_version; > + > + tp_version = omen_get_thermal_policy_version(); > + > + if (tp_version < 0 || tp_version > 1) > + return -EOPNOTSUPP; > > switch (profile) { > case PLATFORM_PROFILE_PERFORMANCE: > - tp = HP_OMEN_THERMAL_PROFILE_PERFORMANCE; > + if (tp_version == 0) > + tp = HP_OMEN_THERMAL_PROFILE_PERFORMANCE; > + else > + tp = HP_OMEN_V1_THERMAL_PROFILE_PERFORMANCE; > break; > case PLATFORM_PROFILE_BALANCED: > - tp = HP_OMEN_THERMAL_PROFILE_DEFAULT; > + if (tp_version == 0) > + tp = HP_OMEN_THERMAL_PROFILE_DEFAULT; > + else > + tp = HP_OMEN_V1_THERMAL_PROFILE_DEFAULT; > break; > case PLATFORM_PROFILE_COOL: > - tp = HP_OMEN_THERMAL_PROFILE_COOL; > + if (tp_version == 0) > + tp = HP_OMEN_THERMAL_PROFILE_COOL; > + else > + tp = HP_OMEN_V1_THERMAL_PROFILE_COOL; > break; > default: > return -EOPNOTSUPP; > } > > err = omen_thermal_profile_set(tp); > + Please drop this unnecessary blank-line addition. > if (err < 0) > return err; > > @@ -1128,6 +1182,7 @@ static int thermal_profile_setup(void) > */ > > err = omen_thermal_profile_set(tp); > + Please drop this unnecessary blank-line addition. > if (err < 0) > return err; > Regards, Hans
