Re: [PATCH v3 1/3] drm/i915/psr: Set plane id bit in crtc_state->async_flip_planes for PSR

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

On Wed, 2025-12-03 at 15:15 +0200, Ville Syrjälä wrote:
> On Mon, Dec 01, 2025 at 03:24:55PM +0200, Jouni Högander wrote:
> > Currently plane id bit is set in crtc_state->async_flip_planes only
> > when
> > async flip toggle workaround is needed. We want to utilize
> > crtc_state->async_flip_planes further in Selective Fetch
> > calculation.
> > 
> > Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_plane.c | 10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_plane.c
> > b/drivers/gpu/drm/i915/display/intel_plane.c
> > index 7b7619d59251..de0a69c55582 100644
> > --- a/drivers/gpu/drm/i915/display/intel_plane.c
> > +++ b/drivers/gpu/drm/i915/display/intel_plane.c
> > @@ -603,8 +603,7 @@ static int
> > intel_plane_atomic_calc_changes(const struct intel_crtc_state
> > *old_cr
> >  	if (intel_plane_do_async_flip(plane, old_crtc_state,
> > new_crtc_state)) {
> >  		new_crtc_state->do_async_flip = true;
> >  		new_crtc_state->async_flip_planes |= BIT(plane-
> > >id);
> > -	} else if (plane->need_async_flip_toggle_wa &&
> > -		   new_crtc_state->uapi.async_flip) {
> > +	} else if (new_crtc_state->uapi.async_flip) {
> 
> I'd get rid of the if-else construct here now, and just do something
> like:
> 
> if (intel_plane_do_async_flip(...))
> 	new_crtc_state->do_async_flip = true;
> 
> if (new_crtc_state->uapi.async_flip) {
> 	/* ... */
> 	new_crtc_state->async_flip_planes |= BIT(plane->id);
> }

> 
> We should probably also move the plane->async_flip check
> out from intel_plane_do_async_flip() and just make it a
> drm_WARN_ON(..., uapi.async_flip && !plane->async_flip).
> But that's probably better left for a separate patch.

Sent new version, please check. Left out this drm_WARN_ON change. I
will send it afterwards.

BR,

Jouni Högander

> 
> >  		/*
> >  		 * On platforms with double buffered async flip
> > bit we
> >  		 * set the bit already one frame early during the
> > sync
> > @@ -612,6 +611,13 @@ static int
> > intel_plane_atomic_calc_changes(const struct intel_crtc_state
> > *old_cr
> >  		 * hardware will therefore be ready to perform a
> > real
> >  		 * async flip during the next commit, without
> > having
> >  		 * to wait yet another frame for the bit to latch.
> > +		 *
> > +		 * async_flip_planes bitmask is also used by
> > selective
> > +		 * fetch calculation to continue full frame
> > updates as
> > +		 * long as there may be pending async flip on any
> > +		 * plane which is part of selective
> > +		 * update. I.e. old_crtc_state->async_flip_planes
> > &
> > +		 * BIT(<plane in su area>->id).
> >  		 */
> >  		new_crtc_state->async_flip_planes |= BIT(plane-
> > >id);
> >  	}
> > -- 
> > 2.43.0
>