| Message ID | 20220714150755.154985-1-jose.souza@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] Revert "drm/i915/display: Ensure PSR gets disabled if no encoders in new state" | expand |
On Thu, 2022-07-14 at 08:07 -0700, José Roberto de Souza wrote: > This patches fixes a issue but not in the right way as > for_each_oldnew_intel_crtc_in_state() will interate over all CRTCs > not only the crtc passed as parameter, also this two for_each loops > are not necessary and only make code harder to understand. Reviewed-by: Jouni Högander <jouni.hogander@intel.com> > > Proper fix will be discussed in the next patch. > > This reverts commit 75f664903d8672897333b86bb450335ec6486ad5. > > Cc: Jouni Högander <jouni.hogander@intel.com> > Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > --- > drivers/gpu/drm/i915/display/intel_psr.c | 53 ++++++++++---------- > ---- > 1 file changed, 22 insertions(+), 31 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c > b/drivers/gpu/drm/i915/display/intel_psr.c > index 90599dd1cb1b5..e6a870641cd25 100644 > --- a/drivers/gpu/drm/i915/display/intel_psr.c > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > @@ -1863,45 +1863,36 @@ 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); > - struct intel_crtc_state *new_crtc_state, *old_crtc_state; > - int i; > + const struct intel_crtc_state *crtc_state = > + intel_atomic_get_new_crtc_state(state, crtc); > + struct intel_encoder *encoder; > > if (!HAS_PSR(i915)) > return; > > - 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; > - > - for_each_intel_encoder_mask_with_psr(state->base.dev, > encoder, > - old_new_encoder_ma > sk) { > - struct intel_dp *intel_dp = > enc_to_intel_dp(encoder); > - struct intel_psr *psr = &intel_dp->psr; > - bool needs_to_disable = false; > + 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; > > - mutex_lock(&psr->lock); > + mutex_lock(&psr->lock); > > - /* > - * 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(&(encode > r)->base)); > + /* > + * 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; > > - if (psr->enabled && needs_to_disable) > - intel_psr_disable_locked(intel_dp); > + if (psr->enabled && needs_to_disable) > + intel_psr_disable_locked(intel_dp); > > - mutex_unlock(&psr->lock); > - } > + mutex_unlock(&psr->lock); > } > } >
diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c index 90599dd1cb1b5..e6a870641cd25 100644 --- a/drivers/gpu/drm/i915/display/intel_psr.c +++ b/drivers/gpu/drm/i915/display/intel_psr.c @@ -1863,45 +1863,36 @@ 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); - struct intel_crtc_state *new_crtc_state, *old_crtc_state; - int i; + const struct intel_crtc_state *crtc_state = + intel_atomic_get_new_crtc_state(state, crtc); + struct intel_encoder *encoder; if (!HAS_PSR(i915)) return; - 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; - - 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; + 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; - mutex_lock(&psr->lock); + mutex_lock(&psr->lock); - /* - * 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)); + /* + * 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; - if (psr->enabled && needs_to_disable) - intel_psr_disable_locked(intel_dp); + if (psr->enabled && needs_to_disable) + intel_psr_disable_locked(intel_dp); - mutex_unlock(&psr->lock); - } + mutex_unlock(&psr->lock); } }
This patches fixes a issue but not in the right way as for_each_oldnew_intel_crtc_in_state() will interate over all CRTCs not only the crtc passed as parameter, also this two for_each loops are not necessary and only make code harder to understand. Proper fix will be discussed in the next patch. This reverts commit 75f664903d8672897333b86bb450335ec6486ad5. Cc: Jouni Högander <jouni.hogander@intel.com> Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> Signed-off-by: José Roberto de Souza <jose.souza@intel.com> --- drivers/gpu/drm/i915/display/intel_psr.c | 53 ++++++++++-------------- 1 file changed, 22 insertions(+), 31 deletions(-)