From 1eb2ecfe4a34d5d411dfab1516e4e3ce2200a0fd Mon Sep 17 00:00:00 2001 From: ramindani Date: Thu, 7 Dec 2023 11:16:21 -0800 Subject: [PATCH 1/2] [HWC3] AIDL change to add refreshPeriodNanos to RefreshRateChangedDebugData Test: atest VtsHalGraphicsComposer3_TargetTest and device boots BUG: 314527560 Change-Id: Iaf961ae6ad118c5cd99b07ec133023297dac7040 --- .../composer3/RefreshRateChangedDebugData.aidl | 1 + .../composer3/RefreshRateChangedDebugData.aidl | 11 +++++++++++ 2 files changed, 12 insertions(+) diff --git a/graphics/composer/aidl/aidl_api/android.hardware.graphics.composer3/current/android/hardware/graphics/composer3/RefreshRateChangedDebugData.aidl b/graphics/composer/aidl/aidl_api/android.hardware.graphics.composer3/current/android/hardware/graphics/composer3/RefreshRateChangedDebugData.aidl index 2b9801a4aa..e9305e152c 100644 --- a/graphics/composer/aidl/aidl_api/android.hardware.graphics.composer3/current/android/hardware/graphics/composer3/RefreshRateChangedDebugData.aidl +++ b/graphics/composer/aidl/aidl_api/android.hardware.graphics.composer3/current/android/hardware/graphics/composer3/RefreshRateChangedDebugData.aidl @@ -36,4 +36,5 @@ package android.hardware.graphics.composer3; parcelable RefreshRateChangedDebugData { long display; int vsyncPeriodNanos; + int refreshPeriodNanos; } diff --git a/graphics/composer/aidl/android/hardware/graphics/composer3/RefreshRateChangedDebugData.aidl b/graphics/composer/aidl/android/hardware/graphics/composer3/RefreshRateChangedDebugData.aidl index c1f78d6f5e..11c0112dfd 100644 --- a/graphics/composer/aidl/android/hardware/graphics/composer3/RefreshRateChangedDebugData.aidl +++ b/graphics/composer/aidl/android/hardware/graphics/composer3/RefreshRateChangedDebugData.aidl @@ -27,4 +27,15 @@ parcelable RefreshRateChangedDebugData { * The display vsync period in nanoseconds. */ int vsyncPeriodNanos; + + /** + * The refresh period of the display in nanoseconds. + * On VRR (Variable Refresh Rate) displays, refreshPeriodNanos can be different from the + * vsyncPeriodNanos because not every vsync cycle of the display is a refresh cycle. + * This should be set to the current refresh period. + * On non-VRR displays this value should be equal to vsyncPeriodNanos + * + * @see vsyncPeriodNanos + */ + int refreshPeriodNanos; } From a2a6deaf5036e081f48379b6573db4465538b5ac Mon Sep 17 00:00:00 2001 From: ramindani Date: Mon, 11 Dec 2023 13:42:36 -0800 Subject: [PATCH 2/2] [Composer3-VTS] Test Composer3-V3 for refreshPeriodNanos and vsyncPeriodNanos Test: atest VtsHalGraphicsComposer3_TargetTest BUG: 314527560 Change-Id: If45c0d8c4b61b8c46c1e4336fe261d11414535e5 --- .../composer/aidl/vts/VtsComposerClient.cpp | 3 +- .../composer/aidl/vts/VtsComposerClient.h | 8 ++- .../VtsHalGraphicsComposer3_TargetTest.cpp | 49 ++++++++++++------- 3 files changed, 39 insertions(+), 21 deletions(-) diff --git a/graphics/composer/aidl/vts/VtsComposerClient.cpp b/graphics/composer/aidl/vts/VtsComposerClient.cpp index 11b995e2e2..ac08cd151f 100644 --- a/graphics/composer/aidl/vts/VtsComposerClient.cpp +++ b/graphics/composer/aidl/vts/VtsComposerClient.cpp @@ -517,7 +517,8 @@ std::pair> VtsComposerClient::getDisplays void VtsComposerClient::addDisplayConfigs(VtsDisplay* vtsDisplay, const std::vector& configs) { for (const auto& config : configs) { - vtsDisplay->addDisplayConfig(config.configId, {config.vsyncPeriod, config.configGroup}); + vtsDisplay->addDisplayConfig(config.configId, + {config.vsyncPeriod, config.configGroup, config.vrrConfig}); } } diff --git a/graphics/composer/aidl/vts/VtsComposerClient.h b/graphics/composer/aidl/vts/VtsComposerClient.h index b45c71f80b..292bc407f8 100644 --- a/graphics/composer/aidl/vts/VtsComposerClient.h +++ b/graphics/composer/aidl/vts/VtsComposerClient.h @@ -253,10 +253,14 @@ class VtsDisplay { int32_t getDisplayHeight() const { return mDisplayHeight; } struct DisplayConfig { - DisplayConfig(int32_t vsyncPeriod_, int32_t configGroup_) - : vsyncPeriod(vsyncPeriod_), configGroup(configGroup_) {} + DisplayConfig(int32_t vsyncPeriod_, int32_t configGroup_, + std::optional vrrConfig_ = {}) + : vsyncPeriod(vsyncPeriod_), + configGroup(configGroup_), + vrrConfig(std::move(vrrConfig_)) {} int32_t vsyncPeriod; int32_t configGroup; + std::optional vrrConfig; }; void addDisplayConfig(int32_t config, DisplayConfig displayConfig) { diff --git a/graphics/composer/aidl/vts/VtsHalGraphicsComposer3_TargetTest.cpp b/graphics/composer/aidl/vts/VtsHalGraphicsComposer3_TargetTest.cpp index c135298560..7fcf2e4cec 100644 --- a/graphics/composer/aidl/vts/VtsHalGraphicsComposer3_TargetTest.cpp +++ b/graphics/composer/aidl/vts/VtsHalGraphicsComposer3_TargetTest.cpp @@ -2664,26 +2664,40 @@ TEST_P(GraphicsComposerAidlCommandV2Test, SetRefreshRateChangedCallbackDebug_Ena return; } - const auto displayId = getPrimaryDisplayId(); - EXPECT_TRUE(mComposerClient->setPowerMode(displayId, PowerMode::ON).isOk()); - // Enable the callback - ASSERT_TRUE(mComposerClient - ->setRefreshRateChangedCallbackDebugEnabled(displayId, - /*enabled*/ true) - .isOk()); - std::this_thread::sleep_for(100ms); + for (VtsDisplay& display : mDisplays) { + const auto displayId = display.getDisplayId(); + EXPECT_TRUE(mComposerClient->setPowerMode(displayId, PowerMode::ON).isOk()); + // Enable the callback + ASSERT_TRUE(mComposerClient + ->setRefreshRateChangedCallbackDebugEnabled(displayId, + /*enabled*/ true) + .isOk()); + std::this_thread::sleep_for(100ms); - const auto displayFilter = [displayId](auto refreshRateChangedDebugData) { - return displayId == refreshRateChangedDebugData.display; - }; + const auto [status, configId] = mComposerClient->getActiveConfig(display.getDisplayId()); + EXPECT_TRUE(status.isOk()); - // Check that we immediately got a callback - EXPECT_TRUE(checkIfCallbackRefreshRateChangedDebugEnabledReceived(displayFilter)); + const auto displayFilter = [&](auto refreshRateChangedDebugData) { + bool nonVrrRateMatching = true; + if (std::optional vrrConfigOpt = + display.getDisplayConfig(configId).vrrConfig; + getInterfaceVersion() >= 3 && !vrrConfigOpt) { + nonVrrRateMatching = refreshRateChangedDebugData.refreshPeriodNanos == + refreshRateChangedDebugData.vsyncPeriodNanos; + } + const bool isDisplaySame = + display.getDisplayId() == refreshRateChangedDebugData.display; + return nonVrrRateMatching && isDisplaySame; + }; - ASSERT_TRUE(mComposerClient - ->setRefreshRateChangedCallbackDebugEnabled(displayId, - /*enabled*/ false) - .isOk()); + // Check that we immediately got a callback + EXPECT_TRUE(checkIfCallbackRefreshRateChangedDebugEnabledReceived(displayFilter)); + + ASSERT_TRUE(mComposerClient + ->setRefreshRateChangedCallbackDebugEnabled(displayId, + /*enabled*/ false) + .isOk()); + } } TEST_P(GraphicsComposerAidlCommandV2Test, @@ -2938,7 +2952,6 @@ INSTANTIATE_TEST_SUITE_P( PerInstance, GraphicsComposerAidlCommandV2Test, testing::ValuesIn(::android::getAidlHalInstanceNames(IComposer::descriptor)), ::android::PrintInstanceNameToString); - } // namespace aidl::android::hardware::graphics::composer3::vts int main(int argc, char** argv) {