Re: [PATCH RFC v3 6/7] gpu: nova-core: add basic timer device

["Alexandre Courbot" <acourbot@nvidia.com>]

On Sat Mar 22, 2025 at 1:20 AM JST, Daniel Brooks wrote:
> "Alexandre Courbot" <acourbot@nvidia.com> writes:
>
>> Hi Boqun,
>>
>> On Fri Mar 21, 2025 at 3:17 AM JST, Boqun Feng wrote:
>>> Also an Instant type has been proposed and reviewed for a while:
>>>
>>> 	https://lore.kernel.org/rust-for-linux/20250220070611.214262-5-fujita.tomonori@gmail.com/
>>>
>>> we should use that type instead of re-inventing the wheel here. Of
>>> course, it's currently not quite working because Instant is only for
>>> CLOCK_MONOTONIC. But there was a proposal to make `Instant` generic over
>>> clock:
>>>
>>> 	https://lore.kernel.org/rust-for-linux/20230714-rust-time-v2-1-f5aed84218c4@asahilina.net/
>>>
>>> if you follow that design, you can implement a `Instant<NovaGpu>`, where
>>>
>>>     ipml Now for NovaGpu {
>>>         fn now() -> Instant<Self> {
>>> 	    // your Timer::read() implementation.
>>> 	}
>>>     }
>>
>> Ah, thanks for pointing this out. I'll keep track of these patches,
>> hopefully they get merged soon!
>
> Would that actually work though? Instant is a ktime_t, which is a signed
> i64 rather than a u64. When I read your patch I assumed that you had to
> add your own Timestamp because the value had to be whatever was read
> from the GPU, independent of the clock value kept by the system or other
> hardware.

That's correct, and honestly there would be little benefit in using an
already existing type outside of not re-writing very similar code as GPU
time stamps are only used locally anyway. I'll keep an eye on these
patches though in case we can harmonize things a bit.