From c92eb3943810d746ac3f97d0ad81f4eee4974ad1 Mon Sep 17 00:00:00 2001 From: Eryk Wieliczko Date: Tue, 27 Oct 2026 03:33:53 +0100 Subject: [PATCH 1/9] drm/apple: skip AFK debugfs setup for unrouted connectors Type-C connectors have no DCP until a pipeline is routed. With DRM_APPLE_DEBUG enabled, registering one dereferences the null platform device. Skip the pipeline debug directory until a DCP is available. Fixes: 2314090e59f0 ("drm: apple: route Type-C displays through a shared DCP fabric") Signed-off-by: Eryk Wieliczko Assisted-by: Claude:claude-opus-5-5 --- drivers/gpu/drm/apple/connector.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/apple/connector.c b/drivers/gpu/drm/apple/connector.c index 8b4e225fbe8c9d..5918084483f6b8 100644 --- a/drivers/gpu/drm/apple/connector.c +++ b/drivers/gpu/drm/apple/connector.c @@ -116,7 +116,14 @@ static void dcp_afk_debugfs_root(struct platform_device *pdev, int ep, struct de { #if IS_ENABLED(CONFIG_DRM_APPLE_DEBUG) struct dentry *entry = NULL; - struct apple_dcp *dcp = platform_get_drvdata(pdev); + struct apple_dcp *dcp; + + /* Type-C connectors can be registered before a pipeline is routed. */ + if (!pdev) + return; + dcp = platform_get_drvdata(pdev); + if (!dcp) + return; switch (ep) { case AV_ENDPOINT: @@ -147,7 +154,7 @@ void apple_connector_debugfs_init(struct drm_connector *connector, struct dentry switch (connector->connector_type) { case DRM_MODE_CONNECTOR_DisplayPort: case DRM_MODE_CONNECTOR_HDMIA: - dcp_afk_debugfs_root(apple_con->dcp, AV_ENDPOINT, root); + dcp_afk_debugfs_root(READ_ONCE(apple_con->dcp), AV_ENDPOINT, root); break; default: break; From c806e16bbdd3e148034dca3e854226b77bb1cc9a Mon Sep 17 00:00:00 2001 From: Eryk Wieliczko Date: Tue, 27 Oct 2026 16:39:52 +0100 Subject: [PATCH 2/9] drm/apple: reject stale native audio prepares Check the published instance before Prepare and after its reply. Record the link owner only after the firmware accepts it. Fixes: f627d7748df5 ("drm/apple: av: carry a Terminate's instance through its whole disconnect") Signed-off-by: Eryk Wieliczko Assisted-by: Claude:claude-opus-5-5 --- drivers/gpu/drm/apple/av-v14-test.c | 73 ++++++++++++++++++++++++++++- drivers/gpu/drm/apple/av.c | 33 +++++++++---- 2 files changed, 96 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/apple/av-v14-test.c b/drivers/gpu/drm/apple/av-v14-test.c index 8d78113fb8f173..e1d13ab00c3ec2 100644 --- a/drivers/gpu/drm/apple/av-v14-test.c +++ b/drivers/gpu/drm/apple/av-v14-test.c @@ -27,9 +27,12 @@ struct av_v14_test { u64 release_gen; /* Within a link release: the old PCM closes, a successor is published and opened */ bool successor_during_release; + bool replace_during_prepare; + u32 prepare_status; }; static struct av_v14_test *av_v14_current; +static void av_v14_replace(struct av_v14_test *t); static int av_v14_call(struct apple_epic_service *srv, u32 command, const void *body, size_t body_len, size_t body_pad, void *reply, size_t reply_len) @@ -45,6 +48,13 @@ static int av_v14_call(struct apple_epic_service *srv, u32 command, const void * /* Open: status 0 at +0, opened at +16 */ if (command == t->asrv->cmds.open && reply_len >= 24) put_unaligned_le64(1, reply + 16); + if (command == t->asrv->cmds.prepare && reply_len >= 52) { + put_unaligned_le32(t->prepare_status, reply + 48); + if (t->replace_during_prepare) { + t->replace_during_prepare = false; + av_v14_replace(t); + } + } return 0; } @@ -53,8 +63,6 @@ static void av_v14_card_disconnect(struct platform_device *pdev) av_v14_current->disconnects++; } -static void av_v14_replace(struct av_v14_test *t); - static void av_v14_release_link(struct platform_device *pdev, const u64 *gen) { struct av_v14_test *t = av_v14_current; @@ -310,6 +318,63 @@ static void av_v14_link_instance(struct kunit *test) KUNIT_EXPECT_EQ(test, t->ncalls, 5U); } +static void av_v14_prepare_rejects_unpublished_open(struct kunit *test) +{ + struct av_v14_test *t = av_v14_setup(test); + struct dcp_sound_cookie cookie = {}; + + KUNIT_ASSERT_EQ(test, dcp_audiosrv_open(t->dcp->dev), 0); + av_v14_replace(t); + /* Same slot and pointer, but the PCM still owns the old instance. */ + KUNIT_EXPECT_EQ(test, dcp_audiosrv_link(t->dcp->dev, DCP_AUDIO_LINK_PREPARE, &cookie), + -ENODEV); + KUNIT_EXPECT_EQ(test, t->ncalls, 1U); + KUNIT_EXPECT_EQ(test, t->asrv->link_gen, 0ULL); +} + +static void av_v14_prepare_replaced_during_reply(struct kunit *test) +{ + struct av_v14_test *t = av_v14_setup(test); + struct dcp_sound_cookie cookie = {}; + + KUNIT_ASSERT_EQ(test, dcp_audiosrv_open(t->dcp->dev), 0); + t->replace_during_prepare = true; + KUNIT_EXPECT_EQ(test, dcp_audiosrv_link(t->dcp->dev, DCP_AUDIO_LINK_PREPARE, &cookie), + -ENODEV); + KUNIT_EXPECT_EQ(test, t->ncalls, 2U); + KUNIT_EXPECT_EQ(test, t->asrv->link_gen, 0ULL); +} + +static void av_v14_failed_prepare_does_not_own_link(struct kunit *test) +{ + struct av_v14_test *t = av_v14_setup(test); + struct dcp_sound_cookie cookie = {}; + + KUNIT_ASSERT_EQ(test, dcp_audiosrv_open(t->dcp->dev), 0); + t->prepare_status = 0xe00002ea; + KUNIT_EXPECT_EQ(test, dcp_audiosrv_link(t->dcp->dev, DCP_AUDIO_LINK_PREPARE, &cookie), + -EIO); + KUNIT_EXPECT_EQ(test, t->ncalls, 2U); + KUNIT_EXPECT_EQ(test, t->asrv->link_gen, 0ULL); +} + +static void av_v14_failed_reprepare_drops_owner(struct kunit *test) +{ + struct av_v14_test *t = av_v14_setup(test); + struct dcp_sound_cookie cookie = {}; + u64 gen; + + KUNIT_ASSERT_EQ(test, dcp_audiosrv_open(t->dcp->dev), 0); + gen = t->asrv->gen; + KUNIT_ASSERT_EQ(test, dcp_audiosrv_link(t->dcp->dev, DCP_AUDIO_LINK_PREPARE, &cookie), 0); + KUNIT_ASSERT_EQ(test, dcp_audiosrv_link(t->dcp->dev, DCP_AUDIO_LINK_COMPLETE, &cookie), 0); + t->prepare_status = 0xe00002ea; + KUNIT_EXPECT_EQ(test, dcp_audiosrv_link(t->dcp->dev, DCP_AUDIO_LINK_PREPARE, &cookie), + -EIO); + KUNIT_EXPECT_FALSE(test, dcp_audiosrv_link_on(t->dcp->dev, gen)); + KUNIT_EXPECT_EQ(test, t->asrv->link_gen, 0ULL); +} + /* A Terminate from an instance that is not the published one does nothing. */ static void av_v14_terminate_unpublished(struct kunit *test) { @@ -333,6 +398,10 @@ static struct kunit_case av_v14_cases[] = { KUNIT_CASE(av_v14_successor_during_release), KUNIT_CASE(av_v14_terminate_after_successor), KUNIT_CASE(av_v14_link_instance), + KUNIT_CASE(av_v14_prepare_rejects_unpublished_open), + KUNIT_CASE(av_v14_prepare_replaced_during_reply), + KUNIT_CASE(av_v14_failed_prepare_does_not_own_link), + KUNIT_CASE(av_v14_failed_reprepare_drops_owner), KUNIT_CASE(av_v14_terminate_unpublished), {} }; diff --git a/drivers/gpu/drm/apple/av.c b/drivers/gpu/drm/apple/av.c index 461d162e2c6e92..f34b32566adc4e 100644 --- a/drivers/gpu/drm/apple/av.c +++ b/drivers/gpu/drm/apple/av.c @@ -377,6 +377,15 @@ bool dcp_audiosrv_link_on(struct device *dev, u64 gen) return asrv->open_srv && asrv->open_gen == gen && asrv->link_gen == gen; } +/* open_lock pins the client; the endpoint can replace its published instance. */ +static bool av_audiosrv_open_published(struct audiosrv_data *asrv) +{ + lockdep_assert_held(&asrv->open_lock); + guard(rwsem_read)(&asrv->srv_rwsem); + return asrv->open_srv && asrv->srv == asrv->open_srv && + asrv->gen == asrv->open_gen; +} + /* * The link commands of the 14.7 external role's audio service, the native * audio driver's: prepare, start, stop and complete all take the selected @@ -418,20 +427,28 @@ int dcp_audiosrv_link(struct device *dev, enum dcp_audio_link op, scoped_guard(mutex, &asrv->open_lock) { if (!asrv->open_srv) return -ENODEV; + if (op == DCP_AUDIO_LINK_PREPARE) { + asrv->link_gen = 0; + if (!av_audiosrv_open_published(asrv)) + return -ENODEV; + } /* A link ends on the instance it was prepared on; one gone took it along. */ if (op != DCP_AUDIO_LINK_PREPARE && asrv->link_gen != asrv->open_gen) return -ENODEV; ret = av_audiosrv_call(asrv->open_srv, command, body, sizeof(body), 0, reply, sizeof(reply)); - if (!ret && op == DCP_AUDIO_LINK_PREPARE) + if (ret) + return ret; + status = get_unaligned_le32(reply + 48); + if (status) { + dev_err(dev, "audiosrv: link method %u answered %#x\n", command, status); + return -EIO; + } + if (op == DCP_AUDIO_LINK_PREPARE) { + if (!av_audiosrv_open_published(asrv)) + return -ENODEV; asrv->link_gen = asrv->open_gen; - } - if (ret) - return ret; - status = get_unaligned_le32(reply + 48); - if (status) { - dev_err(dev, "audiosrv: link method %u answered %#x\n", command, status); - return -EIO; + } } return 0; } From d00568eaae5b5925cf88087eb31312d0fc27cc4a Mon Sep 17 00:00:00 2001 From: Eryk Wieliczko Date: Tue, 27 Oct 2026 16:40:37 +0100 Subject: [PATCH 3/9] drm/apple: complete live audio links after disconnect A disconnected PCM can still own a prepared firmware link. Complete that link before closing its client, including when sync_stop wins the race against the deferred owner release. Fixes: 3fe0ef05013a ("drm: apple: Add the DCP 14.7 external role's HDMI audio") Signed-off-by: Eryk Wieliczko Assisted-by: Claude:claude-opus-5-5 --- drivers/gpu/drm/apple/audio-link-test.c | 107 +++++++++++++++++++++++- drivers/gpu/drm/apple/audio.c | 22 +++-- drivers/gpu/drm/apple/audio.h | 2 + drivers/gpu/drm/apple/av-v14-test.c | 3 + drivers/gpu/drm/apple/av.c | 14 ++++ 5 files changed, 137 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/apple/audio-link-test.c b/drivers/gpu/drm/apple/audio-link-test.c index a90d85f221ead2..b69d38de25e73a 100644 --- a/drivers/gpu/drm/apple/audio-link-test.c +++ b/drivers/gpu/drm/apple/audio-link-test.c @@ -11,13 +11,47 @@ struct dcpaud_link_test { bool link_on; + bool link_live; u64 asked; unsigned int completes; unsigned int stops; + struct dma_device dma; + struct dma_chan channel; + char sequence[16]; + unsigned int nsequence; }; static struct dcpaud_link_test *dcpaud_link_current; +static void dcpaud_link_record(char step) +{ + struct dcpaud_link_test *t = dcpaud_link_current; + + if (t->nsequence < sizeof(t->sequence) - 1) + t->sequence[t->nsequence++] = step; +} + +static bool dcpaud_link_test_live(struct device *dev) +{ + return dcpaud_link_current->link_live; +} + +static int dcpaud_link_test_terminate(struct dma_chan *chan) +{ + return 0; +} + +static void dcpaud_link_test_drain(struct dma_chan *chan) +{ + dcpaud_link_record('D'); +} + +static enum dma_status dcpaud_link_test_status(struct dma_chan *chan, + dma_cookie_t cookie, struct dma_tx_state *state) +{ + return DMA_COMPLETE; +} + static bool dcpaud_link_test_on(struct device *dev, u64 gen) { dcpaud_link_current->asked = gen; @@ -27,10 +61,13 @@ static bool dcpaud_link_test_on(struct device *dev, u64 gen) static int dcpaud_link_test_call(struct device *dev, enum dcp_audio_link op, const struct dcp_sound_cookie *cookie) { - if (op == DCP_AUDIO_LINK_COMPLETE) + if (op == DCP_AUDIO_LINK_COMPLETE) { dcpaud_link_current->completes++; - else if (op == DCP_AUDIO_LINK_STOP) + dcpaud_link_record('C'); + } else if (op == DCP_AUDIO_LINK_STOP) { dcpaud_link_current->stops++; + dcpaud_link_record('S'); + } return 0; } @@ -49,7 +86,13 @@ static struct dcp_audio *dcpaud_link_setup(struct kunit *test, struct platform_d dcpaud->native_link = true; mutex_init(&dcpaud->link_lock); dcpaud_link_current = t; + t->link_live = true; + t->channel.device = &t->dma; + t->dma.device_terminate_all = dcpaud_link_test_terminate; + t->dma.device_synchronize = dcpaud_link_test_drain; + t->dma.device_tx_status = dcpaud_link_test_status; kunit_activate_static_stub(test, dcp_audiosrv_link_on, dcpaud_link_test_on); + kunit_activate_static_stub(test, dcp_audiosrv_link_live, dcpaud_link_test_live); kunit_activate_static_stub(test, dcp_audiosrv_link, dcpaud_link_test_call); return dcpaud; } @@ -85,8 +128,68 @@ static void dcpaud_link_release_instance(struct kunit *test) KUNIT_EXPECT_EQ(test, dcpaud_link_current->completes, 2U); } +static void dcpaud_link_test_close(void *data) +{ + snd_dmaengine_pcm_close(data); +} + +static struct snd_pcm_substream *dcpaud_link_test_open(struct kunit *test, + struct dcp_audio *dcpaud) +{ + struct snd_pcm_substream *sub = kunit_kzalloc(test, sizeof(*sub), GFP_KERNEL); + struct snd_pcm_runtime *runtime = kunit_kzalloc(test, sizeof(*runtime), GFP_KERNEL); + struct snd_pcm *pcm = kunit_kzalloc(test, sizeof(*pcm), GFP_KERNEL); + + KUNIT_ASSERT_NOT_NULL(test, sub); + KUNIT_ASSERT_NOT_NULL(test, runtime); + KUNIT_ASSERT_NOT_NULL(test, pcm); + pcm->private_data = dcpaud; + pcm->nonatomic = true; + sub->pcm = pcm; + sub->runtime = runtime; + KUNIT_ASSERT_EQ(test, snd_dmaengine_pcm_open(sub, &dcpaud_link_current->channel), 0); + KUNIT_ASSERT_EQ(test, kunit_add_action_or_reset(test, dcpaud_link_test_close, sub), 0); + return sub; +} + +static void dcpaud_disconnect_sync_completes_live_owner(struct kunit *test) +{ + struct platform_device *pdev; + struct dcp_audio *audio = dcpaud_link_setup(test, &pdev); + struct snd_pcm_substream *sub = dcpaud_link_test_open(test, audio); + + /* Disconnect cleared connected; the deferred owner release has not run. */ + audio->connected = false; + audio->link_prepared = true; + audio->link_started = true; + KUNIT_ASSERT_EQ(test, dcp_pcm_native_sync_stop(sub), 0); + KUNIT_EXPECT_STREQ(test, dcpaud_link_current->sequence, "DSC"); + KUNIT_EXPECT_FALSE(test, audio->link_prepared); + KUNIT_EXPECT_FALSE(test, audio->link_started); + dcpaud_release_link(pdev, NULL); + KUNIT_EXPECT_EQ(test, dcpaud_link_current->stops, 1U); + KUNIT_EXPECT_EQ(test, dcpaud_link_current->completes, 1U); +} + +static void dcpaud_gone_service_drops_link(struct kunit *test) +{ + struct platform_device *pdev; + struct dcp_audio *audio = dcpaud_link_setup(test, &pdev); + struct snd_pcm_substream *sub = dcpaud_link_test_open(test, audio); + + audio->link_prepared = true; + audio->link_started = true; + dcpaud_link_current->link_live = false; + KUNIT_ASSERT_EQ(test, dcp_pcm_native_sync_stop(sub), 0); + KUNIT_EXPECT_STREQ(test, dcpaud_link_current->sequence, "D"); + KUNIT_EXPECT_FALSE(test, audio->link_prepared); + KUNIT_EXPECT_FALSE(test, audio->link_started); +} + static struct kunit_case dcpaud_link_cases[] = { KUNIT_CASE(dcpaud_link_release_instance), + KUNIT_CASE(dcpaud_disconnect_sync_completes_live_owner), + KUNIT_CASE(dcpaud_gone_service_drops_link), {} }; diff --git a/drivers/gpu/drm/apple/audio.c b/drivers/gpu/drm/apple/audio.c index 2b507dc1fa1b00..434309e0ff7d5e 100644 --- a/drivers/gpu/drm/apple/audio.c +++ b/drivers/gpu/drm/apple/audio.c @@ -639,25 +639,29 @@ static int dcp_pcm_native_close(struct snd_pcm_substream *substream) } /* - * Stop and complete the link held, on the open audio service (@live), or - * drop both unsent with a service that went away. + * A PCM disconnect does not end its firmware service. Complete the link on + * that instance while it is published; a removed instance took it along. + * The service can disappear after the check, so -ENODEV during teardown is normal. */ -static void dcpaud_link_teardown(struct dcp_audio *dcpaud, bool live) +static void dcpaud_link_teardown(struct dcp_audio *dcpaud) { + bool live; int ret; lockdep_assert_held(&dcpaud->link_lock); + live = (dcpaud->link_prepared || dcpaud->link_started) && + dcp_audiosrv_link_live(dcpaud->dcp_dev); if (dcpaud->link_started && live) { ret = dcp_audiosrv_link(dcpaud->dcp_dev, DCP_AUDIO_LINK_STOP, &dcpaud->selected_cookie); - if (ret) + if (ret && ret != -ENODEV) dev_err(dcpaud->dev, "audio link stop failed: %d\n", ret); } dcpaud->link_started = false; if (dcpaud->link_prepared && live) { ret = dcp_audiosrv_link(dcpaud->dcp_dev, DCP_AUDIO_LINK_COMPLETE, &dcpaud->selected_cookie); - if (ret) + if (ret && ret != -ENODEV) dev_err(dcpaud->dev, "audio link complete failed: %d\n", ret); } dcpaud->link_prepared = false; @@ -670,7 +674,7 @@ static int dcp_pcm_native_hw_params(struct snd_pcm_substream *substream, struct dcp_audio *dcpaud = substream->pcm->private_data; scoped_guard(mutex, &dcpaud->link_lock) - dcpaud_link_teardown(dcpaud, dcpaud_connection_up(dcpaud)); + dcpaud_link_teardown(dcpaud); return dcp_pcm_hw_params(substream, params); } @@ -680,7 +684,7 @@ static int dcp_pcm_native_hw_free(struct snd_pcm_substream *substream) struct dcp_audio *dcpaud = substream->pcm->private_data; guard(mutex)(&dcpaud->link_lock); - dcpaud_link_teardown(dcpaud, dcpaud_connection_up(dcpaud)); + dcpaud_link_teardown(dcpaud); return 0; } @@ -781,7 +785,7 @@ static int dcp_pcm_native_sync_stop(struct snd_pcm_substream *substream) ret = snd_dmaengine_pcm_sync_stop(substream); guard(mutex)(&dcpaud->link_lock); - dcpaud_link_teardown(dcpaud, dcpaud_connection_up(dcpaud)); + dcpaud_link_teardown(dcpaud); return ret; } @@ -810,7 +814,7 @@ void dcpaud_release_link(struct platform_device *pdev, const u64 *gen) return; if (dcpaud->chan) dmaengine_synchronize(dcpaud->chan); - dcpaud_link_teardown(dcpaud, true); + dcpaud_link_teardown(dcpaud); } static const struct snd_pcm_ops dcp_native_playback_ops = { diff --git a/drivers/gpu/drm/apple/audio.h b/drivers/gpu/drm/apple/audio.h index 58633598253eaa..dcf752efb9a121 100644 --- a/drivers/gpu/drm/apple/audio.h +++ b/drivers/gpu/drm/apple/audio.h @@ -35,6 +35,8 @@ int dcp_audiosrv_link(struct device *dev, enum dcp_audio_link op, void dcpaud_release_link(struct platform_device *pdev, const u64 *gen); /* Whether the link prepared last is on instance @gen, still the open one */ bool dcp_audiosrv_link_on(struct device *dev, u64 gen); +/* The prepared link still belongs to the client of the published instance. */ +bool dcp_audiosrv_link_live(struct device *dev); #endif #endif /* __AUDIO_H__ */ diff --git a/drivers/gpu/drm/apple/av-v14-test.c b/drivers/gpu/drm/apple/av-v14-test.c index e1d13ab00c3ec2..b17227890c4c87 100644 --- a/drivers/gpu/drm/apple/av-v14-test.c +++ b/drivers/gpu/drm/apple/av-v14-test.c @@ -303,7 +303,9 @@ static void av_v14_link_instance(struct kunit *test) old = t->asrv->gen; KUNIT_EXPECT_EQ(test, dcp_audiosrv_link(t->dcp->dev, DCP_AUDIO_LINK_PREPARE, &cookie), 0); KUNIT_EXPECT_TRUE(test, dcp_audiosrv_link_on(t->dcp->dev, old)); + KUNIT_EXPECT_TRUE(test, dcp_audiosrv_link_live(t->dcp->dev)); av_v14_replace(t); + KUNIT_EXPECT_FALSE(test, dcp_audiosrv_link_live(t->dcp->dev)); KUNIT_EXPECT_EQ(test, dcp_audiosrv_open(t->dcp->dev), 0); KUNIT_ASSERT_EQ(test, t->ncalls, 3U); KUNIT_EXPECT_EQ(test, dcp_audiosrv_link(t->dcp->dev, DCP_AUDIO_LINK_STOP, &cookie), -ENODEV); @@ -314,6 +316,7 @@ static void av_v14_link_instance(struct kunit *test) KUNIT_EXPECT_FALSE(test, dcp_audiosrv_link_on(t->dcp->dev, t->asrv->gen)); KUNIT_EXPECT_EQ(test, dcp_audiosrv_link(t->dcp->dev, DCP_AUDIO_LINK_PREPARE, &cookie), 0); KUNIT_EXPECT_TRUE(test, dcp_audiosrv_link_on(t->dcp->dev, t->asrv->gen)); + KUNIT_EXPECT_TRUE(test, dcp_audiosrv_link_live(t->dcp->dev)); KUNIT_EXPECT_EQ(test, dcp_audiosrv_link(t->dcp->dev, DCP_AUDIO_LINK_COMPLETE, &cookie), 0); KUNIT_EXPECT_EQ(test, t->ncalls, 5U); } diff --git a/drivers/gpu/drm/apple/av.c b/drivers/gpu/drm/apple/av.c index f34b32566adc4e..c0bacc9956d31e 100644 --- a/drivers/gpu/drm/apple/av.c +++ b/drivers/gpu/drm/apple/av.c @@ -386,6 +386,20 @@ static bool av_audiosrv_open_published(struct audiosrv_data *asrv) asrv->gen == asrv->open_gen; } +bool dcp_audiosrv_link_live(struct device *dev) +{ + struct apple_dcp *dcp; + struct audiosrv_data *asrv; + + KUNIT_STATIC_STUB_REDIRECT(dcp_audiosrv_link_live, dev); + dcp = dev_get_drvdata(dev); + asrv = dcp->audiosrv; + if (!asrv) + return false; + guard(mutex)(&asrv->open_lock); + return asrv->link_gen == asrv->open_gen && av_audiosrv_open_published(asrv); +} + /* * The link commands of the 14.7 external role's audio service, the native * audio driver's: prepare, start, stop and complete all take the selected From 69e431f5787d1825ccbfaea9b8975b79eede21fd Mon Sep 17 00:00:00 2001 From: Eryk Wieliczko Date: Tue, 27 Oct 2026 08:57:12 +0100 Subject: [PATCH 4/9] dmaengine: apple-sio: allocate the full descriptor bitmap The 64-slot descriptor bitmap is only allocated two bytes. Use the bitmap allocator so unsigned-long bitmap operations have enough storage. Fixes: 5fdca02134fa ("dmaengine: apple-sio: Add Apple SIO driver") Signed-off-by: Eryk Wieliczko Assisted-by: Claude:claude-opus-5-5 --- drivers/dma/apple-sio.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/dma/apple-sio.c b/drivers/dma/apple-sio.c index 0c06e0264b7b60..5a125ebb854f7f 100644 --- a/drivers/dma/apple-sio.c +++ b/drivers/dma/apple-sio.c @@ -6,6 +6,7 @@ */ #include +#include #include #include #include @@ -898,8 +899,8 @@ static int sio_alloc_shmem(struct sio_data *sio) return -ENOMEM; sio->shmem_desc_base = (struct sio_coproc_desc *) (sio->shmem + 56); - sio->desc_allocated = devm_kzalloc(sio->dev, SIO_NO_DESC_SLOTS / 32, - GFP_KERNEL); + sio->desc_allocated = devm_bitmap_zalloc(sio->dev, SIO_NO_DESC_SLOTS, + GFP_KERNEL); if (!sio->desc_allocated) return -ENOMEM; From 459ba2444ce53ea30792e3b7ac868c9d8c20313d Mon Sep 17 00:00:00 2001 From: Eryk Wieliczko Date: Tue, 27 Oct 2026 22:56:33 +0100 Subject: [PATCH 5/9] drm/apple: hold the native HDMI audio link between streams The sink behind the J514S HDMI port (PS190 converter, TV) mutes the audio of a link stopped and started again with an unchanged format, until the DP link trains again. macOS never does that: it starts the link once and, between streams, keeps it running on a cleared buffer until the display sleeps or the format changes. Do the same for stereo on the HDMI port: the stopped stream's DMA is replaced by silence and the link is held. The mute was measured behind the HDMI port only, so a Type-C output keeps one link per stream unless hold_link=2; hold_link=0 restores one link per stream everywhere. While links are held, a link is not started in the first 1.5 s after the sink connects: once, at boot, a link started 0.2 s after the display came on was not heard, and holding it kept it silent. A link prepared at START configures the SIO channel first, as the PCM's prepare does. Signed-off-by: Eryk Wieliczko Assisted-by: Claude:claude-opus-5-5 --- drivers/gpu/drm/apple/audio-link-test.c | 237 +++++++++++++++- drivers/gpu/drm/apple/audio.c | 349 +++++++++++++++++++++--- drivers/gpu/drm/apple/audio.h | 2 + drivers/gpu/drm/apple/av.c | 6 + 4 files changed, 557 insertions(+), 37 deletions(-) diff --git a/drivers/gpu/drm/apple/audio-link-test.c b/drivers/gpu/drm/apple/audio-link-test.c index b69d38de25e73a..ef897f94d7f1ce 100644 --- a/drivers/gpu/drm/apple/audio-link-test.c +++ b/drivers/gpu/drm/apple/audio-link-test.c @@ -1,9 +1,9 @@ // SPDX-License-Identifier: GPL-2.0-only OR MIT /* * Included by audio.c: a Terminate's link release ends only the link of the - * instance it is for. The production dcpaud_release_link() runs; the audio - * service's answer to which instance the link is on, and its link calls, - * are stubbed. + * instance it is for, and a stopped stereo link is held between streams. + * The production link lifecycle runs; the audio service's answer to which + * instance the link is on, its link calls and the DMA channel are stubbed. */ #include #include @@ -12,11 +12,13 @@ struct dcpaud_link_test { bool link_on; bool link_live; + bool typec; u64 asked; unsigned int completes; unsigned int stops; struct dma_device dma; struct dma_chan channel; + struct dma_async_tx_descriptor desc; char sequence[16]; unsigned int nsequence; }; @@ -38,9 +40,43 @@ static bool dcpaud_link_test_live(struct device *dev) static int dcpaud_link_test_terminate(struct dma_chan *chan) { + dcpaud_link_record('X'); return 0; } +static dma_cookie_t dcpaud_link_test_submit(struct dma_async_tx_descriptor *tx) +{ + return 1; +} + +static struct dma_async_tx_descriptor * +dcpaud_link_test_prep(struct dma_chan *chan, dma_addr_t buf, size_t len, size_t period, + enum dma_transfer_direction dir, unsigned long flags) +{ + struct dcpaud_link_test *t = dcpaud_link_current; + + dcpaud_link_record('Q'); + t->desc.chan = chan; + t->desc.tx_submit = dcpaud_link_test_submit; + return &t->desc; +} + +/* G the channel configured */ +static int dcpaud_link_test_config(struct dma_chan *chan, struct dma_slave_config *config) +{ + dcpaud_link_record('G'); + return 0; +} + +static bool dcpaud_link_test_typec(struct device *dev) +{ + return dcpaud_link_current->typec; +} + +static void dcpaud_link_test_issue(struct dma_chan *chan) +{ +} + static void dcpaud_link_test_drain(struct dma_chan *chan) { dcpaud_link_record('D'); @@ -67,6 +103,10 @@ static int dcpaud_link_test_call(struct device *dev, enum dcp_audio_link op, } else if (op == DCP_AUDIO_LINK_STOP) { dcpaud_link_current->stops++; dcpaud_link_record('S'); + } else if (op == DCP_AUDIO_LINK_PREPARE) { + dcpaud_link_record('P'); + } else if (op == DCP_AUDIO_LINK_START) { + dcpaud_link_record('T'); } return 0; } @@ -91,9 +131,13 @@ static struct dcp_audio *dcpaud_link_setup(struct kunit *test, struct platform_d t->dma.device_terminate_all = dcpaud_link_test_terminate; t->dma.device_synchronize = dcpaud_link_test_drain; t->dma.device_tx_status = dcpaud_link_test_status; + t->dma.device_prep_dma_cyclic = dcpaud_link_test_prep; + t->dma.device_issue_pending = dcpaud_link_test_issue; + t->dma.device_config = dcpaud_link_test_config; kunit_activate_static_stub(test, dcp_audiosrv_link_on, dcpaud_link_test_on); kunit_activate_static_stub(test, dcp_audiosrv_link_live, dcpaud_link_test_live); kunit_activate_static_stub(test, dcp_audiosrv_link, dcpaud_link_test_call); + kunit_activate_static_stub(test, dcp_audiosrv_typec, dcpaud_link_test_typec); return dcpaud; } @@ -186,10 +230,197 @@ static void dcpaud_gone_service_drops_link(struct kunit *test) KUNIT_EXPECT_FALSE(test, audio->link_started); } +/* A stream stopped by STOP with its DMA playing on, on the PCM's channel */ +static void dcpaud_link_test_held_stop(struct kunit *test, struct dcp_audio *audio) +{ + audio->chan = &dcpaud_link_current->channel; + audio->silence = kunit_kzalloc(test, DCPAUD_SILENCE_BYTES, GFP_KERNEL); + KUNIT_ASSERT_NOT_NULL(test, audio->silence); + audio->link_prepared = true; + audio->link_started = true; + audio->dma_cleared = true; + /* The stream's PM reference, carried by the cleared DMA */ + pm_runtime_get_noresume(audio->dev); +} + +static void dcpaud_stop_holds_link(struct kunit *test) +{ + struct platform_device *pdev; + struct dcp_audio *audio = dcpaud_link_setup(test, &pdev); + struct snd_pcm_substream *sub = dcpaud_link_test_open(test, audio); + + dcpaud_link_test_held_stop(test, audio); + /* The stream's DMA ends, the silence follows it; no link call */ + KUNIT_ASSERT_EQ(test, dcp_pcm_native_sync_stop(sub), 0); + KUNIT_EXPECT_STREQ(test, dcpaud_link_current->sequence, "XQD"); + KUNIT_EXPECT_TRUE(test, audio->silence_running); + KUNIT_EXPECT_TRUE(test, audio->link_started); + + KUNIT_ASSERT_EQ(test, dcp_pcm_native_hw_free(sub), 0); + KUNIT_EXPECT_TRUE(test, audio->link_prepared); + KUNIT_EXPECT_EQ(test, dcpaud_link_current->stops + dcpaud_link_current->completes, 0U); + + /* The display goes off: the silence ends, then the link */ + dcpaud_release_link(pdev, NULL); + KUNIT_EXPECT_STREQ(test, dcpaud_link_current->sequence, "XQDXDDSC"); + KUNIT_EXPECT_FALSE(test, audio->silence_running); + KUNIT_EXPECT_FALSE(test, audio->link_prepared); + /* The stream's reference went with the silence */ + KUNIT_EXPECT_EQ(test, atomic_read(&audio->dev->power.usage_count), 0); +} + +static void dcpaud_held_stop_without_service(struct kunit *test) +{ + struct platform_device *pdev; + struct dcp_audio *audio = dcpaud_link_setup(test, &pdev); + struct snd_pcm_substream *sub = dcpaud_link_test_open(test, audio); + + dcpaud_link_test_held_stop(test, audio); + dcpaud_link_current->link_live = false; + /* No service to hold it for: the DMA ends and the link is dropped */ + KUNIT_ASSERT_EQ(test, dcp_pcm_native_sync_stop(sub), 0); + KUNIT_EXPECT_STREQ(test, dcpaud_link_current->sequence, "XD"); + KUNIT_EXPECT_FALSE(test, audio->silence_running); + KUNIT_EXPECT_FALSE(test, audio->link_prepared); + KUNIT_EXPECT_FALSE(test, audio->link_started); + KUNIT_EXPECT_EQ(test, atomic_read(&audio->dev->power.usage_count), 0); +} + +/* A held link and its silence, and a prepared stream to start on it */ +static struct snd_pcm_substream *dcpaud_link_test_held(struct kunit *test, struct dcp_audio *audio) +{ + struct snd_pcm_substream *sub = dcpaud_link_test_open(test, audio); + + mutex_init(&audio->data_lock); + audio->connected = true; + audio->chan = &dcpaud_link_current->channel; + audio->link_prepared = true; + audio->link_started = true; + audio->silence_running = true; + /* The held link's PM reference, then the stream's from its START */ + pm_runtime_get_noresume(audio->dev); + pm_runtime_get_noresume(audio->dev); + sub->runtime->frame_bits = 64; + sub->runtime->buffer_size = 8192; + sub->runtime->period_size = 512; + return sub; +} + +static void dcpaud_start_on_held_link(struct kunit *test) +{ + struct platform_device *pdev; + struct dcp_audio *audio = dcpaud_link_setup(test, &pdev); + struct snd_pcm_substream *sub = dcpaud_link_test_held(test, audio); + + /* Its element: the stream's DMA replaces the silence, no link call */ + KUNIT_ASSERT_EQ(test, dcp_pcm_native_start(sub, SNDRV_PCM_TRIGGER_START), 0); + KUNIT_EXPECT_STREQ(test, dcpaud_link_current->sequence, "XQ"); + KUNIT_EXPECT_FALSE(test, audio->silence_running); + KUNIT_EXPECT_TRUE(test, audio->link_started); + pm_runtime_put_noidle(audio->dev); +} + +static void dcpaud_start_replaces_held_link(struct kunit *test) +{ + struct platform_device *pdev; + struct dcp_audio *audio = dcpaud_link_setup(test, &pdev); + struct snd_pcm_substream *sub = dcpaud_link_test_held(test, audio); + + /* Another element: the held link ends, the stream's is configured, prepared and started */ + memset(&audio->link_cookie, 1, sizeof(audio->link_cookie)); + memset(&audio->selected_cookie, 2, sizeof(audio->selected_cookie)); + KUNIT_ASSERT_EQ(test, dcp_pcm_native_start(sub, SNDRV_PCM_TRIGGER_START), 0); + KUNIT_EXPECT_STREQ(test, dcpaud_link_current->sequence, "XDSCGPQT"); + KUNIT_EXPECT_EQ(test, memcmp(&audio->link_cookie, &audio->selected_cookie, + sizeof(audio->link_cookie)), 0); + KUNIT_EXPECT_TRUE(test, audio->link_started); + pm_runtime_put_noidle(audio->dev); +} + +static void dcpaud_start_after_held_link_went(struct kunit *test) +{ + struct platform_device *pdev; + struct dcp_audio *audio = dcpaud_link_setup(test, &pdev); + struct snd_pcm_substream *sub = dcpaud_link_test_held(test, audio); + + /* The held link's service went (unplug): the stream gets a link of its own */ + dcpaud_link_current->link_live = false; + KUNIT_ASSERT_EQ(test, dcp_pcm_native_start(sub, SNDRV_PCM_TRIGGER_START), 0); + KUNIT_EXPECT_STREQ(test, dcpaud_link_current->sequence, "XDGPQT"); + KUNIT_EXPECT_FALSE(test, audio->silence_running); + KUNIT_EXPECT_TRUE(test, audio->link_started); + pm_runtime_put_noidle(audio->dev); + KUNIT_EXPECT_EQ(test, atomic_read(&audio->dev->power.usage_count), 0); +} + +static void dcpaud_failed_start_keeps_held_link(struct kunit *test) +{ + struct platform_device *pdev; + struct dcp_audio *audio = dcpaud_link_setup(test, &pdev); + struct snd_pcm_substream *sub = dcpaud_link_test_held(test, audio); + + /* Unplugged before the swap: START fails, its STOP leaves the silence */ + audio->connected = false; + sub->runtime->channels = 2; + sub->runtime->dma_area = kunit_kzalloc(test, 64, GFP_KERNEL); + sub->runtime->dma_bytes = 64; + KUNIT_ASSERT_EQ(test, dcp_pcm_native_start(sub, SNDRV_PCM_TRIGGER_START), -ENXIO); + KUNIT_ASSERT_EQ(test, dcp_pcm_native_trigger(sub, SNDRV_PCM_TRIGGER_STOP), 0); + KUNIT_EXPECT_STREQ(test, dcpaud_link_current->sequence, ""); + KUNIT_EXPECT_FALSE(test, audio->dma_cleared); + KUNIT_EXPECT_TRUE(test, audio->silence_running); + /* Only the held silence's reference is left */ + KUNIT_EXPECT_EQ(test, atomic_read(&audio->dev->power.usage_count), 1); + pm_runtime_put_noidle(audio->dev); +} + +static void dcpaud_link_test_hold_default(void *data) +{ + WRITE_ONCE(hold_link, 1); +} + +static void dcpaud_typec_stop_ends_stream(struct kunit *test) +{ + struct platform_device *pdev; + struct dcp_audio *audio = dcpaud_link_setup(test, &pdev); + struct snd_pcm_substream *sub = dcpaud_link_test_open(test, audio); + + KUNIT_ASSERT_EQ(test, + kunit_add_action_or_reset(test, dcpaud_link_test_hold_default, NULL), 0); + sub->runtime->channels = 2; + sub->runtime->dma_area = kunit_kzalloc(test, 64, GFP_KERNEL); + KUNIT_ASSERT_NOT_NULL(test, sub->runtime->dma_area); + sub->runtime->dma_bytes = 64; + dcpaud_link_current->typec = true; + + /* USB-C by default: STOP ends the stream's DMA, no link is held */ + audio->stream_dma = true; + pm_runtime_get_noresume(audio->dev); + KUNIT_ASSERT_EQ(test, dcp_pcm_native_trigger(sub, SNDRV_PCM_TRIGGER_STOP), 0); + KUNIT_EXPECT_STREQ(test, dcpaud_link_current->sequence, "X"); + KUNIT_EXPECT_FALSE(test, audio->dma_cleared); + KUNIT_EXPECT_EQ(test, atomic_read(&audio->dev->power.usage_count), 0); + + /* hold_link=2: the DMA plays the cleared buffer on for the held link */ + WRITE_ONCE(hold_link, 2); + pm_runtime_get_noresume(audio->dev); + KUNIT_ASSERT_EQ(test, dcp_pcm_native_trigger(sub, SNDRV_PCM_TRIGGER_STOP), 0); + KUNIT_EXPECT_STREQ(test, dcpaud_link_current->sequence, "X"); + KUNIT_EXPECT_TRUE(test, audio->dma_cleared); + pm_runtime_put_noidle(audio->dev); +} + static struct kunit_case dcpaud_link_cases[] = { KUNIT_CASE(dcpaud_link_release_instance), KUNIT_CASE(dcpaud_disconnect_sync_completes_live_owner), KUNIT_CASE(dcpaud_gone_service_drops_link), + KUNIT_CASE(dcpaud_stop_holds_link), + KUNIT_CASE(dcpaud_held_stop_without_service), + KUNIT_CASE(dcpaud_start_on_held_link), + KUNIT_CASE(dcpaud_start_replaces_held_link), + KUNIT_CASE(dcpaud_start_after_held_link_went), + KUNIT_CASE(dcpaud_failed_start_keeps_held_link), + KUNIT_CASE(dcpaud_typec_stop_ends_stream), {} }; diff --git a/drivers/gpu/drm/apple/audio.c b/drivers/gpu/drm/apple/audio.c index 434309e0ff7d5e..f3bd62d3a8375a 100644 --- a/drivers/gpu/drm/apple/audio.c +++ b/drivers/gpu/drm/apple/audio.c @@ -14,7 +14,9 @@ #include #include #include +#include #include +#include #include #include #include @@ -43,6 +45,13 @@ */ #define DCPAUD_PERIODS_MAX 16 +/* + * The silence a held link plays between streams. SIO keeps up to four + * periods of a transfer in flight, so it gets more than that. + */ +#define DCPAUD_SILENCE_PERIOD SZ_16K +#define DCPAUD_SILENCE_BYTES (8 * DCPAUD_SILENCE_PERIOD) + struct dcp_audio { struct device *dev; struct device *dcp_dev; @@ -59,6 +68,8 @@ struct dcp_audio { bool dcp_connected; /// dcp status keep for delayed initialization bool connected; unsigned int connection_cookie; + /* When the sink was last connected */ + ktime_t connected_at; struct snd_pcm_chmap_elem selected_chmap; struct dcp_sound_cookie selected_cookie; @@ -81,6 +92,18 @@ struct dcp_audio { struct mutex link_lock; bool link_prepared; bool link_started; + /* The element the link was prepared with */ + struct dcp_sound_cookie link_cookie; + /* START issued the stream's DMA */ + bool stream_dma; + /* STOP left the DMA playing the cleared buffer, see dcp_pcm_native_sync_stop() */ + bool dma_cleared; + /* Drops a held link whose service went, see dcpaud_disconnect() */ + struct work_struct stale_work; + /* The held link's silence, see dcpaud_silence_start() */ + void *silence; + dma_addr_t silence_addr; + bool silence_running; #endif }; @@ -291,7 +314,7 @@ static int dcpaud_init_dma(struct dcp_audio *dcpaud) { struct dma_chan *chan; if (dcpaud->chan) - return 0; + goto silence; chan = of_dma_request_slave_channel(dcpaud->dev->of_node, "tx"); /* squelch dma channel request errors, the driver will try again alter */ @@ -311,6 +334,14 @@ static int dcpaud_init_dma(struct dcp_audio *dcpaud) dcpaud->chan->device->dev, 1024 * 1024, SIZE_MAX); +silence: +#if IS_ENABLED(CONFIG_DRM_APPLE_DCP_T6030_AUDIO) + /* Without it a stopped link is not held, see dcpaud_silence_start(). */ + if (dcpaud_native_link(dcpaud) && !dcpaud->silence) + dcpaud->silence = dma_alloc_coherent(dcpaud->chan->device->dev, + DCPAUD_SILENCE_BYTES, + &dcpaud->silence_addr, GFP_KERNEL); +#endif return 0; } @@ -599,14 +630,114 @@ struct snd_pcm_ops dcp_playback_ops = { * * prepare: the DMA channel configured, then the link prepared, once; * START: the cyclic DMA issued, then the link started; - * STOP: the DMA terminated; - * sync_stop: the DMA drained, then the link stopped and completed; + * STOP: the DMA terminated, or for a held link (below) left playing + * the cleared buffer; + * sync_stop: the DMA drained, then the link stopped and completed, or the + * held link's silence queued in place of the DMA; * hw_free: a link prepared but never started, completed. * - * Every link method carries the selected element's link data. A link of an - * earlier connection went with its service: its state is dropped unsent. - * The audio service itself is open only while the PCM is. + * Every link method carries its element's link data. A link of an earlier + * connection went with its service: its state is dropped unsent. The audio + * service itself is open while the PCM is, or while its link is held. + */ +/* + * The sink behind the J514S HDMI port (PS190 converter and TV) mutes the + * audio of a link that stops and starts again with an unchanged format, + * until the DP link trains again: only a different channel layout brings + * it back. So a stereo link, once started, is held between streams, as + * macOS holds it: the stopped stream's DMA is swapped for silence and the + * link keeps running until the format changes or the display goes off + * (dcpaud_release_link()). The DMA alone restarts without that loss. + */ +/* + * Read-only at runtime: a stream opened while it was lower keeps periods + * longer than a held link's synchronize allows for. + */ +static int hold_link = 1; +module_param(hold_link, int, 0444); +MODULE_PARM_DESC(hold_link, + "Hold started audio links between streams (silence): 0 never, 1 on the HDMI port (default), 2 also on USB-C"); + +/* + * The mute was measured behind the HDMI port. USB-C sinks keep one link per + * stream unless hold_link=2, until they have been qualified with this. + */ +static bool dcpaud_holds(struct dcp_audio *dcpaud) +{ + int hold = READ_ONCE(hold_link); + + return hold >= 2 || (hold == 1 && !dcp_audiosrv_typec(dcpaud->dcp_dev)); +} + +/* + * Seen once, at boot: a link started about 0.2 s after the display came on + * was not heard, and holding it kept it silent; the next connection's first + * link, started later, was heard. While links are held, the first link of a + * connection starts once the sink has been up this long, a margin rather + * than a measured bound. One link per stream needs no wait: the next stream + * starts a new link. + */ +#define DCPAUD_SINK_SETTLE_MS 1500 + +/* Queue the held link's silence; it carries the PM reference of the stream. */ +static int dcpaud_silence_start(struct dcp_audio *dcpaud) +{ + struct dma_async_tx_descriptor *desc; + + lockdep_assert_held(&dcpaud->link_lock); + if (!dcpaud->silence) + return -ENOMEM; + desc = dmaengine_prep_dma_cyclic(dcpaud->chan, dcpaud->silence_addr, + DCPAUD_SILENCE_BYTES, DCPAUD_SILENCE_PERIOD, + DMA_MEM_TO_DEV, 0); + if (!desc) + return -ENOMEM; + if (dma_submit_error(dmaengine_submit(desc))) + return -EIO; + dma_async_issue_pending(dcpaud->chan); + dcpaud->silence_running = true; + dev_dbg(dcpaud->dev, "link held, playing silence\n"); + return 0; +} + +static void dcpaud_silence_stop(struct dcp_audio *dcpaud) +{ + lockdep_assert_held(&dcpaud->link_lock); + if (!dcpaud->silence_running) + return; + dmaengine_terminate_sync(dcpaud->chan); + dcpaud->silence_running = false; + pm_runtime_mark_last_busy(dcpaud->dev); + pm_runtime_put_autosuspend(dcpaud->dev); + dev_dbg(dcpaud->dev, "held link's silence stopped\n"); +} + +static void dcpaud_link_teardown(struct dcp_audio *dcpaud); + +/* + * A held link went with its audio service (sink change, unplug): its silence + * stops and its state is dropped unsent, so that a new service's stream + * prepares and starts its own link. */ +static void dcpaud_drop_stale(struct dcp_audio *dcpaud) +{ + lockdep_assert_held(&dcpaud->link_lock); + if ((dcpaud->silence_running || dcpaud->link_prepared || dcpaud->link_started) && + !dcp_audiosrv_link_live(dcpaud->dcp_dev)) { + dev_dbg(dcpaud->dev, "held link went with its service\n"); + dcpaud_link_teardown(dcpaud); + } +} + +static void dcpaud_stale_work(struct work_struct *work) +{ + struct dcp_audio *dcpaud = container_of(work, struct dcp_audio, stale_work); + + guard(mutex)(&dcpaud->link_lock); + if (!dcpaud->dma_cleared) + dcpaud_drop_stale(dcpaud); +} + static int dcp_pcm_native_open(struct snd_pcm_substream *substream) { struct dcp_audio *dcpaud = substream->pcm->private_data; @@ -622,8 +753,15 @@ static int dcp_pcm_native_open(struct snd_pcm_substream *substream) } ret = dcp_pcm_open(substream); - if (ret && live) - dcp_audiosrv_close(dcpaud->dcp_dev); + if (ret && live) { + bool held; + + /* A held link keeps its service open. */ + scoped_guard(mutex, &dcpaud->link_lock) + held = dcpaud->silence_running; + if (!held) + dcp_audiosrv_close(dcpaud->dcp_dev); + } return ret; } @@ -632,8 +770,27 @@ static int dcp_pcm_native_close(struct snd_pcm_substream *substream) { struct dcp_audio *dcpaud = substream->pcm->private_data; int ret = dcp_pcm_close(substream); + bool held; - dcp_audiosrv_close(dcpaud->dcp_dev); + /* + * A held link keeps its service open. Closing the PCM terminated its + * channel, the silence with it: that is queued again. + */ + scoped_guard(mutex, &dcpaud->link_lock) { + dcpaud_drop_stale(dcpaud); + held = dcpaud->silence_running; + if (held) { + dcpaud->silence_running = false; + if (dcpaud_silence_start(dcpaud)) { + held = false; + dcpaud_link_teardown(dcpaud); + pm_runtime_mark_last_busy(dcpaud->dev); + pm_runtime_put_autosuspend(dcpaud->dev); + } + } + } + if (!held) + dcp_audiosrv_close(dcpaud->dcp_dev); return ret; } @@ -649,32 +806,46 @@ static void dcpaud_link_teardown(struct dcp_audio *dcpaud) int ret; lockdep_assert_held(&dcpaud->link_lock); + /* A held link's silence ends with it. */ + dcpaud_silence_stop(dcpaud); live = (dcpaud->link_prepared || dcpaud->link_started) && dcp_audiosrv_link_live(dcpaud->dcp_dev); if (dcpaud->link_started && live) { ret = dcp_audiosrv_link(dcpaud->dcp_dev, DCP_AUDIO_LINK_STOP, - &dcpaud->selected_cookie); + &dcpaud->link_cookie); if (ret && ret != -ENODEV) dev_err(dcpaud->dev, "audio link stop failed: %d\n", ret); } dcpaud->link_started = false; if (dcpaud->link_prepared && live) { ret = dcp_audiosrv_link(dcpaud->dcp_dev, DCP_AUDIO_LINK_COMPLETE, - &dcpaud->selected_cookie); + &dcpaud->link_cookie); if (ret && ret != -ENODEV) dev_err(dcpaud->dev, "audio link complete failed: %d\n", ret); } dcpaud->link_prepared = false; } -/* New parameters select a new element: the link of the old one ends first. */ +/* + * New parameters select a new element: the link of the old one ends first. + * A held link stays until a stream of another element starts (it may be + * userspace probing), and its channel stays configured. + */ static int dcp_pcm_native_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params) { struct dcp_audio *dcpaud = substream->pcm->private_data; + bool up = !dcpaud->open_unplugged && dcpaud_connection_up(dcpaud); + int ret; - scoped_guard(mutex, &dcpaud->link_lock) + scoped_guard(mutex, &dcpaud->link_lock) { + dcpaud_drop_stale(dcpaud); + if (up && dcpaud->silence_running) { + ret = dcpaud_select_cookie(dcpaud, params); + return ret < 0 ? ret : ret ? 0 : -EINVAL; + } dcpaud_link_teardown(dcpaud); + } return dcp_pcm_hw_params(substream, params); } @@ -684,14 +855,15 @@ static int dcp_pcm_native_hw_free(struct snd_pcm_substream *substream) struct dcp_audio *dcpaud = substream->pcm->private_data; guard(mutex)(&dcpaud->link_lock); - dcpaud_link_teardown(dcpaud); + if (!dcpaud->silence_running) + dcpaud_link_teardown(dcpaud); return 0; } -static int dcp_pcm_native_prepare(struct snd_pcm_substream *substream) +/* The SIO channel is configured again before every link prepare. */ +static int dcpaud_link_prepare(struct dcp_audio *dcpaud, struct snd_pcm_substream *substream) { - struct dcp_audio *dcpaud = substream->pcm->private_data; struct dma_slave_config slave_config = { .direction = DMA_MEM_TO_DEV, /* One 32-bit DPA entry per sample word, as in hw_params. */ @@ -699,6 +871,23 @@ static int dcp_pcm_native_prepare(struct snd_pcm_substream *substream) }; int ret; + lockdep_assert_held(&dcpaud->link_lock); + ret = dmaengine_slave_config(snd_dmaengine_pcm_get_chan(substream), &slave_config); + if (ret) + return ret; + ret = dcp_audiosrv_link(dcpaud->dcp_dev, DCP_AUDIO_LINK_PREPARE, + &dcpaud->selected_cookie); + if (ret) + return ret; + dcpaud->link_prepared = true; + dcpaud->link_cookie = dcpaud->selected_cookie; + return 0; +} + +static int dcp_pcm_native_prepare(struct snd_pcm_substream *substream) +{ + struct dcp_audio *dcpaud = substream->pcm->private_data; + if (dcpaud->open_unplugged) return 0; @@ -706,44 +895,72 @@ static int dcp_pcm_native_prepare(struct snd_pcm_substream *substream) return -ENXIO; guard(mutex)(&dcpaud->link_lock); + dcpaud_drop_stale(dcpaud); if (dcpaud->link_prepared) return 0; - - /* The SIO channel is configured again before every link prepare. */ - ret = dmaengine_slave_config(snd_dmaengine_pcm_get_chan(substream), - &slave_config); - if (ret) - return ret; - - ret = dcp_audiosrv_link(dcpaud->dcp_dev, DCP_AUDIO_LINK_PREPARE, - &dcpaud->selected_cookie); - if (ret) - return ret; - - dcpaud->link_prepared = true; - return 0; + return dcpaud_link_prepare(dcpaud, substream); } static int dcp_pcm_native_start(struct snd_pcm_substream *substream, int cmd) { struct dcp_audio *dcpaud = substream->pcm->private_data; + s64 up_ms; + bool held; int ret; if (!dcpaud_connection_up(dcpaud)) return -ENXIO; + /* The first link of the connection waits for the sink to settle. */ + scoped_guard(mutex, &dcpaud->data_lock) + up_ms = ktime_ms_delta(ktime_get(), dcpaud->connected_at); + if (dcpaud_holds(dcpaud) && !READ_ONCE(dcpaud->link_started) && + up_ms >= 0 && up_ms < DCPAUD_SINK_SETTLE_MS) { + dev_dbg(dcpaud->dev, "sink up %lld ms, link waits\n", up_ms); + msleep(DCPAUD_SINK_SETTLE_MS - up_ms); + } + + guard(mutex)(&dcpaud->link_lock); + dcpaud_drop_stale(dcpaud); + /* The stream's DMA follows the held link's silence. */ + held = dcpaud->silence_running; + if (held) { + dmaengine_terminate_async(dcpaud->chan); + dcpaud->silence_running = false; + pm_runtime_mark_last_busy(dcpaud->dev); + pm_runtime_put_autosuspend(dcpaud->dev); + } + + /* + * A held link of another element ends, and the stream's is prepared; + * so is a stream's whose prepare found a link that has gone since. + */ + if (!dcpaud->link_prepared || + memcmp(&dcpaud->link_cookie, &dcpaud->selected_cookie, sizeof(dcpaud->link_cookie))) { + if (dcpaud->link_prepared) { + dmaengine_synchronize(dcpaud->chan); + dcpaud_link_teardown(dcpaud); + } + ret = dcpaud_link_prepare(dcpaud, substream); + if (ret) + return ret; + dev_dbg(dcpaud->dev, "link prepared at START\n"); + } else if (held) { + dev_dbg(dcpaud->dev, "stream follows the held link's silence\n"); + } + /* The first period descriptor is issued before the link starts. */ ret = snd_dmaengine_pcm_trigger(substream, cmd); if (ret) return ret; + WRITE_ONCE(dcpaud->stream_dma, true); - guard(mutex)(&dcpaud->link_lock); if (!dcpaud->link_prepared) return -EBADFD; if (dcpaud->link_started) return 0; ret = dcp_audiosrv_link(dcpaud->dcp_dev, DCP_AUDIO_LINK_START, - &dcpaud->selected_cookie); + &dcpaud->link_cookie); dcpaud->link_started = !ret; return ret; } @@ -765,8 +982,21 @@ static int dcp_pcm_native_trigger(struct snd_pcm_substream *substream, int cmd) return dcp_pcm_native_start(substream, cmd); case SNDRV_PCM_TRIGGER_STOP: + /* + * The DMA plays the cleared buffer on, with the stream's PM + * reference, until sync_stop swaps it for the held link's + * silence: the link must not run dry meanwhile. + */ + if (dcpaud_holds(dcpaud) && READ_ONCE(dcpaud->stream_dma) && + substream->runtime->channels == 2 && substream->runtime->dma_area) { + memset(substream->runtime->dma_area, 0, substream->runtime->dma_bytes); + WRITE_ONCE(dcpaud->dma_cleared, true); + return 0; + } + fallthrough; case SNDRV_PCM_TRIGGER_SUSPEND: - ret = snd_dmaengine_pcm_trigger(substream, cmd); + /* A START that issued no DMA leaves a held link's silence alone. */ + ret = READ_ONCE(dcpaud->stream_dma) ? snd_dmaengine_pcm_trigger(substream, cmd) : 0; pm_runtime_mark_last_busy(dcpaud->dev); __pm_runtime_put_autosuspend(dcpaud->dev); return ret; @@ -781,6 +1011,24 @@ static int dcp_pcm_native_sync_stop(struct snd_pcm_substream *substream) struct dcp_audio *dcpaud = substream->pcm->private_data; int ret; + WRITE_ONCE(dcpaud->stream_dma, false); + if (READ_ONCE(dcpaud->dma_cleared)) { + guard(mutex)(&dcpaud->link_lock); + WRITE_ONCE(dcpaud->dma_cleared, false); + dmaengine_terminate_async(dcpaud->chan); + /* Queued first, the silence follows the stream without a gap. */ + if (dcpaud->silence_running || !dcpaud->link_started || + !dcp_audiosrv_link_live(dcpaud->dcp_dev) || dcpaud_silence_start(dcpaud)) { + dmaengine_synchronize(dcpaud->chan); + dcpaud_link_teardown(dcpaud); + pm_runtime_mark_last_busy(dcpaud->dev); + pm_runtime_put_autosuspend(dcpaud->dev); + return 0; + } + dmaengine_synchronize(dcpaud->chan); + return 0; + } + /* SIO TERMINATE acknowledged and the issued periods reported back. */ ret = snd_dmaengine_pcm_sync_stop(substream); @@ -812,6 +1060,10 @@ void dcpaud_release_link(struct platform_device *pdev, const u64 *gen) /* Only that instance's link: a later instance's PCM keeps its own. */ if (gen && !dcp_audiosrv_link_on(dcpaud->dcp_dev, *gen)) return; + dcpaud_silence_stop(dcpaud); + /* A stopped stream still playing its cleared buffer: sync_stop finishes it. */ + if (dcpaud->chan && READ_ONCE(dcpaud->dma_cleared)) + dmaengine_terminate_async(dcpaud->chan); if (dcpaud->chan) dmaengine_synchronize(dcpaud->chan); dcpaud_link_teardown(dcpaud); @@ -905,8 +1157,10 @@ static void dcpaud_report_hotplug(struct dcp_audio *dcpaud, bool connected) } dcpaud->connected = connected; - if (connected) + if (connected) { dcpaud->connection_cookie++; + dcpaud->connected_at = ktime_get(); + } mutex_unlock(&dcpaud->data_lock); snd_jack_report(dcpaud->jack, connected ? SND_JACK_AVOUT : 0); @@ -1010,6 +1264,11 @@ void dcpaud_disconnect(struct platform_device *pdev) mutex_lock(&dcpaud->data_lock); dcpaud_report_hotplug(dcpaud, false); +#if IS_ENABLED(CONFIG_DRM_APPLE_DCP_T6030_AUDIO) + /* The service may go without a release, and a held link with it. */ + if (dcpaud->native_link) + schedule_work(&dcpaud->stale_work); +#endif } static int dcpaud_comp_bind(struct device *dev, struct device *main, void *data) @@ -1112,6 +1371,27 @@ static void dcpaud_comp_unbind(struct device *dev, struct device *main, /* snd_card_free_when_closed() checks for NULL */ snd_card_free_when_closed(dcpaud->card); +#if IS_ENABLED(CONFIG_DRM_APPLE_DCP_T6030_AUDIO) + if (dcpaud->native_link) { + bool held; + + cancel_work_sync(&dcpaud->stale_work); + /* A held link ends with the card; an open PCM's close finds it gone. */ + scoped_guard(mutex, &dcpaud->link_lock) { + held = dcpaud->silence_running; + if (held) + dcpaud_link_teardown(dcpaud); + if (dcpaud->silence) { + dma_free_coherent(dcpaud->chan->device->dev, DCPAUD_SILENCE_BYTES, + dcpaud->silence, dcpaud->silence_addr); + dcpaud->silence = NULL; + } + } + if (held) + dcp_audiosrv_close(dcpaud->dcp_dev); + } +#endif + if (dcpaud->dma_link) device_link_del(dcpaud->dma_link); } @@ -1143,6 +1423,7 @@ static int dcpaud_probe(struct platform_device *pdev) mutex_init(&dcpaud->data_lock); #if IS_ENABLED(CONFIG_DRM_APPLE_DCP_T6030_AUDIO) mutex_init(&dcpaud->link_lock); + INIT_WORK(&dcpaud->stale_work, dcpaud_stale_work); #endif INIT_DELAYED_WORK(&dcpaud->void_work, dcpaud_void_work); platform_set_drvdata(pdev, dcpaud); diff --git a/drivers/gpu/drm/apple/audio.h b/drivers/gpu/drm/apple/audio.h index dcf752efb9a121..259703b55fb287 100644 --- a/drivers/gpu/drm/apple/audio.h +++ b/drivers/gpu/drm/apple/audio.h @@ -37,6 +37,8 @@ void dcpaud_release_link(struct platform_device *pdev, const u64 *gen); bool dcp_audiosrv_link_on(struct device *dev, u64 gen); /* The prepared link still belongs to the client of the published instance. */ bool dcp_audiosrv_link_live(struct device *dev); +/* The display processor drives a Type-C route rather than the HDMI port. */ +bool dcp_audiosrv_typec(struct device *dev); #endif #endif /* __AUDIO_H__ */ diff --git a/drivers/gpu/drm/apple/av.c b/drivers/gpu/drm/apple/av.c index c0bacc9956d31e..e5b987be12c2b8 100644 --- a/drivers/gpu/drm/apple/av.c +++ b/drivers/gpu/drm/apple/av.c @@ -400,6 +400,12 @@ bool dcp_audiosrv_link_live(struct device *dev) return asrv->link_gen == asrv->open_gen && av_audiosrv_open_published(asrv); } +bool dcp_audiosrv_typec(struct device *dev) +{ + KUNIT_STATIC_STUB_REDIRECT(dcp_audiosrv_typec, dev); + return dcp_is_typec_output(dev_get_drvdata(dev)); +} + /* * The link commands of the 14.7 external role's audio service, the native * audio driver's: prepare, start, stop and complete all take the selected From 5ab5f59ee8312523c85fc479cc0c2f847aaf2803 Mon Sep 17 00:00:00 2001 From: Eryk Wieliczko Date: Wed, 28 Oct 2026 02:33:53 +0100 Subject: [PATCH 6/9] arm64: dts: apple: t603x-j514s-j516s: add Type-C DisplayPort audio Link the Type-C DisplayPort processor (dcpext1) to DPA2 on SIO channel 0x68, as the ADT links dcpext1 to dp-audio2, so a display on a USB-C port gets an audio device too. Signed-off-by: Eryk Wieliczko Assisted-by: Claude:claude-opus-5-5 --- .../dts/apple/t603x-j514s-j516s-usb4.dtsi | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/arch/arm64/boot/dts/apple/t603x-j514s-j516s-usb4.dtsi b/arch/arm64/boot/dts/apple/t603x-j514s-j516s-usb4.dtsi index b6c397e17c9057..7ba4fed43c0ad9 100644 --- a/arch/arm64/boot/dts/apple/t603x-j514s-j516s-usb4.dtsi +++ b/arch/arm64/boot/dts/apple/t603x-j514s-j516s-usb4.dtsi @@ -70,3 +70,41 @@ }; }; +/* + * DPA2: the Type-C DisplayPort processor's (dcpext1) audio, SIO channel 0x68, + * as the ADT links dcpext1 to dp-audio2. + */ +&dcpext1 { + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + dcpext1_audio: endpoint { + remote-endpoint = <&dpaudio2_dcpext1>; + }; + }; + }; +}; + +&soc { + dpaudio2: audio-controller-2 { + compatible = "apple,t6030-dpaudio", "apple,dpaudio"; + dmas = <&sio 0x68>; + dma-names = "tx"; + power-domains = <&ps_dpa2>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + dpaudio2_dcpext1: endpoint { + remote-endpoint = <&dcpext1_audio>; + }; + }; + }; + }; +}; From 0ec6112931eceaaec119d04283ed838f70fa5678 Mon Sep 17 00:00:00 2001 From: Eryk Wieliczko Date: Wed, 28 Oct 2026 06:47:17 +0100 Subject: [PATCH 7/9] drm/apple: hold the HDMI audio link at one rate and sample size PipeWire changes the device rate to follow its streams: 44.1 kHz music, 48 kHz video. A held link of another rate or sample size was replaced at START by one with the same channel layout, which the J514S sink mutes like a same-format restart. After 48 kHz then 44.1 kHz streams, every stream was silent until a replug. macOS runs the link at one nominal rate and format and converts streams to it. While links are held, the PCM now offers one rate (48 kHz, else 44.1 or 32 kHz) and one sample size (24 bits, else 16), taken from one element that offers them together in stereo, and userspace resamples. The pair covers every channel layout: userspace sets the format before the channel count, so a pair applied only to stereo let it rule stereo out. Channel layouts stay free: a layout change is heard. Signed-off-by: Eryk Wieliczko Assisted-by: Claude:claude-opus-5-5 --- drivers/gpu/drm/apple/audio.c | 74 +++++++++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/drivers/gpu/drm/apple/audio.c b/drivers/gpu/drm/apple/audio.c index f3bd62d3a8375a..0fc2cf136abe2c 100644 --- a/drivers/gpu/drm/apple/audio.c +++ b/drivers/gpu/drm/apple/audio.c @@ -669,6 +669,75 @@ static bool dcpaud_holds(struct dcp_audio *dcpaud) return hold >= 2 || (hold == 1 && !dcp_audiosrv_typec(dcpaud->dcp_dev)); } +/* + * A change of sample rate or sample size with an unchanged channel layout + * replaces the link with a new one, which this sink mutes just as it does a + * same-format restart. PipeWire changes the rate to follow its streams + * (44.1 kHz music, 48 kHz video). macOS runs the link at one nominal rate + * and format and converts the streams to it; so does this driver while it + * holds links: the PCM offers one rate (48 kHz, else 44.1 or 32 kHz) and one + * sample size (24 bits, else 16), both of one element that takes them in + * stereo, and userspace resamples. Channel layouts stay free: a layout + * change is heard. + * + * The pair covers every layout, not only the stereo that links are held in: + * userspace commonly sets the sample format before the channel count, and a + * pair applied only once a stream is stereo let it set a format that then + * ruled stereo out (PipeWire built no HDMI sink). + */ +static int dcpaud_pin_rate_format(struct dcp_audio *dcpaud, + struct snd_pcm_runtime *runtime) +{ + static const unsigned int rates[] = { 48000, 44100, 32000 }; + static const snd_pcm_format_t formats[] = { + SNDRV_PCM_FORMAT_S24_LE, SNDRV_PCM_FORMAT_S16_LE, + }; + struct dcp_sound_format_mask sieve = { + .formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE, + .rates = GENMASK(snd_pcm_known_rates.count - 1, 0), + .nchans = BIT(2), + }; + struct dcp_sound_format_mask hits = { 0, 0, 0 }; + struct dcp_parse_ctx elements = { + .dcp = dev_get_drvdata(dcpaud->dcp_dev), + .blob = dcpaud->elements + 4, + .len = dcpaud_elements_size(dcpaud) - 4, + .pos = 0, + .v14_7_audio = dcpaud->native_link, + }; + unsigned int rate = 0, rate_bit = 0, i, j; + int ret; + + parse_sound_constraints(&elements, &sieve, &hits); + for (i = 0; i < ARRAY_SIZE(rates) && !rate; i++) + for (j = 0; j < snd_pcm_known_rates.count; j++) + if (snd_pcm_known_rates.list[j] == rates[i] && (hits.rates & BIT(j))) { + rate = rates[i]; + rate_bit = j; + } + if (!rate) + return 0; /* no stereo LPCM element: the sink decides */ + + /* The sample size of an element that takes that rate in stereo */ + sieve.rates = BIT(rate_bit); + elements.pos = 0; + parse_sound_constraints(&elements, &sieve, &hits); + for (i = 0; i < ARRAY_SIZE(formats); i++) + if (hits.formats & pcm_format_to_bits(formats[i])) + break; + if (i == ARRAY_SIZE(formats)) + return 0; + + dev_dbg(dcpaud->dev, "held links run at %u Hz, format %d\n", rate, + (__force int)formats[i]); + ret = snd_pcm_hw_constraint_single(runtime, SNDRV_PCM_HW_PARAM_RATE, rate); + if (ret < 0) + return ret; + ret = snd_pcm_hw_constraint_mask64(runtime, SNDRV_PCM_HW_PARAM_FORMAT, + pcm_format_to_bits(formats[i])); + return ret < 0 ? ret : 0; +} + /* * Seen once, at boot: a link started about 0.2 s after the display came on * was not heard, and holding it kept it silent; the next connection's first @@ -753,6 +822,11 @@ static int dcp_pcm_native_open(struct snd_pcm_substream *substream) } ret = dcp_pcm_open(substream); + if (!ret && live && !dcpaud->open_unplugged && dcpaud_holds(dcpaud)) { + ret = dcpaud_pin_rate_format(dcpaud, substream->runtime); + if (ret) + dcp_pcm_close(substream); + } if (ret && live) { bool held; From f4dd5a24825605d3ee8a791d7621a5ca5bfbaa74 Mon Sep 17 00:00:00 2001 From: Eryk Wieliczko Date: Wed, 28 Oct 2026 08:05:18 +0100 Subject: [PATCH 8/9] dmaengine: apple-sio: hand over to an idle transfer without a gap A DP audio sink behind the J514S HDMI port mutes for good when its samples stop for 20 ms or more, until the DP link trains again. Swapping one cyclic transfer for another stops the channel with TERMINATE and starts the next one after its acknowledgment: usually short enough, sometimes not. Let a client set an idle transfer (struct apple_sio_idle, passed as peripheral_config in a slave config call of its own). The channel issues it while no other transfer runs. Terminating a transfer then hands over to it after the issued descriptors, with no TERMINATE, and a transfer issued later follows its issued descriptors the same way. From its first idle transfer until it is quiet again, a channel accounts for its issued descriptors in a ring, seeded with what it has in flight: reports complete them in issue order, so each is charged to the transfer that issued it, and one ISSUE is outstanding at a time. dmaengine_synchronize() waits for an ended transfer's descriptors. If they do not complete within 500 ms, it stops the channel with TERMINATE and waits for their reports again, and resets the ring if they never come. Removing the idle transfer does the same, and fails if the channel cannot be confirmed stopped. A channel that never had an idle transfer keeps the old accounting unchanged: the DP audio of M1 and M2 Macs sets none. Signed-off-by: Eryk Wieliczko Assisted-by: Claude:claude-opus-5-5 --- MAINTAINERS | 1 + drivers/dma/apple-sio.c | 569 ++++++++++++++++++++++++++++++++-- include/linux/dma/apple-sio.h | 44 +++ 3 files changed, 580 insertions(+), 34 deletions(-) create mode 100644 include/linux/dma/apple-sio.h diff --git a/MAINTAINERS b/MAINTAINERS index 1d10b8f9158edc..491823243a5fc1 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2636,6 +2636,7 @@ F: drivers/dma/apple-sio-probe-test.dtso F: drivers/dma/apple-sio-t6030-test.dtso F: drivers/dma/apple-sio-test.c F: drivers/dma/apple-sio.c +F: include/linux/dma/apple-sio.h F: sound/soc/apple/* F: sound/soc/codecs/cs42l83-i2c.c F: sound/soc/codecs/cs42l84.* diff --git a/drivers/dma/apple-sio.c b/drivers/dma/apple-sio.c index 5a125ebb854f7f..b212460a125bb1 100644 --- a/drivers/dma/apple-sio.c +++ b/drivers/dma/apple-sio.c @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -21,6 +22,7 @@ #include #include #include +#include #include "dmaengine.h" #include "virt-dma.h" @@ -103,6 +105,38 @@ struct sio_chan { struct sio_shmem_chan_config cfg; struct sio_tx *current_tx; + + /* + * Until a client sets an idle transfer, the channel keeps the old + * accounting, charging ACKs and REPORTs to the current transfer; its + * ISSUEs not yet acknowledged are only counted. + */ + unsigned int pending; + + /* + * The ring accounting, from the first idle transfer until the channel + * is quiet again (sio_ring_leave_locked()): the client's idle transfer + * (struct apple_sio_idle), issued while no other transfer is; + * transfers ended while it is set, until their issued descriptors + * completed; and the owners of the issued descriptors in issue order, + * which is the order of their reports. The last one may still wait for + * its ISSUE to be acknowledged. + */ + bool ring; + struct sio_tx *idle; + struct list_head retired; + wait_queue_head_t drained; + struct sio_tx *issued[SIO_MAX_NINFLIGHT]; + unsigned int issued_head; + unsigned int nissued; + bool issuing; + /* Nothing is issued between a TERMINATE and the ring's reset. */ + bool stopping; + /* + * Ended transfers with nothing in flight, freed once no completion + * callback can still be running (sio_free_dead()). + */ + struct list_head dead; }; #define SIO_NTAGS 16 @@ -142,6 +176,7 @@ struct sio_tx { struct completion done; bool terminated; + bool retired; size_t period_len; int nperiods; int ninflight; @@ -426,23 +461,22 @@ static void sio_tx_free(struct virt_dma_desc *vd) kfree(siotx); } -static struct dma_async_tx_descriptor *sio_prep_dma_cyclic( - struct dma_chan *chan, dma_addr_t buf_addr, size_t buf_len, - size_t period_len, enum dma_transfer_direction direction, - unsigned long flags) +static struct sio_tx *sio_alloc_tx(struct sio_chan *siochan, dma_addr_t buf_addr, + size_t buf_len, size_t period_len, gfp_t gfp) { - struct sio_chan *siochan = to_sio_chan(chan); struct sio_tx *siotx = NULL; - int i, nperiods = buf_len / period_len; + int i, nperiods; - if (direction != sio_chan_direction(siochan->no)) + if (!period_len || buf_len < period_len) return NULL; + nperiods = buf_len / period_len; - siotx = kzalloc(struct_size(siotx, siodesc, nperiods), GFP_NOWAIT); + siotx = kzalloc(struct_size(siotx, siodesc, nperiods), gfp); if (!siotx) return NULL; init_completion(&siotx->done); + siotx->vd.tx.chan = &siochan->vc.chan; siotx->period_len = period_len; siotx->nperiods = nperiods; @@ -451,7 +485,6 @@ static struct dma_async_tx_descriptor *sio_prep_dma_cyclic( siotx->siodesc[i] = d = sio_alloc_desc(siochan->host); if (!d) { - siotx->vd.tx.chan = &siochan->vc.chan; sio_tx_free(&siotx->vd); return NULL; } @@ -468,6 +501,24 @@ static struct dma_async_tx_descriptor *sio_prep_dma_cyclic( } dma_wmb(); + return siotx; +} + +static struct dma_async_tx_descriptor *sio_prep_dma_cyclic( + struct dma_chan *chan, dma_addr_t buf_addr, size_t buf_len, + size_t period_len, enum dma_transfer_direction direction, + unsigned long flags) +{ + struct sio_chan *siochan = to_sio_chan(chan); + struct sio_tx *siotx; + + if (direction != sio_chan_direction(siochan->no)) + return NULL; + + siotx = sio_alloc_tx(siochan, buf_addr, buf_len, period_len, GFP_NOWAIT); + if (!siotx) + return NULL; + return vchan_tx_prep(&siochan->vc, &siotx->vd, flags); } @@ -512,32 +563,172 @@ static enum dma_status sio_tx_status(struct dma_chan *chan, dma_cookie_t cookie, static bool sio_fill_in_locked(struct sio_chan *siochan); -static void sio_handle_issue_ack(struct sio_chan *siochan, void *cookie, bool ok) +static struct sio_tx **sio_issued_slot(struct sio_chan *siochan, unsigned int i) { - dma_cookie_t tx_cookie = (unsigned long) cookie; - unsigned long flags; - struct sio_tx *tx; + return &siochan->issued[(siochan->issued_head + i) % SIO_MAX_NINFLIGHT]; +} + +/* Neither in flight nor waiting for its ISSUE to be acknowledged */ +static bool sio_tx_quiet_locked(struct sio_chan *siochan, struct sio_tx *tx) +{ + unsigned int i; + + if (tx->ninflight) + return false; + for (i = 0; i < siochan->nissued; i++) + if (*sio_issued_slot(siochan, i) == tx) + return false; + return true; +} + +/* A transfer that is freed no longer owns issued descriptors */ +static void sio_scrub_locked(struct sio_chan *siochan, struct sio_tx *tx) +{ + unsigned int i; + + for (i = 0; i < siochan->nissued; i++) + if (*sio_issued_slot(siochan, i) == tx) + *sio_issued_slot(siochan, i) = NULL; +} + +/* Descriptors are issued from the current transfer, else from the idle one. */ +static struct sio_tx *sio_source_locked(struct sio_chan *siochan) +{ + struct sio_tx *tx = siochan->current_tx; + if (tx) + return tx->terminated ? NULL : tx; + if (siochan->idle && !siochan->idle->terminated) + return siochan->idle; + return NULL; +} + +/* + * The first idle transfer: the ring accounting starts from what the old one + * has in flight, all of it the current transfer's. It cannot place two ISSUEs + * still waiting for their acks, nor an ended transfer's descriptors. + */ +static int sio_ring_enter_locked(struct sio_chan *siochan) +{ + struct sio_tx *tx = siochan->current_tx; + unsigned int i, n = tx ? tx->ninflight : 0; + + if (siochan->pending > 1 || (siochan->pending && !tx) || (tx && tx->terminated) || + n + siochan->pending > SIO_MAX_NINFLIGHT) + return -EBUSY; + siochan->issued_head = 0; + siochan->nissued = n + siochan->pending; + for (i = 0; i < siochan->nissued; i++) + siochan->issued[i] = tx; + siochan->issuing = siochan->pending; + siochan->pending = 0; + siochan->ring = true; + return 0; +} + +/* + * The resync point: once the channel is quiet, with no idle, current or + * retired transfer and nothing issued, the old accounting resumes. + */ +static void sio_ring_leave_locked(struct sio_chan *siochan) +{ + if (!siochan->ring || siochan->idle || siochan->current_tx || + !list_empty(&siochan->retired) || siochan->nissued || siochan->issuing) + return; + siochan->ring = false; + siochan->issued_head = 0; +} + +/* + * An ended transfer with no descriptor left in flight: a retired one goes to + * be freed, the waiter of another one is woken. + */ +static void sio_settle_locked(struct sio_chan *siochan, struct sio_tx *tx) +{ + if (!tx || !tx->terminated || !sio_tx_quiet_locked(siochan, tx)) + return; + if (tx->retired) { + list_move_tail(&tx->vd.node, &siochan->dead); + wake_up(&siochan->drained); + return; + } + /* sio_terminate_work() or sio_clear_idle() */ + complete(&tx->done); +} + +/* + * The old accounting, for channels that never had an idle transfer: an ACK + * is the current transfer's, if its cookie still is. + */ +static void sio_old_issue_ack_locked(struct sio_chan *siochan, dma_cookie_t tx_cookie, bool ok) +{ + struct sio_tx *tx = siochan->current_tx; + + if (siochan->pending) + siochan->pending--; if (!ok) { dev_err(siochan->host->dev, "nacked issue on chan %d\n", siochan->no); return; } + if (!tx || tx_cookie != tx->vd.tx.cookie || tx->terminated) + return; - spin_lock_irqsave(&siochan->vc.lock, flags); - if (!siochan->current_tx || tx_cookie != siochan->current_tx->vd.tx.cookie || - siochan->current_tx->terminated) - goto out; - - tx = siochan->current_tx; tx->next = (tx->next + 1) % tx->nperiods; tx->ninflight++; sio_fill_in_locked(siochan); +} + +/* The ring accounting: an ACK is the last issued descriptor's. */ +static void sio_ring_issue_ack_locked(struct sio_chan *siochan, bool ok) +{ + struct sio_tx *tx; -out: + siochan->issuing = false; + if (!siochan->nissued) { + dev_err_ratelimited(siochan->host->dev, "chan %d: ack without an issued descriptor\n", + siochan->no); + return; + } + + tx = *sio_issued_slot(siochan, siochan->nissued - 1); + if (!ok) { + dev_err(siochan->host->dev, "nacked issue on chan %d\n", siochan->no); + siochan->nissued--; + if (!siochan->nissued) + wake_up(&siochan->drained); + /* + * During a handover this may have been the ended transfer's + * last ISSUE, and no report is left to start the next one. A + * transfer whose own ISSUE was refused is not issued again. + */ + if (sio_source_locked(siochan) != tx) + sio_fill_in_locked(siochan); + sio_settle_locked(siochan, tx); + return; + } + + /* An ended transfer's descriptor is in flight all the same. */ + if (tx) { + tx->next = (tx->next + 1) % tx->nperiods; + tx->ninflight++; + } + sio_fill_in_locked(siochan); +} + +static void sio_handle_issue_ack(struct sio_chan *siochan, void *cookie, bool ok) +{ + unsigned long flags; + + spin_lock_irqsave(&siochan->vc.lock, flags); + if (siochan->ring) + sio_ring_issue_ack_locked(siochan, ok); + else + sio_old_issue_ack_locked(siochan, (unsigned long)cookie, ok); spin_unlock_irqrestore(&siochan->vc.lock, flags); } -static bool sio_fill_in_locked(struct sio_chan *siochan) +/* The old accounting: the current transfer's next descriptor */ +static bool sio_fill_in_old_locked(struct sio_chan *siochan) { struct sio_data *sio = siochan->host; struct sio_tx *tx = siochan->current_tx; @@ -551,13 +742,50 @@ static bool sio_fill_in_locked(struct sio_chan *siochan) ret = sio_send_siomsg_atomic(sio, FIELD_PREP(SIOMSG_EP, siochan->no) | FIELD_PREP(SIOMSG_TYPE, MSG_ISSUE) | FIELD_PREP(SIOMSG_DATA, sio_coproc_desc_slot(sio, d)), - sio_handle_issue_ack, (void *) (uintptr_t) tx->vd.tx.cookie); + sio_handle_issue_ack, (void *)(uintptr_t)tx->vd.tx.cookie); if (ret < 0) dev_err_ratelimited(sio->dev, "can't issue on chan %d ninflight %d: %d\n", siochan->no, tx->ninflight, ret); + else + siochan->pending++; + return true; +} + +/* The ring accounting: one ISSUE at a time, each after the previous one's ack */ +static bool sio_fill_in_ring_locked(struct sio_chan *siochan) +{ + struct sio_data *sio = siochan->host; + struct sio_tx *tx = sio_source_locked(siochan); + struct sio_coproc_desc *d; + int ret; + + if (!tx || siochan->issuing || siochan->stopping || + siochan->nissued >= SIO_MAX_NINFLIGHT) + return false; + + d = tx->siodesc[tx->next]; + ret = sio_send_siomsg_atomic(sio, FIELD_PREP(SIOMSG_EP, siochan->no) | + FIELD_PREP(SIOMSG_TYPE, MSG_ISSUE) | + FIELD_PREP(SIOMSG_DATA, sio_coproc_desc_slot(sio, d)), + sio_handle_issue_ack, NULL); + if (ret < 0) { + dev_err_ratelimited(sio->dev, "can't issue on chan %d ninflight %d: %d\n", + siochan->no, siochan->nissued, ret); + return false; + } + /* The ack takes the channel lock, held here. */ + *sio_issued_slot(siochan, siochan->nissued++) = tx; + siochan->issuing = true; return true; } +static bool sio_fill_in_locked(struct sio_chan *siochan) +{ + if (siochan->ring) + return sio_fill_in_ring_locked(siochan); + return siochan->current_tx && sio_fill_in_old_locked(siochan); +} + static void sio_update_current_tx_locked(struct sio_chan *siochan) { struct virt_dma_desc *vd = vchan_next_desc(&siochan->vc); @@ -583,14 +811,37 @@ static void sio_issue_pending(struct dma_chan *chan) static int sio_terminate_all(struct dma_chan *chan) { struct sio_chan *siochan = to_sio_chan(chan); + struct sio_tx *tx; unsigned long flags; LIST_HEAD(to_free); spin_lock_irqsave(&siochan->vc.lock, flags); - if (siochan->current_tx && !siochan->current_tx->terminated) { - dma_cookie_complete(&siochan->current_tx->vd.tx); - siochan->current_tx->terminated = true; - schedule_work(&siochan->terminate_wq); + tx = siochan->current_tx; + if (tx && !tx->terminated) { + dma_cookie_complete(&tx->vd.tx); + tx->terminated = true; + if (siochan->ring && siochan->idle && !siochan->idle->terminated) { + /* + * No TERMINATE: the idle transfer follows the issued + * descriptors, and the peripheral runs on without a gap. + * sio_synchronize() waits for those to complete. + */ + if (siochan->vc.cyclic == &tx->vd) + siochan->vc.cyclic = NULL; + siochan->current_tx = NULL; + /* Its callback may be running: sio_free_dead() frees it. */ + if (sio_tx_quiet_locked(siochan, tx)) { + list_add_tail(&tx->vd.node, &siochan->dead); + } else { + tx->retired = true; + list_add_tail(&tx->vd.node, &siochan->retired); + } + sio_fill_in_locked(siochan); + } else { + if (siochan->ring && sio_tx_quiet_locked(siochan, tx)) + complete(&tx->done); + schedule_work(&siochan->terminate_wq); + } } vchan_get_all_descriptors(&siochan->vc, &to_free); spin_unlock_irqrestore(&siochan->vc.lock, flags); @@ -629,23 +880,223 @@ static void sio_terminate_work(struct work_struct *wq) spin_lock_irqsave(&siochan->vc.lock, flags); WARN_ON(siochan->current_tx != tx); siochan->current_tx = NULL; + sio_scrub_locked(siochan, tx); sio_update_current_tx_locked(siochan); + sio_ring_leave_locked(siochan); spin_unlock_irqrestore(&siochan->vc.lock, flags); sio_tx_free(&tx->vd); } +static bool sio_drained(struct sio_chan *siochan) +{ + unsigned long flags; + bool drained; + + spin_lock_irqsave(&siochan->vc.lock, flags); + drained = list_empty(&siochan->retired); + spin_unlock_irqrestore(&siochan->vc.lock, flags); + + return drained; +} + +static bool sio_ring_empty(struct sio_chan *siochan) +{ + unsigned long flags; + bool empty; + + spin_lock_irqsave(&siochan->vc.lock, flags); + empty = !siochan->nissued && !siochan->issuing; + spin_unlock_irqrestore(&siochan->vc.lock, flags); + + return empty; +} + +/* + * Issued descriptors that did not complete in time: stop the channel and wait + * for their reports, so that their buffers are no longer read. Nothing is + * issued from the TERMINATE until the channel is known stopped. A channel + * that acknowledged TERMINATE and still reports nothing is resynced: its ring + * is emptied, as nothing it holds is played any more. A channel that refused + * TERMINATE or did not answer cannot be known to have stopped: -EIO, and its + * transfers stay as they are. Then the current or idle transfer is issued + * again. + */ +static int sio_stop_quiet(struct sio_chan *siochan) +{ + struct device *dev = siochan->host->dev; + unsigned long flags; + struct sio_tx *tx; + unsigned int i; + int ret; + + dev_err(dev, "chan %d: issued descriptors did not complete, stopping it\n", siochan->no); + spin_lock_irqsave(&siochan->vc.lock, flags); + siochan->stopping = true; + spin_unlock_irqrestore(&siochan->vc.lock, flags); + + /* 1 on ACK, 0 on NACK, negative without an answer */ + ret = sio_call(siochan->host, FIELD_PREP(SIOMSG_EP, siochan->no) | + FIELD_PREP(SIOMSG_TYPE, MSG_TERMINATE)); + if (ret == 1) + wait_event_timeout(siochan->drained, sio_ring_empty(siochan), + msecs_to_jiffies(500)); + else if (!ret) + dev_err(dev, "chan %d: TERMINATE refused, its buffers may still be read\n", + siochan->no); + else + dev_err(dev, "chan %d: not stopped (%d), its buffers may still be read\n", + siochan->no, ret); + + spin_lock_irqsave(&siochan->vc.lock, flags); + if (ret == 1 && (siochan->nissued || siochan->issuing)) { + dev_err(dev, "chan %d: stopped with %u descriptors unreported, ring reset\n", + siochan->no, siochan->nissued); + for (i = 0; i < siochan->nissued; i++) { + tx = *sio_issued_slot(siochan, i); + if (tx) + tx->ninflight = 0; + } + siochan->nissued = 0; + siochan->issuing = false; + siochan->issued_head = 0; + } + if (ret == 1) { + list_splice_tail_init(&siochan->retired, &siochan->dead); + if (siochan->idle && siochan->idle->terminated) + complete(&siochan->idle->done); + } + siochan->stopping = false; + sio_fill_in_locked(siochan); + spin_unlock_irqrestore(&siochan->vc.lock, flags); + + return ret == 1 ? 0 : -EIO; +} + +/* Frees the ended transfers once no completion callback can still use them. */ +static void sio_free_dead(struct sio_chan *siochan) +{ + struct sio_tx *tx, *n; + unsigned long flags; + LIST_HEAD(dead); + + spin_lock_irqsave(&siochan->vc.lock, flags); + list_splice_init(&siochan->dead, &dead); + spin_unlock_irqrestore(&siochan->vc.lock, flags); + if (list_empty(&dead)) + return; + + /* vchan_complete() runs a callback with its descriptor unlocked. */ + tasklet_kill(&siochan->vc.task); + list_for_each_entry_safe(tx, n, &dead, vd.node) { + list_del(&tx->vd.node); + sio_tx_free(&tx->vd); + } +} + static void sio_synchronize(struct dma_chan *chan) { struct sio_chan *siochan = to_sio_chan(chan); flush_work(&siochan->terminate_wq); + if (READ_ONCE(siochan->ring)) { + /* The caller frees the buffer on return; nothing can stop that. */ + if (!wait_event_timeout(siochan->drained, sio_drained(siochan), + msecs_to_jiffies(500)) && + sio_stop_quiet(siochan)) + dev_WARN_ONCE(siochan->host->dev, 1, + "chan %d: synchronized without being stopped\n", + siochan->no); + /* No callback of an ended transfer runs after this. */ + tasklet_kill(&siochan->vc.task); + } + sio_free_dead(siochan); +} + +/* + * Returns once the idle transfer's descriptors completed. If the channel + * cannot be confirmed stopped, the idle transfer stays, its buffer in use. + */ +static int sio_clear_idle(struct sio_chan *siochan) +{ + struct sio_tx *idle; + unsigned long flags; + bool quiet, stop; + int ret; + + spin_lock_irqsave(&siochan->vc.lock, flags); + idle = siochan->idle; + if (!idle) { + spin_unlock_irqrestore(&siochan->vc.lock, flags); + return 0; + } + idle->terminated = true; + quiet = sio_tx_quiet_locked(siochan, idle); + /* Nothing follows the idle transfer: the channel stops. */ + stop = !quiet && !siochan->current_tx; + spin_unlock_irqrestore(&siochan->vc.lock, flags); + + if (stop) { + ret = sio_call(siochan->host, FIELD_PREP(SIOMSG_EP, siochan->no) | + FIELD_PREP(SIOMSG_TYPE, MSG_TERMINATE)); + if (ret < 0) + dev_err(siochan->host->dev, "terminate call on chan %d failed: %d\n", + siochan->no, ret); + } + if (!quiet && !wait_for_completion_timeout(&idle->done, msecs_to_jiffies(500))) { + ret = sio_stop_quiet(siochan); + if (ret) + return ret; + } + + spin_lock_irqsave(&siochan->vc.lock, flags); + siochan->idle = NULL; + sio_scrub_locked(siochan, idle); + sio_ring_leave_locked(siochan); + spin_unlock_irqrestore(&siochan->vc.lock, flags); + + sio_tx_free(&idle->vd); + sio_free_dead(siochan); + return 0; +} + +/* See struct apple_sio_idle */ +static int sio_set_idle(struct sio_chan *siochan, const struct apple_sio_idle *config) +{ + struct sio_tx *idle; + unsigned long flags; + int ret; + + if (!config->len) + return sio_clear_idle(siochan); + if (!config->period_len || config->len < config->period_len) + return -EINVAL; + + idle = sio_alloc_tx(siochan, config->addr, config->len, config->period_len, GFP_KERNEL); + if (!idle) + return -ENOMEM; + + spin_lock_irqsave(&siochan->vc.lock, flags); + ret = siochan->idle ? -EBUSY : 0; + if (!ret && !siochan->ring) + ret = sio_ring_enter_locked(siochan); + if (ret) { + spin_unlock_irqrestore(&siochan->vc.lock, flags); + sio_tx_free(&idle->vd); + return ret; + } + siochan->idle = idle; + sio_fill_in_locked(siochan); + spin_unlock_irqrestore(&siochan->vc.lock, flags); + + return 0; } static void sio_free_chan_resources(struct dma_chan *chan) { sio_terminate_all(chan); sio_synchronize(chan); + sio_clear_idle(to_sio_chan(chan)); vchan_free_chan_resources(&to_sio_chan(chan)->vc); } @@ -668,20 +1119,61 @@ static void sio_rtk_crashed(void *cookie, const void *crashlog, size_t crashlog_ dev_err(sio->dev, "SIO down (crashed)"); } -static void sio_process_report(struct sio_chan *siochan) +/* The old accounting: a report is the current transfer's. */ +static void sio_old_report_locked(struct sio_chan *siochan) { - unsigned long flags; + struct sio_tx *tx = siochan->current_tx; - spin_lock_irqsave(&siochan->vc.lock, flags); - if (siochan->current_tx) { - struct sio_tx *tx = siochan->current_tx; + if (!tx) + return; + if (tx->ninflight) + tx->ninflight--; + vchan_cyclic_callback(&tx->vd); + if (!sio_fill_in_locked(siochan) && !tx->ninflight) + complete(&tx->done); +} + +/* The ring accounting: the oldest issued descriptor completed. */ +static void sio_ring_report_locked(struct sio_chan *siochan) +{ + struct sio_tx *tx; + /* An ISSUE not yet acknowledged has no completed descriptor. */ + if (siochan->nissued <= (siochan->issuing ? 1 : 0)) { + dev_warn_ratelimited(siochan->host->dev, "chan %d: report without a descriptor\n", + siochan->no); + return; + } + + tx = *sio_issued_slot(siochan, 0); + siochan->issued_head = (siochan->issued_head + 1) % SIO_MAX_NINFLIGHT; + siochan->nissued--; + if (tx) { if (tx->ninflight) tx->ninflight--; - vchan_cyclic_callback(&tx->vd); - if (!sio_fill_in_locked(siochan) && !tx->ninflight) - complete(&tx->done); + else + dev_err_ratelimited(siochan->host->dev, + "chan %d: report for a descriptor not in flight\n", + siochan->no); + if (tx == siochan->current_tx && !tx->terminated) + vchan_cyclic_callback(&tx->vd); } + sio_fill_in_locked(siochan); + sio_settle_locked(siochan, tx); + if (!siochan->nissued) + wake_up(&siochan->drained); + sio_ring_leave_locked(siochan); +} + +static void sio_process_report(struct sio_chan *siochan) +{ + unsigned long flags; + + spin_lock_irqsave(&siochan->vc.lock, flags); + if (siochan->ring) + sio_ring_report_locked(siochan); + else + sio_old_report_locked(siochan); spin_unlock_irqrestore(&siochan->vc.lock, flags); } @@ -818,6 +1310,12 @@ static int sio_device_config(struct dma_chan *chan, struct sio_shmem_chan_config cfg; int ret; + if (config->peripheral_config) { + if (config->peripheral_size != sizeof(struct apple_sio_idle)) + return -EINVAL; + return sio_set_idle(siochan, config->peripheral_config); + } + switch (is_tx ? config->dst_addr_width : config->src_addr_width) { case DMA_SLAVE_BUSWIDTH_1_BYTE: cfg.datashape = 0; @@ -1056,6 +1554,9 @@ static int sio_probe(struct platform_device *pdev) siochan->no = i; siochan->vc.desc_free = sio_tx_free; INIT_WORK(&siochan->terminate_wq, sio_terminate_work); + INIT_LIST_HEAD(&siochan->retired); + INIT_LIST_HEAD(&siochan->dead); + init_waitqueue_head(&siochan->drained); vchan_init(&siochan->vc, dma); } diff --git a/include/linux/dma/apple-sio.h b/include/linux/dma/apple-sio.h new file mode 100644 index 00000000000000..8665b511659b6c --- /dev/null +++ b/include/linux/dma/apple-sio.h @@ -0,0 +1,44 @@ +/* SPDX-License-Identifier: GPL-2.0-only OR MIT */ +#ifndef _LINUX_DMA_APPLE_SIO_H +#define _LINUX_DMA_APPLE_SIO_H + +#include + +/** + * struct apple_sio_idle - idle transfer of an Apple SIO channel + * @addr: DMA address of the buffer + * @len: buffer length; 0 removes the channel's idle transfer + * @period_len: length of each of its descriptors, not zero and at most @len + * + * Passed as &dma_slave_config.peripheral_config in a dmaengine_slave_config() + * call of its own, which sets or removes the idle transfer and nothing else: + * the channel's other settings take a separate call. + * + * While a channel has an idle transfer, it plays that buffer cyclically + * whenever no other transfer runs, and goes from one to the other in + * descriptor order: terminating a transfer hands over to the idle one instead + * of stopping the channel, and a transfer issued later follows the idle one's + * issued descriptors. The peripheral sees no gap in between. + * + * dmaengine_synchronize() returns once the descriptors of a terminated + * transfer have completed. If they do not in time, it stops the channel and + * waits for them first; the idle transfer then starts again, after a gap. + * Only an acknowledged TERMINATE counts as stopped. If the channel cannot be + * confirmed stopped, dmaengine_synchronize() warns, as it cannot fail. + * Removing the idle transfer stops the channel if nothing else runs, and + * returns 0 once its descriptors completed. If the channel cannot be + * confirmed stopped, it returns an error and the buffer is still in use. + * + * Setting an idle transfer fails with -EBUSY while the channel has one, or + * while it cannot take over the channel's issued descriptors. Setting and + * removing it, terminating and synchronizing must not run concurrently on a + * channel: its client serializes them. A channel that never had an idle + * transfer works as before. + */ +struct apple_sio_idle { + dma_addr_t addr; + size_t len; + size_t period_len; +}; + +#endif From c0b3d14f91bb82c09ce2f878b238495c409355ab Mon Sep 17 00:00:00 2001 From: Eryk Wieliczko Date: Wed, 28 Oct 2026 08:05:18 +0100 Subject: [PATCH 9/9] drm/apple: keep the held HDMI audio link's samples flowing The held link's silence replaced a stopped stream's DMA and was replaced by the next stream's with a SIO TERMINATE in between, and closing the PCM terminated it once more. A gap of 20 ms in the samples mutes the J514S sink until the DP link trains again, as a link restart does: with a gap forced to 20 ms, every later stream was silent, and Chromium resuming after a 30 s pause was silent in 2 of 7 runs without one. Make the silence the SIO channel's idle transfer while the link is held. A stream's DMA follows it and hands back to it in descriptor order, and the PCM's terminate at close leaves it running. Its periods shrink to 4 KiB, so a stream starts at most about 85 ms after its START. A stream that ends into the silence leaves up to four periods issued, which dmaengine_synchronize() waits for, 500 ms at most: on a port that holds links, stream periods are kept to 50 ms. A silence whose channel could not be stopped is not freed. Signed-off-by: Eryk Wieliczko Assisted-by: Claude:claude-opus-5-5 --- drivers/gpu/drm/apple/audio-link-test.c | 117 +++++++++++++++--- drivers/gpu/drm/apple/audio.c | 150 +++++++++++++++++------- 2 files changed, 205 insertions(+), 62 deletions(-) diff --git a/drivers/gpu/drm/apple/audio-link-test.c b/drivers/gpu/drm/apple/audio-link-test.c index ef897f94d7f1ce..40f928396297b6 100644 --- a/drivers/gpu/drm/apple/audio-link-test.c +++ b/drivers/gpu/drm/apple/audio-link-test.c @@ -13,6 +13,8 @@ struct dcpaud_link_test { bool link_on; bool link_live; bool typec; + /* The error removing the idle transfer returns */ + int idle_remove_err; u64 asked; unsigned int completes; unsigned int stops; @@ -38,12 +40,32 @@ static bool dcpaud_link_test_live(struct device *dev) return dcpaud_link_current->link_live; } +static bool dcpaud_link_test_typec(struct device *dev) +{ + return dcpaud_link_current->typec; +} + static int dcpaud_link_test_terminate(struct dma_chan *chan) { dcpaud_link_record('X'); return 0; } +/* G the channel configured; the silence as its idle transfer: I set, i removed */ +static int dcpaud_link_test_config(struct dma_chan *chan, struct dma_slave_config *config) +{ + const struct apple_sio_idle *idle = config->peripheral_config; + + if (!idle) { + dcpaud_link_record('G'); + return 0; + } + if (config->peripheral_size != sizeof(*idle)) + return -EINVAL; + dcpaud_link_record(idle->len ? 'I' : 'i'); + return idle->len ? 0 : dcpaud_link_current->idle_remove_err; +} + static dma_cookie_t dcpaud_link_test_submit(struct dma_async_tx_descriptor *tx) { return 1; @@ -61,18 +83,6 @@ dcpaud_link_test_prep(struct dma_chan *chan, dma_addr_t buf, size_t len, size_t return &t->desc; } -/* G the channel configured */ -static int dcpaud_link_test_config(struct dma_chan *chan, struct dma_slave_config *config) -{ - dcpaud_link_record('G'); - return 0; -} - -static bool dcpaud_link_test_typec(struct device *dev) -{ - return dcpaud_link_current->typec; -} - static void dcpaud_link_test_issue(struct dma_chan *chan) { } @@ -250,10 +260,11 @@ static void dcpaud_stop_holds_link(struct kunit *test) struct snd_pcm_substream *sub = dcpaud_link_test_open(test, audio); dcpaud_link_test_held_stop(test, audio); - /* The stream's DMA ends, the silence follows it; no link call */ + /* The silence set first, the stream's DMA ends into it; no link call */ KUNIT_ASSERT_EQ(test, dcp_pcm_native_sync_stop(sub), 0); - KUNIT_EXPECT_STREQ(test, dcpaud_link_current->sequence, "XQD"); + KUNIT_EXPECT_STREQ(test, dcpaud_link_current->sequence, "IXD"); KUNIT_EXPECT_TRUE(test, audio->silence_running); + KUNIT_EXPECT_TRUE(test, audio->idle_set); KUNIT_EXPECT_TRUE(test, audio->link_started); KUNIT_ASSERT_EQ(test, dcp_pcm_native_hw_free(sub), 0); @@ -262,13 +273,43 @@ static void dcpaud_stop_holds_link(struct kunit *test) /* The display goes off: the silence ends, then the link */ dcpaud_release_link(pdev, NULL); - KUNIT_EXPECT_STREQ(test, dcpaud_link_current->sequence, "XQDXDDSC"); + KUNIT_EXPECT_STREQ(test, dcpaud_link_current->sequence, "IXDiDSC"); KUNIT_EXPECT_FALSE(test, audio->silence_running); + KUNIT_EXPECT_FALSE(test, audio->idle_set); KUNIT_EXPECT_FALSE(test, audio->link_prepared); /* The stream's reference went with the silence */ KUNIT_EXPECT_EQ(test, atomic_read(&audio->dev->power.usage_count), 0); } +static void dcpaud_stuck_silence_keeps_pm(struct kunit *test) +{ + struct platform_device *pdev; + struct dcp_audio *audio = dcpaud_link_setup(test, &pdev); + struct snd_pcm_substream *sub = dcpaud_link_test_open(test, audio); + + dcpaud_link_test_held_stop(test, audio); + KUNIT_ASSERT_EQ(test, dcp_pcm_native_sync_stop(sub), 0); + KUNIT_ASSERT_EQ(test, dcp_pcm_native_hw_free(sub), 0); + + /* The channel cannot be confirmed stopped: it may still read the silence */ + dcpaud_link_current->idle_remove_err = -EIO; + dcpaud_release_link(pdev, NULL); + KUNIT_EXPECT_FALSE(test, audio->silence_running); + KUNIT_EXPECT_FALSE(test, audio->link_prepared); + KUNIT_EXPECT_TRUE(test, audio->idle_set); + KUNIT_EXPECT_TRUE(test, audio->idle_pm); + /* The stream's reference stays with the silence */ + KUNIT_EXPECT_EQ(test, atomic_read(&audio->dev->power.usage_count), 1); + + /* Removed later, it gives the reference back */ + dcpaud_link_current->idle_remove_err = 0; + scoped_guard(mutex, &audio->link_lock) + KUNIT_EXPECT_EQ(test, dcpaud_idle(audio, false), 0); + KUNIT_EXPECT_FALSE(test, audio->idle_set); + KUNIT_EXPECT_FALSE(test, audio->idle_pm); + KUNIT_EXPECT_EQ(test, atomic_read(&audio->dev->power.usage_count), 0); +} + static void dcpaud_held_stop_without_service(struct kunit *test) { struct platform_device *pdev; @@ -297,6 +338,7 @@ static struct snd_pcm_substream *dcpaud_link_test_held(struct kunit *test, struc audio->link_prepared = true; audio->link_started = true; audio->silence_running = true; + audio->idle_set = true; /* The held link's PM reference, then the stream's from its START */ pm_runtime_get_noresume(audio->dev); pm_runtime_get_noresume(audio->dev); @@ -312,10 +354,11 @@ static void dcpaud_start_on_held_link(struct kunit *test) struct dcp_audio *audio = dcpaud_link_setup(test, &pdev); struct snd_pcm_substream *sub = dcpaud_link_test_held(test, audio); - /* Its element: the stream's DMA replaces the silence, no link call */ + /* Its element: the stream's DMA follows the silence, no link call */ KUNIT_ASSERT_EQ(test, dcp_pcm_native_start(sub, SNDRV_PCM_TRIGGER_START), 0); - KUNIT_EXPECT_STREQ(test, dcpaud_link_current->sequence, "XQ"); + KUNIT_EXPECT_STREQ(test, dcpaud_link_current->sequence, "Q"); KUNIT_EXPECT_FALSE(test, audio->silence_running); + KUNIT_EXPECT_TRUE(test, audio->idle_set); KUNIT_EXPECT_TRUE(test, audio->link_started); pm_runtime_put_noidle(audio->dev); } @@ -330,7 +373,8 @@ static void dcpaud_start_replaces_held_link(struct kunit *test) memset(&audio->link_cookie, 1, sizeof(audio->link_cookie)); memset(&audio->selected_cookie, 2, sizeof(audio->selected_cookie)); KUNIT_ASSERT_EQ(test, dcp_pcm_native_start(sub, SNDRV_PCM_TRIGGER_START), 0); - KUNIT_EXPECT_STREQ(test, dcpaud_link_current->sequence, "XDSCGPQT"); + KUNIT_EXPECT_STREQ(test, dcpaud_link_current->sequence, "iSCGPQT"); + KUNIT_EXPECT_FALSE(test, audio->idle_set); KUNIT_EXPECT_EQ(test, memcmp(&audio->link_cookie, &audio->selected_cookie, sizeof(audio->link_cookie)), 0); KUNIT_EXPECT_TRUE(test, audio->link_started); @@ -346,7 +390,7 @@ static void dcpaud_start_after_held_link_went(struct kunit *test) /* The held link's service went (unplug): the stream gets a link of its own */ dcpaud_link_current->link_live = false; KUNIT_ASSERT_EQ(test, dcp_pcm_native_start(sub, SNDRV_PCM_TRIGGER_START), 0); - KUNIT_EXPECT_STREQ(test, dcpaud_link_current->sequence, "XDGPQT"); + KUNIT_EXPECT_STREQ(test, dcpaud_link_current->sequence, "iGPQT"); KUNIT_EXPECT_FALSE(test, audio->silence_running); KUNIT_EXPECT_TRUE(test, audio->link_started); pm_runtime_put_noidle(audio->dev); @@ -374,6 +418,39 @@ static void dcpaud_failed_start_keeps_held_link(struct kunit *test) pm_runtime_put_noidle(audio->dev); } +static void dcpaud_close_keeps_held_silence(struct kunit *test) +{ + struct platform_device *pdev; + struct dcp_audio *audio = dcpaud_link_setup(test, &pdev); + struct snd_pcm_substream *sub = kunit_kzalloc(test, sizeof(*sub), GFP_KERNEL); + struct snd_pcm_runtime *runtime = kunit_kzalloc(test, sizeof(*runtime), GFP_KERNEL); + struct snd_pcm *pcm = kunit_kzalloc(test, sizeof(*pcm), GFP_KERNEL); + + KUNIT_ASSERT_NOT_NULL(test, sub); + KUNIT_ASSERT_NOT_NULL(test, runtime); + KUNIT_ASSERT_NOT_NULL(test, pcm); + pcm->private_data = audio; + pcm->nonatomic = true; + sub->pcm = pcm; + sub->runtime = runtime; + KUNIT_ASSERT_EQ(test, snd_dmaengine_pcm_open(sub, &dcpaud_link_current->channel), 0); + audio->chan = &dcpaud_link_current->channel; + audio->link_prepared = true; + audio->link_started = true; + audio->silence_running = true; + audio->idle_set = true; + pm_runtime_get_noresume(audio->dev); + + /* The PCM's terminate leaves the idle silence: nothing queued, no link call */ + KUNIT_ASSERT_EQ(test, dcp_pcm_native_close(sub), 0); + KUNIT_EXPECT_STREQ(test, dcpaud_link_current->sequence, "XD"); + KUNIT_EXPECT_TRUE(test, audio->silence_running); + KUNIT_EXPECT_TRUE(test, audio->idle_set); + KUNIT_EXPECT_TRUE(test, audio->link_started); + KUNIT_EXPECT_EQ(test, atomic_read(&audio->dev->power.usage_count), 1); + pm_runtime_put_noidle(audio->dev); +} + static void dcpaud_link_test_hold_default(void *data) { WRITE_ONCE(hold_link, 1); @@ -415,11 +492,13 @@ static struct kunit_case dcpaud_link_cases[] = { KUNIT_CASE(dcpaud_disconnect_sync_completes_live_owner), KUNIT_CASE(dcpaud_gone_service_drops_link), KUNIT_CASE(dcpaud_stop_holds_link), + KUNIT_CASE(dcpaud_stuck_silence_keeps_pm), KUNIT_CASE(dcpaud_held_stop_without_service), KUNIT_CASE(dcpaud_start_on_held_link), KUNIT_CASE(dcpaud_start_replaces_held_link), KUNIT_CASE(dcpaud_start_after_held_link_went), KUNIT_CASE(dcpaud_failed_start_keeps_held_link), + KUNIT_CASE(dcpaud_close_keeps_held_silence), KUNIT_CASE(dcpaud_typec_stop_ends_stream), {} }; diff --git a/drivers/gpu/drm/apple/audio.c b/drivers/gpu/drm/apple/audio.c index 0fc2cf136abe2c..14bd1129fb84bc 100644 --- a/drivers/gpu/drm/apple/audio.c +++ b/drivers/gpu/drm/apple/audio.c @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -46,11 +47,20 @@ #define DCPAUD_PERIODS_MAX 16 /* - * The silence a held link plays between streams. SIO keeps up to four - * periods of a transfer in flight, so it gets more than that. + * The silence a held link plays between streams, as the SIO channel's idle + * transfer: SIO keeps up to four periods in flight, and a stream's first + * period follows those (about 85 ms). Its four descriptor slots stay taken + * while the link is held, next to a stream's sixteen at most. */ -#define DCPAUD_SILENCE_PERIOD SZ_16K -#define DCPAUD_SILENCE_BYTES (8 * DCPAUD_SILENCE_PERIOD) +#define DCPAUD_SILENCE_PERIOD SZ_4K +#define DCPAUD_SILENCE_BYTES (4 * DCPAUD_SILENCE_PERIOD) + +/* + * A stream that ends into the held link's silence leaves up to four periods + * issued, and dmaengine_synchronize() waits 500 ms for them before it stops + * the channel. Streams on a port that holds links keep to 50 ms periods. + */ +#define DCPAUD_HELD_PERIOD_MAX_US 50000 struct dcp_audio { struct device *dev; @@ -104,6 +114,10 @@ struct dcp_audio { void *silence; dma_addr_t silence_addr; bool silence_running; + /* The silence is the channel's idle transfer, see dcpaud_idle() */ + bool idle_set; + /* It holds a PM reference: its removal failed, see dcpaud_silence_stop() */ + bool idle_pm; #endif }; @@ -633,7 +647,7 @@ struct snd_pcm_ops dcp_playback_ops = { * STOP: the DMA terminated, or for a held link (below) left playing * the cleared buffer; * sync_stop: the DMA drained, then the link stopped and completed, or the - * held link's silence queued in place of the DMA; + * held link's silence taking over from the DMA; * hw_free: a link prepared but never started, completed. * * Every link method carries its element's link data. A link of an earlier @@ -645,9 +659,14 @@ struct snd_pcm_ops dcp_playback_ops = { * audio of a link that stops and starts again with an unchanged format, * until the DP link trains again: only a different channel layout brings * it back. So a stereo link, once started, is held between streams, as - * macOS holds it: the stopped stream's DMA is swapped for silence and the - * link keeps running until the format changes or the display goes off - * (dcpaud_release_link()). The DMA alone restarts without that loss. + * macOS holds it: silence follows the stopped stream's DMA and the link + * keeps running until the format changes or the display goes off + * (dcpaud_release_link()). + * + * The link must not run dry either: a gap of 20 ms in its samples mutes the + * sink the same way. The silence is the SIO channel's idle transfer, which + * the channel issues whenever no stream's DMA does, in descriptor order: + * a stream's DMA takes over from it and hands back to it without a gap. */ /* * Read-only at runtime: a stream opened while it was lower keeps periods @@ -748,34 +767,72 @@ static int dcpaud_pin_rate_format(struct dcp_audio *dcpaud, */ #define DCPAUD_SINK_SETTLE_MS 1500 -/* Queue the held link's silence; it carries the PM reference of the stream. */ +/* + * Sets or removes the silence as the channel's idle transfer. Removed, it + * stops the channel unless a stream's DMA runs, once its periods played. + */ +static int dcpaud_idle(struct dcp_audio *dcpaud, bool on) +{ + struct apple_sio_idle idle = { + .addr = dcpaud->silence_addr, + .len = on ? DCPAUD_SILENCE_BYTES : 0, + .period_len = DCPAUD_SILENCE_PERIOD, + }; + struct dma_slave_config config = { + .direction = DMA_MEM_TO_DEV, + .peripheral_config = &idle, + .peripheral_size = sizeof(idle), + }; + int ret; + + ret = dmaengine_slave_config(dcpaud->chan, &config); + /* Not removed: the channel may still read the silence. */ + if (ret) + return ret; + dcpaud->idle_set = on; + if (!on && dcpaud->idle_pm) { + dcpaud->idle_pm = false; + pm_runtime_mark_last_busy(dcpaud->dev); + pm_runtime_put_autosuspend(dcpaud->dev); + } + return 0; +} + +/* + * The held link's silence carries the PM reference of the stream. It is set + * before the stream's DMA ends, which it then follows without a gap. + */ static int dcpaud_silence_start(struct dcp_audio *dcpaud) { - struct dma_async_tx_descriptor *desc; + int ret; lockdep_assert_held(&dcpaud->link_lock); if (!dcpaud->silence) return -ENOMEM; - desc = dmaengine_prep_dma_cyclic(dcpaud->chan, dcpaud->silence_addr, - DCPAUD_SILENCE_BYTES, DCPAUD_SILENCE_PERIOD, - DMA_MEM_TO_DEV, 0); - if (!desc) - return -ENOMEM; - if (dma_submit_error(dmaengine_submit(desc))) - return -EIO; - dma_async_issue_pending(dcpaud->chan); + if (!dcpaud->idle_set) { + ret = dcpaud_idle(dcpaud, true); + if (ret) + return ret; + } dcpaud->silence_running = true; dev_dbg(dcpaud->dev, "link held, playing silence\n"); return 0; } +/* The link ends: so does its silence, which a stream may have taken over from. */ static void dcpaud_silence_stop(struct dcp_audio *dcpaud) { lockdep_assert_held(&dcpaud->link_lock); + if (dcpaud->idle_set) + dcpaud_idle(dcpaud, false); if (!dcpaud->silence_running) return; - dmaengine_terminate_sync(dcpaud->chan); dcpaud->silence_running = false; + /* A channel that may still read the silence keeps the device up. */ + if (dcpaud->idle_set && !dcpaud->idle_pm) { + dcpaud->idle_pm = true; + return; + } pm_runtime_mark_last_busy(dcpaud->dev); pm_runtime_put_autosuspend(dcpaud->dev); dev_dbg(dcpaud->dev, "held link's silence stopped\n"); @@ -824,6 +881,13 @@ static int dcp_pcm_native_open(struct snd_pcm_substream *substream) ret = dcp_pcm_open(substream); if (!ret && live && !dcpaud->open_unplugged && dcpaud_holds(dcpaud)) { ret = dcpaud_pin_rate_format(dcpaud, substream->runtime); + if (!ret) + ret = snd_pcm_hw_constraint_minmax(substream->runtime, + SNDRV_PCM_HW_PARAM_PERIOD_TIME, 0, + DCPAUD_HELD_PERIOD_MAX_US); + /* Positive: the constraint narrowed the period time. */ + if (ret > 0) + ret = 0; if (ret) dcp_pcm_close(substream); } @@ -847,21 +911,12 @@ static int dcp_pcm_native_close(struct snd_pcm_substream *substream) bool held; /* - * A held link keeps its service open. Closing the PCM terminated its - * channel, the silence with it: that is queued again. + * A held link keeps its service open. Closing the PCM terminated the + * channel's transfer, which leaves the idle one, the silence, running. */ scoped_guard(mutex, &dcpaud->link_lock) { dcpaud_drop_stale(dcpaud); held = dcpaud->silence_running; - if (held) { - dcpaud->silence_running = false; - if (dcpaud_silence_start(dcpaud)) { - held = false; - dcpaud_link_teardown(dcpaud); - pm_runtime_mark_last_busy(dcpaud->dev); - pm_runtime_put_autosuspend(dcpaud->dev); - } - } } if (!held) dcp_audiosrv_close(dcpaud->dcp_dev); @@ -996,25 +1051,26 @@ static int dcp_pcm_native_start(struct snd_pcm_substream *substream, int cmd) guard(mutex)(&dcpaud->link_lock); dcpaud_drop_stale(dcpaud); - /* The stream's DMA follows the held link's silence. */ + /* + * The stream's DMA follows the held link's silence, which stays the + * channel's idle transfer, and takes over the PM reference. + */ held = dcpaud->silence_running; if (held) { - dmaengine_terminate_async(dcpaud->chan); dcpaud->silence_running = false; pm_runtime_mark_last_busy(dcpaud->dev); pm_runtime_put_autosuspend(dcpaud->dev); } /* - * A held link of another element ends, and the stream's is prepared; - * so is a stream's whose prepare found a link that has gone since. + * A held link of another element ends, its silence with it, and the + * stream's is prepared; so is a stream's whose prepare found a link that + * has gone since. */ if (!dcpaud->link_prepared || memcmp(&dcpaud->link_cookie, &dcpaud->selected_cookie, sizeof(dcpaud->link_cookie))) { - if (dcpaud->link_prepared) { - dmaengine_synchronize(dcpaud->chan); + if (dcpaud->link_prepared) dcpaud_link_teardown(dcpaud); - } ret = dcpaud_link_prepare(dcpaud, substream); if (ret) return ret; @@ -1058,7 +1114,7 @@ static int dcp_pcm_native_trigger(struct snd_pcm_substream *substream, int cmd) case SNDRV_PCM_TRIGGER_STOP: /* * The DMA plays the cleared buffer on, with the stream's PM - * reference, until sync_stop swaps it for the held link's + * reference, until sync_stop hands over to the held link's * silence: the link must not run dry meanwhile. */ if (dcpaud_holds(dcpaud) && READ_ONCE(dcpaud->stream_dma) && @@ -1089,16 +1145,21 @@ static int dcp_pcm_native_sync_stop(struct snd_pcm_substream *substream) if (READ_ONCE(dcpaud->dma_cleared)) { guard(mutex)(&dcpaud->link_lock); WRITE_ONCE(dcpaud->dma_cleared, false); - dmaengine_terminate_async(dcpaud->chan); - /* Queued first, the silence follows the stream without a gap. */ + /* + * The silence set first, the channel hands over to it in + * descriptor order when the stream's DMA ends; synchronize + * returns once the stream's issued periods have played. + */ if (dcpaud->silence_running || !dcpaud->link_started || !dcp_audiosrv_link_live(dcpaud->dcp_dev) || dcpaud_silence_start(dcpaud)) { + dmaengine_terminate_async(dcpaud->chan); dmaengine_synchronize(dcpaud->chan); dcpaud_link_teardown(dcpaud); pm_runtime_mark_last_busy(dcpaud->dev); pm_runtime_put_autosuspend(dcpaud->dev); return 0; } + dmaengine_terminate_async(dcpaud->chan); dmaengine_synchronize(dcpaud->chan); return 0; } @@ -1455,11 +1516,14 @@ static void dcpaud_comp_unbind(struct device *dev, struct device *main, held = dcpaud->silence_running; if (held) dcpaud_link_teardown(dcpaud); - if (dcpaud->silence) { + else if (dcpaud->idle_set) + dcpaud_idle(dcpaud, false); + if (dcpaud->silence && dcpaud->idle_set) + dev_err(dcpaud->dev, "silence kept: its DMA channel did not stop\n"); + else if (dcpaud->silence) dma_free_coherent(dcpaud->chan->device->dev, DCPAUD_SILENCE_BYTES, dcpaud->silence, dcpaud->silence_addr); - dcpaud->silence = NULL; - } + dcpaud->silence = NULL; } if (held) dcp_audiosrv_close(dcpaud->dcp_dev);