drm/asahi: g17: keep innocent clients alive across another client's GPU stall or recovery

drivers/gpu/drm/asahi/driver.rs

@@ -341,6 +341,11 @@ impl AsahiPlatformDriver {
                 return Err(error);
             }
         };
+        if let Err(error) =
+            crate::sysfs_exports::register_idle(raw_dev, crate::g17::power::IDLE_HOLD_MS.as_ptr())
+        {
+            dev_warn!(pdev.as_ref(), "idle_off_delay_ms not registered: {:?}\n", error);
+        }
         Ok(AsahiPlatformDriverData {
             _device: reg.device().into(),
             _reg: reg,

drivers/gpu/drm/asahi/g17/completion.rs

@@ -242,6 +242,9 @@ impl Drop for PooledTimestamps {
     }
 }
 
+/// Republications of one command whose earlier publications firmware dropped unexecuted.
+pub(crate) const MAX_REPLAYS: u8 = 3;
+
 #[pin_data]
 pub(crate) struct Completion {
     status: Arc<VmStatus>,
@@ -385,7 +388,7 @@ impl Completion {
         let accepted = self
             .replays
             .fetch_update(Ordering::AcqRel, Ordering::Acquire, |count| {
-                (count < 3).then_some(count + 1)
+                (count < MAX_REPLAYS).then_some(count + 1)
             })
             .is_ok();
         if accepted {
@@ -464,6 +467,10 @@ impl Completion {
         self.class.load(Ordering::Acquire) == 1
     }
 
+    pub(crate) fn guilty(&self) -> bool {
+        self.class.load(Ordering::Acquire) == 2
+    }
+
     /// Installed before the first firmware-visible prefix. Cancellation takes
     /// the same lock after claiming the signal, so it cannot miss a late lease.
     pub(crate) fn attach_render_lease(
@@ -497,7 +504,7 @@ impl Completion {
     /// Called under the device mutex before deferring the error signal.
     pub(crate) fn quarantine_render(&self) {
         if let Some(lease) = self.render.lock().as_ref() {
-            lease.quarantine();
+            lease.quarantine(self.spared());
         }
     }
 
@@ -528,8 +535,11 @@ impl Completion {
             feed.note_render_pass(ticks[2], ticks[3]);
         }
         if let Err(error) = result {
+            // Spared work cancelled after another VM's recovery is not this client's device loss.
             if self.spared() {
-                self.status.report_failure(error);
+                if error != ECANCELED {
+                    self.status.report_failure(error);
+                }
             } else if error != ECANCELED {
                 self.status.record(error);
             }

drivers/gpu/drm/asahi/g17/job.rs

@@ -336,7 +336,7 @@ impl<B: Backend> sched::JobImpl for Job<B> {
         }
         let deferred = job.backend.defer_timeout(&job.packet, job.victim_renewals);
         if deferred {
-            job.victim_renewals += 1;
+            job.victim_renewals = job.victim_renewals.saturating_add(1);
         }
         deferred
     }

drivers/gpu/drm/asahi/g17/kick.rs

@@ -63,7 +63,7 @@ struct Identity {
 pub(crate) struct QueueIds {
     entries: [Option<Identity>; QID_COUNT],
     initial_render: [Option<Id>; 2],
-    compute_publication: u64,
+    publication: u64,
     leased: usize,
 }
 
@@ -72,7 +72,7 @@ impl QueueIds {
         kernel::try_init!(Self {
             entries <- pin_init::init_array_from_fn(|_| None),
             initial_render: [None; 2],
-            compute_publication: 0,
+            publication: 0,
             leased: 0,
         })
     }
@@ -174,10 +174,11 @@ impl QueueIds {
         Ok(id)
     }
 
-    /// Ordered under the device mutex; no atomic operation on the submission path.
-    pub(crate) fn next_compute_publication(&mut self) -> Result<u64> {
-        self.compute_publication = self.compute_publication.checked_add(1).ok_or(EOVERFLOW)?;
-        Ok(self.compute_publication)
+    /// Ordered under the device mutex; no atomic operation on the submission path. Compute and
+    /// render publications share it, so a watchdog can order work across both engines.
+    pub(crate) fn next_publication(&mut self) -> Result<u64> {
+        self.publication = self.publication.checked_add(1).ok_or(EOVERFLOW)?;
+        Ok(self.publication)
     }
 
     /// Call after preflighting the complete installation, before any address

drivers/gpu/drm/asahi/g17/mod.rs

@@ -36,11 +36,12 @@ mod job;
 mod capacity;
 mod kick;
 mod object;
-mod power;
+pub(crate) mod power;
 mod preparation;
 mod qos;
 mod queue;
 mod recovery;
+mod replay;
 mod runtime;
 pub(crate) mod status;
 mod teardown;
@@ -477,6 +478,7 @@ struct Firmware {
     rings: channel::Rings,
     recovery: recovery::State,
     effort: power::Effort,
+    hold: power::Hold,
     registration_power_release_pending: bool,
     queues: runtime::Registry,
     pool: Arc<object::Pool>,
@@ -520,6 +522,11 @@ impl Firmware {
     }
 
     fn release_registration_power(&mut self) -> Result {
+        // An idle hold keeps the cores powered past registration; it releases them itself.
+        if self.hold.active() {
+            self.registration_power_release_pending = false;
+            return Ok(());
+        }
         power::release(self)
     }
 
@@ -549,14 +556,49 @@ impl Firmware {
     /// Returns the delay in ns for a follow-up idle check. Queue owners report only submitted,
     /// unretired work; quarantined work retaining mappings is excluded.
     fn idle_check(&mut self, now_ns: u64, in_flight: bool) -> Result<Option<u64>> {
-        match self.effort.idle(now_ns, in_flight) {
-            power::IdleAction::None => Ok(None),
+        let effort = match self.effort.idle(now_ns, in_flight) {
+            power::IdleAction::None => None,
             power::IdleAction::Clear => {
                 self.apply_effort(0)?;
-                Ok(None)
+                None
+            }
+            power::IdleAction::Recheck(delay) => Some(delay),
+        };
+        Ok(match (effort, self.idle_hold(now_ns, in_flight)) {
+            (Some(a), Some(b)) => Some(a.min(b)),
+            (a, b) => a.or(b),
+        })
+    }
+
+    /// Drives the idle hold (see power::Hold); returns the delay in ns for its next check. A
+    /// refused control (recovery in progress) is retried on a later check.
+    fn idle_hold(&mut self, now_ns: u64, in_flight: bool) -> Option<u64> {
+        const RETRY_NS: u64 = 1_000_000;
+        let hold_ms = power::IDLE_HOLD_MS.load(Ordering::Relaxed);
+        for _ in 0..2 {
+            match self.hold.step(now_ns, in_flight, self.effort.submissions(), hold_ms) {
+                power::HoldAction::None => return None,
+                power::HoldAction::Recheck(delay) => return Some(delay),
+                power::HoldAction::Assert => {
+                    if self.set_idle_control(false).is_err() {
+                        return None;
+                    }
+                    self.hold.asserted();
+                }
+                power::HoldAction::Release => {
+                    if self.set_idle_control(true).is_err() {
+                        return Some(RETRY_NS);
+                    }
+                    self.hold.released();
+                    return None;
+                }
             }
-            power::IdleAction::Recheck(delay) => Ok(Some(delay)),
         }
+        None
+    }
+
+    fn set_idle_control(&mut self, enabled: bool) -> Result<u32> {
+        power::Control::set_idle(self, enabled)
     }
 }
 
@@ -980,6 +1022,7 @@ impl Gpu {
                 rings: channel::Rings::new(),
                 recovery: recovery::State::new(),
                 effort: power::Effort::new(),
+                hold: power::Hold::new(),
                 registration_power_release_pending: false,
                 queues <- runtime::Registry::new(shared.clusters, shared.descriptor_flags),
                 pool: object::Pool::new()?,

drivers/gpu/drm/asahi/g17/power.rs

@@ -3,9 +3,19 @@
 //! Work-based firmware performance effort. Idle power gating is managed separately by the
 //! device-control ring; this input never keeps an otherwise idle GPU powered.
 
+use core::sync::atomic::AtomicU32;
+
 pub(super) const FULL_EFFORT: u32 = 0x10000;
 const IDLE_HOLD_NS: u64 = 16_000_000;
 
+/// The firmware's idle power-off delay as measured on T6040/25G83: the cores power off 2 ms after
+/// the last job. Not measured on other firmware; at or below it the hold stays off.
+pub(crate) const FIRMWARE_IDLE_OFF_MS: u32 = 2;
+
+/// The `idle_off_delay_ms` sysfs knob. Above [`FIRMWARE_IDLE_OFF_MS`] the driver keeps the GPU
+/// cores powered until the GPU has been idle this long ([`Hold`]).
+pub(crate) static IDLE_HOLD_MS: AtomicU32 = AtomicU32::new(FIRMWARE_IDLE_OFF_MS);
+
 #[derive(Copy, Clone, Debug, PartialEq, Eq)]
 pub(super) enum IdleAction {
     None,
@@ -17,6 +27,7 @@ pub(super) struct Effort {
     streaming: bool,
     idle_since: Option<u64>,
     applied: u32,
+    submissions: u64,
 }
 
 impl Effort {
@@ -25,12 +36,19 @@ impl Effort {
             streaming: false,
             idle_since: None,
             applied: 0,
+            submissions: 0,
         }
     }
 
     pub(super) fn submit(&mut self) {
         self.streaming = true;
         self.idle_since = None;
+        self.submissions = self.submissions.wrapping_add(1);
+    }
+
+    /// Count of published submissions, for [`Hold::step`].
+    pub(super) fn submissions(&self) -> u64 {
+        self.submissions
     }
 
     pub(super) fn applied(&self) -> u32 {
@@ -62,6 +80,83 @@ impl Effort {
     }
 }
 
+#[derive(Copy, Clone, Debug, PartialEq, Eq)]
+pub(super) enum HoldAction {
+    None,
+    /// Send the keep-powered idle policy.
+    Assert,
+    /// Send the power-off idle policy again.
+    Release,
+    Recheck(u64),
+}
+
+/// Host-side idle hold. On T6040/25G83 the firmware powers the cores off 2 ms after the last job
+/// and reads that delay only at boot; every wake-up then costs about 1 ms. With a longer
+/// [`IDLE_HOLD_MS`] the host sends the keep-powered idle policy once work appears and the power-off
+/// policy once no work has been submitted or in flight for that long. Checked from the event
+/// worker, which runs on every completion, well inside the firmware's 2 ms.
+pub(super) struct Hold {
+    active: bool,
+    seen: u64,
+    since: Option<u64>,
+}
+
+impl Hold {
+    pub(super) const fn new() -> Self {
+        Self {
+            active: false,
+            seen: 0,
+            since: None,
+        }
+    }
+
+    pub(super) fn active(&self) -> bool {
+        self.active
+    }
+
+    pub(super) fn step(
+        &mut self,
+        now_ns: u64,
+        in_flight: bool,
+        submissions: u64,
+        hold_ms: u32,
+    ) -> HoldAction {
+        let enabled = hold_ms > FIRMWARE_IDLE_OFF_MS;
+        let fresh = submissions != self.seen;
+        self.seen = submissions;
+        if in_flight || fresh {
+            self.since = None;
+        }
+        if !self.active {
+            return if enabled && (in_flight || fresh) {
+                HoldAction::Assert
+            } else {
+                HoldAction::None
+            };
+        }
+        if in_flight {
+            return HoldAction::None;
+        }
+        let hold_ns = if enabled { u64::from(hold_ms) * 1_000_000 } else { 0 };
+        let since = *self.since.get_or_insert(now_ns.max(1));
+        let idle = now_ns.saturating_sub(since);
+        if idle >= hold_ns {
+            HoldAction::Release
+        } else {
+            HoldAction::Recheck(hold_ns - idle)
+        }
+    }
+
+    pub(super) fn asserted(&mut self) {
+        self.active = true;
+    }
+
+    pub(super) fn released(&mut self) {
+        self.active = false;
+        self.since = None;
+    }
+}
+
 /// Single-producer control path under the device mutex. Event draining must not publish another
 /// control between `set_idle` and the exact three-counter retirement witness.
 pub(super) trait Control {
@@ -137,3 +232,31 @@ pub(super) fn release<C: Control>(control: &mut C) -> kernel::error::Result {
     }
     result
 }
+
+#[cfg_attr(not(test), kernel::macros::kunit_tests(asahi_g17_idle_hold))]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn holds_until_idle_for_the_delay_then_releases() {
+        let mut hold = Hold::new();
+        assert_eq!(hold.step(0, true, 1, 10), HoldAction::Assert);
+        hold.asserted();
+        assert_eq!(hold.step(1_000_000, false, 1, 10), HoldAction::Recheck(10_000_000));
+        assert_eq!(hold.step(6_000_000, false, 1, 10), HoldAction::Recheck(5_000_000));
+        // A new submission restarts the idle period.
+        assert_eq!(hold.step(7_000_000, false, 2, 10), HoldAction::Recheck(10_000_000));
+        assert_eq!(hold.step(17_000_000, false, 2, 10), HoldAction::Release);
+        hold.released();
+        assert!(!hold.active());
+    }
+
+    #[test]
+    fn the_default_delay_never_holds_and_lowering_it_releases() {
+        let mut hold = Hold::new();
+        assert_eq!(hold.step(0, true, 1, FIRMWARE_IDLE_OFF_MS), HoldAction::None);
+        assert_eq!(hold.step(0, true, 2, 10), HoldAction::Assert);
+        hold.asserted();
+        assert_eq!(hold.step(1, false, 2, FIRMWARE_IDLE_OFF_MS), HoldAction::Release);
+    }
+}