From 09b6e32c1d812bbbf869826ea2cfecdd53d4b6bb Mon Sep 17 00:00:00 2001 From: Marin Shalamanov Date: Tue, 23 Feb 2021 15:39:43 +0100 Subject: [PATCH] DisplayModeDirector: Prefer the default mode if available We should prefer the default display mode if it's available. Currently we select the default only if there are no available modes after filtering, or if the default happens to be the first on the list of filtered modes. This fixes a bug where a TV with default display mode of 60Hz boots in 24 Hz. Fixes: 181010496 Test: atest DisplayModeDirectorTest Change-Id: I06a7133ea23c36443f48903eccb6623da7fd93ea --- .../server/display/DisplayModeDirector.java | 30 +++++-- .../display/DisplayModeDirectorTest.java | 90 +++++++++++-------- 2 files changed, 77 insertions(+), 43 deletions(-) diff --git a/services/core/java/com/android/server/display/DisplayModeDirector.java b/services/core/java/com/android/server/display/DisplayModeDirector.java index dce6bd849953f..645ca7ac33e04 100644 --- a/services/core/java/com/android/server/display/DisplayModeDirector.java +++ b/services/core/java/com/android/server/display/DisplayModeDirector.java @@ -31,7 +31,6 @@ import android.net.Uri; import android.os.Handler; import android.os.Looper; import android.os.Message; -import android.os.PowerManager; import android.os.SystemClock; import android.os.UserHandle; import android.provider.DeviceConfig; @@ -80,6 +79,8 @@ public class DisplayModeDirector { // specific display. private static final int GLOBAL_ID = -1; + private static final int INVALID_DISPLAY_MODE_ID = -1; + // The tolerance within which we consider something approximately equals. private static final float FLOAT_TOLERANCE = 0.01f; @@ -322,12 +323,30 @@ public class DisplayModeDirector { appRequestSummary.maxRefreshRate)); } - // If the application requests a given mode with preferredModeId function, it will be - // stored as baseModeId. - int baseModeId = defaultMode.getModeId(); - if (availableModes.length > 0) { + int baseModeId = INVALID_DISPLAY_MODE_ID; + + // Select the default mode if available. This is important because SurfaceFlinger + // can do only seamless switches by default. Some devices (e.g. TV) don't support + // seamless switching so the mode we select here won't be changed. + for (int availableMode : availableModes) { + if (availableMode == defaultMode.getModeId()) { + baseModeId = defaultMode.getModeId(); + break; + } + } + + // If the application requests a display mode by setting + // LayoutParams.preferredDisplayModeId, it will be the only available mode and it'll + // be stored as baseModeId. + if (baseModeId == INVALID_DISPLAY_MODE_ID && availableModes.length > 0) { baseModeId = availableModes[0]; } + + if (baseModeId == INVALID_DISPLAY_MODE_ID) { + throw new IllegalStateException("Can't select a base display mode for display " + + displayId + ". The votes are " + mVotesByDisplay.valueAt(displayId)); + } + if (mModeSwitchingType == DisplayManager.SWITCHING_TYPE_NONE) { Display.Mode baseMode = null; for (Display.Mode mode : modes) { @@ -351,6 +370,7 @@ public class DisplayModeDirector { boolean allowGroupSwitching = mModeSwitchingType == DisplayManager.SWITCHING_TYPE_ACROSS_AND_WITHIN_GROUPS; + return new DesiredDisplayModeSpecs(baseModeId, allowGroupSwitching, new RefreshRateRange( 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 81b2381cd629d..15ada896512b4 100644 --- a/services/tests/servicestests/src/com/android/server/display/DisplayModeDirectorTest.java +++ b/services/tests/servicestests/src/com/android/server/display/DisplayModeDirectorTest.java @@ -88,6 +88,7 @@ public class DisplayModeDirectorTest { private static final String TAG = "DisplayModeDirectorTest"; private static final boolean DEBUG = false; private static final float FLOAT_TOLERANCE = 0.01f; + private static final int DISPLAY_ID = 0; private Context mContext; private FakesInjector mInjector; @@ -107,19 +108,29 @@ public class DisplayModeDirectorTest { private DisplayModeDirector createDirectorFromRefreshRateArray( float[] refreshRates, int baseModeId) { + return createDirectorFromRefreshRateArray(refreshRates, baseModeId, refreshRates[0]); + } + + private DisplayModeDirector createDirectorFromRefreshRateArray( + float[] refreshRates, int baseModeId, float defaultRefreshRate) { DisplayModeDirector director = new DisplayModeDirector(mContext, mHandler, mInjector); - int displayId = 0; Display.Mode[] modes = new Display.Mode[refreshRates.length]; + Display.Mode defaultMode = null; for (int i = 0; i < refreshRates.length; i++) { modes[i] = new Display.Mode( /*modeId=*/baseModeId + i, /*width=*/1000, /*height=*/1000, refreshRates[i]); + if (refreshRates[i] == defaultRefreshRate) { + defaultMode = modes[i]; + } } + assertThat(defaultMode).isNotNull(); + SparseArray supportedModesByDisplay = new SparseArray<>(); - supportedModesByDisplay.put(displayId, modes); + supportedModesByDisplay.put(DISPLAY_ID, modes); director.injectSupportedModesByDisplay(supportedModesByDisplay); SparseArray defaultModesByDisplay = new SparseArray<>(); - defaultModesByDisplay.put(displayId, modes[0]); + defaultModesByDisplay.put(DISPLAY_ID, defaultMode); director.injectDefaultModeByDisplay(defaultModesByDisplay); return director; } @@ -130,16 +141,15 @@ public class DisplayModeDirectorTest { for (int i = 0; i < numRefreshRates; i++) { refreshRates[i] = minFps + i; } - return createDirectorFromRefreshRateArray(refreshRates, /*baseModeId=*/minFps); + return createDirectorFromRefreshRateArray(refreshRates, /*baseModeId=*/minFps, + /*defaultRefreshRate=*/minFps); } @Test public void testDisplayModeVoting() { - int displayId = 0; - // With no votes present, DisplayModeDirector should allow any refresh rate. DesiredDisplayModeSpecs modeSpecs = - createDirectorFromFpsRange(60, 90).getDesiredDisplayModeSpecs(displayId); + createDirectorFromFpsRange(60, 90).getDesiredDisplayModeSpecs(DISPLAY_ID); assertThat(modeSpecs.baseModeId).isEqualTo(60); assertThat(modeSpecs.primaryRefreshRateRange.min).isEqualTo(0f); assertThat(modeSpecs.primaryRefreshRateRange.max).isEqualTo(Float.POSITIVE_INFINITY); @@ -156,12 +166,12 @@ public class DisplayModeDirectorTest { assertTrue(2 * numPriorities < maxFps - minFps + 1); SparseArray votes = new SparseArray<>(); SparseArray> votesByDisplay = new SparseArray<>(); - votesByDisplay.put(displayId, votes); + votesByDisplay.put(DISPLAY_ID, votes); for (int i = 0; i < numPriorities; i++) { int priority = Vote.MIN_PRIORITY + i; votes.put(priority, Vote.forRefreshRates(minFps + i, maxFps - i)); director.injectVotesByDisplay(votesByDisplay); - modeSpecs = director.getDesiredDisplayModeSpecs(displayId); + modeSpecs = director.getDesiredDisplayModeSpecs(DISPLAY_ID); assertThat(modeSpecs.baseModeId).isEqualTo(minFps + i); assertThat(modeSpecs.primaryRefreshRateRange.min) .isEqualTo((float) (minFps + i)); @@ -177,11 +187,11 @@ public class DisplayModeDirectorTest { DisplayModeDirector director = createDirectorFromFpsRange(60, 90); SparseArray votes = new SparseArray<>(); SparseArray> votesByDisplay = new SparseArray<>(); - votesByDisplay.put(displayId, votes); + votesByDisplay.put(DISPLAY_ID, votes); votes.put(Vote.MAX_PRIORITY, Vote.forRefreshRates(65, 85)); votes.put(Vote.MIN_PRIORITY, Vote.forRefreshRates(70, 80)); director.injectVotesByDisplay(votesByDisplay); - modeSpecs = director.getDesiredDisplayModeSpecs(displayId); + modeSpecs = director.getDesiredDisplayModeSpecs(DISPLAY_ID); assertThat(modeSpecs.baseModeId).isEqualTo(70); assertThat(modeSpecs.primaryRefreshRateRange.min).isEqualTo(70f); assertThat(modeSpecs.primaryRefreshRateRange.max).isEqualTo(80f); @@ -190,18 +200,17 @@ public class DisplayModeDirectorTest { @Test public void testVotingWithFloatingPointErrors() { - int displayId = 0; DisplayModeDirector director = createDirectorFromFpsRange(50, 90); SparseArray votes = new SparseArray<>(); SparseArray> votesByDisplay = new SparseArray<>(); - votesByDisplay.put(displayId, votes); + votesByDisplay.put(DISPLAY_ID, votes); float error = FLOAT_TOLERANCE / 4; votes.put(Vote.PRIORITY_USER_SETTING_PEAK_REFRESH_RATE, Vote.forRefreshRates(0, 60)); votes.put(Vote.PRIORITY_APP_REQUEST_SIZE, Vote.forRefreshRates(60 + error, 60 + error)); votes.put(Vote.PRIORITY_APP_REQUEST_REFRESH_RATE, Vote.forRefreshRates(60 - error, 60 - error)); director.injectVotesByDisplay(votesByDisplay); - DesiredDisplayModeSpecs desiredSpecs = director.getDesiredDisplayModeSpecs(displayId); + DesiredDisplayModeSpecs desiredSpecs = director.getDesiredDisplayModeSpecs(DISPLAY_ID); assertThat(desiredSpecs.primaryRefreshRateRange.min).isWithin(FLOAT_TOLERANCE).of(60); assertThat(desiredSpecs.primaryRefreshRateRange.max).isWithin(FLOAT_TOLERANCE).of(60); @@ -213,15 +222,14 @@ public class DisplayModeDirectorTest { assertTrue(PRIORITY_FLICKER < Vote.PRIORITY_APP_REQUEST_REFRESH_RATE); assertTrue(PRIORITY_FLICKER < Vote.PRIORITY_APP_REQUEST_SIZE); - int displayId = 0; DisplayModeDirector director = createDirectorFromFpsRange(60, 90); SparseArray votes = new SparseArray<>(); SparseArray> votesByDisplay = new SparseArray<>(); - votesByDisplay.put(displayId, votes); + votesByDisplay.put(DISPLAY_ID, votes); votes.put(Vote.PRIORITY_APP_REQUEST_REFRESH_RATE, Vote.forRefreshRates(60, 90)); votes.put(PRIORITY_FLICKER, Vote.forRefreshRates(60, 60)); director.injectVotesByDisplay(votesByDisplay); - DesiredDisplayModeSpecs desiredSpecs = director.getDesiredDisplayModeSpecs(displayId); + DesiredDisplayModeSpecs desiredSpecs = director.getDesiredDisplayModeSpecs(DISPLAY_ID); assertThat(desiredSpecs.primaryRefreshRateRange.min).isWithin(FLOAT_TOLERANCE).of(60); assertThat(desiredSpecs.primaryRefreshRateRange.max).isWithin(FLOAT_TOLERANCE).of(60); @@ -229,7 +237,7 @@ public class DisplayModeDirectorTest { votes.put(Vote.PRIORITY_APP_REQUEST_REFRESH_RATE, Vote.forRefreshRates(60, 90)); votes.put(PRIORITY_FLICKER, Vote.forRefreshRates(90, 90)); director.injectVotesByDisplay(votesByDisplay); - desiredSpecs = director.getDesiredDisplayModeSpecs(displayId); + desiredSpecs = director.getDesiredDisplayModeSpecs(DISPLAY_ID); assertThat(desiredSpecs.primaryRefreshRateRange.min).isWithin(FLOAT_TOLERANCE).of(90); assertThat(desiredSpecs.primaryRefreshRateRange.max).isWithin(FLOAT_TOLERANCE).of(90); @@ -237,7 +245,7 @@ public class DisplayModeDirectorTest { votes.put(Vote.PRIORITY_APP_REQUEST_REFRESH_RATE, Vote.forRefreshRates(90, 90)); votes.put(PRIORITY_FLICKER, Vote.forRefreshRates(60, 60)); director.injectVotesByDisplay(votesByDisplay); - desiredSpecs = director.getDesiredDisplayModeSpecs(displayId); + desiredSpecs = director.getDesiredDisplayModeSpecs(DISPLAY_ID); assertThat(desiredSpecs.primaryRefreshRateRange.min).isWithin(FLOAT_TOLERANCE).of(90); assertThat(desiredSpecs.primaryRefreshRateRange.max).isWithin(FLOAT_TOLERANCE).of(90); @@ -245,7 +253,7 @@ public class DisplayModeDirectorTest { votes.put(Vote.PRIORITY_APP_REQUEST_REFRESH_RATE, Vote.forRefreshRates(60, 60)); votes.put(PRIORITY_FLICKER, Vote.forRefreshRates(90, 90)); director.injectVotesByDisplay(votesByDisplay); - desiredSpecs = director.getDesiredDisplayModeSpecs(displayId); + desiredSpecs = director.getDesiredDisplayModeSpecs(DISPLAY_ID); assertThat(desiredSpecs.primaryRefreshRateRange.min).isWithin(FLOAT_TOLERANCE).of(60); assertThat(desiredSpecs.primaryRefreshRateRange.max).isWithin(FLOAT_TOLERANCE).of(60); } @@ -261,14 +269,13 @@ public class DisplayModeDirectorTest { assertTrue(Vote.PRIORITY_APP_REQUEST_REFRESH_RATE >= Vote.APP_REQUEST_REFRESH_RATE_RANGE_PRIORITY_CUTOFF); - int displayId = 0; DisplayModeDirector director = createDirectorFromFpsRange(60, 90); SparseArray votes = new SparseArray<>(); SparseArray> votesByDisplay = new SparseArray<>(); - votesByDisplay.put(displayId, votes); + votesByDisplay.put(DISPLAY_ID, votes); votes.put(PRIORITY_FLICKER, Vote.forRefreshRates(60, 60)); director.injectVotesByDisplay(votesByDisplay); - DesiredDisplayModeSpecs desiredSpecs = director.getDesiredDisplayModeSpecs(displayId); + DesiredDisplayModeSpecs desiredSpecs = director.getDesiredDisplayModeSpecs(DISPLAY_ID); assertThat(desiredSpecs.primaryRefreshRateRange.min).isWithin(FLOAT_TOLERANCE).of(60); assertThat(desiredSpecs.primaryRefreshRateRange.max).isWithin(FLOAT_TOLERANCE).of(60); assertThat(desiredSpecs.appRequestRefreshRateRange.min).isAtMost(60f); @@ -277,7 +284,7 @@ public class DisplayModeDirectorTest { votes.put(Vote.PRIORITY_USER_SETTING_MIN_REFRESH_RATE, Vote.forRefreshRates(90, Float.POSITIVE_INFINITY)); director.injectVotesByDisplay(votesByDisplay); - desiredSpecs = director.getDesiredDisplayModeSpecs(displayId); + desiredSpecs = director.getDesiredDisplayModeSpecs(DISPLAY_ID); assertThat(desiredSpecs.primaryRefreshRateRange.min).isWithin(FLOAT_TOLERANCE).of(90); assertThat(desiredSpecs.primaryRefreshRateRange.max).isAtLeast(90f); assertThat(desiredSpecs.appRequestRefreshRateRange.min).isAtMost(60f); @@ -285,7 +292,7 @@ public class DisplayModeDirectorTest { votes.put(Vote.PRIORITY_APP_REQUEST_REFRESH_RATE, Vote.forRefreshRates(75, 75)); director.injectVotesByDisplay(votesByDisplay); - desiredSpecs = director.getDesiredDisplayModeSpecs(displayId); + desiredSpecs = director.getDesiredDisplayModeSpecs(DISPLAY_ID); assertThat(desiredSpecs.primaryRefreshRateRange.min).isWithin(FLOAT_TOLERANCE).of(75); assertThat(desiredSpecs.primaryRefreshRateRange.max).isWithin(FLOAT_TOLERANCE).of(75); assertThat(desiredSpecs.appRequestRefreshRateRange.min) @@ -355,11 +362,10 @@ public class DisplayModeDirectorTest { @Test public void testVotingWithAlwaysRespectAppRequest() { - final int displayId = 0; DisplayModeDirector director = createDirectorFromFpsRange(50, 90); SparseArray votes = new SparseArray<>(); SparseArray> votesByDisplay = new SparseArray<>(); - votesByDisplay.put(displayId, votes); + votesByDisplay.put(DISPLAY_ID, votes); votes.put(PRIORITY_FLICKER, Vote.forRefreshRates(0, 60)); votes.put(Vote.PRIORITY_USER_SETTING_MIN_REFRESH_RATE, Vote.forRefreshRates(60, 90)); votes.put(Vote.PRIORITY_APP_REQUEST_REFRESH_RATE, Vote.forRefreshRates(90, 90)); @@ -369,7 +375,7 @@ public class DisplayModeDirectorTest { director.injectVotesByDisplay(votesByDisplay); assertThat(director.shouldAlwaysRespectAppRequestedMode()).isFalse(); - DesiredDisplayModeSpecs desiredSpecs = director.getDesiredDisplayModeSpecs(displayId); + DesiredDisplayModeSpecs desiredSpecs = director.getDesiredDisplayModeSpecs(DISPLAY_ID); assertThat(desiredSpecs.primaryRefreshRateRange.min).isWithin(FLOAT_TOLERANCE).of(60); assertThat(desiredSpecs.primaryRefreshRateRange.max).isWithin(FLOAT_TOLERANCE).of(60); @@ -377,7 +383,7 @@ public class DisplayModeDirectorTest { director.setShouldAlwaysRespectAppRequestedMode(true); assertThat(director.shouldAlwaysRespectAppRequestedMode()).isTrue(); - desiredSpecs = director.getDesiredDisplayModeSpecs(displayId); + desiredSpecs = director.getDesiredDisplayModeSpecs(DISPLAY_ID); assertThat(desiredSpecs.primaryRefreshRateRange.min).isWithin(FLOAT_TOLERANCE).of(90); assertThat(desiredSpecs.primaryRefreshRateRange.max).isWithin(FLOAT_TOLERANCE).of(90); assertThat(desiredSpecs.baseModeId).isEqualTo(90); @@ -385,7 +391,7 @@ public class DisplayModeDirectorTest { director.setShouldAlwaysRespectAppRequestedMode(false); assertThat(director.shouldAlwaysRespectAppRequestedMode()).isFalse(); - desiredSpecs = director.getDesiredDisplayModeSpecs(displayId); + desiredSpecs = director.getDesiredDisplayModeSpecs(DISPLAY_ID); assertThat(desiredSpecs.primaryRefreshRateRange.min).isWithin(FLOAT_TOLERANCE).of(60); assertThat(desiredSpecs.primaryRefreshRateRange.max).isWithin(FLOAT_TOLERANCE).of(60); assertThat(desiredSpecs.baseModeId).isEqualTo(60); @@ -393,11 +399,10 @@ public class DisplayModeDirectorTest { @Test public void testVotingWithSwitchingTypeNone() { - final int displayId = 0; DisplayModeDirector director = createDirectorFromFpsRange(0, 90); SparseArray votes = new SparseArray<>(); SparseArray> votesByDisplay = new SparseArray<>(); - votesByDisplay.put(displayId, votes); + votesByDisplay.put(DISPLAY_ID, votes); votes.put(Vote.PRIORITY_USER_SETTING_MIN_REFRESH_RATE, Vote.forRefreshRates(30, 90)); votes.put(Vote.PRIORITY_LOW_POWER_MODE, Vote.forRefreshRates(0, 60)); @@ -405,7 +410,7 @@ public class DisplayModeDirectorTest { director.injectVotesByDisplay(votesByDisplay); assertThat(director.getModeSwitchingType()) .isNotEqualTo(DisplayManager.SWITCHING_TYPE_NONE); - DesiredDisplayModeSpecs desiredSpecs = director.getDesiredDisplayModeSpecs(displayId); + DesiredDisplayModeSpecs desiredSpecs = director.getDesiredDisplayModeSpecs(DISPLAY_ID); assertThat(desiredSpecs.primaryRefreshRateRange.min).isWithin(FLOAT_TOLERANCE).of(30); assertThat(desiredSpecs.primaryRefreshRateRange.max).isWithin(FLOAT_TOLERANCE).of(60); @@ -417,7 +422,7 @@ public class DisplayModeDirectorTest { assertThat(director.getModeSwitchingType()) .isEqualTo(DisplayManager.SWITCHING_TYPE_NONE); - desiredSpecs = director.getDesiredDisplayModeSpecs(displayId); + desiredSpecs = director.getDesiredDisplayModeSpecs(DISPLAY_ID); assertThat(desiredSpecs.primaryRefreshRateRange.min).isWithin(FLOAT_TOLERANCE).of(30); assertThat(desiredSpecs.primaryRefreshRateRange.max).isWithin(FLOAT_TOLERANCE).of(30); assertThat(desiredSpecs.appRequestRefreshRateRange.min).isWithin(FLOAT_TOLERANCE).of(30); @@ -427,28 +432,37 @@ public class DisplayModeDirectorTest { @Test public void testVotingWithSwitchingTypeWithinGroups() { - final int displayId = 0; DisplayModeDirector director = createDirectorFromFpsRange(0, 90); director.setModeSwitchingType(DisplayManager.SWITCHING_TYPE_WITHIN_GROUPS); assertThat(director.getModeSwitchingType()) .isEqualTo(DisplayManager.SWITCHING_TYPE_WITHIN_GROUPS); - DesiredDisplayModeSpecs desiredSpecs = director.getDesiredDisplayModeSpecs(displayId); + DesiredDisplayModeSpecs desiredSpecs = director.getDesiredDisplayModeSpecs(DISPLAY_ID); assertThat(desiredSpecs.allowGroupSwitching).isFalse(); } @Test public void testVotingWithSwitchingTypeWithinAndAcrossGroups() { - final int displayId = 0; DisplayModeDirector director = createDirectorFromFpsRange(0, 90); director.setModeSwitchingType(DisplayManager.SWITCHING_TYPE_ACROSS_AND_WITHIN_GROUPS); assertThat(director.getModeSwitchingType()) .isEqualTo(DisplayManager.SWITCHING_TYPE_ACROSS_AND_WITHIN_GROUPS); - DesiredDisplayModeSpecs desiredSpecs = director.getDesiredDisplayModeSpecs(displayId); + DesiredDisplayModeSpecs desiredSpecs = director.getDesiredDisplayModeSpecs(DISPLAY_ID); assertThat(desiredSpecs.allowGroupSwitching).isTrue(); } + @Test + public void testDefaultDisplayModeIsSelectedIfAvailable() { + final float[] refreshRates = new float[]{24f, 25f, 30f, 60f, 90f}; + final int defaultModeId = 3; + DisplayModeDirector director = createDirectorFromRefreshRateArray( + refreshRates, /*baseModeId=*/0, refreshRates[defaultModeId]); + + DesiredDisplayModeSpecs specs = director.getDesiredDisplayModeSpecs(DISPLAY_ID); + assertThat(specs.baseModeId).isEqualTo(defaultModeId); + } + @Test public void testBrightnessObserverGetsUpdatedRefreshRatesForZone() { DisplayModeDirector director =