Linux-OMAP Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 1/3] Input: atmel_mxt_ts - use runtime PM instead of custom functions
@ 2020-03-18 23:09 Tony Lindgren
  2020-03-18 23:09 ` [PATCH 2/3] Input: atmel_mxt_ts - add idle power config Tony Lindgren
  2020-03-18 23:09 ` [PATCH 3/3] Input: atmel_mxt_ts - use runtime PM autosuspend for idle config Tony Lindgren
  0 siblings, 2 replies; 4+ messages in thread
From: Tony Lindgren @ 2020-03-18 23:09 UTC (permalink / raw)
  To: Nick Dyer, Dmitry Torokhov
  Cc: Henrik Rydberg, Arthur Demchenkov, Ivaylo Dimitrov,
	Merlijn Wajer, Pavel Machek, Sebastian Reichel, linux-input,
	linux-kernel, linux-omap

Let's use standard runtime PM functions instead of custom start and stop
functions. This way we can implement runtime idle mode using runtime PM
autosuspend in the following patches.

Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 134 ++++++++++++++---------
 1 file changed, 85 insertions(+), 49 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -21,6 +21,7 @@
 #include <linux/input/mt.h>
 #include <linux/interrupt.h>
 #include <linux/of.h>
+#include <linux/pm_runtime.h>
 #include <linux/property.h>
 #include <linux/slab.h>
 #include <linux/gpio/consumer.h>
@@ -2927,58 +2928,27 @@ static const struct attribute_group mxt_attr_group = {
 	.attrs = mxt_attrs,
 };
 
