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

Commit f1c675b3 authored by Lloyd Pique's avatar Lloyd Pique
Browse files

SF: Cleanups to use std::atomic/std::mutex

A few places in the code used "volatile". This patch removes the
volatile.

In most of the cases, the volatile value was converted to a std::atomic
value. The exception was Barrier where it was not actually necessary,
but there I converted the code to use std::mutex.

Bug: None
Test: atest SurfaceFlinger_test CtsViewTestCases
Change-Id: I7b4f5a7398a241db8201dc7f60d7c9cd41e32e1b
parent ea0a5fb7
Loading
Loading
Loading
Loading
+12 −18
Original line number Diff line number Diff line
@@ -18,46 +18,40 @@
#define ANDROID_BARRIER_H

#include <stdint.h>
#include <sys/types.h>
#include <utils/threads.h>
#include <condition_variable>
#include <mutex>

namespace android {

class Barrier
{
public:
    inline Barrier() : state(CLOSED) { }
    inline ~Barrier() { }

    // Release any threads waiting at the Barrier.
    // Provides release semantics: preceding loads and stores will be visible
    // to other threads before they wake up.
    void open() {
        Mutex::Autolock _l(lock);
        state = OPENED;
        cv.broadcast();
        std::lock_guard<std::mutex> lock(mMutex);
        mIsOpen = true;
        mCondition.notify_all();
    }

    // Reset the Barrier, so wait() will block until open() has been called.
    void close() {
        Mutex::Autolock _l(lock);
        state = CLOSED;
        std::lock_guard<std::mutex> lock(mMutex);
        mIsOpen = false;
    }

    // Wait until the Barrier is OPEN.
    // Provides acquire semantics: no subsequent loads or stores will occur
    // until wait() returns.
    void wait() const {
        Mutex::Autolock _l(lock);
        while (state == CLOSED) {
            cv.wait(lock);
        }
        std::unique_lock<std::mutex> lock(mMutex);
        mCondition.wait(lock, [this]() NO_THREAD_SAFETY_ANALYSIS { return mIsOpen; });
    }
private:
    enum { OPENED, CLOSED };
    mutable     Mutex       lock;
    mutable     Condition   cv;
    volatile    int         state;
    mutable std::mutex mMutex;
    mutable std::condition_variable mCondition;
    int mIsOpen GUARDED_BY(mMutex){false};
};

}; // namespace android
+11 −9
Original line number Diff line number Diff line
@@ -207,8 +207,9 @@ bool BufferQueueLayer::getSidebandStreamChanged() const {
}

