On Wed, Oct 15, 2025 at 12:52:17PM +0530, Ankit Nautiyal wrote: > Update allow_vblank_delay_fastset() to permit vblank delay adjustments > during with LRR when VRR TG is always active. > > Signed-off-by: Ankit Nautiyal <[email protected]> > --- > drivers/gpu/drm/i915/display/intel_display.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > b/drivers/gpu/drm/i915/display/intel_display.c > index ceee5ae99c2c..65a7da694ef6 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -4958,9 +4958,15 @@ static bool allow_vblank_delay_fastset(const struct > intel_crtc_state *old_crtc_s > * Allow fastboot to fix up vblank delay (handled via LRR > * codepaths), a bit dodgy as the registers aren't > * double buffered but seems to be working more or less... > + * > + * Also allow this when the VRR timing generator is always on, > + * and optimized guardband is used. In such cases, > + * vblank delay may vary even without inherited state, but it's > + * still safe as VRR guardband is still same. > */ > - return HAS_LRR(display) && old_crtc_state->inherited && > - !intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_DSI); > + return HAS_LRR(display) && > + (old_crtc_state->inherited || > intel_vrr_always_use_vrr_tg(display)) && > + !intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_DSI);
I suppose this won't actually do anything until we get the fixed guardband size in place. But with that I suppose it is the right thing to do. Reviewed-by: Ville Syrjälä <[email protected]> However I was pondering about the place where do timing generator reprogramming. Currently that is done from within the vblank evasion critical section. But that is actually wrong because the VRR registers aren't double buffered. So in the worst case we'll evade the previous vblank start, and then reprogram the timing generator which could move the vblank start to be just ahead of the current scanline and then the commit will end up straddling the start of vblank (which is exactly what the vblank evasion tried to prevent). So I think we'll have to move the timing generator update to happen after we've done all the double buffered register programming. I suppose that might still be technically wrong as then the position of the delayed vblank might still move before the double buffered registers have been latched. I don't think that shouldn't cause any underruns/etc. but in the worst case the start of vblank moves backwards past the current scanline, and then the registers don't actually latch until the next frame. I wonder if we should use the vblank worker here to do the timing generator update right after the delayed vblank? That would guarantee that the current delayed vblank stays in place until the register have been latched. Though we may still end up in at least two weird scenarios: - delayed vblank moves forward, and we might get two delayed vblank events for the same frame - vtotal gets reduced below the current delayed vblank start. Which I suppose means the vtotal for the frame will not necessarily be the old or new vtotal value, but something in between. That's all assuming certain behaviours of the VRR timing generator of course. I haven't actuall confirmed how the hardware behaves in either case. We should probably do some more hw poking at some point to really figure this stuff out... -- Ville Syrjälä Intel
