drm/asahi: g17: stop cancelling behind a failed command once it has retired

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

@@ -606,6 +606,11 @@ impl Completion {
         drop(lease);
     }
 
+    /// Whether the command still holds its VM pin: firmware may still own or replay it.
+    pub(crate) fn retains_vm(&self) -> bool {
+        self.vm_job.lock().is_some()
+    }
+
     fn release_healthy(&self) {
         if let Some(sources) = &self.sources { sources.release(); }
         drop(self.render.lock().take());

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

@@ -25,6 +25,48 @@ pub(crate) fn replay_status(status: i32) -> Result {
     else { Ok(()) }
 }
 
+/// Whether a failed command may still run or write: it holds its VM pin until it has physically
+/// retired, or was never published.
+pub(crate) trait Retention {
+    fn retained(&self) -> bool;
+}
+
+/// The failed ordinals of one engine that later commands must not bypass. A command whose prefix
+/// covers one is cancelled, so none runs before a failed producer the firmware may still replay.
+/// Once that producer has retired nothing of it can run or write any more, so it stops counting:
+/// otherwise one cancelled command cancelled every later command of its queue for good.
+pub(crate) struct FailureRecord<T> {
+    retained: KVec<(u64, T)>,
+    /// A failure that could not be tracked: it keeps cancelling for good, as all failures once did.
+    untracked: Option<(u64, T)>,
+}
+
+impl<T: Retention + Clone> FailureRecord<T> {
+    pub(crate) const fn new() -> Self {
+        Self { retained: KVec::new(), untracked: None }
+    }
+
+    /// A finished frontier entry, at pruning.
+    pub(crate) fn note(&mut self, sequence: u64, status: i32, entry: &T) {
+        if status >= 0 || !entry.retained() {
+            return;
+        }
+        if self.retained.push((sequence, entry.clone()), GFP_KERNEL).is_err()
+            && self.untracked.as_ref().is_none_or(|(first, _)| sequence < *first)
+        {
+            self.untracked = Some((sequence, entry.clone()));
+        }
+    }
+
+    /// The earliest failure later prefixes must not bypass.
+    pub(crate) fn earliest(&mut self) -> Option<(u64, &T)> {
+        self.retained.retain(|(_, entry)| entry.retained());
+        Iterator::chain(self.retained.iter(), self.untracked.iter())
+            .min_by_key(|(sequence, _)| *sequence)
+            .map(|(sequence, entry)| (*sequence, entry))
+    }
+}
+
 pub(crate) struct Frontier {
     dependencies: [KickDependency; KICK_BARRIERS_MAX],
     sequences: [u64; KICK_BARRIERS_MAX],
@@ -89,3 +131,68 @@ impl Frontier {
         self.insert(sequence, dependency)
     }
 }
+
+#[kunit_tests(asahi_g17_dependency)]
+mod tests {
+    use super::*;
+
+    #[derive(Clone)]
+    struct Probe(kernel::sync::Arc<core::sync::atomic::AtomicBool>);
+
+    impl Probe {
+        fn new(retained: bool) -> Self {
+            Self(kernel::sync::Arc::new(core::sync::atomic::AtomicBool::new(retained), GFP_KERNEL).unwrap())
+        }
+        fn retire(&self) {
+            self.0.store(false, core::sync::atomic::Ordering::Relaxed);
+        }
+    }
+
+    impl Retention for Probe {
+        fn retained(&self) -> bool {
+            self.0.load(core::sync::atomic::Ordering::Relaxed)
+        }
+    }
+
+    fn earliest(record: &mut FailureRecord<Probe>) -> Option<u64> {
+        record.earliest().map(|(sequence, _)| sequence)
+    }
+
+    #[test]
+    fn a_cancelled_command_does_not_poison_its_queue() {
+        // Cancelled or failed before publication: it released its pin and can never run.
+        let mut record = FailureRecord::new();
+        record.note(5, ECANCELED.to_errno(), &Probe::new(false));
+        assert_eq!(earliest(&mut record), None);
+    }
+
+    #[test]
+    fn a_retained_failure_cancels_until_it_retires() {
+        let mut record = FailureRecord::new();
+        let failed = Probe::new(true);
+        record.note(5, EIO.to_errno(), &failed);
+        assert_eq!(earliest(&mut record), Some(5));
+        failed.retire();
+        assert_eq!(earliest(&mut record), None);
+    }
+
+    #[test]
+    fn a_later_retained_failure_survives_an_earlier_retirement() {
+        let mut record = FailureRecord::new();
+        let (first, second) = (Probe::new(true), Probe::new(true));
+        record.note(5, EIO.to_errno(), &first);
+        record.note(9, ECANCELED.to_errno(), &second);
+        assert_eq!(earliest(&mut record), Some(5));
+        first.retire();
+        assert_eq!(earliest(&mut record), Some(9));
+        second.retire();
+        assert_eq!(earliest(&mut record), None);
+    }
+
+    #[test]
+    fn successes_are_not_recorded() {
+        let mut record = FailureRecord::new();
+        record.note(5, 1, &Probe::new(true));
+        assert_eq!(earliest(&mut record), None);
+    }
+}

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

@@ -401,6 +401,13 @@ pub(crate) struct Fences {
     pub(crate) ready: Fence,
     pub(crate) completed: Fence,
     pub(crate) publication: Option<Arc<super::completion::Publication>>,
+    pub(crate) owner: Arc<Completion>,
+}
+
+impl super::dependency::Retention for Fences {
+    fn retained(&self) -> bool {
+        self.owner.retains_vm()
+    }
 }
 
 impl Fences {
@@ -479,6 +486,6 @@ impl<B: Backend> Scheduler<B> {
             self.backend.count_backlog(&packet);
         }
         job.push();
-        Ok(Fences { ready, completed, publication: host_publication })
+        Ok(Fences { ready, completed, publication: host_publication, owner: packet.completion.clone() })
     }
 }

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

