Skip to content

Commit e07d8df

Browse files
Dominik LaskowskiAndroid (Google) Code Review
authored andcommitted
Merge "SF: Let DM resize framebuffer on resolution change" into main
2 parents 06b6e6b + 251e612 commit e07d8df

9 files changed

Lines changed: 102 additions & 31 deletions

File tree

libs/gui/include/gui/LayerState.h

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -502,12 +502,18 @@ struct DisplayState {
502502
Rect layerStackSpaceRect = Rect::EMPTY_RECT;
503503
Rect orientedDisplaySpaceRect = Rect::EMPTY_RECT;
504504

505-
// Exclusive to virtual displays: The sink surface into which the virtual display is rendered,
506-
// and an optional resolution that overrides its default dimensions.
507-
sp<IGraphicBufferProducer> surface;
505+
// For physical displays, this is the resolution, which must match the active display mode. To
506+
// change the resolution, the client must first call SurfaceControl.setDesiredDisplayModeSpecs
507+
// with the new DesiredDisplayModeSpecs#defaultMode, then commit the matching width and height.
508+
//
509+
// For virtual displays, this is an optional resolution that overrides its default dimensions.
510+
//
508511
uint32_t width = 0;
509512
uint32_t height = 0;
510513

514+
// For virtual displays, this is the sink surface into which the virtual display is rendered.
515+
sp<IGraphicBufferProducer> surface;
516+
511517
status_t write(Parcel& output) const;
512518
status_t read(const Parcel& input);
513519
};

