On Mon, 04 Aug 2025, Maarten Lankhorst <d...@lankhorst.se> wrote: > Hey, > > Den 2025-08-04 kl. 16:46, skrev Jani Nikula: >> On Thu, 24 Jul 2025, Maarten Lankhorst <d...@lankhorst.se> wrote: >>> drm_crtc_accurate_vblank_count takes a spinlock, which we should avoid >>> in tracepoints and debug functions. >>> >>> This also prevents taking the spinlock 2x during the critical >>> section of pipe updates. >> >> Don't we have vblank->max_vblank_count != 0 in most cases, and don't we >> want accuracy in the rest of the cases? >> >> The commit message should explain why it's okay to make this change. > > The whole point of tracepoints and debugging infra is that the functions > should > observe, not alter. Even for the vblank_count == 0 we should not be taking the > locking that we do. > > If you look at drm_update_vblank_count, that's way more than the simple > atomic64_read(&vblank->count) from drm_vblank_count. > > It takes the vblank_time_lock, loops potentially DRM_TIMESTAMP_MAXRETRIES > times, prints a few of drm_dbg() messages (which also has issues), and then > returns after updating. Being approximately correct is a win here.
It's really hard for me to say okay here when the one person I absolutely know will be using vblank tracing is Ville, and his comment was, "The non-accurate version is completely useless here." [1] BR, Jani. [1] https://lore.kernel.org/r/ahav7vtp2y1rb...@intel.com -- Jani Nikula, Intel
