Re: [PATCH 2/2] drm/i915/icl: Fix PLL mapping sanitization for DP ports

[Clint Taylor <clinton.a.taylor@intel.com>]

On 11/07/2018 12:08 PM, Imre Deak wrote:
> We shouldn't consider an encoder inactive if it doesn't have a CRTC
> linked, but has virtual MST encoders with a crtc linked. Fix this.
>
> Also we should not sanitize the mapping for MST encoders, as it's always
> their primary encoder (which could be even in SST mode) whose active
> state determines if we need the clock being enabled for the
> corresponding physical port. Fix this too.
>
> This fixes at least an existing breakage where we incorrectly disabled
> the clock for an active DP encoder when sanitizing its MST virtual
> encoders. Not sure if there are BIOSes that enable an output in MST
> mode, but our HW readout is mostly missing for it anyway, so just warn
> for that case.
>
> Fixes: 70332ac539c5 ("drm/i915/icl+: Sanitize port to PLL mapping")
> Cc: Antonio Argenziano <antonio.argenziano@intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Reported-by: Antonio Argenziano <antonio.argenziano@intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_ddi.c | 27 +++++++++++++++++++++++++--
>   1 file changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index abc51693eec9..4913bbdac843 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2853,9 +2853,32 @@ void icl_unmap_plls_to_ports(struct drm_crtc *crtc,
>   void icl_sanitize_encoder_pll_mapping(struct intel_encoder *encoder)
>   {
>   	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> -	u32 val = I915_READ(DPCLKA_CFGCR0_ICL);
> +	u32 val;
>   	enum port port = encoder->port;
> -	bool clk_enabled = !(val & icl_dpclka_cfgcr0_clk_off(dev_priv, port));
> +	bool clk_enabled;
> +
> +	/*
> +	 * In case of DP MST, we sanitize the primary encoder only, not the
> +	 * virtual ones.
> +	 */
> +	if (encoder->type == INTEL_OUTPUT_DP_MST)
> +		return;
> +
> +	val = I915_READ(DPCLKA_CFGCR0_ICL);
> +	clk_enabled = !(val & icl_dpclka_cfgcr0_clk_off(dev_priv, port));
> +
> +	if (!encoder->base.crtc && intel_encoder_is_dp(encoder)) {
> +		u8 pipe_mask;
> +		bool is_mst;
> +
> +		intel_ddi_get_encoder_pipes(encoder, &pipe_mask, &is_mst);
> +		/*
> +		 * In the unlikely case that BIOS enables DP in MST mode, just
> +		 * warn since our MST HW readout is incomplete.
> +		 */
> +		if (WARN_ON(is_mst))
> +			return;
> +	}
>   
>   	if (clk_enabled == !!encoder->base.crtc)
>   		return;
Fixes the mDP lock up issue.

Reviewed-by: Clint Taylor <Clinton.A.Taylor@intel.com>
Tested-by: Clint Taylor <Clinton.A.Taylor@intel.com>

-Clint

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx