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

[Jim Bride]

On Thu, Apr 07, 2016 at 11:15:38AM +0300, Ander Conselvan De Oliveira wrote:
> 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 at intel.com>
> > cc: Shubhangi Shrivastava <shubhangi.shrivastava at intel.com>
> > cc: Ander Conselvan de Oliveira <conselvan2 at gmail.com>
> > cc: Nathan D Ciobanu <nathan.d.ciobanu at intel.com>
> 
> Fixes: commit 7d23e3c3 ("drm/i915: Cleaning up intel_dp_hpd_pulse")
> 
> > Signed-off-by: Jim Bride <jim.bride at 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.

I don't think that would work either.  For what you're talking about it seems to
me that the most reliable way to catch this case would be to check that we can
read the branch OUI, and if we cannot then we should shut down MST.

Thoughts?

Jim


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