Claude review: drm/amd/display: Wrap DCN32 phantom-plane allocation in DC_RUN_WITH_PREEMPTION_ENABLED

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

Patch Review

**Positive aspects:**

- Excellent commit message with full root-cause analysis, stack trace, reproducer, and hardware verification.
- Correct identification of the problem: sleeping vmalloc called from softirq-disabled (FP) context.
- The `DC_RUN_WITH_PREEMPTION_ENABLED()` macro is the right mechanism — it's already used in `dc_create_stream_for_sink()` (dc_stream.c:180) and DML2 wrappers for exactly this purpose.

**Issue 1: Redundant `#if !defined` guard (minor, should remove)**

```c
+#if !defined(DC_RUN_WITH_PREEMPTION_ENABLED)
+#define DC_RUN_WITH_PREEMPTION_ENABLED(code) code
+#endif
```

This guard is unnecessary. The patch also adds `#include "dc_fpu.h"` (line 95), and `dc_fpu.h` unconditionally defines `DC_RUN_WITH_PREEMPTION_ENABLED` in all three branches of its preprocessor logic:
- With `CONFIG_DRM_AMD_DC_FP`: full implementation (dc_fpu.h:39-47)
- Without `CONFIG_DRM_AMD_DC_FP`: no-op `code` (dc_fpu.h:49)
- With `_LINUX_FPU_COMPILATION_UNIT`: no-op `code` (dc_fpu.h:54)

The `#if !defined` block can never trigger. It should be removed to avoid implying that the header might not define the macro.

**Issue 2: Fix should be in `dc_create_plane_state()`, not the call site (design concern)**

The analogous function `dc_create_stream_for_sink()` already wraps its own allocation internally:

```c
// dc_stream.c:180
DC_RUN_WITH_PREEMPTION_ENABLED(stream = kzalloc_obj(struct dc_stream_state, GFP_ATOMIC));
```

The fix should follow the same pattern by wrapping the allocation inside `dc_create_plane_state()` itself (dc_surface.c:89):

```c
struct dc_plane_state *dc_create_plane_state(const struct dc *dc)
{
    struct dc_plane_state *plane_state;
    DC_RUN_WITH_PREEMPTION_ENABLED(plane_state = kvzalloc_obj(*plane_state, GFP_ATOMIC));
    ...
}
```

This would protect **all** callers of `dc_create_plane_state()`, not just the one in `dcn32_enable_phantom_plane()`. Currently `dc_state_create_phantom_plane()` (dc_state.c:789) directly calls `dc_create_plane_state(dc)` — it's the only intermediate caller, but fixing at the allocator level is more robust and consistent with how `dc_create_stream_for_sink` handles it.

**Issue 3: `dc_fpu.h` include may already be unnecessary (very minor)**

The `#include "dc_fpu.h"` addition is only needed because the `#if !defined` guard needs to reference the macro. If the guard is dropped and the fix moves into `dc_create_plane_state()` (which is in dc_surface.c, where dc_fpu.h is presumably already included), then this include addition to dcn32_resource.c becomes unnecessary.

**No correctness issue with the current code:** The macro expansion with the `do { ... } while (0)` wrapper correctly assigns to the outer-scope `phantom_plane` variable, and the `if (!phantom_plane) continue;` check on the next line handles allocation failure. The fix as-written does work — the concerns above are about robustness and style.

---
Generated by Claude Code Patch Reviewer