From 7adb0f8a9fdb961692ffd2f0c65cacb155143f64 Mon Sep 17 00:00:00 2001 From: Jesse Hall Date: Wed, 6 Mar 2013 16:13:49 -0800 Subject: [PATCH] Minor cleanups/fixes before virtual display refactoring None of these should change behavior, except for removing some incorrect log messages when using a virtual display. - HWComposer::getAndResetReleaseFenceFd() checks the HWC version, so no need to do that in the DisplayDevice::onSwapBuffersCompleted(). However, it should check that mFramebufferSurface is not NULL like it is for virtual displays. - Comment that FramebufferSurface::dump() overrides the non-virtual ConsumerBase::dump(), and fix it so the right thing happens regardless of the static type of the pointer/reference the callee has. FramebufferSurface::dump() could be removed right now, but I'd need to bring it back in a later change. - Use the right enum for validating display type ids. - Don't try to send hotplug events for virtual displays. - Mark virtual displays as connected so HWComposer::prepare() doesn't think something is wrong when it gets a non-NULL layer list. - Remove unused FramebufferSurface methods. Bug: 8384764 Change-Id: Id28a2f9be86b45f4bb7915fdf7752157035f4294 --- include/gui/ConsumerBase.h | 7 +++---- services/surfaceflinger/DisplayDevice.cpp | 2 +- .../DisplayHardware/FramebufferSurface.cpp | 15 ++++++++------- .../DisplayHardware/FramebufferSurface.h | 9 ++++++--- .../DisplayHardware/HWComposer.cpp | 2 ++ services/surfaceflinger/EventThread.cpp | 16 ++++++++-------- services/surfaceflinger/SurfaceFlinger.cpp | 6 ++++-- 7 files changed, 32 insertions(+), 25 deletions(-) diff --git a/include/gui/ConsumerBase.h b/include/gui/ConsumerBase.h index 49a764f95..78a36089e 100644 --- a/include/gui/ConsumerBase.h +++ b/include/gui/ConsumerBase.h @@ -69,10 +69,9 @@ public: // ConsumerBase is connected. sp getBufferQueue() const; - // dump writes the current state to a string. These methods should NOT be - // overridden by child classes. Instead they should override the - // dumpLocked method, which is called by these methods after locking the - // mutex. + // dump writes the current state to a string. Child classes should add + // their state to the dump by overriding the dumpLocked method, which is + // called by these methods after locking the mutex. void dump(String8& result) const; void dump(String8& result, const char* prefix, char* buffer, size_t SIZE) const; diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp index 2bbe49cde..37a14c8fd 100644 --- a/services/surfaceflinger/DisplayDevice.cpp +++ b/services/surfaceflinger/DisplayDevice.cpp @@ -244,7 +244,7 @@ void DisplayDevice::swapBuffers(HWComposer& hwc) const { void DisplayDevice::onSwapBuffersCompleted(HWComposer& hwc) const { if (hwc.initCheck() == NO_ERROR) { - if (hwc.supportsFramebufferTarget()) { + if (mFramebufferSurface != NULL) { int fd = hwc.getAndResetReleaseFenceFd(mType); mFramebufferSurface->setReleaseFenceFd(fd); } diff --git a/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp b/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp index 7557e3fdc..960f9a953 100644 --- a/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp +++ b/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp @@ -134,7 +134,7 @@ status_t FramebufferSurface::setReleaseFenceFd(int fenceFd) { if (fenceFd >= 0) { sp fence(new Fence(fenceFd)); if (mCurrentBufferSlot != BufferQueue::INVALID_BUFFER_SLOT) { - status_t err = addReleaseFence(mCurrentBufferSlot, fence); + err = addReleaseFence(mCurrentBufferSlot, fence); ALOGE_IF(err, "setReleaseFenceFd: failed to add the fence: %s (%d)", strerror(-err), err); } @@ -142,21 +142,22 @@ status_t FramebufferSurface::setReleaseFenceFd(int fenceFd) { return err; } -status_t FramebufferSurface::setUpdateRectangle(const Rect& r) -{ - return INVALID_OPERATION; -} - status_t FramebufferSurface::compositionComplete() { return mHwc.fbCompositionComplete(); } void FramebufferSurface::dump(String8& result) { - mHwc.fbDump(result); ConsumerBase::dump(result); } +void FramebufferSurface::dumpLocked(String8& result, const char* prefix, + char* buffer, size_t SIZE) const +{ + mHwc.fbDump(result); + ConsumerBase::dumpLocked(result, prefix, buffer, SIZE); +} + // ---------------------------------------------------------------------------- }; // namespace android // ---------------------------------------------------------------------------- diff --git a/services/surfaceflinger/DisplayHardware/FramebufferSurface.h b/services/surfaceflinger/DisplayHardware/FramebufferSurface.h index b61b7f509..639e72061 100644 --- a/services/surfaceflinger/DisplayHardware/FramebufferSurface.h +++ b/services/surfaceflinger/DisplayHardware/FramebufferSurface.h @@ -36,11 +36,11 @@ class FramebufferSurface : public ConsumerBase { public: FramebufferSurface(HWComposer& hwc, int disp); - bool isUpdateOnDemand() const { return false; } - status_t setUpdateRectangle(const Rect& updateRect); status_t compositionComplete(); - virtual void dump(String8& result); + // TODO(jessehall): This overrides the non-virtual ConsumerBase version. + // Will rework slightly in a following change. + void dump(String8& result); // setReleaseFenceFd stores a fence file descriptor that will signal when the // current buffer is no longer being read. This fence will be returned to @@ -56,6 +56,9 @@ private: virtual void onFrameAvailable(); virtual void freeBufferLocked(int slotIndex); + virtual void dumpLocked(String8& result, const char* prefix, + char* buffer, size_t SIZE) const; + // nextBuffer waits for and then latches the next buffer from the // BufferQueue and releases the previously latched buffer to the // BufferQueue. The new buffer is returned in the 'buffer' argument. diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.cpp b/services/surfaceflinger/DisplayHardware/HWComposer.cpp index bb567e270..7df3671ab 100644 --- a/services/surfaceflinger/DisplayHardware/HWComposer.cpp +++ b/services/surfaceflinger/DisplayHardware/HWComposer.cpp @@ -380,6 +380,7 @@ int32_t HWComposer::allocateDisplayId() { } int32_t id = mAllocatedDisplayIDs.firstUnmarkedBit(); mAllocatedDisplayIDs.markBit(id); + mDisplayData[id].connected = true; return id; } @@ -392,6 +393,7 @@ status_t HWComposer::freeDisplayId(int32_t id) { return BAD_INDEX; } mAllocatedDisplayIDs.clearBit(id); + mDisplayData[id].connected = false; return NO_ERROR; } diff --git a/services/surfaceflinger/EventThread.cpp b/services/surfaceflinger/EventThread.cpp index edb9fa580..4d0fc79a1 100644 --- a/services/surfaceflinger/EventThread.cpp +++ b/services/surfaceflinger/EventThread.cpp @@ -41,7 +41,7 @@ EventThread::EventThread(const sp& flinger) mUseSoftwareVSync(false), mDebugVsyncEnabled(false) { - for (int32_t i=0 ; i= HWC_DISPLAY_TYPES_SUPPORTED, - "received event for an invalid display (id=%d)", type); + ALOGE_IF(type >= HWC_NUM_DISPLAY_TYPES, + "received vsync event for an invalid display (id=%d)", type); Mutex::Autolock _l(mLock); - if (type < HWC_DISPLAY_TYPES_SUPPORTED) { + if (type < HWC_NUM_DISPLAY_TYPES) { mVSyncEvent[type].header.type = DisplayEventReceiver::DISPLAY_EVENT_VSYNC; mVSyncEvent[type].header.id = type; mVSyncEvent[type].header.timestamp = timestamp; @@ -126,11 +126,11 @@ void EventThread::onVSyncReceived(int type, nsecs_t timestamp) { } void EventThread::onHotplugReceived(int type, bool connected) { - ALOGE_IF(type >= HWC_DISPLAY_TYPES_SUPPORTED, - "received event for an invalid display (id=%d)", type); + ALOGE_IF(type >= HWC_NUM_DISPLAY_TYPES, + "received hotplug event for an invalid display (id=%d)", type); Mutex::Autolock _l(mLock); - if (type < HWC_DISPLAY_TYPES_SUPPORTED) { + if (type < HWC_NUM_DISPLAY_TYPES) { DisplayEventReceiver::Event event; event.header.type = DisplayEventReceiver::DISPLAY_EVENT_HOTPLUG; event.header.id = type; @@ -184,7 +184,7 @@ Vector< sp > EventThread::waitForEvent( size_t vsyncCount = 0; nsecs_t timestamp = 0; - for (int32_t i=0 ; ionHotplugReceived(draw[i].type, false); + if (draw[i].type < DisplayDevice::NUM_DISPLAY_TYPES) + mEventThread->onHotplugReceived(draw[i].type, false); } else { ALOGW("trying to remove the main display"); } @@ -1204,7 +1205,8 @@ void SurfaceFlinger::handleTransactionLocked(uint32_t transactionFlags) state.viewport, state.frame); hw->setDisplayName(state.displayName); mDisplays.add(display, hw); - mEventThread->onHotplugReceived(state.type, true); + if (state.type < DisplayDevice::NUM_DISPLAY_TYPES) + mEventThread->onHotplugReceived(state.type, true); } } }