Repository navigation
drm: apple, usb: typec: tipd: DCP audio, AV AFK and CD321x unplug fixes, ported onto aurora-wip-rebase - #4
Draft
nekolyanich wants to merge 6 commits into
Draft
nekolyanich wants to merge 6 commits into
nekolyanich wants to merge 6 commits into
Conversation
added 6 commits
October 19, 2026 21:49
afk_recv_handle_reply() dropped a reply shorter than the command header. Its caller then waited the command out; if the command had already timed out (free_on_ack), its DMA buffers and its cmd_map bit were never freed, and the service's slot could not be reclaimed. Complete it instead, as failed. The short replies seen are 4 bytes, which looks like a bare IOReturn (an assumption): a nonzero one is passed on, anything else completes with AFK_RETCODE_SHORT_REPLY (~0), so that no caller reads its output buffer as a reply. AFK v2 replies reach this function with a full header and are not affected. Seen with 13.5 firmware on an M2 Pro (downstream trees), always 4 bytes: 28 times on the DCPDPDevice endpoint across 7 boots, the first in each boot for a command still pending (whose caller then timed out), the rest for commands that had already timed out; and once on the AV endpoint, for an audio getElements stuck behind a blocked rx worker. [Port onto aurora-wip-rebase: afk_recv_handle() has sent only full-size replies to afk_recv_handle_reply() since commit 107036c ("drm/apple, media: apple: avd: T6030 internal panel, external display and AVD fixes") and now lets a short one through again, unless the firmware is 14.7, which keeps its own short-reply path from commit b12111f ("drm/apple: Reclaim timed-out commands on short AFK replies"). A short reply leaves reply_len_valid unset, so callers of afk_send_command_with_reply_len() get -EPROTO.] Assisted-by: Claude:claude-opus-5-5 checkpatch Signed-off-by: nekolyanich <me@nekolyanich.com>
Add the slot index to the "new service" line, and whether the slot was used before, so slot reuse can be told from a table that is merely not full yet. Assisted-by: Claude:claude-opus-5-5 checkpatch Signed-off-by: nekolyanich <me@nekolyanich.com>
is_open said that the audio service had been opened, but not which announce of it: a teardown left it set, and if the display's disconnect came after the teardown, av_service_disconnect() found no service and returned without clearing it, so the next generation was never opened. Keep the generation that was opened instead, counted at each announce, and set it only if no teardown came during the open; a generation torn down meanwhile is not connected to the audio device either. The open and the close checked and changed the state without a lock, and the hotplug, power and start paths call them concurrently (two disconnects 0.7 ms apart seen once): serialise them with open_lock, which the AFK rx worker never takes. Found by reading, not seen to matter in our runs. [Port onto aurora-wip-rebase: rebased by hand onto commit d015393 ("drm/apple: av: recycle AV endpoint service slots"), which keeps is_open; the open and the close keep its afk_service_pin() and its disable of a torn down service. The generation is now also counted at the teardown of the published service, so that a teardown without a new announce during the open is seen too.] Assisted-by: Claude:claude-opus-5-5 checkpatch Signed-off-by: nekolyanich <me@nekolyanich.com>
When the audio elements do not fit the buffer of the get_elements call, DCP returns them cut off at the buffer's end and reports a used size of 0. A Samsung TV behind a USB-C to HDMI adapter has 17312 bytes of elements, so with the 16 KiB we asked for, everything past 16 KiB was lost. macOS 13 asks for up to 256 KiB (max_size 0x40000 in the request, 0x43a0 used in the reply for this TV): do the same. Allocate the elements buffer and the call buffers with kvzalloc(), including the one in afk_service_call_full(), which every AFK service call goes through; it is only copied into the DMA buffers. [Port onto aurora-wip-rebase: the AFK call buffer is now in afk_service_call_full().] Fixes: cb24343 ("gpu: drm: apple: Add DCP audio driver") Assisted-by: Claude:claude-opus-5-5 checkpatch Signed-off-by: nekolyanich <me@nekolyanich.com>
…topped Trigger STOP put the runtime PM reference right after starting the asynchronous DMA terminate. Without autosuspend that suspends the device at once and powers its domain (ps_dpa1/ps_dpa2) down while SIO may still run DMA towards the DPA. SIO then crashed with "Shim DMA unavailable" and the TERMINATE timed out after it, taking DP audio on all outputs down until a reboot. On an M2 Pro with two DP sinks an earlier version of this change on a downstream tree took it from 4 of 4 boots to none in 11. Put the reference in sync_stop, after snd_dmaengine_pcm_sync_stop() waited for the terminate. ALSA calls sync_stop at the next hw_params, prepare or hw_free, at close, at system suspend and at disconnect, not right after the STOP, so after a drain, an xrun or a stop with the stream left open the DPA stays powered until then. Waiting inside trigger would also work (the PCM is nonatomic), but sync_stop is the hook ALSA has for it, as in the AOP audio driver. sync_stop runs outside the stream lock: test and clear pm_held under it. The check in START now matters, since a START can follow a failed one before sync_stop ran. [Port onto aurora-wip-rebase: the parent of this change, "drm: apple: audio: balance the runtime PM reference of a failed START", is left out: since commit cf7ddf4 ("drm: apple: audio: play into the void without a sink") a START without a sink no longer fails before taking the reference. This change adds pm_held itself, and joins the sync_stop added there. A stream stopped by a sink unplug also keeps the DPA powered until sync_stop.] Fixes: f25cb50 ("drm: apple: audio: Implement runtime PM support") Assisted-by: Claude:claude-opus-5-5 checkpatch Signed-off-by: nekolyanich <me@nekolyanich.com>
On an M2 Pro (j416s), replugs of a USB-C to HDMI adapter in DP alt mode often never reached the display: 26 of 56 natively, over three days. In a trace of the controller's status (4 cycles), after the unplug of a session that had DP it reported the cable power path (PP_CABLE) "in" and kept the session's data status (DATA_CONNECTION, USB_DATA_ROLE); the next plug got no VBUS, never turned on 5 V and never reached PD (2 of 2). Unplugging that failed plug cleared the state, so the next one worked. macOS, traced under the m1n1 hypervisor with the same TV and adapter, writes 0x2000 to register 0x50 and sends SSPS 2 on every disconnect (8 of 8 in two traces), and after it writes 0x2000 and 0 to 0x50 in 5 of 8. The port then reads back system power state 2 and stays in it while unplugged (3 of 3, the trace that decodes reads). On the next connect, as soon as STATUS shows the plug, macOS sends SSPS 0, before the display gets HPD. At boot it puts the MagSafe port's controller, which has nothing plugged in, into state 2 as well (2 of 2); no empty USB-C port was traced at boot. We have no documentation for register 0x50; it is 6 bytes long, and macOS also writes 0x2000 to it after each DATA_STATUS HPD_IRQ (11 of 11), so the bit is named by number only. Do the same from the cable work: whenever it finds no plug and the port is not known to be asleep (a disconnect, also a retried one, or an empty port at probe), write 0x2000 to 0x50, SSPS 2, 0x2000 and 0 to 0x50 (at probe too, where macOS's order differs slightly); before a new partner is registered, SSPS 0. STATUS is read again right before the sleep, and a plug present skips it: state 2 was never seen entered with a partner attached. So a plug within the debounce of an unplug, or one landing while the cable work runs, leaves the port awake, as before this change. Both go through switch_power_state(), which also reads the state before SSPS (macOS doesn't) and skips a state already set. A failure only warns and leaves the port state unknown, as does a resume, so the next pass tries again. Only the CD321x does this (tipd_cd321x_data sets port_sleep): nothing was traced for the SN201202x, which shares the cable work, so it keeps its behaviour. Testing: MacBook Pro 16" M2 Pro (j416s), DCP firmware 13.5, a TV on a USB-C to HDMI adapter and a Dell over Thunderbolt. Under the m1n1 hypervisor, guided replugs of the TV's adapter with the same sequence behind a module parameter on one kernel: 4 of 16 lost without it, 0 of 24 with it. Cold boots with both displays: 22 of 22 clean across the review rounds of this change (4 of them with this version, which put the empty MagSafe controller to sleep once at probe). Natively, on a downstream tree with the same change, in two sessions. In the first, MagSafe unplugged and replugged after the probe-time sleep was back on AC (1 of 1); the Dell over Thunderbolt unplugged and replugged came back 3 of 3; audio played on both without a refusal; one known atcphy WARNING (atc.c:2681 in atcphy_mux_set(), called from the cable work) fired on a Dell replug; the same WARNING from the same path also fired in an earlier session without this change. In the second, 16 of 16 replugs of the TV's adapter reached the display (0 lost, 0 refusals), 3 of 3 fast replugs (back within about half a second) came back, and a USB-C charger plugged into the same port after an unplug negotiated a power contract (its power supply went online; the Mac was also powered through another port, and the battery was already full); no kernel warning. [Port onto aurora-wip-rebase: cd321x_check_setup(), from commit 72911ae ("usb: typec: tipd: set a CD321x up again when it lost its setup"), sets a reset controller back to S0 and now also leaves the port state unknown, so that the next disconnect puts the port to sleep again. That check takes any state but S0 for a lost setup; it runs only on an interrupt outside the mask, which the sleep is not known to raise. A controller found in "DBMa" at probe is neither put to sleep nor woken, as probe promises to send it no 4CC command. The Testing above was on aurora-wip, before the port.] Assisted-by: Claude:claude-opus-5-5 checkpatch Signed-off-by: nekolyanich <me@nekolyanich.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
DRAFT in the fork for reading; not sent to aurora-silicon.
What is left of our open aurora PRs aurora-silicon#276 (DCP audio elements and runtime PM) and aurora-silicon#296 (CD321x unplug sleep), and of the closed aurora-silicon#299 (AV AFK slots), ported onto
aurora-wip-rebase(0bbaaa0, after aurora-silicon#328; mirrored here asaurora-wip-rebase) as one series, the way aurora-silicon#328 brought sithglan's four PRs in. Found while bringing up DP and HDMI audio on an M2 Pro (j416s) with a Samsung TV on a USB-C to HDMI adapter and a Dell over Thunderbolt.Commits
cmd_mapbit were never freed. It now completes as failed: a 4-byte reply's nonzero IOReturn is passed on, anything else completes with~0. Port: since 107036cafk_recv_handle()sent only full-size replies to the reply handler, so a short reply on 12.3/13.5 ended as "unhandled" in the std-service handler, whose own short-reply reclaim (b12111f) is for 14.7 only. A short REPLY from firmware other than 14.7 goes to the reply handler again; 14.7 keeps your path. A short reply leavesreply_len_validunset, soafk_send_command_with_reply_len()callers get-EPROTO. AFK v2 is not affected.is_openstayed set after a teardown when the display's disconnect came after it (no service left to close), so the next generation was never opened. Keep the opened generation instead, and serialise the open and close with a mutex. Port: rebuilt by hand on d015393's owner pin (afk_service_pin()), which it keeps; the generation also advances when the published service is torn down.kvzalloc().sync_stopaftersnd_dmaengine_pcm_sync_stop(). Port: joins thesync_stopthat "play into the void" added.cd321x_check_setup()(72911ae), which sets a reset controller back to S0, also marks the port state unknown; a controller found in "DBMa" at probe is left alone, as probe promises.Already in aurora-wip-rebase, so not here
pm_heldflag itself.Testing
Compile and KUnit only so far:
commit <sha> ("title")lines;virt:apple-afk-service-lifetime10/10,cd321x-pm36/36,apple-afk-ringok, 23 of 24 suites ok.apple-display-parserfails inparser_route_programming_guard(parser-test.c:823) the same way onaurora-wip-rebase0bbaaa0 without this series. None of the suites exercises the changed paths directly. The tests in the original PRs ran onaurora-wipbefore these ports. Bench (m1n1 hypervisor) and native tests on these exact commits are pending: DP audio on the TV and the Dell, TV replugs with the CD321x sleep, the MagSafe controller at probe, a USB-C charger after an unplug.Not tested: other Macs and SoCs, DCP firmware 12.3 and H17P, AFK v2, other sinks.
Open questions (for us before sending): the CD321x sleep is on for every CD321x (traced on j416s only); nothing returns an empty port to S0 on remove or shutdown; 256 KiB elements were never asked of DCP 12.3.