Merge "SurfaceFlinger: Dump on layer leaks" into tm-dev
diff --git a/libs/renderengine/skia/ColorSpaces.cpp b/libs/renderengine/skia/ColorSpaces.cpp
index f367a84..37ff5df 100644
--- a/libs/renderengine/skia/ColorSpaces.cpp
+++ b/libs/renderengine/skia/ColorSpaces.cpp
@@ -65,7 +65,15 @@
case HAL_DATASPACE_TRANSFER_SMPTE_170M:
return SkColorSpace::MakeRGB(SkNamedTransferFn::kRec2020, gamut);
case HAL_DATASPACE_TRANSFER_HLG:
- return SkColorSpace::MakeRGB(SkNamedTransferFn::kHLG, gamut);
+ // return HLG transfer but scale by 1/12
+ skcms_TransferFunction hlgFn;
+ if (skcms_TransferFunction_makeScaledHLGish(&hlgFn, 1.f / 12.f, 2.f, 2.f,
+ 1.f / 0.17883277f, 0.28466892f,
+ 0.55991073f)) {
+ return SkColorSpace::MakeRGB(hlgFn, gamut);
+ } else {
+ return SkColorSpace::MakeRGB(SkNamedTransferFn::kSRGB, gamut);
+ }
case HAL_DATASPACE_TRANSFER_UNSPECIFIED:
default:
return SkColorSpace::MakeRGB(SkNamedTransferFn::kSRGB, gamut);
diff --git a/services/gpuservice/gpuwork/bpfprogs/gpu_work.c b/services/gpuservice/gpuwork/bpfprogs/gpu_work.c
index c8e79a2..95cfae7 100644
--- a/services/gpuservice/gpuwork/bpfprogs/gpu_work.c
+++ b/services/gpuservice/gpuwork/bpfprogs/gpu_work.c
@@ -71,11 +71,14 @@
//
// The |total_active_duration_ns| must be set to the approximate total amount of
// time the GPU spent running work for |uid| within the period, without
-// "double-counting" parallel GPU work on the same GPU for the same |uid|. "GPU
-// work" should correspond to the "GPU slices" shown in the AGI (Android GPU
+// "double-counting" parallel GPU work on the same GPU for the same |uid|. Note
+// that even if the parallel GPU work was submitted from several different
+// processes (i.e. different PIDs) with the same UID, this overlapping work must
+// not be double-counted, as it still came from a single |uid|. "GPU work"
+// should correspond to the "GPU slices" shown in the AGI (Android GPU
// Inspector) tool, and so should include work such as fragment and non-fragment
// work/shaders running on the shader cores of the GPU. For example, given the
-// following:
+// following for a single |uid|:
// - A period has:
// - |start_time_ns|: 100,000,000 ns
// - |end_time_ns|: 800,000,000 ns
@@ -99,16 +102,20 @@
// - from 600,000,000 ns to 700,000,000 ns, giving a duration of 100,000,000 ns
// (GPU work D)
//
-// Thus, the |total_active_duration_ns| is the sum of the (non-overlapping)
-// durations. Drivers may not have efficient access to the exact start and end
-// times of all GPU work, as shown above, but drivers should try to
-// approximate/aggregate the value of |total_active_duration_ns| as accurately
-// as possible within the limitations of the hardware, without double-counting
-// parallel GPU work for the same |uid|. The |total_active_duration_ns| value
-// must be less than or equal to the period duration (|end_time_ns| -
-// |start_time_ns|); if the aggregation approach might violate this requirement
-// then the driver must clamp |total_active_duration_ns| to be at most the
-// period duration.
+// Thus, the |total_active_duration_ns| is the sum of these two
+// (non-overlapping) durations. Drivers may not have efficient access to the
+// exact start and end times of all GPU work, as shown above, but drivers should
+// try to approximate/aggregate the value of |total_active_duration_ns| as
+// accurately as possible within the limitations of the hardware, without
+// double-counting parallel GPU work for the same |uid|. The
+// |total_active_duration_ns| value must be less than or equal to the period
+// duration (|end_time_ns| - |start_time_ns|); if the aggregation approach might
+// violate this requirement then the driver must clamp
+// |total_active_duration_ns| to be at most the period duration.
+//
+// Protected mode: protected GPU work must not be reported. Periods must be
+// emitted, and the |total_active_duration_ns| value set, as if the protected
+// GPU work did not occur.
//
// Note that the above description allows for a certain amount of flexibility in
// how the driver tracks periods and emits the events. We list a few examples of
diff --git a/services/surfaceflinger/DisplayHardware/PowerAdvisor.cpp b/services/surfaceflinger/DisplayHardware/PowerAdvisor.cpp
index 05f488b..cbafdd3 100644
--- a/services/surfaceflinger/DisplayHardware/PowerAdvisor.cpp
+++ b/services/surfaceflinger/DisplayHardware/PowerAdvisor.cpp
@@ -463,29 +463,41 @@
ALOGV("Failed to send actual work duration, skipping");
return;
}
-
- WorkDuration duration;
- duration.durationNanos = actualDurationNanos;
- mActualDuration = actualDurationNanos;
+ nsecs_t reportedDuration = actualDurationNanos;
// normalize the sent values to a pre-set target
if (sNormalizeTarget) {
- duration.durationNanos += mLastTargetDurationSent - mTargetDuration;
+ reportedDuration += mLastTargetDurationSent - mTargetDuration;
+ } else {
+ // when target duration change is within deviation and not updated, adjust the actual
+ // duration proportionally based on the difference, e.g. if new target is 5ms longer than
+ // last reported but actual duration is the same as last target, we want to report a smaller
+ // actual work duration now to indicate that we are overshooting
+ if (mLastTargetDurationSent != kDefaultTarget.count() && mTargetDuration != 0) {
+ reportedDuration =
+ static_cast<int64_t>(static_cast<long double>(mLastTargetDurationSent) /
+ mTargetDuration * actualDurationNanos);
+ mActualDuration = reportedDuration;
+ }
}
+ mActualDuration = reportedDuration;
+ WorkDuration duration;
+ duration.durationNanos = reportedDuration;
duration.timeStampNanos = timeStampNanos;
mPowerHintQueue.push_back(duration);
- nsecs_t targetNsec = mTargetDuration;
- nsecs_t durationNsec = actualDurationNanos;
-
if (sTraceHintSessionData) {
- ATRACE_INT64("Measured duration", durationNsec);
- ATRACE_INT64("Target error term", targetNsec - durationNsec);
+ ATRACE_INT64("Measured duration", actualDurationNanos);
+ ATRACE_INT64("Target error term", mTargetDuration - actualDurationNanos);
+
+ ATRACE_INT64("Reported duration", reportedDuration);
+ ATRACE_INT64("Reported target", mLastTargetDurationSent);
+ ATRACE_INT64("Reported target error term", mLastTargetDurationSent - reportedDuration);
}
- ALOGV("Sending actual work duration of: %" PRId64 " on target: %" PRId64
+ ALOGV("Sending actual work duration of: %" PRId64 " on reported target: %" PRId64
" with error: %" PRId64,
- durationNsec, targetNsec, targetNsec - durationNsec);
+ reportedDuration, mLastTargetDurationSent, mLastTargetDurationSent - reportedDuration);
// This rate limiter queues similar duration reports to the powerhal into
// batches to avoid excessive binder calls. The criteria to send a given batch
@@ -501,7 +513,7 @@
}
mPowerHintQueue.clear();
// we save the non-normalized value here to detect % changes
- mLastActualDurationSent = actualDurationNanos;
+ mLastActualDurationSent = reportedDuration;
}
}
diff --git a/services/surfaceflinger/DisplayHardware/PowerAdvisor.h b/services/surfaceflinger/DisplayHardware/PowerAdvisor.h
index 3f47ffd..61bb32b 100644
--- a/services/surfaceflinger/DisplayHardware/PowerAdvisor.h
+++ b/services/surfaceflinger/DisplayHardware/PowerAdvisor.h
@@ -173,8 +173,8 @@
// Max percent the actual duration can vary without causing a report (eg: 0.1 = 10%)
static constexpr double kAllowedActualDeviationPercent = 0.1;
- // Max percent the target duration can vary without causing a report (eg: 0.05 = 5%)
- static constexpr double kAllowedTargetDeviationPercent = 0.05;
+ // Max percent the target duration can vary without causing a report (eg: 0.1 = 10%)
+ static constexpr double kAllowedTargetDeviationPercent = 0.1;
// Target used for init and normalization, the actual value does not really matter
static constexpr const std::chrono::nanoseconds kDefaultTarget = 50ms;
// Amount of time after the last message was sent before the session goes stale
diff --git a/services/surfaceflinger/tests/unittests/AidlPowerHalWrapperTest.cpp b/services/surfaceflinger/tests/unittests/AidlPowerHalWrapperTest.cpp
index e25a0ae..9ab35d7 100644
--- a/services/surfaceflinger/tests/unittests/AidlPowerHalWrapperTest.cpp
+++ b/services/surfaceflinger/tests/unittests/AidlPowerHalWrapperTest.cpp
@@ -17,6 +17,7 @@
#undef LOG_TAG
#define LOG_TAG "AidlPowerHalWrapperTest"
+#include <android-base/stringprintf.h>
#include <android/hardware/power/IPower.h>
#include <android/hardware/power/IPowerHintSession.h>
#include <gmock/gmock.h>
@@ -82,6 +83,15 @@
return duration;
}
+std::string printWorkDurations(const ::std::vector<WorkDuration>& durations) {
+ std::ostringstream os;
+ for (auto duration : durations) {
+ os << duration.toString();
+ os << "\n";
+ }
+ return os.str();
+}
+
namespace {
TEST_F(AidlPowerHalWrapperTest, supportsPowerHintSession) {
ASSERT_TRUE(mWrapper->supportsPowerHintSession());
@@ -143,8 +153,8 @@
{-1ms, false},
{200ms, true},
{2ms, true},
- {96ms, false},
- {104ms, false}};
+ {91ms, false},
+ {109ms, false}};
for (const auto& test : testCases) {
// reset to 100ms baseline
@@ -212,6 +222,40 @@
}
}
+TEST_F(AidlPowerHalWrapperTest, sendAdjustedActualWorkDuration) {
+ ASSERT_TRUE(mWrapper->supportsPowerHintSession());
+
+ std::vector<int32_t> threadIds = {1, 2};
+ mWrapper->setPowerHintSessionThreadIds(threadIds);
+ EXPECT_CALL(*mMockHal.get(), createHintSession(_, _, threadIds, _, _))
+ .WillOnce(DoAll(SetArgPointee<4>(mMockSession), Return(Status::ok())));
+ ASSERT_TRUE(mWrapper->startPowerHintSession());
+ verifyAndClearExpectations();
+
+ std::chrono::nanoseconds lastTarget = 100ms;
+ EXPECT_CALL(*mMockSession.get(), updateTargetWorkDuration(lastTarget.count())).Times(1);
+ mWrapper->setTargetWorkDuration(lastTarget.count());
+ std::chrono::nanoseconds newTarget = 105ms;
+ mWrapper->setTargetWorkDuration(newTarget.count());
+ EXPECT_CALL(*mMockSession.get(), updateTargetWorkDuration(newTarget.count())).Times(0);
+ std::chrono::nanoseconds actual = 21ms;
+ // 100 / 105 * 21ms = 20ms
+ std::chrono::nanoseconds expectedActualSent = 20ms;
+ std::vector<WorkDuration> expectedDurations = {toWorkDuration(expectedActualSent, 1)};
+
+ EXPECT_CALL(*mMockSession.get(), reportActualWorkDuration(_))
+ .WillOnce(DoAll(
+ [expectedDurations](const ::std::vector<WorkDuration>& durationsSent) {
+ EXPECT_EQ(expectedDurations, durationsSent)
+ << base::StringPrintf("actual sent: %s vs expected: %s",
+ printWorkDurations(durationsSent).c_str(),
+ printWorkDurations(expectedDurations)
+ .c_str());
+ },
+ Return(Status::ok())));
+ mWrapper->sendActualWorkDuration(actual.count(), 1);
+}
+
TEST_F(AidlPowerHalWrapperTest, sendActualWorkDuration_exceedsStaleTime) {
ASSERT_TRUE(mWrapper->supportsPowerHintSession());