Remove pending_consumers_ from producer channel
pending_consumers_ was created before the creation of buffer state atomics.
Producer channel need to check and issue signal if pending_consumers_ is
zero. pending_consumers_ == 0 is the same as buffer_state of
non-orphaned buffers are fully released. Thus, removing
pending_consumers_ as it gives redundant information and lower the
complexity of producer channel.
Bug: 119331650
Test: all tests are passing
on Blueline:
AHardwareBufferTest BufferHubBuffer_test BufferHubMetadata_test
buffer_hub_binder_service-test buffer_hub_queue_producer-test
libgui_test libsensor_test vrflinger_test buffer_hub-test
buffer_hub_queue-test dvr_buffer_queue-test dvr_api-test
dvr_display-test
on Vega:
AHardwareBufferTest BufferHubBuffer_test BufferHubMetadata_test
buffer_hub_queue_producer-test buffer_hub-test buffer_hub_queue-test
dvr_buffer_queue-test dvr_api-test libdvrtracking-test
Change-Id: I5ccf3e6b3a2a304e238c272bda443e6d180babc3
diff --git a/services/vr/bufferhubd/producer_channel.cpp b/services/vr/bufferhubd/producer_channel.cpp
index 4b4301f..ab1d94c 100644
--- a/services/vr/bufferhubd/producer_channel.cpp
+++ b/services/vr/bufferhubd/producer_channel.cpp
@@ -56,7 +56,6 @@
uint64_t usage, size_t user_metadata_size,
int* error)
: BufferHubChannel(service, channel_id, channel_id, kProducerType),
- pending_consumers_(0),
user_metadata_size_(user_metadata_size),
metadata_buf_size_(BufferHubDefs::kMetadataHeaderSize +
user_metadata_size) {
@@ -183,7 +182,7 @@
return BufferInfo(buffer_id(), consumer_channels_.size(), buffer_.width(),
buffer_.height(), buffer_.layer_count(), buffer_.format(),
- buffer_.usage(), pending_consumers_,
+ buffer_.usage(),
buffer_state_->load(std::memory_order_acquire),
signaled_mask, metadata_header_->queue_index);
}
@@ -370,8 +369,9 @@
(consumer_state_mask & BufferHubDefs::kHighBitsMask);
}
}
- if (update_buffer_state && consumer->OnProducerPosted())
- pending_consumers_++;
+ if (update_buffer_state) {
+ consumer->OnProducerPosted();
+ }
return {status.take()};
}
@@ -424,13 +424,9 @@
// Signal any interested consumers. If there are none, the buffer will stay
// in posted state until a consumer comes online. This behavior guarantees
// that no frame is silently dropped.
- pending_consumers_ = 0;
for (auto consumer : consumer_channels_) {
- if (consumer->OnProducerPosted())
- pending_consumers_++;
+ consumer->OnProducerPosted();
}
- ALOGD_IF(TRACE, "ProducerChannel::OnProducerPost: %d pending consumers",
- pending_consumers_);
return {};
}
@@ -439,18 +435,6 @@
ATRACE_NAME("ProducerChannel::OnGain");
ALOGD_IF(TRACE, "ProducerChannel::OnGain: buffer_id=%d", buffer_id());
- // There are still pending consumers, return busy.
- if (pending_consumers_ > 0) {
- ALOGE(
- "ProducerChannel::OnGain: Producer (id=%d) is gaining a buffer that "
- "still has %d pending consumer(s).",
- buffer_id(), pending_consumers_);
- // TODO(b/77153033): add gain_posted_buffer to the impulse args, and
- // return busy if the function does not allow gaining posted buffer.
- // TODO(b/119331650): remove this if check if pending_consumers_ is removed.
- // return ErrorStatus(EBUSY);
- }
-
ClearAvailable();
post_fence_.close();
return {std::move(returned_fence_)};
@@ -547,48 +531,25 @@
}
}
- DecrementPendingConsumers();
- if (pending_consumers_ == 0 && orphaned_consumer_bit_mask_) {
- ALOGW(
- "%s: orphaned buffer detected during the this acquire/release cycle: "
- "id=%d orphaned=0x%" PRIx64 " queue_index=%" PRIx64 ".",
- __FUNCTION__, buffer_id(), orphaned_consumer_bit_mask_,
- metadata_header_->queue_index);
- orphaned_consumer_bit_mask_ = 0;
+ uint64_t current_buffer_state =
+ buffer_state_->load(std::memory_order_acquire);
+ if (BufferHubDefs::IsClientReleased(current_buffer_state,
+ ~orphaned_consumer_bit_mask_)) {
+ SignalAvailable();
+ if (orphaned_consumer_bit_mask_) {
+ ALOGW(
+ "%s: orphaned buffer detected during the this acquire/release cycle: "
+ "id=%d orphaned=0x%" PRIx64 " queue_index=%" PRIx64 ".",
+ __FUNCTION__, buffer_id(), orphaned_consumer_bit_mask_,
+ metadata_header_->queue_index);
+ orphaned_consumer_bit_mask_ = 0;
+ }
}
- ALOGE_IF(
- pending_consumers_ && BufferHubDefs::IsBufferReleased(
- buffer_state_->load(std::memory_order_acquire)),
- "ProducerChannel::OnConsumerRelease: buffer state inconsistent: "
- "pending_consumers=%d, buffer buffer is in releaed state.",
- pending_consumers_);
return {};
}
-void ProducerChannel::DecrementPendingConsumers() {
- if (pending_consumers_ == 0) {
- ALOGE("ProducerChannel::DecrementPendingConsumers: no pending consumer.");
- return;
- }
-
- --pending_consumers_;
- ALOGD_IF(TRACE, "%s: buffer_id=%d %d consumers left", __FUNCTION__,
- buffer_id(), pending_consumers_);
-
- if (pending_consumers_ == 0) {
- ALOGD_IF(TRACE,
- "%s: releasing last consumer: buffer_id=%d state=%" PRIx64 ".",
- __FUNCTION__, buffer_id(),
- buffer_state_->load(std::memory_order_acquire));
- SignalAvailable();
- }
-}
-
void ProducerChannel::OnConsumerOrphaned(const uint64_t& consumer_state_mask) {
- // Ignore the orphaned consumer.
- DecrementPendingConsumers();
-
// Remember the ignored consumer so that newly added consumer won't be
// taking the same state mask as this orphaned consumer.
ALOGE_IF(orphaned_consumer_bit_mask_ & consumer_state_mask,
@@ -597,6 +558,13 @@
__FUNCTION__, consumer_state_mask);
orphaned_consumer_bit_mask_ |= consumer_state_mask;
+ uint64_t current_buffer_state =
+ buffer_state_->load(std::memory_order_acquire);
+ if (BufferHubDefs::IsClientReleased(current_buffer_state,
+ ~orphaned_consumer_bit_mask_)) {
+ SignalAvailable();
+ }
+
// Atomically clear the fence state bit as an orphaned consumer will never
// signal a release fence.
fence_state_->fetch_and(~consumer_state_mask, std::memory_order_release);