Change the definition of buffer state and client state bits.
Please refer to go/bufferhub-buffer-state-redesign for more information.
In this change:
1. Every clients takes up two bits in the buffer_state.
One from the higher 32 bits, one from the lower 32 bits. For details:
go/bufferhub-buffer-state-redesign
2. Upon the creation of a new buffer, the buffer is in released state.
Previously, only producer creates buffer, and upon creation, the buffer
was in gained state. Now, producer needs to specifically gain the buffer
before trying to produce and post it.
3. If there is no other clients when a client post a buffer, the buffer
will actually be in released state instead of posted state. This is
because the posted buffer does not have readers and can be reused
immediately.
4. If a new client is added to the buffer when the buffer is in acquired
or posted state, the buffer state of the new client will be set to posted
state and able to acquire the same buffer content as posted.
In the next change:
variables of type std::atomic<uint64_t> in metadata header in shared memory
will be replaced by std::atomic<uint32_t>
Test: marlin-eng passing AHardwareBufferTest BufferHubBuffer_test
BufferHubMetadata_test buffer_hub_binder_service-test
buffer_hub_queue_producer-test dvr_api-test libgui_test
libsensor_test vrflinger_test buffer_hub-test
dvr_buffer_queue-test dvr_display-test buffer_hub_queue-test
Test: smartphone VR works on blueline-eng
Test: vega_xr passing AHardwareBufferTest BufferHubBuffer_test
BufferHubMetadata_test buffer_hub_queue_producer-test buffer_hub-test
buffer_hub_queue-test dvr_buffer_queue-test dvr_api-test
Cherrypicking this changelist to oc-dr1-daydream-dev branch requires
ag/5514563 to be merged at the same time to make Vega actually work.
Bug: 112007999
Change-Id: I86393818ad922a91c709fe22f8e99b0667d2e9ef
diff --git a/libs/vr/libbufferhub/producer_buffer.cpp b/libs/vr/libbufferhub/producer_buffer.cpp
index f36e169..1bfdc8f 100644
--- a/libs/vr/libbufferhub/producer_buffer.cpp
+++ b/libs/vr/libbufferhub/producer_buffer.cpp
@@ -79,14 +79,43 @@
if (const int error = CheckMetadata(meta->user_metadata_size))
return error;
- // Check invalid state transition.
- uint64_t buffer_state = buffer_state_->load(std::memory_order_acquire);
- if (!BufferHubDefs::IsBufferGained(buffer_state)) {
- ALOGE("ProducerBuffer::LocalPost: not gained, id=%d state=%" PRIx64 ".",
- id(), buffer_state);
+ // The buffer can be posted iff the buffer state for this client is gained.
+ uint64_t current_buffer_state =
+ buffer_state_->load(std::memory_order_acquire);
+ if (!BufferHubDefs::IsClientGained(current_buffer_state,
+ client_state_mask())) {
+ ALOGE("%s: not gained, id=%d state=%" PRIx64 ".", __FUNCTION__, id(),
+ current_buffer_state);
return -EBUSY;
}
+ // Set the producer client buffer state to released, other clients' buffer
+ // state to posted.
+ uint64_t current_active_clients_bit_mask =
+ active_clients_bit_mask_->load(std::memory_order_acquire);
+ uint64_t updated_buffer_state = current_active_clients_bit_mask &
+ (~client_state_mask()) &
+ BufferHubDefs::kHighBitsMask;
+ while (!buffer_state_->compare_exchange_weak(
+ current_buffer_state, updated_buffer_state, std::memory_order_acq_rel,
+ std::memory_order_acquire)) {
+ ALOGD(
+ "%s: Failed to post the buffer. Current buffer state was changed to "
+ "%" PRIx64
+ " when trying to post the buffer and modify the buffer state to "
+ "%" PRIx64
+ ". About to try again if the buffer is still gained by this client.",
+ __FUNCTION__, current_buffer_state, updated_buffer_state);
+ if (!BufferHubDefs::IsClientGained(current_buffer_state,
+ client_state_mask())) {
+ ALOGE(
+ "%s: Failed to post the buffer. The buffer is no longer gained, "
+ "id=%d state=%" PRIx64 ".",
+ __FUNCTION__, id(), current_buffer_state);
+ return -EBUSY;
+ }
+ }
+
// Copy the canonical metadata.
void* metadata_ptr = reinterpret_cast<void*>(&metadata_header_->metadata);
memcpy(metadata_ptr, meta, sizeof(DvrNativeBufferMetadata));
@@ -101,10 +130,6 @@
if (const int error = UpdateSharedFence(ready_fence, shared_acquire_fence_))
return error;
- // Set the producer bit atomically to transit into posted state.
- // The producer state bit mask is kFirstClientBitMask for now.
- BufferHubDefs::ModifyBufferState(buffer_state_, 0ULL,
- BufferHubDefs::kFirstClientBitMask);
return 0;
}
@@ -136,25 +161,50 @@
int ProducerBuffer::LocalGain(DvrNativeBufferMetadata* out_meta,
LocalHandle* out_fence, bool gain_posted_buffer) {
- uint64_t buffer_state = buffer_state_->load(std::memory_order_acquire);
- ALOGD_IF(TRACE, "ProducerBuffer::LocalGain: buffer=%d, state=%" PRIx64 ".",
- id(), buffer_state);
-
if (!out_meta)
return -EINVAL;
- if (BufferHubDefs::IsBufferGained(buffer_state)) {
- // We don't want to log error when gaining a newly allocated
- // buffer.
- ALOGI("ProducerBuffer::LocalGain: already gained id=%d.", id());
+ uint64_t current_buffer_state =
+ buffer_state_->load(std::memory_order_acquire);
+ ALOGD_IF(TRACE, "%s: buffer=%d, state=%" PRIx64 ".", __FUNCTION__, id(),
+ current_buffer_state);
+
+ if (BufferHubDefs::IsClientGained(current_buffer_state,
+ client_state_mask())) {
+ ALOGI("%s: already gained id=%d.", __FUNCTION__, id());
return -EALREADY;
}
- if (BufferHubDefs::IsBufferAcquired(buffer_state) ||
- (BufferHubDefs::IsBufferPosted(buffer_state) && !gain_posted_buffer)) {
- ALOGE("ProducerBuffer::LocalGain: not released id=%d state=%" PRIx64 ".",
- id(), buffer_state);
+ if (BufferHubDefs::AnyClientAcquired(current_buffer_state) ||
+ (BufferHubDefs::AnyClientPosted(current_buffer_state) &&
+ !gain_posted_buffer)) {
+ ALOGE("%s: not released id=%d state=%" PRIx64 ".", __FUNCTION__, id(),
+ current_buffer_state);
return -EBUSY;
}
+ // Change the buffer state to gained state.
+ uint64_t updated_buffer_state = client_state_mask();
+ while (!buffer_state_->compare_exchange_weak(
+ current_buffer_state, updated_buffer_state, std::memory_order_acq_rel,
+ std::memory_order_acquire)) {
+ ALOGD(
+ "%s: Failed to gain the buffer. Current buffer state was changed to "
+ "%" PRIx64
+ " when trying to gain the buffer and modify the buffer state to "
+ "%" PRIx64
+ ". About to try again if the buffer is still not read by other "
+ "clients.",
+ __FUNCTION__, current_buffer_state, updated_buffer_state);
+
+ if (BufferHubDefs::AnyClientAcquired(current_buffer_state) ||
+ (BufferHubDefs::AnyClientPosted(current_buffer_state) &&
+ !gain_posted_buffer)) {
+ ALOGE(
+ "%s: Failed to gain the buffer. The buffer is no longer released. "
+ "id=%d state=%" PRIx64 ".",
+ __FUNCTION__, id(), current_buffer_state);
+ return -EBUSY;
+ }
+ }
// Canonical metadata is undefined on Gain. Except for user_metadata and
// release_fence_mask. Fill in the user_metadata_ptr in address space of the
@@ -169,16 +219,20 @@
out_meta->user_metadata_ptr = 0;
}
- uint64_t fence_state = fence_state_->load(std::memory_order_acquire);
+ uint64_t current_fence_state = fence_state_->load(std::memory_order_acquire);
+ uint64_t current_active_clients_bit_mask =
+ active_clients_bit_mask_->load(std::memory_order_acquire);
// If there is an release fence from consumer, we need to return it.
- if (fence_state & BufferHubDefs::kConsumerStateMask) {
+ // TODO(b/112007999) add an atomic variable in metadata header in shared
+ // memory to indicate which client is the last producer of the buffer.
+ // Currently, assume the first client is the only producer to the buffer.
+ if (current_fence_state & current_active_clients_bit_mask &
+ (~BufferHubDefs::kFirstClientBitMask)) {
*out_fence = shared_release_fence_.Duplicate();
out_meta->release_fence_mask =
- fence_state & BufferHubDefs::kConsumerStateMask;
+ current_fence_state & current_active_clients_bit_mask;
}
- // Clear out all bits and the buffer is now back to gained state.
- buffer_state_->store(0ULL);
return 0;
}
@@ -232,7 +286,8 @@
// TODO(b/112338294) Keep here for reference. Remove it after new logic is
// written.
/* uint64_t buffer_state = buffer_state_->load(std::memory_order_acquire);
- if (!BufferHubDefs::IsBufferGained(buffer_state)) {
+ if (!BufferHubDefs::IsClientGained(
+ buffer_state, BufferHubDefs::kFirstClientStateMask)) {
// Can only detach a ProducerBuffer when it's in gained state.
ALOGW("ProducerBuffer::Detach: The buffer (id=%d, state=0x%" PRIx64
") is not in gained state.",