[v2] drm/i915/display: Ensure PSR gets disabled if no encoders in new state

Commit Message

Hogander, Jouni July 11, 2022, 11:17 a.m. UTC

Currently PSR is left enabled when all planes are disabled if there
is no attached encoder in new state. This seems to be causing FIFO
underruns.

Fix this by checking if encoder exists in new crtc state and disable
PSR if it doesn't.

v2: Unify disable logic with existing

Cc: Mika Kahola <[e-mail]>
Reported-by: Stanislav Lisovskiy <[e-mail]>
Signed-off-by: Jouni Högander <[e-mail]>
Signed-off-by: Stanislav Lisovskiy <[e-mail]>
---
 drivers/gpu/drm/i915/display/intel_psr.c | 53 ++++++++++++++----------
 1 file changed, 31 insertions(+), 22 deletions(-)

Comments

Lisovskiy, Stanislav July 11, 2022, 2:10 p.m. UTC | #1

On Mon, Jul 11, 2022 at 02:17:50PM +0300, Jouni Högander wrote:
> Currently PSR is left enabled when all planes are disabled if there
> is no attached encoder in new state. This seems to be causing FIFO
> underruns.
> 
> Fix this by checking if encoder exists in new crtc state and disable
> PSR if it doesn't.
> 
> v2: Unify disable logic with existing

Reviewed-by: Stanislav Lisovskiy <[e-mail]>

> 
> Cc: Mika Kahola <[e-mail]>
> Reported-by: Stanislav Lisovskiy <[e-mail]>
> Signed-off-by: Jouni Högander <[e-mail]>
> Signed-off-by: Stanislav Lisovskiy <[e-mail]>
> ---
>  drivers/gpu/drm/i915/display/intel_psr.c | 53 ++++++++++++++----------
>  1 file changed, 31 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index e6a870641cd2..90599dd1cb1b 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -1863,36 +1863,45 @@ void intel_psr_pre_plane_update(struct intel_atomic_state *state,
>  				struct intel_crtc *crtc)
>  {
>  	struct drm_i915_private *i915 = to_i915(state->base.dev);
> -	const struct intel_crtc_state *crtc_state =
> -		intel_atomic_get_new_crtc_state(state, crtc);
> -	struct intel_encoder *encoder;
> +	struct intel_crtc_state *new_crtc_state, *old_crtc_state;
> +	int i;
>  
>  	if (!HAS_PSR(i915))
>  		return;
>  
> -	for_each_intel_encoder_mask_with_psr(state->base.dev, encoder,
> -					     crtc_state->uapi.encoder_mask) {
> -		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> -		struct intel_psr *psr = &intel_dp->psr;
> -		bool needs_to_disable = false;
> +	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
> +					    new_crtc_state, i) {
> +		struct intel_encoder *encoder;
> +		u32 old_new_encoder_mask = old_crtc_state->uapi.encoder_mask |
> +			new_crtc_state->uapi.encoder_mask;
>  
> -		mutex_lock(&psr->lock);
> +		for_each_intel_encoder_mask_with_psr(state->base.dev, encoder,
> +						     old_new_encoder_mask) {
> +			struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> +			struct intel_psr *psr = &intel_dp->psr;
> +			bool needs_to_disable = false;
>  
> -		/*
> -		 * Reasons to disable:
> -		 * - PSR disabled in new state
> -		 * - All planes will go inactive
> -		 * - Changing between PSR versions
> -		 */
> -		needs_to_disable |= intel_crtc_needs_modeset(crtc_state);
> -		needs_to_disable |= !crtc_state->has_psr;
> -		needs_to_disable |= !crtc_state->active_planes;
> -		needs_to_disable |= crtc_state->has_psr2 != psr->psr2_enabled;
> +			mutex_lock(&psr->lock);
>  
> -		if (psr->enabled && needs_to_disable)
> -			intel_psr_disable_locked(intel_dp);
> +			/*
> +			 * Reasons to disable:
> +			 * - PSR disabled in new state
> +			 * - All planes will go inactive
> +			 * - Changing between PSR versions
> +			 * - Encoder isn't present in new mask
> +			 */
> +			needs_to_disable |= intel_crtc_needs_modeset(new_crtc_state);
> +			needs_to_disable |= !new_crtc_state->has_psr;
> +			needs_to_disable |= !new_crtc_state->active_planes;
> +			needs_to_disable |= new_crtc_state->has_psr2 != psr->psr2_enabled;
> +			needs_to_disable |= !(new_crtc_state->uapi.encoder_mask &
> +					      drm_encoder_mask(&(encoder)->base));
>  
> -		mutex_unlock(&psr->lock);
> +			if (psr->enabled && needs_to_disable)
> +				intel_psr_disable_locked(intel_dp);
> +
> +			mutex_unlock(&psr->lock);
> +		}
>  	}
>  }
>  
> -- 
> 2.25.1
>

