Re: [PATCH v2] drm/i915/dp/mst: Fix MST logic in intel_dp_long_pulse()

Lyude Paul
2016-04-11T21:44:20Z

NAK. Try plugging in an MST display, suspending the machine, then resuming it.
Hotplugging still breaks (which I've traced down to this patch)

I wouldn't worry about fixing this up. I'm probably going to be sending a revert
for this anyway soon along with probably some of the other patches.

On Mon, 2016-04-11 at 10:11 -0700, Jim Bride wrote:
> In commit 7d23e3c3 ("drm/i915: Cleaning up intel_dp_hpd_pulse") some
> much needed clean-up was done, but unfortunately part of the change
> broke DP MST.  The real issue was setting the connector state to
> disconnected in the MST case, which is good, but the code then (after
> a goto) checks if the connector state is not connected and shuts down
> MST if this is the case, which is bad.  With this change both SST and
> MST seem to be happy.
>
> v2: Add removed check further up in the function to be sure that MST
>     is shut down when we lose the link. (Ander)
>
> Fixes: commit 7d23e3c3 ("drm/i915: Cleaning up intel_dp_hpd_pulse")
> cc: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
> cc: Shubhangi Shrivastava <shubhangi.shrivastava@intel.com>
> cc: Ander Conselvan de Oliveira <conselvan2@gmail.com>
> cc: Nathan D Ciobanu <nathan.d.ciobanu@intel.com>
> Signed-off-by: Jim Bride <jim.bride@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 24 +++++++++++-------------
>  1 file changed, 11 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index da0c3d2..31b222a 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4608,6 +4608,15 @@ intel_dp_long_pulse(struct intel_connector
> *intel_connector)
>   intel_dp->compliance_test_type = 0;
>   intel_dp->compliance_test_data = 0;
>
> + if (intel_dp->is_mst) {
> + DRM_DEBUG_KMS("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);
> + }
> +
>   goto out;
>   }
>
> @@ -4665,20 +4674,9 @@ intel_dp_long_pulse(struct intel_connector
> *intel_connector)
>   }
>
>  out:
> - if (status != connector_status_connected) {
> + if ((status != connector_status_connected) &&
> +     (intel_dp->is_mst == false))
>   intel_dp_unset_edid(intel_dp);
> - /*
> -  * If we were in MST mode, and device is not there,
> -  * get out of MST mode
> -  */
> - if (intel_dp->is_mst) {
> - DRM_DEBUG_KMS("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);
> - }
> - }
>
>   intel_display_power_put(to_i915(dev), power_domain);
>   return;