Skip to content

[1/7] pmdomain: apple: Let a consumer switch the J713 external display domains - #1414

Closed
erikwb wants to merge 5 commits into
j713/radio-s2r-bluetoothfrom
j713/dock-s2r-pmgr
Closed

erikwb wants to merge 5 commits into
j713/radio-s2r-bluetoothfrom
j713/dock-s2r-pmgr

Conversation

@erikwb

@erikwb erikwb commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

The J713's external display coprocessors are powered off by a deep sleep, so their driver has to power their CPU, front-end and system domains off after stopping the firmware and on again before restarting it. The system domain has a bus bridge in front of its block whose link must go down before the domain powers off and come back up once it is on.

  • Add apple,bridge-link to the PMGR power-state binding (a phandle to the block, the index of its register range and the offset of the link words) and handle it in the consumer-switched power-off and power-on.
  • Add apple_pmgr_ps_verify_state(), used later in the series by the ACIO host to confirm each of its domains reached the state it asked for. It refuses a device whose power domain is not a genpd.
  • Let a consumer roll back a power-off that stopped partway: the power-on walk treats a state that stayed on as done, and apple_pmgr_ps_consumer_is_off() says whether the consumer's own domain went off, i.e. whether its block went through reset.
  • Mark the six external display domains on the J713 as consumer-switched, with the CPU domains' retention words and, in the board file, the system domains' bridge links. They stay firmware-owned otherwise, so nothing changes at runtime.

Part 1/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 #1384.

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.

[1/7] pmdomain: review at 8f762ae

Gating: fine. The new behaviour is DT opt-in. apple,bridge-link is set only on the two J713 dispext sys states, and consumer-switched/retention only in t8132-j713-pmgr.dtsi. The T8132 platform handoff copies only status and the two ownership flags from that file, so none of these reach other T8132 boards. Nothing changes for T6050/T8140/T60xx. apple_pmgr_ps_verify_state() is a new export and leaves existing callers alone. This file has no changes on the M5 branch, so no conflict.

Findings

  1. Abort path (pre-existing, now load-bearing): apple_pmgr_ps_consumer_on() returns at the first state that is not off. After a consumer_off() that failed partway, for example fe reading -EBUSY because it isn't fully on, the parent-first on-walk gets -EALREADY from sys and stops. cpu stays off. #1420 needs exactly this rollback (see there).
  2. apple,bridge-link is a raw physical address of another block's registers inside a PMGR pwrstate node. Upstream binding review will push back. A phandle to the dcpext node plus an offset carries the same information.
  3. apple_pmgr_ps_verify_state() applies pd_to_genpd() to any dev->pm_domain and dereferences the result before it knows it is a genpd. That's fine for today's ACIO callers, but cheap to guard on an export.
  4. Nit: the bridge dev_info fires twice per DCP per sleep, while the neighbouring power-off/on messages are dev_dbg.

Verdict: OK with minor changes. Fix (1) before #1420 relies on a rollback.


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

- description: value while the domain is off
maxItems: 4

apple,bridge-link:

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.

A physical address of another block's registers in a PMGR node is the part an upstream binding review will object to. Something like apple,bridge-link = <&dcpext0 0x104> (phandle plus offset), resolved through the dcpext's reg, keeps it per-board in DT and doesn't hard-code the SoC map in a property.


if (!domain_dev->pm_domain)
return -EINVAL;
genpd = pd_to_genpd(domain_dev->pm_domain);

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.

pd_to_genpd() is a bare container_of(), and genpd->power_on is read before we know pm_domain belongs to a genpd. All current callers pass ACIO genpd virtual devices, so this is safe today. Since it's exported, a cheap guard first (for example comparing domain_dev->pm_domain->ops.runtime_suspend with what genpd installs, or keeping a list of this driver's genpds) would keep a wrong caller from reading arbitrary memory.

Comment thread drivers/pmdomain/apple/pmgr-pwrstate.c Outdated
/* Complete the stores before the state word changes or its block is used. */
mb();
readl(ps->bridge);
dev_info(ps->dev, "PS %s: bridge link %s\n", ps->genpd.name, up ? "up" : "down");

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.