-static void mxt_start(struct mxt_data *data)
+static int mxt_input_open(struct input_dev *input_dev)
 {
-	switch (data->suspend_mode) {
-	case MXT_SUSPEND_T9_CTRL:
-		mxt_soft_reset(data);
-
-		/* Touch enable */
-		/* 0x83 = SCANEN | RPTEN | ENABLE */
-		mxt_write_object(data,
-				MXT_TOUCH_MULTI_T9, MXT_T9_CTRL, 0x83);
-		break;
-
-	case MXT_SUSPEND_DEEP_SLEEP:
-	default:
-		mxt_set_t7_power_cfg(data, MXT_POWER_CFG_RUN);
-
-		/* Recalibrate since chip has been in deep sleep */
-		mxt_t6_command(data, MXT_COMMAND_CALIBRATE, 1, false);
-		break;
-	}
-}
-
-static void mxt_stop(struct mxt_data *data)
-{
-	switch (data->suspend_mode) {
-	case MXT_SUSPEND_T9_CTRL:
-		/* Touch disable */
-		mxt_write_object(data,
-				MXT_TOUCH_MULTI_T9, MXT_T9_CTRL, 0);
-		break;
+	struct mxt_data *data = input_get_drvdata(input_dev);
+	struct device *dev = &data->client->dev;
+	int error;
 
-	case MXT_SUSPEND_DEEP_SLEEP:
-	default:
-		mxt_set_t7_power_cfg(data, MXT_POWER_CFG_DEEPSLEEP);
-		break;
+	error = pm_runtime_get_sync(dev);
+	if (error < 0) {
+		pm_runtime_put_noidle(dev);
+		return error;
 	}
-}
-
-static int mxt_input_open(struct input_dev *dev)
-{
-	struct mxt_data *data = input_get_drvdata(dev);
-
-	mxt_start(data);
 
 	return 0;
 }
 
-static void mxt_input_close(struct input_dev *dev)
+static void mxt_input_close(struct input_dev *input_dev)
 {
-	struct mxt_data *data = input_get_drvdata(dev);
+	struct mxt_data *data = input_get_drvdata(input_dev);
+	struct device *dev = &data->client->dev;
 
-	mxt_stop(data);
+	pm_runtime_put_sync(dev);
 }
 
 static int mxt_parse_device_properties(struct mxt_data *data)
@@ -3036,6 +3006,7 @@ static const struct dmi_system_id chromebook_T9_suspend_dmi[] = {
 static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id)
 {
 	struct mxt_data *data;
+	struct device *dev;
 	int error;
 
 	/*
@@ -3070,6 +3041,7 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	snprintf(data->phys, sizeof(data->phys), "i2c-%u-%04x/input0",
 		 client->adapter->nr, client->addr);
 
+	dev = &client->dev;
 	data->client = client;
 	data->irq = client->irq;
 	i2c_set_clientdata(client, data);
@@ -3109,20 +3081,23 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id)
 		msleep(MXT_RESET_INVALID_CHG);
 	}
 
+	pm_runtime_enable(dev);
+
 	error = mxt_initialize(data);
 	if (error)
-		return error;
+		goto err_disable;
 
 	error = sysfs_create_group(&client->dev.kobj, &mxt_attr_group);
 	if (error) {
 		dev_err(&client->dev, "Failure %d creating sysfs group\n",
 			error);
-		goto err_free_object;
+		goto err_disable;
 	}
 
 	return 0;
 
-err_free_object:
+err_disable:
+	pm_runtime_disable(dev);
 	mxt_free_input_device(data);
 	mxt_free_object_table(data);
 	return error;
@@ -3131,11 +3106,69 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id)
 static int mxt_remove(struct i2c_client *client)
 {
 	struct mxt_data *data = i2c_get_clientdata(client);
+	struct device *dev = &data->client->dev;
 
 	disable_irq(data->irq);
 	sysfs_remove_group(&client->dev.kobj, &mxt_attr_group);
 	mxt_free_input_device(data);
 	mxt_free_object_table(data);
+	pm_runtime_disable(dev);
+
+	return 0;
+}
+
+static int __maybe_unused mxt_runtime_suspend(struct device *dev)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct mxt_data *data = i2c_get_clientdata(client);
+	struct input_dev *input_dev = data->input_dev;
+
+	if (!input_dev)
+		return 0;
+
+	switch (data->suspend_mode) {
+	case MXT_SUSPEND_T9_CTRL:
+		/* Touch disable */
+		mxt_write_object(data,
+				 MXT_TOUCH_MULTI_T9, MXT_T9_CTRL, 0);
+		break;
+
+	case MXT_SUSPEND_DEEP_SLEEP:
+	default:
+		mxt_set_t7_power_cfg(data, MXT_POWER_CFG_DEEPSLEEP);
+		break;
+	}
+
+	return 0;
+}
+
+static int __maybe_unused mxt_runtime_resume(struct device *dev)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct mxt_data *data = i2c_get_clientdata(client);
+	struct input_dev *input_dev = data->input_dev;
+
+	if (!input_dev)
+		return 0;
+
+	switch (data->suspend_mode) {
+	case MXT_SUSPEND_T9_CTRL:
+		mxt_soft_reset(data);
+
+		/* Touch enable */
+		/* 0x83 = SCANEN | RPTEN | ENABLE */
+		mxt_write_object(data,
+				 MXT_TOUCH_MULTI_T9, MXT_T9_CTRL, 0x83);
+		break;
+
+	case MXT_SUSPEND_DEEP_SLEEP:
+	default:
+		mxt_set_t7_power_cfg(data, MXT_POWER_CFG_RUN);
+
+		/* Recalibrate since chip has been in deep sleep */
+		mxt_t6_command(data, MXT_COMMAND_CALIBRATE, 1, false);
+		break;
+	}
 
 	return 0;
 }
@@ -3152,7 +3185,7 @@ static int __maybe_unused mxt_suspend(struct device *dev)
 	mutex_lock(&input_dev->mutex);
 
 	if (input_dev->users)
-		mxt_stop(data);
+		mxt_runtime_suspend(dev);
 
 	mutex_unlock(&input_dev->mutex);
 
@@ -3175,14 +3208,17 @@ static int __maybe_unused mxt_resume(struct device *dev)
 	mutex_lock(&input_dev->mutex);
 
 	if (input_dev->users)
-		mxt_start(data);
+		mxt_runtime_resume(dev);
 
 	mutex_unlock(&input_dev->mutex);
 
 	return 0;
 }
 
