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

Commit c67d22c8 authored by ramindani's avatar ramindani
Browse files

[SF] Move the notifyExpectedPresentHint call to SF 

This moves the notifyExpectedPresent call off the HWComposer,
HWComposer should only be access with mStateLock or from the main
thread, and moving this to SF achieves that.

Schedule the HWComposer::notifyExpectedPresent
call on the main thread once the decision to
send the hint is made

BUG: 311300327
Test: atest
Change-Id: Ia5f92546028ce104e391364c6696415c29760232
parent 9a296f97

services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h

+1 −2
Original line number Diff line number Diff line
@@ -147,8 +147,7 @@ public: 
    MOCK_METHOD(const aidl::android::hardware::graphics::composer3::OverlayProperties&,

                getOverlaySupport, (), (const, override));

    MOCK_METHOD(status_t, setRefreshRateChangedCallbackDebugEnabled, (PhysicalDisplayId, bool));

    MOCK_METHOD(status_t, notifyExpectedPresentIfRequired,

                (PhysicalDisplayId, Period, TimePoint, Fps, std::optional<Period>));

    MOCK_METHOD(status_t, notifyExpectedPresent, (PhysicalDisplayId, TimePoint, Fps));

};



} // namespace mock

services/surfaceflinger/DisplayHardware/HWComposer.cpp

+7 −106
Original line number Diff line number Diff line
@@ -15,6 +15,7 @@ 
 */



// TODO(b/129481165): remove the #pragma below and fix conversion issues

#include <chrono>

#pragma clang diagnostic push

#pragma clang diagnostic ignored "-Wconversion"
@@ -78,59 +79,6 @@ using aidl::android::hardware::graphics::composer3::Capability; 
using aidl::android::hardware::graphics::composer3::DisplayCapability;

namespace hal = android::hardware::graphics::composer::hal;



namespace {

bool isFrameIntervalOnCadence(android::TimePoint expectedPresentTime,

                              android::TimePoint lastExpectedPresentTimestamp,

                              android::Fps lastFrameInterval, android::Period timeout,

                              android::Duration threshold) {

    if (lastFrameInterval.getPeriodNsecs() == 0) {

        return false;

    }



    const auto expectedPresentTimeDeltaNs =

            expectedPresentTime.ns() - lastExpectedPresentTimestamp.ns();



    if (expectedPresentTimeDeltaNs > timeout.ns()) {

        return false;

    }



    const auto expectedPresentPeriods = static_cast<nsecs_t>(

            std::round(static_cast<float>(expectedPresentTimeDeltaNs) /

                       static_cast<float>(lastFrameInterval.getPeriodNsecs())));

    const auto calculatedPeriodsOutNs = lastFrameInterval.getPeriodNsecs() * expectedPresentPeriods;

    const auto calculatedExpectedPresentTimeNs =

            lastExpectedPresentTimestamp.ns() + calculatedPeriodsOutNs;

    const auto presentTimeDelta =

            std::abs(expectedPresentTime.ns() - calculatedExpectedPresentTimeNs);

    return presentTimeDelta < threshold.ns();

}



bool isExpectedPresentWithinTimeout(android::TimePoint expectedPresentTime,

                                    android::TimePoint lastExpectedPresentTimestamp,

                                    std::optional<android::Period> timeoutOpt,

                                    android::Duration threshold) {

    if (!timeoutOpt) {

        // Always within timeout if timeoutOpt is absent and don't send hint

        // for the timeout

        return true;

    }



    if (timeoutOpt->ns() == 0) {

        // Always outside timeout if timeoutOpt is 0 and always send

        // the hint for the timeout.

        return false;

    }



    if (expectedPresentTime.ns() < lastExpectedPresentTimestamp.ns() + timeoutOpt->ns()) {

        return true;

    }



    // Check if within the threshold as it can be just outside the timeout

    return std::abs(expectedPresentTime.ns() -

                    (lastExpectedPresentTimestamp.ns() + timeoutOpt->ns())) < threshold.ns();

}

} // namespace



namespace android {



HWComposer::~HWComposer() = default;
@@ -538,13 +486,6 @@ status_t HWComposer::getDeviceCompositionChanges( 
    }();



    displayData.validateWasSkipped = false;

    {

        std::scoped_lock lock{displayData.expectedPresentLock};

        if (expectedPresentTime > displayData.lastExpectedPresentTimestamp.ns()) {

            displayData.lastExpectedPresentTimestamp = TimePoint::fromNs(expectedPresentTime);

        }

    }



    ATRACE_FORMAT("NextFrameInterval %d_Hz", frameInterval.getIntValue());

