diff options
28 files changed, 345 insertions, 79 deletions
diff --git a/data/etc/car_core_hardware.xml b/data/etc/car_core_hardware.xml index 50f117dd11..6ffb94721d 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 40dd09b180..ddd9f9b212 100644 --- a/libs/binder/Parcel.cpp +++ b/libs/binder/Parcel.cpp @@ -2668,7 +2668,7 @@ status_t Parcel::restartWrite(size_t desired) 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 6d04f13211..233f12a58c 100644 --- a/libs/binder/include/binder/AppOpsManager.h +++ b/libs/binder/include/binder/AppOpsManager.h @@ -133,7 +133,11 @@ public: 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 2139acb715..af973673d8 100644 --- a/libs/renderengine/gl/GLESRenderEngine.cpp +++ b/libs/renderengine/gl/GLESRenderEngine.cpp @@ -409,6 +409,23 @@ GLESRenderEngine::GLESRenderEngine(const RenderEngineCreationArgs& args, EGLDisp 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 @@ GLESRenderEngine::~GLESRenderEngine() { 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::genTextures(size_t count, uint32_t* names) { } 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 @@ status_t GLESRenderEngine::bindExternalTextureBuffer(uint32_t texName, } bindExternalTextureImage(texName, *cachedImage->second); + mTextureView.insert_or_assign(texName, buffer->getId()); } // Wait for the new buffer to be ready. @@ -887,7 +909,7 @@ void GLESRenderEngine::unbindFrameBuffer(Framebuffer* /*framebuffer*/) { glBindFramebuffer(GL_FRAMEBUFFER, 0); } -bool GLESRenderEngine::cleanupPostRender() { +bool GLESRenderEngine::cleanupPostRender(CleanupMode mode) { ATRACE_CALL(); if (mPriorResourcesCleaned || @@ -896,6 +918,30 @@ bool GLESRenderEngine::cleanupPostRender() { 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); @@ -1617,6 +1663,16 @@ bool GLESRenderEngine::isImageCachedForTesting(uint64_t bufferId) { 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 61986ff721..9484011525 100644 --- a/libs/renderengine/gl/GLESRenderEngine.h +++ b/libs/renderengine/gl/GLESRenderEngine.h @@ -75,7 +75,7 @@ public: 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 @@ public: // 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); @@ -224,6 +230,8 @@ private: // 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 @@ -237,6 +245,11 @@ private: // 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 e06e1287c1..74bc44b22f 100644 --- a/libs/renderengine/include/renderengine/RenderEngine.h +++ b/libs/renderengine/include/renderengine/RenderEngine.h @@ -111,14 +111,25 @@ public: // 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 df0f17a6d5..101cbb70fd 100644 --- a/libs/renderengine/include/renderengine/mock/RenderEngine.h +++ b/libs/renderengine/include/renderengine/mock/RenderEngine.h @@ -56,7 +56,7 @@ public: 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 0b5b1e4be8..0fec99b9b6 100644 --- a/libs/renderengine/tests/RenderEngineTest.cpp +++ b/libs/renderengine/tests/RenderEngineTest.cpp @@ -81,6 +81,7 @@ struct RenderEngineTest : public ::testing::Test { } for (uint32_t texName : mTexNames) { sRE->deleteTextures(1, &texName); + EXPECT_FALSE(sRE->isTextureNameKnownForTesting(texName)); } } @@ -1424,10 +1425,44 @@ TEST_F(RenderEngineTest, cleanupPostRender_cleansUpOnce) { 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/inputflinger/reader/InputDevice.cpp b/services/inputflinger/reader/InputDevice.cpp index 4b19e5e353..3347ba6ad7 100644 --- a/services/inputflinger/reader/InputDevice.cpp +++ b/services/inputflinger/reader/InputDevice.cpp @@ -84,12 +84,13 @@ void InputDevice::setEnabled(bool enabled, nsecs_t when) { bumpGeneration(); } -void InputDevice::dump(std::string& dump) { +void InputDevice::dump(std::string& dump, const std::string& eventHubDevStr) { InputDeviceInfo deviceInfo; getDeviceInfo(&deviceInfo); dump += StringPrintf(INDENT "Device %d: %s\n", deviceInfo.getId(), deviceInfo.getDisplayName().c_str()); + dump += StringPrintf(INDENT "%s", eventHubDevStr.c_str()); dump += StringPrintf(INDENT2 "Generation: %d\n", mGeneration); dump += StringPrintf(INDENT2 "IsExternal: %s\n", toString(mIsExternal)); dump += StringPrintf(INDENT2 "AssociatedDisplayPort: "); @@ -101,6 +102,7 @@ void InputDevice::dump(std::string& dump) { dump += StringPrintf(INDENT2 "HasMic: %s\n", toString(mHasMic)); dump += StringPrintf(INDENT2 "Sources: 0x%08x\n", deviceInfo.getSources()); dump += StringPrintf(INDENT2 "KeyboardType: %d\n", deviceInfo.getKeyboardType()); + dump += StringPrintf(INDENT2 "ControllerNum: %d\n", deviceInfo.getControllerNumber()); const std::vector<InputDeviceInfo::MotionRange>& ranges = deviceInfo.getMotionRanges(); if (!ranges.empty()) { @@ -200,6 +202,8 @@ void InputDevice::addEventHubDevice(int32_t eventHubId, bool populateMappers) { // insert the context into the devices set mDevices.insert({eventHubId, std::make_pair(std::move(contextPtr), std::move(mappers))}); + // Must change generation to flag this device as changed + bumpGeneration(); } void InputDevice::removeEventHubDevice(int32_t eventHubId) { diff --git a/services/inputflinger/reader/InputReader.cpp b/services/inputflinger/reader/InputReader.cpp index 06e374353c..dff830ca7f 100644 --- a/services/inputflinger/reader/InputReader.cpp +++ b/services/inputflinger/reader/InputReader.cpp @@ -206,6 +206,14 @@ void InputReader::addDeviceLocked(nsecs_t when, int32_t eventHubId) { } mDevices.emplace(eventHubId, device); + // Add device to device to EventHub ids map. + const auto mapIt = mDeviceToEventHubIdsMap.find(device); + if (mapIt == mDeviceToEventHubIdsMap.end()) { + std::vector<int32_t> ids = {eventHubId}; + mDeviceToEventHubIdsMap.emplace(device, ids); + } else { + mapIt->second.push_back(eventHubId); + } bumpGenerationLocked(); if (device->getClasses() & INPUT_DEVICE_CLASS_EXTERNAL_STYLUS) { @@ -222,6 +230,17 @@ void InputReader::removeDeviceLocked(nsecs_t when, int32_t eventHubId) { std::shared_ptr<InputDevice> device = std::move(deviceIt->second); mDevices.erase(deviceIt); + // Erase device from device to EventHub ids map. + auto mapIt = mDeviceToEventHubIdsMap.find(device); + if (mapIt != mDeviceToEventHubIdsMap.end()) { + std::vector<int32_t>& eventHubIds = mapIt->second; + eventHubIds.erase(std::remove_if(eventHubIds.begin(), eventHubIds.end(), + [eventHubId](int32_t eId) { return eId == eventHubId; }), + eventHubIds.end()); + if (eventHubIds.size() == 0) { + mDeviceToEventHubIdsMap.erase(mapIt); + } + } bumpGenerationLocked(); if (device->isIgnored()) { @@ -450,8 +469,7 @@ void InputReader::getInputDevices(std::vector<InputDeviceInfo>& outInputDevices) void InputReader::getInputDevicesLocked(std::vector<InputDeviceInfo>& outInputDevices) { outInputDevices.clear(); - for (auto& devicePair : mDevices) { - std::shared_ptr<InputDevice>& device = devicePair.second; + for (const auto& [device, eventHubIds] : mDeviceToEventHubIdsMap) { if (!device->isIgnored()) { InputDeviceInfo info; device->getDeviceInfo(&info); @@ -622,11 +640,17 @@ void InputReader::dump(std::string& dump) { mEventHub->dump(dump); dump += "\n"; - dump += "Input Reader State:\n"; + dump += StringPrintf("Input Reader State (Nums of device: %zu):\n", + mDeviceToEventHubIdsMap.size()); - for (const auto& devicePair : mDevices) { - const std::shared_ptr<InputDevice>& device = devicePair.second; - device->dump(dump); + for (const auto& devicePair : mDeviceToEventHubIdsMap) { + const std::shared_ptr<InputDevice>& device = devicePair.first; + std::string eventHubDevStr = INDENT "EventHub Devices: [ "; + for (const auto& eId : devicePair.second) { + eventHubDevStr += StringPrintf("%d ", eId); + } + eventHubDevStr += "] \n"; + device->dump(dump, eventHubDevStr); } dump += INDENT "Configuration:\n"; diff --git a/services/inputflinger/reader/include/InputDevice.h b/services/inputflinger/reader/include/InputDevice.h index 7c17102826..d2bb4f4b4c 100644 --- a/services/inputflinger/reader/include/InputDevice.h +++ b/services/inputflinger/reader/include/InputDevice.h @@ -66,7 +66,7 @@ public: bool isEnabled(); void setEnabled(bool enabled, nsecs_t when); - void dump(std::string& dump); + void dump(std::string& dump, const std::string& eventHubDevStr); void addEventHubDevice(int32_t eventHubId, bool populateMappers = true); void removeEventHubDevice(int32_t eventHubId); void configure(nsecs_t when, const InputReaderConfiguration* config, uint32_t changes); diff --git a/services/inputflinger/reader/include/InputReader.h b/services/inputflinger/reader/include/InputReader.h index 108b9c236c..2773f709eb 100644 --- a/services/inputflinger/reader/include/InputReader.h +++ b/services/inputflinger/reader/include/InputReader.h @@ -143,6 +143,11 @@ private: // to lookup the input device instance from the EventHub device id. std::unordered_map<int32_t /*eventHubId*/, std::shared_ptr<InputDevice>> mDevices; + // An input device contains one or more eventHubId, this map provides a way to lookup the + // EventHubIds contained in the input device from the input device instance. + std::unordered_map<std::shared_ptr<InputDevice>, std::vector<int32_t> /*eventHubId*/> + mDeviceToEventHubIdsMap; + // low-level input event decoding and device management void processEventsLocked(const RawEvent* rawEvents, size_t count); diff --git a/services/sensorservice/SensorEventConnection.cpp b/services/sensorservice/SensorEventConnection.cpp index 6c8671289d..3cccaf9329 100644 --- a/services/sensorservice/SensorEventConnection.cpp +++ b/services/sensorservice/SensorEventConnection.cpp @@ -29,6 +29,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, @@ -36,9 +42,9 @@ SensorService::SensorEventConnection::SensorEventConnection( : 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; @@ -439,6 +445,14 @@ bool SensorService::SensorEventConnection::noteOpIfRequired(const sensors_event_ 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 60f9cd90c8..3ca34bba1b 100644 --- a/services/sensorservice/SensorService.cpp +++ b/services/sensorservice/SensorService.cpp @@ -79,6 +79,8 @@ uint8_t SensorService::sHmacGlobalKey[128] = {}; 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 @@ bool SensorService::hasPermissionForSensor(const Sensor& sensor) { } 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 3bb8421a14..052cbfe290 100644 --- a/services/sensorservice/SensorService.h +++ b/services/sensorservice/SensorService.h @@ -424,6 +424,7 @@ private: 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 80c3b65ae8..ae0a9843f6 100644 --- a/services/stats/StatsHal.cpp +++ b/services/stats/StatsHal.cpp @@ -58,7 +58,7 @@ hardware::Return<void> StatsHal::reportChargeCycles(const ChargeCycles& chargeCy 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 03903f6d07..e9965d4717 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -1445,6 +1445,13 @@ Layer::FrameRate Layer::getFrameRateForLayerTree() const { 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 @@ InputWindowInfo Layer::fillInputInfo() { 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 ab5773dc09..61f3fbbdf1 100644 --- a/services/surfaceflinger/Scheduler/VSyncPredictor.cpp +++ b/services/surfaceflinger/Scheduler/VSyncPredictor.cpp @@ -255,21 +255,9 @@ void VSyncPredictor::clearTimestamps() { } } -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 3ca878d77d..5f3c418fed 100644 --- a/services/surfaceflinger/Scheduler/VSyncPredictor.h +++ b/services/surfaceflinger/Scheduler/VSyncPredictor.h @@ -52,11 +52,10 @@ public: */ 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 c743de07ae..efa8bab8fe 100644 --- a/services/surfaceflinger/Scheduler/VSyncReactor.cpp +++ b/services/surfaceflinger/Scheduler/VSyncReactor.cpp @@ -233,6 +233,7 @@ nsecs_t VSyncReactor::expectedPresentTime(nsecs_t now) { } void VSyncReactor::startPeriodTransition(nsecs_t newPeriod) { + ATRACE_CALL(); mPeriodConfirmationInProgress = true; mPeriodTransitioningTo = newPeriod; mMoreSamplesNeeded = true; @@ -240,8 +241,7 @@ void VSyncReactor::startPeriodTransition(nsecs_t newPeriod) { } void VSyncReactor::endPeriodTransition() { - setIgnorePresentFencesInternal(false); - mMoreSamplesNeeded = false; + ATRACE_CALL(); mPeriodTransitioningTo.reset(); mPeriodConfirmationInProgress = false; mLastHwVsync.reset(); @@ -254,6 +254,8 @@ void VSyncReactor::setPeriod(nsecs_t period) { if (!mSupportKernelIdleTimer && period == getPeriod()) { endPeriodTransition(); + setIgnorePresentFencesInternal(false); + mMoreSamplesNeeded = false; } else { startPeriodTransition(period); } @@ -303,6 +305,7 @@ bool VSyncReactor::addResyncSample(nsecs_t timestamp, std::optional<nsecs_t> hwc 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 @@ bool VSyncReactor::addResyncSample(nsecs_t timestamp, std::optional<nsecs_t> hwc } *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 05a6fc3a8d..107c5400fe 100644 --- a/services/surfaceflinger/Scheduler/VSyncTracker.h +++ b/services/surfaceflinger/Scheduler/VSyncTracker.h @@ -66,6 +66,8 @@ public: /* 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 9d65f2f2af..ec81bada3d 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -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 @@ -4193,8 +4193,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. @@ -4229,7 +4228,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)); @@ -4238,7 +4237,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; @@ -4265,7 +4264,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 @@ -4378,7 +4377,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]); @@ -4443,7 +4442,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, @@ -6229,6 +6228,11 @@ status_t SurfaceFlinger::setFrameRate(const sp<IGraphicBufferProducer>& surface, 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 ccaeb2d858..c727574780 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -852,7 +852,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. diff --git a/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp b/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp index ce5f35cdf3..06bdcdc666 100644 --- a/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp +++ b/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp @@ -85,7 +85,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; diff --git a/services/surfaceflinger/tests/unittests/VSyncDispatchRealtimeTest.cpp b/services/surfaceflinger/tests/unittests/VSyncDispatchRealtimeTest.cpp index be49ef33f2..c2a77527a1 100644 --- a/services/surfaceflinger/tests/unittests/VSyncDispatchRealtimeTest.cpp +++ b/services/surfaceflinger/tests/unittests/VSyncDispatchRealtimeTest.cpp @@ -51,6 +51,7 @@ public: 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 @@ public: 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 fa5cc32d29..f630e3bb46 100644 --- a/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp +++ b/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp @@ -47,6 +47,7 @@ public: 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 fc39235a93..d4cd11dbe1 100644 --- a/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp +++ b/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp @@ -73,27 +73,27 @@ TEST_F(VSyncPredictorTest, reportsAnticipatedPeriod) { 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 @@ TEST_F(VSyncPredictorTest, willBeAccurateUsingPriorResultsForRate) { 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 @@ TEST_F(VSyncPredictorTest, slopeAlwaysValid) { // 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 a97256203d..6856612b78 100644 --- a/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp +++ b/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp @@ -41,6 +41,7 @@ public: 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 @@ public: 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 @@ TEST_F(VSyncReactorTest, addPresentFenceWhileAwaitingPeriodConfirmationRequestsH 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, beginResyncResetsModel) { 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 @@ TEST_F(VSyncReactorTest, periodIsMeasuredIfIgnoringComposer) { 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 |