On Fri, Jan 24, 2020 at 07:23:01PM +0200, Stanislav Lisovskiy wrote: > Despite that during hw readout we seem to have scalers assigned > to pipes, then call atomic_setup_scalers, at the commit stage in > skl_update_scaler there is a check, that if we have fb src and > dest of same size, we stage freeing of that scaler. > > However we don't update pfit.enabled flag then, which makes > the state inconsistent, which in turn triggers a WARN_ON > in skl_pfit_enable, because we have pfit enabled, > but no assigned scaler. And the reason for not having updates pfit.enabled is that the the modeset was forced by a cdclk change and thus the full state recomputation never happened and we're left with the inherited pfit.enabled. > > To me this looks weird that we kind of do the decision > to use or not use the scaler at skl_update_scaler stage > but not in intel_atomic_setup_scalers, moreover > not updating the whole state consistently. > > This fix is to not free the scaler if we have pfit.enabled > flag set, so that the state is now consistent > and the warnings are gone. > > v2: - Put pfit.enable check into crtc specific place > (Ville Syrjälä) > > Bugzilla: https://gitlab.freedesktop.org/drm/intel/issues/577 Closes: ... > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@xxxxxxxxx> > --- > drivers/gpu/drm/i915/display/intel_display.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index 5768cfcf71c4..cd242d91a924 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -6037,7 +6037,8 @@ int skl_update_scaler_crtc(struct intel_crtc_state *state) > const struct drm_display_mode *adjusted_mode = &state->hw.adjusted_mode; > bool need_scaler = false; > > - if (state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420) > + if (state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 || > + state->pch_pfit.enabled) Hmm, no hw.enable check here for the existing case either. Shouldn't matter now that I made the crtc disable clear the entire state as well. Oh, it was handled via that odd force_detach stuff it seems. Whatever. Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > need_scaler = true; > > return skl_update_scaler(state, !state->hw.active, SKL_CRTC_INDEX, > -- > 2.24.1.485.gad05a3d8e5 -- Ville Syrjälä Intel _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
