diff mbox

[2/3] drm/i915: Wait for pending modesets to complete before trying link training

Message ID 1483981618-10233-3-git-send-email-maarten.lankhorst@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Maarten Lankhorst Jan. 9, 2017, 5:06 p.m. UTC
We're protected by the connection_mutex lock against blocking modesets,
but nonblocking modesets are performed without any locking. This is
causing WARN in drm_wait_for_vblank and in general it's better to wait
before modeset completes before trying anything.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 51 +++++++++++++++++++++++++++++++----------
 1 file changed, 39 insertions(+), 12 deletions(-)

Comments

Ville Syrjälä Jan. 26, 2017, 6:31 p.m. UTC | #1
On Mon, Jan 09, 2017 at 06:06:57PM +0100, Maarten Lankhorst wrote:
> We're protected by the connection_mutex lock against blocking modesets,
> but nonblocking modesets are performed without any locking. This is
> causing WARN in drm_wait_for_vblank and in general it's better to wait
> before modeset completes before trying anything.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 51 +++++++++++++++++++++++++++++++----------
>  1 file changed, 39 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index fb12896bafee..71882d887393 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4080,24 +4080,38 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
>  	struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base;
>  	struct drm_device *dev = intel_dp_to_dev(intel_dp);
>  	u8 link_status[DP_LINK_STATUS_SIZE];
> +	struct drm_modeset_acquire_ctx ctx;
> +	struct intel_connector *intel_connector = intel_dp->attached_connector;
> +	int ret;
> +	struct drm_crtc *crtc;
>  
> -	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
> +	drm_modeset_acquire_init(&ctx, 0);
> +
> +retry:
> +	ret = drm_modeset_lock(&dev->mode_config.connection_mutex, &ctx);
> +	if (ret)
> +		goto out;
>  
>  	if (!intel_dp_get_link_status(intel_dp, link_status)) {
>  		DRM_ERROR("Failed to get link status\n");
> -		return;
> +		goto out;
>  	}
>  
> -	if (!intel_encoder->base.crtc)
> -		return;
> +	crtc = intel_connector->base.state->crtc;
> +	if (!crtc)
> +		goto out;
>  
> -	if (!to_intel_crtc(intel_encoder->base.crtc)->active)
> -		return;
> +	ret = drm_modeset_lock(&crtc->mutex, &ctx);
> +	if (ret)
> +		goto out;
> +
> +	if (!crtc->state->active)
> +		goto out;
>  
>  	/* FIXME: we need to synchronize this sort of stuff with hardware
>  	 * readout. Currently fast link training doesn't work on boot-up. */
>  	if (!intel_dp->lane_count)
> -		return;
> +		goto out;
>  
>  	/* if link training is requested we should perform it always */
>  	if ((intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING) ||
> @@ -4105,8 +4119,26 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
>  		DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
>  			      intel_encoder->base.name);
>  
> +		if (drm_atomic_crtc_needs_modeset(crtc->state)) {
> +			/* wait for atomic modeset to complete */
> +			ret = drm_atomic_helper_wait_for_hw_done(crtc);
> +			if (ret < 0)
> +				DRM_ERROR("Waiting for hw_done timed out\n");

I wonder if we should just skip the hw frobbing in this case. I guess it
might still restore the link if the commit just somehow got stuck
somewhere non-critical.

> +		}
> +
>  		intel_dp_retrain_link(intel_dp);
>  	}
> +
> +out:
> +	if (ret == -EDEADLK) {
> +		drm_modeset_backoff(&ctx);
> +		goto retry;
> +	}
> +
> +	drm_modeset_drop_locks(&ctx);
> +	drm_modeset_acquire_fini(&ctx);
> +	if (ret)
> +		DRM_ERROR("Locking failed with %i\n", ret);

This seems incorrect if we timed out on the wait. Maybe just
clear ret after the hw_done wait?

