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 <[email protected]> > > > > > --- > > > > > 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. > > >
