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

Ander Conselvan De Oliveira
2016-04-07T08:15:44Z

On Wed, 2016-04-06 at 16:00 -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.
>
> 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>

Fixes: commit 7d23e3c3 ("drm/i915: Cleaning up intel_dp_hpd_pulse")

> Signed-off-by: Jim Bride <jim.bride@linux.intel.com>

> ---
> drivers/gpu/drm/i915/intel_dp.c | 15 ++-------------
> 1 file changed, 2 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index da0c3d2..2d8783e 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4665,20 +4665,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);
> - }
> - }

The point of that code was to get out of MST mode in case of a disconnection,
but it does the wrong thing there. With the deletion, the device would be is MST
until the next time something is plugged to the port. I haven't checked what are
the consequences of that though.

So maybe move that code before the first "goto out". That should cover the was
mst and is now disconnected case.

Ander

>
> intel_display_power_put(to_i915(dev), power_domain);
> return;