>  }
>  
>  /*
> @@ -4125,7 +4157,6 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
>  static bool
>  intel_dp_short_pulse(struct intel_dp *intel_dp)
>  {
> -	struct drm_device *dev = intel_dp_to_dev(intel_dp);
>  	u8 sink_irq_vector = 0;
>  	u8 old_sink_count = intel_dp->sink_count;
>  	bool ret;
> @@ -4164,9 +4195,7 @@ intel_dp_short_pulse(struct intel_dp *intel_dp)
>  			DRM_DEBUG_DRIVER("CP or sink specific irq unhandled\n");
>  	}
>  
> -	drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
>  	intel_dp_check_link_status(intel_dp);
> -	drm_modeset_unlock(&dev->mode_config.connection_mutex);
>  
>  	return true;
>  }
> @@ -4499,9 +4528,7 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
>  		 * check links status, there has been known issues of
>  		 * link loss triggerring long pulse!!!!
>  		 */
> -		drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
>  		intel_dp_check_link_status(intel_dp);
> -		drm_modeset_unlock(&dev->mode_config.connection_mutex);
>  		goto out;
>  	}
>  
> -- 
> 2.7.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Maarten Lankhorst Feb. 9, 2017, 11:27 a.m. UTC | #2
Op 26-01-17 om 19:31 schreef Ville Syrjälä:
> On Mon, Jan 09, 2017 at 06:06:57PM +0100, Maarten Lankhorst wrote:
>> We're protected by the connection_mutex lock against blocking modesets,
>> but nonblocking modesets are performed without any locking. This is
>> causing WARN in drm_wait_for_vblank and in general it's better to wait
>> before modeset completes before trying anything.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_dp.c | 51 +++++++++++++++++++++++++++++++----------
>>  1 file changed, 39 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index fb12896bafee..71882d887393 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -4080,24 +4080,38 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
>>  	struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base;
>>  	struct drm_device *dev = intel_dp_to_dev(intel_dp);
>>  	u8 link_status[DP_LINK_STATUS_SIZE];
>> +	struct drm_modeset_acquire_ctx ctx;
>> +	struct intel_connector *intel_connector = intel_dp->attached_connector;
>> +	int ret;
>> +	struct drm_crtc *crtc;
>>  
>> -	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
>> +	drm_modeset_acquire_init(&ctx, 0);
>> +
>> +retry:
>> +	ret = drm_modeset_lock(&dev->mode_config.connection_mutex, &ctx);
>> +	if (ret)
>> +		goto out;
>>  
>>  	if (!intel_dp_get_link_status(intel_dp, link_status)) {
>>  		DRM_ERROR("Failed to get link status\n");
>> -		return;
>> +		goto out;
>>  	}
>>  
>> -	if (!intel_encoder->base.crtc)
>> -		return;
>> +	crtc = intel_connector->base.state->crtc;
>> +	if (!crtc)
>> +		goto out;
>>  
>> -	if (!to_intel_crtc(intel_encoder->base.crtc)->active)
>> -		return;
>> +	ret = drm_modeset_lock(&crtc->mutex, &ctx);
>> +	if (ret)
>> +		goto out;
>> +
>> +	if (!crtc->state->active)
>> +		goto out;
>>  
>>  	/* FIXME: we need to synchronize this sort of stuff with hardware
>>  	 * readout. Currently fast link training doesn't work on boot-up. */
>>  	if (!intel_dp->lane_count)
>> -		return;
>> +		goto out;
>>  
>>  	/* if link training is requested we should perform it always */
>>  	if ((intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING) ||
>> @@ -4105,8 +4119,26 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
>>  		DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
>>  			      intel_encoder->base.name);
>>  
>> +		if (drm_atomic_crtc_needs_modeset(crtc->state)) {
>> +			/* wait for atomic modeset to complete */
>> +			ret = drm_atomic_helper_wait_for_hw_done(crtc);
>> +			if (ret < 0)
>> +				DRM_ERROR("Waiting for hw_done timed out\n");
> I wonder if we should just skip the hw frobbing in this case. I guess it
> might still restore the link if the commit just somehow got stuck
> somewhere non-critical.
Hm,  we could check if the link training is still required after the wait.
Maybe even do the wait unlocked, since it might take an arbitrary amount of time
and then retry?
>> +		}
>> +
>>  		intel_dp_retrain_link(intel_dp);
>>  	}
>> +
>> +out:
>> +	if (ret == -EDEADLK) {
>> +		drm_modeset_backoff(&ctx);
>> +		goto retry;
>> +	}
>> +
>> +	drm_modeset_drop_locks(&ctx);
>> +	drm_modeset_acquire_fini(&ctx);
>> +	if (ret)
>> +		DRM_ERROR("Locking failed with %i\n", ret);
> This seems incorrect if we timed out on the wait. Maybe just
> clear ret after the hw_done wait?
>
>>  }
>>  
>>  /*
>> @@ -4125,7 +4157,6 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
>>  static bool
>>  intel_dp_short_pulse(struct intel_dp *intel_dp)
>>  {
>> -	struct drm_device *dev = intel_dp_to_dev(intel_dp);
>>  	u8 sink_irq_vector = 0;
>>  	u8 old_sink_count = intel_dp->sink_count;
>>  	bool ret;
>> @@ -4164,9 +4195,7 @@ intel_dp_short_pulse(struct intel_dp *intel_dp)
>>  			DRM_DEBUG_DRIVER("CP or sink specific irq unhandled\n");
>>  	}
>>  
>> -	drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
>>  	intel_dp_check_link_status(intel_dp);
>> -	drm_modeset_unlock(&dev->mode_config.connection_mutex);
>>  
>>  	return true;
>>  }
>> @@ -4499,9 +4528,7 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
>>  		 * check links status, there has been known issues of
>>  		 * link loss triggerring long pulse!!!!
>>  		 */
>> -		drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
>>  		intel_dp_check_link_status(intel_dp);
>> -		drm_modeset_unlock(&dev->mode_config.connection_mutex);
>>  		goto out;
>>  	}
>>  
>> -- 
>> 2.7.4
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Ville Syrjälä Feb. 9, 2017, 12:34 p.m. UTC | #3
On Thu, Feb 09, 2017 at 12:27:12PM +0100, Maarten Lankhorst wrote:
> Op 26-01-17 om 19:31 schreef Ville Syrjälä:
> > On Mon, Jan 09, 2017 at 06:06:57PM +0100, Maarten Lankhorst wrote:
> >> We're protected by the connection_mutex lock against blocking modesets,
> >> but nonblocking modesets are performed without any locking. This is
> >> causing WARN in drm_wait_for_vblank and in general it's better to wait
> >> before modeset completes before trying anything.
> >>
> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/intel_dp.c | 51 +++++++++++++++++++++++++++++++----------
> >>  1 file changed, 39 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> >> index fb12896bafee..71882d887393 100644
> >> --- a/drivers/gpu/drm/i915/intel_dp.c
> >> +++ b/drivers/gpu/drm/i915/intel_dp.c
> >> @@ -4080,24 +4080,38 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
> >>  	struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base;
> >>  	struct drm_device *dev = intel_dp_to_dev(intel_dp);
> >>  	u8 link_status[DP_LINK_STATUS_SIZE];
> >> +	struct drm_modeset_acquire_ctx ctx;
> >> +	struct intel_connector *intel_connector = intel_dp->attached_connector;
> >> +	int ret;
> >> +	struct drm_crtc *crtc;
> >>  
> >> -	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
> >> +	drm_modeset_acquire_init(&ctx, 0);
> >> +
> >> +retry:
> >> +	ret = drm_modeset_lock(&dev->mode_config.connection_mutex, &ctx);
> >> +	if (ret)
> >> +		goto out;
> >>  
> >>  	if (!intel_dp_get_link_status(intel_dp, link_status)) {
> >>  		DRM_ERROR("Failed to get link status\n");
> >> -		return;
> >> +		goto out;
> >>  	}
> >>  
> >> -	if (!intel_encoder->base.crtc)
> >> -		return;
> >> +	crtc = intel_connector->base.state->crtc;
> >> +	if (!crtc)
> >> +		goto out;
> >>  
> >> -	if (!to_intel_crtc(intel_encoder->base.crtc)->active)
> >> -		return;
> >> +	ret = drm_modeset_lock(&crtc->mutex, &ctx);
> >> +	if (ret)
> >> +		goto out;
> >> +
> >> +	if (!crtc->state->active)
> >> +		goto out;
> >>  
> >>  	/* FIXME: we need to synchronize this sort of stuff with hardware
> >>  	 * readout. Currently fast link training doesn't work on boot-up. */
> >>  	if (!intel_dp->lane_count)
> >> -		return;
> >> +		goto out;
> >>  
> >>  	/* if link training is requested we should perform it always */
> >>  	if ((intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING) ||
> >> @@ -4105,8 +4119,26 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
> >>  		DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
> >>  			      intel_encoder->base.name);
> >>  
> >> +		if (drm_atomic_crtc_needs_modeset(crtc->state)) {
> >> +			/* wait for atomic modeset to complete */
> >> +			ret = drm_atomic_helper_wait_for_hw_done(crtc);
> >> +			if (ret < 0)
> >> +				DRM_ERROR("Waiting for hw_done timed out\n");
> > I wonder if we should just skip the hw frobbing in this case. I guess it
> > might still restore the link if the commit just somehow got stuck
> > somewhere non-critical.
> Hm,  we could check if the link training is still required after the wait.
> Maybe even do the wait unlocked, since it might take an arbitrary amount of time
> and then retry?

I'm not sure there's much point in trying to make this fancy. Link
problems should not happen with any regularity.


> >> +		}
> >> +
> >>  		intel_dp_retrain_link(intel_dp);
> >>  	}
> >> +
> >> +out:
> >> +	if (ret == -EDEADLK) {
> >> +		drm_modeset_backoff(&ctx);
> >> +		goto retry;
> >> +	}
> >> +
> >> +	drm_modeset_drop_locks(&ctx);
> >> +	drm_modeset_acquire_fini(&ctx);
> >> +	if (ret)
> >> +		DRM_ERROR("Locking failed with %i\n", ret);
> > This seems incorrect if we timed out on the wait. Maybe just
> > clear ret after the hw_done wait?
> >
> >>  }
> >>  
> >>  /*
> >> @@ -4125,7 +4157,6 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
> >>  static bool
> >>  intel_dp_short_pulse(struct intel_dp *intel_dp)
> >>  {
> >> -	struct drm_device *dev = intel_dp_to_dev(intel_dp);
> >>  	u8 sink_irq_vector = 0;
> >>  	u8 old_sink_count = intel_dp->sink_count;
> >>  	bool ret;
> >> @@ -4164,9 +4195,7 @@ intel_dp_short_pulse(struct intel_dp *intel_dp)
> >>  			DRM_DEBUG_DRIVER("CP or sink specific irq unhandled\n");
> >>  	}
> >>  
> >> -	drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
> >>  	intel_dp_check_link_status(intel_dp);
> >> -	drm_modeset_unlock(&dev->mode_config.connection_mutex);
> >>  
> >>  	return true;
> >>  }
> >> @@ -4499,9 +4528,7 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
> >>  		 * check links status, there has been known issues of
> >>  		 * link loss triggerring long pulse!!!!
> >>  		 */
> >> -		drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
> >>  		intel_dp_check_link_status(intel_dp);
> >> -		drm_modeset_unlock(&dev->mode_config.connection_mutex);
> >>  		goto out;
> >>  	}
> >>  
> >> -- 
> >> 2.7.4
> >>
> >> _______________________________________________
> >> dri-devel mailing list
> >> dri-devel@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
Maarten Lankhorst Feb. 9, 2017, 12:37 p.m. UTC | #4
Op 09-02-17 om 13:34 schreef Ville Syrjälä:
> On Thu, Feb 09, 2017 at 12:27:12PM +0100, Maarten Lankhorst wrote:
>> Op 26-01-17 om 19:31 schreef Ville Syrjälä:
>>> On Mon, Jan 09, 2017 at 06:06:57PM +0100, Maarten Lankhorst wrote:
>>>> We're protected by the connection_mutex lock against blocking modesets,
>>>> but nonblocking modesets are performed without any locking. This is
>>>> causing WARN in drm_wait_for_vblank and in general it's better to wait
>>>> before modeset completes before trying anything.
>>>>
>>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>>> ---
>>>>  drivers/gpu/drm/i915/intel_dp.c | 51 +++++++++++++++++++++++++++++++----------
>>>>  1 file changed, 39 insertions(+), 12 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>>>> index fb12896bafee..71882d887393 100644
>>>> --- a/drivers/gpu/drm/i915/intel_dp.c
>>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>>>> @@ -4080,24 +4080,38 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
>>>>  	struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base;
>>>>  	struct drm_device *dev = intel_dp_to_dev(intel_dp);
>>>>  	u8 link_status[DP_LINK_STATUS_SIZE];
>>>> +	struct drm_modeset_acquire_ctx ctx;
>>>> +	struct intel_connector *intel_connector = intel_dp->attached_connector;
>>>> +	int ret;
>>>> +	struct drm_crtc *crtc;
>>>>  
>>>> -	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
>>>> +	drm_modeset_acquire_init(&ctx, 0);
>>>> +
>>>> +retry:
>>>> +	ret = drm_modeset_lock(&dev->mode_config.connection_mutex, &ctx);
>>>> +	if (ret)
>>>> +		goto out;
>>>>  
>>>>  	if (!intel_dp_get_link_status(intel_dp, link_status)) {
>>>>  		DRM_ERROR("Failed to get link status\n");
>>>> -		return;
>>>> +		goto out;
>>>>  	}
>>>>  
>>>> -	if (!intel_encoder->base.crtc)
>>>> -		return;
>>>> +	crtc = intel_connector->base.state->crtc;
>>>> +	if (!crtc)
>>>> +		goto out;
>>>>  
>>>> -	if (!to_intel_crtc(intel_encoder->base.crtc)->active)
>>>> -		return;
>>>> +	ret = drm_modeset_lock(&crtc->mutex, &ctx);
>>>> +	if (ret)
>>>> +		goto out;
>>>> +
>>>> +	if (!crtc->state->active)
>>>> +		goto out;
>>>>  
>>>>  	/* FIXME: we need to synchronize this sort of stuff with hardware
>>>>  	 * readout. Currently fast link training doesn't work on boot-up. */
>>>>  	if (!intel_dp->lane_count)
>>>> -		return;
>>>> +		goto out;
>>>>  
>>>>  	/* if link training is requested we should perform it always */
>>>>  	if ((intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING) ||
>>>> @@ -4105,8 +4119,26 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
>>>>  		DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
>>>>  			      intel_encoder->base.name);
>>>>  
>>>> +		if (drm_atomic_crtc_needs_modeset(crtc->state)) {
>>>> +			/* wait for atomic modeset to complete */
>>>> +			ret = drm_atomic_helper_wait_for_hw_done(crtc);
>>>> +			if (ret < 0)
>>>> +				DRM_ERROR("Waiting for hw_done timed out\n");
>>> I wonder if we should just skip the hw frobbing in this case. I guess it
>>> might still restore the link if the commit just somehow got stuck
>>> somewhere non-critical.
>> Hm,  we could check if the link training is still required after the wait.
>> Maybe even do the wait unlocked, since it might take an arbitrary amount of time
>> and then retry?
> I'm not sure there's much point in trying to make this fancy. Link
> problems should not happen with any regularity.
Indeed, lets just do the simple thing then.
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index fb12896bafee..71882d887393 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4080,24 +4080,38 @@  intel_dp_check_link_status(struct intel_dp *intel_dp)
 	struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base;
 	struct drm_device *dev = intel_dp_to_dev(intel_dp);
 	u8 link_status[DP_LINK_STATUS_SIZE];
+	struct drm_modeset_acquire_ctx ctx;
+	struct intel_connector *intel_connector = intel_dp->attached_connector;
+	int ret;
+	struct drm_crtc *crtc;
 
-	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
+	drm_modeset_acquire_init(&ctx, 0);
+
+retry:
+	ret = drm_modeset_lock(&dev->mode_config.connection_mutex, &ctx);
+	if (ret)
+		goto out;
 
 	if (!intel_dp_get_link_status(intel_dp, link_status)) {
 		DRM_ERROR("Failed to get link status\n");
-		return;
+		goto out;
 	}
 
-	if (!intel_encoder->base.crtc)
-		return;
+	crtc = intel_connector->base.state->crtc;
+	if (!crtc)
+		goto out;
 
-	if (!to_intel_crtc(intel_encoder->base.crtc)->active)
-		return;
+	ret = drm_modeset_lock(&crtc->mutex, &ctx);
+	if (ret)
+		goto out;
+
+	if (!crtc->state->active)
+		goto out;
 
 	/* FIXME: we need to synchronize this sort of stuff with hardware
 	 * readout. Currently fast link training doesn't work on boot-up. */
 	if (!intel_dp->lane_count)
-		return;
+		goto out;
 
 	/* if link training is requested we should perform it always */
 	if ((intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING) ||
@@ -4105,8 +4119,26 @@  intel_dp_check_link_status(struct intel_dp *intel_dp)
 		DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
 			      intel_encoder->base.name);
 
+		if (drm_atomic_crtc_needs_modeset(crtc->state)) {
+			/* wait for atomic modeset to complete */
+			ret = drm_atomic_helper_wait_for_hw_done(crtc);
+			if (ret < 0)
+				DRM_ERROR("Waiting for hw_done timed out\n");
+		}
+
 		intel_dp_retrain_link(intel_dp);
 	}
