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/vblank: Add vblank timer Date: Wed, 27 May 2026 14:49:44 +1000 Message-ID: In-Reply-To: <20260526133123.691465-2-w15303746062@163.com> References: <20260526133123.691465-1-w15303746062@163.com> <20260526133123.691465-2-w15303746062@163.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 Cherry-pick of upstream `74afeb8128502a529041a2566febd26053a7be11`. The core of the series. Introduces `struct drm_vblank_crtc_timer` and three= new exported functions: `drm_crtc_vblank_start_timer()`, `drm_crtc_vblank_= cancel_timer()`, and `drm_crtc_vblank_get_vblank_timeout()`. Also creates `= drm_vblank_helper.c` with wrapper callbacks for `drm_crtc_funcs`. The deadlock avoidance is the key design choice =E2=80=94 in `drm_crtc_vbla= nk_cancel_timer()`: ```c spin_lock_irqsave(&vtimer->interval_lock, flags); vtimer->interval =3D 0; spin_unlock_irqrestore(&vtimer->interval_lock, flags); hrtimer_try_to_cancel(&vtimer->timer); ``` Instead of `hrtimer_cancel()` (which waits and can deadlock), this signals = the timer to self-terminate by zeroing the interval and uses `hrtimer_try_t= o_cancel()`. The timer function checks: ```c if (!interval) return HRTIMER_NORESTART; ``` This is a correct pattern to avoid the ABBA deadlock between `vblank_time_l= ock` and `softirq_expiry_lock`. The comment in `drm_crtc_vblank_cancel_timer()` has a typo: `vblank_time_li= me_lock` instead of `vblank_time_lock`. This exists in the upstream commit = too, so it's not introduced by the backport. One pre-existing note on `drm_crtc_vblank_start_timer()`: the busy-wait `wh= ile (hrtimer_active(...)) hrtimer_try_to_cancel(...)` loop on the non-first= -invocation path could theoretically spin for a long time if the timer func= tion keeps restarting, but in practice the interval-zeroing from cancel ens= ures this converges. This is unchanged from upstream. **No issues with the backport itself.** --- Generated by Claude Code Patch Reviewer