Claude review: Re: [PATCH] drm/amdgpu: deduplicate ring preempt ib function

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

Patch Review

**Correctness: Good.** The extracted function in `amdgpu_gfx.c` is an exact copy of the removed functions from both `gfx_v11_0.c` and `gfx_v12_0.c`. The function signature matches the `preempt_ib` callback in `struct amdgpu_ring_funcs` (`int (*preempt_ib)(struct amdgpu_ring *ring)`), so assigning it directly to `.preempt_ib` in the ring_funcs structs is correct.

**Nit — trailing blank line:** The new function in `amdgpu_gfx.c` is followed by two blank lines before the end of the diff:

```c
+	return r;
+}
+
+
```

Kernel coding style uses a single blank line between functions (or at end-of-file). Remove the extra blank line.

**Nit — alignment inconsistency:** In the new shared function, the `kiq_unmap_queues` call arguments use tab+tabs alignment:

```c
+	kiq->pmf->kiq_unmap_queues(kiq_ring, ring, PREEMPT_QUEUES_NO_UNMAP,
+					ring->trail_fence_gpu_addr,
+					++ring->trail_seq);
```

The original code in both `gfx_v11_0.c` and `gfx_v12_0.c` aligned continuation lines to the opening parenthesis using tabs+spaces:

```c
	kiq->pmf->kiq_unmap_queues(kiq_ring, ring, PREEMPT_QUEUES_NO_UNMAP,
				   ring->trail_fence_gpu_addr,
				   ++ring->trail_seq);
```

The original alignment is the more common kernel style for this codebase. Since the goal is to move the code without modification, please preserve the original alignment.

**Missed opportunity — gfx_v10_0:** The `gfx_v10_0_ring_preempt_ib()` function at `gfx_v10_0.c:8855` is functionally identical to the new shared version, with only one difference: it lacks the `adev->enable_mes` early-return check. Since MES is not supported on GFX10 hardware, the `enable_mes` guard would be harmlessly false on v10 and the shared function could be used there as well. This would eliminate a third copy of the same logic. Worth considering for this patch or a follow-up.

**Not in scope but worth noting — gfx_v9_0:** The `gfx_v9_0` version uses a different fence emission mechanism (direct `gfx_v9_0_ring_emit_fence()` calls instead of `kiq_unmap_queues`), so it cannot be consolidated with this shared function. No action needed.

**Overall:** The patch does what it says on the tin and the logic is correct. Fix the trailing blank line and the alignment nit, and consider also converting `gfx_v10_0` to use the shared function since it's the same code.

---
Generated by Claude Code Patch Reviewer