+
+out:
+	if (ret == -EDEADLK) {
+		drm_modeset_backoff(&ctx);
+		goto retry;
+	}
+
+	drm_modeset_drop_locks(&ctx);
+	drm_modeset_acquire_fini(&ctx);
+	if (ret)
+		DRM_ERROR("Locking failed with %i\n", ret);
 }
 
 /*
@@ -4125,7 +4157,6 @@  intel_dp_check_link_status(struct intel_dp *intel_dp)
 static bool
 intel_dp_short_pulse(struct intel_dp *intel_dp)
 {
-	struct drm_device *dev = intel_dp_to_dev(intel_dp);
 	u8 sink_irq_vector = 0;
 	u8 old_sink_count = intel_dp->sink_count;
 	bool ret;
@@ -4164,9 +4195,7 @@  intel_dp_short_pulse(struct intel_dp *intel_dp)
 			DRM_DEBUG_DRIVER("CP or sink specific irq unhandled\n");
 	}
 
-	drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
 	intel_dp_check_link_status(intel_dp);
-	drm_modeset_unlock(&dev->mode_config.connection_mutex);
 
 	return true;
 }
@@ -4499,9 +4528,7 @@  intel_dp_long_pulse(struct intel_connector *intel_connector)
 		 * check links status, there has been known issues of
 		 * link loss triggerring long pulse!!!!
 		 */
-		drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
 		intel_dp_check_link_status(intel_dp);
-		drm_modeset_unlock(&dev->mode_config.connection_mutex);
 		goto out;
 	}