From cf38d3a6c7190da84dd37ee705acea47fde2af54 Mon Sep 17 00:00:00 2001 From: DjDeveloperr Date: Mon, 26 Oct 2026 20:10:38 -0400 Subject: [PATCH] drm/asahi: observe and retire a G17 render pair in one device-mutex hold The event worker read a render pair's completion memory outside the device mutex. It took the pair out of its entry on loan under one hold, read the front's stamps and graph cursors unlocked, then took the mutex again to return the pair and retire what it had seen. The read is a short memory snapshot, so the loan bought no useful concurrency, and every observed pair cost two acquisitions of the mutex that submission also takes, with publishers able to slip in between. Select the pair, read its front and retire it in one hold. The pair stays in its entry, so observations no longer need the loan bookkeeping (the preparing marker and the borrowed dependency and switch snapshots) or the return path and its invariant-failure handling. Preparers keep using the loan. The read takes the context's work mutex for the shared list head, which now nests inside the device mutex; that mutex is a leaf, held only around accesses to the context's work storage. Measured on Neo across boots, with this change, without it, and with it again (3 rounds each, medians, us per submit): render.light tri queue d8 31.29 / 33.73 / 31.04 (-7.6%) render.light tri queue d1 105.49 / 105.75 / 105.53 render.light tri queue d32/d64 30.06 / 30.01 / 29.68 render.light tri procs d2 52.18 / 52.24 / 52.29 render.light tri queues d2 51.98 / 52.12 / 52.12 compute.light queue d1 103.61 / 103.73 / 104.34 sync.signal queues d2 timeline 22.74 / 22.67 / 22.69 sync.signal queues d2 binary 22.53 / 22.27 / 22.53 --- .../drm/asahi/g17/runtime/render/observe.rs | 99 ++++++------------- 1 file changed, 28 insertions(+), 71 deletions(-) diff --git a/drivers/gpu/drm/asahi/g17/runtime/render/observe.rs b/drivers/gpu/drm/asahi/g17/runtime/render/observe.rs index 5ae76639f6521..e6077624ba4f2 100644 --- a/drivers/gpu/drm/asahi/g17/runtime/render/observe.rs +++ b/drivers/gpu/drm/asahi/g17/runtime/render/observe.rs @@ -1,21 +1,22 @@ // SPDX-License-Identifier: GPL-2.0-only OR MIT -//! Event-driven render snapshots outside the device mutex. The preparation -//! epoch and exact pair owner survive every read, cancellation and retry. +//! Event-driven render snapshots. A pair's completion memory is read and the +//! pair retired in one device-mutex hold, inside the preparation epoch. use super::super::DeferredBatch; -use crate::g17::queue::render::Pair; use core::sync::atomic::Ordering; use kernel::{prelude::*, sync::Arc}; impl crate::g17::Firmware { - fn take_render_observation(&mut self, start: usize) -> Result)>> { + /// The first slot from `start` whose pair has news to observe: published work in flight, both + /// queues' notifications seen, and an observation the pair accepts. + fn next_render_observation(&self, start: usize) -> Result> { if self.recovery.pending() { return Ok(None); } let state = crate::g17::recovery::Memory::recovery_state(&self.init)?; for slot in start..self.queues.render.entries.len() { - let Some(entry) = self.queues.render.entries[slot].as_mut() else { + let Some(entry) = self.queues.render.entries[slot].as_ref() else { continue; }; if entry.preparing.is_some() || entry.growing { @@ -37,35 +38,13 @@ impl crate::g17::Firmware { self.init.stamp(qids[1])?.load(Ordering::Relaxed), ]; core::sync::atomic::fence(Ordering::Acquire); - let Some(packet) = pair.observation_packet(entry.event_masks, stamps, state)? else { - continue; - }; - entry.borrowed_active = pair.published_in_flight(); - entry.borrowed_dependencies = Some(pair.dependency_snapshot()); - entry.borrowed_switch = pair.switch_in_flight(); - entry.preparing = Some(packet); - return Ok(Some((slot, entry.pair.take().ok_or(EIO)?))); + if pair.observation_packet(entry.event_masks, stamps, state)?.is_some() { + return Ok(Some(slot)); + } } Ok(None) } - fn return_render_observation(&mut self, slot: usize, pair: &mut Option>) -> Result { - let returning = pair.as_ref().ok_or(EINVAL)?; - let entry = self.queues.render.entry_mut(slot as u8)?; - if entry.pair.is_some() - || entry.preparing.is_none() - || returning.slot() as usize != slot - || returning.qids() != entry.reservation.ids.map(crate::g17::kick::Id::qid) - { - return Err(EIO); - } - entry.pair = pair.take(); - entry.borrowed_dependencies = None; - entry.borrowed_switch = None; - entry.preparing = None; - Ok(()) - } - pub(super) fn settle_render_terminal( &mut self, slot: usize, @@ -122,59 +101,37 @@ impl crate::g17::Shared { Err(error) if error == EAGAIN || error == ENODEV => break, result => result?, }; - let loan = { - let mut state = self.state.lock(); - let Some(firmware) = (*state).as_deref_mut() else { - return Ok(()); - }; - if !epoch.is_current() || self.crashed.load(Ordering::Acquire) { - break; - } - firmware.take_render_observation(cursor)? + // Selecting, reading and retiring the front share one hold: a + // separate hold for the read would let publishers and the + // retirement contend twice per observed pair. + let mut state = self.state.lock(); + let Some(firmware) = (*state).as_deref_mut() else { + return Ok(()); }; - let Some((slot, pair)) = loan else { + if !epoch.is_current() + || firmware.recovery.pending() + || self.crashed.load(Ordering::Acquire) + { + break; + } + let Some(slot) = firmware.next_render_observation(cursor)? else { break; }; if slot != cursor { remaining = crate::hw::t8140::queues::RENDER_DEPTH; retries = 0; } - let observed = pair.observe_front(); - let mut pair = Some(pair); - let mut state = self.state.lock(); - // Return installed ownership before examining read errors or epoch. - let restored = (*state) - .as_deref_mut() - .ok_or(ENODEV) - .and_then(|firmware| firmware.return_render_observation(slot, &mut pair)); - if let Err(error) = restored { - // A failed invariant must not drop firmware-owned storage. Join - // the other preparers only after releasing our own epoch, and - // keep this detached owner until both processors have stopped. - drop(state); - drop(epoch); - self.lose_device_quietly(); - self.stop_runtime(); - if let Some(mut pair) = pair { - pair.stopped(&mut |event| { - event.finish(); - Ok(()) - })?; - } - return Err(error); - } - let firmware = (*state).as_deref_mut().ok_or(EIO)?; + let observed = firmware.queues.render.entries[slot] + .as_ref() + .and_then(|entry| entry.pair.as_ref()) + .ok_or(EIO)? + .observe_front(); + // Publishers waiting for render capacity recheck once this hold ends. self.note_changed(); if firmware.render_grow_pending() || firmware.queues.render.take_missed_event(slot as u8) { // Observation and the growth check run on every pass. self.queue_event_records(); } - if !epoch.is_current() - || firmware.recovery.pending() - || self.crashed.load(Ordering::Acquire) - { - break; - } let result = (|| { let state = crate::g17::recovery::Memory::recovery_state(&firmware.init)?; let progressed = match observed {