From 76b09427c2295ca237e5253c6dcd6f54427bef9f Mon Sep 17 00:00:00 2001 From: Rolando Trevino Date: Sun, 8 Nov 2026 06:15:05 -0500 Subject: [PATCH 1/2] drm/apple: Fix a GPU fence that never signals wedging the display drm_atomic_helper_commit() waits for every plane's in-fence with dma_fence_wait() and no timeout: in the commit worker before the driver's commit tail, and, for blocking commits, before the swap. The in-fence is the scanout buffer's implicit write fence (zink imports its GPU job's fence into each exported dma-buf) or the compositor's IN_FENCE_FD. When a GPU fence never signals, after a GPU recovery for example, the worker sleeps for good: commit_planes() never runs, no swap reaches DCP, the commit's flip event never comes, and every later nonblocking commit fails with -EBUSY (stall_checks(), only a debug message). The 10 s flip_done timeout is never reached, so nothing is logged. The panel stays black across compositor restarts, which see "Cannot commit when a page-flip is awaiting", until a reboot. Commit through a copy of drm_atomic_helper_commit() whose fence wait is bounded: all of a commit's planes share one second, and a fence that does not signal in time is dropped with a rate-limited warning naming its driver and timeline. The buffer is scanned out as it is, so a GPU bug costs a stale or torn frame instead of the panel. The copy keeps the helper's sequence (setup, prepare, the interruptible pre-swap wait for blocking commits, swap, the worker's wait for dependencies, the tail, cleanup) and its fence deadline hint, and leaves out only the self-refresh accounting, which this driver does not use. Once the commit goes on, its flip completes as usual. If DCP then does not complete the swap within drm_atomic_helper_wait_for_flip_done()'s 10 s, the commit's event stayed pending and a compositor would still wait for it before committing again. Send that event as failed (-ETIMEDOUT on its out-fence) after the wait; only the event that belongs to this commit is sent. The commit then leaves its CRTC's list at cleanup_done as before, so later nonblocking commits are not held by it either. Signed-off-by: Rolando Trevino Assisted-by: LLM Claude --- drivers/gpu/drm/apple/apple_drv.c | 201 +++++++++++++++++++++++++++++- drivers/gpu/drm/apple/dcp.c | 18 +++ drivers/gpu/drm/apple/dcp.h | 2 + 3 files changed, 220 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/apple/apple_drv.c b/drivers/gpu/drm/apple/apple_drv.c index 763ba82ad29ac..fbd0325b963d2 100644 --- a/drivers/gpu/drm/apple/apple_drv.c +++ b/drivers/gpu/drm/apple/apple_drv.c @@ -9,7 +9,9 @@ #include #include +#include #include +#include #include #include #include @@ -18,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -562,12 +565,206 @@ static void apple_atomic_state_free(struct drm_atomic_commit *state) kfree(to_apple_atomic_state(state)); } +/* + * A plane's in-fence (its buffer's implicit write fence, or the compositor's + * IN_FENCE_FD) is waited for at most this long per commit. The helpers wait + * for it without a timeout: a GPU fence that never signals, after a GPU + * recovery for example, leaves the commit worker asleep for good. No later + * frame reaches the display, the commit's flip event never comes, and every + * later nonblocking commit fails with -EBUSY. One second is many frames even + * at the panel's slowest refresh; past it the buffer is scanned out as it is, + * so a GPU bug costs a stale or torn frame instead of the panel. + */ +#define APPLE_PLANE_FENCE_TIMEOUT_MS 1000 + +static void apple_warn_fence_timeout(struct drm_device *dev, + struct drm_plane *plane, + struct dma_fence *fence) +{ + static DEFINE_RATELIMIT_STATE(rs, 60 * HZ, 1); + const char __rcu *driver; + const char __rcu *timeline; + + if (!__ratelimit(&rs)) + return; + rcu_read_lock(); + driver = dma_fence_driver_name(fence); + timeline = dma_fence_timeline_name(fence); + drm_warn(dev, + "[PLANE:%d:%s] in-fence %s:%s seqno %llu not signalled after %u ms, scanning out without it\n", + plane->base.id, plane->name, rcu_dereference(driver), + rcu_dereference(timeline), fence->seqno, + APPLE_PLANE_FENCE_TIMEOUT_MS); + rcu_read_unlock(); +} + +/* As the helpers do: the GPU may finish the frame sooner knowing the vblank. */ +static void apple_set_fence_deadline(struct drm_atomic_commit *state) +{ + struct drm_crtc_state *new_crtc_state; + struct drm_plane_state *new_plane_state; + struct drm_plane *plane; + struct drm_crtc *crtc; + ktime_t vbltime = 0; + int i; + + for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) { + ktime_t v; + + if (drm_atomic_crtc_needs_modeset(new_crtc_state) || + !new_crtc_state->active || + drm_crtc_next_vblank_start(crtc, &v)) + continue; + if (!vbltime || ktime_before(v, vbltime)) + vbltime = v; + } + if (!vbltime) + return; + for_each_new_plane_in_state(state, plane, new_plane_state, i) + if (new_plane_state->fence) + dma_fence_set_deadline(new_plane_state->fence, vbltime); +} + +/* + * drm_atomic_helper_wait_for_fences() with one bound for all of the commit's + * planes (APPLE_PLANE_FENCE_TIMEOUT_MS). A fence that does not signal in time + * is dropped. Before the swap (blocking commits) the wait is interruptible, + * as the helper's is. + */ +static int apple_wait_for_fences(struct drm_device *dev, + struct drm_atomic_commit *state, + bool pre_swap) +{ + unsigned long deadline = + jiffies + msecs_to_jiffies(APPLE_PLANE_FENCE_TIMEOUT_MS); + struct drm_plane_state *new_plane_state; + struct drm_plane *plane; + int i; + + apple_set_fence_deadline(state); + + for_each_new_plane_in_state(state, plane, new_plane_state, i) { + struct dma_fence *fence = new_plane_state->fence; + long left, ret; + + if (!fence) + continue; + + WARN_ON(!new_plane_state->fb); + + left = time_before(jiffies, deadline) ? deadline - jiffies : 0; + ret = dma_fence_wait_timeout(fence, pre_swap, left); + if (ret < 0) + return ret; + if (!ret) + apple_warn_fence_timeout(dev, plane, fence); + + dma_fence_put(fence); + new_plane_state->fence = NULL; + } + + return 0; +} + +/* + * A flip DCP did not complete within the helper's 10 s + * (drm_atomic_helper_wait_for_flip_done() gave up) leaves its event pending. + * A compositor waits for that event before it commits again, so send it now, + * with -ETIMEDOUT on its out-fence, and the next commit can go ahead. A swap + * completion that still comes later finds no event. + */ +static void apple_fail_lost_flips(struct drm_atomic_commit *state) +{ + for (int i = 0; i < state->dev->mode_config.num_crtc; i++) { + struct drm_crtc_commit *commit = state->crtcs[i].commit; + struct drm_crtc *crtc = state->crtcs[i].ptr; + + if (!crtc || !commit || t8132_ext_crtc(crtc) || + completion_done(&commit->flip_done)) + continue; + dcp_drm_crtc_flip_lost(to_apple_crtc(crtc), &commit->flip_done, + -ETIMEDOUT); + } +} + +static void apple_atomic_commit_tail(struct drm_atomic_commit *state); + +/* The helper's commit_tail() with the bounded fence wait. */ +static void apple_commit_tail(struct drm_atomic_commit *state) +{ + apple_wait_for_fences(state->dev, state, false); + drm_atomic_helper_wait_for_dependencies(state); + apple_atomic_commit_tail(state); + drm_atomic_helper_commit_cleanup_done(state); + drm_atomic_commit_put(state); +} + +static void apple_commit_work(struct work_struct *work) +{ + apple_commit_tail(container_of(work, struct drm_atomic_commit, + commit_work)); +} + +/* + * drm_atomic_helper_commit() with apple_wait_for_fences() in place of the + * helper's unbounded wait. The driver keeps no self-refresh state, so the + * helper's self-refresh accounting is left out. + */ +static int apple_atomic_commit(struct drm_device *dev, + struct drm_atomic_commit *state, bool nonblock) +{ + int ret; + + if (state->async_update) { + ret = drm_atomic_helper_prepare_planes(dev, state); + if (ret) + return ret; + + drm_atomic_helper_async_commit(dev, state); + drm_atomic_helper_unprepare_planes(dev, state); + + return 0; + } + + ret = drm_atomic_helper_setup_commit(state, nonblock); + if (ret) + return ret; + + INIT_WORK(&state->commit_work, apple_commit_work); + + ret = drm_atomic_helper_prepare_planes(dev, state); + if (ret) + return ret; + + if (!nonblock) { + ret = apple_wait_for_fences(dev, state, true); + if (ret) + goto err; + } + + ret = drm_atomic_helper_swap_state(state, true); + if (ret) + goto err; + + drm_atomic_commit_get(state); + if (nonblock) + queue_work(system_dfl_wq, &state->commit_work); + else + apple_commit_tail(state); + + return 0; + +err: + drm_atomic_helper_unprepare_planes(dev, state); + return ret; +} + static const struct drm_mode_config_funcs apple_mode_config_funcs = { .atomic_state_alloc = apple_atomic_state_alloc, .atomic_state_clear = apple_atomic_state_clear, .atomic_state_free = apple_atomic_state_free, .atomic_check = drm_atomic_helper_check, - .atomic_commit = drm_atomic_helper_commit, + .atomic_commit = apple_atomic_commit, .fb_create = drm_gem_fb_create, }; @@ -600,9 +797,11 @@ static void apple_atomic_commit_tail(struct drm_atomic_commit *state) drm_atomic_helper_fake_vblank(state); drm_atomic_helper_commit_hw_done(state); drm_atomic_helper_wait_for_flip_done(state->dev, state); + apple_fail_lost_flips(state); drm_atomic_helper_cleanup_planes(state->dev, state); } +/* apple_atomic_commit() calls the tail itself; this serves other helper users. */ static const struct drm_mode_config_helper_funcs apple_mode_config_helpers = { .atomic_commit_tail = apple_atomic_commit_tail, }; diff --git a/drivers/gpu/drm/apple/dcp.c b/drivers/gpu/drm/apple/dcp.c index f56c4423ea247..4669e4fb983c1 100644 --- a/drivers/gpu/drm/apple/dcp.c +++ b/drivers/gpu/drm/apple/dcp.c @@ -531,6 +531,24 @@ void dcp_drm_crtc_error(struct apple_crtc *crtc, spin_unlock_irqrestore(&drm->event_lock, flags); } +/* + * The flip of the commit whose flip_done is @flip_done never completed: send + * its still pending event as failed. Only that commit's event is sent; a newer + * commit's stays for its own swap. + */ +void dcp_drm_crtc_flip_lost(struct apple_crtc *crtc, + struct completion *flip_done, int error) +{ + unsigned long flags; + + spin_lock_irqsave(&crtc->base.dev->event_lock, flags); + if (crtc->event && crtc->event->base.completion == flip_done) { + dcp_failed_page_flip(crtc, crtc->event, error); + crtc->event = NULL; + } + spin_unlock_irqrestore(&crtc->base.dev->event_lock, flags); +} + void dcp_drm_crtc_page_flip(struct apple_dcp *dcp, ktime_t now) { unsigned long flags; diff --git a/drivers/gpu/drm/apple/dcp.h b/drivers/gpu/drm/apple/dcp.h index d65367c8af13f..aaaf2918c4b75 100644 --- a/drivers/gpu/drm/apple/dcp.h +++ b/drivers/gpu/drm/apple/dcp.h @@ -88,6 +88,8 @@ void apple_crtc_vblank(struct apple_crtc *apple); void dcp_drm_crtc_vblank(struct apple_crtc *crtc); void dcp_drm_crtc_error(struct apple_crtc *crtc, struct drm_crtc_state *state, int error); +void dcp_drm_crtc_flip_lost(struct apple_crtc *crtc, + struct completion *flip_done, int error); int dcp_get_modes(struct drm_connector *connector); enum drm_mode_status dcp_mode_valid(struct drm_connector *connector, const struct drm_display_mode *mode); From f2f745f21544211e1598618c6aa2f9599a4f0878 Mon Sep 17 00:00:00 2001 From: Rolando Trevino Date: Sun, 8 Nov 2026 11:20:19 -0500 Subject: [PATCH 2/2] drm/apple: address the review of the bounded fence wait Follow-up to the automated review of this pull request, kept as a separate commit for review and squashed on request. drm/apple: Fix a GPU fence that never signals wedging the display drm_atomic_helper_commit() waits for every plane's in-fence with dma_fence_wait() and no timeout: in the commit worker before the driver's commit tail, and, for blocking commits, before the swap. The in-fence is the scanout buffer's implicit write fence (zink imports its GPU job's fence into each exported dma-buf) or the compositor's IN_FENCE_FD. When a GPU fence never signals, after a GPU recovery for example, the worker sleeps for good: commit_planes() never runs, no swap reaches DCP, the commit's flip event never comes, and every later nonblocking commit fails with -EBUSY (stall_checks(), only a debug message). The 10 s flip_done timeout is never reached, so nothing is logged. The panel stays black across compositor restarts, which see "Cannot commit when a page-flip is awaiting", until a reboot. Commit through a copy of drm_atomic_helper_commit() whose fence wait is bounded: all of a commit's planes share one second, and a fence that does not signal in time is dropped with a rate-limited warning naming its driver and timeline. The buffer is scanned out as it is, so a GPU bug costs a stale or torn frame instead of the panel. The copy keeps the helper's sequence (setup, prepare, the interruptible pre-swap wait for blocking commits, swap, the worker's wait for dependencies, the tail, cleanup) and its fence deadline hint, and leaves out only the self-refresh accounting, which this driver does not use. Once the commit goes on, its flip completes as usual. If DCP then does not complete the swap within drm_atomic_helper_wait_for_flip_done()'s 10 s, the commit's event stayed pending and a compositor would still wait for it before committing again. Send that event as failed (-ETIMEDOUT on its out-fence) after the wait; only the event that belongs to this commit is sent. The commit then leaves its CRTC's list at cleanup_done as before, so later nonblocking commits are not held by it either. The swap of that flip may still complete after the next commit installed its event. The H17P and desktop 26.x paths match a completion to its present; on the older firmware (12.3, 13.5, the 14.7 template, H17G) a completion retires whatever flip is pending, so there the lost swap's completion is now ignored. The framebuffers the lost swap displaced are unarmed, as the swap watchdog does for a dropped swap, so that the next swap re-arms them and its completion releases them; they were otherwise kept until the display powered off. Compile-checked with W=1 CLIPPY=1 over drivers/gpu/drm/{asahi,apple}: no new warnings against the base; checkpatch --strict clean; rustfmt unchanged. Signed-off-by: Rolando Trevino Assisted-by: LLM Claude --- drivers/gpu/drm/apple/apple_drv.c | 2 +- drivers/gpu/drm/apple/dcp-internal.h | 10 +++++++ drivers/gpu/drm/apple/dcp.c | 39 ++++++++++++++++++++++++++ drivers/gpu/drm/apple/iomfb_template.c | 8 ++++++ 4 files changed, 58 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/apple/apple_drv.c b/drivers/gpu/drm/apple/apple_drv.c index fbd0325b963d2..1981a33aff91f 100644 --- a/drivers/gpu/drm/apple/apple_drv.c +++ b/drivers/gpu/drm/apple/apple_drv.c @@ -652,7 +652,7 @@ static int apple_wait_for_fences(struct drm_device *dev, WARN_ON(!new_plane_state->fb); - left = time_before(jiffies, deadline) ? deadline - jiffies : 0; + left = max_t(long, (long)(deadline - jiffies), 0); ret = dma_fence_wait_timeout(fence, pre_swap, left); if (ret < 0) return ret; diff --git a/drivers/gpu/drm/apple/dcp-internal.h b/drivers/gpu/drm/apple/dcp-internal.h index 5b9f50b60d300..26ff331cc27d0 100644 --- a/drivers/gpu/drm/apple/dcp-internal.h +++ b/drivers/gpu/drm/apple/dcp-internal.h @@ -73,6 +73,7 @@ struct apple_dcp_typec_port; void dcp_swap_watchdog_arm(struct apple_dcp *dcp); void dcp_swap_watchdog_complete(struct apple_dcp *dcp); +bool dcp_swap_lost(struct apple_dcp *dcp, u32 swap_id); void dcp_retry_placeholder_edid(struct apple_dcp *dcp, const struct drm_edid *drm_edid); bool dcp_typec_relink(struct apple_dcp *dcp); @@ -573,6 +574,15 @@ struct apple_dcp { u32 last_swap_id; /* last_swap_id is only meaningful after the first swap completes */ bool have_swap_complete; + /* swap id of the last started swap */ + u32 started_swap_id; + /* + * A started swap whose flip a commit gave up on + * (dcp_drm_crtc_flip_lost()); event_lock. Firmware that does not track + * its presents ignores its completion, see dcp_swap_lost(). + */ + u32 lost_swap_id; + bool swap_lost; ktime_t swap_start; u64 swap_submit_timestamp; diff --git a/drivers/gpu/drm/apple/dcp.c b/drivers/gpu/drm/apple/dcp.c index 4669e4fb983c1..efbe331d2467a 100644 --- a/drivers/gpu/drm/apple/dcp.c +++ b/drivers/gpu/drm/apple/dcp.c @@ -535,18 +535,57 @@ void dcp_drm_crtc_error(struct apple_crtc *crtc, * The flip of the commit whose flip_done is @flip_done never completed: send * its still pending event as failed. Only that commit's event is sent; a newer * commit's stays for its own swap. + * + * The swap started last is that flip's. Its displaced framebuffers are + * unarmed, as the swap watchdog does, so that the next swap arms them and its + * completion releases them. Its completion may still come, after the next + * commit installed its event: firmware that does not track its presents + * ignores it (dcp_swap_lost()), so that it does not retire the newer flip. */ void dcp_drm_crtc_flip_lost(struct apple_crtc *crtc, struct completion *flip_done, int error) { + struct apple_dcp *dcp = platform_get_drvdata(crtc->dcp); unsigned long flags; + bool lost = false; + u32 swap_id = 0; spin_lock_irqsave(&crtc->base.dev->event_lock, flags); if (crtc->event && crtc->event->base.completion == flip_done) { dcp_failed_page_flip(crtc, crtc->event, error); crtc->event = NULL; + swap_id = READ_ONCE(dcp->started_swap_id); + dcp->lost_swap_id = swap_id; + dcp->swap_lost = true; + lost = true; } spin_unlock_irqrestore(&crtc->base.dev->event_lock, flags); + + if (lost) + dcp_unarm_retained_framebuffers(dcp, swap_id); +} + +/* + * Is @swap_id the swap whose flip dcp_drm_crtc_flip_lost() gave up on? On + * firmware that does not track its presents, its completion would retire + * whichever flip is pending; it is ignored instead. Swaps complete in order, + * so once another swap completed, the lost one will not. + */ +bool dcp_swap_lost(struct apple_dcp *dcp, u32 swap_id) +{ + unsigned long flags; + bool lost; + + spin_lock_irqsave(&dcp->crtc->base.dev->event_lock, flags); + lost = dcp->swap_lost && dcp->lost_swap_id == swap_id; + dcp->swap_lost = false; + spin_unlock_irqrestore(&dcp->crtc->base.dev->event_lock, flags); + + if (lost) + dev_warn_ratelimited(dcp->dev, + "completion for lost present %u ignored\n", + swap_id); + return lost; } void dcp_drm_crtc_page_flip(struct apple_dcp *dcp, ktime_t now) diff --git a/drivers/gpu/drm/apple/iomfb_template.c b/drivers/gpu/drm/apple/iomfb_template.c index 82e5310f3fd6e..19615309aa2b9 100644 --- a/drivers/gpu/drm/apple/iomfb_template.c +++ b/drivers/gpu/drm/apple/iomfb_template.c @@ -649,6 +649,12 @@ static void dcpep_cb_swap_complete(struct apple_dcp *dcp, if (!dcp_desktop_present_complete(dcp, resp->swap_id)) return; dcp_backlight_desktop_finish(dcp, resp->swap_id, false, true); +#else + /* Nor can a lost flip's on firmware that does not track its presents. */ + if ((!DCP_FW_H17(DCP_FW_VER) || + dcp->hw.iomfb_method_profile == DCP_IOMFB_METHODS_H17G) && + dcp_swap_lost(dcp, resp->swap_id)) + return; #endif trace_iomfb_swap_complete(dcp, resp->swap_id); dcp->last_swap_id = resp->swap_id; @@ -2088,6 +2094,7 @@ static void dcp_swap_clear_started(struct apple_dcp *dcp, void *data, dcp_prepare_clear_swap(dcp, &DCP_FW_SWAP(dcp)); #endif DCP_FW_SWAP(dcp).swap.swap_id = resp->swap_id; + WRITE_ONCE(dcp->started_swap_id, resp->swap_id); if (dcp_present_retires(dcp)) dcp_arm_retained_framebuffers(dcp, resp->swap_id); @@ -2943,6 +2950,7 @@ static void dcp_swap_started(struct apple_dcp *dcp, void *data, void *cookie) return; } DCP_FW_SWAP(dcp).swap.swap_id = resp->swap_id; + WRITE_ONCE(dcp->started_swap_id, resp->swap_id); if (dcp_present_retires(dcp)) dcp_arm_retained_framebuffers(dcp, resp->swap_id);