Claude review: Refactor drm_writeback_connector structure

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

Overall Series Review

Subject: Refactor drm_writeback_connector structure
Author: Suraj Kandpal <suraj.kandpal@intel.com>
Patches: 9
Reviewed: 2026-05-25T19:19:14.562559

---

This is v5 of a series that refactors `drm_writeback_connector` by moving it from a standalone struct (that embeds `drm_connector`) into a field within `drm_connector` itself, in a union with the existing `hdmi` connector data. The motivation is sound: drivers like Intel cannot easily work with the current design where the connector is embedded inside `drm_writeback_connector`, because C's "inheritance via embedding" forces the writeback connector to be the outermost allocation owner, which conflicts with driver connector hierarchies.

The approach of unionizing writeback with HDMI inside `drm_connector` is reasonable since a connector cannot be both HDMI and writeback simultaneously. The series is well-structured, splitting the core refactor (patch 1) from incremental API migrations (patches 2-7).

**Key concerns:**

1. **Build-breaking bug in patch 4** (amdgpu): references `acrtc->connector` which doesn't exist -- the field is `acrtc->wb_conn` and it's a `struct drm_writeback_connector *`, so it needs `drm_writeback_to_connector(acrtc->wb_conn)`.

2. **The `encoder` field is silently dropped** from `drm_writeback_connector` without explanation. The old struct had an embedded `struct drm_encoder encoder` used by `drm_writeback_connector_init()` (the non-`_with_encoder` variant). This series does not address this; the new struct in `drm_connector.h` has no `encoder` field. This means `drm_writeback_connector_init()` (the old-style one that internally creates an encoder) would be broken. The series sits on Dmitry's series [3] which may handle this, but it should be mentioned.

3. **Inconsistency in patch 4** with the `malidp_hw.c` hunk: this is reverted from patch 1's changes, but the reverse direction looks identical to what patch 1 introduced. The diff shows the exact same changes being applied again (patch 1 already changed these lines to `.writeback`), suggesting the series was rebased onto Dmitry's patches that differ from what patch 1 targets.

4. **Minor typo** in the kerneldoc: `valriables` should be `variables` (patch 1, `drm_connector.h`).

Overall the design direction is good and most of the mechanical changes are correct. The amdgpu bug is the blocking issue.

---

---
Generated by Claude Code Patch Reviewer