Skip to content

Commit 575197e

Browse files
danvetpelwell
authored andcommitted
drm/atomic-helpers: remove legacy_cursor_update hacks
The stuff never really worked, and leads to lots of fun because it out-of-order frees atomic states. Which upsets KASAN, among other things. For async updates we now have a more solid solution with the ->atomic_async_check and ->atomic_async_commit hooks. Support for that for msm and vc4 landed. nouveau and i915 have their own commit routines, doing something similar. For everyone else it's probably better to remove the use-after-free bug, and encourage folks to use the async support instead. The affected drivers which register a legacy cursor plane and don't either use the new async stuff or their own commit routine are: amdgpu, atmel, mediatek, qxl, rockchip, sti, sun4i, tegra, virtio, and vmwgfx. Inspired by an amdgpu bug report. v2: Drop RFC, I think with amdgpu converted over to use atomic_async_check/commit done in commit 674e78a Author: Nicholas Kazlauskas <[email protected]> Date: Wed Dec 5 14:59:07 2018 -0500 drm/amd/display: Add fast path for cursor plane updates we don't have any driver anymore where we have userspace expecting solid legacy cursor support _and_ they are using the atomic helpers in their fully glory. So we can retire this. v3: Paper over msm and i915 regression. The complete_all is the only thing missing afaict. v4: Rebased on recent kernel, added extra link for vc4 bug. Link: https://bugzilla.kernel.org/show_bug.cgi?id=199425 Link: https://lore.kernel.org/all/[email protected]/ Cc: [email protected] Cc: Michel Dänzer <[email protected]> Cc: [email protected] Cc: Rob Clark <[email protected]> Cc: "Kazlauskas, Nicholas" <[email protected]> Tested-by: Maxime Ripard <[email protected]> Signed-off-by: Daniel Vetter <[email protected]> Signed-off-by: Maxime Ripard <[email protected]>
1 parent 3dd028b commit 575197e

File tree

3 files changed

+15
-13
lines changed

3 files changed

+15
-13
lines changed

drivers/gpu/drm/drm_atomic_helper.c

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1488,13 +1488,6 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device *dev,
14881488
int i, ret;
14891489
unsigned int crtc_mask = 0;
14901490

1491-
/*
1492-
* Legacy cursor ioctls are completely unsynced, and userspace
1493-
* relies on that (by doing tons of cursor updates).
1494-
*/
1495-
if (old_state->legacy_cursor_update)
1496-
return;
1497-
14981491
for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state, new_crtc_state, i) {
14991492
if (!new_crtc_state->active)
15001493
continue;
@@ -2123,12 +2116,6 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state,
21232116
continue;
21242117
}
21252118

2126-
/* Legacy cursor updates are fully unsynced. */
2127-
if (state->legacy_cursor_update) {
2128-
complete_all(&commit->flip_done);
2129-
continue;
2130-
}
2131-
21322119
if (!new_crtc_state->event) {
21332120
commit->event = kzalloc(sizeof(*commit->event),
21342121
GFP_KERNEL);

drivers/gpu/drm/i915/display/intel_display.c

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10821,6 +10821,19 @@ static int intel_atomic_commit(struct drm_device *dev,
1082110821
state->base.legacy_cursor_update = false;
1082210822
}
1082310823

10824+
/*
10825+
* FIXME: Cut over to (async) commit helpers instead of hand-rolling
10826+
* everything.
10827+
*/
10828+
if (state->base.legacy_cursor_update) {
10829+
struct intel_crtc_state *new_crtc_state;
10830+
struct intel_crtc *crtc;
10831+
int i;
10832+
10833+
for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i)
10834+
complete_all(&new_crtc_state->uapi.commit->flip_done);
10835+
}
10836+
1082410837
ret = intel_atomic_prepare_commit(state);
1082510838
if (ret) {
1082610839
drm_dbg_atomic(&dev_priv->drm,

drivers/gpu/drm/msm/msm_atomic.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,8 @@ void msm_atomic_commit_tail(struct drm_atomic_state *state)
246246
/* async updates are limited to single-crtc updates: */
247247
WARN_ON(crtc_mask != drm_crtc_mask(async_crtc));
248248

249+
complete_all(&async_crtc->state->commit->flip_done);
250+
249251
/*
250252
* Start timer if we don't already have an update pending
251253
* on this crtc:

0 commit comments

Comments
 (0)