> -----Original Message-----
> From: Intel-gfx <intel-gfx-boun...@lists.freedesktop.org> On Behalf Of Imre
> Deak
> Sent: Monday, November 15, 2021 8:11 PM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [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.syrj...@linux.intel.com>
This bug fix seems legit to me.
Reviewed-by: Mika Kahola <mika.kah...@intel.com>
> Signed-off-by: Imre Deak <imre.d...@intel.com>
> ---
> 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
