Merge "[sf] Fix latch unsignaled issues" into udc-dev am: 2ddc65e023 (27dc4cce) · Commits · e / os / android_frameworks_native

services/surfaceflinger/FrontEnd/TransactionHandler.cpp

+58 −43

Original line number	Original line	Diff line number	Diff line
	@@ -64,11 +64,61 @@ std::vector<TransactionState> TransactionHandler::flushTransactions() {
	transactionsPendingBarrier = flushPendingTransactionQueues(transactions, flushState);		transactionsPendingBarrier = flushPendingTransactionQueues(transactions, flushState);
	} while (lastTransactionsPendingBarrier != transactionsPendingBarrier);		} while (lastTransactionsPendingBarrier != transactionsPendingBarrier);

			applyUnsignaledBufferTransaction(transactions, flushState);

	mPendingTransactionCount.fetch_sub(transactions.size());		mPendingTransactionCount.fetch_sub(transactions.size());
	ATRACE_INT("TransactionQueue", static_cast<int>(mPendingTransactionCount.load()));		ATRACE_INT("TransactionQueue", static_cast<int>(mPendingTransactionCount.load()));
	return transactions;		return transactions;
	}		}

			void TransactionHandler::applyUnsignaledBufferTransaction(
			std::vector<TransactionState>& transactions, TransactionFlushState& flushState) {
			// only apply an unsignaled buffer transaction if it's the first one
			if (!transactions.empty()) {
			return;
			}

			if (!flushState.queueWithUnsignaledBuffer) {
			return;
			}

			auto it = mPendingTransactionQueues.find(flushState.queueWithUnsignaledBuffer);
			LOG_ALWAYS_FATAL_IF(it == mPendingTransactionQueues.end(),
			"Could not find queue with unsignaled buffer!");

			auto& queue = it->second;
			popTransactionFromPending(transactions, flushState, queue);
			if (queue.empty()) {
			it = mPendingTransactionQueues.erase(it);
			}
			}

			void TransactionHandler::popTransactionFromPending(std::vector<TransactionState>& transactions,
			TransactionFlushState& flushState,
			std::queue<TransactionState>& queue) {
			auto& transaction = queue.front();
			// Transaction is ready move it from the pending queue.
			flushState.firstTransaction = false;
			removeFromStalledTransactions(transaction.id);
			transactions.emplace_back(std::move(transaction));
			queue.pop();

			auto& readyToApplyTransaction = transactions.back();
			readyToApplyTransaction.traverseStatesWithBuffers([&](const layer_state_t& state) {
			const bool frameNumberChanged =
			state.bufferData->flags.test(BufferData::BufferDataChange::frameNumberChanged);
			if (frameNumberChanged) {
			flushState.bufferLayersReadyToPresent.emplace_or_replace(state.surface.get(),
			state.bufferData->frameNumber);
			} else {
			// Barrier function only used for BBQ which always includes a frame number.
			// This value only used for barrier logic.
			flushState.bufferLayersReadyToPresent
			.emplace_or_replace(state.surface.get(), std::numeric_limits<uint64_t>::max());
			}
			});
			}

	TransactionHandler::TransactionReadiness TransactionHandler::applyFilters(		TransactionHandler::TransactionReadiness TransactionHandler::applyFilters(
	TransactionFlushState& flushState) {		TransactionFlushState& flushState) {
	auto ready = TransactionReadiness::Ready;		auto ready = TransactionReadiness::Ready;
	@@ -79,8 +129,7 @@ TransactionHandler::TransactionReadiness TransactionHandler::applyFilters(
	case TransactionReadiness::NotReadyBarrier:		case TransactionReadiness::NotReadyBarrier:
	return perFilterReady;		return perFilterReady;

	case TransactionReadiness::ReadyUnsignaled:		case TransactionReadiness::NotReadyUnsignaled:
	case TransactionReadiness::ReadyUnsignaledSingle:
	// If one of the filters allows latching an unsignaled buffer, latch this ready		// If one of the filters allows latching an unsignaled buffer, latch this ready
	// state.		// state.
	ready = perFilterReady;		ready = perFilterReady;
	@@ -97,17 +146,7 @@ int TransactionHandler::flushPendingTransactionQueues(std::vector<TransactionSta
	int transactionsPendingBarrier = 0;		int transactionsPendingBarrier = 0;
	auto it = mPendingTransactionQueues.begin();		auto it = mPendingTransactionQueues.begin();
	while (it != mPendingTransactionQueues.end()) {		while (it != mPendingTransactionQueues.end()) {
	auto& queue = it->second;		auto& [applyToken, queue] = *it;
	IBinder* queueToken = it->first.get();

	// if we have already flushed a transaction with an unsignaled buffer then stop queue
	// processing
	if (std::find(flushState.queuesWithUnsignaledBuffers.begin(),
	flushState.queuesWithUnsignaledBuffers.end(),
	queueToken) != flushState.queuesWithUnsignaledBuffers.end()) {
	continue;
	}

	while (!queue.empty()) {		while (!queue.empty()) {
	auto& transaction = queue.front();		auto& transaction = queue.front();
	flushState.transaction = &transaction;		flushState.transaction = &transaction;
	@@ -117,38 +156,14 @@ int TransactionHandler::flushPendingTransactionQueues(std::vector<TransactionSta
	break;		break;
	} else if (ready == TransactionReadiness::NotReady) {		} else if (ready == TransactionReadiness::NotReady) {
	break;		break;
	}		} else if (ready == TransactionReadiness::NotReadyUnsignaled) {
			// We maybe able to latch this transaction if it's the only transaction
	// Transaction is ready move it from the pending queue.		// ready to be applied.
	flushState.firstTransaction = false;		flushState.queueWithUnsignaledBuffer = applyToken;
	removeFromStalledTransactions(transaction.id);
	transactions.emplace_back(std::move(transaction));
	queue.pop();

	// If the buffer is unsignaled, then we don't want to signal other transactions using
	// the buffer as a barrier.
	auto& readyToApplyTransaction = transactions.back();
	if (ready == TransactionReadiness::Ready) {
	readyToApplyTransaction.traverseStatesWithBuffers([&](const layer_state_t& state) {
	const bool frameNumberChanged = state.bufferData->flags.test(
	BufferData::BufferDataChange::frameNumberChanged);
	if (frameNumberChanged) {
	flushState.bufferLayersReadyToPresent
	.emplace_or_replace(state.surface.get(),
	state.bufferData->frameNumber);
	} else {
	// Barrier function only used for BBQ which always includes a frame number.
	// This value only used for barrier logic.
	flushState.bufferLayersReadyToPresent
	.emplace_or_replace(state.surface.get(),
	std::numeric_limits<uint64_t>::max());
	}
	});
	} else if (ready == TransactionReadiness::ReadyUnsignaledSingle) {
	// Track queues with a flushed unsingaled buffer.
	flushState.queuesWithUnsignaledBuffers.emplace_back(queueToken);
	break;		break;
	}		}
			// ready == TransactionReadiness::Ready
			popTransactionFromPending(transactions, flushState, queue);
	}		}

	if (queue.empty()) {		if (queue.empty()) {

services/surfaceflinger/FrontEnd/TransactionHandler.h

+13 −4

Original line number	Original line	Diff line number	Diff line
	@@ -40,14 +40,20 @@ public:
	// Layer handles that have transactions with buffers that are ready to be applied.		// Layer handles that have transactions with buffers that are ready to be applied.
	ftl::SmallMap<IBinder* /* binder address /, uint64_t / framenumber */, 15>		ftl::SmallMap<IBinder* /* binder address /, uint64_t / framenumber */, 15>
	bufferLayersReadyToPresent = {};		bufferLayersReadyToPresent = {};
	ftl::SmallVector<IBinder* /* queueToken */, 15> queuesWithUnsignaledBuffers;		// Tracks the queue with an unsignaled buffer. This is used to handle
			// LatchUnsignaledConfig::AutoSingleLayer to ensure we only apply an unsignaled buffer
			// if it's the only transaction that is ready to be applied.
			sp<IBinder> queueWithUnsignaledBuffer = nullptr;
	};		};
	enum class TransactionReadiness {		enum class TransactionReadiness {
			// Transaction is ready to be applied
			Ready,
			// Transaction has unmet conditions (fence, present time, etc) and cannot be applied.
	NotReady,		NotReady,
			// Transaction is waiting on a barrier (another buffer to be latched first)
	NotReadyBarrier,		NotReadyBarrier,
	Ready,		// Transaction has an unsignaled fence but can be applied if it's the only transaction
	ReadyUnsignaled,		NotReadyUnsignaled,
	ReadyUnsignaledSingle,
	};		};
	using TransactionFilter = std::function<TransactionReadiness(const TransactionFlushState&)>;		using TransactionFilter = std::function<TransactionReadiness(const TransactionFlushState&)>;

	@@ -64,6 +70,9 @@ private:
	friend class ::android::TestableSurfaceFlinger;		friend class ::android::TestableSurfaceFlinger;

	int flushPendingTransactionQueues(std::vector<TransactionState>&, TransactionFlushState&);		int flushPendingTransactionQueues(std::vector<TransactionState>&, TransactionFlushState&);
			void applyUnsignaledBufferTransaction(std::vector<TransactionState>&, TransactionFlushState&);
			void popTransactionFromPending(std::vector<TransactionState>&, TransactionFlushState&,
			std::queue<TransactionState>&);
	TransactionReadiness applyFilters(TransactionFlushState&);		TransactionReadiness applyFilters(TransactionFlushState&);
	std::unordered_map<sp<IBinder>, std::queue<TransactionState>, IListenerHash>		std::unordered_map<sp<IBinder>, std::queue<TransactionState>, IListenerHash>
	mPendingTransactionQueues;		mPendingTransactionQueues;

services/surfaceflinger/SurfaceFlinger.cpp

+14 −15

Original line number	Original line	Diff line number	Diff line
	@@ -4262,20 +4262,23 @@ TransactionHandler::TransactionReadiness SurfaceFlinger::transactionReadyBufferC
	return TraverseBuffersReturnValues::STOP_TRAVERSAL;		return TraverseBuffersReturnValues::STOP_TRAVERSAL;
	}		}

			// ignore the acquire fence if LatchUnsignaledConfig::Always is set.
			const bool checkAcquireFence = enableLatchUnsignaledConfig != LatchUnsignaledConfig::Always;
			const bool acquireFenceAvailable = s.bufferData &&
			s.bufferData->flags.test(BufferData::BufferDataChange::fenceChanged) &&
			s.bufferData->acquireFence;
			const bool fenceSignaled = !checkAcquireFence \|\| !acquireFenceAvailable \|\|
			s.bufferData->acquireFence->getStatus() != Fence::Status::Unsignaled;
			if (!fenceSignaled) {
	// check fence status		// check fence status
	const bool allowLatchUnsignaled = shouldLatchUnsignaled(layer, s, transaction.states.size(),		const bool allowLatchUnsignaled =
			shouldLatchUnsignaled(layer, s, transaction.states.size(),
	flushState.firstTransaction);		flushState.firstTransaction);
	ATRACE_FORMAT("%s allowLatchUnsignaled=%s", layer->getName().c_str(),		ATRACE_FORMAT("%s allowLatchUnsignaled=%s", layer->getName().c_str(),
	allowLatchUnsignaled ? "true" : "false");		allowLatchUnsignaled ? "true" : "false");
			if (allowLatchUnsignaled) {
	const bool acquireFenceChanged = s.bufferData &&		ready = TransactionReadiness::NotReadyUnsignaled;
	s.bufferData->flags.test(BufferData::BufferDataChange::fenceChanged) &&		} else {
	s.bufferData->acquireFence;
	const bool fenceSignaled =
	(!acquireFenceChanged \|\|
	s.bufferData->acquireFence->getStatus() != Fence::Status::Unsignaled);
	if (!fenceSignaled) {
	if (!allowLatchUnsignaled) {
	ready = TransactionReadiness::NotReady;		ready = TransactionReadiness::NotReady;
	auto& listener = s.bufferData->releaseBufferListener;		auto& listener = s.bufferData->releaseBufferListener;
	if (listener &&		if (listener &&
	@@ -4288,10 +4291,6 @@ TransactionHandler::TransactionReadiness SurfaceFlinger::transactionReadyBufferC
	}		}
	return TraverseBuffersReturnValues::STOP_TRAVERSAL;		return TraverseBuffersReturnValues::STOP_TRAVERSAL;
	}		}

	ready = enableLatchUnsignaledConfig == LatchUnsignaledConfig::AutoSingleLayer
	? TransactionReadiness::ReadyUnsignaledSingle
	: TransactionReadiness::ReadyUnsignaled;
	}		}
	return TraverseBuffersReturnValues::CONTINUE_TRAVERSAL;		return TraverseBuffersReturnValues::CONTINUE_TRAVERSAL;
	});		});