Souza, Jose July 13, 2022, 8:58 p.m. UTC | #2

On Mon, 2022-07-11 at 14:17 +0300, Jouni Högander wrote:
> Currently PSR is left enabled when all planes are disabled if there
> is no attached encoder in new state. This seems to be causing FIFO
> underruns.

What is the case were there is no attached encoder and active_planes > 0?

> 
> Fix this by checking if encoder exists in new crtc state and disable
> PSR if it doesn't.
> 
> v2: Unify disable logic with existing
> 
> Cc: Mika Kahola <[e-mail]>
> Reported-by: Stanislav Lisovskiy <[e-mail]>
> Signed-off-by: Jouni Högander <[e-mail]>
> Signed-off-by: Stanislav Lisovskiy <[e-mail]>
> ---
>  drivers/gpu/drm/i915/display/intel_psr.c | 53 ++++++++++++++----------
>  1 file changed, 31 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index e6a870641cd2..90599dd1cb1b 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -1863,36 +1863,45 @@ void intel_psr_pre_plane_update(struct intel_atomic_state *state,
>  				struct intel_crtc *crtc)
>  {
>  	struct drm_i915_private *i915 = to_i915(state->base.dev);
> -	const struct intel_crtc_state *crtc_state =
> -		intel_atomic_get_new_crtc_state(state, crtc);
> -	struct intel_encoder *encoder;
> +	struct intel_crtc_state *new_crtc_state, *old_crtc_state;
> +	int i;
>  
>  	if (!HAS_PSR(i915))
>  		return;
>  
> -	for_each_intel_encoder_mask_with_psr(state->base.dev, encoder,
> -					     crtc_state->uapi.encoder_mask) {
> -		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> -		struct intel_psr *psr = &intel_dp->psr;
> -		bool needs_to_disable = false;
> +	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
> +					    new_crtc_state, i) {
> +		struct intel_encoder *encoder;
> +		u32 old_new_encoder_mask = old_crtc_state->uapi.encoder_mask |
> +			new_crtc_state->uapi.encoder_mask;
>  
> -		mutex_lock(&psr->lock);
> +		for_each_intel_encoder_mask_with_psr(state->base.dev, encoder,
> +						     old_new_encoder_mask) {
> +			struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> +			struct intel_psr *psr = &intel_dp->psr;
> +			bool needs_to_disable = false;
>  
> -		/*
> -		 * Reasons to disable:
> -		 * - PSR disabled in new state
> -		 * - All planes will go inactive
> -		 * - Changing between PSR versions
> -		 */
> -		needs_to_disable |= intel_crtc_needs_modeset(crtc_state);
> -		needs_to_disable |= !crtc_state->has_psr;
> -		needs_to_disable |= !crtc_state->active_planes;
> -		needs_to_disable |= crtc_state->has_psr2 != psr->psr2_enabled;
> +			mutex_lock(&psr->lock);
>  
> -		if (psr->enabled && needs_to_disable)
> -			intel_psr_disable_locked(intel_dp);
> +			/*
> +			 * Reasons to disable:
> +			 * - PSR disabled in new state
> +			 * - All planes will go inactive
> +			 * - Changing between PSR versions
> +			 * - Encoder isn't present in new mask
> +			 */
> +			needs_to_disable |= intel_crtc_needs_modeset(new_crtc_state);
> +			needs_to_disable |= !new_crtc_state->has_psr;
> +			needs_to_disable |= !new_crtc_state->active_planes;
> +			needs_to_disable |= new_crtc_state->has_psr2 != psr->psr2_enabled;
> +			needs_to_disable |= !(new_crtc_state->uapi.encoder_mask &
> +					      drm_encoder_mask(&(encoder)->base));
>  
> -		mutex_unlock(&psr->lock);
> +			if (psr->enabled && needs_to_disable)
> +				intel_psr_disable_locked(intel_dp);
> +
> +			mutex_unlock(&psr->lock);
> +		}
>  	}
>  }
>

Souza, Jose July 13, 2022, 9:04 p.m. UTC | #3

