[PATCH 2/2] drm/tyr: unplug the GPU when reset fails

[Onur Özkan]

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 <work@xxxxxxxxxxxxx>
---
 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