Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit c75b0777 authored by Marin Shalamanov's avatar Marin Shalamanov
Browse files

[SF] Don't cache VSYNC state in SF 

This change removes the cached state in SF which is not managed
properly. This state is also cached in HWComposer.

More specifically, SF caches the primary display vsync state in
mHWCVsyncState. However executing DisplayDevice->disconnect()
turns off vsync w/o updating the cache. This way if we
process a subsequent hotplug event for the primary display
we can end up a situation where the cached state is "enabled",
but the actual state is "disabled". Then Scheduler won't be
able to turn on HWC VSYNC again.

Test: manually test on a TV which sends two
      subsequent hotplug events at boot. Verify that
      after applying this change the TV doesn't
      run at 1 fps.
Test: atest libsurfaceflinger_unittest
Bug: 156331731
Change-Id: I85e72d8d5ba85f0a5f4b4f27a200c1bde4e0b9f4
parent 059a108e

services/surfaceflinger/SurfaceFlinger.cpp

+3 −10
Original line number Original line Diff line number Diff line
@@ -1690,7 +1690,7 @@ void SurfaceFlinger::setPrimaryVsyncEnabledInternal(bool enabled) { 
    if (const auto displayId = getInternalDisplayIdLocked()) {

    if (const auto displayId = getInternalDisplayIdLocked()) {

        sp<DisplayDevice> display = getDefaultDisplayDeviceLocked();

        sp<DisplayDevice> display = getDefaultDisplayDeviceLocked();

        if (display && display->isPoweredOn()) {

        if (display && display->isPoweredOn()) {

            setVsyncEnabledInHWC(*displayId, mHWCVsyncPendingState);

            getHwComposer().setVsyncEnabled(*displayId, mHWCVsyncPendingState);

        }

        }

    }

    }

}

}
@@ -4173,13 +4173,6 @@ void SurfaceFlinger::initializeDisplays() { 
    static_cast<void>(schedule([this]() MAIN_THREAD { onInitializeDisplays(); }));

    static_cast<void>(schedule([this]() MAIN_THREAD { onInitializeDisplays(); }));

}

}





void SurfaceFlinger::setVsyncEnabledInHWC(DisplayId displayId, hal::Vsync enabled) {

    if (mHWCVsyncState != enabled) {

        getHwComposer().setVsyncEnabled(displayId, enabled);

        mHWCVsyncState = enabled;

    }

}



void SurfaceFlinger::setPowerModeInternal(const sp<DisplayDevice>& display, hal::PowerMode mode) {

void SurfaceFlinger::setPowerModeInternal(const sp<DisplayDevice>& display, hal::PowerMode mode) {

    if (display->isVirtual()) {

    if (display->isVirtual()) {

        ALOGE("%s: Invalid operation on virtual display", __FUNCTION__);

        ALOGE("%s: Invalid operation on virtual display", __FUNCTION__);
@@ -4208,7 +4201,7 @@ void SurfaceFlinger::setPowerModeInternal(const sp<DisplayDevice>& display, hal: 
        }

        }

        getHwComposer().setPowerMode(*displayId, mode);

        getHwComposer().setPowerMode(*displayId, mode);

        if (display->isPrimary() && mode != hal::PowerMode::DOZE_SUSPEND) {

        if (display->isPrimary() && mode != hal::PowerMode::DOZE_SUSPEND) {

            setVsyncEnabledInHWC(*displayId, mHWCVsyncPendingState);

            getHwComposer().setVsyncEnabled(*displayId, mHWCVsyncPendingState);

            mScheduler->onScreenAcquired(mAppConnectionHandle);

            mScheduler->onScreenAcquired(mAppConnectionHandle);

            mScheduler->resyncToHardwareVsync(true, getVsyncPeriod());

            mScheduler->resyncToHardwareVsync(true, getVsyncPeriod());

        }

        }
@@ -4227,7 +4220,7 @@ void SurfaceFlinger::setPowerModeInternal(const sp<DisplayDevice>& display, hal: 
        }

        }





        // Make sure HWVsync is disabled before turning off the display

        // Make sure HWVsync is disabled before turning off the display

        setVsyncEnabledInHWC(*displayId, hal::Vsync::DISABLE);

        getHwComposer().setVsyncEnabled(*displayId, hal::Vsync::DISABLE);





        getHwComposer().setPowerMode(*displayId, mode);

        getHwComposer().setPowerMode(*displayId, mode);

        mVisibleRegionsDirty = true;

        mVisibleRegionsDirty = true;

services/surfaceflinger/SurfaceFlinger.h

+1 −5
Original line number Original line Diff line number Diff line
@@ -302,7 +302,6 @@ public: 




    // main thread function to enable/disable h/w composer event

    // main thread function to enable/disable h/w composer event

    void setPrimaryVsyncEnabledInternal(bool enabled) REQUIRES(mStateLock);

    void setPrimaryVsyncEnabledInternal(bool enabled) REQUIRES(mStateLock);

    void setVsyncEnabledInHWC(DisplayId displayId, hal::Vsync enabled);





    // called on the main thread by MessageQueue when an internal message

    // called on the main thread by MessageQueue when an internal message

    // is received

    // is received
@@ -1209,6 +1208,7 @@ private: 
    std::unique_ptr<scheduler::RefreshRateStats> mRefreshRateStats;

    std::unique_ptr<scheduler::RefreshRateStats> mRefreshRateStats;





    std::atomic<nsecs_t> mExpectedPresentTime = 0;

    std::atomic<nsecs_t> mExpectedPresentTime = 0;

    hal::Vsync mHWCVsyncPendingState = hal::Vsync::DISABLE;





    /* ------------------------------------------------------------------------

    /* ------------------------------------------------------------------------

     * Generic Layer Metadata

     * Generic Layer Metadata
@@ -1279,10 +1279,6 @@ private: 
    // be any issues with a raw pointer referencing an invalid object.

    // be any issues with a raw pointer referencing an invalid object.

    std::unordered_set<Layer*> mOffscreenLayers;

    std::unordered_set<Layer*> mOffscreenLayers;





    // Flags to capture the state of Vsync in HWC

    hal::Vsync mHWCVsyncState = hal::Vsync::DISABLE;

    hal::Vsync mHWCVsyncPendingState = hal::Vsync::DISABLE;



    // Fields tracking the current jank event: when it started and how many

    // Fields tracking the current jank event: when it started and how many

    // janky frames there are.

    // janky frames there are.

    nsecs_t mMissedFrameJankStart = 0;

    nsecs_t mMissedFrameJankStart = 0;