From 391f601be6eab934b155251255c4c64d174d8805 Mon Sep 17 00:00:00 2001 From: Darren Hsu Date: Wed, 27 Apr 2022 01:10:12 +0800 Subject: [PATCH 1/4] powerstats: Introduce AoC timed data provider Reading data from an AoC sysfs node by getline takes 1 second. In total there are 17 AoC sysfs nodes that must be read. The worst case is taking 17 seconds long that is greater than dumpsys timeout. Therefore, we need the timeout mechanism to ignore the AoC power stats reporting when AoC latency exceeds the allowed time. Bug: 219630658 Test: dumpsys android.hardware.power.stats.IPowerStats/default Change-Id: Ic5c2a0b36728153fd2e2593599a8f2bcdb50ace4 Signed-off-by: Darren Hsu --- .../AocTimedStateResidencyDataProvider.cpp | 120 ++++++++++++++++++ .../include/AocStateResidencyDataProvider.h | 4 +- .../AocTimedStateResidencyDataProvider.h | 64 ++++++++++ 3 files changed, 186 insertions(+), 2 deletions(-) create mode 100644 powerstats/AocTimedStateResidencyDataProvider.cpp create mode 100644 powerstats/include/AocTimedStateResidencyDataProvider.h diff --git a/powerstats/AocTimedStateResidencyDataProvider.cpp b/powerstats/AocTimedStateResidencyDataProvider.cpp new file mode 100644 index 0000000..ca427f6 --- /dev/null +++ b/powerstats/AocTimedStateResidencyDataProvider.cpp @@ -0,0 +1,120 @@ +/* + * Copyright (C) 2022 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. + */ + +#include "AocTimedStateResidencyDataProvider.h" + +#include +#include + +namespace aidl { +namespace android { +namespace hardware { +namespace power { +namespace stats { + +AocTimedStateResidencyDataProvider::AocTimedStateResidencyDataProvider( + std::vector> ids, + std::vector> states, + const uint64_t timeoutMillis) + : AocStateResidencyDataProvider(ids, states) { + static const uint64_t DEFAULT_MAX_TIME_PER_STATE_MILLIS = 120; + + mTimeoutMillis = + timeoutMillis == 0 ? DEFAULT_MAX_TIME_PER_STATE_MILLIS * states.size() : timeoutMillis; + + mAsyncThread = std::thread(&AocTimedStateResidencyDataProvider::getStateResidenciesAsync, this); +} + +bool AocTimedStateResidencyDataProvider::getStateResidencies( + std::unordered_map> *residencies) { + bool ret = true; + std::unique_lock statusLock(mStatusMutex); + + if (mAsyncStatus != COMPLETED) { + LOG(ERROR) << "The async thread is not ready: " << mAsyncStatus; + return false; + } + + mStateResidencies.clear(); + + mAsyncStatus = RUN; + mRunCond.notify_one(); + + auto timeout = std::chrono::steady_clock::now() + std::chrono::milliseconds(mTimeoutMillis); + auto isCompleted = + mCompletedCond.wait_until(statusLock, timeout, [this]{ return mAsyncStatus == COMPLETED; }); + + if (isCompleted) { + for (const auto &residency : mStateResidencies) { + residencies->emplace(residency.first, residency.second); + } + } else { + LOG(ERROR) << __func__ << " for AoC timed out: " << mTimeoutMillis << " ms"; + ret = false; + } + + return ret; +} + +void AocTimedStateResidencyDataProvider::getStateResidenciesAsync() { + std::unique_lock statusLock(mStatusMutex); + + mAsyncStatus = COMPLETED; + + while (1) { + mRunCond.wait(statusLock, [this]{ return mAsyncStatus == RUN; }); + + mAsyncStatus = RUNNING; + statusLock.unlock(); + + // States from the same power entity are merged. + for (const auto &providerList : mProviders) { + int32_t stateId = 0; + std::string curEntity = providerList.first; + std::vector stateResidencies; + + // Iterate over each provider in the providerList, appending each of the states + for (const auto &provider : providerList.second) { + std::unordered_map> residency; + provider->getStateResidencies(&residency); + + // Each provider should only return data for curEntity but checking anyway + if (residency.find(curEntity) != residency.end()) { + for (auto &r : residency.at(curEntity)) { + /* + * Modifying stateId here because we are stitching together infos from + * multiple GenericStateResidencyDataProviders. stateId must be modified + * to maintain uniqueness for a given entity + */ + r.id = stateId++; + stateResidencies.push_back(r); + } + } + } + mStateResidencies.emplace(curEntity, stateResidencies); + } + + statusLock.lock(); + mAsyncStatus = COMPLETED; + mCompletedCond.notify_one(); + } // while loop +} + +} // namespace stats +} // namespace power +} // namespace hardware +} // namespace android +} // namespace aidl diff --git a/powerstats/include/AocStateResidencyDataProvider.h b/powerstats/include/AocStateResidencyDataProvider.h index 5008912..f02b911 100644 --- a/powerstats/include/AocStateResidencyDataProvider.h +++ b/powerstats/include/AocStateResidencyDataProvider.h @@ -33,7 +33,7 @@ class AocStateResidencyDataProvider : public PowerStats::IStateResidencyDataProv std::unordered_map> *residencies) override; std::unordered_map> getInfo() override; - private: + protected: std::unordered_map> /* providers */> mProviders; }; @@ -42,4 +42,4 @@ class AocStateResidencyDataProvider : public PowerStats::IStateResidencyDataProv } // namespace power } // namespace hardware } // namespace android -} // namespace aidl \ No newline at end of file +} // namespace aidl diff --git a/powerstats/include/AocTimedStateResidencyDataProvider.h b/powerstats/include/AocTimedStateResidencyDataProvider.h new file mode 100644 index 0000000..98724b2 --- /dev/null +++ b/powerstats/include/AocTimedStateResidencyDataProvider.h @@ -0,0 +1,64 @@ +/* + * Copyright (C) 2022 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. + */ +#pragma once + +#include "AocStateResidencyDataProvider.h" + +#include +#include +#include +#include + +namespace aidl { +namespace android { +namespace hardware { +namespace power { +namespace stats { + +enum AsyncStatus { + COMPLETED, + RUN, + RUNNING +}; + +class AocTimedStateResidencyDataProvider : public AocStateResidencyDataProvider { + public: + AocTimedStateResidencyDataProvider( + std::vector> ids, + std::vector> states, + const uint64_t timeoutMillis); + ~AocTimedStateResidencyDataProvider() = default; + + bool getStateResidencies( + std::unordered_map> *residencies) override; + + private: + void getStateResidenciesAsync(); + + uint64_t mTimeoutMillis; + std::thread mAsyncThread; + std::mutex mStatusMutex; + std::condition_variable mRunCond; + std::condition_variable mCompletedCond; + std::unordered_map> mStateResidencies; + AsyncStatus mAsyncStatus; +}; + +} // namespace stats +} // namespace power +} // namespace hardware +} // namespace android +} // namespace aidl From 9f4ac2c565cbc1146792c6ddc3717bb6aa094341 Mon Sep 17 00:00:00 2001 From: Craig Dooley Date: Wed, 25 May 2022 21:21:04 +0000 Subject: [PATCH 2/4] Use libaoc to provide consistent timestamp calculations Bug: 233923713 Change-Id: Iacd541815f40adef879e05d9bd8ded056e9a760d --- powerstats/Android.bp | 1 + powerstats/AocStateResidencyDataProvider.cpp | 9 ++++----- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/powerstats/Android.bp b/powerstats/Android.bp index 74935c7..2dbf8db 100644 --- a/powerstats/Android.bp +++ b/powerstats/Android.bp @@ -20,5 +20,6 @@ cc_library { shared_libs: [ "android.hardware.power.stats-impl.pixel", + "libaoc", ], } diff --git a/powerstats/AocStateResidencyDataProvider.cpp b/powerstats/AocStateResidencyDataProvider.cpp index c64496d..6ab54db 100644 --- a/powerstats/AocStateResidencyDataProvider.cpp +++ b/powerstats/AocStateResidencyDataProvider.cpp @@ -18,6 +18,8 @@ #include +#include + namespace aidl { namespace android { namespace hardware { @@ -26,11 +28,8 @@ namespace stats { AocStateResidencyDataProvider::AocStateResidencyDataProvider(std::vector> ids, std::vector> states) { - // AoC stats are reported in ticks of 244.140625ns. The transform - // function converts ticks to milliseconds. - // 1000000 / 244.140625 = 4096. - static const uint64_t AOC_CLK = 4096; - std::function aocTickToMs = [](uint64_t a) { return a / AOC_CLK; }; + // AoC stats are reported in ticks + std::function aocTickToMs = [](uint64_t a) { return aoc_ticks_to_nanoseconds(a) / 1000000; }; GenericStateResidencyDataProvider::StateResidencyConfig config = { .entryCountSupported = true, .entryCountPrefix = "Counter:", From 33552445b8237b1222f50295c3af478321ea5067 Mon Sep 17 00:00:00 2001 From: Sam Dubey Date: Fri, 10 Jun 2022 03:23:56 +0000 Subject: [PATCH 3/4] Revert "Use libaoc to provide consistent timestamp calculations" Revert "Allow libaoc to be used in more contexts" Revert submission 18605412-b-233923713 Reason for revert: Completely broke master-without-vendor Bug: 235533674 Reverted Changes: Iacd541815:Use libaoc to provide consistent timestamp calcula... Id31ee3439:Allow libaoc to be used in more contexts Change-Id: I3345d0e4baa8d5dce22be20f0213ad1421ce1a90 --- powerstats/Android.bp | 1 - powerstats/AocStateResidencyDataProvider.cpp | 9 +++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/powerstats/Android.bp b/powerstats/Android.bp index 2dbf8db..74935c7 100644 --- a/powerstats/Android.bp +++ b/powerstats/Android.bp @@ -20,6 +20,5 @@ cc_library { shared_libs: [ "android.hardware.power.stats-impl.pixel", - "libaoc", ], } diff --git a/powerstats/AocStateResidencyDataProvider.cpp b/powerstats/AocStateResidencyDataProvider.cpp index 6ab54db..c64496d 100644 --- a/powerstats/AocStateResidencyDataProvider.cpp +++ b/powerstats/AocStateResidencyDataProvider.cpp @@ -18,8 +18,6 @@ #include -#include - namespace aidl { namespace android { namespace hardware { @@ -28,8 +26,11 @@ namespace stats { AocStateResidencyDataProvider::AocStateResidencyDataProvider(std::vector> ids, std::vector> states) { - // AoC stats are reported in ticks - std::function aocTickToMs = [](uint64_t a) { return aoc_ticks_to_nanoseconds(a) / 1000000; }; + // AoC stats are reported in ticks of 244.140625ns. The transform + // function converts ticks to milliseconds. + // 1000000 / 244.140625 = 4096. + static const uint64_t AOC_CLK = 4096; + std::function aocTickToMs = [](uint64_t a) { return a / AOC_CLK; }; GenericStateResidencyDataProvider::StateResidencyConfig config = { .entryCountSupported = true, .entryCountPrefix = "Counter:", From 2756ac31ffa500ed19b1408be9a011a3daff881e Mon Sep 17 00:00:00 2001 From: Darren Hsu Date: Fri, 17 Jun 2022 20:10:29 +0800 Subject: [PATCH 4/4] powerstats: add new parameter to provide aoc clock AoC power stats are reported in ticks that are different frequency in different SoC. Bug: 233923713 Test: dumpsys android.hardware.power.stats.IPowerStats/default Change-Id: I047d7ae163941c2168dcd8a0ea34da73ab4a8477 Merged-In: I047d7ae163941c2168dcd8a0ea34da73ab4a8477 Signed-off-by: Darren Hsu --- powerstats/AocStateResidencyDataProvider.cpp | 12 ++++++------ powerstats/AocTimedStateResidencyDataProvider.cpp | 5 +++-- powerstats/include/AocStateResidencyDataProvider.h | 3 ++- .../include/AocTimedStateResidencyDataProvider.h | 3 ++- 4 files changed, 13 insertions(+), 10 deletions(-) diff --git a/powerstats/AocStateResidencyDataProvider.cpp b/powerstats/AocStateResidencyDataProvider.cpp index c64496d..38cd85f 100644 --- a/powerstats/AocStateResidencyDataProvider.cpp +++ b/powerstats/AocStateResidencyDataProvider.cpp @@ -24,12 +24,12 @@ namespace hardware { namespace power { namespace stats { -AocStateResidencyDataProvider::AocStateResidencyDataProvider(std::vector> ids, std::vector> states) { - // AoC stats are reported in ticks of 244.140625ns. The transform - // function converts ticks to milliseconds. - // 1000000 / 244.140625 = 4096. - static const uint64_t AOC_CLK = 4096; +AocStateResidencyDataProvider::AocStateResidencyDataProvider( + std::vector> ids, + std::vector> states, + const uint64_t aocClock) { + // AoC stats are reported in ticks. + static const uint64_t AOC_CLK = aocClock; std::function aocTickToMs = [](uint64_t a) { return a / AOC_CLK; }; GenericStateResidencyDataProvider::StateResidencyConfig config = { .entryCountSupported = true, diff --git a/powerstats/AocTimedStateResidencyDataProvider.cpp b/powerstats/AocTimedStateResidencyDataProvider.cpp index ca427f6..ead2a42 100644 --- a/powerstats/AocTimedStateResidencyDataProvider.cpp +++ b/powerstats/AocTimedStateResidencyDataProvider.cpp @@ -28,8 +28,9 @@ namespace stats { AocTimedStateResidencyDataProvider::AocTimedStateResidencyDataProvider( std::vector> ids, std::vector> states, - const uint64_t timeoutMillis) - : AocStateResidencyDataProvider(ids, states) { + const uint64_t timeoutMillis, + const uint64_t aocClock) + : AocStateResidencyDataProvider(ids, states, aocClock) { static const uint64_t DEFAULT_MAX_TIME_PER_STATE_MILLIS = 120; mTimeoutMillis = diff --git a/powerstats/include/AocStateResidencyDataProvider.h b/powerstats/include/AocStateResidencyDataProvider.h index f02b911..708ea4c 100644 --- a/powerstats/include/AocStateResidencyDataProvider.h +++ b/powerstats/include/AocStateResidencyDataProvider.h @@ -27,7 +27,8 @@ namespace stats { class AocStateResidencyDataProvider : public PowerStats::IStateResidencyDataProvider { public: AocStateResidencyDataProvider(std::vector> ids, - std::vector> states); + std::vector> states, + const uint64_t aocClock); ~AocStateResidencyDataProvider() = default; bool getStateResidencies( std::unordered_map> *residencies) override; diff --git a/powerstats/include/AocTimedStateResidencyDataProvider.h b/powerstats/include/AocTimedStateResidencyDataProvider.h index 98724b2..8611ae6 100644 --- a/powerstats/include/AocTimedStateResidencyDataProvider.h +++ b/powerstats/include/AocTimedStateResidencyDataProvider.h @@ -39,7 +39,8 @@ class AocTimedStateResidencyDataProvider : public AocStateResidencyDataProvider AocTimedStateResidencyDataProvider( std::vector> ids, std::vector> states, - const uint64_t timeoutMillis); + const uint64_t timeoutMillis, + const uint64_t aocClock); ~AocTimedStateResidencyDataProvider() = default; bool getStateResidencies(