Claude review: drm/mgag200: reject pixel clocks outside PLL range

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

Patch Review

**Correctness: Good.** The check is placed at exactly the right spot — after the search loop exhausts all candidates and before the results are written to `pixpllc->m/n/p/s`. The tolerance formula `clock * 5 / 1000` (0.5% of the requested clock) matches the g200se reference at `mgag200_g200se.c:140` and `mgag200_g200se.c:232`. Verified across all six files that the insertion point is between the end of the search loop and the `pixpllc->m = m` assignment.

**Why the bug matters:** In `atomic_update`, the register values are derived as e.g. `pixpllcm = pixpllc->m - 1` (`mgag200_g200eh.c:112`). With `m = 0`, this underflows to `0xffffffff` (truncated to a `u8`), producing corrupted PLL register writes.

**Style concern — `pr_warn` vs `drm_warn`:** The patch uses bare `pr_warn("PLL delta too large\n")` to match the g200se reference. However, the rest of the driver consistently uses `drm_err`/`drm_warn`/`drm_dbg` (which prepend device identification). A `struct drm_device *` is available in `atomic_check` via `crtc->dev`. This is a pre-existing issue in g200se too, but since six new call sites are being added, it would be worth upgrading to:

```c
drm_warn(crtc->dev, "PLL delta too large\n");
```

across all seven sites (the six new ones plus the existing g200se). This makes log messages attributable to a specific device, which matters on multi-GPU or multi-BMC systems.

**Minor functional observation:** The `pr_warn` fires during `atomic_check`, which is a validation path — userspace probing supported modes will trigger this message for any unsupported clock. This could be chatty. A `drm_dbg_kms` might be more appropriate, since the `-EINVAL` return already communicates the rejection to the DRM core. The g200se code has the same issue, so this is more of a follow-up suggestion than a blocker for this patch.

**No logic bugs found.** The patch is a straightforward, correct backport of the existing g200se pattern to the six files that were missing it. The integer arithmetic is safe for the clock ranges these PLLs support (VCO maxima are all under 1.6 GHz = 1,600,000 kHz, so `clock * 5` won't overflow a `long`).

**Verdict:** The patch fixes a real bug with the right approach. Suggest switching `pr_warn` to `drm_dbg_kms(crtc->dev, ...)` (or at minimum `drm_warn(crtc->dev, ...)`), but the logic itself is correct and ready.

---
Generated by Claude Code Patch Reviewer