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

Commit f669f6a2 authored by Tianyu's avatar Tianyu Committed by Tianyu Jiang
Browse files

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
parent face1763

libs/ui/tests/BufferHubBuffer_test.cpp

+4 −4
Original line number Diff line number Diff line
@@ -35,7 +35,7 @@ const int kFormat = HAL_PIXEL_FORMAT_RGBA_8888; 
const int kUsage = 0;

const size_t kUserMetadataSize = 0;



using dvr::BufferHubDefs::IsBufferGained;

using dvr::BufferHubDefs::IsBufferReleased;

using dvr::BufferHubDefs::kFirstClientBitMask;

using dvr::BufferHubDefs::kMetadataHeaderSize;

using frameworks::bufferhub::V1_0::BufferHubStatus;
@@ -118,9 +118,9 @@ TEST_F(BufferHubBufferTest, DuplicateBufferHubBuffer) { 
    // We use client_state_mask() to tell those two instances apart.

    EXPECT_NE(bufferStateMask1, bufferStateMask2);



    // Both buffer instances should be in gained state.

    EXPECT_TRUE(IsBufferGained(b1->buffer_state()));

    EXPECT_TRUE(IsBufferGained(b2->buffer_state()));

    // Both buffer instances should be in released state currently.

    EXPECT_TRUE(IsBufferReleased(b1->buffer_state()));

    EXPECT_TRUE(IsBufferReleased(b2->buffer_state()));



    // TODO(b/112338294): rewrite test after migration

    return;

libs/ui/tests/BufferHubMetadata_test.cpp

+3 −9
Original line number Diff line number Diff line
@@ -17,7 +17,7 @@ 
#include <gtest/gtest.h>

#include <ui/BufferHubMetadata.h>



using android::dvr::BufferHubDefs::IsBufferGained;

using android::dvr::BufferHubDefs::IsBufferReleased;



namespace android {

namespace dvr {
@@ -52,19 +52,13 @@ TEST_F(BufferHubMetadataTest, Import_Success) { 
  BufferHubDefs::MetadataHeader* mh1 = m1.metadata_header();

  EXPECT_NE(mh1, nullptr);



  // TODO(b/111976433): Update this test once BufferHub state machine gets

  // updated. In the old model, buffer starts in the gained state (i.e.

  // valued 0). In the new model, buffer states in the released state.

  EXPECT_TRUE(IsBufferGained(mh1->fence_state.load()));

  EXPECT_TRUE(IsBufferReleased(mh1->buffer_state.load()));



  EXPECT_TRUE(m2.IsValid());

  BufferHubDefs::MetadataHeader* mh2 = m2.metadata_header();

  EXPECT_NE(mh2, nullptr);



  // TODO(b/111976433): Update this test once BufferHub state machine gets

  // updated. In the old model, buffer starts in the gained state (i.e.

  // valued 0). In the new model, buffer states in the released state.

  EXPECT_TRUE(IsBufferGained(mh2->fence_state.load()));

  EXPECT_TRUE(IsBufferReleased(mh2->buffer_state.load()));

}



TEST_F(BufferHubMetadataTest, MoveMetadataInvalidatesOldOne) {

libs/vr/libbufferhub/buffer_hub-test.cpp

+168 −111

File changed.

Preview size limit exceeded, changes collapsed.

libs/vr/libbufferhub/buffer_hub_base.cpp

+2 −0
Original line number Diff line number Diff line
@@ -26,6 +26,8 @@ BufferHubBase::BufferHubBase(const std::string& endpoint_path) 
      cid_(-1) {}



BufferHubBase::~BufferHubBase() {

  // buffer_state and fence_state are not reset here. They will be used to

  // clean up epoll fd if necessary in ProducerChannel::RemoveConsumer method.

  if (metadata_header_ != nullptr) {

    metadata_buffer_.Unlock();

  }

libs/vr/libbufferhub/consumer_buffer.cpp

+52 −18
Original line number Diff line number Diff line
@@ -35,17 +35,41 @@ int ConsumerBuffer::LocalAcquire(DvrNativeBufferMetadata* out_meta, 
  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 @@ int ConsumerBuffer::LocalAcquire(DvrNativeBufferMetadata* out_meta, 
    *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 @@ int ConsumerBuffer::LocalRelease(const DvrNativeBufferMetadata* meta, 
  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 @@ int ConsumerBuffer::LocalRelease(const DvrNativeBufferMetadata* meta, 
  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;

}