From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: drm/i915/reset: Reorganize display reset code Date: Sun, 12 Apr 2026 12:04:47 +1000 Message-ID: In-Reply-To: <20260408233458.22666-3-ville.syrjala@linux.intel.com> References: <20260408233458.22666-1-ville.syrjala@linux.intel.com> <20260408233458.22666-3-ville.syrjala@linux.intel.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review This patch does two things: (1) extracts an `intel_display_reset_supported(= )` helper returning `HAS_DISPLAY(display)`, and (2) changes `intel_display_= reset_prepare()` from returning `bool` to `void`, moving the `HAS_DISPLAY` = guard into the caller (`intel_gt_reset_global`). Looking at the caller in `intel_reset.c`, after this patch the `reset_displ= ay` flag is no longer conditionally updated by `intel_display_reset_prepare= ()`'s return value =E2=80=94 it stays `true` once set, and `intel_display_r= eset_finish()` is always called when `reset_display` is true: ```c if (reset_display) intel_display_reset_prepare(display, display_reset_modeset_stuck, gt); intel_gt_reset(gt, engine_mask, reason); if (reset_display) intel_display_reset_finish(display, !need_display_reset); ``` In the old code, if `intel_display_reset_prepare()` returned `false` (only = happened when `!HAS_DISPLAY`), `reset_display` would be cleared and `intel_= display_reset_finish()` wouldn't be called. Now with `HAS_DISPLAY` checked = earlier via `intel_display_reset_supported()`, the `reset_display` flag cor= rectly guards both prepare and finish. However, `intel_display_reset_finish()` still has its own `if (!HAS_DISPLAY= (display)) return;` guard in the current tree version, which this patch rem= oves. That's fine because the caller already gates on `intel_display_reset_= supported()`. But there's a subtle behavioral change: previously when `intel_display_rese= t_prepare()` failed to duplicate state (returned `true` but set `modeset_st= ate =3D NULL`), `intel_display_reset_finish()` would still be called and wo= uld drop locks. Now with `void` return type, the same thing happens =E2=80= =94 this is preserved correctly since the `if (!state) goto unlock;` path i= n `_finish` handles this. **Looks correct.** One minor nit: `intel_display_reset_test()` now also checks `HAS_DISPLAY(di= splay)` redundantly =E2=80=94 the caller already gates on `intel_display_re= set_supported()`. Not a bug, just belt-and-suspenders. --- Generated by Claude Code Patch Reviewer