On 9/29/2025 2:15 PM, 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
Alright, so we avoid changing the vblank_start.
It means for platforms with always_use_vrr_tg()==true we directly set
the value for guardband. (Currently I was getting it from vmin_vtotal -
vblank_start)
For platforms ADL+ with always always_use_vrr_tg()== false for the fixed
refresh rate case, guardband is full vblank_length for variable refresh
rate set the guardband directly.
As you have mentioned need to check which all places we need vblank_start.
For ICL/TGL we do not use optimization for now, right?
The extra_vblank_delay quirk is already handled while filling the
registers.
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.
I think I didnt get this part:
Do you mean later at some point we move to option 2:
always_use_vrr_tg()==true for all platforms.(Need to check if we can do
it for ICL, TGL).
I kinda like option 2 because then we'll be doing the vrr vs.
fixed refresh rate always the same way. However we haven't really
tested that mode of operation on the older platforms, so I'd
rather not bet all your work on that working. If we later run
into problems with that then we'd have to revert everything.
So I think we should start with option 1, adjust all the
crtc_vblank_start users approriately (I don't think there are
too many of them), and adjust crtc_vblank_start to match
the guardband only when always_use_vrr_tg()==true.
Sure will start with this for now.
Thanks & Regards,
Ankit
After that I think we might still have some potential issues/race
conditions around the actual vrr <-> fixed refresh rate switch.
Those might require more work later, or if we decide at that point
to try option 2 maybe we could sidestep some/all of them.
