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

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

Remove frame rate flexibility token 

Remove the frame rate flexiblity token which can be replaced with

    DisplayManager.setRefreshRateSwitchingType(
          DisplayManager.SWITCHING_TYPE_ACROSS_AND_WITHIN_GROUPS);
    DisplayManager.setShouldAlwaysRespectAppRequestedMode(true);

Bug: 175371491
Test: m

Test: manually test that backdoor still works
  adb shell dumpsys SurfaceFlinger | grep -A 15 "DesiredDisplayModeSpecs"
  verify no override

  adb shell service call SurfaceFlinger 1036 i32 1
  adb shell dumpsys SurfaceFlinger | grep -A 15 "DesiredDisplayModeSpecs"
  verify there is override

  adb shell service call SurfaceFlinger 1036 i32 0
  adb shell dumpsys SurfaceFlinger | grep -A 15 "DesiredDisplayModeSpecs"
  verify no override

  verify idempotence:
  adb shell service call SurfaceFlinger 1036 i32 0
  adb shell service call SurfaceFlinger 1036 i32 0
  adb shell service call SurfaceFlinger 1036 i32 1
  adb shell service call SurfaceFlinger 1036 i32 1

Change-Id: I848f849037da38d245e940bc9c85727129463a81
parent ccfc0b73

libs/gui/ISurfaceComposer.cpp

