Claude review: drm/vmwgfx: Check whether CRTC is active when updating planes

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

Patch Review

**Correctness: Good**

**The `active` check (line 1417)**:
```c
if (crtc && new_state->fb &&
    drm_atomic_get_new_crtc_state(state, crtc)->active) {
```
This is the right approach. When a CRTC exists but is not active (e.g., DPMS off), the plane update should go through the blank/unbind path rather than trying to bind and update the surface. The `drm_atomic_get_new_crtc_state()` call is safe here because when `new_state->crtc` is non-NULL, the DRM atomic framework guarantees the CRTC state is part of the commit (ensured by `drm_atomic_helper_check_modeset()` which runs before `atomic_update`). This is consistent with how the same function is used elsewhere in this file (line 433, 900).

**The else-branch rework (lines 1441–1442)**:
```c
stdu = vmw_primary_to_stdu(plane);
dev_priv = vmw_priv(plane->dev);
```
This replaces the old unsafe code:
```c
crtc = old_state->crtc;
stdu = vmw_crtc_to_stdu(crtc);
dev_priv = vmw_priv(crtc->dev);
```
The old code dereferenced `old_state->crtc` without a NULL check. Since the else branch is entered when `new_state->crtc` is NULL, there's no guarantee `old_state->crtc` is also non-NULL (e.g., if the plane was never attached to a CRTC). Using `container_of(plane, ...)` via `vmw_primary_to_stdu()` is safe because `base.primary` is an embedded `struct drm_plane` (not a pointer) in `vmw_display_unit`, as confirmed in `vmwgfx_kms.h`. The plane pointer itself is always valid in this callback.

**The new macro (line 47–48)**:
```c
#define vmw_primary_to_stdu(x) \
	container_of(x, struct vmw_screen_target_display_unit, base.primary)
```
Follows the existing pattern of `vmw_crtc_to_stdu`, `vmw_encoder_to_stdu`, and `vmw_connector_to_stdu`. Consistent style.

**One minor concern**: With the added `active` check, the else branch now also handles the case where `crtc` is non-NULL, `fb` is non-NULL, but the CRTC is inactive. In that case, the code falls through to the blank path, which calls `vmw_stdu_bind_st(dev_priv, stdu, NULL)` and `vmw_stdu_update_st()` — but only if `stdu->defined` is true. This is correct behavior: when a CRTC is being deactivated, the STDU should be blanked. The `!stdu->defined` early return guards against double-unbinding.

**Fixes tag**: References `e05162c017e2 ("drm: Store new plane state in a variable for atomic_update and disable")`, which is the commit that restructured how the plane states are accessed. This is the correct provenance for the bug.

**Verdict**: The patch is correct and ready to merge. No issues found.

---
Generated by Claude Code Patch Reviewer