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

@@ -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);
 };

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<VsyncPeriodNanos> 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);
 };

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;

graphics/composer/2.4/utils/hal/include/composer-hal/2.4/ComposerClient.h

@@ -76,6 +76,13 @@ class ComposerClientImpl : public V2_3::hal::detail::ComposerClientImpl<Interfac
             ALOGE_IF(!ret.isOk(), "failed to send onVsync_2_4: %s", ret.description().c_str());
         }
 
+        void onVsyncPeriodTimingChanged(Display display,
+                                        const VsyncPeriodChangeTimeline& updatedTimeline) override {
+            auto ret = mCallback->onVsyncPeriodTimingChanged(display, updatedTimeline);
+            ALOGE_IF(!ret.isOk(), "failed to send onVsyncPeriodTimingChanged: %s",
+                     ret.description().c_str());
+        }
+
       protected:
         const sp<IComposerCallback> mCallback;
         V2_1::hal::ComposerResources* const mResources;
@@ -104,13 +111,12 @@ class ComposerClientImpl : public V2_3::hal::detail::ComposerClientImpl<Interfac
         return Void();
     }
 
-    Return<void> getSupportedDisplayVsyncPeriods(
-            Display display, Config config,
-            IComposerClient::getSupportedDisplayVsyncPeriods_cb hidl_cb) override {
-        std::vector<VsyncPeriodNanos> supportedVsyncPeriods;
-        Error error =
-                mHal->getSupportedDisplayVsyncPeriods(display, config, &supportedVsyncPeriods);
-        hidl_cb(error, supportedVsyncPeriods);
+    Return<void> 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<Interfac
         return Void();
     }
 
-    Return<void> setActiveConfigAndVsyncPeriod(
-            Display display, Config config, VsyncPeriodNanos vsyncPeriodNanos,
+    Return<void> 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();
     }
 

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<IComposerClient::DisplayCapability>* outCapabilities) = 0;
     virtual Error getDisplayConnectionType(Display display,
                                            IComposerClient::DisplayConnectionType* outType) = 0;
