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: replace select with dependency for visible DEBUG_OBJECTS Date: Tue, 05 May 2026 08:46:43 +1000 Message-ID: In-Reply-To: <20260502191932.4491-1-julianbraha@gmail.com> References: <20260502191932.4491-1-julianbraha@gmail.com> <20260502191932.4491-1-julianbraha@gmail.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 **The change:** ```diff - select DEBUG_OBJECTS + depends on DEBUG_OBJECTS ``` **Issue 1 (functional regression =E2=80=94 blocker):** `DRM_I915_DEBUG` (Kc= onfig.debug:36) contains: ``` select DRM_I915_SW_FENCE_DEBUG_OBJECTS ``` With the old code, enabling `DRM_I915_DEBUG` would pull in `DRM_I915_SW_FEN= CE_DEBUG_OBJECTS`, which in turn pulled in `DEBUG_OBJECTS` via `select`. Th= e full debug chain worked automatically. After this patch, `DRM_I915_SW_FENCE_DEBUG_OBJECTS` now `depends on DEBUG_O= BJECTS`. When Kconfig processes the `select DRM_I915_SW_FENCE_DEBUG_OBJECTS= ` from `DRM_I915_DEBUG`, if `DEBUG_OBJECTS` is not already enabled, the `se= lect` is silently ignored because the dependency is unsatisfied. This means= a developer enabling `DRM_I915_DEBUG` (the umbrella debug option) will sil= ently lose sw_fence debug object tracking =E2=80=94 the opposite of the exp= ected behavior. To fix this properly, the author needs to also add `select DEBUG_OBJECTS` (= or `depends on DEBUG_OBJECTS`) to `DRM_I915_DEBUG` itself, so that the depe= ndency is satisfied before `DRM_I915_SW_FENCE_DEBUG_OBJECTS` is selected. F= or example: ``` config DRM_I915_DEBUG ... select DEBUG_OBJECTS select DRM_I915_SW_FENCE_DEBUG_OBJECTS ``` Or alternatively, `DRM_I915_DEBUG` could switch from `select DRM_I915_SW_FE= NCE_DEBUG_OBJECTS` to `select DRM_I915_SW_FENCE_DEBUG_OBJECTS if DEBUG_OBJE= CTS`, making the dependency chain explicit. **Issue 2 (minor =E2=80=94 usability):** `DEBUG_OBJECTS` itself has depende= ncies (`depends on PREEMPT_COUNT || !DEFERRED_STRUCT_PAGE_INIT` and `depend= s on DEBUG_KERNEL`). With the old `select`, these were irrelevant for the i= 915 path =E2=80=94 the symbol was force-enabled. Now users must ensure `DEB= UG_KERNEL` (and potentially `PREEMPT_COUNT`) are enabled for `DRM_I915_SW_F= ENCE_DEBUG_OBJECTS` to even appear in menuconfig. This may be intentional (= making the dependency explicit), but it changes the usability for developer= s who just want fence debugging without the full `DRM_I915_DEBUG` umbrella. **Nit:** The commit message says "Other config options use 'depends on' for= DEBUG_OBJECTS" =E2=80=94 this is true, but all those other options (`DEBUG= _OBJECTS_SELFTEST`, `DEBUG_OBJECTS_FREE`, etc. in `lib/Kconfig.debug`) are = children of the `DEBUG_OBJECTS` menu and are never force-selected by anythi= ng. The i915 case is different because of the `select` chain from `DRM_I915= _DEBUG`, so the analogy doesn't fully hold. --- Generated by Claude Code Patch Reviewer