From 99b9c47dee739ffad470717e3d9a971ceec804c6 Mon Sep 17 00:00:00 2001 From: Nicolo' Mazzucato Date: Thu, 10 Mar 2022 16:43:27 +0000 Subject: [PATCH] Add display committed state to DisplayInfo This adds the committed state to the display. The `state` is used to set the correct power state (e.g. turning display on/off), however there was no way to know when this power state change finished. This new "committedState" aims to fulfill this gap. DisplayListeners receives a `onDisplayChanged` callback whenever the power state change is fully committed, and can get this information from DisplayInfo. This will be used in a follow up cl to improve unfold latency performances. Bug: 223808403 Bug: 220690092 Bug: 197515205 Bug: 273204503 Test: atest LocalDisplayAdapterTest && manual Change-Id: Ia2fbab768b696c59578d974c88157ee3c8f3ddde Merged-In: Ia2fbab768b696c59578d974c88157ee3c8f3ddde (cherry picked from commit 07c6ebdd97ea3e9e3640fa5165b24249a52dd7e5) --- .../display/DisplayManagerGlobal.java | 27 ++++++++++++++- core/java/android/view/Display.java | 15 ++++++++ core/java/android/view/DisplayInfo.java | 12 +++++++ .../server/display/DisplayDeviceInfo.java | 13 +++++-- .../server/display/LocalDisplayAdapter.java | 15 ++++++++ .../server/display/LogicalDisplay.java | 1 + .../display/LocalDisplayAdapterTest.java | 34 +++++++++++++++++++ 7 files changed, 114 insertions(+), 3 deletions(-) diff --git a/core/java/android/hardware/display/DisplayManagerGlobal.java b/core/java/android/hardware/display/DisplayManagerGlobal.java index 63dc7c7ed661..aef2ae26747d 100644 --- a/core/java/android/hardware/display/DisplayManagerGlobal.java +++ b/core/java/android/hardware/display/DisplayManagerGlobal.java @@ -40,6 +40,7 @@ import android.os.Looper; import android.os.Message; import android.os.RemoteException; import android.os.ServiceManager; +import android.os.Trace; import android.util.Log; import android.util.Pair; import android.util.SparseArray; @@ -1006,7 +1007,8 @@ public final class DisplayManagerGlobal { @Override public void onDisplayEvent(int displayId, @DisplayEvent int event) { if (DEBUG) { - Log.d(TAG, "onDisplayEvent: displayId=" + displayId + ", event=" + event); + Log.d(TAG, "onDisplayEvent: displayId=" + displayId + ", event=" + eventToString( + event)); } handleDisplayEvent(displayId, event); } @@ -1040,6 +1042,12 @@ public final class DisplayManagerGlobal { @Override public void handleMessage(Message msg) { + if (DEBUG) { + Trace.beginSection( + "DisplayListenerDelegate(" + eventToString(msg.what) + + ", display=" + msg.arg1 + + ", listener=" + mListener.getClass() + ")"); + } switch (msg.what) { case EVENT_DISPLAY_ADDED: if ((mEventsMask & DisplayManager.EVENT_FLAG_DISPLAY_ADDED) != 0) { @@ -1066,6 +1074,9 @@ public final class DisplayManagerGlobal { } break; } + if (DEBUG) { + Trace.endSection(); + } } } @@ -1172,4 +1183,18 @@ public final class DisplayManagerGlobal { updateCallbackIfNeededLocked(); } } + + private static String eventToString(@DisplayEvent int event) { + switch (event) { + case EVENT_DISPLAY_ADDED: + return "ADDED"; + case EVENT_DISPLAY_CHANGED: + return "CHANGED"; + case EVENT_DISPLAY_REMOVED: + return "REMOVED"; + case EVENT_DISPLAY_BRIGHTNESS_CHANGED: + return "BRIGHTNESS_CHANGED"; + } + return "UNKNOWN"; + } } diff --git a/core/java/android/view/Display.java b/core/java/android/view/Display.java index 52d222b19b6a..f85f9067e347 100644 --- a/core/java/android/view/Display.java +++ b/core/java/android/view/Display.java @@ -1599,6 +1599,21 @@ public final class Display { } } + /** + * Returns the committed state of the display. + * + * @return The latest committed display state, such as {@link #STATE_ON}. The display state + * {@link Display#getState()} is set as committed only after power state changes finish. + * + * @hide + */ + public int getCommittedState() { + synchronized (mLock) { + updateDisplayInfoLocked(); + return mIsValid ? mDisplayInfo.committedState : STATE_UNKNOWN; + } + } + /** * Returns true if the specified UID has access to this display. * @hide diff --git a/core/java/android/view/DisplayInfo.java b/core/java/android/view/DisplayInfo.java index 12ce8ee5e0ad..f65a69a8e2bc 100644 --- a/core/java/android/view/DisplayInfo.java +++ b/core/java/android/view/DisplayInfo.java @@ -252,6 +252,12 @@ public final class DisplayInfo implements Parcelable { */ public int state; + /** + * The current committed state of the display. For example, this becomes + * {@link android.view.Display#STATE_ON} only after the power state ON is fully committed. + */ + public int committedState; + /** * The UID of the application that owns this display, or zero if it is owned by the system. *

