Re: [PATCH v2 4/4] drm/i915/psr: Add poll for checking PSR is idle before starting update

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

On Tue, Aug 26, 2025 at 02:30:50PM +0000, Hogander, Jouni wrote:
> On Tue, 2025-08-26 at 15:36 +0300, Ville Syrjälä wrote:
> > On Wed, Aug 06, 2025 at 08:22:13AM +0300, Jouni Högander wrote:
> > > We are currently observing crc failures after we started using dsb
> > > for PSR
> > > updates as well. This seems to happen because PSR HW is still
> > > sending
> > > couple of updates using old framebuffers on wake-up.
> > > 
> > > Fix this by adding poll ensuring PSR is idle before starting
> > > update.
> > > 
> > > Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_display.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > index c1a3a95c65f0..411c74c73eae 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > @@ -7271,6 +7271,8 @@ static void intel_atomic_dsb_finish(struct
> > > intel_atomic_state *state,
> > >  		intel_psr_trigger_frame_change_event(new_crtc_stat
> > > e->dsb_commit,
> > >  						     state, crtc);
> > >  
> > > +		intel_psr_wait_for_idle_dsb(new_crtc_state);
> > > +
> > >  		if (new_crtc_state->use_dsb)
> > >  			intel_dsb_vblank_evade(state,
> > > new_crtc_state->dsb_commit);
> > 
> > How come the 'evade scanline 0 + normal evasion' in
> > intel_dsb_vblank_evade()
> > is not enough here?
> 
> the problem doesn't happen when PSR is in SRD_ENT/DEEP_SLEEP as
> committing new changes is started. It seems to happen when PSR
> transitioning into SRD_ENT/DEEP_SLEEP is ongoing when new changes are
> being committed. In this case evasion passes and PSR is continuing
> transitioning into SRD_ENT/DEEP_SLEEP. Then wake-up starts immediately
> due to pending "Frame Change" event and in this case HW is sending a
> frame using old plane configuration.

That's a bit weird. I think we are configuring things so that there
should be that extra vblank already for the first frame after PSR
exit. So I would expect things to latch properly even if we write
the registers during the PSR entry sequence.

Hmm, though the DSB itself never waits for the delayed vblank
directly. Maybe the delayed vblank does get suppressed for
one frame during the sequence somewhere, but the undelayed
vblank used by the DSB does not.

One could perhaps try to sample a timestamp from the DSB after
it thinks the vblank has happened, and sample another one from
the CPU delayed vblank interrupt (which I would expect to match
when the hardware really latches stuff), and compare how they
look. Though that does require one to enable the CPU interrupt
which itself will trigger the PSR exit (at least on some hw),
so not sure how easy it is to hit the exact conditions required.
I suppose one might try to wait for the PSR state machine to
be in the right state just before triggering the exit.

> 
> Bspec is saying this transition to SRD_ENT/DEEP_SLEEP can't be
> interrupted.
> 
> BR,
> 
> Jouni Högander
> > 
> 

-- 
Ville Syrjälä
Intel