From 81cd5d3b94d21253a0be925f4ae58cc7f4afeef7 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Thu, 4 Oct 2012 02:34:38 -0700 Subject: [PATCH] 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 --- .../DisplayHardware/HWComposer.cpp | 48 +++++++++++++------ .../DisplayHardware/HWComposer.h | 13 +++-- services/surfaceflinger/EventThread.cpp | 4 +- services/surfaceflinger/SurfaceFlinger.cpp | 4 +- services/surfaceflinger/SurfaceFlinger.h | 2 +- 5 files changed, 47 insertions(+), 24 deletions(-) diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.cpp b/services/surfaceflinger/DisplayHardware/HWComposer.cpp index 302ce8255..597e5d630 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 ; ieventControl(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 af33999dc..269e1472a 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 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 6b7c78bbd..edb9fa580 100644 --- a/services/surfaceflinger/EventThread.cpp +++ b/services/surfaceflinger/EventThread.cpp @@ -308,14 +308,14 @@ Vector< sp > 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 ebec4cb82..8160a7aba 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 82728484a..5bb370368 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