-static SIMPLE_DEV_PM_OPS(mxt_pm_ops, mxt_suspend, mxt_resume);
+const struct dev_pm_ops mxt_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(mxt_suspend, mxt_resume)
+	SET_RUNTIME_PM_OPS(mxt_runtime_suspend, mxt_runtime_resume, NULL)
+};
 
 static const struct of_device_id mxt_of_match[] = {
 	{ .compatible = "atmel,maxtouch", },
-- 
2.25.1

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

* [PATCH 2/3] Input: atmel_mxt_ts - add idle power config
  2020-03-18 23:09 [PATCH 1/3] Input: atmel_mxt_ts - use runtime PM instead of custom functions Tony Lindgren
@ 2020-03-18 23:09 ` Tony Lindgren
  2020-03-18 23:09 ` [PATCH 3/3] Input: atmel_mxt_ts - use runtime PM autosuspend for idle config Tony Lindgren
  1 sibling, 0 replies; 4+ messages in thread
From: Tony Lindgren @ 2020-03-18 23:09 UTC (permalink / raw)
  To: Nick Dyer, Dmitry Torokhov
  Cc: Henrik Rydberg, Arthur Demchenkov, Ivaylo Dimitrov,
	Merlijn Wajer, Pavel Machek, Sebastian Reichel, linux-input,
	linux-kernel, linux-omap

We can save few tens of mW of power during runtime. Let's add a new idle
power config where we minimize the active time and maximize the idle time.

Then we can use the new idle setting based on runtime PM autosuspend in
the following patch.

Let's also start using enum mxt_power_cfg while at it.

Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 27 +++++++++++++++++++-----
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -97,8 +97,11 @@ struct t7_config {
 	u8 active;
 } __packed;
 
-#define MXT_POWER_CFG_RUN		0
-#define MXT_POWER_CFG_DEEPSLEEP		1
+enum mxt_power_cfg {
+	MXT_POWER_CFG_RUN,
+	MXT_POWER_CFG_IDLE,
+	MXT_POWER_CFG_DEEPSLEEP,
+};
 
 /* MXT_TOUCH_MULTI_T9 field */
 #define MXT_T9_CTRL		0
@@ -2155,17 +2158,31 @@ static int mxt_initialize(struct mxt_data *data)
 	return 0;
 }
 
