Skip to content

drm/asahi: g17: stop withholding compute doorbells #278

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
scottjones merged 1 commit into from
Oct 31, 2026
Merged

drm/asahi: g17: stop withholding compute doorbells #278

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 0 additions & 32 deletions drivers/gpu/drm/asahi/g17/port.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
// SPDX-License-Identifier: GPL-2.0-only OR MIT

//! Device-wide publication state that physical queues share without the device mutex.
Expand Down Expand Up @@ -98,11 +98,6 @@
generation: AtomicU64,
/// Device publication order shared by compute and render commands.
publications: AtomicU64,
/// Compute work slots written per priority class.
compute_written: [AtomicU64; PRIORITIES],
/// `compute_written` as read before the latest compute doorbell of each class: every slot
/// counted there was in its ring when the firmware handled that doorbell.
compute_announced: [AtomicU64; PRIORITIES],
/// Full firmware effort is applied; cleared before an idle clear rechecks `submissions`.
effort_full: AtomicBool,
submissions: AtomicU64,
Expand Down Expand Up @@ -152,8 +147,6 @@
render <- new_mutex!((), "g17::Port::render"),
generation: AtomicU64::new(0),
publications: AtomicU64::new(0),
compute_written: [const { AtomicU64::new(0) }; PRIORITIES],
compute_announced: [const { AtomicU64::new(0) }; PRIORITIES],
effort_full: AtomicBool::new(false),
submissions: AtomicU64::new(0),
}),
Expand Down Expand Up @@ -230,35 +223,10 @@
unsafe { (*ring.producer).store(next, Ordering::Relaxed) };
fence(Ordering::SeqCst);
reserved[claim.priority] = reserved[claim.priority].saturating_sub(1);
self.compute_written[claim.priority].fetch_add(1, Ordering::AcqRel);
claim.armed = false;
Ok(next)
}

/// Compute work slots written at `priority` so far.
pub(super) fn compute_written(&self, priority: u8) -> u64 {
self.compute_written
.get(usize::from(priority))
.map_or(u64::MAX, |written| written.load(Ordering::Acquire))
}

/// Compute work slots at `priority` that a doorbell has announced.
pub(super) fn compute_announced(&self, priority: u8) -> u64 {
self.compute_announced
.get(usize::from(priority))
.map_or(0, |announced| announced.load(Ordering::Acquire))
}

/// Rings the compute doorbell `message` of `priority`, after noting which slots it
/// announces.
pub(super) fn notify_compute(&self, priority: u8, message: u64) -> Result {
let index = usize::from(priority);
let written = self.compute_written.get(index).ok_or(EINVAL)?.load(Ordering::Acquire);
self.notify(message)?;
self.compute_announced[index].fetch_max(written, Ordering::AcqRel);
Ok(())
}

/// Taken around every render publication; see `render`.
pub(super) fn render_lock(&self) -> MutexGuard<'_, ()> {
self.render.lock()
Expand Down
101 changes: 3 additions & 98 deletions drivers/gpu/drm/asahi/g17/queue/compute.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
// SPDX-License-Identifier: GPL-2.0-only OR MIT

//! Retained physical compute queues. Logical ownership and command retirement never free an
Expand Down Expand Up @@ -369,10 +369,6 @@
};
use retirement::{Batch, Observation, Recordless, Ticket, IN_FLIGHT};

/// Position in its queue's active ring from which a publication withholds its doorbell:
/// with two unretired commands ahead, it cannot be the next to start.
const WITHHOLD_DEPTH: usize = 3;

struct Fifo<T> {
entries: [Option<T>; IN_FLIGHT],
head: usize,
Expand Down Expand Up @@ -599,65 +595,9 @@
retirement_proved: bool,
source_readers: Arc<core::sync::atomic::AtomicU32>,
source_hint: crate::g17::sync_input::ReaderHint,
/// The doorbell withheld from publications that had [`WITHHOLD_DEPTH`] - 1 or more
/// unretired commands of this queue ahead of them, the publication of the first of them
/// that no doorbell has announced, and the compute work slots written at its priority once
/// the last one's slot was.
withheld_doorbell: Option<(u64, u64, u64)>,
}