@@ -380,6 +386,7 @@ public final class DisplayInfo implements Parcelable { && appVsyncOffsetNanos == other.appVsyncOffsetNanos && presentationDeadlineNanos == other.presentationDeadlineNanos && state == other.state + && committedState == other.committedState && ownerUid == other.ownerUid && Objects.equals(ownerPackageName, other.ownerPackageName) && removeMode == other.removeMode @@ -431,6 +438,7 @@ public final class DisplayInfo implements Parcelable { appVsyncOffsetNanos = other.appVsyncOffsetNanos; presentationDeadlineNanos = other.presentationDeadlineNanos; state = other.state; + committedState = other.committedState; ownerUid = other.ownerUid; ownerPackageName = other.ownerPackageName; removeMode = other.removeMode; @@ -482,6 +490,7 @@ public final class DisplayInfo implements Parcelable { appVsyncOffsetNanos = source.readLong(); presentationDeadlineNanos = source.readLong(); state = source.readInt(); + committedState = source.readInt(); ownerUid = source.readInt(); ownerPackageName = source.readString8(); uniqueId = source.readString8(); @@ -538,6 +547,7 @@ public final class DisplayInfo implements Parcelable { dest.writeLong(appVsyncOffsetNanos); dest.writeLong(presentationDeadlineNanos); dest.writeInt(state); + dest.writeInt(committedState); dest.writeInt(ownerUid); dest.writeString8(ownerPackageName); dest.writeString8(uniqueId); @@ -761,6 +771,8 @@ public final class DisplayInfo implements Parcelable { sb.append(rotation); sb.append(", state "); sb.append(Display.stateToString(state)); + sb.append(", committedState "); + sb.append(Display.stateToString(committedState)); if (Process.myUid() != Process.SYSTEM_UID) { sb.append("}"); diff --git a/services/core/java/com/android/server/display/DisplayDeviceInfo.java b/services/core/java/com/android/server/display/DisplayDeviceInfo.java index dbbd354a2ff0..84dfe860bc84 100644 --- a/services/core/java/com/android/server/display/DisplayDeviceInfo.java +++ b/services/core/java/com/android/server/display/DisplayDeviceInfo.java @@ -180,7 +180,7 @@ final class DisplayDeviceInfo { public static final int TOUCH_VIRTUAL = 3; /** - * Diff result: The {@link #state} fields differ. + * Diff result: The {@link #state} or {@link #committedState} fields differ. */ public static final int DIFF_STATE = 1 << 0; @@ -341,6 +341,13 @@ final class DisplayDeviceInfo { */ public int state = Display.STATE_ON; + /** + * Display committed state. + * + * This matches {@link DisplayDeviceInfo#state} only after the power state change finishes. + */ + public int committedState = Display.STATE_UNKNOWN; + /** * The UID of the application that owns this display, or zero if it is owned by the system. *

@@ -394,7 +401,7 @@ final class DisplayDeviceInfo { */ public int diff(DisplayDeviceInfo other) { int diff = 0; - if (state != other.state) { + if (state != other.state || committedState != other.committedState) { diff |= DIFF_STATE; } if (colorMode != other.colorMode) { @@ -468,6 +475,7 @@ final class DisplayDeviceInfo { address = other.address; deviceProductInfo = other.deviceProductInfo; state = other.state; + committedState = other.committedState; ownerUid = other.ownerUid; ownerPackageName = other.ownerPackageName; frameRateOverrides = other.frameRateOverrides; @@ -508,6 +516,7 @@ final class DisplayDeviceInfo { } sb.append(", deviceProductInfo ").append(deviceProductInfo); sb.append(", state ").append(Display.stateToString(state)); + sb.append(", committedState ").append(Display.stateToString(committedState)); if (ownerUid != 0 || ownerPackageName != null) { sb.append(", owner ").append(ownerPackageName); sb.append(" (uid ").append(ownerUid).append(")"); diff --git a/services/core/java/com/android/server/display/LocalDisplayAdapter.java b/services/core/java/com/android/server/display/LocalDisplayAdapter.java index efb2cb7a3283..58a182a61e44 100644 --- a/services/core/java/com/android/server/display/LocalDisplayAdapter.java +++ b/services/core/java/com/android/server/display/LocalDisplayAdapter.java @@ -198,6 +198,8 @@ final class LocalDisplayAdapter extends DisplayAdapter { private DisplayDeviceInfo mInfo; private boolean mHavePendingChanges; private int mState = Display.STATE_UNKNOWN; + private int mCommittedState = Display.STATE_UNKNOWN; + // This is only set in the runnable returned from requestDisplayStateLocked. private float mBrightnessState = PowerManager.BRIGHTNESS_INVALID_FLOAT; private float mSdrBrightnessState = PowerManager.BRIGHTNESS_INVALID_FLOAT; @@ -635,6 +637,7 @@ final class LocalDisplayAdapter extends DisplayAdapter { mInfo.appVsyncOffsetNanos = mActiveSfDisplayMode.appVsyncOffsetNanos; mInfo.presentationDeadlineNanos = mActiveSfDisplayMode.presentationDeadlineNanos; mInfo.state = mState; + mInfo.committedState = mCommittedState; mInfo.uniqueId = getUniqueId(); final DisplayAddress.Physical physicalAddress = DisplayAddress.fromPhysicalDisplayId(mPhysicalDisplayId); @@ -822,6 +825,7 @@ final class LocalDisplayAdapter extends DisplayAdapter { } finally { Trace.traceEnd(Trace.TRACE_TAG_POWER); } + setCommittedState(state); // If we're entering a suspended (but not OFF) power state and we // have a sidekick available, tell it now that it can take control. if (Display.isSuspendedState(state) && state != Display.STATE_OFF @@ -836,6 +840,16 @@ final class LocalDisplayAdapter extends DisplayAdapter { } } + private void setCommittedState(int state) { + // After the display state is set, let's update the committed state. + getHandler().post(() -> { + synchronized (getSyncRoot()) { + mCommittedState = state; + updateDeviceInfoLocked(); + } + }); + } + private void setDisplayBrightness(float brightnessState, float sdrBrightnessState) { // brightnessState includes invalid, off and full range. @@ -1108,6 +1122,7 @@ final class LocalDisplayAdapter extends DisplayAdapter { pw.println("mDefaultModeId=" + mDefaultModeId); pw.println("mUserPreferredModeId=" + mUserPreferredModeId); pw.println("mState=" + Display.stateToString(mState)); + pw.println("mCommittedState=" + Display.stateToString(mCommittedState)); pw.println("mBrightnessState=" + mBrightnessState); pw.println("mBacklightAdapter=" + mBacklightAdapter); pw.println("mAllmSupported=" + mAllmSupported); diff --git a/services/core/java/com/android/server/display/LogicalDisplay.java b/services/core/java/com/android/server/display/LogicalDisplay.java index 7dc412ed1cf8..178278e7fd2f 100644 --- a/services/core/java/com/android/server/display/LogicalDisplay.java +++ b/services/core/java/com/android/server/display/LogicalDisplay.java @@ -390,6 +390,7 @@ final class LogicalDisplay { mBaseDisplayInfo.appVsyncOffsetNanos = deviceInfo.appVsyncOffsetNanos; mBaseDisplayInfo.presentationDeadlineNanos = deviceInfo.presentationDeadlineNanos; mBaseDisplayInfo.state = deviceInfo.state; + mBaseDisplayInfo.committedState = deviceInfo.committedState; mBaseDisplayInfo.smallestNominalAppWidth = maskedWidth; mBaseDisplayInfo.smallestNominalAppHeight = maskedHeight; mBaseDisplayInfo.largestNominalAppWidth = maskedWidth; diff --git a/services/tests/mockingservicestests/src/com/android/server/display/LocalDisplayAdapterTest.java b/services/tests/mockingservicestests/src/com/android/server/display/LocalDisplayAdapterTest.java index 82236bfd98e0..5f67b6e0c79d 100644 --- a/services/tests/mockingservicestests/src/com/android/server/display/LocalDisplayAdapterTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/display/LocalDisplayAdapterTest.java @@ -575,6 +575,40 @@ public class LocalDisplayAdapterTest { assertThat(displayDeviceInfo.allmSupported).isFalse(); } + @Test + public void testAfterDisplayStateChanges_committedSetAfterState() throws Exception { + FakeDisplay display = new FakeDisplay(PORT_A); + setUpDisplay(display); + updateAvailableDisplays(); + mAdapter.registerLocked(); + waitForHandlerToComplete(mHandler, HANDLER_WAIT_MS); + assertThat(mListener.addedDisplays.size()).isEqualTo(1); + DisplayDevice displayDevice = mListener.addedDisplays.get(0); + + // Turn off. + Runnable changeStateRunnable = displayDevice.requestDisplayStateLocked(Display.STATE_OFF, 0, + 0); + waitForHandlerToComplete(mHandler, HANDLER_WAIT_MS); + assertThat(mListener.changedDisplays.size()).isEqualTo(1); + mListener.changedDisplays.clear(); + assertThat(displayDevice.getDisplayDeviceInfoLocked().state).isEqualTo(Display.STATE_OFF); + assertThat(displayDevice.getDisplayDeviceInfoLocked().committedState).isNotEqualTo( + Display.STATE_OFF); + verify(mSurfaceControlProxy, never()).setDisplayPowerMode(display.token, Display.STATE_OFF); + + // Execute powerstate change. + changeStateRunnable.run(); + waitForHandlerToComplete(mHandler, HANDLER_WAIT_MS); + + + // Verify that committed triggered a new change event and is set correctly. + verify(mSurfaceControlProxy, never()).setDisplayPowerMode(display.token, Display.STATE_OFF); + assertThat(mListener.changedDisplays.size()).isEqualTo(1); + assertThat(displayDevice.getDisplayDeviceInfoLocked().state).isEqualTo(Display.STATE_OFF); + assertThat(displayDevice.getDisplayDeviceInfoLocked().committedState).isEqualTo( + Display.STATE_OFF); + } + @Test public void testAfterDisplayChange_GameContentTypeSupportIsUpdated() throws Exception { FakeDisplay display = new FakeDisplay(PORT_A); -- GitLab