-static int mxt_set_t7_power_cfg(struct mxt_data *data, u8 sleep)
+/*
+ * Note that active value 0 forces the controller to idle so 1 is the shortest
+ * active periiod with interrupts still working. Idle value of 255 blocks idle
+ * completely so 254 is the maximum idle time we can use.
+ */
+static int mxt_set_t7_power_cfg(struct mxt_data *data,
+				enum mxt_power_cfg config)
 {
 	struct device *dev = &data->client->dev;
 	int error;
 	struct t7_config *new_config;
 	struct t7_config deepsleep = { .active = 0, .idle = 0 };
+	struct t7_config idle = { .active = 1, .idle = 254 };
 
-	if (sleep == MXT_POWER_CFG_DEEPSLEEP)
+	switch (config) {
+	case MXT_POWER_CFG_IDLE:
+		new_config = &idle;
+		break;
+	case MXT_POWER_CFG_DEEPSLEEP:
 		new_config = &deepsleep;
-	else
+		break;
+	default:
 		new_config = &data->t7_cfg;
+		break;
+	}
 
 	error = __mxt_write_reg(data->client, data->T7_address,
 				sizeof(data->t7_cfg), new_config);
-- 
2.25.1

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

* [PATCH 3/3] Input: atmel_mxt_ts - use runtime PM autosuspend for idle config
  2020-03-18 23:09 [PATCH 1/3] Input: atmel_mxt_ts - use runtime PM instead of custom functions Tony Lindgren
  2020-03-18 23:09 ` [PATCH 2/3] Input: atmel_mxt_ts - add idle power config Tony Lindgren
@ 2020-03-18 23:09 ` Tony Lindgren
  2020-03-20 19:00   ` Tony Lindgren
  1 sibling, 1 reply; 4+ messages in thread
From: Tony Lindgren @ 2020-03-18 23:09 UTC (permalink / raw)
  To: Nick Dyer, Dmitry Torokhov
  Cc: Henrik Rydberg, Arthur Demchenkov, Ivaylo Dimitrov,
	Merlijn Wajer, Pavel Machek, Sebastian Reichel, linux-input,
	linux-kernel, linux-omap

Let's enable runtime PM autosuspend with a default value of 600 ms and
switch to idle power config for runtime_suspend. The autosuspend timeout
can also be configured also via userspace with value of -1 disabling the
autosuspend.

Let's only allow autosuspend if MXT_SUSPEND_T9_CTRL is not configured for
suspend_mode as MXT_SUSPEND_T9_CTRL mode powers off the controller.

Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 76 ++++++++++++++++++++----
 1 file changed, 66 insertions(+), 10 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -1140,21 +1140,26 @@ static irqreturn_t mxt_process_messages(struct mxt_data *data)
 static irqreturn_t mxt_interrupt(int irq, void *dev_id)
 {
 	struct mxt_data *data = dev_id;
+	irqreturn_t ret = IRQ_HANDLED;
 
 	if (data->in_bootloader) {
 		/* bootloader state transition completion */
 		complete(&data->bl_completion);
-		return IRQ_HANDLED;
+		goto out_done;
 	}
 
 	if (!data->object_table)
-		return IRQ_HANDLED;
+		goto out_done;
 
-	if (data->T44_address) {
-		return mxt_process_messages_t44(data);
-	} else {
-		return mxt_process_messages(data);
-	}
+	if (data->T44_address)
+		ret = mxt_process_messages_t44(data);
+	else
+		ret = mxt_process_messages(data);
+
+out_done:
+	pm_runtime_mark_last_busy(&data->client->dev);
+
+	return ret;
 }
 
 static int mxt_t6_command(struct mxt_data *data, u16 cmd_offset,
@@ -2957,6 +2962,16 @@ static int mxt_input_open(struct input_dev *input_dev)
 		return error;
 	}
 
+	/*
+	 * Prevent autoidle if MXT_SUSPEND_T9_CTRL is the default as it will
+	 * power off the controller. Balanced inmxt_input_close().
+	 */
+	if (data->suspend_mode == MXT_SUSPEND_T9_CTRL)
+		pm_runtime_get(dev);
+
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_put_autosuspend(dev);
+
 	return 0;
 }
 
@@ -2964,8 +2979,21 @@ static void mxt_input_close(struct input_dev *input_dev)
 {
 	struct mxt_data *data = input_get_drvdata(input_dev);
 	struct device *dev = &data->client->dev;
+	int error;
 
-	pm_runtime_put_sync(dev);
+	/* Release extra usage count for MXT_SUSPEND_T9_CTRL done in open */
+	if (data->suspend_mode == MXT_SUSPEND_T9_CTRL)
+		pm_runtime_put(dev);
+
+	/* Wake the device so autosuspend sees correct input_dev->users */
+	error = pm_runtime_get_sync(dev);
+	if (error < 0) {
+		pm_runtime_put_noidle(dev);
+		return;
+	}
+
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_put_autosuspend(dev);
 }
 
 static int mxt_parse_device_properties(struct mxt_data *data)
@@ -3099,6 +3127,14 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	}
 
 	pm_runtime_enable(dev);
+	pm_runtime_use_autosuspend(dev);
+	pm_runtime_set_autosuspend_delay(dev, 600);
+	error = pm_runtime_get_sync(dev);
+	if (error < 0) {
+		pm_runtime_put_noidle(dev);
+		return error;
+	}
+
 
 	error = mxt_initialize(data);
 	if (error)
@@ -3111,9 +3147,14 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id)
 		goto err_disable;
 	}
 
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_put_autosuspend(dev);
+
 	return 0;
 
 err_disable:
+	pm_runtime_dont_use_autosuspend(dev);
+	pm_runtime_put_sync(dev);
 	pm_runtime_disable(dev);
 	mxt_free_input_device(data);
 	mxt_free_object_table(data);
@@ -3124,12 +3165,24 @@ static int mxt_remove(struct i2c_client *client)
 {
 	struct mxt_data *data = i2c_get_clientdata(client);
 	struct device *dev = &data->client->dev;
+	int active;
+
+	/* Attempt to change controller suspend mode to disable on remove */
+	active = pm_runtime_get_sync(dev);
+	if (active < 0)
+		pm_runtime_put_noidle(dev);
+	else
+		data->suspend_mode = MXT_SUSPEND_T9_CTRL;
+
+	pm_runtime_dont_use_autosuspend(dev);
+	if (active >= 0)
+		pm_runtime_put_sync(dev);
+	pm_runtime_disable(dev);
 
 	disable_irq(data->irq);
 	sysfs_remove_group(&client->dev.kobj, &mxt_attr_group);
 	mxt_free_input_device(data);
 	mxt_free_object_table(data);
-	pm_runtime_disable(dev);
 
 	return 0;
 }
@@ -3152,7 +3205,10 @@ static int __maybe_unused mxt_runtime_suspend(struct device *dev)
 
 	case MXT_SUSPEND_DEEP_SLEEP:
 	default:
-		mxt_set_t7_power_cfg(data, MXT_POWER_CFG_DEEPSLEEP);
+		if (input_dev->users)
+			mxt_set_t7_power_cfg(data, MXT_POWER_CFG_IDLE);
+		else
+			mxt_set_t7_power_cfg(data, MXT_POWER_CFG_DEEPSLEEP);
 		break;
 	}
 
-- 
2.25.1

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

* Re: [PATCH 3/3] Input: atmel_mxt_ts - use runtime PM autosuspend for idle config
  2020-03-18 23:09 ` [PATCH 3/3] Input: atmel_mxt_ts - use runtime PM autosuspend for idle config Tony Lindgren
@ 2020-03-20 19:00   ` Tony Lindgren
  0 siblings, 0 replies; 4+ messages in thread
From: Tony Lindgren @ 2020-03-20 19:00 UTC (permalink / raw)
  To: Nick Dyer, Dmitry Torokhov
  Cc: Henrik Rydberg, Arthur Demchenkov, Ivaylo Dimitrov,
	Merlijn Wajer, Pavel Machek, Sebastian Reichel, linux-input,
	linux-kernel, linux-omap

* Tony Lindgren <tony@atomide.com> [200318 23:10]:
> Let's enable runtime PM autosuspend with a default value of 600 ms and
> switch to idle power config for runtime_suspend. The autosuspend timeout
> can also be configured also via userspace with value of -1 disabling the
> autosuspend.
> 
> Let's only allow autosuspend if MXT_SUSPEND_T9_CTRL is not configured for
> suspend_mode as MXT_SUSPEND_T9_CTRL mode powers off the controller.

Hmm looks like with autosuspend enabled I need a short swipe instead of
just clicking on the screen to produce a click. So we may want to default
to autosuspend timeout of -1 and have user set it when suitable and no
other fix is found.

On droid4, I've confirmed that disabling autosuspend fixes the issue FYI:

# echo on > /sys/bus/i2c/drivers/atmel_mxt_ts/1-004a/power/control

Regards,

Tony

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

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-18 23:09 [PATCH 1/3] Input: atmel_mxt_ts - use runtime PM instead of custom functions Tony Lindgren
2020-03-18 23:09 ` [PATCH 2/3] Input: atmel_mxt_ts - add idle power config Tony Lindgren
2020-03-18 23:09 ` [PATCH 3/3] Input: atmel_mxt_ts - use runtime PM autosuspend for idle config Tony Lindgren
2020-03-20 19:00   ` Tony Lindgren

Linux-OMAP Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-omap/0 linux-omap/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-omap linux-omap/ https://lore.kernel.org/linux-omap \
		linux-omap@vger.kernel.org
	public-inbox-index linux-omap

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-omap


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git