services/surfaceflinger/Display/DisplayModeRequest.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@ namespace android::display {
2626
struct DisplayModeRequest {
2727
scheduler::FrameRateMode mode;
2828

29-
// Whether to emit DisplayEventReceiver::DISPLAY_EVENT_MODE_CHANGE.
29+
// Whether to emit DisplayEventReceiver::DISPLAY_EVENT_MODE_CHANGE for a change in refresh rate
30+
// or render rate. Ignored for resolution changes, which always emit the event.
3031
bool emitEvent = false;
3132

3233
// Whether to force the request to be applied, even if the mode is unchanged.

services/surfaceflinger/DisplayDevice.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -223,9 +223,7 @@ void DisplayDevice::setFlags(uint32_t flags) {
223223
mFlags = flags;
224224
}
225225

226-
void DisplayDevice::setDisplaySize(int width, int height) {
227-
LOG_FATAL_IF(!isVirtual(), "Changing the display size is supported only for virtual displays.");
228-
const auto size = ui::Size(width, height);
226+
void DisplayDevice::setDisplaySize(ui::Size size) {
229227
mCompositionDisplay->setDisplaySize(size);
230228
if (mRefreshRateOverlay) {
231229
mRefreshRateOverlay->setViewport(size);

services/surfaceflinger/DisplayDevice.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ class DisplayDevice : public RefBase {
9898
ui::Size getSize() const { return {getWidth(), getHeight()}; }
9999

100100
void setLayerFilter(ui::LayerFilter);
101-
void setDisplaySize(int width, int height);
101+
void setDisplaySize(ui::Size);
102102
void setProjection(ui::Rotation orientation, Rect viewport, Rect frame);
103103
void stageBrightness(float brightness) REQUIRES(kMainThreadContext);
104104
void persistBrightness(bool needsComposite) REQUIRES(kMainThreadContext);

services/surfaceflinger/Scheduler/include/scheduler/FrameRateMode.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@ struct FrameRateMode {
3333
}
3434

3535
bool operator!=(const FrameRateMode& other) const { return !(*this == other); }
36+
37+
bool matchesResolution(const FrameRateMode& other) const {
38+
return modePtr->getResolution() == other.modePtr->getResolution();
39+
}
3640
};
3741

3842
inline std::string to_string(const FrameRateMode& mode) {

services/surfaceflinger/SurfaceFlinger.cpp

Lines changed: 75 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1363,7 +1363,8 @@ void SurfaceFlinger::setDesiredMode(display::DisplayModeRequest&& desiredMode) {
13631363
const auto selectorPtr = mDisplayModeController.selectorPtrFor(displayId);
13641364
if (!selectorPtr) break;
13651365

1366-
const Fps renderRate = selectorPtr->getActiveMode().fps;
1366+
const auto activeMode = selectorPtr->getActiveMode();
1367+
const Fps renderRate = activeMode.fps;
13671368

13681369
// DisplayModeController::setDesiredMode updated the render rate, so inform Scheduler.
13691370
mScheduler->setRenderRate(displayId, renderRate, true /* applyImmediately */);
@@ -1382,6 +1383,15 @@ void SurfaceFlinger::setDesiredMode(display::DisplayModeRequest&& desiredMode) {
13821383

13831384
mScheduler->updatePhaseConfiguration(displayId, mode.fps);
13841385
mScheduler->setModeChangePending(true);
1386+
1387+
// The mode set to switch resolution is not initiated until the display transaction that
1388+
// resizes the display. DM sends this transaction in response to a mode change event, so
1389+
// emit the event now, not when finalizing the mode change as for a refresh rate switch.
1390+
if (FlagManager::getInstance().synced_resolution_switch() &&
1391+
!mode.matchesResolution(activeMode)) {
1392+
mScheduler->onDisplayModeChanged(displayId, mode,
1393+
/*clearContentRequirements*/ true);
1394+
}
13851395
break;
13861396
}
13871397
case DesiredModeAction::InitiateRenderRateSwitch:
@@ -1460,27 +1470,34 @@ bool SurfaceFlinger::finalizeDisplayModeChange(PhysicalDisplayId displayId) {
14601470
}
14611471

14621472
const auto& activeMode = pendingModeOpt->mode;
1463-
1464-
if (const auto oldResolution =
1465-
mDisplayModeController.getActiveMode(displayId).modePtr->getResolution();
1466-
oldResolution != activeMode.modePtr->getResolution()) {
1467-
auto& state = mCurrentState.displays.editValueFor(getPhysicalDisplayTokenLocked(displayId));
1468-
// We need to generate new sequenceId in order to recreate the display (and this
1469-
// way the framebuffer).
1470-
state.sequenceId = DisplayDeviceState{}.sequenceId;
1471-
state.physical->activeMode = activeMode.modePtr.get();
1472-
processDisplayChangesLocked();
1473-
1474-
// The DisplayDevice has been destroyed, so abort the commit for the now dead FrameTargeter.
1475-
return false;
1473+
const bool resolutionMatch = !FlagManager::getInstance().synced_resolution_switch() ||
1474+
activeMode.matchesResolution(mDisplayModeController.getActiveMode(displayId));
1475+
1476+
if (!FlagManager::getInstance().synced_resolution_switch()) {
1477+
if (const auto oldResolution =
1478+
mDisplayModeController.getActiveMode(displayId).modePtr->getResolution();
1479+
oldResolution != activeMode.modePtr->getResolution()) {
1480+
auto& state =
1481+
mCurrentState.displays.editValueFor(getPhysicalDisplayTokenLocked(displayId));
1482+
// We need to generate new sequenceId in order to recreate the display (and this
1483+
// way the framebuffer).
1484+
state.sequenceId = DisplayDeviceState{}.sequenceId;
1485+
state.physical->activeMode = activeMode.modePtr.get();
1486+
processDisplayChangesLocked();
1487+
1488+
// The DisplayDevice has been destroyed, so abort the commit for the now dead
1489+
// FrameTargeter.
1490+
return false;
1491+
}
14761492
}
14771493

14781494
mDisplayModeController.finalizeModeChange(displayId, activeMode.modePtr->getId(),
14791495
activeMode.modePtr->getVsyncRate(), activeMode.fps);
14801496

14811497
mScheduler->updatePhaseConfiguration(displayId, activeMode.fps);
14821498

1483-
if (pendingModeOpt->emitEvent) {
1499+
// Skip for resolution changes, since the event was already emitted on setting the desired mode.
1500+
if (resolutionMatch && pendingModeOpt->emitEvent) {
14841501
mScheduler->onDisplayModeChanged(displayId, activeMode, /*clearContentRequirements*/ true);
14851502
}
14861503

@@ -1532,8 +1549,9 @@ void SurfaceFlinger::initiateDisplayModeChanges() {
15321549
to_string(displayModePtrOpt->get()->getVsyncRate()).c_str(),
15331550
to_string(displayId).c_str());
15341551

1535-
if ((!FlagManager::getInstance().connected_display() || !desiredModeOpt->force) &&
1536-
mDisplayModeController.getActiveMode(displayId) == desiredModeOpt->mode) {
1552+
const auto activeMode = mDisplayModeController.getActiveMode(displayId);
1553+
1554+
if (!desiredModeOpt->force && desiredModeOpt->mode == activeMode) {
15371555
applyActiveMode(displayId);
15381556
continue;
15391557
}
@@ -1554,6 +1572,15 @@ void SurfaceFlinger::initiateDisplayModeChanges() {
15541572
constraints.seamlessRequired = false;
15551573
hal::VsyncPeriodChangeTimeline outTimeline;
15561574

1575+
// When initiating a resolution change, wait until the commit that resizes the display.
1576+
if (FlagManager::getInstance().synced_resolution_switch() &&
1577+
!activeMode.matchesResolution(desiredModeOpt->mode)) {
1578+
const auto display = getDisplayDeviceLocked(displayId);
1579+
if (display->getSize() != desiredModeOpt->mode.modePtr->getResolution()) {
1580+
continue;
1581+
}
1582+
}
1583+
15571584
const auto error =
15581585
mDisplayModeController.initiateModeChange(displayId, std::move(*desiredModeOpt),
15591586
constraints, outTimeline);
@@ -4124,6 +4151,35 @@ void SurfaceFlinger::processDisplayChanged(const wp<IBinder>& displayToken,
41244151
if (currentState.flags != drawingState.flags) {
41254152
display->setFlags(currentState.flags);
41264153
}
4154+
4155+
const auto updateDisplaySize = [&]() {
4156+
if (currentState.width != drawingState.width ||
4157+
currentState.height != drawingState.height) {
4158+
const ui::Size resolution = ui::Size(currentState.width, currentState.height);
4159+
4160+
// Resize the framebuffer. For a virtual display, always do so. For a physical
4161+
// display, only do so if it has a pending modeset for the matching resolution.
4162+
if (!currentState.physical ||
4163+
(FlagManager::getInstance().synced_resolution_switch() &&
4164+
mDisplayModeController.getDesiredMode(display->getPhysicalId())
4165+
.transform([resolution](const auto& request) {
4166+
return resolution == request.mode.modePtr->getResolution();
4167+
})
4168+
.value_or(false))) {
4169+
display->setDisplaySize(resolution);
4170+
}
4171+
4172+
if (display->getId() == mActiveDisplayId) {
4173+
onActiveDisplaySizeChanged(*display);
4174+
}
4175+
}
4176+
};
4177+
4178+
if (FlagManager::getInstance().synced_resolution_switch()) {
4179+
// Update display size first, as display projection below depends on it.
4180+
updateDisplaySize();
4181+
}
4182+
41274183
if ((currentState.orientation != drawingState.orientation) ||
41284184
(currentState.layerStackSpaceRect != drawingState.layerStackSpaceRect) ||
41294185
(currentState.orientedDisplaySpaceRect != drawingState.orientedDisplaySpaceRect)) {
@@ -4135,13 +4191,9 @@ void SurfaceFlinger::processDisplayChanged(const wp<IBinder>& displayToken,
41354191
ui::Transform::toRotationFlags(display->getOrientation());
41364192
}
41374193
}
4138-
if (currentState.width != drawingState.width ||
4139-
currentState.height != drawingState.height) {
4140-
display->setDisplaySize(currentState.width, currentState.height);
41414194

4142-
if (display->getId() == mActiveDisplayId) {
4143-
onActiveDisplaySizeChanged(*display);
4144-
}
4195+
if (!FlagManager::getInstance().synced_resolution_switch()) {
4196+
updateDisplaySize();
41454197
}
41464198
}
41474199
}

services/surfaceflinger/common/FlagManager.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,7 @@ void FlagManager::dump(std::string& result) const {
171171
DUMP_ACONFIG_FLAG(restore_blur_step);
172172
DUMP_ACONFIG_FLAG(skip_invisible_windows_in_input);
173173
DUMP_ACONFIG_FLAG(stable_edid_ids);
174+
DUMP_ACONFIG_FLAG(synced_resolution_switch);
174175
DUMP_ACONFIG_FLAG(trace_frame_rate_override);
175176
DUMP_ACONFIG_FLAG(true_hdr_screenshots);
176177
DUMP_ACONFIG_FLAG(use_known_refresh_rate_for_fps_consistency);
@@ -295,6 +296,7 @@ FLAG_MANAGER_ACONFIG_FLAG(skip_invisible_windows_in_input, "");
295296
FLAG_MANAGER_ACONFIG_FLAG(begone_bright_hlg, "debug.sf.begone_bright_hlg");
296297
FLAG_MANAGER_ACONFIG_FLAG(window_blur_kawase2, "");
297298
FLAG_MANAGER_ACONFIG_FLAG(reject_dupe_layerstacks, "");
299+
FLAG_MANAGER_ACONFIG_FLAG(synced_resolution_switch, "");
298300

299301
/// Trunk stable server (R/W) flags ///
300302
FLAG_MANAGER_ACONFIG_FLAG(refresh_rate_overlay_on_external_display, "")

services/surfaceflinger/common/include/common/FlagManager.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ class FlagManager {
107107
bool restore_blur_step() const;
108108
bool skip_invisible_windows_in_input() const;
109109
bool stable_edid_ids() const;
110+
bool synced_resolution_switch() const;
110111
bool trace_frame_rate_override() const;
111112
bool true_hdr_screenshots() const;
112113
bool use_known_refresh_rate_for_fps_consistency() const;

services/surfaceflinger/surfaceflinger_flags_new.aconfig

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,13 @@ flag {
264264
is_fixed_read_only: true
265265
} # stable_edid_ids
266266

267+
flag {
268+
name: "synced_resolution_switch"
269+
namespace: "core_graphics"
270+
description: "Synchronize resolution modeset with framebuffer resizing"
271+
bug: "355427258"
272+
} # synced_resolution_switch
273+
267274
flag {
268275
name: "true_hdr_screenshots"
269276
namespace: "core_graphics"

0 commit comments

Comments
 (0)