Wrap the DRM registration in Arc<Revocable<_>> and share it with the reset controller. If a reset fails, stop reset scheduling and revoke the registration. Dropping the registration calls drm_dev_unplug() which prevents new DRM critical sections and waits for existing ones before the registration data is released.
Signed-off-by: Onur Özkan <[email protected]> --- drivers/gpu/drm/tyr/driver.rs | 12 +++++++----- drivers/gpu/drm/tyr/reset.rs | 32 +++++++++++++++++++++----------- 2 files changed, 28 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs index 92ed0de0b4e4..03ad00a89420 100644 --- a/drivers/gpu/drm/tyr/driver.rs +++ b/drivers/gpu/drm/tyr/driver.rs @@ -22,6 +22,7 @@ prelude::*, regulator, regulator::Regulator, + revocable::Revocable, sizes::SZ_2M, sync::{ aref::ARef, @@ -56,7 +57,7 @@ pub(crate) struct TyrPlatformDriverData<'bound> { // ensure the correct drop order. #[pin] _reset: reset::ResetHandle<'bound>, - _reg: drm::Registration<'bound, TyrDrmDriver>, + _reg: Arc<Revocable<drm::Registration<'bound, TyrDrmDriver>>>, _device: ARef<TyrDrmDevice>, } @@ -161,17 +162,18 @@ fn probe<'bound>( gpu_info, }); - // SAFETY: `reg` is stored in the platform driver data and is not leaked or - // forgotten, so it is dropped before the `'bound` registration data can become - // invalid. + // SAFETY: `reg` is wrapped in a `Revocable` owned by the platform driver + // data and its reset handle. Neither is leaked or forgotten, so `reg` is + // dropped before the `'bound` registration data can become invalid. let reg = unsafe { drm::Registration::new_with_lt(pdev.as_ref(), unreg_dev, reg_data, 0)? }; let device = reg.device().into(); + let reg = Arc::pin_init(Revocable::new(reg), GFP_KERNEL)?; let driver = try_pin_init!(TyrPlatformDriverData { // SAFETY: `ResetHandle` is stored in platform driver data and is // dropped before the borrowed device and MMIO references expire. - _reset <- unsafe { reset::ResetHandle::new(pdev, hw)? }, + _reset <- unsafe { reset::ResetHandle::new(pdev, hw, reg.clone())? }, _reg: reg, _device: device, _: { diff --git a/drivers/gpu/drm/tyr/reset.rs b/drivers/gpu/drm/tyr/reset.rs index 1abcd25877d3..50690daec31a 100644 --- a/drivers/gpu/drm/tyr/reset.rs +++ b/drivers/gpu/drm/tyr/reset.rs @@ -28,12 +28,14 @@ Bound, Device, // }, + drm, io::{ poll, Io, // }, platform, prelude::*, + revocable::Revocable, sync::{ atomic::{ Atomic, @@ -53,7 +55,10 @@ }; use crate::{ - driver::IoMem, + driver::{ + IoMem, + TyrDrmDriver, // + }, gpu, regs::gpu_control::*, // }; @@ -87,6 +92,8 @@ struct Controller<'ctrl> { state: Atomic<ResetState>, /// Shared gate that coordinates hardware access with GPU reset. hw: Arc<HwGate<'ctrl>>, + /// DRM registration revoked when a reset fails. + registration: Arc<Revocable<drm::Registration<'ctrl, TyrDrmDriver>>>, } impl<'ctrl> ScopedWorkItem for Controller<'ctrl> { @@ -100,11 +107,13 @@ impl<'ctrl> Controller<'ctrl> { fn new( pdev: &'ctrl platform::Device<Bound>, hw: Arc<HwGate<'ctrl>>, + registration: Arc<Revocable<drm::Registration<'ctrl, TyrDrmDriver>>>, ) -> impl PinInit<Self, Error> { try_pin_init!(Self { pdev, state: Atomic::new(ResetState::Idle), hw, + registration, }) } @@ -119,8 +128,8 @@ fn try_transition(&self, from: ResetState, to: ResetState) -> bool { /// If the pending reset cannot be claimed, the worker returns immediately. /// /// It first claims [`ResetState::Pending`], then waits for earlier hardware - /// accesses to complete before issuing the reset and returning the worker - /// state to [`ResetState::Idle`]. + /// accesses to complete before issuing the reset. A successful reset returns + /// the worker state to [`ResetState::Idle`]; a failure unplugs the DRM device. /// /// Panthor reference: /// - drivers/gpu/drm/panthor/panthor_device.c::panthor_device_reset_work() @@ -134,16 +143,13 @@ fn reset_work(&self) { let reset_result = run_reset(self.pdev.as_ref(), &self.hw); if let Err(e) = reset_result { - dev_err!(self.pdev, "GPU reset failed: {:?}\n", e); - - // TODO: Unplug the GPU. - // There is no API for unplugging the GPU and this is unreachable - // for now since there are no hardware users for reset API. + dev_err!(self.pdev, "GPU reset failed, unplugging the GPU: {:?}\n", e); + let _ = self.try_transition(ResetState::InProgress, ResetState::ShuttingDown); + self.registration.revoke(); } else { dev_dbg!(self.pdev, "GPU reset completed.\n"); + let _ = self.try_transition(ResetState::InProgress, ResetState::Idle); } - - let _ = self.try_transition(ResetState::InProgress, ResetState::Idle); } } @@ -168,9 +174,13 @@ impl<'reset> ResetHandle<'reset> { pub(crate) unsafe fn new( pdev: &'reset platform::Device<Bound>, hw: Arc<HwGate<'reset>>, + registration: Arc<Revocable<drm::Registration<'reset, TyrDrmDriver>>>, ) -> Result<impl PinInit<Self, Error>> { Ok(try_pin_init!(Self { - controller <- kernel::new_scoped_work!("tyr::reset", Controller::new(pdev, hw)), + controller <- kernel::new_scoped_work!( + "tyr::reset", + Controller::new(pdev, hw, registration) + ), // SAFETY: The caller guarantees the handle is dropped. wq: unsafe { ScopedQueue::new(c"tyr-reset-wq")? }, })) -- 2.51.2
