[Intel-gfx] [PATCH 4/4] drm/i915/display: Set TRANS_DDI_MODE_SELECT to default value when disabling TRANS_DDI

[Ville Syrjälä]

On Thu, Jan 16, 2020 at 05:58:37PM -0800, José Roberto de Souza wrote:
> TGL timeouts when disabling MST transcoder and fifo underruns over MST
> transcoders are fixed when setting TRANS_DDI_MODE_SELECT to 0(HDMI
> mode) during the disable sequence.
> 
> Although BSpec disable sequence don't require this step it is a
> harmless change and it is also done by Windows driver.
> Anyhow HW team was notified about that but it can take some time to
> documentation to be updated.
> 
> A case that always lead to those issues is:
> - do a modeset enabling pipe A and pipe B in the same MST stream
> leaving A as master
> - disable pipe A, promote B as master doing a full modeset in A
> - enable pipe A, changing the master transcoder back to A(doing a
> full modeset in B)
> - Pow: underruns and timeouts
> 
> The transcoders involved will only work again when complete disabled
> and their power wells turned off causing a reset in their registers.
> 
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Cc: Matt Roper <matthew.d.roper at intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 32ea3c7e8b62..82e90f271974 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -1997,6 +1997,7 @@ void intel_ddi_disable_transcoder_func(const struct intel_crtc_state *crtc_state
>  
>  	val = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
>  	val &= ~TRANS_DDI_FUNC_ENABLE;
> +	val &= ~TRANS_DDI_MODE_SELECT_MASK;

Feels a bit early since IIRC we still leave a bunch of other stuff
enabled/selected here. In fact we don't seem to be clearing the DDI select
anywhere at all? That one I would be more suspicious of than the mode.
But maybe we should just clear both somewhere? I would suggest it should
be when we clear the port select finally.

>  
>  	if (INTEL_GEN(dev_priv) >= 12) {
>  		if (!intel_dp_mst_is_master_trans(crtc_state))
> -- 
> 2.25.0

-- 
Ville Syrjälä
Intel