From 734c1178140cb785eeb806dd53f1761ed0cae2d7 Mon Sep 17 00:00:00 2001 From: Jason Macnak Date: Wed, 9 Feb 2022 13:39:02 -0800 Subject: [PATCH] Map SurfaceFlinger display mode ids to Display.Mode ids Noticed while trying to switch Cuttlefish to HWComposer3 with ag/16624862. By default, Cuttlefish has a single display with a single config: `RanchuHwc: config: id: 0 w:720 h:1280 dpi-x:320 dpi-y:320 ...` In `LocalDeviceDisplay::updateDisplayModesLocked()`, the display config ids which come from HWComposer via SurfaceFlinger are mapped to DisplayRecord's Display.Mode's `mModeId` which are globally unique ids created from the atomic integer `DisplayAdapter.NEXT_DISPLAY_MODE_ID` (which defaults to 1). However, the preferred display config id (`mSystemPreferredModeId`) was not being mapped. This would lead to boot failures such as: ` AndroidRuntime: *** FATAL EXCEPTION IN SYSTEM PROCESS: android.anim AndroidRuntime: java.lang.IllegalStateException: Unable to locate mode 0 ` with ag/16774826 because `mDefaultModeId` was being set to `mSystemPreferredModeId` which was the unmapped `0` from HWComposer. (Perhaps the need for this change suggests there should be strongly typed integers to represent the different id spaces, `SurfaceFlingerModeId` vs `DisplayAdapaterModeId` or something) Bug: b/193240715 Test: lunch cf_x86_64_phone-userdebug && cherry-pick ag/16624862 && m && cvd start Change-Id: I8fc0fca6816e037196f51d256430fad480f3b0ff (cherry picked from commit 53a296d20f4765572190358d9c2982f1f97a2d3d) --- .../server/display/LocalDisplayAdapter.java | 73 +++++++++++-------- 1 file changed, 44 insertions(+), 29 deletions(-) diff --git a/services/core/java/com/android/server/display/LocalDisplayAdapter.java b/services/core/java/com/android/server/display/LocalDisplayAdapter.java index d233c5ee2ffca..2e80efb957d4e 100644 --- a/services/core/java/com/android/server/display/LocalDisplayAdapter.java +++ b/services/core/java/com/android/server/display/LocalDisplayAdapter.java @@ -255,10 +255,9 @@ final class LocalDisplayAdapter extends DisplayAdapter { public boolean updateDisplayPropertiesLocked(SurfaceControl.StaticDisplayInfo staticInfo, SurfaceControl.DynamicDisplayInfo dynamicInfo, SurfaceControl.DesiredDisplayModeSpecs modeSpecs) { - boolean changed = - updateSystemPreferredDisplayMode(dynamicInfo.preferredBootDisplayMode); - changed |= updateDisplayModesLocked( - dynamicInfo.supportedDisplayModes, dynamicInfo.activeDisplayModeId, modeSpecs); + boolean changed = updateDisplayModesLocked( + dynamicInfo.supportedDisplayModes, dynamicInfo.preferredBootDisplayMode, + dynamicInfo.activeDisplayModeId, modeSpecs); changed |= updateStaticInfo(staticInfo); changed |= updateColorModesLocked(dynamicInfo.supportedColorModes, dynamicInfo.activeColorMode); @@ -273,10 +272,12 @@ final class LocalDisplayAdapter extends DisplayAdapter { } public boolean updateDisplayModesLocked( - SurfaceControl.DisplayMode[] displayModes, int activeDisplayModeId, - SurfaceControl.DesiredDisplayModeSpecs modeSpecs) { + SurfaceControl.DisplayMode[] displayModes, int preferredSfDisplayModeId, + int activeSfDisplayModeId, SurfaceControl.DesiredDisplayModeSpecs modeSpecs) { mSfDisplayModes = Arrays.copyOf(displayModes, displayModes.length); - mActiveSfDisplayMode = getModeById(displayModes, activeDisplayModeId); + mActiveSfDisplayMode = getModeById(displayModes, activeSfDisplayModeId); + SurfaceControl.DisplayMode preferredSfDisplayMode = + getModeById(displayModes, preferredSfDisplayModeId); // Build an updated list of all existing modes. ArrayList records = new ArrayList<>(); @@ -335,6 +336,27 @@ final class LocalDisplayAdapter extends DisplayAdapter { } } + boolean preferredModeChanged = false; + + if (preferredSfDisplayModeId != INVALID_MODE_ID && preferredSfDisplayMode != null) { + DisplayModeRecord preferredRecord = null; + for (DisplayModeRecord record : records) { + if (record.hasMatchingMode(preferredSfDisplayMode)) { + preferredRecord = record; + break; + } + } + + if (preferredRecord != null) { + int preferredModeId = preferredRecord.mMode.getModeId(); + if (mSurfaceControlProxy.getBootDisplayModeSupport() + && mSystemPreferredModeId != preferredModeId) { + mSystemPreferredModeId = preferredModeId; + preferredModeChanged = true; + } + } + } + boolean activeModeChanged = false; // Check whether SurfaceFlinger or the display device changed the active mode out from @@ -373,7 +395,7 @@ final class LocalDisplayAdapter extends DisplayAdapter { boolean recordsChanged = records.size() != mSupportedModes.size() || modesAdded; // If the records haven't changed then we're done here. if (!recordsChanged) { - return activeModeChanged; + return activeModeChanged || preferredModeChanged; } mSupportedModes.clear(); @@ -386,14 +408,14 @@ final class LocalDisplayAdapter extends DisplayAdapter { mDefaultModeId = mSystemPreferredModeId != INVALID_MODE_ID ? mSystemPreferredModeId : activeRecord.mMode.getModeId(); mDefaultModeGroup = mSystemPreferredModeId != INVALID_MODE_ID - ? getModeById(mSfDisplayModes, mSystemPreferredModeId).group + ? preferredSfDisplayMode.group : mActiveSfDisplayMode.group; } else if (modesAdded && activeModeChanged) { Slog.d(TAG, "New display modes are added and the active mode has changed, " + "use active mode as default mode."); mDefaultModeId = activeRecord.mMode.getModeId(); mDefaultModeGroup = mActiveSfDisplayMode.group; - } else if (findDisplayModeIdLocked(mDefaultModeId, mDefaultModeGroup) < 0) { + } else if (findSfDisplayModeIdLocked(mDefaultModeId, mDefaultModeGroup) < 0) { Slog.w(TAG, "Default display mode no longer available, using currently" + " active mode as default."); mDefaultModeId = activeRecord.mMode.getModeId(); @@ -545,15 +567,6 @@ final class LocalDisplayAdapter extends DisplayAdapter { return true; } - private boolean updateSystemPreferredDisplayMode(int modeId) { - if (!mSurfaceControlProxy.getBootDisplayModeSupport() - || mSystemPreferredModeId == modeId) { - return false; - } - mSystemPreferredModeId = modeId; - return true; - } - private SurfaceControl.DisplayMode getModeById(SurfaceControl.DisplayMode[] supportedModes, int modeId) { for (SurfaceControl.DisplayMode mode : supportedModes) { @@ -887,8 +900,10 @@ final class LocalDisplayAdapter extends DisplayAdapter { if (mUserPreferredMode == null) { mSurfaceControlProxy.clearBootDisplayMode(getDisplayTokenLocked()); } else { + int preferredSfDisplayModeId = findSfDisplayModeIdLocked( + mUserPreferredMode.getModeId(), mDefaultModeGroup); mSurfaceControlProxy.setBootDisplayMode(getDisplayTokenLocked(), - mUserPreferredMode.getModeId()); + preferredSfDisplayModeId); } } @@ -923,9 +938,9 @@ final class LocalDisplayAdapter extends DisplayAdapter { // mode based on vendor requirements. // Note: We prefer the default mode group over the current one as this is the mode // group the vendor prefers. - int baseModeId = findDisplayModeIdLocked(displayModeSpecs.baseModeId, + int baseSfModeId = findSfDisplayModeIdLocked(displayModeSpecs.baseModeId, mDefaultModeGroup); - if (baseModeId < 0) { + if (baseSfModeId < 0) { // When a display is hotplugged, it's possible for a mode to be removed that was // previously valid. Because of the way display changes are propagated through the // framework, and the caching of the display mode specs in LogicalDisplay, it's @@ -943,7 +958,7 @@ final class LocalDisplayAdapter extends DisplayAdapter { getHandler().sendMessage(PooledLambda.obtainMessage( LocalDisplayDevice::setDesiredDisplayModeSpecsAsync, this, getDisplayTokenLocked(), - new SurfaceControl.DesiredDisplayModeSpecs(baseModeId, + new SurfaceControl.DesiredDisplayModeSpecs(baseSfModeId, mDisplayModeSpecs.allowGroupSwitching, mDisplayModeSpecs.primaryRefreshRateRange.min, mDisplayModeSpecs.primaryRefreshRateRange.max, @@ -1090,14 +1105,14 @@ final class LocalDisplayAdapter extends DisplayAdapter { pw.println("mDisplayDeviceConfig=" + mDisplayDeviceConfig); } - private int findDisplayModeIdLocked(int modeId, int modeGroup) { - int matchingModeId = INVALID_MODE_ID; - DisplayModeRecord record = mSupportedModes.get(modeId); + private int findSfDisplayModeIdLocked(int displayModeId, int modeGroup) { + int matchingSfDisplayModeId = INVALID_MODE_ID; + DisplayModeRecord record = mSupportedModes.get(displayModeId); if (record != null) { for (SurfaceControl.DisplayMode mode : mSfDisplayModes) { if (record.hasMatchingMode(mode)) { - if (matchingModeId == INVALID_MODE_ID) { - matchingModeId = mode.id; + if (matchingSfDisplayModeId == INVALID_MODE_ID) { + matchingSfDisplayModeId = mode.id; } // Prefer to return a mode that matches the modeGroup @@ -1107,7 +1122,7 @@ final class LocalDisplayAdapter extends DisplayAdapter { } } } - return matchingModeId; + return matchingSfDisplayModeId; } // Returns a mode with id = modeId.