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

Commit 023162b7 authored by Tianyu Jiang's avatar Tianyu Jiang Committed by Android (Google) Code Review
Browse files

Merge "Change the definition of buffer state and client state bits."

parents 8f91005f f669f6a2
Loading
Loading
Loading
Loading
+4 −4
Original line number Diff line number Diff line
@@ -36,7 +36,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;
@@ -119,9 +119,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;
+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) {
+168 −111

File changed.

Preview size limit exceeded, changes collapsed.

+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();
  }
+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;
}

Loading