From cbe1ed63e541777a46109bcb1240a45e678fb2ec Mon Sep 17 00:00:00 2001 From: Maciej Gibowicz Date: Fri, 27 Oct 2023 11:31:59 +0200 Subject: [PATCH] [BH-1801] Fix incorrect calculation of requested CPU frequency Sometimes when the frequency is locked, e.g. at 132 MHz, the algorithm still calculated a lower frequency, e.g. 66 MHz. --- harmony_changelog.md | 1 + module-sys/SystemManager/CpuGovernor.cpp | 2 +- module-sys/SystemManager/CpuLogPrinter.cpp | 4 ++-- module-sys/SystemManager/CpuPackPrinter.cpp | 4 ++-- module-sys/SystemManager/PowerManager.cpp | 4 ++-- .../SystemManager/cpu/AlgorithmFactory.cpp | 4 ++-- .../SystemManager/cpu/AlgorithmFactory.hpp | 4 ++-- .../cpu/algorithm/FrequencyStepping.cpp | 10 ++++------ .../cpu/algorithm/FrequencyStepping.hpp | 5 ++--- .../cpu/algorithm/ImmediateUpscale.cpp | 4 ++-- .../include/SystemManager/SentinelView.hpp | 4 ++-- .../tests/test-cpu-algorithms.cpp | 4 ++-- .../tests/unittest_CpuSentinelsGovernor.cpp | 18 +++++++++--------- pure_changelog.md | 1 + 14 files changed, 34 insertions(+), 35 deletions(-) diff --git a/harmony_changelog.md b/harmony_changelog.md index 4bf51b340dd29677675231b21879be225bbd15be..22e2a92b74c258f9e227b84c13e148f09047851c 100644 --- a/harmony_changelog.md +++ b/harmony_changelog.md @@ -10,6 +10,7 @@ * Fixed incorrect message after new alarm setting in some scenarios * Fixed frequency lock during user activity * Fixed possibility of OS crash during update package size check +* Fixed incorrect calculation of requested CPU frequency ### Added * Files not fully transferred via Center will be now removed when USB cable is unplugged diff --git a/module-sys/SystemManager/CpuGovernor.cpp b/module-sys/SystemManager/CpuGovernor.cpp index 8654f647ef371d6e17366e9206dacd66f28387e1..cb3fb56947cd59e1371f418350d3be07b61432ea 100644 --- a/module-sys/SystemManager/CpuGovernor.cpp +++ b/module-sys/SystemManager/CpuGovernor.cpp @@ -163,7 +163,7 @@ namespace sys return (*l).GetRequestedFrequency() < (*r).GetRequestedFrequency(); }); - d.frequency = (*minSentinel)->GetRequestedFrequency(); + d.minFrequency = (*minSentinel)->GetRequestedFrequency(); if (auto p = (*minSentinel)->GetSentinel().lock()) { d.name = p->GetName(); d.reason = p->getReason(); diff --git a/module-sys/SystemManager/CpuLogPrinter.cpp b/module-sys/SystemManager/CpuLogPrinter.cpp index ab27cc63c0bd332d7e54c78c95256a600e392211..ecc8921dbfc30dbafe36033942eaa0078a25d6ea 100644 --- a/module-sys/SystemManager/CpuLogPrinter.cpp +++ b/module-sys/SystemManager/CpuLogPrinter.cpp @@ -1,4 +1,4 @@ -// Copyright (c) 2017-2022, Mudita Sp. z.o.o. All rights reserved. +// Copyright (c) 2017-2023, Mudita Sp. z.o.o. All rights reserved. // For licensing, see https://github.com/mudita/MuditaOS/LICENSE.md #include "SystemManager/CpuPrinter.hpp" @@ -44,7 +44,7 @@ namespace sys::cpu::stats int(ret.frequencySet), ret.data.name.c_str(), ret.data.reason.c_str(), - int(ret.data.frequency), + int(ret.data.minFrequency), CLOCK_GetFreq(0)); } diff --git a/module-sys/SystemManager/CpuPackPrinter.cpp b/module-sys/SystemManager/CpuPackPrinter.cpp index 0044c54976dd2e417ac6f27219b5c19f055fec5d..5a6f167dd226d0560e47004afaa36117044886c0 100644 --- a/module-sys/SystemManager/CpuPackPrinter.cpp +++ b/module-sys/SystemManager/CpuPackPrinter.cpp @@ -1,4 +1,4 @@ -// Copyright (c) 2017-2022, Mudita Sp. z.o.o. All rights reserved. +// Copyright (c) 2017-2023, Mudita Sp. z.o.o. All rights reserved. // For licensing, see https://github.com/mudita/MuditaOS/LICENSE.md #include "SystemManager/CpuPrinter.hpp" @@ -67,7 +67,7 @@ namespace sys::cpu::stats {"freq", uint32_t(ret.frequencySet)}, {"name", ret.data.name}, {"reason", ret.data.reason}, - {"requested", uint32_t(ret.data.frequency)}, + {"requested", uint32_t(ret.data.minFrequency)}, {"avgA", int32_t(bsp::battery_charger::getAvgCurrent())}, {"nowA", int32_t(bsp::battery_charger::getCurrentMeasurement())}, {"ts", getTimestamp()}}; diff --git a/module-sys/SystemManager/PowerManager.cpp b/module-sys/SystemManager/PowerManager.cpp index fb7b2104dd5a0f58715d8530ba810816c88a8fbe..7acd341eda481f497f848045f387d1d10153eab2 100644 --- a/module-sys/SystemManager/PowerManager.cpp +++ b/module-sys/SystemManager/PowerManager.cpp @@ -64,7 +64,7 @@ namespace sys cpuAlgorithms = std::make_unique(); cpuAlgorithms->emplace(sys::cpu::AlgoID::ImmediateUpscale, std::make_unique()); cpuAlgorithms->emplace(sys::cpu::AlgoID::FrequencyStepping, - std::make_unique(powerProfile, *cpuGovernor)); + std::make_unique(powerProfile)); cpuFrequencyMonitor.push_back(CpuFrequencyMonitor(lowestLevelName)); cpuFrequencyMonitor.push_back(CpuFrequencyMonitor(middleLevelName)); @@ -106,7 +106,7 @@ namespace sys { uint32_t cpuLoad = cpuStatistics.GetPercentageCpuLoad(); cpu::UpdateResult retval; - cpu::AlgorithmData data{ + const cpu::AlgorithmData data{ cpuLoad, lowPowerControl->GetCurrentFrequencyLevel(), cpuGovernor->GetMinimumFrequencyRequested()}; auto _ = gsl::finally([&retval, this, data] { diff --git a/module-sys/SystemManager/cpu/AlgorithmFactory.cpp b/module-sys/SystemManager/cpu/AlgorithmFactory.cpp index 0e758f9b2876fd134659f28769d36e82f464521f..0247aaba59f2b5f4d00b469d1f9267a5cbebb209 100644 --- a/module-sys/SystemManager/cpu/AlgorithmFactory.cpp +++ b/module-sys/SystemManager/cpu/AlgorithmFactory.cpp @@ -1,4 +1,4 @@ -// Copyright (c) 2017-2022, Mudita Sp. z.o.o. All rights reserved. +// Copyright (c) 2017-2023, Mudita Sp. z.o.o. All rights reserved. // For licensing, see https://github.com/mudita/MuditaOS/LICENSE.md #include "AlgorithmFactory.hpp" @@ -32,7 +32,7 @@ namespace sys::cpu } AlgorithmResult AlgorithmFactory::calculate(const std::list &algorithms, - cpu::AlgorithmData &data, + const cpu::AlgorithmData &data, AlgoID *used) { for (auto id : algorithms) { diff --git a/module-sys/SystemManager/cpu/AlgorithmFactory.hpp b/module-sys/SystemManager/cpu/AlgorithmFactory.hpp index 5d788bb1d6d58ab77bf98f53d5309f1c268880df..6f3ce48b800cf2f1cd0c9eea283e72cc55ca4ed4 100644 --- a/module-sys/SystemManager/cpu/AlgorithmFactory.hpp +++ b/module-sys/SystemManager/cpu/AlgorithmFactory.hpp @@ -1,4 +1,4 @@ -// Copyright (c) 2017-2022, Mudita Sp. z.o.o. All rights reserved. +// Copyright (c) 2017-2023, Mudita Sp. z.o.o. All rights reserved. // For licensing, see https://github.com/mudita/MuditaOS/LICENSE.md #pragma once @@ -27,7 +27,7 @@ namespace sys::cpu /// use algorithms from factory depending on list to calculate frequency /// return used algorithm in used paramter AlgorithmResult calculate(const std::list &algorithms, - cpu::AlgorithmData &data, + const cpu::AlgorithmData &data, AlgoID *used = nullptr); /// reset internal algorithms data void reset(const std::list &algorithms); diff --git a/module-sys/SystemManager/cpu/algorithm/FrequencyStepping.cpp b/module-sys/SystemManager/cpu/algorithm/FrequencyStepping.cpp index 6f7628cc21789b54c015c71bcd619c30cbe4ce60..41ee2f2a10173f2b8e0d65e911b2334bececf7d2 100644 --- a/module-sys/SystemManager/cpu/algorithm/FrequencyStepping.cpp +++ b/module-sys/SystemManager/cpu/algorithm/FrequencyStepping.cpp @@ -2,13 +2,11 @@ // For licensing, see https://github.com/mudita/MuditaOS/LICENSE.md #include "FrequencyStepping.hpp" -#include "SystemManager/CpuGovernor.hpp" namespace sys::cpu { - FrequencyStepping::FrequencyStepping(const bsp::PowerProfile &powerProfile, CpuGovernor &cpuGovernor) - : powerProfile(powerProfile), cpuGovernor(cpuGovernor) + FrequencyStepping::FrequencyStepping(const bsp::PowerProfile &powerProfile) : powerProfile(powerProfile) {} bsp::CpuFrequencyMHz stepDown(bsp::CpuFrequencyMHz freq, const bsp::PowerProfile &profile) @@ -34,7 +32,7 @@ namespace sys::cpu { const auto load = data.CPUload; const auto startFrequency = data.curentFrequency; - const auto min = cpuGovernor.GetMinimumFrequencyRequested(); + const auto minFrequency = data.sentinel.minFrequency; if (load > powerProfile.frequencyShiftUpperThreshold && startFrequency < bsp::CpuFrequencyMHz::Level_6) { aboveThresholdCounter++; @@ -52,7 +50,7 @@ namespace sys::cpu isFrequencyDownscalingInProgress = false; } - if (min.frequency > startFrequency) {} + if (minFrequency > startFrequency) {} else if (aboveThresholdCounter >= powerProfile.maxAboveThresholdCount) { if (startFrequency < bsp::CpuFrequencyMHz::Level_4) { reset(); @@ -66,7 +64,7 @@ namespace sys::cpu else { if (belowThresholdCounter >= (isFrequencyDownscalingInProgress ? powerProfile.maxBelowThresholdInRowCount : powerProfile.maxBelowThresholdCount) && - startFrequency > min.frequency) { + startFrequency > minFrequency) { isFrequencyDownscalingInProgress = true; reset(); return {algorithm::Change::Downscaled, stepDown(startFrequency, powerProfile)}; diff --git a/module-sys/SystemManager/cpu/algorithm/FrequencyStepping.hpp b/module-sys/SystemManager/cpu/algorithm/FrequencyStepping.hpp index 7f71e8aa10638c091de2be19fceac8236d168872..ab4e594b9bc1bb6b7482070cb37a8ba8037892ab 100644 --- a/module-sys/SystemManager/cpu/algorithm/FrequencyStepping.hpp +++ b/module-sys/SystemManager/cpu/algorithm/FrequencyStepping.hpp @@ -1,4 +1,4 @@ -// Copyright (c) 2017-2022, Mudita Sp. z.o.o. All rights reserved. +// Copyright (c) 2017-2023, Mudita Sp. z.o.o. All rights reserved. // For licensing, see https://github.com/mudita/MuditaOS/LICENSE.md #pragma once @@ -16,13 +16,12 @@ namespace sys::cpu class FrequencyStepping : public Algorithm { const bsp::PowerProfile &powerProfile; - CpuGovernor &cpuGovernor; unsigned int aboveThresholdCounter = 0; unsigned int belowThresholdCounter = 0; bool isFrequencyDownscalingInProgress = true; public: - FrequencyStepping(const bsp::PowerProfile &powerProfile, CpuGovernor &cpuGovernor); + explicit FrequencyStepping(const bsp::PowerProfile &powerProfile); [[nodiscard]] AlgorithmResult calculateImplementation(const AlgorithmData &data) override; void resetImplementation() override; }; diff --git a/module-sys/SystemManager/cpu/algorithm/ImmediateUpscale.cpp b/module-sys/SystemManager/cpu/algorithm/ImmediateUpscale.cpp index 0438955acc49729c12e65075fe3efe157c959b44..b19d37752fa9994cb257377cf224a5dcc92e093f 100644 --- a/module-sys/SystemManager/cpu/algorithm/ImmediateUpscale.cpp +++ b/module-sys/SystemManager/cpu/algorithm/ImmediateUpscale.cpp @@ -1,4 +1,4 @@ -// Copyright (c) 2017-2022, Mudita Sp. z.o.o. All rights reserved. +// Copyright (c) 2017-2023, Mudita Sp. z.o.o. All rights reserved. // For licensing, see https://github.com/mudita/MuditaOS/LICENSE.md #include "ImmediateUpscale.hpp" @@ -7,7 +7,7 @@ namespace sys::cpu { AlgorithmResult ImmediateUpscale::calculateImplementation(const AlgorithmData &data) { - const auto now = data.sentinel.frequency; + const auto now = data.sentinel.minFrequency; const auto was = data.curentFrequency; if (now > was) { return {algorithm::Change::UpScaled, now}; diff --git a/module-sys/SystemManager/include/SystemManager/SentinelView.hpp b/module-sys/SystemManager/include/SystemManager/SentinelView.hpp index 4834884a35bf1dba738e8c141661246007049c23..023f2f4cda5702908d27ca4d7c8ed725972da35b 100644 --- a/module-sys/SystemManager/include/SystemManager/SentinelView.hpp +++ b/module-sys/SystemManager/include/SystemManager/SentinelView.hpp @@ -1,4 +1,4 @@ -// Copyright (c) 2017-2022, Mudita Sp. z.o.o. All rights reserved. +// Copyright (c) 2017-2023, Mudita Sp. z.o.o. All rights reserved. // For licensing, see https://github.com/mudita/MuditaOS/LICENSE.md #pragma once @@ -19,7 +19,7 @@ namespace sys /// name of sentinel thread responsible for curent minimum load std::string name; /// curent minimum frequency set in sentinel - bsp::CpuFrequencyMHz frequency = bsp::CpuFrequencyMHz::Level_0; + bsp::CpuFrequencyMHz minFrequency = bsp::CpuFrequencyMHz::Level_0; /// textual information on what actually happens std::string reason; }; diff --git a/module-sys/SystemManager/tests/test-cpu-algorithms.cpp b/module-sys/SystemManager/tests/test-cpu-algorithms.cpp index 68c952c9aeede2c3d15eb225604027a2040dd06f..a620e686d1892f72f2e261291927adeec18d9e0b 100644 --- a/module-sys/SystemManager/tests/test-cpu-algorithms.cpp +++ b/module-sys/SystemManager/tests/test-cpu-algorithms.cpp @@ -1,4 +1,4 @@ -// Copyright (c) 2017-2022, Mudita Sp. z.o.o. All rights reserved. +// Copyright (c) 2017-2023, Mudita Sp. z.o.o. All rights reserved. // For licensing, see https://github.com/mudita/MuditaOS/LICENSE.md #include @@ -34,7 +34,7 @@ namespace mockup auto cpuAlgorithms = std::make_unique(); cpuAlgorithms->emplace(sys::cpu::AlgoID::ImmediateUpscale, std::make_unique()); cpuAlgorithms->emplace(sys::cpu::AlgoID::FrequencyStepping, - std::make_unique(powerProfile, cpuGovernor)); + std::make_unique(powerProfile)); cpuAlgorithms->emplace(sys::cpu::AlgoID::FrequencyHold, std::make_unique(freq, powerProfile)); return cpuAlgorithms; diff --git a/module-sys/SystemManager/tests/unittest_CpuSentinelsGovernor.cpp b/module-sys/SystemManager/tests/unittest_CpuSentinelsGovernor.cpp index 8de531b39cf3786e227c437f4e46193f69bd1207..2d611a3d4ee5d36cafa186943f8c64654e92399d 100644 --- a/module-sys/SystemManager/tests/unittest_CpuSentinelsGovernor.cpp +++ b/module-sys/SystemManager/tests/unittest_CpuSentinelsGovernor.cpp @@ -1,4 +1,4 @@ -// Copyright (c) 2017-2022, Mudita Sp. z.o.o. All rights reserved. +// Copyright (c) 2017-2023, Mudita Sp. z.o.o. All rights reserved. // For licensing, see https://github.com/mudita/MuditaOS/LICENSE.md #include "SystemManager/GovernorSentinelOperations.hpp" @@ -68,28 +68,28 @@ TEST_CASE("Power Manager CPU sentinels governor test") governor->RegisterNewSentinel(testSentinel_1); governor->RegisterNewSentinel(testSentinel_2); - REQUIRE(governor->GetMinimumFrequencyRequested().frequency == bsp::CpuFrequencyMHz::Level_0); + REQUIRE(governor->GetMinimumFrequencyRequested().minFrequency == bsp::CpuFrequencyMHz::Level_0); governor->SetCpuFrequencyRequest("testSentinel_1", bsp::CpuFrequencyMHz::Level_4); - REQUIRE(governor->GetMinimumFrequencyRequested().frequency == bsp::CpuFrequencyMHz::Level_4); + REQUIRE(governor->GetMinimumFrequencyRequested().minFrequency == bsp::CpuFrequencyMHz::Level_4); governor->SetCpuFrequencyRequest("testSentinel_2", bsp::CpuFrequencyMHz::Level_6); - REQUIRE(governor->GetMinimumFrequencyRequested().frequency == bsp::CpuFrequencyMHz::Level_6); + REQUIRE(governor->GetMinimumFrequencyRequested().minFrequency == bsp::CpuFrequencyMHz::Level_6); governor->SetCpuFrequencyRequest("testSentinel_1", bsp::CpuFrequencyMHz::Level_2); - REQUIRE(governor->GetMinimumFrequencyRequested().frequency == bsp::CpuFrequencyMHz::Level_6); + REQUIRE(governor->GetMinimumFrequencyRequested().minFrequency == bsp::CpuFrequencyMHz::Level_6); governor->ResetCpuFrequencyRequest("testSentinel_2"); - REQUIRE(governor->GetMinimumFrequencyRequested().frequency == bsp::CpuFrequencyMHz::Level_2); + REQUIRE(governor->GetMinimumFrequencyRequested().minFrequency == bsp::CpuFrequencyMHz::Level_2); governor->SetCpuFrequencyRequest("badNameSentinel", bsp::CpuFrequencyMHz::Level_6); - REQUIRE(governor->GetMinimumFrequencyRequested().frequency == bsp::CpuFrequencyMHz::Level_2); + REQUIRE(governor->GetMinimumFrequencyRequested().minFrequency == bsp::CpuFrequencyMHz::Level_2); governor->SetCpuFrequencyRequest("testSentinel_1", bsp::CpuFrequencyMHz::Level_1); - REQUIRE(governor->GetMinimumFrequencyRequested().frequency == bsp::CpuFrequencyMHz::Level_1); + REQUIRE(governor->GetMinimumFrequencyRequested().minFrequency == bsp::CpuFrequencyMHz::Level_1); governor->ResetCpuFrequencyRequest("testSentinel_1"); - REQUIRE(governor->GetMinimumFrequencyRequested().frequency == bsp::CpuFrequencyMHz::Level_0); + REQUIRE(governor->GetMinimumFrequencyRequested().minFrequency == bsp::CpuFrequencyMHz::Level_0); } } diff --git a/pure_changelog.md b/pure_changelog.md index cb26762a0e79cd1cc306b78ba04d7db7af5686ed..3991848c37b5bab6ed725a835b4f0a6cb8901d1e 100644 --- a/pure_changelog.md +++ b/pure_changelog.md @@ -13,6 +13,7 @@ ### Fixed * Fixed possible crash when entering phone number +* Fixed incorrect calculation of requested CPU frequency ## [1.9.0 2023-10-19]