diff --git a/drivers/gpu/drm/asahi/fw/workqueue.rs b/drivers/gpu/drm/asahi/fw/workqueue.rs index b86bd3ff3757a..78ac6c676a6eb 100644 --- a/drivers/gpu/drm/asahi/fw/workqueue.rs +++ b/drivers/gpu/drm/asahi/fw/workqueue.rs @@ -152,7 +152,7 @@ pub(crate) mod raw { pub(crate) unk_94: u32, pub(crate) pending: AtomicU32, pub(crate) unk_9c: u32, - pub(crate) gpu_context: GpuPointer<'a, super::GpuContextData>, + pub(crate) gpu_context: GpuWeakPointer, pub(crate) unk_a8: U64, #[ver(V >= V13_2 && G < G14X)] pub(crate) unk_b0: u32, @@ -171,6 +171,10 @@ impl GpuStruct for GpuContextData { type Raw<'a> = raw::GpuContextData; } +/// The firmware structures of a work queue. +/// +/// The raw structure points to the queue's GPU context weakly. The `GpuContext` keeps these +/// structures until it has been invalidated, and only then frees its own data. #[versions(AGX)] #[derive(Debug)] pub(crate) struct QueueInfo { @@ -178,7 +182,6 @@ pub(crate) struct QueueInfo { pub(crate) ring: GpuArray, pub(crate) gpu_buf: GpuArray, pub(crate) notifier_list: Arc>, - pub(crate) gpu_context: Arc, } #[versions(AGX)] diff --git a/drivers/gpu/drm/asahi/gpu.rs b/drivers/gpu/drm/asahi/gpu.rs index b68381ce60b30..62c049ff7dddc 100644 --- a/drivers/gpu/drm/asahi/gpu.rs +++ b/drivers/gpu/drm/asahi/gpu.rs @@ -260,9 +260,8 @@ pub(crate) struct GpuManager { stats: Arc, buffer_mgr: buffer::BufferManager::ver, ids: SequenceIDs, - #[allow(clippy::vec_box)] #[pin] - garbage_contexts: Mutex>>>, + garbage_contexts: Mutex>, } /// Operations the DRM driver performs on a GPU, independent of its firmware interface. @@ -401,8 +400,8 @@ 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 an unused context as garbage - fn free_context(&self, data: KBox>); + /// Register an unused context, with the objects it retains, for invalidation. + fn free_context(&self, ctx: workqueue::RetiredContext); } /// Private generic trait for functions that don't need to escape this module. @@ -1456,7 +1455,7 @@ impl GpuManager for GpuManager::ver { core::mem::swap(&mut *self.garbage_contexts.lock(), &mut garbage_ctx); for ctx in garbage_ctx { - if self.invalidate_context(&ctx).is_err() { + if self.invalidate_context(&ctx.data).is_err() { dev_err!( self.dev.as_ref(), "GpuContext: Failed to invalidate GPU context!\n" @@ -1465,6 +1464,9 @@ impl GpuManager for GpuManager::ver { panic!("GPU firmware timed out"); } } + // The firmware no longer knows the context's queues, so their structures may go now, + // ahead of the context data they point to. + core::mem::drop(ctx.retained); } let mut guard = self.alloc.lock(); @@ -1733,7 +1735,7 @@ impl GpuManager for GpuManager::ver { &self.dyncfg } - fn free_context(&self, ctx: KBox>) { + fn free_context(&self, ctx: workqueue::RetiredContext) { let mut garbage = self.garbage_contexts.lock(); if garbage.push(ctx, GFP_KERNEL).is_err() { diff --git a/drivers/gpu/drm/asahi/queue/mod.rs b/drivers/gpu/drm/asahi/queue/mod.rs index 2b32b97b5e169..2d9052c2edb51 100644 --- a/drivers/gpu/drm/asahi/queue/mod.rs +++ b/drivers/gpu/drm/asahi/queue/mod.rs @@ -525,6 +525,15 @@ impl Queue::ver { let buffer = buffer::Buffer::ver::new(gpu, alloc, ualloc.clone(), ualloc_priv, mgr)?; + let gpu_context = Arc::new( + workqueue::GpuContext::new(dev.clone(), alloc, buffer.any_ref())?, + GFP_KERNEL, + )?; + // Every command of this queue points the firmware at the notifier, and the notifier links + // into the notifier list of the work queues. Keep it as long as they are kept, until the + // context is invalidated, instead of freeing it with the last command. + gpu_context.retain(KBox::new(notifier.clone(), GFP_KERNEL)?)?; + let mut ret = Queue::ver { dev: dev.clone(), _sched: sched, @@ -538,11 +547,7 @@ impl Queue::ver { inner: QueueInner::ver { dev: dev.clone(), ualloc, - gpu_context: Arc::new( - workqueue::GpuContext::new(dev.clone(), alloc, buffer.any_ref())?, - GFP_KERNEL, - )?, - + gpu_context, buffer, notifier_list: Arc::new(notifier_list, GFP_KERNEL)?, notifier, diff --git a/drivers/gpu/drm/asahi/workqueue.rs b/drivers/gpu/drm/asahi/workqueue.rs index 6281ef684e251..d6695a1b3ee96 100644 --- a/drivers/gpu/drm/asahi/workqueue.rs +++ b/drivers/gpu/drm/asahi/workqueue.rs @@ -95,10 +95,31 @@ impl From for kernel::error::Error { } } +/// An object the firmware may still reference while its GPU context is live. +pub(crate) type RetainedObject = KBox; + +/// Objects retained by every context: the notifier and the vertex, fragment and compute work +/// queues of the owning queue. +const CONTEXT_RETAINED_OBJECTS: usize = 4; + +/// A dropped GPU context waiting for the firmware to invalidate it. +pub(crate) struct RetiredContext { + /// The context data named in the invalidation request. + pub(crate) data: KBox>, + /// Firmware structures of the context's queues. They must not be freed, and so must not be + /// reused, until the invalidation has completed. + pub(crate) retained: KVec, +} + /// A GPU context tracking structure, which must be explicitly invalidated when dropped. +/// +/// The firmware learns about the work queues of a context as they are run, and only forgets them +/// when the context is invalidated. Their firmware structures are therefore handed to the context +/// when their owner drops them, and freed after the invalidation. pub(crate) struct GpuContext { dev: driver::AsahiDevice, data: Option>>, + retained: Pin>>>, } no_debug!(GpuContext); @@ -118,27 +139,42 @@ impl GpuContext { )?, GFP_KERNEL, )?), + retained: KBox::pin_init( + new_mutex!( + KVec::with_capacity(CONTEXT_RETAINED_OBJECTS, GFP_KERNEL)?, + "GpuContext::retained" + ), + GFP_KERNEL, + )?, }) } - /// Returns the GPU pointer to the inner GPU context data structure. - pub(crate) fn gpu_pointer(&self) -> GpuPointer<'_, fw::workqueue::GpuContextData> { - self.data.as_ref().unwrap().gpu_pointer() + /// Returns a weak GPU pointer to the inner GPU context data structure. + pub(crate) fn weak_pointer(&self) -> GpuWeakPointer { + self.data.as_ref().unwrap().weak_pointer() + } + + /// Keep `object` allocated until the firmware has invalidated this context. + pub(crate) fn retain(&self, object: RetainedObject) -> Result { + self.retained.lock().push(object, GFP_KERNEL)?; + Ok(()) } } impl Drop for GpuContext { fn drop(&mut self) { + let data = self.data.take().unwrap(); + let retained = core::mem::take(&mut *self.retained.lock()); + let Some(guard) = self.dev.registration_guard() else { return; }; guard.registration_data_with(|reg_data| { - let data = self.data.take().unwrap(); // A context only exists for the manager whose allocators created it, so the manager is // always present. if let Some(gpu) = reg_data.gpu.manager() { - gpu.free_context(data); + gpu.free_context(RetiredContext { data, retained }); } }) } @@ -237,7 +273,10 @@ impl) + Send + Sync> GenSubm struct WorkQueueInner { dev: driver::AsahiDevice, event_manager: Arc, - info: GpuObject, + /// The firmware structures of this queue. Always `Some` until the queue is dropped, when they + /// move to `gpu_context`. + info: Option>>, + gpu_context: Arc, new: bool, pipe_type: PipeType, size: u32, @@ -265,10 +304,22 @@ pub(crate) struct WorkQueue { #[versions(AGX)] impl WorkQueueInner::ver { + fn info(&self) -> &GpuObject { + self.info + .as_deref() + .expect("WorkQueue: lost its queue info") + } + + fn info_mut(&mut self) -> &mut GpuObject { + self.info + .as_deref_mut() + .expect("WorkQueue: lost its queue info") + } + /// Return the GPU done pointer, representing how many work items have been completed by the /// GPU. fn doneptr(&self) -> u32 { - self.info + self.info() .state .with(|raw, _inner| raw.gpu_doneptr.load(Ordering::Acquire)) } @@ -485,7 +536,7 @@ impl Job::ver { let next_wptr = (wptr + 1) % inner.size; assert!(inner.doneptr() != next_wptr); - inner.info.ring[wptr as usize] = command.inner.gpu_va().get(); + inner.info_mut().ring[wptr as usize] = command.inner.gpu_va().get(); wptr = next_wptr; // Cannot fail, since we did a reserve(1) above @@ -515,7 +566,7 @@ impl<'a> JobSubmission::ver<'a> { core::mem::forget(self); inner - .info + .info() .state .with(|raw, _inner| raw.cpu_wptr.store(wptr, Ordering::Release)); @@ -527,7 +578,7 @@ impl<'a> JobSubmission::ver<'a> { let msg = fw::channels::RunWorkQueueMsg::ver { pipe_type: inner.pipe_type, - work_queue: Some(inner.info.weak_pointer()), + work_queue: Some(inner.info().weak_pointer()), wptr: inner.wptr, event_slot, is_new: inner.new, @@ -617,6 +668,23 @@ impl<'a> Drop for JobSubmission::ver<'a> { } } +#[versions(AGX)] +impl Drop for WorkQueueInner::ver { + fn drop(&mut self) { + // The firmware keeps referring to this queue until its context is invalidated. When the + // queue's owner went away with work in flight, the queue is dropped as soon as that work + // completes, which can be well before the invalidation. + if let Some(info) = self.info.take() { + if self.gpu_context.retain(info).is_err() { + pr_err!( + "WorkQueue({:?}): Failed to keep the queue until its context is invalidated\n", + self.pipe_type + ); + } + } + } +} + #[versions(AGX)] impl WorkQueueInner::ver { /// Return the number of free entries in the workqueue @@ -680,61 +748,65 @@ impl WorkQueue::ver { prio.5 = 1; } + let gpu_context_pointer = gpu_context.weak_pointer(); + + // Use shared (coherent) state with verbose faults so we can dump state correctly + let info = if debug_enabled(DebugFlags::VerboseFaults) { + &mut alloc.shared + } else { + &mut alloc.private + } + .new_init( + try_init!(QueueInfo::ver { + state: { + state.with_mut(|raw, _inner| { + raw.rb_size = size; + }); + state + }, + ring, + gpu_buf, + notifier_list: notifier_list, + }), + |inner, _p| { + try_init!(raw::QueueInfo::ver { + state: inner.state.gpu_pointer(), + ring: inner.ring.gpu_pointer(), + notifier_list: inner.notifier_list.gpu_pointer(), + gpu_buf: inner.gpu_buf.gpu_pointer(), + gpu_rptr1: Default::default(), + gpu_rptr2: Default::default(), + gpu_rptr3: Default::default(), + event_id: AtomicI32::new(-1), + priority: prio, + unk_4c: -1, + uuid: id as u32, + unk_54: -1, + unk_58: Default::default(), + busy: Default::default(), + __pad: Default::default(), + #[ver(V >= V13_2 && G < G14X)] + unk_84_0: 0, + unk_84_state: Default::default(), + error_count: Default::default(), + unk_8c: 0, + unk_90: 0, + unk_94: 0, + pending: Default::default(), + unk_9c: 0, + gpu_context: gpu_context_pointer, + unk_a8: Default::default(), + #[ver(V >= V13_2 && G < G14X)] + unk_b0: 0, + }) + }, + )?; + let inner = WorkQueueInner::ver { dev: dev.into(), event_manager, - // Use shared (coherent) state with verbose faults so we can dump state correctly - info: if debug_enabled(DebugFlags::VerboseFaults) { - &mut alloc.shared - } else { - &mut alloc.private - } - .new_init( - try_init!(QueueInfo::ver { - state: { - state.with_mut(|raw, _inner| { - raw.rb_size = size; - }); - state - }, - ring, - gpu_buf, - notifier_list: notifier_list, - gpu_context: gpu_context, - }), - |inner, _p| { - try_init!(raw::QueueInfo::ver { - state: inner.state.gpu_pointer(), - ring: inner.ring.gpu_pointer(), - notifier_list: inner.notifier_list.gpu_pointer(), - gpu_buf: inner.gpu_buf.gpu_pointer(), - gpu_rptr1: Default::default(), - gpu_rptr2: Default::default(), - gpu_rptr3: Default::default(), - event_id: AtomicI32::new(-1), - priority: prio, - unk_4c: -1, - uuid: id as u32, - unk_54: -1, - unk_58: Default::default(), - busy: Default::default(), - __pad: Default::default(), - #[ver(V >= V13_2 && G < G14X)] - unk_84_0: 0, - unk_84_state: Default::default(), - error_count: Default::default(), - unk_8c: 0, - unk_90: 0, - unk_94: 0, - pending: Default::default(), - unk_9c: 0, - gpu_context: inner.gpu_context.gpu_pointer(), - unk_a8: Default::default(), - #[ver(V >= V13_2 && G < G14X)] - unk_b0: 0, - }) - }, - )?, + info: Some(KBox::new(info, GFP_KERNEL)?), + gpu_context, new: true, pipe_type, size, @@ -751,7 +823,7 @@ impl WorkQueue::ver { last_submitted: None, }; - let info_pointer = inner.info.weak_pointer(); + let info_pointer = inner.info().weak_pointer(); Arc::pin_init( pin_init!(Self { @@ -835,7 +907,7 @@ impl WorkQueue::ver { pub(crate) fn dump_info(&self) { pr_info!("WorkQueue @ {:?}:", self.info_pointer); - self.inner.lock().info.with(|raw, _inner| { + self.inner.lock().info().with(|raw, _inner| { pr_info!(" GPU rptr1: {:#x}", raw.gpu_rptr1.load(Ordering::Relaxed)); pr_info!(" GPU rptr1: {:#x}", raw.gpu_rptr2.load(Ordering::Relaxed)); pr_info!(" GPU rptr1: {:#x}", raw.gpu_rptr3.load(Ordering::Relaxed)); @@ -953,7 +1025,7 @@ impl WorkQueue for WorkQueue::ver { ); inner - .info + .info() .state .with(|raw, _inner| raw.cpu_freeptr.store(last_wptr, Ordering::Release));