Merge "SurfaceFlinger: add VSYNC-predicted as debug option" into rvc-dev
diff --git a/cmds/dumpstate/DumpstateService.cpp b/cmds/dumpstate/DumpstateService.cpp
index 87ea520..466575f 100644
--- a/cmds/dumpstate/DumpstateService.cpp
+++ b/cmds/dumpstate/DumpstateService.cpp
@@ -120,7 +120,7 @@
     options->Initialize(static_cast<Dumpstate::BugreportMode>(bugreport_mode), bugreport_fd,
                         screenshot_fd);
 
-    if (bugreport_fd.get() == -1 || (options->do_fb && screenshot_fd.get() == -1)) {
+    if (bugreport_fd.get() == -1 || (options->do_screenshot && screenshot_fd.get() == -1)) {
         MYLOGE("Invalid filedescriptor");
         signalErrorAndExit(listener, IDumpstateListener::BUGREPORT_ERROR_INVALID_INPUT);
     }
diff --git a/cmds/dumpstate/binder/android/os/IDumpstateListener.aidl b/cmds/dumpstate/binder/android/os/IDumpstateListener.aidl
index e486460..e17f18e 100644
--- a/cmds/dumpstate/binder/android/os/IDumpstateListener.aidl
+++ b/cmds/dumpstate/binder/android/os/IDumpstateListener.aidl
@@ -61,4 +61,9 @@
      * Called when taking bugreport finishes successfully.
      */
     void onFinished();
+
+    /**
+     * Called when screenshot is taken.
+     */
+    void onScreenshotTaken(boolean success);
 }
diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp
index 814a4ed..3440116 100644
--- a/cmds/dumpstate/dumpstate.cpp
+++ b/cmds/dumpstate/dumpstate.cpp
@@ -647,6 +647,24 @@
     std::lock_guard<std::mutex> lock(lock_);
     result_ = APPROVED;
     MYLOGD("User approved consent to share bugreport\n");
+
+    // Maybe copy screenshot so calling app can display the screenshot to the user as soon as
+    // consent is granted.
+    if (ds.options_->is_screenshot_copied) {
+        return android::binder::Status::ok();
+    }
+
+    if (!ds.options_->do_screenshot || ds.options_->screenshot_fd.get() == -1 ||
+        !ds.do_early_screenshot_) {
+        return android::binder::Status::ok();
+    }
+
+    bool copy_succeeded = android::os::CopyFileToFd(ds.screenshot_path_,
+                                                    ds.options_->screenshot_fd.get());
+    ds.options_->is_screenshot_copied = copy_succeeded;
+    if (copy_succeeded) {
+        android::os::UnlinkAndLogOnError(ds.screenshot_path_);
+    }
     return android::binder::Status::ok();
 }
 
@@ -1407,7 +1425,7 @@
     /* Dump Bluetooth HCI logs */
     ds.AddDir("/data/misc/bluetooth/logs", true);
 
-    if (ds.options_->do_fb && !ds.do_early_screenshot_) {
+    if (ds.options_->do_screenshot && !ds.do_early_screenshot_) {
         MYLOGI("taking late screenshot\n");
         ds.TakeScreenshot();
     }
@@ -2149,7 +2167,7 @@
         ds.base_name_ += "-wifi";
     }
 
