On Tue, 18 Feb 2025, Imre Deak <imre.d...@intel.com> wrote:
> During disabling the transcoder in DP 128b/132b mode (both in case of an
> MST master transcoder and in case of SST) the transcoder function must
> be first disabled without changing any other field in the register (in
> particular leaving the DDI port and mode select fields unchanged) and
> clearing the DDI port and mode select fields separately, later during
> the disabling sequences. Fix the sequence accordingly.
>
> Bspec: 54128, 65448, 68849
> Cc: Jani Nikula <jani.nik...@intel.com>
> Fixes: 79a6734cd56e ("drm/i915/ddi: disable trancoder port select for
> 128b/132b SST")
> Signed-off-by: Imre Deak <imre.d...@intel.com>
Reviewed-by: Jani Nikula <jani.nik...@intel.com>
Looks like I've intentionally done it this way. I think I've stumbled on
the bspec text "DP v2.0/128b Primary" and the "primary" has convinced me
this means MST. In most cases one should just read all things MST as
being true for MTP, regardless of 8b/10b or 128b/132b, no matter what
the text actually says. :p
Thanks for debugging and fixing these!
BR,
Jani.
> ---
> drivers/gpu/drm/i915/display/intel_ddi.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 5082f38b0a02e..7937f4de66cb4 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -681,7 +681,6 @@ void intel_ddi_disable_transcoder_func(const struct
> intel_crtc_state *crtc_state
> struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> - bool is_mst = intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST);
> u32 ctl;
>
> if (DISPLAY_VER(dev_priv) >= 11)
> @@ -701,8 +700,7 @@ void intel_ddi_disable_transcoder_func(const struct
> intel_crtc_state *crtc_state
> TRANS_DDI_PORT_SYNC_MASTER_SELECT_MASK);
>
> if (DISPLAY_VER(dev_priv) >= 12) {
> - if (!intel_dp_mst_is_master_trans(crtc_state) ||
> - (!is_mst && intel_dp_is_uhbr(crtc_state))) {
> + if (!intel_dp_mst_is_master_trans(crtc_state)) {
> ctl &= ~(TGL_TRANS_DDI_PORT_MASK |
> TRANS_DDI_MODE_SELECT_MASK);
> }
> @@ -3138,7 +3136,7 @@ static void intel_ddi_post_disable_dp(struct
> intel_atomic_state *state,
> intel_dp_set_power(intel_dp, DP_SET_POWER_D3);
>
> if (DISPLAY_VER(dev_priv) >= 12) {
> - if (is_mst) {
> + if (is_mst || intel_dp_is_uhbr(old_crtc_state)) {
> enum transcoder cpu_transcoder =
> old_crtc_state->cpu_transcoder;
>
> intel_de_rmw(dev_priv,
--
Jani Nikula, Intel
