From e0120892740e207070d9de5639a4c3771b1fc0fa Mon Sep 17 00:00:00 2001 From: Diljit Singh Date: Fri, 13 Nov 2026 10:18:25 -0500 Subject: [PATCH] drm/asahi: g17: refuse a sleep the commit can't stop Where the sleep's commit couldn't take the GPU's stop and the sleep driver didn't require it, .prepare stopped the firmware itself. A later refusal of that sleep then left the GPU stopped, with publication closed, until the next real sleep: the hole #1314 closes for G16. Refuse the sleep instead, as G16 does. On the A18 Pro, the only G17 chip that sleeps, the GPU's .prepare only runs for a sleep the sleep driver takes, and that leaves the stop to its commit, so the fallback wasn't reached. With it go the Rust stop and its tests. Signed-off-by: Diljit Singh --- drivers/gpu/drm/asahi/g17/sleep.rs | 162 +++-------------------------- 1 file changed, 16 insertions(+), 146 deletions(-) diff --git a/drivers/gpu/drm/asahi/g17/sleep.rs b/drivers/gpu/drm/asahi/g17/sleep.rs index fe5779ee5d5e1..ec0dde6e555bd 100644 --- a/drivers/gpu/drm/asahi/g17/sleep.rs +++ b/drivers/gpu/drm/asahi/g17/sleep.rs @@ -14,9 +14,10 @@ //! stopped and publication closed until the next real sleep, with the domain on so that everything //! behind it stays reachable. That sleep then only has the domain left to lower. //! -//! Where the sleep driver takes it, `.prepare` only drains and checks for quiet, and the sleep's -//! commit makes the whole stop in C, with interrupts off and every refusal behind it. A sleep -//! refused before then finds the firmware never stopped. Elsewhere `.prepare` stops it. +//! `.prepare` only drains and checks for quiet, and the sleep's commit makes the whole stop in C, +//! with interrupts off and every refusal behind it. A sleep refused before then finds the firmware +//! never stopped. Where the commit can't take the stop, `.prepare` refuses the sleep: a stop of its +//! own would leave the firmware stopped after any later refusal, until the next real sleep. use super::{Firmware, Role, Shared}; use core::sync::atomic::Ordering; @@ -47,7 +48,6 @@ unsafe extern "C" { bridge: bool, ) -> *mut core::ffi::c_void; fn asahi_g17_commit_finish(commit: *mut core::ffi::c_void, saved: *mut u32) -> bool; - fn asahi_g17_commit_required() -> bool; } /// What `.complete` does after a sleep whose stop was left to the commit. @@ -55,7 +55,7 @@ unsafe extern "C" { enum AfterCommit { /// Refused before the commit: the firmware never stopped, so only publication reopens. Running, - /// Stopped at the commit, or its power lost anyway: as after [`down`]. + /// Stopped at the commit, or its power lost anyway: the firmware has to restart. Stopped, } @@ -67,15 +67,10 @@ fn after_commit(stopped: bool, real: bool) -> AfterCommit { } } -/// What stopping and restarting the firmware touches, in the order [`down`] and [`up`] use it. +/// What restarting the firmware and lowering or raising its domain touches, in the order [`up`] +/// uses it. The stop itself is the sleep's commit's. pub(super) trait Hardware { fn has_secondary(&self) -> bool; - /// RTKit AP and IOP idle, each acknowledged. - fn idle(&mut self, role: Role) -> Result; - /// Back from idle with no restart, after a refusal. One that fails loses the device. - fn wake(&mut self, role: Role); - /// Mailbox off, then run bit off, for an instance that acknowledged idle. - fn stop(&mut self, role: Role); /// The PMP's request for the GPU. It may use a device it's told is in use, so the request is /// only ever set with the domain on. fn request(&mut self, enable: bool); @@ -97,32 +92,6 @@ pub(super) trait Hardware { fn power_on_done(&mut self) -> Result; } -/// Stops the firmware and lowers the domain. An error leaves the firmware running, so the caller -/// refuses the sleep. Both instances idle before either stops, so that a refusal by the second -/// still has a way back. A domain that is slow to go off isn't an error here: the sleep driver -/// checks it at the commit. -pub(super) fn down(hw: &mut H) -> Result { - if let Err(error) = hw.idle(Role::Primary) { - hw.wake(Role::Primary); - return Err(error); - } - if hw.has_secondary() { - if let Err(error) = hw.idle(Role::Secondary) { - hw.wake(Role::Secondary); - hw.wake(Role::Primary); - return Err(error); - } - } - hw.stop(Role::Primary); - let _ = hw.gating(false); - if hw.has_secondary() { - hw.stop(Role::Secondary); - } - hw.request(false); - lower_domain(hw); - Ok(()) -} - /// Lowers the domain, with the firmware stopped. fn lower_domain(hw: &mut H) { let _ = hw.auto_off(); @@ -155,28 +124,6 @@ impl Hardware for Firmware { self.secondary.is_some() } - fn idle(&mut self, role: Role) -> Result { - self.coprocessor(role)?.rtkit.with(|rtkit| rtkit.idle()) - } - - fn wake(&mut self, role: Role) { - let result = match self.coprocessor(role) { - Ok(coprocessor) => coprocessor.rtkit.with(|rtkit| rtkit.wake()), - Err(error) => Err(error), - }; - // EINVAL: it never left the on state. - if result.is_err_and(|error| error != EINVAL) { - self.primary.state.shared.lose_device_quietly(); - } - } - - fn stop(&mut self, role: Role) { - if let Ok(coprocessor) = self.coprocessor(role) { - coprocessor.rtkit.with(|rtkit| rtkit.mailbox_off()); - coprocessor.stop(); - } - } - fn request(&mut self, enable: bool) { self.primary.state.shared.pmp_request(enable); } @@ -428,18 +375,16 @@ impl Shared { true } - /// Leaves the stop to the commit. False, with the reason logged, if it can't be. Where only - /// the commit may stop it, that refuses the sleep instead: a refusal after an earlier stop - /// would leave it stopped. - fn sleep_leave_for_commit(&self, firmware: &mut Firmware) -> Result { + /// Leaves the stop to the commit, or refuses the sleep if the commit can't take it: a stop + /// here would leave the firmware stopped after any later refusal, until the next real sleep. + fn sleep_leave_for_commit(&self, firmware: &mut Firmware) -> Result { match firmware.leave_for_commit() { Ok(commit) => { self.sleep_commit.store(commit, Ordering::Release); dev_dbg!(self.dev.as_ref(), "GPU firmware left running for the sleep's commit\n"); - Ok(true) + Ok(()) } - // SAFETY: Reads the sleep driver's state. - Err(error) if unsafe { asahi_g17_commit_required() } => { + Err(error) => { dev_err!( self.dev.as_ref(), "no GPU stop at the commit ({:?}): refusing the sleep\n", @@ -447,14 +392,6 @@ impl Shared { ); Err(EBUSY) } - Err(error) => { - dev_warn!( - self.dev.as_ref(), - "no GPU stop at the commit ({:?}): stopping it now\n", - error - ); - Ok(false) - } } } @@ -471,7 +408,7 @@ impl Shared { Some(stopped) } - /// After the commit stopped the firmware: the books it left, as [`down`] would have them. + /// After the commit stopped the firmware: the books it left for each stopped instance. fn sleep_settle_commit(&self) { let mut state = self.state.lock(); if let Some(firmware) = (*state).as_deref_mut() { @@ -479,8 +416,8 @@ impl Shared { } } - /// Closes publication, waits for quiet, and leaves the stop to the sleep's commit where the - /// sleep driver takes it, or stops the firmware. A refusal changes nothing. + /// Closes publication, waits for quiet, and leaves the stop to the sleep's commit. A refusal + /// changes nothing. fn sleep_down(self: &Arc) -> Result { if self.sleeping() { let mut state = self.state.lock(); @@ -502,10 +439,7 @@ impl Shared { if !firmware.sleep_quiet(super::now_ns())? { return Err(EBUSY); } - if self.sleep_leave_for_commit(firmware)? { - return Ok(()); - } - down(firmware) + self.sleep_leave_for_commit(firmware) }); match result { Ok(()) => Ok(()), @@ -624,9 +558,6 @@ mod tests { #[derive(Copy, Clone, Debug, PartialEq, Eq)] enum Call { - Idle(Role), - Wake(Role), - Stop(Role), Request(bool), Gating(bool), Bridge(bool), @@ -664,9 +595,6 @@ mod tests { impl Hardware for Fake { fn has_secondary(&self) -> bool { self.secondary } - fn idle(&mut self, role: Role) -> Result { self.note(Call::Idle(role)) } - fn wake(&mut self, role: Role) { let _ = self.note(Call::Wake(role)); } - fn stop(&mut self, role: Role) { let _ = self.note(Call::Stop(role)); } fn request(&mut self, enable: bool) { let _ = self.note(Call::Request(enable)); } fn gating(&mut self, enable: bool) -> Result { self.note(Call::Gating(enable)) } fn bridge(&mut self, enable: bool) -> Result { self.note(Call::Bridge(enable)) } @@ -680,23 +608,6 @@ mod tests { use Role::{Primary as P, Secondary as S}; - #[test] - fn down_idles_the_primary_first_and_lowers_the_domain_last() { - let mut hw = Fake::new(true, None); - assert_eq!(down(&mut hw), Ok(())); - assert!(hw.called(&[ - Call::Idle(P), - Call::Idle(S), - Call::Stop(P), - Call::Gating(false), - Call::Stop(S), - Call::Request(false), - Call::AutoOff, - Call::Bridge(false), - Call::Lower, - ])); - } - #[test] fn up_restarts_the_secondary_first_and_ends_the_transition_last() { let mut hw = Fake::new(true, None); @@ -715,17 +626,6 @@ mod tests { #[test] fn a_single_instance_never_names_a_secondary() { - let mut hw = Fake::new(false, None); - assert_eq!(down(&mut hw), Ok(())); - assert!(hw.called(&[ - Call::Idle(P), - Call::Stop(P), - Call::Gating(false), - Call::Request(false), - Call::AutoOff, - Call::Bridge(false), - Call::Lower, - ])); let mut hw = Fake::new(false, None); assert_eq!(up(&mut hw), Ok(())); assert!(hw.called(&[ @@ -739,30 +639,6 @@ mod tests { ])); } - #[test] - fn a_refused_idle_touches_nothing_past_the_firmware() { - for call in [Call::Idle(P), Call::Idle(S)] { - let mut hw = Fake::new(true, Some(call)); - assert_eq!(down(&mut hw), Err(EIO)); - let past = |c: &Call| !matches!(c, Call::Idle(_) | Call::Wake(_)); - assert!(!hw.calls.iter().flatten().any(past)); - } - } - - #[test] - fn a_primary_that_refuses_idle_is_woken_and_nothing_stops() { - let mut hw = Fake::new(true, Some(Call::Idle(P))); - assert_eq!(down(&mut hw), Err(EIO)); - assert!(hw.called(&[Call::Idle(P), Call::Wake(P)])); - } - - #[test] - fn a_secondary_that_refuses_idle_wakes_both_and_stops_nothing() { - let mut hw = Fake::new(true, Some(Call::Idle(S))); - assert_eq!(down(&mut hw), Err(EIO)); - assert!(hw.called(&[Call::Idle(P), Call::Idle(S), Call::Wake(S), Call::Wake(P)])); - } - #[test] fn a_refused_sleep_only_brings_the_domain_back() { let mut hw = Fake::new(true, None); @@ -780,12 +656,6 @@ mod tests { assert!(hw.called(&[Call::AutoOff, Call::Bridge(false), Call::Lower])); } - #[test] - fn a_slow_domain_does_not_fail_down() { - let mut hw = Fake::new(false, Some(Call::Lower)); - assert_eq!(down(&mut hw), Ok(())); - } - #[test] fn up_stops_at_the_first_failure() { let mut hw = Fake::new(true, Some(Call::Restart(S)));