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

Commit 8735eacc authored by Ady Abraham's avatar Ady Abraham
Browse files

SurfaceFlinger: VSyncReactor should not know VSyncDispatch 

VSyncReactor doesn't use VSyncDispatch anymore with the exception
of logging the predicted vsync. Decouple those two for better logic
separation.

Test: SF unit tests
Test: examine systraces with debug.sf.show_predicted_vsync=1
Bug: 162888874

Change-Id: I368926f55a21663cdea478219bf467fc4772aadb
parent 9c53ee71

services/surfaceflinger/Scheduler/Scheduler.cpp

+37 −4
Original line number Diff line number Diff line
@@ -95,6 +95,26 @@ const char* toContentDetectionString(bool useContentDetection, bool useContentDe 


} // namespace



class PredictedVsyncTracer {

public:

    PredictedVsyncTracer(scheduler::VSyncDispatch& dispatch)

          : mRegistration(dispatch, std::bind(&PredictedVsyncTracer::callback, this),

                          "PredictedVsyncTracer") {

        scheduleRegistration();

    }



private:

    TracedOrdinal<bool> mParity = {"VSYNC-predicted", 0};

    scheduler::VSyncCallbackRegistration mRegistration;



    void scheduleRegistration() { mRegistration.schedule({0, 0, 0}); }



    void callback() {

        mParity = !mParity;

        scheduleRegistration();

    }

};



