Re: [PATCH v4 12/13] drm/i915/display: Ensure we have "Frame Change" event in DSB commit

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

On Fri, Jan 24, 2025 at 01:43:25PM +0200, Ville Syrjälä wrote:
> On Fri, Jan 24, 2025 at 12:56:23PM +0200, Jouni Högander wrote:
> > We may have commit which doesn't have any non-arming plane register
> > writes. In that case there aren't "Frame Change" event before DSB vblank
> > evasion which hangs as PIPEDSL register is reading as 0 when PSR state is
> > SRDENT(PSR1) or DEEP_SLEEP(PSR2). Handle this by adding dummy write
> > triggering the "Frame Change" event.
> > 
> > Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index a189aa437d972..cd7e960bd29f1 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -7666,6 +7666,7 @@ static void intel_atomic_dsb_finish(struct intel_atomic_state *state,
> >  		intel_atomic_get_old_crtc_state(state, crtc);
> >  	struct intel_crtc_state *new_crtc_state =
> >  		intel_atomic_get_new_crtc_state(state, crtc);
> > +	struct intel_display *display = to_intel_display(crtc);
> >  
> >  	if (!new_crtc_state->hw.active)
> >  		return;
> > @@ -7708,6 +7709,15 @@ static void intel_atomic_dsb_finish(struct intel_atomic_state *state,
> >  		intel_crtc_planes_update_noarm(new_crtc_state->dsb_commit,
> >  					       state, crtc);
> >  
> > +		/*
> > +		 * Ensure we have "Frame Change" event when PSR state is
> > +		 * SRDENT(PSR1) or DEEP_SLEEP(PSR2). Otherwise DSB vblank
> > +		 * evasion hangs as PIPEDSL is reading as 0.
> > +		 */
> > +		if (new_crtc_state->has_psr && !new_crtc_state->has_panel_replay)
> > +			intel_de_write_dsb(display, new_crtc_state->dsb_commit,
> > +					   CURSURFLIVE(display, crtc->pipe), 0);
> 
> I don't really want to see the low level detais right here.
> So we should probably should stuff this into some kind of 
> intel_dsb_ensure_psr_frame_change() function or something
> along those lines.

Oh, and I guess this really should be using
intel_pre_commit_crtc_state() as well. I suppose it doesn't
actually matter if we skip commits that change PSR stuff anyway,
but at some point the plan is to attempt full fastsets with
the DSB (if actually possible).

> 
> > +
> >  		intel_dsb_vblank_evade(state, new_crtc_state->dsb_commit);
> >  
> >  		if (intel_crtc_needs_color_update(new_crtc_state))
> > -- 
> > 2.43.0
> 
> -- 
> Ville Syrjälä
> Intel

-- 
Ville Syrjälä
Intel