impl Queue {
/// Returns the withheld doorbell once at most one command of this queue that precedes the
/// first publication it covers has no completion record yet. A doorbell of the same class
/// rung after the last withheld work slot was written announced every one of them: nothing
/// is left to ring. `announced` gives the slots announced per class.
///
/// Retirement waiting for a stamp does not hold the doorbell back, and any departure from
/// ordinary retirement (timeout, kill, quarantine, replay, recovery or exit) rings it at
/// once: the firmware then sees every published command, as without withholding.
pub(crate) fn due_doorbell(&mut self, announced: impl Fn(u8) -> u64) -> Option<u64> {
let (doorbell, first, written) = self.withheld_doorbell?;
if announced(((doorbell >> 2) & 3) as u8) >= written {
self.withheld_doorbell = None;
return None;
}
let settling = self.settling();
let incomplete = self
.active
.iter()
.filter(|active| active.published < first && active.observed.is_none())
.count();
if !settling && incomplete > 1 {
return None;
}
self.withheld_doorbell = None;
Some(doorbell)
}

/// The queue has left ordinary retirement: a timeout, kill, quarantine, replay, recovery
/// witness or exit is in progress, or retirement stopped at a command the firmware
/// finished without a completion record. A front whose record has arrived and that only
/// waits for its stamp is ordinary retirement: [`Self::due_doorbell`] counts it as
/// finished, and ringing at once there rang most withheld doorbells early.
fn settling(&self) -> bool {
self.quarantined
|| self.released
|| self.exit.is_some()
|| self.replays.len != 0
|| self.replay_retirement
|| self.awaiting_witness
|| (self.retire_pending
&& self.active.front().is_none_or(|active| active.observed.is_none()))
|| self.retired_by_teardown
|| self.spared_quarantine
|| self.failure_status_pending
|| self.terminal.is_some()
}

/// Takes the withheld doorbell regardless of its predecessors.
pub(crate) fn take_withheld_doorbell(&mut self) -> Option<u64> {
self.withheld_doorbell.take().map(|(doorbell, _, _)| doorbell)
}
/// The caller reserves a QID first, cancels it on construction failure, and retains every
/// successfully installed queue until processor stop. Shared VM apertures precede this call.
pub(crate) fn new(
Expand Down Expand Up @@ -724,7 +664,6 @@
retirement_proved: false,
source_readers: Arc::new(core::sync::atomic::AtomicU32::new(0), GFP_KERNEL)?,
source_hint: crate::g17::sync_input::ReaderHint::new(),
withheld_doorbell: None,
})
}
/// The firmware failed the command whose completion stamp is `stamp`. It writes a
Expand Down Expand Up @@ -1711,7 +1650,6 @@
ring: DescriptorRing,
) -> Result<(u16, u64)> {
if !self.can_rearm() { return Err(EAGAIN); }
self.withheld_doorbell = None;
self.graph.rearm_ring(ring)?;
self.pool.rearm(pages)?;
self.pool_phase = PoolPhase::Populated;
Expand Down Expand Up @@ -2006,10 +1944,7 @@
fn publish_qid(&mut self, id: kick::Id) -> Result;
fn publish_outer(&mut self, priority: u8, slot: &crate::g17::fw::channels::WorkSlot)
-> Result;
fn notify_compute(&mut self, priority: u8, message: u64) -> Result;
fn compute_written(&self, priority: u8) -> u64;
/// Compute work slots at `priority` that a doorbell has announced.
fn compute_announced(&self, priority: u8) -> u64;
fn notify(&mut self, message: u64) -> Result;
fn note_submission(&mut self) -> Result;
}

