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

Commit f6bbd44a authored by Mathias Agopian's avatar Mathias Agopian
Browse files

simplify further vsync handling 

- we now clean-up "dead" connection in the main loop,
this entirely avoid the problem with the side effects of
releasing strong references. We now only hold on to strong
reference for the connection we will signal.

- also simplify how we build the list of "ready" connections, by
only adding them to the list when we did receive a vsync event

Change-Id: I2a84da431320a2af8e8a93e07622a1d258236f43
parent bc46e0ad

services/surfaceflinger/EventThread.cpp

+75 −84
Original line number Original line Diff line number Diff line
@@ -60,14 +60,6 @@ status_t EventThread::registerDisplayEventConnection( 
    return NO_ERROR;

    return NO_ERROR;

}

}





status_t EventThread::unregisterDisplayEventConnection(

        const wp<EventThread::Connection>& connection) {

    Mutex::Autolock _l(mLock);

    mDisplayEventConnections.remove(connection);

    mCondition.broadcast();

    return NO_ERROR;

}



void EventThread::removeDisplayEventConnection(

void EventThread::removeDisplayEventConnection(

        const wp<EventThread::Connection>& connection) {

        const wp<EventThread::Connection>& connection) {

    Mutex::Autolock _l(mLock);

    Mutex::Autolock _l(mLock);
@@ -122,43 +114,63 @@ void EventThread::onVSyncReceived(int, nsecs_t timestamp) { 
}

}





bool EventThread::threadLoop() {

bool EventThread::threadLoop() {



    nsecs_t timestamp;

    size_t vsyncCount;

    DisplayEventReceiver::Event vsync;

    DisplayEventReceiver::Event vsync;

    Vector< sp<EventThread::Connection> > activeConnections;

    Vector< sp<EventThread::Connection> > signalConnections;

    Vector< sp<EventThread::Connection> > signalConnections;

    signalConnections = waitForEvent(&vsync);





    do {

    // dispatch vsync events to listeners...

        // release our references

    const size_t count = signalConnections.size();

        signalConnections.clear();

    for (size_t i=0 ; i<count ; i++) {

        activeConnections.clear();

        const sp<Connection>& conn(signalConnections[i]);

        // now see if we still need to report this VSYNC event

        status_t err = conn->postEvent(vsync);

        if (err == -EAGAIN || err == -EWOULDBLOCK) {

            // The destination doesn't accept events anymore, it's probably

            // full. For now, we just drop the events on the floor.

            // Note that some events cannot be dropped and would have to be

            // re-sent later. Right-now we don't have the ability to do

            // this, but it doesn't matter for VSYNC.

        } else if (err < 0) {

            // handle any other error on the pipe as fatal. the only

            // reasonable thing to do is to clean-up this connection.

            // The most common error we'll get here is -EPIPE.

            removeDisplayEventConnection(signalConnections[i]);

        }

    }

    return true;

}





Vector< sp<EventThread::Connection> > EventThread::waitForEvent(

        DisplayEventReceiver::Event* event)

{

    Mutex::Autolock _l(mLock);

    Mutex::Autolock _l(mLock);





    size_t vsyncCount;

    nsecs_t timestamp;

    Vector< sp<EventThread::Connection> > signalConnections;



    do {

        // latch VSYNC event if any

        // latch VSYNC event if any

        bool waitForVSync = false;

        bool waitForVSync = false;

        vsyncCount = mVSyncCount;

        vsyncCount = mVSyncCount;

        timestamp = mVSyncTimestamp;

        timestamp = mVSyncTimestamp;

        mVSyncTimestamp = 0;

        mVSyncTimestamp = 0;





        // find out connections waiting for VSYNC events

        // find out connections waiting for events

        size_t count = mDisplayEventConnections.size();

        size_t count = mDisplayEventConnections.size();

        for (size_t i=0 ; i<count ; i++) {

        for (size_t i=0 ; i<count ; i++) {

            sp<Connection> connection(mDisplayEventConnections[i].promote());

            sp<Connection> connection(mDisplayEventConnections[i].promote());

            if (connection != NULL) {

            if (connection != NULL) {

                activeConnections.add(connection);

                if (connection->count >= 0) {

                if (connection->count >= 0) {

                    // we need vsync events because at least

                    // we need vsync events because at least

                    // one connection is waiting for it

                    // one connection is waiting for it

                    waitForVSync = true;

                    waitForVSync = true;

                    if (timestamp) {

                        // we consume the event only if it's time

                        // (ie: we received a vsync event)

                        if (connection->count == 0) {

                        if (connection->count == 0) {

                            // fired this time around

                            // fired this time around

                        if (timestamp) {

                            // only "consume" this event if we're going to

                            // report it

                            connection->count = -1;

                            connection->count = -1;

                        }

                            signalConnections.add(connection);

                            signalConnections.add(connection);

                        } else if (connection->count == 1 ||

                        } else if (connection->count == 1 ||

                                (vsyncCount % connection->count) == 0) {

                                (vsyncCount % connection->count) == 0) {
@@ -167,29 +179,25 @@ bool EventThread::threadLoop() { 
                        }

                        }

                    }

                    }

                }

                }

            } else {

                // we couldn't promote this reference, the connection has

                // died, so clean-up!

                mDisplayEventConnections.removeAt(i);

                --i; --count;

            }

        }

        }





        if (timestamp) {

        // Here we figure out if we need to enable or disable vsyncs

            // we have a vsync event we can dispatch

        if (timestamp && !waitForVSync) {

            if (!waitForVSync) {

            // we received a VSYNC but we have no clients

            // we received a VSYNC but we have no clients

            // don't report it, and disable VSYNC events

            // don't report it, and disable VSYNC events

            disableVSyncLocked();

            disableVSyncLocked();

            } else {

        } else if (!timestamp && waitForVSync) {

                // report VSYNC event

                break;

            }

        } else {

            // never disable VSYNC events immediately, instead

            // we'll wait to receive the event and we'll

            // reevaluate whether we need to dispatch it and/or

            // disable VSYNC events then.

            if (waitForVSync) {

                // enable

            enableVSyncLocked();

            enableVSyncLocked();

        }

        }

        }





        // note: !timestamp implies signalConnections.isEmpty()

        if (!timestamp) {

            // wait for something to happen

            // wait for something to happen

            if (CC_UNLIKELY(mUseSoftwareVSync && waitForVSync)) {

            if (CC_UNLIKELY(mUseSoftwareVSync && waitForVSync)) {

                // h/w vsync cannot be used (screen is off), so we use

                // h/w vsync cannot be used (screen is off), so we use
@@ -200,39 +208,21 @@ bool EventThread::threadLoop() { 
                    mVSyncCount++;

                    mVSyncCount++;

                }

                }

            } else {

            } else {

            if (!timestamp || signalConnections.isEmpty()) {

                // This is where we spend most of our time, waiting

                // This is where we spend most of our time, waiting

                // for a vsync events and registered clients

                // for a vsync events and registered clients

                mCondition.wait(mLock);

                mCondition.wait(mLock);

            }

            }

        }

        }

    } while (!timestamp || signalConnections.isEmpty());

    } while (signalConnections.isEmpty());





    // dispatch vsync events to listeners...

    // here we're guaranteed to have a timestamp and some connections to signal

    vsync.header.type = DisplayEventReceiver::DISPLAY_EVENT_VSYNC;

    vsync.header.timestamp = timestamp;

    vsync.vsync.count = vsyncCount;





    const size_t count = signalConnections.size();

    // dispatch vsync events to listeners...

    for (size_t i=0 ; i<count ; i++) {

    event->header.type = DisplayEventReceiver::DISPLAY_EVENT_VSYNC;

        const sp<Connection>& conn(signalConnections[i]);

    event->header.timestamp = timestamp;

        // now see if we still need to report this VSYNC event

    event->vsync.count = vsyncCount;

        status_t err = conn->postEvent(vsync);

        if (err == -EAGAIN || err == -EWOULDBLOCK) {

            // The destination doesn't accept events anymore, it's probably

            // full. For now, we just drop the events on the floor.

            // Note that some events cannot be dropped and would have to be

            // re-sent later. Right-now we don't have the ability to do

            // this, but it doesn't matter for VSYNC.

        } else if (err < 0) {

            // handle any other error on the pipe as fatal. the only

            // reasonable thing to do is to clean-up this connection.

            // The most common error we'll get here is -EPIPE.

            removeDisplayEventConnection(signalConnections[i]);

        }

    }





    return true;

    return signalConnections;

}

}





void EventThread::enableVSyncLocked() {

void EventThread::enableVSyncLocked() {
@@ -280,7 +270,8 @@ EventThread::Connection::Connection( 
}

}





EventThread::Connection::~Connection() {

EventThread::Connection::~Connection() {

    mEventThread->unregisterDisplayEventConnection(this);

    // do nothing here -- clean-up will happen automatically

    // when the main thread wakes up

}

}





void EventThread::Connection::onFirstRef() {

void EventThread::Connection::onFirstRef() {

services/surfaceflinger/EventThread.h

+3 −1
Original line number Original line Diff line number Diff line
@@ -65,7 +65,6 @@ public: 




    sp<Connection> createEventConnection() const;

    sp<Connection> createEventConnection() const;

    status_t registerDisplayEventConnection(const sp<Connection>& connection);

    status_t registerDisplayEventConnection(const sp<Connection>& connection);

    status_t unregisterDisplayEventConnection(const wp<Connection>& connection);





    void setVsyncRate(uint32_t count, const sp<Connection>& connection);

    void setVsyncRate(uint32_t count, const sp<Connection>& connection);

    void requestNextVsync(const sp<Connection>& connection);

    void requestNextVsync(const sp<Connection>& connection);
@@ -79,6 +78,9 @@ public: 
    // called when receiving a vsync event

    // called when receiving a vsync event

    void onVSyncReceived(int display, nsecs_t timestamp);

    void onVSyncReceived(int display, nsecs_t timestamp);





    Vector< sp<EventThread::Connection> > waitForEvent(

            DisplayEventReceiver::Event* event);



    void dump(String8& result, char* buffer, size_t SIZE) const;

    void dump(String8& result, char* buffer, size_t SIZE) const;





private:

private: