From 675de99813b5aac94cbc6ee47b8f94f22d3e0678 Mon Sep 17 00:00:00 2001 From: DjDeveloperr Date: Tue, 10 Nov 2026 21:04:54 -0500 Subject: [PATCH 1/4] drm/asahi: g17: keep the QoS submission counts the host writes Each QoS publication read the queue, owner and data-master submission counts back from the uncached table before storing them incremented, and the busy-interval count when an engine went busy: seven or eight loads per render pair, made under the device mutex. Only the host writes these words; the firmware reads them. Keep the values last stored in the accounting and count on from those. The rows are still read, and the stores and their order are unchanged. Signed-off-by: Diljit Singh --- drivers/gpu/drm/asahi/g17/qos.rs | 35 +++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/asahi/g17/qos.rs b/drivers/gpu/drm/asahi/g17/qos.rs index a90fe51539cb1..093e61c1b946f 100644 --- a/drivers/gpu/drm/asahi/g17/qos.rs +++ b/drivers/gpu/drm/asahi/g17/qos.rs @@ -95,6 +95,12 @@ pub(crate) struct Accounting { in_flight: [u32; fw::QUEUES], dm_in_flight: [u64; fw::DATA_MASTERS], dm_by_qid: [u8; fw::QUEUES], + /// The table's counts as last stored, indexed as in [`View`]. Only the host writes + /// them, so a publication counts on from these instead of reading them back. + queue_submitted: [u32; fw::QUEUES], + owner_submitted: [u32; fw::QUEUES], + intervals: u64, + times_and_counts: [u64; 7], } impl Accounting { pub(crate) const fn new() -> Self { @@ -102,6 +108,10 @@ impl Accounting { in_flight: [0; fw::QUEUES], dm_in_flight: [0; fw::DATA_MASTERS], dm_by_qid: [0; fw::QUEUES], + queue_submitted: [0; fw::QUEUES], + owner_submitted: [0; fw::QUEUES], + intervals: 0, + times_and_counts: [0; 7], } } @@ -124,7 +134,8 @@ impl Accounting { /// firmware-visible values as one `publish` per owner. The owners use /// distinct queues and data masters. Every check precedes the first store, /// so a refusal changes nothing; every table word is read before the first - /// store, so no uncached read waits for an earlier store to drain. + /// store, so no uncached read waits for an earlier store to drain. The counts come from + /// the host's copy; only the rows are read. pub(crate) fn publish_owners( &mut self, view: &View<'_>, @@ -150,12 +161,12 @@ impl Accounting { self.dm_in_flight[dm].checked_add(1).ok_or(EOVERFLOW)?; } let rows = indices.map(|(qid, _, _)| view.row_word(qid)); - let queue_submitted = indices.map(|(qid, _, _)| view.queue_submitted[qid].load(Ordering::Relaxed)); - let owner_submitted = indices.map(|(_, qos, _)| view.owner_submitted[qos].load(Ordering::Relaxed)); - let dm_submitted = indices.map(|(_, _, dm)| view.times_and_counts[3 + dm].load(Ordering::Relaxed)); + let queue_submitted = indices.map(|(qid, _, _)| self.queue_submitted[qid]); + let owner_submitted = indices.map(|(_, qos, _)| self.owner_submitted[qos]); + let dm_submitted = indices.map(|(_, _, dm)| self.times_and_counts[3 + dm]); let starts_busy = indices.map(|(_, _, dm)| self.dm_in_flight[dm] == 0); let busy = starts_busy.iter().any(|&starts| starts); - let intervals = if busy { view.intervals.load(Ordering::Relaxed) } else { 0 }; + let intervals = if busy { self.intervals } else { 0 }; for (index, owner) in owners.iter().enumerate() { let (qid, _, _) = indices[index]; if self.in_flight[qid] != 0 @@ -191,21 +202,25 @@ impl Accounting { let mut intervals = intervals; for (index, owner) in owners.iter().enumerate() { let (qid, qos, dm) = indices[index]; - view.queue_submitted[qid].store(queue_submitted[index].wrapping_add(1), Ordering::Relaxed); + let queued = queue_submitted[index].wrapping_add(1); + view.queue_submitted[qid].store(queued, Ordering::Relaxed); + self.queue_submitted[qid] = queued; // Owners of one logical queue share their submission count. let earlier = indices[..index].iter().filter(|&&(_, other, _)| other == qos).count(); - view.owner_submitted[qos].store( - owner_submitted[index].wrapping_add(earlier as u32 + 1), - Ordering::Relaxed, - ); + let owned = owner_submitted[index].wrapping_add(earlier as u32 + 1); + view.owner_submitted[qos].store(owned, Ordering::Relaxed); + self.owner_submitted[qos] = owned; if starts_busy[index] { view.times_and_counts[dm].store(time, Ordering::Relaxed); + self.times_and_counts[dm] = time; } let submitted = dm_submitted[index].wrapping_add(1); view.times_and_counts[3 + dm].store(submitted, Ordering::Relaxed); + self.times_and_counts[3 + dm] = submitted; if starts_busy[index] { intervals = intervals.wrapping_add(1); view.intervals.store(intervals, Ordering::Relaxed); + self.intervals = intervals; } let mut record = rows[index]; record[0] = owner.qos; From 568a2f81673117f7d185adc50c5e99d48d6e6f4a Mon Sep 17 00:00:00 2001 From: DjDeveloperr Date: Tue, 10 Nov 2026 21:05:19 -0500 Subject: [PATCH 2/4] drm/asahi: g17: keep the QoS rows the host writes A QoS publication still read each owner's row from the uncached table, for the record a cancellation compares and to refuse an owner change while work is in flight, and every completion read the row's owner byte back to validate it: two loads per render pair under the device mutex and two in the event worker's. Only the host writes the rows. Keep them with the counts and check against the copy; a cancellation still compares the table. Signed-off-by: Diljit Singh --- drivers/gpu/drm/asahi/g17/qos.rs | 29 ++++++++++++----------------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/asahi/g17/qos.rs b/drivers/gpu/drm/asahi/g17/qos.rs index 093e61c1b946f..5adccb14b5cad 100644 --- a/drivers/gpu/drm/asahi/g17/qos.rs +++ b/drivers/gpu/drm/asahi/g17/qos.rs @@ -53,14 +53,6 @@ impl<'a> View<'a> { fn row(&self, qid: usize) -> [u8; 8] { core::array::from_fn(|index| self.rows[qid][index].load(Ordering::Relaxed)) } - /// The complete row in one access. The table is uncached: a publication reads - /// each row once, before it stores anything. - fn row_word(&self, qid: usize) -> [u8; 8] { - let row = core::ptr::from_ref(&self.rows[qid]).cast::(); - // SAFETY: Rows are eight bytes at eight-byte offsets of the eight-byte - // aligned table (checked in from_raw), so the row is one aligned word. - unsafe { (*row).load(Ordering::Relaxed) }.to_le_bytes() - } } #[derive(Clone, Copy, Debug)] @@ -95,8 +87,9 @@ pub(crate) struct Accounting { in_flight: [u32; fw::QUEUES], dm_in_flight: [u64; fw::DATA_MASTERS], dm_by_qid: [u8; fw::QUEUES], - /// The table's counts as last stored, indexed as in [`View`]. Only the host writes - /// them, so a publication counts on from these instead of reading them back. + /// The table's rows and counts as last stored, indexed as in [`View`]. Only the host + /// writes them, so a publication counts on from these instead of reading them back. + rows: [[u8; 8]; fw::QUEUES], queue_submitted: [u32; fw::QUEUES], owner_submitted: [u32; fw::QUEUES], intervals: u64, @@ -108,6 +101,7 @@ impl Accounting { in_flight: [0; fw::QUEUES], dm_in_flight: [0; fw::DATA_MASTERS], dm_by_qid: [0; fw::QUEUES], + rows: [[0; 8]; fw::QUEUES], queue_submitted: [0; fw::QUEUES], owner_submitted: [0; fw::QUEUES], intervals: 0, @@ -133,9 +127,8 @@ impl Accounting { /// Publishes the owners of one command's queues, in order, with the same /// firmware-visible values as one `publish` per owner. The owners use /// distinct queues and data masters. Every check precedes the first store, - /// so a refusal changes nothing; every table word is read before the first - /// store, so no uncached read waits for an earlier store to drain. The counts come from - /// the host's copy; only the rows are read. + /// so a refusal changes nothing. The rows and counts come from the host's + /// copy: nothing is read back from the uncached table. pub(crate) fn publish_owners( &mut self, view: &View<'_>, @@ -160,7 +153,7 @@ impl Accounting { self.in_flight[qid].checked_add(1).ok_or(EOVERFLOW)?; self.dm_in_flight[dm].checked_add(1).ok_or(EOVERFLOW)?; } - let rows = indices.map(|(qid, _, _)| view.row_word(qid)); + let rows = indices.map(|(qid, _, _)| self.rows[qid]); let queue_submitted = indices.map(|(qid, _, _)| self.queue_submitted[qid]); let owner_submitted = indices.map(|(_, qos, _)| self.owner_submitted[qos]); let dm_submitted = indices.map(|(_, _, dm)| self.times_and_counts[3 + dm]); @@ -187,6 +180,7 @@ impl Accounting { let (qid, qos, _) = indices[index]; view.rows[qid][0].store(owner.qos, Ordering::Relaxed); view.rows[qid][1].store(class, Ordering::Relaxed); + self.rows[qid][..2].copy_from_slice(&[owner.qos, class]); view.scheduler[qos].store(scheduler, Ordering::Relaxed); view.share[qos].store(policy.qos_share(), Ordering::Relaxed); first |= self.in_flight[qid] == 0; @@ -262,8 +256,8 @@ impl Accounting { if ids & (1u128 << qos) == 0 { continue; } - if self.in_flight.iter().zip(view.rows.iter()) - .any(|(&count, row)| count != 0 && usize::from(row[0].load(Ordering::Relaxed)) == qos) + if self.in_flight.iter().zip(self.rows.iter()) + .any(|(&count, row)| count != 0 && usize::from(row[0]) == qos) { left |= 1u128 << qos; continue; @@ -285,7 +279,7 @@ impl Accounting { return Err(EINVAL); } if self.in_flight[qid] == 0 - || view.rows[qid][0].load(Ordering::Relaxed) != owner.qos + || self.rows[qid][0] != owner.qos || self.dm_by_qid[qid] != owner.data_master { return Err(EIO); @@ -302,6 +296,7 @@ impl Accounting { if self.in_flight[qid] == 1 { view.rows[qid][0].store(0xff, Ordering::Relaxed); view.rows[qid][1].store(0, Ordering::Relaxed); + self.rows[qid][..2].copy_from_slice(&[0xff, 0]); } } for owner in owners { From e5db4bdd1847c8ad70d2d174cd96afd1cd03efe1 Mon Sep 17 00:00:00 2001 From: DjDeveloperr Date: Tue, 10 Nov 2026 20:09:14 -0500 Subject: [PATCH 3/4] drm/asahi: g17: M5: count a render pair in one submission-history pass The render doorbell counted the pair's tiling and fragment submissions with two passes over the firmware's submission history, each loading the history generation and the slot byte (uncached, under the device mutex) and ending in a full fence. Count both in one pass, in the slot of that moment, as two passes microseconds apart do. Signed-off-by: Diljit Singh --- drivers/gpu/drm/asahi/g17/initdata/m5.rs | 5 +++-- drivers/gpu/drm/asahi/g17/runtime.rs | 2 +- drivers/gpu/drm/asahi/g17/runtime/render.rs | 9 ++++----- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/asahi/g17/initdata/m5.rs b/drivers/gpu/drm/asahi/g17/initdata/m5.rs index 2bc0647da9624..5851c3ae15b14 100644 --- a/drivers/gpu/drm/asahi/g17/initdata/m5.rs +++ b/drivers/gpu/drm/asahi/g17/initdata/m5.rs @@ -48,7 +48,8 @@ impl InitData { Ok(self.object(self.bundle)?.word(at)?.load(Ordering::Relaxed)) } - pub(crate) fn note_m5_submission_history(&self) -> Result { + /// Counts `submissions` in the firmware's submission history, in the slot of this moment. + pub(crate) fn note_m5_submission_history(&self, submissions: u8) -> Result { if !self.m5 { return Err(EINVAL); } let private = self.object(self.cluster)?; let base = m::M5_PRIVATE_LAYOUT.status_config; @@ -81,7 +82,7 @@ impl InitData { // SAFETY: retained, bounds-checked WC byte, written as a single byte // after reading the even, stable generation, as the firmware expects. unsafe { - let value = core::ptr::read_volatile(byte).wrapping_add(1); + let value = core::ptr::read_volatile(byte).wrapping_add(submissions); core::ptr::write_volatile(byte, value); }; fence(Ordering::SeqCst); diff --git a/drivers/gpu/drm/asahi/g17/runtime.rs b/drivers/gpu/drm/asahi/g17/runtime.rs index f0c9ccc8a2ae6..e591c5f99d470 100644 --- a/drivers/gpu/drm/asahi/g17/runtime.rs +++ b/drivers/gpu/drm/asahi/g17/runtime.rs @@ -1559,7 +1559,7 @@ impl compute::Host for ComputeHost<'_> { // Every M5 pipe-2 doorbell (0x83) is noted in the firmware's submission history first, // as the render doorbell notes its pair. if self.init.is_m5() && message >> super::MSG_TYPE_SHIFT == 0x83 && message & 3 == 2 { - if let Err(error) = self.init.note_m5_submission_history() { + if let Err(error) = self.init.note_m5_submission_history(1) { pr_warn!("asahi: M5 submission history skipped: {:?}\n", error); } } diff --git a/drivers/gpu/drm/asahi/g17/runtime/render.rs b/drivers/gpu/drm/asahi/g17/runtime/render.rs index 2a3c78e6f32c5..cb5b4f781af62 100644 --- a/drivers/gpu/drm/asahi/g17/runtime/render.rs +++ b/drivers/gpu/drm/asahi/g17/runtime/render.rs @@ -724,11 +724,10 @@ impl crate::g17::queue::render::Host for Host<'_> { self.release_registration(); let m5 = self.firmware.init.is_m5(); if m5 { - // A paired publication is one tiling and one fragment submission. - for _ in 0..2 { - if let Err(error) = self.firmware.init.note_m5_submission_history() { - pr_warn!("asahi: M5 submission history skipped: {:?}\n", error); - } + // A paired publication is one tiling and one fragment submission, counted in + // one slot. + if let Err(error) = self.firmware.init.note_m5_submission_history(2) { + pr_warn!("asahi: M5 submission history skipped: {:?}\n", error); } } else { crate::g17::trace::doorbell(crate::g17::trace::RENDER, || { From cdbb2e0571edfbb0a0dabbf7b91baa3c2c2ed434 Mon Sep 17 00:00:00 2001 From: DjDeveloperr Date: Tue, 10 Nov 2026 21:27:15 -0500 Subject: [PATCH 4/4] drm/asahi: g17: M5: trust the effort word the host last wrote Every submission re-applies full effort, and when the value is unchanged it read the firmware's effort word back from region C to confirm it: one uncached load per compute and render submission, under the device mutex. On M5 region C is mapped read-only to the firmware, so once the host has written the word, or read it back equal, it holds the value applied. Skip the read-back then; other generations keep it. Signed-off-by: Diljit Singh --- drivers/gpu/drm/asahi/g17/mod.rs | 12 ++++++++++-- drivers/gpu/drm/asahi/g17/power.rs | 10 ++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/asahi/g17/mod.rs b/drivers/gpu/drm/asahi/g17/mod.rs index 4ef0a5d86965f..5efe538a737ef 100644 --- a/drivers/gpu/drm/asahi/g17/mod.rs +++ b/drivers/gpu/drm/asahi/g17/mod.rs @@ -887,8 +887,16 @@ impl Firmware { // given, and the state records, the M5 value (InitData::submission_effort). let published = init.submission_effort(effort, state.waited()); port.set_effort_full(effort == power::FULL_EFFORT); - if state.applied() == published && init.effort()? == published { - return Ok(()); + if state.applied() == published { + // M5 maps region C read-only to the firmware: once confirmed, the word holds + // `applied` without reading it back. + if init.is_m5() && state.confirmed() { + return Ok(()); + } + if init.effort()? == published { + state.confirm(); + return Ok(()); + } } init.publish_effort(published)?; trace::message(trace::EFFORT, u64::from(published)); diff --git a/drivers/gpu/drm/asahi/g17/power.rs b/drivers/gpu/drm/asahi/g17/power.rs index 5c57f6a289440..8be3bae04afb0 100644 --- a/drivers/gpu/drm/asahi/g17/power.rs +++ b/drivers/gpu/drm/asahi/g17/power.rs @@ -39,6 +39,8 @@ pub(super) struct Effort { streaming: bool, idle_since: Option, applied: u32, + /// The firmware's effort word is known to hold `applied`: the host wrote it, or read it. + confirmed: bool, submissions: u64, /// A host thread waits, or waited within [`IDLE_HOLD_NS`], for submitted work /// ([`Self::note_waiters`]). @@ -55,6 +57,7 @@ impl Effort { streaming: false, idle_since: None, applied: 0, + confirmed: false, submissions: 0, waited: false, seen_waits: 0, @@ -102,6 +105,13 @@ impl Effort { } pub(super) fn did_apply(&mut self, value: u32) { self.applied = value; + self.confirmed = true; + } + pub(super) fn confirmed(&self) -> bool { + self.confirmed + } + pub(super) fn confirm(&mut self) { + self.confirmed = true; } /// Quarantined jobs do not count as work in flight; they retain mappings but cannot justify