diff options
-rw-r--r-- | cmds/atrace/atrace.cpp | 1 | ||||
-rw-r--r-- | cmds/atrace/atrace.rc | 2 | ||||
-rw-r--r-- | data/etc/car_core_hardware.xml | 1 | ||||
-rw-r--r-- | libs/dumputils/dump_utils.cpp | 1 | ||||
-rw-r--r-- | services/sensorservice/SensorService.cpp | 246 | ||||
-rw-r--r-- | services/sensorservice/SensorService.h | 76 | ||||
-rw-r--r-- | services/surfaceflinger/Layer.cpp | 2 | ||||
-rw-r--r-- | services/surfaceflinger/Scheduler/LayerInfo.h | 5 | ||||
-rw-r--r-- | services/surfaceflinger/Scheduler/Scheduler.cpp | 10 | ||||
-rw-r--r-- | services/surfaceflinger/Scheduler/SchedulerUtils.h | 6 | ||||
-rw-r--r-- | services/surfaceflinger/SurfaceFlinger.cpp | 23 | ||||
-rw-r--r-- | services/surfaceflinger/SurfaceFlinger.h | 2 |
12 files changed, 233 insertions, 142 deletions
diff --git a/cmds/atrace/atrace.cpp b/cmds/atrace/atrace.cpp index a4b00f8e0c..1429bc8b07 100644 --- a/cmds/atrace/atrace.cpp +++ b/cmds/atrace/atrace.cpp @@ -171,6 +171,7 @@ static const TracingCategory k_categories[] = { { OPT, "events/clk/clk_disable/enable" }, { OPT, "events/clk/clk_enable/enable" }, { OPT, "events/power/cpu_frequency_limits/enable" }, + { OPT, "events/power/suspend_resume/enable" }, } }, { "membus", "Memory Bus Utilization", 0, { { REQ, "events/memory_bus/enable" }, diff --git a/cmds/atrace/atrace.rc b/cmds/atrace/atrace.rc index f1426b6bd0..6e460a0d2e 100644 --- a/cmds/atrace/atrace.rc +++ b/cmds/atrace/atrace.rc @@ -49,6 +49,8 @@ on late-init chmod 0666 /sys/kernel/tracing/events/power/cpu_frequency_limits/enable chmod 0666 /sys/kernel/debug/tracing/events/power/gpu_frequency/enable chmod 0666 /sys/kernel/tracing/events/power/gpu_frequency/enable + chmod 0666 /sys/kernel/debug/tracing/events/power/suspend_resume/enable + chmod 0666 /sys/kernel/tracing/events/power/suspend_resume/enable chmod 0666 /sys/kernel/debug/tracing/events/cpufreq_interactive/enable chmod 0666 /sys/kernel/tracing/events/cpufreq_interactive/enable chmod 0666 /sys/kernel/debug/tracing/events/vmscan/mm_vmscan_direct_reclaim_begin/enable diff --git a/data/etc/car_core_hardware.xml b/data/etc/car_core_hardware.xml index ad7791e078..50f117dd11 100644 --- a/data/etc/car_core_hardware.xml +++ b/data/etc/car_core_hardware.xml @@ -40,7 +40,6 @@ <feature name="android.software.voice_recognizers" notLowRam="true" /> <feature name="android.software.backup" /> <feature name="android.software.home_screen" /> - <feature name="android.software.print" /> <feature name="android.software.companion_device_setup" /> <feature name="android.software.autofill" /> <feature name="android.software.cant_save_state" /> diff --git a/libs/dumputils/dump_utils.cpp b/libs/dumputils/dump_utils.cpp index 8e762f193f..250f902f9d 100644 --- a/libs/dumputils/dump_utils.cpp +++ b/libs/dumputils/dump_utils.cpp @@ -46,6 +46,7 @@ static const char* native_processes_to_dump[] = { static const char* hal_interfaces_to_dump[] { "android.hardware.audio@2.0::IDevicesFactory", "android.hardware.audio@4.0::IDevicesFactory", + "android.hardware.audio@5.0::IDevicesFactory", "android.hardware.biometrics.face@1.0::IBiometricsFace", "android.hardware.bluetooth@1.0::IBluetoothHci", "android.hardware.camera.provider@2.4::ICameraProvider", diff --git a/services/sensorservice/SensorService.cpp b/services/sensorservice/SensorService.cpp index c11b88eac2..14ed73deaf 100644 --- a/services/sensorservice/SensorService.cpp +++ b/services/sensorservice/SensorService.cpp @@ -299,14 +299,10 @@ void SensorService::onFirstRef() { } void SensorService::setSensorAccess(uid_t uid, bool hasAccess) { - SortedVector< sp<SensorEventConnection> > activeConnections; - populateActiveConnections(&activeConnections); - { - Mutex::Autolock _l(mLock); - for (size_t i = 0 ; i < activeConnections.size(); i++) { - if (activeConnections[i] != nullptr && activeConnections[i]->getUid() == uid) { - activeConnections[i]->setSensorAccess(hasAccess); - } + ConnectionSafeAutolock connLock = mConnectionHolder.lock(mLock); + for (const sp<SensorEventConnection>& conn : connLock.getActiveConnections()) { + if (conn->getUid() == uid) { + conn->setSensorAccess(hasAccess); } } } @@ -360,7 +356,7 @@ status_t SensorService::dump(int fd, const Vector<String16>& args) { if (args.size() > 2) { return INVALID_OPERATION; } - Mutex::Autolock _l(mLock); + ConnectionSafeAutolock connLock = mConnectionHolder.lock(mLock); SensorDevice& dev(SensorDevice::getInstance()); if (args.size() == 2 && args[0] == String16("restrict")) { // If already in restricted mode. Ignore. @@ -374,7 +370,7 @@ status_t SensorService::dump(int fd, const Vector<String16>& args) { mCurrentOperatingMode = RESTRICTED; // temporarily stop all sensor direct report and disable sensors - disableAllSensorsLocked(); + disableAllSensorsLocked(&connLock); mWhiteListedPackage.setTo(String8(args[1])); return status_t(NO_ERROR); } else if (args.size() == 1 && args[0] == String16("enable")) { @@ -382,7 +378,7 @@ status_t SensorService::dump(int fd, const Vector<String16>& args) { if (mCurrentOperatingMode == RESTRICTED) { mCurrentOperatingMode = NORMAL; // enable sensors and recover all sensor direct report - enableAllSensorsLocked(); + enableAllSensorsLocked(&connLock); } if (mCurrentOperatingMode == DATA_INJECTION) { resetToNormalModeLocked(); @@ -473,22 +469,18 @@ status_t SensorService::dump(int fd, const Vector<String16>& args) { result.appendFormat("Sensor Privacy: %s\n", mSensorPrivacyPolicy->isSensorPrivacyEnabled() ? "enabled" : "disabled"); - result.appendFormat("%zd active connections\n", mActiveConnections.size()); - for (size_t i=0 ; i < mActiveConnections.size() ; i++) { - sp<SensorEventConnection> connection(mActiveConnections[i].promote()); - if (connection != nullptr) { - result.appendFormat("Connection Number: %zu \n", i); - connection->dump(result); - } + const auto& activeConnections = connLock.getActiveConnections(); + result.appendFormat("%zd active connections\n", activeConnections.size()); + for (size_t i=0 ; i < activeConnections.size() ; i++) { + result.appendFormat("Connection Number: %zu \n", i); + activeConnections[i]->dump(result); } - result.appendFormat("%zd direct connections\n", mDirectConnections.size()); - for (size_t i = 0 ; i < mDirectConnections.size() ; i++) { - sp<SensorDirectConnection> connection(mDirectConnections[i].promote()); - if (connection != nullptr) { - result.appendFormat("Direct connection %zu:\n", i); - connection->dump(result); - } + const auto& directConnections = connLock.getDirectConnections(); + result.appendFormat("%zd direct connections\n", directConnections.size()); + for (size_t i = 0 ; i < directConnections.size() ; i++) { + result.appendFormat("Direct connection %zu:\n", i); + directConnections[i]->dump(result); } result.appendFormat("Previous Registrations:\n"); @@ -515,17 +507,14 @@ status_t SensorService::dump(int fd, const Vector<String16>& args) { } void SensorService::disableAllSensors() { - Mutex::Autolock _l(mLock); - disableAllSensorsLocked(); + ConnectionSafeAutolock connLock = mConnectionHolder.lock(mLock); + disableAllSensorsLocked(&connLock); } -void SensorService::disableAllSensorsLocked() { +void SensorService::disableAllSensorsLocked(ConnectionSafeAutolock* connLock) { SensorDevice& dev(SensorDevice::getInstance()); - for (auto &i : mDirectConnections) { - sp<SensorDirectConnection> connection(i.promote()); - if (connection != nullptr) { - connection->stopAll(true /* backupRecord */); - } + for (const sp<SensorDirectConnection>& connection : connLock->getDirectConnections()) { + connection->stopAll(true /* backupRecord */); } dev.disableAllSensors(); // Clear all pending flush connections for all active sensors. If one of the active @@ -537,11 +526,11 @@ void SensorService::disableAllSensorsLocked() { } void SensorService::enableAllSensors() { - Mutex::Autolock _l(mLock); - enableAllSensorsLocked(); + ConnectionSafeAutolock connLock = mConnectionHolder.lock(mLock); + enableAllSensorsLocked(&connLock); } -void SensorService::enableAllSensorsLocked() { +void SensorService::enableAllSensorsLocked(ConnectionSafeAutolock* connLock) { // sensors should only be enabled if the operating state is not restricted and sensor // privacy is not enabled. if (mCurrentOperatingMode == RESTRICTED || mSensorPrivacyPolicy->isSensorPrivacyEnabled()) { @@ -552,14 +541,12 @@ void SensorService::enableAllSensorsLocked() { } SensorDevice& dev(SensorDevice::getInstance()); dev.enableAllSensors(); - for (auto &i : mDirectConnections) { - sp<SensorDirectConnection> connection(i.promote()); - if (connection != nullptr) { - connection->recoverAll(); - } + for (const sp<SensorDirectConnection>& connection : connLock->getDirectConnections()) { + connection->recoverAll(); } } + // NOTE: This is a remote API - make sure all args are validated status_t SensorService::shellCommand(int in, int out, int err, Vector<String16>& args) { if (!checkCallingPermission(sManageSensorsPermission, nullptr, nullptr)) { @@ -734,17 +721,8 @@ bool SensorService::threadLoop() { for (int i = 0; i < count; i++) { mSensorEventBuffer[i].flags = 0; } + ConnectionSafeAutolock connLock = mConnectionHolder.lock(mLock); - // Make a copy of the connection vector as some connections may be removed during the course - // of this loop (especially when one-shot sensor events are present in the sensor_event - // buffer). Promote all connections to StrongPointers before the lock is acquired. If the - // destructor of the sp gets called when the lock is acquired, it may result in a deadlock - // as ~SensorEventConnection() needs to acquire mLock again for cleanup. So copy all the - // strongPointers to a vector before the lock is acquired. - SortedVector< sp<SensorEventConnection> > activeConnections; - populateActiveConnections(&activeConnections); - - Mutex::Autolock _l(mLock); // Poll has returned. Hold a wakelock if one of the events is from a wake up sensor. The // rest of this loop is under a critical section protected by mLock. Acquiring a wakeLock, // sending events to clients (incrementing SensorEventConnection::mWakeLockRefCount) should @@ -818,6 +796,10 @@ bool SensorService::threadLoop() { } } + // Cache the list of active connections, since we use it in multiple places below but won't + // modify it here + const std::vector<sp<SensorEventConnection>> activeConnections = connLock.getActiveConnections(); + for (int i = 0; i < count; ++i) { // Map flush_complete_events in the buffer to SensorEventConnections which called flush // on the hardware sensor. mapFlushEventsToConnections[i] will be the @@ -869,11 +851,8 @@ bool SensorService::threadLoop() { ALOGE("Dynamic sensor release error."); } - size_t numConnections = activeConnections.size(); - for (size_t i=0 ; i < numConnections; ++i) { - if (activeConnections[i] != nullptr) { - activeConnections[i]->removeSensor(handle); - } + for (const sp<SensorEventConnection>& connection : activeConnections) { + connection->removeSensor(handle); } } } @@ -882,18 +861,14 @@ bool SensorService::threadLoop() { // Send our events to clients. Check the state of wake lock for each client and release the // lock if none of the clients need it. bool needsWakeLock = false; - size_t numConnections = activeConnections.size(); - for (size_t i=0 ; i < numConnections; ++i) { - if (activeConnections[i] != nullptr) { - activeConnections[i]->sendEvents(mSensorEventBuffer, count, mSensorEventScratch, - mMapFlushEventsToConnections); - needsWakeLock |= activeConnections[i]->needsWakeLock(); - // If the connection has one-shot sensors, it may be cleaned up after first trigger. - // Early check for one-shot sensors. - if (activeConnections[i]->hasOneShotSensors()) { - cleanupAutoDisabledSensorLocked(activeConnections[i], mSensorEventBuffer, - count); - } + for (const sp<SensorEventConnection>& connection : activeConnections) { + connection->sendEvents(mSensorEventBuffer, count, mSensorEventScratch, + mMapFlushEventsToConnections); + needsWakeLock |= connection->needsWakeLock(); + // If the connection has one-shot sensors, it may be cleaned up after first trigger. + // Early check for one-shot sensors. + if (connection->hasOneShotSensors()) { + cleanupAutoDisabledSensorLocked(connection, mSensorEventBuffer, count); } } @@ -912,17 +887,11 @@ sp<Looper> SensorService::getLooper() const { } void SensorService::resetAllWakeLockRefCounts() { - SortedVector< sp<SensorEventConnection> > activeConnections; - populateActiveConnections(&activeConnections); - { - Mutex::Autolock _l(mLock); - for (size_t i=0 ; i < activeConnections.size(); ++i) { - if (activeConnections[i] != nullptr) { - activeConnections[i]->resetWakeLockRefCount(); - } - } - setWakeLockAcquiredLocked(false); + ConnectionSafeAutolock connLock = mConnectionHolder.lock(mLock); + for (const sp<SensorEventConnection>& connection : connLock.getActiveConnections()) { + connection->resetWakeLockRefCount(); } + setWakeLockAcquiredLocked(false); } void SensorService::setWakeLockAcquiredLocked(bool acquire) { @@ -1144,9 +1113,7 @@ sp<ISensorEventConnection> SensorService::createSensorEventConnection(const Stri sp<SensorEventConnection> result(new SensorEventConnection(this, uid, connPackageName, requestedMode == DATA_INJECTION, connOpPackageName, hasSensorAccess)); if (requestedMode == DATA_INJECTION) { - if (mActiveConnections.indexOf(result) < 0) { - mActiveConnections.add(result); - } + mConnectionHolder.addEventConnectionIfNotPresent(result); // Add the associated file descriptor to the Looper for polling whenever there is data to // be injected. result->updateLooperRegistration(mLooper); @@ -1162,7 +1129,7 @@ int SensorService::isDataInjectionEnabled() { sp<ISensorEventConnection> SensorService::createSensorDirectConnection( const String16& opPackageName, uint32_t size, int32_t type, int32_t format, const native_handle *resource) { - Mutex::Autolock _l(mLock); + ConnectionSafeAutolock connLock = mConnectionHolder.lock(mLock); // No new direct connections are allowed when sensor privacy is enabled if (mSensorPrivacyPolicy->isSensorPrivacyEnabled()) { @@ -1190,9 +1157,8 @@ sp<ISensorEventConnection> SensorService::createSensorDirectConnection( } // check for duplication - for (auto &i : mDirectConnections) { - sp<SensorDirectConnection> connection(i.promote()); - if (connection != nullptr && connection->isEquivalent(&mem)) { + for (const sp<SensorDirectConnection>& connection : connLock.getDirectConnections()) { + if (connection->isEquivalent(&mem)) { ALOGE("Duplicate create channel request for the same share memory"); return nullptr; } @@ -1229,7 +1195,7 @@ sp<ISensorEventConnection> SensorService::createSensorDirectConnection( return nullptr; } - SensorDirectConnection* conn = nullptr; + sp<SensorDirectConnection> conn; SensorDevice& dev(SensorDevice::getInstance()); int channelHandle = dev.registerDirectChannel(&mem); @@ -1246,7 +1212,7 @@ sp<ISensorEventConnection> SensorService::createSensorDirectConnection( } else { // add to list of direct connections // sensor service should never hold pointer or sp of SensorDirectConnection object. - mDirectConnections.add(wp<SensorDirectConnection>(conn)); + mConnectionHolder.addDirectConnection(conn); } return conn; } @@ -1358,7 +1324,7 @@ status_t SensorService::resetToNormalModeLocked() { } void SensorService::cleanupConnection(SensorEventConnection* c) { - Mutex::Autolock _l(mLock); + ConnectionSafeAutolock connLock = mConnectionHolder.lock(mLock); const wp<SensorEventConnection> connection(c); size_t size = mActiveSensors.size(); ALOGD_IF(DEBUG_CONNECTIONS, "%zu active sensors", size); @@ -1391,10 +1357,10 @@ void SensorService::cleanupConnection(SensorEventConnection* c) { } } c->updateLooperRegistration(mLooper); - mActiveConnections.remove(connection); + mConnectionHolder.removeEventConnection(connection); BatteryService::cleanup(c->getUid()); if (c->needsWakeLock()) { - checkWakeLockStateLocked(); + checkWakeLockStateLocked(&connLock); } { @@ -1414,7 +1380,7 @@ void SensorService::cleanupConnection(SensorDirectConnection* c) { SensorDevice& dev(SensorDevice::getInstance()); dev.unregisterDirectChannel(c->getHalChannelHandle()); - mDirectConnections.remove(c); + mConnectionHolder.removeDirectConnection(c); } sp<SensorInterface> SensorService::getSensorInterfaceFromHandle(int handle) const { @@ -1433,7 +1399,7 @@ status_t SensorService::enable(const sp<SensorEventConnection>& connection, return BAD_VALUE; } - Mutex::Autolock _l(mLock); + ConnectionSafeAutolock connLock = mConnectionHolder.lock(mLock); if (mCurrentOperatingMode != NORMAL && !isWhiteListedPackage(connection->getPackageName())) { return INVALID_OPERATION; @@ -1484,7 +1450,7 @@ status_t SensorService::enable(const sp<SensorEventConnection>& connection, } connection->sendEvents(&event, 1, nullptr); if (!connection->needsWakeLock() && mWakeLockAcquired) { - checkWakeLockStateLocked(); + checkWakeLockStateLocked(&connLock); } } } @@ -1497,9 +1463,7 @@ status_t SensorService::enable(const sp<SensorEventConnection>& connection, BatteryService::enableSensor(connection->getUid(), handle); // the sensor was added (which means it wasn't already there) // so, see if this connection becomes active - if (mActiveConnections.indexOf(connection) < 0) { - mActiveConnections.add(connection); - } + mConnectionHolder.addEventConnectionIfNotPresent(connection); } else { ALOGW("sensor %08x already enabled in connection %p (ignoring)", handle, connection.get()); @@ -1603,7 +1567,7 @@ status_t SensorService::cleanupWithoutDisableLocked( } if (connection->hasAnySensor() == false) { connection->updateLooperRegistration(mLooper); - mActiveConnections.remove(connection); + mConnectionHolder.removeEventConnection(connection); } // see if this sensor becomes inactive if (rec->removeConnection(connection)) { @@ -1762,22 +1726,19 @@ int SensorService::getTargetSdkVersion(const String16& opPackageName) { } void SensorService::checkWakeLockState() { - Mutex::Autolock _l(mLock); - checkWakeLockStateLocked(); + ConnectionSafeAutolock connLock = mConnectionHolder.lock(mLock); + checkWakeLockStateLocked(&connLock); } -void SensorService::checkWakeLockStateLocked() { +void SensorService::checkWakeLockStateLocked(ConnectionSafeAutolock* connLock) { if (!mWakeLockAcquired) { return; } bool releaseLock = true; - for (size_t i=0 ; i<mActiveConnections.size() ; i++) { - sp<SensorEventConnection> connection(mActiveConnections[i].promote()); - if (connection != nullptr) { - if (connection->needsWakeLock()) { - releaseLock = false; - break; - } + for (const sp<SensorEventConnection>& connection : connLock->getActiveConnections()) { + if (connection->needsWakeLock()) { + releaseLock = false; + break; } } if (releaseLock) { @@ -1793,17 +1754,6 @@ void SensorService::sendEventsFromCache(const sp<SensorEventConnection>& connect } } -void SensorService::populateActiveConnections( - SortedVector< sp<SensorEventConnection> >* activeConnections) { - Mutex::Autolock _l(mLock); - for (size_t i=0 ; i < mActiveConnections.size(); ++i) { - sp<SensorEventConnection> connection(mActiveConnections[i].promote()); - if (connection != nullptr) { - activeConnections->add(connection); - } - } -} - bool SensorService::isWhiteListedPackage(const String8& packageName) { return (packageName.contains(mWhiteListedPackage.string())); } @@ -1938,4 +1888,62 @@ binder::Status SensorService::SensorPrivacyPolicy::onSensorPrivacyChanged(bool e } return binder::Status::ok(); } -}; // namespace android + +SensorService::ConnectionSafeAutolock::ConnectionSafeAutolock( + SensorService::SensorConnectionHolder& holder, Mutex& mutex) + : mConnectionHolder(holder), mAutolock(mutex) {} + +template<typename ConnectionType> +const std::vector<sp<ConnectionType>>& SensorService::ConnectionSafeAutolock::getConnectionsHelper( + const SortedVector<wp<ConnectionType>>& connectionList, + std::vector<std::vector<sp<ConnectionType>>>* referenceHolder) { + referenceHolder->emplace_back(); + std::vector<sp<ConnectionType>>& connections = referenceHolder->back(); + for (const wp<ConnectionType>& weakConnection : connectionList){ + sp<ConnectionType> connection = weakConnection.promote(); + if (connection != nullptr) { + connections.push_back(std::move(connection)); + } + } + return connections; +} + +const std::vector<sp<SensorService::SensorEventConnection>>& + SensorService::ConnectionSafeAutolock::getActiveConnections() { + return getConnectionsHelper(mConnectionHolder.mActiveConnections, + &mReferencedActiveConnections); +} + +const std::vector<sp<SensorService::SensorDirectConnection>>& + SensorService::ConnectionSafeAutolock::getDirectConnections() { + return getConnectionsHelper(mConnectionHolder.mDirectConnections, + &mReferencedDirectConnections); +} + +void SensorService::SensorConnectionHolder::addEventConnectionIfNotPresent( + const sp<SensorService::SensorEventConnection>& connection) { + if (mActiveConnections.indexOf(connection) < 0) { + mActiveConnections.add(connection); + } +} + +void SensorService::SensorConnectionHolder::removeEventConnection( + const wp<SensorService::SensorEventConnection>& connection) { + mActiveConnections.remove(connection); +} + +void SensorService::SensorConnectionHolder::addDirectConnection( + const sp<SensorService::SensorDirectConnection>& connection) { + mDirectConnections.add(connection); +} + +void SensorService::SensorConnectionHolder::removeDirectConnection( + const wp<SensorService::SensorDirectConnection>& connection) { + mDirectConnections.remove(connection); +} + +SensorService::ConnectionSafeAutolock SensorService::SensorConnectionHolder::lock(Mutex& mutex) { + return ConnectionSafeAutolock(*this, mutex); +} + +} // namespace android diff --git a/services/sensorservice/SensorService.h b/services/sensorservice/SensorService.h index e6ec96dd0a..060b5eba70 100644 --- a/services/sensorservice/SensorService.h +++ b/services/sensorservice/SensorService.h @@ -20,6 +20,7 @@ #include "SensorList.h" #include "RecentEventLogger.h" +#include <android-base/macros.h> #include <binder/AppOpsManager.h> #include <binder/BinderService.h> #include <binder/IUidObserver.h> @@ -42,6 +43,7 @@ #include <sys/types.h> #include <unordered_map> #include <unordered_set> +#include <vector> #if __clang__ // Clang warns about SensorEventConnection::dump hiding BBinder::dump. The cause isn't fixable @@ -95,10 +97,67 @@ private: friend class BinderService<SensorService>; // nested class/struct for internal use - class SensorRecord; + class ConnectionSafeAutolock; + class SensorConnectionHolder; class SensorEventAckReceiver; + class SensorRecord; class SensorRegistrationInfo; + // Promoting a SensorEventConnection or SensorDirectConnection from wp to sp must be done with + // mLock held, but destroying that sp must be done unlocked to avoid a race condition that + // causes a deadlock (remote dies while we hold a local sp, then our decStrong() call invokes + // the dtor -> cleanupConnection() tries to re-lock the mutex). This class ensures safe usage + // by wrapping a Mutex::Autolock on SensorService's mLock, plus vectors that hold promoted sp<> + // references until the lock is released, when they are safely destroyed. + // All read accesses to the connection lists in mConnectionHolder must be done via this class. + class ConnectionSafeAutolock final { + public: + // Returns a list of non-null promoted connection references + const std::vector<sp<SensorEventConnection>>& getActiveConnections(); + const std::vector<sp<SensorDirectConnection>>& getDirectConnections(); + + private: + // Constructed via SensorConnectionHolder::lock() + friend class SensorConnectionHolder; + explicit ConnectionSafeAutolock(SensorConnectionHolder& holder, Mutex& mutex); + DISALLOW_IMPLICIT_CONSTRUCTORS(ConnectionSafeAutolock); + + // NOTE: Order of these members is important, as the destructor for non-static members + // get invoked in the reverse order of their declaration. Here we are relying on the + // Autolock to be destroyed *before* the vectors, so the sp<> objects are destroyed without + // the lock held, which avoids the deadlock. + SensorConnectionHolder& mConnectionHolder; + std::vector<std::vector<sp<SensorEventConnection>>> mReferencedActiveConnections; + std::vector<std::vector<sp<SensorDirectConnection>>> mReferencedDirectConnections; + Mutex::Autolock mAutolock; + + template<typename ConnectionType> + const std::vector<sp<ConnectionType>>& getConnectionsHelper( + const SortedVector<wp<ConnectionType>>& connectionList, + std::vector<std::vector<sp<ConnectionType>>>* referenceHolder); + }; + + // Encapsulates the collection of active SensorEventConection and SensorDirectConnection + // references. Write access is done through this class with mLock held, but all read access + // must be routed through ConnectionSafeAutolock. + class SensorConnectionHolder { + public: + void addEventConnectionIfNotPresent(const sp<SensorEventConnection>& connection); + void removeEventConnection(const wp<SensorEventConnection>& connection); + + void addDirectConnection(const sp<SensorDirectConnection>& connection); + void removeDirectConnection(const wp<SensorDirectConnection>& connection); + + // Pass in the mutex that protects this connection holder; acquires the lock and returns an + // object that can be used to safely read the lists of connections + ConnectionSafeAutolock lock(Mutex& mutex); + + private: + friend class ConnectionSafeAutolock; + SortedVector< wp<SensorEventConnection> > mActiveConnections; + SortedVector< wp<SensorDirectConnection> > mDirectConnections; + }; + // If accessing a sensor we need to make sure the UID has access to it. If // the app UID is idle then it cannot access sensors and gets no trigger // events, no on-change events, flush event behavior does not change, and @@ -250,7 +309,7 @@ private: // method checks whether all the events from these wake up sensors have been delivered to the // corresponding applications, if yes the wakelock is released. void checkWakeLockState(); - void checkWakeLockStateLocked(); + void checkWakeLockStateLocked(ConnectionSafeAutolock* connLock); bool isWakeLockAcquired(); bool isWakeUpSensorEvent(const sensors_event_t& event) const; @@ -268,10 +327,6 @@ private: // Send events from the event cache for this particular connection. void sendEventsFromCache(const sp<SensorEventConnection>& connection); - // Promote all weak referecences in mActiveConnections vector to strong references and add them - // to the output vector. - void populateActiveConnections( SortedVector< sp<SensorEventConnection> >* activeConnections); - // If SensorService is operating in RESTRICTED mode, only select whitelisted packages are // allowed to register for or call flush on sensors. Typically only cts test packages are // allowed. @@ -306,10 +361,10 @@ private: // temporarily stops all active direct connections and disables all sensors void disableAllSensors(); - void disableAllSensorsLocked(); + void disableAllSensorsLocked(ConnectionSafeAutolock* connLock); // restarts the previously stopped direct connections and enables all sensors void enableAllSensors(); - void enableAllSensorsLocked(); + void enableAllSensorsLocked(ConnectionSafeAutolock* connLock); static uint8_t sHmacGlobalKey[128]; static bool sHmacGlobalKeyIsValid; @@ -327,12 +382,13 @@ private: mutable Mutex mLock; DefaultKeyedVector<int, SensorRecord*> mActiveSensors; std::unordered_set<int> mActiveVirtualSensors; - SortedVector< wp<SensorEventConnection> > mActiveConnections; + SensorConnectionHolder mConnectionHolder; bool mWakeLockAcquired; sensors_event_t *mSensorEventBuffer, *mSensorEventScratch; + // WARNING: these SensorEventConnection instances must not be promoted to sp, except via + // modification to add support for them in ConnectionSafeAutolock wp<const SensorEventConnection> * mMapFlushEventsToConnections; std::unordered_map<int, SensorServiceUtil::RecentEventLogger*> mRecentEvent; - SortedVector< wp<SensorDirectConnection> > mDirectConnections; Mode mCurrentOperatingMode; // This packagaName is set when SensorService is in RESTRICTED or DATA_INJECTION mode. Only diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index d6e86eb0e5..fdf9da342f 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -2048,7 +2048,7 @@ InputWindowInfo Layer::fillInputInfo() { InputWindowInfo info = mDrawingState.inputInfo; if (info.displayId == ADISPLAY_ID_NONE) { - info.displayId = mDrawingState.layerStack; + info.displayId = getLayerStack(); } ui::Transform t = getTransform(); diff --git a/services/surfaceflinger/Scheduler/LayerInfo.h b/services/surfaceflinger/Scheduler/LayerInfo.h index a7337817e3..17afddac28 100644 --- a/services/surfaceflinger/Scheduler/LayerInfo.h +++ b/services/surfaceflinger/Scheduler/LayerInfo.h @@ -109,7 +109,7 @@ class LayerInfo { bool isLowActivityLayer() const { // We want to make sure that we received more than two frames from the layer // in order to check low activity. - if (mElements.size() < 2) { + if (mElements.size() < scheduler::LOW_ACTIVITY_BUFFERS + 1) { return false; } @@ -118,7 +118,8 @@ class LayerInfo { // Check the frame before last to determine whether there is low activity. // If that frame is older than LOW_ACTIVITY_EPSILON_NS, the layer is sending // infrequent updates. - if (mElements.at(mElements.size() - 2) < obsoleteEpsilon) { + if (mElements.at(mElements.size() - (scheduler::LOW_ACTIVITY_BUFFERS + 1)) < + obsoleteEpsilon) { return true; } diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp index 5a98232e97..f8bd95872c 100644 --- a/services/surfaceflinger/Scheduler/Scheduler.cpp +++ b/services/surfaceflinger/Scheduler/Scheduler.cpp @@ -567,9 +567,13 @@ Scheduler::RefreshRateType Scheduler::calculateRefreshRateType() { } // Content detection is on, find the appropriate refresh rate with minimal error - // TODO(b/139751853): Scan allowed refresh rates only (SurfaceFlinger::mAllowedDisplayConfigs) - auto iter = min_element(mRefreshRateConfigs.getRefreshRates().cbegin(), - mRefreshRateConfigs.getRefreshRates().cend(), + auto begin = mRefreshRateConfigs.getRefreshRates().cbegin(); + + // Skip POWER_SAVING config as it is not a real config + if (begin->first == RefreshRateType::POWER_SAVING) { + ++begin; + } + auto iter = min_element(begin, mRefreshRateConfigs.getRefreshRates().cend(), [rate = mContentRefreshRate](const auto& l, const auto& r) -> bool { return std::abs(l.second->fps - static_cast<float>(rate)) < std::abs(r.second->fps - static_cast<float>(rate)); diff --git a/services/surfaceflinger/Scheduler/SchedulerUtils.h b/services/surfaceflinger/Scheduler/SchedulerUtils.h index ced1899109..ac10f83ad9 100644 --- a/services/surfaceflinger/Scheduler/SchedulerUtils.h +++ b/services/surfaceflinger/Scheduler/SchedulerUtils.h @@ -42,9 +42,11 @@ static constexpr uint32_t HWC2_SCREEN_OFF_CONFIG_ID = 0xffffffff; // or waiting idle in messaging app, when cursor is blinking. static constexpr std::chrono::nanoseconds OBSOLETE_TIME_EPSILON_NS = 1200ms; -// Layer is considered low activity if the buffers come more than LOW_ACTIVITY_EPSILON_NS -// apart. This is helping SF to vote for lower refresh rates when there is not activity +// Layer is considered low activity if the LOW_ACTIVITY_BUFFERS buffers come more than +// LOW_ACTIVITY_EPSILON_NS apart. +// This is helping SF to vote for lower refresh rates when there is not activity // in screen. +static constexpr int LOW_ACTIVITY_BUFFERS = 2; static constexpr std::chrono::nanoseconds LOW_ACTIVITY_EPSILON_NS = 250ms; // Calculates the statistical mean (average) in the data structure (array, vector). The diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index ffb7012bb3..23b399f838 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -1681,7 +1681,8 @@ void SurfaceFlinger::updateVrFlinger() { setTransactionFlags(eDisplayTransactionNeeded); } -bool SurfaceFlinger::previousFrameMissed() NO_THREAD_SAFETY_ANALYSIS { +bool SurfaceFlinger::previousFrameMissed(int graceTimeMs) NO_THREAD_SAFETY_ANALYSIS { + ATRACE_CALL(); // We are storing the last 2 present fences. If sf's phase offset is to be // woken up before the actual vsync but targeting the next vsync, we need to check // fence N-2 @@ -1690,7 +1691,15 @@ bool SurfaceFlinger::previousFrameMissed() NO_THREAD_SAFETY_ANALYSIS { ? mPreviousPresentFences[0] : mPreviousPresentFences[1]; - return fence != Fence::NO_FENCE && (fence->getStatus() == Fence::Status::Unsignaled); + if (fence == Fence::NO_FENCE) { + return false; + } + + if (graceTimeMs > 0 && fence->getStatus() == Fence::Status::Unsignaled) { + fence->wait(graceTimeMs); + } + + return (fence->getStatus() == Fence::Status::Unsignaled); } void SurfaceFlinger::populateExpectedPresentTime() NO_THREAD_SAFETY_ANALYSIS { @@ -1713,7 +1722,15 @@ void SurfaceFlinger::onMessageReceived(int32_t what) NO_THREAD_SAFETY_ANALYSIS { // seeing this same value. populateExpectedPresentTime(); - bool frameMissed = previousFrameMissed(); + // When Backpressure propagation is enabled we want to give a small grace period + // for the present fence to fire instead of just giving up on this frame to handle cases + // where present fence is just about to get signaled. + const int graceTimeForPresentFenceMs = + (mPropagateBackpressure && + (mPropagateBackpressureClientComposition || !mHadClientComposition)) + ? 1 + : 0; + bool frameMissed = previousFrameMissed(graceTimeForPresentFenceMs); bool hwcFrameMissed = mHadDeviceComposition && frameMissed; bool gpuFrameMissed = mHadClientComposition && frameMissed; ATRACE_INT("FrameMissed", static_cast<int>(frameMissed)); diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index a59a1e0086..2718e0e860 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -851,7 +851,7 @@ private: return hwcDisplayId ? getHwComposer().toPhysicalDisplayId(*hwcDisplayId) : std::nullopt; } - bool previousFrameMissed(); + bool previousFrameMissed(int graceTimeMs = 0); void setVsyncEnabledInHWC(DisplayId displayId, HWC2::Vsync enabled); /* |