| Message ID | 1475481316-8194-1-git-send-email-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, 2016-10-03 at 10:55 +0300, ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > We can't rely on connector->status in the detect() hook if the long hpd > was already handled by the dig_port_work as that won't update > connector->status. Thus we have to defer the long hpd handling entirely > until the hotplug work runs to avoid the double long hpd handling > the "detect_done" flag is trying to prevent. This is better indeed. But perhaps add a note here about the next patch, since this one doesn't actually change the use of connector->status usage in detect(). For both patches: Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com> > > Cc: Damien Cassou <damien@cassou.me> > Cc: freedesktop.org@gp.mailgun.org > Cc: Arno <blouin.arno@gmail.com> > Cc: Shubhangi Shrivastava <shubhangi.shrivastava@intel.com> > Cc: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> > Cc: Ander Conselvan de Oliveira <conselvan2@gmail.com> > Cc: stable@vger.kernel.org > Tested-by: Arno <blouin.arno@gmail.com> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=83348 > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_dp.c | 48 ++++++++++++++++++++------------------ > --- > 1 file changed, 23 insertions(+), 25 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 9448d898d80b..96caa469e3a8 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -4827,36 +4827,34 @@ intel_dp_hpd_pulse(struct intel_digital_port > *intel_dig_port, bool long_hpd) > port_name(intel_dig_port->port), > long_hpd ? "long" : "short"); > > + if (long_hpd) { > + intel_dp->detect_done = false; > + return IRQ_NONE; > + } > + > power_domain = intel_display_port_aux_power_domain(intel_encoder); > intel_display_power_get(dev_priv, power_domain); > > - if (long_hpd) { > - intel_dp_long_pulse(intel_dp->attached_connector); > - if (intel_dp->is_mst) > - ret = IRQ_HANDLED; > - goto put_power; > - > - } else { > - if (intel_dp->is_mst) { > - if (intel_dp_check_mst_status(intel_dp) == -EINVAL) { > - /* > - * If we were in MST mode, and device is not > - * there, get out of MST mode > - */ > - 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 put_power; > - } > + if (intel_dp->is_mst) { > + if (intel_dp_check_mst_status(intel_dp) == -EINVAL) { > + /* > + * If we were in MST mode, and device is not > + * there, get out of MST mode > + */ > + 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_dp->detect_done = false; > + goto put_power; > } > + } > > - if (!intel_dp->is_mst) { > - if (!intel_dp_short_pulse(intel_dp)) { > - intel_dp_long_pulse(intel_dp- > >attached_connector); > - goto put_power; > - } > + if (!intel_dp->is_mst) { > + if (!intel_dp_short_pulse(intel_dp)) { > + intel_dp->detect_done = false; > + goto put_power; > } > } >
On Mon, Oct 03, 2016 at 02:39:16PM +0300, Ander Conselvan De Oliveira wrote: > On Mon, 2016-10-03 at 10:55 +0300, ville.syrjala@linux.intel.com wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > We can't rely on connector->status in the detect() hook if the long hpd > > was already handled by the dig_port_work as that won't update > > connector->status. Thus we have to defer the long hpd handling entirely > > until the hotplug work runs to avoid the double long hpd handling > > the "detect_done" flag is trying to prevent. > > This is better indeed. But perhaps add a note here about the next patch, since > this one doesn't actually change the use of connector->status usage in detect(). > > For both patches: > > Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com> Thanks. I added a small note to 1/2 and pushed both patches to dinq. > > > > > Cc: Damien Cassou <damien@cassou.me> > > Cc: freedesktop.org@gp.mailgun.org > > Cc: Arno <blouin.arno@gmail.com> > > Cc: Shubhangi Shrivastava <shubhangi.shrivastava@intel.com> > > Cc: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> > > Cc: Ander Conselvan de Oliveira <conselvan2@gmail.com> > > Cc: stable@vger.kernel.org > > Tested-by: Arno <blouin.arno@gmail.com> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=83348 > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > --- > > drivers/gpu/drm/i915/intel_dp.c | 48 ++++++++++++++++++++------------------ > > --- > > 1 file changed, 23 insertions(+), 25 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > > index 9448d898d80b..96caa469e3a8 100644 > > --- a/drivers/gpu/drm/i915/intel_dp.c > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > @@ -4827,36 +4827,34 @@ intel_dp_hpd_pulse(struct intel_digital_port > > *intel_dig_port, bool long_hpd) > > port_name(intel_dig_port->port), > > long_hpd ? "long" : "short"); > > > > + if (long_hpd) { > > + intel_dp->detect_done = false; > > + return IRQ_NONE; > > + } > > + > > power_domain = intel_display_port_aux_power_domain(intel_encoder); > > intel_display_power_get(dev_priv, power_domain); > > > > - if (long_hpd) { > > - intel_dp_long_pulse(intel_dp->attached_connector); > > - if (intel_dp->is_mst) > > - ret = IRQ_HANDLED; > > - goto put_power; > > - > > - } else { > > - if (intel_dp->is_mst) { > > - if (intel_dp_check_mst_status(intel_dp) == -EINVAL) { > > - /* > > - * If we were in MST mode, and device is not > > - * there, get out of MST mode > > - */ > > - 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 put_power; > > - } > > + if (intel_dp->is_mst) { > > + if (intel_dp_check_mst_status(intel_dp) == -EINVAL) { > > + /* > > + * If we were in MST mode, and device is not > > + * there, get out of MST mode > > + */ > > + 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_dp->detect_done = false; > > + goto put_power; > > } > > + } > > > > - if (!intel_dp->is_mst) { > > - if (!intel_dp_short_pulse(intel_dp)) { > > - intel_dp_long_pulse(intel_dp- > > >attached_connector); > > - goto put_power; > > - } > > + if (!intel_dp->is_mst) { > > + if (!intel_dp_short_pulse(intel_dp)) { > > + intel_dp->detect_done = false; > > + goto put_power; > > } > > } > >
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 9448d898d80b..96caa469e3a8 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -4827,36 +4827,34 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd) port_name(intel_dig_port->port), long_hpd ? "long" : "short"); + if (long_hpd) { + intel_dp->detect_done = false; + return IRQ_NONE; + } + power_domain = intel_display_port_aux_power_domain(intel_encoder); intel_display_power_get(dev_priv, power_domain); - if (long_hpd) { - intel_dp_long_pulse(intel_dp->attached_connector); - if (intel_dp->is_mst) - ret = IRQ_HANDLED; - goto put_power; - - } else { - if (intel_dp->is_mst) { - if (intel_dp_check_mst_status(intel_dp) == -EINVAL) { - /* - * If we were in MST mode, and device is not - * there, get out of MST mode - */ - 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 put_power; - } + if (intel_dp->is_mst) { + if (intel_dp_check_mst_status(intel_dp) == -EINVAL) { + /* + * If we were in MST mode, and device is not + * there, get out of MST mode + */ + 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_dp->detect_done = false; + goto put_power; } + } - if (!intel_dp->is_mst) { - if (!intel_dp_short_pulse(intel_dp)) { - intel_dp_long_pulse(intel_dp->attached_connector); - goto put_power; - } + if (!intel_dp->is_mst) { + if (!intel_dp_short_pulse(intel_dp)) { + intel_dp->detect_done = false; + goto put_power; } }