fix(cardwired): on monitor event, re-apply the persisted mode, replace change uevent to add/remove by luytan · Pull Request #180 · OpenGamingCollective/cardwire

luytan · 2026-08-10T14:29:14Z

Description

This PR fixes issues with the auto switch on monitor event:

Re-apply the persisted mode on monitor event
Send add/remove event instead of change event (remove is ignored for gnome)
Monitor display now only listen for "change" events

TODO

Copy-Paste this line

Checklist:

My code follows the style guidelines of this project (cargo fmt)
I have performed a self-review of my code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the mdBook documentation
My changes generate no new warnings (clippy/clang)
New and existing unit tests pass locally with my changes (either use nix flake check or wait for the ci)

coderabbitai · 2026-08-10T14:29:48Z

📝 Walkthrough

Summary by CodeRabbit

Bug Fixes
- Improved display mode restoration after monitor disconnects, including temporary Hybrid overrides.
- Improved GPU blocking and unblocking across supported desktop environments.
- Reduced unnecessary display-event processing and improved retry behavior.
- Suppressed non-actionable background service messages in daemon logs.
Improvements
- Added consistent recognition of Niri, GNOME, Plasma, and COSMIC desktop environments.
- GPU state changes now emit appropriate system notifications, with failures logged without interrupting operations.

Walkthrough

The daemon centralizes desktop parsing, emits desktop-aware add/remove DRM uevents during GPU transitions, restores persisted display modes, and limits immediate DRM reconciliation to change events. It also filters zbus messages below warning level.

Changes

Desktop-aware GPU display flow

Layer / File(s)	Summary
Shared desktop contract `crates/cardwire-daemon/src/core/desktop.rs`, `crates/cardwire-daemon/src/core/mod.rs`, `crates/cardwire-daemon/src/analyzer/dynamic_analysis.rs`, `crates/cardwire-daemon/src/core/gpu/display.rs`	The daemon adds a public `Desktop` enum with case-insensitive parsing and reuses it in analysis and display handling.
Action-based DRM events `crates/cardwire-daemon/src/core/gpu/display.rs`, `crates/cardwire-daemon/src/core/gpu/mod.rs`, `crates/cardwire-daemon/src/interface/gpu.rs`, `crates/cardwire-daemon/src/interface/mode.rs`	DRM uevents support `Add` and `Remove`. GPU block and unblock operations emit the corresponding event, and duplicate emissions are removed.
Persisted-mode reconciliation `crates/cardwire-daemon/src/tasks/monitor_display.rs`, `crates/cardwire-daemon/src/daemon.rs`	Display monitoring restores `CardwireModeState`, processes only `change` events immediately, and uses retries for `add` and `remove`. The daemon filters `zbus` logs below warning level.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant GPU_interface
  participant send_drm_uevent
  participant monitor_display
  participant CardwireModeState
  GPU_interface->>send_drm_uevent: Send UdevAction::Add or UdevAction::Remove
  send_drm_uevent-->>monitor_display: Emit add/remove DRM event
  monitor_display->>CardwireModeState: Read persisted mode
  CardwireModeState-->>monitor_display: Return persisted mode
  monitor_display->>monitor_display: Queue change events for reconciliation

Possibly related PRs

OpenGamingCollective/cardwire#133: Modified the same external-display monitoring and DRM uevent paths.
OpenGamingCollective/cardwire#163: Modified related GPU display and DRM uevent handling.
OpenGamingCollective/cardwire#173: Modified DRM uevent handling in the GPU and mode interfaces.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description summarizes the main changes and includes the required TODO and checklist sections; no related issue is provided.
Title check	✅ Passed	The title clearly summarizes the persisted-mode restoration and add/remove uevent changes.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

crates/cardwire-daemon/src/interface/gpu.rs (1)
97-109: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Release the eBPF blocker write lock before the DRM uevent write.

By the runes, Lines 103 and 147 await a sysfs operation while blocker still holds the shared write lock. A slow driver delays gpu_blocked and all other block or unblock operations. Drop the guard after inode updates and before send_drm_uevent.
Proposed fix
         for inode in inodes {
             blocker.block_inode(inode, value).into_fdo()?;
         }
+        drop(blocker);
 
         if let Err(err) = send_drm_uevent(*self.device.card(), UdevAction::Remove).await {
@@
         for inode in inodes.iter() {
             blocker.unblock_inode(*inode, self.id).into_fdo()?;
         }
+        drop(blocker);
+
         if let Err(err) = send_drm_uevent(*self.device.card(), UdevAction::Add).await {
Also applies to: 142-152
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/cardwire-daemon/src/interface/gpu.rs` around lines 97 - 109, Release
the blocker write guard after completing the inode updates and before awaiting
send_drm_uevent in both affected block and unblock paths. Ensure the blocker
guard is dropped before the DRM uevent operation while preserving the existing
error handling and inode-update behavior.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/cardwire-daemon/src/core/gpu/display.rs`:
- Around line 169-182: Update the desktop handling around Desktop::from_str so
unrecognized desktop values continue into the remove operation instead of
returning early. Preserve the existing no-op exclusively for Desktop::Gnome,
while allowing Desktop::from_str returning None and recognized non-GNOME
desktops to emit the existing "remove\n" event.

---

Outside diff comments:
In `@crates/cardwire-daemon/src/interface/gpu.rs`:
- Around line 97-109: Release the blocker write guard after completing the inode
updates and before awaiting send_drm_uevent in both affected block and unblock
paths. Ensure the blocker guard is dropped before the DRM uevent operation while
preserving the existing error handling and inode-update behavior.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 6380e208-6d75-4b1c-a915-74ea2492a418

📥 Commits

Reviewing files that changed from the base of the PR and between 23a703e and cb85ccf.

📒 Files selected for processing (9)

crates/cardwire-daemon/src/analyzer/dynamic_analysis.rs
crates/cardwire-daemon/src/core/desktop.rs
crates/cardwire-daemon/src/core/gpu/display.rs
crates/cardwire-daemon/src/core/gpu/mod.rs
crates/cardwire-daemon/src/core/mod.rs
crates/cardwire-daemon/src/daemon.rs
crates/cardwire-daemon/src/interface/gpu.rs
crates/cardwire-daemon/src/interface/mode.rs
crates/cardwire-daemon/src/tasks/monitor_display.rs

coderabbitai

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

crates/cardwire-daemon/src/interface/gpu.rs (1)
102-108: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Add a per-GPU transition lock, wizard.

block_gpu and unblock_gpu also run directly from ModeInterface::apply_mode and monitor_display::reconcile_gpu; zbus dispatch does not serialize these paths. Since self.blocker is released before send_drm_uevent, an Add write can overtake a pending Remove write. Serialize the blocker update and uevent with a separate per-GPU lock. Keep self.blocker unlocked during filesystem I/O.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/cardwire-daemon/src/interface/gpu.rs` around lines 102 - 108,
Introduce a separate per-GPU transition lock covering the blocker state update
and the subsequent DRM uevent in the relevant block/unblock transition flow.
Ensure all callers, including ModeInterface::apply_mode and
monitor_display::reconcile_gpu, use this lock so Add and Remove events cannot
overtake each other, while releasing self.blocker before performing filesystem
I/O.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@crates/cardwire-daemon/src/interface/gpu.rs`:
- Around line 102-108: Introduce a separate per-GPU transition lock covering the
blocker state update and the subsequent DRM uevent in the relevant block/unblock
transition flow. Ensure all callers, including ModeInterface::apply_mode and
monitor_display::reconcile_gpu, use this lock so Add and Remove events cannot
overtake each other, while releasing self.blocker before performing filesystem
I/O.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: c6b81391-b586-4d46-a09b-72abb0bcb9b6

📥 Commits

Reviewing files that changed from the base of the PR and between 47cc7c9 and 152d0ab.

📒 Files selected for processing (1)

crates/cardwire-daemon/src/interface/gpu.rs

luytan added 6 commits August 10, 2026 16:10

chore(cardwired): silence zbus log noise

5e36c63

fix(cardwired): reconcile display against persisted mode

08af6a9

fix(cardwired): add uevent on unblock only

d1b9fc3

fix(cardwired): react to change uevents only

3a48028

chore(cardwired): move desktop enum to it's own core file

f1454a3

feat(cardwired): send remove event on non-mutter desktop

cb85ccf

coderabbitai Bot reviewed Aug 10, 2026

View reviewed changes

Comment thread crates/cardwire-daemon/src/core/gpu/display.rs Outdated

luytan linked an issue Aug 10, 2026 that may be closed by this pull request

ExternalDisplayAutoSwitch is unreliable, does not restore Smart mode, and can crash Mutter on NVIDIA HDMI hotplug #179

Open

luytan added 2 commits August 10, 2026 16:42

fix(cardwired): send remove event on non-listed desktops

47cc7c9

fix(cardwired): drop blocker before sending uevent

152d0ab

coderabbitai Bot reviewed Aug 10, 2026

View reviewed changes

luytan marked this pull request as draft August 13, 2026 09:50

Navigation Menu

Uh oh!

fix(cardwired): on monitor event, re-apply the persisted mode, replace change uevent to add/remove - #180

fix(cardwired): on monitor event, re-apply the persisted mode, replace change uevent to add/remove#180
luytan wants to merge 8 commits into
mainfrom
fix-gpu-display

luytan commented Aug 10, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Aug 10, 2026 •

edited

Loading

Summary by CodeRabbit

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

luytan commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

TODO

Checklist:

Uh oh!

coderabbitai Bot commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

luytan commented Aug 10, 2026 •

edited

Loading

coderabbitai Bot commented Aug 10, 2026 •

edited

Loading