Hi, On 1/5/21 2:14 PM, Jiaxun Yang wrote: > Add support to ideapad-laptop for Lenovo platforms that have DYTC > version 5 support or newer to use the platform profile feature. > > Mostly based on Mark Pearson <markpearson@xxxxxxxxxx>'s thinkpad-acpi > work but massaged to fit ideapad driver. > > Note that different from ThinkPads, IdeaPads's Thermal Hotkey won't > trigger profile switch itself, we'll leave it for userspace programs. > > Tested on Lenovo Yoga-14S ARE Chinese Edition. > > Signed-off-by: Jiaxun Yang <jiaxun.yang@xxxxxxxxxxx> Now that the acpi/platform_profile stuff has landed I've merged this patch. Note I've made one small but important change I've replaced all occurrences of QUIET with LOW_POWER as was done in the latest revision of the thinkpad_acpi change. This is not only a cosmetical thing, it also fixes an actual bug: > +static int convert_dytc_to_profile(int dytcmode, enum platform_profile_option *profile) > +{ > + switch (dytcmode) { > + case DYTC_MODE_QUIET: > + *profile = PLATFORM_PROFILE_LOW_POWER; > + break; QUIET -> LOW_POWER, ok there actually is a PLATFORM_PROFILE_QUIET too, but according to the docs LOW_POWER actually is a better match (hence the rename). > + case DYTC_MODE_BALANCE: > + *profile = PLATFORM_PROFILE_BALANCED; > + break; > + case DYTC_MODE_PERFORM: > + *profile = PLATFORM_PROFILE_PERFORMANCE; > + break; > + default: /* Unknown mode */ > + return -EINVAL; > + } > + return 0; > +} > + > +static int convert_profile_to_dytc(enum platform_profile_option profile, int *perfmode) > +{ > + switch (profile) { > + case PLATFORM_PROFILE_QUIET: > + *perfmode = DYTC_MODE_QUIET; QUIET -> QUIET erm, but we weren't using PLATFORM_PROFILE_QUIET in this driver at all, also see: > + /* Setup supported modes */ > + set_bit(PLATFORM_PROFILE_LOW_POWER, priv->dytc->pprof.choices); > + set_bit(PLATFORM_PROFILE_BALANCED, priv->dytc->pprof.choices); > + set_bit(PLATFORM_PROFILE_PERFORMANCE, priv->dytc->pprof.choices); See we are not advertising PLATFORM_PROFILE_LOW_POWER and above we actually map DYTC_MODE_QUIET -> PLATFORM_PROFILE_LOW_POWER So we should do the reverse here. Anyways replacing all instances of QUIET with LOW_POWER clears up the confusion which lead to this bug and also actually fixes the bug. > + break; > + case PLATFORM_PROFILE_BALANCED: > + *perfmode = DYTC_MODE_BALANCE; > + break; > + case PLATFORM_PROFILE_PERFORMANCE: > + *perfmode = DYTC_MODE_PERFORM; > + break; > + default: /* Unknown profile */ > + return -EOPNOTSUPP; > + } > + return 0; > +} Thank you for your patch, I've applied this patch to my review-hans branch: https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans Note it will show up in my review-hans branch once I've pushed my local branch there, which might take a while. Once I've run some tests on this branch the patches there will be added to the platform-drivers-x86/for-next branch and eventually will be included in the pdx86 pull-request to Linus for the next merge-window. Regards, Hans > --- > drivers/platform/x86/Kconfig | 1 + > drivers/platform/x86/ideapad-laptop.c | 289 ++++++++++++++++++++++++++ > 2 files changed, 290 insertions(+) > > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig > index 91e6176cdfbd..8059143c21bb 100644 > --- a/drivers/platform/x86/Kconfig > +++ b/drivers/platform/x86/Kconfig > @@ -624,6 +624,7 @@ config IDEAPAD_LAPTOP > depends on BACKLIGHT_CLASS_DEVICE > depends on ACPI_VIDEO || ACPI_VIDEO = n > depends on ACPI_WMI || ACPI_WMI = n > + depends on ACPI_PLATFORM_PROFILE > select INPUT_SPARSEKMAP > help > This is a driver for Lenovo IdeaPad netbooks contains drivers for > diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c > index 7598cd46cf60..5512367c604a 100644 > --- a/drivers/platform/x86/ideapad-laptop.c > +++ b/drivers/platform/x86/ideapad-laptop.c > @@ -15,6 +15,7 @@ > #include <linux/acpi.h> > #include <linux/rfkill.h> > #include <linux/platform_device.h> > +#include <linux/platform_profile.h> > #include <linux/input.h> > #include <linux/input/sparse-keymap.h> > #include <linux/backlight.h> > @@ -77,6 +78,13 @@ enum { > VPCCMD_W_BL_POWER = 0x33, > }; > > +struct ideapad_dytc_priv { > + enum platform_profile_option current_profile; > + struct platform_profile_handler pprof; > + struct mutex mutex; > + struct ideapad_private *priv; > +}; > + > struct ideapad_rfk_priv { > int dev; > struct ideapad_private *priv; > @@ -89,6 +97,7 @@ struct ideapad_private { > struct platform_device *platform_device; > struct input_dev *inputdev; > struct backlight_device *blightdev; > + struct ideapad_dytc_priv *dytc; > struct dentry *debug; > unsigned long cfg; > bool has_hw_rfkill_switch; > @@ -136,6 +145,28 @@ static int method_int1(acpi_handle handle, char *method, int cmd) > return ACPI_FAILURE(status) ? -1 : 0; > } > > +static int method_dytc(acpi_handle handle, int cmd, int *ret) > +{ > + acpi_status status; > + unsigned long long result; > + struct acpi_object_list params; > + union acpi_object in_obj; > + > + params.count = 1; > + params.pointer = &in_obj; > + in_obj.type = ACPI_TYPE_INTEGER; > + in_obj.integer.value = cmd; > + > + status = acpi_evaluate_integer(handle, "DYTC", ¶ms, &result); > + > + if (ACPI_FAILURE(status)) { > + *ret = -1; > + return -1; > + } > + *ret = result; > + return 0; > +} > + > static int method_vpcr(acpi_handle handle, int cmd, int *ret) > { > acpi_status status; > @@ -546,6 +577,257 @@ static const struct attribute_group ideapad_attribute_group = { > .attrs = ideapad_attributes > }; > > +/* > + * DYTC Platform profile > + */ > +#define DYTC_CMD_QUERY 0 /* To get DYTC status - enable/revision */ > +#define DYTC_CMD_SET 1 /* To enable/disable IC function mode */ > +#define DYTC_CMD_GET 2 /* To get current IC function and mode */ > +#define DYTC_CMD_RESET 0x1ff /* To reset back to default */ > + > +#define DYTC_QUERY_ENABLE_BIT 8 /* Bit 8 - 0 = disabled, 1 = enabled */ > +#define DYTC_QUERY_SUBREV_BIT 16 /* Bits 16 - 27 - sub revision */ > +#define DYTC_QUERY_REV_BIT 28 /* Bits 28 - 31 - revision */ > + > +#define DYTC_GET_FUNCTION_BIT 8 /* Bits 8-11 - function setting */ > +#define DYTC_GET_MODE_BIT 12 /* Bits 12-15 - mode setting */ > + > +#define DYTC_SET_FUNCTION_BIT 12 /* Bits 12-15 - function setting */ > +#define DYTC_SET_MODE_BIT 16 /* Bits 16-19 - mode setting */ > +#define DYTC_SET_VALID_BIT 20 /* Bit 20 - 1 = on, 0 = off */ > + > +#define DYTC_FUNCTION_STD 0 /* Function = 0, standard mode */ > +#define DYTC_FUNCTION_CQL 1 /* Function = 1, lap mode */ > +#define DYTC_FUNCTION_MMC 11 /* Function = 11, desk mode */ > + > +#define DYTC_MODE_PERFORM 2 /* High power mode aka performance */ > +#define DYTC_MODE_QUIET 3 /* Low power mode aka quiet */ > +#define DYTC_MODE_BALANCE 0xF /* Default mode aka balanced */ > + > +#define DYTC_SET_COMMAND(function, mode, on) \ > + (DYTC_CMD_SET | (function) << DYTC_SET_FUNCTION_BIT | \ > + (mode) << DYTC_SET_MODE_BIT | \ > + (on) << DYTC_SET_VALID_BIT) > + > +#define DYTC_DISABLE_CQL DYTC_SET_COMMAND(DYTC_FUNCTION_CQL, DYTC_MODE_BALANCE, 0) > + > +#define DYTC_ENABLE_CQL DYTC_SET_COMMAND(DYTC_FUNCTION_CQL, DYTC_MODE_BALANCE, 1) > + > +static int convert_dytc_to_profile(int dytcmode, enum platform_profile_option *profile) > +{ > + switch (dytcmode) { > + case DYTC_MODE_QUIET: > + *profile = PLATFORM_PROFILE_LOW_POWER; > + break; > + case DYTC_MODE_BALANCE: > + *profile = PLATFORM_PROFILE_BALANCED; > + break; > + case DYTC_MODE_PERFORM: > + *profile = PLATFORM_PROFILE_PERFORMANCE; > + break; > + default: /* Unknown mode */ > + return -EINVAL; > + } > + return 0; > +} > + > +static int convert_profile_to_dytc(enum platform_profile_option profile, int *perfmode) > +{ > + switch (profile) { > + case PLATFORM_PROFILE_QUIET: > + *perfmode = DYTC_MODE_QUIET; > + break; > + case PLATFORM_PROFILE_BALANCED: > + *perfmode = DYTC_MODE_BALANCE; > + break; > + case PLATFORM_PROFILE_PERFORMANCE: > + *perfmode = DYTC_MODE_PERFORM; > + break; > + default: /* Unknown profile */ > + return -EOPNOTSUPP; > + } > + return 0; > +} > + > +/* > + * dytc_profile_get: Function to register with platform_profile > + * handler. Returns current platform profile. > + */ > +int dytc_profile_get(struct platform_profile_handler *pprof, > + enum platform_profile_option *profile) > +{ > + struct ideapad_dytc_priv *dytc; > + > + dytc = container_of(pprof, struct ideapad_dytc_priv, pprof); > + *profile = dytc->current_profile; > + return 0; > +} > + > +/* > + * Helper function - check if we are in CQL mode and if we are > + * - disable CQL, > + * - run the command > + * - enable CQL > + * If not in CQL mode, just run the command > + */ > +int dytc_cql_command(struct ideapad_private *priv, int command, int *output) > +{ > + int err, cmd_err, dummy; > + int cur_funcmode; > + > + /* Determine if we are in CQL mode. This alters the commands we do */ > + err = method_dytc(priv->adev->handle, DYTC_CMD_GET, output); > + if (err) > + return err; > + > + cur_funcmode = (*output >> DYTC_GET_FUNCTION_BIT) & 0xF; > + /* Check if we're OK to return immediately */ > + if ((command == DYTC_CMD_GET) && (cur_funcmode != DYTC_FUNCTION_CQL)) > + return 0; > + > + if (cur_funcmode == DYTC_FUNCTION_CQL) { > + err = method_dytc(priv->adev->handle, DYTC_DISABLE_CQL, &dummy); > + if (err) > + return err; > + } > + > + cmd_err = method_dytc(priv->adev->handle, command, output); > + /* Check return condition after we've restored CQL state */ > + > + if (cur_funcmode == DYTC_FUNCTION_CQL) { > + err = method_dytc(priv->adev->handle, DYTC_ENABLE_CQL, &dummy); > + if (err) > + return err; > + } > + > + return cmd_err; > +} > + > +/* > + * dytc_profile_set: Function to register with platform_profile > + * handler. Sets current platform profile. > + */ > +int dytc_profile_set(struct platform_profile_handler *pprof, > + enum platform_profile_option profile) > +{ > + struct ideapad_dytc_priv *dytc; > + struct ideapad_private *priv; > + int output; > + int err; > + > + dytc = container_of(pprof, struct ideapad_dytc_priv, pprof); > + priv = dytc->priv; > + > + err = mutex_lock_interruptible(&dytc->mutex); > + if (err) > + return err; > + > + if (profile == PLATFORM_PROFILE_BALANCED) { > + /* To get back to balanced mode we just issue a reset command */ > + err = method_dytc(priv->adev->handle, DYTC_CMD_RESET, &output); > + if (err) > + goto unlock; > + } else { > + int perfmode; > + > + err = convert_profile_to_dytc(profile, &perfmode); > + if (err) > + goto unlock; > + > + /* Determine if we are in CQL mode. This alters the commands we do */ > + err = dytc_cql_command(priv, > + DYTC_SET_COMMAND(DYTC_FUNCTION_MMC, perfmode, 1), > + &output); > + if (err) > + goto unlock; > + } > + /* Success - update current profile */ > + dytc->current_profile = profile; > +unlock: > + mutex_unlock(&dytc->mutex); > + return err; > +} > + > +static void dytc_profile_refresh(struct ideapad_private *priv) > +{ > + enum platform_profile_option profile; > + int output, err; > + int perfmode; > + > + mutex_lock(&priv->dytc->mutex); > + err = dytc_cql_command(priv, DYTC_CMD_GET, &output); > + mutex_unlock(&priv->dytc->mutex); > + if (err) > + return; > + > + perfmode = (output >> DYTC_GET_MODE_BIT) & 0xF; > + convert_dytc_to_profile(perfmode, &profile); > + if (profile != priv->dytc->current_profile) { > + priv->dytc->current_profile = profile; > + platform_profile_notify(); > + } > +} > + > +static int ideapad_dytc_profile_init(struct ideapad_private *priv) > +{ > + int err, output, dytc_version; > + > + err = method_dytc(priv->adev->handle, DYTC_CMD_QUERY, &output); > + /* For all other errors we can flag the failure */ > + if (err) > + return err; > + > + /* Check DYTC is enabled and supports mode setting */ > + if (!(output & BIT(DYTC_QUERY_ENABLE_BIT))) > + return -ENODEV; > + > + dytc_version = (output >> DYTC_QUERY_REV_BIT) & 0xF; > + if (dytc_version < 5) > + return -ENODEV; > + > + priv->dytc = kzalloc(sizeof(struct ideapad_dytc_priv), GFP_KERNEL); > + if (!priv->dytc) > + return -ENOMEM; > + > + mutex_init(&priv->dytc->mutex); > + > + priv->dytc->priv = priv; > + priv->dytc->pprof.profile_get = dytc_profile_get; > + priv->dytc->pprof.profile_set = dytc_profile_set; > + > + /* Setup supported modes */ > + set_bit(PLATFORM_PROFILE_LOW_POWER, priv->dytc->pprof.choices); > + set_bit(PLATFORM_PROFILE_BALANCED, priv->dytc->pprof.choices); > + set_bit(PLATFORM_PROFILE_PERFORMANCE, priv->dytc->pprof.choices); > + > + /* Create platform_profile structure and register */ > + err = platform_profile_register(&priv->dytc->pprof); > + if (err) > + goto mutex_destroy; > + > + /* Ensure initial values are correct */ > + dytc_profile_refresh(priv); > + > + return 0; > + > +mutex_destroy: > + mutex_destroy(&priv->dytc->mutex); > + kfree(priv->dytc); > + priv->dytc = NULL; > + return err; > +} > + > +static void ideapad_dytc_profile_exit(struct ideapad_private *priv) > +{ > + if (!priv->dytc) > + return; > + > + platform_profile_remove(); > + mutex_destroy(&priv->dytc->mutex); > + kfree(priv->dytc); > + priv->dytc = NULL; > +} > + > /* > * Rfkill > */ > @@ -1013,6 +1295,8 @@ static int ideapad_acpi_add(struct platform_device *pdev) > ideapad_sync_rfk_state(priv); > ideapad_sync_touchpad_state(priv); > > + ideapad_dytc_profile_init(priv); > + > if (acpi_video_get_backlight_type() == acpi_backlight_vendor) { > ret = ideapad_backlight_init(priv); > if (ret && ret != -ENODEV) > @@ -1066,6 +1350,7 @@ static int ideapad_acpi_remove(struct platform_device *pdev) > acpi_remove_notify_handler(priv->adev->handle, > ACPI_DEVICE_NOTIFY, ideapad_acpi_notify); > ideapad_backlight_exit(priv); > + ideapad_dytc_profile_exit(priv); > for (i = 0; i < IDEAPAD_RFKILL_DEV_NUM; i++) > ideapad_unregister_rfkill(priv, i); > ideapad_input_exit(priv); > @@ -1087,6 +1372,10 @@ static int ideapad_acpi_resume(struct device *device) > > ideapad_sync_rfk_state(priv); > ideapad_sync_touchpad_state(priv); > + > + if (priv->dytc) > + dytc_profile_refresh(priv); > + > return 0; > } > #endif >