Claude review: drm/loongson: stop to set get_vblank_timestamp for LS7A1000 CRTC

[Claude Code Review Bot <claude-review@example.com>]

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