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/nouveau/kms/nvd9-: Use contiguous memory for CRC notifier context
Date: Tue, 05 May 2026 09:05:55 +1000
Message-ID: <review-patch1-20260501215856.840898-1-lyude@redhat.com>
In-Reply-To: <20260501215856.840898-1-lyude@redhat.com>
References: <20260501215856.840898-1-lyude@redhat.com> <20260501215856.840898-1-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

**Correctness of the core fix:**

The old code called `nvif_mem_ctor_map()` which internally passes `NULL, 0`=
 for the argv/argc parameters to `nvif_mem_ctor()`:

```c
// nvif/mem.c:31-32 (old path)
int ret =3D nvif_mem_ctor(mmu, name, mmu->mem, NVIF_MEM_MAPPABLE | type,
                        0, size, NULL, 0, mem);
```

The new code replaces this with a direct `nvif_mem_ctor()` call that passes=
 a `gf100_mem_v0` struct with `.contig =3D true`:

```c
ret =3D nvif_mem_ctor(mmu, "kmsCrcNtfy", mmu->mem, NVIF_MEM_VRAM | NVIF_MEM=
_MAPPABLE, 0, len,
                    &(struct gf100_mem_v0) {
                            .contig =3D true,
                    }, sizeof(struct gf100_mem_v0), &ctx->mem);
```

This is correct. The compound literal zero-initializes `.version` to 0, mat=
ching the `v0` convention. The `gf100_mem_v0` data gets copied into `args->=
data[]` inside `nvif_mem_ctor_type()` (mem.c:70) and forwarded to the firmw=
are/kernel memory handler, which will honor the contiguous flag.

The flags `NVIF_MEM_VRAM | NVIF_MEM_MAPPABLE` match exactly what `nvif_mem_=
ctor_map()` was computing internally (`NVIF_MEM_MAPPABLE | type` where `typ=
e` was `NVIF_MEM_VRAM`).

**Error handling:**

The separate `nvif_object_map()` call after `nvif_mem_ctor()` correctly jum=
ps to `fail_fini` on failure, which calls `nvif_mem_dtor()` to clean up the=
 allocation. This mirrors the cleanup that `nvif_mem_ctor_map()` did intern=
ally (mem.c:36).

**Minor style observation (non-blocking):**

The patch adds `struct drm_device *dev` as a new parameter to `nv50_crc_ctx=
_init()`, but `dev` is already reachable via the existing `head` parameter =
as `head->base.base.dev`. The parameter is only used once to replace that c=
hain:

```c
-	struct nv50_core *core =3D nv50_disp(head->base.base.dev)->core;
+	struct nv50_core *core =3D nv50_disp(dev)->core;
```

For a stable-targeted bug fix, keeping the change minimal by not adding the=
 parameter would be slightly preferable =E2=80=94 just use `head->base.base=
.dev` as before. That said, it's a trivial cleanup and not worth a respin.

**Reviewed-by worthy:** Yes. The fix is well-motivated, correct, and approp=
riately scoped.

---
Generated by Claude Code Patch Reviewer