On Mon, May 20, 2024 at 09:58:04PM +0300, Imre Deak wrote:
> Instead of direct calls to the link train functions, retrain the link
> via a commit modeset. The direct call means that the output port will be
> disabled/re-enabled while the rest of the pipeline (transcoder) is
> active, which doesn't seem to work on MST at least. It leads to
> underruns and black screen, presumedly because the transcoder is not
> disabled/re-enabled along the port.
>
> Leave switching to a commit modeset on SST for a later patchset, as that
> seems to work ok currently (though better to using a commit there too,
> due to the suppressed underruns).
>
> Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_dp.c | 25 +++++++++++++++++++------
> 1 file changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 81e620dd33bb7..120f7b420807b 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -5147,6 +5147,7 @@ int intel_dp_retrain_link(struct intel_encoder *encoder,
> struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> struct intel_crtc *crtc;
> u8 pipe_mask;
> + bool mst_output = false;
nit: maybe move that up one line to maintain a bit more of a steady slope
> int ret;
>
> if (!intel_dp_is_connected(intel_dp))
> @@ -5177,6 +5178,11 @@ int intel_dp_retrain_link(struct intel_encoder *encoder,
> const struct intel_crtc_state *crtc_state =
> to_intel_crtc_state(crtc->base.state);
>
> + if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST)) {
> + mst_output = true;
> + break;
> + }
I was pondering if we need a bit more care to make sure all
the pipes agree, but I suppose if that wasn't the case
check_digital_port_conflicts() would have a failed at its
job. So this seems fine.
Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> +
> /* Suppress underruns caused by re-training */
> intel_set_cpu_fifo_underrun_reporting(dev_priv, crtc->pipe, false);
> if (crtc_state->has_pch_encoder)
> @@ -5184,16 +5190,23 @@ int intel_dp_retrain_link(struct intel_encoder *encoder,
> intel_crtc_pch_transcoder(crtc), false);
> }
>
> + /* TODO: use a modeset for SST as well. */
> + if (mst_output) {
> + ret = intel_modeset_commit_pipes(dev_priv, pipe_mask, ctx);
> +
> + if (ret && ret != -EDEADLK)
> + drm_dbg_kms(&dev_priv->drm,
> + "[ENCODER:%d:%s] link retraining failed: %pe\n",
> + encoder->base.base.id, encoder->base.name,
> + ERR_PTR(ret));
> +
> + return ret;
> + }
> +
> for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, crtc, pipe_mask) {
> const struct intel_crtc_state *crtc_state =
> to_intel_crtc_state(crtc->base.state);
>
> - /* retrain on the MST master transcoder */
> - if (DISPLAY_VER(dev_priv) >= 12 &&
> - intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST) &&
> - !intel_dp_mst_is_master_trans(crtc_state))
> - continue;
> -
> intel_dp_check_frl_training(intel_dp);
> intel_dp_pcon_dsc_configure(intel_dp, crtc_state);
> intel_dp_start_link_train(intel_dp, crtc_state);
> --
> 2.43.3
--
Ville Syrjälä
Intel
