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

[Abhishek Kumar Singh]

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@xxxxxxxxx <dmitry.torokhov@xxxxxxxxx>
Date   : 2023-10-29 07:42 (GMT+5:30)
Title  : Re: [PATCH] input: gpio-keys - optimize wakeup sequence.
To : Abhishek Kumar Singh<abhi1.singh@xxxxxxxxxxx>
CC : robh@xxxxxxxxxx<robh@xxxxxxxxxx>, linux-input@xxxxxxxxxxxxxxx<linux-input@xxxxxxxxxxxxxxx>, linux-kernel@xxxxxxxxxxxxxxx<linux-kernel@xxxxxxxxxxxxxxx>, SRI-N IT Security<sri-n.itsec@xxxxxxxxxxx>
 
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,