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/consumer_buffer.cpp b/libs/vr/libbufferhub/consumer_buffer.cpp
index 8e630ec..62fb5fd 100644
--- a/libs/vr/libbufferhub/consumer_buffer.cpp
+++ b/libs/vr/libbufferhub/consumer_buffer.cpp
@@ -35,17 +35,41 @@
if (!out_meta)
return -EINVAL;
- // Only check producer bit and this consumer buffer's particular consumer bit.
- // The buffer is can be acquired iff: 1) producer bit is set; 2) consumer bit
- // is not set.
- uint64_t buffer_state = buffer_state_->load(std::memory_order_acquire);
- if (!BufferHubDefs::IsBufferPosted(buffer_state, client_state_mask())) {
- ALOGE("ConsumerBuffer::LocalAcquire: not posted, id=%d state=%" PRIx64
- " client_state_mask=%" PRIx64 ".",
- id(), buffer_state, client_state_mask());
+ // The buffer can be acquired iff the buffer state for this client is posted.
+ uint64_t current_buffer_state =
+ buffer_state_->load(std::memory_order_acquire);
+ if (!BufferHubDefs::IsClientPosted(current_buffer_state,
+ client_state_mask())) {
+ ALOGE(
+ "%s: Failed to acquire the buffer. The buffer is not posted, id=%d "
+ "state=%" PRIx64 " client_state_mask=%" PRIx64 ".",
+ __FUNCTION__, id(), current_buffer_state, client_state_mask());
return -EBUSY;
}
+ // Change the buffer state for this consumer from posted to acquired.
+ uint64_t updated_buffer_state = current_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 acquire the buffer. Current buffer state was changed to "
+ "%" PRIx64
+ " when trying to acquire the buffer and modify the buffer state to "
+ "%" PRIx64 ". About to try again if the buffer is still posted.",
+ __FUNCTION__, current_buffer_state, updated_buffer_state);
+ if (!BufferHubDefs::IsClientPosted(current_buffer_state,
+ client_state_mask())) {
+ ALOGE(
+ "%s: Failed to acquire the buffer. The buffer is no longer posted, "
+ "id=%d state=%" PRIx64 " client_state_mask=%" PRIx64 ".",
+ __FUNCTION__, id(), current_buffer_state, client_state_mask());
+ return -EBUSY;
+ }
+ // The failure of compare_exchange_weak updates current_buffer_state.
+ updated_buffer_state = current_buffer_state ^ client_state_mask();
+ }
+
// Copy the canonical metadata.
void* metadata_ptr = reinterpret_cast<void*>(&metadata_header_->metadata);
memcpy(out_meta, metadata_ptr, sizeof(DvrNativeBufferMetadata));
@@ -64,8 +88,6 @@
*out_fence = shared_acquire_fence_.Duplicate();
}
- // Set the consumer bit unique to this consumer.
- BufferHubDefs::ModifyBufferState(buffer_state_, 0ULL, client_state_mask());
return 0;
}
@@ -118,12 +140,26 @@
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::IsBufferAcquired(buffer_state)) {
- ALOGE("ConsumerBuffer::LocalRelease: not acquired id=%d state=%" PRIx64 ".",
- id(), buffer_state);
- return -EBUSY;
+ // Set the buffer state of this client to released if it is not already in
+ // released state.
+ uint64_t current_buffer_state =
+ buffer_state_->load(std::memory_order_acquire);
+ if (BufferHubDefs::IsClientReleased(current_buffer_state,
+ client_state_mask())) {
+ return 0;
+ }
+ uint64_t updated_buffer_state = current_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 release the buffer. Current buffer state was changed to "
+ "%" PRIx64
+ " when trying to release the buffer and modify the buffer state to "
+ "%" PRIx64 ". About to try again.",
+ __FUNCTION__, current_buffer_state, updated_buffer_state);
+ // The failure of compare_exchange_weak updates current_buffer_state.
+ updated_buffer_state = current_buffer_state & (~client_state_mask());
}
// On release, only the user requested metadata is copied back into the shared
@@ -141,8 +177,6 @@
if (const int error = UpdateSharedFence(release_fence, shared_release_fence_))
return error;
- // For release operation, the client don't need to change the state as it's
- // bufferhubd's job to flip the produer bit once all consumers are released.
return 0;
}