    if (canSkipValidate) {

        sp<Fence> outPresentFence = Fence::NO_FENCE;
@@ -939,55 +880,15 @@ status_t HWComposer::setRefreshRateChangedCallbackDebugEnabled(PhysicalDisplayId 
    return NO_ERROR;

}



status_t HWComposer::notifyExpectedPresentIfRequired(PhysicalDisplayId displayId,

                                                     Period vsyncPeriod,

                                                     TimePoint expectedPresentTime,

                                                     Fps frameInterval,

                                                     std::optional<Period> timeoutOpt) {

status_t HWComposer::notifyExpectedPresent(PhysicalDisplayId displayId,

                                           TimePoint expectedPresentTime, Fps frameInterval) {

    RETURN_IF_INVALID_DISPLAY(displayId, BAD_INDEX);

    auto& displayData = mDisplayData[displayId];

    if (!displayData.hwcDisplay) {

        // Display setup has not completed yet

        return BAD_INDEX;

    }

    {

        std::scoped_lock lock{displayData.expectedPresentLock};

        const auto lastExpectedPresentTimestamp = displayData.lastExpectedPresentTimestamp;

        const auto lastFrameInterval = displayData.lastFrameInterval;

        displayData.lastFrameInterval = frameInterval;

        const auto threshold = Duration::fromNs(vsyncPeriod.ns() / 2);



        const constexpr nsecs_t kOneSecondNs =

                std::chrono::duration_cast<std::chrono::nanoseconds>(1s).count();

        const bool frameIntervalIsOnCadence =

                isFrameIntervalOnCadence(expectedPresentTime, lastExpectedPresentTimestamp,

                                         lastFrameInterval,

                                         Period::fromNs(timeoutOpt && timeoutOpt->ns() > 0

                                                                ? timeoutOpt->ns()

                                                                : kOneSecondNs),

                                         threshold);



        const bool expectedPresentWithinTimeout =

                isExpectedPresentWithinTimeout(expectedPresentTime, lastExpectedPresentTimestamp,

                                               timeoutOpt, threshold);



        using fps_approx_ops::operator!=;

        if (frameIntervalIsOnCadence && frameInterval != lastFrameInterval) {

            displayData.lastExpectedPresentTimestamp = expectedPresentTime;

        }



        if (expectedPresentWithinTimeout && frameIntervalIsOnCadence) {

            return NO_ERROR;

        }



        displayData.lastExpectedPresentTimestamp = expectedPresentTime;

    }

    ATRACE_FORMAT("%s ExpectedPresentTime %" PRId64 " frameIntervalNs %d", __func__,

                  expectedPresentTime, frameInterval.getPeriodNsecs());

    const auto error = mComposer->notifyExpectedPresent(displayData.hwcDisplay->getId(),

    ATRACE_FORMAT("%s ExpectedPresentTime in %.2fms frameInterval %.2fms", __func__,

                  ticks<std::milli, float>(expectedPresentTime - TimePoint::now()),

                  ticks<std::milli, float>(Duration::fromNs(frameInterval.getPeriodNsecs())));

    const auto error = mComposer->notifyExpectedPresent(mDisplayData[displayId].hwcDisplay->getId(),

                                                        expectedPresentTime.ns(),

                                                        frameInterval.getPeriodNsecs());



    if (error != hal::Error::NONE) {

        ALOGE("Error in notifyExpectedPresent call %s", to_string(error).c_str());

        return INVALID_OPERATION;

services/surfaceflinger/DisplayHardware/HWComposer.h

+4 −13
Original line number Diff line number Diff line
@@ -302,10 +302,8 @@ public: 
            aidl::android::hardware::graphics::common::HdrConversionStrategy,

            aidl::android::hardware::graphics::common::Hdr*) = 0;

    virtual status_t setRefreshRateChangedCallbackDebugEnabled(PhysicalDisplayId, bool enabled) = 0;

    virtual status_t notifyExpectedPresentIfRequired(PhysicalDisplayId, Period vsyncPeriod,

                                                     TimePoint expectedPresentTime,

                                                     Fps frameInterval,

                                                     std::optional<Period> timeoutOpt) = 0;

    virtual status_t notifyExpectedPresent(PhysicalDisplayId, TimePoint expectedPresentTime,

                                           Fps frameInterval) = 0;

};



static inline bool operator==(const android::HWComposer::DeviceRequestedChanges& lhs,
@@ -464,9 +462,8 @@ public: 
            aidl::android::hardware::graphics::common::HdrConversionStrategy,

            aidl::android::hardware::graphics::common::Hdr*) override;

    status_t setRefreshRateChangedCallbackDebugEnabled(PhysicalDisplayId, bool enabled) override;

    status_t notifyExpectedPresentIfRequired(PhysicalDisplayId, Period vsyncPeriod,

                                             TimePoint expectedPresentTime, Fps frameInterval,

                                             std::optional<Period> timeoutOpt) override;

    status_t notifyExpectedPresent(PhysicalDisplayId, TimePoint expectedPresentTime,

                                   Fps frameInterval) override;



    // for debugging ----------------------------------------------------------

    void dump(std::string& out) const override;
@@ -492,7 +489,6 @@ public: 
private:

    // For unit tests

    friend TestableSurfaceFlinger;

    friend HWComposerTest;



    struct DisplayData {

        std::unique_ptr<HWC2::Display> hwcDisplay;
@@ -500,11 +496,6 @@ private: 
        sp<Fence> lastPresentFence = Fence::NO_FENCE; // signals when the last set op retires

        nsecs_t lastPresentTimestamp = 0;



        std::mutex expectedPresentLock;

        TimePoint lastExpectedPresentTimestamp GUARDED_BY(expectedPresentLock) =

                TimePoint::fromNs(0);

        Fps lastFrameInterval GUARDED_BY(expectedPresentLock) = Fps::fromValue(0);



        std::unordered_map<HWC2::Layer*, sp<Fence>> releaseFences;



        bool validateWasSkipped;

services/surfaceflinger/SurfaceFlinger.cpp

+107 −9
Original line number Diff line number Diff line
@@ -318,6 +318,53 @@ bool fileNewerThan(const std::string& path, std::chrono::minutes duration) { 
    return duration > (Clock::now() - updateTime);

}



bool isFrameIntervalOnCadence(TimePoint expectedPresentTime, TimePoint lastExpectedPresentTimestamp,

                              Fps lastFrameInterval, Period timeout, Duration threshold) {

    if (lastFrameInterval.getPeriodNsecs() == 0) {

        return false;

    }



    const auto expectedPresentTimeDeltaNs =

            expectedPresentTime.ns() - lastExpectedPresentTimestamp.ns();



    if (expectedPresentTimeDeltaNs > timeout.ns()) {

        return false;

    }



    const auto expectedPresentPeriods = static_cast<nsecs_t>(

            std::round(static_cast<float>(expectedPresentTimeDeltaNs) /

                       static_cast<float>(lastFrameInterval.getPeriodNsecs())));

    const auto calculatedPeriodsOutNs = lastFrameInterval.getPeriodNsecs() * expectedPresentPeriods;

    const auto calculatedExpectedPresentTimeNs =

            lastExpectedPresentTimestamp.ns() + calculatedPeriodsOutNs;

    const auto presentTimeDelta =

            std::abs(expectedPresentTime.ns() - calculatedExpectedPresentTimeNs);

    return presentTimeDelta < threshold.ns();

}



bool isExpectedPresentWithinTimeout(TimePoint expectedPresentTime,

                                    TimePoint lastExpectedPresentTimestamp,

                                    std::optional<Period> timeoutOpt, Duration threshold) {

    if (!timeoutOpt) {

        // Always within timeout if timeoutOpt is absent and don't send hint

        // for the timeout

        return true;

    }



    if (timeoutOpt->ns() == 0) {

        // Always outside timeout if timeoutOpt is 0 and always send

        // the hint for the timeout.

        return false;

    }



    if (expectedPresentTime.ns() < lastExpectedPresentTimestamp.ns() + timeoutOpt->ns()) {

        return true;

    }



    // Check if within the threshold as it can be just outside the timeout

    return std::abs(expectedPresentTime.ns() -

                    (lastExpectedPresentTimestamp.ns() + timeoutOpt->ns())) < threshold.ns();

}

}  // namespace anonymous



// ---------------------------------------------------------------------------
@@ -2702,7 +2749,18 @@ CompositeResultsPerDisplay SurfaceFlinger::composite( 
    refreshArgs.scheduledFrameTime = mScheduler->getScheduledFrameTime();

    refreshArgs.expectedPresentTime = expectedPresentTime.ns();

    refreshArgs.hasTrustedPresentationListener = mNumTrustedPresentationListeners > 0;



    {

        auto& notifyExpectedPresentData = mNotifyExpectedPresentMap[pacesetterId];

        auto lastExpectedPresentTimestamp = TimePoint::fromNs(

                notifyExpectedPresentData.lastExpectedPresentTimestamp.load().ns());

        if (expectedPresentTime > lastExpectedPresentTimestamp) {

            // If the values are not same, then hint is sent with newer value.

            // And because composition always follows the notifyExpectedPresentIfRequired, we can

            // skip updating the lastExpectedPresentTimestamp in this case.

            notifyExpectedPresentData.lastExpectedPresentTimestamp

                    .compare_exchange_weak(lastExpectedPresentTimestamp, expectedPresentTime);

        }

    }

    // Store the present time just before calling to the composition engine so we could notify

    // the scheduler.

    const auto presentTime = systemTime();
@@ -4053,7 +4111,7 @@ void SurfaceFlinger::onChoreographerAttached() { 
void SurfaceFlinger::onVsyncGenerated(TimePoint expectedPresentTime,

                                      ftl::NonNull<DisplayModePtr> modePtr, Fps renderRate) {

    const auto vsyncPeriod = modePtr->getVsyncRate().getPeriod();

    const auto timeout = [&]() -> std::optional<Period> {

    const auto timeoutOpt = [&]() -> std::optional<Period> {

        const auto vrrConfig = modePtr->getVrrConfig();

        if (!vrrConfig) return std::nullopt;
@@ -4063,14 +4121,54 @@ void SurfaceFlinger::onVsyncGenerated(TimePoint expectedPresentTime, 
        return Period::fromNs(notifyExpectedPresentConfig->notifyExpectedPresentTimeoutNs);

    }();



    const auto displayId = modePtr->getPhysicalDisplayId();

    const auto status = getHwComposer().notifyExpectedPresentIfRequired(displayId, vsyncPeriod,

                                                                        expectedPresentTime,

                                                                        renderRate, timeout);

    notifyExpectedPresentIfRequired(modePtr->getPhysicalDisplayId(), vsyncPeriod,

                                    expectedPresentTime, renderRate, timeoutOpt);

}



void SurfaceFlinger::notifyExpectedPresentIfRequired(PhysicalDisplayId displayId,

                                                     Period vsyncPeriod,

                                                     TimePoint expectedPresentTime,

                                                     Fps frameInterval,

                                                     std::optional<Period> timeoutOpt) {

    {

        auto& data = mNotifyExpectedPresentMap[displayId];

        const auto lastExpectedPresentTimestamp = data.lastExpectedPresentTimestamp.load();

        const auto lastFrameInterval = data.lastFrameInterval;

        data.lastFrameInterval = frameInterval;

        const auto threshold = Duration::fromNs(vsyncPeriod.ns() / 2);



        const constexpr nsecs_t kOneSecondNs =

                std::chrono::duration_cast<std::chrono::nanoseconds>(1s).count();

        const auto timeout = Period::fromNs(timeoutOpt && timeoutOpt->ns() > 0 ? timeoutOpt->ns()

                                                                               : kOneSecondNs);

        const bool frameIntervalIsOnCadence =

                isFrameIntervalOnCadence(expectedPresentTime, lastExpectedPresentTimestamp,

                                         lastFrameInterval, timeout, threshold);



        const bool expectedPresentWithinTimeout =

                isExpectedPresentWithinTimeout(expectedPresentTime, lastExpectedPresentTimestamp,

                                               timeoutOpt, threshold);



        using fps_approx_ops::operator!=;

        if (frameIntervalIsOnCadence && frameInterval != lastFrameInterval) {

            data.lastExpectedPresentTimestamp = expectedPresentTime;

        }



        if (expectedPresentWithinTimeout && frameIntervalIsOnCadence) {

            return;

        }

        data.lastExpectedPresentTimestamp = expectedPresentTime;

    }



    const char* const whence = __func__;

    static_cast<void>(mScheduler->schedule([=, this]() FTL_FAKE_GUARD(kMainThreadContext) {

        const auto status = getHwComposer().notifyExpectedPresent(displayId, expectedPresentTime,

                                                                  frameInterval);

        if (status != NO_ERROR) {

        ALOGE("%s failed to notifyExpectedPresentHint for display %" PRId64, __func__,

            ALOGE("%s failed to notifyExpectedPresentHint for display %" PRId64, whence,

                  displayId.value);

        }

    }));

}



void SurfaceFlinger::initScheduler(const sp<const DisplayDevice>& display) {

services/surfaceflinger/SurfaceFlinger.h

+13 −0
Original line number Diff line number Diff line
@@ -1460,6 +1460,19 @@ private: 
    // Map of displayid to mirrorRoot

    ftl::SmallMap<int64_t, sp<SurfaceControl>, 3> mMirrorMapForDebug;



    // NotifyExpectedPresentHint

    struct NotifyExpectedPresentData {

        // lastExpectedPresentTimestamp is read and write from multiple threads such as

        // main thread, EventThread, MessageQueue. And is atomic for that reason.

        std::atomic<TimePoint> lastExpectedPresentTimestamp{};

        Fps lastFrameInterval{};

    };

    std::unordered_map<PhysicalDisplayId, NotifyExpectedPresentData> mNotifyExpectedPresentMap;



    void notifyExpectedPresentIfRequired(PhysicalDisplayId, Period vsyncPeriod,

                                         TimePoint expectedPresentTime, Fps frameInterval,

                                         std::optional<Period> timeoutOpt);



    void sfdo_enableRefreshRateOverlay(bool active);

    void sfdo_setDebugFlash(int delay);

    void sfdo_scheduleComposite();