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

Commit 50e2e4d5 authored by Dominik Laskowski's avatar Dominik Laskowski
Browse files

SF: Fix freezing after follower display mode set 

When a mode set is initiated for any display, SF skips committing until
HWC confirms the mode set by signaling the corresponding present fence.

However, if a concurrent follower display misses a frame (i.e. does not
signal the fence) as its mode is set, SF::commit skips endlessly, since
the Scheduler only checked for missed frames on followers after commit.

Fix this by having all displays FrameTargeter::beginFrame before commit.

Fixes: 301082260
Test: Mode set on external display does not sporadically freeze SF.
Test: SchedulerTest.onFrameSignalMultipleDisplays
Change-Id: Ic97f33975f6d30279cc65cefe5d7ccc2561c45aa
parent cc89e85f

services/surfaceflinger/Scheduler/Scheduler.cpp

+2 −3
Original line number Diff line number Diff line
@@ -192,7 +192,8 @@ void Scheduler::onFrameSignal(ICompositor& compositor, VsyncId vsyncId, 
    for (const auto& [id, display] : mDisplays) {

        if (id == pacesetterId) continue;



        const FrameTargeter& targeter = *display.targeterPtr;

        FrameTargeter& targeter = *display.targeterPtr;

        targeter.beginFrame(beginFrameArgs, *display.schedulePtr);

        targets.try_emplace(id, &targeter.target());

    }
@@ -206,8 +207,6 @@ void Scheduler::onFrameSignal(ICompositor& compositor, VsyncId vsyncId, 
        if (id == pacesetterId) continue;



        FrameTargeter& targeter = *display.targeterPtr;

        targeter.beginFrame(beginFrameArgs, *display.schedulePtr);



        targeters.try_emplace(id, &targeter);

    }

services/surfaceflinger/tests/unittests/SchedulerTest.cpp

+57 −0
Original line number Diff line number Diff line
@@ -444,6 +444,63 @@ TEST_F(SchedulerTest, chooseDisplayModesMultipleDisplays) { 
    }

}



TEST_F(SchedulerTest, onFrameSignalMultipleDisplays) {

    mScheduler->registerDisplay(kDisplayId1,

                                std::make_shared<RefreshRateSelector>(kDisplay1Modes,

                                                                      kDisplay1Mode60->getId()));

    mScheduler->registerDisplay(kDisplayId2,

                                std::make_shared<RefreshRateSelector>(kDisplay2Modes,

                                                                      kDisplay2Mode60->getId()));



    using VsyncIds = std::vector<std::pair<PhysicalDisplayId, VsyncId>>;



    struct Compositor final : ICompositor {

        VsyncIds vsyncIds;

        bool committed = true;



        void configure() override {}



        bool commit(PhysicalDisplayId, const scheduler::FrameTargets& targets) override {

            vsyncIds.clear();



            for (const auto& [id, target] : targets) {

                vsyncIds.emplace_back(id, target->vsyncId());

            }



            return committed;

        }



        CompositeResultsPerDisplay composite(PhysicalDisplayId,

                                             const scheduler::FrameTargeters&) override {

            CompositeResultsPerDisplay results;



            for (const auto& [id, _] : vsyncIds) {

                results.try_emplace(id,

                                    CompositeResult{.compositionCoverage =

                                                            CompositionCoverage::Hwc});

            }



            return results;

        }



        void sample() override {}

    } compositor;



    mScheduler->doFrameSignal(compositor, VsyncId(42));



    const auto makeVsyncIds = [](VsyncId vsyncId) -> VsyncIds {

        return {{kDisplayId1, vsyncId}, {kDisplayId2, vsyncId}};

    };



    EXPECT_EQ(makeVsyncIds(VsyncId(42)), compositor.vsyncIds);



    compositor.committed = false;

    mScheduler->doFrameSignal(compositor, VsyncId(43));



    // FrameTargets should be updated despite the skipped commit.

    EXPECT_EQ(makeVsyncIds(VsyncId(43)), compositor.vsyncIds);

}



class AttachedChoreographerTest : public SchedulerTest {

protected:

    void frameRateTestScenario(Fps layerFps, int8_t frameRateCompatibility, Fps displayFps,

services/surfaceflinger/tests/unittests/TestableScheduler.h

+6 −0
Original line number Diff line number Diff line
@@ -59,6 +59,12 @@ public: 
    MOCK_METHOD(void, scheduleFrame, (), (override));

    MOCK_METHOD(void, postMessage, (sp<MessageHandler>&&), (override));



    void doFrameSignal(ICompositor& compositor, VsyncId vsyncId) {

        ftl::FakeGuard guard1(kMainThreadContext);

        ftl::FakeGuard guard2(mDisplayLock);

        Scheduler::onFrameSignal(compositor, vsyncId, TimePoint());

    }



    // Used to inject mock event thread.

    ConnectionHandle createConnection(std::unique_ptr<EventThread> eventThread) {

        return Scheduler::createConnection(std::move(eventThread));