From 5601ad4902fdddabdd52adc1c209efaa83c81871 Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 1 Nov 2026 12:43:40 +0100 Subject: [PATCH 1/6] display-output: the compositor presents to the display driver, and tgl-display answers at vblank (stage 3) PSR needs to know which frame changed, and where. Without that, PSR1 sees a CPU write to the firmware's surface only through FBC host tracking of a graphics-aperture fence, which Vol 12 limits to "projects that support that". The compositor knows both, so it says them. `lib/display-output` (`toyos-display-output`) owns the protocol, in the orchestrator's approved shape: - the driver's first frame is `Hello{size}`, the output it scans out; - the compositor sends `Present{sequence, damage}`, one to eight rectangles each inside that output; - the driver answers `Presented{sequence, vblank_nanos, refresh_interval_nanos}`. One present is outstanding at a time (`Presenting`). Damage composed meanwhile is merged, past eight rectangles into their bounding box, which is the one region a PSR2 selective update takes anyway. Every frame is decoded once, and an answer to no outstanding present is the driver's error. The compositor serves `display-output` to one driver at a time. After each blit it hands the composed regions to `Presenting`, and it never waits on the driver. A refusal, a frame out of protocol, a hello for another size than the desktop's or a hello later than the handshake timeout drops the driver by name. tgl-display offers the firmware's plane (PLANE_SIZE now gives its height) and answers each present at the vblank after it arrived: - `interrupt::claim` masks and disables every display interrupt category but pipe A's, and enables display and primary interrupts. - `arm` clears pipe A's identity and enables its vblank only while a present waits. - `serve` follows Vol 12's service routine and disarms on the vblank. A pending category this driver enabled nothing in is refused by name. - The wait has a hang ceiling of FRAME_CEILING_NANOS. - One connection is served. When it ends, why is said and presents stop, since a driver the compositor dropped would be dropped again. Every compositor row serves the port, and tgl-display receives it. metalcase runs tgl-display beside hda, so its new T14 row `display_output_presents` shows a present answered, and hda playing with PG3 down, in a boot the other metalcase rows already take. Co-Authored-By: Claude Opus 5.5 --- Cargo.lock | 6 + Cargo.toml | 1 + images/system.toml | 17 +- ...ves-its-displays-from-a-userland-driver.md | 7 +- lib/display-output/Cargo.toml | 14 + lib/display-output/src/lib.rs | 412 ++++++++++++++++++ system/compositor/Cargo.toml | 1 + system/compositor/src/main.rs | 5 + system/compositor/src/output.rs | 110 +++++ system/compositor/src/session.rs | 55 ++- system/tgl-display/Cargo.toml | 3 +- system/tgl-display/driver/src/interrupt.rs | 122 ++++++ system/tgl-display/driver/src/lib.rs | 16 +- system/tgl-display/driver/src/model.rs | 31 +- system/tgl-display/driver/src/regs.rs | 26 +- system/tgl-display/driver/src/tests.rs | 2 + system/tgl-display/src/main.rs | 101 ++++- tests/images/clipboardcase.toml | 2 +- tests/images/metalcase.toml | 9 +- tests/images/panelcase.toml | 2 +- tests/images/scalecase.toml | 2 +- tests/images/sessioncase.toml | 2 +- tests/toyos.rs | 32 ++ 23 files changed, 953 insertions(+), 25 deletions(-) create mode 100644 lib/display-output/Cargo.toml create mode 100644 lib/display-output/src/lib.rs create mode 100644 system/compositor/src/output.rs create mode 100644 system/tgl-display/driver/src/interrupt.rs diff --git a/Cargo.lock b/Cargo.lock index bd08fbba552..ed7b89b5fe8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -445,6 +445,7 @@ dependencies = [ "toyos", "toyos-abi", "toyos-desktop", + "toyos-display-output", "toyos-edid", "toyos-font", "toyos-icon", @@ -3339,6 +3340,7 @@ dependencies = [ "toyos", "toyos-abi", "toyos-device-memory", + "toyos-display-output", "toyos-manifest", "toyos-pci-claim", "toyos-tgl-display", @@ -3697,6 +3699,10 @@ dependencies = [ "toyos-net-wire", ] +[[package]] +name = "toyos-display-output" +version = "0.1.0" + [[package]] name = "toyos-dma" version = "0.1.0" diff --git a/Cargo.toml b/Cargo.toml index e668174836e..1e4525cbc5c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -47,6 +47,7 @@ members = [ "lib/device-memory", "lib/devicepath", "lib/elf", + "lib/display-output", "lib/edid", "lib/elide", "lib/fat32", diff --git a/images/system.toml b/images/system.toml index 77241796c90..ac94d4c1694 100644 --- a/images/system.toml +++ b/images/system.toml @@ -92,12 +92,13 @@ receives = ["power"] # `screen`. `volume` is soundserver's master level and mute, which the volume # keys set and no other row receives. The display is a virtio-gpu function the # compositor drives, where the machine has one, and the firmware's framebuffer -# otherwise. `jobs` makes each program it starts a job of its own, with its own -# share of the CPU. `power` is its stop view's: Esc cancels the machine's stop, -# Enter forces it. +# otherwise; `display-output` is where a display driver offers the output it +# scans out and is told each frame composed into it. `jobs` makes each +# program it starts a job of its own, with its own share of the CPU. `power` is +# its stop view's: Esc cancels the machine's stop, Enter forces it. [programs.compositor] service = true -serves = ["compositor", "prompt", "clipboard", "input", "display"] +serves = ["compositor", "prompt", "clipboard", "input", "display", "display-output"] receives = ["soundserver", "filepicker", "volume", "power"] devices = ["pci:03:80:00/1af4:1050", "framebuffer"] starts = ["terminal", "files", "/apps"] @@ -134,11 +135,13 @@ syscap = ["rt"] # The Tiger Lake display driver: the T14's iGPU, held for the boot. It keeps # the firmware's mode and surface, which the compositor draws on through the -# kernel's framebuffer claim, and programs the display engine's power. Not -# started again: the claim's release resets the function, which ends the -# firmware's scan-out (`issues/each-gpu-drives-its-displays-from-a-userland-driver.md`). +# kernel's framebuffer claim, programs the display engine's power and answers +# the compositor's presents on `display-output`. Not started again: the +# claim's release resets the function, which ends the firmware's scan-out +# (`issues/each-gpu-drives-its-displays-from-a-userland-driver.md`). [programs.tgl-display] drives = ["pci:03:00:00/8086:9a49"] +receives = ["display-output"] # The network stack, over every link a NIC driver brings on `link`; it claims # no device. It serves from the start, links or none. diff --git a/issues/each-gpu-drives-its-displays-from-a-userland-driver.md b/issues/each-gpu-drives-its-displays-from-a-userland-driver.md index 5ad6d47f89a..e0fc0d588af 100644 --- a/issues/each-gpu-drives-its-displays-from-a-userland-driver.md +++ b/issues/each-gpu-drives-its-displays-from-a-userland-driver.md @@ -84,7 +84,12 @@ The kernel keeps only generic mechanisms: - DC6 is allowed only with the panel on DDI A, since Vol 12 says only port A's PHY programming survives it. - With the pipe lit, the engine enters DC5 only while PSR is active (Vol 12, "Display C5 and C6"), so this stage alone saves little. - What it can show alone is the program held in storage, PG3-PG5 off, and the scan-out and `hda` unharmed. `hda`'s display codec is in PG3. -3. **Present and damage.** The `display-output` piece PSR needs is the compositor telling the driver which frame changed, and where. Otherwise PSR1 sees a CPU write only through FBC host tracking of a graphics-aperture fence (Vol 12 "CPU Host Front Buffer Tracking", "on projects that support that"). Whether that reaches the GOP's surface is unverified. This is ordered before PSR, rather than an interim signal. Its shape goes to the orchestrator before it is built, because it crosses the compositor. +3. **Present and damage.** The `display-output` piece PSR needs is the compositor telling the driver which frame changed, and where. Without it, PSR1 sees a CPU write only through FBC host tracking of a graphics-aperture fence (Vol 12 "CPU Host Front Buffer Tracking", "on projects that support that"). It is ordered before PSR, rather than an interim signal. Shape approved by the orchestrator, 2026-11-01. + - `lib/display-output` owns the wire types and the compositor's state: `Hello{size}`, `Present{sequence, damage}` and `Presented{sequence, vblank_nanos, refresh_interval_nanos}`. + - One present is outstanding at a time. Damage composed meanwhile is merged, past 8 rectangles into their bounding box, and sent when the answer arrives. + - The compositor serves `display-output`, one driver at a time, and never waits on it. + - `tgl-display` answers each present at the vblank after it arrived. That vblank is pipe A's interrupt, enabled only while a present waits, and its wait has a hang ceiling. + - The compositor does not yet pace frames by `Presented`. It still blits the firmware's framebuffer itself until stage 5. 4. **PSR on the eDP panel**, measured together with stage 2. PSR's first-frame exit latency goes to the owner in absolute numbers before PSR lands. 5. **The rest of `display-output`.** virtio-gpu and the framebuffer become drivers. The panic surface lands here. 6. **Intel modesetting**: combo-PHY DDIs, DPLLs and CDCLK, DP link training with DSC, HDMI 2.0 with SCDC, EDID over AUX and GMBUS, several outputs. diff --git a/lib/display-output/Cargo.toml b/lib/display-output/Cargo.toml new file mode 100644 index 00000000000..b5ae24cac6c --- /dev/null +++ b/lib/display-output/Cargo.toml @@ -0,0 +1,14 @@ +[package] +name = "toyos-display-output" +description = "The compositor's display-output protocol: the output a display driver offers, the frames the compositor presents to it and their answers, decoded once." +version = "0.1.0" +edition = "2024" +license = "MIT OR Apache-2.0" +publish = false + +[lib] +doctest = false + +[lints.rust] +unsafe_code = "forbid" +warnings = "deny" diff --git a/lib/display-output/src/lib.rs b/lib/display-output/src/lib.rs new file mode 100644 index 00000000000..81b67cbbf23 --- /dev/null +++ b/lib/display-output/src/lib.rs @@ -0,0 +1,412 @@ +//! The compositor's display-output protocol: a display driver process, one +//! per GPU function, is a client of the compositor's [`PORT`]. +//! +//! **One connection per output.** The driver's first frame is [`Hello`], the +//! output it scans out. From then on the compositor sends [`Present`] when +//! the frame it composed into the output changed, naming where, and the +//! driver answers [`Presented`] once that frame reached the output's next +//! vertical blank. **One present is outstanding at a time**: damage the +//! compositor composes meanwhile is merged ([`Presenting`]) and sent when the +//! answer arrives, so the compositor never waits on a driver and a driver +//! never queues frames. +//! +//! **Every frame is decoded here, once**, and one that does not decode is the +//! peer's protocol error: the reader ends the connection by name. + +#![no_std] + +/// The port a driver connects to; the compositor serves it. +pub const PORT: &str = "display-output"; + +/// The most rectangles one present names. Damage past it is merged into its +/// bounding box, which is what selective update takes anyway (one region per +/// frame, Vol 12 "Selective Fetch"). +pub const MAX_RECTS: usize = 8; + +// Driver to compositor. +pub const MSG_HELLO: u32 = 1; +pub const MSG_PRESENTED: u32 = 2; +// Compositor to driver. +pub const MSG_PRESENT: u32 = 16; + +/// The widest frame either side sends: a present of [`MAX_RECTS`]. +pub const MAX_FRAME: usize = Present::HEAD + MAX_RECTS * Rect::BYTES; + +fn u32_at(bytes: &[u8], at: usize) -> u32 { + u32::from_le_bytes(bytes[at..at + 4].try_into().expect("four bytes")) +} + +fn u64_at(bytes: &[u8], at: usize) -> u64 { + u64::from_le_bytes(bytes[at..at + 8].try_into().expect("eight bytes")) +} + +/// An output's size in pixels, both nonzero. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub struct Size { + pub width: u32, + pub height: u32, +} + +/// A rectangle of an output's pixels, nonempty: columns `x..x + width`, rows +/// `y..y + height`. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub struct Rect { + pub x: u32, + pub y: u32, + pub width: u32, + pub height: u32, +} + +impl Rect { + pub const BYTES: usize = 16; + + fn right(&self) -> u32 { + self.x + self.width + } + + fn bottom(&self) -> u32 { + self.y + self.height + } + + /// The smallest rectangle covering both. + pub fn union(&self, other: &Self) -> Self { + let (x, y) = (self.x.min(other.x), self.y.min(other.y)); + Self { x, y, width: self.right().max(other.right()) - x, height: self.bottom().max(other.bottom()) - y } + } + + fn encode(&self, out: &mut [u8]) { + for (n, v) in [self.x, self.y, self.width, self.height].into_iter().enumerate() { + out[4 * n..4 * n + 4].copy_from_slice(&v.to_le_bytes()); + } + } + + /// One within `size`, or `None`. + fn decode(bytes: &[u8], size: Size) -> Option { + let r = Self { x: u32_at(bytes, 0), y: u32_at(bytes, 4), width: u32_at(bytes, 8), height: u32_at(bytes, 12) }; + let inside = r.width != 0 + && r.height != 0 + && r.x.checked_add(r.width).is_some_and(|right| right <= size.width) + && r.y.checked_add(r.height).is_some_and(|bottom| bottom <= size.height); + inside.then_some(r) + } +} + +/// The parts of an output a present changed: one to [`MAX_RECTS`] +/// rectangles. +#[derive(Clone, Copy, Debug)] +pub struct Damage { + rects: [Rect; MAX_RECTS], + len: usize, +} + +impl Damage { + pub fn of(rect: Rect) -> Self { + Self { rects: [rect; MAX_RECTS], len: 1 } + } + + pub fn rects(&self) -> &[Rect] { + &self.rects[..self.len] + } + + /// The smallest rectangle covering every one. + pub fn bounds(&self) -> Rect { + self.rects().iter().fold(self.rects[0], |all, r| all.union(r)) + } + + /// `rect` added: past [`MAX_RECTS`], every rectangle is merged into their + /// bounding box. + pub fn add(&mut self, rect: Rect) { + if self.len == MAX_RECTS { + *self = Self::of(self.bounds().union(&rect)); + } else { + self.rects[self.len] = rect; + self.len += 1; + } + } +} + +impl PartialEq for Damage { + fn eq(&self, other: &Self) -> bool { + self.rects() == other.rects() + } +} + +impl Eq for Damage {} + +/// A driver's first frame: the output it scans out. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub struct Hello { + pub size: Size, +} + +impl Hello { + pub const BYTES: usize = 8; + + pub fn encode(&self) -> [u8; Self::BYTES] { + let mut out = [0u8; Self::BYTES]; + out[0..4].copy_from_slice(&self.size.width.to_le_bytes()); + out[4..8].copy_from_slice(&self.size.height.to_le_bytes()); + out + } + + fn decode(bytes: &[u8]) -> Option { + let bytes: &[u8; Self::BYTES] = bytes.try_into().ok()?; + let size = Size { width: u32_at(bytes, 0), height: u32_at(bytes, 4) }; + (size.width != 0 && size.height != 0).then_some(Self { size }) + } +} + +/// The frame numbered `sequence` is composed into the output; `damage` is +/// what changed since the one before. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub struct Present { + pub sequence: u64, + pub damage: Damage, +} + +impl Present { + const HEAD: usize = 8; + + /// The frame's bytes in `out`, and how many there are. + pub fn encode(&self, out: &mut [u8; MAX_FRAME]) -> usize { + out[0..8].copy_from_slice(&self.sequence.to_le_bytes()); + for (n, r) in self.damage.rects().iter().enumerate() { + r.encode(&mut out[Self::HEAD + n * Rect::BYTES..]); + } + Self::HEAD + self.damage.len * Rect::BYTES + } + + /// A present to an output of `size`: every rectangle inside it. + pub fn decode(msg_type: u32, payload: &[u8], size: Size) -> Option { + let rects = payload.get(Self::HEAD..).filter(|_| msg_type == MSG_PRESENT)?; + let (rects, rest) = rects.as_chunks::<{ Rect::BYTES }>(); + if !rest.is_empty() || !(1..=MAX_RECTS).contains(&rects.len()) { + return None; + } + let mut damage = Damage::of(Rect::decode(&rects[0], size)?); + for r in &rects[1..] { + damage.add(Rect::decode(r, size)?); + } + Some(Self { sequence: u64_at(payload, 0), damage }) + } +} + +/// The present numbered `sequence` reached the output in the vertical blank +/// at `vblank_nanos` (since boot); the output refreshes every +/// `refresh_interval_nanos`. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub struct Presented { + pub sequence: u64, + pub vblank_nanos: u64, + pub refresh_interval_nanos: u64, +} + +impl Presented { + pub const BYTES: usize = 24; + + pub fn encode(&self) -> [u8; Self::BYTES] { + let mut out = [0u8; Self::BYTES]; + out[0..8].copy_from_slice(&self.sequence.to_le_bytes()); + out[8..16].copy_from_slice(&self.vblank_nanos.to_le_bytes()); + out[16..24].copy_from_slice(&self.refresh_interval_nanos.to_le_bytes()); + out + } + + fn decode(bytes: &[u8]) -> Option { + let bytes: &[u8; Self::BYTES] = bytes.try_into().ok()?; + let p = Self { sequence: u64_at(bytes, 0), vblank_nanos: u64_at(bytes, 8), refresh_interval_nanos: u64_at(bytes, 16) }; + (p.refresh_interval_nanos != 0).then_some(p) + } +} + +/// What a driver sends. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub enum FromDriver { + /// The connection's first frame. + Hello(Hello), + Presented(Presented), +} + +impl FromDriver { + /// The frame `msg_type` carrying `payload`, or `None` for one this + /// protocol does not have. + pub fn decode(msg_type: u32, payload: &[u8]) -> Option { + match msg_type { + MSG_HELLO => Hello::decode(payload).map(Self::Hello), + MSG_PRESENTED => Presented::decode(payload).map(Self::Presented), + _ => None, + } + } +} + +/// A [`Presented`] that answers no outstanding present: the driver's protocol +/// error. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub struct Unasked { + pub sequence: u64, + pub outstanding: Option, +} + +/// The compositor's side of one output: the one present outstanding, and the +/// damage owed the next. +#[derive(Debug)] +pub struct Presenting { + next: u64, + outstanding: Option, + owed: Option, +} + +impl Default for Presenting { + fn default() -> Self { + Self::new() + } +} + +impl Presenting { + pub fn new() -> Self { + Self { next: 1, outstanding: None, owed: None } + } + + /// `rects` of the output changed: the present to send now, or `None` + /// while one is outstanding, the damage merged into the next. + pub fn damaged(&mut self, rects: impl IntoIterator) -> Option { + for rect in rects { + match &mut self.owed { + Some(damage) => damage.add(rect), + None => self.owed = Some(Damage::of(rect)), + } + } + self.send() + } + + /// The driver answered `presented`: the present owed since, if any. + pub fn presented(&mut self, presented: &Presented) -> Result, Unasked> { + if self.outstanding != Some(presented.sequence) { + return Err(Unasked { sequence: presented.sequence, outstanding: self.outstanding }); + } + self.outstanding = None; + Ok(self.send()) + } + + fn send(&mut self) -> Option { + if self.outstanding.is_some() { + return None; + } + let damage = self.owed.take()?; + let sequence = self.next; + self.next += 1; + self.outstanding = Some(sequence); + Some(Present { sequence, damage }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + const HD: Size = Size { width: 1920, height: 1080 }; + + fn rect(x: u32, y: u32, width: u32, height: u32) -> Rect { + Rect { x, y, width, height } + } + + fn encoded(present: &Present) -> ([u8; MAX_FRAME], usize) { + let mut out = [0u8; MAX_FRAME]; + let n = present.encode(&mut out); + (out, n) + } + + #[test] + fn every_frame_decodes_back_to_what_was_encoded() { + let hello = Hello { size: HD }; + assert_eq!(FromDriver::decode(MSG_HELLO, &hello.encode()), Some(FromDriver::Hello(hello))); + let presented = Presented { sequence: 7, vblank_nanos: 1_000, refresh_interval_nanos: 16_666_667 }; + assert_eq!(FromDriver::decode(MSG_PRESENTED, &presented.encode()), Some(FromDriver::Presented(presented))); + let mut damage = Damage::of(rect(0, 0, 1920, 1080)); + for n in 1..MAX_RECTS as u32 { + damage.add(rect(n, n, 1, 1)); + } + for present in [Present { sequence: 1, damage: Damage::of(rect(10, 20, 30, 40)) }, Present { sequence: u64::MAX, damage }] { + let (bytes, n) = encoded(&present); + assert_eq!(Present::decode(MSG_PRESENT, &bytes[..n], HD), Some(present)); + } + } + + /// A rectangle a present names is inside the output the driver said, and + /// not empty; a present names one to `MAX_RECTS` of them, whole. + #[test] + fn a_present_outside_its_output_or_of_no_whole_rectangles_is_refused() { + let refused = |r: Rect| { + let (bytes, n) = encoded(&Present { sequence: 1, damage: Damage::of(r) }); + assert_eq!(Present::decode(MSG_PRESENT, &bytes[..n], HD), None, "{r:?}"); + }; + refused(rect(0, 0, 0, 1)); + refused(rect(0, 0, 1, 0)); + refused(rect(1, 0, 1920, 1)); + refused(rect(0, 1080, 1, 1)); + refused(rect(u32::MAX, 0, 2, 1)); + refused(rect(0, u32::MAX, 1, 2)); + let edge = rect(1919, 1079, 1, 1); + let (bytes, n) = encoded(&Present { sequence: 1, damage: Damage::of(edge) }); + assert!(Present::decode(MSG_PRESENT, &bytes[..n], HD).is_some()); + assert_eq!(Present::decode(MSG_PRESENT, &bytes[..n - 1], HD), None, "a partial rectangle"); + assert_eq!(Present::decode(MSG_PRESENT, &bytes[..8], HD), None, "no rectangle"); + assert_eq!(Present::decode(MSG_PRESENT, &bytes[..4], HD), None, "no sequence"); + assert_eq!(Present::decode(MSG_HELLO, &bytes[..n], HD), None, "another frame's type"); + let mut wide = [0u8; MAX_FRAME + Rect::BYTES]; + for n in 0..=MAX_RECTS { + edge.encode(&mut wide[Present::HEAD + n * Rect::BYTES..]); + } + assert_eq!(Present::decode(MSG_PRESENT, &wide, HD), None, "more than MAX_RECTS"); + } + + #[test] + fn a_frame_no_driver_sends_is_refused() { + assert_eq!(FromDriver::decode(MSG_HELLO, &Hello { size: Size { width: 0, height: 1080 } }.encode()), None); + assert_eq!(FromDriver::decode(MSG_HELLO, &Hello { size: Size { width: 1920, height: 0 } }.encode()), None); + assert_eq!(FromDriver::decode(MSG_HELLO, &Hello { size: HD }.encode()[..7]), None); + let presented = Presented { sequence: 1, vblank_nanos: 5, refresh_interval_nanos: 0 }; + assert_eq!(FromDriver::decode(MSG_PRESENTED, &presented.encode()), None, "no refresh interval"); + for msg_type in [0, MSG_PRESENT, u32::MAX] { + assert_eq!(FromDriver::decode(msg_type, &Hello { size: HD }.encode()), None); + } + } + + #[test] + fn damage_past_the_most_rectangles_merges_into_their_bounds() { + let mut damage = Damage::of(rect(0, 0, 1, 1)); + for n in 1..MAX_RECTS as u32 { + damage.add(rect(10 * n, 0, 1, 1)); + } + assert_eq!(damage.rects().len(), MAX_RECTS); + damage.add(rect(5, 500, 2, 2)); + assert_eq!(damage.rects(), &[rect(0, 0, 71, 502)]); + assert_eq!(damage.bounds(), rect(0, 0, 71, 502)); + } + + #[test] + fn one_present_is_outstanding_and_the_damage_meanwhile_is_the_next() { + let mut presenting = Presenting::new(); + let first = presenting.damaged([rect(0, 0, 10, 10)]).expect("nothing is outstanding"); + assert_eq!((first.sequence, first.damage.rects()), (1, &[rect(0, 0, 10, 10)][..])); + assert_eq!(presenting.damaged([rect(100, 0, 5, 5)]), None); + assert_eq!(presenting.damaged([rect(0, 100, 5, 5)]), None); + let answer = |sequence| Presented { sequence, vblank_nanos: 1, refresh_interval_nanos: 16_666_667 }; + let second = presenting.presented(&answer(1)).expect("1 is outstanding").expect("damage is owed"); + assert_eq!((second.sequence, second.damage.rects()), (2, &[rect(100, 0, 5, 5), rect(0, 100, 5, 5)][..])); + assert_eq!(presenting.presented(&answer(2)), Ok(None), "nothing is owed"); + assert_eq!(presenting.damaged([]), None, "no damage is no present"); + assert_eq!(presenting.damaged([rect(1, 1, 1, 1)]).map(|p| p.sequence), Some(3)); + } + + #[test] + fn an_answer_to_no_outstanding_present_is_the_drivers_error() { + let mut presenting = Presenting::new(); + let answer = |sequence| Presented { sequence, vblank_nanos: 1, refresh_interval_nanos: 16_666_667 }; + assert_eq!(presenting.presented(&answer(1)), Err(Unasked { sequence: 1, outstanding: None })); + presenting.damaged([rect(0, 0, 1, 1)]); + assert_eq!(presenting.presented(&answer(2)), Err(Unasked { sequence: 2, outstanding: Some(1) })); + assert_eq!(presenting.presented(&answer(1)), Ok(None)); + assert_eq!(presenting.presented(&answer(1)), Err(Unasked { sequence: 1, outstanding: None }), "answered twice"); + } +} diff --git a/system/compositor/Cargo.toml b/system/compositor/Cargo.toml index c4bcfe4aaa4..182c250db55 100644 --- a/system/compositor/Cargo.toml +++ b/system/compositor/Cargo.toml @@ -9,6 +9,7 @@ license = "MIT OR Apache-2.0" toyos = { path = "../../sdk/toyos", features = ["std"] } toyos-abi = { path = "../../sdk/abi" } toyos-desktop = { path = "desktop" } +toyos-display-output = { path = "../../lib/display-output" } toyos-edid = { path = "../../lib/edid" } toyos-input = { path = "../../lib/input" } toyos-inspect = { path = "../../lib/inspect" } diff --git a/system/compositor/src/main.rs b/system/compositor/src/main.rs index ac1dbf4ffe3..f7e947a25cc 100644 --- a/system/compositor/src/main.rs +++ b/system/compositor/src/main.rs @@ -18,6 +18,7 @@ mod client; mod display; +mod output; mod render; mod session; mod signout; @@ -64,6 +65,7 @@ pub const MAX_WINDOW_SLOTS: u32 = Poller::MAX_HANDLES - PROMPT_POLL_HANDLES - CLIPBOARD_POLL_HANDLES - DISPLAY_PORT_POLL_HANDLES + - DISPLAY_OUTPUT_POLL_HANDLES - INPUT_POLL_HANDLES - VOLUME_POLL_HANDLES; @@ -80,6 +82,9 @@ pub const CLIPBOARD_POLL_HANDLES: u32 = 2; /// first frame (`toyos_desktop::pending_admits`). pub const DISPLAY_PORT_POLL_HANDLES: u32 = 2; +/// The `display-output` port's own: its acceptor, and its one driver. +pub const DISPLAY_OUTPUT_POLL_HANDLES: u32 = 2; + /// soundserver's `volume` port: the one connection the volume keys' requests go on. pub const VOLUME_POLL_HANDLES: u32 = 1; diff --git a/system/compositor/src/output.rs b/system/compositor/src/output.rs new file mode 100644 index 00000000000..61e1849565f --- /dev/null +++ b/system/compositor/src/output.rs @@ -0,0 +1,110 @@ +//! The display driver on `display-output` (`toyos_display_output`), one at a +//! time: told every frame composed into the output it scans out, and where. +//! +//! The compositor never waits on it. Each present is one `try_send`. A +//! refusal, a frame out of protocol or a hello that does not come within +//! [`HANDSHAKE_TIMEOUT`] drops the driver by name, and the desktop goes on +//! without it. + +use std::time::Instant; + +use toyos::ipc::{FrameRx, RxStep}; +use toyos::Connection; +use toyos_desktop::{Mode, Rect}; +use toyos_display_output::{FromDriver, Present, Presenting, Size, MAX_FRAME, MSG_PRESENT}; + +use crate::client::HANDSHAKE_TIMEOUT; + +/// The most frames one turn reads off the driver; it sends one per present. +const TURN_FRAMES: usize = 4; + +pub struct Driver { + conn: Connection, + rx: FrameRx, + since: Instant, + serving: Option, +} + +struct Serving { + size: Size, + presenting: Presenting, + answered: u64, +} + +impl Driver { + pub fn new(conn: Connection) -> Self { + Self { conn, rx: FrameRx::new(), since: Instant::now(), serving: None } + } + + pub fn conn(&self) -> &Connection { + &self.conn + } + + /// Whether its hello is overdue. + pub fn overdue(&self, now: Instant) -> bool { + self.serving.is_none() && now.duration_since(self.since) >= HANDSHAKE_TIMEOUT + } + + /// Whatever the driver sent, each frame whole; `Err` names why it is + /// dropped. Its hello must offer the output the desktop is drawn on, + /// `mode`. + pub fn read(&mut self, mode: Mode) -> Result<(), String> { + for _ in 0..TURN_FRAMES { + let (msg_type, payload_len) = match self.rx.pump(&self.conn) { + RxStep::Idle => return Ok(()), + RxStep::Eof => return Err("its connection ended".into()), + RxStep::Malformed => return Err("a frame no protocol here can make".into()), + RxStep::Frame { msg_type, payload_len, .. } => (msg_type, payload_len), + }; + match (FromDriver::decode(msg_type, self.rx.payload(payload_len)), &mut self.serving) { + (Some(FromDriver::Hello(hello)), None) => { + if (hello.size.width, hello.size.height) != (mode.width, mode.height) { + return Err(format!("it offers {}x{}, and the desktop is drawn at {mode}", hello.size.width, hello.size.height)); + } + println!("compositor: a display driver scans out the {mode} output"); + self.serving = Some(Serving { size: hello.size, presenting: Presenting::new(), answered: 0 }); + } + (Some(FromDriver::Presented(presented)), Some(serving)) => { + let next = serving.presenting.presented(&presented).map_err(|unasked| format!("{unasked:?}"))?; + serving.answered += 1; + if serving.answered == 1 { + println!( + "compositor: the display driver answered its first present, refreshing every {} ns", + presented.refresh_interval_nanos + ); + } + if let Some(present) = next { + send(&self.conn, &present)?; + } + } + _ => return Err(format!("frame {msg_type}, which its state has not")), + } + } + Ok(()) + } + + /// `regions` of the output were composed: presented now, or merged into + /// the present after the one outstanding. + pub fn composed(&mut self, regions: &[Rect]) -> Result<(), String> { + let Some(serving) = &mut self.serving else { return Ok(()) }; + let size = serving.size; + let rects = regions.iter().filter_map(|r| { + let r = r.intersect(Rect::new(0, 0, size.width as i32, size.height as i32)); + if r.is_empty() { + return None; + } + let at = |v: i32| u32::try_from(v).expect("a rectangle clipped to the output is inside it"); + Some(toyos_display_output::Rect { x: at(r.x0), y: at(r.y0), width: at(r.w()), height: at(r.h()) }) + }); + match serving.presenting.damaged(rects) { + Some(present) => send(&self.conn, &present), + None => Ok(()), + } + } +} + +fn send(conn: &Connection, present: &Present) -> Result<(), String> { + let mut frame = [0u8; MAX_FRAME]; + let n = present.encode(&mut frame); + conn.try_send_bytes(MSG_PRESENT, &frame[..n]).map_err(|why| format!("present {} refused ({why:?})", present.sequence)) +} diff --git a/system/compositor/src/session.rs b/system/compositor/src/session.rs index 9924ecb1b4b..567979b02f4 100644 --- a/system/compositor/src/session.rs +++ b/system/compositor/src/session.rs @@ -44,7 +44,7 @@ use crate::display::Display; use crate::render::{self, Assets, SystemStats, TitleBarIcons}; use crate::stats::FrameTotals; use crate::{ - launcher_apps, CLIPBOARD_POLL_HANDLES, DISPLAY_PORT_POLL_HANDLES, DOUBLE_CLICK_TIME, DRAIN_BUDGET, + launcher_apps, CLIPBOARD_POLL_HANDLES, DISPLAY_OUTPUT_POLL_HANDLES, DISPLAY_PORT_POLL_HANDLES, DOUBLE_CLICK_TIME, DRAIN_BUDGET, FIXED_POLL_HANDLES, FRAME_INTERVAL, INPUT_POLL_HANDLES, MAX_WINDOW_SLOTS, PROMPT_POLL_HANDLES, VOLUME_POLL_HANDLES, }; @@ -236,6 +236,9 @@ pub struct Session { /// The display's settings' port ([`toyos_manifest::display::PORT`]), which /// one program alone holds. settings: Acceptor, + /// The `display-output` port, and the display driver on it. + outputs: Acceptor, + output: Option, watcher: Option, /// What the watcher has asked and is sent, for the compositor's whole life: /// a copy made while no watcher is there is the next one's. @@ -320,6 +323,8 @@ impl Session { .expect("the manifest declares this program serves `display`"); let inputs = endow::acceptor(wire::PORT) .expect("the manifest declares this program serves `input`"); + let outputs = endow::acceptor(toyos_display_output::PORT) + .expect("the manifest declares this program serves `display-output`"); let (mut display, panel) = Display::claim(); let desktop = crate::signout::Desktop::start(); let mode = display.mode(); @@ -370,6 +375,7 @@ impl Session { + PROMPT_POLL_HANDLES + CLIPBOARD_POLL_HANDLES + DISPLAY_PORT_POLL_HANDLES + + DISPLAY_OUTPUT_POLL_HANDLES + INPUT_POLL_HANDLES + VOLUME_POLL_HANDLES, ); @@ -378,6 +384,7 @@ impl Session { poller.watch(&prompts, READABLE, prompts.as_handle().raw().0 as u64); poller.watch(&clipboards, READABLE, clipboards.as_handle().raw().0 as u64); poller.watch(&settings, READABLE, settings.as_handle().raw().0 as u64); + poller.watch(&outputs, READABLE, outputs.as_handle().raw().0 as u64); if let Some(handle) = display.handle() { poller.watch(&handle, READABLE, handle.raw().0 as u64); } @@ -405,6 +412,8 @@ impl Session { owed: PromptGate::default(), clipboards, settings, + outputs, + output: None, watcher: None, watch: Watch::default(), inputs, @@ -498,6 +507,8 @@ impl Session { let prompt_ready = self.is_ready(self.prompts.as_handle().raw()); let clipboard_ready = self.is_ready(self.clipboards.as_handle().raw()); let settings_ready = self.is_ready(self.settings.as_handle().raw()); + let outputs_ready = self.is_ready(self.outputs.as_handle().raw()); + let output_ready = self.output.as_ref().is_some_and(|d| self.is_ready(d.conn().as_handle().raw())); let volume_ready = self.volume.as_ref().is_some_and(|(conn, _)| self.is_ready(conn.as_handle().raw())); let display_ready = self.display.handle().is_some_and(|handle| self.is_ready(handle.raw())); let session_ready = self.desktop.conn().is_some_and(|c| self.is_ready(c.as_handle().raw())); @@ -519,6 +530,9 @@ impl Session { eprintln!("compositor: dropping client {} — {}", p.conn.as_handle().raw().0, reason); } self.pending.retain(|p| now.duration_since(p.since) < HANDSHAKE_TIMEOUT); + if self.output.as_ref().is_some_and(|d| d.overdue(now)) { + self.drop_output("its hello did not come in time".into()); + } if !inputs_ready && sources_ready.is_empty() @@ -526,6 +540,8 @@ impl Session { && !prompt_ready && !clipboard_ready && !settings_ready + && !outputs_ready + && !output_ready && !client_ready && !volume_ready && !display_ready @@ -557,6 +573,20 @@ impl Session { if settings_ready { self.accept(Port::Display); } + if outputs_ready { + self.accept_output(); + } + if output_ready { + let mode = self.display.mode(); + let read = self.output.as_mut().expect("a ready driver is held").read(mode); + match read { + Ok(()) => { + let conn = self.output.as_ref().expect("a ready driver is held").conn(); + self.poller.watch(conn, READABLE, conn.as_handle().raw().0 as u64); + } + Err(why) => self.drop_output(why), + } + } if volume_ready { self.volume_answers(); } @@ -577,9 +607,29 @@ impl Session { self.reap(); self.prompt_gone(); self.rearm(inputs_ready, accept_ready, prompt_ready, clipboard_ready, settings_ready); + if outputs_ready { + self.poller.watch(&self.outputs, READABLE, self.outputs.as_handle().raw().0 as u64); + } true } + /// A display driver off `display-output`, where none is connected. + fn accept_output(&mut self) { + match self.outputs.accept() { + Err(e) => eprintln!("compositor: a display driver could not be accepted ({e:?})"), + Ok(_) if self.output.is_some() => eprintln!("compositor: refusing a display driver — one is connected"), + Ok(conn) => { + self.poller.watch(&conn, READABLE, conn.as_handle().raw().0 as u64); + self.output = Some(crate::output::Driver::new(conn)); + } + } + } + + fn drop_output(&mut self, why: String) { + eprintln!("compositor: dropping the display driver — {why}"); + self.output = None; + } + /// Nanoseconds since the session's epoch, the clock the overlay is timed by. fn overlay_now(&self) -> u64 { self.epoch.elapsed().as_nanos() as u64 @@ -1948,6 +1998,9 @@ impl Session { let damage_px = blit(self.display.screen(), &self.back, ®ions); self.totals.record(started.elapsed().as_nanos() as u64, regions.len(), damage_px); self.display.present(®ions); + if let Some(Err(why)) = self.output.as_mut().map(|d| d.composed(®ions)) { + self.drop_output(why); + } if self.totals.frames() == 1 { println!("compositor: the first frame is on the panel"); } diff --git a/system/tgl-display/Cargo.toml b/system/tgl-display/Cargo.toml index 6b2010ff09b..aefa4553376 100644 --- a/system/tgl-display/Cargo.toml +++ b/system/tgl-display/Cargo.toml @@ -1,12 +1,13 @@ [package] name = "tgl-display" -description = "The Tiger Lake display driver: one process for the iGPU's display function, holding its claim and programming the display engine's power on the firmware's surface." +description = "The Tiger Lake display driver: one process for the iGPU's display function, holding its claim, programming the display engine's power on the firmware's surface and answering the compositor's presents." edition = "2024" license = "MIT OR Apache-2.0" [dependencies] toyos-abi = { path = "../../sdk/abi" } toyos = { path = "../../sdk/toyos" } +toyos-display-output = { path = "../../lib/display-output" } toyos-device-memory = { path = "../../lib/device-memory" } toyos-manifest = { path = "../../lib/manifest" } toyos-pci-claim = { path = "../../lib/pci-claim" } diff --git a/system/tgl-display/driver/src/interrupt.rs b/system/tgl-display/driver/src/interrupt.rs new file mode 100644 index 00000000000..4b2a1911094 --- /dev/null +++ b/system/tgl-display/driver/src/interrupt.rs @@ -0,0 +1,122 @@ +//! Pipe A's vertical blank as the claim's interrupt, by Vol 12 "Display +//! Interrupts": the one display interrupt this driver enables, and only while +//! it waits for one. + +use toyos_device_memory::Registers; + +use crate::{regs, Refusal}; + +/// Every display interrupt but pipe A's vblank masked and disabled, pipe A's +/// identity cleared and display and primary interrupts enabled. Nothing is +/// raised until [`arm`]. +pub fn claim(regs: &R) { + for ier in regs::UNUSED_IERS { + regs.write32(ier, 0); + regs.write32(ier - 8, !0); + } + regs.write32(regs::PIPE_A_IER, 0); + regs.write32(regs::PIPE_A_IMR, !regs::VBLANK); + clear(regs); + regs.write32(regs::DISPLAY_INT_CTL, regs::DISPLAY_INTERRUPT_ENABLE); + regs.write32(regs::GFX_MSTR_INTR, regs::PRIMARY_INTERRUPT); +} + +/// Raise the next vblank's start: the identity a vblank before this call set +/// is cleared first, so the one raised began after it. +pub fn arm(regs: &R) { + clear(regs); + regs.write32(regs::PIPE_A_IER, regs::VBLANK); +} + +/// The claim's interrupt served (Vol 12 "Interrupt Service Routine"): +/// whether it was the vblank [`arm`] asked for, which disarms it. +/// +/// Primary and display interrupts are disabled while the sources are cleared +/// and enabled after, so a source still pending raises a new message. A +/// category this driver enabled nothing in is refused, and display interrupts +/// stay disabled. +pub fn serve(regs: &R) -> Result { + if regs.read32(regs::GFX_MSTR_INTR) & regs::DISPLAY_PENDING == 0 { + return Ok(false); + } + regs.write32(regs::GFX_MSTR_INTR, 0); + regs.write32(regs::DISPLAY_INT_CTL, 0); + let pending = regs.read32(regs::DISPLAY_INT_CTL) & regs::PENDING; + if pending & !regs::PIPE_A_PENDING != 0 { + return Err(Refusal::Interrupts(pending)); + } + let vblank = clear(regs) & regs::VBLANK != 0; + if vblank { + regs.write32(regs::PIPE_A_IER, 0); + } + regs.write32(regs::DISPLAY_INT_CTL, regs::DISPLAY_INTERRUPT_ENABLE); + regs.write32(regs::GFX_MSTR_INTR, regs::PRIMARY_INTERRUPT); + Ok(vblank) +} + +/// Pipe A's identity bits cleared, both events it can hold ("The IIR can +/// queue up to two interrupt events"); what was set. +fn clear(regs: &R) -> u32 { + let mut seen = 0; + for _ in 0..2 { + let iir = regs.read32(regs::PIPE_A_IIR); + if iir == 0 { + break; + } + seen |= iir; + regs.write32(regs::PIPE_A_IIR, iir); + } + seen +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::model::Engine; + + fn raised(engine: &Engine) -> bool { + engine.read32(regs::GFX_MSTR_INTR) & regs::DISPLAY_PENDING != 0 + } + + #[test] + fn claimed_interrupts_raise_nothing_until_armed() { + let engine = Engine::t14(); + for ier in regs::UNUSED_IERS { + engine.set(ier, !0); + } + claim(&engine); + assert!(regs::UNUSED_IERS.iter().all(|&ier| engine.get(ier) == 0 && engine.get(ier - 8) == !0)); + assert_eq!(engine.get(regs::GFX_MSTR_INTR), regs::PRIMARY_INTERRUPT); + assert_eq!(engine.get(regs::DISPLAY_INT_CTL), regs::DISPLAY_INTERRUPT_ENABLE); + engine.vblank(); + assert!(!raised(&engine)); + assert_eq!(serve(&engine), Ok(false)); + } + + /// A vblank that began before the present arrived is not its vblank, and + /// the one after it is raised once. + #[test] + fn an_armed_vblank_is_the_next_one_and_raises_once() { + let engine = Engine::t14(); + claim(&engine); + engine.vblank(); + arm(&engine); + assert!(!raised(&engine), "the vblank before the arm"); + engine.vblank(); + assert!(raised(&engine)); + assert_eq!(serve(&engine), Ok(true)); + assert_eq!(engine.get(regs::GFX_MSTR_INTR), regs::PRIMARY_INTERRUPT, "re-enabled after the clear"); + engine.vblank(); + assert!(!raised(&engine), "disarmed by the vblank it served"); + } + + #[test] + fn a_category_no_one_enabled_is_refused_and_display_interrupts_stay_off() { + let engine = Engine::t14(); + claim(&engine); + engine.other_pending.set(1 << 23); + assert_eq!(serve(&engine), Err(Refusal::Interrupts(1 << 23))); + assert_eq!(engine.get(regs::DISPLAY_INT_CTL) & regs::DISPLAY_INTERRUPT_ENABLE, 0); + assert_eq!(engine.get(regs::GFX_MSTR_INTR), 0); + } +} diff --git a/system/tgl-display/driver/src/lib.rs b/system/tgl-display/driver/src/lib.rs index f8233b2daef..fb65ff944de 100644 --- a/system/tgl-display/driver/src/lib.rs +++ b/system/tgl-display/driver/src/lib.rs @@ -22,6 +22,7 @@ use core::fmt; use toyos_device_memory::Registers; pub mod dmc; +pub mod interrupt; pub mod regs; pub mod watermark; @@ -44,7 +45,7 @@ pub trait Clock { const MEASURED_FRAMES: u32 = 30; /// The longest one frame may take before the pipe is called stopped: two /// frames at 24 Hz. A hang's ceiling. -const FRAME_CEILING_NANOS: u64 = 84_000_000; +pub const FRAME_CEILING_NANOS: u64 = 84_000_000; /// Between two reads of the frame counter. const FRAME_POLL_NANOS: u64 = 50_000; /// Vol 12 "Retrieve Memory Latency Data" and "SAGV Block Time": "Timeout after @@ -101,6 +102,9 @@ pub enum Refusal { /// A power well stayed on after every request this driver can see was /// withdrawn. PowerWellHeld(u32), + /// DISPLAY_INT_CTL named a pending category this driver enabled nothing + /// in. + Interrupts(u32), } impl fmt::Display for Refusal { @@ -129,6 +133,7 @@ impl fmt::Display for Refusal { write!(f, "the CSR storage at {at:#x} reads {read:#010x}, not the {wrote:#010x} written") } Self::PowerWellHeld(well) => write!(f, "power well {well} stayed on with its requests withdrawn"), + Self::Interrupts(pending) => write!(f, "DISPLAY_INT_CTL names pending categories {pending:#010x} beyond pipe A's"), } } } @@ -139,6 +144,7 @@ pub struct Scanout { /// Which of pipe A's planes, from 1. pub plane: usize, pub width: u64, + pub height: u64, pub bytes_per_pixel: u64, pub tiling: Tiling, /// Blocks, from PLANE_BUF_CFG, whose Buffer End is inclusive: Vol 12 @@ -190,6 +196,7 @@ pub fn scanout(regs: &R) -> Result { Ok(Scanout { plane, width: u64::from(size & 0x1FFF) + 1, + height: u64::from((size >> 16) & 0x1FFF) + 1, bytes_per_pixel, tiling, allocation: (end + 1).saturating_sub(start), @@ -274,6 +281,13 @@ pub struct Programmed { pub before: [u32; regs::LEVELS], } +impl Programmed { + /// One frame of the measured mode. + pub fn refresh_interval_nanos(&self) -> u64 { + self.scanout.htotal * self.scanout.vtotal * 1_000_000 / self.pixel_rate_khz + } +} + /// Watermark levels 0-7 for the firmware's plane, programmed and armed (Vol 12 /// "Watermark Algorithm" step 5). /// diff --git a/system/tgl-display/driver/src/model.rs b/system/tgl-display/driver/src/model.rs index 0bf4ccd6ce9..c5cea10cbbb 100644 --- a/system/tgl-display/driver/src/model.rs +++ b/system/tgl-display/driver/src/model.rs @@ -48,6 +48,8 @@ pub struct Engine { pub preempted: Cell, stale: Cell>, jump: Cell, + /// DISPLAY_INT_CTL categories pending beyond pipe A's. + pub other_pending: Cell, } impl Engine { @@ -94,6 +96,7 @@ impl Engine { preempted: Cell::new(false), stale: Cell::new(None), jump: Cell::new(0), + other_pending: Cell::new(0), } } @@ -106,7 +109,7 @@ impl Engine { } pub fn written(&self) -> Vec<(usize, u32)> { - self.writes.borrow().iter().copied().filter(|(at, _)| !(regs::MAILBOX_INTERFACE..regs::SPAN).contains(at)).collect() + self.writes.borrow().iter().copied().filter(|(at, _)| !(regs::MAILBOX_INTERFACE..=regs::MAILBOX_DATA1).contains(at)).collect() } fn command(&self, interface: u32) { @@ -123,6 +126,21 @@ impl Engine { } } +impl Engine { + /// A vertical blank begins: pipe A's identity bit is set where its mask + /// lets it through. + pub fn vblank(&self) { + if self.get(regs::PIPE_A_IMR) & regs::VBLANK == 0 { + self.set(regs::PIPE_A_IIR, self.get(regs::PIPE_A_IIR) | regs::VBLANK); + } + } + + fn display_pending(&self) -> u32 { + let pipe_a = self.get(regs::PIPE_A_IIR) & self.get(regs::PIPE_A_IER) != 0; + self.other_pending.get() | if pipe_a { regs::PIPE_A_PENDING } else { 0 } + } +} + impl Engine { /// A request written: every instance shows the state of the ORed requests, /// and a well a hidden requester holds stays on. @@ -158,6 +176,13 @@ impl Registers for Engine { return stale; } } + if at == regs::DISPLAY_INT_CTL { + return self.get(at) & regs::DISPLAY_INTERRUPT_ENABLE | self.display_pending(); + } + if at == regs::GFX_MSTR_INTR { + let display = self.get(regs::DISPLAY_INT_CTL) & regs::DISPLAY_INTERRUPT_ENABLE != 0 && self.display_pending() != 0; + return self.get(at) & regs::PRIMARY_INTERRUPT | if display { regs::DISPLAY_PENDING } else { 0 }; + } if at == regs::PIPE_FRMCNT_A { return self.frame_nanos.map_or(0x276, |period| (self.now.get() / period) as u32); } @@ -190,6 +215,10 @@ impl Registers for Engine { self.request(at, value); return; } + if at == regs::PIPE_A_IIR { + self.set(at, self.get(at) & !value); + return; + } if self.storage_drops.get() && crate::dmc::WINDOW.contains(&at) { return; } diff --git a/system/tgl-display/driver/src/regs.rs b/system/tgl-display/driver/src/regs.rs index a0eb91050ea..af555a90257 100644 --- a/system/tgl-display/driver/src/regs.rs +++ b/system/tgl-display/driver/src/regs.rs @@ -119,5 +119,29 @@ pub const MAILBOX_DATA0: usize = 0x13_8128; pub const MAILBOX_DATA1: usize = 0x13_812C; pub const RUN_BUSY: u32 = 1 << 31; +/// DE_PIPE_INTERRUPT_A (Vol 2c), 44400h-4440Fh: ISR, IMR, IIR and IER, bit 0 +/// Vblank. The other categories this driver enables nothing in, at their IER: +/// pipes B-D, DE_PORT_INTERRUPT, DE_MISC_INTERRUPT, DE_HPD_INTERRUPT and +/// AUD_INTERRUPT. Each IMR is 4 below its IER. +pub const PIPE_A_IMR: usize = 0x4_4404; +pub const PIPE_A_IIR: usize = 0x4_4408; +pub const PIPE_A_IER: usize = 0x4_440C; +pub const VBLANK: u32 = 1 << 0; +pub const UNUSED_IERS: [usize; 7] = [0x4_441C, 0x4_442C, 0x4_443C, 0x4_444C, 0x4_446C, 0x4_447C, 0x4_448C]; + +/// DISPLAY_INT_CTL, 44200h: bit 31 Display Interrupt Enable, 25:16 the +/// categories pending, pipe A's at 16. +pub const DISPLAY_INT_CTL: usize = 0x4_4200; +pub const DISPLAY_INTERRUPT_ENABLE: u32 = 1 << 31; +pub const PENDING: u32 = 0x3FF << 16; +pub const PIPE_A_PENDING: u32 = 1 << 16; + +/// GFX_MSTR_INTR, 190010h: bit 31 Primary Interrupt, the control every +/// graphics interrupt needs to reach the PCI function's message; bit 16 +/// Display. +pub const GFX_MSTR_INTR: usize = 0x19_0010; +pub const PRIMARY_INTERRUPT: u32 = 1 << 31; +pub const DISPLAY_PENDING: u32 = 1 << 16; + /// The highest offset this driver reaches, for the window's bound. -pub const SPAN: usize = MAILBOX_DATA1 + 4; +pub const SPAN: usize = GFX_MSTR_INTR + 4; diff --git a/system/tgl-display/driver/src/tests.rs b/system/tgl-display/driver/src/tests.rs index 5e4f0951b4a..6d3e4a2ba85 100644 --- a/system/tgl-display/driver/src/tests.rs +++ b/system/tgl-display/driver/src/tests.rs @@ -68,6 +68,8 @@ fn the_t14s_gop_gets_levels_one_to_seven_and_its_surface_rearmed_last() { let engine = Engine::t14(); let done = keep_memory_low_power(&engine, &engine).expect("the T14's GOP is programmed"); assert!(done.pixel_rate_khz.abs_diff(138_778) < 139, "pixel rate {} kHz", done.pixel_rate_khz); + assert_eq!((done.scanout.width, done.scanout.height), (1920, 1080)); + assert!(done.refresh_interval_nanos().abs_diff(16_666_667) < 20_000, "{} ns a frame", done.refresh_interval_nanos()); assert_eq!(done.before[0], 0x8000_4011); let wm = |level| regs::plane_wm(0, 1, level); let expected = [ diff --git a/system/tgl-display/src/main.rs b/system/tgl-display/src/main.rs index b204f74c012..195123f9953 100644 --- a/system/tgl-display/src/main.rs +++ b/system/tgl-display/src/main.rs @@ -2,21 +2,26 @@ //! (a `drives` row) for the life of the boot. //! //! **Every decision is `toyos-tgl-display`'s**; this file is the claim, its -//! register window and the clock. The firmware's mode and surface stay the -//! GOP's, which the compositor draws on through the kernel's framebuffer -//! claim; the claim's domain keeps the firmware's reserved memory mapped, so -//! the scan-out reaches the framebuffer across the hand-over. +//! register window, the clock and the compositor's `display-output` +//! connection. The firmware's mode and surface stay the GOP's, which the +//! compositor draws on through the kernel's framebuffer claim; the claim's +//! domain keeps the firmware's reserved memory mapped, so the scan-out reaches +//! the framebuffer across the hand-over. Each present the compositor sends is +//! answered at the vblank after it. //! //! **The process never exits**: the claim's release resets the function, //! which ends the scan-out it was given. -use toyos::{endow, say, PciDev}; -use toyos_abi::syscall::{PciId, RegWidth}; +use toyos::ipc::{FrameRx, RxStep}; +use toyos::poller::{Poller, READABLE}; +use toyos::{endow, say, Connection, PciDev}; +use toyos_abi::syscall::{PciId, RegWidth, SyscallError}; +use toyos_display_output::{Hello, Present, Presented, Size, MAX_FRAME, MSG_HELLO, MSG_PRESENTED, PORT}; use toyos_device_memory::Registers; use toyos_manifest::device::PciMatch; use toyos_pci_claim::Bar; use toyos_tgl_display::dmc::{Package, Stepping, TGL_PACKAGE}; -use toyos_tgl_display::Refusal; +use toyos_tgl_display::{interrupt, Programmed, Refusal, FRAME_CEILING_NANOS}; /// The T14's iGPU, Tiger Lake GT2 (`8086:9a49`), a VGA-compatible display /// controller, class `03:00:00`, as the row spells it. @@ -96,10 +101,92 @@ fn main() { ), Err(why) => say!("tgl-display: DC5 and DC6 stay disallowed — {why}"), } + serve(&dev, ®s, &done); } Err(why) => say!("tgl-display: the firmware's display is left as found — {why}"), } + park() +} + +fn park() -> ! { loop { std::thread::park(); } } + +const TOKEN_CLAIM: u64 = 0; +const TOKEN_COMPOSITOR: u64 = 1; + +/// The firmware's output offered on `display-output`, once: when the +/// connection ends, for whatever reason is said, presents stop and the +/// scan-out stays. A connection the compositor refused would only be refused +/// again. +fn serve(dev: &PciDev, regs: &Bar, done: &Programmed) { + let size = Size { width: done.scanout.width as u32, height: done.scanout.height as u32 }; + interrupt::claim(regs); + let conn = match endow::service(PORT) { + Ok(conn) => conn, + Err(why) => return say!("tgl-display: no `{PORT}` connection ({why:?}); presents go unanswered"), + }; + if let Err(why) = conn.send_bytes(MSG_HELLO, &Hello { size }.encode()) { + return say!("tgl-display: the compositor took no hello ({why:?}); presents go unanswered"); + } + say!("tgl-display: offered the compositor its {}x{} output", size.width, size.height); + let why = answer(dev, regs, &conn, size, done.refresh_interval_nanos()); + say!("tgl-display: presents go unanswered — {why}"); +} + +/// Every present on `conn` answered at the vblank after it arrived, until +/// one cannot be: why. +fn answer(dev: &PciDev, regs: &Bar, conn: &Connection, size: Size, refresh_interval_nanos: u64) -> String { + let poller = Poller::new(2); + let mut rx = FrameRx::::new(); + // The present waiting for its vblank, and when that vblank is overdue. + let mut outstanding: Option<(u64, u64)> = None; + loop { + poller.watch(dev, READABLE, TOKEN_CLAIM); + poller.watch(conn, READABLE, TOKEN_COMPOSITOR); + let timeout = outstanding.map_or(u64::MAX, |(_, overdue)| overdue.saturating_sub(toyos_abi::clock::nanos_since_boot())); + poller.wait(1, timeout, |_| {}); + + match dev.irq() { + Ok(record) => match interrupt::serve(regs) { + Ok(true) => { + if let Some((sequence, _)) = outstanding.take() { + let presented = Presented { sequence, vblank_nanos: record.last_nanos, refresh_interval_nanos }; + if let Err(why) = conn.send_bytes(MSG_PRESENTED, &presented.encode()) { + return format!("the compositor took no answer ({why:?})"); + } + } + } + Ok(false) => {} + Err(why) => return why.to_string(), + }, + Err(SyscallError::WouldBlock) => {} + Err(why) => panic!("tgl-display: the claim's interrupt record: {why:?}"), + } + if outstanding.is_some_and(|(_, overdue)| toyos_abi::clock::nanos_since_boot() >= overdue) { + return format!("no vblank within {} ms of a present", FRAME_CEILING_NANOS / 1_000_000); + } + + loop { + let (msg_type, payload_len) = match rx.pump(conn) { + RxStep::Idle => break, + RxStep::Frame { msg_type, payload_len, .. } => (msg_type, payload_len), + RxStep::Eof => return format!("the compositor hung up `{PORT}`"), + RxStep::Malformed => return "the compositor sent a frame no protocol here can make".into(), + }; + let present = Present::decode(msg_type, rx.payload(payload_len), size); + match (present, outstanding) { + (Some(present), None) => { + interrupt::arm(regs); + outstanding = Some((present.sequence, toyos_abi::clock::nanos_since_boot() + FRAME_CEILING_NANOS)); + } + (Some(present), Some((waiting, _))) => { + return format!("the compositor sent present {} while {waiting} waits", present.sequence); + } + (None, _) => return format!("the compositor sent frame {msg_type}, which `{PORT}` has not"), + } + } + } +} diff --git a/tests/images/clipboardcase.toml b/tests/images/clipboardcase.toml index dcb0294d230..a44d10d2369 100644 --- a/tests/images/clipboardcase.toml +++ b/tests/images/clipboardcase.toml @@ -22,7 +22,7 @@ syscap = ["logread"] [programs.compositor] service = true -serves = ["compositor", "prompt", "clipboard", "input", "display"] +serves = ["compositor", "prompt", "clipboard", "input", "display", "display-output"] devices = ["pci:03:80:00/1af4:1050"] args = ["scale=2"] diff --git a/tests/images/metalcase.toml b/tests/images/metalcase.toml index 073aee6b7b6..a08a046df49 100644 --- a/tests/images/metalcase.toml +++ b/tests/images/metalcase.toml @@ -26,7 +26,7 @@ serves = ["log"] [programs.compositor] service = true -serves = ["compositor", "prompt", "clipboard", "input", "display"] +serves = ["compositor", "prompt", "clipboard", "input", "display", "display-output"] receives = ["soundserver"] devices = ["framebuffer"] @@ -45,6 +45,13 @@ drives = ["pci:04:03:*"] receives = ["sound-output"] syscap = ["rt"] +# The T14's display driver: the firmware's output on `display-output`, and +# PG3, which holds `hda`'s display codec, powered down beside `hda`. No other +# bench has its iGPU, so none starts it. +[programs.tgl-display] +drives = ["pci:03:00:00/8086:9a49"] +receives = ["display-output"] + # netstack over every link a NIC driver brings; on a machine with no virtio NIC # the supervisor starts no driver, and netstack serves with no address. [programs.netstack] diff --git a/tests/images/panelcase.toml b/tests/images/panelcase.toml index 169e69f0565..fa466de0ccc 100644 --- a/tests/images/panelcase.toml +++ b/tests/images/panelcase.toml @@ -16,5 +16,5 @@ syscap = ["logread"] [programs.compositor] service = true -serves = ["compositor", "prompt", "clipboard", "input", "display"] +serves = ["compositor", "prompt", "clipboard", "input", "display", "display-output"] devices = ["framebuffer"] diff --git a/tests/images/scalecase.toml b/tests/images/scalecase.toml index 8bb5db7740d..e16d45d88fd 100644 --- a/tests/images/scalecase.toml +++ b/tests/images/scalecase.toml @@ -18,7 +18,7 @@ syscap = ["logread"] [programs.compositor] service = true -serves = ["compositor", "prompt", "clipboard", "input", "display"] +serves = ["compositor", "prompt", "clipboard", "input", "display", "display-output"] devices = ["framebuffer"] args = ["scale=2"] diff --git a/tests/images/sessioncase.toml b/tests/images/sessioncase.toml index 90307fb1230..36dcdbdb556 100644 --- a/tests/images/sessioncase.toml +++ b/tests/images/sessioncase.toml @@ -32,7 +32,7 @@ login = true [programs.compositor] service = true -serves = ["compositor", "prompt", "clipboard", "input", "display"] +serves = ["compositor", "prompt", "clipboard", "input", "display", "display-output"] devices = ["framebuffer"] starts = ["terminal"] syscap = ["jobs"] diff --git a/tests/toyos.rs b/tests/toyos.rs index 6e76b1fdeff..ec165b552dc 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -1395,6 +1395,16 @@ const METAL: &[(&str, metal::Metal)] = &[ }, }, ), + ( + // The T14's display driver offers the compositor the firmware's + // output on `display-output`, and the compositor's presents are + // answered at their vblanks, which only the iGPU's interrupt raises. + // Beside it `hda` still plays to the analog codec with the display + // engine's PG3, which holds the display codec, powered down. The T14 + // alone: no other bench has a Tiger Lake iGPU. + "display_output_presents", + metal::Metal { arms: METALCASE, judge: |b| display_output_presents(&b[0].log()) }, + ), // ---- the `SYS_DEBUG` members' boot, armed ---- // The cheapest cluster there is: every one of these arms a check that runs // at init, logs its verdict and does nothing else, so they cost no flash @@ -1991,6 +2001,28 @@ fn exited(job: &str) -> String { format!("{}{} pid=", toyos_build::bootlog::EXIT, toyos_build::bootlog::recorded_name(job)) } +/// What [`display_output_presents`] reads off the T14's metalcase boot: the +/// driver's DMC line and its hello taken, a present answered, nothing +/// dropped, and soundserver playing to `hda`'s output. Whether `hda` walked +/// its codecs before or after PG3 went down is said, since only the later +/// order tests the display codec's power. +fn display_output_presents(log: &serial::Serial) -> Result<(), String> { + let dc6 = log.must_say("tgl-display: DMC package")?; + log.must_say("compositor: a display driver scans out the 1920x1080 output")?; + log.must_say("compositor: the display driver answered its first present")?; + for refused in ["compositor: dropping the display driver", "tgl-display: presents go unanswered", "tgl-display: no vblank within"] { + if let Some(line) = log.text().lines().find(|line| line.contains(refused)) { + return Err(format!("the driver's presents stopped: {line}")); + } + } + log.must_say(audio::PLAYING)?; + let offered = log.must_say("offered to soundserver")?; + let at = |line: &str| log.text().find(line).expect("a line said is in the log"); + let order = if at(offered) > at(dc6) { "after" } else { "before" }; + println!("display_output_presents: hda offered its output {order} PG3 went down"); + Ok(()) +} + /// `tests/images/metalcase.toml`, ended once ps2server's probe has: a boot /// that ended sooner would cut off what `ps2server_arms` reads. const METALCASE: &[metal::Arm] = &[metal::Arm { From a84892e6feeaefb0d2a69078874971901b6fdddd Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 1 Nov 2026 13:04:33 +0100 Subject: [PATCH 2/6] display_output_presents: the tone after DC6, and pipe A's frames through it Review r1 of #1132 asks for one T14 boot showing `hda` and the panel unharmed by stage 2. This row's metalcase boot already runs tgl-display beside `hda`; what it lacked was order and the panel. - `tone_after_dc6` waits in this boot's log for tgl-display's DC6 allow, then plays `audio_tone`'s muted tone through `hda` to its end, so the analog codec is shown playing with PG3 down. A refusal of DC6 fails it by name. metalcase's test-runner now receives `volume`, which the muted tone needs. - tgl-display logs pipe A's frame counter at its first answered present. The judge holds it past the counter logged with the allow: the timing generator ran on after DC6 was allowed. - The judge requires DC6 allowed on DDI A with DC_STATE_EN 2. It says whether `hda` walked its codecs before or after PG3 went down. - `audio::tone_played` is `tone_on_metal` for any job. Co-Authored-By: Claude Opus 5.5 --- system/tgl-display/src/main.rs | 5 ++ tests/common/audio.rs | 9 +++- .../rust-tests/src/bin/tone_after_dc6.rs | 34 ++++++++++++ tests/images/metalcase.toml | 2 +- tests/toyos.rs | 53 +++++++++++++++---- 5 files changed, 89 insertions(+), 14 deletions(-) create mode 100644 tests/guest/rust-tests/src/bin/tone_after_dc6.rs diff --git a/system/tgl-display/src/main.rs b/system/tgl-display/src/main.rs index 195123f9953..8a301a52e11 100644 --- a/system/tgl-display/src/main.rs +++ b/system/tgl-display/src/main.rs @@ -143,6 +143,7 @@ fn answer(dev: &PciDev, regs: &Bar, conn: &Connection, size: Size, refresh_inter let mut rx = FrameRx::::new(); // The present waiting for its vblank, and when that vblank is overdue. let mut outstanding: Option<(u64, u64)> = None; + let mut answered = false; loop { poller.watch(dev, READABLE, TOKEN_CLAIM); poller.watch(conn, READABLE, TOKEN_COMPOSITOR); @@ -157,6 +158,10 @@ fn answer(dev: &PciDev, regs: &Bar, conn: &Connection, size: Size, refresh_inter if let Err(why) = conn.send_bytes(MSG_PRESENTED, &presented.encode()) { return format!("the compositor took no answer ({why:?})"); } + if !answered { + answered = true; + say!("tgl-display: first present answered at pipe A frame {}", regs.read32(toyos_tgl_display::regs::PIPE_FRMCNT_A)); + } } } Ok(false) => {} diff --git a/tests/common/audio.rs b/tests/common/audio.rs index bd766d80e73..f38b22f3631 100644 --- a/tests/common/audio.rs +++ b/tests/common/audio.rs @@ -71,8 +71,13 @@ fn on_an_output(log: &str, job: &str) -> Result<(), String> { /// The tone client played to its end through the HDA controller an `hda` /// driver drives, played to before it ran, and no period of it went unfilled. pub fn tone_on_metal(log: &Serial) -> Result<(), String> { - on_an_output(log.text(), "test_rs_audio_tone")?; - let window = job_window(log.text(), "test_rs_audio_tone")?; + tone_played(log, "test_rs_audio_tone") +} + +/// `job`'s tone played to an output whole, every period its own. +pub fn tone_played(log: &Serial, job: &str) -> Result<(), String> { + on_an_output(log.text(), job)?; + let window = job_window(log.text(), job)?; let underruns = sum_field(window, "underruns"); if underruns != 0 { return Err(format!( diff --git a/tests/guest/rust-tests/src/bin/tone_after_dc6.rs b/tests/guest/rust-tests/src/bin/tone_after_dc6.rs new file mode 100644 index 00000000000..a8fdaa37d1f --- /dev/null +++ b/tests/guest/rust-tests/src/bin/tone_after_dc6.rs @@ -0,0 +1,34 @@ +//! Wait until `/system/bin/tgl-display` has allowed DC6, with PG3 and the +//! display codec's audio well down, then play `audio_tone`'s tone through +//! `hda` to its end. +//! +//! The `display_output_presents` metal row runs it on the T14: the tone is +//! its evidence that `hda`'s analog codec plays on with PG3 down. + +use std::time::Duration; + +use cpal::traits::{DeviceTrait, HostTrait}; +use toyos_logstream::program_line; + +#[path = "../served_log.rs"] +mod served_log; +#[path = "../tone.rs"] +mod tone; + +/// How long after boot the wait gives up: the runner's bound less a tenth. +const UNTIL_MS: u64 = toyos_tco::JOB_BOUND_MS - toyos_tco::JOB_BOUND_MS / 10; + +fn main() { + let since_boot = toyos_abi::clock::nanos_since_boot() / 1_000_000; + let bound = Duration::from_millis(UNTIL_MS.saturating_sub(since_boot)); + served_log::Log::open().until("tgl-display's DC6 allow", bound, |line| { + program_line(line).is_some_and(|said| { + assert!(!said.text.contains("DC5 and DC6 stay disallowed"), "tgl-display refused DC6: {}", said.text); + said.tag == "tgl-display" && said.text.contains("DMC package") + }) + }); + let device = cpal::default_host().default_output_device().expect("no audio output device"); + let config = device.default_output_config().expect("no audio config"); + tone::play(&device, config.into()); + println!("tone done"); +} diff --git a/tests/images/metalcase.toml b/tests/images/metalcase.toml index a08a046df49..9840a2e46bd 100644 --- a/tests/images/metalcase.toml +++ b/tests/images/metalcase.toml @@ -81,7 +81,7 @@ devices = ["isa:0060,0064:1,12"] # The union its guest binaries need on this machine shape; `log` is what # `ps2server_hold` reads ps2server's probe off. [programs.test-runner] -receives = ["compositor", "soundserver", "netstack", "log"] +receives = ["compositor", "soundserver", "volume", "netstack", "log"] syscap = ["logread"] # The block service: every disk a driver offers on `disk`, listed and opened diff --git a/tests/toyos.rs b/tests/toyos.rs index ec165b552dc..31c475a3c85 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -211,6 +211,9 @@ const RUST_SKIP: &[&str] = &[ // queries, and no guest's machine has that controller: the // `acpi_server_events` metal row runs it. "acpi_counted", + // It waits for tgl-display's DC6 allow, which only the T14's iGPU has: + // the `display_output_presents` metal row runs it. + "tone_after_dc6", // It claims the fixed hardware itself, which needs a boot that starts no // server, and stages the firmware's side of the Global Lock, which needs // the test kernel: `acpi_mediated_access` and `acpi_press_is_one_sci` @@ -1403,7 +1406,7 @@ const METAL: &[(&str, metal::Metal)] = &[ // engine's PG3, which holds the display codec, powered down. The T14 // alone: no other bench has a Tiger Lake iGPU. "display_output_presents", - metal::Metal { arms: METALCASE, judge: |b| display_output_presents(&b[0].log()) }, + metal::Metal { arms: METALCASE_TONE, judge: |b| display_output_presents(b[0]) }, ), // ---- the `SYS_DEBUG` members' boot, armed ---- // The cheapest cluster there is: every one of these arms a check that runs @@ -2001,25 +2004,46 @@ fn exited(job: &str) -> String { format!("{}{} pid=", toyos_build::bootlog::EXIT, toyos_build::bootlog::recorded_name(job)) } -/// What [`display_output_presents`] reads off the T14's metalcase boot: the -/// driver's DMC line and its hello taken, a present answered, nothing -/// dropped, and soundserver playing to `hda`'s output. Whether `hda` walked -/// its codecs before or after PG3 went down is said, since only the later -/// order tests the display codec's power. -fn display_output_presents(log: &serial::Serial) -> Result<(), String> { +/// What [`display_output_presents`] reads off the T14's metalcase boot: +/// - the driver's DMC line, with DC6 allowed on DDI A; +/// - its hello taken and a present answered, at a later frame of pipe A's +/// counter than the allow, so the timing generator ran through DC6 being +/// allowed; +/// - nothing dropped; +/// - `tone_after_dc6`'s tone played whole to `hda`'s output, which it starts +/// only once PG3 is down. +/// +/// Whether `hda` walked its codecs before or after PG3 went down is said, +/// since only the later order tests the display codec's power. +fn display_output_presents(boot: &metal::Readback) -> Result<(), String> { + let log = boot.log(); + let frame = |line: &str, after: &str| -> Result { + line.split_once(after).and_then(|(_, n)| n.trim().parse().ok()).ok_or_else(|| format!("no frame number in {line:?}")) + }; let dc6 = log.must_say("tgl-display: DMC package")?; + if !dc6.contains("DDI select 0x1;") || !dc6.contains(" 00000002, AUDIO_PIN_BUF_CTL") { + return Err(format!("DC6 was not allowed on DDI A: {dc6}")); + } + let answered = log.must_say("tgl-display: first present answered at pipe A frame")?; + let (allowed_at, answered_at) = (frame(dc6, "pipe A frame")?, frame(answered, "pipe A frame")?); + if answered_at <= allowed_at { + return Err(format!("pipe A's frame counter read {allowed_at} at the allow and {answered_at} at the first answer")); + } log.must_say("compositor: a display driver scans out the 1920x1080 output")?; log.must_say("compositor: the display driver answered its first present")?; - for refused in ["compositor: dropping the display driver", "tgl-display: presents go unanswered", "tgl-display: no vblank within"] { + for refused in ["compositor: dropping the display driver", "tgl-display: presents go unanswered"] { if let Some(line) = log.text().lines().find(|line| line.contains(refused)) { return Err(format!("the driver's presents stopped: {line}")); } } - log.must_say(audio::PLAYING)?; - let offered = log.must_say("offered to soundserver")?; + boot.job_passed("test_rs_tone_after_dc6")?; + audio::tone_played(&log, "test_rs_tone_after_dc6")?; let at = |line: &str| log.text().find(line).expect("a line said is in the log"); + let offered = log.must_say("offered to soundserver")?; let order = if at(offered) > at(dc6) { "after" } else { "before" }; - println!("display_output_presents: hda offered its output {order} PG3 went down"); + println!( + "display_output_presents: pipe A frames {allowed_at} -> {answered_at}; hda offered its output {order} PG3 went down, and the tone played after" + ); Ok(()) } @@ -2037,6 +2061,13 @@ const METALCASE_SCALE: &[metal::Arm] = &[metal::Arm { ..metal::once("metalcase", "tests/images/metalcase.toml", &[], &["test_rs_scale_window"]) }]; +/// [`METALCASE`] with `tone_after_dc6`, which `display_output_presents` +/// plays once the display driver has PG3 down, muted like every metal tone. +const METALCASE_TONE: &[metal::Arm] = &[metal::Arm { + last: Some("test_rs_ps2server_hold"), + ..metal::once("metalcase", "tests/images/metalcase.toml", &[], &["test_rs_tone_after_dc6"]) +}]; + /// A launcher and a declared `cat` and shell, which `process_tree`'s subtree /// launches, a `toybox` row holding `roster`, which `launch_toctou` races, and /// the `/apps` `app_view` launches its package from. From 8105cae15d1559e652978a79da5dc937884df232 Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 1 Nov 2026 15:11:19 +0100 Subject: [PATCH 3/6] audio: a tone played to an output is judged at its stream's opening, not its spawn `on_an_output` held soundserver's first output offer to before the tone job's spawn. `tone_after_dc6` spawns at boot and waits for tgl-display's DC6 allow before it plays, so on the T14 (f256164ab) it was judged to play to the null sink, though its stream opened half a second after `hda`'s output was offered. The claim is about the stream, so the judge reads the first `soundserver: opening stream` after the spawn. For a job that plays at once nothing changes. Co-Authored-By: Claude Opus 5.5 --- tests/common/audio.rs | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/tests/common/audio.rs b/tests/common/audio.rs index f38b22f3631..74f7dac90ab 100644 --- a/tests/common/audio.rs +++ b/tests/common/audio.rs @@ -54,20 +54,29 @@ fn is_soundserver_stats(text: &str, at: usize) -> bool { text[line_start..at].contains("soundserver: wakes=") } -/// soundserver played to an output driver's device before `job` ran, so the -/// job's stream did not play to the null sink. +/// soundserver played to an output driver's device before `job` opened its +/// stream, so the stream did not play to the null sink. The stream's +/// opening and not the job's spawn: a job may wait for something before it +/// plays. fn on_an_output(log: &str, job: &str) -> Result<(), String> { let playing = log .find(PLAYING) .ok_or_else(|| format!("soundserver never said `{PLAYING}`: no output was offered, and it stayed the null sink"))?; let head = format!("spawn: /system/bin/{job} "); let spawned = log.find(&head).ok_or_else(|| format!("no `{head}` record: {job} never ran"))?; - if playing > spawned { - return Err(format!("soundserver played to an output only after {job} spawned, so its stream played to the null sink")); + let opened = log[spawned..] + .find(OPENING) + .map(|at| spawned + at) + .ok_or_else(|| format!("soundserver never said `{OPENING}` after {job} spawned: it opened no stream"))?; + if playing > opened { + return Err(format!("soundserver played to an output only after {job} opened its stream, so it played to the null sink")); } Ok(()) } +/// What soundserver says as a client's stream opens. +const OPENING: &str = "soundserver: opening stream"; + /// The tone client played to its end through the HDA controller an `hda` /// driver drives, played to before it ran, and no period of it went unfilled. pub fn tone_on_metal(log: &Serial) -> Result<(), String> { From a2b3e9733fee2473888f734282907518915bd329 Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 1 Nov 2026 15:35:25 +0100 Subject: [PATCH 4/6] audio: a stream is marked by its client connection, which every log of one carries f5668732e marked a tone job's stream by "soundserver: opening stream". The harness's own audio fixture has soundserver say only "client N connected", as every real log also does, so `metal_audio_judges` went red. The judge now takes the first client connection soundserver says after the spawn. Co-Authored-By: Claude Opus 5.5 --- tests/common/audio.rs | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/tests/common/audio.rs b/tests/common/audio.rs index 74f7dac90ab..a155dc59e04 100644 --- a/tests/common/audio.rs +++ b/tests/common/audio.rs @@ -64,18 +64,25 @@ fn on_an_output(log: &str, job: &str) -> Result<(), String> { .ok_or_else(|| format!("soundserver never said `{PLAYING}`: no output was offered, and it stayed the null sink"))?; let head = format!("spawn: /system/bin/{job} "); let spawned = log.find(&head).ok_or_else(|| format!("no `{head}` record: {job} never ran"))?; + let mut at = spawned; let opened = log[spawned..] - .find(OPENING) - .map(|at| spawned + at) - .ok_or_else(|| format!("soundserver never said `{OPENING}` after {job} spawned: it opened no stream"))?; + .split_inclusive('\n') + .find_map(|line| { + let here = at; + at += line.len(); + (line.contains(CLIENT) && line.contains(CONNECTED)).then_some(here) + }) + .ok_or_else(|| format!("soundserver never said a client connected after {job} spawned: it opened no stream"))?; if playing > opened { return Err(format!("soundserver played to an output only after {job} opened its stream, so it played to the null sink")); } Ok(()) } -/// What soundserver says as a client's stream opens. -const OPENING: &str = "soundserver: opening stream"; +/// What soundserver says as a client's stream connects: +/// `soundserver: client 0 connected (id=0)`. +const CLIENT: &str = "soundserver: client "; +const CONNECTED: &str = " connected (id="; /// The tone client played to its end through the HDA controller an `hda` /// driver drives, played to before it ran, and no period of it went unfilled. From 4d54cce89dde7e2d81a260b4de2344d88c4202d5 Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 1 Nov 2026 16:26:02 +0100 Subject: [PATCH 5/6] review r1 of #1134: no panic on a late hello, bounded reconnects, and the interrupt comments true - A driver whose first frame came after the handshake timeout woke the compositor readable and overdue in one turn. Readiness was computed before the sweep dropped it, and the read unwrapped what had gone, so the compositor panicked. The connection's state is now `toyos_display_output::Output`, which refuses a hello past its deadline in the very call that reads it. The session computes readiness after the sweep and unwraps nothing. Host tests cover: - a hello one nanosecond late; - every way out of protocol, a frame carrying handles among them; - a served output's answers. - The driver connects again when a compositor that was answering presents goes away, a restart or a `swap`, at most 8 times a boot, and says each attempt. The queued connection, and the compositor's next present, are the events it waits on; there is no delay. A protocol refusal, or a compositor gone before it took a hello, ends presents by name. Frames carrying handles are refused, since the protocol has none. On the way out the driver disarms everything (`interrupt::release`). - `regs.rs`: each IMR is 8 below its IER, not 4. `interrupt::claim` names what it masks: every category Vol 2c has registers for, with PCH and PICA left as found and refused by `serve` when pending. - Two harness fixtures hold `on_an_output` to the stream's connection: an output offered between a waiting job's spawn and its stream passes, and one offered after the stream fails. Co-Authored-By: Claude Opus 5.5 --- ...ves-its-displays-from-a-userland-driver.md | 1 + lib/display-output/src/lib.rs | 134 ++++++++++++++++++ system/compositor/src/output.rs | 73 +++++----- system/compositor/src/session.rs | 19 ++- system/tgl-display/driver/src/interrupt.rs | 27 +++- system/tgl-display/driver/src/lib.rs | 4 +- system/tgl-display/driver/src/regs.rs | 3 +- system/tgl-display/src/main.rs | 76 +++++++--- tests/checks/audio.rs | 12 ++ 9 files changed, 278 insertions(+), 71 deletions(-) diff --git a/issues/each-gpu-drives-its-displays-from-a-userland-driver.md b/issues/each-gpu-drives-its-displays-from-a-userland-driver.md index e0fc0d588af..60994953ca5 100644 --- a/issues/each-gpu-drives-its-displays-from-a-userland-driver.md +++ b/issues/each-gpu-drives-its-displays-from-a-userland-driver.md @@ -89,6 +89,7 @@ The kernel keeps only generic mechanisms: - One present is outstanding at a time. Damage composed meanwhile is merged, past 8 rectangles into their bounding box, and sent when the answer arrives. - The compositor serves `display-output`, one driver at a time, and never waits on it. - `tgl-display` answers each present at the vblank after it arrived. That vblank is pipe A's interrupt, enabled only while a present waits, and its wait has a hang ceiling. + - When a compositor that was taking presents goes away (a restart, a `swap`), the driver connects again, at most 8 times a boot. A protocol refusal, or a compositor gone before it took a hello, ends presents for the boot, by name. - The compositor does not yet pace frames by `Presented`. It still blits the firmware's framebuffer itself until stage 5. 4. **PSR on the eDP panel**, measured together with stage 2. PSR's first-frame exit latency goes to the owner in absolute numbers before PSR lands. 5. **The rest of `display-output`.** virtio-gpu and the framebuffer become drivers. The panic surface lands here. diff --git a/lib/display-output/src/lib.rs b/lib/display-output/src/lib.rs index 81b67cbbf23..643d36420b4 100644 --- a/lib/display-output/src/lib.rs +++ b/lib/display-output/src/lib.rs @@ -300,6 +300,98 @@ impl Presenting { } } +/// Why the compositor ends a driver's connection. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub enum Ended { + /// No hello came before the deadline. + Late, + /// The hello offers an output of another size than the desktop is drawn + /// at. + Size { offered: Size, drawn: Size }, + /// A frame this protocol does not have, or one carrying handles, which + /// none of its frames carries. + Frame(u32), + /// A frame of this protocol that the connection's state has not: a second + /// hello, or an answer before the hello. + OutOfState(u32), + /// An answer to no outstanding present. + Unasked(Unasked), +} + +/// What a frame from the driver did. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub enum Received { + /// The hello was taken. + Offered(Size), + /// A present was answered: the driver's first answer or a later one, and + /// the present owed since, if any. + Answered { first: bool, presented: Presented, next: Option }, +} + +/// The compositor's side of one driver's connection: its hello awaited until +/// a deadline, then its output presented to. +#[derive(Debug)] +pub struct Output { + deadline_nanos: u64, + serving: Option, +} + +#[derive(Debug)] +struct Serving { + size: Size, + presenting: Presenting, + answered: bool, +} + +impl Output { + /// A connection accepted, whose hello is due by `deadline_nanos`. + pub fn new(deadline_nanos: u64) -> Self { + Self { deadline_nanos, serving: None } + } + + /// Whether its hello is overdue at `now_nanos`. + pub fn overdue(&self, now_nanos: u64) -> bool { + self.serving.is_none() && now_nanos > self.deadline_nanos + } + + /// The output it scans out, once its hello was taken. + pub fn size(&self) -> Option { + self.serving.as_ref().map(|serving| serving.size) + } + + /// A frame of `msg_type` carrying `payload` and `handles` handles, read + /// at `now_nanos` from a driver of a desktop drawn at `drawn`. + pub fn received(&mut self, now_nanos: u64, msg_type: u32, payload: &[u8], handles: usize, drawn: Size) -> Result { + if self.overdue(now_nanos) { + return Err(Ended::Late); + } + if handles != 0 { + return Err(Ended::Frame(msg_type)); + } + match (FromDriver::decode(msg_type, payload).ok_or(Ended::Frame(msg_type))?, &mut self.serving) { + (FromDriver::Hello(hello), None) => { + if hello.size != drawn { + return Err(Ended::Size { offered: hello.size, drawn }); + } + self.serving = Some(Serving { size: hello.size, presenting: Presenting::new(), answered: false }); + Ok(Received::Offered(hello.size)) + } + (FromDriver::Presented(presented), Some(serving)) => { + let next = serving.presenting.presented(&presented).map_err(Ended::Unasked)?; + let first = !core::mem::replace(&mut serving.answered, true); + Ok(Received::Answered { first, presented, next }) + } + _ => Err(Ended::OutOfState(msg_type)), + } + } + + /// `rects` of the output were composed: the present to send now, if one + /// is due. Nothing before the hello. + pub fn composed(&mut self, rects: impl IntoIterator) -> Option { + self.serving.as_mut()?.presenting.damaged(rects) + } +} + #[cfg(test)] mod tests { use super::*; @@ -399,6 +491,48 @@ mod tests { assert_eq!(presenting.damaged([rect(1, 1, 1, 1)]).map(|p| p.sequence), Some(3)); } + /// A hello that comes after the deadline ends the connection by name, + /// in the same call that reads it: no other state can be stale. + #[test] + fn a_hello_after_the_deadline_ends_the_connection() { + let hello = Hello { size: HD }.encode(); + let mut output = Output::new(2_000); + assert!(!output.overdue(2_000)); + assert!(output.overdue(2_001)); + assert_eq!(output.received(2_001, MSG_HELLO, &hello, 0, HD), Err(Ended::Late)); + let mut output = Output::new(2_000); + assert_eq!(output.received(1_999, MSG_HELLO, &hello, 0, HD), Ok(Received::Offered(HD))); + assert!(!output.overdue(u64::MAX), "a hello taken is never overdue"); + } + + #[test] + fn a_driver_out_of_protocol_ends_its_connection_by_name() { + let hello = Hello { size: HD }.encode(); + let other = Size { width: 1280, height: 800 }; + assert_eq!(Output::new(10).received(0, MSG_HELLO, &hello, 0, other), Err(Ended::Size { offered: HD, drawn: other })); + assert_eq!(Output::new(10).received(0, MSG_HELLO, &hello, 1, HD), Err(Ended::Frame(MSG_HELLO)), "a handle"); + assert_eq!(Output::new(10).received(0, 99, &[], 0, HD), Err(Ended::Frame(99))); + let answer = Presented { sequence: 1, vblank_nanos: 5, refresh_interval_nanos: 16_666_667 }.encode(); + assert_eq!(Output::new(10).received(0, MSG_PRESENTED, &answer, 0, HD), Err(Ended::OutOfState(MSG_PRESENTED)), "before the hello"); + let mut output = Output::new(10); + output.received(0, MSG_HELLO, &hello, 0, HD).expect("taken"); + assert_eq!(output.received(0, MSG_HELLO, &hello, 0, HD), Err(Ended::OutOfState(MSG_HELLO)), "a second hello"); + assert_eq!(output.received(0, MSG_PRESENTED, &answer, 0, HD), Err(Ended::Unasked(Unasked { sequence: 1, outstanding: None }))); + } + + #[test] + fn a_served_output_is_presented_to_and_its_first_answer_is_said() { + let mut output = Output::new(10); + assert_eq!(output.composed([rect(0, 0, 1, 1)]), None, "nothing before the hello"); + output.received(0, MSG_HELLO, &Hello { size: HD }.encode(), 0, HD).expect("taken"); + let first = output.composed([rect(0, 0, 1, 1)]).expect("a present"); + let presented = Presented { sequence: first.sequence, vblank_nanos: 5, refresh_interval_nanos: 16_666_667 }; + assert_eq!(output.received(100, MSG_PRESENTED, &presented.encode(), 0, HD), Ok(Received::Answered { first: true, presented, next: None })); + let second = output.composed([rect(1, 1, 1, 1)]).expect("a present"); + let presented = Presented { sequence: second.sequence, ..presented }; + assert!(matches!(output.received(200, MSG_PRESENTED, &presented.encode(), 0, HD), Ok(Received::Answered { first: false, .. }))); + } + #[test] fn an_answer_to_no_outstanding_present_is_the_drivers_error() { let mut presenting = Presenting::new(); diff --git a/system/compositor/src/output.rs b/system/compositor/src/output.rs index 61e1849565f..3ed4b46cf60 100644 --- a/system/compositor/src/output.rs +++ b/system/compositor/src/output.rs @@ -1,5 +1,5 @@ -//! The display driver on `display-output` (`toyos_display_output`), one at a -//! time: told every frame composed into the output it scans out, and where. +//! The display driver on `display-output`, one at a time: its connection's +//! IO. Every decision about it is `toyos_display_output::Output`'s. //! //! The compositor never waits on it. Each present is one `try_send`. A //! refusal, a frame out of protocol or a hello that does not come within @@ -11,7 +11,7 @@ use std::time::Instant; use toyos::ipc::{FrameRx, RxStep}; use toyos::Connection; use toyos_desktop::{Mode, Rect}; -use toyos_display_output::{FromDriver, Present, Presenting, Size, MAX_FRAME, MSG_PRESENT}; +use toyos_display_output::{Ended, Output, Present, Received, Size, MAX_FRAME, MSG_PRESENT}; use crate::client::HANDSHAKE_TIMEOUT; @@ -21,53 +21,46 @@ const TURN_FRAMES: usize = 4; pub struct Driver { conn: Connection, rx: FrameRx, - since: Instant, - serving: Option, -} - -struct Serving { - size: Size, - presenting: Presenting, - answered: u64, + epoch: Instant, + output: Output, } impl Driver { - pub fn new(conn: Connection) -> Self { - Self { conn, rx: FrameRx::new(), since: Instant::now(), serving: None } + pub fn new(conn: Connection, epoch: Instant) -> Self { + let deadline = (Instant::now() + HANDSHAKE_TIMEOUT).duration_since(epoch).as_nanos() as u64; + Self { conn, rx: FrameRx::new(), epoch, output: Output::new(deadline) } } pub fn conn(&self) -> &Connection { &self.conn } + fn now(&self) -> u64 { + self.epoch.elapsed().as_nanos() as u64 + } + /// Whether its hello is overdue. - pub fn overdue(&self, now: Instant) -> bool { - self.serving.is_none() && now.duration_since(self.since) >= HANDSHAKE_TIMEOUT + pub fn overdue(&self) -> bool { + self.output.overdue(self.now()) } /// Whatever the driver sent, each frame whole; `Err` names why it is /// dropped. Its hello must offer the output the desktop is drawn on, /// `mode`. pub fn read(&mut self, mode: Mode) -> Result<(), String> { + let drawn = Size { width: mode.width, height: mode.height }; for _ in 0..TURN_FRAMES { - let (msg_type, payload_len) = match self.rx.pump(&self.conn) { + let (msg_type, payload_len, handles) = match self.rx.pump(&self.conn) { RxStep::Idle => return Ok(()), RxStep::Eof => return Err("its connection ended".into()), RxStep::Malformed => return Err("a frame no protocol here can make".into()), - RxStep::Frame { msg_type, payload_len, .. } => (msg_type, payload_len), + RxStep::Frame { msg_type, payload_len, handles } => (msg_type, payload_len, handles), }; - match (FromDriver::decode(msg_type, self.rx.payload(payload_len)), &mut self.serving) { - (Some(FromDriver::Hello(hello)), None) => { - if (hello.size.width, hello.size.height) != (mode.width, mode.height) { - return Err(format!("it offers {}x{}, and the desktop is drawn at {mode}", hello.size.width, hello.size.height)); - } - println!("compositor: a display driver scans out the {mode} output"); - self.serving = Some(Serving { size: hello.size, presenting: Presenting::new(), answered: 0 }); - } - (Some(FromDriver::Presented(presented)), Some(serving)) => { - let next = serving.presenting.presented(&presented).map_err(|unasked| format!("{unasked:?}"))?; - serving.answered += 1; - if serving.answered == 1 { + let now = self.now(); + match self.output.received(now, msg_type, self.rx.payload(payload_len), handles, drawn) { + Ok(Received::Offered(_)) => println!("compositor: a display driver scans out the {mode} output"), + Ok(Received::Answered { first, presented, next }) => { + if first { println!( "compositor: the display driver answered its first present, refreshing every {} ns", presented.refresh_interval_nanos @@ -77,7 +70,7 @@ impl Driver { send(&self.conn, &present)?; } } - _ => return Err(format!("frame {msg_type}, which its state has not")), + Err(ended) => return Err(said(ended)), } } Ok(()) @@ -86,8 +79,7 @@ impl Driver { /// `regions` of the output were composed: presented now, or merged into /// the present after the one outstanding. pub fn composed(&mut self, regions: &[Rect]) -> Result<(), String> { - let Some(serving) = &mut self.serving else { return Ok(()) }; - let size = serving.size; + let Some(size) = self.output.size() else { return Ok(()) }; let rects = regions.iter().filter_map(|r| { let r = r.intersect(Rect::new(0, 0, size.width as i32, size.height as i32)); if r.is_empty() { @@ -96,13 +88,28 @@ impl Driver { let at = |v: i32| u32::try_from(v).expect("a rectangle clipped to the output is inside it"); Some(toyos_display_output::Rect { x: at(r.x0), y: at(r.y0), width: at(r.w()), height: at(r.h()) }) }); - match serving.presenting.damaged(rects) { + match self.output.composed(rects) { Some(present) => send(&self.conn, &present), None => Ok(()), } } } +fn said(ended: Ended) -> String { + match ended { + Ended::Late => format!("its hello did not come within {HANDSHAKE_TIMEOUT:?}"), + Ended::Size { offered, drawn } => { + format!("it offers {}x{}, and the desktop is drawn at {}x{}", offered.width, offered.height, drawn.width, drawn.height) + } + Ended::Frame(msg_type) => format!("frame {msg_type}, which `display-output` has not"), + Ended::OutOfState(msg_type) => format!("frame {msg_type}, which its state has not"), + Ended::Unasked(unasked) => match unasked.outstanding { + Some(outstanding) => format!("it answered present {} while {outstanding} was outstanding", unasked.sequence), + None => format!("it answered present {} with none outstanding", unasked.sequence), + }, + } +} + fn send(conn: &Connection, present: &Present) -> Result<(), String> { let mut frame = [0u8; MAX_FRAME]; let n = present.encode(&mut frame); diff --git a/system/compositor/src/session.rs b/system/compositor/src/session.rs index 567979b02f4..0c18b5b4494 100644 --- a/system/compositor/src/session.rs +++ b/system/compositor/src/session.rs @@ -508,7 +508,6 @@ impl Session { let clipboard_ready = self.is_ready(self.clipboards.as_handle().raw()); let settings_ready = self.is_ready(self.settings.as_handle().raw()); let outputs_ready = self.is_ready(self.outputs.as_handle().raw()); - let output_ready = self.output.as_ref().is_some_and(|d| self.is_ready(d.conn().as_handle().raw())); let volume_ready = self.volume.as_ref().is_some_and(|(conn, _)| self.is_ready(conn.as_handle().raw())); let display_ready = self.display.handle().is_some_and(|handle| self.is_ready(handle.raw())); let session_ready = self.desktop.conn().is_some_and(|c| self.is_ready(c.as_handle().raw())); @@ -530,9 +529,11 @@ impl Session { eprintln!("compositor: dropping client {} — {}", p.conn.as_handle().raw().0, reason); } self.pending.retain(|p| now.duration_since(p.since) < HANDSHAKE_TIMEOUT); - if self.output.as_ref().is_some_and(|d| d.overdue(now)) { - self.drop_output("its hello did not come in time".into()); + if self.output.as_ref().is_some_and(|d| d.overdue()) { + self.drop_output(format!("its hello did not come within {HANDSHAKE_TIMEOUT:?}")); } + // After the sweep: a driver it dropped is not read. + let output_ready = self.output.as_ref().is_some_and(|d| self.is_ready(d.conn().as_handle().raw())); if !inputs_ready && sources_ready.is_empty() @@ -578,13 +579,11 @@ impl Session { } if output_ready { let mode = self.display.mode(); - let read = self.output.as_mut().expect("a ready driver is held").read(mode); - match read { - Ok(()) => { - let conn = self.output.as_ref().expect("a ready driver is held").conn(); - self.poller.watch(conn, READABLE, conn.as_handle().raw().0 as u64); + if let Some(driver) = self.output.as_mut() { + match driver.read(mode) { + Ok(()) => self.poller.watch(driver.conn(), READABLE, driver.conn().as_handle().raw().0 as u64), + Err(why) => self.drop_output(why), } - Err(why) => self.drop_output(why), } } if volume_ready { @@ -620,7 +619,7 @@ impl Session { Ok(_) if self.output.is_some() => eprintln!("compositor: refusing a display driver — one is connected"), Ok(conn) => { self.poller.watch(&conn, READABLE, conn.as_handle().raw().0 as u64); - self.output = Some(crate::output::Driver::new(conn)); + self.output = Some(crate::output::Driver::new(conn, self.epoch)); } } } diff --git a/system/tgl-display/driver/src/interrupt.rs b/system/tgl-display/driver/src/interrupt.rs index 4b2a1911094..32e1410531a 100644 --- a/system/tgl-display/driver/src/interrupt.rs +++ b/system/tgl-display/driver/src/interrupt.rs @@ -6,9 +6,11 @@ use toyos_device_memory::Registers; use crate::{regs, Refusal}; -/// Every display interrupt but pipe A's vblank masked and disabled, pipe A's -/// identity cleared and display and primary interrupts enabled. Nothing is -/// raised until [`arm`]. +/// Every category Vol 2c gives registers for masked and disabled but pipe +/// A's vblank, pipe A's identity cleared and display and primary interrupts +/// enabled. Nothing is raised until [`arm`]. The PCH and PICA categories +/// have no registers in Vol 2c and are left as found; one pending is +/// refused by [`serve`]. pub fn claim(regs: &R) { for ier in regs::UNUSED_IERS { regs.write32(ier, 0); @@ -54,6 +56,14 @@ pub fn serve(regs: &R) -> Result { Ok(vblank) } +/// Nothing armed and display and primary interrupts disabled: what a driver +/// that has stopped answering presents leaves. +pub fn release(regs: &R) { + regs.write32(regs::PIPE_A_IER, 0); + regs.write32(regs::DISPLAY_INT_CTL, 0); + regs.write32(regs::GFX_MSTR_INTR, 0); +} + /// Pipe A's identity bits cleared, both events it can hold ("The IIR can /// queue up to two interrupt events"); what was set. fn clear(regs: &R) -> u32 { @@ -110,6 +120,17 @@ mod tests { assert!(!raised(&engine), "disarmed by the vblank it served"); } + #[test] + fn a_released_claim_raises_nothing_even_armed() { + let engine = Engine::t14(); + claim(&engine); + arm(&engine); + release(&engine); + engine.vblank(); + assert!(!raised(&engine)); + assert_eq!((engine.get(regs::DISPLAY_INT_CTL), engine.get(regs::GFX_MSTR_INTR)), (0, 0)); + } + #[test] fn a_category_no_one_enabled_is_refused_and_display_interrupts_stay_off() { let engine = Engine::t14(); diff --git a/system/tgl-display/driver/src/lib.rs b/system/tgl-display/driver/src/lib.rs index fb65ff944de..8abbdb5fb77 100644 --- a/system/tgl-display/driver/src/lib.rs +++ b/system/tgl-display/driver/src/lib.rs @@ -60,8 +60,8 @@ const POWER_WELL_NANOS: u64 = 20_000; /// display on eDP"). const UNUSED_WELLS: [u32; 3] = [5, 4, 3]; -/// Why this driver stopped short; [`keep_memory_low_power`] and -/// [`allow_display_c6`] say how far each got. +/// Why this driver stopped short; [`keep_memory_low_power`], +/// [`allow_display_c6`] and [`interrupt::serve`] say how far each got. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum Refusal { /// The firmware's state is not one pipe A plane scanning out alone. diff --git a/system/tgl-display/driver/src/regs.rs b/system/tgl-display/driver/src/regs.rs index af555a90257..455e3a0088f 100644 --- a/system/tgl-display/driver/src/regs.rs +++ b/system/tgl-display/driver/src/regs.rs @@ -122,7 +122,8 @@ pub const RUN_BUSY: u32 = 1 << 31; /// DE_PIPE_INTERRUPT_A (Vol 2c), 44400h-4440Fh: ISR, IMR, IIR and IER, bit 0 /// Vblank. The other categories this driver enables nothing in, at their IER: /// pipes B-D, DE_PORT_INTERRUPT, DE_MISC_INTERRUPT, DE_HPD_INTERRUPT and -/// AUD_INTERRUPT. Each IMR is 4 below its IER. +/// AUD_INTERRUPT. Each IMR is 8 below its IER: ISR, IMR, IIR and IER at +0, +/// +4, +8 and +Ch. pub const PIPE_A_IMR: usize = 0x4_4404; pub const PIPE_A_IIR: usize = 0x4_4408; pub const PIPE_A_IER: usize = 0x4_440C; diff --git a/system/tgl-display/src/main.rs b/system/tgl-display/src/main.rs index 8a301a52e11..f43e8abe2f5 100644 --- a/system/tgl-display/src/main.rs +++ b/system/tgl-display/src/main.rs @@ -117,28 +117,57 @@ fn park() -> ! { const TOKEN_CLAIM: u64 = 0; const TOKEN_COMPOSITOR: u64 = 1; -/// The firmware's output offered on `display-output`, once: when the -/// connection ends, for whatever reason is said, presents stop and the -/// scan-out stays. A connection the compositor refused would only be refused -/// again. +/// How many times the driver connects again after a connection that was +/// answering presents ended with the compositor gone. Past it, presents stop +/// for the boot. +const RECONNECTS: u32 = 8; + +/// Why a connection ended. +enum Ended { + /// The compositor went away: it hung up, or took no answer. `answered`: + /// the connection had answered a present, so its hello was taken. + Gone { answered: bool, why: String }, + /// A frame out of protocol, or this driver could not answer: connecting + /// again would end the same way. + Refused(String), +} + +/// The firmware's output offered on `display-output`, again after a +/// compositor that was taking presents goes away, at most [`RECONNECTS`] +/// times. The connection is made at once whether or not the compositor +/// serves yet; the next present is the event that it does. Presents stop, +/// and say why, at a refusal, or a compositor gone before it took a hello; +/// the scan-out stays either way. fn serve(dev: &PciDev, regs: &Bar, done: &Programmed) { let size = Size { width: done.scanout.width as u32, height: done.scanout.height as u32 }; interrupt::claim(regs); - let conn = match endow::service(PORT) { - Ok(conn) => conn, - Err(why) => return say!("tgl-display: no `{PORT}` connection ({why:?}); presents go unanswered"), - }; - if let Err(why) = conn.send_bytes(MSG_HELLO, &Hello { size }.encode()) { - return say!("tgl-display: the compositor took no hello ({why:?}); presents go unanswered"); + let mut reconnects = 0; + loop { + let conn = match endow::service(PORT) { + Ok(conn) => conn, + Err(why) => break say!("tgl-display: no `{PORT}` connection ({why:?}); presents go unanswered"), + }; + if let Err(why) = conn.send_bytes(MSG_HELLO, &Hello { size }.encode()) { + break say!("tgl-display: the compositor took no hello ({why:?}); presents go unanswered"); + } + say!("tgl-display: offered the compositor its {}x{} output", size.width, size.height); + match answer(dev, regs, &conn, size, done.refresh_interval_nanos()) { + Ended::Gone { answered: true, why } if reconnects < RECONNECTS => { + reconnects += 1; + say!("tgl-display: {why}; connecting again ({reconnects} of {RECONNECTS})"); + } + Ended::Gone { answered: true, why } => { + break say!("tgl-display: presents go unanswered — {why}, after {RECONNECTS} connections again"); + } + Ended::Gone { answered: false, why } | Ended::Refused(why) => break say!("tgl-display: presents go unanswered — {why}"), + } } - say!("tgl-display: offered the compositor its {}x{} output", size.width, size.height); - let why = answer(dev, regs, &conn, size, done.refresh_interval_nanos()); - say!("tgl-display: presents go unanswered — {why}"); + interrupt::release(regs); } /// Every present on `conn` answered at the vblank after it arrived, until /// one cannot be: why. -fn answer(dev: &PciDev, regs: &Bar, conn: &Connection, size: Size, refresh_interval_nanos: u64) -> String { +fn answer(dev: &PciDev, regs: &Bar, conn: &Connection, size: Size, refresh_interval_nanos: u64) -> Ended { let poller = Poller::new(2); let mut rx = FrameRx::::new(); // The present waiting for its vblank, and when that vblank is overdue. @@ -156,7 +185,7 @@ fn answer(dev: &PciDev, regs: &Bar, conn: &Connection, size: Size, refresh_inter if let Some((sequence, _)) = outstanding.take() { let presented = Presented { sequence, vblank_nanos: record.last_nanos, refresh_interval_nanos }; if let Err(why) = conn.send_bytes(MSG_PRESENTED, &presented.encode()) { - return format!("the compositor took no answer ({why:?})"); + return Ended::Gone { answered, why: format!("the compositor took no answer ({why:?})") }; } if !answered { answered = true; @@ -165,21 +194,24 @@ fn answer(dev: &PciDev, regs: &Bar, conn: &Connection, size: Size, refresh_inter } } Ok(false) => {} - Err(why) => return why.to_string(), + Err(why) => return Ended::Refused(why.to_string()), }, Err(SyscallError::WouldBlock) => {} Err(why) => panic!("tgl-display: the claim's interrupt record: {why:?}"), } if outstanding.is_some_and(|(_, overdue)| toyos_abi::clock::nanos_since_boot() >= overdue) { - return format!("no vblank within {} ms of a present", FRAME_CEILING_NANOS / 1_000_000); + return Ended::Refused(format!("no vblank within {} ms of a present", FRAME_CEILING_NANOS / 1_000_000)); } loop { let (msg_type, payload_len) = match rx.pump(conn) { RxStep::Idle => break, - RxStep::Frame { msg_type, payload_len, .. } => (msg_type, payload_len), - RxStep::Eof => return format!("the compositor hung up `{PORT}`"), - RxStep::Malformed => return "the compositor sent a frame no protocol here can make".into(), + RxStep::Frame { msg_type, payload_len, handles: 0 } => (msg_type, payload_len), + RxStep::Frame { msg_type, .. } => { + return Ended::Refused(format!("the compositor sent frame {msg_type} with handles, which `{PORT}` has none of")); + } + RxStep::Eof => return Ended::Gone { answered, why: format!("the compositor hung up `{PORT}`") }, + RxStep::Malformed => return Ended::Refused("the compositor sent a frame no protocol here can make".into()), }; let present = Present::decode(msg_type, rx.payload(payload_len), size); match (present, outstanding) { @@ -188,9 +220,9 @@ fn answer(dev: &PciDev, regs: &Bar, conn: &Connection, size: Size, refresh_inter outstanding = Some((present.sequence, toyos_abi::clock::nanos_since_boot() + FRAME_CEILING_NANOS)); } (Some(present), Some((waiting, _))) => { - return format!("the compositor sent present {} while {waiting} waits", present.sequence); + return Ended::Refused(format!("the compositor sent present {} while {waiting} waits", present.sequence)); } - (None, _) => return format!("the compositor sent frame {msg_type}, which `{PORT}` has not"), + (None, _) => return Ended::Refused(format!("the compositor sent frame {msg_type}, which `{PORT}` has not")), } } } diff --git a/tests/checks/audio.rs b/tests/checks/audio.rs index c90e715dcdc..e73c9a576cd 100644 --- a/tests/checks/audio.rs +++ b/tests/checks/audio.rs @@ -48,6 +48,18 @@ pub fn judges_verdict() -> Result<(), String> { &format!("{}{configured}{next}", tone(stats(0, 0)).replace(configured, "")), false, )?; + // A job that waits after its spawn: the output offered after the spawn and + // before its stream connects is the one it played to; offered after the + // stream connected, it played to the null sink. + let waiting = |offered_between: bool| { + let spawned = spawn("test_rs_audio_tone"); + match offered_between { + true => format!("{spawned}{configured}{}{next}", session(stats(0, 1), stats(0, 0))), + false => format!("{spawned}{}{configured}{next}", session(stats(0, 1), stats(0, 0))), + } + }; + judged("a tone whose output was offered after its spawn and before its stream", tone_on_metal, &waiting(true), true)?; + judged("a tone whose stream connected before the output was offered", tone_on_metal, &waiting(false), false)?; // The next job's stream connected before soundserver let the tone's go, so the // tone's last window is both streams' whatever it counted. judged( From e884ee2f6ed1027015c8efed3fead284098838e1 Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 1 Nov 2026 16:45:28 +0100 Subject: [PATCH 6/6] review r2 of #1134: a connection is live once a present arrives on it The compositor sends a present only once it has taken the hello, so a present arriving proves the connection was served. The driver now connects again after any such connection ends from the compositor's side, so a compositor swapped while the desktop was idle, or before its first present reached a vblank, is reconnected to. Before, the flag was set only when a present was answered. The bound of 8 still contains a loop. `Ended::Gone`'s doc says what EOF can mean: the compositor going away, or it dropping the driver for a fault of the driver's own. `serve`'s doc and the track say exactly what the code does. Co-Authored-By: Claude Opus 5.5 --- ...ves-its-displays-from-a-userland-driver.md | 2 +- system/tgl-display/src/main.rs | 41 +++++++++++-------- 2 files changed, 24 insertions(+), 19 deletions(-) diff --git a/issues/each-gpu-drives-its-displays-from-a-userland-driver.md b/issues/each-gpu-drives-its-displays-from-a-userland-driver.md index 60994953ca5..cf0afc7b2bd 100644 --- a/issues/each-gpu-drives-its-displays-from-a-userland-driver.md +++ b/issues/each-gpu-drives-its-displays-from-a-userland-driver.md @@ -89,7 +89,7 @@ The kernel keeps only generic mechanisms: - One present is outstanding at a time. Damage composed meanwhile is merged, past 8 rectangles into their bounding box, and sent when the answer arrives. - The compositor serves `display-output`, one driver at a time, and never waits on it. - `tgl-display` answers each present at the vblank after it arrived. That vblank is pipe A's interrupt, enabled only while a present waits, and its wait has a hang ceiling. - - When a compositor that was taking presents goes away (a restart, a `swap`), the driver connects again, at most 8 times a boot. A protocol refusal, or a compositor gone before it took a hello, ends presents for the boot, by name. + - When a connection a present arrived on ends from the compositor's side (a restart, a `swap`, or the compositor dropping the driver), the driver connects again, at most 8 times a boot. A protocol refusal, or a connection that ended before any present arrived on it, ends presents for the boot, by name. - The compositor does not yet pace frames by `Presented`. It still blits the firmware's framebuffer itself until stage 5. 4. **PSR on the eDP panel**, measured together with stage 2. PSR's first-frame exit latency goes to the owner in absolute numbers before PSR lands. 5. **The rest of `display-output`.** virtio-gpu and the framebuffer become drivers. The panic surface lands here. diff --git a/system/tgl-display/src/main.rs b/system/tgl-display/src/main.rs index f43e8abe2f5..ebef820a03e 100644 --- a/system/tgl-display/src/main.rs +++ b/system/tgl-display/src/main.rs @@ -117,27 +117,30 @@ fn park() -> ! { const TOKEN_CLAIM: u64 = 0; const TOKEN_COMPOSITOR: u64 = 1; -/// How many times the driver connects again after a connection that was -/// answering presents ended with the compositor gone. Past it, presents stop -/// for the boot. +/// How many times the driver connects again after a connection a present +/// arrived on ended with the compositor gone. Past it, presents stop for the +/// boot. const RECONNECTS: u32 = 8; /// Why a connection ended. enum Ended { - /// The compositor went away: it hung up, or took no answer. `answered`: - /// the connection had answered a present, so its hello was taken. - Gone { answered: bool, why: String }, + /// The connection ended from the compositor's side: it hung up, or took + /// no answer. That is the compositor going away, or it dropping this + /// driver for a fault of the driver's own, which [`RECONNECTS`] bounds. + /// `presented`: a present arrived on it, which the compositor sends only + /// once it has taken the hello. + Gone { presented: bool, why: String }, /// A frame out of protocol, or this driver could not answer: connecting /// again would end the same way. Refused(String), } /// The firmware's output offered on `display-output`, again after a -/// compositor that was taking presents goes away, at most [`RECONNECTS`] -/// times. The connection is made at once whether or not the compositor -/// serves yet; the next present is the event that it does. Presents stop, -/// and say why, at a refusal, or a compositor gone before it took a hello; -/// the scan-out stays either way. +/// connection a present arrived on ends from the compositor's side, at most +/// [`RECONNECTS`] times. The connection is made at once whether or not the +/// compositor serves yet; the next present is the event that it does. +/// Presents stop, and say why, at a refusal, or a connection that ended +/// before any present arrived on it; the scan-out stays either way. fn serve(dev: &PciDev, regs: &Bar, done: &Programmed) { let size = Size { width: done.scanout.width as u32, height: done.scanout.height as u32 }; interrupt::claim(regs); @@ -152,14 +155,14 @@ fn serve(dev: &PciDev, regs: &Bar, done: &Programmed) { } say!("tgl-display: offered the compositor its {}x{} output", size.width, size.height); match answer(dev, regs, &conn, size, done.refresh_interval_nanos()) { - Ended::Gone { answered: true, why } if reconnects < RECONNECTS => { + Ended::Gone { presented: true, why } if reconnects < RECONNECTS => { reconnects += 1; say!("tgl-display: {why}; connecting again ({reconnects} of {RECONNECTS})"); } - Ended::Gone { answered: true, why } => { + Ended::Gone { presented: true, why } => { break say!("tgl-display: presents go unanswered — {why}, after {RECONNECTS} connections again"); } - Ended::Gone { answered: false, why } | Ended::Refused(why) => break say!("tgl-display: presents go unanswered — {why}"), + Ended::Gone { presented: false, why } | Ended::Refused(why) => break say!("tgl-display: presents go unanswered — {why}"), } } interrupt::release(regs); @@ -173,6 +176,7 @@ fn answer(dev: &PciDev, regs: &Bar, conn: &Connection, size: Size, refresh_inter // The present waiting for its vblank, and when that vblank is overdue. let mut outstanding: Option<(u64, u64)> = None; let mut answered = false; + let mut presented = false; loop { poller.watch(dev, READABLE, TOKEN_CLAIM); poller.watch(conn, READABLE, TOKEN_COMPOSITOR); @@ -183,9 +187,9 @@ fn answer(dev: &PciDev, regs: &Bar, conn: &Connection, size: Size, refresh_inter Ok(record) => match interrupt::serve(regs) { Ok(true) => { if let Some((sequence, _)) = outstanding.take() { - let presented = Presented { sequence, vblank_nanos: record.last_nanos, refresh_interval_nanos }; - if let Err(why) = conn.send_bytes(MSG_PRESENTED, &presented.encode()) { - return Ended::Gone { answered, why: format!("the compositor took no answer ({why:?})") }; + let answer = Presented { sequence, vblank_nanos: record.last_nanos, refresh_interval_nanos }; + if let Err(why) = conn.send_bytes(MSG_PRESENTED, &answer.encode()) { + return Ended::Gone { presented, why: format!("the compositor took no answer ({why:?})") }; } if !answered { answered = true; @@ -210,12 +214,13 @@ fn answer(dev: &PciDev, regs: &Bar, conn: &Connection, size: Size, refresh_inter RxStep::Frame { msg_type, .. } => { return Ended::Refused(format!("the compositor sent frame {msg_type} with handles, which `{PORT}` has none of")); } - RxStep::Eof => return Ended::Gone { answered, why: format!("the compositor hung up `{PORT}`") }, + RxStep::Eof => return Ended::Gone { presented, why: format!("the compositor hung up `{PORT}`") }, RxStep::Malformed => return Ended::Refused("the compositor sent a frame no protocol here can make".into()), }; let present = Present::decode(msg_type, rx.payload(payload_len), size); match (present, outstanding) { (Some(present), None) => { + presented = true; interrupt::arm(regs); outstanding = Some((present.sequence, toyos_abi::clock::nanos_since_boot() + FRAME_CEILING_NANOS)); }