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: rust/drm/gem: Use DeviceContext with GEM objects
Date: Sun, 22 Mar 2026 03:17:23 +1000
Message-ID: <review-patch5-20260320233645.950190-6-lyude@redhat.com>
In-Reply-To: <20260320233645.950190-6-lyude@redhat.com>
References: <20260320233645.950190-1-lyude@redhat.com> <20260320233645.950190-6-lyude@redhat.com>
X-Mailer: Claude Code Patch Reviewer
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0

Patch Review

**GAT (Generic Associated Type) usage:**
```rust
type Object<Ctx: drm::DeviceContext>: AllocImpl;
```
This is a significant API change, making `Object` a GAT. This is well-motiv=
ated =E2=80=94 it allows GEM objects to carry their device's context state.

**VTABLE initialization with arbitrary `Uninit` context:**
```rust
// Ignore the Uninit DeviceContext below. It is only provided because it is=
 required by the
// compiler, and it is not actually used by these functions.
gem_create_object: T::Object::<Uninit>::ALLOC_OPS.gem_create_object,
```
The comment explains this well. Since `ALLOC_OPS` is a `const` that doesn't=
 depend on the context type, the choice of `Uninit` is arbitrary and correc=
t. It would be slightly cleaner if `AllocOps` were associated with the driv=
er rather than the object type, but that would be a larger refactor.

**`open_callback` / `close_callback` use `Registered` context:**
```rust
let obj: &DriverAllocImpl<T> =3D unsafe { IntoGEMObject::from_raw(raw_obj) =
};
```
Since `DriverAllocImpl<T>` defaults to `DriverAllocImpl<T, Registered>`, th=
is correctly uses the `Registered` context for callbacks that can only be i=
nvoked after registration. The safety comment explicitly calls this out =E2=
=80=94 good.

**`Object::dev()` propagates `Ctx`:**
```rust
pub fn dev(&self) -> &drm::Device<T::Driver, Ctx> {
```
This is correct =E2=80=94 if you have an `Object<T, Uninit>`, you get back =
a `&Device<T, Uninit>`, which appropriately restricts what you can do with =
the device.

**`BaseObject::create_handle` and `lookup_handle` bound changes:**
```rust
D: drm::Driver<Object<Registered> =3D Self, File =3D F>,
```
This correctly constrains these operations to only work with `Registered` c=
ontext objects, since handle operations require a registered device.

**Overall:** This patch ties the series together cleanly. The PhantomData a=
ddition to `Object` is zero-cost, and the context propagation through the t=
ype system is sound.

---
Generated by Claude Code Patch Reviewer