On Mon, Sep 29, 2025 at 11:45:19AM +0300, Ville Syrjälä wrote:
> On Sun, Sep 28, 2025 at 12:35:40PM +0530, Ankit Nautiyal wrote:
> > +static void intel_crtc_vblank_delay(struct intel_atomic_state *state,
> > + struct intel_crtc *crtc)
> > +{
> > + struct intel_crtc_state *crtc_state =
> > + intel_atomic_get_new_crtc_state(state, crtc);
> > + struct drm_display_mode *adjusted_mode =
> > + &crtc_state->hw.adjusted_mode;
> > + int vblank_delay = 0;
> > +
> > + vblank_delay = intel_crtc_min_guardband_delay(state, crtc);
> > +
> > + adjusted_mode->crtc_vblank_start += vblank_delay;
>
> The situation with crtc_vblank_start is already kinda broken,
> and I think we need to fix that first somehow.
>
> Currently crtc_vblank_start is assumed to be the vblank_start
> for the fixed refresh rate case. That value can be different
> from the variable refresh rate case whenever
> always_use_vrr_tg()==false. On icl/tgl it's always different
> due to the extra vblank delay, and also on adl+ it could be
> different if we were to use an optimized guardband.
>
> I think there are a few options how we might solve this:
> 1. keep crtc_vblank_start as is, and make sure every user of it
> gets adjusted to also deal with the vrr case correctly
> 2. enable always_use_vrr_tg() whenever there might be switch
> between vrr and fixed refresh rate, which I think would mean
> crtc_state->vrr.in_range==true.
And just as a reminder in case I think of it again later:
3. use an optimized guardband only when always_use_vrr_tg()==true
But that wouldn't actually solve the already existing issue
on icl/tgl. So not really a good option.
--
Ville Syrjälä
Intel
