Take advantage of the lifetime-parameterized pci::Bar<'bound> to hold the BAR mapping directly in NovaCore<'bound>, and pass a borrowed reference to Gpu<'bound>.
This eliminates the Arc<Devres<Bar0>> indirection, removes runtime revocation checks for BAR access, and simplifies Gpu::unbind(). Signed-off-by: Danilo Krummrich <[email protected]> --- drivers/gpu/nova-core/driver.rs | 46 +++++++++++++++--------------- drivers/gpu/nova-core/gpu.rs | 27 +++++++----------- drivers/gpu/nova-core/nova_core.rs | 2 +- 3 files changed, 34 insertions(+), 41 deletions(-) diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs index 3f6d8cedf5ab..fb52d21a7d92 100644 --- a/drivers/gpu/nova-core/driver.rs +++ b/drivers/gpu/nova-core/driver.rs @@ -14,12 +14,9 @@ }, prelude::*, sizes::SZ_16M, - sync::{ - atomic::{ - Atomic, - Relaxed, // - }, - Arc, + sync::atomic::{ + Atomic, + Relaxed, // }, types::ForLt, }; @@ -30,9 +27,10 @@ static AUXILIARY_ID_COUNTER: Atomic<u32> = Atomic::new(0); #[pin_data] -pub(crate) struct NovaCore { +pub(crate) struct NovaCore<'bound> { #[pin] - pub(crate) gpu: Gpu, + pub(crate) gpu: Gpu<'bound>, + bar: pci::Bar<'bound, BAR0_SIZE>, #[allow(clippy::type_complexity)] _reg: Devres<auxiliary::Registration<ForLt!(())>>, } @@ -47,12 +45,14 @@ pub(crate) struct NovaCore { // DMA addresses. These systems should be quite rare. const GPU_DMA_BITS: u32 = 47; -pub(crate) type Bar0 = pci::Bar<'static, BAR0_SIZE>; +pub(crate) type Bar0 = kernel::io::Mmio<BAR0_SIZE>; + +pub(crate) struct NovaCoreDriver; kernel::pci_device_table!( PCI_TABLE, MODULE_PCI_TABLE, - <NovaCore as pci::Driver>::IdInfo, + <NovaCoreDriver as pci::Driver>::IdInfo, [ // Modern NVIDIA GPUs will show up as either VGA or 3D controllers. ( @@ -74,15 +74,15 @@ pub(crate) struct NovaCore { ] ); -impl pci::Driver for NovaCore { +impl pci::Driver for NovaCoreDriver { type IdInfo = (); - type Data<'bound> = Self; + type Data<'bound> = NovaCore<'bound>; const ID_TABLE: pci::IdTable<Self::IdInfo> = &PCI_TABLE; fn probe<'bound>( pdev: &'bound pci::Device<Core>, _info: &'bound Self::IdInfo, - ) -> impl PinInit<Self, Error> + 'bound { + ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound { pin_init::pin_init_scope(move || { dev_dbg!(pdev, "Probe Nova Core GPU driver.\n"); @@ -94,14 +94,14 @@ fn probe<'bound>( // other threads of execution. unsafe { pdev.dma_set_mask_and_coherent(DmaMask::new::<GPU_DMA_BITS>())? }; - let bar = Arc::new( - pdev.iomap_region_sized::<BAR0_SIZE>(0, c"nova-core/bar0")? - .into_devres()?, - GFP_KERNEL, - )?; - - Ok(try_pin_init!(Self { - gpu <- Gpu::new(pdev, bar.clone(), bar.access(pdev.as_ref())?), + Ok(try_pin_init!(NovaCore { + bar: pdev.iomap_region_sized::<BAR0_SIZE>(0, c"nova-core/bar0")?, + // TODO: Use `&bar` self-referential pin-init syntax once available. + // + // SAFETY: `bar` is initialized before this expression is evaluated + // (`try_pin_init!()` initializes fields in declaration order), lives at a pinned + // stable address, and is dropped after `gpu` (struct field drop order). + gpu <- Gpu::new(pdev, unsafe { &*core::ptr::from_ref(bar) }), _reg: auxiliary::Registration::new( pdev.as_ref(), c"nova-drm", @@ -115,7 +115,7 @@ fn probe<'bound>( }) } - fn unbind<'bound>(pdev: &'bound pci::Device<Core>, this: Pin<&'bound Self>) { - this.gpu.unbind(pdev.as_ref()); + fn unbind<'bound>(_pdev: &'bound pci::Device<Core>, this: Pin<&'bound Self::Data<'bound>>) { + this.gpu.unbind(); } } diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs index 9288b30fe4c3..03295fa6fb32 100644 --- a/drivers/gpu/nova-core/gpu.rs +++ b/drivers/gpu/nova-core/gpu.rs @@ -2,13 +2,11 @@ use kernel::{ device, - devres::Devres, fmt, io::Io, num::Bounded, pci, - prelude::*, - sync::Arc, // + prelude::*, // }; use crate::{ @@ -224,10 +222,10 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { /// Structure holding the resources required to operate the GPU. #[pin_data] -pub(crate) struct Gpu { +pub(crate) struct Gpu<'bound> { spec: Spec, - /// MMIO mapping of PCI BAR 0 - bar: Arc<Devres<Bar0>>, + /// MMIO mapping of PCI BAR 0. + bar: &'bound Bar0, /// System memory page required for flushing all pending GPU-side memory writes done through /// PCIE into system memory, via sysmembar (A GPU-initiated HW memory-barrier operation). sysmem_flush: SysmemFlush, @@ -240,10 +238,9 @@ pub(crate) struct Gpu { gsp: Gsp, } -impl Gpu { - pub(crate) fn new<'bound>( +impl<'bound> Gpu<'bound> { + pub(crate) fn new( pdev: &'bound pci::Device<device::Bound>, - devres_bar: Arc<Devres<Bar0>>, bar: &'bound Bar0, ) -> impl PinInit<Self, Error> + 'bound { try_pin_init!(Self { @@ -257,6 +254,8 @@ pub(crate) fn new<'bound>( .inspect_err(|_| dev_err!(pdev, "GFW boot did not complete\n"))?; }, + bar, + sysmem_flush: SysmemFlush::register(pdev.as_ref(), bar, spec.chipset)?, gsp_falcon: Falcon::new( @@ -270,19 +269,13 @@ pub(crate) fn new<'bound>( gsp <- Gsp::new(pdev), _: { gsp.boot(pdev, bar, spec.chipset, gsp_falcon, sec2_falcon)? }, - - bar: devres_bar, }) } /// Called when the corresponding [`Device`](device::Device) is unbound. /// /// Note: This method must only be called from `Driver::unbind`. - pub(crate) fn unbind(&self, dev: &device::Device<device::Core>) { - kernel::warn_on!(self - .bar - .access(dev) - .inspect(|bar| self.sysmem_flush.unregister(bar)) - .is_err()); + pub(crate) fn unbind(&self) { + self.sysmem_flush.unregister(self.bar); } } diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs index 04a1fa6b25f8..073d87714d3a 100644 --- a/drivers/gpu/nova-core/nova_core.rs +++ b/drivers/gpu/nova-core/nova_core.rs @@ -47,7 +47,7 @@ struct NovaCoreModule { // Fields are dropped in declaration order, so `_driver` is dropped first, // then `_debugfs_guard` clears `DEBUGFS_ROOT`. #[pin] - _driver: Registration<pci::Adapter<driver::NovaCore>>, + _driver: Registration<pci::Adapter<driver::NovaCoreDriver>>, _debugfs_guard: DebugfsRootGuard, } -- 2.54.0
