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

["Hogander, Jouni" <jouni.hogander@intel.com>]

On Fri, 2025-11-28 at 16:40 +0200, Hogander, Jouni wrote:
> On Fri, 2025-11-28 at 00:02 +0200, Ville Syrjälä wrote:
> > 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_sta
> > > > > te
> > > > > ,
> > > > >  						&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".
> 
> Ok. You earlier mentioned uapi.async_flip being ephemeral. So
> checking
> both old_crtc_state->uapi.async_flip and new_crtc_state-
> > uapi.async_flip is not possible? If both are false then we would
> > know
> there is at least one non-async update after possible async update so
> there can't be pending async flip.

Sent out now version using crtc_state->async_flip_planes.

BR,

Jouni Högander

> 
> BR,
> 
> Jouni Högander
> 
> > 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.
> > 
>