From 5a5d80987d87e2f28b44ac6dd15052e0b58d7a4b Mon Sep 17 00:00:00 2001 From: alicankaralar Date: Sat, 27 Jun 2026 16:14:09 +0200 Subject: [PATCH 01/23] feat(dice): dynamic sample-rate selection (probe caps + CLOCK_SELECT switch) Completes the half-built DICE clock-config machinery so CoreAudio can select among the device's supported sample rates instead of a hardcoded 48 kHz. Validated/gated to 1x rates (32/44.1/48 kHz); 2x/4x change frames-per-packet and per-stream geometry and stay disabled until verified (see kDiceMaxSupportedRateHz; FFADO likewise gates 4x). Probe/encode primitives (DICETypes.hpp), cross-validated with FFADO dice_defines.h: - DiceClockCapsSupportRate(clockCaps, hz) decodes CLOCKCAPABILITIES[15:0]. - DiceClockSelectForRate(hz, source) builds CLOCK_SELECT [rate<<8 | source]. Wiring: - IsSupportedClockConfig now accepts internal-clock 1x rates (was 48k-only), which is the sole gate that previously pinned the (already desiredClock-parameterized) bringup to 48 kHz. - Profile SupportedSampleRates() (DICE default {44100,48000}); PublishNub sets dev.sampleRates; PopulateNubProperties emits kSampleRates + kCurrentSampleRate so the HAL advertises a stream format per rate. - ASFWAudioDevice::HandleChangeSampleRate -> ASFWAudioNub::RequestSampleRateChange -> AudioCoordinator::RequestDiceClockConfig(new clock, kSampleRateChange): streaming -> stop+restart at the new rate; idle -> stored as pendingClock. - RunStartStreaming honors a stored pendingClock (rate chosen while idle) instead of always 48 kHz. For Venice (44.1/48, both 1x) a switch is a pure CLOCK_SELECT change with identical 2x16 geometry. Built + tests pass; HW test pending. Co-Authored-By: Claude Opus 4.8 --- .../Audio/DriverKit/ASFWAudioDevice.cpp | 29 +++++++ .../Audio/DriverKit/ASFWAudioDevice.iig | 4 + ASFWDriver/Audio/DriverKit/ASFWAudioNub.cpp | 30 +++++++ ASFWDriver/Audio/DriverKit/ASFWAudioNub.iig | 6 ++ .../Config/DICE/DiceDeviceProfile.hpp | 9 +++ ASFWDriver/Audio/Model/ASFWAudioDevice.hpp | 19 +++++ .../Backends/AudioDuplexCoordinator.cpp | 8 +- .../Protocols/Backends/DiceAudioBackend.cpp | 5 ++ .../Audio/Protocols/DICE/Core/DICETypes.hpp | 78 ++++++++++++++++++- .../Protocols/DICE/TCAT/DICETcatProtocol.cpp | 10 ++- .../Protocols/Duplex/AudioClockConfig.hpp | 12 ++- tests/devices/AudioDuplexCoordinatorTests.cpp | 5 +- tests/devices/DICETcatProtocolTests.cpp | 15 +++- 13 files changed, 218 insertions(+), 12 deletions(-) diff --git a/ASFWDriver/Audio/DriverKit/ASFWAudioDevice.cpp b/ASFWDriver/Audio/DriverKit/ASFWAudioDevice.cpp index 0db5ed86..a3e78118 100644 --- a/ASFWDriver/Audio/DriverKit/ASFWAudioDevice.cpp +++ b/ASFWDriver/Audio/DriverKit/ASFWAudioDevice.cpp @@ -593,3 +593,32 @@ kern_return_t ASFWAudioDevice::StopIO(IOUserAudioStartStopFlags in_flags) { return kr; } + +kern_return_t ASFWAudioDevice::HandleChangeSampleRate(double in_sample_rate) { + if (!ivars || !ivars->driverIvars) { + return kIOReturnNotReady; + } + auto& ivars = *this->ivars->driverIvars; + + const uint32_t rateHz = static_cast(in_sample_rate); + ASFW_LOG(Audio, "ASFWAudioDevice: HandleChangeSampleRate %.0f Hz", in_sample_rate); + + // Program the device's DICE clock to the new rate via the transport-side + // coordinator (CLOCK_SELECT + duplex reconfigure). If streaming, it stops and + // re-brings-up at the new rate; if idle, it stores the pending clock for the + // next start. Reject the change if the transport can't apply it so CoreAudio + // does not believe the hardware moved. + if (ivars.device.audioNub) { + const kern_return_t kr = + ivars.device.audioNub->RequestSampleRateChange(rateHz); + if (kr != kIOReturnSuccess) { + ASFW_LOG(Audio, + "ASFWAudioDevice: HandleChangeSampleRate transport reconfig failed: 0x%x", + kr); + return kr; + } + ivars.device.currentSampleRate = static_cast(rateHz); + } + + return super::HandleChangeSampleRate(in_sample_rate); +} diff --git a/ASFWDriver/Audio/DriverKit/ASFWAudioDevice.iig b/ASFWDriver/Audio/DriverKit/ASFWAudioDevice.iig index 43fc4ebf..3707f0d1 100644 --- a/ASFWDriver/Audio/DriverKit/ASFWAudioDevice.iig +++ b/ASFWDriver/Audio/DriverKit/ASFWAudioDevice.iig @@ -31,6 +31,10 @@ public: virtual kern_return_t StartIO(IOUserAudioStartStopFlags in_flags) override LOCALONLY; virtual kern_return_t StopIO(IOUserAudioStartStopFlags in_flags) override LOCALONLY; + // CoreAudio sample-rate selection: program the device's DICE clock to the + // new rate (via the transport-side coordinator) before accepting it. + virtual kern_return_t HandleChangeSampleRate(double in_sample_rate) override LOCALONLY; + void SetDriverIvars(ASFWAudioDriver_IVars* ivars) LOCALONLY; }; diff --git a/ASFWDriver/Audio/DriverKit/ASFWAudioNub.cpp b/ASFWDriver/Audio/DriverKit/ASFWAudioNub.cpp index bdf0a631..c5ed8dd2 100644 --- a/ASFWDriver/Audio/DriverKit/ASFWAudioNub.cpp +++ b/ASFWDriver/Audio/DriverKit/ASFWAudioNub.cpp @@ -16,6 +16,8 @@ #include "../../Logging/Logging.hpp" #include "../../Logging/LogConfig.hpp" #include "../Core/AudioCoordinator.hpp" +#include "../Protocols/DICE/Core/DICETypes.hpp" +#include "../Protocols/DICE/Core/DICERestartSession.hpp" #include "../../Protocols/AVC/IAVCDiscovery.hpp" #include "../Protocols/IDeviceProtocol.hpp" #include "../../Service/DriverContext.hpp" @@ -562,6 +564,34 @@ kern_return_t IMPL(ASFWAudioNub, FreeTxIsochResources) return ctx->isoch.FreeTxIsochResources(); } +kern_return_t ASFWAudioNub::RequestSampleRateChange(uint32_t sampleRateHz) +{ + if (!ivars) { + return kIOReturnNotReady; + } + auto* coordinator = GetAudioCoordinator(ivars); + if (!coordinator) { + return kIOReturnNotReady; + } + + uint32_t clockSelect = 0; + if (!ASFW::Audio::DICE::DiceClockSelectForRate( + sampleRateHz, ASFW::Audio::DICE::ClockSource::Internal, clockSelect)) { + ASFW_LOG(Audio, "ASFWAudioNub: RequestSampleRateChange unsupported rate %u Hz", sampleRateHz); + return kIOReturnUnsupported; + } + + const ASFW::Audio::DICE::DiceDesiredClockConfig desired{ + .sampleRateHz = sampleRateHz, + .clockSelect = clockSelect, + }; + ASFW_LOG(Audio, + "ASFWAudioNub: RequestSampleRateChange %u Hz clockSelect=0x%08x guid=0x%016llx", + sampleRateHz, clockSelect, ivars->guid); + return coordinator->RequestDiceClockConfig( + ivars->guid, desired, ASFW::Audio::DICE::DiceRestartReason::kSampleRateChange); +} + void ASFWAudioNub::SetChannelCount(uint32_t channels) { if (!ivars) return; diff --git a/ASFWDriver/Audio/DriverKit/ASFWAudioNub.iig b/ASFWDriver/Audio/DriverKit/ASFWAudioNub.iig index 0a34ccd6..c4a876ff 100644 --- a/ASFWDriver/Audio/DriverKit/ASFWAudioNub.iig +++ b/ASFWDriver/Audio/DriverKit/ASFWAudioNub.iig @@ -108,6 +108,12 @@ public: void SetStreamMode(uint32_t modeRaw) LOCALONLY; uint32_t GetStreamMode() const LOCALONLY; + // Request a device sample-rate change (CoreAudio HandleChangeSampleRate -> + // DICE CLOCK_SELECT + duplex reconfigure). Bridges to the transport-side + // AudioCoordinator. Returns kIOReturnUnsupported for a rate the device does + // not advertise. + kern_return_t RequestSampleRateChange(uint32_t sampleRateHz) LOCALONLY; + // Device identity for protocol bridge routing void SetGuid(uint64_t guid) LOCALONLY; uint64_t GetGuid() const LOCALONLY; diff --git a/ASFWDriver/Audio/DriverKit/Config/DICE/DiceDeviceProfile.hpp b/ASFWDriver/Audio/DriverKit/Config/DICE/DiceDeviceProfile.hpp index 0aa66084..8a19a11f 100644 --- a/ASFWDriver/Audio/DriverKit/Config/DICE/DiceDeviceProfile.hpp +++ b/ASFWDriver/Audio/DriverKit/Config/DICE/DiceDeviceProfile.hpp @@ -11,6 +11,7 @@ #include "Isoch/DiceStreamConfig.hpp" #include +#include namespace ASFW::Isoch::Audio::DICE { @@ -43,6 +44,14 @@ class IDiceDeviceProfile : public IAudioDeviceProfile { [[nodiscard]] virtual uint32_t TxStreamCount() const noexcept { return 1; } [[nodiscard]] virtual uint32_t RxStreamCount() const noexcept { return 1; } + /// Sample rates advertised to CoreAudio. Default is the universal DICE 1x + /// baseline (44.1/48 kHz); 2x/4x are omitted until their stream geometry is + /// verified end-to-end (see kDiceMaxSupportedRateHz). Profiles for devices + /// with a different 1x set may override. + [[nodiscard]] virtual std::vector SupportedSampleRates() const { + return {44100u, 48000u}; + } + // Default implementations mapping DICE structures to the unified IAudioDeviceProfile: [[nodiscard]] Encoding::AudioWireFormat TxWireFormat() const noexcept override { return Quirks().tx.hostToDevicePcmEncoding; diff --git a/ASFWDriver/Audio/Model/ASFWAudioDevice.hpp b/ASFWDriver/Audio/Model/ASFWAudioDevice.hpp index a1ca3b76..92d0ae9f 100644 --- a/ASFWDriver/Audio/Model/ASFWAudioDevice.hpp +++ b/ASFWDriver/Audio/Model/ASFWAudioDevice.hpp @@ -102,6 +102,25 @@ struct ASFWAudioDevice { properties->setObject(PropertyKeys::kOutputPlugName, outputPlugNameStr.get()); properties->setObject(PropertyKeys::kCurrentSampleRate, currentRateNum.get()); + // Sample rates advertised to CoreAudio. The HAL builds a stream format + // per entry (ASFWAudioDriverGraph), so this is what the user can select. + if (!sampleRates.empty()) { + auto rateArray = OSSharedPtr(OSArray::withCapacity( + static_cast(sampleRates.size())), OSNoRetain); + if (rateArray) { + for (uint32_t hz : sampleRates) { + auto n = OSSharedPtr(OSNumber::withNumber(hz, 32), OSNoRetain); + if (n) { + rateArray->setObject(n.get()); + } + } + properties->setObject(PropertyKeys::kSampleRates, rateArray.get()); + } + } + if (auto curRate = OSSharedPtr(OSNumber::withNumber(currentSampleRate, 32), OSNoRetain)) { + properties->setObject(PropertyKeys::kCurrentSampleRate, curRate.get()); + } + // Per-channel device labels (optional). The audio side reads these in // channel order and prefers them over synthesized names. PublishChannelNames(properties, PropertyKeys::kInputChannelNames, inputChannelNames); diff --git a/ASFWDriver/Audio/Protocols/Backends/AudioDuplexCoordinator.cpp b/ASFWDriver/Audio/Protocols/Backends/AudioDuplexCoordinator.cpp index c5dce5c9..886c5eec 100644 --- a/ASFWDriver/Audio/Protocols/Backends/AudioDuplexCoordinator.cpp +++ b/ASFWDriver/Audio/Protocols/Backends/AudioDuplexCoordinator.cpp @@ -419,9 +419,15 @@ IOReturn AudioDuplexCoordinator::RunStartStreaming(uint64_t guid) noexcept { } DuplexRestartSession session = LoadSession(guid); - const AudioClockConfig desiredClock{ + // Honor a clock requested before streaming began (e.g. the user picked a + // non-48k rate in Audio MIDI Setup while idle, stored as pendingClock); + // otherwise default to 48 kHz. + AudioClockConfig desiredClock{ .sampleRateHz = 48000U, }; + if (IsSupportedAudioClockConfig(session.pendingClock)) { + desiredClock = session.pendingClock; + } const DuplexRestartReason reason = HasRestartIntent(session) ? DICE::ClassifyRestartReason(&session, desiredClock) : DuplexRestartReason::kInitialStart; diff --git a/ASFWDriver/Audio/Protocols/Backends/DiceAudioBackend.cpp b/ASFWDriver/Audio/Protocols/Backends/DiceAudioBackend.cpp index cee331e4..573390e1 100644 --- a/ASFWDriver/Audio/Protocols/Backends/DiceAudioBackend.cpp +++ b/ASFWDriver/Audio/Protocols/Backends/DiceAudioBackend.cpp @@ -433,6 +433,11 @@ void DiceAudioBackend::EnsureNubForGuid(uint64_t guid) noexcept { dev.channelCount = std::max(dev.inputChannelCount, dev.outputChannelCount); dev.inputPlugName = "Input"; dev.outputPlugName = "Output"; + dev.sampleRates = profile->SupportedSampleRates(); + if (dev.sampleRates.empty()) { + dev.sampleRates = {48000u}; + } + dev.currentSampleRate = 48000u; // Enrich with the device's real per-channel labels (if the protocol has // loaded them), update the endpoint runtime, then publish the nub. Host diff --git a/ASFWDriver/Audio/Protocols/DICE/Core/DICETypes.hpp b/ASFWDriver/Audio/Protocols/DICE/Core/DICETypes.hpp index 08978468..0d6d4187 100644 --- a/ASFWDriver/Audio/Protocols/DICE/Core/DICETypes.hpp +++ b/ASFWDriver/Audio/Protocols/DICE/Core/DICETypes.hpp @@ -483,6 +483,62 @@ namespace ClockRateIndex { constexpr uint32_t k192000 = 0x06; } +// ---------------------------------------------------------------------------- +// Sample-rate probing / encoding (CLOCKCAPABILITIES 0x64 + CLOCK_SELECT 0x4C). +// The low 16 bits of CLOCKCAPABILITIES are a rate bitmask (RateCaps::*) and +// CLOCK_SELECT packs [rate index << 8 | source]. Cross-validated with FFADO +// dice_defines.h (DICE_CLOCKCAP_RATE_* / DICE_RATE_* / DICE_SET_RATE). +// ---------------------------------------------------------------------------- + +/// One standard rate paired with its CLOCKCAPABILITIES bit and CLOCK_SELECT index. +struct DiceRateMapEntry { + uint32_t hz; + uint32_t capsBit; ///< RateCaps:: bit in CLOCKCAPABILITIES[15:0] + uint32_t rateIndex; ///< ClockRateIndex:: value for CLOCK_SELECT[15:8] +}; + +inline constexpr DiceRateMapEntry kDiceRateTable[] = { + {32000, RateCaps::k32000, ClockRateIndex::k32000}, + {44100, RateCaps::k44100, ClockRateIndex::k44100}, + {48000, RateCaps::k48000, ClockRateIndex::k48000}, + {88200, RateCaps::k88200, ClockRateIndex::k88200}, + {96000, RateCaps::k96000, ClockRateIndex::k96000}, + {176400, RateCaps::k176400, ClockRateIndex::k176400}, + {192000, RateCaps::k192000, ClockRateIndex::k192000}, +}; + +/// Highest rate whose isoch stream geometry is currently validated end-to-end. +/// 1x rates (<=48k) keep the single-DBS, 8-frames-per-packet layout the rest of +/// the stack assumes. 2x/4x change frames-per-packet and per-stream channel +/// splits and are NOT yet validated on hardware (FFADO likewise gates 4x), so we +/// do not advertise them. Raise this once the 2x/4x pipeline is HW-verified. +inline constexpr uint32_t kDiceMaxSupportedRateHz = 48000; + +/// True if CLOCKCAPABILITIES advertises `rateHz`. +[[nodiscard]] inline bool DiceClockCapsSupportRate(uint32_t clockCaps, uint32_t rateHz) noexcept { + for (const auto& e : kDiceRateTable) { + if (e.hz == rateHz) { + return (clockCaps & e.capsBit) != 0; + } + } + return false; +} + +/// Build the CLOCK_SELECT value for `rateHz` on `source`. Returns false for a +/// rate not in the standard table. +[[nodiscard]] inline bool DiceClockSelectForRate(uint32_t rateHz, + ClockSource source, + uint32_t& outClockSelect) noexcept { + for (const auto& e : kDiceRateTable) { + if (e.hz == rateHz) { + outClockSelect = (e.rateIndex << ClockSelect::kRateShift) | + (static_cast(source) & ClockSelect::kSourceMask); + return true; + } + } + return false; +} + // DICE GLOBAL_CLOCK_SELECT encoding. This stays inside the DICE adapter; the // protocol-neutral duplex seam carries only AudioClockConfig::sampleRateHz. struct DiceClockConfiguration { @@ -494,10 +550,26 @@ constexpr uint32_t kDiceClockSelect48kInternal = (ClockRateIndex::k48000 << ClockSelect::kRateShift) | static_cast(ClockSource::Internal); -[[nodiscard]] constexpr bool IsSupportedDiceClockConfiguration( +/// A configuration is supported when the source is Internal (bring-up policy: +/// device is its own clock master), the rate is in the standard table and +/// within the HW-validated ceiling, and the CLOCK_SELECT rate index encodes +/// that same rate. Generalizes the former 48k-internal-only placeholder. +[[nodiscard]] inline bool IsSupportedDiceClockConfiguration( const DiceClockConfiguration& clock) noexcept { - return clock.sampleRateHz == 48000U && - clock.clockSelect == kDiceClockSelect48kInternal; + if (clock.sampleRateHz > kDiceMaxSupportedRateHz) { + return false; + } + if ((clock.clockSelect & ClockSelect::kSourceMask) != + static_cast(ClockSource::Internal)) { + return false; + } + for (const auto& e : kDiceRateTable) { + if (e.hz == clock.sampleRateHz) { + return ((clock.clockSelect & ClockSelect::kRateMask) >> + ClockSelect::kRateShift) == e.rateIndex; + } + } + return false; } /// Parsed global section state diff --git a/ASFWDriver/Audio/Protocols/DICE/TCAT/DICETcatProtocol.cpp b/ASFWDriver/Audio/Protocols/DICE/TCAT/DICETcatProtocol.cpp index d4fff802..eecb8a53 100644 --- a/ASFWDriver/Audio/Protocols/DICE/TCAT/DICETcatProtocol.cpp +++ b/ASFWDriver/Audio/Protocols/DICE/TCAT/DICETcatProtocol.cpp @@ -53,10 +53,16 @@ bool DICETcatProtocol::MakeDiceClockConfiguration( } // The DICE adapter owns the register encoding: Linux selects the requested // rate by updating GLOBAL_CLOCK_SELECT while preserving the source bits - // (dice-stream.c:60-85; dice-interface.h:80-95). + // (dice-stream.c:60-85; dice-interface.h:80-95). Encode the requested rate + // via the standard table; source stays Internal (bring-up policy). + uint32_t clockSelect = 0; + if (!DiceClockSelectForRate(requested.sampleRateHz, ClockSource::Internal, + clockSelect)) { + return false; + } out = DiceClockConfiguration{ .sampleRateHz = requested.sampleRateHz, - .clockSelect = kDiceClockSelect48kInternal, + .clockSelect = clockSelect, }; return true; } diff --git a/ASFWDriver/Audio/Protocols/Duplex/AudioClockConfig.hpp b/ASFWDriver/Audio/Protocols/Duplex/AudioClockConfig.hpp index 7072d6ba..06d9d72f 100644 --- a/ASFWDriver/Audio/Protocols/Duplex/AudioClockConfig.hpp +++ b/ASFWDriver/Audio/Protocols/Duplex/AudioClockConfig.hpp @@ -9,15 +9,21 @@ namespace ASFW::Audio { -// The duplex lifecycle is presently 48 kHz-only. Protocol adapters translate -// this generic request into their device-specific clock command or register value. +// Protocol adapters translate this generic request into their device-specific +// clock command or register value. struct AudioClockConfig { uint32_t sampleRateHz{0}; }; +// Validated 1x rates (32/44.1/48 kHz). 2x/4x rates change frames-per-packet +// and per-stream channel geometry and are not yet validated end-to-end, so +// they are rejected here (protocol adapters additionally gate by device +// capabilities, e.g. DICE CLOCKCAPABILITIES). [[nodiscard]] constexpr bool IsSupportedAudioClockConfig( const AudioClockConfig& desiredClock) noexcept { - return desiredClock.sampleRateHz == 48000U; + return desiredClock.sampleRateHz == 32000U || + desiredClock.sampleRateHz == 44100U || + desiredClock.sampleRateHz == 48000U; } } // namespace ASFW::Audio diff --git a/tests/devices/AudioDuplexCoordinatorTests.cpp b/tests/devices/AudioDuplexCoordinatorTests.cpp index 32976628..c53009aa 100644 --- a/tests/devices/AudioDuplexCoordinatorTests.cpp +++ b/tests/devices/AudioDuplexCoordinatorTests.cpp @@ -1126,8 +1126,11 @@ TEST_F(AudioDuplexCoordinatorTests, ProgramRxFailureRollsBackHostAndDeviceInOrde } TEST_F(AudioDuplexCoordinatorTests, UnsupportedClockConfigFailsBeforeHostAllocation) { + // 2x/4x rates are not yet validated end-to-end and must be rejected before + // any host allocation. (44.1 kHz is a supported 1x rate as of the dynamic + // sample-rate work.) const AudioClockConfig unsupportedClock{ - .sampleRateHz = 44100U, + .sampleRateHz = 96000U, }; EXPECT_EQ(coordinator_.RequestClockConfig(kTestGuid, unsupportedClock, diff --git a/tests/devices/DICETcatProtocolTests.cpp b/tests/devices/DICETcatProtocolTests.cpp index ecedd7df..bd03cb32 100644 --- a/tests/devices/DICETcatProtocolTests.cpp +++ b/tests/devices/DICETcatProtocolTests.cpp @@ -260,15 +260,26 @@ TEST(DICETcatProtocolTests, InitializeIsSideEffectFree) { EXPECT_EQ(bus.lockCount, 0); } -TEST(DICETcatProtocolTests, Neutral48kClockRequestMapsToDiceClockSelectInsideAdapter) { +TEST(DICETcatProtocolTests, NeutralClockRequestMapsToDiceClockSelectInsideAdapter) { DiceClockConfiguration mapped{}; EXPECT_TRUE(ASFW::Audio::DICE::TCAT::DICETcatProtocolTestPeer::MakeDiceClockConfiguration( AudioClockConfig{.sampleRateHz = 48000U}, mapped)); EXPECT_EQ(mapped.sampleRateHz, 48000U); EXPECT_EQ(mapped.clockSelect, kClockSelect48kInternal); - EXPECT_FALSE(ASFW::Audio::DICE::TCAT::DICETcatProtocolTestPeer::MakeDiceClockConfiguration( + // Validated 1x rates map to their CLOCK_SELECT encoding (rate index << 8 | + // internal source). + EXPECT_TRUE(ASFW::Audio::DICE::TCAT::DICETcatProtocolTestPeer::MakeDiceClockConfiguration( AudioClockConfig{.sampleRateHz = 44100U}, mapped)); + EXPECT_EQ(mapped.sampleRateHz, 44100U); + EXPECT_EQ(mapped.clockSelect, + (ASFW::Audio::DICE::ClockRateIndex::k44100 + << ASFW::Audio::DICE::ClockSelect::kRateShift) | + static_cast(ClockSource::Internal)); + + // 2x/4x rates stay rejected until the stream geometry is HW-validated. + EXPECT_FALSE(ASFW::Audio::DICE::TCAT::DICETcatProtocolTestPeer::MakeDiceClockConfiguration( + AudioClockConfig{.sampleRateHz = 96000U}, mapped)); } TEST(DICETcatProtocolTests, RuntimeCapsAggregateTotalConfiguredStreams) { From 5a8e2cf206e7e0e938e789acf271c87b97cc9d3e Mon Sep 17 00:00:00 2001 From: alicankaralar Date: Sat, 27 Jun 2026 16:31:13 +0200 Subject: [PATCH 02/23] fix(dice): drive advertised sample rates from profile in audio graph MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BuildAudioGraph already overrides channel counts from the resolved profile but left sampleRates sourced only from the nub property dict, which did not reliably carry kSampleRates to the HAL — so CoreAudio saw a single 48k format and Audio MIDI Setup showed no rate dropdown. Override parsedConfig.sampleRates/sampleRateCount from profile->SupportedSampleRates() (same authoritative source as the channel counts), and add a base IAudioDeviceProfile::SupportedSampleRates ({48000}) so the method is callable on the base pointer; DICE overrides to {44100,48000}. Co-Authored-By: Claude Opus 4.8 --- .../Audio/DriverKit/ASFWAudioDriverGraph.cpp | 23 +++++++++++++++++++ .../Config/DICE/DiceDeviceProfile.hpp | 2 +- .../DriverKit/Config/IAudioDeviceProfile.hpp | 7 ++++++ 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/ASFWDriver/Audio/DriverKit/ASFWAudioDriverGraph.cpp b/ASFWDriver/Audio/DriverKit/ASFWAudioDriverGraph.cpp index 446fe5ea..e02560fa 100644 --- a/ASFWDriver/Audio/DriverKit/ASFWAudioDriverGraph.cpp +++ b/ASFWDriver/Audio/DriverKit/ASFWAudioDriverGraph.cpp @@ -154,6 +154,29 @@ kern_return_t BuildAudioGraph(ASFWAudioDriver& driver, rxChannels, txChannels); + // Sample rates come from the profile (same authoritative source as the + // channel counts above), so CoreAudio advertises the full set even if the + // nub property dict did not carry kSampleRates. The HAL builds one stream + // format per rate (see SetAvailableSampleRates below). + const auto profileRates = profile->SupportedSampleRates(); + if (!profileRates.empty()) { + parsedConfig.sampleRateCount = 0; + bool currentRateInSet = false; + for (uint32_t hz : profileRates) { + if (parsedConfig.sampleRateCount >= ASFW::Isoch::Audio::kMaxSampleRates) { + break; + } + parsedConfig.sampleRates[parsedConfig.sampleRateCount++] = + static_cast(hz); + if (static_cast(hz) == parsedConfig.currentSampleRate) { + currentRateInSet = true; + } + } + if (!currentRateInSet) { + parsedConfig.currentSampleRate = parsedConfig.sampleRates[0]; + } + } + // Regenerate channel names for the updated channel counts. Prefers the // device's per-channel labels (published by the core side) and falls // back to synthesized " N" for any slot without a real label. diff --git a/ASFWDriver/Audio/DriverKit/Config/DICE/DiceDeviceProfile.hpp b/ASFWDriver/Audio/DriverKit/Config/DICE/DiceDeviceProfile.hpp index 8a19a11f..61185dd7 100644 --- a/ASFWDriver/Audio/DriverKit/Config/DICE/DiceDeviceProfile.hpp +++ b/ASFWDriver/Audio/DriverKit/Config/DICE/DiceDeviceProfile.hpp @@ -48,7 +48,7 @@ class IDiceDeviceProfile : public IAudioDeviceProfile { /// baseline (44.1/48 kHz); 2x/4x are omitted until their stream geometry is /// verified end-to-end (see kDiceMaxSupportedRateHz). Profiles for devices /// with a different 1x set may override. - [[nodiscard]] virtual std::vector SupportedSampleRates() const { + [[nodiscard]] std::vector SupportedSampleRates() const override { return {44100u, 48000u}; } diff --git a/ASFWDriver/Audio/DriverKit/Config/IAudioDeviceProfile.hpp b/ASFWDriver/Audio/DriverKit/Config/IAudioDeviceProfile.hpp index 8ed6a5cc..aa648a76 100644 --- a/ASFWDriver/Audio/DriverKit/Config/IAudioDeviceProfile.hpp +++ b/ASFWDriver/Audio/DriverKit/Config/IAudioDeviceProfile.hpp @@ -8,6 +8,7 @@ #include "../../Wire/AMDTP/AmdtpTypes.hpp" #include +#include namespace ASFW::Isoch::Audio { @@ -59,6 +60,12 @@ class IAudioDeviceProfile { /// Returns the reported receive latency in frames for a given sample rate. [[nodiscard]] virtual uint32_t RxReportedLatencyFrames(double sampleRate) const noexcept = 0; + /// Sample rates advertised to CoreAudio. Default is a single 48 kHz; profiles + /// override to expose the device's supported set (DICE decodes CLOCKCAPABILITIES). + [[nodiscard]] virtual std::vector SupportedSampleRates() const { + return {48000u}; + } + /// IEC 61883-6 presentation delay removed from received SYT before replay. [[nodiscard]] virtual uint32_t RxTransferDelayTicks(double sampleRate) const noexcept { (void)sampleRate; From 9b345f9c573f37f1c84f59c69280753bafa0bbbc Mon Sep 17 00:00:00 2001 From: alicankaralar Date: Sat, 27 Jun 2026 16:38:10 +0200 Subject: [PATCH 03/23] fix(dice): skip single-format bring-up policy for profiled devices MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The real reason the rate dropdown never appeared: BuildAudioGraph called ApplyBringupSingleFormatPolicy() unconditionally, which hard-resets sampleRates to a single 48 kHz entry — and it ran *after* the profile-driven rate override, silently wiping it. CoreAudio then saw one format and Audio MIDI Setup showed no rate selector. Track whether the resolved profile supplied its own advertised rate set and skip the single-format policy in that case. Unprofiled devices, whose multi-rate path is still unvalidated, keep the conservative single-format fallback. Also corrects the now-misleading "Forcing single advertised format" log to report the actual advertised rate count. Co-Authored-By: Claude Opus 4.8 --- .../Audio/DriverKit/ASFWAudioDriverGraph.cpp | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/ASFWDriver/Audio/DriverKit/ASFWAudioDriverGraph.cpp b/ASFWDriver/Audio/DriverKit/ASFWAudioDriverGraph.cpp index e02560fa..a38f6c2a 100644 --- a/ASFWDriver/Audio/DriverKit/ASFWAudioDriverGraph.cpp +++ b/ASFWDriver/Audio/DriverKit/ASFWAudioDriverGraph.cpp @@ -142,6 +142,10 @@ kern_return_t BuildAudioGraph(ASFWAudioDriver& driver, ASFW_LOG(Audio, "ASFWAudioDriver: Using default device configuration (no nub properties)"); } + // Set once a resolved profile supplies its advertised sample-rate set, so the + // bring-up single-format policy below is skipped for profiled devices. + bool profileProvidedSampleRates = false; + // Resolve audio profile registry on startup if (const auto* profile = ASFW::Isoch::Audio::AudioProfileRegistry::FindProfile( parsedConfig.vendorId, parsedConfig.modelId, parsedConfig.guid)) { @@ -175,6 +179,7 @@ kern_return_t BuildAudioGraph(ASFWAudioDriver& driver, if (!currentRateInSet) { parsedConfig.currentSampleRate = parsedConfig.sampleRates[0]; } + profileProvidedSampleRates = true; } // Regenerate channel names for the updated channel counts. Prefers the @@ -184,7 +189,12 @@ kern_return_t BuildAudioGraph(ASFWAudioDriver& driver, } ASFW::Isoch::Audio::BuildFallbackBoolControls(parsedConfig); - ASFW::Isoch::Audio::ApplyBringupSingleFormatPolicy(parsedConfig); + // Profiled devices advertise their own validated rate set (DICE: 44.1/48 kHz); + // only fall back to the single-format bring-up policy for unprofiled devices + // whose multi-rate path is not yet validated end-to-end. + if (!profileProvidedSampleRates) { + ASFW::Isoch::Audio::ApplyBringupSingleFormatPolicy(parsedConfig); + } ASFW::Isoch::Audio::ClampAudioDriverChannels(parsedConfig, ASFW::Encoding::kMaxPcmChannels); CopyParsedConfigToDeviceState(parsedConfig, ivars.device); @@ -236,7 +246,8 @@ kern_return_t BuildAudioGraph(ASFWAudioDriver& driver, ? "blocking" : "non-blocking"); ASFW_LOG(Audio, - "ASFWAudioDriver: Forcing single advertised format: input=Float32 output=Float32"); + "ASFWAudioDriver: Advertising %u Float32 format(s) (input/output) across rates", + ivars.device.sampleRateCount); ASFW_LOG(Audio, "ASFWAudioDriver: Effective runtime channels: input=%u output=%u aggregate=%u", ivars.device.inputChannelCount, From 388e7473de00048e3343e7a80e9f4b15073bf4b2 Mon Sep 17 00:00:00 2001 From: alicankaralar Date: Sat, 27 Jun 2026 16:44:03 +0200 Subject: [PATCH 04/23] fix(dice): commit sample-rate change via SetSampleRate, not super MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit HandleChangeSampleRate reconfigured the transport clock but then delegated to super::HandleChangeSampleRate, which does not move the ADK device's active format — so CoreAudio reverted to the previous rate and the picker snapped back. The validated ADK contract (ADKVirtualAudioLab) applies the change by calling SetSampleRate(in_sample_rate) directly. Call SetSampleRate after the transport reconfig succeeds and return its status, so the HAL actually adopts the new rate. Co-Authored-By: Claude Opus 4.8 --- ASFWDriver/Audio/DriverKit/ASFWAudioDevice.cpp | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/ASFWDriver/Audio/DriverKit/ASFWAudioDevice.cpp b/ASFWDriver/Audio/DriverKit/ASFWAudioDevice.cpp index a3e78118..5f84310a 100644 --- a/ASFWDriver/Audio/DriverKit/ASFWAudioDevice.cpp +++ b/ASFWDriver/Audio/DriverKit/ASFWAudioDevice.cpp @@ -620,5 +620,15 @@ kern_return_t ASFWAudioDevice::HandleChangeSampleRate(double in_sample_rate) { ivars.device.currentSampleRate = static_cast(rateHz); } - return super::HandleChangeSampleRate(in_sample_rate); + // Commit the rate to the ADK device. The validated ADK contract + // (ADKVirtualAudioLab) applies the change by calling SetSampleRate here, not + // by delegating to super — the base HandleChangeSampleRate does not move the + // active format, so without this the HAL reverts to the previous rate. + const kern_return_t setKr = SetSampleRate(in_sample_rate); + if (setKr != kIOReturnSuccess) { + ASFW_LOG(Audio, + "ASFWAudioDevice: HandleChangeSampleRate SetSampleRate(%.0f) failed: 0x%x", + in_sample_rate, setKr); + } + return setKr; } From bb183d515adfdcf9e3229eda242b6004957fcb70 Mon Sep 17 00:00:00 2001 From: alicankaralar Date: Sat, 27 Jun 2026 17:05:49 +0200 Subject: [PATCH 05/23] fix(dice): start streams at the nominal rate; disable format restore MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two bugs kept the device pinned at 44.1 kHz: 1. BuildAudioGraph set each stream's *current* format to inputFormats[0] / outputFormats[0] — i.e. sampleRates[0] (44.1 kHz in profile order) — while setting the device nominal rate to currentSampleRate (48 kHz). The HAL reconciles that mismatch by snapping the device down to the stream's format, so it always came up at 44.1 kHz. Now select the format index whose rate matches currentSampleRate for both streams. 2. IOUserAudioDevice defaults to host save/restore of stream formats, so a stale rate from a prior session could override the rate this graph selects. Call SetWantsStreamFormatsRestored(false) before publish for deterministic bring-up; rate changes go through HandleChangeSampleRate. Also log HandleChangeSampleRate on entry and on the not-ready early return, so a failed pick is no longer silent in the trace. Co-Authored-By: Claude Opus 4.8 --- .../Audio/DriverKit/ASFWAudioDevice.cpp | 6 +++++- .../Audio/DriverKit/ASFWAudioDriverGraph.cpp | 21 +++++++++++++++---- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/ASFWDriver/Audio/DriverKit/ASFWAudioDevice.cpp b/ASFWDriver/Audio/DriverKit/ASFWAudioDevice.cpp index 5f84310a..85dd7c76 100644 --- a/ASFWDriver/Audio/DriverKit/ASFWAudioDevice.cpp +++ b/ASFWDriver/Audio/DriverKit/ASFWAudioDevice.cpp @@ -595,13 +595,17 @@ kern_return_t ASFWAudioDevice::StopIO(IOUserAudioStartStopFlags in_flags) { } kern_return_t ASFWAudioDevice::HandleChangeSampleRate(double in_sample_rate) { + ASFW_LOG(Audio, "ASFWAudioDevice: HandleChangeSampleRate %.0f Hz (entry)", in_sample_rate); if (!ivars || !ivars->driverIvars) { + ASFW_LOG(Audio, + "ASFWAudioDevice: HandleChangeSampleRate NOT READY (ivars=%p driverIvars=%p)", + static_cast(ivars), + static_cast(ivars ? ivars->driverIvars : nullptr)); return kIOReturnNotReady; } auto& ivars = *this->ivars->driverIvars; const uint32_t rateHz = static_cast(in_sample_rate); - ASFW_LOG(Audio, "ASFWAudioDevice: HandleChangeSampleRate %.0f Hz", in_sample_rate); // Program the device's DICE clock to the new rate via the transport-side // coordinator (CLOCK_SELECT + duplex reconfigure). If streaming, it stops and diff --git a/ASFWDriver/Audio/DriverKit/ASFWAudioDriverGraph.cpp b/ASFWDriver/Audio/DriverKit/ASFWAudioDriverGraph.cpp index a38f6c2a..1635856c 100644 --- a/ASFWDriver/Audio/DriverKit/ASFWAudioDriverGraph.cpp +++ b/ASFWDriver/Audio/DriverKit/ASFWAudioDriverGraph.cpp @@ -293,6 +293,12 @@ kern_return_t BuildAudioGraph(ASFWAudioDriver& driver, } ivars.audioDevice->SetDriverIvars(&ivars); + // Do not let the host save/restore a stale stream format from a prior + // session: the device must come up at the rate this graph selects below, + // and we drive rate changes explicitly through HandleChangeSampleRate. + // (Default behavior is restore-enabled; see IOUserAudioDevice header.) + ivars.audioDevice->SetWantsStreamFormatsRestored(false); + const uint32_t current_period = ivars.audioDevice->GetZeroTimestampPeriod(); ASFW_LOG(Audio, "ASFWAudioDriver: IOUserAudioDevice created. GetZeroTimestampPeriod() confirmed: %u frames", current_period); ASFW_LOG(Audio, @@ -327,16 +333,23 @@ kern_return_t BuildAudioGraph(ASFWAudioDriver& driver, IOUserAudioStreamBasicDescription inputFormats[8] = {}; IOUserAudioStreamBasicDescription outputFormats[8] = {}; const uint32_t formatCount = ivars.device.sampleRateCount > 8 ? 8 : ivars.device.sampleRateCount; + uint32_t currentFormatIndex = 0; for (uint32_t i = 0; i < formatCount; i++) { FillFloat32Format(inputFormats[i], ivars.device.sampleRates[i], ivars.device.inputChannelCount); FillFloat32Format(outputFormats[i], ivars.device.sampleRates[i], ivars.device.outputChannelCount); + if (ivars.device.sampleRates[i] == ivars.device.currentSampleRate) { + currentFormatIndex = i; + } } ASFW_LOG(Audio, - "ASFWAudioDriver: Created %u stream formats input=float32/%u ch output=float32/%u ch", + "ASFWAudioDriver: Created %u stream formats input=float32/%u ch output=float32/%u ch; " + "current format index=%u (%.0f Hz)", formatCount, ivars.device.inputChannelCount, - ivars.device.outputChannelCount); + ivars.device.outputChannelCount, + currentFormatIndex, + ivars.device.sampleRates[currentFormatIndex]); IOMemoryDescriptor* rawOutputMemory = nullptr; IOMemoryDescriptor* rawInputMemory = nullptr; @@ -478,7 +491,7 @@ kern_return_t BuildAudioGraph(ASFWAudioDriver& driver, if (!requireAdkSuccess( "inputStream.SetCurrentStreamFormat", ivars.inputStream->SetCurrentStreamFormat( - &inputFormats[0]))) { + &inputFormats[currentFormatIndex]))) { return error; } @@ -513,7 +526,7 @@ kern_return_t BuildAudioGraph(ASFWAudioDriver& driver, if (!requireAdkSuccess( "outputStream.SetCurrentStreamFormat", ivars.outputStream->SetCurrentStreamFormat( - &outputFormats[0]))) { + &outputFormats[currentFormatIndex]))) { return error; } From 35ca7548f74a6913fbdf96fd41c504c46cdbdb6d Mon Sep 17 00:00:00 2001 From: alicankaralar Date: Sat, 27 Jun 2026 17:26:19 +0200 Subject: [PATCH 06/23] fix(dice): make RequestSampleRateChange a cross-process RPC MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit HandleChangeSampleRate runs on the audio device and called the nub's RequestSampleRateChange, which was declared LOCALONLY. ASFWAudioNub and the audio driver are separate IOService objects across the cross-service seam, so the audio side holds a proxy of the nub whose local ivars are null — the call hit `ivars=null` and returned kIOReturnNotReady, so the HAL reverted the rate. Move RequestSampleRateChange to a non-LOCALONLY virtual (IPC) method, mirroring the existing AudioDriver->ASFWDriver RPC bridges (AllocateTxIsochResources, Get/SetProtocolBooleanControl). It now runs in the nub's own process where the parent -> ServiceContext -> AudioCoordinator chain is valid, and updates the nub's cached rate on success. Also logs the no-registry-record not-ready path in the coordinator. Co-Authored-By: Claude Opus 4.8 --- ASFWDriver/Audio/Core/AudioCoordinator.cpp | 3 ++ ASFWDriver/Audio/DriverKit/ASFWAudioNub.cpp | 33 ++++++++++++++------- ASFWDriver/Audio/DriverKit/ASFWAudioNub.iig | 13 ++++---- 3 files changed, 32 insertions(+), 17 deletions(-) diff --git a/ASFWDriver/Audio/Core/AudioCoordinator.cpp b/ASFWDriver/Audio/Core/AudioCoordinator.cpp index 79437abe..86c07724 100644 --- a/ASFWDriver/Audio/Core/AudioCoordinator.cpp +++ b/ASFWDriver/Audio/Core/AudioCoordinator.cpp @@ -304,6 +304,9 @@ IOReturn AudioCoordinator::RequestClockConfig( const auto* record = registry_.FindByGuid(guid); if (!record) { + ASFW_LOG_WARNING(Audio, + "AudioCoordinator: RequestDiceClockConfig no registry record for GUID=0x%016llx (device not registered yet?)", + guid); return kIOReturnNotReady; } diff --git a/ASFWDriver/Audio/DriverKit/ASFWAudioNub.cpp b/ASFWDriver/Audio/DriverKit/ASFWAudioNub.cpp index c5ed8dd2..c050aff5 100644 --- a/ASFWDriver/Audio/DriverKit/ASFWAudioNub.cpp +++ b/ASFWDriver/Audio/DriverKit/ASFWAudioNub.cpp @@ -564,32 +564,43 @@ kern_return_t IMPL(ASFWAudioNub, FreeTxIsochResources) return ctx->isoch.FreeTxIsochResources(); } -kern_return_t ASFWAudioNub::RequestSampleRateChange(uint32_t sampleRateHz) +// Cross-process RPC (AudioDriver -> ASFWDriver process): runs in the nub's own +// process, so ivars and the parent -> ServiceContext -> AudioCoordinator chain +// are valid here (a LOCALONLY variant would dereference the audio side's proxy +// ivars, which are null). +kern_return_t IMPL(ASFWAudioNub, RequestSampleRateChange) { if (!ivars) { + ASFW_LOG(Audio, "ASFWAudioNub: RequestSampleRateChange not ready (ivars=null)"); return kIOReturnNotReady; } auto* coordinator = GetAudioCoordinator(ivars); if (!coordinator) { + ASFW_LOG(Audio, + "ASFWAudioNub: RequestSampleRateChange not ready (no coordinator) guid=0x%016llx", + ivars->guid); return kIOReturnNotReady; } - uint32_t clockSelect = 0; - if (!ASFW::Audio::DICE::DiceClockSelectForRate( - sampleRateHz, ASFW::Audio::DICE::ClockSource::Internal, clockSelect)) { + // The seam is protocol-neutral: carry only the rate. The DICE adapter + // (MakeDiceClockConfiguration) owns the CLOCK_SELECT register encoding. + const ASFW::Audio::AudioClockConfig desired{ + .sampleRateHz = sampleRateHz, + }; + if (!ASFW::Audio::IsSupportedAudioClockConfig(desired)) { ASFW_LOG(Audio, "ASFWAudioNub: RequestSampleRateChange unsupported rate %u Hz", sampleRateHz); return kIOReturnUnsupported; } - const ASFW::Audio::DICE::DiceDesiredClockConfig desired{ - .sampleRateHz = sampleRateHz, - .clockSelect = clockSelect, - }; ASFW_LOG(Audio, - "ASFWAudioNub: RequestSampleRateChange %u Hz clockSelect=0x%08x guid=0x%016llx", - sampleRateHz, clockSelect, ivars->guid); - return coordinator->RequestDiceClockConfig( + "ASFWAudioNub: RequestSampleRateChange %u Hz guid=0x%016llx", + sampleRateHz, ivars->guid); + const kern_return_t kr = coordinator->RequestClockConfig( ivars->guid, desired, ASFW::Audio::DICE::DiceRestartReason::kSampleRateChange); + if (kr == kIOReturnSuccess) { + ivars->currentSampleRateHz = sampleRateHz; + } + return kr; } void ASFWAudioNub::SetChannelCount(uint32_t channels) diff --git a/ASFWDriver/Audio/DriverKit/ASFWAudioNub.iig b/ASFWDriver/Audio/DriverKit/ASFWAudioNub.iig index c4a876ff..7ac43fd0 100644 --- a/ASFWDriver/Audio/DriverKit/ASFWAudioNub.iig +++ b/ASFWDriver/Audio/DriverKit/ASFWAudioNub.iig @@ -108,16 +108,17 @@ public: void SetStreamMode(uint32_t modeRaw) LOCALONLY; uint32_t GetStreamMode() const LOCALONLY; - // Request a device sample-rate change (CoreAudio HandleChangeSampleRate -> - // DICE CLOCK_SELECT + duplex reconfigure). Bridges to the transport-side - // AudioCoordinator. Returns kIOReturnUnsupported for a rate the device does - // not advertise. - kern_return_t RequestSampleRateChange(uint32_t sampleRateHz) LOCALONLY; - // Device identity for protocol bridge routing void SetGuid(uint64_t guid) LOCALONLY; uint64_t GetGuid() const LOCALONLY; + // Request a device sample-rate change (CoreAudio HandleChangeSampleRate -> + // DICE CLOCK_SELECT + duplex reconfigure). Cross-process RPC bridge + // (AudioDriver -> ASFWDriver process): it runs in the nub's own process so + // the parent -> ServiceContext -> AudioCoordinator chain is valid. Returns + // kIOReturnUnsupported for a rate the device does not advertise. + virtual kern_return_t RequestSampleRateChange(uint32_t sampleRateHz); + // RPC protocol-backed boolean control bridge (AudioDriver -> ASFWDriver process) virtual kern_return_t GetProtocolBooleanControl( uint32_t classIdFourCC, From 32d0d8002996a5e935fd145ed7c0fe1b7fcda62e Mon Sep 17 00:00:00 2001 From: alicankaralar Date: Sat, 27 Jun 2026 18:19:29 +0200 Subject: [PATCH 07/23] fix(dice): apply selected rate on start; gate live rate change MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two fixes toward working sample-rate selection: 1. Device clock now adopts the selected rate on (re)start. A rate pick while idle runs RunIdleClockApply, which persists the rate into session.desiredClock/appliedClock — but RunStartStreaming only honored session.pendingClock, so the selection was dropped and the start forced 48 kHz. The device then clocked at 48k while the host ran at the picked rate (no audio). RunStartStreaming now resolves pendingClock -> desiredClock -> appliedClock -> 48k default, mirroring the bus-reset rebind path. 2. Gate live (hot) rate changes. HandleChangeSampleRate now refuses with kIOReturnBusy while IO is active, so CoreAudio keeps the current rate instead of restarting the duplex transport underneath running IO across the cross-service seam (which desynced the clock and bricked audio until replug). Rate changes apply cleanly on the next idle pick + StartIO. Full hot-swap via RequestDeviceConfigurationChange is a follow-up. Co-Authored-By: Claude Opus 4.8 --- .../Audio/DriverKit/ASFWAudioDevice.cpp | 22 +++++++++++++++---- .../Backends/AudioDuplexCoordinator.cpp | 13 ++++++++--- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/ASFWDriver/Audio/DriverKit/ASFWAudioDevice.cpp b/ASFWDriver/Audio/DriverKit/ASFWAudioDevice.cpp index 85dd7c76..9ed38255 100644 --- a/ASFWDriver/Audio/DriverKit/ASFWAudioDevice.cpp +++ b/ASFWDriver/Audio/DriverKit/ASFWAudioDevice.cpp @@ -607,11 +607,25 @@ kern_return_t ASFWAudioDevice::HandleChangeSampleRate(double in_sample_rate) { const uint32_t rateHz = static_cast(in_sample_rate); + // Reject a rate change while IO is active. Live (hot) reconfiguration would + // restart the duplex transport underneath running IO across the cross-service + // seam, which currently desynchronizes the device clock from the host and + // leaves audio dead until a full stop/replug. Until hot-swap is supported, + // require the device to be stopped: returning an error makes CoreAudio keep + // the current rate rather than believe the hardware moved. The rate then + // changes cleanly on the next idle pick + StartIO. + if (ivars.runtime.isRunning.load(std::memory_order_acquire)) { + ASFW_LOG(Audio, + "ASFWAudioDevice: HandleChangeSampleRate %.0f Hz refused - IO active " + "(stop playback to change sample rate)", + in_sample_rate); + return kIOReturnBusy; + } + // Program the device's DICE clock to the new rate via the transport-side - // coordinator (CLOCK_SELECT + duplex reconfigure). If streaming, it stops and - // re-brings-up at the new rate; if idle, it stores the pending clock for the - // next start. Reject the change if the transport can't apply it so CoreAudio - // does not believe the hardware moved. + // coordinator (CLOCK_SELECT + duplex reconfigure). The device is idle here, + // so this stores/applies the clock for the next StartIO. Reject the change if + // the transport can't apply it so CoreAudio does not believe the hardware moved. if (ivars.device.audioNub) { const kern_return_t kr = ivars.device.audioNub->RequestSampleRateChange(rateHz); diff --git a/ASFWDriver/Audio/Protocols/Backends/AudioDuplexCoordinator.cpp b/ASFWDriver/Audio/Protocols/Backends/AudioDuplexCoordinator.cpp index 886c5eec..4ad1ada2 100644 --- a/ASFWDriver/Audio/Protocols/Backends/AudioDuplexCoordinator.cpp +++ b/ASFWDriver/Audio/Protocols/Backends/AudioDuplexCoordinator.cpp @@ -419,14 +419,21 @@ IOReturn AudioDuplexCoordinator::RunStartStreaming(uint64_t guid) noexcept { } DuplexRestartSession session = LoadSession(guid); - // Honor a clock requested before streaming began (e.g. the user picked a - // non-48k rate in Audio MIDI Setup while idle, stored as pendingClock); - // otherwise default to 48 kHz. + // Honor a clock the user selected before streaming began. A rate pick while + // idle runs the idle clock-apply path, which persists the rate into + // desiredClock / appliedClock (NOT pendingClock) — so resolve all three, + // newest first, mirroring the bus-reset rebind path. Without this the start + // drops the selected rate and forces 48 kHz, leaving the device clocked at + // 48k while the host runs at the picked rate. AudioClockConfig desiredClock{ .sampleRateHz = 48000U, }; if (IsSupportedAudioClockConfig(session.pendingClock)) { desiredClock = session.pendingClock; + } else if (IsSupportedAudioClockConfig(session.desiredClock)) { + desiredClock = session.desiredClock; + } else if (IsSupportedAudioClockConfig(session.appliedClock)) { + desiredClock = session.appliedClock; } const DuplexRestartReason reason = HasRestartIntent(session) ? DICE::ClassifyRestartReason(&session, desiredClock) From d01615a2482f08784b280f422789a04a6d393f8a Mon Sep 17 00:00:00 2001 From: alicankaralar Date: Sat, 27 Jun 2026 18:38:53 +0200 Subject: [PATCH 08/23] feat(amdtp): rate-parametric blocking cadence (rational accumulator) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit First step toward 44.1 kHz playback. The blocking cadence was a 48 kHz integer accumulator (6 frames/cycle, emit 8) — correct only because 48000/8000 is integral. At 44.1 kHz the rate is 5.5125 frames/cycle, so it needs fractional frame accounting. Generalize Blocking48kCadence into a rate-parametric BlockingCadence using a rational accumulator over an 8000 cycles/s denominator (Configure(sampleRateHz, sytInterval)). Cross-validated with FFADO iec61883_cip_fill_header (libffado-2.4.9 src/libstreaming/util/cip.c): data iff floor(ready_samples + rate/8000) >= syt_interval. The old 48 kHz integer form is exactly the rate==48000 special case, so default- constructed callers are byte-identical (Blocking48kCadence kept as an alias); the TX packetizer still gates 48 k for now. Tests: 48 k pattern unchanged; 44.1 k carries 8-frame data blocks with zero drift (exactly 88200 frames / 2 s, 441000 / 10 s). Co-Authored-By: Claude Opus 4.8 --- ASFWDriver/Audio/Wire/AMDTP/AmdtpCadence.cpp | 32 ++++++---- ASFWDriver/Audio/Wire/AMDTP/AmdtpCadence.hpp | 31 +++++++++- tests/audio/BlockingCadenceTests.cpp | 63 ++++++++++++++++++++ 3 files changed, 113 insertions(+), 13 deletions(-) diff --git a/ASFWDriver/Audio/Wire/AMDTP/AmdtpCadence.cpp b/ASFWDriver/Audio/Wire/AMDTP/AmdtpCadence.cpp index 65235b2d..28514806 100644 --- a/ASFWDriver/Audio/Wire/AMDTP/AmdtpCadence.cpp +++ b/ASFWDriver/Audio/Wire/AMDTP/AmdtpCadence.cpp @@ -18,29 +18,39 @@ namespace ASFW::Protocols::Audio::AMDTP { namespace { constexpr uint8_t kFramesPerCycle48k = 6; -constexpr uint8_t kSytInterval48k = 8; } // namespace -void Blocking48kCadence::Reset() noexcept { - phase_ = 0; +void BlockingCadence::Configure(uint32_t sampleRateHz, + uint8_t sytIntervalFrames) noexcept { + framesPerCycleNum_ = sampleRateHz; // rate/8000 frames per cycle, scaled by 8000 + sytIntervalFrames_ = sytIntervalFrames; + sytThresholdNum_ = + static_cast(sytIntervalFrames) * kCyclesPerSecond; + Reset(); +} + +void BlockingCadence::Reset() noexcept { + readyNum_ = 0; totalCycles_ = 0; } -bool Blocking48kCadence::CurrentCycleIsData() const noexcept { - return static_cast(phase_ + kFramesPerCycle48k) >= kSytInterval48k; +bool BlockingCadence::CurrentCycleIsData() const noexcept { + // FFADO cip.c: data iff floor(ready_samples + samples_per_cycle) >= + // syt_interval, i.e. (readyNum_ + rate) >= syt_interval * 8000. + return (readyNum_ + framesPerCycleNum_) >= sytThresholdNum_; } -uint8_t Blocking48kCadence::CurrentCycleDataFrames() const noexcept { - return CurrentCycleIsData() ? kSytInterval48k : 0; +uint8_t BlockingCadence::CurrentCycleDataFrames() const noexcept { + return CurrentCycleIsData() ? sytIntervalFrames_ : 0; } -uint64_t Blocking48kCadence::TotalCycles() const noexcept { +uint64_t BlockingCadence::TotalCycles() const noexcept { return totalCycles_; } -void Blocking48kCadence::AdvanceCycle() noexcept { - phase_ = static_cast(phase_ + kFramesPerCycle48k - - CurrentCycleDataFrames()); +void BlockingCadence::AdvanceCycle() noexcept { + readyNum_ = readyNum_ + framesPerCycleNum_ - + (CurrentCycleIsData() ? sytThresholdNum_ : 0u); ++totalCycles_; } diff --git a/ASFWDriver/Audio/Wire/AMDTP/AmdtpCadence.hpp b/ASFWDriver/Audio/Wire/AMDTP/AmdtpCadence.hpp index 35cec74d..aa4d1d5a 100644 --- a/ASFWDriver/Audio/Wire/AMDTP/AmdtpCadence.hpp +++ b/ASFWDriver/Audio/Wire/AMDTP/AmdtpCadence.hpp @@ -29,8 +29,22 @@ class IAmdtpCadence { virtual void AdvanceCycle() noexcept = 0; }; -class Blocking48kCadence final : public IAmdtpCadence { +// Blocking-mode AMDTP cadence for any 1x/2x/4x rate. Defaults to 48 kHz so +// existing default-constructed callers are byte-identical to the old +// Blocking48kCadence. Configure() switches the rate. +// +// Cross-validated with FFADO iec61883_cip_fill_header (libffado-2.4.9 +// src/libstreaming/util/cip.c:130-160): the blocking cadence is a rational +// accumulator of rate/8000 frames per 125 us bus cycle that emits a +// syt_interval-frame data block whenever at least syt_interval frames are +// pending; otherwise a NO-DATA packet. The previous 48 kHz integer form +// (6 frames/cycle, emit 8) is exactly the rate==48000 special case. +class BlockingCadence final : public IAmdtpCadence { public: + // sytIntervalFrames is the blocking frames-per-data-packet: 8 @1x + // (32/44.1/48 k), 16 @2x, 32 @4x (FFADO getSytInterval()). + void Configure(uint32_t sampleRateHz, uint8_t sytIntervalFrames) noexcept; + void Reset() noexcept override; bool CurrentCycleIsData() const noexcept override; @@ -40,10 +54,23 @@ class Blocking48kCadence final : public IAmdtpCadence { void AdvanceCycle() noexcept override; private: - uint8_t phase_{0}; + // Rational accumulator over an 8000 cycles/s denominator: readyNum_ counts + // pending frames * 8000. Each cycle adds framesPerCycleNum_ (== sampleRateHz, + // i.e. rate/8000 frames scaled by 8000) and emits a block of + // sytIntervalFrames_ when readyNum_ + framesPerCycleNum_ reaches + // sytThresholdNum_ (== sytIntervalFrames_ * 8000). Exact, drift-free for + // fractional rates such as 44.1 kHz (5.5125 frames/cycle). + static constexpr uint32_t kCyclesPerSecond = 8000; + uint32_t framesPerCycleNum_{48000}; + uint32_t sytThresholdNum_{8u * kCyclesPerSecond}; + uint8_t sytIntervalFrames_{8}; + uint32_t readyNum_{0}; uint64_t totalCycles_{0}; }; +// Transitional alias: existing callers default-construct this for 48 kHz. +using Blocking48kCadence = BlockingCadence; + class NonBlocking48kCadence final : public IAmdtpCadence { public: void Reset() noexcept override; diff --git a/tests/audio/BlockingCadenceTests.cpp b/tests/audio/BlockingCadenceTests.cpp index 8980c84f..4be9062f 100644 --- a/tests/audio/BlockingCadenceTests.cpp +++ b/tests/audio/BlockingCadenceTests.cpp @@ -141,6 +141,69 @@ TEST(BlockingCadenceTests, ResetClearsState) { // Reference: 000-48kORIG.txt cycles 977-984 //============================================================================== +//============================================================================== +// Rate-parametric cadence (44.1 kHz). Cross-validated with FFADO +// iec61883_cip_fill_header (libffado-2.4.9 src/libstreaming/util/cip.c): the +// blocking cadence is a rational accumulator of rate/8000 frames per cycle. +//============================================================================== + +TEST(BlockingCadenceTests, ConfigureFortyEightKMatchesDefaultPattern) { + BlockingCadence cadence; + cadence.Configure(48000, 8); + + bool expected[] = {false, true, true, true, false, true, true, true}; + for (int i = 0; i < 8; i++) { + SCOPED_TRACE("Cycle " + std::to_string(i)); + EXPECT_EQ(cadence.CurrentCycleIsData(), expected[i]); + cadence.AdvanceCycle(); + } +} + +TEST(BlockingCadenceTests, FortyFourKFirstCycleIsNoData) { + BlockingCadence cadence; + cadence.Configure(44100, 8); + // 44100/8000 = 5.5125 frames pending after cycle 0 -> < syt_interval(8). + EXPECT_FALSE(cadence.CurrentCycleIsData()); + EXPECT_EQ(cadence.CurrentCycleDataFrames(), 0); +} + +TEST(BlockingCadenceTests, FortyFourKDataPacketsCarryEightFrames) { + BlockingCadence cadence; + cadence.Configure(44100, 8); + for (int i = 0; i < 64; ++i) { + if (cadence.CurrentCycleIsData()) { + EXPECT_EQ(cadence.CurrentCycleDataFrames(), 8); + } else { + EXPECT_EQ(cadence.CurrentCycleDataFrames(), 0); + } + cadence.AdvanceCycle(); + } +} + +TEST(BlockingCadenceTests, FortyFourKExactOverTwoSeconds) { + BlockingCadence cadence; + cadence.Configure(44100, 8); + uint64_t totalSamples = 0; + // 44100 * 16000 / 64000 = 11025 data cycles exactly -> 88200 frames, with + // the fractional accumulator returning to zero residual every 2 seconds. + for (int i = 0; i < 16000; ++i) { + totalSamples += cadence.CurrentCycleDataFrames(); + cadence.AdvanceCycle(); + } + EXPECT_EQ(totalSamples, 88200u); +} + +TEST(BlockingCadenceTests, FortyFourKNoDriftAcrossTenSeconds) { + BlockingCadence cadence; + cadence.Configure(44100, 8); + uint64_t totalSamples = 0; + for (int i = 0; i < 80000; ++i) { // 10 s + totalSamples += cadence.CurrentCycleDataFrames(); + cadence.AdvanceCycle(); + } + EXPECT_EQ(totalSamples, 441000u); +} + TEST(BlockingCadenceTests, MatchesFireBugPattern) { Blocking48kCadence cadence; From 0c3492d3948f2353dd47ff6088c48a17305d3242 Mon Sep 17 00:00:00 2001 From: alicankaralar Date: Sat, 27 Jun 2026 18:49:00 +0200 Subject: [PATCH 09/23] feat(amdtp): drive TX cadence + FDF from the live sample rate Wire the rate-parametric cadence into the playback path so 44.1 kHz produces a correct stream instead of a 48 kHz cadence: - AmdtpTxPacketizer::Configure no longer hard-rejects non-48k. It resolves the rate via AmdtpRateGeometryForSampleRate, configures the blocking cadence with the rate + SYT interval, and sets the CIP FDF from the rate's AM824 SFC (44.1k=0x01, 48k=0x02) instead of the profile's hardcoded 0x02. Non-blocking stays integral-rate (48k) only. - AmdtpRateGeometry carries the per-rate FDF/SFC (cross-checked vs FFADO getFDF()). - StartIO overrides the profile's default 48k txConfig.sampleRate with the device's current nominal rate for both the primary and secondary (Venice 2x16) playback streams. Known remaining gap: the core endpoint runtime's config_.currentSampleRate (binding/RX clock nominal) is only refreshed via OnAVCAudioConfigurationReady, not on a DICE rate change, so it can stay 48k. That mainly affects RX/ppm diagnostics; TX SYT follows the device clock. To be verified on hardware. Co-Authored-By: Claude Opus 4.8 --- .../Audio/DriverKit/ASFWAudioDevice.cpp | 10 +++++++ .../Audio/Wire/AMDTP/AmdtpRateGeometry.hpp | 17 ++++++----- .../Audio/Wire/AMDTP/AmdtpTxPacketizer.cpp | 29 +++++++++++++++---- 3 files changed, 44 insertions(+), 12 deletions(-) diff --git a/ASFWDriver/Audio/DriverKit/ASFWAudioDevice.cpp b/ASFWDriver/Audio/DriverKit/ASFWAudioDevice.cpp index 9ed38255..35103de8 100644 --- a/ASFWDriver/Audio/DriverKit/ASFWAudioDevice.cpp +++ b/ASFWDriver/Audio/DriverKit/ASFWAudioDevice.cpp @@ -165,6 +165,12 @@ kern_return_t ASFWAudioDevice::StartIO(IOUserAudioStartStopFlags in_flags) { kr = failStart(kIOReturnError, "BuildDefaultTxStreamConfig"); return; } + // The profile describes the wire geometry at its default (48 kHz); + // the live cadence/FDF follow the device's current nominal rate. + if (ivars.device.currentSampleRate > 0) { + txConfig.sampleRate = + static_cast(ivars.device.currentSampleRate); + } const uint32_t numSlots = ASFW::IsochTransport::AudioTimingGeometry::kTxSharedSlotPackets; @@ -260,6 +266,10 @@ kern_return_t ASFWAudioDevice::StartIO(IOUserAudioStartStopFlags in_flags) { kr = failStart(kIOReturnError, "BuildDefaultTxStreamConfig2"); return; } + if (ivars.device.currentSampleRate > 0) { + txConfig2.sampleRate = + static_cast(ivars.device.currentSampleRate); + } txConfig2.sourceChannelOffset = txConfig2.pcmChannels; const uint32_t numSlots2 = diff --git a/ASFWDriver/Audio/Wire/AMDTP/AmdtpRateGeometry.hpp b/ASFWDriver/Audio/Wire/AMDTP/AmdtpRateGeometry.hpp index dec1fecb..f8af376a 100644 --- a/ASFWDriver/Audio/Wire/AMDTP/AmdtpRateGeometry.hpp +++ b/ASFWDriver/Audio/Wire/AMDTP/AmdtpRateGeometry.hpp @@ -47,18 +47,21 @@ struct AmdtpRateGeometry final { uint32_t sampleRateHz{0}; uint32_t nominalFramesPerCycle{0}; uint32_t sytIntervalFrames{0}; + // AM824 FDF: the IEC 61883-6 SFC sample-rate code (== SampleRate enum value). + // FFADO AmdtpTransmitStreamProcessor::getFDF() maps the same rates to these. + uint8_t fdf{0}; }; [[nodiscard]] constexpr std::optional AmdtpRateGeometryForSampleRate(uint32_t sampleRateHz) noexcept { switch (sampleRateHz) { - case 32000: return AmdtpRateGeometry{32000, 4, 8}; - case 44100: return AmdtpRateGeometry{44100, 6, 8}; - case 48000: return AmdtpRateGeometry{48000, 6, 8}; - case 88200: return AmdtpRateGeometry{88200, 12, 16}; - case 96000: return AmdtpRateGeometry{96000, 12, 16}; - case 176400: return AmdtpRateGeometry{176400, 24, 32}; - case 192000: return AmdtpRateGeometry{192000, 24, 32}; + case 32000: return AmdtpRateGeometry{32000, 4, 8, 0}; + case 44100: return AmdtpRateGeometry{44100, 6, 8, 1}; + case 48000: return AmdtpRateGeometry{48000, 6, 8, 2}; + case 88200: return AmdtpRateGeometry{88200, 12, 16, 3}; + case 96000: return AmdtpRateGeometry{96000, 12, 16, 4}; + case 176400: return AmdtpRateGeometry{176400, 24, 32, 5}; + case 192000: return AmdtpRateGeometry{192000, 24, 32, 6}; default: return std::nullopt; } } diff --git a/ASFWDriver/Audio/Wire/AMDTP/AmdtpTxPacketizer.cpp b/ASFWDriver/Audio/Wire/AMDTP/AmdtpTxPacketizer.cpp index 0c169e32..e470fb77 100644 --- a/ASFWDriver/Audio/Wire/AMDTP/AmdtpTxPacketizer.cpp +++ b/ASFWDriver/Audio/Wire/AMDTP/AmdtpTxPacketizer.cpp @@ -1,5 +1,6 @@ #include "AmdtpTxPacketizer.hpp" +#include "AmdtpRateGeometry.hpp" #include "../IEC61883/Syt.hpp" namespace ASFW::Protocols::Audio::AMDTP { @@ -40,11 +41,24 @@ inline void WriteBE32(uint8_t* dest, uint32_t value) noexcept { bool AmdtpTxPacketizer::Configure(const AmdtpStreamConfig& streamConfig, const AmdtpTxPolicy& txPolicy) noexcept { - if (streamConfig.sampleRate != 48000) { - return false; // only 48 kHz cadences exist; reject untested rates + // Resolve the rate's AMDTP geometry (SYT interval + AM824 FDF/SFC). Unknown + // rates are rejected. Blocking mode handles any rate via the rational + // cadence; non-blocking has no fractional form, so it stays integral-rate + // (48 kHz) only. + const auto geometry = + ASFW::Encoding::AmdtpRateGeometryForSampleRate(streamConfig.sampleRate); + if (!geometry) { + return false; + } + if (streamConfig.streamMode != StreamMode::Blocking && + streamConfig.sampleRate != 48000) { + return false; } AmdtpStreamConfig config = streamConfig; + // FDF (AM824 SFC) must match the actual rate, not whatever the profile + // defaulted (profiles hardcode the 48 kHz SFC 0x02). + config.fdf = geometry->fdf; if (config.dbs == 0) { config.dbs = static_cast(config.pcmChannels + config.midiSlots); } @@ -74,9 +88,14 @@ bool AmdtpTxPacketizer::Configure(const AmdtpStreamConfig& streamConfig, txPolicy.preserveFdfInNoDataPackets ? config.fdf : 0xFF; cipBuilder_.Configure(cipConfig); - cadence_ = (config.streamMode == StreamMode::Blocking) - ? static_cast(&blocking48kCadence_) - : static_cast(&nonBlocking48kCadence_); + if (config.streamMode == StreamMode::Blocking) { + blocking48kCadence_.Configure( + config.sampleRate, + static_cast(geometry->sytIntervalFrames)); + cadence_ = static_cast(&blocking48kCadence_); + } else { + cadence_ = static_cast(&nonBlocking48kCadence_); + } Reset(0, 0); return true; From b5dce90881a047055e3e273446cbbc7f1024fd27 Mon Sep 17 00:00:00 2001 From: alicankaralar Date: Sat, 27 Jun 2026 19:06:20 +0200 Subject: [PATCH 10/23] fix(isoch): reset TX control-block cursors on StartIO MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 44.1 kHz playback never started: the IT prime failed with "committed prefill=7910 must cover 48 descriptors within 384 slots". Root cause: TxStreamControl had no producer-side cursor reset. On a sample-rate change CoreAudio re-probes the device with several rapid StartIO/StopIO cycles, and the shared TX slab is reused without being re-zeroed, so exposeCursor/completionCursor carried over and grew past the 384-slot ring — failing IsochTxDmaRing::Prime (which requires numPackets <= exposeCursor <= numSlots). 48 kHz never tripped this because it starts once and runs. Add TxStreamControl::ResetForStart() (clears the runtime progress cursors, preserves the core-written geometry) and call it in StartIO after mapping each TX control block (primary + Venice secondary), before the prefill and before the transport consumer arms. Makes repeated StartIO idempotent regardless of slab reuse. Co-Authored-By: Claude Opus 4.8 --- .../Audio/DriverKit/ASFWAudioDevice.cpp | 8 +++++++ .../Shared/Isoch/IsochAudioTransport.hpp | 21 +++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/ASFWDriver/Audio/DriverKit/ASFWAudioDevice.cpp b/ASFWDriver/Audio/DriverKit/ASFWAudioDevice.cpp index 35103de8..9bfc60b8 100644 --- a/ASFWDriver/Audio/DriverKit/ASFWAudioDevice.cpp +++ b/ASFWDriver/Audio/DriverKit/ASFWAudioDevice.cpp @@ -218,6 +218,12 @@ kern_return_t ASFWAudioDevice::StartIO(IOUserAudioStartStopFlags in_flags) { auto* metadataRing = reinterpret_cast(ivars.txMetadataMap->GetAddress()); auto* controlBlock = reinterpret_cast(ivars.txControlMap->GetAddress()); + // Clear stale runtime cursors before prefill: the shared slab can be + // reused across StartIO/StopIO probes (CoreAudio re-probes on a + // sample-rate change), and a carried-over exposeCursor fails the IT + // prime ("committed prefill > slots"). + controlBlock->ResetForStart(); + ivars.runtime.txSlotProvider.payloadBase = payloadBase; ivars.runtime.txSlotProvider.metadataRing = metadataRing; ivars.runtime.txSlotProvider.controlBlock = controlBlock; @@ -304,6 +310,8 @@ kern_return_t ASFWAudioDevice::StartIO(IOUserAudioStartStopFlags in_flags) { auto* metadataRing2 = reinterpret_cast(ivars.txMetadataMapSecondary->GetAddress()); auto* controlBlock2 = reinterpret_cast(ivars.txControlMapSecondary->GetAddress()); + controlBlock2->ResetForStart(); + ivars.runtime.txSlotProviderSecondary.payloadBase = payloadBase2; ivars.runtime.txSlotProviderSecondary.metadataRing = metadataRing2; ivars.runtime.txSlotProviderSecondary.controlBlock = controlBlock2; diff --git a/ASFWDriver/Shared/Isoch/IsochAudioTransport.hpp b/ASFWDriver/Shared/Isoch/IsochAudioTransport.hpp index 94625d8e..971963e0 100644 --- a/ASFWDriver/Shared/Isoch/IsochAudioTransport.hpp +++ b/ASFWDriver/Shared/Isoch/IsochAudioTransport.hpp @@ -350,6 +350,27 @@ struct TxStreamControl final { ///< (end-exclusive absolute index). TxProducerFailureSnapshot producerFailure{}; + // Clears all runtime progress cursors to a clean pre-start state while + // preserving the core-written geometry (numSlots, strides, ABI). The shared + // buffer is not guaranteed to be freshly zeroed across StartIO/StopIO cycles + // (CoreAudio re-probes a device on a sample-rate change, reusing the slab), + // so without this exposeCursor/completionCursor carry over and the IT prime + // fails ("committed prefill > slots"). The producer owns this reset; it runs + // in StartIO before the prefill and before the transport consumer arms. + void ResetForStart() noexcept { + startCycleMatch.store(0, std::memory_order_relaxed); + startFirstPacketIndex.store(0, std::memory_order_relaxed); + completionCursor.store(0, std::memory_order_relaxed); + completionStampCount.store(0, std::memory_order_relaxed); + preparationRequestGeneration.store(0, std::memory_order_relaxed); + preparationHandledGeneration.store(0, std::memory_order_relaxed); + preparationRequestHostTicks.store(0, std::memory_order_relaxed); + preparationRequestCount.store(0, std::memory_order_relaxed); + preparationCoalescedCount.store(0, std::memory_order_relaxed); + producerFailure.Reset(); + exposeCursor.store(0, std::memory_order_release); + } + void MarkPreparationHandled(uint64_t generation) noexcept { uint64_t handled = preparationHandledGeneration.load(std::memory_order_relaxed); From dafa5afb2d64cb40568920938dfce63873dfa8b0 Mon Sep 17 00:00:00 2001 From: alicankaralar Date: Sat, 27 Jun 2026 19:25:42 +0200 Subject: [PATCH 11/23] fix(dice): sync endpoint clock to the new rate (stop 44.1k StartIO churn) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After the prime fix, 44.1k TX started but CoreAudio churned StartIO/StopIO every ~1s and shipped only silence. The host streams were correctly at 44100 (ADK STATE rate=44100), but the RX direct-binding clock stayed at 48000 (IR: Arming direct Rx ... rate=48000): the core endpoint's config_.currentSampleRate is seeded at publish (48k) and a DICE rate change never updated it. The ZTS the driver reports to CoreAudio then advanced at 48k while the device ran 44.1k (~8.8% error), so CoreAudio rejected the device and restarted it repeatedly — never settling, so Logic never delivered real audio. Add AudioEndpointRuntime::SetCurrentSampleRate and call it from AudioCoordinator::RequestDiceClockConfig on success, so the next StartIO seeds the direct-binding/ZTS clock at the live rate. Co-Authored-By: Claude Opus 4.8 --- ASFWDriver/Audio/Core/AudioCoordinator.cpp | 8 ++++++++ ASFWDriver/Audio/Core/AudioEndpointRuntime.hpp | 18 ++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/ASFWDriver/Audio/Core/AudioCoordinator.cpp b/ASFWDriver/Audio/Core/AudioCoordinator.cpp index 86c07724..3f907f89 100644 --- a/ASFWDriver/Audio/Core/AudioCoordinator.cpp +++ b/ASFWDriver/Audio/Core/AudioCoordinator.cpp @@ -324,6 +324,14 @@ IOReturn AudioCoordinator::RequestClockConfig( return kr; } + // Keep the endpoint's clock in step with the new device rate so the next + // StartIO seeds the direct-binding/ZTS clock at the live rate. Without this + // the binding stays at the publish-time rate (48 kHz) while the device runs + // 44.1 kHz, and CoreAudio churns StartIO/StopIO on the clock mismatch. + if (auto endpoint = runtime_.EnsureEndpointRuntime(guid)) { + endpoint->SetCurrentSampleRate(desiredClock.sampleRateHz); + } + ASFW_LOG(Audio, "AudioCoordinator: RequestClockConfig ok GUID=0x%016llx rate=%uHz reason=%u", guid, diff --git a/ASFWDriver/Audio/Core/AudioEndpointRuntime.hpp b/ASFWDriver/Audio/Core/AudioEndpointRuntime.hpp index b9414965..8584ef38 100644 --- a/ASFWDriver/Audio/Core/AudioEndpointRuntime.hpp +++ b/ASFWDriver/Audio/Core/AudioEndpointRuntime.hpp @@ -53,6 +53,24 @@ class AudioEndpointRuntime final : public Runtime::IDirectAudioBindingSource { configValid_.store(true, std::memory_order_release); } + // Update only the current sample rate. The DICE clock can change (Audio MIDI + // Setup / Logic) without a full config rebuild; the next StartIO seeds the + // direct-binding/ZTS clock from config_.currentSampleRate, so this must + // reflect the live rate or CoreAudio sees the device clock advancing at the + // wrong rate and churns StartIO/StopIO. + void SetCurrentSampleRate(uint32_t sampleRateHz) noexcept { + if (sampleRateHz == 0) { + return; + } + if (lock_) { + IOLockLock(lock_); + } + config_.currentSampleRate = sampleRateHz; + if (lock_) { + IOLockUnlock(lock_); + } + } + [[nodiscard]] bool CopyConfig(Model::ASFWAudioDevice& outConfig) const noexcept { if (!configValid_.load(std::memory_order_acquire)) { return false; From 7b2f85ff19f79fc796ff68f010310c7f69f47c67 Mon Sep 17 00:00:00 2001 From: alicankaralar Date: Sun, 28 Jun 2026 01:39:44 +0200 Subject: [PATCH 12/23] fix(dice): refresh direct-binding rate + generation on rate change SetCurrentSampleRate updated config_.currentSampleRate but not directSampleRateHz_, the field CopyDirectAudioBinding actually reports to the IR. directSampleRateHz_ is latched only when EnsureDirectAudioMemoryLocked allocates the (rate-independent) ring buffers, which never re-runs on an idle rate change. So the RX/ZTS clock stayed at the publish-time 48 kHz while the device ran 44.1 kHz and CoreAudio churned StartIO/StopIO on the mismatch. Refresh directSampleRateHz_ and bump the binding generation here so the IR re-arms the clock at the live rate; buffers are unchanged, no reallocation. Co-Authored-By: Claude Opus 4.8 --- .../Audio/Core/AudioEndpointRuntime.hpp | 21 ++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/ASFWDriver/Audio/Core/AudioEndpointRuntime.hpp b/ASFWDriver/Audio/Core/AudioEndpointRuntime.hpp index 8584ef38..41fcf5a6 100644 --- a/ASFWDriver/Audio/Core/AudioEndpointRuntime.hpp +++ b/ASFWDriver/Audio/Core/AudioEndpointRuntime.hpp @@ -55,9 +55,20 @@ class AudioEndpointRuntime final : public Runtime::IDirectAudioBindingSource { // Update only the current sample rate. The DICE clock can change (Audio MIDI // Setup / Logic) without a full config rebuild; the next StartIO seeds the - // direct-binding/ZTS clock from config_.currentSampleRate, so this must - // reflect the live rate or CoreAudio sees the device clock advancing at the - // wrong rate and churns StartIO/StopIO. + // direct-binding/ZTS clock from the live rate, so this must reflect it or + // CoreAudio sees the device clock advancing at the wrong rate and churns + // StartIO/StopIO. + // + // The binding the IR actually reads reports directSampleRateHz_, not + // config_.currentSampleRate. That field (and directGeneration_) is latched + // only when the direct-audio memory is allocated in + // EnsureDirectAudioMemoryLocked, which does not re-run on an idle rate change + // because the ring buffers are rate-independent (fixed kAudioRingBufferFrames) + // and are allocated once at bring-up then reused. So updating config_ alone + // leaves the binding (and the ZTS the HAL judges) stuck at the publish-time + // 48 kHz. Refresh directSampleRateHz_ and bump the generation here so + // CopyDirectAudioBinding reports the new rate and the IR re-arms the RX/ZTS + // clock at it. void SetCurrentSampleRate(uint32_t sampleRateHz) noexcept { if (sampleRateHz == 0) { return; @@ -66,6 +77,10 @@ class AudioEndpointRuntime final : public Runtime::IDirectAudioBindingSource { IOLockLock(lock_); } config_.currentSampleRate = sampleRateHz; + if (directSampleRateHz_ != 0 && directSampleRateHz_ != sampleRateHz) { + directSampleRateHz_ = sampleRateHz; + ++directGeneration_; + } if (lock_) { IOLockUnlock(lock_); } From 134c70381c2fab5d608c9a9cf2a99d7dbb8b8216 Mon Sep 17 00:00:00 2001 From: alicankaralar Date: Sun, 28 Jun 2026 01:39:44 +0200 Subject: [PATCH 13/23] fix(dice): keep CLOCK_SELECT at the selected rate across StartIO PrepareDuplex48k, the per-StartIO bring-up hook, hardcoded a 48 kHz CLOCK_SELECT and wrote it to the device on every StartIO. After an idle change to 44.1 kHz, each StartIO rewrote 48 kHz and fought the rate-change path: the device PLL flapped 44.1k<->48k (RX FDF 0x01<->0x02) and audio starved. Remember the last applied clock (ApplyClockConfig / PrepareDuplex) in selectedClock_ and target it from PrepareDuplex48k, falling back to 48 kHz only before any rate has been selected. Co-Authored-By: Claude Opus 4.8 --- .../Protocols/DICE/TCAT/DICETcatProtocol.cpp | 27 ++++++++++++++++--- .../Protocols/DICE/TCAT/DICETcatProtocol.hpp | 9 +++++++ 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/ASFWDriver/Audio/Protocols/DICE/TCAT/DICETcatProtocol.cpp b/ASFWDriver/Audio/Protocols/DICE/TCAT/DICETcatProtocol.cpp index eecb8a53..1fca76ea 100644 --- a/ASFWDriver/Audio/Protocols/DICE/TCAT/DICETcatProtocol.cpp +++ b/ASFWDriver/Audio/Protocols/DICE/TCAT/DICETcatProtocol.cpp @@ -163,6 +163,12 @@ void DICETcatProtocol::PrepareDuplex(const AudioDuplexChannels& channels, return; } + // Remember the live clock so a later per-StartIO PrepareDuplex48k targets it + // rather than reverting the device to 48 kHz (see selectedClock_). + if (desiredClock.sampleRateHz != 0) { + selectedClock_ = desiredClock; + } + duplexCtrl_->PrepareDuplex( channels, diceClock, @@ -220,6 +226,13 @@ void DICETcatProtocol::ApplyClockConfig(const AudioClockConfig& desiredClock, return; } + // An idle sample-rate change lands here (RunIdleClockApply). Remember it so + // the next StartIO's PrepareDuplex48k keeps the device at this rate instead + // of rewriting CLOCK_SELECT back to 48 kHz (see selectedClock_). + if (desiredClock.sampleRateHz != 0) { + selectedClock_ = desiredClock; + } + duplexCtrl_->ApplyClockConfig( diceClock, [this, callback = std::move(callback)](IOReturn status, DiceClockApplyResult result) mutable { @@ -278,10 +291,18 @@ void DICETcatProtocol::ReadDuplexHealth(HealthCallback callback) { } void DICETcatProtocol::PrepareDuplex48k(const AudioDuplexChannels& channels, VoidCallback callback) { + // This per-StartIO bring-up must honor the user's selected clock. Using a + // hardcoded 48 kHz here rewrites CLOCK_SELECT on every StartIO and fights an + // idle 44.1 kHz change (the device PLL flaps 44.1k<->48k and audio starves). + // Fall back to 48 kHz only before any rate has been selected. + AudioClockConfig clock = selectedClock_; + if (clock.sampleRateHz == 0) { + clock = AudioClockConfig{ + .sampleRateHz = 48000U, + }; + } PrepareDuplex(channels, - AudioClockConfig{ - .sampleRateHz = 48000U, - }, + clock, [callback = std::move(callback)](IOReturn status, DiceDuplexPrepareResult) mutable { callback(status); }); diff --git a/ASFWDriver/Audio/Protocols/DICE/TCAT/DICETcatProtocol.hpp b/ASFWDriver/Audio/Protocols/DICE/TCAT/DICETcatProtocol.hpp index a501b2b8..3909b5b8 100644 --- a/ASFWDriver/Audio/Protocols/DICE/TCAT/DICETcatProtocol.hpp +++ b/ASFWDriver/Audio/Protocols/DICE/TCAT/DICETcatProtocol.hpp @@ -95,6 +95,15 @@ class DICETcatProtocol final : public Audio::IDeviceProtocol, bool initialized_{false}; bool sectionsLoaded_{false}; + // The user-selected device clock, remembered across StartIO cycles so the + // per-StartIO bring-up (PrepareDuplex48k) targets the live rate instead of a + // hardcoded 48 kHz. Updated whenever a real clock is applied (ApplyClockConfig + // for idle rate changes, PrepareDuplex for restarts). Default {0} means + // "nothing selected yet" → PrepareDuplex48k falls back to 48 kHz. Without this + // every StartIO rewrites CLOCK_SELECT back to 48 kHz and fights a 44.1 kHz + // selection, flapping the device PLL and starving audio. + AudioClockConfig selectedClock_{}; + std::atomic runtimeSampleRateHz_{0}; std::atomic hostInputPcmChannels_{0}; std::atomic hostOutputPcmChannels_{0}; From b86d6d32616427dfca12288cead47997f5335c9e Mon Sep 17 00:00:00 2001 From: alicankaralar Date: Sun, 28 Jun 2026 01:39:44 +0200 Subject: [PATCH 14/23] chore(audio): log secondary TX stream rate at configure The secondary (multi-stream) TX configure line omitted the sample rate, so the log could not confirm both Venice F32 sub-streams pick up a rate change. Add rate= to match the primary stream's line. Co-Authored-By: Claude Opus 4.8 --- ASFWDriver/Audio/DriverKit/ASFWAudioDevice.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ASFWDriver/Audio/DriverKit/ASFWAudioDevice.cpp b/ASFWDriver/Audio/DriverKit/ASFWAudioDevice.cpp index 9bfc60b8..5426f1ac 100644 --- a/ASFWDriver/Audio/DriverKit/ASFWAudioDevice.cpp +++ b/ASFWDriver/Audio/DriverKit/ASFWAudioDevice.cpp @@ -330,8 +330,9 @@ kern_return_t ASFWAudioDevice::StartIO(IOUserAudioStartStopFlags in_flags) { ivars.runtime.txSecondaryActive = true; ASFW_LOG(Audio, - "ASFWAudioDevice: Allocated & configured SECONDARY TX stream offset=%u dbs=%u slots=%u slotSize=%u", - txConfig2.sourceChannelOffset, txConfig2.dbs, numSlots2, maxPacketBytes2); + "ASFWAudioDevice: Allocated & configured SECONDARY TX stream offset=%u dbs=%u slots=%u slotSize=%u rate=%u", + txConfig2.sourceChannelOffset, txConfig2.dbs, numSlots2, maxPacketBytes2, + txConfig2.sampleRate); } } From 1fc963b6e8520b500fbac5b573649858cdb1d5dd Mon Sep 17 00:00:00 2001 From: alicankaralar Date: Sun, 28 Jun 2026 11:42:28 +0200 Subject: [PATCH 15/23] fix(dice): wait for stable PLL lock before enabling streams The bring-up clock "confirm" could fire on the device's CLOCK_ACCEPTED ack alone (notify bits) while the PLL was still relocking (status reports the old rate / locked=0), so isoch streams were programmed/enabled on an unstable clock. Add DoAwaitStreamingClockLock between clock-confirm and stream discovery in the kPrepareDuplex path: poll global status until IsSourceLocked and the nominal+sample rate equal the target, up to the existing 2 s timeout, then enable. A same-rate restart is already locked so it returns on the first read; kClockApply is untouched (it does not enable streams). HW: verified the gate works (round trip 48k<->44.1k shows "stable-locked ... enabling streams" only after locked=1, FDF tracks 0x02<->0x01). NOTE this corrects the clock layer but does NOT fix the rate-change audio loss -- that root cause is device-side dual-stream re-establishment, not the clock. Co-Authored-By: Claude Opus 4.8 --- .../DICE/Core/DICEDuplexBringupController.cpp | 59 ++++++++++++++++++- .../DICE/Core/DICEDuplexBringupController.hpp | 4 ++ 2 files changed, 61 insertions(+), 2 deletions(-) diff --git a/ASFWDriver/Audio/Protocols/DICE/Core/DICEDuplexBringupController.cpp b/ASFWDriver/Audio/Protocols/DICE/Core/DICEDuplexBringupController.cpp index 328b0dc9..8739e6dd 100644 --- a/ASFWDriver/Audio/Protocols/DICE/Core/DICEDuplexBringupController.cpp +++ b/ASFWDriver/Audio/Protocols/DICE/Core/DICEDuplexBringupController.cpp @@ -602,7 +602,7 @@ void DICEDuplexBringupController::DoActiveClockCheck( DoCompleteClockApply(std::move(cb)); return; } - DoDiscoverStreams(channels, 0, std::move(cb)); + DoAwaitStreamingClockLock(channels, 0, std::move(cb)); return; } @@ -739,10 +739,65 @@ void DICEDuplexBringupController::DoReadGlobalAfterClockAccepted( return; } - DoDiscoverStreams(channels, 0, std::move(cb)); + DoAwaitStreamingClockLock(channels, 0, std::move(cb)); }); } +void DICEDuplexBringupController::DoAwaitStreamingClockLock( + AudioDuplexChannels channels, + uint32_t attempt, + VoidCallback cb) { + if (!EnsureGenerationCurrent()) { + DoRollback(kIOReturnOffline, std::move(cb)); + return; + } + + // The clock "confirm" above can fire on the device's CLOCK_ACCEPTED ack alone, + // which only means the device received the CLOCK_SELECT write -- NOT that its PLL + // has re-locked at the new rate. Enabling the isoch streams while the PLL is still + // relocking (status reports the old rate / locked=0) wedges the device until a hard + // reset: a rate change then kills audio permanently, even after switching back to + // 48 kHz (HW-observed). So before programming/enabling streams, poll the global + // status until it reports a stable lock at the target rate. A same-rate restart is + // already locked, so this returns on the first read. + diceReader_.ReadGlobalStateFull( + sections_, + [this, channels, attempt, cb = std::move(cb)](IOReturn status, GlobalState state) mutable { + if (status != kIOReturnSuccess) { + DoRollback(status, std::move(cb)); + return; + } + + const bool lockedAtTarget = + IsSourceLocked(state.status) && + NominalRateHz(state.status) == restartSession_.desiredClock.sampleRateHz && + state.sampleRate == restartSession_.desiredClock.sampleRateHz; + + if (lockedAtTarget) { + if (attempt > 0) { + ASFW_LOG(DICE, + "PrepareDuplex48k: streaming clock stable-locked at %u Hz after %u ms; enabling streams", + state.sampleRate, attempt * kPollIntervalMs); + } + DoDiscoverStreams(channels, 0, std::move(cb)); + return; + } + + if (attempt * kPollIntervalMs >= kAsyncTimeoutMs) { + ASFW_LOG(DICE, + "PrepareDuplex48k: clock not stable-locked within %u ms (status=0x%08x rate=%u target=%u); aborting bring-up", + kAsyncTimeoutMs, state.status, state.sampleRate, + restartSession_.desiredClock.sampleRateHz); + DoRollback(kIOReturnTimeout, std::move(cb)); + return; + } + + ScheduleRetry(kPollIntervalMs, [this, channels, attempt, cb = std::move(cb)]() mutable { + DoAwaitStreamingClockLock(channels, attempt + 1, std::move(cb)); + }); + }); +} + void DICEDuplexBringupController::DoDiscoverStreams( AudioDuplexChannels channels, uint32_t step, diff --git a/ASFWDriver/Audio/Protocols/DICE/Core/DICEDuplexBringupController.hpp b/ASFWDriver/Audio/Protocols/DICE/Core/DICEDuplexBringupController.hpp index 3e769dc2..ad3c65e4 100644 --- a/ASFWDriver/Audio/Protocols/DICE/Core/DICEDuplexBringupController.hpp +++ b/ASFWDriver/Audio/Protocols/DICE/Core/DICEDuplexBringupController.hpp @@ -88,6 +88,10 @@ class DICEDuplexBringupController { void DoWaitClockAccepted(AudioDuplexChannels channels, uint32_t attempt, VoidCallback cb); void DoConfirmClockAccepted(AudioDuplexChannels channels, uint32_t observedNotify, VoidCallback cb); void DoReadGlobalAfterClockAccepted(AudioDuplexChannels channels, uint32_t observedNotify, IOReturn failureStatus, VoidCallback cb); + // Gate between clock-confirm and stream enable: poll until the PLL is stable-locked + // at the target rate, so streams are never enabled while the device is still + // relocking (which wedges it until a hard reset). + void DoAwaitStreamingClockLock(AudioDuplexChannels channels, uint32_t attempt, VoidCallback cb); void DoDiscoverStreams(AudioDuplexChannels channels, uint32_t step, VoidCallback cb); // Per-stream device programming. A multi-stream DICE device (e.g. Venice F32, // 2×16 channels) requires every advertised stream's ISOC register written From dfabe0d6cae0afd8511b68347570cc0ea1e214aa Mon Sep 17 00:00:00 2001 From: alicankaralar Date: Thu, 2 Jul 2026 20:43:09 +0200 Subject: [PATCH 16/23] checkpoint --- .../DICE/Core/DICEDuplexBringupController.cpp | 19 +++++++++++++++++-- .../DICE/Core/DICEDuplexBringupController.hpp | 5 +++++ .../Isoch/Receive/IsochReceiveContext.cpp | 19 +++++++++++++------ 3 files changed, 35 insertions(+), 8 deletions(-) diff --git a/ASFWDriver/Audio/Protocols/DICE/Core/DICEDuplexBringupController.cpp b/ASFWDriver/Audio/Protocols/DICE/Core/DICEDuplexBringupController.cpp index 8739e6dd..ef222770 100644 --- a/ASFWDriver/Audio/Protocols/DICE/Core/DICEDuplexBringupController.cpp +++ b/ASFWDriver/Audio/Protocols/DICE/Core/DICEDuplexBringupController.cpp @@ -440,11 +440,13 @@ void DICEDuplexBringupController::DoReadGlobalBeforeClaim( return; } + preClaimClockSelect_ = state.clockSelect; ASFW_LOG(DICE, - "PrepareDuplex48k: global pre-claim owner=0x%016llx enable=%u notify=0x%08x", + "PrepareDuplex48k: global pre-claim owner=0x%016llx enable=%u notify=0x%08x clockSelect=0x%08x", state.owner, state.enabled ? 1U : 0U, - state.notification); + state.notification, + state.clockSelect); DoReadOwnerBeforeClaim(channels, std::move(cb)); }); } @@ -536,6 +538,19 @@ void DICEDuplexBringupController::DoWriteClockSelect( return; } + // Skip a redundant CLOCK_SELECT write when the device is already at the target + // clock (e.g. an idle ApplyClockConfig already applied this rate). Rewriting it + // re-triggers the PLL relock during the bring-up, right as streams are being + // enabled, which wedges the device-side streams. The downstream stable-lock gate + // (DoAwaitStreamingClockLock) still waits for the lock to settle before enabling. + if (preClaimClockSelect_ == diceClock_.clockSelect) { + ASFW_LOG(DICE, + "PrepareDuplex48k: device already at target clockSelect=0x%08x; skipping redundant write", + diceClock_.clockSelect); + DoActiveClockCheck(channels, NotificationMailbox::Consume(), std::move(cb)); + return; + } + NotificationMailbox::Reset(); (void)io_.WriteQuadBE(MakeDICEAddress(sections_.global.offset + GlobalOffset::kClockSelect), diceClock_.clockSelect, diff --git a/ASFWDriver/Audio/Protocols/DICE/Core/DICEDuplexBringupController.hpp b/ASFWDriver/Audio/Protocols/DICE/Core/DICEDuplexBringupController.hpp index ad3c65e4..49acbc2d 100644 --- a/ASFWDriver/Audio/Protocols/DICE/Core/DICEDuplexBringupController.hpp +++ b/ASFWDriver/Audio/Protocols/DICE/Core/DICEDuplexBringupController.hpp @@ -143,6 +143,11 @@ class DICEDuplexBringupController { uint32_t confirmExtStatus_{0}; IOReturn stopSequenceError_{kIOReturnSuccess}; bool refreshRuntimeCapsOnPrepare_{true}; + // CLOCK_SELECT read at the start of the current bring-up (DoReadGlobalBeforeClaim). + // Lets DoWriteClockSelect skip a redundant write when the device is already at the + // target clock, so the PLL relock happens once (during the idle ApplyClockConfig) + // instead of again mid-bring-up where it disrupts the streams being enabled. + uint32_t preClaimClockSelect_{0}; const std::atomic* teardownCancel_{nullptr}; }; diff --git a/ASFWDriver/Isoch/Receive/IsochReceiveContext.cpp b/ASFWDriver/Isoch/Receive/IsochReceiveContext.cpp index 3d7240ce..7fbd9340 100644 --- a/ASFWDriver/Isoch/Receive/IsochReceiveContext.cpp +++ b/ASFWDriver/Isoch/Receive/IsochReceiveContext.cpp @@ -194,17 +194,24 @@ uint32_t IsochReceiveContext::Poll() { directInputView_.streamMode = ASFW::Audio::Runtime::AudioStreamMode::kUnknown; directInputView_.hostToDeviceWireFormat = ASFW::Audio::Runtime::AudioWireFormat::kAM824; - // Master-only: reset the shared clock/replay timeline once - // on (re)bind. A secondary slice must never touch the shared - // control block's cadence/replay — the master owns it. - if (!isSecondary_ && - !replayResetForStart_ && - directInputView_.control) { + // Master-only: reset the shared clock/replay timeline on every + // (re)bind. A secondary slice must never touch the shared control + // block's cadence/replay — the master owns it. This block is + // generation-gated (runs once per NEW binding generation), so a + // rate change — which bumps the binding generation while the IR + // context keeps running (no Start(), so replayResetForStart_ stays + // set) — MUST reset here too. Gating on replayResetForStart_ skipped + // it, leaving the master with stale 48k replay/cadence at 44.1k: + // ch1-16 (master) went dead while the secondary (ch17-32) survived. + if (!isSecondary_ && directInputView_.control) { directInputView_.control->rxSytCadence.Reset(); directInputView_.control->rxSequenceReplay.Reset(); directInputView_.control->rxReplayEpochResets.fetch_add( 1, std::memory_order_relaxed); replayResetForStart_ = true; + ASFW_LOG(Isoch, + "IR: master replay/cadence reset on rebind (gen %llu rate=%u)", + snapshot.generation, snapshot.sampleRateHz); } // Data plane (RX -> input buffer) arms for every stream. The From 15f5ad4e035f5c446e17abe5ec68b097118f6180 Mon Sep 17 00:00:00 2001 From: alicankaralar Date: Thu, 2 Jul 2026 23:36:57 +0200 Subject: [PATCH 17/23] fix(dice): clear every stream's ISOC register on stop (not just stream 0) DoStopDisableTx/Rx read the per-stream SIZE stride, discarded it, and wrote kDisabledIsoChannel only to stream[0]. On a multi-stream device (Venice F32 = 2x2 streams) the secondary TX/RX ISOC registers kept their stale channels across every stop. The next bring-up's FirstActiveIsoChannel then seeded stream[0] from the STALE SECONDARY channel, shifting the whole channel map (observed rxIso=0 txIso=1 -> rxIso=3 txIso=2), and DoProgramRx transiently put two device RX streams on the same channel - undefined firmware state that wedged the master stream (ch1-16 dead, ch17-32 alive) until a device power cycle reset the registers. Walk all streams with the SIZE-derived stride (same address math as DoProgramRx/DoProgramTx), clearing ISOC + speed/seq-start per stream; a failed stride read falls back to the legacy stream[0]-only clear. Cross-validated with FFADO dice_avdevice.cpp startstopStreamByIndex:1415-1446 (stop writes 0xFFFFFFFF to EVERY stream's ISO_CHANNEL; a non-cleared register at start is treated as crash recovery). Also: reset the master's shared replay/cadence timeline on every direct-audio rebind (not once per Start) so a rate-change rebind re-establishes cleanly, and log the reset + skip the redundant CLOCK_SELECT write when the device is already at the target clock (single PLL relock per rate change, while idle). HW-verified: channel map now identical across every bring-up, and a rate change no longer permanently wedges the device (full 32ch recovery at both rates after app restart; no power cycle needed). Co-Authored-By: Claude Opus 4.8 --- .../DICE/Core/DICEDuplexBringupController.cpp | 142 +++++++++++++----- .../DICE/Core/DICEDuplexBringupController.hpp | 8 + 2 files changed, 110 insertions(+), 40 deletions(-) diff --git a/ASFWDriver/Audio/Protocols/DICE/Core/DICEDuplexBringupController.cpp b/ASFWDriver/Audio/Protocols/DICE/Core/DICEDuplexBringupController.cpp index ef222770..35fb89ea 100644 --- a/ASFWDriver/Audio/Protocols/DICE/Core/DICEDuplexBringupController.cpp +++ b/ASFWDriver/Audio/Protocols/DICE/Core/DICEDuplexBringupController.cpp @@ -1536,35 +1536,67 @@ void DICEDuplexBringupController::DoStopDisableTx( return; } + // Clear EVERY TX stream's ISOC register, not just stream[0]'s (FFADO + // stopStreamByIndex writes 0xFFFFFFFF per stream). Read TX_SIZE for the + // per-stream register stride, then walk the streams; if the stride read + // fails, fall back to the legacy single-stream clear. (void)io_.ReadQuadBE(MakeDICEAddress(sections_.txStreamFormat.offset + TxOffset::kSize), - [this, releaseOwner, cb = std::move(cb)](Async::AsyncStatus readTransportStatus, uint32_t) mutable { + [this, releaseOwner, cb = std::move(cb)](Async::AsyncStatus readTransportStatus, uint32_t txSize) mutable { const IOReturn readStatus = MapTransportStatus(readTransportStatus); RecordFirstError(stopSequenceError_, readStatus); if (AbortStopIfTeardown("DisableTxReadComplete", cb)) { return; } - (void)io_.WriteQuadBE(MakeDICEAddress(sections_.txStreamFormat.offset + TxOffset::kIsochronous), - kDisabledIsoChannel, - [this, releaseOwner, cb = std::move(cb)](Async::AsyncStatus isoTransportStatus) mutable { - const IOReturn isoStatus = MapTransportStatus(isoTransportStatus); - RecordFirstError(stopSequenceError_, isoStatus); - if (AbortStopIfTeardown("DisableTxIsoComplete", cb)) { - return; - } - (void)io_.WriteQuadBE(MakeDICEAddress(sections_.txStreamFormat.offset + TxOffset::kSpeed), - kTxSpeedS400, - [this, releaseOwner, cb = std::move(cb)](Async::AsyncStatus speedTransportStatus) mutable { - const IOReturn speedStatus = MapTransportStatus(speedTransportStatus); - RecordFirstError(stopSequenceError_, speedStatus); - if (AbortStopIfTeardown("DisableTxSpeedComplete", cb)) { - return; - } - DoStopReleaseTx(releaseOwner, std::move(cb)); - }); - }); + const uint32_t stride = + (readStatus == kIOReturnSuccess) ? txSize * 4u : 0u; + DoStopDisableTxStream(0, stride, releaseOwner, std::move(cb)); }); } +void DICEDuplexBringupController::DoStopDisableTxStream( + uint32_t streamIndex, + uint32_t entrySizeBytes, + bool releaseOwner, + VoidCallback cb) { + uint32_t streamCount = restartSession_.channels.captureStreamCount; + if (streamCount == 0) { + streamCount = 1; + } else if (streamCount > kMaxAudioStreamsPerDirection) { + streamCount = kMaxAudioStreamsPerDirection; + } + // Without a usable stride only stream[0]'s registers are addressable. + if (entrySizeBytes == 0) { + streamCount = 1; + } + + if (streamIndex >= streamCount) { + DoStopReleaseTx(releaseOwner, std::move(cb)); + return; + } + + const uint32_t streamBase = + sections_.txStreamFormat.offset + streamIndex * entrySizeBytes; + (void)io_.WriteQuadBE(MakeDICEAddress(streamBase + TxOffset::kIsochronous), + kDisabledIsoChannel, + [this, streamIndex, entrySizeBytes, streamBase, releaseOwner, cb = std::move(cb)](Async::AsyncStatus isoTransportStatus) mutable { + const IOReturn isoStatus = MapTransportStatus(isoTransportStatus); + RecordFirstError(stopSequenceError_, isoStatus); + if (AbortStopIfTeardown("DisableTxIsoComplete", cb)) { + return; + } + (void)io_.WriteQuadBE(MakeDICEAddress(streamBase + TxOffset::kSpeed), + kTxSpeedS400, + [this, streamIndex, entrySizeBytes, releaseOwner, cb = std::move(cb)](Async::AsyncStatus speedTransportStatus) mutable { + const IOReturn speedStatus = MapTransportStatus(speedTransportStatus); + RecordFirstError(stopSequenceError_, speedStatus); + if (AbortStopIfTeardown("DisableTxSpeedComplete", cb)) { + return; + } + DoStopDisableTxStream(streamIndex + 1, entrySizeBytes, releaseOwner, std::move(cb)); + }); + }); +} + void DICEDuplexBringupController::DoStopReleaseTx( bool releaseOwner, VoidCallback cb) { @@ -1586,35 +1618,65 @@ void DICEDuplexBringupController::DoStopDisableRx( return; } + // Clear EVERY RX stream's ISOC register, not just stream[0]'s (see + // DoStopDisableTx). Stride from RX_SIZE; legacy single-stream clear if the + // read fails. (void)io_.ReadQuadBE(MakeDICEAddress(sections_.rxStreamFormat.offset + RxOffset::kSize), - [this, releaseOwner, cb = std::move(cb)](Async::AsyncStatus readTransportStatus, uint32_t) mutable { + [this, releaseOwner, cb = std::move(cb)](Async::AsyncStatus readTransportStatus, uint32_t rxSize) mutable { const IOReturn readStatus = MapTransportStatus(readTransportStatus); RecordFirstError(stopSequenceError_, readStatus); if (AbortStopIfTeardown("DisableRxReadComplete", cb)) { return; } - (void)io_.WriteQuadBE(MakeDICEAddress(sections_.rxStreamFormat.offset + RxOffset::kIsochronous), - kDisabledIsoChannel, - [this, releaseOwner, cb = std::move(cb)](Async::AsyncStatus isoTransportStatus) mutable { - const IOReturn isoStatus = MapTransportStatus(isoTransportStatus); - RecordFirstError(stopSequenceError_, isoStatus); - if (AbortStopIfTeardown("DisableRxIsoComplete", cb)) { - return; - } - (void)io_.WriteQuadBE(MakeDICEAddress(sections_.rxStreamFormat.offset + RxOffset::kSeqStart), - kRxSeqStartDefault, - [this, releaseOwner, cb = std::move(cb)](Async::AsyncStatus seqTransportStatus) mutable { - const IOReturn seqStatus = MapTransportStatus(seqTransportStatus); - RecordFirstError(stopSequenceError_, seqStatus); - if (AbortStopIfTeardown("DisableRxSeqComplete", cb)) { - return; - } - DoStopReleaseRx(releaseOwner, std::move(cb)); - }); - }); + const uint32_t stride = + (readStatus == kIOReturnSuccess) ? rxSize * 4u : 0u; + DoStopDisableRxStream(0, stride, releaseOwner, std::move(cb)); }); } +void DICEDuplexBringupController::DoStopDisableRxStream( + uint32_t streamIndex, + uint32_t entrySizeBytes, + bool releaseOwner, + VoidCallback cb) { + uint32_t streamCount = restartSession_.channels.playbackStreamCount; + if (streamCount == 0) { + streamCount = 1; + } else if (streamCount > kMaxAudioStreamsPerDirection) { + streamCount = kMaxAudioStreamsPerDirection; + } + if (entrySizeBytes == 0) { + streamCount = 1; + } + + if (streamIndex >= streamCount) { + DoStopReleaseRx(releaseOwner, std::move(cb)); + return; + } + + const uint32_t streamBase = + sections_.rxStreamFormat.offset + streamIndex * entrySizeBytes; + (void)io_.WriteQuadBE(MakeDICEAddress(streamBase + RxOffset::kIsochronous), + kDisabledIsoChannel, + [this, streamIndex, entrySizeBytes, streamBase, releaseOwner, cb = std::move(cb)](Async::AsyncStatus isoTransportStatus) mutable { + const IOReturn isoStatus = MapTransportStatus(isoTransportStatus); + RecordFirstError(stopSequenceError_, isoStatus); + if (AbortStopIfTeardown("DisableRxIsoComplete", cb)) { + return; + } + (void)io_.WriteQuadBE(MakeDICEAddress(streamBase + RxOffset::kSeqStart), + kRxSeqStartDefault, + [this, streamIndex, entrySizeBytes, releaseOwner, cb = std::move(cb)](Async::AsyncStatus seqTransportStatus) mutable { + const IOReturn seqStatus = MapTransportStatus(seqTransportStatus); + RecordFirstError(stopSequenceError_, seqStatus); + if (AbortStopIfTeardown("DisableRxSeqComplete", cb)) { + return; + } + DoStopDisableRxStream(streamIndex + 1, entrySizeBytes, releaseOwner, std::move(cb)); + }); + }); +} + void DICEDuplexBringupController::DoStopReleaseRx( bool releaseOwner, VoidCallback cb) { diff --git a/ASFWDriver/Audio/Protocols/DICE/Core/DICEDuplexBringupController.hpp b/ASFWDriver/Audio/Protocols/DICE/Core/DICEDuplexBringupController.hpp index 49acbc2d..c1439b5a 100644 --- a/ASFWDriver/Audio/Protocols/DICE/Core/DICEDuplexBringupController.hpp +++ b/ASFWDriver/Audio/Protocols/DICE/Core/DICEDuplexBringupController.hpp @@ -93,6 +93,14 @@ class DICEDuplexBringupController { // relocking (which wedges it until a hard reset). void DoAwaitStreamingClockLock(AudioDuplexChannels channels, uint32_t attempt, VoidCallback cb); void DoDiscoverStreams(AudioDuplexChannels channels, uint32_t step, VoidCallback cb); + // Per-stream stop-side disables. The stop sequence must clear EVERY stream's + // ISOC register (FFADO stopStreamByIndex writes 0xFFFFFFFF per stream); a + // stream[0]-only clear leaves the secondary's stale channel in the device, + // which the next bring-up's FirstActiveIsoChannel then adopts as stream[0] + // (channel-map drift) — and a start over a stale duplicate channel wedges the + // device until a power cycle. + void DoStopDisableTxStream(uint32_t streamIndex, uint32_t entrySizeBytes, bool releaseOwner, VoidCallback cb); + void DoStopDisableRxStream(uint32_t streamIndex, uint32_t entrySizeBytes, bool releaseOwner, VoidCallback cb); // Per-stream device programming. A multi-stream DICE device (e.g. Venice F32, // 2×16 channels) requires every advertised stream's ISOC register written // before the single GLOBAL_ENABLE, so these recurse over the stream index. From 85c3bc30333eb99bdf6a1417d25486ea2e464f27 Mon Sep 17 00:00:00 2001 From: alicankaralar Date: Tue, 7 Jul 2026 00:54:59 +0200 Subject: [PATCH 18/23] test(dice): adjust prepare reference window for HW-validated bring-up deviations The reference trace always rewrites CLOCK_SELECT during prepare; ASFW deviates in two HW-validated ways (skip the redundant CLOCK_SELECT write when already at target; one extra global-state read for the stable-lock gate). Build the adjusted expectation from the generated fixture in the test. Co-Authored-By: Claude Fable 5 --- .../DICEDuplexBringupControllerTests.cpp | 25 ++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/tests/devices/DICEDuplexBringupControllerTests.cpp b/tests/devices/DICEDuplexBringupControllerTests.cpp index 5c8963e9..a8a5dc91 100644 --- a/tests/devices/DICEDuplexBringupControllerTests.cpp +++ b/tests/devices/DICEDuplexBringupControllerTests.cpp @@ -988,8 +988,27 @@ TEST(DICEDuplexBringupControllerTests, ProtocolRegisterIOCompareSwap64UsesLockAn TEST(DICEDuplexBringupControllerTests, PrepareSequenceMatchesReferenceWindow) { DuplexRig rig; NotificationMailbox::Reset(); - rig.bus.SetScript(ReferencePhase0ParityFixture::kPrepareExpectedRequests, - ReferencePhase0ParityFixture::kPrepareResponseSteps); + + // The reference trace always rewrites CLOCK_SELECT during prepare. ASFW + // intentionally deviates in two HW-validated ways (see + // DICEDuplexBringupController): + // 1. The CLOCK_SELECT write is skipped when the pre-claim global read + // already reports the target clock (0x020C here) — rewriting it + // re-triggers a PLL relock mid-bring-up and fights an idle rate change. + // 2. DoAwaitStreamingClockLock adds one extra global-state read between + // clock-confirm and stream discovery so streams are never enabled on a + // still-relocking clock. + // Net: the Write op is replaced by a second 380-byte global read, and that + // read consumes one extra scripted response reporting locked-at-target. + const auto& refRequests = ReferencePhase0ParityFixture::kPrepareExpectedRequests; + const auto& refResponses = ReferencePhase0ParityFixture::kPrepareResponseSteps; + std::vector requests(refRequests.begin(), refRequests.end()); + ASSERT_EQ(requests[6].kind, OpKind::Write); // CLOCK_SELECT in the reference + requests[6] = requests[7]; // becomes the await-lock global read + std::vector responses(refResponses.begin(), refResponses.end()); + responses.insert(responses.begin() + 7, responses[6]); // second locked global read + + rig.bus.SetScript(requests, responses); std::optional startStatus; const AudioDuplexChannels channels{ @@ -1005,7 +1024,7 @@ TEST(DICEDuplexBringupControllerTests, PrepareSequenceMatchesReferenceWindow) { EXPECT_EQ(rig.bus.Owner(), 0xFFC0000100000000ULL); EXPECT_EQ(rig.bus.BandwidthAvailable(), 4019U); EXPECT_EQ(rig.bus.ChannelsAvailable31_0(), 0x3FFFFFFFU); - ExpectRequests(rig.bus.Operations(), ReferencePhase0ParityFixture::kPrepareExpectedRequests); + ExpectRequests(rig.bus.Operations(), requests); EXPECT_TRUE(rig.bus.ScriptConsumed()); for (const auto& op : rig.bus.Operations()) { From 2060a19961aacde3c536466d072813ee8fc2a599 Mon Sep 17 00:00:00 2001 From: alicankaralar Date: Fri, 10 Jul 2026 09:19:48 +0200 Subject: [PATCH 19/23] fix(audio): rate-correct ZTS projection, SYT seed, and ZTS-period assert Close items 2, 3, and 6 of the 44.1 production-wiring checklist printed by tools/amdtp_blocking_cadence_sim.py: - ASFWAudioDriverZts: project ticks->frames for the one-shot TX cursor alignment as deltaTicks * liveRate / 24576000 (u64) instead of dividing by the 48k-only 512 ticks/sample (which overshot ~8.8% / ~125 frames at 44.1k). Rate and frames-per-packet come from the TX engine's stream config via a new DiceTxStreamEngine::StreamConfig() accessor; 48k is bit-identical to the old math. - RxSytCadence: the first SYT of a chain now only seeds previousSyt_ instead of recording a synthetic 48k-nominal (4096-tick) delta into the ring; the seed constant is removed. Still returns accepted, so it cannot fake a replay discontinuity. - AudioHalBufferProfiles: require zeroTimestampPeriodFrames % 32 == 0 (32 = 4x-family frames per data packet) so every ZTS boundary stays packet-first-frame observable at all rates (sim's 1544-period counterexample thins anchors ~4x). All existing profiles already comply. Co-Authored-By: Claude Fable 5 --- .../Audio/DriverKit/ASFWAudioDriverZts.cpp | 19 +++++++++++-------- .../Engine/Direct/Tx/DiceTxStreamEngine.cpp | 4 ++++ .../Engine/Direct/Tx/DiceTxStreamEngine.hpp | 2 ++ ASFWDriver/Audio/Wire/AMDTP/RxSytCadence.hpp | 14 +++++++++----- .../Shared/Isoch/AudioHalBufferProfiles.hpp | 13 ++++++++++++- 5 files changed, 38 insertions(+), 14 deletions(-) diff --git a/ASFWDriver/Audio/DriverKit/ASFWAudioDriverZts.cpp b/ASFWDriver/Audio/DriverKit/ASFWAudioDriverZts.cpp index fdcb2ebf..5bba9885 100644 --- a/ASFWDriver/Audio/DriverKit/ASFWAudioDriverZts.cpp +++ b/ASFWDriver/Audio/DriverKit/ASFWAudioDriverZts.cpp @@ -343,16 +343,19 @@ uint32_t PrepareTransmitSlots(ASFWAudioDriver_IVars& ivars, outputPresentationTicks, sourcePresentationTicks); if (presentationDeltaTicks >= 0) { - constexpr uint32_t kFramesPerPacket = - ASFW::IsochTransport:: - AudioTimingGeometry:: - kFramesPerDataPacket; + // ticks -> frames at the live rate. 44.1k has no integer + // ticks/sample (24576000/44100 ~= 557.28), so divide the + // tick*rate product instead of dividing by a per-sample + // constant (the old /512 overshot ~8.8% at 44.1k). + const auto& txConfig = + ivars.runtime.txStreamEngine.StreamConfig(); + const uint32_t kFramesPerPacket = + txConfig.framesPerDataPacket; const uint64_t projectedFrame = replay.firstAudioFrame + - static_cast( - presentationDeltaTicks / - ASFW::Timing:: - kTicksPerSample48k); + (static_cast(presentationDeltaTicks) * + txConfig.sampleRate) / + ASFW::Timing::kTicksPerSecond; const uint64_t alignedFrame = (projectedFrame / kFramesPerPacket) * kFramesPerPacket; diff --git a/ASFWDriver/Audio/Engine/Direct/Tx/DiceTxStreamEngine.cpp b/ASFWDriver/Audio/Engine/Direct/Tx/DiceTxStreamEngine.cpp index 0281b3ec..ab73db6f 100644 --- a/ASFWDriver/Audio/Engine/Direct/Tx/DiceTxStreamEngine.cpp +++ b/ASFWDriver/Audio/Engine/Direct/Tx/DiceTxStreamEngine.cpp @@ -118,6 +118,10 @@ const AMDTP::AmdtpPacketTimeline& DiceTxStreamEngine::Timeline() const noexcept return timeline_; } +const AMDTP::AmdtpStreamConfig& DiceTxStreamEngine::StreamConfig() const noexcept { + return packetizer_.StreamConfig(); +} + const DiceTxEngineCounters& DiceTxStreamEngine::Counters() const noexcept { return counters_; } diff --git a/ASFWDriver/Audio/Engine/Direct/Tx/DiceTxStreamEngine.hpp b/ASFWDriver/Audio/Engine/Direct/Tx/DiceTxStreamEngine.hpp index a76cc0c9..a0b4d8f4 100644 --- a/ASFWDriver/Audio/Engine/Direct/Tx/DiceTxStreamEngine.hpp +++ b/ASFWDriver/Audio/Engine/Direct/Tx/DiceTxStreamEngine.hpp @@ -58,6 +58,8 @@ class DiceTxStreamEngine final { [[nodiscard]] AMDTP::AmdtpPacketTimeline& Timeline() noexcept; [[nodiscard]] const AMDTP::AmdtpPacketTimeline& Timeline() const noexcept; + [[nodiscard]] const AMDTP::AmdtpStreamConfig& StreamConfig() const noexcept; + [[nodiscard]] const DiceTxEngineCounters& Counters() const noexcept; [[nodiscard]] const AMDTP::AmdtpPayloadWriterCounters& diff --git a/ASFWDriver/Audio/Wire/AMDTP/RxSytCadence.hpp b/ASFWDriver/Audio/Wire/AMDTP/RxSytCadence.hpp index 0d87adf3..d4acdba5 100644 --- a/ASFWDriver/Audio/Wire/AMDTP/RxSytCadence.hpp +++ b/ASFWDriver/Audio/Wire/AMDTP/RxSytCadence.hpp @@ -16,8 +16,6 @@ namespace ASFW::Driver { class RxSytCadence final { public: static constexpr uint16_t kNoInfo = 0xFFFF; - static constexpr uint16_t kNominalStepTicks48k = - static_cast(ASFW::Timing::kSytPacketStepTicks48k); static constexpr uint32_t kEntryCount = 512; static constexpr uint32_t kReadDelay = kEntryCount / 2; static constexpr uint32_t kWarmupUpdates = kEntryCount + 1; @@ -57,10 +55,16 @@ class RxSytCadence final { BeginWrite(); const uint16_t previous = previousSyt_.load(std::memory_order_relaxed); - int64_t delta = kNominalStepTicks48k; - if (previous != kNoInfo) { - delta = ASFW::Timing::SYTDiffInOffsets(syt, previous); + if (previous == kNoInfo) { + // First SYT of a chain (start, or restart after an invalid delta) + // only seeds previousSyt_. Recording a synthetic nominal delta + // would bias the ring at any rate whose real step differs from + // the seed constant (48k = 4096 ticks vs 44.1k = 4458/4459). + previousSyt_.store(syt, std::memory_order_relaxed); + EndWrite(); + return true; } + const int64_t delta = ASFW::Timing::SYTDiffInOffsets(syt, previous); if (delta <= 0 || delta > UINT16_MAX) { previousSyt_.store(kNoInfo, std::memory_order_relaxed); EndWrite(); diff --git a/ASFWDriver/Shared/Isoch/AudioHalBufferProfiles.hpp b/ASFWDriver/Shared/Isoch/AudioHalBufferProfiles.hpp index 82c477b1..a96215d6 100644 --- a/ASFWDriver/Shared/Isoch/AudioHalBufferProfiles.hpp +++ b/ASFWDriver/Shared/Isoch/AudioHalBufferProfiles.hpp @@ -41,6 +41,14 @@ inline constexpr AudioHalBufferProfile kAudioHalBufferProfileDiceWorking1536{ 1536, }; +// Largest blocking-mode frames-per-data-packet across the IEC 61883-6 rate +// ladder: the SYT interval is 8 @1x, 16 @2x, 32 @4x. ZTS anchors are only +// published when a boundary lands on a packet-first frame, so the period must +// be a multiple of every tier's frames-per-packet (all divide 32). A period +// that is not silently thins anchors ~4x at 4x rates +// (tools/amdtp_blocking_cadence_sim.py, 1544-period counterexample). +inline constexpr uint32_t kMaxBlockingFramesPerDataPacket = 32; + [[nodiscard]] constexpr bool IsValidAudioHalBufferProfile( const AudioHalBufferProfile& profile) noexcept { return profile.name != nullptr && @@ -48,7 +56,10 @@ inline constexpr AudioHalBufferProfile kAudioHalBufferProfileDiceWorking1536{ profile.clientIoBudgetFrames != 0 && profile.zeroTimestampPeriodFrames != 0 && profile.frameRingFrames % profile.clientIoBudgetFrames == 0 && - profile.frameRingFrames % profile.zeroTimestampPeriodFrames == 0; + profile.frameRingFrames % profile.zeroTimestampPeriodFrames == 0 && + profile.zeroTimestampPeriodFrames % + kMaxBlockingFramesPerDataPacket == + 0; } static_assert(IsValidAudioHalBufferProfile(kAudioHalBufferProfileAligned512)); From be5a96a9a68d10ead63e2b2a46dff0ae9458eb40 Mon Sep 17 00:00:00 2001 From: alicankaralar Date: Fri, 10 Jul 2026 09:20:03 +0200 Subject: [PATCH 20/23] feat(isoch): size TX packet budgets for the worst-case 44.1k cadence Close item 4 of the sim's production-wiring checklist. TX packet budgets derived from frame requirements were sized at 48k's 6 frames/packet average; at 44.1k (441 frames / 80 packets = 5.5125 avg) the exposure window covered ~1058 frames against the 1088-frame design target. Introduce kMinAvgCadencePackets/Frames (80/441, the lowest average across supported rates) and derive kTxExposureLeadPackets from it: ceil(576 / 5.5125) = 105, rounded up to a whole 6-packet interrupt group (108) so every derived budget keeps the group- and cadence-block-aligned ring-wrap asserts. Derived values: exposure window 192 -> 216, preparation lead 336 -> 360, shared slots 384 -> 408 (<= 512 timeline slots), max covered deltaConsumed 288 -> 312. The frame-coverage static_assert now checks the invariant at the worst-case ratio. The shared-ring-wrap tests hardcoded that the wrap lands the hardware pointer back at descriptor 0, true only while numSlots was a multiple of 48. IsochTxDmaRing::Refill rebinds payload IOVAs from fillAbsIdx % numSlots each pass, so no such divisibility is required; the tests now derive the post-wrap hardware index. Co-Authored-By: Claude Fable 5 --- .../Shared/Isoch/AudioTimingGeometry.hpp | 38 +++++++++++++------ tests/audio/AmdtpRateGeometryTests.cpp | 10 +++-- tests/audio/IsochTxDmaRingTests.cpp | 19 +++++++--- tests/audio/RxDrivenTimingTests.cpp | 7 ++-- tests/audio/TxRefillCoverageTests.cpp | 4 +- 5 files changed, 53 insertions(+), 25 deletions(-) diff --git a/ASFWDriver/Shared/Isoch/AudioTimingGeometry.hpp b/ASFWDriver/Shared/Isoch/AudioTimingGeometry.hpp index 7f6d4cda..46ad3086 100644 --- a/ASFWDriver/Shared/Isoch/AudioTimingGeometry.hpp +++ b/ASFWDriver/Shared/Isoch/AudioTimingGeometry.hpp @@ -13,7 +13,8 @@ namespace ASFW::IsochTransport { // ...Blocks OHCI descriptor blocks (4 per TX packet, Z=4) // ...Ticks 24.576 MHz FireWire ticks (3072/cycle) // Never compare a *Packets value to a *Frames value without an explicit -// conversion (the cadence is the only bridge: 6 frames/packet average). +// conversion (the cadence is the only bridge: 6 frames/packet average at 48k, +// 5.5125 at 44.1k -- budgets use the worst case, kMinAvgCadence*). // // CURSOR MODEL (TX): three frame-domain cursors race -- // T hardware transmit pos, W CoreAudio write frontier, E exposure frontier. @@ -34,6 +35,14 @@ struct AudioTimingGeometry final { static constexpr uint32_t kCadenceBlockPackets = 4; static constexpr uint32_t kCadenceBlockFrames = 24; + // Worst-case average cadence across supported 1x rates: the 44.1k family + // carries 441 frames per 80 packets (5.5125 frames/packet average), fewer + // than 48k's 6. Packet budgets that must cover a frame requirement are + // sized with this ratio so they hold at every supported rate + // (tools/amdtp_blocking_cadence_sim.py, production wiring item 4). + static constexpr uint32_t kMinAvgCadencePackets = 80; + static constexpr uint32_t kMinAvgCadenceFrames = 441; + // DMA completion cadence is deliberately independent from the HAL ZTS // grid. Six FireWire cycles give 0.75 ms refill latency. Depending on the // D/D/D/N starting phase, one interrupt carries 32 or 40 decoded frames. @@ -82,12 +91,18 @@ struct AudioTimingGeometry final { // once measured. Frames. static constexpr uint32_t kTxExposureLeadFrames = kHalIoPeriodFrames + kSchedulingJitterFrames; // 512 + 64 = 576 - // Packet lead deep enough to expose that many frames at the six-frame - // average cadence. ceil(576 / 6) = 96 packets. + // Packet lead deep enough to expose that many frames at the worst-case + // (44.1k) average cadence: ceil(576 / 5.5125) = 105 packets, rounded up + // to a whole interrupt group (108) so every budget derived from it keeps + // the group- and cadence-block-aligned ring-wrap asserts below. + static constexpr uint32_t kTxExposureLeadPacketsRaw = + (kTxExposureLeadFrames * kMinAvgCadencePackets + + kMinAvgCadenceFrames - 1) / + kMinAvgCadenceFrames; static constexpr uint32_t kTxExposureLeadPackets = - (kTxExposureLeadFrames * kCadenceBlockPackets + - kCadenceBlockFrames - 1) / - kCadenceBlockFrames; + ((kTxExposureLeadPacketsRaw + kTxPacketsPerGroup - 1) / + kTxPacketsPerGroup) * + kTxPacketsPerGroup; // Packet-domain TX ownership: 48 descriptors on hardware, plus two // independent producer budgets: @@ -110,8 +125,9 @@ struct AudioTimingGeometry final { kTxHardwareRingPackets + kTxPreparationSlackPackets; // Covers a full client write window plus the output exposure cushion when // the producer target is expressed as WriteEnd + kTxExposureLeadFrames. - // 2 * 96 packets = 1152 nominal frames, enough for 512 + 576 = 1088 frames - // while preserving cadence/group-friendly packet counts. + // 2 * 108 packets = 1190 worst-case (44.1k) frames, enough for + // 512 + 576 = 1088 frames while preserving cadence/group-friendly packet + // counts at every supported rate. static constexpr uint32_t kTxFrameExposureWindowPackets = 2 * kTxExposureLeadPackets; static constexpr uint32_t kTxPreparationLeadPackets = @@ -204,12 +220,12 @@ static_assert(AudioTimingGeometry::kTxExposureLeadPackets <= AudioTimingGeometry::kTxSharedSlotPackets, "TX packet lead must be able to hold the required exposure frames"); static_assert(AudioTimingGeometry::kTxFrameExposureWindowPackets * - AudioTimingGeometry::kCadenceBlockFrames >= + AudioTimingGeometry::kMinAvgCadenceFrames >= (AudioTimingGeometry::kHalIoPeriodFrames + AudioTimingGeometry::kTxExposureLeadFrames) * - AudioTimingGeometry::kCadenceBlockPackets, + AudioTimingGeometry::kMinAvgCadencePackets, "TX frame-exposure packet window must cover WriteEnd plus the " - "exposure cushion"); + "exposure cushion at the worst-case (44.1k) cadence"); static_assert(AudioTimingGeometry::kTxSharedSlotPackets <= AudioTimingGeometry::kTimelineSlots, "shared packet ring must fit inside the timeline slot array"); diff --git a/tests/audio/AmdtpRateGeometryTests.cpp b/tests/audio/AmdtpRateGeometryTests.cpp index 1c5adbf9..3661e32d 100644 --- a/tests/audio/AmdtpRateGeometryTests.cpp +++ b/tests/audio/AmdtpRateGeometryTests.cpp @@ -51,12 +51,16 @@ TEST(AudioTimingGeometryTests, SaffireGeometryIsUnified) { EXPECT_EQ(Geometry::kNominalFramesPerTimingGroup, 36U); EXPECT_EQ(Geometry::kInputSafetyFloorFrames, 104U); EXPECT_EQ(Geometry::kRxDescriptorPackets, 504U); - EXPECT_EQ(Geometry::kTxSharedSlotPackets, 384U); + // TX budgets are sized for the worst-case (44.1k) average cadence of + // 441 frames / 80 packets, exposure lead rounded to a whole interrupt + // group: ceil(576 / 5.5125) = 105 -> 108 packets. + EXPECT_EQ(Geometry::kTxSharedSlotPackets, 408U); EXPECT_EQ(Geometry::kTxHardwareRingPackets, 48U); EXPECT_EQ(Geometry::kTxPreparationSlackPackets, 96U); EXPECT_EQ(Geometry::kTxCoverageLeadPackets, 144U); - EXPECT_EQ(Geometry::kTxFrameExposureWindowPackets, 192U); - EXPECT_EQ(Geometry::kTxPreparationLeadPackets, 336U); + EXPECT_EQ(Geometry::kTxExposureLeadPackets, 108U); + EXPECT_EQ(Geometry::kTxFrameExposureWindowPackets, 216U); + EXPECT_EQ(Geometry::kTxPreparationLeadPackets, 360U); // DMA completion cadence and the ZTS grid are intentionally independent. EXPECT_NE(Geometry::kHalZeroTimestampPeriodFrames, diff --git a/tests/audio/IsochTxDmaRingTests.cpp b/tests/audio/IsochTxDmaRingTests.cpp index 822d89dd..7bd40e78 100644 --- a/tests/audio/IsochTxDmaRingTests.cpp +++ b/tests/audio/IsochTxDmaRingTests.cpp @@ -695,10 +695,14 @@ TEST_F(IsochTxDmaRingTest, RefillRejectsStaleGenerationAtFirstSharedRingWrap) { std::memcpy(sharedPayload_.data(), payloadBefore.data(), payloadBefore.size()); + // Hardware descriptor index of the first group past the shared-ring wrap. + // The shared ring need not be a whole number of 48-packet hardware laps, + // so derive it instead of assuming the wrap lands on descriptor 0. const auto firstSecondLapRefill = - refillTo( - ASFW::IsochTransport::AudioTimingGeometry:: - kTxPacketsPerGroup); + refillTo(((kGroupsToSharedRingWrap + 1) * + ASFW::IsochTransport::AudioTimingGeometry:: + kTxPacketsPerGroup) % + Layout::kNumPackets); EXPECT_FALSE(firstSecondLapRefill.ok); EXPECT_EQ(firstSecondLapRefill.packetsFilled, 0U); EXPECT_EQ(metadataRing[0].commitGen.load(std::memory_order_acquire), @@ -791,10 +795,13 @@ TEST_F(IsochTxDmaRingTest, RefillAcceptsGenerationTwoAtFirstSharedRingWrap) { std::memory_order_release); } + // See RefillRejectsStaleGenerationAtFirstSharedRingWrap: the wrap does not + // necessarily land on hardware descriptor 0, so derive the index. const auto firstSecondLapRefill = - refillTo( - ASFW::IsochTransport::AudioTimingGeometry:: - kTxPacketsPerGroup); + refillTo(((kGroupsToSharedRingWrap + 1) * + ASFW::IsochTransport::AudioTimingGeometry:: + kTxPacketsPerGroup) % + Layout::kNumPackets); EXPECT_TRUE(firstSecondLapRefill.ok); EXPECT_EQ( firstSecondLapRefill.packetsFilled, diff --git a/tests/audio/RxDrivenTimingTests.cpp b/tests/audio/RxDrivenTimingTests.cpp index 55d1c670..cdd1cea1 100644 --- a/tests/audio/RxDrivenTimingTests.cpp +++ b/tests/audio/RxDrivenTimingTests.cpp @@ -204,9 +204,10 @@ TEST(RxDrivenTimingTests, GeometryUsesSixCycleInterruptsAndCurrentTxDepths) { EXPECT_EQ(AudioTimingGeometry::kTxHardwareRingPackets, 48U); EXPECT_EQ(AudioTimingGeometry::kTxPreparationSlackPackets, 96U); EXPECT_EQ(AudioTimingGeometry::kTxCoverageLeadPackets, 144U); - EXPECT_EQ(AudioTimingGeometry::kTxFrameExposureWindowPackets, 192U); - EXPECT_EQ(AudioTimingGeometry::kTxPreparationLeadPackets, 336U); - EXPECT_EQ(AudioTimingGeometry::kTxSharedSlotPackets, 384U); + // Worst-case (44.1k) cadence sizing: exposure lead 108 packets. + EXPECT_EQ(AudioTimingGeometry::kTxFrameExposureWindowPackets, 216U); + EXPECT_EQ(AudioTimingGeometry::kTxPreparationLeadPackets, 360U); + EXPECT_EQ(AudioTimingGeometry::kTxSharedSlotPackets, 408U); } TEST(RxDrivenTimingTests, InputSafetyIsVisibilityMarginNotClientWindow) { diff --git a/tests/audio/TxRefillCoverageTests.cpp b/tests/audio/TxRefillCoverageTests.cpp index e19f62b0..63767c07 100644 --- a/tests/audio/TxRefillCoverageTests.cpp +++ b/tests/audio/TxRefillCoverageTests.cpp @@ -34,10 +34,10 @@ namespace { using ASFW::IsochTransport::AudioTimingGeometry; using ASFW::IsochTransport::ExpectedCommitGen; -constexpr uint32_t kNumSlots = AudioTimingGeometry::kTxSharedSlotPackets; // 384 +constexpr uint32_t kNumSlots = AudioTimingGeometry::kTxSharedSlotPackets; // 408 constexpr uint32_t kHwRing = AudioTimingGeometry::kTxHardwareRingPackets; // 48 constexpr uint32_t kCoverageLead = AudioTimingGeometry::kTxCoverageLeadPackets; // 144 -constexpr uint32_t kLead = AudioTimingGeometry::kTxPreparationLeadPackets; // 336 +constexpr uint32_t kLead = AudioTimingGeometry::kTxPreparationLeadPackets; // 360 constexpr uint32_t kGroup = AudioTimingGeometry::kTxPacketsPerGroup; // 6 // The historical pre-fix lead (slack == 2*group) the hardware IT FATAL was From 446bbd12ab1651b0b3238ecc834e31dc0f74d39e Mon Sep 17 00:00:00 2001 From: alicankaralar Date: Fri, 10 Jul 2026 09:25:15 +0200 Subject: [PATCH 21/23] refactor(amdtp): unify blocking cadence on the rational deadline engine Resolve the double-path left by the rebase: BlockingCadence (production, FFADO-style frame accumulator) and RationalBlockingCadence (golden-tested deadline DDA) computed the same schedule with different arithmetic. BlockingCadence is now a thin IAmdtpCadence adapter over RationalBlockingCadence; the accumulator implementation is deleted. Wire behavior is unchanged by construction: the adapter seeds the engine one subtick-cycle short of one full data block (sytInterval * kTicksPerSecond - kTicksPerCycle), which makes the k-th block's deadline land in cycle c exactly when (c+1)*rate >= (k+1)*sytInterval*8000 -- the accumulate-then-test rule (FFADO cip.c) the accumulator implemented, i.e. the HW-verified startup phase. A new regression test pins the adapter against a test-local reference accumulator cycle-for-cycle at all seven rates. This also makes the wire-visible startup seed an explicit, documented policy (sim checklist item 7): the current seed preserves the verified phase; Apple's captured NuDCL seed (44100.md section 7) differs and adopting it stays capture-gated; in duplex, RX sequence replay supersedes the free-run cadence after RX locks. Cadence Configure is now [[nodiscard]] and the packetizer propagates a rate/interval mismatch as a Configure failure instead of silently running an inert cadence. Co-Authored-By: Claude Fable 5 --- ASFWDriver/Audio/Wire/AMDTP/AmdtpCadence.cpp | 49 ++++---- ASFWDriver/Audio/Wire/AMDTP/AmdtpCadence.hpp | 117 +++++++++--------- .../Audio/Wire/AMDTP/AmdtpTxPacketizer.cpp | 8 +- tests/audio/BlockingCadenceTests.cpp | 45 ++++++- 4 files changed, 131 insertions(+), 88 deletions(-) diff --git a/ASFWDriver/Audio/Wire/AMDTP/AmdtpCadence.cpp b/ASFWDriver/Audio/Wire/AMDTP/AmdtpCadence.cpp index 28514806..d6b82a97 100644 --- a/ASFWDriver/Audio/Wire/AMDTP/AmdtpCadence.cpp +++ b/ASFWDriver/Audio/Wire/AMDTP/AmdtpCadence.cpp @@ -4,54 +4,55 @@ namespace ASFW::Protocols::Audio::AMDTP { -// Blocking mode at 48 kHz (IEC 61883-6): 6 audio frames arrive per 125 µs bus -// cycle (48000 / 8000), a data packet always carries SYT_INTERVAL = 8 frames. -// Accumulate 6 frames per cycle, emit 8 whenever ≥ 8 are pending: +// Blocking mode (IEC 61883-6): at 48 kHz, 6 audio frames arrive per 125 µs bus +// cycle (48000 / 8000) and a data packet always carries SYT_INTERVAL = 8 +// frames, yielding N,D,D,D (6000 data packets per 8000 cycles); at 44.1 kHz +// the same rule yields 441 data packets per 640 cycles. // -// pending: 0 → no-data → 6 → data → 4 → data → 2 → data → 0 → ... -// -// yielding the N,D,D,D pattern: exactly 6000 data packets per 8000 cycles. -// phase_ holds the pending (not yet emitted) frame count {0, 6, 4, 2}. +// The adapter seeds the deadline engine one subtick-cycle short of one full +// block: the k-th block's deadline lands in cycle c exactly when +// (c+1)*rate >= (k+1)*sytInterval*8000, i.e. "emit as soon as one full block +// of frames is pending" -- the FFADO accumulate-then-test rule (cip.c) this +// class previously implemented directly. See the header for the seed policy. // // Note: this is cadence only. Alignment of the no-data slot relative to SYT -// is owned by the timing model (Milestone 2), not the cadence. +// is owned by the timing model, not the cadence. namespace { constexpr uint8_t kFramesPerCycle48k = 6; + +constexpr uint64_t AccumulatorEquivalentSeedSubticks( + uint8_t sytIntervalFrames) noexcept { + return Timing::kTicksPerSecond * sytIntervalFrames - + Timing::kTicksPerCycle; +} } // namespace -void BlockingCadence::Configure(uint32_t sampleRateHz, +bool BlockingCadence::Configure(uint32_t sampleRateHz, uint8_t sytIntervalFrames) noexcept { - framesPerCycleNum_ = sampleRateHz; // rate/8000 frames per cycle, scaled by 8000 - sytIntervalFrames_ = sytIntervalFrames; - sytThresholdNum_ = - static_cast(sytIntervalFrames) * kCyclesPerSecond; - Reset(); + return engine_.Configure(sampleRateHz, sytIntervalFrames, + AccumulatorEquivalentSeedSubticks( + sytIntervalFrames)); } void BlockingCadence::Reset() noexcept { - readyNum_ = 0; - totalCycles_ = 0; + engine_.Reset(); } bool BlockingCadence::CurrentCycleIsData() const noexcept { - // FFADO cip.c: data iff floor(ready_samples + samples_per_cycle) >= - // syt_interval, i.e. (readyNum_ + rate) >= syt_interval * 8000. - return (readyNum_ + framesPerCycleNum_) >= sytThresholdNum_; + return engine_.CurrentDecision().isData; } uint8_t BlockingCadence::CurrentCycleDataFrames() const noexcept { - return CurrentCycleIsData() ? sytIntervalFrames_ : 0; + return engine_.CurrentDecision().dataBlocks; } uint64_t BlockingCadence::TotalCycles() const noexcept { - return totalCycles_; + return engine_.TotalCycles(); } void BlockingCadence::AdvanceCycle() noexcept { - readyNum_ = readyNum_ + framesPerCycleNum_ - - (CurrentCycleIsData() ? sytThresholdNum_ : 0u); - ++totalCycles_; + engine_.AdvanceCycle(); } // Non-blocking mode at 48 kHz: the per-cycle frame count is integral (6), so diff --git a/ASFWDriver/Audio/Wire/AMDTP/AmdtpCadence.hpp b/ASFWDriver/Audio/Wire/AMDTP/AmdtpCadence.hpp index aa4d1d5a..45928b87 100644 --- a/ASFWDriver/Audio/Wire/AMDTP/AmdtpCadence.hpp +++ b/ASFWDriver/Audio/Wire/AMDTP/AmdtpCadence.hpp @@ -29,21 +29,70 @@ class IAmdtpCadence { virtual void AdvanceCycle() noexcept = 0; }; -// Blocking-mode AMDTP cadence for any 1x/2x/4x rate. Defaults to 48 kHz so -// existing default-constructed callers are byte-identical to the old -// Blocking48kCadence. Configure() switches the rate. +/// Exact rational blocking-mode cadence, kept in FireWire ticks x sample rate. +/// +/// The single blocking-cadence arithmetic engine: one integer deadline DDA +/// parameterized by (sampleRate, sytInterval), validated against the golden +/// vectors from tools/amdtp_blocking_cadence_sim.py. Production reaches it +/// through the BlockingCadence adapter below; `initialDeadlineSubticks` is +/// relative to the start of the first cycle and is a wire-visible seed policy +/// owned by the adapter (or a future capture-derived policy), not by this +/// arithmetic. +class RationalBlockingCadence final { +public: + /// Configures a schedule with at most one data packet per bus cycle. + /// Returns false for an invalid rate/interval or an event rate over 8 kHz. + [[nodiscard]] bool Configure(uint32_t sampleRateHz, + uint8_t sytInterval, + uint64_t initialDeadlineSubticks = 0) noexcept; + + void Reset() noexcept; + + [[nodiscard]] bool IsConfigured() const noexcept; + [[nodiscard]] RationalBlockingDecision CurrentDecision() const noexcept; + [[nodiscard]] uint64_t TotalCycles() const noexcept; + + void AdvanceCycle() noexcept; + +private: + uint32_t denominator_{0}; + uint8_t sytInterval_{0}; + uint64_t cycleSpanSubticks_{0}; + uint64_t stepSubticks_{0}; + uint64_t initialDeadlineSubticks_{0}; + uint64_t deadlineSubticks_{0}; + uint64_t totalCycles_{0}; +}; + +// Blocking-mode AMDTP cadence for any 1x/2x/4x rate: the IAmdtpCadence +// adapter over RationalBlockingCadence used by the production packetizer. +// Defaults to 48 kHz so default-constructed callers behave like the old +// Blocking48kCadence; Configure() switches the rate. // -// Cross-validated with FFADO iec61883_cip_fill_header (libffado-2.4.9 -// src/libstreaming/util/cip.c:130-160): the blocking cadence is a rational -// accumulator of rate/8000 frames per 125 us bus cycle that emits a -// syt_interval-frame data block whenever at least syt_interval frames are -// pending; otherwise a NO-DATA packet. The previous 48 kHz integer form -// (6 frames/cycle, emit 8) is exactly the rate==48000 special case. +// SEED POLICY (wire-visible): the engine is seeded one subtick-cycle short of +// one full data block, +// seed = sytInterval * kTicksPerSecond - kTicksPerCycle, +// which makes the deadline DDA emit a block exactly when a full block of +// frames is pending -- provably identical, cycle for cycle at every rate, to +// the FFADO-style rational accumulator this class previously implemented +// (cross-validated with libffado-2.4.9 src/libstreaming/util/cip.c:130-160: +// data iff ready_samples + samples_per_cycle >= syt_interval). That is the +// hardware-verified startup phase (48/44.1k duplex), so the adapter preserves +// it. Apple's captured AppleFWAudio seed differs (opens N,N,N,D -- +// documentation/44100.md section 7); adopting a different wire-visible seed +// is capture-gated (SAMPLE_RATE_EXPANSION.md section 8). In live duplex the +// free-running cadence is superseded by RX sequence replay after RX locks, +// so the seed governs only the pre-replay window. class BlockingCadence final : public IAmdtpCadence { public: + BlockingCadence() noexcept { (void)Configure(48000, 8); } + // sytIntervalFrames is the blocking frames-per-data-packet: 8 @1x - // (32/44.1/48 k), 16 @2x, 32 @4x (FFADO getSytInterval()). - void Configure(uint32_t sampleRateHz, uint8_t sytIntervalFrames) noexcept; + // (32/44.1/48 k), 16 @2x, 32 @4x (FFADO getSytInterval()). Returns false + // (and goes inert: every cycle no-data) for an invalid rate/interval + // combination, e.g. more than sytInterval frames per cycle. + [[nodiscard]] bool Configure(uint32_t sampleRateHz, + uint8_t sytIntervalFrames) noexcept; void Reset() noexcept override; @@ -54,18 +103,7 @@ class BlockingCadence final : public IAmdtpCadence { void AdvanceCycle() noexcept override; private: - // Rational accumulator over an 8000 cycles/s denominator: readyNum_ counts - // pending frames * 8000. Each cycle adds framesPerCycleNum_ (== sampleRateHz, - // i.e. rate/8000 frames scaled by 8000) and emits a block of - // sytIntervalFrames_ when readyNum_ + framesPerCycleNum_ reaches - // sytThresholdNum_ (== sytIntervalFrames_ * 8000). Exact, drift-free for - // fractional rates such as 44.1 kHz (5.5125 frames/cycle). - static constexpr uint32_t kCyclesPerSecond = 8000; - uint32_t framesPerCycleNum_{48000}; - uint32_t sytThresholdNum_{8u * kCyclesPerSecond}; - uint8_t sytIntervalFrames_{8}; - uint32_t readyNum_{0}; - uint64_t totalCycles_{0}; + RationalBlockingCadence engine_{}; }; // Transitional alias: existing callers default-construct this for 48 kHz. @@ -85,37 +123,4 @@ class NonBlocking48kCadence final : public IAmdtpCadence { uint64_t totalCycles_{0}; }; -/// Exact rational blocking-mode cadence, kept in FireWire ticks x sample rate. -/// -/// This is deliberately a standalone Phase 2 primitive. The production -/// packetizer remains 48 kHz-only until its rate-selection and presentation -/// timing paths are converted together. `initialDeadlineSubticks` is relative -/// to the start of the first cycle; wire-visible seed/lead policy belongs to -/// that later integration, not to this arithmetic engine. -class RationalBlockingCadence final { -public: - /// Configures a schedule with at most one data packet per bus cycle. - /// Returns false for an invalid rate/interval or an event rate over 8 kHz. - [[nodiscard]] bool Configure(uint32_t sampleRateHz, - uint8_t sytInterval, - uint64_t initialDeadlineSubticks = 0) noexcept; - - void Reset() noexcept; - - [[nodiscard]] bool IsConfigured() const noexcept; - [[nodiscard]] RationalBlockingDecision CurrentDecision() const noexcept; - [[nodiscard]] uint64_t TotalCycles() const noexcept; - - void AdvanceCycle() noexcept; - -private: - uint32_t denominator_{0}; - uint8_t sytInterval_{0}; - uint64_t cycleSpanSubticks_{0}; - uint64_t stepSubticks_{0}; - uint64_t initialDeadlineSubticks_{0}; - uint64_t deadlineSubticks_{0}; - uint64_t totalCycles_{0}; -}; - } // namespace ASFW::Protocols::Audio::AMDTP diff --git a/ASFWDriver/Audio/Wire/AMDTP/AmdtpTxPacketizer.cpp b/ASFWDriver/Audio/Wire/AMDTP/AmdtpTxPacketizer.cpp index e470fb77..b6dae793 100644 --- a/ASFWDriver/Audio/Wire/AMDTP/AmdtpTxPacketizer.cpp +++ b/ASFWDriver/Audio/Wire/AMDTP/AmdtpTxPacketizer.cpp @@ -89,9 +89,11 @@ bool AmdtpTxPacketizer::Configure(const AmdtpStreamConfig& streamConfig, cipBuilder_.Configure(cipConfig); if (config.streamMode == StreamMode::Blocking) { - blocking48kCadence_.Configure( - config.sampleRate, - static_cast(geometry->sytIntervalFrames)); + if (!blocking48kCadence_.Configure( + config.sampleRate, + static_cast(geometry->sytIntervalFrames))) { + return false; + } cadence_ = static_cast(&blocking48kCadence_); } else { cadence_ = static_cast(&nonBlocking48kCadence_); diff --git a/tests/audio/BlockingCadenceTests.cpp b/tests/audio/BlockingCadenceTests.cpp index 4be9062f..1f2ad625 100644 --- a/tests/audio/BlockingCadenceTests.cpp +++ b/tests/audio/BlockingCadenceTests.cpp @@ -149,7 +149,7 @@ TEST(BlockingCadenceTests, ResetClearsState) { TEST(BlockingCadenceTests, ConfigureFortyEightKMatchesDefaultPattern) { BlockingCadence cadence; - cadence.Configure(48000, 8); + ASSERT_TRUE(cadence.Configure(48000, 8)); bool expected[] = {false, true, true, true, false, true, true, true}; for (int i = 0; i < 8; i++) { @@ -161,7 +161,7 @@ TEST(BlockingCadenceTests, ConfigureFortyEightKMatchesDefaultPattern) { TEST(BlockingCadenceTests, FortyFourKFirstCycleIsNoData) { BlockingCadence cadence; - cadence.Configure(44100, 8); + ASSERT_TRUE(cadence.Configure(44100, 8)); // 44100/8000 = 5.5125 frames pending after cycle 0 -> < syt_interval(8). EXPECT_FALSE(cadence.CurrentCycleIsData()); EXPECT_EQ(cadence.CurrentCycleDataFrames(), 0); @@ -169,7 +169,7 @@ TEST(BlockingCadenceTests, FortyFourKFirstCycleIsNoData) { TEST(BlockingCadenceTests, FortyFourKDataPacketsCarryEightFrames) { BlockingCadence cadence; - cadence.Configure(44100, 8); + ASSERT_TRUE(cadence.Configure(44100, 8)); for (int i = 0; i < 64; ++i) { if (cadence.CurrentCycleIsData()) { EXPECT_EQ(cadence.CurrentCycleDataFrames(), 8); @@ -182,7 +182,7 @@ TEST(BlockingCadenceTests, FortyFourKDataPacketsCarryEightFrames) { TEST(BlockingCadenceTests, FortyFourKExactOverTwoSeconds) { BlockingCadence cadence; - cadence.Configure(44100, 8); + ASSERT_TRUE(cadence.Configure(44100, 8)); uint64_t totalSamples = 0; // 44100 * 16000 / 64000 = 11025 data cycles exactly -> 88200 frames, with // the fractional accumulator returning to zero residual every 2 seconds. @@ -195,7 +195,7 @@ TEST(BlockingCadenceTests, FortyFourKExactOverTwoSeconds) { TEST(BlockingCadenceTests, FortyFourKNoDriftAcrossTenSeconds) { BlockingCadence cadence; - cadence.Configure(44100, 8); + ASSERT_TRUE(cadence.Configure(44100, 8)); uint64_t totalSamples = 0; for (int i = 0; i < 80000; ++i) { // 10 s totalSamples += cadence.CurrentCycleDataFrames(); @@ -204,6 +204,41 @@ TEST(BlockingCadenceTests, FortyFourKNoDriftAcrossTenSeconds) { EXPECT_EQ(totalSamples, 441000u); } +// The adapter is a seeded RationalBlockingCadence; it must reproduce the +// FFADO accumulate-then-test rule (cip.c: data iff pending + rate/8000 frames +// >= syt_interval) exactly, cycle for cycle, at every supported rate. The +// reference accumulator lives only in this test; production runs the deadline +// engine with the accumulator-equivalent seed. +TEST(BlockingCadenceTests, AdapterMatchesAccumulatorSemanticsAtEveryRate) { + struct RateCase { + uint32_t rate; + uint8_t interval; + }; + constexpr RateCase kCases[] = {{32000, 8}, {44100, 8}, {48000, 8}, + {88200, 16}, {96000, 16}, {176400, 32}, + {192000, 32}}; + for (const auto& c : kCases) { + SCOPED_TRACE("rate " + std::to_string(c.rate)); + BlockingCadence cadence; + ASSERT_TRUE(cadence.Configure(c.rate, c.interval)); + const uint32_t threshold = uint32_t(c.interval) * 8000u; + uint64_t ready = 0; // pending frames scaled by 8000 + for (uint32_t cycle = 0; cycle < 16000; ++cycle) { // 2 s of bus time + const bool refData = (ready + c.rate) >= threshold; + ASSERT_EQ(cadence.CurrentCycleIsData(), refData) + << "cycle " << cycle; + ASSERT_EQ(cadence.CurrentCycleDataFrames(), + refData ? c.interval : 0) + << "cycle " << cycle; + ready += c.rate; + if (refData) { + ready -= threshold; + } + cadence.AdvanceCycle(); + } + } +} + TEST(BlockingCadenceTests, MatchesFireBugPattern) { Blocking48kCadence cadence; From 242345eeaa60be94474a32938476201981c9fe8e Mon Sep 17 00:00:00 2001 From: alicankaralar Date: Sun, 12 Jul 2026 10:45:27 +0200 Subject: [PATCH 22/23] fix(audio): validate sample-rate changes against advertised rates and require the nub Review findings on HandleChangeSampleRate: - The requested rate went to SetSampleRate unchecked; a rate the device never advertised would commit to the HAL and desync host from the DICE clock. Now refused with kIOReturnUnsupported unless present in the discovered sampleRates table. - With no audioNub the transport reconfigure was silently skipped but the rate still committed, telling CoreAudio the hardware moved when it didn't. Now refused with kIOReturnNotReady. Also add bounds asserts before the index-based fixture rewrite in the DICE prepare-parity test so fixture reshapes fail cleanly instead of reading out of bounds. Co-Authored-By: Claude Fable 5 --- .../Audio/DriverKit/ASFWAudioDevice.cpp | 41 ++++++++++++++----- .../DICEDuplexBringupControllerTests.cpp | 2 + 2 files changed, 33 insertions(+), 10 deletions(-) diff --git a/ASFWDriver/Audio/DriverKit/ASFWAudioDevice.cpp b/ASFWDriver/Audio/DriverKit/ASFWAudioDevice.cpp index 5426f1ac..f8dbfe51 100644 --- a/ASFWDriver/Audio/DriverKit/ASFWAudioDevice.cpp +++ b/ASFWDriver/Audio/DriverKit/ASFWAudioDevice.cpp @@ -626,6 +626,22 @@ kern_return_t ASFWAudioDevice::HandleChangeSampleRate(double in_sample_rate) { const uint32_t rateHz = static_cast(in_sample_rate); + // Only accept rates this device advertised; anything else would program a + // clock the transport can't honor and desync host from device. + bool rateSupported = false; + for (uint32_t i = 0; i < ivars.device.sampleRateCount; ++i) { + if (static_cast(ivars.device.sampleRates[i]) == rateHz) { + rateSupported = true; + break; + } + } + if (!rateSupported) { + ASFW_LOG(Audio, + "ASFWAudioDevice: HandleChangeSampleRate %.0f Hz refused - unsupported rate", + in_sample_rate); + return kIOReturnUnsupported; + } + // Reject a rate change while IO is active. Live (hot) reconfiguration would // restart the duplex transport underneath running IO across the cross-service // seam, which currently desynchronizes the device clock from the host and @@ -645,17 +661,22 @@ kern_return_t ASFWAudioDevice::HandleChangeSampleRate(double in_sample_rate) { // coordinator (CLOCK_SELECT + duplex reconfigure). The device is idle here, // so this stores/applies the clock for the next StartIO. Reject the change if // the transport can't apply it so CoreAudio does not believe the hardware moved. - if (ivars.device.audioNub) { - const kern_return_t kr = - ivars.device.audioNub->RequestSampleRateChange(rateHz); - if (kr != kIOReturnSuccess) { - ASFW_LOG(Audio, - "ASFWAudioDevice: HandleChangeSampleRate transport reconfig failed: 0x%x", - kr); - return kr; - } - ivars.device.currentSampleRate = static_cast(rateHz); + if (!ivars.device.audioNub) { + // Without the nub the device clock can't be programmed; succeeding here + // would make CoreAudio believe the hardware moved when it didn't. + ASFW_LOG(Audio, + "ASFWAudioDevice: HandleChangeSampleRate %.0f Hz refused - no audio nub", + in_sample_rate); + return kIOReturnNotReady; + } + const kern_return_t kr = ivars.device.audioNub->RequestSampleRateChange(rateHz); + if (kr != kIOReturnSuccess) { + ASFW_LOG(Audio, + "ASFWAudioDevice: HandleChangeSampleRate transport reconfig failed: 0x%x", + kr); + return kr; } + ivars.device.currentSampleRate = static_cast(rateHz); // Commit the rate to the ADK device. The validated ADK contract // (ADKVirtualAudioLab) applies the change by calling SetSampleRate here, not diff --git a/tests/devices/DICEDuplexBringupControllerTests.cpp b/tests/devices/DICEDuplexBringupControllerTests.cpp index a8a5dc91..36219aae 100644 --- a/tests/devices/DICEDuplexBringupControllerTests.cpp +++ b/tests/devices/DICEDuplexBringupControllerTests.cpp @@ -1003,9 +1003,11 @@ TEST(DICEDuplexBringupControllerTests, PrepareSequenceMatchesReferenceWindow) { const auto& refRequests = ReferencePhase0ParityFixture::kPrepareExpectedRequests; const auto& refResponses = ReferencePhase0ParityFixture::kPrepareResponseSteps; std::vector requests(refRequests.begin(), refRequests.end()); + ASSERT_GT(requests.size(), 7U); ASSERT_EQ(requests[6].kind, OpKind::Write); // CLOCK_SELECT in the reference requests[6] = requests[7]; // becomes the await-lock global read std::vector responses(refResponses.begin(), refResponses.end()); + ASSERT_GT(responses.size(), 6U); responses.insert(responses.begin() + 7, responses[6]); // second locked global read rig.bus.SetScript(requests, responses); From c2765464bfd833b4b016d057c09bcecb7393abb5 Mon Sep 17 00:00:00 2001 From: alicankaralar Date: Sun, 12 Jul 2026 13:00:27 +0200 Subject: [PATCH 23/23] Stability improvements --- ASFWDriver/ASFWDriver.cpp | 10 +- ASFWDriver/Hardware/InterruptManager.cpp | 40 +++++- ASFWDriver/Hardware/InterruptManager.hpp | 16 ++- .../SCSIController/ASFWSCSIController.cpp | 122 +++++++++++++++++- .../SCSIController/ASFWSCSIController.iig | 12 ++ ASFWDriver/Service/DriverContext.cpp | 55 +++++--- ASFWDriver/Service/DriverContext.hpp | 11 +- ASFWDriver/Testing/HostDriverKitStubs.hpp | 18 +++ 8 files changed, 256 insertions(+), 28 deletions(-) diff --git a/ASFWDriver/ASFWDriver.cpp b/ASFWDriver/ASFWDriver.cpp index 02293a8f..b6222e5e 100644 --- a/ASFWDriver/ASFWDriver.cpp +++ b/ASFWDriver/ASFWDriver.cpp @@ -392,6 +392,12 @@ kern_return_t ASFWDriver::StartRuntime(IOService* provider) { kern_return_t IMPL(ASFWDriver, Stop) { QuiesceRuntime(); + if (ivars && ivars->context && ivars->context->deps.interrupts) { + // Real stop: cancel the interrupt dispatch source now, while the + // provider is still attached, so the kernel-side unregisterInterrupt + // is ordered before termination instead of racing it. + ivars->context->deps.interrupts->Teardown(); + } if (ivars) { if (ivars->wakeVerifyTimer) { // Disable only, then release — Cancel() dispatches async and can @@ -475,7 +481,7 @@ kern_return_t IMPL(ASFWDriver, SetPowerState) { if (!ivars->runtimeSuspended && ivars->context) { ASFW_LOG(Controller, "SetPowerState: quiescing runtime for sleep"); QuiesceRuntime(); - ivars->context->Reset(); + ivars->context->Reset(ServiceContext::ResetMode::ForSuspend); ivars->runtimeSuspended = true; } } else { @@ -566,7 +572,7 @@ void ASFWDriver::VerifyWakeRuntime(uint64_t attempt) { } QuiesceRuntime(); - ctx.Reset(); + ctx.Reset(ServiceContext::ResetMode::ForSuspend); const kern_return_t kr = StartRuntime(ivars->powerProvider); if (kr != kIOReturnSuccess) { ASFW_LOG(Controller, "Wake verify: ❌ rebuild failed: 0x%08x", kr); diff --git a/ASFWDriver/Hardware/InterruptManager.cpp b/ASFWDriver/Hardware/InterruptManager.cpp index 25b34bbc..49ecb465 100644 --- a/ASFWDriver/Hardware/InterruptManager.cpp +++ b/ASFWDriver/Hardware/InterruptManager.cpp @@ -15,7 +15,7 @@ namespace ASFW::Driver { InterruptManager::InterruptManager() = default; -InterruptManager::~InterruptManager() = default; +InterruptManager::~InterruptManager() { Teardown(); } kern_return_t InterruptManager::Initialise(IOService* owner, OSSharedPtr queue, @@ -61,6 +61,44 @@ void InterruptManager::Disable() { if (source_) { source_->SetEnableWithCompletion(false, nullptr); } + // The manager (and its shadow mask) now survives suspend, but the silicon's + // IntMask does not — a stale shadow would make UnmaskInterrupts skip the + // hardware write after the wake re-init's soft reset. + shadowMask_.store(0, std::memory_order_release); +} + +void InterruptManager::Teardown() { + if (!source_) { + queue_.reset(); + handler_.reset(); + return; + } + + source_->SetEnableWithCompletion(false, nullptr); + + // The kernel-side source performs IOService::unregisterInterrupt in its + // free(), i.e. whenever the last reference drops. Hand the final references + // to the cancel completion so that free is ordered after cancellation + // instead of landing whenever the async release RPC is processed. (Releasing + // before the completion ran is the Cancel_Impl crash noted in + // WatchdogCoordinator::Stop; releasing without Cancel at all is the + // 2026-07-11 IOSharedInterruptController panic.) + IOInterruptDispatchSource* source = source_.detach(); + OSAction* handler = handler_.detach(); + const kern_return_t kr = source->Cancel(^{ + if (handler) { + handler->release(); + } + source->release(); + }); + if (kr != kIOReturnSuccess) { + // Completion will never run; fall back to direct release. + if (handler) { + handler->release(); + } + source->release(); + } + queue_.reset(); } void InterruptManager::EnableInterrupts(uint32_t bits) { diff --git a/ASFWDriver/Hardware/InterruptManager.hpp b/ASFWDriver/Hardware/InterruptManager.hpp index af5aa32b..a8b2bce6 100644 --- a/ASFWDriver/Hardware/InterruptManager.hpp +++ b/ASFWDriver/Hardware/InterruptManager.hpp @@ -20,14 +20,28 @@ namespace ASFW::Driver { class InterruptManager { public: InterruptManager(); - ~InterruptManager(); + ~InterruptManager(); // runs Teardown() if the source still exists kern_return_t Initialise(IOService* owner, OSSharedPtr queue, OSSharedPtr handler); + // True once Initialise() has created the dispatch source. The source is + // deliberately kept across sleep/wake (see ServiceContext::Reset ForSuspend): + // destroying and re-creating it re-registers the same interrupt vector and + // races the old source's deferred kernel-side unregisterInterrupt — on a + // shared interrupt controller the second unregister panics the kernel + // (NULL vectorData in IOSharedInterruptController::disableInterrupt). + [[nodiscard]] bool HasSource() const noexcept { return static_cast(source_); } + void Enable(); void Disable(); + + // Final teardown (driver Stop / destruction only — never for suspend). + // Cancels the dispatch source and defers the last releases to the cancel + // completion, so the kernel-side free (which unregisters the interrupt) + // cannot land at an uncontrolled time. + void Teardown(); void EnableInterrupts(uint32_t bits); void DisableInterrupts(uint32_t bits); diff --git a/ASFWDriver/SCSIController/ASFWSCSIController.cpp b/ASFWDriver/SCSIController/ASFWSCSIController.cpp index 3580c7f9..68bdd490 100644 --- a/ASFWDriver/SCSIController/ASFWSCSIController.cpp +++ b/ASFWDriver/SCSIController/ASFWSCSIController.cpp @@ -26,9 +26,11 @@ #include #include #include +#include #include #include +#include "../Common/TimingUtils.hpp" #include "../Logging/Logging.hpp" #include "../Protocols/SBP2/SCSICommandSet.hpp" #include "SBP2BridgeHub.hpp" @@ -71,6 +73,12 @@ struct HeldInquiry { struct PendingState { IOLock* lock; HeldInquiry inquiry; + // Set when a held INQUIRY expired without login. Later pre-login INQUIRYs + // then answer BUSY immediately instead of re-holding: the SAM retries the + // probe a few times, and each re-hold would stack another full hold window + // onto a nub that has been busy since boot — back into the 60 s panic. + // Cleared on the next login-up notification. + bool loginWindowExpired; }; // Must match UserGetDMASpecification's maxTransferSize. Sized as a permissive @@ -83,6 +91,13 @@ constexpr uint64_t kMaxTransferPerTask = 1u * 1024u * 1024u; // to a specific model. constexpr uint32_t kDefaultTaskTimeoutMs = 60'000; +// Upper bound on how long a pre-login probe INQUIRY may be held. The kernel +// probe path has no timeout of its own and blocks the nub's IOConfigThread; +// the registry busy-timeout panics at 60 s of sustained busy (IOService.cpp: +// 5986). 20 s spans a normal login (~2-5 s) plus bus-reset retries while +// leaving ample headroom before the panic. +constexpr uint64_t kHeldInquiryMaxHoldNs = 20'000'000'000ull; + // Map an SBP-2 command result onto the SAM response. Synthetic // kIOReturnNotReady (bridge accepted the task but the session dropped out // before submission) maps to BUSY so the initiator retries; other transport @@ -134,7 +149,7 @@ bool TryHoldInquiry(PendingState* ps, const HeldInquiry& held) { return false; } IOLockLock(ps->lock); - const bool slotFree = !ps->inquiry.inUse; + const bool slotFree = !ps->inquiry.inUse && !ps->loginWindowExpired; if (slotFree) { ps->inquiry = held; } @@ -142,6 +157,24 @@ bool TryHoldInquiry(PendingState* ps, const HeldInquiry& held) { return slotFree; } +void MarkLoginWindowExpired(PendingState* ps) { + if (ps == nullptr || ps->lock == nullptr) { + return; + } + IOLockLock(ps->lock); + ps->loginWindowExpired = true; + IOLockUnlock(ps->lock); +} + +void ClearLoginWindowExpired(PendingState* ps) { + if (ps == nullptr || ps->lock == nullptr) { + return; + } + IOLockLock(ps->lock); + ps->loginWindowExpired = false; + IOLockUnlock(ps->lock); +} + // Take ownership of the held INQUIRY (if any). Exactly one caller wins; the // retains transfer to it. bool ExtractHeldInquiry(PendingState* ps, HeldInquiry* out) { @@ -261,6 +294,11 @@ void ASFWSCSIController::free() IOSafeDeleteNULL(ps, PendingState, 1); ivars->pendingState = nullptr; } + if (ivars != nullptr) { + // Backstop for a Start that failed before Stop could run. + OSSafeReleaseNULL(ivars->holdTimerAction); + OSSafeReleaseNULL(ivars->holdTimer); + } IOSafeDeleteNULL(ivars, ASFWSCSIController_IVars, 1); super::free(); } @@ -283,6 +321,27 @@ kern_return_t IMPL(ASFWSCSIController, Start) ASFW_LOG(Controller, "[SCSIHBA] SetDispatchQueue(AuxiliaryQueue) failed: 0x%x", ret); return ret; } + // The hold-bound timer must exist before the framework can probe target 0 + // (a held INQUIRY without it can wedge the nub's IOConfigThread past the + // 60 s registry busy timeout). Created on the aux queue so the expiry + // handler serializes with the login drain and the Stop flush barrier. + ret = IOTimerDispatchSource::Create(ivars->auxQueue, &ivars->holdTimer); + if (ret != kIOReturnSuccess || ivars->holdTimer == nullptr) { + ASFW_LOG(Controller, "[SCSIHBA] hold timer create failed: 0x%x", ret); + return ret != kIOReturnSuccess ? ret : kIOReturnError; + } + ret = CreateActionHeldInquiryTimerFired(0, &ivars->holdTimerAction); + if (ret != kIOReturnSuccess || ivars->holdTimerAction == nullptr) { + ASFW_LOG(Controller, "[SCSIHBA] hold timer action create failed: 0x%x", ret); + return ret != kIOReturnSuccess ? ret : kIOReturnError; + } + ret = ivars->holdTimer->SetHandler(ivars->holdTimerAction); + if (ret != kIOReturnSuccess) { + ASFW_LOG(Controller, "[SCSIHBA] hold timer SetHandler failed: 0x%x", ret); + return ret; + } + (void)ivars->holdTimer->SetEnableWithCompletion(true, nullptr); + ret = Start(provider, SUPERDISPATCH); if (ret != kIOReturnSuccess) { ASFW_LOG(Controller, "[SCSIHBA] super::Start failed: 0x%x", ret); @@ -303,8 +362,10 @@ kern_return_t IMPL(ASFWSCSIController, Start) } this->retain(); ivars->auxQueue->DispatchAsync(^{ + auto* ps = static_cast(ivars->pendingState); + ClearLoginWindowExpired(ps); HeldInquiry held{}; - if (ExtractHeldInquiry(static_cast(ivars->pendingState), &held)) { + if (ExtractHeldInquiry(ps, &held)) { ASFW_LOG(Controller, "[SCSIHBA] replaying held INQUIRY after login"); SubmitHeldInquiry(this, held); } @@ -314,6 +375,28 @@ kern_return_t IMPL(ASFWSCSIController, Start) return kIOReturnSuccess; } +void ASFWSCSIController::HeldInquiryTimerFired_Impl( + ASFWSCSIController_HeldInquiryTimerFired_Args) +{ + (void)action; + (void)time; + if (ivars == nullptr) { + return; + } + auto* ps = static_cast(ivars->pendingState); + HeldInquiry held{}; + if (ExtractHeldInquiry(ps, &held)) { + // Login never arrived. Refuse further holds until it does (the SAM's + // probe retries would otherwise stack fresh hold windows onto a nub + // that has been busy since boot) and fail the probe with BUSY. + MarkLoginWindowExpired(ps); + ASFW_LOG(Controller, + "[SCSIHBA] held INQUIRY expired without SBP-2 login → BUSY " + "(further pre-login INQUIRYs answer BUSY until login)"); + CompleteHeldInquiryBusy(this, held); + } +} + kern_return_t IMPL(ASFWSCSIController, Stop) { ASFW_LOG(Controller, "[SCSIHBA] Stop"); @@ -332,6 +415,28 @@ kern_return_t IMPL(ASFWSCSIController, Stop) CompleteHeldInquiryBusy(this, held); } }); + if (ivars->holdTimer != nullptr) { + // Same ordering rule as InterruptManager::Teardown: the final + // releases ride in the cancel completion, so the kernel-side free + // cannot land while a fire is still in flight. + IOTimerDispatchSource* timer = ivars->holdTimer; + OSAction* timerAction = ivars->holdTimerAction; + ivars->holdTimer = nullptr; + ivars->holdTimerAction = nullptr; + timer->SetEnableWithCompletion(false, nullptr); + const kern_return_t ckr = timer->Cancel(^{ + if (timerAction != nullptr) { + timerAction->release(); + } + timer->release(); + }); + if (ckr != kIOReturnSuccess) { + if (timerAction != nullptr) { + timerAction->release(); + } + timer->release(); + } + } OSSafeReleaseNULL(ivars->auxQueue); } // No UserDestroyTargetForID: target 0 is framework-auto-created (presence @@ -612,10 +717,21 @@ kern_return_t IMPL(ASFWSCSIController, UserProcessParallelTask) completion->retain(); if (TryHoldInquiry(static_cast(ivars->pendingState), held)) { ASFW_LOG(Controller, "[SCSIHBA] INQUIRY deferred until SBP-2 login"); + // Bound the hold: the kernel probe blocks the nub's + // IOConfigThread with no timeout of its own, and the + // registry panics at 60 s of sustained busy. + if (ivars->holdTimer != nullptr) { + (void)ASFW::Timing::initializeHostTimebase(); + const uint64_t deadline = + mach_absolute_time() + + ASFW::Timing::nanosToHostTicks(kHeldInquiryMaxHoldNs); + (void)ivars->holdTimer->WakeAtTime(kIOTimerClockMachAbsoluteTime, + deadline, 0); + } if (response != nullptr) { *response = kIOReturnSuccess; } - return kIOReturnSuccess; // completion fires on drain/flush + return kIOReturnSuccess; // completion fires on drain/flush/expiry } // Slot already occupied — undo the retain, fall through to BUSY. completion->release(); diff --git a/ASFWDriver/SCSIController/ASFWSCSIController.iig b/ASFWDriver/SCSIController/ASFWSCSIController.iig index ad9429b2..719c01a7 100644 --- a/ASFWDriver/SCSIController/ASFWSCSIController.iig +++ b/ASFWDriver/SCSIController/ASFWSCSIController.iig @@ -30,6 +30,7 @@ #ifndef ASFWSCSIController_h #define ASFWSCSIController_h +#include #include class ASFWSCSIController: public IOUserSCSIParallelInterfaceController @@ -75,6 +76,15 @@ public: uint32_t* response, OSAction* completion) override; + // Bounds the pre-login held INQUIRY. The kernel probe (IOSCSITargetDevice → + // RetrieveDefaultINQUIRYData → WaitForTask) blocks the nub's IOConfigThread + // with no timeout of its own; if SBP-2 login never arrives, the registry + // busy-timeout panics the machine at 60 s (IOService.cpp:5986 — the + // 2026-07-10 boot loop). Fires on the AuxiliaryQueue and flushes the hold + // with BUSY. + virtual void HeldInquiryTimerFired(OSAction* action, uint64_t time) + TYPE(IOTimerDispatchSource::TimerOccurred); + // --- bundled-task path: opt OUT (return failure) so the framework uses the // single-task UserProcessParallelTask path above. Both are pure-virtual // so must be declared; UserProcessBundledParallelTasks is never called. --- @@ -106,6 +116,8 @@ struct ASFWSCSIController_IVars { IODispatchQueue* auxQueue{nullptr}; // the framework's "AuxiliaryQueue" contract uint32_t nextUniqueTaskID{0}; // handed out in UserMapHBAData (must be unique per task) void* pendingState{nullptr}; // PendingState* (deferred pre-login INQUIRY), defined in .cpp + IOTimerDispatchSource* holdTimer{nullptr}; // bounds the held INQUIRY (AuxiliaryQueue) + OSAction* holdTimerAction{nullptr}; }; #endif /* ASFWSCSIController_h */ diff --git a/ASFWDriver/Service/DriverContext.cpp b/ASFWDriver/Service/DriverContext.cpp index 6e8f03b2..7d606682 100644 --- a/ASFWDriver/Service/DriverContext.cpp +++ b/ASFWDriver/Service/DriverContext.cpp @@ -49,7 +49,7 @@ void ServiceContext::DisarmProviderNotifications() { #endif } -void ServiceContext::Reset() { +void ServiceContext::Reset(ResetMode mode) { stopping.store(true, std::memory_order_release); if (audioCoordinator) { audioCoordinator->BeginTeardown(); @@ -79,7 +79,9 @@ void ServiceContext::Reset() { deps.stateMachine.reset(); deps.configRom.reset(); deps.configRomStager.reset(); - deps.interrupts.reset(); + if (mode == ResetMode::Full) { + deps.interrupts.reset(); // ~InterruptManager cancels the dispatch source + } deps.topology.reset(); deps.topologyMapService.reset(); deps.busManagerElectionDriver.reset(); @@ -95,8 +97,10 @@ void ServiceContext::Reset() { statusPublisher.Reset(); watchdog.Reset(); DisarmProviderNotifications(); - workQueue.reset(); - interruptAction.reset(); + if (mode == ResetMode::Full) { + workQueue.reset(); + interruptAction.reset(); + } } namespace ASFW::Driver { @@ -275,30 +279,41 @@ kern_return_t DriverWiring::PrepareInterrupts(ASFWDriver& service, IOService* pr return kIOReturnBadArgument; } - auto pci = OSDynamicCast(IOPCIDevice, provider); - if (!pci) { - return kIOReturnBadArgument; + auto intrMgr = ctx.deps.interrupts; + if (!intrMgr) { + return kIOReturnNoResources; } - auto status = pci->ConfigureInterrupts(kIOInterruptTypePCIMessagedX, 1, 1, 0); - if (status != kIOReturnSuccess) { - status = pci->ConfigureInterrupts(kIOInterruptTypePCIMessaged, 1, 1, 0); + // MSI configuration happens once per provider lifetime, only before the + // dispatch source exists. The source survives suspend/rebuild (see + // ServiceContext::ResetMode): re-running ConfigureInterrupts or creating a + // second source would re-register the same interrupt vector while the old + // registration is still live, and the eventual double-unregister panics + // the kernel on a shared interrupt controller. + if (!intrMgr->HasSource()) { + auto pci = OSDynamicCast(IOPCIDevice, provider); + if (!pci) { + return kIOReturnBadArgument; + } + + auto status = pci->ConfigureInterrupts(kIOInterruptTypePCIMessagedX, 1, 1, 0); if (status != kIOReturnSuccess) { - return status; + status = pci->ConfigureInterrupts(kIOInterruptTypePCIMessaged, 1, 1, 0); + if (status != kIOReturnSuccess) { + return status; + } } } - OSAction* action = nullptr; - auto kr = service.CreateActionInterruptOccurred(0, &action); - if (kr != kIOReturnSuccess || !action) - return kr != kIOReturnSuccess ? kr : kIOReturnError; - ctx.interruptAction = OSSharedPtr(action, OSNoRetain); - auto intrMgr = ctx.deps.interrupts; - if (!intrMgr) { - return kIOReturnNoResources; + if (!ctx.interruptAction) { + OSAction* action = nullptr; + auto kr = service.CreateActionInterruptOccurred(0, &action); + if (kr != kIOReturnSuccess || !action) + return kr != kIOReturnSuccess ? kr : kIOReturnError; + ctx.interruptAction = OSSharedPtr(action, OSNoRetain); } - kr = intrMgr->Initialise(provider, ctx.workQueue, ctx.interruptAction); + auto kr = intrMgr->Initialise(provider, ctx.workQueue, ctx.interruptAction); if (kr != kIOReturnSuccess) { ctx.interruptAction.reset(); return kr; diff --git a/ASFWDriver/Service/DriverContext.hpp b/ASFWDriver/Service/DriverContext.hpp index 024aae78..c8e7d9e0 100644 --- a/ASFWDriver/Service/DriverContext.hpp +++ b/ASFWDriver/Service/DriverContext.hpp @@ -52,7 +52,16 @@ struct ServiceContext { std::shared_ptr sbp2Bridge; void DisarmProviderNotifications(); - void Reset(); + + // Full tears everything down (driver free/Stop). ForSuspend (sleep and the + // wake-verify self-heal) preserves the interrupt machinery — dispatch + // source, work queue, interrupt action — because destroying and re-creating + // the IOInterruptDispatchSource across a rebuild re-registers the same + // interrupt vector while the old source's kernel-side unregister is still + // in flight; the resulting double-unregister panics the kernel on a shared + // interrupt controller (observed 2026-07-10/11 on the FW643). + enum class ResetMode { Full, ForSuspend }; + void Reset(ResetMode mode = ResetMode::Full); }; namespace ASFW::Driver { diff --git a/ASFWDriver/Testing/HostDriverKitStubs.hpp b/ASFWDriver/Testing/HostDriverKitStubs.hpp index 4d8236c7..6fbfaba3 100644 --- a/ASFWDriver/Testing/HostDriverKitStubs.hpp +++ b/ASFWDriver/Testing/HostDriverKitStubs.hpp @@ -240,6 +240,12 @@ class IOInterruptDispatchSource : public OSObject { kern_return_t SetHandler(OSAction*) { return kIOReturnUnsupported; } kern_return_t SetEnableWithCompletion(bool, void*) { return kIOReturnUnsupported; } + kern_return_t Cancel(void (^handler)(void)) { + if (handler) { + handler(); + } + return kIOReturnSuccess; + } }; class IOTimerDispatchSource : public OSObject { @@ -433,6 +439,18 @@ class OSSharedPtr { void reset(T* ptr, OSNoRetainTag) { ptr_.reset(ptr); } void reset(T* ptr, OSRetainTag) { ptr_.reset(ptr); } + // Ownership transfer to the caller. Stub OSObject::release() is a no-op, + // so keep one strong ref alive (intentional leak) instead of letting the + // shared_ptr destroy an object the caller still holds. + T* detach() { + T* raw = ptr_.get(); + if (raw) { + new std::shared_ptr(ptr_); + } + ptr_.reset(); + return raw; + } + private: std::shared_ptr ptr_; };