Repository navigation
[Joshua's agent] arm64: dts: apple: t6050-k114c: describe the camera (draft) - #1756
Draft
joshuaswarren wants to merge 15 commits into
Draft
joshuaswarren wants to merge 15 commits into
joshuaswarren wants to merge 15 commits into
Conversation
Contributor
Author
|
[Joshua's agent] GREEN Head 62fdc34 (base Findings
Checked
Not checked: the K114c ADT values (segment addresses and sizes, IMX958 config index), firmware ranges against the iBoot memory map, a boot of this exact tree, a full |
joshuaswarren
force-pushed
the
w7y/k114c-isp
branch
from
November 13, 2026 04:26
62fdc34 to
79a494e
Compare
Contributor
Author
|
[Joshua's agent] Head moved to
|
Contributor
Author
|
[Joshua's agent] GREEN Head 79a494e (top commit only, Findings
Checked
Not verified: no boot of this tree; K114c domain order not tested on hardware (the earlier note that the PR streamed on K114c was with the old order; the new order is untested there); ADT values (segment addresses, config index) and firmware ranges against iBoot; full GREEN at 79a494e |
Contributor
Author
|
GREEN at 79a494e GREEN for the DT commit What I ran at this head (arm64 cross, dtschema 2026.9 in a venv)
Disclosed: the indicator property is dormant on this tree. In the compiled K114c dtb, the target Dependency on the base. The property and its binding come from #1744, where I have CHANGES at Not verified on my side
|
joshuaswarren
force-pushed
the
w7y/k114c-isp
branch
from
November 13, 2026 05:04
79a494e to
d109144
Compare
Contributor
Author
|
Rebased onto the new #1744 head |
joshuaswarren
force-pushed
the
w7y/k114c-isp
branch
2 times, most recently
from
November 13, 2026 05:09
a68a2f4 to
604da6b
Compare
Contributor
Author
|
Rebased onto the new #1744 head |
Contributor
Author
|
[Joshua's agent] GREEN
|
Contributor
Author
|
GREEN at 604da6b GREEN for the DT commit What changed since my GREEN at 79a494e (comment 6844551294)
Still disclosed from the last round: in the compiled K114c dtb, Dependency. The stack under this commit is #1744 at Not verified on my side: the ADT-derived addresses, |
joshuaswarren
force-pushed
the
w7y/k114c-isp
branch
from
November 13, 2026 05:38
604da6b to
4550bdf
Compare
Contributor
Author
|
Rebased onto the new #1744 head |
joshuaswarren
force-pushed
the
w7y/k114c-isp
branch
from
November 13, 2026 05:57
4550bdf to
7ea893b
Compare
Contributor
Author
|
Rebased onto the new #1744 head |
joshuaswarren
force-pushed
the
w7y/k114c-isp
branch
from
November 13, 2026 06:08
7ea893b to
c671c05
Compare
Contributor
Author
|
Rebased onto #1744 head |
Boards without a hardware camera light have no way to show that the camera is in use. Let a DCP carry a small kernel-owned square on a free swap slot, named per board by apple,camera-indicator-surface. The camera driver calls apple_dcp_camera_indicator_set(dcp, on). The call commits the square non-blocking and waits up to one second for a present that carried it. It returns 0 once one did and an error otherwise. It is a software indicator, not a privacy guarantee: the camera never depends on the answer. The call holds the DCP's device lock, so the DCP cannot unbind under it. The reserved overlay plane belongs to the kernel. Any userspace commit that changes it (FB_ID=0, CRTC_ID=0, a client framebuffer, a move) is put back to the plane's current state instead of failing, so a blind clear of every plane, as the console restore does, still succeeds. Carried unchanged, or disabled with the CRTC it sits on, the plane is left alone. The state policy is a pure header (dcp-indicator.h) with a KUnit suite, apple-dcp-indicator. Proof: the policy suite builds as a userspace program, 5 cases, 0 failed. drivers/gpu/drm/apple builds with LLVM, KUnit enabled; no kernel ran it and no hardware has shown the square yet. Signed-off-by: Joshua Warren <816217+joshuaswarren@users.noreply.github.com>
The camera now requests a software indicator from the DCP named by the optional apple,camera-indicator property, before the firmware starts the channel, and removes it after the stream stops, on every stop path. The indicator never gates streaming: if the DCP is absent, unbound or cannot show the square, the driver warns once and the camera streams anyway. It is not a privacy guarantee. The DCP lookup never defers probe, and holds a device reference only when a device exists. VIDEO_APPLE_ISP depends on DRM_APPLE || !DRM_APPLE so a built-in driver cannot link a modular DCP. KUnit tests pin the contract: a failed request still streams, no DCP means no call, the request comes before the channel start and the removal after the stop, and a failed channel start or failed firmware still removes the square. Proof: built with LLVM with KUnit enabled (compile only; no kernel ran the tests and no hardware showed the square). Signed-off-by: Joshua Warren <816217+joshuaswarren@users.noreply.github.com>
…d mutex A review of the camera indicator found these faults, all in the DCP half. set(true) waited up to 1 s for the present, so a DCP that never composites the slot delayed every stream start by a second and then showed no dot. It now returns once the request is committed, and a delayed work confirms the present or withdraws the square after 1 s with a warning. Releasing the plane cleared has_surf, a device-tree constant, so after a rebind of the DRM master apple_probe_per_dcp built no plane and the indicator was gone until reboot. It also left a LIT state across the unbind because set(false) returned before it ended the state. The release now runs from the DCP's component unbind, while the DCP and the drm_device are both alive (a drmm action ran after the DCP's memory was freed), clears the plane, the framebuffer and the state, and keeps has_surf and surf. A DCP that took the ext failsafe path never ran dcp_indicator_probe, so a camera phandle pointing at it locked an uninitialised mutex. set() now tests has_surf, which is zero there, before it locks. The J700 surface-0 fix-up ran after dcp_indicator_probe, so surface 0 passed the free-surface test and then collided. The probe now runs after it. Also removes the unread indicator_fb field and two comments that said the camera stays off on failure. Tested: drivers/gpu/drm/apple builds with LLVM and no warnings. Not booted, KUnit not run. Signed-off-by: Joshua Warren <816217+joshuaswarren@users.noreply.github.com>
The ISP looked the DCP up once at probe and, if the DCP platform device did not exist yet, silently had no indicator for the life of the driver. It now keeps the phandle's node and looks the device up when a stream starts, and says so when the DCP is still missing. Without CONFIG_DRM_APPLE the property is not looked up, so the probe notice no longer claims an indicator the stub cannot show. The failure warning is rate-limited instead of once per boot, so a later failure (panel off, timeout) is visible. The KUnit case isp_test_stream_failure_lights_off expected the indicator-off call at step 2, but the stubbed firmware start takes step 2, so off is step 3 (on=1, firmware start=2, off=3). The expectation is fixed and now also checks the firmware start. Tested: the ISP builds with LLVM and no warnings. KUnit compiled, not run. Signed-off-by: Joshua Warren <816217+joshuaswarren@users.noreply.github.com>
The session, buffers and returns suites compile apple_isp_stop_capture and isp_cmd_send straight from the driver. The indicator change added the indicator hooks to the first and a KUNIT_STATIC_STUB_REDIRECT call to the second, and the host fixtures defined neither, so the three suites stopped compiling. The fixtures now define the redirect as a no-op and the two indicator helpers as 'no DCP, no call', with the dcp field. Tested: session, buffers, returns, queue and receive all PASS, and every negative control (send, overwrite, stop, reset; address, pool, duplicate, snapshot) still fails. Signed-off-by: Joshua Warren <816217+joshuaswarren@users.noreply.github.com>
…nors w7X found that the DCP lookup at stream start had no test: mutations that skipped apple_isp_indicator_bind() still passed every KUnit case, because the tests set isp->dcp by hand. Claude's review left two minors: a stale watch can withdraw a newer request, and a stream that runs across a DRM master rebind shows no dot afterwards. - isp: the platform device lookup goes through a stubbable wrapper. New case isp_test_dcp_bound_at_stream_start: no DCP means no dot for that stream, the next stream binds it and asks for the dot before the firmware start, and a bound DCP is not looked up again. - drm/apple: set(true) records a deadline. A watch armed for an earlier request that was stopped and asked for again returns when the deadline has not passed, so it cannot withdraw the newer request. - drm/apple: a request that is still wanted survives dcp_comp_unbind (dcp_ind_suspend). The first present after the next bind queues a resume that asks for the square again. A stop, a failed commit or a withdrawal ends the request, so a failing commit cannot loop. Release now drops the fb and the plane under the lock before the works are cancelled, so no present queues them again. - Pure-state case indicator_resumes_only_a_kept_request. Proof: DRM and ISP dirs build with no warnings (arm64, LLVM). ISP KUnit on arm64 QEMU, all suites pass including the new case; the pure-state suite built as a userspace program passes, and fails when dcp_ind_end() keeps the request. Not run: the resume path on a panel, and DRM_APPLE as n. Signed-off-by: Joshua Warren <816217+joshuaswarren@users.noreply.github.com>
The off commit is nonblocking. A flip still pending on the CRTC, plane or connector makes drm_atomic_helper_setup_commit() refuse it with -EBUSY, which a live compositor causes often. The stop path and the watch withdrawal logged that refusal and reported success, so the plane stayed enabled with no retry, and plane.c then restored the square after later userspace clears. The dot stayed on after STREAMOFF. Route the stop path and the watch withdrawal through dcp_indicator_off(). A -EBUSY keeps an off_pending request and queues off_retry (50 ms) until the commit lands; any other error is warned about as final. A new request, the plane going away, or release cancels the retry. The failed-on cleanup no longer commits off: a refused on commit changed nothing on the plane. Also queue the resume work inside the ci->lock section in swap_done, so release, which drops the fb under that lock, cannot cancel before it is queued. Proof: new case indicator_off_retries_a_pending_flip in the pure-state suite. It failed on a version that treats -EBUSY as done (6 expectations), and passes with the change (7/7). The DRM and ISP objects build with no warnings. The retry itself, and a real -EBUSY from a compositor, were not run on hardware. Signed-off-by: Joshua Warren <816217+joshuaswarren@users.noreply.github.com>
plane.c put back any userspace change to the reserved overlay, so a compositor that assigned a framebuffer to it saw the commit succeed while the kernel dropped its buffer, and could not fall back. A commit that clears the plane (FB_ID=0 and CRTC_ID=0), as the console restore does for every plane, still succeeds and is put back. Any other userspace change now returns -EINVAL from atomic_check, so the refusal shows in a TEST_ONLY commit. The kernel's own commit, a plane carried unchanged and a plane disabled with its CRTC pass as before. The decision moves from dcp_ind_plane_must_restore() to dcp_ind_plane_policy(), which returns PASS, RESTORE or REJECT. The old bool function is gone. Proof: indicator_plane_reservation now expects REJECT for an own fb and RESTORE for a clear. It failed to build against the old API, and fails (one expectation) on a version that restores instead of rejecting. The suite passes 7/7 in userspace, and the DRM objects build with no warnings. A real TEST_ONLY commit from a compositor was not run. Signed-off-by: Joshua Warren <816217+joshuaswarren@users.noreply.github.com>
When the on commit got -EBUSY from a pending flip, the request ended and the camera streamed with no square showing: the off hole in reverse. Both directions now go through dcp_ind_apply(). A -EBUSY keeps the request, records which commit was refused in dcp_indicator.retry, and queues the retry work; the work asks again through dcp_ind_retry() until the commit lands. A request that ends (stop, watch withdrawal), a new request, losing the plane and release all drop or cancel the retry. The on request still arms its 1 s watch once the commit is committed or queued, so a square that never lands is withdrawn. Any other error is final: set(true) returns it, and the work warns and ends the request. The retry decision and the queueing now live in the header behind a two-call seam (commit, queue_retry), so the pure-state suite drives the same code the work item runs. The earlier off_pending flag and dcp_ind_off_result() are gone. Proof: indicator_refused_commit_is_retried and indicator_retry_ends_with_the_request drive both directions with a fake commit and queue. A mutant with queue_retry removed fails 4 expectations, and one that does not retry an on commit fails 4. The suite passes 8/8 in userspace, and the DRM objects build with no warnings. The real work item and a real -EBUSY from a compositor were not run on hardware. Signed-off-by: Joshua Warren <816217+joshuaswarren@users.noreply.github.com>
…sume Refusing a userspace assignment to the reserved plane (ed1887f) also refused the kernel's own framebuffer when resume committed it back. drm_atomic_helper_suspend() saves the lit square in the duplicated state, and drm_atomic_helper_resume() resets the plane and commits that state, so atomic_check saw an own fb on the reserved plane that was neither unchanged nor the indicator's commit, returned -EINVAL, and the panel did not come back. Userspace cannot name the framebuffer (it is unregistered, kernel-private), so a new state whose fb is that framebuffer is the kernel's. apple_plane now holds a pointer to it, set at attach and cleared at release, compared by pointer only. dcp_ind_plane_policy() takes an own_fb argument and puts such a state back like a clear: the plane resumes disabled, and the indicator's resume path asks for the square again if the camera still streams. Proof: indicator_plane_reservation pins own_fb to RESTORE, and fails on a version that ignores own_fb. The suite passes 8/8 in userspace, and the DRM objects build with no warnings. The own_fb pointer comparison in plane.c and a real suspend and resume were not run. Signed-off-by: Joshua Warren <816217+joshuaswarren@users.noreply.github.com>
A square lit when the system suspended stayed dark after resume. Resume commits the saved state with the kernel framebuffer, which the reserved-plane policy puts back disabled, while the state machine still said LIT, so nothing asked for the square again. A request made before the display was ready (the ISP resuming first) got -ENODEV, which ended the request and dropped it. - Resume: apple_drm_resume_outputs() calls dcp_indicator_display_resumed() for every CRTC after drm_mode_config_helper_resume(). The plane came back disabled, so the state goes OFF and a request that still stands queues the resume work. - Not ready: -ENODEV from the commit (CRTC inactive, DRM unbound) keeps the request with the square OFF and returns 0. The display resuming asks again. No watch is armed for a commit that did not happen. - Own framebuffer: it is now identified by its funcs, which are the indicator's alone, instead of a pointer kept on the plane. That removes the attach and release bookkeeping and the stale pointer after a rebind. A state carrying it is restored, not refused, as before. - dcp_ind_plane_policy() takes has_fb, has_crtc and own_fb, so the clear test is part of what the suite pins. - tests/kunit-shim.h is added so dcp-indicator-test.c builds as a plain program. Proof: indicator_survives_a_display_that_is_not_ready, indicator_lit_across_suspend_asks_again and the widened indicator_plane_reservation were written first and failed to build. Mutants that drop the -ENODEV keep, the resume reset, or the own-fb/clear rule each fail one case. The suite passes 10/10 in userspace, and the DRM and ISP objects build with no warnings. Not run: the glue in plane.c and apple_drv.c (the own fb argument and the resume call), a real suspend and resume, a panel. Signed-off-by: Joshua Warren <816217+joshuaswarren@users.noreply.github.com>
… the plane check Review of e8e20e8 found three defects and an untested wiring. apple_drm_resume_outputs() read to_apple_crtc(crtc)->dcp for every CRTC, but a T8132 external CRTC is not a struct apple_crtc. Skip it, as the other CRTC loops do. dcp_indicator_display_resumed() reset the state to OFF under ci->lock alone. A set(true) that landed between the resume commit and the call had its state forced to OFF; the re-request then found the plane unchanged, no swap followed, and the 1 s watch withdrew the square while the camera streams. The same happened to a plane that resume had restored lit. The hook now takes ci->ctl and the plane lock, in the order dcp_indicator_commit() does, and resets the state only if the plane came back dark (dcp_ind_display_resumed(ind, plane_dark)). plane.c gathered the policy arguments inline, out of reach of any test. dcp_ind_plane_check() now computes unchanged, going_off and own_fb from the old and new plane state, and dcp_indicator_owns_fb() is gone in favour of the exported dcp_indicator_fb_funcs it compared against. A kept request (-ENODEV) logs at dev_dbg. A DCP on the H17G profile has no swap_done hook to confirm the square, so the probe leaves the indicator off there with a warning instead of arming it. Proof: the userspace test (11 cases) passes; new cases cover a lit plane surviving the resume hook and every field of the plane check, and each mutant of the helpers (own_fb, going_off, each compared field, the dark test, the kernel_commit argument) fails one case. The DRM and ISP objects build with no warnings. The apple_drv.c call, the extern, and the plane.c call site are still not run on hardware. Signed-off-by: Joshua Warren <816217+joshuaswarren@users.noreply.github.com>
On the H17P firmware path a swap with no surface other than the indicator
is dropped: iomfb_apply_backlight_h17p() -> dcp_bl_prepare() returns
-ENODATA for !have_surface, and only firmware 27 takes surface-less swaps.
The off commit holds only the indicator plane, so on a J700 it was dropped
("present dropped: backlight prepare -61", "swap failed!", once per stop,
27 times on one boot). The DRM state said the plane was off, but the
firmware never saw the removal: slot 2 stayed on the panel and the DCP
blend CRC stayed on the "with indicator" value after the stream stopped.
Add the CRTC's other planes to the commit with drm_atomic_add_affected_planes(),
so each indicator swap re-presents the primary. The on commit worked only
because it carries the indicator surface itself; it takes the same path now.
Found by RyanTheTide's J700 run on 648dd4c (Aurora seq 807); the fix is
his proposal. No userspace test can cover it: the rule lives in a DRM
atomic call and in the firmware's reaction to it. The repro is his CRC row
and the paired dmesg lines, to be rerun at the new head. Not run by me: any
hardware, and T6050 may or may not need it. The DRM objects build.
Signed-off-by: Joshua Warren <816217+joshuaswarren@users.noreply.github.com>
joshuaswarren
force-pushed
the
w7y/k114c-isp
branch
from
November 13, 2026 06:21
c671c05 to
d4ae13c
Compare
Contributor
Author
|
Rebased onto #1744 head |
…looks like The indicator square was orange. RyanTheTide asked for green; the fill is 0xff34c759 now. Two comments said a plane can come back from a system resume lit. The resume commit names the kernel's framebuffer and gets DCP_IND_PLANE_RESTORE, which puts the plane back to the disabled state suspend left, so the dark branch is the normal path. The lit branch covers only a set(true) that lands between the resume commit and dcp_indicator_display_resumed(). The code was right; the comments are rewritten. Proof: the userspace suite still passes 11/11. No behaviour changed except the colour value, which no test reads. Signed-off-by: Joshua Warren <816217+joshuaswarren@users.noreply.github.com>
Contributor
Author
|
[Joshua's agent] GREEN The only commit beyond #1744 (f928c0f) adds the K114c camera node:
Low, non-blocking:
Merge order: after #1744. Not run: |
The K114c had no camera node, so apple-isp never bound. Enable isp0, its three DARTs and the camera power domains, as t6050-j716c-isp.dtsi does for the J716c, with one difference: the firmware segments are placed statically from the K114c's own segment-ranges, which iBoot fills, instead of by the loader. The node has no isp alias, so the loader leaves it alone. The addresses move with iBoot and must be re-read after an update. The K114c has no camera-light GPIO, so no light property is given. The node asks the display coprocessor for the best-effort software indicator (apple,camera-indicator, binding in the PR this one is stacked on); the camera streams whether or not the dot shows. No dot shows on the K114c until &dcp is enabled and given apple,camera-indicator-surface; its DCP node is disabled and has neither. The power domains follow the binding order, with the sensor domain fourth, as in the J716c. Only preset 0 (1920x1080 from the 3040x2880 IMX958 input) is described: it is the one that was streamed. Tested: a kernel with this node, built on the same description, streamed /dev/video0 as NV12 1920x1080, 2 cycles of 10 frames on the first attempt after boot, with the sensor domain last. t6050-k114c.dtb builds, and its isp0 and reserved-memory nodes match the booted dtb except for presets 1 and 2, the domain order and the indicator property. dt-validate reports no isp error. This tree itself was not booted, so the domain order is untested on the K114c. Signed-off-by: Joshua Warren <816217+joshuaswarren@users.noreply.github.com>
joshuaswarren
force-pushed
the
w7y/k114c-isp
branch
from
November 13, 2026 06:37
d4ae13c to
1aee062
Compare
Contributor
Author
|
[Joshua's agent] New head |
Contributor
Author
|
[Joshua's agent] GREEN Rebased onto #1744 at 0f6091e with the top commit's message reworded. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
[Joshua's agent] Describes the camera on the K114c (14-inch M5 Max) so apple-isp binds. Draft, and stacked on #1744: the branch carries its commits until that merges, so review only the top commit (
git diff faddbaea..HEAD). This exact tree was not booted; the same node was.What it adds:
t6050-k114c-isp.dtsi, included fromt6050-k114c.dtsafter the GPU description (#1603). It enables isp0, its three DARTs and the camera power domains ast6050-j716c-isp.dtsidoes, with one difference: the firmware text, data and heap are reserved statically from the K114c's own segment-ranges (iBoot fills them) instead of by the loader. The node has noispalias, so the loader leaves it alone. The addresses move with iBoot and must be re-read after an update; the file says so. The K114c has no camera-light GPIO, so no light property is given. The node setsapple,camera-indicator = <&dcp>, so #1744 has a board user; the camera streams whether or not the dot shows. The K114c DCP node is not enabled in this tree, so the stream logs that the DCP is not probed and carries on. The DCP side also needsapple,camera-indicator-surfaceonce the display is enabled on this board. The power domains follow the binding order (sensor fourth), as in the J716c; the booted node had the sensor domain last, so this order is untested on the K114c. Only preset 0 (1920x1080 from the 3040x2880 IMX958 input) is described, because it is the one that was streamed.Evidence (a K114c, kernel built with this node description): first start after boot, no module parameter:
/dev/video0NV12 1920x1080, 2 cycles of 10 frames, 31104000 B per cycle, no timeout, no OPCODE error, smooth dark image (mean luma 25-35 of 255).t6050-k114c.dtbbuilds from this branch; its isp0 and reserved-memory nodes match the booted dtb except for presets 1 and 2, the domain order and the indicator property.dt-validatereports no isp error on the dtb.Not covered: the other presets, other iBoot versions, suspend/resume, the dot on a panel (see #1744).
The 16-inch J716c file is left as it is. A shared include would change a board that has its own loader-fed values; say so if you want that factored.
@RyanTheTide