linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/2] IdeaPad platform profile support
@ 2021-01-05 13:14 Jiaxun Yang
  2021-01-05 13:14 ` [PATCH v2 1/2] ACPI: platform-profile: Introduce object pointers to callbacks Jiaxun Yang
  2021-01-05 13:14 ` [PATCH v2 2/2] platform/x86: ideapad-laptop: DYTC Platform profile support Jiaxun Yang
  0 siblings, 2 replies; 6+ messages in thread
From: Jiaxun Yang @ 2021-01-05 13:14 UTC (permalink / raw)
  To: platform-driver-x86
  Cc: Jiaxun Yang, Rafael J. Wysocki, Len Brown, Hans de Goede,
	Mark Gross, Ike Panhc, Mark Pearson, linux-acpi, linux-kernel

Tested on Lenovo Yoga-14S ARE Chinese Edition.

v2: Use another private data approach (Hans)

Jiaxun Yang (2):
  ACPI: platform-profile: Introduce object pointers to callbacks
  platform/x86: ideapad-laptop: DYTC Platform profile support

 drivers/acpi/platform_profile.c       |   4 +-
 drivers/platform/x86/Kconfig          |   1 +
 drivers/platform/x86/ideapad-laptop.c | 289 ++++++++++++++++++++++++++
 include/linux/platform_profile.h      |   6 +-
 4 files changed, 296 insertions(+), 4 deletions(-)

-- 
2.30.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 1/2] ACPI: platform-profile: Introduce object pointers to callbacks
  2021-01-05 13:14 [PATCH v2 0/2] IdeaPad platform profile support Jiaxun Yang
@ 2021-01-05 13:14 ` Jiaxun Yang
  2021-01-05 13:16   ` Hans de Goede
  2021-01-05 13:14 ` [PATCH v2 2/2] platform/x86: ideapad-laptop: DYTC Platform profile support Jiaxun Yang
  1 sibling, 1 reply; 6+ messages in thread
From: Jiaxun Yang @ 2021-01-05 13:14 UTC (permalink / raw)
  To: platform-driver-x86
  Cc: Jiaxun Yang, Hans de Goede, Rafael J. Wysocki, Len Brown,
	Mark Gross, Ike Panhc, Mark Pearson, linux-acpi, linux-kernel

Add a object pointer to handler callbacks to avoid having
global variables everywhere.

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
Suggested-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/acpi/platform_profile.c  | 4 ++--
 include/linux/platform_profile.h | 6 ++++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/platform_profile.c b/drivers/acpi/platform_profile.c
index 91be50a32cc8..bb4d7b0fe2ac 100644
--- a/drivers/acpi/platform_profile.c
+++ b/drivers/acpi/platform_profile.c
@@ -64,7 +64,7 @@ static ssize_t platform_profile_show(struct device *dev,
 		return -ENODEV;
 	}
 
-	err = cur_profile->profile_get(&profile);
+	err = cur_profile->profile_get(cur_profile, &profile);
 	mutex_unlock(&profile_lock);
 	if (err)
 		return err;
@@ -104,7 +104,7 @@ static ssize_t platform_profile_store(struct device *dev,
 		return -EOPNOTSUPP;
 	}
 
-	err = cur_profile->profile_set(i);
+	err = cur_profile->profile_set(cur_profile, i);
 	mutex_unlock(&profile_lock);
 	if (err)
 		return err;
diff --git a/include/linux/platform_profile.h b/include/linux/platform_profile.h
index 3623d7108421..43f4583b5259 100644
--- a/include/linux/platform_profile.h
+++ b/include/linux/platform_profile.h
@@ -28,8 +28,10 @@ enum platform_profile_option {
 
 struct platform_profile_handler {
 	unsigned long choices[BITS_TO_LONGS(PLATFORM_PROFILE_LAST)];
-	int (*profile_get)(enum platform_profile_option *profile);
-	int (*profile_set)(enum platform_profile_option profile);
+	int (*profile_get)(struct platform_profile_handler *pprof,
+				enum platform_profile_option *profile);
+	int (*profile_set)(struct platform_profile_handler *pprof,
+				enum platform_profile_option profile);
 };
 
 int platform_profile_register(const struct platform_profile_handler *pprof);
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2 2/2] platform/x86: ideapad-laptop: DYTC Platform profile support
  2021-01-05 13:14 [PATCH v2 0/2] IdeaPad platform profile support Jiaxun Yang
  2021-01-05 13:14 ` [PATCH v2 1/2] ACPI: platform-profile: Introduce object pointers to callbacks Jiaxun Yang