Expand Down Expand Up @@ -2342,42 +2277,12 @@
previous.publication = Some((packet.order.sequence, epoch));
}
}
// A publication rings its own pipe and priority class, as render
// Every publication rings its own pipe and priority class, as render
// publication does: pipe 2 (compute) in bits 1:0, priority class in
// bits 3:2. The generic work doorbell left the firmware to find the
// ring itself and delayed dispatch.
//
// A doorbell makes the firmware read the whole work ring of its class,
// and each one it handles while finishing a command delays that
// command's completion and the release of its waiters. A command with
// two or more unretired commands of its queue ahead cannot start soon:
// its doorbell is withheld until at most one precedes it (see
// `due_doorbell`), and any doorbell rung meanwhile on the same ring
// announces its work slot earlier.
fence(Ordering::SeqCst);
let doorbell = (0x83 << 48) | 0x02 | u64::from(policy.priority()) << 2;
if self.active.len >= WITHHOLD_DEPTH
&& !self.settling()
&& self.withheld_doorbell.is_none_or(|(withheld, _, _)| withheld == doorbell)
{
let written = host.compute_written(policy.priority());
// Once another doorbell announced every slot the withheld one covered, it
// starts over from this publication: it then waits for this command's own
// predecessors rather than for those of slots already announced.
if self
.withheld_doorbell
.is_some_and(|(_, _, last)| host.compute_announced(policy.priority()) >= last)
{
self.withheld_doorbell = None;
}
let entry = self.withheld_doorbell.get_or_insert((doorbell, published, written));
entry.2 = written;
} else {
host.notify_compute(policy.priority(), doorbell)?;
if self.withheld_doorbell.is_some_and(|(withheld, _, _)| withheld == doorbell) {
self.withheld_doorbell = None;
}
}
host.notify((0x83 << 48) | 0x02 | u64::from(policy.priority()) << 2)?;
crate::g17::trace::publish(crate::g17::trace::COMPUTE, qid, ordinal, &packet);
host.note_submission()
})();
Expand Down
4 changes: 0 additions & 4 deletions drivers/gpu/drm/asahi/g17/recovery.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
// SPDX-License-Identifier: GPL-2.0-only OR MIT

//! Firmware restart handshake and per-VM failure classification.
Expand Down Expand Up @@ -107,9 +107,6 @@
fn settle_cancelled_renders(&mut self, qids: u128);
fn rescan_compute(&mut self) -> Result;
fn release_retired_compute(&mut self);
/// Ring every compute doorbell still withheld: a restart is a departure from ordinary
/// retirement, after which the firmware must see every published command.
fn announce_withheld(&mut self);
}

/// Live shared-memory operations. The handshake word uses aligned 32-bit accesses; transitions
Expand Down Expand Up @@ -299,7 +296,6 @@
}
let scan_error = host.rescan_compute().err();
host.release_retired_compute();
host.announce_withheld();
Ok(ServiceResult { scan_error, target_error })
}

Expand Down
30 changes: 4 additions & 26 deletions drivers/gpu/drm/asahi/g17/runtime.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
// SPDX-License-Identifier: GPL-2.0-only OR MIT

