[PATCH] drm/vmwgfx: Never destroy STDU zero's Screen Target

[PATCH] drm/vmwgfx: Never destroy STDU zero's Screen Target

[Ian Forbes]

When some VMware clients connect to a VM they query the existing screen
target information to create a window of the same size. If the VM
display is asleep and the CRTC is disabled we don't have a screen target
and the client will not create a window which may lead it to have no
way of accepting user input.

Without input the VM cannot be awoken and the user is left with a blank
screen. This can be overcome by using the 'Send Key' command from the
client's application menu but this is not an ideal experience for users.

This can be fixed by always leaving display unit zero with a screen target.

Fixes: 7ef91dcba172 ("drm/vmwgfx: Don't destroy Screen Target when CRTC is enabled but inactive")
Signed-off-by: Ian Forbes <ian.forbes@broadcom.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
index 4139837f4caf..5ad7e942ceb8 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
@@ -448,6 +448,13 @@ static void vmw_stdu_crtc_atomic_disable(struct drm_crtc *crtc,
 		    !new_crtc_state->mode_changed)
 			return;
 
+		/* Never destroy DU 0's ST so that the VM client always has a
+		 * window to accept input from. Otherwise we may never receive
+		 * input to wake the VM when the CRTC is disabled during idle.
+		 */
+		if (stdu->base.unit == 0)
+			return;
+
 		ret = vmw_stdu_destroy_st(dev_priv, stdu);
 		if (ret)
 			DRM_ERROR("Failed to destroy Screen Target\n");
-- 
2.54.0

Claude review: drm/vmwgfx: Never destroy STDU zero's Screen Target

[Claude Code Review Bot]

Overall Series Review

Subject: drm/vmwgfx: Never destroy STDU zero's Screen Target
Author: Ian Forbes <ian.forbes@broadcom.com>
Patches: 1
Reviewed: 2026-05-16T10:41:44.665104

---

This is a single-patch fix for the VMware SVGA Screen Target Display Unit (STDU) driver. The patch addresses a real usability problem: when a VM display goes to sleep and the CRTC is disabled, the Screen Target for display unit 0 gets destroyed, which causes some VMware clients to fail to create a window (since they query existing screen target info to determine window size). Without a window, the user can't provide input to wake the VM.

The fix is simple and narrowly scoped — it adds an early return in `vmw_stdu_crtc_atomic_disable()` for unit 0, preventing the Screen Target from being destroyed. The approach is reasonable and consistent with how the driver already handles the "inactive but enabled" case (the existing early return at lines 446-449).

**Verdict: Looks correct and well-motivated. One minor concern worth discussing below.**

---
Generated by Claude Code Patch Reviewer

Claude review: drm/vmwgfx: Never destroy STDU zero's Screen Target

[Claude Code Review Bot]

Patch Review

**Commit message:** Well written. Clearly explains the problem (client can't create a window, user can't wake VM), the root cause (Screen Target destroyed on disable), and the fix. The Fixes tag references the commit that introduced the "don't destroy on inactive" logic, which this patch extends.

**Code review:**

The change at `vmwgfx_stdu.c:455-456`:
```c
if (stdu->base.unit == 0)
    return;
```

This is placed after the bind-to-NULL + update sequence (lines 437-441) which blanks the display, and after the existing early return for the "enabled but inactive" case (lines 446-449). So unit 0's Screen Target will still be unbound/blanked — it just won't be destroyed. This is the right behavior: the display goes dark but the ST metadata remains for clients to query.

**Minor concern — skipped `content_fb_type` reset:** When we early-return for unit 0, we also skip the `stdu->content_fb_type = SAME_AS_DISPLAY` assignment at line 462. This is the same situation as the existing early return at line 449, which also skips it, so this is consistent with the existing pattern. However, it's worth noting that `mode_set_nofb()` (line 395-399) always destroys and resets `content_fb_type` before re-defining, so the re-enable path should handle this correctly regardless.

**Interaction with `mode_set_nofb`:** When the CRTC is re-enabled, `vmw_stdu_crtc_mode_set_nofb()` checks `stdu->defined` (line 388) and will destroy + recreate the Screen Target with the new mode. Since we leave `stdu->defined = true` for unit 0, this path works correctly — it will tear down and rebuild as needed.

**No leak on driver teardown:** `vmw_stdu_destroy_st()` checks `stdu->defined` at the top (line 340) and is a no-op if false. Since unit 0 keeps `defined = true`, any cleanup path that calls `destroy_st` will still properly clean up. This looks safe.

**Comment style:** The multi-line comment uses `/* ... */` style which is fine but the kernel generally prefers the opening `/*` on its own line for multi-line comments (network-style). The existing comment above (lines 443-445) uses the same style, so this is at least locally consistent. Not a blocker.

**Overall:** Clean, minimal fix. The approach of special-casing unit 0 is pragmatic — it's the primary display and the one clients will look for. No correctness issues identified.

**Reviewed-by worthy: Yes.**

---
Generated by Claude Code Patch Reviewer