On Wed, 2022-07-13 at 20:58 +0000, Souza, Jose wrote:
> On Mon, 2022-07-11 at 14:17 +0300, Jouni Högander wrote:
> > Currently PSR is left enabled when all planes are disabled if there
> > is no attached encoder in new state. This seems to be causing FIFO
> > underruns.
> 
> What is the case were there is no attached encoder and active_planes > 0?

Can you point to some test?
I believe that a pipe to be enabled needs to have a enconder/port attached, otherwise it will be disabled. 

> 
> > 
> > Fix this by checking if encoder exists in new crtc state and disable
> > PSR if it doesn't.
> > 
> > v2: Unify disable logic with existing
> > 
> > Cc: Mika Kahola <[e-mail]>
> > Reported-by: Stanislav Lisovskiy <[e-mail]>
> > Signed-off-by: Jouni Högander <[e-mail]>
> > Signed-off-by: Stanislav Lisovskiy <[e-mail]>
> > ---
> >  drivers/gpu/drm/i915/display/intel_psr.c | 53 ++++++++++++++----------
> >  1 file changed, 31 insertions(+), 22 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> > index e6a870641cd2..90599dd1cb1b 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -1863,36 +1863,45 @@ void intel_psr_pre_plane_update(struct intel_atomic_state *state,
> >  				struct intel_crtc *crtc)
> >  {
> >  	struct drm_i915_private *i915 = to_i915(state->base.dev);
> > -	const struct intel_crtc_state *crtc_state =
> > -		intel_atomic_get_new_crtc_state(state, crtc);
> > -	struct intel_encoder *encoder;
> > +	struct intel_crtc_state *new_crtc_state, *old_crtc_state;
> > +	int i;
> >  
> >  	if (!HAS_PSR(i915))
> >  		return;
> >  
> > -	for_each_intel_encoder_mask_with_psr(state->base.dev, encoder,
> > -					     crtc_state->uapi.encoder_mask) {
> > -		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> > -		struct intel_psr *psr = &intel_dp->psr;
> > -		bool needs_to_disable = false;
> > +	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
> > +					    new_crtc_state, i) {
> > +		struct intel_encoder *encoder;
> > +		u32 old_new_encoder_mask = old_crtc_state->uapi.encoder_mask |
> > +			new_crtc_state->uapi.encoder_mask;
> >  
> > -		mutex_lock(&psr->lock);
> > +		for_each_intel_encoder_mask_with_psr(state->base.dev, encoder,
> > +						     old_new_encoder_mask) {
> > +			struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> > +			struct intel_psr *psr = &intel_dp->psr;
> > +			bool needs_to_disable = false;
> >  
> > -		/*
> > -		 * Reasons to disable:
> > -		 * - PSR disabled in new state
> > -		 * - All planes will go inactive
> > -		 * - Changing between PSR versions
> > -		 */
> > -		needs_to_disable |= intel_crtc_needs_modeset(crtc_state);
> > -		needs_to_disable |= !crtc_state->has_psr;
> > -		needs_to_disable |= !crtc_state->active_planes;
> > -		needs_to_disable |= crtc_state->has_psr2 != psr->psr2_enabled;
> > +			mutex_lock(&psr->lock);
> >  
> > -		if (psr->enabled && needs_to_disable)
> > -			intel_psr_disable_locked(intel_dp);
> > +			/*
> > +			 * Reasons to disable:
> > +			 * - PSR disabled in new state
> > +			 * - All planes will go inactive
> > +			 * - Changing between PSR versions
> > +			 * - Encoder isn't present in new mask
> > +			 */
> > +			needs_to_disable |= intel_crtc_needs_modeset(new_crtc_state);
> > +			needs_to_disable |= !new_crtc_state->has_psr;
> > +			needs_to_disable |= !new_crtc_state->active_planes;
> > +			needs_to_disable |= new_crtc_state->has_psr2 != psr->psr2_enabled;
> > +			needs_to_disable |= !(new_crtc_state->uapi.encoder_mask &
> > +					      drm_encoder_mask(&(encoder)->base));
> >  
> > -		mutex_unlock(&psr->lock);
> > +			if (psr->enabled && needs_to_disable)
> > +				intel_psr_disable_locked(intel_dp);
> > +
> > +			mutex_unlock(&psr->lock);
> > +		}
> >  	}
> >  }
> >  
>

Hogander, Jouni July 14, 2022, 7:22 a.m. UTC | #4

