There is a spurious timeout error message under the following reproduction
steps:
1. Run "modetest -M msm -r" and press CTRL+Z to pause it.
2. Run "while true; do rtcwake -m mem -s 3 -v; sleep 2; done"

Resulting error message:
[  124.018206] [drm:dpu_encoder_virt_atomic_disable:1425] [dpu error]enc35 timeout pending

dpu_crtc_commit_kickoff() calls dpu_encoder_kickoff() to set frame busy
bits, and then arms the frame done timer. If the frame done IRQ fires
between these two steps, the IRQ handler clears the busy bits before the
timer is armed. As a result, the timer runs with busy bits already cleared
and is never cancelled by subsequent IRQ handlers, leading to a false
timeout warning.

Delete the pending timer when all busy bits are cleared in the frame done
callback to prevent false timeouts.

Fixes: 95bbde1d0d07 ("drm/msm/dpu: Start frame done timer after encoder kickoff")
Signed-off-by: Jun Nie <jun.nie@linaro.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 1f20695f81e3..d83d38797331 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1573,6 +1573,21 @@ void dpu_encoder_frame_done_callback(
 			| DPU_ENCODER_FRAME_EVENT_PANEL_DEAD)) {
 
 		if (!dpu_enc->frame_busy_mask[0]) {
+			/*
+			 * dpu_crtc_commit_kickoff calls dpu_encoder_kickoff to
+			 * mark busy bits, starts the framedone timer next.
+			 * It is possibile that irq happens between the 2
+			 * operations. Thus timer is running with busy bits
+			 * cleared by irq handler and timer will not be deleted
+			 * anymore. Then false timeout introduces unnecessary
+			 * confusion and visual defect.
+			 * delete the timer here to fix it.
+			 */
+			if (atomic_read(&dpu_enc->frame_done_timeout_ms)) {
+				atomic_set(&dpu_enc->frame_done_timeout_ms, 0);
+				timer_delete(&dpu_enc->frame_done_timer);
+			}
+
 			/**
 			 * suppress frame_done without waiter,
 			 * likely autorefresh
-- 
2.53.0

Jun Nie <jun.nie@linaro.org> 于2026年9月3日周四 22:30写道：
>
> There is a spurious timeout error message under the following reproduction
> steps:
> 1. Run "modetest -M msm -r" and press CTRL+Z to pause it.
> 2. Run "while true; do rtcwake -m mem -s 3 -v; sleep 2; done"
>
> Resulting error message:
> [  124.018206] [drm:dpu_encoder_virt_atomic_disable:1425] [dpu error]enc35 timeout pending
>
> dpu_crtc_commit_kickoff() calls dpu_encoder_kickoff() to set frame busy
> bits, and then arms the frame done timer. If the frame done IRQ fires
> between these two steps, the IRQ handler clears the busy bits before the
> timer is armed. As a result, the timer runs with busy bits already cleared
> and is never cancelled by subsequent IRQ handlers, leading to a false
> timeout warning.
>
> Delete the pending timer when all busy bits are cleared in the frame done
> callback to prevent false timeouts.
>

Another option is to wrap the enc_spinlock lock/unlock pair in dpu_encoder.c
and expose it to dpu_crtc.c to make the sequence of dpu_encoder_kickoff()
and dpu_encoder_start_frame_done_timer() atomic against the IRQ handler.
However, this breaks encoder self-containment to some extent. Furthermore,
initial testing revealed a deadlock involving additional locks beyond
dpu_enc->enc_spinlock.

- Jun

On Thu, Sep 03, 2026 at 10:39:22PM +0800, Jun Nie wrote:
> Jun Nie <jun.nie@linaro.org> 于2026年9月3日周四 22:30写道：
> >
> > There is a spurious timeout error message under the following reproduction
> > steps:
> > 1. Run "modetest -M msm -r" and press CTRL+Z to pause it.
> > 2. Run "while true; do rtcwake -m mem -s 3 -v; sleep 2; done"
> >
> > Resulting error message:
> > [  124.018206] [drm:dpu_encoder_virt_atomic_disable:1425] [dpu error]enc35 timeout pending
> >
> > dpu_crtc_commit_kickoff() calls dpu_encoder_kickoff() to set frame busy
> > bits, and then arms the frame done timer. If the frame done IRQ fires
> > between these two steps, the IRQ handler clears the busy bits before the
> > timer is armed. As a result, the timer runs with busy bits already cleared
> > and is never cancelled by subsequent IRQ handlers, leading to a false
> > timeout warning.
> >
> > Delete the pending timer when all busy bits are cleared in the frame done
> > callback to prevent false timeouts.
> >
> 
> Another option is to wrap the enc_spinlock lock/unlock pair in dpu_encoder.c
> and expose it to dpu_crtc.c to make the sequence of dpu_encoder_kickoff()
> and dpu_encoder_start_frame_done_timer() atomic against the IRQ handler.
> However, this breaks encoder self-containment to some extent. Furthermore,
> initial testing revealed a deadlock involving additional locks beyond
> dpu_enc->enc_spinlock.

The patch you posted has an issue of the atomic value being updated in a
non-atomic way. However I think it's not a correct solution. I'd just
delete the if from dpu_encoder_virt_atomic_disable() and always delete
the timer. This also would allow us to delete frame_done_timeout_ms,
simplifying the code.

> 
> - Jun

-- 
With best wishes
Dmitry