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

Commit 2492a02b authored by Ady Abraham's avatar Ady Abraham
Browse files

SF: use cached vsync period 

Don't query the vsync period from hwc but instead use the cached
value in SF. The reason for this change is a race condition with AOD:
1. SF changes refresh rate to from X to Y
2. HWC is caching Y since the display is already in AOD and can't
   change refresh rate.
3. SF calls HWC to get the display out from AOD and query the refresh rate.
   At this point HWC returns X as the refresh rate didn't change yet.
4. HWC changes the refresh rate to Y.
5. SF thinks the refresh rate is X -- mismatch.

Test: enable / disable AOD in a loop
Bug: 160966959

Change-Id: I127c3a445416607dee9402031e07c8fece7d167b
parent dce956c3

services/surfaceflinger/SurfaceFlinger.cpp

+8 −9
Original line number Diff line number Diff line
@@ -1567,7 +1567,7 @@ void SurfaceFlinger::signalRefresh() { 
    mEventQueue->refresh();

}



nsecs_t SurfaceFlinger::getVsyncPeriod() const {

nsecs_t SurfaceFlinger::getVsyncPeriodFromHWC() const {

    const auto displayId = getInternalDisplayIdLocked();

    if (!displayId || !getHwComposer().isConnected(*displayId)) {

        return 0;
@@ -1774,7 +1774,7 @@ void SurfaceFlinger::updateVrFlinger() { 
    setPowerModeInternal(display, currentDisplayPowerMode);



    // Reset the timing values to account for the period of the swapped in HWC

    const nsecs_t vsyncPeriod = getVsyncPeriod();

    const nsecs_t vsyncPeriod = mRefreshRateConfigs->getCurrentRefreshRate().getVsyncPeriod();

    mAnimFrameTracker.setDisplayRefreshPeriod(vsyncPeriod);



    // The present fences returned from vr_hwc are not an accurate
@@ -4208,8 +4208,7 @@ void SurfaceFlinger::onInitializeDisplays() { 
                        {});



    setPowerModeInternal(display, hal::PowerMode::ON);



    const nsecs_t vsyncPeriod = getVsyncPeriod();

    const nsecs_t vsyncPeriod = mRefreshRateConfigs->getCurrentRefreshRate().getVsyncPeriod();

    mAnimFrameTracker.setDisplayRefreshPeriod(vsyncPeriod);



    // Use phase of 0 since phase is not known.
@@ -4244,7 +4243,7 @@ void SurfaceFlinger::setPowerModeInternal(const sp<DisplayDevice>& display, hal: 
    if (mInterceptor->isEnabled()) {

        mInterceptor->savePowerModeUpdate(display->getSequenceId(), static_cast<int32_t>(mode));

    }



    const auto vsyncPeriod = mRefreshRateConfigs->getCurrentRefreshRate().getVsyncPeriod();

    if (currentMode == hal::PowerMode::OFF) {

        if (SurfaceFlinger::setSchedFifo(true) != NO_ERROR) {

            ALOGW("Couldn't set SCHED_FIFO on display on: %s\n", strerror(errno));
@@ -4253,7 +4252,7 @@ void SurfaceFlinger::setPowerModeInternal(const sp<DisplayDevice>& display, hal: 
        if (display->isPrimary() && mode != hal::PowerMode::DOZE_SUSPEND) {

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

            mScheduler->onScreenAcquired(mAppConnectionHandle);

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

            mScheduler->resyncToHardwareVsync(true, vsyncPeriod);

        }



        mVisibleRegionsDirty = true;
@@ -4280,7 +4279,7 @@ void SurfaceFlinger::setPowerModeInternal(const sp<DisplayDevice>& display, hal: 
        getHwComposer().setPowerMode(*displayId, mode);

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

            mScheduler->onScreenAcquired(mAppConnectionHandle);

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

            mScheduler->resyncToHardwareVsync(true, vsyncPeriod);

        }

    } else if (mode == hal::PowerMode::DOZE_SUSPEND) {

        // Leave display going to doze
@@ -4393,7 +4392,7 @@ void SurfaceFlinger::listLayersLocked(std::string& result) const { 
}



void SurfaceFlinger::dumpStatsLocked(const DumpArgs& args, std::string& result) const {

    StringAppendF(&result, "%" PRId64 "\n", getVsyncPeriod());

    StringAppendF(&result, "%" PRId64 "\n", getVsyncPeriodFromHWC());



    if (args.size() > 1) {

        const auto name = String8(args[1]);
@@ -4458,7 +4457,7 @@ void SurfaceFlinger::dumpVSync(std::string& result) const { 
    mPhaseConfiguration->dump(result);

    StringAppendF(&result,

                  "      present offset: %9" PRId64 " ns\t     VSYNC period: %9" PRId64 " ns\n\n",

                  dispSyncPresentTimeOffset, getVsyncPeriod());

                  dispSyncPresentTimeOffset, getVsyncPeriodFromHWC());



    scheduler::RefreshRateConfigs::Policy policy = mRefreshRateConfigs->getDisplayManagerPolicy();

    StringAppendF(&result,

services/surfaceflinger/SurfaceFlinger.h

+1 −1
Original line number Diff line number Diff line
@@ -856,7 +856,7 @@ private: 
    /* ------------------------------------------------------------------------

     * VSync

     */

    nsecs_t getVsyncPeriod() const REQUIRES(mStateLock);

    nsecs_t getVsyncPeriodFromHWC() const REQUIRES(mStateLock);



    // Sets the refresh rate by switching active configs, if they are available for

    // the desired refresh rate.

services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp

+1 −1
Original line number Diff line number Diff line
@@ -88,7 +88,7 @@ using FakeHwcDisplayInjector = TestableSurfaceFlinger::FakeHwcDisplayInjector; 
using HotplugEvent = TestableSurfaceFlinger::HotplugEvent;

using HWC2Display = TestableSurfaceFlinger::HWC2Display;



constexpr int32_t DEFAULT_REFRESH_RATE = 16'666'666;

constexpr int32_t DEFAULT_REFRESH_RATE = 16'666'667;

constexpr int32_t DEFAULT_DPI = 320;

constexpr int DEFAULT_VIRTUAL_DISPLAY_SURFACE_FORMAT = HAL_PIXEL_FORMAT_RGB_565;