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

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

Patch Review

**Concern: The commit message rationale doesn't match the code.**

The commit message states:
> As there's no known hardware capability about querying vblank on the 7A1000 display controller

But the 7A1000 **does** have scanout position querying support. The `ls7a1000_crtc_hw_ops` array provides `.get_scan_pos` for both CRTCs (lines 289, 301), and the driver hooks `lsdc_crtc_get_scanout_position` (line 898) which calls `ops->get_scan_pos()` at line 923. The `ls7a1000_crtc_init` function at line 977 adds `lsdc_crtc_helper_funcs` (which includes `get_scanout_position`) to 7A1000 CRTCs. So the hardware and driver **do** support scanout position queries, which is all that `drm_crtc_vblank_helper_get_vblank_timestamp` needs.

The comment at lines 307-313 is more nuanced:
```c
/*
 * The 32-bit hardware vblank counter has been available since LS7A2000
 * and LS2K2000. The counter increases even though the CRTC is disabled,
 * it will be reset only if the CRTC is being soft reset.
 * Those registers are also readable for ls7a1000, but its value does not
 * change.
 */
```

This explains why 7A1000 doesn't have `.get_vblank_counter` (the hardware counter doesn't work), but scanout position is a different mechanism from the vblank counter — and the 7A1000 does support scanout position queries.

**Concern: This is likely a workaround, not a proper fix.**

The author reports a kernel warning on boot "with injected EDID for replicating the display timing set by the firmware." The warning is not included in the commit message, making it hard to assess the actual root cause. If the warning comes from `drm_calc_timestamping_constants()` or from `drm_crtc_vblank_helper_get_vblank_timestamp_internal()` due to invalid mode timing (e.g., `crtc_clock == 0`), then the proper fix would be to ensure correct timestamping constants are computed, not to disable high-precision vblank timestamps entirely.

**Concern: This degrades functionality for all 7A1000 users.**

Removing `get_vblank_timestamp` means the kernel falls back to `do_gettimeofday()`-based timestamps, which are much less precise. This affects compositors and applications that rely on accurate vblank timing (e.g., Wayland compositors, video playback). This is a regression for 7A1000 systems where scanout position queries work correctly.

**Recommendations:**

1. Include the actual kernel warning/backtrace in the commit message so reviewers can assess the root cause.
2. Investigate whether the issue is specific to the "injected EDID" scenario (incorrect `crtc_clock` or `crtc_htotal`/`crtc_vtotal` values) rather than a fundamental hardware limitation.
3. If `get_scan_pos` truly doesn't work on 7A1000 hardware, then `.get_scanout_position` should **also** be removed from the helper funcs for 7A1000 (perhaps by using a separate `ls7a1000_crtc_helper_funcs` without it). Leaving `get_scanout_position` in the helpers while removing `get_vblank_timestamp` from the CRTC funcs is inconsistent.
4. Note that the 7A2000 at line 738 retains both `.get_vblank_counter` and `.get_vblank_timestamp` — the difference being the working hardware vblank counter, but that is orthogonal to the scanout position mechanism used by `get_vblank_timestamp`.

---
Generated by Claude Code Patch Reviewer