Merge changes I6f68b884,I870d8f13 into main

* changes:
  SF: Clean up helpers for thread priority
  SF: Remove connected_display flag

Author: Treehugger Robot

services/surfaceflinger/Display/DisplayModeController.cpp

@@ -97,9 +97,7 @@ auto DisplayModeController::setDesiredMode(PhysicalDisplayId displayId,
             const bool force = desiredModeOpt->force;
             desiredModeOpt = std::move(desiredMode);
             desiredModeOpt->emitEvent |= emitEvent;
-            if (FlagManager::getInstance().connected_display()) {
-                desiredModeOpt->force |= force;
-            }
+            desiredModeOpt->force |= force;
             return DesiredModeAction::None;
         }
 
@@ -191,7 +189,7 @@ auto DisplayModeController::initiateModeChange(
     // cleared until the next `SF::initiateDisplayModeChanges`. However, the desired mode has been
     // consumed at this point, so clear the `force` flag to prevent an endless loop of
     // `initiateModeChange`.
-    if (FlagManager::getInstance().connected_display()) {
+    {
         std::scoped_lock lock(displayPtr->desiredModeLock);
         if (displayPtr->desiredModeOpt) {
             displayPtr->desiredModeOpt->force = false;

services/surfaceflinger/DisplayHardware/HWC2.cpp

@@ -439,11 +439,8 @@ Error Display::setActiveConfigWithConstraints(hal::HWConfigId configId,
     // FIXME (b/319505580): At least the first config set on an external display must be
     // `setActiveConfig`, so skip over the block that calls `setActiveConfigWithConstraints`
     // for simplicity.
-    const bool connected_display = FlagManager::getInstance().connected_display();
-
     if (isVsyncPeriodSwitchSupported() &&
-        (!connected_display ||
-         getConnectionType().value_opt() != ui::DisplayConnectionType::External)) {
+        getConnectionType().value_opt() != ui::DisplayConnectionType::External) {
         Hwc2::IComposerClient::VsyncPeriodChangeConstraints hwc2Constraints;
         hwc2Constraints.desiredTimeNanos = constraints.desiredTimeNanos;
         hwc2Constraints.seamlessRequired = constraints.seamlessRequired;

services/surfaceflinger/Scheduler/Scheduler.cpp

@@ -986,7 +986,7 @@ std::shared_ptr<VsyncSchedule> Scheduler::promotePacesetterDisplayLocked(
     if (const auto pacesetterOpt = pacesetterDisplayLocked()) {
         const Display& pacesetter = *pacesetterOpt;
 
-        if (!FlagManager::getInstance().connected_display() || params.toggleIdleTimer) {
+        if (params.toggleIdleTimer) {
             pacesetter.selectorPtr->setIdleTimerCallbacks(
                     {.platform = {.onReset = [this] { idleTimerCallback(TimerState::Reset); },
                                   .onExpired = [this] { idleTimerCallback(TimerState::Expired); }},
@@ -1018,7 +1018,7 @@ void Scheduler::applyNewVsyncSchedule(std::shared_ptr<VsyncSchedule> vsyncSchedu
 }
 
 void Scheduler::demotePacesetterDisplay(PromotionParams params) {
-    if (!FlagManager::getInstance().connected_display() || params.toggleIdleTimer) {
+    if (params.toggleIdleTimer) {
         // No need to lock for reads on kMainThreadContext.
         if (const auto pacesetterPtr =
                     FTL_FAKE_GUARD(mDisplayLock, pacesetterSelectorPtrLocked())) {

services/surfaceflinger/Scheduler/Scheduler.h

@@ -386,7 +386,7 @@ class Scheduler : public IEventThreadCallback, android::impl::MessageQueue {
     // a deadlock where the main thread joins with the timer thread as the timer thread waits to
     // lock a mutex held by the main thread.
     struct PromotionParams {
-        // Whether to stop and start the idle timer. Ignored unless connected_display flag is set.
+        // Whether to stop and start the idle timer.
         bool toggleIdleTimer;
     };
 

services/surfaceflinger/SurfaceFlinger.cpp

@@ -1009,9 +1009,8 @@ void SurfaceFlinger::init() FTL_FAKE_GUARD(kMainThreadContext) {
     mPowerAdvisor->init();
 
     if (base::GetBoolProperty("service.sf.prime_shader_cache"s, true)) {
-        if (setSchedFifo(false) != NO_ERROR) {
-            ALOGW("Can't set SCHED_OTHER for primeCache");
-        }
+        constexpr const char* kWhence = "primeCache";
+        setSchedFifo(false, kWhence);
 
         mRenderEnginePrimeCacheFuture.callOnce([this] {
             renderengine::PrimeCacheConfig config;
@@ -1047,9 +1046,7 @@ void SurfaceFlinger::init() FTL_FAKE_GUARD(kMainThreadContext) {
             return getRenderEngine().primeCache(config);
         });
 
-        if (setSchedFifo(true) != NO_ERROR) {
-            ALOGW("Can't set SCHED_FIFO after primeCache");
-        }
+        setSchedFifo(true, kWhence);
     }
 
     // Avoid blocking the main thread on `init` to set properties.
@@ -2286,8 +2283,7 @@ void SurfaceFlinger::scheduleSample() {
 
 void SurfaceFlinger::onComposerHalVsync(hal::HWDisplayId hwcDisplayId, int64_t timestamp,
                                         std::optional<hal::VsyncPeriodNanos> vsyncPeriod) {
-    if (FlagManager::getInstance().connected_display() && timestamp < 0 &&
-        vsyncPeriod.has_value()) {
+    if (timestamp < 0 && vsyncPeriod.has_value()) {
         if (mIsHdcpViaNegVsync && vsyncPeriod.value() == ~1) {
             const int32_t value = static_cast<int32_t>(-timestamp);
             // one byte is good enough to encode android.hardware.drm.HdcpLevel
@@ -3564,9 +3560,8 @@ std::pair<DisplayModes, DisplayModePtr> SurfaceFlinger::loadDisplayModes(
     std::vector<HWComposer::HWCDisplayMode> hwcModes;
     std::optional<hal::HWConfigId> activeModeHwcIdOpt;
 
-    const bool isExternalDisplay = FlagManager::getInstance().connected_display() &&
-            getHwComposer().getDisplayConnectionType(displayId) ==
-                    ui::DisplayConnectionType::External;
+    const bool isExternalDisplay = getHwComposer().getDisplayConnectionType(displayId) ==
+            ui::DisplayConnectionType::External;
 
     int attempt = 0;
     constexpr int kMaxAttempts = 3;
@@ -4066,8 +4061,7 @@ void SurfaceFlinger::processDisplayAdded(const wp<IBinder>& displayToken,
     // For an external display, loadDisplayModes already attempted to select the same mode
     // as DM, but SF still needs to be updated to match.
     // TODO (b/318534874): Let DM decide the initial mode.
-    if (const auto& physical = state.physical;
-        mScheduler && physical && FlagManager::getInstance().connected_display()) {
+    if (const auto& physical = state.physical; mScheduler && physical) {
         const bool isInternalDisplay = mPhysicalDisplays.get(physical->id)
                                                .transform(&PhysicalDisplay::isInternal)
                                                .value_or(false);
@@ -5710,16 +5704,11 @@ void SurfaceFlinger::setPowerModeInternal(const sp<DisplayDevice>& display, hal:
         }
 
         if (displayId == mActiveDisplayId) {
-            // TODO(b/281692563): Merge the syscalls. For now, keep uclamp in a separate syscall and
-            // set it before SCHED_FIFO due to b/190237315.
-            if (setSchedAttr(true) != NO_ERROR) {
-                ALOGW("Failed to set uclamp.min after powering on active display: %s",
-                      strerror(errno));
-            }
-            if (setSchedFifo(true) != NO_ERROR) {
-                ALOGW("Failed to set SCHED_FIFO after powering on active display: %s",
-                      strerror(errno));
-            }
+            // TODO: b/281692563 - Merge the syscalls. For now, keep uclamp in a separate syscall
+            // and set it before SCHED_FIFO due to b/190237315.
+            constexpr const char* kWhence = "setPowerMode(ON)";
+            setSchedAttr(true, kWhence);
+            setSchedFifo(true, kWhence);
         }
 
         getHwComposer().setPowerMode(displayId, mode);
@@ -5746,14 +5735,9 @@ void SurfaceFlinger::setPowerModeInternal(const sp<DisplayDevice>& display, hal:
             if (const auto display = getActivatableDisplay()) {
                 onActiveDisplayChangedLocked(activeDisplay.get(), *display);
             } else {
-                if (setSchedFifo(false) != NO_ERROR) {
-                    ALOGW("Failed to set SCHED_OTHER after powering off active display: %s",
-                          strerror(errno));
-                }
-                if (setSchedAttr(false) != NO_ERROR) {
-                    ALOGW("Failed set uclamp.min after powering off active display: %s",
-                          strerror(errno));
-                }
+                constexpr const char* kWhence = "setPowerMode(OFF)";
+                setSchedFifo(false, kWhence);
+                setSchedAttr(false, kWhence);
 
                 if (currentModeNotDozeSuspend) {
                     if (!FlagManager::getInstance().multithreaded_present()) {
@@ -7216,7 +7200,7 @@ static status_t validateScreenshotPermissions(const CaptureArgs& captureArgs) {
     return PERMISSION_DENIED;
 }
 
-status_t SurfaceFlinger::setSchedFifo(bool enabled) {
+void SurfaceFlinger::setSchedFifo(bool enabled, const char* whence) {
     static constexpr int kFifoPriority = 2;
     static constexpr int kOtherPriority = 0;
 
@@ -7231,19 +7215,19 @@ status_t SurfaceFlinger::setSchedFifo(bool enabled) {
     }
 
     if (sched_setscheduler(0, sched_policy, &param) != 0) {
-        return -errno;
+        const char* kPolicy[] = {"SCHED_OTHER", "SCHED_FIFO"};
+        ALOGW("%s: Failed to set %s: %s", whence, kPolicy[sched_policy == SCHED_FIFO],
+              strerror(errno));
     }
-
-    return NO_ERROR;
 }
 
-status_t SurfaceFlinger::setSchedAttr(bool enabled) {
+void SurfaceFlinger::setSchedAttr(bool enabled, const char* whence) {
     static const unsigned int kUclampMin =
             base::GetUintProperty<unsigned int>("ro.surface_flinger.uclamp.min"s, 0U);
 
     if (!kUclampMin) {
         // uclamp.min set to 0 (default), skip setting
-        return NO_ERROR;
+        return;
     }
 
     sched_attr attr = {};
@@ -7254,10 +7238,9 @@ status_t SurfaceFlinger::setSchedAttr(bool enabled) {
     attr.sched_util_max = 1024;
 
     if (syscall(__NR_sched_setattr, 0, &attr, 0)) {
-        return -errno;
+        const char* kAction[] = {"disable", "enable"};
+        ALOGW("%s: Failed to %s uclamp.min: %s", whence, kAction[enabled], strerror(errno));
     }
-
-    return NO_ERROR;
 }
 
 namespace {
@@ -8377,10 +8360,6 @@ status_t SurfaceFlinger::getStalledTransactionInfo(
 
 void SurfaceFlinger::updateHdcpLevels(hal::HWDisplayId hwcDisplayId, int32_t connectedLevel,
                                       int32_t maxLevel) {
-    if (!FlagManager::getInstance().connected_display()) {
-        return;
-    }
-
     Mutex::Autolock lock(mStateLock);
 
     const auto idOpt = getHwComposer().toPhysicalDisplayId(hwcDisplayId);

services/surfaceflinger/SurfaceFlinger.h

@@ -211,11 +211,9 @@ class SurfaceFlinger : public BnSurfaceComposer,
     SurfaceFlinger(surfaceflinger::Factory&, SkipInitializationTag) ANDROID_API;
     explicit SurfaceFlinger(surfaceflinger::Factory&) ANDROID_API;
 
-    // set main thread scheduling policy
-    static status_t setSchedFifo(bool enabled) ANDROID_API;
-
-    // set main thread scheduling attributes
-    static status_t setSchedAttr(bool enabled);
+    // Set scheduling policy and attributes of main thread.
+    static void setSchedFifo(bool enabled, const char* whence);
+    static void setSchedAttr(bool enabled, const char* whence);
 
     static char const* getServiceName() ANDROID_API { return "SurfaceFlinger"; }
 

services/surfaceflinger/common/FlagManager.cpp

@@ -139,7 +139,6 @@ void FlagManager::dump(std::string& result) const {
     DUMP_ACONFIG_FLAG(begone_bright_hlg);
     DUMP_ACONFIG_FLAG(cache_when_source_crop_layer_only_moved);
     DUMP_ACONFIG_FLAG(commit_not_composited);
-    DUMP_ACONFIG_FLAG(connected_display);
     DUMP_ACONFIG_FLAG(connected_display_hdr);
     DUMP_ACONFIG_FLAG(correct_dpi_with_display_size);
     DUMP_ACONFIG_FLAG(deprecate_frame_tracker);
@@ -252,7 +251,6 @@ FLAG_MANAGER_LEGACY_SERVER_FLAG(use_skia_tracing, PROPERTY_SKIA_ATRACE_ENABLED,
 /// Trunk stable readonly flags ///
 FLAG_MANAGER_ACONFIG_FLAG(adpf_fmq_sf, "")
 FLAG_MANAGER_ACONFIG_FLAG(arr_setframerate_gte_enum, "debug.sf.arr_setframerate_gte_enum")
-FLAG_MANAGER_ACONFIG_FLAG(connected_display, "")
 FLAG_MANAGER_ACONFIG_FLAG(enable_small_area_detection, "")
 FLAG_MANAGER_ACONFIG_FLAG(stable_edid_ids, "debug.sf.stable_edid_ids")
 FLAG_MANAGER_ACONFIG_FLAG(frame_rate_category_mrr, "debug.sf.frame_rate_category_mrr")

services/surfaceflinger/common/include/common/FlagManager.h

@@ -74,7 +74,6 @@ class FlagManager {
     bool begone_bright_hlg() const;
     bool cache_when_source_crop_layer_only_moved() const;
     bool commit_not_composited() const;
-    bool connected_display() const;
     bool connected_display_hdr() const;
     bool correct_dpi_with_display_size() const;
     bool deprecate_frame_tracker() const;

services/surfaceflinger/main_surfaceflinger.cpp

@@ -77,7 +77,7 @@ static void startDisplayService() {
     }
 }
 
-int main(int, char**) {
+int main() {
     signal(SIGPIPE, SIG_IGN);
 
     hardware::configureRpcThreadpool(1 /* maxThreads */,
@@ -91,9 +91,7 @@ int main(int, char**) {
 
     // Set uclamp.min setting on all threads, maybe an overkill but we want
     // to cover important threads like RenderEngine.
-    if (SurfaceFlinger::setSchedAttr(true) != NO_ERROR) {
-        ALOGW("Failed to set uclamp.min during boot: %s", strerror(errno));
-    }
+    SurfaceFlinger::setSchedAttr(true, __func__);
 
     // The binder threadpool we start will inherit sched policy and priority
     // of (this) creating thread. We want the binder thread pool to have
@@ -160,14 +158,8 @@ int main(int, char**) {
 
     startDisplayService(); // dependency on SF getting registered above
 
-    if (SurfaceFlinger::setSchedFifo(true) != NO_ERROR) {
-        ALOGW("Failed to set SCHED_FIFO during boot: %s", strerror(errno));
-    }
-
-    // run surface flinger in this thread
+    SurfaceFlinger::setSchedFifo(true, __func__);
     flinger->run();
-
-    return 0;
 }
 
 // TODO(b/129481165): remove the #pragma below and fix conversion issues

services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp

@@ -407,8 +407,6 @@ TEST_F(DisplayModeSwitchingTest, changeResolutionSynced) {
 }
 
 TEST_F(DisplayModeSwitchingTest, innerXorOuterDisplay) {
-    SET_FLAG_FOR_TEST(flags::connected_display, true);
-
     const auto [innerDisplay, outerDisplay] = injectOuterDisplay();
 
     EXPECT_TRUE(innerDisplay->isPoweredOn());
@@ -473,8 +471,6 @@ TEST_F(DisplayModeSwitchingTest, innerXorOuterDisplay) {
 }
 
 TEST_F(DisplayModeSwitchingTest, innerAndOuterDisplay) {
-    SET_FLAG_FOR_TEST(flags::connected_display, true);
-
     const auto [innerDisplay, outerDisplay] = injectOuterDisplay();
 
     EXPECT_TRUE(innerDisplay->isPoweredOn());
@@ -543,8 +539,6 @@ TEST_F(DisplayModeSwitchingTest, powerOffDuringModeSet) {
 }
 
 TEST_F(DisplayModeSwitchingTest, powerOffDuringConcurrentModeSet) {
-    SET_FLAG_FOR_TEST(flags::connected_display, true);
-
     const auto [innerDisplay, outerDisplay] = injectOuterDisplay();
 
     EXPECT_TRUE(innerDisplay->isPoweredOn());