]>
Commit | Line | Data |
---|---|---|
e2020d86 GKH |
1 | From 2216322919c8608a448d7ebc560a845238a5d6b6 Mon Sep 17 00:00:00 2001 |
2 | From: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com> | |
3 | Date: Mon, 7 Jan 2019 12:41:46 -0500 | |
4 | Subject: drm: Block fb changes for async plane updates | |
5 | ||
6 | From: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com> | |
7 | ||
8 | commit 2216322919c8608a448d7ebc560a845238a5d6b6 upstream. | |
9 | ||
10 | The prepare_fb call always happens on new_plane_state. | |
11 | ||
12 | The drm_atomic_helper_cleanup_planes checks to see if | |
13 | plane state pointer has changed when deciding to call cleanup_fb on | |
14 | either the new_plane_state or the old_plane_state. | |
15 | ||
16 | For a non-async atomic commit the state pointer is swapped, so this | |
17 | helper calls prepare_fb on the new_plane_state and cleanup_fb on the | |
18 | old_plane_state. This makes sense, since we want to prepare the | |
19 | framebuffer we are going to use and cleanup the the framebuffer we are | |
20 | no longer using. | |
21 | ||
22 | For the async atomic update helpers this differs. The async atomic | |
23 | update helpers perform in-place updates on the existing state. They call | |
24 | drm_atomic_helper_cleanup_planes but the state pointer is not swapped. | |
25 | This means that prepare_fb is called on the new_plane_state and | |
26 | cleanup_fb is called on the new_plane_state (not the old). | |
27 | ||
28 | In the case where old_plane_state->fb == new_plane_state->fb then | |
29 | there should be no behavioral difference between an async update | |
30 | and a non-async commit. But there are issues that arise when | |
31 | old_plane_state->fb != new_plane_state->fb. | |
32 | ||
33 | The first is that the new_plane_state->fb is immediately cleaned up | |
34 | after it has been prepared, so we're using a fb that we shouldn't | |
35 | be. | |
36 | ||
37 | The second occurs during a sequence of async atomic updates and | |
38 | non-async regular atomic commits. Suppose there are two framebuffers | |
39 | being interleaved in a double-buffering scenario, fb1 and fb2: | |
40 | ||
41 | - Async update, oldfb = NULL, newfb = fb1, prepare fb1, cleanup fb1 | |
42 | - Async update, oldfb = fb1, newfb = fb2, prepare fb2, cleanup fb2 | |
43 | - Non-async commit, oldfb = fb2, newfb = fb1, prepare fb1, cleanup fb2 | |
44 | ||
45 | We call cleanup_fb on fb2 twice in this example scenario, and any | |
46 | further use will result in use-after-free. | |
47 | ||
48 | The simple fix to this problem is to block framebuffer changes | |
49 | in the drm_atomic_helper_async_check function for now. | |
50 | ||
51 | v2: Move check by itself, add a FIXME (Daniel) | |
52 | ||
53 | Cc: Daniel Vetter <daniel.vetter@ffwll.ch> | |
54 | Cc: Harry Wentland <harry.wentland@amd.com> | |
55 | Cc: Andrey Grodzovsky <andrey.grodzovsky@amd.com> | |
56 | Cc: <stable@vger.kernel.org> # v4.14+ | |
57 | Fixes: fef9df8b5945 ("drm/atomic: initial support for asynchronous plane update") | |
58 | Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com> | |
59 | Acked-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com> | |
60 | Acked-by: Harry Wentland <harry.wentland@amd.com> | |
61 | Reviewed-by: Daniel Vetter <daniel@ffwll.ch> | |
62 | Signed-off-by: Harry Wentland <harry.wentland@amd.com> | |
63 | Link: https://patchwork.freedesktop.org/patch/275364/ | |
64 | Signed-off-by: Dave Airlie <airlied@redhat.com> | |
65 | Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> | |
66 | ||
67 | --- | |
68 | drivers/gpu/drm/drm_atomic_helper.c | 9 +++++++++ | |
69 | 1 file changed, 9 insertions(+) | |
70 | ||
71 | --- a/drivers/gpu/drm/drm_atomic_helper.c | |
72 | +++ b/drivers/gpu/drm/drm_atomic_helper.c | |
73 | @@ -1564,6 +1564,15 @@ int drm_atomic_helper_async_check(struct | |
74 | old_plane_state->crtc != new_plane_state->crtc) | |
75 | return -EINVAL; | |
76 | ||
77 | + /* | |
78 | + * FIXME: Since prepare_fb and cleanup_fb are always called on | |
79 | + * the new_plane_state for async updates we need to block framebuffer | |
80 | + * changes. This prevents use of a fb that's been cleaned up and | |
81 | + * double cleanups from occuring. | |
82 | + */ | |
83 | + if (old_plane_state->fb != new_plane_state->fb) | |
84 | + return -EINVAL; | |
85 | + | |
86 | funcs = plane->helper_private; | |
87 | if (!funcs->atomic_async_update) | |
88 | return -EINVAL; |