Scheduler::Scheduler(const scheduler::RefreshRateConfigs& configs, ISchedulerCallback& callback)

      : Scheduler(configs, callback,

                  {.supportKernelTimer = sysprop::support_kernel_idle_timer(false),
@@ -145,7 +165,11 @@ Scheduler::Scheduler(VsyncSchedule schedule, const scheduler::RefreshRateConfigs 
        mVsyncSchedule(std::move(schedule)),

        mLayerHistory(std::move(layerHistory)),

        mSchedulerCallback(schedulerCallback),

        mRefreshRateConfigs(configs) {

        mRefreshRateConfigs(configs),

        mPredictedVsyncTracer(

                base::GetBoolProperty("debug.sf.show_predicted_vsync", false)

                        ? std::make_unique<PredictedVsyncTracer>(*mVsyncSchedule.dispatch)

                        : nullptr) {

    mSchedulerCallback.setVsyncEnabled(false);

}
@@ -163,8 +187,8 @@ Scheduler::VsyncSchedule Scheduler::createVsyncSchedule(Options options) { 


    // TODO(b/144707443): Tune constants.

    constexpr size_t pendingFenceLimit = 20;

    auto sync = std::make_unique<scheduler::VSyncReactor>(std::move(clock), *dispatch, *tracker,

                                                          pendingFenceLimit,

    auto sync =

            std::make_unique<scheduler::VSyncReactor>(std::move(clock), *tracker, pendingFenceLimit,

                                                      options.supportKernelTimer);

    return {std::move(sync), std::move(tracker), std::move(dispatch)};

}
@@ -600,6 +624,15 @@ void Scheduler::dump(std::string& result) const { 
                  mLayerHistory ? mLayerHistory->dump().c_str() : "(no layer history)");

}



void Scheduler::dumpVSync(std::string& s) const {

    using base::StringAppendF;



    StringAppendF(&s, "VSyncReactor:\n");

    mVsyncSchedule.sync->dump(s);

    StringAppendF(&s, "VSyncDispatch:\n");

    mVsyncSchedule.dispatch->dump(s);

}



template <class T>

bool Scheduler::handleTimerStateChanged(T* currentState, T newState) {

    HwcConfigIndexType newConfigId;

services/surfaceflinger/Scheduler/Scheduler.h

+4 −0
Original line number Diff line number Diff line
@@ -43,6 +43,7 @@ using scheduler::LayerHistory; 
class DispSync;

class FenceTime;

class InjectVSyncSource;

class PredictedVsyncTracer;



namespace scheduler {

class VSyncDispatch;
@@ -137,6 +138,7 @@ public: 


    void dump(std::string&) const;

    void dump(ConnectionHandle, std::string&) const;

    void dumpVSync(std::string&) const;



    // Get the appropriate refresh for current conditions.

    std::optional<HwcConfigIndexType> getPreferredConfigId();
@@ -284,6 +286,8 @@ private: 
    std::optional<hal::VsyncPeriodChangeTimeline> mLastVsyncPeriodChangeTimeline

            GUARDED_BY(mVsyncTimelineLock);

    static constexpr std::chrono::nanoseconds MAX_VSYNC_APPLIED_TIME = 200ms;



    const std::unique_ptr<PredictedVsyncTracer> mPredictedVsyncTracer;

};



} // namespace android

services/surfaceflinger/Scheduler/VSyncReactor.cpp

+2 −29
Original line number Diff line number Diff line
@@ -35,36 +35,11 @@ nsecs_t SystemClock::now() const { 
    return systemTime(SYSTEM_TIME_MONOTONIC);

}



class PredictedVsyncTracer {

public:

    PredictedVsyncTracer(VSyncDispatch& dispatch)

          : mRegistration(dispatch, std::bind(&PredictedVsyncTracer::callback, this),

                          "PredictedVsyncTracer") {

        scheduleRegistration();

    }



private:

    TracedOrdinal<bool> mParity = {"VSYNC-predicted", 0};

    VSyncCallbackRegistration mRegistration;



    void scheduleRegistration() { mRegistration.schedule({0, 0, 0}); }



    void callback() {

        mParity = !mParity;

        scheduleRegistration();

    }

};



VSyncReactor::VSyncReactor(std::unique_ptr<Clock> clock, VSyncDispatch& dispatch,

                           VSyncTracker& tracker, size_t pendingFenceLimit,

                           bool supportKernelIdleTimer)

VSyncReactor::VSyncReactor(std::unique_ptr<Clock> clock, VSyncTracker& tracker,

                           size_t pendingFenceLimit, bool supportKernelIdleTimer)

      : mClock(std::move(clock)),

        mTracker(tracker),

        mDispatch(dispatch),

        mPendingLimit(pendingFenceLimit),

        mPredictedVsyncTracer(property_get_bool("debug.sf.show_predicted_vsync", false)

                                      ? std::make_unique<PredictedVsyncTracer>(mDispatch)

                                      : nullptr),

        mSupportKernelIdleTimer(supportKernelIdleTimer) {}



VSyncReactor::~VSyncReactor() = default;
@@ -267,8 +242,6 @@ void VSyncReactor::dump(std::string& result) const { 


    StringAppendF(&result, "VSyncTracker:\n");

    mTracker.dump(result);

    StringAppendF(&result, "VSyncDispatch:\n");

    mDispatch.dump(result);

}



} // namespace android::scheduler

services/surfaceflinger/Scheduler/VSyncReactor.h

+2 −5
Original line number Diff line number Diff line
@@ -29,13 +29,12 @@ namespace android::scheduler { 
class Clock;

class VSyncDispatch;

class VSyncTracker;

class PredictedVsyncTracer;



// TODO (b/145217110): consider renaming.

class VSyncReactor : public android::DispSync {

public:

    VSyncReactor(std::unique_ptr<Clock> clock, VSyncDispatch& dispatch, VSyncTracker& tracker,

                 size_t pendingFenceLimit, bool supportKernelIdleTimer);

    VSyncReactor(std::unique_ptr<Clock> clock, VSyncTracker& tracker, size_t pendingFenceLimit,

                 bool supportKernelIdleTimer);

    ~VSyncReactor();



    bool addPresentFence(const std::shared_ptr<FenceTime>& fence) final;
@@ -64,7 +63,6 @@ private: 


    std::unique_ptr<Clock> const mClock;

    VSyncTracker& mTracker;

    VSyncDispatch& mDispatch;

    size_t const mPendingLimit;



    mutable std::mutex mMutex;
@@ -77,7 +75,6 @@ private: 
    std::optional<nsecs_t> mPeriodTransitioningTo GUARDED_BY(mMutex);

    std::optional<nsecs_t> mLastHwVsync GUARDED_BY(mMutex);



    const std::unique_ptr<PredictedVsyncTracer> mPredictedVsyncTracer;

    const bool mSupportKernelIdleTimer = false;

};

services/surfaceflinger/SurfaceFlinger.cpp

+1 −2
Original line number Diff line number Diff line
@@ -4352,8 +4352,7 @@ status_t SurfaceFlinger::doDump(int fd, const DumpArgs& args, bool asProto) { 
    } else {

        static const std::unordered_map<std::string, Dumper> dumpers = {

                {"--display-id"s, dumper(&SurfaceFlinger::dumpDisplayIdentificationData)},

                {"--dispsync"s,

                 dumper([this](std::string& s) { mScheduler->getPrimaryDispSync().dump(s); })},

                {"--dispsync"s, dumper([this](std::string& s) { mScheduler->dumpVSync(s); })},

                {"--edid"s, argsDumper(&SurfaceFlinger::dumpRawDisplayIdentificationData)},

                {"--frame-events"s, dumper(&SurfaceFlinger::dumpFrameEventsLocked)},

                {"--latency"s, argsDumper(&SurfaceFlinger::dumpStatsLocked)},