From 752e7c4dd87a9522ba847c3f382640d1003cbb2e Mon Sep 17 00:00:00 2001 From: Jvaxx <82868042+Jvaxx@users.noreply.github.com> Date: Sat, 24 Oct 2026 08:31:45 +0000 Subject: [PATCH] drm/apple: Hold panel presents while its display controller is off A472 with the hardware-power selector stops the J514S panel's display controller. The controller still accepts a present while it is off, but it never completes it. Nothing kept presents away from it: the A472 off is queued after the blank drains, and any present admitted later went out to the stopped controller. Lid-close sleep did that. The DRM suspend helper blanks the panel and turns the controller off, and a present admitted after it (swap 1307 in the captured log) was accepted and then held the IOMFB queue. The sleep drain reported "display updates still pending at suspend", the mailbox hold could not settle, and at resume the A472 on queued behind the present: "display controller on failed: -110". Ten seconds of running time after the present started, the queue timeout declared the coprocessor crashed, and every later atomic commit failed with EINVAL. The panel stayed dark until a reboot. DPMS off never showed it, as nothing presents after a compositor's DPMS off. Hold the panel's presents from just before the A472 off is queued until the A472 on succeeds. A present admitted earlier keeps its place ahead of the A472 off and goes out to a running controller. Admission refuses a held present as it refuses a paused one: an atomic commit completes with a synthetic vblank, and a brightness level stays pending for the backlight restore that follows the A472 on. The hold is separate from the presentation pause, which the mailbox hold for system sleep releases at resume, before DRM resume turns the controller back on. A failed A472 on keeps the hold, and the next power-on tries again. The sleep drain no longer waits for a backlight level that a panel whose controller is off cannot take; the restore presents it. Add KUnit cases for the admission, the order around the A472 off, and the hold outlasting the presentation pause. Signed-off-by: Jvaxx <82868042+Jvaxx@users.noreply.github.com> --- drivers/gpu/drm/apple/Makefile | 1 + drivers/gpu/drm/apple/dcp-internal.h | 7 + drivers/gpu/drm/apple/dcp.c | 14 ++ drivers/gpu/drm/apple/dcp_panel_hold_test.c | 181 ++++++++++++++++++++ drivers/gpu/drm/apple/iomfb.c | 23 ++- 5 files changed, 224 insertions(+), 2 deletions(-) create mode 100644 drivers/gpu/drm/apple/dcp_panel_hold_test.c diff --git a/drivers/gpu/drm/apple/Makefile b/drivers/gpu/drm/apple/Makefile index 52b4de01d3532..85a55ff5507bd 100644 --- a/drivers/gpu/drm/apple/Makefile +++ b/drivers/gpu/drm/apple/Makefile @@ -24,6 +24,7 @@ appledrm-$(CONFIG_DRM_APPLE_KUNIT_TEST) += dcp_external_ready_test.o appledrm-$(CONFIG_DRM_APPLE_KUNIT_TEST) += dcp_external_announce_test.o appledrm-$(CONFIG_DRM_APPLE_KUNIT_TEST) += dcp_external_relink_test.o appledrm-$(CONFIG_DRM_APPLE_KUNIT_TEST) += dcp_dropped_swap_test.o +appledrm-$(CONFIG_DRM_APPLE_KUNIT_TEST) += dcp_panel_hold_test.o appledrm-y += epic/dpavservep.o appledrm-y += plane.o diff --git a/drivers/gpu/drm/apple/dcp-internal.h b/drivers/gpu/drm/apple/dcp-internal.h index c9d99a2dcabc6..00e15ac1919e7 100644 --- a/drivers/gpu/drm/apple/dcp-internal.h +++ b/drivers/gpu/drm/apple/dcp-internal.h @@ -583,6 +583,12 @@ struct apple_dcp { unsigned int queued; bool stopped; bool presentation_paused; + /* + * The 14.7 panel's display controller is off, or its A472 off + * is queued: presents are held until it is back on. Separate + * from presentation_paused, which system sleep also releases. + */ + bool panel_held; bool backlight_queued; atomic_t opaque_x_state; struct iomfb_scanout_h17p *scanout; @@ -684,6 +690,7 @@ void iomfb_queue_stop(struct apple_dcp *dcp); bool iomfb_queue_drain(struct apple_dcp *dcp, unsigned long timeout); int iomfb_queue_settle(struct apple_dcp *dcp, unsigned long timeout); void iomfb_pause_presentation(struct apple_dcp *dcp, bool pause); +void iomfb_hold_panel_presents(struct apple_dcp *dcp, bool hold); int iomfb_external_power_h17p(struct apple_dcp *dcp, bool on); int iomfb_queue(struct apple_dcp *dcp, struct iomfb_transaction *transaction); void iomfb_queue_crc_h17p(struct apple_dcp *dcp, u32 swap_id); diff --git a/drivers/gpu/drm/apple/dcp.c b/drivers/gpu/drm/apple/dcp.c index 5fcf6a47c0eb7..946d3885211a7 100644 --- a/drivers/gpu/drm/apple/dcp.c +++ b/drivers/gpu/drm/apple/dcp.c @@ -3742,9 +3742,15 @@ static void dcp_panel_hw_off(struct apple_dcp *dcp) dev_warn(dcp->dev, "blank still pending; display controller left on\n"); return; } + /* + * Nothing may present to the stopped controller, e.g. the backlight + * core's level at system sleep, or a brightness retry. + */ + iomfb_hold_panel_presents(dcp, true); ret = iomfb_v14_7_panel_power(dcp, false); if (ret) { dev_err(dcp->dev, "display controller off failed: %d\n", ret); + iomfb_hold_panel_presents(dcp, false); return; } dcp->panel_hw_off = true; @@ -3759,10 +3765,12 @@ static void dcp_panel_hw_on(struct apple_dcp *dcp) return; ret = iomfb_v14_7_panel_power(dcp, true); if (ret) { + /* Presents stay held; the next power-on tries again. */ dev_err(dcp->dev, "display controller on failed: %d\n", ret); return; } dcp->panel_hw_off = false; + iomfb_hold_panel_presents(dcp, false); dev_dbg(dcp->dev, "display controller on\n"); } @@ -5340,6 +5348,12 @@ static void dcp_drain_for_sleep(struct apple_dcp *dcp) */ if (READ_ONCE(dcp->crashed) || READ_ONCE(dcp->iomfb.stopped)) return; + /* + * Nor does a panel whose display controller is off: the + * backlight restore at power-on presents the pending level. + */ + if (dcp->panel_hw_off) + return; if (!dcp_backlight_active(dcp) || !dcp_backlight_pending(dcp)) return; } diff --git a/drivers/gpu/drm/apple/dcp_panel_hold_test.c b/drivers/gpu/drm/apple/dcp_panel_hold_test.c new file mode 100644 index 0000000000000..e294a4e91cbec --- /dev/null +++ b/drivers/gpu/drm/apple/dcp_panel_hold_test.c @@ -0,0 +1,181 @@ +// SPDX-License-Identifier: GPL-2.0-only OR MIT + +#include +#include +#include + +#include "dcp.h" +#include "dcp-internal.h" + +/* + * The 14.7 panel's display controller off (A472) goes through the IOMFB + * queue, and a present that reaches the stopped controller is accepted but + * never completed. Lid-close sleep captured it: a present queued behind the + * A472 off held the queue, so the A472 on at resume timed out and the queue + * timeout declared the coprocessor crashed. These cases check admission + * through the real queue; each transaction completes at start, as one the + * firmware answers at once. + */ + +struct hold_transaction { + struct iomfb_transaction transaction; + unsigned int *order; + unsigned int started; +}; + +struct hold_test { + struct apple_dcp dcp; + struct hold_transaction t[4]; + unsigned int order; +}; + +static void hold_test_noop(struct work_struct *work) +{ +} + +static void hold_test_start(struct apple_dcp *dcp, struct iomfb_transaction *transaction) +{ + struct hold_transaction *t = container_of(transaction, struct hold_transaction, + transaction); + + t->started = ++*t->order; +} + +static void hold_test_release(struct iomfb_transaction *transaction) +{ +} + +static void hold_test_exit(void *data) +{ + struct hold_test *h = data; + + iomfb_queue_stop(&h->dcp); +} + +static struct hold_test *hold_test_init(struct kunit *test) +{ + struct hold_test *h = kunit_kzalloc(test, sizeof(*h), GFP_KERNEL); + unsigned int i; + + KUNIT_ASSERT_NOT_NULL(test, h); + h->dcp.fw_compat = DCP_FIRMWARE_V_14_7; + iomfb_queue_init(&h->dcp); + /* Each completion asks the backlight policy for a pending level. */ + spin_lock_init(&h->dcp.backlight.lock); + INIT_WORK(&h->dcp.vblank_wq, hold_test_noop); + for (i = 0; i < ARRAY_SIZE(h->t); i++) { + h->t[i].order = &h->order; + h->t[i].transaction.start = hold_test_start; + h->t[i].transaction.release = hold_test_release; + } + KUNIT_ASSERT_EQ(test, kunit_add_action_or_reset(test, hold_test_exit, h), 0); + return h; +} + +static void hold_test_present(struct hold_transaction *t, bool brightness_only) +{ + t->transaction.presentation = true; + t->transaction.brightness_only = brightness_only; +} + +/* Held: presents are refused, a command such as A472 still goes out. */ +static void panel_hold_refuses_presents_test(struct kunit *test) +{ + struct hold_test *h = hold_test_init(test); + struct apple_dcp *dcp = &h->dcp; + + if (!iomfb_is_v14_7(dcp)) + kunit_skip(test, "needs CONFIG_DRM_APPLE_DCP_T6030"); + + iomfb_hold_panel_presents(dcp, true); + hold_test_present(&h->t[0], false); + hold_test_present(&h->t[1], true); + KUNIT_EXPECT_EQ(test, iomfb_queue(dcp, &h->t[0].transaction), -EAGAIN); + KUNIT_EXPECT_EQ(test, iomfb_queue(dcp, &h->t[1].transaction), -EAGAIN); + /* A refused brightness present leaves no queued level behind. */ + KUNIT_EXPECT_FALSE(test, dcp->iomfb.backlight_queued); + + KUNIT_ASSERT_EQ(test, iomfb_queue(dcp, &h->t[2].transaction), 0); + KUNIT_EXPECT_EQ(test, iomfb_queue_settle(dcp, msecs_to_jiffies(1000)), 0); + KUNIT_EXPECT_EQ(test, h->t[2].started, 1U); + KUNIT_EXPECT_EQ(test, h->t[0].started, 0U); + KUNIT_EXPECT_EQ(test, h->t[1].started, 0U); + + iomfb_hold_panel_presents(dcp, false); + KUNIT_ASSERT_EQ(test, iomfb_queue(dcp, &h->t[0].transaction), 0); + KUNIT_EXPECT_EQ(test, iomfb_queue_settle(dcp, msecs_to_jiffies(1000)), 0); + KUNIT_EXPECT_EQ(test, h->t[0].started, 2U); + KUNIT_EXPECT_FALSE(test, READ_ONCE(dcp->crashed)); +} + +/* + * A present admitted before the hold still goes out, ahead of the A472 off + * queued after it; one admitted after the hold does not. A pending present + * keeps the queue from starting anything until the test lets it go. + */ +static void panel_hold_keeps_order_test(struct kunit *test) +{ + struct hold_test *h = hold_test_init(test); + struct apple_dcp *dcp = &h->dcp; + + if (!iomfb_is_v14_7(dcp)) + kunit_skip(test, "needs CONFIG_DRM_APPLE_DCP_T6030"); + + scoped_guard(mutex, &dcp->iomfb.lock) + dcp->present_state_h17p.pending = true; + hold_test_present(&h->t[0], false); + KUNIT_ASSERT_EQ(test, iomfb_queue(dcp, &h->t[0].transaction), 0); + + iomfb_hold_panel_presents(dcp, true); + /* The A472 off. */ + KUNIT_ASSERT_EQ(test, iomfb_queue(dcp, &h->t[1].transaction), 0); + hold_test_present(&h->t[2], true); + KUNIT_EXPECT_EQ(test, iomfb_queue(dcp, &h->t[2].transaction), -EAGAIN); + + scoped_guard(mutex, &dcp->iomfb.lock) + dcp->present_state_h17p.pending = false; + schedule_work(&dcp->iomfb.work); + KUNIT_EXPECT_EQ(test, iomfb_queue_settle(dcp, msecs_to_jiffies(1000)), 0); + KUNIT_EXPECT_EQ(test, h->t[0].started, 1U); + KUNIT_EXPECT_EQ(test, h->t[1].started, 2U); + KUNIT_EXPECT_EQ(test, h->t[2].started, 0U); +} + +/* + * System sleep pauses and resumes presentation around the mailbox hold. The + * resume must not release presents for a controller that is still off: the + * backlight core restores its level before DRM powers the panel on. + */ +static void panel_hold_outlasts_pause_test(struct kunit *test) +{ + struct hold_test *h = hold_test_init(test); + struct apple_dcp *dcp = &h->dcp; + + if (!iomfb_is_v14_7(dcp)) + kunit_skip(test, "needs CONFIG_DRM_APPLE_DCP_T6030"); + + iomfb_hold_panel_presents(dcp, true); + iomfb_pause_presentation(dcp, true); + iomfb_pause_presentation(dcp, false); + hold_test_present(&h->t[0], true); + KUNIT_EXPECT_EQ(test, iomfb_queue(dcp, &h->t[0].transaction), -EAGAIN); + + iomfb_hold_panel_presents(dcp, false); + KUNIT_EXPECT_EQ(test, iomfb_queue(dcp, &h->t[0].transaction), 0); + KUNIT_EXPECT_EQ(test, iomfb_queue_settle(dcp, msecs_to_jiffies(1000)), 0); + KUNIT_EXPECT_EQ(test, h->t[0].started, 1U); +} + +static struct kunit_case panel_hold_cases[] = { + KUNIT_CASE(panel_hold_refuses_presents_test), + KUNIT_CASE(panel_hold_keeps_order_test), + KUNIT_CASE(panel_hold_outlasts_pause_test), + {} +}; + +static struct kunit_suite panel_hold_suite = { + .name = "apple-dcp-panel-hold", + .test_cases = panel_hold_cases, +}; + +kunit_test_suite(panel_hold_suite); diff --git a/drivers/gpu/drm/apple/iomfb.c b/drivers/gpu/drm/apple/iomfb.c index 5e6aed367f3fb..abe48762ea441 100644 --- a/drivers/gpu/drm/apple/iomfb.c +++ b/drivers/gpu/drm/apple/iomfb.c @@ -462,6 +462,23 @@ void iomfb_pause_presentation(struct apple_dcp *dcp, bool pause) } } +/* + * Hold the 14.7 panel's presents across its display controller's A472 off and + * on. The controller accepts a present while it is off but never completes + * it, and the queue then waits for it: the A472 that would turn the controller + * back on queues behind it until the queue timeout declares the coprocessor + * crashed. Holding before the A472 off is queued keeps the queue order: a + * present admitted earlier still goes out first, to a running controller. + * Admission refuses a held present as a paused one: an atomic commit completes + * with a synthetic vblank, and a brightness level stays pending for the + * backlight restore at power-on. + */ +void iomfb_hold_panel_presents(struct apple_dcp *dcp, bool hold) +{ + guard(mutex)(&dcp->iomfb.lock); + dcp->iomfb.panel_held = hold; +} + /* A dead transport is not proof that DMA or a pending present has stopped. */ int iomfb_queue_settle(struct apple_dcp *dcp, unsigned long timeout) { @@ -493,7 +510,8 @@ int iomfb_queue(struct apple_dcp *dcp, struct iomfb_transaction *transaction) mutex_lock(&dcp->iomfb.lock); if (READ_ONCE(dcp->crashed) || dcp->iomfb.stopped) { ret = -EIO; - } else if (transaction->presentation && dcp->iomfb.presentation_paused) { + } else if (transaction->presentation && + (dcp->iomfb.presentation_paused || dcp->iomfb.panel_held)) { ret = -EAGAIN; } else if (transaction->brightness_only && dcp->iomfb.backlight_queued) { ret = -EALREADY; @@ -2240,7 +2258,8 @@ static void iomfb_queue_atomic(struct apple_dcp *dcp, struct drm_crtc *crtc, struct iomfb_atomic_transaction *atomic; int ret; - if (READ_ONCE(dcp->iomfb.presentation_paused)) { + if (READ_ONCE(dcp->iomfb.presentation_paused) || + READ_ONCE(dcp->iomfb.panel_held)) { schedule_work(&dcp->vblank_wq); return; }