-    virtual Error getSupportedDisplayVsyncPeriods(
-            Display display, Config config, std::vector<VsyncPeriodNanos>* 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

graphics/composer/2.4/utils/passthrough/include/composer-passthrough/2.4/HwcHal.h

@@ -25,6 +25,7 @@
 #include <android/hardware/graphics/composer/2.4/IComposerClient.h>
 #include <composer-hal/2.4/ComposerHal.h>
 #include <composer-passthrough/2.3/HwcHal.h>
+#include <hardware/hwcomposer_defs.h>
 
 namespace android {
 namespace hardware {
@@ -51,14 +52,22 @@ class HwcHalImpl : public V2_3::passthrough::detail::HwcHalImpl<Hal> {
     void registerEventCallback_2_4(hal::ComposerHal::EventCallback_2_4* callback) override {
         mEventCallback_2_4 = callback;
 
-        mDispatch.registerCallback(mDevice, HWC2_CALLBACK_HOTPLUG, this,
-                                   reinterpret_cast<hwc2_function_pointer_t>(hotplugHook));
-        mDispatch.registerCallback(mDevice, HWC2_CALLBACK_REFRESH, this,
-                                   reinterpret_cast<hwc2_function_pointer_t>(refreshHook));
-        mDispatch.registerCallback(mDevice, HWC2_CALLBACK_VSYNC, this,
-                                   reinterpret_cast<hwc2_function_pointer_t>(vsyncHook));
-        mDispatch.registerCallback(mDevice, HWC2_CALLBACK_VSYNC_2_4, this,
-                                   reinterpret_cast<hwc2_function_pointer_t>(vsync_2_4_Hook));
+        BaseType2_1::mDispatch.registerCallback(
+                mDevice, HWC2_CALLBACK_HOTPLUG, this,
+                reinterpret_cast<hwc2_function_pointer_t>(hotplugHook));
+        BaseType2_1::mDispatch.registerCallback(
+                mDevice, HWC2_CALLBACK_REFRESH, this,
+                reinterpret_cast<hwc2_function_pointer_t>(refreshHook));
+        BaseType2_1::mDispatch.registerCallback(
+                mDevice, HWC2_CALLBACK_VSYNC, this,
+                reinterpret_cast<hwc2_function_pointer_t>(vsyncHook));
+        BaseType2_1::mDispatch.registerCallback(
+                mDevice, HWC2_CALLBACK_VSYNC_2_4, this,
+                reinterpret_cast<hwc2_function_pointer_t>(vsync_2_4_Hook));
+
+        BaseType2_1::mDispatch.registerCallback(
+                mDevice, HWC2_CALLBACK_VSYNC_PERIOD_TIMING_CHANGED, this,
+                reinterpret_cast<hwc2_function_pointer_t>(vsyncPeriodTimingChangedHook));
     }
 
     void unregisterEventCallback_2_4() override {
@@ -69,10 +78,12 @@ class HwcHalImpl : public V2_3::passthrough::detail::HwcHalImpl<Hal> {
         //  - 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<Hal> {
         return static_cast<Error>(error);
     }
 
-    Error getSupportedDisplayVsyncPeriods(Display display, Config config,
-                                          std::vector<VsyncPeriodNanos>* 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<int32_t>(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>(error);
-        }
-        outVsyncPeriods->resize(count);
-        error = mDispatch.getSupportedDisplayVsyncPeriods(mDevice, display, config, &count,
-                                                          outVsyncPeriods->data());
-        if (error != HWC2_ERROR_NONE) {
-            *outVsyncPeriods = std::vector<VsyncPeriodNanos>();
-            return static_cast<Error>(error);
-        }
-        return Error::NONE;
+        return static_cast<Error>(err);
     }
 
     Error getDisplayVsyncPeriod(Display display, VsyncPeriodNanos* outVsyncPeriod) override {
@@ -140,11 +141,11 @@ class HwcHalImpl : public V2_3::passthrough::detail::HwcHalImpl<Hal> {
         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<Hal> {
         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>(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<Hal> {
 
         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> {
         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<HwcHalImpl*>(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;

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<VsyncPeriodNanos>* 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;
 }
 

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<VsyncPeriodNanos>* 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<IComposerClient> mClient;

graphics/composer/2.4/vts/functional/VtsHalGraphicsComposerV2_4TargetTest.cpp

@@ -114,7 +114,7 @@ class GraphicsComposerHidlTest : public ::testing::TestWithParam<std::string> {
 
     void execute() { mComposerClient->execute(mReader.get(), mWriter.get()); }
 
-    void Test_setActiveConfigAndVsyncPeriod(
+    void Test_setActiveConfigWithConstraints(
             const IComposerClient::VsyncPeriodChangeConstraints& constraints);
 
     std::unique_ptr<Composer> mComposer;
@@ -194,42 +194,28 @@ TEST_P(GraphicsComposerHidlTest, getDisplayConnectionType) {
     }
 }
 
-TEST_P(GraphicsComposerHidlTest, getSupportedDisplayVsyncPeriods_BadDisplay) {
-    std::vector<VsyncPeriodNanos> supportedVsyncPeriods;
-    EXPECT_EQ(Error::BAD_DISPLAY, mComposerClient->getSupportedDisplayVsyncPeriods(
-                                          mInvalidDisplayId, Config(0), &supportedVsyncPeriods));
-}
+TEST_P(GraphicsComposerHidlTest, GetDisplayAttribute_2_4) {
+    std::vector<Config> configs = mComposerClient->getDisplayConfigs(mPrimaryDisplay);
+    for (auto config : configs) {
+        const std::array<IComposerClient::Attribute, 4> 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<VsyncPeriodNanos> 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<VsyncPeriodNanos> 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<VsyncPeriodNanos> vsyncPeriodSet;
-            for (VsyncPeriodNanos vsyncPeriodNanos : supportedVsyncPeriods) {
-                EXPECT_TRUE(vsyncPeriodSet.insert(vsyncPeriodNanos).second);
-            }
+        const std::array<IComposerClient::Attribute, 2> 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<VsyncPeriodNanos> 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(