Repository navigation
drm/apple: v14.7: J514M panel nits and swap timestamps - #457
scottjones wants to merge 2 commits into
Conversation
The J514M runs the pinned J516C 14.7 DCP image, like the J516M, but its board record lacks the two flags the J516M record has. Without .swap_timestamps the image gets zero presentation stamps and holds the 120 Hz panel to 60 fps. Without .panel_nits, and with apple,j514m missing from dcp_v14_panel_nits_admit(), no swap level block is sent, DCP logs "IOMFB: Could not find Backlight service" and no apple-panel-bl backlight is registered, so brightness keys and brightnessctl do nothing. Give the J514M record both flags and admit apple,j514m to the bounded nits path, as for the J516M. Flip the KUnit and host tests that held the old values, and check that the J514M internal panel stamps its swaps and the external role does not. Two J514M owners (webdevtodayjason and techgyani) reported this change working in #333: 120.07 fps measured at 120 Hz, apple-panel-bl with a maximum of 500, and brightnessctl set and restore. Fixes: 5b4485d ("drm/apple: admit J514M native display on normal boots") Link: #448 Link: #333 Assisted-by: Claude:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Scott Jones <scottajones@gmail.com>
13bf841 to
0c6c113
Compare
|
Fixes #448. @Faeric a test build of this PR is ready: CI run 43150587224, |
maralcbr
left a comment
There was a problem hiding this comment.
Thanks @scottjones, this is the right fix and it's tightly scoped. I found no blockers.
The J514M record now matches the J516M's (iomfb_v14_7.c:250-253 vs :215-218) on the same pinned J516C image, and dcp_v14_board_admits still refuses any other image at probe (:1800-1804). The nits path still needs both the record flag and an admitted root (:1766-1770), and it only reaches the internal role (:397). Swap stamps still skip the external role (:1780), and the new asserts at v14-board-test.c:131-132 pin both sides. The flipped checks are real flips, not loosened ones: t6034-display-tests.py:54 now requires both flags on the J514M record. CI passed: compile, boot, the KUnit job (its aurora.kunitconfig enables the display suites) and the tools host tests, which run t6034-display-tests.py.
Nits, fine as follow-ups:
- Stale comments on which boards stamp.
v14-board-test.c:6-8and:248still say only T6031/J516C records stamp their swaps. Suggest: "the internal panel of a record withswap_timestampsset (J516C, J516M, J514M)". Please don't describe it by firmware image: the J514C runs the same image and doesn't stamp (v14-board-test.c:261). - Pre-existing: the J514M record's comment at
iomfb_v14_7.c:239mentions panel geometry, but the record has none. It's worth rewording sometime.
Behaviour change to note: J514Ms on a normal boot now get stamped swaps and apple-panel-bl by default, with only apple_t6034_display.enable=0 as an opt-out. That matches the J516M. Accepting it is Marcelo's call.
This build itself hasn't run on a J514M yet, so release stays gated on a hardware report at this head. That means the steps in the PR body, ideally with one lid-close resume.
Reviewed at 0c6c113ac69c.
— Marcelo's agent
The v14 board test still said only the T6031 (J516C) internal panel stamps its swaps; the J516M and J514M records now set swap_timestamps too. Name the records by the flag rather than by SoC or firmware image, since the J514C runs the same image and does not stamp. The J514M record's comment also mentioned panel geometry that the record does not set. Comment-only; no functional change. Assisted-by: Claude:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Scott Jones <scottajones@gmail.com>
|
Comment nits fixed in cbe32f3: the |
|
Tested the CI package from run 43150587224 on my J514M (14" M3 Max, 14 CPU / 30 GPU, os-fw 14.7, Omarchy edge, Hyprland 0.57.1). Works. Kernel: Brightness: pass
120 Hz: pass
When the test window shares the screen with a busy terminal, I get about 83 to 90 fps: roughly 60% of frames at 8.3 ms and the rest at about 16.7 ms. That happens with both vkcube and GTK. Fullscreen gets the full rate, so this looks like compositing in Hyprland, not this patch. Before this build, the same windowed test sat at exactly 60.00 fps. No new kernel errors or DCP warnings during the tests. |
maralcbr
left a comment
There was a problem hiding this comment.
@scottjones re-reviewed at cbe32f3: both comment nits are fixed, and I found no blockers.
- The
v14-board-test.cheader (:6-9) and thev14_board_swap_timestampscomment (:249-252) now name the records byswap_timestamps. That matches the tree: J516C, J516M and J514M are the only v14 records that set it (iomfb_v14_7.c:192,:216,:251), and the J514C record doesn't. iomfb_v14_7.c:239no longer mentions panel geometry. The record still sets none.
The interdiff only touches comments, so last round's code review still applies. The nits path still needs both the record flag and an admitted root (iomfb_v14_7.c:1766-1770), and swap stamps still skip the external role (:1780). CI passes on compile, boot, KUnit and all three host-test jobs. "attest" and "approve" were skipped, and neither is a test.
Behaviour change, unchanged from last round: J514Ms on a normal boot now get stamped swaps and apple-panel-bl by default, with only apple_t6034_display.enable=0 as an opt-out, the same as the J516M in #429. Accepting it is Marcelo's call.
On release: thanks @Faeric for the J514M report. It shows brightness and 120 Hz working on the 0c6c113 build, and that build has the same code as this head. The step 6 dmesg output isn't posted yet, and there's no lid-close resume. Whether that's enough for release is Marcelo's call.
Reviewed at cbe32f37b285.
— Marcelo's agent
|
Tested the CI package from run 43150587224 on my J514M (MacBook Pro 14", M3 Max, 14 CPU / 30 GPU), Omarchy edge. Works.
Thanks for the fix. |
Summary
Fixes #448
The J514M runs the same J516C 14.7 DCP image as the J516M, but its board record was missing the two flags #429 gave the J516M. Without them, the 120 Hz panel presents at 60 fps, and no backlight device is registered (
IOMFB: Could not find Backlight service). This PR gives the J514M the same settings.static const struct dcp_v14_board dcp_v14_board_j514m = { ... .promotion = true, .log_bringup = true, + .swap_timestamps = true, + .panel_nits = true, }; dcp_v14_panel_nits_admit() ... || machine_is(ctx, "apple,j516m") + || machine_is(ctx, "apple,j514m")The tests that held the old values are flipped:
v14_board_j514m(KUnit) and the J514M record check intools/asahi/t6034-display-tests.py.v14_bl_boardsnow also admits the J514M root.Evidence
Before: On edge aurora16, from j514m: built-in panel presents 60 fps at 120 Hz and has no brightness control on aurora16 (board record lacks .swap_timestamps / .panel_nits) #448 (Eric's J514M): 60.00 fps in the 120 Hz mode, measured with the GTK 4 frame clock.
/sys/class/backlightis empty.After: From j514m (MacBook Pro 14-inch, M3 Max, 14 CPU cores): test results #333, two J514M owners (webdevtodayjason and techgyani) running this same change: 120.07 fps,
apple-panel-blmax 500, andbrightnessctlset and restore works.Host tests on this branch, all passing:
t6034-display-tests.py,dcp14-panel-tests.py,dcp14-backlight-tests.py;t6030-display-admission-tests.py --mutations;g15-display-order-tests.py,g15-t6034-profile-tests.py,g15-zero-words-tests.py,g15-j514c-display-default-tests.py.The flipped
t6034-display-tests.pyfails against the old J514M record.Kernel build: clean with the aurora16 config. The display KUnit files compile with
CONFIG_DRM_APPLE_DISPLAY_KUNIT_TEST=y, but weren't run.Hardware: this branch itself hasn't been tested on a J514M yet. The steps are below.
Test on a J514M
CI built this PR merged onto aurora16 (
bed544b+0c6c113): Kernel CI run 43150587224. Download thekernel-package-…artifact from that page (you need to be signed in to GitHub; it's kept for 14 days) and unzip it. It contains:linux-aurora-7.2.11.aurora16-1.782-aarch64.pkg.tar.zst, sha2560e411668b9cbbfcdcd38c96b5e8a76a1177c5bdb40748b15d0ea426ed47354b7linux-aurora-headers-7.2.11.aurora16-1.782-aarch64.pkg.tar.zst, sha256d4bd140ca6f09b89c421079f57c03aefb6b9951566895d36b488e9a9d72d811f(only needed if you havelinux-aurora-headersinstalled)This build replaces your stock
linux-aurora(7.2.11.aurora16-1); it doesn't install next to it. The rollback steps are below.uname -rshould print7.2.11-16-1.782-ARCH(stock is7.2.11-16-1-ARCH).uname -r, the output of steps 3 and 4, andsudo dmesg | grep -iE "dcp|iomfb|backlight" | tail -40here.To go back to stock:
sudo pacman -Syuu # -uu lets pacman downgrade to the repo's 7.2.11.aurora16-1Or, offline, reinstall the stock package from your cache:
sudo pacman -U /var/cache/pacman/pkg/linux-aurora-7.2.11.aurora16-1-aarch64.pkg.tar.zst # plus the matching linux-aurora-headers package if you had itThen reboot and check
uname -ris back to7.2.11-16-1-ARCH.The change only touches the J514M panel settings, and CI's boot test passes, but that test runs in an emulator, not on a Mac. If the test kernel doesn't reach a login, say so here before trying anything else.
Merge Danger
Door: two-way
Two flags on one board record, plus one machine-list entry. Reverting the commit restores the old behaviour.
Blast Radius: J514M only
Only
apple,j514monapple,t6034with the pinned J516C image is affected. Other boards' records and the external display role are unchanged:dcp_v14_board_stamps_swaps()still skips the external role.🤖 Generated with Claude Code