Re: [PATCH v3 2/2] drm/i915/alpm: Stop writing ALPM registers when PSR is enabled

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

On Tue, 2025-05-13 at 03:59 +0000, Kandpal, Suraj wrote:
> 
> 
> > -----Original Message-----
> > From: Intel-xe <intel-xe-bounces@lists.freedesktop.org> On Behalf
> > Of Jouni
> > Högander
> > Sent: Thursday, May 8, 2025 7:47 PM
> > To: intel-gfx@lists.freedesktop.org; intel-xe@lists.freedesktop.org
> > Cc: Manna, Animesh <animesh.manna@intel.com>; Hogander, Jouni
> > <jouni.hogander@intel.com>
> > Subject: [PATCH v3 2/2] drm/i915/alpm: Stop writing ALPM registers
> > when PSR
> > is enabled
> > 
> > Currently we are seeing these on PTL:
> > 
> > xe 0000:00:02.0: [drm] *ERROR* Timeout waiting for DDI BUF A to get
> > active
> > 
> > These seem to be caused by writing ALPM registers while Panel
> > Replay is
> > enabled.
> > 
> > Fix this by writing ALPM registers only when Panel Replay is about
> > to be
> > enabled.
> > 
> > v3: enable/disable ALPM from PSR code
> > 
> > Fixes: 172757acd6f6 ("drm/i915/lobf: Add lobf enablement in post
> > plane
> > update")
> > Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_alpm.c | 4 ++--
> > drivers/gpu/drm/i915/display/intel_psr.c  | 8 +++++++-
> >  2 files changed, 9 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c
> > b/drivers/gpu/drm/i915/display/intel_alpm.c
> > index 9442483058d2..bc93e8026703 100644
> > --- a/drivers/gpu/drm/i915/display/intel_alpm.c
> > +++ b/drivers/gpu/drm/i915/display/intel_alpm.c
> > @@ -460,8 +460,8 @@ void intel_alpm_post_plane_update(struct
> > intel_atomic_state *state,
> >  		intel_atomic_get_old_crtc_state(state, crtc);
> >  	struct intel_encoder *encoder;
> > 
> > -	if ((!crtc_state->has_lobf ||
> > -	     crtc_state->has_lobf == old_crtc_state->has_lobf) &&
> > !crtc_state-
> > > has_psr)
> > +	if (crtc_state->has_psr || !crtc_state->has_lobf ||
> > +	    crtc_state->has_lobf == old_crtc_state->has_lobf)
> >  		return;
> > 
> >  	for_each_intel_encoder_mask(display->drm, encoder, diff --
> > git
> > a/drivers/gpu/drm/i915/display/intel_psr.c
> > b/drivers/gpu/drm/i915/display/intel_psr.c
> > index ccd66bbc72f7..0dbe59d06187 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -800,6 +800,8 @@ static void _psr_enable_sink(struct intel_dp
> > *intel_dp,
> > static void intel_psr_enable_sink(struct intel_dp *intel_dp,
> >  				  const struct intel_crtc_state
> > *crtc_state)  {
> > +	intel_alpm_enable_sink(intel_dp, crtc_state);
> > +
> >  	crtc_state->has_panel_replay ?
> >  		_panel_replay_enable_sink(intel_dp, crtc_state) :
> >  		_psr_enable_sink(intel_dp, crtc_state); @@ -1962,6
> > +1964,8
> > @@ static void intel_psr_enable_source(struct intel_dp *intel_dp,
> >  	     IS_DISPLAY_VERx100_STEP(display, 3000, STEP_A0,
> > STEP_B0)) &&
> >  	    !intel_dp->psr.panel_replay_enabled)
> >  		intel_dmc_block_pkgc(display, intel_dp->psr.pipe,
> > true);
> > +
> > +	intel_alpm_configure(intel_dp, crtc_state);
> 
> Now that we do alpm configure and enable sink from psr code do we
> need to do it from the alpm_post_plane code?

alpm_pre_plane and alpm_post_plane are for LOBF only after this change.

> 
> Also on a side note intel_psr_enable_sink has a comment where it is
> called (intel_psr_enable_locked)
> which tells us what all is being done inside the function we need to
> add the alpm enablement too there.

Ok, I will add there.

BR,

Jouni Högander

> 
> Regards,
> Suraj Kandpal
> 
> >  }
> > 
> >  static bool psr_interrupt_error_check(struct intel_dp *intel_dp)
> > @@ -2172,6
> > +2176,9 @@ static void intel_psr_disable_locked(struct intel_dp
> > *intel_dp)
> >  	if (intel_dp_is_edp(intel_dp))
> > 
> > 	intel_snps_phy_update_psr_power_state(&dp_to_dig_port(inte
> > l_dp)-
> > > base, false);
> > 
> > +	if (intel_dp->psr.panel_replay_enabled &&
> > intel_dp_is_edp(intel_dp))
> > +		intel_alpm_disable(intel_dp);
> > +
> >  	/* Disable PSR on Sink */
> >  	if (!intel_dp->psr.panel_replay_enabled) {
> >  		drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
> > 0);
> > @@ -3498,7 +3505,6 @@ static void psr_alpm_check(struct intel_dp
> > *intel_dp)
> >  	if (intel_alpm_get_error(intel_dp)) {
> >  		intel_psr_disable_locked(intel_dp);
> >  		psr->sink_not_reliable = true;
> > -		intel_alpm_disable(intel_dp);
> >  	}
> >  }
> > 
> > --
> > 2.43.0
>