@@ -10,6 +10,7 @@ use super::{
     command::{Parser, UscWindow, Validated},
     completion::{Completion, Destinations, PreparedPublication},
     context::Context,
+    dependency::FailureRecord,
     fence::Outputs,
     job::{self, Backend, Engine, Fences, Order, Packet, Scheduler},
     validation::Proof,
@@ -98,7 +99,7 @@ pub(crate) struct Queue<B: Backend> {
     /// Finished frontier entries pruned during this ioctl. Their last references
     /// can tear a whole completion down; that waits until the ioctl has published.
     finished: KVec<Fences>,
-    failures: [Option<(u64, Fence)>; 2],
+    failures: [FailureRecord<Fences>; 2],
     // Closing the userspace queue releases admission even if firmware still
     // retains one of its execution contexts or installed physical graphs.
     _slot: admission::QueueSlot,
@@ -137,7 +138,7 @@ impl<B: Backend> Queue<B> {
             sequence: [0; 2],
             frontiers: core::array::from_fn(|_| KVec::new()),
             finished: KVec::new(),
-            failures: [None, None],
+            failures: [FailureRecord::new(), FailureRecord::new()],
             _slot: slot,
         })
     }
@@ -172,11 +173,7 @@ impl<B: Backend> Queue<B> {
             if let Some(finished) = finished.as_mut() {
                 let _ = finished.push_within_capacity(fences.clone());
             }
-            // One earliest failed ordinal covers every later prefix without
-            // retaining its packet, mappings, or every failed fence.
-            if status < 0 && failure.as_ref().is_none_or(|(first, _)| *sequence < *first) {
-                *failure = Some((*sequence, fences.completed.clone()));
-            }
+            failure.note(*sequence, status, fences);
             false
         });
     }
@@ -224,9 +221,9 @@ impl<B: Backend> Queue<B> {
                 !Order::contains(order.wait_through[1], *sequence)
                     || fences.ready.raw() != fences.completed.raw()
                     || fences.status() > 0)
-            && self.failures.iter().enumerate().all(|(index, failure)|
-                failure.as_ref().is_none_or(|(sequence, _)|
-                    !Order::contains(order.wait_through[index], *sequence)))
+            && self.failures.iter_mut().enumerate().all(|(index, failure)|
+                failure.earliest().is_none_or(|(sequence, _)|
+                    !Order::contains(order.wait_through[index], sequence)))
         { self.backend.input_sources(inputs) } else { None };
         if engine == Engine::Compute {
             super::trace::inputs(self.context.id(), order.sequence, 1, inputs.len(), sources.as_deref());
@@ -284,11 +281,12 @@ impl<B: Backend> Queue<B> {
             .iter()
             .filter(|(_, fences)| fences.ready.raw() == fences.completed.raw())
             .count();
-        let failures: [Option<&Fence>; 2] = core::array::from_fn(|index| {
-            self.failures[index].as_ref().and_then(|(sequence, fence)| {
-                Order::contains(order.wait_through[index], *sequence).then_some(fence)
-            })
-        });
+        let mut failures: [Option<Fence>; 2] = [None, None];
+        for (index, failure) in self.failures.iter_mut().enumerate() {
+            failures[index] = failure.earliest().and_then(|(sequence, fences)| {
+                Order::contains(order.wait_through[index], sequence).then(|| fences.completed.clone())
+            });
+        }
         let failure_count = failures.iter().flatten().count();
         let mut dependencies =
             KVec::with_capacity(inputs.len() + count + timestamp_count + failure_count, GFP_KERNEL)?;