drm/asahi: Clean up jobs in a workqueue

This eliminates a potential deadlock under load and improves the fence
signaling situation (for when we have a shrinker).

Signed-off-by: Asahi Lina <lina@asahilina.net>

Author: hoshinolina

drivers/gpu/drm/asahi/gpu.rs

@@ -207,8 +207,6 @@ pub(crate) struct GpuManager {
     event_manager: Arc<event::EventManager>,
     buffer_mgr: buffer::BufferManager::ver,
     ids: SequenceIDs,
-    #[pin]
-    garbage_work: Mutex<Vec<Box<dyn workqueue::GenSubmittedWork>>>,
     #[allow(clippy::vec_box)]
     #[pin]
     garbage_contexts: Mutex<Vec<Box<fw::types::GpuObject<fw::workqueue::GpuContextData>>>>,
@@ -274,8 +272,6 @@ pub(crate) trait GpuManager: Send + Sync {
     fn get_cfg(&self) -> &'static hw::HwConfig;
     /// Get the dynamic GPU configuration for this SoC.
     fn get_dyncfg(&self) -> &hw::DynConfig;
-    /// Register completed work as garbage
-    fn add_completed_work(&self, work: Vec<Box<dyn workqueue::GenSubmittedWork>>);
     /// Register an unused context as garbage
     fn free_context(&self, data: Box<fw::types::GpuObject<fw::workqueue::GpuContextData>>);
     /// Check whether the GPU is crashed
@@ -718,7 +714,6 @@ impl GpuManager::ver {
                 pipes,
                 buffer_mgr,
                 ids: Default::default(),
-                garbage_work <- Mutex::new_named(Vec::new(), c_str!("garbage_work")),
                 garbage_contexts <- Mutex::new_named(Vec::new(), c_str!("garbage_contexts")),
             }),
             GFP_KERNEL,
@@ -1155,12 +1150,6 @@ impl GpuManager for GpuManager::ver {
     }
 
     fn alloc(&self) -> Guard<'_, KernelAllocators, MutexBackend> {
-        /*
-         * TODO: This should be done in a workqueue or something.
-         * Clean up completed jobs
-         */
-        self.garbage_work.lock().clear();
-
         /* Clean up idle contexts */
         let mut garbage_ctx = Vec::new();
         core::mem::swap(&mut *self.garbage_contexts.lock(), &mut garbage_ctx);
@@ -1484,24 +1473,6 @@ impl GpuManager for GpuManager::ver {
         &self.dyncfg
     }
 
-    fn add_completed_work(&self, work: Vec<Box<dyn workqueue::GenSubmittedWork>>) {
-        let mut garbage = self.garbage_work.lock();
-
-        if garbage.reserve(work.len(), GFP_KERNEL).is_err() {
-            dev_err!(
-                self.dev,
-                "Failed to reserve space for completed work, deadlock possible.\n"
-            );
-            return;
-        }
-
-        for i in work {
-            garbage
-                .push(i, GFP_KERNEL)
-                .expect("push() failed after reserve()");
-        }
-    }
-
     fn free_context(&self, ctx: Box<fw::types::GpuObject<fw::workqueue::GpuContextData>>) {
         let mut garbage = self.garbage_contexts.lock();
 

drivers/gpu/drm/asahi/workqueue.rs

@@ -34,6 +34,7 @@ use kernel::{
         Arc, Mutex,
     },
     uapi,
+    workqueue::{self, impl_has_work, new_work, Work, WorkItem},
 };
 
 const DEBUG_CLASS: DebugFlags = DebugFlags::WorkQueue;
@@ -173,6 +174,32 @@ pub(crate) trait GenSubmittedWork: Send + Sync {
     fn get_fence(&self) -> dma_fence::Fence;
 }
 
+#[pin_data]
+struct SubmittedWorkContainer {
+    #[pin]
+    work: Work<Self>,
+    inner: Box<dyn GenSubmittedWork>,
+}
+
+impl_has_work! {
+    impl HasWork<Self> for SubmittedWorkContainer { self.work }
+}
+
+impl WorkItem for SubmittedWorkContainer {
+    type Pointer = Pin<Box<SubmittedWorkContainer>>;
+
+    fn run(this: Pin<Box<SubmittedWorkContainer>>) {
+        mod_pr_debug!("WorkQueue: Freeing command @ {:?}\n", this.inner.gpu_va());
+    }
+}
+
+impl SubmittedWorkContainer {
+    fn inner_mut(self: Pin<&mut Self>) -> &mut Box<dyn GenSubmittedWork> {
+        // SAFETY: inner does not require structural pinning.
+        unsafe { &mut self.get_unchecked_mut().inner }
+    }
+}
+
 impl<O: OpaqueGpuObject, C: FnOnce(&mut O, Option<WorkError>) + Send + Sync> GenSubmittedWork
     for SubmittedWork<O, C>
 {
@@ -221,7 +248,7 @@ struct WorkQueueInner {
     pipe_type: PipeType,
     size: u32,
     wptr: u32,
-    pending: Vec<Box<dyn GenSubmittedWork>>,
+    pending: Vec<Pin<Box<SubmittedWorkContainer>>>,
     last_token: Option<event::Token>,
     pending_jobs: usize,
     last_submitted: Option<event::EventValue>,
@@ -270,7 +297,7 @@ pub(crate) struct Job {
     wq: Arc<WorkQueue::ver>,
     event_info: QueueEventInfo::ver,
     start_value: EventValue,
-    pending: Vec<Box<dyn GenSubmittedWork>>,
+    pending: Vec<Pin<Box<SubmittedWorkContainer>>>,
     committed: bool,
     submitted: bool,
     event_count: usize,
@@ -319,17 +346,23 @@ impl Job::ver {
             return Err(EINVAL);
         }
 
+        let fence = self.fence.clone();
+        let value = self.event_info.value.next();
+
         self.pending.push(
-            Box::new(
-                SubmittedWork::<_, _> {
-                    object: command,
-                    value: self.event_info.value.next(),
-                    error: None,
-                    callback: Some(callback),
-                    wptr: 0,
-                    vm_slot,
-                    fence: self.fence.clone(),
-                },
+            Box::try_pin_init(
+                try_pin_init!(SubmittedWorkContainer {
+                    work <- new_work!("SubmittedWorkWrapper::work"),
+                    inner: Box::new(SubmittedWork::<_, _> {
+                        object: command,
+                        value,
+                        error: None,
+                        callback: Some(callback),
+                        wptr: 0,
+                        vm_slot,
+                        fence,
+                    }, GFP_KERNEL)?
+                }),
                 GFP_KERNEL,
             )?,
             GFP_KERNEL,
@@ -376,7 +409,7 @@ impl Job::ver {
         if inner.free_slots() > self.event_count && inner.free_space() > self.pending.len() {
             None
         } else if let Some(work) = inner.pending.first() {
-            Some(work.get_fence())
+            Some(work.inner.get_fence())
         } else {
             pr_err!(
                 "WorkQueue: Cannot submit, but queue is empty? {} > {}, {} > {} (pend={} ls={:#x?} lc={:#x?}) ev={:#x?} cur={:#x?} slot {:?}\n",
@@ -454,11 +487,11 @@ impl Job::ver {
         );
 
         for mut command in self.pending.drain(..) {
-            command.set_wptr(wptr);
+            command.as_mut().inner_mut().set_wptr(wptr);
 
             let next_wptr = (wptr + 1) % inner.size;
             assert!(inner.doneptr() != next_wptr);
-            inner.info.ring[wptr as usize] = command.gpu_va().get();
+            inner.info.ring[wptr as usize] = command.inner.gpu_va().get();
             wptr = next_wptr;
 
             // Cannot fail, since we did a reserve(1) above
@@ -863,11 +896,11 @@ impl WorkQueue for WorkQueue::ver {
         let mut completed_commands: usize = 0;
 
         for cmd in inner.pending.iter() {
-            if cmd.value() <= value {
+            if cmd.inner.value() <= value {
                 mod_pr_debug!(
                     "WorkQueue({:?}): Command at value {:#x?} complete\n",
                     inner.pipe_type,
-                    cmd.value()
+                    cmd.inner.value()
                 );
                 completed_commands += 1;
             } else {
@@ -879,25 +912,17 @@ impl WorkQueue for WorkQueue::ver {
             return inner.pending.is_empty();
         }
 
-        let mut completed = Vec::new();
-
-        if completed.reserve(completed_commands, GFP_KERNEL).is_err() {
-            pr_crit!(
-                "WorkQueue({:?}): Failed to allocate space for {} completed commands\n",
-                inner.pipe_type,
-                completed_commands
-            );
-        }
-
+        let last_wptr = inner.pending[completed_commands - 1].inner.wptr();
         let pipe_type = inner.pipe_type;
 
-        for cmd in inner.pending.drain(..completed_commands) {
-            if completed.push(cmd, GFP_KERNEL).is_err() {
-                pr_crit!(
-                    "WorkQueue({:?}): Failed to signal a completed command\n",
-                    pipe_type,
-                );
-            }
+        for mut cmd in inner.pending.drain(..completed_commands) {
+            mod_pr_debug!(
+                "WorkQueue({:?}): Queueing command @ {:?} for cleanup\n",
+                pipe_type,
+                cmd.inner.gpu_va()
+            );
+            cmd.as_mut().inner_mut().complete();
+            workqueue::system().enqueue(cmd);
         }
 
         mod_pr_debug!(
@@ -909,12 +934,10 @@ impl WorkQueue for WorkQueue::ver {
             inner.last_completed,
         );
 
-        if let Some(i) = completed.last() {
-            inner
-                .info
-                .state
-                .with(|raw, _inner| raw.cpu_freeptr.store(i.wptr(), Ordering::Release));
-        }
+        inner
+            .info
+            .state
+            .with(|raw, _inner| raw.cpu_freeptr.store(last_wptr, Ordering::Release));
 
         let empty = inner.pending.is_empty();
         if empty && inner.pending_jobs == 0 {
@@ -923,16 +946,6 @@ impl WorkQueue for WorkQueue::ver {
             inner.last_completed = None;
         }
 
-        let dev = inner.dev.clone();
-        core::mem::drop(inner);
-
-        for cmd in completed.iter_mut() {
-            cmd.complete();
-        }
-
-        let gpu = &dev.data().gpu;
-        gpu.add_completed_work(completed);
-
         empty
     }
 
@@ -961,8 +974,8 @@ impl WorkQueue for WorkQueue::ver {
         );
 
         for cmd in inner.pending.iter_mut() {
-            if cmd.value() <= value {
-                cmd.mark_error(error);
+            if cmd.inner.value() <= value {
+                cmd.as_mut().inner_mut().mark_error(error);
             } else {
                 break;
             }
@@ -1003,8 +1016,8 @@ impl WorkQueue for WorkQueue::ver {
         core::mem::drop(inner);
 
         for mut cmd in cmds {
-            cmd.mark_error(error);
-            cmd.complete();
+            cmd.as_mut().inner_mut().mark_error(error);
+            cmd.as_mut().inner_mut().complete();
         }
     }
 }