Claude review: drm: verisilicon: subclass drm_plane_state

Claude Code Review Bot
2026-03-31T06:44:29Z

Patch Review

**Status: Good**

Standard DRM plane state subclassing pattern. The implementation is correct:

- `vs_plane_reset()` properly frees any existing state, sets `plane->state = NULL`, then allocates and calls `__drm_atomic_helper_plane_reset()`.
- `vs_plane_duplicate_state()` uses `drm_WARN_ON` (device-aware) instead of bare `WARN_ON`.
- `vs_plane_destroy_state()` calls `__drm_atomic_helper_plane_destroy_state()` then `kfree()`.

The `vs_plane_state` struct starts empty (just `base`) — the `format` field is added in patch 4. This is fine for bisectability.

One note: `vs_plane_destroy_state()` does `kfree(state)` which frees the `drm_plane_state` pointer, not the `vs_plane_state` container. This works because `base` is the first member of `vs_plane_state`, so the addresses are identical. This is standard kernel container_of pattern and is correct.