On Wed, 2022-07-13 at 21:04 +0000, Souza, Jose wrote:
> On Wed, 2022-07-13 at 20:58 +0000, Souza, Jose wrote:
> > On Mon, 2022-07-11 at 14:17 +0300, Jouni Högander wrote:
> > > Currently PSR is left enabled when all planes are disabled if
> > > there
> > > is no attached encoder in new state. This seems to be causing
> > > FIFO
> > > underruns.
> > 
> > What is the case were there is no attached encoder and
> > active_planes > 0?
> 
> Can you point to some test?

You just need to run kms_busy@basic and this triggers when dynamic
subtest modeset is run.

> I believe that a pipe to be enabled needs to have a enconder/port
> attached, otherwise it will be disabled.

To my understanding pipe actually gets eventually disabled in this
testcase as well. Before pipe is disabled we have state where planes
are all disabled, but PSR is kept enabled. This was triggering FIFO
underrun.

> 
> > > Fix this by checking if encoder exists in new crtc state and
> > > disable
> > > PSR if it doesn't.
> > > 
> > > v2: Unify disable logic with existing
> > > 
> > > Cc: Mika Kahola <[e-mail]>
> > > Reported-by: Stanislav Lisovskiy <[e-mail]>
> > > Signed-off-by: Jouni Högander <[e-mail]>
> > > Signed-off-by: Stanislav Lisovskiy <[e-mail]
> > > >
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_psr.c | 53 ++++++++++++++--
> > > --------
> > >  1 file changed, 31 insertions(+), 22 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > > b/drivers/gpu/drm/i915/display/intel_psr.c
> > > index e6a870641cd2..90599dd1cb1b 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > @@ -1863,36 +1863,45 @@ void intel_psr_pre_plane_update(struct
> > > intel_atomic_state *state,
> > >  struct intel_crtc *crtc)
> > >  {
> > >  struct drm_i915_private *i915 = to_i915(state->base.dev);
> > > -const struct intel_crtc_state *crtc_state =
> > > -intel_atomic_get_new_crtc_state(state, crtc);
> > > -struct intel_encoder *encoder;
> > > +struct intel_crtc_state *new_crtc_state, *old_crtc_state;
> > > +int i;
> > > 
> > >  if (!HAS_PSR(i915))
> > >  return;
> > > 
> > > -for_each_intel_encoder_mask_with_psr(state->base.dev, encoder,
> > > -     crtc_state->uapi.encoder_mask) {
> > > -struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> > > -struct intel_psr *psr = &intel_dp->psr;
> > > -bool needs_to_disable = false;
> > > +for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
> > > +    new_crtc_state, i) {
> > > +struct intel_encoder *encoder;
> > > +u32 old_new_encoder_mask = old_crtc_state->uapi.encoder_mask |
> > > +new_crtc_state->uapi.encoder_mask;
> > > 
> > > -mutex_lock(&psr->lock);
> > > +for_each_intel_encoder_mask_with_psr(state->base.dev, encoder,
> > > +     old_new_encoder_mask) {
> > > +struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> > > +struct intel_psr *psr = &intel_dp->psr;
> > > +bool needs_to_disable = false;
> > > 
> > > -/*
> > > - * Reasons to disable:
> > > - * - PSR disabled in new state
> > > - * - All planes will go inactive
> > > - * - Changing between PSR versions
> > > - */
> > > -needs_to_disable |= intel_crtc_needs_modeset(crtc_state);
> > > -needs_to_disable |= !crtc_state->has_psr;
> > > -needs_to_disable |= !crtc_state->active_planes;
> > > -needs_to_disable |= crtc_state->has_psr2 != psr->psr2_enabled;
> > > +mutex_lock(&psr->lock);
> > > 
> > > -if (psr->enabled && needs_to_disable)
> > > -intel_psr_disable_locked(intel_dp);
> > > +/*
> > > + * Reasons to disable:
> > > + * - PSR disabled in new state
> > > + * - All planes will go inactive
> > > + * - Changing between PSR versions
> > > + * - Encoder isn't present in new mask
> > > + */
> > > +needs_to_disable |= intel_crtc_needs_modeset(new_crtc_state);
> > > +needs_to_disable |= !new_crtc_state->has_psr;
> > > +needs_to_disable |= !new_crtc_state->active_planes;
> > > +needs_to_disable |= new_crtc_state->has_psr2 != psr-
> > > >psr2_enabled;
> > > +needs_to_disable |= !(new_crtc_state->uapi.encoder_mask &
> > > +      drm_encoder_mask(&(encoder)->base));
> > > 
> > > -mutex_unlock(&psr->lock);
> > > +if (psr->enabled && needs_to_disable)
> > > +intel_psr_disable_locked(intel_dp);
> > > +
> > > +mutex_unlock(&psr->lock);
> > > +}
> > >  }
> > >  }
> > >

