Repository navigation
drm: apple: read property 19 as the panel level only on 14.7 firmware - #175
Conversation
The 14.7 firmware interface publishes the panel level under property 19 instead of 15, and pr_publish now takes both IDs as the level on every firmware version. 13.x firmware publishes property 19 as well: on a J414s with 13.5 firmware it arrives with value 1 right before the level under 15 (9175040, 140 nits at the 65536 scale). Read as nits it is 0. The backlight device registers from a work item that reads the stored level whenever it runs. If it runs between the two publishes, the backlight registers at 0, the next swap after power-on sends the lowest DAC value, and the panel stays at that level until userspace writes a brightness. A first-boot setup that runs before any desktop session leaves the panel very dim. Before the 14.7 interface was added, only property 15 was read, as on 12.3 and 13.x firmware. Take property 19 as the level only in the 14.7 and later templates. Their behaviour is unchanged. Fixes: 20ea89d ("WIP: drm: apple: Add 14.7 FW ABI") Signed-off-by: DjDeveloperr <43033058+DjDeveloperr@users.noreply.github.com>
maralcbr
left a comment
There was a problem hiding this comment.
Thanks @DjDeveloperr, this is the right fix and the root-cause write-up is clear. Looks right to me, with no code changes needed.
- The gate restores the old behaviour exactly.
iomfb_template.c:479-481takes property 19 only in the 14.7 and 26.x instantiations (iomfb_v14_7_tpl.c:45,iomfb_h17p.c:254). That means 12.3 and 13.x (iomfb_v12_3.c:38,iomfb_v13_3.c:38) read 15 alone, as before 20ea89d, and a property 19 publish on 13.x now ends up in thedev_dbgdefault. 14.7 keeps both IDs, so M3 and later are unchanged. - The race is as you describe.
dcp_backlight_register()computesbrightness.dacfrombrightness.nitsonce (dcp_backlight.c:564). Later level publishes only updatenits, so the swap atiomfb_template.c:2576keeps sending the latched DAC. The bad case is that DAC sample landing between the two publishes. checkpatch.pl --strictis clean, the sign-off is there and the Fixes tag resolves to the right commit.
I'll approve once ci / result is green. commits and compile were still pending when I looked. KUnit, boot and package skipping on the PR run is expected, because the merge queue runs them.
Follow-up, not for this PR: the latch itself is still there, so any other low value stored before registration would pin the DAC the same way. Worth recomputing brightness.dac from later level publishes until userspace first sets a brightness.
For release (not the merge): please post the staged J414s boot when you have it, with the published property IDs, the level the backlight registers at and the first DAC sent. I'll also cold-boot our M2 Max through first-boot setup on the console to confirm the panel comes up at its real level.
|
Hardware result on the J414s (M2 Pro, 13.5 firmware): the shipped 7.2.10 kernel with this change, plus a log line for every This confirms the premise: on 13.5 firmware, property 19 is published with value 1, about 0.5 ms before the level under 15. With the change, 19 no longer reaches the level case. The backlight registered from 15 alone, at 140 nits, and the first swap carried that level. |
maralcbr
left a comment
There was a problem hiding this comment.
ci / result is green now, approving. Thanks @DjDeveloperr.
Move to omacom/linux-aurora 70d5dcedfa7fb7a3e7bd0cdb33d8315d99250142, 25 commits past 1b4997bb. The kernel stays 7.2.10. It brings: - a fix for very dim internal panels at first boot on Macs with 13.x system firmware (omacom/linux-aurora#175) - the Apple watchdog driver built in, fixing resets during MacBook Neo first boot (omacom/linux-aurora#171) - MT7932 Wi-Fi WPA3-SAE and OWE station offload, up to four directed SSIDs per scan and a world regulatory fallback (omacom/linux-aurora#172) - 128-bit integer types in the tools headers, so the libbpf build behind resolve_btfids works with newer arm64 UAPI headers The kernel-owned asahi.config changes only APPLE_WATCHDOG (m to y). The asahi DRM UAPI is byte-identical, so mesa-aurora stays matched. _aurorarel goes 1 to 2 and pkgrel resets to 1, giving 7.2.10.aurora2-1.
M2 Max laptops on the 7.2.10 kernel come up with the built-in panel very dim through Omarchy's first-boot setup, and brightness only becomes controllable once the desktop starts. The 7.1.12 kernel shipped in the previous image did not do this.
Root cause:
iomfb_cb_pr_publish()take both 15 and 19 as the level. The case is not gated byDCP_FW_VER, so the 12.3 and 13.x templates take 19 as well.1(0 nits), then9175040(140 nits at the 65536 scale, the real level under 15).bl_register_wq, anddcp_backlight_register()readsbrightness.nitswhenever the work runs. If it runs between the two publishes, the backlight registers at 0 andbrightness.dacbecomes the table minimum. Later level publishes do not recompute it. The swap after power-on sends that DAC, and the panel stays at it until userspace writes a brightness.Change: take property 19 as the level only in the 14.7 and later templates. The 12.3 and 13.x templates read 15 alone, as before 20ea89d. The 14.7 and 26.x templates are unchanged, so M3, M4, M5 and MacBook Neo behave as before.
Validation:
appledrm.koagainst the shipped7.2.10.aurora1-2headers with LLVM 18, as the package is built.1, then9175040), and the backlight registered from whichever value was stored when the work ran.