From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: Add support for a DRM backlight capability Date: Thu, 04 Jun 2026 14:54:30 +1000 Message-ID: In-Reply-To: <20260531114908.1693426-1-superm1@kernel.org> References: <20260531114908.1693426-1-superm1@kernel.org> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Overall Series Review Subject: Add support for a DRM backlight capability Author: "Mario Limonciello (AMD)" Patches: 12 Reviewed: 2026-06-04T14:54:30.211806 --- This is v5 of a significant series by Mario Limonciello adding DRM connecto= r-level backlight control via a new `LUMINANCE` property, replacing the leg= acy sysfs-only interface. The design is solid: connectors get an atomic pro= perty, a new `DRM_CLIENT_CAP_LUMINANCE` client capability inhibits legacy s= ysfs writes when a compositor takes over, and the backlight notifier is res= tored to allow cross-subsystem coordination. The series has working compositor support in KWin, Mutter, and wlroots, whi= ch is a strong signal. The overall architecture of a `drm_backlight` interm= ediary that links DRM connectors to `backlight_device` instances is clean. = Driver integration for amdgpu, i915/xe, and bridge connectors is provided. However, there are several issues ranging from potential locking bugs to in= complete implementations and UAPI concerns that should be addressed before = merge. **Key concerns:** 1. **Spinlock + sleeping calls**: `drm_backlight_lock` is a spinlock, but s= everal paths under it call functions that may sleep (kobject_uevent_env, ba= cklight notifier calls). 2. **Per-device vs global property range**: The luminance property is globa= l to the DRM device but the range is updated per-connector link, meaning mu= lti-eDP setups will clobber each other's ranges. 3. **Incomplete inhibit implementation** in patch 3 (stubs), then filled in= patch 4 =E2=80=94 the intermediate state is confusing for bisection. 4. **Notification loop risk**: `backlight_update_status` now calls `backlig= ht_notify_brightness` which notifies DRM, which may call back into backligh= t. 5. **drm_atomic_connector_commit_dpms signature change** =E2=80=94 the upst= ream function takes `struct drm_atomic_commit *state`, but the patch diff s= hows `struct drm_atomic_state *state`, suggesting this may be based on an o= lder tree. --- --- Generated by Claude Code Patch Reviewer