Repository navigation
arm64: dts: apple: t8140-j700: Show the software camera indicator - #1758
Draft
RyanTheTide wants to merge 10 commits into
Draft
RyanTheTide wants to merge 10 commits into
RyanTheTide wants to merge 10 commits into
Conversation
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>
The J700 camera has no indicator light on its sensor supply. Reserve DCP swap slot 2, above the one surface DRM uses on this panel, for the kernel's software camera indicator and point the ISP at the DCP that shows it. The camera streams whether or not the indicator is shown. Assisted-by: LLM Signed-off-by: Ryan Murray <ryan@aurorasilicon.org>
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.
Stacked on #1744. Only the last commit belongs to this PR (arm64: dts: apple: t8140-j700: Show the software camera indicator); the commits below it are #1744.
J700 has no indicator light on its camera sensor supply, so it uses the software camera indicator from #1744.
apple,camera-indicator-surface = <2>on the DCP. DRM's primary plane uses swap slot 0 on this panel. Slot 2 is free and inside the 0x7 mask that the clear and power-off swaps reset.apple,camera-indicator = <&dcp>on the ISP.The camera streams whether or not the indicator is shown.
J700 row (Neo, this commit on #1744 cherry-picked onto integration 7945f4b, one-shot kernel, streams to /dev/null): the DCP reserves slot 2 and the ISP takes the phandle; 20/20 stream start/stop cycles with the indicator plane set during each stream; panel off, the stream still runs; s2idle 2/2 and s2r 2/2 with a stream running; 0 AVC. The DCP blend CRC changes when the indicator goes on. The indicator off swap is dropped on H17P (no surface in the swap), reported on #1744; this stays a draft until that is fixed there.
🤖 Generated with Claude Code