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

diff --git a/include/gui/ConsumerBase.h b/include/gui/ConsumerBase.h
index 49a764f..78a3608 100644 (file)
--- a/include/gui/ConsumerBase.h
+++ b/include/gui/ConsumerBase.h
@@ -69,10 +69,9 @@ public:
     // ConsumerBase is connected.
     sp<BufferQueue> 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 2bbe49c..37a14c8 100644 (file)
--- 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 7557e3f..960f9a9 100644 (file)
--- 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> 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 b61b7f5..639e720 100644 (file)
--- 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 bb567e2..7df3671 100644 (file)
--- 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 edb9fa5..4d0fc79 100644 (file)
--- a/services/surfaceflinger/EventThread.cpp
+++ b/services/surfaceflinger/EventThread.cpp
@@ -41,7 +41,7 @@ EventThread::EventThread(const sp<SurfaceFlinger>& flinger)
       mUseSoftwareVSync(false),
       mDebugVsyncEnabled(false) {
 
-    for (int32_t i=0 ; i<HWC_DISPLAY_TYPES_SUPPORTED ; i++) {
+    for (int32_t i=0 ; i<HWC_NUM_DISPLAY_TYPES ; i++) {
         mVSyncEvent[i].header.type = DisplayEventReceiver::DISPLAY_EVENT_VSYNC;
         mVSyncEvent[i].header.id = 0;
         mVSyncEvent[i].header.timestamp = 0;
@@ -112,11 +112,11 @@ void EventThread::onScreenAcquired() {
 
 
 void EventThread::onVSyncReceived(int type, nsecs_t timestamp) {
-    ALOGE_IF(type >= 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::Connection> > EventThread::waitForEvent(
 
         size_t vsyncCount = 0;
         nsecs_t timestamp = 0;
-        for (int32_t i=0 ; i<HWC_DISPLAY_TYPES_SUPPORTED ; i++) {
+        for (int32_t i=0 ; i<HWC_NUM_DISPLAY_TYPES ; i++) {
             timestamp = mVSyncEvent[i].header.timestamp;
             if (timestamp) {
                 // we have a vsync event to dispatch

diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 98f6ded..551f902 100644 (file)
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -1131,7 +1131,8 @@ void SurfaceFlinger::handleTransactionLocked(uint32_t transactionFlags)
                         DisplayDevice::makeCurrent(mEGLDisplay, hw, mEGLContext);
                         mDisplays.removeItem(draw.keyAt(i));
                         getHwComposer().disconnectDisplay(draw[i].type);
-                        mEventThread->onHotplugReceived(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);
                     }
                 }
             }