Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit f2ddca6b authored by Dominik Laskowski's avatar Dominik Laskowski
Browse files

SF: Do not abort on duplicate hotplug disconnect 

processHotplug expects the display token to exist for disconnect events,
so skip events that disconnect an already disconnected display.

Bug: 241286153
Test: HotplugTest.ignoresDuplicateDisconnection
Change-Id: I2b711317acc87f453f73a1aafcc8dd63cc1a8d33
parent bb66e63d

services/surfaceflinger/DisplayHardware/HWComposer.cpp

+8 −15
Original line number Diff line number Diff line
@@ -252,11 +252,7 @@ std::shared_ptr<HWC2::Layer> HWComposer::createLayer(HalDisplayId displayId) { 
}



bool HWComposer::isConnected(PhysicalDisplayId displayId) const {

    if (mDisplayData.count(displayId)) {

        return mDisplayData.at(displayId).hwcDisplay->isConnected();

    }



    return false;

    return mDisplayData.count(displayId) && mDisplayData.at(displayId).hwcDisplay->isConnected();

}



std::vector<HWComposer::HWCDisplayMode> HWComposer::getModes(PhysicalDisplayId displayId) const {
@@ -946,18 +942,15 @@ std::optional<DisplayIdentificationInfo> HWComposer::onHotplugDisconnect( 
        return {};

    }



    // The display will later be destroyed by a call to

    // destroyDisplay(). For now we just mark it disconnected.

    if (isConnected(*displayId)) {

        mDisplayData[*displayId].hwcDisplay->setConnected(false);

    } else {

    if (!isConnected(*displayId)) {

        LOG_HWC_DISPLAY_ERROR(hwcDisplayId, "Already disconnected");

        return {};

    }

    // The cleanup of Disconnect is handled through HWComposer::disconnectDisplay

    // via SurfaceFlinger's onHotplugReceived callback handling

    return DisplayIdentificationInfo{.id = *displayId,

                                     .name = std::string(),

                                     .deviceProductInfo = std::nullopt};



    // The display will later be destroyed by a call to HWComposer::disconnectDisplay. For now, mark

    // it as disconnected.

    mDisplayData.at(*displayId).hwcDisplay->setConnected(false);

    return DisplayIdentificationInfo{.id = *displayId};

}



void HWComposer::loadCapabilities() {

services/surfaceflinger/tests/unittests/SurfaceFlinger_HotplugTest.cpp

+36 −0
Original line number Diff line number Diff line
@@ -41,6 +41,7 @@ TEST_F(HotplugTest, schedulesConfigureToProcessHotplugEvents) { 
}



TEST_F(HotplugTest, schedulesFrameToCommitDisplayTransaction) {

    EXPECT_CALL(*mFlinger.scheduler(), scheduleConfigure()).Times(1);

    EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1);



    constexpr HWDisplayId displayId1 = 456;
@@ -52,6 +53,39 @@ TEST_F(HotplugTest, schedulesFrameToCommitDisplayTransaction) { 
    EXPECT_TRUE(hasTransactionFlagSet(eDisplayTransactionNeeded));

}



TEST_F(HotplugTest, ignoresDuplicateDisconnection) {

    // Inject a primary display.

    PrimaryDisplayVariant::injectHwcDisplay(this);



    using ExternalDisplay = ExternalDisplayVariant;

    ExternalDisplay::setupHwcHotplugCallExpectations(this);

    ExternalDisplay::setupHwcGetActiveConfigCallExpectations(this);



    // TODO(b/241286146): Remove this unnecessary call.

    EXPECT_CALL(*mComposer,

                setVsyncEnabled(ExternalDisplay::HWC_DISPLAY_ID, IComposerClient::Vsync::DISABLE))

            .WillOnce(Return(Error::NONE));



    // A single commit should be scheduled for both configure calls.

    EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1);



    ExternalDisplay::injectPendingHotplugEvent(this, Connection::CONNECTED);

    mFlinger.configure();



    EXPECT_TRUE(hasPhysicalHwcDisplay(ExternalDisplay::HWC_DISPLAY_ID));



    // Disconnecting a display that was already disconnected should be a no-op.

    ExternalDisplay::injectPendingHotplugEvent(this, Connection::DISCONNECTED);

    ExternalDisplay::injectPendingHotplugEvent(this, Connection::DISCONNECTED);

    ExternalDisplay::injectPendingHotplugEvent(this, Connection::DISCONNECTED);

    mFlinger.configure();



    // The display should be scheduled for removal during the next commit. At this point, it should

    // still exist but be marked as disconnected.

    EXPECT_TRUE(hasPhysicalHwcDisplay(ExternalDisplay::HWC_DISPLAY_ID));

    EXPECT_FALSE(mFlinger.getHwComposer().isConnected(ExternalDisplay::DISPLAY_ID::get()));

}



TEST_F(HotplugTest, rejectsHotplugIfFailedToLoadDisplayModes) {

    // Inject a primary display.

    PrimaryDisplayVariant::injectHwcDisplay(this);
@@ -70,6 +104,8 @@ TEST_F(HotplugTest, rejectsHotplugIfFailedToLoadDisplayModes) { 
                setVsyncEnabled(ExternalDisplay::HWC_DISPLAY_ID, IComposerClient::Vsync::DISABLE))

            .WillOnce(Return(Error::NONE));



    EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1);



    ExternalDisplay::injectPendingHotplugEvent(this, Connection::CONNECTED);

    mFlinger.configure();