Re: [PATCH 2/2] drm/i915/dp: Fix disabling the transcoder function in 128b/132b mode

[Jani Nikula <jani.nikula@intel.com>]

On Tue, 18 Feb 2025, Imre Deak <imre.deak@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.nikula@intel.com>
> Fixes: 79a6734cd56e ("drm/i915/ddi: disable trancoder port select for 128b/132b SST")
> Signed-off-by: Imre Deak <imre.deak@intel.com>

Reviewed-by: Jani Nikula <jani.nikula@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