From b329e7affcd4b31dbeb3d50a8cdc3e017ab542a8 Mon Sep 17 00:00:00 2001 From: nekolyanich Date: Sun, 11 Oct 2026 23:04:36 +0300 Subject: [PATCH 1/4] 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. Assisted-by: Claude:claude-opus-5-5 checkpatch Signed-off-by: nekolyanich --- drivers/gpu/drm/apple/afk.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/apple/afk.c b/drivers/gpu/drm/apple/afk.c index 0d598980c96e4..e1dbba048b89b 100644 --- a/drivers/gpu/drm/apple/afk.c +++ b/drivers/gpu/drm/apple/afk.c @@ -52,6 +52,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); @@ -605,6 +608,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; @@ -616,11 +620,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) { @@ -648,7 +662,7 @@ 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].retcode = retcode; if (service->cmds[idx].free_on_ack) { /* defer freeing until we're no longer in atomic context */ rxbuf = service->cmds[idx].rxbuf; From fb0c7845bc3e0f3b45f4b447532cf341fa816e18 Mon Sep 17 00:00:00 2001 From: nekolyanich Date: Sun, 11 Oct 2026 23:05:59 +0300 Subject: [PATCH 2/4] drm: apple: av: pin the AV services and let their AFK slots be reused DCP announces the AV endpoint's services (DCPAVSimpleVideoInterface and DCPAVAudioInterface) again on every display connect. They are not reusable, so each connect takes two of the endpoint's 16 slots for good: after eight connects in one boot the next display got "AFK[ep:29]: no reusable service slots" and no audio service until a reboot (M2 Pro, a TV on a USB-C to HDMI adapter, under the m1n1 hypervisor). Convert their owners the way commit 5837706b10e3 ("drm/apple: pin DP service owners before recycling retired AFK slots") did for the DP service, on which this builds. The audio service keeps an owner pin while asrv->srv points to it, released on its teardown or, after afk_shutdown() on unbind, in av_service_detach(). An announce without a teardown, not seen, warns and drops the older pin, and a late teardown of the older one then leaves the live one alone. The open and close in av_service_connect() and av_service_disconnect(), made without srv_rwsem, pin it for the call. The close no longer disables a torn down service itself: the teardown handler already clears enabled. The video service keeps no pointer to it. Then mark both reusable. Fixes: 02e1582e7d0e ("gpu: drm: apple: Set up client of AV endpoint") Assisted-by: Claude:claude-opus-5-5 checkpatch Signed-off-by: nekolyanich --- drivers/gpu/drm/apple/av.c | 74 +++++++++++++++++++++++++++++++------ drivers/gpu/drm/apple/av.h | 2 + drivers/gpu/drm/apple/dcp.c | 1 + 3 files changed, 65 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/apple/av.c b/drivers/gpu/drm/apple/av.c index 563a02bb4e551..b539fa4bc2046 100644 --- a/drivers/gpu/drm/apple/av.c +++ b/drivers/gpu/drm/apple/av.c @@ -96,12 +96,22 @@ static void av_audiosrv_init(struct apple_epic_service *service, const char *nam { struct apple_dcp *dcp = service->ep->dcp; struct audiosrv_data *asrv = dcp->audiosrv; + struct apple_epic_service *old; mutex_lock(&asrv->plug_lock); + /* + * An owner pin keeps the slot from being reused while srv points to + * it. DCP tears the service down before announcing it again. + */ down_write(&asrv->srv_rwsem); - asrv->srv = service; + old = asrv->srv; + asrv->srv = afk_service_get(service); up_write(&asrv->srv_rwsem); + if (old) { + dev_warn_once(dcp->dev, "audiosrv: announced again without a teardown\n"); + afk_service_put(old); + } asrv->plugged = true; mutex_unlock(&asrv->plug_lock); @@ -112,11 +122,20 @@ static void av_audiosrv_teardown(struct apple_epic_service *service) { struct apple_dcp *dcp = service->ep->dcp; struct audiosrv_data *asrv = dcp->audiosrv; + bool owned; mutex_lock(&asrv->plug_lock); down_write(&asrv->srv_rwsem); - asrv->srv = NULL; + owned = asrv->srv == service; + if (owned) { + asrv->srv = NULL; + } else if (asrv->srv) { + /* announced again without a teardown: the newer one is live */ + up_write(&asrv->srv_rwsem); + mutex_unlock(&asrv->plug_lock); + return; + } up_write(&asrv->srv_rwsem); asrv->plugged = false; @@ -124,6 +143,27 @@ static void av_audiosrv_teardown(struct apple_epic_service *service) dcpaud_disconnect(asrv->audio_dev); mutex_unlock(&asrv->plug_lock); + + if (owned) + afk_service_put(service); +} + +/* after afk_shutdown() on unbind: drop the owner pin */ +void av_service_detach(struct apple_dcp *dcp) +{ + struct audiosrv_data *asrv = dcp->audiosrv; + struct apple_epic_service *service; + + if (!asrv) + return; + scoped_guard(rwsem_write, &asrv->srv_rwsem) { + service = asrv->srv; + asrv->srv = NULL; + } + if (service) { + afk_service_disable(service); + afk_service_put(service); + } } int dcp_audiosrv_prepare(struct device *dev, struct dcp_sound_cookie *cookie) @@ -279,11 +319,15 @@ static int av_audiosrv_report(struct apple_epic_service *service, u32 idx, static const struct apple_epic_service_ops avep_ops[] = { { .name = "DCPAVSimpleVideoInterface", + /* keeps no pointer to the service */ + .reusable = true, .init = av_interface_init, .teardown = av_interface_teardown, }, { .name = "DCPAVAudioInterface", + /* pinned by asrv->srv, and by each call made without srv_rwsem */ + .reusable = true, .init = av_audiosrv_init, .report = av_audiosrv_report, .teardown = av_audiosrv_teardown, @@ -297,19 +341,23 @@ void av_service_connect(struct apple_dcp *dcp) struct audiosrv_data *asrv = dcp->audiosrv; int ret; - scoped_guard(rwsem_write, &asrv->srv_rwsem) { - if (!asrv->srv) + /* pinned for the open below, made without srv_rwsem */ + scoped_guard(rwsem_read, &asrv->srv_rwsem) { + service = afk_service_get(asrv->srv); + if (!service) return; - service = asrv->srv; } /* open AV audio service */ dev_info(dcp->dev, "%s: starting audio service, plugged:%d\n", __func__, asrv->plugged); - if (asrv->is_open) + if (asrv->is_open) { + afk_service_put(service); return; + } ret = afk_service_call(service, 0, asrv->cmds.open, NULL, 0, 32, NULL, 0, 32); + afk_service_put(service); if (ret) { dev_err(dcp->dev, "error opening audio service: %d\n", ret); return; @@ -328,16 +376,19 @@ void av_service_disconnect(struct apple_dcp *dcp) struct audiosrv_data *asrv = dcp->audiosrv; int ret; - scoped_guard(rwsem_write, &asrv->srv_rwsem) { - if (!asrv->srv) + /* pinned for the close below, made without srv_rwsem */ + scoped_guard(rwsem_read, &asrv->srv_rwsem) { + service = afk_service_get(asrv->srv); + if (!service) return; - service = asrv->srv; } /* close AV audio service */ dev_info(dcp->dev, "%s: stopping audio service\n", __func__); - if (!asrv->is_open) + if (!asrv->is_open) { + afk_service_put(service); return; + } mutex_lock(&asrv->plug_lock); @@ -351,8 +402,7 @@ void av_service_disconnect(struct apple_dcp *dcp) if (ret) { dev_err(dcp->dev, "error closing audio service: %d\n", ret); } - if (service->torndown) - afk_service_disable(service); + afk_service_put(service); asrv->is_open = false; } diff --git a/drivers/gpu/drm/apple/av.h b/drivers/gpu/drm/apple/av.h index c00cbef549fd2..66ccff2f9a4bc 100644 --- a/drivers/gpu/drm/apple/av.h +++ b/drivers/gpu/drm/apple/av.h @@ -9,9 +9,11 @@ #if IS_ENABLED(CONFIG_DRM_APPLE_AUDIO) void av_service_connect(struct apple_dcp *dcp); void av_service_disconnect(struct apple_dcp *dcp); +void av_service_detach(struct apple_dcp *dcp); #else static inline void av_service_connect(struct apple_dcp *dcp) { } static inline void av_service_disconnect(struct apple_dcp *dcp) { } +static inline void av_service_detach(struct apple_dcp *dcp) { } #endif #endif /* __AV_H__ */ diff --git a/drivers/gpu/drm/apple/dcp.c b/drivers/gpu/drm/apple/dcp.c index ca5988f81fe20..f8f6377ed0156 100644 --- a/drivers/gpu/drm/apple/dcp.c +++ b/drivers/gpu/drm/apple/dcp.c @@ -4245,6 +4245,7 @@ static void dcp_comp_unbind(struct device *dev, struct device *main, void *data) if (dcp->avep) { av_service_disconnect(dcp); afk_shutdown(dcp->avep); + av_service_detach(dcp); dcp->avep = NULL; } From 3e70137d2319e40c50dc1f3cb0bf5c33931686dc Mon Sep 17 00:00:00 2001 From: nekolyanich Date: Sun, 11 Oct 2026 23:06:33 +0300 Subject: [PATCH 3/4] 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 | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/apple/afk.c b/drivers/gpu/drm/apple/afk.c index e1dbba048b89b..66b1b48065abe 100644 --- a/drivers/gpu/drm/apple/afk.c +++ b/drivers/gpu/drm/apple/afk.c @@ -416,6 +416,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; @@ -494,6 +495,8 @@ 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]; + /* only this rx worker sets ops: no lock needed for the log */ + reused = !!candidate->ops; if (afk_service_reinit(candidate, ep, ops, channel)) { service = candidate; break; @@ -553,8 +556,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 1a6694d5fb56af4a49025d0c5f7e05afb29c2598 Mon Sep 17 00:00:00 2001 From: nekolyanich Date: Sun, 11 Oct 2026 23:06:51 +0300 Subject: [PATCH 4/4] 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. Assisted-by: Claude:claude-opus-5-5 checkpatch Signed-off-by: nekolyanich --- drivers/gpu/drm/apple/av.c | 35 +++++++++++++++++++++++++++-------- 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/apple/av.c b/drivers/gpu/drm/apple/av.c index b539fa4bc2046..7684c1dd5c624 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, @@ -107,6 +111,7 @@ static void av_audiosrv_init(struct apple_epic_service *service, const char *nam down_write(&asrv->srv_rwsem); old = asrv->srv; asrv->srv = afk_service_get(service); + asrv->gen++; up_write(&asrv->srv_rwsem); if (old) { dev_warn_once(dcp->dev, "audiosrv: announced again without a teardown\n"); @@ -339,6 +344,7 @@ void av_service_connect(struct apple_dcp *dcp) { struct apple_epic_service *service; struct audiosrv_data *asrv = dcp->audiosrv; + u64 gen; int ret; /* pinned for the open below, made without srv_rwsem */ @@ -346,11 +352,14 @@ void av_service_connect(struct apple_dcp *dcp) service = afk_service_get(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) { + mutex_unlock(&asrv->open_lock); afk_service_put(service); return; } @@ -359,21 +368,26 @@ void av_service_connect(struct apple_dcp *dcp) NULL, 0, 32); afk_service_put(service); if (ret) { + mutex_unlock(&asrv->open_lock); dev_err(dcp->dev, "error opening audio service: %d\n", ret); return; } 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); + mutex_unlock(&asrv->open_lock); } void av_service_disconnect(struct apple_dcp *dcp) { struct apple_epic_service *service; struct audiosrv_data *asrv = dcp->audiosrv; + u64 gen; int ret; /* pinned for the close below, made without srv_rwsem */ @@ -381,14 +395,18 @@ void av_service_disconnect(struct apple_dcp *dcp) service = afk_service_get(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) { + mutex_unlock(&asrv->open_lock); afk_service_put(service); return; } + asrv->open_gen = 0; mutex_lock(&asrv->plug_lock); @@ -403,7 +421,7 @@ void av_service_disconnect(struct apple_dcp *dcp) dev_err(dcp->dev, "error closing audio service: %d\n", ret); } afk_service_put(service); - asrv->is_open = false; + mutex_unlock(&asrv->open_lock); } static void av_work_service_start(struct work_struct *work) @@ -438,6 +456,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: