Add a timer that works with GPU time and provides the ability to wait on
a condition with a specific timeout.
The `Duration` Rust type is used to keep track is differences between
timestamps ; this will be replaced by the equivalent kernel type once it
lands.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
drivers/gpu/nova-core/gpu.rs | 5 ++
drivers/gpu/nova-core/nova_core.rs | 1 +
drivers/gpu/nova-core/regs.rs | 10 +++
drivers/gpu/nova-core/timer.rs | 133 +++++++++++++++++++++++++++++++++++++
4 files changed, 149 insertions(+)
diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
index d43e710cc983d51f053dacbd77cbbfb79fa882c3..1b3e43e0412e2a2ea178c7404ea647c9e38d4e04 100644
--- a/drivers/gpu/nova-core/gpu.rs
+++ b/drivers/gpu/nova-core/gpu.rs
@@ -7,6 +7,7 @@
use crate::driver::Bar0;
use crate::firmware::Firmware;
use crate::regs;
+use crate::timer::Timer;
use crate::util;
use core::fmt;
@@ -153,6 +154,7 @@ pub(crate) struct Gpu {
bar: Devres<Bar0>,
fw: Firmware,
sysmem_flush: DmaObject,
+ timer: Timer,
}
#[pinned_drop]
@@ -217,11 +219,14 @@ pub(crate) fn new(
page
};
+ let timer = Timer::new();
+
Ok(pin_init!(Self {
spec,
bar,
fw,
sysmem_flush,
+ timer,
}))
}
}
diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
index 37c7eb0ea7a926bee4e3c661028847291bf07fa2..df3468c92c6081b3e2db218d92fbe1c40a0a75c3 100644
--- a/drivers/gpu/nova-core/nova_core.rs
+++ b/drivers/gpu/nova-core/nova_core.rs
@@ -26,6 +26,7 @@ macro_rules! with_bar {
mod firmware;
mod gpu;
mod regs;
+mod timer;
mod util;
kernel::module_pci_driver! {
diff --git a/drivers/gpu/nova-core/regs.rs b/drivers/gpu/nova-core/regs.rs
index 1e24787c4b5f432ac25fe399c8cb38b7350e44ae..f191cf4eb44c2b950e5cfcc6d04f95c122ce29d3 100644
--- a/drivers/gpu/nova-core/regs.rs
+++ b/drivers/gpu/nova-core/regs.rs
@@ -14,6 +14,16 @@
28:20 chipset => try_into Chipset, "chipset model"
);
+/* PTIMER */
+
+register!(PtimerTime0@0x00009400;
+ 31:0 lo => as u32, "low 32-bits of the timer"
+);
+
+register!(PtimerTime1@0x00009410;
+ 31:0 hi => as u32, "high 32 bits of the timer"
+);
+
/* PFB */
register!(PfbNisoFlushSysmemAddr@0x00100c10;
diff --git a/drivers/gpu/nova-core/timer.rs b/drivers/gpu/nova-core/timer.rs
new file mode 100644
index 0000000000000000000000000000000000000000..8987352f4192bc9b4b2fc0fb5f2e8e62ff27be68
--- /dev/null
+++ b/drivers/gpu/nova-core/timer.rs
@@ -0,0 +1,133 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Nova Core Timer subdevice
+
+// To be removed when all code is used.
+#![allow(dead_code)]
+
+use core::fmt::Display;
+use core::ops::{Add, Sub};
+use core::time::Duration;
+
+use kernel::devres::Devres;
+use kernel::num::U64Ext;
+use kernel::prelude::*;
+
+use crate::driver::Bar0;
+use crate::regs;
+
+/// A timestamp with nanosecond granularity obtained from the GPU timer.
+///
+/// A timestamp can also be substracted to another in order to obtain a [`Duration`].
+#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord)]
+pub(crate) struct Timestamp(u64);
+
+impl Display for Timestamp {
+ fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
+ write!(f, "{}", self.0)
+ }
+}
+
+impl Add<Duration> for Timestamp {
+ type Output = Self;
+
+ fn add(mut self, rhs: Duration) -> Self::Output {
+ let mut nanos = rhs.as_nanos();
+ while nanos > u64::MAX as u128 {
+ self.0 = self.0.wrapping_add(nanos as u64);
+ nanos -= u64::MAX as u128;
+ }
+
+ Timestamp(self.0.wrapping_add(nanos as u64))
+ }
+}
+
+impl Sub for Timestamp {
+ type Output = Duration;
+
+ fn sub(self, rhs: Self) -> Self::Output {
+ Duration::from_nanos(self.0.wrapping_sub(rhs.0))
+ }
+}
+
+pub(crate) struct Timer {}
+
+impl Timer {
+ pub(crate) fn new() -> Self {
+ Self {}
+ }
+
+ /// Read the current timer timestamp.
+ pub(crate) fn read(&self, bar: &Bar0) -> Timestamp {
+ loop {
+ let hi = regs::PtimerTime1::read(bar);
+ let lo = regs::PtimerTime0::read(bar);
+
+ if hi.hi() == regs::PtimerTime1::read(bar).hi() {
+ return Timestamp(u64::from_u32s(hi.hi(), lo.lo()));
+ }
+ }
+ }
+
+ #[allow(dead_code)]
+ pub(crate) fn time(bar: &Bar0, time: u64) {
+ regs::PtimerTime1::default()
+ .set_hi(time.upper_32_bits())
+ .write(bar);
+ regs::PtimerTime0::default()
+ .set_lo(time.lower_32_bits())
+ .write(bar);
+ }
+
+ /// Wait until `cond` is true or `timeout` elapsed, based on GPU time.
+ ///
+ /// When `cond` evaluates to `Some`, its return value is returned.
+ ///
+ /// `Err(ETIMEDOUT)` is returned if `timeout` has been reached without `cond` evaluating to
+ /// `Some`, or if the timer device is stuck for some reason.
+ pub(crate) fn wait_on<R, F: Fn() -> Option<R>>(
+ &self,
+ bar: &Devres<Bar0>,
+ timeout: Duration,
+ cond: F,
+ ) -> Result<R> {
+ // Number of consecutive time reads after which we consider the timer frozen if it hasn't
+ // moved forward.
+ const MAX_STALLED_READS: usize = 16;
+
+ let (mut cur_time, mut prev_time, deadline) = {
+ let cur_time = with_bar!(bar, |b| self.read(b))?;
+ let deadline = cur_time + timeout;
+
+ (cur_time, cur_time, deadline)
+ };
+ let mut num_reads = 0;
+
+ loop {
+ if let Some(ret) = cond() {
+ return Ok(ret);
+ }
+
+ (|| {
+ cur_time = with_bar!(bar, |b| self.read(b))?;
+
+ /* Check if the timer is frozen for some reason. */
+ if cur_time == prev_time {
+ if num_reads >= MAX_STALLED_READS {
+ return Err(ETIMEDOUT);
+ }
+ num_reads += 1;
+ } else {
+ if cur_time >= deadline {
+ return Err(ETIMEDOUT);
+ }
+
+ num_reads = 0;
+ prev_time = cur_time;
+ }
+
+ Ok(())
+ })()?;
+ }
+ }
+}
--
2.49.0On Sun, Apr 20, 2025 at 09:19:42PM +0900, Alexandre Courbot wrote:
> Add a timer that works with GPU time and provides the ability to wait on
> a condition with a specific timeout.
What can this timer do for us, what and HrTimer can't do for us?
>
> The `Duration` Rust type is used to keep track is differences between
> timestamps ; this will be replaced by the equivalent kernel type once it
> lands.
Fine for me -- can you please add a corresponding TODO and add it to your list
of follow-up patches?
> diff --git a/drivers/gpu/nova-core/timer.rs b/drivers/gpu/nova-core/timer.rs
> new file mode 100644
> index 0000000000000000000000000000000000000000..8987352f4192bc9b4b2fc0fb5f2e8e62ff27be68
> --- /dev/null
> +++ b/drivers/gpu/nova-core/timer.rs
> @@ -0,0 +1,133 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Nova Core Timer subdevice
> +
> +// To be removed when all code is used.
> +#![allow(dead_code)]
Please prefer 'expect'.
> +
> +use core::fmt::Display;
> +use core::ops::{Add, Sub};
> +use core::time::Duration;
> +
> +use kernel::devres::Devres;
> +use kernel::num::U64Ext;
> +use kernel::prelude::*;
> +
> +use crate::driver::Bar0;
> +use crate::regs;
> +
> +/// A timestamp with nanosecond granularity obtained from the GPU timer.
> +///
> +/// A timestamp can also be substracted to another in order to obtain a [`Duration`].
> +#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord)]
> +pub(crate) struct Timestamp(u64);
> +
> +impl Display for Timestamp {
> + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
> + write!(f, "{}", self.0)
> + }
> +}
> +
> +impl Add<Duration> for Timestamp {
> + type Output = Self;
> +
> + fn add(mut self, rhs: Duration) -> Self::Output {
> + let mut nanos = rhs.as_nanos();
> + while nanos > u64::MAX as u128 {
> + self.0 = self.0.wrapping_add(nanos as u64);
> + nanos -= u64::MAX as u128;
> + }
> +
> + Timestamp(self.0.wrapping_add(nanos as u64))
> + }
> +}
> +
> +impl Sub for Timestamp {
> + type Output = Duration;
> +
> + fn sub(self, rhs: Self) -> Self::Output {
> + Duration::from_nanos(self.0.wrapping_sub(rhs.0))
> + }
> +}
> +
> +pub(crate) struct Timer {}
> +
> +impl Timer {
> + pub(crate) fn new() -> Self {
> + Self {}
> + }
> +
> + /// Read the current timer timestamp.
> + pub(crate) fn read(&self, bar: &Bar0) -> Timestamp {
> + loop {
> + let hi = regs::PtimerTime1::read(bar);
> + let lo = regs::PtimerTime0::read(bar);
> +
> + if hi.hi() == regs::PtimerTime1::read(bar).hi() {
> + return Timestamp(u64::from_u32s(hi.hi(), lo.lo()));
> + }
So, if hi did not change since we've read both hi and lo, we can trust both
values. Probably worth to add a brief comment.
Additionally, we may want to add that if we get unlucky, it takes around 4s to
get unlucky again, even though that's rather obvious.
> + }
> + }
> +
> + #[allow(dead_code)]
> + pub(crate) fn time(bar: &Bar0, time: u64) {
> + regs::PtimerTime1::default()
> + .set_hi(time.upper_32_bits())
> + .write(bar);
> + regs::PtimerTime0::default()
> + .set_lo(time.lower_32_bits())
> + .write(bar);
> + }
> +
> + /// Wait until `cond` is true or `timeout` elapsed, based on GPU time.
> + ///
> + /// When `cond` evaluates to `Some`, its return value is returned.
> + ///
> + /// `Err(ETIMEDOUT)` is returned if `timeout` has been reached without `cond` evaluating to
> + /// `Some`, or if the timer device is stuck for some reason.
> + pub(crate) fn wait_on<R, F: Fn() -> Option<R>>(
> + &self,
> + bar: &Devres<Bar0>,
> + timeout: Duration,
> + cond: F,
> + ) -> Result<R> {
> + // Number of consecutive time reads after which we consider the timer frozen if it hasn't
> + // moved forward.
> + const MAX_STALLED_READS: usize = 16;
Huh! Can't we trust the timer hardware? Probably one reason more to use HrTimer?
> +
> + let (mut cur_time, mut prev_time, deadline) = {
> + let cur_time = with_bar!(bar, |b| self.read(b))?;
> + let deadline = cur_time + timeout;
> +
> + (cur_time, cur_time, deadline)
> + };
> + let mut num_reads = 0;
> +
> + loop {
> + if let Some(ret) = cond() {
> + return Ok(ret);
> + }
> +
> + (|| {
> + cur_time = with_bar!(bar, |b| self.read(b))?;
> +
> + /* Check if the timer is frozen for some reason. */
> + if cur_time == prev_time {
> + if num_reads >= MAX_STALLED_READS {
> + return Err(ETIMEDOUT);
> + }
> + num_reads += 1;
> + } else {
> + if cur_time >= deadline {
> + return Err(ETIMEDOUT);
> + }
> +
> + num_reads = 0;
> + prev_time = cur_time;
> + }
> +
> + Ok(())
> + })()?;
> + }
> + }
> +}
>
> --
> 2.49.0
>
On Tue Apr 22, 2025 at 9:07 PM JST, Danilo Krummrich wrote:
> On Sun, Apr 20, 2025 at 09:19:42PM +0900, Alexandre Courbot wrote:
>> Add a timer that works with GPU time and provides the ability to wait on
>> a condition with a specific timeout.
>
> What can this timer do for us, what and HrTimer can't do for us?
It is local to the GPU, and the source of truth for all GPU-related
operations. Some pushbuffer commands can return timestamps that will
come from this timer and the driver must thus use it as well in
driver-related operations to make sure both are on the same table.
>
>>
>> The `Duration` Rust type is used to keep track is differences between
>> timestamps ; this will be replaced by the equivalent kernel type once it
>> lands.
>
> Fine for me -- can you please add a corresponding TODO and add it to your list
> of follow-up patches?
Sure.
>
>> diff --git a/drivers/gpu/nova-core/timer.rs b/drivers/gpu/nova-core/timer.rs
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..8987352f4192bc9b4b2fc0fb5f2e8e62ff27be68
>> --- /dev/null
>> +++ b/drivers/gpu/nova-core/timer.rs
>> @@ -0,0 +1,133 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +//! Nova Core Timer subdevice
>> +
>> +// To be removed when all code is used.
>> +#![allow(dead_code)]
>
> Please prefer 'expect'.
Ack.
>
>> +
>> +use core::fmt::Display;
>> +use core::ops::{Add, Sub};
>> +use core::time::Duration;
>> +
>> +use kernel::devres::Devres;
>> +use kernel::num::U64Ext;
>> +use kernel::prelude::*;
>> +
>> +use crate::driver::Bar0;
>> +use crate::regs;
>> +
>> +/// A timestamp with nanosecond granularity obtained from the GPU timer.
>> +///
>> +/// A timestamp can also be substracted to another in order to obtain a [`Duration`].
>> +#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord)]
>> +pub(crate) struct Timestamp(u64);
>> +
>> +impl Display for Timestamp {
>> + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
>> + write!(f, "{}", self.0)
>> + }
>> +}
>> +
>> +impl Add<Duration> for Timestamp {
>> + type Output = Self;
>> +
>> + fn add(mut self, rhs: Duration) -> Self::Output {
>> + let mut nanos = rhs.as_nanos();
>> + while nanos > u64::MAX as u128 {
>> + self.0 = self.0.wrapping_add(nanos as u64);
>> + nanos -= u64::MAX as u128;
>> + }
>> +
>> + Timestamp(self.0.wrapping_add(nanos as u64))
>> + }
>> +}
>> +
>> +impl Sub for Timestamp {
>> + type Output = Duration;
>> +
>> + fn sub(self, rhs: Self) -> Self::Output {
>> + Duration::from_nanos(self.0.wrapping_sub(rhs.0))
>> + }
>> +}
>> +
>> +pub(crate) struct Timer {}
>> +
>> +impl Timer {
>> + pub(crate) fn new() -> Self {
>> + Self {}
>> + }
>> +
>> + /// Read the current timer timestamp.
>> + pub(crate) fn read(&self, bar: &Bar0) -> Timestamp {
>> + loop {
>> + let hi = regs::PtimerTime1::read(bar);
>> + let lo = regs::PtimerTime0::read(bar);
>> +
>> + if hi.hi() == regs::PtimerTime1::read(bar).hi() {
>> + return Timestamp(u64::from_u32s(hi.hi(), lo.lo()));
>> + }
>
> So, if hi did not change since we've read both hi and lo, we can trust both
> values. Probably worth to add a brief comment.
>
> Additionally, we may want to add that if we get unlucky, it takes around 4s to
> get unlucky again, even though that's rather obvious.
Added a comment. The odds of being unlucky are infinitesimal and the
consequences (an extra pass of this loop) inconsequential, thankfully.
>
>> + }
>> + }
>> +
>> + #[allow(dead_code)]
>> + pub(crate) fn time(bar: &Bar0, time: u64) {
>> + regs::PtimerTime1::default()
>> + .set_hi(time.upper_32_bits())
>> + .write(bar);
>> + regs::PtimerTime0::default()
>> + .set_lo(time.lower_32_bits())
>> + .write(bar);
>> + }
>> +
>> + /// Wait until `cond` is true or `timeout` elapsed, based on GPU time.
>> + ///
>> + /// When `cond` evaluates to `Some`, its return value is returned.
>> + ///
>> + /// `Err(ETIMEDOUT)` is returned if `timeout` has been reached without `cond` evaluating to
>> + /// `Some`, or if the timer device is stuck for some reason.
>> + pub(crate) fn wait_on<R, F: Fn() -> Option<R>>(
>> + &self,
>> + bar: &Devres<Bar0>,
>> + timeout: Duration,
>> + cond: F,
>> + ) -> Result<R> {
>> + // Number of consecutive time reads after which we consider the timer frozen if it hasn't
>> + // moved forward.
>> + const MAX_STALLED_READS: usize = 16;
>
> Huh! Can't we trust the timer hardware? Probably one reason more to use HrTimer?
No, to be clear I don't expect this to ever happen in real life, but I
also don't want to leave a loop without an exit condition.
OpenRM and Nouveau are both using it so I believe it can be trusted. :)
© 2016 - 2026 Red Hat, Inc.