Repository navigation
drm/asahi: g15: t6031: build the J516C board data without a board file - #249
Conversation
The J516C hardware record took 32 words from asahi/t6031-j516c-board.bin, a per-machine file that installs do not ship, and the driver compared the three images it built against the file before starting the firmware. A J516C without the file could not start the GPU. Take those 32 words from sources every J516C has, as the T6030 path does with its templates: - The six GPU sensor rows (18 words) are entries of the ADT's /arm-io/aon-ptd ptd_data_table, which the boot loader already exports (the phram "adt" region the T6030 path reads): the "RT" entries of kind g (four), r and f. The device tree does not carry the table. Any other count is refused. - The maximum power, the relative powers of DVFS states 1..7 and the MTR fast-die and alarm masks (14 words) are not in the device tree or the ADT. They become the J516C reference machine's values (board::record::J516C), as the T6030 templates carry their reference machine's. The device tree's operating-point powers stay unused; they are a boot-loader stand-in. hw::Board::Blob becomes hw::Board::DeviceTree(&Reference): no firmware request, no file digest, no file cross-check. The firmware identity pins and the apple,j516c machine match are unchanged. board/blob.rs and the RUST_FW_LOADER_ABSTRACTIONS select go. The off-tree J516C fixture test now builds the record from a saved device tree and ADT with the reference words and checks it against the machine's own record: byte-identical except the 42 words shown unneeded earlier. g15-zero-words-tests.py follows the new sources (14 reference + 18 sensor row + 42 zero words = the original 74); its hw.rs and prepare_board() pins are refreshed for the Board change, the T6030 arms are unchanged. Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Naeem Malik <malik2015naeem@gmail.com>
chriskearney
left a comment
There was a problem hiding this comment.
Source review at aeffd0e53b4218633bd1f35fcdfe9e27188a5922: this addresses the missing board-data path we discussed. I found no concrete blocker in the bounded source/host review.
The 18 sensor words now come from the existing ADT export, and the 14 reference words are explicit in record::J516C. The file request and runtime file comparison are removed. Firmware matching/lookup files remain byte-identical; the exact J516C restriction is preserved. The T6030 parser/performance path is unchanged apart from replacing its unused zero power array with a zero sensor array; its template/from-ADT golden checks pass.
Locally reproduced on Rust 1.98.1:
- asahi-g15 host suite: 79 passed, 2 private-fixture cases ignored.
- zero-word suite: 15 passed, 2 private-fixture cases ignored; all three authored mutants caught at runtime.
- Six independent controls pass for the exported ADT path, malformed/truncated trees, missing/excess sensor kinds, per-machine payload preservation and all 720 sensor-row orderings. Three additional compiled parser mutants fail the intended assertions.
I did not access the private fixtures or run a kernel/GPU. The PR correctly discloses that maximum power, relative powers and masks remain one reference machine's values; these host checks do not establish their applicability to every J516C configuration. The no-file native boot/compute results remain author-reported evidence.
Please add the exact tested kernel commit, m1n1 version and relevant boot options to the native result, so it is clear which combination was tested. The next integration step is this data path together with #246, m1n1#19 and the matching package/image delivery. Their changed source-file sets are disjoint. The reported no-file boot covers the data-path change; automatic boot from the shipped installer still needs that combination.
Kernel CI run 40598957505 is still in progress at this read-back. This is source feedback, with no merge or hardware clearance.
Generated with Codex.
|
Boot details for the J516C test boot (2026-10-28, board file removed from Kernel
Boot chain
Result
|
There was a problem hiding this comment.
CI needs one host-fixture update at aeffd0e53b4218633bd1f35fcdfe9e27188a5922 before merge. The driver source assessment above remains positive.
ci / host tests (tools) fails in tools/asahi/g15-maintained-handoff-tests.py: it extracts the production GpuAdt and parse() into generated actual.rs, but that generated crate has no record module. The new record::SENSOR_ROW_COUNT references fail with E0433 before the positive host control runs.
I reproduced the failure locally. The same unchanged fixture passes on base 937b727e, including its five mutants. A fixture-only correction also passes against this head with all five mutants caught: read the SENSOR_ROW_COUNT declaration from production g15/board/record.rs and put it in a mod record { ... } in the generated Rust before the extracted helpers. This keeps the new production type intact and retains the existing admission checks. Please include that correction and rerun the required CI.
CI log: https://github.com/omacom/linux-aurora/actions/runs/40598957505/job/122524644106
Thanks for the native boot details in #249 (comment). Two follow-ups for the release path:
- The tested
ec383c8dcommit does not resolve through the GitHub API. Please make the source revision available, or provide its source-only diff toaeffd0e5, so the stated doc/test-only difference can be checked. - The successful desktop used Mesa
aurora-wip4911479under/opt/mesa-g15c. Correction to my earlier Mesa next-step wording: the currentmesa-aurorarecipe already pins7c86130d26cecd50c7af1ac01ff3fa1705d58045, whoseagx_chip.cadmits G15C and whoseagx_g15_wire.chandles its retained kernel wire. Your reported package refusal does not establish missing G15C support in that source. Please provide the exactmesa-auroraversion and refusal message so the package/kernel pairing can be diagnosed before proposing a new support PR. The manual handoff/display/module settings in this boot establish the no-file data-path result; they do not yet establish the default installer combination.
No private machine data is needed for these source/fixture follow-ups. The one-machine power/mask limit remains as documented.
Generated with Codex.
The handoff test compiles GpuAdt and the T6030 ADT parser from adt.rs on the host. GpuAdt now carries the T6031 sensor rows, sized by board::record::SENSOR_ROW_COUNT, which the generated source did not define, so the test no longer compiled. Take that production constant from board/record.rs into a generated record module, and hash board/record.rs with the other sources in the receipt. Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Naeem Malik <malik2015naeem@gmail.com>
There was a problem hiding this comment.
The requested fixture fix is resolved at 23a7bd131f1b9978d23038bdd61f46479bc3b51d. I verified that this successor changes only g15-maintained-handoff-tests.py; all production files from the prior source review are unchanged.
The exact author fixture now passes locally, with all five existing mutants caught at their intended controls. It imports the production SENSOR_ROW_COUNT into the generated record module and includes record.rs in its source receipt, as requested. I am withdrawing the earlier changes request for that resolved issue. The bounded driver-source assessment remains positive.
Full kernel CI 40603277062 now passes at unchanged head 23a7bd131f1b9978d23038bdd61f46479bc3b51d: self-test, plan, commits, device trees, compilation, both host suites, full kernel package, KUnit, emulated boot and result. Attestation is normally skipped for a PR. The earlier head's host-fixture failure remains historical and resolved. There is no remaining concrete source/CI blocker from this bounded review.
The Mesa question is also narrowed: the group clarification identifies the refusing package as October 17 af3d9c6fc84, whose source does refuse G15C. The current recipe's 7c86130d source explicitly admits G15C and adapts its retained kernel wire. Naeem reports Vulkan/GL and 200,000 compute jobs working with that current package; its desktop session is still untested. Please update the earlier boot-details comment to distinguish the old package refusal from the current package result.
The tested-source binding is now checked against Naeem's public follow-up: full commit inventories against their identical parent, followed by direct blob comparisons, show only three differences between ec383c8ddc898b4c35596ac214201fb837dceae5 and aeffd0e5. They are an off-tree fixture helper under #[cfg(test)], one documentation comment, and the corresponding host-test golden hash. 23a7bd13 then adds only the reviewed host-fixture fix. No runtime driver code differs between the reported booted revision and this review. This binds the reported runtime result to the reviewed driver source; it does not independently verify the hardware run or assert identical kernel binaries.
The 14 one-machine reference words and the combined default-boot installer remain disclosed evidence/delivery limits. No reviewer hardware execution or whole-installer qualification is implied.
Generated with Codex.
Resolved by23a7bd13: author fixture includes production SENSOR_ROW_COUNT namespace; positive control and all5 existing mutants pass locally. Successor CI remains pending. Source feedback at review5675878969.
|
Thanks for the review. All three points: 1. Host fixture. Fixed in 23a7bd1 ( 2. Tested revision. ec383c8 was a local commit on the M3 and was never pushed; it is now on the fork as
No non-test code changed, so the booted kernel is the same as one built from aeffd0e. 23a7bd1 changes only the host test. 3. Mesa. The refusal came from
So I see no missing G15C support in 7c86130d, and no Mesa support PR is needed for this path. The greeter and desktop on the packaged Mesa are not tested yet; that needs the local sddm and uwsm overrides removed and a reboot. |
maralcbr
left a comment
There was a problem hiding this comment.
Thanks @malik-na, this is the right shape. It follows the T6030 templates path, drops the firmware request and the file cross-check, and keeps the firmware identity pins and the apple,j516c match. I checked the new sources against the code:
- The relative-power table takes the seven reference values for states 1..7 and 100 for the top state, and refuses any other state count (
g15/board/record.rs:518-525). build()now zeroes the record and writes the reference words and sensor rows itself, so nothing carries over from the buffer (record.rs:846-864). The newstale-buffer-keptmutant covers that.sensor_rows()refuses anything other than fourRT?g, oneRT?rand oneRT?fentry in whole 12-byte rows (record.rs:778-799). The rows are copied verbatim, sotail()derives the same keys as before.- The ADT export is read before anything firmware-visible, and a missing export fails cleanly (
adt.rs:485-517,glue.c:233-257). The T6030 path is unchanged apart from an unused zero sensor array. - Nothing under
drivers/gpu/drm/asahiuses the firmware loader any more, so droppingselect RUST_FW_LOADER_ABSTRACTIONSis safe.
Locally at 23a7bd1: asahi-g15/run.py 79 passed (2 ignored), g15-zero-words-tests.py --mutations 15 passed with all three mutants caught, and g15-maintained-handoff-tests.py --mutations passes with all five mutants failing. CI is green. The compile job skips Rust, but the package job builds with asahi.config, which sets CONFIG_DRM_ASAHI_G15=y, so the changed driver code was compiled.
No blockers. Small things, fine as follow-ups:
g15/initdata.rs:193-194: theprepare_board()comment still talks about a digest and "a later firmware request". Neither applies to the T6031 arm now.- Optional: log the built record's SHA-256 in
device_tree_board()(initdata.rs:178), astemplates()does atinitdata.rs:143-156. That would let a second J516C's dmesg show straight away whether its record differs, which is how the known gap gets closed. - Please edit the boot-details comment so it separates the October 17
mesa-aurorarefusal from the current package result, as asked earlier. Your follow-up explains it, but the first comment still reads as if the packaged Mesa refuses G15C.
Still open, and not a code issue: whether one J516C's maximum power, relative powers and MTR masks are right for every J516C is a maintainer call. The PR states the limit clearly, and T6030 does the same with its templates. A record from a second J516C would settle it.
For release (not this merge): the T6031 path now also needs the boot loader's phram adt export, so the installer combination (#246, omacom/m1n1#19 and the package/image) has to carry it. It also still needs the greeter and desktop on the packaged Mesa, and a cold boot on a J516C from the shipped installer. The no-file boot and compute results are yours. I have no J516C to repeat them on.
| dev_info!( | ||
| dev, | ||
| "board data: {} images from the device tree match {} ({} record words from the file, {} file words left zero), {} mW maximum power, {} MHz top operating point\n", | ||
| "board data: {} images from the device tree and the ADT sensor rows, reference maximum power {} mW and MTR masks, {} MHz top operating point\n", |
There was a problem hiding this comment.
Optional: log the built record's SHA-256 here, as templates() does (:143-156), so a second J516C's report shows straight away whether its record differs.
Log the SHA-256 of the hardware record built from the device tree, the ADT sensor rows and the reference words, as the T6030 templates path does for a record that differs from its reference. A report from another J516C then shows at once whether its record matches. Also bring the prepare_board() comment up to date: the T6031 arm no longer has a digest to check or a firmware file to request. Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Naeem Malik <malik2015naeem@gmail.com>
|
Thanks @maralcbr. All three are done:
Checks on 79f7891: every G15 suite in Agreed on the release items: the installer combination has to carry the phram |
scottjones
left a comment
There was a problem hiding this comment.
We checked this at 79f7891 (23a7bd1 plus the digest log and the comment fix), alone and merged with #246, clean both ways.
"Without a board file" means the J516C GPU no longer requests asahi/t6031-j516c-board.bin. The 18 sensor-row words now come from the ADT export that m1n1 already writes on every boot (/arm-io/aon-ptd ptd_data_table). The 14 words that neither source carries (maximum power, relative powers of states 1..7, MTR masks) are fixed in record::J516C (g15/board/record.rs:228). The other 42 words were already left zero before this PR, so no data from the file is lost.
Before this PR, a J516C without that exact pinned file failed with ENOENT, and a J516C whose device tree differed from the file was refused. Now every J516C on one of the two pinned GPU firmware stubs (macOS 14.7 or 14.8, hw/t6031.rs:134-160) is admitted, with its own operating points and sensor rows and the reference machine's 14 words. The J514C is still refused by the apple,j516c match, and T6030 is unchanged.
Thanks for adding the record digest and the comment fix.
We ran the host suites at 23a7bd1. 79f7891 adds only a log line and a comment on top of that. asahi-g15/run.py gave 79 passed and 2 ignored, and g15-zero-words-tests.py --mutations and g15-maintained-handoff-tests.py --mutations both pass, on this PR alone and merged with #246. Writing the fast-die mask into the alarm mask fails record_words, and swapping the r and f row order fails sensor_rows_from_the_ptd_table.
chriskearney
left a comment
There was a problem hiding this comment.
Approved at 79f789160c73e1413cf817a1fb4c0beb8ac914f7.
The source/fixture review at 23a7bd13 stands. The only successor delta is the J516C hardware-record SHA-256 log and the corrected prepare_board() documentation. The log uses the existing digest helper with an immutable record borrow; it changes no record values, admission checks, firmware pins or T6030 control flow. The maintained-handoff and zero-word controls also pass on the combined successor source, with their mutations caught; those host controls do not execute the new log line.
Combined-kernel boot regression results: #246 at 41aa66da plus this PR at 23a7bd13, with m1n1 5d17a034, passed boot health, GUI smoke and short native GL/VK checks on M3 Pro, M1 and M1 Air. Every machine was restored and audited. Those machines execute paths unchanged by 79f78916; this does not claim that we booted the successor binary or exercised its J516C log.
Full kernel CI 40616429413 now passes on this exact head: self-test, plan, commits, device trees, both host suites, full kernel package, KUnit, emulated boot and result. The package job supplies the changed Rust driver build evidence with G15 enabled; the separate ci / compile job skips Rust. Attestation is normally skipped for a PR.
No remaining source or CI blocker from this review. The 14 one-machine reference words remain an explicit maintainer judgment. Naeem's Max boots are author evidence, not our hardware validation; the matching Stage2/default cold boot and packaged-Mesa greeter/desktop from the shipped installer remain release checks.
Generated with Codex.
e3ffe88
A J516C (16-inch M3 Max) install could not start the GPU without
asahi/t6031-j516c-board.bin, a per-machinefile that installs do not ship (probe ends in ENOENT). This builds the J516C's hardware record, Globals and Power
from sources every J516C has, following the T6030
templates()path.What changes
adtregion the T6030path reads):
/arm-io/aon-ptdptd_data_table, theRT?g(four),RT0randRT0fentries. No m1n1 change.Any other count is refused.
neither the device tree nor the ADT. They become the J516C reference machine's values (
board::record::J516C),as the T6030 templates carry their reference machine's. The device tree's
opp-microwatt(a boot-loaderstand-in) is no longer read.
hw::Board::Blob→hw::Board::DeviceTree(&Reference): no firmware request, no file digest, no boot-timecomparison against the file.
board/blob.rsandselect RUST_FW_LOADER_ABSTRACTIONSare removed.apple,j516cmachine match, and the T6030 path.Testing (J516C, measured on one machine)
except the 42 words shown unneeded earlier (af6c3b7).
board data: T6031 G15C images from the device tree and the ADT sensor rows, reference maximum power 116739 mW and MTR masks, 1380 MHz top operating point, firmware READY, cores[10,10,10,10], desktop on the GPU; 400,000 compute command buffers PASS (7064/s); no G15 faults or recoveries.
tools/testing/selftests/drivers/gpu/asahi-g15/run.py: 79 pass.tools/asahi/g15-zero-words-tests.py --mutations: 15 pass, all mutants caught. It now partitions the original 74 words as 14 reference + 18 sensorrow + 42 zero; its
hw.rsandprepare_board()pins are refreshed for the Board change (the T6030 module,templates()andfrom_adt()pins are unchanged).Known gap
The 14 reference words are one J516C's. Another J516C gets its own operating points, voltages, gains and sensor
rows, but the reference machine's maximum power and masks. A record from a second J516C would show whether they
differ between machines.
Written for this tree; measured on a J516C.
🤖 Generated with Claude Code