Souza, Jose July 14, 2022, 3:11 p.m. UTC | #5

On Thu, 2022-07-14 at 07:22 +0000, Hogander, Jouni wrote:
> On Wed, 2022-07-13 at 21:04 +0000, Souza, Jose wrote:
> > On Wed, 2022-07-13 at 20:58 +0000, Souza, Jose wrote:
> > > On Mon, 2022-07-11 at 14:17 +0300, Jouni Högander wrote:
> > > > Currently PSR is left enabled when all planes are disabled if
> > > > there
> > > > is no attached encoder in new state. This seems to be causing
> > > > FIFO
> > > > underruns.
> > > 
> > > What is the case were there is no attached encoder and
> > > active_planes > 0?
> > 
> > Can you point to some test?
> 
> You just need to run kms_busy@basic and this triggers when dynamic
> subtest modeset is run.

Thanks, were able to reproduce the issue.

> 
> > I believe that a pipe to be enabled needs to have a enconder/port
> > attached, otherwise it will be disabled.
> 
> To my understanding pipe actually gets eventually disabled in this
> testcase as well. Before pipe is disabled we have state where planes
> are all disabled, but PSR is kept enabled. This was triggering FIFO
> underrun.

Yep that was happening but this is not the proper solution.
for_each_oldnew_intel_crtc_in_state() will iterate over all CRTCs in the drm_i915_private, not only the CRTC passed as parameter.

As this was already merged I have sent the fix: https://patchwork.freedesktop.org/series/106357/