@ 2021-01-05 13:14 ` Jiaxun Yang
  2021-02-02 18:29   ` Hans de Goede
  1 sibling, 1 reply; 6+ messages in thread
From: Jiaxun Yang @ 2021-01-05 13:14 UTC (permalink / raw)
  To: platform-driver-x86
  Cc: Jiaxun Yang, Rafael J. Wysocki, Len Brown, Hans de Goede,
	Mark Gross, Ike Panhc, Mark Pearson, linux-acpi, linux-kernel

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@lenovo.com>'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@flygoat.com>
---
 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", &params, &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
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/2] ACPI: platform-profile: Introduce object pointers to callbacks
  2021-01-05 13:14 ` [PATCH v2 1/2] ACPI: platform-profile: Introduce object pointers to callbacks Jiaxun Yang
@ 2021-01-05 13:16   ` Hans de Goede
  2021-01-07 17:11     ` Rafael J. Wysocki
  0 siblings, 1 reply; 6+ messages in thread
From: Hans de Goede @ 2021-01-05 13:16 UTC (permalink / raw)
  To: Jiaxun Yang, platform-driver-x86
  Cc: Rafael J. Wysocki, Len Brown, Mark Gross, Ike Panhc,
	Mark Pearson, linux-acpi, linux-kernel

Hi,

On 1/5/21 2:14 PM, Jiaxun Yang wrote:
> Add a object pointer to handler callbacks to avoid having
> global variables everywhere.
> 
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> Suggested-by: Hans de Goede <hdegoede@redhat.com>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans



> ---
>  drivers/acpi/platform_profile.c  | 4 ++--
>  include/linux/platform_profile.h | 6 ++++--
>  2 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/acpi/platform_profile.c b/drivers/acpi/platform_profile.c
> index 91be50a32cc8..bb4d7b0fe2ac 100644
> --- a/drivers/acpi/platform_profile.c
> +++ b/drivers/acpi/platform_profile.c
> @@ -64,7 +64,7 @@ static ssize_t platform_profile_show(struct device *dev,
>  		return -ENODEV;
>  	}
>  
> -	err = cur_profile->profile_get(&profile);
> +	err = cur_profile->profile_get(cur_profile, &profile);
>  	mutex_unlock(&profile_lock);
>  	if (err)
>  		return err;
> @@ -104,7 +104,7 @@ static ssize_t platform_profile_store(struct device *dev,
>  		return -EOPNOTSUPP;
>  	}
>  
> -	err = cur_profile->profile_set(i);
> +	err = cur_profile->profile_set(cur_profile, i);
>  	mutex_unlock(&profile_lock);
>  	if (err)
>  		return err;
> diff --git a/include/linux/platform_profile.h b/include/linux/platform_profile.h
> index 3623d7108421..43f4583b5259 100644
> --- a/include/linux/platform_profile.h
> +++ b/include/linux/platform_profile.h
> @@ -28,8 +28,10 @@ enum platform_profile_option {
>  
>  struct platform_profile_handler {
>  	unsigned long choices[BITS_TO_LONGS(PLATFORM_PROFILE_LAST)];
> -	int (*profile_get)(enum platform_profile_option *profile);
> -	int (*profile_set)(enum platform_profile_option profile);
> +	int (*profile_get)(struct platform_profile_handler *pprof,
> +				enum platform_profile_option *profile);
> +	int (*profile_set)(struct platform_profile_handler *pprof,
> +				enum platform_profile_option profile);
>  };
>  
>  int platform_profile_register(const struct platform_profile_handler *pprof);
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/2] ACPI: platform-profile: Introduce object pointers to callbacks
  2021-01-05 13:16   ` Hans de Goede
