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: timer: Estimate vblank timeout if timer is disabled Date: Thu, 04 Jun 2026 14:03:59 +1000 Message-ID: In-Reply-To: <20260601141922.91498-6-tzimmermann@suse.de> References: <20260601141922.91498-1-tzimmermann@suse.de> <20260601141922.91498-6-tzimmermann@suse.de> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review **Same clock domain issue as Patch 3:** ```c *vblank_time =3D roundup(ktime_get_ns(), framedur_ns); ``` This returns a `CLOCK_MONOTONIC` value, while when the timer IS running (th= e `if (vblank->enabled)` branch), `*vblank_time` comes from `vtimer->timer.= node.expires` which is now `CLOCK_BOOTTIME`. The caller receives timestamps= from different clock domains depending on the code path. Should be `ktime_= get_boottime_ns()` to match Patch 3's timer clock. **Conceptual question:** The estimation assumes the frame-aligned phase est= ablished in `drm_crtc_vblank_start_timer` persists across timer disable/ena= ble cycles. But since the alignment is computed fresh at start time (`round= up(now, framedur_ns)`), the phase will shift each time the timer is restart= ed. The "estimate" won't match the actual timer phase that was or will be a= ctive. This might be acceptable as a best-effort approximation, but the com= mit message's claim that "timeouts are aligned to frame duration, so we can= round up to the next alignment" overstates the precision =E2=80=94 the pha= se is arbitrary. --- --- Generated by Claude Code Patch Reviewer