From: "Kandpal, Suraj" <suraj.kandpal@intel.com>
To: "Samala, Pranay" <pranay.samala@intel.com>,
"intel-xe@lists.freedesktop.org" <intel-xe@lists.freedesktop.org>,
"intel-gfx@lists.freedesktop.org"
<intel-gfx@lists.freedesktop.org>
Cc: "Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com>,
"Sharma, Swati2" <swati2.sharma@intel.com>
Subject: RE: [PATCH] drm/i915/display: Avoid stale PIPE_SCANLINE values after crtc_enable
Date: Mon, 4 May 2026 05:03:41 +0000 [thread overview]
Message-ID: <DM3PPF208195D8DEC99CEFB42B9B72839F5E3312@DM3PPF208195D8D.namprd11.prod.outlook.com> (raw)
In-Reply-To: <PH7PR11MB605327187BA6A7B90F219221E7312@PH7PR11MB6053.namprd11.prod.outlook.com>
> > > >
> > > > When a CRTC is moved to a different transcoder (e.g. on DP-MST
> > > > stream allocation), PIPE_SCANLINE and PIPE_FRMCNT can return
> > > > values latched from the previous transcoder/mode for up to one
> > > > vblank period after the new pipe is enabled. The vblank evasion
> > > > code in
> > > > intel_pipe_update_start()/end() then samples a stale or boundary
> > > > scanline and the frame counter ticks during the critical section,
> > > > producing diagnostic errors of the form:
> > >
> > > The stale PIPE_SCANLINE issue happens only when the transcoder
> > > changes, but the vblank wait is added for every crtc_enable. Since
> > > intel_enable_crtc() already has access to the old and new states,
> > > can we add the wait only when the transcoder changes? This would
> > > avoid an
> > extra frame delay in normal cases.
> > >
> > > Regards,
> > > Pranay
> > >
> >
> > Hi Pranay,
> > Thanks for the review
> > So here is my why this is put here:
> > 1) intel_crtc_enable() to begin with run only when
> > intel_crtc_needs_modeset() is true not every commit, Just on full
> > modesets so the latency here is negilble.
> > 2) I don't think the trigger is strictly "transcoder changed." The
> > stale read comes from PIPE_SCANLINE / PIPE_FRMCNT not having seen a
> > live frame yet on the freshly-enabled pipe. That happens any time the
> > pipe transitions off to on, regardless of whether the transcoder
> > mapping changed. skipping wait when transcoder is the same would
> > still leave a window where the next atomic commit can race the first vblank.
> > 3) Also, I don't think "transcoder changed" is the right check.
> > Comparing old_crtc_state->cpu_transcoder to the new one only catches
> > the rebind case - it misses the more common path where the CRTC was
> > fully off and we're just turning it on (which is actually how I hit
> > this most often with kms_rotation_crc on MST). To cover that we'd
> > really be gating on !old_crtc_state->hw.active, just makes it a full
> > modeset check - and that's already exactly when intel_enable_crtc()
> > runs. So the conditional ends up not buying us much.
> >
> > Regards,
> > Suraj Kandpal
>
> Hi Suraj,
> Thanks, got it now.
>
> I was associating the issue mainly with transcoder changes, but now I see that it
> is really about the pipe being freshly enabled.
> Changes LGTM.
>
> Reviewed-by: Pranay Samala <pranay.samala@intel.com>
>
Thanks for the review pushed to din
Regards,
Suraj Kandpal
> >
> > > >
> > > > [243.348405] xe 0000:00:02.0: [drm] *ERROR* Atomic update
> > > > failure on pipe B (start=300 end=301) time 61 us, min 2128, max
> > > > 2161, scanline start 1200, end 2165
> > > > [248.536260] xe 0000:00:02.0: [drm] *ERROR* Atomic update
> > > > failure on pipe B (start=561 end=562) time 61 us, min 2128, max
> > > > 2161, scanline start 2162, end 2167
> > > >
> > > > Here "scanline start 1200" is the vblank_start of a previously
> > > > programmed mode on a different transcoder, while "2162" is the
> > > > current mode's vblank_start sampled before any real frame has been
> > emitted.
> > > > Both indicate a stale read rather than a real evasion miss.
> > > >
> > > > Wait for one vblank after crtc_enable() to give the new transcoder
> > > > a chance to start producing live PIPE_SCANLINE/FRMCNT values
> > > > before any subsequent atomic commit enters the vblank evasion
> > > > section. This adds at most one frame of latency on modeset, which is
> invisible to users.
> > > >
> > > > Reproduced with igt@kms_rotation_crc@sprite-rotation-180 on a DP-
> > MST
> > > > sink; with this patch the failures no longer occur.
> > > >
> > > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> > > > ---
> > > > drivers/gpu/drm/i915/display/intel_display.c | 2 ++
> > > > 1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > > index 674a4ece6d0f..8ebd0df25c11 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > > @@ -6741,6 +6741,8 @@ static void intel_enable_crtc(struct
> > > > intel_atomic_state *state,
> > > >
> > > > display->funcs.display->crtc_enable(state, crtc);
> > > >
> > > > + intel_crtc_wait_for_next_vblank(crtc);
> > > > +
> > > > /* vblanks work again, re-enable pipe CRC. */
> > > > intel_crtc_enable_pipe_crc(crtc); }
> > > > --
> > > > 2.34.1
prev parent reply other threads:[~2026-05-04 5:03 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-29 4:26 [PATCH] drm/i915/display: Avoid stale PIPE_SCANLINE values after crtc_enable Suraj Kandpal
2026-04-29 4:33 ` ✓ CI.KUnit: success for " Patchwork
2026-04-29 5:21 ` ✓ Xe.CI.BAT: " Patchwork
2026-04-29 14:43 ` ✗ Xe.CI.FULL: failure " Patchwork
2026-04-29 17:12 ` [PATCH] " Samala, Pranay
2026-04-30 2:48 ` Kandpal, Suraj
2026-05-04 3:15 ` Samala, Pranay
2026-05-04 5:03 ` Kandpal, Suraj [this message]
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=DM3PPF208195D8DEC99CEFB42B9B72839F5E3312@DM3PPF208195D8D.namprd11.prod.outlook.com \
--to=suraj.kandpal@intel.com \
--cc=ankit.k.nautiyal@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=intel-xe@lists.freedesktop.org \
--cc=pranay.samala@intel.com \
--cc=swati2.sharma@intel.com \
/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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).