Fix #819: preserve user-applied monitor config across DRM hotplug#820
Open
RedDawe wants to merge 2 commits intolinuxmint:masterfrom
Open
Fix #819: preserve user-applied monitor config across DRM hotplug#820RedDawe wants to merge 2 commits intolinuxmint:masterfrom
RedDawe wants to merge 2 commits intolinuxmint:masterfrom
Conversation
…linuxmint#819) Hotplug events that don't change the connected monitor set (e.g. a monitor's auto input scan triggering a DRM link-state ping) caused ensure_configured() to walk its fallback chain and end up at create_suggested(), which re-enabled monitors the user had explicitly turned off via xrandr, Super+P, or the Display GUI. The user's most recent intent only lives in current_config (set on every successful apply, including TEMPORARY ones), and that was never consulted. Insert a current_config-preferring branch at the top of ensure_configured(): if there is an applied config and it is still complete for the detected monitor set, re-apply it. is_config_complete checks the monitor-spec key (connector + EDID), so genuine plug/unplug falls through to the existing logic unchanged. Skipped during in_init so the on-disk stored config still wins on session start. Also guard set_current() against the no-op self-set, otherwise every hotplug would push a duplicate of the current config onto the 3-slot history queue and evict the genuine previous configs.
…eserves-disabled Build a config with the second monitor disabled, mark it current (as apply_monitors_config does after a TEMPORARY apply), then trigger a hotplug with the same monitor topology. Before the fix this re-built a fresh suggested layout and re-enabled the monitor; after the fix the disabled state survives.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #819.
Background
When a user explicitly disables an external monitor (via
xrandr --output X --off, Super+P, or the Cinnamon Display GUI), Muffin re-enables it on the next DRM hotplug event — most commonly triggered by the monitor itself performing an auto input-scan and re-toggling its link state. The user's intent is silently overridden until they re-disable, often within seconds.Root cause
The user's most recent intent lives in
MetaMonitorConfigManager::current_config, which is updated on every successfulapply_monitors_config()(includingMETHOD_TEMPORARYapplies, which xrandr / Super+P / D-Bus go through). On hotplug,meta_monitor_manager_on_hotplug()callsmeta_monitor_manager_ensure_configured(), whose fallback chain isstored → suggested → previous → linear → fallback. The currently-applied config is never consulted, so any state that hasn't also been written tocinnamon-monitors.xmlis lost —create_suggested()rebuilds a fresh layout that re-enables every detected output.This affects the common case where the user applies a temporary off (no GUI save), but also the
should_use_stored_config()branch (hotplug_mode_update) where stored config is bypassed entirely.Fix
src/backends/meta-monitor-manager.c: insert acurrent_config-preferring branch at the top ofensure_configured(). If a config has already been applied this session and is still complete for the detected monitor set (is_config_completechecks the EDID-derived monitor-spec key), re-apply it. Skipped duringin_initso the on-disk stored config still wins on session start. Genuine plug/unplug changes the spec key, sois_config_completereturns false and the existing fallback chain runs as before.src/backends/meta-monitor-config-manager.c: guardset_current()against the no-opcurrent == configself-set. Without this, every hotplug pushes a duplicate of the current config onto the 3-slot history queue, evicting the genuine previous configs that exist for things likepop_previousundo.Test
src/tests/monitor-unit-tests.c: regression test that (1) builds a config with the second monitor disabled, (2) marks it current asapply_monitors_config()would after a TEMPORARY apply, (3) triggers a hotplug with the same monitor topology. Asserts the disabled state is preserved across the hotplug. Without the fix this test fails becauseensure_configured()falls through tocreate_suggested().Confirmation on real hardware (HP Omen 27q via amdgpu, or any monitor with auto input-scan behavior) would be appreciated from anyone with the matching setup.