public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] drm/vkms: Clean up artifacts from old vblank timer
@ 2026-04-20 13:18 Thomas Zimmermann
  2026-04-22 23:36 ` Claude review: " Claude Code Review Bot
  2026-04-22 23:36 ` Claude Code Review Bot
  0 siblings, 2 replies; 3+ messages in thread
From: Thomas Zimmermann @ 2026-04-20 13:18 UTC (permalink / raw)
  To: louis.chauvet, hamohammed.sa, simona, melissa.srw,
	maarten.lankhorst, mripard, airlied
  Cc: dri-devel, Thomas Zimmermann

Vkms uses DRM's shared vblank timer. Do not include hrtimer.h and
fix comments referring to the original implementation. This should
have been done as part of commit 02e2681ffe1a ("drm/vkms: Convert
to DRM's vblank timer").

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Fixes: 02e2681ffe1a ("drm/vkms: Convert to DRM's vblank timer")
Cc: Louis Chauvet <louis.chauvet@bootlin.com>
Cc: Haneen Mohammed <hamohammed.sa@gmail.com>
Cc: Simona Vetter <simona@ffwll.ch>
Cc: Melissa Wen <melissa.srw@gmail.com>
Cc: dri-devel@lists.freedesktop.org
---
 drivers/gpu/drm/vkms/vkms_composer.c | 4 ++--
 drivers/gpu/drm/vkms/vkms_drv.h      | 5 -----
 2 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_composer.c b/drivers/gpu/drm/vkms/vkms_composer.c
index 83d217085ad0..db940eff43d7 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -595,7 +595,7 @@ static int compose_active_planes(struct vkms_writeback_job *active_wb,
  *
  * Work handler for composing and computing CRCs. work_struct scheduled in
  * an ordered workqueue that's periodically scheduled to run by
- * vkms_vblank_simulate() and flushed at vkms_atomic_commit_tail().
+ * vkms_crtc_handle_vblank_timeout() and flushed at vkms_atomic_commit_tail().
  */
 void vkms_composer_worker(struct work_struct *work)
 {
@@ -637,7 +637,7 @@ void vkms_composer_worker(struct work_struct *work)
 	spin_unlock_irq(&out->composer_lock);
 
 	/*
-	 * We raced with the vblank hrtimer and previous work already computed
+	 * We raced with the vblank timer and previous work already computed
 	 * the crc, nothing to do.
 	 */
 	if (!crc_pending)
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index 0933e4ce0ff0..3b5bdf39c031 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -3,8 +3,6 @@
 #ifndef _VKMS_DRV_H_
 #define _VKMS_DRV_H_
 
-#include <linux/hrtimer.h>
-
 #include <drm/drm.h>
 #include <drm/drm_framebuffer.h>
 #include <drm/drm_gem.h>
@@ -205,9 +203,6 @@ struct vkms_crtc_state {
  * @encoder: DRM encoder used for this output
  * @connector: DRM connector used for this output
  * @wb_connecter: DRM writeback connector used for this output
- * @vblank_hrtimer: Timer used to trigger the vblank
- * @period_ns: vblank period, in nanoseconds, used to configure @vblank_hrtimer and to compute
- *	       vblank timestamps
  * @composer_workq: Ordered workqueue for @composer_state.composer_work.
  * @lock: Lock used to protect concurrent access to the composer
  * @composer_enabled: Protected by @lock, true when the VKMS composer is active (crc needed or
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Claude review: drm/vkms: Clean up artifacts from old vblank timer
  2026-04-20 13:18 [PATCH] drm/vkms: Clean up artifacts from old vblank timer Thomas Zimmermann
@ 2026-04-22 23:36 ` Claude Code Review Bot
  2026-04-22 23:36 ` Claude Code Review Bot
  1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-04-22 23:36 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: drm/vkms: Clean up artifacts from old vblank timer
Author: Thomas Zimmermann <tzimmermann@suse.de>
Patches: 1
Reviewed: 2026-04-23T09:36:04.552455

---

This is a single cleanup patch from Thomas Zimmermann that removes leftover artifacts from the old vblank hrtimer implementation after the vkms driver was converted to DRM's shared vblank timer (commit `02e2681ffe1a`). The patch is straightforward and correct in what it does: removing the stale `#include <linux/hrtimer.h>`, removing kernel-doc for deleted struct fields (`vblank_hrtimer`, `period_ns`), and updating two comment references. However, **it misses a third stale "hrtimer" reference** in the same file.

**Verdict**: Needs a small fix before merging — one more comment should be updated.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Claude review: drm/vkms: Clean up artifacts from old vblank timer
  2026-04-20 13:18 [PATCH] drm/vkms: Clean up artifacts from old vblank timer Thomas Zimmermann
  2026-04-22 23:36 ` Claude review: " Claude Code Review Bot
@ 2026-04-22 23:36 ` Claude Code Review Bot
  1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-04-22 23:36 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Changes reviewed:**

1. **`vkms_drv.h`: Remove `#include <linux/hrtimer.h>`** — Correct. No struct members or function signatures in this header use `hrtimer_*` types after the conversion. The include is dead.

2. **`vkms_drv.h`: Remove kernel-doc for `vblank_hrtimer` and `period_ns`** — Correct. These fields were removed in `02e2681ffe1a` but their documentation was left behind. Removing orphaned kernel-doc entries is the right cleanup.

3. **`vkms_composer.c` line 597**: Comment update from `vkms_vblank_simulate()` to `vkms_crtc_handle_vblank_timeout()` — Correct. `vkms_crtc_handle_vblank_timeout` exists in `vkms_crtc.c:16` and is the current function name.

4. **`vkms_composer.c` line 639**: Comment update from "vblank hrtimer" to "vblank timer" — Correct.

**Issue — missed stale reference:**

There is a third "hrtimer" comment in `vkms_composer.c` at line 662 that was not updated:

```c
/*
 * The worker can fall behind the vblank hrtimer, make sure we catch up.
 */
```

This should be changed to "vblank timer" for consistency with the other comment fix in this same patch. Since the stated goal of the patch is to "fix comments referring to the original implementation," this seems like an oversight.

**Nit:** The `Fixes:` tag is arguably unnecessary here — this is a comment/include cleanup, not a functional fix. It won't cause backporting confusion since the referenced commit is recent, but `Fixes:` typically implies a behavioral/correctness issue that stable kernels should pick up. This is minor and a matter of project convention.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-04-22 23:36 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-20 13:18 [PATCH] drm/vkms: Clean up artifacts from old vblank timer Thomas Zimmermann
2026-04-22 23:36 ` Claude review: " Claude Code Review Bot
2026-04-22 23:36 ` Claude Code Review Bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox