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

Commit 0229d25d authored by Tianyu's avatar Tianyu
Browse files

Remove active_buffer_bit_mask_ from BufferNode.

BufferNode already have metadata which contains active_clients_bit_mask,
which serves the same purpose as active_buffer_bit_mask_ but also have
other usage. This change removes the redundant active_buffer_bit_mask_
from BufferNode.

Test: buffer_node-test on marlin-eng and vega_xr build
Bug: 112007999
Change-Id: I7695dc5d6eb84a4a5c73148e9636871cc7776df4
parent b61df911
Loading
Loading
Loading
Loading
+24 −36
Original line number Diff line number Diff line
#include <errno.h>
#include <private/dvr/buffer_channel.h>
#include <private/dvr/producer_channel.h>

@@ -16,9 +17,8 @@ BufferChannel::BufferChannel(BufferHubService* service, int buffer_id,
                             size_t user_metadata_size)
    : BufferHubChannel(service, buffer_id, channel_id, kDetachedBufferType),
      buffer_node_(
          std::make_shared<BufferNode>(std::move(buffer), user_metadata_size)),
      buffer_state_bit_(BufferHubDefs::FindFirstClearedBit()) {
  buffer_node_->set_buffer_state_bit(buffer_state_bit_);
          std::make_shared<BufferNode>(std::move(buffer), user_metadata_size)) {
  buffer_state_bit_ = buffer_node_->AddNewActiveClientsBitToMask();
}

BufferChannel::BufferChannel(BufferHubService* service, int buffer_id,
@@ -27,24 +27,28 @@ BufferChannel::BufferChannel(BufferHubService* service, int buffer_id,
                             uint64_t usage, size_t user_metadata_size)
    : BufferHubChannel(service, buffer_id, buffer_id, kDetachedBufferType),
      buffer_node_(std::make_shared<BufferNode>(
          width, height, layer_count, format, usage, user_metadata_size)),
      buffer_state_bit_(BufferHubDefs::FindFirstClearedBit()) {
  buffer_node_->set_buffer_state_bit(buffer_state_bit_);
          width, height, layer_count, format, usage, user_metadata_size)) {
  buffer_state_bit_ = buffer_node_->AddNewActiveClientsBitToMask();
}

BufferChannel::BufferChannel(BufferHubService* service, int buffer_id,
                             int channel_id,
                             std::shared_ptr<BufferNode> buffer_node,
                             uint64_t buffer_state_bit)
                             std::shared_ptr<BufferNode> buffer_node)
    : BufferHubChannel(service, buffer_id, channel_id, kDetachedBufferType),
      buffer_node_(buffer_node),
      buffer_state_bit_(buffer_state_bit) {
  buffer_node_->set_buffer_state_bit(buffer_state_bit_);
      buffer_node_(buffer_node) {
  buffer_state_bit_ = buffer_node_->AddNewActiveClientsBitToMask();
  if (buffer_state_bit_ == 0ULL) {
    ALOGE("BufferChannel::BufferChannel: %s", strerror(errno));
    buffer_node_ = nullptr;
  }
}

BufferChannel::~BufferChannel() {
  ALOGD_IF(TRACE, "BufferChannel::~BufferChannel: channel_id=%d buffer_id=%d.",
           channel_id(), buffer_id());
  if (buffer_state_bit_ != 0ULL) {
    buffer_node_->RemoveClientsBitFromMask(buffer_state_bit_);
  }
  Hangup();
}

@@ -101,38 +105,22 @@ Status<BufferTraits<BorrowedHandle>> BufferChannel::OnImport(
      /*released_fence_fd=*/BorrowedHandle{}};
}

