diff options
-rw-r--r-- | libs/binder/Parcel.cpp | 8 | ||||
-rw-r--r-- | libs/binder/tests/binderLibTest.cpp | 46 | ||||
-rw-r--r-- | services/surfaceflinger/BufferLayer.cpp | 13 | ||||
-rw-r--r-- | services/surfaceflinger/Scheduler/MessageQueue.cpp | 11 | ||||
-rwxr-xr-x | services/surfaceflinger/SurfaceFlinger.cpp | 256 | ||||
-rw-r--r-- | services/surfaceflinger/SurfaceFlinger.h | 14 |
6 files changed, 261 insertions, 87 deletions
diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp index ee834ea43c..617708f3d4 100644 --- a/libs/binder/Parcel.cpp +++ b/libs/binder/Parcel.cpp @@ -2141,12 +2141,14 @@ void Parcel::ipcSetDataReference(const uint8_t* data, size_t dataSize, type == BINDER_TYPE_FD)) { // We should never receive other types (eg BINDER_TYPE_FDA) as long as we don't support // them in libbinder. If we do receive them, it probably means a kernel bug; try to - // recover gracefully by clearing out the objects, and releasing the objects we do - // know about. + // recover gracefully by clearing out the objects. android_errorWriteLog(0x534e4554, "135930648"); + android_errorWriteLog(0x534e4554, "203847542"); ALOGE("%s: unsupported type object (%" PRIu32 ") at offset %" PRIu64 "\n", __func__, type, (uint64_t)offset); - releaseObjects(); + + // WARNING: callers of ipcSetDataReference need to make sure they + // don't rely on mObjectsSize in their release_func. mObjectsSize = 0; break; } diff --git a/libs/binder/tests/binderLibTest.cpp b/libs/binder/tests/binderLibTest.cpp index 64196ba30f..6cb7e7f104 100644 --- a/libs/binder/tests/binderLibTest.cpp +++ b/libs/binder/tests/binderLibTest.cpp @@ -94,7 +94,7 @@ enum BinderLibTestTranscationCode { BINDER_LIB_TEST_NOP_TRANSACTION_WAIT, BINDER_LIB_TEST_GETPID, BINDER_LIB_TEST_ECHO_VECTOR, - BINDER_LIB_TEST_REJECT_BUF, + BINDER_LIB_TEST_REJECT_OBJECTS, BINDER_LIB_TEST_CAN_GET_SID, }; @@ -1127,13 +1127,53 @@ TEST_F(BinderLibTest, BufRejected) { memcpy(parcelData, &obj, sizeof(obj)); data.setDataSize(sizeof(obj)); + EXPECT_EQ(data.objectsCount(), 1); + // Either the kernel should reject this transaction (if it's correct), but // if it's not, the server implementation should return an error if it // finds an object in the received Parcel. - EXPECT_THAT(server->transact(BINDER_LIB_TEST_REJECT_BUF, data, &reply), + EXPECT_THAT(server->transact(BINDER_LIB_TEST_REJECT_OBJECTS, data, &reply), Not(StatusEq(NO_ERROR))); } +TEST_F(BinderLibTest, WeakRejected) { + Parcel data, reply; + sp<IBinder> server = addServer(); + ASSERT_TRUE(server != nullptr); + + auto binder = sp<BBinder>::make(); + wp<BBinder> wpBinder(binder); + flat_binder_object obj{ + .hdr = {.type = BINDER_TYPE_WEAK_BINDER}, + .flags = 0, + .binder = reinterpret_cast<uintptr_t>(wpBinder.get_refs()), + .cookie = reinterpret_cast<uintptr_t>(wpBinder.unsafe_get()), + }; + data.setDataCapacity(1024); + // Write a bogus object at offset 0 to get an entry in the offset table + data.writeFileDescriptor(0); + EXPECT_EQ(data.objectsCount(), 1); + uint8_t *parcelData = const_cast<uint8_t *>(data.data()); + // And now, overwrite it with the weak binder + memcpy(parcelData, &obj, sizeof(obj)); + data.setDataSize(sizeof(obj)); + + // a previous bug caused other objects to be released an extra time, so we + // test with an object that libbinder will actually try to release + EXPECT_EQ(OK, data.writeStrongBinder(sp<BBinder>::make())); + + EXPECT_EQ(data.objectsCount(), 2); + + // send it many times, since previous error was memory corruption, make it + // more likely that the server crashes + for (size_t i = 0; i < 100; i++) { + EXPECT_THAT(server->transact(BINDER_LIB_TEST_REJECT_OBJECTS, data, &reply), + StatusEq(BAD_VALUE)); + } + + EXPECT_THAT(server->pingBinder(), StatusEq(NO_ERROR)); +} + TEST_F(BinderLibTest, GotSid) { sp<IBinder> server = addServer(); @@ -1440,7 +1480,7 @@ class BinderLibTestService : public BBinder reply->writeUint64Vector(vector); return NO_ERROR; } - case BINDER_LIB_TEST_REJECT_BUF: { + case BINDER_LIB_TEST_REJECT_OBJECTS: { return data.objectsCount() == 0 ? BAD_VALUE : NO_ERROR; } case BINDER_LIB_TEST_CAN_GET_SID: { diff --git a/services/surfaceflinger/BufferLayer.cpp b/services/surfaceflinger/BufferLayer.cpp index b7b4a445c7..ddf6f10d2b 100644 --- a/services/surfaceflinger/BufferLayer.cpp +++ b/services/surfaceflinger/BufferLayer.cpp @@ -571,8 +571,17 @@ bool BufferLayer::latchBuffer(bool& recomputeVisibleRegions, nsecs_t latchTime, recomputeVisibleRegions = true; } - if (mFlinger->mSmoMo) { - mFlinger->mSmoMo->SetPresentTime(getSequence(), mBufferInfo.mDesiredPresentTime); + const uint32_t layerStackId = getLayerStack(); + SmomoIntf *smoMo = nullptr; + for (auto &instance: mFlinger->mSmomoInstances) { + if (instance.layerStackId == layerStackId) { + smoMo = instance.smoMo; + break; + } + } + + if (smoMo) { + smoMo->SetPresentTime(getSequence(), mBufferInfo.mDesiredPresentTime); } return true; diff --git a/services/surfaceflinger/Scheduler/MessageQueue.cpp b/services/surfaceflinger/Scheduler/MessageQueue.cpp index 294783ab34..a339375db0 100644 --- a/services/surfaceflinger/Scheduler/MessageQueue.cpp +++ b/services/surfaceflinger/Scheduler/MessageQueue.cpp @@ -125,8 +125,15 @@ void MessageQueue::vsyncCallback(nsecs_t vsyncTime, nsecs_t targetWakeupTime, ns mFlinger->mDolphinWrapper.dolphinTrackVsyncSignal(vsyncTime, targetWakeupTime, readyTime); } - if (mFlinger->mSmoMo) { - mFlinger->mSmoMo->OnVsync(vsyncTime); + SmomoIntf *smoMo = nullptr; + for (auto &instance: mFlinger->mSmomoInstances) { + if (instance.displayId == 0) { + smoMo = instance.smoMo; + break; + } + } + if (smoMo) { + smoMo->OnVsync(vsyncTime); } mHandler->dispatchInvalidate(mVsync.tokenManager->generateTokenForPredictions( diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 086be820c5..7cea8143d5 100755 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -433,12 +433,9 @@ bool callingThreadHasRotateSurfaceFlingerAccess() { void SurfaceFlinger::setRefreshRates( std::unique_ptr<scheduler::RefreshRateConfigs> &refreshRateConfigs) { + // Get Primary Smomo Instance. std::vector<float> refreshRates; - if (mSmoMo == nullptr) { - return; - } - auto iter = refreshRateConfigs->getAllRefreshRates().cbegin(); while (iter != refreshRateConfigs->getAllRefreshRates().cend()) { if (refreshRateConfigs->isModeAllowed(iter->second->getModeId())) { @@ -446,7 +443,14 @@ void SurfaceFlinger::setRefreshRates( } ++iter; } - mSmoMo->SetDisplayRefreshRates(refreshRates); + SmomoIntf *smoMo = nullptr; + for (auto &instance: mSmomoInstances) { + smoMo = instance.smoMo; + if (smoMo == nullptr) { + continue; + } + smoMo->SetDisplayRefreshRates(refreshRates); + } } bool LayerExtWrapper::init() { @@ -1143,9 +1147,6 @@ void SurfaceFlinger::init() { ALOGE("Run StartPropertySetThread failed!"); } - // Initialize Smomo. - InitSmomo(); - char layerExtProp[PROPERTY_VALUE_MAX]; property_get("vendor.display.use_layer_ext", layerExtProp, "0"); if (atoi(layerExtProp)) { @@ -1200,26 +1201,66 @@ void SurfaceFlinger::InitComposerExtn() { ALOGI("Init: mDisplayExtnIntf: %p", mDisplayExtnIntf); } -void SurfaceFlinger::InitSmomo() { +void SurfaceFlinger::createSmomoInstance(const DisplayDeviceState& state) { + if (state.isVirtual() || state.physical->type == ui::DisplayConnectionType::External) { + return; + } char smomoProp[PROPERTY_VALUE_MAX]; property_get("vendor.display.use_smooth_motion", smomoProp, "0"); if (!atoi(smomoProp)) { + ALOGI("Smomo is disabled through property"); return; } - bool ret = mComposerExtnIntf->CreateSmomoExtn(&mSmoMo); + SmomoInfo smomoInfo; + smomoInfo.displayId = state.physical->hwcDisplayId; + smomoInfo.layerStackId = state.layerStack; + smomoInfo.active = true; + smomo::DisplayInfo displayInfo; + displayInfo.display_id = state.physical->hwcDisplayId; + displayInfo.is_primary = state.physical->hwcDisplayId == 0; + displayInfo.type = smomo::kBuiltin; + mComposerExtnIntf = composer::ComposerExtnLib::GetInstance(); + bool ret = mComposerExtnIntf->CreateSmomoExtn(&smomoInfo.smoMo, displayInfo); if (!ret) { - ALOGI("Unable to create smomo extension"); + ALOGI("Unable to create smomo extension for display: %d", displayInfo.display_id); + return; } - if (mSmoMo != nullptr) { - mSmoMo->SetChangeRefreshRateCallback( - [this](int32_t refreshRate) { + + mSmomoInstances.push_back(smomoInfo); + // Set refresh rates for primary display's instance. + smomoInfo.smoMo->SetChangeRefreshRateCallback( + [this](int32_t refreshRate) { setRefreshRateTo(refreshRate); - }); + }); - setRefreshRates(mRefreshRateConfigs); + setRefreshRates(mRefreshRateConfigs); - ALOGI("SmoMo is enabled"); + if (mSmomoInstances.size() > 1) { + // Disable DRC on all instances. + for (auto &instance : mSmomoInstances) { + instance.smoMo->SetRefreshRateChangeStatus(false); + } } + + ALOGI("SmoMo is enabled for display: %d", displayInfo.display_id); +} + +void SurfaceFlinger::destroySmomoInstance(const sp<DisplayDevice>& display) { + uint32_t hwcDisplayId = 0; + if (!getHwcDisplayId(display, &hwcDisplayId)) { + return; + } + + mSmomoInstances.erase(std::remove_if(mSmomoInstances.begin(), mSmomoInstances.end(), + [&](SmomoInfo const &smomoInfo) { + return smomoInfo.displayId == hwcDisplayId; + }), mSmomoInstances.end()); + + // Enable DRC if only one instance is active. + if (mSmomoInstances.size() == 1) { + // Disable DRC on all instances. + mSmomoInstances.at(0).smoMo->SetRefreshRateChangeStatus(false); + } } void SurfaceFlinger::startUnifiedDraw() { @@ -1468,7 +1509,7 @@ status_t SurfaceFlinger::getDisplayStats(const sp<IBinder>&, DisplayStatInfo* st } bool SurfaceFlinger::isFpsDeferNeeded(const ActiveModeInfo& info) { - const auto display = ON_MAIN_THREAD(getDefaultDisplayDeviceLocked()); + const auto display = getDefaultDisplayDeviceLocked(); if (!display || !mThermalLevelFps) { return false; } @@ -2492,9 +2533,16 @@ void SurfaceFlinger::updateFrameScheduler() NO_THREAD_SAFETY_ANALYSIS { void SurfaceFlinger::syncToDisplayHardware() NO_THREAD_SAFETY_ANALYSIS { ATRACE_CALL(); - if (mSmoMo) { + SmomoIntf *smoMo = nullptr; + for (auto &instance: mSmomoInstances) { + if (instance.displayId == 0) { + smoMo = instance.smoMo; + break; + } + } + if (smoMo) { nsecs_t timestamp = 0; - bool needResync = mSmoMo->SyncToDisplay(previousFrameFence().fence, ×tamp); + bool needResync = smoMo->SyncToDisplay(previousFrameFence().fence, ×tamp); ALOGV("needResync = %d, timestamp = %" PRId64, needResync, timestamp); } } @@ -3136,6 +3184,8 @@ void SurfaceFlinger::postComposition() { const size_t appConnections = mScheduler->getEventThreadConnectionCount(mAppConnectionHandle); mTimeStats->recordDisplayEventConnectionCount(sfConnections + appConnections); + UpdateSmomoState(); + if (isDisplayConnected && !display->isPoweredOn()) { return; } @@ -3180,7 +3230,16 @@ void SurfaceFlinger::postComposition() { bool layerExtEnabled = (mSplitLayerExt && mLayerExt); bool visibleLayersInfo = false; - if (mSmoMo || layerExtEnabled) { + const uint32_t layerStackId = display->getLayerStack(); + SmomoIntf *smoMo = nullptr; + for (auto &instance: mSmomoInstances) { + if (instance.layerStackId == layerStackId) { + smoMo = instance.smoMo; + break; + } + } + + if (smoMo || layerExtEnabled) { const auto compositionDisplay = display->getCompositionDisplay(); compositionDisplay->getVisibleLayerInfo(&layerName, &layerSequence); visibleLayersInfo = (layerName.size() != 0); @@ -3190,50 +3249,70 @@ void SurfaceFlinger::postComposition() { mLayerExt->UpdateLayerState(layerName, mNumLayers); } - if (mSmoMo && visibleLayersInfo) { - ATRACE_NAME("SmoMoUpdateState"); - Mutex::Autolock lock(mStateLock); - uint32_t fps = 0; - std::vector<smomo::SmomoLayerStats> layers; + // Even though ATRACE_INT64 already checks if tracing is enabled, it doesn't prevent the + // side-effect of getTotalSize(), so we check that again here + if (ATRACE_ENABLED()) { + // getTotalSize returns the total number of buffers that were allocated by SurfaceFlinger + ATRACE_INT64("Total Buffer Size", GraphicBufferAllocator::get().getTotalSize()); + } +} - // Disable SmoMo by passing empty layer stack in multiple display case - if (mDisplays.size() == 1) { - for (int i = 0; i < layerName.size(); i++) { - smomo::SmomoLayerStats layerStats; - layerStats.name = layerName.at(i); - layerStats.id = layerSequence.at(i); - layers.push_back(layerStats); +void SurfaceFlinger::UpdateSmomoState() { + ATRACE_NAME("SmoMoUpdateState"); + Mutex::Autolock lock(mStateLock); + // Check if smomo instances exist. + if (!mSmomoInstances.size()) { + return; + } + + // Disable smomo if external or virtual is connected. + bool enableSmomo = mSmomoInstances.size() == mDisplays.size(); + uint32_t fps = 0; + int content_fps = 0; + int numActiveDisplays = 0; + for (auto &instance: mSmomoInstances) { + SmomoIntf *smoMo = instance.smoMo; + sp<DisplayDevice> device = nullptr; + for (const auto& [token, displayDevice] : mDisplays) { + uint32_t hwcDisplayId; + if (!getHwcDisplayId(displayDevice, &hwcDisplayId)) { + continue; } + if (hwcDisplayId == instance.displayId) { + device = displayDevice; + break; + } + } + std::vector<smomo::SmomoLayerStats> layers; + if (enableSmomo && instance.active) { fps = mRefreshRateConfigs->getCurrentRefreshRate().getFps().getValue(); + mDrawingState.traverse([&](Layer* layer) { + if (layer->findOutputLayerForDisplay(device.get())) { + smomo::SmomoLayerStats layerStats; + layerStats.id = layer->getSequence(); + layerStats.name = layer->getName(); + layers.push_back(layerStats); + } + }); } - mSmoMo->UpdateSmomoState(layers, fps); - int content_fps = mSmoMo->GetFrameRate(); - - bool is_valid_content_fps = false; - if (content_fps > 0) { - if (mLayersWithQueuedFrames.size() > 1) { - mUiLayerFrameCount++; - } else { - mUiLayerFrameCount = 0; - } - - is_valid_content_fps = (mUiLayerFrameCount < fps) ? true : false; - } else { - mUiLayerFrameCount = 0; + if (instance.active) { + smoMo->UpdateSmomoState(layers, fps); } - - setContentFps(is_valid_content_fps ? content_fps : fps); + content_fps = smoMo->GetFrameRate(); + bool active = device->getPowerMode() != hal::PowerMode::OFF; + // Cache smommo status. + instance.active = active; + numActiveDisplays += active; } + setContentFps((content_fps > 0) && (mSmomoInstances.size() == 1) ? content_fps : fps); - // Even though ATRACE_INT64 already checks if tracing is enabled, it doesn't prevent the - // side-effect of getTotalSize(), so we check that again here - if (ATRACE_ENABLED()) { - // getTotalSize returns the total number of buffers that were allocated by SurfaceFlinger - ATRACE_INT64("Total Buffer Size", GraphicBufferAllocator::get().getTotalSize()); + // Disable DRC if active displays is more than 1. + for (auto &instance : mSmomoInstances) { + instance.smoMo->SetRefreshRateChangeStatus((numActiveDisplays == 1)); } } @@ -3754,6 +3833,7 @@ void SurfaceFlinger::processDisplayAdded(const wp<IBinder>& displayToken, } } #endif + createSmomoInstance(state); } void SurfaceFlinger::processDisplayRemoved(const wp<IBinder>& displayToken) { @@ -3766,6 +3846,7 @@ void SurfaceFlinger::processDisplayRemoved(const wp<IBinder>& displayToken) { } else { dispatchDisplayHotplugEvent(display->getPhysicalId(), false); } + destroySmomoInstance(display); } mDisplays.erase(displayToken); @@ -3837,6 +3918,13 @@ void SurfaceFlinger::processDisplayChanged(const wp<IBinder>& displayToken, bool displaySizeChanged = false; if (currentState.layerStack != drawingState.layerStack) { display->setLayerStack(currentState.layerStack); + for (auto &instance: mSmomoInstances) { + if ((instance.displayId == currentState.physical->hwcDisplayId) && + instance.layerStackId == drawingState.layerStack) { + instance.layerStackId = currentState.layerStack; + break; + } + } } if ((currentState.orientation != drawingState.orientation) || (currentState.layerStackSpaceRect != drawingState.layerStackSpaceRect) || @@ -4636,10 +4724,20 @@ bool SurfaceFlinger::transactionIsReadyToBeApplied( return false; } - if (mSmoMo) { - if (mSmoMo->FrameIsEarly(layer->getSequence(), desiredPresentTime)) { + const uint32_t layerStackId = layer->getLayerStack(); + SmomoIntf *smoMo = nullptr; + for (auto &instance: mSmomoInstances) { + if (instance.layerStackId == layerStackId) { + smoMo = instance.smoMo; + break; + } + } + if (smoMo) { + ATRACE_BEGIN("smomo_begin"); + if (smoMo->FrameIsEarly(layer->getSequence(), desiredPresentTime)) { return false; } + ATRACE_END(); } } } @@ -4767,26 +4865,36 @@ status_t SurfaceFlinger::setTransactionState( waitForSynchronousTransaction(*state.transactionCommittedSignal); } - if (mSmoMo) { - state.traverseStatesWithBuffers([&](const layer_state_t& state) { - sp<Layer> layer = fromHandle(state.surface).promote(); - if (layer != nullptr) { - smomo::SmomoBufferStats bufferStats; - const nsecs_t now = systemTime(SYSTEM_TIME_MONOTONIC); - bufferStats.id = layer->getSequence(); - bufferStats.auto_timestamp = isAutoTimestamp; - bufferStats.timestamp = now; - bufferStats.dequeue_latency = 0; - bufferStats.key = desiredPresentTime; - mSmoMo->CollectLayerStats(bufferStats); - - const DisplayStatInfo stats = mScheduler->getDisplayStatInfo(now); - if (mSmoMo->FrameIsLate(bufferStats.id, stats.vsyncTime)) { - signalImmedLayerUpdate(); + state.traverseStatesWithBuffers([&](const layer_state_t& state) { + sp<Layer> layer = fromHandle(state.surface).promote(); + if (layer != nullptr) { + const uint32_t layerStackId = layer->getLayerStack(); + SmomoIntf *smoMo = nullptr; + for (auto &instance: mSmomoInstances) { + if (instance.layerStackId == layerStackId) { + smoMo = instance.smoMo; + break; } } - }); - } + + if (smoMo) { + smomo::SmomoBufferStats bufferStats; + const nsecs_t now = systemTime(SYSTEM_TIME_MONOTONIC); + bufferStats.id = layer->getSequence(); + bufferStats.auto_timestamp = isAutoTimestamp; + bufferStats.timestamp = now; + bufferStats.dequeue_latency = 0; + bufferStats.key = desiredPresentTime; + smoMo->CollectLayerStats(bufferStats); + + const DisplayStatInfo stats = mScheduler->getDisplayStatInfo(now); + if (smoMo->FrameIsLate(bufferStats.id, stats.vsyncTime)) { + signalImmedLayerUpdate(); + } + } + } + }); + return NO_ERROR; } diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 8363ee5a62..14bc09e4e2 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -897,7 +897,8 @@ private: // Check if unified draw supported void startUnifiedDraw(); void InitComposerExtn(); - void InitSmomo(); + void createSmomoInstance(const DisplayDeviceState& state); + void destroySmomoInstance(const sp<DisplayDevice>& display); // Returns whether a new buffer has been latched (see handlePageFlip()) bool handleMessageInvalidate(); @@ -1351,7 +1352,7 @@ private: std::chrono::nanoseconds presentLatency); int getMaxAcquiredBufferCountForRefreshRate(Fps refreshRate) const; void setDesiredModeByThermalLevel(float newFpsRequest); - bool isFpsDeferNeeded(const ActiveModeInfo& info); + bool isFpsDeferNeeded(const ActiveModeInfo& info) REQUIRES(mStateLock); virtual void getModeFromFps(float fps,DisplayModePtr& outMode); void handleNewLevelFps(float currFps, float newLevelFps, float* fpsToSet); @@ -1665,12 +1666,19 @@ private: void scheduleRegionSamplingThread(); void notifyRegionSamplingThread(); void setRefreshRates(std::unique_ptr<scheduler::RefreshRateConfigs> &refreshRateConfigs); + void UpdateSmomoState(); public: nsecs_t mVsyncPeriod = -1; DolphinWrapper mDolphinWrapper; LayerExtWrapper mLayerExt; - SmomoIntf *mSmoMo = nullptr; + struct SmomoInfo { + uint32_t displayId; + uint32_t layerStackId; + bool active = false; + SmomoIntf *smoMo = nullptr; + }; + std::vector<SmomoInfo> mSmomoInstances; private: bool mEarlyWakeUpEnabled = false; |