| Message ID | 20170301171749.13053-1-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Mar 01, 2017 at 06:17:49PM +0100, Daniel Vetter wrote: > This reverts commit 233ce881dd91fb13eb6b09deefae33168e6ead4c. > > I assumed it's ok, but really should have double-checked - CI caught > tons of fail :( For the record, the failure comes from the error message in intel_dp_get_link_train_fallback_values() as take the fallback path. As userspace is informed, we don't need an *ERROR* at that point. The really interesting question is why we are seeing link-training failures in CI at all, and whether igt should be checking and reporting link-status=BAD. -Chris
On Wed, 01 Mar 2017, Chris Wilson <chris@chris-wilson.co.uk> wrote: > On Wed, Mar 01, 2017 at 06:17:49PM +0100, Daniel Vetter wrote: >> This reverts commit 233ce881dd91fb13eb6b09deefae33168e6ead4c. >> >> I assumed it's ok, but really should have double-checked - CI caught >> tons of fail :( Considering the velocity of drm-tip, I think any CI results for patches have a rather limited best before date. The patch should've been resent and gone through testing again before merging. > For the record, the failure comes from the error message in > intel_dp_get_link_train_fallback_values() as take the fallback path. As > userspace is informed, we don't need an *ERROR* at that point. > > The really interesting question is why we are seeing link-training > failures in CI at all, and whether igt should be checking and reporting > link-status=BAD. It's possible (I didn't check the logs) this pertains to the failure mode I've sometimes seen, where clock recovery fails, but as we continue with channel equalization anyway (without this patch), everything succeeds there. At worst we need to root cause and fix that issue first. :( BR, Jani.
On Wed, 01 Mar 2017, Jani Nikula <jani.nikula@intel.com> wrote: > On Wed, 01 Mar 2017, Chris Wilson <chris@chris-wilson.co.uk> wrote: >> On Wed, Mar 01, 2017 at 06:17:49PM +0100, Daniel Vetter wrote: >>> This reverts commit 233ce881dd91fb13eb6b09deefae33168e6ead4c. >>> >>> I assumed it's ok, but really should have double-checked - CI caught >>> tons of fail :( > > Considering the velocity of drm-tip, I think any CI results for patches > have a rather limited best before date. The patch should've been resent > and gone through testing again before merging. > >> For the record, the failure comes from the error message in >> intel_dp_get_link_train_fallback_values() as take the fallback path. As >> userspace is informed, we don't need an *ERROR* at that point. >> >> The really interesting question is why we are seeing link-training >> failures in CI at all, and whether igt should be checking and reporting >> link-status=BAD. > > It's possible (I didn't check the logs) this pertains to the failure > mode I've sometimes seen, where clock recovery fails, but as we continue > with channel equalization anyway (without this patch), everything > succeeds there. At worst we need to root cause and fix that issue > first. :( Also, possibly to "avoid noise" in CI, the relevant error messages in intel_dp_link_training_clock_recovery() have been brushed under the carpet by demoting them to DRM_DEBUG_KMS, so we don't really see this happening unless we actually eyeball the logs. > > BR, > Jani.
On Wed, Mar 01, 2017 at 07:35:15PM +0200, Jani Nikula wrote: > On Wed, 01 Mar 2017, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > On Wed, Mar 01, 2017 at 06:17:49PM +0100, Daniel Vetter wrote: > >> This reverts commit 233ce881dd91fb13eb6b09deefae33168e6ead4c. > >> > >> I assumed it's ok, but really should have double-checked - CI caught > >> tons of fail :( > > Considering the velocity of drm-tip, I think any CI results for patches > have a rather limited best before date. The patch should've been resent > and gone through testing again before merging. > > > For the record, the failure comes from the error message in > > intel_dp_get_link_train_fallback_values() as take the fallback path. As > > userspace is informed, we don't need an *ERROR* at that point. > > > > The really interesting question is why we are seeing link-training > > failures in CI at all, and whether igt should be checking and reporting > > link-status=BAD. > > It's possible (I didn't check the logs) this pertains to the failure > mode I've sometimes seen, where clock recovery fails, but as we continue > with channel equalization anyway (without this patch), everything > succeeds there. At worst we need to root cause and fix that issue > first. :( The skl case seems pretty clear. We register DP for both port A and port E even though we should register it only for port E (I think). They both end up both using AUX A and so we think the same sink is connected to both, and then we try to enable port A which fail for obvious reasons. The culprit is init_vbt_defaults() which always sets .supports_dp=true for port A unless later overridden by the VBT. In this case the VBT has no port A, so we leave the .supports_dp flag set. So presumably we should just nuke this stuff from init_vbt_defaults(). IIRC this was discussed at some point between Imre and Paulo, but I can't remember what the conclusion was, or if in fact there was one. The ilk failure case is a lot less clear. It's one of those cases where the sink just keeps requesting the same vswing/preemph all the time. I've seen it sometime in the past, but I've never been able to figure out what has caused it to happen with any specific sink.
On Wed, Mar 01, 2017 at 07:35:15PM +0200, Jani Nikula wrote: > On Wed, 01 Mar 2017, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > On Wed, Mar 01, 2017 at 06:17:49PM +0100, Daniel Vetter wrote: > >> This reverts commit 233ce881dd91fb13eb6b09deefae33168e6ead4c. > >> > >> I assumed it's ok, but really should have double-checked - CI caught > >> tons of fail :( > > Considering the velocity of drm-tip, I think any CI results for patches > have a rather limited best before date. The patch should've been resent > and gone through testing again before merging. Well the original patch also failed in the CI report, so the issue is known even without resubmit. -Daniel
On Wed, Mar 01, 2017 at 06:17:49PM +0100, Daniel Vetter wrote: > This reverts commit 233ce881dd91fb13eb6b09deefae33168e6ead4c. > > I assumed it's ok, but really should have double-checked - CI caught > tons of fail :( > > Cc: Jani Nikula <jani.nikula@intel.com> > Cc: Manasi Navare <manasi.d.navare@intel.com> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Pushed with Manasi' ack on irc. -Daniel > --- > drivers/gpu/drm/i915/intel_dp.c | 27 --------------------------- > drivers/gpu/drm/i915/intel_dp_link_training.c | 22 ++-------------------- > drivers/gpu/drm/i915/intel_drv.h | 3 --- > 3 files changed, 2 insertions(+), 50 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index af07a830fa95..d1670b8afbf5 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -5790,29 +5790,6 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, > return false; > } > > -static void intel_dp_modeset_retry_work_fn(struct work_struct *work) > -{ > - struct intel_connector *intel_connector; > - struct drm_connector *connector; > - > - intel_connector = container_of(work, typeof(*intel_connector), > - modeset_retry_work); > - connector = &intel_connector->base; > - DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", connector->base.id, > - connector->name); > - > - /* Grab the locks before changing connector property*/ > - mutex_lock(&connector->dev->mode_config.mutex); > - /* Set connector link status to BAD and send a Uevent to notify > - * userspace to do a modeset. > - */ > - drm_mode_connector_set_link_status_property(connector, > - DRM_MODE_LINK_STATUS_BAD); > - mutex_unlock(&connector->dev->mode_config.mutex); > - /* Send Hotplug uevent so userspace can reprobe */ > - drm_kms_helper_hotplug_event(connector->dev); > -} > - > bool > intel_dp_init_connector(struct intel_digital_port *intel_dig_port, > struct intel_connector *intel_connector) > @@ -5825,10 +5802,6 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port, > enum port port = intel_dig_port->port; > int type; > > - /* Initialize the work for modeset in case of link train failure */ > - INIT_WORK(&intel_connector->modeset_retry_work, > - intel_dp_modeset_retry_work_fn); > - > if (WARN(intel_dig_port->max_lanes < 1, > "Not enough lanes (%d) for DP on port %c\n", > intel_dig_port->max_lanes, port_name(port))) > diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c > index 955b239e7c2d..0048b520baf7 100644 > --- a/drivers/gpu/drm/i915/intel_dp_link_training.c > +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c > @@ -313,24 +313,6 @@ void intel_dp_stop_link_train(struct intel_dp *intel_dp) > void > intel_dp_start_link_train(struct intel_dp *intel_dp) > { > - struct intel_connector *intel_connector = intel_dp->attached_connector; > - > - if (!intel_dp_link_training_clock_recovery(intel_dp)) > - goto failure_handling; > - if (!intel_dp_link_training_channel_equalization(intel_dp)) > - goto failure_handling; > - > - DRM_DEBUG_KMS("Link Training Passed at Link Rate = %d, Lane count = %d", > - intel_dp->link_rate, intel_dp->lane_count); > - return; > - > - failure_handling: > - DRM_DEBUG_KMS("Link Training failed at link rate = %d, lane count = %d", > - intel_dp->link_rate, intel_dp->lane_count); > - if (!intel_dp_get_link_train_fallback_values(intel_dp, > - intel_dp->link_rate, > - intel_dp->lane_count)) > - /* Schedule a Hotplug Uevent to userspace to start modeset */ > - schedule_work(&intel_connector->modeset_retry_work); > - return; > + intel_dp_link_training_clock_recovery(intel_dp); > + intel_dp_link_training_channel_equalization(intel_dp); > } > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 6c6abc932726..3b797909f631 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -316,9 +316,6 @@ struct intel_connector { > void *port; /* store this opaque as its illegal to dereference it */ > > struct intel_dp *mst_port; > - > - /* Work struct to schedule a uevent on link train failure */ > - struct work_struct modeset_retry_work; > }; > > struct dpll { > -- > 2.11.0 >
On Wed, Mar 01, 2017 at 07:56:26PM +0200, Ville Syrjälä wrote: > On Wed, Mar 01, 2017 at 07:35:15PM +0200, Jani Nikula wrote: > > On Wed, 01 Mar 2017, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > > On Wed, Mar 01, 2017 at 06:17:49PM +0100, Daniel Vetter wrote: > > >> This reverts commit 233ce881dd91fb13eb6b09deefae33168e6ead4c. > > >> > > >> I assumed it's ok, but really should have double-checked - CI caught > > >> tons of fail :( > > > > Considering the velocity of drm-tip, I think any CI results for patches > > have a rather limited best before date. The patch should've been resent > > and gone through testing again before merging. > > > > > For the record, the failure comes from the error message in > > > intel_dp_get_link_train_fallback_values() as take the fallback path. As > > > userspace is informed, we don't need an *ERROR* at that point. > > > > > > The really interesting question is why we are seeing link-training > > > failures in CI at all, and whether igt should be checking and reporting > > > link-status=BAD. > > > > It's possible (I didn't check the logs) this pertains to the failure > > mode I've sometimes seen, where clock recovery fails, but as we continue > > with channel equalization anyway (without this patch), everything > > succeeds there. At worst we need to root cause and fix that issue > > first. :( > > The skl case seems pretty clear. We register DP for both port A and port > E even though we should register it only for port E (I think). They > both end up both using AUX A and so we think the same sink is connected > to both, and then we try to enable port A which fail for obvious reasons. > > The culprit is init_vbt_defaults() which always sets .supports_dp=true > for port A unless later overridden by the VBT. In this case the VBT has > no port A, so we leave the .supports_dp flag set. So presumably we > should just nuke this stuff from init_vbt_defaults(). > > IIRC this was discussed at some point between Imre and Paulo, but I > can't remember what the conclusion was, or if in fact there was one. > > The ilk failure case is a lot less clear. It's one of those cases > where the sink just keeps requesting the same vswing/preemph all > the time. I've seen it sometime in the past, but I've never been > able to figure out what has caused it to happen with any specific > sink. Looks like starting with: [ 3.621706] [drm:drm_dp_i2c_do_msg] I2C nack (result=0, size=1 [ 3.622794] [drm:drm_dp_i2c_do_msg] I2C nack (result=0, size=1 [ 3.623836] [drm:drm_dp_i2c_do_msg] I2C nack (result=0, size=1 [ 3.624903] [drm:drm_dp_i2c_do_msg] I2C nack (result=0, size=1 [ 3.625983] [drm:drm_dp_i2c_do_msg] I2C nack (result=0, size=1 and so failing to read out EDID. As a result we'll use the default 1024x768 which fails. In the working case we use the native 1920x1200 which succeeds. Could be that 1024x768 didn't work even before? I can't see how the patch would affect the I2C transfers. --Imre > > -- > Ville Syrjälä > Intel OTC
On Wed, Mar 01, 2017 at 05:27:53PM +0000, Chris Wilson wrote: > On Wed, Mar 01, 2017 at 06:17:49PM +0100, Daniel Vetter wrote: > > This reverts commit 233ce881dd91fb13eb6b09deefae33168e6ead4c. > > > > I assumed it's ok, but really should have double-checked - CI caught > > tons of fail :( > > For the record, the failure comes from the error message in > intel_dp_get_link_train_fallback_values() as take the fallback path. As > userspace is informed, we don't need an *ERROR* at that point. > Should this be just a DEBUG_KMS instead of DRM_ERROR each time it fails link training since we are sending uevent to userspace for it to retry? That will make CI happy, but I think we still need to fix the real problem of eDP link failures on Port A for SKL. Regards Manasi > The really interesting question is why we are seeing link-training > failures in CI at all, and whether igt should be checking and reporting > link-status=BAD. > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, Mar 02, 2017 at 12:07:05PM +0200, Imre Deak wrote: > On Wed, Mar 01, 2017 at 07:56:26PM +0200, Ville Syrjälä wrote: > > On Wed, Mar 01, 2017 at 07:35:15PM +0200, Jani Nikula wrote: > > > On Wed, 01 Mar 2017, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > > > On Wed, Mar 01, 2017 at 06:17:49PM +0100, Daniel Vetter wrote: > > > >> This reverts commit 233ce881dd91fb13eb6b09deefae33168e6ead4c. > > > >> > > > >> I assumed it's ok, but really should have double-checked - CI caught > > > >> tons of fail :( > > > > > > Considering the velocity of drm-tip, I think any CI results for patches > > > have a rather limited best before date. The patch should've been resent > > > and gone through testing again before merging. > > > > > > > For the record, the failure comes from the error message in > > > > intel_dp_get_link_train_fallback_values() as take the fallback path. As > > > > userspace is informed, we don't need an *ERROR* at that point. > > > > > > > > The really interesting question is why we are seeing link-training > > > > failures in CI at all, and whether igt should be checking and reporting > > > > link-status=BAD. > > > > > > It's possible (I didn't check the logs) this pertains to the failure > > > mode I've sometimes seen, where clock recovery fails, but as we continue > > > with channel equalization anyway (without this patch), everything > > > succeeds there. At worst we need to root cause and fix that issue > > > first. :( > > > > The skl case seems pretty clear. We register DP for both port A and port > > E even though we should register it only for port E (I think). They > > both end up both using AUX A and so we think the same sink is connected > > to both, and then we try to enable port A which fail for obvious reasons. > > > > The culprit is init_vbt_defaults() which always sets .supports_dp=true > > for port A unless later overridden by the VBT. In this case the VBT has > > no port A, so we leave the .supports_dp flag set. So presumably we > > should just nuke this stuff from init_vbt_defaults(). > > > > IIRC this was discussed at some point between Imre and Paulo, but I > > can't remember what the conclusion was, or if in fact there was one. > > > > The ilk failure case is a lot less clear. It's one of those cases > > where the sink just keeps requesting the same vswing/preemph all > > the time. I've seen it sometime in the past, but I've never been > > able to figure out what has caused it to happen with any specific > > sink. > > Looks like starting with: > [ 3.621706] [drm:drm_dp_i2c_do_msg] I2C nack (result=0, size=1 > [ 3.622794] [drm:drm_dp_i2c_do_msg] I2C nack (result=0, size=1 > [ 3.623836] [drm:drm_dp_i2c_do_msg] I2C nack (result=0, size=1 > [ 3.624903] [drm:drm_dp_i2c_do_msg] I2C nack (result=0, size=1 > [ 3.625983] [drm:drm_dp_i2c_do_msg] I2C nack (result=0, size=1 > > and so failing to read out EDID. As a result we'll use the default 1024x768 > which fails. In the working case we use the native 1920x1200 which succeeds. > Could be that 1024x768 didn't work even before? Hm no. The EDID read actually succeeds. The difference in mode is due to a different monitor being attached. In any case I saw the monitor with 1024x768 mode already in earlier logs too where the modeset succeeded, so the problem is somewhere else. --Imre
Hi Daniel, I investigated the CI failures because of this patch. These were essentially the failures which were always there but hidden because they used be DEBUG_KMS messages for link failures so never got caught by CI. But now my patch actually throws DRM_ERROR if the link training fails at RBR and 1 lane. So it caught these errors. So going back to failures, there were two failures: 1. On SKL 6500 this was because the machine in CI lab is a SKL desktop without eDP on Port A. But our VBT initialization code in the driver writes VBT defaults in a way that it always sets DP flag on Port A and this does not get cleared after parsing the VBT outputs. So now there is a fix for this by Jani Nikula in the driver which is merged. "drm/i915/vbt: defaults handling for no VBT case" 2. On ILK desktop - This was happening because of a bad monitor desktop combination. I switched the monitor in the CI lab and that helped get rid of the link failures on ILK system. So now looks like the fixes have gone in to fix the link failures thrown by this patch. Should I resend this patch now? Regards Manasi On Thu, Mar 02, 2017 at 09:17:47AM +0100, Daniel Vetter wrote: > On Wed, Mar 01, 2017 at 06:17:49PM +0100, Daniel Vetter wrote: > > This reverts commit 233ce881dd91fb13eb6b09deefae33168e6ead4c. > > > > I assumed it's ok, but really should have double-checked - CI caught > > tons of fail :( > > > > Cc: Jani Nikula <jani.nikula@intel.com> > > Cc: Manasi Navare <manasi.d.navare@intel.com> > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > Pushed with Manasi' ack on irc. > -Daniel > > > --- > > drivers/gpu/drm/i915/intel_dp.c | 27 --------------------------- > > drivers/gpu/drm/i915/intel_dp_link_training.c | 22 ++-------------------- > > drivers/gpu/drm/i915/intel_drv.h | 3 --- > > 3 files changed, 2 insertions(+), 50 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > > index af07a830fa95..d1670b8afbf5 100644 > > --- a/drivers/gpu/drm/i915/intel_dp.c > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > @@ -5790,29 +5790,6 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, > > return false; > > } > > > > -static void intel_dp_modeset_retry_work_fn(struct work_struct *work) > > -{ > > - struct intel_connector *intel_connector; > > - struct drm_connector *connector; > > - > > - intel_connector = container_of(work, typeof(*intel_connector), > > - modeset_retry_work); > > - connector = &intel_connector->base; > > - DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", connector->base.id, > > - connector->name); > > - > > - /* Grab the locks before changing connector property*/ > > - mutex_lock(&connector->dev->mode_config.mutex); > > - /* Set connector link status to BAD and send a Uevent to notify > > - * userspace to do a modeset. > > - */ > > - drm_mode_connector_set_link_status_property(connector, > > - DRM_MODE_LINK_STATUS_BAD); > > - mutex_unlock(&connector->dev->mode_config.mutex); > > - /* Send Hotplug uevent so userspace can reprobe */ > > - drm_kms_helper_hotplug_event(connector->dev); > > -} > > - > > bool > > intel_dp_init_connector(struct intel_digital_port *intel_dig_port, > > struct intel_connector *intel_connector) > > @@ -5825,10 +5802,6 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port, > > enum port port = intel_dig_port->port; > > int type; > > > > - /* Initialize the work for modeset in case of link train failure */ > > - INIT_WORK(&intel_connector->modeset_retry_work, > > - intel_dp_modeset_retry_work_fn); > > - > > if (WARN(intel_dig_port->max_lanes < 1, > > "Not enough lanes (%d) for DP on port %c\n", > > intel_dig_port->max_lanes, port_name(port))) > > diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c > > index 955b239e7c2d..0048b520baf7 100644 > > --- a/drivers/gpu/drm/i915/intel_dp_link_training.c > > +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c > > @@ -313,24 +313,6 @@ void intel_dp_stop_link_train(struct intel_dp *intel_dp) > > void > > intel_dp_start_link_train(struct intel_dp *intel_dp) > > { > > - struct intel_connector *intel_connector = intel_dp->attached_connector; > > - > > - if (!intel_dp_link_training_clock_recovery(intel_dp)) > > - goto failure_handling; > > - if (!intel_dp_link_training_channel_equalization(intel_dp)) > > - goto failure_handling; > > - > > - DRM_DEBUG_KMS("Link Training Passed at Link Rate = %d, Lane count = %d", > > - intel_dp->link_rate, intel_dp->lane_count); > > - return; > > - > > - failure_handling: > > - DRM_DEBUG_KMS("Link Training failed at link rate = %d, lane count = %d", > > - intel_dp->link_rate, intel_dp->lane_count); > > - if (!intel_dp_get_link_train_fallback_values(intel_dp, > > - intel_dp->link_rate, > > - intel_dp->lane_count)) > > - /* Schedule a Hotplug Uevent to userspace to start modeset */ > > - schedule_work(&intel_connector->modeset_retry_work); > > - return; > > + intel_dp_link_training_clock_recovery(intel_dp); > > + intel_dp_link_training_channel_equalization(intel_dp); > > } > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > index 6c6abc932726..3b797909f631 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -316,9 +316,6 @@ struct intel_connector { > > void *port; /* store this opaque as its illegal to dereference it */ > > > > struct intel_dp *mst_port; > > - > > - /* Work struct to schedule a uevent on link train failure */ > > - struct work_struct modeset_retry_work; > > }; > > > > struct dpll { > > -- > > 2.11.0 > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index af07a830fa95..d1670b8afbf5 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -5790,29 +5790,6 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, return false; } -static void intel_dp_modeset_retry_work_fn(struct work_struct *work) -{ - struct intel_connector *intel_connector; - struct drm_connector *connector; - - intel_connector = container_of(work, typeof(*intel_connector), - modeset_retry_work); - connector = &intel_connector->base; - DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", connector->base.id, - connector->name); - - /* Grab the locks before changing connector property*/ - mutex_lock(&connector->dev->mode_config.mutex); - /* Set connector link status to BAD and send a Uevent to notify - * userspace to do a modeset. - */ - drm_mode_connector_set_link_status_property(connector, - DRM_MODE_LINK_STATUS_BAD); - mutex_unlock(&connector->dev->mode_config.mutex); - /* Send Hotplug uevent so userspace can reprobe */ - drm_kms_helper_hotplug_event(connector->dev); -} - bool intel_dp_init_connector(struct intel_digital_port *intel_dig_port, struct intel_connector *intel_connector) @@ -5825,10 +5802,6 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port, enum port port = intel_dig_port->port; int type; - /* Initialize the work for modeset in case of link train failure */ - INIT_WORK(&intel_connector->modeset_retry_work, - intel_dp_modeset_retry_work_fn); - if (WARN(intel_dig_port->max_lanes < 1, "Not enough lanes (%d) for DP on port %c\n", intel_dig_port->max_lanes, port_name(port))) diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c index 955b239e7c2d..0048b520baf7 100644 --- a/drivers/gpu/drm/i915/intel_dp_link_training.c +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c @@ -313,24 +313,6 @@ void intel_dp_stop_link_train(struct intel_dp *intel_dp) void intel_dp_start_link_train(struct intel_dp *intel_dp) { - struct intel_connector *intel_connector = intel_dp->attached_connector; - - if (!intel_dp_link_training_clock_recovery(intel_dp)) - goto failure_handling; - if (!intel_dp_link_training_channel_equalization(intel_dp)) - goto failure_handling; - - DRM_DEBUG_KMS("Link Training Passed at Link Rate = %d, Lane count = %d", - intel_dp->link_rate, intel_dp->lane_count); - return; - - failure_handling: - DRM_DEBUG_KMS("Link Training failed at link rate = %d, lane count = %d", - intel_dp->link_rate, intel_dp->lane_count); - if (!intel_dp_get_link_train_fallback_values(intel_dp, - intel_dp->link_rate, - intel_dp->lane_count)) - /* Schedule a Hotplug Uevent to userspace to start modeset */ - schedule_work(&intel_connector->modeset_retry_work); - return; + intel_dp_link_training_clock_recovery(intel_dp); + intel_dp_link_training_channel_equalization(intel_dp); } diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 6c6abc932726..3b797909f631 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -316,9 +316,6 @@ struct intel_connector { void *port; /* store this opaque as its illegal to dereference it */ struct intel_dp *mst_port; - - /* Work struct to schedule a uevent on link train failure */ - struct work_struct modeset_retry_work; }; struct dpll {
This reverts commit 233ce881dd91fb13eb6b09deefae33168e6ead4c. I assumed it's ok, but really should have double-checked - CI caught tons of fail :( Cc: Jani Nikula <jani.nikula@intel.com> Cc: Manasi Navare <manasi.d.navare@intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> --- drivers/gpu/drm/i915/intel_dp.c | 27 --------------------------- drivers/gpu/drm/i915/intel_dp_link_training.c | 22 ++-------------------- drivers/gpu/drm/i915/intel_drv.h | 3 --- 3 files changed, 2 insertions(+), 50 deletions(-)