std::optional<Region> BufferQueueLayer::latchSidebandStream(bool& recomputeVisibleRegions) {
    if (android_atomic_acquire_cas(true, false, &mSidebandStreamChanged) == 0) {
        // mSidebandStreamChanged was true
    bool sidebandStreamChanged = true;
    if (mSidebandStreamChanged.compare_exchange_strong(sidebandStreamChanged, false)) {
        // mSidebandStreamChanged was changed to false
        // replicated in LayerBE until FE/BE is ready to be synchronized
        getBE().compositionInfo.hwc.sidebandStream = mConsumer->getSidebandStream();
        if (getBE().compositionInfo.hwc.sidebandStream != nullptr) {
@@ -259,7 +260,7 @@ status_t BufferQueueLayer::updateTexImage(bool& recomputeVisibleRegions, nsecs_t
            Mutex::Autolock lock(mQueueItemLock);
            mTimeStats.removeTimeRecord(getName().c_str(), mQueueItems[0].mFrameNumber);
            mQueueItems.removeAt(0);
            android_atomic_dec(&mQueuedFrames);
            mQueuedFrames--;
        }
        return BAD_VALUE;
    } else if (updateResult != NO_ERROR || mUpdateTexImageFailed) {
@@ -270,7 +271,7 @@ status_t BufferQueueLayer::updateTexImage(bool& recomputeVisibleRegions, nsecs_t
        if (queuedBuffer) {
            Mutex::Autolock lock(mQueueItemLock);
            mQueueItems.clear();
            android_atomic_and(0, &mQueuedFrames);
            mQueuedFrames = 0;
            mTimeStats.clearLayerRecord(getName().c_str());
        }

@@ -294,7 +295,7 @@ status_t BufferQueueLayer::updateTexImage(bool& recomputeVisibleRegions, nsecs_t
        while (mQueueItems[0].mFrameNumber != currentFrameNumber) {
            mTimeStats.removeTimeRecord(getName().c_str(), mQueueItems[0].mFrameNumber);
            mQueueItems.removeAt(0);
            android_atomic_dec(&mQueuedFrames);
            mQueuedFrames--;
        }

        const std::string layerName(getName().c_str());
@@ -306,7 +307,7 @@ status_t BufferQueueLayer::updateTexImage(bool& recomputeVisibleRegions, nsecs_t

    // Decrement the queued-frames count.  Signal another event if we
    // have more frames pending.
    if ((queuedBuffer && android_atomic_dec(&mQueuedFrames) > 1) || mAutoRefresh) {
    if ((queuedBuffer && mQueuedFrames.fetch_sub(1) > 1) || mAutoRefresh) {
        mFlinger->signalLayerUpdate();
    }

@@ -367,7 +368,7 @@ void BufferQueueLayer::onFrameAvailable(const BufferItem& item) {
        }

        mQueueItems.push_back(item);
        android_atomic_inc(&mQueuedFrames);
        mQueuedFrames++;

        // Wake up any pending callbacks
        mLastFrameNumberReceived = item.mFrameNumber;
@@ -404,8 +405,9 @@ void BufferQueueLayer::onFrameReplaced(const BufferItem& item) {
}

void BufferQueueLayer::onSidebandStreamChanged() {
    if (android_atomic_release_cas(false, true, &mSidebandStreamChanged) == 0) {
        // mSidebandStreamChanged was false
    bool sidebandStreamChanged = false;
    if (mSidebandStreamChanged.compare_exchange_strong(sidebandStreamChanged, true)) {
        // mSidebandStreamChanged was changed to true
        mFlinger->signalLayerUpdate();
    }
}
+2 −2
Original line number Diff line number Diff line
@@ -137,8 +137,8 @@ private:
    int mActiveBufferSlot;

    // thread-safe
    volatile int32_t mQueuedFrames;
    volatile int32_t mSidebandStreamChanged; // used like an atomic boolean
    std::atomic<int32_t> mQueuedFrames{0};
    std::atomic<bool> mSidebandStreamChanged{false};
};

} // namespace android
+3 −4
Original line number Diff line number Diff line
@@ -62,12 +62,11 @@

namespace android {

int32_t Layer::sSequence = 1;
std::atomic<int32_t> Layer::sSequence{1};

Layer::Layer(SurfaceFlinger* flinger, const sp<Client>& client, const String8& name, uint32_t w,
             uint32_t h, uint32_t flags)
      : contentDirty(false),
        sequence(uint32_t(android_atomic_inc(&sSequence))),
        mFlinger(flinger),
        mPremultipliedAlpha(true),
        mName(name),
@@ -1019,11 +1018,11 @@ void Layer::commitTransaction(const State& stateToCommit) {
}

uint32_t Layer::getTransactionFlags(uint32_t flags) {
    return android_atomic_and(~flags, &mTransactionFlags) & flags;
    return mTransactionFlags.fetch_and(~flags) & flags;
}

uint32_t Layer::setTransactionFlags(uint32_t flags) {
    return android_atomic_or(flags, &mTransactionFlags);
    return mTransactionFlags.fetch_or(flags);
}

bool Layer::setPosition(float x, float y, bool immediate) {
+4 −3
Original line number Diff line number Diff line
@@ -73,7 +73,7 @@ class SurfaceInterceptor;
// ---------------------------------------------------------------------------

class Layer : public virtual RefBase {
    static int32_t sSequence;
    static std::atomic<int32_t> sSequence;

public:
    friend class LayerBE;
@@ -89,7 +89,7 @@ public:
    // Layer serial number.  This gives layers an explicit ordering, so we
    // have a stable sort order when their layer stack and Z-order are
    // the same.
    int32_t sequence;
    int32_t sequence{sSequence++};

    enum { // flags for doTransaction()
        eDontUpdateGeometryState = 0x00000001,
@@ -271,6 +271,7 @@ public:
    virtual void useSurfaceDamage() {}
    virtual void useEmptyDamage() {}

    uint32_t getTransactionFlags() const { return mTransactionFlags; }
    uint32_t getTransactionFlags(uint32_t flags);
    uint32_t setTransactionFlags(uint32_t flags);

@@ -696,7 +697,7 @@ protected:
    // these are protected by an external lock
    State mCurrentState;
    State mDrawingState;
    volatile int32_t mTransactionFlags;
    std::atomic<uint32_t> mTransactionFlags{0};

    // Accessed from main thread and binder threads
    Mutex mPendingStateMutex;
Loading