On Wed, Nov 26, 2025 at 08:45:31PM +0000, Shankar, Uma wrote: > > > > -----Original Message----- > > From: Intel-xe <[email protected]> On Behalf Of Maarten > > Lankhorst > > Sent: Tuesday, November 4, 2025 2:07 PM > > To: [email protected]; [email protected] > > Cc: [email protected]; Maarten Lankhorst <[email protected]>; > > Mario > > Kleiner <[email protected]>; Mike Galbraith > > <[email protected]>; Thomas Gleixner <[email protected]>; Sebastian > > Andrzej Siewior <[email protected]>; Clark Williams > > <[email protected]>; Steven Rostedt <[email protected]> > > Subject: [PATCH v2 6/7] drm/i915/display: Enable interrupts earlier on > > PREEMPT_RT > > > > The last part of the vblank evasion is about updating bookkeeping, not > > programming hardware registers. > > > > The interrupts cannot stay disabled here on PREEMPT_RT since the spinlocks > > get > > converted to mutexes. > > > > Signed-off-by: Maarten Lankhorst <[email protected]> > > --- > > drivers/gpu/drm/i915/display/intel_crtc.c | 10 ++++++++++ > > 1 file changed, 10 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c > > b/drivers/gpu/drm/i915/display/intel_crtc.c > > index 9d2a23c96c61b..b87f6b4a4f3d7 100644 > > --- a/drivers/gpu/drm/i915/display/intel_crtc.c > > +++ b/drivers/gpu/drm/i915/display/intel_crtc.c > > @@ -688,6 +688,14 @@ void intel_pipe_update_end(struct intel_atomic_state > > *state, > > intel_crtc_has_type(new_crtc_state, INTEL_OUTPUT_DSI)) > > icl_dsi_frame_update(new_crtc_state); > > > > +#if IS_ENABLED(CONFIG_PREEMPT_RT) > > + /* > > + * Timing sensitive register writing completed, non-deterministic > > + * locking from here on out. > > + */ > > + local_irq_enable(); > > +#endif > > I think we do have VRR send push etc handled here, also arming registers are > being updated. > Not sure we can allow interrupts here. Please check once
Yeah, this doesn't seem exactly great. Even without VRR we want the register writes and vblank event arming to happen in the same frame. Though without VRR I suppose the worst that could happen is that we complete the commit one frame too late. With VRR however we need the vblank event arming and push to happen in the same frame. Otherwise we'll complete the flip early and leave push send assrted, which causes the next frame to terminate at vmin. Basically that makes the next frame a mailbox flip as far as push send is concerned. The race is already there, but allowing the CPU to get scheduled away will widen it. We do try to handle it in the vblank evasion, but I think we're handling it way too early (in intel_vblank_evade_init()) so that part itself is racy. I suppose we should rather do the vmin vs. vmax evasion decision after we've actually read out the current scanline. That should at least make it a bit more robust. One other thing we could maybe think about is arming the vblank event after the push is sent (with seq = current+1), and then immediately check if the push bit already cleared, and if so cancel the arming and send the event directly (with seq = current). But that's just a quick idead that popped to my head, didn't really think it through in detail. I'm tempted to say we should just make the vblank locks raw spinlocks as well. But I've not looked at what other drivers do in the vblank hooks so dunno how feasible that is. -- Ville Syrjälä Intel
