On May 11, 2024 9:16:56 a.m. MDT, "Limonciello, Mario" <mario.limonciello@xxxxxxx> wrote: > > >On 5/10/2024 9:36 PM, Lyndon Sanche wrote: >> Some Dell laptops support configuration of preset fan modes through >> smbios tables. >> >> If the platform supports these fan modes, set up platform_profile to >> change these modes. If not supported, skip enabling platform_profile. >> >> Signed-off-by: Lyndon Sanche <lsanche@xxxxxxxxxx> >> --- >> drivers/platform/x86/dell/Kconfig | 2 + >> drivers/platform/x86/dell/dell-laptop.c | 242 +++++++++++++++++++ >> drivers/platform/x86/dell/dell-smbios-base.c | 1 + >> drivers/platform/x86/dell/dell-smbios.h | 1 + >> 4 files changed, 246 insertions(+) >> >> diff --git a/drivers/platform/x86/dell/Kconfig b/drivers/platform/x86/dell/Kconfig >> index bd9f445974cc..26679f22733c 100644 >> --- a/drivers/platform/x86/dell/Kconfig >> +++ b/drivers/platform/x86/dell/Kconfig >> @@ -47,6 +47,7 @@ config DCDBAS >> config DELL_LAPTOP >> tristate "Dell Laptop Extras" >> default m >> + depends on ACPI >> depends on DMI >> depends on BACKLIGHT_CLASS_DEVICE >> depends on ACPI_VIDEO || ACPI_VIDEO = n >> @@ -57,6 +58,7 @@ config DELL_LAPTOP >> select POWER_SUPPLY >> select LEDS_CLASS >> select NEW_LEDS >> + select ACPI_PLATFORM_PROFILE >> help >> This driver adds support for rfkill and backlight control to Dell >> laptops (except for some models covered by the Compal driver). >> diff --git a/drivers/platform/x86/dell/dell-laptop.c b/drivers/platform/x86/dell/dell-laptop.c >> index 42f7de2b4522..07f54f1cbac5 100644 >> --- a/drivers/platform/x86/dell/dell-laptop.c >> +++ b/drivers/platform/x86/dell/dell-laptop.c >> @@ -27,6 +27,9 @@ >> #include <linux/i8042.h> >> #include <linux/debugfs.h> >> #include <linux/seq_file.h> >> +#include <linux/bitfield.h> >> +#include <linux/bits.h> >> +#include <linux/platform_profile.h> >> #include <acpi/video.h> >> #include "dell-rbtn.h" >> #include "dell-smbios.h" >> @@ -95,6 +98,7 @@ static struct backlight_device *dell_backlight_device; >> static struct rfkill *wifi_rfkill; >> static struct rfkill *bluetooth_rfkill; >> static struct rfkill *wwan_rfkill; >> +static struct platform_profile_handler *thermal_handler; >> static bool force_rfkill; >> static bool micmute_led_registered; >> static bool mute_led_registered; >> @@ -2199,6 +2203,236 @@ static int mute_led_set(struct led_classdev *led_cdev, >> return 0; >> } >> +/* Derived from smbios-thermal-ctl >> + * >> + * cbClass 17 >> + * cbSelect 19 >> + * User Selectable Thermal Tables(USTT) >> + * cbArg1 determines the function to be performed >> + * cbArg1 0x0 = Get Thermal Information >> + * cbRES1 Standard return codes (0, -1, -2) >> + * cbRES2, byte 0 Bitmap of supported thermal modes. A mode is supported if >> + * its bit is set to 1 >> + * Bit 0 Balanced >> + * Bit 1 Cool Bottom >> + * Bit 2 Quiet >> + * Bit 3 Performance >> + * cbRES2, byte 1 Bitmap of supported Active Acoustic Controller (AAC) modes. >> + * Each mode corresponds to the supported thermal modes in >> + * byte 0. A mode is supported if its bit is set to 1. >> + * Bit 0 AAC (Balanced) >> + * Bit 1 AAC (Cool Bottom >> + * Bit 2 AAC (Quiet) >> + * Bit 3 AAC (Performance) >> + * cbRes3, byte 0 Current Thermal Mode >> + * Bit 0 Balanced >> + * Bit 1 Cool Bottom >> + * Bit 2 Quiet >> + * Bit 3 Performanc >> + * cbRes3, byte 1 AAC Configuration type >> + * 0 Global (AAC enable/disable applies to all supported USTT modes) >> + * 1 USTT mode specific >> + * cbRes3, byte 2 Current Active Acoustic Controller (AAC) Mode >> + * If AAC Configuration Type is Global, >> + * 0 AAC mode disabled >> + * 1 AAC mode enabled >> + * If AAC Configuration Type is USTT mode specific (multiple bits may be set), >> + * Bit 0 AAC (Balanced) >> + * Bit 1 AAC (Cool Bottom >> + * Bit 2 AAC (Quiet) >> + * Bit 3 AAC (Performance) >> + * cbRes3, byte 3 Current Fan Failure Mode >> + * Bit 0 Minimal Fan Failure (at least one fan has failed, one fan working) >> + * Bit 1 Catastrophic Fan Failure (all fans have failed) >> + * >> + * cbArg1 0x1 (Set Thermal Information), both desired thermal mode and >> + * desired AAC mode shall be applied >> + * cbArg2, byte 0 Desired Thermal Mode to set >> + * (only one bit may be set for this parameter) >> + * Bit 0 Balanced >> + * Bit 1 Cool Bottom >> + * Bit 2 Quiet >> + * Bit 3 Performance >> + * cbArg2, byte 1 Desired Active Acoustic Controller (AAC) Mode to set >> + * If AAC Configuration Type is Global, >> + * 0 AAC mode disabled >> + * 1 AAC mode enabled >> + * If AAC Configuration Type is USTT mode specific >> + * (multiple bits may be set for this parameter), >> + * Bit 0 AAC (Balanced) >> + * Bit 1 AAC (Cool Bottom >> + * Bit 2 AAC (Quiet) >> + * Bit 3 AAC (Performance) >> + */ >> + >> +#define DELL_ACC_GET_FIELD GENMASK(19, 16) >> +#define DELL_ACC_SET_FIELD GENMASK(11, 8) >> +#define DELL_THERMAL_SUPPORTED GENMASK(3, 0) >> + >> +enum thermal_mode_bits { >> + DELL_BALANCED = BIT(0), >> + DELL_COOL_BOTTOM = BIT(1), >> + DELL_QUIET = BIT(2), >> + DELL_PERFORMANCE = BIT(3), >> +}; >> + >> +static int thermal_get_mode(void) >> +{ >> + struct calling_interface_buffer buffer; >> + int state; >> + int ret; >> + >> + dell_fill_request(&buffer, 0x0, 0, 0, 0); >> + ret = dell_send_request(&buffer, CLASS_INFO, SELECT_THERMAL_MANAGEMENT); >> + if (ret) >> + return ret; >> + state = buffer.output[2]; >> + if (state & DELL_BALANCED) >> + return DELL_BALANCED; >> + else if (state & DELL_COOL_BOTTOM) >> + return DELL_COOL_BOTTOM; >> + else if (state & DELL_QUIET) >> + return DELL_QUIET; >> + else if (state & DELL_PERFORMANCE) >> + return DELL_PERFORMANCE; >> + else >> + return -ENXIO; >> +} >> + >> +static int thermal_get_supported_modes(int *supported_bits) >> +{ >> + struct calling_interface_buffer buffer; >> + int ret; >> + >> + dell_fill_request(&buffer, 0x0, 0, 0, 0); >> + ret = dell_send_request(&buffer, CLASS_INFO, SELECT_THERMAL_MANAGEMENT); >> + if (ret) >> + return ret; >> + *supported_bits = FIELD_GET(DELL_THERMAL_SUPPORTED, buffer.output[1]); >> + return 0; >> +} >> + >> +static int thermal_get_acc_mode(int *acc_mode) >> +{ >> + struct calling_interface_buffer buffer; >> + int ret; >> + >> + dell_fill_request(&buffer, 0x0, 0, 0, 0); >> + ret = dell_send_request(&buffer, CLASS_INFO, SELECT_THERMAL_MANAGEMENT); >> + if (ret) >> + return ret; >> + *acc_mode = FIELD_GET(DELL_ACC_GET_FIELD, buffer.output[3]); >> + return 0; >> +} >> + >> +static int thermal_set_mode(enum thermal_mode_bits state) >> +{ >> + struct calling_interface_buffer buffer; >> + int ret; >> + int acc_mode; >> + >> + ret = thermal_get_acc_mode(&acc_mode); >> + if (ret) >> + return ret; >> + >> + dell_fill_request(&buffer, 0x1, FIELD_PREP(DELL_ACC_SET_FIELD, acc_mode) | state, 0, 0); >> + ret = dell_send_request(&buffer, CLASS_INFO, SELECT_THERMAL_MANAGEMENT); >> + return ret; >> +} >> + >> +static int thermal_platform_profile_set(struct platform_profile_handler *pprof, >> + enum platform_profile_option profile) >> +{ >> + switch (profile) { >> + case PLATFORM_PROFILE_BALANCED: >> + return thermal_set_mode(DELL_BALANCED); >> + case PLATFORM_PROFILE_PERFORMANCE: >> + return thermal_set_mode(DELL_PERFORMANCE); >> + case PLATFORM_PROFILE_QUIET: >> + return thermal_set_mode(DELL_QUIET); >> + case PLATFORM_PROFILE_COOL: >> + return thermal_set_mode(DELL_COOL_BOTTOM); >> + default: >> + return -EOPNOTSUPP; >> + } >> +} >> + >> +static int thermal_platform_profile_get(struct platform_profile_handler *pprof, >> + enum platform_profile_option *profile) >> +{ >> + int ret; >> + >> + ret = thermal_get_mode(); >> + if (ret < 0) >> + return ret; >> + >> + switch (ret) { >> + case DELL_BALANCED: >> + *profile = PLATFORM_PROFILE_BALANCED; >> + break; >> + case DELL_PERFORMANCE: >> + *profile = PLATFORM_PROFILE_PERFORMANCE; >> + break; >> + case DELL_COOL_BOTTOM: >> + *profile = PLATFORM_PROFILE_COOL; >> + break; >> + case DELL_QUIET: >> + *profile = PLATFORM_PROFILE_QUIET; >> + break; >> + default: >> + return -EINVAL; >> + } >> + >> + return 0; >> +} >> + >> +static int thermal_init(void) >> +{ >> + int ret; >> + int supported_modes; >> + >> + /* If thermal commands not supported, exit without error */ >> + if (!dell_laptop_check_supported_cmds(CLASS_INFO)) >> + return 0; >> + >> + /* If thermal modes not supported, exit without error */ >> + ret = thermal_get_supported_modes(&supported_modes); >> + if (ret < 0) >> + return ret; >> + if (!supported_modes) >> + return 0; >> + >> + thermal_handler = kzalloc(sizeof(*thermal_handler), GFP_KERNEL); >> + if (!thermal_handler) >> + return -ENOMEM; >> + thermal_handler->profile_get = thermal_platform_profile_get; >> + thermal_handler->profile_set = thermal_platform_profile_set; >> + >> + if (supported_modes & DELL_QUIET) >> + set_bit(PLATFORM_PROFILE_QUIET, thermal_handler->choices); >> + if (supported_modes & DELL_COOL_BOTTOM) >> + set_bit(PLATFORM_PROFILE_COOL, thermal_handler->choices); >> + if (supported_modes & DELL_BALANCED) >> + set_bit(PLATFORM_PROFILE_BALANCED, thermal_handler->choices); >> + if (supported_modes & DELL_PERFORMANCE) >> + set_bit(PLATFORM_PROFILE_PERFORMANCE, thermal_handler->choices); >> + >> + /* Clean up if failed */ >> + ret = platform_profile_register(thermal_handler); >> + if (ret) >> + kfree(thermal_handler); >> + >> + return ret; >> +} >> + >> +static void thermal_cleanup(void) >> +{ >> + if (thermal_handler) { >> + platform_profile_remove(); >> + kfree(thermal_handler); >> + } >> +} >> + >> static struct led_classdev mute_led_cdev = { >> .name = "platform::mute", >> .max_brightness = 1, >> @@ -2238,6 +2472,11 @@ static int __init dell_init(void) >> goto fail_rfkill; >> } >> + /* Do not fail module if thermal modes not supported, just skip */ >> + ret = thermal_init(); >> + if (ret) >> + goto fail_thermal; >> + >> if (quirks && quirks->touchpad_led) >> touchpad_led_init(&platform_device->dev); >> @@ -2317,6 +2556,8 @@ static int __init dell_init(void) >> led_classdev_unregister(&mute_led_cdev); >> fail_led: >> dell_cleanup_rfkill(); >> +fail_thermal: >> + thermal_cleanup(); >> fail_rfkill: >> platform_device_del(platform_device); >> fail_platform_device2: >> @@ -2344,6 +2585,7 @@ static void __exit dell_exit(void) >> platform_device_unregister(platform_device); >> platform_driver_unregister(&platform_driver); >> } >> + thermal_cleanup(); >> } >> /* dell-rbtn.c driver export functions which will not work correctly (and could >> diff --git a/drivers/platform/x86/dell/dell-smbios-base.c b/drivers/platform/x86/dell/dell-smbios-base.c >> index 6ae09d7f76fb..387fa5618f7a 100644 >> --- a/drivers/platform/x86/dell/dell-smbios-base.c >> +++ b/drivers/platform/x86/dell/dell-smbios-base.c >> @@ -71,6 +71,7 @@ static struct smbios_call call_blacklist[] = { >> /* handled by kernel: dell-laptop */ >> {0x0000, CLASS_INFO, SELECT_RFKILL}, >> {0x0000, CLASS_KBD_BACKLIGHT, SELECT_KBD_BACKLIGHT}, >> + {0x0000, CLASS_INFO, SELECT_THERMAL_MANAGEMENT}, >> }; > >So when Alex checked on v5 that this doesn't load on workstations, it has made me realize that doing this will block the interface totally even on workstations. > >So I think there are a few ways to go to handle this: > >1) Rename dell-laptop to dell-client or dell-pc and let dell-laptop load on more form factors. This would require some internal handling in the module for which features make sense for different form factors. > >2) Add a new module just for the thermal handling and put all this code into it instead. > >I don't have a strong opinion, but I do think one of them should be done to ensure there aren't problems on workstations losing access to thermal control. > A dell-client/laptop separation makes more sense IMO. I don't think keyboard control would belong in a the dell-client module either. Separating just thermal control would be easier, but not as clean I think. Thanks, Lyndon