Status<RemoteChannelHandle> BufferChannel::OnDuplicate(
    Message& message) {
Status<RemoteChannelHandle> BufferChannel::OnDuplicate(Message& message) {
  ATRACE_NAME("BufferChannel::OnDuplicate");
  ALOGD_IF(TRACE, "BufferChannel::OnDuplicate: buffer=%d.",
           buffer_id());
  ALOGD_IF(TRACE, "BufferChannel::OnDuplicate: buffer=%d.", buffer_id());

  int channel_id;
  auto status = message.PushChannel(0, nullptr, &channel_id);
  if (!status) {
    ALOGE(
        "BufferChannel::OnDuplicate: Failed to push buffer channel: %s",
  if (!status.ok()) {
    ALOGE("BufferChannel::OnDuplicate: Failed to push buffer channel: %s",
          status.GetErrorMessage().c_str());
    return ErrorStatus(ENOMEM);
  }

  // Try find the next buffer state bit which has not been claimed by any
  // other buffers yet.
  uint64_t buffer_state_bit =
      BufferHubDefs::FindNextClearedBit(buffer_node_->active_buffer_bit_mask() |
                                        BufferHubDefs::kProducerStateBit);
  if (buffer_state_bit == 0ULL) {
    ALOGE(
        "BufferChannel::OnDuplicate: reached the maximum mumber of channels "
        "per buffer node: 63.");
    return ErrorStatus(E2BIG);
  }

  auto channel =
      std::shared_ptr<BufferChannel>(new BufferChannel(
          service(), buffer_id(), channel_id, buffer_node_, buffer_state_bit));
  if (!channel) {
    ALOGE("BufferChannel::OnDuplicate: Invalid buffer.");
  auto channel = std::shared_ptr<BufferChannel>(
      new BufferChannel(service(), buffer_id(), channel_id, buffer_node_));
  if (!channel->IsValid()) {
    ALOGE("BufferChannel::OnDuplicate: Invalid buffer. %s", strerror(errno));
    return ErrorStatus(EINVAL);
  }

+43 −0
Original line number Diff line number Diff line
#include <errno.h>
#include <private/dvr/buffer_hub_defs.h>
#include <private/dvr/buffer_node.h>

namespace android {
namespace dvr {

void BufferNode::InitializeMetadata() {
  // Using placement new here to reuse shared memory instead of new allocation
  // Initialize the atomic variables to zero.
  BufferHubDefs::MetadataHeader* metadata_header = metadata_.metadata_header();
  buffer_state_ = new (&metadata_header->buffer_state) std::atomic<uint64_t>(0);
  fence_state_ = new (&metadata_header->fence_state) std::atomic<uint64_t>(0);
  active_clients_bit_mask_ =
      new (&metadata_header->active_clients_bit_mask) std::atomic<uint64_t>(0);
}

BufferNode::BufferNode(IonBuffer buffer, size_t user_metadata_size)
    : buffer_(std::move(buffer)) {
  metadata_ = BufferHubMetadata::Create(user_metadata_size);
  InitializeMetadata();
}

// Allocates a new BufferNode.
@@ -22,6 +34,37 @@ BufferNode::BufferNode(uint32_t width, uint32_t height, uint32_t layer_count,
  }

  metadata_ = BufferHubMetadata::Create(user_metadata_size);
  InitializeMetadata();
}

uint64_t BufferNode::GetActiveClientsBitMask() const {
  return active_clients_bit_mask_->load(std::memory_order_acquire);
}

uint64_t BufferNode::AddNewActiveClientsBitToMask() {
  uint64_t current_active_clients_bit_mask = GetActiveClientsBitMask();
  uint64_t buffer_state_bit = 0ULL;
  uint64_t updated_active_clients_bit_mask = 0ULL;
  do {
    buffer_state_bit =
        BufferHubDefs::FindNextClearedBit(current_active_clients_bit_mask);
    if (buffer_state_bit == 0ULL) {
      ALOGE(
          "BufferNode::AddNewActiveClientsBitToMask: reached the maximum "
          "mumber of channels per buffer node: 32.");
      errno = E2BIG;
      return 0ULL;
    }
    updated_active_clients_bit_mask =
        current_active_clients_bit_mask | buffer_state_bit;
  } while (!(active_clients_bit_mask_->compare_exchange_weak(
      current_active_clients_bit_mask, updated_active_clients_bit_mask,
      std::memory_order_acq_rel, std::memory_order_acquire)));
  return buffer_state_bit;
}

void BufferNode::RemoveClientsBitFromMask(const uint64_t& value) {
  active_clients_bit_mask_->fetch_and(~value);
}

}  // namespace dvr
+5 −5
Original line number Diff line number Diff line
@@ -42,20 +42,20 @@ class BufferChannel : public BufferHubChannel {
                uint32_t height, uint32_t layer_count, uint32_t format,
                uint64_t usage, size_t user_metadata_size);

  // Creates a detached buffer from an existing BufferNode.
  // Creates a detached buffer from an existing BufferNode. This method is used
  // in OnDuplicate method.
  BufferChannel(BufferHubService* service, int buffer_id, int channel_id,
                std::shared_ptr<BufferNode> buffer_node,
                uint64_t buffer_state_bit);
                std::shared_ptr<BufferNode> buffer_node);

  pdx::Status<BufferTraits<pdx::BorrowedHandle>> OnImport(
      pdx::Message& message);
  pdx::Status<pdx::RemoteChannelHandle> OnDuplicate(pdx::Message& message);

  // The concrete implementation of the Buffer object.
  std::shared_ptr<BufferNode> buffer_node_;
  std::shared_ptr<BufferNode> buffer_node_ = nullptr;

  // The state bit of this buffer. Must be one the lower 63 bits.
  uint64_t buffer_state_bit_;
  uint64_t buffer_state_bit_ = 0ULL;
};

}  // namespace dvr
+40 −10
Original line number Diff line number Diff line
@@ -10,7 +10,7 @@ namespace dvr {
class BufferNode {
 public:
  // Creates a BufferNode from existing IonBuffers, i.e. creating from an
  // existing ProducerChannel.
  // existing ProducerChannel. Allocate a new BufferHubMetadata.
  BufferNode(IonBuffer buffer, size_t user_metadata_size);

  // Allocates a new BufferNode.
@@ -21,26 +21,56 @@ class BufferNode {
  bool IsValid() const { return buffer_.IsValid() && metadata_.IsValid(); }

  size_t user_metadata_size() const { return metadata_.user_metadata_size(); }
  uint64_t active_buffer_bit_mask() const { return active_buffer_bit_mask_; }
  void set_buffer_state_bit(uint64_t buffer_state_bit) {
    active_buffer_bit_mask_ |= buffer_state_bit;
  }

  // Accessor of the IonBuffer.
  // Accessors of the IonBuffer.
  IonBuffer& buffer() { return buffer_; }
  const IonBuffer& buffer() const { return buffer_; }

  // Accessor of the metadata.
  // Accessors of metadata.
  const BufferHubMetadata& metadata() const { return metadata_; }

  // Gets the current value of active_clients_bit_mask in metadata_ with
  // std::memory_order_acquire, so that all previous releases of
  // active_clients_bit_mask from all threads will be returned here.
  uint64_t GetActiveClientsBitMask() const;

  // Find and add a new buffer_state_bit to active_clients_bit_mask in
  // metadata_.
  // Return the new buffer_state_bit that is added to active_clients_bit_mask.
  // Return 0ULL if there are already 32 bp clients of the buffer.
  uint64_t AddNewActiveClientsBitToMask();

  // Removes the value from active_clients_bit_mask in metadata_ with
  // std::memory_order_release, so that the change will be visible to any
  // acquire of active_clients_bit_mask_ in any threads after the succeed of
  // this operation.
  void RemoveClientsBitFromMask(const uint64_t& value);

 private:
  // Helper method for constructors to initialize atomic metadata header
  // variables in shared memory.
  void InitializeMetadata();

  // Gralloc buffer handles.
  IonBuffer buffer_;

  // Metadata in shared memory.
  BufferHubMetadata metadata_;

  // All active buffer bits. Valid bits are the lower 63 bits, while the
  // highest bit is reserved for the exclusive writing and should not be set.
  uint64_t active_buffer_bit_mask_ = 0ULL;
  // The following variables are atomic variables in metadata_ that are visible
  // to Bn object and Bp objects. Please find more info in
  // BufferHubDefs::MetadataHeader.

  // buffer_state_ tracks the state of the buffer. Buffer can be in one of these
  // four states: gained, posted, acquired, released.
  std::atomic<uint64_t>* buffer_state_ = nullptr;

  // TODO(b/112012161): add comments to fence_state_.
  std::atomic<uint64_t>* fence_state_ = nullptr;

  // active_clients_bit_mask_ tracks all the bp clients of the buffer. It is the
  // union of all buffer_state_bit of all bp clients.
  std::atomic<uint64_t>* active_clients_bit_mask_ = nullptr;
};

}  // namespace dvr
+28 −1
Original line number Diff line number Diff line
@@ -24,3 +24,30 @@ cc_test {
    // TODO(b/117568153): Temporarily opt out using libcrt.
    no_libcrt: true,
}

cc_test {
    name: "buffer_node-test",
    srcs: ["buffer_node-test.cpp"],
    cflags: [
        "-DLOG_TAG=\"buffer_node-test\"",
        "-DTRACE=0",
        "-DATRACE_TAG=ATRACE_TAG_GRAPHICS",
    ],
    header_libs: ["libdvr_headers"],
    static_libs: [
        "libbufferhub",
        "libbufferhubd",
        "libgmock",
    ],
    shared_libs: [
        "libbase",
        "libbinder",
        "liblog",
        "libpdx_default_transport",
        "libui",
        "libutils",
    ],
    // TODO(b/117568153): Temporarily opt out using libcrt.
    no_libcrt: true,
}
Loading