+0 −45
Original line number Original line Diff line number Diff line
@@ -1150,41 +1150,6 @@ public: 
        return reply.readInt32();

        return reply.readInt32();

    }

    }





    status_t acquireFrameRateFlexibilityToken(sp<IBinder>* outToken) override {

        if (!outToken) return BAD_VALUE;



        Parcel data, reply;

        status_t err = data.writeInterfaceToken(ISurfaceComposer::getInterfaceDescriptor());

        if (err != NO_ERROR) {

            ALOGE("acquireFrameRateFlexibilityToken: failed writing interface token: %s (%d)",

                  strerror(-err), -err);

            return err;

        }



        err = remote()->transact(BnSurfaceComposer::ACQUIRE_FRAME_RATE_FLEXIBILITY_TOKEN, data,

                                 &reply);

        if (err != NO_ERROR) {

            ALOGE("acquireFrameRateFlexibilityToken: failed to transact: %s (%d)", strerror(-err),

                  err);

            return err;

        }



        err = reply.readInt32();

        if (err != NO_ERROR) {

            ALOGE("acquireFrameRateFlexibilityToken: call failed: %s (%d)", strerror(-err), err);

            return err;

        }



        err = reply.readStrongBinder(outToken);

        if (err != NO_ERROR) {

            ALOGE("acquireFrameRateFlexibilityToken: failed reading binder token: %s (%d)",

                  strerror(-err), err);

            return err;

        }



        return NO_ERROR;

    }



    status_t setFrameTimelineInfo(const sp<IGraphicBufferProducer>& surface,

    status_t setFrameTimelineInfo(const sp<IGraphicBufferProducer>& surface,

                                  const FrameTimelineInfo& frameTimelineInfo) override {

                                  const FrameTimelineInfo& frameTimelineInfo) override {

        Parcel data, reply;

        Parcel data, reply;
@@ -2073,16 +2038,6 @@ status_t BnSurfaceComposer::onTransact( 
            reply->writeInt32(result);

            reply->writeInt32(result);

            return NO_ERROR;

            return NO_ERROR;

        }

        }

        case ACQUIRE_FRAME_RATE_FLEXIBILITY_TOKEN: {

            CHECK_INTERFACE(ISurfaceComposer, data, reply);

            sp<IBinder> token;

            status_t result = acquireFrameRateFlexibilityToken(&token);

            reply->writeInt32(result);

            if (result == NO_ERROR) {

                reply->writeStrongBinder(token);

            }

            return NO_ERROR;

        }

        case SET_FRAME_TIMELINE_INFO: {

        case SET_FRAME_TIMELINE_INFO: {

            CHECK_INTERFACE(ISurfaceComposer, data, reply);

            CHECK_INTERFACE(ISurfaceComposer, data, reply);

            sp<IBinder> binder;

            sp<IBinder> binder;

libs/gui/include/gui/ISurfaceComposer.h

+1 −8
Original line number Original line Diff line number Diff line
@@ -511,14 +511,6 @@ public: 
    virtual status_t setFrameRate(const sp<IGraphicBufferProducer>& surface, float frameRate,

    virtual status_t setFrameRate(const sp<IGraphicBufferProducer>& surface, float frameRate,

                                  int8_t compatibility, int8_t changeFrameRateStrategy) = 0;

                                  int8_t compatibility, int8_t changeFrameRateStrategy) = 0;





    /*

     * Acquire a frame rate flexibility token from SurfaceFlinger. While this token is acquired,

     * surface flinger will freely switch between frame rates in any way it sees fit, regardless of

     * the current restrictions applied by DisplayManager. This is useful to get consistent behavior

     * for tests. Release the token by releasing the returned IBinder reference.

     */

    virtual status_t acquireFrameRateFlexibilityToken(sp<IBinder>* outToken) = 0;



    /*

    /*

     * Sets the frame timeline vsync info received from choreographer that corresponds to next

     * Sets the frame timeline vsync info received from choreographer that corresponds to next

     * buffer submitted on that surface.

     * buffer submitted on that surface.
@@ -616,6 +608,7 @@ public: 
        GET_GAME_CONTENT_TYPE_SUPPORT, // Deprecated. Use GET_DYNAMIC_DISPLAY_INFO instead.

        GET_GAME_CONTENT_TYPE_SUPPORT, // Deprecated. Use GET_DYNAMIC_DISPLAY_INFO instead.

        SET_GAME_CONTENT_TYPE,

        SET_GAME_CONTENT_TYPE,

        SET_FRAME_RATE,

        SET_FRAME_RATE,

        // Deprecated. Use DisplayManager.setShouldAlwaysRespectAppRequestedMode(true);

        ACQUIRE_FRAME_RATE_FLEXIBILITY_TOKEN,

        ACQUIRE_FRAME_RATE_FLEXIBILITY_TOKEN,

        SET_FRAME_TIMELINE_INFO,

        SET_FRAME_TIMELINE_INFO,

        ADD_TRANSACTION_TRACE_LISTENER,

        ADD_TRANSACTION_TRACE_LISTENER,

libs/gui/tests/Surface_test.cpp

+0 −4
Original line number Original line Diff line number Diff line
@@ -885,10 +885,6 @@ public: 
        return NO_ERROR;

        return NO_ERROR;

    }

    }





    status_t acquireFrameRateFlexibilityToken(sp<IBinder>* /*outToken*/) override {

        return NO_ERROR;

    }



    status_t setFrameTimelineInfo(const sp<IGraphicBufferProducer>& /*surface*/,

    status_t setFrameTimelineInfo(const sp<IGraphicBufferProducer>& /*surface*/,

                                  const FrameTimelineInfo& /*frameTimelineInfo*/) override {

                                  const FrameTimelineInfo& /*frameTimelineInfo*/) override {

        return NO_ERROR;

        return NO_ERROR;

services/surfaceflinger/SurfaceFlinger.cpp

+34 −90
Original line number Original line Diff line number Diff line
@@ -263,14 +263,6 @@ bool validateCompositionDataspace(Dataspace dataspace) { 
    return dataspace == Dataspace::V0_SRGB || dataspace == Dataspace::DISPLAY_P3;

    return dataspace == Dataspace::V0_SRGB || dataspace == Dataspace::DISPLAY_P3;

}

}





class FrameRateFlexibilityToken : public BBinder {

public:

    FrameRateFlexibilityToken(std::function<void()> callback) : mCallback(callback) {}

    virtual ~FrameRateFlexibilityToken() { mCallback(); }



private:

    std::function<void()> mCallback;

};





enum Permission {

enum Permission {

    ACCESS_SURFACE_FLINGER = 0x1,

    ACCESS_SURFACE_FLINGER = 0x1,
@@ -5154,11 +5146,9 @@ status_t SurfaceFlinger::CheckTransactCodeCredentials(uint32_t code) { 
        case SET_GLOBAL_SHADOW_SETTINGS:

        case SET_GLOBAL_SHADOW_SETTINGS:

        case GET_PRIMARY_PHYSICAL_DISPLAY_ID:

        case GET_PRIMARY_PHYSICAL_DISPLAY_ID:

        case ACQUIRE_FRAME_RATE_FLEXIBILITY_TOKEN: {

        case ACQUIRE_FRAME_RATE_FLEXIBILITY_TOKEN: {

            // ACQUIRE_FRAME_RATE_FLEXIBILITY_TOKEN and OVERRIDE_HDR_TYPES are used by CTS tests,

            // OVERRIDE_HDR_TYPES is used by CTS tests, which acquire the necessary

            // which acquire the necessary permission dynamically. Don't use the permission cache

            // permission dynamically. Don't use the permission cache for this check.

            // for this check.

            bool usePermissionCache = code != OVERRIDE_HDR_TYPES;

            bool usePermissionCache =

                    code != ACQUIRE_FRAME_RATE_FLEXIBILITY_TOKEN && code != OVERRIDE_HDR_TYPES;

            if (!callingThreadHasUnscopedSurfaceFlingerAccess(usePermissionCache)) {

            if (!callingThreadHasUnscopedSurfaceFlingerAccess(usePermissionCache)) {

                IPCThreadState* ipc = IPCThreadState::self();

                IPCThreadState* ipc = IPCThreadState::self();

                ALOGE("Permission Denial: can't access SurfaceFlinger pid=%d, uid=%d",

                ALOGE("Permission Denial: can't access SurfaceFlinger pid=%d, uid=%d",
@@ -5610,17 +5600,39 @@ status_t SurfaceFlinger::onTransact(uint32_t code, const Parcel& data, Parcel* r 
                mDebugDisplayModeSetByBackdoor = result == NO_ERROR;

                mDebugDisplayModeSetByBackdoor = result == NO_ERROR;

                return result;

                return result;

            }

            }

            // Turn on/off frame rate flexibility mode. When turned on it overrides the display

            // manager frame rate policy a new policy which allows switching between all refresh

            // rates.

            case 1036: {

            case 1036: {

                if (data.readInt32() > 0) {

                if (data.readInt32() > 0) { // turn on

                    status_t result =

                    return schedule([this] {

                            acquireFrameRateFlexibilityToken(&mDebugFrameRateFlexibilityToken);

                               const auto display = ON_MAIN_THREAD(getDefaultDisplayDeviceLocked());

                    if (result != NO_ERROR) {



                        return result;

                               // This is a little racy, but not in a way that hurts anything. As we

                    }

                               // grab the defaultMode from the display manager policy, we could be

                } else {

                               // setting a new display manager policy, leaving us using a stale

                    mDebugFrameRateFlexibilityToken = nullptr;

                               // defaultMode. The defaultMode doesn't matter for the override

                               // policy though, since we set allowGroupSwitching to true, so it's

                               // not a problem.

                               scheduler::RefreshRateConfigs::Policy overridePolicy;

                               overridePolicy.defaultMode = display->refreshRateConfigs()

                                                                    .getDisplayManagerPolicy()

                                                                    .defaultMode;

                               overridePolicy.allowGroupSwitching = true;

                               constexpr bool kOverridePolicy = true;

                               return setDesiredDisplayModeSpecsInternal(display, overridePolicy,

                                                                         kOverridePolicy);

                           })

                            .get();

                } else { // turn off

                    return schedule([this] {

                               const auto display = ON_MAIN_THREAD(getDefaultDisplayDeviceLocked());

                               constexpr bool kOverridePolicy = true;

                               return setDesiredDisplayModeSpecsInternal(display, {},

                                                                         kOverridePolicy);

                           })

                            .get();

                }

                }

                return NO_ERROR;

            }

            }

            // Inject a hotplug connected event for the primary display. This will deallocate and

            // Inject a hotplug connected event for the primary display. This will deallocate and

            // reallocate the display state including framebuffers.

            // reallocate the display state including framebuffers.
@@ -6616,74 +6628,6 @@ status_t SurfaceFlinger::setFrameRate(const sp<IGraphicBufferProducer>& surface, 
    return NO_ERROR;

    return NO_ERROR;

}

}





status_t SurfaceFlinger::acquireFrameRateFlexibilityToken(sp<IBinder>* outToken) {

    if (!outToken) {

        return BAD_VALUE;

    }



    auto future = schedule([this] {

        status_t result = NO_ERROR;

        sp<IBinder> token;



        if (mFrameRateFlexibilityTokenCount == 0) {

            const auto display = ON_MAIN_THREAD(getDefaultDisplayDeviceLocked());



            // This is a little racy, but not in a way that hurts anything. As we grab the

            // defaultMode from the display manager policy, we could be setting a new display

            // manager policy, leaving us using a stale defaultMode. The defaultMode doesn't

            // matter for the override policy though, since we set allowGroupSwitching to

            // true, so it's not a problem.

            scheduler::RefreshRateConfigs::Policy overridePolicy;

            overridePolicy.defaultMode =

                    display->refreshRateConfigs().getDisplayManagerPolicy().defaultMode;

            overridePolicy.allowGroupSwitching = true;

            constexpr bool kOverridePolicy = true;

            result = setDesiredDisplayModeSpecsInternal(display, overridePolicy, kOverridePolicy);

        }



        if (result == NO_ERROR) {

            mFrameRateFlexibilityTokenCount++;

            // Handing out a reference to the SurfaceFlinger object, as we're doing in the line

            // below, is something to consider carefully. The lifetime of the

            // FrameRateFlexibilityToken isn't tied to SurfaceFlinger object lifetime, so if this

            // SurfaceFlinger object were to be destroyed while the token still exists, the token

            // destructor would be accessing a stale SurfaceFlinger reference, and crash. This is ok

            // in this case, for two reasons:

            //   1. Once SurfaceFlinger::run() is called by main_surfaceflinger.cpp, the only way

            //   the program exits is via a crash. So we won't have a situation where the

            //   SurfaceFlinger object is dead but the process is still up.

            //   2. The frame rate flexibility token is acquired/released only by CTS tests, so even

            //   if condition 1 were changed, the problem would only show up when running CTS tests,

            //   not on end user devices, so we could spot it and fix it without serious impact.

            token = new FrameRateFlexibilityToken(

                    [this]() { onFrameRateFlexibilityTokenReleased(); });

            ALOGD("Frame rate flexibility token acquired. count=%d",

                  mFrameRateFlexibilityTokenCount);

        }



        return std::make_pair(result, token);

    });



    status_t result;

    std::tie(result, *outToken) = future.get();

    return result;

}



void SurfaceFlinger::onFrameRateFlexibilityTokenReleased() {

    static_cast<void>(schedule([this] {

        LOG_ALWAYS_FATAL_IF(mFrameRateFlexibilityTokenCount == 0,

                            "Failed tracking frame rate flexibility tokens");

        mFrameRateFlexibilityTokenCount--;

        ALOGD("Frame rate flexibility token released. count=%d", mFrameRateFlexibilityTokenCount);

        if (mFrameRateFlexibilityTokenCount == 0) {

            const auto display = ON_MAIN_THREAD(getDefaultDisplayDeviceLocked());

            constexpr bool kOverridePolicy = true;

            status_t result = setDesiredDisplayModeSpecsInternal(display, {}, kOverridePolicy);

            LOG_ALWAYS_FATAL_IF(result < 0, "Failed releasing frame rate flexibility token");

        }

    }));

}



status_t SurfaceFlinger::setFrameTimelineInfo(const sp<IGraphicBufferProducer>& surface,

status_t SurfaceFlinger::setFrameTimelineInfo(const sp<IGraphicBufferProducer>& surface,

                                              const FrameTimelineInfo& frameTimelineInfo) {

                                              const FrameTimelineInfo& frameTimelineInfo) {

    Mutex::Autolock lock(mStateLock);

    Mutex::Autolock lock(mStateLock);

services/surfaceflinger/SurfaceFlinger.h

+0 −7
Original line number Original line Diff line number Diff line
@@ -601,7 +601,6 @@ private: 
                                     float lightPosY, float lightPosZ, float lightRadius) override;

                                     float lightPosY, float lightPosZ, float lightRadius) override;

    status_t setFrameRate(const sp<IGraphicBufferProducer>& surface, float frameRate,

    status_t setFrameRate(const sp<IGraphicBufferProducer>& surface, float frameRate,

                          int8_t compatibility, int8_t changeFrameRateStrategy) override;

                          int8_t compatibility, int8_t changeFrameRateStrategy) override;

    status_t acquireFrameRateFlexibilityToken(sp<IBinder>* outToken) override;





    status_t setFrameTimelineInfo(const sp<IGraphicBufferProducer>& surface,

    status_t setFrameTimelineInfo(const sp<IGraphicBufferProducer>& surface,

                                  const FrameTimelineInfo& frameTimelineInfo) override;

                                  const FrameTimelineInfo& frameTimelineInfo) override;
@@ -1067,8 +1066,6 @@ private: 
        return doDump(fd, args, asProto);

        return doDump(fd, args, asProto);

    }

    }





    void onFrameRateFlexibilityTokenReleased();



    static mat4 calculateColorMatrix(float saturation);

    static mat4 calculateColorMatrix(float saturation);





    void updateColorMatrixLocked();

    void updateColorMatrixLocked();
@@ -1335,10 +1332,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;





    int mFrameRateFlexibilityTokenCount = 0;



    sp<IBinder> mDebugFrameRateFlexibilityToken;



    BufferCountTracker mBufferCountTracker;

    BufferCountTracker mBufferCountTracker;





    std::unordered_map<DisplayId, sp<HdrLayerInfoReporter>> mHdrLayerInfoListeners

    std::unordered_map<DisplayId, sp<HdrLayerInfoReporter>> mHdrLayerInfoListeners