From 71b626759930963ff2a272dcc6ab732a874e1864 Mon Sep 17 00:00:00 2001 From: Ady Abraham Date: Mon, 7 Jun 2021 20:22:41 -0700 Subject: [PATCH 1/2] DisplayManagerGlobal: suppress redundant AChoreographer_refreshRateCallback calls With Frame Rate Override enabled, an app could get an AChoreographer_refreshRateCallback callbacks which seem redundant. An example would be: 1. Display changed refresh rate to 60Hz - callback 2. App's frame rate override changed to 60Hz - callback To avoid sending these redundant callbacks, DisplayManagerGlobal caches the last DisplayInfo that was reported to the app and based on it, it decides whether to send the event or not. Bug: 184588343 Bug: 189174620 Test: atest SetFrameRateTest Test: atest FrameRateOverrideHostTest Test: atest ChoreographerNativeTest Change-Id: Ia0278e98d06bc790810c31dcda8244181e8e1d5d --- .../hardware/display/DisplayManagerGlobal.java | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/core/java/android/hardware/display/DisplayManagerGlobal.java b/core/java/android/hardware/display/DisplayManagerGlobal.java index df51734bc17d5..a9b95fce87778 100644 --- a/core/java/android/hardware/display/DisplayManagerGlobal.java +++ b/core/java/android/hardware/display/DisplayManagerGlobal.java @@ -94,6 +94,7 @@ public final class DisplayManagerGlobal { // Guarded by mLock private boolean mDispatchNativeCallbacks = false; + private float mNativeCallbackReportedRefreshRate; private final Object mLock = new Object(); @UnsupportedAppUsage @@ -404,10 +405,11 @@ public final class DisplayManagerGlobal { // We can likely save a binder hop if we attach the refresh rate onto the // listener. DisplayInfo display = getDisplayInfoLocked(displayId); - if (display != null) { - float refreshRate = display.getRefreshRate(); + if (display != null + && mNativeCallbackReportedRefreshRate != display.getRefreshRate()) { + mNativeCallbackReportedRefreshRate = display.getRefreshRate(); // Signal native callbacks if we ever set a refresh rate. - nSignalNativeCallbacks(refreshRate); + nSignalNativeCallbacks(mNativeCallbackReportedRefreshRate); } } } @@ -1055,8 +1057,8 @@ public final class DisplayManagerGlobal { if (display != null) { // We need to tell AChoreographer instances the current refresh rate so that apps // can get it for free once a callback first registers. - float refreshRate = display.getRefreshRate(); - nSignalNativeCallbacks(refreshRate); + mNativeCallbackReportedRefreshRate = display.getRefreshRate(); + nSignalNativeCallbacks(mNativeCallbackReportedRefreshRate); } } } From 00eb127ef09988174616d82513e61a7a934c9e2f Mon Sep 17 00:00:00 2001 From: Ady Abraham Date: Mon, 7 Jun 2021 20:25:08 -0700 Subject: [PATCH 2/2] Don't call setFrameRate if refresh rate switching is disabled When refresh rate switching is set to SWITCHING_TYPE_NONE, DisplayManager limits the refresh rate to a single value. Calling setFrameRate to SF is redundant and causes unnecessary duplicate events. Bug: 189174620 Bug: 189191861 Test: atest SetFrameRateTest Test: atest FrameRateOverrideHostTest Test: atest ChoreographerNativeTest Test: atest FrameRateSelectionPriorityTests Change-Id: I77ca430555c57c13a6e36707b344537deee5c2d7 --- .../display/DisplayManagerInternal.java | 6 +++ .../server/display/DisplayManagerService.java | 5 ++ .../com/android/server/wm/WindowState.java | 19 ++++--- .../wm/FrameRateSelectionPriorityTests.java | 51 ++++++++++++++++--- 4 files changed, 67 insertions(+), 14 deletions(-) diff --git a/core/java/android/hardware/display/DisplayManagerInternal.java b/core/java/android/hardware/display/DisplayManagerInternal.java index dce3fefff285e..c739c6a53167b 100644 --- a/core/java/android/hardware/display/DisplayManagerInternal.java +++ b/core/java/android/hardware/display/DisplayManagerInternal.java @@ -286,6 +286,12 @@ public abstract class DisplayManagerInternal { */ public abstract void ignoreProximitySensorUntilChanged(); + /** + * Returns the refresh rate switching type. + */ + @DisplayManager.SwitchingType + public abstract int getRefreshRateSwitchingType(); + /** * Describes the requested power state of the display. * diff --git a/services/core/java/com/android/server/display/DisplayManagerService.java b/services/core/java/com/android/server/display/DisplayManagerService.java index d4920f56a27e2..13b7ebc47301c 100644 --- a/services/core/java/com/android/server/display/DisplayManagerService.java +++ b/services/core/java/com/android/server/display/DisplayManagerService.java @@ -3250,6 +3250,11 @@ public final class DisplayManagerService extends SystemService { mDisplayPowerControllers.get(Display.DEFAULT_DISPLAY) .ignoreProximitySensorUntilChanged(); } + + @Override + public int getRefreshRateSwitchingType() { + return getRefreshRateSwitchingTypeInternal(); + } } class DesiredDisplayModeSpecsObserver diff --git a/services/core/java/com/android/server/wm/WindowState.java b/services/core/java/com/android/server/wm/WindowState.java index 299cde8ad1d8c..03762b3ee8bf2 100644 --- a/services/core/java/com/android/server/wm/WindowState.java +++ b/services/core/java/com/android/server/wm/WindowState.java @@ -25,6 +25,7 @@ import static android.app.WindowConfiguration.ACTIVITY_TYPE_HOME; import static android.app.WindowConfiguration.isSplitScreenWindowingMode; import static android.content.res.Configuration.ORIENTATION_LANDSCAPE; import static android.graphics.GraphicsProtos.dumpPointProto; +import static android.hardware.display.DisplayManager.SWITCHING_TYPE_NONE; import static android.hardware.input.InputManager.BLOCK_UNTRUSTED_TOUCHES; import static android.os.InputConstants.DEFAULT_DISPATCHING_TIMEOUT_MILLIS; import static android.os.PowerManager.DRAW_WAKE_LOCK; @@ -5448,12 +5449,18 @@ class WindowState extends WindowContainer implements WindowManagerP mFrameRateSelectionPriority); } - final float refreshRate = refreshRatePolicy.getPreferredRefreshRate(this); - if (mAppPreferredFrameRate != refreshRate) { - mAppPreferredFrameRate = refreshRate; - getPendingTransaction().setFrameRate( - mSurfaceControl, mAppPreferredFrameRate, - Surface.FRAME_RATE_COMPATIBILITY_EXACT, Surface.CHANGE_FRAME_RATE_ALWAYS); + // If refresh rate switching is disabled there is no point to set the frame rate on the + // surface as the refresh rate will be limited by display manager to a single value + // and SurfaceFlinger wouldn't be able to change it anyways. + if (mWmService.mDisplayManagerInternal.getRefreshRateSwitchingType() + != SWITCHING_TYPE_NONE) { + final float refreshRate = refreshRatePolicy.getPreferredRefreshRate(this); + if (mAppPreferredFrameRate != refreshRate) { + mAppPreferredFrameRate = refreshRate; + getPendingTransaction().setFrameRate( + mSurfaceControl, mAppPreferredFrameRate, + Surface.FRAME_RATE_COMPATIBILITY_EXACT, Surface.CHANGE_FRAME_RATE_ALWAYS); + } } } diff --git a/services/tests/wmtests/src/com/android/server/wm/FrameRateSelectionPriorityTests.java b/services/tests/wmtests/src/com/android/server/wm/FrameRateSelectionPriorityTests.java index 1ee646c57b14e..e6348a50566c9 100644 --- a/services/tests/wmtests/src/com/android/server/wm/FrameRateSelectionPriorityTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/FrameRateSelectionPriorityTests.java @@ -29,6 +29,7 @@ import static org.junit.Assert.assertNotNull; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import android.hardware.display.DisplayManager; import android.platform.test.annotations.Presubmit; import android.view.Display.Mode; import android.view.DisplayInfo; @@ -56,6 +57,13 @@ public class FrameRateSelectionPriorityTests extends WindowTestsBase { private RefreshRatePolicy mRefreshRatePolicy; private HighRefreshRateDenylist mDenylist = mock(HighRefreshRateDenylist.class); + WindowState createWindow(String name) { + WindowState window = createWindow(null, TYPE_APPLICATION, name); + when(window.mWmService.mDisplayManagerInternal.getRefreshRateSwitchingType()) + .thenReturn(DisplayManager.SWITCHING_TYPE_WITHIN_GROUPS); + return window; + } + @Before public void setUp() { DisplayInfo di = new DisplayInfo(mDisplayInfo); @@ -73,7 +81,7 @@ public class FrameRateSelectionPriorityTests extends WindowTestsBase { @Test public void basicTest() { - final WindowState appWindow = createWindow(null, TYPE_APPLICATION, "appWindow"); + final WindowState appWindow = createWindow("appWindow"); assertNotNull("Window state is created", appWindow); assertEquals(appWindow.mFrameRateSelectionPriority, RefreshRatePolicy.LAYER_PRIORITY_UNSET); @@ -97,7 +105,7 @@ public class FrameRateSelectionPriorityTests extends WindowTestsBase { @Test public void testApplicationInFocusWithoutModeId() { - final WindowState appWindow = createWindow(null, TYPE_APPLICATION, "appWindow"); + final WindowState appWindow = createWindow("appWindow"); assertEquals(appWindow.mFrameRateSelectionPriority, RefreshRatePolicy.LAYER_PRIORITY_UNSET); assertEquals(appWindow.getDisplayContent().getDisplayPolicy().getRefreshRatePolicy() .getPreferredModeId(appWindow), 0); @@ -128,7 +136,7 @@ public class FrameRateSelectionPriorityTests extends WindowTestsBase { @Test public void testApplicationInFocusWithModeId() { - final WindowState appWindow = createWindow(null, TYPE_APPLICATION, "appWindow"); + final WindowState appWindow = createWindow("appWindow"); assertEquals(appWindow.mFrameRateSelectionPriority, RefreshRatePolicy.LAYER_PRIORITY_UNSET); assertEquals(appWindow.mAppPreferredFrameRate, 0, FLOAT_TOLERANCE); @@ -165,11 +173,11 @@ public class FrameRateSelectionPriorityTests extends WindowTestsBase { @Test public void testApplicationNotInFocusWithModeId() { - final WindowState appWindow = createWindow(null, TYPE_APPLICATION, "appWindow"); + final WindowState appWindow = createWindow("appWindow"); assertEquals(appWindow.mFrameRateSelectionPriority, RefreshRatePolicy.LAYER_PRIORITY_UNSET); assertEquals(appWindow.mAppPreferredFrameRate, 0, FLOAT_TOLERANCE); - final WindowState inFocusWindow = createWindow(null, TYPE_APPLICATION, "inFocus"); + final WindowState inFocusWindow = createWindow("inFocus"); appWindow.mToken.mDisplayContent.mCurrentFocus = inFocusWindow; appWindow.updateFrameRateSelectionPriorityIfNeeded(); @@ -194,11 +202,11 @@ public class FrameRateSelectionPriorityTests extends WindowTestsBase { @Test public void testApplicationNotInFocusWithoutModeId() { - final WindowState appWindow = createWindow(null, TYPE_APPLICATION, "appWindow"); + final WindowState appWindow = createWindow("appWindow"); assertEquals(appWindow.mFrameRateSelectionPriority, RefreshRatePolicy.LAYER_PRIORITY_UNSET); assertEquals(appWindow.mAppPreferredFrameRate, 0, FLOAT_TOLERANCE); - final WindowState inFocusWindow = createWindow(null, TYPE_APPLICATION, "inFocus"); + final WindowState inFocusWindow = createWindow("inFocus"); appWindow.mToken.mDisplayContent.mCurrentFocus = inFocusWindow; appWindow.updateFrameRateSelectionPriorityIfNeeded(); @@ -221,7 +229,7 @@ public class FrameRateSelectionPriorityTests extends WindowTestsBase { @Test public void testPreferredRefreshRate() { - final WindowState appWindow = createWindow(null, TYPE_APPLICATION, "appWindow"); + final WindowState appWindow = createWindow("appWindow"); assertNotNull("Window state is created", appWindow); when(appWindow.getDisplayContent().getDisplayPolicy()).thenReturn(mDisplayPolicy); @@ -246,4 +254,31 @@ public class FrameRateSelectionPriorityTests extends WindowTestsBase { appWindow.getSurfaceControl(), 60, Surface.FRAME_RATE_COMPATIBILITY_EXACT, Surface.CHANGE_FRAME_RATE_ALWAYS); } + + @Test + public void testSwitchingTypeNone() { + final WindowState appWindow = createWindow("appWindow"); + when(appWindow.mWmService.mDisplayManagerInternal.getRefreshRateSwitchingType()) + .thenReturn(DisplayManager.SWITCHING_TYPE_NONE); + + assertEquals(appWindow.mFrameRateSelectionPriority, RefreshRatePolicy.LAYER_PRIORITY_UNSET); + assertEquals(appWindow.mAppPreferredFrameRate, 0, FLOAT_TOLERANCE); + + // Update the mode ID to a requested number. + appWindow.mAttrs.preferredDisplayModeId = 1; + appWindow.updateFrameRateSelectionPriorityIfNeeded(); + + assertEquals(appWindow.mAppPreferredFrameRate, 0, FLOAT_TOLERANCE); + + // Remove the mode ID request. + appWindow.mAttrs.preferredDisplayModeId = 0; + appWindow.updateFrameRateSelectionPriorityIfNeeded(); + + assertEquals(appWindow.mAppPreferredFrameRate, 0, FLOAT_TOLERANCE); + + verify(appWindow.getPendingTransaction()).setFrameRateSelectionPriority( + appWindow.getSurfaceControl(), RefreshRatePolicy.LAYER_PRIORITY_UNSET); + verify(appWindow.getPendingTransaction(), never()).setFrameRate( + any(SurfaceControl.class), anyInt(), anyInt(), anyInt()); + } }