[PATCH 0/1] MR9866: Revert "ntdll: Switch to CLOCK_BOOTTIME for monotonic counters when available."
This reverts commit 3d373e452388b60bea9dca6d5c4cf3f8b20a0c54. CLOCK_MONOTONIC_RAW is used in vulkan to correlate QPC timestamps, and there's no equivalent of CLOCK_BOOTTIME there. Using CLOCK_BOOTTIME for QPC breaks the vulkan side with no proper way to fix it. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9866
From: Rémi Bernon <rbernon@codeweavers.com> This reverts commit 3d373e452388b60bea9dca6d5c4cf3f8b20a0c54. --- dlls/ntdll/unix/sync.c | 4 ++-- server/request.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/dlls/ntdll/unix/sync.c b/dlls/ntdll/unix/sync.c index e59f2113f5a..bfcf86bc61a 100644 --- a/dlls/ntdll/unix/sync.c +++ b/dlls/ntdll/unix/sync.c @@ -97,8 +97,8 @@ static inline ULONGLONG monotonic_counter(void) return mach_continuous_time() * timebase.numer / timebase.denom / 100; #elif defined(HAVE_CLOCK_GETTIME) struct timespec ts; -#ifdef CLOCK_BOOTTIME - if (!clock_gettime( CLOCK_BOOTTIME, &ts )) +#ifdef CLOCK_MONOTONIC_RAW + if (!clock_gettime( CLOCK_MONOTONIC_RAW, &ts )) return ts.tv_sec * (ULONGLONG)TICKSPERSEC + ts.tv_nsec / 100; #endif if (!clock_gettime( CLOCK_MONOTONIC, &ts )) diff --git a/server/request.c b/server/request.c index 432a5918892..835ea30cec3 100644 --- a/server/request.c +++ b/server/request.c @@ -515,8 +515,8 @@ timeout_t monotonic_counter(void) return mach_continuous_time() * timebase.numer / timebase.denom / 100; #elif defined(HAVE_CLOCK_GETTIME) struct timespec ts; -#ifdef CLOCK_BOOTTIME - if (!clock_gettime( CLOCK_BOOTTIME, &ts )) +#ifdef CLOCK_MONOTONIC_RAW + if (!clock_gettime( CLOCK_MONOTONIC_RAW, &ts )) return (timeout_t)ts.tv_sec * TICKS_PER_SEC + ts.tv_nsec / 100; #endif if (!clock_gettime( CLOCK_MONOTONIC, &ts )) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9866
If the problem is that a monotonic counter without suspend time is needed to implement VK_EXT_calibrated_timestamps (or whatever is needed there) I think it would be better to use `QueryUnbiasedInterruptTime` to obtain these. Granted there currently is a FIXME there for `InterruptTimeBias`, but that is trivially implemented. I think we should stay within documented spec, which is NT ticks and QPC and `QueryInterruptTime` with suspend time, and `QueryUnbiasedInterruptTime` without... How QPC currently behaves in Wine based on CLOCK_BOOTTIME is correct according to MSDN and also my personal testing.
QueryPerformanceCounter reads the performance counter and returns the total number of ticks that have occurred since the Windows operating system was started, including the time when the machine was in a sleep state such as standby, hibernate, or connected standby.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9866#note_126628
The problem is not that *some* monotonic counter is needed, it is that we can only implement the extension with the host provided monotonic counters that match the same clock source used on the CPU side. On Windows the only clock source is `VK_TIME_DOMAIN_QUERY_PERFORMANCE_COUNTER_EXT`, on Linux the only clock sources are `VK_TIME_DOMAIN_CLOCK_MONOTONIC_RAW_EXT` and `VK_TIME_DOMAIN_CLOCK_MONOTONIC_EXT`. The clock source used for QPC need to match the one returned here, so that what we return as `VK_TIME_DOMAIN_QUERY_PERFORMANCE_COUNTER_EXT` is correct. This was correct before, but it's been broken since QPC was changed to use CLOCK_BOOTTIME. Even if QPC is now implemented in a more correct way wrt suspend time, it broke the Vulkan side, and this is technically a regression. There's apparently no way to have both unless Vulkan adds a CLOCK_BOOTTIME domain, and it should be added before we can consider switching both QPC and Vulkan to CLOCK_BOOTIME. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9866#note_126631
participants (3)
-
Marc-Aurel Zent (＠mzent) -
Rémi Bernon
-
Rémi Bernon (＠rbernon)