From 7ef3bc138e59fef9d74964b61513a5be31c35aea Mon Sep 17 00:00:00 2001 From: Eryk Wieliczko Date: Fri, 30 Oct 2026 15:02:51 +0100 Subject: [PATCH 1/2] drm/apple: set an external mode again once the display is on An external mode is set with the display powered down, and the display is powered up after it. For a mode that needs DSC (3840x2160 at 100 and 120 Hz on the J514S HDMI and Type-C ports) the firmware refuses that mode set ("Modeset done, but pipe not enabled") and resets DSC, and the power-on only configures DSC: the timings start in a mode set the firmware makes on its own, after a hotplug notification it sends itself. A swap that reaches it before then is dropped ("swallowed swap ID 1 as timings are not enabled") and never completed, so A407 times out after 5 s and the display stays dark until a reboot. A fresh attach lost that race 3 times out of 3. Without DSC, the power-on starts the timings itself. Set the mode once more after the power-on, with the pipe enabled. The firmware then starts the timings before that call returns, before the first swap; its own mode set finds them running. KUnit cases check the calls of an attach and of a mode change. Signed-off-by: Eryk Wieliczko Assisted-by: Claude:claude-opus-5-5 --- drivers/gpu/drm/apple/iomfb_v14_7.c | 12 ++ .../apple/iomfb_v14_7_external_modeset_test.c | 165 ++++++++++++++++++ 2 files changed, 177 insertions(+) create mode 100644 drivers/gpu/drm/apple/iomfb_v14_7_external_modeset_test.c diff --git a/drivers/gpu/drm/apple/iomfb_v14_7.c b/drivers/gpu/drm/apple/iomfb_v14_7.c index 58a127cb54f062..34054bb56a7671 100644 --- a/drivers/gpu/drm/apple/iomfb_v14_7.c +++ b/drivers/gpu/drm/apple/iomfb_v14_7.c @@ -3271,6 +3271,17 @@ static int dcp_v14_external_modeset(struct apple_dcp *dcp, struct drm_crtc_state ret = dcp_v14_set_mode(v14, mode.color_mode_id, mode.timing_mode_id); if (!ret) ret = dcp_v14_set_power(v14, true); + /* + * The mode was set with the pipe off. A mode that needs DSC is then + * reset again ("pipe not enabled"), and the power-on only configures + * DSC: the firmware starts the timings in a mode set of its own, after + * the hotplug notification it sends itself, and drops a swap that + * reaches it before then without completing it, so A407 times out. + * Setting the mode once more with the pipe on starts the timings + * before the first swap. + */ + if (!ret) + ret = dcp_v14_set_mode(v14, mode.color_mode_id, mode.timing_mode_id); if (!ret) { v14->panel_width = mode.mode.hdisplay; v14->panel_height = mode.mode.vdisplay; @@ -3383,6 +3394,7 @@ void iomfb_v14_7_poweroff(struct apple_dcp *dcp) #if IS_ENABLED(CONFIG_DRM_APPLE_DISPLAY_KUNIT_TEST) #include "iomfb_v14_7_external_audio_test.c" +#include "iomfb_v14_7_external_modeset_test.c" #include "iomfb_v14_7_backlight_test.c" #include "iomfb_v14_7_backlight_lifetime_test.c" #endif diff --git a/drivers/gpu/drm/apple/iomfb_v14_7_external_modeset_test.c b/drivers/gpu/drm/apple/iomfb_v14_7_external_modeset_test.c new file mode 100644 index 00000000000000..170aa59fb4234b --- /dev/null +++ b/drivers/gpu/drm/apple/iomfb_v14_7_external_modeset_test.c @@ -0,0 +1,165 @@ +// SPDX-License-Identifier: GPL-2.0-only OR MIT +/* + * The DCP calls of an external mode set, included at the end of + * iomfb_v14_7.c so that the production dcp_v14_external_modeset() runs. + * Every call is answered as accepted; the audio service is stubbed. + */ +#include +#include +#include + +#define V14_EXT_MODESET_MAX_CALLS 8 + +struct v14_ext_modeset { + struct apple_dcp *dcp; + struct apple_dcp_v14 *v14; + struct dcp_display_mode mode; + u32 tags[V14_EXT_MODESET_MAX_CALLS]; + bool power[V14_EXT_MODESET_MAX_CALLS]; + unsigned int calls; +}; + +static struct v14_ext_modeset *v14_ext_modeset_current; + +static int v14_ext_modeset_call(struct apple_dcp_v14 *v14, u32 tag, const void *in, u32 in_size, + void *out, u32 out_size, u32 completion, dcp_v14_admit_fn admit, + void *admit_cookie) +{ + struct v14_ext_modeset *s = v14_ext_modeset_current; + + if (s->calls < V14_EXT_MODESET_MAX_CALLS) { + s->tags[s->calls] = tag; + s->power[s->calls] = tag == A(472) && in_size >= 8 && get_unaligned_le64(in); + } + s->calls++; + if (out) + memset(out, 0, out_size); + return 0; +} + +static void v14_ext_modeset_audio(struct apple_dcp *dcp) +{ +} + +static void v14_ext_modeset_retry(struct work_struct *work) +{ +} + +static void v14_ext_modeset_stop(void *data) +{ + struct apple_dcp *dcp = data; + + cancel_delayed_work_sync(&dcp->external_retry_wq); +} + +/* A started, described native external session with one mode, its display @powered. */ +static struct v14_ext_modeset *v14_ext_modeset_setup(struct kunit *test, bool powered) +{ + struct v14_ext_modeset *s = kunit_kzalloc(test, sizeof(*s), GFP_KERNEL); + struct device *dev; + + KUNIT_ASSERT_NOT_NULL(test, s); + dev = kunit_device_register(test, "dcp-v14-ext-modeset"); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dev); + s->dcp = devm_kzalloc(dev, sizeof(*s->dcp), GFP_KERNEL); + s->v14 = devm_kzalloc(dev, sizeof(*s->v14), GFP_KERNEL); + KUNIT_ASSERT_NOT_NULL(test, s->dcp); + KUNIT_ASSERT_NOT_NULL(test, s->v14); + s->dcp->dev = dev; + s->dcp->fw_compat = DCP_FIRMWARE_V_14_7; + s->dcp->external = true; + s->dcp->external_native = true; + /* Never dereferenced: the audio service is stubbed. */ + s->dcp->avep = (struct apple_dcp_afkep *)s; + mutex_init(&s->dcp->modes_lock); + mutex_init(&s->dcp->hpd_mutex); + spin_lock_init(&s->dcp->metadata.lock); + spin_lock_init(&s->dcp->mode_state.lock); + INIT_DELAYED_WORK(&s->dcp->external_retry_wq, v14_ext_modeset_retry); + /* 3840x2160 at 100 Hz, timing 167 and color 219 as a J514S TV reports it. */ + s->mode.mode = (struct drm_display_mode){ + DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 1188000, 3840, 4896, 4984, 5280, 0, + 2160, 2168, 2178, 2250, 0, DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC) + }; + s->mode.timing_mode_id = 167; + s->mode.color_mode_id = 219; + s->dcp->modes = &s->mode; + s->dcp->nr_modes = 1; + s->dcp->modes_admitted = true; + s->dcp->v14 = s->v14; + s->v14->dev = dev; + s->v14->dcp = s->dcp; + s->v14->external = true; + s->v14->started = true; + s->v14->opened = true; + s->v14->powered = powered; + s->v14->described = DCP_V14_DESC_ALL; + mutex_init(&s->v14->lock); + init_waitqueue_head(&s->v14->described_wait); + KUNIT_ASSERT_EQ(test, kunit_add_action_or_reset(test, v14_ext_modeset_stop, s->dcp), 0); + v14_ext_modeset_current = s; + kunit_activate_static_stub(test, dcp_v14_call_guarded, v14_ext_modeset_call); + kunit_activate_static_stub(test, dcp_v14_audio_off, v14_ext_modeset_audio); + kunit_activate_static_stub(test, dcp_v14_audio_on, v14_ext_modeset_audio); + return s; +} + +static int v14_ext_modeset_run(struct kunit *test, struct v14_ext_modeset *s) +{ + struct drm_crtc_state *state = kunit_kzalloc(test, sizeof(*state), GFP_KERNEL); + + KUNIT_ASSERT_NOT_NULL(test, state); + drm_mode_copy(&state->mode, &s->mode.mode); + return dcp_v14_external_modeset(s->dcp, state); +} + +/* + * A display that was off, as on attach: the mode is set, the display powered + * on, and the mode set once more with the pipe on, before any swap. A DSC + * mode set with the pipe off only takes effect then. + */ +static void v14_ext_modeset_sets_mode_again_after_power_on(struct kunit *test) +{ + struct v14_ext_modeset *s = v14_ext_modeset_setup(test, false); + + KUNIT_EXPECT_EQ(test, v14_ext_modeset_run(test, s), 0); + KUNIT_ASSERT_EQ(test, s->calls, 3U); + KUNIT_EXPECT_EQ(test, s->tags[0], A(411)); + KUNIT_EXPECT_EQ(test, s->tags[1], A(472)); + KUNIT_EXPECT_TRUE(test, s->power[1]); + KUNIT_EXPECT_EQ(test, s->tags[2], A(411)); + KUNIT_EXPECT_TRUE(test, s->v14->powered); + KUNIT_EXPECT_TRUE(test, s->v14->mode_live); + KUNIT_EXPECT_EQ(test, s->v14->mode_timing, 167U); + KUNIT_EXPECT_EQ(test, s->v14->mode_color, 219U); + KUNIT_EXPECT_FALSE(test, mutex_is_locked(&s->v14->lock)); +} + +/* A mode change on a display that is on: power down, set, power up, set again. */ +static void v14_ext_modeset_mode_change(struct kunit *test) +{ + struct v14_ext_modeset *s = v14_ext_modeset_setup(test, true); + + KUNIT_EXPECT_EQ(test, v14_ext_modeset_run(test, s), 0); + KUNIT_ASSERT_EQ(test, s->calls, 4U); + KUNIT_EXPECT_EQ(test, s->tags[0], A(472)); + KUNIT_EXPECT_FALSE(test, s->power[0]); + KUNIT_EXPECT_EQ(test, s->tags[1], A(411)); + KUNIT_EXPECT_EQ(test, s->tags[2], A(472)); + KUNIT_EXPECT_TRUE(test, s->power[2]); + KUNIT_EXPECT_EQ(test, s->tags[3], A(411)); + KUNIT_EXPECT_TRUE(test, s->v14->powered); +} + +static struct kunit_case v14_ext_modeset_cases[] = { + KUNIT_CASE(v14_ext_modeset_sets_mode_again_after_power_on), + KUNIT_CASE(v14_ext_modeset_mode_change), + {} +}; + +static struct kunit_suite v14_ext_modeset_suite = { + .name = "dcp_v14_external_modeset", + .test_cases = v14_ext_modeset_cases, +}; + +kunit_test_suite(v14_ext_modeset_suite); From 653176dcb94ed7253763428f1f8522cb4685e496 Mon Sep 17 00:00:00 2001 From: Eryk Wieliczko Date: Fri, 30 Oct 2026 20:32:06 +0100 Subject: [PATCH 2/2] drm/apple: hold the audio link on the Type-C ports too A stereo audio link is held between streams so that the sink does not mute it on a restart with the same format. The hold_link parameter limited that to the HDMI port by default (hold_link=1) until the Type-C ports had been tested with it; hold_link=2 extended it to them and 0 turned it off. On the J514S, a TV on a Type-C port behaves as it does on the HDMI port: without the hold, every stream starts a new link with an unchanged format, the TV mutes it, and USB-C audio stays silent after the first stream. With hold_link=2 it played across Chromium stop/start, pw-play runs and a PipeWire restart, and held links survived 90 s idle and sleep. Hold the link on every port and drop the parameter. Nothing needs to turn the hold off, and a parameter that only makes audio go silent is not worth keeping. Streams of more than two channels still get a link of their own, as before. Signed-off-by: Eryk Wieliczko Assisted-by: Claude:claude-opus-5-5 --- drivers/gpu/drm/apple/audio-link-test.c | 44 ------------------------- drivers/gpu/drm/apple/audio.c | 42 +++++++---------------- drivers/gpu/drm/apple/audio.h | 2 -- drivers/gpu/drm/apple/av.c | 6 ---- 4 files changed, 11 insertions(+), 83 deletions(-) diff --git a/drivers/gpu/drm/apple/audio-link-test.c b/drivers/gpu/drm/apple/audio-link-test.c index 40f928396297b6..0ddd42dce072c6 100644 --- a/drivers/gpu/drm/apple/audio-link-test.c +++ b/drivers/gpu/drm/apple/audio-link-test.c @@ -12,7 +12,6 @@ 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; @@ -40,11 +39,6 @@ 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'); @@ -147,7 +141,6 @@ static struct dcp_audio *dcpaud_link_setup(struct kunit *test, struct platform_d 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; } @@ -451,42 +444,6 @@ static void dcpaud_close_keeps_held_silence(struct kunit *test) 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), @@ -499,7 +456,6 @@ static struct kunit_case dcpaud_link_cases[] = { 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 14bd1129fb84bc..7c5c249dff115a 100644 --- a/drivers/gpu/drm/apple/audio.c +++ b/drivers/gpu/drm/apple/audio.c @@ -655,12 +655,12 @@ struct snd_pcm_ops dcp_playback_ops = { * 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: silence follows the stopped stream's DMA and the link - * keeps running until the format changes or the display goes off + * The sinks behind the J514S HDMI port (PS190 converter and TV) and its + * Type-C ports mute 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: 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 @@ -668,26 +668,6 @@ struct snd_pcm_ops dcp_playback_ops = { * 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 - * 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)); -} - /* * 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 @@ -879,7 +859,7 @@ 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)) { + if (!ret && live && !dcpaud->open_unplugged) { ret = dcpaud_pin_rate_format(dcpaud, substream->runtime); if (!ret) ret = snd_pcm_hw_constraint_minmax(substream->runtime, @@ -1043,8 +1023,8 @@ static int dcp_pcm_native_start(struct snd_pcm_substream *substream, int cmd) /* 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) { + if (!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); } @@ -1117,8 +1097,8 @@ static int dcp_pcm_native_trigger(struct snd_pcm_substream *substream, int cmd) * 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) && - substream->runtime->channels == 2 && substream->runtime->dma_area) { + if (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; diff --git a/drivers/gpu/drm/apple/audio.h b/drivers/gpu/drm/apple/audio.h index 259703b55fb287..dcf752efb9a121 100644 --- a/drivers/gpu/drm/apple/audio.h +++ b/drivers/gpu/drm/apple/audio.h @@ -37,8 +37,6 @@ 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 e5b987be12c2b8..c0bacc9956d31e 100644 --- a/drivers/gpu/drm/apple/av.c +++ b/drivers/gpu/drm/apple/av.c @@ -400,12 +400,6 @@ 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