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

Commit 4a42d43b authored by Marin Shalamanov's avatar Marin Shalamanov
Browse files

SF: Update display properties on hotplug connect 

When a hotplug connect event for already connected display
is received, destroy and recreate the display device in SF,
then send a "hotplug connect" event to Display manager. This
way display properties will be updated.

Bug: 143451809
Test: atest libsurfaceflinger_unittest
Test: atest SurfaceFligner_test
Test: atest CompositionTest
Test: Manually on device:
      2. adb shell dumpsys display
      3. unplug primary display
      4. plug another display
      5. adb shell dumpsys display
Test: Manually on device:
      1. disconnect and reconnect display
      2. power off and on the screen
      3. make sure the device didn't crash

Merged-In: I89996d9340f6570fa5ae0cc0977eaba7a2d3693c
Change-Id: I89996d9340f6570fa5ae0cc0977eaba7a2d3693c
(cherry picked from commit 700e639f)
parent bdd5915f

services/surfaceflinger/DisplayDevice.h

+2 −1
Original line number Diff line number Diff line
@@ -183,9 +183,10 @@ struct DisplayDeviceState { 
    struct Physical {

        DisplayId id;

        DisplayConnectionType type;

        hwc2_display_t hwcDisplayId;



        bool operator==(const Physical& other) const {

            return id == other.id && type == other.type;

            return id == other.id && type == other.type && hwcDisplayId == other.hwcDisplayId;

        }

    };

services/surfaceflinger/SurfaceFlinger.cpp

+26 −16
Original line number Diff line number Diff line
@@ -2385,7 +2385,8 @@ void SurfaceFlinger::processDisplayHotplugEventsLocked() { 
                }



                DisplayDeviceState state;

                state.physical = {displayId, getHwComposer().getDisplayConnectionType(displayId)};

                state.physical = {displayId, getHwComposer().getDisplayConnectionType(displayId),

                                  event.hwcDisplayId};

                state.isSecure = true; // All physical displays are currently considered secure.

                state.displayName = info->name;
@@ -2394,6 +2395,12 @@ void SurfaceFlinger::processDisplayHotplugEventsLocked() { 
                mPhysicalDisplayTokens.emplace(displayId, std::move(token));



                mInterceptor->saveDisplayCreation(state);

            } else {

                ALOGV("Recreating display %s", to_string(displayId).c_str());



                const auto token = it->second;

                auto& state = mCurrentState.displays.editValueFor(token);

                state.sequenceId = DisplayDeviceState{}.sequenceId;

            }

        } else {

            ALOGV("Removing display %s", to_string(displayId).c_str());
@@ -2571,20 +2578,17 @@ void SurfaceFlinger::processDisplayAdded(const wp<IBinder>& displayToken, 
        producer = bqProducer;

    }



    if (displaySurface != nullptr) {

        mDisplays.emplace(displayToken,

                          setupNewDisplayDeviceInternal(displayToken, compositionDisplay, state,

                                                        displaySurface, producer));

    LOG_FATAL_IF(!displaySurface);

    const auto display = setupNewDisplayDeviceInternal(displayToken, compositionDisplay, state,

                                                       displaySurface, producer);

    mDisplays.emplace(displayToken, display);

    if (!state.isVirtual()) {

            LOG_ALWAYS_FATAL_IF(!displayId);

        LOG_FATAL_IF(!displayId);

        dispatchDisplayHotplugEvent(displayId->value, true);

    }



        const auto displayDevice = mDisplays[displayToken];

        if (displayDevice->isPrimary()) {

            mScheduler->onPrimaryDisplayAreaChanged(displayDevice->getWidth() *

                                                    displayDevice->getHeight());

        }

    if (display->isPrimary()) {

        mScheduler->onPrimaryDisplayAreaChanged(display->getWidth() * display->getHeight());

    }

}
@@ -2595,7 +2599,7 @@ void SurfaceFlinger::processDisplayRemoved(const wp<IBinder>& displayToken) { 
        display->disconnect();



        if (!display->isVirtual()) {

            LOG_ALWAYS_FATAL_IF(!displayId);

            LOG_FATAL_IF(!displayId);

            dispatchDisplayHotplugEvent(displayId->value, false);

        }

    }
@@ -2608,13 +2612,19 @@ void SurfaceFlinger::processDisplayChanged(const wp<IBinder>& displayToken, 
                                           const DisplayDeviceState& drawingState) {

    const sp<IBinder> currentBinder = IInterface::asBinder(currentState.surface);

    const sp<IBinder> drawingBinder = IInterface::asBinder(drawingState.surface);

    if (currentBinder != drawingBinder) {

    if (currentBinder != drawingBinder || currentState.sequenceId != drawingState.sequenceId) {

        // changing the surface is like destroying and recreating the DisplayDevice

        if (const auto display = getDisplayDeviceLocked(displayToken)) {

            display->disconnect();

        }

        mDisplays.erase(displayToken);

        if (const auto& physical = currentState.physical) {

            getHwComposer().allocatePhysicalDisplay(physical->hwcDisplayId, physical->id);

        }

        processDisplayAdded(displayToken, currentState);

        if (currentState.physical) {

            initializeDisplays();

        }

        return;

    }

services/surfaceflinger/tests/unittests/CompositionTest.cpp

+8 −8
Original line number Diff line number Diff line
@@ -305,9 +305,9 @@ struct BaseDisplayVariant { 
                compositionengine::impl::createDisplay(test->mFlinger.getCompositionEngine(),

                                                       ceDisplayArgs);



        test->mDisplay =

                FakeDisplayDeviceInjector(test->mFlinger, compositionDisplay,

                                          DisplayConnectionType::Internal, true /* isPrimary */)

        test->mDisplay = FakeDisplayDeviceInjector(test->mFlinger, compositionDisplay,

                                                   DisplayConnectionType::Internal, HWC_DISPLAY,

                                                   true /* isPrimary */)

                                 .setDisplaySurface(test->mDisplaySurface)

                                 .setNativeWindow(test->mNativeWindow)

                                 .setSecure(Derived::IS_SECURE)

services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp

+40 −16
Original line number Diff line number Diff line
@@ -246,6 +246,7 @@ sp<DisplayDevice> DisplayTransactionTest::injectDefaultInternalDisplay( 
    constexpr DisplayId DEFAULT_DISPLAY_ID = DisplayId{777};

    constexpr int DEFAULT_DISPLAY_WIDTH = 1080;

    constexpr int DEFAULT_DISPLAY_HEIGHT = 1920;

    constexpr hwc2_display_t DEFAULT_DISPLAY_HWC_DISPLAY_ID = 0;



    // The DisplayDevice is required to have a framebuffer (behind the

    // ANativeWindow interface) which uses the actual hardware display
@@ -270,7 +271,7 @@ sp<DisplayDevice> DisplayTransactionTest::injectDefaultInternalDisplay( 


    auto injector =

            FakeDisplayDeviceInjector(mFlinger, compositionDisplay, DisplayConnectionType::Internal,

                                      true /* isPrimary */);

                                      DEFAULT_DISPLAY_HWC_DISPLAY_ID, true /* isPrimary */);



    injector.setNativeWindow(mNativeWindow);

    if (injectExtra) {
@@ -373,6 +374,23 @@ struct DisplayConnectionTypeGetter<PhysicalDisplayId<PhysicalDisplay>> { 
    static constexpr std::optional<DisplayConnectionType> value = PhysicalDisplay::CONNECTION_TYPE;

};



template <typename>

struct HwcDisplayIdGetter {

    static constexpr std::optional<hwc2_display_t> value;

};



constexpr hwc2_display_t HWC_VIRTUAL_DISPLAY_HWC_DISPLAY_ID = 1010;



template <DisplayId::Type displayId>

struct HwcDisplayIdGetter<VirtualDisplayId<displayId>> {

    static constexpr std::optional<hwc2_display_t> value = HWC_VIRTUAL_DISPLAY_HWC_DISPLAY_ID;

};



template <typename PhysicalDisplay>

struct HwcDisplayIdGetter<PhysicalDisplayId<PhysicalDisplay>> {

    static constexpr std::optional<hwc2_display_t> value = PhysicalDisplay::HWC_DISPLAY_ID;

};



// DisplayIdType can be:

//     1) PhysicalDisplayId<...> for generated ID of physical display backed by HWC.

//     2) VirtualDisplayId<...> for hard-coded ID of virtual display backed by HWC.
@@ -382,6 +400,7 @@ template <typename DisplayIdType, int width, int height, Critical critical, Asyn 
struct DisplayVariant {

    using DISPLAY_ID = DisplayIdGetter<DisplayIdType>;

    using CONNECTION_TYPE = DisplayConnectionTypeGetter<DisplayIdType>;

    using HWC_DISPLAY_ID_OPT = HwcDisplayIdGetter<DisplayIdType>;



    // The display width and height

    static constexpr int WIDTH = width;
@@ -418,9 +437,9 @@ struct DisplayVariant { 
                compositionengine::impl::createDisplay(test->mFlinger.getCompositionEngine(),

                                                       ceDisplayArgs.build());



        auto injector =

                FakeDisplayDeviceInjector(test->mFlinger, compositionDisplay,

                                          CONNECTION_TYPE::value, static_cast<bool>(PRIMARY));

        auto injector = FakeDisplayDeviceInjector(test->mFlinger, compositionDisplay,

                                                  CONNECTION_TYPE::value, HWC_DISPLAY_ID_OPT::value,

                                                  static_cast<bool>(PRIMARY));



        injector.setSecure(static_cast<bool>(SECURE));

        injector.setNativeWindow(test->mNativeWindow);
@@ -603,12 +622,11 @@ struct HwcDisplayVariant { 
constexpr uint32_t GRALLOC_USAGE_PHYSICAL_DISPLAY =

        GRALLOC_USAGE_HW_RENDER | GRALLOC_USAGE_HW_COMPOSER | GRALLOC_USAGE_HW_FB;



template <hwc2_display_t hwcDisplayId, typename PhysicalDisplay, int width, int height,

          Critical critical>

template <typename PhysicalDisplay, int width, int height, Critical critical>

struct PhysicalDisplayVariant

      : DisplayVariant<PhysicalDisplayId<PhysicalDisplay>, width, height, critical, Async::FALSE,

                       Secure::TRUE, PhysicalDisplay::PRIMARY, GRALLOC_USAGE_PHYSICAL_DISPLAY>,

        HwcDisplayVariant<hwcDisplayId, HWC2::DisplayType::Physical,

        HwcDisplayVariant<PhysicalDisplay::HWC_DISPLAY_ID, HWC2::DisplayType::Physical,

                          DisplayVariant<PhysicalDisplayId<PhysicalDisplay>, width, height,

                                         critical, Async::FALSE, Secure::TRUE,

                                         PhysicalDisplay::PRIMARY, GRALLOC_USAGE_PHYSICAL_DISPLAY>,
@@ -619,6 +637,7 @@ struct PrimaryDisplay { 
    static constexpr auto CONNECTION_TYPE = DisplayConnectionType::Internal;

    static constexpr Primary PRIMARY = Primary::TRUE;

    static constexpr uint8_t PORT = 255;

    static constexpr hwc2_display_t HWC_DISPLAY_ID = 1001;

    static constexpr bool HAS_IDENTIFICATION_DATA = hasIdentificationData;

    static constexpr auto GET_IDENTIFICATION_DATA = getInternalEdid;

};
@@ -628,6 +647,7 @@ struct ExternalDisplay { 
    static constexpr auto CONNECTION_TYPE = DisplayConnectionType::External;

    static constexpr Primary PRIMARY = Primary::FALSE;

    static constexpr uint8_t PORT = 254;

    static constexpr hwc2_display_t HWC_DISPLAY_ID = 1002;

    static constexpr bool HAS_IDENTIFICATION_DATA = hasIdentificationData;

    static constexpr auto GET_IDENTIFICATION_DATA = getExternalEdid;

};
@@ -635,19 +655,19 @@ struct ExternalDisplay { 
struct TertiaryDisplay {

    static constexpr Primary PRIMARY = Primary::FALSE;

    static constexpr uint8_t PORT = 253;

    static constexpr hwc2_display_t HWC_DISPLAY_ID = 1003;

    static constexpr auto GET_IDENTIFICATION_DATA = getExternalEdid;

};



// A primary display is a physical display that is critical

using PrimaryDisplayVariant =

        PhysicalDisplayVariant<1001, PrimaryDisplay<false>, 3840, 2160, Critical::TRUE>;

        PhysicalDisplayVariant<PrimaryDisplay<false>, 3840, 2160, Critical::TRUE>;



// An external display is physical display that is not critical.

using ExternalDisplayVariant =

        PhysicalDisplayVariant<1002, ExternalDisplay<false>, 1920, 1280, Critical::FALSE>;

        PhysicalDisplayVariant<ExternalDisplay<false>, 1920, 1280, Critical::FALSE>;



using TertiaryDisplayVariant =

        PhysicalDisplayVariant<1003, TertiaryDisplay, 1600, 1200, Critical::FALSE>;

using TertiaryDisplayVariant = PhysicalDisplayVariant<TertiaryDisplay, 1600, 1200, Critical::FALSE>;



// A virtual display not supported by the HWC.

constexpr uint32_t GRALLOC_USAGE_NONHWC_VIRTUAL_DISPLAY = 0;
@@ -696,7 +716,7 @@ struct HwcVirtualDisplayVariant 
      : DisplayVariant<VirtualDisplayId<42>, width, height, Critical::FALSE, Async::TRUE, secure,

                       Primary::FALSE, GRALLOC_USAGE_HWC_VIRTUAL_DISPLAY>,

        HwcDisplayVariant<

                1010, HWC2::DisplayType::Virtual,

                HWC_VIRTUAL_DISPLAY_HWC_DISPLAY_ID, HWC2::DisplayType::Virtual,

                DisplayVariant<VirtualDisplayId<42>, width, height, Critical::FALSE, Async::TRUE,

                               secure, Primary::FALSE, GRALLOC_USAGE_HWC_VIRTUAL_DISPLAY>> {

    using Base = DisplayVariant<VirtualDisplayId<42>, width, height, Critical::FALSE, Async::TRUE,
@@ -1767,7 +1787,9 @@ void SetupNewDisplayDeviceInternalTest::setupNewDisplayDeviceInternalTest() { 
    if (const auto connectionType = Case::Display::CONNECTION_TYPE::value) {

        const auto displayId = Case::Display::DISPLAY_ID::get();

        ASSERT_TRUE(displayId);

        state.physical = {*displayId, *connectionType};

        const auto hwcDisplayId = Case::Display::HWC_DISPLAY_ID_OPT::value;

        ASSERT_TRUE(hwcDisplayId);

        state.physical = {*displayId, *connectionType, *hwcDisplayId};

    }



    state.isSecure = static_cast<bool>(Case::Display::SECURE);
@@ -1941,7 +1963,9 @@ void HandleTransactionLockedTest::verifyDisplayIsConnected(const sp<IBinder>& di 
    if (const auto connectionType = Case::Display::CONNECTION_TYPE::value) {

        const auto displayId = Case::Display::DISPLAY_ID::get();

        ASSERT_TRUE(displayId);

        expectedPhysical = {*displayId, *connectionType};

        const auto hwcDisplayId = Case::Display::HWC_DISPLAY_ID_OPT::value;

        ASSERT_TRUE(hwcDisplayId);

        expectedPhysical = {*displayId, *connectionType, *hwcDisplayId};

    }



    // The display should have been set up in the current display state
@@ -2124,11 +2148,11 @@ TEST_F(HandleTransactionLockedTest, ignoresHotplugConnectIfExternalForVrComposer 
    ignoresHotplugConnectCommon<SimpleExternalDisplayCase>();

}



TEST_F(HandleTransactionLockedTest, processHotplugDisconnectPrimaryDisplay) {

TEST_F(HandleTransactionLockedTest, processesHotplugDisconnectPrimaryDisplay) {

    processesHotplugDisconnectCommon<SimplePrimaryDisplayCase>();

}



TEST_F(HandleTransactionLockedTest, processHotplugDisconnectExternalDisplay) {

TEST_F(HandleTransactionLockedTest, processesHotplugDisconnectExternalDisplay) {

    processesHotplugDisconnectCommon<SimpleExternalDisplayCase>();

}

services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h

+6 −3
Original line number Diff line number Diff line
@@ -549,9 +549,10 @@ public: 
        FakeDisplayDeviceInjector(TestableSurfaceFlinger& flinger,

                                  std::shared_ptr<compositionengine::Display> compositionDisplay,

                                  std::optional<DisplayConnectionType> connectionType,

                                  bool isPrimary)

                                  std::optional<hwc2_display_t> hwcDisplayId, bool isPrimary)

              : mFlinger(flinger),

                mCreationArgs(flinger.mFlinger.get(), mDisplayToken, compositionDisplay) {

                mCreationArgs(flinger.mFlinger.get(), mDisplayToken, compositionDisplay),

                mHwcDisplayId(hwcDisplayId) {

            mCreationArgs.connectionType = connectionType;

            mCreationArgs.isPrimary = isPrimary;

        }
@@ -619,7 +620,8 @@ public: 
            DisplayDeviceState state;

            if (const auto type = mCreationArgs.connectionType) {

                LOG_ALWAYS_FATAL_IF(!displayId);

                state.physical = {*displayId, *type};

                LOG_ALWAYS_FATAL_IF(!mHwcDisplayId);

                state.physical = {*displayId, *type, *mHwcDisplayId};

            }



            state.isSecure = mCreationArgs.isSecure;
@@ -640,6 +642,7 @@ public: 
        TestableSurfaceFlinger& mFlinger;

        sp<BBinder> mDisplayToken = new BBinder();

        DisplayDeviceCreationArgs mCreationArgs;

        const std::optional<hwc2_display_t> mHwcDisplayId;

    };



    surfaceflinger::test::Factory mFactory;