Merge changes from topic "aidl-ndk-comparison" am: 370181de2d
Original change: https://android-review.googlesource.com/c/platform/frameworks/native/+/1519809
MUST ONLY BE SUBMITTED BY AUTOMERGER
Change-Id: I768a7f6ad34340864cab5296d946536401fb2f1e
diff --git a/data/etc/car_core_hardware.xml b/data/etc/car_core_hardware.xml
index 50f117d..6ffb947 100644
--- a/data/etc/car_core_hardware.xml
+++ b/data/etc/car_core_hardware.xml
@@ -38,7 +38,6 @@
<!-- basic system services -->
<feature name="android.software.connectionservice" />
<feature name="android.software.voice_recognizers" notLowRam="true" />
- <feature name="android.software.backup" />
<feature name="android.software.home_screen" />
<feature name="android.software.companion_device_setup" />
<feature name="android.software.autofill" />
diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp
index 3330a2e..6f26539 100644
--- a/libs/binder/Parcel.cpp
+++ b/libs/binder/Parcel.cpp
@@ -2685,7 +2685,7 @@
releaseObjects();
- if (data) {
+ if (data || desired == 0) {
LOG_ALLOC("Parcel %p: restart from %zu to %zu capacity", this, mDataCapacity, desired);
if (mDataCapacity > desired) {
gParcelGlobalAllocSize -= (mDataCapacity - desired);
diff --git a/libs/binder/include/binder/AppOpsManager.h b/libs/binder/include/binder/AppOpsManager.h
index 18a8a98..35c697e 100644
--- a/libs/binder/include/binder/AppOpsManager.h
+++ b/libs/binder/include/binder/AppOpsManager.h
@@ -132,7 +132,11 @@
OP_DEPRECATED_1 = 96,
OP_AUTO_REVOKE_PERMISSIONS_IF_UNUSED = 97,
OP_AUTO_REVOKE_MANAGED_BY_INSTALLER = 98,
- _NUM_OP = 99
+ OP_NO_ISOLATED_STORAGE = 99,
+ OP_PHONE_CALL_MICROPHONE = 100,
+ OP_PHONE_CALL_CAMERA = 101,
+ OP_RECORD_AUDIO_HOTWORD = 102,
+ _NUM_OP = 103
};
AppOpsManager();
diff --git a/libs/renderengine/gl/GLESRenderEngine.cpp b/libs/renderengine/gl/GLESRenderEngine.cpp
index 02021c9..56d470e 100644
--- a/libs/renderengine/gl/GLESRenderEngine.cpp
+++ b/libs/renderengine/gl/GLESRenderEngine.cpp
@@ -409,6 +409,23 @@
mImageManager = std::make_unique<ImageManager>(this);
mImageManager->initThread();
mDrawingBuffer = createFramebuffer();
+ sp<GraphicBuffer> buf =
+ new GraphicBuffer(1, 1, PIXEL_FORMAT_RGBA_8888, 1,
+ GRALLOC_USAGE_HW_RENDER | GRALLOC_USAGE_HW_TEXTURE, "placeholder");
+
+ const status_t err = buf->initCheck();
+ if (err != OK) {
+ ALOGE("Error allocating placeholder buffer: %d", err);
+ return;
+ }
+ mPlaceholderBuffer = buf.get();
+ EGLint attributes[] = {
+ EGL_NONE,
+ };
+ mPlaceholderImage = eglCreateImageKHR(mEGLDisplay, EGL_NO_CONTEXT, EGL_NATIVE_BUFFER_ANDROID,
+ mPlaceholderBuffer, attributes);
+ ALOGE_IF(mPlaceholderImage == EGL_NO_IMAGE_KHR, "Failed to create placeholder image: %#x",
+ eglGetError());
}
GLESRenderEngine::~GLESRenderEngine() {
@@ -423,6 +440,7 @@
eglDestroyImageKHR(mEGLDisplay, expired);
DEBUG_EGL_IMAGE_TRACKER_DESTROY();
}
+ eglDestroyImageKHR(mEGLDisplay, mPlaceholderImage);
mImageCache.clear();
eglMakeCurrent(mEGLDisplay, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT);
eglTerminate(mEGLDisplay);
@@ -589,6 +607,9 @@
}
void GLESRenderEngine::deleteTextures(size_t count, uint32_t const* names) {
+ for (int i = 0; i < count; ++i) {
+ mTextureView.erase(names[i]);
+ }
glDeleteTextures(count, names);
}
@@ -646,6 +667,7 @@
}
bindExternalTextureImage(texName, *cachedImage->second);
+ mTextureView.insert_or_assign(texName, buffer->getId());
}
// Wait for the new buffer to be ready.
@@ -887,7 +909,7 @@
glBindFramebuffer(GL_FRAMEBUFFER, 0);
}
-bool GLESRenderEngine::cleanupPostRender() {
+bool GLESRenderEngine::cleanupPostRender(CleanupMode mode) {
ATRACE_CALL();
if (mPriorResourcesCleaned ||
@@ -896,6 +918,30 @@
return false;
}
+ // This is a bit of a band-aid fix for FrameCaptureProcessor, as we should
+ // not need to keep memory around if we don't need to do so.
+ if (mode == CleanupMode::CLEAN_ALL) {
+ // TODO: SurfaceFlinger memory utilization may benefit from resetting
+ // texture bindings as well. Assess if it does and there's no performance regression
+ // when rebinding the same image data to the same texture, and if so then its mode
+ // behavior can be tweaked.
+ if (mPlaceholderImage != EGL_NO_IMAGE_KHR) {
+ for (auto [textureName, bufferId] : mTextureView) {
+ if (bufferId && mPlaceholderImage != EGL_NO_IMAGE_KHR) {
+ glBindTexture(GL_TEXTURE_EXTERNAL_OES, textureName);
+ glEGLImageTargetTexture2DOES(GL_TEXTURE_EXTERNAL_OES,
+ static_cast<GLeglImageOES>(mPlaceholderImage));
+ mTextureView[textureName] = std::nullopt;
+ checkErrors();
+ }
+ }
+ }
+ {
+ std::lock_guard<std::mutex> lock(mRenderingMutex);
+ mImageCache.clear();
+ }
+ }
+
// Bind the texture to placeholder so that backing image data can be freed.
GLFramebuffer* glFramebuffer = static_cast<GLFramebuffer*>(getFramebufferForDrawing());
glFramebuffer->allocateBuffers(1, 1, mPlaceholderDrawBuffer);
@@ -1622,6 +1668,16 @@
return cachedImage != mImageCache.end();
}
+bool GLESRenderEngine::isTextureNameKnownForTesting(uint32_t texName) {
+ const auto& entry = mTextureView.find(texName);
+ return entry != mTextureView.end();
+}
+
+std::optional<uint64_t> GLESRenderEngine::getBufferIdForTextureNameForTesting(uint32_t texName) {
+ const auto& entry = mTextureView.find(texName);
+ return entry != mTextureView.end() ? entry->second : std::nullopt;
+}
+
bool GLESRenderEngine::isFramebufferImageCachedForTesting(uint64_t bufferId) {
std::lock_guard<std::mutex> lock(mFramebufferImageCacheMutex);
return std::any_of(mFramebufferImageCache.cbegin(), mFramebufferImageCache.cend(),
diff --git a/libs/renderengine/gl/GLESRenderEngine.h b/libs/renderengine/gl/GLESRenderEngine.h
index 0e003a0..d554041 100644
--- a/libs/renderengine/gl/GLESRenderEngine.h
+++ b/libs/renderengine/gl/GLESRenderEngine.h
@@ -75,7 +75,7 @@
const std::vector<const LayerSettings*>& layers,
ANativeWindowBuffer* buffer, const bool useFramebufferCache,
base::unique_fd&& bufferFence, base::unique_fd* drawFence) override;
- bool cleanupPostRender() override;
+ bool cleanupPostRender(CleanupMode mode) override;
EGLDisplay getEGLDisplay() const { return mEGLDisplay; }
// Creates an output image for rendering to
@@ -86,6 +86,12 @@
// Test-only methods
// Returns true iff mImageCache contains an image keyed by bufferId
bool isImageCachedForTesting(uint64_t bufferId) EXCLUDES(mRenderingMutex);
+ // Returns true iff texName was previously generated by RenderEngine and was
+ // not destroyed.
+ bool isTextureNameKnownForTesting(uint32_t texName);
+ // Returns the buffer ID of the content bound to texName, or nullopt if no
+ // such mapping exists.
+ std::optional<uint64_t> getBufferIdForTextureNameForTesting(uint32_t texName);
// Returns true iff mFramebufferImageCache contains an image keyed by bufferId
bool isFramebufferImageCachedForTesting(uint64_t bufferId)
EXCLUDES(mFramebufferImageCacheMutex);
@@ -225,6 +231,8 @@
// Cache of GL images that we'll store per GraphicBuffer ID
std::unordered_map<uint64_t, std::unique_ptr<Image>> mImageCache GUARDED_BY(mRenderingMutex);
+ std::unordered_map<uint32_t, std::optional<uint64_t>> mTextureView;
+
// Mutex guarding rendering operations, so that:
// 1. GL operations aren't interleaved, and
// 2. Internal state related to rendering that is potentially modified by
@@ -238,6 +246,11 @@
// ensure that we align on a word. Allocating 16 bytes will provide a
// guarantee that we don't clobber memory.
uint32_t mPlaceholderDrawBuffer[4];
+ // Placeholder buffer and image, similar to mPlaceholderDrawBuffer, but
+ // instead these are intended for cleaning up texture memory with the
+ // GL_TEXTURE_EXTERNAL_OES target.
+ ANativeWindowBuffer* mPlaceholderBuffer = nullptr;
+ EGLImage mPlaceholderImage = EGL_NO_IMAGE_KHR;
sp<Fence> mLastDrawFence;
// Store a separate boolean checking if prior resources were cleaned up, as
// devices that don't support native sync fences can't rely on a last draw
diff --git a/libs/renderengine/include/renderengine/RenderEngine.h b/libs/renderengine/include/renderengine/RenderEngine.h
index e06e128..74bc44b 100644
--- a/libs/renderengine/include/renderengine/RenderEngine.h
+++ b/libs/renderengine/include/renderengine/RenderEngine.h
@@ -111,14 +111,25 @@
// Returns NO_ERROR when binds successfully, NO_MEMORY when there's no memory for allocation.
virtual status_t bindFrameBuffer(Framebuffer* framebuffer) = 0;
virtual void unbindFrameBuffer(Framebuffer* framebuffer) = 0;
+
+ enum class CleanupMode {
+ CLEAN_OUTPUT_RESOURCES,
+ CLEAN_ALL,
+ };
// Clean-up method that should be called on the main thread after the
// drawFence returned by drawLayers fires. This method will free up
// resources used by the most recently drawn frame. If the frame is still
// being drawn, then this call is silently ignored.
//
+ // If mode is CLEAN_OUTPUT_RESOURCES, then only resources related to the
+ // output framebuffer are cleaned up, including the sibling texture.
+ //
+ // If mode is CLEAN_ALL, then we also cleanup resources related to any input
+ // buffers.
+ //
// Returns true if resources were cleaned up, and false if we didn't need to
// do any work.
- virtual bool cleanupPostRender() = 0;
+ virtual bool cleanupPostRender(CleanupMode mode = CleanupMode::CLEAN_OUTPUT_RESOURCES) = 0;
// queries
virtual size_t getMaxTextureSize() const = 0;
diff --git a/libs/renderengine/include/renderengine/mock/RenderEngine.h b/libs/renderengine/include/renderengine/mock/RenderEngine.h
index df0f17a..101cbb7 100644
--- a/libs/renderengine/include/renderengine/mock/RenderEngine.h
+++ b/libs/renderengine/include/renderengine/mock/RenderEngine.h
@@ -56,7 +56,7 @@
MOCK_CONST_METHOD0(isProtected, bool());
MOCK_CONST_METHOD0(supportsProtectedContent, bool());
MOCK_METHOD1(useProtectedContext, bool(bool));
- MOCK_METHOD0(cleanupPostRender, bool());
+ MOCK_METHOD1(cleanupPostRender, bool(CleanupMode mode));
MOCK_METHOD6(drawLayers,
status_t(const DisplaySettings&, const std::vector<const LayerSettings*>&,
ANativeWindowBuffer*, const bool, base::unique_fd&&, base::unique_fd*));
diff --git a/libs/renderengine/tests/RenderEngineTest.cpp b/libs/renderengine/tests/RenderEngineTest.cpp
index a720a27..3b0d4f7 100644
--- a/libs/renderengine/tests/RenderEngineTest.cpp
+++ b/libs/renderengine/tests/RenderEngineTest.cpp
@@ -86,6 +86,7 @@
}
for (uint32_t texName : mTexNames) {
sRE->deleteTextures(1, &texName);
+ EXPECT_FALSE(sRE->isTextureNameKnownForTesting(texName));
}
for (uint32_t texName : mTexNamesCM) {
sRECM->deleteTextures(1, &texName);
@@ -1472,10 +1473,44 @@
if (fd >= 0) {
sync_wait(fd, -1);
}
-
// Only cleanup the first time.
- EXPECT_TRUE(sRE->cleanupPostRender());
- EXPECT_FALSE(sRE->cleanupPostRender());
+ EXPECT_TRUE(sRE->cleanupPostRender(
+ renderengine::RenderEngine::CleanupMode::CLEAN_OUTPUT_RESOURCES));
+ EXPECT_FALSE(sRE->cleanupPostRender(
+ renderengine::RenderEngine::CleanupMode::CLEAN_OUTPUT_RESOURCES));
+}
+
+TEST_F(RenderEngineTest, cleanupPostRender_whenCleaningAll_replacesTextureMemory) {
+ renderengine::DisplaySettings settings;
+ settings.physicalDisplay = fullscreenRect();
+ settings.clip = fullscreenRect();
+
+ std::vector<const renderengine::LayerSettings*> layers;
+ renderengine::LayerSettings layer;
+ layer.geometry.boundaries = fullscreenRect().toFloatRect();
+ BufferSourceVariant<ForceOpaqueBufferVariant>::fillColor(layer, 1.0f, 0.0f, 0.0f, this);
+ layer.alpha = 1.0;
+ layers.push_back(&layer);
+
+ base::unique_fd fence;
+ sRE->drawLayers(settings, layers, mBuffer->getNativeBuffer(), true, base::unique_fd(), &fence);
+
+ const int fd = fence.get();
+ if (fd >= 0) {
+ sync_wait(fd, -1);
+ }
+
+ uint64_t bufferId = layer.source.buffer.buffer->getId();
+ uint32_t texName = layer.source.buffer.textureName;
+ EXPECT_TRUE(sRE->isImageCachedForTesting(bufferId));
+ EXPECT_EQ(bufferId, sRE->getBufferIdForTextureNameForTesting(texName));
+
+ EXPECT_TRUE(sRE->cleanupPostRender(renderengine::RenderEngine::CleanupMode::CLEAN_ALL));
+
+ // Now check that our view of memory is good.
+ EXPECT_FALSE(sRE->isImageCachedForTesting(bufferId));
+ EXPECT_EQ(std::nullopt, sRE->getBufferIdForTextureNameForTesting(bufferId));
+ EXPECT_TRUE(sRE->isTextureNameKnownForTesting(texName));
}
} // namespace android
diff --git a/services/sensorservice/SensorEventConnection.cpp b/services/sensorservice/SensorEventConnection.cpp
index b4b5f98..d14a301 100644
--- a/services/sensorservice/SensorEventConnection.cpp
+++ b/services/sensorservice/SensorEventConnection.cpp
@@ -28,6 +28,12 @@
#define UNUSED(x) (void)(x)
namespace android {
+namespace {
+
+// Used as the default value for the target SDK until it's obtained via getTargetSdkVersion.
+constexpr int kTargetSdkUnknown = 0;
+
+} // namespace
SensorService::SensorEventConnection::SensorEventConnection(
const sp<SensorService>& service, uid_t uid, String8 packageName, bool isDataInjectionMode,
@@ -35,9 +41,9 @@
: mService(service), mUid(uid), mWakeLockRefCount(0), mHasLooperCallbacks(false),
mDead(false), mDataInjectionMode(isDataInjectionMode), mEventCache(nullptr),
mCacheSize(0), mMaxCacheSize(0), mTimeOfLastEventDrop(0), mEventsDropped(0),
- mPackageName(packageName), mOpPackageName(opPackageName), mDestroyed(false) {
+ mPackageName(packageName), mOpPackageName(opPackageName), mTargetSdk(kTargetSdkUnknown),
+ mDestroyed(false) {
mChannel = new BitTube(mService->mSocketBufferSize);
- mTargetSdk = SensorService::getTargetSdkVersion(opPackageName);
#if DEBUG_CONNECTIONS
mEventsReceived = mEventsSentFromCache = mEventsSent = 0;
mTotalAcksNeeded = mTotalAcksReceived = 0;
@@ -445,6 +451,14 @@
bool success = true;
const auto iter = mHandleToAppOp.find(event.sensor);
if (iter != mHandleToAppOp.end()) {
+ if (mTargetSdk == kTargetSdkUnknown) {
+ // getTargetSdkVersion returns -1 if it fails so this operation should only be run once
+ // per connection and then cached. Perform this here as opposed to in the constructor to
+ // avoid log spam for NDK/VNDK clients that don't use sensors guarded with permissions
+ // and pass in invalid op package names.
+ mTargetSdk = SensorService::getTargetSdkVersion(mOpPackageName);
+ }
+
// Special handling for step count/detect backwards compatibility: if the app's target SDK
// is pre-Q, still permit delivering events to the app even if permission isn't granted
// (since this permission was only introduced in Q)
diff --git a/services/sensorservice/SensorService.cpp b/services/sensorservice/SensorService.cpp
index 60f9cd9..3ca34bb 100644
--- a/services/sensorservice/SensorService.cpp
+++ b/services/sensorservice/SensorService.cpp
@@ -79,6 +79,8 @@
bool SensorService::sHmacGlobalKeyIsValid = false;
std::map<String16, int> SensorService::sPackageTargetVersion;
Mutex SensorService::sPackageTargetVersionLock;
+String16 SensorService::sSensorInterfaceDescriptorPrefix =
+ String16("android.frameworks.sensorservice@");
AppOpsManager SensorService::sAppOpsManager;
#define SENSOR_SERVICE_DIR "/data/system/sensor_service"
@@ -1847,6 +1849,13 @@
}
int SensorService::getTargetSdkVersion(const String16& opPackageName) {
+ // Don't query the SDK version for the ISensorManager descriptor as it doesn't have one. This
+ // descriptor tends to be used for VNDK clients, but can technically be set by anyone so don't
+ // give it elevated privileges.
+ if (opPackageName.startsWith(sSensorInterfaceDescriptorPrefix)) {
+ return -1;
+ }
+
Mutex::Autolock packageLock(sPackageTargetVersionLock);
int targetSdkVersion = -1;
auto entry = sPackageTargetVersion.find(opPackageName);
diff --git a/services/sensorservice/SensorService.h b/services/sensorservice/SensorService.h
index 3bb8421..052cbfe 100644
--- a/services/sensorservice/SensorService.h
+++ b/services/sensorservice/SensorService.h
@@ -424,6 +424,7 @@
static AppOpsManager sAppOpsManager;
static std::map<String16, int> sPackageTargetVersion;
static Mutex sPackageTargetVersionLock;
+ static String16 sSensorInterfaceDescriptorPrefix;
};
} // namespace android
diff --git a/services/stats/StatsHal.cpp b/services/stats/StatsHal.cpp
index 80c3b65..ae0a984 100644
--- a/services/stats/StatsHal.cpp
+++ b/services/stats/StatsHal.cpp
@@ -58,7 +58,7 @@
std::vector<int32_t> buckets = chargeCycles.cycleBucket;
int initialSize = buckets.size();
for (int i = 0; i < 10 - initialSize; i++) {
- buckets.push_back(-1); // Push -1 for buckets that do not exist.
+ buckets.push_back(0); // Push 0 for buckets that do not exist.
}
android::util::stats_write(android::util::CHARGE_CYCLES_REPORTED, buckets[0], buckets[1],
buckets[2], buckets[3], buckets[4], buckets[5], buckets[6], buckets[7], buckets[8],
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index 03903f6..e9965d4 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -1445,6 +1445,13 @@
void Layer::deferTransactionUntil_legacy(const sp<Layer>& barrierLayer, uint64_t frameNumber) {
ATRACE_CALL();
+ if (mLayerDetached) {
+ // If the layer is detached, then we don't defer this transaction since we will not
+ // commit the pending state while the layer is detached. Adding sync points may cause
+ // the barrier layer to wait for the states to be committed before dequeuing a buffer.
+ return;
+ }
+
mCurrentState.barrierLayer_legacy = barrierLayer;
mCurrentState.frameNumber_legacy = frameNumber;
// We don't set eTransactionNeeded, because just receiving a deferral
@@ -2403,7 +2410,15 @@
xSurfaceInset = (xSurfaceInset >= 0) ? std::min(xSurfaceInset, layerBounds.getWidth() / 2) : 0;
ySurfaceInset = (ySurfaceInset >= 0) ? std::min(ySurfaceInset, layerBounds.getHeight() / 2) : 0;
- layerBounds.inset(xSurfaceInset, ySurfaceInset, xSurfaceInset, ySurfaceInset);
+ // inset while protecting from overflow TODO(b/161235021): What is going wrong
+ // in the overflow scenario?
+ {
+ int32_t tmp;
+ if (!__builtin_add_overflow(layerBounds.left, xSurfaceInset, &tmp)) layerBounds.left = tmp;
+ if (!__builtin_sub_overflow(layerBounds.right, xSurfaceInset, &tmp)) layerBounds.right = tmp;
+ if (!__builtin_add_overflow(layerBounds.top, ySurfaceInset, &tmp)) layerBounds.top = tmp;
+ if (!__builtin_sub_overflow(layerBounds.bottom, ySurfaceInset, &tmp)) layerBounds.bottom = tmp;
+ }
// Input coordinate should match the layer bounds.
info.frameLeft = layerBounds.left;
diff --git a/services/surfaceflinger/Scheduler/VSyncPredictor.cpp b/services/surfaceflinger/Scheduler/VSyncPredictor.cpp
index ab5773d..61f3fbb 100644
--- a/services/surfaceflinger/Scheduler/VSyncPredictor.cpp
+++ b/services/surfaceflinger/Scheduler/VSyncPredictor.cpp
@@ -255,21 +255,9 @@
}
}
-bool VSyncPredictor::needsMoreSamples(nsecs_t now) const {
- using namespace std::literals::chrono_literals;
+bool VSyncPredictor::needsMoreSamples() const {
std::lock_guard<std::mutex> lk(mMutex);
- bool needsMoreSamples = true;
- if (mTimestamps.size() >= kMinimumSamplesForPrediction) {
- nsecs_t constexpr aLongTime =
- std::chrono::duration_cast<std::chrono::nanoseconds>(500ms).count();
- if (!(mLastTimestampIndex < 0 || mTimestamps.empty())) {
- auto const lastTimestamp = mTimestamps[mLastTimestampIndex];
- needsMoreSamples = !((lastTimestamp + aLongTime) > now);
- }
- }
-
- ATRACE_INT("VSP-moreSamples", needsMoreSamples);
- return needsMoreSamples;
+ return mTimestamps.size() < kMinimumSamplesForPrediction;
}
void VSyncPredictor::resetModel() {
diff --git a/services/surfaceflinger/Scheduler/VSyncPredictor.h b/services/surfaceflinger/Scheduler/VSyncPredictor.h
index 3ca878d..5f3c418 100644
--- a/services/surfaceflinger/Scheduler/VSyncPredictor.h
+++ b/services/surfaceflinger/Scheduler/VSyncPredictor.h
@@ -52,11 +52,10 @@
*/
void setPeriod(nsecs_t period) final;
- /* Query if the model is in need of more samples to make a prediction at timePoint.
- * \param [in] timePoint The timePoint to inquire of.
+ /* Query if the model is in need of more samples to make a prediction.
* \return True, if model would benefit from more samples, False if not.
*/
- bool needsMoreSamples(nsecs_t timePoint) const;
+ bool needsMoreSamples() const final;
std::tuple<nsecs_t /* slope */, nsecs_t /* intercept */> getVSyncPredictionModel() const;
diff --git a/services/surfaceflinger/Scheduler/VSyncReactor.cpp b/services/surfaceflinger/Scheduler/VSyncReactor.cpp
index c743de0..efa8bab 100644
--- a/services/surfaceflinger/Scheduler/VSyncReactor.cpp
+++ b/services/surfaceflinger/Scheduler/VSyncReactor.cpp
@@ -233,6 +233,7 @@
}
void VSyncReactor::startPeriodTransition(nsecs_t newPeriod) {
+ ATRACE_CALL();
mPeriodConfirmationInProgress = true;
mPeriodTransitioningTo = newPeriod;
mMoreSamplesNeeded = true;
@@ -240,8 +241,7 @@
}
void VSyncReactor::endPeriodTransition() {
- setIgnorePresentFencesInternal(false);
- mMoreSamplesNeeded = false;
+ ATRACE_CALL();
mPeriodTransitioningTo.reset();
mPeriodConfirmationInProgress = false;
mLastHwVsync.reset();
@@ -254,6 +254,8 @@
if (!mSupportKernelIdleTimer && period == getPeriod()) {
endPeriodTransition();
+ setIgnorePresentFencesInternal(false);
+ mMoreSamplesNeeded = false;
} else {
startPeriodTransition(period);
}
@@ -303,6 +305,7 @@
std::lock_guard<std::mutex> lk(mMutex);
if (periodConfirmed(timestamp, hwcVsyncPeriod)) {
+ ATRACE_NAME("VSR: period confirmed");
if (mPeriodTransitioningTo) {
mTracker->setPeriod(*mPeriodTransitioningTo);
for (auto& entry : mCallbacks) {
@@ -310,17 +313,29 @@
}
*periodFlushed = true;
}
+
+ if (mLastHwVsync) {
+ mTracker->addVsyncTimestamp(*mLastHwVsync);
+ }
+ mTracker->addVsyncTimestamp(timestamp);
+
endPeriodTransition();
+ mMoreSamplesNeeded = mTracker->needsMoreSamples();
} else if (mPeriodConfirmationInProgress) {
+ ATRACE_NAME("VSR: still confirming period");
mLastHwVsync = timestamp;
mMoreSamplesNeeded = true;
*periodFlushed = false;
} else {
- mMoreSamplesNeeded = false;
+ ATRACE_NAME("VSR: adding sample");
*periodFlushed = false;
+ mTracker->addVsyncTimestamp(timestamp);
+ mMoreSamplesNeeded = mTracker->needsMoreSamples();
}
- mTracker->addVsyncTimestamp(timestamp);
+ if (!mMoreSamplesNeeded) {
+ setIgnorePresentFencesInternal(false);
+ }
return mMoreSamplesNeeded;
}
diff --git a/services/surfaceflinger/Scheduler/VSyncTracker.h b/services/surfaceflinger/Scheduler/VSyncTracker.h
index 05a6fc3..107c540 100644
--- a/services/surfaceflinger/Scheduler/VSyncTracker.h
+++ b/services/surfaceflinger/Scheduler/VSyncTracker.h
@@ -66,6 +66,8 @@
/* Inform the tracker that the samples it has are not accurate for prediction. */
virtual void resetModel() = 0;
+ virtual bool needsMoreSamples() const = 0;
+
virtual void dump(std::string& result) const = 0;
protected:
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 6862e62..39f923f 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -1567,7 +1567,7 @@
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 @@
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
@@ -4193,8 +4193,7 @@
{});
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.
@@ -4229,7 +4228,7 @@
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));
@@ -4238,7 +4237,7 @@
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;
@@ -4265,7 +4264,7 @@
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
@@ -4378,7 +4377,7 @@
}
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]);
@@ -4443,7 +4442,7 @@
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,
@@ -6229,6 +6228,11 @@
Mutex::Autolock lock(mStateLock);
if (authenticateSurfaceTextureLocked(surface)) {
sp<Layer> layer = (static_cast<MonitoredProducer*>(surface.get()))->getLayer();
+ if (layer == nullptr) {
+ ALOGE("Attempt to set frame rate on a layer that no longer exists");
+ return BAD_VALUE;
+ }
+
if (layer->setFrameRate(
Layer::FrameRate(frameRate,
Layer::FrameRate::convertCompatibility(compatibility)))) {
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index ccaeb2d..c727574 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -852,7 +852,7 @@
/* ------------------------------------------------------------------------
* 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.
diff --git a/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp b/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp
index ce5f35c..06bdcdc 100644
--- a/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp
+++ b/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp
@@ -85,7 +85,7 @@
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;
diff --git a/services/surfaceflinger/tests/unittests/VSyncDispatchRealtimeTest.cpp b/services/surfaceflinger/tests/unittests/VSyncDispatchRealtimeTest.cpp
index be49ef3..c2a7752 100644
--- a/services/surfaceflinger/tests/unittests/VSyncDispatchRealtimeTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncDispatchRealtimeTest.cpp
@@ -51,6 +51,7 @@
void setPeriod(nsecs_t) final {}
void resetModel() final {}
+ bool needsMoreSamples() const final { return false; }
void dump(std::string&) const final {}
private:
@@ -86,6 +87,7 @@
void setPeriod(nsecs_t) final {}
void resetModel() final {}
+ bool needsMoreSamples() const final { return false; }
void dump(std::string&) const final {}
private:
diff --git a/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp b/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp
index fa5cc32..f630e3b 100644
--- a/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp
@@ -47,6 +47,7 @@
MOCK_CONST_METHOD0(currentPeriod, nsecs_t());
MOCK_METHOD1(setPeriod, void(nsecs_t));
MOCK_METHOD0(resetModel, void());
+ MOCK_CONST_METHOD0(needsMoreSamples, bool());
MOCK_CONST_METHOD1(dump, void(std::string&));
nsecs_t nextVSyncTime(nsecs_t timePoint) const {
diff --git a/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp b/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp
index fc39235..d4cd11d 100644
--- a/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp
@@ -73,27 +73,27 @@
TEST_F(VSyncPredictorTest, reportsSamplesNeededWhenHasNoDataPoints) {
for (auto i = 0u; i < kMinimumSamplesForPrediction; i++) {
- EXPECT_TRUE(tracker.needsMoreSamples(mNow += mPeriod));
- tracker.addVsyncTimestamp(mNow);
+ EXPECT_TRUE(tracker.needsMoreSamples());
+ tracker.addVsyncTimestamp(mNow += mPeriod);
}
- EXPECT_FALSE(tracker.needsMoreSamples(mNow));
+ EXPECT_FALSE(tracker.needsMoreSamples());
}
TEST_F(VSyncPredictorTest, reportsSamplesNeededAfterExplicitRateChange) {
for (auto i = 0u; i < kMinimumSamplesForPrediction; i++) {
tracker.addVsyncTimestamp(mNow += mPeriod);
}
- EXPECT_FALSE(tracker.needsMoreSamples(mNow));
+ EXPECT_FALSE(tracker.needsMoreSamples());
auto const changedPeriod = mPeriod * 2;
tracker.setPeriod(changedPeriod);
- EXPECT_TRUE(tracker.needsMoreSamples(mNow));
+ EXPECT_TRUE(tracker.needsMoreSamples());
for (auto i = 0u; i < kMinimumSamplesForPrediction; i++) {
- EXPECT_TRUE(tracker.needsMoreSamples(mNow += changedPeriod));
- tracker.addVsyncTimestamp(mNow);
+ EXPECT_TRUE(tracker.needsMoreSamples());
+ tracker.addVsyncTimestamp(mNow += changedPeriod);
}
- EXPECT_FALSE(tracker.needsMoreSamples(mNow));
+ EXPECT_FALSE(tracker.needsMoreSamples());
}
TEST_F(VSyncPredictorTest, transitionsToModelledPointsAfterSynthetic) {
@@ -320,20 +320,6 @@
EXPECT_THAT(intercept, Eq(0));
}
-TEST_F(VSyncPredictorTest, willBecomeInaccurateAfterA_longTimeWithNoSamples) {
- auto const simulatedVsyncs = generateVsyncTimestamps(kMinimumSamplesForPrediction, mPeriod, 0);
-
- for (auto const& timestamp : simulatedVsyncs) {
- tracker.addVsyncTimestamp(timestamp);
- }
- auto const mNow = *simulatedVsyncs.rbegin();
- EXPECT_FALSE(tracker.needsMoreSamples(mNow));
-
- // TODO: would be better to decay this as a result of the variance of the samples
- static auto constexpr aLongTimeOut = 1000000000;
- EXPECT_TRUE(tracker.needsMoreSamples(mNow + aLongTimeOut));
-}
-
TEST_F(VSyncPredictorTest, idealModelPredictionsBeforeRegressionModelIsBuilt) {
auto const simulatedVsyncs =
generateVsyncTimestamps(kMinimumSamplesForPrediction + 1, mPeriod, 0);
@@ -443,7 +429,7 @@
// When VsyncPredictor returns the period it means that it doesn't know how to predict and
// it needs to get more samples
if (slope == mPeriod && intercept == 0) {
- EXPECT_TRUE(tracker.needsMoreSamples(now));
+ EXPECT_TRUE(tracker.needsMoreSamples());
}
}
}
diff --git a/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp b/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp
index a972562..6856612 100644
--- a/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp
@@ -41,6 +41,7 @@
MOCK_CONST_METHOD0(currentPeriod, nsecs_t());
MOCK_METHOD1(setPeriod, void(nsecs_t));
MOCK_METHOD0(resetModel, void());
+ MOCK_CONST_METHOD0(needsMoreSamples, bool());
MOCK_CONST_METHOD1(dump, void(std::string&));
};
@@ -57,6 +58,7 @@
nsecs_t currentPeriod() const final { return mTracker->currentPeriod(); }
void setPeriod(nsecs_t period) final { mTracker->setPeriod(period); }
void resetModel() final { mTracker->resetModel(); }
+ bool needsMoreSamples() const final { return mTracker->needsMoreSamples(); }
void dump(std::string& result) const final { mTracker->dump(result); }
private:
@@ -455,6 +457,83 @@
EXPECT_FALSE(mReactor.addPresentFence(generateSignalledFenceWithTime(0)));
}
+TEST_F(VSyncReactorTest, hwVsyncIsRequestedForTracker) {
+ auto time = 0;
+ bool periodFlushed = false;
+ nsecs_t const newPeriod = 4000;
+ mReactor.setPeriod(newPeriod);
+
+ static auto constexpr numSamplesWithNewPeriod = 4;
+ Sequence seq;
+ EXPECT_CALL(*mMockTracker, needsMoreSamples())
+ .Times(numSamplesWithNewPeriod - 2)
+ .InSequence(seq)
+ .WillRepeatedly(Return(true));
+ EXPECT_CALL(*mMockTracker, needsMoreSamples())
+ .Times(1)
+ .InSequence(seq)
+ .WillRepeatedly(Return(false));
+ EXPECT_CALL(*mMockTracker, addVsyncTimestamp(_)).Times(numSamplesWithNewPeriod);
+
+ EXPECT_TRUE(mReactor.addResyncSample(time += period, std::nullopt, &periodFlushed));
+
+ EXPECT_TRUE(mReactor.addResyncSample(time += period, std::nullopt, &periodFlushed));
+ // confirmed period, but predictor wants numRequest samples. This one and prior are valid.
+ EXPECT_TRUE(mReactor.addResyncSample(time += newPeriod, std::nullopt, &periodFlushed));
+ EXPECT_TRUE(mReactor.addResyncSample(time += newPeriod, std::nullopt, &periodFlushed));
+ EXPECT_FALSE(mReactor.addResyncSample(time += newPeriod, std::nullopt, &periodFlushed));
+}
+
+TEST_F(VSyncReactorTest, hwVsyncturnsOffOnConfirmationWhenTrackerDoesntRequest) {
+ auto time = 0;
+ bool periodFlushed = false;
+ nsecs_t const newPeriod = 4000;
+ mReactor.setPeriod(newPeriod);
+
+ Sequence seq;
+ EXPECT_CALL(*mMockTracker, needsMoreSamples())
+ .Times(1)
+ .InSequence(seq)
+ .WillRepeatedly(Return(false));
+ EXPECT_CALL(*mMockTracker, addVsyncTimestamp(_)).Times(2);
+
+ EXPECT_TRUE(mReactor.addResyncSample(time += period, std::nullopt, &periodFlushed));
+ EXPECT_TRUE(mReactor.addResyncSample(time += period, std::nullopt, &periodFlushed));
+ EXPECT_FALSE(mReactor.addResyncSample(time += newPeriod, std::nullopt, &periodFlushed));
+}
+
+TEST_F(VSyncReactorTest, hwVsyncIsRequestedForTrackerMultiplePeriodChanges) {
+ auto time = 0;
+ bool periodFlushed = false;
+ nsecs_t const newPeriod1 = 4000;
+ nsecs_t const newPeriod2 = 7000;
+
+ mReactor.setPeriod(newPeriod1);
+
+ Sequence seq;
+ EXPECT_CALL(*mMockTracker, needsMoreSamples())
+ .Times(4)
+ .InSequence(seq)
+ .WillRepeatedly(Return(true));
+ EXPECT_CALL(*mMockTracker, needsMoreSamples())
+ .Times(1)
+ .InSequence(seq)
+ .WillRepeatedly(Return(false));
+ EXPECT_CALL(*mMockTracker, addVsyncTimestamp(_)).Times(7);
+
+ EXPECT_TRUE(mReactor.addResyncSample(time += period, std::nullopt, &periodFlushed));
+ EXPECT_TRUE(mReactor.addResyncSample(time += period, std::nullopt, &periodFlushed));
+ // confirmed period, but predictor wants numRequest samples. This one and prior are valid.
+ EXPECT_TRUE(mReactor.addResyncSample(time += newPeriod1, std::nullopt, &periodFlushed));
+ EXPECT_TRUE(mReactor.addResyncSample(time += newPeriod1, std::nullopt, &periodFlushed));
+
+ mReactor.setPeriod(newPeriod2);
+ EXPECT_TRUE(mReactor.addResyncSample(time += newPeriod1, std::nullopt, &periodFlushed));
+ EXPECT_TRUE(mReactor.addResyncSample(time += newPeriod2, std::nullopt, &periodFlushed));
+ EXPECT_TRUE(mReactor.addResyncSample(time += newPeriod2, std::nullopt, &periodFlushed));
+ EXPECT_FALSE(mReactor.addResyncSample(time += newPeriod2, std::nullopt, &periodFlushed));
+}
+
static nsecs_t computeWorkload(nsecs_t period, nsecs_t phase) {
return period - phase;
}
@@ -648,7 +727,7 @@
TEST_F(VSyncReactorTest, periodChangeWithGivenVsyncPeriod) {
bool periodFlushed = true;
- EXPECT_CALL(*mMockTracker, addVsyncTimestamp(_)).Times(3);
+ EXPECT_CALL(*mMockTracker, addVsyncTimestamp(_)).Times(2);
mReactor.setIgnorePresentFences(true);
nsecs_t const newPeriod = 5000;
@@ -672,7 +751,7 @@
kPendingLimit, true /* supportKernelIdleTimer */);
bool periodFlushed = true;
- EXPECT_CALL(*mMockTracker, addVsyncTimestamp(_)).Times(5);
+ EXPECT_CALL(*mMockTracker, addVsyncTimestamp(_)).Times(4);
idleReactor.setIgnorePresentFences(true);
// First, set the same period, which should only be confirmed when we receive two