From 51c561f15453f453f7af2f9c35b6f8e59b891813 Mon Sep 17 00:00:00 2001 From: DjDeveloperr Date: Tue, 10 Nov 2026 17:33:59 -0500 Subject: [PATCH 01/14] drm/asahi: g17: M5: skip the install check of an installed compute queue Every M5 compute publication ran the queue-install check, which read the recovery state and the item-ring cursors from shared GPU memory (four uncached loads under the device mutex) before deciding that a queue with a published kick needs nothing. Return first when the queue is installed, its configuration is released and a kick was published; the outcome is the same, without the loads. Signed-off-by: Diljit Singh --- drivers/gpu/drm/asahi/g17/queue/compute.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/gpu/drm/asahi/g17/queue/compute.rs b/drivers/gpu/drm/asahi/g17/queue/compute.rs index 3e9d2002dafc3..e14795fdf5895 100644 --- a/drivers/gpu/drm/asahi/g17/queue/compute.rs +++ b/drivers/gpu/drm/asahi/g17/queue/compute.rs @@ -2980,6 +2980,11 @@ impl Queue { } fn install_m5(&mut self, packet: &Arc, host: &mut impl Host) -> Result { + // Installed, configuration released and a kick published: nothing to check, and no + // reads of shared memory under the device mutex. + if self.m5_install.is_some() && self.ordinal != 0 && self.configuration.is_none() { + return Ok(()); + } let (epoch, state) = host.epoch()?; if state == 0 { self.retire_configuration(epoch)?; } if let Some(end) = self.m5_install { From 1f0d25be913a0213a11ff3a25591f79b6ad6cfad Mon Sep 17 00:00:00 2001 From: DjDeveloperr Date: Tue, 10 Nov 2026 17:34:27 -0500 Subject: [PATCH 02/14] drm/asahi: g17: M5: append compute items from the publication's snapshot The item-ring snapshot that #1401 takes for an M5 compute publication served its room, record-slot and reclaim checks, but both appends (the configuration and its notification) still read the ring cursors again: three uncached loads each under the device mutex. The same argument covers them: the producer is the host's and only this publication moves it, and the firmware's consumer only advances, so the snapshot's consumer is conservative. The room check already required space for both items. Append against the snapshot, with the producer the publication has reached. Signed-off-by: Diljit Singh --- drivers/gpu/drm/asahi/g17/queue/compute.rs | 23 +++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/asahi/g17/queue/compute.rs b/drivers/gpu/drm/asahi/g17/queue/compute.rs index e14795fdf5895..c6a8cc2b46082 100644 --- a/drivers/gpu/drm/asahi/g17/queue/compute.rs +++ b/drivers/gpu/drm/asahi/g17/queue/compute.rs @@ -2408,7 +2408,17 @@ impl Graph { Ok(object.gpu_va() + start as u64) } fn append(&self, expected: u32, address: u64, before_publish: impl FnOnce()) -> Result { - let cursors = self.cursors()?; + self.append_to(self.cursors()?, expected, address, before_publish) + } + /// Appends against `cursors`: a fresh read, or an M5 publication's snapshot whose + /// producer the publication advanced itself (see `publish_with_native_waits`). + fn append_to( + &self, + cursors: Cursors, + expected: u32, + address: u64, + before_publish: impl FnOnce(), + ) -> Result { if cursors.producer != expected || cursors.count != ITEM_RING_ENTRIES || expected >= ITEM_RING_ENTRIES @@ -2777,7 +2787,8 @@ impl Queue { self.pool.cancel_unpublished(target)?; Err(error) })?; self.active.back_mut().ok_or(EIO)?.qos_pending = true; - producer = self.graph.append(cursors.producer, config_va.ok_or(EINVAL)?, + // `cursors` is this publication's snapshot: no second read of the ring. + producer = self.graph.append_to(cursors, cursors.producer, config_va.ok_or(EINVAL)?, || packet.mark_published()) .or_else(|error| { self.pool.cancel_unpublished(target)?; @@ -2878,7 +2889,13 @@ impl Queue { self.graph.record(record_slot, ANNOUNCE, 0x40, crate::g17::fw::m5::queue::notification(qid, kick_timestamp)?)? } else { self.graph.record(record_slot, ANNOUNCE, 0x40, announce)? }; - producer = self.graph.append(producer, announce_va, || {})?; + producer = match snapshot { + // The snapshot's consumer only lags the firmware's, and the free-room check + // above covered both M5 items; the producer is the one this publication set. + Some(snapshot) => self.graph.append_to(Cursors { producer, ..snapshot }, producer, + announce_va, || {})?, + None => self.graph.append(producer, announce_va, || {})?, + }; fence(Ordering::SeqCst); if entry_signal && !self.graph.m5 { let predecessor_va = From c8867686039f3cc92074704b8b745d27a10694de Mon Sep 17 00:00:00 2001 From: DjDeveloperr Date: Tue, 10 Nov 2026 17:35:22 -0500 Subject: [PATCH 03/14] drm/asahi: g17: M5: publish compute on the cursors of its room check An M5 compute publication read the shared compute ring's three cursors twice under the device mutex: for the room check before any state changes, and again when it wrote its record. Between the two, only this publication can move the producer, the firmware's consumers only advance and off-lock cancellation only lowers the reserved credits, so the first sample stays conservative. Keep it in the publication's host and write the record against it; any other record reads the ring as before. Signed-off-by: Diljit Singh --- drivers/gpu/drm/asahi/g17/initdata.rs | 38 ++++++++++++++++++++++++--- drivers/gpu/drm/asahi/g17/runtime.rs | 18 +++++++++---- 2 files changed, 47 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/asahi/g17/initdata.rs b/drivers/gpu/drm/asahi/g17/initdata.rs index 5c333e05f8788..9d8a7e3cdc56a 100644 --- a/drivers/gpu/drm/asahi/g17/initdata.rs +++ b/drivers/gpu/drm/asahi/g17/initdata.rs @@ -691,10 +691,25 @@ impl InitData { Ok(next != consumer && next != consumer_b) } + /// M5 compute: the shared ring's cursors when it has room for one record, as + /// [`Self::work_ready`] judges it. The same critical section may publish against them + /// ([`Self::publish_work_at`]): the firmware's consumers only advance, and the producer + /// is the device mutex holder's. + pub(super) fn m5_compute_room(&self, priority: u8) -> Result> { + use super::fw::queue::DataMaster; + if !self.m5 { + return Err(EINVAL); + } + let cursors = self.work_cursors(priority, DataMaster::Compute)?; + Ok((self.outer_capacity(priority, cursors)? >= 1).then_some(cursors)) + } + + /// `cursors`: an M5 compute room check's sample ([`Self::m5_compute_room`]), else read here. fn work_publication<'a>( &'a self, priority: u8, record: &'a super::fw::channels::WorkSlot, + cursors: Option<[u32; 3]>, ) -> Result> { use super::fw::{channels, queue::DataMaster}; let engine = match record.data_master { @@ -706,7 +721,11 @@ impl InitData { _ => return Err(EINVAL), }; let channel = channels::work_ring_index(priority, engine).ok_or(EINVAL)?; - let [consumer, consumer_b, producer] = self.work_cursors(priority, engine)?; + let [consumer, consumer_b, producer] = match cursors { + Some(cursors) if self.m5 && engine == DataMaster::Compute => cursors, + Some(_) => return Err(EINVAL), + None => self.work_cursors(priority, engine)?, + }; if self.m5 && engine == DataMaster::Compute && self.outer_capacity(priority, [consumer, consumer_b, producer])? < 1 { return Err(EBUSY); @@ -769,7 +788,18 @@ impl InitData { priority: u8, record: &super::fw::channels::WorkSlot, ) -> Result { - Ok(self.work_publication(priority, record)?.commit()) + self.publish_work_at(priority, record, None) + } + + /// [`Self::publish_work`] against the cursors of a room check made earlier under the same + /// hold of the device mutex ([`Self::m5_compute_room`]), or a fresh read for None. + pub(super) fn publish_work_at( + &self, + priority: u8, + record: &super::fw::channels::WorkSlot, + cursors: Option<[u32; 3]>, + ) -> Result { + Ok(self.work_publication(priority, record, cursors)?.commit()) } /// Reserve both common-ring slots before touching either host port. The @@ -835,8 +865,8 @@ impl InitData { publish_late_tiling(); return Ok(if deferred { pair.next() } else { pair.publish() }); } - let fragment = self.work_publication(priority, fragment)?; - let tiling = self.work_publication(priority, tiling)?; + let fragment = self.work_publication(priority, fragment, None)?; + let tiling = self.work_publication(priority, tiling, None)?; fragment.copy_record(); commit_accounting()?; fence(Ordering::SeqCst); diff --git a/drivers/gpu/drm/asahi/g17/runtime.rs b/drivers/gpu/drm/asahi/g17/runtime.rs index 94ed6fc391aa1..f0c9ccc8a2ae6 100644 --- a/drivers/gpu/drm/asahi/g17/runtime.rs +++ b/drivers/gpu/drm/asahi/g17/runtime.rs @@ -1505,6 +1505,8 @@ struct ComputeHost<'a> { regs: &'a crate::regs::G17Resources, port: &'a super::port::Port, outer: Option>, + /// M5: the shared compute ring's room check of this publication, by priority. + outer_cursors: Option<(u8, [u32; 3])>, effort: &'a mut super::power::Effort, qids: &'a mut kick::QueueIds, generation: u64, @@ -1545,7 +1547,11 @@ impl compute::Host for ComputeHost<'_> { } fn publish_outer(&mut self, priority: u8, slot: &super::fw::channels::WorkSlot) -> Result { if self.init.is_m5() { - return self.init.publish_work(priority, slot).map(|_| ()); + // The room check's sample serves the first record only: it moves the producer. + let cursors = self.outer_cursors.take() + .filter(|&(checked, _)| checked == priority) + .map(|(_, cursors)| cursors); + return self.init.publish_work_at(priority, slot, cursors).map(|_| ()); } self.port.publish_compute(self.outer.take().ok_or(EIO)?, slot).map(|_| ()) } @@ -1712,12 +1718,13 @@ impl super::Firmware { // reservation; dropping it unused returns the work-ring slot. M5 compute takes its // slot on the work ring M5 render shares, under this mutex (InitData::publish_work): // a full ring is refused here, before a queue install or kick changes any state. + let mut outer_cursors = None; let outer = if self.init.is_m5() { - if !self.init.work_ready(packet.context.policy().priority(), - super::fw::queue::DataMaster::Compute)? - { + let priority = packet.context.policy().priority(); + let Some(cursors) = self.init.m5_compute_room(priority)? else { return Err(EAGAIN); - } + }; + outer_cursors = Some((priority, cursors)); None } else { Some(self.port.reserve_compute(packet.context.policy().priority())?) @@ -1745,6 +1752,7 @@ impl super::Firmware { regs: &self.bridge.regs, port: &self.port, outer, + outer_cursors, effort: &mut self.effort, qids: &mut queues.qids, generation: self.recovery.generation(), From 64911f1af804ce5a1a565f680781f538e337e0b2 Mon Sep 17 00:00:00 2001 From: DjDeveloperr Date: Tue, 10 Nov 2026 17:35:32 -0500 Subject: [PATCH 04/14] drm/asahi: g17: count compute replays before reading the recovery state replay_compute runs on every event pass and before every inline compute publication, and it read the recovery state from shared GPU memory (an uncached load under the device mutex) before finding, almost always, that no queue retains a command for replay. Count the retained commands first, from host state, and return when there are none. The replay wait markers then name no command, so they are cleared as before; the recovery state still gates every replay. Signed-off-by: Diljit Singh --- drivers/gpu/drm/asahi/g17/runtime/compute_replay.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/asahi/g17/runtime/compute_replay.rs b/drivers/gpu/drm/asahi/g17/runtime/compute_replay.rs index 2b4f43b5685ad..e72fc921834d9 100644 --- a/drivers/gpu/drm/asahi/g17/runtime/compute_replay.rs +++ b/drivers/gpu/drm/asahi/g17/runtime/compute_replay.rs @@ -17,11 +17,8 @@ impl Firmware { if self.recovery.pending() { return Ok(()); } - let state = recovery::Memory::recovery_state(&self.init)?; - if state != 0 { - return Ok(()); - } - self.queues.replay_consumer_wait = None; + // Runs on every event pass and inline publication: with nothing retained for replay, + // return before reading the recovery state from shared memory. let commands = self .queues .compute @@ -31,9 +28,15 @@ impl Firmware { .map(|queue| queue.replay_commands()) .sum::(); if commands == 0 { + self.queues.replay_consumer_wait = None; self.queues.replay_outer_wait = None; return Ok(()); } + let state = recovery::Memory::recovery_state(&self.init)?; + if state != 0 { + return Ok(()); + } + self.queues.replay_consumer_wait = None; deferred.reserve_replays(commands)?; let port = &self.port; for entry in self.queues.compute.iter_mut().flatten() { From 7eca3a79e04fca33477ffe80409f562f358bd614 Mon Sep 17 00:00:00 2001 From: DjDeveloperr Date: Tue, 10 Nov 2026 17:36:18 -0500 Subject: [PATCH 05/14] drm/asahi: g17: M5: keep the submission-history geometry while it holds Every M5 doorbell (compute and render) first counts the submission in the firmware's history, and read its geometry each time: the generation, the four period and slot-count words, and the generation again, all uncached loads under the device mutex. The firmware changes the generation around every change of the geometry, so a geometry read under an even generation stands for as long as the generation reads the same. Keep it with its generation and read the words again only when the generation differs, or after a recovery transition. Signed-off-by: Diljit Singh --- drivers/gpu/drm/asahi/g17/initdata.rs | 6 ++++++ drivers/gpu/drm/asahi/g17/initdata/m5.rs | 23 ++++++++++++++++------- 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/asahi/g17/initdata.rs b/drivers/gpu/drm/asahi/g17/initdata.rs index 9d8a7e3cdc56a..d7e6cb521359f 100644 --- a/drivers/gpu/drm/asahi/g17/initdata.rs +++ b/drivers/gpu/drm/asahi/g17/initdata.rs @@ -205,6 +205,9 @@ pub(super) struct InitData { compute_scheduler: Option, compute_active: core::cell::Cell, event_ring_checked: [core::cell::Cell<(u64, u64, u32)>; 2], + /// M5: the submission-history geometry (period, slot count) read under the firmware's + /// generation, with that generation. + history_geometry: core::cell::Cell>, /// Completion-ring objects; a family sets only the rings it has. completion_rings: [Option; 4], stamps: [usize; 2], @@ -308,6 +311,7 @@ impl InitData { compute_scheduler: None, compute_active: core::cell::Cell::new(false), event_ring_checked: [const { core::cell::Cell::new((0, 0, 0)) }; 2], + history_geometry: core::cell::Cell::new(None), completion_rings: [None; 4], stamps: [0; 2], render_pages: None, @@ -1966,6 +1970,8 @@ impl super::recovery::Memory for InitData { } fn transition(&self, expected: u32, next: u32) -> Result { + // A restarted firmware may lay its submission history out anew. + self.history_geometry.set(None); let state = self .object(self.cluster)? .word(self.recovery_state_offset())?; diff --git a/drivers/gpu/drm/asahi/g17/initdata/m5.rs b/drivers/gpu/drm/asahi/g17/initdata/m5.rs index ec150b54f1bee..2bc0647da9624 100644 --- a/drivers/gpu/drm/asahi/g17/initdata/m5.rs +++ b/drivers/gpu/drm/asahi/g17/initdata/m5.rs @@ -55,13 +55,22 @@ impl InitData { for _ in 0..64 { let generation = private.word(base + 0x74c)?.load(Ordering::Acquire); if generation & 1 != 0 { core::hint::spin_loop(); continue; } - let config = private.read_words::<4>(base + 0x73c)?; - fence(Ordering::Acquire); - if generation != private.word(base + 0x74c)?.load(Ordering::Acquire) { - core::hint::spin_loop(); continue; - } - let period = u64::from(config[0]) | u64::from(config[1]) << 32; - let count = u64::from(config[2]) | u64::from(config[3]) << 32; + // The firmware moves the generation around every change of the geometry: while it + // reads as it did, the geometry read under it stands, without four more loads. + let (period, count) = match self.history_geometry.get() { + Some((seen, period, count)) if seen == generation => (period, count), + _ => { + let config = private.read_words::<4>(base + 0x73c)?; + fence(Ordering::Acquire); + if generation != private.word(base + 0x74c)?.load(Ordering::Acquire) { + core::hint::spin_loop(); continue; + } + let period = u64::from(config[0]) | u64::from(config[1]) << 32; + let count = u64::from(config[2]) | u64::from(config[3]) << 32; + self.history_geometry.set(Some((generation, period, count))); + (period, count) + } + }; if period == 0 || count == 0 || count > 256 { return Err(EINVAL); } let (counter, frequency) = crate::g17::qos::clock(); let ticks = crate::g17::qos::time_from_host(counter, frequency)?; From 2d1161981932a548f28d0b3f9831c111f3b24b68 Mon Sep 17 00:00:00 2001 From: DjDeveloperr Date: Tue, 10 Nov 2026 17:37:22 -0500 Subject: [PATCH 06/14] drm/asahi: g17: M5: share the power observation between statistics drains The M5 event pass drains each role's statistics ring between two power observations, and each observation reads the GPU's core power register. With two roles the observation after the first drain and the one before the second ran back to back, under the device mutex. Observe once before the first drain and once after each; every drain is still bracketed. Signed-off-by: Diljit Singh --- drivers/gpu/drm/asahi/g17/runtime/events.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/asahi/g17/runtime/events.rs b/drivers/gpu/drm/asahi/g17/runtime/events.rs index 5114465145081..10ba2927f7fbe 100644 --- a/drivers/gpu/drm/asahi/g17/runtime/events.rs +++ b/drivers/gpu/drm/asahi/g17/runtime/events.rs @@ -338,8 +338,10 @@ impl Firmware { for &role in self.roles() { self.rings.drain_trace(dev, &self.init, role); } + // Each drain is bracketed by power observations; consecutive roles share the one + // between them (each reads the GPU's power register). + self.observe_statistics_power(); for &role in self.roles() { - self.observe_statistics_power(); self.rings.drain_statistics(&self.primary.state.shared.dev, &self.init, role); self.observe_statistics_power(); } From c267b9298d72f47d32fc3586ec8bd650d12af83b Mon Sep 17 00:00:00 2001 From: DjDeveloperr Date: Tue, 10 Nov 2026 17:37:38 -0500 Subject: [PATCH 07/14] drm/asahi: g17: read firmware records two words per load KernelObject::read_words copied records with one 32-bit volatile load per word. Firmware records live in write-combined (uncached) views, so an 18-word event or report record took 18 uncached loads, under the device mutex in the event pass. Where the copy is 8-byte aligned, load two words at a time; the words and their order in memory are unchanged. Signed-off-by: Diljit Singh --- drivers/gpu/drm/asahi/g17/object.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/drivers/gpu/drm/asahi/g17/object.rs b/drivers/gpu/drm/asahi/g17/object.rs index 0b9854e84de2c..0fd65708922f7 100644 --- a/drivers/gpu/drm/asahi/g17/object.rs +++ b/drivers/gpu/drm/asahi/g17/object.rs @@ -328,6 +328,17 @@ impl KernelObject { pub(crate) fn read_words(&self, offset: usize) -> Result<[u32; N]> { let base = self.aligned::<[u32; N]>(offset)?.cast::(); let mut words = [0u32; N]; + // Most views are uncached: where the copy is 8-byte aligned, load two words at a time. + if N % 2 == 0 && base as usize % 8 == 0 { + let pairs = base.cast::(); + for (i, pair) in words.chunks_exact_mut(2).enumerate() { + // SAFETY: As below; the 8-byte loads cover the same aligned `N` words. + let bytes = unsafe { pairs.add(i).read_volatile() }.to_ne_bytes(); + pair[0] = u32::from_ne_bytes([bytes[0], bytes[1], bytes[2], bytes[3]]); + pair[1] = u32::from_ne_bytes([bytes[4], bytes[5], bytes[6], bytes[7]]); + } + return Ok(words); + } for (i, word) in words.iter_mut().enumerate() { // SAFETY: `aligned()` checked that all `N` words are within the object and aligned. *word = unsafe { base.add(i).read_volatile() }; From 631657f81b446998f9651e4d3b2aff574e647624 Mon Sep 17 00:00:00 2001 From: DjDeveloperr Date: Tue, 10 Nov 2026 17:43:55 -0500 Subject: [PATCH 08/14] drm/asahi: g17: M5: check the shared ring's room once per render publication A render publication checks that both outer destinations, tiling and fragment, have room before any state changes. On M5 both are the shared compute ring of the priority, and both checks asked for the same two free slots, so the second one read the ring's three cursors from shared memory again under the device mutex for the same answer. Check once on M5; other GPUs keep both checks. Signed-off-by: Diljit Singh --- drivers/gpu/drm/asahi/g17/runtime/render/submit.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/asahi/g17/runtime/render/submit.rs b/drivers/gpu/drm/asahi/g17/runtime/render/submit.rs index 431dbbd748ecf..4f10efce29cb9 100644 --- a/drivers/gpu/drm/asahi/g17/runtime/render/submit.rs +++ b/drivers/gpu/drm/asahi/g17/runtime/render/submit.rs @@ -749,10 +749,12 @@ impl crate::g17::Firmware { let port = self.port.clone(); let _render = port.render_lock(); // Both outer destinations must have room before pool accounting or - // either inner queue becomes visible. + // either inner queue becomes visible. On M5 both are the shared compute ring, and + // one check covers the pair (each reads the ring's cursors from shared memory). let priority = packet.context.policy().priority(); if !self.init.work_ready(priority, crate::g17::fw::queue::DataMaster::Fragment)? - || !self.init.work_ready(priority, crate::g17::fw::queue::DataMaster::Tiling)? + || (!self.init.is_m5() + && !self.init.work_ready(priority, crate::g17::fw::queue::DataMaster::Tiling)?) { return Err(EBUSY); } From d0279a26d33f00acca1febad0953e5de192a23a6 Mon Sep 17 00:00:00 2001 From: DjDeveloperr Date: Tue, 10 Nov 2026 18:35:35 -0500 Subject: [PATCH 09/14] drm/asahi: g17: check for parked renders before reading the recovery state replay_render runs on every event pass and read the recovery state from shared GPU memory (an uncached load under the device mutex) before looking for a parked render group, of which there almost never is one. Look first, from host state; the recovery state still gates every replay. Signed-off-by: Diljit Singh --- .../gpu/drm/asahi/g17/runtime/render/replay.rs | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/asahi/g17/runtime/render/replay.rs b/drivers/gpu/drm/asahi/g17/runtime/render/replay.rs index 67488b7299bbc..a37ad6b2c8ab8 100644 --- a/drivers/gpu/drm/asahi/g17/runtime/render/replay.rs +++ b/drivers/gpu/drm/asahi/g17/runtime/render/replay.rs @@ -33,6 +33,15 @@ impl super::RenderState { .any(|pair| pair.owner() == Some(owner) && pair.replay_through(through)) } + /// Whether any pair holds a group parked for replay. + pub(in crate::g17) fn replay_parked(&self) -> bool { + self.entries + .iter() + .flatten() + .filter_map(|entry| entry.pair.as_ref()) + .any(|pair| pair.replay_front().is_some()) + } + pub(in crate::g17) fn replay_polling(&self) -> bool { self.entries .iter() @@ -44,9 +53,12 @@ impl super::RenderState { impl Firmware { pub(in crate::g17) fn replay_render(&mut self, deferred: &mut DeferredBatch) -> Result { - if self.recovery.pending() - || crate::g17::recovery::Memory::recovery_state(&self.init)? != 0 - { + // Runs on every event pass: with no group parked for replay, return before reading the + // recovery state from shared memory. + if self.recovery.pending() || !self.queues.render.replay_parked() { + return Ok(()); + } + if crate::g17::recovery::Memory::recovery_state(&self.init)? != 0 { return Ok(()); } for slot in 0..self.queues.render.entries.len() { From 8e02d22578fec035e0ba0615117fe55ab0274d9a Mon Sep 17 00:00:00 2001 From: DjDeveloperr Date: Tue, 10 Nov 2026 18:35:51 -0500 Subject: [PATCH 10/14] drm/asahi: g17: observe render pool releases only while one is in flight Every render publication (through its pool preparation) and the event worker observed the render pools' free-list releases by reading the device-control ring's cursors from shared GPU memory, two uncached loads under the device mutex. A lease uses them only while a release it sent has not been seen consumed. Return before the read when no lease waits for one; the leases end up in the same state. Signed-off-by: Diljit Singh --- drivers/gpu/drm/asahi/g17/freelist.rs | 5 +++++ drivers/gpu/drm/asahi/g17/runtime/render/pools.rs | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/drivers/gpu/drm/asahi/g17/freelist.rs b/drivers/gpu/drm/asahi/g17/freelist.rs index 3d1449bee2147..05dbf028cbc81 100644 --- a/drivers/gpu/drm/asahi/g17/freelist.rs +++ b/drivers/gpu/drm/asahi/g17/freelist.rs @@ -1371,6 +1371,11 @@ impl Lease { } /// Observes all older release records passed by one consumer snapshot. + /// A sent release whose consumption [`Self::observe_releases`] has yet to witness. + pub(crate) fn awaits_release(&self) -> bool { + self.previous_release.is_some() || matches!(self.phase, LeasePhase::ReleaseSent(_)) + } + pub(crate) fn observe_releases(&mut self, consumer: u32, producer: u32) -> Result { if let Some(cursor) = self.previous_release { if control_consumed(consumer, producer, cursor)? { diff --git a/drivers/gpu/drm/asahi/g17/runtime/render/pools.rs b/drivers/gpu/drm/asahi/g17/runtime/render/pools.rs index ab94764580521..ba287c13456cc 100644 --- a/drivers/gpu/drm/asahi/g17/runtime/render/pools.rs +++ b/drivers/gpu/drm/asahi/g17/runtime/render/pools.rs @@ -219,6 +219,11 @@ impl Pools { } pub(super) fn observe(&mut self, init: &InitData) -> Result { + // Every render publication observes: with no release in flight there is nothing to + // witness, and no reason to read the control ring's cursors from shared memory. + if !self.leases[FIRST_RENDER_POOL..].iter().flatten().any(|lease| lease.awaits_release()) { + return Ok(()); + } fence(Ordering::Acquire); let consumer = init.control_consumer()?.load(Ordering::Relaxed); let producer = init.control_producer()?.load(Ordering::Relaxed); From b5e4784c4cccbcea1f9d547c6659f805e6d37d30 Mon Sep 17 00:00:00 2001 From: DjDeveloperr Date: Tue, 10 Nov 2026 19:48:38 -0500 Subject: [PATCH 11/14] drm/asahi: g17: M5: write a render pair on the cursors of its room check An M5 render publication read the shared compute ring's three cursors twice under the device mutex: for the room check before pool accounting, and again when it wrote the pair. As for a compute publication, only this publication can move the producer between the two and the firmware's consumers only advance, so the first sample stays conservative. Keep it in the publication's host for the first pair written to the ring; any other pair reads the ring as before. Signed-off-by: Diljit Singh --- drivers/gpu/drm/asahi/g17/initdata.rs | 24 +++++++++++++-- drivers/gpu/drm/asahi/g17/mod.rs | 4 ++- drivers/gpu/drm/asahi/g17/runtime/render.rs | 30 ++++++++++++++++++- .../drm/asahi/g17/runtime/render/submit.rs | 26 ++++++++++------ 4 files changed, 70 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/asahi/g17/initdata.rs b/drivers/gpu/drm/asahi/g17/initdata.rs index d7e6cb521359f..3929f82c279cc 100644 --- a/drivers/gpu/drm/asahi/g17/initdata.rs +++ b/drivers/gpu/drm/asahi/g17/initdata.rs @@ -708,6 +708,18 @@ impl InitData { Ok((self.outer_capacity(priority, cursors)? >= 1).then_some(cursors)) } + /// M5 render: the shared ring's cursors when it has room for a pair, as + /// [`Self::work_ready`] judges it, for the pair's publication in the same hold + /// ([`Self::m5_work_pair`]). + pub(super) fn m5_render_room(&self, priority: u8) -> Result> { + use super::fw::queue::DataMaster; + if !self.m5 { + return Err(EINVAL); + } + let cursors = self.work_cursors(priority, DataMaster::Compute)?; + Ok((self.outer_capacity(priority, cursors)? >= 2).then_some(cursors)) + } + /// `cursors`: an M5 compute room check's sample ([`Self::m5_compute_room`]), else read here. fn work_publication<'a>( &'a self, @@ -808,11 +820,14 @@ impl InitData { /// Reserve both common-ring slots before touching either host port. The /// returned views retain the graph borrow until the pair is published. + /// `cursors`: a room check's sample from the same hold ([`Self::m5_render_room`]), + /// else read here. pub(super) fn m5_work_pair( &self, priority: u8, fragment: &super::fw::channels::WorkSlot, tiling: &super::fw::channels::WorkSlot, + cursors: Option<[u32; 3]>, ) -> Result> { use super::fw::{channels, queue::DataMaster}; if !self.m5 @@ -821,7 +836,10 @@ impl InitData { { return Err(EINVAL); } - let cursors = self.work_cursors(priority, DataMaster::Compute)?; + let cursors = match cursors { + Some(cursors) => cursors, + None => self.work_cursors(priority, DataMaster::Compute)?, + }; if self.outer_capacity(priority, cursors)? < 2 { return Err(EBUSY); } let window = work_pair::Window::new(cursors)?; let channel = channels::work_ring_index(priority, DataMaster::Compute).ok_or(EINVAL)?; @@ -863,7 +881,7 @@ impl InitData { return Err(EINVAL); } if self.m5 { - let pair = self.m5_work_pair(priority, fragment, tiling)?; + let pair = self.m5_work_pair(priority, fragment, tiling, None)?; pair.prepare(); commit_accounting()?; publish_late_tiling(); @@ -934,7 +952,7 @@ impl InitData { ) -> Result { use super::fw::queue::DataMaster; if self.m5 { - let pair = self.m5_work_pair(priority, fragment, tiling)?; + let pair = self.m5_work_pair(priority, fragment, tiling, None)?; if pair.next() != next { return Err(EBUSY); } pair.publish(); return Ok(()); diff --git a/drivers/gpu/drm/asahi/g17/mod.rs b/drivers/gpu/drm/asahi/g17/mod.rs index 4e68d0bd74a47..4ef0a5d86965f 100644 --- a/drivers/gpu/drm/asahi/g17/mod.rs +++ b/drivers/gpu/drm/asahi/g17/mod.rs @@ -798,11 +798,13 @@ impl Firmware { commit_accounting: impl FnOnce() -> Result, publish_inner: impl FnOnce(), deferred: bool, + cursors: Option<[u32; 3]>, ) -> Result<[u32; 2]> { let mut commit_accounting = Some(commit_accounting); let mut publish_inner = Some(publish_inner); + let mut cursors = cursors; for attempt in 0..2 { - let pair = self.init.m5_work_pair(priority, &records[0], &records[1])?; + let pair = self.init.m5_work_pair(priority, &records[0], &records[1], cursors.take())?; pair.prepare(); if let Some(commit) = commit_accounting.take() { commit()?; } match self.bridge.regs.submit_m5_work(submission) { diff --git a/drivers/gpu/drm/asahi/g17/runtime/render.rs b/drivers/gpu/drm/asahi/g17/runtime/render.rs index 4b62d96d7cd3c..ede61554da568 100644 --- a/drivers/gpu/drm/asahi/g17/runtime/render.rs +++ b/drivers/gpu/drm/asahi/g17/runtime/render.rs @@ -526,8 +526,19 @@ struct Host<'a> { registration_powered: bool, /// An M5 pair's installation, submitted with its first outer publication. m5_registration: Option, + /// M5: the shared ring's room check of this publication, by priority. It serves the + /// first pair written to the ring only. + outer_cursors: Option<(u8, [u32; 3])>, } impl Host<'_> { + /// The room check's cursors for the first pair this publication writes to the ring. + fn room_cursors(&mut self, priority: u8) -> Option<[u32; 3]> { + self.outer_cursors + .take() + .filter(|&(checked, _)| checked == priority) + .map(|(_, cursors)| cursors) + } + fn release_registration(&mut self) { if !self.registration_powered && !self.firmware.registration_power_release_pending { return; @@ -634,6 +645,7 @@ impl crate::g17::queue::render::Host for Host<'_> { tiling.qid, )?, }; + let cursors = self.room_cursors(priority); return self.firmware.submit_m5_render_pair( priority, &[copy_slot(fragment), copy_slot(tiling)], @@ -641,6 +653,7 @@ impl crate::g17::queue::render::Host for Host<'_> { commit, late_tiling, true, + cursors, ); } self.firmware @@ -654,7 +667,8 @@ impl crate::g17::queue::render::Host for Host<'_> { tiling: &crate::g17::fw::channels::WorkSlot, prepare: impl FnOnce(), ) -> Result { - let pair = self.firmware.init.m5_work_pair(priority, fragment, tiling)?; + let cursors = self.room_cursors(priority); + let pair = self.firmware.init.m5_work_pair(priority, fragment, tiling, cursors)?; pair.prepare(); prepare(); pair.publish(); @@ -677,6 +691,8 @@ impl crate::g17::queue::render::Host for Host<'_> { tiling: &crate::g17::fw::channels::WorkSlot, next: [u32; 2], ) -> Result { + // This pair moves the producer: a room check's sample no longer holds. + self.outer_cursors = None; let result = self .firmware .init @@ -762,6 +778,17 @@ impl crate::g17::Firmware { &mut self, slot: usize, operation: impl FnOnce(&mut Pair, &mut Host<'_>) -> Result, + ) -> Result { + self.with_render_at(slot, None, operation) + } + + /// [`Self::with_render`] for a publication whose shared-ring room check, made in this + /// hold, sampled `outer_cursors`. + fn with_render_at( + &mut self, + slot: usize, + outer_cursors: Option<(u8, [u32; 3])>, + operation: impl FnOnce(&mut Pair, &mut Host<'_>) -> Result, ) -> Result { let mut pair = self .queues @@ -775,6 +802,7 @@ impl crate::g17::Firmware { firmware: self, registration_powered: false, m5_registration: None, + outer_cursors, }; let result = operation(&mut pair, &mut host); host.release_registration(); diff --git a/drivers/gpu/drm/asahi/g17/runtime/render/submit.rs b/drivers/gpu/drm/asahi/g17/runtime/render/submit.rs index 4f10efce29cb9..d35c2cdd66420 100644 --- a/drivers/gpu/drm/asahi/g17/runtime/render/submit.rs +++ b/drivers/gpu/drm/asahi/g17/runtime/render/submit.rs @@ -749,15 +749,23 @@ impl crate::g17::Firmware { let port = self.port.clone(); let _render = port.render_lock(); // Both outer destinations must have room before pool accounting or - // either inner queue becomes visible. On M5 both are the shared compute ring, and - // one check covers the pair (each reads the ring's cursors from shared memory). + // either inner queue becomes visible. On M5 both are the shared compute ring: one + // check covers the pair, and the pair is written against its cursors (this hold + // is the ring's only producer; the firmware's consumers only advance). let priority = packet.context.policy().priority(); - if !self.init.work_ready(priority, crate::g17::fw::queue::DataMaster::Fragment)? - || (!self.init.is_m5() - && !self.init.work_ready(priority, crate::g17::fw::queue::DataMaster::Tiling)?) - { - return Err(EBUSY); - } + let outer_cursors = if self.init.is_m5() { + let Some(cursors) = self.init.m5_render_room(priority)? else { + return Err(EBUSY); + }; + Some((priority, cursors)) + } else { + if !self.init.work_ready(priority, crate::g17::fw::queue::DataMaster::Fragment)? + || !self.init.work_ready(priority, crate::g17::fw::queue::DataMaster::Tiling)? + { + return Err(EBUSY); + } + None + }; if let Some(footprint) = packet.footprint.as_ref() { dependencies.hazard_count = self.queues.admission.publication_barriers( owner, @@ -798,7 +806,7 @@ impl crate::g17::Firmware { buffer_token, buffer_context: buffer_context.map(|(context, _)| context), }; - let result = self.with_render(usize::from(slot), |pair, host| { + let result = self.with_render_at(usize::from(slot), outer_cursors, |pair, host| { pair.publish( packet.clone(), &dependencies, From 35a81e0149e5583a9d6f8fa28d68b9040bdef3f0 Mon Sep 17 00:00:00 2001 From: DjDeveloperr Date: Tue, 10 Nov 2026 19:48:39 -0500 Subject: [PATCH 12/14] drm/asahi: g17: drop the discarded job-list reads from render snapshots Each render observation snapshot loaded the pair's private job-list head and tail and discarded both values. Nothing depends on them (relaxed loads whose values are unused order nothing), and the view is uncached, so each was a DRAM round trip in the event worker's device-mutex hold. Signed-off-by: Diljit Singh --- .../gpu/drm/asahi/g17/queue/render/observation.rs | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/asahi/g17/queue/render/observation.rs b/drivers/gpu/drm/asahi/g17/queue/render/observation.rs index 3e853115e4f75..8c7e836cf1c90 100644 --- a/drivers/gpu/drm/asahi/g17/queue/render/observation.rs +++ b/drivers/gpu/drm/asahi/g17/queue/render/observation.rs @@ -64,18 +64,8 @@ pub(super) fn snapshot(memory: &memory::Memory, ordinal: u64) -> Result Date: Tue, 10 Nov 2026 19:48:39 -0500 Subject: [PATCH 13/14] drm/asahi: g17: read only the producers behind a render in flight A render publication read both item rings' done, read and write cursors, six uncached loads under the device mutex, to check that the producers are where the pair left them. Behind a command in flight the done and read cursors are unused: the next command's previous cursors come from that command's windows. Read the two producers alone then; an idle pair reads all six as before. Signed-off-by: Diljit Singh --- drivers/gpu/drm/asahi/g17/queue/render.rs | 23 ++++++++++++++----- .../gpu/drm/asahi/g17/queue/render/memory.rs | 11 +++++++++ 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/asahi/g17/queue/render.rs b/drivers/gpu/drm/asahi/g17/queue/render.rs index ca504ed87db91..2b7ee531560b2 100644 --- a/drivers/gpu/drm/asahi/g17/queue/render.rs +++ b/drivers/gpu/drm/asahi/g17/queue/render.rs @@ -885,8 +885,17 @@ impl Pair { } publication::required(cache[stage], config) }); - let cursors = self.memory.graph.cursors()?; - if cursors[0].write != self.cursors[0] || cursors[1].write != self.cursors[1] { + // An idle pair's next command starts from the rings' cursors; behind a command in + // flight it starts from that command's windows, and only the producers are checked. + let cursors = match self.active.front() { + Some(_) => None, + None => Some(self.memory.graph.cursors()?), + }; + let producers = match cursors { + Some(cursors) => cursors.map(|cursors| cursors.write), + None => self.memory.graph.write_cursors()?, + }; + if producers != self.cursors { return Err(EBUSY); } let queues = qids.map(|qid| self.memory.graph.queue_va(qid)); @@ -1022,16 +1031,18 @@ impl Pair { )?, ]; let windows = [lanes[0].window, lanes[1].window]; - let previous = self.active.front().map_or(cursors, |active| { - active.ticket.windows.map(|window| { + let previous = match (self.active.front(), cursors) { + (Some(active), _) => active.ticket.windows.map(|window| { let target = window.target(); retirement::Cursors { done: target, read: target, write: target, } - }) - }); + }), + (None, Some(cursors)) => cursors, + (None, None) => return Err(EIO), + }; let fragment_slot = WorkSlot::new( DataMaster::Fragment, queues[1], diff --git a/drivers/gpu/drm/asahi/g17/queue/render/memory.rs b/drivers/gpu/drm/asahi/g17/queue/render/memory.rs index 65615e1087037..e8881cdc0ad80 100644 --- a/drivers/gpu/drm/asahi/g17/queue/render/memory.rs +++ b/drivers/gpu/drm/asahi/g17/queue/render/memory.rs @@ -283,6 +283,17 @@ impl Graph { } Ok(values) } + /// The two rings' producers alone: the `write` of [`Self::cursors`]. + pub(super) fn write_cursors(&self) -> Result<[u32; 2]> { + let mut values = [0; 2]; + for (stage, value) in values.iter_mut().enumerate() { + *value = self + .queues + .word(POINTERS[stage] + self.pointers.producer)? + .load(Ordering::Relaxed); + } + Ok(values) + } pub(super) fn claim(&mut self, ordinal: u64) -> Result { let owner = &mut self.owners[ordinal as usize % RECORDS]; if owner.is_some_and(|old| old != ordinal) { From 4f4deefeb52675567e12c68cde63ff7facf320d7 Mon Sep 17 00:00:00 2001 From: DjDeveloperr Date: Tue, 10 Nov 2026 20:09:13 -0500 Subject: [PATCH 14/14] drm/asahi: g17: M5: release a retained render pair on its room check's consumers An M5 render publication writes its pair deferred and then releases it, and the release read the shared ring's three cursors again under the same hold of the device mutex. Its consumers from the room check stay conservative for the whole hold: the firmware only advances them, and with the publication's own credits lent to it the other reservations can only shrink. Use them, with the producer read again: the release still revalidates the producer the pair was prepared against. Signed-off-by: Diljit Singh --- drivers/gpu/drm/asahi/g17/initdata.rs | 19 ++++++++++- drivers/gpu/drm/asahi/g17/runtime/render.rs | 35 +++++++++++++++------ 2 files changed, 44 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/asahi/g17/initdata.rs b/drivers/gpu/drm/asahi/g17/initdata.rs index 3929f82c279cc..3164097665f5c 100644 --- a/drivers/gpu/drm/asahi/g17/initdata.rs +++ b/drivers/gpu/drm/asahi/g17/initdata.rs @@ -771,6 +771,21 @@ impl InitData { }) } + /// The host producer of the shared M5 ring alone (the last of [`Self::work_cursors`]). + pub(super) fn work_producer(&self, priority: u8) -> Result { + use super::fw::{channels, queue::DataMaster}; + let channel = channels::work_ring_index(priority, DataMaster::Compute).ok_or(EINVAL)?; + let producer = self + .object(self.cluster)? + .word(self.state_offset(Role::Primary) + abi::work_state_offset(channel) + + abi::ChannelState::PRODUCER)? + .load(Ordering::Relaxed); + if producer >= channels::WORK_RING_SLOTS { + return Err(EINVAL); + } + Ok(producer) + } + /// Two firmware consumers and the host producer, sampled in that order. pub(super) fn work_cursors( &self, @@ -943,16 +958,18 @@ impl InitData { /// Release a retained pair after its control transaction. Revalidate the original producer /// witnesses before either copy; both bodies precede the fragment and tiling publications. + /// `cursors`: M5, as for [`Self::m5_work_pair`]. pub(super) fn publish_retained_work_pair( &self, priority: u8, fragment: &super::fw::channels::WorkSlot, tiling: &super::fw::channels::WorkSlot, next: [u32; 2], + cursors: Option<[u32; 3]>, ) -> Result { use super::fw::queue::DataMaster; if self.m5 { - let pair = self.m5_work_pair(priority, fragment, tiling, None)?; + let pair = self.m5_work_pair(priority, fragment, tiling, cursors)?; if pair.next() != next { return Err(EBUSY); } pair.publish(); return Ok(()); diff --git a/drivers/gpu/drm/asahi/g17/runtime/render.rs b/drivers/gpu/drm/asahi/g17/runtime/render.rs index ede61554da568..2a3c78e6f32c5 100644 --- a/drivers/gpu/drm/asahi/g17/runtime/render.rs +++ b/drivers/gpu/drm/asahi/g17/runtime/render.rs @@ -526,19 +526,36 @@ struct Host<'a> { registration_powered: bool, /// An M5 pair's installation, submitted with its first outer publication. m5_registration: Option, - /// M5: the shared ring's room check of this publication, by priority. It serves the - /// first pair written to the ring only. + /// M5: the shared ring's room check of this publication, by priority. Its consumers stay + /// conservative for the whole hold (the firmware only advances them); its producer + /// serves the first pair written to the ring only. outer_cursors: Option<(u8, [u32; 3])>, + outer_producer_used: bool, } impl Host<'_> { /// The room check's cursors for the first pair this publication writes to the ring. fn room_cursors(&mut self, priority: u8) -> Option<[u32; 3]> { + if core::mem::replace(&mut self.outer_producer_used, true) { + return None; + } self.outer_cursors - .take() .filter(|&(checked, _)| checked == priority) .map(|(_, cursors)| cursors) } + /// The room check's consumers with the producer read now: a retained pair revalidates the + /// producer it was prepared against. + fn retained_cursors(&mut self, priority: u8) -> Result> { + self.outer_producer_used = true; + let Some((checked, [consumer, consumer_b, _])) = self.outer_cursors else { + return Ok(None); + }; + if checked != priority { + return Ok(None); + } + Ok(Some([consumer, consumer_b, self.firmware.init.work_producer(priority)?])) + } + fn release_registration(&mut self) { if !self.registration_powered && !self.firmware.registration_power_release_pending { return; @@ -691,12 +708,11 @@ impl crate::g17::queue::render::Host for Host<'_> { tiling: &crate::g17::fw::channels::WorkSlot, next: [u32; 2], ) -> Result { - // This pair moves the producer: a room check's sample no longer holds. - self.outer_cursors = None; - let result = self - .firmware - .init - .publish_retained_work_pair(priority, fragment, tiling, next); + let result = self.retained_cursors(priority).and_then(|cursors| { + self.firmware + .init + .publish_retained_work_pair(priority, fragment, tiling, next, cursors) + }); if result.is_err() { self.firmware.primary.state.shared.lose_device_quietly(); } @@ -803,6 +819,7 @@ impl crate::g17::Firmware { registration_powered: false, m5_registration: None, outer_cursors, + outer_producer_used: false, }; let result = operation(&mut pair, &mut host); host.release_registration();