* [PATCH 1/2] drm/atomic-helper: always track connector commits, too
@ 2017-11-08 10:54 Daniel Vetter
2017-11-08 10:54 ` [PATCH 2/2] drm/i915: sync dp link status checks against atomic commmits Daniel Vetter
` (7 more replies)
0 siblings, 8 replies; 20+ messages in thread
From: Daniel Vetter @ 2017-11-08 10:54 UTC (permalink / raw)
To: DRI Development
Cc: Daniel Vetter, Intel Graphics Development, Manasi Navare, Daniel Vetter
It's useful for syncing async connector work like link retraining.
Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
drivers/gpu/drm/drm_atomic_helper.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 71d712f1b56a..ea781d55f2c1 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1791,7 +1791,8 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state,
if (new_conn_state->crtc)
continue;
- commit = crtc_or_fake_commit(state, old_conn_state->crtc);
+ /* Always track connectors explicitly for e.g. link retraining. */
+ commit = crtc_or_fake_commit(state, new_conn_state->crtc ?: old_conn_state->crtc);
if (!commit)
return -ENOMEM;
@@ -1805,10 +1806,7 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state,
!try_wait_for_completion(&old_plane_state->commit->flip_done))
return -EBUSY;
- /*
- * Unlike connectors, always track planes explicitly for
- * async pageflip support.
- */
+ /* Always track planes explicitly for async pageflip support. */
commit = crtc_or_fake_commit(state, new_plane_state->crtc ?: old_plane_state->crtc);
if (!commit)
return -ENOMEM;
--
2.15.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 2/2] drm/i915: sync dp link status checks against atomic commmits
2017-11-08 10:54 [PATCH 1/2] drm/atomic-helper: always track connector commits, too Daniel Vetter
@ 2017-11-08 10:54 ` Daniel Vetter
2017-11-08 12:57 ` [PATCH] " Daniel Vetter
2017-11-08 11:18 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/atomic-helper: always track connector commits, too Patchwork
` (6 subsequent siblings)
7 siblings, 1 reply; 20+ messages in thread
From: Daniel Vetter @ 2017-11-08 10:54 UTC (permalink / raw)
To: DRI Development; +Cc: Daniel Vetter, Intel Graphics Development, Daniel Vetter
Two bits:
- check actual atomic state, the legacy stuff can only be looked at
from within the atomic_commit_tail function, since it's only
protected by ordering and not by any locks.
- Make sure we don't wreak the work an ongoing nonblocking commit is
doing.
Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
References: https://bugs.freedesktop.org/show_bug.cgi?id=103336
References: https://bugs.freedesktop.org/show_bug.cgi?id=99272
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
drivers/gpu/drm/i915/intel_dp.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index d27c0145ac91..319c372dc26f 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4319,6 +4319,8 @@ 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_connector_state *conn_state =
+ intel_dp->attached_connector->base.state;
struct drm_device *dev = intel_dp_to_dev(intel_dp);
u8 link_status[DP_LINK_STATUS_SIZE];
@@ -4329,10 +4331,10 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
return;
}
- if (!intel_encoder->base.crtc)
+ if (!conn_state->crtc || !conn_state->crtc->state->active)
return;
- if (!to_intel_crtc(intel_encoder->base.crtc)->active)
+ if (!try_wait_for_completion(&conn_state->commit->hw_done))
return;
/*
--
2.15.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [1/2] drm/atomic-helper: always track connector commits, too
2017-11-08 10:54 [PATCH 1/2] drm/atomic-helper: always track connector commits, too Daniel Vetter
2017-11-08 10:54 ` [PATCH 2/2] drm/i915: sync dp link status checks against atomic commmits Daniel Vetter
@ 2017-11-08 11:18 ` Patchwork
2017-11-08 11:32 ` ✗ Fi.CI.BAT: failure " Patchwork
` (5 subsequent siblings)
7 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2017-11-08 11:18 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/atomic-helper: always track connector commits, too
URL : https://patchwork.freedesktop.org/series/33410/
State : success
== Summary ==
Series 33410v1 series starting with [1/2] drm/atomic-helper: always track connector commits, too
https://patchwork.freedesktop.org/api/1.0/series/33410/revisions/1/mbox/
fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:444s
fi-bdw-gvtdvm total:289 pass:264 dwarn:1 dfail:0 fail:0 skip:24 time:451s
fi-blb-e6850 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:385s
fi-bsw-n3050 total:289 pass:242 dwarn:1 dfail:0 fail:0 skip:46 time:534s
fi-bwr-2160 total:289 pass:183 dwarn:0 dfail:0 fail:0 skip:106 time:273s
fi-bxt-j4205 total:12 pass:2 dwarn:0 dfail:0 fail:0 skip:9
fi-byt-j1900 total:289 pass:254 dwarn:0 dfail:0 fail:0 skip:35 time:498s
fi-byt-n2820 total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:489s
fi-cnl-y total:12 pass:2 dwarn:0 dfail:0 fail:0 skip:9
fi-elk-e7500 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:428s
fi-gdg-551 total:289 pass:178 dwarn:1 dfail:0 fail:1 skip:109 time:258s
fi-glk-1 total:12 pass:2 dwarn:0 dfail:0 fail:0 skip:9
fi-glk-dsi total:289 pass:258 dwarn:0 dfail:0 fail:1 skip:30 time:488s
fi-hsw-4770 total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:431s
fi-hsw-4770r total:289 pass:225 dwarn:0 dfail:0 fail:0 skip:64 time:278s
fi-ilk-650 total:1 pass:0 dwarn:0 dfail:0 fail:0 skip:0
fi-ivb-3520m total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:485s
fi-ivb-3770 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:466s
fi-kbl-7500u total:1 pass:0 dwarn:0 dfail:0 fail:0 skip:0
fi-kbl-7567u total:1 pass:0 dwarn:0 dfail:0 fail:0 skip:0
fi-kbl-r total:12 pass:2 dwarn:0 dfail:0 fail:0 skip:9
fi-pnv-d510 total:289 pass:222 dwarn:1 dfail:0 fail:0 skip:66 time:572s
fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:457s
fi-skl-6600u total:1 pass:0 dwarn:0 dfail:0 fail:0 skip:0
fi-skl-6700hq total:1 pass:0 dwarn:0 dfail:0 fail:0 skip:0
fi-skl-6770hq total:1 pass:0 dwarn:0 dfail:0 fail:0 skip:0
fi-skl-gvtdvm total:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:463s
fi-snb-2520m total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:554s
fi-snb-2600 total:289 pass:249 dwarn:0 dfail:0 fail:0 skip:40 time:418s
fi-bxt-dsi failed to connect after reboot
748f2c6b4046b23a623b4af3799563ef3110bb0d drm-tip: 2017y-11m-08d-07h-50m-13s UTC integration manifest
080e074efe20 drm/i915: sync dp link status checks against atomic commmits
414f8018c5b8 drm/atomic-helper: always track connector commits, too
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7009/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* ✗ Fi.CI.BAT: failure for series starting with [1/2] drm/atomic-helper: always track connector commits, too
2017-11-08 10:54 [PATCH 1/2] drm/atomic-helper: always track connector commits, too Daniel Vetter
2017-11-08 10:54 ` [PATCH 2/2] drm/i915: sync dp link status checks against atomic commmits Daniel Vetter
2017-11-08 11:18 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/atomic-helper: always track connector commits, too Patchwork
@ 2017-11-08 11:32 ` Patchwork
2017-11-08 12:16 ` Patchwork
` (4 subsequent siblings)
7 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2017-11-08 11:32 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/atomic-helper: always track connector commits, too
URL : https://patchwork.freedesktop.org/series/33410/
State : failure
== Summary ==
Series 33410v1 series starting with [1/2] drm/atomic-helper: always track connector commits, too
https://patchwork.freedesktop.org/api/1.0/series/33410/revisions/1/mbox/
Test chamelium:
Subgroup dp-hpd-fast:
skip -> INCOMPLETE (fi-ilk-650)
skip -> INCOMPLETE (fi-skl-6600u)
skip -> INCOMPLETE (fi-skl-6700hq)
skip -> INCOMPLETE (fi-skl-6770hq)
pass -> INCOMPLETE (fi-kbl-7500u) fdo#102332 +1
skip -> INCOMPLETE (fi-kbl-7567u)
Test debugfs_test:
Subgroup read_all_entries:
pass -> INCOMPLETE (fi-bxt-j4205)
pass -> INCOMPLETE (fi-kbl-r)
pass -> INCOMPLETE (fi-glk-1)
pass -> INCOMPLETE (fi-cnl-y)
Test gem_ringfill:
Subgroup basic-default-fd:
pass -> DMESG-WARN (fi-bdw-gvtdvm)
Test kms_busy:
Subgroup basic-flip-a:
pass -> SKIP (fi-hsw-4770r)
Subgroup basic-flip-b:
pass -> SKIP (fi-hsw-4770r)
Subgroup basic-flip-c:
pass -> SKIP (fi-hsw-4770r)
Test kms_cursor_legacy:
Subgroup basic-busy-flip-before-cursor-atomic:
pass -> SKIP (fi-hsw-4770r)
Subgroup basic-busy-flip-before-cursor-legacy:
pass -> SKIP (fi-hsw-4770r)
Subgroup basic-flip-after-cursor-atomic:
pass -> SKIP (fi-hsw-4770r)
Subgroup basic-flip-after-cursor-legacy:
pass -> SKIP (fi-hsw-4770r)
Subgroup basic-flip-after-cursor-varying-size:
pass -> SKIP (fi-hsw-4770r)
Subgroup basic-flip-before-cursor-atomic:
pass -> SKIP (fi-hsw-4770r)
Subgroup basic-flip-before-cursor-legacy:
pass -> SKIP (fi-hsw-4770r)
Subgroup basic-flip-before-cursor-varying-size:
pass -> SKIP (fi-hsw-4770r)
Test kms_flip:
Subgroup basic-flip-vs-dpms:
pass -> SKIP (fi-hsw-4770r)
Subgroup basic-flip-vs-modeset:
pass -> SKIP (fi-hsw-4770r)
Subgroup basic-flip-vs-wf_vblank:
pass -> SKIP (fi-hsw-4770r)
Subgroup basic-plain-flip:
pass -> SKIP (fi-hsw-4770r)
Test kms_frontbuffer_tracking:
Subgroup basic:
pass -> SKIP (fi-hsw-4770r)
Test kms_pipe_crc_basic:
Subgroup hang-read-crc-pipe-a:
pass -> SKIP (fi-hsw-4770r)
Subgroup hang-read-crc-pipe-b:
pass -> SKIP (fi-hsw-4770r)
Subgroup hang-read-crc-pipe-c:
pass -> SKIP (fi-hsw-4770r)
Subgroup nonblocking-crc-pipe-a:
pass -> SKIP (fi-hsw-4770r)
Subgroup nonblocking-crc-pipe-a-frame-sequence:
pass -> SKIP (fi-hsw-4770r)
Subgroup nonblocking-crc-pipe-b:
pass -> SKIP (fi-hsw-4770r)
Subgroup nonblocking-crc-pipe-b-frame-sequence:
pass -> SKIP (fi-hsw-4770r)
Subgroup nonblocking-crc-pipe-c:
pass -> SKIP (fi-hsw-4770r)
Subgroup nonblocking-crc-pipe-c-frame-sequence:
pass -> SKIP (fi-hsw-4770r)
Subgroup read-crc-pipe-a:
pass -> SKIP (fi-hsw-4770r)
Subgroup read-crc-pipe-a-frame-sequence:
pass -> SKIP (fi-hsw-4770r)
Subgroup read-crc-pipe-b:
pass -> SKIP (fi-hsw-4770r)
Subgroup read-crc-pipe-c:
pass -> SKIP (fi-hsw-4770r)
Subgroup read-crc-pipe-c-frame-sequence:
pass -> SKIP (fi-hsw-4770r)
Subgroup suspend-read-crc-pipe-a:
pass -> SKIP (fi-hsw-4770r)
Subgroup suspend-read-crc-pipe-b:
pass -> SKIP (fi-hsw-4770r)
Subgroup suspend-read-crc-pipe-c:
pass -> SKIP (fi-hsw-4770r)
Test pm_rpm:
Subgroup basic-pci-d3-state:
pass -> SKIP (fi-hsw-4770r)
Subgroup basic-rte:
pass -> SKIP (fi-hsw-4770r)
Test prime_vgem:
Subgroup basic-fence-flip:
pass -> SKIP (fi-hsw-4770r)
Test drv_module_reload:
WARNING: Long output truncated
fi-bxt-dsi failed to connect after reboot
748f2c6b4046b23a623b4af3799563ef3110bb0d drm-tip: 2017y-11m-08d-07h-50m-13s UTC integration manifest
080e074efe20 drm/i915: sync dp link status checks against atomic commmits
414f8018c5b8 drm/atomic-helper: always track connector commits, too
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7009/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* ✗ Fi.CI.BAT: failure for series starting with [1/2] drm/atomic-helper: always track connector commits, too
2017-11-08 10:54 [PATCH 1/2] drm/atomic-helper: always track connector commits, too Daniel Vetter
` (2 preceding siblings ...)
2017-11-08 11:32 ` ✗ Fi.CI.BAT: failure " Patchwork
@ 2017-11-08 12:16 ` Patchwork
2017-11-08 15:14 ` ✗ Fi.CI.BAT: failure for series starting with [1/2] drm/atomic-helper: always track connector commits, too (rev2) Patchwork
` (3 subsequent siblings)
7 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2017-11-08 12:16 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/atomic-helper: always track connector commits, too
URL : https://patchwork.freedesktop.org/series/33410/
State : failure
== Summary ==
Series 33410v1 series starting with [1/2] drm/atomic-helper: always track connector commits, too
https://patchwork.freedesktop.org/api/1.0/series/33410/revisions/1/mbox/
Test chamelium:
Subgroup dp-hpd-fast:
skip -> INCOMPLETE (fi-ilk-650)
skip -> INCOMPLETE (fi-skl-6600u)
skip -> INCOMPLETE (fi-skl-6700hq)
skip -> INCOMPLETE (fi-skl-6770hq)
pass -> INCOMPLETE (fi-kbl-7500u) fdo#102332 +1
skip -> INCOMPLETE (fi-kbl-7567u)
Test debugfs_test:
Subgroup read_all_entries:
pass -> INCOMPLETE (fi-bxt-j4205)
pass -> INCOMPLETE (fi-kbl-r)
pass -> INCOMPLETE (fi-glk-1)
Test gem_ringfill:
Subgroup basic-default-fd:
pass -> DMESG-WARN (fi-bdw-gvtdvm)
Test kms_busy:
Subgroup basic-flip-a:
pass -> SKIP (fi-hsw-4770r)
Subgroup basic-flip-b:
pass -> SKIP (fi-hsw-4770r)
Subgroup basic-flip-c:
pass -> SKIP (fi-hsw-4770r)
Test kms_cursor_legacy:
Subgroup basic-busy-flip-before-cursor-atomic:
pass -> SKIP (fi-hsw-4770r)
Subgroup basic-busy-flip-before-cursor-legacy:
pass -> SKIP (fi-hsw-4770r)
Subgroup basic-flip-after-cursor-atomic:
pass -> SKIP (fi-hsw-4770r)
Subgroup basic-flip-after-cursor-legacy:
pass -> SKIP (fi-hsw-4770r)
Subgroup basic-flip-after-cursor-varying-size:
pass -> SKIP (fi-hsw-4770r)
Subgroup basic-flip-before-cursor-atomic:
pass -> SKIP (fi-hsw-4770r)
Subgroup basic-flip-before-cursor-legacy:
pass -> SKIP (fi-hsw-4770r)
Subgroup basic-flip-before-cursor-varying-size:
pass -> SKIP (fi-hsw-4770r)
Test kms_flip:
Subgroup basic-flip-vs-dpms:
pass -> SKIP (fi-hsw-4770r)
Subgroup basic-flip-vs-modeset:
pass -> SKIP (fi-hsw-4770r)
Subgroup basic-flip-vs-wf_vblank:
pass -> SKIP (fi-hsw-4770r)
Subgroup basic-plain-flip:
pass -> SKIP (fi-hsw-4770r)
Test kms_frontbuffer_tracking:
Subgroup basic:
pass -> SKIP (fi-hsw-4770r)
Test kms_pipe_crc_basic:
Subgroup hang-read-crc-pipe-a:
pass -> SKIP (fi-hsw-4770r)
Subgroup hang-read-crc-pipe-b:
pass -> SKIP (fi-hsw-4770r)
Subgroup hang-read-crc-pipe-c:
pass -> SKIP (fi-hsw-4770r)
Subgroup nonblocking-crc-pipe-a:
pass -> SKIP (fi-hsw-4770r)
Subgroup nonblocking-crc-pipe-a-frame-sequence:
pass -> SKIP (fi-hsw-4770r)
Subgroup nonblocking-crc-pipe-b:
pass -> SKIP (fi-hsw-4770r)
Subgroup nonblocking-crc-pipe-b-frame-sequence:
pass -> SKIP (fi-hsw-4770r)
Subgroup nonblocking-crc-pipe-c:
pass -> SKIP (fi-hsw-4770r)
Subgroup nonblocking-crc-pipe-c-frame-sequence:
pass -> SKIP (fi-hsw-4770r)
Subgroup read-crc-pipe-a:
pass -> SKIP (fi-hsw-4770r)
Subgroup read-crc-pipe-a-frame-sequence:
pass -> SKIP (fi-hsw-4770r)
Subgroup read-crc-pipe-b:
pass -> SKIP (fi-hsw-4770r)
Subgroup read-crc-pipe-c:
pass -> SKIP (fi-hsw-4770r)
Subgroup read-crc-pipe-c-frame-sequence:
pass -> SKIP (fi-hsw-4770r)
Subgroup suspend-read-crc-pipe-a:
pass -> SKIP (fi-hsw-4770r)
Subgroup suspend-read-crc-pipe-b:
pass -> SKIP (fi-hsw-4770r)
Subgroup suspend-read-crc-pipe-c:
pass -> SKIP (fi-hsw-4770r)
Test pm_rpm:
Subgroup basic-pci-d3-state:
pass -> SKIP (fi-hsw-4770r)
Subgroup basic-rte:
pass -> SKIP (fi-hsw-4770r)
Test prime_vgem:
Subgroup basic-fence-flip:
pass -> SKIP (fi-hsw-4770r)
Test drv_module_reload:
Subgroup basic-reload-inject:
WARNING: Long output truncated
fi-bxt-dsi failed to connect after reboot
748f2c6b4046b23a623b4af3799563ef3110bb0d drm-tip: 2017y-11m-08d-07h-50m-13s UTC integration manifest
080e074efe20 drm/i915: sync dp link status checks against atomic commmits
414f8018c5b8 drm/atomic-helper: always track connector commits, too
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7009/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH] drm/i915: sync dp link status checks against atomic commmits
2017-11-08 10:54 ` [PATCH 2/2] drm/i915: sync dp link status checks against atomic commmits Daniel Vetter
@ 2017-11-08 12:57 ` Daniel Vetter
2017-11-08 13:04 ` Ville Syrjälä
0 siblings, 1 reply; 20+ messages in thread
From: Daniel Vetter @ 2017-11-08 12:57 UTC (permalink / raw)
To: DRI Development
Cc: Daniel Vetter, Intel Graphics Development, Manasi Navare, Daniel Vetter
Two bits:
- check actual atomic state, the legacy stuff can only be looked at
from within the atomic_commit_tail function, since it's only
protected by ordering and not by any locks.
- Make sure we don't wreak the work an ongoing nonblocking commit is
doing.
v2: We need the crtc lock too, because a plane update might change it
without having to acquire the connection_mutex (Maarten). Use
Maarten's changes for this locking, while keeping the logic that uses
the connection->commit->hw_done signal for syncing with nonblocking
commits.
Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
References: https://bugs.freedesktop.org/show_bug.cgi?id=103336
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99272
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
drivers/gpu/drm/i915/intel_dp.c | 56 ++++++++++++++++++++++++++++++++++++-----
1 file changed, 50 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index d27c0145ac91..7cd7ab4fb431 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4319,6 +4319,8 @@ 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_connector_state *conn_state =
+ intel_dp->attached_connector->base.state;
struct drm_device *dev = intel_dp_to_dev(intel_dp);
u8 link_status[DP_LINK_STATUS_SIZE];
@@ -4329,10 +4331,15 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
return;
}
- if (!intel_encoder->base.crtc)
+ if (!conn_state->crtc)
return;
- if (!to_intel_crtc(intel_encoder->base.crtc)->active)
+ WARN_ON(!drm_modeset_is_locked(&conn_state->crtc->mutex));
+
+ if (!conn_state->crtc->state->active)
+ return;
+
+ if (!try_wait_for_completion(&conn_state->commit->hw_done))
return;
/*
@@ -4368,7 +4375,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;
@@ -4408,9 +4414,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 */
@@ -4860,8 +4865,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;
@@ -5146,10 +5162,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.15.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH] drm/i915: sync dp link status checks against atomic commmits
2017-11-08 12:57 ` [PATCH] " Daniel Vetter
@ 2017-11-08 13:04 ` Ville Syrjälä
2017-11-08 13:11 ` Daniel Vetter
2017-11-08 13:13 ` Maarten Lankhorst
0 siblings, 2 replies; 20+ messages in thread
From: Ville Syrjälä @ 2017-11-08 13:04 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Daniel Vetter, Intel Graphics Development, DRI Development
On Wed, Nov 08, 2017 at 01:57:50PM +0100, Daniel Vetter wrote:
> Two bits:
> - check actual atomic state, the legacy stuff can only be looked at
> from within the atomic_commit_tail function, since it's only
> protected by ordering and not by any locks.
>
> - Make sure we don't wreak the work an ongoing nonblocking commit is
> doing.
I still think we need to move the retraining to the hotplug work.
>
> v2: We need the crtc lock too, because a plane update might change it
> without having to acquire the connection_mutex (Maarten). Use
> Maarten's changes for this locking, while keeping the logic that uses
> the connection->commit->hw_done signal for syncing with nonblocking
> commits.
>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=103336
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99272
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
> drivers/gpu/drm/i915/intel_dp.c | 56 ++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 50 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index d27c0145ac91..7cd7ab4fb431 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4319,6 +4319,8 @@ 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_connector_state *conn_state =
> + intel_dp->attached_connector->base.state;
> struct drm_device *dev = intel_dp_to_dev(intel_dp);
> u8 link_status[DP_LINK_STATUS_SIZE];
>
> @@ -4329,10 +4331,15 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
> return;
> }
>
> - if (!intel_encoder->base.crtc)
> + if (!conn_state->crtc)
> return;
>
> - if (!to_intel_crtc(intel_encoder->base.crtc)->active)
> + WARN_ON(!drm_modeset_is_locked(&conn_state->crtc->mutex));
> +
> + if (!conn_state->crtc->state->active)
> + return;
> +
> + if (!try_wait_for_completion(&conn_state->commit->hw_done))
> return;
>
> /*
> @@ -4368,7 +4375,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;
> @@ -4408,9 +4414,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 */
> @@ -4860,8 +4865,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;
>
> @@ -5146,10 +5162,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.15.0
--
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] 20+ messages in thread
* Re: [PATCH] drm/i915: sync dp link status checks against atomic commmits
2017-11-08 13:04 ` Ville Syrjälä
@ 2017-11-08 13:11 ` Daniel Vetter
2017-11-08 13:26 ` Ville Syrjälä
2017-11-08 13:13 ` Maarten Lankhorst
1 sibling, 1 reply; 20+ messages in thread
From: Daniel Vetter @ 2017-11-08 13:11 UTC (permalink / raw)
To: Ville Syrjälä
Cc: Daniel Vetter, Intel Graphics Development, DRI Development,
Daniel Vetter
On Wed, Nov 08, 2017 at 03:04:58PM +0200, Ville Syrjälä wrote:
> On Wed, Nov 08, 2017 at 01:57:50PM +0100, Daniel Vetter wrote:
> > Two bits:
> > - check actual atomic state, the legacy stuff can only be looked at
> > from within the atomic_commit_tail function, since it's only
> > protected by ordering and not by any locks.
> >
> > - Make sure we don't wreak the work an ongoing nonblocking commit is
> > doing.
>
> I still think we need to move the retraining to the hotplug work.
Why? One of the patch series Maarten mentioned says there's a deadlock
with dp aux, but it seems to be all just fine when CI retrains.
And even if there is a deadlock, it's there already, so not sure why we
need to block this bugfix on a different bugfix. Which seems to be what
you're suggesting here (but it's a bit sparse).
-Daniel
> > v2: We need the crtc lock too, because a plane update might change it
> > without having to acquire the connection_mutex (Maarten). Use
> > Maarten's changes for this locking, while keeping the logic that uses
> > the connection->commit->hw_done signal for syncing with nonblocking
> > commits.
> >
> > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=103336
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99272
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_dp.c | 56 ++++++++++++++++++++++++++++++++++++-----
> > 1 file changed, 50 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index d27c0145ac91..7cd7ab4fb431 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -4319,6 +4319,8 @@ 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_connector_state *conn_state =
> > + intel_dp->attached_connector->base.state;
> > struct drm_device *dev = intel_dp_to_dev(intel_dp);
> > u8 link_status[DP_LINK_STATUS_SIZE];
> >
> > @@ -4329,10 +4331,15 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
> > return;
> > }
> >
> > - if (!intel_encoder->base.crtc)
> > + if (!conn_state->crtc)
> > return;
> >
> > - if (!to_intel_crtc(intel_encoder->base.crtc)->active)
> > + WARN_ON(!drm_modeset_is_locked(&conn_state->crtc->mutex));
> > +
> > + if (!conn_state->crtc->state->active)
> > + return;
> > +
> > + if (!try_wait_for_completion(&conn_state->commit->hw_done))
> > return;
> >
> > /*
> > @@ -4368,7 +4375,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;
> > @@ -4408,9 +4414,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 */
> > @@ -4860,8 +4865,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;
> >
> > @@ -5146,10 +5162,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.15.0
>
> --
> Ville Syrjälä
> Intel OTC
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] drm/i915: sync dp link status checks against atomic commmits
2017-11-08 13:04 ` Ville Syrjälä
2017-11-08 13:11 ` Daniel Vetter
@ 2017-11-08 13:13 ` Maarten Lankhorst
1 sibling, 0 replies; 20+ messages in thread
From: Maarten Lankhorst @ 2017-11-08 13:13 UTC (permalink / raw)
To: Ville Syrjälä, Daniel Vetter
Cc: Manasi Navare, Daniel Vetter, Intel Graphics Development,
DRI Development
Op 08-11-17 om 14:04 schreef Ville Syrjälä:
> On Wed, Nov 08, 2017 at 01:57:50PM +0100, Daniel Vetter wrote:
>> Two bits:
>> - check actual atomic state, the legacy stuff can only be looked at
>> from within the atomic_commit_tail function, since it's only
>> protected by ordering and not by any locks.
>>
>> - Make sure we don't wreak the work an ongoing nonblocking commit is
>> doing.
> I still think we need to move the retraining to the hotplug work.
But lets not block a fix for that. :)
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> v2: We need the crtc lock too, because a plane update might change it
>> without having to acquire the connection_mutex (Maarten). Use
>> Maarten's changes for this locking, while keeping the logic that uses
>> the connection->commit->hw_done signal for syncing with nonblocking
>> commits.
>>
>> Cc: Manasi Navare <manasi.d.navare@intel.com>
>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> References: https://bugs.freedesktop.org/show_bug.cgi?id=103336
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99272
>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>> ---
>> drivers/gpu/drm/i915/intel_dp.c | 56 ++++++++++++++++++++++++++++++++++++-----
>> 1 file changed, 50 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index d27c0145ac91..7cd7ab4fb431 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -4319,6 +4319,8 @@ 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_connector_state *conn_state =
>> + intel_dp->attached_connector->base.state;
>> struct drm_device *dev = intel_dp_to_dev(intel_dp);
>> u8 link_status[DP_LINK_STATUS_SIZE];
>>
>> @@ -4329,10 +4331,15 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
>> return;
>> }
>>
>> - if (!intel_encoder->base.crtc)
>> + if (!conn_state->crtc)
>> return;
>>
>> - if (!to_intel_crtc(intel_encoder->base.crtc)->active)
>> + WARN_ON(!drm_modeset_is_locked(&conn_state->crtc->mutex));
>> +
>> + if (!conn_state->crtc->state->active)
>> + return;
>> +
>> + if (!try_wait_for_completion(&conn_state->commit->hw_done))
>> return;
>>
>> /*
>> @@ -4368,7 +4375,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;
>> @@ -4408,9 +4414,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 */
>> @@ -4860,8 +4865,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;
>>
>> @@ -5146,10 +5162,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.15.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] drm/i915: sync dp link status checks against atomic commmits
2017-11-08 13:11 ` Daniel Vetter
@ 2017-11-08 13:26 ` Ville Syrjälä
2017-11-08 13:28 ` Daniel Vetter
0 siblings, 1 reply; 20+ messages in thread
From: Ville Syrjälä @ 2017-11-08 13:26 UTC (permalink / raw)
To: Daniel Vetter
Cc: Daniel Vetter, Intel Graphics Development, DRI Development,
Daniel Vetter
On Wed, Nov 08, 2017 at 02:11:46PM +0100, Daniel Vetter wrote:
> On Wed, Nov 08, 2017 at 03:04:58PM +0200, Ville Syrjälä wrote:
> > On Wed, Nov 08, 2017 at 01:57:50PM +0100, Daniel Vetter wrote:
> > > Two bits:
> > > - check actual atomic state, the legacy stuff can only be looked at
> > > from within the atomic_commit_tail function, since it's only
> > > protected by ordering and not by any locks.
> > >
> > > - Make sure we don't wreak the work an ongoing nonblocking commit is
> > > doing.
> >
> > I still think we need to move the retraining to the hotplug work.
>
> Why? One of the patch series Maarten mentioned says there's a deadlock
> with dp aux, but it seems to be all just fine when CI retrains.
I guess the deadlock is possible only with MST, maybe. Currently MST
has it's own copy of the retraining code without the locks.
At one point I started to rewrite the entire sink irq handling into a
much nicer shape, also unifying the approach between MST and SST. IIRC
I did still make the mistake of having some kind of higher level MST
vs. SST split, but I think the proper solution is to combine the two
almost entirely. I think we should even be using the ESI registers
with SST for DPCD 1.2+. Currently we use ESI for MST and non-ESI for
SST. Sadly I've not found the time to continue that work (same story
with the "shutdown displays prior to rebooting to keep Dell MST
monitors working" thing).
>
> And even if there is a deadlock, it's there already, so not sure why we
> need to block this bugfix on a different bugfix. Which seems to be what
> you're suggesting here (but it's a bit sparse).
I guess what I'm really saying is that we shouldn't stop here.
> -Daniel
>
> > > v2: We need the crtc lock too, because a plane update might change it
> > > without having to acquire the connection_mutex (Maarten). Use
> > > Maarten's changes for this locking, while keeping the logic that uses
> > > the connection->commit->hw_done signal for syncing with nonblocking
> > > commits.
> > >
> > > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > References: https://bugs.freedesktop.org/show_bug.cgi?id=103336
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99272
> > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > > ---
> > > drivers/gpu/drm/i915/intel_dp.c | 56 ++++++++++++++++++++++++++++++++++++-----
> > > 1 file changed, 50 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > index d27c0145ac91..7cd7ab4fb431 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -4319,6 +4319,8 @@ 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_connector_state *conn_state =
> > > + intel_dp->attached_connector->base.state;
> > > struct drm_device *dev = intel_dp_to_dev(intel_dp);
> > > u8 link_status[DP_LINK_STATUS_SIZE];
> > >
> > > @@ -4329,10 +4331,15 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
> > > return;
> > > }
> > >
> > > - if (!intel_encoder->base.crtc)
> > > + if (!conn_state->crtc)
> > > return;
> > >
> > > - if (!to_intel_crtc(intel_encoder->base.crtc)->active)
> > > + WARN_ON(!drm_modeset_is_locked(&conn_state->crtc->mutex));
> > > +
> > > + if (!conn_state->crtc->state->active)
> > > + return;
> > > +
> > > + if (!try_wait_for_completion(&conn_state->commit->hw_done))
> > > return;
> > >
> > > /*
> > > @@ -4368,7 +4375,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;
> > > @@ -4408,9 +4414,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 */
> > > @@ -4860,8 +4865,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;
> > >
> > > @@ -5146,10 +5162,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.15.0
> >
> > --
> > Ville Syrjälä
> > Intel OTC
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
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] 20+ messages in thread
* Re: [PATCH] drm/i915: sync dp link status checks against atomic commmits
2017-11-08 13:26 ` Ville Syrjälä
@ 2017-11-08 13:28 ` Daniel Vetter
2017-11-08 18:09 ` Manasi Navare
0 siblings, 1 reply; 20+ messages in thread
From: Daniel Vetter @ 2017-11-08 13:28 UTC (permalink / raw)
To: Ville Syrjälä
Cc: Daniel Vetter, Intel Graphics Development, DRI Development,
Daniel Vetter
On Wed, Nov 08, 2017 at 03:26:15PM +0200, Ville Syrjälä wrote:
> On Wed, Nov 08, 2017 at 02:11:46PM +0100, Daniel Vetter wrote:
> > On Wed, Nov 08, 2017 at 03:04:58PM +0200, Ville Syrjälä wrote:
> > > On Wed, Nov 08, 2017 at 01:57:50PM +0100, Daniel Vetter wrote:
> > > > Two bits:
> > > > - check actual atomic state, the legacy stuff can only be looked at
> > > > from within the atomic_commit_tail function, since it's only
> > > > protected by ordering and not by any locks.
> > > >
> > > > - Make sure we don't wreak the work an ongoing nonblocking commit is
> > > > doing.
> > >
> > > I still think we need to move the retraining to the hotplug work.
> >
> > Why? One of the patch series Maarten mentioned says there's a deadlock
> > with dp aux, but it seems to be all just fine when CI retrains.
>
> I guess the deadlock is possible only with MST, maybe. Currently MST
> has it's own copy of the retraining code without the locks.
>
> At one point I started to rewrite the entire sink irq handling into a
> much nicer shape, also unifying the approach between MST and SST. IIRC
> I did still make the mistake of having some kind of higher level MST
> vs. SST split, but I think the proper solution is to combine the two
> almost entirely. I think we should even be using the ESI registers
> with SST for DPCD 1.2+. Currently we use ESI for MST and non-ESI for
> SST. Sadly I've not found the time to continue that work (same story
> with the "shutdown displays prior to rebooting to keep Dell MST
> monitors working" thing).
Yeah, merging sst and mst more definitely sounds like a good idea. I've
also been toying with it since forever.
> > And even if there is a deadlock, it's there already, so not sure why we
> > need to block this bugfix on a different bugfix. Which seems to be what
> > you're suggesting here (but it's a bit sparse).
>
> I guess what I'm really saying is that we shouldn't stop here.
Fully agreed.
-Daniel
>
> > -Daniel
> >
> > > > v2: We need the crtc lock too, because a plane update might change it
> > > > without having to acquire the connection_mutex (Maarten). Use
> > > > Maarten's changes for this locking, while keeping the logic that uses
> > > > the connection->commit->hw_done signal for syncing with nonblocking
> > > > commits.
> > > >
> > > > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > References: https://bugs.freedesktop.org/show_bug.cgi?id=103336
> > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99272
> > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > > > ---
> > > > drivers/gpu/drm/i915/intel_dp.c | 56 ++++++++++++++++++++++++++++++++++++-----
> > > > 1 file changed, 50 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > > index d27c0145ac91..7cd7ab4fb431 100644
> > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > @@ -4319,6 +4319,8 @@ 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_connector_state *conn_state =
> > > > + intel_dp->attached_connector->base.state;
> > > > struct drm_device *dev = intel_dp_to_dev(intel_dp);
> > > > u8 link_status[DP_LINK_STATUS_SIZE];
> > > >
> > > > @@ -4329,10 +4331,15 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
> > > > return;
> > > > }
> > > >
> > > > - if (!intel_encoder->base.crtc)
> > > > + if (!conn_state->crtc)
> > > > return;
> > > >
> > > > - if (!to_intel_crtc(intel_encoder->base.crtc)->active)
> > > > + WARN_ON(!drm_modeset_is_locked(&conn_state->crtc->mutex));
> > > > +
> > > > + if (!conn_state->crtc->state->active)
> > > > + return;
> > > > +
> > > > + if (!try_wait_for_completion(&conn_state->commit->hw_done))
> > > > return;
> > > >
> > > > /*
> > > > @@ -4368,7 +4375,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;
> > > > @@ -4408,9 +4414,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 */
> > > > @@ -4860,8 +4865,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;
> > > >
> > > > @@ -5146,10 +5162,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.15.0
> > >
> > > --
> > > Ville Syrjälä
> > > Intel OTC
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
>
> --
> Ville Syrjälä
> Intel OTC
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* ✗ Fi.CI.BAT: failure for series starting with [1/2] drm/atomic-helper: always track connector commits, too (rev2)
2017-11-08 10:54 [PATCH 1/2] drm/atomic-helper: always track connector commits, too Daniel Vetter
` (3 preceding siblings ...)
2017-11-08 12:16 ` Patchwork
@ 2017-11-08 15:14 ` Patchwork
2017-11-08 15:21 ` [PATCH 1/2] drm/atomic-helper: always track connector commits, too Ville Syrjälä
` (2 subsequent siblings)
7 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2017-11-08 15:14 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/atomic-helper: always track connector commits, too (rev2)
URL : https://patchwork.freedesktop.org/series/33410/
State : failure
== Summary ==
Series 33410v2 series starting with [1/2] drm/atomic-helper: always track connector commits, too
https://patchwork.freedesktop.org/api/1.0/series/33410/revisions/2/mbox/
Test chamelium:
Subgroup dp-hpd-fast:
skip -> INCOMPLETE (fi-ilk-650)
skip -> INCOMPLETE (fi-skl-6600u)
skip -> INCOMPLETE (fi-skl-6700hq)
skip -> INCOMPLETE (fi-skl-6770hq)
pass -> INCOMPLETE (fi-kbl-7500u) fdo#102332
Test debugfs_test:
Subgroup read_all_entries:
pass -> INCOMPLETE (fi-bxt-j4205)
pass -> INCOMPLETE (fi-kbl-7567u)
pass -> INCOMPLETE (fi-kbl-r)
pass -> INCOMPLETE (fi-glk-1)
fdo#102332 https://bugs.freedesktop.org/show_bug.cgi?id=102332
fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:443s
fi-blb-e6850 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:379s
fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:536s
fi-bwr-2160 total:289 pass:183 dwarn:0 dfail:0 fail:0 skip:106 time:274s
fi-bxt-dsi total:289 pass:259 dwarn:0 dfail:0 fail:0 skip:30 time:506s
fi-bxt-j4205 total:12 pass:2 dwarn:0 dfail:0 fail:0 skip:9
fi-byt-j1900 total:289 pass:254 dwarn:0 dfail:0 fail:0 skip:35 time:494s
fi-byt-n2820 total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:485s
fi-elk-e7500 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:438s
fi-gdg-551 total:289 pass:178 dwarn:1 dfail:0 fail:1 skip:109 time:264s
fi-glk-1 total:12 pass:2 dwarn:0 dfail:0 fail:0 skip:9
fi-hsw-4770 total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:431s
fi-hsw-4770r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:439s
fi-ilk-650 total:1 pass:0 dwarn:0 dfail:0 fail:0 skip:0
fi-ivb-3520m total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:480s
fi-ivb-3770 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:462s
fi-kbl-7500u total:1 pass:0 dwarn:0 dfail:0 fail:0 skip:0
fi-kbl-7567u total:12 pass:2 dwarn:0 dfail:0 fail:0 skip:9
fi-kbl-r total:12 pass:2 dwarn:0 dfail:0 fail:0 skip:9
fi-pnv-d510 total:289 pass:222 dwarn:1 dfail:0 fail:0 skip:66 time:567s
fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:460s
fi-skl-6600u total:1 pass:0 dwarn:0 dfail:0 fail:0 skip:0
fi-skl-6700hq total:1 pass:0 dwarn:0 dfail:0 fail:0 skip:0
fi-skl-6770hq total:1 pass:0 dwarn:0 dfail:0 fail:0 skip:0
fi-snb-2520m total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:557s
fi-snb-2600 total:289 pass:249 dwarn:0 dfail:0 fail:0 skip:40 time:418s
Blacklisted hosts:
fi-cnl-y total:1 pass:0 dwarn:0 dfail:0 fail:0 skip:0
fi-glk-dsi total:207 pass:184 dwarn:0 dfail:0 fail:0 skip:22
fi-bdw-gvtdvm failed to connect after reboot
748f2c6b4046b23a623b4af3799563ef3110bb0d drm-tip: 2017y-11m-08d-07h-50m-13s UTC integration manifest
9a1a28f12330 drm/i915: sync dp link status checks against atomic commmits
7e50eec7a85a drm/atomic-helper: always track connector commits, too
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7012/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] drm/atomic-helper: always track connector commits, too
2017-11-08 10:54 [PATCH 1/2] drm/atomic-helper: always track connector commits, too Daniel Vetter
` (4 preceding siblings ...)
2017-11-08 15:14 ` ✗ Fi.CI.BAT: failure for series starting with [1/2] drm/atomic-helper: always track connector commits, too (rev2) Patchwork
@ 2017-11-08 15:21 ` Ville Syrjälä
2017-11-08 20:06 ` Manasi Navare
2017-11-09 8:57 ` [PATCH] " Daniel Vetter
2017-11-09 9:56 ` ✗ Fi.CI.BAT: failure for series starting with drm/atomic-helper: always track connector commits, too (rev3) Patchwork
7 siblings, 1 reply; 20+ messages in thread
From: Ville Syrjälä @ 2017-11-08 15:21 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Daniel Vetter, Intel Graphics Development, DRI Development
On Wed, Nov 08, 2017 at 11:54:49AM +0100, Daniel Vetter wrote:
> It's useful for syncing async connector work like link retraining.
>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 71d712f1b56a..ea781d55f2c1 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1791,7 +1791,8 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state,
> if (new_conn_state->crtc)
> continue;
This guy says we will never have new_conn_state->crtc below.
>
> - commit = crtc_or_fake_commit(state, old_conn_state->crtc);
> + /* Always track connectors explicitly for e.g. link retraining. */
> + commit = crtc_or_fake_commit(state, new_conn_state->crtc ?: old_conn_state->crtc);
> if (!commit)
> return -ENOMEM;
>
> @@ -1805,10 +1806,7 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state,
> !try_wait_for_completion(&old_plane_state->commit->flip_done))
> return -EBUSY;
>
> - /*
> - * Unlike connectors, always track planes explicitly for
> - * async pageflip support.
> - */
> + /* Always track planes explicitly for async pageflip support. */
> commit = crtc_or_fake_commit(state, new_plane_state->crtc ?: old_plane_state->crtc);
> if (!commit)
> return -ENOMEM;
> --
> 2.15.0
--
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] 20+ messages in thread
* Re: [PATCH] drm/i915: sync dp link status checks against atomic commmits
2017-11-08 13:28 ` Daniel Vetter
@ 2017-11-08 18:09 ` Manasi Navare
2017-11-08 20:52 ` Manasi Navare
0 siblings, 1 reply; 20+ messages in thread
From: Manasi Navare @ 2017-11-08 18:09 UTC (permalink / raw)
To: Daniel Vetter
Cc: Daniel Vetter, Intel Graphics Development, DRI Development,
Daniel Vetter
On Wed, Nov 08, 2017 at 02:28:23PM +0100, Daniel Vetter wrote:
> On Wed, Nov 08, 2017 at 03:26:15PM +0200, Ville Syrjälä wrote:
> > On Wed, Nov 08, 2017 at 02:11:46PM +0100, Daniel Vetter wrote:
> > > On Wed, Nov 08, 2017 at 03:04:58PM +0200, Ville Syrjälä wrote:
> > > > On Wed, Nov 08, 2017 at 01:57:50PM +0100, Daniel Vetter wrote:
> > > > > Two bits:
> > > > > - check actual atomic state, the legacy stuff can only be looked at
> > > > > from within the atomic_commit_tail function, since it's only
> > > > > protected by ordering and not by any locks.
> > > > >
> > > > > - Make sure we don't wreak the work an ongoing nonblocking commit is
> > > > > doing.
> > > >
> > > > I still think we need to move the retraining to the hotplug work.
> > >
> > > Why? One of the patch series Maarten mentioned says there's a deadlock
> > > with dp aux, but it seems to be all just fine when CI retrains.
This retraining here would be not because of the training failing in the commit but
when we get a short pulse right so when the link was already up and running?
> >
> > I guess the deadlock is possible only with MST, maybe. Currently MST
> > has it's own copy of the retraining code without the locks.
> >
> > At one point I started to rewrite the entire sink irq handling into a
> > much nicer shape, also unifying the approach between MST and SST. IIRC
> > I did still make the mistake of having some kind of higher level MST
> > vs. SST split, but I think the proper solution is to combine the two
> > almost entirely. I think we should even be using the ESI registers
> > with SST for DPCD 1.2+. Currently we use ESI for MST and non-ESI for
> > SST. Sadly I've not found the time to continue that work (same story
> > with the "shutdown displays prior to rebooting to keep Dell MST
> > monitors working" thing).
>
> Yeah, merging sst and mst more definitely sounds like a good idea. I've
> also been toying with it since forever.
>
> > > And even if there is a deadlock, it's there already, so not sure why we
> > > need to block this bugfix on a different bugfix. Which seems to be what
> > > you're suggesting here (but it's a bit sparse).
> >
> > I guess what I'm really saying is that we shouldn't stop here.
>
> Fully agreed.
> -Daniel
>
> >
> > > -Daniel
> > >
> > > > > v2: We need the crtc lock too, because a plane update might change it
> > > > > without having to acquire the connection_mutex (Maarten). Use
> > > > > Maarten's changes for this locking, while keeping the logic that uses
> > > > > the connection->commit->hw_done signal for syncing with nonblocking
> > > > > commits.
> > > > >
> > > > > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > > > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > References: https://bugs.freedesktop.org/show_bug.cgi?id=103336
> > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99272
> > > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > > > > ---
> > > > > drivers/gpu/drm/i915/intel_dp.c | 56 ++++++++++++++++++++++++++++++++++++-----
> > > > > 1 file changed, 50 insertions(+), 6 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > > > index d27c0145ac91..7cd7ab4fb431 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > > @@ -4319,6 +4319,8 @@ static void
> > > > > intel_dp_check_link_status(struct intel_dp *intel_dp)
> > > > > {
> > > > > struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base;
I dont think we need intel_encoder, can we remove this?
Manasi
> > > > > + struct drm_connector_state *conn_state =
> > > > > + intel_dp->attached_connector->base.state;
> > > > > struct drm_device *dev = intel_dp_to_dev(intel_dp);
> > > > > u8 link_status[DP_LINK_STATUS_SIZE];
> > > > >
> > > > > @@ -4329,10 +4331,15 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
> > > > > return;
> > > > > }
> > > > >
> > > > > - if (!intel_encoder->base.crtc)
> > > > > + if (!conn_state->crtc)
> > > > > return;
> > > > >
> > > > > - if (!to_intel_crtc(intel_encoder->base.crtc)->active)
> > > > > + WARN_ON(!drm_modeset_is_locked(&conn_state->crtc->mutex));
> > > > > +
> > > > > + if (!conn_state->crtc->state->active)
> > > > > + return;
> > > > > +
> > > > > + if (!try_wait_for_completion(&conn_state->commit->hw_done))
> > > > > return;
> > > > >
> > > > > /*
> > > > > @@ -4368,7 +4375,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;
> > > > > @@ -4408,9 +4414,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 */
> > > > > @@ -4860,8 +4865,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;
> > > > >
> > > > > @@ -5146,10 +5162,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.15.0
> > > >
> > > > --
> > > > Ville Syrjälä
> > > > Intel OTC
> > >
> > > --
> > > Daniel Vetter
> > > Software Engineer, Intel Corporation
> > > http://blog.ffwll.ch
> >
> > --
> > Ville Syrjälä
> > Intel OTC
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] drm/atomic-helper: always track connector commits, too
2017-11-08 15:21 ` [PATCH 1/2] drm/atomic-helper: always track connector commits, too Ville Syrjälä
@ 2017-11-08 20:06 ` Manasi Navare
0 siblings, 0 replies; 20+ messages in thread
From: Manasi Navare @ 2017-11-08 20:06 UTC (permalink / raw)
To: Ville Syrjälä
Cc: Daniel Vetter, Intel Graphics Development, DRI Development,
Daniel Vetter
On Wed, Nov 08, 2017 at 05:21:04PM +0200, Ville Syrjälä wrote:
> On Wed, Nov 08, 2017 at 11:54:49AM +0100, Daniel Vetter wrote:
> > It's useful for syncing async connector work like link retraining.
> >
> > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> > drivers/gpu/drm/drm_atomic_helper.c | 8 +++-----
> > 1 file changed, 3 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> > index 71d712f1b56a..ea781d55f2c1 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -1791,7 +1791,8 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state,
> > if (new_conn_state->crtc)
> > continue;
>
> This guy says we will never have new_conn_state->crtc below.
>
> >
> > - commit = crtc_or_fake_commit(state, old_conn_state->crtc);
> > + /* Always track connectors explicitly for e.g. link retraining. */
> > + commit = crtc_or_fake_commit(state, new_conn_state->crtc ?: old_conn_state->crtc);
Yes exactly, I had the same thought. Control will never reach here if new_conn_state->crtc is true.
If we want to track it explicitly for link retraining pending modeset retry work then shouldnt we
remove the remove the if (new_conn_state->crtc) continue part?
Manasi
> > if (!commit)
> > return -ENOMEM;
> >
> > @@ -1805,10 +1806,7 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state,
> > !try_wait_for_completion(&old_plane_state->commit->flip_done))
> > return -EBUSY;
> >
> > - /*
> > - * Unlike connectors, always track planes explicitly for
> > - * async pageflip support.
> > - */
> > + /* Always track planes explicitly for async pageflip support. */
> > commit = crtc_or_fake_commit(state, new_plane_state->crtc ?: old_plane_state->crtc);
> > if (!commit)
> > return -ENOMEM;
> > --
> > 2.15.0
>
> --
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] drm/i915: sync dp link status checks against atomic commmits
2017-11-08 18:09 ` Manasi Navare
@ 2017-11-08 20:52 ` Manasi Navare
0 siblings, 0 replies; 20+ messages in thread
From: Manasi Navare @ 2017-11-08 20:52 UTC (permalink / raw)
To: Daniel Vetter
Cc: Daniel Vetter, Intel Graphics Development, DRI Development,
Daniel Vetter
On Wed, Nov 08, 2017 at 10:09:32AM -0800, Manasi Navare wrote:
> On Wed, Nov 08, 2017 at 02:28:23PM +0100, Daniel Vetter wrote:
> > On Wed, Nov 08, 2017 at 03:26:15PM +0200, Ville Syrjälä wrote:
> > > On Wed, Nov 08, 2017 at 02:11:46PM +0100, Daniel Vetter wrote:
> > > > On Wed, Nov 08, 2017 at 03:04:58PM +0200, Ville Syrjälä wrote:
> > > > > On Wed, Nov 08, 2017 at 01:57:50PM +0100, Daniel Vetter wrote:
> > > > > > Two bits:
> > > > > > - check actual atomic state, the legacy stuff can only be looked at
> > > > > > from within the atomic_commit_tail function, since it's only
> > > > > > protected by ordering and not by any locks.
> > > > > >
> > > > > > - Make sure we don't wreak the work an ongoing nonblocking commit is
> > > > > > doing.
> > > > >
> > > > > I still think we need to move the retraining to the hotplug work.
> > > >
> > > > Why? One of the patch series Maarten mentioned says there's a deadlock
> > > > with dp aux, but it seems to be all just fine when CI retrains.
>
> This retraining here would be not because of the training failing in the commit but
> when we get a short pulse right so when the link was already up and running?
>
> > >
> > > I guess the deadlock is possible only with MST, maybe. Currently MST
> > > has it's own copy of the retraining code without the locks.
> > >
> > > At one point I started to rewrite the entire sink irq handling into a
> > > much nicer shape, also unifying the approach between MST and SST. IIRC
> > > I did still make the mistake of having some kind of higher level MST
> > > vs. SST split, but I think the proper solution is to combine the two
> > > almost entirely. I think we should even be using the ESI registers
> > > with SST for DPCD 1.2+. Currently we use ESI for MST and non-ESI for
> > > SST. Sadly I've not found the time to continue that work (same story
> > > with the "shutdown displays prior to rebooting to keep Dell MST
> > > monitors working" thing).
> >
> > Yeah, merging sst and mst more definitely sounds like a good idea. I've
> > also been toying with it since forever.
> >
> > > > And even if there is a deadlock, it's there already, so not sure why we
> > > > need to block this bugfix on a different bugfix. Which seems to be what
> > > > you're suggesting here (but it's a bit sparse).
> > >
> > > I guess what I'm really saying is that we shouldn't stop here.
> >
> > Fully agreed.
> > -Daniel
> >
> > >
> > > > -Daniel
> > > >
> > > > > > v2: We need the crtc lock too, because a plane update might change it
> > > > > > without having to acquire the connection_mutex (Maarten). Use
> > > > > > Maarten's changes for this locking, while keeping the logic that uses
> > > > > > the connection->commit->hw_done signal for syncing with nonblocking
> > > > > > commits.
> > > > > >
> > > > > > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > > > > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > > References: https://bugs.freedesktop.org/show_bug.cgi?id=103336
> > > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99272
> > > > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > > > > > ---
> > > > > > drivers/gpu/drm/i915/intel_dp.c | 56 ++++++++++++++++++++++++++++++++++++-----
> > > > > > 1 file changed, 50 insertions(+), 6 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > > > > index d27c0145ac91..7cd7ab4fb431 100644
> > > > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > > > @@ -4319,6 +4319,8 @@ static void
> > > > > > intel_dp_check_link_status(struct intel_dp *intel_dp)
> > > > > > {
> > > > > > struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base;
>
> I dont think we need intel_encoder, can we remove this?
>
> Manasi
>
Sorry didnt realize we still need to use intel_encoder in the DRM_DEBUG_KMS.
Manasi
> > > > > > + struct drm_connector_state *conn_state =
> > > > > > + intel_dp->attached_connector->base.state;
> > > > > > struct drm_device *dev = intel_dp_to_dev(intel_dp);
> > > > > > u8 link_status[DP_LINK_STATUS_SIZE];
> > > > > >
> > > > > > @@ -4329,10 +4331,15 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
> > > > > > return;
> > > > > > }
> > > > > >
> > > > > > - if (!intel_encoder->base.crtc)
> > > > > > + if (!conn_state->crtc)
> > > > > > return;
> > > > > >
> > > > > > - if (!to_intel_crtc(intel_encoder->base.crtc)->active)
> > > > > > + WARN_ON(!drm_modeset_is_locked(&conn_state->crtc->mutex));
> > > > > > +
> > > > > > + if (!conn_state->crtc->state->active)
> > > > > > + return;
> > > > > > +
> > > > > > + if (!try_wait_for_completion(&conn_state->commit->hw_done))
> > > > > > return;
> > > > > >
> > > > > > /*
> > > > > > @@ -4368,7 +4375,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;
> > > > > > @@ -4408,9 +4414,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 */
> > > > > > @@ -4860,8 +4865,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;
> > > > > >
> > > > > > @@ -5146,10 +5162,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.15.0
> > > > >
> > > > > --
> > > > > Ville Syrjälä
> > > > > Intel OTC
> > > >
> > > > --
> > > > Daniel Vetter
> > > > Software Engineer, Intel Corporation
> > > > http://blog.ffwll.ch
> > >
> > > --
> > > Ville Syrjälä
> > > Intel OTC
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
> _______________________________________________
> 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] 20+ messages in thread
* [PATCH] drm/atomic-helper: always track connector commits, too
2017-11-08 10:54 [PATCH 1/2] drm/atomic-helper: always track connector commits, too Daniel Vetter
` (5 preceding siblings ...)
2017-11-08 15:21 ` [PATCH 1/2] drm/atomic-helper: always track connector commits, too Ville Syrjälä
@ 2017-11-09 8:57 ` Daniel Vetter
2017-11-09 9:56 ` ✗ Fi.CI.BAT: failure for series starting with drm/atomic-helper: always track connector commits, too (rev3) Patchwork
7 siblings, 0 replies; 20+ messages in thread
From: Daniel Vetter @ 2017-11-09 8:57 UTC (permalink / raw)
To: DRI Development; +Cc: Daniel Vetter, Intel Graphics Development, Daniel Vetter
It's useful for syncing async connector work like link retraining.
v2: Make it work (Manasi&Ville)
Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
drivers/gpu/drm/drm_atomic_helper.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index c18c271e7508..ced1ac8e68a0 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1793,11 +1793,8 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state,
!try_wait_for_completion(&old_conn_state->commit->flip_done))
return -EBUSY;
- /* commit tracked through new_crtc_state->commit, no need to do it explicitly */
- if (new_conn_state->crtc)
- continue;
-
- commit = crtc_or_fake_commit(state, old_conn_state->crtc);
+ /* Always track connectors explicitly for e.g. link retraining. */
+ commit = crtc_or_fake_commit(state, new_conn_state->crtc ?: old_conn_state->crtc);
if (!commit)
return -ENOMEM;
@@ -1811,10 +1808,7 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state,
!try_wait_for_completion(&old_plane_state->commit->flip_done))
return -EBUSY;
- /*
- * Unlike connectors, always track planes explicitly for
- * async pageflip support.
- */
+ /* Always track planes explicitly for async pageflip support. */
commit = crtc_or_fake_commit(state, new_plane_state->crtc ?: old_plane_state->crtc);
if (!commit)
return -ENOMEM;
--
2.15.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* ✗ Fi.CI.BAT: failure for series starting with drm/atomic-helper: always track connector commits, too (rev3)
2017-11-08 10:54 [PATCH 1/2] drm/atomic-helper: always track connector commits, too Daniel Vetter
` (6 preceding siblings ...)
2017-11-09 8:57 ` [PATCH] " Daniel Vetter
@ 2017-11-09 9:56 ` Patchwork
7 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2017-11-09 9:56 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
== Series Details ==
Series: series starting with drm/atomic-helper: always track connector commits, too (rev3)
URL : https://patchwork.freedesktop.org/series/33410/
State : failure
== Summary ==
Series 33410v3 series starting with drm/atomic-helper: always track connector commits, too
https://patchwork.freedesktop.org/api/1.0/series/33410/revisions/3/mbox/
Test chamelium:
Subgroup dp-hpd-fast:
skip -> INCOMPLETE (fi-skl-6600u)
skip -> INCOMPLETE (fi-skl-6770hq)
pass -> INCOMPLETE (fi-kbl-7500u) fdo#102332
Test debugfs_test:
Subgroup read_all_entries:
pass -> INCOMPLETE (fi-bxt-j4205)
pass -> INCOMPLETE (fi-kbl-7560u)
pass -> INCOMPLETE (fi-kbl-7567u)
pass -> INCOMPLETE (fi-kbl-r)
pass -> INCOMPLETE (fi-glk-1)
Test gem_ctx_basic:
fail -> SKIP (fi-gdg-551)
Test gem_exec_reloc:
Subgroup basic-cpu-active:
fail -> PASS (fi-gdg-551) fdo#102582 +4
Test gem_sync:
Subgroup basic-store-all:
fail -> PASS (fi-ivb-3520m) fdo#100007
fdo#102332 https://bugs.freedesktop.org/show_bug.cgi?id=102332
fdo#102582 https://bugs.freedesktop.org/show_bug.cgi?id=102582
fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007
fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:444s
fi-bdw-gvtdvm total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:454s
fi-blb-e6850 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:380s
fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:533s
fi-bwr-2160 total:289 pass:183 dwarn:0 dfail:0 fail:0 skip:106 time:274s
fi-bxt-dsi total:289 pass:259 dwarn:0 dfail:0 fail:0 skip:30 time:503s
fi-bxt-j4205 total:12 pass:2 dwarn:0 dfail:0 fail:0 skip:9
fi-byt-j1900 total:289 pass:254 dwarn:0 dfail:0 fail:0 skip:35 time:492s
fi-byt-n2820 total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:488s
fi-elk-e7500 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:426s
fi-gdg-551 total:289 pass:178 dwarn:1 dfail:0 fail:1 skip:109 time:260s
fi-glk-1 total:12 pass:2 dwarn:0 dfail:0 fail:0 skip:9
fi-hsw-4770 total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:428s
fi-hsw-4770r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:437s
fi-ilk-650 total:289 pass:228 dwarn:0 dfail:0 fail:0 skip:61 time:430s
fi-ivb-3520m total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:487s
fi-ivb-3770 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:458s
fi-kbl-7500u total:1 pass:0 dwarn:0 dfail:0 fail:0 skip:0
fi-kbl-7560u total:12 pass:2 dwarn:0 dfail:0 fail:0 skip:9
fi-kbl-7567u total:12 pass:2 dwarn:0 dfail:0 fail:0 skip:9
fi-kbl-r total:12 pass:2 dwarn:0 dfail:0 fail:0 skip:9
fi-pnv-d510 total:289 pass:222 dwarn:1 dfail:0 fail:0 skip:66 time:570s
fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:453s
fi-skl-6600u total:1 pass:0 dwarn:0 dfail:0 fail:0 skip:0
fi-skl-6700hq total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:562s
fi-skl-6700k total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:521s
fi-skl-6770hq total:1 pass:0 dwarn:0 dfail:0 fail:0 skip:0
fi-skl-gvtdvm total:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:459s
fi-snb-2520m total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:559s
fi-snb-2600 total:289 pass:249 dwarn:0 dfail:0 fail:0 skip:40 time:419s
Blacklisted hosts:
fi-cnl-y total:12 pass:2 dwarn:0 dfail:0 fail:0 skip:9
fi-glk-dsi total:289 pass:259 dwarn:0 dfail:0 fail:0 skip:30 time:488s
65dc54b704d3ee0486f9f5b11f00c28973f783a2 drm-tip: 2017y-11m-09d-08h-53m-46s UTC integration manifest
4c9c8b1604ad drm/i915: sync dp link status checks against atomic commmits
db34499804aa drm/atomic-helper: always track connector commits, too
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7028/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] drm/atomic-helper: always track connector commits, too
2017-11-10 10:53 [PATCH 1/2] drm/atomic-helper: always track connector commits, too Daniel Vetter
@ 2017-11-15 11:39 ` Maarten Lankhorst
0 siblings, 0 replies; 20+ messages in thread
From: Maarten Lankhorst @ 2017-11-15 11:39 UTC (permalink / raw)
To: Daniel Vetter, DRI Development; +Cc: Daniel Vetter, Intel Graphics Development
Op 10-11-17 om 11:53 schreef Daniel Vetter:
> It's useful for syncing async connector work like link retraining.
>
> v2: Make it work (Manasi&Ville)
>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 12 +++---------
> 1 file changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index c18c271e7508..ced1ac8e68a0 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1793,11 +1793,8 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state,
> !try_wait_for_completion(&old_conn_state->commit->flip_done))
> return -EBUSY;
>
> - /* commit tracked through new_crtc_state->commit, no need to do it explicitly */
> - if (new_conn_state->crtc)
> - continue;
> -
> - commit = crtc_or_fake_commit(state, old_conn_state->crtc);
> + /* Always track connectors explicitly for e.g. link retraining. */
> + commit = crtc_or_fake_commit(state, new_conn_state->crtc ?: old_conn_state->crtc);
> if (!commit)
> return -ENOMEM;
>
> @@ -1811,10 +1808,7 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state,
> !try_wait_for_completion(&old_plane_state->commit->flip_done))
> return -EBUSY;
>
> - /*
> - * Unlike connectors, always track planes explicitly for
> - * async pageflip support.
> - */
> + /* Always track planes explicitly for async pageflip support. */
> commit = crtc_or_fake_commit(state, new_plane_state->crtc ?: old_plane_state->crtc);
> if (!commit)
> return -ENOMEM;
First patch reviewed and pushed to drm-misc-next. Will wait for CI_DRM results before pushing the second patch, though I don't expect anything breaking from this. :-)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/2] drm/atomic-helper: always track connector commits, too
@ 2017-11-10 10:53 Daniel Vetter
2017-11-15 11:39 ` Maarten Lankhorst
0 siblings, 1 reply; 20+ messages in thread
From: Daniel Vetter @ 2017-11-10 10:53 UTC (permalink / raw)
To: DRI Development; +Cc: Daniel Vetter, Intel Graphics Development, Daniel Vetter
It's useful for syncing async connector work like link retraining.
v2: Make it work (Manasi&Ville)
Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
drivers/gpu/drm/drm_atomic_helper.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index c18c271e7508..ced1ac8e68a0 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1793,11 +1793,8 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state,
!try_wait_for_completion(&old_conn_state->commit->flip_done))
return -EBUSY;
- /* commit tracked through new_crtc_state->commit, no need to do it explicitly */
- if (new_conn_state->crtc)
- continue;
-
- commit = crtc_or_fake_commit(state, old_conn_state->crtc);
+ /* Always track connectors explicitly for e.g. link retraining. */
+ commit = crtc_or_fake_commit(state, new_conn_state->crtc ?: old_conn_state->crtc);
if (!commit)
return -ENOMEM;
@@ -1811,10 +1808,7 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state,
!try_wait_for_completion(&old_plane_state->commit->flip_done))
return -EBUSY;
- /*
- * Unlike connectors, always track planes explicitly for
- * async pageflip support.
- */
+ /* Always track planes explicitly for async pageflip support. */
commit = crtc_or_fake_commit(state, new_plane_state->crtc ?: old_plane_state->crtc);
if (!commit)
return -ENOMEM;
--
2.15.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
end of thread, other threads:[~2017-11-15 11:39 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-08 10:54 [PATCH 1/2] drm/atomic-helper: always track connector commits, too Daniel Vetter
2017-11-08 10:54 ` [PATCH 2/2] drm/i915: sync dp link status checks against atomic commmits Daniel Vetter
2017-11-08 12:57 ` [PATCH] " Daniel Vetter
2017-11-08 13:04 ` Ville Syrjälä
2017-11-08 13:11 ` Daniel Vetter
2017-11-08 13:26 ` Ville Syrjälä
2017-11-08 13:28 ` Daniel Vetter
2017-11-08 18:09 ` Manasi Navare
2017-11-08 20:52 ` Manasi Navare
2017-11-08 13:13 ` Maarten Lankhorst
2017-11-08 11:18 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/atomic-helper: always track connector commits, too Patchwork
2017-11-08 11:32 ` ✗ Fi.CI.BAT: failure " Patchwork
2017-11-08 12:16 ` Patchwork
2017-11-08 15:14 ` ✗ Fi.CI.BAT: failure for series starting with [1/2] drm/atomic-helper: always track connector commits, too (rev2) Patchwork
2017-11-08 15:21 ` [PATCH 1/2] drm/atomic-helper: always track connector commits, too Ville Syrjälä
2017-11-08 20:06 ` Manasi Navare
2017-11-09 8:57 ` [PATCH] " Daniel Vetter
2017-11-09 9:56 ` ✗ Fi.CI.BAT: failure for series starting with drm/atomic-helper: always track connector commits, too (rev3) Patchwork
2017-11-10 10:53 [PATCH 1/2] drm/atomic-helper: always track connector commits, too Daniel Vetter
2017-11-15 11:39 ` 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.