Merge "BLASTBufferQueue/SF: apply transactions with one-way binder" into tm-dev
diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp
index 8e23eb87..ec4c7c1 100644
--- a/libs/gui/BLASTBufferQueue.cpp
+++ b/libs/gui/BLASTBufferQueue.cpp
@@ -182,7 +182,8 @@
static_cast<uint32_t>(mPendingTransactions.size()));
SurfaceComposerClient::Transaction t;
mergePendingTransactions(&t, std::numeric_limits<uint64_t>::max() /* frameNumber */);
- t.setApplyToken(mApplyToken).apply();
+ // All transactions on our apply token are one-way. See comment on mAppliedLastTransaction
+ t.setApplyToken(mApplyToken).apply(false, true);
}
void BLASTBufferQueue::update(const sp<SurfaceControl>& surface, uint32_t width, uint32_t height,
@@ -230,7 +231,8 @@
}
}
if (applyTransaction) {
- t.setApplyToken(mApplyToken).apply();
+ // All transactions on our apply token are one-way. See comment on mAppliedLastTransaction
+ t.setApplyToken(mApplyToken).apply(false, true);
}
}
@@ -551,7 +553,13 @@
mergePendingTransactions(t, bufferItem.mFrameNumber);
if (applyTransaction) {
- t->setApplyToken(mApplyToken).apply();
+ // All transactions on our apply token are one-way. See comment on mAppliedLastTransaction
+ t->setApplyToken(mApplyToken).apply(false, true);
+ mAppliedLastTransaction = true;
+ mLastAppliedFrameNumber = bufferItem.mFrameNumber;
+ } else {
+ t->setBufferHasBarrier(mSurfaceControl, mLastAppliedFrameNumber);
+ mAppliedLastTransaction = false;
}
BQA_LOGV("acquireNextBufferLocked size=%dx%d mFrameNumber=%" PRIu64
@@ -857,7 +865,8 @@
SurfaceComposerClient::Transaction t;
mergePendingTransactions(&t, frameNumber);
- t.setApplyToken(mApplyToken).apply();
+ // All transactions on our apply token are one-way. See comment on mAppliedLastTransaction
+ t.setApplyToken(mApplyToken).apply(false, true);
}
void BLASTBufferQueue::mergePendingTransactions(SurfaceComposerClient::Transaction* t,
@@ -1050,7 +1059,8 @@
static_cast<uint32_t>(mPendingTransactions.size()));
SurfaceComposerClient::Transaction t;
mergePendingTransactions(&t, std::numeric_limits<uint64_t>::max() /* frameNumber */);
- t.setApplyToken(mApplyToken).apply();
+ // All transactions on our apply token are one-way. See comment on mAppliedLastTransaction
+ t.setApplyToken(mApplyToken).apply(false, true);
}
// Clear sync states
diff --git a/libs/gui/ISurfaceComposer.cpp b/libs/gui/ISurfaceComposer.cpp
index 5ab0abc..5532c6e 100644
--- a/libs/gui/ISurfaceComposer.cpp
+++ b/libs/gui/ISurfaceComposer.cpp
@@ -111,7 +111,13 @@
SAFE_PARCEL(data.writeUint64, transactionId);
- return remote()->transact(BnSurfaceComposer::SET_TRANSACTION_STATE, data, &reply);
+ if (flags & ISurfaceComposer::eOneWay) {
+ return remote()->transact(BnSurfaceComposer::SET_TRANSACTION_STATE,
+ data, &reply, IBinder::FLAG_ONEWAY);
+ } else {
+ return remote()->transact(BnSurfaceComposer::SET_TRANSACTION_STATE,
+ data, &reply);
+ }
}
void bootFinished() override {
diff --git a/libs/gui/LayerState.cpp b/libs/gui/LayerState.cpp
index 6944d38..338ff11 100644
--- a/libs/gui/LayerState.cpp
+++ b/libs/gui/LayerState.cpp
@@ -796,6 +796,8 @@
SAFE_PARCEL(output->writeStrongBinder, cachedBuffer.token.promote());
SAFE_PARCEL(output->writeUint64, cachedBuffer.id);
+ SAFE_PARCEL(output->writeBool, hasBarrier);
+ SAFE_PARCEL(output->writeUint64, barrierFrameNumber);
return NO_ERROR;
}
@@ -832,6 +834,9 @@
cachedBuffer.token = tmpBinder;
SAFE_PARCEL(input->readUint64, &cachedBuffer.id);
+ SAFE_PARCEL(input->readBool, &hasBarrier);
+ SAFE_PARCEL(input->readUint64, &barrierFrameNumber);
+
return NO_ERROR;
}
diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index 26ccda5..27856ce 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -930,7 +930,7 @@
}
}
-status_t SurfaceComposerClient::Transaction::apply(bool synchronous) {
+status_t SurfaceComposerClient::Transaction::apply(bool synchronous, bool oneWay) {
if (mStatus != NO_ERROR) {
return mStatus;
}
@@ -984,6 +984,14 @@
if (mAnimation) {
flags |= ISurfaceComposer::eAnimation;
}
+ if (oneWay) {
+ if (mForceSynchronous) {
+ ALOGE("Transaction attempted to set synchronous and one way at the same time"
+ " this is an invalid request. Synchronous will win for safety");
+ } else {
+ flags |= ISurfaceComposer::eOneWay;
+ }
+ }
// If both mEarlyWakeupStart and mEarlyWakeupEnd are set
// it is equivalent for none
@@ -1399,6 +1407,18 @@
return bufferData;
}
+SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setBufferHasBarrier(
+ const sp<SurfaceControl>& sc, uint64_t barrierFrameNumber) {
+ layer_state_t* s = getLayerState(sc);
+ if (!s) {
+ mStatus = BAD_INDEX;
+ return *this;
+ }
+ s->bufferData->hasBarrier = true;
+ s->bufferData->barrierFrameNumber = barrierFrameNumber;
+ return *this;
+}
+
SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setBuffer(
const sp<SurfaceControl>& sc, const sp<GraphicBuffer>& buffer,
const std::optional<sp<Fence>>& fence, const std::optional<uint64_t>& frameNumber,
diff --git a/libs/gui/include/gui/BLASTBufferQueue.h b/libs/gui/include/gui/BLASTBufferQueue.h
index 265ae24..65fc04d 100644
--- a/libs/gui/include/gui/BLASTBufferQueue.h
+++ b/libs/gui/include/gui/BLASTBufferQueue.h
@@ -254,6 +254,21 @@
// surfacecontol. This is useful if the caller wants to synchronize the buffer scale with
// additional scales in the hierarchy.
bool mUpdateDestinationFrame GUARDED_BY(mMutex) = true;
+
+ // We send all transactions on our apply token over one-way binder calls to avoid blocking
+ // client threads. All of our transactions remain in order, since they are one-way binder calls
+ // from a single process, to a single interface. However once we give up a Transaction for sync
+ // we can start to have ordering issues. When we return from sync to normal frame production,
+ // we wait on the commit callback of sync frames ensuring ordering, however we don't want to
+ // wait on the commit callback for every normal frame (since even emitting them has a
+ // performance cost) this means we need a method to ensure frames are in order when switching
+ // from one-way application on our apply token, to application on some other apply token. We
+ // make use of setBufferHasBarrier to declare this ordering. This boolean simply tracks when we
+ // need to set this flag, notably only in the case where we are transitioning from a previous
+ // transaction applied by us (one way, may not yet have reached server) and an upcoming
+ // transaction that will be applied by some sync consumer.
+ bool mAppliedLastTransaction = false;
+ uint64_t mLastAppliedFrameNumber = 0;
};
} // namespace android
diff --git a/libs/gui/include/gui/ISurfaceComposer.h b/libs/gui/include/gui/ISurfaceComposer.h
index 4dfc383..0a3cc19 100644
--- a/libs/gui/include/gui/ISurfaceComposer.h
+++ b/libs/gui/include/gui/ISurfaceComposer.h
@@ -113,6 +113,7 @@
// android.permission.ACCESS_SURFACE_FLINGER
eEarlyWakeupStart = 0x08,
eEarlyWakeupEnd = 0x10,
+ eOneWay = 0x20
};
enum VsyncSource {
diff --git a/libs/gui/include/gui/LayerState.h b/libs/gui/include/gui/LayerState.h
index 885b4ae..0f37dab 100644
--- a/libs/gui/include/gui/LayerState.h
+++ b/libs/gui/include/gui/LayerState.h
@@ -89,6 +89,8 @@
// Used by BlastBufferQueue to forward the framenumber generated by the
// graphics producer.
uint64_t frameNumber = 0;
+ bool hasBarrier = false;
+ uint64_t barrierFrameNumber = 0;
// Listens to when the buffer is safe to be released. This is used for blast
// layers only. The callback includes a release fence as well as the graphic
diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h
index 6c79b5b..c8ac166 100644
--- a/libs/gui/include/gui/SurfaceComposerClient.h
+++ b/libs/gui/include/gui/SurfaceComposerClient.h
@@ -461,7 +461,7 @@
// Clears the contents of the transaction without applying it.
void clear();
- status_t apply(bool synchronous = false);
+ status_t apply(bool synchronous = false, bool oneWay = false);
// Merge another transaction in to this one, clearing other
// as if it had been applied.
Transaction& merge(Transaction&& other);
@@ -521,6 +521,27 @@
const std::optional<uint64_t>& frameNumber = std::nullopt,
ReleaseBufferCallback callback = nullptr);
std::shared_ptr<BufferData> getAndClearBuffer(const sp<SurfaceControl>& sc);
+
+ /**
+ * If this transaction, has a a buffer set for the given SurfaceControl
+ * mark that buffer as ordered after a given barrierFrameNumber.
+ *
+ * SurfaceFlinger will refuse to apply this transaction until after
+ * the frame in barrierFrameNumber has been applied. This transaction may
+ * be applied in the same frame as the barrier buffer or after.
+ *
+ * This is only designed to be used to handle switches between multiple
+ * apply tokens, as explained in the comment for BLASTBufferQueue::mAppliedLastTransaction.
+ *
+ * Has to be called after setBuffer.
+ *
+ * WARNING:
+ * This API is very dangerous to the caller, as if you invoke it without
+ * a frameNumber you have not yet submitted, you can dead-lock your
+ * SurfaceControl's transaction queue.
+ */
+ Transaction& setBufferHasBarrier(const sp<SurfaceControl>& sc,
+ uint64_t barrierFrameNumber);
Transaction& setDataspace(const sp<SurfaceControl>& sc, ui::Dataspace dataspace);
Transaction& setHdrMetadata(const sp<SurfaceControl>& sc, const HdrMetadata& hdrMetadata);
Transaction& setSurfaceDamageRegion(const sp<SurfaceControl>& sc,
diff --git a/services/surfaceflinger/BufferStateLayer.h b/services/surfaceflinger/BufferStateLayer.h
index 669eaad..8a696f1 100644
--- a/services/surfaceflinger/BufferStateLayer.h
+++ b/services/surfaceflinger/BufferStateLayer.h
@@ -141,6 +141,8 @@
ReleaseCallbackId mPreviousReleaseCallbackId = ReleaseCallbackId::INVALID_ID;
uint64_t mPreviousReleasedFrameNumber = 0;
+ uint64_t mPreviousBarrierFrameNumber = 0;
+
bool mReleasePreviousBuffer = false;
// Stores the last set acquire fence signal time used to populate the callback handle's acquire
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 350d709..d4979e8 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -3674,11 +3674,12 @@
return false;
}
-void SurfaceFlinger::flushPendingTransactionQueues(
+int SurfaceFlinger::flushPendingTransactionQueues(
std::vector<TransactionState>& transactions,
- std::unordered_set<sp<IBinder>, SpHash<IBinder>>& bufferLayersReadyToPresent,
+ std::unordered_map<sp<IBinder>, uint64_t, SpHash<IBinder>>& bufferLayersReadyToPresent,
std::unordered_set<sp<IBinder>, SpHash<IBinder>>& applyTokensWithUnsignaledTransactions,
bool tryApplyUnsignaled) {
+ int transactionsPendingBarrier = 0;
auto it = mPendingTransactionQueues.begin();
while (it != mPendingTransactionQueues.end()) {
auto& [applyToken, transactionQueue] = *it;
@@ -3701,8 +3702,21 @@
setTransactionFlags(eTransactionFlushNeeded);
break;
}
+ if (ready == TransactionReadiness::NotReadyBarrier) {
+ transactionsPendingBarrier++;
+ setTransactionFlags(eTransactionFlushNeeded);
+ break;
+ }
transaction.traverseStatesWithBuffers([&](const layer_state_t& state) {
- bufferLayersReadyToPresent.insert(state.surface);
+ const bool frameNumberChanged =
+ state.bufferData->flags.test(BufferData::BufferDataChange::frameNumberChanged);
+ if (frameNumberChanged) {
+ bufferLayersReadyToPresent[state.surface] = state.bufferData->frameNumber;
+ } else {
+ // Barrier function only used for BBQ which always includes a frame number
+ bufferLayersReadyToPresent[state.surface] =
+ std::numeric_limits<uint64_t>::max();
+ }
});
const bool appliedUnsignaled = (ready == TransactionReadiness::ReadyUnsignaled);
if (appliedUnsignaled) {
@@ -3720,6 +3734,7 @@
it = std::next(it, 1);
}
}
+ return transactionsPendingBarrier;
}
bool SurfaceFlinger::flushTransactionQueues(int64_t vsyncId) {
@@ -3728,19 +3743,21 @@
// states) around outside the scope of the lock
std::vector<TransactionState> transactions;
// Layer handles that have transactions with buffers that are ready to be applied.
- std::unordered_set<sp<IBinder>, SpHash<IBinder>> bufferLayersReadyToPresent;
+ std::unordered_map<sp<IBinder>, uint64_t, SpHash<IBinder>> bufferLayersReadyToPresent;
std::unordered_set<sp<IBinder>, SpHash<IBinder>> applyTokensWithUnsignaledTransactions;
{
Mutex::Autolock _l(mStateLock);
{
Mutex::Autolock _l(mQueueLock);
+ int lastTransactionsPendingBarrier = 0;
+ int transactionsPendingBarrier = 0;
// First collect transactions from the pending transaction queues.
// We are not allowing unsignaled buffers here as we want to
// collect all the transactions from applyTokens that are ready first.
- flushPendingTransactionQueues(transactions, bufferLayersReadyToPresent,
- applyTokensWithUnsignaledTransactions,
- /*tryApplyUnsignaled*/ false);
+ transactionsPendingBarrier =
+ flushPendingTransactionQueues(transactions, bufferLayersReadyToPresent,
+ applyTokensWithUnsignaledTransactions, /*tryApplyUnsignaled*/ false);
// Second, collect transactions from the transaction queue.
// Here as well we are not allowing unsignaled buffers for the same
@@ -3765,18 +3782,48 @@
/*tryApplyUnsignaled*/ false);
}();
ATRACE_INT("TransactionReadiness", static_cast<int>(ready));
- if (ready == TransactionReadiness::NotReady) {
+ if (ready != TransactionReadiness::Ready) {
+ if (ready == TransactionReadiness::NotReadyBarrier) {
+ transactionsPendingBarrier++;
+ }
mPendingTransactionQueues[transaction.applyToken].push(std::move(transaction));
} else {
transaction.traverseStatesWithBuffers([&](const layer_state_t& state) {
- bufferLayersReadyToPresent.insert(state.surface);
- });
+ const bool frameNumberChanged =
+ state.bufferData->flags.test(BufferData::BufferDataChange::frameNumberChanged);
+ if (frameNumberChanged) {
+ bufferLayersReadyToPresent[state.surface] = state.bufferData->frameNumber;
+ } else {
+ // Barrier function only used for BBQ which always includes a frame number.
+ // This value only used for barrier logic.
+ bufferLayersReadyToPresent[state.surface] =
+ std::numeric_limits<uint64_t>::max();
+ }
+ });
transactions.emplace_back(std::move(transaction));
}
mTransactionQueue.pop_front();
ATRACE_INT("TransactionQueue", mTransactionQueue.size());
}
+ // Transactions with a buffer pending on a barrier may be on a different applyToken
+ // than the transaction which satisfies our barrier. In fact this is the exact use case
+ // that the primitive is designed for. This means we may first process
+ // the barrier dependent transaction, determine it ineligible to complete
+ // and then satisfy in a later inner iteration of flushPendingTransactionQueues.
+ // The barrier dependent transaction was eligible to be presented in this frame
+ // but we would have prevented it without case. To fix this we continually
+ // loop through flushPendingTransactionQueues until we perform an iteration
+ // where the number of transactionsPendingBarrier doesn't change. This way
+ // we can continue to resolve dependency chains of barriers as far as possible.
+ while (lastTransactionsPendingBarrier != transactionsPendingBarrier) {
+ lastTransactionsPendingBarrier = transactionsPendingBarrier;
+ transactionsPendingBarrier =
+ flushPendingTransactionQueues(transactions, bufferLayersReadyToPresent,
+ applyTokensWithUnsignaledTransactions,
+ /*tryApplyUnsignaled*/ false);
+ }
+
// We collected all transactions that could apply without latching unsignaled buffers.
// If we are allowing latch unsignaled of some form, now it's the time to go over the
// transactions that were not applied and try to apply them unsignaled.
@@ -3892,7 +3939,8 @@
auto SurfaceFlinger::transactionIsReadyToBeApplied(
const FrameTimelineInfo& info, bool isAutoTimestamp, int64_t desiredPresentTime,
uid_t originUid, const Vector<ComposerState>& states,
- const std::unordered_set<sp<IBinder>, SpHash<IBinder>>& bufferLayersReadyToPresent,
+ const std::unordered_map<
+ sp<IBinder>, uint64_t, SpHash<IBinder>>& bufferLayersReadyToPresent,
size_t totalTXapplied, bool tryApplyUnsignaled) const -> TransactionReadiness {
ATRACE_FORMAT("transactionIsReadyToBeApplied vsyncId: %" PRId64, info.vsyncId);
const nsecs_t expectedPresentTime = mExpectedPresentTime.load();
@@ -3930,6 +3978,17 @@
continue;
}
+ if (s.hasBufferChanges() && s.bufferData->hasBarrier &&
+ ((layer->getDrawingState().frameNumber) < s.bufferData->barrierFrameNumber)) {
+ const bool willApplyBarrierFrame =
+ (bufferLayersReadyToPresent.find(s.surface) != bufferLayersReadyToPresent.end()) &&
+ (bufferLayersReadyToPresent.at(s.surface) >= s.bufferData->barrierFrameNumber);
+ if (!willApplyBarrierFrame) {
+ ATRACE_NAME("NotReadyBarrier");
+ return TransactionReadiness::NotReadyBarrier;
+ }
+ }
+
const bool allowLatchUnsignaled = tryApplyUnsignaled &&
shouldLatchUnsignaled(layer, s, states.size(), totalTXapplied);
ATRACE_FORMAT("%s allowLatchUnsignaled=%s", layer->getName().c_str(),
@@ -3950,8 +4009,8 @@
if (s.hasBufferChanges()) {
// If backpressure is enabled and we already have a buffer to commit, keep the
// transaction in the queue.
- const bool hasPendingBuffer =
- bufferLayersReadyToPresent.find(s.surface) != bufferLayersReadyToPresent.end();
+ const bool hasPendingBuffer = bufferLayersReadyToPresent.find(s.surface) !=
+ bufferLayersReadyToPresent.end();
if (layer->backpressureEnabled() && hasPendingBuffer && isAutoTimestamp) {
ATRACE_NAME("hasPendingBuffer");
return TransactionReadiness::NotReady;
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 5829838..0365b37 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -760,9 +760,9 @@
// Returns true if there is at least one transaction that needs to be flushed
bool transactionFlushNeeded();
- void flushPendingTransactionQueues(
+ int flushPendingTransactionQueues(
std::vector<TransactionState>& transactions,
- std::unordered_set<sp<IBinder>, SpHash<IBinder>>& bufferLayersReadyToPresent,
+ std::unordered_map<sp<IBinder>, uint64_t, SpHash<IBinder>>& bufferLayersReadyToPresent,
std::unordered_set<sp<IBinder>, SpHash<IBinder>>& applyTokensWithUnsignaledTransactions,
bool tryApplyUnsignaled) REQUIRES(mStateLock, mQueueLock);
@@ -789,13 +789,15 @@
void commitOffscreenLayers();
enum class TransactionReadiness {
NotReady,
+ NotReadyBarrier,
Ready,
ReadyUnsignaled,
};
TransactionReadiness transactionIsReadyToBeApplied(
const FrameTimelineInfo& info, bool isAutoTimestamp, int64_t desiredPresentTime,
uid_t originUid, const Vector<ComposerState>& states,
- const std::unordered_set<sp<IBinder>, SpHash<IBinder>>& bufferLayersReadyToPresent,
+ const std::unordered_map<
+ sp<IBinder>, uint64_t, SpHash<IBinder>>& bufferLayersReadyToPresent,
size_t totalTXapplied, bool tryApplyUnsignaled) const REQUIRES(mStateLock);
static LatchUnsignaledConfig getLatchUnsignaledConfig();
bool shouldLatchUnsignaled(const sp<Layer>& layer, const layer_state_t&, size_t numStates,