Alexandre Courbot
2025-Apr-20  12:19 UTC
[PATCH 10/16] gpu: nova-core: add basic timer device
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 at 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 at 0x00009400;
+    31:0    lo => as u32, "low 32-bits of the timer"
+);
+
+register!(PtimerTime1 at 0x00009410;
+    31:0    hi => as u32, "high 32 bits of the timer"
+);
+
 /* PFB */
 
 register!(PfbNisoFlushSysmemAddr at 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.0
Danilo Krummrich
2025-Apr-22  12:07 UTC
[PATCH 10/16] gpu: nova-core: add basic timer device
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?> > 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 >