Nit: this logs twice per external DCP on every sleep. The consumer power-off/on messages next to it are dev_dbg.

ret = apple_pmgr_ps_wait(ps, APPLE_PMGR_PS_LEVELS, APPLE_PMGR_PS_ALL_ON, &cur);
if (ret)
goto fail;
apple_pmgr_ps_bridge(ps, true);

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.

Related, though not in this diff: apple_pmgr_ps_consumer_on() walks parent-first and returns on the first error, and consumer_state_on() gives -EALREADY for a state that isn't off. If consumer_off() failed partway (cpu off, then fe -EBUSY because it wasn't fully on), the rollback consumer_on() stops at sys and never powers cpu back on. #1420 needs this rollback on its failed-stop path. Suggest treating -EALREADY as done in the on-walk, or recording which states this call switched.

A consumer-switched power state can have a bus bridge in front of the
block it powers. The bridge's two link words must have bit 0 cleared
before the state powers off and set again once it is on, and they are
only accessible while the state is on. Describe where they are: a
phandle to the block's node, the index of its register range and the
offset of the words in that range. The PMGR driver can then do that as
part of the consumer's power-off and power-on.

The T8132 external display system domains need it.

Signed-off-by: Erik Bourget <erik@ebourget.net>
A T8132 external display system domain must not be powered off with
the bus bridge of its block still linked. When a consumer-switched
state has apple,bridge-link, clear bit 0 of the two link words just
before the state powers off, and set it again once the state reads
fully on, before the retention words are restored. The words are
touched only while the state is on. Their address comes from the
named block's register range and the offset in the property.

This extends the consumer-switched power states.

Based-on-work-by: Bartosz Nitka <niteria@gmail.com>
Signed-off-by: Erik Bourget <erik@ebourget.net>
A driver that takes its power domains down and up itself around a
system sleep, through the genpd suspend and resume helpers, has to know
that each one really reached the state it asked for before it goes on:
the next step either removes power from, or touches, the block behind
it. Add a helper that polls the state word of a virtual power-domain
device for fully on or fully off.

It refuses a device whose power domain is not a genpd, and firmware-owned
and inherited-on states, which the driver does not control.

Signed-off-by: Erik Bourget <erik@ebourget.net>
…er power-off

apple_pmgr_ps_consumer_on() walks the consumer's chain parent first and
stops at the first state it cannot switch. A state that is not off gives
-EALREADY. After a consumer power-off that stopped partway, for example
with the CPU state off and the frontend refusing because it was not fully
on, a rollback power-on stops at the system state, which never went off,
and leaves the CPU state off.

Treat -EALREADY as done in the power-on walk, so that a rollback after a
partial power-off switches back on every state that went off.

A consumer rolling back also has to know whether its own block went
through reset, which decides how its coprocessor is started again. Add
apple_pmgr_ps_consumer_is_off() for that: whether the first state of its
chain, its own domain, was powered off.

Signed-off-by: Erik Bourget <erik@ebourget.net>
…ains

A deep sleep powers the external display coprocessors off, and they
have to be stopped first and started again afterwards. Mark the CPU,
front-end and system domains of both as consumer-switched, with the
CPU domains' power-gate retention words and the system domains' bridge
links, so the coprocessor's driver can power the chain off after it has
stopped the firmware and on again before it restarts it.

The bridge links name the external DCP whose block they are in, so they
live in the board file; the PMGR file is also built into the platform
data, which has no DCP nodes.

The domains stay firmware-owned otherwise: nothing changes at runtime.

Based-on-work-by: Bartosz Nitka <niteria@gmail.com>
Signed-off-by: Erik Bourget <erik@ebourget.net>

@RyanTheTide RyanTheTide 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.

