Now that SysmemFlush can borrow the Bar via HRT lifetime, store a &'bound Bar0 reference and implement Drop to automatically unregister the sysmem flush page. This removes the need for manual unregister() calls and the Gpu::unbind() method.
Reported-by: Eliot Courtney <[email protected]> Closes: https://lore.kernel.org/all/[email protected]/ Fixes: 6554ad65b589 ("gpu: nova-core: register sysmem flush page") Signed-off-by: Danilo Krummrich <[email protected]> --- drivers/gpu/nova-core/driver.rs | 4 ---- drivers/gpu/nova-core/fb.rs | 22 ++++++++++------------ drivers/gpu/nova-core/gpu.rs | 9 +-------- 3 files changed, 11 insertions(+), 24 deletions(-) diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs index 11b632ae3789..31d2a8dadb7b 100644 --- a/drivers/gpu/nova-core/driver.rs +++ b/drivers/gpu/nova-core/driver.rs @@ -111,8 +111,4 @@ fn probe( })) }) } - - fn unbind(_pdev: &'bound pci::Device<Core>, this: Pin<&'bound Self>) { - this.gpu.unbind(); - } } diff --git a/drivers/gpu/nova-core/fb.rs b/drivers/gpu/nova-core/fb.rs index bdd5eed760e1..a707fbfe3ced 100644 --- a/drivers/gpu/nova-core/fb.rs +++ b/drivers/gpu/nova-core/fb.rs @@ -46,21 +46,20 @@ /// Because of this, the sysmem flush memory page must be registered as early as possible during /// driver initialization, and before any falcon is reset. /// -/// Users are responsible for manually calling [`Self::unregister`] before dropping this object, -/// otherwise the GPU might still use it even after it has been freed. -pub(crate) struct SysmemFlush { +pub(crate) struct SysmemFlush<'bound> { /// Chipset we are operating on. chipset: Chipset, device: ARef<device::Device>, + bar: &'bound Bar0, /// Keep the page alive as long as we need it. page: CoherentHandle, } -impl SysmemFlush { +impl<'bound> SysmemFlush<'bound> { /// Allocate a memory page and register it as the sysmem flush page. pub(crate) fn register( dev: &device::Device<device::Bound>, - bar: &Bar0, + bar: &'bound Bar0, chipset: Chipset, ) -> Result<Self> { let page = CoherentHandle::alloc(dev, kernel::page::PAGE_SIZE, GFP_KERNEL)?; @@ -70,19 +69,18 @@ pub(crate) fn register( Ok(Self { chipset, device: dev.into(), + bar, page, }) } +} - /// Unregister the managed sysmem flush page. - /// - /// In order to gracefully tear down the GPU, users must make sure to call this method before - /// dropping the object. - pub(crate) fn unregister(&self, bar: &Bar0) { +impl Drop for SysmemFlush<'_> { + fn drop(&mut self) { let hal = hal::fb_hal(self.chipset); - if hal.read_sysmem_flush_page(bar) == self.page.dma_handle() { - let _ = hal.write_sysmem_flush_page(bar, 0).inspect_err(|e| { + if hal.read_sysmem_flush_page(self.bar) == self.page.dma_handle() { + let _ = hal.write_sysmem_flush_page(self.bar, 0).inspect_err(|e| { dev_warn!( &self.device, "failed to unregister sysmem flush page: {:?}\n", diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs index 03295fa6fb32..c17d068b35eb 100644 --- a/drivers/gpu/nova-core/gpu.rs +++ b/drivers/gpu/nova-core/gpu.rs @@ -228,7 +228,7 @@ pub(crate) struct Gpu<'bound> { 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, + sysmem_flush: SysmemFlush<'bound>, /// GSP falcon instance, used for GSP boot up and cleanup. gsp_falcon: Falcon<GspFalcon>, /// SEC2 falcon instance, used for GSP boot up and cleanup. @@ -271,11 +271,4 @@ pub(crate) fn new( _: { gsp.boot(pdev, bar, spec.chipset, gsp_falcon, sec2_falcon)? }, }) } - - /// Called when the corresponding [`Device`](device::Device) is unbound. - /// - /// Note: This method must only be called from `Driver::unbind`. - pub(crate) fn unbind(&self) { - self.sysmem_flush.unregister(self.bar); - } } -- 2.54.0
