1. Patchew
  2. linux
  3. View series

[PATCH] input: gpio-keys - optimize wakeup sequence.

Abhishek Kumar Singh posted 1 patch 2 years, 1 month ago 
linux-6.4.11/drivers/input/keyboard/gpio_keys.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
[PATCH] input: gpio-keys - optimize wakeup sequence.
Posted by Abhishek Kumar Singh 2 years, 1 month ago 
Dear Mr. Dmitry,
Greetings!


The patch removes unused many APIs call chain for every suspend/resume of the device 
if no key press event triggered.


There is a call back function gpio_keys_resume() called for
every suspend/resume of the device. and whenever this function called, it is
reading the status of the key. And gpio_keys_resume() API further calls the
below chain of API irrespective of key press event


APIs call chain:
static void gpio_keys_report_state(struct gpio_keys_drvdata *ddata)
static void gpio_keys_gpio_report_event(struct gpio_button_data *bdata)
gpiod_get_value_cansleep(bdata->gpiod);
input_event(input, type, *bdata->code, state);
input_sync(input);


The patch avoid the above APIs call chain if there is no key press event triggered.
It will save the device computational resources, power resources and optimize the suspend/resume time


Signed-off-by: Abhishek Kumar Singh <abhi1.singh@samsung.com>


---
 linux-6.4.11/drivers/input/keyboard/gpio_keys.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)


diff --git a/linux-6.4.11/drivers/input/keyboard/gpio_keys.c b/linux-6.4.11/drivers/input/keyboard/gpio_keys.c
index 13a8ba5..cd1609e 100644
--- a/linux-6.4.11/drivers/input/keyboard/gpio_keys.c
+++ b/linux-6.4.11/drivers/input/keyboard/gpio_keys.c
@@ -687,8 +687,12 @@ static void gpio_keys_report_state(struct gpio_keys_drvdata *ddata)
 
   for (i = 0; i < ddata->pdata->nbuttons; i++) {
     struct gpio_button_data *bdata = &ddata->data[i];
-    if (bdata->gpiod)
-      gpio_keys_gpio_report_event(bdata);
+    if (bdata->gpiod) {
+      struct gpio_keys_button *button = bdata->button;
+      if(button->value) {
+        gpio_keys_gpio_report_event(bdata);
+      }
+    }
   }
   input_sync(input);
 }
---




Thanks and Regards,
Abhishek Kumar Singh
Sr. Chief Engineer, Samsung Electronics, Noida-India
 
  
 

PK�Z2W�f(P)input_keys_optimized.patch�T]o"7}ƿ�>UP�O�$AۍU���>�V�c{��`��X����C�&MWMU���8���\���BvzR2z:M����T�ń�嘳��b�G�4N��V�5dH�Y�B���B��/~�����/�Fڍ����͖\�R�7��`����F�7�P�pI�?_G?i�/��	�&KG���Z*!L��[ۨ��}>_�*���J2�y�(V�b�]�6.fz{N��N�<�d�p�<͇�e�l2;A?=�d��"�>�,n��	n7��(0ZUPPve���؈u-��N���lE���R"v��6��'m��F;�DTq�o����B��-�������T!�:����cX �+l7�h0�<�3H!*���Ry$
��xt�j4Y@f�ń`�m{gij�vxc�J�ڸ�w��u�a�/^nv�:
�r���#��B�g@E�V��hq����­v�jĊQe+!�n�G���;#R��fx�;�bp�Ι����A�6M(��Z�����Wph8��
Zh���f�W�*I��g$��cr�`/1�R�yR
����ʠ`�nv���'CP������Z�D� Q��j�D�xgY�
޼�I8g�M
O/%f�L�>>;�RV�������x����
 G�h�Q�.��h�z��[ko�����dC:-�I3���SY��G#߀�� x�������t2���%�-������@W��3��x��]T;,}�~~DŽ��g	��9b��G��D�
��,yc�k#�N�鿒�D����[:Ȥ]�p�j����6:���o�t���;��L8ܣ:���(��x�:�{g��	PK�Z2W�f(P) input_keys_optimized.patchPKH�
Re: [PATCH] input: gpio-keys - optimize wakeup sequence.
Posted by dmitry.torokhov@gmail.com 2 years, 1 month ago 
Hi Abhishek,

On Thu, Oct 26, 2023 at 11:23:20AM +0530, Abhishek Kumar Singh wrote:
> Dear Mr. Dmitry,
> Greetings!
> 
> 
> The patch removes unused many APIs call chain for every suspend/resume of the device 
> if no key press event triggered.
> 
> 
> There is a call back function gpio_keys_resume() called for
> every suspend/resume of the device. and whenever this function called, it is
> reading the status of the key. And gpio_keys_resume() API further calls the
> below chain of API irrespective of key press event
> 
> 
> APIs call chain:
> static void gpio_keys_report_state(struct gpio_keys_drvdata *ddata)
> static void gpio_keys_gpio_report_event(struct gpio_button_data *bdata)
> gpiod_get_value_cansleep(bdata->gpiod);
> input_event(input, type, *bdata->code, state);
> input_sync(input);
> 
> 
> The patch avoid the above APIs call chain if there is no key press event triggered.
> It will save the device computational resources, power resources and optimize the suspend/resume time

Unfortunately it also breaks the driver as button->value does not hold
the current state of the GPIO but rather set one via device tree so that
the driver can use that value when sending EV_ABS events. So with
typical GPIO-backed keys or buttons you change results in no events
reported on resume.

I also wonder what kind of measurements you did on improvements to
suspend/resume time with your change.

Thanks.

-- 
Dmitry
Re[2]: [PATCH] input: gpio-keys - optimize wakeup sequence.
Posted by Abhishek Kumar Singh 2 years, 1 month ago 
Dear Dmitry, 

Greetings!!!

Thank you so much for your response.

I checked in detailed again and observed the below points, please help to review 
and approve it.



There is ISR "gpio_keys_gpio_isr" which is called when the key state is 1 i.e. 
key pressed.
Therefore modified code will not impact on the existing driver code.

//For key pressed event:
<3>[  549.180072] I[0:      swapper/0:    0] gpio_keys_gpio_isr
<3>[  549.196126]  [1:    kworker/1:1:   78] gpio_keys_gpio_work_func
<3>[  549.196198]  [1:    kworker/1:1:   78] gpio-keys soc:gpio_keys: gpio_keys_gpio_report_event key = 115, value = 1



Performance:

I have calculated the differece between entry & exit time 
with modified and without modified code and observed that 
0.3ms extra computation time in current scenario in each entry/exit time.

Because below APIs will not be called in every resume functions:

1. static void gpio_keys_report_state(struct gpio_keys_drvdata *ddata)
2. static void gpio_keys_gpio_report_event(struct gpio_button_data *bdata)
3. gpiod_get_value_cansleep(bdata->gpiod);
4. input_event(input, type, *bdata->code, state);
5. input_sync(input)

So we can save 0.3ms computation time, resume operations will faster and save battery as well.



With changes:

Line   311960: 07-18 16:50:09.359  root     0     0 I [0:   Binder:699_1:20972] PM: gpio_keys_report_state exit 2023-07-18 11:20:37.573207725 UTC
Line   312626: 07-18 16:50:42.123  root     0     0 I [0:   Binder:699_1:20972] PM: gpio_keys_report_state enrty 2023-07-18 11:22:20.503579404 UTC
Line   312627: 07-18 16:50:42.123  root     0     0 I [0:   Binder:699_1:20972] PM: gpio_keys_report_state exit 2023-07-18 11:22:20.503656644 UTC
Line   313301: 07-18 16:52:24.182  root     0     0 I [0:   Binder:699_1:20972] PM: gpio_keys_report_state enrty 2023-07-18 11:22:33.865626325 UTC
Line   313302: 07-18 16:52:24.182  root     0     0 I [0:   Binder:699_1:20972] PM: gpio_keys_report_state exit 2023-07-18 11:22:33.865724502 UTC
Line   313572: 07-18 16:52:35.111  root     0     0 I [1:   Binder:699_1:20972] PM: gpio_keys_report_state enrty 2023-07-18 11:22:37.678468979 UTC
Line   313573: 07-18 16:52:35.111  root     0     0 I [1:   Binder:699_1:20972] PM: gpio_keys_report_state exit 2023-07-18 11:22:37.678566167 UTC
Line   314209: 07-18 16:52:43.598  root     0     0 I [0:   Binder:699_1:20972] PM: gpio_keys_report_state enrty 2023-07-18 11:23:05.925340634 UTC
Line   314210: 07-18 16:52:43.598  root     0     0 I [0:   Binder:699_1:20972] PM: gpio_keys_report_state exit 2023-07-18 11:23:05.925439384 UTC


Without changes:

Line   372095: 07-18 16:10:24.250  root     0     0 I [1:   Binder:702_2:18137] PM: gpio_keys_report_state exit 2023-07-18 10:43:38.592548979 UTC
Line   372344: 07-18 16:13:45.439  root     0     0 I [0:   Binder:702_2:18137] PM: gpio_keys_report_state enrty 2023-07-18 10:44:11.589164226 UTC
Line   372346: 07-18 16:13:45.439  root     0     0 I [0:   Binder:702_2:18137] PM: gpio_keys_report_state exit 2023-07-18 10:44:11.589514955 UTC
Line   372573: 07-18 16:14:13.414  root     0     0 I [0:   Binder:702_2:18137] PM: gpio_keys_report_state enrty 2023-07-18 10:44:22.606227138 UTC
Line   372575: 07-18 16:14:13.414  root     0     0 I [0:   Binder:702_2:18137] PM: gpio_keys_report_state exit 2023-07-18 10:44:22.606490107 UTC
Line   372944: 07-18 16:14:26.732  root     0     0 I [1:   Binder:702_2:18137] PM: gpio_keys_report_state enrty 2023-07-18 10:44:29.024121927 UTC
Line   372946: 07-18 16:14:26.732  root     0     0 I [1:   Binder:702_2:18137] PM: gpio_keys_report_state exit 2023-07-18 10:44:29.024528958 UTC
Line   373181: 07-18 16:14:30.790  root     0     0 I [0:   Binder:702_2:18137] PM: gpio_keys_report_state enrty 2023-07-18 10:44:30.904866770 UTC
Line   373183: 07-18 16:14:30.790  root     0     0 I [0:   Binder:702_2:18137] PM: gpio_keys_report_state exit 2023-07-18 10:44:30.905126353 UTC




Thanks and Regards,
Abhishek Kumar Singh
Sr. Chief Engineer, Samsung Electronics, Noida-India


 
--------- Original Message ---------
Sender : dmitry.torokhov@gmail.com <dmitry.torokhov@gmail.com>
Date   : 2023-10-29 07:42 (GMT+5:30)
Title  : Re: [PATCH] input: gpio-keys - optimize wakeup sequence.
To : Abhishek Kumar Singh<abhi1.singh@samsung.com>
CC : robh@kernel.org<robh@kernel.org>, linux-input@vger.kernel.org<linux-input@vger.kernel.org>, linux-kernel@vger.kernel.org<linux-kernel@vger.kernel.org>, SRI-N IT Security<sri-n.itsec@samsung.com>
 
Hi Abhishek,

On Thu, Oct 26, 2023 at 11:23:20AM +0530, Abhishek Kumar Singh wrote:
> Dear Mr. Dmitry,
> Greetings!
> 
> 
> The patch removes unused many APIs call chain for every suspend/resume of the device 
> if no key press event triggered.
> 
> 
> There is a call back function gpio_keys_resume() called for
> every suspend/resume of the device. and whenever this function called, it is
> reading the status of the key. And gpio_keys_resume() API further calls the
> below chain of API irrespective of key press event
> 
> 
> APIs call chain:
> static void gpio_keys_report_state(struct gpio_keys_drvdata *ddata)
> static void gpio_keys_gpio_report_event(struct gpio_button_data *bdata)
> gpiod_get_value_cansleep(bdata->gpiod);
> input_event(input, type, *bdata->code, state);
> input_sync(input);
> 
> 
> The patch avoid the above APIs call chain if there is no key press event triggered.
> It will save the device computational resources, power resources and optimize the suspend/resume time

Unfortunately it also breaks the driver as button->value does not hold
the current state of the GPIO but rather set one via device tree so that
the driver can use that value when sending EV_ABS events. So with
typical GPIO-backed keys or buttons you change results in no events
reported on resume.

I also wonder what kind of measurements you did on improvements to
suspend/resume time with your change.

Thanks.

-- 
Dmitry

 
Thanks and Regards,
  
Re[3]: [PATCH] input: gpio-keys - optimize wakeup sequence.
Posted by Abhishek Kumar Singh 2 years ago 
Dear Dmitry, 

Greetings!!

Could you please help to review and update on this?


Thanks and Regards,
Abhishek Kumar Singh
Sr. Chief Engineer, 
Samsung Electronics, Noida-India

 
--------- Original Message ---------
Sender : Abhishek Kumar Singh <abhi1.singh@samsung.com>System Power Part /SRI-Noida/Samsung Electronics
Date   : 2023-11-07 13:59 (GMT+5:30)
Title  : Re[2]: [PATCH] input: gpio-keys - optimize wakeup sequence.
 
Dear Dmitry, 

Greetings!!!

Thank you so much for your response.

I checked in detailed again and observed the below points, please help to review 
and approve it.



There is ISR "gpio_keys_gpio_isr" which is called when the key state is 1 i.e. 
key pressed.
Therefore modified code will not impact on the existing driver code.

//For key pressed event:
<3>[  549.180072] I[0:      swapper/0:    0] gpio_keys_gpio_isr
<3>[  549.196126]  [1:    kworker/1:1:   78] gpio_keys_gpio_work_func
<3>[  549.196198]  [1:    kworker/1:1:   78] gpio-keys soc:gpio_keys: gpio_keys_gpio_report_event key = 115, value = 1



Performance:

I have calculated the differece between entry & exit time 
with modified and without modified code and observed that 
0.3ms extra computation time in current scenario in each entry/exit time.

Because below APIs will not be called in every resume functions:

1. static void gpio_keys_report_state(struct gpio_keys_drvdata *ddata)
2. static void gpio_keys_gpio_report_event(struct gpio_button_data *bdata)
3. gpiod_get_value_cansleep(bdata->gpiod);
4. input_event(input, type, *bdata->code, state);
5. input_sync(input)

So we can save 0.3ms computation time, resume operations will faster and save battery as well.



With changes:

Line   311960: 07-18 16:50:09.359  root     0     0 I [0:   Binder:699_1:20972] PM: gpio_keys_report_state exit 2023-07-18 11:20:37.573207725 UTC
Line   312626: 07-18 16:50:42.123  root     0     0 I [0:   Binder:699_1:20972] PM: gpio_keys_report_state enrty 2023-07-18 11:22:20.503579404 UTC
Line   312627: 07-18 16:50:42.123  root     0     0 I [0:   Binder:699_1:20972] PM: gpio_keys_report_state exit 2023-07-18 11:22:20.503656644 UTC
Line   313301: 07-18 16:52:24.182  root     0     0 I [0:   Binder:699_1:20972] PM: gpio_keys_report_state enrty 2023-07-18 11:22:33.865626325 UTC
Line   313302: 07-18 16:52:24.182  root     0     0 I [0:   Binder:699_1:20972] PM: gpio_keys_report_state exit 2023-07-18 11:22:33.865724502 UTC
Line   313572: 07-18 16:52:35.111  root     0     0 I [1:   Binder:699_1:20972] PM: gpio_keys_report_state enrty 2023-07-18 11:22:37.678468979 UTC
Line   313573: 07-18 16:52:35.111  root     0     0 I [1:   Binder:699_1:20972] PM: gpio_keys_report_state exit 2023-07-18 11:22:37.678566167 UTC
Line   314209: 07-18 16:52:43.598  root     0     0 I [0:   Binder:699_1:20972] PM: gpio_keys_report_state enrty 2023-07-18 11:23:05.925340634 UTC
Line   314210: 07-18 16:52:43.598  root     0     0 I [0:   Binder:699_1:20972] PM: gpio_keys_report_state exit 2023-07-18 11:23:05.925439384 UTC


Without changes:

Line   372095: 07-18 16:10:24.250  root     0     0 I [1:   Binder:702_2:18137] PM: gpio_keys_report_state exit 2023-07-18 10:43:38.592548979 UTC
Line   372344: 07-18 16:13:45.439  root     0     0 I [0:   Binder:702_2:18137] PM: gpio_keys_report_state enrty 2023-07-18 10:44:11.589164226 UTC
Line   372346: 07-18 16:13:45.439  root     0     0 I [0:   Binder:702_2:18137] PM: gpio_keys_report_state exit 2023-07-18 10:44:11.589514955 UTC
Line   372573: 07-18 16:14:13.414  root     0     0 I [0:   Binder:702_2:18137] PM: gpio_keys_report_state enrty 2023-07-18 10:44:22.606227138 UTC
Line   372575: 07-18 16:14:13.414  root     0     0 I [0:   Binder:702_2:18137] PM: gpio_keys_report_state exit 2023-07-18 10:44:22.606490107 UTC
Line   372944: 07-18 16:14:26.732  root     0     0 I [1:   Binder:702_2:18137] PM: gpio_keys_report_state enrty 2023-07-18 10:44:29.024121927 UTC
Line   372946: 07-18 16:14:26.732  root     0     0 I [1:   Binder:702_2:18137] PM: gpio_keys_report_state exit 2023-07-18 10:44:29.024528958 UTC
Line   373181: 07-18 16:14:30.790  root     0     0 I [0:   Binder:702_2:18137] PM: gpio_keys_report_state enrty 2023-07-18 10:44:30.904866770 UTC
Line   373183: 07-18 16:14:30.790  root     0     0 I [0:   Binder:702_2:18137] PM: gpio_keys_report_state exit 2023-07-18 10:44:30.905126353 UTC




Thanks and Regards,
Abhishek Kumar Singh
Sr. Chief Engineer, Samsung Electronics, Noida-India


 
--------- Original Message ---------
Sender : dmitry.torokhov@gmail.com <dmitry.torokhov@gmail.com>
Date   : 2023-10-29 07:42 (GMT+5:30)
Title  : Re: [PATCH] input: gpio-keys - optimize wakeup sequence.
To : Abhishek Kumar Singh<abhi1.singh@samsung.com>
CC : robh@kernel.org<robh@kernel.org>, linux-input@vger.kernel.org<linux-input@vger.kernel.org>, linux-kernel@vger.kernel.org<linux-kernel@vger.kernel.org>, SRI-N IT Security<sri-n.itsec@samsung.com>
 
Hi Abhishek,

On Thu, Oct 26, 2023 at 11:23:20AM +0530, Abhishek Kumar Singh wrote:
> Dear Mr. Dmitry,
> Greetings!
> 
> 
> The patch removes unused many APIs call chain for every suspend/resume of the device 
> if no key press event triggered.
> 
> 
> There is a call back function gpio_keys_resume() called for
> every suspend/resume of the device. and whenever this function called, it is
> reading the status of the key. And gpio_keys_resume() API further calls the
> below chain of API irrespective of key press event
> 
> 
> APIs call chain:
> static void gpio_keys_report_state(struct gpio_keys_drvdata *ddata)
> static void gpio_keys_gpio_report_event(struct gpio_button_data *bdata)
> gpiod_get_value_cansleep(bdata->gpiod);
> input_event(input, type, *bdata->code, state);
> input_sync(input);
> 
> 
> The patch avoid the above APIs call chain if there is no key press event triggered.
> It will save the device computational resources, power resources and optimize the suspend/resume time

Unfortunately it also breaks the driver as button->value does not hold
the current state of the GPIO but rather set one via device tree so that
the driver can use that value when sending EV_ABS events. So with
typical GPIO-backed keys or buttons you change results in no events
reported on resume.

I also wonder what kind of measurements you did on improvements to
suspend/resume time with your change.

Thanks.

-- 
Dmitry

 
Thanks and Regards,
  
 
Thanks and Regards,
  

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.