This 7-PR stack (#1414-#1420) sits on the J713 radio stack (#1378-#1385), which sits on #1314. #1314 has commits without Signed-off-by and conflicts with integration/aurora, so none of these can merge yet. Either #1314 gets Signed-off-by and a rebase, or rebase this stack and the radio stack onto integration/aurora without #1314's WIP commits. #1415 and #1419 also have failing/cancelled CI runs to re-run after that.

@erikwb
erikwb force-pushed the j713/dock-s2r-pmgr branch from 8f762ae to fe1184f Compare November 10, 2026 13:31
@RyanTheTide

Copy link
Copy Markdown
Contributor

@erikwb This series is now carried in #1314, rebased onto integration/aurora with your sign-off kept: 10 of the 18 commits are identical and the rest differ only in context and host-test fixtures. One code difference: in tb_switch_suspend_cio() the copy in #1314 checks ret < 0 after tb_switch_clx_disable(), which returns the CL states it disabled, so a positive return no longer refuses the sleep. Since the stack is based on the closed radio PRs, we will merge it through #1314 and close these seven when that lands.

linux#1409 techmore ci: sep-source: cancel superseded runs and use a read-only token
ass: c verdict: MERGE-NOW
b: ok (efeedfe) ci: green (all 7 jobs on efeedfe) mergeable: yes
sk-to-lab: none
w-needed: none
ndings:
permissions: contents: read is enough: source, rust-build and kunit only check out and build. Concurrency group sep-source-<PR|ref> with cancel-in-progress, same pattern as dt-checks.yml and host-tests.yml.
RyanTheTide's earlier Signed-off-by note is resolved: the head commit carries the author's sign-off.
st: |
Read-only token and the concurrency group match the other two workflows; sign-off is on the head commit and all jobs pass. Looks good.

linux#1370 boredcoder411 tools/asahi: Reject missing DART functions with selected cases
ass: c verdict: MERGE-NOW
b: ok (ab08f82, added 11-11 after the 11-10 review) ci: none reported (head branch is the fork's integration/aurora); run host-tests before merge mergeable: yes
sk-to-lab: none (host test tooling only)
w-needed: none (run python3 -I tools/asahi/run-host-tests.py --only apple-dart-fw locally or approve the workflow run)
ndings:
One-line fix: missing production functions are an argument error with or without --case, so the fixture never compiles with a hole. New harness test covers no case, one case and repeated cases and checks no traceback and no output dir. Registered in run-host-tests.py.
RyanTheTide's 11-10 review found it correct and asked only for Signed-off-by; the amended commit has it.
Not in base yet (base still has if missing and not a.case).
st: |
Sign-off is there now and the change is as reviewed before. Looks good.

linux#1369 AxxzyWasTaken soc: apple: clpc, drm/asahi: g17: busy-share memory floor and light effort for T6041
ass: b verdict: CHANGES
b: ok ci: green mergeable: yes
sk-to-lab: J616s and Neo. (1) apple_clpc_hw_t6040 is the J616s table (shared with T6041): the CPU map drops from DCS {1,2,5,5} / fabric {0,1,2,2} to DCS {1,1,3,4} / fabric {0,0,0,0}, and level_khz=4512000 rescales the E cluster. A busy CPU cluster on the J616s no longer gets DCS 5 / fabric 2; the table's own comment measures 237-242 GB/s there against 41 GB/s at 0/0. (2) The light-effort governor is in shared G17 code and on by default (g17_light_effort=0x8000), so it changes GPU effort on T8140 (Neo) and T6040 (J616s) as well as T6041.
w-needed: (after the asks) J616s: STREAM or perf bench mem memcpy on all cores, a kernel build time, and package power at idle and scrolling, base vs PR. Neo + J616s: gpubench/glmark2 and one game frame-time capture (vsync off and at 60 Hz) with the light governor on vs off.
ndings:
The busy-share floor (apple_clpc_gpu_level, 400 ms down-step hold, debugfs reading under the lock) is correct, and the host test covers it with three negative controls.
The T6040 CPU map change is a separate policy change with no M4 Pro data. The author says so ("I haven't tested an M4 Pro"). Either give T6041 its own table and leave T6040 as is, or bring J616s bandwidth/perf numbers.
The light governor can engage on games: a 120 fps game reads about a third busy (the clpc.c comment says so), under g17_light_enter=350 with short jobs, and at light effort the share then reads about 2/3, under g17_light_exit=750, so it stays light. Frame time at light effort is not measured. Page workload score -0.9 % is already a perf loss.
Five new 0644 module parameters (g17_light_effort, _enter, _exit, _job_enter_us, job_exit_us). Earlier review moved the CLPC thresholds out of module params into per-SoC data; the same should apply here.
Global statics (BUSY_WINDOW
*, LIGHT, QUIET_WINDOWS) assume one GPU instance, which holds today.
st: |
@AxxzyWasTaken thanks for the rebase and the power numbers. Three things before this can go in:

  1. apple_clpc_hw_t6040 is also the M4 Pro's table. The CPU map change (DCS 1,1,3,4 and fabric 0,0,0,0 at the top, and level_khz for the E cluster) lowers the memory floor a busy CPU gets on the M4 Pro, where the comment above the table measures 237-242 GB/s at DCS 5 / fabric 2. Please give T6041 its own table and leave T6040 as it is, or add M4 Pro measurements. We can run the M4 Pro side if you split it.
  2. The light-effort governor is in the shared G17 code and on by default, so it also changes the A18 Pro and M4 Pro. A 120 fps game reads about a third busy, under the 350 entry point, and stays under the 750 exit point once it runs at half effort. Please scope it per SoC (match data, T6041 first) until it has frame-time results on games, not only power.
  3. Please replace the five g17_light_* module parameters with per-SoC values, as was done for the CLPC thresholds.

linux#1242 rolotrealanis98 drm/asahi: g17: Name the cause and the clients of every M5 GPU recovery
ass: b verdict: CHANGES
b: ok ci: green (host-tests only; rust-build/kunit did not run on this head) mergeable: yes
sk-to-lab: Neo (T8140) and J616s (T6040) run G17 recovery. Only on the recovery path: report_clients() adds one pr_warn per listed or blamed queue on every G17 recovery (all SoCs, not just M5), and Sources::report() gets a third argument. The M5 record read is gated on self.m5.
w-needed: (after the asks) Neo: build with CLIPPY, then trigger one GPU recovery with the lab's fault reproducer; check the recovery completes, the new "recovery gen N queue Q ... client comm[tgid]" lines appear once per queue, and a following glmark2/vkcube run works. J616s: GPU smoke only.
ndings:
Code and commit messages are clean of host-driver references now (macos_restart_reason() and the field are gone, DSF name replaced). The record is log-only by default.
The PR description is stale and public: it still cites firmware TEXT addresses ("26C5035r primary TEXT 0x7808..0x7834"), "macOS DSF Pause Timeout", "the macOS kext's restart reason" and a log sample with "macOS restart reason Some(7)". RyanTheTide asked for the same removal in the code; the description needs it too.
m5_recovery_reason_blame is an experiment switch for an unproven reason mapping. Drop it and add the blame path when the reasons are confirmed; the log is the useful part.
report_clients() calls queues.visit_owners() inside service() after pending = None and before classify(); visit_owners is the same walk classify uses, so no new locking. Fine.
This head (rebased onto integration/aurora without #1173, status B base from self.graph) has not run on hardware. Only reason 5 was ever read; the other offsets are labelled unconfirmed. Acceptable for a log line.
st: |
@rolotrealanis98 thanks for moving this off #1173. Two asks:

  1. The PR description still has the firmware TEXT address range, the "DSF Pause Timeout" name, the restart-reason mapping and a log sample with "macOS restart reason". The code no longer has them; please update the description to match the commits.
  2. Please drop m5_recovery_reason_blame. The record is useful as a log line now; the blame path can come with the evidence that the M5 reasons mean what G17P's do.
    After that we will build it and run a recovery on our A18 Pro, since report_clients() runs on every G17 recovery.

linux#1477 fbalazsy J616s (M4 Pro) at EL2: hibernation and suspend-to-RAM (status only)
ass: b verdict: ASK-AUTHOR
b: missing(9bee928, c92a639, 9794539, f259e90; all authored by fbalazsy) ci: green on f259e90 (8 jobs) mergeable: conflict (DIRTY against 028d762)
sk-to-lab: high. J616s is our lab Mac, and the series touches shared code on all four: PM core hibernate, arm64 EL2 spin-table/hibernate, KVM arm64 counters, pinctrl-apple, nvme-apple, pcie-apple, brcmfmac, hci_bcm4377, dwc3-apple, cs42l84, macsmc, apple-sio, drm/asahi G17 (Neo), drm/apple, accel/ane, sleep.c (shared with T8140 Neo).
w-needed: after rebase: J616s full S2R/S4 rows (RyanTheTide ran 25/25 RTC wakes on 7aefde0 + 4 fixups; the 35 follow-up commits since have not run on our J616s, and the external-display run on this head is still owed by the author). Neo: s2r row (sleep.c part split). M2/M1 Pro: boot, s2idle, Wi-Fi, NVMe, audio jack.
ndings:
Blockers: conflicts with integration/aurora; four commits without Signed-off-by; 106 commits / +19.6k on a lab Mac's sleep path.
Open from testing: one of 25 wakes lost the dock port's USB3 side (t6040-usb2 "AP idle not acknowledged: -62", then USB2-only for the rest of the boot).
Description still says "Entry follows the order of macOS's quiesce and commit"; same wording issue as the comments that were cleaned up.
Overlaps #1314 in drivers/soc/apple/sleep*.c (both extend the generic driver); whichever lands second rebases. #1664 (T6041 S2R) builds on this.
st: |
@fbalazsy thanks for picking up the fixups and folding the T6040 controller into sleep.c. To merge this we need:

  1. A rebase onto the current integration/aurora (028d762); the branch conflicts now.
  2. Signed-off-by on 9bee928, c92a639, 9794539 and f259e90.
  3. Your external-display run on the rebased head, and whether you have seen the dock port come back USB2-only after a wake (phy-apple-t6040-usb2 "AP idle not acknowledged: -62") on your machine.
  4. Please reword "follows the order of macOS's quiesce and commit" in the description to describe the hardware order itself.
    One PR is fine with us; there is no need to split off the 35 follow-up commits. Once the rebased head is up we will run the J616s sleep rows and a regression pass on our other Macs.

linux#1314 niteria Deep sleep (suspend-to-RAM) for the M4 MacBook Air (T8132, J713) at EL2 (status only)
ass: b verdict: ASK-AUTHOR (push pending)
b: ok (all 52 commits; Erik's carry his then niteria's) ci: none reported on any head (conflicting branch, no merge ref) mergeable: conflict (DIRTY; rebase onto 028d762 done locally, J713 retest running per niteria 11-12 21:27)
sk-to-lab: Neo: sleep.c generic driver gains the T8132 entry, pcie-apple.c conflict with #1717 (Neo radio resume) resolved into one apple_pcie_config_read(), brcmfmac, apple-dart, rtkit reinit refactor (NVMe reset/resume on every Mac), AIC save path. M2/M1 Pro/J616s: pcie-apple config read wrapper, brcmfmac/bcm4377 changes, tb.c refactor (J616s USB4).
w-needed: after the push and green CI: Neo s2r rows (RTC wake cycles, Wi-Fi/BT back, NVMe write/fsync/read, display) and s2idle; M2 and M1 Pro boot + s2idle + Wi-Fi/BT; J616s boot + s2idle + a USB4/TB device if present.
ndings:
Carries the radio stack (#1378, #1381-#1385) and the dock stack (#1414-#1420) with Erik's sign-offs; those PRs close when this lands. Its copy of #1419 has the tb_switch_clx_disable() ret < 0 fix Erik's branch lacks.
Sleep controller node enabled on J713 only now; cio_s2r params default off, so a docked deep sleep is refused (listed under known limitations).
Known limitation in the description: speakerguardd 0.1 speakers near-silent after a wake on J713. Not a merge blocker for the kernel, but worth a tracking issue.
Overlaps #1477 in sleep*.c.
Remaining blockers: the push of the rebased head and CI on it. Another reviewer already asked the merge questions on 11-12 21:23 and niteria answered; nothing more to ask now.
st: |
Thanks for the conflict notes on #1722 and #1717. When the rebased head is pushed and CI has run on it, we will run the T8140 sleep rows and boot/s2idle on our other Macs against it, then merge.

@erikwb

erikwb commented Nov 13, 2026

Copy link
Copy Markdown
Contributor Author

Closing: this commit set landed in integration/aurora through #1314, which carried the whole docked deep-sleep series (#1414 to #1420). The follow-up that turns the docked deep sleep on by default is #1855.

@erikwb erikwb closed this Nov 13, 2026
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.

3 participants