From mboxrd@z Thu Jan  1 00:00:00 1970
From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: drm/bridge: Add new atomic_create_state callback
Date: Thu, 04 Jun 2026 15:45:42 +1000
Message-ID: <review-patch4-20260530-drm-no-more-bridge-reset-v1-4-875d828d31bc@kernel.org>
In-Reply-To: <20260530-drm-no-more-bridge-reset-v1-4-875d828d31bc@kernel.org>
References: <20260530-drm-no-more-bridge-reset-v1-0-875d828d31bc@kernel.org> <20260530-drm-no-more-bridge-reset-v1-4-875d828d31bc@kernel.org>
X-Mailer: Claude Code Patch Reviewer
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0

Patch Review

**Critical bisectability issue.** In `include/drm/drm_bridge.h`, the diff shows:

```c
-	struct drm_bridge_state *(*atomic_reset)(struct drm_bridge *bridge);
+	struct drm_bridge_state *(*atomic_create_state)(struct drm_bridge *bridge);
```

This *replaces* `atomic_reset` in the struct. But in `drivers/gpu/drm/drm_bridge.c`, the same patch adds a fallback that still references the removed field:

```c
+	if (bridge->funcs->atomic_create_state)
+		state = bridge->funcs->atomic_create_state(bridge);
+	else
+		state = bridge->funcs->atomic_reset(bridge);
```

Since `atomic_reset` no longer exists in the struct after this patch, `bridge->funcs->atomic_reset` won't compile. Additionally, all ~60 unconverted drivers still set `.atomic_reset = ...` in their `drm_bridge_funcs` initializers, which also won't compile.

**Fix:** Patch 4 should ADD `atomic_create_state` as a *new* field alongside the existing `atomic_reset`, not replace it. Then patch 76 (the final removal) should delete `atomic_reset` from the struct after all drivers are converted.

**Secondary issue in `drm_bridge_get_current_state`:** Even if the struct issue is fixed, this patch changes the check to only look for `atomic_create_state`:

```c
-	if (!bridge->funcs || !bridge->funcs->atomic_reset)
+	if (!bridge->funcs || !bridge->funcs->atomic_create_state)
```

But `drm_bridge_is_atomic()` correctly checks for both:

```c
+	return (bridge->funcs->atomic_create_state ||
+		bridge->funcs->atomic_reset);
```

During the transition (patches 4-71), `drm_bridge_get_current_state` would return NULL for bridges that only have `atomic_reset` set, even though they are properly atomic. This check should also use `||` with both callbacks until patch 76 removes `atomic_reset`.

---
Generated by Claude Code Patch Reviewer