Claude review: drm/tegra: gr2d/gr3d: Contain PM in the gr*d_probe/gr*d_remove

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

Patch Review

**Assessment: Correct fix with minor observations.**

This patch makes two changes:

1. **Moves `pm_runtime_enable()`, `pm_runtime_use_autosuspend()`, and `pm_runtime_set_autosuspend_delay()` from `gr*d_runtime_resume()` into `gr*d_probe()`.** Having `pm_runtime_enable()` inside the runtime_resume callback was fundamentally wrong — the PM core needs runtime PM to be enabled *before* it can call the resume callback through normal runtime PM operations. Every resume would also redundantly re-configure autosuspend. Moving this to probe is correct.

2. **Removes `pm_runtime_dont_use_autosuspend()` and `pm_runtime_force_suspend()` from `gr*d_exit()`.** The exit callback no longer manages PM lifecycle; this is handled by `pm_runtime_disable()` in `gr*d_remove()`.

**Observations:**

- **Typo in commit message:** `"GR2G/GR3D"` should be `"GR2D/GR3D"`.

- **Ordering relative to other tegra drivers:** The patches place `pm_runtime_enable()` *before* `host1x_client_register()`, which is the safer ordering (avoids a race where userspace submits before PM is enabled). However, the sibling drivers `nvdec.c` and `vic.c` do the opposite — they call `host1x_client_register()` first, then `pm_runtime_enable()`. The new ordering in gr2d/gr3d is arguably more correct, but the inconsistency across the tegra driver family is worth being aware of.

- **Missing `pm_runtime_dont_use_autosuspend()` in remove:** The patch removes `pm_runtime_dont_use_autosuspend()` from `gr*d_exit()` but doesn't add it to `gr*d_remove()` before `pm_runtime_disable()`. The common pattern for drivers using autosuspend is to call `pm_runtime_dont_use_autosuspend()` then `pm_runtime_disable()` in the remove path, to cancel any pending autosuspend timer. In practice, `pm_runtime_disable()` handles cancellation internally, so this is not a bug — just a deviation from the typical pattern. The pre-existing `gr*d_remove()` also lacked it, so this is not a regression.

- **No explicit suspend on removal:** The original `gr*d_exit()` called `pm_runtime_force_suspend()` to ensure the hardware was powered down when the client was torn down. After this patch, if the device is runtime-active when `gr*d_remove()` is called, `pm_runtime_disable()` leaves it active — the clocks stay enabled until devm cleanup. This is acceptable for driver removal but is a minor change in teardown behavior worth noting.

- **Error path in probe:** The error path for `host1x_client_register()` failure correctly calls `pm_runtime_disable()`, which is good:
  ```c
  err = host1x_client_register(&gr2d->client.base);
  if (err < 0) {
      pm_runtime_disable(dev);
      ...
  }
  ```

Overall, this is a well-structured fix for a real resume regression. The minor items above are observations, not blockers.

---
Generated by Claude Code Patch Reviewer