Claude review: drm/xe/ggtt: hold FORCEWAKE while updating GGTT PTEs on LNL

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

Patch Review

**Code correctness:** The usage of `xe_with_force_wake()` is correct. The macro declares `fw_ref` as a scoped local variable, so no pre-declaration is needed. `gt_to_fw(ggtt->tile->primary_gt)` correctly gets the force wake handle from the primary GT, and `XE_FW_GT` is the right domain.

**`xe_ggtt_clear()` wrapping (lines 277–289):**
```c
xe_with_force_wake(fw_ref, gt_to_fw(ggtt->tile->primary_gt), XE_FW_GT) {
    while (start < end) {
        ggtt->pt_ops->ggtt_set_pte(ggtt, start, scratch_pte);
        start += XE_PAGE_SIZE;
    }
}
```
Looks correct. The comment is informative and explains the why clearly.

**`xe_ggtt_insert_node_transform()` wrapping (lines 783–795):**
```c
xe_with_force_wake(fw_ref, gt_to_fw(ggtt->tile->primary_gt), XE_FW_GT) {
    if (transform)
        transform(ggtt, node, pte_flags, ggtt->pt_ops->ggtt_set_pte, arg);
    else
        xe_ggtt_map_bo(ggtt, node, bo, pte_flags);
}
```
Correct. Note that the `transform` callback receives `ggtt_set_pte` and will call it inside the FORCEWAKE scope, which is the right thing.

**`__xe_ggtt_insert_bo_at()` wrapping (lines 867–870):**
```c
xe_with_force_wake(fw_ref, gt_to_fw(ggtt->tile->primary_gt), XE_FW_GT) {
    xe_ggtt_map_bo(ggtt, bo->ggtt_node[tile_id], bo, pte);
}
```
Correct.

**Nit — platform scoping:** The FORCEWAKE is taken unconditionally on all platforms, not just LNL. The commit message frames this as an LNL issue. If FORCEWAKE is cheap and idempotent on platforms that don't need it, this is fine (and is the simpler approach). But if the maintainers want it gated to affected platforms, a workaround check or platform conditional might be requested. The patch as submitted is the simpler and safer approach.

**Missing coverage of `xe_ggtt_map_bo_unlocked()`:** This function at line 735 calls `xe_ggtt_map_bo()` without FORCEWAKE protection:
```c
void xe_ggtt_map_bo_unlocked(struct xe_ggtt *ggtt, struct xe_bo *bo)
{
    ...
    xe_ggtt_map_bo(ggtt, bo->ggtt_node[ggtt->tile->id], bo, pte);
    ...
}
```
This is called from `xe_bo_evict.c:208` during post-suspend BO restore. If the rationale is that GSM writes can hang when the GT is in RC6, this path has the same exposure. It may warrant the same wrapping, or at minimum a comment explaining why it's safe without FORCEWAKE (e.g., if the resume path guarantees GT is awake at that point).

**Missing coverage of SR-IOV paths (`xe_ggtt_assign_locked`, `xe_ggtt_node_load`):** These write PTEs in loops without FORCEWAKE. They are SR-IOV specific and may be less likely to trigger in practice, but are theoretically vulnerable to the same issue. Lower priority since SR-IOV on LNL may not be a common configuration.

**Overall:** The patch is a reasonable, narrowly-scoped fix for a real hang. The code changes are correct. The main question for the maintainer is whether `xe_ggtt_map_bo_unlocked()` should also be covered, and whether the unconditional FORCEWAKE (rather than LNL-gated) is acceptable.

---
Generated by Claude Code Patch Reviewer