From aa7767b5ff646b50a76bf0075e79ebc6f3b11f69 Mon Sep 17 00:00:00 2001 From: japabu Date: Tue, 20 Oct 2026 19:38:13 +0200 Subject: [PATCH 1/3] Audio leaves soundserver for an hda driver per controller; the matcher takes any prog-if #950 left three defects on main, each measured: - GitHub's guest job red `hda_drives_its_codec_through_its_claim`: soundserver configured the path (`volume_step` exited 0 with the codec's readbacks) but said so between the ready marker and the job's window, which neither capture piece holds. The row now awaits the line on the console instead of reading the boot log. - The T14 matched nothing: its HDA function is `00:1f.3 [0403] 8086:a0c8 prog_if=80` (the readback's enumeration line), and `pci:04:03:00` named prog-if 00 only. `lib/manifest`'s grammar gains `pci:CC:SS:*`, every programming interface of a subclass; HD Audio's 00 and 80 are one register model. - The AMD bench's two HDA functions made the single-process row's start refused. By the lifecycle ruling (stage 3 of issues/a-device-function-is-driven-from-its-arrival-to-its-end.md), audio's driver leaves the shared service: `hda` (system/hda) is a `drives` row on `pci:04:03:*`, one process per controller. Each resets its controller, walks its own codecs and finds its output (`toyos-hda`, moved to system/hda/driver; `choose` over several controllers is gone, each driver decides for its own), and a driver that reaches none ends by name. One that does offers it on soundserver's new `sound-output` port (`toyos-sound-output`, every frame decoded and refused there): soundserver lends it the ring of periods, which the driver maps through its claim and points its descriptor list at, and the stream connection carries start, stop and the periods played with the claim's interrupt times. The master's requests go to the driver and its codec readback comes back to the volume port. soundserver is the null sink until an output is offered and plays to it once no stream is open, so a stream never changes device under it; virtio-sound stays driven inside soundserver (stage 3's next item). The tone jobs' master now fails by name on a soundserver with no master rather than on a message number. QEMU: `hda_plays_to_the_controller_that_reaches_an_output` boots two ich9-intel-hda, the first with no codec: its driver plays nothing, soundserver plays to the second's, and `volume_step` steps that codec. Records: the lifecycle track's stage 3, the drivers track's HDA item, the jack-detection track's paths; filed cpal's close bound (issues/cpals-close-bound-is-one-ring-of-device-time.md) and the host-CI red that is main's too (issues/the-llvm-environment-test-dies-where-git-is-the-xcode-shim.md). Co-Authored-By: Claude Opus 5.5 --- Cargo.lock | 23 +- Cargo.toml | 4 +- images/system.toml | 13 +- ...n-is-driven-from-its-arrival-to-its-end.md | 13 + ...-close-bound-is-one-ring-of-device-time.md | 28 ++ issues/every-driver-is-still-in-the-kernel.md | 10 +- ...-no-jack-detection-and-nothing-persists.md | 12 +- ...t-test-dies-where-git-is-the-xcode-shim.md | 23 + lib/manifest/src/device.rs | 73 ++- lib/manifest/src/lib.rs | 2 +- system/hda/Cargo.toml | 16 + .../hda => hda/driver}/Cargo.toml | 14 +- .../hda => hda/driver}/fixtures/amd-bench.txt | 0 .../hda => hda/driver}/fixtures/laptop.txt | 0 .../driver}/fixtures/qemu-intel-hda.txt | 0 .../hda => hda/driver}/src/caps.rs | 0 .../hda => hda/driver}/src/config.rs | 8 +- .../hda => hda/driver}/src/controller.rs | 46 +- .../driver}/src/controller/tests.rs | 26 +- .../hda => hda/driver}/src/fixture.rs | 0 .../hda => hda/driver}/src/graph.rs | 0 .../hda => hda/driver}/src/lib.rs | 8 +- .../hda => hda/driver}/src/path.rs | 74 +-- .../hda => hda/driver}/src/probe.rs | 0 .../hda => hda/driver}/src/stream.rs | 68 +-- .../hda => hda/driver}/src/verb.rs | 0 system/hda/src/main.rs | 452 ++++++++++++++++++ system/soundserver/Cargo.toml | 6 +- system/soundserver/output/Cargo.toml | 22 + system/soundserver/output/src/lib.rs | 364 ++++++++++++++ system/soundserver/src/backend.rs | 52 +- system/soundserver/src/command.rs | 12 +- system/soundserver/src/control.rs | 54 ++- system/soundserver/src/hda.rs | 365 -------------- system/soundserver/src/inspect.rs | 5 +- system/soundserver/src/main.rs | 294 ++++-------- system/soundserver/src/mix.rs | 48 +- system/soundserver/src/output.rs | 394 +++++++++++++++ system/soundserver/src/volume.rs | 53 +- tests/checks/audio.rs | 8 +- tests/common/audio.rs | 29 +- tests/common/qemu.rs | 19 +- tests/guest/rust-tests/src/master.rs | 6 +- tests/images/metalcase.toml | 13 +- tests/images/metaldevicecase.toml | 18 +- tests/images/netcase.toml | 2 +- tests/images/testcases.toml | 13 +- tests/toyos.rs | 79 +-- 48 files changed, 1863 insertions(+), 906 deletions(-) create mode 100644 issues/cpals-close-bound-is-one-ring-of-device-time.md create mode 100644 issues/the-llvm-environment-test-dies-where-git-is-the-xcode-shim.md create mode 100644 system/hda/Cargo.toml rename system/{soundserver/hda => hda/driver}/Cargo.toml (57%) rename system/{soundserver/hda => hda/driver}/fixtures/amd-bench.txt (100%) rename system/{soundserver/hda => hda/driver}/fixtures/laptop.txt (100%) rename system/{soundserver/hda => hda/driver}/fixtures/qemu-intel-hda.txt (100%) rename system/{soundserver/hda => hda/driver}/src/caps.rs (100%) rename system/{soundserver/hda => hda/driver}/src/config.rs (99%) rename system/{soundserver/hda => hda/driver}/src/controller.rs (94%) rename system/{soundserver/hda => hda/driver}/src/controller/tests.rs (95%) rename system/{soundserver/hda => hda/driver}/src/fixture.rs (100%) rename system/{soundserver/hda => hda/driver}/src/graph.rs (100%) rename system/{soundserver/hda => hda/driver}/src/lib.rs (77%) rename system/{soundserver/hda => hda/driver}/src/path.rs (88%) rename system/{soundserver/hda => hda/driver}/src/probe.rs (100%) rename system/{soundserver/hda => hda/driver}/src/stream.rs (82%) rename system/{soundserver/hda => hda/driver}/src/verb.rs (100%) create mode 100644 system/hda/src/main.rs create mode 100644 system/soundserver/output/Cargo.toml create mode 100644 system/soundserver/output/src/lib.rs delete mode 100644 system/soundserver/src/hda.rs create mode 100644 system/soundserver/src/output.rs diff --git a/Cargo.lock b/Cargo.lock index 46aa0d0e030..5d914d430af 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1609,6 +1609,18 @@ version = "0.16.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "841d1cc9bed7f9236f321df977030373f4a4163ae1a7dbfe1a51a2c1a51d9100" +[[package]] +name = "hda" +version = "0.0.0" +dependencies = [ + "toyos", + "toyos-abi", + "toyos-hda", + "toyos-manifest", + "toyos-pci-claim", + "toyos-sound-output", +] + [[package]] name = "heck" version = "0.5.0" @@ -3921,12 +3933,12 @@ dependencies = [ "rubato", "toyos", "toyos-abi", - "toyos-hda", "toyos-inspect", "toyos-manifest", "toyos-mixer", "toyos-pci-claim", "toyos-resample", + "toyos-sound-output", "toyos-virtio", "toyos-virtio-sound", ] @@ -4526,7 +4538,7 @@ name = "toyos-hda" version = "0.1.0" dependencies = [ "toyos-device-memory", - "toyos-mixer", + "toyos-sound-output", ] [[package]] @@ -4776,6 +4788,13 @@ dependencies = [ "toyos-phys", ] +[[package]] +name = "toyos-sound-output" +version = "0.1.0" +dependencies = [ + "toyos-mixer", +] + [[package]] name = "toyos-ssh" version = "0.1.0" diff --git a/Cargo.toml b/Cargo.toml index b354ac7fa4b..6e769c19d2a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -119,10 +119,12 @@ members = [ "system/pkg", "system/ps2server", "system/ps2server/ps2", + "system/hda", + "system/hda/driver", "system/shell", "system/soundserver", - "system/soundserver/hda", "system/soundserver/mixer", + "system/soundserver/output", "system/soundserver/resample", "system/soundserver/virtio-sound", "system/sshserver", diff --git a/images/system.toml b/images/system.toml index 2ac7e2a1966..6b91c9fa582 100644 --- a/images/system.toml +++ b/images/system.toml @@ -100,8 +100,17 @@ devices = ["isa:0060,0064:1,12"] [programs.soundserver] service = true -serves = ["soundserver", "volume"] -devices = ["pci:04:03:00", "pci:04:01:00/1af4:1059"] +serves = ["soundserver", "volume", "sound-output"] +devices = ["pci:04:01:00/1af4:1059"] +syscap = ["rt"] + +# The Intel HD Audio driver: one process per HDA function the kernel does not +# drive, each walking its own controller's codecs and offering soundserver the +# output it reaches. A machine with display audio beside an analog codec runs +# two, and soundserver plays to the one that offers. +[programs.hda] +drives = ["pci:04:03:*"] +receives = ["sound-output"] syscap = ["rt"] # netstack holds the NIC's PCI function and drives it: the virtqueues, the register diff --git a/issues/a-device-function-is-driven-from-its-arrival-to-its-end.md b/issues/a-device-function-is-driven-from-its-arrival-to-its-end.md index 945b289d5b9..14d153dbbcc 100644 --- a/issues/a-device-function-is-driven-from-its-arrival-to-its-end.md +++ b/issues/a-device-function-is-driven-from-its-arrival-to-its-end.md @@ -104,6 +104,19 @@ started. Nothing reaches userland when a device arrives or leaves. 3. **Drivers leave the shared services.** netstack, soundserver and diskserver each become a driver per function plus the service its clients use. The client port carries a per-device source and `gone`. + - **Done for HDA.** `hda` is a `drives` row on `pci:04:03:*`, one process + per controller; each walks its own codecs and offers soundserver the + output it reaches on `sound-output` (`toyos-sound-output`), and one that + reaches none ends by name. soundserver plays to the first offer it can + mix into, beginning when no stream is open; it is the null sink until + then, and refuses every later offer. A driver gone after soundserver + began playing to it ends soundserver. QEMU's two-controller row is + `hda_plays_to_the_controller_that_reaches_an_output`. + - Next: virtio-sound is still driven inside soundserver + (`system/soundserver/src/virtio.rs`); it becomes a driver on the same + port. Then the output set: a second output offered while one plays, a + driver's `gone`, and a stream moved to another output, none of which + soundserver serves yet; and netstack and diskserver. 4. **usbd drives every xHCI**, one per controller, started by stage 1's `drives` rows. This is step 10 of `issues/the-kernel-is-small-interrupts-post-and-threads-wait.md`, whose diff --git a/issues/cpals-close-bound-is-one-ring-of-device-time.md b/issues/cpals-close-bound-is-one-ring-of-device-time.md new file mode 100644 index 00000000000..5bb8465334d --- /dev/null +++ b/issues/cpals-close-bound-is-one-ring-of-device-time.md @@ -0,0 +1,28 @@ +--- +status: open +kind: defect +opened: 2026-10-20 +--- + +# cpal's close is bounded by one ring of device time, a verdict rather than a hang ceiling + +cpal's ToyOS host (`src/host/toyos/mod.rs` in the cpal fork, branch +`toyos-0.18.0`, `Stream`'s `Drop`) waits for its stream thread to end, which +is the event soundserver's release of the closed stream causes, for at most +`release_within`: the stream's fade and one ring of periods at the device's +rate, about 29 ms at 44.1 kHz. A release that takes longer is reported through +the error callback as `soundserver did not let go of the closed stream within +…`, and `tests/guest/rust-tests/src/tone.rs` fails the job on it. + +That bound is the device's playout time, not a hang ceiling. On QEMU 11.1.1's +`ich9-intel-hda` under TCG, periods complete in bursts of up to a ring paced by +the host (`max_batch=7`), and the bound failed `test_rs_audio_tone` and +`test_rs_audio_tone_resampled` at host load averages 22 and 101 with every +period completed and `underruns=0` (#950's evidence, +https://github.com/ToyOSOrg/evidence/tree/b0d5671935257fff8fff68c3b7d9d788ee718292/hda-claim/qemu). A loaded metal machine can be late the same +way. So the QEMU HDA row runs the master job only, and the tone jobs are +judged on the benches alone. + +Owner: the cpal fork. Exit: the close waits for the stream thread's end with a +failure timeout sized as a hang ceiling, and a release that is merely slow is +not an error; the QEMU HDA row then runs `test_rs_audio_tone` too. diff --git a/issues/every-driver-is-still-in-the-kernel.md b/issues/every-driver-is-still-in-the-kernel.md index eed6e64d1a2..957cf92fff4 100644 --- a/issues/every-driver-is-still-in-the-kernel.md +++ b/issues/every-driver-is-still-in-the-kernel.md @@ -32,10 +32,12 @@ is not built. What is left of the staged work: -1. Done for HDA: soundserver claims the function as `pci:04:03:00` and - drives it whole (`system/soundserver/hda/src/controller.rs`) — controller - reset, CORB/RIRB, the stream and its descriptor list in the claim's grant, - its interrupt as the claim's record. The `hda-audio` class, +1. Done for HDA: an `hda` process per function of `pci:04:03:*` (a + `drives` row, `system/hda`) drives it whole — controller reset, CORB/RIRB, + the stream and its descriptor list, the ring soundserver lends it, its + interrupt as the claim's record — and offers soundserver its output + (`issues/a-device-function-is-driven-from-its-arrival-to-its-end.md`'s + stage 3). The `hda-audio` class, `SYS_HDA_BIND`, `SYS_DEVICE_REG_WRITE` and the kernel's HDA vector are gone; `SYS_DEVICE_REG_READ` reads a PCI claim's config space alone. Every machine's DLL is clocked by a claim's record, and a period read before its diff --git a/issues/hda-has-no-jack-detection-and-nothing-persists.md b/issues/hda-has-no-jack-detection-and-nothing-persists.md index e8d0042e29f..6697b07e993 100644 --- a/issues/hda-has-no-jack-detection-and-nothing-persists.md +++ b/issues/hda-has-no-jack-detection-and-nothing-persists.md @@ -6,14 +6,14 @@ opened: 2026-08-03 # HDA plays, but there is no jack detection and nothing persists -The driver is soundserver's, whole, through a `pci` claim: the controller, -codec decoding, output-path selection and the master are a pure crate -(`system/soundserver/hda`). The master volume and mute are the codec's, served on -soundserver's `volume` port, and the volume keys set them from the compositor +The driver is the `hda` program's, one per function, through a `pci` claim: +the controller, codec decoding, output-path selection and the master are a +pure crate (`system/hda/driver`). The master volume and mute are the codec's, +set by the driver for soundserver's `volume` port, and the volume keys set them from the compositor (#877). What remains: - **Jack detection and routing.** `GET_PIN_SENSE` is spelled - (`system/soundserver/hda/src/verb.rs`) and nothing issues it. Still wanted: a poll on the + (`system/hda/driver/src/verb.rs`) and nothing issues it. Still wanted: a poll on the presence bit, a switch between the fixed speaker and the headphone pin, and a ramp across the switch so it is not a click. - **Persistence** of the chosen output and the master level, blocked on the @@ -27,7 +27,7 @@ controller with no codec is refused by name in the controller's host tests). Wit asserted only on a machine that has no controller at all. Constraints worth not re-deriving (each already stated at its site in -`system/soundserver/hda/`, listed here because they are what +`system/hda/driver/`, listed here because they are what the next pass will want): - **The T14's trap is pin 0x1b, not the display codec.** Four pins claim diff --git a/issues/the-llvm-environment-test-dies-where-git-is-the-xcode-shim.md b/issues/the-llvm-environment-test-dies-where-git-is-the-xcode-shim.md new file mode 100644 index 00000000000..a2a91df7cc3 --- /dev/null +++ b/issues/the-llvm-environment-test-dies-where-git-is-the-xcode-shim.md @@ -0,0 +1,23 @@ +--- +status: open +kind: tooling +opened: 2026-10-20 +--- + +# The LLVM environment test dies where `git` is Xcode's shim + +`llvm::tests::the_caller_s_environment_reaches_neither_the_build_nor_the_key` +(`build/llvm.rs`) reruns `llvm::tests::keyed_and_built` with `SDKROOT` set to +`/no/such.sdk`, among the other ambient names it proves never reach the LLVM +build. On a macOS host whose `/usr/bin/git` is Xcode's `xcrun` shim with no +standalone command line tools, the child's first `git` (`sysroot.rs`, the +fork's `.gitmodules` read) resolves through that `SDKROOT` and fails: `Failed +to determine realpath of '/no/such.sdk'`, `xcode-select: Failed to locate +'git'`. So `cargo run -- --ci host` reds its build-system step on such a host, +on `origin/main` at `0e6716fc0` as on any branch. + +The test asserts what the *LLVM build* sees; the harness's own `git` reading +the caller's `SDKROOT` is the test's premise breaking, not the defect it +guards. Owner: the build track. Exit: the rerun's own `git` calls run without +the ambient names (or `git` is resolved before they are set), and the test +passes on a host whose `git` is the shim. diff --git a/lib/manifest/src/device.rs b/lib/manifest/src/device.rs index 6fecd26fcea..c4eb647e4c0 100644 --- a/lib/manifest/src/device.rs +++ b/lib/manifest/src/device.rs @@ -3,7 +3,8 @@ //! A class, a partition or an ISA set is the ABI's own spelling //! ([`DeviceRequest::parse`]). A PCI function is matched here, by its class //! code and optionally its card: `pci:CC:SS:PP` or `pci:CC:SS:PP/VVVV:DDDD`, -//! lowercase hex. **Every matcher carries its class**, so whether two entries +//! lowercase hex, where `PP` may be `*` for every programming interface of the +//! subclass. **Every matcher carries its class**, so whether two entries //! can name one function is decided from their text alone ([`Entry::overlaps`]), //! and a claim names the function a matcher found by its arrival. @@ -33,27 +34,45 @@ impl Entry { pub fn overlaps(&self, other: &Self) -> bool { match (self, other) { (Self::Fixed(a), Self::Fixed(b)) => a == b, - (Self::Pci(a), Self::Pci(b)) => a.class == b.class && (a.id.is_none() || b.id.is_none() || a.id == b.id), + (Self::Pci(a), Self::Pci(b)) => { + a.class == b.class && either(a.prog_if, b.prog_if) && either(a.id, b.id) + } _ => false, } } } +/// Whether two optional fields can hold one value: either is unset, or both +/// are the same. +fn either(x: Option, y: Option) -> bool { + x.is_none() || y.is_none() || x == y +} + /// Which PCI functions an entry names: every function of a class code, or of /// one card within it. #[derive(Clone, Copy, PartialEq, Eq, Debug)] pub struct PciMatch { - /// Base class, subclass and programming interface. - pub class: [u8; 3], + /// Base class and subclass. + pub class: [u8; 2], + /// The programming interface, or `None` for every one: a subclass whose + /// interfaces are one register model told apart by vendor extensions (HD + /// Audio's 00 and 80) is driven alike whichever a function reports. + pub prog_if: Option, pub id: Option, } impl PciMatch { pub const PREFIX: &'static str = "pci:"; - /// Every function of class `class`, of card `id` where it names one. + /// Every function of class code `class`, of card `id` where it names one. pub const fn new(class: [u8; 3], id: Option) -> Self { - Self { class, id } + Self { class: [class[0], class[1]], prog_if: Some(class[2]), id } + } + + /// Every function of `class`'s base class and subclass, whatever its + /// programming interface. + pub const fn any_interface(class: [u8; 2], id: Option) -> Self { + Self { class, prog_if: None, id } } pub fn parse(text: &str) -> Option { @@ -65,27 +84,33 @@ impl PciMatch { } None => (rest, None), }; - let mut class = [0u8; 3]; let mut fields = code.split(':'); - for byte in &mut class { - *byte = hex8(fields.next()?)?; - } + let class = [hex8(fields.next()?)?, hex8(fields.next()?)?]; + let prog_if = match fields.next()? { + "*" => None, + byte => Some(hex8(byte)?), + }; if fields.next().is_some() { return None; } - Some(Self { class, id: card }) + Some(Self { class, prog_if, id: card }) } pub fn matches(&self, pci: &Pci) -> bool { - [pci.class, pci.subclass, pci.prog_if] == self.class + [pci.class, pci.subclass] == self.class + && self.prog_if.is_none_or(|prog_if| prog_if == pci.prog_if) && self.id.is_none_or(|id| id.vendor == pci.vendor && id.device == pci.device) } } impl fmt::Display for PciMatch { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let [class, subclass, prog_if] = self.class; - write!(f, "{}{class:02x}:{subclass:02x}:{prog_if:02x}", Self::PREFIX)?; + let [class, subclass] = self.class; + write!(f, "{}{class:02x}:{subclass:02x}:", Self::PREFIX)?; + match self.prog_if { + Some(prog_if) => write!(f, "{prog_if:02x}")?, + None => f.write_str("*")?, + } match self.id { Some(id) => write!(f, "/{:04x}:{:04x}", id.vendor, id.device), None => Ok(()), @@ -135,7 +160,9 @@ mod tests { #[test] fn a_matcher_writes_back_the_text_it_parsed() { - for text in ["pci:01:08:02/1b36:0010", "pci:0c:03:30", "pci:ff:00:00/1234:11e8", "pci:00:00:00"] { + for text in + ["pci:01:08:02/1b36:0010", "pci:0c:03:30", "pci:ff:00:00/1234:11e8", "pci:00:00:00", "pci:04:03:*", "pci:04:03:*/8086:a0c8"] + { let parsed = PciMatch::parse(text).unwrap_or_else(|| panic!("{text} did not parse")); assert_eq!(parsed.to_string(), text); assert_eq!(Entry::parse(text), Some(Entry::Pci(parsed))); @@ -151,7 +178,7 @@ mod tests { "pci:1b36:0010", // a card without its class "pci:01:08", "pci:01:08:02:00", "pci:1:08:02", "pci:01:08:0A", "pci:01:08:02/", "pci:01:08:02/1B36:0010", "pci:01:08:02/1b36", "pci:01:08:02/1b36:0010/x", "pci:+1:08:02", - "pci:", "pci", + "pci:", "pci", "pci:04:03:**", "pci:04:*:*", "pci:04:03:*:00", "pci:04:03:", // The card is four lowercase hex digits each and nothing else: two // spellings would be two labels for one claim. "pci:02:00:00/1Af4:1041", "pci:02:00:00/1af4:41", "pci:02:00:00/1af4", "pci:02:00:00/1af4:1041:", @@ -178,6 +205,17 @@ mod tests { assert!(any_nvme.matches(&function([0x01, 0x08, 0x02], 0x144d, 0xa808))); } + /// The T14's HD Audio controller reports programming interface 0x80 and + /// QEMU's 0x00: one matcher names both, and nothing outside the subclass. + #[test] + fn a_wildcard_interface_matches_every_interface_of_its_subclass() { + let hda = PciMatch::parse("pci:04:03:*").unwrap(); + assert!(hda.matches(&function([0x04, 0x03, 0x80], 0x8086, 0xa0c8))); + assert!(hda.matches(&function([0x04, 0x03, 0x00], 0x8086, 0x293e))); + assert!(!hda.matches(&function([0x04, 0x01, 0x00], 0x1af4, 0x1059))); + assert!(!PciMatch::parse("pci:04:03:00").unwrap().matches(&function([0x04, 0x03, 0x80], 0x8086, 0xa0c8))); + } + /// Whether two entries can name one device, decided from their text: the /// build refuses a manifest where they can. #[test] @@ -188,6 +226,8 @@ mod tests { ("pci:01:08:02", "pci:01:08:02/1b36:0010"), ("pci:01:08:02/1b36:0010", "pci:01:08:02"), ("pci:0c:03:30", "pci:0c:03:30"), + ("pci:04:03:*", "pci:04:03:80"), + ("pci:04:03:00/8086:293e", "pci:04:03:*"), ("framebuffer", "framebuffer"), ("isa:0060,0064:1,12", "isa:0060,0064:1,12"), ("part:0FC63DAF-8483-4772-8E79-3D69D8477DE4", "part:0FC63DAF-8483-4772-8E79-3D69D8477DE4"), @@ -198,6 +238,7 @@ mod tests { let apart = [ ("pci:01:08:02/1b36:0010", "pci:01:08:02/144d:a808"), ("pci:01:08:02", "pci:01:08:01"), + ("pci:04:03:*", "pci:04:01:00"), ("pci:01:08:02/1b36:0010", "pci:01:08:01/1b36:0010"), ("framebuffer", "keyboard"), ("pci:0c:03:30", "keyboard"), diff --git a/lib/manifest/src/lib.rs b/lib/manifest/src/lib.rs index 5d93d5da70f..3055f855a3e 100644 --- a/lib/manifest/src/lib.rs +++ b/lib/manifest/src/lib.rs @@ -634,7 +634,7 @@ mod tests { name: "soundserver".into(), path: "/system/bin/soundserver".into(), serves: vec!["soundserver".into()], - devices: vec!["pci:04:03:00".into(), "pci:04:01:00/1af4:1059".into()], + devices: vec!["pci:04:01:00/1af4:1059".into()], syscap: vec!["rt".into()], service: true, ..Program::default() diff --git a/system/hda/Cargo.toml b/system/hda/Cargo.toml new file mode 100644 index 00000000000..180d6b8a5ba --- /dev/null +++ b/system/hda/Cargo.toml @@ -0,0 +1,16 @@ +[package] +name = "hda" +description = "The Intel HD Audio driver: one process per HDA function the kernel does not drive, holding its claim, walking its codecs and offering soundserver the output it reaches." +edition = "2024" +license = "MIT OR Apache-2.0" + +[dependencies] +toyos-abi = { path = "../../sdk/abi" } +toyos = { path = "../../sdk/toyos" } +toyos-hda = { path = "driver" } +toyos-manifest = { path = "../../lib/manifest" } +toyos-pci-claim = { path = "../../lib/pci-claim" } +toyos-sound-output = { path = "../soundserver/output" } + +[package.metadata.toyos.host] +exempt.owns = "one Intel HD Audio function, claimed from the kernel, and its output offered on soundserver's sound-output port" diff --git a/system/soundserver/hda/Cargo.toml b/system/hda/driver/Cargo.toml similarity index 57% rename from system/soundserver/hda/Cargo.toml rename to system/hda/driver/Cargo.toml index 7409fbf04cc..f58473faf00 100644 --- a/system/soundserver/hda/Cargo.toml +++ b/system/hda/driver/Cargo.toml @@ -1,6 +1,6 @@ -# A member of the host workspace (root `Cargo.toml`), like `toyos-virtio-sound` -# beside it: pure, and its tests run on the host. A codec's answers and every -# word the controller writes back are input from outside the driver, so the +# A member of the host workspace (root `Cargo.toml`), like `toyos-virtio-sound`: +# pure, and its tests run on the host. A codec's answers and every word the +# controller writes back are input from outside the driver, so the # decoders and the controller's rings are exercised against hostile ones far # more often than a boot allows — and the machines that matter cannot be booted # by a test at all, so their graphs are committed fixtures and the controller is @@ -8,7 +8,7 @@ [package] name = "toyos-hda" -description = "soundserver's Intel HDA driver: the controller's reset, CORB/RIRB and one output stream over one grant, codec decoding and output-path selection, pure." +description = "The hda program's Intel HDA driver: the controller's reset, CORB/RIRB and one output stream, codec decoding and output-path selection, pure." version = "0.1.0" edition = "2021" license = "MIT OR Apache-2.0" @@ -23,9 +23,9 @@ doctest = false toyos-device-memory = { path = "../../../lib/device-memory" } [dev-dependencies] -# The device rates soundserver declares, best first: the rate choice is tested -# against the declaration it runs on. -toyos-mixer = { path = "../mixer" } +# The rates soundserver mixes at, best first, which the rate choice is tested +# against, and the reader's decoding of the masks this crate's stream reports. +toyos-sound-output = { path = "../../soundserver/output" } [lints.rust] warnings = "deny" diff --git a/system/soundserver/hda/fixtures/amd-bench.txt b/system/hda/driver/fixtures/amd-bench.txt similarity index 100% rename from system/soundserver/hda/fixtures/amd-bench.txt rename to system/hda/driver/fixtures/amd-bench.txt diff --git a/system/soundserver/hda/fixtures/laptop.txt b/system/hda/driver/fixtures/laptop.txt similarity index 100% rename from system/soundserver/hda/fixtures/laptop.txt rename to system/hda/driver/fixtures/laptop.txt diff --git a/system/soundserver/hda/fixtures/qemu-intel-hda.txt b/system/hda/driver/fixtures/qemu-intel-hda.txt similarity index 100% rename from system/soundserver/hda/fixtures/qemu-intel-hda.txt rename to system/hda/driver/fixtures/qemu-intel-hda.txt diff --git a/system/soundserver/hda/src/caps.rs b/system/hda/driver/src/caps.rs similarity index 100% rename from system/soundserver/hda/src/caps.rs rename to system/hda/driver/src/caps.rs diff --git a/system/soundserver/hda/src/config.rs b/system/hda/driver/src/config.rs similarity index 99% rename from system/soundserver/hda/src/config.rs rename to system/hda/driver/src/config.rs index 8983b8daa21..60eab9698cd 100644 --- a/system/soundserver/hda/src/config.rs +++ b/system/hda/driver/src/config.rs @@ -330,7 +330,7 @@ mod tests { use super::*; use crate::fixture; use crate::path::find_output_path; - use toyos_mixer::DEVICE_RATES; + use toyos_sound_output::RATES as DEVICE_RATES; /// The bit this sequence never sets. const AMP_MUTE: u16 = AMP_SET_MUTE; @@ -464,9 +464,9 @@ mod tests { /// The AMD bench's analog controller's codecs and the path chosen on them. fn amd_bench() -> (Vec, OutputPath) { - let mut bench = fixture::amd_bench(); - let (index, path) = crate::path::choose(&bench).unwrap(); - (bench.swap_remove(index), path) + let codecs = fixture::amd_bench().swap_remove(1); + let path = crate::path::find_output_path(&codecs).unwrap(); + (codecs, path) } #[test] diff --git a/system/soundserver/hda/src/controller.rs b/system/hda/driver/src/controller.rs similarity index 94% rename from system/soundserver/hda/src/controller.rs rename to system/hda/driver/src/controller.rs index 65629c6b85d..da72c90ba44 100644 --- a/system/soundserver/hda/src/controller.rs +++ b/system/hda/driver/src/controller.rs @@ -1,5 +1,5 @@ -//! The controller: its reset, the codec link over CORB and RIRB, and one -//! output stream, laid out in one grant. +//! The controller: its reset, the codec link over CORB and RIRB in one grant, +//! and one output stream cycling a ring the device reaches. //! //! Every `§` is a section of the *Intel High Definition Audio Specification*, //! Revision 1.0a. @@ -33,20 +33,21 @@ pub const PERIOD_BYTES: usize = 512; /// is told (§3.3.35: tag 0 is reserved as unused). pub const STREAM_TAG: u8 = 1; -/// The grant: CORB, RIRB and the descriptor list each on a 128-byte boundary -/// (§3.6.2, §3.6.4, §3.6.5), then the periods (§4.5.1). +/// The grant: CORB, RIRB and the descriptor list, each on a 128-byte boundary +/// (§3.6.2, §3.6.4, §3.6.5). The periods are the ring soundserver lends. const CORB: usize = 0x0000; const RIRB: usize = 0x0400; const BDL: usize = 0x0C00; -const PCM: usize = 0x1000; -pub const GRANT_BYTES: usize = 0x2000; +pub const GRANT_BYTES: usize = 0x1000; + +/// §4.5.1: a buffer starts on a 128-byte boundary. +const BUFFER_ALIGN: u64 = 128; const _: () = { assert!(CORB + 256 * 4 <= RIRB); assert!(RIRB + 256 * 8 <= BDL); - assert!(BDL + PERIODS * 16 <= PCM); - assert!(PCM + PERIODS * PERIOD_BYTES <= GRANT_BYTES); - assert!(PCM.is_multiple_of(128) && PERIOD_BYTES.is_multiple_of(128)); + assert!(BDL + PERIODS * 16 <= GRANT_BYTES); + assert!(PERIOD_BYTES.is_multiple_of(BUFFER_ALIGN as usize)); }; /// How many times a register is read while a step waits on it. @@ -185,6 +186,8 @@ pub enum Refusal { Beyond32Bits { device_addr: u64 }, /// GCAP reports no output stream descriptor. NoOutputStream { gcap: Gcap }, + /// The lent ring is where §4.5.1 puts no buffer: off a 128-byte boundary. + RingUnaligned { device_addr: u64 }, /// The output descriptor's SRST never read back 1, or 0 (§3.3.35). StreamReset { entered: bool }, } @@ -207,6 +210,7 @@ impl core::fmt::Display for Refusal { write!(f, "it addresses 32 bits and the grant is at {device_addr:#x}") } Self::NoOutputStream { gcap } => write!(f, "it has no output stream (GCAP {:#06x})", gcap.0), + Self::RingUnaligned { device_addr } => write!(f, "the lent ring at {device_addr:#x} is off a 128-byte boundary"), Self::StreamReset { entered: true } => write!(f, "its output stream never entered reset"), Self::StreamReset { entered: false } => write!(f, "its output stream never left reset"), } @@ -519,18 +523,26 @@ pub struct Stream { } impl Stream { - /// §4.5.3's descriptor half: reset it, point it at the cyclic buffer, and - /// arm its interrupt. The codec is told the format and [`STREAM_TAG`] + /// §4.5.3's descriptor half: reset it, point it at the cyclic buffer of + /// [`PERIODS`] periods of [`PERIOD_BYTES`] the device reaches at `ring`, + /// and arm its interrupt. The codec is told the format and [`STREAM_TAG`] /// after this and before [`Self::start`]. /// /// # Panics /// If the grant is shorter than [`GRANT_BYTES`]: the caller's own mistake. - pub fn open(regs: R, mem: &impl DmaBuffers, reset: &Reset, format: u16) -> Result { + pub fn open(regs: R, mem: &impl DmaBuffers, ring: u64, reset: &Reset, format: u16) -> Result { assert!(mem.bytes() >= GRANT_BYTES, "hda: a grant of {:#x} bytes holds no layout", mem.bytes()); let gcap = reset.gcap; if gcap.output_streams() == 0 { return Err(Refusal::NoOutputStream { gcap }); } + if !ring.is_multiple_of(BUFFER_ALIGN) { + return Err(Refusal::RingUnaligned { device_addr: ring }); + } + let end = ring + (PERIODS * PERIOD_BYTES) as u64 - 1; + if !gcap.wide() && end >> 32 != 0 { + return Err(Refusal::Beyond32Bits { device_addr: ring }); + } // §3.3.14: input descriptors are numbered first. let index = gcap.input_streams(); let at = SD_BASE + index * SD_STRIDE; @@ -545,8 +557,8 @@ impl Stream { return Err(Refusal::StreamReset { entered: false }); } - let entries = stream::build_bdl(mem.device_addr(PCM), PERIOD_BYTES as u32, PERIODS) - .expect("hda: the pipeline's own shape builds a descriptor list"); + let entries = stream::build_bdl(ring, PERIOD_BYTES as u32, PERIODS) + .expect("hda: a ring the device reaches builds a descriptor list"); for (i, entry) in entries.iter().enumerate() { let slot = BDL + i * 16; mem.write64(slot, entry.address); @@ -571,12 +583,6 @@ impl Stream { Ok(Self { regs, at, last: 0, running: false }) } - /// Where period `idx`'s samples are in the grant. - pub fn period(idx: usize) -> usize { - assert!(idx < PERIODS, "hda: there is no period {idx}"); - PCM + idx * PERIOD_BYTES - } - pub fn running(&self) -> bool { self.running } diff --git a/system/soundserver/hda/src/controller/tests.rs b/system/hda/driver/src/controller/tests.rs similarity index 95% rename from system/soundserver/hda/src/controller/tests.rs rename to system/hda/driver/src/controller/tests.rs index 16a4a026903..ba7511a0859 100644 --- a/system/soundserver/hda/src/controller/tests.rs +++ b/system/hda/driver/src/controller/tests.rs @@ -18,6 +18,8 @@ use crate::probe; const LAPTOP_GCAP: u16 = 0x9701; const WINDOW: usize = 0x4000; const GRANT_AT: u64 = 0x2000_0000; +/// Where the device reaches the ring soundserver lent. +const RING_AT: u64 = 0x4000_0000; /// What the model's codecs answer one verb with: each a response and its /// `Resp_Ex` word, written to the RIRB in order. @@ -554,7 +556,7 @@ const SD: usize = SD_BASE + 7 * SD_STRIDE; fn streaming(model: &Model) -> Stream { let reset = brought_up(model); - Stream::open(model.regs(), &model.mem(), &reset, 0x4011).expect("the model's stream opens") + Stream::open(model.regs(), &model.mem(), RING_AT, &reset, 0x4011).expect("the model's stream opens") } #[test] @@ -565,7 +567,7 @@ fn the_stream_is_reset_and_pointed_at_the_grants_periods_before_it_runs() { assert!(model.0.borrow().srst_seen); for i in 0..PERIODS { let slot = BDL + i * 16; - assert_eq!(model.mem64(slot), GRANT_AT + (PCM + i * PERIOD_BYTES) as u64); + assert_eq!(model.mem64(slot), RING_AT + (i * PERIOD_BYTES) as u64); assert_eq!(model.mem32(slot + 8), PERIOD_BYTES as u32); assert_eq!(model.mem32(slot + 12), 1, "IOC on every period"); } @@ -586,17 +588,33 @@ fn a_controller_without_an_output_stream_or_whose_stream_stays_in_reset_is_refus let model = Model::new(0x0701, 0b1); let reset = brought_up(&model); assert!(matches!( - Stream::open(model.regs(), &model.mem(), &reset, 0x4011), + Stream::open(model.regs(), &model.mem(), RING_AT, &reset, 0x4011), Err(Refusal::NoOutputStream { .. }) )); let model = Model::laptop().with(|s| s.srst_stuck = true); let reset = brought_up(&model); assert!(matches!( - Stream::open(model.regs(), &model.mem(), &reset, 0x4011), + Stream::open(model.regs(), &model.mem(), RING_AT, &reset, 0x4011), Err(Refusal::StreamReset { entered: true }) )); } +#[test] +fn a_ring_the_engine_cannot_be_pointed_at_is_refused() { + let model = Model::laptop(); + let reset = brought_up(&model); + assert!(matches!( + Stream::open(model.regs(), &model.mem(), RING_AT + 64, &reset, 0x4011), + Err(Refusal::RingUnaligned { device_addr }) if device_addr == RING_AT + 64 + )); + let narrow = Model::new(LAPTOP_GCAP & !1, 0b1); + let reset = brought_up(&narrow); + assert!(matches!( + Stream::open(narrow.regs(), &narrow.mem(), 0xFFFF_FF80, &reset, 0x4011), + Err(Refusal::Beyond32Bits { .. }) + )); +} + #[test] fn played_names_the_periods_between_two_positions_and_clears_the_status() { let model = Model::laptop(); diff --git a/system/soundserver/hda/src/fixture.rs b/system/hda/driver/src/fixture.rs similarity index 100% rename from system/soundserver/hda/src/fixture.rs rename to system/hda/driver/src/fixture.rs diff --git a/system/soundserver/hda/src/graph.rs b/system/hda/driver/src/graph.rs similarity index 100% rename from system/soundserver/hda/src/graph.rs rename to system/hda/driver/src/graph.rs diff --git a/system/soundserver/hda/src/lib.rs b/system/hda/driver/src/lib.rs similarity index 77% rename from system/soundserver/hda/src/lib.rs rename to system/hda/driver/src/lib.rs index 390d09d36de..8c195c47c3f 100644 --- a/system/soundserver/hda/src/lib.rs +++ b/system/hda/driver/src/lib.rs @@ -1,9 +1,9 @@ -//! soundserver's Intel HDA driver, pure: the controller and its codec link -//! ([`controller`]) over `toyos-device-memory`'s boundary, and everything +//! The `hda` program's Intel HDA driver, pure: the controller and its codec +//! link ([`controller`]) over `toyos-device-memory`'s boundary, and everything //! decided from what a codec said about itself — which converter and pins //! carry sound to a speaker, and the verbs that configure them. //! -//! No instruction that reaches a device is here: soundserver implements the +//! No instruction that reaches a device is here: the program implements the //! boundary over its claim. Pure, the controller is host-tested against a //! model of one, and the graph traversal against the committed codec dumps of //! the machines that have to work and against states no machine in reach @@ -32,6 +32,6 @@ mod fixture; pub use caps::{AmpCaps, ConfigDefault, GainRange, PinCaps, WidgetCaps, WidgetKind}; pub use graph::{Codec, FunctionGroup, Widget}; -pub use path::{choose, find_output_path, Hop, OutputPath, PathError, PinSetup, Unchosen}; +pub use path::{find_output_path, Hop, OutputPath, PathError, PinSetup}; pub use probe::{enumerate, CodecFault, Verbs}; pub use verb::{Address, Node, Response, Verb}; diff --git a/system/soundserver/hda/src/path.rs b/system/hda/driver/src/path.rs similarity index 88% rename from system/soundserver/hda/src/path.rs rename to system/hda/driver/src/path.rs index 2f3a87ea091..102ae08df07 100644 --- a/system/soundserver/hda/src/path.rs +++ b/system/hda/driver/src/path.rs @@ -125,31 +125,6 @@ pub fn find_output_path(codecs: &[Codec]) -> Result { })) } -/// Which of a machine's controllers carries its audio: the one whose codecs -/// reach an output ([`find_output_path`]), with that output. -/// -/// None, or several, is [`Unchosen`], carrying what each controller's walk -/// found: choosing between two audible outputs is a default-device policy, and -/// nothing here keeps one. -pub fn choose(controllers: &[Vec]) -> Result<(usize, OutputPath), Unchosen> { - let found: Vec> = - controllers.iter().map(|codecs| find_output_path(codecs)).collect(); - if found.iter().filter(|walk| walk.is_ok()).count() != 1 { - return Err(Unchosen(found)); - } - let (index, path) = found - .into_iter() - .enumerate() - .find_map(|(i, walk)| walk.ok().map(|path| (i, path))) - .expect("counted one"); - Ok((index, path)) -} - -/// Every controller's walk, in the order [`choose`] was given them, where none -/// or several reach an output. -#[derive(Clone, PartialEq, Eq, Debug)] -pub struct Unchosen(pub Vec>); - /// `Err(None)` is a group with no wired output pin to try, which is not a /// fault — display audio is exactly that, and it is the ordinary case on a /// machine with two codecs. @@ -296,51 +271,30 @@ mod tests { (codecs, display) } + /// Each of the AMD bench's controllers walked by a driver of its own: the + /// GPU's display audio reaches no output and the analog codec's speaker + /// does, so only the second driver offers soundserver an output. As the + /// recorded boot read it: `converter 0x02 -> pin 0x14 (speaker), headphone + /// 0x21`. #[test] - fn the_one_controller_of_the_laptop_is_chosen() { - let (index, path) = choose(&[fixture::laptop()]).unwrap(); - assert_eq!(index, 0); - assert_eq!(path, find_output_path(&fixture::laptop()).unwrap()); - } - - #[test] - fn the_amd_bench_binds_its_analog_controller_over_the_gpu_s_display_audio() { - // As soundserver read it on the recorded boot: `hda bound 03:00.6, not - // 03:00.1`, `converter 0x02 -> pin 0x14 (speaker), headphone 0x21`. - let mut bench = fixture::amd_bench(); - let (index, path) = choose(&bench).unwrap(); - assert_eq!(index, 1); + fn of_the_amd_bench_s_two_controllers_only_the_analog_one_reaches_an_output() { + let bench = fixture::amd_bench(); + assert!(matches!(find_output_path(&bench[0]), Err(PathError::NoOutputPin { .. }))); + let path = find_output_path(&bench[1]).expect("the analog codec reaches its speaker"); assert_eq!(path.converter, Node(0x02)); assert_eq!(path.output.node, Node(0x14)); assert_eq!(path.device, DefaultDevice::Speaker); assert_eq!(path.headphone.expect("the jack").node, Node(0x21)); - assert!(matches!(find_output_path(&bench[0]), Err(PathError::NoOutputPin { .. }))); - - bench.reverse(); - assert_eq!(choose(&bench).unwrap().0, 0); - } - - #[test] - fn two_controllers_that_each_reach_an_output_are_refused_naming_both() { - let Err(Unchosen(found)) = choose(&[fixture::laptop(), fixture::qemu()]) else { - panic!("two audible outputs were chosen between"); - }; - assert_eq!(found.len(), 2); - assert!(found.iter().all(Result::is_ok)); } + /// The laptop's display audio, alone on a controller as the GPU's is on + /// the AMD bench, names the codec that reached nothing. #[test] - fn controllers_none_of_which_reaches_an_output_are_refused_naming_both() { + fn display_audio_alone_reaches_no_output_and_names_its_codec() { let (_, display) = analog_and_display(); - let Err(Unchosen(found)) = choose(&[display, fixture::synthetic_dead_end()]) else { - panic!("a controller with no output was chosen"); - }; assert_eq!( - found, - [ - Err(PathError::NoOutputPin { codecs: alloc::vec![Address::new(2).unwrap()] }), - Err(PathError::NoConverter { pin: Node(0x20) }), - ] + find_output_path(&display), + Err(PathError::NoOutputPin { codecs: alloc::vec![Address::new(2).unwrap()] }) ); } diff --git a/system/soundserver/hda/src/probe.rs b/system/hda/driver/src/probe.rs similarity index 100% rename from system/soundserver/hda/src/probe.rs rename to system/hda/driver/src/probe.rs diff --git a/system/soundserver/hda/src/stream.rs b/system/hda/driver/src/stream.rs similarity index 82% rename from system/soundserver/hda/src/stream.rs rename to system/hda/driver/src/stream.rs index c2dded2806d..011f5f4a22b 100644 --- a/system/soundserver/hda/src/stream.rs +++ b/system/hda/driver/src/stream.rs @@ -109,7 +109,8 @@ pub fn last_valid_index(periods: usize) -> Option { (MIN_PERIODS..=MAX_PERIODS).contains(&periods).then(|| (periods - 1) as u8) } -/// Which periods have played since `last`, and where the engine is now. +/// Which periods have played since `last`, and where the engine is now: one +/// contiguous run of the ring, as `toyos-sound-output`'s `run` reads it. /// /// Derived from a position read and never from counting interrupts: one /// interrupt can cover several periods, and soundserver's mix loop already asserts @@ -159,48 +160,11 @@ pub fn period_at(position: u32, period_bytes: u32, periods: usize) -> Option Option { - if !(MIN_PERIODS..=MAX_PERIODS).contains(&periods) || mask >> periods != 0 { - return None; - } - let count = mask.count_ones() as usize; - if count == periods { - return Some(Completed::Lapped); - } - let first = (0..periods) - .find(|&i| mask & 1 << i != 0 && mask & 1 << ((i + periods - 1) % periods) == 0)?; - ((0..count).fold(0u32, |run, i| run | 1 << ((first + i) % periods)) == mask) - .then_some(Completed::Run { first, count }) -} - #[cfg(test)] mod tests { use super::*; + // The reader's half: what soundserver makes of the masks this file builds. + use toyos_sound_output::{run, Run}; /// The rate half of the field for every rate HDA 1.0a Table 53 names /// beside a BASE, MULT and DIV, each word written out from that table's @@ -365,12 +329,6 @@ mod tests { assert_eq!(completed(8, 0, 512, 8), None); } - #[test] - fn a_mask_names_the_period_the_engine_returns_to_first() { - assert_eq!(decode(0b0011_1000, 8), Some(Completed::Run { first: 3, count: 3 })); - assert_eq!(decode(0b0000_0001, 8), Some(Completed::Run { first: 0, count: 1 })); - } - #[test] fn a_run_that_wraps_does_not_start_at_its_lowest_bit() { // The engine was at 6 and is now playing 2: 6, 7, 0 and 1 have played @@ -379,7 +337,7 @@ mod tests { // buffers the engine reaches soonest, which is a splice with no silence // in it for a gap detector to see. let (mask, _) = completed(6, 2 * 512, 512, 8).unwrap(); - assert_eq!(decode(mask, 8), Some(Completed::Run { first: 6, count: 4 })); + assert_eq!(run(mask, 8), Some(Run::Of { first: 6, count: 4 })); assert_ne!(mask.trailing_zeros(), 6); } @@ -389,8 +347,8 @@ mod tests { for position in 0..=8 * 512u32 { let (mask, _) = completed(last, position, 512, 8).unwrap(); let count = mask.count_ones() as usize; - let want = (count > 0).then_some(Completed::Run { first: last, count }); - assert_eq!(decode(mask, 8), want.or(decode(0, 8)), "last={last} pos={position}"); + let want = (count > 0).then_some(Run::Of { first: last, count }); + assert_eq!(run(mask, 8), want, "last={last} pos={position}"); } } } @@ -404,16 +362,6 @@ mod tests { mask |= completed(last, ((last as u32 + 1) % 8) * 512, 512, 8).unwrap().0; } assert_eq!(mask, 0xFF); - assert_eq!(decode(mask, 8), Some(Completed::Lapped)); - } - - #[test] - fn a_mask_that_is_not_one_run_is_refused_rather_than_placed() { - // Two disjoint runs: no walk of the ring produces it, so it is a bug on - // the side that built the mask and not a position to fill from. - assert_eq!(decode(0b0010_0101, 8), None); - // A bit outside the ring. - assert_eq!(decode(1 << 8, 8), None); - assert_eq!(decode(1, 1), None); + assert_eq!(run(mask, 8), Some(Run::Lapped)); } } diff --git a/system/soundserver/hda/src/verb.rs b/system/hda/driver/src/verb.rs similarity index 100% rename from system/soundserver/hda/src/verb.rs rename to system/hda/driver/src/verb.rs diff --git a/system/hda/src/main.rs b/system/hda/src/main.rs new file mode 100644 index 00000000000..2a80c50aec1 --- /dev/null +++ b/system/hda/src/main.rs @@ -0,0 +1,452 @@ +//! The Intel HD Audio driver: one process per HDA function the kernel does not +//! drive, holding its claim (a `drives` row), and a client of soundserver's +//! output port. +//! +//! **The controller and every decision about its codecs are `toyos-hda`'s**: +//! the reset, CORB/RIRB, the stream, the walk, the path, the rate, the format +//! and the master. This file is what is left: the claim's two mappings, the +//! ring soundserver lends, the offer, and the loop that carries the stream's +//! interrupts to soundserver and soundserver's requests to the device. +//! +//! **A controller whose codecs reach no output offers nothing** and ends by +//! name: the GPU's display audio is one, and soundserver plays to the +//! controller that does reach one. A refusal after the offer was accepted ends +//! this program by name too; no conforming controller writes one. + +use std::time::Duration; + +use toyos::endow::{self, Endowments}; +use toyos::ipc::{FrameRx, RxStep}; +use toyos::poller::{Poller, READABLE}; +use toyos::shm::SharedMemory; +use toyos::syscap::SysCap; +use toyos::{say, AsHandle, Connection, DmaRegion, OwnedHandle, PciDev}; +use toyos_abi::syscall::SyscallError; +use toyos_hda::config::{self, Master, NoMaster, Setting}; +use toyos_hda::controller::{ + self, Link, LinkFault, Stream, CODEC_DETECT_MICROS, GRANT_BYTES, PERIODS, PERIOD_BYTES, +}; +use toyos_hda::graph::Codec; +use toyos_hda::path::PathError; +use toyos_hda::probe; +use toyos_hda::verb::Response; +use toyos_manifest::device::PciMatch; +use toyos_pci_claim::{Bar, Grant, KernelRefused}; +use toyos_sound_output::{ + self as output, Engine, Offer, Played, ToDriver, MSG_MASTER, MSG_MASTER_UNAVAILABLE, MSG_OFFER, MSG_PLAYED, + MSG_STREAM, +}; + +/// A multimedia audio device of subclass `03` (PCI Code and ID Assignment: +/// HD Audio), with whatever programming interface: `00` for the base register +/// model and `80` for one with vendor extensions, which the T14's has and this +/// driver does not use. +const FUNCTION: PciMatch = PciMatch::any_interface([0x04, 0x03], None); + +/// The register window: §3.3's registers are memory BAR 0. +const REGISTERS: u32 = 0; + +/// The widest frame soundserver sends: a master step's `i32`. +const MAX_REQUEST: usize = 4; + +const TOKEN_CLAIM: u64 = 0; +const TOKEN_CONTROL: u64 = 1; +const TOKEN_STREAM: u64 = 2; + +/// Why this function carries no audio. Each is the line this program ends on. +enum Refusal { + Kernel(KernelRefused), + NoRegisters, + Controller(controller::Refusal), + Link(LinkFault), + NoOutput(PathError), + /// The converter behind the chosen pin offers none of soundserver's rates + /// at 16 bits; what it does offer, where it has a PCM word at all. + Rate(Option), + /// soundserver plays to another output. + Declined, + Soundserver(String), +} + +impl From for Refusal { + fn from(why: KernelRefused) -> Self { + Self::Kernel(why) + } +} + +impl From for Refusal { + fn from(why: controller::Refusal) -> Self { + Self::Controller(why) + } +} + +fn path_error(f: &mut core::fmt::Formatter<'_>, why: &PathError) -> core::fmt::Result { + match why { + PathError::NoOutputPin { codecs } if codecs.is_empty() => write!(f, "no codec answered"), + PathError::NoOutputPin { codecs } => { + write!(f, "no output a human can hear, on codec")?; + for (i, address) in codecs.iter().enumerate() { + write!(f, "{}{address}", if i == 0 { " " } else { ", " })?; + } + Ok(()) + } + PathError::Cycle { pin, at } => { + write!(f, "pin {:#04x}'s connection list leads back to {:#04x}", pin.0, at.0) + } + PathError::OutsideGroup { pin, named } => write!( + f, + "pin {:#04x} names node {:#04x}, which its function group never declared", + pin.0, named.0 + ), + PathError::NoConverter { pin } => write!(f, "no converter behind pin {:#04x}", pin.0), + } +} + +impl core::fmt::Display for Refusal { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match self { + Self::Kernel(why) => write!(f, "{why}"), + Self::NoRegisters => write!(f, "the claim gives its register BAR no window"), + Self::Controller(why) => write!(f, "the controller refused: {why}"), + Self::Link(why) => write!(f, "the codec link refused: {why}"), + Self::NoOutput(why) => { + write!(f, "its codecs reach no output: ")?; + path_error(f, why) + } + Self::Rate(None) => write!(f, "the converter has no PCM word"), + Self::Rate(Some(pcm)) => { + write!(f, "the converter offers none of {:?} Hz at {} bits; it offers", output::RATES, config::WIDTH)?; + for rate in pcm.rates() { + write!(f, " {rate}")?; + } + write!(f, " Hz at {:?} bits", pcm.widths().collect::>()) + } + Self::Declined => write!(f, "soundserver plays to another output"), + Self::Soundserver(why) => write!(f, "soundserver: {why}"), + } + } +} + +struct NoMasterLine(NoMaster); + +impl core::fmt::Display for NoMasterLine { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match self.0 { + NoMaster::NoGain => write!(f, "no amplifier on the path has a gain range"), + NoMaster::NoMute => write!(f, "neither an output pin nor the converter has a mute bit"), + } + } +} + +/// The function, driven: what the loop needs and what it must keep alive. +struct Driver { + at: String, + dev: PciDev, + link: Link, + stream: Stream, + master: Option<(Master, Setting)>, + control: Connection, + to_engine: Connection, + underruns: u32, + /// Held for their mappings' lives: the register window points into the + /// first, the link and descriptor list into the second, and the device + /// reaches the third while it is mapped. + _bar: SharedMemory, + _region: DmaRegion, + _ring: OwnedHandle, +} + +fn main() { + let dev = endow::claim::(&FUNCTION.to_string()) + .expect("hda: the drives row endows this process its function's claim"); + let info = dev.describe().expect("hda: the claim's description"); + let at = format!("hda {:02x}:{:02x}.{}", info.bus, info.dev, info.func); + let bar_bytes = info.bar_bytes[REGISTERS as usize]; + match Driver::up(dev, bar_bytes, &at) { + Ok(driver) => driver.serve(), + Err(why) => say!("{at}: plays nothing: {why}"), + } +} + +/// Every codec `statests` names, walked over the link. +fn walk(at: &str, link: &mut Link, statests: u16) -> Vec { + let mut codecs = Vec::new(); + for entry in probe::enumerate(link, statests) { + match entry { + Ok(codec) => { + say!( + "{at}: codec{} vendor={:04x} device={:04x}, {} function group(s)", + codec.address, + codec.vendor, + codec.device, + codec.groups.len() + ); + codecs.push(codec); + } + Err((address, fault)) => say!("{at}: codec{address} answered nothing usable ({fault:?})"), + } + } + codecs +} + +impl Driver { + /// Bring the controller up, walk its codecs and find the path that reaches + /// an output; offer it, and on soundserver's acceptance point the stream at + /// the ring it lends and configure the codec, in §4.5.3's order: the + /// descriptor, then the codec. + fn up(dev: PciDev, bar_bytes: u64, at: &str) -> Result { + if bar_bytes == 0 { + return Err(Refusal::NoRegisters); + } + let (bar, bar_mapping) = Bar::map(&dev, REGISTERS, bar_bytes)?; + let reset = controller::reset(&bar, || std::thread::sleep(Duration::from_micros(CODEC_DETECT_MICROS)))?; + say!("{at}: gcap={:#06x} statests={:#06x}", reset.gcap.0, reset.statests); + + let (grant, region) = Grant::alloc(&dev, GRANT_BYTES as u64)?; + grant.window().zero(); + let mut link = Link::open(bar, grant, &reset)?; + let codecs = walk(at, &mut link, reset.statests); + if let Some(fault) = link.refused() { + return Err(Refusal::Link(fault)); + } + let path = toyos_hda::find_output_path(&codecs).map_err(Refusal::NoOutput)?; + let rate = config::rate(&codecs, &path, &output::RATES).map_err(Refusal::Rate)?; + let (format, channels) = + config::format(&codecs, &path, rate).expect("a rate the converter offers has a format"); + say!( + "{at}: codec{} group {:#04x} converter {:#04x} -> pin {:#04x} ({}), headphone {}, \ + format {:#06x} ({} Hz {} ch {}-bit)", + path.codec, + path.group.0, + path.converter.0, + path.output.node.0, + path.device.name(), + match &path.headphone { + Some(hp) => format!("{:#04x}", hp.node.0), + None => String::from("none"), + }, + format, + rate, + channels, + config::WIDTH, + ); + let master = match config::master(&path) { + Ok(master) => Some((master, Setting::TOP)), + Err(why) => { + say!("{at}: the path has no master ({}); volume answers unavailable", NoMasterLine(why)); + None + } + }; + + let offer = Offer { + rate, + channels, + periods: PERIODS as u8, + period_bytes: PERIOD_BYTES as u32, + master_bottom: master.as_ref().map(|(master, _)| master.bottom()), + }; + let control = endow::service(output::PORT) + .map_err(|why| Refusal::Soundserver(format!("no `{}` port ({why:?})", output::PORT)))?; + let ring = offered(&control, &offer)?; + let lent = dev.dma_map(ring.as_handle()).map_err(KernelRefused::on("the lent ring's mapping"))?; + if lent.bytes < offer.ring_bytes() as u64 { + return Err(Refusal::Soundserver(format!( + "it lent a ring of {} bytes for {}", + lent.bytes, + offer.ring_bytes() + ))); + } + + let stream = Stream::open(bar, &grant, lent.device_addr, &reset, format)?; + let verbs = config::verbs(&codecs, &path, format, controller::STREAM_TAG) + .expect("the path names a codec this walk produced"); + let sent = verbs.len(); + for verb in verbs { + link.send(verb).map_err(Refusal::Link)?; + } + let to_engine = endow::service(output::PORT) + .map_err(|why| Refusal::Soundserver(format!("no second `{}` connection ({why:?})", output::PORT)))?; + to_engine + .send_bytes(MSG_STREAM, &[]) + .map_err(|why| Refusal::Soundserver(format!("the stream connection was refused ({why:?})")))?; + say!("{at}: path configured in {sent} verbs, stream tag {}; offered to soundserver", controller::STREAM_TAG); + Ok(Self { + at: at.to_string(), + dev, + link, + stream, + master, + control, + to_engine, + underruns: 0, + _bar: bar_mapping, + _region: region, + _ring: ring, + }) + } + + /// Carry the stream's interrupts to soundserver and soundserver's requests + /// to the device, until soundserver hangs up. + fn serve(mut self) { + rt(); + let poller = Poller::new(4); + let mut control_rx = FrameRx::::new(); + let mut stream_rx = FrameRx::::new(); + loop { + poller.watch(&self.dev, READABLE, TOKEN_CLAIM); + poller.watch(&self.control, READABLE, TOKEN_CONTROL); + poller.watch(&self.to_engine, READABLE, TOKEN_STREAM); + poller.wait(1, u64::MAX, |_| {}); + + self.interrupts(); + while let Some((msg_type, payload)) = frame(&mut stream_rx, &self.to_engine, &self.at, "stream") { + match Engine::decode(msg_type, &payload) { + Some(Engine::Start) => self.stream.start(), + Some(Engine::Stop) => self.stream.stop(), + None => panic!("{}: soundserver sent stream frame {msg_type}, which this protocol has not", self.at), + } + .unwrap_or_else(|why| panic!("{}: cannot be driven on — {why}", self.at)); + } + while let Some((msg_type, payload)) = frame(&mut control_rx, &self.control, &self.at, "control") { + match ToDriver::decode(msg_type, &payload) { + Some(ToDriver::MasterStep(delta)) => { + let target = self.master.as_ref().map(|(master, setting)| master.step(*setting, delta)); + self.answer(target); + } + Some(ToDriver::MasterMute) => { + let target = self.master.as_ref().map(|&(_, setting)| Setting { muted: !setting.muted, ..setting }); + self.answer(target); + } + _ => panic!("{}: soundserver sent control frame {msg_type}, which this state has not", self.at), + } + } + } + } + + /// The periods the stream played, reported with when its interrupts said + /// so. + /// + /// The stream raises one message per status it clears, for the first + /// period to complete after the clear, so the record's newest is never + /// earlier in play order than the first period of the mask: that one is + /// clocked by it, and the rest wait for the next record's. + fn interrupts(&mut self) { + let record = match self.dev.irq() { + Ok(record) => record, + Err(SyscallError::WouldBlock) => return, + Err(why) => panic!("{}: the claim's interrupt record: {why:?}", self.at), + }; + let played = self.stream.played().unwrap_or_else(|why| panic!("{}: cannot be driven on — {why}", self.at)); + if played.underran { + self.underruns += 1; + if self.underruns == 1 { + say!("{}: the stream underran (FIFOE); counted from here and not said again", self.at); + } + } + if played.mask == 0 { + return; + } + let report = Played { mask: played.mask, clocked: 1, first_nanos: record.first_nanos, last_nanos: record.last_nanos }; + if let Err(why) = self.to_engine.send_bytes(MSG_PLAYED, &report.encode()) { + soundserver_gone(&self.at, why); + } + } + + /// Set the master to `target` and answer what the codec read back. + fn answer(&mut self, target: Option) { + let read = target.and_then(|target| self.set(target)); + let sent = match read { + Some(read) => { + let (master, setting) = self.master.as_mut().expect("a read master is one this path has"); + *setting = read; + let answer = output::Master { position: read.position, bottom: master.bottom(), muted: read.muted }; + self.control.send_bytes(MSG_MASTER, &answer.encode()) + } + None => self.control.send_bytes(MSG_MASTER_UNAVAILABLE, &[]), + }; + if let Err(why) = sent { + soundserver_gone(&self.at, why); + } + } + + /// Set `target` on the codec and read it back; `None`, named, where the + /// link refused or the answers name no setting. + fn set(&mut self, target: Setting) -> Option { + let (master, _) = self.master.as_ref()?; + for verb in master.set(target) { + if let Err(why) = self.link.send(verb) { + say!("{}: the master set was refused ({:#010x}): {why}", self.at, verb.raw()); + return None; + } + } + let mut answers = Vec::new(); + for verb in master.read() { + match self.link.send(verb).map(Response::new) { + Ok(Some(answer)) => answers.push(answer), + Ok(None) => { + say!("{}: the master readback answered all ones", self.at); + return None; + } + Err(why) => { + say!("{}: the master readback was refused: {why}", self.at); + return None; + } + } + } + let read = master.decode(&answers); + if read.is_none() { + say!("{}: the master read back as no setting ({answers:?})", self.at); + } + read + } +} + +/// Offer `offer` on `control` and take soundserver's answer: the ring it lends. +fn offered(control: &Connection, offer: &Offer) -> Result { + let refused = |why: String| Refusal::Soundserver(why); + control.send_bytes(MSG_OFFER, &offer.encode()).map_err(|why| refused(format!("the offer was refused ({why:?})")))?; + let header = control.recv_header().map_err(|why| refused(format!("no answer to the offer ({why:?})")))?; + let mut payload = [0u8; MAX_REQUEST]; + let n = control + .recv_bytes(&header, &mut payload) + .map_err(|why| refused(format!("an answer of {} bytes ({why:?})", header.len())))?; + match ToDriver::decode(header.msg_type, &payload[..n]) { + Some(ToDriver::Accepted) => { + let [ring] = control + .recv_handles_exact::<{ output::ACCEPTED_HANDLES }>() + .ok_or_else(|| refused("its acceptance carried no ring".into()))?; + // SAFETY: the kernel moved this handle into this table with the + // frame that names it, and nothing else answers for it. + Ok(unsafe { OwnedHandle::from_raw(ring) }) + } + Some(ToDriver::Refused) => Err(Refusal::Declined), + _ => Err(refused(format!("it answered the offer with frame {}", header.msg_type))), + } +} + +/// The next whole frame soundserver sent on `conn`, if one is there. +fn frame(rx: &mut FrameRx, conn: &Connection, at: &str, which: &str) -> Option<(u32, Vec)> { + match rx.pump(conn) { + RxStep::Idle => None, + RxStep::Frame { msg_type, payload_len } => Some((msg_type, rx.payload(payload_len).to_vec())), + RxStep::Eof => { + say!("{at}: soundserver hung up the {which} connection; the stream stops with it"); + std::process::exit(0); + } + RxStep::Malformed => panic!("{at}: soundserver sent a {which} frame this protocol cannot describe"), + } +} + +fn soundserver_gone(at: &str, why: impl core::fmt::Debug) -> ! { + say!("{at}: soundserver took no report ({why:?}); the stream stops with it"); + std::process::exit(0); +} + +/// The real-time band the manifest grants this row: the stream's interrupt is +/// a period's end, and what it reports soundserver mixes the next one by. +fn rt() { + let rt: SysCap = Endowments::get() + .take(toyos_abi::syscall::SYSCAP_LABEL) + .expect("the manifest declares this program `syscap = [\"rt\"]`"); + rt.enter_rt().expect("an RT capability refused the band it names"); +} diff --git a/system/soundserver/Cargo.toml b/system/soundserver/Cargo.toml index 5d07aae7710..a4c399e3465 100644 --- a/system/soundserver/Cargo.toml +++ b/system/soundserver/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "soundserver" -description = "The audio server: ToyOS's audio devices claimed from the kernel, and every client's stream mixed onto them." +description = "The audio server: every client's stream mixed onto the output an audio driver offers on its sound-output port, or onto the virtio-sound function it claims itself." edition = "2024" license = "MIT OR Apache-2.0" @@ -9,7 +9,7 @@ toyos-abi = { path = "../../sdk/abi" } # The matcher its claim is endowed under, spelled as the manifest spells it. toyos-manifest = { path = "../../lib/manifest" } toyos = { path = "../../sdk/toyos" } -toyos-hda = { path = "hda" } +toyos-sound-output = { path = "output" } toyos-mixer = { path = "mixer" } toyos-virtio-sound = { path = "virtio-sound" } toyos-virtio = { path = "../../lib/virtio" } @@ -19,4 +19,4 @@ toyos-resample = { path = "resample" } rubato = { version = "0.16", default-features = false } [package.metadata.toyos.host] -exempt.owns = "ToyOS's audio devices, claimed from the kernel" +exempt.owns = "ToyOS's audio outputs: the drivers' offers on its sound-output port, and the virtio-sound function it claims" diff --git a/system/soundserver/output/Cargo.toml b/system/soundserver/output/Cargo.toml new file mode 100644 index 00000000000..ea6d2bf083d --- /dev/null +++ b/system/soundserver/output/Cargo.toml @@ -0,0 +1,22 @@ +# A member of the host workspace (root `Cargo.toml`), like `toyos-mixer` +# beside it: pure, and its tests run on the host. Every frame a driver or +# soundserver reads off the other is decoded here, once. + +[package] +name = "toyos-sound-output" +description = "soundserver's output protocol: what an audio driver process offers on the sound-output port, the ring soundserver lends it, and the periods it reports played." +version = "0.1.0" +edition = "2021" +license = "MIT OR Apache-2.0" +publish = false + +[lib] +doctest = false + +[dependencies] +# The device rates soundserver mixes at, best first: an offer at another is +# refused. +toyos-mixer = { path = "../mixer" } + +[lints.rust] +warnings = "deny" diff --git a/system/soundserver/output/src/lib.rs b/system/soundserver/output/src/lib.rs new file mode 100644 index 00000000000..6036abc7e2b --- /dev/null +++ b/system/soundserver/output/src/lib.rs @@ -0,0 +1,364 @@ +//! soundserver's output protocol: an audio driver process, one per function it +//! drives, is a client of soundserver's [`PORT`]. +//! +//! **Two connections per driver, both opened on [`PORT`].** On the first, the +//! *control* connection, the driver offers its output ([`Offer`]), and +//! soundserver answers [`MSG_ACCEPTED`] with the ring of periods it lends the +//! driver, [`Offer::periods`] of [`Offer::period_bytes`] laid end to end, or +//! [`MSG_REFUSED`]. The driver points its device at the ring and opens the +//! second, the *stream* connection, whose first frame is [`MSG_STREAM`]: one +//! offer is accepted at a time, so the stream is that offer's. The master's +//! requests and answers stay on the control connection; [`MSG_START`], +//! [`MSG_STOP`] and [`MSG_PLAYED`] are the stream connection's, so +//! soundserver's mix thread reads nothing but periods. +//! +//! **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; soundserver serves it. +pub const PORT: &str = "sound-output"; + +/// The rates soundserver mixes at, best first: a driver offers the first its +/// device runs at, and an offer at another is refused. +pub use toyos_mixer::DEVICE_RATES as RATES; + +/// The deepest ring an offer may name: [`Played::mask`] is a `u32`. +pub const MAX_PERIODS: usize = 32; + +// The control connection, driver to soundserver. +pub const MSG_OFFER: u32 = 1; +pub const MSG_STREAM: u32 = 2; +pub const MSG_MASTER: u32 = 3; +pub const MSG_MASTER_UNAVAILABLE: u32 = 4; +// The control connection, soundserver to driver. +pub const MSG_ACCEPTED: u32 = 16; +pub const MSG_REFUSED: u32 = 17; +pub const MSG_MASTER_STEP: u32 = 18; +pub const MSG_MASTER_MUTE: u32 = 19; +// The stream connection. +pub const MSG_START: u32 = 32; +pub const MSG_STOP: u32 = 33; +pub const MSG_PLAYED: u32 = 34; + +/// The handles [`MSG_ACCEPTED`] carries: the lent ring. +pub const ACCEPTED_HANDLES: usize = 1; + +/// What a driver's device plays, and whether it has a master. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub struct Offer { + pub rate: u32, + pub channels: u8, + pub periods: u8, + pub period_bytes: u32, + /// The master's quietest position, where the path has a master. + pub master_bottom: Option, +} + +impl Offer { + pub const BYTES: usize = 12; + + pub fn encode(&self) -> [u8; Self::BYTES] { + let mut out = [0u8; Self::BYTES]; + out[0..4].copy_from_slice(&self.rate.to_le_bytes()); + out[4..8].copy_from_slice(&self.period_bytes.to_le_bytes()); + out[8] = self.channels; + out[9] = self.periods; + out[10] = u8::from(self.master_bottom.is_some()); + out[11] = self.master_bottom.unwrap_or(0); + out + } + + /// The bytes of a ring of this shape. + pub fn ring_bytes(&self) -> usize { + self.periods as usize * self.period_bytes as usize + } + + fn decode(bytes: &[u8]) -> Option { + let bytes: &[u8; Self::BYTES] = bytes.try_into().ok()?; + let rate = u32::from_le_bytes(bytes[0..4].try_into().ok()?); + let period_bytes = u32::from_le_bytes(bytes[4..8].try_into().ok()?); + let (channels, periods) = (bytes[8], bytes[9]); + let master_bottom = match (bytes[10], bytes[11]) { + (0, 0) => None, + (1, bottom) => Some(bottom), + _ => return None, + }; + let frame = 2 * channels as u32; + let whole = RATES.contains(&rate) + && matches!(channels, 1 | 2) + && (2..=MAX_PERIODS).contains(&(periods as usize)) + && periods.is_power_of_two() + && period_bytes != 0 + && period_bytes.is_multiple_of(frame); + whole.then_some(Self { rate, channels, periods, period_bytes, master_bottom }) + } +} + +/// The master as the codec read it back. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub struct Master { + /// 0 is the loudest; [`Self::bottom`] the quietest. + pub position: u8, + pub bottom: u8, + pub muted: bool, +} + +impl Master { + pub const BYTES: usize = 3; + + pub fn encode(&self) -> [u8; Self::BYTES] { + [self.position, self.bottom, u8::from(self.muted)] + } + + fn decode(bytes: &[u8]) -> Option { + let [position, bottom, muted]: [u8; Self::BYTES] = bytes.try_into().ok()?; + let muted = match muted { + 0 => false, + 1 => true, + _ => return None, + }; + (position <= bottom).then_some(Self { position, bottom, muted }) + } +} + +/// Periods the device played, and when its interrupts said so. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub struct Played { + /// One bit per period, one contiguous run of the ring ([`run`]). + pub mask: u32, + /// How many of these periods, in play order, end by [`Self::last_nanos`]; + /// the rest are a later report's to clock. + pub clocked: u32, + /// When the first and the newest interrupt this report answers for landed. + pub first_nanos: u64, + pub last_nanos: u64, +} + +impl Played { + pub const BYTES: usize = 24; + + pub fn encode(&self) -> [u8; Self::BYTES] { + let mut out = [0u8; Self::BYTES]; + out[0..4].copy_from_slice(&self.mask.to_le_bytes()); + out[4..8].copy_from_slice(&self.clocked.to_le_bytes()); + out[8..16].copy_from_slice(&self.first_nanos.to_le_bytes()); + out[16..24].copy_from_slice(&self.last_nanos.to_le_bytes()); + out + } + + fn decode(bytes: &[u8]) -> Option { + let bytes: &[u8; Self::BYTES] = bytes.try_into().ok()?; + let played = Self { + mask: u32::from_le_bytes(bytes[0..4].try_into().ok()?), + clocked: u32::from_le_bytes(bytes[4..8].try_into().ok()?), + first_nanos: u64::from_le_bytes(bytes[8..16].try_into().ok()?), + last_nanos: u64::from_le_bytes(bytes[16..24].try_into().ok()?), + }; + let whole = played.mask != 0 + && played.clocked <= played.mask.count_ones() + && played.first_nanos <= played.last_nanos; + whole.then_some(played) + } +} + +/// What a driver says on a connection's first frame, and on its control +/// connection after it. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub enum FromDriver { + Offer(Offer), + /// The stream connection's first frame. + Stream, + Master(Master), + MasterUnavailable, +} + +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_OFFER => Offer::decode(payload).map(Self::Offer), + MSG_STREAM if payload.is_empty() => Some(Self::Stream), + MSG_MASTER => Master::decode(payload).map(Self::Master), + MSG_MASTER_UNAVAILABLE if payload.is_empty() => Some(Self::MasterUnavailable), + _ => None, + } + } +} + +/// What soundserver says on a driver's control connection. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub enum ToDriver { + /// With [`ACCEPTED_HANDLES`] handles. + Accepted, + Refused, + /// Move the master by this many positions; a step unmutes. + MasterStep(i32), + /// Mute, or unmute at the level it had. + MasterMute, +} + +impl ToDriver { + pub fn decode(msg_type: u32, payload: &[u8]) -> Option { + match (msg_type, payload.len()) { + (MSG_ACCEPTED, 0) => Some(Self::Accepted), + (MSG_REFUSED, 0) => Some(Self::Refused), + (MSG_MASTER_STEP, 4) => Some(Self::MasterStep(i32::from_le_bytes(payload.try_into().ok()?))), + (MSG_MASTER_MUTE, 0) => Some(Self::MasterMute), + _ => None, + } + } +} + +/// What soundserver says on a stream connection. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub enum Engine { + Start, + Stop, +} + +impl Engine { + pub fn decode(msg_type: u32, payload: &[u8]) -> Option { + match (msg_type, payload.len()) { + (MSG_START, 0) => Some(Self::Start), + (MSG_STOP, 0) => Some(Self::Stop), + _ => None, + } + } +} + +/// A [`MSG_PLAYED`] frame, the only one a driver sends on its stream +/// connection. +pub fn played(msg_type: u32, payload: &[u8]) -> Option { + (msg_type == MSG_PLAYED).then(|| Played::decode(payload)).flatten() +} + +/// What a played mask says about a cyclic engine. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub enum Run { + /// The engine handed back `count` periods starting at `first`, and is in + /// `first + count` now. + Of { first: usize, count: usize }, + /// Every period in the ring. The engine has been round at least once since + /// the reader last looked, so nothing in the mask says where it is: the + /// whole pipeline has played out. + Lapped, +} + +/// Read a played mask of a `periods`-deep ring. +/// +/// A mask is a set and the engine is a sequence, so the set alone has to say +/// which period the engine returns to *first* — and that is the mask's +/// lowest-numbered bit only while the run does not wrap the ring. `{6, 7, 0, +/// 1}` is played 6, 7, 0, 1, and a reader filling it lowest-index-first writes +/// the later audio into the buffer the engine reaches soonest. +/// +/// It is always one contiguous run: a report a reader takes late is the OR of +/// consecutive ones, and those abut. `None` for anything else, which no walk +/// of a ring produces. +pub fn run(mask: u32, periods: usize) -> Option { + if !(2..=MAX_PERIODS).contains(&periods) || (periods < 32 && mask >> periods != 0) { + return None; + } + let count = mask.count_ones() as usize; + if count == periods { + return Some(Run::Lapped); + } + let first = (0..periods) + .find(|&i| mask & 1 << i != 0 && mask & 1 << ((i + periods - 1) % periods) == 0)?; + ((0..count).fold(0u32, |run, i| run | 1 << ((first + i) % periods)) == mask) + .then_some(Run::Of { first, count }) +} + +#[cfg(test)] +mod tests { + use super::*; + + const OFFER: Offer = Offer { rate: 44_100, channels: 2, periods: 8, period_bytes: 512, master_bottom: Some(87) }; + + #[test] + fn every_frame_decodes_back_to_what_was_encoded() { + assert_eq!(FromDriver::decode(MSG_OFFER, &OFFER.encode()), Some(FromDriver::Offer(OFFER))); + let bare = Offer { master_bottom: None, ..OFFER }; + assert_eq!(FromDriver::decode(MSG_OFFER, &bare.encode()), Some(FromDriver::Offer(bare))); + let master = Master { position: 3, bottom: 87, muted: true }; + assert_eq!(FromDriver::decode(MSG_MASTER, &master.encode()), Some(FromDriver::Master(master))); + assert_eq!(FromDriver::decode(MSG_STREAM, &[]), Some(FromDriver::Stream)); + assert_eq!(FromDriver::decode(MSG_MASTER_UNAVAILABLE, &[]), Some(FromDriver::MasterUnavailable)); + let played = Played { mask: 0b1100_0001, clocked: 1, first_nanos: 5, last_nanos: 9 }; + assert_eq!(super::played(MSG_PLAYED, &played.encode()), Some(played)); + assert_eq!(ToDriver::decode(MSG_MASTER_STEP, &(-3i32).to_le_bytes()), Some(ToDriver::MasterStep(-3))); + assert_eq!(ToDriver::decode(MSG_ACCEPTED, &[]), Some(ToDriver::Accepted)); + assert_eq!(Engine::decode(MSG_START, &[]), Some(Engine::Start)); + assert_eq!(Engine::decode(MSG_STOP, &[]), Some(Engine::Stop)); + } + + /// An offer soundserver cannot mix into is refused at the frame, field by + /// field. + #[test] + fn an_offer_outside_what_soundserver_mixes_is_refused() { + let refused = |offer: Offer| assert_eq!(FromDriver::decode(MSG_OFFER, &offer.encode()), None, "{offer:?}"); + refused(Offer { rate: 96_000, ..OFFER }); + refused(Offer { channels: 0, ..OFFER }); + refused(Offer { channels: 3, ..OFFER }); + refused(Offer { periods: 1, ..OFFER }); + refused(Offer { periods: 6, ..OFFER }); + refused(Offer { periods: 64, ..OFFER }); + refused(Offer { period_bytes: 0, ..OFFER }); + refused(Offer { period_bytes: 514, ..OFFER }); + let mut bytes = OFFER.encode(); + bytes[10] = 2; + assert_eq!(FromDriver::decode(MSG_OFFER, &bytes), None, "a master flag that is no bool"); + let mut bytes = Offer { master_bottom: None, ..OFFER }.encode(); + bytes[11] = 4; + assert_eq!(FromDriver::decode(MSG_OFFER, &bytes), None, "a bottom with no master"); + assert_eq!(FromDriver::decode(MSG_OFFER, &OFFER.encode()[..11]), None); + } + + #[test] + fn a_master_or_a_report_no_device_produces_is_refused() { + assert_eq!(FromDriver::decode(MSG_MASTER, &[9, 8, 0]), None, "past the bottom"); + assert_eq!(FromDriver::decode(MSG_MASTER, &[0, 8, 2]), None, "muted is no bool"); + let played = Played { mask: 0b11, clocked: 1, first_nanos: 5, last_nanos: 9 }; + let refused = |p: Played| assert_eq!(super::played(MSG_PLAYED, &p.encode()), None, "{p:?}"); + refused(Played { mask: 0, clocked: 0, ..played }); + refused(Played { clocked: 3, ..played }); + refused(Played { first_nanos: 10, ..played }); + assert_eq!(super::played(MSG_START, &played.encode()), None); + } + + /// A frame of the wrong type or width, in either direction, is no message. + #[test] + fn a_frame_this_protocol_does_not_have_is_refused() { + for msg_type in [0, MSG_ACCEPTED, MSG_START, MSG_PLAYED, u32::MAX] { + assert_eq!(FromDriver::decode(msg_type, &[]), None); + } + assert_eq!(FromDriver::decode(MSG_STREAM, &[0]), None); + for msg_type in [0, MSG_OFFER, MSG_PLAYED, u32::MAX] { + assert_eq!(ToDriver::decode(msg_type, &[]), None); + assert_eq!(Engine::decode(msg_type, &[]), None); + } + assert_eq!(ToDriver::decode(MSG_MASTER_STEP, &[1, 2, 3]), None); + assert_eq!(Engine::decode(MSG_START, &[0]), None); + } + + #[test] + fn a_mask_names_the_period_the_engine_returns_to_first() { + assert_eq!(run(0b0011_1000, 8), Some(Run::Of { first: 3, count: 3 })); + assert_eq!(run(0b0000_0001, 8), Some(Run::Of { first: 0, count: 1 })); + // Played 6, 7, 0, 1: the run starts at 6, not at its lowest bit. + assert_eq!(run(0b1100_0011, 8), Some(Run::Of { first: 6, count: 4 })); + assert_eq!(run(0xFF, 8), Some(Run::Lapped)); + assert_eq!(run(u32::MAX, 32), Some(Run::Lapped)); + } + + #[test] + fn a_mask_that_is_not_one_run_is_refused_rather_than_placed() { + assert_eq!(run(0b0010_0101, 8), None); + assert_eq!(run(1 << 8, 8), None); + assert_eq!(run(1, 1), None); + assert_eq!(run(0, 8), None, "an empty report is no run"); + } +} diff --git a/system/soundserver/src/backend.rs b/system/soundserver/src/backend.rs index 4cdbab250a7..74977b3fff7 100644 --- a/system/soundserver/src/backend.rs +++ b/system/soundserver/src/backend.rs @@ -1,6 +1,8 @@ -//! The device half of the mix loop, and the two devices behind it. +//! The device half of the mix loop, and the two devices behind it: the +//! virtio-sound function soundserver drives itself, and an output driver's +//! ring (`crate::output`). //! -//! Everything that differs between virtio-sound and HDA is a method below. The +//! Everything that differs between them is a method below. The //! mixer, the ramps, the DLL, the underrun accounting and the suspend/resume //! structure are one body of code either way — and [`Pipeline`] exists to keep //! that honest, because the two differ in *who owns a period soundserver has not @@ -9,7 +11,7 @@ use toyos::AsHandle; use toyos_abi::RawHandle; -use crate::{hda, virtio}; +use crate::virtio; /// Who owns a period soundserver has been given back and has not refilled. /// @@ -26,7 +28,7 @@ pub(crate) enum Pipeline { /// does not have. Holding one costs nothing, indefinitely, and the play /// order is the submit order. Queue, - /// HDA: the engine owns every period for as long as it runs. It returns to + /// An output driver's (HDA's) ring: the engine owns every period for as long as it runs. It returns to /// buffer `i` exactly `num_buffers` periods after completing it and plays /// whatever is there, so a period soundserver holds back is played as the /// silence `released` left in it *and* completed a second time — a @@ -52,11 +54,9 @@ pub(crate) struct Completion { /// The device half of the mix loop. /// -/// Two implementations and no more: a framework before there are three is an -/// abstraction with no evidence behind it. Both are drivers in this process -/// now, and what differs between the two devices is exactly the methods below; -/// the mixer, the ramps, the DLL, the underrun accounting and the -/// suspend/resume structure are one body of code either way. +/// Two implementations and no more: what differs between the two devices is +/// exactly the methods below; the mixer, the ramps, the DLL, the underrun +/// accounting and the suspend/resume structure are one body of code either way. pub(crate) trait Backend { /// Who owns a freed period soundserver does not refill. fn pipeline(&self) -> Pipeline; @@ -122,37 +122,3 @@ impl Backend for VirtioBackend { self.virtio.stop(); } } - -pub(crate) struct HdaBackend { - pub(crate) hda: hda::Hda, -} - -impl Backend for HdaBackend { - fn pipeline(&self) -> Pipeline { - Pipeline::Ring - } - - fn handle(&self) -> RawHandle { - self.hda.dev().as_handle() - } - - fn buffer(&self, idx: usize) -> *mut u8 { - self.hda.buffer(idx).as_ptr() - } - - fn completion(&mut self) -> Option { - self.hda.completion() - } - - fn released(&mut self, idx: usize) { - self.hda.buffer(idx).zero(); - } - - fn submit(&mut self, _idx: usize, _bytes: usize) { - self.hda.start(); - } - - fn stop(&mut self) { - self.hda.stop(); - } -} diff --git a/system/soundserver/src/command.rs b/system/soundserver/src/command.rs index a5647c76e0c..9abe7d82d2a 100644 --- a/system/soundserver/src/command.rs +++ b/system/soundserver/src/command.rs @@ -17,16 +17,22 @@ use toyos_mixer::Gain; use crate::client::{ClientStream, Departure}; use crate::control::MAX_CONTROL_CLIENTS; +use crate::output::Attached; use crate::ring::Spsc; /// Deep enough that one pass of the control loop can never fill it: a pass -/// pushes at most one `AddClient` (there is one accept per wait) plus, per -/// connected client, one coalesced `SetVolume` and one `RemoveClient`. +/// pushes at most one `AddClient` (there is one accept per wait) and one +/// `Attach`, plus, per connected client, one coalesced `SetVolume` and one +/// `RemoveClient`. const CMD_RING_SIZE: usize = 256; -const _: () = assert!(CMD_RING_SIZE >= 1 + 2 * MAX_CONTROL_CLIENTS); +const _: () = assert!(CMD_RING_SIZE >= 2 + 2 * MAX_CONTROL_CLIENTS); pub(crate) enum MixCommand { AddClient(Box), + /// Play to this output from here. Sent only while no stream is open, so + /// the streams the null sink still holds are all ramping out; every + /// command behind it is the output's. + Attach(Box), RemoveClient { client_id: usize, departure: Departure }, SetVolume { client_id: usize, target: Gain }, } diff --git a/system/soundserver/src/control.rs b/system/soundserver/src/control.rs index 75f8e1f6fc8..c51e3b31794 100644 --- a/system/soundserver/src/control.rs +++ b/system/soundserver/src/control.rs @@ -25,6 +25,7 @@ use toyos_mixer::{period_nanos, Gain, MAX_CLIENT_RATE, MIN_CLIENT_RATE}; use crate::client::{open_stream, Departure}; use crate::command::{remove, submit, CommandRing, MixCommand}; use crate::inspect::{self, Device, Published}; +use crate::output::{Event, Outputs}; use crate::volume::{self, VolumeClient}; /// Control connections soundserver will hold at once. @@ -121,24 +122,24 @@ fn description(device: &Device) -> DeviceDescription { } } +/// The tokens [`Outputs::watch`] answers with start here, above every other. +const TOKEN_OUTPUTS: u64 = 1 << 40; + pub(crate) fn control_thread( acceptor: Acceptor, volume_acceptor: Acceptor, - mut master: Option, + mut outputs: Outputs, cmd_ring: &CommandRing, cmd_pipe_write: RawHandle, - device_sample_rate: u32, - device_channels: u16, - device_period_frames: u32, - slot_count: u32, - ramp_frames: u32, - device: &Device, + mut device: Device, published: &Published, ) { - // One handle per client plus the acceptor; `MAX_CONTROL_CLIENTS` is derived - // from this ring, so the set always fits in one batch. - let poller = Poller::new(MAX_CONTROL_CLIENTS as u32 + 1); - let period_nanos = period_nanos(device_period_frames as u64, device_sample_rate as u64); + // One handle per client plus the acceptor, the volume port's two and the + // outputs'. + let watched = MAX_CONTROL_CLIENTS + 1 + 2 + Outputs::tokens(0).count(); + let poller = Poller::new(watched as u32); + let mut period_nanos = period_nanos(device.period_frames as u64, device.rate as u64); + let mut ramp_frames = toyos_mixer::ramp_frames(device.rate); struct ControlClient { conn: Connection, @@ -165,8 +166,19 @@ pub(crate) fn control_thread( /// A `volume` connection's token: its index above every control client's. const TOKEN_VOLUME: u64 = 1 << 32; + let mut events: Vec = Vec::new(); loop { + // An output waits for no stream to be open: from here every stream + // opens on it. + if let Some(attached) = clients.iter().all(|c| c.stream_idx.is_none()).then(|| outputs.attach()).flatten() { + device = crate::output_device(&attached.offer, attached.period_frames); + period_nanos = toyos_mixer::period_nanos(device.period_frames as u64, device.rate as u64); + ramp_frames = toyos_mixer::ramp_frames(device.rate); + submit(cmd_ring, cmd_pipe_write, MixCommand::Attach(attached), period_nanos); + } + poller.watch(&acceptor, READABLE, TOKEN_ACCEPT); + outputs.watch(&poller, TOKEN_OUTPUTS); poller.watch(&volume_acceptor, READABLE, TOKEN_VOLUME_ACCEPT); for (i, client) in clients.iter().enumerate() { poller.watch(&client.conn, READABLE, i as u64); @@ -208,11 +220,17 @@ pub(crate) fn control_thread( Err(e) => say!("soundserver: volume accept failed: {e:?}"), } } + outputs.serve(&ready, TOKEN_OUTPUTS, &mut events); + for event in events.drain(..) { + match event { + Event::Master(master) => volume::answer(&mut volume_clients, master), + } + } let mut at = TOKEN_VOLUME; volume_clients.retain_mut(|client| { let token = at; at += 1; - !ready.contains(&token) || volume::serve(client, master.as_mut()) + !ready.contains(&token) || volume::serve(client, &mut outputs) }); let mut dead: Vec = Vec::new(); @@ -277,10 +295,10 @@ pub(crate) fn control_thread( idx, &req, &clients[i].conn, - device_sample_rate, - device_channels, - device_period_frames, - slot_count, + device.rate, + device.channels, + device.period_frames, + device.buffers, ramp_frames, ) else { let _ = clients[i].conn.signal(MSG_STREAM_ERROR); @@ -312,7 +330,7 @@ pub(crate) fn control_thread( // or not the reader took it: a reader that will not take // one frame is not one this thread waits for. Control::Inspect => { - let answer = inspect::snapshot(device, published); + let answer = inspect::snapshot(&device, published); if let Err(e) = clients[i].conn.try_send_bytes(toyos_inspect::MSG_SNAPSHOT, &answer) { @@ -324,7 +342,7 @@ pub(crate) fn control_thread( } Control::Describe => { if let Err(e) = - clients[i].conn.try_send(MSG_DEVICE_DESCRIBED, &description(device)) + clients[i].conn.try_send(MSG_DEVICE_DESCRIBED, &description(&device)) { say!("soundserver: a device description was not answered ({e:?})"); } diff --git a/system/soundserver/src/hda.rs b/system/soundserver/src/hda.rs deleted file mode 100644 index 235237412de..00000000000 --- a/system/soundserver/src/hda.rs +++ /dev/null @@ -1,365 +0,0 @@ -//! soundserver as the driver of an Intel HDA controller it holds as a PCI -//! claim. -//! -//! What the kernel keeps is the claim: config space, the message it armed, -//! and the address space the function translates through. **The controller — -//! its reset, CORB and RIRB, the stream and its descriptor list — is -//! `toyos-hda`'s `controller`, and every decision about the codec is -//! `toyos-hda`'s too**: which pin, which converter, the rate, the amplifiers, -//! EAPD, the format and the master. This file is what is left: the claim's -//! two mappings, when its periods came back, and what becomes of a refusal. -//! -//! After the claim the control thread owns the codec link -//! ([`MasterControl`]) and the mix thread the stream ([`Hda`]); the two touch -//! disjoint registers. -//! -//! **A refusal after bring-up ends soundserver** by its own name: no -//! conforming controller writes one. - -use std::sync::Arc; -use std::time::Duration; - -use toyos::shm::SharedMemory; -use toyos::volatile::Window; -use toyos::{DmaRegion, PciDev}; -use toyos_abi::syscall::SyscallError; -use toyos_hda::caps::PcmCaps; -use toyos_hda::config::{self, Master, NoMaster, Setting}; -use toyos_hda::controller::{self, Link, LinkFault, Stream, CODEC_DETECT_MICROS, GRANT_BYTES, PERIOD_BYTES}; -use toyos_hda::graph::Codec; -use toyos_hda::path::{PathError, Unchosen}; -use toyos_hda::probe; -use toyos_pci_claim::{Bar, Grant, KernelRefused}; - -use crate::backend::Completion; - -/// A multimedia audio device, class `04:03:00` (PCI Code and ID Assignment: -/// HD Audio, Intel HDA 1.0 compatible), whoever made it, as the manifest row -/// spells the claim. -pub const FUNCTION: toyos_manifest::device::PciMatch = - toyos_manifest::device::PciMatch::new([0x04, 0x03, 0x00], None); - -/// The register window: §3.3's registers are memory BAR 0. -const REGISTERS: u32 = 0; - -/// The two mappings the controller's windows point into, held by both -/// halves for as long as either drives it. -struct Mappings { - _bar: SharedMemory, - _region: DmaRegion, -} - -/// The stream: the mix thread's, which starts and stops it. -pub struct Hda { - dev: PciDev, - stream: Stream, - pcm: Window, - underruns: u32, - _held: Arc, -} - -/// The codec's master: the control thread's, which sets and reads it back. -pub struct MasterControl { - link: Link, - master: Master, - setting: Setting, - _held: Arc, -} - -/// What a claim of the HDA function came to. -pub struct Claimed { - pub hda: Hda, - pub rate: u32, - pub channels: u8, - /// `None` where the path has no master, which soundserver names and serves - /// as unavailable. - pub master: Option, -} - -/// Why this machine's HDA function cannot carry audio. Each is a line -/// soundserver prints before falling back to the null sink. -pub enum Refusal { - Kernel(KernelRefused), - /// The description gives BAR 0 no window. - NoRegisters, - Controller(controller::Refusal), - /// The codec link refused an answer during the walk or the configuration. - Link(LinkFault), - /// The codecs reach no output. - Choice(Unchosen), - /// The converter behind the chosen pin offers none of the device rates, at - /// 16 bits; what it does offer, where it has a PCM word at all. - Rate(Option), -} - -impl From for Refusal { - fn from(why: KernelRefused) -> Self { - Self::Kernel(why) - } -} - -impl From for Refusal { - fn from(why: controller::Refusal) -> Self { - Self::Controller(why) - } -} - -fn path_error(f: &mut core::fmt::Formatter<'_>, why: &PathError) -> core::fmt::Result { - match why { - PathError::NoOutputPin { codecs } if codecs.is_empty() => { - write!(f, "no codec answered") - } - PathError::NoOutputPin { codecs } => { - write!(f, "no output a human can hear, on codec")?; - for (i, address) in codecs.iter().enumerate() { - write!(f, "{}{address}", if i == 0 { " " } else { ", " })?; - } - Ok(()) - } - PathError::Cycle { pin, at } => { - write!(f, "pin {:#04x}'s connection list leads back to {:#04x}", pin.0, at.0) - } - PathError::OutsideGroup { pin, named } => write!( - f, - "pin {:#04x} names node {:#04x}, which its function group never declared", - pin.0, named.0 - ), - PathError::NoConverter { pin } => write!(f, "no converter behind pin {:#04x}", pin.0), - } -} - -impl core::fmt::Display for Refusal { - fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { - match self { - Self::Kernel(why) => write!(f, "{why}"), - Self::NoRegisters => write!(f, "the claim gives its register BAR no window"), - Self::Controller(why) => write!(f, "the controller refused: {why}"), - Self::Link(why) => write!(f, "the codec link refused: {why}"), - Self::Choice(Unchosen(found)) => { - write!(f, "its codecs reach no output")?; - for walk in found.iter().filter_map(|walk| walk.as_ref().err()) { - write!(f, ": ")?; - path_error(f, walk)?; - } - Ok(()) - } - Self::Rate(None) => write!(f, "the converter has no PCM word"), - Self::Rate(Some(pcm)) => { - write!(f, "the converter offers none of {:?} Hz at {} bits; it offers", toyos_mixer::DEVICE_RATES, config::WIDTH)?; - for rate in pcm.rates() { - write!(f, " {rate}")?; - } - write!(f, " Hz at {:?} bits", pcm.widths().collect::>()) - } - } - } -} - -struct NoMasterLine(NoMaster); - -impl core::fmt::Display for NoMasterLine { - fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { - match self.0 { - NoMaster::NoGain => write!(f, "no amplifier on the path has a gain range"), - NoMaster::NoMute => write!(f, "neither an output pin nor the converter has a mute bit"), - } - } -} - -/// Every codec `statests` names, walked over the link. -fn walk(link: &mut Link, statests: u16) -> Vec { - let mut codecs = Vec::new(); - for entry in probe::enumerate(link, statests) { - match entry { - Ok(codec) => { - say!( - "soundserver: hda codec{} vendor={:04x} device={:04x}, {} function group(s)", - codec.address, - codec.vendor, - codec.device, - codec.groups.len() - ); - codecs.push(codec); - } - Err((address, fault)) => { - say!("soundserver: hda codec{address} answered nothing usable ({fault:?})") - } - } - } - codecs -} - -impl Hda { - /// Bring the controller up, walk its codecs, choose the path that reaches - /// an output, and configure it at the first device rate its converter - /// offers, in §4.5.3's order: the descriptor, then the codec. - /// - /// Everything here runs before soundserver starts a thread. - pub fn claim(dev: PciDev) -> Result { - let info = dev.describe().map_err(KernelRefused::on("the description"))?; - let bytes = info.bar_bytes[REGISTERS as usize]; - if bytes == 0 { - return Err(Refusal::NoRegisters); - } - say!("soundserver: hda {:02x}:{:02x}.{}", info.bus, info.dev, info.func); - let (bar, bar_mapping) = Bar::map(&dev, REGISTERS, bytes)?; - let reset = controller::reset(&bar, || std::thread::sleep(Duration::from_micros(CODEC_DETECT_MICROS)))?; - say!("soundserver: hda gcap={:#06x} statests={:#06x}", reset.gcap.0, reset.statests); - - let (grant, region) = Grant::alloc(&dev, GRANT_BYTES as u64)?; - grant.window().zero(); - let held = Arc::new(Mappings { _bar: bar_mapping, _region: region }); - - let mut link = Link::open(bar, grant, &reset)?; - let mut walks = vec![walk(&mut link, reset.statests)]; - if let Some(fault) = link.refused() { - return Err(Refusal::Link(fault)); - } - let (_, path) = toyos_hda::choose(&walks).map_err(Refusal::Choice)?; - let codecs = walks.swap_remove(0); - - let rate = config::rate(&codecs, &path, &toyos_mixer::DEVICE_RATES).map_err(Refusal::Rate)?; - let (format, channels) = - config::format(&codecs, &path, rate).expect("a rate the converter offers has a format"); - say!( - "soundserver: hda codec{} group {:#04x} converter {:#04x} -> pin {:#04x} ({}), \ - headphone {}, format {:#06x} ({} Hz {} ch {}-bit)", - path.codec, - path.group.0, - path.converter.0, - path.output.node.0, - path.device.name(), - match &path.headphone { - Some(hp) => format!("{:#04x}", hp.node.0), - None => String::from("none"), - }, - format, - rate, - channels, - config::WIDTH, - ); - - let stream = Stream::open(bar, &grant, &reset, format)?; - let verbs = config::verbs(&codecs, &path, format, controller::STREAM_TAG) - .expect("the path names a codec this walk produced"); - let sent = verbs.len(); - for verb in verbs { - link.send(verb).map_err(Refusal::Link)?; - } - say!("soundserver: hda path configured in {sent} verbs, stream tag {}", controller::STREAM_TAG); - - let master = match config::master(&path) { - Ok(master) => Some(MasterControl { link, master, setting: Setting::TOP, _held: held.clone() }), - Err(why) => { - say!("soundserver: hda path has no master ({}); volume answers unavailable", NoMasterLine(why)); - None - } - }; - let hda = Hda { dev, stream, pcm: grant.window(), underruns: 0, _held: held }; - Ok(Claimed { hda, rate, channels, master }) - } - - pub fn dev(&self) -> &PciDev { - &self.dev - } - - /// Where period `idx`'s samples go. - pub fn buffer(&self, idx: usize) -> Window { - self.pcm.sub(Stream::::period(idx), PERIOD_BYTES) - } - - /// The periods played since the last call, on the claim's record. - /// - /// The stream raises one message per status it clears, for the first - /// period to complete after the clear, so the record's newest is never - /// earlier in play order than the first period of this mask: that one is - /// clocked by it, never early, and the rest wait for the next record's. - pub fn completion(&mut self) -> Option { - let record = match self.dev.irq() { - Ok(record) => record, - Err(SyscallError::WouldBlock) => return None, - Err(why) => panic!("soundserver: the HDA claim's interrupt record: {why:?}"), - }; - let played = self - .stream - .played() - .unwrap_or_else(|why| panic!("soundserver: hda cannot be driven on — {why}")); - if played.underran { - self.underruns += 1; - if self.underruns == 1 { - say!("soundserver: hda stream underran (FIFOE); counted from here and not said again"); - } - } - (played.mask != 0).then_some(Completion { - mask: played.mask, - first_nanos: record.first_nanos, - last_nanos: record.last_nanos, - clocked: 1, - }) - } - - /// Start the engine, on the edge only: the descriptor list is cyclic, so - /// a period costs this driver a record and no register write. - pub fn start(&mut self) { - self.stream - .start() - .unwrap_or_else(|why| panic!("soundserver: hda could not start its stream: {why}")); - } - - pub fn stop(&mut self) { - self.stream - .stop() - .unwrap_or_else(|why| panic!("soundserver: hda could not stop its stream: {why}")); - } -} - -/// What the master answers a request with: the setting read back from the -/// codec after it was set, never the intent. -#[derive(Clone, Copy, PartialEq, Eq, Debug)] -pub struct Answer { - pub setting: Setting, - pub bottom: u8, -} - -impl MasterControl { - /// Move the level by `delta` positions; a step while muted unmutes. - pub fn step(&mut self, delta: i32) -> Option { - self.apply(self.master.step(self.setting, delta)) - } - - /// Mute, or unmute at the level it had. - pub fn toggle_mute(&mut self) -> Option { - self.apply(Setting { muted: !self.setting.muted, ..self.setting }) - } - - /// Set `setting` and read it back. `None`, named, where the link refused - /// or the answers name no setting. - fn apply(&mut self, setting: Setting) -> Option { - for verb in self.master.set(setting) { - if let Err(why) = self.link.send(verb) { - say!("soundserver: hda master set refused ({:#010x}): {why}", verb.raw()); - return None; - } - } - let mut answers = Vec::new(); - for verb in self.master.read() { - match self.link.send(verb).map(toyos_hda::verb::Response::new) { - Ok(Some(answer)) => answers.push(answer), - Ok(None) => { - say!("soundserver: hda master readback answered all ones"); - return None; - } - Err(why) => { - say!("soundserver: hda master readback refused: {why}"); - return None; - } - } - } - let Some(read) = self.master.decode(&answers) else { - say!("soundserver: hda master read back as no setting ({answers:?})"); - return None; - }; - self.setting = read; - Some(Answer { setting: read, bottom: self.master.bottom() }) - } -} diff --git a/system/soundserver/src/inspect.rs b/system/soundserver/src/inspect.rs index e95a2bd2be3..8c0ab7e2e49 100644 --- a/system/soundserver/src/inspect.rs +++ b/system/soundserver/src/inspect.rs @@ -44,9 +44,10 @@ impl State { } } -/// The device, as it was when the loop started. Fixed for soundserver's life. +/// The device played to: the null sink until an output driver's is. +#[derive(Clone, Copy)] pub(crate) struct Device { - /// `virtio-sound`, `hda` or `null`. + /// `virtio-sound`, `driver` or `null`. pub(crate) kind: &'static str, pub(crate) rate: u32, pub(crate) channels: u16, diff --git a/system/soundserver/src/main.rs b/system/soundserver/src/main.rs index 9d94ca86bb4..df7a04459bb 100644 --- a/system/soundserver/src/main.rs +++ b/system/soundserver/src/main.rs @@ -12,19 +12,21 @@ //! | [`mix`] | the two loops — a device's, and the null sink's | //! | [`ring`] | the lock-free ring every hand-off to or from the mix thread is | //! | [`log`] | soundserver's lines, and why the mix thread's never wait | -//! | [`hda`], [`virtio`] | the two drivers | +//! | [`output`] | the `sound-output` port: the output drivers, and the one played to | +//! | [`virtio`] | the virtio-sound driver, the one device this process drives itself | //! | [`volume`] | the `volume` port: the master level and mute, on the codec | //! -//! This file is the fourth thing: which of them the machine gets. A sound card -//! this process was endowed with runs the device loop; anything else — no card, -//! a card that cannot carry audio, a shape the mixer cannot render — runs the -//! null sink, because soundserver always runs and always accepts streams. +//! This file is which of them the machine gets. A virtio-sound function this +//! process was endowed with runs the device loop; anything else runs the null +//! sink until an output driver's offer is played to, because soundserver always +//! runs and always accepts streams. use toyos::endow; use toyos::port::Acceptor; use toyos::PciDev; use toyos_abi::syscall; use toyos_mixer::{period_frames, ramp_frames, DEVICE_RATES}; +use toyos_sound_output as sound_output; use std::sync::Arc; @@ -39,27 +41,32 @@ mod backend; mod client; mod command; mod control; -mod hda; mod inspect; mod mix; mod log; +mod output; mod ring; mod virtio; mod volume; -use backend::{Backend, HdaBackend, VirtioBackend}; +use backend::{Backend, VirtioBackend}; use command::CommandRing; use inspect::{Device, Published, State}; use control::control_thread; use mix::{mix_thread, null_sink_thread}; +use output::{Attached, Outputs}; -/// The virtual output soundserver presents when the machine has no audio hardware: -/// the first device rate, stereo, 128 frames per period. A stream negotiates +/// The virtual output soundserver presents until it plays to a device: the +/// first device rate, stereo, 128 frames per period. A stream negotiates /// against them exactly as it would a real device, so a no-hardware machine is /// invisible to the client. -const NULL_SINK_RATE: u32 = DEVICE_RATES[0]; -const NULL_SINK_CHANNELS: u16 = 2; -const NULL_SINK_PERIOD_FRAMES: usize = 128; +const NULL_SINK: Device = Device { + kind: "null", + rate: DEVICE_RATES[0], + channels: 2, + period_frames: 128, + buffers: NULL_SINK_BUFFERS as u32, +}; /// Same DMA-pipeline depth as both hardware backends: the client ring /// is as deep, so a client may fill `NULL_SINK_BUFFERS - 1` periods ahead and /// its backpressure is the device's. Power of two — ring indices wrap mod 2^32. @@ -68,210 +75,121 @@ pub(crate) const NULL_SINK_BUFFERS: usize = 8; fn main() { // First: `main` becomes the mix thread, and its lines are a lane's from here. log::claim_lane(); - let acceptor = endow::acceptor("soundserver") - .expect("the manifest declares this program serves `soundserver`"); - let volume = endow::acceptor("volume") - .expect("the manifest declares this program serves `volume`"); - let ports = Ports { acceptor, volume }; + let ports = Ports { + acceptor: endow::acceptor("soundserver").expect("the manifest declares this program serves `soundserver`"), + volume: endow::acceptor("volume").expect("the manifest declares this program serves `volume`"), + output: endow::acceptor(sound_output::PORT) + .expect("the manifest declares this program serves `sound-output`"), + }; - // **"Which sound card does this machine have?" is already answered.** The supervisor - // mints a claim per device entry the manifest names and endows what the machine - // actually had, so an absent card is a label missing from this process's - // own table rather than two probing syscalls — and a card another process - // holds is not a state that can arise, because only the supervisor mints. - // - // A machine with no sound card is a routing state and not a bug: soundserver - // presents a virtual output and discards what is played to it, so a client - // building a stream succeeds whether or not hardware is present. - // - // The order is virtio first, and it is not a preference between two cards: - // no machine in this project has both. The T14 has only the second. + // The virtio-sound function, where the machine has one, is this process's + // own: no machine in this project has it beside an output driver's + // function. The T14 and the AMD bench have output drivers' alone. if let Some(dev) = endow::claim::(&virtio::FUNCTION.to_string()) { match virtio::Virtio::claim(dev) { - Ok((virtio, rate, channels)) => return run_virtio(ports, virtio, rate, channels), - Err(why) => { - say!("soundserver: the virtio-sound device cannot carry audio: {why}"); - return run_null_sink(ports); + Ok((virtio, rate, channels)) => { + let device = Device { + kind: "virtio-sound", + rate, + channels: channels as u16, + period_frames: 0, + buffers: toyos_virtio_sound::PERIODS as u32, + }; + match period_frames(toyos_virtio_sound::PERIODS, device.channels, toyos_virtio_sound::PERIOD_BYTES, rate) { + Ok(frames) => { + let device = Device { period_frames: frames as u32, ..device }; + return run(ports, Some((VirtioBackend { virtio }, device))); + } + Err(why) => say!("soundserver: this audio device's shape cannot carry audio: {why}"), + } } + Err(why) => say!("soundserver: the virtio-sound device cannot carry audio: {why}"), } } - let Some(dev) = endow::claim::(&hda::FUNCTION.to_string()) else { - return run_null_sink(ports); - }; - match hda::Hda::claim(dev) { - Ok(claimed) => run_hda(ports, claimed), - Err(why) => { - say!("soundserver: the HDA controller cannot carry audio: {why}"); - run_null_sink(ports) - } - } + run(ports, None) } -/// The two ports soundserver serves: streams, and the master. +/// The three ports soundserver serves: streams, the master, and its outputs. struct Ports { acceptor: Acceptor, volume: Acceptor, + output: Acceptor, } -fn run_virtio(ports: Ports, virtio: virtio::Virtio, rate: u32, channels: u8) { - run_with_device( - ports, - None, - "virtio-sound", - &mut VirtioBackend { virtio }, - toyos_virtio_sound::PERIODS, - rate, - channels as u16, - toyos_virtio_sound::PERIOD_BYTES, - ); -} - -fn run_hda(ports: Ports, claimed: hda::Claimed) { - let hda::Claimed { hda, rate, channels, master } = claimed; - run_with_device( - ports, - master, - "hda", - &mut HdaBackend { hda }, - toyos_hda::controller::PERIODS, - rate, - channels as u16, - toyos_hda::controller::PERIOD_BYTES, - ); -} - -fn run_with_device( - ports: Ports, - master: Option, - kind: &'static str, - backend: &mut dyn Backend, - num_buffers: usize, - device_sample_rate: u32, - device_channels: u16, - device_period_bytes: usize, -) { - // A shape this mixer cannot render is named and the machine gets the null - // sink, which keeps soundserver always running and accepting streams. It is checked before any arithmetic - // derives anything from it — a zero channel count divides by zero on the - // way to a frame count, which is a panic that names neither the device nor - // the reason. - let device_period_frames = match period_frames(num_buffers, device_channels, device_period_bytes, device_sample_rate) { - Ok(frames) => frames, - Err(why) => { - say!("soundserver: this audio device's shape cannot carry audio: {why}"); - return run_null_sink(ports); - } - }; - - // Client ring depth matches the DMA pipeline depth: a wake gap can free - // at most num_buffers periods, so a full client ring always covers it. - let slot_count = num_buffers as u32; - - let ramp_frames = ramp_frames(device_sample_rate); - - say!("soundserver: ready, {} buffers, {}Hz {}ch, {} bytes/period, {} frames/period", - num_buffers, device_sample_rate, device_channels, device_period_bytes, device_period_frames); - +/// Serve on `own`, the device this process drives, where there is one, and on +/// the null sink until an output is played to otherwise. +fn run(ports: Ports, own: Option<(VirtioBackend, Device)>) { + let device = own.as_ref().map_or(NULL_SINK, |(_, device)| *device); + let outputs = Outputs::new(ports.output, own.is_some()); let cmd_ring = Arc::new(CommandRing::new()); let cmd_pipe = syscall::pipe().expect("soundserver: failed to create the command pipe"); - let published = Arc::new(Published::new(State::Suspended)); - let device = Device { - kind, - rate: device_sample_rate, - channels: device_channels, - period_frames: device_period_frames as u32, - buffers: slot_count, - }; + let initial = if own.is_some() { State::Suspended } else { State::Idle }; + let published = Arc::new(Published::new(initial)); let cmd_ring2 = cmd_ring.clone(); let published2 = published.clone(); std::thread::Builder::new() .name("soundserver-ctrl".into()) .spawn(move || { - control_thread( - ports.acceptor, - ports.volume, - master, - &cmd_ring2, - cmd_pipe.write, - device_sample_rate, - device_channels, - device_period_frames as u32, - slot_count, - ramp_frames, - &device, - &published2, - ); + control_thread(ports.acceptor, ports.volume, outputs, &cmd_ring2, cmd_pipe.write, device, &published2); }) .expect("soundserver: failed to spawn control thread"); - mix_thread( - backend, - &cmd_ring, - &published, - cmd_pipe.read, - num_buffers, - device_sample_rate, - device_channels, - device_period_bytes, - device_period_frames, - ramp_frames, - ); + match own { + Some((mut backend, device)) => { + let period_bytes = toyos_virtio_sound::PERIOD_BYTES; + serve_device(&mut backend, &cmd_ring, &published, cmd_pipe.read, &device, period_bytes) + } + None => { + say!( + "soundserver: no output yet, presenting a null sink ({}Hz {}ch, {} frames/period, streams discarded)", + device.rate, device.channels, device.period_frames + ); + let Attached { mut backend, offer, period_frames } = + *null_sink_thread(&cmd_ring, &published, cmd_pipe.read, &device); + let device = output_device(&offer, period_frames); + say!( + "soundserver: playing to the output offered, {}Hz {}ch, {} frames/period", + device.rate, device.channels, device.period_frames + ); + serve_device(&mut backend, &cmd_ring, &published, cmd_pipe.read, &device, offer.period_bytes as usize) + } + } } -fn run_null_sink(ports: Ports) { - let device_sample_rate = NULL_SINK_RATE; - let device_channels = NULL_SINK_CHANNELS; - let device_period_frames = NULL_SINK_PERIOD_FRAMES; - let slot_count = NULL_SINK_BUFFERS as u32; - - let ramp_frames = ramp_frames(device_sample_rate); +/// An output driver's offer, as the mixer and the control thread see it. +pub(crate) fn output_device(offer: &sound_output::Offer, period_frames: usize) -> Device { + Device { + kind: "driver", + rate: offer.rate, + channels: offer.channels as u16, + period_frames: period_frames as u32, + buffers: offer.periods as u32, + } +} +fn serve_device( + backend: &mut dyn Backend, + cmd_ring: &CommandRing, + published: &Published, + cmd_pipe_read: toyos_abi::RawHandle, + device: &Device, + period_bytes: usize, +) { say!( - "soundserver: no audio device, presenting a null sink ({}Hz {}ch, {} frames/period, streams discarded)", - device_sample_rate, device_channels, device_period_frames + "soundserver: ready, {} buffers, {}Hz {}ch, {} bytes/period, {} frames/period", + device.buffers, device.rate, device.channels, period_bytes, device.period_frames ); - - let cmd_ring = Arc::new(CommandRing::new()); - let cmd_pipe = syscall::pipe().expect("soundserver: failed to create the command pipe"); - let published = Arc::new(Published::new(State::Idle)); - let device = Device { - kind: "null", - rate: device_sample_rate, - channels: device_channels, - period_frames: device_period_frames as u32, - buffers: slot_count, - }; - - let cmd_ring2 = cmd_ring.clone(); - let published2 = published.clone(); - std::thread::Builder::new() - .name("soundserver-ctrl".into()) - .spawn(move || { - control_thread( - ports.acceptor, - ports.volume, - None, - &cmd_ring2, - cmd_pipe.write, - device_sample_rate, - device_channels, - device_period_frames as u32, - slot_count, - ramp_frames, - &device, - &published2, - ); - }) - .expect("soundserver: failed to spawn control thread"); - - null_sink_thread( - &cmd_ring, - &published, - cmd_pipe.read, - device_sample_rate, - device_channels, - device_period_frames, - ramp_frames, + mix_thread( + backend, + cmd_ring, + published, + cmd_pipe_read, + device.buffers as usize, + device.rate, + device.channels, + period_bytes, + device.period_frames as usize, + ramp_frames(device.rate), ); } diff --git a/system/soundserver/src/mix.rs b/system/soundserver/src/mix.rs index d8ed89b9971..9f2b3ad023c 100644 --- a/system/soundserver/src/mix.rs +++ b/system/soundserver/src/mix.rs @@ -16,7 +16,7 @@ use toyos::poller::{Poller, READABLE}; use toyos::syscap::SysCap; use toyos_abi::syscall; use toyos_abi::RawHandle; -use toyos_hda::stream; +use toyos_sound_output::{run, Run}; use toyos_mixer::{ deferral_floor_nanos, period_nanos, quantize_period, scratch_frames, wake_left_idle, Dll, MixStats, Xorshift32, @@ -26,6 +26,8 @@ use crate::backend::{Backend, Pipeline}; use crate::client::{mix_client, ClientStream, Departure}; use crate::command::{CommandRing, MixCommand}; use crate::inspect::{Published, State, Totals}; +use crate::inspect::Device; +use crate::output::Attached; use crate::NULL_SINK_BUFFERS; const STATS_INTERVAL_NANOS: u64 = 2_000_000_000; @@ -97,9 +99,13 @@ fn signal_clients(streams: &mut [ClientStream], ramp_frames: u32) { /// Drain the command ring the control thread fills: connects, disconnects, and /// volume changes. Shared by both sinks — a client's lifecycle is the same /// whether its audio reaches hardware or a discard. -fn apply_commands(cmd_ring: &CommandRing, streams: &mut Vec, ramp_frames: u32) { +/// +/// It stops at an output to play to and answers it: every command behind it +/// is the output's. +fn apply_commands(cmd_ring: &CommandRing, streams: &mut Vec, ramp_frames: u32) -> Option> { while let Some(cmd) = cmd_ring.pop() { match cmd { + MixCommand::Attach(attached) => return Some(attached), MixCommand::AddClient(client) => { say!("soundserver: client {} connected (id={})", streams.len(), client.client_id); let _ = syscall::write_nonblock(client.signal_write, &[1]); @@ -117,6 +123,7 @@ fn apply_commands(cmd_ring: &CommandRing, streams: &mut Vec, ramp_ } } } + None } /// Drop clients whose disconnect ramp has finished. Paused clients mix @@ -333,7 +340,9 @@ pub(crate) fn mix_thread( let mut drain = [0u8; 64]; while matches!(syscall::read_nonblock(cmd_pipe_read, &mut drain), Ok(n) if n == drain.len()) {} } - apply_commands(cmd_ring, &mut streams, ramp_frames); + if apply_commands(cmd_ring, &mut streams, ramp_frames).is_some() { + panic!("soundserver: an output to play to while it plays to a device"); + } if !was_streaming && !streams.is_empty() { stats = MixStats::default(); @@ -357,8 +366,8 @@ pub(crate) fn mix_thread( // stepped itself would have to be right about how many laps // that was. Re-deriving it per record cannot drift. if pipeline == Pipeline::Ring { - match stream::decode(rec.mask, num_buffers) { - Some(stream::Completed::Run { first, count }) => { + match run(rec.mask, num_buffers) { + Some(Run::Of { first, count }) => { ring_cursor = (first + count) % num_buffers; } // Every period played and the mask says no more than @@ -366,7 +375,7 @@ pub(crate) fn mix_thread( // from here is a guess either way, and a lap of silence // has already gone out — it counts as the drain it // is, and the next record re-anchors. - Some(stream::Completed::Lapped) => {} + Some(Run::Lapped) => {} None => panic!( "soundserver: the engine completed {:#x}, which is no walk of a \ {num_buffers}-period ring", @@ -640,7 +649,8 @@ pub(crate) fn mix_thread( } } -/// The default output on a machine with no audio hardware. It presents the same +/// The output until soundserver plays to a device, which it answers with once +/// the streams it holds have ended ([`MixCommand::Attach`]). It presents the same /// virtual device every client negotiates against and drains each stream at that /// real rate off a monotonic software clock, discarding the mix. Hardware /// absence is a routing state, never an error: a client's write and backpressure @@ -661,11 +671,12 @@ pub(crate) fn null_sink_thread( cmd_ring: &CommandRing, published: &Published, cmd_pipe_read: RawHandle, - device_sample_rate: u32, - device_channels: u16, - device_period_frames: usize, - ramp_frames: u32, -) { + device: &Device, +) -> Box { + let device_sample_rate = device.rate; + let device_channels = device.channels; + let device_period_frames = device.period_frames as usize; + let ramp_frames = toyos_mixer::ramp_frames(device_sample_rate); let device_period_samples = device_period_frames * device_channels as usize; let period_nanos = period_nanos(device_period_frames as u64, device_sample_rate as u64); @@ -689,9 +700,16 @@ pub(crate) fn null_sink_thread( // first client of a run connects. let mut next_period_ns = toyos_abi::clock::nanos_since_boot(); + // An output to play to, held until the streams the null sink still has + // have ramped out: the control thread sends one only when none is open. + let mut attach: Option> = None; + say!("soundserver: null sink idle"); loop { + if let Some(attached) = attach.take_if(|_| streams.is_empty()) { + return attached; + } let was_streaming = !streams.is_empty(); signal_clients(&mut streams, ramp_frames); @@ -720,7 +738,11 @@ pub(crate) fn null_sink_thread( let mut drain = [0u8; 64]; while matches!(syscall::read_nonblock(cmd_pipe_read, &mut drain), Ok(n) if n == drain.len()) {} } - apply_commands(cmd_ring, &mut streams, ramp_frames); + // Nothing is drained past an output to play to: what is behind it is + // the output's. + if attach.is_none() { + attach = apply_commands(cmd_ring, &mut streams, ramp_frames); + } // Start the grid when the first client of a run connects, and reset the // reporting window so no idle stretch dilutes it. diff --git a/system/soundserver/src/output.rs b/system/soundserver/src/output.rs new file mode 100644 index 00000000000..ffd63d3a395 --- /dev/null +++ b/system/soundserver/src/output.rs @@ -0,0 +1,394 @@ +//! The output drivers on the `sound-output` port (`toyos-sound-output`): their +//! offers, the one soundserver plays to, and its master. +//! +//! **The first offer soundserver can mix into is the one it plays to**, and +//! every later one is refused: a driver whose controller reaches no output +//! offers nothing, so on a machine with display audio beside an analog codec +//! the analog codec's driver is the one that offers. **Playing to it begins +//! when no stream is open**, so a stream never changes device under it: until +//! then soundserver is the null sink, and the offer waits. +//! +//! The control thread holds everything here but the stream connection, which +//! goes to the mix thread with the ring as [`Attached`]. **A driver that +//! hangs up after soundserver began playing to it ends soundserver** by name; +//! one that hangs up before is forgotten. + +use toyos::ipc::{FrameRx, RxStep}; +use toyos::poller::{Poller, READABLE}; +use toyos::port::Acceptor; +use toyos::shm::SharedMemory; +use toyos::{AsHandle, Connection}; +use toyos_abi::RawHandle; +use toyos_sound_output::{ + FromDriver, Master, Offer, Played, MSG_ACCEPTED, MSG_MASTER_MUTE, MSG_MASTER_STEP, MSG_PLAYED, MSG_REFUSED, + MSG_START, MSG_STOP, +}; + +use crate::backend::{Backend, Completion, Pipeline}; + +/// The widest frame a driver sends: an offer. +const MAX_FRAME: usize = Offer::BYTES; + +/// Connections soundserver holds before their first frame names them. Policy: +/// a machine has a driver per audio function, two on the widest bench. +const MAX_UNNAMED: usize = 8; + +/// An output, handed to the mix thread to play to. +pub(crate) struct Attached { + pub(crate) backend: DriverBackend, + pub(crate) offer: Offer, + pub(crate) period_frames: usize, +} + +/// What a master request comes to on the control thread. +pub(crate) enum Asked { + /// Sent to the driver; its answer comes back through [`Outputs::serve`]. + Forwarded, + /// No output with a master: answered at once. + Unavailable, +} + +/// One event on the outputs, for the control thread to act on. +pub(crate) enum Event { + /// The driver answered a master request. + Master(Option), +} + +enum Phase { + /// No output, and an offer is taken. + Open, + /// An offer accepted and its ring lent: waiting for its stream connection. + Accepted { control: Driver, offer: Offer, period_frames: usize, ring: SharedMemory }, + /// Its stream connection open: waiting for no stream to be open. + Ready { control: Driver, attached: Box }, + /// Played to. `None` where soundserver plays to a device of its own and + /// takes no driver. + Playing(Option, Option), +} + +struct Driver { + conn: Connection, + rx: FrameRx, +} + +/// The port and every connection on it. +pub(crate) struct Outputs { + acceptor: Acceptor, + unnamed: Vec, + phase: Phase, +} + +impl Outputs { + /// `own` is where soundserver plays to a device it drives itself, and so + /// takes no driver. + pub(crate) fn new(acceptor: Acceptor, own: bool) -> Self { + let phase = if own { Phase::Playing(None, None) } else { Phase::Open }; + Self { acceptor, unnamed: Vec::new(), phase } + } + + /// Watch every handle here, with tokens from `base` up. + pub(crate) fn watch(&self, poller: &Poller, base: u64) { + poller.watch(&self.acceptor, READABLE, base); + let control = match &self.phase { + Phase::Accepted { control, .. } | Phase::Ready { control, .. } | Phase::Playing(Some(control), _) => { + Some(control) + } + Phase::Open | Phase::Playing(None, _) => None, + }; + if let Some(control) = control { + poller.watch(&control.conn, READABLE, base + 1); + } + for (i, driver) in self.unnamed.iter().enumerate() { + poller.watch(&driver.conn, READABLE, base + 2 + i as u64); + } + } + + /// The tokens [`Self::watch`] can answer with. + pub(crate) fn tokens(base: u64) -> core::ops::Range { + base..base + 2 + MAX_UNNAMED as u64 + } + + /// Serve every connection `ready` names; the master answers the control + /// thread is to pass on come back in `events`. + pub(crate) fn serve(&mut self, ready: &[u64], base: u64, events: &mut Vec) { + if ready.contains(&base) { + match self.acceptor.accept() { + Ok(conn) if self.unnamed.len() >= MAX_UNNAMED => { + say!("soundserver: refusing an output connection, {MAX_UNNAMED} are unnamed already"); + drop(conn); + } + Ok(conn) => self.unnamed.push(Driver { conn, rx: FrameRx::new() }), + Err(e) => say!("soundserver: output accept failed: {e:?}"), + } + } + if ready.contains(&(base + 1)) { + self.serve_control(events); + } + let unnamed = core::mem::take(&mut self.unnamed); + for (token, mut driver) in (base + 2..).zip(unnamed) { + if !ready.contains(&token) { + self.unnamed.push(driver); + continue; + } + match first_frame(&mut driver) { + None => self.unnamed.push(driver), + Some(Err(why)) => say!("soundserver: an output connection dropped: {why}"), + Some(Ok(FromDriver::Offer(offer))) => self.offered(driver, offer), + Some(Ok(FromDriver::Stream)) => self.streamed(driver), + Some(Ok(_)) => say!("soundserver: an output connection opened on a frame that opens nothing"), + } + } + } + + fn offered(&mut self, driver: Driver, offer: Offer) { + if !matches!(self.phase, Phase::Open) { + say!("soundserver: refusing an output offer ({offer:?}): it plays to another"); + let _ = driver.conn.try_send_bytes(MSG_REFUSED, &[]); + return; + } + let shape = toyos_mixer::period_frames( + offer.periods as usize, + offer.channels as u16, + offer.period_bytes as usize, + offer.rate, + ); + let period_frames = match shape { + Ok(frames) => frames, + Err(why) => { + say!("soundserver: refusing an output offer ({offer:?}): {why}"); + let _ = driver.conn.try_send_bytes(MSG_REFUSED, &[]); + return; + } + }; + let lent = SharedMemory::create(offer.ring_bytes()).and_then(|ring| Ok((ring.share()?, ring))); + let (handle, ring) = match lent { + Ok(lent) => lent, + Err(why) => { + say!("soundserver: refusing an output offer: no ring for it ({why:?})"); + let _ = driver.conn.try_send_bytes(MSG_REFUSED, &[]); + return; + } + }; + if let Err(why) = driver.conn.send_handles([handle]).map_err(|e| format!("{e:?}")).and_then(|()| { + driver.conn.try_send_bytes(MSG_ACCEPTED, &[]).map_err(|e| format!("{e:?}")) + }) { + say!("soundserver: an output offer's driver took no acceptance ({why})"); + return; + } + say!( + "soundserver: accepted an output: {} Hz {} ch, {} periods of {} bytes, master {}", + offer.rate, + offer.channels, + offer.periods, + offer.period_bytes, + match offer.master_bottom { + Some(bottom) => format!("of {} positions", bottom as u32 + 1), + None => String::from("none"), + }, + ); + self.phase = Phase::Accepted { control: driver, offer, period_frames, ring }; + } + + fn streamed(&mut self, stream: Driver) { + match core::mem::replace(&mut self.phase, Phase::Open) { + Phase::Accepted { control, offer, period_frames, ring } => { + let backend = DriverBackend { + stream: stream.conn, + rx: FrameRx::new(), + ring, + periods: offer.periods as usize, + period_bytes: offer.period_bytes as usize, + running: false, + }; + self.phase = Phase::Ready { control, attached: Box::new(Attached { backend, offer, period_frames }) }; + } + other => { + self.phase = other; + say!("soundserver: a stream connection for no accepted output dropped"); + } + } + } + + fn serve_control(&mut self, events: &mut Vec) { + let playing = matches!(self.phase, Phase::Playing(..)); + let control = match &mut self.phase { + Phase::Accepted { control, .. } | Phase::Ready { control, .. } | Phase::Playing(Some(control), _) => control, + Phase::Open | Phase::Playing(None, _) => return, + }; + loop { + let (msg_type, len) = match control.rx.pump(&control.conn) { + RxStep::Idle => return, + RxStep::Frame { msg_type, payload_len } => (msg_type, payload_len), + RxStep::Eof if playing => panic!("soundserver: the output driver soundserver plays to hung up"), + RxStep::Eof => { + say!("soundserver: an accepted output's driver hung up before it was played to"); + self.phase = Phase::Open; + return; + } + RxStep::Malformed => panic!("soundserver: the output driver sent a frame this protocol cannot describe"), + }; + match FromDriver::decode(msg_type, control.rx.payload(len)) { + Some(FromDriver::Master(master)) if playing => events.push(Event::Master(Some(master))), + Some(FromDriver::MasterUnavailable) if playing => events.push(Event::Master(None)), + _ => panic!("soundserver: the output driver sent control frame {msg_type}, which this state has not"), + } + } + } + + /// The output to begin playing to, now that no stream is open. + pub(crate) fn attach(&mut self) -> Option> { + if !matches!(self.phase, Phase::Ready { .. }) { + return None; + } + let Phase::Ready { control, attached } = core::mem::replace(&mut self.phase, Phase::Open) else { + unreachable!("matched as ready"); + }; + self.phase = Phase::Playing(Some(control), attached.offer.master_bottom); + Some(attached) + } + + /// Forward one master request to the output played to, if it has one. + pub(crate) fn ask(&mut self, step: Option) -> Asked { + let Phase::Playing(Some(control), Some(_)) = &self.phase else { return Asked::Unavailable }; + let sent = match step { + Some(delta) => control.conn.try_send_bytes(MSG_MASTER_STEP, &delta.to_le_bytes()), + None => control.conn.try_send_bytes(MSG_MASTER_MUTE, &[]), + }; + if let Err(why) = sent { + panic!("soundserver: the output driver took no master request ({why:?})"); + } + Asked::Forwarded + } +} + +/// The first frame of a connection that has said nothing yet, once it is whole. +fn first_frame(driver: &mut Driver) -> Option> { + match driver.rx.pump(&driver.conn) { + RxStep::Idle => None, + RxStep::Eof => Some(Err("it hung up before its first frame")), + RxStep::Malformed => Some(Err("its first frame is one this protocol cannot describe")), + RxStep::Frame { msg_type, payload_len } => Some( + FromDriver::decode(msg_type, driver.rx.payload(payload_len)).ok_or("its first frame is no message"), + ), + } +} + +/// `played` after `earlier`, as one completion: in play order, so the later +/// report's time clocks every period of the earlier one too. +fn combine(earlier: Option, played: Played) -> Completion { + let Some(earlier) = earlier else { + return Completion { + mask: played.mask, + first_nanos: played.first_nanos, + last_nanos: played.last_nanos, + clocked: played.clocked, + }; + }; + assert_eq!(earlier.mask & played.mask, 0, "soundserver: the output driver reported a period twice"); + Completion { + mask: earlier.mask | played.mask, + first_nanos: earlier.first_nanos, + last_nanos: played.last_nanos, + clocked: earlier.mask.count_ones() + played.clocked, + } +} + +/// The output a driver drives, as the mix thread plays to it: the ring +/// soundserver lent, and the stream connection. +pub(crate) struct DriverBackend { + stream: Connection, + rx: FrameRx<{ Played::BYTES }>, + ring: SharedMemory, + periods: usize, + period_bytes: usize, + running: bool, +} + +impl DriverBackend { + fn engine(&self, msg_type: u32) { + if let Err(why) = self.stream.try_send_bytes(msg_type, &[]) { + panic!("soundserver: the output driver took no {} ({why:?})", if msg_type == MSG_START { "start" } else { "stop" }); + } + } +} + +impl Backend for DriverBackend { + /// The protocol's ring is cyclic: the driver's engine plays it unaided. + fn pipeline(&self) -> Pipeline { + Pipeline::Ring + } + + fn handle(&self) -> RawHandle { + self.stream.as_handle() + } + + fn buffer(&self, idx: usize) -> *mut u8 { + assert!(idx < self.periods, "soundserver: the output has no period {idx}"); + // SAFETY: the ring is `periods * period_bytes` bytes, mapped for as + // long as `self` holds it. + unsafe { self.ring.as_ptr().add(idx * self.period_bytes) } + } + + /// Every report the driver has sent, as one ([`combine`]). + fn completion(&mut self) -> Option { + let mut out: Option = None; + loop { + let len = match self.rx.pump(&self.stream) { + RxStep::Idle => return out, + RxStep::Frame { msg_type: MSG_PLAYED, payload_len } => payload_len, + RxStep::Frame { msg_type, .. } => panic!("soundserver: the output driver sent stream frame {msg_type}"), + RxStep::Eof => panic!("soundserver: the output driver soundserver plays to hung up its stream"), + RxStep::Malformed => panic!("soundserver: the output driver sent a stream frame this protocol cannot describe"), + }; + let played = toyos_sound_output::played(MSG_PLAYED, self.rx.payload(len)) + .unwrap_or_else(|| panic!("soundserver: the output driver's report is no report")); + out = Some(combine(out, played)); + } + } + + /// A cyclic engine replays a period nobody refills: zeroed as it frees, a + /// late soundserver costs silence. + fn released(&mut self, idx: usize) { + // SAFETY: as `buffer`'s, for one period. + unsafe { core::ptr::write_bytes(self.buffer(idx), 0, self.period_bytes) }; + } + + fn submit(&mut self, _idx: usize, _bytes: usize) { + if !self.running { + self.engine(MSG_START); + self.running = true; + } + } + + fn stop(&mut self) { + if self.running { + self.engine(MSG_STOP); + self.running = false; + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Two reports read at one wake: the newer's time clocks the older's + /// periods and its own first, and the older's first interrupt is the + /// wake's. + #[test] + fn reports_read_together_are_clocked_by_the_newest() { + let first = Played { mask: 0b0110, clocked: 1, first_nanos: 10, last_nanos: 10 }; + let second = Played { mask: 0b1000, clocked: 1, first_nanos: 20, last_nanos: 20 }; + let one = combine(None, first); + assert_eq!((one.mask, one.clocked), (0b0110, 1)); + let both = combine(Some(one), second); + assert_eq!((both.mask, both.clocked, both.first_nanos, both.last_nanos), (0b1110, 3, 10, 20)); + } + + #[test] + #[should_panic(expected = "reported a period twice")] + fn a_period_reported_twice_ends_soundserver_by_name() { + let report = Played { mask: 0b1, clocked: 1, first_nanos: 1, last_nanos: 1 }; + combine(Some(combine(None, report)), report); + } +} diff --git a/system/soundserver/src/volume.rs b/system/soundserver/src/volume.rs index b9452f10607..a782b70957d 100644 --- a/system/soundserver/src/volume.rs +++ b/system/soundserver/src/volume.rs @@ -1,17 +1,20 @@ //! The `volume` port: the master level and mute, on the codec. //! -//! The compositor holds the one connector. Each request is set on the device -//! and read back, and the answer is what the device read back, never the -//! intent, sent by one write that does not wait: a peer that will not take it -//! is dropped by name. A machine whose device has no master — virtio-sound, -//! the null sink, an HDA path with no amplifier to set — answers every -//! request unavailable, by name, and no mixer gain stands in for a master. +//! The compositor holds the one connector. Each request goes to the output +//! driver played to, which sets it on the codec and reads it back, and the +//! answer is what the codec read back, never the intent, sent by one write +//! that does not wait: a peer that will not take it is dropped by name. A +//! device with no master — virtio-sound, the null sink, a path with no +//! amplifier to set — answers every request unavailable, by name, and no +//! mixer gain stands in for a master. use toyos::audio::{Volume, VolumeStep, MSG_VOLUME, MSG_VOLUME_MUTE, MSG_VOLUME_STEP, MSG_VOLUME_UNAVAILABLE}; use toyos::ipc::{self, RxStep}; use toyos::Connection; -use crate::hda::{Answer, MasterControl}; +use toyos_sound_output::Master; + +use crate::output::{Asked, Outputs}; pub(crate) struct VolumeClient { pub(crate) conn: Connection, @@ -41,8 +44,9 @@ pub(crate) fn accepted(clients: &mut Vec, conn: Connection) { clients.push(VolumeClient { conn, rx: ipc::FrameRx::new() }); } -/// Serve every request the client has framed; `false` once it is to go. -pub(crate) fn serve(client: &mut VolumeClient, mut master: Option<&mut MasterControl>) -> bool { +/// Serve every request the client has framed; `false` once it is to go. One +/// the output driver answers is answered through [`answer`]. +pub(crate) fn serve(client: &mut VolumeClient, outputs: &mut Outputs) -> bool { loop { let (msg_type, payload_len) = match client.rx.pump(&client.conn) { RxStep::Idle => return true, @@ -53,31 +57,44 @@ pub(crate) fn serve(client: &mut VolumeClient, mut master: Option<&mut MasterCon } RxStep::Frame { msg_type, payload_len } => (msg_type, payload_len), }; - let answer = match classify(msg_type, payload_len) { + let asked = match classify(msg_type, payload_len) { Request::Step => { let step: VolumeStep = ipc::decode_payload(client.rx.payload(payload_len)) .expect("a payload as long as the struct decodes"); - master.as_deref_mut().and_then(|master| master.step(step.delta)) + outputs.ask(Some(step.delta)) } - Request::Mute => master.as_deref_mut().and_then(MasterControl::toggle_mute), + Request::Mute => outputs.ask(None), Request::Violation => { say!("soundserver: volume protocol violation (msg {msg_type}), disconnecting"); return false; } }; - if let Err(e) = send(&client.conn, answer) { + let sent = match asked { + Asked::Forwarded => Ok(()), + Asked::Unavailable => send(&client.conn, None), + }; + if let Err(e) = sent { say!("soundserver: a volume answer was not taken ({e:?}), disconnecting"); return false; } } } -fn send(conn: &Connection, answer: Option) -> Result<(), ipc::TrySendError> { +/// The output driver's answer to the oldest request it was forwarded, +/// to the client holding the port now; a client that will not take it goes. +pub(crate) fn answer(clients: &mut Vec, master: Option) { + let Some(client) = clients.first() else { return }; + if let Err(e) = send(&client.conn, master) { + say!("soundserver: a volume answer was not taken ({e:?}), disconnecting"); + clients.clear(); + } +} + +fn send(conn: &Connection, answer: Option) -> Result<(), ipc::TrySendError> { match answer { - Some(Answer { setting, bottom }) => conn.try_send( - MSG_VOLUME, - &Volume { position: setting.position, bottom, muted: setting.muted as u8, _pad: 0 }, - ), + Some(Master { position, bottom, muted }) => { + conn.try_send(MSG_VOLUME, &Volume { position, bottom, muted: muted as u8, _pad: 0 }) + } None => conn.try_send_bytes(MSG_VOLUME_UNAVAILABLE, &[]), } } diff --git a/tests/checks/audio.rs b/tests/checks/audio.rs index e11bab76dfa..c90e715dcdc 100644 --- a/tests/checks/audio.rs +++ b/tests/checks/audio.rs @@ -30,22 +30,22 @@ pub fn judges_verdict() -> Result<(), String> { }; let next = spawn("test_rs_next"); - let configured = "[ 0.500 soundserver] soundserver: hda path configured in 3 ms\n"; + let configured = "[ 0.500 soundserver] soundserver: playing to the output offered, 44100Hz 2ch, 128 frames/period\n"; let tone = |ended: String| { format!("{configured}{}{}", spawn("test_rs_audio_tone"), session(stats(0, 1), ended)) }; judged("the tone", tone_on_metal, &format!("{}{next}", tone(stats(0, 0))), true)?; judged("a tone short of periods", tone_on_metal, &format!("{}{next}", tone(stats(2, 0))), false)?; judged( - "a tone off no hda path", + "a tone with no output played to", tone_on_metal, &format!("{}{next}", tone(stats(0, 0))).replace(configured, ""), false, )?; judged( - "a tone on the null sink", + "a tone that played before soundserver played to the output", tone_on_metal, - &format!("{}[ 0.600 soundserver] {NULL_SINK}\n{next}", tone(stats(0, 0))), + &format!("{}{configured}{next}", tone(stats(0, 0)).replace(configured, "")), false, )?; // The next job's stream connected before soundserver let the tone's go, so the diff --git a/tests/common/audio.rs b/tests/common/audio.rs index 017edf804f1..ffd381c4033 100644 --- a/tests/common/audio.rs +++ b/tests/common/audio.rs @@ -54,12 +54,24 @@ fn is_soundserver_stats(text: &str, at: usize) -> bool { text[line_start..at].contains("soundserver: wakes=") } -/// The tone client played to its end through the HDA controller soundserver drives -/// itself, soundserver never fell back to the null sink, and no period of it went -/// unfilled. +/// soundserver played to an output driver's device before `job` ran, so the +/// job's stream did not play to the null sink. +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")); + } + Ok(()) +} + +/// 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> { - log.must_say("soundserver: hda path configured in")?; - log.must_not_say(NULL_SINK)?; + on_an_output(log.text(), "test_rs_audio_tone")?; let window = job_window(log.text(), "test_rs_audio_tone")?; let underruns = sum_field(window, "underruns"); if underruns != 0 { @@ -74,8 +86,7 @@ pub fn tone_on_metal(log: &Serial) -> Result<(), String> { /// The resampled tone client played to its end through the HDA controller, at /// a rate the device does not run, and no period of it went unfilled. pub fn resampled_tone_on_metal(log: &Serial) -> Result<(), String> { - log.must_say("soundserver: hda path configured in")?; - log.must_not_say(NULL_SINK)?; + on_an_output(log.text(), "test_rs_audio_tone_resampled")?; let device = rate_after(log.text(), "soundserver: ready, ") .ok_or("soundserver never said the rate it runs the device at")?; let window = job_window(log.text(), "test_rs_audio_tone_resampled")?; @@ -105,8 +116,8 @@ fn rate_after(text: &str, marker: &str) -> Option { digits.parse().ok() } -/// soundserver's own word for the sink it took when the machine has none. -pub const NULL_SINK: &str = "soundserver: no audio device, presenting a null sink"; +/// soundserver's word for beginning to play to an output driver's device. +pub const PLAYING: &str = "soundserver: playing to the output offered"; /// Sum one `soundserver:` counter across every stats window. fn sum_field(serial: &str, key: &str) -> u32 { diff --git a/tests/common/qemu.rs b/tests/common/qemu.rs index fa523def66c..676e4fbe7fc 100644 --- a/tests/common/qemu.rs +++ b/tests/common/qemu.rs @@ -706,9 +706,10 @@ pub enum Profile { /// ===TEST_START=== protocol like any other. [`BootOptions::mute`] takes /// it away for the one test that certifies the T14's literal shape. Metal, - /// [`Profile::Metal`] with an Intel HDA controller behind the unit, the - /// function soundserver drives whole through its claim on the benches. - MetalHda, + /// [`Profile::Metal`] with two Intel HDA controllers behind the unit, the + /// first with no codec and the second with an output codec: a machine + /// whose drivers do not both reach an output, as the AMD bench's do not. + MetalTwoHda, /// [`Profile::Metal`] on a CPU whose CPUID offers no x2APIC, emulated /// whatever the host, so its local APICs run in xAPIC mode: the machine /// an AMD laptop without x2APIC is. @@ -788,7 +789,7 @@ impl Profile { | Self::HeadlessNicBehindPort | Self::HeadlessTwoEdu | Self::Metal - | Self::MetalHda + | Self::MetalTwoHda | Self::MetalNoX2apic | Self::MetalAmdVi | Self::MetalAmdViTestdev @@ -957,9 +958,9 @@ struct Shape { rng: bool, /// A virtio-gpu, which the compositor drives. virtio_gpu: bool, - /// QEMU's ICH9 HDA controller with one output codec behind it, on an - /// audio backend that plays nothing: soundserver's claim, on a machine - /// with no virtio-sound to take first. + /// Two of QEMU's ICH9 HDA controllers, the first with no codec and the + /// second with one output codec, on an audio backend that plays nothing: + /// a machine with no virtio-sound, whose drivers do not both offer. hda: bool, } @@ -1080,7 +1081,7 @@ impl Profile { hda: false, }, Self::MetalNoX2apic => Self::Metal.shape(), - Self::MetalHda => Shape { hda: true, ..Self::Metal.shape() }, + Self::MetalTwoHda => Shape { hda: true, ..Self::Metal.shape() }, Self::MetalAmdVi => Shape { iommu: Some(Unit::AmdVi { intremap: false }), ..Self::Metal.shape() }, Self::HeadlessUsbSpareAmdVi => { Shape { iommu: Some(Unit::AmdVi { intremap: true }), ..Self::HeadlessUsbSpare.shape() } @@ -2368,6 +2369,8 @@ fn qemu_command( qemu.arg("-audiodev") .arg("none,id=hdaout") .arg("-device") + .arg("ich9-intel-hda,id=hdanone") + .arg("-device") .arg("ich9-intel-hda,id=hda0") .arg("-device") .arg("hda-output,audiodev=hdaout,bus=hda0.0"); diff --git a/tests/guest/rust-tests/src/master.rs b/tests/guest/rust-tests/src/master.rs index 04dac97a0a5..d0339d047fe 100644 --- a/tests/guest/rust-tests/src/master.rs +++ b/tests/guest/rust-tests/src/master.rs @@ -1,7 +1,7 @@ //! soundserver's master on its `volume` port, each answer the codec's own //! readback. -use toyos::audio::{Volume, VolumeStep, MSG_VOLUME, MSG_VOLUME_MUTE, MSG_VOLUME_STEP}; +use toyos::audio::{Volume, VolumeStep, MSG_VOLUME, MSG_VOLUME_MUTE, MSG_VOLUME_STEP, MSG_VOLUME_UNAVAILABLE}; pub fn connect() -> toyos::Connection { toyos::endow::service("volume").expect("this job holds soundserver's volume port") @@ -9,6 +9,10 @@ pub fn connect() -> toyos::Connection { fn answer(volume: &toyos::Connection) -> Volume { let header = volume.recv_header().expect("soundserver answers the request"); + assert_ne!( + header.msg_type, MSG_VOLUME_UNAVAILABLE, + "soundserver has no master to set: it plays to no audio device with one" + ); assert_eq!(header.msg_type, MSG_VOLUME, "soundserver answered with message {}", header.msg_type); let read: Volume = volume.recv_payload(&header).expect("the answer is a volume"); println!("master: position {} of {}, muted {}", read.position, read.bottom, read.muted); diff --git a/tests/images/metalcase.toml b/tests/images/metalcase.toml index ab7636b74e0..214b4ed5da9 100644 --- a/tests/images/metalcase.toml +++ b/tests/images/metalcase.toml @@ -32,8 +32,17 @@ devices = ["framebuffer", "keyboard", "mouse"] [programs.soundserver] service = true -serves = ["soundserver", "volume"] -devices = ["pci:04:03:00", "pci:04:01:00/1af4:1059"] +serves = ["soundserver", "volume", "sound-output"] +devices = ["pci:04:01:00/1af4:1059"] +syscap = ["rt"] + +# The Intel HD Audio driver: one process per HDA function the kernel does not +# drive, each walking its own controller's codecs and offering soundserver the +# output it reaches. A machine with display audio beside an analog codec runs +# two, and soundserver plays to the one that offers. +[programs.hda] +drives = ["pci:04:03:*"] +receives = ["sound-output"] syscap = ["rt"] # netstack holds the NIC's PCI function and drives it: the virtqueues, the register diff --git a/tests/images/metaldevicecase.toml b/tests/images/metaldevicecase.toml index b45d12bcfa1..cb08b4f7f6c 100644 --- a/tests/images/metaldevicecase.toml +++ b/tests/images/metaldevicecase.toml @@ -7,9 +7,9 @@ # `exit: pid=N code=N cpu=Nms` record is what carries it off the stick. # # No compositor: the framebuffer jobs claim the display themselves, and a claim -# is minted for one holder. soundserver *is* here, and claims the HDA controller -# without a client: on this machine the controller is real, and its codec walk -# and output-path choice are the only exercise of `toyos-hda` that hardware can +# is minted for one holder. soundserver and the hda driver *are* here, with no +# client: on this machine the controller is real, and its codec walk and +# output-path choice are the only exercise of `toyos-hda` that hardware can # give. [boot] start = ["logkeeper", "diskserver", "fileserver", "soundserver", "test-runner"] @@ -20,8 +20,16 @@ syscap = ["logread"] [programs.soundserver] service = true -serves = ["soundserver", "volume"] -devices = ["pci:04:03:00"] +serves = ["soundserver", "volume", "sound-output"] +syscap = ["rt"] + +# The Intel HD Audio driver: one process per HDA function the kernel does not +# drive, each walking its own controller's codecs and offering soundserver the +# output it reaches. A machine with display audio beside an analog codec runs +# two, and soundserver plays to the one that offers. +[programs.hda] +drives = ["pci:04:03:*"] +receives = ["sound-output"] syscap = ["rt"] # `device` because every framebuffer job mints its own claim and no manifest row diff --git a/tests/images/netcase.toml b/tests/images/netcase.toml index 556f9a2a698..3a21dc8f79f 100644 --- a/tests/images/netcase.toml +++ b/tests/images/netcase.toml @@ -29,7 +29,7 @@ devices = ["pci:02:00:00/1af4:1041"] [programs.soundserver] service = true -serves = ["soundserver", "volume"] +serves = ["soundserver", "volume", "sound-output"] devices = ["pci:04:01:00/1af4:1059"] syscap = ["rt"] diff --git a/tests/images/testcases.toml b/tests/images/testcases.toml index 0935a0a9336..2c141af5c4b 100644 --- a/tests/images/testcases.toml +++ b/tests/images/testcases.toml @@ -25,8 +25,17 @@ receives = ["power"] [programs.soundserver] service = true -serves = ["soundserver", "volume"] -devices = ["pci:04:03:00", "pci:04:01:00/1af4:1059"] +serves = ["soundserver", "volume", "sound-output"] +devices = ["pci:04:01:00/1af4:1059"] +syscap = ["rt"] + +# The Intel HD Audio driver: one process per HDA function the kernel does not +# drive, each walking its own controller's codecs and offering soundserver the +# output it reaches. A machine with display audio beside an analog codec runs +# two, and soundserver plays to the one that offers. +[programs.hda] +drives = ["pci:04:03:*"] +receives = ["sound-output"] syscap = ["rt"] # The test estate's authority. diff --git a/tests/toyos.rs b/tests/toyos.rs index 463979d150d..d2124cd9868 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -165,7 +165,8 @@ const RUST_SKIP: &[&str] = &[ // They need an HDA controller, which the shared boot's machine has not: // the `hda_tone`, `hda_tone_resampled` and `hda_volume_step` metal rows // run them on the T14 and the AMD bench, and - // `hda_drives_its_codec_through_its_claim` runs `volume_step` on QEMU's. + // `hda_plays_to_the_controller_that_reaches_an_output` runs `volume_step` + // on QEMU's. "audio_tone", "audio_tone_resampled", "volume_step", @@ -391,12 +392,14 @@ const MACHINE_TESTS: &[&str] = &[ // spawns, each process holding its own — which no host build has. No // bench has two identical functions the kernel leaves to a claim. "a_drives_row_holds_one_process_per_function", - // soundserver driving an Intel HDA function through its claim behind - // VT-d — reset, CORB/RIRB, the codec walk, the path and the master read - // back — with the master job the benches' rows judge. The tone rows' - // stream is the benches' alone. q35 only: `virt` has no unit to put the - // function behind. - "hda_drives_its_codec_through_its_claim", + // Two HDA controllers, each with an `hda` driver behind VT-d: the one + // with no codec plays nothing, soundserver plays to the other, and the + // master job the benches' rows judge steps that output's codec. Which + // driver offers and which soundserver plays to are three processes and + // two claims, which no host build has. The tone rows' stream is the + // benches' alone. q35 only: `virt` has no unit to put the functions + // behind. + "hda_plays_to_the_controller_that_reaches_an_output", // Whether QEMU's virtio functions negotiated `VIRTIO_F_ACCESS_PLATFORM` // behind its emulated VT-d unit and without one: no shipped machine has a // virtio function, so only a QEMU machine can be asked. @@ -639,7 +642,7 @@ const METAL: &[(&str, metal::Metal)] = &[ arms: TESTCASES, judge: |b| { b[0].job_passed("test_rs_volume_step")?; - b[0].log().must_say("soundserver: hda path configured in").map(|_| ()) + b[0].log().must_say(audio::PLAYING).map(|_| ()) }, }, ), @@ -3891,7 +3894,9 @@ fn run_machine_test(name: &str, test_config: &Path) -> Result<(), String> { claims::NOT_REMAPPED, ), "a_drives_row_holds_one_process_per_function" => a_drives_row_holds_one_process_per_function(), - "hda_drives_its_codec_through_its_claim" => hda_drives_its_codec_through_its_claim(test_config), + "hda_plays_to_the_controller_that_reaches_an_output" => { + hda_plays_to_the_controller_that_reaches_an_output(test_config) + } "usb_keyboard_rollover" => usb_keyboard_rollover(test_config), "vector_state_survives_a_switch" => vector_state_survives_a_switch(test_config), "the_opening_exchange_with_qemus_agent_completes" => the_opening_exchange_with_qemus_agent_completes(), @@ -4250,36 +4255,50 @@ fn a_drives_row_holds_one_process_per_function() -> Result<(), String> { Ok(()) } -/// The benches' master job, in one boot of `tests/images/testcases.toml` on -/// [`qemu::Profile::MetalHda`]: it exits 0 on the HDA path soundserver -/// configured, which a refused codec answer or a stalled ring would stop. +/// Two HDA controllers, the first with no codec and the second with an +/// output one ([`qemu::Profile::MetalTwoHda`]), each with an `hda` driver of +/// its own: the first plays nothing and says why, the second offers its +/// output, and soundserver plays to it. Then the benches' master job on that +/// output: reset, CORB/RIRB, the walk, the path and the master's readback, +/// each behind VT-d and through the claim. /// /// Not the tone jobs: cpal holds a stream's close to a fade and a ring of /// periods at the device's rate, and QEMU's controller completes periods in /// bursts of up to a ring paced by the host, so that bound is a time only the /// benches judge. -fn hda_drives_its_codec_through_its_claim(test_config: &Path) -> Result<(), String> { - const JOBS: [&str; 1] = ["volume_step"]; +fn hda_plays_to_the_controller_that_reaches_an_output(test_config: &Path) -> Result<(), String> { + const JOB: &str = "volume_step"; + const OFFERED: &str = ": path configured in "; let tests = compile::repo_root().join("tests/guest/rust-tests"); - let bins: Vec<(String, Vec)> = - JOBS.iter().map(|job| (job.to_string(), qemu::build_toyos_bin(qemu::SUITE_ARCH, &tests, job))).collect(); - let options = BootOptions { profile: qemu::Profile::MetalHda, ..Default::default() }; - if !qemu::profile_argv(&options).iter().any(|arg| arg.starts_with("ich9-intel-hda")) { - return Err("the machine was to have an Intel HDA controller, and QEMU is given none".into()); + let bins = [(JOB.to_string(), qemu::build_toyos_bin(qemu::SUITE_ARCH, &tests, JOB))]; + let options = BootOptions { profile: qemu::Profile::MetalTwoHda, ..Default::default() }; + let controllers = qemu::profile_argv(&options).iter().filter(|arg| arg.starts_with("ich9-intel-hda")).count(); + if controllers != 2 { + return Err(format!("the machine was to have two Intel HDA controllers, and QEMU is given {controllers}")); } let mut qemu = QemuInstance::boot_with_options(test_config, &[], &bins, options); - let boot = serial::Serial::boot(&qemu); - boot.must_be_clean()?; - // soundserver's walk may end after the ready marker, so the capture is - // the boot's and every job's. - let mut said = boot.text().to_string(); - for job in JOBS { - said += &job_window(&mut qemu, &format!("test_rs_{job}"))?; - } - let said = serial::Serial::named("the HDA boot", said); - said.must_say("soundserver: hda path configured in")?; - said.must_not_say(audio::NULL_SINK)?; + let mut console = format!("{}\n", qemu.boot_log()); + // Both drivers' verdicts and soundserver's: each comes when its walk does, + // which can be after the ready marker. + let decided = |t: &str| t.contains(audio::PLAYING) && t.contains("plays nothing: "); + await_guest(&mut qemu, &mut console, "both drivers' verdicts and soundserver's", decided) + .map_err(|e| format!("{e}\n{console}"))?; + console += &job_window(&mut qemu, &format!("test_rs_{JOB}"))?; + let said = serial::Serial::named("the two-controller boot", console.clone()); + said.must_be_clean()?; said.must_not_say("iommu: DMA FAULT")?; + let driver = |line: &str, what: &str| line.split_once("] hda ")?.1.split_once(what).map(|(at, _)| at.to_string()); + let silent: Vec = console.lines().filter_map(|l| driver(l, ": plays nothing: ")).collect(); + let offered: Vec = console.lines().filter_map(|l| driver(l, OFFERED)).collect(); + let [silent] = silent.as_slice() else { return Err(format!("not one driver played nothing: {silent:?}\n{console}")) }; + let [offered] = offered.as_slice() else { return Err(format!("not one driver offered: {offered:?}\n{console}")) }; + said.must_say(&format!("hda {silent}: plays nothing: the controller refused: no codec is on its link"))?; + if silent > offered { + return Err(format!("the codec-less controller {silent} was to enumerate before {offered}:\n{console}")); + } + if console.matches(audio::PLAYING).count() != 1 { + return Err(format!("soundserver did not play to one output:\n{console}")); + } Ok(()) } From 66776de2f69971bf344c48c6ac1dcbe9bacafc14 Mon Sep 17 00:00:00 2001 From: japabu Date: Tue, 20 Oct 2026 20:10:45 +0200 Subject: [PATCH 2/3] Review round 1: nothing a driver sends ends soundserver; offers and master requests bounded - A reader a ring behind holds reports that overlap ({0..6} then {7, 0..5}); `combine` makes any overlapping, or non-run, union a whole lap, which the mix loop counts as the drain it is. A report that is no run of the ring, a frame the stream protocol has not, or a hang-up now ends the device loop (`Backend::ended`) instead of soundserver: the mix thread ends the streams it held, tells the control thread on a pipe of its own, and soundserver is the null sink again. A control-connection violation drops the driver the same way and sends `MixCommand::Detach`. A stream opened against an output since gone carries that output's generation and is ended where it reaches another's loop. A report read while the engine is stopped is dropped. - `Offer::decode` bounds the ring to the mixer's deepest pipeline (16) and a period to `MAX_PERIOD_BYTES` (16 KiB), before soundserver allocates the lent ring from them. - The volume port forwards one master request at a time (`volume::Relay`): the holder's next is not read, nor its connection watched, until the answer, which goes only to the connection that asked. A driver whose connection takes no request is answered unavailable by name. - Dropped the llvm issue; #952's `issues/the-llvm-environment-test-breaks-git-on-macos.md` is the record. - The `hda` driver's ring comment, `Phase::Playing`'s master as a bool, and the jack-detection track's stale line. Co-Authored-By: Claude Opus 5.5 --- ...-no-jack-detection-and-nothing-persists.md | 7 +- ...t-test-dies-where-git-is-the-xcode-shim.md | 23 --- system/hda/src/main.rs | 5 +- system/soundserver/output/src/lib.rs | 22 ++- system/soundserver/src/backend.rs | 7 + system/soundserver/src/command.rs | 15 +- system/soundserver/src/control.rs | 43 +++- system/soundserver/src/main.rs | 64 +++--- system/soundserver/src/mix.rs | 68 +++++-- system/soundserver/src/output.rs | 185 +++++++++++++----- system/soundserver/src/volume.rs | 108 ++++++++-- 11 files changed, 407 insertions(+), 140 deletions(-) delete mode 100644 issues/the-llvm-environment-test-dies-where-git-is-the-xcode-shim.md diff --git a/issues/hda-has-no-jack-detection-and-nothing-persists.md b/issues/hda-has-no-jack-detection-and-nothing-persists.md index 6697b07e993..e50e2768516 100644 --- a/issues/hda-has-no-jack-detection-and-nothing-persists.md +++ b/issues/hda-has-no-jack-detection-and-nothing-persists.md @@ -22,9 +22,10 @@ set by the driver for soundserver's `volume` port, and the volume keys set them **Two pieces of the shipped work are still owed a verdict.** The four HDA sections of the audio baseline were never recorded, so the HDA arm asserts harm and claims no distribution and has no thorough tier — that is 30 invocations on -a quiet host. And a shape profile is missing: a codec with no speaker pin (a -controller with no codec is refused by name in the controller's host tests). Without them the refusal-to-null-sink path is -asserted only on a machine that has no controller at all. +a quiet host. And a shape profile is missing: a codec with no speaker pin. A +controller with no codec is refused by name, and soundserver stays the null +sink until another offers, on QEMU's two-controller row +(`hda_plays_to_the_controller_that_reaches_an_output`). Constraints worth not re-deriving (each already stated at its site in `system/hda/driver/`, listed here because they are what diff --git a/issues/the-llvm-environment-test-dies-where-git-is-the-xcode-shim.md b/issues/the-llvm-environment-test-dies-where-git-is-the-xcode-shim.md deleted file mode 100644 index a2a91df7cc3..00000000000 --- a/issues/the-llvm-environment-test-dies-where-git-is-the-xcode-shim.md +++ /dev/null @@ -1,23 +0,0 @@ ---- -status: open -kind: tooling -opened: 2026-10-20 ---- - -# The LLVM environment test dies where `git` is Xcode's shim - -`llvm::tests::the_caller_s_environment_reaches_neither_the_build_nor_the_key` -(`build/llvm.rs`) reruns `llvm::tests::keyed_and_built` with `SDKROOT` set to -`/no/such.sdk`, among the other ambient names it proves never reach the LLVM -build. On a macOS host whose `/usr/bin/git` is Xcode's `xcrun` shim with no -standalone command line tools, the child's first `git` (`sysroot.rs`, the -fork's `.gitmodules` read) resolves through that `SDKROOT` and fails: `Failed -to determine realpath of '/no/such.sdk'`, `xcode-select: Failed to locate -'git'`. So `cargo run -- --ci host` reds its build-system step on such a host, -on `origin/main` at `0e6716fc0` as on any branch. - -The test asserts what the *LLVM build* sees; the harness's own `git` reading -the caller's `SDKROOT` is the test's premise breaking, not the defect it -guards. Owner: the build track. Exit: the rerun's own `git` calls run without -the ambient names (or `git` is resolved before they are set), and the test -passes on a host whose `git` is the shim. diff --git a/system/hda/src/main.rs b/system/hda/src/main.rs index 2a80c50aec1..bc311222431 100644 --- a/system/hda/src/main.rs +++ b/system/hda/src/main.rs @@ -149,8 +149,9 @@ struct Driver { to_engine: Connection, underruns: u32, /// Held for their mappings' lives: the register window points into the - /// first, the link and descriptor list into the second, and the device - /// reaches the third while it is mapped. + /// first, and the link and descriptor list into the second. The lent ring + /// stays alive while it is mapped; its handle is kept as the program's + /// own reference to what the device reaches. _bar: SharedMemory, _region: DmaRegion, _ring: OwnedHandle, diff --git a/system/soundserver/output/src/lib.rs b/system/soundserver/output/src/lib.rs index 6036abc7e2b..840b9cd7a1b 100644 --- a/system/soundserver/output/src/lib.rs +++ b/system/soundserver/output/src/lib.rs @@ -24,8 +24,15 @@ pub const PORT: &str = "sound-output"; /// device runs at, and an offer at another is refused. pub use toyos_mixer::DEVICE_RATES as RATES; -/// The deepest ring an offer may name: [`Played::mask`] is a `u32`. -pub const MAX_PERIODS: usize = 32; +/// The deepest ring an offer may name: the mixer's deepest pipeline. +pub const MAX_PERIODS: usize = toyos_mixer::MAX_PIPELINE; + +/// The longest period an offer may name, in bytes: 4096 stereo frames, 93 ms +/// at 44.1 kHz. Policy, not physics: soundserver sizes its mix bus, its +/// scratch and every client's ring from the period, and lends the ring out of +/// its own memory, so a driver's word is bounded here, far past any device's +/// period, before anything is allocated from it. +pub const MAX_PERIOD_BYTES: u32 = 16 * 1024; // The control connection, driver to soundserver. pub const MSG_OFFER: u32 = 1; @@ -91,6 +98,7 @@ impl Offer { && (2..=MAX_PERIODS).contains(&(periods as usize)) && periods.is_power_of_two() && period_bytes != 0 + && period_bytes <= MAX_PERIOD_BYTES && period_bytes.is_multiple_of(frame); whole.then_some(Self { rate, channels, periods, period_bytes, master_bottom }) } @@ -259,7 +267,7 @@ pub enum Run { /// consecutive ones, and those abut. `None` for anything else, which no walk /// of a ring produces. pub fn run(mask: u32, periods: usize) -> Option { - if !(2..=MAX_PERIODS).contains(&periods) || (periods < 32 && mask >> periods != 0) { + if !(2..=MAX_PERIODS).contains(&periods) || mask >> periods != 0 { return None; } let count = mask.count_ones() as usize; @@ -305,7 +313,11 @@ mod tests { refused(Offer { channels: 3, ..OFFER }); refused(Offer { periods: 1, ..OFFER }); refused(Offer { periods: 6, ..OFFER }); - refused(Offer { periods: 64, ..OFFER }); + refused(Offer { periods: 32, ..OFFER }); + refused(Offer { period_bytes: MAX_PERIOD_BYTES + 4, ..OFFER }); + refused(Offer { period_bytes: !3, ..OFFER }); + let widest = Offer { periods: MAX_PERIODS as u8, period_bytes: MAX_PERIOD_BYTES, ..OFFER }; + assert_eq!(FromDriver::decode(MSG_OFFER, &widest.encode()), Some(FromDriver::Offer(widest))); refused(Offer { period_bytes: 0, ..OFFER }); refused(Offer { period_bytes: 514, ..OFFER }); let mut bytes = OFFER.encode(); @@ -351,7 +363,7 @@ mod tests { // Played 6, 7, 0, 1: the run starts at 6, not at its lowest bit. assert_eq!(run(0b1100_0011, 8), Some(Run::Of { first: 6, count: 4 })); assert_eq!(run(0xFF, 8), Some(Run::Lapped)); - assert_eq!(run(u32::MAX, 32), Some(Run::Lapped)); + assert_eq!(run(0xFFFF, 16), Some(Run::Lapped)); } #[test] diff --git a/system/soundserver/src/backend.rs b/system/soundserver/src/backend.rs index 74977b3fff7..be4074d9a9d 100644 --- a/system/soundserver/src/backend.rs +++ b/system/soundserver/src/backend.rs @@ -89,6 +89,13 @@ pub(crate) trait Backend { /// Stop the stream. Idempotent, and cheap when it is already stopped. fn stop(&mut self); + + /// Why the device is no longer played to, once it is not: an output + /// driver that hung up or broke its protocol, which ends the device loop + /// and never soundserver. + fn ended(&self) -> Option<&str> { + None + } } pub(crate) struct VirtioBackend { diff --git a/system/soundserver/src/command.rs b/system/soundserver/src/command.rs index 9abe7d82d2a..6aa961740e3 100644 --- a/system/soundserver/src/command.rs +++ b/system/soundserver/src/command.rs @@ -21,14 +21,19 @@ use crate::output::Attached; use crate::ring::Spsc; /// Deep enough that one pass of the control loop can never fill it: a pass -/// pushes at most one `AddClient` (there is one accept per wait) and one -/// `Attach`, plus, per connected client, one coalesced `SetVolume` and one -/// `RemoveClient`. +/// pushes at most one `AddClient` (there is one accept per wait), one +/// `Detach` and one `Attach`, plus, per connected client, one coalesced +/// `SetVolume` and one `RemoveClient`. const CMD_RING_SIZE: usize = 256; -const _: () = assert!(CMD_RING_SIZE >= 2 + 2 * MAX_CONTROL_CLIENTS); +const _: () = assert!(CMD_RING_SIZE >= 3 + 2 * MAX_CONTROL_CLIENTS); pub(crate) enum MixCommand { - AddClient(Box), + /// A stream, opened against the device of this generation + /// ([`Attached::generation`]): one that reaches another device's loop is + /// ended rather than mixed at a shape it was not opened for. + AddClient(Box, u32), + /// The output driver played to is gone: the device loop ends. + Detach, /// Play to this output from here. Sent only while no stream is open, so /// the streams the null sink still holds are all ramping out; every /// command behind it is the output's. diff --git a/system/soundserver/src/control.rs b/system/soundserver/src/control.rs index c51e3b31794..5a71f0d2280 100644 --- a/system/soundserver/src/control.rs +++ b/system/soundserver/src/control.rs @@ -19,6 +19,7 @@ use toyos::ipc::{self, RxStep}; use toyos::poller::{Poller, READABLE}; use toyos::port::Acceptor; use toyos::Connection; +use toyos_abi::syscall; use toyos_abi::RawHandle; use toyos_mixer::{period_nanos, Gain, MAX_CLIENT_RATE, MIN_CLIENT_RATE}; @@ -124,6 +125,8 @@ fn description(device: &Device) -> DeviceDescription { /// The tokens [`Outputs::watch`] answers with start here, above every other. const TOKEN_OUTPUTS: u64 = 1 << 40; +/// The mix thread ended its device loop on its own finding. +const TOKEN_ENDED: u64 = (1 << 40) - 1; pub(crate) fn control_thread( acceptor: Acceptor, @@ -131,12 +134,13 @@ pub(crate) fn control_thread( mut outputs: Outputs, cmd_ring: &CommandRing, cmd_pipe_write: RawHandle, + ended_read: RawHandle, mut device: Device, published: &Published, ) { // One handle per client plus the acceptor, the volume port's two and the // outputs'. - let watched = MAX_CONTROL_CLIENTS + 1 + 2 + Outputs::tokens(0).count(); + let watched = MAX_CONTROL_CLIENTS + 1 + 2 + 1 + Outputs::tokens(0).count(); let poller = Poller::new(watched as u32); let mut period_nanos = period_nanos(device.period_frames as u64, device.rate as u64); let mut ramp_frames = toyos_mixer::ramp_frames(device.rate); @@ -167,10 +171,16 @@ pub(crate) fn control_thread( const TOKEN_VOLUME: u64 = 1 << 32; let mut events: Vec = Vec::new(); + let mut relay = volume::Relay::default(); + // Which output streams open against: one more at every attach and every + // driver dropped ([`MixCommand::AddClient`]). + let mut generation: u32 = 0; loop { // An output waits for no stream to be open: from here every stream // opens on it. - if let Some(attached) = clients.iter().all(|c| c.stream_idx.is_none()).then(|| outputs.attach()).flatten() { + let quiet = clients.iter().all(|c| c.stream_idx.is_none()); + if let Some(attached) = quiet.then(|| outputs.attach(generation + 1)).flatten() { + generation += 1; device = crate::output_device(&attached.offer, attached.period_frames); period_nanos = toyos_mixer::period_nanos(device.period_frames as u64, device.rate as u64); ramp_frames = toyos_mixer::ramp_frames(device.rate); @@ -178,12 +188,15 @@ pub(crate) fn control_thread( } poller.watch(&acceptor, READABLE, TOKEN_ACCEPT); + poller.watch_raw(ended_read, READABLE, TOKEN_ENDED); outputs.watch(&poller, TOKEN_OUTPUTS); poller.watch(&volume_acceptor, READABLE, TOKEN_VOLUME_ACCEPT); for (i, client) in clients.iter().enumerate() { poller.watch(&client.conn, READABLE, i as u64); } - for (i, client) in volume_clients.iter().enumerate() { + // Not while a request is in flight: its next is not read until then, + // and a connection left readable would wake this loop for nothing. + for (i, client) in volume_clients.iter().enumerate().filter(|_| relay.idle()) { poller.watch(&client.conn, READABLE, TOKEN_VOLUME + i as u64); } @@ -216,21 +229,37 @@ pub(crate) fn control_thread( match volume_acceptor.accept() { // The compositor holds the one connector, so a second // connection is a reconnect and the first is dropped. - Ok(conn) => volume::accepted(&mut volume_clients, conn), + Ok(conn) => volume::accepted(&mut volume_clients, &mut relay, conn), Err(e) => say!("soundserver: volume accept failed: {e:?}"), } } + if ready.contains(&TOKEN_ENDED) { + let mut drain = [0u8; 8]; + while matches!(syscall::read_nonblock(ended_read, &mut drain), Ok(n) if n == drain.len()) {} + outputs.ended(&mut events); + } outputs.serve(&ready, TOKEN_OUTPUTS, &mut events); for event in events.drain(..) { match event { - Event::Master(master) => volume::answer(&mut volume_clients, master), + Event::Master(master) => volume::answer(&mut volume_clients, &mut relay, master), + Event::Gone(why) => { + say!("soundserver: dropped the output driver played to ({why}); the null sink again"); + generation += 1; + device = crate::NULL_SINK; + period_nanos = toyos_mixer::period_nanos(device.period_frames as u64, device.rate as u64); + ramp_frames = toyos_mixer::ramp_frames(device.rate); + submit(cmd_ring, cmd_pipe_write, MixCommand::Detach, period_nanos); + if !relay.idle() { + volume::answer(&mut volume_clients, &mut relay, None); + } + } } } let mut at = TOKEN_VOLUME; volume_clients.retain_mut(|client| { let token = at; at += 1; - !ready.contains(&token) || volume::serve(client, &mut outputs) + !ready.contains(&token) || volume::serve(client, &mut outputs, &mut relay) }); let mut dead: Vec = Vec::new(); @@ -306,7 +335,7 @@ pub(crate) fn control_thread( disconnected = true; break 'msgs; }; - submit(cmd_ring, cmd_pipe_write, MixCommand::AddClient(Box::new(client)), period_nanos); + submit(cmd_ring, cmd_pipe_write, MixCommand::AddClient(Box::new(client), generation), period_nanos); clients[i].stream_idx = Some(idx); } Control::SetVolume { idx } => { diff --git a/system/soundserver/src/main.rs b/system/soundserver/src/main.rs index df7a04459bb..e5e58fb52ae 100644 --- a/system/soundserver/src/main.rs +++ b/system/soundserver/src/main.rs @@ -60,7 +60,7 @@ use output::{Attached, Outputs}; /// first device rate, stereo, 128 frames per period. A stream negotiates /// against them exactly as it would a real device, so a no-hardware machine is /// invisible to the client. -const NULL_SINK: Device = Device { +pub(crate) const NULL_SINK: Device = Device { kind: "null", rate: DEVICE_RATES[0], channels: 2, @@ -123,6 +123,8 @@ fn run(ports: Ports, own: Option<(VirtioBackend, Device)>) { let outputs = Outputs::new(ports.output, own.is_some()); let cmd_ring = Arc::new(CommandRing::new()); let cmd_pipe = syscall::pipe().expect("soundserver: failed to create the command pipe"); + // The mix thread's word that it ended a device loop on its own finding. + let ended = syscall::pipe().expect("soundserver: failed to create the ended pipe"); let initial = if own.is_some() { State::Suspended } else { State::Idle }; let published = Arc::new(Published::new(initial)); @@ -131,29 +133,41 @@ fn run(ports: Ports, own: Option<(VirtioBackend, Device)>) { std::thread::Builder::new() .name("soundserver-ctrl".into()) .spawn(move || { - control_thread(ports.acceptor, ports.volume, outputs, &cmd_ring2, cmd_pipe.write, device, &published2); + control_thread( + ports.acceptor, + ports.volume, + outputs, + &cmd_ring2, + cmd_pipe.write, + ended.read, + device, + &published2, + ); }) .expect("soundserver: failed to spawn control thread"); - match own { - Some((mut backend, device)) => { - let period_bytes = toyos_virtio_sound::PERIOD_BYTES; - serve_device(&mut backend, &cmd_ring, &published, cmd_pipe.read, &device, period_bytes) - } - None => { - say!( - "soundserver: no output yet, presenting a null sink ({}Hz {}ch, {} frames/period, streams discarded)", - device.rate, device.channels, device.period_frames - ); - let Attached { mut backend, offer, period_frames } = - *null_sink_thread(&cmd_ring, &published, cmd_pipe.read, &device); - let device = output_device(&offer, period_frames); - say!( - "soundserver: playing to the output offered, {}Hz {}ch, {} frames/period", - device.rate, device.channels, device.period_frames - ); - serve_device(&mut backend, &cmd_ring, &published, cmd_pipe.read, &device, offer.period_bytes as usize) - } + if let Some((mut backend, device)) = own { + let period_bytes = toyos_virtio_sound::PERIOD_BYTES; + let why = serve_device(&mut backend, &cmd_ring, &published, cmd_pipe.read, &device, period_bytes, 0, ended.write); + panic!("soundserver: its own virtio-sound device loop ended ({why})"); + } + let mut generation = 0; + loop { + say!( + "soundserver: no output yet, presenting a null sink ({}Hz {}ch, {} frames/period, streams discarded)", + device.rate, device.channels, device.period_frames + ); + let attached = null_sink_thread(&cmd_ring, &published, cmd_pipe.read, &device, generation); + let Attached { mut backend, offer, period_frames, generation: playing } = *attached; + let output = output_device(&offer, period_frames); + say!( + "soundserver: playing to the output offered, {}Hz {}ch, {} frames/period", + output.rate, output.channels, output.period_frames + ); + let period_bytes = offer.period_bytes as usize; + let why = serve_device(&mut backend, &cmd_ring, &published, cmd_pipe.read, &output, period_bytes, playing, ended.write); + say!("soundserver: the output is gone ({why})"); + generation = playing + 1; } } @@ -175,7 +189,9 @@ fn serve_device( cmd_pipe_read: toyos_abi::RawHandle, device: &Device, period_bytes: usize, -) { + generation: u32, + ended_write: toyos_abi::RawHandle, +) -> String { say!( "soundserver: ready, {} buffers, {}Hz {}ch, {} bytes/period, {} frames/period", device.buffers, device.rate, device.channels, period_bytes, device.period_frames @@ -191,5 +207,7 @@ fn serve_device( period_bytes, device.period_frames as usize, ramp_frames(device.rate), - ); + generation, + ended_write, + ) } diff --git a/system/soundserver/src/mix.rs b/system/soundserver/src/mix.rs index 9f2b3ad023c..68eb18aac9a 100644 --- a/system/soundserver/src/mix.rs +++ b/system/soundserver/src/mix.rs @@ -102,11 +102,21 @@ fn signal_clients(streams: &mut [ClientStream], ramp_frames: u32) { /// /// It stops at an output to play to and answers it: every command behind it /// is the output's. -fn apply_commands(cmd_ring: &CommandRing, streams: &mut Vec, ramp_frames: u32) -> Option> { +fn apply_commands( + cmd_ring: &CommandRing, + streams: &mut Vec, + ramp_frames: u32, + generation: u32, +) -> Option { while let Some(cmd) = cmd_ring.pop() { match cmd { - MixCommand::Attach(attached) => return Some(attached), - MixCommand::AddClient(client) => { + MixCommand::Attach(attached) => return Some(Change::Attach(attached)), + MixCommand::Detach => return Some(Change::Detach), + MixCommand::AddClient(client, opened_on) if opened_on != generation => { + say!("soundserver: client {} ended: it was opened on an output since gone", client.client_id); + syscall::close(client.signal_write); + } + MixCommand::AddClient(client, _) => { say!("soundserver: client {} connected (id={})", streams.len(), client.client_id); let _ = syscall::write_nonblock(client.signal_write, &[1]); streams.push(*client); @@ -144,6 +154,21 @@ fn retain_active(streams: &mut Vec) { }); } +/// A change of output the command ring carries. +enum Change { + Attach(Box), + Detach, +} + +/// Every stream the device loop held ends with it: its signal pipe closes, so +/// its client sees the stream end, and nothing plays it on. +fn end_streams(streams: &mut Vec) { + for stream in streams.drain(..) { + say!("soundserver: client {} ended with its output", stream.client_id); + syscall::close(stream.signal_write); + } +} + /// How long one wait of the mix loop may last, and the grid point it is armed /// on, `now` being the instant it is armed. **With no client it arms no timer /// at all**, so an audience of zero costs no wake; before the DLL locks there is @@ -204,7 +229,9 @@ pub(crate) fn mix_thread( device_period_bytes: usize, device_period_frames: usize, ramp_frames: u32, -) { + generation: u32, + ended_write: RawHandle, +) -> String { let device_period_samples = device_period_frames * device_channels as usize; let period_nanos = period_nanos(device_period_frames as u64, device_sample_rate as u64); let pipeline = backend.pipeline(); @@ -279,10 +306,14 @@ pub(crate) fn mix_thread( // `RT`-only capability the manifest's `syscap = ["rt"]` row asks the supervisor for, // and soundserver is the only program in the tree that has one. Mixing on // without the band would show up only as glitches, so a refusal is loud. - let rt: SysCap = Endowments::get() - .take(toyos_abi::syscall::SYSCAP_LABEL) - .expect("the manifest declares this program `syscap = [\"rt\"]`"); - rt.enter_rt().expect("an RT capability refused the band it names"); + // Once: the band is the thread's, and the loop runs again on each output. + static RT: std::sync::Once = std::sync::Once::new(); + RT.call_once(|| { + let rt: SysCap = Endowments::get() + .take(toyos_abi::syscall::SYSCAP_LABEL) + .expect("the manifest declares this program `syscap = [\"rt\"]`"); + rt.enter_rt().expect("an RT capability refused the band it names"); + }); let poller = Poller::new(64); let mut mix_f32 = vec![0.0f32; device_period_samples]; @@ -340,8 +371,13 @@ pub(crate) fn mix_thread( let mut drain = [0u8; 64]; while matches!(syscall::read_nonblock(cmd_pipe_read, &mut drain), Ok(n) if n == drain.len()) {} } - if apply_commands(cmd_ring, &mut streams, ramp_frames).is_some() { - panic!("soundserver: an output to play to while it plays to a device"); + match apply_commands(cmd_ring, &mut streams, ramp_frames, generation) { + None => {} + Some(Change::Detach) => { + end_streams(&mut streams); + return String::from("soundserver dropped its driver"); + } + Some(Change::Attach(_)) => panic!("soundserver: an output to play to before the last was detached"), } if !was_streaming && !streams.is_empty() { @@ -351,6 +387,11 @@ pub(crate) fn mix_thread( } let completed = backend.completion(); + if let Some(why) = backend.ended() { + end_streams(&mut streams); + let _ = syscall::write_nonblock(ended_write, &[1]); + return why.to_string(); + } if let Some(rec) = completed { // Read before the record is acted on, because it is what a // *pickup* is measured to: the instant soundserver first held the @@ -672,6 +713,7 @@ pub(crate) fn null_sink_thread( published: &Published, cmd_pipe_read: RawHandle, device: &Device, + generation: u32, ) -> Box { let device_sample_rate = device.rate; let device_channels = device.channels; @@ -741,7 +783,11 @@ pub(crate) fn null_sink_thread( // Nothing is drained past an output to play to: what is behind it is // the output's. if attach.is_none() { - attach = apply_commands(cmd_ring, &mut streams, ramp_frames); + attach = match apply_commands(cmd_ring, &mut streams, ramp_frames, generation) { + Some(Change::Attach(attached)) => Some(attached), + // A driver dropped before it was played to: nothing here to end. + Some(Change::Detach) | None => None, + }; } // Start the grid when the first client of a run connects, and reset the diff --git a/system/soundserver/src/output.rs b/system/soundserver/src/output.rs index ffd63d3a395..560eeb43532 100644 --- a/system/soundserver/src/output.rs +++ b/system/soundserver/src/output.rs @@ -9,9 +9,9 @@ //! then soundserver is the null sink, and the offer waits. //! //! The control thread holds everything here but the stream connection, which -//! goes to the mix thread with the ring as [`Attached`]. **A driver that -//! hangs up after soundserver began playing to it ends soundserver** by name; -//! one that hangs up before is forgotten. +//! goes to the mix thread with the ring as [`Attached`]. **Nothing a driver +//! sends ends soundserver**: one that hangs up or breaks the protocol is +//! dropped by name, and soundserver is the null sink again ([`Event::Gone`]). use toyos::ipc::{FrameRx, RxStep}; use toyos::poller::{Poller, READABLE}; @@ -20,7 +20,7 @@ use toyos::shm::SharedMemory; use toyos::{AsHandle, Connection}; use toyos_abi::RawHandle; use toyos_sound_output::{ - FromDriver, Master, Offer, Played, MSG_ACCEPTED, MSG_MASTER_MUTE, MSG_MASTER_STEP, MSG_PLAYED, MSG_REFUSED, + run, FromDriver, Master, Offer, Played, MSG_ACCEPTED, MSG_MASTER_MUTE, MSG_MASTER_STEP, MSG_PLAYED, MSG_REFUSED, MSG_START, MSG_STOP, }; @@ -38,20 +38,37 @@ pub(crate) struct Attached { pub(crate) backend: DriverBackend, pub(crate) offer: Offer, pub(crate) period_frames: usize, + /// Which output this is, counted by the control thread: a stream opened + /// against another is not mixed on it. + pub(crate) generation: u32, } /// What a master request comes to on the control thread. +#[derive(Debug, PartialEq, Eq)] pub(crate) enum Asked { /// Sent to the driver; its answer comes back through [`Outputs::serve`]. Forwarded, /// No output with a master: answered at once. Unavailable, + /// The driver's connection took no request: answered unavailable, by name. + Refused(String), } /// One event on the outputs, for the control thread to act on. pub(crate) enum Event { /// The driver answered a master request. Master(Option), + /// The driver played to is dropped, for this reason: soundserver is the + /// null sink again. + Gone(String), +} + +/// What sending a request to the driver came to. +pub(crate) fn asked(sent: Result<(), E>) -> Asked { + match sent { + Ok(()) => Asked::Forwarded, + Err(why) => Asked::Refused(format!("the output driver took no master request ({why:?})")), + } } enum Phase { @@ -61,9 +78,9 @@ enum Phase { Accepted { control: Driver, offer: Offer, period_frames: usize, ring: SharedMemory }, /// Its stream connection open: waiting for no stream to be open. Ready { control: Driver, attached: Box }, - /// Played to. `None` where soundserver plays to a device of its own and - /// takes no driver. - Playing(Option, Option), + /// Played to, and whether its path has a master. `None` where + /// soundserver plays to a device of its own and takes no driver. + Playing(Option, bool), } struct Driver { @@ -82,7 +99,7 @@ impl Outputs { /// `own` is where soundserver plays to a device it drives itself, and so /// takes no driver. pub(crate) fn new(acceptor: Acceptor, own: bool) -> Self { - let phase = if own { Phase::Playing(None, None) } else { Phase::Open }; + let phase = if own { Phase::Playing(None, false) } else { Phase::Open }; Self { acceptor, unnamed: Vec::new(), phase } } @@ -199,8 +216,10 @@ impl Outputs { periods: offer.periods as usize, period_bytes: offer.period_bytes as usize, running: false, + ended: None, }; - self.phase = Phase::Ready { control, attached: Box::new(Attached { backend, offer, period_frames }) }; + let attached = Box::new(Attached { backend, offer, period_frames, generation: 0 }); + self.phase = Phase::Ready { control, attached }; } other => { self.phase = other; @@ -219,45 +238,58 @@ impl Outputs { let (msg_type, len) = match control.rx.pump(&control.conn) { RxStep::Idle => return, RxStep::Frame { msg_type, payload_len } => (msg_type, payload_len), - RxStep::Eof if playing => panic!("soundserver: the output driver soundserver plays to hung up"), - RxStep::Eof => { - say!("soundserver: an accepted output's driver hung up before it was played to"); - self.phase = Phase::Open; - return; + RxStep::Eof => return self.drop_driver("it hung up".into(), events), + RxStep::Malformed => { + return self.drop_driver("it sent a frame this protocol cannot describe".into(), events); } - RxStep::Malformed => panic!("soundserver: the output driver sent a frame this protocol cannot describe"), }; match FromDriver::decode(msg_type, control.rx.payload(len)) { Some(FromDriver::Master(master)) if playing => events.push(Event::Master(Some(master))), Some(FromDriver::MasterUnavailable) if playing => events.push(Event::Master(None)), - _ => panic!("soundserver: the output driver sent control frame {msg_type}, which this state has not"), + _ => return self.drop_driver(format!("it sent control frame {msg_type}, which this state has not"), events), + } + } + } + + /// Drop the driver this phase holds; where it was played to, the control + /// thread is told it is [`Event::Gone`]. + fn drop_driver(&mut self, why: String, events: &mut Vec) { + match core::mem::replace(&mut self.phase, Phase::Open) { + Phase::Playing(Some(_), _) => events.push(Event::Gone(why)), + Phase::Accepted { .. } | Phase::Ready { .. } => { + say!("soundserver: an accepted output's driver dropped before it was played to: {why}"); } + other => self.phase = other, + } + } + + /// The mix thread found the driver played to gone: drop it here too. + pub(crate) fn ended(&mut self, events: &mut Vec) { + if matches!(self.phase, Phase::Playing(Some(_), _)) { + self.drop_driver("the mix thread ended its device loop".into(), events); } } /// The output to begin playing to, now that no stream is open. - pub(crate) fn attach(&mut self) -> Option> { + pub(crate) fn attach(&mut self, generation: u32) -> Option> { if !matches!(self.phase, Phase::Ready { .. }) { return None; } - let Phase::Ready { control, attached } = core::mem::replace(&mut self.phase, Phase::Open) else { + let Phase::Ready { control, mut attached } = core::mem::replace(&mut self.phase, Phase::Open) else { unreachable!("matched as ready"); }; - self.phase = Phase::Playing(Some(control), attached.offer.master_bottom); + self.phase = Phase::Playing(Some(control), attached.offer.master_bottom.is_some()); + attached.generation = generation; Some(attached) } /// Forward one master request to the output played to, if it has one. pub(crate) fn ask(&mut self, step: Option) -> Asked { - let Phase::Playing(Some(control), Some(_)) = &self.phase else { return Asked::Unavailable }; - let sent = match step { + let Phase::Playing(Some(control), true) = &self.phase else { return Asked::Unavailable }; + asked(match step { Some(delta) => control.conn.try_send_bytes(MSG_MASTER_STEP, &delta.to_le_bytes()), None => control.conn.try_send_bytes(MSG_MASTER_MUTE, &[]), - }; - if let Err(why) = sent { - panic!("soundserver: the output driver took no master request ({why:?})"); - } - Asked::Forwarded + }) } } @@ -273,9 +305,15 @@ fn first_frame(driver: &mut Driver) -> Option> } } -/// `played` after `earlier`, as one completion: in play order, so the later -/// report's time clocks every period of the earlier one too. -fn combine(earlier: Option, played: Played) -> Completion { +/// `played` after `earlier`, as one completion of a `periods`-deep ring: in +/// play order, so the later report's time clocks every period of the earlier +/// one too. +/// +/// **A reader a ring behind holds reports that overlap**, `{0..6}` then +/// `{7, 0..5}`: the engine has been round since the first, so the union places +/// it nowhere. That is a whole lap, every period, which the mix loop counts as +/// the drain it is — never a protocol error. +fn combine(earlier: Option, played: Played, periods: usize) -> Completion { let Some(earlier) = earlier else { return Completion { mask: played.mask, @@ -284,9 +322,18 @@ fn combine(earlier: Option, played: Played) -> Completion { clocked: played.clocked, }; }; - assert_eq!(earlier.mask & played.mask, 0, "soundserver: the output driver reported a period twice"); + let union = earlier.mask | played.mask; + if earlier.mask & played.mask != 0 || run(union, periods).is_none() { + let lap = (1u32 << periods) - 1; + return Completion { + mask: lap, + first_nanos: earlier.first_nanos, + last_nanos: played.last_nanos, + clocked: periods as u32, + }; + } Completion { - mask: earlier.mask | played.mask, + mask: union, first_nanos: earlier.first_nanos, last_nanos: played.last_nanos, clocked: earlier.mask.count_ones() + played.clocked, @@ -295,6 +342,10 @@ fn combine(earlier: Option, played: Played) -> Completion { /// The output a driver drives, as the mix thread plays to it: the ring /// soundserver lent, and the stream connection. +/// +/// **Nothing the driver sends ends soundserver.** A frame this protocol does +/// not have, a report that is no run of the ring, or a hang-up ends the device +/// loop ([`Backend::ended`]); soundserver is the null sink again. pub(crate) struct DriverBackend { stream: Connection, rx: FrameRx<{ Played::BYTES }>, @@ -302,14 +353,20 @@ pub(crate) struct DriverBackend { periods: usize, period_bytes: usize, running: bool, + ended: Option, } impl DriverBackend { - fn engine(&self, msg_type: u32) { + fn engine(&mut self, msg_type: u32) { if let Err(why) = self.stream.try_send_bytes(msg_type, &[]) { - panic!("soundserver: the output driver took no {} ({why:?})", if msg_type == MSG_START { "start" } else { "stop" }); + let what = if msg_type == MSG_START { "start" } else { "stop" }; + self.end(format!("the output driver took no {what} ({why:?})")); } } + + fn end(&mut self, why: String) { + self.ended.get_or_insert(why); + } } impl Backend for DriverBackend { @@ -329,21 +386,36 @@ impl Backend for DriverBackend { unsafe { self.ring.as_ptr().add(idx * self.period_bytes) } } - /// Every report the driver has sent, as one ([`combine`]). + /// Every report the driver has sent, as one ([`combine`]). One read while + /// the engine is stopped is a period the stop overtook, and is dropped: + /// a stopped ring is soundserver's whole. fn completion(&mut self) -> Option { let mut out: Option = None; - loop { + while self.ended.is_none() { let len = match self.rx.pump(&self.stream) { - RxStep::Idle => return out, + RxStep::Idle => break, RxStep::Frame { msg_type: MSG_PLAYED, payload_len } => payload_len, - RxStep::Frame { msg_type, .. } => panic!("soundserver: the output driver sent stream frame {msg_type}"), - RxStep::Eof => panic!("soundserver: the output driver soundserver plays to hung up its stream"), - RxStep::Malformed => panic!("soundserver: the output driver sent a stream frame this protocol cannot describe"), + RxStep::Frame { msg_type, .. } => { + self.end(format!("the output driver sent stream frame {msg_type}, which this protocol has not")); + break; + } + RxStep::Eof => { + self.end("the output driver hung up its stream".into()); + break; + } + RxStep::Malformed => { + self.end("the output driver sent a stream frame this protocol cannot describe".into()); + break; + } }; - let played = toyos_sound_output::played(MSG_PLAYED, self.rx.payload(len)) - .unwrap_or_else(|| panic!("soundserver: the output driver's report is no report")); - out = Some(combine(out, played)); + let played = toyos_sound_output::played(MSG_PLAYED, self.rx.payload(len)); + match played.filter(|played| run(played.mask, self.periods).is_some()) { + Some(played) if self.running => out = Some(combine(out, played, self.periods)), + Some(_) => {} + None => self.end(format!("the output driver reported no run of its {}-period ring", self.periods)), + } } + if self.ended.is_some() { None } else { out } } /// A cyclic engine replays a period nobody refills: zeroed as it frees, a @@ -366,29 +438,42 @@ impl Backend for DriverBackend { self.running = false; } } + + fn ended(&self) -> Option<&str> { + self.ended.as_deref() + } } #[cfg(test)] mod tests { use super::*; + fn report(mask: u32, at: u64) -> Played { + Played { mask, clocked: 1, first_nanos: at, last_nanos: at } + } + /// Two reports read at one wake: the newer's time clocks the older's /// periods and its own first, and the older's first interrupt is the /// wake's. #[test] fn reports_read_together_are_clocked_by_the_newest() { - let first = Played { mask: 0b0110, clocked: 1, first_nanos: 10, last_nanos: 10 }; - let second = Played { mask: 0b1000, clocked: 1, first_nanos: 20, last_nanos: 20 }; - let one = combine(None, first); + let one = combine(None, report(0b0110, 10), 8); assert_eq!((one.mask, one.clocked), (0b0110, 1)); - let both = combine(Some(one), second); + let both = combine(Some(one), report(0b1000, 20), 8); assert_eq!((both.mask, both.clocked, both.first_nanos, both.last_nanos), (0b1110, 3, 10, 20)); } + /// A reader a ring behind: `{0..6}` then `{7, 0..5}` overlap, a lap then + /// anything overlaps, and a union that skips a period is no run. Each is a + /// whole lap, clocked by the newest report, and never a refusal. #[test] - #[should_panic(expected = "reported a period twice")] - fn a_period_reported_twice_ends_soundserver_by_name() { - let report = Played { mask: 0b1, clocked: 1, first_nanos: 1, last_nanos: 1 }; - combine(Some(combine(None, report)), report); + fn reports_a_lagging_reader_holds_are_a_lap() { + let lap = |c: Completion| (c.mask, c.clocked, c.first_nanos, c.last_nanos); + let first = combine(None, report(0b0111_1111, 10), 8); + assert_eq!(lap(combine(Some(first), report(0b1011_1111, 20), 8)), (0xFF, 8, 10, 20)); + let lapped = combine(None, report(0xFF, 10), 8); + assert_eq!(lap(combine(Some(lapped), report(0b1, 20), 8)), (0xFF, 8, 10, 20)); + let low = combine(None, report(0b0011, 10), 8); + assert_eq!(lap(combine(Some(low), report(0b11_0000, 20), 8)), (0xFF, 8, 10, 20)); } } diff --git a/system/soundserver/src/volume.rs b/system/soundserver/src/volume.rs index a782b70957d..c2dfddbddbd 100644 --- a/system/soundserver/src/volume.rs +++ b/system/soundserver/src/volume.rs @@ -19,6 +19,42 @@ use crate::output::{Asked, Outputs}; pub(crate) struct VolumeClient { pub(crate) conn: Connection, rx: ipc::FrameRx<{ core::mem::size_of::() }>, + /// Which connection this is, for [`Relay`]: a reconnect is another. + id: u64, +} + +/// The one master request forwarded to the output driver and not yet +/// answered, and whose it is. +/// +/// **One at a time**: a holder's next request is not read until its last is +/// answered, so a holder writing faster than the codec answers fills its own +/// connection and never the driver's. **And an answer goes to the connection +/// that asked**: one whose asker reconnected since is dropped. +#[derive(Default)] +pub(crate) struct Relay { + in_flight: Option, + next_id: u64, +} + +impl Relay { + /// Whether a request may be read now. + pub(crate) fn idle(&self) -> bool { + self.in_flight.is_none() + } + + fn forwarded(&mut self, by: u64) { + assert!(self.in_flight.replace(by).is_none(), "soundserver: two master requests in flight"); + } + + /// The asker the answer that just came is owed to, if one is in flight. + fn answered(&mut self) -> Option { + self.in_flight.take() + } + + fn mint(&mut self) -> u64 { + self.next_id += 1; + self.next_id + } } /// What one framed request asks for, decided before anything is done. @@ -39,15 +75,15 @@ fn classify(msg_type: u32, payload_len: usize) -> Request { /// A new connection: one holder at a time, so one before it is a holder that /// reconnected, and it goes. -pub(crate) fn accepted(clients: &mut Vec, conn: Connection) { +pub(crate) fn accepted(clients: &mut Vec, relay: &mut Relay, conn: Connection) { clients.clear(); - clients.push(VolumeClient { conn, rx: ipc::FrameRx::new() }); + clients.push(VolumeClient { conn, rx: ipc::FrameRx::new(), id: relay.mint() }); } -/// Serve every request the client has framed; `false` once it is to go. One -/// the output driver answers is answered through [`answer`]. -pub(crate) fn serve(client: &mut VolumeClient, outputs: &mut Outputs) -> bool { - loop { +/// Serve the requests the client has framed, up to one the output driver +/// answers, which [`answer`] passes on; `false` once it is to go. +pub(crate) fn serve(client: &mut VolumeClient, outputs: &mut Outputs, relay: &mut Relay) -> bool { + while relay.idle() { let (msg_type, payload_len) = match client.rx.pump(&client.conn) { RxStep::Idle => return true, RxStep::Eof => return false, @@ -70,20 +106,33 @@ pub(crate) fn serve(client: &mut VolumeClient, outputs: &mut Outputs) -> bool { } }; let sent = match asked { - Asked::Forwarded => Ok(()), + Asked::Forwarded => { + relay.forwarded(client.id); + Ok(()) + } Asked::Unavailable => send(&client.conn, None), + Asked::Refused(why) => { + say!("soundserver: a master request answered unavailable: {why}"); + send(&client.conn, None) + } }; if let Err(e) = sent { say!("soundserver: a volume answer was not taken ({e:?}), disconnecting"); return false; } } + true } -/// The output driver's answer to the oldest request it was forwarded, -/// to the client holding the port now; a client that will not take it goes. -pub(crate) fn answer(clients: &mut Vec, master: Option) { - let Some(client) = clients.first() else { return }; +/// The output driver's answer to the request in flight, to the connection +/// that asked it, where that is still the holder; a holder that will not take +/// it goes. +pub(crate) fn answer(clients: &mut Vec, relay: &mut Relay, master: Option) { + let Some(asker) = relay.answered() else { + say!("soundserver: the output driver answered a master request nobody made"); + return; + }; + let Some(client) = clients.first().filter(|client| client.id == asker) else { return }; if let Err(e) = send(&client.conn, master) { say!("soundserver: a volume answer was not taken ({e:?}), disconnecting"); clients.clear(); @@ -103,6 +152,43 @@ fn send(conn: &Connection, answer: Option) -> Result<(), ipc::TrySendErr mod tests { use super::*; + /// A flood: one request in flight, and the next read only once it is + /// answered, however many the holder wrote. + #[test] + fn one_master_request_is_in_flight_at_a_time() { + let mut relay = Relay::default(); + let holder = relay.mint(); + assert!(relay.idle()); + relay.forwarded(holder); + for _ in 0..1000 { + assert!(!relay.idle(), "a second request read with one in flight"); + } + assert_eq!(relay.answered(), Some(holder)); + assert!(relay.idle()); + assert_eq!(relay.answered(), None, "an answer nobody asked for is owed to nobody"); + } + + /// A holder that reconnected while its request was in flight: the answer + /// is the old connection's, and the new one is never handed it. + #[test] + fn an_answer_goes_only_to_the_connection_that_asked() { + let mut relay = Relay::default(); + let old = relay.mint(); + relay.forwarded(old); + let new = relay.mint(); + assert_ne!(old, new); + assert_eq!(relay.answered(), Some(old)); + assert!(relay.idle(), "the new holder's requests are read once the old one's is settled"); + } + + /// A driver whose connection takes no request is a refusal by name, + /// answered unavailable, never a panic. + #[test] + fn a_driver_that_takes_no_request_is_a_named_refusal() { + assert_eq!(crate::output::asked::<()>(Ok(())), Asked::Forwarded); + assert!(matches!(crate::output::asked(Err("full")), Asked::Refused(why) if why.contains("took no master request"))); + } + #[test] fn a_volume_request_is_refused_unless_its_type_and_width_fit() { const STEP: usize = core::mem::size_of::(); From 16bb9e74be097c711592906529f34d1e3face95d Mon Sep 17 00:00:00 2001 From: japabu Date: Tue, 20 Oct 2026 20:27:55 +0200 Subject: [PATCH 3/3] Review round 2: the offer's answer is awaited with a failure timeout; reports carry their run - The `hda` driver waits for soundserver's answer to its offer through its poller, for at most `OFFER_ANSWER` (30 s, a hang ceiling: soundserver answers in the pass that reads the offer). Past it the driver ends by name and gives the controller back. - `MSG_START` names a run and every `Played` echoes it; soundserver drops a report of another run than its engine's now, so one the driver sent before reading a stop cannot be taken as the next run's. - Deleted the relay tests that could not fail for the defects they were named after; the one-in-flight rule and the asker filter are `serve`, `answer` and the control loop's watch. Co-Authored-By: Claude Opus 5.5 --- system/hda/src/main.rs | 28 +++++++++++++++++++-- system/soundserver/output/src/lib.rs | 20 ++++++++++----- system/soundserver/src/output.rs | 24 +++++++++++------- system/soundserver/src/volume.rs | 37 ---------------------------- 4 files changed, 55 insertions(+), 54 deletions(-) diff --git a/system/hda/src/main.rs b/system/hda/src/main.rs index bc311222431..a567165f4c2 100644 --- a/system/hda/src/main.rs +++ b/system/hda/src/main.rs @@ -147,6 +147,8 @@ struct Driver { master: Option<(Master, Setting)>, control: Connection, to_engine: Connection, + /// The run soundserver's last start named, which every report carries. + run: u32, underruns: u32, /// Held for their mappings' lives: the register window points into the /// first, and the link and descriptor list into the second. The lent ring @@ -279,6 +281,7 @@ impl Driver { master, control, to_engine, + run: 0, underruns: 0, _bar: bar_mapping, _region: region, @@ -302,7 +305,10 @@ impl Driver { self.interrupts(); while let Some((msg_type, payload)) = frame(&mut stream_rx, &self.to_engine, &self.at, "stream") { match Engine::decode(msg_type, &payload) { - Some(Engine::Start) => self.stream.start(), + Some(Engine::Start(run)) => { + self.run = run; + self.stream.start() + } Some(Engine::Stop) => self.stream.stop(), None => panic!("{}: soundserver sent stream frame {msg_type}, which this protocol has not", self.at), } @@ -347,7 +353,13 @@ impl Driver { if played.mask == 0 { return; } - let report = Played { mask: played.mask, clocked: 1, first_nanos: record.first_nanos, last_nanos: record.last_nanos }; + let report = Played { + mask: played.mask, + clocked: 1, + first_nanos: record.first_nanos, + last_nanos: record.last_nanos, + run: self.run, + }; if let Err(why) = self.to_engine.send_bytes(MSG_PLAYED, &report.encode()) { soundserver_gone(&self.at, why); } @@ -402,10 +414,22 @@ impl Driver { } } +/// How long soundserver has to answer the offer: a hang ceiling, since it +/// answers in the control-thread pass that reads it. Past it the controller is +/// given back rather than held by a driver waiting on nothing. +const OFFER_ANSWER: Duration = Duration::from_secs(30); + /// Offer `offer` on `control` and take soundserver's answer: the ring it lends. fn offered(control: &Connection, offer: &Offer) -> Result { let refused = |why: String| Refusal::Soundserver(why); control.send_bytes(MSG_OFFER, &offer.encode()).map_err(|why| refused(format!("the offer was refused ({why:?})")))?; + let poller = Poller::new(1); + poller.watch(control, READABLE, TOKEN_CONTROL); + let mut answered = false; + poller.wait(1, OFFER_ANSWER.as_nanos() as u64, |_| answered = true); + if !answered { + return Err(refused(format!("it did not answer the offer within {OFFER_ANSWER:?}"))); + } let header = control.recv_header().map_err(|why| refused(format!("no answer to the offer ({why:?})")))?; let mut payload = [0u8; MAX_REQUEST]; let n = control diff --git a/system/soundserver/output/src/lib.rs b/system/soundserver/output/src/lib.rs index 840b9cd7a1b..82595d4cce1 100644 --- a/system/soundserver/output/src/lib.rs +++ b/system/soundserver/output/src/lib.rs @@ -134,6 +134,10 @@ impl Master { /// Periods the device played, and when its interrupts said so. #[derive(Clone, Copy, PartialEq, Eq, Debug)] pub struct Played { + /// The [`Engine::Start`] this report's engine run began with: one sent + /// before a stop is read and landing after the next start is that run's + /// only by its number. + pub run: u32, /// One bit per period, one contiguous run of the ring ([`run`]). pub mask: u32, /// How many of these periods, in play order, end by [`Self::last_nanos`]; @@ -145,7 +149,7 @@ pub struct Played { } impl Played { - pub const BYTES: usize = 24; + pub const BYTES: usize = 28; pub fn encode(&self) -> [u8; Self::BYTES] { let mut out = [0u8; Self::BYTES]; @@ -153,6 +157,7 @@ impl Played { out[4..8].copy_from_slice(&self.clocked.to_le_bytes()); out[8..16].copy_from_slice(&self.first_nanos.to_le_bytes()); out[16..24].copy_from_slice(&self.last_nanos.to_le_bytes()); + out[24..28].copy_from_slice(&self.run.to_le_bytes()); out } @@ -163,6 +168,7 @@ impl Played { clocked: u32::from_le_bytes(bytes[4..8].try_into().ok()?), first_nanos: u64::from_le_bytes(bytes[8..16].try_into().ok()?), last_nanos: u64::from_le_bytes(bytes[16..24].try_into().ok()?), + run: u32::from_le_bytes(bytes[24..28].try_into().ok()?), }; let whole = played.mask != 0 && played.clocked <= played.mask.count_ones() @@ -223,14 +229,15 @@ impl ToDriver { /// What soundserver says on a stream connection. #[derive(Clone, Copy, PartialEq, Eq, Debug)] pub enum Engine { - Start, + /// Run the engine; every report of this run carries the number. + Start(u32), Stop, } impl Engine { pub fn decode(msg_type: u32, payload: &[u8]) -> Option { match (msg_type, payload.len()) { - (MSG_START, 0) => Some(Self::Start), + (MSG_START, 4) => Some(Self::Start(u32::from_le_bytes(payload.try_into().ok()?))), (MSG_STOP, 0) => Some(Self::Stop), _ => None, } @@ -295,11 +302,11 @@ mod tests { assert_eq!(FromDriver::decode(MSG_MASTER, &master.encode()), Some(FromDriver::Master(master))); assert_eq!(FromDriver::decode(MSG_STREAM, &[]), Some(FromDriver::Stream)); assert_eq!(FromDriver::decode(MSG_MASTER_UNAVAILABLE, &[]), Some(FromDriver::MasterUnavailable)); - let played = Played { mask: 0b1100_0001, clocked: 1, first_nanos: 5, last_nanos: 9 }; + let played = Played { mask: 0b1100_0001, clocked: 1, first_nanos: 5, last_nanos: 9, run: 3 }; assert_eq!(super::played(MSG_PLAYED, &played.encode()), Some(played)); assert_eq!(ToDriver::decode(MSG_MASTER_STEP, &(-3i32).to_le_bytes()), Some(ToDriver::MasterStep(-3))); assert_eq!(ToDriver::decode(MSG_ACCEPTED, &[]), Some(ToDriver::Accepted)); - assert_eq!(Engine::decode(MSG_START, &[]), Some(Engine::Start)); + assert_eq!(Engine::decode(MSG_START, &7u32.to_le_bytes()), Some(Engine::Start(7))); assert_eq!(Engine::decode(MSG_STOP, &[]), Some(Engine::Stop)); } @@ -333,7 +340,7 @@ mod tests { fn a_master_or_a_report_no_device_produces_is_refused() { assert_eq!(FromDriver::decode(MSG_MASTER, &[9, 8, 0]), None, "past the bottom"); assert_eq!(FromDriver::decode(MSG_MASTER, &[0, 8, 2]), None, "muted is no bool"); - let played = Played { mask: 0b11, clocked: 1, first_nanos: 5, last_nanos: 9 }; + let played = Played { mask: 0b11, clocked: 1, first_nanos: 5, last_nanos: 9, run: 3 }; let refused = |p: Played| assert_eq!(super::played(MSG_PLAYED, &p.encode()), None, "{p:?}"); refused(Played { mask: 0, clocked: 0, ..played }); refused(Played { clocked: 3, ..played }); @@ -354,6 +361,7 @@ mod tests { } assert_eq!(ToDriver::decode(MSG_MASTER_STEP, &[1, 2, 3]), None); assert_eq!(Engine::decode(MSG_START, &[0]), None); + assert_eq!(Engine::decode(MSG_START, &[]), None, "a start names its run"); } #[test] diff --git a/system/soundserver/src/output.rs b/system/soundserver/src/output.rs index 560eeb43532..5891fd26ed4 100644 --- a/system/soundserver/src/output.rs +++ b/system/soundserver/src/output.rs @@ -216,6 +216,7 @@ impl Outputs { periods: offer.periods as usize, period_bytes: offer.period_bytes as usize, running: false, + run: 0, ended: None, }; let attached = Box::new(Attached { backend, offer, period_frames, generation: 0 }); @@ -353,12 +354,14 @@ pub(crate) struct DriverBackend { periods: usize, period_bytes: usize, running: bool, + /// The engine run the last start began, which a report has to carry. + run: u32, ended: Option, } impl DriverBackend { - fn engine(&mut self, msg_type: u32) { - if let Err(why) = self.stream.try_send_bytes(msg_type, &[]) { + fn engine(&mut self, msg_type: u32, payload: &[u8]) { + if let Err(why) = self.stream.try_send_bytes(msg_type, payload) { let what = if msg_type == MSG_START { "start" } else { "stop" }; self.end(format!("the output driver took no {what} ({why:?})")); } @@ -386,9 +389,9 @@ impl Backend for DriverBackend { unsafe { self.ring.as_ptr().add(idx * self.period_bytes) } } - /// Every report the driver has sent, as one ([`combine`]). One read while - /// the engine is stopped is a period the stop overtook, and is dropped: - /// a stopped ring is soundserver's whole. + /// Every report the driver has sent, as one ([`combine`]). One of another + /// run than the engine's now, or read while it is stopped, is a period a + /// stop overtook, and is dropped: a stopped ring is soundserver's whole. fn completion(&mut self) -> Option { let mut out: Option = None; while self.ended.is_none() { @@ -410,7 +413,9 @@ impl Backend for DriverBackend { }; let played = toyos_sound_output::played(MSG_PLAYED, self.rx.payload(len)); match played.filter(|played| run(played.mask, self.periods).is_some()) { - Some(played) if self.running => out = Some(combine(out, played, self.periods)), + Some(played) if self.running && played.run == self.run => { + out = Some(combine(out, played, self.periods)); + } Some(_) => {} None => self.end(format!("the output driver reported no run of its {}-period ring", self.periods)), } @@ -427,14 +432,15 @@ impl Backend for DriverBackend { fn submit(&mut self, _idx: usize, _bytes: usize) { if !self.running { - self.engine(MSG_START); + self.run = self.run.wrapping_add(1); + self.engine(MSG_START, &self.run.to_le_bytes()); self.running = true; } } fn stop(&mut self) { if self.running { - self.engine(MSG_STOP); + self.engine(MSG_STOP, &[]); self.running = false; } } @@ -449,7 +455,7 @@ mod tests { use super::*; fn report(mask: u32, at: u64) -> Played { - Played { mask, clocked: 1, first_nanos: at, last_nanos: at } + Played { mask, clocked: 1, first_nanos: at, last_nanos: at, run: 1 } } /// Two reports read at one wake: the newer's time clocks the older's diff --git a/system/soundserver/src/volume.rs b/system/soundserver/src/volume.rs index c2dfddbddbd..cd2fc892924 100644 --- a/system/soundserver/src/volume.rs +++ b/system/soundserver/src/volume.rs @@ -152,43 +152,6 @@ fn send(conn: &Connection, answer: Option) -> Result<(), ipc::TrySendErr mod tests { use super::*; - /// A flood: one request in flight, and the next read only once it is - /// answered, however many the holder wrote. - #[test] - fn one_master_request_is_in_flight_at_a_time() { - let mut relay = Relay::default(); - let holder = relay.mint(); - assert!(relay.idle()); - relay.forwarded(holder); - for _ in 0..1000 { - assert!(!relay.idle(), "a second request read with one in flight"); - } - assert_eq!(relay.answered(), Some(holder)); - assert!(relay.idle()); - assert_eq!(relay.answered(), None, "an answer nobody asked for is owed to nobody"); - } - - /// A holder that reconnected while its request was in flight: the answer - /// is the old connection's, and the new one is never handed it. - #[test] - fn an_answer_goes_only_to_the_connection_that_asked() { - let mut relay = Relay::default(); - let old = relay.mint(); - relay.forwarded(old); - let new = relay.mint(); - assert_ne!(old, new); - assert_eq!(relay.answered(), Some(old)); - assert!(relay.idle(), "the new holder's requests are read once the old one's is settled"); - } - - /// A driver whose connection takes no request is a refusal by name, - /// answered unavailable, never a panic. - #[test] - fn a_driver_that_takes_no_request_is_a_named_refusal() { - assert_eq!(crate::output::asked::<()>(Ok(())), Asked::Forwarded); - assert!(matches!(crate::output::asked(Err("full")), Asked::Refused(why) if why.contains("took no master request"))); - } - #[test] fn a_volume_request_is_refused_unless_its_type_and_width_fit() { const STEP: usize = core::mem::size_of::();