Merge "Remove frame rate flexibility token" (3b37f10b) · Commits · e / os / android_frameworks_native

libs/gui/ISurfaceComposer.cpp

+0 −45

Original line number	Diff line number	Diff line
		@@ -1150,41 +1150,6 @@ public:
		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,
		const FrameTimelineInfo& frameTimelineInfo) override {
		Parcel data, reply;
		@@ -2073,16 +2038,6 @@ status_t BnSurfaceComposer::onTransact(
		reply->writeInt32(result);
		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: {
		CHECK_INTERFACE(ISurfaceComposer, data, reply);
		sp<IBinder> binder;

libs/gui/include/gui/ISurfaceComposer.h

+1 −8

Original line number	Diff line number	Diff line
		@@ -511,14 +511,6 @@ public:
		virtual status_t setFrameRate(const sp<IGraphicBufferProducer>& surface, float frameRate,
		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
		* buffer submitted on that surface.
		@@ -616,6 +608,7 @@ public:
		GET_GAME_CONTENT_TYPE_SUPPORT, // Deprecated. Use GET_DYNAMIC_DISPLAY_INFO instead.
		SET_GAME_CONTENT_TYPE,
		SET_FRAME_RATE,
		// Deprecated. Use DisplayManager.setShouldAlwaysRespectAppRequestedMode(true);
		ACQUIRE_FRAME_RATE_FLEXIBILITY_TOKEN,
		SET_FRAME_TIMELINE_INFO,
		ADD_TRANSACTION_TRACE_LISTENER,

libs/gui/tests/Surface_test.cpp

+0 −4

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

		status_t acquireFrameRateFlexibilityToken(sp<IBinder>* /outToken/) override {
		return NO_ERROR;
		}

		status_t setFrameTimelineInfo(const sp<IGraphicBufferProducer>& /surface/,
		const FrameTimelineInfo& /frameTimelineInfo/) override {
		return NO_ERROR;

services/surfaceflinger/SurfaceFlinger.cpp

+34 −90

Original line number	Diff line number	Diff line
		@@ -263,14 +263,6 @@ bool validateCompositionDataspace(Dataspace dataspace) {
		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 {
		ACCESS_SURFACE_FLINGER = 0x1,
		@@ -5165,11 +5157,9 @@ status_t SurfaceFlinger::CheckTransactCodeCredentials(uint32_t code) {
		case SET_GLOBAL_SHADOW_SETTINGS:
		case GET_PRIMARY_PHYSICAL_DISPLAY_ID:
		case ACQUIRE_FRAME_RATE_FLEXIBILITY_TOKEN: {
		// ACQUIRE_FRAME_RATE_FLEXIBILITY_TOKEN and OVERRIDE_HDR_TYPES are used by CTS tests,
		// which acquire the necessary permission dynamically. Don't use the permission cache
		// for this check.
		bool usePermissionCache =
		code != ACQUIRE_FRAME_RATE_FLEXIBILITY_TOKEN && code != OVERRIDE_HDR_TYPES;
		// OVERRIDE_HDR_TYPES is used by CTS tests, which acquire the necessary
		// permission dynamically. Don't use the permission cache for this check.
		bool usePermissionCache = code != OVERRIDE_HDR_TYPES;
		if (!callingThreadHasUnscopedSurfaceFlingerAccess(usePermissionCache)) {
		IPCThreadState* ipc = IPCThreadState::self();
		ALOGE("Permission Denial: can't access SurfaceFlinger pid=%d, uid=%d",
		@@ -5621,17 +5611,39 @@ status_t SurfaceFlinger::onTransact(uint32_t code, const Parcel& data, Parcel* r
		mDebugDisplayModeSetByBackdoor = result == NO_ERROR;
		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: {
		if (data.readInt32() > 0) {
		status_t result =
		acquireFrameRateFlexibilityToken(&mDebugFrameRateFlexibilityToken);
		if (result != NO_ERROR) {
		return result;
		}
		} else {
		mDebugFrameRateFlexibilityToken = nullptr;
		if (data.readInt32() > 0) { // turn on
		return schedule([this] {
		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;
		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
		// reallocate the display state including framebuffers.
		@@ -6620,74 +6632,6 @@ status_t SurfaceFlinger::setFrameRate(const sp<IGraphicBufferProducer>& surface,
		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,
		const FrameTimelineInfo& frameTimelineInfo) {
		Mutex::Autolock lock(mStateLock);

services/surfaceflinger/SurfaceFlinger.h

+0 −7

Original line number	Diff line number	Diff line
		@@ -602,7 +602,6 @@ private:
		float lightPosY, float lightPosZ, float lightRadius) override;
		status_t setFrameRate(const sp<IGraphicBufferProducer>& surface, float frameRate,
		int8_t compatibility, int8_t changeFrameRateStrategy) override;
		status_t acquireFrameRateFlexibilityToken(sp<IBinder>* outToken) override;

		status_t setFrameTimelineInfo(const sp<IGraphicBufferProducer>& surface,
		const FrameTimelineInfo& frameTimelineInfo) override;
		@@ -1068,8 +1067,6 @@ private:
		return doDump(fd, args, asProto);
		}

		void onFrameRateFlexibilityTokenReleased();

		static mat4 calculateColorMatrix(float saturation);

		void updateColorMatrixLocked();
		@@ -1336,10 +1333,6 @@ private:
		// be any issues with a raw pointer referencing an invalid object.
		std::unordered_set<Layer*> mOffscreenLayers;

		int mFrameRateFlexibilityTokenCount = 0;

		sp<IBinder> mDebugFrameRateFlexibilityToken;

		BufferCountTracker mBufferCountTracker;

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