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: Return success status from get_vblank_timeout Date: Thu, 04 Jun 2026 14:03:58 +1000 Message-ID: In-Reply-To: <20260601141922.91498-2-tzimmermann@suse.de> References: <20260601141922.91498-1-tzimmermann@suse.de> <20260601141922.91498-2-tzimmermann@suse.de> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Patch Review **Concept:** Good. Propagating failure instead of silently returning `ktime_get()` is the right thing. **Issue: vmwgfx caller not updated.** `vmw_vkms_get_vblank_timestamp()` at `drivers/gpu/drm/vmwgfx/vmwgfx_vkms.c:245` still calls `drm_crtc_vblank_get_vblank_timeout(crtc, vblank_time)` and ignores the return value, unconditionally returning `true`: ```c drm_crtc_vblank_get_vblank_timeout(crtc, vblank_time); return true; ``` After this patch, when the function returns `false`, `vblank_time` is uninitialized/stale, yet vmwgfx will report success to the vblank core. This needs the same treatment as `drm_crtc_vblank_helper_get_vblank_timestamp_from_timer`: ```c return drm_crtc_vblank_get_vblank_timeout(crtc, vblank_time); ``` **Minor:** The existing `drm_WARN_ON(crtc->dev, !ktime_compare(...))` path in the original code returned `void`, so it was a silent "give up." Now it returns `false`, which is correct behavior, but the `drm_WARN_ON` will print a kernel warning for a condition that may not truly be an error (the timer could have legitimately just expired). Consider whether a `drm_dbg_vbl` is more appropriate here. --- --- Generated by Claude Code Patch Reviewer