queue-4.19/drm-don-t-block-fb-changes-for-async-plane-updates.patch

   1 From 89a4aac0ab0e6f5eea10d7bf4869dd15c3de2cd4 Mon Sep 17 00:00:00 2001
   2 From: Helen Koike <helen.koike@collabora.com>
   3 Date: Mon, 3 Jun 2019 13:56:10 -0300
   4 Subject: drm: don't block fb changes for async plane updates
   5
   6 From: Helen Koike <helen.koike@collabora.com>
   7
   8 commit 89a4aac0ab0e6f5eea10d7bf4869dd15c3de2cd4 upstream.
   9
  10 In the case of a normal sync update, the preparation of framebuffers (be
  11 it calling drm_atomic_helper_prepare_planes() or doing setups with
  12 drm_framebuffer_get()) are performed in the new_state and the respective
  13 cleanups are performed in the old_state.
  14
  15 In the case of async updates, the preparation is also done in the
  16 new_state but the cleanups are done in the new_state (because updates
  17 are performed in place, i.e. in the current state).
  18
  19 The current code blocks async udpates when the fb is changed, turning
  20 async updates into sync updates, slowing down cursor updates and
  21 introducing regressions in igt tests with errors of type:
  22
  23 "CRITICAL: completed 97 cursor updated in a period of 30 flips, we
  24 expect to complete approximately 15360 updates, with the threshold set
  25 at 7680"
  26
  27 Fb changes in async updates were prevented to avoid the following scenario:
  28
  29 - Async update, oldfb = NULL, newfb = fb1, prepare fb1, cleanup fb1
  30 - Async update, oldfb = fb1, newfb = fb2, prepare fb2, cleanup fb2
  31 - Non-async commit, oldfb = fb2, newfb = fb1, prepare fb1, cleanup fb2 (wrong)
  32 Where we have a single call to prepare fb2 but double cleanup call to fb2.
  33
  34 To solve the above problems, instead of blocking async fb changes, we
  35 place the old framebuffer in the new_state object, so when the code
  36 performs cleanups in the new_state it will cleanup the old_fb and we
  37 will have the following scenario instead:
  38
  39 - Async update, oldfb = NULL, newfb = fb1, prepare fb1, no cleanup
  40 - Async update, oldfb = fb1, newfb = fb2, prepare fb2, cleanup fb1
  41 - Non-async commit, oldfb = fb2, newfb = fb1, prepare fb1, cleanup fb2
  42
  43 Where calls to prepare/cleanup are balanced.
  44
  45 Cc: <stable@vger.kernel.org> # v4.14+
  46 Fixes: 25dc194b34dd ("drm: Block fb changes for async plane updates")
  47 Suggested-by: Boris Brezillon <boris.brezillon@collabora.com>
  48 Signed-off-by: Helen Koike <helen.koike@collabora.com>
  49 Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
  50 Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
  51 Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
  52 Link: https://patchwork.freedesktop.org/patch/msgid/20190603165610.24614-6-helen.koike@collabora.com
  53 Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  54
  55 ---
  56  drivers/gpu/drm/drm_atomic_helper.c      |   22 ++++++++++++----------
  57  include/drm/drm_modeset_helper_vtables.h |    8 ++++++++
  58  2 files changed, 20 insertions(+), 10 deletions(-)
  59
  60 --- a/drivers/gpu/drm/drm_atomic_helper.c
  61 +++ b/drivers/gpu/drm/drm_atomic_helper.c
  62 @@ -1573,15 +1573,6 @@ int drm_atomic_helper_async_check(struct
  63         if (old_plane_state->fb != new_plane_state->fb)
  64                 return -EINVAL;
  65
  66 -       /*
  67 -        * FIXME: Since prepare_fb and cleanup_fb are always called on
  68 -        * the new_plane_state for async updates we need to block framebuffer
  69 -        * changes. This prevents use of a fb that's been cleaned up and
  70 -        * double cleanups from occuring.
  71 -        */
  72 -       if (old_plane_state->fb != new_plane_state->fb)
  73 -               return -EINVAL;
  74 -
  75         funcs = plane->helper_private;
  76         if (!funcs->atomic_async_update)
  77                 return -EINVAL;
  78 @@ -1612,6 +1603,8 @@ EXPORT_SYMBOL(drm_atomic_helper_async_ch
  79   * drm_atomic_async_check() succeeds. Async commits are not supposed to swap
  80   * the states like normal sync commits, but just do in-place changes on the
  81   * current state.
  82 + *
  83 + * TODO: Implement full swap instead of doing in-place changes.
  84   */
  85  void drm_atomic_helper_async_commit(struct drm_device *dev,
  86                                     struct drm_atomic_state *state)
  87 @@ -1622,6 +1615,9 @@ void drm_atomic_helper_async_commit(stru
  88         int i;
  89
  90         for_each_new_plane_in_state(state, plane, plane_state, i) {
  91 +               struct drm_framebuffer *new_fb = plane_state->fb;
  92 +               struct drm_framebuffer *old_fb = plane->state->fb;
  93 +
  94                 funcs = plane->helper_private;
  95                 funcs->atomic_async_update(plane, plane_state);
  96
  97 @@ -1630,11 +1626,17 @@ void drm_atomic_helper_async_commit(stru
  98                  * plane->state in-place, make sure at least common
  99                  * properties have been properly updated.
 100                  */
 101 -               WARN_ON_ONCE(plane->state->fb != plane_state->fb);
 102 +               WARN_ON_ONCE(plane->state->fb != new_fb);
 103                 WARN_ON_ONCE(plane->state->crtc_x != plane_state->crtc_x);
 104                 WARN_ON_ONCE(plane->state->crtc_y != plane_state->crtc_y);
 105                 WARN_ON_ONCE(plane->state->src_x != plane_state->src_x);
 106                 WARN_ON_ONCE(plane->state->src_y != plane_state->src_y);
 107 +
 108 +               /*
 109 +                * Make sure the FBs have been swapped so that cleanups in the
 110 +                * new_state performs a cleanup in the old FB.
 111 +                */
 112 +               WARN_ON_ONCE(plane_state->fb != old_fb);
 113         }
 114  }
 115  EXPORT_SYMBOL(drm_atomic_helper_async_commit);
 116 --- a/include/drm/drm_modeset_helper_vtables.h
 117 +++ b/include/drm/drm_modeset_helper_vtables.h
 118 @@ -1174,6 +1174,14 @@ struct drm_plane_helper_funcs {
 119          * current one with the new plane configurations in the new
 120          * plane_state.
 121          *
 122 +        * Drivers should also swap the framebuffers between current plane
 123 +        * state (&drm_plane.state) and new_state.
 124 +        * This is required since cleanup for async commits is performed on
 125 +        * the new state, rather than old state like for traditional commits.
 126 +        * Since we want to give up the reference on the current (old) fb
 127 +        * instead of our brand new one, swap them in the driver during the
 128 +        * async commit.
 129 +        *
 130          * FIXME:
 131          *  - It only works for single plane updates
 132          *  - Async Pageflips are not supported yet