* [PATCH 0/3] drm/i915: MST link training locking and cleanups @ 2015-08-27 15:36 ville.syrjala 2015-08-27 15:36 ` [PATCH 1/3] drm/i915: Protect MST retraining with connection_mutex ville.syrjala ` (2 more replies) 0 siblings, 3 replies; 9+ messages in thread From: ville.syrjala @ 2015-08-27 15:36 UTC (permalink / raw) To: intel-gfx From: Ville Syrjälä <ville.syrjala@linux.intel.com> While glancing at the MST hpd code I noticed we have no locking in there to protect the link retraining against a concurrent modeset. So I added some, and then ocd struct and had to clean up the code a bit. These patches are available at git://github.com/vsyrjala/linux.git mst_fixes_cleanups In the repo I've also included an earlier patch which attempts to enable the DDI PLL for MST on SKL [1] [1] http://lists.freedesktop.org/archives/intel-gfx/2015-August/073965.html Ville Syrjälä (3): drm/i915: Protect MST retraining with connection_mutex drm/i915: Flatten the mst suspend/resume functions a bit drm/i915: Flatten intel_dp_check_mst_status() drivers/gpu/drm/i915/intel_dp.c | 127 ++++++++++++++++++++-------------------- 1 file changed, 65 insertions(+), 62 deletions(-) -- 2.4.6 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/3] drm/i915: Protect MST retraining with connection_mutex 2015-08-27 15:36 [PATCH 0/3] drm/i915: MST link training locking and cleanups ville.syrjala @ 2015-08-27 15:36 ` ville.syrjala 2015-09-03 12:11 ` Ville Syrjälä 2015-08-27 15:36 ` [PATCH 2/3] drm/i915: Flatten the mst suspend/resume functions a bit ville.syrjala 2015-08-27 15:36 ` [PATCH 3/3] drm/i915: Flatten intel_dp_check_mst_status() ville.syrjala 2 siblings, 1 reply; 9+ messages in thread From: ville.syrjala @ 2015-08-27 15:36 UTC (permalink / raw) To: intel-gfx From: Ville Syrjälä <ville.syrjala@linux.intel.com> Grab the connection_mutex around MSR link retraining to protect it against a concurrent modeset. We already do the same for SST. DP hpd_pulse can still otherwise race against modeset and ->detect(), so it's not clear what will happen when both want to scribble into eg. intel_dp->dpcd[] at the same time. But sorting it all out requires way more thought than I'm willing to expend now. Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> --- drivers/gpu/drm/i915/intel_dp.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 9e90a2b..1259c9b 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -4391,6 +4391,7 @@ update_status: static int intel_dp_check_mst_status(struct intel_dp *intel_dp) { + struct drm_device *dev = dp_to_dig_port(intel_dp)->base.base.dev; bool bret; if (intel_dp->is_mst) { @@ -4402,6 +4403,8 @@ intel_dp_check_mst_status(struct intel_dp *intel_dp) go_again: if (bret == true) { + drm_modeset_lock(&dev->mode_config.connection_mutex, NULL); + /* check link status - esi[10] = 0x200c */ if (intel_dp->active_mst_links && !drm_dp_channel_eq_ok(&esi[10], intel_dp->lane_count)) { @@ -4411,6 +4414,8 @@ go_again: intel_dp_stop_link_train(intel_dp); } + drm_modeset_unlock(&dev->mode_config.connection_mutex); + DRM_DEBUG_KMS("got esi %3ph\n", esi); ret = drm_dp_mst_hpd_irq(&intel_dp->mst_mgr, esi, &handled); -- 2.4.6 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] drm/i915: Protect MST retraining with connection_mutex 2015-08-27 15:36 ` [PATCH 1/3] drm/i915: Protect MST retraining with connection_mutex ville.syrjala @ 2015-09-03 12:11 ` Ville Syrjälä 2015-09-16 11:48 ` Maarten Lankhorst 0 siblings, 1 reply; 9+ messages in thread From: Ville Syrjälä @ 2015-09-03 12:11 UTC (permalink / raw) To: intel-gfx On Thu, Aug 27, 2015 at 06:36:48PM +0300, ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Grab the connection_mutex around MSR link retraining to protect it > against a concurrent modeset. We already do the same for SST. > > DP hpd_pulse can still otherwise race against modeset and ->detect(), so > it's not clear what will happen when both want to scribble into eg. > intel_dp->dpcd[] at the same time. But sorting it all out requires way > more thought than I'm willing to expend now. Actually I suppose this might not work out so well after all. I suppose MST depends on short HPDs during modeset due to the sideband stuff. So if we want to grab modeset locks for retraining, I suppose we'd need to move the retraining to happen from .hot_plug() which gets run from the other hotplug work, and so wouldn't interfere with sideband. > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_dp.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 9e90a2b..1259c9b 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -4391,6 +4391,7 @@ update_status: > static int > intel_dp_check_mst_status(struct intel_dp *intel_dp) > { > + struct drm_device *dev = dp_to_dig_port(intel_dp)->base.base.dev; > bool bret; > > if (intel_dp->is_mst) { > @@ -4402,6 +4403,8 @@ intel_dp_check_mst_status(struct intel_dp *intel_dp) > go_again: > if (bret == true) { > > + drm_modeset_lock(&dev->mode_config.connection_mutex, NULL); > + > /* check link status - esi[10] = 0x200c */ > if (intel_dp->active_mst_links && > !drm_dp_channel_eq_ok(&esi[10], intel_dp->lane_count)) { > @@ -4411,6 +4414,8 @@ go_again: > intel_dp_stop_link_train(intel_dp); > } > > + drm_modeset_unlock(&dev->mode_config.connection_mutex); > + > DRM_DEBUG_KMS("got esi %3ph\n", esi); > ret = drm_dp_mst_hpd_irq(&intel_dp->mst_mgr, esi, &handled); > > -- > 2.4.6 -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] drm/i915: Protect MST retraining with connection_mutex 2015-09-03 12:11 ` Ville Syrjälä @ 2015-09-16 11:48 ` Maarten Lankhorst 2015-09-23 8:01 ` Daniel Vetter 0 siblings, 1 reply; 9+ messages in thread From: Maarten Lankhorst @ 2015-09-16 11:48 UTC (permalink / raw) To: Ville Syrjälä, intel-gfx Hey, Op 03-09-15 om 14:11 schreef Ville Syrjälä: > On Thu, Aug 27, 2015 at 06:36:48PM +0300, ville.syrjala@linux.intel.com wrote: >> From: Ville Syrjälä <ville.syrjala@linux.intel.com> >> >> Grab the connection_mutex around MSR link retraining to protect it >> against a concurrent modeset. We already do the same for SST. >> >> DP hpd_pulse can still otherwise race against modeset and ->detect(), so >> it's not clear what will happen when both want to scribble into eg. >> intel_dp->dpcd[] at the same time. But sorting it all out requires way >> more thought than I'm willing to expend now. > Actually I suppose this might not work out so well after all. I suppose > MST depends on short HPDs during modeset due to the sideband stuff. > > So if we want to grab modeset locks for retraining, I suppose we'd > need to move the retraining to happen from .hot_plug() which gets run > from the other hotplug work, and so wouldn't interfere with sideband. > I think it would be better to have a per encoder mutex. In the future we may want to run modeset disable/enable async, in which case it may not hold the connection_mutex. If we do decide on a separate mutex then it would also be useful to also think about how to protect intel_mst_*(dis,en)able_dp. :) ~Maarten _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] drm/i915: Protect MST retraining with connection_mutex 2015-09-16 11:48 ` Maarten Lankhorst @ 2015-09-23 8:01 ` Daniel Vetter 0 siblings, 0 replies; 9+ messages in thread From: Daniel Vetter @ 2015-09-23 8:01 UTC (permalink / raw) To: Maarten Lankhorst; +Cc: intel-gfx On Wed, Sep 16, 2015 at 01:48:49PM +0200, Maarten Lankhorst wrote: > Hey, > > Op 03-09-15 om 14:11 schreef Ville Syrjälä: > > On Thu, Aug 27, 2015 at 06:36:48PM +0300, ville.syrjala@linux.intel.com wrote: > >> From: Ville Syrjälä <ville.syrjala@linux.intel.com> > >> > >> Grab the connection_mutex around MSR link retraining to protect it > >> against a concurrent modeset. We already do the same for SST. > >> > >> DP hpd_pulse can still otherwise race against modeset and ->detect(), so > >> it's not clear what will happen when both want to scribble into eg. > >> intel_dp->dpcd[] at the same time. But sorting it all out requires way > >> more thought than I'm willing to expend now. > > Actually I suppose this might not work out so well after all. I suppose > > MST depends on short HPDs during modeset due to the sideband stuff. > > > > So if we want to grab modeset locks for retraining, I suppose we'd > > need to move the retraining to happen from .hot_plug() which gets run > > from the other hotplug work, and so wouldn't interfere with sideband. > > > I think it would be better to have a per encoder mutex. In the future we may want to run modeset > disable/enable async, in which case it may not hold the connection_mutex. > > If we do decide on a separate mutex then it would also be useful to also think about how to > protect intel_mst_*(dis,en)able_dp. :) We need to sync re-training with any other modeset work. Which means another thing to get right for async modesets. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/3] drm/i915: Flatten the mst suspend/resume functions a bit 2015-08-27 15:36 [PATCH 0/3] drm/i915: MST link training locking and cleanups ville.syrjala 2015-08-27 15:36 ` [PATCH 1/3] drm/i915: Protect MST retraining with connection_mutex ville.syrjala @ 2015-08-27 15:36 ` ville.syrjala 2015-09-16 12:28 ` Maarten Lankhorst 2015-08-27 15:36 ` [PATCH 3/3] drm/i915: Flatten intel_dp_check_mst_status() ville.syrjala 2 siblings, 1 reply; 9+ messages in thread From: ville.syrjala @ 2015-08-27 15:36 UTC (permalink / raw) To: intel-gfx From: Ville Syrjälä <ville.syrjala@linux.intel.com> I'm not a fan of deeply nested ifs. Just pull most of the conditions into a single place to flatten things a bit. Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> --- drivers/gpu/drm/i915/intel_dp.c | 32 ++++++++++++++------------------ 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 1259c9b..6c34784 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -6252,15 +6252,14 @@ void intel_dp_mst_suspend(struct drm_device *dev) /* disable MST */ for (i = 0; i < I915_MAX_PORTS; i++) { struct intel_digital_port *intel_dig_port = dev_priv->hotplug.irq_port[i]; - if (!intel_dig_port) + + if (!intel_dig_port || + intel_dig_port->base.type != INTEL_OUTPUT_DISPLAYPORT || + !intel_dig_port->dp.can_mst) continue; - if (intel_dig_port->base.type == INTEL_OUTPUT_DISPLAYPORT) { - if (!intel_dig_port->dp.can_mst) - continue; - if (intel_dig_port->dp.is_mst) - drm_dp_mst_topology_mgr_suspend(&intel_dig_port->dp.mst_mgr); - } + if (intel_dig_port->dp.is_mst) + drm_dp_mst_topology_mgr_suspend(&intel_dig_port->dp.mst_mgr); } } @@ -6271,18 +6270,15 @@ void intel_dp_mst_resume(struct drm_device *dev) for (i = 0; i < I915_MAX_PORTS; i++) { struct intel_digital_port *intel_dig_port = dev_priv->hotplug.irq_port[i]; - if (!intel_dig_port) - continue; - if (intel_dig_port->base.type == INTEL_OUTPUT_DISPLAYPORT) { - int ret; + int ret; - if (!intel_dig_port->dp.can_mst) - continue; + if (!intel_dig_port || + intel_dig_port->base.type != INTEL_OUTPUT_DISPLAYPORT || + !intel_dig_port->dp.can_mst) + continue; - ret = drm_dp_mst_topology_mgr_resume(&intel_dig_port->dp.mst_mgr); - if (ret != 0) { - intel_dp_check_mst_status(&intel_dig_port->dp); - } - } + ret = drm_dp_mst_topology_mgr_resume(&intel_dig_port->dp.mst_mgr); + if (ret) + intel_dp_check_mst_status(&intel_dig_port->dp); } } -- 2.4.6 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] drm/i915: Flatten the mst suspend/resume functions a bit 2015-08-27 15:36 ` [PATCH 2/3] drm/i915: Flatten the mst suspend/resume functions a bit ville.syrjala @ 2015-09-16 12:28 ` Maarten Lankhorst 0 siblings, 0 replies; 9+ messages in thread From: Maarten Lankhorst @ 2015-09-16 12:28 UTC (permalink / raw) To: ville.syrjala, intel-gfx Hey, Op 27-08-15 om 17:36 schreef ville.syrjala@linux.intel.com: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > I'm not a fan of deeply nested ifs. Just pull most of the conditions > into a single place to flatten things a bit. I like flat.. Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/3] drm/i915: Flatten intel_dp_check_mst_status() 2015-08-27 15:36 [PATCH 0/3] drm/i915: MST link training locking and cleanups ville.syrjala 2015-08-27 15:36 ` [PATCH 1/3] drm/i915: Protect MST retraining with connection_mutex ville.syrjala 2015-08-27 15:36 ` [PATCH 2/3] drm/i915: Flatten the mst suspend/resume functions a bit ville.syrjala @ 2015-08-27 15:36 ` ville.syrjala 2015-09-16 13:07 ` Maarten Lankhorst 2 siblings, 1 reply; 9+ messages in thread From: ville.syrjala @ 2015-08-27 15:36 UTC (permalink / raw) To: intel-gfx From: Ville Syrjälä <ville.syrjala@linux.intel.com> Restructure intel_dp_check_mst_status() to be more straightforward to read. Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> --- drivers/gpu/drm/i915/intel_dp.c | 92 +++++++++++++++++++++-------------------- 1 file changed, 47 insertions(+), 45 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 6c34784..033ee20 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -4392,62 +4392,64 @@ static int intel_dp_check_mst_status(struct intel_dp *intel_dp) { struct drm_device *dev = dp_to_dig_port(intel_dp)->base.base.dev; + u8 esi[16]; bool bret; - if (intel_dp->is_mst) { - u8 esi[16] = { 0 }; - int ret = 0; - int retry; + if (!intel_dp->is_mst) + return -EINVAL; + + bret = intel_dp_get_sink_irq_esi(intel_dp, esi); + + if (!bret) { + DRM_DEBUG_KMS("failed to get ESI - device may have failed\n"); + intel_dp->is_mst = false; + drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr, intel_dp->is_mst); + /* send a hotplug event */ + drm_kms_helper_hotplug_event(dev); + + return -EINVAL; + } + + for (;;) { bool handled; - bret = intel_dp_get_sink_irq_esi(intel_dp, esi); -go_again: - if (bret == true) { + int retry; + int ret; - drm_modeset_lock(&dev->mode_config.connection_mutex, NULL); + drm_modeset_lock(&dev->mode_config.connection_mutex, NULL); - /* check link status - esi[10] = 0x200c */ - if (intel_dp->active_mst_links && - !drm_dp_channel_eq_ok(&esi[10], intel_dp->lane_count)) { - DRM_DEBUG_KMS("channel EQ not ok, retraining\n"); - intel_dp_start_link_train(intel_dp); - intel_dp_complete_link_train(intel_dp); - intel_dp_stop_link_train(intel_dp); - } + /* check link status - esi[10] = 0x200c */ + if (intel_dp->active_mst_links && + !drm_dp_channel_eq_ok(&esi[10], intel_dp->lane_count)) { + DRM_DEBUG_KMS("channel EQ not ok, retraining\n"); + intel_dp_start_link_train(intel_dp); + intel_dp_complete_link_train(intel_dp); + intel_dp_stop_link_train(intel_dp); + } - drm_modeset_unlock(&dev->mode_config.connection_mutex); - DRM_DEBUG_KMS("got esi %3ph\n", esi); - ret = drm_dp_mst_hpd_irq(&intel_dp->mst_mgr, esi, &handled); - - if (handled) { - for (retry = 0; retry < 3; retry++) { - int wret; - wret = drm_dp_dpcd_write(&intel_dp->aux, - DP_SINK_COUNT_ESI+1, - &esi[1], 3); - if (wret == 3) { - break; - } - } + drm_modeset_unlock(&dev->mode_config.connection_mutex); - bret = intel_dp_get_sink_irq_esi(intel_dp, esi); - if (bret == true) { - DRM_DEBUG_KMS("got esi2 %3ph\n", esi); - goto go_again; - } - } else - ret = 0; + DRM_DEBUG_KMS("got esi %3ph\n", esi); + ret = drm_dp_mst_hpd_irq(&intel_dp->mst_mgr, esi, &handled); - return ret; - } else { - struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); - DRM_DEBUG_KMS("failed to get ESI - device may have failed\n"); - intel_dp->is_mst = false; - drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr, intel_dp->is_mst); - /* send a hotplug event */ - drm_kms_helper_hotplug_event(intel_dig_port->base.base.dev); + if (!handled) + return 0; + + for (retry = 0; retry < 3; retry++) { + int wret = drm_dp_dpcd_write(&intel_dp->aux, + DP_SINK_COUNT_ESI+1, + &esi[1], 3); + if (wret == 3) + break; } + + bret = intel_dp_get_sink_irq_esi(intel_dp, esi); + if (!bret) + return ret; + + DRM_DEBUG_KMS("got esi2 %3ph\n", esi); } + return -EINVAL; } -- 2.4.6 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] drm/i915: Flatten intel_dp_check_mst_status() 2015-08-27 15:36 ` [PATCH 3/3] drm/i915: Flatten intel_dp_check_mst_status() ville.syrjala @ 2015-09-16 13:07 ` Maarten Lankhorst 0 siblings, 0 replies; 9+ messages in thread From: Maarten Lankhorst @ 2015-09-16 13:07 UTC (permalink / raw) To: ville.syrjala, intel-gfx Op 27-08-15 om 17:36 schreef ville.syrjala@linux.intel.com: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Restructure intel_dp_check_mst_status() to be more straightforward to > read. > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_dp.c | 92 +++++++++++++++++++++-------------------- > 1 file changed, 47 insertions(+), 45 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 6c34784..033ee20 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > <snip> > + for (;;) { > bool handled; > - bret = intel_dp_get_sink_irq_esi(intel_dp, esi); > -go_again: > - if (bret == true) { > + int retry; > + int ret; > > - drm_modeset_lock(&dev->mode_config.connection_mutex, NULL); > + drm_modeset_lock(&dev->mode_config.connection_mutex, NULL); > > - /* check link status - esi[10] = 0x200c */ > - if (intel_dp->active_mst_links && > - !drm_dp_channel_eq_ok(&esi[10], intel_dp->lane_count)) { > - DRM_DEBUG_KMS("channel EQ not ok, retraining\n"); > - intel_dp_start_link_train(intel_dp); > - intel_dp_complete_link_train(intel_dp); > - intel_dp_stop_link_train(intel_dp); > - } > + /* check link status - esi[10] = 0x200c */ > + if (intel_dp->active_mst_links && > + !drm_dp_channel_eq_ok(&esi[10], intel_dp->lane_count)) { > + DRM_DEBUG_KMS("channel EQ not ok, retraining\n"); > + intel_dp_start_link_train(intel_dp); > + intel_dp_complete_link_train(intel_dp); > + intel_dp_stop_link_train(intel_dp); > + } > > - drm_modeset_unlock(&dev->mode_config.connection_mutex); > > - DRM_DEBUG_KMS("got esi %3ph\n", esi); > - ret = drm_dp_mst_hpd_irq(&intel_dp->mst_mgr, esi, &handled); > - > - if (handled) { > - for (retry = 0; retry < 3; retry++) { > - int wret; > - wret = drm_dp_dpcd_write(&intel_dp->aux, > - DP_SINK_COUNT_ESI+1, > - &esi[1], 3); > - if (wret == 3) { > - break; > - } > - } > + drm_modeset_unlock(&dev->mode_config.connection_mutex); > > - bret = intel_dp_get_sink_irq_esi(intel_dp, esi); > - if (bret == true) { > - DRM_DEBUG_KMS("got esi2 %3ph\n", esi); > - goto go_again; > - } > - } else > - ret = 0; > + DRM_DEBUG_KMS("got esi %3ph\n", esi); > + ret = drm_dp_mst_hpd_irq(&intel_dp->mst_mgr, esi, &handled); > > - return ret; > - } else { > - struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); > - DRM_DEBUG_KMS("failed to get ESI - device may have failed\n"); > - intel_dp->is_mst = false; > - drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr, intel_dp->is_mst); > - /* send a hotplug event */ > - drm_kms_helper_hotplug_event(intel_dig_port->base.base.dev); > + if (!handled) > + return 0; > + > + for (retry = 0; retry < 3; retry++) { > + int wret = drm_dp_dpcd_write(&intel_dp->aux, > + DP_SINK_COUNT_ESI+1, > + &esi[1], 3); > + if (wret == 3) > + break; > } > + > + bret = intel_dp_get_sink_irq_esi(intel_dp, esi); > + if (!bret) > + return ret; ^This seemed like a bug the first time I looked at it with the indent changes. Original indent with if (handled) {.. } seems better here, but with a continue instead of a goto. I think a single return ret; would make it more clear when the loop finishes. The original code sets ret = 0 when handled = false, but looking a drm_dp_mst_hpd_irq this is unneeded. > + > + DRM_DEBUG_KMS("got esi2 %3ph\n", esi); > } > + > return -EINVAL; Can this -EINVAL be removed? It cannot be reached any more. _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2015-09-23 7:58 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-08-27 15:36 [PATCH 0/3] drm/i915: MST link training locking and cleanups ville.syrjala 2015-08-27 15:36 ` [PATCH 1/3] drm/i915: Protect MST retraining with connection_mutex ville.syrjala 2015-09-03 12:11 ` Ville Syrjälä 2015-09-16 11:48 ` Maarten Lankhorst 2015-09-23 8:01 ` Daniel Vetter 2015-08-27 15:36 ` [PATCH 2/3] drm/i915: Flatten the mst suspend/resume functions a bit ville.syrjala 2015-09-16 12:28 ` Maarten Lankhorst 2015-08-27 15:36 ` [PATCH 3/3] drm/i915: Flatten intel_dp_check_mst_status() ville.syrjala 2015-09-16 13:07 ` Maarten Lankhorst
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.