[FFmpeg-devel] [PATCH] aarch64: Use cntvct_el0 as timer register on Android

[Rémi Denis-Courmont]

Le 7 juin 2024 12:12:45 GMT+03:00, "Martin Storsjö" <martin at martin.st> a écrit :
>The default timer register pmccntr_el0 usually requires enabling
>access with e.g. a kernel module.
>---
>cntvct_el0 has significantly better resolution than
>av_gettime_relative (while the unscaled nanosecond output of
>clock_gettime is much higher resolution).
>
>In one tested case, the cntvct_el0 timer has a frequency of 25 MHz
>(readable via the register cntfrq_el0).
>---
> libavutil/aarch64/timer.h | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
>diff --git a/libavutil/aarch64/timer.h b/libavutil/aarch64/timer.h
>index fadc9568f8..966f17081a 100644
>--- a/libavutil/aarch64/timer.h
>+++ b/libavutil/aarch64/timer.h
>@@ -33,7 +33,16 @@ static inline uint64_t read_time(void)
>     uint64_t cycle_counter;
>     __asm__ volatile(
>         "isb                   \t\n"
>+#if defined(__ANDROID__)
>+        // cntvct_el0 has lower resolution than pmccntr_el0, but is usually
>+        // accessible from user space by default.
>+        "mrs %0, cntvct_el0        "
>+#else
>+        // pmccntr_el0 has higher resolution, but is usually not accessible
>+        // from user space by default (but access can be enabled with a custom
>+        // kernel module).
>         "mrs %0, pmccntr_el0       "
>+#endif

It feels a little newbie-hostile choice to pick something that's broken by default but only on non-Android Linux. Is there at least a runtime warning?

Also technically this function is not specific to checkasm and is supposed to return time, so CNTVCT_EL0 actually seems more semantically correct.

IMO we should have checkasm read both cycle and time counters always and print both values but that's obviously beyond the scope here.

>         : "=r"(cycle_counter) :: "memory" );
> 
>     return cycle_counter;