Claude review: drm/amdgpu: clamp user gartsize against device capacity

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

Patch Review

**1. Patch does not apply — based on stale tree (blocking)**

The patch modifies `gmc_v11_0.c` expecting the old inline pattern:
```c
-	/* set the gart size */
-	if (amdgpu_gart_size == -1)
-		adev->gmc.gart_size = 512ULL << 20;
-	else
-		adev->gmc.gart_size = (u64)amdgpu_gart_size << 20;
```
This code was already refactored into `amdgpu_gmc_set_gart_size()` and replaced with:
```c
	amdgpu_gmc_set_gart_size(adev, SZ_512M);
```
The patch must be rebased onto the current tree.

**2. Should modify the existing helper, not add a new one (design)**

The validation logic belongs inside the existing `amdgpu_gmc_set_gart_size()` at `amdgpu_gmc.c:283`. This would:
- Fix all gmc backends (v6–v12) automatically instead of requiring per-backend wiring
- Avoid a parallel API that does almost the same thing
- Eliminate the need for the "follow-up patches" the commit message mentions

**3. Missing `smu_prv_buffer_size` accounting (bug)**

The existing `amdgpu_gmc_set_gart_size()` adds `adev->pm.smu_prv_buffer_size` to the default GART size:
```c
	adev->gmc.gart_size = default_size + adev->pm.smu_prv_buffer_size;
```
The new `amdgpu_gmc_validate_gart_size()` returns `(u64)default_mb << 20` without this addition. This is a functional regression in the auto-default path.

**4. Falls back to default instead of clamping (questionable policy)**

When the user requests too much, the function falls back to the hard-coded default rather than clamping to the computed maximum:
```c
	if (want_bytes > max_bytes) {
		...
		return (u64)default_mb << 20;
	}
```
The function name says "clamp" and the commit message says "clamping," but it doesn't actually clamp — it resets to default. If the user asked for 64 GiB and the maximum is 65536 MiB (64 GiB), they might reasonably expect the maximum to be used, not a fall-back to 512 MiB. The user explicitly set a large value for a reason (e.g., compute workloads); clamping to `max_bytes` would better honor their intent.

**5. `user_mb` type is `int` but format specifier uses `%u` (minor)**

```c
	int user_mb, u32 default_mb)
	...
		dev_warn(adev->dev,
			 "amdgpu.gartsize=%u MiB exceeds device capacity "
```
`user_mb` is `int` but printed with `%u`. Although negative values are handled by the `== -1` check, a negative value other than -1 would print as a large unsigned number. Use `%d` for consistency with the type.

**6. Kernel style: avoid splitting quoted strings across lines (minor)**

```c
		dev_warn(adev->dev,
			 "amdgpu.gartsize=%u MiB exceeds device capacity "
			 "(real_vram=%llu MiB, max sensible=%llu MiB); "
			 "clamping to default %u MiB\n",
```
Kernel convention (per `checkpatch.pl`) prefers keeping format strings as a single searchable string even if it exceeds 80 columns, so that `grep` can find the message in the source. Consider keeping it on one line.

**7. The 1/8 VRAM limit for page table is reasonable but arbitrary (nit)**

The constraint caps the page-table BO at `real_vram_size / 8`. The math is:
- page_table_bytes = gart_bytes / 4096 * 8 = gart_bytes / 512
- constraint: gart_bytes / 512 <= real_vram / 8
- therefore: gart_bytes <= real_vram * 64

For 1 GiB VRAM: max gart = 64 GiB = 65536 MiB, which seems generous. The 1/8 reservation is not justified in the code or commit message — a brief rationale for why 1/8 was chosen would be helpful, especially since the page table only needs to be sized to fit, and the real constraint is that it must be ≤ real_vram_size (not 1/8 of it). A tighter bound (e.g., 1/4 or even 1/2) could also be justified if actual VRAM pressure from other pinned BOs is a concern.

**Summary:** The problem is valid and worth fixing, but this patch needs to be rebased and reworked to modify `amdgpu_gmc_set_gart_size()` directly. That would be a smaller, simpler change that fixes all backends at once.

---
Generated by Claude Code Patch Reviewer