services/surfaceflinger/SurfaceFlinger.h

+5 −0

Original line number	Original line	Diff line number	Diff line
	@@ -174,10 +174,15 @@ enum class LatchUnsignaledConfig {
	// Latch unsignaled is permitted when a single layer is updated in a frame,		// Latch unsignaled is permitted when a single layer is updated in a frame,
	// and the update includes just a buffer update (i.e. no sync transactions		// and the update includes just a buffer update (i.e. no sync transactions
	// or geometry changes).		// or geometry changes).
			// Latch unsignaled is also only permitted when a single transaction is ready
			// to be applied. If we pass an unsignaled fence to HWC, HWC might miss presenting
			// the frame if the fence does not fire in time. If we apply another transaction,
			// we may penalize the other transaction unfairly.
	AutoSingleLayer,		AutoSingleLayer,

	// All buffers are latched unsignaled. This behaviour is discouraged as it		// All buffers are latched unsignaled. This behaviour is discouraged as it
	// can break sync transactions, stall the display and cause undesired side effects.		// can break sync transactions, stall the display and cause undesired side effects.
			// This is equivalent to ignoring the acquire fence when applying transactions.
	Always,		Always,
	};		};

services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp

+81 −3

Original line number	Original line	Diff line number	Diff line
	@@ -294,7 +294,8 @@ public:
	return fence;		return fence;
	}		}

	ComposerState createComposerState(int layerId, sp<Fence> fence, uint64_t what) {		ComposerState createComposerState(int layerId, sp<Fence> fence, uint64_t what,
			std::optional<sp<IBinder>> layerHandle = std::nullopt) {
	ComposerState state;		ComposerState state;
	state.state.bufferData =		state.state.bufferData =
	std::make_shared<fake::BufferData>(/* bufferId / 123L, / width */ 1,		std::make_shared<fake::BufferData>(/* bufferId / 123L, / width */ 1,
	@@ -302,15 +303,20 @@ public:
	/* outUsage */ 0);		/* outUsage */ 0);
	state.state.bufferData->acquireFence = std::move(fence);		state.state.bufferData->acquireFence = std::move(fence);
	state.state.layerId = layerId;		state.state.layerId = layerId;
	state.state.surface =		state.state.surface = layerHandle.value_or(
	sp<Layer>::make(LayerCreationArgs(mFlinger.flinger(), nullptr, "TestLayer", 0, {}))		sp<Layer>::make(LayerCreationArgs(mFlinger.flinger(), nullptr, "TestLayer", 0, {}))
	->getHandle();		->getHandle());
	state.state.bufferData->flags = BufferData::BufferDataChange::fenceChanged;		state.state.bufferData->flags = BufferData::BufferDataChange::fenceChanged;

	state.state.what = what;		state.state.what = what;
	if (what & layer_state_t::eCropChanged) {		if (what & layer_state_t::eCropChanged) {
	state.state.crop = Rect(1, 2, 3, 4);		state.state.crop = Rect(1, 2, 3, 4);
	}		}
			if (what & layer_state_t::eFlagsChanged) {
			state.state.flags = layer_state_t::eEnableBackpressure;
			state.state.mask = layer_state_t::eEnableBackpressure;
			}

	return state;		return state;
	}		}

	@@ -601,6 +607,41 @@ TEST_F(LatchUnsignaledAutoSingleLayerTest, DontLatchUnsignaledWhenEarlyOffset) {
	setTransactionStates({unsignaledTransaction}, kExpectedTransactionsPending);		setTransactionStates({unsignaledTransaction}, kExpectedTransactionsPending);
	}		}

			TEST_F(LatchUnsignaledAutoSingleLayerTest, UnsignaledNotAppliedWhenThereAreSignaled_SignaledFirst) {
			const sp<IBinder> kApplyToken1 =
			IInterface::asBinder(TransactionCompletedListener::getIInstance());
			const sp<IBinder> kApplyToken2 = sp<BBinder>::make();
			const sp<IBinder> kApplyToken3 = sp<BBinder>::make();
			const auto kLayerId1 = 1;
			const auto kLayerId2 = 2;
			const auto kExpectedTransactionsPending = 1u;

			const auto signaledTransaction =
			createTransactionInfo(kApplyToken1,
			{
			createComposerState(kLayerId1,
			fence(Fence::Status::Signaled),
			layer_state_t::eBufferChanged),
			});
			const auto signaledTransaction2 =
			createTransactionInfo(kApplyToken2,
			{
			createComposerState(kLayerId1,
			fence(Fence::Status::Signaled),
			layer_state_t::eBufferChanged),
			});
			const auto unsignaledTransaction =
			createTransactionInfo(kApplyToken3,
			{
			createComposerState(kLayerId2,
			fence(Fence::Status::Unsignaled),
			layer_state_t::eBufferChanged),
			});

			setTransactionStates({signaledTransaction, signaledTransaction2, unsignaledTransaction},
			kExpectedTransactionsPending);
			}

	class LatchUnsignaledDisabledTest : public LatchUnsignaledTest {		class LatchUnsignaledDisabledTest : public LatchUnsignaledTest {
	public:		public:
	void SetUp() override {		void SetUp() override {
	@@ -943,6 +984,43 @@ TEST_F(LatchUnsignaledAlwaysTest, Flush_RemovesUnsignaledFromTheQueue) {
	kExpectedTransactionsPending);		kExpectedTransactionsPending);
	}		}

			TEST_F(LatchUnsignaledAlwaysTest, RespectsBackPressureFlag) {
			const sp<IBinder> kApplyToken1 =
			IInterface::asBinder(TransactionCompletedListener::getIInstance());
			const sp<IBinder> kApplyToken2 = sp<BBinder>::make();
			const auto kLayerId1 = 1;
			const auto kExpectedTransactionsPending = 1u;
			auto layer =
			sp<Layer>::make(LayerCreationArgs(mFlinger.flinger(), nullptr, "TestLayer", 0, {}));
			auto layerHandle = layer->getHandle();
			const auto setBackPressureFlagTransaction =
			createTransactionInfo(kApplyToken1,
			{createComposerState(kLayerId1, fence(Fence::Status::Unsignaled),
			layer_state_t::eBufferChanged \|
			layer_state_t::eFlagsChanged,
			{layerHandle})});
			setTransactionStates({setBackPressureFlagTransaction}, 0u);

			const auto unsignaledTransaction =
			createTransactionInfo(kApplyToken1,
			{
			createComposerState(kLayerId1,
			fence(Fence::Status::Unsignaled),
			layer_state_t::eBufferChanged,
			{layerHandle}),
			});
			const auto unsignaledTransaction2 =
			createTransactionInfo(kApplyToken1,
			{
			createComposerState(kLayerId1,
			fence(Fence::Status::Unsignaled),
			layer_state_t::eBufferChanged,
			{layerHandle}),
			});
			setTransactionStates({unsignaledTransaction, unsignaledTransaction2},
			kExpectedTransactionsPending);
			}

	TEST_F(LatchUnsignaledAlwaysTest, LatchUnsignaledWhenEarlyOffset) {		TEST_F(LatchUnsignaledAlwaysTest, LatchUnsignaledWhenEarlyOffset) {
	const sp<IBinder> kApplyToken =		const sp<IBinder> kApplyToken =
	IInterface::asBinder(TransactionCompletedListener::getIInstance());		IInterface::asBinder(TransactionCompletedListener::getIInstance());