Skip to content

[7/7] drm/apple: m4: Stop the J713 external DCPs for a deep sleep and restart them - #1420

Open
erikwb wants to merge 1 commit into
j713/dock-s2r-thunderboltfrom
j713/dock-s2r-drm
Open

erikwb wants to merge 1 commit into
j713/dock-s2r-thunderboltfrom
j713/dock-s2r-drm

Conversation

@erikwb

@erikwb erikwb commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

The J713's two external display coprocessors are adopted running from the boot loader and until now were left running across a sleep. That is only valid for suspend-to-idle: a deep sleep powers them off, their DARTs refuse it, and the sleep controller refuses to sleep with a bound display coprocessor still running.

Stop an external DCP in its deep-sleep suspend, the way the internal one is powered down: release the AP link, let RTKit idle, stop the CPU, power its display DART and then its firmware DART down, and power its CPU, front-end and system domains off. In resume, power the domains on, restore both DARTs and start the CPU again through RTKit's retained wake. The firmware starts from the image it kept, and the display comes back when the connection manager pairs its tunnel again.

The PMGR chain is asked first. A refusal before RTKit idles changes nothing and keeps the session. A failure after the CPU stopped is undone in reverse and refuses the sleep: a CPU whose domain went off starts again through the retained wake, one that was only stopped runs on with RTKit's ordinary wake. A failed restart still ends both DART transactions and fails the session. A board without DART owners (the J715 for now) says so at probe and keeps refusing a deep sleep with an external display.

A DCP with nothing attached has a session from adoption and is stopped and restarted the same way. With this, the J713 sleeps and wakes with a dock and external displays attached.

