Claude review: fbcon: don't suspend/resume when vc is graphics mode

[Claude Code Review Bot <claude-review@example.com>]

Patch Review

**Code change — looks correct:**

The two hunks add appropriate guards:

```c
/* Don't need to clear cursor and restore saved data in graphic mode */
if (vc->vc_mode != KD_GRAPHICS)
	fbcon_cursor(vc, false);
```

```c
/* Graphics mode is managed by userspace */
if (vc->vc_mode != KD_GRAPHICS)
	update_screen(vc);
```

This is consistent with the existing pattern in fbcon.c. Looking at the rest of the file, essentially every operation that modifies screen content checks for `KD_TEXT` (or `!= KD_GRAPHICS`) before proceeding — see `fbcon_modechanged()` at line 2634, `fbcon_set_all_vcs()` at line 2674, `fbcon_registered_fb_set_buf()` at line 3021, `fbcon_init()` at line 1193, etc. The suspend/resume paths were missing this check, which is arguably a pre-existing bug.

The caller `fb_set_suspend()` in `fbmem.c` (line 652) does hold `console_lock`, so there's no TOCTOU concern with reading `vc->vc_mode`.

**Issues:**

1. **Commit message is poorly written and hard to follow.** The description:
   > "At the beginning, starting the Xorg with single screen and then an external screen was plugged in. After logging out in Xorg, fbdev info may using screen which is connected later on for info always using first connected connector in list in func 'drm_setup_crtcs_fb'. Then, S3 executed, fbcon found that the information did not match and do atomic to switch fb. However, Xorg will not re-bind the crtc fb but continues doing ioctl. At this time, the fb is incorrect."

   This scenario is confusing and mixes several unrelated issues (hotplug ordering, `drm_setup_crtcs_fb` connector selection, suspend/resume, Xorg CRTC rebinding). The commit message should clearly describe: (a) the symptom (what goes wrong visually or functionally), (b) the root cause (fbcon_suspended/resumed unconditionally operate on the framebuffer even in KD_GRAPHICS mode), and (c) why the fix is correct (graphics mode means userspace owns the display, and fbcon should not touch it — consistent with all other fbcon code paths).

2. **The described scenario may indicate a deeper issue.** The commit message describes a problem with `drm_setup_crtcs_fb` picking the wrong connector after hotplug, which wouldn't be fixed by this patch — it would only be masked. If that's a real bug, it should be filed/fixed separately. This patch should stand on its own merits: fbcon should not manipulate the screen during suspend/resume when in graphics mode, period.

3. **Minor style nit:** The first comment says "Don't need to clear cursor and restore saved data in graphic mode" — it would be slightly more consistent to say "in graphics mode" (with the 's'), and honestly neither comment adds much value since the `KD_GRAPHICS` check is self-explanatory. Consider dropping the comments entirely, as most other `KD_TEXT`/`KD_GRAPHICS` checks in this file have no accompanying comment.

**Recommendation:** Request a v2 with a rewritten commit message that clearly states: "fbcon_suspended() and fbcon_resumed() unconditionally clear the cursor and update the screen. When the VC is in KD_GRAPHICS mode, userspace owns the display and fbcon should not touch it. Add KD_GRAPHICS guards consistent with other fbcon code paths." The code itself is fine.

---
Generated by Claude Code Patch Reviewer