diff options
8 files changed, 63 insertions, 17 deletions
diff --git a/services/vr/hardware_composer/impl/vr_composer_client.cpp b/services/vr/hardware_composer/impl/vr_composer_client.cpp index c31417bcc7..abe571ae9e 100644 --- a/services/vr/hardware_composer/impl/vr_composer_client.cpp +++ b/services/vr/hardware_composer/impl/vr_composer_client.cpp @@ -161,6 +161,10 @@ void VrComposerClient::onHotplug(Display display, client_->onHotplug(display, connected); } +void VrComposerClient::onRefresh(Display display) { + client_->onRefresh(display); +} + Return<void> VrComposerClient::registerCallback( const sp<IComposerCallback>& callback) { return client_->registerCallback(callback); diff --git a/services/vr/hardware_composer/impl/vr_composer_client.h b/services/vr/hardware_composer/impl/vr_composer_client.h index f492230215..dfc656a0f1 100644 --- a/services/vr/hardware_composer/impl/vr_composer_client.h +++ b/services/vr/hardware_composer/impl/vr_composer_client.h @@ -35,6 +35,7 @@ class VrComposerClient : public IVrComposerClient { virtual ~VrComposerClient(); void onHotplug(Display display, IComposerCallback::Connection connected); + void onRefresh(Display display); // IComposerClient Return<void> registerCallback(const sp<IComposerCallback>& callback) override; diff --git a/services/vr/hardware_composer/impl/vr_hwc.cpp b/services/vr/hardware_composer/impl/vr_hwc.cpp index 9f069cdf6a..5e32af0bea 100644 --- a/services/vr/hardware_composer/impl/vr_hwc.cpp +++ b/services/vr/hardware_composer/impl/vr_hwc.cpp @@ -851,6 +851,14 @@ Return<void> VrHwc::createClient(createClient_cb hidl_cb) { return Void(); } +void VrHwc::ForceDisplaysRefresh() { + std::lock_guard<std::mutex> guard(mutex_); + if (client_ != nullptr) { + for (const auto& pair : displays_) + client_.promote()->onRefresh(pair.first); + } +} + void VrHwc::RegisterObserver(Observer* observer) { std::lock_guard<std::mutex> guard(mutex_); if (observer_) diff --git a/services/vr/hardware_composer/impl/vr_hwc.h b/services/vr/hardware_composer/impl/vr_hwc.h index 523cda3f69..fce9a063e0 100644 --- a/services/vr/hardware_composer/impl/vr_hwc.h +++ b/services/vr/hardware_composer/impl/vr_hwc.h @@ -103,6 +103,7 @@ class ComposerView { virtual ~ComposerView() {} + virtual void ForceDisplaysRefresh() = 0; virtual void RegisterObserver(Observer* observer) = 0; virtual void UnregisterObserver(Observer* observer) = 0; }; @@ -288,6 +289,7 @@ class VrHwc : public IComposer, public ComposerBase, public ComposerView { Return<void> createClient(createClient_cb hidl_cb) override; // ComposerView: + void ForceDisplaysRefresh() override; void RegisterObserver(Observer* observer) override; void UnregisterObserver(Observer* observer) override; @@ -295,7 +297,6 @@ class VrHwc : public IComposer, public ComposerBase, public ComposerView { HwcDisplay* FindDisplay(Display display); wp<VrComposerClient> client_; - sp<IComposerCallback> callbacks_; // Guard access to internal state from binder threads. std::mutex mutex_; diff --git a/services/vr/hardware_composer/tests/vr_composer_test.cpp b/services/vr/hardware_composer/tests/vr_composer_test.cpp index d082f4bc92..2e70928662 100644 --- a/services/vr/hardware_composer/tests/vr_composer_test.cpp +++ b/services/vr/hardware_composer/tests/vr_composer_test.cpp @@ -10,6 +10,24 @@ namespace { const char kVrDisplayName[] = "VrDisplay_Test"; +class TestComposerView : public ComposerView { + public: + TestComposerView() {} + ~TestComposerView() override = default; + + size_t display_refresh_count() const { return display_refresh_count_; } + + void ForceDisplaysRefresh() override { display_refresh_count_++; } + void RegisterObserver(Observer* observer) override {} + void UnregisterObserver(Observer* observer) override {} + + TestComposerView(const TestComposerView&) = delete; + void operator=(const TestComposerView&) = delete; + + private: + size_t display_refresh_count_ = 0; +}; + class TestComposerCallback : public BnVrComposerCallback { public: TestComposerCallback() {} @@ -57,7 +75,7 @@ sp<GraphicBuffer> CreateBuffer() { class VrComposerTest : public testing::Test { public: - VrComposerTest() : composer_(new VrComposer()) {} + VrComposerTest() : composer_(new VrComposer(&composer_view_)) {} ~VrComposerTest() override = default; sp<IVrComposer> GetComposerProxy() const { @@ -72,6 +90,7 @@ class VrComposerTest : public testing::Test { } protected: + TestComposerView composer_view_; sp<VrComposer> composer_; VrComposerTest(const VrComposerTest&) = delete; @@ -89,7 +108,9 @@ TEST_F(VrComposerTest, TestWithoutObserver) { TEST_F(VrComposerTest, TestWithObserver) { sp<IVrComposer> composer = GetComposerProxy(); sp<TestComposerCallback> callback = new TestComposerCallback(); + ASSERT_EQ(0, composer_view_.display_refresh_count()); ASSERT_TRUE(composer->registerObserver(callback).isOk()); + ASSERT_EQ(1, composer_view_.display_refresh_count()); ComposerView::Frame frame; base::unique_fd fence = composer_->OnNewFrame(frame); diff --git a/services/vr/hardware_composer/vr_composer.cpp b/services/vr/hardware_composer/vr_composer.cpp index 36a313ae24..d93f370945 100644 --- a/services/vr/hardware_composer/vr_composer.cpp +++ b/services/vr/hardware_composer/vr_composer.cpp @@ -21,24 +21,36 @@ bool CheckPermission() { } // namespace -VrComposer::VrComposer() {} +VrComposer::VrComposer(ComposerView* composer_view) + : composer_view_(composer_view) { + composer_view_->RegisterObserver(this); +} -VrComposer::~VrComposer() {} +VrComposer::~VrComposer() { + composer_view_->UnregisterObserver(this); +} binder::Status VrComposer::registerObserver( const sp<IVrComposerCallback>& callback) { - std::lock_guard<std::mutex> guard(mutex_); + { + std::lock_guard<std::mutex> guard(mutex_); + + if (!CheckPermission()) + return binder::Status::fromStatusT(PERMISSION_DENIED); - if (!CheckPermission()) - return binder::Status::fromStatusT(PERMISSION_DENIED); + if (callback_.get()) { + ALOGE("Failed to register callback, already registered"); + return binder::Status::fromStatusT(ALREADY_EXISTS); + } - if (callback_.get()) { - ALOGE("Failed to register callback, already registered"); - return binder::Status::fromStatusT(ALREADY_EXISTS); + callback_ = callback; + IInterface::asBinder(callback_)->linkToDeath(this); } - callback_ = callback; - IInterface::asBinder(callback_)->linkToDeath(this); + // Don't take the lock to force display refresh otherwise it could end in a + // deadlock since HWC calls this with new frames and it has a lock of its own + // to serialize access to the display information. + composer_view_->ForceDisplaysRefresh(); return binder::Status::ok(); } diff --git a/services/vr/hardware_composer/vr_composer.h b/services/vr/hardware_composer/vr_composer.h index 7b580c6cba..1273352ad0 100644 --- a/services/vr/hardware_composer/vr_composer.h +++ b/services/vr/hardware_composer/vr_composer.h @@ -20,7 +20,7 @@ class VrComposer public ComposerView::Observer, public IBinder::DeathRecipient { public: - VrComposer(); + explicit VrComposer(ComposerView* composer_view); ~VrComposer() override; // BnVrComposer: @@ -40,6 +40,8 @@ class VrComposer sp<IVrComposerCallback> callback_; + ComposerView* composer_view_; // Not owned. + VrComposer(const VrComposer&) = delete; void operator=(const VrComposer&) = delete; }; diff --git a/services/vr/hardware_composer/vr_hardware_composer_service.cpp b/services/vr/hardware_composer/vr_hardware_composer_service.cpp index e36b0ae3f1..7701847120 100644 --- a/services/vr/hardware_composer/vr_hardware_composer_service.cpp +++ b/services/vr/hardware_composer/vr_hardware_composer_service.cpp @@ -36,8 +36,7 @@ int main() { "Failed to register service"); android::sp<android::dvr::VrComposer> composer = - new android::dvr::VrComposer(); - service->RegisterObserver(composer.get()); + new android::dvr::VrComposer(service.get()); android::sp<android::IServiceManager> sm(android::defaultServiceManager()); @@ -52,7 +51,5 @@ int main() { android::hardware::ProcessState::self()->startThreadPool(); android::hardware::IPCThreadState::self()->joinThreadPool(); - service->UnregisterObserver(composer.get()); - return 0; } |