Part 7/7 of J713 docked S2R, stacked on the native radio series (#1384), which is stacked on #1314.

Series order: #1414 → #1415 → #1416 → #1417 → #1418 → #1419 → #1420.

Depends on #1419.

Validation of the series on this base:

  • Full arm64 Image, modules and DTBs at every PR tip of the series.
  • Ten focused host suites with negative controls (DART owner and register sequence, PHY sleep transaction, USB3 host model, router state transfer, tunnel replay including a dock lost during the sleep, NHI and tunneled-PCIe ownership including its failure paths, dock PM callback ordering, DP repair, DART writer gate and its callers) plus the device-tree policy check. checkpatch is clean for the driver code; it notes the Based-on-work-by: trailers and style in the tools/asahi fixtures.
  • J713 with a CalDigit TS5 (USB4 link, PCIe tunnel with the dock's Ethernet, USB hub with two keyboards, two 4K DisplayPort monitors), on the first version of the series: one suspend-abort test at the platform stage, then one real deep sleep with an RTC wake on the same boot. After the wake the dock link was at 20 Gb/s, dock Ethernet passed traffic at 2.5G, the USB devices were back, both monitors were lit at their previous mode about 10 s after the wake, Wi-Fi and Bluetooth answered, all ten CPUs were online, and there were no kernel warnings or taint. Repeated with one monitor unplugged; a monitor plugged in after the wake came up.

The changes since that version (the DT-selected DART owners and memory-only updates while closed, the unplug handling, the failure unwinds, the stock tb.c resume restored, the PMGR and RTKit hardening) are built and covered by the host suites, but have not run on the J713 yet.

Not covered: repeated sleeps on one boot, wake sources other than the RTC, the desktop's own sleep path (lid, logind), a dock unplugged during the sleep on hardware, and docks other than the TS5. Both native parameters (pcie_apple.cio_s2r, thunderbolt_apple.cio_s2r) default to off.

This series was developed and tested by Claude (Anthropic), working with me on the J713.

@DjDeveloperr DjDeveloperr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[7/7] drm/apple: m4: review at 2d7bc0a

Gating: fine. iomfb_m4 only builds with CONFIG_DRM_APPLE_DCP_T8132 and runs only for DCP_FIRMWARE_M4, and external means apple,t8132-dcpext, which only t8132-j713-j715.dtsi enables. T6050/M5 never reaches it, and iomfb_m4.c merges cleanly with the M5 branch.

Stop/restart pairing: on success they mirror each other. Stop runs A500, drain, RTKit idle, CPU stop, display DART off, firmware DART off, PMGR off. Restart runs PMGR on, firmware DART on, display DART on, retained wake, finish. The failure paths don't pair.

Findings (most severe first)

  1. A failed stop after ext_stopped = true has no rollback, and nothing ever restarts it. If display-DART off, firmware-DART off or consumer_off() fails, .suspend returns the error, and the PM core never calls .resume for a device whose .suspend failed. .complete only clears external_suspended. The CPU stays stopped, RX held, both DARTs cio_active, and a DART whose off() got past sleep_close() stays closed. The next unmap on it panics through #1416's writer gate, and a release panics.
    There's a concrete trigger: apple_dart_cio_off() returns -EBUSY whenever a map or unmap is in flight on that DART. Suggestion: unwind in reverse with consumer_on (needs #1414 note 1), apple_dart_cio_on() x2 (documented to undo a partial off), the retained wake and cio_finish() x2. At minimum, restart from .complete when ext_stopped is still set.
  2. No pre-check. iomfb_m4_power_gate() first asks every provider (iomfb_m4_gate_check(), including apple_pmgr_ps_consumer_check()); this path doesn't. A failed or already-off PMGR chain shows up only at the last step, after the CPU stopped and both DARTs went off, and lands in (1).
  3. A failed resume leaves the DARTs closed. If iomfb_m4_ext_start() fails at consumer_on or a DART on(), the PM core only logs the error and the system carries on with cio_active set and possibly closed DARTs. The next framebuffer or shared-buffer unmap then panics.
  4. A refused stop quarantines the session. Failures before the CPU stops (A500, drain finding messages, the late message) call dcp_m4_failed() and also refuse the sleep. The internal path does the former but not the latter, so after a refused sleep the external display stays dead until reboot or replug.
  5. J715 shares t8132-j713-j715.dtsi, but #1416 binds owners only on apple,j713. J715 logs "no DART owner for a deep sleep: -95" on every boot and refuses each deep sleep with -ENODEV in .suspend. The outcome is unchanged (its dispext DART refused before), but the refusal now comes later and louder.

Verdict: needs changes, (1) to (3).


Review only: read at the head commit, not built or run (no J713 here). Written with Claude Code.

Comment thread drivers/gpu/drm/apple/iomfb_m4.c Outdated
fail:
dev_err(dcp->dev, "external DCP stop failed at %s: %d\n", step, ret);
dcp_m4_failed(dcp, ret);
if (dcp->ext_stopped)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After ext_stopped = true this returns with the DCP CPU stopped, RX held, both DARTs still cio_active, and possibly one of them closed. .suspend failing means the PM core never calls dcp_platform_resume() for this device, and dcp_platform_complete() doesn't restart it either. A closed DART then panics on the next unmap (#1416's writer gate), and any release panics. apple_dart_cio_off() returns -EBUSY from apple_dart_sleep_close() whenever a writer is in flight, so this is reachable. Suggest unwinding here in reverse (apple_pmgr_ps_consumer_on(), apple_dart_cio_on() on both, which the header says undoes a partial off, apple_rtkit_wake_retained(), then apple_dart_cio_finish() on both), or at least calling iomfb_m4_ext_start() from .complete when ext_stopped is still set.

return -EALREADY;
if (!r || r->ext_qn || dcp->transaction.calls || !READ_ONCE(dcp->ext_dart_down))
return -EBUSY;
ret = dcp_m4_runtime_live(r);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The internal path asks every provider before its first step (iomfb_m4_gate_check()); this one only checks the DART binding. Adding apple_pmgr_ps_consumer_check(dcp->dev) here means a failed or already-off PMGR chain refuses the sleep with nothing touched, instead of failing at the last step, after the CPU and DARTs are down.

return 0;
fail:
dev_err(dcp->dev, "external DCP stop failed at %s: %d\n", step, ret);
dcp_m4_failed(dcp, ret);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This quarantines the session for every failure, including the ones before RTKit idle where nothing has changed yet (A500 NAK, drain finding messages), and the sleep is refused as well. The internal path takes the same dcp_m4_failed() but doesn't refuse the sleep. Here the user gets a refused sleep and a dead external display until reboot or replug. Consider returning -EBUSY without dcp_m4_failed() for failures before apple_rtkit_idle().

apple_dart_cio_finish(dcp->ext_darts[0]);
}
if (ret) {
dev_err(dcp->dev, "external DCP restart failed at %s: %d\n", step, ret);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resume errors are only logged by the PM core, so after this the system runs on with both DARTs cio_active and, if consumer_on or a DART on() failed, closed. The next unmap of a framebuffer or DCP shared buffer then panics (#1416). Even when the session is lost, the DART transactions need to end, with streams off but bookkeeping released.

Comment thread drivers/gpu/drm/apple/iomfb_m4.c Outdated
iomfb_m4_ext_bind_dart(dcp, piodma, &dcp->ext_darts[1]);
of_node_put(piodma);
if (ret)
dev_info(dev, "no DART owner for a deep sleep: %d\n", ret);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On J715 (same t8132-j713-j715.dtsi, dcpext enabled) this always fails with -EOPNOTSUPP, because #1416 only binds on apple,j713. Every deep sleep with an adopted session is then refused with -ENODEV in .suspend. Its dispext DART refused deep sleep before too, so the outcome is unchanged, but either include J715 in #1416 or make this message say the board isn't supported.

A deep sleep powers the J713's external display coprocessors off. Each
one is adopted running from the boot loader and until now was left
running across a sleep, which is only valid for suspend-to-idle: its
DARTs refuse a deep sleep, and the sleep controller refuses one with a
bound display coprocessor still running.

Stop an external DCP that has a session in its deep-sleep suspend, as
the internal one is powered down: release the AP link, let RTKit idle,
stop the CPU, power its display DART and then its firmware DART down
through their owner interface, and power its CPU, front-end and system
domains off. In resume, power the domains on, restore both DARTs and
start the CPU again through RTKit's retained wake. The firmware starts
from its reset vector on the image it kept, and the display comes back
with its next enable.

The PMGR chain is asked first. A refusal before RTKit idles changes
nothing and keeps the session. A failure after the CPU stopped is
undone in reverse, and the sleep is refused: a CPU whose domain went off
starts again through the retained wake, one that was only stopped runs
on and gets RTKit's ordinary wake. A failed restart still ends
both DART transactions and fails the session.

The sequence is the internal DCP's deep-sleep power-down applied to the
external ones, for the T8140 sleep controller's T8132 support.

This covers a DCP with nothing attached too: it has a session from
adoption and is stopped and restarted the same way.

The DART owners are bound at probe. Binding adds a device link, which
must not happen from a PM callback. A board whose DARTs have no owner
(the J715 for now) keeps refusing a deep sleep with an external display.

Based-on-work-by: Bartosz Nitka <niteria@gmail.com>
Based-on-work-by: Diljit Singh <hey@djdev.me>
Signed-off-by: Erik Bourget <erik@ebourget.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants