Merge "Blast: Use a unique id to track buffers" into sc-dev
diff --git a/services/surfaceflinger/BufferStateLayer.cpp b/services/surfaceflinger/BufferStateLayer.cpp
index 8cf0d66..2a49a0a 100644
--- a/services/surfaceflinger/BufferStateLayer.cpp
+++ b/services/surfaceflinger/BufferStateLayer.cpp
@@ -718,7 +718,7 @@
bool BufferStateLayer::hasFrameUpdate() const {
const State& c(getDrawingState());
- return mDrawingStateModified && (c.buffer != nullptr || c.bgColorLayer != nullptr);
+ return (mDrawingStateModified || mDrawingState.modified) && (c.buffer != nullptr || c.bgColorLayer != nullptr);
}
status_t BufferStateLayer::updateTexImage(bool& /*recomputeVisibleRegions*/, nsecs_t latchTime,
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/CachedSet.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/CachedSet.h
index 7cb0f6b..2bf931c 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/CachedSet.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/CachedSet.h
@@ -141,7 +141,7 @@
compositionengine::OutputLayer* getBlurLayer() const;
- bool hasHdrLayers() const;
+ bool hasUnsupportedDataspace() const;
bool hasProtectedLayers() const;
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/LayerState.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/LayerState.h
index a20d7b3..bce438f 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/LayerState.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/LayerState.h
@@ -242,13 +242,6 @@
ui::Dataspace getDataspace() const { return mOutputDataspace.get(); }
- bool isHdr() const {
- const ui::Dataspace transfer =
- static_cast<ui::Dataspace>(getDataspace() & ui::Dataspace::TRANSFER_MASK);
- return (transfer == ui::Dataspace::TRANSFER_ST2084 ||
- transfer == ui::Dataspace::TRANSFER_HLG);
- }
-
bool isProtected() const {
return getOutputLayer()->getLayerFE().getCompositionState()->hasProtectedContent;
}
diff --git a/services/surfaceflinger/CompositionEngine/src/planner/CachedSet.cpp b/services/surfaceflinger/CompositionEngine/src/planner/CachedSet.cpp
index acc7ed2..b24274e 100644
--- a/services/surfaceflinger/CompositionEngine/src/planner/CachedSet.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/planner/CachedSet.cpp
@@ -295,9 +295,7 @@
return false;
}
- // Do not use a hole punch with an HDR layer; this should be done in client
- // composition to properly mix HDR with SDR.
- if (hasHdrLayers()) {
+ if (hasUnsupportedDataspace()) {
return false;
}
@@ -352,9 +350,22 @@
return mBlurLayer ? mBlurLayer->getOutputLayer() : nullptr;
}
-bool CachedSet::hasHdrLayers() const {
- return std::any_of(mLayers.cbegin(), mLayers.cend(),
- [](const Layer& layer) { return layer.getState()->isHdr(); });
+bool CachedSet::hasUnsupportedDataspace() const {
+ return std::any_of(mLayers.cbegin(), mLayers.cend(), [](const Layer& layer) {
+ auto dataspace = layer.getState()->getDataspace();
+ const auto transfer = static_cast<ui::Dataspace>(dataspace & ui::Dataspace::TRANSFER_MASK);
+ if (transfer == ui::Dataspace::TRANSFER_ST2084 || transfer == ui::Dataspace::TRANSFER_HLG) {
+ // Skip HDR.
+ return true;
+ }
+
+ if ((dataspace & HAL_DATASPACE_STANDARD_MASK) == HAL_DATASPACE_STANDARD_BT601_625) {
+ // RenderEngine does not match some DPUs, so skip
+ // to avoid flickering/color differences.
+ return true;
+ }
+ return false;
+ });
}
bool CachedSet::hasProtectedLayers() const {
@@ -378,10 +389,14 @@
if (mLayers.size() == 1) {
base::StringAppendF(&result, " Layer [%s]\n", mLayers[0].getName().c_str());
base::StringAppendF(&result, " Buffer %p", mLayers[0].getBuffer().get());
+ base::StringAppendF(&result, " Protected [%s]",
+ mLayers[0].getState()->isProtected() ? "true" : "false");
} else {
result.append(" Cached set of:");
for (const Layer& layer : mLayers) {
base::StringAppendF(&result, "\n Layer [%s]", layer.getName().c_str());
+ base::StringAppendF(&result, "\n Protected [%s]",
+ layer.getState()->isProtected() ? "true" : "false");
}
}
diff --git a/services/surfaceflinger/CompositionEngine/src/planner/Flattener.cpp b/services/surfaceflinger/CompositionEngine/src/planner/Flattener.cpp
index 2bcaf60..f033279 100644
--- a/services/surfaceflinger/CompositionEngine/src/planner/Flattener.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/planner/Flattener.cpp
@@ -420,7 +420,7 @@
const bool layerIsInactive = now - currentSet->getLastUpdate() > mActiveLayerTimeout;
const bool layerHasBlur = currentSet->hasBlurBehind();
if (layerIsInactive && (firstLayer || runHasFirstLayer || !layerHasBlur) &&
- !currentSet->hasHdrLayers() && !currentSet->hasProtectedLayers()) {
+ !currentSet->hasUnsupportedDataspace()) {
if (isPartOfRun) {
builder.append(currentSet->getLayerCount());
} else {
@@ -491,6 +491,14 @@
return;
}
+ for (const CachedSet& layer : mLayers) {
+ // TODO (b/191997217): make it less aggressive, and sync with findCandidateRuns
+ if (layer.hasProtectedLayers()) {
+ ATRACE_NAME("layer->hasProtectedLayers()");
+ return;
+ }
+ }
+
std::vector<Run> runs = findCandidateRuns(now);
std::optional<Run> bestRun = findBestRun(runs);
diff --git a/services/surfaceflinger/CompositionEngine/tests/planner/CachedSetTest.cpp b/services/surfaceflinger/CompositionEngine/tests/planner/CachedSetTest.cpp
index 0acc317..7f0e186 100644
--- a/services/surfaceflinger/CompositionEngine/tests/planner/CachedSetTest.cpp
+++ b/services/surfaceflinger/CompositionEngine/tests/planner/CachedSetTest.cpp
@@ -457,6 +457,20 @@
EXPECT_FALSE(cachedSet.requiresHolePunch());
}
+TEST_F(CachedSetTest, holePunch_requiresNonBT601_625) {
+ mTestLayers[0]->outputLayerCompositionState.dataspace = ui::Dataspace::STANDARD_BT601_625;
+ mTestLayers[0]->layerState->update(&mTestLayers[0]->outputLayer);
+
+ CachedSet::Layer& layer = *mTestLayers[0]->cachedSetLayer.get();
+ mTestLayers[0]->layerFECompositionState.buffer = sp<GraphicBuffer>::make();
+ sp<mock::LayerFE> layerFE = mTestLayers[0]->layerFE;
+
+ CachedSet cachedSet(layer);
+ EXPECT_CALL(*layerFE, hasRoundedCorners()).WillRepeatedly(Return(true));
+
+ EXPECT_FALSE(cachedSet.requiresHolePunch());
+}
+
TEST_F(CachedSetTest, requiresHolePunch) {
CachedSet::Layer& layer = *mTestLayers[0]->cachedSetLayer.get();
mTestLayers[0]->layerFECompositionState.buffer = sp<GraphicBuffer>::make();
diff --git a/services/surfaceflinger/CompositionEngine/tests/planner/FlattenerTest.cpp b/services/surfaceflinger/CompositionEngine/tests/planner/FlattenerTest.cpp
index 334b855..f5cfd2f 100644
--- a/services/surfaceflinger/CompositionEngine/tests/planner/FlattenerTest.cpp
+++ b/services/surfaceflinger/CompositionEngine/tests/planner/FlattenerTest.cpp
@@ -937,5 +937,152 @@
(kCachedSetRenderDuration + 10ms));
}
+TEST_F(FlattenerTest, flattenLayers_skipsBT601_625) {
+ auto& layerState1 = mTestLayers[0]->layerState;
+ const auto& overrideBuffer1 = layerState1->getOutputLayer()->getState().overrideInfo.buffer;
+
+ auto& layerState2 = mTestLayers[1]->layerState;
+ const auto& overrideBuffer2 = layerState2->getOutputLayer()->getState().overrideInfo.buffer;
+
+ // The third layer uses a dataspace that will not be flattened due to
+ // possible mismatch with DPU rendering.
+ auto& layerState3 = mTestLayers[2]->layerState;
+ const auto& overrideBuffer3 = layerState3->getOutputLayer()->getState().overrideInfo.buffer;
+ mTestLayers[2]->outputLayerCompositionState.dataspace = ui::Dataspace::STANDARD_BT601_625;
+ mTestLayers[2]->layerState->update(&mTestLayers[2]->outputLayer);
+
+ const std::vector<const LayerState*> layers = {
+ layerState1.get(),
+ layerState2.get(),
+ layerState3.get(),
+ };
+
+ initializeFlattener(layers);
+
+ mTime += 200ms;
+ initializeOverrideBuffer(layers);
+ EXPECT_EQ(getNonBufferHash(layers),
+ mFlattener->flattenLayers(layers, getNonBufferHash(layers), mTime));
+
+ // This will render a CachedSet.
+ EXPECT_CALL(mRenderEngine, drawLayers(_, _, _, _, _, _)).WillOnce(Return(NO_ERROR));
+ mFlattener->renderCachedSets(mOutputState, std::nullopt);
+
+ // We've rendered a CachedSet, but we haven't merged it in.
+ EXPECT_EQ(nullptr, overrideBuffer1);
+ EXPECT_EQ(nullptr, overrideBuffer2);
+ EXPECT_EQ(nullptr, overrideBuffer3);
+
+ // This time we merge the CachedSet in, so we have a new hash, and we should
+ // only have two sets.
+ EXPECT_CALL(mRenderEngine, drawLayers(_, _, _, _, _, _)).Times(0);
+ initializeOverrideBuffer(layers);
+ EXPECT_NE(getNonBufferHash(layers),
+ mFlattener->flattenLayers(layers, getNonBufferHash(layers), mTime));
+ mFlattener->renderCachedSets(mOutputState, std::nullopt);
+
+ EXPECT_NE(nullptr, overrideBuffer1);
+ EXPECT_EQ(overrideBuffer1, overrideBuffer2);
+ EXPECT_EQ(nullptr, overrideBuffer3);
+}
+
+TEST_F(FlattenerTest, flattenLayers_skipsHDR) {
+ auto& layerState1 = mTestLayers[0]->layerState;
+ const auto& overrideBuffer1 = layerState1->getOutputLayer()->getState().overrideInfo.buffer;
+
+ auto& layerState2 = mTestLayers[1]->layerState;
+ const auto& overrideBuffer2 = layerState2->getOutputLayer()->getState().overrideInfo.buffer;
+
+ // The third layer uses a dataspace that will not be flattened due to
+ // possible mismatch with DPU rendering.
+ auto& layerState3 = mTestLayers[2]->layerState;
+ const auto& overrideBuffer3 = layerState3->getOutputLayer()->getState().overrideInfo.buffer;
+ mTestLayers[2]->outputLayerCompositionState.dataspace = ui::Dataspace::BT2020_ITU_HLG;
+ mTestLayers[2]->layerState->update(&mTestLayers[2]->outputLayer);
+
+ const std::vector<const LayerState*> layers = {
+ layerState1.get(),
+ layerState2.get(),
+ layerState3.get(),
+ };
+
+ initializeFlattener(layers);
+
+ mTime += 200ms;
+ initializeOverrideBuffer(layers);
+ EXPECT_EQ(getNonBufferHash(layers),
+ mFlattener->flattenLayers(layers, getNonBufferHash(layers), mTime));
+
+ // This will render a CachedSet.
+ EXPECT_CALL(mRenderEngine, drawLayers(_, _, _, _, _, _)).WillOnce(Return(NO_ERROR));
+ mFlattener->renderCachedSets(mOutputState, std::nullopt);
+
+ // We've rendered a CachedSet, but we haven't merged it in.
+ EXPECT_EQ(nullptr, overrideBuffer1);
+ EXPECT_EQ(nullptr, overrideBuffer2);
+ EXPECT_EQ(nullptr, overrideBuffer3);
+
+ // This time we merge the CachedSet in, so we have a new hash, and we should
+ // only have two sets.
+ EXPECT_CALL(mRenderEngine, drawLayers(_, _, _, _, _, _)).Times(0);
+ initializeOverrideBuffer(layers);
+ EXPECT_NE(getNonBufferHash(layers),
+ mFlattener->flattenLayers(layers, getNonBufferHash(layers), mTime));
+ mFlattener->renderCachedSets(mOutputState, std::nullopt);
+
+ EXPECT_NE(nullptr, overrideBuffer1);
+ EXPECT_EQ(overrideBuffer1, overrideBuffer2);
+ EXPECT_EQ(nullptr, overrideBuffer3);
+}
+
+TEST_F(FlattenerTest, flattenLayers_skipsHDR2) {
+ auto& layerState1 = mTestLayers[0]->layerState;
+ const auto& overrideBuffer1 = layerState1->getOutputLayer()->getState().overrideInfo.buffer;
+
+ auto& layerState2 = mTestLayers[1]->layerState;
+ const auto& overrideBuffer2 = layerState2->getOutputLayer()->getState().overrideInfo.buffer;
+
+ // The third layer uses a dataspace that will not be flattened due to
+ // possible mismatch with DPU rendering.
+ auto& layerState3 = mTestLayers[2]->layerState;
+ const auto& overrideBuffer3 = layerState3->getOutputLayer()->getState().overrideInfo.buffer;
+ mTestLayers[2]->outputLayerCompositionState.dataspace = ui::Dataspace::BT2020_PQ;
+ mTestLayers[2]->layerState->update(&mTestLayers[2]->outputLayer);
+
+ const std::vector<const LayerState*> layers = {
+ layerState1.get(),
+ layerState2.get(),
+ layerState3.get(),
+ };
+
+ initializeFlattener(layers);
+
+ mTime += 200ms;
+ initializeOverrideBuffer(layers);
+ EXPECT_EQ(getNonBufferHash(layers),
+ mFlattener->flattenLayers(layers, getNonBufferHash(layers), mTime));
+
+ // This will render a CachedSet.
+ EXPECT_CALL(mRenderEngine, drawLayers(_, _, _, _, _, _)).WillOnce(Return(NO_ERROR));
+ mFlattener->renderCachedSets(mOutputState, std::nullopt);
+
+ // We've rendered a CachedSet, but we haven't merged it in.
+ EXPECT_EQ(nullptr, overrideBuffer1);
+ EXPECT_EQ(nullptr, overrideBuffer2);
+ EXPECT_EQ(nullptr, overrideBuffer3);
+
+ // This time we merge the CachedSet in, so we have a new hash, and we should
+ // only have two sets.
+ EXPECT_CALL(mRenderEngine, drawLayers(_, _, _, _, _, _)).Times(0);
+ initializeOverrideBuffer(layers);
+ EXPECT_NE(getNonBufferHash(layers),
+ mFlattener->flattenLayers(layers, getNonBufferHash(layers), mTime));
+ mFlattener->renderCachedSets(mOutputState, std::nullopt);
+
+ EXPECT_NE(nullptr, overrideBuffer1);
+ EXPECT_EQ(overrideBuffer1, overrideBuffer2);
+ EXPECT_EQ(nullptr, overrideBuffer3);
+}
+
} // namespace
} // namespace android::compositionengine
diff --git a/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp b/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp
index 4f47ed8..0334d70 100644
--- a/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp
+++ b/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp
@@ -889,6 +889,7 @@
const auto& deviceMin = *mMinSupportedRefreshRate;
const auto& minByPolicy = getMinRefreshRateByPolicyLocked();
const auto& maxByPolicy = getMaxRefreshRateByPolicyLocked();
+ const auto& currentPolicy = getCurrentPolicyLocked();
// Kernel idle timer will set the refresh rate to the device min. If DisplayManager says that
// the min allowed refresh rate is higher than the device min, we do not want to enable the
@@ -897,6 +898,10 @@
return RefreshRateConfigs::KernelIdleTimerAction::TurnOff;
}
if (minByPolicy == maxByPolicy) {
+ // when min primary range in display manager policy is below device min turn on the timer.
+ if (currentPolicy->primaryRange.min.lessThanWithMargin(deviceMin.getFps())) {
+ return RefreshRateConfigs::KernelIdleTimerAction::TurnOn;
+ }
return RefreshRateConfigs::KernelIdleTimerAction::TurnOff;
}
// Turn on the timer in all other cases.
diff --git a/services/surfaceflinger/Scheduler/RefreshRateConfigs.h b/services/surfaceflinger/Scheduler/RefreshRateConfigs.h
index 0358e8c..dfd1395 100644
--- a/services/surfaceflinger/Scheduler/RefreshRateConfigs.h
+++ b/services/surfaceflinger/Scheduler/RefreshRateConfigs.h
@@ -326,8 +326,7 @@
return mRefreshRates.size() > 1;
}
- // Class to enumerate options around toggling the kernel timer on and off. We have an option
- // for no change to avoid extra calls to kernel.
+ // Class to enumerate options around toggling the kernel timer on and off.
enum class KernelIdleTimerAction {
TurnOff, // Turn off the idle timer.
TurnOn // Turn on the idle timer.
diff --git a/services/surfaceflinger/tests/unittests/RefreshRateConfigsTest.cpp b/services/surfaceflinger/tests/unittests/RefreshRateConfigsTest.cpp
index 9bb5ca1..3423bd5 100644
--- a/services/surfaceflinger/tests/unittests/RefreshRateConfigsTest.cpp
+++ b/services/surfaceflinger/tests/unittests/RefreshRateConfigsTest.cpp
@@ -2066,6 +2066,35 @@
EXPECT_EQ(KernelIdleTimerAction::TurnOff, refreshRateConfigs->getIdleTimerAction());
}
+TEST_F(RefreshRateConfigsTest, testKernelIdleTimerActionFor120Hz) {
+ using KernelIdleTimerAction = scheduler::RefreshRateConfigs::KernelIdleTimerAction;
+
+ // Tests with 120Hz
+ auto refreshRateConfigs =
+ std::make_unique<RefreshRateConfigs>(m60_120Device,
+ /*currentConfigId=*/HWC_CONFIG_ID_120);
+ // SetPolicy(0, 60), current 60Hz => TurnOn.
+ ASSERT_GE(refreshRateConfigs->setDisplayManagerPolicy({HWC_CONFIG_ID_60, {Fps(0), Fps(60)}}),
+ 0);
+ EXPECT_EQ(KernelIdleTimerAction::TurnOn, refreshRateConfigs->getIdleTimerAction());
+
+ // SetPolicy(60, 60), current 60Hz => TurnOff.
+ ASSERT_GE(refreshRateConfigs->setDisplayManagerPolicy({HWC_CONFIG_ID_60, {Fps(60), Fps(60)}}),
+ 0);
+ EXPECT_EQ(KernelIdleTimerAction::TurnOff, refreshRateConfigs->getIdleTimerAction());
+
+ // SetPolicy(60, 120), current 60Hz => TurnOn.
+ ASSERT_GE(refreshRateConfigs->setDisplayManagerPolicy({HWC_CONFIG_ID_60, {Fps(60), Fps(120)}}),
+ 0);
+ EXPECT_EQ(KernelIdleTimerAction::TurnOn, refreshRateConfigs->getIdleTimerAction());
+
+ // SetPolicy(120, 120), current 120Hz => TurnOff.
+ ASSERT_GE(refreshRateConfigs->setDisplayManagerPolicy(
+ {HWC_CONFIG_ID_120, {Fps(120), Fps(120)}}),
+ 0);
+ EXPECT_EQ(KernelIdleTimerAction::TurnOff, refreshRateConfigs->getIdleTimerAction());
+}
+
TEST_F(RefreshRateConfigsTest, getFrameRateDivider) {
auto refreshRateConfigs =
std::make_unique<RefreshRateConfigs>(m30_60_72_90_120Device,