-    if (ds.options_->do_fb) {
+    if (ds.options_->do_screenshot) {
         ds.screenshot_path_ = ds.GetPath(ds.CalledByApi() ? "-tmp.png" : ".png");
     }
     ds.tmp_path_ = ds.GetPath(".tmp");
@@ -2229,43 +2247,45 @@
 }
 
 static void SetOptionsFromMode(Dumpstate::BugreportMode mode, Dumpstate::DumpOptions* options) {
+    // Modify com.android.shell.BugreportProgressService#isDefaultScreenshotRequired as well for
+    // default system screenshots.
     options->bugreport_mode = ModeToString(mode);
     switch (mode) {
         case Dumpstate::BugreportMode::BUGREPORT_FULL:
-            options->do_fb = true;
+            options->do_screenshot = true;
             options->dumpstate_hal_mode = DumpstateMode::FULL;
             break;
         case Dumpstate::BugreportMode::BUGREPORT_INTERACTIVE:
             // Currently, the dumpstate binder is only used by Shell to update progress.
             options->do_start_service = true;
             options->do_progress_updates = true;
-            options->do_fb = false;
+            options->do_screenshot = true;
             options->dumpstate_hal_mode = DumpstateMode::INTERACTIVE;
             break;
         case Dumpstate::BugreportMode::BUGREPORT_REMOTE:
             options->do_vibrate = false;
             options->is_remote_mode = true;
-            options->do_fb = false;
+            options->do_screenshot = false;
             options->dumpstate_hal_mode = DumpstateMode::REMOTE;
             break;
         case Dumpstate::BugreportMode::BUGREPORT_WEAR:
             options->do_start_service = true;
             options->do_progress_updates = true;
             options->do_zip_file = true;
-            options->do_fb = true;
+            options->do_screenshot = true;
             options->dumpstate_hal_mode = DumpstateMode::WEAR;
             break;
         // TODO(b/148168577) rename TELEPHONY everywhere to CONNECTIVITY.
         case Dumpstate::BugreportMode::BUGREPORT_TELEPHONY:
             options->telephony_only = true;
             options->do_progress_updates = true;
-            options->do_fb = false;
+            options->do_screenshot = false;
             options->dumpstate_hal_mode = DumpstateMode::CONNECTIVITY;
             break;
         case Dumpstate::BugreportMode::BUGREPORT_WIFI:
             options->wifi_only = true;
             options->do_zip_file = true;
-            options->do_fb = false;
+            options->do_screenshot = false;
             options->dumpstate_hal_mode = DumpstateMode::WIFI;
             break;
         case Dumpstate::BugreportMode::BUGREPORT_DEFAULT:
@@ -2275,12 +2295,13 @@
 
 static void LogDumpOptions(const Dumpstate::DumpOptions& options) {
     MYLOGI(
-        "do_zip_file: %d do_vibrate: %d use_socket: %d use_control_socket: %d do_fb: %d "
+        "do_zip_file: %d do_vibrate: %d use_socket: %d use_control_socket: %d do_screenshot: %d "
         "is_remote_mode: %d show_header_only: %d do_start_service: %d telephony_only: %d "
         "wifi_only: %d do_progress_updates: %d fd: %d bugreport_mode: %s dumpstate_hal_mode: %s "
         "args: %s\n",
         options.do_zip_file, options.do_vibrate, options.use_socket, options.use_control_socket,
-        options.do_fb, options.is_remote_mode, options.show_header_only, options.do_start_service,
+        options.do_screenshot, options.is_remote_mode, options.show_header_only,
+        options.do_start_service,
         options.telephony_only, options.wifi_only, options.do_progress_updates,
         options.bugreport_fd.get(), options.bugreport_mode.c_str(),
         toString(options.dumpstate_hal_mode).c_str(), options.args.c_str());
@@ -2313,7 +2334,7 @@
             case 'S': use_control_socket = true;     break;
             case 'v': show_header_only = true;       break;
             case 'q': do_vibrate = false;            break;
-            case 'p': do_fb = true;                  break;
+            case 'p': do_screenshot = true;          break;
             case 'P': do_progress_updates = true;    break;
             case 'R': is_remote_mode = true;         break;
             case 'V':                                break;  // compatibility no-op
@@ -2543,11 +2564,6 @@
         Vibrate(150);
     }
 
-    if (options_->do_fb && do_early_screenshot_) {
-        MYLOGI("taking early screenshot\n");
-        TakeScreenshot();
-    }
-
     if (options_->do_zip_file && zip_file != nullptr) {
         if (chown(path_.c_str(), AID_SHELL, AID_SHELL)) {
             MYLOGE("Unable to change ownership of zip file %s: %s\n", path_.c_str(),
@@ -2593,19 +2609,20 @@
     PrintHeader();
 
     if (options_->telephony_only) {
+        MaybeTakeEarlyScreenshot();
         MaybeCheckUserConsent(calling_uid, calling_package);
         DumpstateTelephonyOnly(calling_package);
         DumpstateBoard();
     } else if (options_->wifi_only) {
+        MaybeTakeEarlyScreenshot();
         MaybeCheckUserConsent(calling_uid, calling_package);
         DumpstateWifiOnly();
     } else {
-        // Invoking the critical dumpsys calls before DumpTraces() to try and
-        // keep the system stats as close to its initial state as possible.
+        // Invoke critical dumpsys first to preserve system state, before doing anything else.
         RunDumpsysCritical();
 
-        // Run consent check only after critical dumpsys has finished -- so the consent
-        // isn't going to pollute the system state / logs.
+        // Take screenshot and get consent only after critical dumpsys has finished.
+        MaybeTakeEarlyScreenshot();
         MaybeCheckUserConsent(calling_uid, calling_package);
 
         // Dump state for the default case. This also drops root.
@@ -2640,7 +2657,9 @@
             MYLOGI("User denied consent. Returning\n");
             return status;
         }
-        if (options_->do_fb && options_->screenshot_fd.get() != -1) {
+        if (options_->do_screenshot &&
+            options_->screenshot_fd.get() != -1 &&
+            !options_->is_screenshot_copied) {
             bool copy_succeeded = android::os::CopyFileToFd(screenshot_path_,
                                                             options_->screenshot_fd.get());
             if (copy_succeeded) {
@@ -2694,6 +2713,14 @@
                : RunStatus::OK;
 }
 
+void Dumpstate::MaybeTakeEarlyScreenshot() {
+    if (!options_->do_screenshot || !do_early_screenshot_) {
+        return;
+    }
+
+    TakeScreenshot();
+}
+
 void Dumpstate::MaybeCheckUserConsent(int32_t calling_uid, const std::string& calling_package) {
     if (calling_uid == AID_SHELL || !CalledByApi()) {
         // No need to get consent for shell triggered dumpstates, or not through
@@ -3630,6 +3657,11 @@
     } else {
         MYLOGE("Failed to take screenshot on %s\n", real_path.c_str());
     }
+    if (listener_ != nullptr) {
+        // Show a visual indication to indicate screenshot is taken via
+        // IDumpstateListener.onScreenshotTaken()
+        listener_->onScreenshotTaken(status == 0);
+    }
 }
 
 bool is_dir(const char* pathname) {
diff --git a/cmds/dumpstate/dumpstate.h b/cmds/dumpstate/dumpstate.h
index 111c098..7e27787 100644
--- a/cmds/dumpstate/dumpstate.h
+++ b/cmds/dumpstate/dumpstate.h
@@ -359,7 +359,8 @@
         // Writes bugreport content to a socket; only flatfile format is supported.
         bool use_socket = false;
         bool use_control_socket = false;
-        bool do_fb = false;
+        bool do_screenshot = false;
+        bool is_screenshot_copied = false;
         bool is_remote_mode = false;
         bool show_header_only = false;
         bool do_start_service = false;
@@ -494,6 +495,8 @@
 
     RunStatus DumpstateDefaultAfterCritical();
 
+    void MaybeTakeEarlyScreenshot();
+
     void MaybeCheckUserConsent(int32_t calling_uid, const std::string& calling_package);
 
     // Removes the in progress files output files (tmp file, zip/txt file, screenshot),
diff --git a/cmds/dumpstate/tests/dumpstate_smoke_test.cpp b/cmds/dumpstate/tests/dumpstate_smoke_test.cpp
index dac90d9..f26e4db 100644
--- a/cmds/dumpstate/tests/dumpstate_smoke_test.cpp
+++ b/cmds/dumpstate/tests/dumpstate_smoke_test.cpp
@@ -167,6 +167,12 @@
         return binder::Status::ok();
     }
 
+    binder::Status onScreenshotTaken(bool success) override {
+        std::lock_guard<std::mutex> lock(lock_);
+        dprintf(out_fd_, "\rResult of taking screenshot: %s", success ? "success" : "failure");
+        return binder::Status::ok();
+    }
+
     bool getIsFinished() {
         std::lock_guard<std::mutex> lock(lock_);
         return is_finished_;
diff --git a/cmds/dumpstate/tests/dumpstate_test.cpp b/cmds/dumpstate/tests/dumpstate_test.cpp
index 76b9960..0a0c40e 100644
--- a/cmds/dumpstate/tests/dumpstate_test.cpp
+++ b/cmds/dumpstate/tests/dumpstate_test.cpp
@@ -65,6 +65,7 @@
     MOCK_METHOD1(onProgress, binder::Status(int32_t progress));
     MOCK_METHOD1(onError, binder::Status(int32_t error_code));
     MOCK_METHOD0(onFinished, binder::Status());
+    MOCK_METHOD1(onScreenshotTaken, binder::Status(bool success));
 
   protected:
     MOCK_METHOD0(onAsBinder, IBinder*());
@@ -173,7 +174,7 @@
     EXPECT_FALSE(options_.use_control_socket);
     EXPECT_FALSE(options_.show_header_only);
     EXPECT_TRUE(options_.do_vibrate);
-    EXPECT_FALSE(options_.do_fb);
+    EXPECT_FALSE(options_.do_screenshot);
     EXPECT_FALSE(options_.do_progress_updates);
     EXPECT_FALSE(options_.is_remote_mode);
     EXPECT_EQ(options_.dumpstate_hal_mode, DumpstateMode::DEFAULT);
@@ -199,7 +200,7 @@
     // Other options retain default values
     EXPECT_TRUE(options_.do_vibrate);
     EXPECT_FALSE(options_.show_header_only);
-    EXPECT_FALSE(options_.do_fb);
+    EXPECT_FALSE(options_.do_screenshot);
     EXPECT_FALSE(options_.do_progress_updates);
     EXPECT_FALSE(options_.is_remote_mode);
     EXPECT_FALSE(options_.use_socket);
@@ -225,7 +226,7 @@
     EXPECT_FALSE(options_.do_zip_file);
     EXPECT_FALSE(options_.use_control_socket);
     EXPECT_FALSE(options_.show_header_only);
-    EXPECT_FALSE(options_.do_fb);
+    EXPECT_FALSE(options_.do_screenshot);
     EXPECT_FALSE(options_.do_progress_updates);
     EXPECT_FALSE(options_.is_remote_mode);
     EXPECT_EQ(options_.dumpstate_hal_mode, DumpstateMode::DEFAULT);
@@ -234,7 +235,7 @@
 TEST_F(DumpOptionsTest, InitializeFullBugReport) {
     options_.Initialize(Dumpstate::BugreportMode::BUGREPORT_FULL, fd, fd);
     EXPECT_TRUE(options_.do_add_date);
-    EXPECT_TRUE(options_.do_fb);
+    EXPECT_TRUE(options_.do_screenshot);
     EXPECT_TRUE(options_.do_zip_file);
     EXPECT_EQ(options_.dumpstate_hal_mode, DumpstateMode::FULL);
 
@@ -254,7 +255,7 @@
     EXPECT_TRUE(options_.do_zip_file);
     EXPECT_TRUE(options_.do_progress_updates);
     EXPECT_TRUE(options_.do_start_service);
-    EXPECT_FALSE(options_.do_fb);
+    EXPECT_TRUE(options_.do_screenshot);
     EXPECT_EQ(options_.dumpstate_hal_mode, DumpstateMode::INTERACTIVE);
 
     // Other options retain default values
@@ -271,7 +272,7 @@
     EXPECT_TRUE(options_.do_zip_file);
     EXPECT_TRUE(options_.is_remote_mode);
     EXPECT_FALSE(options_.do_vibrate);
-    EXPECT_FALSE(options_.do_fb);
+    EXPECT_FALSE(options_.do_screenshot);
     EXPECT_EQ(options_.dumpstate_hal_mode, DumpstateMode::REMOTE);
 
     // Other options retain default values
@@ -284,7 +285,7 @@
 TEST_F(DumpOptionsTest, InitializeWearBugReport) {
     options_.Initialize(Dumpstate::BugreportMode::BUGREPORT_WEAR, fd, fd);
     EXPECT_TRUE(options_.do_add_date);
-    EXPECT_TRUE(options_.do_fb);
+    EXPECT_TRUE(options_.do_screenshot);
     EXPECT_TRUE(options_.do_zip_file);
     EXPECT_TRUE(options_.do_progress_updates);
     EXPECT_TRUE(options_.do_start_service);
@@ -301,7 +302,7 @@
 TEST_F(DumpOptionsTest, InitializeTelephonyBugReport) {
     options_.Initialize(Dumpstate::BugreportMode::BUGREPORT_TELEPHONY, fd, fd);
     EXPECT_TRUE(options_.do_add_date);
-    EXPECT_FALSE(options_.do_fb);
+    EXPECT_FALSE(options_.do_screenshot);
     EXPECT_TRUE(options_.do_zip_file);
     EXPECT_TRUE(options_.telephony_only);
     EXPECT_TRUE(options_.do_progress_updates);
@@ -318,7 +319,7 @@
 TEST_F(DumpOptionsTest, InitializeWifiBugReport) {
     options_.Initialize(Dumpstate::BugreportMode::BUGREPORT_WIFI, fd, fd);
     EXPECT_TRUE(options_.do_add_date);
-    EXPECT_FALSE(options_.do_fb);
+    EXPECT_FALSE(options_.do_screenshot);
     EXPECT_TRUE(options_.do_zip_file);
     EXPECT_TRUE(options_.wifi_only);
     EXPECT_EQ(options_.dumpstate_hal_mode, DumpstateMode::WIFI);
@@ -346,7 +347,7 @@
 
     EXPECT_EQ(status, Dumpstate::RunStatus::OK);
     EXPECT_TRUE(options_.do_add_date);
-    EXPECT_TRUE(options_.do_fb);
+    EXPECT_TRUE(options_.do_screenshot);
     EXPECT_TRUE(options_.do_zip_file);
     EXPECT_EQ(options_.dumpstate_hal_mode, DumpstateMode::DEFAULT);
 
@@ -384,7 +385,7 @@
     // Other options retain default values
     EXPECT_FALSE(options_.show_header_only);
     EXPECT_TRUE(options_.do_vibrate);
-    EXPECT_FALSE(options_.do_fb);
+    EXPECT_FALSE(options_.do_screenshot);
     EXPECT_FALSE(options_.do_progress_updates);
     EXPECT_FALSE(options_.is_remote_mode);
     EXPECT_EQ(options_.dumpstate_hal_mode, DumpstateMode::DEFAULT);
@@ -407,7 +408,7 @@
     EXPECT_EQ(status, Dumpstate::RunStatus::OK);
     EXPECT_TRUE(options_.show_header_only);
     EXPECT_FALSE(options_.do_vibrate);
-    EXPECT_TRUE(options_.do_fb);
+    EXPECT_TRUE(options_.do_screenshot);
     EXPECT_TRUE(options_.do_progress_updates);
     EXPECT_TRUE(options_.is_remote_mode);
 
diff --git a/data/etc/android.hardware.device_unique_attestation.xml b/data/etc/android.hardware.device_unique_attestation.xml
new file mode 100644
index 0000000..309be7a
--- /dev/null
+++ b/data/etc/android.hardware.device_unique_attestation.xml
@@ -0,0 +1,20 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!-- Copyright (C) 2020 The Android Open Source Project
+
+     Licensed under the Apache License, Version 2.0 (the "License");
+     you may not use this file except in compliance with the License.
+     You may obtain a copy of the License at
+
+          http://www.apache.org/licenses/LICENSE-2.0
+
+     Unless required by applicable law or agreed to in writing, software
+     distributed under the License is distributed on an "AS IS" BASIS,
+     WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+     See the License for the specific language governing permissions and
+     limitations under the License.
+-->
+
+<!-- Feature for devices with Keymaster that support unique attestation. -->
+<permissions>
+    <feature name="android.hardware.device_unique_attestation" />
+</permissions>
diff --git a/services/inputflinger/BlockingQueue.h b/services/inputflinger/BlockingQueue.h
index db9f26e..b612ca7 100644
--- a/services/inputflinger/BlockingQueue.h
+++ b/services/inputflinger/BlockingQueue.h
@@ -45,10 +45,7 @@
     T pop() {
         std::unique_lock lock(mLock);
         android::base::ScopedLockAssertion assumeLock(mLock);
-        mHasElements.wait(lock, [this]{
-                android::base::ScopedLockAssertion assumeLock(mLock);
-                return !this->mQueue.empty();
-        });
+        mHasElements.wait(lock, [this]() REQUIRES(mLock) { return !this->mQueue.empty(); });
         T t = std::move(mQueue.front());
         mQueue.erase(mQueue.begin());
         return t;
diff --git a/services/inputflinger/InputClassifier.cpp b/services/inputflinger/InputClassifier.cpp
index ae9a348..e5e83d7 100644
--- a/services/inputflinger/InputClassifier.cpp
+++ b/services/inputflinger/InputClassifier.cpp
@@ -250,7 +250,7 @@
             case ClassifierEventType::DEVICE_RESET: {
                 const int32_t deviceId = *(event.getDeviceId());
                 halResponseOk = mService->resetDevice(deviceId).isOk();
-                setClassification(deviceId, MotionClassification::NONE);
+                clearDeviceState(deviceId);
                 break;
             }
             case ClassifierEventType::HAL_RESET: {
@@ -321,6 +321,12 @@
     mClassifications[deviceId] = MotionClassification::NONE;
 }
 
+void MotionClassifier::clearDeviceState(int32_t deviceId) {
+    std::scoped_lock lock(mLock);
+    mClassifications.erase(deviceId);
+    mLastDownTimes.erase(deviceId);
+}
+
 MotionClassification MotionClassifier::classify(const NotifyMotionArgs& args) {
     if ((args.action & AMOTION_EVENT_ACTION_MASK) == AMOTION_EVENT_ACTION_DOWN) {
         updateLastDownTime(args.deviceId, args.downTime);
@@ -455,6 +461,7 @@
 void InputClassifier::dump(std::string& dump) {
     std::scoped_lock lock(mLock);
     dump += "Input Classifier State:\n";
+    dump += StringPrintf(INDENT1 "Deep press: %s\n", deepPressEnabled() ? "enabled" : "disabled");
 
     dump += INDENT1 "Motion Classifier:\n";
     if (mMotionClassifier) {
diff --git a/services/inputflinger/InputClassifier.h b/services/inputflinger/InputClassifier.h
index 47e20db..9692352 100644
--- a/services/inputflinger/InputClassifier.h
+++ b/services/inputflinger/InputClassifier.h
@@ -212,6 +212,8 @@
 
     void updateLastDownTime(int32_t deviceId, nsecs_t downTime);
 
+    void clearDeviceState(int32_t deviceId);
+
     /**
      * Exit the InputClassifier HAL thread.
      * Useful for tests to ensure proper cleanup.
diff --git a/services/surfaceflinger/DisplayHardware/PowerAdvisor.cpp b/services/surfaceflinger/DisplayHardware/PowerAdvisor.cpp
index 550ec61..06e0cbb 100644
--- a/services/surfaceflinger/DisplayHardware/PowerAdvisor.cpp
+++ b/services/surfaceflinger/DisplayHardware/PowerAdvisor.cpp
@@ -124,24 +124,20 @@
     static std::unique_ptr<HalWrapper> connect() {
         // Power HAL 1.3 is not guaranteed to be available, thus we need to query
         // Power HAL 1.0 first and try to cast it to Power HAL 1.3.
-        // Power HAL 1.0 is always available, thus if we fail to query it, it means
-        // Power HAL is not available temporarily and we should retry later. However,
-        // if Power HAL 1.0 is available and we can't cast it to Power HAL 1.3,
-        // it means Power HAL 1.3 is not available at all, so we should stop trying.
         sp<V1_3::IPower> powerHal = nullptr;
-        if (sHasPowerHal_1_3) {
-            sp<V1_0::IPower> powerHal_1_0 = V1_0::IPower::getService();
-            if (powerHal_1_0 != nullptr) {
-                // Try to cast to Power HAL 1.3
-                powerHal = V1_3::IPower::castFrom(powerHal_1_0);
-                if (powerHal == nullptr) {
-                    ALOGW("No Power HAL 1.3 service in system");
-                    sHasPowerHal_1_3 = false;
-                } else {
-                    ALOGI("Loaded Power HAL 1.3 service");
-                }
+        sp<V1_0::IPower> powerHal_1_0 = V1_0::IPower::getService();
+        if (powerHal_1_0 != nullptr) {
+            // Try to cast to Power HAL 1.3
+            powerHal = V1_3::IPower::castFrom(powerHal_1_0);
+            if (powerHal == nullptr) {
+                ALOGW("No Power HAL 1.3 service in system, disabling PowerAdvisor");
+            } else {
+                ALOGI("Loaded Power HAL 1.3 service");
             }
+        } else {
+            ALOGW("No Power HAL found, disabling PowerAdvisor");
         }
+
         if (powerHal == nullptr) {
             return nullptr;
         }
@@ -162,12 +158,9 @@
     }
 
 private:
-    static bool sHasPowerHal_1_3;
     const sp<V1_3::IPower> mPowerHal = nullptr;
 };
 
-bool HidlPowerHalWrapper::sHasPowerHal_1_3 = true;
-
 class AidlPowerHalWrapper : public PowerAdvisor::HalWrapper {
 public:
     AidlPowerHalWrapper(sp<IPower> powerHal) : mPowerHal(std::move(powerHal)) {
@@ -226,7 +219,13 @@
 
 PowerAdvisor::HalWrapper* PowerAdvisor::getPowerHal() {
     static std::unique_ptr<HalWrapper> sHalWrapper = nullptr;
+    static bool sHasHal = true;
 
+    if (!sHasHal) {
+        return nullptr;
+    }
+
+    // If we used to have a HAL, but it stopped responding, attempt to reconnect
     if (mReconnectPowerHal) {
         sHalWrapper = nullptr;
         mReconnectPowerHal = false;
@@ -244,6 +243,12 @@
         sHalWrapper = HidlPowerHalWrapper::connect();
     }
 
+    // If we make it to this point and still don't have a HAL, it's unlikely we
+    // will, so stop trying
+    if (sHalWrapper == nullptr) {
+        sHasHal = false;
+    }
+
     return sHalWrapper.get();
 }
 
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index 766871e..64cfb3d 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -719,9 +719,14 @@
 Hwc2::IComposerClient::Composition Layer::getCompositionType(
         const sp<const DisplayDevice>& display) const {
     const auto outputLayer = findOutputLayerForDisplay(display);
-    LOG_FATAL_IF(!outputLayer);
-    return outputLayer->getState().hwc ? (*outputLayer->getState().hwc).hwcCompositionType
-                                       : Hwc2::IComposerClient::Composition::CLIENT;
+    if (outputLayer == nullptr) {
+        return Hwc2::IComposerClient::Composition::INVALID;
+    }
+    if (outputLayer->getState().hwc) {
+        return (*outputLayer->getState().hwc).hwcCompositionType;
+    } else {
+        return Hwc2::IComposerClient::Composition::CLIENT;
+    }
 }
 
 bool Layer::getClearClientTarget(const sp<const DisplayDevice>& display) const {
@@ -2052,13 +2057,20 @@
     setTransactionFlags(eTransactionNeeded);
 }
 
-LayerProto* Layer::writeToProto(LayersProto& layersProto, uint32_t traceFlags) const {
+LayerProto* Layer::writeToProto(LayersProto& layersProto, uint32_t traceFlags,
+                                const sp<const DisplayDevice>& device) const {
     LayerProto* layerProto = layersProto.add_layers();
     writeToProtoDrawingState(layerProto, traceFlags);
     writeToProtoCommonState(layerProto, LayerVector::StateSet::Drawing, traceFlags);
 
+    // Only populate for the primary display.
+    if (device) {
+        const Hwc2::IComposerClient::Composition compositionType = getCompositionType(device);
+        layerProto->set_hwc_composition_type(static_cast<HwcCompositionType>(compositionType));
+    }
+
     for (const sp<Layer>& layer : mDrawingChildren) {
-        layer->writeToProto(layersProto, traceFlags);
+        layer->writeToProto(layersProto, traceFlags, device);
     }
 
     return layerProto;
diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index 5d2144a..20d7232 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -506,7 +506,8 @@
     bool isRemovedFromCurrentState() const;
 
     LayerProto* writeToProto(LayersProto& layersProto,
-                             uint32_t traceFlags = SurfaceTracing::TRACE_ALL) const;
+                             uint32_t traceFlags = SurfaceTracing::TRACE_ALL,
+                             const sp<const DisplayDevice>& device = nullptr) const;
 
     // Write states that are modified by the main thread. This includes drawing
     // state as well as buffer data. This should be called in the main or tracing
diff --git a/services/surfaceflinger/Scheduler/LayerInfoV2.cpp b/services/surfaceflinger/Scheduler/LayerInfoV2.cpp
index b755798..b4365bf 100644
--- a/services/surfaceflinger/Scheduler/LayerInfoV2.cpp
+++ b/services/surfaceflinger/Scheduler/LayerInfoV2.cpp
@@ -82,14 +82,14 @@
         }
     }
 
-    const auto numFrames = std::distance(it, mFrameTimes.end()) - 1;
-    if (numFrames <= 0) {
+    const auto numFrames = std::distance(it, mFrameTimes.end());
+    if (numFrames < FREQUENT_LAYER_WINDOW_SIZE) {
         return false;
     }
 
     // Layer is considered frequent if the average frame rate is higher than the threshold
     const auto totalTime = mFrameTimes.back().queueTime - it->queueTime;
-    return (1e9f * numFrames) / totalTime >= MIN_FPS_FOR_FREQUENT_LAYER;
+    return (1e9f * (numFrames - 1)) / totalTime >= MIN_FPS_FOR_FREQUENT_LAYER;
 }
 
 bool LayerInfoV2::hasEnoughDataForHeuristic() const {
diff --git a/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp b/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp
index 1765d2d..15b158d 100644
--- a/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp
+++ b/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp
@@ -235,13 +235,13 @@
             ? getBestRefreshRate(scores.rbegin(), scores.rend())
             : getBestRefreshRate(scores.begin(), scores.end());
 
-    return bestRefreshRate == nullptr ? *mCurrentRefreshRate : *bestRefreshRate;
+    return *bestRefreshRate;
 }
 
 template <typename Iter>
 const RefreshRate* RefreshRateConfigs::getBestRefreshRate(Iter begin, Iter end) const {
-    const RefreshRate* bestRefreshRate = nullptr;
-    float max = 0;
+    const RefreshRate* bestRefreshRate = begin->first;
+    float max = begin->second;
     for (auto i = begin; i != end; ++i) {
         const auto [refreshRate, score] = *i;
         ALOGV("%s scores %.2f", refreshRate->name.c_str(), score);
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 1d00624..27bd53c 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -906,15 +906,24 @@
     auto refreshRate = mRefreshRateConfigs->getRefreshRateFromConfigId(info.configId);
     ALOGV("setDesiredActiveConfig(%s)", refreshRate.name.c_str());
 
-    // Don't check against the current mode yet. Worst case we set the desired
-    // config twice. However event generation config might have changed so we need to update it
-    // accordingly
     std::lock_guard<std::mutex> lock(mActiveConfigLock);
-    const Scheduler::ConfigEvent prevConfig = mDesiredActiveConfig.event;
-    mDesiredActiveConfig = info;
-    mDesiredActiveConfig.event = mDesiredActiveConfig.event | prevConfig;
+    if (mDesiredActiveConfigChanged) {
+        // If a config change is pending, just cache the latest request in
+        // mDesiredActiveConfig
+        const Scheduler::ConfigEvent prevConfig = mDesiredActiveConfig.event;
+        mDesiredActiveConfig = info;
+        mDesiredActiveConfig.event = mDesiredActiveConfig.event | prevConfig;
+    } else {
+        // Check is we are already at the desired config
+        const auto display = getDefaultDisplayDeviceLocked();
+        if (!display || display->getActiveConfig() == refreshRate.configId) {
+            return;
+        }
 
-    if (!mDesiredActiveConfigChanged) {
+        // Initiate a config change.
+        mDesiredActiveConfigChanged = true;
+        mDesiredActiveConfig = info;
+
         // This will trigger HWC refresh without resetting the idle timer.
         repaintEverythingForHWC();
         // Start receiving vsync samples now, so that we can detect a period
@@ -927,7 +936,6 @@
         mPhaseConfiguration->setRefreshRateFps(refreshRate.fps);
         mVSyncModulator->setPhaseOffsets(mPhaseConfiguration->getCurrentOffsets());
     }
-    mDesiredActiveConfigChanged = true;
 
     if (mRefreshRateOverlay) {
         mRefreshRateOverlay->changeRefreshRate(refreshRate);
@@ -4450,13 +4458,20 @@
 }
 
 LayersProto SurfaceFlinger::dumpDrawingStateProto(uint32_t traceFlags) const {
+    Mutex::Autolock _l(mStateLock);
+    const auto device = getDefaultDisplayDeviceLocked();
     LayersProto layersProto;
     for (const sp<Layer>& layer : mDrawingState.layersSortedByZ) {
-        layer->writeToProto(layersProto, traceFlags);
+        layer->writeToProto(layersProto, traceFlags, device);
     }
+
     return layersProto;
 }
 
+void SurfaceFlinger::dumpHwc(std::string& result) const {
+    getHwComposer().dump(result);
+}
+
 void SurfaceFlinger::dumpOffscreenLayersProto(LayersProto& layersProto, uint32_t traceFlags) const {
     // Add a fake invisible root layer to the proto output and parent all the offscreen layers to
     // it.
@@ -4471,7 +4486,8 @@
         rootProto->add_children(offscreenLayer->sequence);
 
         // Add layer
-        LayerProto* layerProto = offscreenLayer->writeToProto(layersProto, traceFlags);
+        LayerProto* layerProto =
+                offscreenLayer->writeToProto(layersProto, traceFlags, nullptr /*device*/);
         layerProto->set_parent(offscreenRootLayerId);
     }
 }
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 477d7a5..83f0131 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -921,6 +921,8 @@
     LayersProto dumpDrawingStateProto(uint32_t traceFlags = SurfaceTracing::TRACE_ALL) const;
     void dumpOffscreenLayersProto(LayersProto& layersProto,
                                   uint32_t traceFlags = SurfaceTracing::TRACE_ALL) const;
+    // Dumps state from HW Composer
+    void dumpHwc(std::string& result) const;
     LayersProto dumpProtoFromMainThread(uint32_t traceFlags = SurfaceTracing::TRACE_ALL)
             EXCLUDES(mStateLock);
     void dumpOffscreenLayers(std::string& result) EXCLUDES(mStateLock);
diff --git a/services/surfaceflinger/SurfaceTracing.cpp b/services/surfaceflinger/SurfaceTracing.cpp
index eb5c7de..c5556ec 100644
--- a/services/surfaceflinger/SurfaceTracing.cpp
+++ b/services/surfaceflinger/SurfaceTracing.cpp
@@ -170,6 +170,12 @@
     mFlinger.dumpOffscreenLayersProto(layers);
     entry.mutable_layers()->Swap(&layers);
 
+    if (mTraceFlags & SurfaceTracing::TRACE_HWC) {
+        std::string hwcDump;
+        mFlinger.dumpHwc(hwcDump);
+        entry.set_hwc_blob(hwcDump);
+    }
+
     return entry;
 }
 
diff --git a/services/surfaceflinger/SurfaceTracing.h b/services/surfaceflinger/SurfaceTracing.h
index 18524f0..3c24881 100644
--- a/services/surfaceflinger/SurfaceTracing.h
+++ b/services/surfaceflinger/SurfaceTracing.h
@@ -56,6 +56,7 @@
         TRACE_CRITICAL = 1 << 0,
         TRACE_INPUT = 1 << 1,
         TRACE_EXTRA = 1 << 2,
+        TRACE_HWC = 1 << 3,
         TRACE_ALL = 0xffffffff
     };
     void setTraceFlags(uint32_t flags);
diff --git a/services/surfaceflinger/layerproto/layers.proto b/services/surfaceflinger/layerproto/layers.proto
index 8afe503..7f1f542 100644
--- a/services/surfaceflinger/layerproto/layers.proto
+++ b/services/surfaceflinger/layerproto/layers.proto
@@ -9,6 +9,23 @@
   repeated LayerProto layers = 1;
 }
 
+// Must match definition in the IComposerClient HAL
+enum HwcCompositionType {
+    // Invalid composition type
+    INVALID = 0;
+    // Layer was composited by the client into the client target buffer
+    CLIENT = 1;
+    // Layer was composited by the device through hardware overlays
+    DEVICE = 2;
+    // Layer was composited by the device using a color
+    SOLID_COLOR = 3;
+    // Similar to DEVICE, but the layer position may have been asynchronously set
+    // through setCursorPosition
+    CURSOR = 4;
+    // Layer was composited by the device via a sideband stream.
+    SIDEBAND = 5;
+}
+
 // Information about each layer.
 message LayerProto {
   // unique id per layer.
@@ -73,7 +90,7 @@
   int32 window_type = 33 [deprecated=true];
   int32 app_id = 34 [deprecated=true];
   // The layer's composition type
-  int32 hwc_composition_type = 35;
+  HwcCompositionType hwc_composition_type = 35;
   // If it's a buffer layer, indicate if the content is protected
   bool is_protected = 36;
   // Current frame number being rendered.
@@ -190,4 +207,4 @@
 message ColorTransformProto {
   // This will be a 4x4 matrix of float values
   repeated float val = 1;
-}
\ No newline at end of file
+}
diff --git a/services/surfaceflinger/layerproto/layerstrace.proto b/services/surfaceflinger/layerproto/layerstrace.proto
index bee17d2..ac33a0e 100644
--- a/services/surfaceflinger/layerproto/layerstrace.proto
+++ b/services/surfaceflinger/layerproto/layerstrace.proto
@@ -48,4 +48,7 @@
     optional string where = 2;
 
     optional LayersProto layers = 3;
+
+    // Blob for the current HWC information for all layers, reported by dumpsys.
+    optional string hwc_blob = 4;
 }
diff --git a/services/surfaceflinger/tests/unittests/LayerHistoryTestV2.cpp b/services/surfaceflinger/tests/unittests/LayerHistoryTestV2.cpp
index 959c256..37da76b 100644
--- a/services/surfaceflinger/tests/unittests/LayerHistoryTestV2.cpp
+++ b/services/surfaceflinger/tests/unittests/LayerHistoryTestV2.cpp
@@ -36,6 +36,7 @@
 protected:
     static constexpr auto PRESENT_TIME_HISTORY_SIZE = LayerInfoV2::HISTORY_SIZE;
     static constexpr auto MAX_FREQUENT_LAYER_PERIOD_NS = LayerInfoV2::MAX_FREQUENT_LAYER_PERIOD_NS;
+    static constexpr auto FREQUENT_LAYER_WINDOW_SIZE = LayerInfoV2::FREQUENT_LAYER_WINDOW_SIZE;
 
     static constexpr float LO_FPS = 30.f;
     static constexpr auto LO_FPS_PERIOD = static_cast<nsecs_t>(1e9f / LO_FPS);
@@ -451,5 +452,61 @@
     EXPECT_EQ(0, frequentLayerCount(time));
 }
 
+TEST_F(LayerHistoryTestV2, inactiveLayers) {
+    auto layer = createLayer();
+
+    EXPECT_CALL(*layer, isVisible()).WillRepeatedly(Return(true));
+    EXPECT_CALL(*layer, getFrameRate()).WillRepeatedly(Return(Layer::FrameRate()));
+
+    nsecs_t time = systemTime();
+
+    // the very first updates makes the layer frequent
+    for (int i = 0; i < FREQUENT_LAYER_WINDOW_SIZE - 1; i++) {
+        history().record(layer.get(), time, time);
+        time += MAX_FREQUENT_LAYER_PERIOD_NS.count();
+
+        EXPECT_EQ(1, layerCount());
+        ASSERT_EQ(1, history().summarize(time).size());
+        EXPECT_EQ(LayerHistory::LayerVoteType::Max, history().summarize(time)[0].vote);
+        EXPECT_EQ(1, activeLayerCount());
+        EXPECT_EQ(1, frequentLayerCount(time));
+    }
+
+    // the next update with the MAX_FREQUENT_LAYER_PERIOD_NS will get us to infrequent
+    history().record(layer.get(), time, time);
+    time += MAX_FREQUENT_LAYER_PERIOD_NS.count();
+
+    EXPECT_EQ(1, layerCount());
+    ASSERT_EQ(1, history().summarize(time).size());
+    EXPECT_EQ(LayerHistory::LayerVoteType::Min, history().summarize(time)[0].vote);
+    EXPECT_EQ(1, activeLayerCount());
+    EXPECT_EQ(0, frequentLayerCount(time));
+
+    // advance the time for the previous frame to be inactive
+    time += MAX_ACTIVE_LAYER_PERIOD_NS.count();
+
+    // Now event if we post a quick few frame we should stay infrequent
+    for (int i = 0; i < FREQUENT_LAYER_WINDOW_SIZE - 1; i++) {
+        history().record(layer.get(), time, time);
+        time += HI_FPS_PERIOD;
+
+        EXPECT_EQ(1, layerCount());
+        ASSERT_EQ(1, history().summarize(time).size());
+        EXPECT_EQ(LayerHistory::LayerVoteType::Min, history().summarize(time)[0].vote);
+        EXPECT_EQ(1, activeLayerCount());
+        EXPECT_EQ(0, frequentLayerCount(time));
+    }
+
+    // More quick frames will get us to frequent again
+    history().record(layer.get(), time, time);
+    time += HI_FPS_PERIOD;
+
+    EXPECT_EQ(1, layerCount());
+    ASSERT_EQ(1, history().summarize(time).size());
+    EXPECT_EQ(LayerHistory::LayerVoteType::Max, history().summarize(time)[0].vote);
+    EXPECT_EQ(1, activeLayerCount());
+    EXPECT_EQ(1, frequentLayerCount(time));
+}
+
 } // namespace
 } // namespace android::scheduler