All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/2] drm/atomic: move waiting for hw_done to a helper
@ 2017-06-14  9:08 Maarten Lankhorst
  2017-06-14  9:08 ` [PATCH v2 2/2] drm/i915: Wait for pending modesets to complete before trying link training, v2 Maarten Lankhorst
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Maarten Lankhorst @ 2017-06-14  9:08 UTC (permalink / raw)
  To: intel-gfx; +Cc: dri-devel

Moving the wait to a helper allows callers outside of the core to
wait for pending updates to complete.

This can be used to prevent races against nonblocking modesets.
Only the hw_done call in swap_state is moved to a helper, doing
it for the other callers requires too many changes and I think this
is the only useful one to export.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/drm_atomic_helper.c | 51 +++++++++++++++++++++++++++----------
 include/drm/drm_atomic_helper.h     |  1 +
 2 files changed, 38 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 86d3093c6c9b..1b068ce1aea9 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -2067,6 +2067,41 @@ void drm_atomic_helper_cleanup_planes(struct drm_device *dev,
 EXPORT_SYMBOL(drm_atomic_helper_cleanup_planes);
 
 /**
+ * drm_atomic_helper_wait_for_hw_done - wait for all previous hw updates to complete.
+ * @crtc: crtc to wait on.
+ *
+ * This function waits for all previous hardware updates queued on @crtc
+ * to complete.
+ *
+ * Returns:
+ * 0 on success, negative error code on failure.
+ */
+int drm_atomic_helper_wait_for_hw_done(struct drm_crtc *crtc)
+{
+	struct drm_crtc_commit *commit;
+	long ret;
+
+	spin_lock(&crtc->commit_lock);
+	commit = list_first_entry_or_null(&crtc->commit_list,
+			struct drm_crtc_commit, commit_entry);
+	if (commit)
+		drm_crtc_commit_get(commit);
+	spin_unlock(&crtc->commit_lock);
+
+	if (!commit)
+		return 0;
+
+	ret = wait_for_completion_timeout(&commit->hw_done, 10 * HZ);
+	drm_crtc_commit_put(commit);
+
+	if (ret == 0)
+		return -EBUSY;
+
+	return ret > 0 ? 0 : ret;
+}
+EXPORT_SYMBOL(drm_atomic_helper_wait_for_hw_done);
+
+/**
  * drm_atomic_helper_swap_state - store atomic state into current sw state
  * @state: atomic state
  * @stall: stall for proceeding commits
@@ -2107,28 +2142,16 @@ void drm_atomic_helper_swap_state(struct drm_atomic_state *state,
 	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
 	struct drm_plane *plane;
 	struct drm_plane_state *old_plane_state, *new_plane_state;
-	struct drm_crtc_commit *commit;
 	void *obj, *obj_state;
 	const struct drm_private_state_funcs *funcs;
 
 	if (stall) {
 		for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
-			spin_lock(&crtc->commit_lock);
-			commit = list_first_entry_or_null(&crtc->commit_list,
-					struct drm_crtc_commit, commit_entry);
-			if (commit)
-				drm_crtc_commit_get(commit);
-			spin_unlock(&crtc->commit_lock);
+			ret = drm_atomic_helper_wait_for_hw_done(crtc);
 
-			if (!commit)
-				continue;
-
-			ret = wait_for_completion_timeout(&commit->hw_done,
-							  10*HZ);
-			if (ret == 0)
+			if (ret < 0)
 				DRM_ERROR("[CRTC:%d:%s] hw_done timed out\n",
 					  crtc->base.id, crtc->name);
-			drm_crtc_commit_put(commit);
 		}
 	}
 
diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
index f0a8678ae98e..175a1a3fed36 100644
--- a/include/drm/drm_atomic_helper.h
+++ b/include/drm/drm_atomic_helper.h
@@ -77,6 +77,7 @@ void
 drm_atomic_helper_disable_planes_on_crtc(struct drm_crtc_state *old_crtc_state,
 					 bool atomic);
 
+int drm_atomic_helper_wait_for_hw_done(struct drm_crtc *crtc);
 void drm_atomic_helper_swap_state(struct drm_atomic_state *state,
 				  bool stall);
 
-- 
2.11.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 2/2] drm/i915: Wait for pending modesets to complete before trying link training, v2.
  2017-06-14  9:08 [PATCH v2 1/2] drm/atomic: move waiting for hw_done to a helper Maarten Lankhorst
@ 2017-06-14  9:08 ` Maarten Lankhorst
  2017-06-14 13:44   ` Ville Syrjälä
  2017-06-14  9:27 ` ✓ Fi.CI.BAT: success for series starting with [v2,1/2] drm/atomic: move waiting for hw_done to a helper Patchwork
  2017-06-14 14:52 ` [PATCH v2 1/2] " Ville Syrjälä
  2 siblings, 1 reply; 8+ messages in thread
From: Maarten Lankhorst @ 2017-06-14  9:08 UTC (permalink / raw)
  To: intel-gfx; +Cc: dri-devel

The nonblocking modeset tests were failing on systems with a DP output,
because lijnk training could occur during the modeset.

With normal modesets we're protected by the connection_mutex lock,
but nonblocking modesets drop locks before completing. To get around
that, check if the most recent crtc_state has a modeset, and if this
is the case, wait for all outstanding atomic updates on that crtc
to complete.

This fixes the following spew when running nonblocking tests:
[ 1061.653778] ------------[ cut here ]------------
[ 1061.653781] WARNING: CPU: 1 PID: 8165 at
drivers/gpu/drm/drm_irq.c:1217 drm_wait_one_vblank+0x165/0x1a0
[ 1061.653782] vblank not available on crtc 0, ret=-22
[ 1061.653783] Modules linked in: vgem snd_hda_intel i915
snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic
x86_pkg_temp_thermal intel_powerclamp coretemp crct10dif_pclmul
crc32_pclmul ghash_clmulni_intel snd_hda_codec snd_hwdep snd_hda_core
snd_pcm mei_me mei e1000e ptp pps_core i2c_hid [last unloaded: vgem]
[ 1061.653796] CPU: 1 PID: 8165 Comm: kworker/1:3 Tainted: G     U  W
4.10.0-rc1-CI-Custom_114+ #1
[ 1061.653797] Hardware name: GIGABYTE GB-BKi5A-7200/MFLP5AP-00, BIOS F1
07/27/2016
[ 1061.653812] Workqueue: events i915_hpd_poll_init_work [i915]
[ 1061.653813] Call Trace:
[ 1061.653816]  dump_stack+0x67/0x92
[ 1061.653818]  __warn+0xc6/0xe0
[ 1061.653819]  warn_slowpath_fmt+0x4a/0x50
[ 1061.653820]  ? drm_vblank_get+0x74/0xc0
[ 1061.653821]  drm_wait_one_vblank+0x165/0x1a0
[ 1061.653823]  ? drm_dp_dpcd_write+0x16/0x20
[ 1061.653837]  ? intel_dp_set_link_train+0x44/0xe0 [i915]
[ 1061.653851]  intel_dp_check_link_status+0x113/0x1a0 [i915]
[ 1061.653864]  intel_dp_detect+0x5a1/0xa40 [i915]
[ 1061.653866]  drm_helper_hpd_irq_event+0x8d/0x110
[ 1061.653880]  i915_hpd_poll_init_work+0xab/0xd0 [i915]
[ 1061.653882]  process_one_work+0x1f4/0x6d0
[ 1061.653883]  ? process_one_work+0x16e/0x6d0
[ 1061.653885]  worker_thread+0x49/0x4a0
[ 1061.653886]  kthread+0x107/0x140
[ 1061.653888]  ? process_one_work+0x6d0/0x6d0
[ 1061.653889]  ? kthread_create_on_node+0x40/0x40
[ 1061.653891]  ret_from_fork+0x27/0x40
[ 1061.653892] ---[ end trace f6037618668d4cbc ]---
[ 1061.656353] [drm:intel_dp_set_idle_link_train [i915]] *ERROR* Timed out waiting for DP idle patterns

Changes since v1:
- intel_dp_detect() now holds connection_mutex and has an acquire_ctx,
  rewrite patch to take advantage of it.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99272
Testcase: kms_atomic_transitions.1x-modeset-transitions-nonblocking
---
 drivers/gpu/drm/i915/intel_dp.c | 72 ++++++++++++++++++++++++++++++++++-------
 1 file changed, 60 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 64fa774c855b..c7a66883e453 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4243,21 +4243,32 @@ static void
 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 intel_connector *intel_connector = intel_dp->attached_connector;
+	struct drm_crtc *crtc;
 
-	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
-
-	if (!intel_dp_get_link_status(intel_dp, link_status)) {
-		DRM_ERROR("Failed to get link status\n");
+	crtc = intel_connector->base.state->crtc;
+	if (!crtc)
 		return;
-	}
 
-	if (!intel_encoder->base.crtc)
+	WARN_ON(!drm_modeset_is_locked(&crtc->mutex));
+
+	if (!crtc->state->active)
 		return;
 
-	if (!to_intel_crtc(intel_encoder->base.crtc)->active)
+	if (drm_atomic_crtc_needs_modeset(crtc->state)) {
+		int ret;
+
+		/* 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");
+	}
+
+	if (!intel_dp_get_link_status(intel_dp, link_status)) {
+		DRM_ERROR("Failed to get link status\n");
 		return;
+	}
 
 	/*
 	 * Validate the cached values of intel_dp->link_rate and
@@ -4291,7 +4302,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);
 	struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base;
 	u8 sink_irq_vector = 0;
 	u8 old_sink_count = intel_dp->sink_count;
@@ -4331,9 +4341,8 @@ 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);
+
 	if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING) {
 		DRM_DEBUG_KMS("Link Training Compliance Test requested\n");
 		/* Send a Hotplug Uevent to userspace to start modeset */
@@ -4735,8 +4744,19 @@ intel_dp_detect(struct drm_connector *connector,
 		      connector->base.id, connector->name);
 
 	/* If full detect is not performed yet, do a full detect */
-	if (!intel_dp->detect_done)
+	if (!intel_dp->detect_done) {
+		struct drm_crtc *crtc;
+		int ret;
+
+		crtc = connector->state->crtc;
+		if (crtc) {
+			ret = drm_modeset_lock(&crtc->mutex, ctx);
+			if (ret)
+				return ret;
+		}
+
 		status = intel_dp_long_pulse(intel_dp->attached_connector);
+	}
 
 	intel_dp->detect_done = false;
 
@@ -5028,10 +5048,38 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
 	}
 
 	if (!intel_dp->is_mst) {
+		struct drm_modeset_acquire_ctx ctx;
+		struct drm_connector *connector = &intel_dp->attached_connector->base;
+		struct drm_crtc *crtc;
+		int iret;
+
+		drm_modeset_acquire_init(&ctx, 0);
+retry:
+		iret = drm_modeset_lock(&dev->mode_config.connection_mutex, &ctx);
+		if (iret)
+			goto err;
+
+		crtc = connector->state->crtc;
+		if (crtc) {
+			iret = drm_modeset_lock(&crtc->mutex, &ctx);
+			if (iret)
+				goto err;
+		}
+
 		if (!intel_dp_short_pulse(intel_dp)) {
 			intel_dp->detect_done = false;
 			goto put_power;
 		}
+
+err:
+		if (iret == -EDEADLK) {
+			drm_modeset_backoff(&ctx);
+			goto retry;
+		}
+
+		drm_modeset_drop_locks(&ctx);
+		drm_modeset_acquire_fini(&ctx);
+		WARN(iret, "Acquiring modeset locks failed with %i\n", iret);
 	}
 
 	ret = IRQ_HANDLED;
-- 
2.11.0

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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* ✓ Fi.CI.BAT: success for series starting with [v2,1/2] drm/atomic: move waiting for hw_done to a helper
  2017-06-14  9:08 [PATCH v2 1/2] drm/atomic: move waiting for hw_done to a helper Maarten Lankhorst
  2017-06-14  9:08 ` [PATCH v2 2/2] drm/i915: Wait for pending modesets to complete before trying link training, v2 Maarten Lankhorst
