public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
From: Lyude <cpaul@redhat.com>
To: Ander Conselvan De Oliveira <conselvan2@gmail.com>,
	Jim Bride <jim.bride@linux.intel.com>,
	intel-gfx@lists.freedesktop.org
Cc: Shubhangi Shrivastava <shubhangi.shrivastava@intel.com>,
	Nathan D Ciobanu <nathan.d.ciobanu@intel.com>
Subject: Re: [PATCH v2] drm/i915/dp/mst: Fix MST logic in intel_dp_long_pulse()
Date: Tue, 12 Apr 2016 16:07:09 -0400	[thread overview]
Message-ID: <1460491629.4684.3.camel@redhat.com> (raw)
In-Reply-To: <1460445087.13545.2.camel@gmail.com>

I'm going to take back the NAK on this, apparently this hotplugging
issue has been around longer then this patchset.

Reviewed-by: Lyude <cpaul@redhat.com>


On Tue, 2016-04-12 at 10:11 +0300, Ander Conselvan De Oliveira wrote:
> 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>
> Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.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;
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
-- 
Cheers,
	Lyude

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2016-04-12 20:07 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-06 23:00 [PATCH] drm/i915/dp/mst: Fix MST logic in intel_dp_long_pulse() Jim Bride
2016-04-07  7:55 ` ✗ Fi.CI.BAT: warning for " Patchwork
2016-04-07  8:15 ` [PATCH] " Ander Conselvan De Oliveira
2016-04-07 16:20   ` Jim Bride
2016-04-08  7:30     ` Ander Conselvan De Oliveira
2016-04-11 17:11 ` [PATCH v2] " Jim Bride
2016-04-11 21:44   ` Lyude Paul
2016-04-12  7:11   ` Ander Conselvan De Oliveira
2016-04-12 20:07     ` Lyude [this message]
2016-04-12  7:39 ` ✗ Fi.CI.BAT: failure for drm/i915/dp/mst: Fix MST logic in intel_dp_long_pulse() (rev3) Patchwork
2016-04-12 20:26   ` Jim Bride
2016-04-13 18:06   ` Jim Bride

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1460491629.4684.3.camel@redhat.com \
    --to=cpaul@redhat.com \
    --cc=conselvan2@gmail.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jim.bride@linux.intel.com \
    --cc=nathan.d.ciobanu@intel.com \
    --cc=shubhangi.shrivastava@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox