Allow a primary display disconnect
This patch forwards the primary display disconnect event to the
Framework, and otherwise ensures that SurfaceFlinger does not crash
while there is no primary display.
Note that the Framework does not yet accept this change. In particular
the ActivityManager ActivityStackSupervisor code promptly asserts that
one cannot remove the primary display. With this assertion disabled, the
framework does not crash (surprisingly). And if the Framework
subsequently receives a primary display connect event, it does not seem
to do anything useful -- the display remains in a default off state, and
no layer stack/viewport/etc is set on it.
Bug: 38464421
Test: Works (with workarounds as noted) on a Chromebook
Test: Added Unit test passes on Pixel 1 XL
Change-Id: Ia11439030efdc53bc17474b71a0ffb3d3085bb49
diff --git a/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp b/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp
index 8b5d4c9..4faba3b 100644
--- a/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp
+++ b/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp
@@ -76,6 +76,10 @@
SurfaceFlinger::maxFrameBufferAcquiredBuffers - 1);
}
+void FramebufferSurface::resizeBuffers(const uint32_t width, const uint32_t height) {
+ mConsumer->setDefaultBufferSize(width, height);
+}
+
status_t FramebufferSurface::beginFrame(bool /*mustRecompose*/) {
return NO_ERROR;
}
diff --git a/services/surfaceflinger/DisplayHardware/FramebufferSurface.h b/services/surfaceflinger/DisplayHardware/FramebufferSurface.h
index eaa5455..ed756c4 100644
--- a/services/surfaceflinger/DisplayHardware/FramebufferSurface.h
+++ b/services/surfaceflinger/DisplayHardware/FramebufferSurface.h
@@ -46,9 +46,7 @@
virtual void onFrameCommitted();
virtual void dumpAsString(String8& result) const;
- // Cannot resize a buffers in a FramebufferSurface. Only works with virtual
- // displays.
- virtual void resizeBuffers(const uint32_t /*w*/, const uint32_t /*h*/) { };
+ virtual void resizeBuffers(const uint32_t width, const uint32_t height);
virtual const sp<Fence>& getClientTargetAcquireFence() const override;
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 3145ac3..bfa239d 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -798,7 +798,7 @@
// TODO: this needs to go away (currently needed only by webkit)
sp<const DisplayDevice> hw(getDefaultDisplayDeviceLocked());
- info.orientation = hw->getOrientation();
+ info.orientation = hw ? hw->getOrientation() : 0;
} else {
// TODO: where should this value come from?
static const int TV_DENSITY = 213;
@@ -1596,7 +1596,7 @@
getBE().mGlCompositionDoneTimeline.updateSignalTimes();
std::shared_ptr<FenceTime> glCompositionDoneFenceTime;
- if (getBE().mHwc->hasClientComposition(HWC_DISPLAY_PRIMARY)) {
+ if (hw && getBE().mHwc->hasClientComposition(HWC_DISPLAY_PRIMARY)) {
glCompositionDoneFenceTime =
std::make_shared<FenceTime>(hw->getClientTargetAcquireFence());
getBE().mGlCompositionDoneTimeline.push(glCompositionDoneFenceTime);
@@ -1641,7 +1641,7 @@
}
if (!hasSyncFramework) {
- if (hw->isDisplayOn()) {
+ if (getBE().mHwc->isConnected(HWC_DISPLAY_PRIMARY) && hw->isDisplayOn()) {
enableHardwareVsync();
}
}
@@ -1652,7 +1652,7 @@
if (presentFenceTime->isValid()) {
mAnimFrameTracker.setActualPresentFence(
std::move(presentFenceTime));
- } else {
+ } else if (getBE().mHwc->isConnected(HWC_DISPLAY_PRIMARY)) {
// The HWC doesn't support present fences, so use the refresh
// timestamp instead.
nsecs_t presentTime =
@@ -1662,7 +1662,8 @@
mAnimFrameTracker.advanceFrame();
}
- if (hw->getPowerMode() == HWC_POWER_MODE_OFF) {
+ if (getBE().mHwc->isConnected(HWC_DISPLAY_PRIMARY) &&
+ hw->getPowerMode() == HWC_POWER_MODE_OFF) {
return;
}
@@ -2005,9 +2006,11 @@
mDebugInSwapBuffers = 0;
// |mStateLock| not needed as we are on the main thread
- uint32_t flipCount = getDefaultDisplayDeviceLocked()->getPageFlipCount();
- if (flipCount % LOG_FRAME_STATS_PERIOD == 0) {
- logFrameStats();
+ if (getBE().mHwc->isConnected(HWC_DISPLAY_PRIMARY)) {
+ uint32_t flipCount = getDefaultDisplayDeviceLocked()->getPageFlipCount();
+ if (flipCount % LOG_FRAME_STATS_PERIOD == 0) {
+ logFrameStats();
+ }
}
}
@@ -2129,20 +2132,16 @@
const ssize_t j = curr.indexOfKey(draw.keyAt(i));
if (j < 0) {
// in drawing state but not in current state
- if (!draw[i].isMainDisplay()) {
- // Call makeCurrent() on the primary display so we can
- // be sure that nothing associated with this display
- // is current.
- const sp<const DisplayDevice> defaultDisplay(getDefaultDisplayDeviceLocked());
- defaultDisplay->makeCurrent();
- sp<DisplayDevice> hw(getDisplayDeviceLocked(draw.keyAt(i)));
- if (hw != nullptr) hw->disconnect(getHwComposer());
- if (draw[i].type < DisplayDevice::NUM_BUILTIN_DISPLAY_TYPES)
- mEventThread->onHotplugReceived(draw[i].type, false);
- mDisplays.removeItem(draw.keyAt(i));
- } else {
- ALOGW("trying to remove the main display");
- }
+ // Call makeCurrent() on the primary display so we can
+ // be sure that nothing associated with this display
+ // is current.
+ const sp<const DisplayDevice> defaultDisplay(getDefaultDisplayDeviceLocked());
+ if (defaultDisplay != nullptr) defaultDisplay->makeCurrent();
+ sp<DisplayDevice> hw(getDisplayDeviceLocked(draw.keyAt(i)));
+ if (hw != nullptr) hw->disconnect(getHwComposer());
+ if (draw[i].type < DisplayDevice::NUM_BUILTIN_DISPLAY_TYPES)
+ mEventThread->onHotplugReceived(draw[i].type, false);
+ mDisplays.removeItem(draw.keyAt(i));
} else {
// this display is in both lists. see if something changed.
const DisplayDeviceState& state(curr[j]);
@@ -2377,6 +2376,9 @@
}
layer->updateTransformHint(disp);
}
+ if (disp != nullptr) {
+ layer->updateTransformHint(disp);
+ }
first = false;
});
@@ -3912,9 +3914,11 @@
getBE().mRenderEngine->dump(result);
- hw->undefinedRegion.dump(result, "undefinedRegion");
- result.appendFormat(" orientation=%d, isDisplayOn=%d\n",
- hw->getOrientation(), hw->isDisplayOn());
+ if (hw) {
+ hw->undefinedRegion.dump(result, "undefinedRegion");
+ result.appendFormat(" orientation=%d, isDisplayOn=%d\n",
+ hw->getOrientation(), hw->isDisplayOn());
+ }
result.appendFormat(
" last eglSwapBuffers() time: %f us\n"
" last transaction time : %f us\n"
diff --git a/services/surfaceflinger/tests/fakehwc/FakeComposerClient.cpp b/services/surfaceflinger/tests/fakehwc/FakeComposerClient.cpp
index cb22932..e16e7ec 100644
--- a/services/surfaceflinger/tests/fakehwc/FakeComposerClient.cpp
+++ b/services/surfaceflinger/tests/fakehwc/FakeComposerClient.cpp
@@ -181,6 +181,12 @@
}
}
+void FakeComposerClient::refreshDisplay(Display display) {
+ if (mCallbacksOn) {
+ mClient->onRefresh(display);
+ }
+}
+
uint32_t FakeComposerClient::getMaxVirtualDisplayCount() {
ALOGV("getMaxVirtualDisplayCount");
return 1;
diff --git a/services/surfaceflinger/tests/fakehwc/FakeComposerClient.h b/services/surfaceflinger/tests/fakehwc/FakeComposerClient.h
index de8cffd..cef7f5b 100644
--- a/services/surfaceflinger/tests/fakehwc/FakeComposerClient.h
+++ b/services/surfaceflinger/tests/fakehwc/FakeComposerClient.h
@@ -142,6 +142,7 @@
Layer getLayer(size_t index) const;
void hotplugDisplay(Display display, IComposerCallback::Connection state);
+ void refreshDisplay(Display display);
private:
LayerImpl& getLayerImpl(Layer handle);
diff --git a/services/surfaceflinger/tests/fakehwc/SFFakeHwc_test.cpp b/services/surfaceflinger/tests/fakehwc/SFFakeHwc_test.cpp
index 8a97ea4..1873832 100644
--- a/services/surfaceflinger/tests/fakehwc/SFFakeHwc_test.cpp
+++ b/services/surfaceflinger/tests/fakehwc/SFFakeHwc_test.cpp
@@ -22,25 +22,22 @@
#include "FakeComposerService.h"
#include "FakeComposerUtils.h"
+#include <gui/DisplayEventReceiver.h>
#include <gui/ISurfaceComposer.h>
#include <gui/LayerDebugInfo.h>
#include <gui/LayerState.h>
#include <gui/Surface.h>
#include <gui/SurfaceComposerClient.h>
-#include <private/gui/ComposerService.h>
-
-#include <ui/DisplayInfo.h>
-
-#include <android/native_window.h>
-
#include <android/hidl/manager/1.0/IServiceManager.h>
-
-#include <hwbinder/ProcessState.h>
-
+#include <android/looper.h>
+#include <android/native_window.h>
#include <binder/ProcessState.h>
-
+#include <hwbinder/ProcessState.h>
#include <log/log.h>
+#include <private/gui/ComposerService.h>
+#include <ui/DisplayInfo.h>
+#include <utils/Looper.h>
#include <gmock/gmock.h>
#include <gtest/gtest.h>
@@ -142,13 +139,22 @@
};
protected:
+ static int processDisplayEvents(int fd, int events, void* data);
+
void SetUp() override;
void TearDown() override;
+ void waitForDisplayTransaction();
+ bool waitForHotplugEvent(uint32_t id, bool connected);
+
sp<IComposer> mFakeService;
sp<SurfaceComposerClient> mComposerClient;
MockComposerClient* mMockComposer;
+
+ std::unique_ptr<DisplayEventReceiver> mReceiver;
+ sp<Looper> mLooper;;
+ std::deque<DisplayEventReceiver::Event> mReceivedDisplayEvents;
};
void DisplayTest::SetUp() {
@@ -188,9 +194,16 @@
mComposerClient = new SurfaceComposerClient;
ASSERT_EQ(NO_ERROR, mComposerClient->initCheck());
+
+ mReceiver.reset(new DisplayEventReceiver());
+ mLooper = new Looper(false);
+ mLooper->addFd(mReceiver->getFd(), 0, ALOOPER_EVENT_INPUT, processDisplayEvents, this);
}
void DisplayTest::TearDown() {
+ mLooper = nullptr;
+ mReceiver = nullptr;
+
mComposerClient->dispose();
mComposerClient = nullptr;
@@ -204,6 +217,55 @@
mMockComposer = nullptr;
}
+
+int DisplayTest::processDisplayEvents(int /*fd*/, int /*events*/, void* data) {
+ auto self = static_cast<DisplayTest*>(data);
+
+ ssize_t n;
+ DisplayEventReceiver::Event buffer[1];
+
+ while ((n = self->mReceiver->getEvents(buffer, 1)) > 0) {
+ for (int i=0 ; i<n ; i++) {
+ self->mReceivedDisplayEvents.push_back(buffer[i]);
+ }
+ }
+ ALOGD_IF(n < 0, "Error reading events (%s)\n", strerror(-n));
+ return 1;
+}
+
+void DisplayTest::waitForDisplayTransaction() {
+ // Both a refresh and a vsync event are needed to apply pending display
+ // transactions.
+ mMockComposer->refreshDisplay(EXTERNAL_DISPLAY);
+ mMockComposer->runVSyncAndWait();
+
+ // Extra vsync and wait to avoid a 10% flake due to a race.
+ mMockComposer->runVSyncAndWait();
+}
+
+bool DisplayTest::waitForHotplugEvent(uint32_t id, bool connected) {
+ int waitCount = 20;
+ while (waitCount--) {
+ while (!mReceivedDisplayEvents.empty()) {
+ auto event = mReceivedDisplayEvents.front();
+ mReceivedDisplayEvents.pop_front();
+
+ ALOGV_IF(event.header.type == DisplayEventReceiver::DISPLAY_EVENT_HOTPLUG,
+ "event hotplug: id %d, connected %d\t", event.header.id,
+ event.hotplug.connected);
+
+ if (event.header.type == DisplayEventReceiver::DISPLAY_EVENT_HOTPLUG &&
+ event.header.id == id && event.hotplug.connected == connected) {
+ return true;
+ }
+ }
+
+ mLooper->pollOnce(1);
+ }
+
+ return false;
+}
+
TEST_F(DisplayTest, Hotplug) {
ALOGD("DisplayTest::Hotplug");
@@ -215,7 +277,7 @@
EXPECT_CALL(*mMockComposer, getDisplayAttribute(EXTERNAL_DISPLAY, 1, _, _))
.Times(2 * 3)
.WillRepeatedly(Invoke(mMockComposer, &MockComposerClient::getDisplayAttributeFake));
- // ... and then special handling for dimensions. Specifying this
+ // ... and then special handling for dimensions. Specifying these
// rules later means that gmock will try them first, i.e.,
// ordering of width/height vs. the default implementation for
// other queries is significant.
@@ -229,11 +291,12 @@
.Times(2)
.WillRepeatedly(DoAll(SetArgPointee<3>(200), Return(Error::NONE)));
- // TODO: Width and height queries are not actually called. Display
- // info returns dimensions 0x0 in display info. Why?
-
mMockComposer->hotplugDisplay(EXTERNAL_DISPLAY, IComposerCallback::Connection::CONNECTED);
+ waitForDisplayTransaction();
+
+ EXPECT_TRUE(waitForHotplugEvent(ISurfaceComposer::eDisplayIdHdmi, true));
+
{
sp<android::IBinder> display(
SurfaceComposerClient::getBuiltInDisplay(ISurfaceComposer::eDisplayIdHdmi));
@@ -264,6 +327,11 @@
mMockComposer->hotplugDisplay(EXTERNAL_DISPLAY, IComposerCallback::Connection::CONNECTED);
+ waitForDisplayTransaction();
+
+ EXPECT_TRUE(waitForHotplugEvent(ISurfaceComposer::eDisplayIdHdmi, false));
+ EXPECT_TRUE(waitForHotplugEvent(ISurfaceComposer::eDisplayIdHdmi, true));
+
{
sp<android::IBinder> display(
SurfaceComposerClient::getBuiltInDisplay(ISurfaceComposer::eDisplayIdHdmi));
@@ -290,6 +358,64 @@
mMockComposer->hotplugDisplay(EXTERNAL_DISPLAY, IComposerCallback::Connection::DISCONNECTED);
}
+TEST_F(DisplayTest, HotplugPrimaryDisplay) {
+ ALOGD("DisplayTest::HotplugPrimaryDisplay");
+
+ mMockComposer->hotplugDisplay(PRIMARY_DISPLAY, IComposerCallback::Connection::DISCONNECTED);
+
+ waitForDisplayTransaction();
+
+ EXPECT_TRUE(waitForHotplugEvent(ISurfaceComposer::eDisplayIdMain, false));
+
+ {
+ sp<android::IBinder> display(
+ SurfaceComposerClient::getBuiltInDisplay(ISurfaceComposer::eDisplayIdMain));
+ DisplayInfo info;
+ auto result = SurfaceComposerClient::getDisplayInfo(display, &info);
+ EXPECT_NE(NO_ERROR, result);
+ }
+
+ mMockComposer->clearFrames();
+
+ EXPECT_CALL(*mMockComposer, getDisplayType(PRIMARY_DISPLAY, _))
+ .Times(2)
+ .WillRepeatedly(DoAll(SetArgPointee<1>(IComposerClient::DisplayType::PHYSICAL),
+ Return(Error::NONE)));
+ // The attribute queries will get done twice. This is for defaults
+ EXPECT_CALL(*mMockComposer, getDisplayAttribute(PRIMARY_DISPLAY, 1, _, _))
+ .Times(2 * 3)
+ .WillRepeatedly(Invoke(mMockComposer, &MockComposerClient::getDisplayAttributeFake));
+ // ... and then special handling for dimensions. Specifying these
+ // rules later means that gmock will try them first, i.e.,
+ // ordering of width/height vs. the default implementation for
+ // other queries is significant.
+ EXPECT_CALL(*mMockComposer,
+ getDisplayAttribute(PRIMARY_DISPLAY, 1, IComposerClient::Attribute::WIDTH, _))
+ .Times(2)
+ .WillRepeatedly(DoAll(SetArgPointee<3>(400), Return(Error::NONE)));
+
+ EXPECT_CALL(*mMockComposer,
+ getDisplayAttribute(PRIMARY_DISPLAY, 1, IComposerClient::Attribute::HEIGHT, _))
+ .Times(2)
+ .WillRepeatedly(DoAll(SetArgPointee<3>(200), Return(Error::NONE)));
+
+ mMockComposer->hotplugDisplay(PRIMARY_DISPLAY, IComposerCallback::Connection::CONNECTED);
+
+ waitForDisplayTransaction();
+
+ EXPECT_TRUE(waitForHotplugEvent(ISurfaceComposer::eDisplayIdMain, true));
+
+ {
+ sp<android::IBinder> display(
+ SurfaceComposerClient::getBuiltInDisplay(ISurfaceComposer::eDisplayIdMain));
+ DisplayInfo info;
+ auto result = SurfaceComposerClient::getDisplayInfo(display, &info);
+ EXPECT_EQ(NO_ERROR, result);
+ ASSERT_EQ(400u, info.w);
+ ASSERT_EQ(200u, info.h);
+ }
+}
+
////////////////////////////////////////////////
class TransactionTest : public ::testing::Test {