> -----Original Message----- > From: Intel-gfx <intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Imre > Deak > Sent: Monday, November 15, 2021 8:11 PM > To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx > Subject: [PATCH v2] drm/i915: Fix fastsets on TypeC ports following a > non-blocking modeset > > After a non-blocking modeset on a TypeC port's CRTC - possibly blocked later in > drm_atomic_helper_wait_for_dependencies() - a fastset on the same CRTC may > copy the state of CRTC before this gets updated to reflect the up-to-date DP-alt > vs. TBT-alt TypeC mode DPLL used for the CRTC. In this case after the first (non- > blocking) commit completes enabling the DPLL required for the up-to-date > TypeC mode the following fastset will update the CRTC state pointing to the > wrong DPLL. A subsequent disabling modeset will try to disable the wrong PLL, > triggering a state checker WARN (and leaving the DPLL which is actually used > active for good). > > Fix the above race by copying the DPLL state for fastset CRTCs from the old > CRTC state at the point where it's guaranteed to be up-to-date already. This > could be handled in the encoder's update_prepare() hook as well, but that's a > bigger change, which is better done as a follow-up. > > v2: Copy dpll_hw_state as well. (Ville) > > Testcase: igt/kms_busy/extended-modeset-hang-newfb-with-reset > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/4308 > Cc: Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx> This bug fix seems legit to me. Reviewed-by: Mika Kahola <mika.kahola@xxxxxxxxx> > Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx> > --- > drivers/gpu/drm/i915/display/intel_display.c | 31 ++++++++++++++++---- > 1 file changed, 26 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > b/drivers/gpu/drm/i915/display/intel_display.c > index 0ceee8ac66717..f3c9208a30b16 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -1572,10 +1572,30 @@ intel_connector_primary_encoder(struct > intel_connector *connector) > > static void intel_encoders_update_prepare(struct intel_atomic_state *state) { > + struct drm_i915_private *i915 = to_i915(state->base.dev); > + struct intel_crtc_state *new_crtc_state, *old_crtc_state; > + struct intel_crtc *crtc; > struct drm_connector_state *new_conn_state; > struct drm_connector *connector; > int i; > > + /* > + * Make sure the DPLL state is up-to-date for fastset TypeC ports after > non-blocking commits. > + * TODO: Update the DPLL state for all cases in the encoder- > >update_prepare() hook. > + */ > + if (i915->dpll.mgr) { > + for_each_oldnew_intel_crtc_in_state(state, crtc, > old_crtc_state, new_crtc_state, i) { > + if (intel_crtc_needs_modeset(new_crtc_state)) > + continue; > + > + new_crtc_state->shared_dpll = old_crtc_state- > >shared_dpll; > + new_crtc_state->dpll_hw_state = old_crtc_state- > >dpll_hw_state; > + } > + } > + > + if (!state->modeset) > + return; > + > for_each_new_connector_in_state(&state->base, connector, > new_conn_state, > i) { > struct intel_connector *intel_connector; @@ -1602,6 +1622,9 > @@ static void intel_encoders_update_complete(struct intel_atomic_state > *state) > struct drm_connector *connector; > int i; > > + if (!state->modeset) > + return; > + > for_each_new_connector_in_state(&state->base, connector, > new_conn_state, > i) { > struct intel_connector *intel_connector; @@ -8670,8 +8693,7 > @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state) > } > } > > - if (state->modeset) > - intel_encoders_update_prepare(state); > + intel_encoders_update_prepare(state); > > intel_dbuf_pre_plane_update(state); > > @@ -8683,11 +8705,10 @@ static void intel_atomic_commit_tail(struct > intel_atomic_state *state) > /* Now enable the clocks, plane, pipe, and connectors that we set up. > */ > dev_priv->display->commit_modeset_enables(state); > > - if (state->modeset) { > - intel_encoders_update_complete(state); > + intel_encoders_update_complete(state); > > + if (state->modeset) > intel_set_cdclk_post_plane_update(state); > - } > > intel_wait_for_vblank_workers(state); > > -- > 2.27.0
