[Intel-gfx] [PATCH 1/3] drm/i915/dp_mst: Fix disabling MST on a port

[Souza, Jose]

On Thu, 2020-06-04 at 00:10 +0300, Imre Deak wrote:
> Currently MST on a port can get enabled/disabled from the hotplug work
> and get disabled from the short pulse work in a racy way. Fix this by
> relying on the MST state checking in the hotplug work and just schedule
> a hotplug work from the short pulse handler if some problem happened
> during the MST interrupt handling.

Nice

> 
> This removes the explicit MST disabling in case of an AUX failure, but
> if AUX fails, then probably the detection will also fail during the
> scheduled hotplug work and it's not guaranteed that we'll see
> intermittent errors anyway.
> 
> While at it also simplify the error checking of the MST interrupt
> handler.
> 

Reviewed-by: José Roberto de Souza <jose.souza at intel.com>

> Signed-off-by: Imre Deak <imre.deak at intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 33 +++----------------------
>  1 file changed, 4 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 55fda074c0ad..befbcacddaa1 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -5604,7 +5604,7 @@ intel_dp_check_mst_status(struct intel_dp *intel_dp)
>  		}
>  	}
>  
> -	return need_retrain;
> +	return need_retrain ? -EINVAL : 0;
>  }
>  
>  static bool
> @@ -7255,35 +7255,10 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
>  	}
>  
>  	if (intel_dp->is_mst) {
> -		switch (intel_dp_check_mst_status(intel_dp)) {
> -		case -EINVAL:
> -			/*
> -			 * If we were in MST mode, and device is not
> -			 * there, get out of MST mode
> -			 */
> -			drm_dbg_kms(&i915->drm,
> -				    "MST device may have disappeared %d vs %d\n",
> -				    intel_dp->is_mst,
> -				    intel_dp->mst_mgr.mst_state);
> -			intel_dp->is_mst = false;
> -			drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr,
> -							intel_dp->is_mst);
> -
> -			return IRQ_NONE;
> -		case 1:
> -			return IRQ_NONE;
> -		default:
> -			break;
> -		}
> -	}
> -
> -	if (!intel_dp->is_mst) {
> -		bool handled;
> -
> -		handled = intel_dp_short_pulse(intel_dp);
> -
> -		if (!handled)
> +		if (intel_dp_check_mst_status(intel_dp) < 0)
>  			return IRQ_NONE;
> +	} else if (!intel_dp_short_pulse(intel_dp)) {
> +		return IRQ_NONE;
>  	}
>  
>  	return IRQ_HANDLED;