//! Device ownership of physical queues and off-lock construction reservations.
Expand Down Expand Up @@ -1393,14 +1393,8 @@
fn publish_outer(&mut self, _priority: u8, slot: &super::fw::channels::WorkSlot) -> Result {
self.port.publish_compute(self.outer.take().ok_or(EIO)?, slot).map(|_| ())
}
fn notify_compute(&mut self, priority: u8, message: u64) -> Result {
self.port.notify_compute(priority, message)
}
fn compute_written(&self, priority: u8) -> u64 {
self.port.compute_written(priority)
}
fn compute_announced(&self, priority: u8) -> u64 {
self.port.compute_announced(priority)
fn notify(&mut self, message: u64) -> Result {
self.port.notify(message)
}
fn note_submission(&mut self) -> Result {
self.effort.submit();
Expand Down Expand Up @@ -1443,14 +1437,8 @@
fn publish_outer(&mut self, _priority: u8, slot: &super::fw::channels::WorkSlot) -> Result {
self.port.publish_compute(self.outer.take().ok_or(EIO)?, slot).map(|_| ())
}
fn notify_compute(&mut self, priority: u8, message: u64) -> Result {
self.port.notify_compute(priority, message)
}
fn compute_written(&self, priority: u8) -> u64 {
self.port.compute_written(priority)
}
fn compute_announced(&self, priority: u8) -> u64 {
self.port.compute_announced(priority)
fn notify(&mut self, message: u64) -> Result {
self.port.notify(message)
}
fn note_submission(&mut self) -> Result {
Ok(())
Expand Down Expand Up @@ -1675,7 +1663,6 @@
let port = &self.port;
let retired = deferred.pending();
let mut freed = false;
let mut doorbells = [0u64; 4];
for mut queue in registry
.compute
.iter_mut()
Expand All @@ -1692,15 +1679,6 @@
&mut |event| deferred.push(event),
)?;
freed |= busy && queue.channel_free();
// Withheld doorbells are rung once their command is next but one; one per
// priority class announces every queue's work slots on that ring.
if let Some(doorbell) = queue.due_doorbell(|class| port.compute_announced(class)) {
let class = ((doorbell >> 2) & 3) as usize;
doorbells[class] = doorbell;
}
}
for doorbell in doorbells.into_iter().filter(|doorbell| *doorbell != 0) {
self.port.notify_compute(((doorbell >> 2) & 3) as u8, doorbell)?;
}
// All compute observations in this batch are latched before a VM
// failure can settle its render peers. Exact successes win first.
Expand Down
14 changes: 1 addition & 13 deletions drivers/gpu/drm/asahi/g17/runtime/compute_control.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
// SPDX-License-Identifier: GPL-2.0-only OR MIT

//! Scheduler cancellation and final retirement checks for retained compute work.
Expand Down Expand Up @@ -69,14 +69,7 @@
if !queue.owns_packet(packet) {
return Ok(None);
}
let status = queue.timeout(&mut |event| deferred.push(event));
// A timed-out queue leaves ordinary retirement: announce what it withheld.
let withheld = queue.take_withheld_doorbell();
drop(queue);
if let Some(doorbell) = withheld {
firmware.port.notify_compute(((doorbell >> 2) & 3) as u8, doorbell)?;
}
status
queue.timeout(&mut |event| deferred.push(event))
})();
if result.as_ref().is_ok_and(|status| status.is_some()) {
// VM status and peer settlement precede signalling this timeout. The backend
Expand All @@ -99,11 +92,6 @@
firmware.scan_compute(None, &mut deferred)?;
if let Some(mut queue) = firmware.queues.compute_mut(self.owner) {
queue.cancel(packet, &mut |event| deferred.push(event))?;
let withheld = queue.take_withheld_doorbell();
drop(queue);
if let Some(doorbell) = withheld {
firmware.port.notify_compute(((doorbell >> 2) & 3) as u8, doorbell)?;
}
}
Ok(())
})();
Expand Down
20 changes: 0 additions & 20 deletions drivers/gpu/drm/asahi/g17/runtime/events.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
// SPDX-License-Identifier: GPL-2.0-only OR MIT

//! Event and restart callbacks borrow the complete firmware owner. The reusable
Expand Down Expand Up @@ -248,26 +248,6 @@
let result = self.firmware.release_compute_witnesses(self.deferred);
self.settlement_error = self.settlement_error.or(result.err());
}
fn announce_withheld(&mut self) {
let mut doorbells = [0u64; 4];
for mut queue in self
.firmware
.queues
.compute
.iter_mut()
.flatten()
.filter_map(|entry| entry.queue.as_deref_mut())
{
if let Some(doorbell) = queue.take_withheld_doorbell() {
doorbells[((doorbell >> 2) & 3) as usize] = doorbell;
}
}
// One doorbell per priority class announces every queue's work slots on that ring.
for doorbell in doorbells.into_iter().filter(|doorbell| *doorbell != 0) {
let result = self.firmware.port.notify_compute(((doorbell >> 2) & 3) as u8, doorbell);
self.settlement_error = self.settlement_error.or(result.err());
}
}
}

impl Firmware {
Expand Down