From 3bb0fbeaeab8e55b1856c908f7e0b918f3d174e5 Mon Sep 17 00:00:00 2001 From: raylinhsu Date: Mon, 11 Jan 2021 11:56:27 +0800 Subject: [PATCH 1/2] Added to enable/disable log for DisplayModeDirector Added to enable/disable log for DisplayModeDirector dynamically Bug: 177975767 Test: enable DMD logging with $adb shell cmd display dmd-logging-enable This should start logging the collected for DMD. You can filter it with : $adb logcat | grep 'DisplayModeDirector' Change-Id: I66478dda66d5d89193ef1838ab3afca94bf72313 --- .../server/display/DisplayManagerService.java | 8 +++ .../display/DisplayManagerShellCommand.java | 13 ++++ .../server/display/DisplayModeDirector.java | 66 ++++++++++++++++--- 3 files changed, 77 insertions(+), 10 deletions(-) diff --git a/services/core/java/com/android/server/display/DisplayManagerService.java b/services/core/java/com/android/server/display/DisplayManagerService.java index 55103ca6cd1c9..acddb3c6519e9 100644 --- a/services/core/java/com/android/server/display/DisplayManagerService.java +++ b/services/core/java/com/android/server/display/DisplayManagerService.java @@ -1538,6 +1538,14 @@ public final class DisplayManagerService extends SystemService { } } + void setDisplayModeDirectorLoggingEnabled(boolean enabled) { + synchronized (mSyncRoot) { + if (mDisplayModeDirector != null) { + mDisplayModeDirector.setLoggingEnabled(enabled); + } + } + } + void setAmbientColorTemperatureOverride(float cct) { synchronized (mSyncRoot) { final DisplayPowerController displayPowerController = mDisplayPowerControllers.get( diff --git a/services/core/java/com/android/server/display/DisplayManagerShellCommand.java b/services/core/java/com/android/server/display/DisplayManagerShellCommand.java index 111664a078df5..aaea15a27e015 100644 --- a/services/core/java/com/android/server/display/DisplayManagerShellCommand.java +++ b/services/core/java/com/android/server/display/DisplayManagerShellCommand.java @@ -54,6 +54,10 @@ class DisplayManagerShellCommand extends ShellCommand { return setDisplayWhiteBalanceLoggingEnabled(true); case "dwb-logging-disable": return setDisplayWhiteBalanceLoggingEnabled(false); + case "dmd-logging-enable": + return setDisplayModeDirectorLoggingEnabled(true); + case "dmd-logging-disable": + return setDisplayModeDirectorLoggingEnabled(false); case "dwb-set-cct": return setAmbientColorTemperatureOverride(); case "set-fold": @@ -82,6 +86,10 @@ class DisplayManagerShellCommand extends ShellCommand { pw.println(" Enable display white-balance logging."); pw.println(" dwb-logging-disable"); pw.println(" Disable display white-balance logging."); + pw.println(" dmd-logging-enable"); + pw.println(" Enable display mode director logging."); + pw.println(" dmd-logging-disable"); + pw.println(" Disable display mode director logging."); pw.println(" dwb-set-cct CCT"); pw.println(" Sets the ambient color temperature override to CCT (use -1 to disable)."); pw.println(" set-fold [fold|unfold|reset]"); @@ -136,6 +144,11 @@ class DisplayManagerShellCommand extends ShellCommand { return 0; } + private int setDisplayModeDirectorLoggingEnabled(boolean enabled) { + mService.setDisplayModeDirectorLoggingEnabled(enabled); + return 0; + } + private int setAmbientColorTemperatureOverride() { String cctText = getNextArg(); if (cctText == null) { diff --git a/services/core/java/com/android/server/display/DisplayModeDirector.java b/services/core/java/com/android/server/display/DisplayModeDirector.java index 006f87571f829..cafa45bb4df5d 100644 --- a/services/core/java/com/android/server/display/DisplayModeDirector.java +++ b/services/core/java/com/android/server/display/DisplayModeDirector.java @@ -67,7 +67,7 @@ import java.util.Objects; */ public class DisplayModeDirector { private static final String TAG = "DisplayModeDirector"; - private static final boolean DEBUG = false; + private boolean mLoggingEnabled; private static final int MSG_REFRESH_RATE_RANGE_CHANGED = 1; private static final int MSG_LOW_BRIGHTNESS_THRESHOLDS_CHANGED = 2; @@ -155,6 +155,14 @@ public class DisplayModeDirector { } } + public void setLoggingEnabled(boolean loggingEnabled) { + if (mLoggingEnabled == loggingEnabled) { + return; + } + mLoggingEnabled = loggingEnabled; + mBrightnessObserver.setLoggingEnabled(loggingEnabled); + } + @NonNull private SparseArray getVotesLocked(int displayId) { SparseArray displayVotes = mVotesByDisplay.get(displayId); @@ -269,7 +277,7 @@ public class DisplayModeDirector { availableModes = filterModes(modes, primarySummary); if (availableModes.length > 0) { - if (DEBUG) { + if (mLoggingEnabled) { Slog.w(TAG, "Found available modes=" + Arrays.toString(availableModes) + " with lowest priority considered " + Vote.priorityToString(lowestConsideredPriority) @@ -282,7 +290,7 @@ public class DisplayModeDirector { break; } - if (DEBUG) { + if (mLoggingEnabled) { Slog.w(TAG, "Couldn't find available modes with lowest priority set to " + Vote.priorityToString(lowestConsideredPriority) + " and with the following constraints: " @@ -307,7 +315,7 @@ public class DisplayModeDirector { Math.min(appRequestSummary.minRefreshRate, primarySummary.minRefreshRate); appRequestSummary.maxRefreshRate = Math.max(appRequestSummary.maxRefreshRate, primarySummary.maxRefreshRate); - if (DEBUG) { + if (mLoggingEnabled) { Slog.i(TAG, String.format("App request range: [%.0f %.0f]", appRequestSummary.minRefreshRate, @@ -357,7 +365,7 @@ public class DisplayModeDirector { for (Display.Mode mode : supportedModes) { if (mode.getPhysicalWidth() != summary.width || mode.getPhysicalHeight() != summary.height) { - if (DEBUG) { + if (mLoggingEnabled) { Slog.w(TAG, "Discarding mode " + mode.getModeId() + ", wrong size" + ": desiredWidth=" + summary.width + ": desiredHeight=" + summary.height @@ -372,7 +380,7 @@ public class DisplayModeDirector { // comparison. if (refreshRate < (summary.minRefreshRate - FLOAT_TOLERANCE) || refreshRate > (summary.maxRefreshRate + FLOAT_TOLERANCE)) { - if (DEBUG) { + if (mLoggingEnabled) { Slog.w(TAG, "Discarding mode " + mode.getModeId() + ", outside refresh rate bounds" + ": minRefreshRate=" + summary.minRefreshRate @@ -516,7 +524,7 @@ public class DisplayModeDirector { } private void updateVoteLocked(int displayId, int priority, Vote vote) { - if (DEBUG) { + if (mLoggingEnabled) { Slog.i(TAG, "updateVoteLocked(displayId=" + displayId + ", priority=" + Vote.priorityToString(priority) + ", vote=" + vote + ")"); @@ -537,7 +545,7 @@ public class DisplayModeDirector { } if (votes.size() == 0) { - if (DEBUG) { + if (mLoggingEnabled) { Slog.i(TAG, "No votes left for display " + displayId + ", removing."); } mVotesByDisplay.remove(displayId); @@ -1287,6 +1295,7 @@ public class DisplayModeDirector { private boolean mShouldObserveAmbientLowChange; private boolean mShouldObserveDisplayHighChange; private boolean mShouldObserveAmbientHighChange; + private boolean mLoggingEnabled; private SensorManager mSensorManager; private Sensor mLightSensor; @@ -1415,6 +1424,14 @@ public class DisplayModeDirector { mDeviceConfigDisplaySettings.startListening(); } + public void setLoggingEnabled(boolean loggingEnabled) { + if (mLoggingEnabled == loggingEnabled) { + return; + } + mLoggingEnabled = loggingEnabled; + mLightSensorListener.setLoggingEnabled(loggingEnabled); + } + public void onRefreshRateSettingChangedLocked(float min, float max) { boolean changeable = (max - min > 1f && max > 60f); if (mRefreshRateChangeable != changeable) { @@ -1691,7 +1708,7 @@ public class DisplayModeDirector { vote = Vote.forRefreshRates(mRefreshRateInHighZone, mRefreshRateInHighZone); } - if (DEBUG) { + if (mLoggingEnabled) { Slog.d(TAG, "Display brightness " + mBrightness + ", ambient lux " + mAmbientLux + ", Vote " + vote); } @@ -1720,6 +1737,11 @@ public class DisplayModeDirector { @VisibleForTesting public void setDefaultDisplayState(int state) { + if (mLoggingEnabled) { + Slog.d(TAG, "setDefaultDisplayState: mDefaultDisplayState = " + + mDefaultDisplayState + ", state = " + state); + } + if (mDefaultDisplayState != state) { mDefaultDisplayState = state; updateSensorStatus(); @@ -1731,13 +1753,28 @@ public class DisplayModeDirector { return; } + if (mLoggingEnabled) { + Slog.d(TAG, "updateSensorStatus: mShouldObserveAmbientLowChange = " + + mShouldObserveAmbientLowChange + ", mShouldObserveAmbientHighChange = " + + mShouldObserveAmbientHighChange); + Slog.d(TAG, "updateSensorStatus: mLowPowerModeEnabled = " + + mLowPowerModeEnabled + ", mRefreshRateChangeable = " + + mRefreshRateChangeable); + } + if ((mShouldObserveAmbientLowChange || mShouldObserveAmbientHighChange) && isDeviceActive() && !mLowPowerModeEnabled && mRefreshRateChangeable) { mSensorManager.registerListener(mLightSensorListener, mLightSensor, LIGHT_SENSOR_RATE_MS * 1000, mHandler); + if (mLoggingEnabled) { + Slog.d(TAG, "updateSensorStatus: registerListener"); + } } else { mLightSensorListener.removeCallbacks(); mSensorManager.unregisterListener(mLightSensorListener); + if (mLoggingEnabled) { + Slog.d(TAG, "updateSensorStatus: unregisterListener"); + } } } @@ -1751,16 +1788,25 @@ public class DisplayModeDirector { final private static int INJECT_EVENTS_INTERVAL_MS = LIGHT_SENSOR_RATE_MS; private float mLastSensorData; private long mTimestamp; + private boolean mLoggingEnabled; public void dumpLocked(PrintWriter pw) { pw.println(" mLastSensorData: " + mLastSensorData); pw.println(" mTimestamp: " + formatTimestamp(mTimestamp)); } + + public void setLoggingEnabled(boolean loggingEnabled) { + if (mLoggingEnabled == loggingEnabled) { + return; + } + mLoggingEnabled = loggingEnabled; + } + @Override public void onSensorChanged(SensorEvent event) { mLastSensorData = event.values[0]; - if (DEBUG) { + if (mLoggingEnabled) { Slog.d(TAG, "On sensor changed: " + mLastSensorData); } From a1324f308b657113242ef574cbb050223b887fb8 Mon Sep 17 00:00:00 2001 From: raylinhsu Date: Fri, 22 Jan 2021 11:41:05 +0800 Subject: [PATCH 2/2] Fix ambient information which did not get for locking refresh rate Sometimes we will see that the display state is ON but the DeviceInteractive is false and then BrightnessObserver can not register the light sensor.Hence, it can not judge if the threshold meets the condition for blocking zones.This patch is for fixing this kind of case. Bug: 176962616 Test: atest DisplayModeDirectorTest Test: no fps transition in the blocking zone when the display state change Test: check if the lux will update when the display state change Change-Id: Idcfcbfa9e41bb9a34e4808db6fdd7f8a5901457b --- .../android/server/display/DisplayModeDirector.java | 13 +------------ .../server/display/DisplayModeDirectorTest.java | 5 ----- 2 files changed, 1 insertion(+), 17 deletions(-) diff --git a/services/core/java/com/android/server/display/DisplayModeDirector.java b/services/core/java/com/android/server/display/DisplayModeDirector.java index cafa45bb4df5d..dce6bd849953f 100644 --- a/services/core/java/com/android/server/display/DisplayModeDirector.java +++ b/services/core/java/com/android/server/display/DisplayModeDirector.java @@ -1312,7 +1312,6 @@ public class DisplayModeDirector { // changeable and low power mode off. After initialization, these states will // be updated from the same handler thread. private int mDefaultDisplayState = Display.STATE_UNKNOWN; - private boolean mIsDeviceActive = false; private boolean mRefreshRateChangeable = false; private boolean mLowPowerModeEnabled = false; @@ -1502,7 +1501,6 @@ public class DisplayModeDirector { pw.println(" mAmbientLux: " + mAmbientLux); pw.println(" mBrightness: " + mBrightness); pw.println(" mDefaultDisplayState: " + mDefaultDisplayState); - pw.println(" mIsDeviceActive: " + mIsDeviceActive); pw.println(" mLowPowerModeEnabled: " + mLowPowerModeEnabled); pw.println(" mRefreshRateChangeable: " + mRefreshRateChangeable); pw.println(" mShouldObserveDisplayLowChange: " + mShouldObserveDisplayLowChange); @@ -1779,9 +1777,7 @@ public class DisplayModeDirector { } private boolean isDeviceActive() { - mIsDeviceActive = mInjector.isDeviceInteractive(mContext); - return (mDefaultDisplayState == Display.STATE_ON) - && mIsDeviceActive; + return mDefaultDisplayState == Display.STATE_ON; } private final class LightSensorEventListener implements SensorEventListener { @@ -2055,8 +2051,6 @@ public class DisplayModeDirector { void registerPeakRefreshRateObserver(@NonNull ContentResolver cr, @NonNull ContentObserver observer); - - boolean isDeviceInteractive(@NonNull Context context); } @VisibleForTesting @@ -2087,11 +2081,6 @@ public class DisplayModeDirector { cr.registerContentObserver(PEAK_REFRESH_RATE_URI, false /*notifyDescendants*/, observer, UserHandle.USER_SYSTEM); } - - @Override - public boolean isDeviceInteractive(@NonNull Context ctx) { - return ctx.getSystemService(PowerManager.class).isInteractive(); - } } } diff --git a/services/tests/servicestests/src/com/android/server/display/DisplayModeDirectorTest.java b/services/tests/servicestests/src/com/android/server/display/DisplayModeDirectorTest.java index 26c304f763d62..2e3178b138929 100644 --- a/services/tests/servicestests/src/com/android/server/display/DisplayModeDirectorTest.java +++ b/services/tests/servicestests/src/com/android/server/display/DisplayModeDirectorTest.java @@ -818,10 +818,5 @@ public class DisplayModeDirectorTest { PEAK_REFRESH_RATE_URI); } } - - @Override - public boolean isDeviceInteractive(@NonNull Context context) { - return true; - } } }