* [PATCH v2] drm/loongson: stop to set get_vblank_timestamp for LS7A1000 CRTC
@ 2026-03-21 9:28 Icenowy Zheng
2026-03-21 9:29 ` Icenowy Zheng
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Icenowy Zheng @ 2026-03-21 9:28 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Jianmin Lv, Qianhai Wu, Huacai Chen, Mingcong Bai,
Xi Ruoyao
Cc: dri-devel, linux-kernel, Icenowy Zheng
As there's no known hardware capability about querying vblank on the
LS7A1000 display controller, setting get_vblank_timestamp will mislead the
kernel about the support of DC-backed high precision vblank query.
Drop this function pointer in the CRTC function table for LS7A1000.
This solves a kernel warning when booting Linux 7.0-rc3 on a
Loongson-3A4000+LS7A1000 Haier Boyue G51 laptop (with injected EDID for
replicating the display timing set by the firmware).
Signed-off-by: Icenowy Zheng <zhengxingda@iscas.ac.cn>
---
Changes in v2:
- Re-formatted Loongson product model numbers per request from Huacai.
drivers/gpu/drm/loongson/lsdc_crtc.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/gpu/drm/loongson/lsdc_crtc.c b/drivers/gpu/drm/loongson/lsdc_crtc.c
index 587fbe285e9ef..b3af8e0cdb15f 100644
--- a/drivers/gpu/drm/loongson/lsdc_crtc.c
+++ b/drivers/gpu/drm/loongson/lsdc_crtc.c
@@ -721,7 +721,6 @@ static const struct drm_crtc_funcs ls7a1000_crtc_funcs = {
.late_register = lsdc_crtc_late_register,
.enable_vblank = lsdc_crtc_enable_vblank,
.disable_vblank = lsdc_crtc_disable_vblank,
- .get_vblank_timestamp = drm_crtc_vblank_helper_get_vblank_timestamp,
.atomic_print_state = lsdc_crtc_atomic_print_state,
};
--
2.52.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v2] drm/loongson: stop to set get_vblank_timestamp for LS7A1000 CRTC
2026-03-21 9:28 [PATCH v2] drm/loongson: stop to set get_vblank_timestamp for LS7A1000 CRTC Icenowy Zheng
@ 2026-03-21 9:29 ` Icenowy Zheng
2026-03-21 17:09 ` Claude review: " Claude Code Review Bot
2026-03-21 17:09 ` Claude Code Review Bot
2 siblings, 0 replies; 4+ messages in thread
From: Icenowy Zheng @ 2026-03-21 9:29 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Jianmin Lv, Qianhai Wu, Huacai Chen, Mingcong Bai,
Xi Ruoyao
Cc: dri-devel, linux-kernel
在 2026-03-21六的 17:28 +0800,Icenowy Zheng写道:
> As there's no known hardware capability about querying vblank on the
> LS7A1000 display controller, setting get_vblank_timestamp will
> mislead the
> kernel about the support of DC-backed high precision vblank query.
>
> Drop this function pointer in the CRTC function table for LS7A1000.
>
> This solves a kernel warning when booting Linux 7.0-rc3 on a
> Loongson-3A4000+LS7A1000 Haier Boyue G51 laptop (with injected EDID
> for
> replicating the display timing set by the firmware).
>
> Signed-off-by: Icenowy Zheng <zhengxingda@iscas.ac.cn>
Oh forgot to attach when crafting v2:
```
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
```
> ---
> Changes in v2:
> - Re-formatted Loongson product model numbers per request from
> Huacai.
>
> drivers/gpu/drm/loongson/lsdc_crtc.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/loongson/lsdc_crtc.c
> b/drivers/gpu/drm/loongson/lsdc_crtc.c
> index 587fbe285e9ef..b3af8e0cdb15f 100644
> --- a/drivers/gpu/drm/loongson/lsdc_crtc.c
> +++ b/drivers/gpu/drm/loongson/lsdc_crtc.c
> @@ -721,7 +721,6 @@ static const struct drm_crtc_funcs
> ls7a1000_crtc_funcs = {
> .late_register = lsdc_crtc_late_register,
> .enable_vblank = lsdc_crtc_enable_vblank,
> .disable_vblank = lsdc_crtc_disable_vblank,
> - .get_vblank_timestamp =
> drm_crtc_vblank_helper_get_vblank_timestamp,
> .atomic_print_state = lsdc_crtc_atomic_print_state,
> };
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Claude review: drm/loongson: stop to set get_vblank_timestamp for LS7A1000 CRTC
2026-03-21 9:28 [PATCH v2] drm/loongson: stop to set get_vblank_timestamp for LS7A1000 CRTC Icenowy Zheng
2026-03-21 9:29 ` Icenowy Zheng
@ 2026-03-21 17:09 ` Claude Code Review Bot
2026-03-21 17:09 ` Claude Code Review Bot
2 siblings, 0 replies; 4+ messages in thread
From: Claude Code Review Bot @ 2026-03-21 17:09 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**The fix itself is correct in what it does** — removing `.get_vblank_timestamp` from the CRTC funcs table will cause the DRM core to fall back to default software-based vblank timestamping, avoiding the `drm_WARN_ON_ONCE()` at `drm_vblank.c:747` that fires when `crtc_clock == 0` during early boot (before mode is configured).
**However, I have a few concerns:**
1. **Root cause clarity**: The commit message says "no known hardware capability about querying vblank on the LS7A1000 display controller," but the driver currently has `get_scan_pos` ops populated in `ls7a1000_crtc_hw_ops` (at `lsdc_crtc.c:289,300`), and the shared `lsdc_crtc_helper_funcs` (used for both ls7a1000 and ls7a2000 at line 978) still sets `.get_scanout_position`. If the scan position registers truly don't work on ls7a1000, then `get_scan_pos` should also be removed from `ls7a1000_crtc_hw_ops` and ideally a separate `lsdc_crtc_helper_funcs` without `get_scanout_position` should be used for ls7a1000. Leaving dead code behind is confusing.
2. **Is the real problem just a timing issue?** The warning at `drm_vblank.c:747` fires when `mode->crtc_clock == 0`, which happens before mode is configured. This is a known issue pattern — it's not that the hardware can't do scanout position queries, it's that the timestamp function is called before the CRTC has a valid mode. The `ls7a2000_crtc_funcs` at line 739 still has `.get_vblank_timestamp` set — does ls7a2000 not hit this same warning? If so, the difference might be that ls7a2000 has `.get_vblank_counter` (line 736) which could affect the code path. This should be clarified.
3. **Incomplete cleanup**: If the scan position hardware truly doesn't work on ls7a1000, then `lsdc_crtc0_scan_pos` and `lsdc_crtc1_scan_pos` will still be called via the shared `lsdc_crtc_helper_funcs.get_scanout_position` in other code paths (e.g., `drm_crtc_accurate_vblank_count()` can use `.get_scanout_position` directly). A more thorough fix would also address the helper funcs.
4. **Minor**: The subject line "stop to set" reads oddly — "stop setting" or "remove" would be more natural English.
**Summary**: The one-line deletion will suppress the warning and is safe to apply. But the commit message's rationale about "no known hardware capability" contradicts the existing code that populates `get_scan_pos` for ls7a1000. I'd like clarification on whether the scan position registers are truly unreliable on ls7a1000, or whether this is just a timing issue during early boot. If the former, more cleanup is warranted; if the latter, a more targeted fix (or a better commit message) would be appropriate.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 4+ messages in thread
* Claude review: drm/loongson: stop to set get_vblank_timestamp for LS7A1000 CRTC
2026-03-21 9:28 [PATCH v2] drm/loongson: stop to set get_vblank_timestamp for LS7A1000 CRTC Icenowy Zheng
2026-03-21 9:29 ` Icenowy Zheng
2026-03-21 17:09 ` Claude review: " Claude Code Review Bot
@ 2026-03-21 17:09 ` Claude Code Review Bot
2 siblings, 0 replies; 4+ messages in thread
From: Claude Code Review Bot @ 2026-03-21 17:09 UTC (permalink / raw)
To: dri-devel-reviews
Overall Series Review
Subject: drm/loongson: stop to set get_vblank_timestamp for LS7A1000 CRTC
Author: Icenowy Zheng <zhengxingda@iscas.ac.cn>
Patches: 2
Reviewed: 2026-03-22T03:09:17.309159
---
This is a single-patch fix for a kernel warning on Loongson LS7A1000 hardware. The patch removes `.get_vblank_timestamp = drm_crtc_vblank_helper_get_vblank_timestamp` from `ls7a1000_crtc_funcs`, preventing the DRM core from attempting high-precision vblank timestamping on this CRTC.
The fix is reasonable as a pragmatic workaround but deserves some scrutiny — the LS7A1000 hardware **does** appear to have scan position registers (`LSDC_CRTC0_SCAN_POS_REG` / `LSDC_CRTC1_SCAN_POS_REG`) and the shared `lsdc_crtc_helper_funcs` still sets `.get_scanout_position = lsdc_crtc_get_scanout_position` for ls7a1000 CRTCs. So the hardware capability is there in some form.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-03-21 17:09 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-21 9:28 [PATCH v2] drm/loongson: stop to set get_vblank_timestamp for LS7A1000 CRTC Icenowy Zheng
2026-03-21 9:29 ` Icenowy Zheng
2026-03-21 17:09 ` Claude review: " Claude Code Review Bot
2026-03-21 17:09 ` 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