On Fri, Aug 07, 2026 at 12:15:38PM +0000, Mika Kahola wrote: > Flipq commits execute asynchronously via the pipe DMC, but we don't > notify frontbuffer tracking when they actually complete. A CRC grabbed > right after can therefore observe stale state (kms_cursor_crc's > cursor-alpha-opaque hits this).
There's nothing special about flipq vs. commit completion. The interrupt is supposed to fire on the delayed vblank after the flip has executed. > Stash the fb_bits on the crtc when arming the commit, and flush > frontbuffer/FBC from a worker once intel_dsb_irq_handler() sees the > completion interrupt. Can't call these directly from there, it's > hardirq context and they may sleep. > > Also flush (rather than drop) any pending completion when the crtc > gets disabled out from under it. > > Assisted-by: Copilot:claude-sonnet-5 > Signed-off-by: Mika Kahola <[email protected]> > --- > drivers/gpu/drm/i915/display/intel_crtc.c | 3 + > drivers/gpu/drm/i915/display/intel_display.c | 3 + > .../drm/i915/display/intel_display_types.h | 9 +++ > drivers/gpu/drm/i915/display/intel_dsb.c | 65 +++++++++++++++++++ > drivers/gpu/drm/i915/display/intel_dsb.h | 2 + > drivers/gpu/drm/i915/display/intel_fbc.c | 21 ++++++ > drivers/gpu/drm/i915/display/intel_fbc.h | 1 + > 7 files changed, 104 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c > b/drivers/gpu/drm/i915/display/intel_crtc.c > index 10ed9bdfee76..71559f86fa63 100644 > --- a/drivers/gpu/drm/i915/display/intel_crtc.c > +++ b/drivers/gpu/drm/i915/display/intel_crtc.c > @@ -25,6 +25,7 @@ > #include "intel_display_trace.h" > #include "intel_display_types.h" > #include "intel_drrs.h" > +#include "intel_dsb.h" > #include "intel_dsi.h" > #include "intel_fifo_underrun.h" > #include "intel_parent.h" > @@ -341,6 +342,8 @@ static int __intel_crtc_init(struct intel_display > *display, enum pipe pipe) > crtc->pipe = pipe; > crtc->num_scalers = DISPLAY_RUNTIME_INFO(display)->num_scalers[pipe]; > > + intel_dsb_crtc_init(crtc); > + > if (DISPLAY_VER(display) >= 9) > primary = skl_universal_plane_create(display, pipe, PLANE_1); > else > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > b/drivers/gpu/drm/i915/display/intel_display.c > index 829d7a411dcc..5cdaa59f005e 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -6889,6 +6889,8 @@ static void intel_update_crtc(struct intel_atomic_state > *state, > if (new_crtc_state->use_flipq) { > intel_flipq_enable(new_crtc_state); > > + crtc->flipq_fb_bits = new_crtc_state->fb_bits; > + > intel_crtc_prepare_vblank_event(new_crtc_state, > &crtc->flipq_event); > > intel_flipq_add(crtc, INTEL_FLIPQ_PLANE_1, 0, INTEL_DSB_0, > @@ -6969,6 +6971,7 @@ static void intel_old_crtc_state_disables(struct > intel_atomic_state *state, > intel_atomic_get_new_crtc_state(state, pipe_crtc); > > pipe_crtc->active = false; > + intel_dsb_crtc_disable(pipe_crtc); > intel_fbc_disable(pipe_crtc); > > if (!new_pipe_crtc_state->hw.active) > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h > b/drivers/gpu/drm/i915/display/intel_display_types.h > index 20a07ea06b5e..c644ca0fe9b8 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > @@ -1522,6 +1522,15 @@ struct intel_crtc { > struct drm_pending_vblank_event *dsb_event; > /* armed event for flip queue based updates */ > struct drm_pending_vblank_event *flipq_event; > + /* > + * Frontbuffer bits for a pending flipq/DSB-driven visible update. > + */ > + unsigned int flipq_fb_bits; > + /* > + * intel_dsb_irq_handler() runs in hardirq context, so the > + * frontbuffer/FBC flush for flipq_fb_bits is deferred to this worker. > + */ > + struct work_struct flipq_fb_bits_work; > > /* Access to these should be protected by display->irq.lock. */ > bool cpu_fifo_underrun_disabled; > diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c > b/drivers/gpu/drm/i915/display/intel_dsb.c > index d9a270362a82..66f107493f93 100644 > --- a/drivers/gpu/drm/i915/display/intel_dsb.c > +++ b/drivers/gpu/drm/i915/display/intel_dsb.c > @@ -17,6 +17,8 @@ > #include "intel_display_types.h" > #include "intel_dsb.h" > #include "intel_dsb_regs.h" > +#include "intel_fbc.h" > +#include "intel_frontbuffer.h" > #include "intel_psr.h" > #include "intel_vblank.h" > #include "intel_vrr.h" > @@ -1106,6 +1108,59 @@ void intel_dsb_cleanup(struct intel_dsb *dsb) > kfree(dsb); > } > > +/* > + * Frontbuffer/FBC bookkeeping for a completed flipq commit may sleep (takes > + * mutexes), but intel_dsb_irq_handler() runs in hardirq context, so do it > + * here instead, on display->wq.unordered. > + */ > +static void intel_dsb_flipq_fb_bits_work(struct work_struct *work) > +{ > + struct intel_crtc *crtc = > + container_of(work, typeof(*crtc), flipq_fb_bits_work); > + struct intel_display *display = to_intel_display(crtc); > + unsigned int flipq_fb_bits; > + > + spin_lock_irq(&display->drm->event_lock); > + flipq_fb_bits = crtc->flipq_fb_bits; > + crtc->flipq_fb_bits = 0; > + spin_unlock_irq(&display->drm->event_lock); > + > + if (!flipq_fb_bits) > + return; > + > + intel_frontbuffer_flip(display, flipq_fb_bits); > + intel_fbc_flipq_post_update(display, crtc->pipe); > +} > + > +/** > + * intel_dsb_crtc_init - Initialize the per-CRTC DSB/flipq worker state. > + * @crtc: the CRTC > + */ > +void intel_dsb_crtc_init(struct intel_crtc *crtc) > +{ > + INIT_WORK(&crtc->flipq_fb_bits_work, intel_dsb_flipq_fb_bits_work); > +} > + > +/** > + * intel_dsb_crtc_disable - Drop any stale pending flipq FBC completion. > + * @crtc: the CRTC > + * > + * A flipq commit's completion (and hence its deferred frontbuffer/FBC > + * flush) can still be outstanding when @crtc gets disabled, eg. across a > + * modeset. Cancel and discard it so it can't run later against whatever > + * @crtc is reconfigured to next. > + */ > +void intel_dsb_crtc_disable(struct intel_crtc *crtc) > +{ > + struct intel_display *display = to_intel_display(crtc); > + > + cancel_work_sync(&crtc->flipq_fb_bits_work); > + > + spin_lock_irq(&display->drm->event_lock); > + crtc->flipq_fb_bits = 0; > + spin_unlock_irq(&display->drm->event_lock); > +} > + > void intel_dsb_irq_handler(struct intel_display *display, > enum pipe pipe, enum intel_dsb_id dsb_id) > { > @@ -1130,6 +1185,16 @@ void intel_dsb_irq_handler(struct intel_display > *display, > } > > spin_unlock(&display->drm->event_lock); > + > + /* > + * Flipq-driven commits complete asynchronously via DSB/DMC. > + * Notify frontbuffer/FBC tracking only once the queued update > + * has actually completed. The unlocked peek is just to avoid > + * pointlessly queueing work; intel_dsb_flipq_fb_bits_work() > + * re-checks flipq_fb_bits under event_lock. > + */ > + if (READ_ONCE(crtc->flipq_fb_bits)) > + queue_work(display->wq.unordered, > &crtc->flipq_fb_bits_work); > } > > errors = tmp & dsb_error_int_status(display); > diff --git a/drivers/gpu/drm/i915/display/intel_dsb.h > b/drivers/gpu/drm/i915/display/intel_dsb.h > index 3dcca9ed5371..86d4111c1850 100644 > --- a/drivers/gpu/drm/i915/display/intel_dsb.h > +++ b/drivers/gpu/drm/i915/display/intel_dsb.h > @@ -73,5 +73,7 @@ void intel_dsb_wait(struct intel_dsb *dsb); > > void intel_dsb_irq_handler(struct intel_display *display, > enum pipe pipe, enum intel_dsb_id dsb_id); > +void intel_dsb_crtc_init(struct intel_crtc *crtc); > +void intel_dsb_crtc_disable(struct intel_crtc *crtc); > > #endif > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c > b/drivers/gpu/drm/i915/display/intel_fbc.c > index f61b4a218d6e..05a26d991cd2 100644 > --- a/drivers/gpu/drm/i915/display/intel_fbc.c > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c > @@ -1930,6 +1930,27 @@ void intel_fbc_post_update(struct intel_atomic_state > *state, > } > } > > +/* > + * Flipq commits complete asynchronously, well after intel_fbc_post_update() > + * already ran against the not-yet-latched surface. Re-run the post-update > + * step for @pipe's FBC once the queued commit has actually completed, so any > + * flip nuke recompresses the surface that is really being scanned out. > + */ > +void intel_fbc_flipq_post_update(struct intel_display *display, enum pipe > pipe) > +{ > + struct intel_fbc *fbc = intel_fbc_for_pipe(display, pipe); > + > + if (!fbc) > + return; > + > + mutex_lock(&fbc->lock); > + > + if (fbc->state.plane && fbc->state.plane->pipe == pipe) > + __intel_fbc_post_update(fbc); > + > + mutex_unlock(&fbc->lock); > +} > + > static unsigned int intel_fbc_get_frontbuffer_bit(struct intel_fbc *fbc) > { > if (fbc->state.plane) > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.h > b/drivers/gpu/drm/i915/display/intel_fbc.h > index 6c96d690a2f5..ba7b93eb1424 100644 > --- a/drivers/gpu/drm/i915/display/intel_fbc.h > +++ b/drivers/gpu/drm/i915/display/intel_fbc.h > @@ -34,6 +34,7 @@ bool intel_fbc_pre_update(struct intel_atomic_state *state, > struct intel_crtc *crtc); > void intel_fbc_post_update(struct intel_atomic_state *state, > struct intel_crtc *crtc); > +void intel_fbc_flipq_post_update(struct intel_display *display, enum pipe > pipe); > void intel_fbc_init(struct intel_display *display); > void intel_fbc_cleanup(struct intel_display *display); > void intel_fbc_sanitize(struct intel_display *display); > -- > 2.43.0 -- Ville Syrjälä Intel