> 
> > 
> > > > Fix this by checking if encoder exists in new crtc state and
> > > > disable
> > > > PSR if it doesn't.
> > > > 
> > > > v2: Unify disable logic with existing
> > > > 
> > > > Cc: Mika Kahola <[e-mail]>
> > > > Reported-by: Stanislav Lisovskiy <[e-mail]>
> > > > Signed-off-by: Jouni Högander <[e-mail]>
> > > > Signed-off-by: Stanislav Lisovskiy <[e-mail]
> > > > > 
> > > > ---
> > > >  drivers/gpu/drm/i915/display/intel_psr.c | 53 ++++++++++++++--
> > > > --------
> > > >  1 file changed, 31 insertions(+), 22 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > index e6a870641cd2..90599dd1cb1b 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > @@ -1863,36 +1863,45 @@ void intel_psr_pre_plane_update(struct
> > > > intel_atomic_state *state,
> > > >  struct intel_crtc *crtc)
> > > >  {
> > > >  struct drm_i915_private *i915 = to_i915(state->base.dev);
> > > > -const struct intel_crtc_state *crtc_state =
> > > > -intel_atomic_get_new_crtc_state(state, crtc);
> > > > -struct intel_encoder *encoder;
> > > > +struct intel_crtc_state *new_crtc_state, *old_crtc_state;
> > > > +int i;
> > > > 
> > > >  if (!HAS_PSR(i915))
> > > >  return;
> > > > 
> > > > -for_each_intel_encoder_mask_with_psr(state->base.dev, encoder,
> > > > -     crtc_state->uapi.encoder_mask) {
> > > > -struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> > > > -struct intel_psr *psr = &intel_dp->psr;
> > > > -bool needs_to_disable = false;
> > > > +for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
> > > > +    new_crtc_state, i) {
> > > > +struct intel_encoder *encoder;
> > > > +u32 old_new_encoder_mask = old_crtc_state->uapi.encoder_mask |
> > > > +new_crtc_state->uapi.encoder_mask;
> > > > 
> > > > -mutex_lock(&psr->lock);
> > > > +for_each_intel_encoder_mask_with_psr(state->base.dev, encoder,
> > > > +     old_new_encoder_mask) {
> > > > +struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> > > > +struct intel_psr *psr = &intel_dp->psr;
> > > > +bool needs_to_disable = false;
> > > > 
> > > > -/*
> > > > - * Reasons to disable:
> > > > - * - PSR disabled in new state
> > > > - * - All planes will go inactive
> > > > - * - Changing between PSR versions
> > > > - */
> > > > -needs_to_disable |= intel_crtc_needs_modeset(crtc_state);
> > > > -needs_to_disable |= !crtc_state->has_psr;
> > > > -needs_to_disable |= !crtc_state->active_planes;
> > > > -needs_to_disable |= crtc_state->has_psr2 != psr->psr2_enabled;
> > > > +mutex_lock(&psr->lock);
> > > > 
> > > > -if (psr->enabled && needs_to_disable)
> > > > -intel_psr_disable_locked(intel_dp);
> > > > +/*
> > > > + * Reasons to disable:
> > > > + * - PSR disabled in new state
> > > > + * - All planes will go inactive
> > > > + * - Changing between PSR versions
> > > > + * - Encoder isn't present in new mask
> > > > + */
> > > > +needs_to_disable |= intel_crtc_needs_modeset(new_crtc_state);
> > > > +needs_to_disable |= !new_crtc_state->has_psr;
> > > > +needs_to_disable |= !new_crtc_state->active_planes;
> > > > +needs_to_disable |= new_crtc_state->has_psr2 != psr-
> > > > > psr2_enabled;
> > > > +needs_to_disable |= !(new_crtc_state->uapi.encoder_mask &
> > > > +      drm_encoder_mask(&(encoder)->base));
> > > > 
> > > > -mutex_unlock(&psr->lock);
> > > > +if (psr->enabled && needs_to_disable)
> > > > +intel_psr_disable_locked(intel_dp);
> > > > +
> > > > +mutex_unlock(&psr->lock);
> > > > +}
> > > >  }
> > > >  }
> > > > 
>

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index e6a870641cd2..90599dd1cb1b 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -1863,36 +1863,45 @@  void intel_psr_pre_plane_update(struct intel_atomic_state *state,
 				struct intel_crtc *crtc)
 {
 	struct drm_i915_private *i915 = to_i915(state->base.dev);
-	const struct intel_crtc_state *crtc_state =
-		intel_atomic_get_new_crtc_state(state, crtc);
-	struct intel_encoder *encoder;
+	struct intel_crtc_state *new_crtc_state, *old_crtc_state;
+	int i;
 
 	if (!HAS_PSR(i915))
 		return;
 
-	for_each_intel_encoder_mask_with_psr(state->base.dev, encoder,
-					     crtc_state->uapi.encoder_mask) {
-		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
-		struct intel_psr *psr = &intel_dp->psr;
-		bool needs_to_disable = false;
+	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
+					    new_crtc_state, i) {
+		struct intel_encoder *encoder;
+		u32 old_new_encoder_mask = old_crtc_state->uapi.encoder_mask |
+			new_crtc_state->uapi.encoder_mask;
 
-		mutex_lock(&psr->lock);
+		for_each_intel_encoder_mask_with_psr(state->base.dev, encoder,
+						     old_new_encoder_mask) {
+			struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
+			struct intel_psr *psr = &intel_dp->psr;
+			bool needs_to_disable = false;
 
-		/*
-		 * Reasons to disable:
-		 * - PSR disabled in new state
-		 * - All planes will go inactive
-		 * - Changing between PSR versions
-		 */
-		needs_to_disable |= intel_crtc_needs_modeset(crtc_state);
-		needs_to_disable |= !crtc_state->has_psr;
-		needs_to_disable |= !crtc_state->active_planes;
-		needs_to_disable |= crtc_state->has_psr2 != psr->psr2_enabled;
+			mutex_lock(&psr->lock);
 
-		if (psr->enabled && needs_to_disable)
-			intel_psr_disable_locked(intel_dp);
+			/*
+			 * Reasons to disable:
+			 * - PSR disabled in new state
+			 * - All planes will go inactive
+			 * - Changing between PSR versions
+			 * - Encoder isn't present in new mask
+			 */
+			needs_to_disable |= intel_crtc_needs_modeset(new_crtc_state);
+			needs_to_disable |= !new_crtc_state->has_psr;
+			needs_to_disable |= !new_crtc_state->active_planes;
+			needs_to_disable |= new_crtc_state->has_psr2 != psr->psr2_enabled;
+			needs_to_disable |= !(new_crtc_state->uapi.encoder_mask &
+					      drm_encoder_mask(&(encoder)->base));
 
-		mutex_unlock(&psr->lock);
+			if (psr->enabled && needs_to_disable)
+				intel_psr_disable_locked(intel_dp);
+
+			mutex_unlock(&psr->lock);
+		}
 	}
 }