@ 2021-01-07 17:11     ` Rafael J. Wysocki
  0 siblings, 0 replies; 6+ messages in thread
From: Rafael J. Wysocki @ 2021-01-07 17:11 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Platform Driver, Jiaxun Yang, Rafael J. Wysocki, Len Brown,
	Mark Gross, Ike Panhc, Mark Pearson, ACPI Devel Maling List,
	Linux Kernel Mailing List

On Tue, Jan 5, 2021 at 2:18 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
>
> On 1/5/21 2:14 PM, Jiaxun Yang wrote:
> > Add a object pointer to handler callbacks to avoid having
> > global variables everywhere.
> >
> > Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> > Suggested-by: Hans de Goede <hdegoede@redhat.com>
>
> Thanks, patch looks good to me:
>
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
>
> Regards,
>
> Hans
>
>
>
> > ---
> >  drivers/acpi/platform_profile.c  | 4 ++--
> >  include/linux/platform_profile.h | 6 ++++--
> >  2 files changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/acpi/platform_profile.c b/drivers/acpi/platform_profile.c
> > index 91be50a32cc8..bb4d7b0fe2ac 100644
> > --- a/drivers/acpi/platform_profile.c
> > +++ b/drivers/acpi/platform_profile.c
> > @@ -64,7 +64,7 @@ static ssize_t platform_profile_show(struct device *dev,
> >               return -ENODEV;
> >       }
> >
> > -     err = cur_profile->profile_get(&profile);
> > +     err = cur_profile->profile_get(cur_profile, &profile);
> >       mutex_unlock(&profile_lock);
> >       if (err)
> >               return err;
> > @@ -104,7 +104,7 @@ static ssize_t platform_profile_store(struct device *dev,
> >               return -EOPNOTSUPP;
> >       }
> >
> > -     err = cur_profile->profile_set(i);
> > +     err = cur_profile->profile_set(cur_profile, i);
> >       mutex_unlock(&profile_lock);
> >       if (err)
> >               return err;
> > diff --git a/include/linux/platform_profile.h b/include/linux/platform_profile.h
> > index 3623d7108421..43f4583b5259 100644
> > --- a/include/linux/platform_profile.h
> > +++ b/include/linux/platform_profile.h
> > @@ -28,8 +28,10 @@ enum platform_profile_option {
> >
> >  struct platform_profile_handler {
> >       unsigned long choices[BITS_TO_LONGS(PLATFORM_PROFILE_LAST)];
> > -     int (*profile_get)(enum platform_profile_option *profile);
> > -     int (*profile_set)(enum platform_profile_option profile);
> > +     int (*profile_get)(struct platform_profile_handler *pprof,
> > +                             enum platform_profile_option *profile);
> > +     int (*profile_set)(struct platform_profile_handler *pprof,
> > +                             enum platform_profile_option profile);
> >  };
> >
> >  int platform_profile_register(const struct platform_profile_handler *pprof);
> >
>

Applied with a modified subject and some edits in the changelog as
5.12 material, and I'm leaving the [2/2] to you.

Thanks!

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 2/2] platform/x86: ideapad-laptop: DYTC Platform profile support
  2021-01-05 13:14 ` [PATCH v2 2/2] platform/x86: ideapad-laptop: DYTC Platform profile support Jiaxun Yang
@ 2021-02-02 18:29   ` Hans de Goede
  0 siblings, 0 replies; 6+ messages in thread
From: Hans de Goede @ 2021-02-02 18:29 UTC (permalink / raw)
  To: Jiaxun Yang, platform-driver-x86
  Cc: Rafael J. Wysocki, Len Brown, Mark Gross, Ike Panhc,
	Mark Pearson, linux-acpi, linux-kernel

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@lenovo.com>'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@flygoat.com>

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", &params, &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
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-02-02 18:33 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-05 13:14 [PATCH v2 0/2] IdeaPad platform profile support Jiaxun Yang
2021-01-05 13:14 ` [PATCH v2 1/2] ACPI: platform-profile: Introduce object pointers to callbacks Jiaxun Yang
2021-01-05 13:16   ` Hans de Goede
2021-01-07 17:11     ` Rafael J. Wysocki
2021-01-05 13:14 ` [PATCH v2 2/2] platform/x86: ideapad-laptop: DYTC Platform profile support Jiaxun Yang
2021-02-02 18:29   ` Hans de Goede

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).