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: Fix timestamp calculation Date: Thu, 04 Jun 2026 14:03:58 +1000 Message-ID: In-Reply-To: <20260601141922.91498-3-tzimmermann@suse.de> References: <20260601141922.91498-1-tzimmermann@suse.de> <20260601141922.91498-3-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 **Concept:** Correct. The returned timestamp should be the start of the act= ive display period, not the start of the vblank period. Subtracting `active= dur_ns` instead of `framedur_ns` (i.e., `vtimer->interval`) achieves this. **The math:** ```c activedur_ns =3D div_s64(framedur_ns * mode->crtc_vdisplay, mode->crtc_vtot= al); *vblank_time =3D ktime_sub_ns(*vblank_time, activedur_ns); ``` `next_expiry - active_duration =3D start_of_active_period`. This is correct= geometry. **Observation on `drm_WARN_ON(dev, !mode->crtc_vtotal)`:** This is a good s= afety check, but after this returns `false`, the caller (`drm_crtc_vblank_h= elper_get_vblank_timestamp_from_timer`) now also returns `false`. Make sure= callers handle that gracefully =E2=80=94 this may trigger DRM's "failed to= get vblank timestamp" warnings. A `vtotal` of 0 indicates a fundamentally = broken mode, so warning is appropriate. **No issues with the logic itself.** This is a real bug fix for a long-stan= ding issue. --- --- Generated by Claude Code Patch Reviewer