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

Commit 6200eacd authored by Chia-I Wu's avatar Chia-I Wu
Browse files

surfaceflinger: make vsync injection more robust 

There are more issues than I expected :)

 - no lock to synchronize enable/disable and injection
 - Every time injection is diabled and enabled, a new EventThread is
   created
 - mCallback might be nullptr
 - ENABLE_VSYNC_INJECTIONS/INJECT_VSYNC should require special
   permission
 - MessageQueue::setEventThread must be called from the main thread
 - MessageQueue::setEventThread does not handle EventThread switch
   well

Bug: 65483324
Test: manual
Merged-In: I7d7b98d1f57afc64af0f2065a9bc7c8ad004ca9f
Change-Id: I7d7b98d1f57afc64af0f2065a9bc7c8ad004ca9f
parent 6afce442

services/surfaceflinger/MessageQueue.cpp

+8 −0
Original line number Diff line number Diff line
@@ -91,6 +91,14 @@ void MessageQueue::init(const sp<SurfaceFlinger>& flinger) 


void MessageQueue::setEventThread(const sp<EventThread>& eventThread)

{

    if (mEventThread == eventThread) {

        return;

    }



    if (mEventTube.getFd() >= 0) {

        mLooper->removeFd(mEventTube.getFd());

    }



    mEventThread = eventThread;

    mEvents = eventThread->createEventConnection();

    mEvents->stealReceiveChannel(&mEventTube);

services/surfaceflinger/SurfaceFlinger.cpp

+30 −7
Original line number Diff line number Diff line
@@ -515,8 +515,10 @@ public: 


    virtual void onInjectSyncEvent(nsecs_t when) {

        std::lock_guard<std::mutex> lock(mCallbackMutex);

        if (mCallback != nullptr) {

            mCallback->onVSyncEvent(when);

        }

    }



    virtual void setVSyncEnabled(bool) {}

    virtual void setPhaseOffset(nsecs_t) {}
@@ -988,13 +990,14 @@ status_t SurfaceFlinger::getHdrCapabilities(const sp<IBinder>& display, 
    return NO_ERROR;

}



status_t SurfaceFlinger::enableVSyncInjections(bool enable) {

    if (enable == mInjectVSyncs) {

        return NO_ERROR;

void SurfaceFlinger::enableVSyncInjectionsInternal(bool enable) {

    Mutex::Autolock _l(mStateLock);



    if (mInjectVSyncs == enable) {

        return;

    }



    if (enable) {

        mInjectVSyncs = enable;

        ALOGV("VSync Injections enabled");

        if (mVSyncInjector.get() == nullptr) {

            mVSyncInjector = new InjectVSyncSource();
@@ -1002,15 +1005,33 @@ status_t SurfaceFlinger::enableVSyncInjections(bool enable) { 
        }

        mEventQueue.setEventThread(mInjectorEventThread);

    } else {

        mInjectVSyncs = enable;

        ALOGV("VSync Injections disabled");

        mEventQueue.setEventThread(mSFEventThread);

        mVSyncInjector.clear();

    }



    mInjectVSyncs = enable;

}



status_t SurfaceFlinger::enableVSyncInjections(bool enable) {

    class MessageEnableVSyncInjections : public MessageBase {

        SurfaceFlinger* mFlinger;

        bool mEnable;

    public:

        MessageEnableVSyncInjections(SurfaceFlinger* flinger, bool enable)

            : mFlinger(flinger), mEnable(enable) { }

        virtual bool handler() {

            mFlinger->enableVSyncInjectionsInternal(mEnable);

            return true;

        }

    };

    sp<MessageBase> msg = new MessageEnableVSyncInjections(this, enable);

    postMessageSync(msg);

    return NO_ERROR;

}



status_t SurfaceFlinger::injectVSync(nsecs_t when) {

    Mutex::Autolock _l(mStateLock);



    if (!mInjectVSyncs) {

        ALOGE("VSync Injections not enabled");

        return BAD_VALUE;
@@ -3757,6 +3778,8 @@ status_t SurfaceFlinger::CheckTransactCodeCredentials(uint32_t code) { 
        case GET_ANIMATION_FRAME_STATS:

        case SET_POWER_MODE:

        case GET_HDR_CAPABILITIES:

        case ENABLE_VSYNC_INJECTIONS:

        case INJECT_VSYNC:

        {

            // codes that require permission check

            IPCThreadState* ipc = IPCThreadState::self();

services/surfaceflinger/SurfaceFlinger.h

+3 −0
Original line number Diff line number Diff line
@@ -338,6 +338,9 @@ private: 
    // Called on the main thread in response to setActiveColorMode()

    void setActiveColorModeInternal(const sp<DisplayDevice>& hw, android_color_mode_t colorMode);



    // Called on the main thread in response to enableVSyncInjections()

    void enableVSyncInjectionsInternal(bool enable);



    // Returns whether the transaction actually modified any state

    bool handleMessageTransaction();