@ 2017-06-14  9:27 ` Patchwork
  2017-06-14 14:52 ` [PATCH v2 1/2] " Ville Syrjälä
  2 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2017-06-14  9:27 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v2,1/2] drm/atomic: move waiting for hw_done to a helper
URL   : https://patchwork.freedesktop.org/series/25761/
State : success

== Summary ==

Series 25761v1 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/25761/revisions/1/mbox/

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time:447s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time:432s
fi-bsw-n3050     total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  time:580s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:506s
fi-byt-j1900     total:278  pass:254  dwarn:0   dfail:0   fail:0   skip:24  time:484s
fi-byt-n2820     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:482s
fi-glk-2a        total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:586s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:434s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:418s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time:417s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:495s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:471s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:466s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:454s
fi-skl-6700hq    total:278  pass:228  dwarn:1   dfail:0   fail:27  skip:22  time:401s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time:462s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:475s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time:434s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:532s
fi-snb-2600      total:278  pass:248  dwarn:0   dfail:0   fail:1   skip:29  time:401s

d4bedb8b0f9ba91df2e8cb136a489145a83e96a7 drm-tip: 2017y-06m-13d-14h-22m-46s UTC integration manifest
b79675f drm/i915: Wait for pending modesets to complete before trying link training, v2.
48dd8f5 drm/atomic: move waiting for hw_done to a helper

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4949/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 2/2] drm/i915: Wait for pending modesets to complete before trying link training, v2.
  2017-06-14  9:08 ` [PATCH v2 2/2] drm/i915: Wait for pending modesets to complete before trying link training, v2 Maarten Lankhorst
@ 2017-06-14 13:44   ` Ville Syrjälä
  2017-06-26  9:24     ` Maarten Lankhorst
  0 siblings, 1 reply; 8+ messages in thread
From: Ville Syrjälä @ 2017-06-14 13:44 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: intel-gfx, dri-devel

On Wed, Jun 14, 2017 at 11:08:42AM +0200, Maarten Lankhorst wrote:
> The nonblocking modeset tests were failing on systems with a DP output,
> because lijnk training could occur during the modeset.
> 
> With normal modesets we're protected by the connection_mutex lock,
> but nonblocking modesets drop locks before completing. To get around
> that, check if the most recent crtc_state has a modeset, and if this
> is the case, wait for all outstanding atomic updates on that crtc
> to complete.
> 
> This fixes the following spew when running nonblocking tests:
> [ 1061.653778] ------------[ cut here ]------------
> [ 1061.653781] WARNING: CPU: 1 PID: 8165 at
> drivers/gpu/drm/drm_irq.c:1217 drm_wait_one_vblank+0x165/0x1a0
> [ 1061.653782] vblank not available on crtc 0, ret=-22
> [ 1061.653783] Modules linked in: vgem snd_hda_intel i915
> snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic
> x86_pkg_temp_thermal intel_powerclamp coretemp crct10dif_pclmul
> crc32_pclmul ghash_clmulni_intel snd_hda_codec snd_hwdep snd_hda_core
> snd_pcm mei_me mei e1000e ptp pps_core i2c_hid [last unloaded: vgem]
> [ 1061.653796] CPU: 1 PID: 8165 Comm: kworker/1:3 Tainted: G     U  W
> 4.10.0-rc1-CI-Custom_114+ #1
> [ 1061.653797] Hardware name: GIGABYTE GB-BKi5A-7200/MFLP5AP-00, BIOS F1
> 07/27/2016
> [ 1061.653812] Workqueue: events i915_hpd_poll_init_work [i915]
> [ 1061.653813] Call Trace:
> [ 1061.653816]  dump_stack+0x67/0x92
> [ 1061.653818]  __warn+0xc6/0xe0
> [ 1061.653819]  warn_slowpath_fmt+0x4a/0x50
> [ 1061.653820]  ? drm_vblank_get+0x74/0xc0
> [ 1061.653821]  drm_wait_one_vblank+0x165/0x1a0
> [ 1061.653823]  ? drm_dp_dpcd_write+0x16/0x20
> [ 1061.653837]  ? intel_dp_set_link_train+0x44/0xe0 [i915]
> [ 1061.653851]  intel_dp_check_link_status+0x113/0x1a0 [i915]
> [ 1061.653864]  intel_dp_detect+0x5a1/0xa40 [i915]
> [ 1061.653866]  drm_helper_hpd_irq_event+0x8d/0x110
> [ 1061.653880]  i915_hpd_poll_init_work+0xab/0xd0 [i915]
> [ 1061.653882]  process_one_work+0x1f4/0x6d0
> [ 1061.653883]  ? process_one_work+0x16e/0x6d0
> [ 1061.653885]  worker_thread+0x49/0x4a0
> [ 1061.653886]  kthread+0x107/0x140
> [ 1061.653888]  ? process_one_work+0x6d0/0x6d0
> [ 1061.653889]  ? kthread_create_on_node+0x40/0x40
> [ 1061.653891]  ret_from_fork+0x27/0x40
> [ 1061.653892] ---[ end trace f6037618668d4cbc ]---
> [ 1061.656353] [drm:intel_dp_set_idle_link_train [i915]] *ERROR* Timed out waiting for DP idle patterns
> 
> Changes since v1:
> - intel_dp_detect() now holds connection_mutex and has an acquire_ctx,
>   rewrite patch to take advantage of it.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99272
> Testcase: kms_atomic_transitions.1x-modeset-transitions-nonblocking
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 72 ++++++++++++++++++++++++++++++++++-------
>  1 file changed, 60 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 64fa774c855b..c7a66883e453 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4243,21 +4243,32 @@ static void
>  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 intel_connector *intel_connector = intel_dp->attached_connector;
> +	struct drm_crtc *crtc;
>  
> -	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
> -
> -	if (!intel_dp_get_link_status(intel_dp, link_status)) {
> -		DRM_ERROR("Failed to get link status\n");
> +	crtc = intel_connector->base.state->crtc;
> +	if (!crtc)
>  		return;
> -	}
>  
> -	if (!intel_encoder->base.crtc)
> +	WARN_ON(!drm_modeset_is_locked(&crtc->mutex));
> +
> +	if (!crtc->state->active)
>  		return;
>  
> -	if (!to_intel_crtc(intel_encoder->base.crtc)->active)
> +	if (drm_atomic_crtc_needs_modeset(crtc->state)) {
> +		int ret;
> +
> +		/* 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");
> +	}
> +
> +	if (!intel_dp_get_link_status(intel_dp, link_status)) {
> +		DRM_ERROR("Failed to get link status\n");
>  		return;
> +	}
>  
>  	/*
>  	 * Validate the cached values of intel_dp->link_rate and
> @@ -4291,7 +4302,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);
>  	struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base;
>  	u8 sink_irq_vector = 0;
>  	u8 old_sink_count = intel_dp->sink_count;
> @@ -4331,9 +4341,8 @@ 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);
> +
>  	if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING) {
>  		DRM_DEBUG_KMS("Link Training Compliance Test requested\n");
>  		/* Send a Hotplug Uevent to userspace to start modeset */
> @@ -4735,8 +4744,19 @@ intel_dp_detect(struct drm_connector *connector,
>  		      connector->base.id, connector->name);
>  
>  	/* If full detect is not performed yet, do a full detect */
> -	if (!intel_dp->detect_done)
> +	if (!intel_dp->detect_done) {
> +		struct drm_crtc *crtc;
> +		int ret;
> +
> +		crtc = connector->state->crtc;
> +		if (crtc) {
> +			ret = drm_modeset_lock(&crtc->mutex, ctx);
> +			if (ret)
> +				return ret;
> +		}
> +
>  		status = intel_dp_long_pulse(intel_dp->attached_connector);
> +	}
>  
>  	intel_dp->detect_done = false;
>  
> @@ -5028,10 +5048,38 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
>  	}
>  
>  	if (!intel_dp->is_mst) {
> +		struct drm_modeset_acquire_ctx ctx;
> +		struct drm_connector *connector = &intel_dp->attached_connector->base;
> +		struct drm_crtc *crtc;
> +		int iret;
> +
> +		drm_modeset_acquire_init(&ctx, 0);
> +retry:
> +		iret = drm_modeset_lock(&dev->mode_config.connection_mutex, &ctx);
> +		if (iret)
> +			goto err;
> +
> +		crtc = connector->state->crtc;
> +		if (crtc) {
> +			iret = drm_modeset_lock(&crtc->mutex, &ctx);

Please no modeset locks in the dig_work. Hmm. Looks like we already have
it there though :( I thought we had moved the link retraining fully into
the hoplug_work, but I guess not. Can you try to do that instead?

> +			if (iret)
> +				goto err;
> +		}
> +
>  		if (!intel_dp_short_pulse(intel_dp)) {
>  			intel_dp->detect_done = false;
>  			goto put_power;
>  		}
> +
> +err:
> +		if (iret == -EDEADLK) {
> +			drm_modeset_backoff(&ctx);
> +			goto retry;
> +		}
> +
> +		drm_modeset_drop_locks(&ctx);
> +		drm_modeset_acquire_fini(&ctx);
> +		WARN(iret, "Acquiring modeset locks failed with %i\n", iret);
>  	}
>  
>  	ret = IRQ_HANDLED;
> -- 
> 2.11.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/2] drm/atomic: move waiting for hw_done to a helper
  2017-06-14  9:08 [PATCH v2 1/2] drm/atomic: move waiting for hw_done to a helper Maarten Lankhorst
  2017-06-14  9:08 ` [PATCH v2 2/2] drm/i915: Wait for pending modesets to complete before trying link training, v2 Maarten Lankhorst
  2017-06-14  9:27 ` ✓ Fi.CI.BAT: success for series starting with [v2,1/2] drm/atomic: move waiting for hw_done to a helper Patchwork
@ 2017-06-14 14:52 ` Ville Syrjälä
  2017-06-14 18:11   ` [Intel-gfx] " Maarten Lankhorst
  2 siblings, 1 reply; 8+ messages in thread
From: Ville Syrjälä @ 2017-06-14 14:52 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: intel-gfx, dri-devel

On Wed, Jun 14, 2017 at 11:08:41AM +0200, Maarten Lankhorst wrote:
> Moving the wait to a helper allows callers outside of the core to
> wait for pending updates to complete.
> 
> This can be used to prevent races against nonblocking modesets.
> Only the hw_done call in swap_state is moved to a helper, doing
> it for the other callers requires too many changes and I think this
> is the only useful one to export.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 51 +++++++++++++++++++++++++++----------
>  include/drm/drm_atomic_helper.h     |  1 +
>  2 files changed, 38 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 86d3093c6c9b..1b068ce1aea9 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -2067,6 +2067,41 @@ void drm_atomic_helper_cleanup_planes(struct drm_device *dev,
>  EXPORT_SYMBOL(drm_atomic_helper_cleanup_planes);
>  
>  /**
> + * drm_atomic_helper_wait_for_hw_done - wait for all previous hw updates to complete.
> + * @crtc: crtc to wait on.
> + *
> + * This function waits for all previous hardware updates queued on @crtc
> + * to complete.
> + *
> + * Returns:
> + * 0 on success, negative error code on failure.
> + */
> +int drm_atomic_helper_wait_for_hw_done(struct drm_crtc *crtc)
> +{
> +	struct drm_crtc_commit *commit;
> +	long ret;
> +
> +	spin_lock(&crtc->commit_lock);
> +	commit = list_first_entry_or_null(&crtc->commit_list,
> +			struct drm_crtc_commit, commit_entry);
> +	if (commit)
> +		drm_crtc_commit_get(commit);
> +	spin_unlock(&crtc->commit_lock);
> +
> +	if (!commit)
> +		return 0;
> +
> +	ret = wait_for_completion_timeout(&commit->hw_done, 10 * HZ);
> +	drm_crtc_commit_put(commit);
> +
> +	if (ret == 0)
> +		return -EBUSY;

-ETIMEDOUT ?

> +
> +	return ret > 0 ? 0 : ret;
> +}
> +EXPORT_SYMBOL(drm_atomic_helper_wait_for_hw_done);
> +
> +/**
>   * drm_atomic_helper_swap_state - store atomic state into current sw state
>   * @state: atomic state
>   * @stall: stall for proceeding commits
> @@ -2107,28 +2142,16 @@ void drm_atomic_helper_swap_state(struct drm_atomic_state *state,
>  	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
>  	struct drm_plane *plane;
>  	struct drm_plane_state *old_plane_state, *new_plane_state;
> -	struct drm_crtc_commit *commit;
>  	void *obj, *obj_state;
>  	const struct drm_private_state_funcs *funcs;
>  
>  	if (stall) {
>  		for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
> -			spin_lock(&crtc->commit_lock);
> -			commit = list_first_entry_or_null(&crtc->commit_list,
> -					struct drm_crtc_commit, commit_entry);
> -			if (commit)
> -				drm_crtc_commit_get(commit);
> -			spin_unlock(&crtc->commit_lock);
> +			ret = drm_atomic_helper_wait_for_hw_done(crtc);
>  
> -			if (!commit)
> -				continue;
> -
> -			ret = wait_for_completion_timeout(&commit->hw_done,
> -							  10*HZ);
> -			if (ret == 0)
> +			if (ret < 0)
>  				DRM_ERROR("[CRTC:%d:%s] hw_done timed out\n",
>  					  crtc->base.id, crtc->name);
> -			drm_crtc_commit_put(commit);
>  		}
>  	}
>  
> diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
> index f0a8678ae98e..175a1a3fed36 100644
> --- a/include/drm/drm_atomic_helper.h
> +++ b/include/drm/drm_atomic_helper.h
> @@ -77,6 +77,7 @@ void
>  drm_atomic_helper_disable_planes_on_crtc(struct drm_crtc_state *old_crtc_state,
>  					 bool atomic);
>  
> +int drm_atomic_helper_wait_for_hw_done(struct drm_crtc *crtc);
>  void drm_atomic_helper_swap_state(struct drm_atomic_state *state,
>  				  bool stall);
>  
> -- 
> 2.11.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Intel-gfx] [PATCH v2 1/2] drm/atomic: move waiting for hw_done to a helper
  2017-06-14 14:52 ` [PATCH v2 1/2] " Ville Syrjälä
