From cb6416ce0a83e64508b3d0724b155ea44e782a92 Mon Sep 17 00:00:00 2001 From: nekolyanich Date: Sun, 11 Oct 2026 23:04:36 +0300 Subject: [PATCH 1/6] drm: apple: afk: complete a command whose reply is too short as failed 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 107036c9c167 ("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 b12111f4d48f ("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 --- drivers/gpu/drm/apple/afk.c | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/apple/afk.c b/drivers/gpu/drm/apple/afk.c index 5daa26990a02b..751311d05ca47 100644 --- a/drivers/gpu/drm/apple/afk.c +++ b/drivers/gpu/drm/apple/afk.c @@ -58,6 +58,9 @@ enum rbep_msg_type { #define SEND_WPTR GENMASK(31, 0) +/* retcode of a reply too short to carry a nonzero IOReturn */ +#define AFK_RETCODE_SHORT_REPLY 0xffffffff + static void afk_service_worker(struct work_struct *work); static void afk_discard_pending_open(struct apple_epic_service *service); @@ -676,6 +679,7 @@ static void afk_recv_handle_reply(struct apple_dcp_afkep *ep, u32 channel, struct apple_epic_service *service; unsigned long flags; u8 idx = tag & (ep->wire_v2 ? 0xf : 0xff); + u32 retcode; void *rxbuf, *txbuf; dma_addr_t rxbuf_dma, txbuf_dma; size_t rxlen, txlen; @@ -687,11 +691,21 @@ static void afk_recv_handle_reply(struct apple_dcp_afkep *ep, u32 channel, return; } + /* + * A reply shorter than the command header completes the command as + * failed, so that its caller returns and a timed-out command's + * buffers and cmd_map bit are freed. The 4-byte replies seen look + * like an IOReturn: a nonzero one is passed on. + */ if (payload_size < sizeof(*cmd)) { dev_err(ep->dcp->dev, "AFK[ep:%02x]: command reply on channel %d too small: %ld\n", ep->endpoint, channel, payload_size); - return; + retcode = payload_size >= sizeof(cmd->retcode) ? + le32_to_cpu(cmd->retcode) : 0; + retcode = retcode ?: AFK_RETCODE_SHORT_REPLY; + } else { + retcode = le32_to_cpu(cmd->retcode); } if (idx >= MAX_PENDING_CMDS) { @@ -719,9 +733,12 @@ static void afk_recv_handle_reply(struct apple_dcp_afkep *ep, u32 channel, } service->cmds[idx].done = true; - service->cmds[idx].retcode = le32_to_cpu(cmd->retcode); - service->cmds[idx].reply_len = le32_to_cpu(cmd->rxlen); - service->cmds[idx].reply_len_valid = true; + service->cmds[idx].retcode = retcode; + /* a short reply has no rxlen: its length stays invalid */ + if (payload_size >= sizeof(*cmd)) { + service->cmds[idx].reply_len = le32_to_cpu(cmd->rxlen); + service->cmds[idx].reply_len_valid = true; + } if (service->cmds[idx].free_on_ack) { /* defer freeing until we're no longer in atomic context */ rxbuf = service->cmds[idx].rxbuf; @@ -1250,12 +1267,15 @@ static void afk_recv_handle(struct apple_dcp_afkep *ep, u32 channel, u32 type, * 14.7 returns a command result as a NOTIFY whose category is REPLY * and whose body is a normal epic_cmd. The tag check inside the * reply handler ignores anything that is not one of our commands. + * A short reply from 14.7 goes to the std service handler; one from + * any other firmware to the reply handler, which completes it as failed. */ if ((type == EPIC_TYPE_REPLY || (type == EPIC_TYPE_NOTIFY && dcp_fw_is_14_7(ep->dcp))) && eshdr->category == EPIC_CAT_REPLY && - payload_size >= sizeof(struct epic_cmd)) + (payload_size >= sizeof(struct epic_cmd) || + !dcp_fw_is_14_7(ep->dcp))) return afk_recv_handle_reply(ep, channel, le16_to_cpu(eshdr->tag), payload, payload_size); From be2f84c83228b9ac920adfca9586b2bc44741e1e Mon Sep 17 00:00:00 2001 From: nekolyanich Date: Sun, 11 Oct 2026 23:06:33 +0300 Subject: [PATCH 2/6] drm: apple: afk: log which slot a service gets 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 --- drivers/gpu/drm/apple/afk.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/apple/afk.c b/drivers/gpu/drm/apple/afk.c index 751311d05ca47..d69067c051f8a 100644 --- a/drivers/gpu/drm/apple/afk.c +++ b/drivers/gpu/drm/apple/afk.c @@ -478,6 +478,7 @@ static void afk_recv_handle_init(struct apple_dcp_afkep *ep, u32 channel, struct apple_epic_service *service = NULL, *previous; unsigned long flags; bool retired = false; + bool reused = false; u32 ch_idx; const char *service_name = name; const char *epic_name = NULL, *epic_class = NULL; @@ -563,6 +564,11 @@ static void afk_recv_handle_init(struct apple_dcp_afkep *ep, u32 channel, for (ch_idx = 0; ch_idx < AFK_MAX_CHANNEL; ch_idx++) { struct apple_epic_service *candidate = &ep->services[ch_idx]; + /* + * ops change only on ep->wq, where this runs, or once it is + * gone: no lock needed for the log + */ + reused = !!candidate->ops; if (afk_service_reinit(candidate, ep, ops, channel)) { service = candidate; break; @@ -622,8 +628,9 @@ static void afk_recv_handle_init(struct apple_dcp_afkep *ep, u32 channel, afk_service_publish(service); if (ops->init) ops->init(service, epic_name, epic_class, epic_unit); - dev_info(ep->dcp->dev, "AFK[ep:%02x]: new service %s on channel %d\n", - ep->endpoint, service_name, channel); + dev_info(ep->dcp->dev, "AFK[ep:%02x]: new service %s on channel %d, slot %u%s\n", + ep->endpoint, service_name, channel, ch_idx, + reused ? " (reused)" : ""); afk_populate_service_debugfs(service); From afc8186075d6d8a3cbfe8f78c982b618b48a87ed Mon Sep 17 00:00:00 2001 From: nekolyanich Date: Sun, 11 Oct 2026 23:06:51 +0300 Subject: [PATCH 3/6] drm: apple: av: open each generation of the audio service 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 d0153933646a ("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 --- drivers/gpu/drm/apple/av.c | 37 ++++++++++++++++++++++++++++--------- 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/apple/av.c b/drivers/gpu/drm/apple/av.c index f79fbd8e32614..e3d87ee2490af 100644 --- a/drivers/gpu/drm/apple/av.c +++ b/drivers/gpu/drm/apple/av.c @@ -67,7 +67,11 @@ struct audiosrv_data { bool warned_get_elements; bool warned_get_product_attrs; - bool is_open; + /* generation of srv, and the one that was opened (0: none) */ + u64 gen; + u64 open_gen; + /* serialises the open and the close; never taken on the AFK rx worker */ + struct mutex open_lock; }; static void av_interface_init(struct apple_epic_service *service, const char *name, @@ -108,6 +112,7 @@ static void av_audiosrv_init(struct apple_epic_service *service, const char *nam down_write(&asrv->srv_rwsem); old = asrv->srv; asrv->srv = service; + asrv->gen++; up_write(&asrv->srv_rwsem); if (old) afk_service_put(old); @@ -127,8 +132,11 @@ static void av_audiosrv_teardown(struct apple_epic_service *service) down_write(&asrv->srv_rwsem); owned = asrv->srv == service; - if (owned) + if (owned) { asrv->srv = NULL; + /* an open in flight must not count as this one's */ + asrv->gen++; + } up_write(&asrv->srv_rwsem); /* A stale generation must not unplug its successor. */ @@ -315,17 +323,20 @@ void av_service_connect(struct apple_dcp *dcp) { struct apple_epic_service *service; struct audiosrv_data *asrv = dcp->audiosrv; + u64 gen; int ret; scoped_guard(rwsem_write, &asrv->srv_rwsem) { service = afk_service_pin(asrv->srv); if (!service) return; + gen = asrv->gen; } /* open AV audio service */ dev_info(dcp->dev, "%s: starting audio service, plugged:%d\n", __func__, asrv->plugged); - if (asrv->is_open) + mutex_lock(&asrv->open_lock); + if (asrv->open_gen == gen) goto out; ret = afk_service_call(service, 0, asrv->cmds.open, NULL, 0, 32, @@ -335,12 +346,15 @@ void av_service_connect(struct apple_dcp *dcp) goto out; } mutex_lock(&asrv->plug_lock); - asrv->is_open = true; - - if (asrv->audio_dev) - dcpaud_connect(asrv->audio_dev, asrv->plugged); + /* torn down during the open: the next generation is opened afresh */ + if (asrv->gen == gen) { + asrv->open_gen = gen; + if (asrv->audio_dev) + dcpaud_connect(asrv->audio_dev, asrv->plugged); + } mutex_unlock(&asrv->plug_lock); out: + mutex_unlock(&asrv->open_lock); afk_service_put(service); } @@ -348,18 +362,22 @@ void av_service_disconnect(struct apple_dcp *dcp) { struct apple_epic_service *service; struct audiosrv_data *asrv = dcp->audiosrv; + u64 gen; int ret; scoped_guard(rwsem_write, &asrv->srv_rwsem) { service = afk_service_pin(asrv->srv); if (!service) return; + gen = asrv->gen; } /* close AV audio service */ dev_info(dcp->dev, "%s: stopping audio service\n", __func__); - if (!asrv->is_open) + mutex_lock(&asrv->open_lock); + if (asrv->open_gen != gen) goto out; + asrv->open_gen = 0; mutex_lock(&asrv->plug_lock); @@ -375,8 +393,8 @@ void av_service_disconnect(struct apple_dcp *dcp) } if (service->torndown) afk_service_disable(service); - asrv->is_open = false; out: + mutex_unlock(&asrv->open_lock); afk_service_put(service); } @@ -412,6 +430,7 @@ int avep_init(struct apple_dcp *dcp) return -ENOMEM; init_rwsem(&audiosrv_data->srv_rwsem); mutex_init(&audiosrv_data->plug_lock); + mutex_init(&audiosrv_data->open_lock); switch (dcp->fw_compat) { case DCP_FIRMWARE_V_12_3: From 9be0906dde81612cb6585d86488c3a84f73d7488 Mon Sep 17 00:00:00 2001 From: nekolyanich Date: Sun, 11 Oct 2026 18:01:28 +0300 Subject: [PATCH 4/6] drm: apple: audio: ask for up to 256 KiB of audio elements, as macOS 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: cb24343242a7 ("gpu: drm: apple: Add DCP audio driver") Assisted-by: Claude:claude-opus-5-5 checkpatch Signed-off-by: nekolyanich --- drivers/gpu/drm/apple/afk.c | 4 ++-- drivers/gpu/drm/apple/audio.c | 21 ++++++++++++++++++--- drivers/gpu/drm/apple/av.c | 4 ++-- 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/apple/afk.c b/drivers/gpu/drm/apple/afk.c index d69067c051f8a..09e56e4971139 100644 --- a/drivers/gpu/drm/apple/afk.c +++ b/drivers/gpu/drm/apple/afk.c @@ -2022,7 +2022,7 @@ static int afk_service_call_full(struct apple_epic_service *service, u16 group, &bfr_len) || bfr_len > U32_MAX) return -EOVERFLOW; - bfr = kzalloc(bfr_len, GFP_KERNEL); + bfr = kvzalloc(bfr_len, GFP_KERNEL); if (!bfr) return -ENOMEM; @@ -2102,7 +2102,7 @@ static int afk_service_call_full(struct apple_epic_service *service, u16 group, } out: - kfree(bfr); + kvfree(bfr); return ret; } diff --git a/drivers/gpu/drm/apple/audio.c b/drivers/gpu/drm/apple/audio.c index cfdfd606cf129..ed6cf9a1e168e 100644 --- a/drivers/gpu/drm/apple/audio.c +++ b/drivers/gpu/drm/apple/audio.c @@ -19,6 +19,8 @@ #include #include #include +#include +#include #include #include #include @@ -31,7 +33,11 @@ #include "audio.h" #include "parser.h" -#define DCPAUD_ELEMENTS_MAXSIZE 16384 +/* + * As macOS 13 asks for. Elements that do not fit come back cut off at the + * buffer's end, with a used size of 0. + */ +#define DCPAUD_ELEMENTS_MAXSIZE SZ_256K #define DCPAUD_PRODUCTATTRS_MAXSIZE 1024 /* @@ -842,18 +848,27 @@ static const struct component_ops dcpaud_comp_ops = { .unbind = dcpaud_comp_unbind, }; +static void dcpaud_free_elements(void *elements) +{ + kvfree(elements); +} + static int dcpaud_probe(struct platform_device *pdev) { struct dcp_audio *dcpaud; + int ret; dcpaud = devm_kzalloc(&pdev->dev, sizeof(*dcpaud), GFP_KERNEL); if (!dcpaud) return -ENOMEM; - dcpaud->elements = devm_kzalloc(&pdev->dev, DCPAUD_ELEMENTS_MAXSIZE, - GFP_KERNEL); + dcpaud->elements = kvzalloc(DCPAUD_ELEMENTS_MAXSIZE, GFP_KERNEL); if (!dcpaud->elements) return -ENOMEM; + ret = devm_add_action_or_reset(&pdev->dev, dcpaud_free_elements, + dcpaud->elements); + if (ret) + return ret; dcpaud->productattrs = devm_kzalloc(&pdev->dev, DCPAUD_PRODUCTATTRS_MAXSIZE, GFP_KERNEL); diff --git a/drivers/gpu/drm/apple/av.c b/drivers/gpu/drm/apple/av.c index e3d87ee2490af..83edabb932b77 100644 --- a/drivers/gpu/drm/apple/av.c +++ b/drivers/gpu/drm/apple/av.c @@ -223,10 +223,10 @@ dcp_audiosrv_osobject_call(struct apple_epic_service *service, u16 group, } __attribute__((packed)) *hdr; static_assert(sizeof(*hdr) == 48); size_t bfr_len = output_maxsize + sizeof(*hdr); - void *bfr __free(kfree) = NULL; + void *bfr __free(kvfree) = NULL; int ret; - bfr = kzalloc(bfr_len, GFP_KERNEL); + bfr = kvzalloc(bfr_len, GFP_KERNEL); if (!bfr) return -ENOMEM; From e58d7a0116cbb95f4c31936ef4014907eb6531a4 Mon Sep 17 00:00:00 2001 From: nekolyanich Date: Sun, 11 Oct 2026 18:01:42 +0300 Subject: [PATCH 5/6] drm: apple: audio: drop the runtime PM reference only after the DMA stopped 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 cf7ddf4de16c ("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: f25cb501c5fc ("drm: apple: audio: Implement runtime PM support") Assisted-by: Claude:claude-opus-5-5 checkpatch Signed-off-by: nekolyanich --- drivers/gpu/drm/apple/audio.c | 32 +++++++++++++++++++++++++++----- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/apple/audio.c b/drivers/gpu/drm/apple/audio.c index ed6cf9a1e168e..aa0fb24e82fd3 100644 --- a/drivers/gpu/drm/apple/audio.c +++ b/drivers/gpu/drm/apple/audio.c @@ -71,6 +71,8 @@ struct dcp_audio { ktime_t void_start; u64 void_periods; struct delayed_work void_work; + /* the stream holds a runtime PM reference, put in sync_stop */ + bool pm_held; void *elements; void *productattrs; @@ -510,7 +512,10 @@ static int dcp_pcm_trigger(struct snd_pcm_substream *substream, int cmd) if (dcpaud->open_unplugged || !dcpaud_connection_up(dcpaud)) return dcpaud_void_start(substream); - WARN_ON(pm_runtime_get_sync(dcpaud->dev) < 0); + if (!dcpaud->pm_held) { + WARN_ON(pm_runtime_get_sync(dcpaud->dev) < 0); + dcpaud->pm_held = true; + } ret = dcp_audiosrv_startlink(dcpaud->dcp_dev, &dcpaud->selected_cookie); if (ret < 0) @@ -541,9 +546,8 @@ static int dcp_pcm_trigger(struct snd_pcm_substream *substream, int cmd) case SNDRV_PCM_TRIGGER_STOP: case SNDRV_PCM_TRIGGER_SUSPEND: + /* the runtime PM reference goes in sync_stop */ ret = dcp_audiosrv_stoplink(dcpaud->dcp_dev); - pm_runtime_mark_last_busy(dcpaud->dev); - __pm_runtime_put_autosuspend(dcpaud->dev); if (ret < 0) return ret; break; @@ -553,14 +557,32 @@ static int dcp_pcm_trigger(struct snd_pcm_substream *substream, int cmd) } /* - * ALSA calls this once a stream has stopped, outside the stream lock, before - * the next hw_params, prepare or hw_free and at close. + * ALSA calls this once a stream has stopped (STOP, SUSPEND, or the STOP + * that undoes a failed START), before the next hw_params, prepare or + * hw_free, at close, at system suspend and at disconnect, outside the + * stream lock. The DMA terminate is asynchronous: putting the runtime PM + * reference in trigger STOP powered the DPA's domain (ps_dpa*) down while + * SIO still ran DMA towards it, and SIO crashed ("Shim DMA unavailable", + * the TERMINATE timing out after it). Wait for the terminate first. */ static int dcp_pcm_sync_stop(struct snd_pcm_substream *substream) { struct dcp_audio *dcpaud = substream->pcm->private_data; + bool held; cancel_delayed_work_sync(&dcpaud->void_work); + snd_dmaengine_pcm_sync_stop(substream); + + /* START sets it under the stream lock */ + snd_pcm_stream_lock_irq(substream); + held = dcpaud->pm_held; + dcpaud->pm_held = false; + snd_pcm_stream_unlock_irq(substream); + + if (held) { + pm_runtime_mark_last_busy(dcpaud->dev); + __pm_runtime_put_autosuspend(dcpaud->dev); + } return 0; } From c52233c8d4e046b1665fd4937510be3067fff63a Mon Sep 17 00:00:00 2001 From: nekolyanich Date: Tue, 13 Oct 2026 08:06:46 +0300 Subject: [PATCH 6/6] usb: typec: tipd: put an empty CD321x port to sleep, as macOS 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 72911ae266cd ("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 --- drivers/usb/typec/tipd/core.c | 92 ++++++++++++++++++++++++++++++- drivers/usb/typec/tipd/tps6598x.h | 15 +++++ 2 files changed, 106 insertions(+), 1 deletion(-) diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c index 6c90eb3221adf..6c179f4479e4b 100644 --- a/drivers/usb/typec/tipd/core.c +++ b/drivers/usb/typec/tipd/core.c @@ -53,6 +53,15 @@ #define TPS_REG_DATA_STATUS 0x5f #define TPS_REG_SLEEP_CONF 0x70 +/* + * CD321x register 0x50, for which we have no documentation: 6 bytes, of + * which macOS reads and writes 4. It writes BIT(13) after each DATA_STATUS + * HPD_IRQ and around a disconnect, then clears it; what the bit does is not + * known. + */ +#define CD321X_REG_0X50 0x50 +#define CD321X_0X50_BIT13 BIT(13) + /* TPS_REG_SYSTEM_CONF bits */ #define TPS_SYSCONF_PORTINFO(c) ((c) & 7) @@ -847,6 +856,76 @@ static void cd321x_retry_revalidation(struct cd321x *cd321x, bool reconnect) CD321X_RESUME_ATTEMPTS); } +static void cd321x_write_0x50(struct tps6598x *tps, u32 val) +{ + __le32 v = cpu_to_le32(val); + int ret; + + ret = tps6598x_block_write(tps, CD321X_REG_0X50, &v, sizeof(v)); + if (ret) + dev_warn(tps->dev, "failed to write 0x%x to register 0x50: %d\n", + val, ret); +} + +/* + * After a session in DP alt mode, the controller reports the cable power + * path "in" and the session's data status once the partner is gone, and + * then gives the next partner no VBUS. macOS puts the port into system + * power state 2 on disconnect (and its empty MagSafe controller at boot) + * and back into S0 on the next connect; do the same, here also for a + * USB-C port found empty at probe. State 2 was never seen entered with a + * plug present, so STATUS is read again first and a plug skips the sleep. + * switch_power_state() reads the state before and after SSPS (macOS only + * after) and skips a state already set. A failure only warns, and leaves + * the port state unknown so that the next pass tries again: the cable + * state above is already applied, and a later event still reaches the port. + */ +static void cd321x_port_sleep(struct cd321x *cd321x) +{ + struct tps6598x *tps = &cd321x->tps; + u32 status; + int ret; + + /* a controller found in "DBMa" is left as it is, see probe */ + if (!tps->data->port_sleep || tps->dbma_at_probe) + return; + + ret = tps6598x_read32(tps, TPS_REG_STATUS, &status); + if (ret || (status & TPS_STATUS_PLUG_PRESENT)) { + cd321x->port_power = CD321X_PORT_UNKNOWN; + return; + } + + cd321x_write_0x50(tps, CD321X_0X50_BIT13); + ret = tps->data->switch_power_state(tps, CD321X_SYSTEM_POWER_STATE_UNPLUGGED); + if (ret) { + dev_warn(tps->dev, "failed to enter the unplugged power state: %d\n", + ret); + cd321x->port_power = CD321X_PORT_UNKNOWN; + } else { + cd321x->port_power = CD321X_PORT_ASLEEP; + } + cd321x_write_0x50(tps, CD321X_0X50_BIT13); + cd321x_write_0x50(tps, 0); +} + +static void cd321x_port_wake(struct cd321x *cd321x) +{ + struct tps6598x *tps = &cd321x->tps; + int ret; + + if (!tps->data->port_sleep || tps->dbma_at_probe) + return; + + ret = tps->data->switch_power_state(tps, TPS_SYSTEM_POWER_STATE_S0); + if (ret) { + dev_warn(tps->dev, "failed to return to S0 on connect: %d\n", ret); + cd321x->port_power = CD321X_PORT_UNKNOWN; + } else { + cd321x->port_power = CD321X_PORT_AWAKE; + } +} + static void cd321x_update_work(struct work_struct *work) { struct cd321x *cd321x = container_of(to_delayed_work(work), @@ -1006,6 +1085,9 @@ static void cd321x_update_work(struct work_struct *work) /* If the plug is disconnected, we are done */ if (!new_connected) { + /* also an empty port at probe, and a disconnect retried */ + if (cd321x->port_power != CD321X_PORT_ASLEEP) + cd321x_port_sleep(cd321x); cd321x_pm_complete(&cd321x->pm); return; } @@ -1014,6 +1096,8 @@ static void cd321x_update_work(struct work_struct *work) if (!tps->partner) { struct typec_partner_desc desc; + cd321x_port_wake(cd321x); + desc.usb_pd = is_pd; desc.accessory = TYPEC_ACCESSORY_NONE; /* XXX: handle accessories */ desc.identity = NULL; @@ -1213,6 +1297,8 @@ static void cd321x_resume_reverify(struct tps6598x *tps) struct cd321x *cd321x = container_of(tps, struct cd321x, tps); guard(mutex)(&tps->lock); + /* the controller may have been re-initialised over sleep */ + cd321x->port_power = CD321X_PORT_UNKNOWN; if (cd321x_pm_resume(&cd321x->pm)) mod_delayed_work(system_freezable_wq, &cd321x->resume_work, 0); } @@ -1276,8 +1362,11 @@ static int cd321x_check_setup(struct cd321x *cd321x, u64 event) dev_err(tps->dev, "controller setup recovery exhausted: %d\n", ret); return ret; } - if (ret) + if (ret) { dev_warn(tps->dev, "restored controller power state and interrupt mask\n"); + /* back in S0, also a port put to sleep before */ + cd321x->port_power = CD321X_PORT_UNKNOWN; + } /* Only a fresh snapshot may be applied after restoring the controller. */ cd321x->state_valid = false; cd321x->pm.phase = CD321X_PM_REVALIDATE; @@ -2544,6 +2633,7 @@ const struct tipd_data tipd_cd321x_data = { .read_data_status = cd321x_read_data_status, .reset = cd321x_reset, .switch_power_state = cd321x_switch_power_state, + .port_sleep = true, .connect = cd321x_connect, .suspend_prepare = cd321x_suspend_prepare, .resume_reverify = cd321x_resume_reverify, diff --git a/drivers/usb/typec/tipd/tps6598x.h b/drivers/usb/typec/tipd/tps6598x.h index de74a55c3f8b1..c420de3b618f5 100644 --- a/drivers/usb/typec/tipd/tps6598x.h +++ b/drivers/usb/typec/tipd/tps6598x.h @@ -158,6 +158,8 @@ #define TPS_SYSTEM_POWER_STATE_S3 0x03 #define TPS_SYSTEM_POWER_STATE_S4 0x04 #define TPS_SYSTEM_POWER_STATE_S5 0x05 +/* CD321x: what macOS sets on a port with nothing plugged in */ +#define CD321X_SYSTEM_POWER_STATE_UNPLUGGED 0x02 /* TPS_REG_POWER_STATUS bits */ #define TPS_POWER_STATUS_CONNECTION(x) TPS_FIELD_GET(BIT(0), (x)) @@ -313,6 +315,11 @@ struct tipd_data { int (*apply_patch)(struct tps6598x *tps); int (*init)(struct tps6598x *tps); int (*switch_power_state)(struct tps6598x *tps, u8 target_state); + /* + * CD321x: sleep an empty port and wake it on connect, as macOS does + * (traced on CD321x only) + */ + bool port_sleep; bool (*read_data_status)(struct tps6598x *tps); int (*reset)(struct tps6598x *tps); int (*connect)(struct tps6598x *tps, u32 status); @@ -397,8 +404,16 @@ struct cd321x_status { bool dp_sid_irq; }; +/* CD321x port power, as last set by the driver (UNKNOWN after probe) */ +enum cd321x_port_power { + CD321X_PORT_UNKNOWN, + CD321X_PORT_AWAKE, + CD321X_PORT_ASLEEP, +}; + struct cd321x { struct tps6598x tps; + enum cd321x_port_power port_power; struct tps6598x_dp_sid_status_reg dp_sid_status; struct tps6598x_intel_vid_status_reg intel_vid_status;