[Intel-gfx] [PATCH] drm/i915: Provide more clues as to why MST is/is not used

[Dhinakaran Pandiyan]

On Wednesday, October 3, 2018 11:42:10 AM PDT Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> Always print out the information whether the port and sink can each
> do MST. And let's include the modparam in the debug output as well.
> Makes life a little less confusing when you don't have to wonder
> why MST isn't kicking in.
> 
> This does cause a slight change in our behaviour towards the sink.
> Previously we only read the MSTM_CAP register after passing all
> the other checks. Now we will read that register regardless. Hopefully
> some crazy sink doesn't get confused by a simple register read.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 33 ++++++++++++++++++---------------
>  1 file changed, 18 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> b/drivers/gpu/drm/i915/intel_dp.c index 15a981ef5966..1a510cf58fb5 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4050,16 +4050,10 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
>  }
> 
>  static bool
> -intel_dp_can_mst(struct intel_dp *intel_dp)
> +intel_dp_sink_can_mst(struct intel_dp *intel_dp)
>  {
>  	u8 mstm_cap;
> 
> -	if (!i915_modparams.enable_dp_mst)
> -		return false;
> -
> -	if (!intel_dp->can_mst)
> -		return false;
> -
>  	if (intel_dp->dpcd[DP_DPCD_REV] < 0x12)
>  		return false;
> 
> @@ -4069,21 +4063,30 @@ intel_dp_can_mst(struct intel_dp *intel_dp)
>  	return mstm_cap & DP_MST_CAP;
>  }
> 
> +static bool
> +intel_dp_can_mst(struct intel_dp *intel_dp)
> +{
> +	return i915_modparams.enable_dp_mst &&
> +		intel_dp->can_mst &&
> +		intel_dp_sink_can_mst(intel_dp);
> +}
> +
>  static void
>  intel_dp_configure_mst(struct intel_dp *intel_dp)
>  {
> -	if (!i915_modparams.enable_dp_mst)
> -		return;
> +	struct intel_encoder *encoder =
> +		&dp_to_dig_port(intel_dp)->base;
> +	bool sink_can_mst = intel_dp_sink_can_mst(intel_dp);
> +
> +	DRM_DEBUG_KMS("MST support? port %c: %s, sink: %s, modparam: %s\n",
> +		      port_name(encoder->port), yesno(intel_dp->can_mst),
> +		      yesno(sink_can_mst), yesno(i915_modparams.enable_dp_mst));
> 
>  	if (!intel_dp->can_mst)
>  		return;
> 
> -	intel_dp->is_mst = intel_dp_can_mst(intel_dp);
> -
> -	if (intel_dp->is_mst)
> -		DRM_DEBUG_KMS("Sink is MST capable\n");
> -	else
> -		DRM_DEBUG_KMS("Sink is not MST capable\n");
> +	intel_dp->is_mst = sink_can_mst &&
> +		i915_modparams.enable_dp_mst;
Should we still keep this modparam around? I don't know if anyone actually 
uses it to work around MST issues. Even if there are MST issues, we should be 
fixing them.

I see the value in the debug message, 
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>

> 
>  	drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr,
>  					intel_dp->is_mst);