Re: [PATCH 2/4] drm/i915/psr: Perform vblank evasion on async flip as well for PSR

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Nov 27, 2025 at 10:57:09AM +0000, Hogander, Jouni wrote:
> On Tue, 2025-11-25 at 23:19 +0200, Ville Syrjälä wrote:
> > On Tue, Nov 25, 2025 at 08:32:51AM +0200, Jouni Högander wrote:
> > > PSR2_MAN_TRK_CTL[SF Continuous full frame] is sampled on the rising
> > > edge of
> > > delayed vblank. SW must ensure this bit is not changing around
> > > that. Due to
> > > this PSR2 Selective Fetch needs vblank evasion.
> > > 
> > > Currently vblank evasion is not done on async flip. Perform it in
> > > case
> > > required by PSR.
> > > 
> > > Bspec: 50424
> > > Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_crtc.c | 6 ++++--
> > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c
> > > b/drivers/gpu/drm/i915/display/intel_crtc.c
> > > index 153ff4b4b52c..42c4ce07f8c0 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_crtc.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_crtc.c
> > > @@ -433,7 +433,8 @@ static bool intel_crtc_needs_vblank_work(const
> > > struct intel_crtc_state *crtc_sta
> > >  		(intel_crtc_needs_color_update(crtc_state) &&
> > >  		 !HAS_DOUBLE_BUFFERED_LUT(display)) &&
> > >  		!intel_color_uses_dsb(crtc_state) &&
> > > -		!crtc_state->use_dsb;
> > > +		!crtc_state->use_dsb &&
> > > +		!crtc_state->do_async_flip;
> > >  }
> > >  
> > >  static void intel_crtc_vblank_work(struct kthread_work *base)
> > > @@ -539,7 +540,8 @@ void intel_pipe_update_start(struct
> > > intel_atomic_state *state,
> > >  	if (new_crtc_state->do_async_flip) {
> > >  		intel_crtc_prepare_vblank_event(new_crtc_state,
> > >  						&crtc-
> > > >flip_done_event);
> > > -		return;
> > > +		if (!intel_psr_needs_evasion(new_crtc_state))
> > > +			return;
> > 
> > I don't think we want hack this into such low level code. We
> > anyway convert the first async flip to a sync flip (see
> > intel_plane_do_async_flip()), so that's when you should disable
> > selective fetch, and keep it disabled afterwards as long as
> > async flips are being requested for the plane by userspace.
> 
> Isn't async flip always initiated by user space (uapi.async_flip == 1)?
> Are you concerned on this sequence:
> 
> 1. async flip on primary plane (full frame update)
> 2. normal flip on secondary plane (selective fetch/update)
> 3. async flip on primary plane (full frame update)
> 
> Is there some problem in performing selective fetch/update on step 2?
> Please note that we are not disabling PSR2 at step 2. We are just
> performing 1 selective fetch/update in between there.

That selective update may pull in planes that are doing async flips
currently, and I'm certain we don't have the code to update the state
tracking to indicate that they're no longer in, what I like to think
as, "async flip mode". I suppose the distinction might not matter
too much for these platforms (assuming has_sel_fetch and
need_async_flip_toggle_wa don't overlap), but we should still keep
the code consistent to make it easier to understand.

I suppose you could handle it correctly by clearing async_flip_planes
in appropriate places, but I still don't like adding yet another
special case to the commit codepaths. I think that code is
complex enough already.

-- 
Ville Syrjälä
Intel