Re: [PATCH v3 2/3] drm/i915/psr: Perform full frame update on async flip

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

On Wed, Dec 03, 2025 at 01:58:23PM +0000, Hogander, Jouni wrote:
> On Wed, 2025-12-03 at 15:22 +0200, Ville Syrjälä wrote:
> > On Mon, Dec 01, 2025 at 03:24:56PM +0200, Jouni Högander wrote:
> > > According to bspec selective fetch is not supported with async
> > > flips and
> > > instructing full frame update on async flip.
> > > 
> > > v3:
> > >   - rebase
> > >   - fix old_crtc_state->pipe_srcsz_early_tpt
> > >   - fix using intel_atomic_get_new_crtc_state
> > > v2:
> > >   - check also crtc_state->async_flip_planes in
> > >     psr2_sel_fetch_plane_state_supported
> > > 
> > > Bspec: 55229
> > > Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_psr.c | 72 ++++++++++++++------
> > > ----
> > >  1 file changed, 41 insertions(+), 31 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > > b/drivers/gpu/drm/i915/display/intel_psr.c
> > > index 15ef3b6caad6..53cf292247d7 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > @@ -2728,13 +2728,20 @@ intel_psr2_sel_fetch_et_alignment(struct
> > > intel_atomic_state *state,
> > >   * Plane scaling and rotation is not supported by selective fetch
> > > and both
> > >   * properties can change without a modeset, so need to be check at
> > > every
> > >   * atomic commit.
> > > + *
> > > + * If plane was having async flip previously we can't use
> > > selective
> > > + * fetch as we don't know if the flip is completed.
> > >   */
> > > -static bool psr2_sel_fetch_plane_state_supported(const struct
> > > intel_plane_state *plane_state)
> > > +static bool psr2_sel_fetch_plane_state_supported(const struct
> > > intel_crtc_state *old_crtc_state,
> > > +						 const struct
> > > intel_plane_state *plane_state)
> > >  {
> > > +	struct intel_plane *plane = to_intel_plane(plane_state-
> > > >uapi.plane);
> > > +
> > >  	if (plane_state->uapi.dst.y1 < 0 ||
> > >  	    plane_state->uapi.dst.x1 < 0 ||
> > >  	    plane_state->scaler_id >= 0 ||
> > > -	    plane_state->hw.rotation != DRM_MODE_ROTATE_0)
> > > +	    plane_state->hw.rotation != DRM_MODE_ROTATE_0 ||
> > > +	    old_crtc_state->async_flip_planes & plane->id)
> > 
> > Why are you looking at the old crtc state? There should be nothing of
> > interest to us there.
> 
> To continue keeping CFF bit set if previous update was async flip. This
> is how I understood it (please correct):
> 
> 0. syncronous update
> 
> 1. async_flip:
>    new_crtc_state->async_flip_planes != 0
>    old_crtc_state->async_flip_planes == 0
>    crtc_state->uapi.async_flip == true)
>    -> full frame update
> 
> 2. async_flip:
>    new_crtc_state->async_flip_planes != 0
>    old_crtc_state->async_flip_planes != 0
>    crtc_state->uapi.async_flip == true
>    -> full frame update
> 
> 3. syncronous update
>    new_crtc_state->async_flip_planes == 0
>    old_crtc_state->async_flip_planes != 0
>    crtc_state->uapi.async_flip == false
>    -> full frame update
> 
> 4. syncronous update
>    new_crtc_state->async_flip_planes == 0
>    old_crtc_state->async_flip_planes == 0
>    crtc_state->uapi.async_flip == false
>    -> selective update
> 
> > 
> > >  		return false;
> > >  
> > >  	return true;
> > > @@ -2749,7 +2756,8 @@ static bool
> > > psr2_sel_fetch_plane_state_supported(const struct intel_plane_state
> > >   */
> > >  static bool psr2_sel_fetch_pipe_state_supported(const struct
> > > intel_crtc_state *crtc_state)
> > >  {
> > > -	if (crtc_state->scaler_state.scaler_id >= 0)
> > > +	if (crtc_state->scaler_state.scaler_id >= 0 ||
> > > +	    crtc_state->uapi.async_flip)
> > 
> > I think just checking crtc_state->async_flip_planes!=0 here should be
> > sufficient.
> 
> I'm doing this to handle step 1. above. Alternatively I could check
> both new_crtc_state->async_flip_planes and old_crtc_state-
> >async_flip_planes. When using crtc_state->uapi.async_flip I can decide
> earlier.
> 
> > The rest of the patch seems unnecessary.
> 
> No need to handle selective update where planes having pending async
> flip are not involved?

There won't be pending flips of any kind. We whole CRTC commit queue
is handled in a strict FIFO order (apart from the legacy cursor
special case).

-- 
Ville Syrjälä
Intel