@ 2017-06-14 18:11   ` Maarten Lankhorst
  2017-06-14 18:35     ` Ville Syrjälä
  0 siblings, 1 reply; 8+ messages in thread
From: Maarten Lankhorst @ 2017-06-14 18:11 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx, dri-devel

Op 14-06-17 om 16:52 schreef Ville Syrjälä:
> On Wed, Jun 14, 2017 at 11:08:41AM +0200, Maarten Lankhorst wrote:
>> Moving the wait to a helper allows callers outside of the core to
>> wait for pending updates to complete.
>>
>> This can be used to prevent races against nonblocking modesets.
>> Only the hw_done call in swap_state is moved to a helper, doing
>> it for the other callers requires too many changes and I think this
>> is the only useful one to export.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>>  drivers/gpu/drm/drm_atomic_helper.c | 51 +++++++++++++++++++++++++++----------
>>  include/drm/drm_atomic_helper.h     |  1 +
>>  2 files changed, 38 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
>> index 86d3093c6c9b..1b068ce1aea9 100644
>> --- a/drivers/gpu/drm/drm_atomic_helper.c
>> +++ b/drivers/gpu/drm/drm_atomic_helper.c
>> @@ -2067,6 +2067,41 @@ void drm_atomic_helper_cleanup_planes(struct drm_device *dev,
>>  EXPORT_SYMBOL(drm_atomic_helper_cleanup_planes);
>>  
>>  /**
>> + * drm_atomic_helper_wait_for_hw_done - wait for all previous hw updates to complete.
>> + * @crtc: crtc to wait on.
>> + *
>> + * This function waits for all previous hardware updates queued on @crtc
>> + * to complete.
>> + *
>> + * Returns:
>> + * 0 on success, negative error code on failure.
>> + */
>> +int drm_atomic_helper_wait_for_hw_done(struct drm_crtc *crtc)
>> +{
>> +	struct drm_crtc_commit *commit;
>> +	long ret;
>> +
>> +	spin_lock(&crtc->commit_lock);
>> +	commit = list_first_entry_or_null(&crtc->commit_list,
>> +			struct drm_crtc_commit, commit_entry);
>> +	if (commit)
>> +		drm_crtc_commit_get(commit);
>> +	spin_unlock(&crtc->commit_lock);
>> +
>> +	if (!commit)
>> +		return 0;
>> +
>> +	ret = wait_for_completion_timeout(&commit->hw_done, 10 * HZ);
>> +	drm_crtc_commit_put(commit);
>> +
>> +	if (ret == 0)
>> +		return -EBUSY;
> -ETIMEDOUT ?
ETIMEDOUT -> /* Connection timed out */

No I don't think that would match.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Intel-gfx] [PATCH v2 1/2] drm/atomic: move waiting for hw_done to a helper
  2017-06-14 18:11   ` [Intel-gfx] " Maarten Lankhorst
@ 2017-06-14 18:35     ` Ville Syrjälä
  0 siblings, 0 replies; 8+ messages in thread
From: Ville Syrjälä @ 2017-06-14 18:35 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: intel-gfx, dri-devel

On Wed, Jun 14, 2017 at 08:11:18PM +0200, Maarten Lankhorst wrote:
> Op 14-06-17 om 16:52 schreef Ville Syrjälä:
> > On Wed, Jun 14, 2017 at 11:08:41AM +0200, Maarten Lankhorst wrote:
> >> Moving the wait to a helper allows callers outside of the core to
> >> wait for pending updates to complete.
> >>
> >> This can be used to prevent races against nonblocking modesets.
> >> Only the hw_done call in swap_state is moved to a helper, doing
> >> it for the other callers requires too many changes and I think this
> >> is the only useful one to export.
> >>
> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >> ---
> >>  drivers/gpu/drm/drm_atomic_helper.c | 51 +++++++++++++++++++++++++++----------
> >>  include/drm/drm_atomic_helper.h     |  1 +
> >>  2 files changed, 38 insertions(+), 14 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> >> index 86d3093c6c9b..1b068ce1aea9 100644
> >> --- a/drivers/gpu/drm/drm_atomic_helper.c
> >> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> >> @@ -2067,6 +2067,41 @@ void drm_atomic_helper_cleanup_planes(struct drm_device *dev,
> >>  EXPORT_SYMBOL(drm_atomic_helper_cleanup_planes);
> >>  
> >>  /**
> >> + * drm_atomic_helper_wait_for_hw_done - wait for all previous hw updates to complete.
> >> + * @crtc: crtc to wait on.
> >> + *
> >> + * This function waits for all previous hardware updates queued on @crtc
> >> + * to complete.
> >> + *
> >> + * Returns:
> >> + * 0 on success, negative error code on failure.
> >> + */
> >> +int drm_atomic_helper_wait_for_hw_done(struct drm_crtc *crtc)
> >> +{
> >> +	struct drm_crtc_commit *commit;
> >> +	long ret;
> >> +
> >> +	spin_lock(&crtc->commit_lock);
> >> +	commit = list_first_entry_or_null(&crtc->commit_list,
> >> +			struct drm_crtc_commit, commit_entry);
> >> +	if (commit)
> >> +		drm_crtc_commit_get(commit);
> >> +	spin_unlock(&crtc->commit_lock);
> >> +
> >> +	if (!commit)
> >> +		return 0;
> >> +
> >> +	ret = wait_for_completion_timeout(&commit->hw_done, 10 * HZ);
> >> +	drm_crtc_commit_put(commit);
> >> +
> >> +	if (ret == 0)
> >> +		return -EBUSY;
> > -ETIMEDOUT ?
> ETIMEDOUT -> /* Connection timed out */

It's the standard "something timed out" errno. Used all over drm.

> 
> No I don't think that would match.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 2/2] drm/i915: Wait for pending modesets to complete before trying link training, v2.
  2017-06-14 13:44   ` Ville Syrjälä
@ 2017-06-26  9:24     ` Maarten Lankhorst
  0 siblings, 0 replies; 8+ messages in thread
From: Maarten Lankhorst @ 2017-06-26  9:24 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx, dri-devel

Op 14-06-17 om 15:44 schreef Ville Syrjälä:
> On Wed, Jun 14, 2017 at 11:08:42AM +0200, Maarten Lankhorst wrote:
>> The nonblocking modeset tests were failing on systems with a DP output,
>> because lijnk training could occur during the modeset.
>>
>> With normal modesets we're protected by the connection_mutex lock,
>> but nonblocking modesets drop locks before completing. To get around
>> that, check if the most recent crtc_state has a modeset, and if this
>> is the case, wait for all outstanding atomic updates on that crtc
>> to complete.
>>
>> This fixes the following spew when running nonblocking tests:
>> [ 1061.653778] ------------[ cut here ]------------
>> [ 1061.653781] WARNING: CPU: 1 PID: 8165 at
>> drivers/gpu/drm/drm_irq.c:1217 drm_wait_one_vblank+0x165/0x1a0
>> [ 1061.653782] vblank not available on crtc 0, ret=-22
>> [ 1061.653783] Modules linked in: vgem snd_hda_intel i915
>> snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic
>> x86_pkg_temp_thermal intel_powerclamp coretemp crct10dif_pclmul
>> crc32_pclmul ghash_clmulni_intel snd_hda_codec snd_hwdep snd_hda_core
>> snd_pcm mei_me mei e1000e ptp pps_core i2c_hid [last unloaded: vgem]
>> [ 1061.653796] CPU: 1 PID: 8165 Comm: kworker/1:3 Tainted: G     U  W
>> 4.10.0-rc1-CI-Custom_114+ #1
>> [ 1061.653797] Hardware name: GIGABYTE GB-BKi5A-7200/MFLP5AP-00, BIOS F1
>> 07/27/2016
>> [ 1061.653812] Workqueue: events i915_hpd_poll_init_work [i915]
>> [ 1061.653813] Call Trace:
>> [ 1061.653816]  dump_stack+0x67/0x92
>> [ 1061.653818]  __warn+0xc6/0xe0
>> [ 1061.653819]  warn_slowpath_fmt+0x4a/0x50
>> [ 1061.653820]  ? drm_vblank_get+0x74/0xc0
>> [ 1061.653821]  drm_wait_one_vblank+0x165/0x1a0
>> [ 1061.653823]  ? drm_dp_dpcd_write+0x16/0x20
>> [ 1061.653837]  ? intel_dp_set_link_train+0x44/0xe0 [i915]
>> [ 1061.653851]  intel_dp_check_link_status+0x113/0x1a0 [i915]
>> [ 1061.653864]  intel_dp_detect+0x5a1/0xa40 [i915]
>> [ 1061.653866]  drm_helper_hpd_irq_event+0x8d/0x110
>> [ 1061.653880]  i915_hpd_poll_init_work+0xab/0xd0 [i915]
>> [ 1061.653882]  process_one_work+0x1f4/0x6d0
>> [ 1061.653883]  ? process_one_work+0x16e/0x6d0
>> [ 1061.653885]  worker_thread+0x49/0x4a0
>> [ 1061.653886]  kthread+0x107/0x140
>> [ 1061.653888]  ? process_one_work+0x6d0/0x6d0
>> [ 1061.653889]  ? kthread_create_on_node+0x40/0x40
>> [ 1061.653891]  ret_from_fork+0x27/0x40
>> [ 1061.653892] ---[ end trace f6037618668d4cbc ]---
>> [ 1061.656353] [drm:intel_dp_set_idle_link_train [i915]] *ERROR* Timed out waiting for DP idle patterns
>>
>> Changes since v1:
>> - intel_dp_detect() now holds connection_mutex and has an acquire_ctx,
>>   rewrite patch to take advantage of it.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99272
>> Testcase: kms_atomic_transitions.1x-modeset-transitions-nonblocking
>> ---
>>  drivers/gpu/drm/i915/intel_dp.c | 72 ++++++++++++++++++++++++++++++++++-------
>>  1 file changed, 60 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index 64fa774c855b..c7a66883e453 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -4243,21 +4243,32 @@ static void
>>  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 intel_connector *intel_connector = intel_dp->attached_connector;
>> +	struct drm_crtc *crtc;
>>  
>> -	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
>> -
>> -	if (!intel_dp_get_link_status(intel_dp, link_status)) {
>> -		DRM_ERROR("Failed to get link status\n");
>> +	crtc = intel_connector->base.state->crtc;
>> +	if (!crtc)
>>  		return;
>> -	}
>>  
>> -	if (!intel_encoder->base.crtc)
>> +	WARN_ON(!drm_modeset_is_locked(&crtc->mutex));
>> +
>> +	if (!crtc->state->active)
>>  		return;
>>  
>> -	if (!to_intel_crtc(intel_encoder->base.crtc)->active)
>> +	if (drm_atomic_crtc_needs_modeset(crtc->state)) {
>> +		int ret;
>> +
>> +		/* 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");
>> +	}
>> +
>> +	if (!intel_dp_get_link_status(intel_dp, link_status)) {
>> +		DRM_ERROR("Failed to get link status\n");
>>  		return;
>> +	}
>>  
>>  	/*
>>  	 * Validate the cached values of intel_dp->link_rate and
>> @@ -4291,7 +4302,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);
>>  	struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base;
>>  	u8 sink_irq_vector = 0;
>>  	u8 old_sink_count = intel_dp->sink_count;
>> @@ -4331,9 +4341,8 @@ 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);
>> +
>>  	if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING) {
>>  		DRM_DEBUG_KMS("Link Training Compliance Test requested\n");
>>  		/* Send a Hotplug Uevent to userspace to start modeset */
>> @@ -4735,8 +4744,19 @@ intel_dp_detect(struct drm_connector *connector,
>>  		      connector->base.id, connector->name);
>>  
>>  	/* If full detect is not performed yet, do a full detect */
>> -	if (!intel_dp->detect_done)
>> +	if (!intel_dp->detect_done) {
>> +		struct drm_crtc *crtc;
>> +		int ret;
>> +
>> +		crtc = connector->state->crtc;
>> +		if (crtc) {
>> +			ret = drm_modeset_lock(&crtc->mutex, ctx);
>> +			if (ret)
>> +				return ret;
>> +		}
>> +
>>  		status = intel_dp_long_pulse(intel_dp->attached_connector);
>> +	}
>>  
>>  	intel_dp->detect_done = false;
>>  
>> @@ -5028,10 +5048,38 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
>>  	}
>>  
>>  	if (!intel_dp->is_mst) {
>> +		struct drm_modeset_acquire_ctx ctx;
>> +		struct drm_connector *connector = &intel_dp->attached_connector->base;
>> +		struct drm_crtc *crtc;
>> +		int iret;
>> +
>> +		drm_modeset_acquire_init(&ctx, 0);
>> +retry:
>> +		iret = drm_modeset_lock(&dev->mode_config.connection_mutex, &ctx);
>> +		if (iret)
>> +			goto err;
>> +
>> +		crtc = connector->state->crtc;
>> +		if (crtc) {
>> +			iret = drm_modeset_lock(&crtc->mutex, &ctx);
> Please no modeset locks in the dig_work. Hmm. Looks like we already have
> it there though :( I thought we had moved the link retraining fully into
> the hoplug_work, but I guess not. Can you try to do that instead?
I see you've had a series to fix this, what happened to it?
>> +			if (iret)
>> +				goto err;
>> +		}
>> +
>>  		if (!intel_dp_short_pulse(intel_dp)) {
>>  			intel_dp->detect_done = false;
>>  			goto put_power;
>>  		}
>> +
>> +err:
>> +		if (iret == -EDEADLK) {
>> +			drm_modeset_backoff(&ctx);
>> +			goto retry;
>> +		}
>> +
>> +		drm_modeset_drop_locks(&ctx);
>> +		drm_modeset_acquire_fini(&ctx);
>> +		WARN(iret, "Acquiring modeset locks failed with %i\n", iret);
>>  	}
>>  
>>  	ret = IRQ_HANDLED;
>> -- 
>> 2.11.0
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2017-06-26  9:24 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-14  9:08 [PATCH v2 1/2] drm/atomic: move waiting for hw_done to a helper Maarten Lankhorst
2017-06-14  9:08 ` [PATCH v2 2/2] drm/i915: Wait for pending modesets to complete before trying link training, v2 Maarten Lankhorst
2017-06-14 13:44   ` Ville Syrjälä
2017-06-26  9:24     ` Maarten Lankhorst
2017-06-14  9:27 ` ✓ Fi.CI.BAT: success for series starting with [v2,1/2] drm/atomic: move waiting for hw_done to a helper Patchwork
2017-06-14 14:52 ` [PATCH v2 1/2] " Ville Syrjälä
2017-06-14 18:11   ` [Intel-gfx] " Maarten Lankhorst
2017-06-14 18:35     ` Ville Syrjälä

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.