make sure we don't call into the HWC HAL when not needed

when enabling/disabling vsync we now make sure to
not call into the HAL if the state wouldn't change.

Bug: 7274951

Change-Id: Ie24a6d68888a51b577acf9c2a973d85437cbacaf

5 files changed, 47 insertions, 24 deletions

diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.cpp b/services/surfaceflinger/DisplayHardware/HWComposer.cpp
index 302ce82..597e5d6 100644
--- a/services/surfaceflinger/DisplayHardware/HWComposer.cpp
+++ b/services/surfaceflinger/DisplayHardware/HWComposer.cpp
@@ -147,7 +147,7 @@ HWComposer::HWComposer(
         // don't need a vsync thread if we have a hardware composer
         needVSyncThread = false;
         // always turn vsync off when we start
-        mHwc->eventControl(mHwc, HWC_DISPLAY_PRIMARY, HWC_EVENT_VSYNC, 0);
+        eventControl(HWC_DISPLAY_PRIMARY, HWC_EVENT_VSYNC, 0);
 
         // these IDs are always reserved
         for (size_t i=0 ; i<HWC_NUM_DISPLAY_TYPES ; i++) {
@@ -202,7 +202,7 @@ HWComposer::HWComposer(
 
 HWComposer::~HWComposer() {
     if (mHwc) {
-        mHwc->eventControl(mHwc, HWC_DISPLAY_PRIMARY, HWC_EVENT_VSYNC, 0);
+        eventControl(HWC_DISPLAY_PRIMARY, HWC_EVENT_VSYNC, 0);
     }
     if (mVSyncThread != NULL) {
         mVSyncThread->requestExitAndWait();
@@ -443,16 +443,32 @@ bool HWComposer::isConnected(int disp) const {
     return mDisplayData[disp].connected;
 }
 
-void HWComposer::eventControl(int event, int enabled) {
+void HWComposer::eventControl(int disp, int event, int enabled) {
+    if (uint32_t(disp)>31 || !mAllocatedDisplayIDs.hasBit(disp)) {
+        return;
+    }
     status_t err = NO_ERROR;
-    if (mHwc) {
-        if (!mDebugForceFakeVSync) {
-            err = mHwc->eventControl(mHwc, 0, event, enabled);
-            // error here should not happen -- not sure what we should
-            // do if it does.
-            ALOGE_IF(err, "eventControl(%d, %d) failed %s",
-                    event, enabled, strerror(-err));
+    if (mHwc && !mDebugForceFakeVSync) {
+        // NOTE: we use our own internal lock here because we have to call
+        // into the HWC with the lock held, and we want to make sure
+        // that even if HWC blocks (which it shouldn't), it won't
+        // affect other threads.
+        Mutex::Autolock _l(mEventControlLock);
+        const int32_t eventBit = 1UL << event;
+        const int32_t newValue = enabled ? eventBit : 0;
+        const int32_t oldValue = mDisplayData[disp].events & eventBit;
+        if (newValue != oldValue) {
+            ATRACE_CALL();
+            err = mHwc->eventControl(mHwc, disp, event, enabled);
+            if (!err) {
+                int32_t& events(mDisplayData[disp].events);
+                events = (events & ~eventBit) | newValue;
+            }
         }
+        // error here should not happen -- not sure what we should
+        // do if it does.
+        ALOGE_IF(err, "eventControl(%d, %d) failed %s",
+                event, enabled, strerror(-err));
     }
 
     if (err == NO_ERROR && mVSyncThread != NULL) {
@@ -652,16 +668,16 @@ status_t HWComposer::commit() {
     return (status_t)err;
 }
 
-status_t HWComposer::release(int disp) const {
+status_t HWComposer::release(int disp) {
     LOG_FATAL_IF(disp >= HWC_NUM_DISPLAY_TYPES);
     if (mHwc) {
-        mHwc->eventControl(mHwc, disp, HWC_EVENT_VSYNC, 0);
+        eventControl(disp, HWC_EVENT_VSYNC, 0);
         return (status_t)mHwc->blank(mHwc, disp, 1);
     }
     return NO_ERROR;
 }
 
-status_t HWComposer::acquire(int disp) const {
+status_t HWComposer::acquire(int disp) {
     LOG_FATAL_IF(disp >= HWC_NUM_DISPLAY_TYPES);
     if (mHwc) {
         return (status_t)mHwc->blank(mHwc, disp, 0);
@@ -965,8 +981,10 @@ HWComposer::VSyncThread::VSyncThread(HWComposer& hwc)
 
 void HWComposer::VSyncThread::setEnabled(bool enabled) {
     Mutex::Autolock _l(mLock);
-    mEnabled = enabled;
-    mCondition.signal();
+    if (mEnabled != enabled) {
+        mEnabled = enabled;
+        mCondition.signal();
+    }
 }
 
 void HWComposer::VSyncThread::onFirstRef() {
diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.h b/services/surfaceflinger/DisplayHardware/HWComposer.h
index af33999..269e147 100644
--- a/services/surfaceflinger/DisplayHardware/HWComposer.h
+++ b/services/surfaceflinger/DisplayHardware/HWComposer.h
@@ -92,10 +92,10 @@ public:
     status_t commit();
 
     // release hardware resources and blank screen
-    status_t release(int disp) const;
+    status_t release(int disp);
 
     // acquire hardware resources and unblank screen
-    status_t acquire(int disp) const;
+    status_t acquire(int disp);
 
     // reset state when an external, non-virtual display is disconnected
     void disconnectDisplay(int disp);
@@ -226,7 +226,7 @@ public:
         EVENT_VSYNC = HWC_EVENT_VSYNC
     };
 
-    void eventControl(int event, int enabled);
+    void eventControl(int disp, int event, int enabled);
 
     // Query display parameters.  Pass in a display index (e.g.
     // HWC_DISPLAY_PRIMARY).
@@ -289,7 +289,7 @@ private:
         DisplayData() : xdpi(0), ydpi(0), refresh(0),
             connected(false), hasFbComp(false), hasOvComp(false),
             capacity(0), list(NULL),
-            framebufferTarget(NULL), fbTargetHandle(NULL) { }
+            framebufferTarget(NULL), fbTargetHandle(NULL), events(0) { }
         ~DisplayData() {
             free(list);
         }
@@ -306,6 +306,8 @@ private:
         hwc_display_contents_1* list;
         hwc_layer_1* framebufferTarget;
         buffer_handle_t fbTargetHandle;
+        // protected by mEventControlLock
+        int32_t events;
     };
 
     sp<SurfaceFlinger>              mFlinger;
@@ -327,6 +329,9 @@ private:
     // protected by mLock
     mutable Mutex mLock;
     mutable nsecs_t mLastHwVSync;
+
+    // thread-safe
+    mutable Mutex mEventControlLock;
 };
 
 // ---------------------------------------------------------------------------
diff --git a/services/surfaceflinger/EventThread.cpp b/services/surfaceflinger/EventThread.cpp
index 6b7c78b..edb9fa5 100644
--- a/services/surfaceflinger/EventThread.cpp
+++ b/services/surfaceflinger/EventThread.cpp
@@ -308,14 +308,14 @@ Vector< sp<EventThread::Connection> > EventThread::waitForEvent(
 void EventThread::enableVSyncLocked() {
     if (!mUseSoftwareVSync) {
         // never enable h/w VSYNC when screen is off
-        mFlinger->eventControl(SurfaceFlinger::EVENT_VSYNC, true);
+        mFlinger->eventControl(HWC_DISPLAY_PRIMARY, SurfaceFlinger::EVENT_VSYNC, true);
         mPowerHAL.vsyncHint(true);
     }
     mDebugVsyncEnabled = true;
 }
 
 void EventThread::disableVSyncLocked() {
-    mFlinger->eventControl(SurfaceFlinger::EVENT_VSYNC, false);
+    mFlinger->eventControl(HWC_DISPLAY_PRIMARY, SurfaceFlinger::EVENT_VSYNC, false);
     mPowerHAL.vsyncHint(false);
     mDebugVsyncEnabled = false;
 }
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index ebec4cb..8160a7a 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -777,8 +777,8 @@ void SurfaceFlinger::onHotplugReceived(int type, bool connected) {
     }
 }
 
-void SurfaceFlinger::eventControl(int event, int enabled) {
-    getHwComposer().eventControl(event, enabled);
+void SurfaceFlinger::eventControl(int disp, int event, int enabled) {
+    getHwComposer().eventControl(disp, event, enabled);
 }
 
 void SurfaceFlinger::onMessageReceived(int32_t what) {
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 8272848..5bb3703 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -123,7 +123,7 @@ public:
 
     // enable/disable h/w composer event
     // TODO: this should be made accessible only to EventThread
-    void eventControl(int event, int enabled);
+    void eventControl(int disp, int event, int enabled);
 
     // called on the main thread by MessageQueue when an internal message
     // is received