mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Michael Burch <me@michaelburch.net>
To: Rob Clark <robin.clark@oss.qualcomm.com>,
	Dmitry Baryshkov <lumag@kernel.org>
Cc: Abhinav Kumar <abhinav.kumar@linux.dev>,
	Jessica Zhang <jesszhan0024@gmail.com>,
	Sean Paul <sean@poorly.run>,
	Marijn Suijten <marijn.suijten@somainline.org>,
	David Airlie <airlied@gmail.com>, Simona Vetter <simona@ffwll.ch>,
	Jun Nie <jun.nie@linaro.org>,
	linux-arm-msm@vger.kernel.org, dri-devel@lists.freedesktop.org,
	freedreno@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	stable@vger.kernel.org
Subject: [PATCH v2] drm/msm/dpu: count the pending frame before kicking it off
Date: Tue, 13 Oct 2026 12:27:00 -0500	[thread overview]
Message-ID: <20261013172703.12273-1-me@michaelburch.net> (raw)

dpu_crtc_commit_kickoff() increments frame_pending only after
dpu_encoder_kickoff() has sent the frame to the hardware. If the frame
completes before the increment, dpu_crtc_frame_event_work() sees
frame_pending == 0 and ignores the frame-done event. The increment that
follows is never matched, so the count stays one too high until
dpu_crtc_disable() resets it.

With frame_pending stuck at 1 or more:

- dpu_encoder_resource_control(FRAME_DONE) sees a pending count above 1
  on every frame and never queues delayed_off_work. A video-mode encoder
  therefore never reaches ENTER_IDLE and keeps its vsync interrupt
  registered for as long as the display is on.
- dpu_crtc_frame_event_work() never sees the count drop to zero, so
  dpu_core_perf_crtc_release_bw() is never called.

On an X1E80100 laptop with a 120 Hz eDP panel this left the vsync
interrupt firing 120 times a second on an idle desktop: 14,407 vblank
callbacks against 21 commits in 120 s, with no vblank events requested
by userspace. Every frame-done took the frame_event_more_pending path
(591 of 591 in one trace) and the encoder never entered idle. After a
suspend cycle reset the count, the same desktop showed 3 commits, 3
frame_event_done, 3 encoder idle transitions and 25 MDSS interrupts in
60 s.

Until commit ad06972d5365 ("drm/msm/dpu: Reorder encoder kickoff for
CWB") the increment came before the kickoff. Move it back: count the
frame in each kickoff path just before its encoders are kicked off. A
clone-mode commit that fails its encoder lookup is then never counted.

Fixes: ad06972d5365 ("drm/msm/dpu: Reorder encoder kickoff for CWB")
Cc: stable@vger.kernel.org
Assisted-by: Copilot:claude-opus-5.5
Signed-off-by: Michael Burch <me@michaelburch.net>
---
Changes in v2:
- Count the frame inside each kickoff path, after the clone-mode encoder
  lookup, so the clone-mode error path has nothing to undo. v1 counted
  first and undid it with atomic_dec() on that path; if a frame-done
  dropped the count to 1 in between, that atomic_dec() took it to 0
  without calling dpu_core_perf_crtc_release_bw() (Sashiko review).
- Link to v1: https://lore.kernel.org/r/20261013145007.13111-1-me@michaelburch.net

Tested on a Dell XPS 13 9345 (X1E80100, 120 Hz eDP, video mode). v1 on
v7.3-rc6 based kernels: the count no longer leaks in normal use. A 120 s
trace 50 minutes after boot showed 22 commits, 22 frame_event_done and
22 encoder idle transitions. About 2 hours after boot there were 27,479
vblanks against 16,693 commits over 6,040 s, where a leaked count gives
about 725,000. v2 on a v7.3-rc7 based kernel: a 60 s trace under load
showed 5,648 commits, 5,649 frame_event_done and no
frame_event_more_pending. Not tested: clone mode or writeback, which
this machine does not have.

Jun Nie's "drm/msm/dpu: delete timeout timer when encoder is not busy"
[1] hits the same window: the frame-done interrupt arriving before
dpu_crtc_commit_kickoff() gets past dpu_encoder_kickoff().

[1] https://lore.kernel.org/r/20260903143004.1026181-1-jun.nie@linaro.org

 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 42d0a529b4d5..81282a8a5d9d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1037,6 +1037,19 @@ static int _dpu_crtc_wait_for_frame_done(struct drm_crtc *crtc)
 	return rc;
 }
 
+/* Must precede the kickoff: the frame-done can be handled before it returns. */
+static void dpu_crtc_count_pending_frame(struct drm_crtc *crtc)
+{
+	struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
+
+	if (atomic_inc_return(&dpu_crtc->frame_pending) == 1) {
+		/* acquire bandwidth and other resources */
+		DRM_DEBUG_ATOMIC("crtc%d first commit\n", crtc->base.id);
+	} else {
+		DRM_DEBUG_ATOMIC("crtc%d commit\n", crtc->base.id);
+	}
+}
+
 static int dpu_crtc_kickoff_clone_mode(struct drm_crtc *crtc)
 {
 	struct drm_encoder *encoder;
@@ -1062,6 +1075,8 @@ static int dpu_crtc_kickoff_clone_mode(struct drm_crtc *crtc)
 
 	dpu_vbif_clear_errors(dpu_kms);
 
+	dpu_crtc_count_pending_frame(crtc);
+
 	/*
 	 * Kickoff real time encoder last as it's the encoder that
 	 * will do the flush
@@ -1119,6 +1134,8 @@ void dpu_crtc_commit_kickoff(struct drm_crtc *crtc)
 
 		dpu_vbif_clear_errors(dpu_kms);
 
+		dpu_crtc_count_pending_frame(crtc);
+
 		drm_for_each_encoder_mask(encoder, crtc->dev,
 				crtc->state->encoder_mask) {
 			dpu_encoder_kickoff(encoder);
@@ -1126,12 +1143,6 @@ void dpu_crtc_commit_kickoff(struct drm_crtc *crtc)
 		}
 	}
 
-	if (atomic_inc_return(&dpu_crtc->frame_pending) == 1) {
-		/* acquire bandwidth and other resources */
-		DRM_DEBUG_ATOMIC("crtc%d first commit\n", crtc->base.id);
-	} else
-		DRM_DEBUG_ATOMIC("crtc%d commit\n", crtc->base.id);
-
 	dpu_crtc->play_count++;
 
 	reinit_completion(&dpu_crtc->frame_done_comp);

base-commit: 24a19b953e2a3d0ea5a8c4951a0ab8f32720c907
-- 
2.56.0


                 reply	other threads:[~2026-10-13 17:27 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261013172703.12273-1-me@michaelburch.net \
    --to=me@michaelburch.net \
    --cc=abhinav.kumar@linux.dev \
    --cc=airlied@gmail.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=freedreno@lists.freedesktop.org \
    --cc=jesszhan0024@gmail.com \
    --cc=jun.nie@linaro.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lumag@kernel.org \
    --cc=marijn.suijten@somainline.org \
    --cc=robin.clark@oss.qualcomm.com \
    --cc=sean@poorly.run \
    --cc=simona@ffwll.ch \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®