From a1df7c3d1f3aa2cad2439222c5da8163c1f486d9 Mon Sep 17 00:00:00 2001 From: Ady Abraham Date: Tue, 22 Oct 2019 16:56:08 -0700 Subject: [PATCH] composer: few changes for refresh rate switching - getDisplayConfigs is required to remain backward compatible with composer@2.3 and older versions. To keep it that way a new config attribute was added to group similar configs. - getSupportedDisplayVsyncPeriods was removed as the vsync period can be obtained by getDisplayAttribute for each individual config. - renamed setActiveConfigAndVsyncPeriod -> setActiveConfigWithConstraints to better describe what this function does. - added VsyncPeriodChangeTimeline and onVsyncPeriodTimingChanged With these changes composer is expected to return all possible configurations when getDisplayConfigs (similar to older composer versions). The client knows whether a seamless vsync period is possible or not by the config groups. Test: rev up composer to 2.4 and test refresh rate switching Bug: 141329414 Change-Id: I1b4525f7e00b62cdaf260274abe4a6a5962ed1ab --- graphics/composer/2.4/IComposerCallback.hal | 10 + graphics/composer/2.4/IComposerClient.hal | 54 ++-- graphics/composer/2.4/types.hal | 27 +- .../include/composer-hal/2.4/ComposerClient.h | 35 +-- .../include/composer-hal/2.4/ComposerHal.h | 13 +- .../include/composer-passthrough/2.4/HwcHal.h | 102 ++++---- .../composer/2.4/utils/vts/ComposerVts.cpp | 36 +-- .../include/composer-vts/2.4/ComposerVts.h | 10 +- .../VtsHalGraphicsComposerV2_4TargetTest.cpp | 233 ++++++++++-------- 9 files changed, 308 insertions(+), 212 deletions(-) diff --git a/graphics/composer/2.4/IComposerCallback.hal b/graphics/composer/2.4/IComposerCallback.hal index 5c3f8bd99c..fea24a1cbf 100644 --- a/graphics/composer/2.4/IComposerCallback.hal +++ b/graphics/composer/2.4/IComposerCallback.hal @@ -32,4 +32,14 @@ interface IComposerCallback extends @2.1::IComposerCallback { * is expected to be called vsyncPeriodNanos nanoseconds after this call. */ oneway onVsync_2_4(Display display, int64_t timestamp, VsyncPeriodNanos vsyncPeriodNanos); + + /** + * Notifies the client that the previously reported timing for vsync period change has been + * updated. This may occur if the composer missed the deadline for changing the vsync period + * or the client submitted a refresh frame too late. + * + * @param display is the display which vsync period change is in progress + * @param updatedTimeline is the new timeline for the vsync period change. + */ + oneway onVsyncPeriodTimingChanged(Display display, VsyncPeriodChangeTimeline updatedTimeline); }; diff --git a/graphics/composer/2.4/IComposerClient.hal b/graphics/composer/2.4/IComposerClient.hal index c2102d5f53..f23536cd2d 100644 --- a/graphics/composer/2.4/IComposerClient.hal +++ b/graphics/composer/2.4/IComposerClient.hal @@ -20,9 +20,22 @@ import IComposerCallback; import @2.1::Config; import @2.1::Display; import @2.1::Error; +import @2.1::IComposerClient; import @2.3::IComposerClient; interface IComposerClient extends @2.3::IComposerClient { + /** + * Display attributes queryable through getDisplayAttribute_2_4. + */ + enum Attribute : @2.1::IComposerClient.Attribute { + /** + * The configuration group ID (as int32_t) this config is associated to. + * Switching between configurations within the same group may be done seamlessly + * in some conditions via setActiveConfigWithConstraints. + */ + CONFIG_GROUP = 7, + }; + /** * Required capabilities which are supported by the display. The * particular set of supported capabilities for a given display may be @@ -60,6 +73,7 @@ interface IComposerClient extends @2.3::IComposerClient { * (i.e., the vsync period must not change before this time). */ int64_t desiredTimeNanos; + /** * If true, requires that the vsync period change must happen seamlessly without * a noticeable visual artifact. @@ -74,7 +88,6 @@ interface IComposerClient extends @2.3::IComposerClient { * * @param callback is the IComposerCallback object. */ - @entry registerCallback_2_4(IComposerCallback callback); /** @@ -99,25 +112,29 @@ interface IComposerClient extends @2.3::IComposerClient { getDisplayConnectionType(Display display) generates (Error error, DisplayConnectionType type); /** - * Provides a list of the vsync periods supported by the display in the given configuration - * - * @param display is the display for which the vsync periods are queried. - * @param config is the display configuration for which the vsync periods are queried. + * Returns a display attribute value for a particular display + * configuration. * + * @param display is the display to query. + * @param config is the display configuration for which to return + * attribute values. * @return error is NONE upon success. Otherwise, - * BAD_DISPLAY when an invalid display handle was passed in. - * BAD_CONFIG when an invalid config handle was passed in. - * @return supportedVsyncPeriods is a list of supported vsync periods. + * BAD_DISPLAY when an invalid display handle was passed in. + * BAD_CONFIG when config does not name a valid configuration for + * this display. + * BAD_PARAMETER when attribute is unrecognized. + * UNSUPPORTED when attribute cannot be queried for the config. + * @return value is the value of the attribute. */ - getSupportedDisplayVsyncPeriods(Display display, Config config) - generates (Error error, vec supportedVsyncPeriods); + getDisplayAttribute_2_4(Display display, Config config, Attribute attribute) + generates (Error error, int32_t value); /** * Retrieves which vsync period the display is currently using. * * If no display configuration is currently active, this function must * return BAD_CONFIG. If the vsync period is about to change due to a - * setActiveConfigAndVsyncPeriod call, this function must return the current vsync period + * setActiveConfigWithConstraints call, this function must return the current vsync period * until the change takes place. * * @param display is the display for which the vsync period is queried. @@ -131,7 +148,8 @@ interface IComposerClient extends @2.3::IComposerClient { /** * Sets the active configuration and the refresh rate for this display. - * If the config is the same as the current config, only the vsync period shall change. + * If the new config shares the same config group as the current config, + * only the vsync period shall change. * Upon returning, the given display configuration, except vsync period, must be active and * remain so until either this function is called again or the display is disconnected. * When the display starts to refresh at the new vsync period, onVsync_2_4 callback must be @@ -139,21 +157,19 @@ interface IComposerClient extends @2.3::IComposerClient { * * @param display is the display for which the active config is set. * @param config is the new display configuration. - * @param vsyncPeriodNanos is the new display vsync period. * @param vsyncPeriodChangeConstraints are the constraints required for changing vsync period. * * @return error is NONE upon success. Otherwise, * BAD_DISPLAY when an invalid display handle was passed in. * BAD_CONFIG when the configuration handle passed in is not valid * for this display. - * BAD_VSYNC_PERIOD when an invalid vsync period is passed in. + * SEAMLESS_NOT_ALLOWED when seamlessRequired was true but config provided doesn't + * share the same config group as the current config. * SEAMLESS_NOT_POSSIBLE when seamlessRequired was true but the display cannot achieve * the vsync period change without a noticeable visual artifact. - * @return newVsyncAppliedTime is the time in CLOCK_MONOTONIC when the new display will start to - * refresh at the new vsync period. + * @return timeline is the timeline for the vsync period change. */ - setActiveConfigAndVsyncPeriod(Display display, Config config, - VsyncPeriodNanos vsyncPeriodNanos, + setActiveConfigWithConstraints(Display display, Config config, VsyncPeriodChangeConstraints vsyncPeriodChangeConstraints) - generates (Error error, int64_t newVsyncAppliedTime); + generates (Error error, VsyncPeriodChangeTimeline timeline); }; diff --git a/graphics/composer/2.4/types.hal b/graphics/composer/2.4/types.hal index b45d7a69f7..065f024551 100644 --- a/graphics/composer/2.4/types.hal +++ b/graphics/composer/2.4/types.hal @@ -20,13 +20,36 @@ import @2.1::Error; enum Error : @2.1::Error { /** - * Invalid vsync period + * Seamless cannot be required for configurations that don't share a config group */ - BAD_VSYNC_PERIOD = 9, + SEAMLESS_NOT_ALLOWED = 9, /** * Seamless requirements cannot be met */ SEAMLESS_NOT_POSSIBLE = 10, }; +/** + * Timing for a vsync period change. + */ +struct VsyncPeriodChangeTimeline { + /** + * The time in CLOCK_MONOTONIC when the new display will start to refresh at + * the new vsync period. + */ + int64_t newVsyncAppliedTimeNanos; + + /** + * Set to true if the client is required to send a frame to be displayed before + * the change can take place. + */ + bool refreshRequired; + + /** + * The time in CLOCK_MONOTONIC when the client is expected to send the new frame. + * Should be ignored if refreshRequired is false. + */ + int64_t refreshTimeNanos; +}; + typedef uint32_t VsyncPeriodNanos; diff --git a/graphics/composer/2.4/utils/hal/include/composer-hal/2.4/ComposerClient.h b/graphics/composer/2.4/utils/hal/include/composer-hal/2.4/ComposerClient.h index ddf209bee3..4160ed97bf 100644 --- a/graphics/composer/2.4/utils/hal/include/composer-hal/2.4/ComposerClient.h +++ b/graphics/composer/2.4/utils/hal/include/composer-hal/2.4/ComposerClient.h @@ -76,6 +76,13 @@ class ComposerClientImpl : public V2_3::hal::detail::ComposerClientImplonVsyncPeriodTimingChanged(display, updatedTimeline); + ALOGE_IF(!ret.isOk(), "failed to send onVsyncPeriodTimingChanged: %s", + ret.description().c_str()); + } + protected: const sp mCallback; V2_1::hal::ComposerResources* const mResources; @@ -104,13 +111,12 @@ class ComposerClientImpl : public V2_3::hal::detail::ComposerClientImpl getSupportedDisplayVsyncPeriods( - Display display, Config config, - IComposerClient::getSupportedDisplayVsyncPeriods_cb hidl_cb) override { - std::vector supportedVsyncPeriods; - Error error = - mHal->getSupportedDisplayVsyncPeriods(display, config, &supportedVsyncPeriods); - hidl_cb(error, supportedVsyncPeriods); + Return getDisplayAttribute_2_4( + Display display, Config config, IComposerClient::Attribute attribute, + IComposerClient::getDisplayAttribute_2_4_cb hidl_cb) override { + int32_t value = 0; + Error error = mHal->getDisplayAttribute_2_4(display, config, attribute, &value); + hidl_cb(error, value); return Void(); } @@ -122,15 +128,14 @@ class ComposerClientImpl : public V2_3::hal::detail::ComposerClientImpl setActiveConfigAndVsyncPeriod( - Display display, Config config, VsyncPeriodNanos vsyncPeriodNanos, + Return setActiveConfigWithConstraints( + Display display, Config config, const IComposerClient::VsyncPeriodChangeConstraints& vsyncPeriodChangeConstraints, - IComposerClient::setActiveConfigAndVsyncPeriod_cb hidl_cb) override { - int64_t newVsyncAppliedTime = 0; - Error error = mHal->setActiveConfigAndVsyncPeriod(display, config, vsyncPeriodNanos, - vsyncPeriodChangeConstraints, - &newVsyncAppliedTime); - hidl_cb(error, newVsyncAppliedTime); + IComposerClient::setActiveConfigWithConstraints_cb hidl_cb) override { + VsyncPeriodChangeTimeline timeline = {}; + Error error = mHal->setActiveConfigWithConstraints(display, config, + vsyncPeriodChangeConstraints, &timeline); + hidl_cb(error, timeline); return Void(); } diff --git a/graphics/composer/2.4/utils/hal/include/composer-hal/2.4/ComposerHal.h b/graphics/composer/2.4/utils/hal/include/composer-hal/2.4/ComposerHal.h index 0739f625ab..89dbe66d60 100644 --- a/graphics/composer/2.4/utils/hal/include/composer-hal/2.4/ComposerHal.h +++ b/graphics/composer/2.4/utils/hal/include/composer-hal/2.4/ComposerHal.h @@ -47,6 +47,8 @@ class ComposerHal : public V2_3::hal::ComposerHal { virtual void onVsync(Display display, int64_t timestamp) = 0; virtual void onVsync_2_4(Display display, int64_t timestamp, VsyncPeriodNanos vsyncPeriodNanos) = 0; + virtual void onVsyncPeriodTimingChanged(Display display, + const VsyncPeriodChangeTimeline& timeline) = 0; }; virtual void registerEventCallback_2_4(EventCallback_2_4* callback) = 0; @@ -57,13 +59,14 @@ class ComposerHal : public V2_3::hal::ComposerHal { Display display, std::vector* outCapabilities) = 0; virtual Error getDisplayConnectionType(Display display, IComposerClient::DisplayConnectionType* outType) = 0; - virtual Error getSupportedDisplayVsyncPeriods( - Display display, Config config, std::vector* outVsyncPeriod) = 0; + virtual Error getDisplayAttribute_2_4(Display display, Config config, + IComposerClient::Attribute attribute, + int32_t* outValue) = 0; virtual Error getDisplayVsyncPeriod(Display display, VsyncPeriodNanos* outVsyncPeriod) = 0; - virtual Error setActiveConfigAndVsyncPeriod( - Display display, Config config, VsyncPeriodNanos vsyncPeriodNanos, + virtual Error setActiveConfigWithConstraints( + Display display, Config config, const IComposerClient::VsyncPeriodChangeConstraints& vsyncPeriodChangeConstraints, - int64_t* outNewVsyncAppliedTime) = 0; + VsyncPeriodChangeTimeline* timeline) = 0; }; } // namespace hal diff --git a/graphics/composer/2.4/utils/passthrough/include/composer-passthrough/2.4/HwcHal.h b/graphics/composer/2.4/utils/passthrough/include/composer-passthrough/2.4/HwcHal.h index 3420c8cc07..d59d0d5361 100644 --- a/graphics/composer/2.4/utils/passthrough/include/composer-passthrough/2.4/HwcHal.h +++ b/graphics/composer/2.4/utils/passthrough/include/composer-passthrough/2.4/HwcHal.h @@ -25,6 +25,7 @@ #include #include #include +#include namespace android { namespace hardware { @@ -51,14 +52,22 @@ class HwcHalImpl : public V2_3::passthrough::detail::HwcHalImpl { void registerEventCallback_2_4(hal::ComposerHal::EventCallback_2_4* callback) override { mEventCallback_2_4 = callback; - mDispatch.registerCallback(mDevice, HWC2_CALLBACK_HOTPLUG, this, - reinterpret_cast(hotplugHook)); - mDispatch.registerCallback(mDevice, HWC2_CALLBACK_REFRESH, this, - reinterpret_cast(refreshHook)); - mDispatch.registerCallback(mDevice, HWC2_CALLBACK_VSYNC, this, - reinterpret_cast(vsyncHook)); - mDispatch.registerCallback(mDevice, HWC2_CALLBACK_VSYNC_2_4, this, - reinterpret_cast(vsync_2_4_Hook)); + BaseType2_1::mDispatch.registerCallback( + mDevice, HWC2_CALLBACK_HOTPLUG, this, + reinterpret_cast(hotplugHook)); + BaseType2_1::mDispatch.registerCallback( + mDevice, HWC2_CALLBACK_REFRESH, this, + reinterpret_cast(refreshHook)); + BaseType2_1::mDispatch.registerCallback( + mDevice, HWC2_CALLBACK_VSYNC, this, + reinterpret_cast(vsyncHook)); + BaseType2_1::mDispatch.registerCallback( + mDevice, HWC2_CALLBACK_VSYNC_2_4, this, + reinterpret_cast(vsync_2_4_Hook)); + + BaseType2_1::mDispatch.registerCallback( + mDevice, HWC2_CALLBACK_VSYNC_PERIOD_TIMING_CHANGED, this, + reinterpret_cast(vsyncPeriodTimingChangedHook)); } void unregisterEventCallback_2_4() override { @@ -69,10 +78,12 @@ class HwcHalImpl : public V2_3::passthrough::detail::HwcHalImpl { // - will never be called afterward // // which is likely incorrect - mDispatch.registerCallback(mDevice, HWC2_CALLBACK_HOTPLUG, this, nullptr); - mDispatch.registerCallback(mDevice, HWC2_CALLBACK_REFRESH, this, nullptr); - mDispatch.registerCallback(mDevice, HWC2_CALLBACK_VSYNC, this, nullptr); - mDispatch.registerCallback(mDevice, HWC2_CALLBACK_VSYNC_2_4, this, nullptr); + BaseType2_1::mDispatch.registerCallback(mDevice, HWC2_CALLBACK_HOTPLUG, this, nullptr); + BaseType2_1::mDispatch.registerCallback(mDevice, HWC2_CALLBACK_REFRESH, this, nullptr); + BaseType2_1::mDispatch.registerCallback(mDevice, HWC2_CALLBACK_VSYNC, this, nullptr); + BaseType2_1::mDispatch.registerCallback(mDevice, HWC2_CALLBACK_VSYNC_2_4, this, nullptr); + BaseType2_1::mDispatch.registerCallback(mDevice, HWC2_CALLBACK_VSYNC_PERIOD_TIMING_CHANGED, + this, nullptr); mEventCallback_2_4 = nullptr; } @@ -106,26 +117,16 @@ class HwcHalImpl : public V2_3::passthrough::detail::HwcHalImpl { return static_cast(error); } - Error getSupportedDisplayVsyncPeriods(Display display, Config config, - std::vector* outVsyncPeriods) override { - if (!mDispatch.getSupportedDisplayVsyncPeriods) { - return Error::UNSUPPORTED; + Error getDisplayAttribute_2_4(Display display, Config config, + IComposerClient::Attribute attribute, + int32_t* outValue) override { + int32_t err = BaseType2_1::mDispatch.getDisplayAttribute( + mDevice, display, config, static_cast(attribute), outValue); + if (err != HWC2_ERROR_NONE && *outValue == -1) { + // Convert the error from hwcomposer2 to IComposerClient definition + return Error::BAD_PARAMETER; } - - uint32_t count = 0; - int32_t error = mDispatch.getSupportedDisplayVsyncPeriods(mDevice, display, config, &count, - nullptr); - if (error != HWC2_ERROR_NONE) { - return static_cast(error); - } - outVsyncPeriods->resize(count); - error = mDispatch.getSupportedDisplayVsyncPeriods(mDevice, display, config, &count, - outVsyncPeriods->data()); - if (error != HWC2_ERROR_NONE) { - *outVsyncPeriods = std::vector(); - return static_cast(error); - } - return Error::NONE; + return static_cast(err); } Error getDisplayVsyncPeriod(Display display, VsyncPeriodNanos* outVsyncPeriod) override { @@ -140,11 +141,11 @@ class HwcHalImpl : public V2_3::passthrough::detail::HwcHalImpl { return Error::NONE; } - Error setActiveConfigAndVsyncPeriod( - Display display, Config config, VsyncPeriodNanos vsyncPeriodNanos, + Error setActiveConfigWithConstraints( + Display display, Config config, const IComposerClient::VsyncPeriodChangeConstraints& vsyncPeriodChangeConstraints, - int64_t* outNewVsyncAppliedTime) override { - if (!mDispatch.setActiveConfigAndVsyncPeriod) { + VsyncPeriodChangeTimeline* timeline) override { + if (!mDispatch.setActiveConfigWithConstraints) { return Error::UNSUPPORTED; } @@ -154,12 +155,15 @@ class HwcHalImpl : public V2_3::passthrough::detail::HwcHalImpl { vsync_period_change_constraints.seamlessRequired = vsyncPeriodChangeConstraints.seamlessRequired; - int32_t error = mDispatch.setActiveConfigAndVsyncPeriod( - mDevice, display, config, vsyncPeriodNanos, &vsync_period_change_constraints, - outNewVsyncAppliedTime); + hwc_vsync_period_change_timeline_t out_timeline; + int32_t error = mDispatch.setActiveConfigWithConstraints( + mDevice, display, config, &vsync_period_change_constraints, &out_timeline); if (error != HWC2_ERROR_NONE) { return static_cast(error); } + timeline->newVsyncAppliedTimeNanos = out_timeline.newVsyncAppliedTimeNanos; + timeline->refreshRequired = out_timeline.refreshRequired; + timeline->refreshTimeNanos = out_timeline.refreshTimeNanos; return Error::NONE; } @@ -171,13 +175,10 @@ class HwcHalImpl : public V2_3::passthrough::detail::HwcHalImpl { this->initOptionalDispatch(HWC2_FUNCTION_GET_DISPLAY_CONNECTION_TYPE, &mDispatch.getDisplayConnectionType); - this->initOptionalDispatch(HWC2_FUNCTION_REGISTER_CALLBACK, &mDispatch.registerCallback); - this->initOptionalDispatch(HWC2_FUNCTION_GET_SUPPORTED_DISPLAY_VSYNC_PERIODS, - &mDispatch.getSupportedDisplayVsyncPeriods); this->initOptionalDispatch(HWC2_FUNCTION_GET_DISPLAY_VSYNC_PERIOD, &mDispatch.getDisplayVsyncPeriod); - this->initOptionalDispatch(HWC2_FUNCTION_SET_ACTIVE_CONFIG_AND_VSYNC_PERIOD, - &mDispatch.setActiveConfigAndVsyncPeriod); + this->initOptionalDispatch(HWC2_FUNCTION_SET_ACTIVE_CONFIG_WITH_CONSTRAINTS, + &mDispatch.setActiveConfigWithConstraints); return true; } @@ -205,13 +206,22 @@ class HwcHalImpl : public V2_3::passthrough::detail::HwcHalImpl { hal->mEventCallback_2_4->onVsync_2_4(display, timestamp, vsyncPeriodNanos); } + static void vsyncPeriodTimingChangedHook(hwc2_callback_data_t callbackData, + hwc2_display_t display, + hwc_vsync_period_change_timeline_t* updated_timeline) { + auto hal = static_cast(callbackData); + VsyncPeriodChangeTimeline timeline; + timeline.newVsyncAppliedTimeNanos = updated_timeline->newVsyncAppliedTimeNanos; + timeline.refreshRequired = updated_timeline->refreshRequired; + timeline.refreshTimeNanos = updated_timeline->refreshTimeNanos; + hal->mEventCallback_2_4->onVsyncPeriodTimingChanged(display, timeline); + } + private: struct { HWC2_PFN_GET_DISPLAY_CONNECTION_TYPE getDisplayConnectionType; - HWC2_PFN_REGISTER_CALLBACK registerCallback; - HWC2_PFN_GET_SUPPORTED_DISPLAY_VSYNC_PERIODS getSupportedDisplayVsyncPeriods; HWC2_PFN_GET_DISPLAY_VSYNC_PERIOD getDisplayVsyncPeriod; - HWC2_PFN_SET_ACTIVE_CONFIG_AND_VSYNC_PERIOD setActiveConfigAndVsyncPeriod; + HWC2_PFN_SET_ACTIVE_CONFIG_WITH_CONSTRAINTS setActiveConfigWithConstraints; } mDispatch = {}; hal::ComposerHal::EventCallback_2_4* mEventCallback_2_4 = nullptr; diff --git a/graphics/composer/2.4/utils/vts/ComposerVts.cpp b/graphics/composer/2.4/utils/vts/ComposerVts.cpp index b02a59a90a..35ac23f7ff 100644 --- a/graphics/composer/2.4/utils/vts/ComposerVts.cpp +++ b/graphics/composer/2.4/utils/vts/ComposerVts.cpp @@ -70,15 +70,18 @@ Error ComposerClient::getDisplayConnectionType(Display display, return error; } -Error ComposerClient::getSupportedDisplayVsyncPeriods( - Display display, Config config, std::vector* outSupportedVsyncPeriods) { - Error error = Error::NONE; - mClient->getSupportedDisplayVsyncPeriods( - display, config, [&](const auto& tmpError, const auto& tmpSupportedVsyncPeriods) { - error = tmpError; - *outSupportedVsyncPeriods = tmpSupportedVsyncPeriods; +int32_t ComposerClient::getDisplayAttribute_2_4( + android::hardware::graphics::composer::V2_1::Display display, + android::hardware::graphics::composer::V2_1::Config config, + IComposerClient::Attribute attribute) { + int32_t value = 0; + mClient->getDisplayAttribute_2_4( + display, config, attribute, [&](const auto& tmpError, const auto& tmpValue) { + ASSERT_EQ(Error::NONE, tmpError) << "failed to get display attribute"; + value = tmpValue; }); - return error; + + return value; } Error ComposerClient::getDisplayVsyncPeriod(Display display, VsyncPeriodNanos* outVsyncPeriod) { @@ -90,17 +93,16 @@ Error ComposerClient::getDisplayVsyncPeriod(Display display, VsyncPeriodNanos* o return error; } -Error ComposerClient::setActiveConfigAndVsyncPeriod( - Display display, Config config, VsyncPeriodNanos vsyncPeriodNanos, +Error ComposerClient::setActiveConfigWithConstraints( + Display display, Config config, const IComposerClient::VsyncPeriodChangeConstraints& vsyncPeriodChangeConstraints, - int64_t* outNewVsyncAppliedTime) { + VsyncPeriodChangeTimeline* timeline) { Error error = Error::NONE; - mClient->setActiveConfigAndVsyncPeriod( - display, config, vsyncPeriodNanos, vsyncPeriodChangeConstraints, - [&](const auto& tmpError, const auto& tmpNewVsyncAppliedTime) { - error = tmpError; - *outNewVsyncAppliedTime = tmpNewVsyncAppliedTime; - }); + mClient->setActiveConfigWithConstraints(display, config, vsyncPeriodChangeConstraints, + [&](const auto& tmpError, const auto& tmpTimeline) { + error = tmpError; + *timeline = tmpTimeline; + }); return error; } diff --git a/graphics/composer/2.4/utils/vts/include/composer-vts/2.4/ComposerVts.h b/graphics/composer/2.4/utils/vts/include/composer-vts/2.4/ComposerVts.h index 5db3e16eaf..83e74ed698 100644 --- a/graphics/composer/2.4/utils/vts/include/composer-vts/2.4/ComposerVts.h +++ b/graphics/composer/2.4/utils/vts/include/composer-vts/2.4/ComposerVts.h @@ -74,15 +74,15 @@ class ComposerClient : public V2_3::vts::ComposerClient { Error getDisplayConnectionType(Display display, IComposerClient::DisplayConnectionType* outType); - Error getSupportedDisplayVsyncPeriods(Display display, Config config, - std::vector* outSupportedVsyncPeriods); + int32_t getDisplayAttribute_2_4(Display display, Config config, + IComposerClient::Attribute attribute); Error getDisplayVsyncPeriod(Display display, VsyncPeriodNanos* outVsyncPeriods); - Error setActiveConfigAndVsyncPeriod( - Display display, Config config, VsyncPeriodNanos vsyncPeriodNanos, + Error setActiveConfigWithConstraints( + Display display, Config config, const IComposerClient::VsyncPeriodChangeConstraints& vsyncPeriodChangeConstraints, - int64_t* outNewVsyncAppliedTime); + VsyncPeriodChangeTimeline* timeline); private: const sp mClient; diff --git a/graphics/composer/2.4/vts/functional/VtsHalGraphicsComposerV2_4TargetTest.cpp b/graphics/composer/2.4/vts/functional/VtsHalGraphicsComposerV2_4TargetTest.cpp index 2c87666efb..f038f551e3 100644 --- a/graphics/composer/2.4/vts/functional/VtsHalGraphicsComposerV2_4TargetTest.cpp +++ b/graphics/composer/2.4/vts/functional/VtsHalGraphicsComposerV2_4TargetTest.cpp @@ -114,7 +114,7 @@ class GraphicsComposerHidlTest : public ::testing::TestWithParam { void execute() { mComposerClient->execute(mReader.get(), mWriter.get()); } - void Test_setActiveConfigAndVsyncPeriod( + void Test_setActiveConfigWithConstraints( const IComposerClient::VsyncPeriodChangeConstraints& constraints); std::unique_ptr mComposer; @@ -194,42 +194,28 @@ TEST_P(GraphicsComposerHidlTest, getDisplayConnectionType) { } } -TEST_P(GraphicsComposerHidlTest, getSupportedDisplayVsyncPeriods_BadDisplay) { - std::vector supportedVsyncPeriods; - EXPECT_EQ(Error::BAD_DISPLAY, mComposerClient->getSupportedDisplayVsyncPeriods( - mInvalidDisplayId, Config(0), &supportedVsyncPeriods)); -} +TEST_P(GraphicsComposerHidlTest, GetDisplayAttribute_2_4) { + std::vector configs = mComposerClient->getDisplayConfigs(mPrimaryDisplay); + for (auto config : configs) { + const std::array requiredAttributes = {{ + IComposerClient::Attribute::WIDTH, + IComposerClient::Attribute::HEIGHT, + IComposerClient::Attribute::VSYNC_PERIOD, + IComposerClient::Attribute::CONFIG_GROUP, + }}; + for (auto attribute : requiredAttributes) { + mComposerClient->getDisplayAttribute_2_4(mPrimaryDisplay, config, attribute); + } -TEST_P(GraphicsComposerHidlTest, getSupportedDisplayVsyncPeriods_BadConfig) { - for (Display display : mComposerCallback->getDisplays()) { - Config invalidConfigId = GetInvalidConfigId(display); - std::vector supportedVsyncPeriods; - EXPECT_EQ(Error::BAD_CONFIG, mComposerClient->getSupportedDisplayVsyncPeriods( - display, invalidConfigId, &supportedVsyncPeriods)); - } -} - -TEST_P(GraphicsComposerHidlTest, getSupportedDisplayVsyncPeriods) { - for (Display display : mComposerCallback->getDisplays()) { - for (Config config : mComposerClient->getDisplayConfigs(display)) { - std::vector supportedVsyncPeriods; - - // Get the default vsync period from the config - VsyncPeriodNanos defaultVsyncPeiord = mComposerClient->getDisplayAttribute( - display, config, IComposerClient::Attribute::VSYNC_PERIOD); - // Get all supported vsync periods for this config - EXPECT_EQ(Error::NONE, mComposerClient->getSupportedDisplayVsyncPeriods( - display, config, &supportedVsyncPeriods)); - // Default vsync period must be present in the list - EXPECT_NE(std::find(supportedVsyncPeriods.begin(), supportedVsyncPeriods.end(), - defaultVsyncPeiord), - supportedVsyncPeriods.end()); - - // Each vsync period must be unique - std::unordered_set vsyncPeriodSet; - for (VsyncPeriodNanos vsyncPeriodNanos : supportedVsyncPeriods) { - EXPECT_TRUE(vsyncPeriodSet.insert(vsyncPeriodNanos).second); - } + const std::array optionalAttributes = {{ + IComposerClient::Attribute::DPI_X, + IComposerClient::Attribute::DPI_Y, + }}; + for (auto attribute : optionalAttributes) { + mComposerClient->getRaw()->getDisplayAttribute_2_4( + mPrimaryDisplay, config, attribute, [&](const auto& tmpError, const auto&) { + EXPECT_TRUE(tmpError == Error::NONE || tmpError == Error::UNSUPPORTED); + }); } } } @@ -240,20 +226,40 @@ TEST_P(GraphicsComposerHidlTest, getDisplayVsyncPeriod_BadDisplay) { mComposerClient->getDisplayVsyncPeriod(mInvalidDisplayId, &vsyncPeriodNanos)); } -TEST_P(GraphicsComposerHidlTest, setActiveConfigAndVsyncPeriod_BadDisplay) { - int64_t newVsyncAppliedTime; +TEST_P(GraphicsComposerHidlTest, getDisplayVsyncPeriod) { + for (Display display : mComposerCallback->getDisplays()) { + for (Config config : mComposerClient->getDisplayConfigs(display)) { + mComposerClient->setActiveConfig(display, config); + + VsyncPeriodNanos vsyncPeriodNanos; + VsyncPeriodNanos expectedvsyncPeriodNanos = mComposerClient->getDisplayAttribute_2_4( + display, config, IComposerClient::IComposerClient::Attribute::VSYNC_PERIOD); + int retryCount = 100; + do { + std::this_thread::sleep_for(10ms); + EXPECT_EQ(Error::NONE, + mComposerClient->getDisplayVsyncPeriod(display, &vsyncPeriodNanos)); + --retryCount; + } while (retryCount > 0); + + EXPECT_EQ(vsyncPeriodNanos, expectedvsyncPeriodNanos); + } + } +} + +TEST_P(GraphicsComposerHidlTest, setActiveConfigWithConstraints_BadDisplay) { + VsyncPeriodChangeTimeline timeline; IComposerClient::VsyncPeriodChangeConstraints constraints; constraints.seamlessRequired = false; constraints.desiredTimeNanos = systemTime(); - EXPECT_EQ(Error::BAD_DISPLAY, mComposerClient->setActiveConfigAndVsyncPeriod( - mInvalidDisplayId, Config(0), VsyncPeriodNanos(0), - constraints, &newVsyncAppliedTime)); + EXPECT_EQ(Error::BAD_DISPLAY, mComposerClient->setActiveConfigWithConstraints( + mInvalidDisplayId, Config(0), constraints, &timeline)); } -TEST_P(GraphicsComposerHidlTest, setActiveConfigAndVsyncPeriod_BadConfig) { - int64_t newVsyncAppliedTime; +TEST_P(GraphicsComposerHidlTest, setActiveConfigWithConstraints_BadConfig) { + VsyncPeriodChangeTimeline timeline; IComposerClient::VsyncPeriodChangeConstraints constraints; constraints.seamlessRequired = false; @@ -261,93 +267,114 @@ TEST_P(GraphicsComposerHidlTest, setActiveConfigAndVsyncPeriod_BadConfig) { for (Display display : mComposerCallback->getDisplays()) { Config invalidConfigId = GetInvalidConfigId(display); - EXPECT_EQ(Error::BAD_CONFIG, mComposerClient->setActiveConfigAndVsyncPeriod( - display, invalidConfigId, VsyncPeriodNanos(0), - constraints, &newVsyncAppliedTime)); + EXPECT_EQ(Error::BAD_CONFIG, mComposerClient->setActiveConfigWithConstraints( + display, invalidConfigId, constraints, &timeline)); } } -TEST_P(GraphicsComposerHidlTest, setActiveConfigAndVsyncPeriod_BadVsyncPeriod) { - int64_t newVsyncAppliedTime; +TEST_P(GraphicsComposerHidlTest, setActiveConfigWithConstraints_SeamlessNotAllowed) { + VsyncPeriodChangeTimeline timeline; IComposerClient::VsyncPeriodChangeConstraints constraints; - constraints.seamlessRequired = false; + constraints.seamlessRequired = true; constraints.desiredTimeNanos = systemTime(); for (Display display : mComposerCallback->getDisplays()) { for (Config config : mComposerClient->getDisplayConfigs(display)) { - EXPECT_EQ(Error::BAD_VSYNC_PERIOD, mComposerClient->setActiveConfigAndVsyncPeriod( - display, config, VsyncPeriodNanos(0), - constraints, &newVsyncAppliedTime)); - } - } -} + int32_t configGroup = mComposerClient->getDisplayAttribute_2_4( + display, config, IComposerClient::IComposerClient::Attribute::CONFIG_GROUP); -void GraphicsComposerHidlTest::Test_setActiveConfigAndVsyncPeriod( - const IComposerClient::VsyncPeriodChangeConstraints& constraints) { - int64_t newVsyncAppliedTime; - - for (Display display : mComposerCallback->getDisplays()) { - for (Config config : mComposerClient->getDisplayConfigs(display)) { - std::vector supportedVsyncPeriods; - - EXPECT_EQ(Error::NONE, mComposerClient->getSupportedDisplayVsyncPeriods( - display, config, &supportedVsyncPeriods)); - for (VsyncPeriodNanos newVsyncPeriod : supportedVsyncPeriods) { - VsyncPeriodNanos vsyncPeriodNanos; - EXPECT_EQ(Error::NONE, - mComposerClient->getDisplayVsyncPeriod(display, &vsyncPeriodNanos)); - - if (vsyncPeriodNanos == newVsyncPeriod) { - continue; + for (Config otherConfig : mComposerClient->getDisplayConfigs(display)) { + int32_t otherConfigGroup = mComposerClient->getDisplayAttribute_2_4( + display, otherConfig, + IComposerClient::IComposerClient::Attribute::CONFIG_GROUP); + if (configGroup != otherConfigGroup) { + mComposerClient->setActiveConfig(display, config); + EXPECT_EQ(Error::SEAMLESS_NOT_ALLOWED, + mComposerClient->setActiveConfigWithConstraints( + display, otherConfig, constraints, &timeline)); } - - EXPECT_EQ(Error::NONE, mComposerClient->setActiveConfigAndVsyncPeriod( - display, config, newVsyncPeriod, constraints, - &newVsyncAppliedTime)); - - EXPECT_TRUE(newVsyncAppliedTime >= constraints.desiredTimeNanos); - - // Refresh rate should change within a reasonable time - constexpr nsecs_t kReasonableTimeForChange = 1'000'000'000; // 1 second - EXPECT_TRUE(newVsyncAppliedTime - constraints.desiredTimeNanos <= - kReasonableTimeForChange); - - while (systemTime() <= newVsyncAppliedTime) { - EXPECT_EQ(Error::NONE, - mComposerClient->getDisplayVsyncPeriod(display, &vsyncPeriodNanos)); - if (systemTime() <= constraints.desiredTimeNanos) { - EXPECT_NE(vsyncPeriodNanos, newVsyncPeriod); - } - - if (vsyncPeriodNanos == newVsyncPeriod) { - break; - } - std::this_thread::sleep_for(10ms); - } - EXPECT_EQ(Error::NONE, - mComposerClient->getDisplayVsyncPeriod(display, &vsyncPeriodNanos)); - EXPECT_EQ(vsyncPeriodNanos, newVsyncPeriod); } } } } -TEST_P(GraphicsComposerHidlTest, setActiveConfigAndVsyncPeriod) { +void GraphicsComposerHidlTest::Test_setActiveConfigWithConstraints( + const IComposerClient::VsyncPeriodChangeConstraints& constraints) { + VsyncPeriodChangeTimeline timeline = {}; + + for (Display display : mComposerCallback->getDisplays()) { + for (Config config : mComposerClient->getDisplayConfigs(display)) { + mComposerClient->setActiveConfig(display, config); + + int32_t configVsyncPeriod = mComposerClient->getDisplayAttribute_2_4( + display, config, IComposerClient::IComposerClient::Attribute::VSYNC_PERIOD); + for (Config otherConfig : mComposerClient->getDisplayConfigs(display)) { + if (config == otherConfig) { + continue; + } + + int32_t otherVsyncPeriod = mComposerClient->getDisplayAttribute_2_4( + display, otherConfig, + IComposerClient::IComposerClient::Attribute::VSYNC_PERIOD); + + if (configVsyncPeriod == otherVsyncPeriod) { + continue; + } + + EXPECT_EQ(Error::NONE, mComposerClient->setActiveConfigWithConstraints( + display, otherConfig, constraints, &timeline)); + + if (timeline.refreshRequired) { + // TODO(b/143775556): handle this case; + continue; + } + + EXPECT_TRUE(timeline.newVsyncAppliedTimeNanos >= constraints.desiredTimeNanos); + + // Refresh rate should change within a reasonable time + constexpr nsecs_t kReasonableTimeForChange = 1'000'000'000; // 1 second + EXPECT_TRUE(timeline.newVsyncAppliedTimeNanos - constraints.desiredTimeNanos <= + kReasonableTimeForChange); + + while (systemTime() <= timeline.newVsyncAppliedTimeNanos) { + VsyncPeriodNanos vsyncPeriodNanos; + EXPECT_EQ(Error::NONE, + mComposerClient->getDisplayVsyncPeriod(display, &vsyncPeriodNanos)); + + if (systemTime() <= constraints.desiredTimeNanos) { + EXPECT_NE(vsyncPeriodNanos, otherVsyncPeriod); + } + + if (vsyncPeriodNanos == otherVsyncPeriod) { + break; + } + std::this_thread::sleep_for(10ms); + } + VsyncPeriodNanos vsyncPeriodNanos; + EXPECT_EQ(Error::NONE, + mComposerClient->getDisplayVsyncPeriod(display, &vsyncPeriodNanos)); + EXPECT_EQ(vsyncPeriodNanos, otherVsyncPeriod); + } + } + } +} + +TEST_P(GraphicsComposerHidlTest, setActiveConfigWithConstraints) { IComposerClient::VsyncPeriodChangeConstraints constraints; constraints.seamlessRequired = false; constraints.desiredTimeNanos = systemTime(); - Test_setActiveConfigAndVsyncPeriod(constraints); + Test_setActiveConfigWithConstraints(constraints); } -TEST_P(GraphicsComposerHidlTest, setActiveConfigAndVsyncPeriod_delayed) { +TEST_P(GraphicsComposerHidlTest, setActiveConfigWithConstraints_delayed) { IComposerClient::VsyncPeriodChangeConstraints constraints; constexpr auto kDelayForChange = 300ms; constraints.seamlessRequired = false; constraints.desiredTimeNanos = systemTime() + kDelayForChange.count(); - Test_setActiveConfigAndVsyncPeriod(constraints); + Test_setActiveConfigWithConstraints(constraints); } INSTANTIATE_TEST_SUITE_P(