Merge lp:~vanvugt/mir/wake-lock-on-input into lp:mir

[Daniel van Vugt]

Re-introduce wake locks to Android input (LP: #1465514)

It appears there was no good reason for us to have ever commented this
out in Mir. Upstream Android still has it, and in theory input
responsiveness will be helped by ensuring the CPU never goes to low-power
mode in the middle of processing an input event. Which it might, as we
have to dance between server and client processes which takes system calls
and context switches.

Although I am yet to see this make a measurable difference, it doesn't
make anything worse either. And in theory in some cases it should
improve performance.

Note (i): To get wake lock functionality you need to be root (ie. a
system compositor). This is because of the restricted permissions on /sys/power/*. I don't think it matters if you fail to open them.

Note (ii): Regular Linux kernels implement the same /sys/power interface now, as first introduced in Android kernels. But most desktop kernels
don't implement the behaviour behind it by default (it's an option
apparently).

Note (iii): Yes power.c is Android code. I'm trying to avoid modifying it.

This is a request for comments/testing. There might be a phone where this makes a significant difference but I've only tried mako so far...

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:2676
http://jenkins.qa.ubuntu.com/job/mir-ci/4149/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/2926
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/441/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/2874
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/305
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/305/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2874
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2874/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/5700
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/21318

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/4149/rebuild

review: Needs Fixing (continuous-integration)

[Andreas Pokorny (andreas-pokorny)]

Better look where the calls to acquire_wake_lock and release_wake_lock are done.
Why should that improve the input event transfer to the nested server and client?
It should not have any effect at all, unless of course the auto suspend is configured to be active.
So this does not affect dynamic/runtime PM or cpu frequency, this is only needed to keep the system from suspending it entirely. Which is not used in ubuntu phone.

[Alberto Aguirre (albaguirre)]

There is absolutely no need for MIR to acquire wakelocks ever. Wakelocks are a suspend prevention mechanism which in Ubuntu Touch is taken care of by powerd.

The user is already interacting with the screen in any case so the system will not be suspending during interaction.

review: Disapprove

Unmerged revisions

2676. By Daniel van Vugt on 2015-06-19

Merge latest trunk and fix conflict

2675. By Daniel van Vugt on 2015-06-17

Reintroduce wake locks from the Android source tree

2674. By Daniel van Vugt on 2015-06-17

sendFinishedSignal at the correct time.

=== modified file '3rd_party/android-input/android/CMakeLists.txt'
--- 3rd_party/android-input/android/CMakeLists.txt	2015-06-17 05:20:42 +0000
+++ 3rd_party/android-input/android/CMakeLists.txt	2015-06-19 06:37:26 +0000
@@ -15,6 +15,11 @@
   -DANDROID_SMP=1
   )
 
+# Required by power.c:
+set(CMAKE_C_FLAGS
+  "${CMAKE_C_FLAGS} -D_POSIX_C_SOURCE=200809L -std=c99 -Wno-unused-function"
+)
+
 # This is stuff for which we want non-android versions
 set(
   UTIL_SOURCES
@@ -28,6 +33,7 @@
 add_library(
   android-input-static STATIC
   # The stuff that we want
+  hardware/libhardware_legacy/power/power.c
   frameworks/base/services/input/EventHub.cpp
   frameworks/base/services/input/InputApplication.cpp
   frameworks/base/services/input/InputDispatcher.cpp

=== modified file '3rd_party/android-input/android/frameworks/base/services/input/EventHub.cpp'
--- 3rd_party/android-input/android/frameworks/base/services/input/EventHub.cpp	2015-06-17 05:20:42 +0000
+++ 3rd_party/android-input/android/frameworks/base/services/input/EventHub.cpp	2015-06-19 06:37:26 +0000
@@ -20,12 +20,11 @@
 
 #include "EventHub.h"
 
-#define acquire_wake_lock(lock, id) {}
-#define release_wake_lock(id) {}
-
 #include "mir/input/input_report.h"
 #include "mir/udev/wrapper.h"
 
+#include <hardware_legacy/power.h>
+
 #include <cutils/properties.h>
 #include <std/Log.h>
 #include <std/Timers.h>

=== added directory '3rd_party/android-input/android/hardware/libhardware_legacy/power'
=== added file '3rd_party/android-input/android/hardware/libhardware_legacy/power/power.c'
--- 3rd_party/android-input/android/hardware/libhardware_legacy/power/power.c	1970-01-01 00:00:00 +0000
+++ 3rd_party/android-input/android/hardware/libhardware_legacy/power/power.c	2015-06-19 06:37:26 +0000
@@ -0,0 +1,125 @@
+/*
+ * Copyright (C) 2008 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#include <hardware_legacy/power.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <sys/time.h>
+#include <time.h>
+#include <errno.h>
+#include <string.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <pthread.h>
+
+//#define LOG_TAG "power"
+//#include <utils/Log.h>
+
+enum {
+    ACQUIRE_PARTIAL_WAKE_LOCK = 0,
+    RELEASE_WAKE_LOCK,
+    OUR_FD_COUNT
+};
+
+const char * const OLD_PATHS[] = {
+    "/sys/android_power/acquire_partial_wake_lock",
+    "/sys/android_power/release_wake_lock",
+};
+
+const char * const NEW_PATHS[] = {
+    "/sys/power/wake_lock",
+    "/sys/power/wake_unlock",
+};
+
+//XXX static pthread_once_t g_initialized = THREAD_ONCE_INIT;
+static int g_initialized = 0;
+static int g_fds[OUR_FD_COUNT];
+static int g_error = 1;
+
+static int64_t systemTime()
+{
+    struct timespec t;
+    t.tv_sec = t.tv_nsec = 0;
+    clock_gettime(CLOCK_MONOTONIC, &t);
+    return t.tv_sec*1000000000LL + t.tv_nsec;
+}
+
+static int
+open_file_descriptors(const char * const paths[])
+{
+    int i;
+    for (i=0; i<OUR_FD_COUNT; i++) {
+        int fd = open(paths[i], O_RDWR | O_CLOEXEC);
+        if (fd < 0) {
+            fprintf(stderr, "fatal error opening \"%s\"\n", paths[i]);
+            g_error = errno;
+            return -1;
+        }
+        g_fds[i] = fd;
+    }
+
+    g_error = 0;
+    return 0;
+}
+
+static inline void
+initialize_fds(void)
+{
+    // XXX: should be this:
+    //pthread_once(&g_initialized, open_file_descriptors);
+    // XXX: not this:
+    if (g_initialized == 0) {
+        if(open_file_descriptors(NEW_PATHS) < 0)
+            open_file_descriptors(OLD_PATHS);
+        g_initialized = 1;
+    }
+}
+
+int
+acquire_wake_lock(int lock, const char* id)
+{
+    initialize_fds();
+
+//    ALOGI("acquire_wake_lock lock=%d id='%s'\n", lock, id);
+
+    if (g_error) return g_error;
+
+    int fd;
+
+    if (lock == PARTIAL_WAKE_LOCK) {
+        fd = g_fds[ACQUIRE_PARTIAL_WAKE_LOCK];
+    }
+    else {
+        return EINVAL;
+    }
+
+    return write(fd, id, strlen(id));
+}
+
+int
+release_wake_lock(const char* id)
+{
+    initialize_fds();
+
+//    ALOGI("release_wake_lock id='%s'\n", id);
+
+    if (g_error) return g_error;
+
+    ssize_t len = write(g_fds[RELEASE_WAKE_LOCK], id, strlen(id));
+    return len >= 0;
+}