diff --git a/drivers/gpu/drm/apple/afk.c b/drivers/gpu/drm/apple/afk.c index 0d598980c96e4..66b1b48065abe 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); @@ -413,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; @@ -491,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; @@ -550,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); @@ -605,6 +612,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 +624,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 +666,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; diff --git a/drivers/gpu/drm/apple/av.c b/drivers/gpu/drm/apple/av.c index 563a02bb4e551..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, @@ -96,12 +100,23 @@ 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); + asrv->gen++; 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 +127,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 +148,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 +324,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, @@ -295,49 +344,69 @@ 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) { - 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; + 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; + } ret = afk_service_call(service, 0, asrv->cmds.open, NULL, 0, 32, 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; - 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; + 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); @@ -351,9 +420,8 @@ 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); - asrv->is_open = false; + afk_service_put(service); + mutex_unlock(&asrv->open_lock); } static void av_work_service_start(struct work_struct *work) @@ -388,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: 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; }